-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[DebugInfo] Emit witness and objc method declarations in debug info #71965
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@swift-ci smoke test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great find!
if (Rep == SILFunctionTypeRepresentation::Method) { | ||
if (Rep == SILFunctionTypeRepresentation::Method || | ||
Rep == SILFunctionTypeRepresentation::ObjCMethod || | ||
Rep == SILFunctionTypeRepresentation::WitnessMethod) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity — what other representations are there and do they all not appear as children of a DICompositeType?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the entire list:
enum class SILFunctionTypeRepresentation : uint8_t {
/// A freestanding thick function.
Thick = uint8_t(FunctionTypeRepresentation::Swift),
/// A thick function that is represented as an Objective-C block.
Block = uint8_t(FunctionTypeRepresentation::Block),
/// A freestanding thin function that needs no context.
Thin = uint8_t(FunctionTypeRepresentation::Thin),
/// A C function pointer, which is thin and also uses the C calling
/// convention.
CFunctionPointer = uint8_t(FunctionTypeRepresentation::CFunctionPointer),
/// The value of the greatest AST function representation.
LastAST = CFunctionPointer,
/// The value of the least SIL-only function representation.
FirstSIL = 8,
/// A Swift instance method.
Method = FirstSIL,
/// An Objective-C method.
ObjCMethod,
/// A Swift protocol witness.
WitnessMethod,
/// A closure invocation function that has not been bound to a context.
Closure,
/// A C++ method that takes a "this" argument (not a static C++ method or
/// constructor). Except for
/// handling the "this" argument, has the same behavior as "CFunctionPointer".
CXXMethod,
/// A KeyPath accessor function, which is thin and also uses the variadic
/// length generic components serialization in trailing buffer.
/// Each representation has a different convention for which parameters
/// have serialized generic type info.
KeyPathAccessorGetter,
KeyPathAccessorSetter,
KeyPathAccessorEquals,
KeyPathAccessorHash,
}
Could a CXXMethod
conceivably show up here? I added an assertion to check for that and ran the swift test suite, but didn't encounter any such cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the inlining problem could happen we probably need to do this for every function that has a DICompositeType as parent.
@swift-ci test source compatibility |
When emitting a method definition in debug info, the compiler should also emit the method's declaration, because LLVM LTO can't unify type definitions when a child DIE is a full subprogram definition. This is already the behavior for standard methods, this patch implements the same behavior for witness and objc methods as well. rdar://123334375
648d239
to
53d1844
Compare
@swift-ci smoke test |
@swift-ci test source compatibility |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Slightly surprised this didn't break earlier!
I don't think lto is often used when compiling Swift :) |
I'm going to merge this regardless, but I think we need to add more cases. |
When emitting a method definition in debug info, the compiler should also emit the method's declaration, because LLVM LTO can't unify type definitions when a child DIE is a full subprogram definition. This is already the behavior for standard methods, this patch implements the same behavior for witness and objc methods as well.
rdar://123334375