[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:237
     /// \param StrLocationExp   Optional memory location of the string.
-    DIStringType *createStringTypeExp(StringRef Name,
+    DIStringType *createStringType(StringRef Name,
                                       DIExpression *StringLengthExp,
----------------
bwyma wrote:
> cchen15 wrote:
> > One last suggestion: We can combine the two new functions into one by typing the length argument PointerUnion<DIVariable*, DIExpression*>.  Please see DIBuilder::createArrayType for a precedent.
> In the createArrayType() instance, the expression and variable arguments are interchangeable metadata to the same composite type parameter. In DIStringType, at least currently, the variable string length and string length expression are independent parameters. To reflect this in the DIBuilder interface I would prefer to have independent createStringType() calls.
DIStringType should have only one field to represent the string length. I think this boils down to if the DIBuilder interface needs to reflect the current state of the underlying implementation. My take is that we can go ahead with what the user should see with this patch, and then change the implementation at a later time if necessary to avoid an additional API change down the road.



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

https://reviews.llvm.org/D118276