Skip to content

[FiniteElementMapInterface] Detect unimplemented methods at compile time instead of via stack overflows.

"Recent" FiniteElementMaps must implement a method hasDOFs(int codim). I had an older FiniteElementMap that did not have that method (yet). This did not result in a compile time error, but in a segmentation fault at runtime. The cause turned out to be

bool hasDOFs(int codim) const
{
  return asImp().hasDOFs(codim);
}

Since the implementation did not declare a member hasDOFs, a call to fem.hasDOFs() will find the member in the interface class. That will try to forward the call to the implementation, but that will again resolve to hasDOFs() on the interface class, resulting in an infinite recursion.

This will happen for all interface functions that are not implemented in the implementation, since all of them attempt to forward to the implementation. The only way this forward could occur in correct code is if a FiniteElementMap implementation is cast to a reference of it's interface Base class, and when methods being invoked on that reference. However, I don't see any practical use for that, and I don't think that this is done anywhere. (A unit test might do that, but I would consider such a unit test buggy, since it is testing a slightly different thing compared to what will be used by production code.)

There are several ways to turn the runtime error into something that can be detected during build time:

  • Just declare, but don't define the functions on the interface. This would result in link time errors, which are better then runtime errors, but still somewhat inconvenient to track down.
  • Put a static_assert(AlwaysFalse<Imp>::value, ...) into each method. I haven't tried this, so I'm not 100% certain that actually works, but if it does it would lead to a real compile time error.
  • Declare all methods as deleted. I think this has the advantage over the previous method that it will error out even when the call appears in an unevaluated context (such as sizeof() or decltype()). And it does not depend on the unclear future of AlwaysTrue. So this is what I went with.

Merge request reports

Loading