[PATCH] D82284: [AST][RecoveryAST] Preseve invalid return stmt, and suppress the diagnostics for missing return stmt in constexpr func.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

sorry for the long delay, picking it up.



================
Comment at: clang/test/SemaCXX/constant-expression-cxx11.cpp:1798
+  constexpr int error() { // expected-error {{constexpr function never produces a constant expression}}
+    return foobar; // expected-error {{undeclared identifier}} expected-note 2{{subexpression not valid in a constant expression}}
   }
----------------
sammccall wrote:
> an idea (not sure if feasible) - do we want to suppress "not valid in constant expression" errors when the expressions contains errors?
> If so, may want a (different) FIXME here
the extra diagnostics don't exit any more in the trunk (I think it is because we support the error-dependent expression in const evaluator).


================
Comment at: clang/test/SemaCXX/lambda-expressions.cpp:522
       return undeclared_error; // expected-error {{use of undeclared identifier}}
-    return 0;
+    return 0; // expected-error {{cannot initialize return object of type 'auto' with an rvalue of type 'int'}}
   };
----------------
sammccall wrote:
> this seems like a regression - what exactly is going on here?
> 
> in a perfect world I guess we'd e.g. leave the type undeduced from the first return, rather than deduce it as undeduced-auto (at least for the purposes of diagnostics).
this is an exiting issue in clang, and our tests don't cover that. https://reviews.llvm.org/D92211 will fix that.


================
Comment at: clang/test/SemaCXX/typo-correction-crash.cpp:19
 using T1 = decltype(L1());
-static_assert(is_same<T1, void>::value, "Return statement should be discarded");
+// FIXME: Suppress the 'undeclared identifier T1' diagnostic, the UsingDecl T1 is discarded because of an invalid L1().
+static_assert(is_same<T1, void>::value, "Return statement should be discarded"); // expected-error {{use of undeclared identifier 'T1'}}
----------------
sammccall wrote:
> this is also a regression, though this code is pretty broken and is really just asserting no crash.
this seems non-trivial to fix (we have a too strong invalidation for `decltype` on an error expression, the declarator is marked invalid), I'd defer the fix (since this is not a critical case, and the code is pretty broken). 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82284