[PATCH] D153163: Add __attribute__((warn_unused_result)) to LLVMErrorRef

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/include/llvm-c/ExternC.h:47-51
+#if LLVM_HAS_ATTRIBUTE(warn_unused_result) && defined(__clang__)
+#define LLVM_NODISCARD_GNU_STYLE_CLANG __attribute__((warn_unused_result))
+#else
+#define LLVM_NODISCARD_GNU_STYLE_CLANG
+#endif
----------------
aaron.ballman wrote:
> dblaikie wrote:
> > aaron.ballman wrote:
> > > dblaikie wrote:
> > > > aaron.ballman wrote:
> > > > > Hmm, worth it to add `__has_c_attribute(nodiscard)` as a branch, or given C2x's newness, not worth it?
> > > > Does C2x support the attribute on typedefs? (the C++ committee wasn't so enthusiastic about that change & I don't follow the C committee directly)
> > > > 
> > > > I'm happy enough to include C support, if it applies.
> > > Ah good point, it's not allowed on typedefs there either. Neeeevermind. :-)
> > could rename the macro `WARN_UNUSED_RESULT` rather than `NODISCARD_GNU_STYLE`? 
> Hmmm, either seems about equally as confusing. What about `LLVM_NODISCARD_TYPEDEF` to make it clear that this should be used for typedefs and leave it to another macro to handle non-discardable function markings?
I guess, sure.

I guess this also needs a more narrow macro check to verify the clang version (since it seems we support back to clang 5, and this only works on clang 15 and above) - I can't seem to find an example of a macro check for clang version in any of our `Compiler.h` and similar macro-y/compat headers in LLVM - do you happen to know of an example off-hand?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153163/new/

https://reviews.llvm.org/D153163