On Thu, Apr 28, 2016 at 11:02 AM, Ehsan Akhgari <ehsan.akhg...@gmail.com>
wrote:
> On 2016-04-28 9:52 AM, Gerald Squelart wrote:
> > On Thursday, April 28, 2016 at 11:25:38 AM UTC+10, Ehsan Akhgari wrote:
> >> On 2016-04-28 9:00 AM, Gerald Squelart wrote:
> >>> On Thursday, April 28, 2016 at 10:41:21 AM UTC+10, Ehsan Akhgari wrote:
> >>>> On 2016-04-28 8:00 AM, Gerald Squelart wrote:
> >>>>> On Thursday, April 28, 2016 at 9:35:56 AM UTC+10, Kyle Huey wrote:
> >>>>>> Can we catch this pattern with a compiler somehow?
> >>>>>>
> >>>>>> Foo foo;
> >>>>>> foo.x = thing;
> >>>>>> DoBar(mozilla::Move(foo));
> >>>>>> if (foo.x) { /* do stuff */ }
> >>>>
> >>>> I think so. We already have an analysis which would detect whether
> the
> >>>> return value of a function is used somewhere or not. We should be
> able
> >>>> to reuse that to find the call to DoBar(), and then look for future
> >>>> occurrences of foo used as an rvalue in the rest of the function.
> Once
> >>>> we detect a use of "foo" as an lvalue, further usages of it as an
> rvalue
> >>>> in the same function should be OK and not trigger the error. File a
> bug?
> >>>>
> >>>>> Definitely something that would be nice.
> >>>>>
> >>>>> But if we have/implement such a catcher, I'd like to have an
> annotation to say "yep I really want to reuse this moved-from object".
> >>>>> Because sometimes the function will choose not to actually move from
> an rvalue-ref, or the object knows to revert to a fully-reusable state, etc.
> >>>>
> >>>> What you're describing sounds like a violation of move semantics,
> right?
> >>>> The first case should only happen if DoBar doesn't accept an rvalue
> >>>> reference, in which case the code above is definitely doing something
> >>>> that the author did not expect, given that they have used Move(). The
> >>>> latter case sounds completely broken, and if there is an actual good
> use
> >>>> case for it, the C++ move semantics sound like the wrong tool to
> achieve
> >>>> that goal to me.
> >>>>
> >>>> If you feel like I'm missing something or you can make a strong
> argument
> >>>> on why breaking move semantics is OK in some cases, please let me
> know. :-)
> >>>>
> >>>> Cheers,
> >>>> Ehsan
> >>>
> >>> std::move and mozilla:Move are just casts that make an l-value object
> *look* like an r-value, so that when the compiler considers which 'DoBar'
> to use, one that takes an r-value reference will be picked first.
> >>>
> >>> "Move" is probably not the best name because it gives this impression
> that an actual move happens, but that's what we're stuck with in the
> standard.
> >>
> >> Yes, I understand that.
> >>
> >>> I don't see a "violation of move semantics" in there, could you please
> elaborate on what exact move semantics are violated?
> >>> I'd say it's probably more a "perversion of the move spirit". :-)
> >>
> >> Even though Move() doesn't "actually" move anything, it has a meaning to
> >> the author: "when I have an lvalue object, I want to move it to some
> >> other place".
> >>
> >> While it's true that if DoBar() doesn't really move its argument
> >> somewhere the code compiles, in that case DoBar() is breaking the
> >> expectation of the caller since the caller clearly intended for a move
> >> to happen. In C++ there is no way for the author to know that a move
> >> actually happened, so the semantics of using the object after the move
> >> are fuzzy. This is not really helpful since there is no clear way to
> >> "check" whether a move actually happened, and it's not quite clear how
> >> the caller should act if the callee decides to not move. IOW, without
> >> something enforcing that a move actually happens, it is impossible to
> >> understand the code Kyle posted above without knowing the implementation
> >> of DoBar(), which is a problem with the C++ move semantics.
> >>
> >> However if we change the contract so that the callee is expected to
> >> always perform a move, then we can perform a useful check on the caller
> >> side to enforce not touching the object after it has been moved, which
> >> is what Kyle was asking for.
> >>
> >>> In any case, a moved-from object *must* stay valid after that call,
> because at the minimum it will be destroyed at the end of its enclosing
> scope, by invoking its normal destructor, no magic or special path there.
> >>
> >> Define valid. The object will not be *destroyed* after a move, but it
> >> should be "semantically empty" (the definition of "semantically empty"
> >> is different depending on the class of the object.)
> >>
> >> I can see how it could be useful to for example call some methods on the
> >> object such as IsEmpty(), but using the object willy-nilly is not OK
> >> unless if we want to preserve the loose built-in C++ semantics where
> >> Move() doesn't really mean much if anything unless if stars collide and
> >> the callee and the caller both do something in concert. :-)
> >>
> >>> Now what to do with a moved-from object, is I think more a
> philosophical question! Some argue that we should do nothing (except the
> inevitable implied destruction). Others think it should be fine to also
> allow re-assignment (i.e. reuse the variable for something completely
> different). And yet others would allow total reuse.
> >>
> >> I don't think it's a philosophical question at all, quite to the
> >> contrary it is a very practical question. Using such an object as an
> >> lvalue is fine (the re-assignment case for example). Using the object
> >> is any other way is not. Maybe you'll get away with that for *some*
> >> classes, but we're talking about enforcing a more strict set of
> >> semantics on our code which allows us to actually reason about what
> happens.
> >
> > Ok, maybe not philosophical, but definitely bikesheddingly a coding
> style issue which we (the Mozilla developers) should discuss and decide
> upon -- as I think is what is happening here. ;-)
> >
> >>> My position is that 'Move(x)' *usually* means we give the value away
> and don't want to use it again, and therefore a compiler warning/error
> would help catch unexpected reuses; but also that some situations call for
> reuse (e.g. the function doesn't always steal the object's contents, based
> on other factors) and a programmer in the know should be allowed to
> annotate this special case.
> >>
> >> Can you give a practical example of when a sane function would choose to
> >> move some of the times but not others, and also explain how the caller
> >> is supposed to know what to do, and also how it is supposed to know
> >> whether the callee may not move in the first place?
> >
> > Say we have a resource held in a UniquePtr. Then I may have a number of
> potential suitors that could handle its content, so I will go through that
> list and for each, I will say 'take this if you want it', until someone
> actually takes it.
> > E.g.:
> > UniquePtr<MediaFile> file;
> > for (auto& reader : mMediaReaders) {
> > reader.TakeMediaIfKnown(file);
> > if (!file) { // 'file' is now nullptr, reader eated it, we're done.
> > break;
> > }
> > }
> > if (file) {
> > ReportFailure(file);
> > }
> > and a reader's method could look like:
> > void MP4Reader::TakeMediaIfKnown(UniquePtr<MediaFile>&& aFile)
> > {
> > if (aFile && aFile->HasMimeType("video/mp4") {
> > mMyFileUniquePtr = Move(aFile); // Actual move.
> > ...
> > }
> > }
>
> Hmm, OK, this is a good example. :-)
>
> Even though Xidorn's suggestion may work in some cases, I can imagine
> that in other cases you wouldn't want the caller to have to know the
> preconditions of calling TakeMedia.
>
I think this specific case should actually use UniquePtr<MediaFile>& rather
than && in parameter for conditional move, so that callsite can only pass
in an lvalue, and we don't need a Move there.
- Xidorn
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
