[PATCH] D64480: [ASTImporter] Added visibility context check for TypedefNameDecl.

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.


================
Comment at: lib/AST/ASTImporter.cpp:949
+    return Importer.GetFromTU(Found) == From->getTranslationUnitDecl();
+  return From->isInAnonymousNamespace() == Found->isInAnonymousNamespace();
+}
----------------
I am not sure what case this covers? Can you elaborate? I see the case the condition above is catching.


================
Comment at: unittests/AST/ASTImporterVisibilityTest.cpp:348
+        ::testing::Values(
+            std::make_tuple(ExternTypedef, ExternTypedef, ExpectLink),
+            std::make_tuple(ExternTypedef, AnonTypedef, ExpectNotLink),
----------------
Perhaps `ExpectLink` would be better as `ExpectLinkedDeclChain` and the same for `ExpectNotLink`.

It was actually confusing at first because link is an overload term.


================
Comment at: unittests/AST/ASTImporterVisibilityTest.cpp:352
+            std::make_tuple(AnonTypedef, AnonTypedef, ExpectNotLink))), );
+INSTANTIATE_TEST_CASE_P(
+    ParameterizedTests, ImportTypeAliasVisibility,
----------------
We need another set of tests for typedefs and using e.g. `ExternTypedef` and `ExternUsing` since they are equivalent. 


Repository:
  rC Clang

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

https://reviews.llvm.org/D64480