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

itkMacros less robust / break external code that also has an itk namespace #4074

Closed
kislinsk opened this issue Jun 17, 2023 · 6 comments
Closed
Assignees
Labels
type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances type:Compiler Compiler support or related warnings

Comments

@kislinsk
Copy link
Contributor

Description

In 24e7b45, leading colons for namespaces have been removed with an automated search and replace. Unfortunately this also affected some macros and made them less robust resp. introduced a regression. We have a few projects that also have itk (sub-)namespaces and the lookup for classes like LightObject and ObjectFactory used in macros like itkNewMacro() is now broken in these cases. It is not a compiler-specific issue as it happens on MSVC, gcc, and clang.

Expected behavior

Macros always should be as robust as possible in particular regarding parenthesis or namespaces.

Versions

Worked in ITK v5.2.1, broken in ITK v5.3.0.

Environment

Windows, Ubuntu, macOS, MSVC, GCC, clang.

Additional Information

@kislinsk kislinsk added the type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances label Jun 17, 2023
@github-actions
Copy link

Thank you for contributing an issue! 🙏

Welcome to the ITK community! 🤗👋☀️

We are glad you are here and appreciate your contribution. Please keep in mind our community participation guidelines. 📜
Also, please check existing open issues and consider discussion on the ITK Discourse. 📖

@dzenanz
Copy link
Member

dzenanz commented Jun 19, 2023

Having a minimal example demonstrating the problem would probably be helpful. @Leengit could you tackle this without a demonstrative example?

@kislinsk
Copy link
Contributor Author

Here's a minimal example:

example.cpp:

#include <itkLightObject.h>
#include <itkObjectFactory.h>

namespace example
{
  namespace itk
  {
    // These two lines shouldn't be necessary and weren't until ITK v5.3.0.
    // Please keep the absolute ::itk namespaces in macros like itkNewMacro().
    using LightObject = ::itk::LightObject;
    template<typename T> using ObjectFactory = ::itk::ObjectFactory<T>;

    class Example : public ::itk::LightObject
    {
    public:
      using Self = Example;

      itkTypeMacro(Example, ::itk::LightObject)
      itkNewMacro(Self)

    protected:
      Example() = default;
      ~Example() override = default;
    };
  }
}

CMakeLists.txt:

cmake_minimum_required(VERSION 3.16.3...3.19.7 FATAL_ERROR)
project(Example)
find_package(ITK 5.3 REQUIRED COMPONENTS ITKCommon)
add_library(example example.cpp)
target_link_libraries(example PRIVATE ITKCommon)

@thewtex thewtex added the type:Compiler Compiler support or related warnings label Jun 23, 2023
thewtex added a commit to thewtex/ITK that referenced this issue Jun 23, 2023
@thewtex
Copy link
Member

thewtex commented Jun 23, 2023

@kislinsk thanks for the report. Can you please verify that #4087 addresses your use case?

@kislinsk
Copy link
Contributor Author

@thewtex Thanks, it works!

Not an issue for us but you may want to consider to do the same for the std namespace in macros.

thewtex added a commit to thewtex/ITK that referenced this issue Jun 27, 2023
@thewtex
Copy link
Member

thewtex commented Jun 27, 2023

@kislinsk thanks for the verification!

Good idea regarding std -- let's add that if it is found that it is needed.

@thewtex thewtex closed this as completed Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances type:Compiler Compiler support or related warnings
Projects
None yet
Development

No branches or pull requests

4 participants