On Sat, Dec 13, 2025 at 12:31:28PM -0500, Patrick Palka wrote:
> On Sat, 13 Dec 2025, Patrick Palka wrote:
> 
> > On Sat, 13 Dec 2025, Jason Merrill wrote:
> > 
> > > On 12/13/25 7:50 PM, Nathaniel Shead wrote:
> > > > Bootstrapped and regtested on x86_64-pc-linux-gnu (so far just dg.exp
> > > > and modules.exp), OK for trunk if full regtest succeeds?
> > > > 
> > > > -- >8 --
> > > > 
> > > > When evaluating a concept definition in a template, any lambdas in the
> > > > definition of the concept get instantiated in the context of where the
> > > > evaluation occurred.
> > > > 
> > > > This causes two issues:
> > > > 
> > > > - Any lambdas declared later in the body of the function get the wrong
> > > >    discriminator, which causes ABI divergences with Clang.
> > > > 
> > > > - Modules streaming gets confused, because the lambda is keyed to an
> > > >    unrelated declaration.  Keying the lambda to the concept also doesn't
> > > >    work because we'd really want to key it to a concept instantiation
> > > >    (that doesn't exist) so that merging works correctly.
> > > > 
> > > > I think really we just want to throw away these lambdas declarations
> > > > after evaluating the concept.  They can (and will) be recreated in
> > > > importers re-evaluating the concept with the given args regardless.
> > > > 
> > > > This patch implements this by disabling scope recording for an
> > > > instantiation of a lambda keyed to a concept, and ensuring that the
> > > > lambda tag is added to an unrelated block that is then thrown away.
> > > 
> > > Would it make sense to just push_to(/pop_from)_top_level in
> > > evaluate_concept_check?  This seems like another instance of the recurring
> > > problem of not pushing out of a local scope sufficiently before handling a
> > > template.
> > 
> > This is related to PR104111.  Some downsides of going this route:
> > 
> >   template<class T> requires C<T> || D<T>
> >   void f() {
> >     if constexpr (C<T>) // potentially IFNDR if evaluation of C<T>
> >                         // depends on access context of f (though
> >                         // in practice we'll just reuse the cached
> >                         // value obtained earlier during satisfaction
> >                         // with the right access context)
> >       ...
> >     else
> >       ...
> >   }
> > 
> > --
> > 
> >   template<class T> requires (!C<T>) // C<T> is not checked in
> >                                      // access context of g
> >   void g();
> > 
> > 
> > To me it seems that evaluating a concept-id in the access context
> > of where the concept-id appears is the better choice once we extend
> > the satisfaction cache to consider access context (which it currently
> > doesn't).  Doing push_to_top_level would mean the above two testcases
> > could never work "as expected" even after we fix the satisfaction cache.
> 
> Oops, perhaps you didn't mean to just do push_to_top_level.  If
> we do push_to_top_level followed by push_access_scope to restore
> the previous access context perhaps this wouldn't have an effect
> on the PR104111 testcases.
> 

Thanks for the comments.  I think that makes sense to me; so something
like the following perhaps?  

-- >8 --

When evaluating a concept definition in a template, any lambdas in the
definition of the concept get instantiated in the context of where the
evaluation occurred.

This causes two issues:

- Any lambdas declared later in the body of the function get the wrong
  discriminator, which causes ABI divergences with Clang.

- Modules streaming gets confused, because the lambda is keyed to an
  unrelated declaration.  Keying the lambda to the concept also doesn't
  work because we'd really want to key it to a concept instantiation
  (that doesn't exist) so that merging works correctly.

I think really we just want to throw away these lambdas declarations
after evaluating the concept.  They can (and will) be recreated in
importers re-evaluating the concept with the given args regardless.

This patch implements this by disabling scope recording for an
instantiation of a lambda keyed to a concept, and pushing into an
unrelated context so that the lambda's type is not mistakenly added
into the scope it was instantiated from.

        PR c++/123075

gcc/cp/ChangeLog:

        * constraint.cc (evaluate_concept_check): Push to an unrelated
        scope, but keep the same access context.
        * pt.cc (tsubst_lambda_expr): Don't record lambda scopes for
        lambdas attached to a concept.

gcc/testsuite/ChangeLog:

        * g++.dg/cpp2a/concepts-lambda25.C: New test.
        * g++.dg/modules/lambda-13.h: New test.
        * g++.dg/modules/lambda-13_a.H: New test.
        * g++.dg/modules/lambda-13_b.C: New test.

Signed-off-by: Nathaniel Shead <[email protected]>
Reviewed-by: Jason Merrill <[email protected]>
Reviewed-by: Patrick Palka <[email protected]>
---
 gcc/cp/constraint.cc                          | 20 ++++++++++++-
 gcc/cp/pt.cc                                  |  7 ++++-
 .../g++.dg/cpp2a/concepts-lambda25.C          | 28 +++++++++++++++++++
 gcc/testsuite/g++.dg/modules/lambda-13.h      | 22 +++++++++++++++
 gcc/testsuite/g++.dg/modules/lambda-13_a.H    |  6 ++++
 gcc/testsuite/g++.dg/modules/lambda-13_b.C    |  6 ++++
 6 files changed, 87 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda25.C
 create mode 100644 gcc/testsuite/g++.dg/modules/lambda-13.h
 create mode 100644 gcc/testsuite/g++.dg/modules/lambda-13_a.H
 create mode 100644 gcc/testsuite/g++.dg/modules/lambda-13_b.C

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 6abd0966fcd..92a3a780008 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -2860,9 +2860,27 @@ evaluate_concept_check (tree check)
 
   gcc_assert (concept_check_p (check));
 
+  /* We don't want any declarations instantiated from a concept evaluation
+     to enter the binding table for the current scope, such as lambdas, so
+     leave that scope.  But maintain the access context (PR104111).  */
+  tree scope = current_scope ();
+  if (CLASS_TYPE_P (scope))
+    scope = TYPE_MAIN_DECL (scope);
+  else if (TREE_CODE (scope) != FUNCTION_DECL)
+    scope = NULL_TREE;
+
+  push_to_top_level ();
+  if (scope)
+    push_access_scope (scope);
+
   /* Check for satisfaction without diagnostics.  */
   sat_info quiet (tf_none, NULL_TREE);
-  return constraint_satisfaction_value (check, /*args=*/NULL_TREE, quiet);
+  tree r = constraint_satisfaction_value (check, /*args=*/NULL_TREE, quiet);
+
+  if (scope)
+    pop_access_scope (scope);
+  pop_from_top_level ();
+  return r;
 }
 
 /* Evaluate the requires-expression T, returning either boolean_true_node
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 341e5ab8808..b24e646cc29 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -20589,7 +20589,12 @@ tsubst_lambda_expr (tree t, tree args, tsubst_flags_t 
complain, tree in_decl)
       return error_mark_node;
     }
 
-  if (LAMBDA_EXPR_EXTRA_SCOPE (t))
+  if (LAMBDA_EXPR_EXTRA_SCOPE (t)
+      /* When evaluating a concept we instantiate any lambda bodies
+        in the context of the evaluation.  For ABI reasons don't
+        record a scope for this instantiated lambda so we don't
+        throw off the scope counter.  */
+      && TREE_CODE (LAMBDA_EXPR_EXTRA_SCOPE (t)) != CONCEPT_DECL)
     record_lambda_scope (r);
   if (TYPE_NAMESPACE_SCOPE_P (TREE_TYPE (t)))
     /* If we're pushed into another scope (PR105652), fix it.  */
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-lambda25.C 
b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda25.C
new file mode 100644
index 00000000000..188a52c7fd9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda25.C
@@ -0,0 +1,28 @@
+// PR c++/123075
+// { dg-do compile { target c++20 } }
+// { dg-additional-options "-fkeep-inline-functions" }
+
+template <typename T>
+concept r = []{ return true; }();
+
+template <typename T, typename U>
+inline auto foo() {
+  static_assert(r<T>);
+  r<U>;
+  return []{ return false; };
+}
+
+template <typename T>
+struct S {
+  static_assert(r<T>);
+  decltype([]{ return true; }) l;
+};
+S<char> s;
+
+bool use = (foo<int, double>()() || s.l());
+
+// There should only be one lambda keyed to 'foo()' and 'S::l'
+// { dg-final { scan-assembler {_ZZ3fooIidEDavENKUlvE_clEv:} } }
+// { dg-final { scan-assembler {_ZNK1SIcEUlvE_clEv:} } }
+// { dg-final { scan-assembler-not {_ZZ3fooIidEDavENKUlvE0_clEv:} } }
+// { dg-final { scan-assembler-not {_ZNK1SIcEUlvE0_clEv:} } }
diff --git a/gcc/testsuite/g++.dg/modules/lambda-13.h 
b/gcc/testsuite/g++.dg/modules/lambda-13.h
new file mode 100644
index 00000000000..275e6d2269a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/lambda-13.h
@@ -0,0 +1,22 @@
+// PR c++/123075
+
+template <typename T>
+concept r = []{ return true; }();
+
+template <typename T>
+inline void foo() {
+  static_assert(r<T>);
+}
+
+template void foo<int>();
+
+template <typename T>
+struct S {
+  static_assert(r<T>);
+};
+
+template struct S<double>;
+
+enum E {
+  X = r<E>,
+};
diff --git a/gcc/testsuite/g++.dg/modules/lambda-13_a.H 
b/gcc/testsuite/g++.dg/modules/lambda-13_a.H
new file mode 100644
index 00000000000..2a748fef88f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/lambda-13_a.H
@@ -0,0 +1,6 @@
+// PR c++/123075
+// { dg-do compile { target c++20 } }
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+
+#include "lambda-13.h"
diff --git a/gcc/testsuite/g++.dg/modules/lambda-13_b.C 
b/gcc/testsuite/g++.dg/modules/lambda-13_b.C
new file mode 100644
index 00000000000..fac66bc5c23
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/lambda-13_b.C
@@ -0,0 +1,6 @@
+// PR c++/123075
+// { dg-do compile { target c++20 } }
+// { dg-additional-options "-fmodules -fno-module-lazy" }
+
+#include "lambda-13.h"
+import "lambda-13_a.H";
-- 
2.51.0

Reply via email to