Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The removal of the
thiskeyword from theerrormethod's signature on this line fundamentally changes its meaning. In C++23,this Self&& selfis used for deducingthis, whereselfrefers to the object itself. By removingthis,Self&& selfbecomes a regular parameter. This makes the function signature incorrect for a member function intended to return theerr_member of thestd::bad_expected_accessinstance, asSelfcannot be deduced from a non-existent parameter, andself.err_would attempt to access a member of the parameterself, not the class'serr_. This will lead to a compilation error. If the compiler does not support C++23's deducingthis, the correct approach is to provide explicitconst&,&, and&&overloads for theerror()method, which would also require removing thetemplate <typename Self>line.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, perhaps this indicates I'm just not using a sufficiently new version of GCC?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@danielmohansahu I think so.
However, I think we don't have to use this feature here. I will take a closer look tomorrow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I'll leave this PR open for continuity, but please close it as appropriate.