ned14 / outcome

Provides very lightweight outcome<T> and result<T> (non-Boost edition)
https://ned14.github.io/outcome
Other
704 stars 62 forks source link

Documentation issues #167

Closed cstratopoulos closed 5 years ago

cstratopoulos commented 5 years ago

Hi there

I've been revisiting the outcome docs pending its inclusion in a new project I'm working on (it seems they've had a nice face lift since I was last looking at them!), and I wanted to flag some (mostly minor) issues with the documentation surrounding outcome/C-interop

It looks like the TOC is broken on the boost-ified docs: https://boostorg.github.io/outcome/index.html and https://www.boost.org/doc/libs/develop/libs/outcome/doc/html/index.html

The limitations discussion here still makes reference (and has a broken link) to AFIO, which you've since renamed LLFIO http://ned14.github.io/outcome/tutorial/c-api/limitations/

The code snippet in the C-calling tutorial is empty: http://ned14.github.io/outcome/tutorial/c-api/example2/

The FAQ entry about extern APIs links to the tutorial entry Using Result from C code. However this entry is no longer present in the tutorial topics list.

There is an experimental C macro page which does not appear in the TOC anchors (I'm not sure how I got there to be honest 😅), but also a experimental/c-api/reference on macros.

It seems maybe these are issues related to the creation of the c-api page under the "Experimental" header. Is this meant to coexist with or just replace the tutorial one?

My operating impression is that the "experimental" part of using outcome in extern APIs surrounds using it with status_result or status_outcome which depends on what you describe as the widely-implemented notion of "Almost-standard-layout". But at the same time there exist choices of the T,E, parameters for result<T, E> that pose no problem for the strict requirements of types that can appear in extern "C" APIs, which I guess is what you say in this footnote: https://www.boost.org/doc/libs/develop/libs/outcome/doc/html/index.html#fn:1

ned14 commented 5 years ago

Thanks for the bug report.

It looks like the TOC is broken on the boost-ified docs: https://boostorg.github.io/outcome/index.html and https://www.boost.org/doc/libs/develop/libs/outcome/doc/html/index.html

I think this will be fixed next git push. Go templating is a bit anodyne.

The limitations discussion here still makes reference (and has a broken link) to AFIO, which you've since renamed LLFIO

Fixed next git push.

The code snippet in the C-calling tutorial is empty:

Fixed next git push.

The FAQ entry about extern APIs links to the tutorial entry Using Result from C code. However this entry is no longer present in the tutorial topics list.

The FAQ will be reflowed to match Outcome 2.1 before the Boost 1.70 deadline.

It seems maybe these are issues related to the creation of the c-api page under the "Experimental" header. Is this meant to coexist with or just replace the tutorial one?

It actually looks like the generated gh-pages has become corrupted. It's strayed quite far from where it is supposed to be. I hadn't realised this, and it'll be reset next git push.

My operating impression is that the "experimental" part of using outcome in extern APIs surrounds using it with status_result or status_outcome which depends on what you describe as the widely-implemented notion of "Almost-standard-layout".

Is "almost standard layout" not sufficiently well specified to be absolutely clear?

If not, I've love to hear more on what needs to be made more clear.

cstratopoulos commented 5 years ago

Oh no your "almost standard layout" specification was quite clear! (And interesting to learn about). My confusion was just due to these TOC/gh-pages issues you've described, I was worried that somehow the tutorial/c-api section was meant to be removed or in some way superseded by the experimental/c-api section but I see now that's not the case.

ned14 commented 5 years ago

tutorial/c-api is indeed removed. During the peer review, people complained that I couldn't support the use of std::error_code in C, because its layout is not guaranteed by the standard.

So I've had to relocate and rebase that support into Experimental and not using std::error_code. It is what it is.

cstratopoulos commented 5 years ago

Ahh ok that makes sense. And it seems the situation is similar with boost::system::error_code which currently satisfies standard layout and trivially copyable; but there is no mention of this in the documentation, and its layout is specifically described as // exposition only.

As an aside the arguments in the experimental page are quite convincing and you're frank about the caveats. Probably a bit of discussion/experimentation will determine whether I introduce that to a new project that will be using Outcome at extern API boundaries or just use some Acme::ErrorCode.

ned14 commented 5 years ago

I've pushed fixes to most of the issues you raised. We will need to wait for tomorrow to see if the Boost index is fixed.

ned14 commented 5 years ago

Ok, FAQ is reflowed, Boost index generation fixed. I'd say this issue is closed. Thanks for the report!