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.

Sign up for GitHub

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

Jump to bottom

SI-9473 Cleaner references to statically owned symbols #4749

Merged
merged 1 commit into from
Sep 22, 2015
Merged

SI-9473 Cleaner references to statically owned symbols #4749

merged 1 commit into from
Sep 22, 2015

Conversation

retronym
Copy link
Member

Ever wonder why identity("") typechecks to
scala.this.Predef.identity("")?

It turns out that mkAttributedRef was importing
q"$scalaPackageClass.this.Predef._" for all these years,
rather than q"$scalaModule.Predef._".

This commit makes mkAttributedRef special case static owners
by referring the the corresponding module, instead.

@scala-jenkins scala-jenkins added this to the 2.12.0-M3 milestone Sep 17, 2015
@retronym
Copy link
Member Author

Review by @lrytz

I tried an followup commit that uses more concise trees, so a reference to ScalaRuntime could just be Ident(ScalaRuntimeModule), rather than Select(Select(ScalaPackageClass.sourceModule, "runtime"), TermName("ScalaRuntime")).

This broke some assumptions about trees in some tests, so I didn't pursue it further.

@lrytz
Copy link
Member

lrytz commented Sep 18, 2015

the test output suggests that a qualifying package is now missing - i think it should still be there:

% diff /home/jenkins/workspace/scala-2.12.x-validate-test/test/files/presentation/t8941-presentation.log /home/jenkins/workspace/scala-2.12.x-validate-test/test/files/presentation/t8941.check
@@ -3,5 +3,5 @@ reload: Source.scala
 askType at Source.scala(6,7)
 ================================================================================
 [response] askTypeAt (6,7)
-scala.Predef.???
+Predef.???
 ================================================================================

% diff /home/jenkins/workspace/scala-2.12.x-validate-test/test/files/presentation/random-presentation.log /home/jenkins/workspace/scala-2.12.x-validate-test/test/files/presentation/random.check
@@ -4,7 +4,7 @@ askType at Random.scala(18,14)
 ================================================================================
 [response] askTypeAt (18,14)
 val filter: Int => Boolean = try {
-  java.lang.Integer.parseInt(args.apply(0)) match {
+  lang.Integer.parseInt(args.apply(0)) match {
     case 1 => ((x: Int) => x.%(2).!=(0))
     case 2 => ((x: Int) => x.%(2).==(0))

@retronym
Copy link
Member Author

Pushed updated checkfiles.

@retronym
Copy link
Member Author

Pushed updated checkfiles, for real this time.

@retronym
SI-9473 Cleaner references to statically owned symbols
e265373 
Ever wonder why `identity("")` typechecks to
`scala.this.Predef.identity("")`?

It turns out that `mkAttributedRef` was importing
`q"$scalaPackageClass.this.Predef._"` for all these years,
rather than `q"$scalaModule.Predef._"`.

This commit makes `mkAttributedRef` special case static owners
by referring the the corresponding module, instead.
@lrytz
Copy link
Member

lrytz commented Sep 22, 2015

LGTM

lrytz added a commit that referenced this pull request Sep 22, 2015
@lrytz
Merge pull request #4749 from retronym/ticket/9473
128d632 
SI-9473 Cleaner references to statically owned symbols
@lrytz lrytz merged commit 128d632 into scala:2.12.x Sep 22, 2015
@sjrd sjrd mentioned this pull request Oct 5, 2015
Closed
sjrd added a commit to sjrd/scala-js that referenced this pull request Oct 5, 2015
@sjrd
Fix scala-js#1932: Recognize 2.12.0-M3's new shape of spurious classO…
eefb50a 
…f[T] trees.

The scala/scala PR scala/scala#4749 changed
the shape of the spurious `Predef.classOf[T]` that reach the
`jsinterop` phase. Previously they looked like
`scala.this.Predef.classOf[T]`, where as now they are
`scala.Predef.classOf[T]`.

This commit generalizes the shapes of trees we recognize, so that
both patterns are matched.
@sjrd sjrd mentioned this pull request Oct 5, 2015
Merged
@SethTisue SethTisue added the release-notes worth highlighting in next release notes label Oct 5, 2015
sjrd added a commit to sjrd/scala-js that referenced this pull request Oct 27, 2015
@sjrd
Fix scala-js#1932: Recognize 2.12.0-M3's new shape of spurious classO…
f1cd9cf 
…f[T] trees.

The scala/scala PR scala/scala#4749 changed
the shape of the spurious `Predef.classOf[T]` that reach the
`jsinterop` phase. Previously they looked like
`scala.this.Predef.classOf[T]`, where as now they are
`scala.Predef.classOf[T]`.

This commit generalizes the shapes of trees we recognize, so that
both patterns are matched.
ilinum added a commit to ilinum/intellij-scala that referenced this pull request Nov 19, 2015
@ilinum
Fix decompilation of Predef for 2.12-M3+
92d4ffd 
It was caused by this PR: scala/scala#4749 #SCL-9457 Fixed
ilinum added a commit to ilinum/intellij-scala that referenced this pull request Nov 19, 2015
@ilinum
Fix decompilation of Predef for 2.12-M3+
c01e1e4 
It was caused by this PR: scala/scala#4749
#SCL-9457 Fixed
@ilinum ilinum mentioned this pull request Nov 19, 2015
Merged
retronym added a commit to retronym/scala that referenced this pull request Nov 26, 2015
@retronym
SI-9542 Unify different reprs. of module type refs
e7980e4 
The new unit test shows failures in transitivity of subtyping
and type equivalence, which boil down the the inconsistent
handling of the semantically equivalent:

   ThisType(pre, ModuleClass)
   ModuleTypeRef(pre, ModuleClass)
   SingleType(pre, Module)

This commit:

   - adds a case to `normalizePlus` to unwrap a `ThisType` to
     a `ModuleTypeRef`
   - Use `normalizePlus` more widely during subtype comparison
   - refactor `fourthTry` (part of `isSubType`) to remove code
     that becomes obviated by the use of `normalizePlus`.

This fixes the regression in the extension methods phase which
was triggered by scala#4749.
We can also fix that regression by tweaking the extension methods
phase itself to emit the `ThisType` representation of the owner
of the value class, as before.

I plan to demonstrate the two approaches to fixing the regression
on separate branches, and the propose that the merged result of these
two is useds.
@joroKr21 joroKr21 mentioned this pull request Jan 11, 2016
Closed
sjrd added a commit to sjrd/scala-js that referenced this pull request Jan 12, 2016
@sjrd
Fix scala-js#1932: Recognize 2.12.0-M3's new shape of spurious classO…
800081e 
…f[T] trees.

The scala/scala PR scala/scala#4749 changed
the shape of the spurious `Predef.classOf[T]` that reach the
`jsinterop` phase. Previously they looked like
`scala.this.Predef.classOf[T]`, where as now they are
`scala.Predef.classOf[T]`.

This commit generalizes the shapes of trees we recognize, so that
both patterns are matched.

(cherry picked from commit eefb50a)
@fschueler fschueler mentioned this pull request Jan 21, 2016
Closed
retronym added a commit to retronym/scala that referenced this pull request Feb 1, 2016
@retronym
SI-9542 Unify different reprs. of module type refs
809440f 
The new unit test shows failures in transitivity of subtyping
and type equivalence, which boil down the the inconsistent
handling of the semantically equivalent:

   ThisType(pre, ModuleClass)
   ModuleTypeRef(pre, ModuleClass)
   SingleType(pre, Module)

This commit:

   - adds a case to `normalizePlus` to unwrap a `ThisType` to
     a `ModuleTypeRef`
   - Use `normalizePlus` more widely during subtype comparison
   - refactor `fourthTry` (part of `isSubType`) to remove code
     that becomes obviated by the use of `normalizePlus`.

This fixes the regression in the extension methods phase which
was triggered by scala#4749.
We can also fix that regression by tweaking the extension methods
phase itself to emit the `ThisType` representation of the owner
of the value class, as before.

I plan to demonstrate the two approaches to fixing the regression
on separate branches, and the propose that the merged result of these
two is useds.
retronym added a commit to retronym/scala that referenced this pull request Feb 1, 2016
@retronym
SI-9542 Unify different reprs. of module type refs
6f8c057 
The new unit test shows failures in transitivity of subtyping
and type equivalence, which boil down the the inconsistent
handling of the semantically equivalent:

   ThisType(pre, ModuleClass)
   ModuleTypeRef(pre, ModuleClass)
   SingleType(pre, Module)

This commit:

   - adds a case to `normalizePlus` to unwrap a `ThisType` to
     a `ModuleTypeRef`
   - Use `normalizePlus` more widely during subtype comparison
   - refactor `fourthTry` (part of `isSubType`) to remove code
     that becomes obviated by the use of `normalizePlus`.

This fixes the regression in the extension methods phase which
was triggered by scala#4749.
We can also fix that regression by tweaking the extension methods
phase itself to emit the `ThisType` representation of the owner
of the value class, as before.

I plan to demonstrate the two approaches to fixing the regression
on separate branches, and the propose that the merged result of these
two is useds.
@adriaanm adriaanm added 2.12 and removed 2.12 labels Oct 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
release-notes worth highlighting in next release notes
Projects
None yet
Milestone
2.12.0-M3
Development

Successfully merging this pull request may close these issues.
5 participants
@retronym @lrytz @adriaanm @SethTisue @scala-jenkins