Add support for transparent overloadable functions in clang

[george.burgess.iv]

Summary

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 __asm__("foo"); it ensures that foo isn't mangled. Our docs state
// that the mangling isn't stable, so we encourage users to either use __asm__
// to force a stable name, or to only use overloads with internal linkage
__attribute__((overloadable))
int foo(int i) __asm__("foo");

__attribute__((overloadable))
static inline int foo(long l) {
  assert(l <= INT_MAX && l >= INT_MIN && "Out of bounds number!");
  return foo(l);
}

"Library v2" is suboptimal for a few reasons:

• It can break existing code; if users added int foo(int); declarations to their code, upgrading to library v2 requires that the users either remove these declarations or add __attribute__((overloadable)) to them.
• The __asm__ name is both redundant and easy to typo (or forget to update, or ...).
• The __asm__ name also makes it quite a bit more difficult to hide all of this behind a macro, since we need a different name for each function.

This patch aims to fix these usability challenges with the idea of transparent
function overloads.

For any overloadable function in C, one of the overloads is allowed to be
transparent. Transparency means that the name will not be mangled, and later
redeclarations of that particular overload don't require
__attribute__((overloadable)).

Users can make an overload transparent by using the transparently_overloadable
spelling of overloadable.

This also includes a bugfix for how we handled overloadable functions declared
inside functions. I can factor that out if that would make life easier. :)

[george.burgess.iv]

ping :)

[aaron.ballman]

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 is fairly fundamental, I don't know that it would break code either (but I could be wrong). I think that forcing the user to choice which spelling of "overloadable" they want to use is not very user friendly.

include/clang/Basic/Attr.td

1416 ↗

(On Diff #96074)

I'm not too keen on the name, especially since there's already a "transparent_union" attribute, where the "transparent" means something slightly different than it does here. However, my attempts to dream up a better name are coming up short...

include/clang/Basic/DiagnosticSemaKinds.td

3293

Why should this be an error instead of simply an ignored attribute?

test/Sema/overloadable.c

189

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.

[george.burgess.iv]

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 __asm__("name-you'd-like-the-function-to-have") to rename the function. if you can think of a better way to go about this, even if it requires that we drop the "no overloadable required on some redeclarations" feature this adds, i'm all ears. :)

Is there a reason we want the second spelling instead of making the current spelling behave in the manner you describe?

there are two features this is adding, and i'm not sure which you want overloadable to imply.

1. this new spelling brings with it the promise that we won't mangle function names; if we made every overloadable act this way, we'd have:

void foo(int) __attribute__((overloadable)); // emitted as `declare void @foo(i32)`
void foo(long) __attribute__((overloadable)); // emitted as `declare void @foo(i64)`. LLVM gets angry, since we have different declarations for @foo.

the only way i can think of to get "no indication of which overloads aren't mangled" to work is choosing which overload to mangle based on source order, and that seems overly subtle to me. so, i think this behavior needs to be spelled out in the source.

2. for the "no overloadable required on redecls" feature this adds: i wasn't there for the initial design of the overloadable attribute, so i can't say for certain, but i think part of the reason that we required overloadable to be on every redecl was so that people would have an indication of "hey, by the way, this name will probably be mangled and there may be other functions you end up calling." in C, i can totally see that being valuable. if you're suggesting we relax that for all overloads, i'll find who originally implemented all of this to figure out what their reasoning was and get back to you. :)

---

in any case, whether we actually do this as a new spelling, an optional argument to overloadable, etc. makes no difference to me. i chose a new spelling because AFAICT, we don't currently have a standard way for a user to query clang for if it has support for specific features in an attribute. (outside of checking clang's version, but that's discouraged AFAIK.) if we decide an optional arg would be a better approach than a new spelling, i'm happy to add something like __has_attribute_ext so you can do __has_attribute_ext(overloadable, transparent_overloads).

I think that forcing the user to choice which spelling of "overloadable" they want to use is not very user friendly.

yeah. the idea was that users should only need to reach for transparently_overloadable if they're trying to make a previously non-overloadable function overloadable. like said, if you think there's a better way to surface this functionality, i'm all for it.

include/clang/Basic/Attr.td

1416 ↗

(On Diff #96074)

yeah :/

my only other idea was implicitly_overloadable. maybe that would be preferable?

include/clang/Basic/DiagnosticSemaKinds.td

3293

if the user asks for us to not mangle two overloads with the same name, i don't think we can (sanely) go to CodeGen. see below.

test/Sema/overloadable.c

189

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.

[aaron.ballman]

In 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 it the function name, since you currently need to use __asm__("name-you'd-like-the-function-to-have") to rename the function. if you can think of a better way to go about this, even if it requires that we drop the "no overloadable required on some redeclarations" feature this adds, i'm all ears. :)

Yeah, it would require the function name (then you could use the stringize preprocessor directive).

Is there a reason we want the second spelling instead of making the current spelling behave in the manner you describe?

there are two features this is adding, and i'm not sure which you want overloadable to imply.

1. this new spelling brings with it the promise that we won't mangle function names; if we made every overloadable act this way, we'd have:

void foo(int) __attribute__((overloadable)); // emitted as `declare void @foo(i32)`
void foo(long) __attribute__((overloadable)); // emitted as `declare void @foo(i64)`. LLVM gets angry, since we have different declarations for @foo.

the only way i can think of to get "no indication of which overloads aren't mangled" to work is choosing which overload to mangle based on source order, and that seems overly subtle to me. so, i think this behavior needs to be spelled out in the source.

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

2. for the "no overloadable required on redecls" feature this adds: i wasn't there for the initial design of the overloadable attribute, so i can't say for certain, but i think part of the reason that we required overloadable to be on every redecl was so that people would have an indication of "hey, by the way, this name will probably be mangled and there may be other functions you end up calling." in C, i can totally see that being valuable. if you're suggesting we relax that for all overloads, i'll find who originally implemented all of this to figure out what their reasoning was and get back to you. :)

The logic as to why overloadable is required on every redeclaration makes sense, but also flies in the face of your rational for why you want a new spelling of the same attribute that doesn't need to be on every redeclaration. I worry about this difference being mildly confusing to user.

in any case, whether we actually do this as a new spelling, an optional argument to overloadable, etc. makes no difference to me. i chose a new spelling because AFAICT, we don't currently have a standard way for a user to query clang for if it has support for specific features in an attribute. (outside of checking clang's version, but that's discouraged AFAIK.) if we decide an optional arg would be a better approach than a new spelling, i'm happy to add something like __has_attribute_ext so you can do __has_attribute_ext(overloadable, transparent_overloads).

That's a fair reason for a new spelling rather than an argument.

I think that forcing the user to choice which spelling of "overloadable" they want to use is not very user friendly.

yeah. the idea was that users should only need to reach for transparently_overloadable if they're trying to make a previously non-overloadable function overloadable. like said, if you think there's a better way to surface this functionality, i'm all for it.

[george.burgess.iv]

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: transparently_overloadable only enforces no mangling on one function out of the set of functions with the same name. So,

void foo(int) __attribute__((transparently_overloadable));
void foo(float) __attribute__((overloadable));

void bar() {
  foo(1); // calls @foo
  foo(1f); // calls @_Z3foof
}

The logic as to why overloadable is required on every redeclaration makes sense, but also flies in the face of your rational for why you want a new spelling of the same attribute that doesn't need to be on every redeclaration. I worry about this difference being mildly confusing to user.

Yeah, good point. The more I think about that, the more I don't like the inconsistency; I'll drop that change.

[george.burgess.iv]

We now require transparently_overloadable on all redeclarations. Allowing mixed transparently_overloadable and overloadable coming soon.

[aaron.ballman]

lib/AST/ItaniumMangle.cpp

583 ↗

(On Diff #97698)

const auto *

lib/AST/MicrosoftMangle.cpp

372 ↗

(On Diff #97698)

const auto *

lib/Sema/SemaDecl.cpp

2875

const auto *

2944

Can NewOvl be null here (in an error case)?

9374

const auto *

9377–9378

const auto * in both places.

test/Sema/overloadable.c

189

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?

[george.burgess.iv]

• Addressed all feedback
• Now we emit a warning when transparently_overloadable "overrides" overloadable, rather than an error

lib/Sema/SemaDecl.cpp

2944

If it's null here, then the bug is in another piece of code.

In theory, after one FunctionDecl with some name N is tagged with overloadable, all proceeding declarations/definition(s) with the name N should be tagged with overloadable. We'll make implicit OverloadableAttrs if the user fails to do this (fixMissingOverloadableAttr).

...Though, this code doesn't do a good job of calling that out. I tried adding a note of this to Sema::CheckFunctionDeclaration; I dunno if there's a better place to put it.

Regardless, added an assert here to clarify. :)

test/Sema/overloadable.c

189

Done.

I wasn't sure what you thought was best in the case of

void foo(int) __attribute__((overloadable));
void foo(int) __attribute__((transparently_overloadable));
void foo(int) __attribute__((overloadable));

so I made clang emit an error on the last redeclaration of foo because it's not transparently_overloadable. If you'd prefer for us to silently turn the last overloadable into transparently_overloadable, I'm happy to allow that, too.

(In any case, we'll now warn about going from overloadable -> transparently_overloadable on the middle decl)

[rsmith]

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 transparently_overloadable semantics: it can be overloaded by overloadable functions and doesn't get a mangled name. This is basically exactly how extern "C" and extern "C++" functions overload in C++: you can have up to one extern "C" function plus as many (overloadable) C++ functions as you like.

Does that seem reasonable? Or is there some reason we need or want an explicit attribute for this case?

[george.burgess.iv]

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 own good." The issue it was trying to solve was

double sin(double) __attribute__((overloadable));
#include <math.h>

// calls to `sin` are now mangled, which probably resulted in fun linker errors.

If we go back to no longer requiring some spelling of overloadable on all (re)decls of this special non-mangled function, the above problem shouldn't reappear.

[aaron.ballman]

In 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 all overloads must have overloadable (r64414) did so to keep users from "trying to be too sneaky for their own good." The issue it was trying to solve was

double sin(double) __attribute__((overloadable));
#include <math.h>

// calls to `sin` are now mangled, which probably resulted in fun linker errors.

If we go back to no longer requiring some spelling of overloadable on all (re)decls of this special non-mangled function, the above problem shouldn't reappear.

[george.burgess.iv]

Remove the transparent_overloadable attribute entirely.

This approach presents one problem that I didn't see until I implemented it: I'd like to have something to detect that this feature exists. The quick fix seems to be "readd transparently_overloadable, make it equivalent to not having the attribute at all, and be happy," but that feels really icky (as does adding a special __has_enhanced_overloadable or whatever macro just for this).

Do we have a standard way of saying "does clang support an enhanced version of attribute X"? If not, I'm happy to put together a patch so people can query for that in a somewhat uniform way. This would let users write something like __has_attribute_enhancement(overloadable, unmarked_overloads), and would be more broadly useful if we decide to ever add features to another attribute in the future (diagnose_if comes to mind if I can ever find time to get back to it...).

Apologies for the lag; life is busy. :)

[george.burgess.iv]

Shortly after I pressed submit, I realized that this patch allows the following code if you tweak an assert to check for overloadable on the most recent redecl of a function:

void foo(int);
void foo(int) __attribute__((overloadable));
void foo(float);
void foo(float) __attribute__((overloadable));
void foo(double);
void foo(double) __attribute__((overloadable));

...Which is bad. I'll fix that soon. :)

[george.burgess.iv]

Fix the aforementioned issue; PTAL.

Note that this fix is slightly backwards incompatible. It disallows code like:

void foo(int);
void foo(int) __attribute__((overloadable)); // first decl lacked overloadable, so this one must lack it, as well.

I chose to do it this way because r64414 wanted to make us reject code like:

double sin(double) __attribute__((overloadable));
#include <math.h> // error: sin redecl without overloadable

...but it was trivial to get around that by moving the overloadable sin decl below the include. So, I feel like us accepting the first code snippet is a small bug.

I tested this change on ChromeOS, which uses overloadable in some parts of its C standard library. This backwards incompatibility caused no build breakages on that platform.

If we'd rather stay as backwards-compatible as possible, I have no issues with tweaking this to allow the first example.

[george.burgess.iv]

Ping :)

https://reviews.llvm.org/D33904 is what I imagine would be a good way to detect this change.

[george.burgess.iv]

Swap to using __has_feature(overloadable_unmarked) for detection, as recommended by Hal in https://reviews.llvm.org/D33904

[aaron.ballman]

This is generally looking good to me, with a few small nits. @rsmith, do you have thought on this?

lib/Sema/SemaDecl.cpp

2875–2876

Seems to be a formatting-only change that's unrelated to the patch; can be reverted.

9225

Why add Sema:: to these?

9383

Why does this need a capture?

[george.burgess.iv]

Addressed all feedback

[rsmith]

include/clang/Basic/AttrDocs.td

608–611

Can you update this to point out the exception below?

include/clang/Basic/DiagnosticSemaKinds.td

3294–3295

I think calling the functions 'overloadable' in this case is confusing.

lib/Lex/PPMacroExpansion.cpp

1136

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.)

lib/Sema/SemaDecl.cpp

9249–9252

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).

9375–9381

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.

test/Sema/overloadable.c

112–113

It'd be nice to check in these changes from @NNN -> @-M separately.

[george.burgess.iv]

Addressed all feedback

[george.burgess.iv]

Thank you both for the comments!

include/clang/Basic/DiagnosticSemaKinds.td

3294–3295

Yeah, I couldn't think of a good name, either. Is this better?

lib/Lex/PPMacroExpansion.cpp

1136

Ah, I missed the "For backward compatibility" bit. Good call

lib/Sema/SemaDecl.cpp

9225

Artifact of me moving code around and then putting it back. :)

9249–9252

Isn't part of the goal of checkForConflictWithNonVisibleExternC to report things that (potentially) aren't visible? If so, I don't see how it can also be its responsibility to always report things that are visible, unless it's a best-effort sort of thing.

In any case, this bit of code boils down to me trying to slip a bugfix in here. Since I think fixing this properly will just pollute this diff even more, I'll back it out and try again in a separate patch. :)

9375–9381

With the removal of one unnecessary change (see a later response), this is no longer repeated. Happy to still pull it out into a function if you think that helps readability.

test/Sema/overloadable.c

112–113

r305947.

[rsmith]

LGTM, thanks!

[george.burgess.iv]

Woohoo! Thanks again to both of you.

include/clang/Basic/AttrDocs.td

 
 * If no viable candidates are otherwise available, we allow a conversion from a
   pointer of type ``T*`` to a pointer of type ``U*``, where ``T`` and ``U`` are
   incompatible. This conversion is ranked below all other types of conversions.
   Please note: ``U`` lacking qualifiers that are present on ``T`` is sufficient
   for ``T`` and ``U`` to be incompatible.
 
 The declaration of ``overloadable`` functions is restricted to function
 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
 functions whose original declaration had the ``overloadable`` attribute, e.g.,
 
 .. code-block:: c
 
   int f(int) __attribute__((overloadable));
   float f(float); // error: declaration of "f" must have the "overloadable" attribute
 
   int g(int) __attribute__((overloadable));
* Future versions of Clang may change the name mangling of functions overloaded
   functions.
 
 * The ``overloadable`` attribute has almost no meaning when used in C++,
   because names will already be mangled and functions are already overloadable.
   However, when an ``overloadable`` function occurs within an ``extern "C"``
   linkage specification, it's name *will* be mangled in the same way as it
   would in C.
 
-Query for this feature with ``__has_extension(attribute_overloadable)``.
+For the purpose of backwards compatibility, at most one function with the same
+name as other ``overloadable`` functions may omit the ``overloadable``
+attribute. In this case, the function without the ``overloadable`` attribute
+will not have its name mangled.
+
+For example:
+
+.. code-block:: c
+
+  // Notes with mangled names assume Itanium mangling.
+  int f(int);
+  int f(double) __attribute__((overloadable));
+  void foo() {
+    f(5); // Emits a call to f (not _Z1fi, as it would with an overload that
+          // was marked with overloadable).
+    f(1.0); // Emits a call to _Z1fd.
+  }
+
+Support for unmarked overloads is not present in some versions of clang. You may
+query for it using ``__has_feature(overloadable_unmarked)``.
+
+Query for this attribute with ``__has_attribute(overloadable)``.
   }];
 }
 
 def ObjCMethodFamilyDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
 Many methods in Objective-C have conventional meanings determined by their
 selectors. It is sometimes useful to be able to mark a method as having a

include/clang/Basic/DiagnosticSemaKinds.td

def err_iboutletcollection_builtintype : Error<
   "type argument of iboutletcollection attribute cannot be a builtin type">;
 def warn_iboutlet_object_type : Warning<
   "%select{instance variable|property}2 with %0 attribute must "
   "be an object type (invalid %1)">, InGroup<ObjCInvalidIBOutletProperty>;
 def warn_iboutletcollection_property_assign : Warning<
   "IBOutletCollection properties should be copy/strong and not assign">,
   InGroup<ObjCInvalidIBOutletProperty>;
   
-def err_attribute_overloadable_missing : Error<
-  "%select{overloaded function|redeclaration of}0 %1 must have the "
-  "'overloadable' attribute">;
+def err_attribute_overloadable_mismatch : Error<
+  "redeclaration of %0 must %select{not |}1have the 'overloadable' attribute">;
 def note_attribute_overloadable_prev_overload : Note<
-  "previous overload of function is here">;
+  "previous %select{unmarked |}0overload of function is here">;
 def err_attribute_overloadable_no_prototype : Error<
   "'overloadable' function %0 must have a prototype">;
+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<
   "%0 attribute only applies to %select{functions|methods|properties}1 that "
   "return %select{an Objective-C object|a pointer|a non-retainable pointer}2">,
   InGroup<IgnoredAttributes>;
 def err_ns_attribute_wrong_parameter_type : Error<
   "%0 attribute only applies to "
   "%select{Objective-C object|pointer|pointer-to-CF-pointer}1 parameters">;
 def warn_ns_attribute_wrong_parameter_type : Warning<

include/clang/Sema/Sema.h

public:
   /// same list more than once.
   std::unique_ptr<RecordDeclSetTy> PureVirtualClassDiagSet;
 
   /// ParsingInitForAutoVars - a set of declarations with auto types for which
   /// we are currently parsing the initializer.
   llvm::SmallPtrSet<const Decl*, 4> ParsingInitForAutoVars;
 
   /// \brief Look for a locally scoped extern "C" declaration by the given name.
-  NamedDecl *findLocallyScopedExternCDecl(DeclarationName Name);
+  ///
+  /// If AllResults is not null, any results we find will be appended to it.
+  NamedDecl *findLocallyScopedExternCDecl(
+      DeclarationName Name, SmallVectorImpl<NamedDecl *> *AllResults = nullptr);
 
   typedef LazyVector<VarDecl *, ExternalSemaSource,
                      &ExternalSemaSource::ReadTentativeDefinitions, 2, 2>
     TentativeDefinitionsType;
 
   /// \brief All the tentative definitions encountered in the TU.
   TentativeDefinitionsType TentativeDefinitions;
 

lib/Lex/PPMacroExpansion.cpp

return llvm::StringSwitch<bool>(Feature)
       .Case("attribute_diagnose_if_objc", true)
       .Case("blocks", LangOpts.Blocks)
       .Case("c_thread_safety_attributes", true)
       .Case("cxx_exceptions", LangOpts.CXXExceptions)
       .Case("cxx_rtti", LangOpts.RTTI && LangOpts.RTTIData)
       .Case("enumerator_attributes", true)
       .Case("nullability", true)
       .Case("nullability_on_arrays", true)
+      .Case("overloadable_unmarked", true)
       .Case("memory_sanitizer", LangOpts.Sanitize.has(SanitizerKind::Memory))
       .Case("thread_sanitizer", LangOpts.Sanitize.has(SanitizerKind::Thread))
       .Case("dataflow_sanitizer", LangOpts.Sanitize.has(SanitizerKind::DataFlow))
       .Case("efficiency_sanitizer",
             LangOpts.Sanitize.hasOneOf(SanitizerKind::Efficiency))
       // Objective-C features
       .Case("objc_arr", LangOpts.ObjCAutoRefCount) // FIXME: REMOVE?
       .Case("objc_arc", LangOpts.ObjCAutoRefCount)

lib/Sema/SemaDecl.cpp

 ///
 /// This routine determines whether overloading is possible, not
 /// whether some new function is actually an overload. It will return
 /// true in C++ (where we can always provide overloads) or, as an
 /// extension, in C when the previous function is already an
 /// overloaded function declaration or has the "overloadable"
 /// attribute.
 static bool AllowOverloadingOfFunction(LookupResult &Previous,
-                                       ASTContext &Context) {
+                                       ASTContext &Context,
+                                       const FunctionDecl *New) {
   if (Context.getLangOpts().CPlusPlus)
     return true;
 
   if (Previous.getResultKind() == LookupResult::FoundOverloaded)
     return true;
 
-  return (Previous.getResultKind() == LookupResult::Found
-          && Previous.getFoundDecl()->hasAttr<OverloadableAttr>());
+  return Previous.getResultKind() == LookupResult::Found &&
+         (Previous.getFoundDecl()->hasAttr<OverloadableAttr>() ||
+          New->hasAttr<OverloadableAttr>());
 }
 
 /// Add this decl to the scope shadowed decl chains.
 void Sema::PushOnScopeChains(NamedDecl *D, Scope *S, bool AddToContext) {
   // Move up the scope chain until we find the nearest enclosing
   // non-transparent context. The declaration will be introduced into this
   // scope.
   while (S->getEntity() && S->getEntity()->isTransparentContext())

 /// merging decls or emitting diagnostics as appropriate.
 ///
 /// In C++, New and Old must be declarations that are not
 /// overloaded. Use IsOverload to determine whether New and Old are
 /// overloaded, and to select the Old declaration that New should be
 /// merged with.
 ///
 /// Returns true if there was an error, false otherwise.
-bool Sema::MergeFunctionDecl(FunctionDecl *New, NamedDecl *&OldD,
-                             Scope *S, bool MergeTypeWithOld) {
+bool Sema::MergeFunctionDecl(FunctionDecl *New, NamedDecl *&OldD, Scope *S,
+                             bool MergeTypeWithOld) {
   // Verify the old decl was also a function.
   FunctionDecl *Old = OldD->getAsFunction();
   if (!Old) {
     if (UsingShadowDecl *Shadow = dyn_cast<UsingShadowDecl>(OldD)) {
       if (New->getFriendObjectKind()) {
         Diag(New->getLocation(), diag::err_using_decl_friend);
         Diag(Shadow->getTargetDecl()->getLocation(),
              diag::note_using_decl_target);
bool Sema::MergeFunctionDecl(FunctionDecl *New, NamedDecl *&OldD, Scope *S,
   if (New->hasAttr<InternalLinkageAttr>() &&
       !Old->hasAttr<InternalLinkageAttr>()) {
     Diag(New->getLocation(), diag::err_internal_linkage_redeclaration)
         << New->getDeclName();
     notePreviousDefinition(Old, New->getLocation());
     New->dropAttr<InternalLinkageAttr>();
   }
 
+  if (!getLangOpts().CPlusPlus) {
+    const NamedDecl *MostRecentOld = Old->getMostRecentDecl();
+    bool OldOvl = MostRecentOld->hasAttr<OverloadableAttr>();
+    if (OldOvl != New->hasAttr<OverloadableAttr>() &&
+        !MostRecentOld->isImplicit()) {
+      Diag(New->getLocation(), diag::err_attribute_overloadable_mismatch)
+        << New << OldOvl;
+
+      // Try our best to find a decl that actually has (or lacks) the
+      // overloadable attribute for the note. In most cases (e.g. programs with
+      // only one broken declaration/definition), this won't matter.
+      //
+      // FIXME: We could do this if we juggled some extra state in
+      // OverloadableAttr, rather than just removing it.
+      const Decl *DiagOld = MostRecentOld;
+      if (OldOvl) {
+        auto OldIter =
+            llvm::find_if(MostRecentOld->redecls(), [](const Decl *D) {
+              const auto *A = D->getAttr<OverloadableAttr>();
+              return A && !A->isImplicit();
+            });
+        // If we've implicitly added *all* of the overloadable attrs to this
+        // chain, emitting a "previous redecl" note is pointless.
+        DiagOld = OldIter == MostRecentOld->redecls_end() ? nullptr : *OldIter;
+      }
+
+      if (DiagOld)
+        Diag(DiagOld->getLocation(),
+             diag::note_attribute_overloadable_prev_overload)
+          << OldOvl;
+
+      if (OldOvl)
+        New->addAttr(OverloadableAttr::CreateImplicit(Context));
+      else
+        New->dropAttr<OverloadableAttr>();
+    }
+  }
+
   // If a function is first declared with a calling convention, but is later
   // declared or defined without one, all following decls assume the calling
   // convention of the first.
   //
   // It's OK if a function is first declared without a calling convention,
   // but is later declared or defined with the default calling convention.
   //
   // To test if either decl has an explicit calling convention, we look for
if (!getLangOpts().CPlusPlus &&
       ND->getLexicalDeclContext()->getRedeclContext()->isTranslationUnit())
     // Don't need to track declarations in the TU in C.
     return;
 
   // Note that we have a locally-scoped external with this name.
   Context.getExternCContextDecl()->makeDeclVisibleInContext(ND);
 }
 
-NamedDecl *Sema::findLocallyScopedExternCDecl(DeclarationName Name) {
-  // FIXME: We can have multiple results via __attribute__((overloadable)).
+NamedDecl *
+Sema::findLocallyScopedExternCDecl(DeclarationName Name,
+                                   SmallVectorImpl<NamedDecl *> *AllResults) {
   auto Result = Context.getExternCContextDecl()->lookup(Name);
+  if (AllResults)
+    AllResults->append(Result.begin(), Result.end());
   return Result.empty() ? nullptr : *Result.begin();
 }
 
 /// \brief Diagnose function specifiers on a declaration of an identifier that
 /// does not identify a function.
 void Sema::DiagnoseFunctionSpecifiers(const DeclSpec &DS) {
   // FIXME: We should probably indicate the identifier in question to avoid
   // confusion for constructs like "virtual int a(), b;"

 template<typename T>
 static bool checkForConflictWithNonVisibleExternC(Sema &S, const T *ND,
                                                   LookupResult &Previous) {
   if (!S.getLangOpts().CPlusPlus) {
     // In C, when declaring a global variable, look for a corresponding 'extern'
     // variable declared in function scope. We don't need this in C++, because
     // we find local extern decls in the surrounding file-scope DeclContext.
     if (ND->getDeclContext()->getRedeclContext()->isTranslationUnit()) {
-      if (NamedDecl *Prev = S.findLocallyScopedExternCDecl(ND->getDeclName())) {
+      SmallVector<NamedDecl *, 4> Prevs;
+      if (S.findLocallyScopedExternCDecl(ND->getDeclName(), &Prevs)) {
         Previous.clear();
-        Previous.addDecl(Prev);
+        for (NamedDecl *ND : Prevs)
+          Previous.addDecl(ND);
+        Previous.resolveKind();
         return true;
       }
     }
     return false;
   }
 
   // A declaration in the translation unit can conflict with an extern "C"
   // declaration.
assert(!NewFD->getReturnType()->isVariablyModifiedType() &&
          "Variably modified return types are not handled here");
 
   // Determine whether the type of this function should be merged with
   // a previous visible declaration. This never happens for functions in C++,
   // and always happens in C if the previous declaration was visible.
   bool MergeTypeWithPrevious = !getLangOpts().CPlusPlus &&
                                !Previous.isShadowed();
 
+  bool MayNeedOverloadableChecks = false;
   bool Redeclaration = false;
   NamedDecl *OldDecl = nullptr;
 
   // Merge or overload the declaration with an existing declaration of
   // the same name, if appropriate.
   if (!Previous.empty()) {
     // Determine whether NewFD is an overload of PrevDecl or
     // a declaration that requires merging. If it's an overload,
     // there's no more work to do here; we'll just add the new
     // function to the scope.
-    if (!AllowOverloadingOfFunction(Previous, Context)) {
+    if (!AllowOverloadingOfFunction(Previous, Context, NewFD)) {
       NamedDecl *Candidate = Previous.getRepresentativeDecl();
       if (shouldLinkPossiblyHiddenDecl(Candidate, NewFD)) {
         Redeclaration = true;
         OldDecl = Candidate;
       }
     } else {
+      MayNeedOverloadableChecks = true;
       switch (CheckOverload(S, NewFD, Previous, OldDecl,
-                            /*NewIsUsingDecl*/ false)) {
-      case Ovl_Match:
-        Redeclaration = true;
-        break;
-
-      case Ovl_NonFunction:
+                            /*NewIsUsingDecl=*/false)) {
+      case Sema::Ovl_Match:
+      case Sema::Ovl_NonFunction:
         Redeclaration = true;
         break;
 
-      case Ovl_Overload:
+      case Sema::Ovl_Overload:
         Redeclaration = false;
         break;
       }
-
-      if (!getLangOpts().CPlusPlus && !NewFD->hasAttr<OverloadableAttr>()) {
-        // If a function name is overloadable in C, then every function
-        // with that name must be marked "overloadable".
-        Diag(NewFD->getLocation(), diag::err_attribute_overloadable_missing)
-          << Redeclaration << NewFD;
-        NamedDecl *OverloadedDecl =
-            Redeclaration ? OldDecl : Previous.getRepresentativeDecl();
-        Diag(OverloadedDecl->getLocation(),
-             diag::note_attribute_overloadable_prev_overload);
-        NewFD->addAttr(OverloadableAttr::CreateImplicit(Context));
-      }
     }
   }
 
   // Check for a previous extern "C" declaration with this name.
   if (!Redeclaration &&
-      checkForConflictWithNonVisibleExternC(*this, NewFD, Previous)) {
-    if (!Previous.empty()) {
-      // This is an extern "C" declaration with the same name as a previous
-      // declaration, and thus redeclares that entity...
-      Redeclaration = true;
-      OldDecl = Previous.getFoundDecl();
-      MergeTypeWithPrevious = false;
+      checkForConflictWithNonVisibleExternC(*this, NewFD, Previous) &&
+      !Previous.empty()) {
+    // declaration, and thus redeclares that entity, unless the overloadable
+    // attribute is present.
+    Redeclaration = true;
 
-      // ... except in the presence of __attribute__((overloadable)).
-      if (OldDecl->hasAttr<OverloadableAttr>()) {
-        if (!getLangOpts().CPlusPlus && !NewFD->hasAttr<OverloadableAttr>()) {
-          Diag(NewFD->getLocation(), diag::err_attribute_overloadable_missing)
-            << Redeclaration << NewFD;
-          Diag(Previous.getFoundDecl()->getLocation(),
-               diag::note_attribute_overloadable_prev_overload);
-          NewFD->addAttr(OverloadableAttr::CreateImplicit(Context));
-        }
-        if (IsOverload(NewFD, cast<FunctionDecl>(OldDecl), false)) {
-          Redeclaration = false;
-          OldDecl = nullptr;
-        }
-      }
+    switch (CheckOverload(S, NewFD, Previous, OldDecl,
+                          /*NewIsUsingDecl=*/false)) {
+    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();
+      break;
+
+    case Ovl_NonFunction:
+      break;
+
+    case Ovl_Overload: {
+      // Otherwise, we should be in Ovl_NonFunction. This matters because we
+      // don't want to hide diags for finding NonFunctions if we find an
+      // overloadable function.
+      assert(llvm::all_of(
+          Previous, [](const NamedDecl *ND) { return isa<FunctionDecl>(ND); }));
+
+      bool FoundOverloadableAttr = false;
+      if (!getLangOpts().CPlusPlus) {
+        MayNeedOverloadableChecks = true;
+        FoundOverloadableAttr =
+            NewFD->hasAttr<OverloadableAttr>() ||
+            llvm::any_of(Previous, [](const NamedDecl *ND) {
+              return ND->getMostRecentDecl()->hasAttr<OverloadableAttr>();
+            });
+
+      Redeclaration = !FoundOverloadableAttr;
+      // If we didn't find an exact match and none of the overloads have the
+      // overloadable attribute, we can't do much more than picking an arbitrary
+      // decl for diags.
+      if (Redeclaration)
+        OldDecl = Previous.getRepresentativeDecl();
+      break;
     }
   }
 
   // C++11 [dcl.constexpr]p8:
   //   A constexpr specifier for a non-static member function that is not
   //   a constructor declares that member function to be const.
   //
   // This needs to be delayed until we know whether this is an out-of-line
if (Redeclaration) {
     } else {
       if (shouldLinkDependentDeclWithPrevious(NewFD, OldDecl)) {
         // This needs to happen first so that 'inline' propagates.
         NewFD->setPreviousDeclaration(cast<FunctionDecl>(OldDecl));
         if (isa<CXXMethodDecl>(NewFD))
           NewFD->setAccess(OldDecl->getAccess());
       }
     }
+  } else if (!getLangOpts().CPlusPlus && MayNeedOverloadableChecks &&
+             !NewFD->getAttr<OverloadableAttr>()) {
+    assert((Previous.empty() ||
+            llvm::any_of(
+                Previous,
+                [](const NamedDecl *ND) {
+                  return ND->getMostRecentDecl()->hasAttr<OverloadableAttr>();
+                })) &&
+           "Non-redecls shouldn't happen without overloadable present");
+
+    auto OtherUnmarkedIter = llvm::find_if(Previous, [&](const NamedDecl *ND) {
+      const auto *FD = dyn_cast<FunctionDecl>(ND);
+      return FD && !FD->getMostRecentDecl()->hasAttr<OverloadableAttr>();
+    });
+
+    if (OtherUnmarkedIter != Previous.end()) {
+      const auto *OtherUnmarked =
+          cast<FunctionDecl>((*OtherUnmarkedIter)->getMostRecentDecl());
+      Diag(NewFD->getLocation(),
+           diag::err_attribute_overloadable_multiple_unmarked_overloads);
+      Diag(OtherUnmarked->getLocation(),
+           diag::note_attribute_overloadable_prev_overload)
+          << false;
+
+      NewFD->addAttr(OverloadableAttr::CreateImplicit(Context));
+    }
   }
 
   // Semantic checking for this function declaration (in isolation).
 
   if (getLangOpts().CPlusPlus) {
     // C++-specific checks.
     if (CXXConstructorDecl *Constructor = dyn_cast<CXXConstructorDecl>(NewFD)) {
       CheckConstructor(Constructor);

test/CodeGen/mangle-ms.c

 // RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-pc-win32 | FileCheck %s
 
 // CHECK: define void @"\01?f@@$$J0YAXP6AX@Z@Z"
 __attribute__((overloadable)) void f(void (*x)()) {}
+
+// CHECK: define void @f
+void f(void (*x)(int)) {}
+
+// CHECK: define void @g
+void g(void (*x)(int)) {}
+
+// CHECK: define void @"\01?g@@$$J0YAXP6AX@Z@Z"
+__attribute__((overloadable)) void g(void (*x)()) {}

test/CodeGen/mangle.c

 // RUN: %clang_cc1 -triple i386-pc-linux-gnu -emit-llvm -o - %s | FileCheck %s
 
 // CHECK: @foo
 
 // Make sure we mangle overloadable, even in C system headers.
 # 1 "somesystemheader.h" 1 3 4
 // CHECK: @_Z2f0i
 void __attribute__((__overloadable__)) f0(int a) {}
 // CHECK: @_Z2f0l
 void __attribute__((__overloadable__)) f0(long b) {}
 
+// Unless it's unmarked.
+// CHECK: @f0
+void f0(float b) {}
+
 // CHECK: @bar
 
 // These should get merged.
 void foo() __asm__("bar");
 void foo2() __asm__("bar");
 
 int nux __asm__("foo");
 extern float nux2 __asm__("foo");

test/CodeGenCXX/mangle-ms.cpp

 void fn_tmpl() {}
 
 template void fn_tmpl<extern_c_func>();
 // CHECK-DAG: @"\01??$fn_tmpl@$1?extern_c_func@@YAXXZ@@YAXXZ"
 
 extern "C" void __attribute__((overloadable)) overloaded_fn() {}
 // CHECK-DAG: @"\01?overloaded_fn@@$$J0YAXXZ"
 
+extern "C" void overloaded_fn2() {}
+// CHECK-DAG: @overloaded_fn2
+//
+extern "C" void __attribute__((overloadable)) overloaded_fn3();
+extern "C" void overloaded_fn3() {}
+// CHECK-DAG: @overloaded_fn3
+
 namespace UnnamedType {
 struct S {
   typedef struct {} *T1[1];
   typedef struct {} T2;
   typedef struct {} *T3, T4;
   using T5 = struct {};
   using T6 = struct {} *;
 };

test/PCH/attrs.c

 // Test this without pch.
 // RUN: %clang_cc1 -include %s -fsyntax-only -verify %s
 
 // Test with pch.
 // RUN: %clang_cc1 -emit-pch -o %t %s
 // RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify %s 
 
 #ifndef HEADER
 #define HEADER
 
 int f(int) __attribute__((visibility("default"), overloadable));
 int g(int) __attribute__((abi_tag("foo", "bar", "baz"), no_sanitize("address", "memory")));
 
 #else
 
+float f(float);
 double f(double); // expected-error{{overloadable}}
-                  // expected-note@11{{previous overload}}
+                  // expected-note@-2{{previous unmarked overload}}
 void h() { g(0); }
 
 #endif

test/Sema/overloadable.c

 // RUN: %clang_cc1 -fsyntax-only -verify %s -Wincompatible-pointer-types
 
 int var __attribute__((overloadable)); // expected-error{{'overloadable' attribute only applies to functions}}
 void params(void) __attribute__((overloadable(12))); // expected-error {{'overloadable' attribute takes no arguments}}
 
-int *f(int) __attribute__((overloadable)); // expected-note 2{{previous overload of function is here}}
-float *f(float); // expected-error{{overloaded function 'f' must have the 'overloadable' attribute}}
+int *f(int) __attribute__((overloadable)); // expected-note{{previous overload of function is here}}
+float *f(float);
 int *f(int); // expected-error{{redeclaration of 'f' must have the 'overloadable' attribute}} \
              // expected-note{{previous declaration is here}}
 double *f(double) __attribute__((overloadable)); // okay, new
 
+// Ensure we don't complain about overloadable on implicitly declared functions.
+int isdigit(int) __attribute__((overloadable));
+
 void test_f(int iv, float fv, double dv) {
   int *ip = f(iv);
   float *fp = f(fv);
   double *dp = f(dv);
 }
 
 int *accept_funcptr(int (*)()) __attribute__((overloadable)); //         \
   // expected-note{{candidate function}}

 typedef int f1_type();
 f1_type __attribute__((overloadable)) f1; // expected-error{{'overloadable' function 'f1' must have a prototype}}
 
 void test() {
   f0();
   f1();
 }
 
-void before_local_1(int) __attribute__((overloadable)); // expected-note {{here}}
+void before_local_1(int) __attribute__((overloadable));
 void before_local_2(int); // expected-note {{here}}
 void before_local_3(int) __attribute__((overloadable));
 void local() {
-  void before_local_1(char); // expected-error {{must have the 'overloadable' attribute}}
-  void before_local_2(char) __attribute__((overloadable)); // expected-error {{conflicting types}}
+  void before_local_1(char);
+  void before_local_2(char); // expected-error {{conflicting types}}
   void before_local_3(char) __attribute__((overloadable));
-  void after_local_1(char); // expected-note {{here}}
-  void after_local_2(char) __attribute__((overloadable)); // expected-note {{here}}
+  void after_local_1(char);
+  void after_local_2(char) __attribute__((overloadable));
   void after_local_3(char) __attribute__((overloadable));
 }
-void after_local_1(int) __attribute__((overloadable)); // expected-error {{conflicting types}}
-void after_local_2(int); // expected-error {{must have the 'overloadable' attribute}}
+void after_local_1(int) __attribute__((overloadable));
+void after_local_2(int);
 void after_local_3(int) __attribute__((overloadable));
 
 // Make sure we allow C-specific conversions in C.
 void conversions() {
   void foo(char *c) __attribute__((overloadable));
   void foo(char *c) __attribute__((overloadable, enable_if(c, "nope.jpg")));
 
   void *ptr;
   foo(ptr);
 
   void multi_type(unsigned char *c) __attribute__((overloadable));
   void multi_type(signed char *c) __attribute__((overloadable));
   unsigned char *c;
   multi_type(c);
 }
 
 // Ensure that we allow C-specific type conversions in C
 void fn_type_conversions() {
   void foo(void *c) __attribute__((overloadable));
   void foo(char *c) __attribute__((overloadable));
   void (*ptr1)(void *) = &foo;
   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}}
 
   void (*specific1)(int *) = (void (*)(void *))&foo; // expected-warning{{incompatible function pointer types initializing 'void (*)(int *)' with an expression of type 'void (*)(void *)'}}
   void *specific2 = (void (*)(void *))&foo;
 
   void disabled(void *c) __attribute__((overloadable, enable_if(0, "")));
   void disabled(int *c) __attribute__((overloadable, enable_if(c, "")));
   void disabled(char *c) __attribute__((overloadable, enable_if(1, "The function name lies.")));
   // To be clear, these should all point to the last overload of 'disabled'
   void (*dptr1)(char *c) = &disabled;
-  void (*dptr2)(void *c) = &disabled; // expected-warning{{incompatible pointer types initializing 'void (*)(void *)' with an expression of type '<overloaded function type>'}} expected-note@115{{candidate function made ineligible by enable_if}} expected-note@116{{candidate function made ineligible by enable_if}} expected-note@117{{candidate function has type mismatch at 1st parameter (expected 'void *' but has 'char *')}}
-  void (*dptr3)(int *c) = &disabled; // expected-warning{{incompatible pointer types initializing 'void (*)(int *)' with an expression of type '<overloaded function type>'}} expected-note@115{{candidate function made ineligible by enable_if}} expected-note@116{{candidate function made ineligible by enable_if}} expected-note@117{{candidate function has type mismatch at 1st parameter (expected 'int *' but has 'char *')}}
+  void (*dptr2)(void *c) = &disabled; // expected-warning{{incompatible pointer types initializing 'void (*)(void *)' with an expression of type '<overloaded function type>'}} expected-note@-5{{candidate function made ineligible by enable_if}} expected-note@-4{{candidate function made ineligible by enable_if}} expected-note@-3{{candidate function has type mismatch at 1st parameter (expected 'void *' but has 'char *')}}
+  void (*dptr3)(int *c) = &disabled; // expected-warning{{incompatible pointer types initializing 'void (*)(int *)' with an expression of type '<overloaded function type>'}} expected-note@-6{{candidate function made ineligible by enable_if}} expected-note@-5{{candidate function made ineligible by enable_if}} expected-note@-4{{candidate function has type mismatch at 1st parameter (expected 'int *' but has 'char *')}}
 
   void *specific_disabled = &disabled;
 }
 
 void incompatible_pointer_type_conversions() {
   char charbuf[1];
   unsigned char ucharbuf[1];
   int intbuf[1];
 
   void foo(char *c) __attribute__((overloadable));
   void foo(short *c) __attribute__((overloadable));
   foo(charbuf);
-  foo(ucharbuf); // expected-error{{call to 'foo' is ambiguous}} expected-note@131{{candidate function}} expected-note@132{{candidate function}}
-  foo(intbuf); // expected-error{{call to 'foo' is ambiguous}} expected-note@131{{candidate function}} expected-note@132{{candidate function}}
+  foo(ucharbuf); // expected-error{{call to 'foo' is ambiguous}} expected-note@-3{{candidate function}} expected-note@-2{{candidate function}}
+  foo(intbuf); // expected-error{{call to 'foo' is ambiguous}} expected-note@-4{{candidate function}} expected-note@-3{{candidate function}}
 
   void bar(unsigned char *c) __attribute__((overloadable));
   void bar(signed char *c) __attribute__((overloadable));
-  bar(charbuf); // expected-error{{call to 'bar' is ambiguous}} expected-note@137{{candidate function}} expected-note@138{{candidate function}}
+  bar(charbuf); // expected-error{{call to 'bar' is ambiguous}} expected-note@-2{{candidate function}} expected-note@-1{{candidate function}}
   bar(ucharbuf);
-  bar(intbuf); // expected-error{{call to 'bar' is ambiguous}} expected-note@137{{candidate function}} expected-note@138{{candidate function}}
+  bar(intbuf); // expected-error{{call to 'bar' is ambiguous}} expected-note@-4{{candidate function}} expected-note@-3{{candidate function}}
 }
 
 void dropping_qualifiers_is_incompatible() {
   const char ccharbuf[1];
   volatile char vcharbuf[1];
 
   void foo(char *c) __attribute__((overloadable));
   void foo(const volatile unsigned char *c) __attribute__((overloadable));
 
-  foo(ccharbuf); // expected-error{{call to 'foo' is ambiguous}} expected-note@148{{candidate function}} expected-note@149{{candidate function}}
-  foo(vcharbuf); // expected-error{{call to 'foo' is ambiguous}} expected-note@148{{candidate function}} expected-note@149{{candidate function}}
+  foo(ccharbuf); // expected-error{{call to 'foo' is ambiguous}} expected-note@-3{{candidate function}} expected-note@-2{{candidate function}}
+  foo(vcharbuf); // expected-error{{call to 'foo' is ambiguous}} expected-note@-4{{candidate function}} expected-note@-3{{candidate function}}
+}
+
+void overloadable_with_global() {
+  void wg_foo(void) __attribute__((overloadable)); // expected-note{{previous}}
+  void wg_foo(int) __attribute__((overloadable));
+}
+
+int wg_foo; // expected-error{{redefinition of 'wg_foo' as different kind of symbol}}
+
+#if !__has_feature(overloadable_unmarked)
+#error "We should have unmarked overload support"
+#endif
+
+void unmarked_overloadable() {
+  void to_foo0(int);
+  void to_foo0(double) __attribute__((overloadable)); // expected-note{{previous overload}}
+  void to_foo0(int);
+  void to_foo0(double); // expected-error{{must have the 'overloadable' attribute}}
+  void to_foo0(int);
+
+  void to_foo1(int) __attribute__((overloadable)); // expected-note 2{{previous overload}}
+  void to_foo1(double);
+  void to_foo1(int); // expected-error{{must have the 'overloadable' attribute}}
+  void to_foo1(double);
+  void to_foo1(int); // expected-error{{must have the 'overloadable' attribute}}
+
+  void to_foo2(int); // expected-note{{previous unmarked overload}}
+  void to_foo2(double) __attribute__((overloadable)); // expected-note 2{{previous overload}}
+  void to_foo2(int) __attribute__((overloadable)); // expected-error {{must not have the 'overloadable' attribute}}
+  void to_foo2(double); // expected-error{{must have the 'overloadable' attribute}}
+  void to_foo2(int);
+  void to_foo2(double); // expected-error{{must have the 'overloadable' attribute}}
+  void to_foo2(int);
+
+  void to_foo3(int);
+  void to_foo3(double) __attribute__((overloadable)); // expected-note{{previous overload}}
+  void to_foo3(int);
+  void to_foo3(double); // expected-error{{must have the 'overloadable' attribute}}
+
+  void to_foo4(int) __attribute__((overloadable)); // expected-note{{previous overload}}
+  void to_foo4(int); // expected-error{{must have the 'overloadable' attribute}}
+  void to_foo4(double) __attribute__((overloadable));
+
+  void to_foo5(int);
+  void to_foo5(int); // expected-note 3{{previous unmarked overload}}
+  void to_foo5(float) __attribute__((overloadable));
+  void to_foo5(double); // expected-error{{at most one 'overloadable' function for a given name may lack the 'overloadable' attribute}}
+  void to_foo5(float) __attribute__((overloadable));
+  void to_foo5(short); // expected-error{{at most one 'overloadable' function for a given name may lack the 'overloadable' attribute}}
+  void to_foo5(long); // expected-error{{at most one 'overloadable' function for a given name may lack the 'overloadable' attribute}}
+  void to_foo5(double) __attribute__((overloadable));
+
+  void to_foo6(int) __attribute__((enable_if(1, ""), overloadable)); // expected-note{{previous overload}}
+  void to_foo6(int) __attribute__((enable_if(1, ""))); // expected-error{{must have the 'overloadable' attribute}}
+  void to_foo6(int) __attribute__((enable_if(1, ""), overloadable));
+
+  void to_foo7(int) __attribute__((enable_if(1, ""))); // expected-note{{previous unmarked overload}}
+  void to_foo7(int) __attribute__((enable_if(1, ""), overloadable)); // expected-error{{must not have the 'overloadable' attribute}}
+  void to_foo7(int) __attribute__((enable_if(1, "")));
+
+  void to_foo8(char *__attribute__((pass_object_size(0))))
+    __attribute__((enable_if(1, "")));
+  void to_foo8(char *__attribute__((pass_object_size(0))))
+    __attribute__((overloadable));
+
+  void to_foo9(int); // expected-note{{previous unmarked overload}}
+  // FIXME: It would be nice if we did better with the "previous unmarked
+  // overload" diag.
+  void to_foo9(int) __attribute__((overloadable)); // expected-error{{must not have the 'overloadable' attribute}} expected-note{{previous declaration}} expected-note{{previous unmarked overload}}
+  void to_foo9(float); // expected-error{{conflicting types for 'to_foo9'}}
+  void to_foo9(float) __attribute__((overloadable));
+  void to_foo9(double); // expected-error{{at most one 'overloadable' function for a given name may lack the 'overloadable' attribute}}
+  void to_foo9(double) __attribute__((overloadable));
+
+  void to_foo10(int) __attribute__((overloadable));
+  void to_foo10(double); // expected-note{{previous unmarked overload}}
+  // no "note: previous redecl" if no previous decl has `overloadable`
+  // spelled out
+  void to_foo10(float); // expected-error{{at most one 'overloadable' function for a given name may lack the 'overloadable' attribute}}
+  void to_foo10(float); // expected-error{{must have the 'overloadable' attribute}}
+  void to_foo10(float); // expected-error{{must have the 'overloadable' attribute}}
 }
 
 // Bug: we used to treat `__typeof__(foo)` as though it was `__typeof__(&foo)`
 // if `foo` was overloaded with only one function that could have its address
 // taken.
 void typeof_function_is_not_a_pointer() {
   void not_a_pointer(void *) __attribute__((overloadable));
   void not_a_pointer(char *__attribute__((pass_object_size(1))))
     __attribute__((overloadable));
 
   __typeof__(not_a_pointer) *fn;
 
   void take_fn(void (*)(void *));
   // if take_fn is passed a void (**)(void *), we'll get a warning.
   take_fn(fn);
 }