[PATCH] D153131: [clang analysis][thread-safety] Handle return-by-reference...

thakis added a comment. FWIW, the standard procedure for adding new functionality to existing warnings is (assuming that it makes the warning fire a lot, else no extra group is needed): - Add it in a subgroup with its own flag - Enable it by default The reasoning is that people who aren't read

[PATCH] D158069: [clang][Interp] Fix getIndex() for composite array elements

thakis added inline comments. Comment at: clang/unittests/AST/Interp/CMakeLists.txt:2 +add_clang_unittest(InterpTests + Descriptor.cpp + ) Why is this in a separate executable instead of in ASTTests? So it can have fewer deps? Repository: rG LLVM Github Mo

[PATCH] D155775: [HIP][Clang][Driver][RFC] Add driver support for C++ Parallel Algorithm Offload

thakis added a comment. Windows: http://45.33.8.238/win/84498/step_7.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155775/new/ https://reviews.llvm.org/D155775 ___ cfe-commits mailing list cfe-commit

[PATCH] D155775: [HIP][Clang][Driver][RFC] Add driver support for C++ Parallel Algorithm Offload

thakis added a comment. This seems to break tests on Mac and windows, see eg http://45.33.8.238/macm1/70415/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155775/new/ htt

[PATCH] D35780: Introduce -nostdlib++ flag to disable linking the C++ standard library.

thakis added a comment. Herald added subscribers: pmatos, asb, abrachet. Herald added a project: All. Also useful for random other projects on github: https://github.com/mapbox/mason/pull/545#issuecomment-367082479 GCC also added support for this flag: https://github.com/gcc-mirror/gcc/commit/f

[PATCH] D157913: [Coverage] Allow Clang coverage to be used with debug info correlation.

thakis added a comment. Looks like this breaks check-clang on mac: http://45.33.8.238/macm1/69297/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157913/new/ https://revie

[PATCH] D153924: [OpenMP] Allow exceptions in target regions when offloading to GPUs

thakis added inline comments. Comment at: clang/test/OpenMP/amdgpu_throw_trap.cpp:4 +// RUN: %clang_cc1 -fopenmp -triple amdgcn-amd-amdhsa -fopenmp-is-target-device %s -emit-llvm -S -Wno-openmp-target-exception -o - | FileCheck -check-prefix=DEVICE %s +// RUN: %clang_cc1 -fopen

[PATCH] D153924: [OpenMP] Allow exceptions in target regions when offloading to GPUs

thakis added inline comments. Comment at: clang/test/OpenMP/amdgpu_throw_trap.cpp:4 +// RUN: %clang_cc1 -fopenmp -triple amdgcn-amd-amdhsa -fopenmp-is-target-device %s -emit-llvm -S -Wno-openmp-target-exception -o - | FileCheck -check-prefix=DEVICE %s +// RUN: %clang_cc1 -fopen

[PATCH] D153924: [OpenMP] Allow exceptions in target regions when offloading to GPUs

thakis added inline comments. Comment at: clang/test/OpenMP/amdgpu_throw_trap.cpp:4 +// RUN: %clang_cc1 -fopenmp -triple amdgcn-amd-amdhsa -fopenmp-is-target-device %s -emit-llvm -S -Wno-openmp-target-exception -o - | FileCheck -check-prefix=DEVICE %s +// RUN: %clang_cc1 -fopen

[PATCH] D151834: Include math-errno with fast-math

thakis added a comment. This breaks check-clang on macOS: http://45.33.8.238/macm1/68762/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151834/new/ https://reviews.llvm.or

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

thakis added a comment. Looks like the test is missing a RUN line and hence makes lit fail: http://45.33.8.238/linux/117631/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D158476: [driver] Search for compatible Android runtime directories

thakis added a comment. Works for me. How do you imagine the transition happening? Should we emit some kind of warning if the old fallback is hit? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158476/new/ https://reviews.llvm.org/D158476

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

thakis added a comment. Looks like this breaks tests on windows: http://45.33.8.238/win/83485/step_7.txt Please take a look and revert for now if it takes a while to fix. (Also, if the patch doesn't already do it, it probably shouldn't change defaults in clang-cl mode?) Repository: rG LLVM

[PATCH] D158476: [driver] Search for compatible Android runtime directories

thakis added a comment. I think this is a useful feature, for the reasons mentioned on the thread. Since this is a superset of D115049 (... right?), this should probably revert the changes over there? Maybe we should dump searched paths in `-v` mode? Reposit

[PATCH] D157565: [CodeGen] Add AArch64 behavior to existing MFS tests

thakis added a comment. reverted in 3d22dac6c3b97d7bb92f243886dfb0d32a5c42e9 for now Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157565/new/ https://reviews.llvm.org/D157565

[PATCH] D158279: clang: Make rewrite-includes-macros.cpp runnable on non-Win

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG960881a7f32a: clang: Make rewrite-includes-macros.cpp runnable on non-Win (authored by thakis). Herald added a project: clang. Repository: rG LLVM

[PATCH] D157565: [CodeGen] Add AArch64 behavior to existing MFS tests

thakis added a comment. This breaks tests on windows: http://45.33.8.238/win/82918/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157565/new/ https://reviews.llvm.org/D157

[PATCH] D158279: clang: Make rewrite-includes-macros.cpp runnable on non-Win

thakis created this revision. thakis added a reviewer: aganea. Herald added a project: All. thakis requested review of this revision. As far as I can tell, there's nothing Windows-specific about the test and it passes fine on other platforms. I found this test when running rg clang_cl clang/te

[PATCH] D157680: [X86]Support options -mno-gather -mno-scatter

thakis added inline comments. Comment at: clang/test/Driver/x86-no-gather-no-scatter.cpp:3 +// RUN: %clang -c -mno-gather -### %s 2>&1 | FileCheck --check-prefix=NOGATHER %s +// RUN: %clang_cl -c /Qgather- -### %s 2>&1 | FileCheck --check-prefix=NOGATHER %s +// NOGATHER: "-targ

[PATCH] D157808: [clang] Add missing field to VisibilityAttr json AST dump

thakis added a comment. In D157808#4587014 , @thakis wrote: > Hello, one of the many dump commits broke tests on macOS: > http://45.33.8.238/macm1/67030/step_7.txt > > Please take a look and revert for now if it takes a while to fix. > > Also, consider s

[PATCH] D157808: [clang] Add missing field to VisibilityAttr json AST dump

thakis added a comment. Hello, one of the many dump commits broke tests on macOS: http://45.33.8.238/macm1/67030/step_7.txt Please take a look and revert for now if it takes a while to fix. Also, consider spreading out commits a bit so that if one breaks something, it's easily to see which one

[PATCH] D144634: [Clang][OpenMP] Support for Code Generation of loop bind clause

thakis added a comment. In D144634#4573483 , @koops wrote: > Can someone please check for MacOS? Yesterday when this support was > committed, the CHECK statements in tests loop_bind_codegen.cpp and > loop_bind_enclosed.cpp had failed on Mac I patched

[PATCH] D157321: [InstrProf] Fix macOS profile tests after D156569

thakis accepted this revision. thakis 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/D157321/new/ https://reviews.llvm.org/D157321

[PATCH] D155895: Anonymous unions should be transparent wrt `[[clang::trivial_abi]]`.

thakis added a comment. Reverted in 0342bbf223fa12701a0570a23f9eac433b8b341c for now, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155895/new/ https://reviews.llvm.org/D155895 ___ cfe-commits ma

[PATCH] D155895: Anonymous unions should be transparent wrt `[[clang::trivial_abi]]`.

thakis added a comment. Looks like this breaks tests on windows: http://45.33.8.238/win/82239/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155895/new/ https://reviews.ll

[PATCH] D157245: [clang/cxx-interop] Teach clang to ignore availability errors that come from CF_OPTIONS

thakis added a comment. In D157245#4565166 , @hans wrote: > lgtm Thanks! > Maybe we want this for the release/17.x branch too? Sure, trying that here: https://github.com/llvm/llvm-project/issues/64496 CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D157245: [clang/cxx-interop] Teach clang to ignore availability errors that come from CF_OPTIONS

thakis closed this revision. thakis added a comment. bb58748e52ebd48a46de20525ef2c594db044a11 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157245/new/ https://reviews.llvm.org/D157245 __

[PATCH] D157245: [clang/cxx-interop] Teach clang to ignore availability errors that come from CF_OPTIONS

thakis created this revision. thakis added a reviewer: hans. Herald added a project: All. thakis requested review of this revision. This cherry-picks https://github.com/apple/llvm-project/pull/6431 since without it, macOS 14 SDK headers don't compile when targeting catalyst. Fixes #64438.

[PATCH] D155736: Fix __cfi_check not aligned to 4k on relocatable files with no executable code

thakis added a comment. Looks like this breaks check-clang: http://45.33.8.238/linux/114361/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155736/new/ https://reviews.llvm

[PATCH] D156363: [Driver] -###: exit with code 1 if hasErrorOccurred

thakis added a comment. http://45.33.8.238/linux/113921/step_7.txt is failing due to this. Is this something on my end? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156363/new/ https://reviews.llvm.org/D156363 ___

[PATCH] D156533: [clang][DeclPrinter] Fix missing semicolon in AST print for methods that are definitions without having a body

thakis added a comment. This breaks tests on Mac: http://45.33.8.238/macm1/65934/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156533/new/ https://reviews.llvm.org/D15653

[PATCH] D156452: [clang-tidy] Sort options in --dump-config

thakis added a comment. Thanks for the quick revert! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156452/new/ https://reviews.llvm.org/D156452 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

[PATCH] D156452: [clang-tidy] Sort options in --dump-config

thakis added a comment. This might have broken tests: http://45.33.8.238/linux/113736/step_8.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156452/new/ https://reviews.llvm.org/D

[PATCH] D152093: [clang][Analysis] Handle && and || against variable and its negation as tautology

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. lg with aaron's suggestion. thanks for the patch, and for your patience! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152093/new/ https://reviews.llvm.org/D152093 __

[PATCH] D156116: [Clang][LoongArch] Fix ABI handling of empty structs in C++ to match GCC behaviour

thakis added a comment. This seems to not build: http://45.33.8.238/linux/113405/step_4.txt (?) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156116/new/ https://reviews.llvm.org/D156116 ___ cfe-commits

[PATCH] D154658: Optimize emission of `dynamic_cast` to final classes.

thakis added a comment. This caused https://github.com/llvm/llvm-project/issues/64088 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154658/new/ https://reviews.llvm.org/D154658 ___ cfe-commits mailing li

[PATCH] D134544: [clang-cl] Implement /ZH: flag

thakis added inline comments. Comment at: clang/include/clang/Driver/Options.td:3220 Values<"simple,mangled">, Flags<[CC1Option, NoDriverOption]>; +def gsrc_hash_EQ : Joined<["-"], "gsrc-hash=">, + Group, Flags<[CC1Option, NoDriverOption]>, dblaikie wrote

[PATCH] D152093: [clang][Analysis] Handle && and || against variable and its negation as tautology

thakis added a comment. Seems like a nice idea to me, thanks! Do you have any numbers on how often this fires in practice, and what the true positive rate is? (Build some large-ish open source project with this, and see what it finds.) Did you verify that this has negligible compile time impac

[PATCH] D153366: [dataflow] Add dedicated representation of boolean formulas

thakis added a comment. Looks like this breaks tests on windows: http://45.33.8.238/win/80815/step_7.txt Please take a look etc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153366/new/ https://reviews.llvm.org/D153366 ___

[PATCH] D149280: [clang-tidy] Add modernize-printf-to-std-print check

thakis added a comment. None i know of, but given the bot's already red, you won't make it worse :) Tests shouldn't include any headers generally, not even inttypes.h (unless the test tests that header). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D149280: [clang-tidy] Add modernize-printf-to-std-print check

thakis added a comment. This breaks tests on windows: http://45.33.8.238/win/80283/step_8.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149280/new/ https://reviews.llvm.org/D149

[PATCH] D152785: [COFF] Support -gsplit-dwarf for COFF on Windows

thakis added a comment. Reverted in b851308b870a67ccb606d0d33f159a70fd3522c7 for now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152785/new/ https://reviews.llvm.org/D152785 ___ cfe-commits mailing li

[PATCH] D152785: [COFF] Support -gsplit-dwarf for COFF on Windows

thakis added a comment. Looks like this breaks tests on Mac: http://45.33.8.238/macm1/63504/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152785/new/ https://reviews.llvm

[PATCH] D152472: [Clang][MS] Remove assertion on BaseOffset can't be smaller than Size.

thakis added a comment. This is still breaking check-clang on windows: http://45.33.8.238/win/79908/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152472/new/ https://rev

[PATCH] D152472: [Clang][MS] Remove assertion on BaseOffset can't be smaller than Size.

thakis added a comment. Also on http://45.33.8.238/win/79874/step_7.txt Reverted in 9c560350dd57d0dcb849c5915fa7c50b139ce671 for now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152472/new/ https://reviews.llvm.org/D152472 _

[PATCH] D150023: [ABI] [C++20] [Modules] Don't generate vtable if the class is defined in other module unit

thakis added a comment. Reverted in dbdd6372b7af2f6df5f41d19d966e6bac1b30208 for now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150023/new/ https://reviews.llvm.org/D150023

[PATCH] D150023: [ABI] [C++20] [Modules] Don't generate vtable if the class is defined in other module unit

thakis added a comment. Looks like this breaks check-clang on Mac: http://45.33.8.238/macm1/62779/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150023/new/ https://revie

[PATCH] D151954: [clang-format] Fix overlapping whitespace replacements before PPDirective

thakis added a comment. Looks like the revert broke tests: http://45.33.8.238/linux/109159/step_7.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151954/new/ https://reviews.llvm.org/D151954 ___ cfe-co

[PATCH] D151608: [Sema] Remove unused function getFloat128Identifier

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

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

thakis added subscribers: sammccall, thakis. thakis added inline comments. Comment at: clang-tools-extra/pseudo/lib/grammar/CMakeLists.txt:5 -# Dependencies should be minimal to avoid long dep paths in the build graph. -# It does use clangBasic headers (tok::TokenKind), but linki

[PATCH] D151609: [Driver] Remove unused class ForceSuccessCommand

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. lgtm, thanks! (clicky: https://reviews.llvm.org/rG6625680a581c5e29c53d9f58d864cc6cd3cd05f6) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151609

[PATCH] D151042: [clang] Add tests for CWG issues 977, 1482, 2516

thakis added a comment. In D151042#4364131 , @Endill wrote: > All tests pass for me on Linux. > As far as I understand, diagnostics on Windows are not supposed to be > different given the same `-cc1` flags. Reverting for further investigation. Thanks fo

[PATCH] D151042: [clang] Add tests for CWG issues 977, 1482, 2516

thakis added a comment. On my bots (which cover linux, mac/arm, and windows), it's only failing on the windows bot. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151042/new/ https://reviews.llvm.org/D151042 ___

[PATCH] D151042: [clang] Add tests for CWG issues 977, 1482, 2516

thakis added a comment. This breaks check-clang on windows: http://45.33.8.238/win/78827/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151042/new/ https://reviews.llvm.or

[PATCH] D144999: [Clang][MC][MachO]Only emits compact-unwind format for "canonical" personality symbols. For the rest, use DWARFs.

thakis added a comment. Reverted in 4980eead4d0b4666d53dad07afb091375b3a13a0 for now. (The assert only happens when targeting x86_64, not arm64. The repro script above has the right `-arch` flag to toggle it.) Repository:

[PATCH] D144999: [Clang][MC][MachO]Only emits compact-unwind format for "canonical" personality symbols. For the rest, use DWARFs.

thakis added a comment. This makes lld crash when linking a libc++ with object files built by a clang that also contains the patch. Here's a reduced (to just libc++), stand-alone repro that repros the crash in 6 seconds on my m1 mbp: https://bugs.chromium.org/p/chromium/issues/detail?id=1446967

[PATCH] D146664: Apply the same fallbacks as runtimes search for stdlib search

thakis added a comment. Is it possible to test this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146664/new/ https://reviews.llvm.org/D146664 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

[PATCH] D144509: [CMake] Bumps minimum version to 3.20.0.

thakis added a comment. Is https://reviews.llvm.org/D144509#4347562 fixed? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144509/new/ https://reviews.llvm.org/D144509 ___ cfe-commits mailing list cfe-comm

[PATCH] D144509: [CMake] Bumps minimum version to 3.20.0.

thakis added a comment. https://github.com/llvm/llvm-project/issues/62719 is independent of chromium and others have reported problems above too, from what I understand. Is that not accurate? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144509/ne

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

thakis added a comment. Reverted in cf8c358dc9414e3de9f65eebdfdcb66dd5817857 for now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144654/new/ https://reviews.llvm.org/D144654 ___ cfe-commits mailing li

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

thakis added a comment. In D144654#4349908 , @john.brawn wrote: > This patch caused a failure in AArch64 buildbots due to > AArch64TargetInfo::getTargetDefines redefining _LP64 and __LP64__. Fixed in > https://reviews.llvm.org/rGe55d52cd34fb7a6a6617639

[PATCH] D144509: [CMake] Bumps minimum version to 3.20.0.

thakis added a comment. Reverted this and follow-ups in d763c6e5e2d0a6b34097aa7dabca31e9aff9b0b6 for now. Sorry this is such a pain to land :( (See also discussion over in D150688 ) Reposit

[PATCH] D150668: Add doc link to missing include diagnostics.

thakis added a comment. For the records: Fixed in 76941b68ecd9 . Thanks for the quick fix! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150668/new/ https://reviews.llvm.org/D15

[PATCH] D150668: Add doc link to missing include diagnostics.

thakis added a comment. This seems to break tests: http://45.33.8.238/linux/107099/step_9.txt Please take a look, and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150668/new/ https://reviews.llvm.org/D15

[PATCH] D150282: [Driver] -ftime-trace: derive trace file names from -o and -dumpdir

thakis added a comment. Apologies for the noise, looks like my win bot hasn't built in a while. Looks like this is already fixed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150282/new/ https://reviews.llvm.org/D150282 _

[PATCH] D150282: [Driver] -ftime-trace: derive trace file names from -o and -dumpdir

thakis added a comment. This breaks tests on windows: http://45.33.8.238/win/78516/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150282/new/ https://reviews.llvm.org/D150

[PATCH] D149997: [clang] [test] Narrow down MSVC specific behaviours from "any windows" to only MSVC/clang-cl

thakis added a comment. Fails with cmake too: https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket/8781510258907322449/+/u/package_clang/stdout?format=raw Reverted in 56bac6c87a2d9046dbac36aae4910b4b90edf643 for

[PATCH] D149997: [clang] [test] Narrow down MSVC specific behaviours from "any windows" to only MSVC/clang-cl

thakis added a comment. This broke check-clang on windows: http://45.33.8.238/win/78359/step_7.txt (The Driver/split-debug.c failure is something else and since fixed, but the other two tests are due to this change.) Please take a look and revert for now if it takes a while to fix. Repository

[PATCH] D148769: Split out `CodeGenTypes` from `CodeGen` for LLT/MVT

thakis added a comment. In D148769#4311232 , @dblaikie wrote: > I wouldn't mind a follow-up commit that creates a new/separate directory > (llvm/{lib,include/llvm}/CodeGenTypes) but that's probably a lot of > unnecessary churn too, so I can appreciate t

[PATCH] D147281: Stop modifying trailing return types.

thakis added a comment. Looks like this caused a change in behavior: https://github.com/llvm/llvm-project/issues/62361 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147281/new/ https://reviews.llvm.org/D147281

[PATCH] D138329: [-Wunsafe-buffer-usage] Add a new recursive matcher to replace `forEachDescendant` in unsafe buffer check

thakis added a comment. > ASTMatchers have been baked into clang binary for at least a few years > already, actively used by the static analyzer (some backstory in D25429 > ). This warning is, however, probably the > first use in clang proper, so they'll be ther

[PATCH] D134456: [PGO] Consider parent context when weighing branches with likelyhood.

thakis added a comment. Herald added a subscriber: wlei. >> Another point: having a diagnostic fire (failing a `-Werror` build) >> depending on the content of the profile passed to `-fprofile-use` seems >> pretty hostile to build workflows. > > Okay, this last point is especially compelling to m

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

thakis added inline comments. Comment at: clang/utils/bundle_resources.py:6 +# 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 ---

[PATCH] D144828: [clang-tidy] Add misc-header-include-cycle check

thakis added a comment. This breaks tests: http://45.33.8.238/linux/104455/step_8.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144828/new/ https://reviews.llvm.org/D144828 ___

[PATCH] D146417: [clangd] Fix AddUsing in the face of typo-correction

thakis added a comment. This breaks tests on windows: http://45.33.8.238/win/76177/step_9.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146417/new/ https://reviews.llvm.org/D146

[PATCH] D134827: [clangd] Avoid recursion on UnresolvedUsingValueDecl during semantic highlighting

thakis added a comment. This breaks tests: http://45.33.8.238/linux/102084/step_9.txt Please take a look and revert for now if it takes a while to fix. Did you not run tests before commiting? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134827/ne

[PATCH] D146280: [clang] Include the error message in file reading error diagnostic

thakis added a comment. (if the presubmit is related, maybe some flang test needs updating? Weird that flang uses clang's diags.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146280/new/ https://reviews.llvm.org/D146280 _

[PATCH] D146280: [clang] Include the error message in file reading error diagnostic

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

[PATCH] D143496: [clangd] Add support for missing includes analysis.

thakis added a comment. Oh, that was reported a while ago already. Reverted in 2eb5ac99a76dbbf8ac68c538211fabeaa5ac0bfd for now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D14

[PATCH] D143496: [clangd] Add support for missing includes analysis.

thakis added a comment. This breaks tests on windows: http://45.33.8.238/win/75486/step_9.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143496/new/ https://reviews.llvm.org/D143

[PATCH] D137527: [C++20] [Modules] [ClangScanDeps] Add ClangScanDeps support for C++20 Named Modules in P1689 format (2/4)

thakis added a comment. Either this or D139168 is breaking tests on Windows: http://45.33.8.238/win/74780/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D137526: [C++20] [Modules] [NFC] Add Preprocessor methods for named modules - for ClangScanDeps (1/4)

thakis added inline comments. Comment at: clang/unittests/Lex/CMakeLists.txt:21 clangBasic + clangFrontend clangLex It seems strange layering-wise that the Lex unit tests would depend on Frontend. Is there any way to prevent that dep? Repository: rG L

[PATCH] D142757: [clang][driver] Emit an error for `/clang:-x`

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

[PATCH] D142757: [clang][driver] Emit an error for `/clang:-x`

thakis added a comment. This breaks tests on Mac: http://45.33.8.238/macm1/54097/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142757/new/ https://reviews.llvm.org/D14275

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

thakis added a comment. Reverted in adf7ffd51ee34c3a72d3168f5aed8b946ba3d2cc for now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142388/new/ https://reviews.llvm.org/D142388

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

thakis added a comment. Looks like the test fails on win: http://45.33.8.238/win/74290/step_7.txt And Mac: http://45.33.8.238/macm1/54080/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

[PATCH] D141738: Add initial support for cross compile Windows runtimes under Linux when building Fuchsia clang toolchain

thakis added inline comments. Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:95 +# and remove these libpath flags. +-libpath:"${LLVM_WINSYSROOT}/VC/Tools/MSVC/14.34.31933/lib/x64" +-libpath:"${LLVM_WINSYSROOT}/VC/Tools/MSVC/14.34.31933/atlmfc/lib/x64"

[PATCH] D141929: Add support for clang-cl's option -fexcess-precision.

thakis added a comment. This breaks tests on macOS: http://45.33.8.238/macm1/53215/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141929/new/ https://reviews.llvm.org/D141

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

thakis added a comment. If it's not too much trouble, maybe you could cherry-pick and amend 8d498e08deaf6e06a578cfedb4eb259b722ac7f6 into the commit that relands this. (If not, no worries.) Repository: rG LLVM Github Mon

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

thakis added a comment. Reverted in a5f446bc4bb1ac78d6852cc8e251a1229899b783 for now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136315/new/ https://reviews.llvm.org/D136315 ___ cfe-commits mailing li

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

thakis added a comment. I've confirmed that this breaks a pretty vanilla build setup. /Applications/CMake.app/Contents/bin/cmake -GNinja -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=ON -DLLVM_ENABLE_PROJECTS='clang;compiler-rt;clang-tools-extra;lld' -DLLVM_APPEND_VC_REV=NO -DLLVM_TARG

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

thakis added a comment. In D136315#4065426 , @calebzulawski wrote: > I don't think we necessarily need to revert due to the xcrun quirk since I > think it's harmless, though I can provide a follow up change if necessary. I > just checked Rust's source

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

thakis added a comment. I just verified that that does indeed happen, and it does: $ strace out/gn/bin/clang --target=x86_64-apple-macos -x c -c /dev/null 2>&1 | grep xcrun access("/usr/bin/xcrun", F_OK) = -1 ENOENT (No such file or directory) Repository: rG LLVM Github Monorep

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

thakis added a comment. My bot is a Mac. It doesn't set any env vars (other than the ones that lit always sets). In theory it shouldn't matter since tests aren't supposed to depend on system headers. (Tangential: maybe there should be a flag to turn this off and %clang should expand to `clang`

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

thakis added a comment. This breaks oodles of tests for me: http://45.33.8.238/macm1/52994/step_7.txt Is anyone else seeing this? Also, independently of that, shouldn't this check that the _host_ os is macOS as well? Doesn't the current code try to run `xcrun` if I do `clang --target=arm64-app

[PATCH] D141992: [NFC] [Serialization] Add static assert for the size of the decls to mention developers to remember to touch the serializer after them modified the field of decls

thakis added a comment. Thanks for the fast revert! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141992/new/ https://reviews.llvm.org/D141992 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D141992: [NFC] [Serialization] Add static assert for the size of the decls to mention developers to remember to touch the serializer after them modified the field of decls

thakis added a comment. Also fails on 64-bit windows fwiw: http://45.33.8.238/win/73344/step_4.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141992/new/ https://reviews.llvm.org/D141992 ___ cfe-commi

[PATCH] D141992: [NFC] [Serialization] Add static assert for the size of the decls to mention developers to remember to touch the serializer after them modified the field of decls

thakis added inline comments. Comment at: clang/lib/Serialization/ASTWriterDecl.cpp:2313 void ASTDeclWriter::VisitOMPDeclareMapperDecl(OMPDeclareMapperDecl *D) { + static_assert(sizeof(OMPDeclareMapperDecl) == 120, +"You need to update the serializer after you c

[PATCH] D100955: [-Wcalled-once] Do not run analysis on Obj-C++

thakis added a comment. Herald added a project: All. Can you say a few words on _why_ this is emitted in only Objective-C? What's missing for Objective-C++? I was surprised that this warning fired in a .m file but not in a .mm file. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D140800: [OptTable] Precompute OptTable prefixes union table through tablegen

thakis added inline comments. Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:141 -class WrapperOptTable : public opt::OptTable { +class WrapperOptTable : public opt::GenericOptTable { public: thakis wrote: > C …not sure what this was, plea

  1   2   3   4   5   6   7   8   9   10   >