Rewrite classOf and getClass handling, implement support for @throws
#9095
Conversation
There was a problem hiding this comment.
I don't understand what takes care of transforming sideEffect().getClass into { sideEffect(); classOf[Int] } anymore. I see that there's code to handle constant receives, but it seems the code to handle non-constant receivers is gone without replacement, so I'd expect it to now emit BoxesRunTime.boxToInteger(sideEffect()).getClass(), which will return classOf[Integer].
|
I wish we could just get rid of this non-LSP-abiding semantics of |
|
smarter
force-pushed
the
throws-annot-3
branch
3 times, most recently
from
49df6d9 to
74e4029
Compare
|
Ping for review. |
| @@ -251,14 +251,12 @@ class DottyBackendInterface(outputDirectory: AbstractFile, val superCallsMap: Ma | |||
| case StringTag => | |||
| assert(const.value != null, const) // TODO this invariant isn't documented in `case class Constant` | |||
| av.visit(name, const.stringValue) // `stringValue` special-cases null, but that execution path isn't exercised for a const with StringTag | |||
| case ClazzTag => av.visit(name, const.typeValue.toTypeKind(bcodeStore)(innerClasesStore).toASMType) | |||
| case ClazzTag => av.visit(name, TypeErasure.erasure(const.typeValue).toTypeKind(bcodeStore)(innerClasesStore).toASMType) | |||
There was a problem hiding this comment.
Erasure should have erased the typeValues of Constants. It's not good to keep a non-erased type in the Constants after erasure.
There was a problem hiding this comment.
Annotation trees are never transformed so they require some special care like this.
There was a problem hiding this comment.
Nevertheless, scalac doesn't do this in the codegen for Literals:
https://github.com/scala/scala/blob/cb31fee193b1d9031a59d1b9fccb18aa7b4404c0/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala#L456-L460
Maybe we need to handle annotation trees specially, but for regular Literals in the code I believe they should be erased.
There was a problem hiding this comment.
Yes, regular literals are fine, in dotty like in scalac they're handled by genConstant which doesn't call erasure.
compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala
Outdated
Show resolved
Hide resolved
So far, a miniphase took care of rewriting Predef.classOf to a class constant, but this means that `classOf` could survive in annotations (since they're not transformed) and needed to be handled by the backend. To simplify things, we take advantage of the constant folding done in the typer to directly replace `classOf` calls by a constant.
The implementation and tests are based on the scalac implementation, the implementation relies on the classOf rewrite in the previous commit.
Previously, getClass was implemented as a miniphase, but that miniphase completely neglected to preserve potential side-effects in the prefix of the call. Instead of doing this manually in the miniphase, we extend our constant folding logic to recognize getClass calls, this simplifies things since constant folding already knows how to deal with side-effects.
After the previous commit, t5568 started crashing, but it turns out that it had always been wrong: 1.asInstanceOf[Int & AnyRef].getClass is supposed to return classOf[Integer] but actually returns classOf[Int], when this test was imported from scalac, the checkfile was changed to make the test pass instead of aligining the compiler behavior with scalac. This commit fixes that: by dropping the `getClass` overload defined in primitive value classes which confuses the compiler, see `hasProblematicGetClass`.
Any#getClass is a polymorphic nilary method, handle this properly in ConstFold instead of folding just the selection, this could not be done when the constant-folding of getClass was first introduced two commits ago due to the weird overloading of getClass, but that has been take care of by the previous commit.
No description provided.