This is an archive of the discontinued LLVM Phabricator instance.

clang-rename: check that the source location we find actually has the old name
ClosedPublic

Authored by vmiklos on May 12 2016, 11:36 AM.

Diff Detail

Event Timeline

vmiklos updated this revision to Diff 57077.May 12 2016, 11:36 AM
vmiklos retitled this revision from to clang-rename: check that the source location we find actually has the old name.
vmiklos updated this object.
vmiklos added reviewers: cfe-commits, klimek.
vmiklos added subscribers: cfe-commits, klimek.
vmiklos removed subscribers: klimek, cfe-commits.
klimek added inline comments.May 13 2016, 1:07 AM
clang-rename/USRLocFinder.cpp
37

For constructors, we nowadays usually want to take strings by value and then std::move them into the fields.

75

Why startswith as opposed to ==?

clang-rename/USRLocFinder.h
32–35

Now that I see this - is there a reason not to use const std::string& for the usr? Also, should we instead use StringRef?

vmiklos added inline comments.May 13 2016, 1:13 AM
clang-rename/USRLocFinder.cpp
75

The source range is "foo(0)", the old name is "foo". Or is there a way to get the range of the name part of the initializer, without the "(0)" part?

I'll fix the rest, those are easy. :-)

klimek added inline comments.May 13 2016, 1:20 AM
clang-rename/USRLocFinder.cpp
75

Yep, if you want to get an identifier, you just get the code for the range getLocation(), getLocation() (note that ranges are inclusive token-ranges by default)

vmiklos updated this revision to Diff 57140.May 13 2016, 1:38 AM

Got rid of startswith() and now using StringRef everywhere instead of a mix of std::string, const std::string and const std::string&.

klimek accepted this revision.May 13 2016, 1:47 AM
klimek edited edge metadata.

lg

clang-rename/USRLocFinder.cpp
126–128

Usually storing StringRefs in the class is dangerous, but as this is defined in the .cpp file and only used as part of the function, that's probably fine.
(I personally would still make them std::string members, and copy either in the constructor or when passing into the function).

This revision is now accepted and ready to land.May 13 2016, 1:47 AM
This revision was automatically updated to reflect the committed changes.