[clang][ASTImporter] Import source location of explicit object parameter instead of copying it

[Michael137]

We used to copy the SourceLocation instead of importing it, which isn't correct since the SourceManager's of the source and target ASTContext might differ.

Also adds test that confirms that we import the explicit object parameter location for ParmVarDecls. This is how Clang determines whether a parameter isExplicitObjectParamater. The LLDB expression evaluator relies on this for calling "explicit object member functions".

[llvmbot]

@llvm/pr-subscribers-clang

Author: Michael Buch (Michael137)

Changes

Adds test that confirms that we import the explicit object parameter location for ParmVarDecls. This is how Clang determines whether a parameter isExplicitObjectParamater. The LLDB expression evaluator relies on this for calling "explicit object member functions".

---

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

1 Files Affected:

• (modified) clang/unittests/AST/ASTImporterTest.cpp (+20)

diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp
index 791248e7a394f1..187fe651f6f19a 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -3441,6 +3441,7 @@ TEST_P(ASTImporterOptionSpecificTestBase, ImportParmVarDecl) {
   ASSERT_TRUE(FromVar);
   ASSERT_TRUE(FromVar->hasUninstantiatedDefaultArg());
   ASSERT_TRUE(FromVar->getUninstantiatedDefaultArg());
+  ASSERT_FALSE(FromVar->isExplicitObjectParameter());
 
   const auto *ToVar = Import(FromVar, Lang_CXX11);
   EXPECT_TRUE(ToVar);
@@ -3448,6 +3449,25 @@ TEST_P(ASTImporterOptionSpecificTestBase, ImportParmVarDecl) {
   EXPECT_TRUE(ToVar->getUninstantiatedDefaultArg());
   EXPECT_NE(FromVar->getUninstantiatedDefaultArg(),
             ToVar->getUninstantiatedDefaultArg());
+  EXPECT_FALSE(ToVar->isExplicitObjectParameter());
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, ImportParmVarDecl_Explicit) {
+  const auto *Code = R"(
+    struct Wrapper {
+      Wrapper(this Wrapper) {}
+    };
+    )";
+  Decl *FromTU = getTuDecl(Code, Lang_CXX23);
+  auto *FromVar = FirstDeclMatcher<ParmVarDecl>().match(FromTU, parmVarDecl());
+  ASSERT_TRUE(FromVar);
+  ASSERT_TRUE(FromVar->isExplicitObjectParameter());
+
+  const auto *ToVar = Import(FromVar, Lang_CXX23);
+  EXPECT_TRUE(ToVar);
+  EXPECT_TRUE(ToVar->isExplicitObjectParameter());
+  EXPECT_EQ(ToVar->getExplicitObjectParamThisLoc(),
+            FromVar->getExplicitObjectParamThisLoc());
 }
 
 TEST_P(ASTImporterOptionSpecificTestBase, ImportOfNonEquivalentField) {

[balazske]

It looks not correct to copy a SourceLocation from the "from" into the "to" context. They have a different source manager instance. Function ASTNodeImporter::ImportDefaultArgOfParmVarDecl is the place where this is done. Copy of getExplicitObjectParamThisLoc should be replaced with normal import.

[Michael137]

Yea good catch, i was wondering about that when reading it initially. I'll adjust it

[Michael137]

Latest commit updates code to import the location.