[PATCH] Implement categories for global lists.

[Peter Collingbourne]

  Added back compat.


================
Comment at: include/llvm/Transforms/Utils/GlobalList.h:76
@@ +75,3 @@
+
+  /// Returns whether either this global or its source file are listed in the
+  /// empty category.
----------------
Alexey Samsonov wrote:
> or its type
Fixed.

================
Comment at: include/llvm/Transforms/Utils/GlobalList.h:104
@@ -56,3 +103,3 @@
  private:
-  StringMap<Regex*> Entries;
+  StringMap<StringMap<Regex*> > Entries;
 
----------------
Alexey Samsonov wrote:
> I think this is now mapping <pair of strings> -> Regex*
Yes, but findCategory needs to be able to enumerate the categories for a given prefix, and this would be cumbersome if we represented this as a mapping from a pair of strings.  (Also, we probably wouldn't be able to use the specialised StringMap data structure.)

================
Comment at: include/llvm/Transforms/Utils/GlobalList.h:65
@@ +64,3 @@
+  /// empty category.
+  bool isIn(const Function &F) const { return isInCategory(F, StringRef()); }
+
----------------
Alexey Samsonov wrote:
> I would group "isIn", "isInCategory", and "findCategory" overloads together and use a common each of them.
> I would group "isIn", "isInCategory", and "findCategory" overloads together

OK, done.  I also realised that I can use a default argument for the category to cut down on duplication.

> and use a common each of them.

I don't understand this.

================
Comment at: include/llvm/Transforms/Utils/GlobalList.h:70
@@ +69,3 @@
+  /// which this function is listed.
+  bool isIn(const Function &F, StringRef &Category) const;
+
----------------
Alexey Samsonov wrote:
> I think this function (and friends) should be renamed to findCategory(const {Function,Module,GlobalVariable} &F, StringRef &Category);
> 
> What is the use case for these functions? Why return an arbitrary category instead of all of them?
> I think this function (and friends) should be renamed to findCategory(const {Function,Module,GlobalVariable} &F, StringRef &Category);

Yes, that's definitely a better name.

> What is the use case for these functions? Why return an arbitrary category instead of all of them?

The file format supports many-to-one function/global/module -> category mapping, as well as many-to-many.  This function is designed for the case where the sanitizer expects the mapping to be many-to-one.  We could conceivably return some failure state from this function if a function maps to more than one category, but I'd prefer to avoid complexity here at least for now.

================
Comment at: include/llvm/Transforms/Utils/GlobalList.h:107
@@ -59,2 +106,3 @@
   void init(MemoryBuffer *MB);
-  bool inSection(const StringRef Section, const StringRef Query) const;
+  bool inSection(const StringRef Section, const StringRef Query,
+                 StringRef &Category) const;
----------------
Alexey Samsonov wrote:
> findCategory
Done.

================
Comment at: lib/Transforms/Utils/GlobalList.cpp:99
@@ +98,3 @@
+         II != IE; ++II) {
+      Entries[I->getKey()][II->getKey()] = new Regex(II->getValue());
+    }
----------------
Alexey Samsonov wrote:
> who will destroy this Regex?
> Looks like you need ~BlackList();
Yes, added.


http://llvm-reviews.chandlerc.com/D1092