This is an archive of the discontinued LLVM Phabricator instance.

[clang] [MinGW] Fix paths on Gentoo
ClosedPublic

Authored by maharmstone on Oct 4 2021, 11:18 AM.

Details

Summary

There's code in clang/lib/Driver/ToolChains/Gnu.cpp for Clang to use Gentoo's include and lib paths, but this is missing for mingw, meaning that any C++ programs using the STL will fail to compile.

See https://bugs.gentoo.org/788430

Diff Detail

Event Timeline

maharmstone created this revision.Oct 4 2021, 11:18 AM
maharmstone requested review of this revision.Oct 4 2021, 11:18 AM
mstorsjo added a subscriber: cfe-commits.

(I’ll have a look and comment on the patch later.)

The change looks ok to me, but I think it'd be good to have a testcase for finding these paths (IIRC there are a bunch of simulated sysroots under clang/test/Driver).

The change looks ok to me, but I think it'd be good to have a testcase for finding these paths (IIRC there are a bunch of simulated sysroots under clang/test/Driver).

Seconded. There are already some Gentoo trees there, so it shouldn't be hard to add one more.

Added tests

I can't see that any of the lib paths are tested at all, though I may be missing something.

Looks ok to me - WDYT @mgorny?

Well, I dunno how MinGW works, so can't judge that part. Furthermore, I'm thoroughly confused why this works on top of existing (non-Gentoo?) test tree. Could you explain it a bit more?

Well, I dunno how MinGW works, so can't judge that part. Furthermore, I'm thoroughly confused why this works on top of existing (non-Gentoo?) test tree. Could you explain it a bit more?

It looks like the clang driver just blindly adds these directories to the list of include paths, regardless of whether they exist or not. So with the current test, they don't need to exist (but maybe it would be good to create such empty directories to make the test a bit more correct, in case the driver is modified to not add nonexistent directories blindly).

FYI @maharmstone the test needs to be rebased to fit on top of current git, after rG897c86dec5af2780d443edd852aa5911e2650ec6.

Herald added a project: Restricted Project. · View Herald TranscriptJun 23 2022, 1:10 AM
Herald added a subscriber: StephenFan. · View Herald Transcript

Rebased patch against current upstream

Does Gentoo patch upstream to do something different? (.../g++-v10/...) Well, I wish that Gentoo does not do this.
This adds complexity to Clang which in practice tries to provide some drop-in replacement ability.

Does Gentoo patch upstream to do something different? (.../g++-v10/...) Well, I wish that Gentoo does not do this.
This adds complexity to Clang which in practice tries to provide some drop-in replacement ability.

Clang doesn't work at all on Gentoo at the moment with any of the mingw targets, hence this patch.

As I said in the top comment, these changes are mirroring what's in clang/lib/Driver/ToolChains/Gnu.cpp for the Linux triplets.

mstorsjo accepted this revision.Jul 6 2022, 2:23 PM

LGTM, thanks, and sorry for losing track of it earlier.

Do you need someone to push the patch for you? In that case, can you provide your preferred git author line for the patch?

This revision is now accepted and ready to land.Jul 6 2022, 2:23 PM
maharmstone added a comment.EditedJul 7 2022, 2:29 PM

LGTM, thanks, and sorry for losing track of it earlier.

Do you need someone to push the patch for you? In that case, can you provide your preferred git author line for the patch?

Thanks Martin. Yes please - "Mark Harmstone <mark@harmstone.com>"

This revision was landed with ongoing or failed builds.Jul 7 2022, 2:37 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJul 7 2022, 2:37 PM