[clang] 55cbbce - [clang][ASTImporter] Fix of unchecked Error object (#112688)

[via cfe-commits]

Author: Balázs Kéri
Date: 2024-10-18T12:53:47+02:00
New Revision: 55cbbce0958c8dbd4ae800d16d1d12a31173ace4

URL: https://github.com/llvm/llvm-project/commit/55cbbce0958c8dbd4ae800d16d1d12a31173ace4
DIFF: https://github.com/llvm/llvm-project/commit/55cbbce0958c8dbd4ae800d16d1d12a31173ace4.diff

LOG: [clang][ASTImporter] Fix of unchecked Error object (#112688)

After commits 9c72a30 and 30a9cac error handling in function
'importTemplateParameterDefaultArgument' was not correct,
probably related to (not) using std::move. A crash with unchecked
Error result could happen when the import error path was taken.

Here a test is added that reproduces this case and the problem is
fixed.

Added: 
    

Modified: 
    clang/lib/AST/ASTImporter.cpp
    clang/unittests/AST/ASTImporterTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 020a2f396b5aa0..e7a6509167f0a0 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -362,24 +362,24 @@ namespace clang {
     template <typename TemplateParmDeclT>
     Error importTemplateParameterDefaultArgument(const TemplateParmDeclT *D,
                                                  TemplateParmDeclT *ToD) {
-      Error Err = Error::success();
       if (D->hasDefaultArgument()) {
         if (D->defaultArgumentWasInherited()) {
-          auto *ToInheritedFrom = const_cast<TemplateParmDeclT *>(
-              importChecked(Err, D->getDefaultArgStorage().getInheritedFrom()));
-          if (Err)
-            return Err;
+          Expected<TemplateParmDeclT *> ToInheritedFromOrErr =
+              import(D->getDefaultArgStorage().getInheritedFrom());
+          if (!ToInheritedFromOrErr)
+            return ToInheritedFromOrErr.takeError();
+          TemplateParmDeclT *ToInheritedFrom = *ToInheritedFromOrErr;
           if (!ToInheritedFrom->hasDefaultArgument()) {
             // Resolve possible circular dependency between default value of the
             // template argument and the template declaration.
-            const auto ToInheritedDefaultArg =
-                importChecked(Err, D->getDefaultArgStorage()
-                                       .getInheritedFrom()
-                                       ->getDefaultArgument());
-            if (Err)
-              return Err;
+            Expected<TemplateArgumentLoc> ToInheritedDefaultArgOrErr =
+                import(D->getDefaultArgStorage()
+                           .getInheritedFrom()
+                           ->getDefaultArgument());
+            if (!ToInheritedDefaultArgOrErr)
+              return ToInheritedDefaultArgOrErr.takeError();
             ToInheritedFrom->setDefaultArgument(Importer.getToContext(),
-                                                ToInheritedDefaultArg);
+                                                *ToInheritedDefaultArgOrErr);
           }
           ToD->setInheritedDefaultArgument(ToD->getASTContext(),
                                            ToInheritedFrom);
@@ -395,7 +395,7 @@ namespace clang {
                                     *ToDefaultArgOrErr);
         }
       }
-      return Err;
+      return Error::success();
     }
 
   public:

diff  --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp
index aacecd3fbcd902..bf7313f882e455 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -9986,6 +9986,34 @@ TEST_P(ImportTemplateParmDeclDefaultValue, InvisibleInheritedFrom) {
             ToFDef->getTemplateParameters()->getParam(0));
 }
 
+TEST_P(ImportTemplateParmDeclDefaultValue, DefValImportError) {
+  const char *ToCode =
+      R"(
+      class X {
+        int A;
+      };
+      )";
+  getToTuDecl(ToCode, Lang_CXX14);
+
+  const char *FromCode =
+      R"(
+      class X;
+
+      template <typename P = X>
+      void f() {}
+
+      class X {
+        char A;
+      };
+      )";
+  TranslationUnitDecl *FromTU = getTuDecl(FromCode, Lang_CXX14);
+  auto *FromF = FirstDeclMatcher<FunctionTemplateDecl>().match(
+      FromTU, functionTemplateDecl(hasName("f")));
+
+  auto *ToFImported = Import(FromF, Lang_CXX14);
+  EXPECT_FALSE(ToFImported);
+}
+
 TEST_P(ImportTemplateParmDeclDefaultValue, ImportFunctionTemplate) {
   TranslationUnitDecl *FromTU = getTuDecl(CodeFunction, Lang_CXX14);
   auto *D3 = LastDeclMatcher<FunctionTemplateDecl>().match(