[Clang][Sema] Fix crash when type used in return statement contains errors

[shafik]

In Sema in BuildReturnStmt(...) when we try to determine is the type is move eligible or copy elidable we don't currently check of the init of the VarDecl contain errors or not. This can lead to a crash since we may send a type that is not complete into getTypeInfo(...) which does not allow this.

This fixes: #63244
#79745

[llvmbot]

@llvm/pr-subscribers-clang

Author: Shafik Yaghmour (shafik)

Changes

In Sema in BuildReturnStmt(...) when we try to determine is the type is move eligable or copy elidable we don't currently check of the init of the VarDecl contain errors or not. This can lead to a crash since we may send a type that is not complete into getTypeInfo(...) which does not allow this.

This fixes: #63244
#79745

---

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

2 Files Affected:

• (modified) clang/lib/Sema/SemaStmt.cpp (+2)
• (modified) clang/test/SemaCXX/deduced-return-type-cxx14.cpp (+9)

diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 9e7c8c7e4e8c12..5d5a29b825ae7d 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -3391,6 +3391,8 @@ Sema::NamedReturnInfo Sema::getNamedReturnInfo(Expr *&E,
   const auto *VD = dyn_cast<VarDecl>(DR->getDecl());
   if (!VD)
     return NamedReturnInfo();
+  if (VD->getInit() && VD->getInit()->containsErrors())
+    return NamedReturnInfo();
   NamedReturnInfo Res = getNamedReturnInfo(VD);
   if (Res.Candidate && !E->isXValue() &&
       (Mode == SimplerImplicitMoveMode::ForceOn ||
diff --git a/clang/test/SemaCXX/deduced-return-type-cxx14.cpp b/clang/test/SemaCXX/deduced-return-type-cxx14.cpp
index c0d43911b8c717..415bbbf1a0bc50 100644
--- a/clang/test/SemaCXX/deduced-return-type-cxx14.cpp
+++ b/clang/test/SemaCXX/deduced-return-type-cxx14.cpp
@@ -724,3 +724,12 @@ struct DeducedTargetTypeOfConversionFunction {
   // since-cxx20-error@-1 {{'decltype(auto)' not allowed in declaration of conversion function template}}
 #endif
 };
+
+namespace GH79745 {
+template <typename = int> struct a; // expected-note {{template is declared here}}
+auto f() {
+  a c; // cxx20_23-error {{implicit instantiation of undefined template}} \
+       // cxx14-error {{use of class template 'a' requires template arguments}}
+  return c;
+}
+}

[cor3ntin]

Missing changelog, otherwise LGTM

[Fznamznon]

NIT: eligable -> eligible in the description.

[ahatanak]

I think this change can confuse users in some cases.

class Error {
public:
  Error() = default;
  Error(Error &&);
  Error &operator=(Error &&);
  explicit operator bool() const;
};

Error getError();

Error foo() {
  if (Error e = getError(1)) { // error: no matching function for call to 'getError'
    return e; // call to implicitly-deleted copy constructor of 'Error'
  }
  return Error();
}

When the code above is compiled, clang complains that the implicitly-deleted copy constructor of 'Error' is called. This makes users wonder why return e isn't eligible for copy elision.

I wonder whether it would be better to check that VD's type is incomplete rather than calling containsErrors.