Skip to content

Regression Aug 15th-17th in community build compiling Squants #2994

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

Closed
olafurpg opened this issue Aug 18, 2017 · 11 comments
Closed

Regression Aug 15th-17th in community build compiling Squants #2994

olafurpg opened this issue Aug 18, 2017 · 11 comments
Assignees

Comments

@olafurpg
Copy link
Contributor

Compilation errors

[error] -- [E049] Reference Error: /home/travis/build/lampepfl/dotty-community-build/squants/shared/src/main/scala/squants/energy/Energy.scala:242:19 
[error] 242 |    def toEnergy = Energy(s)
[error]     |                   ^^^^^^
[error]     |              object Energy in package energy does not take parameters
[error] -- [E049] Reference Error: /home/travis/build/lampepfl/dotty-community-build/squants/shared/src/main/scala/squants/energy/Power.scala:145:18 
[error] 145 |    def toPower = Power(s)
[error]     |                  ^^^^^
[error]     |               object Power in package energy does not take parameters
[error] -- [E049] Reference Error: /home/travis/build/lampepfl/dotty-community-build/squants/shared/src/main/scala/squants/energy/PowerRamp.scala:109:22 
[error] 109 |    def toPowerRamp = PowerRamp(s)
[error]     |                      ^^^^^^^^^
[error]     |           object PowerRamp in package energy does not take parameters
[error] -- [E049] Reference Error: /home/travis/build/lampepfl/dotty-community-build/squants/shared/src/main/scala/squants/information/Information.scala:214:24 0m
[error] 214 |    def toInformation = Information(s)
[error]     |                        ^^^^^^^^^^^
[error]     |    object Information in package information does not take parameters
[warn] -- Migration Warning: /home/travis/build/lampepfl/dotty-community-build/squants/shared/src/main/scala/squants/market/Money.scala:465:27 
[warn] 465 |    def negate(x: Money) = -x
[warn]     |                           ^^
[warn]     |                 method unary_- in class Quantity requires () argument
[error] -- [E049] Reference Error: /home/travis/build/lampepfl/dotty-community-build/squants/shared/src/main/scala/squants/mass/Mass.scala:216:17 
[error] 216 |    def toMass = Mass(s)
[error]     |                 ^^^^
[error]     |                 object Mass in package mass does not take parameters
[error] -- [E049] Reference Error: /home/travis/build/lampepfl/dotty-community-build/squants/shared/src/main/scala/squants/space/Length.scala:290:19 
[error] 290 |    def toLength = Length(s)
[error]     |                   ^^^^^^
[error]     |               object Length in package space does not take parameters
[warn] 5 warnings found
[error] 6 errors found
[error] (squantsJVM/compile:compileIncremental) Compilation failed

Definition of Energy https://github.com/dotty-staging/squants/blob/616fe0d74bedea35a5b2f0e79dc3c42cbaa5b0ce/shared/src/main/scala/squants/energy/Energy.scala#L27

@smarter
Copy link
Member

smarter commented Aug 18, 2017

My guess would be #2962

@felixmulder
Copy link
Contributor

Minimized:

object Test {
  object Energy {
    private def parse(value: Any) = ???
    def apply = parse _
  }

  // Energy.apply("") // works!
  Energy("") // crashy-boom-boom
}

@smarter
Copy link
Member

smarter commented Aug 21, 2017

Felix's reduction is incorrect because it doesn't compile with scalac either.

@felixmulder
Copy link
Contributor

Here's the corrected version, I'm hoping github mail works - because we
can't access the web ui from EPFL currently.

object Test {

  object Energy {
    def apply(load: Int): Int = load
    private def parse(value: Any) = ???
    def apply = parse _
  }

  Energy("")
}

@olafurpg
Copy link
Contributor Author

olafurpg commented Sep 1, 2017

Any update on this? The community build is still red for Squants. https://travis-ci.org/lampepfl/dotty-community-build/jobs/270746972#L588

@felixmulder
Copy link
Contributor

We concluded that the code is bad and that it should be changed. What happens is that the call Energy("") has to be adapted with an apply. The problem then becomes, how many applys and where?

So it tries:

Energy.apply(""), but that is not legal since we're actually getting back the function apply which in turn has to be adapted, and #2962 makes that an impossibility.

The code should be changed to: def apply(a: Any) = parse(a) in Squants.

Sorry for the delayed response,
F

@olafurpg
Copy link
Contributor Author

olafurpg commented Sep 1, 2017

Fair enough, I agree with the conclusion 👍

@olafurpg olafurpg closed this as completed Sep 1, 2017
@felixmulder
Copy link
Contributor

We should put this in the spec, though :)

@allanrenucci
Copy link
Contributor

I am reopening this issue. The following code snippet is rejected by Dotty but with an unrelated error message:

object Test {
  object Energy {
    private def parse(value: Any) = ???
    def apply = parse _
  }

  Energy("")
}
-- [E043] Syntax Error: tests/allan/Test.scala:23:2 ----------------------------
23 |  Energy("")
   |  ^^^^^^
   |  overloaded or recursive method Energy needs return type

@allanrenucci allanrenucci reopened this Sep 28, 2017
@smarter
Copy link
Member

smarter commented Sep 28, 2017

I think the culprit is 30ac9e2#diff-7c63b7bfffa9340fb48ac9b61fa56beeR1931, some conditions were added to check if we're in an overloaded or recursive method call but I don't understand what they're based on. We could revert to the previous logic here.

allanrenucci added a commit to dotty-staging/squants that referenced this issue Sep 28, 2017
- Rewrite `apply = parse _` to `apply(x: Any) = parse(x)`:
  scala/scala3#2994
- Remove self type: scala/scala3#2214
allanrenucci added a commit to dotty-staging/squants that referenced this issue Sep 28, 2017
- Rewrite `apply = parse _` to `apply(x: Any) = parse(x)`:
  scala/scala3#2994
- Remove self type: scala/scala3#2214
@odersky odersky assigned allanrenucci and unassigned odersky Dec 29, 2017
@odersky
Copy link
Contributor

odersky commented Dec 29, 2017

I think the culprit is 30ac9e2#diff-7c63b7bfffa9340fb48ac9b61fa56beeR1931

I would guess the same. Allan can you revert that commit and see whether it passes?

odersky referenced this issue Jan 13, 2018
Fix #1905: Detect case where bridge would clash with the member
allanrenucci added a commit to dotty-staging/squants that referenced this issue Apr 18, 2018
- Rewrite `apply = parse _` to `apply(x: Any) = parse(x)`:
  scala/scala3#2994
- Remove self type: scala/scala3#2214
allanrenucci added a commit to dotty-staging/squants that referenced this issue Apr 18, 2018
- Rewrite `apply = parse _` to `apply(x: Any) = parse(x)`:
  scala/scala3#2994
- Remove self type: scala/scala3#2214
allanrenucci added a commit to dotty-staging/squants that referenced this issue Apr 19, 2018
- Rewrite `apply = parse _` to `apply(x: Any) = parse(x)`:
  scala/scala3#2994
- Remove self type: scala/scala3#2214
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants