[Clang] Reject increment of bool value in unevaluated contexts after C++17

[yronglin]

Summary

Clang now incorrectly allowed increment of bool in unevaluated contexts, we set diagnostic::ext_increment_bool to be SFINAEFailure to fix this issue.

template<class T> auto f(T t) -> decltype(++t);
auto f(...) -> void;
void g() {
  f(true);  // Clang wrongly makes this a hard error
}

template <class T>
concept can_increment = requires(T t) { ++t; };

template <class T> void f() {
  static_assert(requires(T t) { ++t; }); // Incorrectly allowed
}

int main() {
  f<bool>();

  static_assert(!can_increment<bool>); // Incorrectly fails

  bool b = false;
  ++b; // Correctly rejected
}

Fix issue: https://github.com/llvm/llvm-project/issues/47517

[erichkeane]

This should have a release note.

I think the bug is asking to make this a SFINAE failure (it isn't clear what you mean by "SFINAE Friendly"). Our mechanism for that is to mark the diagnostic SFINAEError.

clang/lib/Sema/SemaExpr.cpp

14603

I think what you really need to do is just set ext_increment_bool to be SFINAEError in DiagnosticSemaKinds.td, right? Not all of this?

[yronglin]

Address comments

[yronglin]

In D152259#4399217, @erichkeane wrote:

This should have a release note.

I think the bug is asking to make this a SFINAE failure (it isn't clear what you mean by "SFINAE Friendly"). Our mechanism for that is to mark the diagnostic SFINAEError.

Thanks for your review and advice @erichkeane ! I think the title need update.

clang/lib/Sema/SemaExpr.cpp

14603

Thanks for your advice, your are right.

[yronglin]

Add release notes.

[yronglin]

Thanks for your review @erichkeane ! Seems the CI failure not caused by this patch.

[erichkeane]

In D152259#4399781, @yronglin wrote:

Thanks for your review @erichkeane ! Seems the CI failure not caused by this patch.

I agree, I don't think it is related.

[yronglin]

In D152259#4399793, @erichkeane wrote:

In D152259#4399781, @yronglin wrote:

Thanks for your review @erichkeane ! Seems the CI failure not caused by this patch.

I agree, I don't think it is related.

Thanks, landed in 540294845babbb2be909ea456323d7bc8a1763df

clang/lib/Sema/SemaExpr.cpp

static QualType CheckIncrementDecrementOperand(Sema &S, Expr *Op,
   assert(!ResType.isNull() && "no type for increment/decrement expression");
 
   if (S.getLangOpts().CPlusPlus && ResType->isBooleanType()) {
     // Decrement of bool is not allowed.
     if (!IsInc) {
       S.Diag(OpLoc, diag::err_decrement_bool) << Op->getSourceRange();
       return QualType();
     }
+
+    if (S.getLangOpts().CPlusPlus17) {
       // Increment of bool sets it to true, but is deprecated.
-    S.Diag(OpLoc, S.getLangOpts().CPlusPlus17 ? diag::ext_increment_bool
-                                              : diag::warn_increment_bool)
-      << Op->getSourceRange();
+      S.Diag(OpLoc, diag::ext_increment_bool) << Op->getSourceRange();
+
+      // Ensure increment bool SFINAE-friendly.
+      if (S.isUnevaluatedContext() || S.isSFINAEContext())
+        return QualType();
+    } else {
+      S.Diag(OpLoc, diag::warn_increment_bool) << Op->getSourceRange();
+    }
   } else if (S.getLangOpts().CPlusPlus && ResType->isEnumeralType()) {
     // Error on enum increments and decrements in C++ mode
     S.Diag(OpLoc, diag::err_increment_decrement_enum) << IsInc << ResType;
     return QualType();
   } else if (ResType->isRealType()) {
     // OK!
   } else if (ResType->isPointerType()) {
     // C99 6.5.2.4p2, 6.5.6p2

clang/test/SemaCXX/bool-SFINAE-friendly.cpp

+// RUN: %clang_cc1 %std_cxx98-14 -fsyntax-only -verify=precxx17 %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify=expected %s
+// RUN: %clang_cc1 %std_cxx20- -fsyntax-only -verify=cxx20 %s
+// expected-no-diagnostics
+
+template<class T> auto f(T t) -> decltype(++t); // precxx17-warning {{incrementing expression of type bool is deprecated}}
+
+auto f(...) -> void;
+void g() { f(true); }
+
+#if __cplusplus >= 202002L
+template <class T>
+concept can_increment = requires(T t) {
+  ++t;
+};
+
+template <class T>
+void f() {
+  static_assert(requires(T t) { ++t; }); // cxx20-error {{static assertion failed due to requirement 'requires (bool t) { <<error-expression>>; }'}}
+}
+
+int main() {
+  f<bool>(); // cxx20-note {{in instantiation of function template specialization 'f<bool>' requested here}}
+  static_assert(!can_increment<bool>); 
+
+  return 0;
+}
+#endif