Add __attribute__((warn_unused_result)) to LLVMErrorRef

[dwblaikie]

Sending this out a bit half-baked unfortunately - we added support for
this context a couple of years ago in D102122 (which turned up in clang 15
onwards: https://godbolt.org/z/haz8qrTT3 ), not sure if the macro detection
will correctly detect that the attribute is usable in a typedef context...

Original Differential Revision: https://reviews.llvm.org/D153163

[dwblaikie]

@AaronBallman updated this from the phab review to use your suggested LLVM_NODISCARD_TYPEDEF naming. Though my own comment that we probably need a narrower clang version check ( https://reviews.llvm.org/D153163#inline-1484444 ) is still outstanding.

If folks have ideas on that, otherwise I'll try to throw something together to address that as best I can soon.

[AaronBallman]

Code seems okay, but I did have some questions.

[AaronBallman]

Hmm, is this the best place for the code to live given the comment at the top of the file: This file defines an 'extern "C"' wrapper ?

Maybe this belongs in Core.h? or some new file for compiler-based macro definitions?

[dwblaikie]

Fair point - think I semi-randomly picked this as somewhere in llvm-c that was defining macros. But yeah, ExternC.h has a fairly clear purpose and this isn't related to that.

Core.h sounds OK - but also happy to start another header. It's sort of a lower-level (in the C API) version of Compiler.h - could pull some of Compiler.h down into this - but maybe not necessary to do that proactively (except maybe for the LLVM_*_PREREQ macros you mentioned - if we add a LLVM_CLANG_PREREQ macro down here, probably good to pull the others down so they're in the same place)

[AaronBallman]

In the Phab review, you commented:

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?

Do we still need to do this? We don't have a macro for version checking Clang in Compiler.h, but we probably could use one. We have LLVM_GNUC_PREREQ and LLVM_MSC_PREREQ, so adding LLVM_CLANG_PREREQ would be pretty natural.

[dwblaikie]

Do we still need to do this?

I think the original rationale still applies - we support building LLVM with Clang 5.0 and support for __attribute__((warn_unused_result)) was added in Clang 15. Prior to that clang warns that the attribute is used out of place - so we'd want to conditionalize the attribute so as not to warn/werror-break the build when using a supported (>= 5) clang < 15.

We don't have a macro for version checking Clang in Compiler.h, but we probably could use one. We have LLVM_GNUC_PREREQ and LLVM_MSC_PREREQ, so adding LLVM_CLANG_PREREQ would be pretty natural.

Fair - though this one can't go in Compiler.h because it needs to be down in llvm-c and I don't think Compiler.h is that low-level. So we might have to pull the other _PREREQ macros down into llvm-c for symmetry.