ChuanqiXu accepted this revision.
ChuanqiXu added a comment.
This revision is now accepted and ready to land.

I've tried my best to review this one and it looks good to me basically. Since 
@aaron.ballman plans to review this one. I think it'd better to wait for this 
respond.



================
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++;
----------------
cor3ntin wrote:
> 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
Oh, sorry, I misread the code. The code now should be correct.


================
Comment at: clang/lib/Sema/SemaLambda.cpp:1127-1132
+      auto it = LSI->DelayedCaptures.end();
+      if (Var)
+        it = llvm::find_if(LSI->DelayedCaptures, [&Var](auto &&Pair) {
+          return Pair.second.Var == Var;
+        });
+      if (it != LSI->DelayedCaptures.end()) {
----------------
I feel like my suggested change is simpler.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119136

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to