Allocator template parameter of classes without memory management

Several classes in bvector.hh and basearray.hh have an allocator template parameter but don't do any memory management. This is true for base_array_unmanaged, base_array_window, compressed_base_array_unmanaged, block_vector_unmanaged, BlockVectorWindow, compressed_block_vector_unmanaged, CompressedBlockVectorWindow.

Changed title: Allocate template parameter of classes without memory management → Allocator template parameter of classes without memory management

I have noticed this, too. In some cases, the allocator is only used to get the index type. I was unsure whether to propose patches. Are these classes internal to dune-istl, or are they public interfaces?

Since they are not explicitly marked as implementation detail, you never know if and how they are used outside of the core.

they are supposed to be internal. Yet there is no guarantee that nobody uses them downstream.

Maybe we should start moving such stuff into implementation namespaces to help sorting out such questions in the future.

Should we mark them as internal / deprecate external use for Dune 2.5? Then we can make the changes afterwards.

Mentioned in commit 8649fc02

Mentioned in merge request !65 (merged)

I opened merge request !65 (merged) , which moves all those classes into the 'Imp' namespace. I am tempted to just merge it, ignoring the possibility that the classes might be used outside of dune-istl.

Unfortunately we never had an official way to mark implementation details (despite not documenting them). Hence things were internal if they were intended to be so. Let's move it now to officially document this.

I'm against just moving this and possibly breaking code so close to 2.5. Is there a better argument for doing this expect that it is cleaner. I agree that we should introduce an 'Imp' namespace like this to clarify for he user which classes might be changed without too much notice and what is considered interface. But for me this is not enough of a reason to do this before 2.5 and break user code. At lease import them back into the dune namespace for 2.5 and then remove these imports in the master after 2.5 - we can afford to wait that long surely.

In this special case I'm for quickly hiding theses classes, because we can only hope that no one used them before. They rely on undefined behaviour by using down-casting.

@andreas.dedner , my argument is that a) I don't believe that anybody is really using this, b) moving it into the Imp namespace makes sure nobody starts using it.

But I have no intention to force this into 2.5 against opposition. If you don't like it I will update the documentation for 2.5, and merge the namespace change into master after the release.

I don't think anybody is using them (that would just be insane). So I do not care how we proceed, as in this case nobody uses the unallowed downcasts and changing would break nobody's code.

The unallowed casts from basearray.hh and bvector.hh are actually used in istl/tutorial/example.cc, basearraytest.cc, and bvectortest.cc. However, these uses do explicitly test the constructors and assignments containing the casts. Removing them does not effect other checks.

I opened MR !66 (merged) for the removal of the unsafe and unused methods containing the downcasts. This also removes the corresponding tests while it does not break the others. Notice that this is orthogonal to !65 (merged).

Mentioned in commit 13ec7d1b

Mentioned in commit ec617e3b

mentioned in commit 03ed8718

mentioned in merge request !201 (merged)