[Sofa.Core] Implements a template deduction system in the object factory

[damienmarchal]

Currently canCreate is used for several things, one of the most common use is to implement template type deduction.

On this PR we implement a mechanism that will allow to separate the type deduction to actual possibilities of creation of the component.

---

By submitting this pull request, I acknowledge that
I have read, understand, and agree SOFA Developer Certificate of Origin (DCO).

---

Reviewers will merge this pull-request only if

• it builds with SUCCESS for all platforms on the CI.
• it does not generate new warnings.
• it does not generate new unit test failures.
• it does not generate new scene test failures.
• it does not break API compatibility.
• it is more than 1 week old (or has fast-merge label).

[alxbilger]

So, if I understand correctly, you implemented 2 methods to deduce the template: 1) a static method in the component, 2) an additional line when registering in the object factory.

[damienmarchal]

So, if I understand correctly, you implemented 2 methods to deduce the template: 1) a static method in the component, 2) an additional line when registering in the object factory.

Yes, despite it is possible to have only the system with the static method I find the explict method more clear despite it does not allow to cover the same set of feature. We may remove it if needed.

[damienmarchal]

[ci-build][with-scene-tests]

[damienmarchal]

@alxbilger do you want me to remove the approach based on explicit registration so that there is only one single way of doing that ?

[alxbilger]

Do you consider that this PR supersedes #3309 ?

[damienmarchal]

You mean in term of what priority for merging ?

[alxbilger]

I mean they both solve the same problem using different approaches, right?

[damienmarchal]

Not exactly, the first is about refactoring canCreate and create while the other is about providing a system for template deduction.
It seems connected because canCreate is widely used to implement template deduction (but it is not its sole purpose). If template deduction is merged this will remove a lot of canCreate... but some may still. So some ideas in #3309 may still have values independently of #3938.

[hugtalbot]

[ci-build][with-all-tests]

[damienmarchal]

@hugtalbot do we move forward on that one ?

[alxbilger]

The "problem" I see when the template deduction is set in the registry, is for the SofaCUDA plugin, where the component is added in the registry a second time with different template parameters. If I understood correctly, the components from SofaCUDA will not "inherit" from the same deduction rule, creating an inconsistency. So it must be added again in SofaCUDA, but it is easy to forget...

[damienmarchal]

I see two way of fixing that:
clean the object factory implementation so there is one deduction rule per component registered (which actually that it should be). There exists several previous experimental PR on how we could refactor the object factory but this is far beyond the template deduction system. And thus not recommending coupling these two topics.
only use the static method way of defining a template deduction rule. It make this PR simpler :)

EDIT: The templateDeductionMethod (TDM) is shared by all components registered with the same className. In the use case you are pointing, if the CUDA implementation are implemented with child classes instead of template specialization then they will have to re-define the TDM. If they are implemented using class specialization then it should work as expected (and thus re-use the existing TDM)

[alxbilger]

I know you're not fan of the static method. Could you give the reasons?

[damienmarchal]

Because it is using the c++ name resolution order to implement static method overrides.

This make the static method override very implicit and also very fragile.
Implicit because: when looking at a static method signature in a code base, nothing tells the developer that the method is not just a normal "individual" but is in fact "part" of a complex multi-class design involving at least the parent and child classes as well as a third party object (here the ObjectFactory) which call the static method.
Fragile because if you decide to refactor your design, like removing the static method in the base class and its corresponding call (in the ObjectFactory in our case), the compiler will not tell you anything that all the intermediates method are useless.

So to me it would way better if there was a c++ keyword to specify that a static method can be "override" (a kind of static overridable) and a second keyword to indicate a static method actually "override".
So something similar as what we have with the "virtual/override" keyword.

EDIT: One extra point which make me dislike the approach is that it is supposed to be used from a templated function, so this induce more "template" and given that templates are harder to understand for average level developer, I have the feeling we should restrict its usage everywhere there is no alternative way. Actually it is very visible, if you ask to developer to explain how things work on part of sofa that use static method overrides, very few can do that as they don't get the whole picture. Eg: Typeinfo_xxx, ObjetFactory, ....

[damienmarchal]

This is the pattern:

#include<string>
#include <iostream>

class ParentClass
{
public:
    /// This method is used in ObjectFactoryPrintYourName... please don't remove it ! 
    static std::string GetName(){ return "BaseClass"; }
};

class ChildClass : public ParentClass
{
public:
     /// This method overrides the ParentClass::GetName()...
    static std::string GetName(){  return "ChildClass"; }
};

class GrandChildClass : public ChildClass
{
public:
     /// This method overrides the ChildClass::GetName()...
    static std::string GetName(){  return "GrandChildClass"; }
};


template<class T>
void ObjectFactoryPrintYourName()
{
    std::cout << T::GetName() << std::endl;
}

int main(int argc, char** argv)
{
    ObjectFactoryPrintYourName<ParentClass>();
    ObjectFactoryPrintYourName<ChildClass>();
    ObjectFactoryPrintYourName<GrandChildClass>();
}

Could we make better than that ?

[damienmarchal]

After thinking a bit more on that... I'm pretty sure that the the problem pointed by @alxbilger only exists because of the way ObjectFactory is implemented. Restricting the ObjectFactory to only allow a single deduction rule for a component type name would solve that (removing the need for the static method overrides).
As I don't see the added value in the current system that allows FEMForceField to deduced a different type compared what FemForceField does.

In addition, the type deduction is actually a feature for

[hugtalbot]

@damienmarchal @alxbilger it would be nice to discuss this directly with you
@alxbilger what's your opinion on this?

[damienmarchal]

@bakpaul as you set it to wip can you elaborate what should be done.

[bakpaul]

@damienmarchal yes, we should schedule a discussion between you, alex, us (Hugo and/or me) and anyone interested, but outside the sofa dev meeting to come to a solution.

[damienmarchal]

Thank for the answer...can you explain a solution to which problem ?

[bakpaul]

To the related discussions (#3938, #3987 & #3990). This is a big subject and it has been concluded that it would require more direct discussions to come to a agreement.

[damienmarchal]

I set it back to review... otherwise we forgot to discuss about it at code review.

[hugtalbot]

We had a meeting planed to discuss this along with @bakpaul @alxbilger but your did not show up @damienmarchal
I think we should discuss it as planed separately from the SOFA dev meeting:

• [SofaCore] Reorganize ObjectFactory so it becomes possible to refactor it in a smooth way #2712
• [Sofa.Core] Implements a template deduction system in the object factory #3938
• [all] Replace canCreate with the getTemplateFromMechanicalState deduction rule #3987
• [all] Replace canCreate with init() when approapriate #3990

[bakpaul]

I may also add two related old pr to the list, they are on the refactoring of create and canCreate methods :

• [Sofa.Core] Refactor BaseObject::canCreate & BaseObject::create #3309
• Remove canCreate in components. #3311