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

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 8 10:56:19 PDT 2022

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.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?

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list