[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: lib/AST/ExprConstant.cpp:2050
@@ +2049,3 @@
+    if (!Result && isLambdaCallOperator(Frame->Callee) &&
+        VD->getDeclContext() != Frame->Callee) {
+      // Assume variables referenced within a lambda's call operator that were
----------------
What's the DeclContext of a variable we synthesize to represent an init-capture?

================
Comment at: lib/Parse/ParseExprCXX.cpp:1045
@@ -1044,1 +1044,3 @@
 
+static inline void
+tryConsumeMutableOrConstexprToken(Parser &P, SourceLocation &MutableLoc,
----------------
No need to mark this `inline`.

================
Comment at: lib/Parse/ParseExprCXX.cpp:1051
@@ +1050,3 @@
+  assert(ConstexprLoc.isInvalid());
+  // consume constexpr-opt mutable-opt in any sequence, and set the DeclEndLoc
+  // to the final of those locations. Emit an error if we have multiple
----------------
consume -> Consume

================
Comment at: lib/Parse/ParseExprCXX.cpp:1056-1058
@@ +1055,5 @@
+  
+  // If we see multiple decl specifiers, emit a diagnostic once.
+  enum { MUTABLE_IDX = 0, CONSTEXPR_IDX = 1 };
+  bool EmittedMultipleDeclSpecDiag[] = { false, false };
+
----------------
Any reason to have an array here rather than two separate variables?

================
Comment at: lib/Parse/ParseExprCXX.cpp:1066
@@ +1065,3 @@
+               diag::err_lambda_declspecifier_repeated)
+            << 0;
+        EmittedMultipleDeclSpecDiag[MUTABLE_IDX] = true;
----------------
Please add a `FixItHint::CreateRemoval(P.getCurToken().getLocation())` here.

================
Comment at: lib/Parse/ParseExprCXX.cpp:1067
@@ +1066,3 @@
+            << 0;
+        EmittedMultipleDeclSpecDiag[MUTABLE_IDX] = true;
+      }
----------------
Why suppress the diagnostic in this case? If there are three `mutable` specifiers, I'd like two diagnostics and two FixItHints removing the extra ones. That's what we do for duplicated specifiers in other contexts.

================
Comment at: lib/Parse/ParseExprCXX.cpp:1085
@@ +1084,3 @@
+    default:
+      DoneCheckingForMutableOrConstexprSeq = true;
+    }
----------------
Please spell this as `return;` instead :)

================
Comment at: lib/Parse/ParseExprCXX.cpp:1090
@@ +1089,3 @@
+
+static inline void
+addConstexprToLambdaDeclSpecifier(Parser &P, SourceLocation ConstexprLoc,
----------------
No need to mark this `inline`.


http://reviews.llvm.org/D14905