ned14 / outcome

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

Document simple basic_result/basic_outcome usage #149

Closed johnthagen closed 6 years ago

johnthagen commented 6 years ago

It would be very helpful to have some short documentation on how to use basic_result<> / basic_outcome<>. It's not the common case, so I think it could go in an appendix. I'd be happy to contribute something, but I'm not sure how to figure out how to use it other than looking at the code.

For example, it seems to need a NoValuePolicy, but I don't see where it's documented what to pass to that.

akrzemi1 commented 6 years ago

I think the idea is that outcome::result<T> is an alias for

outcome::basic_result<
  T, 
  std::error_code,
  outcome::policy::default_policy<T, std::error_code, void>
>

<basic_result.hpp> where basic_result<> is defined does not depend on <error_code> and therefore compiles faster. You can use it when you want a different (presumably cheaper) type for representing errors, such as the experimental status_code which I think is included in this library.

I think basic_result<> should have the default 'value' for the third parameter, and this way we could document that result<T> is an alias for basic_result<T, std::error_code>.

ned14 commented 6 years ago

The basic_*<> types were introduced due to feedback from the Boost peer review. The documentation is the only part of Outcome not updated since the Boost peer review. I am awaiting the end of my current work contract before I finish Outcome and submit it to Boost, I simply don't have the spare time until this contract ends, as it is in a city far from home. But otherwise I agree with the ticket, so I'll leave it open.

johnthagen commented 6 years ago

I think basic_result<> should have the default 'value' for the third parameter, and this way we could document that result is an alias for basic_result<T, std::error_code>.

That would be great! In the meantime, I'll try to play around with specifying a policy manually now that you've provided an example, thanks.

I simply don't have the spare time until this contract ends

Totally fine! Just wanted to provide feedback. 😄

I'm trying to use Outcome in an embedded environment, and from the looks of it, dragging in the normal outcome.hpp header brings quite a lot extra binary size (even with LTO), so I'm hoping the basic_result<> will be better suited for this environment.

ned14 commented 6 years ago

I'm trying to use Outcome in an embedded environment, and from the looks of it, dragging in the normal outcome.hpp header brings quite a lot extra binary size (even with LTO), so I'm hoping the basic_result<> will be better suited for this environment.

You definitely want to use the experimental status_code edition of Outcome. It was specifically designed for use in embedded environments, specifically Freestanding C++. You'll find it drags in almost nothing from the standard library, and it generates much tighter code. I would however caveat the experimental status of that edition, WG21 may significantly modify status_code.

johnthagen commented 6 years ago

You definitely want to use the experimental status_code edition of Outcome.

Sounds awesome! Is this documented anywhere? Or how would I get started trying it out? Is it part of the single header include? Oops, I see @akrzemi1 already mentioned this as part of basic_result.

As a small (very unscientific) test, I compared the following with -O3, LTO:

I haven't yet found out how to use status_code, but I'll keep looking around. basic.hpp seems like a huge win for embedded on first glance though!

johnthagen commented 6 years ago

Now that I'm finally coming around to how policies work and such, is there a reason that unchecked and checked aren't a part of basic.hpp? It seems like they would just as easily be type aliased to basic_result, and wouldn't impact code size or compile time?

I feel like unchecked will be likely used a lot in the "basic" mode that targets minimal code size and especially in no-exception environments.

Though "unchecked" I feel like is a somewhat unfortunate name. It almost reads to someone new to Outcome that the "result is unchecked, or shouldn't be checked." But I can't think of a better name off hand, and I'm sure that ship has long sailed. I'm just trying to think through how other developers not familiar with Outcome will read a function signature with unchecked as the return type.

ned14 commented 6 years ago

Is this documented anywhere?

You probably see now where my lack of time to finish the docs stems from. You can find very terse reference docs on status_code at https://ned14.github.io/status-code/ (scroll to the very bottom).

Or how would I get started trying it out? Is it part of the single header include?

I see no reason why we can't spin you a single header include edition. status_code has become pretty stable recently. A big games studio deployed it into their code which resulted in a raft of corner case bug fixes, since then it's been boring. In fact I just pushed it there to develop branch. It's completely untested as I'm on the train to work, so see how you fare.

I haven't yet found out how to use status_code, but I'll keep looking around.

https://github.com/ned14/outcome/blob/develop/include/outcome/experimental/status_result.hpp

So instead of outcome::result<T>, use outcome::experimental::status_result<T> or outcome::experimental::erased_result<T> depending on what you're doing. See the status_code docs for the difference.

is there a reason that unchecked and checked aren't a part of basic.hpp?

unchecked and checked default to using std::error_code, and including its system header <system_error> is what drags in most of the STL, which in turn is generating all that bloat you saw.

Also, they're not as useful with the experimental status code. status_code understands exceptions being disabled, and "does the right thing". In other words, you can probably just use the default template alias and you'll be fine. That said, if you feel there is a pressing need, we can add an outcome::experimental::status_unchecked or something?

Let's see how you fare with the experimental single header edition, and go from there.

johnthagen commented 6 years ago

That said, if you feel there is a pressing need, we can add an outcome::experimental::status_unchecked or something?

From what you've said, I don't think that's necessary. I was mostly coming at it from a "I only have basic_result<> but would like to have an alias that knows I don't want exceptions". It sounds like status_code is basically that.

Eventually it will probably be helpful to have a dedicated page for embedded/no-exception environments to help steer them towards either basic_result<T,E,all_narrow> or status_code, explaining the tradeoffs. I totally understand this stuff isn't documented yet because it's not totally baked. If I get a handle on it, I can try to help with some docs.

Let's see how you fare with the experimental single header edition, and go from there.

I'll give it a spin. For reference my environment is:

johnthagen commented 6 years ago

I looked at outcome::experimental::status_result<T, E> but wasn't able to figure out how to get it working with a simple enum class example.

It does seem that outcome::basic_result<T, E, outcome::policy::all_narrow> appears to be working and not causing extra code bloat, so I'll probably continue forward with it in the meantime. I also like that I don't need to do any registration into any other error codes to keep things simple (though I realize this hampers automatic propagation with TRY).

I'm probably going to prototype out with something like:

template<class T, class E>
using Result = outcome::basic_result<T, E, outcome::policy::all_narrow>;

with basic.hpp and see if I hit any showstoppers.

Thanks for the info so far!

Edit:

Ah, okay so I can't compile even basic.hpp with -fno-exceptions or else:

basic.hpp:1764:10: fatal error: execinfo.h: No such file or directory
 #include <execinfo.h>
          ^~~~~~~~~~~~

Edit 2:

I am a bit confused by this because I had remembered reading this from the FAQ:

Outcome works perfectly with C++ exceptions and RTTI globally disabled.

Is the error above expected?

Edit 3:

After some more testing, it looks like <execinfo.h> is simply unavailable in gcc-arm-none-eabi. I'll file that into another issue.

This link has some details about *-none-eabi:

I think the difference is calling convention and availability of system calls for the C library to use.

With "arm-eabi-gcc" you have the Linux system C library which will make calls into the kernel IOCTLs, e.g. for allocating memory pages to the process.

With "arm-eabi-none-gcc" you are running on platform which doesn't have an operating system at all - so the C library is different to cope with that.

Other differences include how processes get bootstrapped by the runtime linker / loader (i.e. a lot happens at runtime on Linux, which cannot happen in a baremetal platform).

johnthagen commented 6 years ago

Just wanted to leave on this thread as well that on gcc-arm-none-eabi that

#define OUTCOME_DISABLE_EXECINFO

is necessary if compiling with -fno-exceptions. This could be added to an eventual embedded/bare-metal section of the docs.

ned14 commented 6 years ago

Note that on my ARM dev board here, its SDK does include <execinfo.h>. So it very much depends on your particular ARM SDK.

johnthagen commented 6 years ago

@ned14 Out of curiosity which toolchain are you using? (Is it bare-metal, or something like a low power ARM Linux machine?) The -none- part of my toolchain means bare-metal with no OS.

I'm curious because other than what maybe Clang/LLVM are doing, the toolchain that I'm using is the canonical open source toolchain for C/C++ bare-metal, I believe. Do you have a proprietary IDE/Toolchain setup? (Of which there are plenty as well).

akrzemi1 commented 6 years ago

@johnthagen, Does the updated tutorial address your issue? Especially, the part about no-value policies menstions basic_result: https://ned14.github.io/outcome/tutorial/no-value/

ned14 commented 6 years ago

Also, there is now a OUTCOME_DISABLE_EXECINFO macro to disable the inclusion of <execinfo.h>

johnthagen commented 6 years ago

@akrzemi1 That helps, thanks!