[SpecialCaseList] Add backward compatible dot-slash handling

This commit introduces backward compatible dot-slash handling for
SpecialCaseList.

The new behavior is controlled by the `Version` field in the special
case list file.

- Version 1 and 2: Path is matches as-is, regardless of presense of "./".
- Version 4 and higher: Paths with leading dot-slash are canonicalized
  to paths without dot-slash before matching. This means that a rule
  like `src=./foo` will never match, and `src=foo` will match both
  `foo` and `./foo`.
- Version 3: Transitionary version. Paths is
  matched both, without canonicalization, and with
  canonicalized. If non-canonicalized path
  matches, but canonicalized does not, a warning
  is emitted.

This change allows for a gradual transition to the new behavior, while
maintaining backward compatibility with existing special case list
files.

Pull Request: llvm#162511

Author: vitalybuka

llvm/lib/Support/SpecialCaseList.cpp

@@ -26,8 +26,10 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/VirtualFileSystem.h"
-#include "llvm/Support/raw_ostream.h"
+#include "llvm/Support/WithColor.h"
+#include <cassert>
 #include <memory>
+#include <mutex>
 #include <stdio.h>
 #include <string>
 #include <system_error>
@@ -44,6 +46,7 @@ class RegexMatcher {
 public:
   Error insert(StringRef Pattern, unsigned LineNumber);
   unsigned match(StringRef Query) const;
+  StringRef findRule(unsigned LineNo) const;
 
 private:
   struct Reg {
@@ -61,6 +64,7 @@ class GlobMatcher {
 public:
   Error insert(StringRef Pattern, unsigned LineNumber);
   unsigned match(StringRef Query) const;
+  StringRef findRule(unsigned LineNo) const;
 
 private:
   struct Glob {
@@ -90,15 +94,25 @@ class GlobMatcher {
 /// Represents a set of patterns and their line numbers
 class Matcher {
 public:
-  Matcher(bool UseGlobs, bool RemoveDotSlash);
+  enum class QueryOptions {
+    kUnchanged,
+    kNoSlashDot,
+    kNoSlashDotWithFallback,
+  };
+
+  Matcher(bool UseGlobs, QueryOptions QOpts = QueryOptions::kUnchanged);
 
   Error insert(StringRef Pattern, unsigned LineNumber);
   unsigned match(StringRef Query) const;
 
   bool matchAny(StringRef Query) const { return match(Query); }
 
+private:
+  unsigned matchInternal(StringRef Query) const;
+  StringRef findRule(unsigned LineNo) const;
+
   std::variant<RegexMatcher, GlobMatcher> M;
-  bool RemoveDotSlash;
+  QueryOptions Options;
 };
 
 Error RegexMatcher::insert(StringRef Pattern, unsigned LineNumber) {
@@ -132,6 +146,14 @@ unsigned RegexMatcher::match(StringRef Query) const {
   return 0;
 }
 
+StringRef RegexMatcher::findRule(unsigned LineNo) const {
+  for (const auto &R : RegExes)
+    if (R.LineNo == LineNo)
+      return R.Name;
+  assert(!"`findRule` should be called only with correct `LineNo`");
+  return {};
+}
+
 Error GlobMatcher::insert(StringRef Pattern, unsigned LineNumber) {
   if (Pattern.empty())
     return createStringError(errc::invalid_argument, "Supplied glob was blank");
@@ -218,8 +240,15 @@ unsigned GlobMatcher::match(StringRef Query) const {
   return Best < 0 ? 0 : Globs[Best].LineNo;
 }
 
-Matcher::Matcher(bool UseGlobs, bool RemoveDotSlash)
-    : RemoveDotSlash(RemoveDotSlash) {
+StringRef GlobMatcher::findRule(unsigned LineNo) const {
+  for (const auto &G : Globs)
+    if (G.LineNo == LineNo)
+      return G.Name;
+  assert(!"`findRule` should be called only with correct `LineNo`");
+  return {};
+}
+
+Matcher::Matcher(bool UseGlobs, QueryOptions QOpts) : Options(QOpts) {
   if (UseGlobs)
     M.emplace<GlobMatcher>();
   else
@@ -231,10 +260,42 @@ Error Matcher::insert(StringRef Pattern, unsigned LineNumber) {
 }
 
 unsigned Matcher::match(StringRef Query) const {
-  if (RemoveDotSlash)
-    Query = llvm::sys::path::remove_leading_dotslash(Query);
+  switch (Options) {
+  case QueryOptions::kUnchanged:
+    return matchInternal(Query);
+  case QueryOptions::kNoSlashDot:
+    return matchInternal(llvm::sys::path::remove_leading_dotslash(Query));
+  case QueryOptions::kNoSlashDotWithFallback:
+    break;
+  }
+
+  StringRef FixedQuery = llvm::sys::path::remove_leading_dotslash(Query);
+  unsigned FixedMatched = matchInternal(FixedQuery);
+  if (FixedQuery == Query)
+    return FixedMatched;
+
+  unsigned OriginalMatch = matchInternal(Query);
+  if (OriginalMatch > FixedMatched) {
+    static std::once_flag Warned;
+    std::call_once(Warned, [&]() {
+      WithColor::warning() << "Deprecated behaviour: '"
+                           << findRule(OriginalMatch) << "' matches '" << Query
+                           << "', but it should match '" << FixedQuery
+                           << "'.\n";
+    });
+  }
+  return std::max(OriginalMatch, FixedMatched);
+}
+
+unsigned Matcher::matchInternal(StringRef Query) const {
   return std::visit([&](auto &V) -> unsigned { return V.match(Query); }, M);
 }
+
+StringRef Matcher::findRule(unsigned LineNo) const {
+  return std::visit([&](auto &V) -> StringRef { return V.findRule(LineNo); },
+                    M);
+}
+
 } // namespace
 
 class SpecialCaseList::Section::SectionImpl {
@@ -244,8 +305,7 @@ class SpecialCaseList::Section::SectionImpl {
 public:
   using SectionEntries = StringMap<StringMap<Matcher>>;
 
-  SectionImpl(StringRef Str, bool UseGlobs)
-      : SectionMatcher(UseGlobs, /*RemoveDotSlash=*/false) {}
+  SectionImpl(StringRef Str, bool UseGlobs) : SectionMatcher(UseGlobs) {}
 
   Matcher SectionMatcher;
   SectionEntries Entries;
@@ -350,8 +410,6 @@ bool SpecialCaseList::parse(unsigned FileIdx, const MemoryBuffer *MB,
   // https://discourse.llvm.org/t/use-glob-instead-of-regex-for-specialcaselists/71666
   bool UseGlobs = Version > 1;
 
-  bool RemoveDotSlash = Version > 2;
-
   auto ErrOrSection = addSection("*", FileIdx, 1, true);
   if (auto Err = ErrOrSection.takeError()) {
     Error = toString(std::move(Err));
@@ -397,10 +455,17 @@ bool SpecialCaseList::parse(unsigned FileIdx, const MemoryBuffer *MB,
       return false;
     }
 
+    Matcher::QueryOptions QOpts = Matcher::QueryOptions::kUnchanged;
+    if (llvm::is_contained(PathPrefixes, Prefix)) {
+      if (Version == 3)
+        QOpts = Matcher::QueryOptions::kNoSlashDotWithFallback;
+      else if (Version > 4)
+        QOpts = Matcher::QueryOptions::kNoSlashDot;
+    }
+
     auto [Pattern, Category] = Postfix.split("=");
-    auto [It, _] = CurrentImpl->Entries[Prefix].try_emplace(
-        Category, UseGlobs,
-        RemoveDotSlash && llvm::is_contained(PathPrefixes, Prefix));
+    auto [It, _] =
+        CurrentImpl->Entries[Prefix].try_emplace(Category, UseGlobs, QOpts);
     Pattern = Pattern.copy(StrAlloc);
     if (auto Err = It->second.insert(Pattern, LineNo)) {
       Error =

llvm/unittests/Support/SpecialCaseListTest.cpp

@@ -329,8 +329,8 @@ TEST_F(SpecialCaseListTest, DotSlash) {
   EXPECT_FALSE(SCL4->inSection("dot", "fun", "foo"));
 
   EXPECT_TRUE(SCL2->inSection("dot", "src", "./bar"));
-  EXPECT_FALSE(SCL3->inSection("dot", "src", "./bar"));
-  EXPECT_FALSE(SCL4->inSection("dot", "src", "./bar"));
+  EXPECT_TRUE(SCL3->inSection("dot", "src", "./bar"));
+  EXPECT_TRUE(SCL4->inSection("dot", "src", "./bar"));
 
   EXPECT_FALSE(SCL2->inSection("dot", "src", "bar"));
   EXPECT_FALSE(SCL3->inSection("dot", "src", "bar"));
@@ -346,7 +346,7 @@ TEST_F(SpecialCaseListTest, DotSlash) {
 
   EXPECT_FALSE(SCL2->inSection("not", "src", "./bar"));
   EXPECT_TRUE(SCL3->inSection("not", "src", "./bar"));
-  EXPECT_TRUE(SCL4->inSection("not", "src", "./bar"));
+  EXPECT_FALSE(SCL4->inSection("not", "src", "./bar"));
 
   EXPECT_TRUE(SCL2->inSection("not", "src", "bar"));
   EXPECT_TRUE(SCL3->inSection("not", "src", "bar"));