-
Notifications
You must be signed in to change notification settings - Fork 82
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: always use absolute (and simplified) header path whenever possible #362
base: master
Are you sure you want to change the base?
Conversation
This fix an issue of importing the same header by different addresses. The issue is trickier than seems, because if a user used the include dir flag `-I...` with an absolute path, then every matched header was detected to be absolute, but if the import was a relative import to the source file and the source file isn't with an absolute path, than the header is identified by the combined relative path. See https://sourceforge.net/p/cppcheck/discussion/general/thread/c01f80556b/
This change might be problematic because it is obviously requires a test but so far there are no tests at all utilizing includes (see #261 for a related discussion). This should probably be implemented by introducing Python tests similar to what Cppcheck is doing. @danmar would that be acceptable for tests which actually require (multiple) files? |
What about these (failed) tests? https://ci.appveyor.com/project/danmar/simplecpp/builds/50353854 |
sure, it would definitely make sense to include actual files in some python tests but as @Tal500 pointed out there are some tests for the handling at least.
yes imho those are valuable also. |
@danmar if I guess correctly, the failure is due to that the header files are not really in the working directory or so, right? |
yes that sounds probable to me. I don't know how your changes will affect the output from simplecpp if all filepaths will always be absolute that is also unfortunate. Is the output changed? |
Yes, except for the system headers that were not found in the scan paths, e.g. STL headers (that simplecpp doesn't open them at all and it's not consider to be an error). We could have a more complicated solution, that uses two header attributes:
The first attribute is mandatory, and the second one is optional and exists only when the header is found. An alternative solution is to have a user configuration that toggles the include path absoluteness. |
|
I am very sorry for the delay. I was at cppcon last week and had a presentation. As far as I understand the solution you suggest makes sense to me. We don't need to have these properties for each token right. |
@Tal500 just making sure you see my comment. |
I'd like too see it when it will be published!
I'll look into that more |
👍 for your information here is a short description: I guess it will take a while until it is published. |
This fix an issue of importing the same header by different addresses.
The issue is trickier than seems, because if a user used the include dir flag
-I...
with an absolute path, then every matched header was detected to be absolute, but if the import was a relative import to the source file and the source file isn't with an absolute path, than the header is identified by the combined relative path.See https://sourceforge.net/p/cppcheck/discussion/general/thread/c01f80556b/
Note - I tried to make the code compatible with Windows, but was tested only on linux.