[Added] ZScript: Allow covariant return types for virtual methods

Moderator: GZDoom Developers

ZScript: Allow covariant return types for virtual methods

Postby Player701 » Sat Nov 09, 2019 6:36 am

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 allExpand view
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
Player701
 
Joined: 13 May 2009
Location: Russia
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

Re: ZScript: Allow covariant return types for virtual method

Postby phantombeta » Sat Nov 09, 2019 9:48 am

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
phantombeta
In the meadow of sinful thoughts, every flower's a perfect one
 
Joined: 02 May 2013
Location: Brazil, South America, Earth, Orion-Cygnus Arm, Milky Way
Discord: phantombeta#2461
Twitch ID: phantombeta_
Github ID: Doom2fan
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

Postby Player701 » Sat Nov 09, 2019 9:58 am

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.
User avatar
Player701
 
Joined: 13 May 2009
Location: Russia
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

Re: ZScript: Allow covariant return types for virtual method

Postby _mental_ » Sat Nov 09, 2019 10:08 am

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.
_mental_
 
 
 
Joined: 07 Aug 2011

Re: ZScript: Allow covariant return types for virtual method

Postby Player701 » Sat Nov 09, 2019 10:42 am

I will wait with the PR until this one is resolved, since there might be conflicts.
User avatar
Player701
 
Joined: 13 May 2009
Location: Russia
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

Re: ZScript: Allow covariant return types for virtual method

Postby Player701 » Sun Nov 10, 2019 2:32 am

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 allExpand view
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
Player701
 
Joined: 13 May 2009
Location: Russia
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

Re: ZScript: Allow covariant return types for virtual method

Postby Graf Zahl » Sun Nov 10, 2019 2:39 am

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
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: ZScript: Allow covariant return types for virtual method

Postby Player701 » Sun Nov 10, 2019 2:45 am

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
Player701
 
Joined: 13 May 2009
Location: Russia
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

Re: ZScript: Allow covariant return types for virtual method

Postby Graf Zahl » Sun Nov 10, 2019 2:52 am

Yes, they should always match the current version.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: ZScript: Allow covariant return types for virtual method

Postby Player701 » Sun Nov 10, 2019 3:15 am

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
Player701
 
Joined: 13 May 2009
Location: Russia
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

Re: ZScript: Allow covariant return types for virtual method

Postby Major Cooke » Mon Aug 03, 2020 1:51 pm

Moved this to Code Submissions since it has one.

Once it's updated, I'll merge it into QZDoom for testing.
User avatar
Major Cooke
QZDoom Maintenance Team
 
Joined: 28 Jan 2007


Return to Closed Feature Suggestions

Who is online

Users browsing this forum: No registered users and 0 guests