Clarify use of import via pseudo dir... #18569
Conversation
The thamming_orc.nim code now counts all created objects being tested, not just the ones following the "first 20" test, and the position of the `destroyed += 1` counter has been adjusted so it counts all the calls that are as a result of `=trace` tracing and not just the original destruction calls.
The following nuances weren't previously fully explained: 1. That `=trace` is only used by `--gc:orc`. 2. That `=trace` is almost certainly used along with `=destroy` when manual resource allocation has been used, but it is only required if there is a possibility of cyclic references in the wrapped types (ie. generic types). 3. That, currently, a forward definition is required for the second of the pair to avoid an auto compiler generation conflict. The pattern of the use of `=trace` has also been made more extensive, showing how both a custom `=destroy` and `=trace` are used for manual allocation of resources when there is any possibility of a cyclic reference in the resource-wrapped values.
Two changes, as follows: 1. There is no mention of the newish `std/` library location and how it relates to the pseudo `std/` import search location, so this change tries to rectify that. 2. Add a link to the new `std/channels` library intended to work with Arc/Orc.
This is recommended but not currently enforced due to legacy code not using pseudo directories. Make the point that the search path to the desired module is not guaranteed without the use of a pseudo directory qualification.
doc/manual.rst
Outdated
| @@ -6190,6 +6190,7 @@ There are two pseudo directories: | |||
| its semantics are: *Use the search path to look for module name but ignore the standard | |||
| library locations*. In other words, it is the opposite of `std`. | |||
|
|
|||
| It is recommended and preferred but not currently enforced that all imports include a "pseudo directorY" to qualify the search path in order to guaranty the correct search path to the desired module. | |||
There was a problem hiding this comment.
Well no, std is recommended, the other pseudo directories (you have a typo in "directorY" btw) are not recommended and hurt code evolution (code moves from Nimble package to somewhere else).
There was a problem hiding this comment.
I understood that there were only two "pseudo directories" std/ and pkg/ and an import should choose one or the other. The manual says here that std/ is the whole standard library and pkg/ is everything except the standard library`. What you say here seems to mean there are other "pseudo directories". Do you mean core/pure/impure/js/wrappers/windows/posix, etc.? I understood that these are just physical directories within the standard library code, and it would seem to be very bad form to use any of them, which I have never seen done.
It would seem to just be a coincidence that there is actually a std/ real sub directory in the standard library and that there is also this std/ "pseudo directory" as they aren't directly related to each other (other than not using the std/ pseudo directory doesn't search the real std/ sub directory, which was my problem before.
How about if, along with correcting the typo, I change the above to make it clear there is a choice between just the two "pseudo directories", either std/ or pkg/ as in the following:
"""
It is recommended and preferred but not currently enforced that all imports include either the std/ or pkg/ "pseudo directory" as part of the import name to qualify the search path in order to guarantee the correct search path to the desired module.
"""
There was a problem hiding this comment.
As far as I understand, std/ should be used for all stdlib imports, but pkg/ should only be used if a module name from a 3rd party package clashes with a local module name.
There was a problem hiding this comment.
Okay, it that is what is intended, I can do a further change little change to doc/manual.rst. Can someone confirm?
There was a problem hiding this comment.
That is what is intended indeed, I can confirm.
There was a problem hiding this comment.
but pkg/ should only be used if a module name from a 3rd party package clashes with a local module name.
this is a bad rule; it requires knowing about context (path, packages installed, file listing etc) to tell whether an import is local or from a 3rd party package (or from stdlib since std/ isn't always used).
Also, you're contradicting what you said here: nim-lang/RFCs#267 (comment)
Things work better when you don't need context to disambiguate, just like precedence rules work better without having to use a symbol table.
pkg/ should be recommended (and later, required with deprecation period) for external package imports
There was a problem hiding this comment.
Also, you're contradicting what you said here: nim-lang/RFCs#267 (comment)
What I said there is a proposal and potential solution for RFC 267. There is currently no guideline that says to prefer pkg/. We currently all agree on using std; we can recommend in the manual to use pkg later.
There was a problem hiding this comment.
we can recommend in the manual to use pkg later.
or we can do it now; the longer we wait, the more work there'll be to change imports to pkg. Adding pkg is forward and backward compatible so doesn't create complications.
We should recommend => deprecate => error
There was a problem hiding this comment.
I cannot "recommend" what is unused by any codebase that I'm aware of.
There was a problem hiding this comment.
Mandating std/ I understand, but I think pkg/ should only be used for disambiguation.
|
Most of my suggestions from #18523 still apply (it's called modules, not libraries, |
I listened to your suggestions in the previous PR and only used the word "library" when referring to the entire Standard Library, using the word(s) "module(s)" when referring to what is being imported. In this PR, I am not talking about Implied but not mentioned is that using the correct pseudo directory of I can add one more word "module" to the suggested change if you prefer as follows: """ |
Make it clear there are only two "pseudo directories".
No you didn't, almost every mention of "library" in your changes to |
No, I didn't intend that, I guess I missed a "rebase"? to only include this single tiny addition to I'm new at the more advanced GitHub stuff, as other times I have just contributed a whole file at a time to my own testing suites. Can you remove all commits except for this tiny one to |
Commit was part of a PR #18523 that was rejected and closed...
This is recommended but not currently enforced due to legacy code not using pseudo directories.
Added clause to make the point that the search path to the desired module is not guaranteed without the use of a pseudo directory qualification.