"multiply defined symbols found" how is my destructor being defined twice?

Question

Question: What am I doing to cause a multiple definition symbol linker error?

OSFrameworkWindows10Module.ixx

module;

#include <memory>

export module OSFrameworkWindows10Module;

export class OSFramework {
private:
    struct impl;
    std::unique_ptr<impl> _impl;
public:
    OSFramework();
    ~OSFramework();
};

typedef struct OSFramework::impl {
    impl();
    ~impl();
} impl;

impl::impl()
{}

impl::~impl() = default;

OSFramework::OSFramework() : _impl{ std::make_unique<impl>() } {}
OSFramework::~OSFramework() = default;

winmain.cpp

#define WIN32_LEAN_AND_MEAN
#include <windows.h>

import OSFrameworkWindows10Module;

int WINAPI wWinMain (
    _In_ HINSTANCE hinst,
    _In_opt_ HINSTANCE,
    _In_ PWSTR,
    _In_ int
)
{
    auto os_framework = std::make_unique<OSFramework>();
    return S_OK;
};

compiler output

1>------ Build started: Project: Vegas22Metroidvania1, Configuration: Debug x64 ------
1>Scanning sources for module dependencies...
1>OSFrameworkWindows10Module.ixx
1>Compiling...
1>OSFrameworkWindows10Module.ixx
1>winmain.cpp
1>OSFrameworkWindows10Module.ixx.obj : error LNK2005: "public: __cdecl OSFramework::~OSFramework(void)" (??1OSFramework@@QEAA@XZ::<!OSFrameworkWindows10Module>) already defined in winmain.obj
1>D:\projects\Vegas22Metroidvania1\x64\Debug\Vegas22Metroidvania1.exe : fatal error LNK1169: one or more multiply defined symbols found
1>Done building project "Vegas22Metroidvania1.vcxproj" -- FAILED.

Pretty lost here as to why OSFramework::~OSframework() is being counted as defined more than once. I'm somehow instantiating it in my winmain.cpp file even though I'm just setting up a smart pointer for the class instance.

Maybe relevant info: I'm using Visual Studio 17.4.3

Starting a question with a wall of text is a turn-off. [ask] recommends introducing your question before posting *any* code. — JaMiT, Dec 17 '22 at 01:46
`typedef struct`: You're writing a C++20 module; you're *really far* from a place where `typedef struct` is a useful construct. — Nicol Bolas, Dec 17 '22 at 01:48
*"I don't know if it's due to [...] or if it has something to do with templates since I'm using `auto...` to pass the parameter pack through to the implementation constructor."* -- this is testable. Remove `auto...` and the parameter pack -- does the error disappear? — JaMiT, Dec 17 '22 at 01:50
Do you need to mark the out of line definitions as `inline` like you would in a header? (Just a guess, I've not played with modules yet) — Alan Birtles, Dec 17 '22 at 07:08
That's a good point about removing the `auto...`, I didn't think of that. I'm updating the above question to take that out. — Teeeeeeeeeeeeeeeeeeeeeeeeeeeej, Dec 17 '22 at 15:22
@NicolBolas You say that but clearly I have no idea what you mean because I'm using this idiom still. I'd appreciate it if you pointed out where I can read up on why you think this is no longer a useful construct. — Teeeeeeeeeeeeeeeeeeeeeeeeeeeej, Dec 17 '22 at 15:25
@Teeeeeeeeeeeeeeeeeeeeeeeeeeeej: "*I'd appreciate it if you pointed out where I can read up on why you think this is no longer a useful construct.*" It's just a C-ism. C++ doesn't have a separate namespace for "struct"s the way C does. It serves no real purpose, and the few times it is theoretically useful can be served by just making a separate `using alias = type_name;` declaration. — Nicol Bolas, Dec 17 '22 at 15:32
I recall the reason I started doing this in my own code was to save myself from having to type "ClassName::impl" in a lot more places in the source file. If this is just a comment about style then /shrug. — Teeeeeeeeeeeeeeeeeeeeeeeeeeeej, Dec 17 '22 at 15:33
@AlanBirtles This suggestion fixed my issue. I'd appreciate if you put an answer down so I could mark it as the solution. — Teeeeeeeeeeeeeeeeeeeeeeeeeeeej, Dec 17 '22 at 16:00
@AlanBirtles: If that helps (as it seems to), that’s a compiler bug. Modules are not literally headers. — Davis Herring, Dec 17 '22 at 16:09
@Teeeeeeeeeeeeeeeeeeeeeeeeeeeej: "*I'd appreciate if you put an answer down so I could mark it as the solution.*" That does appear to solve the problem, but it *shouldn't*. This seems like a compiler bug, because using `= {}` instead of `= default;` for the constructor *also* makes the compiler shut up. As does `= default;`ing it in the class definition rather than outside of it. And none of that ought to matter. — Nicol Bolas, Dec 17 '22 at 16:58
I also think I was not understanding the difference between the "module interface unit" and "module implementation unit". I tried packing the whole thing into the module file, whereas I think I really should have broken up the declaration from the implementation by having just the declaration in the module, then implementation in a regular .cpp file. — Teeeeeeeeeeeeeeeeeeeeeeeeeeeej, Dec 17 '22 at 17:02
Although come to think of it, I was initially attempting to learn how to use modules in an effort to avoid having to maintain two files for each of my class definitions, but the way this is going it seems I'm just replacing my header files for module interface definitions. — Teeeeeeeeeeeeeeeeeeeeeeeeeeeej, Dec 17 '22 at 17:06
Please consider removing Windows/MSVC-specific stuff from your [mcve]. None of it should matter for the question. — n. m. could be an AI, Dec 17 '22 at 17:41
well if this is indeed a compiler bug then context of this being in MSVC is pretty relevant. — Teeeeeeeeeeeeeeeeeeeeeeeeeeeej, Dec 17 '22 at 17:55
@Teeeeeeeeeeeeeeeeeeeeeeeeeeeej: "*the way this is going it seems I'm just replacing my header files for module interface definitions.*" Um... why? You're the one who choose to put this stuff in out-of-line definitions. I don't really see how that's the problem of the module system. You are perfectly capable of putting everything in the same file if you want. You're also perfectly capable of not putting everything in the same file. How you use the system is up to you. — Nicol Bolas, Dec 17 '22 at 17:59
Is it a compiler bug? If only we had a quick way to check against some kind of independent, dare I say, validation mechanism... — n. m. could be an AI, Dec 17 '22 at 18:11

score 0 · Answer 1 · answered Dec 17 '22 at 17:05

This feels like a compiler bug. Here are a list of things that make the compiler cooperate, but none of them should matter:

Declaring the destructor declaration inline (inline ~OSFramework();).
Defining the out-of-line destructor inline (inline OSFramework::~OSFramework() = default;)
Defining the out-of-line destructor with = {} instead of = default;.
Moving the = default; constructor back to within the class definition.

None of these should fix anything. Member functions in modules are not implicitly inline, but that shouldn't cause a multiple-definition error. Importing a module is not like including a header.

And changing from = default; to = {} shouldn't change whether multiple definitions are created.

No, this is a compiler bug. I'd say that you should just = default it inline. It makes the code clearer anyway.

Teeeeeeeeeeeeeeeeeeeeeeeeeeeej · Answer 2 · 2022-12-17T18:10:19.700

I ended up separating implementation from interface:

winmain.cpp

#define WIN32_LEAN_AND_MEAN
#include <windows.h>

#include <memory>

import OSFrameworkWindows10Module;

int WINAPI wWinMain (
    _In_ HINSTANCE hinst,
    _In_opt_ HINSTANCE,
    _In_ PWSTR,
    _In_ int
)
{
    auto os_framework = std::make_unique<OSFramework>();
    return S_OK;
};

OSFrameworkWindows10Module.ixx

module;

#include <memory>

export module OSFrameworkWindows10Module;

export class OSFramework {
private:
    struct impl;
    std::unique_ptr<impl> _impl;
public:
    OSFramework();
    ~OSFramework();
};

OSFrameworkWindows10.cpp (new)

#include <memory>

import OSFrameworkWindows10Module;

using impl = struct OSFramework::impl {
    impl();
    ~impl();
};

impl::impl()
{}

impl::~impl() = default;

OSFramework::OSFramework() :_impl{ std::make_unique<impl>() } {}
OSFramework::~OSFramework() = default;

compiler output

1>------ Build started: Project: Vegas22Metroidvania1, Configuration: Debug x64 ------
1>Scanning sources for module dependencies...
1>OSFrameworkWindows10Module.ixx
1>Compiling...
1>OSFrameworkWindows10Module.ixx
1>OSFrameworkWindows10.cpp
1>winmain.cpp
1>Generating Code...
1>Vegas22Metroidvania1.vcxproj -> D:\projects\Vegas22Metroidvania1\x64\Debug\Vegas22Metroidvania1.exe

I want to thank @alanbirtles for the tip about the inline keyword when used in a header file, because this got me thinking about how I would normally do this just using a header file and a source file. This allowed me to re-read some blog posts about separating module interface and implementation and better understand what was being discussed.

"multiply defined symbols found" how is my destructor being defined twice?

2 Answers2