This is an archive of the discontinued LLVM Phabricator instance.

[find-all-symbol] Add macro support.
ClosedPublic

Authored by hokein on May 19 2016, 2:43 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

hokein updated this revision to Diff 57752.May 19 2016, 2:43 AM
hokein retitled this revision from to [find-all-symbol] Add macro support..
hokein updated this object.
hokein added a reviewer: bkramer.
hokein added subscribers: ioeric, cfe-commits.
hokein updated this revision to Diff 57754.May 19 2016, 2:45 AM

Fix code-style.

ioeric added inline comments.May 19 2016, 3:17 AM
include-fixer/find-all-symbols/FindAllMacros.h
22 ↗(On Diff #57754)

nit: "A preprocessor that collects..."

36 ↗(On Diff #57754)

nit: "allowing clients to include..."

39 ↗(On Diff #57754)

This comment does not introduce useful information IMO.

include-fixer/find-all-symbols/SymbolReporter.h
18 ↗(On Diff #57754)

This doesn't make sense to me?

Also, I think it should be "An interface for classes that/to collect symbols" grammatically.

hokein updated this revision to Diff 57762.May 19 2016, 4:51 AM
hokein marked 4 inline comments as done.

Address comments.

bkramer accepted this revision.May 19 2016, 9:33 AM
bkramer edited edge metadata.

In the future I'd prefer to do renaming changes (ResultReporter->SymbolReporter) in a separate change, but this is fine now.

include-fixer/find-all-symbols/SymbolReporter.h
1 ↗(On Diff #57762)

find all symbols?

This revision is now accepted and ready to land.May 19 2016, 9:33 AM
hokein updated this revision to Diff 57901.May 20 2016, 1:02 AM
hokein edited edge metadata.

Correct the header comments.

hokein updated this revision to Diff 57902.May 20 2016, 1:07 AM

Remove an unneeded file.

This revision was automatically updated to reflect the committed changes.