Extend SpecialCaseList to allow users to blame precise entries in the SCL file.

[hctim]

Summary

Extends SCL functionality to allow users to find the line number in the file the SCL is built from through SpecialCaseList::inSectionBlame(...).

include/llvm/Support/SpecialCaseList.h

public:
   /// Returns true, if special case list contains a line
   /// \code
   ///   @Prefix:<E>=@Category
   /// \endcode
   /// where @Query satisfies wildcard expression <E> in a given @Section.
   bool inSection(StringRef Section, StringRef Prefix, StringRef Query,
                  StringRef Category = StringRef()) const;
 
+  /// Returns the line number corresponding to the special case list entry if
+  /// the special case list contains a line
+  /// \code
+  ///   @Prefix:<E>=@Category
+  /// \endcode
+  /// where @Query satisfies wildcard expression <E> in a given @Section.
+  /// Returns zero if there is no blacklist entry corresponding to this
+  /// expression.
+  unsigned inSectionBlame(StringRef Section, StringRef Prefix, StringRef Query,
+                          StringRef Category = StringRef()) const;
+
 protected:
   // Implementations of the create*() functions that can also be used by derived
   // classes.
   bool createInternal(const std::vector<std::string> &Paths,
                       std::string &Error);
   bool createInternal(const MemoryBuffer *MB, std::string &Error);
 
   SpecialCaseList(SpecialCaseList const &) = delete;
   SpecialCaseList &operator=(SpecialCaseList const &) = delete;
 
   /// Represents a set of regular expressions.  Regular expressions which are
   /// "literal" (i.e. no regex metacharacters) are stored in Strings, while all
   /// others are represented as a single pipe-separated regex in RegEx.  The
   /// reason for doing so is efficiency; StringSet is much faster at matching
   /// literal strings than Regex.
   class Matcher {
   public:
-    bool insert(std::string Regexp, std::string &REError);
+    bool insert(std::string Regexp, unsigned LineNumber, std::string &REError);
     void compile();
-    bool match(StringRef Query) const;
+    unsigned match(StringRef Query) const;
 
   private:
-    StringSet<> Strings;
+    StringMap<unsigned> Strings;
     TrigramIndex Trigrams;
-    std::unique_ptr<Regex> RegEx;
-    std::string UncompiledRegEx;
+    std::vector<std::pair<std::unique_ptr<Regex>, unsigned>> RegExes;
   };
 
   using SectionEntries = StringMap<StringMap<Matcher>>;
 
   struct Section {
     Section(std::unique_ptr<Matcher> M) : SectionMatcher(std::move(M)){};
 
     std::unique_ptr<Matcher> SectionMatcher;
     SectionEntries Entries;
   };
 
   std::vector<Section> Sections;
   bool IsCompiled;
 
   SpecialCaseList();
   /// Parses just-constructed SpecialCaseList entries from a memory buffer.
   bool parse(const MemoryBuffer *MB, StringMap<size_t> &SectionsMap,
              std::string &Error);
   /// compile() should be called once, after parsing all the memory buffers.
   void compile();
 
   // Helper method for derived classes to search by Prefix, Query, and Category
   // once they have already resolved a section entry.
-  bool inSection(const SectionEntries &Entries, StringRef Prefix,
+  unsigned inSectionBlame(const SectionEntries &Entries, StringRef Prefix,
                           StringRef Query, StringRef Category) const;
 };
 
 }  // namespace llvm
 
 #endif  // LLVM_SUPPORT_SPECIALCASELIST_H
 

lib/Support/SpecialCaseList.cpp

 #include <string>
 #include <system_error>
 #include <utility>
 
 #include <stdio.h>
 namespace llvm {
 
 bool SpecialCaseList::Matcher::insert(std::string Regexp,
+                                      unsigned LineNumber,
                                       std::string &REError) {
   if (Regexp.empty()) {
     REError = "Supplied regexp was blank";
     return false;
   }
 
   if (Regex::isLiteralERE(Regexp)) {
-    Strings.insert(Regexp);
+    Strings[Regexp] = LineNumber;
     return true;
   }
   Trigrams.insert(Regexp);
 
   // Replace * with .*
   for (size_t pos = 0; (pos = Regexp.find('*', pos)) != std::string::npos;
        pos += strlen(".*")) {
     Regexp.replace(pos, strlen("*"), ".*");
   }
 
+  Regexp = "^(" + Regexp + ")$";
+
   // Check that the regexp is valid.
   Regex CheckRE(Regexp);
   if (!CheckRE.isValid(REError))
     return false;
 
-  if (!UncompiledRegEx.empty())
-    UncompiledRegEx += "|";
-  UncompiledRegEx += "^(" + Regexp + ")$";
+  RegExes.emplace_back(
+      std::make_pair(make_unique<Regex>(std::move(CheckRE)), LineNumber));
   return true;
 }
 
-void SpecialCaseList::Matcher::compile() {
-  if (!UncompiledRegEx.empty()) {
-    RegEx.reset(new Regex(UncompiledRegEx));
-    UncompiledRegEx.clear();
-  }
-}
-
-bool SpecialCaseList::Matcher::match(StringRef Query) const {
-  if (Strings.count(Query))
-    return true;
+unsigned SpecialCaseList::Matcher::match(StringRef Query) const {
+  auto It = Strings.find(Query);
+  if (It != Strings.end())
+    return It->second;
   if (Trigrams.isDefinitelyOut(Query))
     return false;
-  return RegEx && RegEx->match(Query);
+  for (auto& RegExKV : RegExes)
+    if (auto It = RegExKV.first->match(Query))
+      return RegExKV.second;
+  return 0;
 }
 
 SpecialCaseList::SpecialCaseList() : Sections(), IsCompiled(false) {}
 
 std::unique_ptr<SpecialCaseList>
 SpecialCaseList::create(const std::vector<std::string> &Paths,
                         std::string &Error) {
   std::unique_ptr<SpecialCaseList> SCL(new SpecialCaseList());
if (std::error_code EC = FileOrErr.getError()) {
       return false;
     }
     std::string ParseError;
     if (!parse(FileOrErr.get().get(), Sections, ParseError)) {
       Error = (Twine("error parsing file '") + Path + "': " + ParseError).str();
       return false;
     }
   }
-  compile();
   return true;
 }
 
 bool SpecialCaseList::createInternal(const MemoryBuffer *MB,
                                      std::string &Error) {
   StringMap<size_t> Sections;
   if (!parse(MB, Sections, Error))
     return false;
-  compile();
   return true;
 }
 
 bool SpecialCaseList::parse(const MemoryBuffer *MB,
                             StringMap<size_t> &SectionsMap,
                             std::string &Error) {
   // Iterate through each line in the blacklist file.
   SmallVector<StringRef, 16> Lines;
   SplitString(MB->getBuffer(), Lines, "\n\r");
 
-  int LineNo = 1;
+  unsigned LineNo = 1;
   StringRef Section = "*";
   for (auto I = Lines.begin(), E = Lines.end(); I != E; ++I, ++LineNo) {
     // Ignore empty lines and lines starting with "#"
-    if (I->empty() || I->startswith("#"))
+    if (I->empty() || I->startswith("#")){
+      ++LineNo;
       continue;
+    }
 
     // Save section names
     if (I->startswith("[")) {
       if (!I->endswith("]")) {
         Error = (Twine("malformed section header on line ") + Twine(LineNo) +
                  ": " + *I).str();
         return false;
       }
for (auto I = Lines.begin(), E = Lines.end(); I != E; ++I, ++LineNo) {
     std::pair<StringRef, StringRef> SplitRegexp = SplitLine.second.split("=");
     std::string Regexp = SplitRegexp.first;
     StringRef Category = SplitRegexp.second;
 
     // Create this section if it has not been seen before.
     if (SectionsMap.find(Section) == SectionsMap.end()) {
       std::unique_ptr<Matcher> M = make_unique<Matcher>();
       std::string REError;
-      if (!M->insert(Section, REError)) {
+      if (!M->insert(Section, LineNo, REError)) {
         Error = (Twine("malformed section ") + Section + ": '" + REError).str();
         return false;
       }
-      M->compile();
 
       SectionsMap[Section] = Sections.size();
       Sections.emplace_back(std::move(M));
     }
 
     auto &Entry = Sections[SectionsMap[Section]].Entries[Prefix][Category];
     std::string REError;
-    if (!Entry.insert(std::move(Regexp), REError)) {
+    if (!Entry.insert(std::move(Regexp), LineNo, REError)) {
       Error = (Twine("malformed regex in line ") + Twine(LineNo) + ": '" +
                SplitLine.second + "': " + REError).str();
       return false;
     }
   }
   return true;
 }
 
-void SpecialCaseList::compile() {
-  assert(!IsCompiled && "compile() should only be called once");
-  // Iterate through every section compiling regular expressions for every query
-  // and creating Section entries.
-  for (auto &Section : Sections)
-    for (auto &Prefix : Section.Entries)
-      for (auto &Category : Prefix.getValue())
-        Category.getValue().compile();
-
-  IsCompiled = true;
-}
-
 SpecialCaseList::~SpecialCaseList() {}
 
 bool SpecialCaseList::inSection(StringRef Section, StringRef Prefix,
                                 StringRef Query, StringRef Category) const {
-  assert(IsCompiled && "SpecialCaseList::compile() was not called!");
+  return inSectionBlame(Section, Prefix, Query, Category);
+}
 
+unsigned SpecialCaseList::inSectionBlame(StringRef Section, StringRef Prefix,
+                                         StringRef Query,
+                                         StringRef Category) const {
   for (auto &SectionIter : Sections)
-    if (SectionIter.SectionMatcher->match(Section) &&
-        inSection(SectionIter.Entries, Prefix, Query, Category))
-      return true;
-
-  return false;
+    if (SectionIter.SectionMatcher->match(Section)) {
+      unsigned Blame =
+          inSectionBlame(SectionIter.Entries, Prefix, Query, Category);
+      if (Blame)
+        return Blame;
+    }
+  return 0;
 }
 
-bool SpecialCaseList::inSection(const SectionEntries &Entries, StringRef Prefix,
-                                StringRef Query, StringRef Category) const {
+unsigned SpecialCaseList::inSectionBlame(const SectionEntries &Entries,
+                                         StringRef Prefix, StringRef Query,
+                                         StringRef Category) const {
   SectionEntries::const_iterator I = Entries.find(Prefix);
-  if (I == Entries.end()) return false;
+  if (I == Entries.end()) return 0;
   StringMap<Matcher>::const_iterator II = I->second.find(Category);
-  if (II == I->second.end()) return false;
+  if (II == I->second.end()) return 0;
 
   return II->getValue().match(Query);
 }
 
 }  // namespace llvm

unittests/Support/SpecialCaseListTest.cpp

TEST_F(SpecialCaseListTest, Basic) {
   EXPECT_TRUE(SCL->inSection("", "src", "bye"));
   EXPECT_TRUE(SCL->inSection("", "src", "hi", "category"));
   EXPECT_TRUE(SCL->inSection("", "src", "zzzz", "category"));
   EXPECT_FALSE(SCL->inSection("", "src", "hi"));
   EXPECT_FALSE(SCL->inSection("", "fun", "hello"));
   EXPECT_FALSE(SCL->inSection("", "src", "hello", "category"));
 }
 
+TEST_F(SpecialCaseListTest, BasicBlame) {
+  std::unique_ptr<SpecialCaseList> SCL =
+      makeSpecialCaseList("# This is a comment.\n"
+                          "\n"
+                          "src:hello\n"
+                          "src:bye\n"
+                          "src:hi=category\n"
+                          "src:z*=category\n");
+  EXPECT_EQ(3u, SCL->inSectionBlame("", "src", "hello"));
+  EXPECT_EQ(4u, SCL->inSectionBlame("", "src", "bye"));
+  EXPECT_EQ(5u, SCL->inSectionBlame("", "src", "hi", "category"));
+  EXPECT_EQ(6u, SCL->inSectionBlame("", "src", "zzzz", "category"));
+  EXPECT_EQ(0u, SCL->inSectionBlame("", "src", "hi"));
+  EXPECT_EQ(0u, SCL->inSectionBlame("", "fun", "hello"));
+  EXPECT_EQ(0u, SCL->inSectionBlame("", "src", "hello", "category"));
+}
+
+TEST_F(SpecialCaseListTest, CorrectErrorLineNumberWithBlankLine) {
+  std::string Error;
+  EXPECT_EQ(nullptr, makeSpecialCaseList("# This is a comment.\n"
+                                         "\n"
+                                         "[not valid\n",
+                                         Error));
+  EXPECT_TRUE(
+      ((StringRef)Error).startswith("malformed section header on line 3:"));
+}
+
 TEST_F(SpecialCaseListTest, SectionRegexErrorHandling) {
   std::string Error;
   EXPECT_EQ(makeSpecialCaseList("[address", Error), nullptr);
   EXPECT_TRUE(((StringRef)Error).startswith("malformed section header "));
 
   EXPECT_EQ(makeSpecialCaseList("[[]", Error), nullptr);
   EXPECT_TRUE(((StringRef)Error).startswith("malformed regex for section [: "));