Skip to content

Remove unnecessary, WASM-specific code and configurations#173

Open
tzimnoch wants to merge 1 commit into
dds-bridge:developfrom
tzimnoch:optimize_wasm_pipeline
Open

Remove unnecessary, WASM-specific code and configurations#173
tzimnoch wants to merge 1 commit into
dds-bridge:developfrom
tzimnoch:optimize_wasm_pipeline

Conversation

@tzimnoch

@tzimnoch tzimnoch commented Jun 7, 2026

Copy link
Copy Markdown

See #168 for further details.

The (SetMaxThreads) call is no longer necessary in 3.0.0 and can be removed from all examples, tests, etc.

I only removed it for __linux__ and  __WASM__ flags as I'm not able to test on __APPLE__, but I suspect this call can be removed for all platforms.

Comment on lines -65 to -70
#ifdef __WASM__
// For WASM, we can't use ErrorMessage, so we'll just print the error code
snprintf(line, sizeof(line), "error code %d", res);
#else
ErrorMessage(res, line);
#endif

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I don't know what verification proved this was initially necessary, but I haven't found it to be true. To verify I added

    ErrorMessage(RETURN_TOO_MANY_CARDS, line);
    printf("DDS error: %s\n", line);

outside this if block so that it would always execute and then ran node bazel-bin/examples/wasm/AnalysePlayBin.js

DDS error: Too many cards
AnalysePlayBin, hand 1: OK

WASM appears to have no problem calling this function to copy the text for error message res to line.

Copilot AI left a comment

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.

Pull request overview

This PR removes WASM-specific configuration and conditional code paths, primarily by eliminating the custom __WASM__ define from the Bazel build and trimming platform-guarded initialization/workarounds in examples and docs.

Changes:

  • Dropped the Bazel __WASM__ define for //:build_wasm and removed related WASM-specific notes from the WASM build documentation.
  • Narrowed SetMaxThreads(0) calls in several examples/tests to only run on __APPLE__.
  • Removed a WASM-only error handling branch in examples/analyse_play_bin.cpp, always using ErrorMessage().

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
examples/wasm/calc_dd_table_pbn_test.cpp Restricts SetMaxThreads(0) call to __APPLE__ in the WASM example test.
examples/solve_board.cpp Restricts SetMaxThreads(0) call to __APPLE__ in the SolveBoard example.
examples/solve_board_pbn.cpp Restricts SetMaxThreads(0) call to __APPLE__ in the SolveBoardPBN example.
examples/calc_dd_table_pbn.cpp Restricts SetMaxThreads(0) call to __APPLE__ in the CalcDDtablePBN example.
examples/analyse_play_bin.cpp Restricts SetMaxThreads(0) call to __APPLE__ and removes WASM-only error formatting fallback.
docs/wasm_build.md Removes development notes describing the repo-defined __WASM__ macro and WASM-specific stubs.
CPPVARIABLES.bzl Removes __WASM__ from DDS_LOCAL_DEFINES for the WASM build select() branch.

Comment thread examples/solve_board.cpp
Comment on lines +36 to 38
#if defined(__APPLE__)
SetMaxThreads(0);
#endif
Comment on lines +35 to 37
#if defined(__APPLE__)
SetMaxThreads(0);
#endif
Comment on lines +30 to 32
#if defined(__APPLE__)
SetMaxThreads(0);
#endif
Comment on lines +33 to 35
#if defined(__APPLE__)
SetMaxThreads(0);
#endif
Comment on lines +16 to 18
#if defined(__APPLE__)
SetMaxThreads(0);
#endif
Comment thread docs/wasm_build.md
Comment on lines 137 to 141
## Development notes

- The `__WASM__` preprocessor constant is defined for WASM builds (`CPPVARIABLES.bzl`). It was added to work around platform-specific code paths; revisit whether it can be narrowed or removed as WASM support matures.
- Some threading and platform-specific features are disabled or stubbed when `__WASM__` is set.
- A reusable `cc_library` WASM artifact (not only example binaries) is not yet provided; today only `wasm_cc_binary` example targets are wired up.
- The browser MVP lives under `web/`; see **Web browser (DDS MVP)** above and `//web:web_system_tests`.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants