This is an archive of the discontinued LLVM Phabricator instance.

[clang][DependencyScanner] Remove all warning flags when suppressing warnings

Authored by Bigcheese on May 16 2023, 9:39 AM.



Since system modules don't emit most warnings, remove the warning
flags to increase module reuse.

Diff Detail

Event Timeline

Bigcheese created this revision.May 16 2023, 9:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 16 2023, 9:39 AM
Herald added a subscriber: ributzka. · View Herald Transcript
Bigcheese requested review of this revision.May 16 2023, 9:39 AM
jansvoboda11 added inline comments.May 16 2023, 10:06 AM

I'd like to see a check line that would fail if the scanner reports another variant of module A. The CHECK: lines make it possible for FileCheck to skip an entire element in the modules array. (Structural matching on JSON in FileCheck would be a godsend.)


I'd expect the scanner to still produce two variants of module B, since it's a user module. Is that correct? If so, let's add explicit check for it, since we intentionally want to preserve the warning flags there.

Bigcheese added inline comments.May 17 2023, 3:33 PM

The way FileCheck works I do not believe this can happen. A CHECK: line finds the first line that matches, and CHECK-NEXT: follows exactly after that. There's no case where we have a CHECK: that can skip over another entire module entry.


Both A and B are system modules. A is found via -isystem, and B has the [system] attribute. The FileCheck lines will only succeed if there are exactly two modules returned.

jansvoboda11 accepted this revision.May 17 2023, 4:36 PM

LGTM, thanks!


You're right, this is good then.


Thanks for pointing that out. Can you put a comment saying just that at the start of the test along with description of the expected behavior?

This revision is now accepted and ready to land.May 17 2023, 4:36 PM
ributzka added inline comments.May 18 2023, 12:29 PM

This removes also all warnings when building your own module, because of the [system] attribute. I would prefer this optimization to be limited to modules that are in the sysroot and therefore ignore the [system] attribute.

Bigcheese added inline comments.May 30 2023, 6:08 PM

This is not really simple to do. We don't track why a module is a system module, and it's not really correct to do a string based match on the path for if it's from a system directory.

Bigcheese marked an inline comment as not done.May 30 2023, 6:08 PM
ributzka accepted this revision.May 31 2023, 11:24 AM

I see. LGTM