[PATCH] D147661: [Sema] Tweak merging of availability attributes

Thu Apr 6 10:07:03 PDT 2023

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