[PATCH] D119130: [clangd] NFC: Move stdlib headers handling to Clang

thakis added a comment. Or, alternatively, maybe Recognizer could stay in clangd (or move to a separate lib); looks like that's the only thing in StandardLibrary.h that needs `clang/AST/Decl.h`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119130

[PATCH] D135154: Keep inherited dllimport/export attrs for explicit specialization of template class member functions

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. lg Comment at: clang/lib/Sema/SemaDecl.cpp:7047 +NewDecl->getLocation(), +diag::err_attribute_dllimport_function_specialization_definition); +

[PATCH] D135245: [clang][Tooling] Move STL recognizer to its own library

thakis added a comment. Thanks! Comment at: clang/lib/Tooling/Inclusions/STL/CMakeLists.txt:2 +add_clang_library(clangToolingInclusionsSTL + StandardLibrary.cpp + kadircet wrote: > sammccall wrote: > > This means the implementation files and the header files h

[PATCH] D135245: [clang][Tooling] Move STL recognizer to its own library

thakis added a comment. (Alternatively, it looks like the only client is clangd, so maybe the file could just live in there instead of being in a dedicated library with a single client.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135245/new/ h

[PATCH] D135356: [Format] Fix crash when hitting eof while lexing JS template string

thakis added a comment. This breaks tests: http://45.33.8.238/linux/88341/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/D135356/new/ https://reviews.llvm.org/D135356

[PATCH] D135389: [Clang] Emit a warning for ambiguous joined '-o' arguments

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

[PATCH] D135526: [clang][LoongArch] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for LoongArch

thakis added a comment. This seems to break check-clamg Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135526/new/ https://reviews.llvm.org/D135526 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

thakis added a comment. Looks like this breaks check-clangd everywhere, eg http://45.33.8.238/linux/89237/step_9.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127403/new/ https://reviews.llvm.org/D127403 __

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

thakis added a comment. In D127403#3863641 , @nridge wrote: > Landed https://reviews.llvm.org/rGc93430bae4fc Still failing with that: http://45.33.8.238/linux/89240/step_9.txt Time to revert to green? Things have been broken for a few hours now. Maybe

[PATCH] D137838: [Support] Move TargetParsers to new component

thakis added a comment. Thanks for the heads-up. I updated the GN build in 4ac51dd53d93b8dd18c58093766483c657fe3a08 and 2aa998d22fe09191cd6c1b697e373266c1131502

[PATCH] D139266: Headers: tweak inclusion condition for stdatomic.h

thakis added a comment. We think this is incorrect, see comments here: https://boringssl-review.googlesource.com/c/boringssl/+/55845/3/crypto/refcount_c11.c#18 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139266/new/ https://reviews.llvm.org/D139

[PATCH] D139266: Headers: tweak inclusion condition for stdatomic.h

thakis added a comment. (and https://bugs.chromium.org/p/chromium/issues/detail?id=1402885#c3) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139266/new/ https://reviews.llvm.org/D139266 ___ cfe-commits m

[PATCH] D140543: [clang-format] Add an option to format integer literal separators

thakis added a comment. This seems to break tests everywhere, eg http://45.33.8.238/linux/95289/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/D140543/new/ https://reviews

[PATCH] D140543: [clang-format] Add an option to format integer literal separators

thakis added a comment. In D140543#4016181 , @owenpan wrote: > In D140543#4016156 , @thakis wrote: > >> This seems to break tests everywhere, eg >> http://45.33.8.238/linux/95289/step_7.txt >> >> Please take a lo

[PATCH] D140543: [clang-format] Add an option to format integer literal separators

thakis added a comment. It was also failing on the official bots: https://lab.llvm.org/buildbot/#/builders/109/builds/54291 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140543/new/ https://reviews.llvm.org/D140543

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

thakis added a comment. Could we make this an optional opt-in maybe? I imagine for the majority of the tools, this has no measurable effect, but it currently makes the Opt library harder to use for every client. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

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

thakis added a comment. I'd only use it in clang tbh. Clang uses many orders of magnitudes more opts than all other tools, and even there it's not a _huge_ win. (2% on empty file is very nice! But most of the time, people don't compile empty files, and I'm guessing on non-empty files this is a

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

thakis added a comment. The `clang` binary should not depend on `ASTMatchers`. I see this isn't new in this patch, but `ASTMatchers` should only be used from clang-tidy, not from the compiler itself. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[PATCH] D51948: [analyzer] Add ExprMutationAnalyzer

thakis added a comment. Herald added subscribers: manas, steakhal, ASDenysPetrov, dkrupp, donat.nagy, baloghadamsoftware. Herald added a project: All. Old review, but `clang` binary should not depend on ASTMatchers. It's fine if ExprMutationAnalyzer isn't used within clang, but then it should g

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

thakis added a comment. Unrelatedly, this doesn't build on Windows: http://45.33.8.238/win/72750/step_4.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138329/new/ https://reviews.llvm.org/D138329 ___

[PATCH] D140694: [clang][dataflow] Only model struct fields that are used in the function being analyzed.

thakis added a comment. This breaks tests on Mac: http://45.33.8.238/macm1/52112/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/D140694/new/ https://reviews.llvm.org/D14069

[PATCH] D137517: [TargetParser] Generate the defs for RISCV CPUs using llvm-tblgen.

thakis added inline comments. Comment at: llvm/include/llvm/TargetParser/CMakeLists.txt:3 +tablegen(LLVM RISCVTargetParserDef.inc -gen-riscv-target-def -I ${CMAKE_SOURCE_DIR}/lib/Target/RISCV/) +add_public_tablegen_target(RISCVTargetParserTableGen) + All other t

[PATCH] D120175: [Driver] Re-run lld with --reproduce when it crashes

thakis added a comment. I noticed that when using `-fcrash-diagnostics=all` and the linker crashes, clang creates both preprocessor and linker repros: clang: note: diagnostic msg: /var/folders/w6/wpbtszrs7jl9dc9l5qtdkvg0gn/T/main-241332.c clang: note: diagnostic msg: /var/folders/w6/wp

[PATCH] D120175: [Driver] Re-run lld with --reproduce when it crashes

thakis added a comment. > I didn't really make a conscious decision one way or the other. I guess > reproing all jobs leading to a crash somewhat makes sense, though it also > doesn't seem that helpful to get the preprocessed source file. I'll send out > a patch to fix this Great, thanks :) F

[PATCH] D136533: [clang] Fix missing diagnostic of declaration use when accessing TypeDecls through typename access

thakis added a comment. We're happy to switch to any setup that allows us to do a bootstrap build of clang on macOS (ie one where we build clang with whatever host compiler, and then build it again with just-built clang). We've switched to the currently recommended method several times; if ther

[PATCH] D136283: [clang-tools-extra] [clangd] Split huge generated CompletionModel.cpp into smaller files

thakis added inline comments. Comment at: clang-tools-extra/clangd/quality/CompletionModel.cmake:25 + + file(GLOB cpp_files "${output_dir}/${filename}*.cpp") + Can we list them? We try not to use globs in cmakelists.txt files. Repository: rG LLVM Github Mono

[PATCH] D137338: Fix dupe word typos

thakis added inline comments. Herald added a reviewer: njames93. Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:47 // complete architecture list, nor a reasonable subset. The problem is that - // historically the driver driver accepts this and also ties its -march= //

[PATCH] D137838: [RFC][Support] Move TargetParsers to new component

thakis added a comment. This is a gigantic diff. I'd recommend keeping the .h files in the old place for v0 and make them just forwarding headers that include the header at the new location. That way, you don't have to update oodles of include lines in this patch, and it makes it a bit easier t

[PATCH] D137269: [Clang][AArch64][Darwin] Enable GlobalISel by default for Darwin ARM64 platforms.

thakis added a comment. Heads-up: This very visibly broke web page rendering in Chromium (but only when using LTO) (https://crbug.com/1383873) . Not clear yet if this exposed UB on our end or not. One thing that we did notice is that adding `-fno-global-isel` to cflags and ldflags only had an

[PATCH] D137269: [Clang][AArch64][Darwin] Enable GlobalISel by default for Darwin ARM64 platforms.

thakis added a comment. > One thing that we did notice is that adding `-fno-global-isel` to cflags and > ldflags only had an effect when manually blowing away the thinlto cache. > Maybe whatever decides when to invalidate the thinlto cache needs to learn to > invalidate it when the `-global-ise

[PATCH] D137962: [clang][Tooling] Make the filename behaviour consistent

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

[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] 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] 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] 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] D141437: [HIP] Use .hipi as preprocessor output extension

thakis added a comment. I think this breaks tests on Windows: http://45.33.8.238/win/73191/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/D141437/new/ https://reviews.llvm.

[PATCH] D141543: [LinkerWrapper] Emit assembly files from LTO with `save-temps`

thakis added a comment. Looks like this breaks tests on Windows: http://45.33.8.238/win/73276/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/D141543/new/ https://reviews.ll

[PATCH] D141543: [LinkerWrapper] Emit assembly files from LTO with `save-temps`

thakis added a comment. Thanks! Still failing with that though: http://45.33.8.238/win/73278/step_7.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141543/new/ https://reviews.llvm.org/D141543 ___ cfe-

[PATCH] D141543: [LinkerWrapper] Emit assembly files from LTO with `save-temps`

thakis added a comment. cb2f23a9b0de9c6747afb1efdc6e05b4691e0c50 did the trick. Thanks for the fix! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141543/new/ https://reviews.ll

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

thakis added a comment. Thanks for this change :) Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:141 -class WrapperOptTable : public opt::OptTable { +class WrapperOptTable : public opt::GenericOptTable { public: C Comm

[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

[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] 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] 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 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] 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] 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. 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. 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'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. 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] 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] 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] 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] D126215: [analyzer] Deprecate `-analyzer-store region` flag

thakis added a comment. (As long as this builds and passes tests, I don't have an opinion on this change.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126215/new/ https://reviews.llvm.org/D126215 ___

[PATCH] D122258: [MC] Omit DWARF unwind info if compact unwind is present where eligible

thakis added a comment. I think it should work if you put the test in llvm/test/MC/MachO/AArch64 instead of in llvm/test/MC/MachO, because of: % cat llvm/test/MC/MachO/AArch64/lit.local.cfg if not 'AArch64' in config.root.targets: config.unsupported = True Repository: rG LLVM Gith

[PATCH] D126929: Add sanitizer metadata attributes to clang IR gen.

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

[PATCH] D126929: Add sanitizer metadata attributes to clang IR gen.

thakis added a comment. The reland broke fewer tests, but it still broke tests: http://45.33.8.238/win/60026/step_7.txt If you update the patch on here, the presubmit bot should also run windows tests, so you don't have to break main to find out if the patch works. Repository: rG LLVM Githu

[PATCH] D126929: Add sanitizer metadata attributes to clang IR gen.

thakis added a comment. Ping – windows bots are still red. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126929/new/ https://reviews.llvm.org/D126929 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D126929: Add sanitizer metadata attributes to clang IR gen.

thakis added a comment. Thanks for the successful whack-a-moling :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126929/new/ https://reviews.llvm.org/D126929 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D127163: [clang] Add -fsanitize=memtag-globals (no-op).

thakis added a comment. FYI: If you change serialized opts in a def file, you have to bump the pch version, else with builds that don't embed the llvm revision (LLVM_APPEND_VC_REV=OFF) won't notice they have to invalidate pchs. (In this case, I did this in https://github.com/llvm/llvm-project/

[PATCH] D127863: [clang] Dont print implicit forrange initializer

thakis added a comment. Before landing changes, please: - Run tests - Look at the result from the presubmit checks After landing changes, please watch your favorite bot cycle green with it, and pay attention for buildbot email for a while. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D126373: [pseudo-gen] Add -o flag, make --grammar required

thakis created this revision. thakis added a reviewer: hokein. Herald added a subscriber: mgorny. Herald added a project: All. thakis requested review of this revision. Herald added a project: clang-tools-extra. Virtually all LLVM tools accept a `-o` flag, so add one. This will make it possible to

[PATCH] D126373: [pseudo-gen] Add -o flag, make --grammar required

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG788463e72af8: [pseudo-gen] Add -o flag, make --grammar required (authored by thakis). Changed prior to commit: https://reviews.llvm.org/D126373?vs

[PATCH] D120201: [Clang] Extend -gen-reproducer flag

thakis added a comment. Looks like this breaks tests on (at least) Mac and window, see eg http://45.33.8.238/macm1/36198/step_7.txt (passes on my Linux bit though). Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D126597: [clang] Remove `rm` script which is no longer necessary

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126597/new/ https://reviews.llvm.org/D126597 _

[PATCH] D125052: [HLSL] Enable vector types for hlsl.

thakis added a comment. Looks like this breaks tests on Mac/arm: http://45.33.8.238/macm1/36297/step_7.txt Please take a look and revert for now if it takes a while to fix. (Maybe the test just needs an explicit triple?) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D125052: [HLSL] Enable vector types for hlsl.

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

[PATCH] D126397: [pseudo] Fix pseudo-gen usage when cross-compiling

thakis added a comment. Even after this, you still have to explicitly set `-DCMAKE_SYSTEM_NAME=Darwin` when building clang for mac/arm on mac/intel. Given that the host and target systems are both mac, that's a bit weird. llvm-tblgen doesn't need this, it just works as long as you set `LLVM_USE

[PATCH] D126719: [clang-cl] Add support for /kernel

thakis added a comment. https://docs.microsoft.com/en-us/cpp/build/reference/kernel-create-kernel-mode-binary?view=msvc-170 says we should err when `/kernel` is used with various flags (e.g. most `/arch:`) flags. Want to add that too? It also says `/kernel` is passed on to the linker. What doe

[PATCH] D126719: [clang-cl] Add support for /kernel

thakis added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:7620 + else if (Arch == llvm::Triple::x86_64) + UnsupportedArches = {"AVX", "AVX2", "AVX512"}; + Making this list opt-in instead of opt-out (ie "SupportedArchs") seems a bit

[PATCH] D126340: [clang][AIX] add option -mdefault-visibility-export-mapping

thakis added a comment. When you reland this, please reland 4463bd0f89181234e0cef982e21de2e96038f873 at the same time. Else, this breaks tests in some build configurations. Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D112916: Confusable identifiers detection

thakis added a comment. I haven't checked, but this probably doesn't do the right thing in cross builds (eg building a Mac/arm binary on a Mac/Intel machine - i think the gen_confusables binary will likely end up being an arm binary then, and it won't be able to run during the build). See https

[PATCH] D112916: Confusable identifiers detection

thakis added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/ConfusableTable/build_confusable_table.cpp:55 + std::sort(Entries.begin(), Entries.end()); + errs() << "Parsed " << Entries.size() << " Entries\n"; + Could we not print this line? The

[PATCH] D112916: Confusable identifiers detection

thakis added a comment. This also makes clang-tidy crash for fairly normal inputs, see below. Comment at: clang-tools-extra/clang-tidy/misc/Homoglyph.cpp:84 + if (const auto *ND = Result.Nodes.getNodeAs("nameddecl")) { +StringRef NDName = ND->getName(); +auto &Mapped =

[PATCH] D112916: Confusable identifiers detection

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

[PATCH] D126725: [pseudo] rename pseudo-gen -> clang-pseudo-gen. NFC

thakis added a comment. Renaming this seems like a great change to me fwiw :) Comment at: llvm/utils/gn/secondary/clang-tools-extra/pseudo/gen/BUILD.gn:1 -executable("pseudo-gen") { +executable("clang-pseudo-gen") { configs += [ "//llvm/utils/gn/build:clang_code" ] -

[PATCH] D126719: [clang-cl] Add support for /kernel

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Looking pretty good! Comment at: clang/lib/Driver/ToolChains/Clang.cpp:7620 + std::vector Arches = {"IA32"}; + SupportedArches.insert(Arches.begin(), Arches.end()

[PATCH] D109977: LLVM Driver Multicall tool

thakis added inline comments. Comment at: llvm/test/lit.cfg.py:142 tools = [ +ToolSubst('%llvm', FindTool('llvm')), ToolSubst('%lli', FindTool('lli'), post='.', extra_args=lli_args), This causes `llvm-lit: /Users/thakis/src/llvm-project/llvm/utils/lit/

[PATCH] D109977: LLVM Driver Multicall tool

thakis added inline comments. Comment at: llvm/test/lit.cfg.py:142 tools = [ +ToolSubst('%llvm', FindTool('llvm')), ToolSubst('%lli', FindTool('lli'), post='.', extra_args=lli_args), abrachet wrote: > thakis wrote: > > This causes `llvm-lit: > > /Users

[PATCH] D126215: [analyzer] Deprecate `-analyzer-store region` flag

thakis added a comment. Looks like this breaks building clang-tidy: http://45.33.8.238/linux/78232/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/D126215/new/ https://rev

[PATCH] D126067: [analyzer] Deprecate the unused 'analyzer-opt-analyze-nested-blocks' cc1 flag

thakis added a comment. One of your changes broke check-clang: http://45.33.8.238/linux/78236/step_7.txt Please take a look and revert for now if it takes a while to fix. (Please run tests locally before committing.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[PATCH] D126067: [analyzer] Deprecate the unused 'analyzer-opt-analyze-nested-blocks' cc1 flag

thakis added a comment. Reverted in 8406839d1926486de900c7cabeea9f841bd3edf2 for now. See comment on https://reviews.llvm.org/rG90374df15ddc58d823ca42326a76f58e748f20eb. Repository: rG LLVM Github Monorepo CHANGES SINCE

[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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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

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. 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] 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] 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

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