[PATCH] Implement categories for global lists.

Alexey Samsonov samsonov at google.com
Thu Jul 4 06:05:46 PDT 2013


  Note: you will need backward compatibility for global-init, global-init-type and global-init-src. There are actual users of these functionality, and we shouldn't silently break them with a random Clang update.


================
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.
----------------
or its type

================
Comment at: include/llvm/Transforms/Utils/GlobalList.h:70
@@ +69,3 @@
+  /// which this function is listed.
+  bool isIn(const Function &F, StringRef &Category) const;
+
----------------
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?

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

================
Comment at: include/llvm/Transforms/Utils/GlobalList.h:104
@@ -56,3 +103,3 @@
  private:
-  StringMap<Regex*> Entries;
+  StringMap<StringMap<Regex*> > Entries;
 
----------------
I think this is now mapping <pair of strings> -> Regex*

================
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;
----------------
findCategory

================
Comment at: lib/Transforms/Utils/GlobalList.cpp:54
@@ -52,3 +53,3 @@
   SplitString(MB->getBuffer(), Lines, "\n\r");
-  StringMap<std::string> Regexps;
+  StringMap<StringMap<std::string> > Regexps;
   for (SmallVector<StringRef, 16>::iterator I = Lines.begin(), E = Lines.end();
----------------
see comment about [pair of strings]->[regexp] above.

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


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



More information about the llvm-commits mailing list