aprantl added inline comments.
================ Comment at: llvm/include/llvm/IR/DIBuilder.h:243 + unsigned LineNo, DIScope *Context, + uint32_t AlignInBits = 0); ---------------- awpandey wrote: > aprantl wrote: > > This should be `Optional<uint32_t>` AlignInBits. Even better perhaps > > `llvm::Align` but perhaps that's too restrictive. > Yes @aprantl this should be of `Optional<unit_32t>` type but changing this > will require change in the `DIDerivedType::get` macro and this macro is used > by many other functions. Please correct me if I am wrong. ?? > > Current functionality of the `createTypeDef` is like the default value of the > `alignInBits` will be `0`. Consider below comment in `DIBuilder.cpp`. Updating `DIDerivedType::get` to use Optional would also be a good change, but for this patch I'd be happy if at least the DIBuilder interface is using optional and then treats None as `0` in the implemenation. We can optionally do another patch to change the interface of `DIDerivedType::get` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70111/new/ https://reviews.llvm.org/D70111 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits