sammccall accepted this revision.
sammccall added a comment.
Still LG once you're happy
================
Comment at: clang/include/clang/Sema/Sema.h:3026
+ void ActOnParamDefaultArgumentError(Decl *param, SourceLocation EqualLoc,
+ ExprResult DefaultArg);
ExprResult ConvertParamDefaultArgument(ParmVarDecl *Param, Expr *DefaultArg,
----------------
kadircet wrote:
> sammccall wrote:
> > nit: Nullable `ExprResult*` since there are only two states?
> > Extra get() in some callers, but less mysterious
> i guess you meant `Expr*` ?
Yes, sorry!
================
Comment at: clang/lib/Parse/ParseCXXInlineMethods.cpp:400
if (DefArgResult.isInvalid()) {
- Actions.ActOnParamDefaultArgumentError(Param, EqualLoc);
+ Actions.ActOnParamDefaultArgumentError(Param, EqualLoc, DefArgResult);
} else {
----------------
kadircet wrote:
> sammccall wrote:
> > DefArgResult is never anything here. I'd probably find
> > `/*DefaultArg=*/nullptr` more obvious
> maybe i am missing something, but "invalid" doesn't mean "unusable".
ActionResult can be pointer-and-valid, null-and-valid, or null-and-invalid.
So invalid does indeed mean unusable ("usable" means pointer-and-valid).
Its documentation strongly suggests an ActionResult can be pointer-and-invalid,
but good luck constructing one :-)
================
Comment at: clang/test/AST/ast-dump-default-arg-recovery.cpp:6
+// CHECK: -ParmVarDecl {{.*}} <col:10, col:24> col:14 invalid arg 'int'
cinit
+// CHECK-NEXT: -RecoveryExpr {{.*}} <col:18, col:24> 'int' contains-errors
----------------
Can you also check the contents (there should be a declrefexpr at least)?
Preserving this internal structure is important for clangd, the recoveryexpr
alone doesn't help much.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157868/new/
https://reviews.llvm.org/D157868
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
