[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/AST/DeclCXX.h:1834-1846
+  void setLambdaTypeInfo(TypeSourceInfo *TS) {
+    auto *DD = DefinitionData;
+    assert(DD && DD->IsLambda && "setting lambda property of non-lambda class");
+    auto &DL = static_cast<LambdaDefinitionData &>(*DD);
+    DL.MethodTyInfo = TS;
+  }
+
----------------
Minor simplification


================
Comment at: clang/include/clang/Sema/Scope.h:148
+    /// This is the scope for a lambda, after the lambda introducer.
+    /// Lambdas need 2 FunctionPrototypeScope scopes (because there is a
+    /// template scope in between), the outer scope does not increase the
----------------



================
Comment at: clang/include/clang/Sema/ScopeInfo.h:854-857
   /// Whether this is a mutable lambda.
-  bool Mutable = false;
+  /// Until the mutable keyword is parsed,
+  /// we assume the lambda is mutable
+  bool Mutable = true;
----------------
Re-flow comment (I might have gotten that wrong) and add a full stop at the end of the comment.


================
Comment at: clang/include/clang/Sema/Sema.h:7115-7119
+  /// Once the Lambdas capture are known, we can
+  /// start to create the closure, call operator method,
+  /// and keep track of the captures.
+  /// We do the capture lookup here, but they are not actually captured
+  /// until after we know what the qualifiers of the call operator are.
----------------
You can re-flow this comment to 80 col as well.


================
Comment at: clang/include/clang/Sema/Sema.h:7238
+  /// Introduce the instantiated captures of the lambda into the local
+  /// instantiation scope
+  bool addInstantiatedCapturesToScope(
----------------



================
Comment at: clang/lib/Parse/ParseExprCXX.cpp:1293
   Actions.PushLambdaScope();
+  Actions.ActOnLambdaIntroducer(Intro, getCurScope());
 
----------------
Typically, we call an `ActOn` method after having parsed the construct; in this case, we're calling `ActOnLambdaIntroducer()` when it was parsed elsewhere (this is the parsing code for after the introducer). So perhaps this should be moved elsewhere or renamed?


================
Comment at: clang/lib/Parse/ParseExprCXX.cpp:1381
+  SourceLocation MutableLoc;
+  LateParsedAttrList LateParsedAttrs(true);
+
----------------
This isn't being used?


================
Comment at: clang/lib/Parse/ParseExprCXX.cpp:1387-1393
+    // However, because GNU attributes could refer to captured variables,
+    // which only become visible after the mutable keyword is parsed
+    // we delay the parsing of gnu attributes - by reusing the mechanism used
+    // for C++ late method parsing. Note, __declspec attributes do not make
+    // use of late parsing (expressions cannot appear in __declspec arguments),
+    // so only GNU style attributes are affected here.
+    MaybeParseAttributes(PAKM_GNU | PAKM_Declspec, Attributes);
----------------
The comment doesn't seem to match the code -- this isn't parsing into the late parsed attribute list?


================
Comment at: clang/lib/Parse/ParseExprCXX.cpp:1521-1523
+  if (HasParentheses || HasSpecifiers) {
+    ParseConstexprAndMutableSpecifiers();
   }
----------------



================
Comment at: clang/lib/Parse/ParseExprCXX.cpp:1527-1529
+  if (!HasParentheses) {
+    Actions.ActOnLambdaClosureQualifiers(Intro, MutableLoc);
+  }
----------------



================
Comment at: clang/lib/Sema/SemaConcept.cpp:503-505
+  const CXXRecordDecl *LambdaClass = cast<CXXMethodDecl>(Function)->getParent();
+  const CXXRecordDecl *LambdaPattern =
+      cast<CXXMethodDecl>(PatternDecl)->getParent();
----------------



================
Comment at: clang/lib/Sema/SemaConcept.cpp:508-509
+  unsigned Instantiated = 0;
+  for (unsigned I = 0; I < LambdaPattern->capture_size(); I++) {
+    const LambdaCapture *CapturePattern = LambdaPattern->getCapture(I);
+    if (!CapturePattern->capturesVariable()) {
----------------



================
Comment at: clang/lib/Sema/SemaConcept.cpp:584-588
+      if (isLambdaCallOperator(FD)) {
+        if (addInstantiatedCapturesToScope(FD, FromMemTempl->getTemplatedDecl(),
+                                           Scope, MLTAL))
+          return true;
+      }
----------------



================
Comment at: clang/lib/Sema/SemaConcept.cpp:615-618
+    if (isLambdaCallOperator(FD)) {
+      if (addInstantiatedCapturesToScope(FD, InstantiatedFrom, Scope, MLTAL))
+        return true;
+    }
----------------



================
Comment at: clang/lib/Sema/SemaLambda.cpp:449-452
+    if (!LSI->ReturnType->isDependentType() && !LSI->ReturnType->isVoidType()) {
+      S.RequireCompleteType(CallOperator->getBeginLoc(), LSI->ReturnType,
+                            diag::err_lambda_incomplete_result);
+    }
----------------



================
Comment at: clang/lib/Sema/SemaLambda.cpp:875-877
     if (!FTI.hasMutableQualifier() && !IsLambdaStatic) {
-      FTI.getOrCreateMethodQualifiers().SetTypeQual(DeclSpec::TQ_const,
-                                                    SourceLocation());
+      FTI.getOrCreateMethodQualifiers().SetTypeQual(DeclSpec::TQ_const, Loc);
     }
----------------



================
Comment at: clang/lib/Sema/SemaLambda.cpp:891-893
+    MethodTyInfo = S.GetTypeForDeclarator(ParamInfo, CurScope);
+
+    assert(MethodTyInfo && "no type from lambda-declarator");
----------------



================
Comment at: clang/lib/Sema/SemaLambda.cpp:933
 
-  // Attributes on the lambda apply to the method.
-  ProcessDeclAttributes(CurScope, Method, ParamInfo);
+  LambdaScopeInfo *const LSI = getCurrentLambdaScopeUnsafe(*this);
 
----------------



================
Comment at: clang/lib/Sema/SemaLambda.cpp:941
+
+  auto DC = Method->getLexicalDeclContext();
+  Method->setLexicalDeclContext(LSI->Lambda);
----------------



================
Comment at: clang/lib/Sema/SemaLambda.cpp:944
+  if (TemplateParams) {
+    FunctionTemplateDecl *const TemplateMethod = FunctionTemplateDecl::Create(
+        Context, LSI->Lambda, Method->getLocation(), Method->getDeclName(),
----------------



================
Comment at: clang/lib/Sema/SemaLambda.cpp:995
+  if (LSI->NumExplicitTemplateParams > 0) {
+    auto *TemplateParamScope = CurScope->getTemplateParamParent();
+    assert(TemplateParamScope &&
----------------
Spell out the type


================
Comment at: clang/lib/Sema/SemaLambda.cpp:1244
 
-  // Add lambda parameters into scope.
-  addLambdaParameters(Intro.Captures, Method, CurScope);
+  LambdaScopeInfo *const LSI = getCurrentLambdaScopeUnsafe(*this);
+  LSI->Mutable = MutableLoc.isValid();
----------------



================
Comment at: clang/lib/Sema/SemaLambda.cpp:1286
+
+  LambdaScopeInfo *const LSI = getCurrentLambdaScopeUnsafe(*this);
+  LSI->CallOperator->setConstexprKind(DS.getConstexprSpecifier());
----------------



================
Comment at: clang/lib/Sema/SemaLambda.cpp:1296
+  } else {
+    unsigned index;
+    ParamInfo.isFunctionDeclarator(index);
----------------



================
Comment at: clang/lib/Sema/TreeTransform.h:13382-13383
+    TypeSourceInfo *OldCallOpTSI = E->getCallOperator()->getTypeSourceInfo();
+    FunctionProtoTypeLoc OldCallOpFPTL =
+        OldCallOpTSI->getTypeLoc().getAs<FunctionProtoTypeLoc>();
+
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124351