[PATCH] D159435: [NFC] remove unneded header includes

JDevlieghere added a comment. This breaks the LLDB (modules) build: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/59738/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159435/new/ https://reviews.llvm.org/D159435 __

[PATCH] D158071: [clang] Remove rdar links; NFC

JDevlieghere added a comment. I had a cursory look and the handful of radar links I opened and read through didn't have any context that wasn't already in the tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158071/new/ https://reviews.llvm.or

[PATCH] D154329: [lldb] Replace llvm::writeFileAtomically with llvm::writeToOutput API.

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154329/new/ https://reviews.llvm.org/D154329 ___

[PATCH] D118754: [DebugInfo] Always emit `.debug_names` with dwarf 5 for Apple platforms

This revision was automatically updated to reflect the committed changes. Closed by commit rGe0d57295bf6a: [DebugInfo] Always emit `.debug_names` with DWARF 5 for Apple platforms (authored by JDevlieghere). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LL

[PATCH] D136290: [clang] Disable assertion that can "easily happen"

This revision was automatically updated to reflect the committed changes. Closed by commit rG97b91307b00e: [clang] Disable assertion that can "easily happen" (authored by JDevlieghere). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo C

[PATCH] D131422: [lldb] Remove include/lldb/lldb-private.h

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131422/new/ https://reviews.llvm.org/D131422 ___

[PATCH] D131020: Reland "[lldb/Fuzzer] Add fuzzer for expression evaluator"

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131020/new/ https://reviews.llvm.org/D131020 ___ cfe-commits mailing list c

[PATCH] D131020: Reland "[lldb/Fuzzer] Add fuzzer for expression evaluator"

JDevlieghere added inline comments. Comment at: lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/CMakeLists.txt:51-56 +add_custom_target(fuzz-lldb-expression + COMMENT "Running the LLDB expression evaluator fuzzer..." + WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/fuzzer-art

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

JDevlieghere added a comment. In D112374#3665989 , @mizvekov wrote: > If anyone wants to take a look at the new changes to lldb tests, be my guest. > Otherwise I will try to land this again soon. It might well be that we figure > out some other in-tree

[PATCH] D129377: [lldb/Fuzzer] Add fuzzer for expression evaluator

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. A few nits about naming, but otherwise this LGTM. Comment at: lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/lldb-expression-fuzzer.cpp:47 +DEFINE_BINARY_PROTO_FU

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

JDevlieghere added a subscriber: teemperor. JDevlieghere added a comment. In D112374#3653702 , @mizvekov wrote: > @JDevlieghere I spent a lot of time trying to get this test running on my > machine to no avail. I think lldb build and test setup is quite

[PATCH] D129377: [lldb/Fuzzer] Add fuzzer for expression evaluator

JDevlieghere added inline comments. Comment at: lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/cxx_proto.proto:1 +//===-- cxx_proto.proto - Protobuf description of C++ -===// +// Do we still need a copy of this for LLDB? CHANGES SINCE LAST A

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

JDevlieghere added a comment. This breaks all the LLDB tests that import the std module: import-std-module/array.TestArrayFromStdModule.py import-std-module/deque-basic.TestDequeFromStdModule.py import-std-module/deque-dbg-info-content.TestDbgInfoContentDequeFromStdModule.py import-std-m

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

JDevlieghere added a comment. In D128059#3646953 , @cor3ntin wrote: > In D128059#3646695 , @JDevlieghere > wrote: > >> I had to revert this because it breaks a bunch of LLDB tests: >> https://green.lab.llvm.org/

[PATCH] D129377: [lldb/Fuzzer] Add fuzzer for expression evaluator

JDevlieghere added inline comments. Comment at: clang/cmake/modules/ProtobufMutator.cmake:4-5 + set (PBM_PREFIX clang_protobuf_mutator) +elseif(${CMAKE_CURRENT_SOURCE_DIR} MATCHES "lldb") + set (PBM_PREFIX lldb_protobuf_mutator) +endif() cassanova wrote: > mib

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

JDevlieghere added a comment. I had to revert this because it breaks a bunch of LLDB tests: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/45288/#showFailuresLink. It looks like this causes an error when building some SDK modules. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D129456: [lldb] Add image dump pcm-info command

JDevlieghere added a comment. [bikeshedding] `pcm-info` seems a little odd. Do we have other command that end with `-info`? `target modules dump` already sounds like you're about to dump some kind of "info". What about just `pcm`? [/bikeshedding] Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

JDevlieghere added a comment. In D126189#3643045 , @iains wrote: > In D126189#3643021 , @JDevlieghere > wrote: > >> In D126189#3643001 , @iains wrote: >> >>> In D126189#3

[PATCH] D129377: [lldb/Fuzzer] Add fuzzer for expression evaluator

JDevlieghere added a comment. The review shows the diff with the previous iteration of the patch. Can you generate a diff with the current top-of-tree? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129377/new/ https://reviews.llvm.org/D129377 ___

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

JDevlieghere added a comment. In D126189#3643001 , @iains wrote: > In D126189#3642992 , @JDevlieghere > wrote: > >> In D126189#3642820 , @iains wrote: >> >>> JFTR, I did

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

JDevlieghere added a comment. In D126189#3642820 , @iains wrote: > JFTR, I did not get any notification from green dragon (which is odd, AFAIR > it's sent email before) or I would have looked right away - kicked off a > build will take a look as soon a

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

JDevlieghere added a comment. In D126189#3642777 , @iains wrote: > In D126189#3642762 , @JDevlieghere > wrote: > >> This breaks TestDataFormatterLibcxxSpan.py on GreenDragon: >> >> Undefined symbols for archite

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

JDevlieghere added a comment. This breaks TestDataFormatterLibcxxSpan.py on GreenDragon: Undefined symbols for architecture x86_64: "__ZGIW10std_config", referenced from: __GLOBAL__sub_I_main.cpp in main.o "__ZGIW4span", referenced from: __GLOBAL__sub_I_main.cpp in main.

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

JDevlieghere added a comment. This change triggers an assertion when building an LLDB test case: UNREACHABLE executed at /Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/clang/lib/Sema/SemaTemplate.cpp:1726! Assertion failed: (InstantiatingSpecializations.empty() && "failed to cle

[PATCH] D127684: [NFC] Use `https` instead of `http` in the `LLVM.org` URLs

JDevlieghere requested changes to this revision. JDevlieghere added a comment. This revision now requires changes to proceed. +1 on breaking this down per sub-project and creating separate patches for review. I sampled the patch and about a quarter or so of the links I tried didn't work, even b

[PATCH] D115438: [lldb] Remove unused lldb.cpp

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

[PATCH] D113743: [RFC][clang][DebugInfo] Allow function-local statics and types to be scoped within a lexical block

JDevlieghere added a comment. Hey Kristina, this broke TestSetData.py on GreenDragon: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/39089/ Since the bot has been red for several hours I went ahead and reverted your change in 4cb79294e8df8c91ae15264d1014361815d34a53

[PATCH] D112767: [clang][driver] Fix multiarch output name with -Wl arg

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112767/new/ https://reviews.llvm.org/D112767 ___

[PATCH] D109128: [VFS] Use original path when falling back to external FS

JDevlieghere added inline comments. Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1179-1180 + if (ExternalFS) +ExternalFS->setCurrentWorkingDirectory(Path); + I'm pretty sure there was a reason we stopped doing this. There should be some discussion a

[PATCH] D111269: [clang][Driver] Make multiarch output file basenames reproducible

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111269/new/ https://reviews.llvm.org/D111269 ___

[PATCH] D110041: [clang] Use portable "#!/usr/bin/env bash" shebang for tools and utils.

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110041/new/ https://reviews.llvm.org/D110041 ___

[PATCH] D109128: [VFS] Use original path when falling back to external FS

JDevlieghere added a comment. Yes, Keith and I came to the same conclusion yesterday. I was worried about tracking both paths at all times, but I like your suggestion of only changing the path when requested. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D109128: [VFS] Use original path when falling back to external FS

JDevlieghere added a comment. Keith and I discussed this offline. My suggestion was to do the following: 1. Check the overlay for the canonicalized path 2. Check the fall-through for the canonicalized path 3. Check the fall-through for the original path If I understand correctly, this patch does

[PATCH] D109128: [VFS] Use original path when falling back to external FS

JDevlieghere added a comment. I'm sure I'm missing something, but after rereading the patch several times I still don't see the functional change. It just looks like it's renaming every instance of `Path` to `CanonicalPath` and `Path_` to `Path`? Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D107878: [SampleFDO] Flow Sensitive Sample FDO (FSAFDO) profile loader

JDevlieghere added inline comments. Comment at: llvm/include/llvm/CodeGen/MIRSampleProfile.h:59 +MIRSampleLoader( +std::make_unique(FileName, RemappingFileName)) { +LowBit = getFSPassBitBegin(P); You're instantiating a forward-declared typ

[PATCH] D106084: [DebugInfo] Switch to using constructor homing (-debug-info-kind=constructor) by default when debug info is enabled

JDevlieghere accepted this revision. JDevlieghere added a comment. Adrian said "Let's do it!" on the mailing list so LGTM for Darin. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106084/new/ https://reviews.llvm.org/D106084 ___

[PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

JDevlieghere added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:402-403 case DW_AT_type: - type = form_value; + if (!type.IsValid()) +type = form_value; break; What's the purpose of this?

[PATCH] D100901: [CMake][llvm] avoid conflict w/ (and use when available) new builtin check_linker_flag

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

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

JDevlieghere added a comment. In D45639#2706605 , @ldionne wrote: > In D45639#2706589 , @dexonsmith > wrote: > >> I'm not sure I'm totally following, but just want to double check that the >> tests won't somehow u

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

JDevlieghere added a comment. In D45639#2706364 , @JDevlieghere wrote: > Given that these tests are macOS specific and already require a specific SDK, > I'll just update them to use the compiler from the SDK instead of the > just-built one. Done in 5d1

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

JDevlieghere added a comment. Given that these tests are macOS specific and already require a specific SDK, I'll just update them to use the compiler from the SDK instead of the just-built one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D45639/n

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

JDevlieghere added a comment. In D45639#2705919 , @phosek wrote: > In D45639#2705702 , @ldionne wrote: > >> In D45639#2703913 , @JDevlieghere >> wrote: >> >>> This breaks `T

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

JDevlieghere added a comment. @phosek I've reverted this in 05eeed9691aeb3e0316712195b998e9078cdceb0 to turn the bot green again. Happy to help you look into this tomorrow. Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

JDevlieghere added a comment. This breaks `TestAppleSimulatorOSType.py ` on GreenDragon. First failed build: http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/31346/ /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/bin/clang main.o -g -O0 -fno-builtin -isysroot "/Application

[PATCH] D100086: Include `llvm-config` and `not` in AppleClang toolchains.

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

[PATCH] D95119: Prefer /usr/bin/env xxx over /usr/bin/xxx where xxx = perl, python, awk

JDevlieghere added a comment. In D95119#2584709 , @haampie wrote: > Should this be merged? Do you have commit access? If not I can land this for you. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95119/new/ https://reviews.llvm.org/D95119

[PATCH] D96363: Mark output as text if it is really text

JDevlieghere accepted this revision. JDevlieghere added a comment. Thanks. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96363/new/ https://reviews.llvm.org/D96363 ___ cfe-commits mailing list cfe-

[PATCH] D96363: Mark output as text if it is really text

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM with two inline nits. Comment at: clang/lib/Frontend/Rewrite/FrontendActions.cpp:188 std::unique_ptr OS = - CI.createDefaultOutputFile(true, getCurre

[PATCH] D96427: Support multi-configuration generators correctly in several config files

JDevlieghere accepted this revision. JDevlieghere added a comment. LGTM. I'm surprised the `dsymutil` one slipped through the cracks, we have a bot that should (?) be testing this configuration: http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake-standalone/ Repository: rG LLVM Github M

[PATCH] D96049: [Timer] On macOS count number of executed instructions

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

[PATCH] D95635: [CMake] Actually require python 3.6 or greater

JDevlieghere added a comment. Thanks for brining this up on the mailing list, I think that's a good place to discuss! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95635/new/ https://reviews.llvm.org/D95635 ___

[PATCH] D95635: [CMake] Actually require python 3.6 or greater

JDevlieghere reopened this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. In D95635#2529027 , @ctetreau wrote: > In D95635#2528851 , @JDevlieghere > wrote: > >>> However, t

[PATCH] D95119: Prefer /usr/bin/env xxx over /usr/bin/xxx where xxx = perl, python, awk

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM, unless @MaskRay has any other concerns. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95119/new/ https://reviews.llvm.org/D95119 ___

[PATCH] D95635: [CMake] Actually require python 3.6 or greater

JDevlieghere added a comment. > However, the project claims to require 3.6 or greater, and 3.6 features are > being used. Can you link to where it says that? I'm not opposed to making 3.6 minimally required version, but at least for LLDB we don't have that requirement and we have documentation

[PATCH] D95119: Prefer /usr/bin/env xxx over /usr/bin/xxx where xxx = perl, python, awk

JDevlieghere requested changes to this revision. JDevlieghere added a comment. This revision now requires changes to proceed. Oops, didn't mean to accept. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95119/new/ https://reviews.llvm.org/D95119 ___

[PATCH] D95119: Prefer /usr/bin/env xxx over /usr/bin/xxx where xxx = perl, python, awk

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. In D95119#2525925 , @MaskRay wrote: > `#!/usr/bin/env perl -w` (2 arguments) unfortunately does not work on Linux > and many other systems.

[PATCH] D94844: [VFS] Add support to RedirectingFileSystem for mapping a virtual directory to one in the external FS.

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. In D94844#2524854 , @nathawes wrote: > @JDevlieghere and @dexonsmith would you mind taking another look? > > I ended up changing the 'lookup

[PATCH] D95279: Support: Remove duplicated code in {File,clang::ModulesDependency}Collector, NFC

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM, thanks for cleaning this up! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95279/new/ https://reviews.llvm.org/D95279 ___

[PATCH] D94844: [VFS] Add support to RedirectingFileSystem for mapping a virtual directory to one in the external FS.

JDevlieghere added a comment. Overall this looks good. I wonder if abstracting the ExternalRedirect as a small wrapper class around a SmallString would help. There's a few operations that are repeated, like the example below, and it could also house the logic that's currently in the lambda.

[PATCH] D94374: [CMake] Remove dead code setting policies to NEW

JDevlieghere added a comment. LGTM for LLDB Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94374/new/ https://reviews.llvm.org/D94374 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.

[PATCH] D91317: Support: Add RedirectingFileSystem::create from simple list of redirections

JDevlieghere accepted this revision. JDevlieghere added a comment. LGTM with the other patches. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91317/new/ https://reviews.llvm.org/D91317 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D92888: ADT: Allow IntrusiveRefCntPtr construction from std::unique_ptr, NFC

JDevlieghere accepted this revision. JDevlieghere added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92888/new/ https://reviews.llvm.org/D92888 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D91317: Support: Add RedirectingFileSystem::create from simple list of redirections

JDevlieghere added inline comments. Comment at: llvm/include/llvm/Support/VirtualFileSystem.h:735 + /// Redirect each of the remapped files from first to second. + static std::unique_ptr + create(ArrayRef> RemappedFiles, - Do you need to know that this is a `

[PATCH] D92597: ARCMigrate: Initialize fields in EditEntry inline, NFC

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

[PATCH] D90890: Frontend: Change ComputePreambleBounds to take MemoryBufferRef, NFC

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90890/new/ https://reviews.llvm.org/D90890 ___ cfe-commits mailing list cfe

[PATCH] D89836: Change Module::ASTFile and ModuleFile::File => Optional, NFC

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89836/new/ https://reviews.llvm.org/D89836 ___ cfe-commits mailing list cfe

[PATCH] D89835: ModuleManager: Simplify lookupModuleFile by only setting the out parameter once, NFC

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89835/new/ https://reviews.llvm.org/D89835 ___ cfe-commits mailing list cfe

[PATCH] D89913: SourceManager: Encapsulate line number mapping into SrcMgr::LineOffsetMapping

JDevlieghere accepted this revision. JDevlieghere added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Basic/SourceManager.h:118 + private: +unsigned *Storage = nullptr; + }; I guess it's implicit in the im

[PATCH] D89580: SourceManager: Fix an SLocEntry memory regression introduced with FileEntryRef

JDevlieghere added inline comments. Comment at: clang/include/clang/Basic/SourceManager.h:293 X.ContentAndKind.setInt(FileCharacter); - X.Filename = Filename; + const_cast(Con).Filename = Filename; return X; Would it possibly make more sen

[PATCH] D89554: SourceManager: Clarify that FileInfo always has a ContentCache, NFC

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM if Shafik is happy CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89554/new/ https://reviews.llvm.org/D89554 ___ cfe-commi

[PATCH] D89763: [Apple-stage2] Install FileCheck and yaml2obj in the toolchain

This revision was automatically updated to reflect the committed changes. Closed by commit rG27a909a24f99: [Apple-stage2] Install FileCheck and yaml2obj in the toolchain (authored by JDevlieghere). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: h

[PATCH] D89429: clang/Basic: Replace SourceManager::getMemoryBufferForFile, NFC

JDevlieghere accepted this revision. JDevlieghere added a comment. Thanks for the explanation! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89429/new/ https://reviews.llvm.org/D89429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D89761: Split out llvm/Support/FileSystem/UniqueID.h and clang/Basic/FileEntry.h, NFC

JDevlieghere added inline comments. Comment at: clang/include/clang/Basic/FileEntry.h:33 + +using llvm::Optional; +using llvm::StringRef; Won't this now make `llvm::Optional` visible as `clang::Optional` everywhere this header is included? Isn't this considered

[PATCH] D89398: Lexer: Update the Lexer to use MemoryBufferRef, NFC

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/include/clang/Lex/Lexer.h:145 /// outlive it, so it doesn't take ownership of either of them. - Lexer(FileID FID, const llvm::MemoryBuf

[PATCH] D89430: clang/Basic: Remove SourceManager::getBufferPointer, NFC

JDevlieghere added a comment. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89430/new/ https://reviews.llvm.org/D89430 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89398: Lexer: Update the Lexer to use MemoryBufferRef, NFC

JDevlieghere added inline comments. Comment at: clang/include/clang/Lex/Lexer.h:145 /// outlive it, so it doesn't take ownership of either of them. - Lexer(FileID FID, const llvm::MemoryBuffer *InputFile, Preprocessor &PP); + Lexer(FileID FID, const llvm::MemoryBufferRef &In

[PATCH] D89430: clang/Basic: Remove SourceManager::getBufferPointer, NFC

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/lib/Basic/SourceManager.cpp:167 Buffer.setInt(Buffer.getInt() | InvalidFlag); +return None; Orthogonal to this

[PATCH] D89429: clang/Basic: Replace SourceManager::getMemoryBufferForFile, NFC

JDevlieghere added inline comments. Comment at: clang/lib/Basic/SourceManager.cpp:705 +llvm::Optional +SourceManager::getMemoryBufferForFileOrNone(const FileEntry *File) { const SrcMgr::ContentCache *IR = getOrCreateContentCache(File); Would it make sense to r

[PATCH] D88497: [objc] Fix memory leak in CGObjCMac.cpp

JDevlieghere added a comment. Thanks for updating the comment in dsymutil! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88497/new/ https://reviews.llvm.org/D88497 ___ cfe-commits mailing list cfe-commit

[PATCH] D87243: [cmake] Centralize LLVM_ENABLE_WARNINGS option

JDevlieghere accepted this revision. JDevlieghere added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87243/new/ https://reviews.llvm.org/D87243 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D87243: [cmake] Centralize LLVM_ENABLE_WARNINGS option

JDevlieghere added a comment. In D87243#2261687 , @kastiglione wrote: > If an LLVM install disabled `LLVM_ENABLE_WARNINGS`, should other builds > inherit that? I would think no, but is there a precedent for that that to be > the case? Yes, most of the `

[PATCH] D87243: [cmake] Centralize LLVM_ENABLE_WARNINGS option

JDevlieghere added a comment. You need to add `LLVM_ENABLE_WARNINGS` to `LLVMConfig.cmake.in` so that the standalone builds know what value was set in the LLVM build. I think with the current patch the other projects won't inherit the value and just default to `ON`? Repository: rG LLVM Gith

[PATCH] D85231: Protect against filenames with no extension at all.

JDevlieghere added a comment. Can you add a test that exercises this code path? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85231/new/ https://reviews.llvm.org/D85231 ___ cfe-commits mailing list cfe-c

[PATCH] D84572: Allow .dSYM's to be directly placed in an alternate directory

JDevlieghere accepted this revision. JDevlieghere 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/D84572/new/ https://reviews.llvm.org/D84572 __

[PATCH] D84572: Allow .dSYM's to be directly placed in an alternate directory

JDevlieghere added inline comments. Comment at: clang/include/clang/Driver/Options.td:693 def e : JoinedOrSeparate<["-"], "e">, Group; +def external_dsym_dir : JoinedOrSeparate<["-"], "external-dsym-dir">, + Flags<[DriverOption, RenderAsInput]>, Could we drop t

[PATCH] D84565: [Darwin] [Driver] Clang should invoke dsymutil for lto builds -g*

JDevlieghere requested changes to this revision. JDevlieghere added a comment. This revision now requires changes to proceed. When you don’t pass any specific options to the linker, it’s going to generate a temporary `lto.o` file in `/tmp` and delete it after the link. When `dsymutil` will try t

[PATCH] D83454: [CMake] Make `intrinsics_gen` dependency unconditional.

JDevlieghere added a comment. In D83454#2142719 , @michele.scandale wrote: > I tested locally the standalone build of Clang with D83426 > . > I just want to make sure that we all agree this is right way moving forward.

[PATCH] D83454: [CMake] Make `intrinsics_gen` dependency unconditional.

JDevlieghere added a comment. LGTM. I verified this works for the build-tree standalone build of LLDB, but please still keep an eye on http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake-standalone/ after you land this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

JDevlieghere accepted this revision. JDevlieghere added a comment. Thanks Petr, LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79219/new/ https://reviews.llvm.org/D79219 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://li

[PATCH] D82733: [clang][docs] Add note about using `-flto` with `-g` on macOS

This revision was automatically updated to reflect the committed changes. Closed by commit rGa1f4e48c4aca: [clang][docs] Add note about using `-flto` with `-g` on macOS (authored by phil-blain, committed by JDevlieghere). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D82733: [clang][docs] Add note about using `-flto` with `-g` on macOS

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM. Thank you for adding this! In D82733#2121414 , @phil-blain wrote: > I tested the sphinx build for the `man` and `html` targets. Any o

[PATCH] D82733: [clang][docs] Add note about using `-flto` with `-g` on macOS

JDevlieghere added inline comments. Comment at: clang/docs/CommandGuide/clang.rst:477 + Note: on Darwin, when using :option:`-flto` along with :option:`-g` and + compiling and linking in separate steps, you also need to pass Any reason not to use the sphinx n

[PATCH] D80770: [diagtool] Install diagtool when LLVM_INSTALL_TOOLCHAIN_ONLY is ON.

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

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

JDevlieghere added inline comments. Comment at: llvm/cmake/config-ix.cmake:514 + if(ZLIB_FOUND) +set(LLVM_ENABLE_ZLIB "YES" CACHE STRING + "Use zlib for compression/decompression if available. Can be ON, OFF, or FORCE_ON" phosek wrote: > JDevlieghere w

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

JDevlieghere added a comment. I'm in favor of this change. I'm not too happy with how this works in CMake, I've expressed similar concerns when the FORCE_ON approach was suggested in D71306 . I really like what we ended up with in LLDB. The TL;DR is that we have

[PATCH] D78762: build: use `find_package(Python3)` if available

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. I talked to Saleem and he cleared up some of my concerns. Given that the community seems to have agreed to support only Python 3, this change seems a lot more reasonable. My earlie

[PATCH] D78762: build: use `find_package(Python3)` if available

JDevlieghere added a comment. In D78762#2002390 , @compnerd wrote: > @JDevlieghere I think that adding another mechanism for finding the python > executable is not the right approach. You already have the variables that > you are talking about, you just

[PATCH] D78762: build: use `find_package(Python3)` if available

JDevlieghere requested changes to this revision. JDevlieghere added a comment. This revision now requires changes to proceed. I would strongly prefer to do this differently. While we hope to drop Python 2 support in LLDB as soon as possible, we are not there yet. This patch as it stands will mak

[PATCH] D78807: Fix gendered documentation

JDevlieghere accepted this revision. JDevlieghere added a comment. Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78807/new/ https://reviews.llvm.org/D78807 ___ cfe-commits mailing list cfe-c

[PATCH] D75360: [analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow CheckerManager to be constructed for non-analysis purposes

JDevlieghere added a comment. In D75360#1937556 , @Szelethus wrote: > In D75360#1937415 , @JDevlieghere > wrote: > > > It appears this broke the modules build: > > http://green.lab.llvm.org/green/view/LLDB/job/lld

  1   2   3   >