[llvm-commits] [PATCH] Refactor ASan passes options and add blacklist file to them

[Chandler Carruth]

================
Comment at: include/llvm/Transforms/Instrumentation.h:38-51
@@ -35,1 +37,16 @@
 
+struct AddressSanitizerOptions {
+  const bool CheckInitOrder;
+  const bool CheckUseAfterReturn;
+  const bool CheckLifetime;
+  const StringRef BlacklistFile;
+  AddressSanitizerOptions(bool CheckInitOrder = false,
+                          bool CheckUseAfterReturn = false,
+                          bool CheckLifetime = false,
+                          StringRef BlacklistFile = StringRef())
+      : CheckInitOrder(CheckInitOrder),
+        CheckUseAfterReturn(CheckUseAfterReturn),
+        CheckLifetime(CheckLifetime),
+        BlacklistFile(BlacklistFile) {}
+};
+
----------------
Hmm. Why factor this into a struct? Do you forsee this continuing to grow? Adding the stringref param (with default) to both pass create functions seems not a big incremental growth.

I'm not sure we have any really good precedent for nice options structs in LLVM, but if we want to go this route, I think we could fix up the API some to make callers more easy to read. But I'd like to not cross that bridge yet if we can avoid it...

================
Comment at: lib/Transforms/Instrumentation/AddressSanitizer.cpp:206
@@ -200,1 +205,3 @@
+        CheckLifetime(Options.CheckLifetime || ClCheckLifetime),
+        BlacklistFile(chooseBlacklist(Options.BlacklistFile, ClBlackListFile)) {}
   virtual const char *getPassName() const {
----------------
Rather than this, I'd rather something similar to the above: if the options blacklist file isn't empty use it, if it is empty then use the Cl one above. Seems simpler.

I would write it as:

  BlacklistFile(Options.BlacklistFile.empty() ? ClBlackListFile
                                              : Options.BlacklistFile) {}


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