[HeaderSearch] Use `isImport` only for imported headers and not for `#pragma once`.

[vsapsai]

Summary

There is a separate field isPragmaOnce and when isImport combines
both, it complicates HeaderFileInfo serialization as #pragma once is
the inherent property of the header while isImport reflects how other
headers use it. The usage of the header can be different in different
contexts, that's why isImport requires tracking separate from #pragma once.

[vsapsai]

clang/include/clang/Lex/HeaderSearch.h

455–458

This affects the result of hasFileBeenImported that is used only in Indexing.cpp as

bool isParsedOnceInclude(const FileEntry *FE) {
  return PP.getHeaderSearchInfo().isFileMultipleIncludeGuarded(FE) ||
         PP.getHeaderSearchInfo().hasFileBeenImported(FE);
}

and isFileMultipleIncludeGuarded is checking isPragmaOnce already, so no behavior change as far as I can tell.

[jansvoboda11]

This seems reasonable on first look. Can you add a test that demonstrates the problem this patch solves?

[vsapsai]

In D104351#2824505, @jansvoboda11 wrote:

This seems reasonable on first look. Can you add a test that demonstrates the problem this patch solves?

There is no externally observable change that can be tested, I'm only making sure the existing tests keep passing. In the subsequent commit I change the restoration of isImport flag from .pcm because it reflects how a header is used, not an inherent property of a header that stays the same regardless of the header usage. To make things easier to implement I've decided to separate these two qualities of a header. And I think it fits names better, like hasFileBeenImported. With this change we don't need to add in the comments that it means hasFileBeenImportedOrHasPragmaOnce.

[jansvoboda11]

Ok, this makes sense. LGTM.

[vsapsai]

Thanks for the review! I'll wait with committing this change until https://reviews.llvm.org/D104344 is ready to avoid bumping VERSION_MAJOR twice. While this change doesn't introduce any syntactical differences to serialized AST, it does introduce semantic differences.

clang/include/clang/Lex/HeaderSearch.h

 class LangOptions;
 class Module;
 class Preprocessor;
 class TargetInfo;
 
 /// The preprocessor keeps track of this information for each
 /// file that is \#included.
 struct HeaderFileInfo {
-  /// True if this is a \#import'd or \#pragma once file.
+  /// True if this is a \#import'd file.
   unsigned isImport : 1;
 
   /// True if this is a \#pragma once file.
   unsigned isPragmaOnce : 1;
 
   /// Keep track of whether this is a system header, and if so,
   /// whether it is C++ clean or not.  This can be set by the include paths or
   /// by \#pragma gcc system_header.  This is an instance of
bool ShouldEnterIncludeFile(Preprocessor &PP, const FileEntry *File,
                               Module *M);
 
   /// Return whether the specified file is a normal header,
   /// a system header, or a C++ friendly system header.
   SrcMgr::CharacteristicKind getFileDirFlavor(const FileEntry *File) {
     return (SrcMgr::CharacteristicKind)getFileInfo(File).DirInfo;
   }
 
-  /// Mark the specified file as a "once only" file, e.g. due to
+  /// Mark the specified file as a "once only" file due to
   /// \#pragma once.
   void MarkFileIncludeOnce(const FileEntry *File) {
     HeaderFileInfo &FI = getFileInfo(File);
-    FI.isImport = true;
     FI.isPragmaOnce = true;
   }
 
   /// Mark the specified file as a system header, e.g. due to
   /// \#pragma GCC system_header.
   void MarkFileSystemHeader(const FileEntry *File) {
     getFileInfo(File).DirInfo = SrcMgr::C_System;
   }
 
public:
 
   /// Determine whether this file is intended to be safe from
   /// multiple inclusions, e.g., it has \#pragma once or a controlling
   /// macro.
   ///
   /// This routine does not consider the effect of \#import
   bool isFileMultipleIncludeGuarded(const FileEntry *File);
 
-  /// Determine whether the given file is known to have ever been \#imported
-  /// (or if it has been \#included and we've encountered a \#pragma once).
+  /// Determine whether the given file is known to have ever been \#imported.
   bool hasFileBeenImported(const FileEntry *File) {
     const HeaderFileInfo *FI = getExistingFileInfo(File);
     return FI && FI->isImport;
   }
 
   /// This method returns a HeaderMap for the specified
   /// FileEntry, uniquing them through the 'HeaderMaps' datastructure.
   const HeaderMap *CreateHeaderMap(const FileEntry *FE);

clang/lib/Lex/HeaderSearch.cpp

: HSOpts(std::move(HSOpts)), Diags(Diags),
       FileMgr(SourceMgr.getFileManager()), FrameworkMap(64),
       ModMap(SourceMgr, Diags, LangOpts, Target, *this) {}
 
 void HeaderSearch::PrintStats() {
   llvm::errs() << "\n*** HeaderSearch Stats:\n"
                << FileInfo.size() << " files tracked.\n";
   unsigned NumOnceOnlyFiles = 0, MaxNumIncludes = 0, NumSingleIncludedFiles = 0;
   for (unsigned i = 0, e = FileInfo.size(); i != e; ++i) {
-    NumOnceOnlyFiles += FileInfo[i].isImport;
+    NumOnceOnlyFiles += (FileInfo[i].isPragmaOnce || FileInfo[i].isImport);
     if (MaxNumIncludes < FileInfo[i].NumIncludes)
       MaxNumIncludes = FileInfo[i].NumIncludes;
     NumSingleIncludedFiles += FileInfo[i].NumIncludes == 1;
   }
   llvm::errs() << "  " << NumOnceOnlyFiles << " #import/#pragma once files.\n"
                << "  " << NumSingleIncludedFiles << " included exactly once.\n"
                << "  " << MaxNumIncludes << " max times a file is included.\n";
 
if (isImport) {
     FileInfo.isImport = true;
 
     // Has this already been #import'ed or #include'd?
     if (FileInfo.NumIncludes && !TryEnterImported())
       return false;
   } else {
     // Otherwise, if this is a #include of a file that was previously #import'd
     // or if this is the second #include of a #pragma once file, ignore it.
-    if (FileInfo.isImport && !TryEnterImported())
+    if ((FileInfo.isPragmaOnce || FileInfo.isImport) && !TryEnterImported())
       return false;
   }
 
   // Next, check to see if the file is wrapped with #ifndef guards.  If so, and
   // if the macro that guards it is defined, we know the #include has no effect.
   if (const IdentifierInfo *ControllingMacro
       = FileInfo.getControllingMacro(ExternalLookup)) {
     // If the header corresponds to a module, check whether the macro is already