Skip to content

Use std::optional/string_view #20213

Open
cyyever wants to merge 5 commits into
pytorch:mainfrom
cyyever:std_opt
Open

Use std::optional/string_view #20213
cyyever wants to merge 5 commits into
pytorch:mainfrom
cyyever:std_opt

Conversation

@cyyever

@cyyever cyyever commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR changes callers to use std::optional/string_view.

Test plan

Check whether the system can build.

@pytorch-bot

pytorch-bot Bot commented Jun 11, 2026

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/20213

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 11, 2026
@cyyever

cyyever commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

@pytorchbot label "release notes: none"

@pytorch-bot pytorch-bot Bot added the release notes: none Do not include this in the release notes label Jun 11, 2026
@rascani

rascani commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Thanks for making this change!

Does this remove all uses of torch::executor::optional and executorch::runtime::etensor::optional? If so, then it would probably be good to remove those type aliases to ensure new usages don't get re-introduced.

There are a few clang-format errors coming from lintrunner that need to be cleaned up.

@cyyever

cyyever commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

@rascani I added more cleanup and fixed the linters.

@rascani

rascani commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

I'm sorry @cyyever, I gave you bad advice. We won't be able to remove the type aliases or header files as they are part of the public API and are subject to the API Life Cycle and Deprecation Policy. We'll need to mark them as deprecated first, and cannot remove them for 2 minor releases.

Here's an example of how the optional aliases would look:

#include <optional>
#include <executorch/runtime/platform/compiler.h>

namespace executorch::runtime::etensor {
template <typename T>
using optional ET_DEPRECATED = std::optional<T>;
using nullopt_t ET_DEPRECATED = std::nullopt_t;
ET_DEPRECATED inline constexpr std::nullopt_t nullopt{std::nullopt};
}  // namespace executorch::runtime::etensor

namespace torch::executor {  // legacy spelling
template <typename T>
using optional ET_DEPRECATED = std::optional<T>;
using nullopt_t ET_DEPRECATED = std::nullopt_t;
ET_DEPRECATED inline constexpr std::nullopt_t nullopt{std::nullopt};
}  // namespace torch::executor

I'll put up a separate PR for adding message support to ET_DEPRECATED, so ideally we could do:

template <typename T>
using optional ET_DEPRECATED("use std::optional; removal in 1.6") = std::optional<T>;

@nil-is-all

Copy link
Copy Markdown
Contributor

I'm sorry @cyyever, I gave you bad advice. We won't be able to remove the type aliases or header files as they are part of the public API and are subject to the API Life Cycle and Deprecation Policy. We'll need to mark them as deprecated first, and cannot remove them for 2 minor releases.

Here's an example of how the optional aliases would look:

#include <optional>
#include <executorch/runtime/platform/compiler.h>

namespace executorch::runtime::etensor {
template <typename T>
using optional ET_DEPRECATED = std::optional<T>;
using nullopt_t ET_DEPRECATED = std::nullopt_t;
ET_DEPRECATED inline constexpr std::nullopt_t nullopt{std::nullopt};
}  // namespace executorch::runtime::etensor

namespace torch::executor {  // legacy spelling
template <typename T>
using optional ET_DEPRECATED = std::optional<T>;
using nullopt_t ET_DEPRECATED = std::nullopt_t;
ET_DEPRECATED inline constexpr std::nullopt_t nullopt{std::nullopt};
}  // namespace torch::executor

I'll put up a separate PR for adding message support to ET_DEPRECATED, so ideally we could do:

template <typename T>
using optional ET_DEPRECATED("use std::optional; removal in 1.6") = std::optional<T>;

Hi @rascani, in that case do we close this PR for now?

@rascani

rascani commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

I'm sorry @cyyever, I gave you bad advice. We won't be able to remove the type aliases or header files as they are part of the public API and are subject to the API Life Cycle and Deprecation Policy. We'll need to mark them as deprecated first, and cannot remove them for 2 minor releases.
Here's an example of how the optional aliases would look:

#include <optional>
#include <executorch/runtime/platform/compiler.h>

namespace executorch::runtime::etensor {
template <typename T>
using optional ET_DEPRECATED = std::optional<T>;
using nullopt_t ET_DEPRECATED = std::nullopt_t;
ET_DEPRECATED inline constexpr std::nullopt_t nullopt{std::nullopt};
}  // namespace executorch::runtime::etensor

namespace torch::executor {  // legacy spelling
template <typename T>
using optional ET_DEPRECATED = std::optional<T>;
using nullopt_t ET_DEPRECATED = std::nullopt_t;
ET_DEPRECATED inline constexpr std::nullopt_t nullopt{std::nullopt};
}  // namespace torch::executor

I'll put up a separate PR for adding message support to ET_DEPRECATED, so ideally we could do:

template <typename T>
using optional ET_DEPRECATED("use std::optional; removal in 1.6") = std::optional<T>;

Hi @rascani, in that case do we close this PR for now?

No, not unless any other maintainer objects to deprecating the optional/string_view aliases.

When we add the ET_DEPRECATED macro, we'll start getting compiler warnings on all uses in the executorch codebase. The bulk of this PR is the migration of those uses to the std:: equivalents.

We just need to revert the header file removals and add the ET_DEPRECATED macros.

cyyever added 4 commits June 16, 2026 11:19
Migrate the remaining executorch::aten::optional callers (mostly the
Cadence operators) to std::optional, then delete the deprecated
torch::executor and executorch::runtime::etensor optional/nullopt
aliases so they can't be reintroduced. portable_type/optional.h held
only those aliases and is removed; exec_aten.h now includes <optional>
directly. optional_test.cpp covered only the alias (now plain
std::optional) and is dropped along with its build targets.

The substantive changes are in runtime/core/exec_aten/exec_aten.h and
the removal of runtime/core/portable_type/optional.h; everything under
backends/cadence and kernels/ is the mechanical caller migration.
…_view

Migrate the remaining executorch::aten::string_view callers (the data-map
APIs and op_linalg_svd) to std::string_view, then delete the deprecated
torch::executor and executorch::runtime::etensor string_view aliases.
portable_type/string_view.h held only those aliases and is removed;
exec_aten.h and scalar_type_util.h now include <string_view> directly.

This parallels the optional cleanup in the previous commit. The data-map
header signatures (NamedDataMap and its overrides) move together since
the underlying type is unchanged.
The optional/string_view type aliases and their headers are public API
under the API Life Cycle and Deprecation Policy, so they cannot be removed
until two minor releases after being deprecated. Restore
portable_type/optional.h and string_view.h (and their buck targets), now
aliasing the etensor and legacy torch::executor names to std:: behind
ET_DEPRECATED.

The caller migration to std:: is kept; exec_aten.h and scalar_type_util.h
retain the migrated std:: usages while re-including the restored headers so
external users still resolve the deprecated names.
@cyyever

cyyever commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

@rascani @nil-is-all Restored them and added ET_DEPRECATED

@rascani

rascani commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

@cyyever - Can you revert the last commit 901df7a? It looks like that clang-format run disagreed with the clang-format that is run by lintrunner (could be different versions or config).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. release notes: none Do not include this in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants