[Lldb-commits] [PATCH] D146221: [CodeView] Add source languages ObjC and ObjC++
David Spickett via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Fri Mar 17 08:39:56 PDT 2023
DavidSpickett added inline comments.
================
Comment at: llvm/test/DebugInfo/COFF/objc.ll:4-6
+; ASM: .short 4412 # Record kind: S_COMPILE3
+; ASM-NEXT: .long 17 # Flags and language
+; ASM-NEXT: .short 208 # CPUType
----------------
DavidSpickett wrote:
> sgraenitz wrote:
> > DavidSpickett wrote:
> > > Are these checked, should they be?
> > Thanks for taking a look! What we check here is the `17` in language flags. The rest is just to make sure we get the right context. (Same for the others.) Is that your question?
> Right but I don't know what that adds once you get beyond the Language line. It's a small thing, so that the test doesn't fail if a new key is added.
Ignore the previous comment, wrong line.
So yes you can check these, but no one actually does. If you look at the filecheck line it only does OBJ.
Same for the rust change you linked. Unless there is some implicit behaviour here. One way to find out, put a bogus value in there and it should fail.
================
Comment at: llvm/test/DebugInfo/COFF/objc.ll:11-17
+; OBJ-NEXT: Flags [ (0x0)
+; OBJ-NEXT: ]
+; OBJ-NEXT: Machine: X64 (0xD0)
+; OBJ-NEXT: FrontendVersion: {{[0-9\.]*}}
+; OBJ-NEXT: BackendVersion: {{[0-9\.]*}}
+; OBJ-NEXT: VersionName: clang version 17.0.0 (https://github.com/llvm/llvm-project a8e9beca6bee1f248ef4be7892802c4d091b7fcb)
+; OBJ-NEXT: }
----------------
DavidSpickett wrote:
> Do we need to check for these lines? Just the first few should be fine.
So here it's fine to check up to language, mostly, but after that what does it matter? If someone adds a new key the test breaks.
(and granted, it won't happen often if at all but the general principle holds)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146221/new/
https://reviews.llvm.org/D146221
More information about the lldb-commits
mailing list