Skip to content

COMP: Supress warning in MSVC for non-float complex#3897

Merged
blowekamp merged 1 commit intoInsightSoftwareConsortium:masterfrom
blowekamp:silence_nonfloating_complex
Feb 3, 2023
Merged

COMP: Supress warning in MSVC for non-float complex#3897
blowekamp merged 1 commit intoInsightSoftwareConsortium:masterfrom
blowekamp:silence_nonfloating_complex

Conversation

@blowekamp
Copy link
Copy Markdown
Member

@blowekamp blowekamp commented Jan 31, 2023

PR Checklist

  • No API changes were made (or the changes have been approved)
  • No major design changes were made (or the changes have been approved)
  • Added test (or behavior not changed)
  • Updated API documentation (or API not changed)
  • Added license to new files (if any)
  • Added Python wrapping to new files (if any) as described in ITK Software Guide Section 9.5
  • Added ITK examples for all new major features (if any)

Refer to the ITK Software Guide for
further development details if necessary.

@blowekamp blowekamp requested a review from dzenanz January 31, 2023 18:34
@github-actions github-actions bot added area:Core Issues affecting the Core module type:Compiler Compiler support or related warnings labels Jan 31, 2023
@N-Dekker
Copy link
Copy Markdown
Contributor

N-Dekker commented Jan 31, 2023

Thanks @blowekamp Can you please add this _SILENCE_NONFLOATING_COMPLEX_DEPRECATION_WARNING to the other cxx files where the warnings appear as well? At least, I think Modules\Core\Common\test\itkNumericTraitsTest.cxx and Modules\Core\Common\test\itkMathTest.cxx

@blowekamp
Copy link
Copy Markdown
Member Author

blowekamp commented Feb 2, 2023

@N-Dekker I was just addressed warning I saw on the SimpleITK dashboard. I was unsure if the definition needed to occur at before the point of template instantiation/usage or before the header include. Are you able to verify if this approach takes care of the warnings in MSVC?

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Feb 2, 2023

I changed my ryzenator nightly build configuration to also test ITK_LEGACY_REMOVE:BOOL=OFF. We should have the results on the dashboard tomorrow.

@N-Dekker
Copy link
Copy Markdown
Contributor

N-Dekker commented Feb 2, 2023

I'm not sure, maybe we should add _SILENCE_NONFLOATING_COMPLEX_DEPRECATION_WARNING to the entire project, via CMake. Unless LEGACY_REMOVE=ON, of course.

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Feb 2, 2023

I just kicked of a build manually. Hopefully I got the settings right, so this will be tested.

@blowekamp
Copy link
Copy Markdown
Member Author

blowekamp commented Feb 2, 2023

I'm not sure, maybe we should add _SILENCE_NONFLOATING_COMPLEX_DEPRECATION_WARNING to the entire project, via CMake. Unless LEGACY_REMOVE=ON, of course.

If a project is using these deprecated std::complex types, IHMO they should get the warning as an indication they may get undefined behavior from their compiler. However, when compiling ITK we want to suppress the warning.

IMHO that is the goal here, so that could be a "private" define for the project.

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Feb 2, 2023

The build is finished. It doesn't have any warnings.

Aaaand I just figured out why. This build is done using VS2017, and VS2022 has introduced the warning.

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Feb 2, 2023

I have changed my dashboard build configuration to use multiple versions of Visual Studio. It should be exercised overnight.

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Feb 3, 2023

Here are the warnings. A sample:

M:\Dashboard\ITK\Modules\Core\Common\src\itkNumericTraits.cxx(76,86): warning C4996: 'std::complex<char>::complex': warning STL4037: The effect of instantiating the template std::complex for any type other than float, double, or long double is unspecified. You can define _SILENCE_NONFLOATING_COMPLEX_DEPRECATION_WARNING to suppress this warning.[M:\Dashboard\ITK-build\Modules\Core\Common\src\ITKCommon.vcxproj]
M:\Dashboard\ITK\Modules\Core\Common\test\itkMathTest.cxx(674,37): warning C4996: 'std::complex<int>::complex': warning STL4037: The effect of instantiating the template std::complex for any type other than float, double, or long double is unspecified. You can define _SILENCE_NONFLOATING_COMPLEX_DEPRECATION_WARNING to suppress this warning. [M:\Dashboard\ITK-build\Modules\Core\Common\test\itkMathTest.vcxproj]
M:\Dashboard\ITK\Modules\Core\Common\test\itkNumericTraitsTest.cxx(1276,43): warning C4996: 'std::complex<char>::complex': warning STL4037: The effect of instantiating the template std::complex for any type other than float, double, or long double is unspecified. You can define _SILENCE_NONFLOATING_COMPLEX_DEPRECATION_WARNING to suppress this warning. [M:\Dashboard\ITK-build\Modules\Core\Common\test\ITKCommon1TestDriver.vcxproj]
M:\Dashboard\ITK\Modules\Core\Common\include\itkNumericTraits.h(1939,16): warning C4996: 'std::complex<long>::complex': warning STL4037: The effect of instantiating the template std::complex for any type other than float, double, or long double is unspecified. You can define _SILENCE_NONFLOATING_COMPLEX_DEPRECATION_WARNING to suppress this warning. [M:\Dashboard\ITK-build\Modules\Core\Common\test\ITKCommon1TestDriver.vcxproj]

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Feb 3, 2023

This PR will silence many of the warnings. But definitely not the ones coming from itkMathTest.cxx, and probably not those coming from itkNumericTraitsTest.cxx.

@blowekamp
Copy link
Copy Markdown
Member Author

blowekamp commented Feb 3, 2023

OK, so the define before template instantiation was successful to suppress the warning. It is preferred to add the define in the code or as private properties for compiler flags?

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Feb 3, 2023

I don't have a preference for this.

@blowekamp blowekamp force-pushed the silence_nonfloating_complex branch from 8cdeffe to bf52233 Compare February 3, 2023 16:21
@github-actions github-actions bot added the type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct label Feb 3, 2023
dzenanz
dzenanz reviewed Feb 3, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

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

Thanks for the improvement.

Comment thread Modules/Core/Common/include/itkMath.h Outdated
@blowekamp blowekamp force-pushed the silence_nonfloating_complex branch from bf52233 to 3a9c762 Compare February 3, 2023 16:55
@blowekamp blowekamp merged commit 01b9f67 into InsightSoftwareConsortium:master Feb 3, 2023
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Mar 18, 2023
@N-Dekker
COMP: Place #define _SILENCE_..._DEPRECATION_WARNING before #include
3147972 
Moved each `_SILENCE_NONFLOATING_COMPLEX_DEPRECATION_WARNING` macro definition
upward, placing it before the very first `#include` statement.

Fixes Visual C++ 2022/C++17 warnings like:

> warning C4996: 'std::complex<char>::complex': warning STL4037: The effect of instantiating the template std::complex for any type other than float, double, or long double is unspecified. You can define _SILENCE_NONFLOATING_COMPLEX_DEPRECATION_WARNING to suppress this warning.

Follow-up to pull request InsightSoftwareConsortium#3897
commit 3a9c762
"COMP: Supress warning in MSVC for non-float complex"
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Mar 18, 2023
@N-Dekker
COMP: Place #define _SILENCE_..._DEPRECATION_WARNING before #include
14782c4 
Moved each `_SILENCE_NONFLOATING_COMPLEX_DEPRECATION_WARNING` macro definition
upward, placing it before the very first `#include` statement.

Fixes Visual C++ 2022/C++17 warnings like:

> warning C4996: 'std::complex<char>::complex': warning STL4037: The effect of instantiating the template std::complex for any type other than float, double, or long double is unspecified. You can define _SILENCE_NONFLOATING_COMPLEX_DEPRECATION_WARNING to suppress this warning.

Follow-up to pull request InsightSoftwareConsortium#3897
commit 3a9c762
"COMP: Supress warning in MSVC for non-float complex"
@N-Dekker N-Dekker mentioned this pull request Mar 18, 2023
Merged
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Mar 18, 2023
@N-Dekker
COMP: Place #define _SILENCE_..._DEPRECATION_WARNING before #include
8e60d58 
Moved each `_SILENCE_NONFLOATING_COMPLEX_DEPRECATION_WARNING` macro definition
upward, placing it before the very first `#include` statement.

Fixes Visual C++ 2022/C++17 warnings like:

> warning C4996: 'std::complex<char>::complex': warning STL4037: The effect of instantiating the template std::complex for any type other than float, double, or long double is unspecified. You can define _SILENCE_NONFLOATING_COMPLEX_DEPRECATION_WARNING to suppress this warning.

Follow-up to pull request InsightSoftwareConsortium#3897
commit 3a9c762
"COMP: Supress warning in MSVC for non-float complex"
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Mar 20, 2023
@N-Dekker
COMP: Place #define _SILENCE_..._DEPRECATION_WARNING before #include
51e4618 
Moved each `_SILENCE_NONFLOATING_COMPLEX_DEPRECATION_WARNING` macro definition
upward, placing it before the very first `#include` statement.

Fixes Visual C++ 2022 warnings like:

> warning C4996: 'std::complex<char>::complex': warning STL4037: The effect of instantiating the template std::complex for any type other than float, double, or long double is unspecified. You can define _SILENCE_NONFLOATING_COMPLEX_DEPRECATION_WARNING to suppress this warning.

Follow-up to pull request InsightSoftwareConsortium#3897
commit 3a9c762
"COMP: Supress warning in MSVC for non-float complex"
dzenanz pushed a commit that referenced this pull request Mar 20, 2023
@N-Dekker @dzenanz
COMP: Place #define _SILENCE_..._DEPRECATION_WARNING before #include
cd0ff94 
Moved each `_SILENCE_NONFLOATING_COMPLEX_DEPRECATION_WARNING` macro definition
upward, placing it before the very first `#include` statement.

Fixes Visual C++ 2022 warnings like:

> warning C4996: 'std::complex<char>::complex': warning STL4037: The effect of instantiating the template std::complex for any type other than float, double, or long double is unspecified. You can define _SILENCE_NONFLOATING_COMPLEX_DEPRECATION_WARNING to suppress this warning.

Follow-up to pull request #3897
commit 3a9c762
"COMP: Supress warning in MSVC for non-float complex"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dzenanz dzenanz dzenanz approved these changes

@N-Dekker N-Dekker N-Dekker approved these changes

Assignees

No one assigned

Labels

area:Core Issues affecting the Core module type:Compiler Compiler support or related warnings type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@blowekamp @N-Dekker @dzenanz