Page MenuHomePhabricator

[clang-query] Add option to enable only displaying main file matches

Authored by njames93 on Mar 2 2021, 1:50 PM.



Add option set match-scope (include-headers|main-file-only) to control only printing matches from the main file.
This can be set once and then avoids the need to pollute all your match expressions with isExpansionInMainFile()
By default it is enabled to only show match results from the main file.

Diff Detail

Event Timeline

njames93 created this revision.Mar 2 2021, 1:50 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 2 2021, 1:51 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
aaron.ballman added inline comments.Mar 3 2021, 6:51 AM

I'm not super keen on this being the default behavior. One of the big uses for clang-query is to experiment with matchers and I think ignoring nodes by default makes it harder to know whether you've written the matcher correctly or not. e.g., you write the matcher, run it over a source file, don't get any results -- is that because the matcher was nonsense or because the mode excluded matching on nodes in headers by default?

The fact that we print how many nodes were skipped does help (so it's not a silent when you match only in headers), but I think the default behavior should be to match on what's in the source code (which includes header contents) and users should have to opt into getting less results. WDYT?

njames93 added inline comments.Mar 3 2021, 7:12 AM

I thought of making main-file-only the default as its the behaviour that clang-tidy exhibits by default. That's also the reason I included printing details of skipped matching nodes. But you're right there's definitely a compelling argument to leave the default as matching from header files as well.

njames93 updated this revision to Diff 327921.Mar 3 2021, 2:29 PM

Change default match mode to include all files.
Update so the prompt to switch match mode is only displayed once.

aaron.ballman accepted this revision.Mar 4 2021, 5:09 AM

LGTM aside from a typo fix.

This revision is now accepted and ready to land.Mar 4 2021, 5:09 AM
njames93 marked an inline comment as done.Mar 4 2021, 5:13 AM