NWChemEx / PluginPlay

An inversion-of-control framework for developing modular scientific software.
https://nwchemex.github.io/PluginPlay/
Apache License 2.0
10 stars 2 forks source link

Merge SDEAny with ModuleInput/ModuleResult? #168

Closed ryanmrichard closed 2 years ago

ryanmrichard commented 3 years ago

SDEAny was originally intended to factor out the common type-erasure needs for ModuleInput and ModuleResult; however, as pointed out by @wavefunction91 the way SDEAny currently works it imposes the both the input and result requirements on all values. Some of these requirements are input- or result-specific, such as being hashable (results do not need to be hashable, but inputs do).

What I'm wondering is if it makes sense to move the type-erasure into the PIMPLs for the ModuleInput and ModuleResult. Alternatively, we could specialize SDEAny into an input and a result version.

wavefunction91 commented 3 years ago

I think the latter provides us with the most flexibility (in case we want to exploit type erasure in other uses of SDEAny, not saying I'd know what those are).

Something like

template < sde_input_result_tag Tag >
class SDEAnyWrapperBase {
  // SFINAE enable/disable APIs based on Tag
};

template <typename T, sde_input_result_tag Tag, enable_if_satisfies_api_req_t<T, Tag>=0>
class SDEAnyWrapper : public SDEAnyWrapperBase<Tag> {
  // Similarly enable/disable APIs based on Tag
};

Clearly, this usage screams Concepts, but we could use SFINAE for now. This might also add additionaly flexibility if we want to ensure that the type in an SDEAny satisfies some other API besides ModuleInput or ModuleOutput.

keceli commented 3 years ago

Thanks to @wavefunction91, we can enable/disable hash or serialize functions with a template parameter. While the code compiles at linking stage I get undefined reference topluginplay::detail::AnyWrapperBase<pluginplay::detail::AnyGeneral>::m_anymaker'` error. I have a draft PR here: #195. Please let me know if you have any suggestions.

ryanmrichard commented 2 years ago

Will be tackled by #216