ymandel created this revision. ymandel added a reviewer: tdl-g. ymandel requested review of this revision. Herald added a project: clang.
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. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D93703 Files: clang/include/clang/Tooling/Transformer/RewriteRule.h clang/lib/Tooling/Transformer/RewriteRule.cpp Index: clang/lib/Tooling/Transformer/RewriteRule.cpp =================================================================== --- clang/lib/Tooling/Transformer/RewriteRule.cpp +++ clang/lib/Tooling/Transformer/RewriteRule.cpp @@ -42,7 +42,12 @@ 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 differs 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); Index: clang/include/clang/Tooling/Transformer/RewriteRule.h =================================================================== --- clang/include/clang/Tooling/Transformer/RewriteRule.h +++ clang/include/clang/Tooling/Transformer/RewriteRule.h @@ -213,10 +213,12 @@ 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 @@ /// \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);
Index: clang/lib/Tooling/Transformer/RewriteRule.cpp =================================================================== --- clang/lib/Tooling/Transformer/RewriteRule.cpp +++ clang/lib/Tooling/Transformer/RewriteRule.cpp @@ -42,7 +42,12 @@ 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 differs 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); Index: clang/include/clang/Tooling/Transformer/RewriteRule.h =================================================================== --- clang/include/clang/Tooling/Transformer/RewriteRule.h +++ clang/include/clang/Tooling/Transformer/RewriteRule.h @@ -213,10 +213,12 @@ 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 @@ /// \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);
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits