search

pytroll / aggdraw

Python package wrapping AGG2 drawing functionality
https://aggdraw.readthedocs.io/en/latest/
Other
98 stars 47 forks source link

Can implicit fallthroughs be made explicit? #97

Closed r-barnes closed 1 day ago

r-barnes commented 1 month ago

Compiling the code with clang's -Wimplicit-fallthrough reveals ~5 cases where fallthroughs are not explicitly marked (example). Are these bugs where break should have been used or are they intentional? If they are intentional, can they be marked with [[fallthrough]]?

djhoese commented 1 month ago

My guess is a lot of the code identified is from the upstream agg C++ library. We (aggdraw) simply vendor this code in our repository, but are not the creators or even maintainers of that code. I would recommend finding the upstream project and seeing if these cases have been changed since the version we're using here.

As far as cleaning things up in this repository, I'd prefer to limit modifications to the agg C++ library for the reasons I mentioned above and that it makes it harder to update the library in the future. If they are actual bugs though that's a different story.

r-barnes commented 1 month ago

I've opened an upstream issue here: https://github.com/ghaerr/agg-2.6/issues/12

djhoese commented 1 month ago

I think the official project is on sourceforge although not very active. That github repository is likely not the official repository of the project.

r-barnes commented 1 month ago

@djhoese - Ah, I hadn't found that, thanks. You're right: there are bugfix patches on sourceforge that are more than a year old. It looks as though several downstream forks have been adding bugfixes without the ability to upstream.

djhoese commented 1 day ago

I'm going to close this for now. I have added a patches/ directory in the maint/1.3 branch if you wanted to make a PR to address these you could put fixes there, but I think I would prefer that we don't change them. This isn't something we/aggdraw should be fixing and any fix for them could cause unexpected consequences/differences with the upstream library and make it harder to transition to new versions in the future.