================
@@ -165,22 +165,36 @@ void EnumInitialValueCheck::registerMatchers(MatchFinder 
*Finder) {
 
 void EnumInitialValueCheck::check(const MatchFinder::MatchResult &Result) {
   if (const auto *Enum = Result.Nodes.getNodeAs<EnumDecl>("inconsistent")) {
-    const DiagnosticBuilder Diag =
-        diag(
-            Enum->getBeginLoc(),
-            "initial values in enum '%0' are not consistent, consider explicit 
"
-            "initialization of all, none or only the first enumerator")
-        << getName(Enum);
-    for (const EnumConstantDecl *ECD : Enum->enumerators())
-      if (ECD->getInitExpr() == nullptr) {
-        const SourceLocation EndLoc = Lexer::getLocForEndOfToken(
-            ECD->getLocation(), 0, *Result.SourceManager, getLangOpts());
-        if (EndLoc.isMacroID())
-          continue;
-        llvm::SmallString<8> Str{" = "};
-        ECD->getInitVal().toString(Str);
-        Diag << FixItHint::CreateInsertion(EndLoc, Str);
+    // Emit warning first (DiagnosticBuilder emits on destruction), then notes.
+    // Notes must follow the primary diagnostic or they may be dropped.
+    {
+      const DiagnosticBuilder Diag =
+          diag(Enum->getBeginLoc(), "initial values in enum '%0' are not "
+                                    "consistent, consider explicit "
+                                    "initialization of all, none or only the "
+                                    "first enumerator")
+          << getName(Enum);
+
+      for (const EnumConstantDecl *ECD : Enum->enumerators()) {
+        if (ECD->getInitExpr() == nullptr) {
+          const SourceLocation EndLoc = Lexer::getLocForEndOfToken(
+              ECD->getLocation(), 0, *Result.SourceManager, getLangOpts());
+          if (EndLoc.isMacroID())
+            continue;
+          llvm::SmallString<8> Str{" = "};
+          ECD->getInitVal().toString(Str);
+          Diag << FixItHint::CreateInsertion(EndLoc, Str);
+        }
+      }
+    }
+
+    for (const EnumConstantDecl *ECD : Enum->enumerators()) {
+      if (ECD->getInitExpr() == nullptr && ECD->getDeclName()) {
----------------
gamesh411 wrote:

The codebase shows mixed usage.
Places for defensive checks:
- 
[`clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp:128`](https://github.com/llvm/llvm-project/blob/f006cd76790e0d7cf34b8c1dbd6be7baf4842158/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp#L128)
- 
[`clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp:193`](https://github.com/llvm/llvm-project/blob/f006cd76790e0d7cf34b8c1dbd6be7baf4842158/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp#L193)

Places without null checks:
- 
[`clang-tools-extra/clang-tidy/bugprone/TaggedUnionMemberCountCheck.cpp:146`](https://github.com/llvm/llvm-project/blob/f006cd76790e0d7cf34b8c1dbd6be7baf4842158/clang-tools-extra/clang-tidy/bugprone/TaggedUnionMemberCountCheck.cpp#L146)
- 
[`clang/lib/CodeGen/CGDebugInfo.cpp:3915`](https://github.com/llvm/llvm-project/blob/f006cd76790e0d7cf34b8c1dbd6be7baf4842158/clang/lib/CodeGen/CGDebugInfo.cpp#L3915)
- 
[`clang/lib/Sema/SemaLookup.cpp:1580`](https://github.com/llvm/llvm-project/blob/f006cd76790e0d7cf34b8c1dbd6be7baf4842158/clang/lib/Sema/SemaLookup.cpp#L1580)
- 
[`clang/lib/Index/IndexingContext.cpp:251`](https://github.com/llvm/llvm-project/blob/f006cd76790e0d7cf34b8c1dbd6be7baf4842158/clang/lib/Index/IndexingContext.cpp#L251)

I have found one place where `EnumConstantDecl` can be created with `nullptr` 
for identifier:
- 
[`clang/lib/AST/Decl.cpp:5715`](https://github.com/llvm/llvm-project/blob/f006cd76790e0d7cf34b8c1dbd6be7baf4842158/clang/lib/AST/Decl.cpp#L5715)
 — `EnumConstantDecl::CreateDeserialized()` creates an `EnumConstantDecl` with 
`nullptr` for the `IdentifierInfo *Id` parameter, which results in an empty 
`DeclarationName` when passed to the `ValueDecl` constructor
  this is called from here for example:
    
[`clang/lib/Serialization/ASTReaderDecl.cpp:3969`](https://github.com/llvm/llvm-project/blob/f006cd76790e0d7cf34b8c1dbd6be7baf4842158/clang/lib/Serialization/ASTReaderDecl.cpp#L3969)
 — calls `CreateDeserialized` during AST deserialization

Most likely those fields get filled out later during deserialization, so this 
not likely to be an issue.

I'm leaning toward putting an assert here, and we can always change it back to 
a branch if there is an issue.

https://github.com/llvm/llvm-project/pull/176485
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to