-
Notifications
You must be signed in to change notification settings - Fork 424
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
Submodules in different files #13524
Comments
Would we allow submodules defined in this way to override their privacy by defining themselves as public module L { ... } ? Would it be confusing for the default privacy of a declared module to change based on its location in a directory structure? Or would this be sufficiently distinct for us to reasonably assume that users will understand the difference in behavior? |
I would expect that if the user wanted to make the submodule public, they would
I'm not sure it will be different longer term, because I think we'll move towards private-by-default. Also, I don't understand how to make something that is already public become private, but I do understand how to make something private become public, by re-exporting it with |
Private symbols are always ignored outside the scope where they would be visible. I didn't do anything to public uses to override that privacy, and modules aren't treated differently than other private symbols. |
@lydia-duncan - is there any way a user can override how public these submodules are? Is the only way for their module declarations to include public/private? Re-exporting with public use would be a thing we could support, not something we can do now. |
I believe the only way is for the declaration to include public/private. I don't know how I feel about public use overriding private modules - I'll make a separate issue and we can discuss it there :) |
I opened #13528 |
At least until we have a better strategy, I see no reason not to allow users to do this (and to use the strategy to make the module private or public). Syntactically and conceptually it is reasonable (by analogy to submodules that aren't in separate files). It just looks a little strange (since the public/private refers to M but appears in L.chpl). |
Working example from Rust.
// M/L.rs
pub fn fn1() { println!("fn1"); }
// M.rs
pub mod L;
// MyPackage.rs
pub mod M;
fn main() {
M::L::fn1();
} |
@BryantLam - thanks for clarifying about Rust here and over on #13528 (comment) We could certainly consider allowing
However the situation in this issue is slightly different. We can make Another direction we could go with it is to make
I wrote earlier, assuming that
This gets to what I think is an interesting clarification. If From that point, since We could also consider enabling this behavior for |
If we required one to |
At this point, in answer to @lydia-duncan's question about how one would indicate the public/private visibility of a submodule, I think there are currently 3 proposals:
|
To clarify , would proposals (2) and (3) remove the need for defining submodules as public vs. private, since the privacy is defined at the point of import/usage? Or can the privacy of the submodules be defined at both the submodule definition and the import line? If this is still an open question, I'd advocate for privacy to be determined at one point rather than mixing. In that case, I would think of (2) and (3) as defining the privacy rather than overriding the privacy of submodule I don't have a strong preference between the 3 proposals, but I do lean towards (2) and (3) since it potentially eliminates the need for submodules to have a private/public state. Do any of these proposals generalize better to the case of any submodule rather than just a submodule in a file (the focus of this design issue)? Ideally, users shouldn't have to think about special rules for dealing with submodules in a file. |
No. Private still impacts whether or not a symbol is generated in documentation, and that is absolutely not something a |
I agree with this |
In the OP, what kind of code should I imagine to be within What should I expect to happen if the contents of M.chpl: module P { // Whoops, M.chpl doesn't define a module named M!
writeln("In P");
} Would it still consider |
I was imagining that both I will update the issue description to include some code in the example.
I'd make this case an error. I think such a pattern is more likely to represent confusion (as you were hinting) and generally speaking I think we should get to a style with one module per file (possibly containing submodules). I think for this pattern specifically (submodules in different files) we should enforce that. I.e. I also would expect this to be an error: M.chpl: module M { }
module Q { } and likewise this L.chpl: module P { } or this L.chpl: module L { }
module X { } |
I'm not in favor of enforcing the rule in Michael's latest comment. Allowing the module name to differ from the file name permits programs that swap different implementations in depending the situation. I have a vague recollection of several user codes doing this (maybe Brian Dolan's, maybe the HPO program). Removing that option forces the user to copy their code for the particular implementation into a different file every time they build a different version, instead of just altering which file they include on the compilation command. This seems unnecessary and like it would be annoying to deal with. |
@lydia-duncan - To be clear, I was proposing that rule specifically for modules involved in submodules-in-different-files. That is,
I don't see how using different file names would be possible for submodules-in-different files even without this rule? Especially not for For If it contained just one module, it might be possible for it to have a different name, but I still think that'd be more confusing than useful.
I think a more reasonable way to achieve this, whether we have the rule above or not, would be to have different directories containing different implementations that use the same file name (and module name). Modules stored in files with different names confuse the compiler's current ability to find such modules, so they have to be named on the command line. But the thing that bothers me more about them is that they confuse people looking at the source code. Having a two directories, say |
That's fair, though I think different behavior on this front will be a bit confusing (but that's sort of the nature of the beast). I don't think this has been covered already, what are your thoughts on the case where the file name doesn't match a directory, but the (top-level?) module name does? E.g. M/L.chpl module M { ... } or P.chpl: module Outer {
module M { ... }
...
} |
I think those should be errors. |
I'm also not crazy about the proposed "one module per file" and "module names must match their filenames" proposals, at least as language rules that the compiler would create errors for. I definitely could imagine them being reasonable rules of thumb or style suggestions/requirements for a specific context, like a standard library. Specifically, I think there's been practical value in writing Chapel that has multiple modules in a file (e.g., for use with TIO and writing tests, if nothing else). I also tend to think that there shouldn't be too rigid a relationship between filenames and modules, such that I should be able to take your code, type it into a filename of my choosing (or feed it to the compiler via stdout?) and compile it without complaints. As a specific example, TIO users have no control (that I've found) over what filename is used for the source code. I also think it's nice to be able to establish filename conventions like M-1.0.0.chpl and M-2.0.1.chpl or M-blc-hack.chpl to define different versions of a module M without having that choice result in an error. More generally, it seems like an unnecessary redundancy and something you can only get wrong / mess up without obvious benefit (aside from the organizational / clarity one which is why I'm OK with it as a rule of thumb, just not part of Chapel's definition / requirements). |
Popping up a level: Stewing on this overall proposal the past week, I think the main things that give me pause about it are how implicit the behavior is and how dependent on the layout of the files in the directory structure it is. For example, if you handed me a printout of the files in question so that I could recreate your work locally, nothing about the printouts says to me that So, off the top of my head, let me think about how I would tweak the proposal, if I were championing it, to try and address these concerns. I think first, I would have the default "submodule directory name" probably be something more like I'd probably also have the Next, I think I would put some sort of indicator into a module's source code to indicate that I wanted to invoke this "treat-all-modules-defined-within-a-given-specially-named-subdirectory-that-happens-to-live-adjacent-to-this-file-as-submodules-to-this-module" rule, so that if I were reading printouts of the code, I'd have some sort of visual cue that the intention was to invoke this feature to make additional submodules available. This would also save me from surprises if I happened to have a directory of Chapel source that matched my module name which I didn't intend to get parsed and compiled in this way. As a dumb example, if I saw something like this: M.chpl: module M {
inject submodules;
...
} it seems like it would clearly indicate that some submodules were being injected here by the compiler (though I might need to go read the documentation to understand exactly what was happening... but at least I'd have something to go search on when reading unlike in the OP?). Of course, if I was given this as a printout, it still wouldn't tell me what was being injected nor where I should arrange to save the That said,
So then, feeling stuck, my head goes to:
or perhaps:
or even:
This approach embeds a directory location in the source code, which I think @mppf objected to on principle in earlier discussions of the So if we ignore that concern momentarily, it seems to me that this approach has other advantages:
It seems like we might still want some way to say "inject modules from the directory that shares my modulename or my filename (or is based off of it)", but perhaps we can do that using some special param/reserved string symbol name rather than using another keyword (as I was originally trying to do with my I would imagine supporting Of course at this point, my head goes to things like "What if, rather than specifying a directory name whose files we should blanket inject as modules, we could name a specific filename instead?" e.g., And then we could even rename [Note: I promise that in starting into this brainstorming, I honestly didn't set out to try to end up proposing a variant of [[I'm honestly truly nervous about hitting the "Comment" button because I feel like anytime I mention |
Right, but TIO users also could not make a directory for submodules-in-different-files.
Sure. I think using
That would be fine with me.
I'm not opposed to some syntax like this to indicate the feature is being used but I think we could come up with better syntax.
I'd prefer that the directory hierarchy always matched the module hierarchy when using this feature. I want it to be done in a way that is more understandable than configurable. For that reason, I lean against allowing the name to be changed from
I don't see this as an advantage. It just makes code using the pattern more configurable and less understandable. I think we should aim for understability on this feature before configurability.
I don't know if this has been proposed before, but if we were certain that we wanted to be able to bring in one submodule at a time in different files (as you are proposing but which I am not certain is the approach we should choose), why wouldn't we simply write it as // M.chpl
module M {
module "M.submodules/L.chpl";
} ? I'm saying that it seems to me that Anyway, to my eyes, this is more or less the same as // M.chpl
module M {
module L;
} meaning that the compiler should go look for Perhaps it would be productive to discuss configurability vs understandability/consistency as regards directories and files? Certainly we could keep discussing specific proposals but it seems that we need to either choose one of these over the other or else to try to balance them? (Otherwise I feel we are having the same argument over and over again in different issues). |
I really like where Brad went with this - I felt like it addressed many of the things that were making me wary as well, though I am sure there are still improvements that can be made to it |
I could definitely get behind |
It is paramount to understand my example output #13524 (comment) and why Rust implemented this feature the way it did.
These rationales are not unreasonable or incompatible with the Chapel language, compiler, or other tools.
All proposals have downsides and (1) this concern can be mitigated with
It is unfortunate that the name of the module directory and the default executable name are the same. Ways to mitigate:
(Click here for another alternative.)Python has a packaging layout where the module directory includes an Rust adopted this layout as its first design.
I consider these two languages (and maybe JavaScript) the most successful at packaging today and they're both moving away from this version.
Foreshadowing from below: Would you rather (a) learn that that
Continuing the inference, the proposed solution wouldn't just look in
Use a symlink. #10946 (comment)
(It's neat that You could also introduce a new compiler flag: However, let's say for some reason someone wanted to do this or to have two sibling modules to have identical submodules, a directory symlink would work, but that's hacky to me. The actual solution to that issue involves re-exporting the symbol with
This situation is why forcing users to not learn two ways to do something is better. It so happens that the alternative to this proposal is an On reading printouts, if a
I am vehemently against the arbitrary naming of the injection target. My guiding principle is that learning a coding style became obsolete when a style checker like
This is effectively a re-export. Re-exporting (once implemented) solves this issue and is capable of re-exporting only some symbols or renaming the symbols. There is one assumption in some of my counter arguments: files are modules. The overall rationale is sound because it is true in most situations, but there are some behaviors today that make it not true. These current behaviors would have to be re-evaluated or changed. (This is for a different issue.)
Prototype modules are the default behavior for file-scope script-like code (no Success in packaging requires success in building, testing, etc. and by no means do I consider C/C++ to be successful at packaging what with the tools like gmake, CMake, autotools, SCons, gn, build2, Meson, Spack, and Conan all trying to solve one problem of how to find code when the organization is arbitrary. These tools usually do something explicitly declarative, which is build-engineer-level of work that To focus the discussion, what properties of this design are things you absolutely could not live with? |
Both Michael's and Bryant's most recent responses seemed to lean on the importance of hierarchies of modules / directories-within-directories being an important driver of this proposal rather than just a single level of submodules as illustrated in the OP. Is that right? If so, Michael, would you consider extending the issue text to include multiple levels of submodules? I've also been curious about whether there are any implications about non-
Sorry, I thought you were saying that the compiler would always require a single module per file whose name matched the filename. If I'm understanding you, I now think you're suggesting that the filename==module name rule would only apply in the event of these automatically-injected directories/files—is that right?
In case it wasn't clear, I wasn't counter-proposing that users would have to only bring in a single submodule at a time. They could definitely bring in an entire directory of files as well. (I can't begin to tell you how often I weep over |
Sure, I'll do that in a moment.
I would hope that the subdirectory would be available (e.g. to C compiler header or library paths) when compiling code in that subdirectory but other than that would not expect any of these to happen automatically. We could consider them as future improvements, though.
Yes, that's what I'm proposing right now. (I like to talk about going further than that, but I don't want to muddy the waters in this issue. I'm intrigued by @BryantLam's suggestion that the implicit file-level module is always created unless there is a single module declaration matching the file name. But these things are almost certainly for another issue). |
FWIW at the moment I like
// M/L.rs
pub fn fn1() { println!("fn1"); }
// M.rs
pub mod L;
fn main() { }
In other words, Rust simply doesn't allow a module with a submodules-subdirectory to be compiled by itself. I suppose it might view these as always being libraries. I like naming the submodules-directory
However if I'm the only one with this view, I could live with |
That's correct, though I attribute it to a technical design decision with the initial Rust modules design that should be corrected. Refer to my previous long-form #13524 (comment) inside the "(Click here for another alternative.)". In lieu of repeating myself, the alternative's design of "the module hierarchy not equaling the directory hierarchy" is occurring in your code and in my original working example from Rust in #13524 (comment) for only the main module. Specifically, the When trying to define a (This is why I didn't like that original design / the No.5 alternative to begin with. It's confusing.) There's a few ways to resolve this inelegance:
|
I could live with it too, though I really push that we should name it For reference, Rust gets around the problem of having to "compile test binaries from |
Maybe we already know this but... Let's suppose for a moment that we chose to require use/import for submodules (discussed in #13536). Then there would be a statement upon which to put the visibility of the submodule. Excerpting and adjusting the example from the top, where we are using M.chpl and M/ to make a submodule with M/L.chpl: M.chpl module M {
import L; // make L visible here; public/private control its visibility elsewhere
proc mFunction() {
L.lFunction();
}
} M/L.chpl module L {
proc lFunction { ... }
} This seems to make the submodule-in-different-file behavior less implicit. |
Following my thoughts in #14407 (comment) - I am currently thinking that e.g. The impact to this issue is that I now have a potentially better answer to the question about how to indicate public/private of a submodule and how to indicate if it is visible outside of the module. To recap this question:
I'm currently thinking that 1) or 2) are reasonable answers. In contrast, for 3), |
PR #15279 implemented something close to this. |
This is a proposal for supporting submodules in different files that was pulled out of
#12923 (comment)
It is an alternative to #10946 and #10909 and is expected to resolve #8470.
The basic idea is that next to a Chapel file such as
M.chpl
(which containsmodule M
) one can place a directoryM/
which contains modules that will be compiled as submodules withinM
.Example and Details
Directory Layout:
Compilation of Main Module:
M/
available to code inM.chpl
(just as they would be with submodules withinM
). As a result, M.chpl could have a call likeL.foo()
which would be allowed inM.chpl
even without a use statement. M.chpl would need to include a use statement if it wanted to write the call toL.foo()
asfoo()
.use
sM
unlessM.chpl
also includedpublic use L
or similar (note that todayuse L
is the same aspublic use L
but that may change).main-module.chpl
would not be able to useL
or to refer to it unless M.chpl includespublic use L
, just as it cannot refer to a private submodule.L
to be a submodule ofM
for privacy / scoping purposes. In particular that means that code inL
can refer to private things inM
.Example contents of the files:
main/main-module.chpl
use M; mFunction();
M.chpl
M/L.chpl
M/L/K.chpl
The text was updated successfully, but these errors were encountered: