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

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG636dd1e8a178: Make explicit the single-argument constructors of At

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

aaron.ballman commandeered this revision. aaron.ballman edited reviewers, added: rsandifo-arm; removed: aaron.ballman. aaron.ballman added a comment. In D147661#4255043 , @rsandifo-arm wrote: >> I threw together a patch to make the constructors `explicit

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

rsandifo-arm added a comment. > I threw together a patch to make the constructors `explicit` and the only two > compile failures I have are with what you're fixing in this patch. If you'd > like, I can commandeer this patch and subsume it with the larger refactor. > Alternatively, we can land t

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

aaron.ballman added a comment. In D147661#4249696 , @aaron.ballman wrote: > I don't think there's a way to test this change, unfortunately. However, I > think this exposed a bigger concern, which is that `AttributeCommonInfo` has > some single-argument

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

aaron.ballman added a comment. I don't think there's a way to test this change, unfortunately. However, I think this exposed a bigger concern, which is that `AttributeCommonInfo` has some single-argument constructors that are not `explicit` so that we don't run into this issue again. However, f

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

erichkeane added a comment. In D147661#4249244 , @rsandifo-arm wrote: > In D147661#4249004 , @erichkeane > wrote: > >> This test doesn't show a change in behavior. > > Yeah. Is there a way to force the syntax a

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

rsandifo-arm added a comment. In 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

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

erichkeane added a comment. This test doesn't show a change in behavior. See: https://godbolt.org/z/vrWM1bTPE |-FunctionDecl 0xdbfa5a20 <:3:1, col:60> col:6 foo 'void (void)' | |-AvailabilityAttr 0xdbfa5ac8 ios 2.0 0 0 "" "" 0 | `-AvailabilityAttr 0xdbfa5b80 Implicit maccata

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

rsandifo-arm added a comment. In D147661#4248574 , @erichkeane wrote: > I think the change is probably fine, Thanks! > but can you add an AST-dump test for this one? Sure. How does this look? Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

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

rsandifo-arm updated this revision to Diff 511425. rsandifo-arm added a comment. Add AST dump test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147661/new/ https://reviews.llvm.org/D147661 Files: clang/lib/Sema/SemaDeclAttr.cpp clang/test/Sem

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

erichkeane added a comment. I think the change is probably fine, but can you add an AST-dump test for this one? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147661/new/ https://reviews.llvm.org/D147661 ___

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

rsandifo-arm created this revision. rsandifo-arm added reviewers: arphaman, erichkeane. Herald added a reviewer: aaron.ballman. Herald added a project: All. rsandifo-arm requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Two calls to mergeAvai