aaron.ballman added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp:98 + "::access;" + "::bind;" + "::connect;" ---------------- sammccall wrote: > aaron.ballman wrote: > > jranieri-grammatech wrote: > > > alexfh wrote: > > > > bind has a side effect and returns a success status. Thus, the result > > > > being unused isn't necessarily a bug. Same for `connect`. And probably > > > > for `setjmp` as well. > > > In terms of bind, connect, and setjmp: while I personally would say that > > > code not using the return value is bugprone, the data suggests that the > > > vast majority of developers are using these functions in the intended > > > manner and the false-positive rate should be low. > > I think we have sufficient statistical data to suggest that these APIs > > should be on the list because the majority of programmers *do not* use them > > solely for side effects without using the return value, so my preference is > > to keep them in the list. > I stumbled upon this review as we're considering turning this check on by > default in clangd. > > There's a significant difference between unused std::async() (programmer > misunderstood contract) and unused ::connect() (ignoring error conditions). > The former is ~never noise, and the latter may be (e.g. in experimental or > incomplete code). > > So there's some value in separating these two lists out either as an option > or a separate named check (bugprone-unhandled-error?) I think we probably > wouldn't enable this check by default if it includes the error-code functions. > > > the majority of programmers *do not* use them solely for side effects > ...in popular, distributed software :-) > So there's some value in separating these two lists out either as an option > or a separate named check (bugprone-unhandled-error?) I think we probably > wouldn't enable this check by default if it includes the error-code functions. I think that adds complexity to this check when the complexity isn't necessary. clang-tidy has traditionally been a place for checks that are chattier than what the compiler should provide, and this check has a trivial, well-understood mechanism to silence the diagnostics (cast to void) which also expresses intent properly to the toolchain. >>the majority of programmers *do not* use them solely for side effects > ...in popular, distributed software :-) I have not seen anyone provide data to suggest that the functions in question appear in any statistically significant amount in practice without checking the return value, just worries that they *could*. I don't think that's compelling in the face of data. Remember, this is for bugprone patterns, not bugknown patterns. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76083/new/ https://reviews.llvm.org/D76083 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
