aaron.ballman created this revision.
aaron.ballman added reviewers: erichkeane, cor3ntin, rsmith.
aaron.ballman requested review of this revision.
Herald added a project: clang.

When forming the function type from a declarator, we look for an overloadable 
attribute before issuing a diagnostic in C about a function signature 
containing only `...`. When the attribute is present, we allow such a 
declaration for compatibility with the overloading rules in C++. However, we 
were not looking for the attribute in all of the places it is legal to write it 
on a declarator and so we only accepted the signature in some forms and 
incorrectly rejected the signature in others.

We now check for the attribute preceding the declarator instead of only being 
applied to the declarator directly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119664

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/overloadable.c


Index: clang/test/Sema/overloadable.c
===================================================================
--- clang/test/Sema/overloadable.c
+++ clang/test/Sema/overloadable.c
@@ -248,3 +248,14 @@
   // if take_fn is passed a void (**)(void *), we'll get a warning.
   take_fn(fn);
 }
+
+// PR53805
+// We previously failed to consider the attribute being written before the
+// declaration when considering whether to allow a variadic signature with no
+// other parameters, and so we handled these cases differently.
+__attribute__((overloadable)) void can_overload_1(...); // ok, was previously 
rejected
+void can_overload_2(...) __attribute__((overloadable)); // ok
+[[clang::overloadable]] void can_overload_3(...);       // ok, was previously 
rejected
+void can_overload_4 [[clang::overloadable]] (...);      // ok
+void cannot_overload(...) [[clang::overloadable]];      // expected-error 
{{ISO C requires a named parameter before '...'}} \
+                                                        // expected-error 
{{'overloadable' attribute cannot be applied to types}}
Index: clang/lib/Sema/SemaType.cpp
===================================================================
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -5237,7 +5237,9 @@
         // function is marked with the "overloadable" attribute. Scan
         // for this attribute now.
         if (!FTI.NumParams && FTI.isVariadic && !LangOpts.CPlusPlus)
-          if (!D.getAttributes().hasAttribute(ParsedAttr::AT_Overloadable))
+          if (!D.getAttributes().hasAttribute(ParsedAttr::AT_Overloadable) &&
+              !D.getDeclSpec().getAttributes().hasAttribute(
+                  ParsedAttr::AT_Overloadable))
             S.Diag(FTI.getEllipsisLoc(), diag::err_ellipsis_first_param);
 
         if (FTI.NumParams && FTI.Params[0].Param == nullptr) {
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -77,6 +77,9 @@
 
 - Added support for parameter pack expansion in `clang::annotate`.
 
+- The ``overloadable`` attribute can now be written in all of the syntactic
+  locations a declaration attribute may appear. Fixes PR53805.
+
 Windows Support
 ---------------
 


Index: clang/test/Sema/overloadable.c
===================================================================
--- clang/test/Sema/overloadable.c
+++ clang/test/Sema/overloadable.c
@@ -248,3 +248,14 @@
   // if take_fn is passed a void (**)(void *), we'll get a warning.
   take_fn(fn);
 }
+
+// PR53805
+// We previously failed to consider the attribute being written before the
+// declaration when considering whether to allow a variadic signature with no
+// other parameters, and so we handled these cases differently.
+__attribute__((overloadable)) void can_overload_1(...); // ok, was previously rejected
+void can_overload_2(...) __attribute__((overloadable)); // ok
+[[clang::overloadable]] void can_overload_3(...);       // ok, was previously rejected
+void can_overload_4 [[clang::overloadable]] (...);      // ok
+void cannot_overload(...) [[clang::overloadable]];      // expected-error {{ISO C requires a named parameter before '...'}} \
+                                                        // expected-error {{'overloadable' attribute cannot be applied to types}}
Index: clang/lib/Sema/SemaType.cpp
===================================================================
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -5237,7 +5237,9 @@
         // function is marked with the "overloadable" attribute. Scan
         // for this attribute now.
         if (!FTI.NumParams && FTI.isVariadic && !LangOpts.CPlusPlus)
-          if (!D.getAttributes().hasAttribute(ParsedAttr::AT_Overloadable))
+          if (!D.getAttributes().hasAttribute(ParsedAttr::AT_Overloadable) &&
+              !D.getDeclSpec().getAttributes().hasAttribute(
+                  ParsedAttr::AT_Overloadable))
             S.Diag(FTI.getEllipsisLoc(), diag::err_ellipsis_first_param);
 
         if (FTI.NumParams && FTI.Params[0].Param == nullptr) {
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -77,6 +77,9 @@
 
 - Added support for parameter pack expansion in `clang::annotate`.
 
+- The ``overloadable`` attribute can now be written in all of the syntactic
+  locations a declaration attribute may appear. Fixes PR53805.
+
 Windows Support
 ---------------
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to