Enums are not defined as macros

[artman41]

During development using a shared proto, if a proto Enum is renamed but index is the same as the previous version of that enum compilation will not current fail.

If a message has a field renamed, you would notice as the compilation would fail due to a record field no longer existing.

Ideally, the atoms generated by the Enum definitions would be implemented as -define(ENUM, 'ENUM') so that compilation would fail if the field was renamed.

Would it be possible to have this implemented?

[tomas-abrahamsson]

I understand the issue you are describing, with enum symbols that can change, and I do sympathize with catching stuff as early as possible. Before going into details, I suppose you know about the verify option and about types for enum-fields? Agreed, it would be at run-time and dialyzer-time respectively, thus later than compile-time.

I'm thinking about what implications your suggestion would bring, and have arrived at the following, and wanted to ask what your thoughts are around this.

One thought is that the enum macros should probably be namespaced. Suppose we have a proto that has changed like this:

message A {
  enum B1 { FALSE = 0; TRUE = 1; }
  enum B2 { NO = 0; YES = 1; }  // was: enum B2 { NO = 0; TRUE = 1; }
  message S {
    B1 b  = 1;
    B2 yn = 2;
  }
}

Suppose you have code like #{yn => ?'TRUE'} but the proto has changed so it should now be #{yn => ?'YES'}. With just -define(TRUE, 'TRUE'). and -define(YES, 'YES'). you would still not get a compilation error for this change. Something like -define('A.B2.TRUE', 'TRUE'). changing into -define('A.B2.YES', 'YES'). would be needed to catch this.

Another thought is that the defines might need some prefix to make sure they don't collide with other macros.

Yet another thought is that Gpb would need to generate hrl files even with the -maps option.

[artman41]

Thanks for responding quick on this one @tomas-abrahamsson

I think you're right about the issue you've pointed out, we've looked at the potential of using the verify option but for now we've settled on using eunit tests as a half-way fix; eunit tests will decode a proto-blob which only has the enum defined but asserts that it has the expected value when decoded.

I think what you're suggesting in terms of the prefix would work given the issue - something else we've seen that I don't know if it would impact this is the circumstance of if the enum is defined in a separate package and then imported to be used in a message in a separate packet.

i.e

package my_enums;

enum B1 { FALSE = 0; TRUE = 1; }
enum B2 { NO = 0; YES = 1; }  // was: enum B2 { NO = 0; TRUE = 1; }

package my_message;

import "my_enums.proto";

message A {
  message S {
    my_enums.B1 b  = 1;
    my_enums.B2 yn = 2;
  }
}

I imagine that if you use the package as part of the prefix it would solve this edge-case but you'd likely be the best to advise

What you've described does sound like an ideal solution for what we're attempting to achieve though 👍

[tomas-abrahamsson]

Yes, using the package name as part of the prefix is the way to go, I think. There's a use_packages option and treating this as part of enum macros name would fit nicely.

I'm thinking the whole feature would need some option to enable it, because it could otherwise come as a surprise for maps option users if gpb would start to generate hrl files.

[tomas-abrahamsson]

You might want to try out the enum-macros branch to see if it works the way you imagined?

[artman41]

@tomas-abrahamsson looks perfect 👍 only feedback I've heard about it is that an option to uppercase the whole macro would be cool :)

otherwise I think it solves the problem perfectly :D

[tomas-abrahamsson]

I pushed an update of the branch, where I extended the {rename,{What,How}} option. I added enum_macro and uppercase, to it, so it is now possible to specify {rename,{enum_macro,uppercase}}, and I think it should do the trick.

For more info on the rename option, see the https://hexdocs.pm/gpb/gpb_compile.html#option-rename. Note that this documents the option on the master branch, so it doesn't include neither enum_macro nor uppercase just yet.

I dropped the enum_macro_prefix and enum_macro_suffix options that I introduced in the previous version of the enum-macros branch. I don't know if you had seen them or not. The {rename,{What,How}} has prefix and suffix sub-options that can be used instead.

[tomas-abrahamsson]

I included this in 4.20.0 which is just released, so I'm closing this. If you would find anything more, please always feel free to open or re-open.