BINLOG_ADAPT_ENUM fails for nested enums

[andrewkcorcoran]

namespace ns {
   struct Test
   {
      enum class Enum
      {
         value
      } Enum;
   };
} // namespace ns
BINLOG_ADAPT_ENUM(ns::Test::Enum)

results in template argument 1 is invalid at include/binlog/adapt_enum.hpp:4 changing it to BINLOG_ADAPT_ENUM(enum ns::Test::Enum) fixes the error in clang but results in declaration enum class ns::Test::Enum does not declare anything error on gcc at include/mserialize/make_struct_serializable.hpp:7

[erenon]

Hi, the issue is that Enum is both the type name and the field name, making the ns::Test::Enum expression reference the field, not the type. Is it possible to make these two different?

[andrewkcorcoran]

Yea so this disambiguation can be avoided in the general case by prefacing the reference with the keyword 'enum', this solution works for me for all compilers. However this doesn't work in combination with the binlog macro using GCC (but does with clang). Do you think it would be possible to preface enum usages in the macro with the keyword enum and solve this without user changed required?

…

On Wed, 23 Jun 2021, 11:37 am Thaler Benedek, ***@***.***> wrote: Hi, the issue is that Enum is both the type name and the field name, making the ns::Test::Enum expression reference the field, not the type. Is it possible to make these two different? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#124 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AL5LS3F4R4DLWYYNEWLK6P3TUGFNPANCNFSM47FEG6BQ> .

[erenon]

Let's see if CI likes this: #125

[erenon]

It did. Please reopen if it doesn't fix your issue

[andrewkcorcoran]

Just realised this change may break old c style enums :/

typedef enum {
   field1
} name;

[erenon]

Unfortunate. In C++20, using enum works. I don't see a way to solve this with a single macro.

[erenon]

I don't have a solution that works for both enums shadowed by values (Enum in the OP) and typedefs of enums, on clang, msvc and gcc. The closest I got is this:
https://github.com/morganstanley/binlog/pull/126/checks?check_run_id=2926324358
that works for every case even on GCC9, but fails on clang and msvc, and I suspect it is ill-formed code (see https://stackoverflow.com/questions/40523513/typename-type-members-and-non-type-members-is-it-valid-code)

I can recommend this workaround:

namespace ns {
   struct Test
   {
      enum class Enum
      {
         value
      } Enum;
      using EnumT = enum Enum;  // workaround
   };
} // namespace ns
BINLOG_ADAPT_ENUM(ns::Test::EnumT)

what do you think?

[andrewkcorcoran]

That doesn't seem too onerous, but won't work for adapting code that's not owned (unmodifiable) by the user.

[erenon]

What about this?

// vendor code, unmodified
namespace ns {
   struct Test
   {
      enum class Enum
      {
         value
      } Enum;
      using EnumT = enum Enum;  // workaround
   };
} // namespace ns

// user code
namespace ns {
      using TestEnum = enum Test::Enum;  // workaround
   };
} // namespace ns
BINLOG_ADAPT_ENUM(ns::TestEnum)

(Or do not even re-open the vendors namespace, just use NsTestEnum)

Ideal would be to make BINLOG_ADAPT_ENUM(enum ns::Test::Enum) work, but I couldn't figure out how, without adding yet another macro, and keeping typedefs supported.

[andrewkcorcoran]

Yea that should solve the issue of thirdparty code. It's not ideal but as long as we add a limitations section to the documentation with details of how to workaround it I think that acceptable.

[erenon]

Done in #128