Skip to content

Conversation

@ezyang
Copy link
Contributor

@ezyang ezyang commented Aug 15, 2018

Differential Revision: D9346040

ezyang added 2 commits August 15, 2018 14:50
Summary:
Have you ever written an operator<< overload in the caffe2 namespace
in a core Caffe2 header, and then been stunned when some completely
unrelated code started breaking?  This diff fixes this problem!

The problem looks like this:
1. You're building against a really old version of glog (think 0.3.2,
   or something like that)
2. This version of glog defines operator<< overloads for std containers
   in the global namespace
3. You add a new overload in your current namespace (e.g., caffe2).
   Congratulations: this overload is *preferentially* chosen over
   the global namespace one for all calls to << in that namespace.
   And since it doesn't actually have std::vector overloads, unrelated
   Caffe2 code breaks.

Newer versions of glog have a fix for this: they have the line:

  namespace std { using ::operator<<; }

in their header.  So let's help old versions of glog out and do this ourselves.

In our new world order, operator<< overloads defined in the global namespace
won't work (unless they're for std containers, which work because of ADL).
So this diff also moves all those overloads to the correct namespace.

Differential Revision: D9344540

fbshipit-source-id: 1f3d5c69ccf0fd2003db6f7737b76914dc4dcdaa
Summary: Pull Request resolved: pytorch#10547

Differential Revision: D9346040

fbshipit-source-id: 14d5cf3480c2a87a8feb36a17a28ef8b1c8bb228
Copy link
Contributor

@gchanan gchanan left a comment

Choose a reason for hiding this comment

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

last commit looks good.

zdevito pushed a commit to zdevito/ATen that referenced this pull request Aug 16, 2018
Summary: Pull Request resolved: pytorch/pytorch#10547

Reviewed By: soumith

Differential Revision: D9346040

fbshipit-source-id: 1069a44182ccff68b1694086c8b709ba2046b22b
@ezyang ezyang added the merged label Jun 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants