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

Static[T] fixes #7333

Merged
merged 8 commits into from
Mar 24, 2018
Merged

Static[T] fixes #7333

merged 8 commits into from
Mar 24, 2018

Conversation

zah
Copy link
Member

@zah zah commented Mar 14, 2018

These are some static[T] fixes for problems discovered while wrapping and authoring fixed-size message digest cryptographic libraries at Status.

@zah zah force-pushed the nimbus branch 2 times, most recently from 06fd219 to e8b273b Compare March 15, 2018 19:03
@@ -1396,7 +1396,10 @@ proc semTypeNode(c: PContext, n: PNode, prev: PType): PType =
fixupTypeOf(c, prev, typExpr)
result = typExpr.typ
else:
result = semTypeExpr(c, n, prev)
if c.inGenericContext > 0 and n.kind == nkCall:
Copy link
Member

Choose a reason for hiding this comment

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

why is only nkCall checked here and not all call kinds?

Copy link
Member Author

Choose a reason for hiding this comment

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

This fix turned out to be a bit tricky, due to some tricks involving the use of types and operators (as in the T -> U syntax from module for example). That's why I avoid the handling of such operators here.

All in all, I'm not particularly happy with this broad application of tyFromExpr and my first attempt was to try to introduce more logic in the compiler allowing unresolved statics to pass-through sem without executing the macros where they are used as arguments, but this turned out to be quite involved. We still need a lot of work on enabling implicit statics by default and I think these developments will allow for a nicer solution in the future.

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.

2 participants