AMDMIGraphX icon indicating copy to clipboard operation
AMDMIGraphX copied to clipboard

Add constraints to type-erased class constructors

Open pfultz2 opened this issue 1 year ago • 0 comments

For type-erased classes this will now check that the class implements the methods its asking for. Currently it only check the method names and correct parameters, it is still missing checking the return type and operators, but this could be added at a later time.

This can help improve overloading with type erased classes. Before, the type erased class would accept everything whether or not it implemented the interface, which means a function couldn't overload to accept another class.

This also will help improve compilation error. It will now report at the place where an incompatible class is being passed, and it will report what methods are missing. Here is an example of an error from a class that is missing the name method:

/data/src/split_reduce.cpp:241:18: error: no viable conversion from 'fuse_pointwise' to 'const pass'
  241 |     mpm.run_pass(fuse_pointwise{.enable_rewrite_broadcasts = true});
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/data/src/include/migraphx/pass.hpp:99:8: note: candidate constructor (the implicit copy constructor) not viable: no known conversion from 'fuse_pointwise' to 'const pass &' for 1st argument
   99 | struct pass
      |        ^~~~
/data/src/include/migraphx/pass.hpp:99:8: note: candidate constructor (the implicit move constructor) not viable: no known conversion from 'fuse_pointwise' to 'pass &&' for 1st argument
   99 | struct pass
      |        ^~~~
/data/src/include/migraphx/pass.hpp:144:5: note: candidate template ignored: substitution failure [with PrivateDetailTypeErasedT = fuse_pointwise]: no member named 'name' in 'migraphx::fuse_pointwise'
  136 |                   decltype(std::declval<PrivateDetailTypeErasedT>().name(),
      |                                                                     ~~~~
  137 |                            private_detail_te_default_apply(char(0),
  138 |                                                            std::declval<PrivateDetailTypeErasedT>(),
  139 |                                                            std::declval<module_pass_manager&>()),
  140 |                            private_detail_te_default_apply(char(0),
  141 |                                                            std::declval<PrivateDetailTypeErasedT>(),
  142 |                                                            std::declval<program&>()),
  143 |                            void())>
  144 |     pass(PrivateDetailTypeErasedT value)
      |     ^
/data/src/include/migraphx/pass_manager.hpp:46:39: note: passing argument to parameter 'p' here
   46 |     virtual void run_pass(const pass& p)                   = 0;
      |                                       ^

Which is much shorter than the previous errors and its much clearer.

pfultz2 avatar Aug 01 '24 18:08 pfultz2