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 that scope) must have the
``overloadable`` attribute. This rule even applies to redeclarations of
----------------
Can you update this to point out the exception below?
================
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 "
+ "'overloadable' attribute">;
def warn_ns_attribute_wrong_return_type : Warning<
----------------
I think calling the functions 'overloadable' in this case is confusing.
================
Comment at: lib/Lex/PPMacroExpansion.cpp:1136
.Case("nullability_on_arrays", true)
+ .Case("overloadable_unmarked", true)
.Case("memory_sanitizer", LangOpts.Sanitize.has(SanitizerKind::Memory))
----------------
Should this be `__has_extension` rather than `__has_feature`, since it's not a
standard feature? (Per
http://clang.llvm.org/docs/LanguageExtensions.html#has-feature-and-has-extension
`__has_feature` is only for detecting standard features, despite us using it
for other things.)
================
Comment at: lib/Sema/SemaDecl.cpp:9249-9252
+ case Ovl_Match:
+ // We're not guaranteed to be handed the most recent decl, and
+ // __attribute__((overloadable)) depends somewhat on source order.
+ OldDecl = OldDecl->getMostRecentDecl();
----------------
`checkForConflictWithnonVisibleExternC` should probably be responsible for
finding the most recent declaration of the entity that is actually visible (and
not, say, a block-scope declaration in some unrelated function, or a
declaration in an unimported module).
================
Comment at: lib/Sema/SemaDecl.cpp:9375-9381
+ assert((Previous.empty() ||
+ llvm::any_of(
+ Previous,
+ [](const NamedDecl *ND) {
+ return ND->getMostRecentDecl()->hasAttr<OverloadableAttr>();
+ })) &&
+ "Non-redecls shouldn't happen without overloadable present");
----------------
Seems worth factoring out this "contains any overloadable functions" check,
since it's quite a few lines and you're doing it in two different places.
================
Comment at: test/Sema/overloadable.c:112
void (*ptr2)(char *) = &foo;
- void (*ambiguous)(int *) = &foo; // expected-error{{initializing 'void
(*)(int *)' with an expression of incompatible type '<overloaded function
type>'}} expected-note@105{{candidate function}} expected-note@106{{candidate
function}}
- void *vp_ambiguous = &foo; // expected-error{{initializing 'void *' with an
expression of incompatible type '<overloaded function type>'}}
expected-note@105{{candidate function}} expected-note@106{{candidate function}}
+ void (*ambiguous)(int *) = &foo; // expected-error{{initializing 'void
(*)(int *)' with an expression of incompatible type '<overloaded function
type>'}} expected-note@-4{{candidate function}} expected-note@-3{{candidate
function}}
+ void *vp_ambiguous = &foo; // expected-error{{initializing 'void *' with an
expression of incompatible type '<overloaded function type>'}}
expected-note@-5{{candidate function}} expected-note@-4{{candidate function}}
----------------
It'd be nice to check in these changes from @NNN -> @-M separately.
https://reviews.llvm.org/D32332
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
