getml / reflect-cpp

A C++20 library for fast serialization, deserialization and validation using reflection. Supports JSON, BSON, CBOR, flexbuffers, msgpack, TOML, XML, YAML / msgpack.org[C++20]
https://getml.github.io/reflect-cpp/
MIT License
901 stars 76 forks source link

Serialize deep class fails at compilation #145

Closed Alecsou closed 1 month ago

Alecsou commented 1 month ago

Expected Behavior

No compilation error

Current Behavior

error C1054: compiler limit: initializers nested too deeply

How to Reproduce

main.cpp

#include "DeepClass.h"
#include "rfl.hpp"
#include "rfl/json.hpp"
int main()
{
    DeepClass dc{};

    const std::string json_string = rfl::json::write(dc);
}

DeepClass.h

#ifndef _DEEPCLASSH_
#define _DEEPCLASSH_

#include "LowHeightClass.h"

class DeepClass
{
public:
    LowHeightClass LHC;

    int dummy1;
    int dummy2;
    double dummy3;
};

#endif

LowHeightClass.h

#ifndef _LOWHEIGHTCLASS_
#define _LOWHEIGHTCLASS_

/*****************/
/* LowHeightClassSub */
/*****************/

class LowHeightClassSub
{
public:
    double dummy1 = 0;                          
    double dummy2 = 0;                          
    double dummy3 = 0;                      
    double dummy4 = 0;                      
    double dummy5 = 0;                      
    double dummy6 = 0;                      
    double dummy7 = 0;                  
    double dummy8 = 0;                  
    double dummy9 = 0;                  
    double dummy10 = 0;                     
    double dummy11 = 0;                 
    double dummy12 = 0;     
    double dummy13 = 0;         
    double dummy14 = 0;         
    double dummy15 = 0;         
    double dummy16 = 0;     
    double dummy17 = 0; 
    double dummy18 = 0;         
    double dummy19 = 0;         

    // Extra channels
    double dummy20 = 0; 
    double dummy21 = 0;                 
};

/***********/
/* LowHeightClass */
/***********/

class LowHeightClass
{
public:
    double dummy1 = 0;              
    double dummy2 = 0;  
    double dummy3 = 0;

    double dummy4 = 0;              
    double dummy5 = 0;  
    double dummy6 = 0;

    double dummy7 = 0;      
    double dummy8 = 0;      

    double dummy9 = 0;
    double dummy10 = 0;

    double dummy11 = 0;     
    double dummy12 = 0;
    double dummy13 = 0;
    double dummy14 = 0;

    LowHeightClassSub dummy15{};
    LowHeightClassSub dummy16{};
    LowHeightClassSub dummy17{};
    LowHeightClassSub dummy18{};

};

#endif

The error is raised in rfl\internal\get_field_names.hpp(142,39).

I was expecting it to work since the data structure is not that deep (in this example, 3 classes deep) but still doesn't compile.

Context (Environment)

Visual Studio 2022 (v143) 17.10.0 Windows 10.0 C++20

liuzicheng1987 commented 1 month ago

Thank you for reporting this. I will take a Look.

My suspicion is that's it isn't the the depth of the struct but the total number of fields that's the problem here. It may be due to std::tuple. I will take a closer look and let you know.

liuzicheng1987 commented 1 month ago

Hi @Alecsou , I was able to reproduce the problem. It appears to be an MSVC-only problem, though, all other compilers work fine:

https://github.com/getml/reflect-cpp/pull/147

I will try to find a fix for this.

Alecsou commented 1 month ago

Hi, quick update :

This problem is a more broad one, the library fails to deal with "big" classes in multiple cases with always the same error. I for example added an operator using the library in a class with a lot (30~ double) of fields and it failed compilation also.

double operator [] (std::string field) {
    std::map m = std::map < std::string, double>();
    const auto view = rfl::to_view(*this);
    view.apply([](const auto& f) {
        if (f.name() == field) {
            return (double)f.value();
        }
        });
}

This makes the library completely unusable under MSVC right now... Hope this can be solved, as this library seems really promising to say the least !

liuzicheng1987 commented 1 month ago

Yes, that is what I thought. It's not the depth, it's the number of fields.

I suspect that the issue is std::tuple and I am afraid that I might have to implement my own version. I am on this.

RampantDespair commented 1 month ago

I was about to create an issue, but I am facing the same one as described here. Hope this gets resolved soon, as I wanted to implement reflect-cpp into my project.

liuzicheng1987 commented 1 month ago

@RampantDespair , I am on it.

I have conducted a couple of experiments. I am afraid the issue is with the way std::tuple is implemented in the standard library in combination with MSVC compiler restrictions and I will probably have to implement my own version. It will take me a couple of days, but I think I might get this resolved by next week.

liuzicheng1987 commented 1 month ago

@Alecsou and @RampantDespair , good news: My initial hypothesis turned out to be true. It was indeed std::tuple that was the problem here. I have now found a tuple-free way of getting the field names out of the structs:

https://github.com/getml/reflect-cpp/pull/160

As you can see, the test compiles even on MSVC. This still needs some code beautification, but the proof-of-concept is there. I am confident I will be able to merge this over the next couple of days.

RampantDespair commented 1 month ago

@liuzicheng1987 That's great news! I'm eager to try it it out

liuzicheng1987 commented 1 month ago

@RampantDespair and @Alecsou , the issue is resolved: 9c9eceab929a65aa9a7c846940ba60675fd243b5