cor3ntin added inline comments.
================
Comment at: clang/lib/Analysis/AnalysisDeclContext.cpp:173
+ ValueDecl *VD = LC.getCapturedVar();
+ if (isSelfDecl(dyn_cast<VarDecl>(VD)))
return dyn_cast<ImplicitParamDecl>(VD);
----------------
erichkeane wrote:
> aaron.ballman wrote:
> > This looks dangerous -- `isSelfDecl()` uses the pointer variable in ways
> > that would be Bad News for null pointers.
> Yep, `isSelfDecl` seems to do:
>
> `return isa<ImplicitParamDecl>(VD) && VD->getName() == "self";`
>
> `isa` isn't nullptr safe, we have `isa_and_nonnull` for that (if we want to
> update `isSelfDecl`).
Using `isa_and_nonnull` looks like the best solution here
================
Comment at: clang/lib/Sema/SemaDecl.cpp:14594-14596
+ ValueDecl *VD = C.getCapturedVar();
+ if (VarDecl *Var = dyn_cast<VarDecl>(VD)) {
+ if (Var->isInitCapture())
----------------
erichkeane wrote:
>
I'm not sure I understood your suggestion.
The reason there is a local `ValueDecl` in addition of the `VarDecl` is because
we use it Line 14660 just below.
But that was reworked when lifting isInitCapture in ValueDecl anyway.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:16393
- VarDecl *Var = Cap.getVariable();
+ VarDecl *Var = cast<VarDecl>(Cap.getVariable());
Expr *CopyExpr = nullptr;
----------------
aaron.ballman wrote:
> Is `cast<>` safe here?
Yes, I added a comment to make that clear
================
Comment at: clang/lib/Sema/SemaExpr.cpp:18324
+
+ assert((isa<VarDecl>(Var) || isa<BindingDecl>(Var)) &&
+ "Only variables and structured bindings can be captured");
----------------
aaron.ballman wrote:
>
Fancy. The parentheses are still needed though, because C macros are amazing
like that :)
================
Comment at: clang/lib/Sema/SemaExpr.cpp:18531
+ } else if ((BD = dyn_cast<BindingDecl>(Var))) {
+ ME = dyn_cast_or_null<MemberExpr>(BD->getBinding());
+ }
----------------
aaron.ballman wrote:
> Can this return nullptr?
I think so
```
/// Get the expression to which this declaration is bound. This may be null
/// in two different cases: while parsing the initializer for the
/// decomposition declaration, and when the initializer is type-dependent.
Expr *getBinding() const { return Binding; }
```
The initializer could be type dependant here, and so it could be null
================
Comment at: clang/lib/Sema/SemaExpr.cpp:18539
+ !S.isOpenMPCapturedDecl(Var))) {
+ FieldDecl *FD = dyn_cast_or_null<FieldDecl>(ME->getMemberDecl());
+ if (FD &&
----------------
aaron.ballman wrote:
> Can this return nullptr?
Yes, same thing as above
================
Comment at: clang/lib/Sema/SemaLambda.cpp:1208-1209
+ if (isa<BindingDecl>(Var)) {
+ Underlying = dyn_cast_or_null<VarDecl>(
+ cast<BindingDecl>(Var)->getDecomposedDecl());
+ } else
----------------
aaron.ballman wrote:
> Does `getDecomposedDecl()` ever actually return nullptr?
I don't think that's necessary indeed, nice catch
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122768/new/
https://reviews.llvm.org/D122768
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
