================
Comment at: cpp11-migrate/Core/FileOverrides.cpp:104
@@ +103,3 @@
+
+ for (Replacements::iterator I = Replaces.begin(), E = Replaces.end(); I != E;
+ ++I) {
----------------
Edwin Vane wrote:
> Perhaps a comment saying you're grouping Replacements by file name.
Sure. Done.
================
Comment at: cpp11-migrate/Core/FileOverrides.cpp:207
@@ +206,3 @@
+ // replacement inside range -> resize the range
+ if (Range.contains(Replace)) {
+ int Difference = ReplaceNewSize - Replace.getLength();
----------------
Edwin Vane wrote:
> Why change the range at all? If a replacement is completely within an
> existing range, the part of the range outside of the replacement is still
> something that has changed right?
Yes you are right it is changed. What I'm doing here is resizing the range so
it takes the replacement into account.
Let's say we have a range: `Range(/*offset=*/0, /*length=*/5)`
And a replacement: `Replacement(/*offset=*/2, /*length=*/1,
/*replacement-text=*/"~~~")`
The replacement deletes 1 character but adds 3 new characters, a difference of
+2 characters.
That's why I'm resizing the range to take the new characters into account,
final range is `Range(0, 7)`.
================
Comment at: cpp11-migrate/Core/FileOverrides.cpp:224
@@ +223,3 @@
+
+ const tooling::Range Replace;
+ const unsigned ReplaceNewSize;
----------------
Edwin Vane wrote:
> Can't you just use const lvalue ref here instead of constructing a new Range?
A range is just 2 integers, it's cheaper to pass by value.
http://llvm-reviews.chandlerc.com/D1136
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
