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

Allow Node functionality to use methods with noexcept modifiers. #2654

Open
scrouthtv opened this issue Oct 18, 2024 · 2 comments
Open

Allow Node functionality to use methods with noexcept modifiers. #2654

scrouthtv opened this issue Oct 18, 2024 · 2 comments
Labels

Comments

@scrouthtv
Copy link

Feature request

Feature description

The rclcpp::Node methods

  • create_subscription()
  • create_wall_timer()
  • create_service()
  • add_on_set_parameters_callback(), remove_on_set_parameters_callback(), set_on_parameters_set_callback()
    all take a callback as parameter. I think, the callback methods should be able to be marked as noexcept. For example, this currently does not work:
#include <chrono>
#include <functional>
#include <memory>

#include <rclcpp/rclcpp.hpp>

class DemoNode : public rclcpp::Node {
 public:
  DemoNode() : rclcpp::Node("demo127") {
    this->create_wall_timer(
      std::chrono::milliseconds(100),
      std::bind(&DemoNode::read, this));
    }
	
 private:
  void read() noexcept {
    // etc.
  }
};

int main(int argc, char ** argv) {
  rclcpp::init(argc, argv);
  auto node = std::make_shared<DemoNode>();
  rclcpp::spin(node);
  return rclcpp::shutdown();
}

Won't compile with this error:

error: no matching function for call to ‘DemoNode::create_wall_timer(std::chrono::milliseconds, std::_Bind_helper<false, void (DemoNode::*)() noexcept, DemoNode*>::type)

Removing the noexcept modifier fixes the issue.

I think it should be possible to add the noexcept modifier to callbacks in timers, services, etc.

Going further, it may only make sense to have these callbacks forced to be noexcept?

Implementation considerations

noexcept has only been available since C++11.

@cottsay
Copy link
Member

cottsay commented Nov 1, 2024

Hi there - thanks for filing the ticket.

As an immediate path forward, is there any reason you couldn't use a lambda to wrap your noexcept function?

@cottsay cottsay added the backlog label Nov 1, 2024
@clalancette
Copy link
Contributor

Going further, it may only make sense to have these callbacks forced to be noexcept?

I don't think we should force the callbacks to be noexcept; there are legitimate reasons for throwing exceptions in them.

That said, I do agree that it would be nice to accept methods that have the noexcept attribute on them. We probably won't have time to look into it right now, but if you'd like to contribute a pull request to add it we'd be happy to review it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants