Stop capturing a Context in KnownBTypes#26174
Conversation
|
|
||
| def close(): Unit = | ||
| protected def runLocalOptimizations(classNode: ClassNode): Unit = | ||
| () // no optimizations by default |
There was a problem hiding this comment.
IIUC, PostProcessorWithOptimizations is only created when global optimizations are enabled. But we should still run local optimizations (if enabled) in the plain PostProcessor, no?
In Scala 2, dce/removeUnreachableCode is enabled by default. Is that also the case in Scala 3?
There was a problem hiding this comment.
None of them are enabled by default, though we remove unused line numbers unconditionally (I'm not sure if that can have any effect in practice if no opts have run).
| } | ||
| } | ||
|
|
||
| final class PostProcessorWithOptimizations(frontendAccess: PostProcessorFrontendAccess, |
There was a problem hiding this comment.
PostProcessorWithGlobalOptimizations?
| if _codeGen eq null then | ||
| _codeGen = new CodeGen(backendUtils, primitives, frontendAccess, callGraph, bTypeLoader, wellKnownBTypes, generatedClassHandler) | ||
| val cg = Option.when(ctx.settings.optInlineEnabled || ctx.settings.optClosureInvocations)(callGraph) | ||
| _codeGen = new CodeGen(primitives, cg, bTypeLoader, knownBTypes, generatedClassHandler) |
There was a problem hiding this comment.
If the optimizer is enabled, should we pass optimizerKnownBTypes here? Otherwise the btypes in the base class KnownBTypes are initialized twice. I guess correctness is OK because ClassBTypes are interened, so we get the same instances in both and reference equality holds.
There was a problem hiding this comment.
I'll try, just need to double-check it won't cause anything to be double-initialized due to cycles either. I really need to refactor the collection of indy lambdas / call graph edges so we don't need all this ceremony to instantiate things...
There was a problem hiding this comment.
Done, I changed knownBTypes to return the optimizer version when enabled
SolalPirelli
force-pushed
the
solal/backendutils
branch
from
a8b1d0b to
fa95a2c
Compare
SolalPirelli
force-pushed
the
solal/backendutils
branch
from
fa95a2c to
5880752
Compare
…ff out of OptimizerUtils because I forgot to do this earlier.
…into AnalysisUtils
SolalPirelli
force-pushed
the
solal/backendutils
branch
from
5880752 to
459548b
Compare
2 participants
Part of #25218
Next step towards cleaning up the backend and eventually enabling parallel classfile emitting / optimization with confidence.
Review commit by commit, they're all fairly straightforward.
This PR:
BackendUtilsintoAnalysisUtils,OptimizerUtils, and the few things not for the optimizer moved somewhere elsePostProcessorFrontendAccess.Lazyas it is no longer neededNext steps:
ContextinScalaPrimitives#26177How much have you relied on LLM-based tools in this contribution?
Not at all
How was the solution tested?
Covered by existing tests (this is a refactoring)