TObject: extend doc warning against defaulted constructor

[pcanal]

[github-actions[bot]]

Test Results

    13 files      13 suites   2d 19h 36m 55s :stopwatch:  3 025 tests  3 025 :white_check_mark: 0 :zzz: 0 :x: 33 799 runs  33 799 :white_check_mark: 0 :zzz: 0 :x:

Results for commit f38f0f3b.

:recycle: This comment has been updated with latest results.

[dpiparo]

The change looks fine to me, thanks for proposing it. Maybe a doubt, which could be clarified by C++ experts (@silverweed @hahnjo) do you know whether we can prevent at compile time with an error/warning the situation where a user defines a default c'tor for classes ultimately inheriting from TObject? If that's not possible, should we also think about a way to check this at dictionary generation level via the Clang interface, when we have the entire AST at disposal?

[hahnjo]

@pcanal which compiler do we observe = default doing 'more' than expected? From a C++ point of view, I thought it just means that the compiler is supposed to generate a default constructor, initializing all direct members and calling the parent class constructor. On the other hand, the implementation of IsOnHeap relies on UB, so I guess anything can happen...

[pcanal]

@hahnjo At least Alma9 + gcc11 (both in compiled and interpreted mode). Simple reproducer:

{
  struct Derived : public TObject { Derived() = default; };
  auto obj = new Derived{};
  if (! obj->IsOnHeap())
    throw "TObject::IsOnHeap is broken for MyType";
  delete obj;
}

The 'more' than expected is that the defaulted constructor includes the execution of a bzero (or memset) on the object's memory. This does not violate the C++ standard but make the "undefined behavior" that IsOnHeap relies to not longer 'work'.

@dpiparo I think it is indeed possible to add Clang based code in rootcling to detect if the constructors are defaulted.

[ChristianTackeGSI]

If that's not possible, should we also think about a way to check this at dictionary generation level via the Clang interface, when we have the entire AST at disposal?

May I comment / ask something here?

If I got this right, the IsOnHeap-behaviour depends on UB? And not allowing = default on derived classes to support this UB is at least unintuitive. (It's good, that it's documented, and it's good to consider checking it!)

Are there any plans to phase out (and then deprecate) this? So that people actually can use = default?

The first step towards phasing out would be to document the issues involved when using = default and what a non-functional IsOnHeap would actually induce?

[ChristianTackeGSI]

Just to interlink things (please update as you see fit):

• #4320