On Sun, Jan 4, 2015 at 4:30 PM, Richard Smith <[email protected]> wrote:
> Maybe we should fix this when we create the Expr rather than putting parse > error recovery logic in AST? > Is that how this is usually done? I thought usually AST nodes for invalid code end up with invalid SourceLocs. > On 4 Jan 2015 20:37, "Nico Weber" <[email protected]> wrote: > >> Author: nico >> Date: Sun Jan 4 14:32:12 2015 >> New Revision: 225140 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=225140&view=rev >> Log: >> Remove an assert that's not true on invalid code. >> >> r185773 added an assert that checked that a CXXUnresolvedConstructExpr >> either >> has a valid rparen, or exactly one argument. This doesn't have to be >> true for >> invalid inputs. Convert the assert to an if, and add a test for this >> case. >> >> Found by SLi's afl bot. >> >> Added: >> cfe/trunk/test/Misc/ast-dump-invalid.cpp >> Modified: >> cfe/trunk/include/clang/AST/ExprCXX.h >> cfe/trunk/test/SemaCXX/return.cpp >> >> Modified: cfe/trunk/include/clang/AST/ExprCXX.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExprCXX.h?rev=225140&r1=225139&r2=225140&view=diff >> >> ============================================================================== >> --- cfe/trunk/include/clang/AST/ExprCXX.h (original) >> +++ cfe/trunk/include/clang/AST/ExprCXX.h Sun Jan 4 14:32:12 2015 >> @@ -2915,8 +2915,9 @@ public: >> >> SourceLocation getLocStart() const LLVM_READONLY; >> SourceLocation getLocEnd() const LLVM_READONLY { >> - assert(RParenLoc.isValid() || NumArgs == 1); >> - return RParenLoc.isValid() ? RParenLoc : getArg(0)->getLocEnd(); >> + if (!RParenLoc.isValid() && NumArgs > 0) >> + return getArg(NumArgs - 1)->getLocEnd(); >> + return RParenLoc; >> } >> >> static bool classof(const Stmt *T) { >> >> Added: cfe/trunk/test/Misc/ast-dump-invalid.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/ast-dump-invalid.cpp?rev=225140&view=auto >> >> ============================================================================== >> --- cfe/trunk/test/Misc/ast-dump-invalid.cpp (added) >> +++ cfe/trunk/test/Misc/ast-dump-invalid.cpp Sun Jan 4 14:32:12 2015 >> @@ -0,0 +1,20 @@ >> +// 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 >> + >> +namespace TestInvalidRParenOnCXXUnresolvedConstructExpr { >> +template <class T> >> +void f(T i, T j) { >> + return T (i, j; >> +} >> +} >> + >> +// CHECK: NamespaceDecl {{.*}} <{{.*}}> {{.*}} >> TestInvalidRParenOnCXXUnresolvedConstructExpr >> +// CHECK-NEXT: `-FunctionTemplateDecl >> +// CHECK-NEXT: |-TemplateTypeParmDecl >> +// CHECK-NEXT: `-FunctionDecl >> +// CHECK-NEXT: |-ParmVarDecl >> +// CHECK-NEXT: |-ParmVarDecl >> +// CHECK-NEXT: `-CompoundStmt >> +// CHECK-NEXT: `-ReturnStmt >> +// CHECK-NEXT: `-CXXUnresolvedConstructExpr {{.*}} <col:10, >> col:16> 'T' >> +// CHECK-NEXT: |-DeclRefExpr {{.*}} <col:13> 'T' lvalue >> ParmVar {{.*}} 'i' 'T' >> +// CHECK-NEXT: `-DeclRefExpr {{.*}} <col:16> 'T' lvalue >> ParmVar {{.*}} 'j' 'T' >> >> Modified: cfe/trunk/test/SemaCXX/return.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/return.cpp?rev=225140&r1=225139&r2=225140&view=diff >> >> ============================================================================== >> --- cfe/trunk/test/SemaCXX/return.cpp (original) >> +++ cfe/trunk/test/SemaCXX/return.cpp Sun Jan 4 14:32:12 2015 >> @@ -112,3 +112,11 @@ namespace ctor_returns_void { >> ~S() { return f(); } // expected-error {{destructor '~S' must not >> return void expression}} >> }; >> } >> + >> +void cxx_unresolved_expr() { >> + // The use of an undeclared variable tricks clang into building a >> + // CXXUnresolvedConstructExpr, and the missing ')' gives it an invalid >> source >> + // location for its rparen. Check that emitting a diag on the range >> of the >> + // expr doesn't assert. >> + 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'}} >> +} >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
