[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] 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] 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] 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. > 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] 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] 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] 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 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] 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] 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] 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] 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] 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] 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] 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] 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] D158021: [clang][modules] Mark builtin header 'inttypes.h' for modules

benlangmuir created this revision. benlangmuir added reviewers: iana, jansvoboda11. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Like the other enumerated builtin headers, inttypes.h can be declar

[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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] D154130: [lit][clang] Avoid realpath on Windows due to MAX_PATH limitations

benlangmuir added inline comments. Comment at: clang/lib/Basic/FileManager.cpp:655 + SmallString<4096> AbsPathBuf = Name; + SmallString<4096> RealPathBuf; + if (!FS->makeAbsolute(AbsPathBuf)) { 8k is a lot of stack space. The only reason this was 4k in the fir

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

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

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

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

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

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

  1   2   3   4   >