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 missing case for SyntheticBounds #5475

Merged
merged 3 commits into from Nov 23, 2018

Conversation

Projects
None yet
3 participants
@nicolasstucki
Copy link
Contributor

nicolasstucki commented Nov 20, 2018

No description provided.

@@ -98,6 +98,7 @@ trait TreeUtils
case TypeTree.ByName(result) => foldTypeTree(x, result)
case TypeTree.Annotated(arg, annot) => foldTree(foldTypeTree(x, arg), annot)
case TypeBoundsTree(lo, hi) => foldTypeTree(foldTypeTree(x, lo), hi)
case SyntheticBounds() => x

This comment has been minimized.

@odersky

odersky Nov 20, 2018

Contributor

What is SyntheticBounds?

This comment has been minimized.

@nicolasstucki

nicolasstucki Nov 20, 2018

Contributor

Internally it is a TypeTree that has a type bound. We needed two different extractors Synthetic/SyntheticBounds as we differentiate separate abstract types for types and type bounds.

This comment has been minimized.

@odersky

odersky Nov 20, 2018

Contributor

You mean a "type" of the form _ >: L <: U? If yes, should it be called WildcardType then? SyntheticBounds tells me not a lot.

This comment has been minimized.

@nicolasstucki

nicolasstucki Nov 20, 2018

Contributor

Yes, it is for wildcards but aslo for some synthetic bounds.

https://github.com/lampepfl/dotty/blob/master/compiler/src/dotty/tools/dotc/tastyreflect/TypeOrBoundsTreesOpsImpl.scala#L162-L168

After some exploration I discovered that the second case only apears with enums. It looks like we generate some type parameter bounds as a TypeTree wheras all other cases are in a TypeBoundsTree.

I will rename it to WildcardType and move the other case to TypeBoundsTree.

@liufengyun

This comment has been minimized.

Copy link
Contributor

liufengyun commented Nov 21, 2018

@nicolasstucki The CI is not happy.

@nicolasstucki nicolasstucki force-pushed the dotty-staging:add-missing-tasty-tree-fold-cases branch from 7708510 to c4b52a0 Nov 21, 2018

@nicolasstucki

This comment has been minimized.

Copy link
Contributor

nicolasstucki commented Nov 21, 2018

@liufengyun the CI is happy again.

@nicolasstucki nicolasstucki force-pushed the dotty-staging:add-missing-tasty-tree-fold-cases branch 3 times, most recently from 906796e to 15ab19c Nov 21, 2018

@nicolasstucki nicolasstucki force-pushed the dotty-staging:add-missing-tasty-tree-fold-cases branch from 15ab19c to 12e0e3e Nov 23, 2018

@nicolasstucki

This comment has been minimized.

Copy link
Contributor

nicolasstucki commented Nov 23, 2018

Rebased

@liufengyun
Copy link
Contributor

liufengyun left a comment

LGTM

@@ -311,6 +311,9 @@ trait Core {
/** Type tree representing a type bound written in the source */
type TypeBoundsTree <: TypeOrBoundsTree

/** Type tree representing wildcard type bounds written in the source */
type WildcardType <: TypeOrBoundsTree

This comment has been minimized.

@liufengyun

liufengyun Nov 23, 2018

Contributor

I think the document is not precise. Given the following code:

val a: List[_] = List(1, "String")

The underscore become TypeBoundsTree(TypeTree( | scala.this.Nothing), TypeTree( | scala.this.Any) | )

This comment has been minimized.

@liufengyun

liufengyun Nov 23, 2018

Contributor

I think we can give the hypothesis in #5483 a second thought.

Even if we want to report errors for type trees:

  • it's completely acceptable for end users to see the error report on the whole type tree
  • they work for type alias and infered types as well
  • In practice, I conjecture that macro authors will check types to detect errors, and then just report errors on the whole type tree instead of inspecting the components of type trees & deal with inferred and aliased types.
fooMacro[List[Int]]
              ^^^

@nicolasstucki nicolasstucki merged commit d7e06ef into lampepfl:master Nov 23, 2018

2 checks passed

CLA User signed CLA
Details
continuous-integration/drone/pr the build was successful
Details

@nicolasstucki nicolasstucki deleted the dotty-staging:add-missing-tasty-tree-fold-cases branch Nov 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment