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

by-name argument incorrectly evaluated on :-ending operator #1980

Closed
scabug opened this issue May 14, 2009 · 12 comments · Fixed by scala/scala#5969
Closed

by-name argument incorrectly evaluated on :-ending operator #1980

scabug opened this issue May 14, 2009 · 12 comments · Fixed by scala/scala#5969
Assignees
Milestone

Comments

@scabug
Copy link

scabug commented May 14, 2009

scala> def foo() = { println("foo") ; 5 }
foo: ()Int

scala> class C { def m1(f: => Int) = () ; def m2_:(f: => Int) = () }
defined class C

scala> val c = new C
c: C = C@96d484

scala> c m1 foo()

scala> foo() m2_: c
foo

But it is not evaluated if invoked directly:

scala> c.m2_:(foo())

scala>
@scabug
Copy link
Author

scabug commented May 14, 2009

Imported From: https://issues.scala-lang.org/browse/SI-1980?orig=1
Reporter: @paulp

@scabug
Copy link
Author

scabug commented May 14, 2009

Channing Walton (channingwalton) said:
I don't really understand the intricacies here but the following is closer to the use case I had which exhibited this behaviour and may be the same problem, or perhaps another problem:

scala> object D { def ?: [T](block: => T) = { println("In D"); block } }
defined module D

scala> foo() ?: D
foo
In D
res5: Int = 5

scala> D.?:(foo())
In D
foo
res6: Int = 5

Note that the evaluation order appears to be different as the printed statement are in a different order.

@scabug
Copy link
Author

scabug commented May 14, 2009

Channing Walton (channingwalton) said:
sorry the formatting was wrong:

scala> object D { def ?: [T](block: => T) = { println("In D"); block } }
defined module D

scala> foo() ?: D                                                       
foo
In D
res5: Int = 5

scala> D.?:(foo())                                                      
In D
foo
res6: Int = 5

@scabug
Copy link
Author

scabug commented Jun 9, 2009

@dubochet said:
The problem here is that, to maintain left-to-right evaluation semantics, the parser will pre-calculate the argument of m2_: (foo). In other words, a call like foo() m2_: c will be parsed as:

{
  <synthetic> val x$$1 = foo;
  c.m2_$$colon(x$$1)
};

Obviously, this compilation scheme forces the evaluation of foo, before it is used in the body of m2_: (and if at all).

Fixing this isn't trivial: During parsing, when the expansion described above is triggered, there is no symbol information. Therefore, the type of the argument of m2_: is unknown. This makes it impossible to use a special parsing scheme for by-name parameters (i.e. do not pre-calculate the value). The colon-inverting logic would have to happen later, at least after the namer has run, which is rather unpleasant to do.

@scabug
Copy link
Author

scabug commented Jun 9, 2009

@paulp said:
Funnily enough, on saturday I briefly advocated to martin that the parser does too much and that I would be very keen on our separating some of the fancier desugaring that's done on the fly from the pure parsing. The context for that discussion was that we are losing parentheses too early to disambiguate foo(x = 5) from foo((x = 5)), but this issue is also a direct result of it.

Yet another justification for it came up sunday when brainstorming about the viability of scala->javascript translation. We decided the most viable approach was likely to involve using scalac on the original source and running it at least past the typer phase, then doing a syntax-based translation from the original parse tree which essentially abandoned all the types. However to do that we need an "original" parse tree, something not presently offered by scalac!

And the eclipse and other IDE people would gain much from this as well.

And all forms of translation (I would be tempted to pick up scalify again and perform a direct AST->AST translation.) A nice consistent scala pretty printer could be written to go AST->source.

The downside would be the creation of some additional AST nodes which would quickly be eliminated, and a (very minor I believe) performance penalty. Defining a canonical AST representation for a given source representation has many many upsides which I think dwarf the downside.

@scabug
Copy link
Author

scabug commented Oct 14, 2009

Eric Willigers (ewilligers) said:

The problem here is that, to maintain left-to-right evaluation semantics

My first thought is that for right-associative operators (i.e. operators ending in a colon ‘:’), right-to-left evaluation makes more sense:-

For

    lazy val three: Int = { println("three"); 3 }
    lazy val nil: List[Int] = { println("nil"); Nil }

    three :: nil

the output could sanely be

nil
three

instead of the current

three
nil

I expect this avoids the parser complications.

The spec would change from

A left-associative binary operation e1 op e2 is interpreted
as e1.op(e2). If op is right associative, the same operation
is interpreted as { val x=e1; e2.op(x ) }, where x is a
fresh name.

to the simpler

A left-associative binary operation e1 op e2 is interpreted
as e1.op(e2). If op is right associative, the same operation
is interpreted as e2.op(e1)

@scabug
Copy link
Author

scabug commented Jul 6, 2011

@paulp said:
See also #4773 which is more about the inference, but it seems both inference and the incorrect evaluation aspect could be achieved with the desugaring proposed in both tickets.

@scabug
Copy link
Author

scabug commented May 16, 2012

@paulp said:
I apparently failed to update this ticket when I tried this out and found the real deal killer.

scala> val nextid = { var x = 1 ; () => try "x" + x finally x += 1 }
nextid: () => String = <function0>

scala> nextid() #:: nextid() #:: nextid() #:: Stream[String]()
res0: scala.collection.immutable.Stream[String] = Stream(x1, ?)

scala> res1.toList
res1: List[String] = List(x1, x2, x3)

If this evaluates right to left you get List(x3, x2, x1), and because whether an argument is by-name or not is not visible at the call site, you always have to worry about it.

@scabug
Copy link
Author

scabug commented Dec 5, 2012

@retronym said:
Another type inference implication: dependent types from #4518.

class Broke {

  val tempval = new AnyRef {val roleA = new AnyRef with Bar}.roleA

  new AnyRef {} -: tempval // when not assigning to anything, no problem
  val broke_val = new AnyRef {} -: tempval // type mismatch error only when assigning

  trait Foo[AnyRef] {  }

  trait Bar extends Foo[AnyRef] {
    def -:(core: AnyRef): this.type with Foo[core.type] = throw new Exception()
  }
}

@scabug
Copy link
Author

scabug commented Aug 18, 2013

@retronym said:
A good suggestion from the mailing list:

From: Roman Janusz <romeqjanoosh@gmail.com>
Date: Sat, Aug 17, 2013 at 11:40 PM
Subject: Re: [scala-user] Colon-ending operators and by-name parameters
To: scala-user@googlegroups.com
Cc: Roman Janusz <romeqjanoosh@gmail.com>, Sergii Starodubtsev <ses.box@gmail.com>


Anyway, this is really counterintuitive and effectively means that by-name parameters make no sense with right-associative operators. I think it would be nice if the compiler at least issued a warning about this.

W dniu sobota, 17 sierpnia 2013 23:18:44 UTC+2 użytkownik Jason Zaugg napisał:
On Sat, Aug 17, 2013 at 11:07 PM, Roman Janusz <romeqj...@gmail.com> wrote:
SLS 4.6.1 says that call-by-name argument "is not evaluated at the point of function application, but instead is evaluated at each use within the function".

So as for me, current behavior is clearly against the spec.

But 6.12.3 offers:

"If op is right- associative, the same operation is interpreted as { val x=e1; e2.op(x ) }, where x is a fresh name."

-jason

@scabug
Copy link
Author

scabug commented Aug 19, 2013

@retronym said:
Adding a lint warning: scala/scala#2852

@scabug
Copy link
Author

scabug commented Aug 23, 2013

@ghik said:
If I understand correctly, another problem with this parser behaviour is that right-associative macros also make no sense.

@scabug scabug added this to the Backlog milestone Apr 6, 2017
szeiger added a commit to szeiger/scala that referenced this issue Jun 30, 2017
This fixes scala/bug#1980 by changing the
desugaring of right-associative operator syntax in the spec such that
by-name operands now get the same desugaring as left-associative
operators (except for the reversed operands, of course). Only by-value
operands are pulled out into intermediate vals to preserve their
left-to-right evaluation order.

The implementation is as follows:

- `Parsers` still performs the `val` desugaring for all calls, except
  that the generated synthetic names use the new `RIGHT_ASSOC_OP_PREFIX`
  to identify them later.

- When the operator method application is checked in `Typers`, the
  synthetic val’s `Symbol` is marked as `LAZY` if the parameter is
  by-name.

- The `ValDef` of the marked `Symbol` is eliminated in `RefChecks` and
  its RHS inlined at the call site.

This is rather hackish and there is no particular reason for doing it
in `Typers` and `RefChecks` other than the desire to do it early and
without the overhead of extra tree transforms.
szeiger added a commit to szeiger/scala that referenced this issue Jul 10, 2017
This fixes scala/bug#1980 by changing the
desugaring of right-associative operator syntax in the spec such that
by-name operands now get the same desugaring as left-associative
operators (except for the reversed operands, of course). Only by-value
operands are pulled out into intermediate vals to preserve their
left-to-right evaluation order.

The implementation is as follows:

- `Parsers` still performs the `val` desugaring for all calls, except
  that the generated synthetic names use the new `RIGHT_ASSOC_OP_PREFIX`
  to identify them later.

- Everything else happens in `Typers`: After typechecking a ValDef
  resulting from desugaring of a right-associative operator its Symbol
  and RHS are stored in a Map (without knowing at that point if they are
  by-name or by-value).

- After typechecking a method application with an Ident for one of these
  Symbols, check if the parameter is by-name in which case it is
  replaced with the RHS and the Symbol added to a Set.

- After typechecking a Block, check for a leading ValDef with the
  Symbol that was inlined and remove the ValDef.

Fixes scala/bug#1980
szeiger added a commit to szeiger/scala that referenced this issue Nov 16, 2017
This fixes scala/bug#1980 as specified in
SIP-34 (http://docs.scala-lang.org/sips/right-associative-by-name-operators.html)
by changing the desugaring of right-associative operator syntax such
that by-name operands now get the same desugaring as left-associative
operators (except for the reversed operands, of course). Only by-value
operands are pulled out into intermediate vals to preserve their
left-to-right evaluation order.

The implementation is as follows:

- `Parsers` still performs the `val` desugaring for all calls, except
  that the generated synthetic names use the new `RIGHT_ASSOC_OP_PREFIX`
  to identify them later.

- Everything else happens in `Typers`: After typechecking a ValDef
  resulting from desugaring of a right-associative operator its Symbol
  and RHS are stored in a Map (without knowing at that point if they are
  by-name or by-value).

- After typechecking a method application with an Ident for one of these
  Symbols, check if the parameter is by-name in which case it is
  replaced with the RHS and the Symbol added to a Set.

- After typechecking a Block, check for a leading ValDef with the
  Symbol that was inlined and remove the ValDef.

Fixes scala/bug#1980
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 a pull request may close this issue.

2 participants