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

Issue 89: warning on using namespace in header #90

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

WimLeflere
Copy link

Added a warning when 'using namespace' is used in a header file.
The rest of the file is ignored because the output could be incorrect.

Wim Leflere added 3 commits February 17, 2016 13:59
included_files, forward_declarations = self._read_and_parse_includes()
self._find_unused_warnings(included_files, forward_declarations)
self._find_using_namespaces()
if(len(self.warnings) == 0):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like it would be better if self._find_using_namespaces() returned True if it finds a problem. That way you could do something like:

if not self._find_using_namespaces():
    included_files, forward_declarations = self._read_and_parse_includes()
    self._find_unused_warnings(included_files, forward_declarations)

The above doesn't rely on the self.warnings, which might be modified by other methods (at least in future implementations).

@myint
Copy link
Owner

myint commented Feb 18, 2016

@r-e-d, probably has a better idea than me on whether this is a good approach.

@r-e-d
Copy link
Collaborator

r-e-d commented Feb 18, 2016

This new warning is fine for me. But I'm not sure that we really want to abort parsing the file.

One solution would be to mark the file as used if it contains any using statement to prevent incorrect header unused warning.

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

Successfully merging this pull request may close these issues.

3 participants