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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
