This is an archive of the discontinued LLVM Phabricator instance.

[clang] [MinGW] Fix gcc version detection/picking
ClosedPublic

Authored by mstorsjo on May 20 2021, 1:53 PM.

Details

Summary

Actually compare each version to the version of the last chosen one.

There's no guarantee that the added test case does showcase the
previous issue (it depends on the order that directory entries
are returned when iterating), but with the issue fixed it should behave
deterministically in any case.

Diff Detail

Event Timeline

mstorsjo requested review of this revision.May 20 2021, 1:53 PM
mstorsjo created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptMay 20 2021, 1:53 PM
mstorsjo updated this revision to Diff 346850.May 20 2021, 2:01 PM

Update another affected testcase.

Adding some more reviewers, this is a trivial bug fix.

mati865 accepted this revision.May 27 2021, 4:04 PM
This revision is now accepted and ready to land.May 27 2021, 4:04 PM
MaskRay accepted this revision.May 27 2021, 4:09 PM

LGTM.

clang/test/Driver/mingw-sysroot.cpp
20–21

You may want to the adopt the -SAME: {{^}} scheme I used in linux-cross.cpp

It can make sure the include paths are consecutive.

Also @mati865 FYI, phabricator has this odd behaviour, that if you press "accept" but don't write a textual message, the notification mail only gets sent to the person receivers but not to the mailing list - so readers of the list will only see the original messages and then e.g. when the patch is pushed with no messages inbetween, it can seem a bit puzzling. So it's usually good to fill it out with a plain "LGTM" message or something if accepting a review.

clang/test/Driver/mingw-sysroot.cpp
20–21

Thanks! Note for reference - I had to change the pattern {{.*}} into {{[^"]*}} (or better, [[SYSROOT:[^"]+]] as in linux-cross.cpp) for this to work properly, otherwise that wildcard can end up capturing the "-internal-isystem" which we want to spell out, if we want to force them to be consecutive. And not all of these are entirely consecutive, so the last one ends up without -SAME.

Ah, so that explains why everyone sends LGTM text.
I'll try to remember it next time.

This revision was automatically updated to reflect the committed changes.