[clang-tools-extra] r189584 - cpp11-migrate: Add Pass-By-Value Transform

David Blaikie dblaikie at gmail.com
Thu Aug 29 08:23:03 PDT 2013


On Thu, Aug 29, 2013 at 6:42 AM, Guillaume Papin
<guillaume.papin at epitech.eu> wrote:
> Author: papin_g
> Date: Thu Aug 29 08:42:13 2013
> New Revision: 189584
>
> URL: http://llvm.org/viewvc/llvm-project?rev=189584&view=rev
> Log:
> cpp11-migrate: Add Pass-By-Value Transform
>
> Currently only constructor parameters stored in class-local storage are modified
> to make use of the pass-by-value idiom but this is a base that can be be further
> improved to handle more situations.

Awesome! I've been following along with this & really looking forward
to seeing this transformation grow with more common idioms. (& if we
ever see something like this run over a large-ish existing code base
with perf comparisons... I'll be quite curious to see the results)

>
> This commit is the same as r189363 with additionnal fixes for the build issues.
>
> Added:
>     clang-tools-extra/trunk/cpp11-migrate/PassByValue/
>     clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValue.cpp
>     clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValue.h
>     clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValueActions.cpp
>     clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValueActions.h
>     clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValueMatchers.cpp
>     clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValueMatchers.h
>     clang-tools-extra/trunk/docs/PassByValueTransform.rst
>     clang-tools-extra/trunk/test/cpp11-migrate/PassByValue/
>     clang-tools-extra/trunk/test/cpp11-migrate/PassByValue/basic.cpp
>     clang-tools-extra/trunk/test/cpp11-migrate/PassByValue/basic.h
> Modified:
>     clang-tools-extra/trunk/cpp11-migrate/Core/Transform.h
>     clang-tools-extra/trunk/cpp11-migrate/tool/CMakeLists.txt
>     clang-tools-extra/trunk/cpp11-migrate/tool/Cpp11Migrate.cpp
>     clang-tools-extra/trunk/cpp11-migrate/tool/Makefile
>     clang-tools-extra/trunk/docs/MigratorUsage.rst
>     clang-tools-extra/trunk/docs/cpp11-migrate.rst
>
> Modified: clang-tools-extra/trunk/cpp11-migrate/Core/Transform.h
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/Core/Transform.h?rev=189584&r1=189583&r2=189584&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/cpp11-migrate/Core/Transform.h (original)
> +++ clang-tools-extra/trunk/cpp11-migrate/Core/Transform.h Thu Aug 29 08:42:13 2013
> @@ -139,6 +139,12 @@ public:
>    bool isFileModifiable(const clang::SourceManager &SM,
>                          const clang::SourceLocation &Loc) const;
>
> +  /// \brief Whether a transformation with a risk level of \p RiskLevel is
> +  /// acceptable or not.
> +  bool isAcceptableRiskLevel(RiskLevel RiskLevel) const {
> +    return RiskLevel <= GlobalOptions.MaxRiskLevel;
> +  }
> +
>    /// \brief Called before parsing a translation unit for a FrontendAction.
>    ///
>    /// Transform uses this function to apply file overrides and start
>
> Added: clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValue.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValue.cpp?rev=189584&view=auto
> ==============================================================================
> --- clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValue.cpp (added)
> +++ clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValue.cpp Thu Aug 29 08:42:13 2013
> @@ -0,0 +1,80 @@
> +//===-- PassByValue.cpp ---------------------------------------------------===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +///
> +/// \file
> +/// \brief This file provides the implementation of the ReplaceAutoPtrTransform
> +/// class.
> +///
> +//===----------------------------------------------------------------------===//
> +
> +#include "PassByValue.h"
> +#include "PassByValueActions.h"
> +#include "PassByValueMatchers.h"
> +
> +using namespace clang;
> +using namespace clang::tooling;
> +using namespace clang::ast_matchers;
> +
> +int PassByValueTransform::apply(
> +    FileOverrides &InputStates, const tooling::CompilationDatabase &Database,
> +    const std::vector<std::string> &SourcePaths) {
> +  ClangTool Tool(Database, SourcePaths);
> +  unsigned AcceptedChanges = 0;
> +  unsigned RejectedChanges = 0;
> +  MatchFinder Finder;
> +  ConstructorParamReplacer Replacer(getReplacements(), AcceptedChanges,
> +                                    RejectedChanges,
> +                                    /*Owner=*/ *this);
> +
> +  Finder.addMatcher(makePassByValueCtorParamMatcher(), &Replacer);
> +
> +  // make the replacer available to handleBeginSource()
> +  this->Replacer = &Replacer;
> +
> +  setOverrides(InputStates);
> +
> +  if (Tool.run(createActionFactory(Finder))) {
> +    llvm::errs() << "Error encountered during translation.\n";
> +    return 1;
> +  }
> +
> +  setAcceptedChanges(AcceptedChanges);
> +  setRejectedChanges(RejectedChanges);
> +  return 0;
> +}
> +
> +bool PassByValueTransform::handleBeginSource(CompilerInstance &CI,
> +                                             llvm::StringRef Filename) {
> +  assert(Replacer && "Replacer not set");
> +  IncludeManager.reset(new IncludeDirectives(CI));
> +  Replacer->setIncludeDirectives(IncludeManager.get());
> +  return Transform::handleBeginSource(CI, Filename);
> +}
> +
> +struct PassByValueFactory : TransformFactory {
> +  PassByValueFactory() {
> +    // Based on the Replace Auto-Ptr Transform that is also using std::move().
> +    Since.Clang = Version(3, 0);
> +    Since.Gcc = Version(4, 6);
> +    Since.Icc = Version(13);
> +    Since.Msvc = Version(11);
> +  }
> +
> +  Transform *createTransform(const TransformOptions &Opts) LLVM_OVERRIDE {
> +    return new PassByValueTransform(Opts);
> +  }
> +};
> +
> +// Register the factory using this statically initialized variable.
> +static TransformFactoryRegistry::Add<PassByValueFactory>
> +X("pass-by-value", "Pass parameters by value where possible");
> +
> +// This anchor is used to force the linker to link in the generated object file
> +// and thus register the factory.
> +volatile int PassByValueTransformAnchorSource = 0;
>
> Added: clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValue.h
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValue.h?rev=189584&view=auto
> ==============================================================================
> --- clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValue.h (added)
> +++ clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValue.h Thu Aug 29 08:42:13 2013
> @@ -0,0 +1,74 @@
> +//===-- PassByValue.h -------------------------------------------*- C++ -*-===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +///
> +/// \file
> +/// \brief This file provides the declaration of the PassByValueTransform
> +/// class.
> +///
> +//===----------------------------------------------------------------------===//
> +
> +#ifndef CPP11_MIGRATE_PASS_BY_VALUE_H
> +#define CPP11_MIGRATE_PASS_BY_VALUE_H
> +
> +#include "Core/Transform.h"
> +#include "Core/IncludeDirectives.h"
> +
> +class ConstructorParamReplacer;
> +
> +/// \brief Subclass of Transform that uses pass-by-value semantic when move
> +/// constructors are available to avoid copies.
> +///
> +/// When a class constructor accepts an object by const reference with the
> +/// intention of copying the object the copy can be avoided in certain
> +/// situations if the object has a move constructor. First, the constructor is
> +/// changed to accept the object by value instead. Then this argument is moved
> +/// instead of copied into class-local storage. If an l-value is provided to the
> +/// constructor, there is no difference in the number of copies made. However,
> +/// if an r-value is passed, the copy is avoided completely.
> +///
> +/// For example, given:
> +/// \code
> +/// #include <string>
> +///
> +/// class A {
> +///   std::string S;
> +/// public:
> +///   A(const std::string &S) : S(S) {}
> +/// };
> +/// \endcode
> +/// the code is transformed to:
> +/// \code
> +/// #include <string>
> +///
> +/// class A {
> +///   std::string S;
> +/// public:
> +///   A(std::string S) : S(std::move(S)) {}
> +/// };
> +/// \endcode
> +class PassByValueTransform : public Transform {
> +public:
> +  PassByValueTransform(const TransformOptions &Options)
> +      : Transform("PassByValue", Options), Replacer(0) {}
> +
> +  /// \see Transform::apply().
> +  virtual int apply(FileOverrides &InputStates,
> +                    const clang::tooling::CompilationDatabase &Database,
> +                    const std::vector<std::string> &SourcePaths) LLVM_OVERRIDE;
> +
> +private:
> +  /// \brief Setups the \c IncludeDirectives for the replacer.
> +  virtual bool handleBeginSource(clang::CompilerInstance &CI,
> +                                 llvm::StringRef Filename) LLVM_OVERRIDE;
> +
> +  llvm::OwningPtr<IncludeDirectives> IncludeManager;
> +  ConstructorParamReplacer *Replacer;
> +};
> +
> +#endif // CPP11_MIGRATE_PASS_BY_VALUE_H
>
> Added: clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValueActions.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValueActions.cpp?rev=189584&view=auto
> ==============================================================================
> --- clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValueActions.cpp (added)
> +++ clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValueActions.cpp Thu Aug 29 08:42:13 2013
> @@ -0,0 +1,167 @@
> +//===-- PassByValueActions.cpp --------------------------------------------===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +///
> +/// \file
> +/// \brief This file contains the definition of the ASTMatcher callback for the
> +/// PassByValue transform.
> +///
> +//===----------------------------------------------------------------------===//
> +
> +#include "PassByValueActions.h"
> +#include "PassByValueMatchers.h"
> +#include "Core/IncludeDirectives.h"
> +#include "Core/Transform.h"
> +#include "clang/AST/RecursiveASTVisitor.h"
> +#include "clang/Basic/SourceManager.h"
> +#include "clang/Lex/Lexer.h"
> +
> +using namespace clang;
> +using namespace clang::tooling;
> +using namespace clang::ast_matchers;
> +
> +namespace {
> +/// \brief \c clang::RecursiveASTVisitor that checks that the given
> +/// \c ParmVarDecl is used exactly one time.
> +///
> +/// \see ExactlyOneUsageVisitor::hasExactlyOneUsageIn()
> +class ExactlyOneUsageVisitor
> +    : public RecursiveASTVisitor<ExactlyOneUsageVisitor> {
> +  friend class RecursiveASTVisitor<ExactlyOneUsageVisitor>;
> +
> +public:
> +  ExactlyOneUsageVisitor(const ParmVarDecl *ParamDecl) : ParamDecl(ParamDecl) {}
> +
> +  /// \brief Whether or not the parameter variable is referred only once in the
> +  /// given constructor.
> +  bool hasExactlyOneUsageIn(const CXXConstructorDecl *Ctor) {
> +    Count = 0;
> +    TraverseDecl(const_cast<CXXConstructorDecl *>(Ctor));
> +    return Count == 1;
> +  }
> +
> +private:
> +  /// \brief Counts the number of references to a variable.
> +  ///
> +  /// Stops the AST traversal if more than one usage is found.
> +  bool VisitDeclRefExpr(DeclRefExpr *D) {
> +    if (const ParmVarDecl *To = llvm::dyn_cast<ParmVarDecl>(D->getDecl()))
> +      if (To == ParamDecl) {
> +        ++Count;
> +        if (Count > 1)
> +          // no need to look further, used more than once
> +          return false;
> +      }
> +    return true;
> +  }
> +
> +  const ParmVarDecl *ParamDecl;
> +  unsigned Count;
> +};
> +} // end anonymous namespace
> +
> +/// \brief Whether or not \p ParamDecl is used exactly one time in \p Ctor.
> +///
> +/// Checks both in the init-list and the body of the constructor.
> +static bool paramReferredExactlyOnce(const CXXConstructorDecl *Ctor,
> +                                     const ParmVarDecl *ParamDecl) {
> +  ExactlyOneUsageVisitor Visitor(ParamDecl);
> +  return Visitor.hasExactlyOneUsageIn(Ctor);
> +}
> +
> +/// \brief Find all references to \p ParamDecl accross all of the
> +/// redeclarations of \p Ctor.
> +static void
> +collectParamDecls(const CXXConstructorDecl *Ctor, const ParmVarDecl *ParamDecl,
> +                  llvm::SmallVectorImpl<const ParmVarDecl *> &Results) {
> +  unsigned ParamIdx = ParamDecl->getFunctionScopeIndex();
> +
> +  for (CXXConstructorDecl::redecl_iterator I = Ctor->redecls_begin(),
> +                                           E = Ctor->redecls_end();
> +       I != E; ++I)
> +    Results.push_back((*I)->getParamDecl(ParamIdx));
> +}
> +
> +void ConstructorParamReplacer::run(const MatchFinder::MatchResult &Result) {
> +  assert(IncludeManager && "Include directives manager not set.");
> +  SourceManager &SM = *Result.SourceManager;
> +  const CXXConstructorDecl *Ctor =
> +      Result.Nodes.getNodeAs<CXXConstructorDecl>(PassByValueCtorId);
> +  const ParmVarDecl *ParamDecl =
> +      Result.Nodes.getNodeAs<ParmVarDecl>(PassByValueParamId);
> +  const CXXCtorInitializer *Initializer =
> +      Result.Nodes.getNodeAs<CXXCtorInitializer>(PassByValueInitializerId);
> +  assert(Ctor && ParamDecl && Initializer && "Bad Callback, missing node.");
> +
> +  // Check this now to avoid unecessary work. The param locations are checked
> +  // later.
> +  if (!Owner.isFileModifiable(SM, Initializer->getSourceLocation()))
> +    return;
> +
> +  // The parameter will be in an unspecified state after the move, so check if
> +  // the parameter is used for anything else other than the copy. If so do not
> +  // apply any changes.
> +  if (!paramReferredExactlyOnce(Ctor, ParamDecl))
> +    return;
> +
> +  llvm::SmallVector<const ParmVarDecl *, 2> AllParamDecls;
> +  collectParamDecls(Ctor, ParamDecl, AllParamDecls);
> +
> +  // Generate all replacements for the params.
> +  llvm::SmallVector<Replacement, 2> ParamReplaces(AllParamDecls.size());
> +  for (unsigned I = 0, E = AllParamDecls.size(); I != E; ++I) {
> +    TypeLoc ParamTL = AllParamDecls[I]->getTypeSourceInfo()->getTypeLoc();
> +    SourceRange Range(AllParamDecls[I]->getLocStart(), ParamTL.getLocEnd());
> +    CharSourceRange CharRange = Lexer::makeFileCharRange(
> +        CharSourceRange::getTokenRange(Range), SM, LangOptions());
> +    TypeLoc ValueTypeLoc = ParamTL;
> +    // transform non-value parameters (e.g: const-ref) to values
> +    if (!ParamTL.getNextTypeLoc().isNull())
> +      ValueTypeLoc = ParamTL.getNextTypeLoc();
> +    llvm::SmallString<32> ValueStr = Lexer::getSourceText(
> +        CharSourceRange::getTokenRange(ValueTypeLoc.getSourceRange()), SM,
> +        LangOptions());
> +
> +    // If it's impossible to change one of the parameter (e.g: comes from an
> +    // unmodifiable header) quit the callback now, do not generate any changes.
> +    if (CharRange.isInvalid() || ValueStr.empty() ||
> +        !Owner.isFileModifiable(SM, CharRange.getBegin()))
> +      return;
> +
> +    // 'const Foo &param' -> 'Foo param'
> +    //  ~~~~~~~~~~~           ~~~^
> +    ValueStr += ' ';
> +    ParamReplaces[I] = Replacement(SM, CharRange, ValueStr);
> +  }
> +
> +  // Reject the changes if the the risk level is not acceptable.
> +  if (!Owner.isAcceptableRiskLevel(RL_Reasonable)) {
> +    RejectedChanges++;
> +    return;
> +  }
> +
> +  // if needed, include <utility> in the file that uses std::move()
> +  const FileEntry *STDMoveFile =
> +      SM.getFileEntryForID(SM.getFileID(Initializer->getLParenLoc()));
> +  const tooling::Replacement &IncludeReplace =
> +      IncludeManager->addAngledInclude(STDMoveFile, "utility");
> +  if (IncludeReplace.isApplicable()) {
> +    Replaces.insert(IncludeReplace);
> +    AcceptedChanges++;
> +  }
> +
> +  // const-ref params becomes values (const Foo & -> Foo)
> +  Replaces.insert(ParamReplaces.begin(), ParamReplaces.end());
> +  AcceptedChanges += ParamReplaces.size();
> +
> +  // move the value in the init-list
> +  Replaces.insert(Replacement(
> +      SM, Initializer->getLParenLoc().getLocWithOffset(1), 0, "std::move("));
> +  Replaces.insert(Replacement(SM, Initializer->getRParenLoc(), 0, ")"));
> +  AcceptedChanges += 2;
> +}
>
> Added: clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValueActions.h
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValueActions.h?rev=189584&view=auto
> ==============================================================================
> --- clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValueActions.h (added)
> +++ clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValueActions.h Thu Aug 29 08:42:13 2013
> @@ -0,0 +1,76 @@
> +//===-- PassByValueActions.h ------------------------------------*- C++ -*-===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +///
> +/// \file
> +/// \brief This file contains the declaration of the ASTMatcher callback for the
> +/// PassByValue transform.
> +///
> +//===----------------------------------------------------------------------===//
> +
> +#ifndef CPP11_MIGRATE_PASS_BY_VALUE_ACTIONS_H
> +#define CPP11_MIGRATE_PASS_BY_VALUE_ACTIONS_H
> +
> +#include "clang/ASTMatchers/ASTMatchFinder.h"
> +#include "clang/Tooling/Refactoring.h"
> +
> +class Transform;
> +class IncludeDirectives;
> +
> +/// \brief Callback that replaces const-ref parameters in constructors to use
> +/// pass-by-value semantic where applicable.
> +///
> +/// Modifications done by the callback:
> +/// - \#include \<utility\> is added if necessary for the definition of
> +///   \c std::move() to be available.
> +/// - The parameter type is changed from const-ref to value-type.
> +/// - In the init-list the parameter is moved.
> +///
> +/// Example:
> +/// \code
> +/// + #include <utility>
> +///
> +/// class Foo(const std::string &S) {
> +/// public:
> +///   - Foo(const std::string &S) : S(S) {}
> +///   + Foo(std::string S) : S(std::move(S)) {}
> +///
> +/// private:
> +///   std::string S;
> +/// };
> +/// \endcode
> +///
> +/// \note Since an include may be added by this matcher it's necessary to call
> +/// \c setIncludeDirectives() with an up-to-date \c IncludeDirectives. This is
> +/// typically done by overloading \c Transform::handleBeginSource().
> +class ConstructorParamReplacer
> +    : public clang::ast_matchers::MatchFinder::MatchCallback {
> +public:
> +  ConstructorParamReplacer(clang::tooling::Replacements &Replaces,
> +                           unsigned &AcceptedChanges, unsigned &RejectedChanges,
> +                           const Transform &Owner)
> +      : Replaces(Replaces), AcceptedChanges(AcceptedChanges),
> +        RejectedChanges(RejectedChanges), Owner(Owner), IncludeManager(0) {}
> +
> +  void setIncludeDirectives(IncludeDirectives *Includes) {
> +    IncludeManager = Includes;
> +  }
> +
> +private:
> +  /// \brief Entry point to the callback called when matches are made.
> +  virtual void run(const clang::ast_matchers::MatchFinder::MatchResult &Result)
> +      LLVM_OVERRIDE;
> +
> +  clang::tooling::Replacements &Replaces;
> +  unsigned &AcceptedChanges;
> +  unsigned &RejectedChanges;
> +  const Transform &Owner;
> +  IncludeDirectives *IncludeManager;
> +};
> +
> +#endif // CPP11_MIGRATE_PASS_BY_VALUE_ACTIONS_H
>
> Added: clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValueMatchers.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValueMatchers.cpp?rev=189584&view=auto
> ==============================================================================
> --- clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValueMatchers.cpp (added)
> +++ clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValueMatchers.cpp Thu Aug 29 08:42:13 2013
> @@ -0,0 +1,80 @@
> +//===-- PassByValueMatchers.cpp -------------------------------------------===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +///
> +/// \file
> +/// \brief This file contains the definitions for matcher-generating functions
> +/// and names for bound nodes found by AST matchers.
> +///
> +//===----------------------------------------------------------------------===//
> +
> +#include "PassByValueMatchers.h"
> +
> +const char *PassByValueCtorId = "Ctor";
> +const char *PassByValueParamId = "Param";
> +const char *PassByValueInitializerId = "Initializer";
> +
> +namespace clang {
> +namespace ast_matchers {
> +
> +/// \brief Matches move constructible classes.
> +///
> +/// Given
> +/// \code
> +///   // POD types are trivially move constructible
> +///   struct Foo { int a; };
> +///
> +///   struct Bar {
> +///     Bar(Bar &&) = deleted;
> +///     int a;
> +///   };
> +/// \endcode
> +/// recordDecl(isMoveConstructible())
> +///   matches "Foo".
> +AST_MATCHER(CXXRecordDecl, isMoveConstructible) {
> +  for (CXXRecordDecl::ctor_iterator I = Node.ctor_begin(), E = Node.ctor_end(); I != E; ++I) {
> +    const CXXConstructorDecl *Ctor = *I;
> +    if (Ctor->isMoveConstructor() && !Ctor->isDeleted())
> +      return true;
> +  }
> +  return false;
> +}
> +
> +/// \brief Matches non-deleted copy constructors.
> +///
> +/// Given
> +/// \code
> +///   struct Foo { Foo(const Foo &) = default; };
> +///   struct Bar { Bar(const Bar &) = deleted; };
> +/// \endcode
> +/// constructorDecl(isNonDeletedCopyConstructor())
> +///   matches "Foo(const Foo &)".
> +AST_MATCHER(CXXConstructorDecl, isNonDeletedCopyConstructor) {
> +  return Node.isCopyConstructor() && !Node.isDeleted();
> +}
> +} // namespace ast_matchers
> +} // namespace clang
> +
> +using namespace clang;
> +using namespace clang::ast_matchers;
> +
> +DeclarationMatcher makePassByValueCtorParamMatcher() {
> +  return constructorDecl(
> +      forEachConstructorInitializer(ctorInitializer(
> +          // Clang builds a CXXConstructExpr only when it knowns which
> +          // constructor will be called. In dependent contexts a ParenListExpr
> +          // is generated instead of a CXXConstructExpr, filtering out templates
> +          // automatically for us.
> +          withInitializer(constructExpr(
> +              has(declRefExpr(to(parmVarDecl().bind(PassByValueParamId)))),
> +              hasDeclaration(constructorDecl(
> +                  isNonDeletedCopyConstructor(),
> +                  hasDeclContext(recordDecl(isMoveConstructible())))))))
> +                                        .bind(PassByValueInitializerId)))
> +      .bind(PassByValueCtorId);
> +}
>
> Added: clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValueMatchers.h
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValueMatchers.h?rev=189584&view=auto
> ==============================================================================
> --- clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValueMatchers.h (added)
> +++ clang-tools-extra/trunk/cpp11-migrate/PassByValue/PassByValueMatchers.h Thu Aug 29 08:42:13 2013
> @@ -0,0 +1,44 @@
> +//===-- PassByValueMatchers.h -----------------------------------*- C++ -*-===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +///
> +/// \file
> +/// \brief This file contains the declarations for matcher-generating functions
> +/// and names for bound nodes found by AST matchers.
> +///
> +//===----------------------------------------------------------------------===//
> +
> +#ifndef CPP11_MIGRATE_REPLACE_AUTO_PTR_MATCHERS_H
> +#define CPP11_MIGRATE_REPLACE_AUTO_PTR_MATCHERS_H
> +
> +#include "clang/ASTMatchers/ASTMatchers.h"
> +
> +/// \name Names to bind with matched expressions
> +/// @{
> +extern const char *PassByValueCtorId;
> +extern const char *PassByValueParamId;
> +extern const char *PassByValueInitializerId;
> +/// @}
> +
> +/// \brief Creates a matcher that finds class field initializations that can
> +/// benefit from using the move constructor.
> +///
> +/// \code
> +///   class A {
> +///   public:
> +///    A(const std::string &S) : S(S) {}
> +///    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ PassByValueCtorId
> +///      ~~~~~~~~~~~~~~~~~~~~ PassByValueParamId
> +///                                ~ PassByValueInitializerId
> +///   private:
> +///    std::string S;
> +///  };
> +/// \endcode
> +clang::ast_matchers::DeclarationMatcher makePassByValueCtorParamMatcher();
> +
> +#endif // CPP11_MIGRATE_REPLACE_AUTO_PTR_MATCHERS_H
>
> Modified: clang-tools-extra/trunk/cpp11-migrate/tool/CMakeLists.txt
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/tool/CMakeLists.txt?rev=189584&r1=189583&r2=189584&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/cpp11-migrate/tool/CMakeLists.txt (original)
> +++ clang-tools-extra/trunk/cpp11-migrate/tool/CMakeLists.txt Thu Aug 29 08:42:13 2013
> @@ -19,6 +19,9 @@ list(APPEND Cpp11MigrateSources ${UseAut
>  file(GLOB_RECURSE AddOverrideSources "../AddOverride/*.cpp")
>  list(APPEND Cpp11MigrateSources ${AddOverrideSources})
>
> +file(GLOB_RECURSE PassByValueSources "../PassByValue/*.cpp")
> +list(APPEND Cpp11MigrateSources ${PassByValueSources})
> +
>  file(GLOB_RECURSE ReplaceAutoPtrSources "../ReplaceAutoPtr/*.cpp")
>  list(APPEND Cpp11MigrateSources ${ReplaceAutoPtrSources})
>
>
> Modified: clang-tools-extra/trunk/cpp11-migrate/tool/Cpp11Migrate.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/tool/Cpp11Migrate.cpp?rev=189584&r1=189583&r2=189584&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/cpp11-migrate/tool/Cpp11Migrate.cpp (original)
> +++ clang-tools-extra/trunk/cpp11-migrate/tool/Cpp11Migrate.cpp Thu Aug 29 08:42:13 2013
> @@ -414,6 +414,7 @@ int main(int argc, const char **argv) {
>  // These anchors are used to force the linker to link the transforms
>  extern volatile int AddOverrideTransformAnchorSource;
>  extern volatile int LoopConvertTransformAnchorSource;
> +extern volatile int PassByValueTransformAnchorSource;
>  extern volatile int ReplaceAutoPtrTransformAnchorSource;
>  extern volatile int UseAutoTransformAnchorSource;
>  extern volatile int UseNullptrTransformAnchorSource;
> @@ -421,6 +422,7 @@ extern volatile int UseNullptrTransformA
>  static int TransformsAnchorsDestination[] = {
>    AddOverrideTransformAnchorSource,
>    LoopConvertTransformAnchorSource,
> +  PassByValueTransformAnchorSource,
>    ReplaceAutoPtrTransformAnchorSource,
>    UseAutoTransformAnchorSource,
>    UseNullptrTransformAnchorSource
>
> Modified: clang-tools-extra/trunk/cpp11-migrate/tool/Makefile
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/tool/Makefile?rev=189584&r1=189583&r2=189584&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/cpp11-migrate/tool/Makefile (original)
> +++ clang-tools-extra/trunk/cpp11-migrate/tool/Makefile Thu Aug 29 08:42:13 2013
> @@ -30,6 +30,8 @@ SOURCES += $(addprefix ../UseAuto/,$(not
>  BUILT_SOURCES += $(ObjDir)/../UseAuto/.objdir
>  SOURCES += $(addprefix ../AddOverride/,$(notdir $(wildcard $(PROJ_SRC_DIR)/../AddOverride/*.cpp)))
>  BUILT_SOURCES += $(ObjDir)/../AddOverride/.objdir
> +SOURCES += $(addprefix ../PassByValue/,$(notdir $(wildcard $(PROJ_SRC_DIR)/../PassByValue/*.cpp)))
> +BUILT_SOURCES += $(ObjDir)/../PassByValue/.objdir
>  SOURCES += $(addprefix ../ReplaceAutoPtr/,$(notdir $(wildcard $(PROJ_SRC_DIR)/../ReplaceAutoPtr/*.cpp)))
>  BUILT_SOURCES += $(ObjDir)/../ReplaceAutoPtr/.objdir
>
>
> Modified: clang-tools-extra/trunk/docs/MigratorUsage.rst
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/MigratorUsage.rst?rev=189584&r1=189583&r2=189584&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/docs/MigratorUsage.rst (original)
> +++ clang-tools-extra/trunk/docs/MigratorUsage.rst Thu Aug 29 08:42:13 2013
> @@ -141,6 +141,7 @@ General Command Line Options
>    ===============  =====  ===  ====  ====
>    AddOverride (1)  3.0    4.7  14    8
>    LoopConvert      3.0    4.6  13    11
> +  PassByValue      3.0    4.6  13    11
>    ReplaceAutoPtr   3.0    4.6  13    11
>    UseAuto          2.9    4.4  12    10
>    UseNullptr       3.0    4.6  12.1  10
> @@ -226,6 +227,12 @@ Transform-Specific Command Line Options
>    projects that use such macros to maintain build compatibility with non-C++11
>    code.
>
> +.. option:: -pass-by-value
> +
> +  Replace const-reference parameters by values in situations where it can be
> +  beneficial.
> +  See :doc:`PassByValueTransform`.
> +
>  .. option:: -replace-auto_ptr
>
>    Replace ``std::auto_ptr`` (deprecated in C++11) by ``std::unique_ptr`` and
>
> Added: clang-tools-extra/trunk/docs/PassByValueTransform.rst
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/PassByValueTransform.rst?rev=189584&view=auto
> ==============================================================================
> --- clang-tools-extra/trunk/docs/PassByValueTransform.rst (added)
> +++ clang-tools-extra/trunk/docs/PassByValueTransform.rst Thu Aug 29 08:42:13 2013
> @@ -0,0 +1,165 @@
> +.. index:: Pass-By-Value Transform
> +
> +=======================
> +Pass-By-Value Transform
> +=======================
> +
> +The Pass-By-Value Transform makes use of the pass-by-value idiom when possible.
> +
> +With move semantics added to the language and the standard library updated with
> +move constructors added for many types it is now interesting to take an argument
> +directly by value, instead of by const-reference, and then copy. This
> +transformation allows the compiler to take care of choosing the best way to
> +construct the copy.
> +
> +The transformation is usually beneficial when the calling code passes an
> +*rvalue* and assumes the move construction is a cheap operation. This short
> +example illustrates how the construction of the value happens:
> +
> +  .. code-block:: c++
> +
> +    void foo(std::string s);
> +    std::string get_str();
> +
> +    void f(const std::string &str) {
> +      foo(str);       // lvalue  -> copy construction
> +      foo(get_str()); // prvalue -> move construction
> +    }
> +
> +.. note::
> +
> +   Currently only constructors are transformed to make use of pass-by-value.
> +   Contributions that handle other situations are welcome!
> +
> +
> +Pass-by-value in constructors
> +-----------------------------
> +
> +Replaces the uses of const-references constructor parameters that are copied
> +into class fields. The parameter is then moved with `std::move()`.
> +
> +Since `std::move()` is a library function declared in `<utility>` it may be
> +necessary to add this include. The transform will add the include directive when
> +necessary.
> +
> +Example::
> +
> +  $ cpp11-migrate -pass-by-value ctor.cpp
> +
> +**ctor.cpp**
> +
> +  .. code-block:: c++
> +
> +     #include <string>
> +
> +     class Foo {
> +     public:
> +    -  Foo(const std::string &Copied, const std::string &ReadOnly)
> +    -    : Copied(Copied), ReadOnly(ReadOnly)
> +    +  Foo(std::string Copied, const std::string &ReadOnly)
> +    +    : Copied(std::move(Copied)), ReadOnly(ReadOnly)
> +       {}
> +
> +     private:
> +       std::string Copied;
> +       const std::string &ReadOnly;
> +     };
> +
> +     std::string get_cwd();
> +
> +     void f(const std::string &Path) {
> +       // The parameter corresponding to 'get_cwd()' is move-constructed. By
> +       // using pass-by-value in the Foo constructor we managed to avoid a
> +       // copy-construction.
> +       Foo foo(get_cwd(), Path);
> +     }
> +
> +
> +If the parameter is used more than once no transformation is performed since
> +moved objects have an undefined state. It means the following code will be left
> +untouched:
> +
> +.. code-block:: c++
> +
> +  #include <string>
> +
> +  void pass(const std::string &S);
> +
> +  struct Foo {
> +    Foo(const std::string &S) : Str(S) {
> +      pass(S);
> +    }
> +
> +    std::string Str;
> +  };
> +
> +
> +Risk
> +^^^^
> +
> +This modification is considered **reasonably safe** (see :option:`-risk`
> +option).
> +
> +A situation where the generated code can be wrong is when the object referenced
> +is modified before the assignment in the init-list through a "hidden" reference.
> +
> +Example:
> +
> +.. code-block:: c++
> +
> +   std::string s("foo");
> +
> +   struct Base {
> +     Base() {
> +       s = "bar";
> +     }
> +   };
> +
> +   struct Derived : Base {
> +  -  Derived(const std::string &S) : Field(S)
> +  +  Derived(std::string S) : Field(std::move(S))
> +     { }
> +
> +     std::string Field;
> +   };
> +
> +   void f() {
> +  -  Derived d(s); // d.Field holds "bar"
> +  +  Derived d(s); // d.Field holds "foo"
> +   }
> +
> +
> +Note about delayed template parsing
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +When delayed template parsing is enabled, constructors part of templated
> +contexts; templated constructors, constructors in class templates, constructors
> +of inner classes of template classes, etc., are not transformed. Delayed
> +template parsing is enabled by default on Windows as a Microsoft extension:
> +`Clang Compiler User’s Manual - Microsoft extensions`_.
> +
> +Delayed template parsing can be enabled using the `-fdelayed-template-parsing`
> +flag and disabled using `-fno-delayed-template-parsing`.
> +
> +Example:
> +
> +.. code-block:: c++
> +
> +   template <typename T> class C {
> +     std::string S;
> +
> +   public:
> + =  // using -fdelayed-template-parsing (default on Windows)
> + =  C(const std::string &S) : S(S) {}
> +
> + +  // using -fno-delayed-template-parsing (default on non-Windows systems)
> + +  C(std::string S) : S(std::move(S)) {}
> +   };
> +
> +.. _Clang Compiler User’s Manual - Microsoft extensions: http://clang.llvm.org/docs/UsersManual.html#microsoft-extensions
> +
> +.. seealso::
> +
> +  For more information about the pass-by-value idiom, read: `Want Speed? Pass by Value`_.
> +
> +  .. _Want Speed? Pass by Value: http://cpp-next.com/archive/2009/08/want-speed-pass-by-value/
>
> Modified: clang-tools-extra/trunk/docs/cpp11-migrate.rst
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/cpp11-migrate.rst?rev=189584&r1=189583&r2=189584&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/docs/cpp11-migrate.rst (original)
> +++ clang-tools-extra/trunk/docs/cpp11-migrate.rst Thu Aug 29 08:42:13 2013
> @@ -11,6 +11,7 @@ C++11 Migrator User's Manual
>     UseNullptrTransform
>     LoopConvertTransform
>     AddOverrideTransform
> +   PassByValueTransform
>     ReplaceAutoPtrTransform
>     MigratorUsage
>
> @@ -116,4 +117,6 @@ independently enabled. The transforms cu
>
>  * :doc:`AddOverrideTransform`
>
> +* :doc:`PassByValueTransform`
> +
>  * :doc:`ReplaceAutoPtrTransform`
>
> Added: clang-tools-extra/trunk/test/cpp11-migrate/PassByValue/basic.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/cpp11-migrate/PassByValue/basic.cpp?rev=189584&view=auto
> ==============================================================================
> --- clang-tools-extra/trunk/test/cpp11-migrate/PassByValue/basic.cpp (added)
> +++ clang-tools-extra/trunk/test/cpp11-migrate/PassByValue/basic.cpp Thu Aug 29 08:42:13 2013
> @@ -0,0 +1,168 @@
> +// Since -fdelayed-template-parsing is enabled by default on Windows (as a
> +// Microsoft extension), -fno-delayed-template-parsing is used for the tests in
> +// order to have the same behavior on all systems.
> +//
> +// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
> +// RUN: cpp11-migrate -pass-by-value %t.cpp -- -std=c++11 -fno-delayed-template-parsing -I %S
> +// RUN: FileCheck -input-file=%t.cpp %s
> +//
> +// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
> +// RUN: cpp11-migrate -pass-by-value %t.cpp -- -std=c++11 -fno-delayed-template-parsing -I %S
> +// RUN: FileCheck -check-prefix=SAFE_RISK -input-file=%t.cpp %s
> +
> +#include "basic.h"
> +// CHECK: #include <utility>
> +
> +// Test that when the class declaration can't be modified we won't modify the
> +// definition either.
> +UnmodifiableClass::UnmodifiableClass(const Movable &M) : M(M) {}
> +// CHECK: UnmodifiableClass::UnmodifiableClass(const Movable &M) : M(M) {}
> +
> +struct A {
> +  A(const Movable &M) : M(M) {}
> +  // CHECK: A(Movable M) : M(std::move(M)) {}
> +  // SAFE_RISK: A(const Movable &M) : M(M) {}
> +  Movable M;
> +};
> +
> +// Test that we aren't modifying other things than a parameter
> +Movable GlobalObj;
> +struct B {
> +  B(const Movable &M) : M(GlobalObj) {}
> +  // CHECK: B(const Movable &M) : M(GlobalObj) {}
> +  Movable M;
> +};
> +
> +// Test that a parameter with more than one reference to it won't be changed.
> +struct C {
> +  // Tests extra-reference in body
> +  C(const Movable &M) : M(M) { this->i = M.a; }
> +  // CHECK: C(const Movable &M) : M(M) { this->i = M.a; }
> +
> +  // Tests extra-reference in init-list
> +  C(const Movable &M, int) : M(M), i(M.a) {}
> +  // CHECK: C(const Movable &M, int) : M(M), i(M.a) {}
> +  Movable M;
> +  int i;
> +};
> +
> +// Test that both declaration and definition are updated
> +struct D {
> +  D(const Movable &M);
> +  // CHECK: D(Movable M);
> +  Movable M;
> +};
> +D::D(const Movable &M) : M(M) {}
> +// CHECK: D::D(Movable M) : M(std::move(M)) {}
> +
> +// Test with default parameter
> +struct E {
> +  E(const Movable &M = Movable()) : M(M) {}
> +  // CHECK: E(Movable M = Movable()) : M(std::move(M)) {}
> +  Movable M;
> +};
> +
> +// Test with object that can't be moved
> +struct F {
> +  F(const NotMovable &NM) : NM(NM) {}
> +  // CHECK: F(const NotMovable &NM) : NM(NM) {}
> +  NotMovable NM;
> +};
> +
> +// Test unnamed parameter in declaration
> +struct G {
> +  G(const Movable &);
> +  // CHECK: G(Movable );
> +  Movable M;
> +};
> +G::G(const Movable &M) : M(M) {}
> +// CHECK: G::G(Movable M) : M(std::move(M)) {}
> +
> +// Test parameter with and without qualifier
> +namespace ns_H {
> +typedef ::Movable HMovable;
> +}
> +struct H {
> +  H(const ns_H::HMovable &M);
> +  // CHECK: H(ns_H::HMovable M);
> +  ns_H::HMovable M;
> +};
> +using namespace ns_H;
> +H::H(const HMovable &M) : M(M) {}
> +// CHECK: H(HMovable M) : M(std::move(M)) {}
> +
> +// Try messing up with macros
> +#define MOVABLE_PARAM(Name) const Movable & Name
> +// CHECK: #define MOVABLE_PARAM(Name) const Movable & Name
> +struct I {
> +  I(MOVABLE_PARAM(M)) : M(M) {}
> +  // CHECK: I(MOVABLE_PARAM(M)) : M(M) {}
> +  Movable M;
> +};
> +#undef MOVABLE_PARAM
> +
> +// Test that templates aren't modified
> +template <typename T> struct J {
> +  J(const T &M) : M(M) {}
> +  // CHECK: J(const T &M) : M(M) {}
> +  T M;
> +};
> +J<Movable> j1(Movable());
> +J<NotMovable> j2(NotMovable());
> +
> +struct K_Movable {
> +  K_Movable() = default;
> +  K_Movable(const K_Movable &) = default;
> +  K_Movable(K_Movable &&o) { dummy = o.dummy; }
> +  int dummy;
> +};
> +
> +// Test with movable type with an user defined move constructor.
> +struct K {
> +  K(const K_Movable &M) : M(M) {}
> +  // CHECK: K(K_Movable M) : M(std::move(M)) {}
> +  K_Movable M;
> +};
> +
> +template <typename T> struct L {
> +  L(const Movable &M) : M(M) {}
> +  // CHECK: L(Movable M) : M(std::move(M)) {}
> +  Movable M;
> +};
> +L<int> l(Movable());
> +
> +// Test with a non-instantiated template class
> +template <typename T> struct N {
> +  N(const Movable &M) : M(M) {}
> +  // CHECK: N(Movable M) : M(std::move(M)) {}
> +
> +  Movable M;
> +  T A;
> +};
> +
> +// Test with value parameter
> +struct O {
> +  O(Movable M) : M(M) {}
> +  // CHECK: O(Movable M) : M(std::move(M)) {}
> +  Movable M;
> +};
> +
> +// Test with a const-value parameter
> +struct P {
> +  P(const Movable M) : M(M) {}
> +  // CHECK: P(Movable M) : M(std::move(M)) {}
> +  Movable M;
> +};
> +
> +// Test with multiples parameters where some need to be changed and some don't
> +// need to.
> +struct Q {
> +  Q(const Movable &A, const Movable &B, const Movable &C, double D)
> +      : A(A), B(B), C(C), D(D) {}
> +  // CHECK:      Q(const Movable &A, Movable B, Movable C, double D)
> +  // CHECK-NEXT:     : A(A), B(std::move(B)), C(std::move(C)), D(D) {}
> +  const Movable &A;
> +  Movable B;
> +  Movable C;
> +  double D;
> +};
>
> Added: clang-tools-extra/trunk/test/cpp11-migrate/PassByValue/basic.h
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/cpp11-migrate/PassByValue/basic.h?rev=189584&view=auto
> ==============================================================================
> --- clang-tools-extra/trunk/test/cpp11-migrate/PassByValue/basic.h (added)
> +++ clang-tools-extra/trunk/test/cpp11-migrate/PassByValue/basic.h Thu Aug 29 08:42:13 2013
> @@ -0,0 +1,23 @@
> +#ifndef BASIC_H
> +#define BASIC_H
> +
> +// POD types are trivially move constructible
> +struct Movable {
> +  int a, b, c;
> +};
> +
> +struct NotMovable {
> +  NotMovable() = default;
> +  NotMovable(const NotMovable &) = default;
> +  NotMovable(NotMovable &&) = delete;
> +  int a, b, c;
> +};
> +
> +// The test runs the migrator without header modifications enabled for this
> +// header making the constructor parameter M unmodifiable.
> +struct UnmodifiableClass {
> +  UnmodifiableClass(const Movable &M);
> +  Movable M;
> +};
> +
> +#endif // BASIC_H
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits




More information about the cfe-commits mailing list