Revert "Reapply "[clang] Extend diagnose_if to accept more detailed w…

…arning information (llvm#70976)" (llvm#108453)" This reverts commit e7f782e. This had UBSan failures: [----------] 1 test from ConfigCompileTests [ RUN ] ConfigCompileTests.DiagnosticSuppression Config fragment: compiling <unknown>:0 -> 0x00007B8366E2F7D8 (trusted=false) /usr/local/google/home/fmayer/large/llvm-project/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:203:33: runtime error: reference binding to null pointer of type 'clang::DiagnosticIDs' UndefinedBehaviorSanitizer: undefined-behavior /usr/local/google/home/fmayer/large/llvm-project/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:203:33 Pull Request: llvm#108645
llvmbot · Sep 13, 2024 · e1bd974 · e1bd974
1 parent f885e02
commit e1bd974
Show file tree

Hide file tree

Showing 29 changed files with 225 additions and 546 deletions.
diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -579,17 +579,7 @@ std::vector<Diag> StoreDiags::take(const clang::tidy::ClangTidyContext *Tidy) {
   for (auto &Diag : Output) {
     if (const char *ClangDiag = getDiagnosticCode(Diag.ID)) {
       // Warnings controlled by -Wfoo are better recognized by that name.
-      const StringRef Warning = [&] {
-        if (OrigSrcMgr) {
-          return OrigSrcMgr->getDiagnostics()
-              .getDiagnosticIDs()
-              ->getWarningOptionForDiag(Diag.ID);
-        }
-        if (!DiagnosticIDs::IsCustomDiag(Diag.ID))
-          return DiagnosticIDs{}.getWarningOptionForDiag(Diag.ID);
-        return StringRef{};
-      }();
-
+      StringRef Warning = DiagnosticIDs::getWarningOptionForDiag(Diag.ID);
       if (!Warning.empty()) {
         Diag.Name = ("-W" + Warning).str();
       } else {
@@ -906,23 +896,20 @@ void StoreDiags::flushLastDiag() {
   Output.push_back(std::move(*LastDiag));
 }
 
-bool isDiagnosticSuppressed(const clang::Diagnostic &Diag,
-                            const llvm::StringSet<> &Suppress,
-                            const LangOptions &LangOpts) {
+bool isBuiltinDiagnosticSuppressed(unsigned ID,
+                                   const llvm::StringSet<> &Suppress,
+                                   const LangOptions &LangOpts) {
   // Don't complain about header-only stuff in mainfiles if it's a header.
   // FIXME: would be cleaner to suppress in clang, once we decide whether the
   //        behavior should be to silently-ignore or respect the pragma.
-  if (Diag.getID() == diag::pp_pragma_sysheader_in_main_file &&
-      LangOpts.IsHeaderFile)
+  if (ID == diag::pp_pragma_sysheader_in_main_file && LangOpts.IsHeaderFile)
     return true;
 
-  if (const char *CodePtr = getDiagnosticCode(Diag.getID())) {
+  if (const char *CodePtr = getDiagnosticCode(ID)) {
     if (Suppress.contains(normalizeSuppressedCode(CodePtr)))
       return true;
   }
-  StringRef Warning =
-      Diag.getDiags()->getDiagnosticIDs()->getWarningOptionForDiag(
-          Diag.getID());
+  StringRef Warning = DiagnosticIDs::getWarningOptionForDiag(ID);
   if (!Warning.empty() && Suppress.contains(Warning))
     return true;
   return false;

diff --git a/clang-tools-extra/clangd/Diagnostics.h b/clang-tools-extra/clangd/Diagnostics.h
@@ -181,11 +181,11 @@ class StoreDiags : public DiagnosticConsumer {
 };
 
 /// Determine whether a (non-clang-tidy) diagnostic is suppressed by config.
-bool isDiagnosticSuppressed(const clang::Diagnostic &Diag,
-                            const llvm::StringSet<> &Suppressed,
-                            const LangOptions &);
+bool isBuiltinDiagnosticSuppressed(unsigned ID,
+                                   const llvm::StringSet<> &Suppressed,
+                                   const LangOptions &);
 /// Take a user-specified diagnostic code, and convert it to a normalized form
-/// stored in the config and consumed by isDiagnosticsSuppressed.
+/// stored in the config and consumed by isBuiltinDiagnosticsSuppressed.
 ///
 /// (This strips err_ and -W prefix so we can match with or without them.)
 llvm::StringRef normalizeSuppressedCode(llvm::StringRef);

diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -340,7 +340,7 @@ void applyWarningOptions(llvm::ArrayRef<std::string> ExtraArgs,
       if (Enable) {
         if (Diags.getDiagnosticLevel(ID, SourceLocation()) <
             DiagnosticsEngine::Warning) {
-          auto Group = Diags.getDiagnosticIDs()->getGroupForDiag(ID);
+          auto Group = DiagnosticIDs::getGroupForDiag(ID);
           if (!Group || !EnabledGroups(*Group))
             continue;
           Diags.setSeverity(ID, diag::Severity::Warning, SourceLocation());
@@ -583,8 +583,8 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
     ASTDiags.setLevelAdjuster([&](DiagnosticsEngine::Level DiagLevel,
                                   const clang::Diagnostic &Info) {
       if (Cfg.Diagnostics.SuppressAll ||
-          isDiagnosticSuppressed(Info, Cfg.Diagnostics.Suppress,
-                                 Clang->getLangOpts()))
+          isBuiltinDiagnosticSuppressed(Info.getID(), Cfg.Diagnostics.Suppress,
+                                        Clang->getLangOpts()))
         return DiagnosticsEngine::Ignored;
 
       auto It = OverriddenSeverity.find(Info.getID());

diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp
@@ -621,8 +621,8 @@ buildPreamble(PathRef FileName, CompilerInvocation CI,
   PreambleDiagnostics.setLevelAdjuster([&](DiagnosticsEngine::Level DiagLevel,
                                            const clang::Diagnostic &Info) {
     if (Cfg.Diagnostics.SuppressAll ||
-        isDiagnosticSuppressed(Info, Cfg.Diagnostics.Suppress,
-                               CI.getLangOpts()))
+        isBuiltinDiagnosticSuppressed(Info.getID(), Cfg.Diagnostics.Suppress,
+                                      CI.getLangOpts()))
       return DiagnosticsEngine::Ignored;
     switch (Info.getID()) {
     case diag::warn_no_newline_eof:

diff --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -298,41 +298,20 @@ TEST_F(ConfigCompileTests, DiagnosticSuppression) {
                                    "unreachable-code", "unused-variable",
                                    "typecheck_bool_condition",
                                    "unexpected_friend", "warn_alloca"));
-  clang::DiagnosticsEngine DiagEngine(nullptr, nullptr,
-                                      new clang::IgnoringDiagConsumer);
-
-  using Diag = clang::Diagnostic;
-  {
-    auto D = DiagEngine.Report(diag::warn_unreachable);
-    EXPECT_TRUE(isDiagnosticSuppressed(
-        Diag{&DiagEngine}, Conf.Diagnostics.Suppress, LangOptions()));
-  }
+  EXPECT_TRUE(isBuiltinDiagnosticSuppressed(
+      diag::warn_unreachable, Conf.Diagnostics.Suppress, LangOptions()));
   // Subcategory not respected/suppressed.
-  {
-    auto D = DiagEngine.Report(diag::warn_unreachable_break);
-    EXPECT_FALSE(isDiagnosticSuppressed(
-        Diag{&DiagEngine}, Conf.Diagnostics.Suppress, LangOptions()));
-  }
-  {
-    auto D = DiagEngine.Report(diag::warn_unused_variable);
-    EXPECT_TRUE(isDiagnosticSuppressed(
-        Diag{&DiagEngine}, Conf.Diagnostics.Suppress, LangOptions()));
-  }
-  {
-    auto D = DiagEngine.Report(diag::err_typecheck_bool_condition);
-    EXPECT_TRUE(isDiagnosticSuppressed(
-        Diag{&DiagEngine}, Conf.Diagnostics.Suppress, LangOptions()));
-  }
-  {
-    auto D = DiagEngine.Report(diag::err_unexpected_friend);
-    EXPECT_TRUE(isDiagnosticSuppressed(
-        Diag{&DiagEngine}, Conf.Diagnostics.Suppress, LangOptions()));
-  }
-  {
-    auto D = DiagEngine.Report(diag::warn_alloca);
-    EXPECT_TRUE(isDiagnosticSuppressed(
-        Diag{&DiagEngine}, Conf.Diagnostics.Suppress, LangOptions()));
-  }
+  EXPECT_FALSE(isBuiltinDiagnosticSuppressed(
+      diag::warn_unreachable_break, Conf.Diagnostics.Suppress, LangOptions()));
+  EXPECT_TRUE(isBuiltinDiagnosticSuppressed(
+      diag::warn_unused_variable, Conf.Diagnostics.Suppress, LangOptions()));
+  EXPECT_TRUE(isBuiltinDiagnosticSuppressed(diag::err_typecheck_bool_condition,
+                                            Conf.Diagnostics.Suppress,
+                                            LangOptions()));
+  EXPECT_TRUE(isBuiltinDiagnosticSuppressed(
+      diag::err_unexpected_friend, Conf.Diagnostics.Suppress, LangOptions()));
+  EXPECT_TRUE(isBuiltinDiagnosticSuppressed(
+      diag::warn_alloca, Conf.Diagnostics.Suppress, LangOptions()));
 
   Frag.Diagnostics.Suppress.emplace_back("*");
   EXPECT_TRUE(compileAndApply());

diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
@@ -3358,16 +3358,18 @@ def DiagnoseIf : InheritableAttr {
   let Spellings = [GNU<"diagnose_if">];
   let Subjects = SubjectList<[Function, ObjCMethod, ObjCProperty]>;
   let Args = [ExprArgument<"Cond">, StringArgument<"Message">,
-              EnumArgument<"DefaultSeverity",
-                           "DefaultSeverity",
+              EnumArgument<"DiagnosticType", "DiagnosticType",
                            /*is_string=*/true,
-                           ["error",    "warning"],
-                           ["DS_error", "DS_warning"]>,
-              StringArgument<"WarningGroup", /*optional*/ 1>,
+                           ["error", "warning"],
+                           ["DT_Error", "DT_Warning"]>,
               BoolArgument<"ArgDependent", 0, /*fake*/ 1>,
               DeclArgument<Named, "Parent", 0, /*fake*/ 1>];
   let InheritEvenIfAlreadyPresent = 1;
   let LateParsed = LateAttrParseStandard;
+  let AdditionalMembers = [{
+    bool isError() const { return diagnosticType == DT_Error; }
+    bool isWarning() const { return diagnosticType == DT_Warning; }
+  }];
   let TemplateDependent = 1;
   let Documentation = [DiagnoseIfDocs];
 }

diff --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h
@@ -336,12 +336,10 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
     // Map extensions to warnings or errors?
     diag::Severity ExtBehavior = diag::Severity::Ignored;
 
-    DiagnosticIDs &DiagIDs;
-
-    DiagState(DiagnosticIDs &DiagIDs)
+    DiagState()
         : IgnoreAllWarnings(false), EnableAllWarnings(false),
           WarningsAsErrors(false), ErrorsAsFatal(false),
-          SuppressSystemWarnings(false), DiagIDs(DiagIDs) {}
+          SuppressSystemWarnings(false) {}
 
     using iterator = llvm::DenseMap<unsigned, DiagnosticMapping>::iterator;
     using const_iterator =
@@ -872,8 +870,6 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
   /// \param FormatString A fixed diagnostic format string that will be hashed
   /// and mapped to a unique DiagID.
   template <unsigned N>
-  // TODO: Deprecate this once all uses are removed from LLVM
-  // [[deprecated("Use a CustomDiagDesc instead of a Level")]]
   unsigned getCustomDiagID(Level L, const char (&FormatString)[N]) {
     return Diags->getCustomDiagID((DiagnosticIDs::Level)L,
                                   StringRef(FormatString, N - 1));

diff --git a/clang/include/clang/Basic/DiagnosticCategories.h b/clang/include/clang/Basic/DiagnosticCategories.h
@@ -21,12 +21,11 @@ namespace clang {
     };
 
     enum class Group {
-#define DIAG_ENTRY(GroupName, FlagNameOffset, Members, SubGroups, Docs)    \
-      GroupName,
+#define DIAG_ENTRY(GroupName, FlagNameOffset, Members, SubGroups, Docs)        \
+  GroupName,
 #include "clang/Basic/DiagnosticGroups.inc"
 #undef CATEGORY
 #undef DIAG_ENTRY
-      NUM_GROUPS
     };
   }  // end namespace diag
 }  // end namespace clang