Opened 8 years ago

Closed 7 years ago

#2104 closed defect (wontfix)

bare shared_ptr in static_datasrc_link.cc

Reported by: fdupont Owned by:
Priority: high Milestone:
Component: Unclassified Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: Medium
Sub-Project: Core Feature Depending on Ticket:
Estimated Difficulty: 3 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

in src/lib/datasrc/static_datasrc_link.cc :
shared_ptr -> boost::shared_ptr

Subtickets

Attachments (4)

client_list_unittest.cc.diff (3.1 KB) - added by fdupont 8 years ago.
client_list_unittest.cc fix
memory_datasrc_unittest.cc.diff (1023 bytes) - added by fdupont 8 years ago.
memory_datasrc_unittest.cc fix
zone_finder_context_unittest.cc.diff (1.2 KB) - added by fdupont 8 years ago.
zone_finder_context_unittest.cc fix
test_client.cc.diff (1.4 KB) - added by fdupont 8 years ago.
test_client.cc fix

Download all attachments as: .zip

Change History (13)

Changed 8 years ago by fdupont

client_list_unittest.cc fix

Changed 8 years ago by fdupont

memory_datasrc_unittest.cc fix

Changed 8 years ago by fdupont

zone_finder_context_unittest.cc fix

Changed 8 years ago by fdupont

test_client.cc fix

comment:1 Changed 8 years ago by fdupont

Got the same issue in 4 other files (in src/lib/datasrc/tests).
Attached fixes as they are less trivial.

comment:2 Changed 8 years ago by shane

  • Milestone changed from New Tasks to Next-Sprint-Proposed

comment:3 follow-up: Changed 8 years ago by vorner

Why is this „fix“ needed? There should be using boost at the top, so it should be found and I think the longer name is uglier.

comment:4 in reply to: ↑ 3 ; follow-up: Changed 8 years ago by fdupont

Replying to vorner:

Why is this „fix“ needed? There should be using boost at the top, so it should be found and I think the longer name is uglier.

=> you have (cur & paste from the file in the title):

using namespace boost;
using namespace std;

so how to choose between boost::shared_ptr and std::shared_ptr when you have a bare shared_ptr? In conclusion namespaces can solve conflicts only when they are use.
(BTW the source of the issue is the TR1: Microsoft implements it and shared_ptr is part of it)

comment:5 in reply to: ↑ 4 ; follow-up: Changed 8 years ago by jinmei

Replying to fdupont:

Why is this „fix“ needed? There should be using boost at the top, so it should be found and I think the longer name is uglier.

=> you have (cur & paste from the file in the title):

using namespace boost;
using namespace std;

so how to choose between boost::shared_ptr and std::shared_ptr when you have a bare shared_ptr? In conclusion namespaces can solve conflicts only when they are use.
(BTW the source of the issue is the TR1: Microsoft implements it and shared_ptr is part of it)

Just checking: so, as long as we do 'using namespace std', 'using
boost::shared_ptr' (instead of 'using namespace boost') doesn't work
either, right?

Now (with C++11) the standard namespace is going to contain a lot more
symbols, maybe we should use the same practice for 'std' as that for
the name space boost, i.e, at least don't incorporate the entire
namespace but be reasonably specific to avoid conflict (like 'using
std::string').

comment:6 Changed 8 years ago by shane

  • Estimated Difficulty changed from 0 to 3

comment:7 in reply to: ↑ 5 Changed 8 years ago by fdupont

Replying to jinmei:

Now (with C++11) the standard namespace is going to contain a lot more
symbols, maybe we should use the same practice for 'std' as that for
the name space boost, i.e, at least don't incorporate the entire
namespace but be reasonably specific to avoid conflict (like 'using
std::string').

=> fully agree: when importing the universe the probability of a name collision increases up to one...

comment:8 Changed 7 years ago by jinmei

We are (probably) going to deprecate static_datasrc in (as a side effect of) #2833 anyway.
I'll close this ticket as "wontfix".

comment:9 Changed 7 years ago by jinmei

  • Resolution set to wontfix
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.