[PATCH] D155387: [Clang] Fix member lookup so that we don't ignore ambiguous lookups in some cases

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 11 05:42:04 PDT 2023


ilya-biryukov added a comment.

This change seems to expose a bug in Clang in C++20 see godbolt <https://gcc.godbolt.org/z/o8E3nonG8>.
I believe following code should compile, but it does not:

  struct X {
    virtual ~X();
    virtual bool operator ==(X);
    bool operator !=(X);
  };
  
  struct Y {
    virtual ~Y();
    virtual bool operator ==(Y);
    bool operator !=(Y);
  };
  
  struct Z : X, Y {
    virtual bool operator==(Z);
    bool operator==(X) override;
    bool operator==(Y) override;
  };
  
  int main() {
    bool b = Z() == Z(); // Clang errors saying `lookup for operator != is ambiguous`
    (void)b;
  }

The `operator !=` is coming from checking if we need to add a reversed candidates.
I believe this should compile as according to (over.match.oper)p4 <https://eel.is/c++draft/over.match.funcs#over.match.oper-4>:

> A non-template function or function template F named operator== is a rewrite target with first operand o unless **a search for the name operator!= in the scope S** from the instantiation context of the operator expression finds a function …

And 'search' is defined without any checks for ambiguous bases in (basic.lookup.general)p3 <https://eel.is/c++draft/basic.lookup#general-3>

> A single search in a scope S for a name N from a program point P finds all declarations that precede P to which any name that is the same as N ([basic.pre]) is bound in S. If any such declaration is a using-declarator whose terminal name ([expr.prim.id.unqual]) is not dependent ([temp.dep.type]), it is replaced by the declarations named by the using-declarator ([namespace.udecl]).

Clang does a full lookup instead, therefore exposing the errors that it should not diagnose.

We actually hit this on real code <https://android.googlesource.com/platform/external/libabigail/+/c0d418081e5cab43d5266ca2e01e6e876b641be3/src/abg-ir.cc#15832> and found this during integration.

I have sent a fix for review in D157708 <https://reviews.llvm.org/D157708>.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155387/new/

https://reviews.llvm.org/D155387



More information about the cfe-commits mailing list