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

Build with MSVC /W4 #1432

Closed
wants to merge 2 commits into from
Closed

Build with MSVC /W4 #1432

wants to merge 2 commits into from

Conversation

NickGerleman
Copy link
Contributor

@NickGerleman NickGerleman commented Oct 18, 2023

The reference Clang/GCC build has a pretty strict set of warnings enabled. The reference MSVC build has less strict warnings, which can be a problem for MSVC users building at higher warning levels (e.g. React Native for Windows in OSS uses /W4 as its baseline warning level).

This bumps up the MSVC warning level to /W4, since we are nearly clean already.

There are some limitations. E.g. we don't test binary with MSVC (some issues I didn't work out), and only test building statically linked. But but we do have a minimal C benchmark we compile with MSVC.

The reference Clang/GCC build has a pretty strict set of warnings enabled. The reference MSVC build has less strict warnings, which can be a problem for MSVC users building at higher warning levels.

Let’s see how clean this is, and impact to devx (mismatched from local warnings) for developers mostly using Clang.
@NickGerleman NickGerleman changed the title Try building with /W4 Try building with MSVC /W4 Oct 18, 2023
@NickGerleman NickGerleman changed the title Try building with MSVC /W4 Build with MSVC /W4 Oct 18, 2023
@facebook-github-bot
Copy link
Contributor

@NickGerleman has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Oct 18, 2023
Summary:
The reference Clang/GCC build has a pretty strict set of warnings enabled. The reference MSVC build has less strict warnings, which can be a problem for MSVC users building at higher warning levels (e.g. React Native for Windows in OSS uses `/W4` as its baseline warning level).

This bumps up the MSVC warning level to `/W4`, since we are nearly clean already.

There are some limitations. E.g. we don't test binary with MSVC (some issues I didn't work out), and only test building statically linked. But but we do have a minimal C benchmark we compile with MSVC.

X-link: facebook/yoga#1432

Differential Revision: D50398443

Pulled By: NickGerleman
NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Oct 18, 2023
Summary:
Pull Request resolved: facebook#41044

The reference Clang/GCC build has a pretty strict set of warnings enabled. The reference MSVC build has less strict warnings, which can be a problem for MSVC users building at higher warning levels (e.g. React Native for Windows in OSS uses `/W4` as its baseline warning level).

This bumps up the MSVC warning level to `/W4`, since we are nearly clean already.

There are some limitations. E.g. we don't test binary with MSVC (some issues I didn't work out), and only test building statically linked. But but we do have a minimal C benchmark we compile with MSVC.

X-link: facebook/yoga#1432

Test Plan: GitHub Actions running benchmark MSVC build.

Reviewed By: yungsters

Differential Revision: D50398443

Pulled By: NickGerleman

fbshipit-source-id: 87a9f521830adea8964aeae2028d05ec50b87d76
facebook-github-bot pushed a commit to facebook/litho that referenced this pull request Oct 19, 2023
Summary:
X-link: facebook/react-native#41044

The reference Clang/GCC build has a pretty strict set of warnings enabled. The reference MSVC build has less strict warnings, which can be a problem for MSVC users building at higher warning levels (e.g. React Native for Windows in OSS uses `/W4` as its baseline warning level).

This bumps up the MSVC warning level to `/W4`, since we are nearly clean already.

There are some limitations. E.g. we don't test binary with MSVC (some issues I didn't work out), and only test building statically linked. But but we do have a minimal C benchmark we compile with MSVC.

X-link: facebook/yoga#1432

Reviewed By: yungsters

Differential Revision: D50398443

Pulled By: NickGerleman

fbshipit-source-id: 6616034d79b1a308b32d5d3387bae70f40b7b5ab
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Oct 19, 2023
Summary:
Pull Request resolved: #41044

The reference Clang/GCC build has a pretty strict set of warnings enabled. The reference MSVC build has less strict warnings, which can be a problem for MSVC users building at higher warning levels (e.g. React Native for Windows in OSS uses `/W4` as its baseline warning level).

This bumps up the MSVC warning level to `/W4`, since we are nearly clean already.

There are some limitations. E.g. we don't test binary with MSVC (some issues I didn't work out), and only test building statically linked. But but we do have a minimal C benchmark we compile with MSVC.

X-link: facebook/yoga#1432

Test Plan: GitHub Actions running benchmark MSVC build.

Reviewed By: yungsters

Differential Revision: D50398443

Pulled By: NickGerleman

fbshipit-source-id: 6616034d79b1a308b32d5d3387bae70f40b7b5ab
@facebook-github-bot
Copy link
Contributor

@NickGerleman merged this pull request in f1f869b.

Othinn pushed a commit to Othinn/react-native that referenced this pull request Oct 30, 2023
Summary:
Pull Request resolved: facebook#41044

The reference Clang/GCC build has a pretty strict set of warnings enabled. The reference MSVC build has less strict warnings, which can be a problem for MSVC users building at higher warning levels (e.g. React Native for Windows in OSS uses `/W4` as its baseline warning level).

This bumps up the MSVC warning level to `/W4`, since we are nearly clean already.

There are some limitations. E.g. we don't test binary with MSVC (some issues I didn't work out), and only test building statically linked. But but we do have a minimal C benchmark we compile with MSVC.

X-link: facebook/yoga#1432

Test Plan: GitHub Actions running benchmark MSVC build.

Reviewed By: yungsters

Differential Revision: D50398443

Pulled By: NickGerleman

fbshipit-source-id: 6616034d79b1a308b32d5d3387bae70f40b7b5ab
@NickGerleman NickGerleman deleted the NickGerleman-w4 branch November 1, 2023 04:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants