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

inferring system includes #109

Closed
esquires opened this issue Aug 19, 2017 · 4 comments
Closed

inferring system includes #109

esquires opened this issue Aug 19, 2017 · 4 comments

Comments

@esquires
Copy link
Contributor

For projects that use #include <someInclude.h> rather than #include "someInclude.h" for non-system includes, cppclean will print

filename:line: 'Func' not found in any directly #included header

The logic is found in ast.py here. (If there is < then it is a system include). It looks like whether a include is system or not is used in find_warnings.py here. Removing this logic (making all includes non-system includes) removes the false positive described above but introduces a new false positive:

filename:line: unable to find 'cmath'

This may be obvious but can you help me understand why system includes aren't being checked in the same way?

Assuming system includes do need to be differentiated, a solution to the above problem is to have users distinguish system includes on the cppclean call (e.g., using -I for system includes and -i for non-system includes.). It would then be a simple check in ast.py for whether a given include is in one of those sets of paths. If it is in neither, system could be assumed.

I am happy to submit a pull request if desired.

A practical example can be found from cloning SCRIMMAGE here and running the following:

cppclean -I src/proto_conversions/ProtoConversions.cpp
@myint
Copy link
Owner

myint commented Aug 20, 2017

Perhaps it doesn't check the system includes for simplicity. It avoids requiring the user to specify the system include paths.

I wonder if the proposed solution would break backward compatibility.

@esquires
Copy link
Contributor Author

It would. What do you think about keeping the existing functionality as is but let users use the new logic by specifying -Is for system includes and -In for non-system includes. The logic would be

  • if the file is found in the includes specified by -Is (-In) then it is a system include (not a system include)
  • all other cases use the old logic

@myint
Copy link
Owner

myint commented Aug 25, 2017

@esquires I think that sounds reasonable.

@myint
Copy link
Owner

myint commented Sep 9, 2017

This is closed by #110.

@myint myint closed this as completed Sep 9, 2017
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