This revision was automatically updated to reflect the committed changes.
Closed by commit rL306467: [Sema] Allow unmarked overloadable functions.
(authored by gbiv).
Changed prior to commit:
https://reviews.llvm.org/D32332?vs=103448=104277#toc
Repository:
rL LLVM
george.burgess.iv added a comment.
Woohoo! Thanks again to both of you.
https://reviews.llvm.org/D32332
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM, thanks!
https://reviews.llvm.org/D32332
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
george.burgess.iv added a comment.
Thank you both for the comments!
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:3294-3295
+def err_attribute_overloadable_multiple_unmarked_overloads : Error<
+ "at most one 'overloadable' function for a given name may lack the "
+
george.burgess.iv updated this revision to Diff 103448.
george.burgess.iv marked 6 inline comments as done.
george.burgess.iv added a comment.
Addressed all feedback
https://reviews.llvm.org/D32332
Files:
include/clang/Basic/AttrDocs.td
include/clang/Basic/DiagnosticSemaKinds.td
rsmith added inline comments.
Comment at: include/clang/Basic/AttrDocs.td:608-611
declarations and definitions. Most importantly, if any function with a given
name is given the ``overloadable`` attribute, then all function declarations
and definitions with that name (and in
george.burgess.iv updated this revision to Diff 102850.
george.burgess.iv marked 3 inline comments as done.
george.burgess.iv added a comment.
Addressed all feedback
https://reviews.llvm.org/D32332
Files:
include/clang/Basic/AttrDocs.td
include/clang/Basic/DiagnosticSemaKinds.td
aaron.ballman added a comment.
This is generally looking good to me, with a few small nits. @rsmith, do you
have thought on this?
Comment at: lib/Sema/SemaDecl.cpp:2875-2876
/// Returns true if there was an error, false otherwise.
-bool Sema::MergeFunctionDecl(FunctionDecl
george.burgess.iv updated this revision to Diff 102207.
george.burgess.iv added a comment.
Swap to using `__has_feature(overloadable_unmarked)` for detection, as
recommended by Hal in https://reviews.llvm.org/D33904
https://reviews.llvm.org/D32332
Files:
include/clang/Basic/AttrDocs.td
george.burgess.iv added a comment.
Ping :)
https://reviews.llvm.org/D33904 is what I imagine would be a good way to detect
this change.
https://reviews.llvm.org/D32332
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
george.burgess.iv updated this revision to Diff 100201.
george.burgess.iv added a comment.
Herald added a subscriber: jfb.
Fix the aforementioned issue; PTAL.
Note that this fix is slightly backwards incompatible. It disallows code like:
void foo(int);
void foo(int)
aaron.ballman added a comment.
In https://reviews.llvm.org/D32332#751785, @george.burgess.iv wrote:
> I'd be happy with that approach. Do you like it, Aaron?
I think that makes a lot of sense.
> FWIW, I did a bit of archaeology, and it looks like the commit that added the
> requirement that
george.burgess.iv added a comment.
I'd be happy with that approach. Do you like it, Aaron?
FWIW, I did a bit of archaeology, and it looks like the commit that added the
requirement that all overloads must have `overloadable` (r64414) did so to keep
users from "trying to be too sneaky for their
rsmith added a comment.
I'd like to suggest an alternative design: don't add a new attribute., and
instead change the semantics of `__attribute__((overloadable))` to permit at
most one non-overloadable function in an overload set. That one function would
implicitly get the
george.burgess.iv added inline comments.
Comment at: lib/Sema/SemaDecl.cpp:2921
+ const auto *NewOvl = New->getAttr();
+ if (NewOvl->isTransparent() != OldOvl->isTransparent()) {
+assert(!NewOvl->isImplicit() &&
aaron.ballman wrote:
> Can
george.burgess.iv updated this revision to Diff 98569.
george.burgess.iv marked 7 inline comments as done.
george.burgess.iv added a comment.
- Addressed all feedback
- Now we emit a warning when `transparently_overloadable` "overrides"
`overloadable`, rather than an error
aaron.ballman added inline comments.
Comment at: lib/AST/ItaniumMangle.cpp:583
+// Non-transparent overloadable functions need mangling.
+if (auto *A = FD->getAttr())
+ if (!A->isTransparent())
`const auto *`
Comment at:
george.burgess.iv updated this revision to Diff 97698.
george.burgess.iv added a comment.
We now require `transparently_overloadable` on all redeclarations. Allowing
mixed `transparently_overloadable` and `overloadable` coming soon.
https://reviews.llvm.org/D32332
Files:
george.burgess.iv added a comment.
> I'd like to understand this use case a little bit better. If you don't
> perform the mangling, then choosing which overload gets called is already
> based on the source order, isn't it? See https://godbolt.org/g/8b10Ns
I think I might have miscommunicated:
aaron.ballman added a comment.
In https://reviews.llvm.org/D32332#742796, @george.burgess.iv wrote:
> thanks for the feedback!
>
> fwiw, at a high level, the main problem i'm trying to solve with this is that
> you can't really make a `__overloadable_without_mangling` macro without
> handing
george.burgess.iv added a comment.
thanks for the feedback!
fwiw, at a high level, the main problem i'm trying to solve with this is that
you can't really make a `__overloadable_without_mangling` macro without handing
it the function name, since you currently need to use
aaron.ballman added a reviewer: rsmith.
aaron.ballman added a comment.
I'm adding Richard to the review because he may have opinions on this
functionality.
Is there a reason we want the second spelling instead of making the current
spelling behave in the manner you describe? While the change
george.burgess.iv added a comment.
ping :)
https://reviews.llvm.org/D32332
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
george.burgess.iv created this revision.
One of the common use-cases for the `overloadable` attribute is to create small
wrapper functions around an existing function that was previously not
overloadable. For example:
// C Library v1
void foo(int i);
// C Library v2
// Note the
24 matches
Mail list logo
