Moving to Dynamic Polymorphism: Calling Overridden Functions from the Constructor

[samasri]

Motivation

As part of the Centre of Academic studies, a research group (myself, @snadi , @rwy0717 , @xliang6 , @nbhuiyan , @mgaudet ) is working on changing the variability implementation in OMR to use dynamic polymorphism instead of static polymorphism. We have already virtualized the overridden functions in some class hierarchies and created PRs for them (our PRs so far include: CodeGenerator, MemoryReference, and Symbol).

Consequences of static polymorphism can be found in an issue opened earlier by mgaudet as well as in our previous proposal discussed in the OMR Compiler Architecture Meeting. Please note that our proposal has slightly changed since the OMR Compiler Architecture Meeting; it was decided to change the proposal to moving completely to dynamic polymorphism (virtualize all overridden functions), instead of hybrid polymorphism, as was described in the proposal.

Challenge Faced

While virtualizing all overridden functions of the CodeGenerator class hierarchy, we realized that there are two overridden functions that are being called from the constructor: OMR::CodeGenerator::createLinkage(TR_LinkageConventions lc) and OMR::CodeGenerator::createLinkageForCompilation(). After virtualizing those functions, calling them from the constructor would not result in their most specific implementation to be triggered as explained in this isocpp FAQ.

Proposed Solution

In order to solve this, we propose and apply in our CodeGenerator PR the following solution:

1. Move all the code of the CodeGenerator constructor, from the point of calling any of the virtual functions (createLinkage or createLinkageForCompilation) to another function called initializeConstruction.
2. Call initializeConstruction() after every CodeGenerator constructor call. Since the OMR project only calls the CodeGenerator constructor once, here, we only had to add a call for initializeConstruction after that line.

@dsouzai, @fjeremic, @dibyendumajumdar, @0xdaryl: since you've commented or reacted to previous discussions about this, you might be interested in our proposed solution. We would appreciate your comments on it.

[dibyendumajumdar]

Hi, sorry I haven't looked at the code, so my comments are general ones. Presumably the problem is that a derived typed of CodeGenerator is being constructed? Is it possible to do the initialization in the derived constructor? I personally dislike the idea of performing initialization post construction.

[mgaudet]

I think this is an interesting notion; perhaps the general plan should be to transition to factory functions to allow this sort of two phase initialization?

[dibyendumajumdar]

Yes, as is done in Java apps, the implementation should be wired in somehow, maybe via configuration. Harder to do in C++ of course.

[samasri]

@dibyendumajumdar The problem is not that a constructor of a derived type is being called, it is the fact that the constructor of the derived type is calling virtual functions that are implemented in that same derived type. When a derived class's constructor calls a virtual function that is implemented in both, the base and derived classes. The call, in that case, is not resolved to the implementation in the derived class, instead, it is resolved to the implementation in the base class, because the function is virtual. This virtual function (and all the code related to it) is extracted to the initializeConstruction function that is then called after the constructor.

I agree that post construction has consequences, and thanks for pointing that out. As @mgaudet mentioned, it is a good idea in this case to transition to factory functions that allow such two phased initialization. However, it appears to me that CodeGenerator already has that factory function and this factory function (allocateCodeGenerator). And this is where we added our initializeConstruction call.

[0xdaryl]

I am in the process of adapting the CodeGenerator hierarchy construction to use a factory pattern. It will arrive in a series of staged PRs as there is work needed in consuming downstream projects to keep up with the changes. The goal of this initial work will be to simply replicate the functionality that is there but follow a factory allocation pattern that separates construction from initialization. Feedback on the PRs is certainly welcome.

In studying the initialization code there is obviously more work that could be done to streamline and logically order the initialization events, but that is not the subject of this work.

[0xdaryl]

With the merging of #5647 the CodeGenerator has moved to a factory construction pattern. The constructors simply perform member initialization now with the original initialization logic moved to a separate initialize() function. Closing this issue now.