[clang] [clang][modules] Non-modular #import Should Respect Submodule Visibility (PR #170215)

[Qiongsi Wu via cfe-commits]

================
@@ -1550,19 +1553,46 @@ class Preprocessor {
   /// Mark the file as included.
   /// Returns true if this is the first time the file was included.
   bool markIncluded(FileEntryRef File) {
+    bool AlreadyIncluded = alreadyIncluded(File);
     HeaderInfo.getFileInfo(File).IsLocallyIncluded = true;
-    return IncludedFiles.insert(File).second;
+    CurSubmoduleState->VisibleIncludedFiles.insert(File);
+
+    Module *M = BuildingSubmoduleStack.empty()
+                    ? getCurrentModule()
+                    : BuildingSubmoduleStack.back().M;
+    if (M)
+      M->Includes.insert(File);
+    else
+      Includes.insert(File);
+
+    return !AlreadyIncluded;
   }
 
   /// Return true if this header has already been included.
   bool alreadyIncluded(FileEntryRef File) const {
     HeaderInfo.getFileInfo(File);
-    return IncludedFiles.count(File);
+    return CurSubmoduleState->VisibleIncludedFiles.contains(File);
----------------
qiongsiwu wrote:

Good catch! I think this is actually https://github.com/llvm/llvm-project/pull/170215#discussion_r2620047816 reincarnated. The case above is exactly what I missed when looking into this earlier, and I thought the call back was always a no-op. Specifically, I fixed the problem in `makeModuleVisible` instead of here. I don't think we need to loop here once `makeModuleVisible` gets the included headers correctly. 

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