[clang] [clang-tools-extra] [llvm] [clang] Introduce diagnostics suppression mappings (PR #112517)

kadir çetinkaya via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 28 08:02:49 PDT 2024


================
@@ -477,6 +485,100 @@ void DiagnosticsEngine::setSeverityForAll(diag::Flavor Flavor,
       setSeverity(Diag, Map, Loc);
 }
 
+namespace {
+class WarningsSpecialCaseList : public llvm::SpecialCaseList {
+public:
+  static std::unique_ptr<WarningsSpecialCaseList>
+  create(const llvm::MemoryBuffer &MB, std::string &Err) {
+    auto SCL = std::make_unique<WarningsSpecialCaseList>();
+    if (SCL->createInternal(&MB, Err))
+      return SCL;
+    return nullptr;
+  }
+
+  // Section names refer to diagnostic groups, which cover multiple individual
+  // diagnostics. Expand diagnostic groups here to individual diagnostics.
+  // A diagnostic can have multiple diagnostic groups associated with it, we let
+  // the last section take precedence in such cases.
+  void processSections(DiagnosticsEngine &Diags) {
+    // Drop the default section introduced by special case list, we only support
+    // exact diagnostic group names.
+    Sections.erase("*");
+    // Make sure we iterate sections by their line numbers.
+    std::vector<std::pair<unsigned, const llvm::StringMapEntry<Section> *>>
+        LineAndSection;
+    for (const auto &Entry : Sections) {
+      LineAndSection.emplace_back(
+          Entry.second.SectionMatcher->Globs.at(Entry.first()).second, &Entry);
+    }
+    llvm::sort(LineAndSection);
+    static constexpr auto kFlavor = clang::diag::Flavor::WarningOrError;
+    for (const auto &[_, Entry] : LineAndSection) {
+      SmallVector<diag::kind, 256> GroupDiags;
+      if (Diags.getDiagnosticIDs()->getDiagnosticsInGroup(
+              kFlavor, Entry->first(), GroupDiags)) {
+        StringRef Suggestion =
+            DiagnosticIDs::getNearestOption(kFlavor, Entry->first());
+        Diags.Report(diag::warn_unknown_diag_option)
----------------
kadircet wrote:

> Assuming we hide some functions, this is definitely something that should be fixed.
> Also, if the API of the new class, because it inherits from the base class, is wider than necessary, it's also a good reason to refactor.
> We can do it in a followup.

I guess this comment is rather about the inheritance vs composition of the new `WarningSpecialCaseList`. All the newly exposed APIs are already tested through public interfaces in https://github.com/llvm/llvm-project/pull/112517/files#diff-a8d03c054381627f238bdb8f7914daa4abebdae705d313dda026ce36eca0fe2f. Because this is all the functionality being introduced as well, I don't see the point in testing those separately.

Still happy to do the refactoring for exposing specialcaselist just as a parser though.

> Seems useful to test end-to-end including flags, reading files, etc.
> Hard for me to estimate the cost of generating or maintaining this.

There's some end-to-end [integration test](https://github.com/llvm/llvm-project/pull/112517/files#diff-0f0bd95286925c4909908958c1104decb1068507c0ef516382a779ce463bb83d) that ensures the flag is propagated all the way from driver to diagsengine. I'd rather keep rest of the functionality in unittests as they're much easier to reason about when they go wrong, and resort to integration tests when there are bits of logic that can't really be tested through a unittests (which isn't the case here).

https://github.com/llvm/llvm-project/pull/112517


More information about the cfe-commits mailing list