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 false linter errors for os.h in Visual Studio/VSCode #2368

Closed
AidanSun05 opened this issue Jun 21, 2021 · 2 comments
Closed

Fix false linter errors for os.h in Visual Studio/VSCode #2368

AidanSun05 opened this issue Jun 21, 2021 · 2 comments

Comments

@AidanSun05
Copy link
Contributor

The Problem:
When using fmt::file and fmt::output_file with Visual Studio 2019 and Visual Studio Code on Windows, the linters in both yield the following errors because they can't find the required definitions:

image

However, the actual compile succeeds. This is because a macro (FMT_HAS_INCLUDE) is disabled only when IntelliSense is linting, not when MSVC is compiling (this workaround is from a VS 2015 IntelliSense bug: #842). But disabling this macro in newer versions of VS is no longer necessary and leads to the errors mentioned above.

The Proposed Fix:
Change the definition of FMT_HAS_INCLUDE to:

#if defined(__has_include) &&                                               \
    (!defined(__INTELLISENSE__) || (!FMT_MSC_VER || FMT_MSC_VER > 1900)) && \  // <== (CHANGED THIS)
    (!FMT_ICC_VERSION || FMT_ICC_VERSION >= 1600)
#  define FMT_HAS_INCLUDE(x) __has_include(x)
#else
#  define FMT_HAS_INCLUDE(x) 0
#endif

In addition to making a decision based on the absence/presence of IntelliSense, this revised code will also check if MSVC's version macro is above 1900 (the value corresponding to VS2015). This retains the original workaround while removing the false errors on newer VS versions.

Is this feasible and can this work in all cases? If so, will a PR be accepted? Thanks!

@vitaut
Copy link
Contributor

vitaut commented Jun 21, 2021

I can't tell if it will work in all cases but a PR is welcome and if it passes the CI it should be OK.

@AidanSun05
Copy link
Contributor Author

I have opened: #2370

@vitaut vitaut closed this as completed Jun 27, 2021
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