Proto for Boost mailing list page: Re: [proto] problem with constness of operator return types

Subject: Re: [proto] problem with constness of operator return types
From: Eric Niebler (eric_at_[hidden])
Date: 2010-12-02 09:25:49

On 12/2/2010 6:51 AM, Thomas Heller wrote:
> Hi,
> 
> I just encountered a somehow stupid problem. Possibility is high that i 
> missed something.
> 
> The problem is, that proto's default transform cannot handle op_assign 
> correctly. This is due to the fact that operator OP returns a const proto 
> expression, which turns every value type in proto terminals into a const 
> value. 

Actually, it doesn't. See below:

> Meaning, that codes like the following don't compile:
> 
> // following lines copy the literal 9 into the terminal expression:
> boost::proto::terminal<int>::type t1 = {9};
> boost::proto::terminal<int>::type t2 = {9};
> 
> // try to plus assign t2 to t1:
> boost::proto::default_context ctx;
> boost::proto::eval(t1 += t2, ctx);
> 
> This fails due to the following code:

Did you actually try to compile this code?
<snip>

> With that type of expression creation, proto's default evaluation fails with 
> this error message:
> ./boost/proto/context/default.hpp:142:13: error: read-only variable is not 
> assignable

The following code compiles for me (msvc-10)

  #include <boost/proto/proto.hpp>

  int main()
  {
    // following lines copy the literal 9 into the terminal expression:
    boost::proto::terminal<int>::type t1 = {9};
    boost::proto::terminal<int>::type t2 = {9};

    // try to plus assign t2 to t1:
    boost::proto::default_context ctx;
    boost::proto::eval(t1 += t2, ctx);
  }

But please don't use contexts. :-) The following also works:

  boost::proto::_default<> eval;
  eval(t1 += t2);

Either way, t1 ends up with the value 18.

> So far so good ... or not good. This should work!
> The problem is that expressions const qualified above, return a const 
> terminal value as well.

Consider the following code:

  struct S { int & i; }
  int i = 9;
  S const s = {i}; // S is const, s.i is not.
  s.i += 9; // OK

Proto holds children by reference by default, and respects their
constness. So what problem are you seeing, exactly?

I can guess that in Phoenix, you are seeing this problem because we told
Proto that in the Phoenix domain, children need to be held by value. In
this case, the top-level const really is a problem.

What's the right way to fix this? For one thing, your patch is *very*
incomplete. There are a million places in Proto where return types are
const-qualified. I do it in the absence of rvalue references to get
Proto temporaries to bind to lvalue references. That reduces the number
of overloads needed, which (I think) brings down compilation times. If
you removed *all* return type const-qualification, Proto would stop
compiling. You'd need to add a bunch of overloads to make it work again,
and then benchmark that Proto didn't compile slower.

I don't actualy believe this is the right fix. Phoenix stores terminals
by value *by design*. I argue that "t1 += t2" should NOT compile.
Consider this Phoenix code:

  auto t1 = val(9);
  std::for_each(c.begin(), c.end(), t1 += _1);

What's the value of t1 after for_each returns? It's 9! The for_each is
actually mutating a temporary object. The const stuff catches this error
for you at compile time. You're forced to write:

  int t1 = 9;
  std::for_each(c.begin(), c.end(), ref(t1) += _1);

Now the terminal is held by reference, and it works as expected.

If you think there are legitimate usage scenarios that are busted by
this const stuff, please let me know.

-- 
Eric Niebler
BoostPro Computing
http://www.boostpro.com