From 0a6c79e6ccd463e384855001dc918eb8d45c8311 Mon Sep 17 00:00:00 2001 From: Artem Shumov Date: Thu, 14 Apr 2022 17:17:02 +0300 Subject: [PATCH 1/3] Add a check if the second argument of logging macros is a string literal Signed-off-by: Artem Shumov --- rclcpp/resource/logging.hpp.em | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/rclcpp/resource/logging.hpp.em b/rclcpp/resource/logging.hpp.em index 7b5b0f349f..050c2afc5b 100644 --- a/rclcpp/resource/logging.hpp.em +++ b/rclcpp/resource/logging.hpp.em @@ -143,6 +143,12 @@ def get_rclcpp_suffix_from_features(features): @[ if 'stream' in feature_combination]@ std::stringstream rclcpp_stream_ss_; \ rclcpp_stream_ss_ << @(stream_arg); \ +@[ else]@ + { \ + constexpr bool starts_with_double_quotes = (#__VA_ARGS__)[0] == '"'; \ + static_assert(starts_with_double_quotes, \ + "Second argument to RCLCPP_@(severity) must be a string literal"); \ + } \ @[ end if]@ RCUTILS_LOG_@(severity)@(get_suffix_from_features(feature_combination))_NAMED( \ @{params = ['get_time_point' if p == 'clock' and 'throttle' in feature_combination else p for p in params]}@ From abbac7c4a31cf0a12826bb4d908dadf953480d58 Mon Sep 17 00:00:00 2001 From: Artem Shumov Date: Fri, 15 Apr 2022 15:40:31 +0300 Subject: [PATCH 2/3] Add a build failure test and disable it like other build failure tests Signed-off-by: Artem Shumov --- rclcpp/test/rclcpp/CMakeLists.txt | 5 ++++ ...ss_non_string_literal_to_logging_macro.cpp | 24 +++++++++++++++++++ 2 files changed, 29 insertions(+) create mode 100644 rclcpp/test/rclcpp/test_does_not_compile/pass_non_string_literal_to_logging_macro.cpp diff --git a/rclcpp/test/rclcpp/CMakeLists.txt b/rclcpp/test/rclcpp/CMakeLists.txt index 6f915feef5..92bb060bd2 100644 --- a/rclcpp/test/rclcpp/CMakeLists.txt +++ b/rclcpp/test/rclcpp/CMakeLists.txt @@ -279,6 +279,11 @@ endif() # target_link_libraries(build_failure__get_node_topics_interface_const_ptr_rclcpp_node # ${PROJECT_NAME}) +# rclcpp_add_build_failure_test(build_failure__pass_non_string_literal_to_logging_macro +# test_does_not_compile/pass_non_string_literal_to_logging_macro.cpp) +# target_link_libraries(build_failure__pass_non_string_literal_to_logging_macro +# ${PROJECT_NAME}) + ament_add_gtest(test_node_global_args test_node_global_args.cpp) if(TARGET test_node_global_args) ament_target_dependencies(test_node_global_args diff --git a/rclcpp/test/rclcpp/test_does_not_compile/pass_non_string_literal_to_logging_macro.cpp b/rclcpp/test/rclcpp/test_does_not_compile/pass_non_string_literal_to_logging_macro.cpp new file mode 100644 index 0000000000..5b5e5427ff --- /dev/null +++ b/rclcpp/test/rclcpp/test_does_not_compile/pass_non_string_literal_to_logging_macro.cpp @@ -0,0 +1,24 @@ +// Copyright 2022 Open Source Robotics Foundation, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include + +#include "rclcpp/logger.hpp" +#include "rclcpp/logging.hpp" + +int main(void) +{ + std::string s = "message"; + RCLCPP_ERROR(rclcpp::get_logger("name"), s.c_str()); +} From 135aa9af2fb1ee4acb4f7bf141c851a00f1e6efd Mon Sep 17 00:00:00 2001 From: Artem Shumov Date: Fri, 15 Apr 2022 15:40:54 +0300 Subject: [PATCH 3/3] Fix copypaste error in disabled tests Signed-off-by: Artem Shumov --- rclcpp/test/rclcpp/CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rclcpp/test/rclcpp/CMakeLists.txt b/rclcpp/test/rclcpp/CMakeLists.txt index 92bb060bd2..4fb9acbc5f 100644 --- a/rclcpp/test/rclcpp/CMakeLists.txt +++ b/rclcpp/test/rclcpp/CMakeLists.txt @@ -271,12 +271,12 @@ endif() # rclcpp_add_build_failure_test(build_failure__get_node_topics_interface_const_ref_wrapped_node # node_interfaces/test_does_not_compile/get_node_topics_interface_const_ref_wrapped_node.cpp) -# target_link_libraries(build_failure__get_node_topics_interface_const_ref_rclcpp_node +# target_link_libraries(build_failure__get_node_topics_interface_const_ref_wrapped_node # ${PROJECT_NAME}) # rclcpp_add_build_failure_test(build_failure__get_node_topics_interface_const_ptr_wrapped_node # node_interfaces/test_does_not_compile/get_node_topics_interface_const_ptr_wrapped_node.cpp) -# target_link_libraries(build_failure__get_node_topics_interface_const_ptr_rclcpp_node +# target_link_libraries(build_failure__get_node_topics_interface_const_ptr_wrapped_node # ${PROJECT_NAME}) # rclcpp_add_build_failure_test(build_failure__pass_non_string_literal_to_logging_macro