[PATCH] D158573: [clang][modules] Move `UNHASHED_CONTROL_BLOCK` up in the AST file

benlangmuir added inline comments. Comment at: clang/lib/Serialization/ASTWriter.cpp:1169 + writeSignature(Sig, Out); + std::copy_n(Out.begin(), Out.size(), Buffer.begin() + Offset); +}; jansvoboda11 wrote: > I don't feel great about removing `const

[PATCH] D158572: [clang][modules] Use relative offsets for input files

benlangmuir added inline comments. Comment at: clang/include/clang/Serialization/ModuleFile.h:249 + /// Absolute offset of the start of the input-files block. + uint64_t InputFilesOffsetBase; + jansvoboda11 wrote: > benlangmuir wrote: > > Doesn't `InputFilesCur

[PATCH] D158469: [clang][deps] Compute command lines and file deps on-demand

benlangmuir added a comment. I find this a bit hard to understand as far as object lifetime is concerned: you're storing the `ScanInstance` in `TranslationUnitDeps`, but that's a layer above the actual consumer interface, which means every consumer needs to understand how this lifetime is manag

[PATCH] D158469: [clang][deps] Compute command lines and file deps on-demand

benlangmuir added a comment. > I tried that approach, but found it way too easy to keep `ModuleDeps` around, > which keep scanning instances alive, and use tons of memory. It seems like the problem (too easy to keep around MD) is the same either way, it's just instead of wasting memory it's lea

[PATCH] D159016: [clang] Fix assertion failure using -MJ with -fsyntax-only

benlangmuir created this revision. benlangmuir added a reviewer: MaskRay. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. If there is no output filename we should not assert when writing output for -

[PATCH] D159016: [clang] Fix assertion failure using -MJ with -fsyntax-only

This revision was automatically updated to reflect the committed changes. Closed by commit rGaca23d8ac30d: [clang] Fix assertion failure using -MJ with -fsyntax-only (authored by benlangmuir). Changed prior to commit: https://reviews.llvm.org/D159016?vs=554046&id=554765#toc Repository: rG LL

[PATCH] D159016: [clang] Fix assertion failure using -MJ with -fsyntax-only

benlangmuir added inline comments. Comment at: clang/test/Driver/compilation_database_fsyntax_only.c:1 +// RUN: mkdir -p %t.workdir && cd %t.workdir +// RUN: %clang -fsyntax-only %s -MJ - 2>&1 | FileCheck %s MaskRay wrote: > The test can be added into `compilatio

[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

benlangmuir added inline comments. Comment at: clang/lib/Headers/module.modulemap:156 -module _Builtin_stddef_max_align_t [system] [extern_c] { - header "__stddef_max_align_t.h" Is the assumption here that directly `@import _Builtin_stddef_max_align_t` is uns

[PATCH] D138322: [Index] Add various missing USR generation

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. LGTM once the test failure is fixed Comment at: clang/test/Index/index-builtin-sve.cpp:7 + +// RUN: c-index-test -index-file %s -target-feature +bf16 -target-featur

[PATCH] D135841: [clang][deps][lex] Avoid canonicalization of remapped framework directories

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. Please add a comment about why this equality check is necessary. Comment at: clang/lib/Lex/ModuleMap.cpp:1331 +auto CanonicalDirEntry = FM.getDirectory(Canonica

[PATCH] D135841: [clang][deps][lex] Avoid canonicalization of remapped framework directories

benlangmuir added inline comments. Comment at: clang/lib/Lex/ModuleMap.cpp:1331 +auto CanonicalDirEntry = FM.getDirectory(CanonicalDir); +if (CanonicalDirEntry && *CanonicalDirEntry == *DirEntry) { + bool Done = llvm::sys::path::replace_path_prefix(Path, Dir, Canoni

[PATCH] D135634: [clang][modules] Serialize VFS overlay paths into PCMs

benlangmuir added a comment. > If the original HeaderSearchOptions didn't have any VFS overlay files, adopt > the PCM ones. IIUC this is what the patch does, right? > If the original HeaderSearchOptions did have VFS overlay files of its own, > error out if the PCM has different ones. Where di

[PATCH] D135634: [clang][modules] Serialize VFS overlay paths into PCMs

benlangmuir accepted this revision. benlangmuir added a comment. Thanks for the explanation, make sense. LGTM with the documentation tweak! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135634/new/ https://reviews.llvm.org/D135634

[PATCH] D139052: [NFC][Profile] Access profile through VirtualFileSystem

benlangmuir added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:194 : Diags(_Diags), HSOpts(HeaderSearchOpts), CodeGenOpts(CGOpts), -TargetOpts(TOpts), LangOpts(LOpts), TheModule(M), +TargetOpts(TOpts), LangOpts(LOpts), TheModule(M), VFS(VF

[PATCH] D77159: [pch] Honour -fallow-pch-with-compiler-errors for overall compilation status

benlangmuir created this revision. benlangmuir added a reviewer: akyrtzi. Herald added subscribers: cfe-commits, arphaman, dexonsmith. Herald added a project: clang. Previously we would emit a PCH with errors, but fail the overall compilation. If run using the driver, that would result in remo

[PATCH] D77159: [pch] Honour -fallow-pch-with-compiler-errors for overall compilation status

This revision was automatically updated to reflect the committed changes. Closed by commit rGcc3fddb411d5: [pch] Honour -fallow-pch-with-compiler-errors for overall compilation status (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D77180: Forward WrapperFrontendAction::shouldEraseOutputFiles()

benlangmuir created this revision. benlangmuir added a reviewer: akyrtzi. Herald added subscribers: cfe-commits, dexonsmith. Herald added a project: clang. Per the documentation, this class is supposed to forward every virtual method, but we had missed on (shouldEraseOutputFiles). This fixes using

[PATCH] D77180: Forward WrapperFrontendAction::shouldEraseOutputFiles()

This revision was automatically updated to reflect the committed changes. Closed by commit rGc322d328aa33: Forward WrapperFrontendAction::shouldEraseOutputFiles() (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77180/new/ ht

[PATCH] D76291: [Support] Fix formatted_raw_ostream for UTF-8

benlangmuir added inline comments. Comment at: llvm/include/llvm/Support/FormattedStream.h:49 + /// once we have the rest of it. + SmallString<4> PartialUTF8Char; + The changes related to `PartialUTF8Char` LGTM, thanks! Repository: rG LLVM Github Monorepo

[PATCH] D74371: [DirectoryWatcher] Fix misuse of FSEvents API and data race

benlangmuir created this revision. benlangmuir added reviewers: jkorous, akyrtzi. Herald added subscribers: cfe-commits, dexonsmith. Herald added a project: clang. I observed two bugs in the DirectoryWatcher on macOS 1. We were calling FSEventStreamStop and FSEventStreamInvalidate before

[PATCH] D74371: [DirectoryWatcher] Fix misuse of FSEvents API and data race

This revision was automatically updated to reflect the committed changes. Closed by commit rG2ac0c4b46ee2: [DirectoryWatcher] Fix misuse of FSEvents API and data race (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74371/new/

[PATCH] D44652: [vfs] Don't bail out after a missing -ivfsoverlay file

benlangmuir closed this revision. benlangmuir marked an inline comment as done. benlangmuir added a comment. r328337 Repository: rC Clang https://reviews.llvm.org/D44652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.or

[PATCH] D145098: [clang][deps] Preserve input ordering in the full output

benlangmuir added inline comments. Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:291 -std::unique_lock ul(Lock); -Inputs.push_back(std::move(ID)); +Inputs[InputIndex] = std::move(ID); } Since the input index is coming from "outside":

[PATCH] D145101: [clang][deps] NFC: Simplify worker loop

benlangmuir added inline comments. Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:802 for (unsigned I = 0; I < Pool.getThreadCount(); ++I) { -Pool.async([I, &Lock, &Index, &Inputs, &HadErrors, &FD, &PD, &WorkerTools, -&DependencyOS, &Errs]() { +

[PATCH] D145101: [clang][deps] NFC: Simplify worker loop

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. > I guess it was originally done this way because we have a number of > DependencyScanning{Tool,Worker} instances Oh of course, this makes sense. Yeah we could maybe find a differen

[PATCH] D144058: [clang][deps] Split lookupModuleOutput out of DependencyConsumer NFC

benlangmuir updated this revision to Diff 504196. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144058/new/ https://reviews.llvm.org/D144058 Files: clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h clang/include/clang/Tooling/DependencyScanning/DependencyScannin

[PATCH] D144058: [clang][deps] Split lookupModuleOutput out of DependencyConsumer NFC

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG296ba5bbd387: [clang][deps] Split lookupModuleOutput out of DependencyConsumer NFC (authored by benlangmuir). Repository: rG LLVM Github Monorepo

[PATCH] D145838: [clang][deps] Handle response files in dep scanner

benlangmuir created this revision. benlangmuir added reviewers: jansvoboda11, Bigcheese. Herald added a project: All. benlangmuir requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. Extract the code the driver uses to expand response fi

[PATCH] D145838: [clang][deps] Handle response files in dep scanner

This revision was automatically updated to reflect the committed changes. Closed by commit rGfcab930cd32e: [clang][deps] Handle response files in dep scanner (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145838/new/ https:

[PATCH] D153670: [clang/HeaderSearch] Make sure `loadSubdirectoryModuleMaps` doesn't cause loading of regular files

benlangmuir added inline comments. Comment at: clang/unittests/Tooling/DependencyScannerTest.cpp:274 +llvm::ErrorOr> +openFileForRead(const Twine &Path) override { + ReadFiles.push_back(Path.str()); Should we add `status` override as well? I think we

[PATCH] D154016: [clang][modules] Avoid serializing all diag mappings in non-deterministic order

benlangmuir created this revision. benlangmuir added reviewers: steven_wu, jansvoboda11, akyrtzi. Herald added a subscriber: mgrang. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When writing a pcm

[PATCH] D154016: [clang][modules] Avoid serializing all diag mappings in non-deterministic order

benlangmuir updated this revision to Diff 535882. benlangmuir added a comment. Extended test to include diagnostic pragmas CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154016/new/ https://reviews.llvm.org/D154016 Files: clang/include/clang/Basic/DiagnosticIDs.h clang/lib/Basic/Dia

[PATCH] D154016: [clang][modules] Avoid serializing all diag mappings in non-deterministic order

benlangmuir marked an inline comment as done. benlangmuir added inline comments. Comment at: clang/lib/Serialization/ASTWriter.cpp:3016 for (const auto &I : *State) { -if (I.second.isPragma() || IncludeNonPragmaStates) { - Record.push_back(I.first); -

[PATCH] D154016: [clang][modules] Avoid serializing all diag mappings in non-deterministic order

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. benlangmuir marked an inline comment as done. Closed by commit rG1ede7b47493f: [clang][modules] Avoid serializing all diag mappings in non-deterministic order (authored

[PATCH] D135849: [llvm] Return canonical virtual path from `RedirectingFileSystem`

benlangmuir added a comment. > Just realized this most likely won't work if the case-insensitive VFS is > specified with wrong spelling too, e.g. Fw.framework. Is this about the spelling in the VFS definition itself, or about the path being looked up? If it's the VFS definition maybe we can say

[PATCH] D154257: [clang] Fix leak in LoadFromCommandLineWorkingDirectory unit test

benlangmuir accepted this revision. benlangmuir added a comment. Gotta love `return AST.release()` being passed directly back into `std::unique_ptr(...)`. Thanks for fixing! Comment at: clang/tools/libclang/CIndex.cpp:3965 unsaved_files); - std::unique_ptr Unit(ASTUni

[PATCH] D147282: [clang][deps] Remove -coverage-data-file and -coverage-notes-file from modules

benlangmuir created this revision. benlangmuir added reviewers: akyrtzi, jansvoboda11. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When not performing codegen, we can strip the coverage-data-file

[PATCH] D147282: [clang][deps] Remove -coverage-data-file and -coverage-notes-file from modules

This revision was automatically updated to reflect the committed changes. Closed by commit rG758bca648385: [clang][deps] Remove -coverage-data-file and -coverage-notes-file from modules (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D147477: [clang][modules] Handle explicit modules when checking for .Private -> _Private

benlangmuir created this revision. benlangmuir added reviewers: akyrtzi, jansvoboda11. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. While we eventually want to remove the mapping from .Private to

[PATCH] D147477: [clang][modules] Handle explicit modules when checking for .Private -> _Private

benlangmuir updated this revision to Diff 510623. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147477/new/ https://reviews.llvm.org/D147477 Files: clang/lib/Frontend/CompilerInstance.cpp clang/test/Modules/implicit-private-with-submodule-explicit.m Index: clang/test/Modules/implic

[PATCH] D147477: [clang][modules] Handle explicit modules when checking for .Private -> _Private

benlangmuir updated this revision to Diff 510628. benlangmuir added a comment. Also test eagerly-loaded pcm CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147477/new/ https://reviews.llvm.org/D147477 Files: clang/lib/Frontend/CompilerInstance.cpp clang/test/Modules/implicit-private-

[PATCH] D147477: [clang][modules] Handle explicit modules when checking for .Private -> _Private

benlangmuir updated this revision to Diff 510629. benlangmuir added a comment. Upload correct diff this time CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147477/new/ https://reviews.llvm.org/D147477 Files: clang/lib/Frontend/CompilerInstance.cpp clang/test/Modules/implicit-private

[PATCH] D147477: [clang][modules] Handle explicit modules when checking for .Private -> _Private

This revision was automatically updated to reflect the committed changes. Closed by commit rG8ec36e6956cb: [clang][modules] Handle explicit modules when checking for .Private -> _Private (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D148176: [clang][modules] Avoid re-exporting PCH imports on every later module import

benlangmuir created this revision. benlangmuir added reviewers: akyrtzi, jansvoboda11, Bigcheese. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. We only want to make PCH imports visible once for the

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. > @benlangmuir, do you need any assistance from my side with it? @ivanmurashko thanks for your patience. I discussed this with some colleagues and we're in favour of making this fix.

[PATCH] D151581: [clang][modules] NFCI: Distinguish written/effective umbrella directories

benlangmuir accepted this revision. benlangmuir added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Basic/Module.h:655 + /// Retrieve the explicitly written umbrella directory for this module. + DirectoryName getWrittenUmbrell

[PATCH] D151586: [clang][modules] NFCI: Extract optionality out of `Module::{Header,DirectoryName}`

benlangmuir accepted this revision. benlangmuir added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Basic/Module.cpp:486 - if (Header H = getWrittenUmbrellaHeader()) { + if (auto H = getWrittenUmbrellaHeader()) { OS.indent(Indent

[PATCH] D151584: [clang][modules] NFCI: Use `DirectoryEntryRef` for umbrella directory

benlangmuir accepted this revision. benlangmuir added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Basic/Module.h:160 + llvm::PointerUnion Umbrella; Would it make sense to implement `PointerLikeTypeTrai

[PATCH] D151277: [clang][modules] Mark fewer identifiers as out-of-date

benlangmuir added inline comments. Comment at: clang/lib/Serialization/ASTReader.cpp:4413 +// first/next use via ASTReader::updateOutOfDateIdentifier(). +II = &PP.getIdentifierTable().get(Key); + } Why did this change from `getOwn` to `get`?

[PATCH] D151854: [clang] Use `FileEntryRef` in modular header search (part 1/2)

benlangmuir added a comment. Comment at: clang/include/clang/Lex/HeaderSearch.h:763 /// find this file due to requirements from \p RequestingModule. - bool findUsableModuleForHeader(const FileEntry *File, + bool findUsableModuleForHeader(OptionalFileEntryRef File,

[PATCH] D151852: [clang] NFCI: Use `FileEntryRef` in `ModuleMapCallbacks`

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. Nice simplification! Comment at: clang/include/clang/Lex/ModuleMap.h:72 /// \param Header The umbrella header to collect. - virtual void moduleMapAddUmbrellaHea

[PATCH] D151931: [clang] Remove `DirectoryEntry::getName()`

benlangmuir added a comment. Given this was a commonly used function before and there's a decent chance it's used out of tree somewhere, should we wait until the next llvm release branch has been cut before landing this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[PATCH] D151938: [clang][index] NFCI: Make `CXFile` a `FileEntryRef`

benlangmuir added inline comments. Comment at: clang/test/Modules/crash-vfs-umbrella-frameworks.m:13 // RUN: env FORCE_CLANG_DIAGNOSTICS_CRASH= TMPDIR=%t TEMP=%t TMP=%t \ -// RUN: not %clang -nostdinc -fsyntax-only %s \ +// RUN: not --crash %clang -nostdinc -fsyntax-only %s \ /

[PATCH] D151277: [clang][modules] Mark fewer identifiers as out-of-date

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. LGTM, but I would prefer at least one more person who understands the identifier handling code review this. It's been years since I thought about this code. Repository: rG LLVM G

[PATCH] D143446: [clang][deps] Ensure module invocation can be serialized

benlangmuir created this revision. benlangmuir added reviewers: jansvoboda11, Bigcheese. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When reseting modular options, propagate the values from certa

[PATCH] D143461: [ClangScanDeps] Fix data race in `clang-scan-deps` tool

benlangmuir added a comment. Dupe of https://reviews.llvm.org/D143428 ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143461/new/ https://reviews.llvm.org/D143461 ___ cfe-commits mailing list cfe-commits

[PATCH] D143414: [clang] refactor FileManager::GetUniqueIDMapping

benlangmuir added a comment. > This should allow the path serialization of input files to use the paths used > when looking up a file entry, instead of the last reference. Isn't this at odds with not having the VFS-mapped paths? It's not obvious to me why we want these specific semantics. Else

[PATCH] D143446: [clang][deps] Ensure module invocation can be serialized

benlangmuir updated this revision to Diff 495573. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143446/new/ https://reviews.llvm.org/D143446 Files: clang/include/clang/Frontend/CompilerInvocation.h clang/lib/Basic/LangOptions.cpp clang/lib/Frontend/CompilerInvocation.cpp clang/te

[PATCH] D143414: [clang] refactor FileManager::GetUniqueIDMapping

benlangmuir added a comment. +1 for determinism. Okay I spent some time trying to understand how this code is used and the non-virtual paths make some sense now. I am a bit skeptical about this on-disk-hash-table by filepath but that's separate from this patch. Comment at:

[PATCH] D143446: [clang][deps] Ensure module invocation can be serialized

benlangmuir updated this revision to Diff 495935. benlangmuir added a comment. - Improved doc comment for RoundTrip (mostly followed the suggested text; also converted from // to ///). - Renamed variables - Moved default value for RoundTripArgs CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D143446: [clang][deps] Ensure module invocation can be serialized

benlangmuir marked 3 inline comments as done. benlangmuir added inline comments. Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:658 +#endif + if (RoundTripArgs.getNumOccurrences() ? RoundTripArgs : DoRoundTripDefault) +if (FD.roundTripCommands(llvm::errs())) -

[PATCH] D143446: [clang][deps] Ensure module invocation can be serialized

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. benlangmuir marked an inline comment as done. Closed by commit rGcf73d3f07b5b: [clang][deps] Ensure module invocation can be serialized (authored by benlangmuir). Repo

[PATCH] D143613: [clang][deps] Migrate ModuleDepCollector to LexedFileChanged NFCI

benlangmuir created this revision. benlangmuir added reviewers: akyrtzi, jansvoboda11. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. LexedFileChanged has the semantics we want of ignoring #line/etc

[PATCH] D143615: [clang][deps] NFC: Refactor inferred modules test

benlangmuir accepted this revision. benlangmuir added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/ClangScanDeps/modules-inferred.m:54 +// CHECK-NEXT: { +// CHECK-NEXT: "context-hash": "{{.*}}", +// CHECK-NEXT:

[PATCH] D143613: [clang][deps] Migrate ModuleDepCollector to LexedFileChanged NFCI

benlangmuir updated this revision to Diff 496173. benlangmuir added a comment. Add a test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143613/new/ https://reviews.llvm.org/D143613 Files: clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h clang/lib/Tooling/Dependen

[PATCH] D143613: [clang][deps] Migrate ModuleDepCollector to LexedFileChanged NFCI

benlangmuir added a comment. In D143613#4115857 , @jansvoboda11 wrote: > LGTM, but would be nice to have a test for this. Oh, I didn't realize we didn't have one already. Added. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143613/new/ https

[PATCH] D143613: [clang][deps] Migrate ModuleDepCollector to LexedFileChanged NFCI

This revision was automatically updated to reflect the committed changes. Closed by commit rG8711120e8bcf: [clang][deps] Migrate ModuleDepCollector to LexedFileChanged NFCI (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1436

[PATCH] D144058: [clang][deps] Split lookupModuleOutput out of DependencyConsumer NFC

benlangmuir created this revision. benlangmuir added reviewers: jansvoboda11, akyrtzi, steven_wu. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The idea is to split the callbacks that are used to c

[PATCH] D144058: [clang][deps] Split lookupModuleOutput out of DependencyConsumer NFC

benlangmuir added inline comments. Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h:64-66 +/// Dependency scanner callbacks that are used during scanning to influence the +/// behaviour of the scan - for example, to customize the scanned invo

[PATCH] D144495: [NFC][clang] Refine tests by adding `:` to checks

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. LGTM. The vfsroot-include.c is a -cc1 invocation, so it could probably be switched to use `-verify` instead of FileCheck'ing the output, unless there's something I'm missing. But th

[PATCH] D149884: [clang][deps] Teach dep directive scanner about _Pragma

benlangmuir created this revision. benlangmuir added reviewers: akyrtzi, Bigcheese, jansvoboda11. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. While we cannot handle `_Pragma` used inside macros,

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

benlangmuir added a comment. In D103930#4310061 , @ivanmurashko wrote: > Friendly ping > > @arphaman, @jansvoboda11, I have made the patch buildable on all platforms > and have all tests passed. There was also a small fix (temp path for modules > artef

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

benlangmuir added a comment. We have several new build failures with this change that I'm looking through. So far, a common one is an error of the form /source/module.modulemap: error: redefinition of module /build/Foo.framework/Modules/module.modulemap: note: previously defined here ie. we

[PATCH] D150151: [clang] Prevent creation of new submodules in ASTWriter

benlangmuir created this revision. benlangmuir added a reviewer: jansvoboda11. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Avoid inferring new submodules for headers in ASTWriter's collection of

[PATCH] D150151: [clang] Prevent creation of new submodules in ASTWriter

benlangmuir added a comment. No test, because I haven't come up with a test case that wouldn't be invalidated by https://reviews.llvm.org/D103930. Let me know if you have any ideas. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150151/new/ https:

[PATCH] D149884: [clang][deps] Teach dep directive scanner about _Pragma

This revision was automatically updated to reflect the committed changes. Closed by commit rGee8ed0b3099e: [clang][deps] Teach dep directive scanner about _Pragma (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149884/new/ h

[PATCH] D150151: [clang] Prevent creation of new submodules in ASTWriter

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5984ea216d2a: [clang] Prevent creation of new submodules in ASTWriter (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D150318: [clang][deps] NFC: Pass around the whole scanning service

benlangmuir added inline comments. Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h:69 + /// The preprocessing mode used for scanning. + ScanningMode Mode; + /// The output format. Why drop `const`? Com

[PATCH] D150319: [clang][deps] Always use -fmodules-validate-once-per-build-session

benlangmuir added inline comments. Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp:26 + ? BuildSessionTimestamp + : std::chrono::system_clock::now().time_since_epoch().count()) { // Initialize targets for object file su

[PATCH] D150320: [clang][deps] Avoid relocatable modules checks

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. With my naming comment addressed, LGTM Comment at: clang/include/clang/Lex/PreprocessorOptions.h:73 + /// Perform extra checks for relocatable modules when loading

[PATCH] D150318: [clang][deps] NFC: Pass around the whole scanning service

benlangmuir added inline comments. Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h:69 + /// The preprocessing mode used for scanning. + ScanningMode Mode; + /// The output format. jansvoboda11 wrote: > benlangmuir wrote:

[PATCH] D150478: [clang][modules][deps] Parse "FW_Private" module map even after loading "FW" PCM

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. LGTM. I ran into a similar issue in a downstream branch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150478/new/ https://reviews.llvm

[PATCH] D150479: [clang][modules][deps] Allow skipping submodule definitions

benlangmuir added inline comments. Comment at: clang/test/ClangScanDeps/modules-private-framework-submodule.c:43 // CHECK-NEXT: "context-hash": "{{.*}}", -// CHECK-NEXT: "module-name": "FW2_Private" +// CHECK-NEXT: "module-name": "FW2"

[PATCH] D150479: [clang][modules][deps] Allow skipping submodule definitions

benlangmuir added inline comments. Comment at: clang/test/ClangScanDeps/modules-private-framework-submodule.c:43 // CHECK-NEXT: "context-hash": "{{.*}}", -// CHECK-NEXT: "module-name": "FW2_Private" +// CHECK-NEXT: "module-name": "FW2"

[PATCH] D148176: [clang][modules] Avoid re-exporting PCH imports on every later module import

benlangmuir added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148176/new/ https://reviews.llvm.org/D148176 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org

[PATCH] D148176: [clang][modules] Avoid re-exporting PCH imports on every later module import

benlangmuir added a comment. In D148176#4283942 , @jansvoboda11 wrote: > Can you explain why we need to keep separate `PendingImportedModulesSema` > now? In what situation will it end up aggregating more than one > `PendingImportedModules`? I don't kn

[PATCH] D148176: [clang][modules] Avoid re-exporting PCH imports on every later module import

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe06a91c5996b: [clang][modules] Avoid re-exporting PCH imports on every later module import (authored by benlangmuir). Repository: rG LLVM Github M

[PATCH] D149693: [clang][deps] Make clang-scan-deps write modules in raw format

benlangmuir created this revision. benlangmuir added reviewers: jansvoboda11, akyrtzi, Bigcheese. Herald added a subscriber: arphaman. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. We have no use f

[PATCH] D149693: [clang][deps] Make clang-scan-deps write modules in raw format

benlangmuir updated this revision to Diff 518891. Herald added a subscriber: kadircet. Herald added a project: clang-tools-extra. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149693/new/ https://reviews.llvm.org/D149693 Files: clang-tools-extra/clangd/Compiler.cpp clang/include/clan

[PATCH] D149693: [clang][deps] Make clang-scan-deps write modules in raw format

This revision was automatically updated to reflect the committed changes. Closed by commit rG8fe8d69ddf88: [clang][deps] Make clang-scan-deps write modules in raw format (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149693/

[PATCH] D149777: [clang][deps] Teach dep directive scanner about #pragma clang system_header

benlangmuir created this revision. benlangmuir added a reviewer: akyrtzi. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This ensures we get the correct FileCharacteristic during scanning. In a yet-

[PATCH] D149777: [clang][deps] Teach dep directive scanner about #pragma clang system_header

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7b492d1be0ea: [clang][deps] Teach dep directive scanner about #pragma clang system_header (authored by benlangmuir). Repository: rG LLVM Github Mo

[PATCH] D151855: [clang] Use `{File,Directory}EntryRef` in modular header search (part 2/2)

benlangmuir added a subscriber: JDevlieghere. benlangmuir added a comment. > I think it should be fine to allow dropping the > A.framework/Frameworks/B.framework directory from the reproducer VFS I think technically this is wrong, since if you're missing the symlink, then A might not build -- e

[PATCH] D151855: [clang] Use `{File,Directory}EntryRef` in modular header search (part 2/2)

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. > I don't think it's worth blocking this patch on it, though. What do you think? Agreed, I don't think this is making it worse. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D139052: [NFC][Profile] Access profile through VirtualFileSystem

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. All my comments are addressed, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139052/new/ https://reviews.llvm.org/D139052 ___

[PATCH] D139115: [clang][ExtractAPI] Add support for single symbol SGF

benlangmuir added subscribers: akyrtzi, benlangmuir. benlangmuir added inline comments. Comment at: clang/include/clang-c/Documentation.h:549 +typedef struct CXAPISetImpl *CXAPISet; + @dang please document what this type represents. @akyrtzi what's the preferr

[PATCH] D139115: [clang][ExtractAPI] Add support for single symbol SGF

benlangmuir added a comment. Thanks, the libclang change LGTM other than my comment about the function names. Comment at: clang/include/clang-c/Documentation.h:589 + */ +CINDEX_LINKAGE CXString clang_getSingleSymbolSymbolGraphForUSR(const char *usr, +

[PATCH] D139115: [clang][ExtractAPI] Add support for single symbol SGF

benlangmuir added a comment. libclang changes LGTM, thanks! I'll let the other reviewers who have domain expertise speak for the rest of the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139115/new/ https://reviews.llvm.org/D139115 __

[PATCH] D28299: Module: use PCMCache to manage memory buffers for pcm files.

benlangmuir added a comment. Can we test the already-validated diagnostics? Comment at: include/clang/Basic/FileManager.h:176 + /// Manage memory buffers associated with pcm files. + std::unique_ptr BufferMgr; + Why is this inside the FileManager? It isn't us

[PATCH] D28415: PCH: fix a regression that reports a module is defined in both pch and pcm

benlangmuir added a comment. LGTM https://reviews.llvm.org/D28415 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

<    3   4   5   6   7   8   9   >