On Thu, 24 Jul 2025, Jason Merrill wrote:

> On 7/23/25 8:29 PM, Patrick Palka wrote:
> > On Wed, 23 Jul 2025, Jason Merrill wrote:
> > 
> > > On 7/23/25 3:46 PM, Patrick Palka wrote:
> > > > As a follow-up to r16-2448-g7590c14b53a762, this patch attempts to teach
> > > > build_min_non_dep_op_overload how to rebuild all rewritten comparison
> > > > operators, not just != -> == ones, so that we don't incorrectly repeat
> > > > the unqualified name lookup at instantiation time.
> > > 
> > > Talking about mangling earlier made me wonder how we were handling
> > > non-dependent operator expressions, and indeed it seems we get it wrong
> > > since
> > > GCC 6:
> > > 
> > > struct A { };
> > > A operator+(A,A);
> > > template <class T>
> > > void f(decltype(T(),A()+A())) { }
> > > int main()
> > > {
> > >    f<int>(A()); // oops, mangles as operator+(A(),A()) instead of A()+A()
> > > }
> > > 
> > > while clang and EDG corretly use the latter mangling.
> > > 
> > > With the current code I would think we could fix this by handling
> > > CALL_EXPR_OPERATOR_SYNTAX in mangle.cc, but your patch (and indeed the
> > > earlier
> > > one) would further obscure the original syntax.
> > 
> > Does this mean it's also incorrect to mangle the ordinary non-dependent f(0)
> > call in:
> > 
> >      template<class T> void f(T);
> > 
> >      template<class T> decltype(T(),f(0)) g();
> > 
> >      int main() {
> >        g<int>();
> >      }
> > 
> > as f<int>(0) i.e. with an explicit template argument list even though it was
> > written without one?  Clang mangles it as f<int>(0) too, not sure about EDG.
> > This changed in GCC 12 with the non-dependent overload set pruning
> > optimization.
> > 
> > And does this have any declaration matching implications?  Say for
> > 
> >      struct A { };
> > 
> >      template<class T> int operator+(A,T);
> >      template<class T> decltype(T(),A()+A()) f();
> > 
> >      A operator+(A,A);
> >      template<class T> decltype(T(),A()+A()) f();
> > 
> >      int main() {
> >        f<int>();
> >      }
> > 
> > should we still reject the f<int>() call as ambiguous, or treat the second
> > declaration as a redeclaration (since they have the same mangling?)  This
> > seems
> > related to CWG1321 but for non-dependent calls.
> 
> Indeed.  In general there's a tension between wanting to mangle the expression
> as written and wanting to mangle the results of name binding;
> neither answer is completely correct, since both affect the ODR, but leaning
> toward the latter seems less likely to produce harmful collisions.  See also
> https://github.com/itanium-cxx-abi/cxx-abi/issues/38 for instance.
> 
> > I'm not sure how or if we want to address the mangling concern.
> > With this patch we'll now at least our non-dependent operator
> > expression mangling will be consistent :)
> 
> I suppose open and suspend a PR.
> 
> Perhaps -fabi-version should control this transformation?  Maybe only if
> cp_unevaluated_operand?

Depending on -fabi-version seems kinda overkill to me -- this would only
change the mangling of (a likely tiny fraction of) C++20 code support
for which is still considered experimental, and at the same time this
change fixes a much more likely to encounter name lookup bug.  (Indeed
the inadvertent GCC 6/12 mangling changes were never reported upstream
yet PR121179 was!)  And -fabi-version usually controls only mangling,
whereas here it'd control mangling and semantics (name lookup), which
sounds unprecedented/undesirable?

Reply via email to