[PATCH] Generic Lambdas: A first step

Doug Gregor dgregor at apple.com
Mon Sep 2 21:33:49 PDT 2013


  A few comments above, and we obviously have some more work to do, but this is going in the right direction. One general comment is that I'd like us to limit the C++1y checks as much as possible: the language dialect should be checked to drive diagnostics, construct the AST appropriately to get the effect we need, or when there is a behavioral divergence between two dialects. However, we'd like the AST to speak for itself as much as possible so that future dialect checks aren't needed. This helps us exercise more code paths, and let's us easily make policy decisions like "we'll only warn about generic lambdas in C++11 mode, not error" without having to change a lot of code.


================
Comment at: include/clang/Sema/ScopeInfo.h:629
@@ +628,3 @@
+  SmallVector<TemplateTypeParmDecl*, 4> AutoTemplateParams;
+
+
----------------
Please put the \brief at the top, and use /// for the rest of the comment (so it shows up in Doxygen).



================
Comment at: include/clang/Sema/Sema.h:977
@@ +976,3 @@
+  // when parsing generic lambda 'auto' parameters.
+  void RecordParsingTemplateParameterDepth(unsigned Depth);
+  
----------------
/// for Doxygen comments, please.

================
Comment at: include/clang/Sema/SemaLambda.h:32
@@ +31,3 @@
+      return LambdaClass->getLambdaCallOperator() 
+                  == MD->getTemplateInstantiationPattern();
+    }
----------------
Wouldn't getTemplateSpecializationKind() == TSK_ImplicitInstantiation suffice, rather than hunting down the pattern?

================
Comment at: lib/Parse/ParseExprCXX.cpp:943
@@ -935,1 +942,3 @@
+      if (getLangOpts().CPlusPlus1y)
+        Actions.RecordParsingTemplateParameterDepth(TemplateParameterDepth);
       ParseParameterDeclarationClause(D, Attr, ParamInfo, EllipsisLoc);
----------------
I'd rather that we do this semantic action all the time, rather than just under C++1y mode. It means we get more consistent behavior, and lets us recover better if someone tries to use generic lambdas in < C++1y mode.

================
Comment at: lib/Parse/ParseExprCXX.cpp:947
@@ +946,3 @@
+      // clause creates a template type parameter, so increment the depth.
+      if (getLangOpts().CPlusPlus1y && Actions.getCurGenericLambda()) 
+        ++CurTemplateDepthTracker;
----------------
This should just check Actions.getCurGenericLambda(). The C++1y check is redundant.

================
Comment at: lib/Sema/SemaDecl.cpp:9034
@@ +9033,3 @@
+  QualType ParamType = New->getType();
+  if (getLangOpts().CPlusPlus1y && ParamType->getContainedAutoType()) { 
+    assert(getCurLambda() && 
----------------
Shouldn't need the C++1y check here, either.

================
Comment at: lib/Sema/SemaDecl.cpp:9296
@@ +9295,3 @@
+  // template version, to prime the current LambdaScopeInfo. 
+  if (getLangOpts().CPlusPlus1y 
+                        && isGenericLambdaCallOperatorSpecialization(D)) {
----------------
Unnecessary C++1y check.

================
Comment at: include/clang/Sema/ScopeInfo.h:627
@@ +626,3 @@
+  // parameter list, during initial AST construction.
+  /// \brief Store the list of the auto parameters for a generic lambda.
+  SmallVector<TemplateTypeParmDecl*, 4> AutoTemplateParams;
----------------
Please put the \brief first.

================
Comment at: lib/Sema/Sema.cpp:1019
@@ +1018,3 @@
+  } 
+  assert(false && 
+      "Remove assertion if intentionally called in a non-lambda context.");
----------------
This should be an llvm_unreachable.

================
Comment at: lib/Sema/SemaLambda.cpp:160
@@ +159,3 @@
+static inline TemplateParameterList *
+              getGenericLambdaTemplateParameterList(LambdaScopeInfo *LSI, 
+                            Sema &SemaRef) {
----------------
The formatting here is odd. Please slide the function name all the way left.

================
Comment at: lib/Sema/SemaLambda.cpp:624
@@ -544,2 +623,3 @@
     EPI.TypeQuals |= DeclSpec::TQ_const;
-    QualType MethodTy = Context.getFunctionType(Context.DependentTy, None,
+    // For C++1y, use the new return type deduction machinery, by imaginging
+    // 'auto' if no trailing return type.
----------------
Typo "imaginging".

================
Comment at: lib/Sema/SemaLambda.cpp:1088
@@ -1003,1 +1087,3 @@
+    // different machinery currently.
+    // FIXME: Refactor and Merge the return type deduction machinery.
     // FIXME: Assumes current resolution to core issue 975.
----------------
Ah. Is this a follow-up patch, then?


http://llvm-reviews.chandlerc.com/D1174



More information about the cfe-commits mailing list