sammccall added inline comments.
================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5258
// Peel off the ParenListExpr by chosing the last one, as they don't have a
// predefined type.
----------------
I think we should merge the two comments, something like...
```
/// If LHS is ParenListExpr, assume a chain of comma operators and pick the
last expr.
/// We expect other ParenListExprs to be resolved to e.g. constructor calls
before here.
/// (So the ParenListExpr should be nonempty, but check just in case)
```
================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5260
// predefined type.
- if (auto *PLE = llvm::dyn_cast<ParenListExpr>(Base))
+ if (auto *PLE = llvm::dyn_cast<ParenListExpr>(Base)) {
+ // FIXME: Get rid of this check once we are sure the ParenListExpr in here
----------------
now this is getting a little more complicated, we could pull out a function
Expr* unwrapParenList(Expr*) and avoid duplicating code & comments, but up to
you.
================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5261
+ if (auto *PLE = llvm::dyn_cast<ParenListExpr>(Base)) {
+ // FIXME: Get rid of this check once we are sure the ParenListExpr in here
+ // cannot be empty. This is assumed to ultimately be a chain of comma
----------------
I'm not actually sure we should fix this. Nonempty is a subtle invariant that
could also be violated by e.g. broken code. And the check is cheap.
================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5272
+ if (PLE->getNumExprs() == 0)
+ return;
OtherOpBase = PLE->getExpr(PLE->getNumExprs() - 1);
----------------
I think you want OtherOpBase = null here, not return
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D96950/new/
https://reviews.llvm.org/D96950
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
