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

Type mismatches when using Interfaces with _NoncopyableType decoration #3898

Closed
sriramm-nv opened this issue Apr 5, 2024 · 3 comments
Closed
Labels
goal:quality & productivity Quality issues and issues that impact our productivity coding day to day inside slang kind:enhancement a desirable new feature, option, or behavior

Comments

@sriramm-nv
Copy link
Collaborator

Creating this as a child issue of #3671 where the problem was originally found while writing the following targeted test:

Test Case: ctor-retval-legal.slang.txt
The root-cause is that there is a mismatch between the call operands and the callee signature when invoking a constructor or a member function within a struct that is derived from an interface.

Something like this:

interface IFoo
{
    ...
}

[__NonCopyableType] struct Impl : IFoo
{
     ...
}

The function signature of the following
__init(float);
__init();

are represented as

  • for ordinary constructor:
{272}	func %Implx5Fx24init	: Func(%Impl, Float)
    ....
	{329}	return_val(%21)

{234}	func %Implx5Fx24init1	: Func(%Impl)
    ...
	{338}	return_val(%23)
  • for NonCopyableType constructor
{277}	func %Implx5Fx24init	: Func(Void, Float, Ref(%Impl))
    ....
 	{333}	return_val(void_constant)

{236}	func %Implx5Fx24init1	: Func(Void, Ref(%Impl))
    ....
	{341}	return_val(void_constant)`

Further digging down to the kIROp_Call where the crash happens, there is only one operand set as kIROp_Func with the assumption of ordinary struct type function signatures

{234}	func %Implx5Fx24init1	: Func(%Impl)

but for this case, it should be two:

{236}	func %Implx5Fx24init1	: Func(Void, Ref(%Impl))

that is why the assertion happens, when we do a

auto arg = inst->getArg(argCounter++);

when argCounter is 1 to access the Ref(%Impl), but the total args is 1.

Some comments from Yong on this topic:
The problem is that in the generic function, T is assumed to be an ordinary type, so the call to ctor is in the ordinary form. When we substitute the callee with the actual ctor, that will cause a mismatch. In general, the function signature defined in the interface must be binary compatible with the function signature of the actual implementation. In the case of non copyablr type, this assumption is broken.
The right fix is for the type system to not think that a noncopyable type’s ctor can satisfy the ctor requirement defined in the interface. Or we need to make compiler understand what it means for an interface to be decorated noncopyable, and only allow a noncopyable type to implement a noncopyable interface.

@sriramm-nv sriramm-nv added kind:bug something doesn't work like it should priority:low things we'll get to if we can GoodFirstBug Great bug for people getting going in slang codebase labels Apr 5, 2024
@csyonghe
Copy link
Collaborator

csyonghe commented Apr 5, 2024

The issue here is calling for some language design around non copyable types.

Copyability is currently not a concept modeled in the type system, and we generally assume all types are copyable.
When a builtin type is decorated as [NonCopyableType], we enable some special IR gen rules to avoid generating copies, but this is completely transparent to the type system, and therefore leading to issues such as when specializing a generic function with non copyable type.

WIthout extending the type system to reason about copyability, our best solution here is to disallow non copyable type to implement any interface.

@sriramm-nv sriramm-nv added kind:enhancement a desirable new feature, option, or behavior and removed kind:bug something doesn't work like it should GoodFirstBug Great bug for people getting going in slang codebase labels Apr 5, 2024
@swoods-nv swoods-nv added goal:quality & productivity Quality issues and issues that impact our productivity coding day to day inside slang and removed priority:low things we'll get to if we can labels Apr 11, 2024
@swoods-nv
Copy link
Collaborator

Leaving this as backlog for now, because this is an internal compiler interface, but should get cleaned up once we're on to tail end test perfection.

@swoods-nv
Copy link
Collaborator

Compiler internals only, no plan to address, so closing as not planned.

@swoods-nv swoods-nv closed this as not planned Won't fix, can't repro, duplicate, stale Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
goal:quality & productivity Quality issues and issues that impact our productivity coding day to day inside slang kind:enhancement a desirable new feature, option, or behavior
Projects
None yet
Development

No branches or pull requests

3 participants