[Bug c++/100135] New: ICE when using constants in a mdoule

[Bug c++/100135] New: ICE when using constants in a mdoule

[patrick.kox at commandoregel dot be]

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

            Bug ID: 100135
           Summary: ICE when using constants in a mdoule
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: patrick.kox at commandoregel dot be
  Target Milestone: ---

Using the following code from the book (Professional C++ 5th Edition) by Marc
Grergoire causes an ICE when trying to compile the "Employee" module.

I have 2 files:
Employee.cpp (the Interface, called Employee.cppm in the book):
===
export module employee;
import<string>;
namespace Records {

const int DefaultStartingSalary{30'000};
export const int DefaultRaiseAndDemeritAmount{1'000};

export class Employee{
        public:
        Employee(const std::string& firstName, const std::string& lastName);

        void promote(int raiseAmount = DefaultRaiseAndDemeritAmount);
        void demote(int demeritAmount = DefaultRaiseAndDemeritAmount);
        void hire();            // Hires or rehires the employee
        void fire();            // Dismisses the employee
        void display() const;   // Display employee information on the console

        // Getters and setters
        void setFirstName(const std::string& firstName);
        const std::string& getFirstName() const;

        void setLastName(const std::string& lastName);
        const std::string& getLastName() const;

        void setEmployeeNumber(int employeeNumber);
        int getEmployeeNumber()const;

        void setSalary(int salary);
        int getSalary()const;

        bool isHired() const;

        private:
        std::string m_firstName;
        std::string m_lastName;
        int m_employeeNumber{-1};
        int m_salary{DefaultStartingSalary};
        bool m_hired{false};
};
}
===
And Employee-i.cpp (called Employee.cpp in the book):
===
module employee;
import<iostream>;
//import <format>;      // Not Available
using namespace std;

namespace Records {
Employee::Employee(const string& firstName, const string& lastName) :
m_firstName{firstName}, m_lastName{lastName}
{
}

void Employee::promote(int raiseAmount)
{
        setSalary(getSalary() + raiseAmount);
}

void Employee::demote(int demeritAmount)
{
        setSalary(getSalary() - demeritAmount);
}

void Employee::hire()
{
        m_hired = true;
}

void Employee::fire()
{
        m_hired = false;
}

void Employee::display() const
{
        cout << "Employee: " << getLastName() << ", " << getFirstName() <<
endl;
        cout << "-------------------------" << endl;
        cout << (isHired() ? "Current employee" : "Former employee") << endl;
        cout << "Employee number: " << getEmployeeNumber() << endl;
        cout << "Salary: $" << getSalary() << endl;
        cout << endl;
}

// Getters and setters
void Employee::setFirstName(const string& firstName)
{
        m_firstName = firstName;
}

const string& Employee::getFirstName() const
{
        return m_firstName;
}

void Employee::setLastName(const string& lastName)
{
        m_lastName = lastName;
}

const string& Employee::getLastName() const
{
        return m_lastName;
}

void Employee::setEmployeeNumber(int employeeNumber)
{
        m_employeeNumber = employeeNumber;
}
int Employee::getEmployeeNumber() const
{
        return m_employeeNumber;
}

void Employee::setSalary(int salary)
{
        m_salary = salary;
}

int Employee::getSalary() const
{
        return m_salary;
}

bool Employee::isHired() const
{
        return m_hired;
}
}
===
Note: import <format>; is commented out since this module is not available yet
(the code is also modified to use the "old cout" syntax and not the new
format(""); syntax.

The error message thie causes is:
Employee.cpp:19:14: error: ‘void Records::Employee::promote(int)’ references
internal linkage entity ‘const int Records::DefaultRaiseAndDemeritAmount’
   19 |         void promote(int raiseAmount = DefaultRaiseAndDemeritAmount);
      |              ^~~~~~~
Employee.cpp:20:14: error: ‘void Records::Employee::demote(int)’ references
internal linkage entity ‘const int Records::DefaultRaiseAndDemeritAmount’
   20 |         void demote(int demeritAmount = DefaultRaiseAndDemeritAmount);
      |              ^~~~~~
Employee.cpp:15:14: error: ‘class Records::Employee’ references internal
linkage entity ‘const int Records::DefaultStartingSalary’
   15 | export class Employee{
      |              ^~~~~~~~
Employee.cpp:4:8: error: failed to write compiled module: Bad file data
    4 | export module employee;
      |        ^~~~~~
Employee.cpp:4:8: note: compiled module file is ‘gcm.cache/employee.gcm’
In module imported at Employee-i.cpp:5:1:
employee: error: failed to read compiled module: No such file or directory
employee: note: compiled module file is ‘gcm.cache/employee.gcm’
employee: note: imports must be built before being imported
employee: fatal error: returning to the gate for a mechanical issue
compilation terminated.
make: *** [Makefile:55: gcm] Error 1

[Bug c++/100135] [modules] ICE when using constants in a module

[nicolas.werner at hotmail dot de]

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

Nicolas Werner <nicolas.werner at hotmail dot de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |nicolas.werner at hotmail dot de

--- Comment #1 from Nicolas Werner <nicolas.werner at hotmail dot de> ---
A more minimal example of the issue here, I think:

export module internalname;

constexpr int radix_16 = 16;

export {
  int foo(int in = radix_16) {
    return in;
  }
}

I *think* this is legal module code and MSVC as well as clang do accept it.
However it is a weird edge case. This also isn't exclusive to default function
arguments, you can trigger the same issue with templates:

export module internalname;

constexpr int radix_16 = 16;

template <class T, auto Param>
inline auto do_from_chars() -> T {
  if (Param > 4) {
    return 5;
  }
  else {
    return 4;
  }
}

export {
  template <class T> struct parse_number {
    auto operator()() -> T {
      return do_from_chars<T, radix_16>();
    }
  };
}

Specifically gcc seems to be overly strict here in how it handles internal
linkage entities, that are required on the call site for default parameters or
template instantiations.

[Bug c++/100135] [modules] ICE when using constants in a module

[nicolas.werner at hotmail dot de]

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

--- Comment #2 from Nicolas Werner <nicolas.werner at hotmail dot de> ---
Possibly the gcc behaviour here is correct. There is a clang bug open for it
not rejecting calls to functions with default parameters, when the default
parameter is not exported: https://github.com/llvm/llvm-project/issues/57459

However possibly that should only be an error at the call site instead of when
compiling the module with that function declaration, when the function
parameter can be legally specified at the call site. For example for foo(int i
= unexported_constant) a call to foo(5) could be legal, even if only foo but
not unexported_constant is exported? Not sure what the standard says on that,
it sounds similar to CWG2631.

[Bug c++/100135] [modules] ICE when using constants in a module

[ppalka at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |14.0
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=99232
                 CC|                            |ppalka at gcc dot gnu.org
         Resolution|---                         |FIXED
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #3 from Patrick Palka <ppalka at gcc dot gnu.org> ---
GCC 14 accepts this since the fix for PR99232.