ChuanqiXu marked an inline comment as done.
ChuanqiXu added inline comments.


================
Comment at: clang/test/Driver/save-std-c++-module-file.cpp:8-9
+//
+// RUN: %clang -std=c++20 %t/Hello.cppm -fmodule-output -o %t/output/Hello.o \
+// RUN:   -### 2>&1 | FileCheck %t/Hello.cppm -DPREFIX=%t 
--check-prefix=CHECK-WITH-OUTPUT
+
----------------
dblaikie wrote:
> Not sure I understand the need for two tests here - they both specify an 
> absolute path to a .o file & CHECK that the absolute path matches the .pcm 
> output file path, so they don't seem to be testing distinct scenarios?
The above one doesn't specify the path for the .o file. So in the first test 
the .pcm file will be in the same directory with the input source file. And in 
the second command line, the .pcm file will be in the same directory with the 
.o file.


================
Comment at: clang/test/Driver/save-std-c++-module-file.cpp:1-4
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
----------------
dblaikie wrote:
> dblaikie wrote:
> > Is this needed? Maybe we don't need to split the file, if it's just the one 
> > file anyway?
> Ping on this ^
Sorry for missing it. Thanks for clarification.


================
Comment at: clang/test/Driver/save-std-c++-module-file.cpp:6
+// RUN: %clang -std=c++20 %t/Hello.cppm -fsave-std-c++-module-file -### 2>&1 | 
\
+// RUN:   FileCheck %t/Hello.cppm -DPREFIX=%t
+//
----------------
dblaikie wrote:
> Is the prefix needed? I'd expect we'd usually regex match away the actual 
> directory with `{{.*}}` in tests like this?
Yeah, generally we would use `{{.*}}`. And `-DPREFIX` is more precise for what 
we want to test. So I feel it wouldn't be bad.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137058/new/

https://reviews.llvm.org/D137058

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to