xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
The address-of part looks good to me. However, just realized that I'm not
convinced whether the dereference operator, or references, in general, are
correctly handled.
================
Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:190
Env.setValue(Loc, Env.takeOwnership(std::make_unique<ReferenceValue>(
SubExprVal->getPointeeLoc())));
+ break;
----------------
I know this is not strictly related to this patch, but why do we actually need
do `getPointeeLoc` here? I'd expect `UO_Deref` to be a noop for all intents and
purposes.
E.g. for a snippet like:
```
int f(int *p) {
return *p;
}
```
The AST looks like:
```
`-CompoundStmt 0x5565d151d038 <col:15, line:4:1>
`-ReturnStmt 0x5565d151d028 <line:3:5, col:13>
`-ImplicitCastExpr 0x5565d151d010 <col:12, col:13> 'int' <LValueToRValue>
`-UnaryOperator 0x5565d151cff8 <col:12, col:13> 'int' lvalue prefix '*'
cannot overflow
`-ImplicitCastExpr 0x5565d151cfe0 <col:13> 'int *' <LValueToRValue>
`-DeclRefExpr 0x5565d151cfc0 <col:13> 'int *' lvalue ParmVar
0x5565d151ce00 'p' 'int *'
```
I'd expect any actual dereference to happen in the `LValueToRValue` cast.
The reason is, this is how references can be handled. E.g.:
```
void f(int &p) {
int &q = p;
int r = p;
}
```
Has the AST:
```
|-DeclStmt 0x55d49a1f00b0 <line:3:5, col:15>
| `-VarDecl 0x55d49a1effd0 <col:5, col:14> col:10 q 'int &' cinit
| `-DeclRefExpr 0x55d49a1f0038 <col:14> 'int' lvalue ParmVar
0x55d49a1efe00 'p' 'int &'
`-DeclStmt 0x55d49a1f0180 <line:4:5, col:14>
`-VarDecl 0x55d49a1f00e0 <col:5, col:13> col:9 r 'int' cinit
`-ImplicitCastExpr 0x55d49a1f0168 <col:13> 'int' <LValueToRValue>
`-DeclRefExpr 0x55d49a1f0148 <col:13> 'int' lvalue ParmVar
0x55d49a1efe00 'p' 'int &'
```
The reason why you know when should you propagate the reference or the pointee
of the reference from the subexpression is because of the `LValueToRValue`
cast. So you already need to do the "dereference" operator there to correctly
handle references. And if you already do that, I see no reason to duplicate
this logic here for pointers. Do I miss something?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D117496/new/
https://reviews.llvm.org/D117496
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
