Skip to content

Localized suppression of compiler warnings and minor fixes#110

Open
jbcoe wants to merge 1 commit intoNetHack-LE:mainfrom
jbcoe:jbcoe/compile-warning-suppression
Open

Localized suppression of compiler warnings and minor fixes#110
jbcoe wants to merge 1 commit intoNetHack-LE:mainfrom
jbcoe:jbcoe/compile-warning-suppression

Conversation

@jbcoe
Copy link
Copy Markdown
Collaborator

@jbcoe jbcoe commented Mar 23, 2026

  • Suppress target-specific compiler warnings for non-MSVC builds

  • Fix format specifier in error message

@jbcoe jbcoe marked this pull request as draft March 23, 2026 16:34
@jbcoe jbcoe marked this pull request as ready for review March 24, 2026 08:53
@jbcoe jbcoe requested a review from StephenOman March 24, 2026 08:53
@StephenOman
Copy link
Copy Markdown
Collaborator

What is the purpose of limiting the noisy warning suppression to non-MVSC?

@jbcoe
Copy link
Copy Markdown
Collaborator Author

jbcoe commented Mar 24, 2026

I don't think that the suppressions I have specified will work on MSVC.

We could consider

target_compile_options(makedefs PRIVATE 
    $<$<CXX_COMPILER_ID:MSVC>:/wd4996>
    $<$<NOT:$<CXX_COMPILER_ID:MSVC>>:-Wno-deprecated-non-prototype>
)

(or equivalent) as I think that MSVC will likely treat -Wno-deprecated-non-prototype as an unknown command-line option and trigger an error (or a warning about an invalid argument).

I don't have access to MSVC to be able to check.

@StephenOman
Copy link
Copy Markdown
Collaborator

I have a Windows machine here so I can see what happens with the current setup.

@StephenOman
Copy link
Copy Markdown
Collaborator

Pretty much what you expected:

cl : command line  error D8021: invalid numeric argument '/Wno-deprecated-non-prototype'

There are other issues too though - tmt.c (in third_party/libtmt) has variable length arrays and compiling it causes:

 error C2057: expected constant expression

These aren't supported in MVSC, so I'm not sure that NLE ever worked on Windows.

@jbcoe jbcoe changed the title Fixes and constrained suppression of compiler warnings Localized suppression of compiler warnings and minor fixes Mar 31, 2026
@jbcoe
Copy link
Copy Markdown
Collaborator Author

jbcoe commented Mar 31, 2026

Presumably one could build a more modern C-compiler for Windows and get Nethack working?

We should avoid checking in code that we know won't work on Windows. Fixing the Windows build is probably beyond the scope of this PR.

@StephenOman
Copy link
Copy Markdown
Collaborator

According to the original team, NLE never worked under Windows. However, NetHack itself can be built and run.

I agree with your suggestion about not making it worse. These warning suppressions were added only because they were annoying me when I was adding new C code, so perhaps this PR should remove them completely for now to align with not adding more Windows-breaking code/configuration. They don't appear if the build is successful.

We already have an open issue with Windows Install (#34) and fixing it sounds like a bit more work.

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