[lldb][NativePDB] Recognize bound generic pattern#12824
[lldb][NativePDB] Recognize bound generic pattern#12824speednoisemovement wants to merge 2 commits intoswiftlang:stable/21.xfrom
Conversation
Bound generic types are represented by nested structs arranged in a certain way (see swiftlang/swift#87093 for details). Similar to DWARF, this change adds special handling to recognize this pattern and recover the mangled name of the actual type.
|
@swift-ci Please test |
speednoisemovement
requested review from
adrian-prantl,
charles-zablit and
compnerd
| @@ -18,6 +18,15 @@ enum Direction { | |||
| case south | |||
There was a problem hiding this comment.
Since this is a NativePDB test, is it running on macOS? Asking since the lldb swift tests are currently not enabled on Windows.
There was a problem hiding this comment.
As of #12610 yes, unless something changed (I know because I had to remove the XFAIL on this test to land it!)
There was a problem hiding this comment.
Wait, I misread the question. It's not running on macOS (requires system-windows) is running on Windows last I checked.
| // https://github.com/swiftlang/swift/issues/87093 | ||
| // FIXME: This is more of a heuristic than a definitive check, but we would need to do a | ||
| // second field-list walk to be sure. | ||
| return cr.Name.ends_with("::<unnamed-tag>"); |
There was a problem hiding this comment.
Given the example you gave here: swiftlang/swift#87093 (comment), could MSVC emit ::<unnamed-tag> for a C/C++ type?
If tha'ts the case, thisIsSwiftType would return true on a non Swift type.
There was a problem hiding this comment.
If this is possible, we should:
- Avoid false positives by checking that the outer type is a swift type.
- Add a negative test in
swift-frame-var.test.
There was a problem hiding this comment.
Yup, I can wire the field-list walk up twice. My thinking for not doing it was the perf gain and of not doing it is worth the edge case (no unique name, no name C/C++ type that nevertheless has something useful that TypeSystemClang can squeeze out of it) and cleaner code. I think probably both the perf hit is small and the edge case is not likely to matter though.
What would a good negative case look like?
There was a problem hiding this comment.
I tried to build a C++ type that has ::<unnamed-tag> in its name and does not have a unique name but I could not manage to do it. That would have been a good negative test but it does not seem to be possible.
My concern was unfounded.
There was a problem hiding this comment.
Yep, AFAICT Clang can't produce it, but maybe another compiler can? I added the full check to IsSwiftType since the perf hit isn't that bad and the cognitive overhead is probably worth us being sure.
2 participants
Bound generic types are represented by nested structs arranged in a certain way (see swiftlang/swift#87093 for details).
Similar to DWARF, this change adds special handling to recognize this pattern and recover the mangled name of the actual type.