Don't export members that will be synthesized in case classes #13234

odersky · 2021-08-02T10:14:35Z

Fixes #13228

Fixes scala#13228

compiler/src/dotty/tools/dotc/core/Definitions.scala

compiler/src/dotty/tools/dotc/transform/SyntheticMembers.scala

odersky · 2021-08-05T21:10:46Z

idempotency of tasty bootstrap still fails. @bishabosha or @nicolasstucki can you figure out what's wrong?

nicolasstucki · 2021-08-09T16:05:07Z

The methods are synthesized in different orders when executing

sbt> scala3-bootstrapped/testOnly dotty.tools.dotc.CompilationTests -- *tastyBootstrap

For example the tasty of dotty.tools.repl.Quit.tasty has the following diff

<    246:         DEFDEF(12) 70 [productPrefix]
...
<    260:         DEFDEF(14) 73 [hashCode]
...
<    276:         DEFDEF(49) 75 [productElement]
...
<    327:         DEFDEF(10) 81 [toString]
...
<    339:         DEFDEF(48) 83 [productElementName]
...
<    389:         DEFDEF(8) 84 [productArity]
...
---
>    246:         DEFDEF(14) 70 [hashCode]
...
>    262:         DEFDEF(49) 72 [productElement]
...
>    313:         DEFDEF(52) 81 [productElementName]
...
>    367:         DEFDEF(8) 83 [productPrefix]
...
>    377:         DEFDEF(8) 84 [productArity]
...
>    387:         DEFDEF(10) 79 [toString]
...

Full diff of dotty.tools.repl.Quit.tasty

<   70: productPrefix
<   71: String
<   72: Predef
<   73: hashCode
<   74: Int
<   75: productElement
<   76: n
<   77: IndexOutOfBoundsException
<   78: java[Qualified . lang][Qualified . IndexOutOfBoundsException]
<   79: java[Qualified . lang][Qualified . String]
<   80: <init>[Signed Signature(List(java.lang.String),java.lang.IndexOutOfBoundsException) @<init>]
<   81: toString
<   82: toString[Signed Signature(List(),java.lang.String) @toString]
<   83: productElementName
---
>   70: hashCode
>   71: Int
>   72: productElement
>   73: n
>   74: IndexOutOfBoundsException
>   75: java[Qualified . lang][Qualified . IndexOutOfBoundsException]
>   76: String
>   77: java[Qualified . lang][Qualified . String]
>   78: <init>[Signed Signature(List(java.lang.String),java.lang.IndexOutOfBoundsException) @<init>]
>   79: toString
>   80: toString[Signed Signature(List(),java.lang.String) @toString]
>   81: productElementName
>   82: Predef
>   83: productPrefix
232,298c232,298
<    246:         DEFDEF(12) 70 [productPrefix]
<    249:           TYPEREF 71 [String]
<    251:             TERMREF 72 [Predef]
<    253:               SHAREDtype 169
<    256:           STRINGconst 29 [Quit]
<    258:           OVERRIDE
<    259:           SYNTHETIC
<    260:         DEFDEF(14) 73 [hashCode]
<    263:           EMPTYCLAUSE
<    264:           TYPEREF 74 [Int]
<    266:             SHAREDtype 169
<    269:           INTconst 2528879
<    274:           OVERRIDE
<    275:           SYNTHETIC
<    276:         DEFDEF(49) 75 [productElement]
<    279:           PARAM(4) 76 [n]
<    282:             SHAREDtype 264
<    285:           SHAREDtype 205
<    288:           MATCH(35)
<    290:             TERMREFdirect 279
<    293:             CASEDEF(30)
<    295:               IDENT 13 [_]
<    297:                 SHAREDtype 264
<    300:               THROW
<    301:                 APPLY(22)
<    303:                   SELECTin(9) 80 [<init>[Signed Signature(List(java.lang.String),java.lang.IndexOutOfBoundsException) @<init>]]
<    306:                     NEW
<    307:                       TYPEREF 77 [IndexOutOfBoundsException]
<    309:                         SHAREDtype 126
<    311:                     SHAREDtype 307
<    314:                   APPLY(9)
<    316:                     SELECTin(7) 82 [toString[Signed Signature(List(),java.lang.String) @toString]]
<    319:                       SHAREDterm 290
<    322:                       SHAREDtype 205
<    325:           OVERRIDE
<    326:           SYNTHETIC
<    327:         DEFDEF(10) 81 [toString]
<    330:           EMPTYCLAUSE
<    331:           TYPEREF 71 [String]
<    333:             SHAREDtype 126
<    335:           STRINGconst 29 [Quit]
<    337:           OVERRIDE
<    338:           SYNTHETIC
<    339:         DEFDEF(48) 83 [productElementName]
<    342:           PARAM(4) 76 [n]
<    345:             SHAREDtype 264
<    348:           SHAREDtype 249
<    351:           MATCH(34)
<    353:             TERMREFdirect 342
<    356:             CASEDEF(29)
<    358:               IDENT 13 [_]
<    360:                 SHAREDtype 264
<    363:               THROW
<    364:                 APPLY(21)
<    366:                   SELECTin(8) 80 [<init>[Signed Signature(List(java.lang.String),java.lang.IndexOutOfBoundsException) @<init>]]
<    369:                     NEW
<    370:                       SHAREDtype 307
<    373:                     SHAREDtype 307
<    376:                   APPLY(9)
<    378:                     SELECTin(7) 82 [toString[Signed Signature(List(),java.lang.String) @toString]]
<    381:                       SHAREDterm 353
<    384:                       SHAREDtype 205
<    387:           OVERRIDE
<    388:           SYNTHETIC
<    389:         DEFDEF(8) 84 [productArity]
<    392:           SHAREDtype 264
<    395:           INTconst 0
---
>    246:         DEFDEF(14) 70 [hashCode]
>    249:           EMPTYCLAUSE
>    250:           TYPEREF 71 [Int]
>    252:             SHAREDtype 169
>    255:           INTconst 2528879
>    260:           OVERRIDE
>    261:           SYNTHETIC
>    262:         DEFDEF(49) 72 [productElement]
>    265:           PARAM(4) 73 [n]
>    268:             SHAREDtype 250
>    271:           SHAREDtype 205
>    274:           MATCH(35)
>    276:             TERMREFdirect 265
>    279:             CASEDEF(30)
>    281:               IDENT 13 [_]
>    283:                 SHAREDtype 250
>    286:               THROW
>    287:                 APPLY(22)
>    289:                   SELECTin(9) 78 [<init>[Signed Signature(List(java.lang.String),java.lang.IndexOutOfBoundsException) @<init>]]
>    292:                     NEW
>    293:                       TYPEREF 74 [IndexOutOfBoundsException]
>    295:                         SHAREDtype 126
>    297:                     SHAREDtype 293
>    300:                   APPLY(9)
>    302:                     SELECTin(7) 80 [toString[Signed Signature(List(),java.lang.String) @toString]]
>    305:                       SHAREDterm 276
>    308:                       SHAREDtype 205
>    311:           OVERRIDE
>    312:           SYNTHETIC
>    313:         DEFDEF(52) 81 [productElementName]
>    316:           PARAM(4) 73 [n]
>    319:             SHAREDtype 250
>    322:           TYPEREF 76 [String]
>    324:             TERMREF 82 [Predef]
>    326:               SHAREDtype 169
>    329:           MATCH(34)
>    331:             TERMREFdirect 316
>    334:             CASEDEF(29)
>    336:               IDENT 13 [_]
>    338:                 SHAREDtype 250
>    341:               THROW
>    342:                 APPLY(21)
>    344:                   SELECTin(8) 78 [<init>[Signed Signature(List(java.lang.String),java.lang.IndexOutOfBoundsException) @<init>]]
>    347:                     NEW
>    348:                       SHAREDtype 293
>    351:                     SHAREDtype 293
>    354:                   APPLY(9)
>    356:                     SELECTin(7) 80 [toString[Signed Signature(List(),java.lang.String) @toString]]
>    359:                       SHAREDterm 331
>    362:                       SHAREDtype 205
>    365:           OVERRIDE
>    366:           SYNTHETIC
>    367:         DEFDEF(8) 83 [productPrefix]
>    370:           SHAREDtype 322
>    373:           STRINGconst 29 [Quit]
>    375:           OVERRIDE
>    376:           SYNTHETIC
>    377:         DEFDEF(8) 84 [productArity]
>    380:           SHAREDtype 250
>    383:           INTconst 0
>    385:           OVERRIDE
>    386:           SYNTHETIC
>    387:         DEFDEF(10) 79 [toString]
>    390:           EMPTYCLAUSE
>    391:           TYPEREF 76 [String]
>    393:             SHAREDtype 126
>    395:           STRINGconst 29 [Quit]
302,303c302,303
<    402:           IDENTtpt 71 [String]
<    404:             SHAREDtype 249
---
>    402:           IDENTtpt 76 [String]
>    404:             SHAREDtype 322
306,307c306,307
<    412:           IDENTtpt 71 [String]
<    414:             SHAREDtype 249
---
>    412:           IDENTtpt 76 [String]
>    414:             SHAREDtype 322
322c322
<  455 position bytes:
---
>  456 position bytes:
391,395c391,396
<        249: 2393 .. 2393
<        256: 2393 .. 2393
<        260: 2393 .. 2393
<        264: 2393 .. 2393
<        269: 2393 .. 2393
---
>        250: 2393 .. 2393
>        255: 2393 .. 2393
>        262: 2393 .. 2393
>        265: 2393 .. 2393
>        268: 2393 .. 2393
>        271: 2393 .. 2393
397,403c398,403
<        279: 2393 .. 2393
<        282: 2393 .. 2393
<        285: 2393 .. 2393
<        290: 2393 .. 2393
<        295: 2393 .. 2393
<        307: 2393 .. 2393
<        327: 2393 .. 2393
---
>        281: 2393 .. 2393
>        293: 2393 .. 2393
>        313: 2393 .. 2393
>        316: 2393 .. 2393
>        319: 2393 .. 2393
>        322: 2393 .. 2393
405,408c405
<        335: 2393 .. 2393
<        339: 2393 .. 2393
<        342: 2393 .. 2393
<        345: 2393 .. 2393
---
>        336: 2393 .. 2393
410,411c407
<        353: 2393 .. 2393
<        358: 2393 .. 2393
---
>        367: 2393 .. 2393
413,414c409,414
<        389: 2393 .. 2393
<        392: 2393 .. 2393
---
>        373: 2393 .. 2393
>        377: 2393 .. 2393
>        380: 2393 .. 2393
>        383: 2393 .. 2393
>        387: 2393 .. 2393
>        391: 2393 .. 2393
431c431

It is a bit strange that the idempotency tests for tests/pos passed but not these ones. Maybe there is something in the test setup that is different. Or maybe we have a set/map that only loses the order with larger code or more compilation units.

I will keep investigating and will open another PR with some extra code that is useful to debug tasty files.

@odersky maybe have a look at the place where we add those methods to see if there is the potential for nondeterminism.

nicolasstucki · 2021-08-09T16:07:10Z

compiler/src/dotty/tools/dotc/transform/SyntheticMembers.scala

-      myCaseSymbols = myValueSymbols ++ List(defn.Any_toString, defn.Product_canEqual,
-        defn.Product_productArity, defn.Product_productPrefix, defn.Product_productElement,
-        defn.Product_productElementName)
+      myCaseSymbols = defn.caseClassSynthesized.toList


This seems to generate a list without a predefined order.

I may have fixed it. Now it works locally. We should re-run the tests a couple of times to make sure.

My kingdom for a UniqueSeq... Or an OrderedSet.

Usually we fall back to https://www.scala-lang.org/api/current/scala/collection/mutable/LinkedHashSet.html

OrderedSet exists under the name SortedSet. But like LinkedHashSet it is usually less efficient than a plain HashSet.

An alternative would be to define a method toSortedList in sets. That could be very efficient for SortedSets but could still be O(n log n) for other sets.

OrderedSet exists under the name SortedSet. But like LinkedHashSet it is usually less efficient than a plain HashSet.

Somewhere I picked up the notion that "sorted" means "has an associated sorting function", while "ordered" means "someone gave it an order", as in prepend/append/insertion-order. I like it the distinction, but perhaps it's not universally agreed nomenclature.

An alternative would be to define a method toSortedList in sets. That could be very efficient for SortedSets but could still be O(n log n) for other sets.

Looks like this the most recent thread on that: scala/scala-library-next#19

nicolasstucki · 2021-08-10T11:36:59Z

@odersky, now it works but I used a List instead of a Set to retain the order of the methods. Not sure if the .contains on List is good enough for this use case.

odersky · 2021-08-13T13:15:12Z

@nicolasstucki :Facepalm: Yes, contains works fine for lists, it's just less efficient. But for short lists it should not make a difference.

odersky added 2 commits August 2, 2021 12:13

Don't export members that will be synthesized in case classes

5dd0ba5

Fixes scala#13228

Integrate tests in method where they are needed

b7b4746

dwijnand approved these changes Aug 2, 2021

View reviewed changes

bishabosha reviewed Aug 2, 2021

View reviewed changes

compiler/src/dotty/tools/dotc/core/Definitions.scala Outdated Show resolved Hide resolved

Address review comments

ec82153

bishabosha reviewed Aug 2, 2021

View reviewed changes

compiler/src/dotty/tools/dotc/transform/SyntheticMembers.scala Outdated Show resolved Hide resolved

nicolasstucki assigned odersky Aug 3, 2021

odersky added 2 commits August 5, 2021 19:43

Fix typo

c91a99a

Fix tasty extractors expect file

1526271

odersky assigned bishabosha and nicolasstucki and unassigned odersky Aug 5, 2021

Fix checkfile for tasty-extractors-2

c0d0d5d

nicolasstucki assigned odersky Aug 9, 2021

nicolasstucki reviewed Aug 9, 2021

View reviewed changes

nicolasstucki added 2 commits August 9, 2021 18:11

Make caseClassSynthesized idempotent

48aac8e

Fix checkfile for tasty-extractors-2

db38c76

nicolasstucki unassigned nicolasstucki and bishabosha Aug 10, 2021

nicolasstucki mentioned this pull request Aug 10, 2021

Decouple -print-tasty from compiler #13276

Merged

odersky merged commit e4b421c into scala:master Aug 13, 2021

odersky deleted the fix-13228 branch August 13, 2021 13:18

prolativ mentioned this pull request May 25, 2023

Wildcard export from non-case class #17588

Closed

Kordyjan added this to the 3.1.0 milestone Aug 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't export members that will be synthesized in case classes #13234

Don't export members that will be synthesized in case classes #13234

odersky commented Aug 2, 2021

odersky commented Aug 5, 2021

nicolasstucki commented Aug 9, 2021

nicolasstucki Aug 9, 2021

nicolasstucki Aug 9, 2021

dwijnand Aug 9, 2021 •

edited

Loading

smarter Aug 10, 2021

odersky Aug 13, 2021

dwijnand Aug 13, 2021

nicolasstucki commented Aug 10, 2021

odersky commented Aug 13, 2021

Don't export members that will be synthesized in case classes #13234

Don't export members that will be synthesized in case classes #13234

Conversation

odersky commented Aug 2, 2021

odersky commented Aug 5, 2021

nicolasstucki commented Aug 9, 2021

nicolasstucki Aug 9, 2021

Choose a reason for hiding this comment

nicolasstucki Aug 9, 2021

Choose a reason for hiding this comment

dwijnand Aug 9, 2021 • edited Loading

Choose a reason for hiding this comment

smarter Aug 10, 2021

Choose a reason for hiding this comment

odersky Aug 13, 2021

Choose a reason for hiding this comment

dwijnand Aug 13, 2021

Choose a reason for hiding this comment

nicolasstucki commented Aug 10, 2021

odersky commented Aug 13, 2021

dwijnand Aug 9, 2021 •

edited

Loading