jansvoboda11 updated this revision to Diff 438367.
jansvoboda11 added a comment.
Code review feedback
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122385/new/
https://reviews.llvm.org/D122385
Files:
clang/test/ClangScanDeps/cl-output.c
clang/tools/clang-scan-deps/ClangScanDeps.cpp
Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===================================================================
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -462,27 +462,31 @@
llvm::sys::path::stem(Args[0]).contains_insensitive("clang-cl") ||
llvm::is_contained(Args, "--driver-mode=cl");
- // Reverse scan, starting at the end or at the element before "--".
- auto R = std::make_reverse_iterator(FlagsEnd);
- for (auto I = R, E = Args.rend(); I != E; ++I) {
+ for (auto I = Args.begin(); I < FlagsEnd; ++I) {
StringRef Arg = *I;
if (ClangCLMode) {
// Ignore arguments that are preceded by "-Xclang".
- if ((I + 1) != E && I[1] == "-Xclang")
+ if (Arg == "-Xclang") {
+ ++I;
continue;
- if (LastO.empty()) {
- // With clang-cl, the output obj file can be specified with
- // "/opath", "/o path", "/Fopath", and the dash counterparts.
- // Also, clang-cl adds ".obj" extension if none is found.
- if ((Arg == "-o" || Arg == "/o") && I != R)
- LastO = I[-1]; // Next argument (reverse iterator)
- else if (Arg.startswith("/Fo") || Arg.startswith("-Fo"))
- LastO = Arg.drop_front(3).str();
- else if (Arg.startswith("/o") || Arg.startswith("-o"))
- LastO = Arg.drop_front(2).str();
-
- if (!LastO.empty() && !llvm::sys::path::has_extension(LastO))
- LastO.append(".obj");
+ }
+
+ // With clang-cl, the output obj file can be specified with
+ // "/opath", "/o path", "/Fopath", and the dash counterparts.
+ // Also, clang-cl adds ".obj" extension if none is found.
+ StringRef CurrentO;
+ if ((Arg == "/o" || Arg == "-o") && I + 1 < FlagsEnd)
+ CurrentO = *++I;
+ else if (Arg.startswith("/Fo") || Arg.startswith("-Fo"))
+ CurrentO = Arg.drop_front(3);
+ else if (Arg.startswith("/o") || Arg.startswith("-o"))
+ CurrentO = Arg.drop_front(2);
+
+ if (!CurrentO.empty()) {
+ if (!llvm::sys::path::has_extension(CurrentO))
+ LastO = (CurrentO + ".obj").str();
+ else
+ LastO = CurrentO.str();
}
}
if (Arg == "-resource-dir")
Index: clang/test/ClangScanDeps/cl-output.c
===================================================================
--- /dev/null
+++ clang/test/ClangScanDeps/cl-output.c
@@ -0,0 +1,93 @@
+// This test checks that the output path is correctly deduced/recognized in clang-cl mode.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+//--- deduce-cdb.json.template
+[{
+ "file": "DIR/test.c",
+ "directory": "DIR",
+ "command": "clang --driver-mode=cl /c -- DIR/test.c"
+},{
+ "file": "DIR/test.c",
+ "directory": "DIR",
+ "command": "clang-cl /c -- DIR/test.c"
+}]
+
+//--- recognize-cdb.json.template
+[{
+ "file": "DIR/test.c",
+ "directory": "DIR",
+ "command": "clang-cl /c -o DIR/test.o -- DIR/test.c"
+},{
+ "file": "DIR/test.c",
+ "directory": "DIR",
+ "command": "clang-cl /c /o DIR/test.o -- DIR/test.c"
+},{
+ "file": "DIR/test.c",
+ "directory": "DIR",
+ "command": "clang-cl /c -oDIR/test.o -- DIR/test.c"
+},{
+ "file": "DIR/test.c",
+ "directory": "DIR",
+ "command": "clang-cl /c /oDIR/test.o -- DIR/test.c"
+},{
+ "file": "DIR/test.c",
+ "directory": "DIR",
+ "command": "clang-cl /c -FoDIR/test.o -- DIR/test.c"
+},{
+ "file": "DIR/test.c",
+ "directory": "DIR",
+ "command": "clang-cl /c /FoDIR/test.o -- DIR/test.c"
+}]
+
+//--- last-arg-cdb.json.template
+[{
+ "file": "DIR/test.c",
+ "directory": "DIR",
+ "command": "clang-cl /c -o DIR/test.o -o DIR/last.o -- DIR/test.c"
+},{
+ "file": "DIR/test.c",
+ "directory": "DIR",
+ "command": "clang-cl /c /o /opt/test.o -- DIR/test.c"
+}]
+
+//--- test.c
+
+// Check that missing output path is deduced (with both clang-cl executable and driver mode flag):
+//
+// RUN: sed -e "s|DIR|%/t|g" %t/deduce-cdb.json.template > %t/deduce-cdb.json
+// RUN: clang-scan-deps -compilation-database %t/deduce-cdb.json -j 1 > %t/deduce-result.d
+// RUN: cat %t/deduce-result.d | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t --check-prefix=CHECK-DEDUCE
+// CHECK-DEDUCE: test.obj:
+// CHECK-DEDUCE-NEXT: [[PREFIX]]/test.c
+// CHECK-DEDUCE-NEXT: test.obj:
+// CHECK-DEDUCE-NEXT: [[PREFIX]]/test.c
+
+// Check that all the different ways to specify output file are recognized:
+//
+// RUN: sed -e "s|DIR|%/t|g" %t/recognize-cdb.json.template > %t/recognize-cdb.json
+// RUN: clang-scan-deps -compilation-database %t/recognize-cdb.json -j 1 > %t/recognize-result.d
+// RUN: cat %t/recognize-result.d | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t --check-prefix=CHECK-RECOGNIZE
+// CHECK-RECOGNIZE: [[PREFIX]]/test.o:
+// CHECK-RECOGNIZE-NEXT: [[PREFIX]]/test.c
+// CHECK-RECOGNIZE-NEXT: [[PREFIX]]/test.o:
+// CHECK-RECOGNIZE-NEXT: [[PREFIX]]/test.c
+// CHECK-RECOGNIZE-NEXT: [[PREFIX]]/test.o:
+// CHECK-RECOGNIZE-NEXT: [[PREFIX]]/test.c
+// CHECK-RECOGNIZE-NEXT: [[PREFIX]]/test.o:
+// CHECK-RECOGNIZE-NEXT: [[PREFIX]]/test.c
+// CHECK-RECOGNIZE-NEXT: [[PREFIX]]/test.o:
+// CHECK-RECOGNIZE-NEXT: [[PREFIX]]/test.c
+// CHECK-RECOGNIZE-NEXT: [[PREFIX]]/test.o:
+// CHECK-RECOGNIZE-NEXT: [[PREFIX]]/test.c
+
+// Check that the last argument specifying the output path wins.
+//
+// RUN: sed -e "s|DIR|%/t|g" %t/last-arg-cdb.json.template > %t/last-arg-cdb.json
+// RUN: clang-scan-deps -compilation-database %t/last-arg-cdb.json -j 1 > %t/last-arg-result.d
+// RUN: cat %t/last-arg-result.d | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t --check-prefix=CHECK-LAST
+// CHECK-LAST: [[PREFIX]]/last.o:
+// CHECK-LAST-NEXT: [[PREFIX]]/test.c
+// CHECK-LAST-NEXT: /opt/test.o:
+// CHECK-LAST-NEXT: [[PREFIX]]/test.c
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
