[Sema] Fix PR27122: ICE with enable_if+ill-formed call.

In some cases, when we encounter a direct function call with an
incorrect number of arguments, we'll emit a diagnostic, and pretend that
the call to the function was valid. For example, in C:

int foo();
int a = foo(1);

Prior to this patch, we'd get an ICE if foo had an enable_if attribute,
because CheckEnableIf assumes that the number of arguments it gets
passed is valid for the function it's passed. Now, we check that the
number of args looks valid prior to checking enable_if conditions.

This fix was not done inside of CheckEnableIf because the problem
presently can only occur in one caller of CheckEnableIf (ActOnCallExpr).
Additionally, checking inside of CheckEnableIf would make us emit
multiple diagnostics for the same error (one "enable_if failed", one
"you gave this function the wrong number of arguments"), which seems
worse than just complaining about the latter.

llvm-svn: 264975

diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index b93a568..93b5ff9 100644 (file)
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -5044,6 +5044,14 @@ static FunctionDecl *rewriteBuiltinFunctionDecl(Sema *Sema, ASTContext &Context,
   return OverloadDecl;
 }
 
+static bool isNumberOfArgsValidForCall(Sema &S, const FunctionDecl *Callee,
+                                       std::size_t NumArgs) {
+  if (S.TooManyArguments(Callee->getNumParams(), NumArgs,
+                         /*PartialOverloading=*/false))
+    return Callee->isVariadic();
+  return Callee->getMinRequiredArguments() <= NumArgs;
+}
+
 /// ActOnCallExpr - Handle a call to Fn with the specified array of arguments.
 /// This provides the location of the left/right parens and a list of comma
 /// locations.
@@ -5175,7 +5183,14 @@ Sema::ActOnCallExpr(Scope *S, Expr *Fn, SourceLocation LParenLoc,
                                            Fn->getLocStart()))
       return ExprError();
 
-    if (FD->hasAttr<EnableIfAttr>()) {
+    // CheckEnableIf assumes that the we're passing in a sane number of args for
+    // FD, but that doesn't always hold true here. This is because, in some
+    // cases, we'll emit a diag about an ill-formed function call, but then
+    // we'll continue on as if the function call wasn't ill-formed. So, if the
+    // number of args looks incorrect, don't do enable_if checks; we should've
+    // already emitted an error about the bad call.
+    if (FD->hasAttr<EnableIfAttr>() &&
+        isNumberOfArgsValidForCall(*this, FD, ArgExprs.size())) {
       if (const EnableIfAttr *Attr = CheckEnableIf(FD, ArgExprs, true)) {
         Diag(Fn->getLocStart(),
              isa<CXXMethodDecl>(FD) ?

diff --git a/clang/test/Sema/enable_if.c b/clang/test/Sema/enable_if.c
index 0cd9c48..4034aa2 100644 (file)
--- a/clang/test/Sema/enable_if.c
+++ b/clang/test/Sema/enable_if.c
@@ -142,4 +142,11 @@ void test8() {
   void (*p1)(int) = &f4; // expected-error{{cannot take address of function 'f4' becuase it has one or more non-tautological enable_if conditions}}
   void (*p2)(int) = f4; // expected-error{{cannot take address of function 'f4' becuase it has one or more non-tautological enable_if conditions}}
 }
+
+void regular_enable_if(int a) __attribute__((enable_if(a, ""))); // expected-note 3{{declared here}}
+void PR27122_ext() {
+  regular_enable_if(0, 2); // expected-error{{too many arguments}}
+  regular_enable_if(1, 2); // expected-error{{too many arguments}}
+  regular_enable_if(); // expected-error{{too few arguments}}
+}
 #endif

diff --git a/clang/test/SemaCXX/enable_if.cpp b/clang/test/SemaCXX/enable_if.cpp
index 14152d3..a2795c4 100644 (file)
--- a/clang/test/SemaCXX/enable_if.cpp
+++ b/clang/test/SemaCXX/enable_if.cpp
@@ -346,3 +346,43 @@ void testIt() {
   auto CRef = (NoMatchTy)&foo; // expected-error{{address of overloaded function 'foo' does not match required type 'void ()'}}
 }
 }
+
+namespace PR27122 {
+// (slightly reduced) code that motivated the bug...
+namespace ns {
+void Function(int num)
+  __attribute__((enable_if(num != 0, "")));
+void Function(int num, int a0)
+  __attribute__((enable_if(num != 1, "")));
+}  // namespace ns
+
+using ns::Function; // expected-note 3{{declared here}}
+void Run() {
+  Functioon(0); // expected-error{{use of undeclared identifier}} expected-error{{too few arguments}}
+  Functioon(0, 1); // expected-error{{use of undeclared identifier}}
+  Functioon(0, 1, 2); // expected-error{{use of undeclared identifier}}
+}
+
+// Extra tests
+void regularEnableIf(int a) __attribute__((enable_if(a, ""))); // expected-note 3{{declared here}} expected-note 3{{candidate function not viable}}
+void runRegularEnableIf() {
+  regularEnableIf(0, 2); // expected-error{{no matching function}}
+  regularEnableIf(1, 2); // expected-error{{no matching function}}
+  regularEnableIf(); // expected-error{{no matching function}}
+
+  // Test without getting overload resolution involved
+  ::PR27122::regularEnableIf(0, 2); // expected-error{{too many arguments}}
+  ::PR27122::regularEnableIf(1, 2); // expected-error{{too many arguments}}
+  ::PR27122::regularEnableIf(); // expected-error{{too few arguments}}
+}
+
+struct Foo {
+  void bar(int i) __attribute__((enable_if(i, ""))); // expected-note 2{{declared here}}
+};
+
+void runFoo() {
+  Foo f;
+  f.bar(); // expected-error{{too few arguments}}
+  f.bar(1, 2); // expected-error{{too many arguments}}
+}
+}