Skip to content

Conversation

@sumir0
Copy link
Contributor

@sumir0 sumir0 commented Nov 29, 2025

  • Move (H|h)as_ traits from point_types.hpp into field_traits.(h|hpp)
  • Add field_traits.h include where field traits are used
  • Change a MSVC compiler warning reset via #pragma warning((push|pop))
  • Add field_traits.h include in point_types.h for backward compatibility
  • Remove unnecessary pcl/point_types.h includes in some files
  • Add missing pcl/point_types.h includes in some files
  • Add missing cstring include in pcl/common/impl/copy_point.hpp

* Move `(H|h)as_` traits from `point_types.hpp` into `field_traits.(h|hpp)`
* Add `field_traits.h` include where field traits are used
* Change a MSVC compiler warning reset via `#pragma warning((push|pop))`
* Add `field_traits.h` include in `point_types.h` for backward compatibility

Signed-off-by: Ramir Sultanov <[email protected]>
@sumir0 sumir0 changed the title Split point_types.hpp part into field_traits.(h|hpp) WIP: Split point_types.hpp part into field_traits.(h|hpp) Nov 29, 2025
@sumir0
Copy link
Contributor Author

sumir0 commented Nov 29, 2025

Also may fix #5040

@sumir0
Copy link
Contributor Author

sumir0 commented Nov 29, 2025

Based on #6367

@sumir0
Copy link
Contributor Author

sumir0 commented Nov 29, 2025

I will try to build manually with PCL_OPTIMIZE_IMPORTS_FIELD_TRAITS. Maybe additional pipelines can be considered.


Edit: Compilation finished successfully.

@sumir0 sumir0 changed the title WIP: Split point_types.hpp part into field_traits.(h|hpp) Split point_types.hpp part into field_traits.(h|hpp) Nov 29, 2025
@sumir0 sumir0 marked this pull request as ready for review November 29, 2025 13:31
* Remove unnecessary `pcl/point_types.h` includes in some files
* Add missing `pcl/point_types.h` includes in some files

Signed-off-by: Ramir Sultanov <[email protected]>
* Change license description in `pcl/field_traits.(h|hpp)`

Signed-off-by: Ramir Sultanov <[email protected]>
* Add missing `pcl/point_types.h` includes in `examples`

Signed-off-by: Ramir Sultanov <[email protected]>
@sumir0 sumir0 changed the title Split point_types.hpp part into field_traits.(h|hpp) WIP: Split point_types.hpp part into field_traits.(h|hpp) Dec 3, 2025
@sumir0 sumir0 marked this pull request as draft December 3, 2025 15:00
* Add missing `pcl/point_types.h` includes in some files

Signed-off-by: Ramir Sultanov <[email protected]>
Add missing `cstring` include in `common/include/pcl/common/impl/copy_point.hpp`

Signed-off-by: Ramir Sultanov <[email protected]>
@sumir0
Copy link
Contributor Author

sumir0 commented Dec 4, 2025

For a moment I thought I was going under some water here... Phew, it was close! Excuse me for informality. As the pipelines are passing it seems that we are good to go (or maybe not, because there are still some files for which it would be nice to add missing includes, but they can be added in other PRs, possibly)

@sumir0 sumir0 changed the title WIP: Split point_types.hpp part into field_traits.(h|hpp) Split point_types.hpp part into field_traits.(h|hpp) Dec 4, 2025
@sumir0 sumir0 marked this pull request as ready for review December 4, 2025 21:29
@larshg
Copy link
Contributor

larshg commented Dec 5, 2025

I tried it on windows yesterday and noticed a lot of: no suitable definition provided for explicit template instantiation request.

See windows ci: https://dev.azure.com/PointCloudLibrary/pcl/_build/results?buildId=25918&view=logs&j=b7dd6ba8-1ad9-5018-2280-3f36c72607c9&t=2fd7de77-c8c6-5479-ec09-1c18859bcc95&l=61

I haven't quite figured out why its not provided, but we should look into it before merging.

@sumir0
Copy link
Contributor Author

sumir0 commented Dec 5, 2025

Oh, nice catch! @larshg

As far as I understand these warnings were disabled by pcl/common/include/pcl/pcl_macros.h (link to the latest commit in master at the moment - search for "4661").

pcl/common/include/pcl/pcl_macros.h is included by pcl/common/include/pcl/impl/point_types.hpp which is included by pcl/common/include/pcl/point_types.h.

But pcl/common/include/pcl/pcl_macros.h is not included by pcl/common/include/pcl/field_traits.h.

Therefore, in the translation units where pcl/common/include/pcl/pcl_macros.h is not included (for example: where I replaced pcl/common/include/pcl/point_types.h with pcl/common/include/pcl/field_traits.h and pcl/common/include/pcl/pcl_macros.h did not get to be included) these warnings are not disabled.

@larshg
Copy link
Contributor

larshg commented Dec 5, 2025

Ahh, yes. And it doesn't seem to be worth looking into fixing that warning, so we just need to include pcl_macros.h accordingly?

@sumir0
Copy link
Contributor Author

sumir0 commented Dec 8, 2025

Hmm. Disabling warnings in a header file (without returning to the state before the disabling) will affect warnings in user code which includes (directly or transitively) that header. Would it not be better to disable project scope warnings using a build system (in our case CMake)?

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.

3 participants