This is an archive of the discontinued LLVM Phabricator instance.

[clang] [test] Use %clang_cc1 substitution consistently
ClosedPublic

Authored by mgorny on Sep 29 2022, 7:18 AM.

Details

Summary

Use the %clang_cc1 substitution consistently across the test suite,
replacing inline %clang -cc1 invocations, except for one Preprocessor
test where this is causing breakage. This is necessary to ensure that
additional parameters passed via %clang do not interfere with -cc1
that must always be passed as the first command-line argument.

Remove the additional substitution blocking %clang_cc1 use in Driver
tests. It has been added in 2013 and was supposed to prevent tests
calling clang -cc1 from being added to Driver. The state of the test
suite proves that it did not succeed at all.

Diff Detail

Event Timeline

mgorny created this revision.Sep 29 2022, 7:18 AM
mgorny requested review of this revision.Sep 29 2022, 7:18 AM
MaskRay accepted this revision.Sep 29 2022, 9:40 AM

LGTM.

This revision is now accepted and ready to land.Sep 29 2022, 9:40 AM
probinson added inline comments.
clang/test/Driver/lit.local.cfg
8

But we _don't_ want to use %clang_cc1 in Driver tests; if we do, then we're not testing the driver! Those tests really belong somewhere else, like Frontend.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptSep 29 2022, 12:00 PM
MaskRay added inline comments.Sep 29 2022, 12:12 PM
clang/test/Driver/lit.local.cfg
8

Sorry, I missed this change. I think a lot of %clang -cc1 tests in test/Driver are actually wrong and should be moved to appropriate components.

The test/Driver part of this patch should be reverted.

mgorny added inline comments.Sep 29 2022, 12:58 PM
clang/test/Driver/lit.local.cfg
8

I agree we don't want these tests there but 1) does anyone really volunteer to fix the problem, and 2) did the substitution override really prevent people from adding more wrong tests? I'm not against this, not at all. Just feel like we're fighting windmills here.

MaskRay added a subscriber: yaxunl.Sep 29 2022, 1:29 PM
MaskRay added inline comments.
clang/test/Driver/lit.local.cfg
8

There aren't many %clang -cc1 tests in test/Driver. We can just notify people working on the components and hope that they will move tests to the appropriate components.

@python3kgae @yaxunl

8