ahrefs / atd

Static types for JSON APIs
Other
308 stars 53 forks source link

atdcpp: new backend #404

Closed elrandar closed 1 month ago

elrandar commented 3 months ago

New atd backend for C++

elrandar commented 2 months ago

issue where if you define multiple types with generics, that depend on each other, the order of c++ declarations will not necessarily be correct, and will prevent compilation unless header is manually edited

This issue is related to the way of how atd will sort type declarations. This problem arises because there is a circular dependancy, but one order still works, since C++ only need to know the memory taken by the type (full definition) if it occurs in a struct, without being wrapped in a pointer or a list.

Exemple of that issue: test.atd

type field =
  {
    field : string;
  }

type 'a combinator =
  {
    integer : int;
    list_of_things : 'a list;
  }

type 'a query' =
  [ Elt of 'a
  | Or of 'a query' combinator
  ]

type fq = field query'

test.hpp

namespace FieldQuery {
    namespace Types {
        // Original type: _field_query = [ ... | Elt of ... | ... ]
        struct Elt
        {
            typedefs::Field value;
            static void to_json(const Elt &e, rapidjson::Writer<rapidjson::StringBuffer> &writer);
        };
        // Original type: _field_query = [ ... | Or of ... | ... ]
        struct Or
        {
            typedefs::FieldQueryCombinator value;
            static void to_json(const Or &e, rapidjson::Writer<rapidjson::StringBuffer> &writer);
        };
    }

    typedefs::FieldQuery from_json(const rapidjson::Value &x);
    // ...
}

struct FieldQueryCombinator {
    int integer;
    std::vector<typedefs::FieldQuery> list_of_things;

    static FieldQueryCombinator from_json(const rapidjson::Value & doc);
    // ...
};

As you can see, defining FieldQueryCombinator before FieldQuerywould solve the issue. In the cases where that is happening, I fixed by just wrapping with a pointer in c++, because otherwise I think it would require too many changes on the frontend. Maybe I should open an issue for that, but not sure if it applies to any other languages.

elrandar commented 2 months ago

I think this PR is now ready for review. I'm quite happy with the code produced, especially compared to the dlang backend. The separation between interface and implementation really helps when actually using the types. The main problem that still needs to be addressed is how to cleanly integrate rapidjson to the project, since it is needed to run and compile c++ code.

I think the best way would be to add the install step to the CI, but I'm not sure about it. If you could tell me which way is preferred @mjambon @Khady.

mjambon commented 2 months ago

The main problem that still needs to be addressed is how to cleanly integrate rapidjson to the project, since it is needed to run and compile c++ code.

I assume we don't need rapidjson to compile the OCaml program atdcpp. Rapidjson is a runtime library needed for testing atdcpp, so I suggest installing the dependency from a reliable source from the script ~/atd/.circleci/setup-system. This script will also be used by other developers of the ATD project to set things up without being familiar with all the target ecosystems.

mjambon commented 2 months ago

I didn't say it but having C++ support is a massive achievement. Thank you! You should make a big announcement once it's in a usable state (I suggest as soon as it's available in opam or earlier).

elrandar commented 1 month ago

I made the necessary changes, I think it should be mergeable as is.