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

Singleton types are erased at display. #43

Open
tribbloid opened this issue Jan 31, 2021 · 14 comments
Open

Singleton types are erased at display. #43

tribbloid opened this issue Jan 31, 2021 · 14 comments

Comments

@tribbloid
Copy link
Collaborator

Here is a simple example:


    val a = 1
    val b = 2

    implicitly[a.type =:= b.type]

Generates the error::

[Error] /......:46: implicit error;
!I e (Cannot prove that a.type =:= b.type.): Int =:= Int
one error found

The first section Cannot prove that a.type =:= b.type is a customised implicitNotFound message, which is helpful, the second part Int =:= Int is added by splain, which is clueless.

If the ImplicitNotFound message hasn't been defined, or it is buried deep in a chain implicit resolution tree, there will be no way to figure out the cause.

My proposed fix is to show the full inheritance tree for not just the type, but all its parameters, e.g. for type a.type =:= b.type, the result should look like this:

-+ a.type =:= b.type
 :       `-+ [ 2 ARGS ] :
 :         !-+ a.type .................................................................................................................. [0]
 :         : !-+ Int ..................................................................................................................... [1]
 :         :   !-+ AnyVal
 :         :     !-- Any ..................................................................................................................... [2]
 :         !-+ b.type .................................................................................................................. [3]
 :           !-- Int ..................................................................................................................... [1]
 !-+ a.type <:< b.type
   :       `-+ [ 2 ARGS ] :
   :         !-- a.type .................................................................................................................. [0]
   :         !-- b.type .................................................................................................................. [3]
   !-+ java.io.Serializable
   : !-- Any ..................................................................................................................... [2]
   !-+ a.type => b.type
     :       `-+ [ 2 ARGS ] :
     :         !-- a.type .................................................................................................................. [0]
     :         !-- b.type .................................................................................................................. [3]

I already have the code extract the tree, I just don't know where to inject into your library. Could you point me to the right part of your code?

@tek
Copy link
Owner

tek commented Feb 1, 2021

interesting find!

you can add a SpecialFormatter subclass. its first parameter is a Type that should contain a.type =.= b.type, you can then pattern match and turn it into a Formatted, for which you can also define a new variant so it can be handled later here if needed.

you will need to add the SpecialFormatter object to this list.

do you need directions for setting up a unit test?

@tribbloid
Copy link
Collaborator Author

tribbloid commented Feb 1, 2021

So It should be a 'SingleTypeFormatter' that handles UniqueSingleType. This is the easy part, in fact, the line of code of a can be easily found and displayed. (The same thing applies to path-dependent type)

The difficult part would be to find the code where the 'erasure' from a.type to Int happens: the code is displaying Int =:= Int, technically this is incorrect, as =:= is not covariant. I prefer to also get rid of it

The equally difficult part would be adding the parameter to display inheritance & ARG tree for All kinds of Formatted, not just UniqueSingleType. Should it be considered later for being too complex ?

BTW, this tree can be quite long and complex, but does covers all the edge cases when an implicit search fails. E.g. for a record type in shapeless:

    val book =
      ("author" ->> "Benjamin Pierce") ::
        ("price" ->> 44.11) ::
        HNil

It's tree is:

-+ String with shapeless.labelled.KeyTag[String("author"),String] :: Double with shapeless.labelled.KeyTag[String("price"),Double] :: shapeless.HNil
 :       `-+ [ 2 ARGS ] :
 :         !-+ String with shapeless.labelled.KeyTag[String("author"),String]
 :         : !-+ shapeless.labelled.KeyTag[String("author"),String]
 :         : : :       `-+ [ 2 ARGS ] :
 :         : : :         !-+ String("author")
 :         : : :         : !-- String .................................................................................................................. [0]
 :         : : :         !-- String .................................................................................................................. [0]
 :         : : !-- Object .................................................................................................................. [1]
 :         : !-+ String .................................................................................................................. [0]
 :         :   !-- CharSequence
 :         :   !-- Comparable[String]
 :         :   !-- java.io.Serializable .................................................................................................... [2]
 :         !-+ Double with shapeless.labelled.KeyTag[String("price"),Double] :: shapeless.HNil
 :           :       `-+ [ 2 ARGS ] :
 :           :         !-+ Double with shapeless.labelled.KeyTag[String("price"),Double]
 :           :         : !-+ shapeless.labelled.KeyTag[String("price"),Double]
 :           :         : : :       `-+ [ 2 ARGS ] :
 :           :         : : :         !-+ String("price")
 :           :         : : :         : !-- String .................................................................................................................. [0]
 :           :         : : :         !-- Double .................................................................................................................. [3]
 :           :         : : !-- Object .................................................................................................................. [1]
 :           :         : !-+ Double .................................................................................................................. [3]
 :           :         :   !-- AnyVal
 :           :         !-+ shapeless.HNil
 :           :           !-- shapeless.HList ......................................................................................................... [4]
 :           !-- shapeless.HList ......................................................................................................... [4]
 !-+ shapeless.HList ......................................................................................................... [4]
   !-+ java.io.Serializable .................................................................................................... [2]
   : !-- Any
   !-+ Product
   : !-- Equals
   !-- Object .................................................................................................................. [1]

@tek
Copy link
Owner

tek commented Feb 1, 2021

The difficult part would be to find the code where the 'erasure' from a.type to Int happens: the code is displaying Int =:= Int, technically this is incorrect, as =:= is not covariant. I prefer to also get rid of it

Not quite sure I know what the proper behaviour here would be – is Int =:= Int only an artifact of splain or is it also present in the regular error message?

The equally difficult part would be adding the parameter to display inheritance & ARG tree for All kinds of Formatted, not just UniqueSingleType. Should it be considered later for being too complex ?

What do you mean by "adding the parameter"? Do you want to have an option that toggles displaying the trees for all types?
Would you then still want to treat singleton types separately?

@tribbloid
Copy link
Collaborator Author

@tek I'm quite sure Int =:= Int is only an artifact of splain. If I disable it in compiler plugin the only message being displayed is Cannot prove that a.type =:= b.type, displaying a.type =:= b.type should be the correct way

What do you mean by "adding the parameter"? Do you want to have an option that toggles displaying the trees for all types?
Would you then still want to treat singleton types separately?

Yes That's my intention, otherwise the fix for the singleton types will make the Int type invisible in the error message. The added configuration option to display the entire tree will make it obvious that a.type <:< Int

@tek
Copy link
Owner

tek commented Feb 1, 2021

Wouldn't the tree algorithm work entirely on the root type? You could just check the option in formatSpecial and return a custom Formatted with the tree if it was enabled.

@tribbloid
Copy link
Collaborator Author

@tek sounds good, already implemented. How about my first question? Namely, how to avoid a.type being erased into Int?

Also I'm trying to run the test in my IDE (IntelliJ IDEA, not through sbt), I always got the following error:

bad option: -P:splain:color:false
bad option: -P:splain:bounds
bad option: -P:splain:tree:false

Any idea what could be the cause?

@tek
Copy link
Owner

tek commented Feb 1, 2021

regarding the erasure, I would start by debug printing the type in a function like symbolPath in Formatting and then going up or down the call stack depending on what it shows to find the rule that chooses a supertype. There are a lot of gaps in splain's coverage of the type system 🙂

as for these errors: the tests use a reflect.ToolBox to load the plugin jar built by the main project and passes those options to it, then compiles the code at runtime. I would assume that IntelliJ somehow interfers with something in there, like via the classloader…but I have no idea how, never used IntelliJ.

@tribbloid
Copy link
Collaborator Author

Thanks a lot, made it temporarily working in IDEA by the following monkey patch:

  def base = Option(System.getProperty("splain.tests"))
      .getOrElse(userDir + "/" + "tests")

  val plugin = Option(System.getProperty("splain.jar")).getOrElse {
    userDir + "/target/scala-2.13/splain_2.13.4-0.5.9-SNAPSHOT.jar"
  }

Need to synchronise version with build file or discover the jar name automatically tho

@tek
Copy link
Owner

tek commented Feb 1, 2021

oh, huh. I would assume that the cause of this could be that the tests are executed in a different sbt scope by IntelliJ. If you can find out how it works please advise!

tribbloid pushed a commit to tribbloid/splain that referenced this issue Feb 2, 2021
add 4 test cases for singleton & witness value types

base and plugin now defaults to default tests and bytecode directories, allowing tests to be run in IDE

add special handling for SingletonType (for displaying term.type) & RefinedType (for displaying Witness value type)
@tribbloid
Copy link
Collaborator Author

OK fixed without tree-visualizing capability

#46

I saw a lot of idiosyncratic black magic for type symbol manipulation. I'm afraid some of them needs a more generalisable overhaul. Some of them are quite far from scala convention. The simpliest & safest way to avoid erasing information is to display 2 Strings: 1 from scala team (tpe.toString directly) & 1 from you

@tribbloid
Copy link
Collaborator Author

I have to add some type annotation to avoid the 2.13 compiler from inferring singleton type automatically

Regardless, the only significant change I made is to use type.toString and type.prefixString directly, instead of type.typeSymbol and type.termSymbol. These implementations are incrementally & carelessly patched by a series of PhD students, I don't have a lot of faith in them

@tek
Copy link
Owner

tek commented Feb 2, 2021

yes, I made it up as I went along 🙂
I see you found a location where the singleton type was truncated, that's a little more obvious!

since the tests succeed, do you want it merged now or do you want to supply part 2 first?

@tribbloid
Copy link
Collaborator Author

If you think all the changes make sense then let's merge it, always easier to have many decoupled commits instead of a big one.

I'll need some time to figure out how to embed a tree with interlinked references (those ............ [1] notations) into a type display

@tek
Copy link
Owner

tek commented Feb 2, 2021

sounds good!

tek pushed a commit that referenced this issue Feb 2, 2021
add 4 test cases for singleton & witness value types

base and plugin now defaults to default tests and bytecode directories, allowing tests to be run in IDE

add special handling for SingletonType (for displaying term.type) & RefinedType (for displaying Witness value type)
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

No branches or pull requests

2 participants