microsoft / yardl

Tooling for streaming instrument data
https://microsoft.github.io/yardl/
MIT License
29 stars 5 forks source link

C++ date types depend on C++ standard #160

Closed naegelejd closed 2 days ago

naegelejd commented 3 days ago

Yardl defines the yardl::Date type differently depending on the C++ standard used for compilation (std::chrono::local_days for C++20 and newer, date::local_days otherwise).

Yardl officially supports C++17 and C++20, but code generated from the same model and compiled separately for each standard is not compatible.

This becomes a problem when publishing a shared library for a Yardl model. Which C++ standard should be used to build the shared library? If we build and publish using C++20 or newer, users need to compile their client code with C++20 or newer. Likewise for C++17 or older.

naegelejd commented 3 days ago

Two potential solutions:

  1. Publish two versions of your shared library, one for C++17 and older, the other for C++20 and newer. This is a terrible idea.
  2. Change Yardl to always use HowardHinnant date.h, even if C++20 library features are available. It is already a required dependency in Yardl anyway.
naegelejd commented 3 days ago

For more context, the two underlying types std::chrono::local_days and date::local_days are actually compatible in-memory, so in many cases, there's no issue implicitly "casting" them if mixing code compiled with C++17 with code compiled with C++20.

Issues do manifest, however, when using a std::optional<Date> (and possibly others like std::variant):

Yardl model:

RecordWithOptionalDate: !record
  fields:
    dateField: date?

ProtocolWithOptionalDate: !protocol
  sequence:
    record: RecordWithOptionalDate?

C++ client executable:

#include "generated/binary/protocols.h"

int main(void) {
  test_model::binary::ProtocolWithOptionalDateWriter w("test.bin");
  test_model::RecordWithOptionalDate rec1;
  w.WriteRecord(rec1);
  w.Close();

  test_model::binary::ProtocolWithOptionalDateReader r("test.bin");
  std::optional<test_model::RecordWithOptionalDate> rec2_opt;
  r.ReadRecord(rec2_opt);
  r.Close();
  if (!rec2_opt.has_value()) {
    throw std::runtime_error("ERROR: Expected to read a valid record");
  }
  return 0;
}

The serialized record is valid, but when it is deserialized, it is a std::nullopt!!