[PATCH] D125340: [clang][NFC][AST] rename the ImportError to ASTImportError

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

Conflict markers fixed by 6ccc2733e72017999a94c10147a71ff595286080 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125340

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

[PATCH] D125340: [clang][NFC][AST] rename the ImportError to ASTImportError

[Shivam Rajput via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9d637956b792: [clang][NFC][AST] rename the ImportError to 
ASTImportError (authored by phyBrackets).

Changed prior to commit:
  https://reviews.llvm.org/D125340?vs=428508=436881#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125340

Files:
  clang/include/clang/AST/ASTImportError.h
  clang/include/clang/AST/ASTImporter.h
  clang/include/clang/AST/ASTImporterSharedState.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/unittests/AST/ASTImporterFixtures.h
  clang/unittests/AST/ASTImporterODRStrategiesTest.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -3208,8 +3208,8 @@
 
   auto ToBlockOrError = importOrError(FromBlock, Lang_CXX03);
 
-  const auto ExpectUnsupportedConstructError = [](const ImportError ) {
-EXPECT_EQ(ImportError::UnsupportedConstruct, Error.Error);
+  const auto ExpectUnsupportedConstructError = [](const ASTImportError ) {
+EXPECT_EQ(ASTImportError::UnsupportedConstruct, Error.Error);
   };
   llvm::handleAllErrors(ToBlockOrError.takeError(),
 ExpectUnsupportedConstructError);
@@ -5469,9 +5469,9 @@
 
   // But an error is set to the counterpart in the "from" context.
   ASTImporter *Importer = findFromTU(FromSpec)->Importer.get();
-  Optional OptErr = Importer->getImportDeclErrorIfAny(FromSpec);
+  Optional OptErr = Importer->getImportDeclErrorIfAny(FromSpec);
   ASSERT_TRUE(OptErr);
-  EXPECT_EQ(OptErr->Error, ImportError::NameConflict);
+  EXPECT_EQ(OptErr->Error, ASTImportError::NameConflict);
 }
 
 // Check a case when a new AST node is created but not linked to the AST before
@@ -5493,9 +5493,9 @@
   0u);
 
   ASTImporter *Importer = findFromTU(FromFoo)->Importer.get();
-  Optional OptErr = Importer->getImportDeclErrorIfAny(FromFoo);
+  Optional OptErr = Importer->getImportDeclErrorIfAny(FromFoo);
   ASSERT_TRUE(OptErr);
-  EXPECT_EQ(OptErr->Error, ImportError::UnsupportedConstruct);
+  EXPECT_EQ(OptErr->Error, ASTImportError::UnsupportedConstruct);
 }
 
 // Check a case when a new AST node is created and linked to the AST before
@@ -5526,12 +5526,13 @@
   // An error is set to the counterpart in the "from" context both for the fwd
   // decl and the definition.
   ASTImporter *Importer = findFromTU(FromProto)->Importer.get();
-  Optional OptErr = Importer->getImportDeclErrorIfAny(FromProto);
+  Optional OptErr =
+  Importer->getImportDeclErrorIfAny(FromProto);
   ASSERT_TRUE(OptErr);
-  EXPECT_EQ(OptErr->Error, ImportError::UnsupportedConstruct);
+  EXPECT_EQ(OptErr->Error, ASTImportError::UnsupportedConstruct);
   OptErr = Importer->getImportDeclErrorIfAny(FromDef);
   ASSERT_TRUE(OptErr);
-  EXPECT_EQ(OptErr->Error, ImportError::UnsupportedConstruct);
+  EXPECT_EQ(OptErr->Error, ASTImportError::UnsupportedConstruct);
 }
 
 // An error should be set for a class if we cannot import one member.
@@ -5551,16 +5552,16 @@
   // An error is set for X.
   EXPECT_FALSE(ImportedX);
   ASTImporter *Importer = findFromTU(FromX)->Importer.get();
-  Optional OptErr = Importer->getImportDeclErrorIfAny(FromX);
+  Optional OptErr = Importer->getImportDeclErrorIfAny(FromX);
   ASSERT_TRUE(OptErr);
-  EXPECT_EQ(OptErr->Error, ImportError::UnsupportedConstruct);
+  EXPECT_EQ(OptErr->Error, ASTImportError::UnsupportedConstruct);
 
   // An error is set for f().
   auto *FromF = FirstDeclMatcher().match(
   FromTU, cxxMethodDecl(hasName("f")));
   OptErr = Importer->getImportDeclErrorIfAny(FromF);
   ASSERT_TRUE(OptErr);
-  EXPECT_EQ(OptErr->Error, ImportError::UnsupportedConstruct);
+  EXPECT_EQ(OptErr->Error, ASTImportError::UnsupportedConstruct);
   // And any subsequent import should fail.
   CXXMethodDecl *ImportedF = Import(FromF, Lang_CXX03);
   EXPECT_FALSE(ImportedF);
@@ -5618,7 +5619,7 @@
 
   // An error is set to the templated CXXRecordDecl of F.
   ASTImporter *Importer = findFromTU(FromFRD)->Importer.get();
-  Optional OptErr = Importer->getImportDeclErrorIfAny(FromFRD);
+  Optional OptErr = Importer->getImportDeclErrorIfAny(FromFRD);
   EXPECT_TRUE(OptErr);
 
   // An error is set to A.
@@ -5676,7 +5677,7 @@
   // There is no error set for X.
   EXPECT_TRUE(ImportedX);
   ASTImporter *Importer = findFromTU(FromX)->Importer.get();
-  Optional OptErr = Importer->getImportDeclErrorIfAny(FromX);
+  Optional OptErr = Importer->getImportDeclErrorIfAny(FromX);
   ASSERT_FALSE(OptErr);
 
   // An error is set for f().
@@ -5684,7 +5685,7 @@
   FromTU, functionDecl(hasName("f")));
   OptErr = Importer->getImportDeclErrorIfAny(FromF);
   ASSERT_TRUE(OptErr);
-  EXPECT_EQ(OptErr->Error, ImportError::UnsupportedConstruct);
+  

[PATCH] D125340: [clang][NFC][AST] rename the ImportError to ASTImportError

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

LLDB should be at least compilable after the change. If it has an internal 
`ImportError` we must not change it. If it uses the renamed class we should 
rename it, or make an "using" alias and no rename (but not this is the best 
solution).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125340

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

[PATCH] D125340: [clang][NFC][AST] rename the ImportError to ASTImportError

[Gabor Marton via Phabricator via cfe-commits]

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

Yeah, okay, this patch makes sense now that I've seen a clash with python's 
ImportError  . I've checked 
lldb c++ files and `ImportError` is not used. LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125340

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

[PATCH] D125340: [clang][NFC][AST] rename the ImportError to ASTImportError

[Shivam Rajput via Phabricator via cfe-commits]

phyBrackets added a comment.

In D125340#3506210 , @balazske wrote:

> I found one other place in **LibASTImporter.rst** where `ImportError` is 
> used. LLDB should be checked too.

Yeah , I update the file **LibASTImporter.rst**, I'm not sure if there is need 
to change the ImportError that are used in lldb ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125340

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

[PATCH] D125340: [clang][NFC][AST] rename the ImportError to ASTImportError

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

I found one other place in **LibASTImporter.rst** where `ImportError` is used. 
LLDB should be checked too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125340

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

[PATCH] D125340: [clang][NFC][AST] rename the ImportError to ASTImportError

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

Initially the `ImportError` was very related to `ASTImporter` but now the class 
is used at other less related places and files. `ImportError` is a too generic 
name for this unless it can be used at any other "import" in clang which is not 
the case (the class can be extended with other special data about the import 
error). Maybe the whole ASTImporter should go into a new namespace, specially 
if `ASTImporter.cpp` would be split into parts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125340

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

[PATCH] D125340: [clang][NFC][AST] rename the ImportError to ASTImportError

[Shivam Rajput via Phabricator via cfe-commits]

phyBrackets added a comment.

In D125340#3505723 , @martong wrote:

> What are the benefits of this renaming? I mean is there a name clash? Do we 
> have another kind of "import" in Clang or in some of the dependent projects, 
> don't we?

AS it suggested by @balazske in the previous patch , and I also think if we 
moved the ImportError into it's own header then it's better to be named as 
ASTImportError .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125340

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

[PATCH] D125340: [clang][NFC][AST] rename the ImportError to ASTImportError

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

What are the benefits of this renaming? I mean is there a name clash? Do we 
have another kind of "import" in Clang or in some of the dependent projects, 
don't we?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125340

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

[PATCH] D125340: [clang][NFC][AST] rename the ImportError to ASTImportError

[Shivam Rajput via Phabricator via cfe-commits]

phyBrackets created this revision.
Herald added a subscriber: martong.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: All.
phyBrackets requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

this patch is the continuation of my previous patch regarding the ImportError 
in ASTImportError.h


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125340

Files:
  clang/include/clang/AST/ASTImportError.h
  clang/include/clang/AST/ASTImporter.h
  clang/include/clang/AST/ASTImporterSharedState.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/unittests/AST/ASTImporterFixtures.h
  clang/unittests/AST/ASTImporterODRStrategiesTest.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -3178,8 +3178,8 @@
 
   auto ToBlockOrError = importOrError(FromBlock, Lang_CXX03);
 
-  const auto ExpectUnsupportedConstructError = [](const ImportError ) {
-EXPECT_EQ(ImportError::UnsupportedConstruct, Error.Error);
+  const auto ExpectUnsupportedConstructError = [](const ASTImportError ) {
+EXPECT_EQ(ASTImportError::UnsupportedConstruct, Error.Error);
   };
   llvm::handleAllErrors(ToBlockOrError.takeError(),
 ExpectUnsupportedConstructError);
@@ -5435,9 +5435,9 @@
 
   // But an error is set to the counterpart in the "from" context.
   ASTImporter *Importer = findFromTU(FromSpec)->Importer.get();
-  Optional OptErr = Importer->getImportDeclErrorIfAny(FromSpec);
+  Optional OptErr = Importer->getImportDeclErrorIfAny(FromSpec);
   ASSERT_TRUE(OptErr);
-  EXPECT_EQ(OptErr->Error, ImportError::NameConflict);
+  EXPECT_EQ(OptErr->Error, ASTImportError::NameConflict);
 }
 
 // Check a case when a new AST node is created but not linked to the AST before
@@ -5459,9 +5459,9 @@
   0u);
 
   ASTImporter *Importer = findFromTU(FromFoo)->Importer.get();
-  Optional OptErr = Importer->getImportDeclErrorIfAny(FromFoo);
+  Optional OptErr = Importer->getImportDeclErrorIfAny(FromFoo);
   ASSERT_TRUE(OptErr);
-  EXPECT_EQ(OptErr->Error, ImportError::UnsupportedConstruct);
+  EXPECT_EQ(OptErr->Error, ASTImportError::UnsupportedConstruct);
 }
 
 // Check a case when a new AST node is created and linked to the AST before
@@ -5492,12 +5492,13 @@
   // An error is set to the counterpart in the "from" context both for the fwd
   // decl and the definition.
   ASTImporter *Importer = findFromTU(FromProto)->Importer.get();
-  Optional OptErr = Importer->getImportDeclErrorIfAny(FromProto);
+  Optional OptErr =
+  Importer->getImportDeclErrorIfAny(FromProto);
   ASSERT_TRUE(OptErr);
-  EXPECT_EQ(OptErr->Error, ImportError::UnsupportedConstruct);
+  EXPECT_EQ(OptErr->Error, ASTImportError::UnsupportedConstruct);
   OptErr = Importer->getImportDeclErrorIfAny(FromDef);
   ASSERT_TRUE(OptErr);
-  EXPECT_EQ(OptErr->Error, ImportError::UnsupportedConstruct);
+  EXPECT_EQ(OptErr->Error, ASTImportError::UnsupportedConstruct);
 }
 
 // An error should be set for a class if we cannot import one member.
@@ -5517,16 +5518,16 @@
   // An error is set for X.
   EXPECT_FALSE(ImportedX);
   ASTImporter *Importer = findFromTU(FromX)->Importer.get();
-  Optional OptErr = Importer->getImportDeclErrorIfAny(FromX);
+  Optional OptErr = Importer->getImportDeclErrorIfAny(FromX);
   ASSERT_TRUE(OptErr);
-  EXPECT_EQ(OptErr->Error, ImportError::UnsupportedConstruct);
+  EXPECT_EQ(OptErr->Error, ASTImportError::UnsupportedConstruct);
 
   // An error is set for f().
   auto *FromF = FirstDeclMatcher().match(
   FromTU, cxxMethodDecl(hasName("f")));
   OptErr = Importer->getImportDeclErrorIfAny(FromF);
   ASSERT_TRUE(OptErr);
-  EXPECT_EQ(OptErr->Error, ImportError::UnsupportedConstruct);
+  EXPECT_EQ(OptErr->Error, ASTImportError::UnsupportedConstruct);
   // And any subsequent import should fail.
   CXXMethodDecl *ImportedF = Import(FromF, Lang_CXX03);
   EXPECT_FALSE(ImportedF);
@@ -5584,7 +5585,7 @@
 
   // An error is set to the templated CXXRecordDecl of F.
   ASTImporter *Importer = findFromTU(FromFRD)->Importer.get();
-  Optional OptErr = Importer->getImportDeclErrorIfAny(FromFRD);
+  Optional OptErr = Importer->getImportDeclErrorIfAny(FromFRD);
   EXPECT_TRUE(OptErr);
 
   // An error is set to A.
@@ -5642,7 +5643,7 @@
   // There is no error set for X.
   EXPECT_TRUE(ImportedX);
   ASTImporter *Importer = findFromTU(FromX)->Importer.get();
-  Optional OptErr = Importer->getImportDeclErrorIfAny(FromX);
+  Optional OptErr = Importer->getImportDeclErrorIfAny(FromX);
   ASSERT_FALSE(OptErr);
 
   // An error is set for f().
@@ -5650,7 +5651,7 @@
   FromTU, functionDecl(hasName("f")));
   OptErr = Importer->getImportDeclErrorIfAny(FromF);
   ASSERT_TRUE(OptErr);
-  EXPECT_EQ(OptErr->Error,