estan added a comment. In D88833#3225842 <https://reviews.llvm.org/D88833#3225842>, @aaron.ballman wrote:
> In D88833#3224389 <https://reviews.llvm.org/D88833#3224389>, @carlosgalvezp > wrote: > >> Please note: I have a patch that disables warnings from system macros for >> all clang-tidy warnings, it would be good to review it so we don't need to >> implement the same mechanism in all 400+ checks :) >> https://reviews.llvm.org/D116378 > > Thank you for pointing that out; that's a far better approach. > > In D88833#3224799 <https://reviews.llvm.org/D88833#3224799>, @estan wrote: > >> In D88833#3222829 <https://reviews.llvm.org/D88833#3222829>, @estan wrote: >> >>> Sounds good @aaron.ballman, let's wait for @fiesh. >>> >>> Though I realize now that the scope of this patch is probably not enough to >>> solve a problem we have in our code base. The check will warn about (for >>> example) things like this: >>> >>> In a third party lib outside our control: >>> >>> void f(double out[3]); >>> >>> In our code: >>> >>> double out[3]; >>> f(out); >>> >>> Include paths for the third party lib are added with -isystem. >>> >>> Am I right that we're still going to get warnings for this with this patch? >>> >>> Full disclosure. The third party lib is VTK, which is littered with APIs >>> like these. > > Yes, you will still get warnings with that code. > >> @aaron.ballman BTW, would you guys be open to a patch that makes the check >> not warn in cases like the above, or at least an option to make it not warn? >> >> We like the spirit of this check, to make sure we use >> gsl::array_view/std::span instead of (T *data, int size)/(T*) style >> interfaces in our own APIs. But we cannot change the (T*) APIs of our third >> party libraries. Having to litter our code with hundreds of casts is not >> very nice. It would be great if there was an option to tell the check to >> ignore all decays that occur in calls to functions in system headers. > > You can't change the 3rd party APIs because they're system headers, but you > can add a cast to the call site to disable the diagnostic. Does that suffice > to meet your needs, or do you still think an option is a better approach? > https://godbolt.org/z/q41Wdn3xa Well it's not just one call site, but a whole bunch of them, and having to add casts at all these places is a bit cumbersome. I think an option to only warn for cases where you can meaningfully improve the code (by changing the API to use std::span / gsl::array_view) would be good. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88833/new/ https://reviews.llvm.org/D88833 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
