Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

default value of 0 for field is not part of enum #5108

Closed
zplzpl opened this issue Jan 8, 2019 · 5 comments
Closed

default value of 0 for field is not part of enum #5108

zplzpl opened this issue Jan 8, 2019 · 5 comments

Comments

@zplzpl
Copy link

zplzpl commented Jan 8, 2019

$ ./flatc.exe --version
flatc version 1.10.0 (Oct  3 2018 12:56:55)

Some sensitive data has been modified.

$ ./flatc.exe ./path/file.fbs --go
error: path\file.fbs(x, y): error: default value of 0 for field fields_merchant_update is not part of enum MerchantCanChanged
enum MerchantCanChanged : byte { x= 1, y, z, v, s}
table ProRequest{
	fields_merchant_update:[MerchantCanChanged];
}

On the flatc.exe 1.5.0 version, it works fine.
How should I fix it?

@vglavnyy
Copy link
Contributor

vglavnyy commented Jan 8, 2019

I have reproduced this error on the HEAD of the master.
@aardappel
Probably this error was introduced by (f431a96).
Before f431a96:

  if (type.enum_def && IsScalar(type.base_type) && !struct_def.fixed &&
      !type.enum_def->attributes.Lookup("bit_flags") &&
      !type.enum_def->ReverseLookup(
          static_cast<int>(StringToInt(field->value.constant.c_str()))))
    Warning("enum " + type.enum_def->name +
            " does not have a declaration for this field\'s default of " +
            field->value.constant);

Since f431a96 (idl_parser.cpp 656-663):

  if (type.enum_def &&
      !type.enum_def->is_union &&
      !type.enum_def->attributes.Lookup("bit_flags") &&
      !type.enum_def->ReverseLookup(static_cast<int>(
                                 StringToInt(field->value.constant.c_str())))) {
    return Error("default value of " + field->value.constant + " for field " +
                 name + " is not part of enum " + type.enum_def->name);
  }

Checking && IsScalar(type.base_type) && !struct_def.fixed check was removed by this commit.

@aardappel
Copy link
Collaborator

Hah, didn't spot that, those definitely should be in there. Either of you care to fix it?

@vglavnyy
Copy link
Contributor

@zplzpl Can you fix it and add a test for this case?

vglavnyy added a commit to vglavnyy/flatbuffers that referenced this issue Apr 3, 2019
vglavnyy added a commit to vglavnyy/flatbuffers that referenced this issue Apr 9, 2019
- hide the implementation of enums from code generators
- fix uint64 the issue in the cpp-generator
- fix google#5108
- fix issues in cpp code-generator
- new tests
- add monster_enum.fbs for checking of the cpp code generator
vglavnyy added a commit to vglavnyy/flatbuffers that referenced this issue Apr 10, 2019
- hide the implementation of enums from code generators
- fix uint64 the issue in the cpp-generator
- fix google#5108
- fix issues in cpp code-generator
- new tests
- add monster_enum.fbs for checking of the cpp code generator
vglavnyy added a commit to vglavnyy/flatbuffers that referenced this issue Apr 12, 2019
- hide the implementation of enums from code generators
- fix uint64 the issue in the cpp-generator
- fix google#5108
- fix issues in cpp code-generator
- new tests
- add monster_enum.fbs for checking of the cpp code generator
vglavnyy added a commit to vglavnyy/flatbuffers that referenced this issue Apr 12, 2019
- hide the implementation of enums from code generators
- fix uint64 the issue in the cpp-generator
- fix google#5108
- fix issues in cpp code-generator
- new tests
- add monster_enum.fbs for checking of the cpp code generator
- enums with bit_flags attribute should be unsigned
vglavnyy added a commit to vglavnyy/flatbuffers that referenced this issue Apr 12, 2019
- hide the implementation of enums from code generators
- fix uint64 the issue in the cpp-generator
- fix google#5108
- fix issues in cpp code-generator
- new tests
- enums with bit_flags attribute should be unsigned
vglavnyy added a commit to vglavnyy/flatbuffers that referenced this issue Apr 12, 2019
- hide the implementation of enums from code generators
- fix uint64 the issue in the cpp-generator
- fix google#5108
- fix issues in cpp code-generator
- new tests
- enums with bit_flags attribute should be unsigned
vglavnyy added a commit to vglavnyy/flatbuffers that referenced this issue Apr 27, 2019
- hide the implementation of enums from code generators
- fix uint64 the issue in the cpp-generator
- fix google#5108
- new tests
- enums with bit_flags attribute should be unsigned
vglavnyy added a commit to vglavnyy/flatbuffers that referenced this issue Apr 27, 2019
- hide the implementation of enums from code generators
- fix uint64 the issue in the cpp-generator
- fix google#5108
- new tests
- enums with bit_flags attribute should be unsigned
vglavnyy added a commit to vglavnyy/flatbuffers that referenced this issue Apr 27, 2019
- hide the implementation of enums from code generators
- fix uint64 the issue in the cpp-generator
- fix google#5108
- new tests
- enums with bit_flags attribute should be unsigned
vglavnyy added a commit to vglavnyy/flatbuffers that referenced this issue Apr 27, 2019
- hide the implementation of enums from code generators
- fix uint64 the issue in the cpp-generator
- fix google#5108
- new tests
- enums with bit_flags attribute should be unsigned
@precisionconage
Copy link

precisionconage commented Oct 15, 2019

I can still reproduce this with latest master, e.g. by compiling the following:

enum TestEnum:ubyte {
    Test1 = 1
}

table Test {
    t:TestEnum;
}

root_type Test;

Edit: Never mind, this isn't quite the same issue. It looks like a default must always be provided for fields with an enum type where the enum doesn't contain 0. An attribute to specify a default for that case might be useful, but I'm not sure how much work that would be to implement.

@vglavnyy
Copy link
Contributor

@precisionconage you are right.
Implicit "NONE" field added only for empty enum declarations, like enum TestEnum {}.
I will add tests and fix this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants