-2

I was trying to do an exercise on Professional C++ 5th edition, and I met a problem. I have defined Person::Impl in Person.cpp, but MSVC always shows me it's undefined.

I have these files:

CMakeLists.txt

CMAKE_MINIMUM_REQUIRED(VERSION 3.26)

set(CMAKE_EXPERIMENTAL_CXX_MODULE_CMAKE_API "2182bf5c-ef0d-489a-91da-49dbc3090d2a")
set(CMAKE_EXPERIMENTAL_CXX_MODULE_DYNDEP ON)

set_property(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} PROPERTY VS_STARTUP_PROJECT ${CMAKE_PROJECT_NAME})

set(CMAKE_CXX_STANDARD 23)
project(9-4)

add_executable(${CMAKE_PROJECT_NAME})
target_sources(${CMAKE_PROJECT_NAME}
    PUBLIC
    FILE_SET all_my_modules TYPE CXX_MODULES FILES
    main.cpp
    Person.cppm
    PRIVATE
    Person.cpp
)

Person.cppm

export module person;

import <string>;
import <memory>;

export class Person{
    public:
        Person(const std::string_view firstName,const std::string_view lastName);
        Person(const std::string_view firstName,const std::string_view lastName,const char firstUnitOfName);
        Person()=default;
        std::string getFirstName() const;
        std::string getLastName() const;
        char getFirstUnitOfName() const;
        void setFirstName(const std::string_view firstName);
        void setLastName(const std::string_view lastName);
        void setFirstUnitOfName(const char c);
    private:
        class Impl;
        std::unique_ptr<Impl> m_impl;
};

Person.cpp

module person;

import <string>;

using namespace std;

class Person::Impl
{
private:
    string m_FirstName;
    string m_LastName;
    char m_FirstUnitOfName;

public:
    Impl() = default;
    Impl(const string_view firstName, const string_view lastName) : m_FirstName(firstName), m_LastName(lastName), m_FirstUnitOfName(firstName.front()){};
    Impl(const string_view firstName, const string_view lastName,const char c) : m_FirstName(firstName), m_LastName(lastName), m_FirstUnitOfName(c){};
    string getFirstName() const
    {
        return m_FirstName;
    };
    string getLastName() const
    {
        return m_LastName;
    };
    char getFirstUnitOfName() const
    {
        return m_FirstUnitOfName;
    };
    void setFirstName(const string_view firstName)
    {
        m_FirstName = firstName;
    }
    void setLastName(const string_view lastName)
    {
        m_LastName = lastName;
    }
    void setFirstUnitOfName(const char c)
    {
        m_FirstUnitOfName = c;
    }
};

Person::Person(const std::string_view firstName, const std::string_view lastName) : m_impl{make_unique<Impl>(firstName, lastName)} {};
Person::Person(const string_view firstName, const string_view lastName,const char firstUnitOfName) : m_impl{make_unique<Impl>(firstName, lastName,firstUnitOfName)}{};

std::string Person::getFirstName() const{
    return m_impl->getFirstName();
};
std::string Person::getLastName() const{
    return m_impl->getLastName();
};
char Person::getFirstUnitOfName() const{
    return m_impl->getFirstUnitOfName();
};
void Person::setFirstName(const std::string_view firstName){
    m_impl->setFirstName(firstName);
};
void Person::setLastName(const std::string_view lastName){
    m_impl->setLastName(lastName);
};
void Person::setFirstUnitOfName(const char c){
    m_impl->setFirstUnitOfName(c);
};
Person::~Person() = default;

main.cpp

import <iostream>;
import <format>;
import <string>;
import person;
using namespace std;

int main(){
    Person testman("Test","Man");
    cout<<format("The man's firstname is {},lastname is {},first unit is {}.\n",testman.getFirstName(),testman.getLastName(),testman.getFirstUnitOfName());
    testman.setFirstName("Man");
    testman.setLastName("Test");
    testman.setFirstUnitOfName('T');
    cout<<format("The man's firstname is {},lastname is {},first unit is {}.\n",testman.getFirstName(),testman.getLastName(),testman.getFirstUnitOfName());

I have seen the book writer Marc's answer, but I sadly failed to find where it is wrong(not exactly after being tutoring, see below), because C++ modules seem to be too new, and there isn't any difference between mine and Marc's answer.

The whole error message is:

[build] C:\Environment\VisualStudio\2022\Community\VC\Tools\MSVC\14.35.32215\include\memory(3065,1): error C2027: Used the undefined type "Person::Impl" [C:\Users\leen\workspace\cpp\professional_c++_answer\chapter-9\9-4\build\9-4.vcxproj]
[build] C:\Users\leen\workspace\cpp\professional_c++_answer\chapter-9\9-4\Person.cppm(18,15): message: See the declaration of "Person::Impl" [C:\Users\leen\workspace\cpp\professional_c++_answer\chapter-9\9-4\build\9-4.vcxproj]
[build] C:\Environment\VisualStudio\2022\Community\VC\Tools\MSVC\14.35.32215\include\memory(3064,1): message: When compiling class template member functions "void std::default_delete<Person::Impl>::operator ()(_Ty *) noexcept const" [C:\Users\leen\workspace\cpp\professional_c++_answer\chapter-9\9-4\build\9-4.vcxproj]
[build]           with
[build]           [
[build]               _Ty=Person::Impl
[build]           ]
[build] C:\Environment\VisualStudio\2022\Community\VC\Tools\MSVC\14.35.32215\include\memory(3177,33): message: View a reference to the function template instantiation "void std::default_delete<Person::Impl>::operator ()(_Ty *) noexcept const" being compiled [C:\Users\leen\workspace\cpp\professional_c++_answer\chapter-9\9-4\build\9-4.vcxproj]
[build]           with
[build]           [
[build]               _Ty=Person::Impl
[build]           ]
[build] C:\Environment\VisualStudio\2022\Community\VC\Tools\MSVC\14.35.32215\include\memory(3101,1): message: View a reference to the class template instantiation "std::default_delete<Person::Impl>" being compiled [C:\Users\leen\workspace\cpp\professional_c++_answer\chapter-9\9-4\build\9-4.vcxproj]
[build] C:\Users\leen\workspace\cpp\professional_c++_answer\chapter-9\9-4\main.cpp(8,1): message: View class template instantiation of are compiling "STD: : unique_ptr < Person: : Impl, STD: : default_delete < Person: : Impl > >" [C:\Users\leen\workspace\cpp\professional_c++_answer\chapter-9\9-4\build\9-4.vcxproj]
[build] C:\Environment\VisualStudio\2022\Community\VC\Tools\MSVC\14.35.32215\include\memory(3065,25): error C2338: static_assert failed:  'can't delete an incomplete type' [C:\Users\leen\workspace\cpp\professional_c++_answer\chapter-9\9-4\build\9-4.vcxproj]
[build] Build finished with exit code -1

Added as being required after solved:

If you rewrite the above in the all old #include fashion, MSVC will show:

error C2600: "Person::~Person": Cannot define special compiler-generated member functions (must first be declared in the class)

It's like Forward declaration with unique_ptr.

mkrieger1
  • 19,194
  • 5
  • 54
  • 65
Leen Hawk
  • 101
  • 7
  • 5
    This question is currently [being discussed on the meta site](https://meta.stackoverflow.com/q/424703/) – Hovercraft Full Of Eels May 14 '23 at 13:30
  • What does "Marc's answer" refer to? There is (currently) only one answer to this question, by user "ComicSansMS" (real name *Andreas Weis*)? E.g., does it refer to an answer to another Stack Overflow question? – Peter Mortensen May 15 '23 at 11:10
  • Emm, the writer of professional c++ is Marc... as mentioned writer Marc... Sorry for ambiguous expression. @PeterMortensen – Leen Hawk May 15 '23 at 14:49

1 Answers1

1

You need to add the declaration for the destructor to the class definiton of Person:

Person.cppm

export class Person{
    public:
        ~Person();
[...]

The situation is similar to this code in a pre-modules world. If the destructor is not included in the class definition, the compiler will assume that you want a compiler-defined implicit destructor. This will fail, because that destructor would need to be generated in the context of main.cpp, where the type Person::Impl is not known.

The fact that MSVC accepted your code as-is, with the destructor definition in the module implementation, but no declaration of the destructor in the class definition looks weird to me. This might actually be a compiler bug (though I'll have to dig through the standard wording first to confirm).

Alternatively, you can also move the entire class definition for Person::Impl into the primary module interface without exporting it. Clients of person will still not be able to instantiate that class because of it being declared as a private nested class, but it will provide sufficient information for the compiler to instantiate ~Person() in the context of main.cpp.

ComicSansMS
  • 51,484
  • 14
  • 155
  • 166
  • 5
    @LeenHawk: "it doesn't work" tells us *nothing* of use. How doesn't this suggestion work? What exactly have you tried? Please understand that ComicSansMS went to bat for you to get your question re-opened, and so it is time that you up your game and clarify your issues. – Hovercraft Full Of Eels May 14 '23 at 16:49
  • @HovercraftFullOfEels emm, the error message is same as I mentioned above and not changed... I'm sure that it work for old `#include` way but I don't know much about how to adjust cmake and something else to use module in a correct way – Leen Hawk May 14 '23 at 16:51
  • @HovercraftFullOfEels As I change to old `#include` way, the msvc will show:`error C2600: "Person::~Person": Cannot define special compiler-generated member functions (must first be declared in the class)` and in module way, it only show as error c2027. Thank you. I copy wrong to `~Person()=default;` of ComicSansMS – Leen Hawk May 14 '23 at 17:09
  • @LeenHawk This definitely works with modules, I made sure and tested your example with MSVC myself before writing the answer. – ComicSansMS May 15 '23 at 06:36
  • 1
    Yes, I commented twice to express it. Only difference is msvc won't show c2600. I forget to delete `it doesn't work`,sorry... – Leen Hawk May 15 '23 at 08:14