QMCPACK antipattern: Throw runtime error if override isn't implemented.

[PDoakORNL]

SPOSet::makeClone is a glaring example of this but I believe it is present many other places as well.

When you want to insure that all derived classes implement a method make it a pure virtual method i.e.

  /** make a clone of itself
   * every derived class must implement this to have threading working correctly.
   */
  virtual std::unique_ptr<SPOSetT> makeClone() const = 0;

This causes a compile time error as it should, and gives an immediate explanation. Any error that can be stopped at compile time should be.

Don't do this

   /** make a clone of itself
    * every derived class must implement this to have threading working correctly.  
    */
[[noreturn]] virtual std::unique_ptr<SPOSetT> makeClone() const;

A runtime error will someday occur hopefully its detailed enough make finding the mplementation that threw it easy, but potentially it will even be seen by a user and not at development time. This is not really a runtime error though is it.

[ye-luo]

I guess that was introduced to minimize a needed implementation for a new SPOSet. It is maybe needed for some functions but definitely not for makeClone.

I'd like to restrict the scope of this issue to SPOSet::makeClone to prevent long standing issues. If more were found, just report separate issues with reference to this one.

[PDoakORNL]

I think you are right about long standing issues. We have far too many just lying open that were basically written to never die, this has proven to be bad practice. I think for those issues where its a coding style or design issue important enough it should go into the manual or other documentation. For the design issues where no agreement has ever been reached I think the competing sides should write up their designs, reference their example implementations in the code, prove the design meets the long term goals for the code and we should resolve the disagreements (input handling comes to mind).