[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 ↗

(On Diff #528795)

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 ↗

(On Diff #528795)

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/docs/ReleaseNotes.rst

 - Fix a crash when instantiating a non-type template argument in a dependent scope.
   (`#62533 <https://github.com/llvm/llvm-project/issues/62533>`_).
 - Fix crash when diagnosing default comparison method.
   (`#62791 <https://github.com/llvm/llvm-project/issues/62791>`_) and
   (`#62102 <https://github.com/llvm/llvm-project/issues/62102>`_).
 - Fix crash when passing a braced initializer list to a parentehsized aggregate
   initialization expression.
   (`#63008 <https://github.com/llvm/llvm-project/issues/63008>`_).
+- Reject increment of bool value in unevaluated contexts after C++17.
+  (`#47517 <https://github.com/llvm/llvm-project/issues/47517>`_).
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Bug Fixes to Attribute Support
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 - Fixed a bug where attribute annotations on type specifiers (enums, classes,
   structs, unions, and scoped enums) were not properly ignored, resulting in

clang/include/clang/Basic/DiagnosticSemaKinds.td

def warn_bitwise_negation_bool : Warning<
   "did you mean logical negation?">,
   InGroup<BoolOperation>, DefaultIgnore;
 def err_decrement_bool : Error<"cannot decrement expression of type bool">;
 def warn_increment_bool : Warning<
   "incrementing expression of type bool is deprecated and "
   "incompatible with C++17">, InGroup<DeprecatedIncrementBool>;
 def ext_increment_bool : ExtWarn<
   "ISO C++17 does not allow incrementing expression of type bool">,
-  DefaultError, InGroup<IncrementBool>;
+  DefaultError, SFINAEFailure, InGroup<IncrementBool>;
 def err_increment_decrement_enum : Error<
   "cannot %select{decrement|increment}0 expression of enum type %1">;
 
 def warn_deprecated_increment_decrement_volatile : Warning<
   "%select{decrement|increment}0 of object of volatile-qualified type %1 "
   "is deprecated">, InGroup<DeprecatedVolatile>;
 def warn_deprecated_simple_assign_volatile : Warning<
   "use of result of assignment to object of volatile-qualified type %0 "

clang/test/SemaCXX/bool-increment-SFINAE.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=c++17 -DFAILED_CXX17 -fsyntax-only -verify=failcxx17 %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); }
+
+#ifdef FAILED_CXX17
+
+template<class T> auto f1(T t) -> decltype(++t); // failcxx17-note {{candidate template ignored: substitution failure [with T = bool]: ISO C++17 does not allow incrementing expression of type bool}}
+auto f1(void) -> void; // failcxx17-note {{candidate function not viable: requires 0 arguments, but 1 was provided}}
+void g1() { f1(true); } // failcxx17-error {{no matching function for call to 'f1'}}
+
+#endif
+
+#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