Page MenuHomePhabricator

[Tooling] Add "-filter" option to AllTUsExecution
ClosedPublic

Authored by hokein on Nov 5 2018, 1:47 AM.

Event Timeline

hokein created this revision.Nov 5 2018, 1:47 AM
ioeric added inline comments.Nov 5 2018, 1:55 AM
lib/Tooling/AllTUsExecution.cpp
58 ↗(On Diff #172574)

Please also mention that this only applies to all-TUs.

121 ↗(On Diff #172574)

Filter.getNumOccurrences() != 0

Would this work if Filter is set grammatically? Although it's not exposed now, it might make more sense to check Filter != ".*".

hokein updated this revision to Diff 172560.Nov 5 2018, 3:17 AM
hokein marked an inline comment as done.

Update comment.

hokein added inline comments.Nov 5 2018, 3:17 AM
lib/Tooling/AllTUsExecution.cpp
121 ↗(On Diff #172574)

Yes, getNumOccurrences returns non-zero only when the flag is set by the command line.

clang-tool -executor=all-TUs .  # => getNumOccurrences() == 0
clang-tool -executor=all-TUs -filter="xx" .  #  > getNumOccurrences() != 0
ioeric added inline comments.Nov 5 2018, 3:22 AM
lib/Tooling/AllTUsExecution.cpp
121 ↗(On Diff #172574)

Sorry, I meant "programmatically"... something like Filter.setInitialValue("...").

hokein updated this revision to Diff 172571.Nov 5 2018, 4:56 AM

Remove the default value check.

lib/Tooling/AllTUsExecution.cpp
121 ↗(On Diff #172574)

hmm, it works the same as llvm::cl::init (getNumOccurrences returns 0).

I removed this check (just use if (!Filter.match)), the reason I added here is to avoid the cost of regex match, but it seems pre-optimization.

ioeric accepted this revision.Nov 5 2018, 5:01 AM
ioeric added inline comments.
lib/Tooling/AllTUsExecution.cpp
121 ↗(On Diff #172574)

sounds good.

This revision is now accepted and ready to land.Nov 5 2018, 5:01 AM
ioeric added a comment.Nov 5 2018, 5:01 AM

maybe add a test?

hokein updated this revision to Diff 172574.Nov 5 2018, 5:31 AM

Add test.

This revision was automatically updated to reflect the committed changes.