Revert "Revert "[clang][extract-api] Use relative includes""

[zixuw]

Summary

Reapply the change after fixing sanitizer errors.
The original problem was that StringRefs in Matches are pointing to
temporary local std::strings created by path::convert_to_slash in
the regex match call. This patch does the conversion up front in
container FilePath.

This reverts commit 2966f0fa505266735dbc8324b8821b7f0aa901ff.

[zixuw]

Whitespace change: clang-format removed an empty line

[dang]

LGTM! If I understand the issue correctly we gave llvm::Regex::match a string temporary to match against before and now we store it for long enough to process the match results?

[zixuw]

In D124964#3492423, @dang wrote:

LGTM! If I understand the issue correctly we gave llvm::Regex::match a string temporary to match against before and now we store it for long enough to process the match results?

Yup, exactly.

clang/include/clang/ExtractAPI/FrontendActions.h

private:
   /// The product this action is extracting API information for.
   std::string ProductName;
 
   /// The synthesized input buffer that contains all the provided input header
   /// files.
   std::unique_ptr<llvm::MemoryBuffer> Buffer;
 
   /// The input file originally provided on the command line.
-  std::vector<std::string> KnownInputFiles;
+  ///
+  /// This captures the spelling used to include the file and whether the
+  /// include is quoted or not.
+  SmallVector<std::pair<SmallString<32>, bool>> KnownInputFiles;
 
   /// Prepare to execute the action on the given CompilerInstance.
   ///
   /// This is called before executing the action on any inputs. This generates a
   /// single header that includes all of CI's inputs and replaces CI's input
   /// list with it before actually executing the action.
   bool PrepareToExecuteAction(CompilerInstance &CI) override;
 

clang/lib/ExtractAPI/ExtractAPIConsumer.cpp

 #include "clang/Frontend/FrontendOptions.h"
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/PPCallbacks.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Regex.h"
 #include "llvm/Support/raw_ostream.h"
 #include <memory>
 #include <utility>
 
 using namespace clang;
 using namespace extractapi;
 
 namespace {
 
 StringRef getTypedefName(const TagDecl *Decl) {
   if (const auto *TypedefDecl = Decl->getTypedefNameForAnonDecl())
     return TypedefDecl->getName();
 
   return {};
 }
 
+Optional<std::string> getRelativeIncludeName(const CompilerInstance &CI,
+                                             StringRef File,
+                                             bool *IsQuoted = nullptr) {
+  assert(CI.hasFileManager() &&
+         "CompilerInstance does not have a FileNamager!");
+
+  using namespace llvm::sys;
+  // Matches framework include patterns
+  const llvm::Regex Rule("/(.+)\\.framework/(.+)?Headers/(.+)");
+
+  const auto &FS = CI.getVirtualFileSystem();
+
+  SmallString<128> FilePath(File.begin(), File.end());
+  FS.makeAbsolute(FilePath);
+  path::remove_dots(FilePath, true);
+  FilePath = path::convert_to_slash(FilePath);
+  File = FilePath;
+
+  // Checks whether `Dir` is a strict path prefix of `File`. If so returns
+  // the prefix length. Otherwise return 0.
+  auto CheckDir = [&](llvm::StringRef Dir) -> unsigned {
+    llvm::SmallString<32> DirPath(Dir.begin(), Dir.end());
+    FS.makeAbsolute(DirPath);
+    path::remove_dots(DirPath, true);
+    Dir = DirPath;
+    for (auto NI = path::begin(File), NE = path::end(File),
+              DI = path::begin(Dir), DE = path::end(Dir);
+         /*termination condition in loop*/; ++NI, ++DI) {
+      // '.' components in File are ignored.
+      while (NI != NE && *NI == ".")
+        ++NI;
+      if (NI == NE)
+        break;
+
+      // '.' components in Dir are ignored.
+      while (DI != DE && *DI == ".")
+        ++DI;
+
+      // Dir is a prefix of File, up to '.' components and choice of path
+      // separators.
+      if (DI == DE)
+        return NI - path::begin(File);
+
+      // Consider all path separators equal.
+      if (NI->size() == 1 && DI->size() == 1 &&
+          path::is_separator(NI->front()) && path::is_separator(DI->front()))
+        continue;
+
+      // Special case Apple .sdk folders since the search path is typically a
+      // symlink like `iPhoneSimulator14.5.sdk` while the file is instead
+      // located in `iPhoneSimulator.sdk` (the real folder).
+      if (NI->endswith(".sdk") && DI->endswith(".sdk")) {
+        StringRef NBasename = path::stem(*NI);
+        StringRef DBasename = path::stem(*DI);
+        if (DBasename.startswith(NBasename))
+          continue;
+      }
+
+      if (*NI != *DI)
+        break;
+    }
+    return 0;
+  };
+
+  unsigned PrefixLength = 0;
+
+  // Go through the search paths and find the first one that is a prefix of
+  // the header.
+  for (const auto &Entry : CI.getHeaderSearchOpts().UserEntries) {
+    // Note whether the match is found in a quoted entry.
+    if (IsQuoted)
+      *IsQuoted = Entry.Group == frontend::Quoted;
+
+    if (auto EntryFile = CI.getFileManager().getOptionalFileRef(Entry.Path)) {
+      if (auto HMap = HeaderMap::Create(*EntryFile, CI.getFileManager())) {
+        // If this is a headermap entry, try to reverse lookup the full path
+        // for a spelled name before mapping.
+        StringRef SpelledFilename = HMap->reverseLookupFilename(File);
+        if (!SpelledFilename.empty())
+          return SpelledFilename.str();
+
+        // No matching mapping in this headermap, try next search entry.
+        continue;
+      }
+    }
+
+    // Entry is a directory search entry, try to check if it's a prefix of File.
+    PrefixLength = CheckDir(Entry.Path);
+    if (PrefixLength > 0) {
+      // The header is found in a framework path, construct the framework-style
+      // include name `<Framework/Header.h>`
+      if (Entry.IsFramework) {
+        SmallVector<StringRef, 4> Matches;
+        Rule.match(File, &Matches);
+        // Returned matches are always in stable order.
+        if (Matches.size() != 4)
+          return None;
+
+        return path::convert_to_slash(
+            (Matches[1].drop_front(Matches[1].rfind('/') + 1) + "/" +
+             Matches[3])
+                .str());
+      }
+
+      // The header is found in a normal search path, strip the search path
+      // prefix to get an include name.
+      return path::convert_to_slash(File.drop_front(PrefixLength));
+    }
+  }
+
+  // Couldn't determine a include name, use full path instead.
+  return None;
+}
+
 struct LocationFileChecker {
   bool isLocationInKnownFile(SourceLocation Loc) {
     // If the loc refers to a macro expansion we need to first get the file
     // location of the expansion.
+    auto &SM = CI.getSourceManager();
     auto FileLoc = SM.getFileLoc(Loc);
     FileID FID = SM.getFileID(FileLoc);
     if (FID.isInvalid())
       return false;
 
     const auto *File = SM.getFileEntryForID(FID);
     if (!File)
       return false;
 
     if (KnownFileEntries.count(File))
       return true;
 
+    if (ExternalFileEntries.count(File))
+      return false;
+
+    StringRef FileName = File->tryGetRealPathName().empty()
+                             ? File->getName()
+                             : File->tryGetRealPathName();
+
+    // Try to reduce the include name the same way we tried to include it.
+    bool IsQuoted = false;
+    if (auto IncludeName = getRelativeIncludeName(CI, FileName, &IsQuoted))
+      if (llvm::find_if(KnownFiles,
+                        [&IsQuoted, &IncludeName](const auto &KnownFile) {
+                          return KnownFile.first.equals(*IncludeName) &&
+                                 KnownFile.second == IsQuoted;
+                        }) != KnownFiles.end()) {
+        KnownFileEntries.insert(File);
+        return true;
+      }
+
+    // Record that the file was not found to avoid future reverse lookup for
+    // the same file.
+    ExternalFileEntries.insert(File);
     return false;
   }
 
-  LocationFileChecker(const SourceManager &SM,
-                      const std::vector<std::string> &KnownFiles)
-      : SM(SM) {
-    for (const auto &KnownFilePath : KnownFiles)
-      if (auto FileEntry = SM.getFileManager().getFile(KnownFilePath))
+  LocationFileChecker(const CompilerInstance &CI,
+                      SmallVector<std::pair<SmallString<32>, bool>> &KnownFiles)
+      : CI(CI), KnownFiles(KnownFiles), ExternalFileEntries() {
+    for (const auto &KnownFile : KnownFiles)
+      if (auto FileEntry = CI.getFileManager().getFile(KnownFile.first))
         KnownFileEntries.insert(*FileEntry);
   }
 
 private:
-  const SourceManager &SM;
+  const CompilerInstance &CI;
+  SmallVector<std::pair<SmallString<32>, bool>> &KnownFiles;
   llvm::DenseSet<const FileEntry *> KnownFileEntries;
+  llvm::DenseSet<const FileEntry *> ExternalFileEntries;
 };
 
 /// The RecursiveASTVisitor to traverse symbol declarations and collect API
 /// information.
 class ExtractAPIVisitor : public RecursiveASTVisitor<ExtractAPIVisitor> {
 public:
   ExtractAPIVisitor(ASTContext &Context, LocationFileChecker &LCF, APISet &API)
       : Context(Context), API(API), LCF(LCF) {}
ExtractAPIAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) {
   ProductName = CI.getFrontendOpts().ProductName;
 
   // Now that we have enough information about the language options and the
   // target triple, let's create the APISet before anyone uses it.
   API = std::make_unique<APISet>(
       CI.getTarget().getTriple(),
       CI.getFrontendOpts().Inputs.back().getKind().getLanguage());
 
-  auto LCF = std::make_unique<LocationFileChecker>(CI.getSourceManager(),
-                                                   KnownInputFiles);
+  auto LCF = std::make_unique<LocationFileChecker>(CI, KnownInputFiles);
 
   CI.getPreprocessor().addPPCallbacks(std::make_unique<MacroCallback>(
       CI.getSourceManager(), *LCF, *API, CI.getPreprocessor()));
 
   return std::make_unique<ExtractAPIConsumer>(CI.getASTContext(),
                                               std::move(LCF), *API);
 }
 
 bool ExtractAPIAction::PrepareToExecuteAction(CompilerInstance &CI) {
   auto &Inputs = CI.getFrontendOpts().Inputs;
   if (Inputs.empty())
     return true;
 
+  if (!CI.hasFileManager())
+    if (!CI.createFileManager())
+      return false;
+
   auto Kind = Inputs[0].getKind();
 
   // Convert the header file inputs into a single input buffer.
   SmallString<256> HeaderContents;
+  bool IsQuoted = false;
   for (const FrontendInputFile &FIF : Inputs) {
     if (Kind.isObjectiveC())
       HeaderContents += "#import";
     else
       HeaderContents += "#include";
+
+    StringRef FilePath = FIF.getFile();
+    if (auto RelativeName = getRelativeIncludeName(CI, FilePath, &IsQuoted)) {
+      if (IsQuoted)
         HeaderContents += " \"";
-    HeaderContents += FIF.getFile();
-    HeaderContents += "\"\n";
+      else
+        HeaderContents += " <";
 
-    KnownInputFiles.emplace_back(FIF.getFile());
+      HeaderContents += *RelativeName;
+
+      if (IsQuoted)
+        HeaderContents += "\"\n";
+      else
+        HeaderContents += ">\n";
+      KnownInputFiles.emplace_back(*RelativeName, IsQuoted);
+    } else {
+      HeaderContents += " \"";
+      HeaderContents += FilePath;
+      HeaderContents += "\"\n";
+      KnownInputFiles.emplace_back(FilePath, true);
     }
+  }
+
+  if (CI.getHeaderSearchOpts().Verbose)
+    CI.getVerboseOutputStream() << getInputBufferName() << ":\n"
+                                << HeaderContents << "\n";
 
   Buffer = llvm::MemoryBuffer::getMemBufferCopy(HeaderContents,
                                                 getInputBufferName());
 
   // Set that buffer up as our "real" input in the CompilerInstance.
   Inputs.clear();
   Inputs.emplace_back(Buffer->getMemBufferRef(), Kind, /*IsSystem*/ false);
 

clang/test/ExtractAPI/known_files_only_hmap.c

-// RUN: rm -rf %t
-// RUN: split-file %s %t
-// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
-// RUN: %t/reference.output.json.in >> %t/reference.output.json
-// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
-// RUN: %t/known_files_only.hmap.json.in >> %t/known_files_only.hmap.json
-// RUN: %hmaptool write %t/known_files_only.hmap.json %t/known_files_only.hmap
-// RUN: %clang -extract-api --product-name=KnownFilesOnlyHmap -target arm64-apple-macosx \
-// RUN: -I%t/known_files_only.hmap -I%t/subdir %t/subdir/subdir1/input.h \
-// RUN: %t/subdir/subdir2/known_file.h -o %t/output.json | FileCheck -allow-empty %s
-
-// Generator version is not consistent across test runs, normalize it.
-// RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \
-// RUN: %t/output.json >> %t/output-normalized.json
-// RUN: diff %t/reference.output.json %t/output-normalized.json
-
-// CHECK-NOT: error:
-// CHECK-NOT: warning:
-//--- known_files_only.hmap.json.in
-{
-  "mappings" :
-    {
-     "subdir2/known_file.h" : "INPUT_DIR/subdir/subdir3/unknown.h"
-    }
-}
-
-//--- subdir/subdir1/input.h
-int num;
-#include "subdir2/known_file.h"
-
-//--- subdir/subdir2/known_file.h
-int known_num;
-
-//--- subdir/subdir3/unknown.h
-// Ensure that these symbols are not emitted in the Symbol Graph.
-#ifndef INPUT4_H
-#define INPUT4_H
-
-#define HELLO 1
-char not_emitted;
-void foo(int);
-struct Foo { int a; };
-
-#endif
-
-//--- reference.output.json.in
-{
-  "metadata": {
-    "formatVersion": {
-      "major": 0,
-      "minor": 5,
-      "patch": 3
-    },
-    "generator": "?"
-  },
-  "module": {
-    "name": "KnownFilesOnlyHmap",
-    "platform": {
-      "architecture": "arm64",
-      "operatingSystem": {
-        "minimumVersion": {
-          "major": 11,
-          "minor": 0,
-          "patch": 0
-        },
-        "name": "macosx"
-      },
-      "vendor": "apple"
-    }
-  },
-  "relationships": [],
-  "symbols": [
-    {
-      "accessLevel": "public",
-      "declarationFragments": [
-        {
-          "kind": "typeIdentifier",
-          "preciseIdentifier": "c:I",
-          "spelling": "int"
-        },
-        {
-          "kind": "text",
-          "spelling": " "
-        },
-        {
-          "kind": "identifier",
-          "spelling": "num"
-        }
-      ],
-      "identifier": {
-        "interfaceLanguage": "c",
-        "precise": "c:@num"
-      },
-      "kind": {
-        "displayName": "Global Variable",
-        "identifier": "c.var"
-      },
-      "location": {
-        "position": {
-          "character": 5,
-          "line": 1
-        },
-        "uri": "file://INPUT_DIR/subdir/subdir1/input.h"
-      },
-      "names": {
-        "navigator": [
-          {
-            "kind": "identifier",
-            "spelling": "num"
-          }
-        ],
-        "subHeading": [
-          {
-            "kind": "identifier",
-            "spelling": "num"
-          }
-        ],
-        "title": "num"
-      },
-      "pathComponents": [
-        "num"
-      ]
-    },
-    {
-      "accessLevel": "public",
-      "declarationFragments": [
-        {
-          "kind": "typeIdentifier",
-          "preciseIdentifier": "c:I",
-          "spelling": "int"
-        },
-        {
-          "kind": "text",
-          "spelling": " "
-        },
-        {
-          "kind": "identifier",
-          "spelling": "known_num"
-        }
-      ],
-      "identifier": {
-        "interfaceLanguage": "c",
-        "precise": "c:@known_num"
-      },
-      "kind": {
-        "displayName": "Global Variable",
-        "identifier": "c.var"
-      },
-      "location": {
-        "position": {
-          "character": 5,
-          "line": 1
-        },
-        "uri": "file://INPUT_DIR/subdir/subdir2/known_file.h"
-      },
-      "names": {
-        "navigator": [
-          {
-            "kind": "identifier",
-            "spelling": "known_num"
-          }
-        ],
-        "subHeading": [
-          {
-            "kind": "identifier",
-            "spelling": "known_num"
-          }
-        ],
-        "title": "known_num"
-      },
-      "pathComponents": [
-        "known_num"
-      ]
-    }
-  ]
-}

clang/test/ExtractAPI/relative_include.m

+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// Setup framework root
+// RUN: mkdir -p %t/Frameworks/MyFramework.framework/Headers
+// RUN: cp %t/MyFramework.h %t/Frameworks/MyFramework.framework/Headers/
+// RUN: cp %t/MyHeader.h %t/Frameworks/MyFramework.framework/Headers/
+
+// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
+
+// Headermap maps headers to the source root SRCROOT
+// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
+// RUN: %t/headermap.hmap.json.in >> %t/headermap.hmap.json
+// RUN: %hmaptool write %t/headermap.hmap.json %t/headermap.hmap
+
+// Input headers use paths to the framework root/DSTROOT
+// RUN: %clang_cc1 -extract-api -v --product-name=MyFramework \
+// RUN: -triple arm64-apple-macosx \
+// RUN: -iquote%t -I%t/headermap.hmap -F%t/Frameworks \
+// RUN: -x objective-c-header \
+// RUN: %t/Frameworks/MyFramework.framework/Headers/MyFramework.h \
+// RUN: %t/Frameworks/MyFramework.framework/Headers/MyHeader.h \
+// RUN: %t/QuotedHeader.h \
+// RUN: -o %t/output.json 2>&1 -verify | FileCheck -allow-empty %s
+
+// Generator version is not consistent across test runs, normalize it.
+// RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \
+// RUN: %t/output.json >> %t/output-normalized.json
+// RUN: diff %t/reference.output.json %t/output-normalized.json
+
+// CHECK:      <extract-api-includes>:
+// CHECK-NEXT: #import <MyFramework/MyFramework.h>
+// CHECK-NEXT: #import <MyFramework/MyHeader.h>
+// CHECK-NEXT: #import "QuotedHeader.h"
+
+//--- headermap.hmap.json.in
+{
+  "mappings" :
+    {
+     "MyFramework/MyHeader.h" : "SRCROOT/MyHeader.h"
+    }
+}
+
+//--- MyFramework.h
+// Umbrella for MyFramework
+#import <MyFramework/MyHeader.h>
+// expected-no-diagnostics
+
+//--- MyHeader.h
+#import <OtherFramework/OtherHeader.h>
+int MyInt;
+// expected-no-diagnostics
+
+//--- QuotedHeader.h
+char MyChar;
+// expected-no-diagnostics
+
+//--- Frameworks/OtherFramework.framework/Headers/OtherHeader.h
+int OtherInt;
+// expected-no-diagnostics
+
+//--- reference.output.json.in
+{
+  "metadata": {
+    "formatVersion": {
+      "major": 0,
+      "minor": 5,
+      "patch": 3
+    },
+    "generator": "?"
+  },
+  "module": {
+    "name": "MyFramework",
+    "platform": {
+      "architecture": "arm64",
+      "operatingSystem": {
+        "minimumVersion": {
+          "major": 11,
+          "minor": 0,
+          "patch": 0
+        },
+        "name": "macosx"
+      },
+      "vendor": "apple"
+    }
+  },
+  "relationships": [],
+  "symbols": [
+    {
+      "accessLevel": "public",
+      "declarationFragments": [
+        {
+          "kind": "typeIdentifier",
+          "preciseIdentifier": "c:I",
+          "spelling": "int"
+        },
+        {
+          "kind": "text",
+          "spelling": " "
+        },
+        {
+          "kind": "identifier",
+          "spelling": "MyInt"
+        }
+      ],
+      "identifier": {
+        "interfaceLanguage": "objective-c",
+        "precise": "c:@MyInt"
+      },
+      "kind": {
+        "displayName": "Global Variable",
+        "identifier": "objective-c.var"
+      },
+      "location": {
+        "position": {
+          "character": 5,
+          "line": 2
+        },
+        "uri": "file://SRCROOT/MyHeader.h"
+      },
+      "names": {
+        "navigator": [
+          {
+            "kind": "identifier",
+            "spelling": "MyInt"
+          }
+        ],
+        "subHeading": [
+          {
+            "kind": "identifier",
+            "spelling": "MyInt"
+          }
+        ],
+        "title": "MyInt"
+      },
+      "pathComponents": [
+        "MyInt"
+      ]
+    },
+    {
+      "accessLevel": "public",
+      "declarationFragments": [
+        {
+          "kind": "typeIdentifier",
+          "preciseIdentifier": "c:C",
+          "spelling": "char"
+        },
+        {
+          "kind": "text",
+          "spelling": " "
+        },
+        {
+          "kind": "identifier",
+          "spelling": "MyChar"
+        }
+      ],
+      "identifier": {
+        "interfaceLanguage": "objective-c",
+        "precise": "c:@MyChar"
+      },
+      "kind": {
+        "displayName": "Global Variable",
+        "identifier": "objective-c.var"
+      },
+      "location": {
+        "position": {
+          "character": 6,
+          "line": 1
+        },
+        "uri": "file://SRCROOT/QuotedHeader.h"
+      },
+      "names": {
+        "navigator": [
+          {
+            "kind": "identifier",
+            "spelling": "MyChar"
+          }
+        ],
+        "subHeading": [
+          {
+            "kind": "identifier",
+            "spelling": "MyChar"
+          }
+        ],
+        "title": "MyChar"
+      },
+      "pathComponents": [
+        "MyChar"
+      ]
+    }
+  ]
+}