[[maybe-unused]] vs __attribute__((unused)) vs (void) var for assert variables

LLVM Project
1

When defining a variable that is used only in an assert, the LLVM style guide https://llvm.org/docs/CodingStandards.html#id47 recommends using

(void) var;

Why is this preferred over [[maybe-unused]] or _attribute_((unused))?

Thanks

2

This should probably be changed to [[maybe_unused]] when we have merged the C++17 standards update.

3

The challenge can also be addressed with control flow, but the community decided to use a c solution in a in C++ codebase.

4

Thanks for the reply @tschuett, please can you elaborate on ‘The challenge can also be addressed with control flow’ ?

5

The alternative is to use #ifndef NDEBUG, I thought. (void) is tidier to look at.

6

I agree it is tidier, but I also thought the intention could be a bit unclear to the uninitiated.

7 
int var = 5;
if (assert_will_fail(var)) {
  assert(false && "bad things")
}
8

I don’t think this is an area where we should try to innovate. Casting to void is a tried and true idiom for indicating that a value is unused. It’s even documented in the coding standards for folks who aren’t familiar with the idiom. I don’t see value in deviating from C just for it’s own sake, there should be some value proposition, like safety, for doing so.

4 Likes
9


 which is unidiomatic and risks that the compiler is unable to eliminate whatever is in assert_will_fail in builds without assertions enabled. (void) or [[maybe_unused]] work better and are idiomatic.

10

Totally agreed, but I prefer the new [[maybe_unused]] attribute. It better describes the intent than a Cish idiom.

3 Likes
11

Just MHO I don’t see an advantage to using the attribute. It is more verbose and the cast to void idiom is obvious enough.

The one advantage I see of the attribute is that it is more “explicit”, but this is C++ which is full of magic and special cases (sadly). I don’t think this is better than the old C idiom.

1 Like
12

The advantage of [[maybe_unused]] attribute is that it can apply to pretty much anything. I.e. you can mark functions/arguments with it, if there is some rarely used value (like for printf-like functions). But for simple unused variables, (void) is more concise idiom understood by pretty much everyone.

2 Likes
13

The attribute is more verbose in terms of character length, but it is less verbose in terms of statement count:

[[maybe_unused]] int answer = 42;

vs

int answer = 42;
(void)answer;

Personally, I prefer the attribute version more. In addition, as @danilaml mentioned, the attribute is more versatile than the cast-to-void.

There is one more option to handle the unused variables which is derivative of the cast-to-void version:

#define LLVM_UNSUSED(arg) (void)(arg)

int answer = 42;
LLVM_UNUSED(answer);

It is more verbose for sure, yet probably more readable.

14

The statement count being shorter is persuasive to me that we should be using [[maybe_unused]], although I don’t strongly object to using the (void) approach, and wouldn’t advocate a mass-switching.

15

I would be fine with new code using maybe_unused, I think it’s clearer than the void cast. But I also agree we shouldn’t mass switch

16

It doesn’t look like the developer guide was ever updated. Now LLVM has been on c++17 for a little while, is now a good time to bump this and write something down?

A coarse comparison of usage in the LLVM codebase, with just under a year of having c++17 enabled: \s\(void\)[^ ] matches ~1k files, whereas \[\[maybe_unused\]\] only matches ~50. Both numbers are excluding files named “test”, and both likely are overcounting, although it seems accurate enough for relative comparison. So: it is clear that people are using this new attribute, even if only a little bit.

If I’m judging the posts in this thread correctly, the majority opinion is to use [[maybe_unused]] in new code, although it is not a consensus: some people still find (void) more idiomatic.

There is consensus that if we do recommend [[maybe_unused]], we should not bulk switch.

Would people be open to amending the guide to say:

  • In new code, prefer [[maybe_unused]]. Using (void) is still allowed, but the intent is less clear.
    • (TODO: in what cases is (void) allowed? Only when consistent with old code? Anytime the author prefers it?)
  • Do not switch from (void) to [[maybe_unused]] in bulk.

Also, LLVM_DEBUG is sometimes appropriate – I think we could mention that. And the guide also recommends folding the unused expression into the assert itself. Are there other canonical approaches?

3 Likes
17

I totally agree. We have c++17, we have powerful computers, we have great development tools, but we still use some ancient weird fixtures, kind of idioms, we reinvent the wheel when we can use it out of the box.

(Just slightly promoting my discussion about #pragma once)

18

In the past, “don’t mass-convert” was generally applied to more extensive changes such as clang-formatting entire files or renaming methods/fields to conform to newer spelling recommendations. The latter sorts of changes are commonly held to interfere with other people’s work on the same files. (Although, clang-formatting a file prior to doing extensive work on that file is actually seen as a good thing, because the individual changes are expected to be clang-formatted.)

In this case, however, changing

int foo = 12;
(void)foo;

to

[[maybe_unused]] foo = 12;

is not an extensive change within a file, and so is highly unlikely to interfere with anyone else’s work-in-progress. The changes may touch a bunch of files, but the individual changes aren’t any more intrusive than many other refactorings that go on. So, I argue in favor of getting it done and over with. (Preferably in commits that batch 10s of files, rather than 100s or even all 1000 at once; the odds of needing to revert one are low but non-zero IMO, so some caution is appropriate.)

I for one would like to see at least one of the many style changes we agree on actually get applied uniformly, instead of leaving the entire codebase in an inconsistent mess. (One of the very first lessons I ever learned as a pro programmer came when I was confronted with a chunk of 20-odd lines of code with at least 6 different styles. Ever since, I’ve not cared so much about the exact style, as long as it was consistently used.)

6 Likes
19

Is there interest in continuing this discussion? There is a discussion about casting code standards and this topic was mentioned again.

20

Personally, I think I’m now leaning towards preferring [[maybe_unused]] for clarity (and also it allows avoiding the exception I needed to add to the coding standard in the casting rules).

I also feel now that if somebody is happy to do the work for the mass-switching themselves, it’s probably for the best, given the options we have in git for ignoring the relevant changes. The turbulence it causes should be minimal.

1 Like