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

remove some exports from Broadcast #40543

Merged
merged 1 commit into from
Apr 21, 2021
Merged

remove some exports from Broadcast #40543

merged 1 commit into from
Apr 21, 2021

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Apr 20, 2021

It seems like these were unintentional (and undocumented). Refs #26919.

It seems like these were unintentional (and undocumented). Refs #26919.
@vtjnash vtjnash requested a review from mbauman April 20, 2021 19:58
@mbauman
Copy link
Member

mbauman commented Apr 20, 2021

andand and oror were exported because the parser generates them; the reason I marked them for export from the Broadcast module was to match the other Broadcast machinery the frontend uses (like broadcasted and materialize). But they're new and can easily go.

broadcast_preserving_zero_d is used elsewhere (and is in 1.3+), but all the uses are fully qualified (both in the stdlib and in the ecosystem).

@mbauman mbauman added the broadcast Applying a function over a collection label Apr 20, 2021
@vtjnash vtjnash merged commit 7fe05db into master Apr 21, 2021
@vtjnash vtjnash deleted the jn/40543 branch April 21, 2021 19:06
@StefanKarpinski
Copy link
Member

For many syntax lowerings, we lower to a name in the current scope, which allows users to override behavior by defining a different function with a given name. In this case, however, && and || are not overridable syntax, so I don't think it makes sense for .&& or .|| to be overridable (by redefining the andand or oror in the current scope). So it seems like it would be fine for the lowering to emit fully qualified calls to Base.andand and Base.oror so it wouldn't be necessary for these to be exported.

@simeonschaub
Copy link
Member

I don't think they were ever exported from Base, they were only exported from the Broadcast submodule.

@StefanKarpinski
Copy link
Member

Ah, ok. But still: having lowering emit Base.Broadcast.andand and Base.Broadcast.oror seems best.

ElOceanografo pushed a commit to ElOceanografo/julia that referenced this pull request May 4, 2021
It seems like these were unintentional (and undocumented). Refs JuliaLang#26919.
antoine-levitt pushed a commit to antoine-levitt/julia that referenced this pull request May 9, 2021
It seems like these were unintentional (and undocumented). Refs JuliaLang#26919.
johanmon pushed a commit to johanmon/julia that referenced this pull request Jul 5, 2021
It seems like these were unintentional (and undocumented). Refs JuliaLang#26919.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
broadcast Applying a function over a collection
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants