root-project
[io] Make case values constant expressions
Current clang-16 from main complains: "case value is not a constant expression". Even if that error is probably relaxed before Clang 16 is released early next year, there is really no point in converting an integer into an enum just to get a numeric value back.
This PR fixes #11128
Starting build on ROOT-debian10-i386/soversion, ROOT-performance-centos8-multicore/cxx17, ROOT-ubuntu18.04/nortcxxmod, ROOT-ubuntu2004/python3, mac1015/cxx17, mac11/cxx14, windows10/cxx14
How to customize builds
A tool (Coverity) was complaining that the values used in the switch statement did not belong to the enum. (See ac36d879629fd67cf6fc9e955923ec9f98a2361c). I am guessing to avoid this we might also want to apply something like:
switch (static_cast<Int_t>(fVal->fCase)) {
A tool (Coverity) was complaining that the values used in the switch statement did not belong to the enum. (See ac36d87). I am guessing to avoid this we might also want to apply something like:
switch (static_cast<Int_t>(fVal->fCase)) {
Unless I'm looking at the wrong data structure, fCase already is a UInt_t:
https://github.com/root-project/root/blob/cd992545ae18f0d036e382408d4acfccfa16db48/io/io/inc/TGenCollectionProxy.h#L64
I don't think casting will change anything here.
I don't know if I'm misinterpreting something, but I noticed something very interesting: For this compilation error, clang gives a note message:
/home/jun/dev/root/io/io/src/TGenCollectionStreamer.cxx:392:18: note: integer value 536870912 is outside the valid range of values [0, 63] for this enumeration type
Is this enum type EProperty really only range from 0 - 63?
clangd told me that the definition of first case kIsClass is from
https://github.com/root-project/root/blob/master/core/meta/inc/TDictionary.h#L64 ,
but if I try the below cases, it told me that this enum type EProperty from
https://github.com/root-project/root/blob/master/core/cont/inc/TVirtualCollectionProxy.h#L49 ???
That may said, the compiler got confused about these two types that have same name?
Is this enum type EProperty really only range from 0 - 63?
No, it uses (sparsely) the full range of a 32 bis integer (by using it essentially as a bitfield).
I don't think casting will change anything here.
You a right. I am guessing that Coverity noticed that some of the case value were EProperty and complained that some weren't (since there operator |ed values.)
"case value is not a constant expression".
Humm ... either I don't understand the meaning or Clang is 'wrong' on this part. Both
EProperty(kIsPointer | kBIT_ISSTRING)
and
kIsPointer | kBIT_ISSTRING
are "constant" (i.e calculatable at compile time).
"note: integer value 536870912 is outside the valid range of values [0, 63] for this enumeration type" That may said, the compiler got confused about these two types that have same name?
That is likely the cause of the complaints albeit it is the developer (and possibly Coverity too) that got confused here as it was meant to be written as:
case ::EProperty(kBIT_ISSTRING):
"note: integer value 536870912 is outside the valid range of values [0, 63] for this enumeration type" That may said, the compiler got confused about these two types that have same name?
That is likely the cause of the complaints albeit it is the developer (and possibly Coverity too) that got confused here as it was meant to be written as:
case ::EProperty(kBIT_ISSTRING):
I can confirm this fixes the error, using the diff below:
diff --git a/io/io/src/TGenCollectionStreamer.cxx b/io/io/src/TGenCollectionStreamer.cxx
index da045d2035..dd255f42a5 100644
--- a/io/io/src/TGenCollectionStreamer.cxx
+++ b/io/io/src/TGenCollectionStreamer.cxx
@@ -389,13 +389,13 @@ void TGenCollectionStreamer::ReadObjects(int nElements, TBuffer &b, const TClass
switch (fVal->fCase) {
case kIsClass:
DOLOOP(b.StreamObject(i, fVal->fType, onFileValClass ));
- case EProperty(kBIT_ISSTRING):
+ case ::EProperty(kBIT_ISSTRING):
DOLOOP(i->read_std_string(b));
- case EProperty(kIsPointer | kIsClass):
+ case ::EProperty(kIsPointer | kIsClass):
DOLOOP(i->set(b.ReadObjectAny(fVal->fType)));
- case EProperty(kIsPointer | kBIT_ISSTRING):
+ case ::EProperty(kIsPointer | kBIT_ISSTRING):
DOLOOP(i->read_std_string_pointer(b));
- case EProperty(kIsPointer | kBIT_ISTSTRING | kIsClass):
+ case ::EProperty(kIsPointer | kBIT_ISTSTRING | kIsClass):
DOLOOP(i->read_tstring_pointer(vsn3, b));
}
#undef DOLOOP
@@ -442,20 +442,20 @@ void TGenCollectionStreamer::ReadObjects(int nElements, TBuffer &b, const TClass
fFeed(fEnv->fStart,fEnv->fObject,fEnv->fSize);
fDestruct(fEnv->fStart,fEnv->fSize);
break;
- case EProperty(kBIT_ISSTRING):
+ case ::EProperty(kBIT_ISSTRING):
DOLOOP(i->read_std_string(b))
fFeed(fEnv->fStart,fEnv->fObject,fEnv->fSize);
fDestruct(fEnv->fStart,fEnv->fSize);
break;
- case EProperty(kIsPointer | kIsClass):
+ case ::EProperty(kIsPointer | kIsClass):
DOLOOP(i->set(b.ReadObjectAny(fVal->fType)));
fFeed(fEnv->fStart,fEnv->fObject,fEnv->fSize);
break;
- case EProperty(kIsPointer | kBIT_ISSTRING):
+ case ::EProperty(kIsPointer | kBIT_ISSTRING):
DOLOOP(i->read_std_string_pointer(b))
fFeed(fEnv->fStart,fEnv->fObject,fEnv->fSize);
break;
- case EProperty(kIsPointer | kBIT_ISTSTRING | kIsClass):
+ case ::EProperty(kIsPointer | kBIT_ISTSTRING | kIsClass):
DOLOOP(i->read_tstring_pointer(vsn3, b));
fFeed(fEnv->fStart,fEnv->fObject,fEnv->fSize);
break;
@@ -864,16 +864,16 @@ void TGenCollectionStreamer::ReadMap(int nElements, TBuffer &b, const TClass *on
case kIsClass:
b.StreamObject(i, v->fType);
break;
- case EProperty(kBIT_ISSTRING):
+ case ::EProperty(kBIT_ISSTRING):
i->read_std_string(b);
break;
- case EProperty(kIsPointer | kIsClass):
+ case ::EProperty(kIsPointer | kIsClass):
i->set(b.ReadObjectAny(v->fType));
break;
- case EProperty(kIsPointer | kBIT_ISSTRING):
+ case ::EProperty(kIsPointer | kBIT_ISSTRING):
i->read_std_string_pointer(b);
break;
- case EProperty(kIsPointer | kBIT_ISTSTRING | kIsClass):
+ case ::EProperty(kIsPointer | kBIT_ISTSTRING | kIsClass):
i->read_tstring_pointer(vsn3, b);
break;
}
However, then another error occured:
[19/1526] Generating G__ROOTVecOps.cxx, ../../lib/ROOTVecOps.pcm
FAILED: math/vecops/G__ROOTVecOps.cxx lib/ROOTVecOps.pcm /home/jun/dev/root/Build/math/vecops/G__ROOTVecOps.cxx /home/jun/dev/root/Build/lib/ROOTVecOps.pcm
cd /home/jun/dev/root/Build/math/vecops && /usr/bin/cmake -E env LD_LIBRARY_PATH=/home/jun/dev/root/Build/lib: ROOTIGNOREPREFIX=1 /home/jun/dev/root/Build/bin/rootcling -rootbuild -v2 -f G__ROOTVecOps.cxx -cxxmodule -s /home/jun/dev/root/Build/lib/libROOTVecOps.so -m Core.pcm -excludePath /home/jun/dev/root -excludePath /home/jun/dev/root/Build/ginclude -excludePath /home/jun/dev/root/Build/externals -excludePath /home/jun/dev/root/Build/builtins -writeEmptyRootPCM -compilerI/usr/include/c++/11 -compilerI/usr/include/x86_64-linux-gnu/c++/11 -compilerI/usr/include/c++/11/backward -compilerI/home/jun/opt/clang/lib/clang/16.0.0/include -compilerI/usr/local/include -compilerI/usr/include/x86_64-linux-gnu -compilerI/usr/include -compilerI/home/jun/opt/clang/lib/clang/16.0.0/include -compilerI/usr/local/include -compilerI/usr/include/x86_64-linux-gnu -compilerI/usr/include -I/home/jun/dev/root/Build/include -I/home/jun/dev/root/math/vecops/inc -I/home/jun/dev/root/core/unix/inc -I/home/jun/dev/root/core/foundation/v7/inc -I/home/jun/dev/root/core/base/v7/inc -I/home/jun/dev/root/core/clingutils/inc -I/home/jun/dev/root/core/textinput/inc -I/home/jun/dev/root/core/thread/inc -I/home/jun/dev/root/core/zip/inc -I/home/jun/dev/root/core/rint/inc -I/home/jun/dev/root/core/clib/inc -I/home/jun/dev/root/core/meta/inc -I/home/jun/dev/root/core/gui/inc -I/home/jun/dev/root/core/cont/inc -I/home/jun/dev/root/core/foundation/inc -I/home/jun/dev/root/core/base/inc -I/home/jun/dev/root/Build/ginclude ROOT/RVec.hxx /home/jun/dev/root/math/vecops/inc/LinkDef.h
In file included from input_line_10:3:
/home/jun/dev/root/Build/include/ROOT/RVec.hxx:135:39: error: no member named 'numeric_limits' in namespace 'std'
size_t SizeTypeMax() { return std::numeric_limits<Size_T>::max(); }
~~~~~^
/home/jun/dev/root/Build/include/ROOT/RVec.hxx:135:54: error: unexpected type name 'Size_T': expected expression
size_t SizeTypeMax() { return std::numeric_limits<Size_T>::max(); }
^
/home/jun/dev/root/Build/include/ROOT/RVec.hxx:135:61: error: no matching function for call to 'max'
size_t SizeTypeMax() { return std::numeric_limits<Size_T>::max(); }
^~~~~
/usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/algorithmfwd.h:407:5: note: candidate function template not viable: requires 2 arguments, but 0 were provided
max(const _Tp&, const _Tp&);
^
/usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/algorithmfwd.h:412:5: note: candidate function template not viable: requires 3 arguments, but 0 were provided
max(const _Tp&, const _Tp&, _Compare);
^
/usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_algo.h:3461:5: note: candidate function template not viable: requires single argument '__l', but no arguments were provided
max(initializer_list<_Tp> __l)
^
/usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_algo.h:3467:5: note: candidate function template not viable: requires 2 arguments, but 0 were provided
max(initializer_list<_Tp> __l, _Compare __comp)
^
Error: /home/jun/dev/root/Build/bin/rootcling: compilation failure (/home/jun/dev/root/Build/lib/libROOTVecOps578190a973_dictUmbrella.h)
[40/1526] Generating G__Hist.cxx, ../../lib/Hist.pcm
ninja: build stopped: subcommand failed.
I don't know if I'm misinterpreting something, but I noticed something very interesting: For this compilation error, clang gives a note message:
/home/jun/dev/root/io/io/src/TGenCollectionStreamer.cxx:392:18: note: integer value 536870912 is outside the valid range of values [0, 63] for this enumeration typeIs this enum type
EPropertyreally only range from 0 - 63? clangd told me that the definition of first casekIsClassis from https://github.com/root-project/root/blob/master/core/meta/inc/TDictionary.h#L64 , but if I try the below cases, it told me that this enum typeEPropertyfrom https://github.com/root-project/root/blob/master/core/cont/inc/TVirtualCollectionProxy.h#L49 ???That may said, the compiler got confused about these two types that have same name?
That's interesting: TGenCollectionStreamer inherits from TVirtualCollectionProxy (via TGenCollectionProxy), so that EProperty is indeed "closer" than ::EProperty.
"case value is not a constant expression".
Humm ... either I don't understand the meaning or Clang is 'wrong' on this part. Both
EProperty(kIsPointer | kBIT_ISSTRING) and kIsPointer | kBIT_ISSTRINGare "constant" (i.e calculatable at compile time).
As discussed post-commit in https://reviews.llvm.org/D130058, the check is indeed questionable in a few corner cases. But that shouldn't stop us from finding a portable solution that avoids the problems altogether.
"note: integer value 536870912 is outside the valid range of values [0, 63] for this enumeration type" That may said, the compiler got confused about these two types that have same name?
That is likely the cause of the complaints albeit it is the developer (and possibly Coverity too) that got confused here as it was meant to be written as:
case ::EProperty(kBIT_ISSTRING):
The range checks of that new warning are also known bad for some cases, not sure if that is one of them...
"note: integer value 536870912 is outside the valid range of values [0, 63] for this enumeration type" That may said, the compiler got confused about these two types that have same name?
That is likely the cause of the complaints albeit it is the developer (and possibly Coverity too) that got confused here as it was meant to be written as:
case ::EProperty(kBIT_ISSTRING):I can confirm this fixes the error, using the diff below:
diff --git a/io/io/src/TGenCollectionStreamer.cxx b/io/io/src/TGenCollectionStreamer.cxx index da045d2035..dd255f42a5 100644 --- a/io/io/src/TGenCollectionStreamer.cxx +++ b/io/io/src/TGenCollectionStreamer.cxx @@ -389,13 +389,13 @@ void TGenCollectionStreamer::ReadObjects(int nElements, TBuffer &b, const TClass switch (fVal->fCase) { case kIsClass: DOLOOP(b.StreamObject(i, fVal->fType, onFileValClass )); - case EProperty(kBIT_ISSTRING): + case ::EProperty(kBIT_ISSTRING): DOLOOP(i->read_std_string(b)); - case EProperty(kIsPointer | kIsClass): + case ::EProperty(kIsPointer | kIsClass): DOLOOP(i->set(b.ReadObjectAny(fVal->fType))); - case EProperty(kIsPointer | kBIT_ISSTRING): + case ::EProperty(kIsPointer | kBIT_ISSTRING): DOLOOP(i->read_std_string_pointer(b)); - case EProperty(kIsPointer | kBIT_ISTSTRING | kIsClass): + case ::EProperty(kIsPointer | kBIT_ISTSTRING | kIsClass): DOLOOP(i->read_tstring_pointer(vsn3, b)); } #undef DOLOOP @@ -442,20 +442,20 @@ void TGenCollectionStreamer::ReadObjects(int nElements, TBuffer &b, const TClass fFeed(fEnv->fStart,fEnv->fObject,fEnv->fSize); fDestruct(fEnv->fStart,fEnv->fSize); break; - case EProperty(kBIT_ISSTRING): + case ::EProperty(kBIT_ISSTRING): DOLOOP(i->read_std_string(b)) fFeed(fEnv->fStart,fEnv->fObject,fEnv->fSize); fDestruct(fEnv->fStart,fEnv->fSize); break; - case EProperty(kIsPointer | kIsClass): + case ::EProperty(kIsPointer | kIsClass): DOLOOP(i->set(b.ReadObjectAny(fVal->fType))); fFeed(fEnv->fStart,fEnv->fObject,fEnv->fSize); break; - case EProperty(kIsPointer | kBIT_ISSTRING): + case ::EProperty(kIsPointer | kBIT_ISSTRING): DOLOOP(i->read_std_string_pointer(b)) fFeed(fEnv->fStart,fEnv->fObject,fEnv->fSize); break; - case EProperty(kIsPointer | kBIT_ISTSTRING | kIsClass): + case ::EProperty(kIsPointer | kBIT_ISTSTRING | kIsClass): DOLOOP(i->read_tstring_pointer(vsn3, b)); fFeed(fEnv->fStart,fEnv->fObject,fEnv->fSize); break; @@ -864,16 +864,16 @@ void TGenCollectionStreamer::ReadMap(int nElements, TBuffer &b, const TClass *on case kIsClass: b.StreamObject(i, v->fType); break; - case EProperty(kBIT_ISSTRING): + case ::EProperty(kBIT_ISSTRING): i->read_std_string(b); break; - case EProperty(kIsPointer | kIsClass): + case ::EProperty(kIsPointer | kIsClass): i->set(b.ReadObjectAny(v->fType)); break; - case EProperty(kIsPointer | kBIT_ISSTRING): + case ::EProperty(kIsPointer | kBIT_ISSTRING): i->read_std_string_pointer(b); break; - case EProperty(kIsPointer | kBIT_ISTSTRING | kIsClass): + case ::EProperty(kIsPointer | kBIT_ISTSTRING | kIsClass): i->read_tstring_pointer(vsn3, b); break; }
I still find it interesting that this works. However, as said before, the type of fCase is an integer anyway, so IMO we should compare against integer constants. Bit-wise combinations of enum values are not enum values themselves...
However, then another error occured:
[19/1526] Generating G__ROOTVecOps.cxx, ../../lib/ROOTVecOps.pcm FAILED: math/vecops/G__ROOTVecOps.cxx lib/ROOTVecOps.pcm /home/jun/dev/root/Build/math/vecops/G__ROOTVecOps.cxx /home/jun/dev/root/Build/lib/ROOTVecOps.pcm cd /home/jun/dev/root/Build/math/vecops && /usr/bin/cmake -E env LD_LIBRARY_PATH=/home/jun/dev/root/Build/lib: ROOTIGNOREPREFIX=1 /home/jun/dev/root/Build/bin/rootcling -rootbuild -v2 -f G__ROOTVecOps.cxx -cxxmodule -s /home/jun/dev/root/Build/lib/libROOTVecOps.so -m Core.pcm -excludePath /home/jun/dev/root -excludePath /home/jun/dev/root/Build/ginclude -excludePath /home/jun/dev/root/Build/externals -excludePath /home/jun/dev/root/Build/builtins -writeEmptyRootPCM -compilerI/usr/include/c++/11 -compilerI/usr/include/x86_64-linux-gnu/c++/11 -compilerI/usr/include/c++/11/backward -compilerI/home/jun/opt/clang/lib/clang/16.0.0/include -compilerI/usr/local/include -compilerI/usr/include/x86_64-linux-gnu -compilerI/usr/include -compilerI/home/jun/opt/clang/lib/clang/16.0.0/include -compilerI/usr/local/include -compilerI/usr/include/x86_64-linux-gnu -compilerI/usr/include -I/home/jun/dev/root/Build/include -I/home/jun/dev/root/math/vecops/inc -I/home/jun/dev/root/core/unix/inc -I/home/jun/dev/root/core/foundation/v7/inc -I/home/jun/dev/root/core/base/v7/inc -I/home/jun/dev/root/core/clingutils/inc -I/home/jun/dev/root/core/textinput/inc -I/home/jun/dev/root/core/thread/inc -I/home/jun/dev/root/core/zip/inc -I/home/jun/dev/root/core/rint/inc -I/home/jun/dev/root/core/clib/inc -I/home/jun/dev/root/core/meta/inc -I/home/jun/dev/root/core/gui/inc -I/home/jun/dev/root/core/cont/inc -I/home/jun/dev/root/core/foundation/inc -I/home/jun/dev/root/core/base/inc -I/home/jun/dev/root/Build/ginclude ROOT/RVec.hxx /home/jun/dev/root/math/vecops/inc/LinkDef.h In file included from input_line_10:3: /home/jun/dev/root/Build/include/ROOT/RVec.hxx:135:39: error: no member named 'numeric_limits' in namespace 'std' size_t SizeTypeMax() { return std::numeric_limits<Size_T>::max(); } ~~~~~^ /home/jun/dev/root/Build/include/ROOT/RVec.hxx:135:54: error: unexpected type name 'Size_T': expected expression size_t SizeTypeMax() { return std::numeric_limits<Size_T>::max(); } ^ /home/jun/dev/root/Build/include/ROOT/RVec.hxx:135:61: error: no matching function for call to 'max' size_t SizeTypeMax() { return std::numeric_limits<Size_T>::max(); } ^~~~~ /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/algorithmfwd.h:407:5: note: candidate function template not viable: requires 2 arguments, but 0 were provided max(const _Tp&, const _Tp&); ^ /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/algorithmfwd.h:412:5: note: candidate function template not viable: requires 3 arguments, but 0 were provided max(const _Tp&, const _Tp&, _Compare); ^ /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_algo.h:3461:5: note: candidate function template not viable: requires single argument '__l', but no arguments were provided max(initializer_list<_Tp> __l) ^ /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_algo.h:3467:5: note: candidate function template not viable: requires 2 arguments, but 0 were provided max(initializer_list<_Tp> __l, _Compare __comp) ^ Error: /home/jun/dev/root/Build/bin/rootcling: compilation failure (/home/jun/dev/root/Build/lib/libROOTVecOps578190a973_dictUmbrella.h) [40/1526] Generating G__Hist.cxx, ../../lib/Hist.pcm ninja: build stopped: subcommand failed.
This is actually a different, unrelated problem. Should be fixed by https://github.com/root-project/root/pull/11152
The range checks of that new warning are also known bad for some cases, not sure if that is one of them...
In this case it ended being relevant as indeed the wrong enum type was being used and it could (theoretically) have lead to slicing the value.
Bit-wise combinations of enum values are not enum values themselves...
However, in this case, they are "semantically" part of the enums values.
However, as said before, the type of fCase is an integer anyway,
That did not prevent Coverity from complaining that some of the case value where from an enum and some where numerical value (that it assumed were unrelated).
Bit-wise combinations of enum values are not enum values themselves...
However, in this case, they are "semantically" part of the enums values.
Doesn't matter what we semantically want them to be. For the compiler they aren't, so it's undefined behavior (at least that's how I understand the entire discussion).
However, as said before, the type of fCase is an integer anyway,
That did not prevent Coverity from complaining that some of the case value where from an enum and some where numerical value (that it assumed were unrelated).
I don't care what Coverity thinks or complains about. I'm arguing that having integer constants is more correct than what we do right now, and it seems you don't disagree with this point, do you?
I don't care what Coverity thinks or complains about. I'm arguing that having integer constants is more correct than what we do right now
Fair enough. Can you update the git log to reflect that the 'reversion' (instead of tweak) of the previous commit is intentional (and needed).
Starting build on ROOT-debian10-i386/soversion, ROOT-performance-centos8-multicore/cxx17, ROOT-ubuntu18.04/nortcxxmod, ROOT-ubuntu2004/python3, mac1015/cxx17, mac11/cxx14, windows10/cxx14
How to customize builds
I don't care what Coverity thinks or complains about. I'm arguing that having integer constants is more correct than what we do right now
Fair enough. Can you update the git log to reflect that the 'reversion' (instead of tweak) of the previous commit is intentional (and needed).
Done. I've also mentioned the ambiguity between EProperty and ::EProperty, should we ever come across this code again.
Build failed on ROOT-performance-centos8-multicore/cxx17. Running on olbdw-01.cern.ch:/data/sftnight/workspace/root-pullrequests-build See console output.
Failing tests:
- projectroot.runtutorials.tutorial_roostats_StandardHistFactoryPlotsWithCategories
- projectroot.runtutorials.tutorial_roostats_StandardProfileLikelihoodDemo
Thanks, that looks good but maybe we can apply clang-format?
If I run git-clang-format HEAD^, the result is
diff --git a/io/io/src/TGenCollectionStreamer.cxx b/io/io/src/TGenCollectionStreamer.cxx
index a2acf0b8d0..9979884dae 100644
--- a/io/io/src/TGenCollectionStreamer.cxx
+++ b/io/io/src/TGenCollectionStreamer.cxx
@@ -389,14 +389,10 @@ void TGenCollectionStreamer::ReadObjects(int nElements, TBuffer &b, const TClass
switch (fVal->fCase) {
case kIsClass:
DOLOOP(b.StreamObject(i, fVal->fType, onFileValClass ));
- case kBIT_ISSTRING:
- DOLOOP(i->read_std_string(b));
- case kIsPointer | kIsClass:
- DOLOOP(i->set(b.ReadObjectAny(fVal->fType)));
- case kIsPointer | kBIT_ISSTRING:
- DOLOOP(i->read_std_string_pointer(b));
- case kIsPointer | kBIT_ISTSTRING | kIsClass:
- DOLOOP(i->read_tstring_pointer(vsn3, b));
+ case kBIT_ISSTRING: DOLOOP(i->read_std_string(b));
+ case kIsPointer | kIsClass: DOLOOP(i->set(b.ReadObjectAny(fVal->fType)));
+ case kIsPointer | kBIT_ISSTRING: DOLOOP(i->read_std_string_pointer(b));
+ case kIsPointer | kBIT_ISTSTRING | kIsClass: DOLOOP(i->read_tstring_pointer(vsn3, b));
}
#undef DOLOOP
break;
@@ -864,18 +860,10 @@ void TGenCollectionStreamer::ReadMap(int nElements, TBuffer &b, const TClass *on
case kIsClass:
b.StreamObject(i, v->fType);
break;
- case kBIT_ISSTRING:
- i->read_std_string(b);
- break;
- case kIsPointer | kIsClass:
- i->set(b.ReadObjectAny(v->fType));
- break;
- case kIsPointer | kBIT_ISSTRING:
- i->read_std_string_pointer(b);
- break;
- case kIsPointer | kBIT_ISTSTRING | kIsClass:
- i->read_tstring_pointer(vsn3, b);
- break;
+ case kBIT_ISSTRING: i->read_std_string(b); break;
+ case kIsPointer | kIsClass: i->set(b.ReadObjectAny(v->fType)); break;
+ case kIsPointer | kBIT_ISSTRING: i->read_std_string_pointer(b); break;
+ case kIsPointer | kBIT_ISTSTRING | kIsClass: i->read_tstring_pointer(vsn3, b); break;
}
v = fVal;
addr += fValOffset;
which I find much worse to read...
which I find much worse to read...
I agree ... let's keep the code formatted as is.
Done. I've also mentioned the ambiguity between
EPropertyand::EProperty, should we ever come across this code again.
Thanks.