[clang] 1fabe6e - [libTooling] Change `addInclude` to use expansion locs.

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2021-01-15T03:08:56Z
New Revision: 1fabe6e51917bcd7a1242294069c682fe6dffa45

URL: https://github.com/llvm/llvm-project/commit/1fabe6e51917bcd7a1242294069c682fe6dffa45
DIFF: https://github.com/llvm/llvm-project/commit/1fabe6e51917bcd7a1242294069c682fe6dffa45.diff

LOG: [libTooling] Change `addInclude` to use expansion locs.

This patch changes the default range used to anchor the include insertion to use
an expansion loc.  This ensures that the location is valid, when the user relies
on the default range.

Driveby: extend a FIXME for a problem that was emphasized by this change; fix some spellings.

Differential Revision: https://reviews.llvm.org/D93703

Added: 
    

Modified: 
    clang/include/clang/Tooling/Transformer/RewriteRule.h
    clang/lib/Tooling/Transformer/RewriteRule.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Tooling/Transformer/RewriteRule.h b/clang/include/clang/Tooling/Transformer/RewriteRule.h
index 355b3249d3bc..ac93db8446df 100644
--- a/clang/include/clang/Tooling/Transformer/RewriteRule.h
+++ b/clang/include/clang/Tooling/Transformer/RewriteRule.h
@@ -213,10 +213,12 @@ ASTEdit addInclude(RangeSelector Target, StringRef Header,
                    IncludeFormat Format = IncludeFormat::Quoted);
 
 /// Adds an include directive for the given header to the file associated with
-/// `RootID`.
+/// `RootID`. If `RootID` matches inside a macro expansion, will add the
+/// directive to the file in which the macro was expanded (as opposed to the
+/// file in which the macro is defined).
 inline ASTEdit addInclude(StringRef Header,
                           IncludeFormat Format = IncludeFormat::Quoted) {
-  return addInclude(node(RootID), Header, Format);
+  return addInclude(expansion(node(RootID)), Header, Format);
 }
 
 // FIXME: If `Metadata` returns an `llvm::Expected<T>` the `AnyGenerator` will
@@ -312,8 +314,8 @@ inline RewriteRule makeRule(ast_matchers::internal::DynTypedMatcher M,
 /// \code
 ///   auto R = makeRule(callExpr(callee(functionDecl(hasName("foo")))),
 ///            changeTo(cat("bar()")));
-///   AddInclude(R, "path/to/bar_header.h");
-///   AddInclude(R, "vector", IncludeFormat::Angled);
+///   addInclude(R, "path/to/bar_header.h");
+///   addInclude(R, "vector", IncludeFormat::Angled);
 /// \endcode
 void addInclude(RewriteRule &Rule, llvm::StringRef Header,
                 IncludeFormat Format = IncludeFormat::Quoted);

diff  --git a/clang/lib/Tooling/Transformer/RewriteRule.cpp b/clang/lib/Tooling/Transformer/RewriteRule.cpp
index acff1e83c4e1..93bd7e91dba7 100644
--- a/clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ b/clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -42,7 +42,12 @@ translateEdits(const MatchResult &Result, ArrayRef<ASTEdit> ASTEdits) {
     llvm::Optional<CharSourceRange> EditRange =
         tooling::getRangeForEdit(*Range, *Result.Context);
     // FIXME: let user specify whether to treat this case as an error or ignore
-    // it as is currently done.
+    // it as is currently done. This behavior is problematic in that it hides
+    // failures from bad ranges. Also, the behavior here 
diff ers from
+    // `flatten`. Here, we abort (without error), whereas flatten, if it hits an
+    // empty list, does not abort. As a result, `editList({A,B})` is not
+    // equivalent to `flatten(edit(A), edit(B))`. The former will abort if `A`
+    // produces a bad range, whereas the latter will simply ignore A.
     if (!EditRange)
       return SmallVector<Edit, 0>();
     auto Replacement = E.Replacement->eval(Result);