[clang] Support target names with dots in more utilities (PR #65812)
dankm wrote: Sounds good. Everything I want is in the pull request, I'm going to create a new pull request to update some tests as we discussed above. I'll update the description to be a bit more clear. Thanks for all help. https://github.com/llvm/llvm-project/pull/65812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support target names with dots in more utilities (PR #65812)
jh7370 wrote: @dankm, is there a particular reason you haven't merged this change in yet? FWIW, the formatter check failed for some reason, but I'm not sure it's related to any formatting issue. Please verify by running clang-format on your changes before merging. https://github.com/llvm/llvm-project/pull/65812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support target names with dots in more utilities (PR #65812)
dankm wrote: No _good_ particular reason. I got hung up on the formatting issues then ran out of steam, and busy with $job. I just ran clang-format on this change and it came up clean. And now that I've done that the only reason I have left is I'm unable to merge my own changes. Would you mind, @jh7370? https://github.com/llvm/llvm-project/pull/65812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support target names with dots in more utilities (PR #65812)
dankm wrote: Hm. I have "fixup" commits in this branch, should I rebase those, or can we squash merge as-is? https://github.com/llvm/llvm-project/pull/65812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support target names with dots in more utilities (PR #65812)
jh7370 wrote: I can merge it for you, but first I just want to confirm that all 4 commits in this PR are intended to be for the PR? If not, you'll need to rebase and force push. There's no need for you to manually squash the fixups (although you might as well if you do end up rebasing). When I do the merge via the UI, it'll be using the Squash and merge button, which squashes everything into a single commit. The final commit message will be the PR description. I found the description a little confusing at first, without reading the code and test, so it may be beneficial to add an example of the before and after behaviour to that description before I merge it. Please could you update the patch description accordingly. https://github.com/llvm/llvm-project/pull/65812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support target names with dots in more utilities (PR #65812)
@@ -1696,6 +1696,40 @@ TEST(Support, ReplacePathPrefix) { EXPECT_EQ(Path, "C:\\old/foo\\bar"); } +TEST(Support, FindProgramName) { + StringRef WindowsProgName = + path::program_name("C:\\Test\\foo.exe", path::Style::windows); + EXPECT_EQ(WindowsProgName, "foo"); + + StringRef WindowsProgNameManyDots = path::program_name( + "C:\\Test.7\\x86_64-freebsd14.0-clang.exe", path::Style::windows); + EXPECT_EQ(WindowsProgNameManyDots, "x86_64-freebsd14.0-clang"); + + StringRef PosixProgName = + path::program_name("/var/empty/clang.exe", path::Style::posix); + EXPECT_EQ(PosixProgName, "clang"); + + StringRef PosixProgNameManyDotsExe = path::program_name( + "/llvm-test16.4/x86_64-portbld-freebsd13.2-llvm-ar.exe", + path::Style::posix); + EXPECT_EQ(PosixProgNameManyDotsExe, "x86_64-portbld-freebsd13.2-llvm-ar"); + + StringRef PosixProgNameManyDots = path::program_name( + "/llvm-test16.4/x86_64-portbld-freebsd13.2-llvm-ar", path::Style::posix); + EXPECT_EQ(PosixProgNameManyDots, "x86_64-portbld-freebsd13.2-llvm-ar"); + + StringRef PosixProgNameSh = + path::program_name("/llvm-test16.4/x86_64-portbld-freebsd13.2-llvm-ar.sh", + path::Style::posix); + EXPECT_EQ(PosixProgNameSh, "x86_64-portbld-freebsd13.2-llvm-ar.sh"); + + // TODO: determine if this is correct. What happens on windows with an executable + // named ".exe"? dankm wrote: Makes sense, I'll do that in a subsequent commit.. hopefully that keeps the review intact.. still getting used to github. https://github.com/llvm/llvm-project/pull/65812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support target names with dots in more utilities (PR #65812)
https://github.com/dankm updated https://github.com/llvm/llvm-project/pull/65812 >From 5f5e57a5c01088e7f0b9276ed02dd436a3f9cc0a Mon Sep 17 00:00:00 2001 From: Dan McGregor Date: Mon, 14 Aug 2023 18:44:08 -0600 Subject: [PATCH 1/3] Support: hoist lld's executable name code to Path Instead of using custom code to find the program name throughout the codebase, write one function as a path helper to consistently determine the program name. This globally correctly finds target names with dots in them (ie freebsd13.2). --- lld/COFF/Driver.cpp| 2 +- lld/Common/Args.cpp| 6 -- lld/ELF/Driver.cpp | 2 +- lld/MachO/Driver.cpp | 2 +- lld/include/lld/Common/Args.h | 2 -- lld/wasm/Driver.cpp| 2 +- lldb/tools/driver/Driver.cpp | 2 +- llvm/include/llvm/Support/Path.h | 15 +++ llvm/lib/Support/Path.cpp | 7 +++ llvm/tools/llvm-ar/llvm-ar.cpp | 3 ++- llvm/tools/llvm-cov/llvm-cov.cpp | 2 +- llvm/tools/llvm-driver/llvm-driver.cpp | 2 +- llvm/tools/llvm-objcopy/llvm-objcopy.cpp | 2 +- llvm/tools/llvm-objdump/llvm-objdump.cpp | 2 +- llvm/tools/llvm-rc/llvm-rc.cpp | 2 +- llvm/tools/llvm-readobj/llvm-readobj.cpp | 2 +- llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp | 2 +- 17 files changed, 36 insertions(+), 21 deletions(-) diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp index d7476e91e03e384..03a63bf3a306b4d 100644 --- a/lld/COFF/Driver.cpp +++ b/lld/COFF/Driver.cpp @@ -68,7 +68,7 @@ bool link(ArrayRef args, llvm::raw_ostream &stdoutOS, auto *ctx = new COFFLinkerContext; ctx->e.initialize(stdoutOS, stderrOS, exitEarly, disableOutput); - ctx->e.logName = args::getFilenameWithoutExe(args[0]); + ctx->e.logName = sys::path::program_name(args[0]); ctx->e.errorLimitExceededMsg = "too many errors emitted, stopping now" " (use /errorlimit:0 to see all errors)"; diff --git a/lld/Common/Args.cpp b/lld/Common/Args.cpp index 48c934df3a2c931..b02d023fe19aeb5 100644 --- a/lld/Common/Args.cpp +++ b/lld/Common/Args.cpp @@ -85,9 +85,3 @@ std::vector lld::args::getLines(MemoryBufferRef mb) { } return ret; } - -StringRef lld::args::getFilenameWithoutExe(StringRef path) { - if (path.ends_with_insensitive(".exe")) -return sys::path::stem(path); - return sys::path::filename(path); -} diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp index 9d167293574fa64..957d0da1589caf8 100644 --- a/lld/ELF/Driver.cpp +++ b/lld/ELF/Driver.cpp @@ -143,7 +143,7 @@ bool link(ArrayRef args, llvm::raw_ostream &stdoutOS, SharedFile::vernauxNum = 0; }; - ctx->e.logName = args::getFilenameWithoutExe(args[0]); + ctx->e.logName = sys::path::program_name(args[0]); ctx->e.errorLimitExceededMsg = "too many errors emitted, stopping now (use " "--error-limit=0 to see all errors)"; diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp index 00ff3439a043be9..ed0b66d4b2ec6ba 100644 --- a/lld/MachO/Driver.cpp +++ b/lld/MachO/Driver.cpp @@ -1428,7 +1428,7 @@ bool link(ArrayRef argsArr, llvm::raw_ostream &stdoutOS, InputFile::resetIdCount(); }; - ctx->e.logName = args::getFilenameWithoutExe(argsArr[0]); + ctx->e.logName = sys::path::program_name(argsArr[0]); MachOOptTable parser; InputArgList args = parser.parse(argsArr.slice(1)); diff --git a/lld/include/lld/Common/Args.h b/lld/include/lld/Common/Args.h index 60f83fbbbf1a3c9..8822707a6a1f212 100644 --- a/lld/include/lld/Common/Args.h +++ b/lld/include/lld/Common/Args.h @@ -38,8 +38,6 @@ uint64_t getZOptionValue(llvm::opt::InputArgList &args, int id, StringRef key, std::vector getLines(MemoryBufferRef mb); -StringRef getFilenameWithoutExe(StringRef path); - } // namespace args } // namespace lld diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp index c2f5f0185781f36..f13aa520f35b100 100644 --- a/lld/wasm/Driver.cpp +++ b/lld/wasm/Driver.cpp @@ -89,7 +89,7 @@ bool link(ArrayRef args, llvm::raw_ostream &stdoutOS, auto *ctx = new CommonLinkerContext; ctx->e.initialize(stdoutOS, stderrOS, exitEarly, disableOutput); - ctx->e.logName = args::getFilenameWithoutExe(args[0]); + ctx->e.logName = sys::path::program_name(args[0]); ctx->e.errorLimitExceededMsg = "too many errors emitted, stopping now (use " "-error-limit=0 to see all errors)"; diff --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp index f8058f868d53ffe..2fb760ca97a001d 100644 --- a/lldb/tools/driver/Driver.cpp +++ b/lldb/tools/driver/Driver.cpp @@ -754,7 +754,7 @@ int main(int argc, char const *argv[]) { ArrayRef arg_arr = ArrayRef(argv + 1, argc - 1); opt::InputArgList input_args = T.ParseArgs(arg_arr, MissingA
[clang] Support target names with dots in more utilities (PR #65812)
https://github.com/dankm resolved https://github.com/llvm/llvm-project/pull/65812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support target names with dots in more utilities (PR #65812)
https://github.com/dankm updated https://github.com/llvm/llvm-project/pull/65812 >From 5f5e57a5c01088e7f0b9276ed02dd436a3f9cc0a Mon Sep 17 00:00:00 2001 From: Dan McGregor Date: Mon, 14 Aug 2023 18:44:08 -0600 Subject: [PATCH 1/4] Support: hoist lld's executable name code to Path Instead of using custom code to find the program name throughout the codebase, write one function as a path helper to consistently determine the program name. This globally correctly finds target names with dots in them (ie freebsd13.2). --- lld/COFF/Driver.cpp| 2 +- lld/Common/Args.cpp| 6 -- lld/ELF/Driver.cpp | 2 +- lld/MachO/Driver.cpp | 2 +- lld/include/lld/Common/Args.h | 2 -- lld/wasm/Driver.cpp| 2 +- lldb/tools/driver/Driver.cpp | 2 +- llvm/include/llvm/Support/Path.h | 15 +++ llvm/lib/Support/Path.cpp | 7 +++ llvm/tools/llvm-ar/llvm-ar.cpp | 3 ++- llvm/tools/llvm-cov/llvm-cov.cpp | 2 +- llvm/tools/llvm-driver/llvm-driver.cpp | 2 +- llvm/tools/llvm-objcopy/llvm-objcopy.cpp | 2 +- llvm/tools/llvm-objdump/llvm-objdump.cpp | 2 +- llvm/tools/llvm-rc/llvm-rc.cpp | 2 +- llvm/tools/llvm-readobj/llvm-readobj.cpp | 2 +- llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp | 2 +- 17 files changed, 36 insertions(+), 21 deletions(-) diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp index d7476e91e03e384..03a63bf3a306b4d 100644 --- a/lld/COFF/Driver.cpp +++ b/lld/COFF/Driver.cpp @@ -68,7 +68,7 @@ bool link(ArrayRef args, llvm::raw_ostream &stdoutOS, auto *ctx = new COFFLinkerContext; ctx->e.initialize(stdoutOS, stderrOS, exitEarly, disableOutput); - ctx->e.logName = args::getFilenameWithoutExe(args[0]); + ctx->e.logName = sys::path::program_name(args[0]); ctx->e.errorLimitExceededMsg = "too many errors emitted, stopping now" " (use /errorlimit:0 to see all errors)"; diff --git a/lld/Common/Args.cpp b/lld/Common/Args.cpp index 48c934df3a2c931..b02d023fe19aeb5 100644 --- a/lld/Common/Args.cpp +++ b/lld/Common/Args.cpp @@ -85,9 +85,3 @@ std::vector lld::args::getLines(MemoryBufferRef mb) { } return ret; } - -StringRef lld::args::getFilenameWithoutExe(StringRef path) { - if (path.ends_with_insensitive(".exe")) -return sys::path::stem(path); - return sys::path::filename(path); -} diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp index 9d167293574fa64..957d0da1589caf8 100644 --- a/lld/ELF/Driver.cpp +++ b/lld/ELF/Driver.cpp @@ -143,7 +143,7 @@ bool link(ArrayRef args, llvm::raw_ostream &stdoutOS, SharedFile::vernauxNum = 0; }; - ctx->e.logName = args::getFilenameWithoutExe(args[0]); + ctx->e.logName = sys::path::program_name(args[0]); ctx->e.errorLimitExceededMsg = "too many errors emitted, stopping now (use " "--error-limit=0 to see all errors)"; diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp index 00ff3439a043be9..ed0b66d4b2ec6ba 100644 --- a/lld/MachO/Driver.cpp +++ b/lld/MachO/Driver.cpp @@ -1428,7 +1428,7 @@ bool link(ArrayRef argsArr, llvm::raw_ostream &stdoutOS, InputFile::resetIdCount(); }; - ctx->e.logName = args::getFilenameWithoutExe(argsArr[0]); + ctx->e.logName = sys::path::program_name(argsArr[0]); MachOOptTable parser; InputArgList args = parser.parse(argsArr.slice(1)); diff --git a/lld/include/lld/Common/Args.h b/lld/include/lld/Common/Args.h index 60f83fbbbf1a3c9..8822707a6a1f212 100644 --- a/lld/include/lld/Common/Args.h +++ b/lld/include/lld/Common/Args.h @@ -38,8 +38,6 @@ uint64_t getZOptionValue(llvm::opt::InputArgList &args, int id, StringRef key, std::vector getLines(MemoryBufferRef mb); -StringRef getFilenameWithoutExe(StringRef path); - } // namespace args } // namespace lld diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp index c2f5f0185781f36..f13aa520f35b100 100644 --- a/lld/wasm/Driver.cpp +++ b/lld/wasm/Driver.cpp @@ -89,7 +89,7 @@ bool link(ArrayRef args, llvm::raw_ostream &stdoutOS, auto *ctx = new CommonLinkerContext; ctx->e.initialize(stdoutOS, stderrOS, exitEarly, disableOutput); - ctx->e.logName = args::getFilenameWithoutExe(args[0]); + ctx->e.logName = sys::path::program_name(args[0]); ctx->e.errorLimitExceededMsg = "too many errors emitted, stopping now (use " "-error-limit=0 to see all errors)"; diff --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp index f8058f868d53ffe..2fb760ca97a001d 100644 --- a/lldb/tools/driver/Driver.cpp +++ b/lldb/tools/driver/Driver.cpp @@ -754,7 +754,7 @@ int main(int argc, char const *argv[]) { ArrayRef arg_arr = ArrayRef(argv + 1, argc - 1); opt::InputArgList input_args = T.ParseArgs(arg_arr, MissingA
[clang] Support target names with dots in more utilities (PR #65812)
https://github.com/dankm updated https://github.com/llvm/llvm-project/pull/65812: >From a42cbcc0731725929c60ba4c1fd5254ae5e46613 Mon Sep 17 00:00:00 2001 From: Dan McGregor Date: Mon, 14 Aug 2023 18:44:08 -0600 Subject: [PATCH] Support: hoist lld's executable name code to Path Instead of using custom code to find the program name throughout the codebase, write one function as a path helper to consistently determine the program name. This globally correctly finds target names with dots in them (ie freebsd13.2). --- lld/COFF/Driver.cpp| 2 +- lld/Common/Args.cpp| 6 -- lld/ELF/Driver.cpp | 2 +- lld/MachO/Driver.cpp | 2 +- lld/include/lld/Common/Args.h | 2 -- lld/wasm/Driver.cpp| 2 +- llvm/include/llvm/Support/Path.h | 15 +++ llvm/lib/Support/Path.cpp | 8 llvm/tools/llvm-ar/llvm-ar.cpp | 3 ++- llvm/tools/llvm-cov/llvm-cov.cpp | 2 +- llvm/tools/llvm-driver/llvm-driver.cpp | 2 +- llvm/tools/llvm-objcopy/llvm-objcopy.cpp | 2 +- llvm/tools/llvm-objdump/llvm-objdump.cpp | 2 +- llvm/tools/llvm-rc/llvm-rc.cpp | 2 +- llvm/tools/llvm-readobj/llvm-readobj.cpp | 2 +- llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp | 2 +- 16 files changed, 36 insertions(+), 20 deletions(-) diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp index d7476e91e03e384..03a63bf3a306b4d 100644 --- a/lld/COFF/Driver.cpp +++ b/lld/COFF/Driver.cpp @@ -68,7 +68,7 @@ bool link(ArrayRef args, llvm::raw_ostream &stdoutOS, auto *ctx = new COFFLinkerContext; ctx->e.initialize(stdoutOS, stderrOS, exitEarly, disableOutput); - ctx->e.logName = args::getFilenameWithoutExe(args[0]); + ctx->e.logName = sys::path::program_name(args[0]); ctx->e.errorLimitExceededMsg = "too many errors emitted, stopping now" " (use /errorlimit:0 to see all errors)"; diff --git a/lld/Common/Args.cpp b/lld/Common/Args.cpp index 48c934df3a2c931..b02d023fe19aeb5 100644 --- a/lld/Common/Args.cpp +++ b/lld/Common/Args.cpp @@ -85,9 +85,3 @@ std::vector lld::args::getLines(MemoryBufferRef mb) { } return ret; } - -StringRef lld::args::getFilenameWithoutExe(StringRef path) { - if (path.ends_with_insensitive(".exe")) -return sys::path::stem(path); - return sys::path::filename(path); -} diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp index 9219314111610d1..0eef7bbf1baa213 100644 --- a/lld/ELF/Driver.cpp +++ b/lld/ELF/Driver.cpp @@ -143,7 +143,7 @@ bool link(ArrayRef args, llvm::raw_ostream &stdoutOS, SharedFile::vernauxNum = 0; }; - ctx->e.logName = args::getFilenameWithoutExe(args[0]); + ctx->e.logName = sys::path::program_name(args[0]); ctx->e.errorLimitExceededMsg = "too many errors emitted, stopping now (use " "--error-limit=0 to see all errors)"; diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp index 00ff3439a043be9..ed0b66d4b2ec6ba 100644 --- a/lld/MachO/Driver.cpp +++ b/lld/MachO/Driver.cpp @@ -1428,7 +1428,7 @@ bool link(ArrayRef argsArr, llvm::raw_ostream &stdoutOS, InputFile::resetIdCount(); }; - ctx->e.logName = args::getFilenameWithoutExe(argsArr[0]); + ctx->e.logName = sys::path::program_name(argsArr[0]); MachOOptTable parser; InputArgList args = parser.parse(argsArr.slice(1)); diff --git a/lld/include/lld/Common/Args.h b/lld/include/lld/Common/Args.h index 60f83fbbbf1a3c9..8822707a6a1f212 100644 --- a/lld/include/lld/Common/Args.h +++ b/lld/include/lld/Common/Args.h @@ -38,8 +38,6 @@ uint64_t getZOptionValue(llvm::opt::InputArgList &args, int id, StringRef key, std::vector getLines(MemoryBufferRef mb); -StringRef getFilenameWithoutExe(StringRef path); - } // namespace args } // namespace lld diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp index c2f5f0185781f36..f13aa520f35b100 100644 --- a/lld/wasm/Driver.cpp +++ b/lld/wasm/Driver.cpp @@ -89,7 +89,7 @@ bool link(ArrayRef args, llvm::raw_ostream &stdoutOS, auto *ctx = new CommonLinkerContext; ctx->e.initialize(stdoutOS, stderrOS, exitEarly, disableOutput); - ctx->e.logName = args::getFilenameWithoutExe(args[0]); + ctx->e.logName = sys::path::program_name(args[0]); ctx->e.errorLimitExceededMsg = "too many errors emitted, stopping now (use " "-error-limit=0 to see all errors)"; diff --git a/llvm/include/llvm/Support/Path.h b/llvm/include/llvm/Support/Path.h index ce69f32b6cc81ba..405fdc03e020851 100644 --- a/llvm/include/llvm/Support/Path.h +++ b/llvm/include/llvm/Support/Path.h @@ -390,6 +390,21 @@ StringRef stem(StringRef path, Style style = Style::native); /// @result The extension of \a path. StringRef extension(StringRef path, Style style = Style::native); +/// Get the program's name +///
[clang] Support target names with dots in more utilities (PR #65812)
https://github.com/dankm updated https://github.com/llvm/llvm-project/pull/65812: >From a42cbcc0731725929c60ba4c1fd5254ae5e46613 Mon Sep 17 00:00:00 2001 From: Dan McGregor Date: Mon, 14 Aug 2023 18:44:08 -0600 Subject: [PATCH] Support: hoist lld's executable name code to Path Instead of using custom code to find the program name throughout the codebase, write one function as a path helper to consistently determine the program name. This globally correctly finds target names with dots in them (ie freebsd13.2). --- lld/COFF/Driver.cpp| 2 +- lld/Common/Args.cpp| 6 -- lld/ELF/Driver.cpp | 2 +- lld/MachO/Driver.cpp | 2 +- lld/include/lld/Common/Args.h | 2 -- lld/wasm/Driver.cpp| 2 +- llvm/include/llvm/Support/Path.h | 15 +++ llvm/lib/Support/Path.cpp | 8 llvm/tools/llvm-ar/llvm-ar.cpp | 3 ++- llvm/tools/llvm-cov/llvm-cov.cpp | 2 +- llvm/tools/llvm-driver/llvm-driver.cpp | 2 +- llvm/tools/llvm-objcopy/llvm-objcopy.cpp | 2 +- llvm/tools/llvm-objdump/llvm-objdump.cpp | 2 +- llvm/tools/llvm-rc/llvm-rc.cpp | 2 +- llvm/tools/llvm-readobj/llvm-readobj.cpp | 2 +- llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp | 2 +- 16 files changed, 36 insertions(+), 20 deletions(-) diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp index d7476e91e03e384..03a63bf3a306b4d 100644 --- a/lld/COFF/Driver.cpp +++ b/lld/COFF/Driver.cpp @@ -68,7 +68,7 @@ bool link(ArrayRef args, llvm::raw_ostream &stdoutOS, auto *ctx = new COFFLinkerContext; ctx->e.initialize(stdoutOS, stderrOS, exitEarly, disableOutput); - ctx->e.logName = args::getFilenameWithoutExe(args[0]); + ctx->e.logName = sys::path::program_name(args[0]); ctx->e.errorLimitExceededMsg = "too many errors emitted, stopping now" " (use /errorlimit:0 to see all errors)"; diff --git a/lld/Common/Args.cpp b/lld/Common/Args.cpp index 48c934df3a2c931..b02d023fe19aeb5 100644 --- a/lld/Common/Args.cpp +++ b/lld/Common/Args.cpp @@ -85,9 +85,3 @@ std::vector lld::args::getLines(MemoryBufferRef mb) { } return ret; } - -StringRef lld::args::getFilenameWithoutExe(StringRef path) { - if (path.ends_with_insensitive(".exe")) -return sys::path::stem(path); - return sys::path::filename(path); -} diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp index 9219314111610d1..0eef7bbf1baa213 100644 --- a/lld/ELF/Driver.cpp +++ b/lld/ELF/Driver.cpp @@ -143,7 +143,7 @@ bool link(ArrayRef args, llvm::raw_ostream &stdoutOS, SharedFile::vernauxNum = 0; }; - ctx->e.logName = args::getFilenameWithoutExe(args[0]); + ctx->e.logName = sys::path::program_name(args[0]); ctx->e.errorLimitExceededMsg = "too many errors emitted, stopping now (use " "--error-limit=0 to see all errors)"; diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp index 00ff3439a043be9..ed0b66d4b2ec6ba 100644 --- a/lld/MachO/Driver.cpp +++ b/lld/MachO/Driver.cpp @@ -1428,7 +1428,7 @@ bool link(ArrayRef argsArr, llvm::raw_ostream &stdoutOS, InputFile::resetIdCount(); }; - ctx->e.logName = args::getFilenameWithoutExe(argsArr[0]); + ctx->e.logName = sys::path::program_name(argsArr[0]); MachOOptTable parser; InputArgList args = parser.parse(argsArr.slice(1)); diff --git a/lld/include/lld/Common/Args.h b/lld/include/lld/Common/Args.h index 60f83fbbbf1a3c9..8822707a6a1f212 100644 --- a/lld/include/lld/Common/Args.h +++ b/lld/include/lld/Common/Args.h @@ -38,8 +38,6 @@ uint64_t getZOptionValue(llvm::opt::InputArgList &args, int id, StringRef key, std::vector getLines(MemoryBufferRef mb); -StringRef getFilenameWithoutExe(StringRef path); - } // namespace args } // namespace lld diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp index c2f5f0185781f36..f13aa520f35b100 100644 --- a/lld/wasm/Driver.cpp +++ b/lld/wasm/Driver.cpp @@ -89,7 +89,7 @@ bool link(ArrayRef args, llvm::raw_ostream &stdoutOS, auto *ctx = new CommonLinkerContext; ctx->e.initialize(stdoutOS, stderrOS, exitEarly, disableOutput); - ctx->e.logName = args::getFilenameWithoutExe(args[0]); + ctx->e.logName = sys::path::program_name(args[0]); ctx->e.errorLimitExceededMsg = "too many errors emitted, stopping now (use " "-error-limit=0 to see all errors)"; diff --git a/llvm/include/llvm/Support/Path.h b/llvm/include/llvm/Support/Path.h index ce69f32b6cc81ba..405fdc03e020851 100644 --- a/llvm/include/llvm/Support/Path.h +++ b/llvm/include/llvm/Support/Path.h @@ -390,6 +390,21 @@ StringRef stem(StringRef path, Style style = Style::native); /// @result The extension of \a path. StringRef extension(StringRef path, Style style = Style::native); +/// Get the program's name +///
[clang] Support target names with dots in more utilities (PR #65812)
https://github.com/dankm updated https://github.com/llvm/llvm-project/pull/65812: >From 416d75e617cc87b6e497815ce6ad1da630eb14a1 Mon Sep 17 00:00:00 2001 From: Dan McGregor Date: Mon, 14 Aug 2023 18:44:08 -0600 Subject: [PATCH 1/2] Support: hoist lld's executable name code to Path Instead of using custom code to find the program name throughout the codebase, write one function as a path helper to consistently determine the program name. This globally correctly finds target names with dots in them (ie freebsd13.2). --- lld/COFF/Driver.cpp| 2 +- lld/Common/Args.cpp| 6 -- lld/ELF/Driver.cpp | 2 +- lld/MachO/Driver.cpp | 2 +- lld/include/lld/Common/Args.h | 2 -- lld/wasm/Driver.cpp| 2 +- lldb/tools/driver/Driver.cpp | 2 +- llvm/include/llvm/Support/Path.h | 15 +++ llvm/lib/Support/Path.cpp | 8 llvm/tools/llvm-ar/llvm-ar.cpp | 3 ++- llvm/tools/llvm-cov/llvm-cov.cpp | 2 +- llvm/tools/llvm-driver/llvm-driver.cpp | 2 +- llvm/tools/llvm-objcopy/llvm-objcopy.cpp | 2 +- llvm/tools/llvm-objdump/llvm-objdump.cpp | 2 +- llvm/tools/llvm-rc/llvm-rc.cpp | 2 +- llvm/tools/llvm-readobj/llvm-readobj.cpp | 2 +- llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp | 2 +- 17 files changed, 37 insertions(+), 21 deletions(-) diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp index d7476e91e03e384..03a63bf3a306b4d 100644 --- a/lld/COFF/Driver.cpp +++ b/lld/COFF/Driver.cpp @@ -68,7 +68,7 @@ bool link(ArrayRef args, llvm::raw_ostream &stdoutOS, auto *ctx = new COFFLinkerContext; ctx->e.initialize(stdoutOS, stderrOS, exitEarly, disableOutput); - ctx->e.logName = args::getFilenameWithoutExe(args[0]); + ctx->e.logName = sys::path::program_name(args[0]); ctx->e.errorLimitExceededMsg = "too many errors emitted, stopping now" " (use /errorlimit:0 to see all errors)"; diff --git a/lld/Common/Args.cpp b/lld/Common/Args.cpp index 48c934df3a2c931..b02d023fe19aeb5 100644 --- a/lld/Common/Args.cpp +++ b/lld/Common/Args.cpp @@ -85,9 +85,3 @@ std::vector lld::args::getLines(MemoryBufferRef mb) { } return ret; } - -StringRef lld::args::getFilenameWithoutExe(StringRef path) { - if (path.ends_with_insensitive(".exe")) -return sys::path::stem(path); - return sys::path::filename(path); -} diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp index 9d167293574fa64..957d0da1589caf8 100644 --- a/lld/ELF/Driver.cpp +++ b/lld/ELF/Driver.cpp @@ -143,7 +143,7 @@ bool link(ArrayRef args, llvm::raw_ostream &stdoutOS, SharedFile::vernauxNum = 0; }; - ctx->e.logName = args::getFilenameWithoutExe(args[0]); + ctx->e.logName = sys::path::program_name(args[0]); ctx->e.errorLimitExceededMsg = "too many errors emitted, stopping now (use " "--error-limit=0 to see all errors)"; diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp index 00ff3439a043be9..ed0b66d4b2ec6ba 100644 --- a/lld/MachO/Driver.cpp +++ b/lld/MachO/Driver.cpp @@ -1428,7 +1428,7 @@ bool link(ArrayRef argsArr, llvm::raw_ostream &stdoutOS, InputFile::resetIdCount(); }; - ctx->e.logName = args::getFilenameWithoutExe(argsArr[0]); + ctx->e.logName = sys::path::program_name(argsArr[0]); MachOOptTable parser; InputArgList args = parser.parse(argsArr.slice(1)); diff --git a/lld/include/lld/Common/Args.h b/lld/include/lld/Common/Args.h index 60f83fbbbf1a3c9..8822707a6a1f212 100644 --- a/lld/include/lld/Common/Args.h +++ b/lld/include/lld/Common/Args.h @@ -38,8 +38,6 @@ uint64_t getZOptionValue(llvm::opt::InputArgList &args, int id, StringRef key, std::vector getLines(MemoryBufferRef mb); -StringRef getFilenameWithoutExe(StringRef path); - } // namespace args } // namespace lld diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp index c2f5f0185781f36..f13aa520f35b100 100644 --- a/lld/wasm/Driver.cpp +++ b/lld/wasm/Driver.cpp @@ -89,7 +89,7 @@ bool link(ArrayRef args, llvm::raw_ostream &stdoutOS, auto *ctx = new CommonLinkerContext; ctx->e.initialize(stdoutOS, stderrOS, exitEarly, disableOutput); - ctx->e.logName = args::getFilenameWithoutExe(args[0]); + ctx->e.logName = sys::path::program_name(args[0]); ctx->e.errorLimitExceededMsg = "too many errors emitted, stopping now (use " "-error-limit=0 to see all errors)"; diff --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp index f8058f868d53ffe..2fb760ca97a001d 100644 --- a/lldb/tools/driver/Driver.cpp +++ b/lldb/tools/driver/Driver.cpp @@ -754,7 +754,7 @@ int main(int argc, char const *argv[]) { ArrayRef arg_arr = ArrayRef(argv + 1, argc - 1); opt::InputArgList input_args = T.ParseArgs(arg_arr, Missi
[clang] Support target names with dots in more utilities (PR #65812)
https://github.com/llvmbot labeled https://github.com/llvm/llvm-project/pull/65812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support target names with dots in more utilities (PR #65812)
https://github.com/llvmbot labeled https://github.com/llvm/llvm-project/pull/65812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support target names with dots in more utilities (PR #65812)
https://github.com/llvmbot labeled https://github.com/llvm/llvm-project/pull/65812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support target names with dots in more utilities (PR #65812)
dankm wrote: Hm. I'm happy with this file, but I also discovered a bunch of other `tool-name` tests that can be updated. I'll add those too. https://github.com/llvm/llvm-project/pull/65812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support target names with dots in more utilities (PR #65812)
https://github.com/MaskRay edited https://github.com/llvm/llvm-project/pull/65812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support target names with dots in more utilities (PR #65812)
https://github.com/MaskRay approved this pull request. LGTM, but @jh7370 usually wants to have an eye on such changes. https://github.com/llvm/llvm-project/pull/65812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support target names with dots in more utilities (PR #65812)
@@ -390,6 +390,21 @@ StringRef stem(StringRef path, Style style = Style::native); /// @result The extension of \a path. StringRef extension(StringRef path, Style style = Style::native); +/// Get the program's name +/// +/// If the path ends with the string .exe, returns the stem of MaskRay wrote: .exe => ".exe" https://github.com/llvm/llvm-project/pull/65812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support target names with dots in more utilities (PR #65812)
@@ -5,16 +5,22 @@ # RUN: mkdir %t # RUN: ln -s llvm-ar %t/llvm-ar-9 # RUN: ln -s llvm-ar %t/ar.exe +# RUN: ln -s llvm-ar %t/x86_64-portbld-freebsd13.2-llvm-ar +# RUN: ln -s llvm-ar %t/x86_64-portbld-freebsd13.2-llvm-ar.exe # RUN: ln -s llvm-ar %t/arm-pokymllib32-linux-gnueabi-llvm-ar-9 # RUN: llvm-ar h | FileCheck %s --check-prefix=DEFAULT # RUN: %t/llvm-ar-9 h | FileCheck %s --check-prefix=VERSION # RUN: %t/ar.exe h | FileCheck %s --check-prefix=SUFFIX +# RUN: %t/x86_64-portbld-freebsd13.2-llvm-ar h | FileCheck %s --check-prefix=TARGETDOT +# RUN: %t/x86_64-portbld-freebsd13.2-llvm-ar.exe h | FileCheck %s --check-prefix=MULTIDOT ## Ensure that the "lib" substring does not result in misidentification as the ## llvm-lib tool. # RUN: %t/arm-pokymllib32-linux-gnueabi-llvm-ar-9 h | FileCheck %s --check-prefix=ARM # DEFAULT: USAGE: llvm-ar{{ }} # VERSION: USAGE: llvm-ar-9{{ }} # SUFFIX: USAGE: ar{{ }} +# TARGETDOT: USAGE: x86_64-portbld-freebsd13.2-llvm-ar{{ }} +# MULTIDOT: USAGE: x86_64-portbld-freebsd13.2-llvm-ar{{ }} jh7370 wrote: These don't need to be separate CHECK patterns, as they check the same thing. Simply reuse the prefix and only have one of them. https://github.com/llvm/llvm-project/pull/65812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support target names with dots in more utilities (PR #65812)
@@ -390,6 +390,21 @@ StringRef stem(StringRef path, Style style = Style::native); /// @result The extension of \a path. StringRef extension(StringRef path, Style style = Style::native); +/// Get the program's name +/// +/// If the path ends with the string .exe, returns the stem of +/// \a path. Otherwise returns the filename of \a path. +/// +/// @code +/// /foo/prog.exe => prog +/// /bar/prog => prog +/// /foo/prog1.2 => prog1.2 +/// @endcode +/// +/// @param path Input path. +/// @result The filename of \a path. Without any .exe component jh7370 wrote: ```suggestion /// @result The filename of \a path without any ".exe" component. ``` https://github.com/llvm/llvm-project/pull/65812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support target names with dots in more utilities (PR #65812)
@@ -600,6 +600,14 @@ StringRef extension(StringRef path, Style style) { return fname.substr(pos); } +StringRef program_name(StringRef path, Style style) { + // In the future this may need to be extended to other + // program suffixes. jh7370 wrote: Unnecessarily wrapped. https://github.com/llvm/llvm-project/pull/65812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support target names with dots in more utilities (PR #65812)
@@ -1696,6 +1696,38 @@ TEST(Support, ReplacePathPrefix) { EXPECT_EQ(Path, "C:\\old/foo\\bar"); } +TEST(Support, FindProgramName) { + StringRef WindowsProgName = + path::program_name("C:\\Test\\foo.exe", path::Style::windows); + EXPECT_EQ(WindowsProgName, "foo"); + + StringRef WindowsProgNameManyDots = path::program_name( + "C:\\Test.7\\x86_64-freebsd14.0-clang.exe", path::Style::windows); + EXPECT_EQ(WindowsProgNameManyDots, "x86_64-freebsd14.0-clang"); + + StringRef PosixProgName = + path::program_name("/var/empty/clang.exe", path::Style::posix); + EXPECT_EQ(PosixProgName, "clang"); + + StringRef PosixProgNameManyDotsExe = path::program_name( + "/llvm-test16.4/x86_64-portbld-freebsd13.2-llvm-ar.exe", + path::Style::posix); + EXPECT_EQ(PosixProgNameManyDotsExe, "x86_64-portbld-freebsd13.2-llvm-ar"); + + StringRef PosixProgNameManyDots = path::program_name( + "/llvm-test16.4/x86_64-portbld-freebsd13.2-llvm-ar", path::Style::posix); + EXPECT_EQ(PosixProgNameManyDots, "x86_64-portbld-freebsd13.2-llvm-ar"); + + StringRef PosixProgNameSh = + path::program_name("/llvm-test16.4/x86_64-portbld-freebsd13.2-llvm-ar.sh", + path::Style::posix); + EXPECT_EQ(PosixProgNameSh, "x86_64-portbld-freebsd13.2-llvm-ar.sh"); + + StringRef WorseThanFailure = jh7370 wrote: `WorseThanFailure` doesn't describe the case particularly well, especially as the behaviour here is the same as `stem`. Could it simply be `OnlyExe`? https://github.com/llvm/llvm-project/pull/65812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support target names with dots in more utilities (PR #65812)
dankm wrote: > Just making sure I follow: the problem with `stem` is that if a filename > includes a dot, but no ".exe" or equivalent suffix, it would drop the bit > from the last dot onwards, even though it might be part of the tool's actual > name. Is that correct? Yes, that's correct. The real examples I've run into have been `x86_64-unknown-freebsd13.2-clang`, fixed in [D135284](https://reviews.llvm.org/D135284), and the `llvm-ar` and `llvm-ranlib` cases in this PR. In both cases they had previously been using `stem` and returning `x86_64-unknown-freebsd13` and then failing to determine the tool name. I was mostly concerned with the cases where behaviour was affected. Tool name printing is valid too, though, of course. > It looks like a number of tools have changed, but I don't see equivalent test > changes/new tests for all of them. It seems like we should have testing for > all cases where the code goes to some attempt to print the tool name. > Assuming it is, I note that some tools no longer will drop "other" file > extensions. However, I don't know of any real such extensions other than > Window's ".exe". Do any others exist on other platforms? This makes sense. For my first round I did leave out the cases where having multiple dots in the tool name didn't make sense, such as for `dlltool` or `llvm-rc`. You're right though that we should test those. I'll add more tests, and see if I can find more examples of using `stem` or `filename` when printing a tool name. https://github.com/llvm/llvm-project/pull/65812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support target names with dots in more utilities (PR #65812)
dankm wrote: > Do any others exist on other platforms? I don't know. I explicitly tested that `.sh` remains, but that was just because it's relatively common (in my experience) to see tools like clang wrapped in a shell script. On Windows there are several executable extensions but I think the only one that makes sense in our use case is `.exe`, though `.ex_` might also be a possibility. https://github.com/llvm/llvm-project/pull/65812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support target names with dots in more utilities (PR #65812)
@@ -1696,6 +1696,38 @@ TEST(Support, ReplacePathPrefix) { EXPECT_EQ(Path, "C:\\old/foo\\bar"); } +TEST(Support, FindProgramName) { + StringRef WindowsProgName = + path::program_name("C:\\Test\\foo.exe", path::Style::windows); + EXPECT_EQ(WindowsProgName, "foo"); + + StringRef WindowsProgNameManyDots = path::program_name( + "C:\\Test.7\\x86_64-freebsd14.0-clang.exe", path::Style::windows); + EXPECT_EQ(WindowsProgNameManyDots, "x86_64-freebsd14.0-clang"); + + StringRef PosixProgName = + path::program_name("/var/empty/clang.exe", path::Style::posix); + EXPECT_EQ(PosixProgName, "clang"); + + StringRef PosixProgNameManyDotsExe = path::program_name( + "/llvm-test16.4/x86_64-portbld-freebsd13.2-llvm-ar.exe", + path::Style::posix); + EXPECT_EQ(PosixProgNameManyDotsExe, "x86_64-portbld-freebsd13.2-llvm-ar"); + + StringRef PosixProgNameManyDots = path::program_name( + "/llvm-test16.4/x86_64-portbld-freebsd13.2-llvm-ar", path::Style::posix); + EXPECT_EQ(PosixProgNameManyDots, "x86_64-portbld-freebsd13.2-llvm-ar"); + + StringRef PosixProgNameSh = + path::program_name("/llvm-test16.4/x86_64-portbld-freebsd13.2-llvm-ar.sh", + path::Style::posix); + EXPECT_EQ(PosixProgNameSh, "x86_64-portbld-freebsd13.2-llvm-ar.sh"); + + StringRef WorseThanFailure = dankm wrote: Yeah, that one was purely for my own benefit. I was describing behaviour I wasn't sure I wanted to keep, just what it was currently doing. It was intended as a reminder to me to come up 1. A better name, and 2. what the intended output should be. https://github.com/llvm/llvm-project/pull/65812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support target names with dots in more utilities (PR #65812)
@@ -5,11 +5,14 @@ # RUN: mkdir %t # RUN: ln -s llvm-ranlib %t/llvm-ranlib-9 # RUN: ln -s llvm-ranlib %t/ranlib.exe +# RUN: ln -s llvm-ranlib %t/x86_64-unknown-freebsd13.2-llvm-ranlib dankm wrote: Yep, your second point is why I didn't bother making the second case for llvm-ranlib. On my first pass I just updated the existing testcases I found, but for ranlib I realised the same thing you did. I may just drop the ranlib test entirely. https://github.com/llvm/llvm-project/pull/65812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support target names with dots in more utilities (PR #65812)
https://github.com/dankm updated https://github.com/llvm/llvm-project/pull/65812 >From 4a30a862b41aa6c570b0dc4d1eb82a7feef4ed8e Mon Sep 17 00:00:00 2001 From: Dan McGregor Date: Mon, 14 Aug 2023 18:44:08 -0600 Subject: [PATCH 1/2] Support: hoist lld's executable name code to Path Instead of using custom code to find the program name throughout the codebase, write one function as a path helper to consistently determine the program name. This globally correctly finds target names with dots in them (ie freebsd13.2). --- lld/COFF/Driver.cpp| 2 +- lld/Common/Args.cpp| 6 -- lld/ELF/Driver.cpp | 2 +- lld/MachO/Driver.cpp | 2 +- lld/include/lld/Common/Args.h | 2 -- lld/wasm/Driver.cpp| 2 +- lldb/tools/driver/Driver.cpp | 2 +- llvm/include/llvm/Support/Path.h | 15 +++ llvm/lib/Support/Path.cpp | 7 +++ llvm/tools/llvm-ar/llvm-ar.cpp | 3 ++- llvm/tools/llvm-cov/llvm-cov.cpp | 2 +- llvm/tools/llvm-driver/llvm-driver.cpp | 2 +- llvm/tools/llvm-objcopy/llvm-objcopy.cpp | 2 +- llvm/tools/llvm-objdump/llvm-objdump.cpp | 2 +- llvm/tools/llvm-rc/llvm-rc.cpp | 2 +- llvm/tools/llvm-readobj/llvm-readobj.cpp | 2 +- llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp | 2 +- 17 files changed, 36 insertions(+), 21 deletions(-) diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp index d7476e91e03e384..03a63bf3a306b4d 100644 --- a/lld/COFF/Driver.cpp +++ b/lld/COFF/Driver.cpp @@ -68,7 +68,7 @@ bool link(ArrayRef args, llvm::raw_ostream &stdoutOS, auto *ctx = new COFFLinkerContext; ctx->e.initialize(stdoutOS, stderrOS, exitEarly, disableOutput); - ctx->e.logName = args::getFilenameWithoutExe(args[0]); + ctx->e.logName = sys::path::program_name(args[0]); ctx->e.errorLimitExceededMsg = "too many errors emitted, stopping now" " (use /errorlimit:0 to see all errors)"; diff --git a/lld/Common/Args.cpp b/lld/Common/Args.cpp index 48c934df3a2c931..b02d023fe19aeb5 100644 --- a/lld/Common/Args.cpp +++ b/lld/Common/Args.cpp @@ -85,9 +85,3 @@ std::vector lld::args::getLines(MemoryBufferRef mb) { } return ret; } - -StringRef lld::args::getFilenameWithoutExe(StringRef path) { - if (path.ends_with_insensitive(".exe")) -return sys::path::stem(path); - return sys::path::filename(path); -} diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp index 9d167293574fa64..957d0da1589caf8 100644 --- a/lld/ELF/Driver.cpp +++ b/lld/ELF/Driver.cpp @@ -143,7 +143,7 @@ bool link(ArrayRef args, llvm::raw_ostream &stdoutOS, SharedFile::vernauxNum = 0; }; - ctx->e.logName = args::getFilenameWithoutExe(args[0]); + ctx->e.logName = sys::path::program_name(args[0]); ctx->e.errorLimitExceededMsg = "too many errors emitted, stopping now (use " "--error-limit=0 to see all errors)"; diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp index 00ff3439a043be9..ed0b66d4b2ec6ba 100644 --- a/lld/MachO/Driver.cpp +++ b/lld/MachO/Driver.cpp @@ -1428,7 +1428,7 @@ bool link(ArrayRef argsArr, llvm::raw_ostream &stdoutOS, InputFile::resetIdCount(); }; - ctx->e.logName = args::getFilenameWithoutExe(argsArr[0]); + ctx->e.logName = sys::path::program_name(argsArr[0]); MachOOptTable parser; InputArgList args = parser.parse(argsArr.slice(1)); diff --git a/lld/include/lld/Common/Args.h b/lld/include/lld/Common/Args.h index 60f83fbbbf1a3c9..8822707a6a1f212 100644 --- a/lld/include/lld/Common/Args.h +++ b/lld/include/lld/Common/Args.h @@ -38,8 +38,6 @@ uint64_t getZOptionValue(llvm::opt::InputArgList &args, int id, StringRef key, std::vector getLines(MemoryBufferRef mb); -StringRef getFilenameWithoutExe(StringRef path); - } // namespace args } // namespace lld diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp index c2f5f0185781f36..f13aa520f35b100 100644 --- a/lld/wasm/Driver.cpp +++ b/lld/wasm/Driver.cpp @@ -89,7 +89,7 @@ bool link(ArrayRef args, llvm::raw_ostream &stdoutOS, auto *ctx = new CommonLinkerContext; ctx->e.initialize(stdoutOS, stderrOS, exitEarly, disableOutput); - ctx->e.logName = args::getFilenameWithoutExe(args[0]); + ctx->e.logName = sys::path::program_name(args[0]); ctx->e.errorLimitExceededMsg = "too many errors emitted, stopping now (use " "-error-limit=0 to see all errors)"; diff --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp index f8058f868d53ffe..2fb760ca97a001d 100644 --- a/lldb/tools/driver/Driver.cpp +++ b/lldb/tools/driver/Driver.cpp @@ -754,7 +754,7 @@ int main(int argc, char const *argv[]) { ArrayRef arg_arr = ArrayRef(argv + 1, argc - 1); opt::InputArgList input_args = T.ParseArgs(arg_arr, MissingA
[clang] Support target names with dots in more utilities (PR #65812)
https://github.com/dankm resolved https://github.com/llvm/llvm-project/pull/65812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support target names with dots in more utilities (PR #65812)
https://github.com/dankm updated https://github.com/llvm/llvm-project/pull/65812 >From 5f5e57a5c01088e7f0b9276ed02dd436a3f9cc0a Mon Sep 17 00:00:00 2001 From: Dan McGregor Date: Mon, 14 Aug 2023 18:44:08 -0600 Subject: [PATCH 1/2] Support: hoist lld's executable name code to Path Instead of using custom code to find the program name throughout the codebase, write one function as a path helper to consistently determine the program name. This globally correctly finds target names with dots in them (ie freebsd13.2). --- lld/COFF/Driver.cpp| 2 +- lld/Common/Args.cpp| 6 -- lld/ELF/Driver.cpp | 2 +- lld/MachO/Driver.cpp | 2 +- lld/include/lld/Common/Args.h | 2 -- lld/wasm/Driver.cpp| 2 +- lldb/tools/driver/Driver.cpp | 2 +- llvm/include/llvm/Support/Path.h | 15 +++ llvm/lib/Support/Path.cpp | 7 +++ llvm/tools/llvm-ar/llvm-ar.cpp | 3 ++- llvm/tools/llvm-cov/llvm-cov.cpp | 2 +- llvm/tools/llvm-driver/llvm-driver.cpp | 2 +- llvm/tools/llvm-objcopy/llvm-objcopy.cpp | 2 +- llvm/tools/llvm-objdump/llvm-objdump.cpp | 2 +- llvm/tools/llvm-rc/llvm-rc.cpp | 2 +- llvm/tools/llvm-readobj/llvm-readobj.cpp | 2 +- llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp | 2 +- 17 files changed, 36 insertions(+), 21 deletions(-) diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp index d7476e91e03e384..03a63bf3a306b4d 100644 --- a/lld/COFF/Driver.cpp +++ b/lld/COFF/Driver.cpp @@ -68,7 +68,7 @@ bool link(ArrayRef args, llvm::raw_ostream &stdoutOS, auto *ctx = new COFFLinkerContext; ctx->e.initialize(stdoutOS, stderrOS, exitEarly, disableOutput); - ctx->e.logName = args::getFilenameWithoutExe(args[0]); + ctx->e.logName = sys::path::program_name(args[0]); ctx->e.errorLimitExceededMsg = "too many errors emitted, stopping now" " (use /errorlimit:0 to see all errors)"; diff --git a/lld/Common/Args.cpp b/lld/Common/Args.cpp index 48c934df3a2c931..b02d023fe19aeb5 100644 --- a/lld/Common/Args.cpp +++ b/lld/Common/Args.cpp @@ -85,9 +85,3 @@ std::vector lld::args::getLines(MemoryBufferRef mb) { } return ret; } - -StringRef lld::args::getFilenameWithoutExe(StringRef path) { - if (path.ends_with_insensitive(".exe")) -return sys::path::stem(path); - return sys::path::filename(path); -} diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp index 9d167293574fa64..957d0da1589caf8 100644 --- a/lld/ELF/Driver.cpp +++ b/lld/ELF/Driver.cpp @@ -143,7 +143,7 @@ bool link(ArrayRef args, llvm::raw_ostream &stdoutOS, SharedFile::vernauxNum = 0; }; - ctx->e.logName = args::getFilenameWithoutExe(args[0]); + ctx->e.logName = sys::path::program_name(args[0]); ctx->e.errorLimitExceededMsg = "too many errors emitted, stopping now (use " "--error-limit=0 to see all errors)"; diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp index 00ff3439a043be9..ed0b66d4b2ec6ba 100644 --- a/lld/MachO/Driver.cpp +++ b/lld/MachO/Driver.cpp @@ -1428,7 +1428,7 @@ bool link(ArrayRef argsArr, llvm::raw_ostream &stdoutOS, InputFile::resetIdCount(); }; - ctx->e.logName = args::getFilenameWithoutExe(argsArr[0]); + ctx->e.logName = sys::path::program_name(argsArr[0]); MachOOptTable parser; InputArgList args = parser.parse(argsArr.slice(1)); diff --git a/lld/include/lld/Common/Args.h b/lld/include/lld/Common/Args.h index 60f83fbbbf1a3c9..8822707a6a1f212 100644 --- a/lld/include/lld/Common/Args.h +++ b/lld/include/lld/Common/Args.h @@ -38,8 +38,6 @@ uint64_t getZOptionValue(llvm::opt::InputArgList &args, int id, StringRef key, std::vector getLines(MemoryBufferRef mb); -StringRef getFilenameWithoutExe(StringRef path); - } // namespace args } // namespace lld diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp index c2f5f0185781f36..f13aa520f35b100 100644 --- a/lld/wasm/Driver.cpp +++ b/lld/wasm/Driver.cpp @@ -89,7 +89,7 @@ bool link(ArrayRef args, llvm::raw_ostream &stdoutOS, auto *ctx = new CommonLinkerContext; ctx->e.initialize(stdoutOS, stderrOS, exitEarly, disableOutput); - ctx->e.logName = args::getFilenameWithoutExe(args[0]); + ctx->e.logName = sys::path::program_name(args[0]); ctx->e.errorLimitExceededMsg = "too many errors emitted, stopping now (use " "-error-limit=0 to see all errors)"; diff --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp index f8058f868d53ffe..2fb760ca97a001d 100644 --- a/lldb/tools/driver/Driver.cpp +++ b/lldb/tools/driver/Driver.cpp @@ -754,7 +754,7 @@ int main(int argc, char const *argv[]) { ArrayRef arg_arr = ArrayRef(argv + 1, argc - 1); opt::InputArgList input_args = T.ParseArgs(arg_arr, MissingA
[clang] Support target names with dots in more utilities (PR #65812)
https://github.com/dankm resolved https://github.com/llvm/llvm-project/pull/65812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support target names with dots in more utilities (PR #65812)
https://github.com/jh7370 edited https://github.com/llvm/llvm-project/pull/65812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support target names with dots in more utilities (PR #65812)
@@ -1696,6 +1696,40 @@ TEST(Support, ReplacePathPrefix) { EXPECT_EQ(Path, "C:\\old/foo\\bar"); } +TEST(Support, FindProgramName) { + StringRef WindowsProgName = + path::program_name("C:\\Test\\foo.exe", path::Style::windows); + EXPECT_EQ(WindowsProgName, "foo"); + + StringRef WindowsProgNameManyDots = path::program_name( + "C:\\Test.7\\x86_64-freebsd14.0-clang.exe", path::Style::windows); + EXPECT_EQ(WindowsProgNameManyDots, "x86_64-freebsd14.0-clang"); + + StringRef PosixProgName = + path::program_name("/var/empty/clang.exe", path::Style::posix); + EXPECT_EQ(PosixProgName, "clang"); + + StringRef PosixProgNameManyDotsExe = path::program_name( + "/llvm-test16.4/x86_64-portbld-freebsd13.2-llvm-ar.exe", + path::Style::posix); + EXPECT_EQ(PosixProgNameManyDotsExe, "x86_64-portbld-freebsd13.2-llvm-ar"); + + StringRef PosixProgNameManyDots = path::program_name( + "/llvm-test16.4/x86_64-portbld-freebsd13.2-llvm-ar", path::Style::posix); + EXPECT_EQ(PosixProgNameManyDots, "x86_64-portbld-freebsd13.2-llvm-ar"); + + StringRef PosixProgNameSh = + path::program_name("/llvm-test16.4/x86_64-portbld-freebsd13.2-llvm-ar.sh", + path::Style::posix); + EXPECT_EQ(PosixProgNameSh, "x86_64-portbld-freebsd13.2-llvm-ar.sh"); + + // TODO: determine if this is correct. What happens on windows with an executable + // named ".exe"? jh7370 wrote: I just used the Windows UI to rename a file to ".exe" and it works fine (both renaming and susbequent execution of the file from the command-line). Meanwhile, the documented behaviour of `stem` is that something containing only a single dot will result in an empty string, which I think we should be mirroring here. I think the behaviour tested here is therefore correct and you can probably drop the TODO note. https://github.com/llvm/llvm-project/pull/65812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support target names with dots in more utilities (PR #65812)
@@ -5,11 +5,14 @@ # RUN: mkdir %t # RUN: ln -s llvm-ranlib %t/llvm-ranlib-9 # RUN: ln -s llvm-ranlib %t/ranlib.exe +# RUN: ln -s llvm-ranlib %t/x86_64-unknown-freebsd13.2-llvm-ranlib jh7370 wrote: Let's put the new test files and deletion of this old test in a different PR. The old code was untested, so we're not making things worse, but it also helps keep the PRs focused. Aside: if we're deleting this old file, I think it would be a good idea to add one or two cases to the llvm-ar test showing the "llvm-ranlib" name. https://github.com/llvm/llvm-project/pull/65812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits