Throw an exception if an unimplemented function is called in BP5

[eisenhauer]

No description provided.

[anagainaru]

Why do we have this function is nobody can/should call it?

[eisenhauer]

That's a bit of a long story, but the short version is that the BP3/4 reader engines parse their metadata and build blocksinfo structures (BPInfo) associated with the Variable class in the IO. Those structures weren't necessarily involved in most operations, but building them was a very high overhead part of read-side BP3/4 metadata operation. BP5 takes a different tack, interprets its metadata directly out of the read buffer and trying not to put anything engine-specific into the Var and IO classes. However, BlocksInfo is part of the external API, (with a 2nd, different, but identically named "BPInfo" structure in the C++ bindings, so BP5 couldn't do away with it entirely, nor could did we want to incur the overhead of building the internal structures that BP3/4 built. So engines can now have a MinBlocksInfo that generates externally-requested on-demand rather than all the time. This required changes in all the bindings to call MinBlocksInfo to see if it is implemented and only fallback to DoBlocksInfo if it is not. Unfortunately there was a spot in the Python bindings that I missed with these mods and because DoBlocksInfo for BP5 was essentially "return NULL" rather than throw an exception it took longer to find. So this is just a defensive mechanism in case something else is missed or someone makes a mistake with new code.

But, the question does remind me that there are many calls in Engine.cpp that are meant to be overridden by specific engine classes and that just throw an exception if they are not. It would likely make more sense to simply not override the default and so get that exception rather than specifically adding a duplicative exception in the subclass... I'll sort that mod tomorrow.

[anagainaru]

Thanks for the explanation. That makes sense, it's unfortunate we need to replicate the functions and throw exceptions but it does make sense why this is the more convenient solution. We need to make sure all functions throw an exception, otherwise a misuse of BP5 will be very hard to debug.

[williamfgc]

Yes, this was probably me rushing with some deadline to get reading working. An option would be static_assert since this is for developers, not users.

[eisenhauer]

New version simply doesn't override DoBlocksInfo in the BP5Reader class. If that method gets called in BP5 it will end up with the default version in Engine which throws an exception (because it's never meant to be called).