[Lldb-commits] [PATCH] D146221: [CodeView] Add source languages ObjC and ObjC++

[David Spickett via Phabricator via lldb-commits]

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