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

Use of companionSymbol in custom namer leads to cyclic errors. #7

Closed
retronym opened this issue Nov 27, 2013 · 16 comments
Closed

Use of companionSymbol in custom namer leads to cyclic errors. #7

retronym opened this issue Nov 27, 2013 · 16 comments
Labels
Milestone

Comments

@retronym
Copy link
Collaborator

This code:

        // this will only show companions defined above ourselves
        // so when finishing `class C` in `{ class C; object C }`
        // we won't see `object C` in `companion` - we will see NoSymbol
        // that's the limitation of how namer works, but nevertheless it's not a problem for us
        // because if finishing `class C` doesn't set up the things, finishing `object C` will
        val sym = tree.symbol
        val companion = companionSymbolOf(sym, context)

Leads to a cyclic error compiling Specs2 under Scala 2.11.0, which tries to initialize sym.owner if !currentRun.compiles(sym.owner).

That check is buggy in the compiler.

Here's the error I saw:

7: illegal cyclic reference involving type l
[error] case class RightCheckedMatcher[T](check: ValueCheck[T]) extends OptionLikeCheckedMatcher[({type l[a]=Either[_, a]})#l, T]("Right", (_:Either[Any, T]).right.toOption, check)
[error]                                                                                                             ^
[error] /Users/jason/code/specs2/matcher/src/main/scala/org/specs2/matcher/EitherMatchers.scala:46: illegal cyclic reference involving type l
[error] case class RightMatcher[T]() extends OptionLikeMatcher[({type l[a]=Either[_, a]})#l, T]("Right", (_:Either[Any, T]).right.toOption)
[error]                                                                           ^
[error] /Users/jason/code/specs2/matcher/src/main/scala/org/specs2/matcher/EitherMatchers.scala:50: illegal cyclic reference involving type l
[error] case class LeftCheckedMatcher[T](check: ValueCheck[T]) extends OptionLikeCheckedMatcher[({type l[a]=Either[a, _]})#l, T]("Left", (_:Either[T, Any]).left.toOption, check)
[error]                                                                                                               ^
[error] /Users/jason/code/specs2/matcher/src/main/scala/org/specs2/matcher/EitherMatchers.scala:49: illegal cyclic reference involving type l
[error] case class LeftMatcher[T]() extends OptionLikeMatcher[({type l[a]=Either[a, _]})#l, T]("Left", (_:Either[T, Any]).left.toOption)
[error]                                                                             ^
[info] Compiling 96 Scala sources to

Debug notes:

Context(EitherMatchers.scala) {
   owner       = type l
   tree        = ExistentialTypeTree:Either[_$2, a] forSome { <synthetic> type _$2 }
   scope       = 2 decls
   contextMode = AmbiguousErrors ImplicitsEnabled MacrosEnabled ReportErrors TypeConstructorAllowed
   outer.owner = type l
}

owner = type l
owner.enclosingTopLevelClass = <refinement of AnyRef>

You could workaround this by checking for !sym.owner.enclosingTopLevelClass.isRefinementClass before you try to call that, and submit a patch to the compiler to fold this into CurrentRun#compiles.

I noted another bug with compiles: https://issues.scala-lang.org/browse/SI-7758 that you should also try to guard against.

@retronym
Copy link
Collaborator Author

What's worse, is that the subproject (specs2-matcher) that exhibits the failure itself does not include the macro-paradise plugin. However, if the entire project is built, which includes specs2-matcher-extra that does add paradise as a compiler plugin, specs2-matcher is somehow compiled with the paradise namer.

image

@retronym
Copy link
Collaborator Author

This might be stemming from some aspect of SBT's compiler caching, I'm checking with @harrah

@retronym
Copy link
Collaborator Author

I'm working with: retronym/specs2@f75bc21

retronym added a commit to retronym/specs2 that referenced this issue Nov 27, 2013
  scalamacros/paradise#7

Also of great interest is how paradise, included only in
specs-matchers-extra, is leaking into the compilers of other
sub-projects.
@retronym
Copy link
Collaborator Author

I found the SBT problem: the specs2 build defined an überproject that aggregates the sources/classpaths of all others. That's why saw paradise involved in compiling sources from other modules.

That said, this goes to highlight a funny problem with hacks/hijacking etc: it is much harder to know what to trust and who to blame. Even when the hack isn't to blame, it is! :)

@xeno-by
Copy link
Member

xeno-by commented Nov 28, 2013

@retronym Thank you for the detailed investigation. Indeed, using paradise here led to unpredictable effects. How could we guard ourselves against them in the future?

@retronym
Copy link
Collaborator Author

  • use paradise only where needed. @etorreborre did all the work to get there with his internal modularization, the one residual problem is the überbuild that he has a a backwards compatibility mechanism for users that still want the one-big-JAR. I will submit a patch to his build that lets us remove that module altogether with a flag, and we'll use that to get our integration builds working.
  • be cautious calling companionSymbol -- only do so for symbols that might have companions

@xeno-by
Copy link
Member

xeno-by commented Nov 28, 2013

Are you in Lausanne now? I could drop by, so that we could decide what immediate steps should be taken on my side.

@retronym
Copy link
Collaborator Author

Still a few hours away. I'll drop by when I get there.

retronym added a commit to retronym/specs2 that referenced this issue Nov 28, 2013
This is the only part that depends on macro paradise.
In order to be able to easily test Specs2 against arbitrary
snapshot builds of Scala 2.11, it's advantageous to be able
to build the other 95% independently. This also prevents us
getting stuck in the face of bugs like:

  scalamacros/paradise#7

	> show specs2-matcher-extra/scalacOptions
	[info] List(-Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xplugin:/Users/jason/.ivy2/cache/org.scala-lang.plugins/macro-paradise_2.10.3/jars/macro-paradise_2.10.3-2.0.0-SNAPSHOT.jar)
	[success] Total time: 0 s, completed Nov 28, 2013 11:07:12 AM
	> set disableMatcherExtras in ThisBuild := true
	[info] Defining {.}/*:disableMatcherExtras
	[info] The new value will be used by specs2-matcher-extra/*:libraryDependencies, specs2-matcher-extra/compile:sources, specs2-matcher-extra/test:sources
	[info] Reapplying settings...
	[info] Set current project to specs2 (in build file:/Users/jason/code/specs2/)
	> show specs2-matcher-extra/scalacOptions
	[info] Resolving org.fusesource.jansi#jansi;1.4 ...
	[info] Done updating.
	[info] List(-Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_)
retronym added a commit to retronym/specs2 that referenced this issue Nov 28, 2013
  scalamacros/paradise#7

Also of great interest is how paradise, included only in
specs-matchers-extra, is leaking into the compilers of other
sub-projects.
retronym added a commit to retronym/specs2 that referenced this issue Nov 28, 2013
This is the only part that depends on macro paradise.
In order to be able to easily test Specs2 against arbitrary
snapshot builds of Scala 2.11, it's advantageous to be able
to build the other 95% independently. This also prevents us
getting stuck in the face of bugs like:

  scalamacros/paradise#7

	> show specs2-matcher-extra/scalacOptions
	[info] List(-Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xplugin:/Users/jason/.ivy2/cache/org.scala-lang.plugins/macro-paradise_2.10.3/jars/macro-paradise_2.10.3-2.0.0-SNAPSHOT.jar)
	[success] Total time: 0 s, completed Nov 28, 2013 11:07:12 AM
	> set disableMatcherExtras in ThisBuild := true
	[info] Defining {.}/*:disableMatcherExtras
	[info] The new value will be used by specs2-matcher-extra/*:libraryDependencies, specs2-matcher-extra/compile:sources, specs2-matcher-extra/test:sources
	[info] Reapplying settings...
	[info] Set current project to specs2 (in build file:/Users/jason/code/specs2/)
	> show specs2-matcher-extra/scalacOptions
	[info] Resolving org.fusesource.jansi#jansi;1.4 ...
	[info] Done updating.
	[info] List(-Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_, -Xlint, -deprecation, -unchecked, -feature, -language:_)
retronym added a commit to retronym/specs2 that referenced this issue Nov 28, 2013
  scalamacros/paradise#7

Also of great interest is how paradise, included only in
specs-matchers-extra, is leaking into the compilers of other
sub-projects.
@retronym
Copy link
Collaborator Author

Specs PR: etorreborre/specs2#217

retronym added a commit to retronym/specs2 that referenced this issue Nov 28, 2013
The required functionality (quasiquites) is now part of the normal
compiler.

This has the nice effect of avoiding bugs like:

  scalamacros/paradise#7
@retronym
Copy link
Collaborator Author

The good news is that the standard 2.11 compiler is sufficient to build Specs; we just needed to conditionally disable macro-paradise. This shows that the effort to keep quasiquotes in from scala/scala in sync with those in paradise is worthwhile.

@xeno-by
Copy link
Member

xeno-by commented Nov 28, 2013

@retronym @densh Maybe it would make sense to backport quasiquotes to 2.10.x?

@retronym
Copy link
Collaborator Author

Didn't we have binary compatibility shackles that prevented that?

@xeno-by
Copy link
Member

xeno-by commented Nov 29, 2013

@retronym We did, but maybe we can lift them, because in this particular case only compile time is affected, not runtime. What do you think? /cc @adriaanm

@retronym
Copy link
Collaborator Author

I think we're better off focusing our attentions on 2.11 at this point.

@adriaanm
Copy link
Contributor

adriaanm commented Dec 3, 2013

What Jason said.

xeno-by added a commit that referenced this issue Mar 22, 2014
@xeno-by
Copy link
Member

xeno-by commented Mar 22, 2014

Fixed in 22c6bad

@xeno-by xeno-by closed this as completed Mar 22, 2014
xeno-by added a commit that referenced this issue Mar 22, 2014
xeno-by added a commit that referenced this issue Mar 22, 2014
xeno-by added a commit that referenced this issue Mar 22, 2014
xeno-by added a commit that referenced this issue Mar 22, 2014
xeno-by added a commit that referenced this issue Mar 22, 2014
xeno-by added a commit that referenced this issue Mar 22, 2014
xeno-by added a commit that referenced this issue Mar 22, 2014
xeno-by added a commit that referenced this issue Mar 22, 2014
xeno-by added a commit that referenced this issue Mar 22, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants