-
- Notifications
You must be signed in to change notification settings - Fork 1.5k
import foo {.all.}: import private symbols #17706
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
Conversation
I'm behind this language extension and will help out with the implementation, if required. |
20f522e to 8c06a0e Compare bbbfc12 to 8519225 Compare | This lacks IC support. ;-) |
16429d1 to 2910048 Compare | | ||
| template getC(): untyped = | ||
| when c is PContext: c | ||
| else: c.c |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Too ugly... Can we find a better solution?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is pre-existing and I've strictly improved things in this PR via getC by avoiding this:
when compiles(c.c.graph): if c.c.graph.onUsage != nil: c.c.graph.onUsage(c.c.graph, s, info) else: if c.graph.onUsage != nil: c.graph.onUsage(c.graph, s, info) which had compiles and was not DRY
But I've added a future work item to further improve this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was only code for "nimfind" which is dead code now that we got "nim check --defusages"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Araq i see you've removed nimfind in #17737 (good or bad, I'm not sure), but please don't remove onDef + friends, they're very useful during debugging (and i wrote a competitor tool of nimfind that added more features, which relied on those onDef, onUsage etc). It's very useful to have 1 place defined that catches all definition/usage events.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to distinguish between nkSym and nkSymDef instead but it would break the macro system until we have an AST to AST translation step. :-/
2910048 to c985e81 Compare | Also ... what if every top level symbol is in the "exported" scope and then there is another filtering step? This could also be used to improve the error message to "symbol module.X exists but is private". I think this would simplify the logic quite a bit. |
| PTAL (btw, in code comments (like #17706 (comment)) instead of at PR level (like #17706 (comment)) are easier to tackle, so that threading is maintained)
I thought about this but let's please defer this to future work after evaluating that it doesn't hurt CT performance (because the common case is
ya, that's another benefit i had listed in #11865; but note that this can be done even with the 2-tables approach i've added those 2 items in future work section |
e90296e to b58761e Compare | PTAL |
| Merging this for now but the feature itself might not be the best idea. It must be documented as "experimental". |
import foo {.all.}import foo {.all.}: import private symbols
supersedes #11865 which was ready for ~2 years but which I had to re-implement from scratch because #16550 changed too many things.
This represents a lot of effort, I hope this PR can be merged soon to avoid rebasing/bitrot/re-implementing many times.
import foo {.all.}#11865import foo {.all.}#11865getField(someObj, someField)to access private fields; use case: custom serialization/deserialization RFCs#106 via scopableprivateAccessrfc
details
proc privateAccess*(t: typedesc)enables access to private fields oftin current scopeimport foo {.all.}enables access to private symbols offoo, plus all existingimportsyntaxes are supported. Private fields of symbols from foo are not exposed;privateAccesscan be used for thatthis gives maximum flexiblity, allowing by-passing of symbol/field visibility in a given scope without affecting other clients of the imported module / type, and without any of the gotchas that come with
include.note to reviewer
future work
cvariable with dual meaning, seegetC, onUse, onDef, onDefResolveForwardin compiler/modulegraphs.nimstyleCheckDefin onDef/onDefResolveForwardimport foo(notimport foo {.all.}) should be evaluatedisDefined(nimDebug)forimport foo {.all.}: import private symbols #17706 (comment) (=> -d:nimDebug: calls doAssert false instead of quit in compiler code #17739)