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

Conflicting Enums in global scope #290

Open
vbartusevicius opened this issue Jan 25, 2024 · 7 comments
Open

Conflicting Enums in global scope #290

vbartusevicius opened this issue Jan 25, 2024 · 7 comments
Assignees

Comments

@vbartusevicius
Copy link

It is impossible to use s00500/ESPUI and dirkhillbrecht/UiUiUi in a single project because both define global constants.

.pio/libdeps/nodemcu/UiUiUi/src/UIEnums.h:12:20: error: 'None' conflicts with a previous declaration
   12 | enum UIExpansion { None,Horizontal,Vertical,Both };
      |                   ^~~~
In file included from .pio/libdeps/nodemcu/ESPUI/src/ESPUI.h:22,
                 from lib/WebAdmin/WebAdmin.h:7,
                 from src/main.cpp:7:
@iangray001
Copy link
Collaborator

Yeah, C is kinda dumb with this. There are of course ways it can be fixed but it would be a huge breaking change. @MartinMueller2003 is there a way to switch a project to scoped enums without breaking downstream projects?

@MartinMueller2003
Copy link
Collaborator

Not really. You would want to push the enums into a class to make them class local. I suspect now one is using the correct annotation EnumName::Value and that is why there is a conflict. The same can be said for the other application. They have put the enums in the global scope instead of in a class public scope and that causes issue like this.

If we do this then people will have a breaking issue but the fix is simple. Add

ContainingClass::EnumName:: in front of the enum value usage. That is not a big change (ie PlatformIO can do a show all references making it easy to change). Additionally, Instances in the code will get quickly highlighted by compilation failures.

@vbartusevicius
Copy link
Author

Added same issue to the other repo - dirkhillbrecht/UiUiUi#14

@MartinMueller2003
Copy link
Collaborator

Any decision here? I would go for the breaking change because it is the right way to do this.

@s00500
Copy link
Owner

s00500 commented Feb 26, 2024

Hm... pretty frustrating... do we have any chances of adding at least editor highlights for the break ? otherwise we could do a version bump and add a little migration guide for this...

@MartinMueller2003
Copy link
Collaborator

I would say we bump and create an upgrade guide. I would also suggest that a lot of other global structures currently in place should become private with accessors created as needed.

@s00500
Copy link
Owner

s00500 commented Mar 1, 2024

I think I would be ok with that if you start working on a PR :-)

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

No branches or pull requests

4 participants