[Bug c++/104919] New: [modules] enum in constexpr function causes "failed to read compiled module cluster 1: Bad file data"

[Bug c++/104919] New: [modules] enum in constexpr function causes "failed to read compiled module cluster 1: Bad file data"

[ensadc at mailnesia dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104919

            Bug ID: 104919
           Summary: [modules] enum in constexpr function causes "failed to
                    read compiled module cluster 1: Bad file data"
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ensadc at mailnesia dot com
  Target Milestone: ---

https://godbolt.org/z/d3sdeEz1r

====
$ cat mod.cpp
export module mod;

export constexpr void f() {
    enum { a };
    a;
}

$ cat example.cpp
import mod;

int main() {
    f();
}

$ g++ -std=c++20 -fmodules-ts mod.cpp example.cpp
In module imported at example.cpp:1:1:
mod: In function ‘int main()’:
mod: error: failed to read compiled module cluster 1: Bad file data
mod: note: compiled module file is ‘gcm.cache/mod.gcm’
example.cpp:4:5: fatal error: failed to load binding ‘::f@mod’
    4 |     f();
      |     ^
compilation terminated.

====
It compiles fine without `constexpr`.

[Bug c++/104919] [modules] enum in constexpr function causes "failed to read compiled module cluster 1: Bad file data"

[ppalka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104919

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ppalka at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2024-02-16
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=106009
     Ever confirmed|0                           |1

--- Comment #1 from Patrick Palka <ppalka at gcc dot gnu.org> ---
s/constexpr/inline also triggers the bug, unsurprisingly.  PR106009 about
templated local enums might be related.

[Bug c++/104919] [modules] enum in constexpr function causes "failed to read compiled module cluster 1: Bad file data"

[ppalka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104919

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |ppalka at gcc dot gnu.org
   Target Milestone|---                         |14.0
             Status|NEW                         |ASSIGNED

[Bug c++/104919] [modules] enum in constexpr function causes "failed to read compiled module cluster 1: Bad file data"

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104919

--- Comment #2 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:574fd1f17f100c7c355ad26bc525ab5a3386bb2d

commit r14-9268-g574fd1f17f100c7c355ad26bc525ab5a3386bb2d
Author: Patrick Palka <ppalka@redhat.com>
Date:   Fri Mar 1 17:24:15 2024 -0500

    c++/modules: depending local enums [PR104919, PR106009]

    For local enums defined in a non-template function or a function template
    instantiation it seems we neglect to make the function depend on the enum
    definition (which modules considers logically separate), which ultimately
    causes the enum definition to not be properly streamed before uses
    within the function definition are streamed.

    The code responsible for noting such dependencies is

    gcc/cp/module.cc
    @@ -8784,17 +8784,6 @@ trees_out::decl_node (tree decl, walk_kind ref)
       depset *dep = NULL;
       if (streaming_p ())
         dep = dep_hash->find_dependency (decl);
    !  else if (TREE_CODE (ctx) != FUNCTION_DECL
    !          || TREE_CODE (decl) == TEMPLATE_DECL
    !          || (dep_hash->sneakoscope && DECL_IMPLICIT_TYPEDEF_P (decl))
    !          || (DECL_LANG_SPECIFIC (decl)
    !              && DECL_MODULE_IMPORT_P (decl)))
    !    {
    !      auto kind = (TREE_CODE (decl) == NAMESPACE_DECL
    !                  && !DECL_NAMESPACE_ALIAS (decl)
    !                  ? depset::EK_NAMESPACE : depset::EK_DECL);
    !      dep = dep_hash->add_dependency (decl, kind);
    !    }

       if (!dep)
         {

    and the condition there notably excludes local TYPE_DECLs from a
    non-template-pattern function (when streaming a template pattern
    we'll see be dealing with the corresponding TEMPLATE_DECL of the
    local TYPE_DECL here, so we'll add the dependency).

    Local classes on the other hand seem to work properly, but perhaps by
    accident: with a local class we end up making the function depend on the
    injected-class-name of the local class rather than the local class as a
    whole because the injected-class-name satisfies the criteria (since its
    context is the local class, not the function).

    The 'sneakoscope' flag is set when walking a function declaration and
    its purpose seems to be to catch a local type that escapes the function
    via a deduced return type (so called voldemort types) and note a
    dependency on them.  But there seems to be no reason to restrict this
    behavior to voldemort types, and indeed consistently noting the dependency
    for all local types fixes these PRs (almost).  So this patch gets rid of
    this flag and enables the dependency tracking unconditionally.

    This was nearly enough to make things work, except we now ran into
    issues with the local TYPE_/CONST_DECL copies from the pre-gimplified
    version of a constexpr function body during streaming.  Rather than
    making modules cope with this, it occurred to me that we don't need to
    make copies of local types when saving the pre-gimplified body (and when
    making further copies thereof); only VAR_DECLs etc need to be copied
    (so that we don't conflate local variables from different recursive
    calls to the same function during constexpr evaluation).  So this patch
    adjusts copy_fn accordingly.

            PR c++/104919
            PR c++/106009

    gcc/cp/ChangeLog:

            * module.cc (depset::hash::sneakoscope): Remove.
            (trees_out::decl_node): Always add a dependency on a local type.
            (depset::hash::find_dependencies): Remove sneakoscope stuff.

    gcc/ChangeLog:

            * tree-inline.cc (remap_decl): Handle copy_decl returning the
            original decl.
            (remap_decls): Handle remap_decl returning the original decl.
            (copy_fn): Adjust copy_decl callback to skip TYPE_DECL and
            CONST_DECL.

    gcc/testsuite/ChangeLog:

            * g++.dg/modules/tdef-7.h: Remove outdated comment.
            * g++.dg/modules/tdef-7_b.C: Don't expect two TYPE_DECLs.
            * g++.dg/modules/enum-13_a.C: New test.
            * g++.dg/modules/enum-13_b.C: New test.

[Bug c++/104919] [modules] enum in constexpr function causes "failed to read compiled module cluster 1: Bad file data"

[ppalka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104919

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #3 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Fixed for GCC 14.