Skip to content
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

'implements' list order not deterministic #20496

Closed
raboof opened this issue May 30, 2024 · 1 comment · Fixed by #20593
Closed

'implements' list order not deterministic #20496

raboof opened this issue May 30, 2024 · 1 comment · Fixed by #20593
Labels
itype:bug stat:needs minimization Needs a self contained minimization
Milestone

Comments

@raboof
Copy link
Contributor

raboof commented May 30, 2024

Compiler version

3.3.3

Minimized code

(not minimized yet)

On version 1.1.0-M1 of Pekko, the order of the imports on org.apache.pekko/cluster.sharding.PersistentShardCoordinator was not deterministic: it ended with Timers, Actor in one build but with Actor, Timers on another.

The definition of this class is:

class PersistentShardCoordinator(
    override val typeName: String,
    settings: ClusterShardingSettings,
    allocationStrategy: ShardCoordinator.ShardAllocationStrategy)
    extends ShardCoordinator(settings, allocationStrategy)
    with PersistentActor

where:

class ShardCoordinator extends Actor with Timers
trait PersistentActor extends Eventsourced with PersistenceIdentity
trait Eventsourced extends Snapshotter with PersistenceStash with PersistenceIdentity with PersistenceRecovery
trait Snapshotter extends Actor
trait PersistenceRecovery
trait PersistenceIdentity
trait PersistenceStash extends Stash with StashFactory
trait Stash extends UnrestrictedStash with RequiresMessageQueue[DequeBasedMessageQueueSemantics]
trait UnrestrictedStash extends Actor with StashSupport
trait StashSupport
trait RequiresMessageQueue[T]
trait StashFactory { this: Actor =>
}

I tried to put together a reproducer at https://codeberg.org/raboof/scala3-reproduce-20496/src/branch/main/Test.scala but there ShardCoordinator.class doesn't have Actor and Timer on the implements list directly at all - I'm not sure how exactly that's triggered, happy to take suggestions :)

Expectation

Compiling the same source on a clean system should produce the same bit-by-bit result each time.

@raboof raboof added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels May 30, 2024
@Gedochao Gedochao added stat:needs minimization Needs a self contained minimization stat:needs triage Every issue needs to have an "area" and "itype" label and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Jun 3, 2024
@Gedochao Gedochao removed the stat:needs triage Every issue needs to have an "area" and "itype" label label Jun 11, 2024
@raboof
Copy link
Contributor Author

raboof commented Jun 18, 2024

Looking at the code it seems pretty plausible that this problem was caused by the set logic in Symbol.superInterfaces (in BTypesFromSymbols.scala).

Updated https://codeberg.org/raboof/scala3-reproduce-20496 so it now recreates that scenario, though being a nondeterminism it's still hard to trigger.

Created a tentative fix in #20593, though this is unfamiliar territory for me so I'd appreciate any feedback!

raboof added a commit to raboof/dotty that referenced this issue Jun 18, 2024
When a class contains calls to 'super' for traits it does
not directly implement, these are added to the list of interfaces
of the generated class. Previously, because these interfaces were
determined using set logic, the ordering of that list was not
deterministic.

This makes the order deterministic assuming the order in which
these calls are registered using `registerSuperCall` in the
`CollectSuperCalls` phase is deterministic within each class.
This seems likely to me but it'd be great if someone could
confirm.

To add a test for this change, creating a class that has many
'additional' traits and checking they are generated in the right
order would make sense, but I couldn't find an obvious place for
such a test. Any recommendations?

Fixes scala#20496
raboof added a commit to raboof/dotty that referenced this issue Jun 19, 2024
When a class contains calls to 'super' for traits it does
not directly implement, these are added to the list of interfaces
of the generated class. Previously, because these interfaces were
determined using set logic, the ordering of that list was not
deterministic.

This change makes the order deterministic (assuming the order in
which these calls are registered using `registerSuperCall` in the
`CollectSuperCalls` phase is deterministic within each class)

Fixes scala#20496
@Kordyjan Kordyjan added this to the 3.5.1 milestone Jul 3, 2024
WojciechMazur added a commit that referenced this issue Jul 10, 2024
When a class contains calls to 'super' for traits it does
not directly implement, these are added to the list of interfaces
of the generated class. Previously, because these interfaces were
determined using set logic, the ordering of that list was not
deterministic.

This change makes the order deterministic (assuming the order in
which these calls are registered using `registerSuperCall` in the
`CollectSuperCalls` phase is deterministic within each class)

Fixes #20496

[Cherry-picked 7500b05][modified]
WojciechMazur added a commit that referenced this issue Jul 10, 2024
When a class contains calls to 'super' for traits it does
not directly implement, these are added to the list of interfaces
of the generated class. Previously, because these interfaces were
determined using set logic, the ordering of that list was not
deterministic.

This change makes the order deterministic (assuming the order in
which these calls are registered using `registerSuperCall` in the
`CollectSuperCalls` phase is deterministic within each class)

Fixes #20496

[Cherry-picked 7500b05][modified]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
itype:bug stat:needs minimization Needs a self contained minimization
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants