Skip to content

Commit fee1689

Browse files
jansvoboda11jansvoboda11
authored
[clang] Refactor LangOptions to specify compatibility as X macro arg (#146766)
This removes the `{BENIGN,COMPATIBLE}{,_ENUM,_VALUE}_LANGOPT` X macros controlling `LangOptions`. These are permutations of the base `LANGOPT`, `ENUM_LANGOPT` and `VALUE_LANGOPT` X macros that also carry the information of their effect on AST (and therefore module compatibility). Their functionality is now implemented by passing `Benign`, `Compatible` or `NotCompatible` argument to the base X macros and using C++17 `if constexpr` in the clients to achieve the same codegen. This PR solves this FIXME: ``` // FIXME: Clients should be able to more easily select whether they want // different levels of compatibility versus how to handle different kinds // of option. ``` The base X macros are preserved, since they are used in `LangOptions.h` to generate different kinds of field and function declarations for flags, values and enums, which can't be achieved with `if constexpr`. The new syntax also forces developers to think about compatibility when adding new language option, hopefully reducing the number of new options that are affecting by default even though they are benign or compatible. Note that the `BENIGN_` macros used to forward to their `COMPATIBLE_` counterparts. I don't think this ever kicked in, since there are no clients of the `.def` file that define `COMPATIBLE_` without also defining `BENIGN_`. However, this might be something downstream forks need to take care of by doing `if constexpr (CK::Compatibility == CK::Benign || CK::Compatibility == CK::Compatible)` in place of `#define COMPATIBLE_`.
1 parent 3c76a05 commit fee1689

File tree

7 files changed

+469
-487
lines changed

7 files changed

+469
-487
lines changed

‎clang/include/clang/Basic/LangOptions.def

Lines changed: 370 additions & 421 deletions
Large diffs are not rendered by default.

‎clang/include/clang/Basic/LangOptions.h

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,22 @@ class LangOptionsBase {
7777
using RoundingMode = llvm::RoundingMode;
7878
using CFBranchLabelSchemeKind = clang::CFBranchLabelSchemeKind;
7979

80+
/// For ASTs produced with different option value, signifies their level of
81+
/// compatibility.
82+
enum class CompatibilityKind {
83+
/// Does affect the construction of the AST in a way that does prevent
84+
/// module interoperability.
85+
NotCompatible,
86+
/// Does affect the construction of the AST in a way that doesn't prevent
87+
/// interoperability (that is, the value can be different between an
88+
/// explicit module and the user of that module).
89+
Compatible,
90+
/// Does not affect the construction of the AST in any way (that is, the
91+
/// value can be different between an implicit module and the user of that
92+
/// module).
93+
Benign,
94+
};
95+
8096
enum GCMode { NonGC, GCOnly, HybridGC };
8197
enum StackProtectorMode { SSPOff, SSPOn, SSPStrong, SSPReq };
8298

@@ -486,16 +502,17 @@ class LangOptionsBase {
486502
};
487503

488504
// Define simple language options (with no accessors).
489-
#define LANGOPT(Name, Bits, Default, Description) unsigned Name : Bits;
490-
#define ENUM_LANGOPT(Name, Type, Bits, Default, Description)
505+
#define LANGOPT(Name, Bits, Default, Compatibility, Description) \
506+
unsigned Name : Bits;
507+
#define ENUM_LANGOPT(Name, Type, Bits, Default, Compatibility, Description)
491508
#include "clang/Basic/LangOptions.def"
492509

493510
protected:
494511
// Define language options of enumeration type. These are private, and will
495512
// have accessors (below).
496-
#define LANGOPT(Name, Bits, Default, Description)
497-
#define ENUM_LANGOPT(Name, Type, Bits, Default, Description) \
498-
LLVM_PREFERRED_TYPE(Type) \
513+
#define LANGOPT(Name, Bits, Default, Compatibility, Description)
514+
#define ENUM_LANGOPT(Name, Type, Bits, Default, Compatibility, Description) \
515+
LLVM_PREFERRED_TYPE(Type) \
499516
unsigned Name : Bits;
500517
#include "clang/Basic/LangOptions.def"
501518
};
@@ -655,8 +672,8 @@ class LangOptions : public LangOptionsBase {
655672
LangStandard::Kind LangStd = LangStandard::lang_unspecified);
656673

657674
// Define accessors/mutators for language options of enumeration type.
658-
#define LANGOPT(Name, Bits, Default, Description)
659-
#define ENUM_LANGOPT(Name, Type, Bits, Default, Description) \
675+
#define LANGOPT(Name, Bits, Default, Compatibility, Description)
676+
#define ENUM_LANGOPT(Name, Type, Bits, Default, Compatibility, Description) \
660677
Type get##Name() const { return static_cast<Type>(Name); } \
661678
void set##Name(Type Value) { \
662679
assert(static_cast<unsigned>(Value) < (1u << Bits)); \

‎clang/lib/Basic/LangOptions.cpp

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,19 @@
1616
using namespace clang;
1717

1818
LangOptions::LangOptions() : LangStd(LangStandard::lang_unspecified) {
19-
#define LANGOPT(Name, Bits, Default, Description) Name = Default;
20-
#define ENUM_LANGOPT(Name, Type, Bits, Default, Description) set##Name(Default);
19+
#define LANGOPT(Name, Bits, Default, Compatibility, Description) Name = Default;
20+
#define ENUM_LANGOPT(Name, Type, Bits, Default, Compatibility, Description) \
21+
set##Name(Default);
2122
#include "clang/Basic/LangOptions.def"
2223
}
2324

2425
void LangOptions::resetNonModularOptions() {
25-
#define LANGOPT(Name, Bits, Default, Description)
26-
#define BENIGN_LANGOPT(Name, Bits, Default, Description) Name = Default;
27-
#define BENIGN_ENUM_LANGOPT(Name, Type, Bits, Default, Description) \
28-
Name = static_cast<unsigned>(Default);
26+
#define LANGOPT(Name, Bits, Default, Compatibility, Description) \
27+
if constexpr (CompatibilityKind::Compatibility == CompatibilityKind::Benign) \
28+
Name = Default;
29+
#define ENUM_LANGOPT(Name, Type, Bits, Default, Compatibility, Description) \
30+
if constexpr (CompatibilityKind::Compatibility == CompatibilityKind::Benign) \
31+
Name = static_cast<unsigned>(Default);
2932
#include "clang/Basic/LangOptions.def"
3033

3134
// Reset "benign" options with implied values (Options.td ImpliedBy relations)

‎clang/lib/Frontend/CompilerInvocation.cpp

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5222,11 +5222,14 @@ std::string CompilerInvocation::getModuleHash() const {
52225222
HBuilder.add(serialization::VERSION_MAJOR, serialization::VERSION_MINOR);
52235223

52245224
// Extend the signature with the language options
5225-
#define LANGOPT(Name, Bits, Default, Description) HBuilder.add(LangOpts->Name);
5226-
#define ENUM_LANGOPT(Name, Type, Bits, Default, Description) \
5227-
HBuilder.add(static_cast<unsigned>(LangOpts->get##Name()));
5228-
#define BENIGN_LANGOPT(Name, Bits, Default, Description)
5229-
#define BENIGN_ENUM_LANGOPT(Name, Type, Bits, Default, Description)
5225+
// FIXME: Replace with C++20 `using enum LangOptions::CompatibilityKind`.
5226+
using CK = LangOptions::CompatibilityKind;
5227+
#define LANGOPT(Name, Bits, Default, Compatibility, Description) \
5228+
if constexpr (CK::Compatibility != CK::Benign) \
5229+
HBuilder.add(LangOpts->Name);
5230+
#define ENUM_LANGOPT(Name, Type, Bits, Default, Compatibility, Description) \
5231+
if constexpr (CK::Compatibility != CK::Benign) \
5232+
HBuilder.add(static_cast<unsigned>(LangOpts->get##Name()));
52305233
#include "clang/Basic/LangOptions.def"
52315234

52325235
HBuilder.addRange(getLangOpts().ModuleFeatures);

‎clang/lib/Frontend/FrontendActions.cpp

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -643,16 +643,20 @@ namespace {
643643
bool ReadLanguageOptions(const LangOptions &LangOpts,
644644
StringRef ModuleFilename, bool Complain,
645645
bool AllowCompatibleDifferences) override {
646+
// FIXME: Replace with C++20 `using enum LangOptions::CompatibilityKind`.
647+
using CK = LangOptions::CompatibilityKind;
648+
646649
Out.indent(2) << "Language options:\n";
647-
#define LANGOPT(Name, Bits, Default, Description) \
650+
#define LANGOPT(Name, Bits, Default, Compatibility, Description) \
651+
if constexpr (CK::Compatibility != CK::Benign) \
648652
DUMP_BOOLEAN(LangOpts.Name, Description);
649-
#define ENUM_LANGOPT(Name, Type, Bits, Default, Description) \
650-
Out.indent(4) << Description << ": " \
653+
#define ENUM_LANGOPT(Name, Type, Bits, Default, Compatibility, Description) \
654+
if constexpr (CK::Compatibility != CK::Benign) \
655+
Out.indent(4) << Description << ": " \
651656
<< static_cast<unsigned>(LangOpts.get##Name()) << "\n";
652-
#define VALUE_LANGOPT(Name, Bits, Default, Description) \
657+
#define VALUE_LANGOPT(Name, Bits, Default, Compatibility, Description) \
658+
if constexpr (CK::Compatibility != CK::Benign) \
653659
Out.indent(4) << Description << ": " << LangOpts.Name << "\n";
654-
#define BENIGN_LANGOPT(Name, Bits, Default, Description)
655-
#define BENIGN_ENUM_LANGOPT(Name, Type, Bits, Default, Description)
656660
#include "clang/Basic/LangOptions.def"
657661

658662
if (!LangOpts.ModuleFeatures.empty()) {

‎clang/lib/Serialization/ASTReader.cpp

Lines changed: 46 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -278,51 +278,57 @@ static bool checkLanguageOptions(const LangOptions &LangOpts,
278278
StringRef ModuleFilename,
279279
DiagnosticsEngine *Diags,
280280
bool AllowCompatibleDifferences = true) {
281-
#define LANGOPT(Name, Bits, Default, Description) \
282-
if (ExistingLangOpts.Name != LangOpts.Name) { \
283-
if (Diags) { \
284-
if (Bits == 1) \
285-
Diags->Report(diag::err_ast_file_langopt_mismatch) \
286-
<< Description << LangOpts.Name << ExistingLangOpts.Name \
287-
<< ModuleFilename; \
288-
else \
289-
Diags->Report(diag::err_ast_file_langopt_value_mismatch) \
290-
<< Description << ModuleFilename; \
281+
// FIXME: Replace with C++20 `using enum LangOptions::CompatibilityKind`.
282+
using CK = LangOptions::CompatibilityKind;
283+
284+
#define LANGOPT(Name, Bits, Default, Compatibility, Description) \
285+
if constexpr (CK::Compatibility != CK::Benign) { \
286+
if ((CK::Compatibility == CK::NotCompatible) || \
287+
(CK::Compatibility == CK::Compatible && \
288+
!AllowCompatibleDifferences)) { \
289+
if (ExistingLangOpts.Name != LangOpts.Name) { \
290+
if (Diags) { \
291+
if (Bits == 1) \
292+
Diags->Report(diag::err_ast_file_langopt_mismatch) \
293+
<< Description << LangOpts.Name << ExistingLangOpts.Name \
294+
<< ModuleFilename; \
295+
else \
296+
Diags->Report(diag::err_ast_file_langopt_value_mismatch) \
297+
<< Description << ModuleFilename; \
298+
} \
299+
return true; \
300+
} \
291301
} \
292-
return true; \
293302
}
294303

295-
#define VALUE_LANGOPT(Name, Bits, Default, Description) \
296-
if (ExistingLangOpts.Name != LangOpts.Name) { \
297-
if (Diags) \
298-
Diags->Report(diag::err_ast_file_langopt_value_mismatch) \
299-
<< Description << ModuleFilename; \
300-
return true; \
304+
#define VALUE_LANGOPT(Name, Bits, Default, Compatibility, Description) \
305+
if constexpr (CK::Compatibility != CK::Benign) { \
306+
if ((CK::Compatibility == CK::NotCompatible) || \
307+
(CK::Compatibility == CK::Compatible && \
308+
!AllowCompatibleDifferences)) { \
309+
if (ExistingLangOpts.Name != LangOpts.Name) { \
310+
if (Diags) \
311+
Diags->Report(diag::err_ast_file_langopt_value_mismatch) \
312+
<< Description << ModuleFilename; \
313+
return true; \
314+
} \
315+
} \
301316
}
302317

303-
#define ENUM_LANGOPT(Name, Type, Bits, Default, Description) \
304-
if (ExistingLangOpts.get##Name() != LangOpts.get##Name()) { \
305-
if (Diags) \
306-
Diags->Report(diag::err_ast_file_langopt_value_mismatch) \
307-
<< Description << ModuleFilename; \
308-
return true; \
318+
#define ENUM_LANGOPT(Name, Type, Bits, Default, Compatibility, Description) \
319+
if constexpr (CK::Compatibility != CK::Benign) { \
320+
if ((CK::Compatibility == CK::NotCompatible) || \
321+
(CK::Compatibility == CK::Compatible && \
322+
!AllowCompatibleDifferences)) { \
323+
if (ExistingLangOpts.get##Name() != LangOpts.get##Name()) { \
324+
if (Diags) \
325+
Diags->Report(diag::err_ast_file_langopt_value_mismatch) \
326+
<< Description << ModuleFilename; \
327+
return true; \
328+
} \
329+
} \
309330
}
310331

311-
#define COMPATIBLE_LANGOPT(Name, Bits, Default, Description) \
312-
if (!AllowCompatibleDifferences) \
313-
LANGOPT(Name, Bits, Default, Description)
314-
315-
#define COMPATIBLE_ENUM_LANGOPT(Name, Bits, Default, Description) \
316-
if (!AllowCompatibleDifferences) \
317-
ENUM_LANGOPT(Name, Bits, Default, Description)
318-
319-
#define COMPATIBLE_VALUE_LANGOPT(Name, Bits, Default, Description) \
320-
if (!AllowCompatibleDifferences) \
321-
VALUE_LANGOPT(Name, Bits, Default, Description)
322-
323-
#define BENIGN_LANGOPT(Name, Bits, Default, Description)
324-
#define BENIGN_ENUM_LANGOPT(Name, Type, Bits, Default, Description)
325-
#define BENIGN_VALUE_LANGOPT(Name, Bits, Default, Description)
326332
#include "clang/Basic/LangOptions.def"
327333

328334
if (ExistingLangOpts.ModuleFeatures != LangOpts.ModuleFeatures) {
@@ -6353,9 +6359,9 @@ bool ASTReader::ParseLanguageOptions(const RecordData &Record,
63536359
bool AllowCompatibleDifferences) {
63546360
LangOptions LangOpts;
63556361
unsigned Idx = 0;
6356-
#define LANGOPT(Name, Bits, Default, Description) \
6362+
#define LANGOPT(Name, Bits, Default, Compatibility, Description) \
63576363
LangOpts.Name = Record[Idx++];
6358-
#define ENUM_LANGOPT(Name, Type, Bits, Default, Description) \
6364+
#define ENUM_LANGOPT(Name, Type, Bits, Default, Compatibility, Description) \
63596365
LangOpts.set##Name(static_cast<LangOptions::Type>(Record[Idx++]));
63606366
#include "clang/Basic/LangOptions.def"
63616367
#define SANITIZER(NAME, ID) \

‎clang/lib/Serialization/ASTWriter.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1612,9 +1612,9 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, StringRef isysroot) {
16121612
// Language options.
16131613
Record.clear();
16141614
const LangOptions &LangOpts = PP.getLangOpts();
1615-
#define LANGOPT(Name, Bits, Default, Description) \
1615+
#define LANGOPT(Name, Bits, Default, Compatibility, Description) \
16161616
Record.push_back(LangOpts.Name);
1617-
#define ENUM_LANGOPT(Name, Type, Bits, Default, Description) \
1617+
#define ENUM_LANGOPT(Name, Type, Bits, Default, Compatibility, Description) \
16181618
Record.push_back(static_cast<unsigned>(LangOpts.get##Name()));
16191619
#include "clang/Basic/LangOptions.def"
16201620
#define SANITIZER(NAME, ID) \

0 commit comments

Comments
 (0)