This is an archive of the discontinued LLVM Phabricator instance.

Update coding standards for constexpr if statements; NFC
ClosedPublic

Authored by aaron.ballman on Aug 19 2022, 7:37 AM.

Details

Summary

We currently suggest that users not use an else clause after a return statement in a prior if branch. e.g.,

if (foo)
  return 1;
else // Should remove this else clause
  return 10;

however, this suggestion is incorrect for a constexpr if statement because one of the two branches will be a discarded statement and thus can impact template instantiation behavior. This updates the coding standard to make it clear that it's okay to have a return after an else in a constexpr if statement.

I think this is an NFC change to the intent of the rule, which is why I've not started an RFC for the changes.

Diff Detail

Event Timeline

aaron.ballman created this revision.Aug 19 2022, 7:37 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 19 2022, 7:37 AM
aaron.ballman requested review of this revision.Aug 19 2022, 7:37 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 19 2022, 7:37 AM

Agreed. For other reviewers, note that this is the compelling example:

template<typename T>
static constexpr bool VarTempl = true;

template<typename T>
int func() {
  if constexpr (VarTempl<T>)
    return 1;
  
  static_assert(!VarTempl<T>);
}

void use() {
    func<int>();
}

The static-assert would fail, since it isn't a discarded statement.

template<typename T>
static constexpr bool VarTempl = true;

template<typename T>
int func() {
  if constexpr (VarTempl<T>)
    return 1;
  else 
  static_assert(!VarTempl<T>);
}

void use() {
    func<int>();
}

But in THIS case the static-assert is discarded. That is, the 'else' in a if-constexpr isn't required to be anything other than lexically valid, so dependent lookup doesn't need to happen, instantiations aren't required, etc.

Therefore there is EXTREME value to having an 'else' there after a return.

dblaikie accepted this revision.Aug 19 2022, 11:33 AM

Yeah, I'm OK with this, though yeah, having an example where the else is necessary, but I don't mind too much.

This revision is now accepted and ready to land.Aug 19 2022, 11:33 AM

Yeah, I'm OK with this, though yeah, having an example where the else is necessary, but I don't mind too much.

I went ahead and switched the example to Erich's because it wasn't much longer than my example anyway. Thank you!

nridge added a subscriber: nridge.Aug 22 2022, 11:00 AM