This is an archive of the discontinued LLVM Phabricator instance.

[Driver] Support obtaining active toolchain from gcc-config on Gentoo
ClosedPublic

Authored by mgorny on Oct 16 2016, 10:21 AM.

Details

Summary

Support using gcc-config to determine the correct GCC toolchain location
on Gentoo. In order to do that, attempt to read gcc-config configuration
form [[sysroot]]/etc/env.d/gcc, if no custom toolchain location is
provided.

Diff Detail

Repository
rL LLVM

Event Timeline

mgorny updated this revision to Diff 74801.Oct 16 2016, 10:21 AM
mgorny retitled this revision from to [Driver] Support obtaining active toolchain from gcc-config on Gentoo .
mgorny updated this object.
mgorny added reviewers: rafael, chandlerc, bruno, bkramer.
mgorny added a subscriber: cfe-commits.
ABataev added inline comments.
lib/Driver/ToolChains.cpp
1438 ↗(On Diff #74801)

Use range-based for here

1446–1463 ↗(On Diff #74801)

I think it is better to use llvm::Triple class here for parsing ARCHITECTURE-VENDOR-OPERATING_SYSTEM-ENVIRONMENT string rather than StringRef::split()

mgorny added inline comments.Oct 17 2016, 6:46 AM
lib/Driver/ToolChains.cpp
1446–1463 ↗(On Diff #74801)

This is not parsing the triple but detaching the version appended to it.

mgorny marked an inline comment as done.Oct 17 2016, 8:51 AM
mgorny updated this revision to Diff 74855.Oct 17 2016, 8:57 AM

Updated to use range-based for loop as suggested by @ABataev.

ABataev added inline comments.Oct 17 2016, 9:00 AM
lib/Driver/ToolChains.cpp
1446–1463 ↗(On Diff #74801)

But still it is better to use some standard infrastructure rather than inventing a new one (with possible bugs, incompatibilities etc.)

mgorny added inline comments.Oct 17 2016, 9:13 AM
lib/Driver/ToolChains.cpp
1446–1463 ↗(On Diff #74801)

I still don't understand what you want me to do. llvm::Triple does not support triple + version thingy that is the case here. I don't need to parse the triple, just split the version out of it.

ABataev added inline comments.Oct 17 2016, 9:24 AM
lib/Driver/ToolChains.cpp
1446–1463 ↗(On Diff #74801)

Can you use Triple::getEnvironmentVersion()?

mgorny added inline comments.Oct 17 2016, 9:51 AM
lib/Driver/ToolChains.cpp
1446–1463 ↗(On Diff #74801)

No. This is not embedded in environment field but used as a separate component following it. Triple class doesn't cover it at all.

ABataev added inline comments.Oct 17 2016, 10:06 AM
lib/Driver/ToolChains.cpp
1446–1463 ↗(On Diff #74801)

Ok

mgorny marked 7 inline comments as done.Oct 17 2016, 11:19 AM
bkramer added inline comments.Oct 25 2016, 2:40 AM
lib/Driver/ToolChains.cpp
1438 ↗(On Diff #74855)

drop the const&. Using StringRef by value is fine.

1445 ↗(On Diff #74855)

drop ref

1447 ↗(On Diff #74855)

We have StringRef::consume_front now to make this more convenient.

mgorny updated this revision to Diff 75707.Oct 25 2016, 8:06 AM
mgorny marked 3 inline comments as done.

Thanks for the review. Implemented all three suggestions.

bkramer accepted this revision.Oct 25 2016, 8:13 AM
bkramer edited edge metadata.

ship it.

This revision is now accepted and ready to land.Oct 25 2016, 8:13 AM
This revision was automatically updated to reflect the committed changes.