[PATCH] Introduce factory methods for SpecialCaseList

Peter Collingbourne peter at pcc.me.uk
Thu Aug 8 17:35:32 PDT 2013



================
Comment at: lib/Transforms/Utils/SpecialCaseList.cpp:77
@@ +76,3 @@
+  if (Path.empty()) {
+    Error = "Empty path to blacklist file";
+    return 0;
----------------
Why not s/blacklist/special case list/ here? (Actually I can think of a few reasons but I want to know yours.)

================
Comment at: lib/Transforms/Utils/SpecialCaseList.cpp:82
@@ +81,3 @@
+  if (error_code EC = MemoryBuffer::getFile(Path, File)) {
+    Error = (Twine("Can't open blacklist file: ") + Path + ": " +
+        EC.message()).str();
----------------
Same here.

================
Comment at: lib/Transforms/Utils/SpecialCaseList.cpp:76
@@ +75,3 @@
+    const StringRef Path, std::string &Error) {
+  if (Path.empty()) {
+    Error = "Empty path to blacklist file";
----------------
This is a change in behaviour from the StringRef-taking constructor.  There should probably be a non-deprecated way to create an empty special case list, as each of the sanitizers currently needs to do when their special case list parameter is an empty string (which suggests that there should be a function somewhere (which perhaps ought to be this function) which preserves the original behaviour of the StringRef-taking constructor).

================
Comment at: lib/Transforms/Utils/SpecialCaseList.cpp:103
@@ -72,2 +102,3 @@
   StringMap<StringMap<std::string> > Regexps;
+  SCL.Entries.clear();
   for (SmallVectorImpl<StringRef>::iterator I = Lines.begin(), E = Lines.end();
----------------
This may leak memory if this function is called with a nonempty Entries, and it will do nothing if Entries is empty.  Since this function will only do something sensible if Entries is empty, you could assert it here instead.

================
Comment at: include/llvm/Transforms/Utils/SpecialCaseList.h:106
@@ +105,3 @@
+  SpecialCaseList();
+  static bool parse(const MemoryBuffer *MB, SpecialCaseList &SCL,
+                    std::string &Error);
----------------
Nitpick: why make this a static member function?


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



More information about the llvm-commits mailing list