[PATCH] D118276: Adding a DIBuilder interface for Fortran's assumed length string

[Chih-Ping Chen via Phabricator via llvm-commits]

cchen15 added inline comments.


================
Comment at: llvm/include/llvm/IR/DIBuilder.h:229
     /// \param StrLocationExp  Optional memory location of the string.
-    DIStringType *createStringType(StringRef Name,  PointerUnion<Metadata *, DIVariable *>StringLength, Metadata *StrLocationExp=nullptr);
-    
+    DIStringType *createStringType(StringRef Name, Metadata *StringLength,
+                                   Metadata *StrLocationExp = nullptr);
----------------
Use DIVariable * for StringLength.


================
Comment at: llvm/include/llvm/IR/DIBuilder.h:230
+    DIStringType *createStringType(StringRef Name, Metadata *StringLength,
+                                   Metadata *StrLocationExp = nullptr);
+
----------------
Use DIExpression * for StrLocationExp.


================
Comment at: llvm/include/llvm/IR/DIBuilder.h:239
+                                      DIExpression *StringLengthExp,
+                                      Metadata *StrLocationExp = nullptr);
 
----------------
Ditto.


================
Comment at: llvm/unittests/IR/DebugInfoTest.cpp:255
   StringRef StrName = "string";
-  DIStringType *StringType = DIB.createStringType(StrName,nullptr);
+  DIStringType *StringType = DIB.createStringType(StrName, nullptr);
 
----------------
Using nullptr for the length argument seems too generic. Could you mock up a DIVairbale/DIExpression for length and a DIExpression for location for both new functions?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118276/new/

https://reviews.llvm.org/D118276