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

Update catch_suppress_warnings.h #1508

Closed
wants to merge 3 commits into from
Closed

Update catch_suppress_warnings.h #1508

wants to merge 3 commits into from

Conversation

YarikTH
Copy link
Contributor

@YarikTH YarikTH commented Jan 24, 2019

fixed "pragma GCC diagnostic push" after "pragma GCC diagnostic ignored"

Description

I found unusual gcc warnings pattern in catch_suppress_warnings.h that obviosly a bug added by someone who doesn't familar with diagnostic push-ignored-pop pattern.

#    pragma GCC diagnostic ignored "-Wparentheses"
#    pragma GCC diagnostic push
#    pragma GCC diagnostic ignored "-Wunused-variable"
#    pragma GCC diagnostic ignored "-Wpadded"

So I just moved "push" before first "ignored"

fixed "pragma GCC diagnostic push" after "pragma GCC diagnostic ignored"
@codecov
Copy link

codecov bot commented Jan 24, 2019

Codecov Report

Merging #1508 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1508   +/-   ##
=======================================
  Coverage   80.53%   80.53%           
=======================================
  Files         121      121           
  Lines        3436     3436           
=======================================
  Hits         2767     2767           
  Misses        669      669

1 similar comment
@codecov
Copy link

codecov bot commented Jan 24, 2019

Codecov Report

Merging #1508 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1508   +/-   ##
=======================================
  Coverage   80.53%   80.53%           
=======================================
  Files         121      121           
  Lines        3436     3436           
=======================================
  Hits         2767     2767           
  Misses        669      669

@YarikTH
Copy link
Contributor Author

YarikTH commented Jan 24, 2019

Locally suppress hidden warnings on Class and Compile tests that appeared on gcc-4.8 gcc-4.9 and gcc-5.0

@YarikTH
Copy link
Contributor Author

YarikTH commented Jan 24, 2019

I found #674 and understand comment above suppression

// GCC likes to warn on REQUIREs, and we cannot suppress them
// locally because g++'s support for _Pragma is lacking in older,
// still supported, versions
#    pragma GCC diagnostic ignored "-Wparentheses"
#    pragma GCC diagnostic push

So I close request. But it would be better to mark this more obviously. Because now it's unclear from comment before. e.g.:

// GCC likes to warn on REQUIREs, and we cannot suppress them
// locally because g++'s support for _Pragma is lacking in older,
// still supported, versions
#    pragma GCC diagnostic ignored "-Wparentheses" //< Don't move under push/pop. See #674
#    pragma GCC diagnostic push

or

// GCC likes to warn on REQUIREs, and we cannot suppress them
// locally because g++'s support for _Pragma is lacking in older,
// still supported, versions
// [
#    pragma GCC diagnostic ignored "-Wparentheses"
// ]
#    pragma GCC diagnostic push

@YarikTH YarikTH closed this Jan 24, 2019
horenmar added a commit that referenced this pull request Jan 25, 2019
@horenmar
Copy link
Member

Ok, I modified the comment somewhat to be clearer.

@YarikTH YarikTH deleted the patch-1 branch January 25, 2019 21:45
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.

2 participants