etienneb added a comment.
I added the "using statements".
landing.
================
Comment at: include/clang/Tooling/Fixit.h:35
@@ +34,3 @@
+/// used by the following template abstractions.
+inline SourceRange getSourceRange(const SourceRange &Range) { return Range; }
+
----------------
alexfh wrote:
> Do we want to expose `getSourceRange` as an API? If no, it should also be in
> the `namespace internal`.
Good question.
I moved it into the internal namespace.
If it's become needed, we can re-expose it.
================
Comment at: include/clang/Tooling/Fixit.h:55
@@ +54,3 @@
+// \brief Returns a FixItHint to remove \p Node.
+template <typename T> FixItHint createRemoval(const T &Node) {
+ return FixItHint::CreateRemoval(getSourceRange(Node));
----------------
alexfh wrote:
> I wonder whether we should later extend this to remove comments attached to
> nodes like Stmt and FunctionDecl? It's out of this patches scope, but maybe
> add a FIXME to sketch the further development plans.
I like the idea. I didn't think about comments and other syntactical elements
that may be related to a Node.
But, I believe we will have a other set of functions for Node and comments.
An other concept that I believe it's missing is to provide helper to append
expression (operators) and stay semantically correct with operator precedence.
Some cases needed to add parentheses.
================
Comment at: unittests/Tooling/FixitTest.cpp:53
@@ +52,3 @@
+
+TEST(FixitTest, getTextWithMacro) {
+ CallsVisitor Visitor;
----------------
alexfh wrote:
> Could you add a test where getText returns an empty string?
see line 68, already there!
================
Comment at: unittests/Tooling/FixitTest.cpp:119
@@ +118,3 @@
+ LocationToString(Hint0.RemoveRange.getBegin(), Context));
+ EXPECT_EQ("input.cc:2:26 <Spelling=input.cc:1:17>",
+ LocationToString(Hint0.RemoveRange.getEnd(), Context));
----------------
alexfh wrote:
> One character range is boring. Can you make the parameter longer and also
> verify whether the range is a token range or a character range?
In this case, it's not one character... it's one token.
As it's a "token range", the whole token is used.
But, I can use an expression too.
Not a bad idea: added but in 'createRemoval'
There is already a reand at line 138:
EXPECT_EQ("input.cc:2:26 <Spelling=input.cc:1:37>",
EXPECT_EQ("input.cc:2:26 <Spelling=input.cc:1:45>",
http://reviews.llvm.org/D19941
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
