[LibTooling] Fix dangling references in RangeSelector.

[ymandel]

Summary

RangeSelector had a number of cases of capturing a StringRef in a lambda, which
lead to dangling references. This change converts all uses in the API of
StringRef to std::string to avoid this problem. std::string in the API is
a reasonable choice, because the combinators are always storing the string
beyond the life of the combinator construction.

[ilya-biryukov]

LGTM

cfe/trunk/include/clang/Tooling/Refactoring/RangeSelector.h

 ///
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLING_REFACTOR_RANGE_SELECTOR_H_
 #define LLVM_CLANG_TOOLING_REFACTOR_RANGE_SELECTOR_H_
 
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/SourceLocation.h"
-#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include <functional>
+#include <string>
 
 namespace clang {
 namespace tooling {
 using RangeSelector = std::function<Expected<CharSourceRange>(
     const ast_matchers::MatchFinder::MatchResult &)>;
 
 inline RangeSelector charRange(CharSourceRange R) {
   return [R](const ast_matchers::MatchFinder::MatchResult &)
              -> Expected<CharSourceRange> { return R; };
 }
 
 /// Selects from the start of \p Begin and to the end of \p End.
 RangeSelector range(RangeSelector Begin, RangeSelector End);
 
 /// Convenience version of \c range where end-points are bound nodes.
-RangeSelector range(StringRef BeginID, StringRef EndID);
+RangeSelector range(std::string BeginID, std::string EndID);
 
 /// Selects a node, including trailing semicolon (for non-expression
 /// statements). \p ID is the node's binding in the match result.
-RangeSelector node(StringRef ID);
+RangeSelector node(std::string ID);
 
 /// Selects a node, including trailing semicolon (always). Useful for selecting
 /// expression statements. \p ID is the node's binding in the match result.
-RangeSelector statement(StringRef ID);
+RangeSelector statement(std::string ID);
 
 /// Given a \c MemberExpr, selects the member token. \p ID is the node's
 /// binding in the match result.
-RangeSelector member(StringRef ID);
+RangeSelector member(std::string ID);
 
 /// Given a node with a "name", (like \c NamedDecl, \c DeclRefExpr or \c
 /// CxxCtorInitializer) selects the name's token.  Only selects the final
 /// identifier of a qualified name, but not any qualifiers or template
 /// arguments.  For example, for `::foo::bar::baz` and `::foo::bar::baz<int>`,
 /// it selects only `baz`.
 ///
 /// \param ID is the node's binding in the match result.
-RangeSelector name(StringRef ID);
+RangeSelector name(std::string ID);
 
 // Given a \c CallExpr (bound to \p ID), selects the arguments' source text (all
 // source between the call's parentheses).
-RangeSelector callArgs(StringRef ID);
+RangeSelector callArgs(std::string ID);
 
 // Given a \c CompoundStmt (bound to \p ID), selects the source of the
 // statements (all source between the braces).
-RangeSelector statements(StringRef ID);
+RangeSelector statements(std::string ID);
 
 // Given a \c InitListExpr (bound to \p ID), selects the range of the elements
 // (all source between the braces).
-RangeSelector initListElements(StringRef ID);
+RangeSelector initListElements(std::string ID);
 
 /// Selects the range from which `S` was expanded (possibly along with other
 /// source), if `S` is an expansion, and `S` itself, otherwise.  Corresponds to
 /// `SourceManager::getExpansionRange`.
 RangeSelector expansion(RangeSelector S);
 } // namespace tooling
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLING_REFACTOR_RANGE_SELECTOR_H_

cfe/trunk/lib/Tooling/Refactoring/RangeSelector.cpp

 
 static SourceLocation findOpenParen(const CallExpr &E, const SourceManager &SM,
                                     const LangOptions &LangOpts) {
   SourceLocation EndLoc =
       E.getNumArgs() == 0 ? E.getRParenLoc() : E.getArg(0)->getBeginLoc();
   return findPreviousTokenKind(EndLoc, SM, LangOpts, tok::TokenKind::l_paren);
 }
 
-RangeSelector tooling::node(StringRef ID) {
+RangeSelector tooling::node(std::string ID) {
   return [ID](const MatchResult &Result) -> Expected<CharSourceRange> {
     Expected<DynTypedNode> Node = getNode(Result.Nodes, ID);
     if (!Node)
       return Node.takeError();
     return Node->get<Stmt>() != nullptr && Node->get<Expr>() == nullptr
                ? getExtendedRange(*Node, tok::TokenKind::semi, *Result.Context)
                : CharSourceRange::getTokenRange(Node->getSourceRange());
   };
 }
 
-RangeSelector tooling::statement(StringRef ID) {
+RangeSelector tooling::statement(std::string ID) {
   return [ID](const MatchResult &Result) -> Expected<CharSourceRange> {
     Expected<DynTypedNode> Node = getNode(Result.Nodes, ID);
     if (!Node)
       return Node.takeError();
     return getExtendedRange(*Node, tok::TokenKind::semi, *Result.Context);
   };
 }
 
return [Begin, End](const MatchResult &Result) -> Expected<CharSourceRange> {
     // that EndRange is a token range.
     if (Result.SourceManager->isBeforeInTranslationUnit(E, B)) {
       return invalidArgumentError("Bad range: out of order");
     }
     return CharSourceRange(SourceRange(B, E), EndRange->isTokenRange());
   };
 }
 
-RangeSelector tooling::range(StringRef BeginID, StringRef EndID) {
-  return tooling::range(node(BeginID), node(EndID));
+RangeSelector tooling::range(std::string BeginID, std::string EndID) {
+  return tooling::range(node(std::move(BeginID)), node(std::move(EndID)));
 }
 
-RangeSelector tooling::member(StringRef ID) {
+RangeSelector tooling::member(std::string ID) {
   return [ID](const MatchResult &Result) -> Expected<CharSourceRange> {
     Expected<DynTypedNode> Node = getNode(Result.Nodes, ID);
     if (!Node)
       return Node.takeError();
     if (auto *M = Node->get<clang::MemberExpr>())
       return CharSourceRange::getTokenRange(
           M->getMemberNameInfo().getSourceRange());
     return typeError(ID, Node->getNodeKind(), "MemberExpr");
   };
 }
 
-RangeSelector tooling::name(StringRef ID) {
+RangeSelector tooling::name(std::string ID) {
   return [ID](const MatchResult &Result) -> Expected<CharSourceRange> {
     Expected<DynTypedNode> N = getNode(Result.Nodes, ID);
     if (!N)
       return N.takeError();
     auto &Node = *N;
     if (const auto *D = Node.get<NamedDecl>()) {
       if (!D->getDeclName().isIdentifier())
         return missingPropertyError(ID, "name", "identifier");

 // Creates a selector from a range-selection function \p Func, which selects a
 // range that is relative to a bound node id.  \c T is the node type expected by
 // \p Func.
 template <typename T, CharSourceRange (*Func)(const MatchResult &, const T &)>
 class RelativeSelector {
   std::string ID;
 
 public:
-  RelativeSelector(StringRef ID) : ID(ID) {}
+  RelativeSelector(std::string ID) : ID(std::move(ID)) {}
 
   Expected<CharSourceRange> operator()(const MatchResult &Result) {
     Expected<DynTypedNode> N = getNode(Result.Nodes, ID);
     if (!N)
       return N.takeError();
     if (const auto *Arg = N->get<T>())
       return Func(Result, *Arg);
     return typeError(ID, N->getNodeKind());

 // Returns the range of the statements (all source between the braces).
 CharSourceRange getStatementsRange(const MatchResult &,
                                    const CompoundStmt &CS) {
   return CharSourceRange::getCharRange(CS.getLBracLoc().getLocWithOffset(1),
                                        CS.getRBracLoc());
 }
 } // namespace
 
-RangeSelector tooling::statements(StringRef ID) {
-  return RelativeSelector<CompoundStmt, getStatementsRange>(ID);
+RangeSelector tooling::statements(std::string ID) {
+  return RelativeSelector<CompoundStmt, getStatementsRange>(std::move(ID));
 }
 
 namespace {
 // Returns the range of the source between the call's parentheses.
 CharSourceRange getCallArgumentsRange(const MatchResult &Result,
                                       const CallExpr &CE) {
   return CharSourceRange::getCharRange(
       findOpenParen(CE, *Result.SourceManager, Result.Context->getLangOpts())
           .getLocWithOffset(1),
       CE.getRParenLoc());
 }
 } // namespace
 
-RangeSelector tooling::callArgs(StringRef ID) {
-  return RelativeSelector<CallExpr, getCallArgumentsRange>(ID);
+RangeSelector tooling::callArgs(std::string ID) {
+  return RelativeSelector<CallExpr, getCallArgumentsRange>(std::move(ID));
 }
 
 namespace {
 // Returns the range of the elements of the initializer list. Includes all
 // source between the braces.
 CharSourceRange getElementsRange(const MatchResult &,
                                  const InitListExpr &E) {
   return CharSourceRange::getCharRange(E.getLBraceLoc().getLocWithOffset(1),
                                        E.getRBraceLoc());
 }
 } // namespace
 
-RangeSelector tooling::initListElements(StringRef ID) {
-  return RelativeSelector<InitListExpr, getElementsRange>(ID);
+RangeSelector tooling::initListElements(std::string ID) {
+  return RelativeSelector<InitListExpr, getElementsRange>(std::move(ID));
 }
 
 RangeSelector tooling::expansion(RangeSelector S) {
   return [S](const MatchResult &Result) -> Expected<CharSourceRange> {
     Expected<CharSourceRange> SRange = S(Result);
     if (!SRange)
       return SRange.takeError();
     return Result.SourceManager->getExpansionRange(*SRange);
   };
 }