> While constructors are an interesting case - do you intend to/is there any
reason this hasn't been generalized over arbitrary functions that involve
taking exactly one copy of a parameter & otherwise not referencing that
parameter?
I agree with you, I tried to make the structure of the code and the
documentation open to new additions of the same kind. Assuming the transform
gets accepted I will log an issue for enhancement in JIRA. I stick to this use
case for now because I think it's already useful and the others introduce a lot
more difficulty IMHO.
> & it's clear that this is an unsafe transformation, right? If, in the
original example, the parameter object is referenced indirectly elsewhere (eg:
another reference parameter has an indirect reference to the movable parameter,
or the movable parameter is a reference to a global) causing the copy to happen
at the time the argument was passed, rather than when the original code copied
it could cause an observably different object to be copied. I don't think that
negates the value in having this transformation - just something to be aware of.
You are right. I put it as a reasonable risk, do we agree on this or this
should be considered risky? The two cases I see (but I think I may have a
limited imagination here) are:
1. The exception locality, if the constructor throws it will throw at a
different place (outside the constructor). Should I look for a function-try
block and avoid transforming if one is present? Unless we known the
constructors can't throw.
2. If the object referenced is modified before in the init-list by a hidden
reference. The best example I could come up with is:
#include <string>
#include <iostream>
struct StringHolder {
std::string S;
};
struct EvilBase {
EvilBase(StringHolder &H) { H.S = "Evil things just happened!"; }
};
struct A : EvilBase {
A(StringHolder &H, const std::string &S) : EvilBase(H), S(S) {}
void dump() { std::cout << S << "\n"; }
private:
std::string S;
};
int main() {
StringHolder H{ "I won't get printed!" };
A a(H, H.S);
a.dump(); // prints "Evil things just happened!\n"
return 0;
}
Did you happen to have something like this in your mind or a less contrived
example? I would like to document why the transform is risky and if there is a
better example than that I guess it's better.
> Also the "pass by value" approach here isn't optimal but is the
nicest/simplest way to write this - in the case where the caller passes a
non-temporary there will be one copy construction + one move construction when
pass by value is used. In the original code (pass by const ref) there will be
one copy construction and no move construction. So the really efficient way is
to have two functions - one for rvalue ref (that move constructs) and one for
const ref (that copy constructs). But that results in a ridiculous explosion of
functions that's just silly - making the pass by value quite sensible as a
default until you find some reason to need to split the two functions (because
the cost of the extra move is unacceptable in some particular case for some
reason)
I agree with you. There is an article about that explains this issue
(http://www.codesynthesis.com/~boris/blog/2012/06/19/efficient-argument-passing-cxx11-part1/).
It shows how difficult it is to make one function of the parameter with an
rvalue and one without. If you get more than one parameter to transform it gets
super-messy very fast (with N being the number of argument copied it requires
2^N overloads).
> (of course, as an extension - detecting read-only operations on the movable
object & replacing references to the parameter with references to the copy (if
it hasn't been mutated) could be cool too, but much more work I assume)
Yeah I thought about this one too but it gets messy if the reference is done
before the assignment for example
struct A {
A(const std::string &M) : Ref(M), Copy(M) {}
const std::string &Ref;
std::string Copy;
};
Or if one of the reference is used to compare against the newly made copy.
struct A {
A(const std::string &S) : Copy(S) {
// suppress duplicate spaces in Copy
Copy.erase(std::unique(Copy.begin(), Copy.end(), [](char c1, char c2) {
return c1 == ' ' && c2 == ' ';
}),
Copy.end());
if (Copy != S)
<do something special if the original value has changed>;
}
std::string Copy;
};
And more things I'm sure. I think this is part of the enhancements but IMHO
in its current state the transform is already useful with a very low
possibility of error.
http://llvm-reviews.chandlerc.com/D1342
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
