[PATCH] D28286: [CodeCompletion] Code complete the missing C++11 keywords

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. LGTM Repository: rL LLVM https://reviews.llvm.org/D28286 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. Sorry for the delay, I though you were still iterating with @aprantl for some reason. LGTM https://reviews.llvm.org/D28299 ___

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

benlangmuir added inline comments. Comment at: include/clang/Basic/FileManager.h:176 + /// Manage memory buffers associated with pcm files. + std::unique_ptr BufferMgr; + manmanren wrote: > benlangmuir wrote: > > Why is this inside the FileManager? It isn't

[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

[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

[PATCH] D32081: Add support for editor placeholders to Clang's lexer

benlangmuir added inline comments. Comment at: include/clang/Basic/IdentifierTable.h:358 + /// Return true if this identifier is an editor placeholder. + bool isEditorPlaceholder() const { Nitpick: There should probably be an example in the doc comment. The

[PATCH] D32010: [indexer] Record class template specializations using a new 'SpecializationOf' relationship

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. LGTM Repository: rL LLVM https://reviews.llvm.org/D32010 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D32081: Add support for editor placeholders to Clang's lexer

benlangmuir added a comment. Rather than stick ranges into the diagnostic engine, I think it would be cleaner to have the identifier have a method like `isEditorPlaceholder()` that can be used to avoid situations like this in a principled way, or to customize behaviour for placeholders in the

[PATCH] D32081: Add support for editor placeholders to Clang's lexer

benlangmuir added a comment. > What do you think about a hybrid approach: Don't suppress diagnostics in the > placeholder range when we handle the placeholders in parser/sema, but do > suppress the range when the placeholder isn't explicitly handled? I'd really rather not plumb this into the

[PATCH] D32972: [index] Index simple dependent declaration references

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. A couple of minor comments, but otherwise LGTM. Comment at: include/clang/AST/DeclCXX.h:1569 + bool lookupInBases(BaseMatchesCallback BaseMatches, CXXBasePaths ,

[PATCH] D33324: [index] Avoid infinite recursion when looking up a dependent name

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. LGTM. One stylistic comment, but I'll leave that up to you. Comment at: lib/AST/CXXInheritance.cpp:286 +BaseRecord = nullptr; + else if

[PATCH] D34263: [preprocessor] When preprocessor option 'SingleFileParseMode' is enabled, parse all directive blocks if the condition uses undefined macros

benlangmuir added inline comments. Comment at: lib/Lex/PPDirectives.cpp:2774 +// the directive blocks. +CurPPLexer->pushConditionalLevel(CI.IfLoc, /*wasskip*/false, + /*foundnonskip*/true, /*foundelse*/true); akyrtzi

[PATCH] D34263: [preprocessor] When preprocessor option 'SingleFileParseMode' is enabled, parse all directive blocks if the condition uses undefined macros

benlangmuir added a comment. It would be nice if the doc comment for the single file parse mode flag was updated to include this new functionality. Comment at: lib/Lex/PPDirectives.cpp:2709 +CurPPLexer->pushConditionalLevel(IfToken.getLocation(), /*wasskip*/true, +

[PATCH] D34263: [preprocessor] When preprocessor option 'SingleFileParseMode' is enabled, parse all directive blocks if the condition uses undefined macros

benlangmuir added a comment. Thanks, this looks good to me. I'd appreciate if @klimek could take a quick look though. Comment at: include/clang/Lex/PreprocessorOptions.h:102 + /// in preprocessor directive conditions it causes all blocks to be parsed so + /// that the

[PATCH] D34256: [PR33394] Avoid lexing editor placeholders when running the preprocessor only

benlangmuir added a comment. I agree with not detecting these during PP-only, but there's nothing wrong with `<#>`. It's either not a placeholder, or it's part of a placeholder like `<#>#>`, which is a placeholder containing the text ">". Similarly, `<##` could be the start of an empty

[PATCH] D34256: [PR33394] Avoid lexing editor placeholders when running the preprocessor only

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rL LLVM https://reviews.llvm.org/D34256 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D34512: Add preliminary Cross Translation Unit support library

benlangmuir added a comment. In https://reviews.llvm.org/D34512#856301, @xazax.hun wrote: > In https://reviews.llvm.org/D34512#856184, @dcoughlin wrote: > > > In either case, the important scenario I think we should support is > > choosing at a call site to a C function the most likely

[PATCH] D47273: [bash-completion] Fix tab separation on macOS

benlangmuir added a comment. In https://reviews.llvm.org/D47273#1109985, @ruiu wrote: > sed -e "$(echo -e 's/\t.*//')" Yep, that works! Is there a reason you prefer that over the more succinct `$'s/\t.*//'`? Repository: rC Clang https://reviews.llvm.org/D47273

[PATCH] D47273: [bash-completion] Fix tab separation on macOS

benlangmuir updated this revision to Diff 148278. benlangmuir edited the summary of this revision. benlangmuir added a comment. Thanks for looking up the supported bash version! Updated diff. https://reviews.llvm.org/D47273 Files: utils/bash-autocomplete.sh Index:

[PATCH] D47273: [bash-completion] Fix tab separation on macOS

benlangmuir closed this revision. benlangmuir added a comment. r333202 https://reviews.llvm.org/D47273 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47273: [bash-completion] Fix tab separation on macOS

benlangmuir created this revision. benlangmuir added reviewers: yamaguchi, v.g.vassilev, ruiu, teemperor. Herald added a subscriber: cfe-commits. We have a regex that needs to match a tab character in the command output, but on macOS `sed` doesn't support '\t', causing it to split on the 't'

[PATCH] D47273: [bash-completion] Fix tab separation on macOS

benlangmuir added a comment. In https://reviews.llvm.org/D47273#1109934, @ruiu wrote: > I wonder if you could replace \t with \0x09. At least it works on my machine > which has GNU sed. Unfortunately that doesn't work either on mac :-\ Repository: rC Clang https://reviews.llvm.org/D47273

[PATCH] D41526: [libclang] Avoid builtin trap for __debug parser_crash pragma

benlangmuir added a comment. Did you consider just using a different pragma that triggers this behaviour instead of avoiding the crash? I don't really have a strong preference but I'd be interested to hear what you think the pros/cons are. Comment at:

[PATCH] D48685: [PCH+Modules] Load -fmodule-map-file content before including PCHs

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D48685 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

benlangmuir created this revision. benlangmuir added reviewers: bruno, vsapsai. Herald added a subscriber: cfe-commits. This make -ivfsoverlay behave more like other fatal errors (e.g. missing -include file) by skipping the missing file instead of bailing out of the whole compilation. This makes

[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

[PATCH] D54529: [clangd] Add USR to textDocument/definition response

benlangmuir added a comment. @sammccall RE using USRs: we want to integrate clangd into a larger source tooling system that is already using USRs extensively to identify symbols. The most obvious case is that we have an index outside of clangd that uses USRs from clang-the-compiler, so

[PATCH] D58924: Replace clang::FileData with llvm::vfs::Status

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. LGTM, although I made a small suggestion for clarity. FYI `InPCH` was used by PTH, which was removed a couple of months ago. Comment at:

[PATCH] D58924: Replace clang::FileData with llvm::vfs::Status

benlangmuir added inline comments. Comment at: clang/lib/Basic/FileManager.cpp:305 +UFE = [Status.getUniqueID()]; +Status = llvm::vfs::Status( + Status.getName(), Status.getUniqueID(), harlanhaskins wrote: > benlangmuir wrote: > > Why copy Status

[PATCH] D57965: Clean up ObjCPropertyDecl printing

benlangmuir added subscribers: akyrtzi, benlangmuir. benlangmuir added a comment. @arphaman @akyrtzi could you take a look at this? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57965/new/ https://reviews.llvm.org/D57965

[PATCH] D57965: Clean up ObjCPropertyDecl printing

benlangmuir added a comment. > @property(attr, attr2) instead of @property ( attr,attr2 ). The style I see most often is `@property (attr, attr2)`, which is in between those two. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57965/new/

[PATCH] D60735: [FileSystemStatCache] Return std::error_code from stat cache methods

benlangmuir added a comment. LGTM, but I'd appreciate someone who has worked on this more recently taking a look. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60735/new/ https://reviews.llvm.org/D60735

[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

[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

[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

[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

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

[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] D92480: [llvm] Unify interface of (ThreadSafe)?RefCountedBase

benlangmuir removed a reviewer: benlangmuir. benlangmuir added a comment. Removing myself as reviewer since I no longer work in this area. Someone who is more involved with llvm Support/ADT should review this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D104348: [index] Fix performance regression with indexing macros

benlangmuir created this revision. benlangmuir added a reviewer: arphaman. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When using FileIndexRecord with macros, symbol references can be seen out of source order, which was

[PATCH] D104348: [index] Fix performance regression with indexing macros

benlangmuir closed this revision. benlangmuir added a comment. Pushed rG773ad55a393f Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104348/new/ https://reviews.llvm.org/D104348

[PATCH] D101645: [clang] RecursiveASTVisitor visits ObjCPropertyRefExpr's class receiver

benlangmuir added a subscriber: akyrtzi. benlangmuir added inline comments. Comment at: clang/test/Index/Core/index-source.m:410 // CHECK-NEXT: RelCont | classReceivers | c:@F@classReceivers +// CHECK: [[@LINE-3]]:3 | class/ObjC | ClassReceivers | c:objc(cs)ClassReceivers |

[PATCH] D99758: [index] Improve macro indexing support

benlangmuir created this revision. benlangmuir added reviewers: akyrtzi, sammccall. Herald added a subscriber: arphaman. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The major change here is to index macro occurrences in

[PATCH] D99758: [index] Improve macro indexing support

benlangmuir updated this revision to Diff 334808. benlangmuir added a comment. Fixed clang-format issue. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99758/new/ https://reviews.llvm.org/D99758 Files: clang/include/clang/Index/DeclOccurrence.h

[PATCH] D99758: [index] Improve macro indexing support

benlangmuir updated this revision to Diff 334837. benlangmuir added a comment. Fix clang-tidy warnings CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99758/new/ https://reviews.llvm.org/D99758 Files: clang/include/clang/Index/DeclOccurrence.h

[PATCH] D99758: [index] Improve macro indexing support

benlangmuir updated this revision to Diff 334820. benlangmuir added a comment. Fix the incomplete type issue with PointerUnion in DeclOccurrence. h. I'm not actually sure how that code compiles without error with other versions of clang. CHANGES SINCE LAST ACTION

[PATCH] D99758: [index] Improve macro indexing support

This revision was automatically updated to reflect the committed changes. Closed by commit rG93c87fc06eca: [index] Improve macro indexing support (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99758/new/

[PATCH] D112229: Reapply [ORC-RT] Configure the ORC runtime for more architectures and platforms

benlangmuir created this revision. benlangmuir added a reviewer: lhames. Herald added subscribers: kristof.beyls, mgorny. benlangmuir requested review of this revision. Herald added projects: clang, Sanitizers. Herald added subscribers: Sanitizers, cfe-commits. Reapply 5692ed0cce8c95

[PATCH] D112229: Reapply [ORC-RT] Configure the ORC runtime for more architectures and platforms

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb8da59475076: Reapply [ORC-RT] Configure the ORC runtime for more architectures and platforms (authored by benlangmuir). Repository: rG LLVM

[PATCH] D112229: Reapply [ORC-RT] Configure the ORC runtime for more architectures and platforms

benlangmuir added inline comments. Comment at: clang/cmake/caches/CrossWinToARMLinux.cmake:104 set(COMPILER_RT_BUILD_CRT OFF CACHE BOOL "") +set(COMPILER_RT_BUILD_ORC OFF CACHE BOOL "") set(COMPILER_RT_DEFAULT_TARGET_ONLY ON CACHE

[PATCH] D116659: [llvm][clang][vfs] NFC: Simplify directory iteration

benlangmuir added a comment. > I don't know, I'm a bit skeptical we want to make it so easy to ignore errors > so easily. I'd rather require clients to explicitly ignore the error. That's my gut feeling too. I suspect a bunch of this code should be explicitly ignoring `ENOENT`, but not

[PATCH] D116174: [clang] support relative roots to vfs overlays

benlangmuir added a comment. Each VFS may have its own working directory, so it could be surprising if we use the OS working directory instead of that. This is complicated by the fact the VFS working directory may not be set yet during parsing the yaml (I haven't checked). I'm not really

[PATCH] D116174: [clang] support relative roots to vfs overlays

benlangmuir added a comment. Make sure the test failure gets fixed, but otherwise LGTM. > x64 debian > LLVM-Unit.Support/_/SupportTests::VFSFromYAMLTest.RelativePaths Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116174/new/

[PATCH] D118087: Fix running orc-rt tests with LLVM_BUILD_EXTERNAL_COMPILER_RT

benlangmuir created this revision. benlangmuir added reviewers: lhames, fhahn. Herald added a subscriber: mgorny. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Add missing dependency on llvm-jitlink when building compiler-rt

[PATCH] D118087: Fix running orc-rt tests with LLVM_BUILD_EXTERNAL_COMPILER_RT

benlangmuir added a comment. > Does check-orc-rt need to be added to COMPILER_RT_TEST_SUITES too? I guess > not, otherwise we wouldn't have seen this missing dependency in the first > place? It doesn't affect `check-compiler-rt`, because `check-compiler-rt` actually wraps the `check-all`

[PATCH] D118087: Fix running orc-rt tests with LLVM_BUILD_EXTERNAL_COMPILER_RT

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0e5ea403e8de: Fix running orc-rt tests with LLVM_BUILD_EXTERNAL_COMPILER_RT (authored by benlangmuir). Repository: rG LLVM Github Monorepo

[PATCH] D123104: [Modules] Use looked-up filename when looking for module maps

benlangmuir accepted this revision. benlangmuir added inline comments. Comment at: clang/include/clang/Lex/HeaderSearch.h:758 + bool IsSystemHeaderDir, + StringRef FileName = ""); This parameter

[PATCH] D123197: Remove a few effectively-unused FileEntry APIs. NFC

benlangmuir added inline comments. Comment at: clang/unittests/Basic/FileEntryTest.cpp:47-52 FileEntryRef addFile(StringRef Name) { -FEs.push_back(std::make_unique()); +const FileEntry *FE = +FM.getVirtualFile(std::to_string(UniqueNameCounter++), 0, 0);

[PATCH] D123197: Remove a few effectively-unused FileEntry APIs. NFC

benlangmuir added inline comments. Comment at: clang/unittests/Basic/FileEntryTest.cpp:47-52 FileEntryRef addFile(StringRef Name) { -FEs.push_back(std::make_unique()); +const FileEntry *FE = +FM.getVirtualFile(std::to_string(UniqueNameCounter++), 0, 0);

[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

[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=554765#toc Repository: rG LLVM

[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

[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

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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

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

[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] D124288: [Index] Remove reference to `UnresolvedUsingIfExists`

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. Minor suggestion for the test case, but otherwise LGTM. Comment at: clang/test/Index/using_if_exists.cpp:9 +// CHECK: [[@LINE-1]]:11 | using/C++ | foo | c:@UD@foo

[PATCH] D121733: Clean pathnames in FileManager.

benlangmuir added inline comments. Comment at: clang/lib/Basic/FileManager.cpp:218 + llvm::sys::path::remove_dots(CleanFilename, /*remove_dot_dot=*/false); + Filename = CleanFilename; + ppluzhnikov wrote: > kadircet wrote: > > this is actually breaking the

[PATCH] D129220: [clang] Cleanup ASTContext before output files in crash recovery for modules

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG67a84ec8105e: [clang] Cleanup ASTContext before output files in crash recovery for modules (authored by benlangmuir). Repository: rG LLVM Github

[PATCH] D128947: [Lex] Introduce `PPCallbacks::LexedFileChanged()` preprocessor callback

benlangmuir added inline comments. Comment at: clang/lib/Basic/SourceManager.cpp:1016 +return *Name; return StringRef(); } Just a suggestion: `value_or` can be a nice way to express simple cases like this: ```

[PATCH] D129389: [clang][deps] Override dependency and serialized diag files for modules

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 building modules, override secondary outputs (dependency

[PATCH] D129389: [clang][deps] Override dependency and serialized diag files for modules

benlangmuir updated this revision to Diff 443379. benlangmuir added a comment. Updates: - Made lookup of module outputs fallible. Not currently used by clang-scan-deps, but since the expectation is for a build system to provide these settings account for possibility of errors. - Attempt to fix

[PATCH] D129389: [clang][deps] Override dependency and serialized diag files for modules

benlangmuir updated this revision to Diff 443755. benlangmuir added a comment. Updates per review - Switched to a per-output callback - Removed preserved-args.c test - Removed error handling that I no longer have a real use for - Only request .d and .diag paths if they were enabled in the

[PATCH] D129389: [clang][deps] Override dependency and serialized diag files for modules

benlangmuir marked an inline comment as done. benlangmuir added inline comments. Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h:55 + getCommandLine(llvm::function_ref< + Expected(const ModuleID &)> +

[PATCH] D129504: [libclang][ObjC] Inherit availability attribute from containing decls or interface decls

benlangmuir added inline comments. Comment at: clang/tools/libclang/CIndex.cpp:8271 +else if (auto *IMD = dyn_cast(D)) + CD = IMD->getCategoryDecl(); +else if (auto *ID = dyn_cast(DC)) So this goes Impl -> Class, and CategoryImpl -> Category.

[PATCH] D129389: [clang][deps] Override dependency and serialized diag files for modules

This revision was automatically updated to reflect the committed changes. Closed by commit rG6626f6fec3d3: [clang][deps] Override dependency and serialized diag files for modules (authored by benlangmuir). Changed prior to commit: https://reviews.llvm.org/D129389?vs=443755=443955#toc

[PATCH] D129389: [clang][deps] Override dependency and serialized diag files for modules

benlangmuir added inline comments. Comment at: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp:341-342 + !MDC.OriginalInvocation.getDependencyOutputOpts().OutputFile.empty(); + // FIXME: HadSerializedDiagnostics and HadDependencyFile should be included in +

[PATCH] D128772: [Lex] Make sure to notify `MultipleIncludeOpt` for "read tokens" during fast dependency directive lexing

benlangmuir added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:4251 DepDirectives.front().Tokens[NextDepDirectiveTokenIndex++]; + if (NextDepDirectiveTokenIndex > 1 || DDTok.Kind != tok::hash) { +// Read something other than a preprocessor directive hash.

[PATCH] D129220: [clang] Cleanup ASTContext before output files in crash recovery for modules

benlangmuir created this revision. benlangmuir added reviewers: steven_wu, akyrtzi. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When we recover from a crash in a module compilation thread, we

[PATCH] D129220: [clang] Cleanup ASTContext before output files in crash recovery for modules

benlangmuir updated this revision to Diff 442706. benlangmuir added a comment. Updated per out-of-band feedback from @steven_wu : - Added an assert to `clearOutputFiles` that the `ASTConsumer` is removed. Normally this would be done in `FrontendAction::EndSourceFile`. This should help avoid

[PATCH] D127243: [clang][deps] Make order of module dependencies deterministic

benlangmuir updated this revision to Diff 435237. benlangmuir added a comment. Removed use of std::unique_ptr in DependencyScanningTool.cpp, per review feedback. @jansvoboda11 Note: there is another map containing `std::unique_ptr` in `ModuleDepCollector`, but that one is required for

[PATCH] D127243: [clang][deps] Make order of module dependencies deterministic

benlangmuir added inline comments. Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp:181 std::vector PrebuiltModuleDeps; -std::map ClangModuleDeps; +llvm::MapVector, +llvm::StringMap> jansvoboda11 wrote:

[PATCH] D127229: [clang][deps] Set -disable-free for module compilations

benlangmuir added inline comments. Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp:338 +// always true for a driver invocation. +bool DisableFree = true;

[PATCH] D127229: [clang][deps] Set -disable-free for module compilations

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7a72dca74a27: [clang][deps] Set -disable-free for module compilations (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D127243: [clang][deps] Make order of module dependencies deterministic

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG835fcf2aa512: [clang][deps] Make order of module dependencies deterministic (authored by benlangmuir). Repository: rG LLVM Github Monorepo

[PATCH] D127229: [clang][deps] Set -disable-free for module compilations

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. The command-line arguments for module builds are cc1

[PATCH] D127379: [Lex] Keep track of skipped preprocessor blocks and advance the lexer directly if they are revisited

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. Couple more minor things, but basically LGTM. Comment at: clang/include/clang/Lex/Preprocessor.h:2601 + /// This is used to guard against calling this function

[PATCH] D127883: [clang][deps] Further canonicalize implicit modules options in dep scan

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. Disable or canonicalize compiler options that are not

  1   2   3   4   >