Skip to content

Avoid "message" naming conflicts exception macro, inherit constructors, remove ITK_MACROEND_NOOP_STATEMENT calls#2405

Merged
dzenanz merged 3 commits intoInsightSoftwareConsortium:masterfrom
N-Dekker:Replace-message-in-ExceptionMacro
Mar 15, 2021
Merged

Avoid "message" naming conflicts exception macro, inherit constructors, remove ITK_MACROEND_NOOP_STATEMENT calls#2405
dzenanz merged 3 commits intoInsightSoftwareConsortium:masterfrom
N-Dekker:Replace-message-in-ExceptionMacro

Conversation

@N-Dekker
Copy link
Copy Markdown
Contributor

@N-Dekker N-Dekker commented Mar 13, 2021

Replaced the very general term "message" inside the definition of
itkSpecializedMessageExceptionMacro by a much more specific
identifier, "exceptionDescriptionOutputStringStream", to avoid naming
conflicts in user code.

Removed the unnecessary std::string{ ... } construction for the result
of std::ostringstream::str(), as it is an std::string already.

Did inherit the exception constructors inside itkDeclareExceptionMacro,
as a follow-up to pull request #2398 commit e41b2ef
"STYLE: C++11 inheriting constructors from ExceptionObject for 4 classes"

Removed unnecessary ITK_MACROEND_NOOP_STATEMENT calls from exception macro definitions.

@N-Dekker N-Dekker marked this pull request as draft March 13, 2021 14:14
Replaced the very general term "message" inside the definition of
`itkSpecializedMessageExceptionMacro` by a much more specific
identifier, "exceptionDescriptionOutputStringStream", to avoid naming
conflicts in user code.

Removed the unnecessary `std::string{ ... }` construction for the result
of `std::ostringstream::str()`, as it is an `std::string` already.
@N-Dekker N-Dekker force-pushed the Replace-message-in-ExceptionMacro branch from 119abb9 to 50b707a Compare March 13, 2021 14:21
@N-Dekker N-Dekker requested a review from hjmjohnson March 13, 2021 14:23
@N-Dekker N-Dekker marked this pull request as ready for review March 13, 2021 14:35
@N-Dekker
Copy link
Copy Markdown
Contributor Author

As far as I can see, the one CI failure from ITK.Linux appears unrelated to this pull request:

Test: itkMRIBiasFieldCorrectionFilterTest (Failed)
Build: Linux-Build5621-PR2405-Replace-message-in-ExceptionMacro (Azure.fv-az47-680) on 2021-03-13 14:23:17
Repository revision: 50b707a
Test Details: Completed (SEGFAULT)

https://open.cdash.org/test/362575185

Follow-up to:
Pull request InsightSoftwareConsortium#2398
Commit e41b2ef
"STYLE: C++11 inheriting constructors from ExceptionObject for 4 classes"
@N-Dekker N-Dekker changed the title COMP: Avoid "message" naming conflicts when calling an exception macro Avoid "message" naming conflicts exception macro, inherit constructors Mar 14, 2021
It appears that the `ITK_MACROEND_NOOP_STATEMENT` calls at the end of the
definitions of `itkSpecializedExceptionMacro`, `itkExceptionMacro`, and
`itkGenericExceptionMacro` are not really necessary, as these three
macro definitions do not need to open a new scope either.
@N-Dekker N-Dekker requested a review from dzenanz March 15, 2021 13:11
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.

Maybe rename the PR?

@N-Dekker N-Dekker changed the title Avoid "message" naming conflicts exception macro, inherit constructors Avoid "message" naming conflicts exception macro, inherit constructors, remove ITK_MACROEND_NOOP_STATEMENT calls Mar 15, 2021
@N-Dekker
Copy link
Copy Markdown
Contributor Author

Thansk for the approval, @dzenanz

Maybe rename the PR?

While I always try to have at most 72 characters as subject line of any commit, the PR subject line length is unlimited, right? :-)

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Mar 15, 2021

I thought that PR might not be about "message" naming conflicts any more.

@dzenanz dzenanz merged commit 3f6c15e into InsightSoftwareConsortium:master Mar 15, 2021
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