[Lldb-commits] [PATCH] D146041: initial commit

mehdi_amini added a comment. Herald added a subscriber: JDevlieghere. You should look into the title and description of the commit: https://cbea.ms/git-commit/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146041/new/

[Lldb-commits] [PATCH] D141298: Move from llvm::makeArrayRef to ArrayRef deduction guides - last part

mehdi_amini added a comment. Seems mechanical, and if it build everywhere LGTM :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141298/new/ https://reviews.llvm.org/D141298 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D140896: [WIP] Move from llvm::makeArrayRef to ArrayRef deduction guides

mehdi_amini added inline comments. Herald added a subscriber: Michael137. Comment at: llvm/include/llvm/ADT/ArrayRef.h:502 + /// Deduction guide to construct an ArrayRef from a C array. + template ArrayRef(const T ()[N]) -> ArrayRef; Can we keep the

[Lldb-commits] [PATCH] D134878: Update developer policy on potentially breaking changes

mehdi_amini added a comment. From what I saw when you posted the discourse thread initially, I understand you're targeting user-visible features, and mostly from the "toolchain" side of the project. However I find the language here to be potentially confusing for the API surface of the

[Lldb-commits] [PATCH] D130689: [LLVM] Update C++ standard to 17

mehdi_amini added a comment. > There should be a better way than this. Comprehensive pre-merge testing of > all PRs etc. We already have pre-commit tests on Phabricator on Windows and Linux, but that's not exhaustive and for many reasons I don't believe this is realistic in any way: we will

[Lldb-commits] [PATCH] D130689: [LLVM] Update C++ standard to 17

mehdi_amini added a comment. > Agreed, I think we need to update the protocol for changing the C++ standard > in the future to account for more testing beforehand. The way I would do it is: wait for a Sunday so that the bots aren't loaded, land the change, wait for a couple of hours to ensure

[Lldb-commits] [PATCH] D130689: [LLVM] Update C++ standard to 17

mehdi_amini added a comment. In D130689#3686760 , @h-vetinari wrote: > My point boils down to: "written using standard C++17 > code" does not sound at all like "core language, no stdlib", but very much > like "core+stdlib". We're allowing C++17

[Lldb-commits] [PATCH] D130689: [LLVM] Update C++ standard to 17

mehdi_amini added a comment. In D130689#3684333 , @thieta wrote: > In D130689#3684330 , @mehdi_amini > wrote: > >> What does it mean exactly? We can't use **anything** C++17 without writing >> it in the coding

[Lldb-commits] [PATCH] D130689: [LLVM] Update C++ standard to 17

mehdi_amini added a comment. Herald added a subscriber: JDevlieghere. Nice, LGTM, thanks for driving this! > Remember that if we want to adopt some new feature in a bigger way it should > be discussed and added to the CodingStandard document. What does it mean exactly? We can't use

[Lldb-commits] [PATCH] D114699: Fixed a memory leak in the PDLToPDLInterp RootOrderingTest.

mehdi_amini added inline comments. Comment at: mlir/unittests/Conversion/PDLToPDLInterp/RootOrderingTest.cpp:37 + ~RootOrderingTest() { +for (int i = 0; i < 4; ++i) + v[i].getDefiningOp()->destroy(); bondhugula wrote: > bondhugula wrote: > > You need

[Lldb-commits] [PATCH] D98179: [lit] Sort test start times based on prior test timing data

mehdi_amini added a comment. > Can we revert to the previous behavior please? The current behavior is not > user friendly. Thanks! To be clear: you car about the order in the final summary, not the actually execution order, right? How are you diffing? Copy-pasting the terminal output to a

[Lldb-commits] [PATCH] D98179: [lit] Sort test start times based on prior test timing data

mehdi_amini added a comment. > No, I'm running lit and dumping into a file. sorting is not as easy as piping > through sort, as I'm running with -vv (required). Since tests run in parallel, don't you already have some non-determinism in the -vv output? Isn't it printing as test finish? (just

[Lldb-commits] [PATCH] D98179: [lit] Sort test start times based on prior test timing data

mehdi_amini added inline comments. Herald added a subscriber: JDevlieghere. Comment at: llvm/utils/lit/lit/cl_arguments.py:157 selection_group.add_argument("-i", "--incremental", -help="Run modified and failing tests first (updates mtimes)", +

[Lldb-commits] [PATCH] D95766: [Branch-Rename] Fix some links

mehdi_amini accepted this revision. mehdi_amini added inline comments. Comment at: clang-tools-extra/clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.h:20 // Check for underscores in the names of googletest tests, per -//

[Lldb-commits] [PATCH] D84691: [CMake] Move find_package(ZLIB) to LLVMConfig

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

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

mehdi_amini added inline comments. Comment at: mlir/examples/standalone/CMakeLists.txt:35 +endif() + include(TableGen) I am a bit unsure that it is desirable that it is desirable to need this change? This requires every single user of LLVM to do this. Is there

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

mehdi_amini added inline comments. Comment at: mlir/examples/standalone/CMakeLists.txt:35 +endif() + include(TableGen) mehdi_amini wrote: > I am a bit unsure that it is desirable that it is desirable to need this > change? > This requires every single user of

[Lldb-commits] [PATCH] D80130: [mlir][SystemZ] Fix incompatible datalayout in SystemZ

mehdi_amini accepted this revision. mehdi_amini added inline comments. Comment at: mlir/lib/ExecutionEngine/ExecutionEngine.cpp:123 } - std::unique_ptr machine( - target->createTargetMachine(targetTriple, "generic", "", {}, {})); + std::string cpu =

[Lldb-commits] [PATCH] D80130: [mlir][SystemZ] Fix incompatible datalayout in SystemZ

This revision was automatically updated to reflect the committed changes. Closed by commit rG9f2ce5b915a5: [mlir][SystemZ] Fix incompatible datalayout in SystemZ (authored by imaihal, committed by mehdi_amini). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D57330: Adjust documentation for git migration.

mehdi_amini added a comment. > You can avoid the git status pollution by adding the build directory to > .git/info/exclude. Good to know! Should we include this in the doc? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57330/new/

[Lldb-commits] [PATCH] D57330: Adjust documentation for git migration.

mehdi_amini added a comment. In D57330#1375096 , @labath wrote: > This is not an full out-of-source build, since the build folder is still a > subfolder of the repo root My definition of what qualify an "out-of-source" build is that the build process

[Lldb-commits] [PATCH] D57330: Adjust documentation for git migration.

mehdi_amini added a comment. LGTM, but for one comment that requires a fix I believe (lld on Windows) Comment at: libcxx/docs/BuildingLibcxx.rst:47 - * ``cd build`` - * ``cmake -G [options] `` So nice to see these steps going away :)

[Lldb-commits] [PATCH] D34550: Fix typo: using && instead of & when evaluating a mask

This revision was automatically updated to reflect the committed changes. Closed by commit rL306134: Fix typo: using && instead of & when evaluating a mask (authored by mehdi_amini). Changed prior to commit: https://reviews.llvm.org/D34550?vs=103690=103752#toc Repository: rL LLVM

[Lldb-commits] [PATCH] D34550: Fix typo: using && instead of & when evaluating a mask

mehdi_amini created this revision. Herald added a subscriber: emaste. Reported by coverity, I don't know how to provide a test. https://reviews.llvm.org/D34550 Files: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

[Lldb-commits] [PATCH] D29288: Switch std::call_once to llvm::call_once

mehdi_amini added a comment. I'm fine with this change, but I'll leave the approval to one of the LLDB developer :) (Thanks for following-up with the libstdc++ on these platforms!) Repository: rL LLVM https://reviews.llvm.org/D29288 ___