Skip to content
Snippets Groups Projects

Draft: Capture args by reference instead of copy

Closed Rainer Kartmann requested to merge feature/passing-non-const-ref-to-skill-factory into master

I have skills that were initialized with connectTo(armarx::armem::client::MemoryNameSystem& mns) function after creation. Note that the mns is passed by reference to allow subscriptions etc. (see https://git.h2t.iar.kit.edu/research-projects/jubot/demo/-/merge_requests/11)

I updated them now to the changes in the skill framework. This lead to the skill receiving the MNS reference via constructor, and the component passing the MNS reference via the addSkillFactory<>() function.

Currently, this leads to

error message
In file included from /usr/include/c++/8/memory:80,
                 from /home/.../armarx/ArmarXCore/source/ArmarXCore/core/logging/LoggingUtil.h:26,
                 from /home/.../armarx/ArmarXCore/source/ArmarXCore/core/logging/LogSender.h:28,
                 from /home/.../armarx/ArmarXCore/source/ArmarXCore/core/logging/Logging.h:30,
                 from /home/.../armarx/ArmarXCore/source/ArmarXCore/core/ManagedIceObject.h:31,
                 from /home/.../armarx/ArmarXCore/source/ArmarXCore/core/Component.h:28,
                 from /home/.../h2t/research_projects/jubot/demo/source/jubot/demo/components/personalized_dialog_skills/Component.h:29,
                 from /home/.../h2t/research_projects/jubot/demo/source/jubot/demo/components/personalized_dialog_skills/Component.cpp:24:
/usr/include/c++/8/bits/unique_ptr.h: In instantiation of ‘typename std::_MakeUniq<_Tp>::__single_object std::make_unique(_Args&& ...) [with _Tp = jubot::demo::personalized_dialog::skills::Hello; _Args = {const armarx::armem::client::MemoryNameSystem&}; typename std::_MakeUniq<_Tp>::__single_object = std::unique_ptr<jubot::demo::personalized_dialog::skills::Hello, std::default_delete<jubot::demo::personalized_dialog::skills::Hello> >]’:
/home/.../armarx/RobotAPI/source/RobotAPI/libraries/skills/provider/SkillFactory.h:36:56:   required from ‘static std::unique_ptr<armarx::skills::SkillBlueprint> armarx::skills::SkillBlueprint::ForSkill(Args&& ...) [with _Skill = jubot::demo::personalized_dialog::skills::Hello; Args = {armarx::armem::client::MemoryNameSystem&}]’
/home/.../armarx/RobotAPI/source/RobotAPI/libraries/skills/provider/SkillProviderComponentPlugin.h:62:64:   required from ‘armarx::skills::SkillBlueprint* armarx::plugins::SkillProviderComponentPlugin::addSkillFactory(Args&& ...) [with SkillT = jubot::demo::personalized_dialog::skills::Hello; Args = {armarx::armem::client::MemoryNameSystem&}]’
/home/.../armarx/RobotAPI/source/RobotAPI/libraries/skills/provider/SkillProviderComponentPlugin.h:184:79:   required from ‘armarx::skills::SkillBlueprint* armarx::SkillProviderComponentPluginUser::addSkillFactory(Args&& ...) [with SkillT = jubot::demo::personalized_dialog::skills::Hello; Args = {armarx::armem::client::MemoryNameSystem&}]’
/home/.../h2t/research_projects/jubot/demo/source/jubot/demo/components/personalized_dialog_skills/Component.cpp:72:43:   required from here
/usr/include/c++/8/bits/unique_ptr.h:835:30: error: binding reference of type ‘armarx::armem::client::MemoryNameSystem&’ to ‘const armarx::armem::client::MemoryNameSystem’ discards qualifiers
     { return unique_ptr<_Tp>(new _Tp(std::forward<_Args>(__args)...)); }
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/.../h2t/research_projects/jubot/demo/source/jubot/demo/components/personalized_dialog_skills/Component.cpp:34:
/home/.../h2t/research_projects/jubot/demo/source/jubot/demo/personalized_dialog/skills/Hello.h:28:9: note:   initializing argument 1 of ‘jubot::demo::personalized_dialog::skills::Hello::Hello(armarx::armem::client::MemoryNameSystem&)’
         Hello(armarx::armem::client::MemoryNameSystem& mns);
         ^~~~~
In file included from /usr/include/c++/8/memory:80,
                 from /home/.../armarx/ArmarXCore/source/ArmarXCore/core/logging/LoggingUtil.h:26,
                 from /home/.../armarx/ArmarXCore/source/ArmarXCore/core/logging/LogSender.h:28,
                 from /home/.../armarx/ArmarXCore/source/ArmarXCore/core/logging/Logging.h:30,
                 from /home/.../armarx/ArmarXCore/source/ArmarXCore/core/ManagedIceObject.h:31,
                 from /home/.../armarx/ArmarXCore/source/ArmarXCore/core/Component.h:28,
                 from /home/.../h2t/research_projects/jubot/demo/source/jubot/demo/components/personalized_dialog_skills/Component.h:29,
                 from /home/.../h2t/research_projects/jubot/demo/source/jubot/demo/components/personalized_dialog_skills/Component.cpp:24:
/usr/include/c++/8/bits/unique_ptr.h: In instantiation of ‘typename std::_MakeUniq<_Tp>::__single_object std::make_unique(_Args&& ...) [with _Tp = jubot::demo::personalized_dialog::skills::LearnCurrentPerson; _Args = {const armarx::armem::client::MemoryNameSystem&, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&}; typename std::_MakeUniq<_Tp>::__single_object = std::unique_ptr<jubot::demo::personalized_dialog::skills::LearnCurrentPerson, std::default_delete<jubot::demo::personalized_dialog::skills::LearnCurrentPerson> >]’:
/home/.../armarx/RobotAPI/source/RobotAPI/libraries/skills/provider/SkillFactory.h:36:56:   required from ‘static std::unique_ptr<armarx::skills::SkillBlueprint> armarx::skills::SkillBlueprint::ForSkill(Args&& ...) [with _Skill = jubot::demo::personalized_dialog::skills::LearnCurrentPerson; Args = {armarx::armem::client::MemoryNameSystem&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&}]’
/home/.../armarx/RobotAPI/source/RobotAPI/libraries/skills/provider/SkillProviderComponentPlugin.h:62:64:   required from ‘armarx::skills::SkillBlueprint* armarx::plugins::SkillProviderComponentPlugin::addSkillFactory(Args&& ...) [with SkillT = jubot::demo::personalized_dialog::skills::LearnCurrentPerson; Args = {armarx::armem::client::MemoryNameSystem&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&}]’
/home/.../armarx/RobotAPI/source/RobotAPI/libraries/skills/provider/SkillProviderComponentPlugin.h:184:79:   required from ‘armarx::skills::SkillBlueprint* armarx::SkillProviderComponentPluginUser::addSkillFactory(Args&& ...) [with SkillT = jubot::demo::personalized_dialog::skills::LearnCurrentPerson; Args = {armarx::armem::client::MemoryNameSystem&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&}]’
/home/.../h2t/research_projects/jubot/demo/source/jubot/demo/components/personalized_dialog_skills/Component.cpp:74:93:   required from here
/usr/include/c++/8/bits/unique_ptr.h:835:30: error: binding reference of type ‘armarx::armem::client::MemoryNameSystem&’ to ‘const armarx::armem::client::MemoryNameSystem’ discards qualifiers
     { return unique_ptr<_Tp>(new _Tp(std::forward<_Args>(__args)...)); }
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/.../h2t/research_projects/jubot/demo/source/jubot/demo/components/personalized_dialog_skills/Component.cpp:35:
/home/.../h2t/research_projects/jubot/demo/source/jubot/demo/personalized_dialog/skills/LearnCurrentPerson.h:29:9: note:   initializing argument 1 of ‘jubot::demo::personalized_dialog::skills::LearnCurrentPerson::LearnCurrentPerson(armarx::armem::client::MemoryNameSystem&, const string&)’
         LearnCurrentPerson(armarx::armem::client::MemoryNameSystem& mns,
         ^~~~~~~~~~~~~~~~~~

which is fixed by this MR.

However, this requires that the passed arguments exist as long as the skill can be created, which might be non-obvious. So I am not sure whether we should keep this.

@peller @dreher Do you have ideas/opinions on this situation?

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Author Maintainer

    Update: I can work around it by passing a std::reference_wrapper to addSkillFactory (and being aware that the reference exists after the calling function).

    armarx::armem::client::MemoryNameSystem& mns = 
       plugins.armemClient->getMemoryNameSystemClient();
    
    addSkillFactory<skills::Hello>(std::reference_wrapper(mns));

    Maybe this is good enough and we can close this MR

    Edited by Rainer Kartmann
  • Actually it was intended that the arguments are passed by value to the lambda. That way its ensured, that all arguments exist when the factory creates a skill.

    I would say, if you really need a reference then you should use a reference_wrapper (as you did). Then its your responsibility that the reference still exists when the skill is created.

Please register or sign in to reply
Loading