Skip to content
This repository has been archived by the owner on Oct 12, 2019. It is now read-only.

Implementation of the Optional macro #77

Merged
merged 5 commits into from
May 16, 2017
Merged

Implementation of the Optional macro #77

merged 5 commits into from
May 16, 2017

Conversation

liufengyun
Copy link
Owner

@liufengyun
Copy link
Owner Author

Implementation of this macro shows that writing macros in gestalt is much easier than scala.reflect, as programmers don't have to deal with owners. Due to the separation of typed and untyped trees, this implementation is also much solid.


val Function(param :: Nil, body) = f
val tempValDef = ValDef(toolbox.fresh("_temp"), prefix)
val tempIdent = Ident(tempValDef.symbol)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether just using tempValDef.name should be enough here.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually not, because tempIdent has to be a typed tree.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why's that?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because transform take typed tree to typed tree.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the tpd => tpd transform essential to this macro, or we can be just fine with untpd => untpd?

q"""
$tempValDef
if ($tempIdent.isEmpty) new Optional(null)
else new Optional($newBody)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's a quick idea. How about we prohibit all non-fresh/non-fullyqualified names in untyped portions of macro expansions? This will make new macros even more robust.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we should write fully qualified name, but here Optional is in the empty package.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should add support for _empty_, much like we have support for _root_. Wdyt?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems difficult to impose this constraint in a simple and consistent way -- there are also local variables.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Best practice in macrology involves gensymming local variables. Otherwise, there's always the danger of inadvertent name capture (and, in the case of Scala, potentially import conflicts).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is definitely a degree of inconvenience that would be imposed on macro programmers, but it'll also fix the hygiene problems that plague almost all macro tutorials and presentations.

Copy link
Owner Author

@liufengyun liufengyun May 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see -- then it means we can remove the constructors for untyped Ident and Select, then we have hygiene for free? That's a very good idea.

Copy link

@xeno-by xeno-by May 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could spin it like that, too. I'm not sure I fully understand the consequences of this particular implementation, but I'm really looking forward to seeing further experiments!

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's indeed a very cool idea -- we also solve the hygiene problem in the new macro system. I'll experiment on it tomorrow.

@xeno-by
Copy link

xeno-by commented May 15, 2017

@liufengyun The new macro system is indeed significantly more robust that the old one.

What I find extremely satisfying is that all these robustness guarantees are achieved by simply separating typed and untyped trees in API signatures, without any additional APIs like currentOwner, changeOwner, newSymbol, etc.

@liufengyun
Copy link
Owner Author

I don't think we can avoid newSymbol, it's required to write mindless like macros to create dummy ident to be subst later.

@xeno-by
Copy link

xeno-by commented May 15, 2017

Not trying to make any argument whether monadless should or shouldn't be supported - that'll be a different discussion. Just noted that this discussion is orthogonal to the already non-trivial benefits provided by the new macro system.

@liufengyun
Copy link
Owner Author

liufengyun commented May 15, 2017 via email

@liufengyun
Copy link
Owner Author

Yes, I agree it's fine to delay the support for monadless, and it's also good to delay the support for problematic extractors. Showing Optional is also a big progress.

But generally, I see there's no escape from hiding the concept type and symbol from meta-programmers. Any efforts in the direction is problematic, in my opinion.

@liufengyun
Copy link
Owner Author

Another observation: macro writers (including me) tend to use extractors directly in patmat a lot:

https://github.com/arosenberger/nalloc/blob/master/macros/src/main/scala/org/nalloc/bitb/kcits/macros/Inliner.scala#L50

My guess is that in patmat, extractors give us more certainty -- there's always some obscurity about what's happening behind quasiqutoes.

@liufengyun liufengyun merged commit 7dc905d into master May 16, 2017
@liufengyun liufengyun deleted the optional branch May 16, 2017 11:38
@liufengyun liufengyun mentioned this pull request May 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants