ilya-biryukov added a comment.
This clearly increases the utility of the library, but also seems to add corner
cases that the library won't handle (see the comment about unittests for an
example).
WDYT about those? Are they important, should we support producing warnings in
those cases to let the users know things might get broken?
================
Comment at: clang/lib/Tooling/Refactoring/Transformer.cpp:76
+static llvm::Optional<CharSourceRange>
+makeValidRange(CharSourceRange Range, const MatchResult &Result) {
+ const SourceManager &SM = *Result.SourceManager;
----------------
Could we add unit tests for this particular function?
Interesting cases (`[[` and `]]` mark the start and end of a range):
```
#define FOO(a) a+a;
#define BAR 10+
// change part of a macro argument
int a = FOO([[10]] + 10);
// change the whole macro expansion
int b = [[FOO(10+10)]];
// Try to change 10 inside 'BAR', but not '+'.
// Should this fail? Should we give a warning?
int c = BAR 3;
// Try changing the lhs (10) of a binary expr, but not rhs.
// Is that allowed? Should we give a warning?
int d = FOO(10);
```
================
Comment at: clang/lib/Tooling/Refactoring/Transformer.cpp:99
+
+ std::pair<FileID, unsigned> beginInfo =
SM.getDecomposedLoc(Range.getBegin());
+ std::pair<FileID, unsigned> endInfo = SM.getDecomposedLoc(Range.getEnd());
----------------
naming NIT: use `BeginInfo`
================
Comment at: clang/unittests/Tooling/TransformerTest.cpp:625
+ RewriteRule R = makeRule(integerLiteral(equals(0)).bind(zero),
+ change(node(zero), text("0")));
+ testRule(R, Input, Expected);
----------------
could we change to something other than `0` to make sure it's not the macro
being expanded?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64518/new/
https://reviews.llvm.org/D64518
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
