Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bugfix for FwdRef #3850

Merged
merged 1 commit into from May 16, 2014
Merged

bugfix for FwdRef #3850

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 5 additions & 4 deletions DataFormats/Common/interface/FwdPtr.h
View file
Open in desktop
Expand Up @@ -34,6 +34,7 @@
// system include files
#include "boost/type_traits/is_base_of.hpp"
#include "boost/utility/enable_if.hpp"
#include <cassert>

// forward declarations
namespace edm {
Expand All @@ -49,7 +50,7 @@ namespace edm {
template<typename C>
FwdPtr(const Ptr<C>& f, const Ptr<C>& b):
ptr_(f), backPtr_(b)
{}
{ assert(f.isNull() == b.isNull()); }

FwdPtr() :
ptr_(), backPtr_()
Expand All @@ -72,14 +73,14 @@ namespace edm {

/// Dereference operator
T const&
operator*() const { return ptr_.isNonnull() ? ptr_.operator*() : backPtr_.operator*();}
operator*() const { return ptr_.isAvailable() ? ptr_.operator*() : backPtr_.operator*();}

/// Member dereference operator
T const*
operator->() const{ return ptr_.isNonnull() ? ptr_.operator->() : backPtr_.operator->();}
operator->() const{ return ptr_.isAvailable() ? ptr_.operator->() : backPtr_.operator->();}

/// Returns C++ pointer to the item
T const* get() const { return ptr_.isNonnull() ? ptr_.get() : backPtr_.get();}
T const* get() const { return ptr_.isAvailable() ? ptr_.get() : backPtr_.get();}


/// Checks for null
Expand Down
8 changes: 3 additions & 5 deletions DataFormats/Common/interface/FwdRef.h
View file
Open in desktop
Expand Up @@ -140,7 +140,7 @@ namespace edm {
/// General purpose constructor from 2 refs (forward and backward.
FwdRef(Ref<C,T,F> const & ref,
Ref<C,T,F> const & backRef) :
ref_(ref), backRef_(backRef) {}
ref_(ref), backRef_(backRef) { assert(ref.isNull() == backRef.isNull()); }

/// Destructor
~FwdRef() {}
Expand All @@ -155,12 +155,10 @@ namespace edm {

/// Returns C++ pointer to the item
T const* get() const {
if ( ref_.isNonnull() ) {
if ( ref_.isAvailable() ) {
return ref_.get();
} else if ( backRef_.isNonnull() ) {
return backRef_.get();
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need for the else here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it hurt? do you want us to update the PR to remove that? (please note that this commit is also used in other branches and we would prefered not to change this unless really required, to avoid merge/rebase nightmares)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It used to be that some compilers would complain because they would think it was possible to reach the end of the function and not return a value. Could you please make another pull to remove it which we will include after this one to avoid your nightmare scenario?

return 0;
return backRef_.get();
}
}

Expand Down