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.

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

Jump to bottom

Various improvements to Tasty Reader [ci: last-only] #9271

Merged
merged 14 commits into from Oct 29, 2020
Merged

Various improvements to Tasty Reader [ci: last-only] #9271

merged 14 commits into from Oct 29, 2020

Conversation

bishabosha
Copy link
Member

this adds a number of improvements to tasty reader suggested in scala/bug#12197

  • block opaque types
  • block context function types
  • block refinements to scala.PolyFunction
  • block methods with @scala.annotation.static
  • block definitions with @scala.annotation.alpha annotation
  • block generic tuple types
  • block large function types
  • handle java defined annotations
  • block trait parameters for top level traits in a package
  • consume values of types that extend traits with parameters

@scala-jenkins scala-jenkins added this to the 2.13.5 milestone Oct 23, 2020
bishabosha
bishabosha commented Oct 23, 2020
View reviewed changes
src/compiler/scala/tools/nsc/tasty/bridge/ContextOps.scala
val parameters = traitParams.map(_.nameString)
s"parameterized trait ${parameters.mkString(s"${cls.nameString}(", ", ", ")")}"
}
cls.updateAttachment(new u.DottyParameterisedTrait(traitParams.toList))
Copy link
Member Author

Choose a reason for hiding this comment

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

perhaps theres a more efficient representation we can use for this

Copy link
Member Author

Choose a reason for hiding this comment

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

or I guess more lightweight would be that its not necessary to record the actual parameters to present a correct error

@bishabosha bishabosha changed the title Various improvements to Tasty Reader Various improvements to Tasty Reader [ci: last-only] Oct 23, 2020
@SethTisue SethTisue modified the milestones: 2.13.5, 2.13.4 Oct 23, 2020
@bishabosha
Copy link
Member Author

I've rebased and added a few new things in preparation for supporting Scala 3.0.0-M1 release

dwijnand
dwijnand approved these changes Oct 26, 2020
View reviewed changes
Copy link
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

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

The changes outside of the DMZ of tasty look fine, so I'm happy for this to land. CI failed for the rec depth issue that I have a PR for (I restarted the job).

@bishabosha
Copy link
Member Author

bishabosha commented Oct 27, 2020 •
edited

I have added another commit for supporting 3.0.0-M1, which should be frozen tastywise now
edit as of 8706224 this commit was lost in the history, will need to add it in again
second edit: it has been added back

@bishabosha
Copy link
Member Author

error in CI waiting for #9280

@bishabosha bishabosha mentioned this pull request Oct 28, 2020
Closed
@bishabosha
Copy link
Member Author

bishabosha commented Oct 28, 2020 •
edited

I've opened #9286, unless i should rebase and add the changes to this PR
Edit - closed the PR

@bishabosha
handle polyfunction and contextfunction
c2e2423
@bishabosha
Block opaque type aliases
adc9940 
Following a discussion, it makes sense to prevent opaque type
aliases. When the alias is visible in Scala 2 then the
intended API of the opaque type is broken.

It is possible to re-enable opaque types as long as they remain
opaque until erasure, but they must be erased to their underlying
type - so an efficient and correct representation should be found.
@bishabosha
block large function types
308ef39
@bishabosha
Block generic tuple cons type.
3f4ad1f 
This also escapes Predef.classOf when creating a TypeApply
@bishabosha
Block alpha annotation by addition of compileTimeOnly
41d723c
@bishabosha
Block static annotation by addition of compileTimeOnly
0f850a9
@bishabosha
only forbid class mixin of traits with params
1cf69a7
@bishabosha
enable java annotations - escape overload resolution
a3bf01e
@bishabosha
add local syms for field accessors, enable tasty tests
8706224
@bishabosha
Copy link
Member Author

I've now also added the changes to create local field symbols - which means pattern matching on dotty case classes works again

dwijnand
dwijnand approved these changes Oct 28, 2020
View reviewed changes
Copy link
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

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

LGTM again, thank you Jamie!

@dwijnand dwijnand mentioned this pull request Oct 28, 2020
Closed
72 tasks
@SethTisue SethTisue added the prio:blocker release blocker (used only by core team, only near release time) label Oct 28, 2020
@SethTisue
Copy link
Member

@lrytz you want to give this a once-over, or shall we just merge it?

@bishabosha
Copy link
Member Author

bishabosha commented Oct 28, 2020 •
edited

I realised I accidentally overwrote the commits that made this work with 3.0.0 - so either this one should wait or I will open a new PR to add the changes again (I need to wait for tomorrow to access my other work laptop)

Edit - second option is probably most simple and then I can add the settings flag to toggle the tasty reader also

Second edit - those commits have been added back to the end of this PR now the CI for 8706224 passed

@lrytz
Copy link
Member

lrytz commented Oct 28, 2020

I'll take a quick look as well.

dwijnand
dwijnand approved these changes Oct 29, 2020
View reviewed changes
Copy link
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

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

Lovely, thanks for the speedy turnaround.

@bishabosha
Copy link
Member Author

I've added back the commits that bring it to date with Scala 3.0.0-M1 nightly

lrytz
lrytz approved these changes Oct 29, 2020
View reviewed changes
src/compiler/scala/tools/nsc/typechecker/Typers.scala
@@ -1793,6 +1793,16 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
val ps = psym.info.parents
if (!ps.isEmpty && !superclazz.isSubClass(ps.head.typeSymbol))
pending += ParentSuperSubclassError(parent, superclazz, ps.head.typeSymbol, psym)
if (!clazzIsTrait) {
// TODO perhaps there can be a flag to skip this when we know there can be no Scala 3 definitions
// or otherwise use an optimised representation for trait parameters
Copy link
Member

Choose a reason for hiding this comment

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

Use -Ytasty-reader #9293?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes that would work nicely

@lrytz lrytz merged commit 200c122 into scala:2.13.x Oct 29, 2020
@lrytz lrytz mentioned this pull request Oct 29, 2020
Closed
16 tasks
@sjrd sjrd deleted the tasty/read-java-annotations branch October 29, 2020 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lrytz lrytz lrytz approved these changes

@dwijnand dwijnand dwijnand approved these changes

Assignees
No one assigned
Labels
prio:blocker release blocker (used only by core team, only near release time)
Projects
None yet
Milestone
2.13.4
5 participants
@bishabosha @SethTisue @lrytz @dwijnand @scala-jenkins