SI-8321 bundles can't be whitebox #3571

Merged
merged 7 commits into from Feb 23, 2014

Conversation

Projects
None yet
3 participants
Member

xeno-by commented Feb 21, 2014

review @retronym

xeno-by added some commits Feb 21, 2014

@xeno-by xeno-by minor code cleanup in the macro engine 6ce573b
@xeno-by xeno-by SI-8321 whitebox bundles are now recognized as such
whitebox.Context <: blackbox.Context, so in order to check for blackboxity
it's not enough to check whether the context used is <: blackbox.Context.
4203170

xeno-by added this to the 2.11.0-RC1 milestone Feb 21, 2014

retronym was assigned by xeno-by Feb 21, 2014

@xeno-by xeno-by bundles now reject invalid context types
Vanilla macros only allow blackbox.Context, whitebox.Context and
PrefixType refinements thereof. Bundles should behave in the same way.
31b52ed

xeno-by added some commits Feb 21, 2014

@xeno-by xeno-by prohibits polymorphic bundles
It's not like they were inducing bugs, but I can't see how polymorphism
can be useful for macro bundles, hence imho it's better to reduce the
number of degrees of freedom of the system.
da1032c
@xeno-by xeno-by more helpful bundle error messages
At the moment, bundle selection mechanism is pretty picky. If a candidate
bundle's parameter isn't either blackbox.Context, whitebox.Context or
PrefixType refinement thereof, then it's not a bundle and the user
will get a generic error.

However we can be a bit more helpful and admit classes that are almost
like bundles (looksLikeMacroBundleType), have them fail isMacroBundleType,
and then emit a much prettier error message to the user that would tell them
that bundles must be monomorphic and their sole parameter should not just
be any subtype of blackbox.Context or whitebox.Context.
64edb44
@xeno-by xeno-by more tests for macro bundles
Given the recent glaring oversight in macro bundles, I have to have more
tests in order to make sure that things are going to work as they should.
fb0c25c
@xeno-by xeno-by more clean up in the macro engine
Now when SI-7507 is fixed in starr, we can actually remove a workaround
and make a 10 line reduction in the size of Resolvers.scala.
dded01b
Owner

retronym commented Feb 21, 2014

LGTM. Well tested and presented. Macro bundles are themselves a new feature, so the surface area for regressions is low.

@adriaanm I recommend this one for RC1, but I'll let you make the call on that as its hard to argue that this is a blocker, other than noting that by not fixing these quirks now, we're probably locked into them for 2.11.N

Member

xeno-by commented Feb 22, 2014

Hey guys! Everything is green, so how about we merge? Please, let this thing in - I would be very ashamed if macro bundles will be released like that.

Owner

adriaanm commented Feb 23, 2014

What Jason said. Merging.

@adriaanm adriaanm added a commit that referenced this pull request Feb 23, 2014

@adriaanm adriaanm Merge pull request #3571 from xeno-by/ticket/8321
SI-8321 bundles can't be whitebox
efa99c0

@adriaanm adriaanm merged commit efa99c0 into scala:master Feb 23, 2014

1 check was pending

default pr-scala OK but waiting for commit da1032 [error](https://scala-webapps.epfl.ch/jenkins/job/pr-scala/6817/): pr-scala failed: Took 80 min.
Details
Member

xeno-by commented Feb 23, 2014

Oh yeah! This made my weekend!!

xeno-by deleted the xeno-by:ticket/8321 branch Feb 23, 2014

scabug referenced this pull request in scala/bug Apr 7, 2017

Closed

bundles can't be whitebox #8321

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment