This is an archive of the discontinued LLVM Phabricator instance.

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

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

Diff Detail

Repository
rC Clang

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

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

121

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

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

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

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

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.