Skip to content

Use std::optional #12707

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 6 commits into from
Nov 5, 2025
Merged

Use std::optional #12707

merged 6 commits into from
Nov 5, 2025

Conversation

@cyyever
Copy link
Contributor

@cyyever cyyever commented Jul 22, 2025

Summary

Move remaining usage of optional to std::optional.

@pytorch-bot
Copy link

pytorch-bot bot commented Jul 22, 2025 •
edited
Loading

🔗 Helpful Links

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

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:

❌ 1 New Failure, 3 Unrelated Failures

As of commit df13091 with merge base 892d40e (image):

NEW FAILURE - The following job has failed:

FLAKY - The following job failed but was likely due to flakiness present on trunk:

BROKEN TRUNK - The following jobs failed but was present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

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

@facebook-github-bot facebook-github-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 Jul 22, 2025
@manuelcandales manuelcandales added topic: not user facing release notes: none Do not include this in the release notes labels Jul 29, 2025
@cyyever cyyever changed the title Use std::optinal Use std::optional Jul 30, 2025
@cyyever
Copy link
Contributor Author

cyyever commented Jul 30, 2025

@manuelcandales I have fixed the build errors.

@cyyever cyyever marked this pull request as draft August 13, 2025 00:45
@cyyever
Copy link
Contributor Author

cyyever commented Aug 13, 2025

@JacobSzwejbka Fixed

@cyyever cyyever marked this pull request as ready for review August 13, 2025 00:55
@cyyever cyyever requested a review from SS-JIA as a code owner August 13, 2025 01:00
@swolchok
Copy link
Contributor

swolchok commented Sep 11, 2025

the optional classes being replaced here are already synonymous with std::optional and therefore this is a cosmetic change, right? If so, with the 1.0 branch cut looming, let's hold off on merging this one.

@cyyever
Copy link
Contributor Author

cyyever commented Sep 11, 2025

@swolchok Thank you for your kindly support.

@swolchok
Copy link
Contributor

swolchok commented Nov 3, 2025

1.0 is safely out the door! mind knocking out a rebase?

@cyyever
Copy link
Contributor Author

cyyever commented Nov 3, 2025

@swolchok Done

swolchok
swolchok approved these changes Nov 3, 2025
View reviewed changes
Copy link
Contributor

@swolchok swolchok left a comment

Choose a reason for hiding this comment

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

thanks! intend to merge when CI is green

@cyyever cyyever requested a review from shoumikhin as a code owner November 4, 2025 01:24
@cyyever
Copy link
Contributor Author

cyyever commented Nov 4, 2025

@swolchok I need some iterations to fix CI errors.

@pytorch-bot
Copy link

pytorch-bot bot commented Nov 4, 2025

To add the ciflow label ciflow/trunk please first approve the workflows that are awaiting approval (scroll to the bottom of this page).

This helps ensure we don't trigger CI on this PR until it is actually authorized to do so. Please ping one of the reviewers if you do not have access to approve and run workflows.

@pytorch-bot pytorch-bot bot removed the ciflow/trunk label Nov 4, 2025
@cyyever
Use std::optinal
8955acd
@cyyever
Fix compile errors
39a5c76 
Signed-off-by: cyy <cyyever@outlook.com>
@cyyever
Use std::optional
2adc08c 
Signed-off-by: cyy <cyyever@outlook.com>
@cyyever
More fixes
1734ec5 
Signed-off-by: Yuanyuan Chen <cyyever@outlook.com>
@cyyever
More fixes
e693340 
Signed-off-by: Yuanyuan Chen <cyyever@outlook.com>
@cyyever
Format code
df13091 
Signed-off-by: Yuanyuan Chen <cyyever@outlook.com>
@cyyever
Copy link
Contributor Author

cyyever commented Nov 5, 2025

@swolchok @shoumikhin The current CI failures are unrelated.

@swolchok swolchok merged commit 3f4f500 into pytorch:main Nov 5, 2025
138 of 142 checks passed
@cyyever cyyever deleted the stdsv5 branch November 5, 2025 01:12
metascroy added a commit that referenced this pull request Nov 5, 2025
@metascroy
Revert "Use std::optional (#12707)"
ece021b 
This reverts commit 3f4f500.
@metascroy metascroy mentioned this pull request Nov 5, 2025
Merged
metascroy added a commit that referenced this pull request Nov 5, 2025
@metascroy
Revert "Use std::optional" (#15612)
1523606 
Reverts #12707

This causes 1000s of internal CI failures, e.g.,

error: no template named 'optional' in namespace 'torch::executor'; did
you mean simply 'optional'?
using optional = torch::executor::optional<T>;
                 ^~~~~~~~~~~~~~~~~~~~~~~~~
                 optional
note: 'optional' declared here
using ::executorch::aten::optional;
@cyyever cyyever restored the stdsv5 branch November 6, 2025 00:59
abhinaykukkadapu pushed a commit to abhinaykukkadapu/executorch that referenced this pull request Nov 6, 2025
@cyyever @abhinaykukkadapu
Use std::optional (pytorch#12707)
258f1f1 
### Summary
Move remaining usage of optional to `std::optional`.

---------

Signed-off-by: cyy <cyyever@outlook.com>
Signed-off-by: Yuanyuan Chen <cyyever@outlook.com>
abhinaykukkadapu pushed a commit to abhinaykukkadapu/executorch that referenced this pull request Nov 6, 2025
@metascroy @abhinaykukkadapu
Revert "Use std::optional" (pytorch#15612)
9c28244 
Reverts pytorch#12707

This causes 1000s of internal CI failures, e.g.,

error: no template named 'optional' in namespace 'torch::executor'; did
you mean simply 'optional'?
using optional = torch::executor::optional<T>;
                 ^~~~~~~~~~~~~~~~~~~~~~~~~
                 optional
note: 'optional' declared here
using ::executorch::aten::optional;
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@swolchok swolchok swolchok approved these changes

@larryliu0820 larryliu0820 Awaiting requested review from larryliu0820 larryliu0820 is a code owner

@lucylq lucylq Awaiting requested review from lucylq lucylq is a code owner

@JacobSzwejbka JacobSzwejbka Awaiting requested review from JacobSzwejbka JacobSzwejbka is a code owner

@manuelcandales manuelcandales Awaiting requested review from manuelcandales manuelcandales is a code owner

@jackzhxng jackzhxng Awaiting requested review from jackzhxng jackzhxng is a code owner

@mergennachin mergennachin Awaiting requested review from mergennachin mergennachin is a code owner

@kimishpatel kimishpatel Awaiting requested review from kimishpatel kimishpatel is a code owner

@SS-JIA SS-JIA Awaiting requested review from SS-JIA SS-JIA is a code owner

@shoumikhin shoumikhin Awaiting requested review from shoumikhin

Assignees

No one assigned

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 topic: not user facing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@cyyever @swolchok @shoumikhin @facebook-github-bot @manuelcandales