[PATCH] Analyzer: model trivial copy constructors directly

[Jordan Rose]

On Jan 28, 2013, at 18:43 , Anna Zaks <ganna at apple.com> wrote:

> 
> On Jan 24, 2013, at 5:35 PM, Jordan Rose <jordan_rose at apple.com> wrote:
> 
> +    // Note that we have to be careful here because constructors embedded
> +    // in DeclStmts are not marked as lvalues.
> +    if (!E->isGLValue())
> +      if (const MemRegion *MR = ThisV.getAsRegion())
> +        if (isa<CXXTempObjectRegion>(MR))
> I do not fully understand what the comment means and why the test for CXXTempObjectRegion is needed (if I comment it out all tests pass). So maybe better comment and more tests?

The first line of the comment is "If the constructed object is a prvalue, get its bindings", meaning that the rvalue representation of a struct is its bindings rather than a region. I'll change the comment, though, to say "If the constructed object is a temporary prvalue, get its bindings."

The reason all the tests pass with the CXXTempObjectRegion test commented out is because then a statement like this:

IntWrapper w(3);

will turn into

- Construct '3' in 'w'.
- Set the bindings of 'w' as the "return value" of the constructor.
- Store the bindings of 'w' into 'w'.

That last bit is unnecessary and inefficient (more so because it means keeping an old Store alive for LazyCompoundVal), but doesn't actually affect behavior. By having the constructor return the 'w' region instead, though, it's dead simple to check in VisitDeclStmt if the constructor has done all the necessary initialization of the region. And this is true for any constructor that knows what it's constructing into, so only temporaries actually appear in a legitimate prvalue context.

Jordan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130130/1574ce53/attachment.html>