[PATCH] D78356: [flang] Use a better definition for ATTRIBUTE_UNUSED

Aaron Ballman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 20 06:59:45 PDT 2020


aaron.ballman added a comment.

In D78356#1991971 <https://reviews.llvm.org/D78356#1991971>, @DavidTruby wrote:

> @aaron.ballman tried to give a review here but couldn't add himself so he sent this review by email, which I will include here for completeness:


Thank you for adding this, I think I may be able to comment on the review now (I'll find out for sure when I hit Submit).

>> Is there a reason to
>>  not use LLVM_ATTRIBUTE_UNUSED from Compiler.h? (Sorry if this is a
>>  dumb drive-by question.)
> 
> In answer to the question (which isn't a dumb one at all!) I would prefer LLVM_ATTRIBUTE_UNUSED to our custom one; however since Flang requires C++17 (unlike most of LLVM) I think we should just use the standard attribute everywhere as it's guaranteed to be available on any C++17 compatible compiler.

A compiler can be compatible with C++17 and still ignore this attribute, but may support other spellings of it, which is a (very) minor consideration to keep in mind. More importantly, if the expectation is that the implementation will support the attribute, why is the macro needed at all?

My preference is that we either go with the attribute spelling directly (no macro), or we go with the existing macro in Compilers.h rather than invent a second place for general purpose support macros to live. (If the macro is specific to flang, then I think it makes sense to add to `idioms.h`, but not for general purpose macros like this.)



================
Comment at: flang/include/flang/Common/idioms.h:101
 
-#if !defined ATTRIBUTE_UNUSED && (__clang__ || __GNUC__)
-#define ATTRIBUTE_UNUSED __attribute__((unused))
-#endif
+#define ATTRIBUTE_UNUSED [[maybe_unused]]
 
----------------
Is there a reason to not use `LLVM_ATTRIBUTE_UNUSED` from Compiler.h? (Sorry if this is a dumb drive-by question.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78356





More information about the llvm-commits mailing list