[SERVER-68246] change calls to `boost::optional` members `is_initialized` and `get` (via clang-tidy) Created: 23/Jul/22  Updated: 06/Dec/23  Resolved: 28/Jul/22

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: 6.1.0-rc0

Type: Improvement Priority: Major - P3
Reporter: Billy Donahue Assignee: Billy Donahue
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
is depended on by SERVER-68376 IDL: don't emit optional get and is_i... Closed
Related
related to SERVER-68247 Support mongo-authored clang-tidy plu... Closed
is related to SERVER-26542 Migrate to std::optional Backlog
is related to SERVER-68121 clang-tool boost::optional -> std::op... Backlog
Backwards Compatibility: Fully Compatible
Sprint: Service Arch 2022-08-08
Participants:

 Description   

Update all occurrences of boost::optional's is_initialized or get members.

(comes out of Skunkworks July 2022)

We should prefer using the API that's common between boost::optional and std::optional where we can, and we can in these cases.

  • boost::optional has deprecated the is_initialized member, in favor of the std::optional-compatible name has_value. Change all occurrences of is_initialized to has_value.
  • boost::optional has a get member, and std::optional does not. The common subset has two choices to get() calls: operator*() and value(). operator*() does exactly what get() does, but has a weird name and may not actually represent the intent of the caller. get has undefined behavior if the object is empty. Change these to value, which is supported by std::optional and throws on empty, making it a safer default choice. get() can be changed to operator*() if the value() behavior is inappropriate.

Future recurrences will be hard to lint on, but easy to clang-tidy on when we can load plugins into clang-tidy. This conversion is done by a clang-tidy plugin in the first place. In the meantime, the clang-tidy plugin can be compiled in.

The plugin: https://github.com/BillyDonahue/mongo_std_optional_migration



 Comments   
Comment by Billy Donahue [ 03/Aug/22 ]

The tool was pretty effective but actually missed a number of cases. Don't know why yet.
Maybe the clang ast couldn't be generated for some reason on some .cpp files.
Maybe it doesn't work on headers. Something like that. Mystery for next time, maybe.

Comment by Billy Donahue [ 27/Jul/22 ]

Well, I've mowed the lawn once, pretty much.
I still need to change IDL so that it doesn't produce .get and .is_initialized calls (SERVER-68376).
The tool will continue to be available to rerun at will.
Hopefully we can add it to the clang-tidy in Evergreen CI.

Comment by Githook User [ 27/Jul/22 ]

Author:

{'name': 'Billy Donahue', 'email': 'billy.donahue@gmail.com', 'username': 'BillyDonahue'}

Message: SERVER-68246 rewrite calls to boost::optional get and is_initialized
Branch: master
https://github.com/10gen/mongo-enterprise-modules/commit/a24ec7e8ce3cb4821f1abcfcb479a50428c655f0

Comment by Githook User [ 27/Jul/22 ]

Author:

{'name': 'Billy Donahue', 'email': 'BillyDonahue@users.noreply.github.com', 'username': 'BillyDonahue'}

Message: SERVER-68246 rewrite calls to boost::optional get and is_initialized
Branch: master
https://github.com/mongodb/mongo/commit/958ad9abfc80861d3f43f44da694e83464b01e1d

Comment by Billy Donahue [ 27/Jul/22 ]

Woot! Finally got it. Thanks to the guy on the LLVM Discord #clang-tidy chat room who gave me some tips. This is pretty damn elegant considering what it's doing under the hood.

https://github.com/BillyDonahue/mongo_std_optional_migration/blob/master/tool/lib/MongoStdOptionalMigration.cpp

Using it to generate a PR now.

Generated at Thu Feb 08 06:10:17 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.