Conversation
Halide's Target::OS used the increasingly-out-of-date "osx" string and enum name for macOS. Rename the canonical name to MacOS/"macos" everywhere internally, while keeping OSX/"osx" working as a deprecated alias: the enum still defines OSX (marked [[deprecated]]) equal to MacOS, and target string parsing still accepts "osx" but emits a user_warning and prints "macos" back out. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
alexreinking
approved these changes
Sep 15, 2026
alexreinking
left a comment
Member
There was a problem hiding this comment.
LGTM with one possible enhancement
Wrap the pybind11-registered TargetOS.OSX class attribute with a descriptor in halide/__init__.py so accessing it raises a DeprecationWarning pointing at TargetOS.MacOS, matching the C++-side [[deprecated]] attribute on Target::OSX. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
I actually wouldn't mind seeing |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9451 +/- ##
==========================================
- Coverage 70.12% 70.06% -0.06%
==========================================
Files 261 261
Lines 79818 79937 +119
Branches 19445 19479 +34
==========================================
+ Hits 55972 56009 +37
- Misses 17991 18077 +86
+ Partials 5855 5851 -4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Move the deprecation shim below the module imports (E402) and name TargetOS explicitly rather than relying on the star import (F405). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Target::OSused the increasingly out-of-date string/enum nameosxfor macOS. This renames the canonical form toMacOS/"macos"throughout the codebase.Target::OSXis kept as a[[deprecated]]alias enumerator equal toTarget::MacOS, so old C++ code still compiles but gets a-Wdeprecated-declarationswarning."osx"as an alias for"macos", but emits auser_warningwhen it does, andTarget::to_string()now always prints"macos".hl.TargetOS.OSX) similarly: kept as a deprecated alias, following the same pattern used forTailStrategy::Predicate.HalideTargetHelpers.cmake,HalideGeneratorHelpers.cmake), the autoscheduler/packaging CMakeLists that referenced-osxtriples, and docs, so host-detected macOS targets now come out as-macosend-to-end.Target::OSX/hl.TargetOS.OSX(tests, tutorials, codegen) over to the newMacOSname so nothing in-tree triggers the new deprecation warning.In the future, using the deprecated
OSX/"osx"spelling could be made a hard error, but for now it's just deprecated.Test plan
g++ -fsyntax-onlysanity checks on all touched.cpp/.hfiles (no LLVM-linked build available in this environment)Target::OSXnow triggers-Wdeprecated-declarationsstd::mapiteration order makesto_string()print"macos"(not"osx") for the parsed/host targetctest -R correctness_target(please run in CI / on a full build)