RESOLVED FIXED 29919
HashTable.h has using directives for std::pair and std::make_pair
https://bugs.webkit.org/show_bug.cgi?id=29919
Summary HashTable.h has using directives for std::pair and std::make_pair
Kent Hansen
Reported 2009-09-30 05:20:39 PDT
Created attachment 40362 [details] Proposed patch The compiler complains about ambiguity in calls, apparently because there is a "using std::make_pair" in the WTF namespace (wtf/HashTraits.h) and Structure.cpp has both "using namespace std" and "using namespace WTF". Removing "using namespace std" makes it compile without any other changes needed. Compiler output: ..\3rdparty\javascriptcore\JavaScriptCore\runtime\Structure.cpp(157) : error C2872: 'make_pair' : ambiguous symbol could be ''unknown-type' make_pair' or '..\3rdparty\javascriptcore\JavaScriptCore\wtf\HashTraits.h(32) : std::pair<_Ty1,_Ty2> std::make_pair(_Ty1,_Ty2)' or 'make_pair' ..\3rdparty\javascriptcore\JavaScriptCore\runtime\Structure.cpp(397) : error C2872: 'make_pair' : ambiguous symbol could be ''unknown-type' make_pair' or '..\3rdparty\javascriptcore\JavaScriptCore\wtf\HashTraits.h(32) : std::pair<_Ty1,_Ty2> std::make_pair(_Ty1,_Ty2)' or 'make_pair' ..\3rdparty\javascriptcore\JavaScriptCore\runtime\Structure.cpp(451) : error C2872: 'make_pair' : ambiguous symbol could be ''unknown-type' make_pair' or '..\3rdparty\javascriptcore\JavaScriptCore\wtf\HashTraits.h(32) : std::pair<_Ty1,_Ty2> std::make_pair(_Ty1,_Ty2)' or 'make_pair' ..\3rdparty\javascriptcore\JavaScriptCore\runtime\Structure.cpp(875) : error C2872: 'make_pair' : ambiguous symbol could be ''unknown-type' make_pair' or '..\3rdparty\javascriptcore\JavaScriptCore\wtf\HashTraits.h(32) : std::pair<_Ty1,_Ty2> std::make_pair(_Ty1,_Ty2)' or 'make_pair' ..\3rdparty\javascriptcore\JavaScriptCore\runtime\Structure.cpp(1037) : error C2668: 'std::max' : ambiguous call to overloaded function C:\Program Files\Microsoft Visual Studio .NET 2003\Vc7\include\xutility(1242): could be 'const unsigned int &std::max(const unsigned int &,const unsigned int &)' C:\Program Files\Microsoft Visual Studio .NET 2003\Vc7\include\xutility(1242): or 'const unsigned int &std::max(const unsigned int &,const unsigned int &)' while trying to match the argument list '(unsigned int, unsigned int)'
Attachments
Proposed patch (338 bytes, patch)
2009-09-30 05:20 PDT, Kent Hansen
no flags
Patch (10.84 KB, patch)
2012-05-29 07:01 PDT, Caio Marcelo de Oliveira Filho
no flags
Patch (11.81 KB, patch)
2012-05-29 07:54 PDT, Caio Marcelo de Oliveira Filho
darin: review+
Darin Adler
Comment 1 2009-09-30 10:18:49 PDT
I think the better fix is to find a way remove the using std::make_pair from HashTraits.h. Generally we don't want to do using in headers.
Darin Adler
Comment 2 2009-09-30 10:19:13 PDT
Is compiling in the two-versions-old version of Visual Studio a goal for the project?
Eric Seidel (no email)
Comment 3 2009-10-02 12:45:25 PDT
Comment on attachment 40362 [details] Proposed patch Every patch requires a ChangeLog. Also darin's comments above suggest this should not be landed as-is. http://webkit.org/coding/contributing.html
Caio Marcelo de Oliveira Filho
Comment 4 2012-05-29 07:01:33 PDT
Andreas Kling
Comment 5 2012-05-29 07:03:57 PDT
Comment on attachment 144549 [details] Patch Seems legit. r=me. Probably wanna wait for EWSes just to be on the safe side.
Build Bot
Comment 6 2012-05-29 07:33:54 PDT
Caio Marcelo de Oliveira Filho
Comment 7 2012-05-29 07:54:32 PDT
Darin Adler
Comment 8 2012-05-29 13:26:55 PDT
Comment on attachment 144558 [details] Patch Normally it’s safer to do such patches in pairs. The first patch adds all the "std::" prefixes. Then the second one only removes the “using”. That way, if there is a problem and a roll-out is necessary, we only roll out a small change.
Caio Marcelo de Oliveira Filho
Comment 9 2012-05-30 09:26:20 PDT
(In reply to comment #8) > (From update of attachment 144558 [details]) > Normally it’s safer to do such patches in pairs. The first patch adds all the "std::" prefixes. Then the second one only removes the “using”. That way, if there is a problem and a roll-out is necessary, we only roll out a small change. Thanks. Will land that way.
Caio Marcelo de Oliveira Filho
Comment 10 2012-05-30 10:20:13 PDT
Caio Marcelo de Oliveira Filho
Comment 11 2012-05-30 14:45:23 PDT
Note You need to log in before you can comment on or make changes to this bug.