[PATCH] D61015: [LibTooing] Change Transformer's TextGenerator to a partial function.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added a reviewer: ilya-biryukov.
Herald added a project: clang.

Changes the signature of the std::function to return an Expected<std::string>
instead of std::string to allow for (non-fatal) failures.  Previously, we
expected that any failures would be expressed with assertions. However, that's
unfriendly to running the code in servers or other places that don't want their
library calls to crash the program.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61015

Files:
  clang/include/clang/Tooling/Refactoring/Transformer.h
  clang/lib/Tooling/Refactoring/Transformer.cpp


Index: clang/lib/Tooling/Refactoring/Transformer.cpp
===================================================================
--- clang/lib/Tooling/Refactoring/Transformer.cpp
+++ clang/lib/Tooling/Refactoring/Transformer.cpp
@@ -157,12 +157,16 @@
         Edit.Target, It->second, Edit.Kind, Edit.Part, *Result.Context);
     if (auto Err = RangeOrErr.takeError())
       return std::move(Err);
-    Transformation T;
-    T.Range = *RangeOrErr;
-    if (T.Range.isInvalid() ||
-        isOriginMacroBody(*Result.SourceManager, T.Range.getBegin()))
+    auto &Range = *RangeOrErr;
+    if (Range.isInvalid() ||
+        isOriginMacroBody(*Result.SourceManager, Range.getBegin()))
       return SmallVector<Transformation, 0>();
-    T.Replacement = Edit.Replacement(Result);
+    auto ReplacementOrErr = Edit.Replacement(Result);
+    if (auto Err = ReplacementOrErr.takeError())
+      return std::move(Err);
+    Transformation T;
+    T.Range = Range;
+    T.Replacement = std::move(*ReplacementOrErr);
     Transformations.push_back(std::move(T));
   }
   return Transformations;
Index: clang/include/clang/Tooling/Refactoring/Transformer.h
===================================================================
--- clang/include/clang/Tooling/Refactoring/Transformer.h
+++ clang/include/clang/Tooling/Refactoring/Transformer.h
@@ -44,12 +44,18 @@
   Name,
 };
 
-using TextGenerator =
-    std::function<std::string(const ast_matchers::MatchFinder::MatchResult &)>;
-
-/// Wraps a string as a TextGenerator.
+// \c TextGenerator may fail because it processes dynamically-bound match
+// results. For example, a typo in the name of a bound node can lead to a
+// failure in the string generation code. We prefer to return \c Expected rather
+// than assert on such failures to allow this code to be run in a variety of
+// settings (including servers).
+using TextGenerator = std::function<Expected<std::string>(
+    const ast_matchers::MatchFinder::MatchResult &)>;
+
+/// Wraps a string as a (trivially successful) TextGenerator.
 inline TextGenerator text(std::string M) {
-  return [M](const ast_matchers::MatchFinder::MatchResult &) { return M; };
+  return [M](const ast_matchers::MatchFinder::MatchResult &)
+             -> Expected<std::string> { return M; };
 }
 
 // Description of a source-code edit, expressed in terms of an AST node.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D61015.196242.patch
Type: text/x-patch
Size: 2346 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190423/adb3b583/attachment.bin>