[clang][Sema] Fix assertion in tryDiagnoseOverloadedCast

[alejandro-alvarez-sonarsource]

Fixed an assertion failure in debug mode, and potential crashes in release mode, when
diagnosing a failed cast caused indirectly by a failed implicit conversion to the type of the constructor parameter.

For instance

template<typename>
struct StringTrait {};

template< int N >
struct StringTrait< const char[ N ] > {
  typedef char CharType;
  static const MissingIntT length = N - 1;
};

class String {
public:
  template <typename T>
  String(T& str, typename StringTrait<T>::CharType = 0);
};


class Exception {
public:
  Exception(String const&);
};

void foo() {
  throw Exception("some error");
}

Exception(String const&) is a matching constructor for Exception from a const char*, via an implicit conversion to String. However, the instantiation of the String constructor will fail because of the missing type MissingIntT inside the specialization of StringTrait.

When trying to emit a diagnosis, tryDiagnoseOverloadedCast expects not to have a matching constructor, but there is; it just could not be instantiated.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Alejandro Álvarez Ayllón (alejandro-alvarez-sonarsource)

Changes

A constructor may have failed to be used due to a broken templated dependent parameter. The InitializationSequence itself can succeed.

Due to the assumption that it is in a failed state, it can either cause an assertion failure, or undefined behavior in release mode, since sequence.Failure is not initialized.

---

Full diff: https://github.com/llvm/llvm-project/pull/108021.diff

2 Files Affected:

• (modified) clang/lib/Sema/SemaCast.cpp (+6-1)
• (added) clang/test/Parser/cxx-bad-cast-diagnose-broken-template.cpp (+31)

diff --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp
index f01b22a72915c8..6ac6201843476b 100644
--- a/clang/lib/Sema/SemaCast.cpp
+++ b/clang/lib/Sema/SemaCast.cpp
@@ -446,7 +446,12 @@ static bool tryDiagnoseOverloadedCast(Sema &S, CastType CT,
     : InitializationKind::CreateCast(/*type range?*/ range);
   InitializationSequence sequence(S, entity, initKind, src);
 
-  assert(sequence.Failed() && "initialization succeeded on second try?");
+  // It could happen that a constructor failed to be used because
+  // it requires a temporary of a broken type. Still, it will be found when
+  // looking for a match.
+  if (!sequence.Failed())
+    return false;
+
   switch (sequence.getFailureKind()) {
   default: return false;
 
diff --git a/clang/test/Parser/cxx-bad-cast-diagnose-broken-template.cpp b/clang/test/Parser/cxx-bad-cast-diagnose-broken-template.cpp
new file mode 100644
index 00000000000000..f9fc3e6082da10
--- /dev/null
+++ b/clang/test/Parser/cxx-bad-cast-diagnose-broken-template.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -fsyntax-only -verify %s
+
+template< typename>
+struct IsConstCharArray
+{
+  static const bool value = false;
+};
+
+template< int N >
+struct IsConstCharArray< const char[ N ] >
+{
+  typedef char CharType;
+  static const bool value = true;
+  static const missing_int_t length = N - 1; // expected-error {{unknown type name 'missing_int_t'}}
+};
+
+class String {
+public:
+  template <typename T>
+  String(T& str, typename IsConstCharArray<T>::CharType = 0);
+};
+
+
+class Exception {
+public:
+  Exception(String const&);
+};
+
+void foo() {
+  throw Exception("some error"); // expected-error {{functional-style cast from 'const char[11]' to 'Exception' is not allowed}}
+}

[shafik]

Thank you for the fix.

Is this from a bug report, if so it should be mentioned in the summary.

The summary itself could be a big clearer, maybe a small code example could help.

I think this also needs a release note.

[alejandro-alvarez-sonarsource]

I have tried to clarify the PR with a snippet and aligned the test with it.

Is this from a bug report, if so it should be mentioned in the summary.

It isn't. Or, as far as I know, there is no mention of it.

[shafik]

LGTM

[alejandro-alvarez-sonarsource]

Thanks! Do you mind merging the PR? I do not have permission to do so.