aaron.ballman added inline comments.
================
Comment at: lib/AST/ItaniumMangle.cpp:583
+ // Non-transparent overloadable functions need mangling.
+ if (auto *A = FD->getAttr<OverloadableAttr>())
+ if (!A->isTransparent())
----------------
`const auto *`
================
Comment at: lib/AST/MicrosoftMangle.cpp:372
+ // Non-transparent overloadable functions need mangling.
+ if (auto *A = FD->getAttr<OverloadableAttr>())
+ if (!A->isTransparent())
----------------
`const auto *`
================
Comment at: lib/Sema/SemaDecl.cpp:2820
+ for (const Decl *ND : Base->redecls())
+ if (auto *Ovl = ND->getAttr<OverloadableAttr>())
+ if (!Ovl->isImplicit())
----------------
`const auto *`
================
Comment at: lib/Sema/SemaDecl.cpp:2921
+ const auto *NewOvl = New->getAttr<OverloadableAttr>();
+ if (NewOvl->isTransparent() != OldOvl->isTransparent()) {
+ assert(!NewOvl->isImplicit() &&
----------------
Can `NewOvl` be null here (in an error case)?
================
Comment at: lib/Sema/SemaDecl.cpp:9303
+ } else if (!getLangOpts().CPlusPlus) {
+ if (auto *NewOvl = NewFD->getAttr<OverloadableAttr>()) {
+ if (NewOvl->isTransparent()) {
----------------
`const auto *`
================
Comment at: lib/Sema/SemaDecl.cpp:9306-9307
+ auto Transparent = llvm::find_if(Previous, [](const NamedDecl *ND) {
+ if (auto *FD = dyn_cast<FunctionDecl>(ND))
+ if (auto *Ovl =
+ FD->getMostRecentDecl()->getAttr<OverloadableAttr>())
----------------
`const auto *` in both places.
================
Comment at: test/Sema/overloadable.c:189
+ void to_foo5(int) __attribute__((overloadable)); // expected-note {{previous
overload}}
+ void to_foo5(int) __attribute__((transparently_overloadable)); //
expected-error{{mismatched transparency}}
+
----------------
george.burgess.iv wrote:
> aaron.ballman wrote:
> > Why should this be a mismatch? Attributes can usually be added to
> > redeclarations without an issue, and it's not unheard of for subsequent
> > redeclarations to gain additional attributes. It seems like the behavior
> > the user would expect from this is for the `transparently_overloadable`
> > attribute to "win" and simply replaces, or silently augments, the
> > `overloadable` attribute.
> my issue with this was:
>
> ```
> // foo.h
> void foo(int) __attribute__((overloadable));
>
> // foo.c
> void foo(int) __attribute__((overloadable)) {}
>
> // bar.c
> #include "foo.h"
>
> void foo(int) __attribute__((transparently_overloadable));
>
> // calls to foo(int) now silently call @foo instead of the mangled version,
> but only in this TU
> ```
>
> though, i suppose this code going against our guidance of "overloads should
> generally have internal linkage", and it's already possible to get yourself
> in a similar situation today. so, as long as we don't allow `overloadable` to
> "win" against `transparently_overloadable`, i'm OK with this.
Hmm, I can see how your example might cause confusion for the user as well.
Perhaps downgrading it from an error to a warning and maybe putting something
in the docs about why that warning could lead to bad things would be a good
approach?
https://reviews.llvm.org/D32332
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
