erichkeane added a comment. In D147661#4249244 <https://reviews.llvm.org/D147661#4249244>, @rsandifo-arm wrote:
> In D147661#4249004 <https://reviews.llvm.org/D147661#4249004>, @erichkeane > wrote: > >> This test doesn't show a change in behavior. > > Yeah. Is there a way to force the syntax and spelling fields to be dumped, > even for implicit attributes? I was struggling to find one, which is why I > wasn't sure this made a difference in practice. > > The patch is supposed to be a minor clean-up, since similar calls in > surrounding code uses `AL` rather than `AL.getRange()`. The reason I > originally hit this was that the series I'm working on removes the: > > AttributeCommonInfo(SourceRange) > AttributeCommonInfo(SourceLocation) > > constructors (but keeps the ability to use `FooAttr::CreateImplicit` without > more than a source range). It might be easier to explain why that seemed a > good idea alongside the patches themselves. > > The two main uses of these constructors outside tablegen-generated code were > the two in this patch and the one in https://reviews.llvm.org/D147657. Ah, hrm... I don't have a really good idea of how to test this now then. Typically you'd have to do manual dump logic for an attribute like this i think? Perhaps @aaron.ballman knows better. I see the value to this, but I guess I want to see if we can capture the change somehow in a test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147661/new/ https://reviews.llvm.org/D147661 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
