Tweak safety paper - Githubissues

mpusz / wg21-papers

ISO C++ Committee papers by Mateusz Pusz

https://mpusz.github.io/wg21-papers

4 stars 7 forks source link

Tweak safety paper #59

Closed chiphogg closed 10 months ago

chiphogg commented 10 months ago

Fix language errors: "although"; "preceding"; "order of operations"
Call out the shaky foundations of conflating value category with lifetime

I have other feedback that I didn't know how to express as edits. Importantly, we need to balance these considerations with the need to minimize changes. It may be best to wait for the committee to provide feedback to guide our focus. That said, here's the rest of my feedback.

8.8.4: I have a hard time understanding the template parameters in the API. Sometimes kind is the 2nd parameter; sometimes the 3rd.
8.10: What is the "modulo" of two vectors?
8.10: More generally, the "character" feature seems more speculative. It may be better to omit or greatly reduce its presence here. As far as I know, we don't know of anyone actually using it in real code --- is that right? If so, we should at least highlight its speculative and untested nature.

JohelEGP commented 10 months ago

8.8.4: I have a hard time understanding the template parameters in the API. Sometimes kind is the 2nd parameter; sometimes the 3rd.

The API is implemented with a parameter pack to reuse named_unit (https://github.com/mpusz/mp-units/issues/155). As the comment suggests, one is for base units, the other for derived units.

mpusz commented 10 months ago

8.8.4: I have a hard time understanding the template parameters in the API. Sometimes kind is the 2nd parameter; sometimes the 3rd.

Providing a kind is optional for units. For example, you will not provide those for some natural units systems. The typical convention is to put the optional parameters at the end and this was the rationale here. We can refactor if we find it confusing

mpusz commented 10 months ago

8.10: What is the "modulo" of two vectors?

Right, the text should be fixed. My implementation on a branch already constraints it to scalars only.

mpusz commented 10 months ago

8.10: More generally, the "character" feature seems more speculative. It may be better to omit or greatly reduce its presence here. As far as I know, we don't know of anyone actually using it in real code --- is that right? If so, we should at least highlight its speculative and untested nature.

Actually, all the users of V2 use characters already, but in a limited scope. All the quantity specs we have provided have a proper quantity character assigned and require a proper representation type. There is no support for vector or tensor operations in the library as of now. I have it on my branch and hoped to finish it before Kona, but unfortunately, I did not have enough time to do that.

mpusz commented 10 months ago

@chiphogg, could you please submit issues for the above? Your comments will "disappear" from our radar when I will merge this PR.