Skip to content
This repository was archived by the owner on Sep 22, 2023. It is now read-only.

Conversation

@jbcoe
Copy link
Owner

@jbcoe jbcoe commented Jul 24, 2022

Work in progress, does not compile.

Work in progress, does not compile.
@jbcoe jbcoe requested a review from Twon July 24, 2022 22:59
@jbcoe jbcoe marked this pull request as draft July 24, 2022 22:59
@codecov
Copy link

codecov bot commented Jul 26, 2022

Codecov Report

Patch coverage: 99.05% and project coverage change: -0.71 ⚠️

Comparison is base (4ac5a7e) 99.79% compared to head (84e2a7b) 99.09%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #103      +/-   ##
==========================================
- Coverage   99.79%   99.09%   -0.71%     
==========================================
  Files           5        6       +1     
  Lines         986      885     -101     
==========================================
- Hits          984      877     -107     
- Misses          2        8       +6     
Impacted Files Coverage Δ
pimpl.h 100.00% <ø> (ø)
pimpl_test.cpp 100.00% <ø> (ø)
indirect_value.h 93.75% <94.64%> (-6.25%) ⬇️
indirect_value_test.cpp 99.63% <100.00%> (-0.12%) ⬇️
optional_test.cpp 100.00% <100.00%> (ø)
pimpl.cpp 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Twon
Copy link
Collaborator

Twon commented Jul 26, 2022

A couple of consequences of these changes. It no longer makes sense to compare indirect_value against nullptr as it now had no observable null state.

However, the construction interface now diverges from that of polymorphic value. Default construction of indirect_value is contingent upon the underlying T being default constructible. But default construction of polymophic_value has been removed (well hidden as a private method) as now there is no observable null state there it does not make sense to allow default construction (as the underlying derived type must be known to do this)

@jbcoe
Copy link
Owner Author

jbcoe commented Jul 27, 2022

Should we make indirect_value non-default constructible or should we live with the API difference?

From a teachability standpoint, symmetry is appealing.

@jbcoe jbcoe closed this Oct 11, 2022
@jbcoe jbcoe reopened this Jun 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants