[PATCH] D16077: DIBuilder support DI Macro creation

[Adrian Prantl via llvm-commits]

aprantl added a comment.

IIRC DIBuilder has a couple of unit tests (unittests/IR/IRBuilderTest.cpp). Could you add tests for the new functionality?


================
Comment at: include/llvm/IR/DIBuilder.h:49
@@ -48,2 +48,3 @@
     SmallVector<TrackingMDNodeRef, 4> AllImportedModules;
+    DenseMap<MDNode *, std::vector<Metadata*>> AllMacrosPerParent;
 
----------------
What are the allowed types of parents?

================
Comment at: include/llvm/IR/DIBuilder.h:124
@@ +123,3 @@
+    /// \param Value      Macro value.
+    DIMacro *createMacro(DIMacroFile *Parent, unsigned LineNumber, bool IsUndef,
+                         StringRef Name, StringRef Value = "");
----------------
Instead of passing a bool, which leads to call sites like
/* undef */ true
would it make sense to have inline convenience wrappers like "createMacroDefine/Undef" or passing in the DW_MACINFO_undef value directly?

================
Comment at: include/llvm/IR/DIBuilder.h:125
@@ +124,3 @@
+    DIMacro *createMacro(DIMacroFile *Parent, unsigned LineNumber, bool IsUndef,
+                         StringRef Name, StringRef Value = "");
+
----------------
This is usually written as = StringRef()


http://reviews.llvm.org/D16077