[hist] Make abstract TH1,2,3 methods truly abstract and add missing implementations

[guitargeek]

The TH1K class implementation forgot to implement some abstract methods
from the TH1 class that need to be implemented.

As the TH1K is almost identical to TH1F (besides the KDE algorithm in
the bin lookup), the TH1F implementations are copy-pasted.

The incomplete implementation was noticed thanks to the test in #17399.

To make sure that these mistakes can't happen anymore, the virtual methods of TH1 are made truly virtual in the sense of the C++ language standard. So far, the implementation was only telling you at runtime that the method should be overridden.

[ferdymercury]

Looks awesome, thanks.

Not sure if this change might break user code in some situations but thery were already getting a warning before so I'm up for it.

Is there a plan to replace "AbstracMethod strings" in other Root classes into pure virtuals? Such as in Tunixsystem etc?

[vepadulano]

Looks like a clear improvement to me, thanks! One small comment for consideration

[pcanal]

Not sure if this change might break user code in some situations but thery were already getting a warning before so I'm up for it.

It might break user code and it might not have been noticed before. The case is a class derived from TH1 where they both only overloaded a subset of the function and only use the overloaded functions.

[github-actions]

Test Results

    18 files      18 suites   4d 4h 1m 28s ⏱️
 2 685 tests  2 661 ✅ 23 💤 1 ❌
46 606 runs  46 605 ✅  0 💤 1 ❌

For more details on these failures, see this check.

Results for commit 5e8667c.

[guitargeek]

Is there a plan to replace "AbstracMethod strings" in other Root classes into pure virtuals? Such as in Tunixsystem etc?

@ferdymercury, no. I just did it it this PR because I was doing it locally anyway to hunt for the cases where these methods were not overridden. There are maybe other cases like that though! But I guess sometimes it's actually better than a "dummy override" in case you actually don't want to implement the method.

It might break user code and it might not have been noticed before. The case is a class derived from TH1 where they both only overloaded a subset of the function and only use the overloaded functions.

@pcanal, I can't see why anyone would like to do that, but of course you are right to point out this corner case. But does that mean you are against the suggested changes? It's not clear to me if you only made a note or requesting changes. Personally I think this corner case is so obscure that we don't need to consider it.

[pcanal]

@guitargeek My comments was a general one and making a point that we can not "blindly" replace all the AbstractMethod with C++ abstract method and that some would require thoughts and/or communication to user.

In this case, I agree with the change as AddBinContent is fundamental to the functionality of the class (example of functions that are 'not' fundamental are things like GetTitle, Print, etc.)

[guitargeek]

@pcanal, thanks for clarifying!

[ferdymercury]

@guitargeek Is it possible that this change produced a regression?

auto h2 = new TH2I("h2", "h2", 10, 0, 1, 2, 0, 2);
h2->AddBinContent(1,1,1);

works on 6.34.02 but fails on master. (We should add a test, too).

[guitargeek]

Yes possible! I forgot some "using" statements.