[clang] [clang][Modules] Diagnosing Module Redefinition Across ModuleMaps (PR #190085)

[Qiongsi Wu via cfe-commits]

================
@@ -1756,9 +1756,24 @@ void ModuleMapLoader::handleModuleDecl(const modulemap::ModuleDecl &MD) {
   if (Module *Existing = Map.lookupModuleQualified(ModuleName, ActiveModule)) {
     // We might see a (re)definition of a module that we already have a
     // definition for in four cases:
-    //  - If we loaded one definition from an AST file and we've just found a
-    //    corresponding definition in a module map file, or
+    //  - If we loaded one definition from an AST file and we've just found the
+    //    corresponding definition in the same module map file, or
----------------
qiongsiwu wrote:

I think of this as changing the scope of loaded from AST file, rather than the 5th case, because in the case we try to cover (e.g. https://github.com/llvm/llvm-project/pull/190085/changes#diff-937e85d6ec1af0867c29730516fbb534d9d803e7432833b2b4407822b95df6ebR1), `LoadedFromASTFile` is true for module `b` when we load `a`'s module map. If we find out that `b` is from a module file, we [skip the duplicating definition check](https://github.com/llvm/llvm-project/pull/190085/changes#diff-d94525ab4a88e7d89af25aefc58749191bd3482aecb157c8d13c7efde65e61e1R1805) even though `b` may come from a different modulemap. That was the reason we did not diagnose duplicating definitions in this specific case before. Thus I don't think of this as a separate fifth case, rather we change the scope of this current case. 

https://github.com/llvm/llvm-project/pull/190085