Skip to content
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

Hide internal module names used by ChapelStandard #19788

Merged
merged 2 commits into from
May 11, 2022

Conversation

bradcray
Copy link
Member

This is a follow-up to #19306 which took pains to keep internal module symbols
visible to code by default via its use of ChapelStandard. We don't generally
want to make these internal module names known/knowable to users, so this
PR hides them again and updates the one place I found that had been relying
on them being auto-available.

---
Signed-off-by: Brad Chamberlain <bradcray@users.noreply.github.com>
@bradcray
Copy link
Member Author

@mppf: See what you think.
@e-kayrakli : Tagging you since the one place that was relying on the Bytes/String module names was developed (or last touched?) by you.

modules/internal/ChapelStandard.chpl Outdated Show resolved Hide resolved
@mppf
Copy link
Member

mppf commented May 11, 2022

I think an interesting next step would be to make these internal modules all into submodules of ChapelStandard (so you would e.g. import ChapelStandard.Bytes in the one code change here). That would avoid naming conflicts with any user-level modules / mason packages that happen to want the same name.

@bradcray
Copy link
Member Author

I had a different and more aggressive next step in mind: Simply not make the internal module names ever be visible to user code so that we'd avoid conflicts and even avoid letting the user refer to them. Note that even ChapelStandard is a name that isn't really intended to be user facing (and if it were, I'd probably call it something else anyway).

@bradcray
Copy link
Member Author

Proposed the idea in #19793.

---
Signed-off-by: Brad Chamberlain <bradcray@users.noreply.github.com>
@bradcray bradcray merged commit ccf507e into chapel-lang:main May 11, 2022
@bradcray bradcray deleted the hide-internal-module-names branch May 11, 2022 17:10
@bradcray bradcray changed the title Hide internal module names in ChapelStandard Hide internal module names used by ChapelStandard May 11, 2022
bradcray added a commit to bradcray/chapel that referenced this pull request May 13, 2022
This test is only run on linux32, so escaped my testing and refers to
an internal module, which is no longer available by default.  Here,
I'm using an explicit `use` to bring the module's name into scope,
though this will also break if/when we implement the proposal in

---
Signed-off-by: Brad Chamberlain <bradcray@users.noreply.github.com>
bradcray added a commit that referenced this pull request May 13, 2022
…le-ref

Fix failing test fallout from #19788

[trivial, not reviewed]

This test is only run on linux32, so escaped my testing and refers to
an internal module, which is no longer available by default.  Here,
I'm using an explicit `use` to bring the module's name into scope,
though this will also break if/when we implement the proposal in
#19793.
dlongnecke-cray pushed a commit to dlongnecke-cray/chapel that referenced this pull request May 19, 2022
This test is only run on linux32, so escaped my testing and refers to
an internal module, which is no longer available by default.  Here,
I'm using an explicit `use` to bring the module's name into scope,
though this will also break if/when we implement the proposal in

---
Signed-off-by: Brad Chamberlain <bradcray@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants