Coding standard: Deprecate use of void cast to suppress unused variable warnings? (original) (raw)

LLVM Coding Standard mentions using void casting to suppress unused variables warnings (for variables used only in asserts) at 2 places:

When casting, use static_cast, reinterpret_cast, and const_cast, rather than C-style casts. There are two exceptions to this:

and

bool NewToSet = Myset.insert(Value); (void)NewToSet;
assert(NewToSet && "The value shouldn't be in the set yet");

Should we always recommend using [[maybe_unused]] over void casting in these cases?

Endill June 4, 2025, 7:13pm 2

This depends on availability of [[maybe_unused]] across the compilers we support.

jurahul June 4, 2025, 7:14pm 3

My understanding is that LLVM requires C++17 support and [[maybe_unused]] is a C++ 17 standard feature.

Endill June 4, 2025, 7:17pm 4

The fact that we build in C++17 mode doesn’t mean that we require all of the features of the C++17. For instance, certain behaviors of template template parameters introduced in C++17 hasn’t been entirely implemented anywhere, so there’s no way we can rely on them.

So checking for compiler support of this attribute and coming back to this thread is a useful piece of information.

jurahul June 4, 2025, 7:32pm 5

Fair enough. The host compilers LLVM supports are listed here: Getting Started with the LLVM System — LLVM 21.0.0git documentation

@Endill as you suspected, it’s not supported with GCC 7.4. Google search AI overview for “GCC maybe_unused which version” says:

Therefore, to ensure full and proper functionality of [[maybe_unused]] in GCC, it is recommended to use GCC version 10 or later , as the GCC 10 release notes mention explicit support for standard attributes, including [[maybe_unused]] .

For Clang,

The [[maybe_unused]] attribute in C++ is a C++17 feature. Clang supports this attribute starting from Clang 9

For Visual studio:

The [[maybe_unused]] attribute in C++ was introduced in C++17 and is available in Visual Studio 2017 version 15.3 and later

So at least 2 of the supported host compilers do not seem to support it fully. Note though that there is already plenty of usage of this attribute in our code.

jurahul June 4, 2025, 7:53pm 6

Release dates for comparison:

and

Endill June 4, 2025, 7:58pm 7

Then it’s clearly too early to change our policy.

nikic June 4, 2025, 8:01pm 8

@jurahul I think you may be confusing support in C and in C++?

We know that all supported host compilers support [[maybe_unused]] in C++ code by the simple fact that the attribute is already extensively used in LLVM.

jurahul June 4, 2025, 8:07pm 9

That’s what I thought. My proposal is only for C++ code, so looks like the coding standard change I am proposing might be ok (ofcourse this will apply going forward and no one is going to do a mass rewrite here). I didn’t realize that C also has support for this in C23 and that is what the Google search results are probably referring to.

jurahul June 4, 2025, 8:10pm 10

Tweaking my query a bit: “GCC maybe_unused which version C++”

GCC has had full support for C++17 features since version 8.

Clang version 5 and later fully support all C++17 language features, including [[maybe_unused]]

So GCC version 7.4 may be odd man out but close enough that its ok for a coding standard change.

It’s probably better to just look at whether or not the compilers actually respect/support this:

Apple Clang doesn’t exist on Compiler Explorer, but presumably also supports this feature. It seems like all of the minimum compiler versions support [[maybe_unused]], at least with -std=c++17.

bogner June 4, 2025, 8:19pm 12

We already use [[maybe_unused]] for this in hundreds of places, and it certainly works in all of our tested (or even working) configurations. In fact, even if it isn’t supported by some compiler, if it supports C++11 it would simply warn that this attribute was unknown and ignore it.

Yes, let’s update the docs to recommend [[maybe_unused]] instead of recommending (void) casts.

jurahul June 4, 2025, 8:23pm 13

Sounds good. I’ll prepare a PR

jurahul June 4, 2025, 8:54pm 14

arsenm June 5, 2025, 12:45am 15

One common unused variable pattern is

bool X = foo();
(void) X;
assert(x && "message");

In this case I think it should be rewritten as

  if (!foo())
    llvm_unreachable("message");

I would discourage the use of unreachable unless we have a performance proof its necessary. If we want to mention techniques to consider, I’d recommend assert and then return on the violated path to get consistent and safe behavior even if some precondition is violated. I’d go so far to not even mention unreachable because that would make folks believe its a good tool for the job. People who need unreachable will anyway know when to use it. Its far too often used already without any warrant.

arsenm June 5, 2025, 4:10am 17

It has nothing to do with performance, it’s a different style of assert.

You cannot get safe behavior when the precondition is violated

jh7370 June 5, 2025, 8:59am 18

+1 to preferring [[maybe_unused]] over casting to void. I’ve recently started using this approach in an internal codebase and find it more readable and satisfying syntactically.

For the record, I made the change around casting in the coding standard a couple of years ago (⚙ D151187 [doc] Add casting style preference to coding standards). At that point, I went with either/or purely so that we didn’t have to address the issue at that time (I was more focused on the use of C++ style casts in general).

Personally, I’m neither in favor nor opposed; I think it should be recommended as a choice but not necessarily prescribed as a requirement. IMO, there are some situations where maybe_unused is a distraction and other times when it makes the code more clear. e.g.,

void func([[maybe_unused]] int x) { // #1
  int bar = foo();
  (void)bar; // #2
  assert(bar != 0);
  assert(x != 12);
}

Personally, I think #1 is not a good use of the attribute because callers do not need to care about that information, it’s purely an implementation detail of the function. But I think #2 is not a good use of the cast because it’s purely a local decision to write the code that way. We could just use the attribute to make it more clear.

kparzysz June 5, 2025, 12:45pm 20

I can’t get either gcc or clang to warn on an unused function parameter (with -Wall -pedantic). Unless I’m missing something, there is no strong reason to do anything about x in your example.