<p dir="ltr">Maybe we should fix this when we create the Expr rather than putting parse error recovery logic in AST?</p>
<div class="gmail_quote">On 4 Jan 2015 20:37, "Nico Weber" <<a href="mailto:nicolasweber@gmx.de">nicolasweber@gmx.de</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: nico<br>
Date: Sun Jan  4 14:32:12 2015<br>
New Revision: 225140<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=225140&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=225140&view=rev</a><br>
Log:<br>
Remove an assert that's not true on invalid code.<br>
<br>
r185773 added an assert that checked that a CXXUnresolvedConstructExpr either<br>
has a valid rparen, or exactly one argument.  This doesn't have to be true for<br>
invalid inputs.  Convert the assert to an if, and add a test for this case.<br>
<br>
Found by SLi's afl bot.<br>
<br>
Added:<br>
    cfe/trunk/test/Misc/ast-dump-invalid.cpp<br>
Modified:<br>
    cfe/trunk/include/clang/AST/ExprCXX.h<br>
    cfe/trunk/test/SemaCXX/return.cpp<br>
<br>
Modified: cfe/trunk/include/clang/AST/ExprCXX.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExprCXX.h?rev=225140&r1=225139&r2=225140&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExprCXX.h?rev=225140&r1=225139&r2=225140&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/AST/ExprCXX.h (original)<br>
+++ cfe/trunk/include/clang/AST/ExprCXX.h Sun Jan  4 14:32:12 2015<br>
@@ -2915,8 +2915,9 @@ public:<br>
<br>
   SourceLocation getLocStart() const LLVM_READONLY;<br>
   SourceLocation getLocEnd() const LLVM_READONLY {<br>
-    assert(RParenLoc.isValid() || NumArgs == 1);<br>
-    return RParenLoc.isValid() ? RParenLoc : getArg(0)->getLocEnd();<br>
+    if (!RParenLoc.isValid() && NumArgs > 0)<br>
+      return getArg(NumArgs - 1)->getLocEnd();<br>
+    return RParenLoc;<br>
   }<br>
<br>
   static bool classof(const Stmt *T) {<br>
<br>
Added: cfe/trunk/test/Misc/ast-dump-invalid.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/ast-dump-invalid.cpp?rev=225140&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/ast-dump-invalid.cpp?rev=225140&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/Misc/ast-dump-invalid.cpp (added)<br>
+++ cfe/trunk/test/Misc/ast-dump-invalid.cpp Sun Jan  4 14:32:12 2015<br>
@@ -0,0 +1,20 @@<br>
+// RUN: not %clang_cc1 -std=c++11 -triple x86_64-linux-gnu -fms-extensions -ast-dump -ast-dump-filter Test %s | FileCheck -check-prefix CHECK -strict-whitespace %s<br>
+<br>
+namespace TestInvalidRParenOnCXXUnresolvedConstructExpr {<br>
+template <class T><br>
+void f(T i, T j) {<br>
+  return T (i, j;<br>
+}<br>
+}<br>
+<br>
+// CHECK: NamespaceDecl {{.*}} <{{.*}}> {{.*}} TestInvalidRParenOnCXXUnresolvedConstructExpr<br>
+// CHECK-NEXT: `-FunctionTemplateDecl<br>
+// CHECK-NEXT:   |-TemplateTypeParmDecl<br>
+// CHECK-NEXT:   `-FunctionDecl<br>
+// CHECK-NEXT:     |-ParmVarDecl<br>
+// CHECK-NEXT:     |-ParmVarDecl<br>
+// CHECK-NEXT:     `-CompoundStmt<br>
+// CHECK-NEXT:       `-ReturnStmt<br>
+// CHECK-NEXT:         `-CXXUnresolvedConstructExpr {{.*}} <col:10, col:16> 'T'<br>
+// CHECK-NEXT:           |-DeclRefExpr {{.*}} <col:13> 'T' lvalue ParmVar {{.*}} 'i' 'T'<br>
+// CHECK-NEXT:           `-DeclRefExpr {{.*}} <col:16> 'T' lvalue ParmVar {{.*}} 'j' 'T'<br>
<br>
Modified: cfe/trunk/test/SemaCXX/return.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/return.cpp?rev=225140&r1=225139&r2=225140&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/return.cpp?rev=225140&r1=225139&r2=225140&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaCXX/return.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/return.cpp Sun Jan  4 14:32:12 2015<br>
@@ -112,3 +112,11 @@ namespace ctor_returns_void {<br>
     ~S() { return f(); } // expected-error {{destructor '~S' must not return void expression}}<br>
   };<br>
 }<br>
+<br>
+void cxx_unresolved_expr() {<br>
+  // The use of an undeclared variable tricks clang into building a<br>
+  // CXXUnresolvedConstructExpr, and the missing ')' gives it an invalid source<br>
+  // location for its rparen.  Check that emitting a diag on the range of the<br>
+  // expr doesn't assert.<br>
+  return int(undeclared, 4; // expected-error {{expected ')'}} expected-note{{to match this '('}} expected-error {{void function 'cxx_unresolved_expr' should not return a value}} expected-error {{use of undeclared identifier 'undeclared'}}<br>
+}<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>