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

Revert "SI-7624 Fix -feature warnings in scala/tools/scalap" #3587

Closed
wants to merge 1 commit into from

Conversation

gkossakowski
Copy link
Member

This reverts commit f2de2c4 because it
broke both lift-json and json4s libraries that depend on scalap's APIs.
Arguably, those libraries shouldn't depend on unofficial APIs but they do
because they had no better alternative at the time (no Scala reflection).

The cost of breaking them is not worth minor change of the package.

The f2de2c4 mixed two things:

  1. Fixing feature warnings
  2. Changing package name

When reverting (and resolving conflicts) I tried to keep 1. and revert just
2. However, there were also some questionable changes related to 1. that
got reverted. In particular, a package object with implicit members that
enable language features is an anti-pattern because members of package
object are visible both within and outside of the package. Therefore,
user could use wildcard import for importing everything from scalap
package and enabled postfixOps language feature unknowingly. I went for
just adding imports in just those few files where they were needed.

Conflicts:
src/partest/scala/tools/partest/nest/Runner.scala
src/scalap/scala/tools/scalap/scalax/rules/Memoisable.scala
src/scalap/scala/tools/scalap/scalax/rules/Rule.scala
src/scalap/scala/tools/scalap/scalax/rules/Rules.scala
src/scalap/scala/tools/scalap/scalax/rules/scalasig/ClassFileParser.scala
src/scalap/scala/tools/scalap/scalax/rules/scalasig/ScalaSig.scala

This reverts commit f2de2c4 because it
broke both lift-json and json4s libraries that depend on scalap's APIs.
Arguably, those libraries shouldn't depend on unofficial APIs but they do
because they had no better alternative at the time (no Scala reflection).

The cost of breaking them is not worth minor change of the package.

The f2de2c4 mixed two things:

  1. Fixing feature warnings
  2. Changing package name

When reverting (and resolving conflicts) I tried to keep 1. and revert just
2. However, there were also some questionable changes related to 1. that
got reverted. In particular, a package object with implicit members that
enable language features is an anti-pattern because members of package
object are visible both _within_ and _outside_ of the package. Therefore,
user could use wildcard import for importing everything from scalap
package and enabled postfixOps language feature unknowingly. I went for
just adding imports in just those few files where they were needed.

Conflicts:
	src/partest/scala/tools/partest/nest/Runner.scala
	src/scalap/scala/tools/scalap/scalax/rules/Memoisable.scala
	src/scalap/scala/tools/scalap/scalax/rules/Rule.scala
	src/scalap/scala/tools/scalap/scalax/rules/Rules.scala
	src/scalap/scala/tools/scalap/scalax/rules/scalasig/ClassFileParser.scala
	src/scalap/scala/tools/scalap/scalax/rules/scalasig/ScalaSig.scala
@gkossakowski
Copy link
Member Author

Review by @adriaanm
//cc @soc

@adriaanm
Copy link
Contributor

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants