Usage of start()

[Tectu]

@0x00002a I have a question regarding the current way of starting a "session". I feel like I lack some C++ knowledge here but I'd rather just ask: start() is a private friend function. In the various examples, we just call start() like a free standing function from main(). How does this work?

In an application which would consume malloy-server, how would one call this start() private friend function from an application specific controller class which internally uses malloy-server? Unless I am missing something very obvious we'd have to call malloy::server::routing_context::start() from a class such as myapp::controller but it won't have access to this private friend function.

[0x00002a]

@Tectu This is a weirdness with how friend functions work. Basically: friend functions ignore namespacing when pariticpating in overload resolution for arguments of the type they are friended to (ADL, other contraints still apply afaik) and access specifiers have no effect, additionally it is both implicitly in the namespace enclosing its frend and inaccessible without ADL unless a forward declaration for it is in said namespace.

tl;dr for start the only way currently to access it is by calling it with the routing context or controller. We could make it accessible via malloy:: ... :: by adding a forward declaration for it in the respective namespaces

[Tectu]

@Tectu This is a weirdness with how friend functions work. Basically: friend functions ignore namespacing when pariticpating in overload resolution for arguments of the type they are friended to (ADL, other contraints still apply afaik) and access specifiers have no effect, additionally it is both implicitly in the namespace enclosing its frend and inaccessible without ADL unless a forward declaration for it is in said namespace.

Well - that is something I didn't know :D Thanks!

tl;dr for start the only way currently to access it is by calling it with the routing context or controller. We could make it accessible via malloy:: ... :: by adding a forward declaration for it in the respective namespaces

So how would I convert this to the new API?

namespace myapp
{
    class controller
    {
    public:
        controller()
    {
        m_malloy_ctrl = std::make_shared<malloy::server::controller>();
        if (!m_malloy_controller->init(cfg.malloy))
            throw std::runtime_error("could not initialize malloy controller.");
    }

    void start()
    {
        m_malloy_ctrl->start();
    }

    void stop()
    {
        m_malloy_ctrl->stop().wait();
    }

    private:
        std::shared_ptr<malloy::server::controller> m_malloy_ctrl;
    };
}

There is obviously going more on in that class but that would be the malloy specific parts.

[0x00002a]

you'll need something like std::optional to store the session, you could do something like:

controller()
    {
try { 
        m_malloy_ctrl = std::make_shared<malloy::server::controller>(cfg.malloy); // this will throw a runtime_error if the config is wrong
catch (const std::runtime_error&){
            throw std::runtime_error("could not initialize malloy controller.");
}
    }

void start() {
    m_session = start(std::move(*m_malloy_ctrl));
}
void stop() {
    m_session = std::nullopt;
}
private: 
    std::optional<malloy::server::routing_context::session> m_session;

If you need it to be copyable you could make that optional a shared_ptr, I guess you could also use a variant for either the routing context or session (std::variant<std::shared_ptr<...routing_context>, ...session>), thereby preventing any invalid accesses (although silently)

[Tectu]

Yeah that's what I tried but I didn't get it to work (didn't get it to compile):

C:\Users\joel\Documents\projects\myapp\server\controller.cpp: In member function 'void elx::server::controller::start()':
C:\Users\joel\Documents\projects\myapp\server\controller.cpp:83:29: error: no matching function for call to 'elx::server::controller::start(std::remove_reference<malloy::server::routing_context&>::type)'
   83 |     m_malloy_session = start(std::move(*m_malloy_controller));
      |                        ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
C:\Users\joel\Documents\projects\myapp\server\controller.cpp:68:6: note: candidate: 'void elx::server::controller::start()'
   68 | void elx::server::controller::start()
      |      ^~~
C:\Users\joel\Documents\projects\elx\lib\elx\server\controller.cpp:68:6: note:   candidate expects 0 arguments, 1 provided

The call to start() is resolving to myapp::controller::start(). If I specify start() explicitly:

malloy::server::routing_context::start(std::move(*m_malloy_controller));

compiling:

C:\Users\joel\Documents\projects\myapp\server\controller.cpp: In member function 'void myapp::server::controller::start()':
C:\Users\joel\Documents\projects\myapp\server\controller.cpp:83:57: error: 'start' is not a member of 'malloy::server::routing_context'
   83 |     m_malloy_session = malloy::server::routing_context::start(std::move(*m_malloy_controller));
      |                                                         ^~~~~

As I said: I feel like I'm missing something ridiculously obvious here.

[0x00002a]

ah, yeah thats a problem. There's no way to access it if you've also got a member function with that name. A fix is to add this to your file:

namespace malloy::server {
auto start(malloy::server::routing_context&&) -> malloy::server::routing_context::session;
}

you can then call it directly with malloy::server::start(...). Really though I think we should add this to routing_context.hpp and the equivalent in controller.hpp

[Tectu]

you can then call it directly with malloy::server::start(...). Really though I think we should add this to routing_context.hpp and the equivalent in controller.hpp

I really think so too :D You wanna PR that or shall I handle it?

Anyway - after workarounding that I'm back to the other problem which lead me to create this issue to check whether I'm doing things wrong: malloy::server::routing_context::session does not seem to be movable or copyable so I can't assign it to an std::optional in my own start(). I don't really need it to be copyable, but movable for sure.

Anyway, I don't mean to make this issue become a "noob support ticket" or something like that. I honestly didn't know about the weirdness going on with the start() private friend function. For the rest I just want to be sure that I'm not missing something obvious.

[0x00002a]

Are you using the latest commit? (or at least one since #108 was merged), it absolutely should be moveable since then (theres even a test case for it: https://github.com/Tectu/malloy/blob/17b285dafdbcd4b3487dce5378a1bafcef57657f/test/test_suites/components/controller.cpp#L25). As for adding the namespace stuff I will do that later today

[Tectu]

Yeah, this is on latest main branch:

controller
{
public:

void init(const config& cfg)
{
    try {
        m_malloy_controller = std::make_shared<malloy::server::routing_context>(cfg.malloy);
    }
    catch (const std::exception& e) {
        throw std::runtime_error("could not initialize malloy controller.");
    }
}

void start()
{
    m_malloy_session = std::move(malloy::server::start(std::move(*m_malloy_controller)));
}

void stop()
{
    m_malloy_session = std::nullopt;
}

private:
    std::shared_ptr<malloy::server::routing_context> m_malloy_controller;
    std::optional<malloy::server::routing_context::session> m_malloy_session;
};

Using gcc 10.2.0:

C:\Users\joel\Documents\projects\myapp\lib\myapp\server\controller.cpp: In member function 'void myapp::server::controller::start()':
C:\Users\joel\Documents\projects\myapp\lib\myapp\server\controller.cpp:88:88: error: use of deleted function 'std::optional<malloy::detail::controller_run_result<std::shared_ptr<malloy::server::listener> > >& std::optional<malloy::detail::controller_run_result<std::shared_ptr<malloy::server::listener> > >::operator=(std::optional<malloy::detail::controller_run_result<std::shared_ptr<malloy::server::listener> > >&&)'
   88 |     m_malloy_session = std::move(malloy::server::start(std::move(*m_malloy_controller)));
      |                                                                                        ^
In file included from C:\Users\joel\Documents\projects\myapp\lib\myapp\server\controller.hpp:5,
                 from C:\Users\joel\Documents\projects\myapp\lib\myapp\server\controller.cpp:1:
C:/msys64/mingw64/include/c++/11.2.0/optional:663:11: note: 'std::optional<malloy::detail::controller_run_result<std::shared_ptr<malloy::server::listener> > >& std::optional<malloy::detail::controller_run_result<std::shared_ptr<malloy::server::listener> > >::operator=(std::optional<malloy::detail::controller_run_result<std::shared_ptr<malloy::server::listener> > >&&)' is implicitly deleted because the default definition would be ill-formed:
  663 |     class optional
      |           ^~~~~~~~
C:/msys64/mingw64/include/c++/11.2.0/optional:663:11: error: use of deleted function 'std::_Enable_copy_move<false, false, true, false, _Tag>& std::_Enable_copy_move<false, false, true, false, _Tag>::operator=(std::_Enable_copy_move<false, false, true, false, _Tag>&&) [with _Tag = std::optional<malloy::detail::controller_run_result<std::shared_ptr<malloy::server::listener> > >]'
In file included from C:/msys64/mingw64/include/c++/11.2.0/optional:43,
                 from C:\Users\joel\Documents\projects\myapp\lib\myapp\server\controller.hpp:5,
                 from C:\Users\joel\Documents\projects\myapp\lib\myapp\server\controller.cpp:1:
C:/msys64/mingw64/include/c++/11.2.0/bits/enable_special_members.h:260:5: note: declared here
  260 |     operator=(_Enable_copy_move&&) noexcept                         = delete;
      |     ^~~~~~~~

As for adding the namespace stuff I will do that later today

Alright

[0x00002a]

That... is incredibly weird. When I try it with a basic example it works fine, but it does indeed fail for session, I have no idea why and it is certainly moveable. Anyway you can get around it with emplace(start(...)) instead