[Lldb-commits] [PATCH] D79811: WIP: Reenable creation of artificial methods in AddMethodToCXXRecordType(...)

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik created this revision.
shafik added reviewers: clayborg, jingham, aprantl, teemperor.
Herald added subscribers: kbarton, nemanjai.
shafik planned changes to this revision.
Herald added a subscriber: wuzish.
shafik requested review of this revision.
shafik retitled this revision from "Reenable creation of artificial methods in AddMethodToCXXRecordType(...)" to "WIP: Reenable creation of artificial methods in AddMethodToCXXRecordType(...)".
shafik added a comment.
shafik added a reviewer: labath.

I labeled this as WIP in progress since I expect some discussion and maybe a test case for which the change fails.


The framework for dealing with artificial methods is in place in `AddMethodToCXXRecordType(...)` but was gated by an early exit at some point.

This was due to implicit methods being emitted in some translations units but not others and then sometimes during import via the `ASTImporter` this would fail since they would no longer be structurally equivalent.

We have not been able to come up with a reproducer for the original issue and after fixing D79251 <https://reviews.llvm.org/D79251> which was accidentally working the test suite passes with this change and we have cases that are broken without this change.


https://reviews.llvm.org/D79811

Files:
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/test/API/lang/cpp/constructors/TestCppConstructors.py
  lldb/test/API/lang/cpp/constructors/main.cpp


Index: lldb/test/API/lang/cpp/constructors/main.cpp
===================================================================
--- lldb/test/API/lang/cpp/constructors/main.cpp
+++ lldb/test/API/lang/cpp/constructors/main.cpp
@@ -1,5 +1,14 @@
+int flag=0;
+
+struct ClassForSideEffect {
+  ClassForSideEffect() { flag=1; }
+  ClassForSideEffect(int x) { flag=x; }
+};
+
 struct ClassWithImplicitCtor {
   int foo() { return 1; }
+  int x=10;
+  ClassForSideEffect cse = ClassForSideEffect(100);
 };
 
 struct ClassWithDefaultedCtor {
Index: lldb/test/API/lang/cpp/constructors/TestCppConstructors.py
===================================================================
--- lldb/test/API/lang/cpp/constructors/TestCppConstructors.py
+++ lldb/test/API/lang/cpp/constructors/TestCppConstructors.py
@@ -18,6 +18,9 @@
         self.expect_expr("ClassWithDeletedCtor().value", result_type="int", result_value="6")
         self.expect_expr("ClassWithDeletedDefaultCtor(7).value", result_type="int", result_value="7")
 
+        self.expect_expr("ClassWithImplicitCtor a{}; a.x", result_type="int", result_value="10")
+        self.expect_expr("ClassWithImplicitCtor a{}; flag", result_type="int", result_value="100")
+
         # FIXME: It seems we try to call the non-existent default constructor here which is wrong.
         self.expect("expr ClassWithDefaultedCtor().foo()", error=True, substrs="Couldn't lookup symbols:")
 
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===================================================================
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -7306,9 +7306,6 @@
   clang::CXXDestructorDecl *cxx_dtor_decl(nullptr);
   clang::CXXConstructorDecl *cxx_ctor_decl(nullptr);
 
-  if (is_artificial)
-    return nullptr; // skip everything artificial
-
   const clang::ExplicitSpecifier explicit_spec(
       nullptr /*expr*/, is_explicit ? clang::ExplicitSpecKind::ResolvedTrue
                                     : clang::ExplicitSpecKind::ResolvedFalse);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D79811.263511.patch
Type: text/x-patch
Size: 2080 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20200512/48fd6742/attachment-0001.bin>