This is an archive of the discontinued LLVM Phabricator instance.

[clang] Put original flags on 'Driver args:' crash report line
ClosedPublic

Authored by thakis on Sep 24 2021, 4:46 PM.

Details

Summary

We used to put the canonical spelling of flags after alias processing
on that line. For clang-cl in particular, that meant that we put flags
on that line that the clang-cl driver doesn't even accept, and the
"Driver args:" line wasn't usable.

Diff Detail

Event Timeline

thakis requested review of this revision.Sep 24 2021, 4:46 PM
thakis created this revision.
hans accepted this revision.Sep 27 2021, 5:57 AM

lgtm

clang/lib/Driver/Driver.cpp
1220

Could put a comment here about why it's calling getAlias() (Or maybe it's just me that finds the getAlias() name confusing.)

clang/test/Driver/crash-report-clang-cl.c
2

These always make me thing the test should "requires: shell", but I think for these commands it should work without, right?

This revision is now accepted and ready to land.Sep 27 2021, 5:57 AM
thakis marked an inline comment as done.Sep 27 2021, 6:03 AM

Thanks!

clang/test/Driver/crash-report-clang-cl.c
2

Yes, these work without REQUIRES: shell. We require a whole bunch of unix utils on Windows, but we don't require a shell for most tests. That's usually required if doing advanced piping maybe? Actually, looking at REQUIRES: shell in tests, many put it there needlessly :/

thakis updated this revision to Diff 375232.Sep 27 2021, 6:04 AM

add comment

This revision was landed with ongoing or failed builds.Sep 27 2021, 7:25 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptSep 27 2021, 7:25 AM