teemperor updated this revision to Diff 237953.
teemperor added a comment.

- Rebased to get rid of shady StringRef -> C-String conversion.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72684/new/

https://reviews.llvm.org/D72684

Files:
  
lldb/packages/Python/lldbsuite/test/commands/expression/completion-crash1/TestCompletionCrash1.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/completion-crash1/main.cpp
  
lldb/packages/Python/lldbsuite/test/commands/expression/deleting-implicit-copy-constructor/TestDeletingImplicitCopyConstructor.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/deleting-implicit-copy-constructor/main.cpp
  lldb/source/Symbol/ClangASTContext.cpp
  lldb/unittests/Symbol/TestClangASTContext.cpp

Index: lldb/unittests/Symbol/TestClangASTContext.cpp
===================================================================
--- lldb/unittests/Symbol/TestClangASTContext.cpp
+++ lldb/unittests/Symbol/TestClangASTContext.cpp
@@ -523,3 +523,89 @@
   EXPECT_EQ("foo", func_template->getName());
   EXPECT_EQ(clang::AccessSpecifier::AS_public, func_template->getAccess());
 }
+
+TEST_F(TestClangASTContext, TestDeletingImplicitCopyCstrDueToMoveCStr) {
+  // We need to simulate this behavior in our AST that we construct as we don't
+  // have a Sema instance that can do this for us:
+  // C++11 [class.copy]p7, p18:
+  //  If the class definition declares a move constructor or move assignment
+  //  operator, an implicitly declared copy constructor or copy assignment
+  //  operator is defined as deleted.
+
+  // Create a record and start defining it.
+  llvm::StringRef class_name = "S";
+  CompilerType t = clang_utils::createRecord(*m_ast, class_name);
+  m_ast->StartTagDeclarationDefinition(t);
+
+  // Create a move constructor that will delete the implicit copy constructor.
+  CompilerType return_type = m_ast->GetBasicType(lldb::eBasicTypeVoid);
+  CompilerType param_type = t.GetRValueReferenceType();
+  CompilerType function_type =
+      m_ast->CreateFunctionType(return_type, &param_type, /*num_params*/ 1,
+                                /*variadic=*/false, /*quals*/ 0U);
+  bool is_virtual = false;
+  bool is_static = false;
+  bool is_inline = false;
+  bool is_explicit = true;
+  bool is_attr_used = false;
+  bool is_artificial = false;
+  m_ast->AddMethodToCXXRecordType(
+      t.GetOpaqueQualType(), class_name, nullptr, function_type,
+      lldb::AccessType::eAccessPublic, is_virtual, is_static, is_inline,
+      is_explicit, is_attr_used, is_artificial);
+
+  // Complete the definition and check the created record.
+  m_ast->CompleteTagDeclarationDefinition(t);
+  auto *record = llvm::cast<CXXRecordDecl>(ClangUtil::GetAsTagDecl(t));
+  // We can't call defaultedCopyConstructorIsDeleted() as this requires that
+  // the Decl passes through Sema which will actually compute this field.
+  // Instead we check that there is no copy constructor declared by the user
+  // which only leaves a non-deleted defaulted copy constructor as an option
+  // that our record will have no simple copy constructor.
+  EXPECT_FALSE(record->hasUserDeclaredCopyConstructor());
+  EXPECT_FALSE(record->hasSimpleCopyConstructor());
+}
+
+TEST_F(TestClangASTContext, TestNotDeletingUserCopyCstrDueToMoveCStr) {
+  // Tests that we don't delete the a user-defined copy constructor when
+  // a move constructor is provided.
+  // See also the TestDeletingImplicitCopyCstrDueToMoveCStr test.
+  llvm::StringRef class_name = "S";
+  CompilerType t = clang_utils::createRecord(*m_ast, class_name);
+  m_ast->StartTagDeclarationDefinition(t);
+
+  CompilerType return_type = m_ast->GetBasicType(lldb::eBasicTypeVoid);
+  bool is_virtual = false;
+  bool is_static = false;
+  bool is_inline = false;
+  bool is_explicit = true;
+  bool is_attr_used = false;
+  bool is_artificial = false;
+  // Create a move constructor.
+  {
+    CompilerType param_type = t.GetRValueReferenceType();
+    CompilerType function_type =
+        m_ast->CreateFunctionType(return_type, &param_type, /*num_params*/ 1,
+                                  /*variadic=*/false, /*quals*/ 0U);
+    m_ast->AddMethodToCXXRecordType(
+        t.GetOpaqueQualType(), class_name, nullptr, function_type,
+        lldb::AccessType::eAccessPublic, is_virtual, is_static, is_inline,
+        is_explicit, is_attr_used, is_artificial);
+  }
+  // Create a copy constructor.
+  {
+    CompilerType param_type = t.GetLValueReferenceType().AddConstModifier();
+    CompilerType function_type =
+        m_ast->CreateFunctionType(return_type, &param_type, /*num_params*/ 1,
+                                  /*variadic=*/false, /*quals*/ 0U);
+    m_ast->AddMethodToCXXRecordType(
+        t.GetOpaqueQualType(), class_name, nullptr, function_type,
+        lldb::AccessType::eAccessPublic, is_virtual, is_static, is_inline,
+        is_explicit, is_attr_used, is_artificial);
+  }
+
+  // Complete the definition and check the created record.
+  m_ast->CompleteTagDeclarationDefinition(t);
+  auto *record = llvm::cast<CXXRecordDecl>(ClangUtil::GetAsTagDecl(t));
+  EXPECT_TRUE(record->hasUserDeclaredCopyConstructor());
+}
Index: lldb/source/Symbol/ClangASTContext.cpp
===================================================================
--- lldb/source/Symbol/ClangASTContext.cpp
+++ lldb/source/Symbol/ClangASTContext.cpp
@@ -7783,6 +7783,19 @@
     clang::TagDecl *tag_decl = tag_type->getDecl();
 
     if (auto *cxx_record_decl = llvm::dyn_cast<CXXRecordDecl>(tag_decl)) {
+      // If we have a move constructor declared but no copy constructor we
+      // need to explicitly mark it as deleted. Usually Sema would do this for
+      // us in Sema::DeclareImplicitCopyConstructor but we don't have a Sema
+      // when building an AST from debug information.
+      // See also:
+      // C++11 [class.copy]p7, p18:
+      //  If the class definition declares a move constructor or move assignment
+      //  operator, an implicitly declared copy constructor or copy assignment
+      //  operator is defined as deleted.
+      if (cxx_record_decl->hasUserDeclaredMoveConstructor() &&
+          cxx_record_decl->needsImplicitCopyConstructor())
+        cxx_record_decl->setImplicitCopyConstructorIsDeleted();
+
       if (!cxx_record_decl->isCompleteDefinition())
         cxx_record_decl->completeDefinition();
       cxx_record_decl->setHasLoadedFieldsFromExternalStorage(true);
Index: lldb/packages/Python/lldbsuite/test/commands/expression/deleting-implicit-copy-constructor/main.cpp
===================================================================
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/commands/expression/deleting-implicit-copy-constructor/main.cpp
@@ -0,0 +1,19 @@
+struct NoCopyCstr {
+  NoCopyCstr() {}
+  // No copy constructor but a move constructor means we have an
+  // implicitly deleted copy constructor (C++11 [class.copy]p7, p18).
+  NoCopyCstr(NoCopyCstr &&);
+};
+struct IndirectlyDeletedCopyCstr {
+  // This field indirectly deletes the implicit copy constructor.
+  NoCopyCstr field;
+  // Completing in the constructor will cause Sema to potentially declare
+  // the special members of IndirectlyDeletedCopyCstr. If we correctly
+  // set the deleted implicit copy constructor in NoCopyCstr then this
+  // should have propagated to this record and Clang won't crash.
+  IndirectlyDeletedCopyCstr() { //%self.dbg.GetCommandInterpreter().HandleCompletion("e ", len("e "), 0, -1, lldb.SBStringList())
+  }
+};
+int main() {
+  IndirectlyDeletedCopyCstr{};
+}
Index: lldb/packages/Python/lldbsuite/test/commands/expression/deleting-implicit-copy-constructor/TestDeletingImplicitCopyConstructor.py
===================================================================
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/commands/expression/deleting-implicit-copy-constructor/TestDeletingImplicitCopyConstructor.py
@@ -0,0 +1,4 @@
+from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
+
+lldbinline.MakeInlineTest(__file__, globals())
Index: lldb/packages/Python/lldbsuite/test/commands/expression/completion-crash1/main.cpp
===================================================================
--- lldb/packages/Python/lldbsuite/test/commands/expression/completion-crash1/main.cpp
+++ /dev/null
@@ -1,12 +0,0 @@
-namespace std {
-struct a {
-  a() {}
-  a(a &&);
-};
-template <class> struct au {
-  a ay;
-  ~au() { //%self.dbg.GetCommandInterpreter().HandleCompletion("e ", len("e "), 0, -1, lldb.SBStringList())
-  }
-};
-}
-int main() { std::au<int>{}; }
Index: lldb/packages/Python/lldbsuite/test/commands/expression/completion-crash1/TestCompletionCrash1.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/commands/expression/completion-crash1/TestCompletionCrash1.py
+++ /dev/null
@@ -1,4 +0,0 @@
-from lldbsuite.test import lldbinline
-from lldbsuite.test import decorators
-
-lldbinline.MakeInlineTest(__file__, globals(), [decorators.skipIf(bugnumber="rdar://53659341")])
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to