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

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.


================
Comment at: clang/lib/Sema/Scope.cpp:72
+  // Lambdas have an extra prototype scope that doesn't add any depth
+  if (flags & FunctionPrototypeScope && !(flags & LambdaScope))
+    PrototypeDepth++;
----------------
ChuanqiXu wrote:
> cor3ntin wrote:
> > ChuanqiXu wrote:
> > > ChuanqiXu wrote:
> > > > This looks like:
> > > This isn't addressed. I mean the flags would always be true if the last clause get evaluated.
> > I'm not sure I understand your point. We are checking that it's a function scope that is not a lambda scope.
> > They are probably other ways to write that but they aren't clearer
> I mean the suggested change is equal to the original one and it is shorter. I don't think it would lost any information since they are all in the same line.
`LambdaScope` is a constant here
Both `FunctionPrototypeScope` and `FunctionPrototypeScope | LambdaScope` are valid values, and we only want the condition to be true when `flags & FunctionPrototypeScope` is true and `flags & LambdaScope` is false.
Maybe we misunderstand each other


================
Comment at: clang/lib/Sema/SemaLambda.cpp:892
 
-void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro,
-                                        Declarator &ParamInfo,
-                                        Scope *CurScope) {
+static LambdaScopeInfo *getCurrentLambdaScopeUnsafe(Sema &S) {
+  assert(!S.FunctionScopes.empty());
----------------
ChuanqiXu wrote:
> I guess it is needed to explain the meaning of `Unsafe`
Agreed, I added a comment


================
Comment at: clang/lib/Sema/SemaLambda.cpp:1194
+  unsigned I = 0;
+  SourceLocation PrevCaptureLoc;
+  for (auto C = Intro.Captures.begin(), E = Intro.Captures.end(); C != E;
----------------
ChuanqiXu wrote:
> It looks like we could define `PrevCaptureLoc` in the loop.
I think that would be worse, as this is the location of the previous capture.
it would looks something like.
`SourceLocation PrevCaptureLoc = C == Intro.Captures.begin() ? {} : (C-1)->Loc`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119136