fbc icon indicating copy to clipboard operation
fbc copied to clipboard

fbc: add static operator to user defined types

Open jayrm opened this issue 5 years ago • 6 comments

Operator overloads declared in a UDT produce the error message: Operator cannot be a member function (TODO)

For example,

type T
    __ as integer
    declare operator len( byref arg T ) as uinteger
end type

'' output
'' todo.bas(3) error 153: Operator cannot be a member function (TODO), before '(' in 'declare operator len( byref arg T ) as uinteger'

This change includes the following:

  • allow declare static operator in UDT
  • operators are (still) added to global overloaded operator lookup
  • overloaded operators respect visibility (public, protected, private)
  • static operators must have at least one parameter with same type as parent
  • new error messages FB_ERRMSG_OPERATORMUSTBESTATIC and FB_ERRMSG_ATLEASTONEPARAMMUSTBEPARENTTYPE

Fixes issue #903 Operator cannot be a member function (TODO)

Related (not addressed in this PR): #902 Overloaded operators do not respect namespace

jayrm avatar May 25 '19 15:05 jayrm

Latest force push only to rebase on to current master.

jayrm avatar Jun 01 '19 22:06 jayrm

@dkl, I requested review from you for this change. I know you did a lot of work on classes and operator overloading. This change does not exactly follow g++, but in my opinion, not entirely illegal either. I welcome any criticism of code, but mostly I want to know your thoughts on the idea of static operators. Is this an acceptable first step? Thanks.

jayrm avatar Jun 01 '19 22:06 jayrm

I don't know what advantage there is to allowing this (static i.e. non-member operator overloads inside UDT/class) particularly for operators, but maybe it's about the same as for static member methods in general: the UDT/class acting as namespace, and the member visibility.

So for that reason it seems quite logical to allow it. Not sure why C++ apparently didn't allow it.

dkl avatar Jun 06 '19 16:06 dkl

The advantages for this PR change are small, but notable, I think:

  • static, in my opinion, is better than non-static overloaded member, since for binary operators, can make operator commutative. This is not possible with non-static since the implicit instance parameter must always be first.
  • static overloaded operators produce a similar procedure signature as c++ non-static overloaded operator members; Compatible linkage, though not compatible syntax fbc versus c++. Also maybe not compatible semantics for virtual overloaded operators (not sure on this one as I am not confident on the benefits/usage in c++ to say).
  • by method of declaration (static overloaded member), gives access to private data.
  • semantically in fbc, the overloaded operator is grouped (by type declaration) with the type it is related to.

If I had to guess why static operator it is disallowed in c++, I would guess that can not have both a static overloaded operator and a non-static overloaded operator taking the same parameters, since would produce same mangled name/function signature and therefore a duplicate definition, so the the design choice was to disallow static overloaded operators, and instead prefer the logic for virtual overrides on derived types. Though I'm not sure how much sense that makes, since the all the virtual procs would need to return same type, even in the derived types.

For comparison consider this c++ snippet:

class T {
	public:

	float number;

	// mangled as T::operator+(float)
	// has instance parameter, therefore mangled with "namespace" T
	// however, overloaded operator is still automatically imported
	// to global namespace

	T operator +( float k )
	{
		T ret;
		ret.number = this->number + k;
		return ret;
	}
};

// mangled as operator+(T&, float)
// no duplicate defintion here, though it's in the global namespace
T operator +( T& lhs, float rhs )
{
	T ret;	
	ret.number = lhs.number + rhs;
	return ret;
}

// allow overloaded operator to be commutative be swapping parameters
// - which is not possible if only defined taking instance parameter in class T (as in c++)
// mangled as operator+(float, T&)
T operator +( float lhs, T& rhs )
{
	T ret;	
	ret.number = lhs + rhs.number;
	return ret;
}

int main() {
	T a, b;
	// error: ambiguous overload for 'operator+'
	// what is syntax to resolve this ambiguity?
	b = a + 5.0;
	return 0;
}

While c++ allows both a overload operator member and a global overloaded operator taking the same type, resulting in different mangled names, I'm not sure this would ever been seen in actual code since I don not think there is anyway to resolve the ambiguity.

For comparison, with the this PR change, here's what we can do :

type T
	number as single
	'' non-static operator with implicit instance parameter 
	'' - not currently allowed in fbc -- must be static with this PR change
	'' declare operator +(rhs as single) as T

	'' mangled as T::operator+(T&, float)
	declare static operator +( byref lhs as T, rhs as single ) as T

	'' mangled as T::operator+(float, T&)
	'' allow commutative
	declare static operator +( lhs as single, byref rhs as T ) as T
end type

/'
'' global version, will produce duplicate definition in fbc
'' even though mangled name would be different
'' while c++ allows this, there is no way I know of to 
'' disambiguate in c++ anyway, so should be OK to allow
'' allow one or the other but not both in fbc.

'' mangled as operator+(T&, float)
operator +( byref lhs as T, rhs as single ) as T
	operator = type<T>(lhs.number + rhs )
end operator

'' mangled as operator+(float, T& float)
operator +( lhs as single, byref rhs as T ) as T
	operator = type<T>(lhs + rhs.number)
end operator
'/

'' mangled as T::operator+(T&, float)
static operator T.+( byref lhs as T, rhs as single ) as T
	operator = type<T>(lhs.number + rhs )
end operator

'' mangled as T::operator+(float, T&)
static operator T.+( lhs as single, byref rhs as T ) as T
	operator = type<T>(lhs + rhs.number)
end operator


dim x as T, k as single
dim y as T

y = x + k
y = k + x

jayrm avatar Jun 08 '19 14:06 jayrm

From previous post:

static overloaded operators produce a similar procedure signature as c++ non-static overloaded operator members; Compatible linkage, though not compatible syntax fbc versus c++

Correction: My own comments in the example code show this statement to be incorrect. Somehow I missed the mismatch between the overloaded operator signatures:

T operator +( float k ) which is mangled as T::operator+(float)

versus

declare static operator +( byref lhs as T, rhs as single ) as T which is mangled as T::operator+(T&, float)

So for compatibility with c++ (for example writing a compatible binding), we would still need to allow non-static operator declaration in fbc at some point to produce the correct matching function signature for linkage to c++.

Therefore, fbc should probably have a custom mangling scheme for static overloaded operators to prevent any collision with c++ mangling rules.

jayrm avatar Jun 08 '19 17:06 jayrm

@dkl, thank you for your insights. Very helpful to keep this moving forward.

I started writing tests for fbc/gcc name mangling of member procs, then realized that don't have thiscall calling convention. I might take a side trip for that, even if only for gcc backend, haven't decided yet, then return to this PR. So this PR might hang open for a while.

And I need to rethink the logic of static and self-op. In my head, just now, I realize I am confusing Self-Op with non-static.

Should have, I think:

  • Self-ops, non-static only, c++ compatible name mangling
  • Non-Self-ops, non-static, c++ compatible name mangling
  • Non-Self-ops, static, fb custom name mangling

And will have to take care to detect the duplicate definitions, as there are multiple forms for the operator in the global namespace.

jayrm avatar Jun 26 '19 04:06 jayrm