[PATCH] D119136: [clang] Implement Change scope of lambda trailing-return-type

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Some drive by comments, I plan to do a more thorough review when I have the chance.



================
Comment at: clang/docs/ReleaseNotes.rst:97
   and `51641 <https://github.com/llvm/llvm-project/issues/51641>`_.
-- The builtin function __builtin_dump_struct would crash clang when the target 
+- The builtin function __builtin_dump_struct would crash clang when the target
   struct contains a bitfield. It now correctly handles bitfields.
----------------
Unintended whitespace change?


================
Comment at: clang/docs/ReleaseNotes.rst:217
 - Implemented `P2242R3: Non-literal variables (and labels and gotos) in constexpr functions	<https://wg21.link/P2242R3>`_.
+- implemented `P2036R3: Change scope of lambda trailing-return-type <https://wg21.link/P2036R3>`_.
+  This proposal modifies how variables captured in lambdas can appear in trailing return type
----------------



================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7702
+  def err_lambda_used_before_capture: Error<
+    "captured variable %0 cannot appear before the end of the lambda's parameter list">;
   def note_lambda_variable_capture_fixit : Note<
----------------



================
Comment at: clang/include/clang/Sema/Scope.h:47-51
+    FnScope = 0x01,
 
     /// This is a while, do, switch, for, etc that can have break
     /// statements embedded into it.
+    BreakScope = 0x02,
----------------
Unintended whitespace changes?


================
Comment at: clang/include/clang/Sema/Scope.h:145-147
+    /// Lambdas need 2 FunctionPrototypeScope scopes ( because there is a
+    /// template scope in between),
+    /// the outer scope does not increase the depth of recursion.
----------------



================
Comment at: clang/include/clang/Sema/ScopeInfo.h:840
+
+  /// Holds the captures until we parsed the qualifiers,
+  /// as the cv qualified type of capture can only be computed at that point,
----------------
Can you re-flow the comments to 80 cols?


================
Comment at: clang/lib/Parse/ParseExprCXX.cpp:1259
   Actions.PushLambdaScope();
+  Actions.ActOnLambdaIntroducer(Intro, getCurScope());
 
----------------
This call surprises me here -- I would expect to see this called from `ParseLambdaIntroducer()` so that anyone who parses a lambda introducer gets the action.


================
Comment at: clang/lib/Sema/Scope.cpp:71
   // If this is a prototype scope, record that.
-  if (flags & FunctionPrototypeScope) PrototypeDepth++;
+  // Lambdas have an extra prototype scope that doesn't add any depth
+  if (flags & FunctionPrototypeScope && !(flags & LambdaScope))
----------------
Should re-flow comments with the above line.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:14352
   DeclarationNameInfo DNI = CallOperator->getNameInfo();
-
   LSI->IntroducerRange = DNI.getCXXOperatorNameRange();
----------------
Unintended whitespace change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119136