[PATCH] D14905: [constexpr-lambda] Support parsing of constexpr specifier (and its inference) on lambda expressions

[Richard Smith via cfe-commits]

rsmith added inline comments.

================
Comment at: include/clang/Basic/DiagnosticParseKinds.td:772
@@ +771,3 @@
+def warn_cxx14_compat_constexpr_on_lambda : Warning<
+  "constexpr on lambda expressions are incompatible with C++14">,
+  InGroup<CXX14Compat>, DefaultIgnore;
----------------
are -> is

================
Comment at: include/clang/Basic/DiagnosticParseKinds.td:773
@@ +772,3 @@
+  "constexpr on lambda expressions are incompatible with C++14">,
+  InGroup<CXX14Compat>, DefaultIgnore;
+def err_constexpr_on_lambda : Error<
----------------
Should be CXXPre1zCompat, it's also incompatible with C++11.

================
Comment at: include/clang/Basic/DiagnosticParseKinds.td:775
@@ +774,3 @@
+def err_constexpr_on_lambda : Error<
+  "constexpr on lambda expressions is permitted only in C++1z">;
+
----------------
Any reason not to allow this as an extension in earlier standards?

================
Comment at: lib/Parse/ParseExprCXX.cpp:1049
@@ +1048,3 @@
+                                  SourceLocation &DeclEndLoc, DeclSpec &DS) {
+  if (P.TryConsumeToken(tok::kw_mutable, MutableLoc)) {
+    if (P.TryConsumeToken(tok::kw_constexpr, ConstexprLoc))
----------------
This doesn't look like it'll handle duplicates very well. We should consume and diagnose them, and recover as if they weren't there.

================
Comment at: lib/Parse/ParseExprCXX.cpp:1061
@@ +1060,3 @@
+
+  if (ConstexprLoc.isValid()) {
+    if (!P.getLangOpts().CPlusPlus1z) {
----------------
Move this into the caller. It's inconsistent (and doesn't match the function name) to handle this but not 'mutable' here.

================
Comment at: lib/Sema/SemaLambda.cpp:1599
@@ +1598,3 @@
+      !Class->getDeclContext()->isDependentContext()) {
+    TentativeAnalysisScope DiagnosticScopeGuard(*this);
+    CallOperator->setConstexpr(
----------------
Ah, I now see why you were asking about this... :)

I would prefer a Diagnose flag here, but we can make that change later.

================
Comment at: test/Parser/cxx1z-constexpr-lambdas.cpp:5
@@ +4,3 @@
+auto L0 = [] constexpr { }; //expected-error{{requires '()'}} expected-error{{expected body}}
+#ifdef CPP1Z
+
----------------
Maybe test __cplusplus > 201402L rather than passing a separate macro?

================
Comment at: test/SemaCXX/cxx1z-constexpr-lambdas.cpp:4
@@ +3,3 @@
+// RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks -fms-extensions %s -DMS_EXTENSIONS
+// RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks -fdelayed-template-parsing -fms-extensions %s -DMS_EXTENSIONS -DDELAYED_TEMPLATE_PARSING
+
----------------
You're not using these -D flags for anything; remove them.


http://reviews.llvm.org/D14905