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

[fhahn]

@jh7370

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).

Personally I also think [[maybe_unused]] is preferable due to clarity

[chrisjbris]

If we’re happy to change this I can write a script to perform the replacements and generate diffs in chunks of 10-20 files and start pushing up diffs for review. I’ll see if I can get an idea of how many files would need to be changed before I start in ernest.

My grep & regex skills are poor, so please help me correct the following if you can. Certainly the type list probably has a nicer experssion, at the very least. Using the rudementary grep below in the llvm-project root:

pcregrep -M -rl --include='\.cpp$' --include='\.c$' --include='\.h$'    '(uint[0-9]+t|int|char|float|double|bool|unsigned)[ ]([a-zA-Z]+)[ ].*\n.*\(void\)[ ]*\g{-1}' ../ | wc -l

I get 183 source files as candidates for modification.

[kuhar]

@chrisjbris

My grep & regex skills are poor, so please help me correct the following if you can.

This would be great to have as a clang-tidy check.

[s-barannikov]

I’m not sure why this is being discussed at all.
Just because we’re using C++17 does not mean we should abuse it.

[chrisjbris]

Hi @s-barannikov, I’m happy to hear reasons against using the attribute and why it constitutes abuse. After all, the current idiom does function.

However, the discussion above seems to indicate a desire to move to the attribute.

[s-barannikov]

The reasons have already been spelled by others, I have nothing to add.
The main point is that (void) is a well recognized idiom.
[[maybe_unused]] must have a significant advantage for it to be considered.

@chrisjbris

However, the discussion above seems to indicate a desire to move to the attribute.

The number of hearts each post received indicates otherwise, if I count correctly.

[kparzysz-quic]

@s-barannikov

[[maybe_unused]] must have a significant advantage for it to be considered.

There you go:

@danilaml

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).

If we want consistency, then the attribute is the way to go.

[s-barannikov]

I don’t mind using it on functions. I think nobody would oppose that, because the alternative is LLVM_ATTRIBUTE_UNUSED.

When it comes to variables, this becomes more sensitive, because there are many more of them. There couldn’t be more low level component of a product than a coding style. Not only the code is already written, but the habbits are developed. The Big Switch can be carried out, but I can imagine people silently asking “But why? I’m so used to it.”.

@kparzysz-quic

@danilaml

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).

If we want consistency, then the attribute is the way to go.

There is no contradiction, we can consistently use it on functions. An analogy is the ‘const’ qualifier. We use it, but we’re not obsessed with it.

So far I’ve seen justifications for using the attribute, but not arguments for switching to it.
I won’t object if I see the use of the attribute in a review, but I don’t want to be forced to use it.
(The same applies to the other RFC that proposes switching to C++-style casts.)

[s-barannikov]

[offtopic]
Is there a way to organize a poll to eliminate the “active minority” factor on either side?
Does discourse allow it, e.g. by sending a notification? This would convince me.

[mehdi_amini]

@s-barannikov

I won’t object if I see the use of the attribute in a review, but I don’t want to be forced to use it.

Consistency of the codebase matters a lot to me.

[s-barannikov]

@mehdi_amini

Consistency of the codebase matters a lot to me.

To me, too. But is this particular case that important?
There are several different styles for function argument comments in the codebase:

1. /*name=*/value
2. /*name*/ value
3. /* name */value
4. value/*name*/

I personally prefer (1). Just becase my IDE can insert it automatically. But I’m used to it and wouldn’t want to be forced to use any other style. I consider (void) vs [[maybe_unused]] the same kind of issue.

[mehdi_amini]

@s-barannikov

But is this particular case that important?

Just as much as almost anything else that is in the coding guidelines?

@s-barannikov

There are several different styles for function argument comments in the codebase:

Actually we have a clang-tidy check that relies on 1 to check consistency between caller and caller (so this is more than a style issue) and I am quite sure I ask for this format in code review.
If we don’t document it I would definitely look into doing it.
There is a lot inconsistency from history in the codebase, but that’s not a good excuse to avoid standardizing the style guide IMO

[s-barannikov]

@mehdi_amini

Just as much as almost anything else that is in the coding guidelines?

It is not in the coding guidelines (yet). If it is standardized, I’ll be obliged to use it and won’t complain.

Actually we have a clang-tidy check to enforce 1, and I am quite sure I ask for this format in code review.

Good to know, thanks!

There is a lot inconsistency from history in the codebase, but that’s not a good excuse to avoid standardizing the style guide IMO

I agree. However, in order to standardize it, there should be a consensus. I’m afraid there will not be one, there is just not enough data to say which option wins. Looks like I’m kind of safe for now

[tschuett]

@mehdi_amini

Consistency of the codebase matters a lot to me.

Then you should have a chat with the Flang people.

[mehdi_amini]

If you have specific things about flang where it does not follow the style guide we can discuss it (in another place than this thread), as far as I remember the agreement to merge flang in LLVM was that it would adhere to the LLVM coding standard.

[tschuett]

“” Always wrap the bodies of if() , else , while() , for() , do , &c. with braces, even when the body is a single statement or empty. The opening { goes on the end of the line, not on the next line. Functions also put the opening { after the formal arguments or new-style result type, not on the next line. Use {} for empty inline constructors and destructors in classes.“”

[jurahul]

I want to note that both flang and MLIR deviate from LLVM coding standards AFAICT. Flang C++ style is documented here: Flang C++ Style Guide — The Flang Compiler

also in MLIR, I see that variable names do not follow the LLVM coding standard, for example. LLVM says " The name should be camel case, and start with an upper case letter", whereas I see in MLIR variables names frequently start with lower case. For just one example:

llvm-project/mlir/lib/IR/Diagnostics.cpp at main · llvm/llvm-project

[mehdi_amini]

MLIR was started at a point where there was an RFC to change LLVM coding convention on variable naming, we adopted this because it seemed at the time that LLVM would be updated. Instead only lld was updated (it uses the same lower-case for variable name convention as MLIR I believe), and LLVM ended up not being updated. MLIR got on the wrong side of it, but that wasn’t intentional: we aimed initially at being just aligned with the project. It’s not an example to take as a justification to diverge further. MLIR has a style-guide section documented but it is small and meant to complement LLVM, not override anything (other than camelCase).

Did you find some divergence from LLVM in the Flang document you linked to?
Edit: actually the topic of flang was discussed right above already.

[osirisgothra]

I’m only posting this because this is still a very valid concern and new programmers need to be aware. Before anyhing, steer clear of compiler-specific attributes if you can (such as gnu:whatever) unless you are 1) not porting or 2) prepared to add #guards and handle every single incantation of the attribute (and the absence of). Usually its best to just define it blank when there is an unexpected compiler. As of now, Seems like [[maybe_unused]] has support most of the time (but not always) but there will be others as well, accumulated from years of updates and standards and flavors of compilers (eg, [[gnu:unused]], [[_maybe_unused_]]) but modern mainstream compilers almost all support [[maybe_unused]] (msvc, gnu g++, etc)

As for which to use, I always use [[maybe_unused]] because it can be used in places where void casts cannot, such as function parameters: int func(int param1, [[maybe_unused]] int param2)

Otherwise you’d use the void cast, which technically incorrect anyway since it is “using” the symbol. Using the attribute is TRULY marking it as possibly unused so the compiler might decide to optimize away the usage altogether. This is particularly helpful in cases such as templates where said object needs to be optimized away. for instance template int func(T param1, [[maybe_unused]] T param2) … when a template is constructed and param2 never gets used, the compiler can then optimize it away completely instead of allocating a second unused someBigNastyObject onto the function stack. If you went the void cast route, you would use the object, so the compiler would keep it on the stack and it would just be taking up space. If the function was, say a recursive function, that would exponentially either waste space or save space depending on your approach.

[dblaikie]

@osirisgothra

for instance template int func(T param1, [[maybe_unused]] T param2) … when a template is constructed and param2 never gets used, the compiler can then optimize it away completely instead of allocating a second unused someBigNastyObject onto the function stack. If you went the void cast route, you would use the object, so the compiler would keep it on the stack and it would just be taking up space.

I don’t think this is true in practice for LLVM/Clang and probably other modern compilers. Do you have an example (godbolt.org link, ideally) showing this difference in result between cast-to-void and [[maybe_unused]] (& I guess even a difference between these two and no annotation at all - just a silently unused parameter/variable/etc)?