[PATCH] D121868: [cc1as] Add support for emitting the build version load command for -darwin-target-variant
bc-lee added inline comments. Comment at: clang/test/Misc/cc1as-darwin-target-variant-triple.s:2 +// Run cc1as using darwin-target-variant-triple +// RUN: %clang -cc1as -triple x86_64-apple-macos10.9 -darwin-target-variant-triple x86_64-apple-ios13.1-macabi -filetype obj %s -o - \ +// RUN: | llvm-readobj --file-headers --macho-version-min - \ thakis wrote: > bc-lee wrote: > > MaskRay wrote: > > > `-filetype obj` on the `%clang -cc1as` line seems weird. > > Without this, I cannot create object file and validate it using > > `llvm-readobj`. > If you use `emit-obj` I think this test needs a `REQUIRES: > x86-registered-target`. > > I'm not sure if piping binary data works on Windows – it might have stdout in > text mode, which will do some byte replacements. A search for `-o -.*\n.*RUN.*llvm-readobj` in the LLVM codebase suggests that there are already several tests using that pattern. Does that really matter? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121868/new/ https://reviews.llvm.org/D121868 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D121868: [cc1as] Add support for emitting the build version load command for -darwin-target-variant
bc-lee updated this revision to Diff 424209.
bc-lee added a comment.
Add `REQUIRES: x86-registered-target` on the test.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121868/new/
https://reviews.llvm.org/D121868
Files:
clang/include/clang/Driver/Options.td
clang/include/clang/Driver/ToolChain.h
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Driver/ToolChains/Darwin.h
clang/test/Misc/cc1as-darwin-target-variant-triple.s
clang/tools/driver/cc1as_main.cpp
Index: clang/tools/driver/cc1as_main.cpp
===
--- clang/tools/driver/cc1as_main.cpp
+++ clang/tools/driver/cc1as_main.cpp
@@ -144,6 +144,9 @@
/// otherwise.
std::string TargetABI;
+ /// Darwin target variant triple, the variant of the deployment target
+ /// for which the code is being compiled.
+ llvm::Optional DarwinTargetVariantTriple;
/// @}
public:
@@ -209,6 +212,9 @@
// Target Options
Opts.Triple = llvm::Triple::normalize(Args.getLastArgValue(OPT_triple));
+ if (Arg *A = Args.getLastArg(options::OPT_darwin_target_variant_triple))
+Opts.DarwinTargetVariantTriple = llvm::Triple(A->getValue());
+
Opts.CPU = std::string(Args.getLastArgValue(OPT_target_cpu));
Opts.Features = Args.getAllArgValues(OPT_target_feature);
@@ -407,6 +413,8 @@
// MCObjectFileInfo needs a MCContext reference in order to initialize itself.
std::unique_ptr MOFI(
TheTarget->createMCObjectFileInfo(Ctx, PIC));
+ if (Opts.DarwinTargetVariantTriple)
+MOFI->setDarwinTargetVariantTriple(*Opts.DarwinTargetVariantTriple);
Ctx.setObjectFileInfo(MOFI.get());
if (Opts.SaveTemporaryLabels)
Index: clang/test/Misc/cc1as-darwin-target-variant-triple.s
===
--- /dev/null
+++ clang/test/Misc/cc1as-darwin-target-variant-triple.s
@@ -0,0 +1,34 @@
+// Run cc1as using darwin-target-variant-triple
+// REQUIRES: x86-registered-target
+// RUN: %clang -cc1as -triple x86_64-apple-macos10.9 -darwin-target-variant-triple x86_64-apple-ios13.1-macabi -filetype obj %s -o - \
+// RUN: | llvm-readobj --file-headers --macho-version-min - \
+// RUN: | FileCheck --check-prefix=CHECK %s
+
+// CHECK: File:
+// CHECK-NEXT: Format: Mach-O 64-bit x86-64
+// CHECK-NEXT: Arch: x86_64
+// CHECK-NEXT: AddressSize: 64bit
+// CHECK-NEXT: MachHeader {
+// CHECK-NEXT: Magic: Magic64 (0xFEEDFACF)
+// CHECK-NEXT: CpuType: X86-64 (0x107)
+// CHECK-NEXT: CpuSubType: CPU_SUBTYPE_X86_64_ALL (0x3)
+// CHECK-NEXT: FileType: Relocatable (0x1)
+// CHECK-NEXT: NumOfLoadCommands: 3
+// CHECK-NEXT: SizeOfLoadCommands: 192
+// CHECK-NEXT: Flags [ (0x0)
+// CHECK-NEXT: ]
+// CHECK-NEXT: Reserved: 0x0
+// CHECK-NEXT: }
+// CHECK-NEXT: MinVersion {
+// CHECK-NEXT: Cmd: LC_VERSION_MIN_MACOSX
+// CHECK-NEXT: Size: 16
+// CHECK-NEXT: Version: 10.9
+// CHECK-NEXT: SDK: n/a
+// CHECK-NEXT: }
+// CHECK-NEXT: MinVersion {
+// CHECK-NEXT: Cmd: LC_BUILD_VERSION
+// CHECK-NEXT: Size: 24
+// CHECK-NEXT: Platform: macCatalyst
+// CHECK-NEXT: Version: 13.1
+// CHECK-NEXT: SDK: n/a
+// CHECK-NEXT: }
Index: clang/lib/Driver/ToolChains/Darwin.h
===
--- clang/lib/Driver/ToolChains/Darwin.h
+++ clang/lib/Driver/ToolChains/Darwin.h
@@ -489,6 +489,12 @@
: TargetVersion) < VersionTuple(V0, V1, V2);
}
+ /// Returns the darwin target variant triple, the variant of the deployment
+ /// target for which the code is being compiled.
+ Optional getTargetVariantTriple() const override {
+return TargetVariantTriple;
+ }
+
protected:
/// Return true if c++17 aligned allocation/deallocation functions are not
/// implemented in the c++ standard library of the deployment target we are
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7719,6 +7719,8 @@
const llvm::Triple = getToolChain().getEffectiveTriple();
const std::string = Triple.getTriple();
+ const Optional TargetVariantTriple =
+ getToolChain().getTargetVariantTriple();
const auto = getToolChain().getDriver();
// Don't warn about "clang -w -c foo.s"
@@ -7736,6 +7738,10 @@
// Add the "effective" target triple.
CmdArgs.push_back("-triple");
CmdArgs.push_back(Args.MakeArgString(TripleStr));
+ if (TargetVariantTriple) {
+CmdArgs.push_back("-darwin-target-variant-triple");
+CmdArgs.push_back(Args.MakeArgString(TargetVariantTriple->getTriple()));
+ }
// Set the output mode, we currently only expect to be used as a real
// assembler.
Index: clang/include/clang/Driver/ToolChain.h
===
--- clang/include/clang/Driver/ToolChain.h
+++ clang/include/clang/Driver/ToolChain.h
@@ -717,6
[PATCH] D121868: [cc1as] Add support for emitting the build version load command for -darwin-target-variant
bc-lee added a comment. As you confirmed at https://crbug.com/1259122#c21, there is a problem with assembly files like `floatundidf.S` and this patch is intended to fix it. C/C++ files are fine. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121868/new/ https://reviews.llvm.org/D121868 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D121868: [cc1as] Add support for emitting the build version load command for -darwin-target-variant
bc-lee marked 3 inline comments as done. bc-lee added inline comments. Comment at: clang/test/Misc/cc1as-darwin-target-variant-triple.s:2 +// Run cc1as using darwin-target-variant-triple +// RUN: %clang -cc1as -triple x86_64-apple-macos10.9 -darwin-target-variant-triple x86_64-apple-ios13.1-macabi -filetype obj %s -o - \ +// RUN: | llvm-readobj --file-headers --macho-version-min - \ MaskRay wrote: > `-filetype obj` on the `%clang -cc1as` line seems weird. Without this, I cannot create object file and validate it using `llvm-readobj`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121868/new/ https://reviews.llvm.org/D121868 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D121868: [cc1as] Add support for emitting the build version load command for -darwin-target-variant
bc-lee updated this revision to Diff 423878.
bc-lee added a comment.
Address review comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121868/new/
https://reviews.llvm.org/D121868
Files:
clang/include/clang/Driver/Options.td
clang/include/clang/Driver/ToolChain.h
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Driver/ToolChains/Darwin.h
clang/test/Misc/cc1as-darwin-target-variant-triple.s
clang/tools/driver/cc1as_main.cpp
Index: clang/tools/driver/cc1as_main.cpp
===
--- clang/tools/driver/cc1as_main.cpp
+++ clang/tools/driver/cc1as_main.cpp
@@ -144,6 +144,9 @@
/// otherwise.
std::string TargetABI;
+ /// Darwin target variant triple, the variant of the deployment target
+ /// for which the code is being compiled.
+ llvm::Optional DarwinTargetVariantTriple;
/// @}
public:
@@ -209,6 +212,9 @@
// Target Options
Opts.Triple = llvm::Triple::normalize(Args.getLastArgValue(OPT_triple));
+ if (Arg *A = Args.getLastArg(options::OPT_darwin_target_variant_triple))
+Opts.DarwinTargetVariantTriple = llvm::Triple(A->getValue());
+
Opts.CPU = std::string(Args.getLastArgValue(OPT_target_cpu));
Opts.Features = Args.getAllArgValues(OPT_target_feature);
@@ -407,6 +413,8 @@
// MCObjectFileInfo needs a MCContext reference in order to initialize itself.
std::unique_ptr MOFI(
TheTarget->createMCObjectFileInfo(Ctx, PIC));
+ if (Opts.DarwinTargetVariantTriple)
+MOFI->setDarwinTargetVariantTriple(*Opts.DarwinTargetVariantTriple);
Ctx.setObjectFileInfo(MOFI.get());
if (Opts.SaveTemporaryLabels)
Index: clang/test/Misc/cc1as-darwin-target-variant-triple.s
===
--- /dev/null
+++ clang/test/Misc/cc1as-darwin-target-variant-triple.s
@@ -0,0 +1,33 @@
+// Run cc1as using darwin-target-variant-triple
+// RUN: %clang -cc1as -triple x86_64-apple-macos10.9 -darwin-target-variant-triple x86_64-apple-ios13.1-macabi -filetype obj %s -o - \
+// RUN: | llvm-readobj --file-headers --macho-version-min - \
+// RUN: | FileCheck --check-prefix=CHECK %s
+
+// CHECK: File:
+// CHECK-NEXT: Format: Mach-O 64-bit x86-64
+// CHECK-NEXT: Arch: x86_64
+// CHECK-NEXT: AddressSize: 64bit
+// CHECK-NEXT: MachHeader {
+// CHECK-NEXT: Magic: Magic64 (0xFEEDFACF)
+// CHECK-NEXT: CpuType: X86-64 (0x107)
+// CHECK-NEXT: CpuSubType: CPU_SUBTYPE_X86_64_ALL (0x3)
+// CHECK-NEXT: FileType: Relocatable (0x1)
+// CHECK-NEXT: NumOfLoadCommands: 3
+// CHECK-NEXT: SizeOfLoadCommands: 192
+// CHECK-NEXT: Flags [ (0x0)
+// CHECK-NEXT: ]
+// CHECK-NEXT: Reserved: 0x0
+// CHECK-NEXT: }
+// CHECK-NEXT: MinVersion {
+// CHECK-NEXT: Cmd: LC_VERSION_MIN_MACOSX
+// CHECK-NEXT: Size: 16
+// CHECK-NEXT: Version: 10.9
+// CHECK-NEXT: SDK: n/a
+// CHECK-NEXT: }
+// CHECK-NEXT: MinVersion {
+// CHECK-NEXT: Cmd: LC_BUILD_VERSION
+// CHECK-NEXT: Size: 24
+// CHECK-NEXT: Platform: macCatalyst
+// CHECK-NEXT: Version: 13.1
+// CHECK-NEXT: SDK: n/a
+// CHECK-NEXT: }
Index: clang/lib/Driver/ToolChains/Darwin.h
===
--- clang/lib/Driver/ToolChains/Darwin.h
+++ clang/lib/Driver/ToolChains/Darwin.h
@@ -489,6 +489,12 @@
: TargetVersion) < VersionTuple(V0, V1, V2);
}
+ /// Returns the darwin target variant triple, the variant of the deployment
+ /// target for which the code is being compiled.
+ Optional getTargetVariantTriple() const override {
+return TargetVariantTriple;
+ }
+
protected:
/// Return true if c++17 aligned allocation/deallocation functions are not
/// implemented in the c++ standard library of the deployment target we are
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7719,6 +7719,8 @@
const llvm::Triple = getToolChain().getEffectiveTriple();
const std::string = Triple.getTriple();
+ const Optional TargetVariantTriple =
+ getToolChain().getTargetVariantTriple();
const auto = getToolChain().getDriver();
// Don't warn about "clang -w -c foo.s"
@@ -7736,6 +7738,10 @@
// Add the "effective" target triple.
CmdArgs.push_back("-triple");
CmdArgs.push_back(Args.MakeArgString(TripleStr));
+ if (TargetVariantTriple) {
+CmdArgs.push_back("-darwin-target-variant-triple");
+CmdArgs.push_back(Args.MakeArgString(TargetVariantTriple->getTriple()));
+ }
// Set the output mode, we currently only expect to be used as a real
// assembler.
Index: clang/include/clang/Driver/ToolChain.h
===
--- clang/include/clang/Driver/ToolChain.h
+++ clang/include/clang/Driver/ToolChain.h
@@ -717,6 +717,10 @@
return llvm::DenormalMode::getIEEE();
}
+
[PATCH] D121868: [cc1as] Add support for emitting the build version load command for -darwin-target-variant
bc-lee added inline comments.
Comment at: clang/lib/Driver/ToolChains/Darwin.h:495
+ Optional getTargetVariantTriple() const override {
+return TargetVariantTriple;
+ }
MaskRay wrote:
> llvm::None
Do you mean to modify the method in `clang/include/clang/Driver/ToolChain.h`?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121868/new/
https://reviews.llvm.org/D121868
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D121868: [cc1as] Add support for emitting the build version load command for -darwin-target-variant
bc-lee added a comment. Hi, could you have a look at it once more? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121868/new/ https://reviews.llvm.org/D121868 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D121868: [cc1as] Add support for emitting the build version load command for -darwin-target-variant
bc-lee updated this revision to Diff 417595.
bc-lee added a comment.
Addressing review comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121868/new/
https://reviews.llvm.org/D121868
Files:
clang/include/clang/Driver/Options.td
clang/include/clang/Driver/ToolChain.h
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Driver/ToolChains/Darwin.h
clang/test/Misc/cc1as-darwin-target-variant-triple.s
clang/tools/driver/cc1as_main.cpp
Index: clang/tools/driver/cc1as_main.cpp
===
--- clang/tools/driver/cc1as_main.cpp
+++ clang/tools/driver/cc1as_main.cpp
@@ -144,6 +144,9 @@
/// otherwise.
std::string TargetABI;
+ /// Darwin target variant triple, the variant of the deployment target
+ /// for which the code is being compiled.
+ llvm::Optional DarwinTargetVariantTriple;
/// @}
public:
@@ -209,6 +212,9 @@
// Target Options
Opts.Triple = llvm::Triple::normalize(Args.getLastArgValue(OPT_triple));
+ if (Arg *A = Args.getLastArg(options::OPT_darwin_target_variant_triple))
+Opts.DarwinTargetVariantTriple = llvm::Triple(A->getValue());
+
Opts.CPU = std::string(Args.getLastArgValue(OPT_target_cpu));
Opts.Features = Args.getAllArgValues(OPT_target_feature);
@@ -407,6 +413,8 @@
// MCObjectFileInfo needs a MCContext reference in order to initialize itself.
std::unique_ptr MOFI(
TheTarget->createMCObjectFileInfo(Ctx, PIC));
+ if (Opts.DarwinTargetVariantTriple)
+MOFI->setDarwinTargetVariantTriple(*Opts.DarwinTargetVariantTriple);
Ctx.setObjectFileInfo(MOFI.get());
if (Opts.SaveTemporaryLabels)
Index: clang/test/Misc/cc1as-darwin-target-variant-triple.s
===
--- /dev/null
+++ clang/test/Misc/cc1as-darwin-target-variant-triple.s
@@ -0,0 +1,33 @@
+// Run cc1as using darwin-target-variant-triple
+// RUN: %clang -cc1as -triple x86_64-apple-macos10.9 -darwin-target-variant-triple x86_64-apple-ios13.1-macabi -filetype obj %s -o - \
+// RUN: | llvm-readobj --file-headers --macho-version-min - \
+// RUN: | FileCheck --check-prefix=CHECK %s
+
+// CHECK: File:
+// CHECK: Format: Mach-O 64-bit x86-64
+// CHECK: Arch: x86_64
+// CHECK: AddressSize: 64bit
+// CHECK: MachHeader {
+// CHECK: Magic: Magic64 (0xFEEDFACF)
+// CHECK: CpuType: X86-64 (0x107)
+// CHECK: CpuSubType: CPU_SUBTYPE_X86_64_ALL (0x3)
+// CHECK: FileType: Relocatable (0x1)
+// CHECK: NumOfLoadCommands: 3
+// CHECK: SizeOfLoadCommands: 192
+// CHECK: Flags [ (0x0)
+// CHECK: ]
+// CHECK: Reserved: 0x0
+// CHECK: }
+// CHECK: MinVersion {
+// CHECK: Cmd: LC_VERSION_MIN_MACOSX
+// CHECK: Size: 16
+// CHECK: Version: 10.9
+// CHECK: SDK: n/a
+// CHECK: }
+// CHECK: MinVersion {
+// CHECK: Cmd: LC_BUILD_VERSION
+// CHECK: Size: 24
+// CHECK: Platform: macCatalyst
+// CHECK: Version: 13.1
+// CHECK: SDK: n/a
+// CHECK: }
Index: clang/lib/Driver/ToolChains/Darwin.h
===
--- clang/lib/Driver/ToolChains/Darwin.h
+++ clang/lib/Driver/ToolChains/Darwin.h
@@ -489,6 +489,12 @@
: TargetVersion) < VersionTuple(V0, V1, V2);
}
+ /// Returns the darwin target variant triple, the variant of the deployment
+ /// target for which the code is being compiled.
+ Optional getTargetVariantTriple() const override {
+return TargetVariantTriple;
+ }
+
protected:
/// Return true if c++17 aligned allocation/deallocation functions are not
/// implemented in the c++ standard library of the deployment target we are
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7754,6 +7754,8 @@
const llvm::Triple = getToolChain().getEffectiveTriple();
const std::string = Triple.getTriple();
+ const Optional TargetVariantTriple =
+ getToolChain().getTargetVariantTriple();
const auto = getToolChain().getDriver();
// Don't warn about "clang -w -c foo.s"
@@ -7771,6 +7773,10 @@
// Add the "effective" target triple.
CmdArgs.push_back("-triple");
CmdArgs.push_back(Args.MakeArgString(TripleStr));
+ if (TargetVariantTriple) {
+CmdArgs.push_back("-darwin-target-variant-triple");
+CmdArgs.push_back(Args.MakeArgString(TargetVariantTriple->getTriple()));
+ }
// Set the output mode, we currently only expect to be used as a real
// assembler.
Index: clang/include/clang/Driver/ToolChain.h
===
--- clang/include/clang/Driver/ToolChain.h
+++ clang/include/clang/Driver/ToolChain.h
@@ -717,6 +717,10 @@
return llvm::DenormalMode::getIEEE();
}
+ virtual Optional getTargetVariantTriple() const {
+return llvm::Optional();
+ }
+
// We want to expand the shortened versions
[PATCH] D121868: [cc1as] Add support for emitting the build version load command for -darwin-target-variant
bc-lee created this revision.
bc-lee added reviewers: arphaman, MaskRay.
Herald added a project: All.
bc-lee requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
This patch extends cc1as to export the build version load command with
LC_VERSION_MIN_MACOSX.
This is especially important for Mac Catalyst as Mac Catalyst uses
the MacOS's compiler rt built-ins.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D121868
Files:
clang/include/clang/Driver/Options.td
clang/include/clang/Driver/ToolChain.h
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Driver/ToolChains/Darwin.h
clang/tools/driver/cc1as_main.cpp
Index: clang/tools/driver/cc1as_main.cpp
===
--- clang/tools/driver/cc1as_main.cpp
+++ clang/tools/driver/cc1as_main.cpp
@@ -144,6 +144,9 @@
/// otherwise.
std::string TargetABI;
+ /// Darwin target variant triple, the variant of the deployment target
+ /// for which the code is being compiled.
+ llvm::Optional DarwinTargetVariantTriple;
/// @}
public:
@@ -209,6 +212,11 @@
// Target Options
Opts.Triple = llvm::Triple::normalize(Args.getLastArgValue(OPT_triple));
+ if (Arg *A = Args.getLastArg(options::OPT_darwin_target_variant_triple)) {
+llvm::Triple TVT(A->getValue());
+Opts.DarwinTargetVariantTriple = TVT;
+ }
+
Opts.CPU = std::string(Args.getLastArgValue(OPT_target_cpu));
Opts.Features = Args.getAllArgValues(OPT_target_feature);
@@ -407,6 +415,9 @@
// MCObjectFileInfo needs a MCContext reference in order to initialize itself.
std::unique_ptr MOFI(
TheTarget->createMCObjectFileInfo(Ctx, PIC));
+ if (Opts.DarwinTargetVariantTriple) {
+MOFI->setDarwinTargetVariantTriple(*Opts.DarwinTargetVariantTriple);
+ }
Ctx.setObjectFileInfo(MOFI.get());
if (Opts.SaveTemporaryLabels)
Index: clang/lib/Driver/ToolChains/Darwin.h
===
--- clang/lib/Driver/ToolChains/Darwin.h
+++ clang/lib/Driver/ToolChains/Darwin.h
@@ -489,6 +489,12 @@
: TargetVersion) < VersionTuple(V0, V1, V2);
}
+ /// Returns the darwin target variant triple, the variant of the deployment
+ /// target for which the code is being compiled.
+ Optional getTargetVariantTriple() const override {
+return TargetVariantTriple;
+ }
+
protected:
/// Return true if c++17 aligned allocation/deallocation functions are not
/// implemented in the c++ standard library of the deployment target we are
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7754,6 +7754,8 @@
const llvm::Triple = getToolChain().getEffectiveTriple();
const std::string = Triple.getTriple();
+ const Optional TargetVariantTriple =
+ getToolChain().getTargetVariantTriple();
const auto = getToolChain().getDriver();
// Don't warn about "clang -w -c foo.s"
@@ -7771,6 +7773,10 @@
// Add the "effective" target triple.
CmdArgs.push_back("-triple");
CmdArgs.push_back(Args.MakeArgString(TripleStr));
+ if (TargetVariantTriple) {
+CmdArgs.push_back("-darwin-target-variant-triple");
+CmdArgs.push_back(Args.MakeArgString(TargetVariantTriple->getTriple()));
+ }
// Set the output mode, we currently only expect to be used as a real
// assembler.
Index: clang/include/clang/Driver/ToolChain.h
===
--- clang/include/clang/Driver/ToolChain.h
+++ clang/include/clang/Driver/ToolChain.h
@@ -717,6 +717,10 @@
return llvm::DenormalMode::getIEEE();
}
+ virtual Optional getTargetVariantTriple() const {
+return llvm::Optional();
+ }
+
// We want to expand the shortened versions of the triples passed in to
// the values used for the bitcode libraries.
static llvm::Triple getOpenMPTriple(StringRef TripleStr) {
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -4832,16 +4832,21 @@
MarshallingInfoString>;
def target_sdk_version_EQ : Joined<["-"], "target-sdk-version=">,
HelpText<"The version of target SDK used for compilation">;
-def darwin_target_variant_triple : Separate<["-"], "darwin-target-variant-triple">,
- HelpText<"Specify the darwin target variant triple">,
- MarshallingInfoString>,
- Normalizer<"normalizeTriple">;
def darwin_target_variant_sdk_version_EQ : Joined<["-"],
"darwin-target-variant-sdk-version=">,
HelpText<"The version of darwin target variant SDK used for compilation">;
} // let Flags = [CC1Option, CC1AsOption, NoDriverOption]
+let Flags = [CC1Option, CC1AsOption] in {
+
+def darwin_target_variant_triple : Separate<["-"],
[PATCH] D117924: [compiler_rt] Add a seperate runtime for Mac Catalyst
bc-lee abandoned this revision. bc-lee added a comment. Closed in favor of https://reviews.llvm.org/D118862 and https://reviews.llvm.org/D118875 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117924/new/ https://reviews.llvm.org/D117924 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D118875: [compiler-rt][builtins] build the macOS compiler-rt built-ins with Mac Catalyst support
bc-lee added a comment. Hi, I built and tested clang using this patch and https://reviews.llvm.org/D118862. It works great. The current patch adds the -darwin-target-variant flag to all object files for MacOS targets in the compiler_rt's builtin library. However, users building compiler_rt may have compilers that don't support such flag, so it would be nice to have a CMake option to enable compiler flags like `COMPILER_RT_ENABLE_CATALYST`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118875/new/ https://reviews.llvm.org/D118875 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D117924: [compiler_rt] Add a seperate runtime for Mac Catalyst
bc-lee updated this revision to Diff 405461.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D117924/new/
https://reviews.llvm.org/D117924
Files:
clang/lib/Driver/ToolChains/Darwin.cpp
clang/test/Driver/darwin-sanitizer-ld.c
compiler-rt/cmake/builtin-config-ix.cmake
compiler-rt/cmake/config-ix.cmake
Index: compiler-rt/cmake/config-ix.cmake
===
--- compiler-rt/cmake/config-ix.cmake
+++ compiler-rt/cmake/config-ix.cmake
@@ -304,7 +304,7 @@
if ("${platform}" STREQUAL "")
message(FATAL_ERROR "platform cannot be empty")
endif()
- if ("${platform}" MATCHES "^(osx|((ios|watchos|tvos)(sim)?))$")
+ if ("${platform}" MATCHES "^(osx|((ios|watchos|tvos)(sim)?)|catalyst)$")
set(is_valid TRUE)
endif()
set(${is_valid_out} ${is_valid} PARENT_SCOPE)
@@ -341,6 +341,7 @@
include(CompilerRTDarwinUtils)
find_darwin_sdk_dir(DARWIN_osx_SYSROOT macosx)
+ find_darwin_sdk_dir(DARWIN_catalyst_SYSROOT macosx)
find_darwin_sdk_dir(DARWIN_iossim_SYSROOT iphonesimulator)
find_darwin_sdk_dir(DARWIN_ios_SYSROOT iphoneos)
find_darwin_sdk_dir(DARWIN_watchossim_SYSROOT watchsimulator)
@@ -366,6 +367,12 @@
set(DARWIN_iossim_MIN_VER_FLAG -mios-simulator-version-min)
set(DARWIN_iossim_SANITIZER_MIN_VER_FLAG
${DARWIN_iossim_MIN_VER_FLAG}=${DARWIN_ios_MIN_VER})
+list(APPEND DARWIN_EMBEDDED_PLATFORMS catalyst)
+set(DARWIN_catalyst_MIN_VER 13.0)
+set(DARWIN_catalyst_SANITIZER_MIN_VER_FLAG
+${DARWIN_ios_MIN_VER_FLAG}=${DARWIN_catalyst_MIN_VER}
+-target
+${LIB_ARCH}-apple-ios${DARWIN_catalyst_MIN_VER}-macabi)
endif()
if(COMPILER_RT_ENABLE_WATCHOS)
list(APPEND DARWIN_EMBEDDED_PLATFORMS watchos)
@@ -493,6 +500,10 @@
endforeach()
endif()
+ if("${platform}" STREQUAL "catalyst")
+set(DARWIN_${platform}_SKIP_CC_KEXT On)
+ endif()
+
if(DARWIN_${platform}_SYSROOT)
set(DARWIN_${platform}_CFLAGS
${DARWIN_COMMON_CFLAGS}
Index: compiler-rt/cmake/builtin-config-ix.cmake
===
--- compiler-rt/cmake/builtin-config-ix.cmake
+++ compiler-rt/cmake/builtin-config-ix.cmake
@@ -71,6 +71,7 @@
if(APPLE)
find_darwin_sdk_dir(DARWIN_osx_SYSROOT macosx)
+ find_darwin_sdk_dir(DARWIN_catalyst_SYSROOT macosx)
find_darwin_sdk_dir(DARWIN_iossim_SYSROOT iphonesimulator)
find_darwin_sdk_dir(DARWIN_ios_SYSROOT iphoneos)
find_darwin_sdk_dir(DARWIN_watchossim_SYSROOT watchsimulator)
@@ -116,6 +117,13 @@
${DARWIN_ios_MIN_VER_FLAG}=${DARWIN_ios_BUILTIN_MIN_VER})
set(DARWIN_ios_BUILTIN_ALL_POSSIBLE_ARCHS ${ARM64} ${ARM32})
set(DARWIN_iossim_BUILTIN_ALL_POSSIBLE_ARCHS ${X86} ${X86_64} arm64)
+list(APPEND DARWIN_EMBEDDED_PLATFORMS catalyst)
+set(DARWIN_catalyst_BUILTIN_MIN_VER 13.0)
+set(DARWIN_catalyst_BUILTIN_MIN_VER_FLAG
+${DARWIN_ios_MIN_VER_FLAG}=${DARWIN_catalyst_BUILTIN_MIN_VER}
+-target
+${LIB_ARCH}-apple-ios${DARWIN_catalyst_BUILTIN_MIN_VER}-macabi)
+set(DARWIN_catalyst_BUILTIN_ALL_POSSIBLE_ARCHS ${X86_64} ${ARM64})
endif()
if(COMPILER_RT_ENABLE_WATCHOS)
list(APPEND DARWIN_EMBEDDED_PLATFORMS watchos)
@@ -178,6 +186,10 @@
endforeach()
endif()
+if("${platform}" STREQUAL "catalyst")
+ set(DARWIN_${platform}_SKIP_CC_KEXT On)
+endif()
+
if(DARWIN_${platform}_SYSROOT)
darwin_test_archs(${platform}
DARWIN_${platform}_BUILTIN_ARCHS
Index: clang/test/Driver/darwin-sanitizer-ld.c
===
--- clang/test/Driver/darwin-sanitizer-ld.c
+++ clang/test/Driver/darwin-sanitizer-ld.c
@@ -101,7 +101,7 @@
// CHECK-ASAN-MACCATALYST: "{{.*}}ld{{(.exe)?}}"
// CHECK-ASAN-MACCATALYST-NOT: "-lstdc++"
// CHECK-ASAN-MACCATALYST-NOT: "-lc++"
-// CHECK-ASAN-MACCATALYST: libclang_rt.asan_osx_dynamic.dylib"
+// CHECK-ASAN-MACCATALYST: libclang_rt.asan_catalyst_dynamic.dylib"
// CHECK-ASAN-MACCATALYST: "-rpath" "@executable_path"
// CHECK-ASAN-MACCATALYST: "-rpath" "{{.*}}lib{{.*}}darwin"
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1219,7 +1219,7 @@
return "osx";
case DarwinPlatformKind::IPhoneOS:
if (TargetEnvironment == MacCatalyst)
- return "osx";
+ return "catalyst";
return TargetEnvironment == NativeEnvironment || IgnoreSim ? "ios"
: "iossim";
case DarwinPlatformKind::TvOS:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D117924: [compiler_rt] Add a seperate runtime for Mac Catalyst
bc-lee added a comment. Some changes in this patch were applied as https://reviews.llvm.org/D118759, so I'm re-uploading this patch to get rid of merge conflicts. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117924/new/ https://reviews.llvm.org/D117924 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D117924: [compiler_rt] Add a seperate runtime for Mac Catalyst
bc-lee added a comment. For the Chromium side issue is https://crbug.com/1259122. Also I opened LLVM issue https://github.com/llvm/llvm-project/issues/53477. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117924/new/ https://reviews.llvm.org/D117924 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D117924: [compiler_rt] Add a seperate runtime for Mac Catalyst
bc-lee added a comment. It may not be appropriate to add other runtime libraries specifically for Mac Catalyst. However, currently `lld` does not allow linking with dynamic libraries with different types of build_version_command, whereas `ld64` does allow for Mac Catalyst. Considering compatibility with lld, Mac Catalyst’s runtime libraries need to be added also. See my other patch https://reviews.llvm.org/D117925 for more details. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117924/new/ https://reviews.llvm.org/D117924 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D117924: [compiler_rt] Add a seperate runtime for Mac Catalyst
bc-lee updated this revision to Diff 402201.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D117924/new/
https://reviews.llvm.org/D117924
Files:
clang/lib/Driver/ToolChains/Darwin.cpp
clang/test/Driver/darwin-sanitizer-ld.c
compiler-rt/cmake/builtin-config-ix.cmake
compiler-rt/cmake/config-ix.cmake
Index: compiler-rt/cmake/config-ix.cmake
===
--- compiler-rt/cmake/config-ix.cmake
+++ compiler-rt/cmake/config-ix.cmake
@@ -310,7 +310,7 @@
if ("${platform}" STREQUAL "")
message(FATAL_ERROR "platform cannot be empty")
endif()
- if ("${platform}" MATCHES "^(osx|((ios|watchos|tvos)(sim)?))$")
+ if ("${platform}" MATCHES "^(osx|((ios|watchos|tvos)(sim)?)|catalyst)$")
set(is_valid TRUE)
endif()
set(${is_valid_out} ${is_valid} PARENT_SCOPE)
@@ -347,6 +347,7 @@
include(CompilerRTDarwinUtils)
find_darwin_sdk_dir(DARWIN_osx_SYSROOT macosx)
+ find_darwin_sdk_dir(DARWIN_catalyst_SYSROOT macosx)
find_darwin_sdk_dir(DARWIN_iossim_SYSROOT iphonesimulator)
find_darwin_sdk_dir(DARWIN_ios_SYSROOT iphoneos)
find_darwin_sdk_dir(DARWIN_watchossim_SYSROOT watchsimulator)
@@ -372,6 +373,12 @@
set(DARWIN_iossim_MIN_VER_FLAG -mios-simulator-version-min)
set(DARWIN_iossim_SANITIZER_MIN_VER_FLAG
${DARWIN_iossim_MIN_VER_FLAG}=${DARWIN_ios_MIN_VER})
+list(APPEND DARWIN_EMBEDDED_PLATFORMS catalyst)
+set(DARWIN_catalyst_MIN_VER 13.0)
+set(DARWIN_catalyst_SANITIZER_MIN_VER_FLAG
+${DARWIN_ios_MIN_VER_FLAG}=${DARWIN_catalyst_MIN_VER}
+-target
+${LIB_ARCH}-apple-ios${DARWIN_catalyst_MIN_VER}-macabi)
endif()
if(COMPILER_RT_ENABLE_WATCHOS)
list(APPEND DARWIN_EMBEDDED_PLATFORMS watchos)
@@ -499,6 +506,10 @@
endforeach()
endif()
+ if("${platform}" STREQUAL "catalyst")
+set(DARWIN_${platform}_SKIP_CC_KEXT On)
+ endif()
+
if(DARWIN_${platform}_SYSROOT)
set(DARWIN_${platform}_CFLAGS
${DARWIN_COMMON_CFLAGS}
Index: compiler-rt/cmake/builtin-config-ix.cmake
===
--- compiler-rt/cmake/builtin-config-ix.cmake
+++ compiler-rt/cmake/builtin-config-ix.cmake
@@ -71,6 +71,7 @@
if(APPLE)
find_darwin_sdk_dir(DARWIN_osx_SYSROOT macosx)
+ find_darwin_sdk_dir(DARWIN_catalyst_SYSROOT macosx)
find_darwin_sdk_dir(DARWIN_iossim_SYSROOT iphonesimulator)
find_darwin_sdk_dir(DARWIN_ios_SYSROOT iphoneos)
find_darwin_sdk_dir(DARWIN_watchossim_SYSROOT watchsimulator)
@@ -115,7 +116,14 @@
set(DARWIN_ios_BUILTIN_MIN_VER_FLAG
${DARWIN_ios_MIN_VER_FLAG}=${DARWIN_ios_BUILTIN_MIN_VER})
set(DARWIN_ios_BUILTIN_ALL_POSSIBLE_ARCHS ${ARM64} ${ARM32})
-set(DARWIN_iossim_BUILTIN_ALL_POSSIBLE_ARCHS ${X86} ${X86_64})
+set(DARWIN_iossim_BUILTIN_ALL_POSSIBLE_ARCHS ${X86} ${X86_64} ${ARM64})
+list(APPEND DARWIN_EMBEDDED_PLATFORMS catalyst)
+set(DARWIN_catalyst_BUILTIN_MIN_VER 13.0)
+set(DARWIN_catalyst_BUILTIN_MIN_VER_FLAG
+${DARWIN_ios_MIN_VER_FLAG}=${DARWIN_catalyst_BUILTIN_MIN_VER}
+-target
+${LIB_ARCH}-apple-ios${DARWIN_catalyst_BUILTIN_MIN_VER}-macabi)
+set(DARWIN_catalyst_BUILTIN_ALL_POSSIBLE_ARCHS ${X86_64} ${ARM64})
endif()
if(COMPILER_RT_ENABLE_WATCHOS)
list(APPEND DARWIN_EMBEDDED_PLATFORMS watchos)
@@ -178,6 +186,10 @@
endforeach()
endif()
+if("${platform}" STREQUAL "catalyst")
+ set(DARWIN_${platform}_SKIP_CC_KEXT On)
+endif()
+
if(DARWIN_${platform}_SYSROOT)
darwin_test_archs(${platform}
DARWIN_${platform}_BUILTIN_ARCHS
Index: clang/test/Driver/darwin-sanitizer-ld.c
===
--- clang/test/Driver/darwin-sanitizer-ld.c
+++ clang/test/Driver/darwin-sanitizer-ld.c
@@ -101,7 +101,7 @@
// CHECK-ASAN-MACCATALYST: "{{.*}}ld{{(.exe)?}}"
// CHECK-ASAN-MACCATALYST-NOT: "-lstdc++"
// CHECK-ASAN-MACCATALYST-NOT: "-lc++"
-// CHECK-ASAN-MACCATALYST: libclang_rt.asan_osx_dynamic.dylib"
+// CHECK-ASAN-MACCATALYST: libclang_rt.asan_catalyst_dynamic.dylib"
// CHECK-ASAN-MACCATALYST: "-rpath" "@executable_path"
// CHECK-ASAN-MACCATALYST: "-rpath" "{{.*}}lib{{.*}}darwin"
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1219,7 +1219,7 @@
return "osx";
case DarwinPlatformKind::IPhoneOS:
if (TargetEnvironment == MacCatalyst)
- return "osx";
+ return "catalyst";
return TargetEnvironment == NativeEnvironment || IgnoreSim ? "ios"
: "iossim";
case DarwinPlatformKind::TvOS:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
[PATCH] D117924: [compiler_rt] Add a seperate runtime for Mac Catalyst
bc-lee created this revision.
Herald added a subscriber: mgorny.
bc-lee requested review of this revision.
Herald added projects: clang, Sanitizers.
Herald added subscribers: Sanitizers, cfe-commits.
Mac Catalyst has different platform fields in build_version_command,
so it's natural to create a separate libclang_rt.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D117924
Files:
clang/lib/Driver/ToolChains/Darwin.cpp
compiler-rt/cmake/builtin-config-ix.cmake
compiler-rt/cmake/config-ix.cmake
Index: compiler-rt/cmake/config-ix.cmake
===
--- compiler-rt/cmake/config-ix.cmake
+++ compiler-rt/cmake/config-ix.cmake
@@ -310,7 +310,7 @@
if ("${platform}" STREQUAL "")
message(FATAL_ERROR "platform cannot be empty")
endif()
- if ("${platform}" MATCHES "^(osx|((ios|watchos|tvos)(sim)?))$")
+ if ("${platform}" MATCHES "^(osx|((ios|watchos|tvos)(sim)?)|catalyst)$")
set(is_valid TRUE)
endif()
set(${is_valid_out} ${is_valid} PARENT_SCOPE)
@@ -347,6 +347,7 @@
include(CompilerRTDarwinUtils)
find_darwin_sdk_dir(DARWIN_osx_SYSROOT macosx)
+ find_darwin_sdk_dir(DARWIN_catalyst_SYSROOT macosx)
find_darwin_sdk_dir(DARWIN_iossim_SYSROOT iphonesimulator)
find_darwin_sdk_dir(DARWIN_ios_SYSROOT iphoneos)
find_darwin_sdk_dir(DARWIN_watchossim_SYSROOT watchsimulator)
@@ -372,6 +373,12 @@
set(DARWIN_iossim_MIN_VER_FLAG -mios-simulator-version-min)
set(DARWIN_iossim_SANITIZER_MIN_VER_FLAG
${DARWIN_iossim_MIN_VER_FLAG}=${DARWIN_ios_MIN_VER})
+list(APPEND DARWIN_EMBEDDED_PLATFORMS catalyst)
+set(DARWIN_catalyst_MIN_VER 13.0)
+set(DARWIN_catalyst_SANITIZER_MIN_VER_FLAG
+${DARWIN_ios_MIN_VER_FLAG}=${DARWIN_catalyst_MIN_VER}
+-target
+${LIB_ARCH}-apple-ios${DARWIN_catalyst_MIN_VER}-macabi)
endif()
if(COMPILER_RT_ENABLE_WATCHOS)
list(APPEND DARWIN_EMBEDDED_PLATFORMS watchos)
@@ -499,6 +506,10 @@
endforeach()
endif()
+ if("${platform}" STREQUAL "catalyst")
+set(DARWIN_${platform}_SKIP_CC_KEXT On)
+ endif()
+
if(DARWIN_${platform}_SYSROOT)
set(DARWIN_${platform}_CFLAGS
${DARWIN_COMMON_CFLAGS}
Index: compiler-rt/cmake/builtin-config-ix.cmake
===
--- compiler-rt/cmake/builtin-config-ix.cmake
+++ compiler-rt/cmake/builtin-config-ix.cmake
@@ -71,6 +71,7 @@
if(APPLE)
find_darwin_sdk_dir(DARWIN_osx_SYSROOT macosx)
+ find_darwin_sdk_dir(DARWIN_catalyst_SYSROOT macosx)
find_darwin_sdk_dir(DARWIN_iossim_SYSROOT iphonesimulator)
find_darwin_sdk_dir(DARWIN_ios_SYSROOT iphoneos)
find_darwin_sdk_dir(DARWIN_watchossim_SYSROOT watchsimulator)
@@ -115,7 +116,14 @@
set(DARWIN_ios_BUILTIN_MIN_VER_FLAG
${DARWIN_ios_MIN_VER_FLAG}=${DARWIN_ios_BUILTIN_MIN_VER})
set(DARWIN_ios_BUILTIN_ALL_POSSIBLE_ARCHS ${ARM64} ${ARM32})
-set(DARWIN_iossim_BUILTIN_ALL_POSSIBLE_ARCHS ${X86} ${X86_64})
+set(DARWIN_iossim_BUILTIN_ALL_POSSIBLE_ARCHS ${X86} ${X86_64} ${ARM64})
+list(APPEND DARWIN_EMBEDDED_PLATFORMS catalyst)
+set(DARWIN_catalyst_BUILTIN_MIN_VER 13.0)
+set(DARWIN_catalyst_BUILTIN_MIN_VER_FLAG
+${DARWIN_ios_MIN_VER_FLAG}=${DARWIN_catalyst_BUILTIN_MIN_VER}
+-target
+${LIB_ARCH}-apple-ios${DARWIN_catalyst_BUILTIN_MIN_VER}-macabi)
+set(DARWIN_catalyst_BUILTIN_ALL_POSSIBLE_ARCHS ${X86_64} ${ARM64})
endif()
if(COMPILER_RT_ENABLE_WATCHOS)
list(APPEND DARWIN_EMBEDDED_PLATFORMS watchos)
@@ -178,6 +186,10 @@
endforeach()
endif()
+if("${platform}" STREQUAL "catalyst")
+ set(DARWIN_${platform}_SKIP_CC_KEXT On)
+endif()
+
if(DARWIN_${platform}_SYSROOT)
darwin_test_archs(${platform}
DARWIN_${platform}_BUILTIN_ARCHS
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1219,7 +1219,7 @@
return "osx";
case DarwinPlatformKind::IPhoneOS:
if (TargetEnvironment == MacCatalyst)
- return "osx";
+ return "catalyst";
return TargetEnvironment == NativeEnvironment || IgnoreSim ? "ios"
: "iossim";
case DarwinPlatformKind::TvOS:
Index: compiler-rt/cmake/config-ix.cmake
===
--- compiler-rt/cmake/config-ix.cmake
+++ compiler-rt/cmake/config-ix.cmake
@@ -310,7 +310,7 @@
if ("${platform}" STREQUAL "")
message(FATAL_ERROR "platform cannot be empty")
endif()
- if ("${platform}" MATCHES "^(osx|((ios|watchos|tvos)(sim)?))$")
+ if ("${platform}" MATCHES "^(osx|((ios|watchos|tvos)(sim)?)|catalyst)$")
set(is_valid TRUE)
endif()
set(${is_valid_out} ${is_valid} PARENT_SCOPE)
@@ -347,6 +347,7 @@
[PATCH] D87201: [clang-format] Add a option for the position of Java static import
bc-lee added a comment. Thanks for the review. Since I don't have commit access, so I want someone else to apply this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87201/new/ https://reviews.llvm.org/D87201 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D87201: [clang-format] Add a option for the position of Java static import
bc-lee added a comment. Can someone commit my changes on behalf of it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87201/new/ https://reviews.llvm.org/D87201 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D87201: [clang-format] Add a option for the position of Java static import
bc-lee marked an inline comment as done. bc-lee added a comment. It's okay for some reviewers to make this change on my behalf. Thanks for reviewing this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87201/new/ https://reviews.llvm.org/D87201 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D87201: [clang-format] Add a option for the position of Java static import
bc-lee added inline comments.
Comment at: clang/docs/ClangFormatStyleOptions.rst:1970
+ but this behavior is changed by another option,
+ ``JavaStaticImportAfterImport``.
MyDeveloperDay wrote:
> Can you add a test that shows if the sorting is still in the groups, i.e. I
> can't tell if
>
> ```
> import com.test.a;
> import static org.a;
>
> import com.test.b;
> import static org.b;
> ```
>
> becomes
>
> ```
> import static org.a;
> import static org.b;
> import com.test.a;
>
> import com.test.b;
> ```
>
> or
>
> ```
> import static org.a;
> import static org.b;
>
> import com.test.a;
>
> import com.test.b;
> ```
>
> or
>
> ```
> import static org.a;
> import com.test.a;
>
> import static org.b;
> import com.test.b;
>
> ```
>
>
>
>
>
Done.
Comment at: clang/include/clang/Format/Format.h:1708
+ /// \endcode
+ bool JavaStaticImportAfterImport;
+
JakeMerdichAMD wrote:
> MyDeveloperDay wrote:
> > Can we consider changing the name or the option to make it clearer what its
> > for?
> >
> > `SortJavaStaticImport`
> >
> > and make it an enumeration rather than true/false
> >
> > `Before,After,Never`
> >
> > There need to be a "don't touch it option (.e.g. Never)" that does what it
> > does today (and that should be the default, otherwise we end up causing
> > clang-format to change ALL java code" which could be 100's of millions of
> > lines+
> >
> >
> I'm confused, there is not currently a never option... Right now the behavior
> is always 'before', there is no 'after' or 'never'.
>
> Making it an enum would probably be more ergonomic though, even there is no
> never option.
If `SortIncludes` is `true`, it doesn't make sense to not touch Java static
include.
Making it an enum is also good for me.
Comment at: clang/unittests/Format/SortImportsTestJava.cpp:253
+TEST_F(SortImportsTestJava, FormatJavaStaticImportAfterImport) {
+ FmtStyle.JavaStaticImportAfterImport = true;
JakeMerdichAMD wrote:
> MyDeveloperDay wrote:
> > please test all options
> >
> > You are also missing a test for checking the PARSING of the options
> Parsing test was already noted and done (unless this changes to an enum of
> course). But testing the 'false' case here makes sense.
Done
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87201/new/
https://reviews.llvm.org/D87201
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D87201: [clang-format] Add a option for the position of Java static import
bc-lee updated this revision to Diff 290619.
bc-lee added a comment.
Add more tests and rename options.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87201/new/
https://reviews.llvm.org/D87201
Files:
clang/docs/ClangFormatStyleOptions.rst
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/SortImportsTestJava.cpp
Index: clang/unittests/Format/SortImportsTestJava.cpp
===
--- clang/unittests/Format/SortImportsTestJava.cpp
+++ clang/unittests/Format/SortImportsTestJava.cpp
@@ -250,6 +250,62 @@
"import org.c;\n"));
}
+TEST_F(SortImportsTestJava, SortJavaStaticImport) {
+ FmtStyle.SortJavaStaticImport = FormatStyle::SJSIO_Before;
+ EXPECT_EQ("import static com.test.a;\n"
+"\n"
+"import static org.a;\n"
+"\n"
+"import static com.a;\n"
+"\n"
+"import com.test.b;\n"
+"\n"
+"import org.b;\n"
+"\n"
+"import com.b;\n",
+sort("import static com.test.a;\n"
+ "import static org.a;\n"
+ "import static com.a;\n"
+ "import com.test.b;\n"
+ "import org.b;\n"
+ "import com.b;\n"));
+
+ FmtStyle.SortJavaStaticImport = FormatStyle::SJSIO_After;
+ EXPECT_EQ("import com.test.b;\n"
+"import com.test.c;\n"
+"\n"
+"import org.b;\n"
+"\n"
+"import com.b;\n"
+"\n"
+"import static com.test.a;\n"
+"\n"
+"import static org.a;\n"
+"\n"
+"import static com.a;\n",
+sort("import static com.test.a;\n"
+ "import static org.a;\n"
+ "import static com.a;\n"
+ "import com.test.b;\n"
+ "import org.b;\n"
+ "import com.b;\n"
+ "import com.test.c;\n"));
+}
+
+TEST_F(SortImportsTestJava, SortJavaStaticImportAsGroup) {
+ FmtStyle.SortJavaStaticImport = FormatStyle::SJSIO_After;
+
+ EXPECT_EQ("import com.test.a;\n"
+"import com.test.b;\n"
+"\n"
+"import static org.a;\n"
+"import static org.b;\n",
+sort("import com.test.a;\n"
+ "import static org.a;\n"
+ "import com.test.b;\n"
+ "import static org.b;\n"));
+}
+
TEST_F(SortImportsTestJava, DeduplicateImports) {
EXPECT_EQ("import org.a;\n", sort("import org.a;\n"
"import org.a;\n"));
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14276,6 +14276,12 @@
CHECK_PARSE("BitFieldColonSpacing: After", BitFieldColonSpacing,
FormatStyle::BFCS_After);
+ Style.SortJavaStaticImport = FormatStyle::SJSIO_Before;
+ CHECK_PARSE("SortJavaStaticImport: After", SortJavaStaticImport,
+ FormatStyle::SJSIO_After);
+ CHECK_PARSE("SortJavaStaticImport: Before", SortJavaStaticImport,
+ FormatStyle::SJSIO_Before);
+
// FIXME: This is required because parsing a configuration simply overwrites
// the first N elements of the list instead of resetting it.
Style.ForEachMacros.clear();
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -377,6 +377,15 @@
}
};
+template <>
+struct ScalarEnumerationTraits {
+ static void enumeration(IO ,
+ FormatStyle::SortJavaStaticImportOptions ) {
+IO.enumCase(Value, "Before", FormatStyle::SJSIO_Before);
+IO.enumCase(Value, "After", FormatStyle::SJSIO_After);
+ }
+};
+
template <> struct MappingTraits {
static void mapping(IO , FormatStyle ) {
// When reading, read the language first, we need it for getPredefinedStyle.
@@ -574,6 +583,7 @@
IO.mapOptional("RawStringFormats", Style.RawStringFormats);
IO.mapOptional("ReflowComments", Style.ReflowComments);
IO.mapOptional("SortIncludes", Style.SortIncludes);
+IO.mapOptional("SortJavaStaticImport", Style.SortJavaStaticImport);
IO.mapOptional("SortUsingDeclarations", Style.SortUsingDeclarations);
IO.mapOptional("SpaceAfterCStyleCast", Style.SpaceAfterCStyleCast);
IO.mapOptional("SpaceAfterLogicalNot", Style.SpaceAfterLogicalNot);
@@ -947,6 +957,7 @@
LLVMStyle.DisableFormat = false;
LLVMStyle.SortIncludes = true;
+ LLVMStyle.SortJavaStaticImport = FormatStyle::SJSIO_Before;
LLVMStyle.SortUsingDeclarations = true;
LLVMStyle.StatementMacros.push_back("Q_UNUSED");
LLVMStyle.StatementMacros.push_back("QT_REQUIRE_VERSION");
@@
[PATCH] D87201: [clang-format] Add a option for the position of Java static import
bc-lee updated this revision to Diff 290381.
bc-lee added a comment.
Fix the example to match the description.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87201/new/
https://reviews.llvm.org/D87201
Files:
clang/docs/ClangFormatStyleOptions.rst
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/SortImportsTestJava.cpp
Index: clang/unittests/Format/SortImportsTestJava.cpp
===
--- clang/unittests/Format/SortImportsTestJava.cpp
+++ clang/unittests/Format/SortImportsTestJava.cpp
@@ -250,6 +250,30 @@
"import org.c;\n"));
}
+TEST_F(SortImportsTestJava, FormatJavaStaticImportAfterImport) {
+ FmtStyle.JavaStaticImportAfterImport = true;
+
+ EXPECT_EQ("import com.test.b;\n"
+"import com.test.c;\n"
+"\n"
+"import org.b;\n"
+"\n"
+"import com.b;\n"
+"\n"
+"import static com.test.a;\n"
+"\n"
+"import static org.a;\n"
+"\n"
+"import static com.a;\n",
+sort("import static com.test.a;\n"
+ "import static org.a;\n"
+ "import static com.a;\n"
+ "import com.test.b;\n"
+ "import org.b;\n"
+ "import com.b;\n"
+ "import com.test.c;\n"));
+}
+
TEST_F(SortImportsTestJava, DeduplicateImports) {
EXPECT_EQ("import org.a;\n", sort("import org.a;\n"
"import org.a;\n"));
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13929,6 +13929,7 @@
CHECK_PARSE_BOOL(IndentCaseBlocks);
CHECK_PARSE_BOOL(IndentGotoLabels);
CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
+ CHECK_PARSE_BOOL(JavaStaticImportAfterImport);
CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
CHECK_PARSE_BOOL(ObjCSpaceBeforeProtocolList);
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -544,6 +544,8 @@
IO.mapOptional("JavaImportGroups", Style.JavaImportGroups);
IO.mapOptional("JavaScriptQuotes", Style.JavaScriptQuotes);
IO.mapOptional("JavaScriptWrapImports", Style.JavaScriptWrapImports);
+IO.mapOptional("JavaStaticImportAfterImport",
+ Style.JavaStaticImportAfterImport);
IO.mapOptional("KeepEmptyLinesAtTheStartOfBlocks",
Style.KeepEmptyLinesAtTheStartOfBlocks);
IO.mapOptional("MacroBlockBegin", Style.MacroBlockBegin);
@@ -901,6 +903,7 @@
LLVMStyle.InsertTrailingCommas = FormatStyle::TCS_None;
LLVMStyle.JavaScriptQuotes = FormatStyle::JSQS_Leave;
LLVMStyle.JavaScriptWrapImports = true;
+ LLVMStyle.JavaStaticImportAfterImport = false;
LLVMStyle.TabWidth = 8;
LLVMStyle.MaxEmptyLinesToKeep = 1;
LLVMStyle.KeepEmptyLinesAtTheStartOfBlocks = true;
@@ -2312,12 +2315,15 @@
JavaImportGroups.push_back(
findJavaImportGroup(Style, Imports[i].Identifier));
}
+ bool StaticImportAfterNormalImport = Style.JavaStaticImportAfterImport;
llvm::sort(Indices, [&](unsigned LHSI, unsigned RHSI) {
// Negating IsStatic to push static imports above non-static imports.
-return std::make_tuple(!Imports[LHSI].IsStatic, JavaImportGroups[LHSI],
- Imports[LHSI].Identifier) <
- std::make_tuple(!Imports[RHSI].IsStatic, JavaImportGroups[RHSI],
- Imports[RHSI].Identifier);
+return std::make_tuple(!Imports[LHSI].IsStatic ^
+ StaticImportAfterNormalImport,
+ JavaImportGroups[LHSI], Imports[LHSI].Identifier) <
+ std::make_tuple(!Imports[RHSI].IsStatic ^
+ StaticImportAfterNormalImport,
+ JavaImportGroups[RHSI], Imports[RHSI].Identifier);
});
// Deduplicate imports.
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1618,10 +1618,12 @@
/// A vector of prefixes ordered by the desired groups for Java imports.
///
- /// Each group is separated by a newline. Static imports will also follow the
- /// same grouping convention above all non-static imports. One group's prefix
- /// can be a subset of another - the longest prefix is always matched. Within
- /// a group, the imports are ordered lexicographically.
+ /// One group's prefix can be a subset of another - the longest prefix is
+ /// always matched. Within a group, the imports
[PATCH] D87201: [clang-format] Add a option for the position of Java static import
bc-lee updated this revision to Diff 290303.
bc-lee added a comment.
Some comments have been corrected and a unittest has been added in
FormatTest.ParsesConfigurationBools
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87201/new/
https://reviews.llvm.org/D87201
Files:
clang/docs/ClangFormatStyleOptions.rst
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/SortImportsTestJava.cpp
Index: clang/unittests/Format/SortImportsTestJava.cpp
===
--- clang/unittests/Format/SortImportsTestJava.cpp
+++ clang/unittests/Format/SortImportsTestJava.cpp
@@ -250,6 +250,30 @@
"import org.c;\n"));
}
+TEST_F(SortImportsTestJava, FormatJavaStaticImportAfterImport) {
+ FmtStyle.JavaStaticImportAfterImport = true;
+
+ EXPECT_EQ("import com.test.b;\n"
+"import com.test.c;\n"
+"\n"
+"import org.b;\n"
+"\n"
+"import com.b;\n"
+"\n"
+"import static com.test.a;\n"
+"\n"
+"import static org.a;\n"
+"\n"
+"import static com.a;\n",
+sort("import static com.test.a;\n"
+ "import static org.a;\n"
+ "import static com.a;\n"
+ "import com.test.b;\n"
+ "import org.b;\n"
+ "import com.b;\n"
+ "import com.test.c;\n"));
+}
+
TEST_F(SortImportsTestJava, DeduplicateImports) {
EXPECT_EQ("import org.a;\n", sort("import org.a;\n"
"import org.a;\n"));
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13929,6 +13929,7 @@
CHECK_PARSE_BOOL(IndentCaseBlocks);
CHECK_PARSE_BOOL(IndentGotoLabels);
CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
+ CHECK_PARSE_BOOL(JavaStaticImportAfterImport);
CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
CHECK_PARSE_BOOL(ObjCSpaceBeforeProtocolList);
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -544,6 +544,8 @@
IO.mapOptional("JavaImportGroups", Style.JavaImportGroups);
IO.mapOptional("JavaScriptQuotes", Style.JavaScriptQuotes);
IO.mapOptional("JavaScriptWrapImports", Style.JavaScriptWrapImports);
+IO.mapOptional("JavaStaticImportAfterImport",
+ Style.JavaStaticImportAfterImport);
IO.mapOptional("KeepEmptyLinesAtTheStartOfBlocks",
Style.KeepEmptyLinesAtTheStartOfBlocks);
IO.mapOptional("MacroBlockBegin", Style.MacroBlockBegin);
@@ -901,6 +903,7 @@
LLVMStyle.InsertTrailingCommas = FormatStyle::TCS_None;
LLVMStyle.JavaScriptQuotes = FormatStyle::JSQS_Leave;
LLVMStyle.JavaScriptWrapImports = true;
+ LLVMStyle.JavaStaticImportAfterImport = false;
LLVMStyle.TabWidth = 8;
LLVMStyle.MaxEmptyLinesToKeep = 1;
LLVMStyle.KeepEmptyLinesAtTheStartOfBlocks = true;
@@ -2312,12 +2315,15 @@
JavaImportGroups.push_back(
findJavaImportGroup(Style, Imports[i].Identifier));
}
+ bool StaticImportAfterNormalImport = Style.JavaStaticImportAfterImport;
llvm::sort(Indices, [&](unsigned LHSI, unsigned RHSI) {
// Negating IsStatic to push static imports above non-static imports.
-return std::make_tuple(!Imports[LHSI].IsStatic, JavaImportGroups[LHSI],
- Imports[LHSI].Identifier) <
- std::make_tuple(!Imports[RHSI].IsStatic, JavaImportGroups[RHSI],
- Imports[RHSI].Identifier);
+return std::make_tuple(!Imports[LHSI].IsStatic ^
+ StaticImportAfterNormalImport,
+ JavaImportGroups[LHSI], Imports[LHSI].Identifier) <
+ std::make_tuple(!Imports[RHSI].IsStatic ^
+ StaticImportAfterNormalImport,
+ JavaImportGroups[RHSI], Imports[RHSI].Identifier);
});
// Deduplicate imports.
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1618,10 +1618,12 @@
/// A vector of prefixes ordered by the desired groups for Java imports.
///
- /// Each group is separated by a newline. Static imports will also follow the
- /// same grouping convention above all non-static imports. One group's prefix
- /// can be a subset of another - the longest prefix is always matched. Within
- /// a group, the imports are ordered lexicographically.
+ /// One group's prefix can be a subset of another - the longest
[PATCH] D87201: [clang-format] Add a option for the position of Java static import
bc-lee added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:2027 + + .. code-block:: java + true: MyDeveloperDay wrote: > The ClangFormatStyleOptions.rst is generated using > doc/tools/dump_format_style.py which reads Format.h and generates this, > > If this code block in not in the Format.h it will get removed the next time > the script is run, please don't change ClangFormatStyleOption.rst by hand use > the script, so add the code block to the Format.h file (see others options > for now to do this) Done. Comment at: clang/include/clang/Format/Format.h:1705 + /// \endcode + bool JavaStaticImportAfterImport; + JakeMerdichAMD wrote: > 3 things here: > > 1. Did you mix up the true and false cases? > 2. (nit) You should also note that if this option is false, all static > imports are before all non-static imports (as opposed to mixed together > alphabetically). I was confused on first glance. > 3. Please add this config option to FormatTests.cpp:ParsesConfigurationBools. I understand. The description is somewhat misleading. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87201/new/ https://reviews.llvm.org/D87201 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D87201: [clang-format] Add a option for the position of Java static import
bc-lee updated this revision to Diff 290246.
bc-lee added a comment.
Modify the comment of Format.h to sync ClangFormatStyleOptions.rst
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87201/new/
https://reviews.llvm.org/D87201
Files:
clang/docs/ClangFormatStyleOptions.rst
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/unittests/Format/SortImportsTestJava.cpp
Index: clang/unittests/Format/SortImportsTestJava.cpp
===
--- clang/unittests/Format/SortImportsTestJava.cpp
+++ clang/unittests/Format/SortImportsTestJava.cpp
@@ -250,6 +250,30 @@
"import org.c;\n"));
}
+TEST_F(SortImportsTestJava, FormatJavaStaticImportAfterImport) {
+ FmtStyle.JavaStaticImportAfterImport = true;
+
+ EXPECT_EQ("import com.test.b;\n"
+"import com.test.c;\n"
+"\n"
+"import org.b;\n"
+"\n"
+"import com.b;\n"
+"\n"
+"import static com.test.a;\n"
+"\n"
+"import static org.a;\n"
+"\n"
+"import static com.a;\n",
+sort("import static com.test.a;\n"
+ "import static org.a;\n"
+ "import static com.a;\n"
+ "import com.test.b;\n"
+ "import org.b;\n"
+ "import com.b;\n"
+ "import com.test.c;\n"));
+}
+
TEST_F(SortImportsTestJava, DeduplicateImports) {
EXPECT_EQ("import org.a;\n", sort("import org.a;\n"
"import org.a;\n"));
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -544,6 +544,8 @@
IO.mapOptional("JavaImportGroups", Style.JavaImportGroups);
IO.mapOptional("JavaScriptQuotes", Style.JavaScriptQuotes);
IO.mapOptional("JavaScriptWrapImports", Style.JavaScriptWrapImports);
+IO.mapOptional("JavaStaticImportAfterImport",
+ Style.JavaStaticImportAfterImport);
IO.mapOptional("KeepEmptyLinesAtTheStartOfBlocks",
Style.KeepEmptyLinesAtTheStartOfBlocks);
IO.mapOptional("MacroBlockBegin", Style.MacroBlockBegin);
@@ -901,6 +903,7 @@
LLVMStyle.InsertTrailingCommas = FormatStyle::TCS_None;
LLVMStyle.JavaScriptQuotes = FormatStyle::JSQS_Leave;
LLVMStyle.JavaScriptWrapImports = true;
+ LLVMStyle.JavaStaticImportAfterImport = false;
LLVMStyle.TabWidth = 8;
LLVMStyle.MaxEmptyLinesToKeep = 1;
LLVMStyle.KeepEmptyLinesAtTheStartOfBlocks = true;
@@ -2312,12 +2315,15 @@
JavaImportGroups.push_back(
findJavaImportGroup(Style, Imports[i].Identifier));
}
+ bool StaticImportAfterNormalImport = Style.JavaStaticImportAfterImport;
llvm::sort(Indices, [&](unsigned LHSI, unsigned RHSI) {
// Negating IsStatic to push static imports above non-static imports.
-return std::make_tuple(!Imports[LHSI].IsStatic, JavaImportGroups[LHSI],
- Imports[LHSI].Identifier) <
- std::make_tuple(!Imports[RHSI].IsStatic, JavaImportGroups[RHSI],
- Imports[RHSI].Identifier);
+return std::make_tuple(!Imports[LHSI].IsStatic ^
+ StaticImportAfterNormalImport,
+ JavaImportGroups[LHSI], Imports[LHSI].Identifier) <
+ std::make_tuple(!Imports[RHSI].IsStatic ^
+ StaticImportAfterNormalImport,
+ JavaImportGroups[RHSI], Imports[RHSI].Identifier);
});
// Deduplicate imports.
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1689,6 +1689,21 @@
bool JavaScriptWrapImports;
// clang-format on
+ /// If true, clang-format will put Java static imports after all non-static
+ /// imports.
+ /// \code{.java}
+ /// true:
+ /// import static org.example.function1;
+ ///
+ /// import org.example.ClassA;
+ ///
+ /// false:
+ /// import org.example.ClassA;
+ ///
+ /// import static org.example.function1;
+ /// \endcode
+ bool JavaStaticImportAfterImport;
+
/// If true, the empty line at the start of blocks is kept.
/// \code
///true: false:
@@ -2410,6 +2425,7 @@
JavaImportGroups == R.JavaImportGroups &&
JavaScriptQuotes == R.JavaScriptQuotes &&
JavaScriptWrapImports == R.JavaScriptWrapImports &&
+ JavaStaticImportAfterImport == R.JavaStaticImportAfterImport &&
KeepEmptyLinesAtTheStartOfBlocks ==
R.KeepEmptyLinesAtTheStartOfBlocks &&
MacroBlockBegin == R.MacroBlockBegin &&
Index: clang/docs/ClangFormatStyleOptions.rst
[PATCH] D87201: [clang-format] Add a option for the position of Java static import
bc-lee updated this revision to Diff 290137.
bc-lee added a comment.
Add missing initializer.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87201/new/
https://reviews.llvm.org/D87201
Files:
clang/docs/ClangFormatStyleOptions.rst
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/unittests/Format/SortImportsTestJava.cpp
Index: clang/unittests/Format/SortImportsTestJava.cpp
===
--- clang/unittests/Format/SortImportsTestJava.cpp
+++ clang/unittests/Format/SortImportsTestJava.cpp
@@ -250,6 +250,30 @@
"import org.c;\n"));
}
+TEST_F(SortImportsTestJava, FormatJavaStaticImportAfterImport) {
+ FmtStyle.JavaStaticImportAfterImport = true;
+
+ EXPECT_EQ("import com.test.b;\n"
+"import com.test.c;\n"
+"\n"
+"import org.b;\n"
+"\n"
+"import com.b;\n"
+"\n"
+"import static com.test.a;\n"
+"\n"
+"import static org.a;\n"
+"\n"
+"import static com.a;\n",
+sort("import static com.test.a;\n"
+ "import static org.a;\n"
+ "import static com.a;\n"
+ "import com.test.b;\n"
+ "import org.b;\n"
+ "import com.b;\n"
+ "import com.test.c;\n"));
+}
+
TEST_F(SortImportsTestJava, DeduplicateImports) {
EXPECT_EQ("import org.a;\n", sort("import org.a;\n"
"import org.a;\n"));
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -543,6 +543,8 @@
IO.mapOptional("JavaImportGroups", Style.JavaImportGroups);
IO.mapOptional("JavaScriptQuotes", Style.JavaScriptQuotes);
IO.mapOptional("JavaScriptWrapImports", Style.JavaScriptWrapImports);
+IO.mapOptional("JavaStaticImportAfterImport",
+ Style.JavaStaticImportAfterImport);
IO.mapOptional("KeepEmptyLinesAtTheStartOfBlocks",
Style.KeepEmptyLinesAtTheStartOfBlocks);
IO.mapOptional("MacroBlockBegin", Style.MacroBlockBegin);
@@ -899,6 +901,7 @@
LLVMStyle.InsertTrailingCommas = FormatStyle::TCS_None;
LLVMStyle.JavaScriptQuotes = FormatStyle::JSQS_Leave;
LLVMStyle.JavaScriptWrapImports = true;
+ LLVMStyle.JavaStaticImportAfterImport = false;
LLVMStyle.TabWidth = 8;
LLVMStyle.MaxEmptyLinesToKeep = 1;
LLVMStyle.KeepEmptyLinesAtTheStartOfBlocks = true;
@@ -2310,12 +2313,15 @@
JavaImportGroups.push_back(
findJavaImportGroup(Style, Imports[i].Identifier));
}
+ bool StaticImportAfterNormalImport = Style.JavaStaticImportAfterImport;
llvm::sort(Indices, [&](unsigned LHSI, unsigned RHSI) {
// Negating IsStatic to push static imports above non-static imports.
-return std::make_tuple(!Imports[LHSI].IsStatic, JavaImportGroups[LHSI],
- Imports[LHSI].Identifier) <
- std::make_tuple(!Imports[RHSI].IsStatic, JavaImportGroups[RHSI],
- Imports[RHSI].Identifier);
+return std::make_tuple(!Imports[LHSI].IsStatic ^
+ StaticImportAfterNormalImport,
+ JavaImportGroups[LHSI], Imports[LHSI].Identifier) <
+ std::make_tuple(!Imports[RHSI].IsStatic ^
+ StaticImportAfterNormalImport,
+ JavaImportGroups[RHSI], Imports[RHSI].Identifier);
});
// Deduplicate imports.
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1671,6 +1671,10 @@
bool JavaScriptWrapImports;
// clang-format on
+ /// If true, clang-format will put Java Static imports after all non-static
+ /// imports.
+ bool JavaStaticImportAfterImport;
+
/// If true, the empty line at the start of blocks is kept.
/// \code
///true: false:
@@ -2391,6 +2395,7 @@
JavaImportGroups == R.JavaImportGroups &&
JavaScriptQuotes == R.JavaScriptQuotes &&
JavaScriptWrapImports == R.JavaScriptWrapImports &&
+ JavaStaticImportAfterImport == R.JavaStaticImportAfterImport &&
KeepEmptyLinesAtTheStartOfBlocks ==
R.KeepEmptyLinesAtTheStartOfBlocks &&
MacroBlockBegin == R.MacroBlockBegin &&
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2021,6 +2021,20 @@
false:
import {VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying,} from "some/module.js"
[PATCH] D87201: [clang-format] Add a option for the position of Java static import
bc-lee created this revision.
bc-lee added a reviewer: MyDeveloperDay.
bc-lee added projects: clang, clang-format.
Herald added a subscriber: cfe-commits.
bc-lee requested review of this revision.
Some Java style guides and IDEs group Java static imports after
non-static imports. This patch allows clang-format to control
the location of static imports.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D87201
Files:
clang/docs/ClangFormatStyleOptions.rst
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/unittests/Format/SortImportsTestJava.cpp
Index: clang/unittests/Format/SortImportsTestJava.cpp
===
--- clang/unittests/Format/SortImportsTestJava.cpp
+++ clang/unittests/Format/SortImportsTestJava.cpp
@@ -250,6 +250,30 @@
"import org.c;\n"));
}
+TEST_F(SortImportsTestJava, FormatJavaStaticImportAfterImport) {
+ FmtStyle.JavaStaticImportAfterImport = true;
+
+ EXPECT_EQ("import com.test.b;\n"
+"import com.test.c;\n"
+"\n"
+"import org.b;\n"
+"\n"
+"import com.b;\n"
+"\n"
+"import static com.test.a;\n"
+"\n"
+"import static org.a;\n"
+"\n"
+"import static com.a;\n",
+sort("import static com.test.a;\n"
+ "import static org.a;\n"
+ "import static com.a;\n"
+ "import com.test.b;\n"
+ "import org.b;\n"
+ "import com.b;\n"
+ "import com.test.c;\n"));
+}
+
TEST_F(SortImportsTestJava, DeduplicateImports) {
EXPECT_EQ("import org.a;\n", sort("import org.a;\n"
"import org.a;\n"));
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -541,6 +541,8 @@
Style.IndentWrappedFunctionNames);
IO.mapOptional("InsertTrailingCommas", Style.InsertTrailingCommas);
IO.mapOptional("JavaImportGroups", Style.JavaImportGroups);
+IO.mapOptional("JavaStaticImportAfterImport",
+ Style.JavaStaticImportAfterImport);
IO.mapOptional("JavaScriptQuotes", Style.JavaScriptQuotes);
IO.mapOptional("JavaScriptWrapImports", Style.JavaScriptWrapImports);
IO.mapOptional("KeepEmptyLinesAtTheStartOfBlocks",
@@ -2310,12 +2312,15 @@
JavaImportGroups.push_back(
findJavaImportGroup(Style, Imports[i].Identifier));
}
+ bool StaticImportAfterNormalImport = Style.JavaStaticImportAfterImport;
llvm::sort(Indices, [&](unsigned LHSI, unsigned RHSI) {
// Negating IsStatic to push static imports above non-static imports.
-return std::make_tuple(!Imports[LHSI].IsStatic, JavaImportGroups[LHSI],
- Imports[LHSI].Identifier) <
- std::make_tuple(!Imports[RHSI].IsStatic, JavaImportGroups[RHSI],
- Imports[RHSI].Identifier);
+return std::make_tuple(!Imports[LHSI].IsStatic ^
+ StaticImportAfterNormalImport,
+ JavaImportGroups[LHSI], Imports[LHSI].Identifier) <
+ std::make_tuple(!Imports[RHSI].IsStatic ^
+ StaticImportAfterNormalImport,
+ JavaImportGroups[RHSI], Imports[RHSI].Identifier);
});
// Deduplicate imports.
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1671,6 +1671,10 @@
bool JavaScriptWrapImports;
// clang-format on
+ /// If true, clang-format will put Java Static imports after all non-static
+ /// imports.
+ bool JavaStaticImportAfterImport;
+
/// If true, the empty line at the start of blocks is kept.
/// \code
///true: false:
@@ -2389,6 +2393,7 @@
IndentWidth == R.IndentWidth && Language == R.Language &&
IndentWrappedFunctionNames == R.IndentWrappedFunctionNames &&
JavaImportGroups == R.JavaImportGroups &&
+ JavaStaticImportAfterImport == R.JavaStaticImportAfterImport &&
JavaScriptQuotes == R.JavaScriptQuotes &&
JavaScriptWrapImports == R.JavaScriptWrapImports &&
KeepEmptyLinesAtTheStartOfBlocks ==
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2021,6 +2021,20 @@
false:
import {VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying,} from "some/module.js"
+**JavaStaticImportAfterImport** (``bool``)
+ If true, clang-format will put Java static imports after all non-static imports.
