[PATCH] D75740: [ASTImporter] Corrected import of repeated friend declarations.

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

LGTM WDYT @teemperor



================
Comment at: clang/lib/AST/ASTImporter.cpp:3638
+  /// Number of similar looking friends.
+  unsigned int TotalCount;
+  /// Index of the specific FriendDecl.
----------------
`uint32_t`

Is there a reason to not prefer fixed width integer types?


================
Comment at: clang/lib/AST/ASTImporter.cpp:3640
+  /// Index of the specific FriendDecl.
+  unsigned int IndexOfDecl;
+};
----------------
`uint32_t`


================
Comment at: clang/lib/AST/ASTImporter.cpp:3644
+FriendCountAndIndex getFriendCountAndPosition(const FriendDecl *FD) {
+  unsigned int FriendCount = 0;
+  llvm::Optional<unsigned int> FriendPosition;
----------------
`uint32_t`


================
Comment at: clang/lib/AST/ASTImporter.cpp:3660
+    const Decl *CanDeclOfFriend = FD->getFriendDecl()->getCanonicalDecl();
+    for (const FriendDecl *FoundFriend : RD->friends()) {
+      if (FoundFriend == FD) {
----------------
Can we refactor to not repeat code? They look identical. Duplicate code is a source for errors when once piece of code is fixed both the other is not etc


================
Comment at: clang/unittests/AST/ASTImporterTest.cpp:4009
+TEST_P(ImportFriendClasses, ImportOfRepeatedFriendType) {
+  auto Code =
+      R"(
----------------
a_sidorin wrote:
> clang-tidy wants this to be `const auto* Code`.
Does `auto` really buy us anything here? Why not use use `const char*`? 


================
Comment at: clang/unittests/AST/ASTImporterTest.cpp:4034
+TEST_P(ImportFriendClasses, ImportOfRepeatedFriendDecl) {
+  auto Code =
+      R"(
----------------
a_sidorin wrote:
> clang-tidy wants this to be `const auto* Code`.
`const char*`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75740