From 4fbfdc801f6f296a8021f8efaea3a0da887f276f Mon Sep 17 00:00:00 2001 From: erichkeane Date: Thu, 9 Jan 2025 10:47:59 -0800 Subject: [PATCH 1/3] Add 'enum_select' diagnostic selection to clang. This causes us to generate an enum to go along with the select diagnostic, which allows for clearer diagnostic error emit lines. The syntax for this is: %enum_select{%OptionalEnumeratorName{Text}|{Text2}}0 Where the curley brackets around the select-text are only required if an Enumerator name is provided. The TableGen here emits this as a normal 'select' to the frontend, which permits us to reuse all of the existing 'select' infrastructure. Documentation is the same as well. --- clang/include/clang/Basic/CMakeLists.txt | 5 + clang/include/clang/Basic/DiagnosticAST.h | 13 ++ .../include/clang/Basic/DiagnosticAnalysis.h | 12 ++ clang/include/clang/Basic/DiagnosticComment.h | 13 ++ clang/include/clang/Basic/DiagnosticCrossTU.h | 13 ++ clang/include/clang/Basic/DiagnosticDriver.h | 13 ++ .../include/clang/Basic/DiagnosticFrontend.h | 13 ++ .../clang/Basic/DiagnosticInstallAPI.h | 13 ++ clang/include/clang/Basic/DiagnosticLex.h | 12 ++ clang/include/clang/Basic/DiagnosticParse.h | 13 ++ .../clang/Basic/DiagnosticRefactoring.h | 13 ++ clang/include/clang/Basic/DiagnosticSema.h | 13 ++ .../clang/Basic/DiagnosticSemaKinds.td | 8 +- .../clang/Basic/DiagnosticSerialization.h | 13 ++ clang/lib/Sema/SemaDeclCXX.cpp | 25 +-- clang/test/TableGen/select-enum-errors.td | 16 ++ clang/test/TableGen/select-enum.td | 26 +++ .../TableGen/ClangDiagnosticsEmitter.cpp | 192 +++++++++++++++++- clang/utils/TableGen/TableGen.cpp | 6 + clang/utils/TableGen/TableGenBackends.h | 2 + 20 files changed, 417 insertions(+), 17 deletions(-) create mode 100644 clang/test/TableGen/select-enum-errors.td create mode 100644 clang/test/TableGen/select-enum.td diff --git a/clang/include/clang/Basic/CMakeLists.txt b/clang/include/clang/Basic/CMakeLists.txt index 897a610b7f908..56c27bacdb20b 100644 --- a/clang/include/clang/Basic/CMakeLists.txt +++ b/clang/include/clang/Basic/CMakeLists.txt @@ -3,6 +3,11 @@ macro(clang_diag_gen component) -gen-clang-diags-defs -clang-component=${component} SOURCE Diagnostic.td TARGET ClangDiagnostic${component}) + + clang_tablegen(Diagnostic${component}Enums.inc + -gen-clang-diags-enums -clang-component=${component} + SOURCE Diagnostic.td + TARGET ClangDiagnostic${component}Enums) endmacro(clang_diag_gen) clang_diag_gen(Analysis) diff --git a/clang/include/clang/Basic/DiagnosticAST.h b/clang/include/clang/Basic/DiagnosticAST.h index 24ef2689eac01..4f82114b7406b 100644 --- a/clang/include/clang/Basic/DiagnosticAST.h +++ b/clang/include/clang/Basic/DiagnosticAST.h @@ -22,6 +22,19 @@ enum { #undef DIAG NUM_BUILTIN_AST_DIAGNOSTICS }; + +#define DIAG_ENUM(ENUM_NAME) \ + namespace ENUM_NAME { \ + enum { +#define DIAG_ENUM_ITEM(IDX, NAME) NAME = IDX, +#define DIAG_ENUM_END() \ + } \ + ; \ + } +#include "clang/Basic/DiagnosticASTEnums.inc" +#undef DIAG_ENUM_END +#undef DIAG_ENUM_ITEM +#undef DIAG_ENUM } // end namespace diag } // end namespace clang diff --git a/clang/include/clang/Basic/DiagnosticAnalysis.h b/clang/include/clang/Basic/DiagnosticAnalysis.h index 676b58f7d6ef2..1a49461bcd173 100644 --- a/clang/include/clang/Basic/DiagnosticAnalysis.h +++ b/clang/include/clang/Basic/DiagnosticAnalysis.h @@ -22,6 +22,18 @@ enum { #undef DIAG NUM_BUILTIN_ANALYSIS_DIAGNOSTICS }; +#define DIAG_ENUM(ENUM_NAME) \ + namespace ENUM_NAME { \ + enum { +#define DIAG_ENUM_ITEM(IDX, NAME) NAME = IDX, +#define DIAG_ENUM_END() \ + } \ + ; \ + } +#include "clang/Basic/DiagnosticAnalysisEnums.inc" +#undef DIAG_ENUM_END +#undef DIAG_ENUM_ITEM +#undef DIAG_ENUM } // end namespace diag } // end namespace clang diff --git a/clang/include/clang/Basic/DiagnosticComment.h b/clang/include/clang/Basic/DiagnosticComment.h index 17c0053e9a33d..53143ef132e4b 100644 --- a/clang/include/clang/Basic/DiagnosticComment.h +++ b/clang/include/clang/Basic/DiagnosticComment.h @@ -22,6 +22,19 @@ enum { #undef DIAG NUM_BUILTIN_COMMENT_DIAGNOSTICS }; + +#define DIAG_ENUM(ENUM_NAME) \ + namespace ENUM_NAME { \ + enum { +#define DIAG_ENUM_ITEM(IDX, NAME) NAME = IDX, +#define DIAG_ENUM_END() \ + } \ + ; \ + } +#include "clang/Basic/DiagnosticCommentEnums.inc" +#undef DIAG_ENUM_END +#undef DIAG_ENUM_ITEM +#undef DIAG_ENUM } // end namespace diag } // end namespace clang diff --git a/clang/include/clang/Basic/DiagnosticCrossTU.h b/clang/include/clang/Basic/DiagnosticCrossTU.h index 4341bf327b69c..428da95011027 100644 --- a/clang/include/clang/Basic/DiagnosticCrossTU.h +++ b/clang/include/clang/Basic/DiagnosticCrossTU.h @@ -22,6 +22,19 @@ enum { #undef DIAG NUM_BUILTIN_CROSSTU_DIAGNOSTICS }; + +#define DIAG_ENUM(ENUM_NAME) \ + namespace ENUM_NAME { \ + enum { +#define DIAG_ENUM_ITEM(IDX, NAME) NAME = IDX, +#define DIAG_ENUM_END() \ + } \ + ; \ + } +#include "clang/Basic/DiagnosticCrossTUEnums.inc" +#undef DIAG_ENUM_END +#undef DIAG_ENUM_ITEM +#undef DIAG_ENUM } // end namespace diag } // end namespace clang diff --git a/clang/include/clang/Basic/DiagnosticDriver.h b/clang/include/clang/Basic/DiagnosticDriver.h index 6931bd46542e8..c472afa3f6e96 100644 --- a/clang/include/clang/Basic/DiagnosticDriver.h +++ b/clang/include/clang/Basic/DiagnosticDriver.h @@ -22,6 +22,19 @@ enum { #undef DIAG NUM_BUILTIN_DRIVER_DIAGNOSTICS }; + +#define DIAG_ENUM(ENUM_NAME) \ + namespace ENUM_NAME { \ + enum { +#define DIAG_ENUM_ITEM(IDX, NAME) NAME = IDX, +#define DIAG_ENUM_END() \ + } \ + ; \ + } +#include "clang/Basic/DiagnosticDriverEnums.inc" +#undef DIAG_ENUM_END +#undef DIAG_ENUM_ITEM +#undef DIAG_ENUM } // end namespace diag } // end namespace clang diff --git a/clang/include/clang/Basic/DiagnosticFrontend.h b/clang/include/clang/Basic/DiagnosticFrontend.h index ab4e855f2de02..766cac3d655b3 100644 --- a/clang/include/clang/Basic/DiagnosticFrontend.h +++ b/clang/include/clang/Basic/DiagnosticFrontend.h @@ -22,6 +22,19 @@ enum { #undef DIAG NUM_BUILTIN_FRONTEND_DIAGNOSTICS }; + +#define DIAG_ENUM(ENUM_NAME) \ + namespace ENUM_NAME { \ + enum { +#define DIAG_ENUM_ITEM(IDX, NAME) NAME = IDX, +#define DIAG_ENUM_END() \ + } \ + ; \ + } +#include "clang/Basic/DiagnosticFrontendEnums.inc" +#undef DIAG_ENUM_END +#undef DIAG_ENUM_ITEM +#undef DIAG_ENUM } // end namespace diag } // end namespace clang diff --git a/clang/include/clang/Basic/DiagnosticInstallAPI.h b/clang/include/clang/Basic/DiagnosticInstallAPI.h index a76f6e087a2b0..cbdb00362624b 100644 --- a/clang/include/clang/Basic/DiagnosticInstallAPI.h +++ b/clang/include/clang/Basic/DiagnosticInstallAPI.h @@ -21,6 +21,19 @@ enum { #undef DIAG NUM_BUILTIN_INSTALLAPI_DIAGNOSTICS }; + +#define DIAG_ENUM(ENUM_NAME) \ + namespace ENUM_NAME { \ + enum { +#define DIAG_ENUM_ITEM(IDX, NAME) NAME = IDX, +#define DIAG_ENUM_END() \ + } \ + ; \ + } +#include "clang/Basic/DiagnosticInstallAPIEnums.inc" +#undef DIAG_ENUM_END +#undef DIAG_ENUM_ITEM +#undef DIAG_ENUM } // namespace diag } // namespace clang #endif // LLVM_CLANG_BASIC_DIAGNOSTICINSTALLAPI_H diff --git a/clang/include/clang/Basic/DiagnosticLex.h b/clang/include/clang/Basic/DiagnosticLex.h index 5f237085ae03a..d14bf97e8642e 100644 --- a/clang/include/clang/Basic/DiagnosticLex.h +++ b/clang/include/clang/Basic/DiagnosticLex.h @@ -22,6 +22,18 @@ enum { #undef DIAG NUM_BUILTIN_LEX_DIAGNOSTICS }; +#define DIAG_ENUM(ENUM_NAME) \ + namespace ENUM_NAME { \ + enum { +#define DIAG_ENUM_ITEM(IDX, NAME) NAME = IDX, +#define DIAG_ENUM_END() \ + } \ + ; \ + } +#include "clang/Basic/DiagnosticLexEnums.inc" +#undef DIAG_ENUM_END +#undef DIAG_ENUM_ITEM +#undef DIAG_ENUM } // end namespace diag } // end namespace clang diff --git a/clang/include/clang/Basic/DiagnosticParse.h b/clang/include/clang/Basic/DiagnosticParse.h index 81a8185d25fb7..275e1a4c39b3f 100644 --- a/clang/include/clang/Basic/DiagnosticParse.h +++ b/clang/include/clang/Basic/DiagnosticParse.h @@ -22,6 +22,19 @@ enum { #undef DIAG NUM_BUILTIN_PARSE_DIAGNOSTICS }; + +#define DIAG_ENUM(ENUM_NAME) \ + namespace ENUM_NAME { \ + enum { +#define DIAG_ENUM_ITEM(IDX, NAME) NAME = IDX, +#define DIAG_ENUM_END() \ + } \ + ; \ + } +#include "clang/Basic/DiagnosticParseEnums.inc" +#undef DIAG_ENUM_END +#undef DIAG_ENUM_ITEM +#undef DIAG_ENUM } // end namespace diag } // end namespace clang diff --git a/clang/include/clang/Basic/DiagnosticRefactoring.h b/clang/include/clang/Basic/DiagnosticRefactoring.h index 9b628dbeb7c26..59d4bc912733a 100644 --- a/clang/include/clang/Basic/DiagnosticRefactoring.h +++ b/clang/include/clang/Basic/DiagnosticRefactoring.h @@ -22,6 +22,19 @@ enum { #undef DIAG NUM_BUILTIN_REFACTORING_DIAGNOSTICS }; + +#define DIAG_ENUM(ENUM_NAME) \ + namespace ENUM_NAME { \ + enum { +#define DIAG_ENUM_ITEM(IDX, NAME) NAME = IDX, +#define DIAG_ENUM_END() \ + } \ + ; \ + } +#include "clang/Basic/DiagnosticRefactoringEnums.inc" +#undef DIAG_ENUM_END +#undef DIAG_ENUM_ITEM +#undef DIAG_ENUM } // end namespace diag } // end namespace clang diff --git a/clang/include/clang/Basic/DiagnosticSema.h b/clang/include/clang/Basic/DiagnosticSema.h index 45014fe21271d..84986c7bccf71 100644 --- a/clang/include/clang/Basic/DiagnosticSema.h +++ b/clang/include/clang/Basic/DiagnosticSema.h @@ -22,6 +22,19 @@ enum { #undef DIAG NUM_BUILTIN_SEMA_DIAGNOSTICS }; + +#define DIAG_ENUM(ENUM_NAME) \ + namespace ENUM_NAME { \ + enum { +#define DIAG_ENUM_ITEM(IDX, NAME) NAME = IDX, +#define DIAG_ENUM_END() \ + } \ + ; \ + } +#include "clang/Basic/DiagnosticSemaEnums.inc" +#undef DIAG_ENUM_END +#undef DIAG_ENUM_ITEM +#undef DIAG_ENUM } // end namespace diag } // end namespace clang diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index d4e897868f1a9..bbac4d3fe71a6 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -617,9 +617,11 @@ def err_ambiguous_inherited_constructor : Error< "constructor of %0 inherited from multiple base class subobjects">; def note_ambiguous_inherited_constructor_using : Note< "inherited from base class %0 here">; -def note_using_decl_class_member_workaround : Note< - "use %select{an alias declaration|a typedef declaration|a reference|" - "a const variable|a constexpr variable}0 instead">; +def note_using_decl_class_member_workaround + : Note<"use %enum_select{%AliasDecl{an alias " + "declaration}|%TypedefDecl{a typedef declaration}|%ReferenceDecl{a " + "reference}|%ConstVar{a const variable}|%ConstexprVar{a constexpr " + "variable}}0 instead">; def err_using_decl_can_not_refer_to_namespace : Error< "using declaration cannot refer to a namespace">; def note_namespace_using_decl : Note< diff --git a/clang/include/clang/Basic/DiagnosticSerialization.h b/clang/include/clang/Basic/DiagnosticSerialization.h index 0c622a5657737..6fb836dca1b04 100644 --- a/clang/include/clang/Basic/DiagnosticSerialization.h +++ b/clang/include/clang/Basic/DiagnosticSerialization.h @@ -22,6 +22,19 @@ enum { #undef DIAG NUM_BUILTIN_SERIALIZATION_DIAGNOSTICS }; + +#define DIAG_ENUM(ENUM_NAME) \ + namespace ENUM_NAME { \ + enum { +#define DIAG_ENUM_ITEM(IDX, NAME) NAME = IDX, +#define DIAG_ENUM_END() \ + } \ + ; \ + } +#include "clang/Basic/DiagnosticSerializationEnums.inc" +#undef DIAG_ENUM_END +#undef DIAG_ENUM_ITEM +#undef DIAG_ENUM } // end namespace diag } // end namespace clang diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index c5a72cf812ebc..eb8a9c85c8ebb 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -13217,18 +13217,18 @@ bool Sema::CheckUsingDeclQualifier(SourceLocation UsingLoc, bool HasTypename, if (getLangOpts().CPlusPlus11) { // Convert 'using X::Y;' to 'using Y = X::Y;'. Diag(SS.getBeginLoc(), diag::note_using_decl_class_member_workaround) - << 0 // alias declaration - << FixItHint::CreateInsertion(SS.getBeginLoc(), - NameInfo.getName().getAsString() + - " = "); + << diag::MemClassWorkaround::AliasDecl + << FixItHint::CreateInsertion(SS.getBeginLoc(), + NameInfo.getName().getAsString() + + " = "); } else { // Convert 'using X::Y;' to 'typedef X::Y Y;'. SourceLocation InsertLoc = getLocForEndOfToken(NameInfo.getEndLoc()); Diag(InsertLoc, diag::note_using_decl_class_member_workaround) - << 1 // typedef declaration - << FixItHint::CreateReplacement(UsingLoc, "typedef") - << FixItHint::CreateInsertion( - InsertLoc, " " + NameInfo.getName().getAsString()); + << diag::MemClassWorkaround::TypedefDecl + << FixItHint::CreateReplacement(UsingLoc, "typedef") + << FixItHint::CreateInsertion( + InsertLoc, " " + NameInfo.getName().getAsString()); } } else if (R->getAsSingle()) { // Don't provide a fixit outside C++11 mode; we don't want to suggest @@ -13241,8 +13241,7 @@ bool Sema::CheckUsingDeclQualifier(SourceLocation UsingLoc, bool HasTypename, } Diag(UsingLoc, diag::note_using_decl_class_member_workaround) - << 2 // reference declaration - << FixIt; + << diag::MemClassWorkaround::ReferenceDecl << FixIt; } else if (R->getAsSingle()) { // Don't provide a fixit outside C++11 mode; we don't want to suggest // repeating the type of the enumeration here, and we can't do so if @@ -13256,8 +13255,10 @@ bool Sema::CheckUsingDeclQualifier(SourceLocation UsingLoc, bool HasTypename, } Diag(UsingLoc, diag::note_using_decl_class_member_workaround) - << (getLangOpts().CPlusPlus11 ? 4 : 3) // const[expr] variable - << FixIt; + << (getLangOpts().CPlusPlus11 + ? diag::MemClassWorkaround::ConstexprVar + : diag::MemClassWorkaround::ConstVar) + << FixIt; } } diff --git a/clang/test/TableGen/select-enum-errors.td b/clang/test/TableGen/select-enum-errors.td new file mode 100644 index 0000000000000..a2e8c09361760 --- /dev/null +++ b/clang/test/TableGen/select-enum-errors.td @@ -0,0 +1,16 @@ +// RUN: clang-tblgen --gen-clang-diags-enums -I%S %s 2>&1 | FileCheck %s +include "DiagnosticBase.inc" + +def DupeNames1 : Error<"%enum_select{}0">; +def DupeNames2 : Error<"%enum_select{}0">; +// CHECK: error: Duplicate enumeration name 'DupeName' +// CHECK-NEXT: def DupeNames2 +// CHECK: note: Previous diagnostic is here +// CHECK-NEXT: def DupeNames1 + +def DupeValue : Error<"%enum_select{%Name{V1}|%Name{V2}}0">; +// CHECK: error: Duplicate enumerator name 'Name' + +def EnumValNotExpected : Error<"%enum_select{V1|%Val2{V2}}0">; +// CHECK: expected '<' after enum_select + diff --git a/clang/test/TableGen/select-enum.td b/clang/test/TableGen/select-enum.td new file mode 100644 index 0000000000000..8a92acec62cfb --- /dev/null +++ b/clang/test/TableGen/select-enum.td @@ -0,0 +1,26 @@ +// RUN: clang-tblgen --gen-clang-diags-enums -I%S %s 2>&1 | FileCheck %s +include "DiagnosticBase.inc" + +def Diag : Error<"%enum_select{%Val1{V1}|%Val2{V2}|%Val3{V3}}0">; +// CHECK: DIAG_ENUM(EnumName) +// CHECK-NEXT: DIAG_ENUM_ITEM(0, Val1) +// CHECK-NEXT: DIAG_ENUM_ITEM(1, Val2) +// CHECK-NEXT: DIAG_ENUM_ITEM(2, Val3) +// CHECK-NEXT: DIAG_ENUM_END() + +// These are OK, we permit missing values since they might not be useful. +def Missing1 : Error<"%enum_select{V1|%Val2{V2}|%Val3{V3}}0">; +// CHECK: DIAG_ENUM(DupeEnumName1) +// CHECK-NEXT: DIAG_ENUM_ITEM(1, Val2) +// CHECK-NEXT: DIAG_ENUM_ITEM(2, Val3) +// CHECK-NEXT: DIAG_ENUM_END() +def Missing2 : Error<"%enum_select{%Val1{V1}|V2|%Val3{V3}}0">; +// CHECK: DIAG_ENUM(DupeEnumName2) +// CHECK-NEXT: DIAG_ENUM_ITEM(0, Val1) +// CHECK-NEXT: DIAG_ENUM_ITEM(2, Val3) +// CHECK-NEXT: DIAG_ENUM_END() +def Missing3 : Error<"%enum_select{%Val1{V1}|%Val2{V2}|V3}0">; +// CHECK: DIAG_ENUM(DupeEnumName3) +// CHECK-NEXT: DIAG_ENUM_ITEM(0, Val1) +// CHECK-NEXT: DIAG_ENUM_ITEM(1, Val2) +// CHECK-NEXT: DIAG_ENUM_END() diff --git a/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp b/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp index 72b3468dac486..824f0682f5531 100644 --- a/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp +++ b/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp @@ -399,6 +399,7 @@ enum PieceKind { TextPieceClass, PlaceholderPieceClass, SelectPieceClass, + EnumSelectPieceClass, PluralPieceClass, DiffPieceClass, SubstitutionPieceClass, @@ -408,6 +409,7 @@ enum ModifierType { MT_Unknown, MT_Placeholder, MT_Select, + MT_EnumSelect, MT_Sub, MT_Plural, MT_Diff, @@ -421,6 +423,7 @@ enum ModifierType { static StringRef getModifierName(ModifierType MT) { switch (MT) { + case MT_EnumSelect: case MT_Select: return "select"; case MT_Sub: @@ -512,10 +515,26 @@ struct SelectPiece : Piece { static bool classof(const Piece *P) { return P->getPieceClass() == SelectPieceClass || + P->getPieceClass() == EnumSelectPieceClass || P->getPieceClass() == PluralPieceClass; } }; +struct EnumSelectPiece : SelectPiece { + EnumSelectPiece() : SelectPiece(EnumSelectPieceClass, MT_EnumSelect) {} + + StringRef EnumName; + std::vector OptionEnumNames; + + static bool classof(const Piece *P) { + return P->getPieceClass() == EnumSelectPieceClass; + } +}; + +struct EnumValuePiece : Piece { + ModifierType Kind; +}; + struct PluralPiece : SelectPiece { PluralPiece() : SelectPiece(PluralPieceClass, MT_Plural) {} @@ -579,6 +598,9 @@ struct DiagnosticTextBuilder { std::vector buildForDocumentation(StringRef Role, const Record *R); std::string buildForDefinition(const Record *R); + llvm::SmallVector>>> + buildForEnum(const Record *R); Piece *getSubstitution(SubstitutionPiece *S) const { auto It = Substitutions.find(S->Name); @@ -707,6 +729,7 @@ template struct DiagTextVisitor { CASE(Text); CASE(Placeholder); CASE(Select); + CASE(EnumSelect); CASE(Plural); CASE(Diff); CASE(Substitution); @@ -886,6 +909,11 @@ struct DiagTextDocPrinter : DiagTextVisitor { makeRowSeparator(RST[I]); } + void VisitEnumSelect(EnumSelectPiece *P) { + // For now, document this as if it were a 'select'. + VisitSelect(P); + } + void VisitPlural(PluralPiece *P) { VisitSelect(P); } void VisitDiff(DiffPiece *P) { @@ -910,6 +938,47 @@ struct DiagTextDocPrinter : DiagTextVisitor { std::vector &RST; }; +struct DiagEnumPrinter : DiagTextVisitor { +public: + using BaseTy = DiagTextVisitor; + using EnumeratorItem = std::pair; + using EnumeratorList = llvm::SmallVector; + using ResultTy = llvm::SmallVector>; + + DiagEnumPrinter(DiagnosticTextBuilder &Builder, ResultTy &Result) + : BaseTy(Builder), Result(Result) {} + + ResultTy &Result; + + void VisitMulti(MultiPiece *P) { + for (auto *Child : P->Pieces) + Visit(Child); + } + void VisitText(TextPiece *P) {} + void VisitPlaceholder(PlaceholderPiece *P) {} + void VisitDiff(DiffPiece *P) {} + void VisitSelect(SelectPiece *P) { + for (auto *D : P->Options) + Visit(D); + } + void VisitPlural(PluralPiece *P) { VisitSelect(P); } + void VisitEnumSelect(EnumSelectPiece *P) { + assert(P->Options.size() == P->OptionEnumNames.size()); + + if (!P->EnumName.empty()) { + EnumeratorList List; + + for (const auto &Tup : llvm::enumerate(P->OptionEnumNames)) + if (!Tup.value().empty()) + List.emplace_back(Tup.index(), Tup.value()); + + Result.emplace_back(P->EnumName, List); + } + + VisitSelect(P); + } +}; + struct DiagTextPrinter : DiagTextVisitor { public: using BaseTy = DiagTextVisitor; @@ -929,7 +998,7 @@ struct DiagTextPrinter : DiagTextVisitor { void VisitSelect(SelectPiece *P) { Result += "%"; Result += getModifierName(P->ModKind); - if (P->ModKind == MT_Select) { + if (P->ModKind == MT_Select || P->ModKind == MT_EnumSelect) { Result += "{"; for (auto *D : P->Options) { Visit(D); @@ -958,6 +1027,13 @@ struct DiagTextPrinter : DiagTextVisitor { addInt(mapIndex(P->Index)); } + void VisitEnumSelect(EnumSelectPiece *P) { + // Print as if we are a 'select', which will result in the compiler just + // treating this like a normal select. This way we don't have to do any + // special work for the compiler to consume these. + VisitSelect(P); + } + void VisitDiff(DiffPiece *P) { Result += "%diff{"; Visit(P->Parts[0]); @@ -1019,11 +1095,12 @@ Piece *DiagnosticTextBuilder::DiagText::parseDiagText(StringRef &Text, Text = Text.drop_front(); // Extract the (optional) modifier. - size_t ModLength = Text.find_first_of("0123456789{"); + size_t ModLength = Text.find_first_of("0123456789<{"); StringRef Modifier = Text.slice(0, ModLength); Text = Text.slice(ModLength, StringRef::npos); ModifierType ModType = StringSwitch{Modifier} .Case("select", MT_Select) + .Case("enum_select", MT_EnumSelect) .Case("sub", MT_Sub) .Case("diff", MT_Diff) .Case("plural", MT_Plural) @@ -1042,6 +1119,9 @@ Piece *DiagnosticTextBuilder::DiagText::parseDiagText(StringRef &Text, Modifier); }; + if (ModType != MT_EnumSelect && Text[0] == '<') + Builder.PrintFatalError("modifier '<' syntax not valid with " + Modifier); + switch (ModType) { case MT_Unknown: Builder.PrintFatalError("Unknown modifier type: " + Modifier); @@ -1058,6 +1138,55 @@ Piece *DiagnosticTextBuilder::DiagText::parseDiagText(StringRef &Text, Parsed.push_back(Select); continue; } + case MT_EnumSelect: { + EnumSelectPiece *EnumSelect = New(); + if (Text[0] != '<') + Builder.PrintFatalError("expected '<' after " + Modifier); + + Text = Text.drop_front(); // Drop '<' + size_t EnumNameLen = Text.find_first_of('>'); + EnumSelect->EnumName = Text.slice(0, EnumNameLen); + Text = Text.slice(EnumNameLen, StringRef::npos); + ExpectAndConsume(">"); + + if (Text[0] != '{') + Builder.PrintFatalError("expected '{' after " + Modifier); + + do { + Text = Text.drop_front(); // '{' or '|' + + bool BracketsRequired = false; + if (Text[0] == '%') { + BracketsRequired = true; + Text = Text.drop_front(); // '%' + size_t OptionNameLen = Text.find_first_of("{"); + EnumSelect->OptionEnumNames.push_back(Text.slice(0, OptionNameLen)); + Text = Text.slice(OptionNameLen, StringRef::npos); + } else { + EnumSelect->OptionEnumNames.push_back({}); + } + + if (BracketsRequired) + ExpectAndConsume("{"); + else if (Text.front() == '{') { + Text = Text.drop_front(); + BracketsRequired = true; + } + + EnumSelect->Options.push_back( + parseDiagText(Text, StopAt::PipeOrCloseBrace)); + + if (BracketsRequired) + ExpectAndConsume("}"); + + assert(!Text.empty() && "malformed %select"); + } while (Text.front() == '|'); + + ExpectAndConsume("}"); + EnumSelect->Index = parseModifier(Text); + Parsed.push_back(EnumSelect); + continue; + } case MT_Plural: { PluralPiece *Plural = New(); do { @@ -1163,6 +1292,15 @@ DiagnosticTextBuilder::buildForDocumentation(StringRef Severity, return Result; } +DiagEnumPrinter::ResultTy DiagnosticTextBuilder::buildForEnum(const Record *R) { + EvaluatingRecordGuard Guard(&EvaluatingRecord, R); + StringRef Text = R->getValueAsString("Summary"); + DiagText D(*this, Text); + DiagEnumPrinter::ResultTy Result; + DiagEnumPrinter{*this, Result}.Visit(D.Root); + return Result; +} + std::string DiagnosticTextBuilder::buildForDefinition(const Record *R) { EvaluatingRecordGuard Guard(&EvaluatingRecord, R); StringRef Text = R->getValueAsString("Summary"); @@ -1377,6 +1515,56 @@ static void verifyDiagnosticWording(const Record &Diag) { // runs into odd situations like [[clang::warn_unused_result]], // #pragma clang, or --unwindlib=libgcc. } +/// ClangDiagsEnumsEmitter - The top-level class emits .def files containing +/// declarations of Clang diagnostic enums for selects. +void clang::EmitClangDiagsEnums(const RecordKeeper &Records, raw_ostream &OS, + const std::string &Component) { + DiagnosticTextBuilder DiagTextBuilder(Records); + ArrayRef Diags = + Records.getAllDerivedDefinitions("Diagnostic"); + + llvm::SmallVector> EnumerationNames; + + for (const Record &R : make_pointee_range(Diags)) { + DiagEnumPrinter::ResultTy Enums = DiagTextBuilder.buildForEnum(&R); + + for (auto &Enumeration : Enums) { + bool ShouldPrint = + Component.empty() || Component == R.getValueAsString("Component"); + + auto PreviousByName = llvm::find_if(EnumerationNames, [&](auto &Prev) { + return Prev.second == Enumeration.first; + }); + + if (PreviousByName != EnumerationNames.end()) { + PrintError(&R, + "Duplicate enumeration name '" + Enumeration.first + "'"); + PrintNote(PreviousByName->first->getLoc(), + "Previous diagnostic is here"); + } + + EnumerationNames.emplace_back(&R, Enumeration.first); + + if (ShouldPrint) + OS << "DIAG_ENUM(" << Enumeration.first << ")\n"; + + llvm::SmallVector EnumeratorNames; + for (auto &Enumerator : Enumeration.second) { + if (llvm::find(EnumeratorNames, Enumerator.second) != + EnumeratorNames.end()) + PrintError(&R, + "Duplicate enumerator name '" + Enumerator.second + "'"); + EnumeratorNames.push_back(Enumerator.second); + + if (ShouldPrint) + OS << "DIAG_ENUM_ITEM(" << Enumerator.first << ", " + << Enumerator.second << ")\n"; + } + if (ShouldPrint) + OS << "DIAG_ENUM_END()\n"; + } + } +} /// ClangDiagsDefsEmitter - The top-level class emits .def files containing /// declarations of Clang diagnostics. diff --git a/clang/utils/TableGen/TableGen.cpp b/clang/utils/TableGen/TableGen.cpp index 6e2bd0c9f819b..8b8eadbe7f7e5 100644 --- a/clang/utils/TableGen/TableGen.cpp +++ b/clang/utils/TableGen/TableGen.cpp @@ -48,6 +48,7 @@ enum ActionType { GenClangBasicWriter, GenClangBuiltins, GenClangDiagsDefs, + GenClangDiagsEnums, GenClangDiagGroups, GenClangDiagsIndexName, GenClangCommentNodes, @@ -173,6 +174,8 @@ cl::opt Action( "Generate clang builtins list"), clEnumValN(GenClangDiagsDefs, "gen-clang-diags-defs", "Generate Clang diagnostics definitions"), + clEnumValN(GenClangDiagsEnums, "gen-clang-diags-enums", + "Generate Clang diagnostic enums for selects"), clEnumValN(GenClangDiagGroups, "gen-clang-diag-groups", "Generate Clang diagnostic groups"), clEnumValN(GenClangDiagsIndexName, "gen-clang-diags-index-name", @@ -387,6 +390,9 @@ bool ClangTableGenMain(raw_ostream &OS, const RecordKeeper &Records) { case GenClangDiagsDefs: EmitClangDiagsDefs(Records, OS, ClangComponent); break; + case GenClangDiagsEnums: + EmitClangDiagsEnums(Records, OS, ClangComponent); + break; case GenClangDiagGroups: EmitClangDiagGroups(Records, OS); break; diff --git a/clang/utils/TableGen/TableGenBackends.h b/clang/utils/TableGen/TableGenBackends.h index f7527ac535a87..0448c94de08e3 100644 --- a/clang/utils/TableGen/TableGenBackends.h +++ b/clang/utils/TableGen/TableGenBackends.h @@ -89,6 +89,8 @@ void EmitClangBuiltins(const llvm::RecordKeeper &Records, void EmitClangDiagsDefs(const llvm::RecordKeeper &Records, llvm::raw_ostream &OS, const std::string &Component); +void EmitClangDiagsEnums(const llvm::RecordKeeper &Records, + llvm::raw_ostream &OS, const std::string &Component); void EmitClangDiagGroups(const llvm::RecordKeeper &Records, llvm::raw_ostream &OS); void EmitClangDiagsIndexName(const llvm::RecordKeeper &Records, From 116d20e39cc8e9df8a530adf0ef8c6508530fdcf Mon Sep 17 00:00:00 2001 From: erichkeane Date: Tue, 14 Jan 2025 13:34:56 -0800 Subject: [PATCH 2/3] Fix Review comments: -Update internals-manual -Update document generator comment, clarify we intend to document as if it is a 'select' -Add test coverage for other modifiers using '<' -Add additional suggested test coverage --- clang/docs/InternalsManual.rst | 15 ++++++++ clang/test/TableGen/select-enum-errors.td | 37 +++++++++++++++++-- .../TableGen/ClangDiagnosticsEmitter.cpp | 7 +++- 3 files changed, 53 insertions(+), 6 deletions(-) diff --git a/clang/docs/InternalsManual.rst b/clang/docs/InternalsManual.rst index 39d389b816f12..1823b9045ca61 100644 --- a/clang/docs/InternalsManual.rst +++ b/clang/docs/InternalsManual.rst @@ -276,6 +276,21 @@ Description: diagnostic instead of having to do things textually. The selected string does undergo formatting. +**"enum_select format** + +Example: + ``unknown frobbling of a %enum_select{%VarDecl{variable declaration}|%FuncDecl{function declaration}}0 when blarging`` +Class: + Integers +Description: + This format specifier is used exactly like a ``select`` specifier, except it + additionally generates a namespace, enumeration, and enumerator list based on + the format string given. In the above case, a namespace is generated named + ``FrobbleKind`` that has a anonymous enumeration with the enumerators + ``VarDecl`` and ``FuncDecl`` which correspond to the values 0 and 1. This + permits a clearer use of the ``Diag`` in source code, as the above could be + called as: ``Diag(Loc, diag::frobble) << diag::FrobbleKind::VarDecl``. + **"plural" format** Example: diff --git a/clang/test/TableGen/select-enum-errors.td b/clang/test/TableGen/select-enum-errors.td index a2e8c09361760..c39c9a27c461f 100644 --- a/clang/test/TableGen/select-enum-errors.td +++ b/clang/test/TableGen/select-enum-errors.td @@ -1,6 +1,17 @@ -// RUN: clang-tblgen --gen-clang-diags-enums -I%S %s 2>&1 | FileCheck %s +// RUN: not clang-tblgen --gen-clang-diags-enums -DERROR1 -I%S %s 2>&1 | FileCheck %s --check-prefixes=CHECK,C1 +// RUN: not clang-tblgen --gen-clang-diags-enums -DERROR2 -I%S %s 2>&1 | FileCheck %s --check-prefixes=CHECK,C2 +// RUN: not clang-tblgen --gen-clang-diags-enums -DERROR3 -I%S %s 2>&1 | FileCheck %s --check-prefixes=CHECK,C3 +// RUN: not clang-tblgen --gen-clang-diags-enums -DERROR4 -I%S %s 2>&1 | FileCheck %s --check-prefixes=CHECK,C4 include "DiagnosticBase.inc" +// No real reason to diagnose these, the namespace generated as the +// 'enumeration' name will never conflict with the enumerator. +def EnumerationEnumeratorDupe : Error<"%enum_select{%Matchy{haha}}0">; + +// Enumerator values aren't required, though this does seem kind of silly/not +// particularly useful? +def NoEnumerators : Error<"%enum_select{foo|bar|baz}0">; + def DupeNames1 : Error<"%enum_select{}0">; def DupeNames2 : Error<"%enum_select{}0">; // CHECK: error: Duplicate enumeration name 'DupeName' @@ -8,9 +19,27 @@ def DupeNames2 : Error<"%enum_select{}0">; // CHECK: note: Previous diagnostic is here // CHECK-NEXT: def DupeNames1 -def DupeValue : Error<"%enum_select{%Name{V1}|%Name{V2}}0">; -// CHECK: error: Duplicate enumerator name 'Name' +def DupeValue : Error<"%enum_select{%DName{V1}|%DName{V2}}0">; +// CHECK: error: Duplicate enumerator name 'DName' +#ifdef ERROR1 def EnumValNotExpected : Error<"%enum_select{V1|%Val2{V2}}0">; -// CHECK: expected '<' after enum_select +// C1: expected '<' after enum_select +#endif + +#ifdef ERROR2 +def SelectWithArrow : Error<"%select{V1|%Val2{V2}}0">; +// C2: modifier '<' syntax not valid with %select +#endif + +#ifdef ERROR3 +// Missing closing > after the name of the enumeration +def MissingClosing : Error<"%enum_select; +// C3: expected '>' while parsing %enum_select +#endif +#ifdef ERROR4 +// Missing { after the name of an enumerator +def MissingTextAfterEnumerator: Error<"%enum_select{%OtherName|foo}0">; +// C4: expected '{' while parsing %enum_select +#endif diff --git a/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp b/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp index 824f0682f5531..fb00c640d6b14 100644 --- a/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp +++ b/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp @@ -910,7 +910,9 @@ struct DiagTextDocPrinter : DiagTextVisitor { } void VisitEnumSelect(EnumSelectPiece *P) { - // For now, document this as if it were a 'select'. + // Document this as if it were a 'select', which properly prints all of the + // options correctly in a readable/reasonable manner. There isn't really + // anything valuable we could add to readers here. VisitSelect(P); } @@ -1120,7 +1122,8 @@ Piece *DiagnosticTextBuilder::DiagText::parseDiagText(StringRef &Text, }; if (ModType != MT_EnumSelect && Text[0] == '<') - Builder.PrintFatalError("modifier '<' syntax not valid with " + Modifier); + Builder.PrintFatalError("modifier '<' syntax not valid with %" + + Modifier); switch (ModType) { case MT_Unknown: From 83ad071afa1f7e9e5852b7f4d48260d25820ab4f Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Wed, 15 Jan 2025 12:16:46 -0800 Subject: [PATCH 3/3] Update clang/docs/InternalsManual.rst Change anonymous to unscoped. The name of it is irrelevant to what we're talking about anyway, since it is unscoped. Co-authored-by: Aaron Ballman --- clang/docs/InternalsManual.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/docs/InternalsManual.rst b/clang/docs/InternalsManual.rst index 1823b9045ca61..a2b551b6f333e 100644 --- a/clang/docs/InternalsManual.rst +++ b/clang/docs/InternalsManual.rst @@ -286,7 +286,7 @@ Description: This format specifier is used exactly like a ``select`` specifier, except it additionally generates a namespace, enumeration, and enumerator list based on the format string given. In the above case, a namespace is generated named - ``FrobbleKind`` that has a anonymous enumeration with the enumerators + ``FrobbleKind`` that has an unscoped enumeration with the enumerators ``VarDecl`` and ``FuncDecl`` which correspond to the values 0 and 1. This permits a clearer use of the ``Diag`` in source code, as the above could be called as: ``Diag(Loc, diag::frobble) << diag::FrobbleKind::VarDecl``.