Date: Mon, 11 Dec 2023 10:03:42 +0000
Hi all,
Thanks for all the suggestions! I updated the wording following your suggestions, you can find it at https://stephanlachnit.github.io/mdspan-at/. Please let me know what you think.
Cheers,
Stephan
________________________________
From: Arthur O'Dwyer <arthur.j.odwyer_at_[hidden]>
Sent: Thursday, December 7, 2023 2:45 PM
To: std-proposals_at_[hidden] <std-proposals_at_[hidden]>
Cc: Stephan Lachnit <stephan.lachnit_at_[hidden]>; Hewill Kang <hewillk_at_[hidden]>; Christian Trott <crtrott_at_[hidden]>; Mark Hoemmen <mhoemme_at_[hidden]>
Subject: Re: [std-proposals] Proposal: add `.at()` to `mdspan`
On Thu, Dec 7, 2023 at 8:01 AM Hewill Kang via Std-Proposals <std-proposals_at_[hidden]<mailto:std-proposals_at_[hidden]>> wrote:
Please feel free to give me some feedback on the proposal.
The "Effects: Equivalent to:" can be replaced with the simpler form of "Returns: operator[](...).", and there is no need to mention "with bounds checking" as this is already reflected in "Throws", although "Throws"'s description seems too short and vague.
+1. I recently learned that "Effects: Equivalent to:" implies the throwing behavior; if you change it to "Returns:" then you need to add a "Throws:" (even if "Throws: Nothing").
So that's why `operator[]` has "Effects: Equivalent to" instead of "Returns:" — so that it doesn't need a "Throws:". But `at` already "Throws:", so it might as well "Returns:".
Also, you have "Returns: `operator[](OtherIndexTypes... indices)` with bounds checking." What you should say is "Returns: (*this)[indices...]".
The best I've got for the "Throws" element is (still not good): "Throws: out_of_range if indices<sub>i</sub> >= extent(i) for any indices<sub>i</sub> in indices"
More arcane question: Can you explain why you're proposing `at` to have overloads taking both `span` and `const array&`?
I know it's for consistency with `span.at<http://span.at>`; but then why does `span.at<http://span.at>` have them?
I know it's for consistency with `span.operator[]`; but then why does `span.operator[]` have them?
My next hypothesis was that this was a relic of the world pre-P2447<https://github.com/cplusplus/draft/pull/6691>, where if you only had a `span<Extent>` overload you wouldn't be able to write `myspan[{1,2,3}]`. But now that that's fixed, we should be able to eliminate that overload. This is totally orthogonal to your paper, and I suppose the ball would be in my court to fix it since I authored P2447... :)
But when I try it out<https://godbolt.org/z/83187WxMn>, I discover that `myspan[{1,2,3}]` never worked! `operator[]` is a template that takes `span<OtherExtent>`, and of course there's no way to deduce what that type should be.
I'm cc'ing Mark Hoemmen and Christian Trott — do you guys know what's the deal with mdspan's `at(const array&)` and `at(const span&)` overloads? What would go wrong if both of them were removed? what if only `at(const array&)` were removed?
Thanks,
Arthur
Thanks for all the suggestions! I updated the wording following your suggestions, you can find it at https://stephanlachnit.github.io/mdspan-at/. Please let me know what you think.
Cheers,
Stephan
________________________________
From: Arthur O'Dwyer <arthur.j.odwyer_at_[hidden]>
Sent: Thursday, December 7, 2023 2:45 PM
To: std-proposals_at_[hidden] <std-proposals_at_[hidden]>
Cc: Stephan Lachnit <stephan.lachnit_at_[hidden]>; Hewill Kang <hewillk_at_[hidden]>; Christian Trott <crtrott_at_[hidden]>; Mark Hoemmen <mhoemme_at_[hidden]>
Subject: Re: [std-proposals] Proposal: add `.at()` to `mdspan`
On Thu, Dec 7, 2023 at 8:01 AM Hewill Kang via Std-Proposals <std-proposals_at_[hidden]<mailto:std-proposals_at_[hidden]>> wrote:
Please feel free to give me some feedback on the proposal.
The "Effects: Equivalent to:" can be replaced with the simpler form of "Returns: operator[](...).", and there is no need to mention "with bounds checking" as this is already reflected in "Throws", although "Throws"'s description seems too short and vague.
+1. I recently learned that "Effects: Equivalent to:" implies the throwing behavior; if you change it to "Returns:" then you need to add a "Throws:" (even if "Throws: Nothing").
So that's why `operator[]` has "Effects: Equivalent to" instead of "Returns:" — so that it doesn't need a "Throws:". But `at` already "Throws:", so it might as well "Returns:".
Also, you have "Returns: `operator[](OtherIndexTypes... indices)` with bounds checking." What you should say is "Returns: (*this)[indices...]".
The best I've got for the "Throws" element is (still not good): "Throws: out_of_range if indices<sub>i</sub> >= extent(i) for any indices<sub>i</sub> in indices"
More arcane question: Can you explain why you're proposing `at` to have overloads taking both `span` and `const array&`?
I know it's for consistency with `span.at<http://span.at>`; but then why does `span.at<http://span.at>` have them?
I know it's for consistency with `span.operator[]`; but then why does `span.operator[]` have them?
My next hypothesis was that this was a relic of the world pre-P2447<https://github.com/cplusplus/draft/pull/6691>, where if you only had a `span<Extent>` overload you wouldn't be able to write `myspan[{1,2,3}]`. But now that that's fixed, we should be able to eliminate that overload. This is totally orthogonal to your paper, and I suppose the ball would be in my court to fix it since I authored P2447... :)
But when I try it out<https://godbolt.org/z/83187WxMn>, I discover that `myspan[{1,2,3}]` never worked! `operator[]` is a template that takes `span<OtherExtent>`, and of course there's no way to deduce what that type should be.
I'm cc'ing Mark Hoemmen and Christian Trott — do you guys know what's the deal with mdspan's `at(const array&)` and `at(const span&)` overloads? What would go wrong if both of them were removed? what if only `at(const array&)` were removed?
Thanks,
Arthur
Received on 2023-12-11 10:03:50