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

Warning c6319 in Visual Studio #359

Closed
Cvelth opened this issue Apr 23, 2020 · 9 comments
Closed

Warning c6319 in Visual Studio #359

Cvelth opened this issue Apr 23, 2020 · 9 comments

Comments

@Cvelth
Copy link
Contributor

Cvelth commented Apr 23, 2020

Description

Visual Studio marks asserts with warning c6319.

Steps to reproduce

  • $ git clone https://github.com/Cvelth/cpp_starter_fork
  • $ cmake.exe -Htest -Bbuild -G "Visual Studio 16 2019"
  • open build/GreeterTests.sln
  • navigate to GreeterTests/Source Files/greeter.cpp
    image
    image

Extra information

  • doctest version: v2.3.7
  • Operating System: Windows 10 (1909, build 18363.778)
  • Compiler+version: Microsoft Visual Studio 2019 (16.5.1) with MSVC 19.25.28611.0
@onqtam
Copy link
Member

onqtam commented Apr 24, 2020

Seems that there's no easy way to make all the static code analysis tools happy...

I used the do { ... } while((void)0, 0) trick to avoid warnings because do { ... } while(false) was a problem with some compilers a few years back (conditional expression is constant)... But it seems that the latest MSVC complains about this. I'll try again with do { ... } while(false).

@Cvelth
Copy link
Contributor Author

Cvelth commented Apr 26, 2020

Can confirm that do { ... } while(false) resolves the issue and works great. Not sure about other compilers, though.

@onqtam
Copy link
Member

onqtam commented Apr 26, 2020

I'll probably do a push with such a change in a test branch to see what the CI reports (if all supported compilers are fine with that) but I can't promise that will happen anytime soon.

@Cvelth
Copy link
Contributor Author

Cvelth commented Apr 26, 2020

I doesn't seem to cause any problems (proof).
The only failing build seems to be a compiler issue (a typo in a header file), which should be fixed with the release of visual studio 16.6 (msvc 14.26), as it's already fixed in 16.6 preview 2. Proofs: 1, 2.

EDIT: <concepts> header is implicitly included by <utility>.
EDIT_2: Exact issue: (link)
image

@onqtam
Copy link
Member

onqtam commented Apr 26, 2020

Well ok, that sounds reasonable. You can open a PR from your branch and we'll see if all the builds pass (including travis & appveyor - besides just github actions)

@Cvelth
Copy link
Contributor Author

Cvelth commented Apr 26, 2020

Travis CI and AppVeyor both pass.
image

I was hesitant to open PR as it does fail on a single build-system.

@onqtam
Copy link
Member

onqtam commented Apr 26, 2020

I don't see how your changes could be triggering that failure - I also just looked at the previous github actions builds of doctest and seems that this failure is already present: https://github.com/onqtam/doctest/actions

So a PR would be just fine - just make sure to open it against the dev branch - seems that you made the changes on top of master.

@Cvelth
Copy link
Contributor Author

Cvelth commented Apr 26, 2020

Should I PR changes after the target assemble_single_header is run, or just doctest_fwd.h changes?

@onqtam
Copy link
Member

onqtam commented Apr 26, 2020

preferably after that target has run - I like to keep the resulting header in sync, but it's not a deal-breaker.

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

No branches or pull requests

2 participants