[PATCH] D157868: [clang] Use RecoveryExprs for broken defaultargs, instead of OpaqueValueExprs

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Nice, recoveryexpr is a better fit here. These changes tend to cause occasional new error-handling crashes on under-tested paths, but I guess it's a good time in the release cycle for that!

You might want a clangd or ast-dump test showing that we now preserve the internal structure of (some) invalid default initializes.



================
Comment at: clang/include/clang/Sema/Sema.h:3026
+  void ActOnParamDefaultArgumentError(Decl *param, SourceLocation EqualLoc,
+                                      ExprResult DefaultArg);
   ExprResult ConvertParamDefaultArgument(ParmVarDecl *Param, Expr *DefaultArg,
----------------
nit: Nullable `ExprResult*` since there are only two states?
Extra get() in some callers, but less mysterious


================
Comment at: clang/lib/Parse/ParseCXXInlineMethods.cpp:398
         DefArgResult = ParseAssignmentExpression();
-      DefArgResult = Actions.CorrectDelayedTyposInExpr(DefArgResult);
+      DefArgResult = Actions.CorrectDelayedTyposInExpr(DefArgResult, Param);
       if (DefArgResult.isInvalid()) {
----------------
If this fixes the recursive copy-constructor case you mentioned, can you add a test?

(Else does it do anything? Or just cleanup)


================
Comment at: clang/lib/Parse/ParseCXXInlineMethods.cpp:400
       if (DefArgResult.isInvalid()) {
-        Actions.ActOnParamDefaultArgumentError(Param, EqualLoc);
+        Actions.ActOnParamDefaultArgumentError(Param, EqualLoc, DefArgResult);
       } else {
----------------
DefArgResult is never anything here. I'd probably find `/*DefaultArg=*/nullptr` more obvious


================
Comment at: clang/lib/Parse/ParseDecl.cpp:7478
+            Actions.ActOnParamDefaultArgumentError(Param, EqualLoc,
+                                                   DefArgResult);
             SkipUntil(tok::comma, tok::r_paren, StopAtSemi | StopBeforeMatch);
----------------
Ditto


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157868