[PATCH] D48941: [ASTImporter] import FunctionDecl end locations

[Rafael Stahl via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC336523: [ASTImporter] import FunctionDecl end locations 
(authored by r.stahl, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D48941?vs=154215=154549#toc

Repository:
  rC Clang

https://reviews.llvm.org/D48941

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp


Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2554,16 +2554,16 @@
D->isInlineSpecified(),
FromConversion->isExplicit(),
D->isConstexpr(),
-   Importer.Import(D->getLocEnd()));
+   SourceLocation());
   } else if (auto *Method = dyn_cast(D)) {
 ToFunction = CXXMethodDecl::Create(Importer.getToContext(), 
cast(DC),
InnerLocStart,
NameInfo, T, TInfo,
Method->getStorageClass(),
Method->isInlineSpecified(),
D->isConstexpr(),
-   Importer.Import(D->getLocEnd()));
+   SourceLocation());
   } else {
 ToFunction = FunctionDecl::Create(Importer.getToContext(), DC,
   InnerLocStart,
@@ -2580,6 +2580,7 @@
   ToFunction->setVirtualAsWritten(D->isVirtualAsWritten());
   ToFunction->setTrivial(D->isTrivial());
   ToFunction->setPure(D->isPure());
+  ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
   Importer.Imported(D, ToFunction);
 
   // Set the parameters.
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1620,7 +1620,7 @@
   FromSM);
 }
 
-TEST_P(ASTImporterTestBase, DISABLED_ImportNestedMacro) {
+TEST_P(ASTImporterTestBase, ImportNestedMacro) {
   Decl *FromTU = getTuDecl(
   R"(
   #define FUNC_INT void declToImport


Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2554,16 +2554,16 @@
D->isInlineSpecified(),
FromConversion->isExplicit(),
D->isConstexpr(),
-   Importer.Import(D->getLocEnd()));
+   SourceLocation());
   } else if (auto *Method = dyn_cast(D)) {
 ToFunction = CXXMethodDecl::Create(Importer.getToContext(), 
cast(DC),
InnerLocStart,
NameInfo, T, TInfo,
Method->getStorageClass(),
Method->isInlineSpecified(),
D->isConstexpr(),
-   Importer.Import(D->getLocEnd()));
+   SourceLocation());
   } else {
 ToFunction = FunctionDecl::Create(Importer.getToContext(), DC,
   InnerLocStart,
@@ -2580,6 +2580,7 @@
   ToFunction->setVirtualAsWritten(D->isVirtualAsWritten());
   ToFunction->setTrivial(D->isTrivial());
   ToFunction->setPure(D->isPure());
+  ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
   Importer.Imported(D, ToFunction);
 
   // Set the parameters.
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1620,7 +1620,7 @@
   FromSM);
 }
 
-TEST_P(ASTImporterTestBase, DISABLED_ImportNestedMacro) {
+TEST_P(ASTImporterTestBase, ImportNestedMacro) {
   Decl *FromTU = getTuDecl(
   R"(
   #define FUNC_INT void declToImport
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48941: [ASTImporter] import FunctionDecl end locations

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin accepted this revision.
a_sidorin added a comment.

LGTM too. Thank you!


Repository:
  rC Clang

https://reviews.llvm.org/D48941



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48941: [ASTImporter] import FunctionDecl end locations

[Gabor Marton via Phabricator via cfe-commits]

martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rC Clang

https://reviews.llvm.org/D48941



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48941: [ASTImporter] import FunctionDecl end locations

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 154215.
r.stahl marked 2 inline comments as done.
r.stahl added a comment.

Alright, but then I would suggest to pass an invalid loc to the constructors 
instead to make it more explicit and save a map lookup in the import function.


Repository:
  rC Clang

https://reviews.llvm.org/D48941

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1620,7 +1620,7 @@
   FromSM);
 }
 
-TEST_P(ASTImporterTestBase, DISABLED_ImportNestedMacro) {
+TEST_P(ASTImporterTestBase, ImportNestedMacro) {
   Decl *FromTU = getTuDecl(
   R"(
   #define FUNC_INT void declToImport
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2563,16 +2563,16 @@
D->isInlineSpecified(),
FromConversion->isExplicit(),
D->isConstexpr(),
-   Importer.Import(D->getLocEnd()));
+   SourceLocation());
   } else if (auto *Method = dyn_cast(D)) {
 ToFunction = CXXMethodDecl::Create(Importer.getToContext(), 
cast(DC),
InnerLocStart,
NameInfo, T, TInfo,
Method->getStorageClass(),
Method->isInlineSpecified(),
D->isConstexpr(),
-   Importer.Import(D->getLocEnd()));
+   SourceLocation());
   } else {
 ToFunction = FunctionDecl::Create(Importer.getToContext(), DC,
   InnerLocStart,
@@ -2589,6 +2589,7 @@
   ToFunction->setVirtualAsWritten(D->isVirtualAsWritten());
   ToFunction->setTrivial(D->isTrivial());
   ToFunction->setPure(D->isPure());
+  ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
   Importer.Imported(D, ToFunction);
 
   // Set the parameters.


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1620,7 +1620,7 @@
   FromSM);
 }
 
-TEST_P(ASTImporterTestBase, DISABLED_ImportNestedMacro) {
+TEST_P(ASTImporterTestBase, ImportNestedMacro) {
   Decl *FromTU = getTuDecl(
   R"(
   #define FUNC_INT void declToImport
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2563,16 +2563,16 @@
D->isInlineSpecified(),
FromConversion->isExplicit(),
D->isConstexpr(),
-   Importer.Import(D->getLocEnd()));
+   SourceLocation());
   } else if (auto *Method = dyn_cast(D)) {
 ToFunction = CXXMethodDecl::Create(Importer.getToContext(), 
cast(DC),
InnerLocStart,
NameInfo, T, TInfo,
Method->getStorageClass(),
Method->isInlineSpecified(),
D->isConstexpr(),
-   Importer.Import(D->getLocEnd()));
+   SourceLocation());
   } else {
 ToFunction = FunctionDecl::Create(Importer.getToContext(), DC,
   InnerLocStart,
@@ -2589,6 +2589,7 @@
   ToFunction->setVirtualAsWritten(D->isVirtualAsWritten());
   ToFunction->setTrivial(D->isTrivial());
   ToFunction->setPure(D->isPure());
+  ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
   Importer.Imported(D, ToFunction);
 
   // Set the parameters.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48941: [ASTImporter] import FunctionDecl end locations

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin added a comment.

Hello Rafael.
This change is good, just some cleanup is needed.




Comment at: lib/AST/ASTImporter.cpp:2559
D->isImplicit());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
   } else if (auto *FromConversion = dyn_cast(D)) {

martong wrote:
> Why don't we need to call the `setRangeEnd()` when e.g. we create a 
> `CXXMethodDecl` or a `CXXConversionDecl`? Couldn't we just call this after 
> all these branches (at line 2587)?
I guess the reason is that they take LocEnd as ctor arguments. However, I think 
that having a single point is better.


Repository:
  rC Clang

https://reviews.llvm.org/D48941



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48941: [ASTImporter] import FunctionDecl end locations

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: lib/AST/ASTImporter.cpp:2559
D->isImplicit());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
   } else if (auto *FromConversion = dyn_cast(D)) {

Why don't we need to call the `setRangeEnd()` when e.g. we create a 
`CXXMethodDecl` or a `CXXConversionDecl`? Couldn't we just call this after all 
these branches (at line 2587)?


Repository:
  rC Clang

https://reviews.llvm.org/D48941



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48941: [ASTImporter] import FunctionDecl end locations

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

In https://reviews.llvm.org/D47698#1141871, @r.stahl wrote:

> improved code quality; added nested macro test. it "works", but is disabled 
> because it revealed another bug: the function end location is not imported. 
> will send a patch


Related to this.


Repository:
  rC Clang

https://reviews.llvm.org/D48941



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48941: [ASTImporter] import FunctionDecl end locations

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl created this revision.
r.stahl added reviewers: martong, a.sidorin, balazske, xazax.hun.
Herald added subscribers: cfe-commits, rnkovacs.

On constructors that do not take the end source location, it was not imported. 
Fixes test from https://reviews.llvm.org/D47698 / 
https://reviews.llvm.org/rC336269.


Repository:
  rC Clang

https://reviews.llvm.org/D48941

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1620,7 +1620,7 @@
   FromSM);
 }
 
-TEST_P(ASTImporterTestBase, DISABLED_ImportNestedMacro) {
+TEST_P(ASTImporterTestBase, ImportNestedMacro) {
   Decl *FromTU = getTuDecl(
   R"(
   #define FUNC_INT void declToImport
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2533,6 +2533,7 @@
 D->isInlineSpecified(), 
 D->isImplicit(),
 D->isConstexpr());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
 if (unsigned NumInitializers = FromConstructor->getNumCtorInitializers()) {
   SmallVector CtorInitializers;
   for (auto *I : FromConstructor->inits()) {
@@ -2555,6 +2556,7 @@
NameInfo, T, TInfo,
D->isInlineSpecified(),
D->isImplicit());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
   } else if (auto *FromConversion = dyn_cast(D)) {
 ToFunction = CXXConversionDecl::Create(Importer.getToContext(), 
cast(DC),
@@ -2580,6 +2582,7 @@
   D->isInlineSpecified(),
   D->hasWrittenPrototype(),
   D->isConstexpr());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
   }
 
   // Import the qualifier, if any.


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1620,7 +1620,7 @@
   FromSM);
 }
 
-TEST_P(ASTImporterTestBase, DISABLED_ImportNestedMacro) {
+TEST_P(ASTImporterTestBase, ImportNestedMacro) {
   Decl *FromTU = getTuDecl(
   R"(
   #define FUNC_INT void declToImport
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2533,6 +2533,7 @@
 D->isInlineSpecified(), 
 D->isImplicit(),
 D->isConstexpr());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
 if (unsigned NumInitializers = FromConstructor->getNumCtorInitializers()) {
   SmallVector CtorInitializers;
   for (auto *I : FromConstructor->inits()) {
@@ -2555,6 +2556,7 @@
NameInfo, T, TInfo,
D->isInlineSpecified(),
D->isImplicit());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
   } else if (auto *FromConversion = dyn_cast(D)) {
 ToFunction = CXXConversionDecl::Create(Importer.getToContext(), 
cast(DC),
@@ -2580,6 +2582,7 @@
   D->isInlineSpecified(),
   D->hasWrittenPrototype(),
   D->isConstexpr());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
   }
 
   // Import the qualifier, if any.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits