Add readAll() to Resource and ResourceRetriever

[jslee02]

No description provided.

[mkoval]

I am personally fine with this. But I thought DART tried explicitly avoided using exceptions. For consistency, it might be better to make the std::string an output parameter and return a bool success flag.

[mxgrey]

I think there's some leniency with exceptions used during parsing, since someone would have to be pretty self-destructive to be doing any parsing during a critical control loop.

[jslee02]

But I thought DART tried explicitly avoided using exceptions.

That is true, but I thought we would like to use exceptions when we handle external resources such as files. For example, we have already used exceptions in the parsers.

[mkoval]

I don't think "read remaining data" is true. I suspect that this function will fail if it is called on a Resource that has been partially consumed because getSize() returns the size of the entire resource and read starts reading from the current cursor position.

[mkoval]

I suggest marking this virtual. Some classes may be able to implement this more efficiently than the generic implementation used in the base class.

[mkoval]

I suggest also making this virtual, for the same reason as I explained above.

[mkoval]

LGTM. If exceptions are good enough for @mxgrey, then they're good enough for me!

[psigen]

@mxgrey: Just a side question: Does DART already make use of noexcept to distinguish code paths that never throw exceptions from ones that do?

It seems like that could make it clearer to end-users which DART functions have this guarantee and which don't, since you are right that many loading/visualization/debugging functions don't need it and could have very nice std::exceptions instead.

I don't know what the pros/cons are here though, just curious.

[mxgrey]

I agree that we should consider using noexcept for the benefit clarity and potential performance improvements. The catch (no pun intended) is that we do allow exceptions (in the form of assert) everywhere when DART is compiled in debug mode. The logic is that unwinding the stack is likely to be problematic when running a critical control loop, but it can be very useful when debugging errors.

Perhaps even more problematic is that applying the noexcept specifier to a function which calls any potentially-throwing functions will result in std::terminate if the noexcept function does not handle the exception itself. So if anything used inside of a DART noexcept function does throw an exception, then that DART function must handle the exception or else the entire program will be terminated.

I would like to be able to offer a guarantee to the user that DART functions will handle all possible internal exceptions in a reasonable way, but I think we would need to take some time to examine where all the potential exceptions could pop up inside of DART before we commit to applying the noexcept specifier anywhere.

Conclusion: I think if we have an opportunity to review the exception safety of DART in detail and can determine that using the noexcept specifier is safe, then we could use a macro which expands to noexcept in release mode while being blank in debug mode.

[psigen]

Thanks @mxgrey. I moved the discussion to #878 so we can continue brainstorming about this.