[ASTImporter][NFC] Add comments to code where we cannot use HandleNameConflict

[martong]

Summary

This is the continuation of https://reviews.llvm.org/D59692 where we started to
use HandleNameConflict in almost all cases.
The remaining cases are dealt with here.

[shafik]

clang/lib/AST/ASTImporter.cpp

3454

Do we have an example that hits this case?

5262

I believe this is ill-formed no diagnostic required.

clang/lib/AST/ASTImporter.cpp

if (FieldDecl *FoundField = dyn_cast<FieldDecl>(FoundDecl)) {
               consumeError(ToInitializerOrErr.takeError());
               return FoundField;
             }
           }
         }
         return FoundField;
       }
 
-      // FIXME: Why is this case not handled with calling HandleNameConflict?
       Importer.ToDiag(Loc, diag::warn_odr_field_type_inconsistent)
         << Name << D->getType() << FoundField->getType();
       Importer.ToDiag(FoundField->getLocation(), diag::note_odr_value_here)
         << FoundField->getType();
-
+      // This case is not handled with HandleNameConflict, because by the time
+      // when we reach here, the enclosing class is already imported. If there
+      // was any NameConflict during the import of that class it is already
+      // handled. (Otherwise we have a contradiction between the structural
+      // equivalency check of that class and its field [this field].)
       return make_error<ImportError>(ImportError::NameConflict);
     }
   }
 
   QualType ToType;
   TypeSourceInfo *ToTInfo;
   Expr *ToBitWidth;
   SourceLocation ToInnerLocStart;
if (auto *FoundField = dyn_cast<IndirectFieldDecl>(FoundDecls[I])) {
         Importer.MapImported(D, FoundField);
         return FoundField;
       }
 
       // If there are more anonymous fields to check, continue.
       if (!Name && I < N-1)
         continue;
 
-      // FIXME: Why is this case not handled with calling HandleNameConflict?
       Importer.ToDiag(Loc, diag::warn_odr_field_type_inconsistent)
         << Name << D->getType() << FoundField->getType();
       Importer.ToDiag(FoundField->getLocation(), diag::note_odr_value_here)
         << FoundField->getType();
-
+      // This case is not handled with HandleNameConflict, because by the time
+      // when we reach here, the enclosing class is already imported. If there
+      // was any NameConflict during the import of that class it is already
+      // handled. (Otherwise we have a contradiction between the structural
+      // equivalency check of that class and its field [this field].)
       return make_error<ImportError>(ImportError::NameConflict);
     }
   }
 
   // Import the type.
   auto TypeOrErr = import(D->getType());
   if (!TypeOrErr)
     return TypeOrErr.takeError();
if (IsStructuralMatch(D, PrevDecl)) {
         // TODO Import instantiated default arguments.
         // TODO Import instantiated exception specifications.
         //
         // Generally, ASTCommon.h/DeclUpdateKind enum gives a very good hint
         // what else could be fused during an AST merge.
         return PrevDecl;
       }
     } else { // ODR violation.
-      // FIXME HandleNameConflict
+      // This is not a name conflict, we have specializations with same
+      // parameters, but with different definitions.
+      // HandleNameConflict cannot be used here and Liberal strategy could not
+      // work because we cannot register two specializations for a primary
+      // template.
+      // FIXME Perhaps return with a different error?
       return make_error<ImportError>(ImportError::NameConflict);
     }
   }
 
   // Import the location of this declaration.
   ExpectedSLoc BeginLocOrErr = import(D->getBeginLoc());
   if (!BeginLocOrErr)
     return BeginLocOrErr.takeError();