ZScript: Allow covariant return types for virtual methods

Moderator: GZDoom Developers

User avatar
Player701
 
 
Posts: 1394
Joined: Wed May 13, 2009 3:15 am
Discord: Player701#8214
Operating System: Windows 10/8.1/8/201x 64-bit
OS Test Version: No (Using Stable Public Version)
Graphics Processor: nVidia with Vulkan support
Location: Russia

ZScript: Allow covariant return types for virtual methods

Post by Player701 »

Current behavior:

When overriding a virtual function in a subclass, the overridden function must have the exact same return type as the one the parent virtual function does. For example, the following ZScript will not compile:

Code: Select all

class Base1
{
    virtual Base2 GetSomething()
    {
        return null;
    }
}

class Child1 : Base1
{
    override Child2 GetSomething() // Script error: Attempt to override non-existent virtual function GetSomething
    {
        return null;
    }
}

class Base2
{
}

class Child2 : Base2
{
}
Suggested new behavior:

Allow the return type of the overridden function to be either the same as, or a subtype of the parent function's return type.

Justification:

Imagine I'm working with an instance of Child1. If I needed to make use of the extra functionality of the type Child2 that is returned by Child1::GetSomething, the only solution would be to cast the returned Base2 to Child2. This breaks encapsulation and is also dangerous, because if Child1::GetSomething is changed to return something other than a Child2, the cast will return null, and a VM abort could be triggered.

However, if Child1 could advertise right away that its GetSomething returns not just a Base2 but actually a Child2, then changing the return type would trigger a compile error, which could make it easier to avoid potential bugs.
User avatar
phantombeta
Posts: 1982
Joined: Thu May 02, 2013 1:27 am
Operating System: Windows 10/8.1/8/201x 64-bit
OS Test Version: No (Using Stable Public Version)
Graphics Processor: nVidia with Vulkan support

Re: ZScript: Allow covariant return types for virtual method

Post by phantombeta »

IMO this is not a good idea. Virtual functions should not be modifiable like this. As much as I like things that make it easier to code and catch bugs, I really don't think something like this should be added.
User avatar
Player701
 
 
Posts: 1394
Joined: Wed May 13, 2009 3:15 am
Discord: Player701#8214
Operating System: Windows 10/8.1/8/201x 64-bit
OS Test Version: No (Using Stable Public Version)
Graphics Processor: nVidia with Vulkan support
Location: Russia

Re: ZScript: Allow covariant return types for virtual method

Post by Player701 »

I'm not sure if there is any actual danger in adding this, and it might make code easier to read and understand. In fact, I think I already know how to implement it, and was preparing a PR when you wrote this... But there may be some subtle pitfalls I'm not seeing, since I only resumed coding in ZScript a week ago. I'm also currently developing an Android app in Java at work, and I know that Java is one such language that has covariant return types, and there were some use cases where they came in handy. In my current ZScript project I have a similar use case, and that's why I remembered about this feature.
_mental_
 
 
Posts: 3771
Joined: Sun Aug 07, 2011 4:32 am

Re: ZScript: Allow covariant return types for virtual method

Post by _mental_ »

phantombeta wrote:IMO this is not a good idea. Virtual functions should not be modifiable like this. As much as I like things that make it easier to code and catch bugs, I really don't think something like this should be added.
C++ allows covariant return types. This feature is pretty useful in real world programs. So called virtual constructor is one of its applications.
User avatar
Player701
 
 
Posts: 1394
Joined: Wed May 13, 2009 3:15 am
Discord: Player701#8214
Operating System: Windows 10/8.1/8/201x 64-bit
OS Test Version: No (Using Stable Public Version)
Graphics Processor: nVidia with Vulkan support
Location: Russia

Re: ZScript: Allow covariant return types for virtual method

Post by Player701 »

I will wait with the PR until this one is resolved, since there might be conflicts.
User avatar
Player701
 
 
Posts: 1394
Joined: Wed May 13, 2009 3:15 am
Discord: Player701#8214
Operating System: Windows 10/8.1/8/201x 64-bit
OS Test Version: No (Using Stable Public Version)
Graphics Processor: nVidia with Vulkan support
Location: Russia

Re: ZScript: Allow covariant return types for virtual method

Post by Player701 »

Okay, that other PR has been resolved, but there is still something important that has to be considered.

Take a look at the following ZScript:

Code: Select all

class A
{
    virtual C GetSomething()
    {
        ...
    }
}

class B : A
{
    D GetSomething() // Note: not virtual
    {
        ...
    }
}

class C
{
}

class D : C
{
}
As of now, this code compiles without errors. However, if the changes to implement this feature, they way I've coded it, are introduced, compilation will fail because of D::GetSomething missing the override qualifier.

This can be mitigated with a ZScript version bump. PClass::FindVirtualIndex could use the old, exact-matching algorithm to check return values in the old version, and the new algorithm to allow covariant return types in the new version.

If there are any objections to this particular issue, then I will probably give up on the PR, since otherwise it will require more extensive changes to ZCCCompiler::CompileFunction with a much greater risk to break something unintentionally. But then, I hope, it could eventually be done later by someone more experienced in this matter.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 47983
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: ZScript: Allow covariant return types for virtual method

Post by Graf Zahl »

Yes, that needs to be versioned. I'd consider that case incorrect, but if it passed the compiler it needs to remain operational for old script versions.
User avatar
Player701
 
 
Posts: 1394
Joined: Wed May 13, 2009 3:15 am
Discord: Player701#8214
Operating System: Windows 10/8.1/8/201x 64-bit
OS Test Version: No (Using Stable Public Version)
Graphics Processor: nVidia with Vulkan support
Location: Russia

Re: ZScript: Allow covariant return types for virtual method

Post by Player701 »

Okay. Should I also increase the version number in GZDoom's built-in script files? I did some tests yesterday, and there weren't any compilation errors.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 47983
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: ZScript: Allow covariant return types for virtual method

Post by Graf Zahl »

Yes, they should always match the current version.
User avatar
Player701
 
 
Posts: 1394
Joined: Wed May 13, 2009 3:15 am
Discord: Player701#8214
Operating System: Windows 10/8.1/8/201x 64-bit
OS Test Version: No (Using Stable Public Version)
Graphics Processor: nVidia with Vulkan support
Location: Russia

Re: ZScript: Allow covariant return types for virtual method

Post by Player701 »

Okay then, here it is. I hope I haven't overlooked anything, as it didn't seem to be that difficult to implement. If I have... well, at least I tried. :P
User avatar
Major Cooke
Posts: 8058
Joined: Sun Jan 28, 2007 3:55 pm

Re: ZScript: Allow covariant return types for virtual method

Post by Major Cooke »

Moved this to Code Submissions since it has one.

Once it's updated, I'll merge it into QZDoom for testing.

Return to “Closed Feature Suggestions”