Page MenuHomePhabricator

[Driver][Darwin] Use Host Triple to infer target os version
ClosedPublic

Authored by steven_wu on Jul 2 2018, 2:30 PM.

Details

Summary

When clang required to infer target os version from --target option and
the os version is not specified in targets, check the host triple. If the
host and target are both macOS, use host triple to infer target os
version.

rdar://problem/41651999

Diff Detail

Repository
rL LLVM

Event Timeline

steven_wu created this revision.Jul 2 2018, 2:30 PM

Unfortunately, I wasn't able to write a test for this because the host triple in the configuration can either be x86_64-apple-darwin* or x86_64-apple-macosx*, but the one used passed by driver is always macosx one. I can't reliably compare those two.

steven_wu updated this revision to Diff 153814.Jul 2 2018, 3:33 PM

Update patch. Use a better API.

steven_wu updated this revision to Diff 153816.Jul 2 2018, 3:36 PM

Rebase the commit correctly

Hmm, the driver should not call inferDeploymentTargetFromArch when -target is passed. Or am I missing something?

Hmm, the driver should not call inferDeploymentTargetFromArch when -target is passed. Or am I missing something?

Good call. This only handles the *-apple-darwin case. Maybe the same should happen to *-apple-macosx.

steven_wu updated this revision to Diff 153822.Jul 2 2018, 4:18 PM

handle *-apple-macosx target option

Nit: Could you please try to extract the shared code into a function, e.g.

Optional<std::string> overrideMacOSTripleDefaultVersion(const llvm::Triple &Triple, ... OSTy, ... TheDriver) {
    if (Triple.getOSMajorVersion())
      return None;
    llvm::Triple SystemTriple(llvm::sys::getProcessTriple());
    if (SystemTriple.isMacOSX())
      return getOSVersion(OSTy, SystemTriple, TheDriver));
   return None;
}
steven_wu updated this revision to Diff 153836.Jul 2 2018, 6:01 PM

It is easier and cleaner if I just fold everything into getOSVersion.

arphaman accepted this revision.Jul 2 2018, 6:02 PM

LGTM

This revision is now accepted and ready to land.Jul 2 2018, 6:02 PM
This revision was automatically updated to reflect the committed changes.