[SERVER-56571] Remove invariant.h allowlist Created: 03/May/21  Updated: 29/Oct/23  Resolved: 20/Mar/22

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

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

Issue Links:
Related
related to SERVER-51061 Rename c preprocessor macro that guar... Closed
Backwards Compatibility: Fully Compatible
Sprint: Service Arch 2022-03-21, Service Arch 2022-04-04
Participants:
Story Points: 1

 Description   

Propose removing MONGO_ALLOW_INCLUDE_INVARIANT_H (nee MONGO_INCLUDE_INVARIANT_H_WHITELISTED).
Maybe rename invariant.h to assert_util_detail.h or similar to indicate the relationship better.

The #ifdef/#error/#endif handshake in invariant.h interferes with indexing that header, as it can't be included by a trivial .cpp file to yield a successful compilation. This is mildly annoying.

The benefit it is meant to provide is hazy or maybe obsolete. It's meant to force invariant callers to include assert_util.h. But the allowlist requirement is not transitive and cannot be. Most high-level files using invariant.h without assert_util.h do so by including one of the files on the allow list (usually base/string_data.h). So they skirt the requirement, perhaps inadvertently.

I can't see what the allowlist is trying to protect. I think it's trying to produce a culture where assert_util.h will always be preferred to direct inclusion of invariant.h, which is a good thing. I think maybe renaming invariant.h to assert_util_core.h or assert_util_detail.h and retaining the admonition at the top:

/**
 * This header is separated from assert_util.h so that the low-level
 * dependencies of assert_util.h (e.g. mongo/base/status_with.h,
 * mongo/base/status.h, mongo/base/string_data.h) can use the invariant macro
 * without causing a circular include chain. It should never be included
 * directly in any other files.
 */

would do this in a more direct way that would not have any technical side effects.



 Comments   
Comment by Githook User [ 20/Mar/22 ]

Author:

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

Message: SERVER-56571 rename and unprotect invariant.h
Branch: master
https://github.com/mongodb/mongo/commit/829a112787ec85d89388f6fff1ece798508b309b

Comment by Githook User [ 20/Mar/22 ]

Author:

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

Message: SERVER-56571 rename invariant.h
Branch: master
https://github.com/10gen/mongo-enterprise-modules/commit/be504f7fb251f32d496543310fc112c99feeec95

Generated at Thu Feb 08 05:39:37 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.