Forward declaration to break cyclic dependency in C++20 modules doesn't work
Asked Answered
L

4

14

I've been banging my head on this problem for days, I read a lot of documentation and posts about new C++20 modules among which this official one, this one and this other one on Stackoverflow, but I really cannot solve this problem.

I'm using MSVC compiler delivered with Visual Studio Preview 16.6.0 2.0. I know it is not a stable release yet, but I'd like to mess around with new features to start learning them.

Basically I wrote a module (myModule) and 2 partitions of this module (mySubmodule1 and mySubmodule2) and I implemented them in two module implementation files (mySubmodule1Impl.cpp and mySubmodule2Impl.cpp).

mySubmodule1 have a dependency on mySubmodule2, and vice-versa. Here is the source:

mySubmodule1.ixx

export module myModule:mySubmodule1;

export namespace myNamespace{

class MyClass2;

class MyClass1{
    public:
    int foo(MyClass2& c);
    int x = 9;
};
}

mySubmodule2.ixx

export module myModule:mySubmodule2;
import :mySubmodule1;

export namespace myNamespace{

class MyClass2 {
    public:
    MyClass2(MyClass1 x);
    int x = 14;
    MyClass1 c;
};
}

mySubmodule1Impl.cpp

module myModule:mySubmodule1;
import :mySubmodule2;

int myNamespace::MyClass1::foo(myNamespace::MyClass2& c) {
    this->x = c.x-14;
    return x;
}

mySubmodule2Impl.cpp

module myModule:mySubmodule2;
import :mySubmodule1;

myNamespace::MyClass2::MyClass2(myNamespace::MyClass1 c) {
    this->x = c.x + 419;
}

myModule.ixx

export module myModule;

export import :mySubmodule1;
export import :mySubmodule2;

As you can see I can forward declare MyClass2 in mySubmodule1, but I cannot forward declare MyClass1 in mySubmodule2, because in MyClass2 I use a concrete object of type MyClass1.

I compile with this line: cl /EHsc /experimental:module /std:c++latest mySubmodule1.ixx mySubmodule2.ixx myModule.ixx mySubmodule1Impl.cpp mySubmodule2Impl.cpp Source.cpp where Source.cpp is just the main.

I get the infamous error C2027: use of undefined type 'myNamespace::MyClass2' in mySubmodule1Impl.cpp and mySubmodule2Impl.cpp at the lines where I use MyClass2. Moreover the compiler tells me to look at the declaration of MyClass2 in mySubmodule1.ixx where there is the forward declaration.

Now, I really do not understand where is my mistake. I checked over and over but the logic of the program seems perfect to me. The order of compilation of the files should define MyClass2 before it is even used in the implementation!

I tried to compile this exact program using the "old" .h and .cpp files instead of modules, and it compiles and run fine. So I guess I'm missing something regarding these new modules.

I checked on the first official proposal of modules (paragraph 10.7.5), and in the first one there was a construct named proclaimed ownership declaration which seemed to be perfect in such cases. Basically it allows you to import an entity owned by another module in the current module, but without importing the module itself. But in later revisions of the proposal there is no sign of it. Abslolutely nothing. And in the "changelog" section of the new proposal it isn't even cited.

Please don't tell me cyclic dependencies are bad. I know often they are bad, but not always. And even if you think they are always bad I'm not asking for a rule of the thumb. I'm asking why my code compiles with "old" .h + .cpp but not with new modules. Why the linker doesn't see the definition of MyClass2.


EDIT 1

Here is the new design suggested in the answer, but it still doesn't work. I get the exact same errors:

mySubmodule1Impl.cpp

module myModule;

int myNamespace::MyClass1::foo(myNamespace::MyClass2& c) {
    this->x = c.x-14;
    return x;
}

mySubmodule2Impl.cpp

module myModule;

myNamespace::MyClass2::MyClass2(myNamespace::MyClass1 c) {
    this->x = c.x + 419;
}

All of the other files are unchanged.

Lainelainey answered 27/3, 2020 at 18:42 Comment(6)
Some of this code use mySubmodule and some mySubmodule1. What is it?Selfexecuting
"A named module shall not contain multiple module partitions with the same module-partition." I don't think you are doing your partitions right.Selfexecuting
@Selfexecuting Sorry, you're right, it is the same. In my code I used mySubmodule but I thought it would be better to name it mySubmodule1 in the example to avoid misunderstandings. I probably forgot to change some occurrences. Now I correct them and all should be ok.Lainelainey
@Selfexecuting Mhhh I don't have two module partitions with the same name. By the way if I remove the cyclic dependency it works, so I don't think the problem is with the names, but with something about the cyclic dependency itself. Thanks!Lainelainey
Note that this whole problem is unrelated to using Partitions. I don't use partitions, and I have the exact same issue with two classes in two modules that reference each other, and therefore forward declare each other. Compiling either one of the module complains that the 'class is incomplete' and refers to the forward declaration - instead of using the full class definition. Sometimes it even complains that the class 'namespace::A' cannot be converted to 'namespace::A' - obviously having internally two instances of it.Lanita
I've posted a possible solution under two questions: https://mcmap.net/q/885534/-c-modules-forward-declaring-entity-from-another-module https://mcmap.net/q/901865/-c-20-modules-cyclic-dependencyCork
D
10

The immediate problem is that you can’t have an “interface file” and an “implementation file” for a single module partition (as if it were a header file and source file pair). There are interface partitions and implementation partitions, but each must have its own name because each exists to be imported. Of course, it is also one of the purposes of modules to allow a single file where header/source pairs were needed: you can often include the implementation in the same file as the interface but use export and/or inline only with the latter. This does come with the usual header-only downside of causing more frequent downstream rebuilds.

The metaproblem is that there is no circularity here: you’ve already addressed it with the forward declaration of MyClass2. That’s the right thing to do: modules don’t change the basic semantics of C++, so such techniques remain applicable and necessary. You can still divide the classes into two files for the usual organizational reasons, but there’s no need for the method definitions to be in partitions at all (nor in separate module myModule; implementation units, which automatically import all of the interface). The import :mySubmodule1 that remains (in the interface partition mySubmodule2) is then unambiguous and correct.

As for proclaimed-ownership-declarations, they appeared in the Modules TS that didn’t have module partitions, such that cases like this could not be handled otherwise (since you can use a normal forward declaration for an entity from another partition but not another module).

Dhumma answered 28/3, 2020 at 16:43 Comment(11)
Ok, first of all, thanks a lot! I admit I'm now a bit confused, and I have many questions, but in particular 2: 1) Why, if I remove all of the references to MyClass1 in mySubmodule2.ixx all works fine (even tho I have "interface file" and "implementation file")? 2) How do I separe the interface of a partition from its implementation? I think this can be very useful design wise, but now I really cannot come up with a solution... I thought partitions were useful to divide logical parts of a module into smaller files, but if I can't separate their implementation they're useless... No?Lainelainey
Ok, after some thinking I think I understood. Basically I have to remove the lines module; import :mySubmodule; and replace it with a simple import myModule in the mySubmoduleImpl.cpp file. And I change in the same way mySubmodule2Impl.cpp file. I did it and it works, is this the right way? Basically I have my partitions interfaces files and I implement them in plain .cpp files. Thanks a whole lot, it took me ages to solve this. If you have some references or can elaborate a little bit more on modules, I'll listen to very gladly. My only question is the 1) in my previous comment.Lainelainey
@Lapo: Violations of the rules about translation units do not require a diagnostic: the compiler may not examine the other files or not detect collisions because of separate processing. You should put the implementation in normal implementation units (with just module A;); you’re not (supposed to be) able to define something from a module in a translation unit that merely imports it. (You already found the original, approved wording, but I did also write a little bit more on modules.)Dhumma
Ok, thanks a lot, now it is clear. I'll try as soon as possible the correct solution. But can I have more than one implementation unit for the same module? So that I can separate (in different files) the implementation of each different partition interface of the module?Lainelainey
@Lapo: You can have as many partitions (each with a different in its [export] module foo:…;) and as many “plain implementation units” (all with the same module foo;) as you want for one foo. (Obviously each of either kind also has a file name.)Dhumma
Perfect, thanks again for your clear explaination! Now it works perfectly! The ndr thing can make you lose a ton of time sometimes, I had no clue what the error was, even tho it was very easy to spot if I knew the modules rule better.Lainelainey
@Lapo: Great; you’re welcome. There is some hope that refinements to implementations and conventions will reduce the possibility of silent misbehavior here, but separate translation is fundamentally hard to make user-friendly.Dhumma
No, man, I'm sorry, but it still doesn't work and I get the exact same error. I noticed that I forgot to uncomment the line where I import :mySubmodule1 in mySubmodule2, (I also commented out the MyClass1 c; line). I did this to test if it worked without cyclic dependencies, and it did (both with my old wrong design and with the one you suggested). But as soon as I put those lines in again, I get the exact same errors. I don't know, your suggestions made perfectly sense, but it still doesn't work. If you have another idea please tell me. I edited the question with the new design.Lainelainey
Mhhh no, it's not that. It is just a typo because in my real code I have mySubmodule and MyClass without 1. I corrected it now, but that's not the issue...Lainelainey
@Lapo: As far as I can tell, the code is correct as amended. Make sure you aren’t using any old compiled module interface files—I don’t know how those work with MSVC.Dhumma
It doesn't seem I'm using any old compiled files, even because I delete them before every new compilation, just not to be wrong. I'm really out of ideas, I'm for sure missing something, but I don't know what. Of course this can even be a bug, since modules aren't yet stable in MSVC. I'll try with a different compiler as soon as I can. As a last resource I'll open a issue feedback in the VS dev community. If something comes to your mind about this problem, please let me know. Thanks again for your help so far!Lainelainey
D
0

Try exporting the forward declarations. e.g.

// A.cc

export module Cyclic:A;

export class B;
export class A {
public:
    char name() { return 'A'; }
    void f(B& b);
};
// B.cc

export module Cyclic:B;

export class A;
export class B {
public:
    char name() { return 'B'; }
    void f(A& a);
};
// A_impl.cc

module Cyclic;

import Cyclic:A;
import Cyclic:B;

import <iostream>;

void A::f(B& b) {
  std::cout << name() << " calling " << b.name() << std::endl;
}
// B_impl.cc

module Cyclic;

import Cyclic:B;
import Cyclic:A;

import <iostream>;

void B::f(A& a) {
  std::cout << name() << " calling " << a.name() << std::endl;
}
// Cyclic.cc

export module Cyclic;
export import :A;
export import :B;
Dioecious answered 17/5, 2022 at 4:28 Comment(6)
Nothing imports the _impl partitions, which is invalid since they’re interface partitions but more importantly means they need not be partitions at all.Dhumma
the _impl partitions can be imported into Cyclic.cc, but since they don't export anything export module can be reduced to just module to be minimalisticDioecious
Importing them still wouldn’t do anything.Dhumma
@DavisHerring "Nothing imports the _impl partitions, which is invalid" It's not invalid. It is valid. Nothing has or should need to import the implementations. That's the whole point of partitioning the classes into headers/implementations and module interface units and module implementation units: so that nobody knows about the implementation parts of the classes, except the things that absolutely have to, to not have to recompile things when they don't really need to.Spile
You can see my answer on how I fixed this problem without exporting the forward declaration and nothing imports the module implementation units.Spile
@KulaGGin: My comment referred to the first version of this answer; there aren’t any _impl partitions now.Dhumma
S
-1

I found 2 solutions to this problem.

Just trying to do forward declarations instead of imports in module interface units to try to improve compilation times is enough to get you on a goose chase like I just got an hour ago.

So, let's say we have 2 modules:

MyType1.ixx:

export module MyType1;

namespace MyNamespace {
    class MyType2;

    export class MyType1 {
    public:
        MyType1(MyType2* MyType2);
    private:
        MyType2* MyType2Intance{};
    };
}

Module1.cpp:

module MyType1;

namespace MyNamespace {
    MyType1::MyType1(MyType2* MyType2) {
        MyType2Intance = MyType2;
    };
}

MyType2.ixx:

export module MyType2;

namespace MyNamespace {
    export class MyType2 {
    public:
    };
}

Then if you try to instantiate MyType1 in Main.ixx module:

export module Main;

import MyType1;
import MyType2;

using namespace MyNamespace;

int main() {
    MyType2* MyType2Ptr = nullptr;
    MyType1 MyType1(MyType2Ptr);
}

You'll get a Error C2665 'MyNamespace::MyType1::MyType1': no overloaded function could convert all the argument types. Which is absolutely bizarre, since there's enough information to resolve everything. We don't need imports in neither .ixx nor in the .cpp file of MyType1 module for a simple pointer reference. Even if we actually used MyType2 in MyType1.cpp file, we should only need to import MyType2 in the .cpp file and we shouldn't have to import MyType2 in the MyType1.ixx file.

So, what do you do to fix this annoying illogical error that shouldn't exist in the first place?

One solution is to also export the forward-declared class MyType2 in MyType1.ixx:

export module MyType1;

namespace MyNamespace {
    export class MyType2;

    export class MyType1 {
    public:
        MyType1(MyType2* MyType2);
    private:
        MyType2* MyType2Intance{};
    };
}

Another solution is to forward-declare the MyType2 class before the export module MyType1 line:

namespace MyNamespace {
    class MyType2;
}

export module MyType1;

namespace MyNamespace {
    export class MyType1 {
    public:
        MyType1(MyType2* MyType2);
    private:
        MyType2* MyType2Intance{};
    };
}

And if you use MyType2 in the module implementation file of MyType2, of course, you'll have to import it properly.

I'd show a running compiling example on godbolt or something, but I don't think there's a way to create multi-file solutions like that there.

Now, why this works, I don't really know as I'm only starting with modules myself. Visual Studio warned me yesterday in the build output log that the #include <Windows.h> after an export module declaration line export module MyModule; is probably erroneous(but it compiled the solution just fine) and that I should move my #include <Windows.h> before that line. I did that, it worked, and so that means that there are multiple sections of the module in the module interface file: before export module MyModule; line and after at the very least.

Also, why does the first solution with exporting the forward declaration fixes the compilation error? The Main module imports both modules, so it doesn't have problems having all information needed.

I did a few tutorials on modules, like this one by Microsoft: https://learn.microsoft.com/en-us/cpp/cpp/tutorial-named-modules-cpp?view=msvc-170

But I don't think any of them explained this forward declaration buggy quirk, and also how #including headers is different before and after the export module MyModule; line. Let me know if know why and even better if you have a link to video/article/book where it's explained how all this works and why all of this is happening.

Spile answered 23/11, 2023 at 19:42 Comment(9)
I’m not sure this is an answer, but both of these approaches declare MyType2 with declarations attached to two different modules, which is IFNDR (as neither imports the other).Dhumma
@Davis "I’m not sure this is an answer" It is an answer. "as neither imports the other" importing the module isn't required if you don't actually use anything from its' interface. If I import MyType2 inside MyType1.cpp, I can actually use MyType2 class inside MyType1 module implementation file there and it will work just fine and run everything correctly. I just tested it and have a running example.Spile
"but both of these approaches declare MyType2 with declarations attached to two different modules, which is IFNDR" This isn't IFNDR: inside MyType1.ixx I forward-declare a MyType2 class. It doesn't say that it's a MyType2 class that is a part of MyType1 module. It says that MyType2 class that's part of the MyNamespace namespace. Then I can either just define this MyType2 class inside the same MyType1.ixx interface unit, or I can import MyType2 from inside MyType1.cpp and define this MyType2 class inside a different MyType2.ixx module, and it all works just fine.Spile
But I will gladly see an example(video, article, chapter in a book, whatever) where it breaks if you have one.Spile
Your export class MyType2; appears in the purview of MyType1 and is therefore attached to it ([module.unit]/7.3). Your export class MyType2 { appears in the purview of MyType2 and is similarly attached to it, violating [basic.link]/10 because they declare the same entity (/8, in particular /8.3). The fact that you "just tested it and have a running example" means very little: IFNDR means anything can happen, including what you were expecting, even aside from the fact that at this stage of modules deployment it's still quite possible that you're observing a compiler bug.Dhumma
@DavisHerring OK, I see where you're coming from, but just above that 10.1(7.2) says this: (7.2)Otherwise, if the declaration (7.2.1) — is a namespace-definition with external linkage or (7.2.2) — appears within a linkage-specification (9.11) it is attached to the global module.Spile
And then 10.1(6): The global module is the collection of all global-module-fragments and all translation units that are not module units. And from what I've read online, doing export makes it external linkage(well, duh). So it is a declaration with external linkage and therefore attached to the global module and is considered to be not a part of the module. At least, that's how I understand it. So it correctly works.Spile
Also, what do you think about the other solution with declaring it before export module MyType1; like I did in the second solution? Do you think it is correct according to the specification?Spile
Only namespaces themselves are attached to the global module by that rule, not their contents. Declarations before the module-declaration must appear via #include ([cpp.pre]/3), but even then the global-module attachment is incompatible with the declaration in MyType2.Dhumma
N
-2

See my answer to this post. You probably need to export your forward declaration to have external rather than module linkage for MyClass2.

Negrophobe answered 7/10, 2021 at 9:25 Comment(1)
But why do you have to export? It doesn't make any sense.Spile

© 2022 - 2024 — McMap. All rights reserved.