KarthikRIyer / swiftplot

Swift library for Data Visualization :bar_chart:
Apache License 2.0
398 stars 36 forks source link

Remove [-Wswitch] warning during import #91

Closed boronhub closed 4 years ago

boronhub commented 4 years ago

A small commit, but my first to this project nevertheless !

Removed redundant end_of_markers case declaration in AGG/include/agg_renderer_markers.h, which gets rid of the following warning during import :

warning: enumeration value 'end_of_markers' not handled in switch [-Wswitch]
            switch(type)
                   ^
KarthikRIyer commented 4 years ago

@boronhub the code definitely compiles on removing end_of_markers. From what I can make out it is there to make the code more readable. I would prefer not to make too many changes unless its very necessary. Just having a default: case in switch should suppress the warning. I wouldn't remove anything from the enum.

karwa commented 4 years ago

The better solution would be to use an updated version of AGG. We're only using 2.4, which produces a lot of warnings that appear to be silenced by 2.6 (e.g. this issue is fixed here by adding the end_of_markers case).

FWIW, it doesn't seem that AGG will ever get any releases beyond 2.6, for rather tragic reasons. We might as well update to the final version of AGG.

KarthikRIyer commented 4 years ago

I used 2.4 because matplotlib uses that version and also I was having some trouble building with other versions that time. 2.6 doesn't seem to be an official release but it does seem to fix the warnings. When I tried building I got no warnings related to AGG.

93

Although the tests seem to fail with the update.

boronhub commented 4 years ago

@boronhub the code definitely compiles on removing end_of_markers. From what I can make out it is there to make the code more readable. I would prefer not to make too many changes unless its very necessary. Just having a default: case in switch should suppress the warning. I wouldn't remove anything from the enum.

default isn't suppressing the warning on it's own, I tried.