[PATCH] D158433: [Clang] Do not change the type of captured vars when checking lambda constraints

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I agree with this approach, and think we should backport this if at all possible.  The original solution is a bit problematic, and I don't believe this solution is particularly risky.



================
Comment at: clang/lib/Sema/SemaConcept.cpp:727
+  } else
+    FuncScope.disable();
+
----------------
curleys required here.


================
Comment at: clang/lib/Sema/SemaConcept.cpp:922
+  } else
     FuncScope.disable();
 
----------------
here too


================
Comment at: clang/lib/Sema/SemaExpr.cpp:19754
+
+    // When evaluating some attributes (like enable_if) we might refer to a
+    // function parameter appertaining to the same declaration as that
----------------
cor3ntin wrote:
> shafik wrote:
> > Why move this block of code for?
> `isVariableAlreadyCapturedInScopeInfo` is kinda badly name because it will adjust `DeclRefType` by adding `const` as necessary.
> 
> if we capture a parameter, and then use it in a concept - which are not checked from within the scope of the lambda, we need to add const to it, which we can only do by reordering these paths. It's a bit subtle, and could do with some improvement as I'm not sure parameters will always be const correct in attributes currently. 
> But I tried to do a minimal fix
> 
> 
> 
> 
Yeah, this function is unfortunately doing a lot of work and is a bit of a mess unfortunately.  I think I see the logic of the reorder, but its a sensitive enough function that a minimal fix is prudent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158433