[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] 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] 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] 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] 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 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] 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] 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] 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] D41102: Setup clang-doc frontend framework

JDevlieghere requested changes to this revision. JDevlieghere added inline comments. This revision now requires changes to proceed. Comment at: tools/clang-doc/ClangDocReporter.cpp:106 + FI.Filename = Filename; + FileRecords.insert(std::pair(Filename, std::move(FI))); +} -

[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] 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] 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] 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] 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] 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] 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. 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#3643001 , @iains wrote: > In D126189#3642992 , @JDevlieghere > wrote: > >> In D126189#3642820 , @iains wrote: >> >>> JFTR, I did

[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#3643045 , @iains wrote: > In D126189#3643021 , @JDevlieghere > wrote: > >> In D126189#3643001 , @iains wrote: >> >>> In D126189#3

[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] 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] 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. 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] 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] 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 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 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 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] D67373: Don't emit .gnu_pubnames when tuning for LLDB

JDevlieghere added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3400 if (DwarfFission != DwarfFissionKind::None || - DebuggerTuning == llvm::DebuggerKind::LLDB || (PubnamesArg && checkDebugInfoOption(PubnamesArg, Args, D, TC))) ---

[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] 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 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] 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] 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 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] 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] 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] 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] 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] 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] 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 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 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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 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] 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] 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] 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 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 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] D75360: [analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow CheckerManager to be constructed for non-analysis purposes

JDevlieghere added a comment. It appears this broke the modules build: http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/12905/console Can you please take a look? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75360/new/ https://reviews.llv

[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

[PATCH] D70544: Debug info: Emit objc_direct methods as members of their containing class

JDevlieghere accepted this revision. JDevlieghere added a comment. LGTM with two minor comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4729 - SmallVector EltTys; - auto CurrenetElts = InterfaceDecl->getElements(); - EltTys.append(CurrenetElts.begin(), C

[PATCH] D70764: build: reduce CMake handling for zlib

JDevlieghere added a comment. Having one canonical variable controlling zlib support seems indeed desirable. In D70519#1754618 , @labath wrote: > With this patch, what is the output of `llvm-config --system-libs` ? @compnerd What's the answer to this fo

[PATCH] D70764: build: reduce CMake handling for zlib

JDevlieghere added a comment. In D70764#1767413 , @compnerd wrote: > In D70764#1767395 , @JDevlieghere > wrote: > > > Having one canonical variable controlling zlib support seems indeed > > desirable. > > > > In D

[PATCH] D71675: [Remarks][Driver] Run dsymutil when remarks are enabled

JDevlieghere added inline comments. Comment at: clang/test/Driver/darwin-opt-record.c:21 +// -gline-tables-only and would need -fno-save-optimization-record to +// completely disable it. +// CHECK-DSYMUTIL-G0: "-cc1" thegameg wrote: > The other choice would be ho

[PATCH] D71675: [Remarks][Driver] Run dsymutil when remarks are enabled

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71675/new/ https://reviews.llvm.org/D71675 ___ cf

[PATCH] D76100: Debug Info: Store the SDK in the DICompileUnit.

JDevlieghere added a comment. Looks good to me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76100/new/ https://reviews.llvm.org/D76100 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo

[PATCH] D76385: Allow remapping Clang module include paths

JDevlieghere added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2488 + auto RemapPath = [&](std::string &Path) { +Path = remapDIPath(Path); +StringRef Relative(Path); aprantl wrote: > Is this legal? > > remapDIPath takes a StringRef to

[PATCH] D73676: [Remarks] Extend the RemarkStreamer to support other emitters

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM with some nits inline. Comment at: llvm/docs/Remarks.rst:630 +example, LLVM IR passes will emit ``llvm::DiagnosticInfoOptimization*`` that +get converted to

[PATCH] D74168: [CMake] Make EXCLUDE_FROM_ALL an argument to add_lit_testsuite

This revision was automatically updated to reflect the committed changes. Closed by commit rG4fe839ef3a51: [CMake] Rename EXCLUDE_FROM_ALL and make it an argument to add_lit_testsuite (authored by JDevlieghere). Herald added projects: clang, Sanitizers, OpenMP. Herald added subscribers: openmp-com

[PATCH] D74168: [CMake] Make EXCLUDE_FROM_ALL an argument to add_lit_testsuite

JDevlieghere marked an inline comment as done. JDevlieghere added inline comments. Comment at: llvm/test/CMakeLists.txt:171 +if(LLVM_BUILD_TOOLS) + set(exclude_from_check_all "EXCLUDE_FROM_CHECK_ALL") JDevlieghere wrote: > kschwarz wrote: > > Hi @JDevlieghere,

[PATCH] D74168: [CMake] Make EXCLUDE_FROM_ALL an argument to add_lit_testsuite

JDevlieghere marked an inline comment as done. JDevlieghere added inline comments. Comment at: llvm/test/CMakeLists.txt:171 +if(LLVM_BUILD_TOOLS) + set(exclude_from_check_all "EXCLUDE_FROM_CHECK_ALL") kschwarz wrote: > Hi @JDevlieghere, we've noticed that with

[PATCH] D72940: Add a support for clang tidy to import another configurations files from .clang-tidy

JDevlieghere added a comment. Please upload the diff with full context . This would also need a test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72940/new/ https://reviews.llvm.org/D72940 _

[PATCH] D72552: [Concepts] Constraint Satisfaction Caching

JDevlieghere added a comment. It looks like the concept changes broke the debugger (quite spectacularly actually): http://lab.llvm.org:8011/builders/lldb-x86_64-debian/builds/3356 http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/12872 http://green.lab.llvm.org/green/view/LLDB/job/

[PATCH] D72552: [Concepts] Constraint Satisfaction Caching

JDevlieghere added a comment. In D72552#1832875 , @JDevlieghere wrote: > It looks like the concept changes broke the debugger (quite spectacularly > actually): > > http://lab.llvm.org:8011/builders/lldb-x86_64-debian/builds/3356 > http://lab.llvm.org:80

[PATCH] D65042: [Concept] Placeholder constraints and abbreviated templates

JDevlieghere added a comment. Hey Saar, I have temporarily reverted this because it broke the LLDB bots. Please run the LLDB test suite when you make changes to the AST importer & keep an eye on the bots when you re-land this. Thanks! commit 62e4b501ab3bc4c5815a179fdd2c4b49574506c1 (HEAD -> m

[PATCH] D42351: Emit DWARF "constructor" calling convention for every supported Clang CC

This revision was automatically updated to reflect the committed changes. Closed by commit rL328196: [CodeGen] Emit DWARF "constructor" calling convention (authored by JDevlieghere, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D42351

[PATCH] D44842: Add Parameters to DW_AT_name Attribute of Template Variables

JDevlieghere added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:2992 + std::string NameString = Name.str(); + llvm::raw_string_ostream ParameterizedName(NameString); + ParameterizedName << "<"; Why not use Name.str() directly? Reposit

[PATCH] D44842: Add Parameters to DW_AT_name Attribute of Template Variables

JDevlieghere added a comment. @jingham is this safe to land? Comment at: lib/CodeGen/CGDebugInfo.cpp:2992 + std::string NameString = Name.str(); + llvm::raw_string_ostream ParameterizedName(NameString); + ParameterizedName << "<"; ormris wrote: >

[PATCH] D39310: [CGBlocks] Improve line info in backtraces containing *_helper_block

JDevlieghere added inline comments. Comment at: lib/CodeGen/CGBlocks.cpp:1651 StartFunction(FD, C.VoidTy, Fn, FI, args); - // Create a scope with an artificial location for the body of this function. - auto AL = ApplyDebugLocation::CreateArtificial(*this); + ApplyDebugLocat

[PATCH] D39310: [CGBlocks] Improve line info in backtraces containing *_helper_block

JDevlieghere added inline comments. Comment at: lib/CodeGen/CGBlocks.cpp:1651 StartFunction(FD, C.VoidTy, Fn, FI, args); - // Create a scope with an artificial location for the body of this function. - auto AL = ApplyDebugLocation::CreateArtificial(*this); + ApplyDebugLocat

[PATCH] D39834: [clang] -foptimization-record-file= should imply -fsave-optimization-record

JDevlieghere added a comment. This also ensures that if `fno_save_optimization_record` is specified, you don't overwrite it by setting the file. This is definitely something you'd want to add to your test case. https://reviews.llvm.org/D39834 ___

[PATCH] D39834: [clang] -foptimization-record-file= should imply -fsave-optimization-record

JDevlieghere added a comment. I think you can achieve the same result with less code by checking for the flag's presence higher up, where currently `OPT_fsave_optimization_record` is handled (Clang.cpp:4329). Something like: if (Args.hasFlag(options::OPT_fsave_optimization_record,

<    1   2   3   >