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

Fix MinGW vsnprintf compile errors and warnings #205

Closed
wants to merge 3 commits into
base: develop
from

Conversation

Projects
None yet
2 participants
@Lastique
Copy link
Member

Lastique commented Feb 10, 2019

This PR:

  • Adds Appveyor CI config file.
  • Fixes compile errors on MinGW about missing declaration of vsnprintf. The attempted fix in #195, although correct, did not fix the error.
  • Silences gcc warnings about unused global variables.

Lastique added some commits Feb 10, 2019

Use legacy _vsnprintf on MinGW.
Apparently, some versions of legacy MinGW headers don't provide
the standard vsnprintf function, as Appveyor CI fails for this target.
Thus we use the non-standard MSVC-specific _vsnprintf instead.
Added unused variable markup to silence gcc warnings.
The singleton reference variables and a few other globals may not be actually
used in the user's code, which makes gcc emit warnings. This commit marks these
variables as potentially unused, which silences the warnings.

@Lastique Lastique force-pushed the Lastique:topic/fix-mingw-vsnprintf branch from 3dfc5dc to 2d4c263 Feb 10, 2019

@raffienficiaud

This comment has been minimized.

Copy link
Member

raffienficiaud commented Feb 10, 2019

There is already a PR about AppVeyor, see #185 . For some reason it is not active yet. It would be good if you can split the PR on those orthogonal issues. Thanks!

@Lastique

This comment has been minimized.

Copy link
Member Author

Lastique commented Feb 10, 2019

That PR sets up MinGW differently than my config, I'm not sure it is equivalent. Also, it doesn't enable CI for topic branches. I've included the config because I had to ensure the problem reproduces and test the change in Appveyor since I couldn't reproduce it locally.

The config is in a separate commit, you can skip merging it. If you want, I can remove this commit from this PR. Please, confirm.

For some reason it is not active yet. It would be good if you can split the PR on those orthogonal issues.

You have to add the GitHub project to your Appveyor project list in order to get it running. You should have admin rights for the GitHub project though.

@raffienficiaud

This comment has been minimized.

Copy link
Member

raffienficiaud commented Feb 12, 2019

I split the PR in 3 branches:

Concerning the vsnprintf issue (that is the purpose of the PR), it is currently running the tests in the branch topic/PR-205-Fix-MinGW-vsnprintf-compile-errors-warnings. I'll merge that once the tests have cycled.

The GCC warnings is on branch topic/silence-gcc-warnings and the AppVeyor one is in the branch topic/CI-appveyor. I need to merge the Appveyor work with the other running PR on CI. Concerning the gcc warning, I am not sure: I can see some of the warnings, but all those symbols are used at some point. Since I do not know what BOOST_ATTRIBUTE_UNUSED does and since this is just about warnings (lesser priority), I suggest we wait a bit. The other option for silencing the warnings would be to add the proper pragmas in the suppress_warnings.hpp file.

@Lastique

This comment has been minimized.

Copy link
Member Author

Lastique commented Feb 12, 2019

Since I do not know what BOOST_ATTRIBUTE_UNUSED does...

This is __attribute__((unused)) for gcc, which explicitly indicates that the variable may appear unused in the program. Disabling the warning with a pragma may work, but it may also disable the warning for genuinely leftover unused variables, so I prefer the attribute.

@raffienficiaud

This comment has been minimized.

Copy link
Member

raffienficiaud commented Feb 14, 2019

Changes merged to next except for the AppVeyor (tried to enable the thing from Appveyor but still waiting for some approval by boost.org).

Closing, thanks!

@Lastique Lastique deleted the Lastique:topic/fix-mingw-vsnprintf branch Feb 14, 2019

@raffienficiaud raffienficiaud added develop and removed develop labels Feb 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.