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

[Louis Dionne via Phabricator via lldb-commits]

ldionne added a comment.

(sorry, no idea why it says "this revision is now accepted and ready to land on 
my behalf, I was just removing the libc++ review group to clear our review 
queue)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112374/new/

https://reviews.llvm.org/D112374

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

[Louis Dionne via Phabricator via lldb-commits]

ldionne added a comment.
Herald added subscribers: wangpc, gysit, Dinistro, hoy, bviyer, wlei, jplehr, 
PiotrZSL, luke, sunshaoce.
Herald added a reviewer: springerm.
Herald added a reviewer: kiranchandramohan.
Herald added a project: clang-format.
Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay.

The libcxx, libcxxabi and libunwind parts were committed in 
b397921fc7ef4e2882b3ae9c5f12fb40daca4078 
. Is it 
possible to close this or rebase it so we can remove the libc++ subscription to 
clear the review queue? We're trying to clean things up as part of the Github 
PR transition.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137338/new/

https://reviews.llvm.org/D137338

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D106339: Add support to generate Sphinx DOCX documentation

[Louis Dionne via Phabricator via lldb-commits]

ldionne commandeered this revision.
ldionne added a reviewer: t-tye.
ldionne added a comment.
Herald added a subscriber: jplehr.
Herald added a project: All.

[GitHub PR transition cleanup]

This has been open for 2 years with multiple subscribers able to see this 
discussion, but there doesn't seem to be a lot of interest for moving forward 
with this. I will commandeer and abandon to clear the review queue in the 
context of moving to Github PRs. If you'd still like to push for this to 
happen, feel free to re-open this as a Github PR.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106339/new/

https://reviews.llvm.org/D106339

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D158863: Implement the monolithic CI pipeline in the monorepo

[Louis Dionne via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcf1a3d93581f: Implement the monolithic CI pipeline in the 
monorepo (authored by ldionne).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158863/new/

https://reviews.llvm.org/D158863

Files:
  .ci/generate-buildkite-pipeline-premerge
  .ci/generate-buildkite-pipeline-scheduled
  .ci/monolithic-linux.sh
  .ci/monolithic-windows.sh

Index: .ci/monolithic-windows.sh
===
--- /dev/null
+++ .ci/monolithic-windows.sh
@@ -0,0 +1,48 @@
+#!/usr/bin/env bash
+#===--===##
+#
+# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+# See https://llvm.org/LICENSE.txt for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+#
+#===--===##
+
+#
+# This script performs a monolithic build of the monorepo and runs the tests of
+# most projects on Windows. This should be replaced by per-project scripts that
+# run only the relevant tests.
+#
+
+set -ex
+set -o pipefail
+
+MONOREPO_ROOT="${MONOREPO_ROOT:="$(git rev-parse --show-toplevel)"}"
+BUILD_DIR="${BUILD_DIR:=${MONOREPO_ROOT}/build/monolithic-windows}"
+
+rm -rf ${BUILD_DIR}
+
+sccache --zero-stats
+function show-stats {
+  sccache --show-stats
+}
+trap show-stats EXIT
+
+projects="${1}"
+targets="${2}"
+
+echo "--- cmake"
+pip install -q -r ${MONOREPO_ROOT}/mlir/python/requirements.txt
+cmake -S ${MONOREPO_ROOT}/llvm -B ${BUILD_DIR} \
+  -D LLVM_ENABLE_PROJECTS="${projects}" \
+  -G Ninja \
+  -D CMAKE_BUILD_TYPE=Release \
+  -D LLVM_ENABLE_ASSERTIONS=ON \
+  -D LLVM_BUILD_EXAMPLES=ON \
+  -D COMPILER_RT_BUILD_LIBFUZZER=OFF \
+  -D LLVM_LIT_ARGS="-v --xunit-xml-output ${BUILD_DIR}/test-results.xml" \
+  -D COMPILER_RT_BUILD_ORC=OFF \
+  -D CMAKE_C_COMPILER_LAUNCHER=sccache \
+  -D CMAKE_CXX_COMPILER_LAUNCHER=sccache
+
+echo "--- ninja"
+ninja -C ${BUILD_DIR} ${targets}
Index: .ci/monolithic-linux.sh
===
--- /dev/null
+++ .ci/monolithic-linux.sh
@@ -0,0 +1,50 @@
+#!/usr/bin/env bash
+#===--===##
+#
+# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+# See https://llvm.org/LICENSE.txt for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+#
+#===--===##
+
+#
+# This script performs a monolithic build of the monorepo and runs the tests of
+# most projects on Linux. This should be replaced by per-project scripts that
+# run only the relevant tests.
+#
+
+set -ex
+set -o pipefail
+
+MONOREPO_ROOT="${MONOREPO_ROOT:="$(git rev-parse --show-toplevel)"}"
+BUILD_DIR="${BUILD_DIR:=${MONOREPO_ROOT}/build/monolithic-linux}"
+
+rm -rf ${BUILD_DIR}
+
+ccache --zero-stats
+ccache --show-config
+function show-stats {
+  ccache --print-stats
+}
+trap show-stats EXIT
+
+projects="${1}"
+targets="${2}"
+
+echo "--- cmake"
+pip install -q -r ${MONOREPO_ROOT}/mlir/python/requirements.txt
+cmake -S ${MONOREPO_ROOT}/llvm -B ${BUILD_DIR} \
+  -D LLVM_ENABLE_PROJECTS="${projects}" \
+  -G Ninja \
+  -D CMAKE_BUILD_TYPE=Release \
+  -D LLVM_ENABLE_ASSERTIONS=ON \
+  -D LLVM_BUILD_EXAMPLES=ON \
+  -D COMPILER_RT_BUILD_LIBFUZZER=OFF \
+  -D LLVM_LIT_ARGS="-v --xunit-xml-output ${BUILD_DIR}/test-results.xml" \
+  -D LLVM_ENABLE_LLD=ON \
+  -D CMAKE_CXX_FLAGS=-gmlt \
+  -D BOLT_CLANG_EXE=/usr/bin/clang \
+  -D LLVM_CCACHE_BUILD=ON
+
+echo "--- ninja"
+ninja -C ${BUILD_DIR} ${targets}
Index: .ci/generate-buildkite-pipeline-scheduled
===
--- .ci/generate-buildkite-pipeline-scheduled
+++ .ci/generate-buildkite-pipeline-scheduled
@@ -27,4 +27,43 @@
   message: "${BUILDKITE_MESSAGE}"
   commit: "${BUILDKITE_COMMIT}"
   branch: "${BUILDKITE_BRANCH}"
-EOF
+
+  - label: ':linux: x64 Debian'
+artifact_paths:
+  - '*_result.json'
+  - 'build/monolithic-linux/test-results.xml'
+agents:
+  queue: 'linux'
+retry:
+  automatic:
+- exit_status: -1  # Agent was lost
+  limit: 2
+- exit_status: 255
+  limit: 2 # Forced agent shutdown
+timeout_in_minutes: 120
+env:
+  CC: 'clang'
+  CXX: 'clang++'
+commands:
+  - './.ci/monolithic-linux.sh "bolt;clang-tools-extra;compiler-rt;flang;libc;libclc;lld;llvm;mlir;polly;pstl" "check-all"'
+
+  - label: ':windows: x64 Windows'
+artifact_paths:
+  - '*_result.json'
+  - 'build/monolithic-windows/test-results.xml'
+agents:
+   

[Lldb-commits] [PATCH] D158863: Implement the monolithic CI pipeline in the monorepo

[Louis Dionne via Phabricator via lldb-commits]

ldionne updated this revision to Diff 554420.
ldionne marked an inline comment as done.
ldionne added a comment.

Address review comments. I did some testing using GH PRs and this should work, 
but some tweaks might be necessary. After discussing with Mikhail, I'll merge 
this now and we can make tweaks and fixes on top of it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158863/new/

https://reviews.llvm.org/D158863

Files:
  .ci/generate-buildkite-pipeline-premerge
  .ci/generate-buildkite-pipeline-scheduled
  .ci/monolithic-linux.sh
  .ci/monolithic-windows.sh

Index: .ci/monolithic-windows.sh
===
--- /dev/null
+++ .ci/monolithic-windows.sh
@@ -0,0 +1,48 @@
+#!/usr/bin/env bash
+#===--===##
+#
+# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+# See https://llvm.org/LICENSE.txt for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+#
+#===--===##
+
+#
+# This script performs a monolithic build of the monorepo and runs the tests of
+# most projects on Windows. This should be replaced by per-project scripts that
+# run only the relevant tests.
+#
+
+set -ex
+set -o pipefail
+
+MONOREPO_ROOT="${MONOREPO_ROOT:="$(git rev-parse --show-toplevel)"}"
+BUILD_DIR="${BUILD_DIR:=${MONOREPO_ROOT}/build/monolithic-windows}"
+
+rm -rf ${BUILD_DIR}
+
+sccache --zero-stats
+function show-stats {
+  sccache --show-stats
+}
+trap show-stats EXIT
+
+projects="${1}"
+targets="${2}"
+
+echo "--- cmake"
+pip install -q -r ${MONOREPO_ROOT}/mlir/python/requirements.txt
+cmake -S ${MONOREPO_ROOT}/llvm -B ${BUILD_DIR} \
+  -D LLVM_ENABLE_PROJECTS="${projects}" \
+  -G Ninja \
+  -D CMAKE_BUILD_TYPE=Release \
+  -D LLVM_ENABLE_ASSERTIONS=ON \
+  -D LLVM_BUILD_EXAMPLES=ON \
+  -D COMPILER_RT_BUILD_LIBFUZZER=OFF \
+  -D LLVM_LIT_ARGS="-v --xunit-xml-output ${BUILD_DIR}/test-results.xml" \
+  -D COMPILER_RT_BUILD_ORC=OFF \
+  -D CMAKE_C_COMPILER_LAUNCHER=sccache \
+  -D CMAKE_CXX_COMPILER_LAUNCHER=sccache
+
+echo "--- ninja"
+ninja -C ${BUILD_DIR} ${targets}
Index: .ci/monolithic-linux.sh
===
--- /dev/null
+++ .ci/monolithic-linux.sh
@@ -0,0 +1,50 @@
+#!/usr/bin/env bash
+#===--===##
+#
+# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+# See https://llvm.org/LICENSE.txt for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+#
+#===--===##
+
+#
+# This script performs a monolithic build of the monorepo and runs the tests of
+# most projects on Linux. This should be replaced by per-project scripts that
+# run only the relevant tests.
+#
+
+set -ex
+set -o pipefail
+
+MONOREPO_ROOT="${MONOREPO_ROOT:="$(git rev-parse --show-toplevel)"}"
+BUILD_DIR="${BUILD_DIR:=${MONOREPO_ROOT}/build/monolithic-linux}"
+
+rm -rf ${BUILD_DIR}
+
+ccache --zero-stats
+ccache --show-config
+function show-stats {
+  ccache --print-stats
+}
+trap show-stats EXIT
+
+projects="${1}"
+targets="${2}"
+
+echo "--- cmake"
+pip install -q -r ${MONOREPO_ROOT}/mlir/python/requirements.txt
+cmake -S ${MONOREPO_ROOT}/llvm -B ${BUILD_DIR} \
+  -D LLVM_ENABLE_PROJECTS="${projects}" \
+  -G Ninja \
+  -D CMAKE_BUILD_TYPE=Release \
+  -D LLVM_ENABLE_ASSERTIONS=ON \
+  -D LLVM_BUILD_EXAMPLES=ON \
+  -D COMPILER_RT_BUILD_LIBFUZZER=OFF \
+  -D LLVM_LIT_ARGS="-v --xunit-xml-output ${BUILD_DIR}/test-results.xml" \
+  -D LLVM_ENABLE_LLD=ON \
+  -D CMAKE_CXX_FLAGS=-gmlt \
+  -D BOLT_CLANG_EXE=/usr/bin/clang \
+  -D LLVM_CCACHE_BUILD=ON
+
+echo "--- ninja"
+ninja -C ${BUILD_DIR} ${targets}
Index: .ci/generate-buildkite-pipeline-scheduled
===
--- .ci/generate-buildkite-pipeline-scheduled
+++ .ci/generate-buildkite-pipeline-scheduled
@@ -27,4 +27,43 @@
   message: "${BUILDKITE_MESSAGE}"
   commit: "${BUILDKITE_COMMIT}"
   branch: "${BUILDKITE_BRANCH}"
-EOF
+
+  - label: ':linux: x64 Debian'
+artifact_paths:
+  - '*_result.json'
+  - 'build/monolithic-linux/test-results.xml'
+agents:
+  queue: 'linux'
+retry:
+  automatic:
+- exit_status: -1  # Agent was lost
+  limit: 2
+- exit_status: 255
+  limit: 2 # Forced agent shutdown
+timeout_in_minutes: 120
+env:
+  CC: 'clang'
+  CXX: 'clang++'
+commands:
+  - './.ci/monolithic-linux.sh "bolt;clang-tools-extra;compiler-rt;flang;libc;libclc;lld;llvm;mlir;polly;pstl" "check-all"'
+
+  - label: ':windows: x64 Windows'
+artifact_paths:
+

[Lldb-commits] [PATCH] D158863: Implement the monolithic CI pipeline in the monorepo

[Louis Dionne via Phabricator via lldb-commits]

ldionne marked an inline comment as done.
ldionne added inline comments.



Comment at: .ci/generate-buildkite-pipeline-premerge:163
+
+if [[ ! ${SPECIFIC_PIPELINE_AVAILABLE} -eq 1 ]]; then
+  # Figure out which projects need to be built on each platform

goncharov wrote:
> ldionne wrote:
> > goncharov wrote:
> > > do I understand correctly that if mlir and libcxx modified then only 
> > > libcxx will be run as SPECIFIC_PIPELINE_AVAILABLE=1?
> > Yes. This corresponds to the current logic as well. Basically if a project 
> > has some custom CI set up, we don't want to also run the general CI since 
> > that's just a waste of resources.
> The previous logic was that we caclulated first all projects that might be 
> affected by current set (e.g. mlir is affected by llvm), then we added add 
> dependencies.
> So e.g. if mlir was modified, we should add "flang" to test set and then add 
> "llvm clang" as dependincies, resutling in "mlir flang llvm clang" set. I can 
> re-implement this logic later here no problem.
> Also, it seems incorrect to only run "libc++" tests if libc++ was modified 
> among other things. E.g. if something has touched libc++ and mlir than mlir 
> should still run.
Ah, I see. Yeah I guess that makes sense. I'll update this review with the 
updated logic.

> Also, it seems incorrect to only run "libc++" tests if libc++ was modified 
> among other things. E.g. if something has touched libc++ and mlir than mlir 
> should still run.

You're right, I guess in that case we should run both jobs. I'll remove the 
check.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158863/new/

https://reviews.llvm.org/D158863

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D158863: Implement the monolithic CI pipeline in the monorepo

[Louis Dionne via Phabricator via lldb-commits]

ldionne added a comment.
Herald added a subscriber: JDevlieghere.

Sorry for spamming everyone, I'm trying to ensure that the CI will work with 
all the sub-projects in the monorepo once we move to GH PRs.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158863/new/

https://reviews.llvm.org/D158863

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D158863: Implement the monolithic CI pipeline in the monorepo

[Louis Dionne via Phabricator via lldb-commits]

ldionne updated this revision to Diff 554016.
ldionne marked an inline comment as done.
ldionne added a comment.
Herald added a reviewer: bollu.
Herald added subscribers: cfe-commits, libc-commits, openmp-commits, 
libcxx-commits, lldb-commits, Sanitizers, Enna1, yota9, ayermolo, jvesely.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added projects: Sanitizers, LLDB, libc++, OpenMP, libc-project, 
clang-tools-extra, Flang.
Herald added a reviewer: libc++.
This revision now requires review to proceed.

Test building all projects.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158863/new/

https://reviews.llvm.org/D158863

Files:
  .ci/generate-buildkite-pipeline-premerge
  .ci/monolithic-linux.sh
  .ci/monolithic-windows.sh
  bolt/foo
  clang-tools-extra/foo
  compiler-rt/foo
  cross-project-tests/foo
  flang/foo
  libc/foo
  libclc/foo
  lld/foo
  lldb/foo
  llvm-libgcc/foo
  llvm/foo
  mlir/foo
  openmp/foo
  polly/foo
  pstl/foo

Index: .ci/monolithic-windows.sh
===
--- /dev/null
+++ .ci/monolithic-windows.sh
@@ -0,0 +1,48 @@
+#!/usr/bin/env bash
+#===--===##
+#
+# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+# See https://llvm.org/LICENSE.txt for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+#
+#===--===##
+
+#
+# This script performs a monolithic build of the monorepo and runs the tests of
+# most projects on Windows. This should be replaced by per-project scripts that
+# run only the relevant tests.
+#
+
+set -ex
+set -o pipefail
+
+MONOREPO_ROOT="${MONOREPO_ROOT:="$(git rev-parse --show-toplevel)"}"
+BUILD_DIR="${BUILD_DIR:=${MONOREPO_ROOT}/build/monolithic-windows}"
+
+rm -rf ${BUILD_DIR}
+
+sccache --zero-stats
+function show-stats {
+  sccache --show-stats
+}
+trap show-stats EXIT
+
+projects="${1}"
+targets="${2}"
+
+echo "--- cmake"
+pip install -q -r ${MONOREPO_ROOT}/mlir/python/requirements.txt
+cmake -S ${MONOREPO_ROOT}/llvm -B ${BUILD_DIR} \
+  -D LLVM_ENABLE_PROJECTS="${projects}" \
+  -G Ninja \
+  -D CMAKE_BUILD_TYPE=Release \
+  -D LLVM_ENABLE_ASSERTIONS=ON \
+  -D LLVM_BUILD_EXAMPLES=ON \
+  -D COMPILER_RT_BUILD_LIBFUZZER=OFF \
+  -D LLVM_LIT_ARGS="-v --xunit-xml-output ${BUILD_DIR}/test-results.xml" \
+  -D COMPILER_RT_BUILD_ORC=OFF \
+  -D CMAKE_C_COMPILER_LAUNCHER=sccache \
+  -D CMAKE_CXX_COMPILER_LAUNCHER=sccache
+
+echo "--- ninja"
+ninja -C ${BUILD_DIR} ${targets}
Index: .ci/monolithic-linux.sh
===
--- /dev/null
+++ .ci/monolithic-linux.sh
@@ -0,0 +1,50 @@
+#!/usr/bin/env bash
+#===--===##
+#
+# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+# See https://llvm.org/LICENSE.txt for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+#
+#===--===##
+
+#
+# This script performs a monolithic build of the monorepo and runs the tests of
+# most projects on Linux. This should be replaced by per-project scripts that
+# run only the relevant tests.
+#
+
+set -ex
+set -o pipefail
+
+MONOREPO_ROOT="${MONOREPO_ROOT:="$(git rev-parse --show-toplevel)"}"
+BUILD_DIR="${BUILD_DIR:=${MONOREPO_ROOT}/build/monolithic-linux}"
+
+rm -rf ${BUILD_DIR}
+
+ccache --zero-stats
+ccache --show-config
+function show-stats {
+  ccache --print-stats
+}
+trap show-stats EXIT
+
+projects="${1}"
+targets="${2}"
+
+echo "--- cmake"
+pip install -q -r ${MONOREPO_ROOT}/mlir/python/requirements.txt
+cmake -S ${MONOREPO_ROOT}/llvm -B ${BUILD_DIR} \
+  -D LLVM_ENABLE_PROJECTS="${projects}" \
+  -G Ninja \
+  -D CMAKE_BUILD_TYPE=Release \
+  -D LLVM_ENABLE_ASSERTIONS=ON \
+  -D LLVM_BUILD_EXAMPLES=ON \
+  -D COMPILER_RT_BUILD_LIBFUZZER=OFF \
+  -D LLVM_LIT_ARGS="-v --xunit-xml-output ${BUILD_DIR}/test-results.xml" \
+  -D LLVM_ENABLE_LLD=ON \
+  -D CMAKE_CXX_FLAGS=-gmlt \
+  -D BOLT_CLANG_EXE=/usr/bin/clang \
+  -D LLVM_CCACHE_BUILD=ON
+
+echo "--- ninja"
+ninja -C ${BUILD_DIR} ${targets}
Index: .ci/generate-buildkite-pipeline-premerge
===
--- .ci/generate-buildkite-pipeline-premerge
+++ .ci/generate-buildkite-pipeline-premerge
@@ -14,11 +14,6 @@
 # See https://buildkite.com/docs/agent/v3/cli-pipeline#pipeline-format.
 #
 
-if ! git diff --name-only HEAD~1 | grep -q -E "^libcxx/|^libcxxabi/|^libunwind/|^runtimes/|^cmake/|^clang/"; then
-  # libcxx/, libcxxabi/, libunwind/, runtimes/, cmake/ or clang/ are not affected
-  exit 0
-fi
-
 reviewID="$(git 

[Lldb-commits] [PATCH] D142007: [NFC] Fix "form/from" typos

[Louis Dionne via Phabricator via lldb-commits]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

LGTM, thanks! I don't think you need to wait for other owners to approve before 
landing this, this is pretty clearly an improvement.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142007/new/

https://reviews.llvm.org/D142007

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137724: [CMake] Warn when the version is older than 3.20.0.

[Louis Dionne via Phabricator via lldb-commits]

ldionne added a comment.

LGTM (sorry, it looks like I approved on behalf of all libc++ vendors but that 
wasn't my intention).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137724/new/

https://reviews.llvm.org/D137724

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

[Louis Dionne via Phabricator via lldb-commits]

ldionne accepted this revision.
ldionne added a comment.

Thanks for the fixes. LGTM for `libcxx/`, `libcxxabi/` and `libunwind/`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137338/new/

https://reviews.llvm.org/D137338

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D132608: [CMake] Clean up CMake binary dir handling

[Louis Dionne via Phabricator via lldb-commits]

ldionne added inline comments.



Comment at: libcxx/CMakeLists.txt:421
+if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
+  set(default_install_path 
"${CMAKE_INSTALL_INCLUDEDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++/v1")
+else()

ldionne wrote:
> Instead, I'd do this:
> 
> ```
> if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
>   set(_include_target_dir 
> "${CMAKE_INSTALL_INCLUDEDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++/v1")
>   set(_install_library_dir 
> "lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE}")
> else()
>   set(_include_target_dir "${LIBCXX_INSTALL_INCLUDE_DIR}")
>   set(_install_library_dir "lib${LLVM_LIBDIR_SUFFIX}")
> endif()
> 
> set(LIBCXX_INSTALL_INCLUDE_TARGET_DIR "${_include_target_dir}" CACHE PATH
> "Path where target-specific libc++ headers should be installed.")
> set(LIBCXX_INSTALL_LIBRARY_DIR "${_install_library_dir}" CACHE PATH
> "Path where built libc++ libraries should be installed.")
> ```
> 
> IMO that's easier on the eye.
Gentle ping on this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132608/new/

https://reviews.llvm.org/D132608

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D132608: [CMake] Clean up CMake binary dir handling

[Louis Dionne via Phabricator via lldb-commits]

ldionne added a comment.

I think I like the spirit of this change, which seems to be to move us closer 
to `CMAKE_foo` variables and further from `LLVM_foo` variables for equivalent 
functionality. I have a comment, but this essentially LGTM. The libc++ CI 
failures (in particular the bootstrapping build failure) seems to be real, 
though, so it should be understood before landing the patch.




Comment at: libcxx/CMakeLists.txt:421
+if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
+  set(default_install_path 
"${CMAKE_INSTALL_INCLUDEDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++/v1")
+else()

Instead, I'd do this:

```
if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
  set(_include_target_dir 
"${CMAKE_INSTALL_INCLUDEDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++/v1")
  set(_install_library_dir 
"lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE}")
else()
  set(_include_target_dir "${LIBCXX_INSTALL_INCLUDE_DIR}")
  set(_install_library_dir "lib${LLVM_LIBDIR_SUFFIX}")
endif()

set(LIBCXX_INSTALL_INCLUDE_TARGET_DIR "${_include_target_dir}" CACHE PATH
"Path where target-specific libc++ headers should be installed.")
set(LIBCXX_INSTALL_LIBRARY_DIR "${_install_library_dir}" CACHE PATH
"Path where built libc++ libraries should be installed.")
```

IMO that's easier on the eye.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132608/new/

https://reviews.llvm.org/D132608

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

[Louis Dionne via Phabricator via lldb-commits]

ldionne accepted this revision as: libc++, libc++abi, ldionne.
ldionne added a comment.

I'm happy with this from the libc++/libc++abi side of things. You can ignore 
the failing CI job, it's been addressed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130586/new/

https://reviews.llvm.org/D130586

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Louis Dionne via Phabricator via lldb-commits]

ldionne accepted this revision.
ldionne added inline comments.



Comment at: llvm/docs/DeveloperPolicy.rst:140
+
+* After the change has been committed to the repository, the potentially
+  disruptive changes described in the release notes should be posted to the

aaron.ballman wrote:
> ldionne wrote:
> > I wonder whether `Announcements` is truly a lower-traffic alternative to 
> > `vendors` groups, if we end up posting each potentially breaking change to 
> > the list and tagging vendors on each such review. I'm not against posting 
> > on Discourse, however it seems to me like basically another equivalent 
> > channel of communication for these changes (which might be beneficial, I'm 
> > neutral on that).
> The reason we have a split like that is for timing and chattiness. If you're 
> a downstream like Intel has with ICX, you might want to be in `clang-vendors` 
> so that you are involved in conversations about potentially breaking changes. 
> You'll be getting emails for all review comments on that review. But if 
> you're a downstream like a Gentoo package maintainer, you might not want to 
> be *that* involved in the development of the compiler, but still want to know 
> when changes are coming down the pipeline to do early pre-release testing 
> while there's still time to put the brakes on before a release goes out.
Okay, makes sense. Let's go for it.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134878/new/

https://reviews.llvm.org/D134878

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Louis Dionne via Phabricator via lldb-commits]

ldionne added a comment.

Thanks for working on this! FWIW, this more or less standardizes what we've 
been doing in libc++ for the past ~1.5 years and it's been pretty low effort 
for us to do. And putting my vendor hat on, it's been extremely useful for me 
to track down potential issues when trying to ship a new version of libc++. 
This LGTM, although I'm somewhat neutral on whether to post on Discourse as 
well as having the vendor groups on Phabricator. I'm not sure I understand the 
benefit of doing both, but I will happily conform if folks see value in it.




Comment at: llvm/docs/DeveloperPolicy.rst:140
+
+* After the change has been committed to the repository, the potentially
+  disruptive changes described in the release notes should be posted to the

I wonder whether `Announcements` is truly a lower-traffic alternative to 
`vendors` groups, if we end up posting each potentially breaking change to the 
list and tagging vendors on each such review. I'm not against posting on 
Discourse, however it seems to me like basically another equivalent channel of 
communication for these changes (which might be beneficial, I'm neutral on 
that).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134878/new/

https://reviews.llvm.org/D134878

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D133618: Adapt LLDB dataformatters for libcxx change D129386

[Louis Dionne via Phabricator via lldb-commits]

ldionne accepted this revision.
ldionne added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:1004
+  auto dataobj = GetChildMemberWithName(
+  valobj, {ConstString("__data_"), ConstString("__data")});
+  auto sizeobj = GetChildMemberWithName(

IIUC you're trying to support both the old and the new naming?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133618/new/

https://reviews.llvm.org/D133618

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D128465: [llvm] cmake config groundwork to have ZSTD in LLVM

[Louis Dionne via Phabricator via lldb-commits]

ldionne added inline comments.



Comment at: llvm/CMakeLists.txt:441
 
+set(LLVM_ENABLE_ZSTD "ON" CACHE STRING "Use zstd for compression/decompression 
if available. Can be ON, OFF, or FORCE_ON")
+

This broke builds that do not specify `LLVM_ENABLE_ZSTD=OFF` explicitly on 
macOS, and I suspect on other platforms as well:

```
ld: library not found for -lzstd
clang: error: linker command failed with exit code 1 (use -v to see invocation)
```

It looks to me like the build should not require `zstd` to be installed by 
default.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128465/new/

https://reviews.llvm.org/D128465

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D127444: [libc++] Use ABI tags instead of internal linkage to provide per-TU insulation

[Louis Dionne via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd2e86866be0f: [libc++] Re-apply the use of ABI tags to 
provide per-TU insulation (authored by ldionne).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127444/new/

https://reviews.llvm.org/D127444

Files:
  libcxx/CMakeLists.txt
  libcxx/cmake/caches/Apple.cmake
  libcxx/docs/BuildingLibcxx.rst
  libcxx/docs/DesignDocs/VisibilityMacros.rst
  libcxx/docs/ReleaseNotes.rst
  libcxx/include/__config
  libcxx/include/__config_site.in
  libcxx/test/libcxx/strings/basic.string/PR42676.sh.cpp
  lldb/packages/Python/lldbsuite/test/make/Makefile.rules
  llvm/utils/gn/secondary/libcxx/include/BUILD.gn

Index: llvm/utils/gn/secondary/libcxx/include/BUILD.gn
===
--- llvm/utils/gn/secondary/libcxx/include/BUILD.gn
+++ llvm/utils/gn/secondary/libcxx/include/BUILD.gn
@@ -19,7 +19,6 @@
   "_LIBCPP_ABI_FORCE_ITANIUM=",
   "_LIBCPP_ABI_FORCE_MICROSOFT=",
   "_LIBCPP_EXTRA_SITE_DEFINES=",
-  "_LIBCPP_HIDE_FROM_ABI_PER_TU_BY_DEFAULT=",
   "_LIBCPP_HAS_NO_FILESYSTEM_LIBRARY=",
   "_LIBCPP_HAS_NO_INCOMPLETE_FORMAT=",
   "_LIBCPP_HAS_NO_INCOMPLETE_RANGES=",
Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -387,7 +387,8 @@
 endif
 
 ifeq (1,$(USE_LIBCPP))
-	CXXFLAGS += -DLLDB_USING_LIBCPP
+	# TODO: Teach LLDB to handle ABI tags in libc++ namespaces.
+	CXXFLAGS += -DLLDB_USING_LIBCPP -D_LIBCPP_NO_ABI_TAG
 	ifeq "$(OS)" "Android"
 		# Nothing to do, this is already handled in
 		# Android.rules.
Index: libcxx/test/libcxx/strings/basic.string/PR42676.sh.cpp
===
--- libcxx/test/libcxx/strings/basic.string/PR42676.sh.cpp
+++ /dev/null
@@ -1,22 +0,0 @@
-//===--===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-// Regression test for PR42676.
-
-// RUN: %{cxx} %{flags} %s -o %t.exe %{compile_flags} %{link_flags} -D_LIBCPP_HIDE_FROM_ABI_PER_TU
-// RUN: %{run}
-
-#include 
-#include 
-
-int main(int, char**) {
-std::string s1(10u, '-', std::allocator()); (void)s1;
-std::string s2("hello", std::allocator()); (void)s2;
-std::string s3("hello"); (void)s3;
-return 0;
-}
Index: libcxx/include/__config_site.in
===
--- libcxx/include/__config_site.in
+++ libcxx/include/__config_site.in
@@ -13,7 +13,6 @@
 #cmakedefine _LIBCPP_ABI_NAMESPACE @_LIBCPP_ABI_NAMESPACE@
 #cmakedefine _LIBCPP_ABI_FORCE_ITANIUM
 #cmakedefine _LIBCPP_ABI_FORCE_MICROSOFT
-#cmakedefine _LIBCPP_HIDE_FROM_ABI_PER_TU_BY_DEFAULT
 #cmakedefine _LIBCPP_HAS_NO_THREADS
 #cmakedefine _LIBCPP_HAS_NO_MONOTONIC_CLOCK
 #cmakedefine _LIBCPP_HAS_MUSL_LIBC
Index: libcxx/include/__config
===
--- libcxx/include/__config
+++ libcxx/include/__config
@@ -26,6 +26,13 @@
 
 #  define _LIBCPP_VERSION 15000
 
+#  define _LIBCPP_CONCAT_IMPL(_X, _Y) _X##_Y
+#  define _LIBCPP_CONCAT(_X, _Y) _LIBCPP_CONCAT_IMPL(_X, _Y)
+
+// Valid C++ identifier that revs with every libc++ version. This can be used to
+// generate identifiers that must be unique for every released libc++ version.
+#  define _LIBCPP_VERSIONED_IDENTIFIER _LIBCPP_CONCAT(v, _LIBCPP_VERSION)
+
 #  if __STDC_HOSTED__ == 0
 #define _LIBCPP_FREESTANDING
 #  endif
@@ -568,12 +575,6 @@
 
 #  endif // defined(_LIBCPP_OBJECT_FORMAT_COFF)
 
-#  if __has_attribute(internal_linkage)
-#define _LIBCPP_INTERNAL_LINKAGE __attribute__((internal_linkage))
-#  else
-#define _LIBCPP_INTERNAL_LINKAGE _LIBCPP_ALWAYS_INLINE
-#  endif
-
 #  if __has_attribute(exclude_from_explicit_instantiation)
 #define _LIBCPP_EXCLUDE_FROM_EXPLICIT_INSTANTIATION __attribute__((__exclude_from_explicit_instantiation__))
 #  else
@@ -583,20 +584,35 @@
 #define _LIBCPP_EXCLUDE_FROM_EXPLICIT_INSTANTIATION _LIBCPP_ALWAYS_INLINE
 #  endif
 
-#  ifndef _LIBCPP_HIDE_FROM_ABI_PER_TU
-#ifndef _LIBCPP_HIDE_FROM_ABI_PER_TU_BY_DEFAULT
-#  define _LIBCPP_HIDE_FROM_ABI_PER_TU 0
-#else
-#  define _LIBCPP_HIDE_FROM_ABI_PER_TU 1
-#endif
-#  endif
-
-#  ifndef _LIBCPP_HIDE_FROM_ABI
-#if _LIBCPP_HIDE_FROM_ABI_PER_TU
-#  define _LIBCPP_HIDE_FROM_ABI _LIBCPP_HIDDEN _LIBCPP_INTERNAL_LINKAGE
-#else
-#  define _LIBCPP_HIDE_FROM_ABI _LIBCPP_HIDDEN 

[Lldb-commits] [PATCH] D127444: [libc++] Use ABI tags instead of internal linkage to provide per-TU insulation

[Louis Dionne via Phabricator via lldb-commits]

ldionne updated this revision to Diff 443066.
ldionne added a comment.
Herald added projects: LLDB, LLVM.
Herald added subscribers: llvm-commits, lldb-commits.

Band-aid for the LLDB issue, and also include 6148c79a 
 in the 
commit.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127444/new/

https://reviews.llvm.org/D127444

Files:
  libcxx/CMakeLists.txt
  libcxx/cmake/caches/Apple.cmake
  libcxx/docs/BuildingLibcxx.rst
  libcxx/docs/DesignDocs/VisibilityMacros.rst
  libcxx/docs/ReleaseNotes.rst
  libcxx/include/__config
  libcxx/include/__config_site.in
  libcxx/test/libcxx/strings/basic.string/PR42676.sh.cpp
  lldb/packages/Python/lldbsuite/test/make/Makefile.rules
  llvm/utils/gn/secondary/libcxx/include/BUILD.gn

Index: llvm/utils/gn/secondary/libcxx/include/BUILD.gn
===
--- llvm/utils/gn/secondary/libcxx/include/BUILD.gn
+++ llvm/utils/gn/secondary/libcxx/include/BUILD.gn
@@ -19,7 +19,6 @@
   "_LIBCPP_ABI_FORCE_ITANIUM=",
   "_LIBCPP_ABI_FORCE_MICROSOFT=",
   "_LIBCPP_EXTRA_SITE_DEFINES=",
-  "_LIBCPP_HIDE_FROM_ABI_PER_TU_BY_DEFAULT=",
   "_LIBCPP_HAS_NO_FILESYSTEM_LIBRARY=",
   "_LIBCPP_HAS_NO_INCOMPLETE_FORMAT=",
   "_LIBCPP_HAS_NO_INCOMPLETE_RANGES=",
Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -387,7 +387,8 @@
 endif
 
 ifeq (1,$(USE_LIBCPP))
-	CXXFLAGS += -DLLDB_USING_LIBCPP
+	# TODO: Teach LLDB to handle ABI tags in libc++ namespaces.
+	CXXFLAGS += -DLLDB_USING_LIBCPP -D_LIBCPP_NO_ABI_TAG
 	ifeq "$(OS)" "Android"
 		# Nothing to do, this is already handled in
 		# Android.rules.
Index: libcxx/test/libcxx/strings/basic.string/PR42676.sh.cpp
===
--- libcxx/test/libcxx/strings/basic.string/PR42676.sh.cpp
+++ /dev/null
@@ -1,22 +0,0 @@
-//===--===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-// Regression test for PR42676.
-
-// RUN: %{cxx} %{flags} %s -o %t.exe %{compile_flags} %{link_flags} -D_LIBCPP_HIDE_FROM_ABI_PER_TU
-// RUN: %{run}
-
-#include 
-#include 
-
-int main(int, char**) {
-std::string s1(10u, '-', std::allocator()); (void)s1;
-std::string s2("hello", std::allocator()); (void)s2;
-std::string s3("hello"); (void)s3;
-return 0;
-}
Index: libcxx/include/__config_site.in
===
--- libcxx/include/__config_site.in
+++ libcxx/include/__config_site.in
@@ -13,7 +13,6 @@
 #cmakedefine _LIBCPP_ABI_NAMESPACE @_LIBCPP_ABI_NAMESPACE@
 #cmakedefine _LIBCPP_ABI_FORCE_ITANIUM
 #cmakedefine _LIBCPP_ABI_FORCE_MICROSOFT
-#cmakedefine _LIBCPP_HIDE_FROM_ABI_PER_TU_BY_DEFAULT
 #cmakedefine _LIBCPP_HAS_NO_THREADS
 #cmakedefine _LIBCPP_HAS_NO_MONOTONIC_CLOCK
 #cmakedefine _LIBCPP_HAS_MUSL_LIBC
Index: libcxx/include/__config
===
--- libcxx/include/__config
+++ libcxx/include/__config
@@ -26,6 +26,13 @@
 
 #  define _LIBCPP_VERSION 15000
 
+#  define _LIBCPP_CONCAT_IMPL(_X, _Y) _X##_Y
+#  define _LIBCPP_CONCAT(_X, _Y) _LIBCPP_CONCAT_IMPL(_X, _Y)
+
+// Valid C++ identifier that revs with every libc++ version. This can be used to
+// generate identifiers that must be unique for every released libc++ version.
+#  define _LIBCPP_VERSIONED_IDENTIFIER _LIBCPP_CONCAT(v, _LIBCPP_VERSION)
+
 #  if __STDC_HOSTED__ == 0
 #define _LIBCPP_FREESTANDING
 #  endif
@@ -568,12 +575,6 @@
 
 #  endif // defined(_LIBCPP_OBJECT_FORMAT_COFF)
 
-#  if __has_attribute(internal_linkage)
-#define _LIBCPP_INTERNAL_LINKAGE __attribute__((internal_linkage))
-#  else
-#define _LIBCPP_INTERNAL_LINKAGE _LIBCPP_ALWAYS_INLINE
-#  endif
-
 #  if __has_attribute(exclude_from_explicit_instantiation)
 #define _LIBCPP_EXCLUDE_FROM_EXPLICIT_INSTANTIATION __attribute__((__exclude_from_explicit_instantiation__))
 #  else
@@ -583,20 +584,35 @@
 #define _LIBCPP_EXCLUDE_FROM_EXPLICIT_INSTANTIATION _LIBCPP_ALWAYS_INLINE
 #  endif
 
-#  ifndef _LIBCPP_HIDE_FROM_ABI_PER_TU
-#ifndef _LIBCPP_HIDE_FROM_ABI_PER_TU_BY_DEFAULT
-#  define _LIBCPP_HIDE_FROM_ABI_PER_TU 0
-#else
-#  define _LIBCPP_HIDE_FROM_ABI_PER_TU 1
-#endif
-#  endif
-
-#  ifndef _LIBCPP_HIDE_FROM_ABI
-#if _LIBCPP_HIDE_FROM_ABI_PER_TU
-#  define _LIBCPP_HIDE_FROM_ABI _LIBCPP_HIDDEN _LIBCPP_INTERNAL_LINKAGE
-#else

[Lldb-commits] [PATCH] D128694: [lldb/Dataformatters] Adapt C++ std::string dataformatter for D128285

[Louis Dionne via Phabricator via lldb-commits]

ldionne added inline comments.



Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:637
 if (location_sp->GetName() == g_size_name)
-  location_sp = short_sp->GetChildAtIndex(3, true);
+  location_sp = short_sp->GetChildAtIndex(2, true);
 if (using_bitmasks)

mib wrote:
> mib wrote:
> > aprantl wrote:
> > > Let me know if I',m misunderstanding what the code is doing, but this 
> > > looks like it is replacing the previous implementation? Ideally we would 
> > > detect the layout and then parse it correctly depending on which version 
> > > we're dealing with. Otherwise we risk breaking the matrix bot that checks 
> > > that LLDB can debug C++ produced by older versions of LLVM (and by 
> > > extension libcxx).
> > I've look at D12828 and D123580, and I don't see any way of versioning 
> > these changes ... may be @ldionne have an idea on how we could do this 
> > properly ?
> > 
> > Also, in D124113, @labath mentions that this data formatter uses mostly 
> > indices to parse and access the various fields of the type data structure 
> > (because it uses some anonymous structs). This makes it very fragile on our 
> > end because our data formatter break every time they make a change in the 
> > layout ...
> > 
> > @aprantl, I'll update the line your pointed at to the the field identifier 
> > instead of using changing the index while waiting for a better way to 
> > version this.
> @aprantl, I'll update the line you pointed at to *use* the field identifier 
> instead of using changing the index, while waiting for a better way to 
> version this.
I don't see a way to version this. You don't have access to the value of macros 
that were defined when the executable was compiled, right? If you did, you 
could check `_LIBCPP_VERSION` (1400 = old implementation, 1500 = current 
implementation). I'm very naive when it comes to debuggers but I assume that's 
not possible.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128694/new/

https://reviews.llvm.org/D128694

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D123580: [libc++] Use bit field for checking if string is in long or short mode

[Louis Dionne via Phabricator via lldb-commits]

ldionne added inline comments.



Comment at: libcxx/utils/gdb/libcxx/printers.py:192
 
 class StdStringPrinter(object):
 """Print a std::string."""

philnik wrote:
> labath wrote:
> > philnik wrote:
> > > dblaikie wrote:
> > > > philnik wrote:
> > > > > jgorbe wrote:
> > > > > > Mordante wrote:
> > > > > > > philnik wrote:
> > > > > > > > Mordante wrote:
> > > > > > > > > philnik wrote:
> > > > > > > > > > Mordante wrote:
> > > > > > > > > > > Does this also break the LLDB pretty printer?
> > > > > > > > > > Probably. Would be nice to have a test runner for that.
> > > > > > > > > I already planned to look into that, D97044#3440904 ;-)
> > > > > > > > Do you know where I would have to look to know what the LLDB 
> > > > > > > > pretty printers do?
> > > > > > > Unfortunately no. @jingham seems to be the Data formatter code 
> > > > > > > owner.
> > > > > > There was a recent lldb change fixing prettyprinters after a 
> > > > > > similar change to string: 
> > > > > > https://github.com/llvm/llvm-project/commit/45428412fd7c9900d3d6ac9803aa7dcf6adfa6fe
> > > > > > 
> > > > > > If the gdb prettyprinter needed fixing for this change, chances are 
> > > > > > that lldb will need a similar update too.
> > > > > Could someone from #lldb help me figure out what to change in the 
> > > > > pretty printers? I looked at the file, but I don't really understand 
> > > > > how it works and TBH I don't really feel like spending a lot of time 
> > > > > figuring it out. If nobody says anything I'll land this in a week.
> > > > > 
> > > > > As a side note: it would be really nice if there were a few more 
> > > > > comments inside `LibCxx.cpp` to explain what happens there. That 
> > > > > would make fixing the pretty printer a lot easier. The code is 
> > > > > probably not very hard (at least it doesn't look like it), but I am 
> > > > > looking for a few things that I can't find and I have no idea what 
> > > > > some of the things mean.
> > > > Looks like something around 
> > > > https://github.com/llvm/llvm-project/blob/2e6ac54cf48aa04f7b05c382c33135b16d3f01ea/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp#L597
> > > >  (& the similar masking in the `else` block a few lines down) - I guess 
> > > > a specific lookup for the new field would be needed, rather than the 
> > > > bitmasking.
> > > Yes, but what do the numbers in `size_mode_locations` mean? Why is there 
> > > no checking if it's big or little endian? Is it unsupported maybe? Does 
> > > it work because of something else? Is there a reason that `g_data_name` 
> > > exists instead of comparing directly? Should I add another layout style 
> > > or should I just update the code for the new layout?
> > > I don't know anything about the LLDB codebase, so I don't understand the 
> > > code and I don't know how I should change it.
> > I don't think there's been any official policy decision either way, but 
> > historically we haven't been asking libc++ authors to update lldb pretty 
> > printers -- we would just fix them up on the lldb side when we noticed the 
> > change. The thing that has changed recently is that google started relying 
> > (and testing) more on lldb, which considerably shortened the time it takes 
> > to notice this change, and also makes it difficult for some people to make 
> > progress while we are in this state. But I don't think that means that 
> > updating the pretty printer is suddenly your responsibility.
> > 
> > As for your questions, I'll try to answer them as best as I can:
> > > what do the numbers in size_mode_locations mean?
> > These are the indexes of fields in the string object. For some reason 
> > (unknown to me), the pretty printer uses indexes rather than field names 
> > for its work. Prompted by the previous patch, I've been trying to change 
> > that, but I haven't done it yet, as I was trying to improve the testing 
> > story (more on that later).
> > > Why is there no checking if it's big or little endian? Is it unsupported 
> > > maybe?
> > Most likely yes. Although most parts of lldb support big endian, I am not 
> > aware of anyone testing it on a regular basis, so it's quite likely that a 
> > lot of things are in fact broken.
> > > Is there a reason that g_data_name exists instead of comparing directly?
> > LLDB uses a global string pool, so this is an attempt to reduce the number 
> > of string pool queries. The pattern is not consistently used everywhere, 
> > and overall, I wouldn't be too worried about it.
> > > Should I add another layout style or should I just update the code for 
> > > the new layout?
> > As the pretty printers ship with lldb, they are expected to support not 
> > just the current format, but also the past ones (within reason). This is 
> > what makes adding a new format (or just refactoring the existing code) 
> > difficult, and it's why I was trying to come up with better tests for this 
> > (it remains to be seen if I am successful).
> > 
> > 

[Lldb-commits] [PATCH] D121078: Replace links to archived mailing lists by links to Discourse forums

[Louis Dionne via Phabricator via lldb-commits]

ldionne accepted this revision.
ldionne added a comment.

LGTM, thanks for fixing the documentation quirks in libc++/libunwind! And sorry 
this went under my Radar last week :-).

@tonic Is this OK with you? I don't want to override your "request for changes" 
by committing this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121078/new/

https://reviews.llvm.org/D121078

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D121078: Replace links to archived mailing lists by links to Discourse forums

[Louis Dionne via Phabricator via lldb-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

Thanks for doing this! We need to fix a few undefined references, though.




Comment at: libcxx/docs/index.rst:223
 * `libcxx-commits Mailing List`_
 * `libcxx-dev Mailing List`_
 * `Browse libc++ Sources 
`_

I think this needs to be updated!



Comment at: libunwind/docs/index.rst:103
 * `cfe-commits Mailing List`_
 * `cfe-dev Mailing List`_
 * `Browse libunwind Sources 
`_

Probably this one too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121078/new/

https://reviews.llvm.org/D121078

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D119918: [CMake] Rename TARGET_TRIPLE to LLVM_TARGET_TRIPLE

[Louis Dionne via Phabricator via lldb-commits]

ldionne accepted this revision.
ldionne added a comment.

LGTM from libc++/libc++abi's perspective.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119918/new/

https://reviews.llvm.org/D119918

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D116224: Revert "[amdgpu] Enable selection of `s_cselect_b64`."

[Louis Dionne via Phabricator via lldb-commits]

ldionne commandeered this revision.
ldionne added a reviewer: david-salinas.
ldionne added a comment.

Yeah, I think you messed something up with your git commands. I'm going to 
commandeer and abandon this revision to get it out of the libc++ review queue 
since it's been 10 days since this has been open. Please re-open another review 
reverting just the intended commit.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116224/new/

https://reviews.llvm.org/D116224

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D116351: Update Bug report URL to Github Issues

[Louis Dionne via Phabricator via lldb-commits]

ldionne accepted this revision as: libc++, libunwind.
ldionne added a comment.

Libc++ and libunwind changes LGTM


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116351/new/

https://reviews.llvm.org/D116351

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D115566: Quote some more destination paths with variables

[Louis Dionne via Phabricator via lldb-commits]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

This looks reasonable to me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115566/new/

https://reviews.llvm.org/D115566

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D106339: Add support to generate Sphinx DOCX documentation

[Louis Dionne via Phabricator via lldb-commits]

ldionne removed projects: libc++, libunwind.
ldionne removed reviewers: libunwind, libc++, ldionne.
ldionne added a comment.
Herald added projects: libc++, libunwind.
Herald added a reviewer: libc++.
Herald added a reviewer: libunwind.

Removing from the runtimes review queue -- please ping me directly if you need 
my attention on this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106339/new/

https://reviews.llvm.org/D106339

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D110216: [clang] retain type sugar in auto / template argument deduction

[Louis Dionne via Phabricator via lldb-commits]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

LGTM from the libc++ perspective.

Unsubscribing to reduce spam, please ping me on Discord if you need further 
input. Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110216/new/

https://reviews.llvm.org/D110216

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D106339: Add support to generate Sphinx DOCX documentation

[Louis Dionne via Phabricator via lldb-commits]

ldionne added a comment.
Herald added a project: Flang.

If there is no plan to move forward with generating `.docx` documentation, can 
we please abandon this review? I'm trying to clean up libc++'s review queue.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106339/new/

https://reviews.llvm.org/D106339

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D107717: [LLVM][CMake][NFC] Resolve FIXME: Rename LLVM_CMAKE_PATH to LLVM_CMAKE_DIR throughout the project

[Louis Dionne via Phabricator via lldb-commits]

ldionne accepted this revision.
ldionne added a comment.

This LGTM.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107717/new/

https://reviews.llvm.org/D107717

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D106339: Add support to generate Sphinx DOCX documentation

[Louis Dionne via Phabricator via lldb-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

What's the benefit of having docx documentation? We generate HTML 
documentation, which ends up in the website, and that seems strictly superior 
to generating docx. What do you need it for?

The libc++ changes are almost trivial so I would not object to the change on 
that basis, however in general I think it's better to avoid adding support for 
things we won't be using on a regular basis.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106339/new/

https://reviews.llvm.org/D106339

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

[Louis Dionne via Phabricator via lldb-commits]

ldionne added a comment.

In D100581#2793926 , @dblaikie wrote:

> In D100581#2793775 , @ldionne wrote:
>
>> Hello! There are still some false positives, for example this one is 
>> breaking the libc++ CI: 
>> https://buildkite.com/llvm-project/libcxx-ci/builds/3530#8679608a-200b-4a48-beb4-a9e9924a8848
>>
>> Would it be possible to either fix this quickly or revert the change until 
>> the issue has been fixed? Our pre-commit CI is going to be stalled until 
>> this is fixed. Thanks!
>
> Looks like a true positive in libc++ ( 
> https://github.com/llvm/llvm-project/blob/main/libcxx/include/regex ) - the 
> "__j" variable is initialized, then incremented, but never read (except to 
> increment). Is that a bug in libc++? Was __j meant to be used for something?

Oh, you're right! I was tripped by the fact that we did in fact "use" `__j` (in 
the sense that we do `__j += ...`), but indeed we never read the result. I'll 
work on fixing that issue. I'm not sure whether it's a bug or not yet, that 
code was modified 11 years ago, but I'll do my research.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100581/new/

https://reviews.llvm.org/D100581

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

[Louis Dionne via Phabricator via lldb-commits]

ldionne added a comment.

Hello! There are still some false positives, for example this one is breaking 
the libc++ CI: 
https://buildkite.com/llvm-project/libcxx-ci/builds/3530#8679608a-200b-4a48-beb4-a9e9924a8848

Would it be possible to either fix this quickly or revert the change until the 
issue has been fixed? Our pre-commit CI is going to be stalled until this is 
fixed. Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100581/new/

https://reviews.llvm.org/D100581

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

[Louis Dionne via Phabricator via lldb-commits]

ldionne accepted this revision as: libc++, libc++abi.
ldionne added a comment.

LGTM for libcxx and libcxxabi.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99484/new/

https://reviews.llvm.org/D99484

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

[Louis Dionne via Phabricator via lldb-commits]

ldionne added inline comments.



Comment at: libcxx/cmake/Modules/HandleLibCXXABI.cmake:66
   install(FILES "${LIBCXX_BINARY_INCLUDE_DIR}/${fpath}"
-DESTINATION ${LIBCXX_INSTALL_HEADER_PREFIX}include/c++/v1/${dstdir}
+DESTINATION 
${LIBCXX_INSTALL_HEADER_PREFIX}${CMAKE_INSTALL_INCLUDEDIR}/c++/v1/${dstdir}
 COMPONENT cxx-headers

Ericson2314 wrote:
> ldionne wrote:
> > compnerd wrote:
> > > Ericson2314 wrote:
> > > > compnerd wrote:
> > > > > @ldionne - how is the `LIBCXX_INSTALL_HEADER_PREFIX` used?  Can 
> > > > > altering the value of `CMAKE_INSTALL_INCLUDEDIR` serve the purpose?
> > > > It is sometimes modified to be per target when multiple targets are 
> > > > being used at once. All things `CMAKE_INSTALL_*` are globally scoped so 
> > > > in general the combination builds are quite awkward.
> > > > 
> > > > (Having worked on Meson, I am really missing 
> > > > https://mesonbuild.com/Subprojects.html which is exactly what's needed 
> > > > to do this without these bespoke idioms that never work well enough . 
> > > > Alas...)
> > > I don't think that bringing up other build systems is particularly 
> > > helpful.
> > > 
> > > I do expect it to be modified, and I suspect that this is used 
> > > specifically for builds that @ldionne supports.
> > Actually, I've never used it myself, but @phosek seems to be using it for 
> > the Runtimes build to output one set of headers for each target, as 
> > mentioned above.
> > 
> > It seems to me that tweaking `CMAKE_INSTALL_PREFIX` globally when driving 
> > the libc++ build from the runtimes build would be more in-line with the 
> > CMake way of doing things (one configuration == one build), but I'm curious 
> > to hear what @phosek has to say about that.
> > It seems to me that tweaking CMAKE_INSTALL_PREFIX globally when driving the 
> > libc++ build from the runtimes build would be more in-line with the CMake 
> > way of doing things (one configuration == one buid)
> 
> You mean trying to mutate it during the libc++ CMake eval and then set it 
> back after? That would keep the intended meaning of  `CMAKE_INSTALL_PREFIX` 
> being the actual prefix, but that feels awfully fragile to me. Or do you mean 
> something else?
I keep forgetting that the runtimes build uses `add_subdirectory` to include 
each sub-project instead of driving a proper CMake build for each of those.

Basically, what I'm saying is that whatever place we decide to build for N 
multiple targets, we should perform N different CMake builds setting the 
appropriate `CMAKE_INSTALL_PREFIX`, one for each target. But that discussion 
should happen elsewhere, not on this review.

As far as this review is concerned, I do believe you want instead:

```
${CMAKE_INSTALL_INCLUDEDIR}${LIBCXX_INSTALL_HEADER_PREFIX}
```

(reversed the order of variables)

We should have @phosek confirm.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99484/new/

https://reviews.llvm.org/D99484

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

[Louis Dionne via Phabricator via lldb-commits]

ldionne added a subscriber: phosek.
ldionne added a comment.

I am generally OK with the libcxx and libcxxabi changes.




Comment at: compiler-rt/cmake/Modules/CompilerRTUtils.cmake:389
 get_compiler_rt_target(${arch} target)
-set(${install_dir} ${COMPILER_RT_INSTALL_PATH}/lib/${target} PARENT_SCOPE)
+set(${install_dir} 
${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_FULL_LIBDIR}/${target} PARENT_SCOPE)
   else()

lebedev.ri wrote:
> This looks suspect
Yeah, I don't understand why this isn't just `CMAKE_INSTALL_LIBDIR` like 
elsewhere.



Comment at: libcxx/cmake/Modules/HandleLibCXXABI.cmake:66
   install(FILES "${LIBCXX_BINARY_INCLUDE_DIR}/${fpath}"
-DESTINATION ${LIBCXX_INSTALL_HEADER_PREFIX}include/c++/v1/${dstdir}
+DESTINATION 
${LIBCXX_INSTALL_HEADER_PREFIX}${CMAKE_INSTALL_INCLUDEDIR}/c++/v1/${dstdir}
 COMPONENT cxx-headers

compnerd wrote:
> Ericson2314 wrote:
> > compnerd wrote:
> > > @ldionne - how is the `LIBCXX_INSTALL_HEADER_PREFIX` used?  Can altering 
> > > the value of `CMAKE_INSTALL_INCLUDEDIR` serve the purpose?
> > It is sometimes modified to be per target when multiple targets are being 
> > used at once. All things `CMAKE_INSTALL_*` are globally scoped so in 
> > general the combination builds are quite awkward.
> > 
> > (Having worked on Meson, I am really missing 
> > https://mesonbuild.com/Subprojects.html which is exactly what's needed to 
> > do this without these bespoke idioms that never work well enough . Alas...)
> I don't think that bringing up other build systems is particularly helpful.
> 
> I do expect it to be modified, and I suspect that this is used specifically 
> for builds that @ldionne supports.
Actually, I've never used it myself, but @phosek seems to be using it for the 
Runtimes build to output one set of headers for each target, as mentioned above.

It seems to me that tweaking `CMAKE_INSTALL_PREFIX` globally when driving the 
libc++ build from the runtimes build would be more in-line with the CMake way 
of doing things (one configuration == one build), but I'm curious to hear what 
@phosek has to say about that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99484/new/

https://reviews.llvm.org/D99484

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Louis Dionne via Phabricator via lldb-commits]

ldionne accepted this revision.
ldionne 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/D94374/new/

https://reviews.llvm.org/D94374

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Louis Dionne via Phabricator via lldb-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

Except for the google-benchmark nit, LGTM. Thanks a lot for the cleanup!




Comment at: libcxx/utils/google-benchmark/CMakeLists.txt:3
-
-project (benchmark)
-

I don't think we want to change this. It's a third-party project (which is 
inconveniently checked-in as-is into our tree..).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94374/new/

https://reviews.llvm.org/D94374

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D89813: [CMake][NFC] Limit the use of uppercase_CMAKE_BUILD_TYPE

[Louis Dionne via Phabricator via lldb-commits]

ldionne added a comment.

Are generator expressions expanded in `if()`? If so, we could at least change 
those to `if ("$" STREQUAL "DEBUG")`. This would 
preserve the case-insensitivity while solving the problem you were concerned 
about, and make the code easier to read.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89813/new/

https://reviews.llvm.org/D89813

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D89813: [CMake][NFC] Limit the use of uppercase_CMAKE_BUILD_TYPE

[Louis Dionne via Phabricator via lldb-commits]

ldionne added a comment.

I quite like this. We should strive to use the same name as CMake uses for its 
build types, i.e. `Debug`, `Release`, etc.

If I configure CMake with `-DCMAKE_BUILD_TYPE=DEBUG`, is `CMAKE_BUILD_TYPE` 
normalized back to `Debug` by CMake, or is it defined to `DEBUG`? If the 
latter, then I suggest we might want to add a temporary error message when the 
`CMAKE_BUILD_TYPE` is defined to something all-uppercase. This would help catch 
cases where this patch will be a change in behavior. That would be a nice place 
to tell users to use the right build type names.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89813/new/

https://reviews.llvm.org/D89813

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D87051: scan-build-py: fix multiprocessing error

[Louis Dionne via Phabricator via lldb-commits]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

From the docs:

> `multiprocessing.freeze_support()`
> Add support for when a program which uses multiprocessing has been frozen to 
> produce a Windows executable. (Has been tested with py2exe, PyInstaller and 
> cx_Freeze.)
>
> One needs to call this function straight after the `if __name__ == 
> '__main__'` line of the main module. For example: [...]

So this looks right to me, but I'm not a Clang owner.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87051/new/

https://reviews.llvm.org/D87051

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D86616: [cmake] Make gtest include directories a part of the library interface

[Louis Dionne via Phabricator via lldb-commits]

ldionne added a comment.

In D86616#2241936 , @labath wrote:

> In D86616#2238946 , @ldionne wrote:
>
>> LGTM, but I'm not an owner for any of the projects touched by this change.
>
> I picked you because you seemed interested in the overall direction that our 
> cmake support is going :), and I believe that one of our biggest problem with 
> cmake is the lack of a unified direction of where our cmake support is going. 
> (Also, my previous go-to person for that (@beanz), seems to be busy with 
> other stuff these days.)

Yup, I am interested by that indeed :). I just wanted to make it clear my LGTM 
alone shouldn't be considered enough to commit to these other projects that 
might have active owners. But it does look like you've got enough approvals for 
an obviously correct improvement, IMO.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86616/new/

https://reviews.llvm.org/D86616

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D86616: [cmake] Make gtest include directories a part of the library interface

[Louis Dionne via Phabricator via lldb-commits]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

LGTM, but I'm not an owner for any of the projects touched by this change.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86616/new/

https://reviews.llvm.org/D86616

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D84748: [cmake] Make gtest macro definitions a part the library interface

[Louis Dionne via Phabricator via lldb-commits]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

Can you confirm that all the targets that need to "link" (in the CMake sense) 
against gtest use the gtest CMake target?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84748/new/

https://reviews.llvm.org/D84748

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [Differential] D78648: [CMake] Bump CMake minimum version to 3.13.4

[Louis Dionne via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGafa1afd4108d: [CMake] Bump CMake minimum version to 3.13.4 
(authored by ldionne).
Herald added subscribers: llvm-commits, msifontes, sstefan1, jurahul, 
stephenneuendorffer, aartbik.
Herald added projects: MLIR, LLVM.

Changed prior to commit:
  https://reviews.llvm.org/D78648?vs=259300=279101#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78648/new/

https://reviews.llvm.org/D78648

Files:
  clang/CMakeLists.txt
  clang/tools/scan-build-py/tests/functional/exec/CMakeLists.txt
  compiler-rt/CMakeLists.txt
  compiler-rt/cmake/Modules/CustomLibcxx/CMakeLists.txt
  compiler-rt/lib/builtins/CMakeLists.txt
  flang/CMakeLists.txt
  libclc/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxx/utils/ci/runtimes/CMakeLists.txt
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  lld/CMakeLists.txt
  lldb/CMakeLists.txt
  lldb/tools/debugserver/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/docs/CMake.rst
  llvm/docs/CMakePrimer.rst
  llvm/docs/GettingStarted.rst
  llvm/runtimes/CMakeLists.txt
  llvm/utils/benchmark/CMakeLists.txt
  mlir/examples/standalone/CMakeLists.txt
  openmp/CMakeLists.txt
  openmp/cmake/DetectTestCompiler/CMakeLists.txt
  openmp/runtime/cmake/LibompCheckLinkerFlag.cmake
  parallel-libs/CMakeLists.txt
  parallel-libs/acxxel/CMakeLists.txt
  polly/CMakeLists.txt
  pstl/CMakeLists.txt

Index: pstl/CMakeLists.txt
===
--- pstl/CMakeLists.txt
+++ pstl/CMakeLists.txt
@@ -5,7 +5,7 @@
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 #
 #===--===##
-cmake_minimum_required(VERSION 3.4.3)
+cmake_minimum_required(VERSION 3.13.4)
 
 set(PARALLELSTL_VERSION_FILE "${CMAKE_CURRENT_SOURCE_DIR}/include/pstl/internal/pstl_config.h")
 file(STRINGS "${PARALLELSTL_VERSION_FILE}" PARALLELSTL_VERSION_SOURCE REGEX "#define _PSTL_VERSION .*$")
Index: polly/CMakeLists.txt
===
--- polly/CMakeLists.txt
+++ polly/CMakeLists.txt
@@ -1,7 +1,7 @@
 # Check if this is a in tree build.
 if (NOT DEFINED LLVM_MAIN_SRC_DIR)
   project(Polly)
-  cmake_minimum_required(VERSION 3.4.3)
+  cmake_minimum_required(VERSION 3.13.4)
 
   # Where is LLVM installed?
   find_package(LLVM CONFIG REQUIRED)
Index: parallel-libs/acxxel/CMakeLists.txt
===
--- parallel-libs/acxxel/CMakeLists.txt
+++ parallel-libs/acxxel/CMakeLists.txt
@@ -1,4 +1,4 @@
-cmake_minimum_required(VERSION 3.1)
+cmake_minimum_required(VERSION 3.13.4)
 
 option(ACXXEL_ENABLE_UNIT_TESTS "enable acxxel unit tests" ON)
 option(ACXXEL_ENABLE_MULTI_DEVICE_UNIT_TESTS "enable acxxel multi-device unit tests" OFF)
Index: parallel-libs/CMakeLists.txt
===
--- parallel-libs/CMakeLists.txt
+++ parallel-libs/CMakeLists.txt
@@ -1 +1 @@
-cmake_minimum_required(VERSION 3.1)
+cmake_minimum_required(VERSION 3.13.4)
Index: openmp/runtime/cmake/LibompCheckLinkerFlag.cmake
===
--- openmp/runtime/cmake/LibompCheckLinkerFlag.cmake
+++ openmp/runtime/cmake/LibompCheckLinkerFlag.cmake
@@ -17,7 +17,7 @@
   set(library_source
 "int foo(int a) { return a*a; }")
   set(cmake_source
-"cmake_minimum_required(VERSION 2.8)
+"cmake_minimum_required(VERSION 3.13.4)
  project(foo C)
  set(CMAKE_SHARED_LINKER_FLAGS \"${flag}\")
  add_library(foo SHARED src_to_link.c)")
Index: openmp/cmake/DetectTestCompiler/CMakeLists.txt
===
--- openmp/cmake/DetectTestCompiler/CMakeLists.txt
+++ openmp/cmake/DetectTestCompiler/CMakeLists.txt
@@ -1,4 +1,4 @@
-cmake_minimum_required(VERSION 2.8)
+cmake_minimum_required(VERSION 3.13.4)
 project(DetectTestCompiler C CXX)
 
 include(CheckCCompilerFlag)
Index: openmp/CMakeLists.txt
===
--- openmp/CMakeLists.txt
+++ openmp/CMakeLists.txt
@@ -1,4 +1,4 @@
-cmake_minimum_required(VERSION 2.8 FATAL_ERROR)
+cmake_minimum_required(VERSION 3.13.4)
 
 # Add cmake directory to search for custom cmake functions.
 set(CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/cmake ${CMAKE_MODULE_PATH})
Index: mlir/examples/standalone/CMakeLists.txt
===
--- mlir/examples/standalone/CMakeLists.txt
+++ mlir/examples/standalone/CMakeLists.txt
@@ -1,4 +1,4 @@
-cmake_minimum_required(VERSION 3.10)
+cmake_minimum_required(VERSION 3.13.4)
 
 if(POLICY CMP0068)
   cmake_policy(SET CMP0068 NEW)
Index: llvm/utils/benchmark/CMakeLists.txt
===
--- 

[Lldb-commits] [PATCH] D78648: [CMake] Bump CMake minimum version to 3.13.4

[Louis Dionne via Phabricator via lldb-commits]

ldionne added a comment.
Herald added subscribers: llvm-commits, msifontes, sstefan1, jurahul, 
stephenneuendorffer, aartbik.
Herald added projects: MLIR, LLVM.

Okay, the previous patch has landed and no issues have come up, so I'm going to 
move forward with this patch now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78648/new/

https://reviews.llvm.org/D78648



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D78648: [CMake] Bump CMake minimum version to 3.13.4

[Louis Dionne via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGafa1afd4108d: [CMake] Bump CMake minimum version to 3.13.4 
(authored by ldionne).

Changed prior to commit:
  https://reviews.llvm.org/D78648?vs=259300=279895#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78648/new/

https://reviews.llvm.org/D78648

Files:
  clang/CMakeLists.txt
  clang/tools/scan-build-py/tests/functional/exec/CMakeLists.txt
  compiler-rt/CMakeLists.txt
  compiler-rt/cmake/Modules/CustomLibcxx/CMakeLists.txt
  compiler-rt/lib/builtins/CMakeLists.txt
  flang/CMakeLists.txt
  libclc/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxx/utils/ci/runtimes/CMakeLists.txt
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  lld/CMakeLists.txt
  lldb/CMakeLists.txt
  lldb/tools/debugserver/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/docs/CMake.rst
  llvm/docs/CMakePrimer.rst
  llvm/docs/GettingStarted.rst
  llvm/runtimes/CMakeLists.txt
  llvm/utils/benchmark/CMakeLists.txt
  mlir/examples/standalone/CMakeLists.txt
  openmp/CMakeLists.txt
  openmp/cmake/DetectTestCompiler/CMakeLists.txt
  openmp/runtime/cmake/LibompCheckLinkerFlag.cmake
  parallel-libs/CMakeLists.txt
  parallel-libs/acxxel/CMakeLists.txt
  polly/CMakeLists.txt
  pstl/CMakeLists.txt

Index: pstl/CMakeLists.txt
===
--- pstl/CMakeLists.txt
+++ pstl/CMakeLists.txt
@@ -5,7 +5,7 @@
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 #
 #===--===##
-cmake_minimum_required(VERSION 3.4.3)
+cmake_minimum_required(VERSION 3.13.4)
 
 set(PARALLELSTL_VERSION_FILE "${CMAKE_CURRENT_SOURCE_DIR}/include/pstl/internal/pstl_config.h")
 file(STRINGS "${PARALLELSTL_VERSION_FILE}" PARALLELSTL_VERSION_SOURCE REGEX "#define _PSTL_VERSION .*$")
Index: polly/CMakeLists.txt
===
--- polly/CMakeLists.txt
+++ polly/CMakeLists.txt
@@ -1,7 +1,7 @@
 # Check if this is a in tree build.
 if (NOT DEFINED LLVM_MAIN_SRC_DIR)
   project(Polly)
-  cmake_minimum_required(VERSION 3.4.3)
+  cmake_minimum_required(VERSION 3.13.4)
 
   # Where is LLVM installed?
   find_package(LLVM CONFIG REQUIRED)
Index: parallel-libs/acxxel/CMakeLists.txt
===
--- parallel-libs/acxxel/CMakeLists.txt
+++ parallel-libs/acxxel/CMakeLists.txt
@@ -1,4 +1,4 @@
-cmake_minimum_required(VERSION 3.1)
+cmake_minimum_required(VERSION 3.13.4)
 
 option(ACXXEL_ENABLE_UNIT_TESTS "enable acxxel unit tests" ON)
 option(ACXXEL_ENABLE_MULTI_DEVICE_UNIT_TESTS "enable acxxel multi-device unit tests" OFF)
Index: parallel-libs/CMakeLists.txt
===
--- parallel-libs/CMakeLists.txt
+++ parallel-libs/CMakeLists.txt
@@ -1 +1 @@
-cmake_minimum_required(VERSION 3.1)
+cmake_minimum_required(VERSION 3.13.4)
Index: openmp/runtime/cmake/LibompCheckLinkerFlag.cmake
===
--- openmp/runtime/cmake/LibompCheckLinkerFlag.cmake
+++ openmp/runtime/cmake/LibompCheckLinkerFlag.cmake
@@ -17,7 +17,7 @@
   set(library_source
 "int foo(int a) { return a*a; }")
   set(cmake_source
-"cmake_minimum_required(VERSION 2.8)
+"cmake_minimum_required(VERSION 3.13.4)
  project(foo C)
  set(CMAKE_SHARED_LINKER_FLAGS \"${flag}\")
  add_library(foo SHARED src_to_link.c)")
Index: openmp/cmake/DetectTestCompiler/CMakeLists.txt
===
--- openmp/cmake/DetectTestCompiler/CMakeLists.txt
+++ openmp/cmake/DetectTestCompiler/CMakeLists.txt
@@ -1,4 +1,4 @@
-cmake_minimum_required(VERSION 2.8)
+cmake_minimum_required(VERSION 3.13.4)
 project(DetectTestCompiler C CXX)
 
 include(CheckCCompilerFlag)
Index: openmp/CMakeLists.txt
===
--- openmp/CMakeLists.txt
+++ openmp/CMakeLists.txt
@@ -1,4 +1,4 @@
-cmake_minimum_required(VERSION 2.8 FATAL_ERROR)
+cmake_minimum_required(VERSION 3.13.4)
 
 # Add cmake directory to search for custom cmake functions.
 set(CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/cmake ${CMAKE_MODULE_PATH})
Index: mlir/examples/standalone/CMakeLists.txt
===
--- mlir/examples/standalone/CMakeLists.txt
+++ mlir/examples/standalone/CMakeLists.txt
@@ -1,4 +1,4 @@
-cmake_minimum_required(VERSION 3.10)
+cmake_minimum_required(VERSION 3.13.4)
 
 if(POLICY CMP0068)
   cmake_policy(SET CMP0068 NEW)
Index: llvm/utils/benchmark/CMakeLists.txt
===
--- llvm/utils/benchmark/CMakeLists.txt
+++ llvm/utils/benchmark/CMakeLists.txt
@@ -1,4 +1,4 @@
-cmake_minimum_required (VERSION 2.8.12)
+cmake_minimum_required(VERSION 3.13.4)
 
 # Tell cmake 3.0+ that it's 

[Lldb-commits] [PATCH] D78648: [CMake] Bump CMake minimum version to 3.13.4

[Louis Dionne via Phabricator via lldb-commits]

ldionne created this revision.
Herald added subscribers: libcxx-commits, openmp-commits, lldb-commits, 
Sanitizers, cfe-commits, frgossen, grosul1, jvesely, Joonsoo, liufengdb, 
lucyrfox, mgester, arpith-jacob, nicolasvasilache, antiagainst, shauheen, 
jpienaar, rriddle, mehdi_amini, lebedev.ri, dexonsmith, jkorous, whisperity, 
mgorny.
Herald added a reviewer: bollu.
Herald added a reviewer: lebedev.ri.
Herald added a reviewer: DavidTruby.
Herald added projects: clang, Sanitizers, LLDB, libc++, OpenMP, libc++abi, 
libunwind.
Herald added a reviewer: libc++.
Herald added a reviewer: libc++abi.
Herald added a reviewer: libunwind.
ldionne added a parent revision: D78646: [CMake] Enforce the minimum CMake 
version to be at least 3.13.4.
Herald added a reviewer: jdoerfert.
ldionne added a comment.
ldionne accepted this revision as: libc++, libc++abi, libunwind.

This review is for the upcoming CMake upgrade once we branch off the LLVM 11 
release branch.

I won't apply it until after the branch has been created, which is around July.


This upgrade should be friction-less because we've already been ensuring
that CMake >= 3.13.4 is used.

This is part of the effort discussed on llvm-dev here:

  http://lists.llvm.org/pipermail/llvm-dev/2020-April/140578.html


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78648

Files:
  clang/CMakeLists.txt
  clang/tools/scan-build-py/tests/functional/exec/CMakeLists.txt
  compiler-rt/CMakeLists.txt
  compiler-rt/cmake/Modules/CustomLibcxx/CMakeLists.txt
  compiler-rt/lib/builtins/CMakeLists.txt
  flang/CMakeLists.txt
  libclc/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  lld/CMakeLists.txt
  lldb/CMakeLists.txt
  lldb/tools/debugserver/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/docs/CMake.rst
  llvm/docs/CMakePrimer.rst
  llvm/docs/GettingStarted.rst
  llvm/runtimes/CMakeLists.txt
  llvm/utils/benchmark/CMakeLists.txt
  mlir/examples/standalone/CMakeLists.txt
  openmp/CMakeLists.txt
  openmp/cmake/DetectTestCompiler/CMakeLists.txt
  openmp/runtime/cmake/LibompCheckLinkerFlag.cmake
  parallel-libs/CMakeLists.txt
  parallel-libs/acxxel/CMakeLists.txt
  polly/CMakeLists.txt
  pstl/CMakeLists.txt

Index: pstl/CMakeLists.txt
===
--- pstl/CMakeLists.txt
+++ pstl/CMakeLists.txt
@@ -5,7 +5,7 @@
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 #
 #===--===##
-cmake_minimum_required(VERSION 3.4.3)
+cmake_minimum_required(VERSION 3.13.4)
 
 set(PARALLELSTL_VERSION_FILE "${CMAKE_CURRENT_SOURCE_DIR}/include/pstl/internal/pstl_config.h")
 file(STRINGS "${PARALLELSTL_VERSION_FILE}" PARALLELSTL_VERSION_SOURCE REGEX "#define _PSTL_VERSION .*$")
Index: polly/CMakeLists.txt
===
--- polly/CMakeLists.txt
+++ polly/CMakeLists.txt
@@ -1,7 +1,7 @@
 # Check if this is a in tree build.
 if (NOT DEFINED LLVM_MAIN_SRC_DIR)
   project(Polly)
-  cmake_minimum_required(VERSION 3.4.3)
+  cmake_minimum_required(VERSION 3.13.4)
 
   # Where is LLVM installed?
   find_package(LLVM CONFIG REQUIRED)
Index: parallel-libs/acxxel/CMakeLists.txt
===
--- parallel-libs/acxxel/CMakeLists.txt
+++ parallel-libs/acxxel/CMakeLists.txt
@@ -1,4 +1,4 @@
-cmake_minimum_required(VERSION 3.1)
+cmake_minimum_required(VERSION 3.13.4)
 
 option(ACXXEL_ENABLE_UNIT_TESTS "enable acxxel unit tests" ON)
 option(ACXXEL_ENABLE_MULTI_DEVICE_UNIT_TESTS "enable acxxel multi-device unit tests" OFF)
Index: parallel-libs/CMakeLists.txt
===
--- parallel-libs/CMakeLists.txt
+++ parallel-libs/CMakeLists.txt
@@ -1 +1 @@
-cmake_minimum_required(VERSION 3.1)
+cmake_minimum_required(VERSION 3.13.4)
Index: openmp/runtime/cmake/LibompCheckLinkerFlag.cmake
===
--- openmp/runtime/cmake/LibompCheckLinkerFlag.cmake
+++ openmp/runtime/cmake/LibompCheckLinkerFlag.cmake
@@ -17,7 +17,7 @@
   set(library_source
 "int foo(int a) { return a*a; }")
   set(cmake_source
-"cmake_minimum_required(VERSION 2.8)
+"cmake_minimum_required(VERSION 3.13.4)
  project(foo C)
  set(CMAKE_SHARED_LINKER_FLAGS \"${flag}\")
  add_library(foo SHARED src_to_link.c)")
Index: openmp/cmake/DetectTestCompiler/CMakeLists.txt
===
--- openmp/cmake/DetectTestCompiler/CMakeLists.txt
+++ openmp/cmake/DetectTestCompiler/CMakeLists.txt
@@ -1,4 +1,4 @@
-cmake_minimum_required(VERSION 2.8)
+cmake_minimum_required(VERSION 3.13.4)
 project(DetectTestCompiler C CXX)
 
 include(CheckCCompilerFlag)
Index: openmp/CMakeLists.txt
===
--- openmp/CMakeLists.txt
+++ openmp/CMakeLists.txt
@@ -1,4 

[Lldb-commits] [PATCH] D78648: [CMake] Bump CMake minimum version to 3.13.4

[Louis Dionne via Phabricator via lldb-commits]

ldionne added a comment.

This review is for the upcoming CMake upgrade once we branch off the LLVM 11 
release branch.

I won't apply it until after the branch has been created, which is around July.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78648/new/

https://reviews.llvm.org/D78648



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D64395: [CMake] Avoid libcxxabi dependency when building LLDB from the monorepo on macOS by default

[Louis Dionne via Phabricator via lldb-commits]

ldionne added a comment.

In D64395#1575999 , @sgraenitz wrote:

> Thanks for taking a look.
>
> In D64395#1575967 , @ldionne wrote:
>
> > When is `LLDBConfig.cmake` used?
>
>
> Very early in the LLDB config process: 
> https://github.com/llvm/llvm-project/blob/e0a3ee79c5ff/lldb/CMakeLists.txt#L15
>  Globally, I think LLDB is always configured after libc++. Does libc++ make 
> decisions based on these values?


libc++ decides to build or not build `libc++.dylib`/`libc++.a` based on the 
value of `LIBCXX_ENABLE_SHARED` and `LIBCXX_ENABLE_STATIC`, if that's your 
question.

> 
> 
>> Will it influence whether `libc++.dylib` is built in the monorepo whenever 
>> you also happen to build LLDB?
> 
> Yes, I think so. Adding lldb to `LLVM_ENABLE_PROJECTS` will disable 
> `libc++.dylib` globally, ...

In that case, I'm really not a big fan of this approach. I think it's not good 
hygiene for another project to be poking at libc++'s build options like that. 
Those are meant to be set by end-users configuring what they want from libc++ 
only, not set by other projects in the monorepo. To illustrate, I think it's 
highly confusing if adding `-DLLVM_ENABLE_PROJECTS=lldb` suddenly disables the 
building of `libc++.dylib`.

I think your problem might fix itself with https://reviews.llvm.org/D63883. 
What exactly is the problem you're trying to solve?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64395/new/

https://reviews.llvm.org/D64395



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D64395: [CMake] Avoid libcxxabi dependency when building LLDB from the monorepo on macOS by default

[Louis Dionne via Phabricator via lldb-commits]

ldionne added a comment.

When is `LLDBConfig.cmake` used? I'm trying to understand when the 
`LIBCXX_ENABLE_SHARED=OFF` setting will come into play. Will it influence 
whether `libc++.dylib` is built in the monorepo whenever you also happen to 
build LLDB?




Comment at: lldb/cmake/modules/LLDBConfig.cmake:79
+  # if not enabled explicitly in builds from the monorepo.
+  if("libcxx" IN_LIST LLVM_ENABLE_PROJECTS AND NOT "libcxxabi" IN_LIST 
LLVM_ENABLE_PROJECTS)
+set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "" FORCE)

So when both `libcxxabi` and `libcxx` are enabled, we will build `libc++.dylib` 
and `libc++.a`? Why not always disable it?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64395/new/

https://reviews.llvm.org/D64395



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits