[Bug c++/98885] New: [modules] forward declaration of classes prevent them from being exported at the point of actual declaration

[Bug c++/98885] New: [modules] forward declaration of classes prevent them from being exported at the point of actual declaration

[pilarlatiesa at gmail dot com]

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

            Bug ID: 98885
           Summary: [modules] forward declaration of classes prevent them
                    from being exported at the point of actual declaration
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: pilarlatiesa at gmail dot com
  Target Milestone: ---

$ /home/pililatiesa/GCC-11/bin/g++ -v

Using built-in specs.
COLLECT_GCC=/home/pililatiesa/GCC-11/bin/g++
COLLECT_LTO_WRAPPER=/home/pililatiesa/GCC-11/bin/../libexec/gcc/x86_64-pc-linux-gnu/11.0.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: ../gcc-master/configure --prefix=/home/pililatiesa/GCC-11
--enable-languages=c++ --disable-bootstrap
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 11.0.0 20210129 (experimental) (GCC)


$ cat A.cpp

export module A;

class A; // forward declaration

export class A {}; // actual declaration


$ cat main.cpp

import A;

int main()
{
  A a;

  return 0;
}


$ /home/pililatiesa/GCC-11/bin/g++ -fmodules-ts A.cpp main.cpp -o foo
main.cpp: In function ‘int main()’:
main.cpp:6:3: error: ‘A’ was not declared in this scope
    6 |   A a;
      |   ^


It can be worked around by typing "export" in the forward declaration as well.

Not sure this is a bug, but it that's the specified behaviour, I’d say it’s
very counter-intuitive. For example, this won’t work:

$ cat A.cpp

export module A;

class B;

export
class A
{
  void
  f(B const &) const;
};

$ cat B.cpp

export module B;

class A;

export
class B
{
  void
  f(A const &) const;
};

$ cat A-impl.cpp

module A;

import B;

void
A::f(B const &) const {}

$ cat B-impl.cpp

module B;

import A;

void
B::f(A const &) const {}

[Bug c++/98885] [modules] forward declaration of classes prevent them from being exported at the point of actual declaration

[nathan at gcc dot gnu.org]

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

Nathan Sidwell <nathan at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|rejects-valid               |accepts-invalid
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2021-01-29
     Ever confirmed|0                           |1
           Assignee|unassigned at gcc dot gnu.org      |nathan at gcc dot gnu.org

--- Comment #1 from Nathan Sidwell <nathan at gcc dot gnu.org> ---
This is ill-formed.  an exported entity must be so-declared on its first
declaration.

A diagnostic is missing on the definition of the class.

[Bug c++/98885] [modules] forward declaration of classes prevent them from being exported at the point of actual declaration

[pilarlatiesa at gmail dot com]

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

--- Comment #2 from Pilar Latiesa <pilarlatiesa at gmail dot com> ---
(In reply to Nathan Sidwell from comment #1)
> This is ill-formed.  an exported entity must be so-declared on its first
> declaration.
> 
> A diagnostic is missing on the definition of the class.

I see. Thanks.

What about the second testcase? How a pattern like that is supposed to be
modularized?

[Bug c++/98885] [modules] forward declaration of classes prevent them from being exported at the point of actual declaration

[nathan at gcc dot gnu.org]

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

--- Comment #3 from Nathan Sidwell <nathan at gcc dot gnu.org> ---
module partitions are probably what you want for such mutually dependent types.

[Bug c++/98885] [modules] forward declaration of classes prevent them from being exported at the point of actual declaration

[pilarlatiesa at gmail dot com]

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

--- Comment #4 from Pilar Latiesa <pilarlatiesa at gmail dot com> ---

I tried:

$ cat M.cpp

export module M;

export import :A;
export import :B;


$ cat A.cpp

export module M:A;

export class B;

export
class A
{
  void
  f(B const &) const;
};


$ cat B.cpp

export module M:B;

export class A;

export
class B
{
  void
  f(A const &) const;
};


$ cat A-impl.cpp

module M:A;

void
A::f(B const &) const {}


$ cat B-impl.cpp

module M:B;

void
B::f(A const &) const {}


and I got:

$ ../GCC-11/bin/g++ -fmodules-ts A.cpp B.cpp M.cpp A-impl.cpp B-impl.cpp
main.cpp -o foo
A-impl.cpp:5:1: error: ‘A’ has not been declared
    5 | A::f(B const &) const {}
      | ^
A-impl.cpp:5:1: error: variable or field ‘f’ declared void
A-impl.cpp:5:6: error: ‘B’ was not declared in this scope
    5 | A::f(B const &) const {}
      |      ^
A-impl.cpp:2:1: warning: not writing module ‘M:A’ due to errors
    2 | module M:A;
      | ^~~~~~
B-impl.cpp:5:1: error: ‘B’ has not been declared
    5 | B::f(A const &) const {}
      | ^
B-impl.cpp:5:1: error: variable or field ‘f’ declared void
B-impl.cpp:5:6: error: ‘A’ was not declared in this scope
    5 | B::f(A const &) const {}
      |      ^
B-impl.cpp:2:1: warning: not writing module ‘M:B’ due to errors
    2 | module M:B;
      | ^~~~~~

[Bug c++/98885] [modules] forward declaration of classes prevent them from being exported at the point of actual declaration

[nathan at gcc dot gnu.org]

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

--- Comment #5 from Nathan Sidwell <nathan at gcc dot gnu.org> ---
your A-impl.cpp needs `import :B` and vice-versa

[Bug c++/98885] [modules] forward declaration of classes prevent them from being exported at the point of actual declaration

[pilarlatiesa at gmail dot com]

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

--- Comment #6 from Pilar Latiesa <pilarlatiesa at gmail dot com> ---
(In reply to Nathan Sidwell from comment #5)
> your A-impl.cpp needs `import :B` and vice-versa

Yep. Thanks. But that's not enough for it to compile:

A-impl.cpp:7:17: error: invalid use of incomplete type ‘class A@M:B’
    7 | A::f(B const &) const {}
      |                 ^~~~~
In module M:B, imported at A-impl.cpp:4:
B.cpp:4:14: note: forward declaration of ‘class A@M:B’
    4 | export class A;
      |              ^
A-impl.cpp:2:1: warning: not writing module ‘M:A’ due to errors
    2 | module M:A;
      | ^~~~~~
In module imported at B-impl.cpp:4:1:
M:A: error: failed to read compiled module: No such file or directory
M:A: note: compiled module file is ‘gcm.cache/M-A.gcm’
M:A: note: imports must be built before being imported
M:A: fatal error: returning to the gate for a mechanical issue
compilation terminated.

[Bug c++/98885] [modules] forward declaration of classes prevent them from being exported at the point of actual declaration

[nathan at gcc dot gnu.org]

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

--- Comment #7 from Nathan Sidwell <nathan at gcc dot gnu.org> ---
My mistake.  interface and implementation partitions do not follow the
non-partition behaviour.  You have two M:A partitions (one an interface and one
an implementation).  This is what you want:

A.cc:
export module M:A;
export class B;
export
class A
{
  void
  f(B const &) const;
};


B.cc:
export module M:B;
export class A;
export
class B
{
  void
  f(A const &) const;
};

M.cc:
export module M;

export import :A;
export import :B;

M-impl.cc:
module M;

void
A::f(B const &) const {}
void
B::f(A const &) const {}


devvm1702:10>./cc1plus -fmodules-ts -quiet A.cc     
devvm1702:11>./cc1plus -fmodules-ts -quiet B.cc     
devvm1702:12>./cc1plus -fmodules-ts -quiet M.cc     
devvm1702:13>./cc1plus -fmodules-ts -quiet M-impl.cc


you could of course split M-impl.cc into two if you want, one for the A bits
and one for the B bits

[Bug c++/98885] [modules] forward declaration of classes prevent them from being exported at the point of actual declaration

[pilarlatiesa at gmail dot com]

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

--- Comment #8 from Pilar Latiesa <pilarlatiesa at gmail dot com> ---
(In reply to Nathan Sidwell from comment #7)

That works. Thanks so much.

When there are several partitions involved it is not entirely clear to me what
declarations are first seen by the compiler compared with the legacy textual
inclusion model, so I assume that the rule would be to annotate every forward
declaration of an exported entity with 'export'.

[Bug c++/98885] [modules] forward declaration of classes prevent them from being exported at the point of actual declaration

[nathan at gcc dot gnu.org]

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

--- Comment #9 from Nathan Sidwell <nathan at gcc dot gnu.org> ---
This is highlighting a missing diagnostic on:

export module A;

class A; // forward declaration

export class A {}; // <-- here, cannot add 'export'

[Bug c++/98885] [modules] forward declaration of classes prevent them from being exported at the point of actual declaration

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |redi at gcc dot gnu.org

--- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> ---
*** Bug 102341 has been marked as a duplicate of this bug. ***

[Bug c++/98885] [modules] forward declaration of classes prevent them from being exported at the point of actual declaration

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Nathaniel Shead <nshead@gcc.gnu.org>:

https://gcc.gnu.org/g:d89903ff29473e6e64f032ecee5c72d1584546dc

commit r14-5808-gd89903ff29473e6e64f032ecee5c72d1584546dc
Author: Nathaniel Shead <nathanieloshead@gmail.com>
Date:   Sun Nov 12 22:49:45 2023 +1100

    c++: check mismatching exports for class tags [PR98885]

    Checks for exporting a declaration that was previously declared as not
    exported is implemented in 'duplicate_decls', but this doesn't handle
    declarations of classes. This patch adds these checks and slightly
    adjusts the associated error messages for clarity.

            PR c++/98885

    gcc/cp/ChangeLog:

            * decl.cc (duplicate_decls): Adjust error message.
            (xref_tag): Adjust error message. Check exporting decl that is
            already declared as non-exporting.

    gcc/testsuite/ChangeLog:

            * g++.dg/modules/export-1.C: Adjust error messages. Remove
            xfails for working case. Add new test case.

    Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>

[Bug c++/98885] [modules] forward declaration of classes prevent them from being exported at the point of actual declaration

[nshead at gcc dot gnu.org]

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

Nathaniel Shead <nshead at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |14.0
                 CC|                            |nshead at gcc dot gnu.org

--- Comment #12 from Nathaniel Shead <nshead at gcc dot gnu.org> ---
Fixed for GCC 14.

[Bug c++/98885] [modules] forward declaration of classes prevent them from being exported at the point of actual declaration

[nshead at gcc dot gnu.org]

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

Nathaniel Shead <nshead at gcc dot gnu.org> changed:

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

--- Comment #13 from Nathaniel Shead <nshead at gcc dot gnu.org> ---
.