Skip to content

Fix build with newer Apple clang#13157

Open
maskit wants to merge 1 commit into
apache:9.2.xfrom
maskit:fix_build_92x
Open

Fix build with newer Apple clang#13157
maskit wants to merge 1 commit into
apache:9.2.xfrom
maskit:fix_build_92x

Conversation

@maskit
Copy link
Copy Markdown
Member

@maskit maskit commented May 12, 2026

Four issues from newer Apple clang (version 21):

  1. TextView.h: -Wdeprecated-literal-operator flags the whitespace between "" and the suffix identifier in user-defined literal operator declarations. Remove the space in operator"" _tv and operator"" _sv.

  2. configure.ac: the version check added in PR Make 9.2.x buildable on macOS 26 #12834 only applied -Wno-cast-function-type-mismatch when Apple clang was exactly version 17, so the flag was silently skipped on 21+. Extend the check to match version >= 17.

  3. CryptoHash.h: memset/memcpy on non-trivially-copyable union type trip -Wnontrivial-memcall. Cast this to void *.

  4. HostStatus.cc: same issue with bzero on HostStatRec.

Four issues from newer Apple clang (version 21):

1. TextView.h: -Wdeprecated-literal-operator flags the whitespace
   between `""` and the suffix identifier in user-defined literal
   operator declarations. Remove the space in `operator"" _tv` and
   `operator"" _sv`.

2. configure.ac: the version check added in PR apache#12834 only applied
   -Wno-cast-function-type-mismatch when Apple clang was exactly
   version 17, so the flag was silently skipped on 21+. Extend the
   check to match version >= 17.

3. CryptoHash.h: memset/memcpy on non-trivially-copyable union type
   trip -Wnontrivial-memcall. Cast `this` to `void *`.

4. HostStatus.cc: same issue with bzero on HostStatRec.
@maskit maskit self-assigned this May 12, 2026
@maskit maskit requested review from bryancall and zwoop as code owners May 12, 2026 22:27
@maskit maskit added the Build work related to build configuration or environment label May 12, 2026
@maskit maskit requested a review from ezelkow1 May 12, 2026 22:27
@maskit
Copy link
Copy Markdown
Member Author

maskit commented May 13, 2026

[approve ci autest 2]


/// Default constructor - init to zero.
CryptoHash() { memset(this, 0, sizeof(*this)); }
CryptoHash() { memset(static_cast<void *>(this), 0, sizeof(*this)); }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Claude had an interesting thought on this, not for now, but for future. Maybe he's right, and we could simplify this "core" class.

  The warning symptom is that a user-defined default constructor and
  user-defined copy-assignment make the union non-trivially-copyable even
  though every member is a `uint*_t` array. A cleaner long-term fix
  (master only — not for 9.2.x) would be to initialize `u64{}` in-class
  and default `operator=`, making the union genuinely trivially copyable
  and retiring the memset/memcpy entirely. Filing a follow-up on master is
  the right venue for that — this PR is the correct minimum for the
  release branch.

@github-project-automation github-project-automation Bot moved this from In progress to Ready to Merge in 9.2.x Branch and Release May 13, 2026
@zwoop
Copy link
Copy Markdown
Contributor

zwoop commented May 13, 2026

One thought / comment (not sure what we did for master), and likely for @bneradt :

Does the changes to TextView also need to go into the libswoc code repository ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Build work related to build configuration or environment

Projects

Status: Ready to Merge

Development

Successfully merging this pull request may close these issues.

2 participants