-
Notifications
You must be signed in to change notification settings - Fork 564
Conversation
private fun ObjCExportCodeGenerator.generateExceptionTypeInfoArray(baseMethod: IrFunction): LLVMValueRef = | ||
exceptionTypeInfoArrays.getOrPut(baseMethod) { | ||
val throwsAnnotation = findThrowsAnnotation(baseMethod)!! | ||
val types = (throwsAnnotation.getValueArgument(0) as IrVararg).elements |
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.
Why those casts are safe?
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.
Frontend and psi2ir are supposed to guarantee this.
I will make cast failures more readable though.
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.
Somewhat improved.
if (baseMethod !is IrSimpleFunction || baseMethod.overriddenSymbols.isEmpty()) { | ||
baseMethod.annotations.findAnnotation(KonanFqNames.throws) | ||
} else { | ||
// Note: frontend ensures that all topmost overridden methods have equal @Throws annotations. |
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.
Can/shall we assert for that condition?
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.
Technically adding such an assertion should be easy.
However, considering binary compatibility (e.g. linking with other versions of libraries), it is actually possible that such a condition isn't met.
We don't have much choice in this case: either report compile (link?) error or handle this silently.
And I would strongly prefer to avoid reporting any errors from compiler, at least because we don't have proper stable infrastructure (in IR linker) to do this reliably. WDYT?
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.
OK, let's keep it silently processed so far. Maybe warning?
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.
Reporting warning makes sense, however we still don't have proper infrastructure for this. Generally I believe we should cease reporting compiler diagnostics from backend the way we are doing it now.
I expect this particular case to be quite rare, so it should be ok to postpone adding such a warning. Especially considering that I'm not completely sure about the details of new @Throws
design, so it can change before 1.4.
It now enables propagation only for instances of listed classes.
8408272
to
43f59c7
Compare
It now enables propagation only for instances of listed classes.