Skip to content

fix(nimble): Make the shared dictionary export build and pass checks - #18642

Open
kgpai wants to merge 1 commit into
mainfrom
nimble-shared-dictionary-build-fixes
Open

fix(nimble): Make the shared dictionary export build and pass checks#18642
kgpai wants to merge 1 commit into
mainfrom
nimble-shared-dictionary-build-fixes

Conversation

@kgpai

@kgpai kgpai commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

#18640 exported the shared dictionary work byte-identical to fbcode, which
leaves this tree not compiling and failing pre-commit. This fixes that.

  • Encoding::Options::sharedDictionaryAlphabet and
    SharedDictionaryConfig::alphabetEncodings had no default initializer, so
    every brace-initialization of those structs failed under -Wextra -Werror.
    201 errors across six test files.
  • ExternalSharedDictionaryBuilder::resetImpl passed a Kind enum to
    NIMBLE_UNSUPPORTED; fmt v11 will not format an enum. Uses the existing
    kindString() instead.
  • Four headers had no CMake entry, breaking check-header-ownership. Each is
    now listed beside the .cpp already in its target.

Kept separate from #18640 so the export stays a clean mirror of fbcode.

See #18643 for why this keeps happening and how to stop it at source.

@kgpai
kgpai requested a review from majetideepak as a code owner August 23, 2026 22:14
@netlify

netlify Bot commented Aug 23, 2026

Copy link
Copy Markdown

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 1fd78f8
🔍 Latest deploy log https://app.netlify.com/projects/meta-velox/deploys/6a8bc2683e598c00085167ae

@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 Aug 23, 2026
@github-actions

Copy link
Copy Markdown

Selective Build Plan

Linux release with adapters is running a full build (PR has a standing approval). See the CI workflows README for what this means.


Selective build plan

@kgpai
kgpai force-pushed the nimble-shared-dictionary-build-fixes branch from 56fedd4 to 34583da Compare August 24, 2026 03:14
@meta-codesync

meta-codesync Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

@kgpai has imported this pull request. If you are a Meta employee, you can view this in D117152048.

#18640 exported the shared dictionary work byte-identical to fbcode, which
leaves this tree not compiling and failing pre-commit. This fixes that.

Four struct members had no default initializer, so every brace-initialization
of those structs failed under -Wextra -Werror. fbcode does not promote
-Wmissing-field-initializers, so the omissions are invisible internally:

- Encoding::Options::sharedDictionaryAlphabet, 196 errors
- SharedDictionaryConfig::alphabetEncodings, 5 errors
- ExternalDictionaryBuilder::Options::readFactors, 4 errors
- ExternalDictionaryBuilder::Options::alphabetEncoding, 1 error

ExternalSharedDictionaryBuilder::resetImpl passed a Kind enum to
NIMBLE_UNSUPPORTED, and fmt v11 will not format an enum implicitly. Uses the
existing kindString() instead, which also turns the message from an integer
into "External shared dictionary builder does not support reset()".

Four headers had no CMake entry, breaking check-header-ownership. Each is now
listed beside the .cpp already in its target.

tools/tests/CMakeLists.txt is reformatted by gersemi. It landed unformatted
with #18634, and pre-commit runs over all files, so it fails on main too.

See #18643 for why this keeps recurring and how to stop it at source.
@kgpai
kgpai force-pushed the nimble-shared-dictionary-build-fixes branch from 34583da to 1fd78f8 Compare August 24, 2026 04:02
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants