[PATCH] D74554: [ASTImporter] Added visibility check for scoped enums.

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

balazske marked an inline comment as done.
balazske added inline comments.


================
Comment at: clang/lib/AST/ASTImporter.cpp:2600
+          EnumDecl *FoundDef = FoundEnum->getDefinition();
+          if (D->isThisDeclarationADefinition() && FoundDef)
+            return Importer.MapImported(D, FoundDef);
----------------
martong wrote:
> shafik wrote:
> > Can you explain why we need to check `D->isThisDeclarationADefinition()` 
> > 
> > Does the test added hit all the combination of cases?
> By the time of this check, we already know that D and the existing (found) decl are structurally equivalent. If they are both definitions then they must be the same, so we keep on with the existing. Actually, this is not unorthodox with enums, this is the pattern that we follow with all kind of declarations. 
Good observation: The `enum class` case could be added to the generic redecl and ODR tests. This makes the single test in **ASTImporterTest.cpp** unnecessary, this case should be covered by the tests in **ASTImporterGenericRedeclTest.cpp**.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74554