Skip to content

Allow anonymous namespaces #1031

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

Merged
merged 1 commit into from
Jul 17, 2017
Merged

Allow anonymous namespaces #1031

merged 1 commit into from
Jul 17, 2017

Conversation

pkesseli
Copy link
Contributor

Only give warnings for namespaces with actual names, in accordance with
ad41375.
This enables the use of anonymous namespaces without warnings.

@pkesseli
Copy link
Contributor Author

Rebased @peterschrammel. Linter fails because of the regression tests for the linter.

@reuk
Copy link
Contributor

reuk commented Jul 10, 2017

How does this interact with namespace aliases i.e. namespace xyz = my::nested::namespaces;? From glancing at the code, it looks like aliases will be treated the same as declarations. The regression test should include some aliases to document that the current behaviour is desired. I don't think the coding standard mentions aliases, so either behaviour (allowing or disallowing) is valid, although the choice should be documented somehow.

Copy link
Contributor

@reuk reuk left a comment

Choose a reason for hiding this comment

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

Please document the expected linter behaviour in the presence of namespace aliases.

@pkesseli
Copy link
Contributor Author

pkesseli commented Jul 11, 2017

This pull request doesn't change the script's behaviour with respect to namespace aliases. They were marked as violations before and they are marked as violations with this new implementation too. I added an additional case in the regression test to confirm this.

If we want to explicitly handle or document namespace aliases, I thus suggest to do it in a separate pull request.

@reuk
Copy link
Contributor

reuk commented Jul 12, 2017

OK, makes sense. Thanks for adding the extra test case.

@tautschnig
Copy link
Collaborator

@pkesseli Would you mind adding "Fixes: #932" to the commit message so that the merge will auto-close issue #932?

Only give warnings for namespaces with actual names, in accordance with
ad41375.
This enables the use of anonymous namespaces without warnings.

Fixes: #932
@pkesseli
Copy link
Contributor Author

Done.

@kroening kroening merged commit d80b10e into diffblue:master Jul 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants