[Sema] Allow unmarked overloadable functions.

This patch extends the `overloadable` attribute to allow for one
function with a given name to not be marked with the `overloadable`
attribute. The overload without the `overloadable` attribute will not
have its name mangled.

So, the following code is now legal:

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

In addition, this patch fixes a bug where we'd accept code with
`__attribute__((overloadable))` inconsistently applied. In other words,
we used to accept:

  void foo(void);
  void foo(void) __attribute__((overloadable));

But we will do this no longer, since it defeats the original purpose of
requiring `__attribute__((overloadable))` on all redeclarations of a
function.

This breakage seems to not be an issue in practice, since the only code
I could find that had this pattern often looked like:

  void foo(void);
  void foo(void) __attribute__((overloadable)) __asm__("foo");
  void foo(int) __attribute__((overloadable));

...Which can now be simplified by simply removing the asm label and
overloadable attribute from the redeclaration of `void foo(void);`

Differential Revision: https://reviews.llvm.org/D32332


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@306467 91177308-0d34-0410-b5e6-96231b3b80d8

9 files changed, 224 insertions, 44 deletions

diff --git a/include/clang/Basic/AttrDocs.td b/include/clang/Basic/AttrDocs.td
index 65dd7445ba..2987f07d8b 100644
--- a/include/clang/Basic/AttrDocs.td
+++ b/include/clang/Basic/AttrDocs.td
@@ -605,20 +605,27 @@ semantics:
   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.,
+declarations and definitions.  If a function is marked with the ``overloadable``
+attribute, then all declarations and definitions of functions with that name,
+except for at most one (see the note below about unmarked overloads), must have
+the ``overloadable`` attribute.  In addition, redeclarations of a function with
+the ``overloadable`` attribute must have the ``overloadable`` attribute, and
+redeclarations of a function without the ``overloadable`` attribute must *not*
+have 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 f(int); // error: redeclaration of "f" must have the "overloadable" attribute
 
   int g(int) __attribute__((overloadable));
   int g(int) { } // error: redeclaration of "g" must also have the "overloadable" attribute
 
+  int h(int);
+  int h(int) __attribute__((overloadable)); // error: declaration of "h" must not
+                                            // have the "overloadable" attribute
+
 Functions marked ``overloadable`` must have prototypes.  Therefore, the
 following code is ill-formed:
 
@@ -651,7 +658,28 @@ caveats to this use of name mangling:
   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_extension(overloadable_unmarked)``.
+
+Query for this attribute with ``__has_attribute(overloadable)``.
   }];
 }
 
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index cba9b25121..6224f13ce7 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3294,13 +3294,15 @@ 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 overload 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">,
diff --git a/lib/Lex/PPMacroExpansion.cpp b/lib/Lex/PPMacroExpansion.cpp
index a6bfc32e22..8af9a50cc2 100644
--- a/lib/Lex/PPMacroExpansion.cpp
+++ b/lib/Lex/PPMacroExpansion.cpp
@@ -1315,6 +1315,8 @@ static bool HasExtension(const Preprocessor &PP, StringRef Extension) {
            .Case("cxx_binary_literals", true)
            .Case("cxx_init_captures", LangOpts.CPlusPlus11)
            .Case("cxx_variable_templates", LangOpts.CPlusPlus)
+           // Miscellaneous language extensions
+           .Case("overloadable_unmarked", true)
            .Default(false);
 }
 
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 71f4d069b8..fd172def2d 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -1327,15 +1327,17 @@ void Sema::ActOnExitFunctionContext() {
 /// 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.
@@ -2933,6 +2935,41 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, NamedDecl *&OldD,
     New->dropAttr<InternalLinkageAttr>();
   }
 
+  if (!getLangOpts().CPlusPlus) {
+    bool OldOvl = Old->hasAttr<OverloadableAttr>();
+    if (OldOvl != New->hasAttr<OverloadableAttr>() && !Old->isImplicit()) {
+      Diag(New->getLocation(), diag::err_attribute_overloadable_mismatch)
+        << New << OldOvl;
+
+      // Try our best to find a decl that actually has 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 = Old;
+      if (OldOvl) {
+        auto OldIter = llvm::find_if(Old->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 == Old->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.
@@ -9179,6 +9216,7 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD,
 
   bool Redeclaration = false;
   NamedDecl *OldDecl = nullptr;
+  bool MayNeedOverloadableChecks = false;
 
   // Merge or overload the declaration with an existing declaration of
   // the same name, if appropriate.
@@ -9187,13 +9225,14 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD,
     // 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:
@@ -9208,18 +9247,6 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD,
         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));
-      }
     }
   }
 
@@ -9234,15 +9261,10 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD,
       MergeTypeWithPrevious = false;
 
       // ... 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 (OldDecl->hasAttr<OverloadableAttr>() ||
+          NewFD->hasAttr<OverloadableAttr>()) {
         if (IsOverload(NewFD, cast<FunctionDecl>(OldDecl), false)) {
+          MayNeedOverloadableChecks = true;
           Redeclaration = false;
           OldDecl = nullptr;
         }
@@ -9337,6 +9359,29 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *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->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->hasAttr<OverloadableAttr>();
+    });
+
+    if (OtherUnmarkedIter != Previous.end()) {
+      Diag(NewFD->getLocation(),
+           diag::err_attribute_overloadable_multiple_unmarked_overloads);
+      Diag((*OtherUnmarkedIter)->getLocation(),
+           diag::note_attribute_overloadable_prev_overload)
+          << false;
+
+      NewFD->addAttr(OverloadableAttr::CreateImplicit(Context));
+    }
   }
 
   // Semantic checking for this function declaration (in isolation).
diff --git a/test/CodeGen/mangle-ms.c b/test/CodeGen/mangle-ms.c
index 0ad43d5e06..042c72e6d7 100644
--- a/test/CodeGen/mangle-ms.c
+++ b/test/CodeGen/mangle-ms.c
@@ -2,3 +2,12 @@
 
 // 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)()) {}
diff --git a/test/CodeGen/mangle.c b/test/CodeGen/mangle.c
index 46ef512f69..f94139bc5e 100644
--- a/test/CodeGen/mangle.c
+++ b/test/CodeGen/mangle.c
@@ -9,6 +9,10 @@ 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.
diff --git a/test/CodeGenCXX/mangle-ms.cpp b/test/CodeGenCXX/mangle-ms.cpp
index ee0f50e5e2..7e2f17f27a 100644
--- a/test/CodeGenCXX/mangle-ms.cpp
+++ b/test/CodeGenCXX/mangle-ms.cpp
@@ -399,6 +399,13 @@ template void fn_tmpl<extern_c_func>();
 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];
diff --git a/test/PCH/attrs.c b/test/PCH/attrs.c
index 3f34d4d009..3bf660cf28 100644
--- a/test/PCH/attrs.c
+++ b/test/PCH/attrs.c
@@ -13,8 +13,9 @@ int g(int) __attribute__((abi_tag("foo", "bar", "baz"), no_sanitize("address", "
 
 #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
diff --git a/test/Sema/overloadable.c b/test/Sema/overloadable.c
index be9b862f29..4bdec85f9c 100644
--- a/test/Sema/overloadable.c
+++ b/test/Sema/overloadable.c
@@ -3,12 +3,15 @@
 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);
@@ -71,19 +74,19 @@ void test() {
   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.
@@ -152,6 +155,85 @@ void dropping_qualifiers_is_incompatible() {
   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_extension(overloadable_unmarked)
+#error "We should have unmarked overload support"
+#endif
+
+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 overload for a given name may lack the 'overloadable' attribute}}
+void to_foo5(float) __attribute__((overloadable));
+void to_foo5(short); // expected-error{{at most one overload for a given name may lack the 'overloadable' attribute}}
+void to_foo5(long); // expected-error{{at most one overload 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 overload 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 overload 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.