Skip to content

Add missing case for SyntheticBounds #5475

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

Merged

Conversation

nicolasstucki
Copy link
Contributor

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
Copy link
Contributor

Choose a reason for hiding this comment

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

What is SyntheticBounds?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@odersky odersky Nov 20, 2018

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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
Copy link
Contributor

@nicolasstucki The CI is not happy.

@nicolasstucki nicolasstucki force-pushed the add-missing-tasty-tree-fold-cases branch from 7708510 to c4b52a0 Compare November 21, 2018 09:08
@nicolasstucki
Copy link
Contributor Author

@liufengyun the CI is happy again.

@nicolasstucki nicolasstucki force-pushed the add-missing-tasty-tree-fold-cases branch 3 times, most recently from 906796e to 15ab19c Compare November 22, 2018 06:40
@nicolasstucki nicolasstucki force-pushed the add-missing-tasty-tree-fold-cases branch from 15ab19c to 12e0e3e Compare November 23, 2018 07:00
@nicolasstucki
Copy link
Contributor Author

Rebased

Copy link
Contributor

@liufengyun liufengyun left a comment

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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) | )

Copy link
Contributor

@liufengyun liufengyun Nov 23, 2018

Choose a reason for hiding this comment

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

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 scala:master Nov 23, 2018
@nicolasstucki nicolasstucki deleted the add-missing-tasty-tree-fold-cases branch November 23, 2018 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants