[PATCH] D157684: [clang][ASTImporter] Repeated friend templates are partially imported

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

balazske added inline comments.


================
Comment at: clang/lib/AST/ASTImporter.cpp:4072
+
+  if (auto *TSI = FD1->getFriendType())
+    return Importer.IsStructurallyEquivalent(
----------------
According to the coding rules `auto` should not be used here.


================
Comment at: clang/lib/AST/ASTImporter.cpp:4079
+      Importer.getNonEquivalentDecls(), getStructuralEquivalenceKind(Importer),
+      /* StrictTypeSpelling = */ false, /* Complain = */ false);
+  return Ctx.IsEquivalent(FD1, FD2);
----------------
This is a comparison in the same AST context ("From" side). `Importer.getNonEquivalentDecls()` returns a cache that is used at compares between "From" and "To" context. This is not valid for this use, you can simply pass an empty map instead, or add a new member that is used (only) here. `getStructuralEquivalenceKind(Importer)` is not needed for this compare, it can be always "Default".


================
Comment at: clang/lib/AST/ASTImporter.cpp:4089
 
-  T TypeOrDecl = GetCanTypeOrDecl(FD);
-
-  for (const FriendDecl *FoundFriend : RD->friends()) {
+  for (auto *FoundFriend : RD->friends()) {
     if (FoundFriend == FD) {
----------------
`auto` is not good here too.


================
Comment at: clang/lib/AST/ASTImporter.cpp:4114
   SmallVector<FriendDecl *, 2> ImportedEquivalentFriends;
-
-  while (ImportedFriend) {
-    bool Match = false;
-    if (D->getFriendDecl() && ImportedFriend->getFriendDecl()) {
-      Match =
-          IsStructuralMatch(D->getFriendDecl(), ImportedFriend->getFriendDecl(),
-                            /*Complain=*/false);
-    } else if (D->getFriendType() && ImportedFriend->getFriendType()) {
-      Match = Importer.IsStructurallyEquivalent(
-          D->getFriendType()->getType(),
-          ImportedFriend->getFriendType()->getType(), /*Complain=*/false);
-    }
-    if (Match)
+  for (auto *ImportedFriend : RD->friends())
+    if (IsEquivalentFriend(Importer, D, ImportedFriend))
----------------
`auto` should not be used here, this loop could be replaced by some generic "algorithm" function call (`llvm::copy_if`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157684