]> granicus.if.org Git - clang/commitdiff
[Sema] Fix PR27122: ICE with enable_if+ill-formed call.
authorGeorge Burgess IV <george.burgess.iv@gmail.com>
Thu, 31 Mar 2016 00:16:25 +0000 (00:16 +0000)
committerGeorge Burgess IV <george.burgess.iv@gmail.com>
Thu, 31 Mar 2016 00:16:25 +0000 (00:16 +0000)
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.

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

lib/Sema/SemaExpr.cpp
test/Sema/enable_if.c
test/SemaCXX/enable_if.cpp

index b93a568ebee80183e3ffa71f79366198e647bd87..93b5ff9c99657f2e164fea8a063e3912cc817141 100644 (file)
@@ -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) ?
index 0cd9c48f42b58a9a5c0ab8324b7302665ef7a070..4034aa2bfab05bc7e26526c66807e0a497107f1d 100644 (file)
@@ -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
index 14152d308970ff6d425c17394d2ea072ce919230..a2795c418c635122bd67256f48667ec793bf4d8c 100644 (file)
@@ -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}}
+}
+}