[clang] Adding CharacteristicKind to PPCallbacks::InclusionDirective

[juliehockett]

Summary

Adding a SrcMgr::CharacteristicKind parameter to the InclusionDirective in PPCallbacks, and updating calls to that function. This will be useful in https://reviews.llvm.org/D43778 to determine which includes are system headers.

[aaron.ballman]

LGTM with a small nit.

unittests/Lex/PPCallbacksTest.cpp

53 ↗

(On Diff #145820)

Can you add a test that uses this field and checks its has the expected value?

[juliehockett]

Adding test

[aaron.ballman]

unittests/Lex/PPCallbacksTest.cpp

142–143 ↗

(On Diff #145934)

It would probably be cleaner to provide the old interface as well, but define it to:

CharSourceRange InclusionDirectiveFilenameRange(const char* SourceText, const char* HeaderPath, bool SystemHeader) {
  return InclusionDirectiveCallback(SourceText, HeaderPath, SystemHeader)->FilenameRange;
}

142–144 ↗

(On Diff #145934)

The formatting looks off here, did clang-format do this?

[juliehockett]

Fixing formatting and tests.

[aaron.ballman]

LGTM!

[juliehockett]

This will break things in clang-tools-extra without D46615, so I'm going to hold off landing this until that goes through

[juliehockett]

Reverted because of memory leak in PPCallbacksTest, this fixes it.

[aaron.ballman]

unittests/Lex/PPCallbacksTest.cpp

140 ↗

(On Diff #146032)

This function appears to be unused?

179 ↗

(On Diff #146032)

Did you intend to make use of it here?

200 ↗

(On Diff #146032)

and here?

[juliehockett]

Removing unused function

[aaron.ballman]

unittests/Lex/PPCallbacksTest.cpp

155–160 ↗

(On Diff #146158)

I'm likely just missing something, but why is the unique_ptr required at all? The Preprocessor object will be destroyed on exit from this function, so it seems like it could be an automatic variable that's passed by reference to InclusionDirectiveCallback(), and same below.

[aaron.ballman]

LGTM!

cfe/trunk/include/clang/Lex/PPCallbacks.h

public:
   /// "Some.h".
   ///
   /// \param RelativePath The path relative to SearchPath, at which the include
   /// file was found. This is equal to FileName except for framework includes.
   ///
   /// \param Imported The module, whenever an inclusion directive was
   /// automatically turned into a module import or null otherwise.
   ///
+  /// \param FileType The characteristic kind, indicates whether a file or
+  /// directory holds normal user code, system code, or system code which is
+  /// implicitly 'extern "C"' in C++ mode.
+  ///
   virtual void InclusionDirective(SourceLocation HashLoc,
                                   const Token &IncludeTok,
                                   StringRef FileName,
                                   bool IsAngled,
                                   CharSourceRange FilenameRange,
                                   const FileEntry *File,
                                   StringRef SearchPath,
                                   StringRef RelativePath,
-                                  const Module *Imported) {
+                                  const Module *Imported,
+                                  SrcMgr::CharacteristicKind FileType) {
   }
 
   /// Callback invoked whenever there was an explicit module-import
   /// syntax.
   ///
   /// \param ImportLoc The location of import directive token.
   ///
   /// \param Path The identifiers (and their locations) of the module
bool FileNotFound(StringRef FileName,
     return First->FileNotFound(FileName, RecoveryPath) ||
            Second->FileNotFound(FileName, RecoveryPath);
   }
 
   void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
                           StringRef FileName, bool IsAngled,
                           CharSourceRange FilenameRange, const FileEntry *File,
                           StringRef SearchPath, StringRef RelativePath,
-                          const Module *Imported) override {
+                          const Module *Imported,
+                          SrcMgr::CharacteristicKind FileType) override {
     First->InclusionDirective(HashLoc, IncludeTok, FileName, IsAngled,
                               FilenameRange, File, SearchPath, RelativePath,
-                              Imported);
+                              Imported, FileType);
     Second->InclusionDirective(HashLoc, IncludeTok, FileName, IsAngled,
                                FilenameRange, File, SearchPath, RelativePath,
-                               Imported);
+                               Imported, FileType);
   }
 
   void moduleImport(SourceLocation ImportLoc, ModuleIdPath Path,
                     const Module *Imported) override {
     First->moduleImport(ImportLoc, Path, Imported);
     Second->moduleImport(ImportLoc, Path, Imported);
   }
 

cfe/trunk/include/clang/Lex/PreprocessingRecord.h

void MacroExpands(const Token &Id, const MacroDefinition &MD,
                       SourceRange Range, const MacroArgs *Args) override;
     void MacroDefined(const Token &Id, const MacroDirective *MD) override;
     void MacroUndefined(const Token &Id, const MacroDefinition &MD,
                         const MacroDirective *Undef) override;
     void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
                             StringRef FileName, bool IsAngled,
                             CharSourceRange FilenameRange,
                             const FileEntry *File, StringRef SearchPath,
-                            StringRef RelativePath,
-                            const Module *Imported) override;
+                            StringRef RelativePath, const Module *Imported,
+                            SrcMgr::CharacteristicKind FileType) override;
     void Ifdef(SourceLocation Loc, const Token &MacroNameTok,
                const MacroDefinition &MD) override;
     void Ifndef(SourceLocation Loc, const Token &MacroNameTok,
                 const MacroDefinition &MD) override;
 
     /// Hook called whenever the 'defined' operator is seen.
     void Defined(const Token &MacroNameTok, const MacroDefinition &MD,
                  SourceRange Range) override;

cfe/trunk/lib/CodeGen/MacroPPCallbacks.h

void FileChanged(SourceLocation Loc, FileChangeReason Reason,
                    SrcMgr::CharacteristicKind FileType,
                    FileID PrevFID = FileID()) override;
 
   /// Callback invoked whenever a directive (#xxx) is processed.
   void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
                           StringRef FileName, bool IsAngled,
                           CharSourceRange FilenameRange, const FileEntry *File,
                           StringRef SearchPath, StringRef RelativePath,
-                          const Module *Imported) override;
+                          const Module *Imported,
+                          SrcMgr::CharacteristicKind FileType) override;
 
   /// Hook called whenever a macro definition is seen.
   void MacroDefined(const Token &MacroNameTok,
                     const MacroDirective *MD) override;
 
   /// Hook called whenever a macro \#undef is seen.
   ///
   /// MD is released immediately following this callback.
   void MacroUndefined(const Token &MacroNameTok, const MacroDefinition &MD,
                       const MacroDirective *Undef) override;
 };
 
 } // end namespace clang

cfe/trunk/lib/CodeGen/MacroPPCallbacks.cpp

if (Reason == EnterFile)
     FileEntered(Loc);
   else if (Reason == ExitFile)
     FileExited(Loc);
 }
 
 void MacroPPCallbacks::InclusionDirective(
     SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName,
     bool IsAngled, CharSourceRange FilenameRange, const FileEntry *File,
-    StringRef SearchPath, StringRef RelativePath, const Module *Imported) {
+    StringRef SearchPath, StringRef RelativePath, const Module *Imported,
+    SrcMgr::CharacteristicKind FileType) {
 
   // Record the line location of the current included file.
   LastHashLoc = HashLoc;
 }
 
 void MacroPPCallbacks::MacroDefined(const Token &MacroNameTok,
                                     const MacroDirective *MD) {
   IdentifierInfo *Id = MacroNameTok.getIdentifierInfo();

cfe/trunk/lib/Frontend/DependencyFile.cpp

DepCollector.maybeAddDependency(Filename, /*FromModule*/false,
                                     isSystem(FileType),
                                     /*IsModuleFile*/false, /*IsMissing*/false);
   }
 
   void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
                           StringRef FileName, bool IsAngled,
                           CharSourceRange FilenameRange, const FileEntry *File,
                           StringRef SearchPath, StringRef RelativePath,
-                          const Module *Imported) override {
+                          const Module *Imported,
+                          SrcMgr::CharacteristicKind FileType) override {
     if (!File)
       DepCollector.maybeAddDependency(FileName, /*FromModule*/false,
                                      /*IsSystem*/false, /*IsModuleFile*/false,
                                      /*IsMissing*/true);
     // Files that actually exist are handled by FileChanged.
   }
 
   void EndOfMainFile() override {
public:
 
   void FileSkipped(const FileEntry &SkippedFile, const Token &FilenameTok,
                    SrcMgr::CharacteristicKind FileType) override;
 
   void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
                           StringRef FileName, bool IsAngled,
                           CharSourceRange FilenameRange, const FileEntry *File,
                           StringRef SearchPath, StringRef RelativePath,
-                          const Module *Imported) override;
+                          const Module *Imported,
+                          SrcMgr::CharacteristicKind FileType) override;
 
   void EndOfMainFile() override {
     OutputDependencyFile();
   }
 
   void AddFilename(StringRef Filename);
   bool includeSystemHeaders() const { return IncludeSystemHeaders; }
   bool includeModuleFiles() const { return IncludeModuleFiles; }

 void DFGImpl::InclusionDirective(SourceLocation HashLoc,
                                  const Token &IncludeTok,
                                  StringRef FileName,
                                  bool IsAngled,
                                  CharSourceRange FilenameRange,
                                  const FileEntry *File,
                                  StringRef SearchPath,
                                  StringRef RelativePath,
-                                 const Module *Imported) {
+                                 const Module *Imported, 
+                                 SrcMgr::CharacteristicKind FileType) {
   if (!File) {
     if (AddMissingHeaderDeps)
       AddFilename(FileName);
     else
       SeenMissingHeader = true;
   }
 }
 

cfe/trunk/lib/Frontend/DependencyGraph.cpp

public:
   DependencyGraphCallback(const Preprocessor *_PP, StringRef OutputFile,
                           StringRef SysRoot)
     : PP(_PP), OutputFile(OutputFile.str()), SysRoot(SysRoot.str()) { }
 
   void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
                           StringRef FileName, bool IsAngled,
                           CharSourceRange FilenameRange, const FileEntry *File,
                           StringRef SearchPath, StringRef RelativePath,
-                          const Module *Imported) override;
+                          const Module *Imported,
+                          SrcMgr::CharacteristicKind FileType) override;
 
   void EndOfMainFile() override {
     OutputGraphFile();
   }
   
 };
 }
 
 void clang::AttachDependencyGraphGen(Preprocessor &PP, StringRef OutputFile,
                                      StringRef SysRoot) {
   PP.addPPCallbacks(llvm::make_unique<DependencyGraphCallback>(&PP, OutputFile,
                                                                SysRoot));
 }
 
-void DependencyGraphCallback::InclusionDirective(SourceLocation HashLoc,
-                                                 const Token &IncludeTok,
-                                                 StringRef FileName,
-                                                 bool IsAngled,
-                                                 CharSourceRange FilenameRange,
-                                                 const FileEntry *File,
-                                                 StringRef SearchPath,
-                                                 StringRef RelativePath,
-                                                 const Module *Imported) {
+void DependencyGraphCallback::InclusionDirective(
+    SourceLocation HashLoc,
+    const Module *Imported, 
+    SrcMgr::CharacteristicKind FileType) {
   if (!File)
     return;
   
   SourceManager &SM = PP->getSourceManager();
   const FileEntry *FromFile
     = SM.getFileEntryForID(SM.getFileID(SM.getExpansionLoc(HashLoc)));
   if (!FromFile)
     return;

cfe/trunk/lib/Frontend/ModuleDependencyCollector.cpp

struct ModuleDependencyPPCallbacks : public PPCallbacks {
   ModuleDependencyPPCallbacks(ModuleDependencyCollector &Collector,
                               SourceManager &SM)
       : Collector(Collector), SM(SM) {}
 
   void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
                           StringRef FileName, bool IsAngled,
                           CharSourceRange FilenameRange, const FileEntry *File,
                           StringRef SearchPath, StringRef RelativePath,
-                          const Module *Imported) override {
+                          const Module *Imported,
+                          SrcMgr::CharacteristicKind FileType) override {
     if (!File)
       return;
     Collector.addFile(File->getName());
   }
 };
 
 struct ModuleDependencyMMCallbacks : public ModuleMapCallbacks {
   ModuleDependencyCollector &Collector;

cfe/trunk/lib/Frontend/PrintPreprocessedOutput.cpp

public:
 
   void FileChanged(SourceLocation Loc, FileChangeReason Reason,
                    SrcMgr::CharacteristicKind FileType,
                    FileID PrevFID) override;
   void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
                           StringRef FileName, bool IsAngled,
                           CharSourceRange FilenameRange, const FileEntry *File,
                           StringRef SearchPath, StringRef RelativePath,
-                          const Module *Imported) override;
+                          const Module *Imported,
+                          SrcMgr::CharacteristicKind FileType) override;
   void Ident(SourceLocation Loc, StringRef str) override;
   void PragmaMessage(SourceLocation Loc, StringRef Namespace,
                      PragmaMessageKind Kind, StringRef Str) override;
   void PragmaDebug(SourceLocation Loc, StringRef DebugType) override;
   void PragmaDiagnosticPush(SourceLocation Loc, StringRef Namespace) override;
   void PragmaDiagnosticPop(SourceLocation Loc, StringRef Namespace) override;
   void PragmaDiagnostic(SourceLocation Loc, StringRef Namespace,
                         diag::Severity Map, StringRef Str) override;
case PPCallbacks::ExitFile:
     break;
   case PPCallbacks::SystemHeaderPragma:
   case PPCallbacks::RenameFile:
     WriteLineInfo(CurLine);
     break;
   }
 }
 
-void PrintPPOutputPPCallbacks::InclusionDirective(SourceLocation HashLoc,
-                                                  const Token &IncludeTok,
-                                                  StringRef FileName,
-                                                  bool IsAngled,
-                                                  CharSourceRange FilenameRange,
-                                                  const FileEntry *File,
-                                                  StringRef SearchPath,
-                                                  StringRef RelativePath,
-                                                  const Module *Imported) {
+void PrintPPOutputPPCallbacks::InclusionDirective(
+    SourceLocation HashLoc,
+    const Module *Imported,
+    SrcMgr::CharacteristicKind FileType) {
   // In -dI mode, dump #include directives prior to dumping their content or
   // interpretation.
   if (DumpIncludeDirectives) {
     startNewLineIfNeeded();
     MoveToLine(HashLoc);
     const std::string TokenText = PP.getSpelling(IncludeTok);
     assert(!TokenText.empty());
     OS << "#" << TokenText << " "

cfe/trunk/lib/Frontend/Rewrite/InclusionRewriter.cpp

void FileChanged(SourceLocation Loc, FileChangeReason Reason,
                    SrcMgr::CharacteristicKind FileType,
                    FileID PrevFID) override;
   void FileSkipped(const FileEntry &SkippedFile, const Token &FilenameTok,
                    SrcMgr::CharacteristicKind FileType) override;
   void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
                           StringRef FileName, bool IsAngled,
                           CharSourceRange FilenameRange, const FileEntry *File,
                           StringRef SearchPath, StringRef RelativePath,
-                          const Module *Imported) override;
+                          const Module *Imported,
+                          SrcMgr::CharacteristicKind FileType) override;
   void WriteLineInfo(StringRef Filename, int Line,
                      SrcMgr::CharacteristicKind FileType,
                      StringRef Extra = StringRef());
   void WriteImplicitModuleImport(const Module *Mod);
   void OutputContentUpTo(const MemoryBuffer &FromFile,
                          unsigned &WriteFrom, unsigned WriteTo,
                          StringRef EOL, int &lines,
                          bool EnsureNewline);

 void InclusionRewriter::InclusionDirective(SourceLocation HashLoc,
                                            const Token &/*IncludeTok*/,
                                            StringRef /*FileName*/,
                                            bool /*IsAngled*/,
                                            CharSourceRange /*FilenameRange*/,
                                            const FileEntry * /*File*/,
                                            StringRef /*SearchPath*/,
                                            StringRef /*RelativePath*/,
-                                           const Module *Imported) {
+                                           const Module *Imported,
+                                           SrcMgr::CharacteristicKind FileType){
   if (Imported) {
     auto P = ModuleIncludes.insert(
         std::make_pair(HashLoc.getRawEncoding(), Imported));
     (void)P;
     assert(P.second && "Unexpected revisitation of the same include directive");
   } else
     LastInclusionLocation = HashLoc;
 }

cfe/trunk/lib/Lex/PPDirectives.cpp

#endif
   }
 
   if (Callbacks) {
     // Notify the callback object that we've seen an inclusion directive.
     Callbacks->InclusionDirective(
         HashLoc, IncludeTok,
         LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled,
         FilenameRange, File, SearchPath, RelativePath,
-        ShouldEnter ? nullptr : SuggestedModule.getModule());
+        ShouldEnter ? nullptr : SuggestedModule.getModule(), FileCharacter);
     if (SkipHeader && !SuggestedModule.getModule())
       Callbacks->FileSkipped(*File, FilenameTok, FileCharacter);
   }
 
   if (!File)
     return;
 
   // FIXME: If we have a suggested module, and we've already visited this file,

cfe/trunk/lib/Lex/PreprocessingRecord.cpp

void PreprocessingRecord::InclusionDirective(
     SourceLocation HashLoc,
     const Token &IncludeTok,
     StringRef FileName,
     bool IsAngled,
     CharSourceRange FilenameRange,
     const FileEntry *File,
     StringRef SearchPath,
     StringRef RelativePath,
-    const Module *Imported) {
+    const Module *Imported, 
+    SrcMgr::CharacteristicKind FileType) {
   InclusionDirective::InclusionKind Kind = InclusionDirective::Include;
   
   switch (IncludeTok.getIdentifierInfo()->getPPKeywordID()) {
   case tok::pp_include: 
     Kind = InclusionDirective::Include; 
     break;
     
   case tok::pp_import: 

cfe/trunk/tools/libclang/Indexing.cpp

if (Loc == MainFileLoc && Reason == PPCallbacks::EnterFile) {
       DataConsumer.enteredMainFile(SM.getFileEntryForID(SM.getMainFileID()));
     }
   }
 
   void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
                           StringRef FileName, bool IsAngled,
                           CharSourceRange FilenameRange, const FileEntry *File,
                           StringRef SearchPath, StringRef RelativePath,
-                          const Module *Imported) override {
+                          const Module *Imported,
+                          SrcMgr::CharacteristicKind FileType) override {
     bool isImport = (IncludeTok.is(tok::identifier) &&
             IncludeTok.getIdentifierInfo()->getPPKeywordID() == tok::pp_import);
     DataConsumer.ppIncludedFile(HashLoc, FileName, File, isImport, IsAngled,
                             Imported);
   }
 
   /// MacroDefined - This hook is called whenever a macro definition is seen.
   void MacroDefined(const Token &Id, const MacroDirective *MD) override {}

cfe/trunk/unittests/Lex/PPCallbacksTest.cpp

 
 // Stub to collect data from InclusionDirective callbacks.
 class InclusionDirectiveCallbacks : public PPCallbacks {
 public:
   void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
                           StringRef FileName, bool IsAngled,
                           CharSourceRange FilenameRange, const FileEntry *File,
                           StringRef SearchPath, StringRef RelativePath,
-                          const Module *Imported) override {
-      this->HashLoc = HashLoc;
-      this->IncludeTok = IncludeTok;
-      this->FileName = FileName.str();
-      this->IsAngled = IsAngled;
-      this->FilenameRange = FilenameRange;
-      this->File = File;
-      this->SearchPath = SearchPath.str();
-      this->RelativePath = RelativePath.str();
-      this->Imported = Imported;
+                          const Module *Imported,
+                          SrcMgr::CharacteristicKind FileType) override {
+    this->FileType = FileType;
   }
 
   SourceLocation HashLoc;
   Token IncludeTok;
   SmallString<16> FileName;
   bool IsAngled;
   CharSourceRange FilenameRange;
   const FileEntry* File;
   SmallString<16> SearchPath;
   SmallString<16> RelativePath;
   const Module* Imported;
+  SrcMgr::CharacteristicKind FileType;
 };
 
 // Stub to collect data from PragmaOpenCLExtension callbacks.
 class PragmaOpenCLExtensionCallbacks : public PPCallbacks {
 public:
   typedef struct {
     SmallString<16> Name;
     unsigned State;
StringRef GetSourceString(CharSourceRange Range) {
 
     return StringRef(B, E - B);
   }
 
   // Run lexer over SourceText and collect FilenameRange from
   // the InclusionDirective callback.
   CharSourceRange InclusionDirectiveFilenameRange(const char* SourceText, 
       const char* HeaderPath, bool SystemHeader) {
+    return InclusionDirectiveCallback(SourceText, HeaderPath, SystemHeader)
+        ->FilenameRange;
+  }
+
+  InclusionDirectiveCallbacks *
+  InclusionDirectiveCallback(const char *SourceText, const char *HeaderPath,
+                             bool SystemHeader) {
     std::unique_ptr<llvm::MemoryBuffer> Buf =
         llvm::MemoryBuffer::getMemBuffer(SourceText);
     SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(Buf)));
 
     TrivialModuleLoader ModLoader;
     MemoryBufferCache PCMCache;
 
     HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,
InclusionDirectiveCallback(const char *SourceText, const char *HeaderPath,
     while (true) {
       Token Tok;
       PP.Lex(Tok);
       if (Tok.is(tok::eof))
         break;
     }
 
     // Callbacks have been executed at this point -- return filename range.
-    return Callbacks->FilenameRange;
+    return Callbacks;
   }
 
   PragmaOpenCLExtensionCallbacks::CallbackParameters 
   PragmaOpenCLExtensionCall(const char* SourceText) {
     LangOptions OpenCLLangOpts;
     OpenCLLangOpts.OpenCL = 1;
 
     std::unique_ptr<llvm::MemoryBuffer> SourceBuf =
PragmaOpenCLExtensionCall(const char* SourceText) {
     PragmaOpenCLExtensionCallbacks::CallbackParameters RetVal = {
       Callbacks->Name,
       Callbacks->State
     };
     return RetVal;    
   }
 };
 
+TEST_F(PPCallbacksTest, UserFileCharacteristics) {
+  const char *Source = "#include \"quoted.h\"\n";
+
+  SrcMgr::CharacteristicKind Kind =
+      InclusionDirectiveCallback(Source, "/quoted.h", false)->FileType;
+
+  ASSERT_EQ(SrcMgr::CharacteristicKind::C_User, Kind);
+}
+
 TEST_F(PPCallbacksTest, QuotedFilename) {
   const char* Source =
     "#include \"quoted.h\"\n";
 
   CharSourceRange Range =
     InclusionDirectiveFilenameRange(Source, "/quoted.h", false);
 
   ASSERT_EQ("\"quoted.h\"", GetSourceString(Range));