This is an archive of the discontinued LLVM Phabricator instance.

[clang-rename] fix bug with initializer lists
ClosedPublic

Authored by omtcyfz on Aug 5 2016, 1:03 AM.

Details

Summary

clang-rename is currently not able to find a symbol in initializer list. This patch is a fix of that bug.

Diff Detail

Repository
rL LLVM

Event Timeline

omtcyfz updated this revision to Diff 66915.Aug 5 2016, 1:03 AM
omtcyfz retitled this revision from to [clang-rename] fix bug with initializer lists.
omtcyfz updated this object.
omtcyfz added a reviewer: alexfh.
omtcyfz added a subscriber: cfe-commits.
alexfh accepted this revision.Aug 8 2016, 7:06 AM
alexfh edited edge metadata.

LG with a nit.

clang-rename/USRFinder.cpp
100 ↗(On Diff #66915)

It might be not clear from the context what auto means here, I'd use the explicit type.

This revision is now accepted and ready to land.Aug 8 2016, 7:06 AM
alexfh requested changes to this revision.Aug 8 2016, 7:07 AM
alexfh edited edge metadata.

Wait, please add a test.

This revision now requires changes to proceed.Aug 8 2016, 7:07 AM
omtcyfz updated this revision to Diff 67165.EditedAug 8 2016, 7:42 AM
omtcyfz edited edge metadata.

Do these symbolic pointers in test seem reasonable?

alexfh added a comment.Aug 8 2016, 9:09 AM

Do these symbolic pointers in test seem reasonable?

Yes, kind of. I'd change these to C-style comments, place them on the same line with the interesting identifier (probably, right after the identifier), and document a generic way of finding the offset using grep -Ubo 'Foo.*' or whatever grep flags are needed.

For example:

$ cat /tmp/a.h 
#include <string>

struct Foo /* test 1 */ {
  Foo /* test 2 */ (const std::string &s);
  std::string s;
};

// Test 1.
// RUN: ..... -offset=26 ...
// Test2.
// RUN: ..... -offset=47 ...

// To find offsets after modifying the file, use:
//   grep -Ubo 'Foo.*' <file>
$ grep -Ubo 'Foo.*' /tmp/a.h 
26:Foo /* test 1 */ {
47:Foo /* test 2 */ (const std::string &s);
255:Foo.*' <file>
alexfh requested changes to this revision.Aug 8 2016, 9:14 AM
alexfh edited edge metadata.
This revision now requires changes to proceed.Aug 8 2016, 9:14 AM
omtcyfz updated this revision to Diff 67199.Aug 8 2016, 10:33 AM
omtcyfz edited edge metadata.

Added more info to the test.

alexfh accepted this revision.Aug 8 2016, 2:57 PM
alexfh edited edge metadata.

The test is fine now. Looks good once the other comment is addressed.

This revision is now accepted and ready to land.Aug 8 2016, 2:57 PM
omtcyfz updated this revision to Diff 67289.Aug 8 2016, 11:50 PM
omtcyfz edited edge metadata.
omtcyfz marked an inline comment as done.

auto -> SourceLocation

This revision was automatically updated to reflect the committed changes.