Skip to content

Commit 332506f

Browse files
martinboehmemartinboehme
committed
Respond to review comments from @compnerd.
1 parent 72c4cd0 commit 332506f

File tree

3 files changed

+57
-35
lines changed

3 files changed

+57
-35
lines changed

‎lib/Driver/UnixToolChains.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -186,15 +186,15 @@ toolchains::GenericUnix::constructInvocation(const DynamicLinkJobAction &job,
186186
// a C++ standard library if it's not needed, in particular because the
187187
// standard library that `clang++` selects by default may not be the one that
188188
// is desired.
189-
const char *Clang =
189+
const char *LinkerDriver =
190190
context.Args.hasArg(options::OPT_enable_experimental_cxx_interop) ?
191191
"clang++" : "clang";
192192
if (const Arg *A = context.Args.getLastArg(options::OPT_tools_directory)) {
193193
StringRef toolchainPath(A->getValue());
194194

195-
// If there is a clang in the toolchain folder, use that instead.
196-
if (auto tool = llvm::sys::findProgramByName(Clang, {toolchainPath})) {
197-
Clang = context.Args.MakeArgString(tool.get());
195+
// If there is a linker driver in the toolchain folder, use that instead.
196+
if (auto tool = llvm::sys::findProgramByName(LinkerDriver, {toolchainPath})) {
197+
LinkerDriver = context.Args.MakeArgString(tool.get());
198198
}
199199

200200
// Look for binutils in the toolchain folder.
@@ -350,7 +350,7 @@ toolchains::GenericUnix::constructInvocation(const DynamicLinkJobAction &job,
350350
Arguments.push_back(
351351
context.Args.MakeArgString(context.Output.getPrimaryOutputFilename()));
352352

353-
InvocationInfo II{Clang, Arguments};
353+
InvocationInfo II{LinkerDriver, Arguments};
354354
II.allowsResponseFiles = true;
355355

356356
return II;

‎lib/Driver/WindowsToolChains.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,15 +85,15 @@ toolchains::Windows::constructInvocation(const DynamicLinkJobAction &job,
8585
}
8686

8787
// Configure the toolchain.
88-
const char *Clang =
88+
const char *LinkerDriver =
8989
context.Args.hasArg(options::OPT_enable_experimental_cxx_interop) ?
9090
"clang++" : "clang";
9191
if (const Arg *A = context.Args.getLastArg(options::OPT_tools_directory)) {
9292
StringRef toolchainPath(A->getValue());
9393

94-
// If there is a clang in the toolchain folder, use that instead.
95-
if (auto tool = llvm::sys::findProgramByName(Clang, {toolchainPath}))
96-
Clang = context.Args.MakeArgString(tool.get());
94+
// If there is a linker driver in the toolchain folder, use that instead.
95+
if (auto tool = llvm::sys::findProgramByName(LinkerDriver, {toolchainPath}))
96+
LinkerDriver = context.Args.MakeArgString(tool.get());
9797
}
9898

9999
// Rely on `-libc` to correctly identify the MSVC Runtime Library. We use
@@ -189,7 +189,7 @@ toolchains::Windows::constructInvocation(const DynamicLinkJobAction &job,
189189
Arguments.push_back(
190190
context.Args.MakeArgString(context.Output.getPrimaryOutputFilename()));
191191

192-
InvocationInfo II{Clang, Arguments};
192+
InvocationInfo II{LinkerDriver, Arguments};
193193
II.allowsResponseFiles = true;
194194

195195
return II;

‎test/Driver/linker.swift

Lines changed: 47 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -101,12 +101,20 @@
101101
// INFERRED_NAMED_DARWIN tests above: 'libLINKER.dylib'.
102102
// RUN: %swiftc_driver -driver-print-jobs -target x86_64-apple-macosx10.9 -emit-library %s -o libLINKER.dylib | %FileCheck -check-prefix INFERRED_NAME_DARWIN %s
103103

104-
// RUN: %swiftc_driver -driver-print-jobs -target x86_64-apple-ios7.1 -enable-experimental-cxx-interop -experimental-cxx-stdlib libc++ %s 2>&1 | %FileCheck -check-prefix IOS-cxx-interop %s
104+
// On Darwin, when C++ interop is turned on, we link against libc++ explicitly
105+
// regardless of whether -experimental-cxx-stdlib is specified or not. So also
106+
// run a test where C++ interop is turned off to make sure we don't link
107+
// against libc++ in this case.
108+
// RUN: %swiftc_driver -driver-print-jobs -target x86_64-apple-ios7.1 %s 2>&1 | %FileCheck -check-prefix IOS-no-cxx-interop %s
109+
// RUN: %swiftc_driver -driver-print-jobs -target x86_64-apple-ios7.1 -enable-experimental-cxx-interop %s 2>&1 | %FileCheck -check-prefix IOS-cxx-interop-libcxx %s
110+
// RUN: %swiftc_driver -driver-print-jobs -target x86_64-apple-ios7.1 -enable-experimental-cxx-interop -experimental-cxx-stdlib libc++ %s 2>&1 | %FileCheck -check-prefix IOS-cxx-interop-libcxx %s
105111
// RUN: not %swiftc_driver -driver-print-jobs -target x86_64-apple-ios7.1 -enable-experimental-cxx-interop -experimental-cxx-stdlib libstdc++ %s 2>&1 | %FileCheck -check-prefix IOS-cxx-interop-libstdcxx %s
106112

107-
// RUN: %swiftc_driver -driver-print-jobs -target x86_64-unknown-linux-gnu -enable-experimental-cxx-interop -experimental-cxx-stdlib libc++ %s 2>&1 | %FileCheck -check-prefix LINUX-cxx-interop %s
113+
// RUN: %swiftc_driver -driver-print-jobs -target x86_64-unknown-linux-gnu -enable-experimental-cxx-interop %s 2>&1 | %FileCheck -check-prefix LINUX-cxx-interop %s
114+
// RUN: %swiftc_driver -driver-print-jobs -target x86_64-unknown-linux-gnu -enable-experimental-cxx-interop -experimental-cxx-stdlib libc++ %s 2>&1 | %FileCheck -check-prefix LINUX-cxx-interop-libcxx %s
108115

109-
// RUN: %swiftc_driver -driver-print-jobs -target x86_64-unknown-windows-msvc -enable-experimental-cxx-interop -experimental-cxx-stdlib libc++ %s 2>&1 | %FileCheck -check-prefix WINDOWS-cxx-interop %s
116+
// RUN: %swiftc_driver -driver-print-jobs -target x86_64-unknown-windows-msvc -enable-experimental-cxx-interop %s 2>&1 | %FileCheck -check-prefix WINDOWS-cxx-interop %s
117+
// RUN: %swiftc_driver -driver-print-jobs -target x86_64-unknown-windows-msvc -enable-experimental-cxx-interop -experimental-cxx-stdlib libc++ %s 2>&1 | %FileCheck -check-prefix WINDOWS-cxx-interop-libcxx %s
110118

111119
// Check reading the SDKSettings.json from an SDK
112120
// RUN: %swiftc_driver -driver-print-jobs -target x86_64-apple-macosx10.9 -sdk %S/Inputs/MacOSX10.15.versioned.sdk %s 2>&1 | %FileCheck -check-prefix MACOS_10_15 %s
@@ -415,34 +423,48 @@
415423
// INFERRED_NAME_WINDOWS: -o LINKER.dll
416424
// INFERRED_NAME_WASI: -o libLINKER.so
417425

418-
// IOS-cxx-interop: swift
419-
// IOS-cxx-interop-DAG: -enable-cxx-interop
420-
// IOS-cxx-interop-DAG: -o [[OBJECTFILE:.*]]
421-
422-
// IOS-cxx-interop: {{(bin/)?}}ld{{"? }}
423-
// IOS-cxx-interop-DAG: [[OBJECTFILE]]
424-
// IOS-cxx-interop-DAG: -lc++
425-
// IOS-cxx-interop: -o linker
426+
// Instead of a single "NOT" check for this run, we would really want to check
427+
// for all of the driver arguments that we _do_ expect, and then use an
428+
// --implicit-check-not to check that -lc++ doesn't occur.
429+
// However, --implicit-check-not has a bug where it fails to flag the
430+
// unexpected text when it occurs after text matched by a CHECK-DAG; see
431+
// https://bugs.llvm.org/show_bug.cgi?id=45629
432+
// For this reason, we use a single "NOT" check for the time being here.
433+
// The same consideration applies to the Linux and Windows cases below.
434+
// IOS-no-cxx-interop-NOT: -lc++
435+
436+
// IOS-cxx-interop-libcxx: swift
437+
// IOS-cxx-interop-libcxx-DAG: -enable-cxx-interop
438+
// IOS-cxx-interop-libcxx-DAG: -o [[OBJECTFILE:.*]]
439+
440+
// IOS-cxx-interop-libcxx: {{(bin/)?}}ld{{"? }}
441+
// IOS-cxx-interop-libcxx-DAG: [[OBJECTFILE]]
442+
// IOS-cxx-interop-libcxx-DAG: -lc++
443+
// IOS-cxx-interop-libcxx: -o linker
426444

427445
// IOS-cxx-interop-libstdcxx: error: The only C++ standard library supported on Apple platforms is libc++
428446

429-
// LINUX-cxx-interop: swift
430-
// LINUX-cxx-interop-DAG: -enable-cxx-interop
431-
// LINUX-cxx-interop-DAG: -o [[OBJECTFILE:.*]]
447+
// LINUX-cxx-interop-NOT: -stdlib
448+
449+
// LINUX-cxx-interop-libcxx: swift
450+
// LINUX-cxx-interop-libcxx-DAG: -enable-cxx-interop
451+
// LINUX-cxx-interop-libcxx-DAG: -o [[OBJECTFILE:.*]]
452+
453+
// LINUX-cxx-interop-libcxx: clang++{{(\.exe)?"? }}
454+
// LINUX-cxx-interop-libcxx-DAG: [[OBJECTFILE]]
455+
// LINUX-cxx-interop-libcxx-DAG: -stdlib=libc++
456+
// LINUX-cxx-interop-libcxx: -o linker
432457

433-
// LINUX-cxx-interop: clang++{{(\.exe)?"? }}
434-
// LINUX-cxx-interop-DAG: [[OBJECTFILE]]
435-
// LINUX-cxx-interop-DAG: -stdlib=libc++
436-
// LINUX-cxx-interop: -o linker
458+
// WINDOWS-cxx-interop-NOT: -stdlib
437459

438-
// WINDOWS-cxx-interop: swift
439-
// WINDOWS-cxx-interop-DAG: -enable-cxx-interop
440-
// WINDOWS-cxx-interop-DAG: -o [[OBJECTFILE:.*]]
460+
// WINDOWS-cxx-interop-libcxx: swift
461+
// WINDOWS-cxx-interop-libcxx-DAG: -enable-cxx-interop
462+
// WINDOWS-cxx-interop-libcxx-DAG: -o [[OBJECTFILE:.*]]
441463

442-
// WINDOWS-cxx-interop: clang++{{(\.exe)?"? }}
443-
// WINDOWS-cxx-interop-DAG: [[OBJECTFILE]]
444-
// WINDOWS-cxx-interop-DAG: -stdlib=libc++
445-
// WINDOWS-cxx-interop: -o linker
464+
// WINDOWS-cxx-interop-libcxx: clang++{{(\.exe)?"? }}
465+
// WINDOWS-cxx-interop-libcxx-DAG: [[OBJECTFILE]]
466+
// WINDOWS-cxx-interop-libcxx-DAG: -stdlib=libc++
467+
// WINDOWS-cxx-interop-libcxx: -o linker
446468

447469
// Test ld detection. We use hard links to make sure
448470
// the Swift driver really thinks it's been moved.

0 commit comments

Comments
 (0)