[PATCH] PR16182 - Visit parameter declaration of implicitly declared copy assignment operator.

[Michael Han]

Ping.

To recap: this patch fixes PR16182 (
http://llvm.org/bugs/show_bug.cgi?id=16182). OK to commit?

Thanks.


On Tue, Jun 11, 2013 at 1:31 PM, Michael Han <fragmentshaders at gmail.com>wrote:

> Hi klimek, rsmith,
>
> This patch fixes PR16182
> http://llvm.org/bugs/show_bug.cgi?id=16182
>
> A valid TypeSourceInfo of a function is required for the RAV to visit the
> parameter declaration of the function, and previously we don't have a valid
> TypeSourceInfo when creating the function declaration of implicit copy
> assignment operator.
>
> This patch fixes this by building a TypeSourceInfo from the function
> prototype of copy assignment operator and associate its ParmVarDecl with
> this function type location.
>
> The test is done by matching that an empty name (we don't have a name for
> the parameter of the implicit copy assignment function) appears at expect
> location, which is the source location of the class that is also used as
> the location of the implicit copy assignment function.
>
>
> http://llvm-reviews.chandlerc.com/D958
>
> Files:
>   unittests/Tooling/TestVisitor.h
>   unittests/Tooling/RecursiveASTVisitorTest.cpp
>   lib/Sema/SemaDeclCXX.cpp
>
> Index: unittests/Tooling/TestVisitor.h
> ===================================================================
> --- unittests/Tooling/TestVisitor.h
> +++ unittests/Tooling/TestVisitor.h
> @@ -31,7 +31,7 @@
>  /// This is a drop-in replacement for RecursiveASTVisitor itself, with the
>  /// additional capability of running it over a snippet of code.
>  ///
> -/// Visits template instantiations (but not implicit code) by default.
> +/// Visits template instantiations and implicit code by default.
>  template <typename T>
>  class TestVisitor : public RecursiveASTVisitor<T> {
>  public:
> @@ -55,6 +55,10 @@
>      return true;
>    }
>
> +  bool shouldVisitImplicitCode() const {
> +    return true;
> +  }
> +
>  protected:
>    virtual ASTFrontendAction* CreateTestAction() {
>      return new TestAction(this);
> Index: unittests/Tooling/RecursiveASTVisitorTest.cpp
> ===================================================================
> --- unittests/Tooling/RecursiveASTVisitorTest.cpp
> +++ unittests/Tooling/RecursiveASTVisitorTest.cpp
> @@ -35,6 +35,14 @@
>   }
>  };
>
> +class ParmVarDeclVisitor : public
> ExpectedLocationVisitor<ParmVarDeclVisitor> {
> +public:
> +  bool VisitParmVarDecl(ParmVarDecl *ParamVar) {
> +    Match(ParamVar->getNameAsString(), ParamVar->getLocStart());
> +    return true;
> +  }
> +};
> +
>  class CXXMemberCallVisitor
>    : public ExpectedLocationVisitor<CXXMemberCallVisitor> {
>  public:
> @@ -106,6 +114,14 @@
>    }
>  };
>
> +TEST(RecursiveASTVisitor, VisitsParmVarDecl) {
> +  ParmVarDeclVisitor Visitor;
> +  Visitor.ExpectMatch("", 1, 7);
> +  EXPECT_TRUE(Visitor.runOver(
> +    "class X {};\n"
> +    "void foo(X a, X b) {a = b;}"));
> +}
> +
>  TEST(RecursiveASTVisitor, VisitsBaseClassDeclarations) {
>    TypeLocVisitor Visitor;
>    Visitor.ExpectMatch("class X", 1, 30);
> Index: lib/Sema/SemaDeclCXX.cpp
> ===================================================================
> --- lib/Sema/SemaDeclCXX.cpp
> +++ lib/Sema/SemaDeclCXX.cpp
> @@ -8788,14 +8788,24 @@
>    FunctionProtoType::ExtProtoInfo EPI;
>    EPI.ExceptionSpecType = EST_Unevaluated;
>    EPI.ExceptionSpecDecl = CopyAssignment;
> -  CopyAssignment->setType(Context.getFunctionType(RetType, ArgType, EPI));
> +  QualType T = Context.getFunctionType(RetType, ArgType, EPI);
> +  CopyAssignment->setType(T);
>
> +  // PR16182. Build type source info for copy assignment operator. RAV
> relies on
> +  // type source info to traverse parameter declaration of implicit
> +  // declared copy assignment operator.
> +  TypeSourceInfo *TSInfo = Context.getTrivialTypeSourceInfo(T, ClassLoc);
> +  CopyAssignment->setTypeSourceInfo(TSInfo);
> +  UnqualTypeLoc UnQualTL = TSInfo->getTypeLoc().getUnqualifiedLoc();
> +  FunctionTypeLoc FTL = UnQualTL.getAs<FunctionTypeLoc>();
> +
>    // Add the parameter to the operator.
>    ParmVarDecl *FromParam = ParmVarDecl::Create(Context, CopyAssignment,
>                                                 ClassLoc, ClassLoc,
> /*Id=*/0,
>                                                 ArgType, /*TInfo=*/0,
>                                                 SC_None, 0);
>    CopyAssignment->setParams(FromParam);
> +  FTL.setArg(0, FromParam);
>
>    AddOverriddenMethods(ClassDecl, CopyAssignment);
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130801/e6574615/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: implicit.patch
Type: application/octet-stream
Size: 3623 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130801/e6574615/attachment.obj>