-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
import foo {.all.}
: import private symbols
#17706
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
@@ -343,31 +364,44 @@ proc hash*(u: SigHash): Hash = | |||
|
|||
proc hash*(x: FileIndex): Hash {.borrow.} | |||
|
|||
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 scopableprivateAccess
rfc
details
proc privateAccess*(t: typedesc)
enables access to private fields oft
in current scopeimport foo {.all.}
enables access to private symbols offoo
, plus all existingimport
syntaxes are supported. Private fields of symbols from foo are not exposed;privateAccess
can 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
c
variable with dual meaning, seegetC, onUse, onDef, onDefResolveForward
in compiler/modulegraphs.nimstyleCheckDef
in 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)