-
Notifications
You must be signed in to change notification settings - Fork 86
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
Inherited defines not being passed to cppcheck #84
Comments
Changing AnalyzeCppcheck.cs to:
should work when checking a single configuration. I'm not sure what you would do for multiple configurations but multiple configuration checking is fundamentally broken for cross platform code because you will at best get superficial checking and at worse get incorrect results because the platform type will be wrong. Each configuration may require a different platform type. Checking win64 code with a win32A platform type may give incorrect results. Checking linux code with a windows platform type makes no sense. You will get the platform type cppcheck was compiled with as the default if a platform type is not specified. This will most likely be wrong for code for a single windows platform and will definitely be wrong for multiple platform code. |
That's why I was reluctant to specify platforms in the first place. |
You can specify the correct platform type for a single configuration properly 100 % of the time. The current implementation only has a 1 in 3 chance of being right which means it will be wrong most of the time. The multi configuration will almost always be wrong no matter if you specify a platform or not so it's not fixable but that shouldn't stop you from fixing the single configuration. Single configuration checking and the correct platform type set should be the default so you get the best possible checking out of the box. Why would you want less checking and wrong results to be the default? If someone wants to enable multiple configuration checking, they should get a warning that the results could possibly be wrong if the code supports being compiled for both 32 and 64 bit or ASCII and UNICODE or non Windows desktop operating systems or if cppcheck was compiled for a different platform than the current project. This feature should be used with caution because it really doesn't do what you would think it should do. Multiple configuration checking should be done by checking each configuration separately. That way you get the best possible checking for each configuration. |
I do not intend to ever check multiple Visual Studio project configurations at the same time. Of course what you propose should be implemented. I do, however, often check all the |
It turns out that _UNICODE and UNICODE are inherited defines and are not being passed to cppcheck (see #92). These defines need to be seen before the win32 A/W detection will work automatically. |
The problem is I couldn't find how to query the inherited defines using VS SDK. |
Why not check only the current configuration and use all of its macros? I'm using wxWidgets GUI portability library and it fails badly without any macros defined. |
Why do you mean by "using all the macros"? |
Since you're running in the VS context and have access to the current configuration, you should be able to expand its PreprocessorDefinitions and generate -D arguments for cppcheck. Although you might need to synthesize the internal compiler ones. I don't know what the IDE exposes to an addon. |
I couldn't see any -D options in the generated command line. |
@SpareSimian, I think you haven't ticked this checkbox in the CppCheck add-in settings: I don't recall why it's off by default. Probably to facilitate checking all possible configurations - I write a lot of multi-platform code, no good if only the Windows branches are checked properly. |
That must be it. Only the last checkbox was checked. Recall that checking one file insisted on checking the whole project, but it wasn't including macros on the command line. It's still insisting on checking the whole project but now it looks like it's including a lot of preprocessor macros at the end of the command line, so it's a lot more successful. |
I've just double-checked: if you have "Check file after save" enabled and you hit Ctrl+S on a source file, only this one file is checked. |
We should be setting the platform type on the command line based on CPU type and UNICODE or _UNICODE being defined. We have no idea how cppcheck was compiled so not setting a platform type only give us a 1 in 3 chance of getting it right.
The text was updated successfully, but these errors were encountered: