Fix two fmt uses that fail to compile under C++20 - #7574
Open
Tobias-Fischer wants to merge 1 commit into
Open
Tobias-Fischer wants to merge 1 commit into
Tobias-Fischer wants to merge 1 commit into
Conversation
Both are latent: they compile today because Open3D sets CMAKE_CXX_STANDARD 17, and break as soon as anything raises the standard to 20 -- for example linking a dependency whose imported targets declare INTERFACE_COMPILE_FEATURES cxx_std_20. TensorMap::ToString() passes a runtime std::string as the format string. In C++20 fmt's format-string checking becomes consteval, so this is rejected: TensorMap.cpp:167: error: call to consteval function 'fmt::fstring<...>::fstring<std::string, 0>' is not a constant expression fmt::runtime() is the documented way to opt a runtime format string out of compile-time checking, and is correct under both standards. MetricParameters::ToString() formats a std::vector<float>, which needs fmt/ranges.h. It currently relies on that header arriving transitively: error: implicit instantiation of undefined template 'fmt::detail::type_is_unformattable_for<std::vector<float>, char>' Both APIs are available in the bundled fmt (9.1.0 on MSVC, 12.1.0 elsewhere); fmt::runtime() has existed since fmt 8. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017R4DS6whKCY8xGcA61kni2
|
Thanks for submitting this pull request! The maintainers of this repository would appreciate if you could update the CHANGELOG.md based on your changes. |
Tobias-Fischer
added a commit
to regro-cf-autotick-bot/open3d-feedstock
that referenced
this pull request
Sep 19, 2026
conda-forge's filament now ships the image and ktxreader libraries (conda-forge/filament-feedstock#15), which were the missing pieces that made USE_SYSTEM_FILAMENT unusable, so stop vendoring Filament. This also brings osx-64 back. That platform was skipped only because Open3D downloads filament-v1.76.0-mac.tgz, which contains lib/arm64 and no x86_64 slice. With the system package that tarball is never fetched, and conda-forge's filament has a real osx-64 build. Two patches are carried, each mirroring an upstream pull request so they can be dropped individually once merged: fix-system-filament.patch isl-org/Open3D#7573 -- USE_SYSTEM_FILAMENT looked for a lowercase filament package exporting filament::, did not request a ktxreader target although FilamentResourceManager needs it, hardcoded FILAMENT_MATC to /usr/bin/matc, and never linked AppKit, Metal or QuartzCore, which Open3D's own Objective-C++ sources need. fix-fmt-cxx20.patch isl-org/Open3D#7574 -- Filament's imported targets declare cxx_std_20, and compile features propagate as a floor, so Open3D's own sources move from C++17 to C++20. Two fmt calls only compile as C++17. Filament's licence is dropped from license_file since it is no longer bundled; it travels with the filament package instead. Open3D silently falls back to downloading Filament when the package is not found, which would bundle it without that licence, so build.sh now asserts that libOpen3D really has a libfilament shared-library dependency. A vendored Filament is static and leaves none. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017R4DS6whKCY8xGcA61kni2
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.
Type
Motivation and Context
Two fmt call sites compile today only because Open3D sets
CMAKE_CXX_STANDARD 17. They break as soon as anything raises the standard to C++20 — which happens without any change to Open3D itself, sinceINTERFACE_COMPILE_FEATURESis a floor that propagates: linking one dependency whose imported targets declarecxx_std_20raises every Open3D translation unit to C++20.I hit this building against a packaged Filament whose CMake config declares
cxx_std_20, but nothing here is specific to that; setting-DCMAKE_CXX_STANDARD=20reproduces both.1.
TensorMap::ToString()passes a runtimestd::stringas a format string. Under C++20 fmt's format-string checking becomesconsteval, so it is rejected:fmt::runtime()is fmt's documented way to opt a runtime format string out of compile-time checking. It is correct under both standards, so this is arguably the right call regardless.2.
MetricParameters::ToString()formats astd::vector<float>without includingfmt/ranges.h. It currently works only if that header arrives transitively:Both APIs exist in the bundled fmt — 9.1.0 on MSVC and 12.1.0 elsewhere — and
fmt::runtime()has been available since fmt 8, so this does not raise the fmt floor.Checklist:
python util/check_style.py --applyto apply Open3D code style to my code. — verified withclang-format18.1.8, matchingpython/requirements_style.txt.Description
Two lines changed plus one include. No behaviour change:
fmt::runtime()selects the runtime formatting path that C++17 already takes implicitly, and the added include only makes an existing dependency explicit.Test results
Built Open3D 0.20.0 on osx-arm64 with both fixes, in a configuration where a dependency forces C++20 (confirmed via CMake's generated
flags.make,-std=c++20). Both files compile and the build's packaging tests pass:Without these two changes the same build fails at
TensorMap.cppand thenVtkUtils.cpp(viaGeometry.h).Scope and limitations
Tested on osx-arm64 with clang only. I have not built this on MSVC, where the bundled fmt is 9.1.0 rather than 12.1.0 —
fmt::runtimeandfmt/ranges.hboth exist in 9.1.0, but I am relying on the API being present rather than having compiled it there.I found these while making
USE_SYSTEM_FILAMENTwork (#7573). The two PRs are independent and can be reviewed and merged in either order.🤖 This pull request was written by Claude Code (Claude Opus 5), acting on behalf of @Tobias-Fischer, who has reviewed it.
https://claude.ai/code/session_017R4DS6whKCY8xGcA61kni2