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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
