ned14 / status-code

Proposed SG14 status_code for the C++ standard
Other
64 stars 13 forks source link

Friend declaration of `indirecting_domain` & nested_status_code/status_code_ptr in single header version #58

Closed cstratopoulos closed 1 year ago

cstratopoulos commented 1 year ago

Hi again!

I was trying to follow the docs from https://ned14.github.io/outcome/experimental/worked-example/implicit_conversion/ to create a status code with a value_type bigger than an intptr_t. A couple things

  1. Very possibly I'm missing something, but it seems that using make_nested_status_code (or make_status_code_ptr in the old version) requires a friend declaration for detail::indirecting_status_domain--maybe this should be documented?

  2. I believe the friend declaration may be misspelled in the abstract base class here? There is no detail:: namespace as there is for posix_code, generic_code, etc. https://github.com/ned14/status-code/blob/6bd2d565fd4377e16614c6c5beb495c33bfa835b/include/status-code/status_code_domain.hpp#L107 https://github.com/ned14/status-code/blob/6bd2d565fd4377e16614c6c5beb495c33bfa835b/include/status-code/posix_code.hpp#L78

  3. This just came up incidentally when trying to isolate my compile error on godbolt, but it looks like the single header version does not include the indirecting_domain stuff. I notice that the single-header readme still refers to status_code_ptr.hpp, but there is no mention of make_status_code_ptr or make_nested_status_code in any of the generated headers.

All the best

ned14 commented 1 year ago

I fixed the other two. Can you explain more this one?

Very possibly I'm missing something, but it seems that using make_nested_status_code (or make_status_code_ptr in the old version) requires a friend declaration for detail::indirecting_status_domain--maybe this should be documented?

cstratopoulos commented 1 year ago

Cool! And yeah regarding the first one, let's say we have class MyDomain and using MyCode = status_code<MyDomain>. Then indirecting_domain<MyCode, Alloc> was giving me the error

MyDomain::_do_throw_exception: cannot access protected member declared in class MyDomain

I think this is because most of the protected members just indirect to the underlying domain type, e.g., https://github.com/ned14/status-code/blob/6bd2d565fd4377e16614c6c5beb495c33bfa835b/include/status-code/nested_status_code.hpp#L99

But _do_failure is in all likelihood protected in the domain being indirected, and indirecting_domain has no inheritance relationship to that domain, so it doesn't work.

BurningEnlightenment commented 1 year ago

Well, status_code_domain already befriends indirecting_domain here: https://github.com/ned14/status-code/blob/2086dafa6c26a50ddca0e1ecde56a7fb6728b486/include/status-code/status_code_domain.hpp#L104-L107

I.e. we can remove the need for friend declarations in every domain type by upcasting to status_code_domain like so:

diff --git a/include/status-code/nested_status_code.hpp b/include/status-code/nested_status_code.hpp
--- include/status-code/nested_status_code.hpp
+++ include/status-code/nested_status_code.hpp
@@ -101,9 +101,10 @@
     virtual bool _do_equivalent(const status_code<void> &code1, const status_code<void> &code2) const noexcept override  // NOLINT
     {
       assert(code1.domain() == *this);
       const auto &c1 = static_cast<const _mycode &>(code1);  // NOLINT
-      return typename StatusCode::domain_type()._do_equivalent(c1.value()->sc, code2);
+      typename StatusCode::domain_type foreignDomain{};
+      return static_cast<status_code_domain &>(foreignDomain)._do_equivalent(c1.value()->sc, code2);
     }
     virtual generic_code _generic_code(const status_code<void> &code) const noexcept override  // NOLINT
     {
       assert(code.domain() == *this);

I've put a minimal demonstrator on godbolt

ned14 commented 1 year ago

@BurningEnlightenment beat me to writing that comment. Thanks!

Is this issue now fully addressed?

cstratopoulos commented 1 year ago

Yeah thanks both of you!

ned14 commented 1 year ago

Thanks for reporting it!

BurningEnlightenment commented 1 year ago

No, derived domains still need to explicitly befriend the detail::indirecting_domain<> as demonstrated here on godbolt (you can toggle the DO_REPRO definition). This is expected as we still need to apply the fix I posted above to every wrapped status_code_domain method.

I wanted to submit a PR, but didn't get to it ☹️