diff --git a/clang-tools-extra/cpp11-migrate/Core/Transform.h b/clang-tools-extra/cpp11-migrate/Core/Transform.h index bb8c116726dd..43082111d544 100644 --- a/clang-tools-extra/cpp11-migrate/Core/Transform.h +++ b/clang-tools-extra/cpp11-migrate/Core/Transform.h @@ -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 diff --git a/clang-tools-extra/cpp11-migrate/PassByValue/PassByValue.cpp b/clang-tools-extra/cpp11-migrate/PassByValue/PassByValue.cpp new file mode 100644 index 000000000000..968fb5c8fff2 --- /dev/null +++ b/clang-tools-extra/cpp11-migrate/PassByValue/PassByValue.cpp @@ -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 &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 +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; diff --git a/clang-tools-extra/cpp11-migrate/PassByValue/PassByValue.h b/clang-tools-extra/cpp11-migrate/PassByValue/PassByValue.h new file mode 100644 index 000000000000..009495262546 --- /dev/null +++ b/clang-tools-extra/cpp11-migrate/PassByValue/PassByValue.h @@ -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 +/// +/// class A { +/// std::string S; +/// public: +/// A(const std::string &S) : S(S) {} +/// }; +/// \endcode +/// the code is transformed to: +/// \code +/// #include +/// +/// 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 &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 IncludeManager; + ConstructorParamReplacer *Replacer; +}; + +#endif // CPP11_MIGRATE_PASS_BY_VALUE_H diff --git a/clang-tools-extra/cpp11-migrate/PassByValue/PassByValueActions.cpp b/clang-tools-extra/cpp11-migrate/PassByValue/PassByValueActions.cpp new file mode 100644 index 000000000000..612850a70dbf --- /dev/null +++ b/clang-tools-extra/cpp11-migrate/PassByValue/PassByValueActions.cpp @@ -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 { + friend class RecursiveASTVisitor; + +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(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(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 &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(PassByValueCtorId); + const ParmVarDecl *ParamDecl = + Result.Nodes.getNodeAs(PassByValueParamId); + const CXXCtorInitializer *Initializer = + Result.Nodes.getNodeAs(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 AllParamDecls; + collectParamDecls(Ctor, ParamDecl, AllParamDecls); + + // Generate all replacements for the params. + llvm::SmallVector 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 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; +} diff --git a/clang-tools-extra/cpp11-migrate/PassByValue/PassByValueActions.h b/clang-tools-extra/cpp11-migrate/PassByValue/PassByValueActions.h new file mode 100644 index 000000000000..5564fc54caf5 --- /dev/null +++ b/clang-tools-extra/cpp11-migrate/PassByValue/PassByValueActions.h @@ -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 \ 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 +/// +/// 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 diff --git a/clang-tools-extra/cpp11-migrate/PassByValue/PassByValueMatchers.cpp b/clang-tools-extra/cpp11-migrate/PassByValue/PassByValueMatchers.cpp new file mode 100644 index 000000000000..32095e167b76 --- /dev/null +++ b/clang-tools-extra/cpp11-migrate/PassByValue/PassByValueMatchers.cpp @@ -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); +} diff --git a/clang-tools-extra/cpp11-migrate/PassByValue/PassByValueMatchers.h b/clang-tools-extra/cpp11-migrate/PassByValue/PassByValueMatchers.h new file mode 100644 index 000000000000..8c2bade43859 --- /dev/null +++ b/clang-tools-extra/cpp11-migrate/PassByValue/PassByValueMatchers.h @@ -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 diff --git a/clang-tools-extra/cpp11-migrate/tool/CMakeLists.txt b/clang-tools-extra/cpp11-migrate/tool/CMakeLists.txt index e03fbee3424e..a40e0f3e31a0 100644 --- a/clang-tools-extra/cpp11-migrate/tool/CMakeLists.txt +++ b/clang-tools-extra/cpp11-migrate/tool/CMakeLists.txt @@ -19,6 +19,9 @@ list(APPEND Cpp11MigrateSources ${UseAutoSources}) 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}) diff --git a/clang-tools-extra/cpp11-migrate/tool/Cpp11Migrate.cpp b/clang-tools-extra/cpp11-migrate/tool/Cpp11Migrate.cpp index a6240c48e7e1..eaaec3c91deb 100644 --- a/clang-tools-extra/cpp11-migrate/tool/Cpp11Migrate.cpp +++ b/clang-tools-extra/cpp11-migrate/tool/Cpp11Migrate.cpp @@ -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 UseNullptrTransformAnchorSource; static int TransformsAnchorsDestination[] = { AddOverrideTransformAnchorSource, LoopConvertTransformAnchorSource, + PassByValueTransformAnchorSource, ReplaceAutoPtrTransformAnchorSource, UseAutoTransformAnchorSource, UseNullptrTransformAnchorSource diff --git a/clang-tools-extra/cpp11-migrate/tool/Makefile b/clang-tools-extra/cpp11-migrate/tool/Makefile index d59fa14062c2..f1780265aa9a 100644 --- a/clang-tools-extra/cpp11-migrate/tool/Makefile +++ b/clang-tools-extra/cpp11-migrate/tool/Makefile @@ -30,6 +30,8 @@ SOURCES += $(addprefix ../UseAuto/,$(notdir $(wildcard $(PROJ_SRC_DIR)/../UseAut 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 diff --git a/clang-tools-extra/docs/MigratorUsage.rst b/clang-tools-extra/docs/MigratorUsage.rst index 8822db2519f5..4063bc0554e1 100644 --- a/clang-tools-extra/docs/MigratorUsage.rst +++ b/clang-tools-extra/docs/MigratorUsage.rst @@ -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 diff --git a/clang-tools-extra/docs/PassByValueTransform.rst b/clang-tools-extra/docs/PassByValueTransform.rst new file mode 100644 index 000000000000..05c254ce8a08 --- /dev/null +++ b/clang-tools-extra/docs/PassByValueTransform.rst @@ -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 `` 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 + + 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 + + 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 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/ diff --git a/clang-tools-extra/docs/cpp11-migrate.rst b/clang-tools-extra/docs/cpp11-migrate.rst index f7f474314724..3febf7ff9332 100644 --- a/clang-tools-extra/docs/cpp11-migrate.rst +++ b/clang-tools-extra/docs/cpp11-migrate.rst @@ -11,6 +11,7 @@ C++11 Migrator User's Manual UseNullptrTransform LoopConvertTransform AddOverrideTransform + PassByValueTransform ReplaceAutoPtrTransform MigratorUsage @@ -116,4 +117,6 @@ independently enabled. The transforms currently implemented are: * :doc:`AddOverrideTransform` +* :doc:`PassByValueTransform` + * :doc:`ReplaceAutoPtrTransform` diff --git a/clang-tools-extra/test/cpp11-migrate/PassByValue/basic.cpp b/clang-tools-extra/test/cpp11-migrate/PassByValue/basic.cpp new file mode 100644 index 000000000000..ce23f047edb1 --- /dev/null +++ b/clang-tools-extra/test/cpp11-migrate/PassByValue/basic.cpp @@ -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 + +// 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 struct J { + J(const T &M) : M(M) {} + // CHECK: J(const T &M) : M(M) {} + T M; +}; +J j1(Movable()); +J 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 struct L { + L(const Movable &M) : M(M) {} + // CHECK: L(Movable M) : M(std::move(M)) {} + Movable M; +}; +L l(Movable()); + +// Test with a non-instantiated template class +template 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; +}; diff --git a/clang-tools-extra/test/cpp11-migrate/PassByValue/basic.h b/clang-tools-extra/test/cpp11-migrate/PassByValue/basic.h new file mode 100644 index 000000000000..4b551f7d908a --- /dev/null +++ b/clang-tools-extra/test/cpp11-migrate/PassByValue/basic.h @@ -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