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
fix: compiler crash on primitive type checking against opaque types #3712
Conversation
opaque type cmark_event_type = CUnsignedInt | ||
object cmark_event_type: | ||
inline def define(inline a: Long): cmark_event_type = a.toUInt | ||
val CMARK_EVENT_NONE = define(0) | ||
|
||
@main def hello(): Unit = | ||
val evtype = cmark_event_type.CMARK_EVENT_NONE | ||
stdio.printf(c"bla: %s, hello: %d", evtype) | ||
|
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.
Instead of expensive scripted tests, let's define it as quick unit test in nscplugin, for example in this test
scala-native/nscplugin/src/test/scala-3/scala/scalanative/NIRCompilerTest3.scala
Lines 12 to 13 in 8df8dce
class NIRCompilerTest3 { | |
inline def nativeCompilation(source: String): Unit = { |
We only need to ensure that is compiles, we don't require linking
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.
Thanks! done in 1a3dbcc
(I added clib.cp to Test/ nscplugin / javaOptions
since I couldn't find a way to reproduce the compilation crash without using clib.stdio
🤔
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.
I think you might have problems with that, becouse there are 2 definitions of printf. One is defined as extension here:
implicit class StdioHelpers(val _stdio: libc.stdio.type) extends AnyVal { | |
def printf(format: CString, args: CVarArg*): CInt = | |
Zone { implicit z => stdio.vprintf(format, toCVarArgList(args.toSeq)) } | |
The second one is standard extern binding
scala-native/clib/src/main/scala/scala/scalanative/libc/stdio.scala
Lines 587 to 589 in ba2bd19
@blocking def fprintf(stream: Ptr[FILE], format: CString, vargs: Any*): CInt = | |
extern | |
I belive it could have been coused by the clash of the two. It would be great if we could get rid of the dependency. I'd like to remove legacy version in the future.
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.
Thanks @WojciechMazur!
I'm not sure the duplication of printf
is the problem, I could reproduce it without clib
74fbddd
It seems like we hit the ensureUnboxed
-> isPrimitiveType
when the argument is for extern method's varargs (e.g. printf
's vargs
)
scala-native/nscplugin/src/main/scala-3/scala/scalanative/nscplugin/NirGenExpr.scala
Lines 1722 to 1723 in 74fbddd
if (isVarArg) ensureUnboxed(rawValue, paramTpe.finalResultType) | |
else rawValue |
With something like def test(t: Any*) = extern
, and if I pass the opaque typed value to the test
method, we could break the compiler 👍
Adding clib classpath to nscplugin in test scope I couldn't find a way to reproduce the compiler crash without using clib
We could reproduce the issue of compiler crash on opaque-types without clib
fix #3700
It is caused because in
isPrimitiveValueType
against opaque type,t.widenDealias.typeSymbol
isn'tClassSymbol
but aTypeSymbol
forcmark_event_type
. This commit adds an additional check that is the type symbolClassSymbol
before casting.