module use pollutes local scopes due to public/transitive use ChapelStandard
#13118
Comments
|
A simple modification to I have a vague memory that we might do some fun stuff where we only insert the use of ChapelStandard if the module doesn't have other uses . . . |
|
Does putting a |
|
Yup, but then we encounter it for DefaultAssociative.
If my memory serves (and if I'm not making this up), we did that as a stopgap and that it would be appropriate to remove that special case code in favor of privately using ChapelStandard. |
|
My (vague) recollection is that we stopped auto-inserting |
|
Oooh, that also sounds familiar! Okay, I'll keep pushing along this path, then. |
|
I am down to 7 failures on a rebased version of the branch. This is after a few merges to master so that I could disentangle them from the other error messages. I still need to look into the cause of these four. I suspect that is only really two errors remaining. These three are not entirely unexpected and will be resolved once everything else has been cleaned up (to avoid wasting time continually updating them): |
Make the use of ChapelStandard private by default [reviewed by @mppf] Prior to this change, ChapelStandard was used publicly. This meant that if you used a top-level module, the symbols brought in by ChapelStandard would potentially shadow symbols defined at an outer scope - this was most egregious when defining a variable named e, which was getting shadowed by the Math module's e. Now that we can make use statements private, this change makes the use of ChapelStandard private, so that we have less unexpected symbol shadowing. As a result, we discovered many places where the internal modules were relying on ChapelStandard to find symbols that they used. In response, we have chosen to explicitly add the needed uses privately, and to move functions to different locations when the uses resulted in otherwise unsolvable circular dependencies (see #13356, #13342, and #13339). In the case where we discovered standard modules were previously being exposed by default, we chose to add explicit uses of those modules to ChapelStandard to maintain behavior - we will need to discuss as a group whether bringing these modules in by default is appropriate. This change also alters the behavior of `--library-fortran` - previously, a use of ISO_Fortran_binding was getting inserted for every standard and top-level user module. This caused some circular dependencies as a result of this change. Now, we instead insert the use of this module once, at the end of ChapelStandard. Resolves #13118 Removes the .bad and .future file for the test tracking that issue. Also updates the tests that track module init and deinit order for the reshuffling. Some questions to consider: - Should we move dsiNewSparseDom, etc, to the modules with the symbols they use? - Should we move the array read/writeThis functions to ChapelIO as well? Testing: - full standard paratest with futures - checked test/modules/sungeun/init/printModuleInitOrder.chpl with CHPL_NETWORK_ATOMICS=ugni because I noticed we had a different version of the .good file for that environment variable. It looks like we don't actually run the test with this configuration nightly, though, as it is a bit out of date. - full gasnet paratest with futures
[Forked from @BryantLam's comment: https://github.com//issues/13041#issuecomment-496724325]
Summary of Problem
The
useof a seemingly innocuous library module can eclipse user variables when there are naming conflicts due to the compiler-injecteduseofChapelStandardwhich, like alluses currently, is transitive by nature. I believe the fix here is to implementprivate use(issue #6093) and then utilize it for the compiler-injectedprivate use ChapelStandard).Steps to Reproduce
Associated Future Test(s):
test/modules/use/usePollutesDueToChplStandard.chpl#13117Configuration Information
chpl --version:chpl version 1.20.0 pre-release (f8efa5c)The text was updated successfully, but these errors were encountered: