[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] 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] 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] 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] 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] 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 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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. 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] D138301: [llvm, polly, clang] Stop setting config.enable_shared in most places

This revision was automatically updated to reflect the committed changes. Closed by commit rG281a5c7ef112: [llvm,polly,clang] Stop setting config.enable_shared in most places (authored by thakis). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github

[PATCH] D137217: [LTO][COFF] Use bitcode file names in lto native object file names.

thakis added a comment. This doesn't build: http://45.33.8.238/win/70474/step_4.txt http://45.33.8.238/linux/92146/step_4.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/D137217/new

[PATCH] D138073: [clang-doc] Move file layout to the generators.

thakis added a comment. This breaks tests on mac: http://45.33.8.238/macm1/49211/step_8.txt (and possibly win; that's still cycling.) Please take a look and revert for now if it takes a while to fix. (Also, 👋 Mr W!) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[PATCH] D138073: [clang-doc] Move file layout to the generators.

thakis added a comment. In D138073#3944730 , @paulkirth wrote: > @thakis thanks for the report. I've reverted for now. @brettw can you take a > look? I'm a bit surprised that passed on the Linux and windows bot, but > failed on Mac. Probably a BSD find

[PATCH] D90568: [clang] Add [is|set]Nested methods to NamespaceDecl

thakis added a comment. This breaks the build: http://45.33.8.238/linux/92294/step_4.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/D90568/new/ https://reviews.llvm.org/D90568 __

[PATCH] D90568: [clang] Add [is|set]Nested methods to NamespaceDecl

thakis added a comment. My bots look happy at least. Thanks for the quick fix! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90568/new/ https://reviews.llvm.org/D90568 ___ cfe-commits mailing list cfe-co

[PATCH] D138677: [Lex] Fix suggested spelling of /usr/bin/../include/foo

thakis added a comment. This breaks tests on windows: http://45.33.8.238/win/70650/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/D138677/new/ https://reviews.llvm.org/D138

[PATCH] D77022: Use IgnoreImpCasts() instead of reimplementing it.

thakis created this revision. thakis added a reviewer: dcoughlin. Herald added subscribers: martong, Charusso. No intended behavior change. https://reviews.llvm.org/D77022 Files: clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp Index: clang/lib/StaticAnalyzer/Checkers/NullabilityChe

[PATCH] D77022: [analyzer] Use IgnoreImpCasts() instead of reimplementing it.

thakis marked an inline comment as done. thakis added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:512 + return E->IgnoreImpCasts(); } NoQ wrote: > Charusso wrote: > > I think it would make sense to remove the helper-f

[PATCH] D75914: systemz: allow configuring default CLANG_SYSTEMZ_ARCH

thakis added inline comments. Comment at: clang/CMakeLists.txt:311 + "SystemZ Default Arch") +add_definitions( -DCLANG_SYSTEMZ_DEFAULT_ARCH="${CLANG_SYSTEMZ_DEFAULT_ARCH}") + Hahnfeld wrote: > Passing values like this is unusual for CMake and causes all source f

[PATCH] D75914: systemz: allow configuring default CLANG_SYSTEMZ_ARCH

thakis added inline comments. Comment at: clang/CMakeLists.txt:311 + "SystemZ Default Arch") +add_definitions( -DCLANG_SYSTEMZ_DEFAULT_ARCH="${CLANG_SYSTEMZ_DEFAULT_ARCH}") + thakis wrote: > thakis wrote: > > Hahnfeld wrote: > > > Passing values like this is unu

[PATCH] D75914: systemz: allow configuring default CLANG_SYSTEMZ_ARCH

thakis added inline comments. Comment at: clang/CMakeLists.txt:311 + "SystemZ Default Arch") +add_definitions( -DCLANG_SYSTEMZ_DEFAULT_ARCH="${CLANG_SYSTEMZ_DEFAULT_ARCH}") + thakis wrote: > Hahnfeld wrote: > > Passing values like this is unusual for CMake and c

[PATCH] D77022: [analyzer] Use IgnoreImpCasts() instead of reimplementing it.

This revision was automatically updated to reflect the committed changes. Closed by commit rG7ea64ae3afe4: [analyzer] Use IgnoreImpCasts() instead of reimplementing it. (authored by thakis). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D77184: Make it possible for lit.site.cfg to contain relative paths, and use it for llvm and clang

This revision was automatically updated to reflect the committed changes. Closed by commit rGfb80b6b2d58c: Make it possible for lit.site.cfg to contain relative paths, and use it for… (authored by thakis). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to com

[PATCH] D74564: libclang: Add static build support for Windows

thakis added inline comments. Comment at: clang/tools/libclang/CMakeLists.txt:117 if(ENABLE_SHARED) + target_compile_definitions(libclang PUBLIC CINDEX_EXPORTS) if(WIN32) Is this enough? Every target that depends on libclang now needs to define CINDEX_EXPOR

[PATCH] D74564: libclang: Add static build support for Windows

thakis added a comment. Another question: We have a bot that builds LLVM with this cmake config: 'cmake', '-GNinja', '-DCMAKE_BUILD_TYPE=Release', '-DLLVM_ENABLE_ASSERTIONS=OFF', '-DLLVM_ENABLE_PROJECTS=clang;compiler-rt;lld;chrometools;clang-tools-extra', '-DLLVM_TARGETS_TO_BUILD=AArch64;ARM;

[PATCH] D74564: libclang: Add static build support for Windows

thakis added a comment. Great, thanks. Since we're currently broken due to this change, any ETA for the follow-up? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74564/new/ https://reviews.llvm.org/D74564

[PATCH] D74907: libclang: Make shared object symbol exporting by default

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Thanks! Does this make it so that libclang.dll is built again with -DLLVM_ENABLE_PIC=NO on Window? From what I understand, it doesn't. Is that correct? If so, could you reinstantiate that too?

[PATCH] D74907: libclang: Make shared object symbol exporting by default

thakis added a comment. In D74907#1884842 , @cristian.adam wrote: > In D74907#1884795 , @thakis wrote: > > > Thanks! Does this make it so that libclang.dll is built again with > > -DLLVM_ENABLE_PIC=NO on Window? F

[PATCH] D74907: libclang: Make shared object symbol exporting by default

thakis accepted this revision. thakis added a comment. Thanks! Let me try landing this and see what happens :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74907/new/ https://reviews.llvm.org/D74907 _

[PATCH] D74907: libclang: Make shared object symbol exporting by default

This revision was automatically updated to reflect the committed changes. Closed by commit rG7a7c753b0cca: libclang: Make shared object symbol exporting by default (authored by cristian.adam, committed by thakis). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D74907: libclang: Make shared object symbol exporting by default

thakis added a comment. Looks like things are still broken in several ways even with this in: [2600/2950] Linking CXX executable bin\c-index-test.exe FAILED: bin/c-index-test.exe cmd.exe /C "cd . && C:\b\s\w\ir\cache\builder\src\third_party\llvm-build-tools\cmake-3.12.1-win32-x86\bin\cm

[PATCH] D75093: clang-cl: Add a `/showIncludes:user` flag.

thakis created this revision. thakis added a reviewer: hans. This flag is like /showIncludes, but it only includes user headers and omits system headers (similar to MD and MMD). The motivation is that projects that already track system includes though other means can use this flag to get consis

[PATCH] D75093: clang-cl: Add a `/showIncludes:user` flag.

thakis added a comment. This is what -MD passes (which is why the -sys flag already exists), and more orthogonal flags instead of fewer, tangled ones is what we usually go for at the cc1 layer. So I like it more as is. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75093/new/ https://r

[PATCH] D75093: clang-cl: Add a `/showIncludes:user` flag.

thakis updated this revision to Diff 246417. thakis added a comment. add one more test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75093/new/ https://reviews.llvm.org/D75093 Files: clang/include/clang/Driver/CLCompatOptions.td clang/lib/Driver/ToolChains/Clang.cpp clang/lib/Fro

[PATCH] D75093: clang-cl: Add a `/showIncludes:user` flag.

This revision was automatically updated to reflect the committed changes. Closed by commit rGbcda1269c4c4: clang-cl: Add a `/showIncludes:user` flag. (authored by thakis). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7

[PATCH] D71775: [ThreadPool] On Windows, extend usage to all CPU sockets and all NUMA groups

thakis added a comment. https://bugs.chromium.org/p/chromium/issues/detail?id=1051578#c12 : """ FYI for those building on AMD Bulldozer family of processors and its various iterations after this commit: https://reviews.llvm.org/D71775 Building with ThinLTO on Bulldozer and similar appears to n

[PATCH] D72153: [libTooling] Add function to determine associated text of a declaration.

thakis added a comment. http://45.33.8.238/win/9173/step_7.txt Ptal! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72153/new/ https://reviews.llvm.org/D72153 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

thakis added a comment. This landed here: https://github.com/llvm/llvm-project/commit/bd97704eb5a95ecb048ce343c1a4be5d94e5 It broke tests on mac: http://45.33.8.238/mac/9011/step_7.txt Please take a look, and if it takes a while please revert while you investigate. Repository: rG LLVM G

[PATCH] D74842: [clangd] Make use of syntax tokens in ReplayPreamble

thakis added a comment. Breaks win: http://45.33.8.238/win/9705/step_9.txt (Or your other change that landed at the same time) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74842/new/ https://reviews.llvm.org/D74842

[PATCH] D75602: [clangd] Cancel certain operations if the file changes before we start.

thakis added a comment. This broke building on my bots: http://45.33.8.238/linux/11862/step_4.txt `ld.lld: error: undefined symbol: clang::clangd::TUScheduler::InvalidatedError::ID` Looks like that is indeed declared in this change, but it doesn't have a definition. Repository: rG LLVM Gi

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

thakis added a comment. Sorry, I had missed the RfC, but it looks like there wasn't a lot of discussion on it anyways. Adding fortran support to clang's driver has been suggested and decided against before, see "[cfe-commits] [RFC and PATCH] Fortran" In D63607#1726895

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

thakis added a comment. > Interesting, I had missed that conversation! Google and DDG fail to find it > for me even quoting sentences out of it. Found it by wgetting the gzip > mailing list archives and grepping them - is there a better way? :) (If you want, I can forward you the thread. But as

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

thakis added a comment. In D63607#1727037 , @peterwaller-arm wrote: > Thanks for chiming in. > > > Sorry, I had missed the RfC, but it looks like there wasn't a lot of > > discussion on it anyways. > > Apologies @thakis - did I jump the gun? And if so, w

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

thakis added a comment. > @thakis: I can't make quick sense of the failure from those logs alone. I > would appreciate it very much to see the output of the `clang -###` run lines > to see what's missing. It should be showing that it would invoke flang. It's the very first run line that's faili

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

thakis added a comment. Thanks! Looks like it's just the `-E` test that's causing problems; the test passes with that disabled: $ git diff diff --git a/clang/test/Driver/flang/flang.f90 b/clang/test/Driver/flang/flang.f90 index 97e4847f843..4cbc2cd8754 100644 --- a/clang/test/Driver/fl

[PATCH] D69582: Let clang driver support parallel jobs

thakis added inline comments. Comment at: clang/lib/Driver/Compilation.cpp:303 +} +std::thread Th(Work); +Th.detach(); Maybe a select() / fork() loop is a better approach than spawning one thread per subprocess? This is doing thread-level parallelism

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

thakis added a comment. I had some time to take a look. D69636 makes things work on mac. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63607/new/ https://reviews.llvm.org/D63607 ___ cfe-commits mailing lis

[PATCH] D69626: Fix Microsoft compatibility handling of commas in nested macro expansions.

thakis added a comment. Is this needed to parse system headers? In general, we try to emulate only warts we must emulate, and if feasible we try to emit some -Wmicrosoft warning when the compat path is taken. (MSVC added /experimental:preprocessor like 2 years ago, but it hasn't made it to a /

[PATCH] D69636: Make flang driver stuff work on macOS

This revision was automatically updated to reflect the committed changes. Closed by commit rG4b6597f49896: Make flang driver stuff work on macOS (authored by thakis). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D68937: [clangd] Add parameter renaming to define-inline code action

thakis added inline comments. Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:1388 // results in arbitrary failures as function body becomes NULL. ExtraArgs.push_back("-fno-delayed-template-parsing"); + for (const auto &Case : Cases) You migh

[PATCH] D68937: [clangd] Add parameter renaming to define-inline code action

thakis added a comment. This is failing on Windows: http://45.33.8.238/win/1481/step_7.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68937/new/ https://reviews.llvm.org/D68937 ___ cfe-commits maili

[PATCH] D68937: [clangd] Add parameter renaming to define-inline code action

thakis added a comment. Tests are happy again after 1c66d09 , thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68937/new/ https://reviews.llvm.org/D68937 ___

[PATCH] D68969: [clang-format] Remove the dependency on frontend

thakis added a comment. Since this was reverted: are you looking into relanding this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68969/new/ https://reviews.llvm.org/D68969 ___ cfe-commits mailing li

[PATCH] D68969: [clang-format] Remove the dependency on frontend

thakis added a subscriber: vlad.tsyrklevich. thakis added a comment. The revert message said: Vlad Tsyrklevich via cfe-commits Tue, Oct 29, 1:51 PM (7 days ago) to via, mydeveloperday I've reverted this commit as it was causing UBSan failures on the ubsan bot. These failures looked l

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

thakis added a comment. Thanks for sending this out! Instead of the dynamic lookup of that symbol, what do you think about passing in the function via a normal api? That way, the type checker and linker help us keep things working; dynamic lookup is always a bit subtle and hard to grep for. (l

[PATCH] D69760: [Clang][Driver] Don't pun -fuse-ld=lld as -fuse-ld=lld-link with msvc

thakis added a comment. We currently do cross builds of chrome/win on linux, and this breaks that. As far as I know, no linker other than lld-link is able to write PDB files, so when targeting windows-msvc we definitely shouldn't change the current behavior. I don't have an opinion on windows-m

[PATCH] D69760: [Clang][Driver] Don't pun -fuse-ld=lld as -fuse-ld=lld-link with msvc

thakis added a comment. In D69760#1734524 , @thakis wrote: > We currently do cross builds of chrome/win on linux, and this breaks that. > > As far as I know, no linker other than lld-link is able to write PDB files, > so when targeting windows-msvc we def

[PATCH] D69781: [analyzer] Add test directory for scan-build

thakis added a comment. If CLANG_INSTALL_SCANBUILD=NO, then this tries to run scan-build from PATH, where it might not exist. Does this need some lit feature to check if scan-build is being copied to the bin dir by the build system? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D69854: [clang-format] [RELAND] Remove the dependency on frontend

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Thanks! One comment (the stale pointer one) and a more general suggestion (potentially for a future change) below, else I think this is good to go. Comment at: clang/tools/c

[PATCH] D69781: [analyzer] Add test directory for scan-build

thakis added a comment. Also, you probably need to make clang/test/CMakeLists.txt depend on the install target, to make sure scan-build is in bin/. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69781/new/ https://reviews.llvm.org/D69781 ___

[PATCH] D68391: [RISCV] Improve sysroot computation if no GCC install detected

thakis added a comment. The test seems to fail on Windows, could you take a look and revert if it takes a while to fix? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68391/new/ https://reviews.llvm.org/D68391 ___

[PATCH] D68391: [RISCV] Improve sysroot computation if no GCC install detected

thakis added a comment. nvm, rnk just got to it in https://github.com/llvm/llvm-project/commit/f37b5c800e150ad915c4e0571edd2c92c0160d89 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68391/new/ https://reviews.llvm.org/D68391 ___

[PATCH] D68391: [RISCV] Improve sysroot computation if no GCC install detected

thakis added a comment. Sorry, meant to also paste a link to a failing build: http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/11994/steps/stage%201%20check/logs/stdio Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68391/new/ https:/

[PATCH] D69921: [clang-format] refactor the use of the SMDiagnostics in replacement warnings

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Looks great, thanks! Do you remember where you got the original code from? Would this work there too? Or does that not use the llvm diag classes? CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D70340: Add a key method to Sema to optimize debug info size

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. I don't see any reason not to do this. What's there to discuss? I'm probably missing something obvious. dblaikie added anchor functions in many places a while ago (but iirc for vtables, not d

[PATCH] D70340: Add a key method to Sema to optimize debug info size

thakis added a comment. PS: nice find! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70340/new/ https://reviews.llvm.org/D70340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llv

[PATCH] D69360: [NFC] Refactor representation of materialized temporaries

thakis added a comment. Reverted in c9276fbfdf0c7caf1576b2 for now. Please watch the bots after landing things, and revert if something breaks and it takes you a while to fix. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D70340: Add a key method to Sema to optimize debug info size

thakis added a comment. In D70340#1748975 , @rnk wrote: > In D70340#1748712 , @ > > > > > > I guess I was thinking about enabling this only in +asserts builds, so we pay > zero overhead in release builds. I was als

[PATCH] D69934: [clangd] Implement rename by using SelectionTree and findExplicitReferences.

thakis added a comment. The test fails on Windows: http://45.33.8.238/win/2544/step_7.txt Failing Tests (2): Clangd Unit Tests :: ./ClangdTests.exe/RenameTest.Renameable Clangd Unit Tests :: ./ClangdTests.exe/RenameTest.WithinFileRename I'm guessing this needs the -fno-delayed-tem

[PATCH] D67508: [RISCV] support mutilib in baremetal environment

thakis added a comment. The test fails on Windows: http://45.33.8.238/win/2576/step_6.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67508/new/ https://reviews.llvm.org/D67508 ___ cfe-commits mailin

[PATCH] D67508: [RISCV] support mutilib in baremetal environment

thakis added a comment. Reverted in 3de7cc9fc01c8 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67508/new/ https://reviews.llvm.org/D67508 ___

[PATCH] D70481: [clangd] Fix a crash in expected types

thakis added a comment. This broke clangd tests on Windows: http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/12324 We now have clangd tests running on main llvm bots (namely, on http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/). Do they not send mail? Repository: rG

[PATCH] D70481: [clangd] Fix a crash in expected types

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

[PATCH] D70440: [Driver] Use VFS to check if sanitizer blacklists exist

thakis added a comment. Do we need to add a dep on Frontend to DriverTests here? That's a heavy dependency (it pulls in Sema etc). If it is needed, maybe the test is in the wrong binary? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70440/new/ ht

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

thakis added a comment. ps: The test fails for me, probably because I don't have wasm-opt on PATH? How does this work on the regular bots? Do they all have emscripten installed? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70500/new/ https://revi

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

thakis reopened this revision. thakis added a comment. This revision is now accepted and ready to land. Please don't add code to the driver that runs programs off PATH. Nothing else does this. If you need to run an external program, look for it next to the compiler, like we do for gas with -fno-

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

thakis added inline comments. Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:137 + getFilePaths().push_back(getDriver().SysRoot + "/lib/" + MultiarchTriple + + "/llvm-lto"); +} sunfish wrote: > sbc100 wrote: > > Is

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

thakis added a comment. In D70500#1757738 , @thakis wrote: > ps: The test fails for me, probably because I don't have wasm-opt on PATH? > How does this work on the regular bots? Do they all have emscripten installed? Looks like it might fail with LLVM_A

[PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

thakis added a comment. There's still one failing test on Windows after the fix attempt: http://45.33.8.238/win/3052/step_6.txt Please take a look and revert if it's not an easy fix. (And please watch bots after committing stuff.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D70512: [clangd] Rethink how SelectionTree deals with macros and #includes.

thakis added a comment. hover.test is failing on Mac: http://45.33.8.238/mac/3308/step_7.txt Please take a look, and revert if it takes a while. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70512/new/ https://reviews.llvm.org/D70512 __

[PATCH] D70512: [clangd] Rethink how SelectionTree deals with macros and #includes.

thakis added a comment. That particular bot does GN builds, but these test failures repro in the cmake build for me on both macs I tried. (Remember that the official mac bots are on greendragon, not on buildbot -- I'd guess it shows there too.) Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D70512: [clangd] Rethink how SelectionTree deals with macros and #includes.

thakis added a comment. Mac is happy now, but it fails to build on Windows: http://45.33.8.238/win/3327/step_4.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70512/new/ https://reviews.llvm.org/D70512

[PATCH] D69298: [clangd] Define out-of-line initial apply logic

thakis added a comment. ApplyTest fails on Windows; probably the usual delayed template parsing thing: http://45.33.8.238/win/3368/step_7.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69298/new/ https://reviews.llvm.org/D69298 _

[PATCH] D70857: [llvm][Support] Take in CurrentDirectory as a parameter in ExpandResponseFiles

thakis added a comment. Test fails on Windows: http://45.33.8.238/win/3379/step_10.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70857/new/ https://reviews.llvm.org/D70857 ___ cfe-commits mailing l

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

thakis added a comment. The tests fail on Windows, http://45.33.8.238/win/3405/step_6.txt Please take a look, and if it takes a while to fix please revert while you investigte. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62731/new/ https://revi

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

thakis added a comment. > It appears that only the 1st failure there is the fault of this patch. The > 2nd seems to have come from some openmp patch (that didn't consider dso_local > on windows). > > The first (fpconstrained.cpp) likely just needs the check-lines to NOT > explicitly say the %

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

thakis added a comment. In D69825#1768111 , @arphaman wrote: > @aganea Please disable the new behavior for the Darwin platform. We rely on > the fact that Clang `-cc1` processes crash to report crashes using system's > crash reporting infrastructure. W

<    10   11   12   13   14   15   16   17   18   19   >