multiscale / muscle3

The third major version of the MUltiScale Coupling Library and Environment
Apache License 2.0
25 stars 13 forks source link

Compiler error when constructing Data::list with Data elements #186

Closed maarten-ic closed 1 year ago

maarten-ic commented 1 year ago

Attempting to create a Data::list with Data elements:

auto this_works = Data::list(1, 2, 3);
auto this_does_not = Data::list(Data(1), 2, 3);

Gives the following compiler error:

/home/maarten/projects/muscle3/libmuscle/cpp/build/msgpack/msgpack/include/msgpack/v1/object.hpp:658:7: error: ‘const class libmuscle::impl::Data’ has no member named ‘msgpack_object’
  658 |     v.msgpack_object(static_cast<msgpack::object*>(&o), o.zone);
      |     ~~^~~~~~~~~~~~~~

Note: my actual use case is slightly more complicated, but this simple example illustrates the problem well.

Workaround:

// auto list = Data::dict(Data::grid("a", 1), Data::dict("b", 2));

auto list = Data::nils(2);
list[0] = Data::dict("a", 1);
list[1] = Data::dict("b", 2);
LourensVeen commented 1 year ago

I'm having some trouble reproducing this with 0.6.0:

$ cat test.cpp
#include <libmuscle/libmuscle.hpp>

#include <iostream>
#include <ostream>

using libmuscle::Data;

int main() {
    auto list = Data::list(Data(1), 2, 3);
    std::cout << list[0].as<int>() << std::endl;
}

$ g++ -o test $(pkg-config --cflags ymmsl libmuscle) test.cpp $(pkg-config --libs ymmsl libmuscle)

$ ./test
1

It seems to work also on clang++ or icpx.

The line

// auto list = Data::list(Data::grid("a", 1), Data::grid("b", 2));

won't work anyway as Data::grid() doesn't take those arguments.

So I'm a little confused, can you post a complete failing example?

maarten-ic commented 1 year ago

Using the public API works fine indeed! I did this inside a unit test, but it only compiles when I include both <libmuscle/data.hpp> and <libmuscle/mcp/data_pack.hpp>. Just the data.hpp file was not sufficient apparently.

Closing this issue.

won't work anyway as Data::grid() doesn't take those arguments.

That should have been Data::dict(), sorry. Updated above as well.

LourensVeen commented 1 year ago

Ah, yes, that is correct. I think there's a reason it's in a separate header, some kind of circular dependency IIRC. data_pack.hpp specialises some templates, so it doesn't give very good error messages, but it's how msgpack works so there's nothing we can do about it. Glad it's solved!