freak82 opened a new issue, #11377: URL: https://github.com/apache/trafficserver/issues/11377
Hi there, So, I did the above experiment for myself and there are lots of places which need to be fixed. However, the warnings also revealed few places with potential issues/bugs and I've opened issues here about them. You can find them by my user, I guess. Everything else seems to be purely mechanical work and I can do it if there is a desire for this. I just believe that warning suppression is bad because the warnings sometimes point to real issues/bugs and we should use any help from the compiler and the other tools to catch bugs as early as possible (preferably at compile time, instead at runtime). If there is a desire for this it needs to be decided how you want the changes. I mean that in the project currently there are 5 types of suppressing this warning: 1. `ATS_UNUSED` after the parameter name Drawbacks of this solution: - `tscore/ink_defs.h` needs to be included in order of the above macro to be visible - it's easy to forget to remove this tag if the parameter becomes used in the future and this potentially may confuse some readers of the code 2. `/* <parameter-name> ATS_UNUSED */` or just `/* <parameter-name> */ Drawbacks of this solution: - the solution which includes `ATS_UNUSED` in the commented part become a bit longer and too much (IMO) but is probably better for the readers? 3. `[[maybe_unused]]` Drawbacks of this solution: - it's easy to forget to remove this attribute if the parameter becomes used in the future and this potentially may confuse some readers of the code 4. `(void)<parameter-name>` at the beginning of the function Drawbacks of this solution: - it's easy to forget to remove this suppression if the parameter becomes used in the future and the function is longer and this potentially may confuse some readers of the code - the unused parameters are not visible from the function signature - this way of changing will require more work manual work compared to some of the above solutions. 5. Just removing the name of the parameter Drawbacks of this solution: - it becomes unclear in some cases what this parameter is supposed to be Note that all of the above are currently used in the ATS code as far as I checked. The 5-th way is probably the least used but there are few places still. I'd personally go with 1, 2 or 3 and I'm leaning towards 2. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@trafficserver.apache.org.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
