[PATCH] D51189: [Sema][ObjC] Infer availability of +new from availability of -init

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added a comment.

TAPI mostly cares about linkable symbols, so this shouldn't be a problem.


Repository:
  rC Clang

https://reviews.llvm.org/D51189



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55151: [gcov/Darwin] Ensure external symbols are exported when using an export list

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added inline comments.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:1037
   if (hasExportSymbolDirective(Args)) {
-addExportedSymbol(CmdArgs, "___llvm_profile_filename");
-addExportedSymbol(CmdArgs, "___llvm_profile_raw_version");
-addExportedSymbol(CmdArgs, "_lprofCurFilename");
+if (needsGCovInstrumentation(Args)) {
+  addExportedSymbol(CmdArgs, "___gcov_flush");

Are the symbols mutually exclusive?


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

https://reviews.llvm.org/D55151



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55151: [gcov/Darwin] Ensure external symbols are exported when using an export list

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka accepted this revision.
ributzka added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D55151



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67249: [Modules][PCH] Hash input files content

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added a comment.

Did you try xxHash64?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67249



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67249: [Modules][PCH] Hash input files content

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added a comment.

xxhash64 is apparently faster than MD5 and SHA1, and produces good quality 
hashes. I am not sure about the hash quality of hash_code for this purpose.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67249



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80751: [clang][diagnostics] Add '-Wundef-prefix' warning option

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added inline comments.



Comment at: clang/include/clang/Driver/Options.td:475
+  Alias,
+  HelpText<"Aliased to '-Wundef-prefix=\"\"', enable warnings for undefined 
macros">;
 def Wwrite_strings : Flag<["-"], "Wwrite-strings">, Group, 
Flags<[CC1Option, HelpHidden]>;

We should provide a description that doesn't mention that this is an alias of 
`-Wundef-prefix`, because the alias is an implementation detail.



Comment at: clang/lib/Lex/PPExpressions.cpp:262
+  // string to UndefPrefixes as an explicit "-Wundef" does.
+  if (UndefPrefixes.empty() ||
+  llvm::any_of(UndefPrefixes,

What happens when you use `-Werror=undef` and `-Wundef-prefix=`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80751



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80751: [clang][diagnostics] Add '-Wundef-prefix' warning option

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka accepted this revision.
ributzka added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80751



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132971: [clang][modules] Don't hard code [no_undeclared_includes] for the Darwin module

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka accepted this revision.
ributzka added a comment.
This revision is now accepted and ready to land.

Seems straightforward. LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132971

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122175: [clang][extract-api] Enable processing of multiple headers

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka accepted this revision.
ributzka added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122175

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119479: [clang][extract-api] Add global record support

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added inline comments.



Comment at: clang/include/clang/SymbolGraph/AvailabilityInfo.h:31
+  bool Unavailable{false};
+  bool UnconditionallyDeprecated{false};
+

We also need unconditionally unavailable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119479

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121936: Ensure -extract-api handles multiple headers correctly

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added inline comments.



Comment at: clang/include/clang/Driver/Phases.h:17
   /// compilation process which interact with user options.
-  enum ID {
-Preprocess,

Is this an unrelated formatting change?



Comment at: clang/lib/Driver/Phases.cpp:19
   case Precompile: return "precompiler";
-  case Compile: return "compiler";
   case Backend: return "backend";

Unrelated format change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121936

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121936: Ensure -extract-api handles multiple headers correctly

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added a comment.

Nice! Thank you for adding support for multiple headers. Is this a step towards 
the json input file list?




Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:467
 
+def err_drv_extract_api_wrong_kind : Error<
+  "header file '%0' input '%1' does not match the type of prior input "

Could you please add a test for this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121936

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122175: [clang][extract-api] Enable processing of multiple headers

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added inline comments.



Comment at: clang/include/clang/SymbolGraph/FrontendActions.h:35
+  /// This is called before executing the action on any inputs. This merges all
+  /// the provided headers into a single header for processing.
+  bool PrepareToExecuteAction(CompilerInstance &CI) override;

This comment confused me a bit. By looking at the code further down, it only 
adds include statements for each header, which is different from merging all 
headers into a single header/buffer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122175

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added a comment.

In D123831#3458774 , @dang wrote:

> In D123831#3455048 , @cishida wrote:
>
>>> we might not always want to transform an absolute path because the 
>>> resulting relative include name might get remapped in a headermap, for 
>>> example in test known_files_only_hmap.c. But how does it work with modules 
>>> where we need relative includes? Is the setup in known_files_only_hmap even 
>>> valid?
>>
>> I think, in most cases, this shouldn't matter because if the header path 
>> input doesn't match the location stored in the header map, they should still 
>> have the same source content. The same should be true with header search 
>> resolution with modules & vfsoverlay
>
> Agreed, I think it would be classified as a user error to remap to different 
> source content via headermap.

This is happening today in Xcode. Headers that are mapped from the DSTROOT back 
to the SRCROOT can be different, because they are not simply copied. Xcode also 
runs unifdef on them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123831

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67249: [Modules][PCH] Hash input files content

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added inline comments.



Comment at: clang/lib/Serialization/ASTWriter.cpp:1767
   bool IsTopLevelModuleMap;
+  uint32_t ContentHash[2];
 };

bruno wrote:
> aprantl wrote:
> > Why is this not a uint64_t?
> Serializing a `uint64_t` directly instead of two `uint32_t` gives me a 
> slightly bigger final cache. One could argue that the value is negligible 
> (+800 bytes for a 40MB cache), but here's the rationale :)
Creating an abbrev might fix this, because this should be a fixed size field. 
The generic emission code for a record without an abbrev is not very space 
efficient for single fields.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67249



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35385: [Driver] Darwin: Link in the profile runtime archive first

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka accepted this revision.
ributzka added a comment.
This revision is now accepted and ready to land.

Nice cleanup with the RuntimeLinkOptions enum. LGTM


https://reviews.llvm.org/D35385



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136019: [clang][lex] Avoid `DirectoryLookup` copies

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added inline comments.



Comment at: clang/test/Modules/file-manager-lookup-count.m:73
+// RUN: FileCheck --input-file %t/stats %s
+// CHECK:  {
+// CHECK-NEXT:   "file-search.NumDirCacheMisses": 156,

The content of the stats file may change over time, so the test shouldn't limit 
it to just the listed entries. I would drop the first and last `CHECK` and 
change `CHECK-NEXT` to just `CHECK`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136019

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136019: [clang][lex] Avoid `DirectoryLookup` copies

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka accepted this revision.
ributzka added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136019

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135232: [modules] Allow to validate system headers less often with `-fmodules-validate-once-per-build-session`.

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added a comment.

This makes sense. We shouldn't re-validate system headers when 
`-fmodules-validate-once-per-build-session` is passed too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135232

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135295: [clang][ExtractAPI] Don't print locations for anonymous tags

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added a comment.

This doesn't affect any tests?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135295

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135232: [modules] Allow to validate system headers less often with `-fmodules-validate-once-per-build-session`.

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka accepted this revision.
ributzka added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135232

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136019: [clang][lex] Avoid `DirectoryLookup` copies

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added a comment.

We could invoke clang with the `-stats` option and compare the result against 
the expected number of stat calls.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136019

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159312: [Headers] Remove a space in NULL define

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added a comment.

Are we keeping the comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159312

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159383: [Headers] Remove musl-related comment about NULL

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka accepted this revision.
ributzka added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159383

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159312: [Headers] Remove a space in NULL define

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added a comment.

Thank you @aheejin


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159312

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157810: [clang][ExtractAPI] Create extractapi::RecordLocation

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added inline comments.



Comment at: clang/include/clang/ExtractAPI/API.h:141
+  unsigned Line, Col;
+  std::string Filename;
+

There is an opportunity for optimization by avoiding the allocation of separate 
strings for each source location, especially since many source locations will 
be in the same file. As an example, APISet utilizes a BumpPtrAllocator to 
allocate and deduplicate strings. It is recommended to consider using the same 
allocator or a similar concept.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157810

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146255: [clang] Unconditionally add autolink hints for frameworks.

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added a comment.

Thanks for the prompt reviews


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146255

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146255: [clang] Unconditionally add autolink hints for frameworks.

[Juergen Ributzka via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG29e2a4eff8f6: [clang] Unconditionally add autolink hints for 
frameworks. (authored by ributzka).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146255

Files:
  clang/lib/Lex/ModuleMap.cpp
  clang/test/Modules/use-exportas-for-link.m


Index: clang/test/Modules/use-exportas-for-link.m
===
--- clang/test/Modules/use-exportas-for-link.m
+++ clang/test/Modules/use-exportas-for-link.m
@@ -31,15 +31,17 @@
 #endif
 
 // RUN: %clang_cc1 -emit-llvm -o - -fmodules-cache-path=%t -DE -fmodules 
-fimplicit-module-maps -F %S/Inputs/exportas-link %s | FileCheck 
--check-prefix=CHECK_E %s
-// CHECK_E: !llvm.linker.options = !{![[MODULE:[0-9]+]]}
-// CHECK_E: ![[MODULE]] = !{!"-framework", !"SomeKitCore"}
+// CHECK_E: !llvm.linker.options = !{![[MODULE1:[0-9]+]], ![[MODULE2:[0-9]+]]}
+// CHECK_E: ![[MODULE1]] = !{!"-framework", !"OtherKit"}
+// CHECK_E: ![[MODULE2]] = !{!"-framework", !"SomeKitCore"}
 #ifdef E
 @import OtherKit;
 #endif
 
 // RUN: %clang_cc1 -emit-llvm -o - -fmodules-cache-path=%t -DF -fmodules 
-fimplicit-module-maps -F %S/Inputs/exportas-link %s | FileCheck 
--check-prefix=CHECK_F %s
-// CHECK_F: !llvm.linker.options = !{![[MODULE:[0-9]+]]}
-// CHECK_F: ![[MODULE]] = !{!"-framework", !"SomeKit"}
+// CHECK_F: !llvm.linker.options = !{![[MODULE1:[0-9]+]], ![[MODULE2:[0-9]+]]}
+// CHECK_F: ![[MODULE1]] = !{!"-framework", !"OtherKit"}
+// CHECK_F: ![[MODULE2]] = !{!"-framework", !"SomeKit"}
 #ifdef F
 @import OtherKit;
 #endif
Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -930,27 +930,13 @@
 
 /// For a framework module, infer the framework against which we
 /// should link.
-static void inferFrameworkLink(Module *Mod, const DirectoryEntry *FrameworkDir,
-   FileManager &FileMgr) {
+static void inferFrameworkLink(Module *Mod) {
   assert(Mod->IsFramework && "Can only infer linking for framework modules");
   assert(!Mod->isSubFramework() &&
  "Can only infer linking for top-level frameworks");
 
-  SmallString<128> LibName;
-  LibName += FrameworkDir->getName();
-  llvm::sys::path::append(LibName, Mod->Name);
-
-  // The library name of a framework has more than one possible extension since
-  // the introduction of the text-based dynamic library format. We need to 
check
-  // for both before we give up.
-  for (const char *extension : {"", ".tbd"}) {
-llvm::sys::path::replace_extension(LibName, extension);
-if (FileMgr.getFile(LibName)) {
-  Mod->LinkLibraries.push_back(Module::LinkLibrary(Mod->Name,
-   /*IsFramework=*/true));
-  return;
-}
-  }
+  Mod->LinkLibraries.push_back(Module::LinkLibrary(Mod->Name,
+   /*IsFramework=*/true));
 }
 
 Module *ModuleMap::inferFrameworkModule(const DirectoryEntry *FrameworkDir,
@@ -1129,9 +1115,8 @@
 
   // If the module is a top-level framework, automatically link against the
   // framework.
-  if (!Result->isSubFramework()) {
-inferFrameworkLink(Result, FrameworkDir, FileMgr);
-  }
+  if (!Result->isSubFramework())
+inferFrameworkLink(Result);
 
   return Result;
 }
@@ -2185,9 +2170,8 @@
   // If the active module is a top-level framework, and there are no link
   // libraries, automatically link against the framework.
   if (ActiveModule->IsFramework && !ActiveModule->isSubFramework() &&
-  ActiveModule->LinkLibraries.empty()) {
-inferFrameworkLink(ActiveModule, Directory, SourceMgr.getFileManager());
-  }
+  ActiveModule->LinkLibraries.empty())
+inferFrameworkLink(ActiveModule);
 
   // If the module meets all requirements but is still unavailable, mark the
   // whole tree as unavailable to prevent it from building.


Index: clang/test/Modules/use-exportas-for-link.m
===
--- clang/test/Modules/use-exportas-for-link.m
+++ clang/test/Modules/use-exportas-for-link.m
@@ -31,15 +31,17 @@
 #endif
 
 // RUN: %clang_cc1 -emit-llvm -o - -fmodules-cache-path=%t -DE -fmodules -fimplicit-module-maps -F %S/Inputs/exportas-link %s | FileCheck --check-prefix=CHECK_E %s
-// CHECK_E: !llvm.linker.options = !{![[MODULE:[0-9]+]]}
-// CHECK_E: ![[MODULE]] = !{!"-framework", !"SomeKitCore"}
+// CHECK_E: !llvm.linker.options = !{![[MODULE1:[0-9]+]], ![[MODULE2:[0-9]+]]}
+// CHECK_E: ![[MODULE1]] = !{!"-framework", !"OtherKit"}
+// CHECK_E: ![[MODULE2]] = !{!"-framework", !"SomeKitCore"}
 #ifdef E
 @import OtherKit;
 #endif
 
 // RUN: %clang_cc1 -emit-llvm -o - -fmodules-cache-path=%t -DF -fmodules -fimplicit-module-maps -F %S/Inputs/exportas-link %s | FileCheck --check-prefix=CHECK_

[PATCH] D146328: [clang][deps] Only cache files with specific extension

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added inline comments.



Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:161
 
-/// Whitelist file extensions that should be minimized, treating no extension 
as
-/// a source file that should be minimized.
+/// Whitelist file extensions that should be cached/scanned.
 ///

s/Whitelist/Allowlist


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146328

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150689: [clang][DependencyScanner] Remove all warning flags when suppressing warnings

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka accepted this revision.
ributzka added a comment.

I see. LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150494: [clang][modules] NFC: Only sort interesting identifiers

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added inline comments.



Comment at: clang/lib/Serialization/ASTWriter.cpp:3646
 // file.
 SmallVector IIs;
 for (const auto &ID : PP.getIdentifierTable())

Would it make sense to reduce the size of the SmallVector too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150494

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150689: [clang][DependencyScanner] Remove all warning flags when suppressing warnings

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added inline comments.



Comment at: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp:50
+  // optimize.
+  if (!IsSystemModule)
+return;

This removes also all warnings when building your own module, because of the 
`[system]` attribute. I would prefer this optimization to be limited to modules 
that are in the sysroot and therefore ignore the `[system]` attribute. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150689

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30768: [PATCH][VFS] Ignore broken symlinks in the directory iterator.

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka created this revision.

The VFS directory iterator and recursive directory iterator behave differently
from the LLVM counterparts. Once the VFS iterators hit a broken symlink they
immediately abort. The LLVM counterparts allow to recover from this issue by
clearing the error code and skipping to the next entry.

  

This change adds the same functionality to the VFS iterators. There should be
no change in current behavior in the current CLANG source base, because all
clients have loop exit conditions that also check the error code.


https://reviews.llvm.org/D30768

Files:
  include/clang/Basic/VirtualFileSystem.h
  lib/Basic/VirtualFileSystem.cpp
  unittests/Basic/VirtualFileSystemTest.cpp

Index: unittests/Basic/VirtualFileSystemTest.cpp
===
--- unittests/Basic/VirtualFileSystemTest.cpp
+++ unittests/Basic/VirtualFileSystemTest.cpp
@@ -305,6 +305,22 @@
   }
   operator StringRef() { return Path.str(); }
 };
+
+struct ScopedLink {
+  SmallString<128> Path;
+  ScopedLink(const Twine &To, const Twine &From) {
+Path = From.str();
+std::error_code EC = sys::fs::create_link(To, From);
+if (EC)
+  Path = "";
+EXPECT_FALSE(EC);
+  }
+  ~ScopedLink() {
+if (Path != "")
+  EXPECT_FALSE(llvm::sys::fs::remove(Path.str()));
+  }
+  operator StringRef() { return Path.str(); }
+};
 } // end anonymous namespace
 
 TEST(VirtualFileSystemTest, BasicRealFSIteration) {
@@ -334,6 +350,35 @@
   EXPECT_EQ(vfs::directory_iterator(), I);
 }
 
+TEST(VirtualFileSystemTest, BrokenSymlinkRealFSIteration) {
+  ScopedDir TestDirectory("virtual-file-system-test", /*Unique*/ true);
+  IntrusiveRefCntPtr FS = vfs::getRealFileSystem();
+
+  ScopedLink _a("no_such_file", TestDirectory + "/a");
+  ScopedDir _b(TestDirectory + "/b");
+  ScopedLink _c("no_such_file", TestDirectory + "/c");
+
+  std::error_code EC;
+  vfs::directory_iterator I = FS->dir_begin(Twine(TestDirectory), EC);
+  EXPECT_TRUE(EC);
+  EXPECT_NE(vfs::directory_iterator(), I);
+  EC = std::error_code();
+  EXPECT_TRUE(I->getName() == _a);
+  I.increment(EC);
+  EXPECT_FALSE(EC);
+  EXPECT_NE(vfs::directory_iterator(), I);
+  EXPECT_TRUE(I->getName() == _b);
+  I.increment(EC);
+  EXPECT_TRUE(EC);
+  EXPECT_NE(vfs::directory_iterator(), I);
+  EC = std::error_code();
+  EXPECT_NE(vfs::directory_iterator(), I);
+  EXPECT_TRUE(I->getName() == _c);
+  I.increment(EC);
+  EXPECT_FALSE(EC);
+  EXPECT_EQ(vfs::directory_iterator(), I);
+}
+
 TEST(VirtualFileSystemTest, BasicRealFSRecursiveIteration) {
   ScopedDir TestDirectory("virtual-file-system-test", /*Unique*/true);
   IntrusiveRefCntPtr FS = vfs::getRealFileSystem();
@@ -373,6 +418,44 @@
   EXPECT_EQ(1, Counts[3]); // d
 }
 
+TEST(VirtualFileSystemTest, BrokenSymlinkRealFSRecursiveIteration) {
+  ScopedDir TestDirectory("virtual-file-system-test", /*Unique*/ true);
+  IntrusiveRefCntPtr FS = vfs::getRealFileSystem();
+
+  ScopedLink _a("no_such_file", TestDirectory + "/a");
+  ScopedDir _b(TestDirectory + "/b");
+  ScopedLink _ba("no_such_file", TestDirectory + "/b/a");
+  ScopedDir _bb(TestDirectory + "/b/b");
+  ScopedLink _bc("no_such_file", TestDirectory + "/b/c");
+  ScopedLink _c("no_such_file", TestDirectory + "/c");
+  ScopedDir _d(TestDirectory + "/d");
+  ScopedDir _dd(TestDirectory + "/d/d");
+  ScopedDir _ddd(TestDirectory + "/d/d/d");
+  ScopedLink _e("no_such_file", TestDirectory + "/e");
+
+  std::vector Contents;
+  std::error_code EC;
+  for (vfs::recursive_directory_iterator I(*FS, Twine(TestDirectory), EC), E;
+   I != E; I.increment(EC)) {
+// Skip broken symlinks.
+if (EC == std::errc::no_such_file_or_directory) {
+  EC = std::error_code();
+  continue;
+} else if (EC) {
+  break;
+}
+Contents.push_back(I->getName());
+  }
+
+  // Check contents.
+  EXPECT_EQ(5U, Contents.size());
+  EXPECT_TRUE(Contents[0] == _b);
+  EXPECT_TRUE(Contents[1] == _bb);
+  EXPECT_TRUE(Contents[2] == _d);
+  EXPECT_TRUE(Contents[3] == _dd);
+  EXPECT_TRUE(Contents[4] == _ddd);
+}
+
 template 
 static void checkContents(DirIter I, ArrayRef ExpectedOut) {
   std::error_code EC;
Index: lib/Basic/VirtualFileSystem.cpp
===
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -246,8 +246,7 @@
 if (!EC && Iter != llvm::sys::fs::directory_iterator()) {
   llvm::sys::fs::file_status S;
   EC = Iter->status(S);
-  if (!EC)
-CurrentEntry = Status::copyWithNewName(S, Iter->path());
+  CurrentEntry = Status::copyWithNewName(S, Iter->path());
 }
   }
 
@@ -1858,7 +1857,7 @@
std::error_code &EC)
 : FS(&FS_) {
   directory_iterator I = FS->dir_begin(Path, EC);
-  if (!EC && I != directory_iterator()) {
+  if (I != directory_iterator()) {
 State = std::make_shared();
 State->push(I);
   }
@@ -1871,7 +1870,7 @@
   v

[PATCH] D30768: [PATCH][VFS] Ignore broken symlinks in the directory iterator.

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added inline comments.



Comment at: lib/Basic/VirtualFileSystem.cpp:1873
 vfs::directory_iterator I = FS->dir_begin(State->top()->getName(), EC);
-if (EC)
+if (EC && EC != std::errc::no_such_file_or_directory)
   return *this;

bruno wrote:
> Can you add a comment explaining why you are doing it? I would prefer if we 
> reset the `EC` state here than having the callers ignoring `EC` results.
If we reset the EC here, then the caller won't know that there was an issue. 
The idea is that we still want the caller to check EC. It should be up to the 
caller to decide how to act on this particular error.

I guess since the caller has to check for the error anyway I could even remove 
this check completely and not check EC at all here.


https://reviews.llvm.org/D30768



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30768: [PATCH][VFS] Ignore broken symlinks in the directory iterator.

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added inline comments.



Comment at: include/clang/Basic/VirtualFileSystem.h:164
 EC = Impl->increment();
-if (EC || !Impl->CurrentEntry.isStatusKnown())
+if (!Impl->CurrentEntry.isStatusKnown())
   Impl.reset(); // Normalize the end iterator to Impl == nullptr.

bruno wrote:
> I would rather we don't drop checks for `EC`s - it's commonly assumed that 
> whenever they are passed in we wanna check them for errors, can't you just 
> skip the specific case you want to avoid?
EC is an output parameter here. The client is supposed to check it.


https://reviews.llvm.org/D30768



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30768: [PATCH][VFS] Ignore broken symlinks in the directory iterator.

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka updated this revision to Diff 91371.
ributzka added a comment.

Remove the EC check completely.


https://reviews.llvm.org/D30768

Files:
  include/clang/Basic/VirtualFileSystem.h
  lib/Basic/VirtualFileSystem.cpp
  unittests/Basic/VirtualFileSystemTest.cpp

Index: unittests/Basic/VirtualFileSystemTest.cpp
===
--- unittests/Basic/VirtualFileSystemTest.cpp
+++ unittests/Basic/VirtualFileSystemTest.cpp
@@ -305,6 +305,22 @@
   }
   operator StringRef() { return Path.str(); }
 };
+
+struct ScopedLink {
+  SmallString<128> Path;
+  ScopedLink(const Twine &To, const Twine &From) {
+Path = From.str();
+std::error_code EC = sys::fs::create_link(To, From);
+if (EC)
+  Path = "";
+EXPECT_FALSE(EC);
+  }
+  ~ScopedLink() {
+if (Path != "")
+  EXPECT_FALSE(llvm::sys::fs::remove(Path.str()));
+  }
+  operator StringRef() { return Path.str(); }
+};
 } // end anonymous namespace
 
 TEST(VirtualFileSystemTest, BasicRealFSIteration) {
@@ -334,6 +350,35 @@
   EXPECT_EQ(vfs::directory_iterator(), I);
 }
 
+TEST(VirtualFileSystemTest, BrokenSymlinkRealFSIteration) {
+  ScopedDir TestDirectory("virtual-file-system-test", /*Unique*/ true);
+  IntrusiveRefCntPtr FS = vfs::getRealFileSystem();
+
+  ScopedLink _a("no_such_file", TestDirectory + "/a");
+  ScopedDir _b(TestDirectory + "/b");
+  ScopedLink _c("no_such_file", TestDirectory + "/c");
+
+  std::error_code EC;
+  vfs::directory_iterator I = FS->dir_begin(Twine(TestDirectory), EC);
+  EXPECT_TRUE(EC);
+  EXPECT_NE(vfs::directory_iterator(), I);
+  EC = std::error_code();
+  EXPECT_TRUE(I->getName() == _a);
+  I.increment(EC);
+  EXPECT_FALSE(EC);
+  EXPECT_NE(vfs::directory_iterator(), I);
+  EXPECT_TRUE(I->getName() == _b);
+  I.increment(EC);
+  EXPECT_TRUE(EC);
+  EXPECT_NE(vfs::directory_iterator(), I);
+  EC = std::error_code();
+  EXPECT_NE(vfs::directory_iterator(), I);
+  EXPECT_TRUE(I->getName() == _c);
+  I.increment(EC);
+  EXPECT_FALSE(EC);
+  EXPECT_EQ(vfs::directory_iterator(), I);
+}
+
 TEST(VirtualFileSystemTest, BasicRealFSRecursiveIteration) {
   ScopedDir TestDirectory("virtual-file-system-test", /*Unique*/true);
   IntrusiveRefCntPtr FS = vfs::getRealFileSystem();
@@ -373,6 +418,44 @@
   EXPECT_EQ(1, Counts[3]); // d
 }
 
+TEST(VirtualFileSystemTest, BrokenSymlinkRealFSRecursiveIteration) {
+  ScopedDir TestDirectory("virtual-file-system-test", /*Unique*/ true);
+  IntrusiveRefCntPtr FS = vfs::getRealFileSystem();
+
+  ScopedLink _a("no_such_file", TestDirectory + "/a");
+  ScopedDir _b(TestDirectory + "/b");
+  ScopedLink _ba("no_such_file", TestDirectory + "/b/a");
+  ScopedDir _bb(TestDirectory + "/b/b");
+  ScopedLink _bc("no_such_file", TestDirectory + "/b/c");
+  ScopedLink _c("no_such_file", TestDirectory + "/c");
+  ScopedDir _d(TestDirectory + "/d");
+  ScopedDir _dd(TestDirectory + "/d/d");
+  ScopedDir _ddd(TestDirectory + "/d/d/d");
+  ScopedLink _e("no_such_file", TestDirectory + "/e");
+
+  std::vector Contents;
+  std::error_code EC;
+  for (vfs::recursive_directory_iterator I(*FS, Twine(TestDirectory), EC), E;
+   I != E; I.increment(EC)) {
+// Skip broken symlinks.
+if (EC == std::errc::no_such_file_or_directory) {
+  EC = std::error_code();
+  continue;
+} else if (EC) {
+  break;
+}
+Contents.push_back(I->getName());
+  }
+
+  // Check contents.
+  EXPECT_EQ(5U, Contents.size());
+  EXPECT_TRUE(Contents[0] == _b);
+  EXPECT_TRUE(Contents[1] == _bb);
+  EXPECT_TRUE(Contents[2] == _d);
+  EXPECT_TRUE(Contents[3] == _dd);
+  EXPECT_TRUE(Contents[4] == _ddd);
+}
+
 template 
 static void checkContents(DirIter I, ArrayRef ExpectedOut) {
   std::error_code EC;
Index: lib/Basic/VirtualFileSystem.cpp
===
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -246,8 +246,7 @@
 if (!EC && Iter != llvm::sys::fs::directory_iterator()) {
   llvm::sys::fs::file_status S;
   EC = Iter->status(S);
-  if (!EC)
-CurrentEntry = Status::copyWithNewName(S, Iter->path());
+  CurrentEntry = Status::copyWithNewName(S, Iter->path());
 }
   }
 
@@ -1858,7 +1857,7 @@
std::error_code &EC)
 : FS(&FS_) {
   directory_iterator I = FS->dir_begin(Path, EC);
-  if (!EC && I != directory_iterator()) {
+  if (I != directory_iterator()) {
 State = std::make_shared();
 State->push(I);
   }
@@ -1871,8 +1870,6 @@
   vfs::directory_iterator End;
   if (State->top()->isDirectory()) {
 vfs::directory_iterator I = FS->dir_begin(State->top()->getName(), EC);
-if (EC)
-  return *this;
 if (I != End) {
   State->push(I);
   return *this;
Index: include/clang/Basic/VirtualFileSystem.h
===
--- include/clang/Basic/VirtualFileSystem.h
+++ include/clang/Basic/VirtualFileSystem.h
@

[PATCH] D30768: [PATCH][VFS] Ignore broken symlinks in the directory iterator.

[Juergen Ributzka via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL297510: [VFS] Ignore broken symlinks in the directory 
iterator. (authored by ributzka).

Changed prior to commit:
  https://reviews.llvm.org/D30768?vs=91371&id=91403#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30768

Files:
  cfe/trunk/include/clang/Basic/VirtualFileSystem.h
  cfe/trunk/lib/Basic/VirtualFileSystem.cpp
  cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp

Index: cfe/trunk/include/clang/Basic/VirtualFileSystem.h
===
--- cfe/trunk/include/clang/Basic/VirtualFileSystem.h
+++ cfe/trunk/include/clang/Basic/VirtualFileSystem.h
@@ -161,7 +161,7 @@
   directory_iterator &increment(std::error_code &EC) {
 assert(Impl && "attempting to increment past end");
 EC = Impl->increment();
-if (EC || !Impl->CurrentEntry.isStatusKnown())
+if (!Impl->CurrentEntry.isStatusKnown())
   Impl.reset(); // Normalize the end iterator to Impl == nullptr.
 return *this;
   }
Index: cfe/trunk/lib/Basic/VirtualFileSystem.cpp
===
--- cfe/trunk/lib/Basic/VirtualFileSystem.cpp
+++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp
@@ -246,8 +246,7 @@
 if (!EC && Iter != llvm::sys::fs::directory_iterator()) {
   llvm::sys::fs::file_status S;
   EC = Iter->status(S);
-  if (!EC)
-CurrentEntry = Status::copyWithNewName(S, Iter->path());
+  CurrentEntry = Status::copyWithNewName(S, Iter->path());
 }
   }
 
@@ -1858,7 +1857,7 @@
std::error_code &EC)
 : FS(&FS_) {
   directory_iterator I = FS->dir_begin(Path, EC);
-  if (!EC && I != directory_iterator()) {
+  if (I != directory_iterator()) {
 State = std::make_shared();
 State->push(I);
   }
@@ -1871,8 +1870,6 @@
   vfs::directory_iterator End;
   if (State->top()->isDirectory()) {
 vfs::directory_iterator I = FS->dir_begin(State->top()->getName(), EC);
-if (EC)
-  return *this;
 if (I != End) {
   State->push(I);
   return *this;
Index: cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
===
--- cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
+++ cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
@@ -305,6 +305,22 @@
   }
   operator StringRef() { return Path.str(); }
 };
+
+struct ScopedLink {
+  SmallString<128> Path;
+  ScopedLink(const Twine &To, const Twine &From) {
+Path = From.str();
+std::error_code EC = sys::fs::create_link(To, From);
+if (EC)
+  Path = "";
+EXPECT_FALSE(EC);
+  }
+  ~ScopedLink() {
+if (Path != "")
+  EXPECT_FALSE(llvm::sys::fs::remove(Path.str()));
+  }
+  operator StringRef() { return Path.str(); }
+};
 } // end anonymous namespace
 
 TEST(VirtualFileSystemTest, BasicRealFSIteration) {
@@ -334,6 +350,35 @@
   EXPECT_EQ(vfs::directory_iterator(), I);
 }
 
+TEST(VirtualFileSystemTest, BrokenSymlinkRealFSIteration) {
+  ScopedDir TestDirectory("virtual-file-system-test", /*Unique*/ true);
+  IntrusiveRefCntPtr FS = vfs::getRealFileSystem();
+
+  ScopedLink _a("no_such_file", TestDirectory + "/a");
+  ScopedDir _b(TestDirectory + "/b");
+  ScopedLink _c("no_such_file", TestDirectory + "/c");
+
+  std::error_code EC;
+  vfs::directory_iterator I = FS->dir_begin(Twine(TestDirectory), EC);
+  EXPECT_TRUE(EC);
+  EXPECT_NE(vfs::directory_iterator(), I);
+  EC = std::error_code();
+  EXPECT_TRUE(I->getName() == _a);
+  I.increment(EC);
+  EXPECT_FALSE(EC);
+  EXPECT_NE(vfs::directory_iterator(), I);
+  EXPECT_TRUE(I->getName() == _b);
+  I.increment(EC);
+  EXPECT_TRUE(EC);
+  EXPECT_NE(vfs::directory_iterator(), I);
+  EC = std::error_code();
+  EXPECT_NE(vfs::directory_iterator(), I);
+  EXPECT_TRUE(I->getName() == _c);
+  I.increment(EC);
+  EXPECT_FALSE(EC);
+  EXPECT_EQ(vfs::directory_iterator(), I);
+}
+
 TEST(VirtualFileSystemTest, BasicRealFSRecursiveIteration) {
   ScopedDir TestDirectory("virtual-file-system-test", /*Unique*/true);
   IntrusiveRefCntPtr FS = vfs::getRealFileSystem();
@@ -373,6 +418,44 @@
   EXPECT_EQ(1, Counts[3]); // d
 }
 
+TEST(VirtualFileSystemTest, BrokenSymlinkRealFSRecursiveIteration) {
+  ScopedDir TestDirectory("virtual-file-system-test", /*Unique*/ true);
+  IntrusiveRefCntPtr FS = vfs::getRealFileSystem();
+
+  ScopedLink _a("no_such_file", TestDirectory + "/a");
+  ScopedDir _b(TestDirectory + "/b");
+  ScopedLink _ba("no_such_file", TestDirectory + "/b/a");
+  ScopedDir _bb(TestDirectory + "/b/b");
+  ScopedLink _bc("no_such_file", TestDirectory + "/b/c");
+  ScopedLink _c("no_such_file", TestDirectory + "/c");
+  ScopedDir _d(TestDirectory + "/d");
+  ScopedDir _dd(TestDirectory + "/d/d");
+  ScopedDir _ddd(TestDirectory + "/d/d/d");
+  ScopedLink _e("no_such_file", TestDirectory + "/e");
+
+  std::vector Contents;
+  std::error_code EC;
+  for (vfs::recur

[PATCH] D30768: [PATCH][VFS] Ignore broken symlinks in the directory iterator.

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka added a comment.

Thanks Bruno. Committed in r297510.


Repository:
  rL LLVM

https://reviews.llvm.org/D30768



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117809: [clang] Add an extract-api driver option

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka accepted this revision.
ributzka added a comment.
This revision is now accepted and ready to land.

LGTM. Just a few minor nits.




Comment at: clang/lib/Driver/ToolChains/Clang.cpp:7082
   CmdArgs.push_back(Args.MakeArgString(OutputFilename));
+} else if (Output.getType() == types::TY_API_INFO) {
+  SmallString<128> OutputFilename(Output.getFilename());

Is this change required for extract-api? I don't think the output file needs to 
be renamed, because there is only one output in this case.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2431
   {frontend::PrintDependencyDirectivesSourceMinimizerOutput,
-  OPT_print_dependency_directives_minimized_source},
   };

This looks like an unrelated change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117809

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117348: [Preprocessor] Reduce the memory overhead of `#define` directives

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka accepted this revision.
ributzka added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D117348

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99811: [TextAPI] move source code files out of subdirectory, NFC

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka accepted this revision.
ributzka added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99811

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits