Skip to content

Hide c10::optional and nullopt in torch namespace#12927

Closed
goldsborough wants to merge 2 commits intopytorch:masterfrom
goldsborough:cpp-optional
Closed

Hide c10::optional and nullopt in torch namespace#12927
goldsborough wants to merge 2 commits intopytorch:masterfrom
goldsborough:cpp-optional

Conversation

@goldsborough
Copy link
Copy Markdown
Contributor

Does

namespace torch {
using c10::optional;
using c10::nullopt;
}

So that users can be oblivious of our changes with ATen/c10 happening in the background, and also don't have to deal with multiple namespaces (which is very confusing).

@ezyang

@goldsborough goldsborough requested a review from ebetica as a code owner October 22, 2018 12:34
Copy link
Copy Markdown
Contributor

@ezyang ezyang left a comment

Choose a reason for hiding this comment

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

This headed the right direction, so I'll accept.

NB: The PLAN ON RECORD is to add using namespace c10 inside at and caffe2 (and torch too, I suppose) so that all access here can be un-namespaced, except when you get to the user.

CC @Yangqing

@goldsborough goldsborough force-pushed the cpp-optional branch 5 times, most recently from 418a9dc to 7fc9539 Compare October 23, 2018 16:27
Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

goldsborough has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

goldsborough is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

goldsborough is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@ezyang ezyang added the merged label Jun 25, 2019
laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
Summary:
Does

```cpp
namespace torch {
using c10::optional;
using c10::nullopt;
}
```

So that users can be oblivious of our changes with ATen/c10 happening in the background, and also don't have to deal with multiple namespaces (which is very confusing).

ezyang
Pull Request resolved: pytorch#12927

Differential Revision: D10510630

Pulled By: goldsborough

fbshipit-source-id: e456264f2fbca3eda277712de11cdd8acc77fbd4
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.

3 participants