Skip to content

Start migration BinnedArray to using span for the const arrayObjects interface

Making TrackingVolume ctor thread safe remove ATLAS_CTORDTOR_NOT_THREAD_SAFE

  • This is the main goal. With a non-MT safe ctor there is no way for any builder to be marked as MT safe.

  • Now we have a chance. Note there are still non-safe methods and const_casts . But we make a small step

const BinnedArray<T> vs BinnedArray<const T> vs BinnedArray<T>

So far when using BinnedArray types

  • BinnedArrayT<T> or
  • BinnedArrayT<const T> .

We had just const std::vector<T*>& arrayObjects() const

So for a BinnedArrayT<T> the above would still allow to modify the T which is not what we want, since the method is const. i.e here we would like to get back "immutableT when we ask for the arrayObjectsof aconst BinnedArrayT``

These 2 examples can prb highlight this issue / solution

Ideally in order to avoid const_cast but also retain const correctness We want to have also the semantics for a const BinnedArray<T> on top of BinnedArray<T> and BinnedArray<const T>

We do

const std::vector<T*>&  arrayObjects()

and

  using const_ptr_span = CxxUtils::span<const T* const>;
  const_ptr_span arrayObjects() const 

so as to have proper semantics for a const BinnedArray<T> At the same time a type BinnedArray<const T> remains

  • Now, having CxxUtils::span there is a way to express that. This also allows to have some proper const/non-const overlaods in TrkGeometry classes which could hopefully helps us rm some const_cast

Un-needed const

  • One of the major issues we have is this.

A file has something like

const foo* = new foo();

Note this foo is created as const . But needs not be.

Then the foo is passed

const newObjectOfFoos(const foo* passAFoo)

Then something like this occurs

auto* mutfoo =  const_cast<foo*> (passAFoo)
mutfoo -> modifySomethingForFoo(); 

Now strictly speaking this is UB. If

const foo* = new foo();

was

foo* = new foo();

This is not UB. [https://en.cppreference.com/w/cpp/language/const_cast] i.e foo* = new foo(); falls here

const_cast makes it possible to form a reference or pointer to non-const type that is actually referring to a const object const foo* = new foo(); falls here Modifying a const object through a non-const access path and referring results in undefined behavior.

Of course to have an compiler optimize this is rare so the UB prb is not "observed", especially as usuall things are even in different .so.

But we want to get rid also of the un-needed const here. e.g we want to avoid the chain that would be UB as far as we can even before getting rid the const_cast

Btw the most obvious unneeded used of const are :

  - const Trk::LayerArray* dummyLayers = nullptr;
  - const Trk::TrackingVolumeArray* dummyVolumes = nullptr;
  + Trk::LayerArray* dummyLayers = nullptr;
  + Trk::TrackingVolumeArray* dummyVolumes = nullptr;

These are prb superfluous .

Ping @sroe, @ssnyder

Edited by Christos Anastopoulos

Merge request reports