[PATCH] D44238: [CFG] Fix automatic destructors when a member is bound to a reference.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: lib/Analysis/CFG.cpp:1435
 /// extended by a local reference with the given initializer.
 static QualType getReferenceInitTemporaryType(ASTContext &Context,
                                               const Expr *Init,
----------------
rsmith wrote:
> NoQ wrote:
> > rsmith wrote:
> > > Can you replace this with `Expr::skipRValueSubobjectAdjustments`? That's how we compute this elsewhere. (You'll need to skip a top-level `ExprWithCleanups` and check for the search terminating in a `MaterializeTemporaryExpr` yourself, but this will reduce duplication and fix a couple more cases this function is getting wrong).
> > Hmm, right, ok, yeah. I'm in no rush with this one, so i'll spend some time trying to understand what //rvalue// adjustments do we still have after rC288563.
> We still have to deal with rvalue adjustments in C++98 compilations, because we don't create xvalue `MaterializeTemporaryExpr` nodes there (because C++98 doesn't have xvalues). I think if/when we carry out the plan to remove `CXXBindTemporaryExpr`, skipping rvalue adjustments will only be interesting to the code that performs lifetime extension, and everything else can just deal with the `MaterializeTemporaryExpr` nodes. But for now, you'll need to skip rvalue adjustments if you want to handle our C++98 ASTs.
> 
> As a quick example, take:
> 
> ```
> struct A { int n; };
> const int &r = A().*&A::n;
> ```
> 
> ... which produces this AST in C++11 onwards:
> 
> ```
> `-VarDecl 0xc2681f0 <col:22, col:46> col:33 r 'const int &' cinit
>   `-ExprWithCleanups 0xc268b28 <col:37, col:46> 'const int' xvalue
>     `-ImplicitCastExpr 0xc2689f0 <col:37, col:46> 'const int' xvalue <NoOp>
>       `-BinaryOperator 0xc2689c8 <col:37, col:46> 'int' xvalue '.*'
>         |-MaterializeTemporaryExpr 0xc2689b0 <col:37, col:39> 'A' xvalue extended by Var 0xc2681f0 'r' 'const int &'
>         | `-CXXTemporaryObjectExpr 0xc2687b0 <col:37, col:39> 'A' 'void () noexcept' zeroing
>         `-UnaryOperator 0xc268990 <col:42, col:46> 'int A::*' prefix '&' cannot overflow
>           `-DeclRefExpr 0xc268928 <col:43, col:46> 'int' lvalue Field 0xc268148 'n' 'int'
> ```
> 
> ... but produces this in C++98 mode (which I think this function will mishandle because it doesn't know how to look through the binary `.*` operator):
> 
> ```
> `-VarDecl 0xcd61c90 <col:22, col:46> col:33 r 'const int &' cinit
>   `-ExprWithCleanups 0xcd62398 <col:37, col:46> 'const int' lvalue
>     `-MaterializeTemporaryExpr 0xcd622a8 <col:37, col:46> 'const int' lvalue extended by Var 0xcd61c90 'r' 'const int &'
>       `-BinaryOperator 0xcd62280 <col:37, col:46> 'int' '.*'
>         |-CXXTemporaryObjectExpr 0xcd62080 <col:37, col:39> 'A' 'void () throw()' zeroing
>         `-UnaryOperator 0xcd62260 <col:42, col:46> 'int A::*' prefix '&' cannot overflow
>           `-DeclRefExpr 0xcd621f8 <col:43, col:46> 'int' lvalue Field 0xcd61be8 'n' 'int'
> ```
Aha, yay, thanks, i'll add this one, and also it seems that CFG and Analyzer C++ tests need way more different `-std=` run-lines :)


Repository:
  rC Clang

https://reviews.llvm.org/D44238