Optimize include headers

[ahcorde]

Optimize include headers

Reduce in my computer around 2.5s. From 22.6 s to 21.00 s

Claude Opus 4.7

[mergify]

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

• Queue this pull request

[asymingt]

Can you ask your AI agent to stick with the public-facing interface and avoid using the headers in detail, as they may change. IMHO the brittleness added by this is not worth any performance gain.

[asymingt]

Is this what you wanted? I don't think we should be depending on the internal headers in /detail/.

[wjwwood]

Well, the struct isn't necessarily bad to use, even if it's in the detail namespace. It's just the C++ struct for the message, but I don't know if saves much to include this over the standard header, for example the standard header includes stuff like this:

% cat string.hpp
// generated from rosidl_generator_cpp/resource/idl.hpp.em
// generated code does not contain a copyright notice

#ifndef STD_MSGS__MSG__STRING_HPP_
#define STD_MSGS__MSG__STRING_HPP_

#include "std_msgs/msg/detail/string__struct.hpp"  // IWYU pragma: export
#include "std_msgs/msg/detail/string__builder.hpp"    // IWYU pragma: export
#include "std_msgs/msg/detail/string__traits.hpp"    // IWYU pragma: export
#include "std_msgs/msg/detail/string__type_support.hpp"    // IWYU pragma: export

#endif  // STD_MSGS__MSG__STRING_HPP_

Each header shouldn't include too much that isn't already being included:

detail/string__struct.hpp

% cat detail/string__struct.hpp
// generated from rosidl_generator_cpp/resource/idl__struct.hpp.em
// with input from std_msgs:msg/String.idl
// generated code does not contain a copyright notice

// IWYU pragma: private, include "std_msgs/msg/string.hpp"


#ifndef STD_MSGS__MSG__DETAIL__STRING__STRUCT_HPP_
#define STD_MSGS__MSG__DETAIL__STRING__STRUCT_HPP_

#include <cstdint>
#include <memory>
#include <string>

#include "rosidl_runtime_cpp/message_initialization.hpp"


#ifndef _WIN32
# define DEPRECATED__std_msgs__msg__String __attribute__((deprecated))
#else
# define DEPRECATED__std_msgs__msg__String __declspec(deprecated)
#endif

namespace std_msgs
{

namespace msg
{

// message struct
template<class ContainerAllocator>
struct String_
{
[ ... ]
};  // struct String_

// alias to use template instance with default allocator
using String =
  std_msgs::msg::String_<std::allocator<void>>;

// constant definitions

}  // namespace msg

}  // namespace std_msgs

#endif  // STD_MSGS__MSG__DETAIL__STRING__STRUCT_HPP_

detail/string__builder.hpp

% cat detail/string__builder.hpp
// generated from rosidl_generator_cpp/resource/idl__builder.hpp.em
// with input from std_msgs:msg/String.idl
// generated code does not contain a copyright notice

// IWYU pragma: private, include "std_msgs/msg/string.hpp"

#ifndef STD_MSGS__MSG__DETAIL__STRING__BUILDER_HPP_
#define STD_MSGS__MSG__DETAIL__STRING__BUILDER_HPP_

#include

#include "std_msgs/msg/detail/string__struct.hpp"
#include "rosidl_runtime_cpp/message_initialization.hpp"

namespace std_msgs
{

namespace msg
{

namespace builder
{

class Init_String_data
{
public:
Init_String_data()
: msg_(::rosidl_runtime_cpp::MessageInitialization::SKIP)
{}
::std_msgs::msg::String data(::std_msgs::msg::String::data_type arg)
{
msg.data = std::move(arg);
return std::move(msg_);
}

private:
::std_msgs::msg::String msg_;
};

} // namespace builder

} // namespace msg

template
auto build();

template<>
inline
auto build<::std_msgs::msg::String>()
{
return std_msgs::msg::builder::Init_String_data();
}

} // namespace std_msgs

#endif // STD_MSGS__MSG__DETAIL__STRING__BUILDER_HPP_

detail/string__traits.hpp

% cat detail/string__traits.hpp
// generated from rosidl_generator_cpp/resource/idl__traits.hpp.em
// with input from std_msgs:msg/String.idl
// generated code does not contain a copyright notice

// IWYU pragma: private, include "std_msgs/msg/string.hpp"

#ifndef STD_MSGS__MSG__DETAIL__STRING__TRAITS_HPP_
#define STD_MSGS__MSG__DETAIL__STRING__TRAITS_HPP_

#include <stdint.h>

#include
#include
#include
#include
#include <string_view>
#include
#include <type_traits>
#include

#include "std_msgs/msg/detail/string__struct.hpp"
#include "rosidl_runtime_cpp/buffer__traits.hpp"
#include "rosidl_runtime_cpp/traits.hpp"

namespace std_msgs
{

namespace msg
{

inline void to_flow_style_yaml(
const String & msg,
std::ostream & out)
{
out << "{";
// member: data
{
out << "data: ";
rosidl_generator_traits::value_to_yaml(msg.data, out);
}
out << "}";
} // NOLINT(readability/fn_size)

inline void to_block_style_yaml(
const String & msg,
std::ostream & out, size_t indentation = 0)
{
// member: data
{
if (indentation > 0) {
out << std::string(indentation, ' ');
}
out << "data: ";
rosidl_generator_traits::value_to_yaml(msg.data, out);
out << "\n";
}
} // NOLINT(readability/fn_size)

inline std::string to_yaml(const String & msg, bool use_flow_style = false)
{
std::ostringstream out;
if (use_flow_style) {
to_flow_style_yaml(msg, out);
} else {
to_block_style_yaml(msg, out);
}
return out.str();
}

template<typename T, std::enable_if_t<std::is_same_v<std::decay_t, std_msgs::msg::String>, int> = 0>
constexpr auto as_tuple_ref(T && msg)
{
return std::forward_as_tuple(std::forward(msg).data);
}

} // namespace msg

} // namespace std_msgs

namespace rosidl_generator_traits
{

template<>
constexpr const char * data_type<std_msgs::msg::String>()
{
return "std_msgs::msg::String";
}

template<>
constexpr const char * name<std_msgs::msg::String>()
{
return "std_msgs/msg/String";
}

template<>
struct has_fixed_size<std_msgs::msg::String>
: std::integral_constant<bool, false> {};

template<>
struct has_bounded_size<std_msgs::msg::String>
: std::integral_constant<bool, false> {};

template<>
struct is_message<std_msgs::msg::String>
: std::true_type {};

template<>
struct MessageTraits<std_msgs::msg::String>
{
static constexpr std::size_t member_count = 1;
static constexpr std::array<std::string_view, member_count> member_names = {
"data",
};
};

} // namespace rosidl_generator_traits

#endif // STD_MSGS__MSG__DETAIL__STRING__TRAITS_HPP_

detail/string__type_support.hpp

% cat detail/string__type_support.hpp
// generated from rosidl_generator_cpp/resource/idl__type_support.hpp.em
// with input from std_msgs:msg/String.idl
// generated code does not contain a copyright notice

#ifndef STD_MSGS__MSG__DETAIL__STRING__TYPE_SUPPORT_HPP_
#define STD_MSGS__MSG__DETAIL__STRING__TYPE_SUPPORT_HPP_

#include "rosidl_typesupport_interface/macros.h"

#include "std_msgs/msg/rosidl_generator_cpp__visibility_control.hpp"

#include "rosidl_typesupport_cpp/message_type_support.hpp"

#ifdef __cplusplus
extern "C"
{
#endif
// Forward declare the get type support functions for this type.
ROSIDL_GENERATOR_CPP_PUBLIC_std_msgs
const rosidl_message_type_support_t *
ROSIDL_TYPESUPPORT_INTERFACE__MESSAGE_SYMBOL_NAME(
rosidl_typesupport_cpp,
std_msgs,
msg,
String
)();
#ifdef __cplusplus
}
#endif

#endif // STD_MSGS__MSG__DETAIL__STRING__TYPE_SUPPORT_HPP_

Also, the traits and stuff are sometimes assumed to exist if you've using the message, though I guess you'd just get a compiler error if that was actually an issue.

On balance, I would agree that we should just include the main header for the messages, unless we have some evidence that including more than the struct is a significant burden.

[asymingt]

Same here.

[asymingt]

Same here.

[asymingt]

Same here.