dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.
I'll commit this shortly.
================
Comment at: llvm/lib/IR/DIBuilder.cpp:351
DIScope *Context, uint32_t AlignInBits,
+ DINode::DIFlags Flags,
DINodeArray Annotations) {
----------------
J-Camilleri wrote:
> dblaikie wrote:
> > While we usually split llvm & clang commits, in this case since it changes
> > an API used by clang, this part of the llvm change should go along with the
> > clang change.
> Regarding this and the previous comment about separating the llvm commit.
>
> Am I to have 2 commits as follows?
> 1. change llvm to emit flag + test case
> 2. modify llvm interface + change clang to emit flag + test case
>
> I do not have merge rights so I will ask for this when I get to the merge
> stage.
>
> Thanks for the review.
If you don't have commit rights, no worries - I can sort this out when I commit
this on your behalf.
But yes, your list of the two steps/order/pieces is correct.
================
Comment at: llvm/test/DebugInfo/X86/debug-info-access.ll:12-32
+;
+; using pub_default_using = int;
+; pub_default_using a_pub;
; };
;
; class B : public A {
; public:
----------------
J-Camilleri wrote:
> dblaikie wrote:
> > Probably don't need all this test coverage - since most of this is
> > motivated by the features up in clang - on the LLVM side we just need one
> > case that demonstrates that if we put an access specifier in the flags,
> > that gets emitted.
> I left the llvm to be the same as generated by the cpp file but changed the
> `CHECK` to just verify that the accessibility flag is emitted for a `typedef`.
Ah, makes sense - thanks!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134339/new/
https://reviews.llvm.org/D134339
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
