Skip to content

Fix nullable-to-nonnull conversion warnings in ExecuTorchModule#19462

Open
FieryRobot wants to merge 1 commit into
pytorch:mainfrom
FieryRobot:export-D104697173
Open

Fix nullable-to-nonnull conversion warnings in ExecuTorchModule#19462
FieryRobot wants to merge 1 commit into
pytorch:mainfrom
FieryRobot:export-D104697173

Conversation

@FieryRobot
Copy link
Copy Markdown

Summary:

Motivation

We have a build failure because the Xcode staging toolchain (clang19) treats -Wnullable-to-nonnull-conversion as an error. NSString.UTF8String is declared as returning const char * _Nullable, but ExecuTorch's C++ Module methods expect const char * _Nonnull. The production toolchain doesn't flag this, but the staging toolchain does, blocking the affected build.

This Diff

Adds a safeUTF8String() helper that returns string.UTF8String ?: "" — falling back to an empty string if UTF8String ever returns nil. Applied to all 8 call sites in ExecuTorchModule.mm.

Alternatives considered:

  • Explicit nil check + error return at each site: Safest, but adds 5+ lines of boilerplate at each of 8 sites. Overkill given UTF8String on a valid NSString essentially never returns nil.
  • Cast to _Nonnull: Simpler one-liner but crashes with undefined behavior if UTF8String ever did return nil.
  • #pragma clang diagnostic ignored: Suppresses the warning but doesn't fix the underlying issue.

The ?: "" approach was chosen because: it won't crash (empty string is valid), ExecuTorch will return a "method not found" error which existing error handling already surfaces, and the nil case is essentially impossible for ASCII method name strings. The only downside is a slightly misleading error message in the astronomically unlikely failure case.

Differential Revision: D104697173

Summary:
## Motivation

We have a build failure because the Xcode staging toolchain (clang19) treats `-Wnullable-to-nonnull-conversion` as an error. `NSString.UTF8String` is declared as returning `const char * _Nullable`, but ExecuTorch's C++ Module methods expect `const char * _Nonnull`. The production toolchain doesn't flag this, but the staging toolchain does, blocking the affected build.

## This Diff

Adds a `safeUTF8String()` helper that returns `string.UTF8String ?: ""` — falling back to an empty string if UTF8String ever returns nil. Applied to all 8 call sites in ExecuTorchModule.mm.

Alternatives considered:
- **Explicit nil check + error return at each site**: Safest, but adds 5+ lines of boilerplate at each of 8 sites. Overkill given UTF8String on a valid NSString essentially never returns nil.
- **Cast to _Nonnull**: Simpler one-liner but crashes with undefined behavior if UTF8String ever did return nil.
- **`#pragma clang diagnostic ignored`**: Suppresses the warning but doesn't fix the underlying issue.

The `?: ""` approach was chosen because: it won't crash (empty string is valid), ExecuTorch will return a "method not found" error which existing error handling already surfaces, and the nil case is essentially impossible for ASCII method name strings. The only downside is a slightly misleading error message in the astronomically unlikely failure case.

Differential Revision: D104697173
@FieryRobot FieryRobot requested a review from shoumikhin as a code owner May 11, 2026 18:44
@pytorch-bot
Copy link
Copy Markdown

pytorch-bot Bot commented May 11, 2026

🔗 Helpful Links

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

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

❌ 1 Awaiting Approval, 1 New Failure

As of commit 8002ef0 with merge base 126507c (image):

AWAITING APPROVAL - The following workflow needs approval before CI can run:

NEW FAILURE - The following job has failed:

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 May 11, 2026
@meta-codesync
Copy link
Copy Markdown
Contributor

meta-codesync Bot commented May 11, 2026

@FieryRobot has exported this pull request. If you are a Meta employee, you can view the originating Diff in D104697173.

@github-actions
Copy link
Copy Markdown

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

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. fb-exported meta-exported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant