Skip to content

Disable nullable-to-nonnull-conversion on ExecuTorch (#19407)#19407

Closed
chatura-atapattu wants to merge 1 commit into
pytorch:mainfrom
chatura-atapattu:export-D104463212
Closed

Disable nullable-to-nonnull-conversion on ExecuTorch (#19407)#19407
chatura-atapattu wants to merge 1 commit into
pytorch:mainfrom
chatura-atapattu:export-D104463212

Conversation

@chatura-atapattu
Copy link
Copy Markdown

@chatura-atapattu chatura-atapattu commented May 8, 2026

Summary:

On the Xcode 26.4 compiler upgrade, we error on:

fbsource//xplat/executorch/extension/apple:ExecuTorch (objcxx_compile ExecuTorch/Exported/ExecuTorchModule.mm (pic))
Local command returned non-zero exit code 1
Reproduce locally: `env -C "$(buck2 root --kind project)" -- 'BUCK_SCRATCH_PATH=buck-out/v2/tmp/fbsource/32bf73e24dec3a3 ...<omitted>... tion___0__/output_artifacts/__dep_files_intermediaries__/ExecuTorch/Exported/ExecuTorchModule.mm.pic (run `buck2 log what-failed` to get the full command)`
stdout:
stderr:
clang++: warning: argument unused during compilation: '-fstack-clash-protection' [-Wunused-command-line-argument]
xplat/executorch/extension/apple/ExecuTorch/Exported/ExecuTorchModule.mm:317:47: error: implicit conversion from nullable pointer 'NS_RETURNS_INNER_POINTER const char *' to non-nullable pointer type 'const char * _Nonnull' [-Werror,-Wnullable-to-nonnull-conversion]
  317 |   const auto errorCode = _module->load_method(methodName.UTF8String);
      |                                               ^```

Disable this error.

Differential Revision: D104463212


@pytorch-bot
Copy link
Copy Markdown

pytorch-bot Bot commented May 8, 2026

🔗 Helpful Links

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

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 Pending, 1 Unrelated Failure

As of commit 2aa8174 with merge base 29761c8 (image):

NEW FAILURE - The following job has failed:

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

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

meta-codesync Bot commented May 8, 2026

@chatura-atapattu has exported this pull request. If you are a Meta employee, you can view the originating Diff in D104463212.

@meta-codesync meta-codesync Bot changed the title Disable nullable-to-nonnull-conversion on ExecuTorch Disable nullable-to-nonnull-conversion on ExecuTorch (#19407) May 11, 2026
chatura-atapattu added a commit to chatura-atapattu/executorch that referenced this pull request May 11, 2026
Summary:

On the Xcode 26.4 compiler upgrade, we error on:

```
fbsource//xplat/executorch/extension/apple:ExecuTorch (objcxx_compile ExecuTorch/Exported/ExecuTorchModule.mm (pic))
Local command returned non-zero exit code 1
Reproduce locally: `env -C "$(buck2 root --kind project)" -- 'BUCK_SCRATCH_PATH=buck-out/v2/tmp/fbsource/32bf73e24dec3a3 ...<omitted>... tion___0__/output_artifacts/__dep_files_intermediaries__/ExecuTorch/Exported/ExecuTorchModule.mm.pic (run `buck2 log what-failed` to get the full command)`
stdout:
stderr:
clang++: warning: argument unused during compilation: '-fstack-clash-protection' [-Wunused-command-line-argument]
xplat/executorch/extension/apple/ExecuTorch/Exported/ExecuTorchModule.mm:317:47: error: implicit conversion from nullable pointer 'NS_RETURNS_INNER_POINTER const char *' to non-nullable pointer type 'const char * _Nonnull' [-Werror,-Wnullable-to-nonnull-conversion]
  317 |   const auto errorCode = _module->load_method(methodName.UTF8String);
      |                                               ^```

Disable this error.

Differential Revision: D104463212
Summary:

On the Xcode 26.4 compiler upgrade, we error on:

```
fbsource//xplat/executorch/extension/apple:ExecuTorch (objcxx_compile ExecuTorch/Exported/ExecuTorchModule.mm (pic))
Local command returned non-zero exit code 1
Reproduce locally: `env -C "$(buck2 root --kind project)" -- 'BUCK_SCRATCH_PATH=buck-out/v2/tmp/fbsource/32bf73e24dec3a3 ...<omitted>... tion___0__/output_artifacts/__dep_files_intermediaries__/ExecuTorch/Exported/ExecuTorchModule.mm.pic (run `buck2 log what-failed` to get the full command)`
stdout:
stderr:
clang++: warning: argument unused during compilation: '-fstack-clash-protection' [-Wunused-command-line-argument]
xplat/executorch/extension/apple/ExecuTorch/Exported/ExecuTorchModule.mm:317:47: error: implicit conversion from nullable pointer 'NS_RETURNS_INNER_POINTER const char *' to non-nullable pointer type 'const char * _Nonnull' [-Werror,-Wnullable-to-nonnull-conversion]
  317 |   const auto errorCode = _module->load_method(methodName.UTF8String);
      |                                               ^```

Disable this error.

Differential Revision: D104463212
@chatura-atapattu
Copy link
Copy Markdown
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 May 11, 2026
@shoumikhin
Copy link
Copy Markdown
Contributor

Heads up: #19335 appears to address the same Xcode 26.4 issue with a different approach (inline ?: "" instead of pragma wrappers). Worth syncing before landing duplicate fixes.

@chatura-atapattu
Copy link
Copy Markdown
Author

Heads up: #19335 appears to address the same Xcode 26.4 issue with a different approach (inline ?: "" instead of pragma wrappers). Worth syncing before landing duplicate fixes.

Yeah, this was one of the first solutions I got too generated by Claude. Wasn't sure whether we really wanted to return an empty string here, which in the future will likely be taken as intended behavior, vs the diagnostic push/pop that would warrant investigating further to fix the API and why its empty in the first place.

Will mention that in the diff, but in the end, both options get me my desired end state, so happy to go with either.

@shoumikhin
Copy link
Copy Markdown
Contributor

One thing worth flagging on the pragma approach: those C++ methods take const std::string&, so the implicit conversion happens at the call site. If UTF8String ever did return NULL, std::string(nullptr) is undefined behavior (segfault on libc++ in practice). The pragma silences the warning but doesn't actually make the code safer. ?: "" at least avoids that path.

@chatura-atapattu
Copy link
Copy Markdown
Author

I'm not much of a C++ expert, so I'm happy to defer to owners of the codebase.

Will close this pull request and look to the other change.

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 release notes: none Do not include this in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants