[PATCH] D123831: [POC][WIP] Use relative include in extract-api

Daniel Grumberg via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 15 00:34:00 PDT 2022


dang added inline comments.


================
Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:65
+  // Matches framework include patterns
+  const llvm::Regex Rule("/(.+)\\.framework/(.+)?Headers/(.+)");
+  StringRef WorkingDir = CI.getFileSystemOpts().WorkingDir;
----------------
This does rely on the path separator being "/". If stick with this regex, do we need to convert all paths to POSIX format? I think the best thing to do is to iterate through the given path components and match for just "(.+)\\.framework" to match just the framework name and the use the base name of the file directly instead of matching it via the regex as well...


================
Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:185-186
+         CI.getPreprocessor().getHeaderSearchInfo().search_dir_range()) {
+      if (!DL.isHeaderMap())
+        continue;
+
----------------
Not sure that just accounting for the header map case is the correct thing to do here. Ideally we would like to know what the include string was, e.g. <MyFramework/MyHeader.h> and match that with how we included one of the original files. This would account for all remapping functionality.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123831/new/

https://reviews.llvm.org/D123831



More information about the cfe-commits mailing list