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

COMP: ISO C++17 does not allow dynamic exception #48

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

Conversation

hjmjohnson
Copy link
Member

LesionSizingToolkit/src/itkCannyEdgeDetectionImageFilter2.cxx:30: LesionSizingToolkit/include/itkCannyEdgeDetectionRecursiveGaussianImageFilter.h:201:34: error: ISO C++17 does not allow dynamic exception specifications [-Wdynamic-exception-spec]
201 | GenerateInputRequestedRegion() throw(InvalidRequestedRegionError) override;
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
LesionSizingToolkit/include/itkCannyEdgeDetectionRecursiveGaussianImageFilter.h:201:34: note: use 'noexcept(false)' instead
201 | GenerateInputRequestedRegion() throw(InvalidRequestedRegionError) override;
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| noexcept(false)

@hjmjohnson hjmjohnson force-pushed the dynamic-exceptions-cxx17 branch 2 times, most recently from 0b7dc90 to 8cccf8b Compare November 25, 2024 14:12
@hjmjohnson hjmjohnson self-assigned this Nov 25, 2024
@hjmjohnson hjmjohnson added the bug label Nov 25, 2024
Copy link
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.

Is this supposed to pass the CI?

Copy link
Member

@jhlegarreta jhlegarreta left a comment

Choose a reason for hiding this comment

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

The CI is not being able to run because the default branch needs to be renamed to main: I asked Hans privately if I can proceed.
https://github.com/InsightSoftwareConsortium/LesionSizingToolkit/blob/master/.github/workflows/build-test-package.yml#L6

Also, it may still fail due to the TIFF library issues steming from ITK:
https://open.cdash.org/viewBuildError.php?buildid=10048610

We'll see after I rename the branch.

But changes look OK to me.

@jhlegarreta
Copy link
Member

I have renamed the default branch. Builds have been triggered for main. This PR will need to be rebased and force pushed.

LesionSizingToolkit/src/itkCannyEdgeDetectionImageFilter2.cxx:30:
LesionSizingToolkit/include/itkCannyEdgeDetectionRecursiveGaussianImageFilter.h:201:34: error: ISO C++17 does not allow dynamic exception specifications [-Wdynamic-exception-spec]
  201 |   GenerateInputRequestedRegion() throw(InvalidRequestedRegionError) override;
      |                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
LesionSizingToolkit/include/itkCannyEdgeDetectionRecursiveGaussianImageFilter.h:201:34: note: use 'noexcept(false)' instead
  201 |   GenerateInputRequestedRegion() throw(InvalidRequestedRegionError) override;
      |                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                  noexcept(false)
Prefer inintialization to assignment.

Ignoring return value of function declared with 'nodiscard' attribute
The sigmoidAlpha value was not tested in the copied
and pasted code that was not properly modified.
@jhlegarreta jhlegarreta force-pushed the dynamic-exceptions-cxx17 branch from 8cccf8b to 76247aa Compare November 29, 2024 14:30
@jhlegarreta
Copy link
Member

CMake config step is failing because ITKVtkGlue is not being found:
https://open.cdash.org/builds/10057132/configure

@dzenanz
Copy link
Member

dzenanz commented Nov 29, 2024

Isn't that option off by default?

option(LSTK_USE_VTK "Build visualization helper tools." OFF)

@dzenanz
Copy link
Member

dzenanz commented Nov 29, 2024

Ah, this:


should be conditional upon LSTK_USE_VTK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants