sepavloff added inline comments.
================
Comment at: llvm/lib/Support/CommandLine.cpp:1188
+ // macros.
+ if (!RelativeNames && !InConfigFile)
return Error::success();
----------------
rovka wrote:
> Why do you need to add `!InConfigFile` here and below?
It is not necessary, because now configuration file is expanded with the flag
`RelativeNames` set. It however seems more correct to check for `InConfigFile`
also, hopefully it make code a bit clearly.
================
Comment at: llvm/lib/Support/CommandLine.cpp:1204
+ StringRef FileName;
+ if (ArgStr[0] == '@') {
+ FileName = ArgStr.drop_front(1);
----------------
rovka wrote:
> Nit: Why reverse this branch? The code around it `continue`s early, so it's
> easier to read if this `continue`s early too.
You are right. Initially this patch was combined with D136354, where additional
checks was added. I changed this code to be more natural.
================
Comment at: llvm/lib/Support/CommandLine.cpp:1205
+ if (ArgStr[0] == '@') {
+ FileName = ArgStr.drop_front(1);
+ if (!llvm::sys::path::is_relative(FileName))
----------------
mgorny wrote:
> Also, I think you can use `consume_front()` here.
Yes, changed code accordingly.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136090/new/
https://reviews.llvm.org/D136090
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
