ponca icon indicating copy to clipboard operation
ponca copied to clipboard

Extend requirement system to optional properties

Open ThibaultLejemble opened this issue 1 year ago • 5 comments

This is a draft for #107. The requires/provides system is updated to handle optional capabilities that can be checked at compile-time. Enums are replaced by constexpr bool data members and the verification is done using SFINAE.

Things that change

  • a capability is declared using PROVIDES(CAPABILITY)
  • a capability is required by REQUIRES(CAPABILITY)
  • a macro IS_PROVIDED(CAPABILITY)) checks at compile-tile if a capability is provided or not by a base class
  • enums in the form PROVIDES_... are removed (they are now constexpr bool, and someone that uses myfit.PROVIDES_PLANE might still do the same since PROVIDES(PLANE) actually declares a data member named PROVIDES_PLANE)

Note that a class can still override a capability and no error or warning is printed. I was thinking about printing at least a warning, but there are some capability such as NORMAL_DERIVATIVE that is provided by both MlsSphereFitDer and OrientedSphereDerImpl, but it is not really a programming mistake, one class overrides the capability and by design inherits the other class.

Potential next steps

  • document all the capabilities and generates the list of classes that provides the capabilities automatically (I am not 100% sure it is possible with doxygen)
  • extend the system with a macro like FIT(PLANE) that check if there is no other class that also fits the plane (in replacement to the run-time isValid() function)

Since enum {Check = PROVIDES_PLANE} is replaced by PROVIDES(PLANE), I changed all references of PROVIDES_PLANE to PLANE in the documentation.

ThibaultLejemble avatar Nov 21 '23 16:11 ThibaultLejemble

Thanks for this very nice proposal. I suggest to add the detection of duplicated classes in the hierarchy. My suggestion is to add the check in the declaration macro: the base class should not already have the same capability. As this feature is not always wanted (we only want it for container classes), maybe add PROVIDES_UNIQUE in addition to PROVIDES.

Does that make sense ?

nmellado avatar Nov 27 '23 14:11 nmellado

Another question: could you demonstrate the use of the conditional capabilities usage ? (maybe a test of the macro IS_PROVIDED)

nmellado avatar Nov 27 '23 14:11 nmellado

Thanks for this very nice proposal. I suggest to add the detection of duplicated classes in the hierarchy. My suggestion is to add the check in the declaration macro: the base class should not already have the same capability. As this feature is not always wanted (we only want it for container classes), maybe add PROVIDES_UNIQUE in addition to PROVIDES.

Does that make sense ?

The problem is that it is difficult to know in advance if a capability is unique or not. When declaring a new primitive, or a new fitting operation, how to know a priori if some feature won't be overridden by another class, by another user? For the current tests to pass, every capabilities can be provided by PROVIDES_UNIQUE except NORMAL_DERIVATIVE in OrientedSphereDerImpl that is overridden by MlsSphereFitDer. Should I just remove the PROVIDES(NORMAL_DERIVATIVE) of MlsSphereFitDer (but keeping the method dNormal)?

ThibaultLejemble avatar Nov 28 '23 14:11 ThibaultLejemble

Another question: could you demonstrate the use of the conditional capabilities usage ? (maybe a test of the macro IS_PROVIDED)

I added a small test for IS_PROVIDED : https://github.com/poncateam/ponca/pull/117/commits/f8900b4980893361214716d8c45aef1dc7d3de15

ThibaultLejemble avatar Nov 28 '23 14:11 ThibaultLejemble

Thanks for this very nice proposal. I suggest to add the detection of duplicated classes in the hierarchy. My suggestion is to add the check in the declaration macro: the base class should not already have the same capability. As this feature is not always wanted (we only want it for container classes), maybe add PROVIDES_UNIQUE in addition to PROVIDES. Does that make sense ?

The problem is that it is difficult to know in advance if a capability is unique or not. When declaring a new primitive, or a new fitting operation, how to know a priori if some feature won't be overridden by another class, by another user? For the current tests to pass, every capabilities can be provided by PROVIDES_UNIQUE except NORMAL_DERIVATIVE in OrientedSphereDerImpl that is overridden by MlsSphereFitDer. Should I just remove the PROVIDES(NORMAL_DERIVATIVE) of MlsSphereFitDer (but keeping the method dNormal)?

We need to take some time to discuss this AFK: some fitting extensions are Storage types, while others are Computation: some Computation object store their results in Storage types : this is a side effect, it must be unique in the Basket.

nmellado avatar Nov 28 '23 16:11 nmellado