[hist,bindings,test,tutorials] rename CINT to Cling #20758
Conversation
ferdymercury
requested review from
couet,
dpiparo,
lmoneta and
vepadulano
as code owners
Test Results 22 files 22 suites 3d 20h 53m 57s ⏱️ Results for commit a40c06f. ♻️ This comment has been updated with latest results. |
| if (strchr(cr->GetName(), '<')) | ||
| gInterpreter->ProcessLine(((std::string)cr->GetName()+"::"+m->GetName()+";").c_str()); | ||
| offset = (intptr_t)m->GetOffsetCint(); // yes, Cling (GetOffset() is both wrong | ||
| offset = (intptr_t)m->GetOffsetCint(); // yes, Cling GetOffset() is both wrong (TODO: rename to GetOffsetCling?) |
There was a problem hiding this comment.
I would remove this change, as we're doing so many changes to clingwrapper anyway these days that it will become soon outdated (possibly removed?)
|
|
||
| /// Forwards to THnBase::SetBinContent(). | ||
| /// Non-virtual, CINT-compatible replacement of a using declaration. | ||
| /// Non-virtual, CINT-compatible replacement of a using declaration. @todo recheck with Cling |
There was a problem hiding this comment.
What is the purpose of all these @todos?
There was a problem hiding this comment.
in a follow-up PR, the idea is to replace all these functions with just
using THnBase::SetBinContent
in the hope that Cling does not longer have the mentioned problem.
Since this PR was touching mostly comments, not code changes.
Or do you think Cling will still need that workaround in the same way as CINT?
| //Begin_Html | ||
| /* | ||
| <img src="gif/TGRootIDE.gif"> | ||
| /** |
There was a problem hiding this comment.
Is this change really related to renaming CINT to Cling?
| mess->ReadInt(clientId); | ||
| mess->ReadTString(filename); | ||
| mess->ReadLong64(length); // '*mess >> length;' is broken in CINT for Long64_t. | ||
| mess->ReadLong64(length); // '*mess >> length;' was broken in CINT for Long64_t. TODO: recheck with Cling |
There was a problem hiding this comment.
Also what is the purpose of this TODO (and why is it different from @todo above)?
There was a problem hiding this comment.
why is it different from
@todoabove)
@todo only works within doxygen documentation
TODO is more for code comments that do not appear in doxygen comments.
grep -i todo should work for both for searching these
I could also switch to FIXME if that's preferred
| mess.WriteInt(idx); | ||
| mess.WriteTString(file->GetName()); | ||
| mess.WriteLong64(file->GetEND()); // 'mess << file->GetEND();' is broken in CINT for Long64_t | ||
| mess.WriteLong64(file->GetEND()); // 'mess << file->GetEND();' was broken in CINT for Long64_t |
There was a problem hiding this comment.
Is the comment relevant at all, since it refers to a previously broken feature of CINT?
There was a problem hiding this comment.
The idea is to replace in a follow-up PR this line with just
mess << file->GetEND()
but if you prefer I can fully remove this comment since it's not relevant?
| //============================================================================== | ||
|
|
||
| // Should be run in compiled mode -- CINT has issues with recursion. | ||
| // Should be run in compiled mode -- CINT had issues with recursion. TODO: recheck with Cling |
There was a problem hiding this comment.
Next step would be to run this tutorial in interpreted mode, if Cling has no issues, then fully remove this line.
bindings/pyroot/cppyy/cppyy-backend/clingwrapper/src/clingwrapper.cxx
Outdated
Show resolved
Hide resolved
3 participants
Subpart of work towards: #10058