what is the typer contract?

[Paul Phillips]

See the comments in https://github.com/scala/scala/pull/2134 for the full context.

Lukas said "for starters, the patch fixes an obvious contract violation of the typer: he's not supposed to a) set the "tpe" field of trees that are passed into it, b) return the same tree that's passed into it, and c) return a tree which is partially typed (subtrees of the returned tree have tpe / symbol still null)." He was responding thinking an earlier comment had been on a different commit, so don't try too hard to match the specifics in that comment up to the pull request. (Also, he amended to allow the same tree returned if the tpe field is already set.)

One of the distinctions which is visible between people who have spent years in proximity to martin vs. people who haven't is that the former group can say things like "obvious contract violation of the typer" where the latter group would say "there's a typer contract?" This would not be a problem except that the latter group commits more code than the former group.

Is the typer contract written down somewhere? In the legal arena they say "a verbal contract isn't worth the paper it's written on."

Both a) and b) above are violated with impunity in the compiler, so if that is the contract then it is time to amend the contract or undertake a huge cleanup. I expect c) mostly holds.

Mostly I'd just like to know what the contract is supposed to be. Having code flagged as an obvious violation of an unwritten contract is like being arrested on the basis of a secret law.

[Lukas Rytz]

Let me weaken it up a bit :) The third is most certainly true (in general, some un-typed trees might still be around i guess, for instance `original` trees of TypeTree. but these are more like attachments, not semantically relevant for the program).

The other two might not be problematic and maybe not part of "the contract" if "everything else goes well" - but observe that in general new trees are

created and a type is assigned - is that just a convention or a requirement?

--
You received this message because you are subscribed to the Google Groups "scala-internals" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-interna...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

[Paul Phillips]

On Sat, Feb 23, 2013 at 11:19 AM, Lukas Rytz <lukas...@epfl.ch> wrote:

but observe that in general new trees are

created and a type is assigned - is that just a convention or a requirement?

The way I've understood it in the past, the caller is supposed to always assume a new tree (in other words, use the tree returned by typer, not the one you passed) but the typer might return (and indeed, usually does) return the same tree.

[Lukas Rytz]

"usually" here means "if the tree is already typed", right? in case the input is not typed, typer would "usually" (always?) create a copy first, no?

 

return the same tree.

[Jason Zaugg]

On Sat, Feb 23, 2013 at 9:57 PM, Paul Phillips <pa...@improving.org> wrote:

The way I've understood it in the past, the caller is supposed to always assume a new tree (in other words, use the tree returned by typer, not the one you passed) but the typer might return (and indeed, usually does) return the same tree.

As an experiment, I've modified typed to return shallow duplicated trees [1] and let Jenkins have at it [2].

A sample of the problems:

    % qbin/scalac -Yclone-trees test/files/pos/t6722.scala
    test/files/pos/t6722.scala:10: error: erroneous or inaccessible type
    dyn.foo.bar.baz.get[String]
                       ^
    one error found

    % qbin/scalac -Yclone-trees test/files/pos/t6201.scala
    test/files/pos/t6201.scala:12: error: type mismatch;
     found   : scala.xml.Elem
     required: ?{def must: ?}
    Note that implicit conversions are not applicable because they are ambiguous:
     both method toFoo1 in class Test of type (s: scala.xml.Elem)Test.this.Foo1
     and method toFoo2 in class Test of type (s: scala.xml.Elem)Test.this.Foo2
     are possible conversion functions from scala.xml.Elem to ?{def must: ?}
      def is: Unit = { (<a>{"a"}</a>).must(<a>{"b"}</a>) }
                         ^

Not sure how much of the blame lies with the code and how much with the canary.

-jason

[1] https://github.com/retronym/scala/compare/scala:2.10.x...retronym:topic/tree-clone

[2] https://scala-webapps.epfl.ch/jenkins/job/scala-checkin-manual/829/console

[Jason Zaugg]

On Sat, Feb 23, 2013 at 11:33 PM, Jason Zaugg <jza...@gmail.com> wrote:

As an experiment, I've modified typed to return shallow duplicated trees [1] and let Jenkins have at it [2].

This build only enables the tree duplication during the test run, which gives a broader look at things than the previous build.

https://scala-webapps.epfl.ch/jenkins/job/scala-checkin-manual/830/console

-jason 

[Paul Phillips]

Boy, talk about dizzying.

Implicit search appears to be one of the worst culprits. Lots of vanishing views. This comes as no surprise, recalling https://github.com/scala/scala/commit/58bfa19332 .

[Jason Zaugg]

On Sun, Feb 24, 2013 at 1:04 AM, Paul Phillips <pa...@improving.org> wrote:

Boy, talk about dizzying.

Implicit search appears to be one of the worst culprits. Lots of vanishing views. This comes as no surprise, recalling https://github.com/scala/scala/commit/58bfa19332 .

The failures in the Dynamic tests show one root cause, the use of `Tree#==`.

   def matches(t: Tree) = isDesugaredApply || treeInfo.dissectApplied(t).core == treeSelection

Here, it's comparing `Context#tree` with the tree returned from `typed`.

-jason

[Paul Phillips]

On Sat, Feb 23, 2013 at 2:07 PM, Lukas Rytz <lukas...@epfl.ch> wrote:

"usually" here means "if the tree is already typed", right? in case the input is not typed, typer would "usually" (always?) create a copy first, no?

scala> val t = Ident(ListClass.companionSymbol)

t: $r.intp.global.Ident = List

scala> t.tpe

res0: $r.intp.global.Type = null

scala> val t2 = typer typed t

t2: $r.intp.global.analyzer.global.Tree = List

scala> t2.tpe

res1: $r.intp.global.analyzer.global.Type = scala.collection.immutable.List.type

scala> t eq t2

res2: Boolean = true

[Paul Phillips]

On Sat, Feb 23, 2013 at 4:07 PM, Jason Zaugg <jza...@gmail.com> wrote:

The failures in the Dynamic tests show one root cause, the use of `Tree#==`.

Yeah, I've been logging that to see how things get there. A whole lot of ways, it turns out.

[Jason Zaugg]

Let's collect the details on this ticket:

https://issues.scala-lang.org/browse/SI-7176

-jason 

[Lukas Rytz]

On Sun, Feb 24, 2013 at 1:08 AM, Paul Phillips <pa...@improving.org> wrote:

I see.. Is it also the case if the Ident has no symbol yet? In any case, the example shows that returning the

same tree is and assigning tpe happens and is probably ok..

[Lukas Rytz]

Hey, here's a bug caused by the fact that "typed" does not return a new tree, but instead assigns

the tpe / symbol fields of the "Ident" tree that's passed into it: SI-6921.

What happens: you have something like

   var reason = ""

   val arg = None

   foo(reason = arg)

The implementation of named arguments checks, using a silent type check, if the expression

"reason=arg" is a valid assignment - if it was, we would issue an ambiguity error saying "I don't

know if the thing is an assignment or a named argument".

So we do a silent type check of the assignment expression. The type check fails, which means

we're good, we can continue with the named argument. However, type checking the "Ident(arg)"

assigned ErrorType and ErrorSymbol to the Ident tree. We end up putting that error typed tree

into the resulting tree and things crash later on.

So the question is: what's the correct fix? It seems to me that we should strengthen the "typer

contract" to make sure typing always never modifies the trees that are passed into it. Not doing

so is a notorious source of bugs.

Lukas

[Eugene Burmako]

If the typer stops modifying trees in place, reification won't work, because then the originals of type trees won't have symbols any more, and those are essential to reification. Well, on a second thought, we could probably go out of our way and assign symful originals to typetrees produced by typed...

[Lukas Rytz]

On Wed, Mar 6, 2013 at 9:53 AM, Eugene Burmako <eugene....@epfl.ch> wrote:

If the typer stops modifying trees in place, reification won't work, because then the originals of type trees won't have symbols any more, and those are essential to reification. Well, on a second thought, we could probably go out of our way and assign symful originals to typetrees produced by typed...

indeed that sounds more robust - do we have those symful/typed type trees available at the place where they are replaced by a TypeTree?

[Eugene Burmako]

I'm afk, so I can't check unfortunately

[Jason Zaugg]

On Wed, Mar 6, 2013 at 9:56 AM, Lukas Rytz <lukas...@epfl.ch> wrote:

On Wed, Mar 6, 2013 at 9:53 AM, Eugene Burmako <eugene....@epfl.ch> wrote:

If the typer stops modifying trees in place, reification won't work, because then the originals of type trees won't have symbols any more, and those are essential to reification. Well, on a second thought, we could probably go out of our way and assign symful originals to typetrees produced by typed...

indeed that sounds more robust - do we have those symful/typed type trees available at the place where they are replaced by a TypeTree?

As I found out when I tried this, any code the interrogates Context#tree breaks when typed doesn't operate in place. Examples over on SI-7176. I discussed this quickly with Martin yesterday, he suggested that certain at points during type checking (e.g typedApply) should add more specific information to the Context, so that places like `adaptToMemberWithArgs` wouldn't need to rely on tree equality.

-jason 

[Lukas Rytz]

On Wed, Mar 6, 2013 at 10:09 AM, Jason Zaugg <jza...@gmail.com> wrote:

On Wed, Mar 6, 2013 at 9:56 AM, Lukas Rytz <lukas...@epfl.ch> wrote:

On Wed, Mar 6, 2013 at 9:53 AM, Eugene Burmako <eugene....@epfl.ch> wrote:

If the typer stops modifying trees in place, reification won't work, because then the originals of type trees won't have symbols any more, and those are essential to reification. Well, on a second thought, we could probably go out of our way and assign symful originals to typetrees produced by typed...

indeed that sounds more robust - do we have those symful/typed type trees available at the place where they are replaced by a TypeTree?

As I found out when I tried this, any code the interrogates Context#tree breaks when typed doesn't operate in place.

But in most of the cases, type checking is NOT in place, right? e.g. Select, _Def, Block, Function, ... we do something like

  treeCopy.XYZ(inputTree, newArg1, newArg2).setType(tpe)

The fact that the some parts of the compiler rely on in place typing seems also fishy - we don't really know in which cases the same reference is returned, or do we..?

 

Examples over on SI-7176. I discussed this quickly with Martin yesterday, he suggested that certain at points during type checking (e.g typedApply) should add more specific information to the Context, so that places like `adaptToMemberWithArgs` wouldn't need to rely on tree equality.

-jason 

[Jason Zaugg]

On Wed, Mar 6, 2013 at 10:18 AM, Lukas Rytz <lukas...@epfl.ch> wrote:

As I found out when I tried this, any code the interrogates Context#tree breaks when typed doesn't operate in place.

But in most of the cases, type checking is NOT in place, right? e.g. Select, _Def, Block, Function, ... we do something like

  treeCopy.XYZ(inputTree, newArg1, newArg2).setType(tpe)

The fact that the some parts of the compiler rely on in place typing seems also fishy - we don't really know in which cases the same reference is returned, or do we..?

I completely agree that the current situation is horrible, and should be remedied. Would you like to brainstorm a solution to the context.tree problem today?

-jason

[Lukas Rytz]

Yes, I'll be around in the afternoon starting 14:00. Will that work? Maybe Eugen wants to join.

--

[Eugene Burmako]

Yep I'd be glad to join!

--