[clang-tidy] Fix redefinition of module in the same module.modulemap file

[DmitryPolukhin]

Summary

In memory VFS cannot handle aceesssing the same file with different paths.
This diff just stops using VFS for modulemap files.

Fixes PR47839

[DmitryPolukhin]

Linting diff

[DmitryPolukhin]

And one more time linting

[alexfh]

Looks good!

clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp

27

Looking at the relevant code I find special file names like "module.map", "module_private.map", "module.private.modulemap". Is this problem relevant for any of those?

[alexfh]

Ah, btw, any chance of adding a test for this?

[DmitryPolukhin]

Added all module map file names

[DmitryPolukhin]

In D89886#2346851, @alexfh wrote:

Ah, btw, any chance of adding a test for this?

Oh, I was not able to create small reproducer that without including large Apple Frameworks with modules :( My hypothesis that it is side effect of module cache that triggers module load before it is referenced from sources. I tested it on reproducer from PR47839 + my real internal example.

clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp

27

I think it is relevant for these files too, added them all.

[alexfh]

In D89886#2347376, @DmitryPolukhin wrote:

In D89886#2346851, @alexfh wrote:

Ah, btw, any chance of adding a test for this?

Oh, I was not able to create small reproducer that without including large Apple Frameworks with modules :( My hypothesis that it is side effect of module cache that triggers module load before it is referenced from sources. I tested it on reproducer from PR47839 + my real internal example.

I see. Let's keep it as is then.

Looks good!

clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp

 //===- ExpandModularHeadersPPCallbacks.h - clang-tidy -----------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "ExpandModularHeadersPPCallbacks.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Serialization/ASTReader.h"
 
+#define DEBUG_TYPE "clang-tidy"
+
 namespace clang {
 namespace tooling {
 
 class ExpandModularHeadersPPCallbacks::FileRecorder {
 public:
   /// Records that a given file entry is needed for replaying callbacks.
-  void addNecessaryFile(const FileEntry *File) { FilesToRecord.insert(File); }
+  void addNecessaryFile(const FileEntry *File) {
+    // Don't record modulemap files because it breaks same file detection.
+    if (!(File->getName().endswith("module.modulemap") ||
+          File->getName().endswith("module.private.modulemap") ||
+          File->getName().endswith("module.map") ||
+          File->getName().endswith("module_private.map")))
+      FilesToRecord.insert(File);
+  }
 
   /// Records content for a file and adds it to the FileSystem.
   void recordFileContent(const FileEntry *File,
                          const SrcMgr::ContentCache &ContentCache,
                          llvm::vfs::InMemoryFileSystem &InMemoryFs) {
     // Return if we are not interested in the contents of this file.
     if (!FilesToRecord.count(File))
       return;
 
     // FIXME: Why is this happening? We might be losing contents here.
     llvm::Optional<StringRef> Data = ContentCache.getBufferDataIfLoaded();
     if (!Data)
       return;
 
     InMemoryFs.addFile(File->getName(), /*ModificationTime=*/0,
                        llvm::MemoryBuffer::getMemBufferCopy(*Data));
     // Remove the file from the set of necessary files.
     FilesToRecord.erase(File);
   }
 
   /// Makes sure we have contents for all the files we were interested in. Ideally
   /// `FilesToRecord` should be empty.
   void checkAllFilesRecorded() {
     for (auto FileEntry : FilesToRecord)
-      llvm::errs() << "Did not record contents for input file: "
-                   << FileEntry->getName() << "\n";
+      LLVM_DEBUG(llvm::dbgs() << "Did not record contents for input file: "
+                              << FileEntry->getName() << "\n");
   }
 
 private:
   /// A set of files whose contents are to be recorded.
   llvm::DenseSet<const FileEntry *> FilesToRecord;
 };
 
 ExpandModularHeadersPPCallbacks::ExpandModularHeadersPPCallbacks(