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

Include @​propagate_inbounds docstring in documentation #30403

Merged
merged 2 commits into from
Mar 14, 2019

Conversation

tkf
Copy link
Member

@tkf tkf commented Dec 16, 2018

Just an addition of one line.

@KristofferC KristofferC added docs This change adds or pertains to documentation backport pending 1.0 labels Dec 16, 2018
@fredrikekre
Copy link
Member

fredrikekre commented Dec 17, 2018

AFAICT we don't usually include unexported things in the manual.

Edit: Maybe you can include it in the devdocs for boundschecking instead?

@tkf
Copy link
Member Author

tkf commented Dec 17, 2018

@pure is mentioned but included in the documentation: https://docs.julialang.org/en/v1.2-dev/base/base/#Base.@pure. Other examples: require, summarysize

Even if it is not exported, I suppose @​propagate_inbounds is considered a public API, right? It is impossible (extremely tedious) to write a proper bound checking without it. If it is considered public, I suggest to include in the documentation as otherwise there is no way ATM to mark something is a public API.

include it in the devdocs for boundschecking instead?

It's already mentioned in devdocs: https://docs.julialang.org/en/v1.2-dev/devdocs/boundscheck/#Propagating-inbounds-1

But I think it's better to have it in the manual to make it easier to be discovered.

@fredrikekre
Copy link
Member

Even if it is not exported, I suppose @​propagate_inbounds is considered a public API, right?

Not sure, wouldn't it have been exported in that case?

It's already mentioned in devdocs: https://docs.julialang.org/en/v1.2-dev/devdocs/boundscheck/#Propagating-inbounds-1

Yea, but I was suggesting adding the docstring to that page instead. At least make it a reference.

@tkf
Copy link
Member Author

tkf commented Dec 18, 2018

wouldn't it have been exported in that case?

I already mentioned the counter examples (@pure, require and summarysize). I think more important examples are IteratorSize and IteratorEltype. They are not exported but their docstring are included in the manual https://docs.julialang.org/en/v1.2-dev/base/collections/#Base.IteratorSize and also this blog post https://julialang.org/blog/2018/07/iterators-in-julia-0.7 mentioned them. I would be very surprised if IteratorSize and IteratorEltype are considered non-public.

@KristofferC KristofferC mentioned this pull request Dec 20, 2018
52 tasks
@fredrikekre
Copy link
Member

Ok, but

At least make it a reference.

@tkf
Copy link
Member Author

tkf commented Dec 26, 2018

I added the link.

@KristofferC KristofferC mentioned this pull request Dec 30, 2018
53 tasks
@mbauman
Copy link
Member

mbauman commented Dec 31, 2018

Yeah, I say it's worth documenting this even if we don't export it.

Heck, I'd even be okay exporting it, but it is a bit of an expert feature.

@KristofferC KristofferC mentioned this pull request Jan 5, 2019
15 tasks
@StefanKarpinski StefanKarpinski added backport 1.1 triage This should be discussed on a triage call and removed backport 1.1 labels Jan 31, 2019
@Keno Keno removed backport 1.1 triage This should be discussed on a triage call labels Mar 14, 2019
@Keno
Copy link
Member

Keno commented Mar 14, 2019

Triage: Fine change, but no backport.

@mbauman
Copy link
Member

mbauman commented Mar 14, 2019

Especially since we already reference this in other documentation!

@mbauman mbauman merged commit 9f69b2e into JuliaLang:master Mar 14, 2019
@tkf tkf deleted the doc-propagate_inbounds branch August 18, 2019 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants