root-project / root

The official repository for ROOT: analyzing, storing and visualizing big data, scientifically
https://root.cern
Other
2.66k stars 1.27k forks source link

Handling of std::conditional by Cling in ROOT 6.32.02. #16119

Open ktf opened 2 months ago

ktf commented 2 months ago

Check duplicate issues.

Description

When running with 6.32.02 the ALICE reconstruction we were plagued by errors like the following:

./itstpcMatch.log[16548:itstpc-track-writer]: Error in : o2::dataformats::AbstractRef<25,5,2>, unknown type: typename std::conditional<(32> mRef

which were not there when using 6.30.0. We have since been able to workaround this by doing:

https://github.com/AliceO2Group/AliceO2/pull/13310/files

however I was wondering if this is not some more serious issue. In particular I am puzzled by the "(32" template argument, which seems to indicate some weird memory overwrite / macro expansion of NBIT.

Reproducer

I do not have a simple reproducer.

ROOT version

6.32.02

Installation method

aliBuild

Operating system

Linux

Additional context

No response

dpiparo commented 2 months ago

I quickly tried to see whether the name normalisation had a bug, and that is not obvious

template<int NBIT>
struct S {
  using type = typename std::conditional<(NBIT > 32), uint64_t, typename std::conditional<(NBIT > 16), uint32_t, typename std::conditional<(NBIT > 8), uint16_t, uint8_t>::type>::type>;
};

void a(){
  using T = typename S<32>::type;
  auto c = TClass::GetClass(typeid(T));
  cout << c->GetName() << endl;
  // gives conditional<false,ULong64_t,unsigned int>
}
ktf commented 2 months ago

Indeed, that I tried as well and I can confirm it works also for us ... It looks like you need the whole parent class.

ktf commented 1 month ago

Ok, I have a very trivial reproducer. In our environment, when running the following on the root prompt:

root [19] TClassEdit::GetNormalizedName(foo, "std::conditional<(NBIT > 32), uint64_t, typename std::conditional<(NBIT > 16), uint32_t, typename std::conditional<(NBIT > 8), uint16_t, uint8_t>::type>::type>::type")

I get:

root [20] foo
(std::string &) "conditional<(NBIT>"
ktf commented 1 month ago

actually, even easier:

root [23] TClassEdit::GetNormalizedName(foo, "std::conditional<1 > 32, int, float>")
root [24] foo
(std::string &) "conditional<1>"
ktf commented 1 month ago

For the record, I confirm also that your example works for me a well. However I also have:

root [4] TClassEdit::GetNormalizedName(foo, "std::conditional<(1 > 32), int, float>")
root [5] foo
(std::string &) "conditional<(1>"

which is exactly the wrong behaviour I originally reported...

ktf commented 1 month ago

BTW, TClassEdit::GetNormalizedName is also broken in 6.30.01, so I suspect the issue we had is due to the way it is being used by TStreamerInfo::Build.

ktf commented 1 month ago

Any news on this issue? Do you need anything else on my side? Apparently my workaround prevents reading old data, so we might have to go back to the version which breaks.