dblaikie added a comment. In D102122#2748206 <https://reviews.llvm.org/D102122#2748206>, @aaron.ballman wrote:
> Let me start off by saying: thanks, I think this is really useful > functionality. As a ridiculously obvious example, Annex K has an integer type > alias `errno_t` and it would incredibly handy to be able to mark that as > `[[nodiscard]]` to strongly encourage checking for errors for functions that > return that type (not that we support Annex K, but the general idea extends > to any integer-based error type). > > That said, there are multiple different ways to spell the same semantic > attribute, and it's not clear to me that we want to change them all the same > way. > > `[[clang::warn_unused_result]]` is ours to do with as we please, so there's > no issue changing that behavior. > > `__attribute__((warn_unused_result))` and `[[gnu::warn_unused_result]]` are > governed by GCC and we try to be compatible with their behavior. GCC does not > appear to support the attribute on typedefs from my testing, so we'd want to > coordinate with the GCC folks to see if there's an appetite for the change. FWIW, looks like we already diverge from GCC's behavior - GCC doesn't seem to support this attribute on types at all: https://godbolt.org/z/8YjqnE4cv (but does support [[nodiscard]] in that place) > `[[nodiscard]]` is governed by both the C++ and C standards and we should not > be changing its behavior without some extra diagnostics about extensions > (and, preferably, some sort of attempt to standardize the behavior with > WG14/WG21). Might be a compatible extension, though - to use a standard attribute in a non-standard context? (at least clang and gcc only warn on putting [[nodiscard]] in non-standard places, they don't error) > Do you have an appetite to talk to the GCC and standards folks? Medium interest. > If not, then I think the safest bet is to only change the behavior for the > `[[clang::warn_unused_result]]` and to leave the other forms alone for now. Happy to go either way. Is there an easy/recommended way to split these things up? Duplicate the records in the td/come up with separate names, etc?) > In D102122#2746426 <https://reviews.llvm.org/D102122#2746426>, @dblaikie > wrote: > >> Fixes for a few other test cases (though I wonder if these tests are >> overconstrained - do we need to be testing the list of things this attribute >> can be applied to in so many places?) > > If the semantics of the attribute are identical regardless of how it's > spelled, then we probably don't need the tests all spread out in multiple > files. However, I don't think there's an intention to keep all of the > spellings with identical semantics, so the different tests might still make > sense (but could perhaps be cleaned up). > > In D102122#2746424 <https://reviews.llvm.org/D102122#2746424>, @dblaikie > wrote: > >> Oh, one question: If we do move forward with this patch, how would we detect >> that the compiler supports this new use of warn_unused_result? (so that the >> feature detection macros in LLVM properly make the attribute a no-op unless >> we have a version of clang that supports it) > > `__has_[c|cpp]_attribute` returns a value, so we'd likely wind up using that > return value to distinguish between versions. Hmm - what if we end up with different behavior for the different spellings of the attribute (as GCC does)? Can we check them separately? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102122/new/ https://reviews.llvm.org/D102122 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
