On Thu, Aug 29, 2013 at 6:42 AM, Guillaume Papin <[email protected]> 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 ¶m' -> '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 > [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
