[PATCH] D158021: [clang][modules] Mark builtin header 'inttypes.h' for modules

benlangmuir updated this revision to Diff 550474. benlangmuir added a reviewer: vsapsai. benlangmuir added a comment. Add missing test updates: tests using the `Inputs/System/usr/include` should be using `-internal-isystem` to get the correct search path order with respect to the resource dir. T

[PATCH] D158136: [clang][modules] Avoid storing command-line macro definitions into implicitly built PCM files

benlangmuir accepted this revision. benlangmuir added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/Modules/module_file_info.m:44 // CHECK: Uses compiler/target-specific predefines [-undef]: Yes // CHECK: Uses detailed preprocessin

[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; + Doesn't `InputFilesCursor` already know where the input files block

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

benlangmuir added inline comments. Comment at: clang/lib/Serialization/ASTReader.cpp:2685 + for (unsigned I = 0; I < ASTFileSignature::size; ++I) +Sig[I] = endian::readNext(Blob); + return Sig; uint8_t has no endianness and has alignment 1 anyway; you can j

[PATCH] D154130: [lit][clang] Avoid realpath on Windows due to MAX_PATH limitations

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. LGTM other than my suggestion to add a comment. @tahonermann are all your comments addressed at this point? Comment at: clang/lib/Basic/FileManager.cpp:663 +}

[PATCH] D156492: [clang][deps] Make the C++ API more type-safe

benlangmuir accepted this revision. benlangmuir added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h:175 std::vector PrebuiltModuleDeps; - llvm::MapVector> + llvm::MapVect

[PATCH] D156563: [clang][deps] Remove `ModuleDeps::ImportedByMainFile`

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. This flag was always weird to me, thanks for cleaning it up. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156563/new/ https://reviews

[PATCH] D154130: [lit][clang] Avoid realpath on Windows due to MAX_PATH limitations

benlangmuir added inline comments. Comment at: clang/lib/Basic/FileManager.cpp:663 +} else { + llvm::sys::path::remove_dots(AbsPathBuf, /*remove_dot_dot=*/true); + CanonicalName = AbsPathBuf.str().copy(CanonicalNameStorage); MrTrillian wrote: > benl

[PATCH] D156948: [clang][modules] Add -Wsystem-headers-in-module=

benlangmuir created this revision. benlangmuir added reviewers: jansvoboda11, iana. Herald added a project: All. benlangmuir requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. Add a way to enable -Wsystem-headers only for a specific mo

[PATCH] D157035: [clang][cli] Accept option spelling as `Twine`

benlangmuir added a comment. This code asserts that the Twine is a single stringref, but it's also relying on the fact it's null-terminated; can we check for that as well, or at least document it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1570

[PATCH] D157050: [clang] NFC: Avoid double allocation when generating command line

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

[PATCH] D157046: [clang] Abstract away string allocation in command line generation

benlangmuir accepted this revision. benlangmuir added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4323 +GenerateArg(Consumer, OPT_darwin_target_variant_sdk_version_EQ, +Opts.DarwinTarg

[PATCH] D156948: [clang][modules] Add -Wsystem-headers-in-module=

benlangmuir updated this revision to Diff 547266. benlangmuir added a comment. Use `llvm::is_contained` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156948/new/ https://reviews.llvm.org/D156948 Files: clang/include/clang/Basic/DiagnosticOptions.h clang/include/clang/Driver/Options

[PATCH] D156948: [clang][modules] Add -Wsystem-headers-in-module=

benlangmuir marked an inline comment as done. benlangmuir added inline comments. Comment at: clang/include/clang/Basic/DiagnosticOptions.h:128 + /// whether -Wsystem-headers is enabled on a per-module basis. + std::vector SystemHeaderWarningsModules; + jansvobo

[PATCH] D157029: [llvm] Construct option spelling at compile-time

benlangmuir added inline comments. Comment at: llvm/include/llvm/Option/Option.h:103 + StringLiteral getSpelling() const { +assert(Info && "Must have a valid info!"); This could use a doc comment to differentiate it from other string representations. How

[PATCH] D157066: [clang][modules][deps] Create more efficient API for visitation of `ModuleFile` inputs

benlangmuir added a subscriber: vsapsai. benlangmuir added a comment. CC @vsapsai since he was also making name vs. name-as-requested change in modules Comment at: clang/include/clang/Serialization/ModuleFile.h:72 + bool TopLevel; + bool ModuleMap; }; Is th

[PATCH] D157066: [clang][modules][deps] Create more efficient API for visitation of `ModuleFile` inputs

benlangmuir accepted this revision. benlangmuir added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Serialization/ModuleFile.h:72 + bool TopLevel; + bool ModuleMap; }; jansvoboda11 wrote: > benlangmuir wrote:

[PATCH] D156948: [clang][modules] Add -Wsystem-headers-in-module=

benlangmuir marked an inline comment as done. benlangmuir added inline comments. Comment at: clang/include/clang/Basic/DiagnosticOptions.h:128 + /// whether -Wsystem-headers is enabled on a per-module basis. + std::vector SystemHeaderWarningsModules; + iana wro

[PATCH] D156948: [clang][modules] Add -Wsystem-headers-in-module=

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGdc5cbba3196d: [clang][modules] Add -Wsystem-headers-in-module= (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

benlangmuir added a comment. > but need to be repeatedly included when they're used in system modules How does this work today? Wouldn't the include guard prevent this? Comment at: clang/include/clang/Basic/Features.def:233 FEATURE(modules, LangOpts.Modules) +FEATURE(builtin_

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

benlangmuir added a comment. >> How does this work today? Wouldn't the include guard prevent this? > > Today they don't define their include guard when building with modules. Thanks - I can see some headers behave that way, or in other cases there are other sorts of has_feature(modules) checks t

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

benlangmuir added a comment. I suggest we add a comment explaining the weird has_feature checks being added here (even if they're less weird than what they're replacing). Maybe just a comment in stdarg.h and/or stddef.h and then the __std(arg|def) headers could just add a one-liner reference

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

benlangmuir added inline comments. Comment at: clang/test/Modules/Inputs/System/usr/include/stdint.h:2 typedef int my_awesome_nonstandard_integer_type; + +/* C99 7.18.1.1 Exact-width integer types. Why do we need all this code now (I assume this is copied from t

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

benlangmuir added a comment. Other than the giant header in the test we're still discussing, this basically LGTM. Comment at: clang/test/Modules/Inputs/System/usr/include/stdint.h:2 typedef int my_awesome_nonstandard_integer_type; + +/* C99 7.18.1.1 Exact-width integer types.

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

benlangmuir added a comment. > You mean diagnosing whenever the spelling in the VFS definition differs from > its realpath? Right, this would be ideal, but may be too expensive in practice. > How could we make that work with symlinks in place? Ah right, we are intentionally allowing symlinks i

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

benlangmuir added a comment. In D135849#4469347 , @jansvoboda11 wrote: > It'd be really nice to have DirectoryEntry::getDir() API, so that we can walk > up the directory hierarchy while preserving the virtual/real distinction > between directories in t

[PATCH] D154905: [clang] Implement `PointerLikeTraits` for `{File,Directory}EntryRef`

benlangmuir added inline comments. Comment at: clang/include/clang/Basic/DirectoryEntry.h:211 + + static constexpr int NumLowBitsAvailable = 3; +}; I suggest not hard-coding it if you can get away with it; maybe `PointerLikeTypeTraits::NumLowBitsAvailable`? I t

[PATCH] D154905: [clang] Implement `PointerLikeTraits` for `{File,Directory}EntryRef`

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154905/new/ https://reviews.llvm.org/D154905 __

[PATCH] D155131: [clang][modules] Deserialize included files lazily

benlangmuir added a comment. Now that it's not eagerly deserialized, should `Preprocessor::alreadyIncluded` call `HeaderInfo.getFileInfo(File)` to ensure the information is up to date? Similarly, we expose the list of files in `Preprocessor::getIncludedFiles` -- is it okay if this list is inco

[PATCH] D154130: [lit] Avoid os.path.realpath on Windows due to MAX_PATH limitations

benlangmuir added a comment. > I took a look at the code and it looks to me like it would be safe to change > ModuleMap::canonicalizeModuleMapPath() to use a drive preserving > canonicalization This sounds reasonable to me (the person who added canonicalizeModuleMapPath), though I am not at al

[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

<    1   2   3   4   >