Meinersbur requested changes to this revision.
Meinersbur added a comment.
This revision now requires changes to proceed.
Thanks for the path, but command line parsing should be done properly.
================
Comment at: clang/tools/clang-nvlink-wrapper/ClangNvlinkWrapper.cpp:50
+static cl::opt<std::string>
+ NvlinkUserPath("path", cl::desc("path of directory containing nvlink"),
+ cl::cat(ClangNvlinkWrapperCategory));
----------------
I suggest a more descriptive flag, `path` is too general, like `--nvlink` or
`--nvlink-command` (bugpoint uses `--opt-command` for the path to `opt`)
================
Comment at: clang/tools/clang-nvlink-wrapper/ClangNvlinkWrapper.cpp:124
sys::PrintStackTraceOnErrorSignal(argv[0]);
-
if (Help) {
----------------
[nit] unrelated whitespace change
================
Comment at: clang/tools/clang-nvlink-wrapper/ClangNvlinkWrapper.cpp:133
sys::PrintStackTraceOnErrorSignal(argv[0]);
-
if (Help) {
cl::PrintHelpMessage();
----------------
`cl::ParseCommandLineOptions(argc, argv,"Program description")` must have been
called beforehand to make this work.
================
Comment at: clang/tools/clang-nvlink-wrapper/ClangNvlinkWrapper.cpp:154
+ StringRef ArgRef(Arg);
+ auto NvlPath = ArgRef.startswith_insensitive("--path=");
if (sys::path::extension(Arg) == ".a") {
----------------
This way of parsing does not use the declared `NvlinkUserPath` option. Use
`cl::ParseCommandLineOptions` do parse the arguments and
```
cl::list<std::string> InputFiles(cl::Positional, cl::desc("<input files>..."));
```
to catch all non-option arguments.
The way it is done here also does not catch the `--path <nvlink>` (without
equal sign) syntax.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111488/new/
https://reviews.llvm.org/D111488
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
