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

[Cyndy Ishida 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
     bool LoadedFromASTFile = Existing->IsFromModuleFile;
+    if (LoadedFromASTFile) {
----------------
cyndyishida wrote:

It sounds like it would be much cleaner to semantically compare `ModuleDecl`s in general. But I don't actually see that stored in `clang::Module`, so I don't know how difficult that is. I think the current state is ok, but now I realize this case is described slightly wrong.

```
//  - If the module was loaded from an AST file and we've found its original
// source module map

```
Since the definition is not actually checked. 

I think a later refactor may be useful, but I don't think it has to block this fix.

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