ilya-biryukov added a comment.
Thanks, the APIs totally make sense. And seem to fit into the other functions
we have in `FixIt.h`, although the name of the file is somewhat misleading.
Mostly comments about naming and one comment about the necessity of using
matchers from my side.
================
Comment at: clang/include/clang/Tooling/FixIt.h:60
+// future to include more associated text (like comments).
+CharSourceRange getSourceRangeAuto(const Stmt &S, ASTContext &Context);
+
----------------
Do you have alternative names in mind? It would be nice to (1) not mention the
SourceRange now that we return CharSourceRange, (2) change "auto" to something
more descriptive.
Was thinking about `getNodeRange()` or `getSpannedRange()`, but that completely
misses the "auto" part (maybe it's fine, though).
WDYT? Maybe other ideas?
================
Comment at: clang/include/clang/Tooling/FixIt.h:73
+// context. In contrast with \p getText(), this function selects a source range
+// "automatically", extracting text that a reader might intuitively associate
+// with a node. Currently, only specialized for \p clang::Stmt, where it will
----------------
What are other tricky cases you have in mind for the future?
================
Comment at: clang/include/clang/Tooling/FixIt.h:77
+template <typename T>
+StringRef getTextAuto(const T &Node, ASTContext &Context) {
+ return internal::getText(getSourceRangeAuto(Node, Context), Context);
----------------
Could you add an example of the API use here? The anticipated use-case are
removing or textually replacing a node, right?
================
Comment at: clang/lib/Tooling/FixIt.cpp:52
+
+ auto NotCondition = unless(hasCondition(equalsNode(&S)));
+ auto Standalone =
----------------
Do you expect this function to be on the hot path?
If so, I'd advice against using the matchers here. They do add enough overhead
to be avoided in hot functions.
I guess the problem is that we can't get a hold of the parent node with using
the matchers, right?
Not sure if there's an easy way out of it in that case.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58556/new/
https://reviews.llvm.org/D58556
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
