xazax.hun marked an inline comment as done.
xazax.hun added inline comments.


================
Comment at: clang/lib/Sema/SemaInit.cpp:6775
+    if (!pathOnlyInitializesGslPointer(Path))
+      Init = const_cast<Expr *>(Init->skipRValueSubobjectAdjustments());
 
----------------
gribozavr wrote:
> I'm afraid this change could disable some other analysis, which would hide 
> other lifetime issues. For example, while 'ptr' can't itself dangle, if 
> 'Temp().ptr' is bound to a local reference, it might be subject to complex 
> lifetime extension rules, which this warning originally tried to check for.
I understand your concern, and I thin this code path is very tricky as it is 
doing multiple things at once: lifetime extension and emitting warnings. There 
are two cases when `pathOnlyInitializesGslPointer(Path)` returns true:
* We are initializing a gsl::Pointer, so no references involved, we should not 
do lifetime extension.
* We are visiting the initialization of a reference (like walking a def-use 
chain) that was used to 

In the second case, the reference initialization was already visited earlier 
(when we first saw the initialization and did not follow any use of the 
reference). In this earlier instance, since the reference is not a 
gsl::Pointer, the call to `pathOnlyInitializesGslPointer` is evaluated to 
false, and we did the lifetime extension. 

To observe this behavior you can dump the AST for the following code:
```
namespace std {
template<typename T>
struct reference_wrapper {
  reference_wrapper(T &&);
};

template<typename T>
reference_wrapper<T> ref(T& t) noexcept;
}

std::reference_wrapper<const int> treatForwardingRefAsLifetimeConst() {
  const int &b = int(6);
  return std::ref(b); 
}
```

The result for me is:
```
`-FunctionDecl 0xfa5828 <line:34:1, line:37:1> line:34:35 
treatForwardingRefAsLifetimeConst 'std::reference_wrapper<const int> ()'
  `-CompoundStmt 0xfa7848 <col:71, line:37:1>
    |-DeclStmt 0xfa5e58 <line:35:3, col:24>
    | `-VarDecl 0xfa5cf0 <col:3, col:23> col:14 used b 'const int &' cinit
    |   `-ExprWithCleanups 0xfa5de8 <col:18, col:23> 'const int' lvalue
    |     `-MaterializeTemporaryExpr 0xfa5db8 <col:18, col:23> 'const int' 
lvalue extended by Var 0xfa5cf0 'b' 'const int &'
    |       `-CXXFunctionalCastExpr 0xfa5d90 <col:18, col:23> 'int' functional 
cast to int <NoOp>
    |         `-IntegerLiteral 0xfa5d70 <col:22> 'int' 6
    `-ReturnStmt 0xfa7838 <line:36:3, col:20>
      `-ExprWithCleanups 0xfa7820 <col:10, col:20> 
'std::reference_wrapper<const int>':'std::reference_wrapper<const int>'
        `-CXXConstructExpr 0xfa77f0 <col:10, col:20> 
'std::reference_wrapper<const int>':'std::reference_wrapper<const int>' 'void 
(std::reference_wrapper<const int> &&) noexcept' elidable
          `-MaterializeTemporaryExpr 0xfa7798 <col:10, col:20> 
'reference_wrapper<const int>':'std::reference_wrapper<const int>' xvalue
            `-CallExpr 0xfa7300 <col:10, col:20> 'reference_wrapper<const 
int>':'std::reference_wrapper<const int>'
              |-ImplicitCastExpr 0xfa72e8 <col:10, col:15> 
'reference_wrapper<const int> (*)(const int &) noexcept' 
<FunctionToPointerDecay>
              | `-DeclRefExpr 0xfa7250 <col:10, col:15> 
'reference_wrapper<const int> (const int &) noexcept' lvalue Function 0xfa7150 
'ref' 'reference_wrapper<const int> (const int &) noexcept' (FunctionTemplate 
0xfa5500 'ref')
              `-DeclRefExpr 0xfa5ed8 <col:19> 'const int' lvalue Var 0xfa5cf0 
'b' 'const int &'
```

Please let me know if you had a different example in mind.

(Also, this unfortunately is a false negative example for now, as we tend to 
start with the less risky warnings :))


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66486/new/

https://reviews.llvm.org/D66486



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to