dexonsmith added inline comments.
================
Comment at: clang/lib/Tooling/Tooling.cpp:119-122
+ if (isa<driver::AssembleJobAction>(A)) {
+ ExternalAssembler = true;
+ break;
+ }
----------------
Seems like this could (unexpectedly?) let through a command with multiple
`-cc1`s (if I'm reading the code right). Consider these two cases:
```
lang=zsh
% clang -x c a.c b.c -c -fno-integrated-as -###
% clang -arch x86_64 -arch arm64 -x c /dev/null -c -fno-integrated-as -###
```
The first compiles two separate files; the second compiles a file with two
separate architectures. But IIUC, the logic below doesn't expect this and will
now let them through (ultimately, there's no reason scan-deps couldn't be
updated to handle this! but that's not in scope for this patch, and maybe
doesn't apply generally to clang-tooling).
Also, I'm not sure it's good that it was rejected, but I think the previous
code would reject the following command-line and the new code will accept it:
```
% clang -x c /dev/null -###
```
I wonder if it'd be simpler to search for a supported action, and reject if
zero or multiple are found. Something like:
```
lang=c++
const driver::Command *Cmd = nullptr;
for (const Action *A : Actions) {
A = lookThrough(A);
if (!shouldIgnore(*A)) // ignore some actions...
continue;
if (shouldReject(*A)) // if needed, error on some actions...
return error(...);
if (Cmd) // error if we hit a second candidate
return error(...);
Cmd = cast<driver::Command>(*A);
}
if (!Cmd) // error if we found no candidate
return error(...);
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105695/new/
https://reviews.llvm.org/D105695
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
