Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
cstratopoulos opened this issue Aug 18, 2023 · 7 comments · Fixed by #62

Comments

@cstratopoulos
Copy link

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.

    template <class StatusCode, class Allocator> friend class indirecting_domain;

    template <class StatusCode, class Allocator> friend class detail::indirecting_domain;

  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 added a commit that referenced this issue Aug 21, 2023
edition. Also fix incorrect friend declaration.
@ned14
Copy link
Owner

ned14 commented Aug 21, 2023

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
Copy link
Author

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.,

return typename StatusCode::domain_type()._do_failure(c.value()->sc);

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
Copy link
Collaborator

Well, status_code_domain already befriends indirecting_domain here:

class status_code_domain
{
template <class DomainType> friend class status_code;
template <class StatusCode, class Allocator> friend class detail::indirecting_domain;

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
Copy link
Owner

ned14 commented Sep 7, 2023

@BurningEnlightenment beat me to writing that comment. Thanks!

Is this issue now fully addressed?

@cstratopoulos
Copy link
Author

Yeah thanks both of you!

@ned14
Copy link
Owner

ned14 commented Sep 7, 2023

Thanks for reporting it!

@ned14 ned14 closed this as completed Sep 7, 2023
@BurningEnlightenment
Copy link
Collaborator

BurningEnlightenment commented Sep 7, 2023

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 ☹️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants