[PATCH] D43002: [CodeView] Emit S_OBJNAME record

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 21 10:14:22 PST 2021


MaskRay added a comment.

Some test nits:) I don't know CodeView :(



================
Comment at: clang/test/CodeGenCXX/debug-info-objname.cpp:1
+// RUN: cp %s %T/debug-info-objname.cpp
+// RUN: cd %T
----------------
The whole file appears to be CRLF where almost all other tests use LF in llvm-project.


================
Comment at: clang/test/CodeGenCXX/debug-info-objname.cpp:2
+// RUN: cp %s %T/debug-info-objname.cpp
+// RUN: cd %T
+
----------------
%T is deprecated in lit (https://llvm.org/docs/CommandGuide/lit.html#substitutions): it is easy to cause multi-threading file reuse problems because lit runs tests parallelly.

I personally prefer `RUN: rm -rf %t && mkdir %t && cd %t` if you need to cd.

My personal comment style for non-RUN non-CHECK lines is `///` - it stands out in many editors and may help possible FileCheck/lit's future direction to catch stale CHECK prefixes.


================
Comment at: clang/test/CodeGenCXX/debug-info-objname.cpp:30
+// RUN: %clang_cl /c /Z7 -nostdinc -fdebug-compilation-dir=. /clang:-save-temps debug-info-objname.cpp
+// RUN: cat debug-info-objname.asm | FileCheck %s --check-prefix=ASM
+
----------------
Prefer `--input-file` or input redirection to `cat xxx | yyy`


================
Comment at: clang/test/CodeGenCXX/debug-info-objname.cpp:36
+
+// ABSOLUTE: S_OBJNAME [size = {{[0-9]+}}] sig=0, `{{.+}}debug-info-objname.obj`
+// RELATIVE: S_OBJNAME [size = {{[0-9]+}}] sig=0, `debug-info-objname.obj`
----------------
`{{[0-9]+}}` -> `[[#]]`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D43002



More information about the llvm-commits mailing list