Chapel: Should submodules require a use/import of parent modules to see their use/import statements?

[lydia-duncan]

In #15312, we changed the language so that submodules required a use or import of their parent modules in order to access symbols defined by them. In looking for potential cases where we had missed a necessary update to our internal, standard and package modules, I noticed that we were not treating the module-level uses and imports from a parent module the same way. For example, the GlobWrappers submodule in FileSystem.chpl is able to use qualified access to HaltWrappers on this line, despite not importing HaltWrappers itself - it is making use of the private use of HaltWrappers here.

I think this is a bug (we should treat symbols obtained via imports and uses similarly to those defined at the same scope in terms of whether a submodule can reference them). I'm opening this issue to see if there is disagreement.

[lydia-duncan]

@mppf @bradcray (when you get back) - thoughts?

[mppf]

I think this is a bug (we should treat symbols obtained via imports and uses similarly to those defined at the same scope in terms of whether a submodule can reference them).

I agree.

[bradcray]

Yeah, this seems like an obvious oversight. Sorry for not thinking of it /noticing it earlier.

[lydia-duncan]

Sorry for not thinking of it /noticing it earlier.

I think it was a reasonable oversight, our strategy for traversing scopes seems to get reimplemented in several places - I know there's at least one issue on unifying the traversal between scope resolution and function resolution but it might be good to be more active about it (especially in our compiler rework)

[lydia-duncan]

The change that I currently have is also impacting resolution of use statements of sibling modules. For instance:

module Outer {
   module A { ... }
   module B {
      use A; // This line now breaks.  It has to be `use super.A;` instead.
   }
}

I think that makes sense, A is a symbol defined by Outer and so shouldn't be referenceable without a use of Outer. However, it is impacting a fair number of package modules, so I wanted to make sure we were okay with it.

[bradcray]

@lydia-duncan: That sounds right to me. I'd be curious to see the diff that this causes just to double-check myself.

[lydia-duncan]

It turns out that the HaltWrappers specific imports were also due to internal modules have some public imports of HaltWrappers at the top level (perhaps because we didn't realize the default privacy was also impacting import statements). With the current change I have, and making the imports in ChapelLocale and CPtr explicitly private, the references in Random and FileSystem in submodules now appropriately fail to resolve.

[bradcray]

Not particularly important, but I think I ran into these same HaltWrappers issues last night while working on making use in internal modules private by default.

[lydia-duncan]

I figured if you hadn't already with that branch that you'd see it at some point. Definitely let me know if you don't want me to handle it in that PR (which is now open :) )

[lydia-duncan]

To go back to the earlier comment I made about uses and sibling modules, only 17 tests were impacted and a couple of package and standard modules, which are the second and third commits of that PR

[bradcray]

Definitely let me know if you don't want me to handle it in that PR

Should be a trivial merge, no worries. I just thought it was funny that I'd seen this the evening before you mentioned it.