[PATCH] D155661: [clang][ASTImporter] Fix friend class template import within dependent context

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

balazske added inline comments.


================
Comment at: clang/lib/AST/ASTImporter.cpp:2862-2866
+  bool ShouldAddRedecl = !(IsFriendTemplate && IsDependentContext);
+
   // We may already have a record of the same name; try to find and match it.
   RecordDecl *PrevDecl = nullptr;
   if (!DC->isFunctionOrMethod() && !D->isLambda()) {
----------------
The code seems to work but I was confused by the different conditions used (is it possible that `IsFriendTemplate` and `ShouldAddRedecl` is true at the same time?). I had the observation that if `ShouldAddRedecl` is false we do not need the whole search for previous decl. At a friend template we shall not find a definition, the loop would just find the last declaration (this value is not used). So I have the idea of this code (could not find the "suggest edit" command):

```
  bool DependentFriend = IsFriendTemplate && IsDependentContext;

  // We may already have a record of the same name; try to find and match it.
  RecordDecl *PrevDecl = nullptr;
  if (!DependentFriend && !DC->isFunctionOrMethod() && !D->isLambda()) {

```


================
Comment at: clang/lib/AST/ASTImporter.cpp:2904
 
-        if (IsStructuralMatch(D, FoundRecord)) {
+        if (IsFriendTemplate || IsStructuralMatch(D, FoundRecord)) {
           RecordDecl *FoundDef = FoundRecord->getDefinition();
----------------
This change is not needed if the code above is used.


================
Comment at: clang/lib/AST/ASTImporter.cpp:2976
+              D2CXX, D, Importer.getToContext(), D->getTagKind(), DC,
+              *BeginLocOrErr, Loc, Name.getAsIdentifierInfo(),
+              ShouldAddRedecl ? cast_or_null<CXXRecordDecl>(PrevDecl)
----------------
This change is not needed if the code above is used.


================
Comment at: clang/lib/AST/ASTImporter.cpp:5805-5812
+  bool IsDependentContext = DC != LexicalDC ? LexicalDC->isDependentContext()
+                                            : DC->isDependentContext();
+  bool ShouldAddRedecl = !(IsFriendTemplate && IsDependentContext);
+
   ClassTemplateDecl *FoundByLookup = nullptr;
 
   // We may already have a template of the same name; try to find and match it.
----------------
Similar change here:
```
  bool DependentFriend = IsFriendTemplate && IsDependentContext;

  ClassTemplateDecl *FoundByLookup = nullptr;

  // We may already have a template of the same name; try to find and match it.
  if (!DependentFriend && !DC->isFunctionOrMethod()) {
```
`IsFriendTemplate` and `ShouldAddRedecl` is not needed (no changes in the later lines).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155661