Skip to content

Actively remove Scala 2 pickles and emit synthetic TASTy attribute for copied stdlib .class files #24846

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.

Sign up for GitHub

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

Merged
WojciechMazur merged 10 commits into from
Jan 7, 2026
Merged

Actively remove Scala 2 pickles and emit synthetic TASTy attribute for copied stdlib .class files #24846

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
6499b0d
fix bytecode attributes for patched files
hamzaremmal Oct 14, 2025
57dac79
Add validation to ensure Scala 2 pickles are not stored
WojciechMazur Dec 28, 2025
a7e214e
Rename unpickler to removeScala2Pickles
WojciechMazur Dec 28, 2025
ac420c3
Add validation for existance of TASTy attribute. Add synthetic TASTY …
WojciechMazur Dec 29, 2025
d673395
Fix sjs builds - use fixed scala-library-nonboostrapped source directory
WojciechMazur Dec 29, 2025
c2d9e9e
Remove dependency on scala-library-nonbootstraped project - detect .c…
WojciechMazur Dec 30, 2025
e831daa
Ensure that Scala 2 pickle attributes are also removed
WojciechMazur Dec 30, 2025
596a5fe
Remove `TastyAttributeReader` replace with inline reader using protot…
WojciechMazur Dec 30, 2025
9178964
Add assertion to ensure that no new TASTy attributes are generated or…
WojciechMazur Dec 30, 2025
47e2dd3
Validate existance of 'Scala' attribute in all .scala outputputs. Pat…
WojciechMazur Jan 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix sjs builds - use fixed scala-library-nonboostrapped source directory
  • Loading branch information
@WojciechMazur
WojciechMazur committed Dec 29, 2025
commit d67339594ff4ba0551c69f747c286a4288f1e361
4 changes: 2 additions & 2 deletions project/ScalaLibraryPlugin.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ object ScalaLibraryPlugin extends AutoPlugin {
var stamps = analysis.stamps

val classDir = (Compile / classDirectory).value
val sourceDir = sourceDirectory.value
val sourceDir = (LocalProject("scala-library-nonbootstrapped") / sourceDirectory).value
Copy link
Member

@hamzaremmal hamzaremmal Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does the plugin know about scala-library-nonbootstrapped? It shouldn't.
And even if it were to know about it, why doesn't it know about scala-library-bootstrapped too?

Copy link
Contributor Author

@WojciechMazur WojciechMazur Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've rewritten this part so that sourceDir is not required at all. We now identify .java sources based on the Classfile Source attribute


// Patch the files that are in the list
for {
Expand Down Expand Up @@ -194,7 +194,7 @@ object ScalaLibraryPlugin extends AutoPlugin {
if (javaSourceFile.exists()) None
else {
val tastyFile = classDirectory / (basePath + ".tasty")
assert(tastyFile.exists(), s"TASTY file $tastyFile does not exist for $relativePath / $javaSourceFile")
assert(tastyFile.exists(), s"TASTY file $tastyFile does not exist for $relativePath")

// Only add TASTY attribute if this is the primary class (class path equals base path)
// Inner classes, companion objects ($), anonymous classes ($$anon), etc. don't get TASTY attribute
Copy link
Member

@hamzaremmal hamzaremmal Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember this honestly but it doesn't seem like a little detail. This is part of the specification of what attributes generated classfiles contain. Where is this specification written?

Copy link
Contributor Author

@WojciechMazur WojciechMazur Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea, it's based on the observations and outputs of compilation. Partially it's also based on the CodeGen logic:

scala3/compiler/src/dotty/tools/backend/jvm/CodeGen.scala

Lines 61 to 71 in d673395

val mainClassNode = genClass(cd, unit)
val mirrorClassNode =
if !sym.isTopLevelModuleClass then null
else if sym.companionClass == NoSymbol then genMirrorClass(sym, unit)
else
report.log(s"No mirror class for module with linked class: ${sym.fullName}", NoSourcePosition)
null
if sym.isClass then
val tastyAttrNode = if (mirrorClassNode ne null) mirrorClassNode else mainClassNode
genTastyAndSetAttributes(sym, tastyAttrNode)

Copy link
Member

@hamzaremmal hamzaremmal Dec 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need the answer to this question before blindly committing to a design. That's why #24180 is delayed, I still have more questions than answers. See this too: #24846 (comment)

Copy link
Contributor Author

@WojciechMazur WojciechMazur Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To summarize behaviour based on the compiler implementation, since there is no dedicated specification:

  1. Only one class per top-level definition gets the TASTy attribute - either the main class or the mirror class, never both
  2. TASTy pickling only iterates over topLevelClasses (defined in TreeInfo.scala), not nested classes
  3. Nested/inner classes are stored inside their parent's .tasty file, not in separate files
  4. The TASTy attribute contains only a 16-byte UUID pointing to the .tasty file - if there's no separate .tasty file, there's nothing to point to

Verification from ClassfileParser.scala:

  • If a .class file has a TASTy attribute, the compiler expects a corresponding .tasty file
  • If the UUID doesn't match, it warns about sync issues

Based on that current behavior is consistent with the design that one .tasty file corresponds to one top-level class and contains all nested definitions within it.

TASTy for companion module is store in companion class. Even if we'd define just an object foo a mirror ``foo.classwould be created and it would contain TASTy attribute, not thefoo$.class`

There is no TASTy attribute for anonynous classes e.g. foo$anon$1.class.
Since specialized class is basically a new anonymous class it should not define TASTY attribute either.

If in the future we'd decide that TASTy should be also stored for synthetics then it can be easily added in the future. Right now TASTy attrs have been added only to the .class files that should have produced TASTy right now. We can add additional validation to ensure that's the case

Copy link
Contributor Author

@WojciechMazur WojciechMazur Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional assertions were added in 9178964

Copy link
Member

@sjrd sjrd Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@WojciechMazur's analysis is correct.

In addition: all .class files produced by a Scala compiler (2 or 3, primary class or not) must have the Scala attribute. You might want to add that to the checks you perform. However, you shouldn't need to add/remove it since they will be present from the Scala 2 compilation.

Copy link
Contributor Author

@WojciechMazur WojciechMazur Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good catch and it seems the Scala 2 compiler might have a bug - there are 15 .class files that miss Scala attribute:

[error] (scala-library-nonbootstrapped / Compile / packageBin) java.lang.AssertionError: assertion failed: JAR scala-library-3.8.1-RC1-bin-SNAPSHOT-nonbootstrapped.jar contains 15 class files without 'Scala' attribute: 
[error]   - scala/Tuple1.class
[error]   - scala/Tuple2.class
[error]   - scala/collection/DoubleStepper.class
[error]   - scala/collection/IntStepper.class
[error]   - scala/collection/LongStepper.class
[error]   - scala/collection/Stepper.class
[error]   - scala/collection/immutable/DoubleVectorStepper.class
[error]   - scala/collection/immutable/IntVectorStepper.class
[error]   - scala/collection/immutable/LongVectorStepper.class
[error]   - scala/collection/immutable/Range.class
[error]   - scala/jdk/DoubleAccumulator.class
[error]   - scala/jdk/IntAccumulator.class
[error]   - scala/jdk/LongAccumulator.class
[error]   - scala/runtime/NonLocalReturnControl.class
[error]   - scala/util/Sorting.class

These seems to match the list of specialized classes we copy. It appears that we have following attributes:

  • in Tuple1.class : ScalaInlineInfo, ScalaSig (no Scala)
  • in Tuple1$.class: ScalaInlineInfo, Scala
  • in Tuple2$mcCC$sp.class: ScalaInlineInfo, Scala

Unless ScalaSig was a special case for Scala attribute it seems to not match the spec.
Since Scala attribute is empty we can easily add to the copied files

Expand Down
Loading