Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clang++ incorrectly rejects program containing nested classes and operator= overload #59684

Open
NobodyNada opened this issue Dec 23, 2022 · 2 comments
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party rejects-valid

Comments

@NobodyNada
Copy link

NobodyNada commented Dec 23, 2022

I'm seeing a strange compile error when trying to compile the bsnes-plus emulator. Here's the minimal excerpt of the source code that reproduces the problem:

class CPUcore {
public:
  struct flag_t {
    bool n, v, m, x, d, i, z, c;

    inline operator unsigned() const {
      return (n << 7) + (v << 6) + (m << 5) + (x << 4)
           + (d << 3) + (i << 2) + (z << 1) + (c << 0);
    }

    inline unsigned operator=(unsigned data) {
      n = data & 0x80; v = data & 0x40; m = data & 0x20; x = data & 0x10;
      d = data & 0x08; i = data & 0x04; z = data & 0x02; c = data & 0x01;
      return data;
    }

    inline unsigned operator|=(unsigned data) { return operator=(operator unsigned() | data); }
    inline unsigned operator^=(unsigned data) { return operator=(operator unsigned() ^ data); }
    inline unsigned operator&=(unsigned data) { return operator=(operator unsigned() & data); }

    flag_t() : n(0), v(0), m(0), x(0), d(0), i(0), z(0), c(0) {}
  };

  struct regs_t {
    int &a;
    flag_t p;
  };

  regs_t regs;
};

int main() {

}

The error is:

> clang++ test.cpp
test.cpp:17:56: error: no matching member function for call to 'operator='
    inline unsigned operator|=(unsigned data) { return operator=(operator unsigned() | data); }
                                                       ^~~~~~~~~
test.cpp:3:10: note: candidate function (the implicit move assignment operator) not viable: no known conversion from 'unsigned int' to 'CPUcore::flag_t' for 1st argument
  struct flag_t {
         ^
test.cpp:3:10: note: candidate function (the implicit copy assignment operator) not viable: no known conversion from 'unsigned int' to 'const CPUcore::flag_t' for 1st argument
  struct flag_t {
         ^
test.cpp:18:56: error: no matching member function for call to 'operator='
    inline unsigned operator^=(unsigned data) { return operator=(operator unsigned() ^ data); }
                                                       ^~~~~~~~~
test.cpp:3:10: note: candidate function (the implicit move assignment operator) not viable: no known conversion from 'unsigned int' to 'CPUcore::flag_t' for 1st argument
  struct flag_t {
         ^
test.cpp:3:10: note: candidate function (the implicit copy assignment operator) not viable: no known conversion from 'unsigned int' to 'const CPUcore::flag_t' for 1st argument
  struct flag_t {
         ^
test.cpp:19:56: error: no matching member function for call to 'operator='
    inline unsigned operator&=(unsigned data) { return operator=(operator unsigned() & data); }
                                                       ^~~~~~~~~
test.cpp:3:10: note: candidate function (the implicit move assignment operator) not viable: no known conversion from 'unsigned int' to 'CPUcore::flag_t' for 1st argument
  struct flag_t {
         ^
test.cpp:3:10: note: candidate function (the implicit copy assignment operator) not viable: no known conversion from 'unsigned int' to 'const CPUcore::flag_t' for 1st argument
  struct flag_t {
         ^
3 errors generated.

It appears the compiler is not recognizing the operator= overload.

Every component of the CPUcore declaration reproduced here is necessary to trigger the error -- the flag_t struct must be declared inside CPUcore, it must be used a member variable of regs_t alongside a reference variable, and the regs_t struct must be used as a member variable of CPUcore Changing any one of those conditions will cause the code to compile successfully.

I'm running macOS 13 on an M1 Pro. The bug is reproducible in LLVM 11.0 and newer when built from source or installed via Homebrew; bisecting finds the bug was introduced in d144601. Curiously, the bug is not reproducible on the version of Clang that ships with the Apple developer tools.

@EugeneZelenko EugeneZelenko added clang:frontend Language frontend issues, e.g. anything involving "Sema" and removed new issue labels Dec 24, 2022
@llvmbot
Copy link
Collaborator

llvmbot commented Dec 24, 2022

@llvm/issue-subscribers-clang-frontend

@AaronBallman AaronBallman added confirmed Verified by a second party rejects-valid labels Jan 24, 2023
@Abstract-Everything
Copy link
Contributor

I have attempted to look into this issue, the problem seems to be that CppLookupName only returns the implicitly declared operator=.

I have put up a diff on phabricator which resolves this issue. I am uncertain about my approach however. I would like to discuss the solution with developers with more experience in the codebase. https://reviews.llvm.org/D147782

In more detail. The following code sample reproduces the issue:

struct Outer {
  void invokeAssign() { operator=({}); }

  struct Inner {
    Inner &operator=(int);
    void invokeAssign(int D) { operator=(D); }
  };
};

int main() {}

First I will describe what happens if we omit Outer::invokeAssign from the sample and then describe why its inclusion causes the operator to not be found.

If we omit Outer::invokeAssign, CppLookupName will be called when compiling Inner::invokeAssign. This causes the implicit functions for Inner and then for Outer to be generated. These functions are stored in order inside of the IdResolver. The IdResolver is iterated in reverse order so CppLookupName first checks if the scope of Inner contains Outer::operator=, the last implicit operator generated, which it does not. It then continues on and calls LookupQualifiedName which finds the user defined operator and the program successfully compiles.

When we include Outer::invokeAssign the implicit Outer::operator= declarations are generated earlier. This time when we encounter the call to operator= inside of Inner::invokeAssign we just generate Inner::operator=, since the Outer ones are already generated, these are then appended to the IdResolver. Since we iterate backwards, this time we will check if Inner::operator= is contained in the Inner scope, which it is. Seeing as the loop found something, it is content and just returns those.

Thus the fact that IdResolver contains the declarations for Outer then the declarations for Inner causes the reverse interation to check the Inner ones first and just returns them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party rejects-valid
Projects
None yet
Development

No branches or pull requests

5 participants