[clang] Switch warning suppression multi-match rule to "last match takes precedence" (#162237)

The current "longest match takes precedence" rule
for warning suppression mappings can be confusing,
especially in long suppression files where
tracking the length relationship between globs is
difficult.

For example, with the following rules, it's not
immediately obvious why the first one should
currently take precedence:

```
src:*test/*
src:*lld/*=emit
```

This commit changes the multi-match behavior so
the last match takes precedence. This rule is
easier to understand and consistent with the
approach used by sanitizers, simplifying the
mechanism by providing a uniform experience across
different tools.

This is potentially breaking, but very unlikely.
An investigation of known uses showed they do not
rely on the length.

Author: vitalybuka

clang/docs/ReleaseNotes.rst

@@ -69,6 +69,8 @@ Potentially Breaking Changes
   call the member ``operator delete`` instead of the expected global
   delete operator. The old behavior is retained under ``-fclang-abi-compat=21``
   flag.
+- Clang warning suppressions file, ``--warning-suppression-mappings=``, now will
+  use the last matching entry instead of the longest one.
 - Trailing null statements in GNU statement expressions are no longer
   ignored by Clang; they now result in a void type. Clang previously
   matched GCC's behavior, which was recently clarified to be incorrect.

clang/docs/WarningSuppressionMappings.rst

@@ -63,7 +63,7 @@ Format
 Warning suppression mappings uses the same format as
 :doc:`SanitizerSpecialCaseList`.
 
-Sections describe which diagnostic group's behaviour to change, e.g.
+Sections describe which diagnostic group's behavior to change, e.g.
 ``[unused]``. When a diagnostic is matched by multiple sections, the latest
 section takes precedence.
 
@@ -76,7 +76,7 @@ Source files are matched against these globs either:
 - as paths relative to the current working directory
 - as absolute paths.
 
-When a source file matches multiple globs in a section, the longest one takes
+When a source file matches multiple globs in a section, the last one takes
 precedence.
 
 .. code-block:: bash

clang/include/clang/Basic/Diagnostic.h

@@ -971,7 +971,7 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
   /// diagnostics in specific files.
   /// Mapping file is expected to be a special case list with sections denoting
   /// diagnostic groups and `src` entries for globs to suppress. `emit` category
-  /// can be used to disable suppression. Longest glob that matches a filepath
+  /// can be used to disable suppression. The last glob that matches a filepath
   /// takes precedence. For example:
   ///   [unused]
   ///   src:clang/*

clang/lib/Basic/Diagnostic.cpp

@@ -525,8 +525,7 @@ std::unique_ptr<WarningsSpecialCaseList>
 WarningsSpecialCaseList::create(const llvm::MemoryBuffer &Input,
                                 std::string &Err) {
   auto WarningSuppressionList = std::make_unique<WarningsSpecialCaseList>();
-  if (!WarningSuppressionList->createInternal(&Input, Err,
-                                              /*OrderBySize=*/true))
+  if (!WarningSuppressionList->createInternal(&Input, Err))
     return nullptr;
   return WarningSuppressionList;
 }
@@ -588,15 +587,12 @@ bool WarningsSpecialCaseList::isDiagSuppressed(diag::kind DiagId,
 
   StringRef F = llvm::sys::path::remove_leading_dotslash(PLoc.getFilename());
 
-  StringRef LongestSup = DiagSection->getLongestMatch("src", F, "");
-  if (LongestSup.empty())
+  unsigned LastSup = DiagSection->getLastMatch("src", F, "");
+  if (LastSup == 0)
     return false;
 
-  StringRef LongestEmit = DiagSection->getLongestMatch("src", F, "emit");
-  if (LongestEmit.empty())
-    return true;
-
-  return LongestSup.size() > LongestEmit.size();
+  unsigned LastEmit = DiagSection->getLastMatch("src", F, "emit");
+  return LastSup > LastEmit;
 }
 
 bool DiagnosticsEngine::isSuppressedViaMapping(diag::kind DiagId,

clang/unittests/Basic/DiagnosticTest.cpp

@@ -294,7 +294,7 @@ TEST_F(SuppressionMappingTest, EmitCategoryIsExcluded) {
                                             locForFile("foo.cpp")));
 }
 
-TEST_F(SuppressionMappingTest, LongestMatchWins) {
+TEST_F(SuppressionMappingTest, LastMatchWins) {
   llvm::StringLiteral SuppressionMappingFile = R"(
   [unused]
   src:*clang/*
@@ -327,10 +327,8 @@ TEST_F(SuppressionMappingTest, LongShortMatch) {
 
   EXPECT_TRUE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
                                            locForFile("test/t1.cpp")));
-
-  // FIXME: This is confusing.
-  EXPECT_TRUE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
-                                           locForFile("lld/test/t2.cpp")));
+  EXPECT_FALSE(Diags.isSuppressedViaMapping(diag::warn_unused_function,
+                                            locForFile("lld/test/t2.cpp")));
 }
 
 TEST_F(SuppressionMappingTest, ShortLongMatch) {