-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add compilation unit info to ClassSymbol
#19010
Add compilation unit info to ClassSymbol
#19010
Conversation
eff9c27
to
2a0282f
Compare
2a0282f
to
ada4552
Compare
6722aaa
to
b00bb30
Compare
else compUnitInfo.tastyVersion | ||
|
||
/** If this class has explicit nulls enabled */ | ||
def explicitNulls(using Context): Boolean = |
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.
@noti0na1 here is what you need to know if a symbol (from TASTy or source) was compiled using explicit nulls.
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!
lastDenot.topLevelClass.compilationUnitInfo | ||
|
||
/** The version of TASTy from which the symbol was loaded, None if not applicable. */ | ||
def tastyVersion(using Context): Option[TastyVersion] = |
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.
@EugeneFlesselle here is the new version of the TASTy version. I changed a bit your original design, now we only have a TASTy version if the symbol vas loaded from TASTy. For symbols created from source we can assume they have the current TASTy version.
@bishabosha I sugest looking at the individual commits. |
Use it to store the `associatedFile` allow for later expansion.
This is the first part of scala#18870
Also avoid reading header twice to check the UUID.
b00bb30
to
2f36da4
Compare
All commits passed the CI. I had to rebase due to a conflict with a new test that used infix notation with non-infix operations. |
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.
Only other comment would be - can the "compilation unit info" be cached per source file? (would require source file tasty attribute), then the symbols from the classpath may share a "compilation unit info", but still have independent associatedFile
Edit: this is probably not useful because then it makes it impossible to read older tasty
We only set it on top-level classes. The cache might not be that useful.
That will be added in #18948. I needed to add the CompitionUnitInfo first to be able to use it properly. |
- Test major TASTy version - strip margins of `s".."` - Cache `CompilationUnit` in `SymbolLoaders`
71266bf
to
f360a33
Compare
f360a33
to
9597a61
Compare
I analyzed this a bit and we might end up boxing in some other places. The cached case class might be the best option memory-wise. |
|
||
object TastyVersion { | ||
|
||
private val cache: java.util.concurrent.ConcurrentHashMap[TastyVersion, TastyVersion] = |
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 fails the re-entrant check
private val cache: java.util.concurrent.ConcurrentHashMap[TastyVersion, TastyVersion] = | |
@scala.annotation.internal.sharable | |
private val cache: java.util.concurrent.ConcurrentHashMap[TastyVersion, TastyVersion] = |
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.
solved
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.
actually we can't use this annotation in tasty-core because it needs to compile with Scala 2
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.
perhaps we have to remove the re-entrant check for this project?
or we never actually published a tasty-core for scala 2, it was only to check source compatibility, so maybe we can add the scala 3 class path
Co-authored-by: Nicolas Stucki <[email protected]> Co-authored-by: Jamie Thompson <[email protected]>
9597a61
to
97d8171
Compare
compilationUnitInfo
to class symbolsassocFile
intoCompilationUnitInfo
CompilationUnitInfo
CompilationUnitInfo
CompilationUnitInfo
Fixes #18933