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

optional<T&> should have the assignment behavior of a T& #21

Open
hostilefork opened this issue Jul 9, 2014 · 5 comments
Open

optional<T&> should have the assignment behavior of a T& #21

hostilefork opened this issue Jul 9, 2014 · 5 comments

Comments

@hostilefork
Copy link

(Took a while to get this posted, but here is sort of my line of thinking.)

References cannot be default constructed or changed to point to a different address. So when assignment to one reference from another is performed, the value the right-hand reference points to is copied over the value in the left-hand reference:

int i1 = 1;
int i2 = 2;
int & ir1 = i1;
int & ir2 = i2;

ir1 = ir2;
cout << i1;  // Will output 2

Using std::experimental::optional gives a different result:

int i1 = 1;
int i2 = 2;
optional<int &> ir1 = i1;
optional<int &> ir2 = i2;

ir1 = ir2;
cout << i1;  // Will output 1

There has apparently been lengthy debate on the subject of whether optional references should act more like references in this case (copying their target values) or more like pointers (retargeting to the new reference). The conclusion is summarized here:

http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3527.html#optional_ref.rationale.assign

This statement is made:

Neither of these models appears more valid than the other.

Not having easy access to the full range of debate, I wanted to mention something that seems it would be addressed in that document but is not.

What I think tips the scale to where it is not all equal is that many cases of optional will come by introduction into existing codebases...perhaps they could be piecewise transforming a T into an optional<T>. In this light, the difference seen in my integer reference transformation above would be a source of error. Such sources of error can be big problems in large codebases attempting to leverage a new standard library feature. And of course, they get trickier if they're not source-level but in general-purpose abstractions, that used to work one way but now work differently.

If std::reference_wrapper did not exist, then perhaps there would be a stronger argument to have optional adopt this container-friendly behavior. Yet the composability of C++ is reliant upon the parts following predictable rules; and breaking those understandings leads to things like auto_ptr. I'd argue this case shows optional<T> deviating in its characteristics relative to T in a way that I think is beyond the scope of its proposal...and could be achieved other ways. Achieving a smoother transition from T to optional<T> cannot be achieved another way.

So it seems to me that the assignment of an optional reference to another should fail with a runtime error on nullopt source or target, and do a value copy assignment if they both contain a reference. If it's too late to consider such a change, then perhaps at least this line of argumentation could be addressed in the proposal...as it currently does not seem to be.

@akrzemi1
Copy link
Owner

akrzemi1 commented Jul 9, 2014

Optional references have not been added to the Fundamentals TS as they were too controversial. So, it is not too late to change them. However, the semantics of throwing an exception upon assignment doesn't look like the right way to go.

@hostilefork
Copy link
Author

Not the right way to go because... (?) I don't see why doing the same thing that happens when dereferencing a nullopt in other cases would be taboo. That is effectively what you would be trying to do with an optional reference in an assignment, so why not?

Wish I'd known a bit sooner that optional references might not make the cut, as I've been changing code to use them! :-/ I like them, but I do have the concern above about not bending the semantics for containership convenience.

@akrzemi1
Copy link
Owner

akrzemi1 commented Jul 9, 2014

I expect optional<T&> to bare resemblance to optional. The latter gives you an option of deferred initialization: you start with an optional containing no value, and you can assign value at a later stage. In your proposal, if I understand it correctly, the syntax would indicate that this is possible, but I would get a crash or a throw. If this is the case, I would rather not have the assignment at all. ... But that upsets other people...

@hostilefork
Copy link
Author

I expect optional to bare resemblance to optional.

That old StackOverflow question I cited (that you weighed in on) is where I felt the understanding had been hammered out that optional<T> was more like T than it was like a vector containing zero or one elements.

(At the time I was trying to understand optional<T> vs optional<const T> vs. optional<T> const vs optional<const T> const. The verdict emerged from deciding the distinction didn't really apply; that there were really only two cases there and not four.)

When dealing with an abstraction that by its nature has a potential for run-time failure in its dereference. I think it goes with the territory to have a run-time failure case. But I'd prefer disallowing assignment to doing something different from what a reference would do (the way the motivating example I gave does).

Is there any chance to start with disallowance as the default for now, and then reviewing the consideration of one behavior or another in a later iteration...when perhaps more attention has been drawn out in practice? It would be better than having to omit optional references entirely.

@akrzemi1
Copy link
Owner

akrzemi1 commented Jul 9, 2014

Is there any chance to start with disallowance as the default for now

I am not sure I understand. Are you asking about what gets standardized, or about this attempt at implementation on GitHub. If it is the former, I do not have in my plans proposing them. They are so controversial that I do not anticipate a success. They are waiting for a champion willing to fight for them. If you are asking about this implementation, you really want to remove the assignment (as it is already there), but I am reluctant to do this. Boost.Optional has had them for years with rebinding semantics. They were settled upon after long discussions in the mailing list.

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

No branches or pull requests

2 participants