Skip to content

Conversation

@adriaanm
Copy link
Contributor

@xeno-by
Copy link
Contributor

xeno-by commented Nov 12, 2013

@adriaanm Thanks! It is definitely progress, because judging from the compilation errors now it's definitely using a correct build of the compiler.

The version of scala-reflect.jar is still old, though, which causes awkward messages like this one ('s are BlackboxContexts, I believe):

[error] /localhome/jenkins/a/workspace/pr-scala-integrate-partest/scala-partest/src/main/scala/scala/tools/partest/package.scala:200: macro implementation has wrong shape:
[error]  required: (c: <notype>)(a: c.<none>[A]): c.<none>[A]
[error]  or      : (c: <notype>)(a: c.<none>[A]): c.<none>[A]
[error]  found   : (c: scala.reflect.macros.Context)(a: c.Expr[A])(implicit evidence$1: c.WeakTypeTag[A]): c.Expr[A]
[error] macro implementations cannot have implicit parameters other than WeakTypeTag evidences
[error]   def trace[A](a: A) = macro traceImpl[A]
[error]       ^

I previously messed this up in a24e7fa, which caused
partest classpath again to include multiple version
of scala-library and friends.

We should really automate enforcing uniqueness of packages.
@xeno-by
Copy link
Contributor

xeno-by commented Nov 12, 2013

@adriaanm As for this failure: https://scala-webapps.epfl.ch/jenkins/job/pr-scala-test/3578/console, I will try to take a look in the evening or tomorrow in the morning.

@adriaanm
Copy link
Contributor Author

Gah, yeah, I realized I "missed a spot"... Rebasing again.

@adriaanm
Copy link
Contributor Author

I re-ran the partest integration against adriaanm/scala-partest master and it passed, so this is good to go once all of these jobs go green: https://scala-webapps.epfl.ch/jenkins/job/pr-scala-integrate-partest/3395/console (up to 3399).

You can do this yourself with the new PR validation by forking scala/jenkins-scripts and fiddling with the settings (in this case in jobs/pr-scala-integrate-partest) and then hitting rebuild on the failed jobs, changing the scriptsRemote parameter to your fork.

@adriaanm
Copy link
Contributor Author

LGTM

Thanks, @xeno-by, this was very nice to review!
I think this is ready to go as-is, but I would appreciate some boy scout duty over at scala/scala-partest to remove the traceImpl macro there, as it seems this needs to stay with the compiler. EDIT: Done

This is the first commit in the series. This commit only:
1) Splits Context into BlackboxContext and WhiteboxContext
2) Splits Macro into BlackboxMacro and WhiteboxMacro
3) Introduces the isBundle property in the macro impl binding

Here we just teach the compiler that macros can now be blackbox and whitebox,
without actually imposing any restrictions on blackbox macros. These
restrictions will come in subsequent commits.

For description and documentation of the blackbox/whitebox separation
see the official macro guide at the scaladoc website:
http://docs.scala-lang.org/overviews/macros/blackbox-whitebox.html

Some infrastructure work to make evolving macros easier:
compile partest-extras with quick so they can use latest library/reflect/...
When an application of a blackbox macro expands into a tree `x`,
the expansion is wrapped into a type ascription `(x: T)`, where `T` is
the declared return type of the blackbox macro with type arguments and
path dependencies applied in consistency with the particular macro
application being expanded.

This invalidates blackbox macros as an implementation vehicle
of type providers.
When an application of a blackbox macro still has undetermined type
parameters after Scala’s type inference algorithm has finished working,
these type parameters are inferred forcedly, in exactly the same manner
as type inference happens for normal methods.

This makes it impossible for blackbox macros to influence type inference,
prohibiting fundep materialization.
When an application of a blackbox macro is used as an implicit candidate,
no expansion is performed until the macro is selected as the result of
the implicit search.

This makes it impossible to dynamically calculate availability of
implicit macros.
When an application of a blackbox macro is used as an extractor in a
pattern match, it triggers an unconditional compiler error,
preventing customizations of pattern matching implemented with macros.
@adriaanm
Copy link
Contributor Author

I amended the first commit a little to fix the build so that partest-extras is compiled with quick, so that BlackboxContext is available to the trace macro in scala.tools.partest.Util.

@adriaanm
Copy link
Contributor Author

Would be great to have a mechanism to configure PR validation from the PR so we could override a property. Here, we'd say: "partest.head=master" (rather than the default "v${partest_version_number}")

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 this pull request may close these issues.

4 participants