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

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 si

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

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://lis

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

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 ___

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

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 ht

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

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@

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

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 w

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

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 G

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

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

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

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/ h

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

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

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

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 ___

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

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,

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

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

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

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 ___

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

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

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

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 ___

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

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 ___

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

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 migh

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

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.l

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

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(Compil

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

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'

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

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/P

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

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 SI

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

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.g

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

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-commi

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

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.llv

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

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.l

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

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]>; ---

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

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://revie

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

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://

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

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 `u

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

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-commi

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

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(Cmd

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

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

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

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 h

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

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] D30768: [PATCH][VFS] Ignore broken symlinks in the directory iterator.

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://rev

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

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/VirtualF

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

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 == nul

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

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:

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

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 skipp