This is an archive of the discontinued LLVM Phabricator instance.

[Driver][Modules] Remove dependence on linking support from clang/test/Driver/modules.cpp
ClosedPublic

Authored by DanielMcIntosh-IBM on May 30 2022, 10:24 AM.

Details

Summary

The new tests in clang/test/Driver/modules.cpp added by D120540 will fail if the
toolchain getting tested doesn't support linking. This reduces the utility of
the test since we would like a failure of this test to reflect a problem with
modules. We should already have other tests that validate linking support.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptMay 30 2022, 10:24 AM
Herald added a subscriber: StephenFan. · View Herald Transcript
DanielMcIntosh-IBM requested review of this revision.May 30 2022, 10:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 30 2022, 10:24 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
DanielMcIntosh-IBM added a subscriber: Restricted Project.May 30 2022, 10:29 AM
DanielMcIntosh-IBM retitled this revision from Remove dependence on linking support from clang/test/Driver/modules.cpp to [Driver][Modules] Remove dependence on linking support from clang/test/Driver/modules.cpp.May 30 2022, 10:32 AM

This change is fine (if I were adding the tests in the first place, I'd add -c or -S), but I'd like to understand why you have such a toolchain not supporting linking.
Such special requirements make me feel odd.

This change is fine (if I were adding the tests in the first place, I'd add -c or -S), but I'd like to understand why you have such a toolchain not supporting linking.
Such special requirements make me feel odd.

Downstream we have a branch that only contains some of the required changes for z/OS support, but which we still run tests on (why this is the case is a long story, but it's something we're working on fixing, it's just going to take a very long time to do so). This test is failing on that branch. Rather than loose the test, I figured I'd improve the test and in the process get it working again on that branch.

This revision is now accepted and ready to land.May 30 2022, 6:58 PM