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

[Gabor Marton via Phabricator via cfe-commits]

martong created this revision.
martong added reviewers: shafik, a_sidorin.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a project: clang.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66999

Files:
  clang/lib/AST/ASTImporter.cpp


Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -3447,12 +3447,15 @@
         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);
     }
   }
@@ -3518,12 +3521,15 @@
       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);
     }
   }
@@ -5248,7 +5254,12 @@
         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);
     }
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D66999.218074.patch
Type: text/x-patch
Size: 2437 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190830/b5ed77f0/attachment.bin>