readability-braces-around-statements broken for if constexpr

[gnzlbg]

Bugzilla Link	32203
Resolution	FIXED
Resolved on	Jan 22, 2020 16:03
Version	unspecified
OS	All
Blocks	#35172 #43900
CC	@daphnediane,@JonasToth,@zmodem,@craffael,@tg-m,@Xazax-hun
Fixed by commit(s)	rGf9c46229e4ac29053747c96e08c574c6c48d544b

Extended Description

I have:

template <typename T, typename U>
static constexpr auto eq_impl(T&& t, U&& u) {
if
constexpr(std::is_floating_point<uncvref_t>{}
and std::is_floating_point<uncvref_t>{}) {
// do something
}
else {
// do something else
}
}

and the check readability-braces-around-statements broken for if constexpr complains that

warning: statement should be inside braces [readability-braces-around-statements]
and std::is_floating_point<uncvref_t>{}) {
^

That is, it doesn't recognize the braces after if constexpr :/

[daphnediane]

clang-tidy also reports a misc-suspicious-semicolon just after the condition. Further when used with -fix adds an extra closing brace before the body of the if statement but leaving the original closing statement.

I've worked around in my own code with:

// NOLINTNEXTLINE(readability-braces-around-statements,misc-suspicious-semicolon)
if constexpr ( cond ) {
body;
}

Which also reveals a limitation of check aliases in clang tidy if any of the checks that alias to readability-braces-around-statements are enabled they still report failures and attempt to fix the code with -fix.

[tg-m]

Example file

[tg-m]

cmake file for building example

[tg-m]

I've encountered the same problem (in master/trunk branch -- details below) and I was able to narrow down the problem a little bit. Mainly, it occurs only for instantiated template functions and only when the condition inside the if constexpr is false, i.e.

template
inline static constexpr void foo_template_ok(T/* unused /) {
if constexpr(true) { // clang-tidy emits no warning
}
else {
}
}
template
inline static constexpr void foo_template(T/ unused /) {
if constexpr(false) { // clang-tidy emits warning
}
else {
}
}
static void foo(int / unused */) {
if constexpr(false) { // clang-tidy emits no warning
}
else {
}
}

llvm commit: 4c4779b
tools/clang commit: c803508dbb4cad9c28f73ba6fd5820cdc09a17e5
tools/clang/tools/extra commit: 2af1c81

Files are in attachmets: 22290 and 22291.

[Xazax-hun]

Some related changes were proposed in https://reviews.llvm.org/D46234

Note that, the suggestion was to do not synthesize a semi but leave the then branch as null for IfStmts. That might have considerable amount of fallout everywhere, so I did not have time to follow through. But anyone willing to pick this up this can be a good reference.

[llvmbot]

proposed fix
Here is another workaround where the 'False' 'Then' branch of a template instantiated 'if constexpr' is redeclared to an empty compound statement

[llvmbot]

Fixed in https://reviews.llvm.org/rGf9c46229e4ac29053747c96e08c574c6c48d544b

[JonasToth]

Reopened, so that release managers see this is not in the release branch yet and that it can be backported.

Once that is done, the bug should be closed again.

[zmodem]

Cherry-picked to 10.x in 1f98c2b

[llvmbot]

mentioned in issue llvm/llvm-bugzilla-archive#32204

[llvmbot]

mentioned in issue #35172

[zmodem]

mentioned in issue #43900