]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
c++/modules: Handle exposures of TU-local types in uninstantiated member templates
authorNathaniel Shead <nathanieloshead@gmail.com>
Fri, 31 Jan 2025 12:53:35 +0000 (23:53 +1100)
committerNathaniel Shead <nathanieloshead@gmail.com>
Mon, 10 Mar 2025 14:23:47 +0000 (01:23 +1100)
Previously, 'is_tu_local_entity' wouldn't detect the exposure of the (in
practice) TU-local lambda in the following example, unless instantiated:

  struct S {
    template <typename>
    static inline decltype([]{}) x = {};
  };

This is for two reasons.  Firstly, when traversing the TYPE_FIELDS of S
we only see the TEMPLATE_DECL, and never end up building a dependency on
its DECL_TEMPLATE_RESULT (due to not being instantiated).  This patch
fixes this by stripping any templates before checking for unnamed types.

The second reason is that we currently assume all class-scope entities
are not TU-local.  Despite this being unambiguous in the standard, this
is not actually true in our implementation just yet, due to issues with
mangling lambdas in some circumstances.  Allowing these lambdas to be
exported can cause issues in importers with apparently conflicting
declarations, so this patch treats them as TU-local as well.

After these changes, we now get double diagnostics from the two ways
that we can see the above lambda being exposed, via 'S' (through
TYPE_FIELDS) or via 'S::x'.  To workaround this we hide diagnostics from
the first case, so we only get errors from 'S::x' which will be closer
to the point the offending lambda is declared.

gcc/cp/ChangeLog:

* module.cc (trees_out::has_tu_local_dep): Also look at the
TI_TEMPLATE if we don't find a dep for a decl.
(depset::hash::is_tu_local_entity): Handle unnamed template
types, treat lambdas specially.
(is_exposure_of_member_type): New function.
(depset::hash::add_dependency): Use it.
(depset::hash::finalize_dependencies): Likewise.

gcc/testsuite/ChangeLog:

* g++.dg/modules/internal-10.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
Reviewed-by: Jason Merrill <jason@redhat.com>
gcc/cp/module.cc
gcc/testsuite/g++.dg/modules/internal-10.C [new file with mode: 0644]

index 8b0f42951c242eac7e1e3fdabbb84119a5567445..97c3549093c9836c37028b8dd4c01735c1d8288f 100644 (file)
@@ -9632,6 +9632,15 @@ trees_out::has_tu_local_dep (tree decl) const
     decl = TYPE_NAME (DECL_CONTEXT (decl));
 
   depset *dep = dep_hash->find_dependency (decl);
+  if (!dep)
+    {
+      /* This might be the DECL_TEMPLATE_RESULT of a TEMPLATE_DECL
+        which we found was TU-local and gave up early.  */
+      int use_tpl = -1;
+      if (tree ti = node_template_info (decl, use_tpl))
+       dep = dep_hash->find_dependency (TI_TEMPLATE (ti));
+    }
+
   return dep && dep->is_tu_local ();
 }
 
@@ -13504,19 +13513,31 @@ depset::hash::is_tu_local_entity (tree decl, bool explain/*=false*/)
 
      We consider types with names for linkage purposes as having names, since
      these aren't really TU-local.  */
-  if (TREE_CODE (decl) == TYPE_DECL
+  tree inner = STRIP_TEMPLATE (decl);
+  if (inner
+      && TREE_CODE (inner) == TYPE_DECL
       && TYPE_ANON_P (type)
-      && !DECL_SELF_REFERENCE_P (decl)
+      && !DECL_SELF_REFERENCE_P (inner)
       /* An enum with an enumerator name for linkage.  */
       && !(UNSCOPED_ENUM_P (type) && TYPE_VALUES (type)))
     {
       tree main_decl = TYPE_MAIN_DECL (type);
-      if (!DECL_CLASS_SCOPE_P (main_decl)
-         && !decl_function_context (main_decl)
-         /* LAMBDA_EXPR_EXTRA_SCOPE will be set for lambdas defined in
-            contexts where they would not be TU-local.  */
-         && !(LAMBDA_TYPE_P (type)
-              && LAMBDA_TYPE_EXTRA_SCOPE (type)))
+      if (LAMBDA_TYPE_P (type))
+       {
+         /* A lambda expression is, in practice, TU-local iff it has no
+            mangling scope.  This currently doesn't line up exactly with
+            the standard's definition due to some ABI issues, but it's
+            pretty close, and avoids other issues down the line.  */
+         if (!LAMBDA_TYPE_EXTRA_SCOPE (type))
+           {
+             if (explain)
+               inform (loc, "%qT has no name and cannot be differentiated "
+                       "from similar lambdas in other TUs", type);
+             return true;
+           }
+       }
+      else if (!DECL_CLASS_SCOPE_P (main_decl)
+              && !decl_function_context (main_decl))
        {
          if (explain)
            inform (loc, "%qT has no name and is not defined within a class, "
@@ -13820,6 +13841,35 @@ depset::hash::make_dependency (tree decl, entity_kind ek)
   return dep;
 }
 
+/* Whether REF is an exposure of a member type of SOURCE.
+
+   This comes up with exposures of class-scope lambdas, that we currently
+   treat as TU-local due to ABI reasons.  In such a case the type of the
+   lambda will be exposed in two places, first by the class type it is in
+   the TYPE_FIELDS list of, and second by the actual member declaring that
+   lambda.  We only want the second case to warn.  */
+
+static bool
+is_exposure_of_member_type (depset *source, depset *ref)
+{
+  gcc_checking_assert (source->refs_tu_local () && ref->is_tu_local ());
+  tree source_entity = STRIP_TEMPLATE (source->get_entity ());
+  tree ref_entity = STRIP_TEMPLATE (ref->get_entity ());
+
+  if (source_entity
+      && ref_entity
+      && DECL_IMPLICIT_TYPEDEF_P (source_entity)
+      && DECL_IMPLICIT_TYPEDEF_P (ref_entity)
+      && DECL_CLASS_SCOPE_P (ref_entity)
+      && DECL_CONTEXT (ref_entity) == TREE_TYPE (source_entity))
+    {
+      gcc_checking_assert (LAMBDA_TYPE_P (TREE_TYPE (ref_entity)));
+      return true;
+    }
+  else
+    return false;
+}
+
 /* DEP is a newly discovered dependency.  Append it to current's
    depset.  */
 
@@ -13832,7 +13882,7 @@ depset::hash::add_dependency (depset *dep)
   if (dep->is_tu_local ())
     {
       current->set_flag_bit<DB_REFS_TU_LOCAL_BIT> ();
-      if (!ignore_tu_local)
+      if (!ignore_tu_local && !is_exposure_of_member_type (current, dep))
        current->set_flag_bit<DB_EXPOSURE_BIT> ();
     }
 
@@ -14705,7 +14755,9 @@ depset::hash::finalize_dependencies ()
          tree decl = dep->get_entity ();
 
          for (depset *rdep : dep->deps)
-           if (!rdep->is_binding () && rdep->is_tu_local ())
+           if (!rdep->is_binding ()
+               && rdep->is_tu_local ()
+               && !is_exposure_of_member_type (dep, rdep))
              {
                // FIXME:QOI Better location information?  We're
                // losing, so it doesn't matter about efficiency
diff --git a/gcc/testsuite/g++.dg/modules/internal-10.C b/gcc/testsuite/g++.dg/modules/internal-10.C
new file mode 100644 (file)
index 0000000..e374aaa
--- /dev/null
@@ -0,0 +1,25 @@
+// { dg-additional-options "-fmodules -std=c++20" }
+// { dg-module-cmi !M }
+
+// These are not TU-local according to the standard,
+// but we treat as TU-local due to ABI limitations.
+
+export module M;
+
+struct A {
+  static inline auto x = []{};
+
+  template <typename T>
+  static inline auto y = []{};
+};
+
+struct B {
+  template <typename T>
+  static inline decltype([]{}) x = {};  // { dg-bogus "TU-local" "" { xfail *-*-* } }
+};
+
+template <typename T>
+struct C {
+  template <typename U>
+  static inline decltype([]{}) x = {};  // { dg-bogus "TU-local" "" { xfail *-*-* } }
+};