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

Remove MSVC shim header cruft, reduce CMake overhead #4104

Closed

Conversation

marcusmueller
Copy link
Member

@marcusmueller marcusmueller commented Jan 16, 2021

closes #1680, closes #1674

Copy link
Contributor

@ryanvolz ryanvolz left a comment

Choose a reason for hiding this comment

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

This looks good to my eye, with the caveat that additionally you'll need to remove add_definitions(-DHAVE_CONFIG_H) from CMakeLists.txt to get it compiling! A conda build using these changes compiles successfully with MSVC (and on Linux and macOS).

Signed-off-by: Marcus Müller <mmueller@gnuradio.org>
Signed-off-by: Marcus Müller <mmueller@gnuradio.org>
@marcusmueller
Copy link
Member Author

I've removed the CMake Check cleanup, which is a separate issue (and now in #4256), and added in -DHAVE_CONFIG_H

@mbr0wn
Copy link
Member

mbr0wn commented Feb 18, 2021

Introduces new warnings:

/__w/gnuradio/gnuradio/gr-blocks/lib/udp_sink_impl.cc: In member function 'virtual int gr::blocks::udp_sink_impl::work(int, gr_vector_const_void_star&, gr_vector_void_star&)':
/__w/gnuradio/gnuradio/gr-blocks/lib/udp_sink_impl.cc:110:23: error: comparison of integer expressions of different signedness: 'std::ptrdiff_t' {aka 'long int'} and 'const size_t' {aka 'const long unsigned int'} [-Werror=sign-compare]
  110 |     while (bytes_sent < total_size) {
      |            ~~~~~~~~~~~^~~~~~~~~~~~
/__w/gnuradio/gnuradio/gr-blocks/lib/udp_sink_impl.cc:111:23: error: operation on 'bytes_to_send' may be undefined [-Werror=sequence-point]
  111 |         bytes_to_send = bytes_to_send =
      |         ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
  112 |             std::min(static_cast<std::ptrdiff_t>(d_payload_size),
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  113 |                      static_cast<std::ptrdiff_t>(total_size - bytes_sent));
      |                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[ 36%] Building CXX object gr-zeromq/python/zeromq/bindings/CMakeFiles/zeromq_python.dir/pull_msg_source_python.cc.o
cc1plus: all warnings being treated as errors

Copy link
Member

@mbr0wn mbr0wn left a comment

Choose a reason for hiding this comment

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

Can you please fix the compiler warnings -- we no do -Werror where applicable.

Signed-off-by: Josh Morman <jmorman@perspectalabs.com>
@mbr0wn
Copy link
Member

mbr0wn commented Feb 22, 2021

Maybe merge the fixup UDP commit with the first one?

@mbr0wn
Copy link
Member

mbr0wn commented Feb 23, 2021

Merged manually + squashed.

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.

msvc: remove cmake/msvc/config.h CMake: Be sure we still need all checks we do in 3.8
4 participants