Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Missing Include: ostream #60

Merged
merged 1 commit into from
Dec 6, 2024
Merged

Conversation

Daniel9z
Copy link
Contributor

@Daniel9z Daniel9z commented Dec 5, 2024

The ValKind::operator<< uses ostream, but the include was missing. ostream is used so commonly that it's usually included by some other header. However, if you use wasmtime.hh as your first (or only) include, then you get a massive compile error on Apple toolchain compilers (I'm using xcode 16). The error does not reproduce on Linux.

The error when this include is missing is:

include/wasmtime.hh:519:32: error: invalid operands to binary expression ('std::ostream' (aka 'basic_ostream<char>') and 'const char[4]')
  519 |     WASMTIME_FOR_EACH_VAL_KIND(CASE_KIND_PRINT_NAME)
include/wasmtime.hh:504:3: note: expanded from macro 'WASMTIME_FOR_EACH_VAL_KIND'
  504 |   X(I32, "i32", WASM_I32)                                                      \
      |   ^~~~~~~~~~~~~~~~~~~~~~~
include/wasmtime.hh:517:8: note: expanded from macro 'CASE_KIND_PRINT_NAME'
  517 |     os << name;                                                                \

@Daniel9z
Copy link
Contributor Author

Daniel9z commented Dec 5, 2024

To reproduce the issue, we could move the wasmtime.hh include in hello.cc to be the first one (before the iostream include). I could include that change if you'd like.

Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Thanks for this!

@alexcrichton
Copy link
Member

And yeah if you're up for it having this reproduce in CI I think would be good too

@Daniel9z Daniel9z force-pushed the main branch 3 times, most recently from 392af80 to e07dbc5 Compare December 6, 2024 00:21
@Daniel9z
Copy link
Contributor Author

Daniel9z commented Dec 6, 2024

Ok, I got CI to fail with the exact same error I saw locally when I pushed this (f4c369f) change. The change was simple, just include the wasmtime.hh header first:

Resulting CI Error:
https://github.com/bytecodealliance/wasmtime-cpp/actions/runs/12190299788/job/34007145848

I adjusted this pull request with the updated example and, if CI passes, this will confirm the issue is resolved.

…tream is used so commonly that it's usually included by some other header. However, if you use wasmtime.hh as your first (or only) include, then you get a massive compile error on Apple toolchain compilers (I'm using xcode 16). The error does not reproduce on Linux.

The error when this include is missing is:
```
include/wasmtime.hh:519:32: error: invalid operands to binary expression ('std::ostream' (aka 'basic_ostream<char>') and 'const char[4]')
  519 |     WASMTIME_FOR_EACH_VAL_KIND(CASE_KIND_PRINT_NAME)
third-party/wasmtime-cpp/include/wasmtime.hh:504:3: note: expanded from macro 'WASMTIME_FOR_EACH_VAL_KIND'
  504 |   X(I32, "i32", WASM_I32)                                                      \
      |   ^~~~~~~~~~~~~~~~~~~~~~~
third-party/wasmtime-cpp/include/wasmtime.hh:517:8: note: expanded from macro 'CASE_KIND_PRINT_NAME'
  517 |     os << name;                                                                \
```
@alexcrichton alexcrichton merged commit c0aa342 into bytecodealliance:main Dec 6, 2024
13 checks passed
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