From b3b0b8034c60c382516448cea0abacdbdff8bc8b Mon Sep 17 00:00:00 2001 From: Daniel Jasper Date: Fri, 20 Jun 2014 08:44:22 +0000 Subject: [PATCH] Fix/Improve SourceRange of explicitly defaulted members When adding the implicit compound statement (required for Codegen?), the end location was previously overridden by the start location, probably based on the assumptions: * The location of the compound statement should be the member's location * The compound statement if present is the last element of a FunctionDecl This patch changes the location of the compound statement to the member's end location. Code review: http://reviews.llvm.org/D4175 llvm-svn: 211344 --- clang/lib/Sema/SemaDeclCXX.cpp | 37 +++++++++++++------ clang/test/Analysis/inlining/path-notes.cpp | 10 ++--- clang/test/Misc/ast-dump-decl.cpp | 25 +++++++++++++ ...nstantiate-default-assignment-operator.cpp | 2 +- 4 files changed, 56 insertions(+), 18 deletions(-) diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index b9eda84e5ae5..85394bb4de75 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -8407,7 +8407,9 @@ void Sema::DefineImplicitDefaultConstructor(SourceLocation CurrentLocation, return; } - SourceLocation Loc = Constructor->getLocation(); + SourceLocation Loc = Constructor->getLocEnd().isValid() + ? Constructor->getLocEnd() + : Constructor->getLocation(); Constructor->setBody(new (Context) CompoundStmt(Loc)); Constructor->markUsed(Context); @@ -8869,7 +8871,9 @@ void Sema::DefineImplicitDestructor(SourceLocation CurrentLocation, return; } - SourceLocation Loc = Destructor->getLocation(); + SourceLocation Loc = Destructor->getLocEnd().isValid() + ? Destructor->getLocEnd() + : Destructor->getLocation(); Destructor->setBody(new (Context) CompoundStmt(Loc)); Destructor->markUsed(Context); MarkVTableUsed(CurrentLocation, ClassDecl); @@ -9569,8 +9573,10 @@ void Sema::DefineImplicitCopyAssignment(SourceLocation CurrentLocation, } // Our location for everything implicitly-generated. - SourceLocation Loc = CopyAssignOperator->getLocation(); - + SourceLocation Loc = CopyAssignOperator->getLocEnd().isValid() + ? CopyAssignOperator->getLocEnd() + : CopyAssignOperator->getLocation(); + // Builds a DeclRefExpr for the "other" object. RefBuilder OtherRef(Other, OtherRefType); @@ -9974,7 +9980,9 @@ void Sema::DefineImplicitMoveAssignment(SourceLocation CurrentLocation, "Bad argument type of defaulted move assignment"); // Our location for everything implicitly-generated. - SourceLocation Loc = MoveAssignOperator->getLocation(); + SourceLocation Loc = MoveAssignOperator->getLocEnd().isValid() + ? MoveAssignOperator->getLocEnd() + : MoveAssignOperator->getLocation(); // Builds a reference to the "other" object. RefBuilder OtherRef(Other, OtherRefType); @@ -10111,8 +10119,9 @@ void Sema::DefineImplicitMoveAssignment(SourceLocation CurrentLocation, if (!Invalid) { // Add a "return *this;" - ExprResult ThisObj = CreateBuiltinUnaryOp(Loc, UO_Deref, This.build(*this, Loc)); - + ExprResult ThisObj = + CreateBuiltinUnaryOp(Loc, UO_Deref, This.build(*this, Loc)); + StmtResult Return = BuildReturnStmt(Loc, ThisObj.get()); if (Return.isInvalid()) Invalid = true; @@ -10288,10 +10297,12 @@ void Sema::DefineImplicitCopyConstructor(SourceLocation CurrentLocation, << CXXCopyConstructor << Context.getTagDeclType(ClassDecl); CopyConstructor->setInvalidDecl(); } else { + SourceLocation Loc = CopyConstructor->getLocEnd().isValid() + ? CopyConstructor->getLocEnd() + : CopyConstructor->getLocation(); Sema::CompoundScopeRAII CompoundScope(*this); - CopyConstructor->setBody(ActOnCompoundStmt( - CopyConstructor->getLocation(), CopyConstructor->getLocation(), None, - /*isStmtExpr=*/ false).getAs()); + CopyConstructor->setBody( + ActOnCompoundStmt(Loc, Loc, None, /*isStmtExpr=*/false).getAs()); } CopyConstructor->markUsed(Context); @@ -10444,10 +10455,12 @@ void Sema::DefineImplicitMoveConstructor(SourceLocation CurrentLocation, << CXXMoveConstructor << Context.getTagDeclType(ClassDecl); MoveConstructor->setInvalidDecl(); } else { + SourceLocation Loc = MoveConstructor->getLocEnd().isValid() + ? MoveConstructor->getLocEnd() + : MoveConstructor->getLocation(); Sema::CompoundScopeRAII CompoundScope(*this); MoveConstructor->setBody(ActOnCompoundStmt( - MoveConstructor->getLocation(), MoveConstructor->getLocation(), None, - /*isStmtExpr=*/ false).getAs()); + Loc, Loc, None, /*isStmtExpr=*/ false).getAs()); } MoveConstructor->markUsed(Context); diff --git a/clang/test/Analysis/inlining/path-notes.cpp b/clang/test/Analysis/inlining/path-notes.cpp index a1ac53c50359..1e230740cf05 100644 --- a/clang/test/Analysis/inlining/path-notes.cpp +++ b/clang/test/Analysis/inlining/path-notes.cpp @@ -2452,12 +2452,12 @@ namespace PR17746 { // CHECK-NEXT: // CHECK-NEXT: // CHECK-NEXT: line105 -// CHECK-NEXT: col21 +// CHECK-NEXT: col53 // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: // CHECK-NEXT: line105 -// CHECK-NEXT: col28 +// CHECK-NEXT: col53 // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: @@ -2469,7 +2469,7 @@ namespace PR17746 { // CHECK-NEXT: location // CHECK-NEXT: // CHECK-NEXT: line105 -// CHECK-NEXT: col21 +// CHECK-NEXT: col53 // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: ranges @@ -2477,12 +2477,12 @@ namespace PR17746 { // CHECK-NEXT: // CHECK-NEXT: // CHECK-NEXT: line105 -// CHECK-NEXT: col21 +// CHECK-NEXT: col53 // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: // CHECK-NEXT: line105 -// CHECK-NEXT: col28 +// CHECK-NEXT: col53 // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: diff --git a/clang/test/Misc/ast-dump-decl.cpp b/clang/test/Misc/ast-dump-decl.cpp index 8f20c46c0acd..b41e4ee3d0f0 100644 --- a/clang/test/Misc/ast-dump-decl.cpp +++ b/clang/test/Misc/ast-dump-decl.cpp @@ -133,6 +133,31 @@ class TestCXXDestructorDecl { // CHECK: CXXDestructorDecl{{.*}} ~TestCXXDestructorDecl 'void (void) noexcept' // CHECK-NEXT: CompoundStmt +// Test that the range of a defaulted members is computed correctly. +// FIXME: This should include the "= default". +class TestMemberRanges { +public: + TestMemberRanges() = default; + TestMemberRanges(const TestMemberRanges &Other) = default; + TestMemberRanges(TestMemberRanges &&Other) = default; + ~TestMemberRanges() = default; + TestMemberRanges &operator=(const TestMemberRanges &Other) = default; + TestMemberRanges &operator=(TestMemberRanges &&Other) = default; +}; +void SomeFunction() { + TestMemberRanges A; + TestMemberRanges B(A); + B = A; + A = static_cast(B); + TestMemberRanges C(static_cast(A)); +} +// CHECK: CXXConstructorDecl{{.*}} +// CHECK: CXXConstructorDecl{{.*}} +// CHECK: CXXConstructorDecl{{.*}} +// CHECK: CXXDestructorDecl{{.*}} +// CHECK: CXXMethodDecl{{.*}} +// CHECK: CXXMethodDecl{{.*}} + class TestCXXConversionDecl { operator int() { return 0; } }; diff --git a/clang/test/SemaTemplate/instantiate-default-assignment-operator.cpp b/clang/test/SemaTemplate/instantiate-default-assignment-operator.cpp index 31cdef59d9fd..67946713c019 100644 --- a/clang/test/SemaTemplate/instantiate-default-assignment-operator.cpp +++ b/clang/test/SemaTemplate/instantiate-default-assignment-operator.cpp @@ -13,5 +13,5 @@ void f() { a1 = a2; B b1, b2; - b1 = b2; + b1 = b2; }