Skip to content

Commit

Permalink
[tree] Change return type of TBranch::GetAddress() et al to void*
Browse files Browse the repository at this point in the history
So far, the `TBranch::GetAddress` method returned a `char *`. This
causes incompatibility with PyROOT, because `char *` is automatically
converted to a Python string, which is the wrong thing to for an
address.

This made the return value unusable in PyROOT. That's a bug that
currently prevents us from reimplementing some pythonizations in native
Python.

Therefore, the return type of `TBranch::GetAddress()` was changed to
`void *` in this release. Even if this change is not backwards
compatible for typing reasons, the returned value is the same.

With this change, the code is also more consistent because
`GetAddress()` and `SetBranchAddress()` use the same type for the
address.
  • Loading branch information
guitargeek committed Jan 25, 2024
1 parent e49a596 commit da36e02
Show file tree
Hide file tree
Showing 9 changed files with 40 additions and 17 deletions.
23 changes: 23 additions & 0 deletions README/ReleaseNotes/v632/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ The following people have contributed to this new version:


## TTree Libraries

### Add files from subdirectories with `TChain::Add` globbing
It is now possible to add files from multiple subdirectories with `TChain::Add` globbing. For example,
```
Expand All @@ -65,6 +66,28 @@ TChain::Add("/path/to/tree/subdir[0-9]/*.root")
```
This grabs all the root files in subdirectories that have a name starting with `subdir` and ending with some digit.

### Changing return type of TBranch::GetAddress() and others to `void *`

So far, the `TBranch::GetAddress` method returned a `char *`. This causes
incompatibility with PyROOT, because `char *` is automatically converted to a
Python string, which is the wrong thing to for an address. This made the return
value unusable in PyROOT.

Therefore, the return type of `TBranch::GetAddress()` was changed to `void *`
in this release. Even if this change is not backwards compatible for typing
reasons, the returned value is the same. In case your C++ code doesn't compile
because of this, just explicitly cast the return value to what you want with a
C-style or better `reinterpret_cast`. Like this, your code will stay compatible
with all ROOT versions. For example:
```c++
void *addr = reinterpret_cast<void *>(branch->GetAddress());
```

The same change also applies to other functions, the full list is here:

* TBranch::GetAddress()
* TBranchElement::GetObject()

## Histogram Libraries


Expand Down
2 changes: 1 addition & 1 deletion bindings/pyroot/pythonizations/src/TTreePyz.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ static PyObject *BindBranchToProxy(TTree *tree, const char *name, TBranch *branc
TBranchElement *be = (TBranchElement *)branch;
if (be->GetCurrentClass() && (be->GetCurrentClass() != be->GetTargetClass()) && (0 <= be->GetID())) {
Long_t offset = ((TStreamerElement *)be->GetInfo()->GetElements()->At(be->GetID()))->GetOffset();
return BindCppObjectNoCast(be->GetObject() + offset, Cppyy::GetScope(be->GetCurrentClass()->GetName()));
return BindCppObjectNoCast((char *)be->GetObject() + offset, Cppyy::GetScope(be->GetCurrentClass()->GetName()));
}
}

Expand Down
2 changes: 1 addition & 1 deletion tree/tree/inc/TBranch.h
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ class TBranch : public TNamed, public TAttFill {
Int_t FlushBaskets();
Int_t FlushOneBasket(UInt_t which);

virtual char *GetAddress() const {return fAddress;}
virtual void *GetAddress() const {return (void*)fAddress;}
TBasket *GetBasket(Int_t basket) {return GetBasketImpl(basket, nullptr);}
Int_t *GetBasketBytes() const {return fBasketBytes;}
Long64_t *GetBasketEntry() const {return fBasketEntry;}
Expand Down
4 changes: 2 additions & 2 deletions tree/tree/inc/TBranchElement.h
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ class TBranchElement : public TBranch {
void Browse(TBrowser* b) override;
TBranch *FindBranch(const char *name) override;
TLeaf *FindLeaf(const char *name) override;
char *GetAddress() const override;
void *GetAddress() const override;
TBranchElement *GetBranchCount() const { return fBranchCount; }
TBranchElement *GetBranchCount2() const { return fBranchCount2; }
Int_t *GetBranchOffset() const { return fBranchOffset; }
Expand All @@ -195,7 +195,7 @@ class TBranchElement : public TBranch {
Int_t GetID() const { return fID; }
TStreamerInfo *GetInfo() const;
bool GetMakeClass() const override;
char *GetObject() const;
void *GetObject() const;
TVirtualArray *GetOnfileObject() const { return fOnfileObject; }
virtual const char *GetParentName() const { return fParentName.Data(); }
virtual Int_t GetMaximum() const;
Expand Down
8 changes: 4 additions & 4 deletions tree/tree/src/TBranchElement.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -1920,10 +1920,10 @@ TLeaf* TBranchElement::FindLeaf(const char *name)
///
/// - Return a pointer to our object.

char* TBranchElement::GetAddress() const
void* TBranchElement::GetAddress() const
{
ValidateAddress();
return fAddress;
return (void*)fAddress;
}


Expand Down Expand Up @@ -2884,10 +2884,10 @@ Int_t TBranchElement::GetMaximum() const
////////////////////////////////////////////////////////////////////////////////
/// Return a pointer to our object.

char* TBranchElement::GetObject() const
void* TBranchElement::GetObject() const
{
ValidateAddress();
return fObject;
return (void*)fObject;
}

////////////////////////////////////////////////////////////////////////////////
Expand Down
2 changes: 1 addition & 1 deletion tree/tree/src/TTree.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -3308,7 +3308,7 @@ void TTree::CopyAddresses(TTree* tree, bool undo)
TBranch* br = tree->GetBranch(branch->GetName());
tree->ResetBranchAddress(br);
} else {
char* addr = branch->GetAddress();
void* addr = branch->GetAddress();
if (!addr) {
if (branch->IsA() == TBranch::Class()) {
// If the branch was created using a leaflist, the branch itself may not have
Expand Down
6 changes: 3 additions & 3 deletions tree/treeplayer/src/TFormLeafInfo.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,9 @@ char* TFormLeafInfo::GetObjectAddress(TLeafElement* leaf, Int_t& instance)
// Branch is a top-level branch.
if (branch->GetTree()->GetMakeClass()) {
// Branch belongs to a MakeClass tree.
return branch->GetAddress();
return (char*) branch->GetAddress();
} else {
return branch->GetObject();
return (char*) branch->GetObject();
}
}
TStreamerInfo* info = branch->GetInfo();
Expand All @@ -211,7 +211,7 @@ char* TFormLeafInfo::GetObjectAddress(TLeafElement* leaf, Int_t& instance)
char* thisobj = nullptr;
if (!address) {
// FIXME: This makes no sense, if the branch address is not set, then object will not be set either.
thisobj = branch->GetObject();
thisobj = (char*) branch->GetObject();
} else {
Int_t type = -1;
if (id > -1) {
Expand Down
8 changes: 4 additions & 4 deletions tree/treeplayer/src/TTreeFormula.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -4480,11 +4480,11 @@ Double_t TTreeFormula::GetValueFromMethod(Int_t i, TLeaf* leaf) const
}
}
if (id < 0) {
char* address = branch->GetObject();
char* address = (char*) branch->GetObject();
thisobj = address;
} else {
//char* address = branch->GetAddress();
char* address = branch->GetObject();
char* address = (char*) branch->GetObject();
if (address) {
thisobj = *((char**) (address + offset));
} else {
Expand Down Expand Up @@ -4540,11 +4540,11 @@ void* TTreeFormula::GetValuePointerFromMethod(Int_t i, TLeaf* leaf) const
}
}
if (id < 0) {
char* address = branch->GetObject();
char* address = (char*) branch->GetObject();
thisobj = address;
} else {
//char* address = branch->GetAddress();
char* address = branch->GetObject();
char* address = (char*) branch->GetObject();
if (address) {
thisobj = *((char**) (address + offset));
} else {
Expand Down
2 changes: 1 addition & 1 deletion tree/treeplayer/src/TTreeGeneratorBase.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ namespace Internal {
Long64_t i = branch->GetTree()->GetReadEntry();
if (i<0) i = 0;
branch->GetEntry(i);
char *obj = branch->GetObject();
char *obj = (char*) branch->GetObject();

TBranchElement *parent = (TBranchElement*)branch->GetMother()->GetSubBranch(branch);
const char *pclname = parent->GetClassName();
Expand Down

0 comments on commit da36e02

Please sign in to comment.