[PATCH] D95534: clang-cl: Invent a /winsysroot concept

This revision was automatically updated to reflect the committed changes. Closed by commit rG82847436e925: clang-cl: Invent a /winsysroot concept (authored by thakis). Herald added a project: clang. Changed prior to commit: https://reviews.llvm.org/D95534?vs=319949&id=320126#toc Repository:

[PATCH] D95575: clang-cl: Accept /std:c11, /std:c17 flags

This revision was automatically updated to reflect the committed changes. Closed by commit rGd087d805acb6: clang-cl: Accept /std:c11, /std:c17 flags (authored by thakis). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95

[PATCH] D95166: Disable rosegment for old Android versions.

thakis added a comment. This breaks us too: https://bugs.chromium.org/p/chromium/issues/detail?id=1172291 Given that this breaks at least 3 distinct build configs, let's revert this for now until we've figured out a path forward. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D95166: Disable rosegment for old Android versions.

thakis added a comment. Landed revert in 1608ba09462d877111230e9461b895f696f8fcb1 . Someone should file a PR to make sure that gets merged to the 12.0 branch. We can then reland on trunk once there are clear instructions for f

[PATCH] D95166: Disable rosegment for old Android versions.

thakis added a comment. Alternatively, there's `LinkerIsLLD` in the driver code, maybe the driver wants to check that in some way (if only to emit a better diag if it's false, I guess) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95166/new/ https

[PATCH] D95739: [ASTMatchers] Add matchers for decomposition decls

thakis added a comment. Looks like this doesn't link on windows: http://45.33.8.238/win/32481/step_4.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95739/new/ https://reviews.llvm.org/D95739 ___ cfe-c

[PATCH] D95739: [ASTMatchers] Add matchers for decomposition decls

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

[PATCH] D95876: [clang-cl] Remove the /fallback option

thakis added a comment. Probably get a short release notes entry too. Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:669 const InputInfoList &Inputs, const ArgList &Args, const char *LinkingOutput) const { ArgStringList CmdArgs; Doesn't this whol

[PATCH] D95166: Disable rosegment for old Android versions.

thakis added a comment. In D95166#2531194 , @danalbert wrote: > In D95166#2530791 , @thakis wrote: > >> Landed revert in 1608ba09462d877111230e9461b895f696f8fcb1 >>

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

thakis added a comment. Hi, this causes a warning in harfbuzz on code that looks like so: UChar decomposed[4]; ... int len = unorm2_getRawDecomposition(...decomposed, ARRAY_LENGTH (decomposed)...) if (len == 1) { U16_GET_UNSAFE (decomposed, 0, *a); where `U16_GET_UNSAFE` looks like

[PATCH] D95166: Disable rosegment for old Android versions.

thakis added a comment. In D95166#2540399 , @danalbert wrote: >> We've since added -fuse-ld=lld to the three CMAKE_*_FLAGS so I think we're >> likely set on our end. > > https://reviews.llvm.org/D76452 not being accepted means that Android > toolchains m

[PATCH] D69322: [hip][cuda] Enable extended lambda support on Windows.

thakis added a comment. Hi, a whole bunch of cuda-related tests failed on windows with this in (http://45.33.8.238/win/32620/summary.html). Did you run check-clang before committing? I reverted this for now in 4874ff02417916cc9ff994b34abcb5e563056546

[PATCH] D80623: WIP: Add an API to simplify setting TraversalKind in clang-tidy matchers

thakis added a comment. This breaks check-clang everywhere: http://45.33.8.238/linux/38812/step_7.txt Please run tests before landing. 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/

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

thakis added a comment. @vabridgers: ping ^ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71714/new/ https://reviews.llvm.org/D71714 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.

[PATCH] D109828: [clang-cl] Add a /diasdkdir flag and make /winsysroot imply it

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG951f362e2560: [clang-cl] Add a /diasdkdir flag and make /winsysroot imply it (authored by thakis). Herald added a project: clang. Changed prior to c

[PATCH] D91944: OpenMP 5.0 metadirective

thakis added a comment. (Reverted for now since mac bots have been broken for 16 hours now.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91944/new/ https://reviews.llvm.org/D91944 ___ cfe-commits maili

[PATCH] D109611: Fix CLANG_ENABLE_STATIC_ANALYZER=OFF building all analyzer source

thakis added a comment. This breaks the build for us: Running cmake -GNinja -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=ON '-DLLVM_ENABLE_PROJECTS=clang;compiler-rt;lld;chrometools;clang-tools-extra' -DLLVM_CHECK_ENABLED_PROJECTS=OFF '-DLLVM_TARGETS_TO_BUILD=AArch64;ARM;Mips;PowerPC;

[PATCH] D109611: Fix CLANG_ENABLE_STATIC_ANALYZER=OFF building all analyzer source

thakis added a comment. Thanks, reverted in 9197834535364efff505580ef940ad41cd293275 for now. Here's a shorter cmake invocation that repros it: `cmake -GNinja -DCLANG_ENABLE_STATIC_ANALYZER=OFF ../llvm-project/llvm '-DLLVM_E

[PATCH] D103938: Diagnose -Wunused-value based on CFG reachability

thakis added a comment. This flags this code from absl: template ::value, int>::type = (GenT{}, 0)> constexpr FlagDefaultArg DefaultArg(int) { return {FlagDefaultSrc(GenT{}.value), FlagDefaultKind::kOneWord}; } (https://source.chromium.org/chromium/chromium/src/+/main:

[PATCH] D110276: Clean up large copies of binaries copied into temp directories in tests

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/D110276/new/ https://reviews.llvm.org/D110276 ___ c

[PATCH] D110276: Clean up large copies of binaries copied into temp directories in tests

thakis added a comment. Oh, the win failure (https://buildkite.com/llvm-project/premerge-checks/builds/57480#f7da3275-775a-4cf4-9624-d3539dd1f709) might actually be real. Windows doesn't allow deleting files that are still in use, and due to AV or similar, the file might still be in use for a b

[PATCH] D110315: [Sema] Fix a null pointer reference crash.

thakis added a comment. Arm Mac: http://45.33.8.238/macm1/18463/step_7.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110315/new/ https://reviews.llvm.org/D110315 ___ cfe-commits mailing list cfe-comm

[PATCH] D103938: Diagnose -Wunused-value based on CFG reachability

thakis added a comment. In D103938#3013503 , @thakis wrote: > This flags this code from absl: Did you see this comment? Looks like this relanded, but it still warns there even though it has an effect there I think (?) Repository: rG LLVM Github Mono

[PATCH] D110315: [Sema] Fix a null pointer reference crash.

thakis added a comment. Fixed in https://github.com/llvm/llvm-project/commit/8ecf3660f2de3b88d10db0fd52d0bc80bda33dcc , thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110315/new/ https://reviews.llvm.org/D110315 __

[PATCH] D110128: [Driver] Correctly handle static C++ standard library

thakis added a comment. This breaks our packaging builders too: https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket/8835201798501639249/+/u/package_clang/stdout?format=raw Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110128/new/ ht

[PATCH] D110128: [Driver] Correctly handle static C++ standard library

thakis added a comment. Note that stage 2 fails, where we're supposed to use lld. So I think this is probably some compiler-rt test config problem you need to sort out, and not a fundamental problem with the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

[PATCH] D110128: [Driver] Correctly handle static C++ standard library

thakis added a comment. Re-reverted for now in 6ece82e9006d16b7fba7660ce09b2c62ab8460fa . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110128/new/ https://reviews.llvm.org/D1101

[PATCH] D110458: [clang] Put original flags on 'Driver args:' crash report line

thakis created this revision. thakis added a reviewer: hans. thakis requested review of this revision. We used to put the canonical spelling of flags after alias processing on that line. For clang-cl in particular, that meant that we put flags on that line that the clang-cl driver doesn't even acc

[PATCH] D110458: [clang] Put original flags on 'Driver args:' crash report line

thakis marked an inline comment as done. thakis added a comment. Thanks! Comment at: clang/test/Driver/crash-report-clang-cl.c:2 +// RUN: rm -rf %t +// RUN: mkdir %t + hans wrote: > These always make me thing the test should "requires: shell", but I think for

[PATCH] D110458: [clang] Put original flags on 'Driver args:' crash report line

thakis updated this revision to Diff 375232. thakis added a comment. add comment CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110458/new/ https://reviews.llvm.org/D110458 Files: clang/lib/Driver/Driver.cpp clang/test/Driver/crash-report-clang-cl.c Index: clang/test/Driver/crash-

[PATCH] D110458: [clang] Put original flags on 'Driver args:' crash report line

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG63bb2d585e97: [clang] Put original flags on 'Driver args:' crash report line (authored by thakis). Herald added a project: clang. Repository: rG L

[PATCH] D110386: [clangd] Refactor IncludeStructure: use File (unsigned) for most computations

thakis added a comment. Looks like this breaks tests on Windows: http://45.33.8.238/win/45971/step_9.txt PTAL! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110386/new/ https://reviews.llvm.org/D110386

[PATCH] D110386: [clangd] Refactor IncludeStructure: use File (unsigned) for most computations

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

[PATCH] D110635: [clang] Let PPCallbacks::PragmaWarning() pass specifier as enum instead of string

thakis created this revision. thakis added a reviewer: hans. thakis added a project: clang. Herald added subscribers: shchenz, kbarton, nemanjai. thakis requested review of this revision. Herald added a project: clang-tools-extra. https://reviews.llvm.org/D110635 Files: clang-tools-extra/clang-

[PATCH] D109967: Simplify handling of builtin with inline redefinition

thakis added a comment. This breaks tests on windows and macOS eg http://45.33.8.238/win/46075/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/D109967/new/ https://reviews

[PATCH] D109967: Simplify handling of builtin with inline redefinition

thakis added a comment. For macOS, see eg http://45.33.8.238/macm1/18806/step_7.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109967/new/ https://reviews.llvm.org/D109967 ___ cfe-commits mailing list

[PATCH] D110668: [clang-cl] Accept `#pragma warning(disable : N)` for some N

thakis created this revision. thakis added a reviewer: hans. thakis added a project: clang. Herald added subscribers: dexonsmith, dang, mgorny. thakis requested review of this revision. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. clang-cl maps /wd to -Wno-flags for a

[PATCH] D110635: [clang] Let PPCallbacks::PragmaWarning() pass specifier as enum instead of string

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5cf060614097: [clang] Let PPCallbacks::PragmaWarning() pass specifier as enum instead of… (authored by thakis). Repository: rG LLVM Github Monorep

[PATCH] D110364: [clang] Rework dontcall attributes

thakis added a comment. Looks like this breaks tests on windows: http://45.33.8.238/win/46089/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/D110364/new/ https://reviews.l

[PATCH] D110668: [clang-cl] Accept `#pragma warning(disable : N)` for some N

thakis updated this revision to Diff 375749. thakis added a comment. some clang-format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110668/new/ https://reviews.llvm.org/D110668 Files: clang/include/clang/Basic/CLWarnings.h clang/include/clang/Driver/Options.td clang/lib/Basic/CL

[PATCH] D110668: [clang-cl] Accept `#pragma warning(disable : N)` for some N

thakis updated this revision to Diff 375750. thakis added a comment. tweak CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110668/new/ https://reviews.llvm.org/D110668 Files: clang/include/clang/Basic/CLWarnings.h clang/include/clang/Driver/Options.td clang/lib/Basic/CLWarnings.cpp

[PATCH] D110276: Clean up large copies of binaries copied into temp directories in tests

thakis added a comment. Softlinks don't work on Windows, that's why these tests use copies. clang_f_opts.c sadly started flaking almost immediately after this went in: http://45.33.8.238/win/46115/step_7.txt So I fear the Windows file system can't handle this patch as-is. Maybe we can use `rm

[PATCH] D103938: Diagnose -Wunused-value based on CFG reachability

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

[PATCH] D110668: [clang-cl] Accept `#pragma warning(disable : N)` for some N

thakis added inline comments. Comment at: clang/lib/Basic/CLWarnings.cpp:17 + switch (CLWarningID) { + case 4005: return "macro-redefined"; + case 4018: return "sign-compare"; hans wrote: > Would it be possible to reference the DiagGroup symbolically here some

[PATCH] D110668: [clang-cl] Accept `#pragma warning(disable : N)` for some N

thakis updated this revision to Diff 375891. thakis added a comment. Herald added subscribers: usaxena95, arphaman. tblgen group IDs Done, please take a look. We now have a `diag::Group` enum. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110668/new/ https://reviews.llvm.org/D110668 F

[PATCH] D110668: [clang-cl] Accept `#pragma warning(disable : N)` for some N

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. thakis marked an inline comment as done. Closed by commit rGb2de52bec17b: [clang-cl] Accept `#pragma warning(disable : N)` for some N (authored by thakis). Changed pri

[PATCH] D110276: Clean up large copies of binaries copied into temp directories in tests

thakis added a comment. It's my personal bot. It doesn't send email. IMHO the problem of how to make bots send emails hasn't been solved (they either send too much or too little), so I'm not pretending that it has been ;) Sadly the test is still failing after the `-f` change: http://45.33.8.23

[PATCH] D110770: [clang] Remove duplication in types::getCompilationPhases()

thakis created this revision. thakis added a reviewer: hans. thakis added a project: clang. thakis requested review of this revision. Call Driver::getFinalPhase() instead of duplicating it. https://reviews.llvm.org/D65993 added the duplication, then 02e35832c301e

[PATCH] D110783: [clang] Make crash reproducer work with clang-cl

thakis created this revision. thakis added a reviewer: hans. thakis added a project: clang. thakis requested review of this revision. When clang crashes, it writes a standalone source file and shell script to reproduce the crash. The Driver used to set `Mode = CPPMode` in generateCompilationDiagn

[PATCH] D110770: [clang] Remove duplication in types::getCompilationPhases()

thakis added inline comments. Comment at: clang/include/clang/Driver/Phases.h:25 IfsMerge, -LastPhase = IfsMerge, }; hans wrote: > Any reason not to keep the LastPhase alias? I found it more confusing than helpful. `Link` and `LastPhase` both start wi

[PATCH] D110770: [clang] Remove duplication in types::getCompilationPhases()

thakis added a comment. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110770/new/ https://reviews.llvm.org/D110770 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110770: [clang] Remove duplication in types::getCompilationPhases()

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfa32fd3bf7c0: [clang] Remove duplication in types::getCompilationPhases() (authored by thakis). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D110783: [clang] Make crash reproducer work with clang-cl

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8dfbe9b0aeb6: [clang] Make crash reproducer work with clang-cl (authored by thakis). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D110668: [clang-cl] Accept `#pragma warning(disable : N)` for some N

thakis added a comment. Thanks for the revert. The problem was that this `continue` was in the wrong spot: diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 9450e8b154c5..369c12aea523 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b

[PATCH] D110276: Clean up large copies of binaries copied into temp directories in tests

thakis added a comment. In D110276#3032448 , @tejohnson wrote: > In D110276#3031133 , @thakis wrote: > >> It's my personal bot. It doesn't send email. IMHO the problem of how to make >> bots send emails hasn't be

[PATCH] D110668: [clang-cl] Accept `#pragma warning(disable : N)` for some N

thakis added a comment. In D110668#3034576 , @xbolva00 wrote: > Please next time give a bit more time to potential reviewers / other folks > outside your org. The whole lifecycle of this patch (posted - landed) took < > 24h. Is there anything wrong wit

[PATCH] D110783: [clang] Make crash reproducer work with clang-cl

thakis added a comment. In D110783#3035158 , @dyung wrote: > Hi, the test you modified Driver/crash-report.cpp is failing on the PS4 bot > after your change. Can you take a look? > > https://lab.llvm.org/buildbot/#/builders/139/builds/10939 Hopefully be

[PATCH] D110783: [clang] Make crash reproducer work with clang-cl

thakis added a comment. > Hopefully better after ec4a82286674c44c9216e9585235b0fa5df4ae9f > Your bot cycled green after that change: https://lab.llvm.org/buildbot/#/builders/139/builds/10976 Repository: rG LLVM Github Mon

[PATCH] D110668: [clang-cl] Accept `#pragma warning(disable : N)` for some N

thakis added a comment. > What I mean, for example: > > def UnusedParameter : DiagGroup<"unused-parameter", 4100>; That's an interesting idea. Given how seldom this is used, I weakly prefer having actual code for this though: It makes it easy to see all of those mapped flags, and it keeps ra

[PATCH] D110668: [clang-cl] Accept `#pragma warning(disable : N)` for some N

thakis added a comment. Thanks for the feedback. I'll work on a follow-up that implements that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110668/new/ https://reviews.llvm.org/D110668 ___ cfe-commits

[PATCH] D95204: [lld-macho] Switch default to new Darwin backend

thakis added inline comments. Comment at: lld/tools/lld/lld.cpp:74 .CasesLower("ld64", "ld64.lld", "darwin", Darwin) - .CasesLower("darwinnew", "ld64.lld.darwinnew", DarwinNew) .Default(Invalid); very nit: You could keep darwinnew around for a

[PATCH] D95534: clang-cl: Invent a /winsysroot concept

thakis added a comment. In D95534#2534510 , @smeenai wrote: > I saw this on LLVM Weekly, and I like it a lot :) > > Now if only Windows could case-correct their SDKs so we didn't need VFS > overlays and symlinks for the linker... We use ciopfs for this,

[PATCH] D95534: clang-cl: Invent a /winsysroot concept

thakis added a comment. > We use ciopfs for this, > https://source.chromium.org/chromium/chromium/src/+/master:build/vs_toolchain.py;l=485?q=ciopfs > That code has worked without changes for the last 2.5 years. (ciopfs is user-space, so it can be set up transparently by a normal post-pull hook

[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.rv' instead of explicitly emitting retainRV or claimRV calls in the IR

thakis added a comment. This makes clang assert for some inputs. See https://bugs.chromium.org/p/chromium/issues/detail?id=1175886#c10 for a repro. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92808/new/ https://reviews.llvm.org/D92808 _

[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.rv' instead of explicitly emitting retainRV or claimRV calls in the IR

thakis added a comment. Here's a reduced repro: thakis@thakis:~/src/llvm-project$ cat GREYAssertDefaultConfiguration-9e1f3b.reduced.m inline id a() {} void b() { a(); a(); } thakis@thakis:~/src/llvm-project$ out/gn/bin/clang -cc1 -cc1 -triple arm64-apple-ios12.2.0 -Oz -fobjc-a

[PATCH] D96049: [Timer] On macOS count number of executed instructions

thakis added inline comments. Comment at: llvm/include/llvm/Config/config.h.cmake:356 +#cmakedefine HAVE_PROC_PID_RUSAGE 1 + Any reason to use `#cmakedefine` instead of `cmakedefine01` here? This type of check usually uses `#cmakedefine01` in this file. Repo

[PATCH] D96049: [Timer] On macOS count number of executed instructions

thakis added inline comments. Comment at: llvm/CMakeLists.txt:656 + list(APPEND CMAKE_REQUIRED_LIBRARIES proc) +endif() + also, any reason to not do this in `llvm/cmake/config-ix.cmake` where all the other checks like this are? Repository: rG LLVM Github Mo

[PATCH] D96049: [Timer] On macOS count number of executed instructions

thakis added inline comments. Comment at: llvm/include/llvm/Config/config.h.cmake:356 +#cmakedefine HAVE_PROC_PID_RUSAGE 1 + thakis wrote: > Any reason to use `#cmakedefine` instead of `cmakedefine01` here? This type > of check usually uses `#cmakedefine01` in

[PATCH] D96507: [clangd] Move command handlers into a map in ClangdLSPServer. NFC

thakis added a comment. Looks like this doesn't build on Windows: http://45.33.8.238/win/33161/step_4.txt (clang-cl + msvc stdlib) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96507/new/ https://reviews.llvm.org/D96507 __

[PATCH] D96886: [Driver] Clean up some Separate form options

thakis accepted this revision. thakis added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Driver/Options.td:1104 Group, Flags<[CC1Option, CC1AsOption, CoreOption]>, HelpText<"The compilation directory to embed in the d

[PATCH] D96816: [ObjC] Encode pointers to C++ classes as "^v" if the encoded string would otherwise include template specialization types

thakis added a comment. Very cool change, I wanted something like this for a long time :) It seems to break Index/preamble-reparse-changed-module.m on some platforms: http://45.33.8.238/linux/39727/step_7.txt http://45.33.8.238/macm1/3788/step_6.txt Does the failure make sense to you? Reposit

[PATCH] D96971: Allow but ignore `-Wreturn-std-move-in-c++11`

thakis added a comment. Since the fix for this seems to be "just remove the flag", and since the flag can be removed while on the old clang version without problems, it's possible to migrate to the new way without having to do this at the same time as a compiler update already. So I'm not sure

[PATCH] D96816: [ObjC] Encode pointers to C++ classes as "^v" if the encoded string would otherwise include template specialization types

thakis added a comment. I ssh'd into one of the failing boxes and ran the failing command without piping it into filecheck. Here's the output: thakis@dotc:~/src/llvm-project$ env CINDEXTEST_EDITING=1 CINDEXTEST_EXECUTE_COMMAND="cp /usr/local/google/home/thakis/src/llvm-project/clang/test/Ind

[PATCH] D96816: [ObjC] Encode pointers to C++ classes as "^v" if the encoded string would otherwise include template specialization types

thakis added a comment. #3 0x778ee662 in __GI___assert_fail (assertion=0x90165c "idx < size()", file=0xc60efe "../../llvm/include/llvm/ADT/SmallVector.h", line=281, function=0xa8780d "llvm::SmallVectorTemplateCommon::const_reference llvm::SmallVectorTemplateCommon::opera

[PATCH] D96816: [ObjC] Encode pointers to C++ classes as "^v" if the encoded string would otherwise include template specialization types

thakis added a comment. See maybe also https://reviews.llvm.org/D73202 . Do we need to bump serialization::VERSION_MINOR ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96816/new/ https://reviews.llvm.org/D96816 __

[PATCH] D96825: [AArch64] Adding Neon Polynomial vadd Intrinsics

thakis added a comment. Looks like this breaks check-clang: http://45.33.8.238/win/33493/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/D96825/new/ https://reviews.llvm.or

[PATCH] D96816: [ObjC] Encode pointers to C++ classes as "^v" if the encoded string would otherwise include template specialization types

thakis added a comment. In D96816#2572431 , @thakis wrote: > See maybe also https://reviews.llvm.org/D73202 . Do we need to bump > serialization::VERSION_MINOR ? I gave Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D96816: [ObjC] Encode pointers to C++ classes as "^v" if the encoded string would otherwise include template specialization types

thakis added a comment. In D96816#2572431 , @thakis wrote: > See maybe also https://reviews.llvm.org/D73202 . Do we need to bump > serialization::VERSION_MINOR ? I gave that a try in http://reviews.llvm.org/rG063236646849564094f5fcfc947ad36dba0efedb . L

[PATCH] D96825: [AArch64] Adding Neon Polynomial vadd Intrinsics

thakis added a comment. Thanks for the revert! FWIW the failure repros for me locally if I just do `ninja check-clang` (as long as I have the aarch64 target enabled, which it is by default in the cmake build). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D96816: [ObjC] Encode pointers to C++ classes as "^v" if the encoded string would otherwise include template specialization types

thakis added a comment. In D96816#2572589 , @dexonsmith wrote: > In D96816#2572552 , @ahatanak wrote: > >> I still don't understand why the test was failing, but did we have to bump >> the version because a new Lan

[PATCH] D96515: [OpenCL] Add builtin declarations by default.

thakis added a comment. Looks like this breaks tests: http://45.33.8.238/linux/39988/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/D96515/new/ https://reviews.llvm.org/D9

[PATCH] D96515: [OpenCL] Add builtin declarations by default.

thakis added a comment. In D96515#2578666 , @Anastasia wrote: > In D96515#2578629 , @thakis wrote: > >> Looks like this breaks tests: http://45.33.8.238/linux/39988/step_8.txt >> >> Please take a look, and revert fo

[PATCH] D97098: [Utils] Add an option to specify number of cores to use in creduce-clang-crash.py

thakis added a comment. In D97098#2579620 , @zequanwu wrote: > Address comment, using `max(4, multiprocessing.cpu_count() / 2)` because > `os.cpu_count` is not available in python2. We require py3 these days, so it's ok to assume it. Repository: rG L

[PATCH] D97098: [Utils] Add an option to specify number of cores to use in creduce-clang-crash.py

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

[PATCH] D96638: [Driver][Windows] Support per-target runtimes dir layout for profile instr generate

thakis added a comment. This broke check-clang on all platforms, e.g. here: http://45.33.8.238/linux/40161/step_7.txt http://45.33.8.238/macm1/4018/step_6.txt http://45.33.8.238/win/33805/step_7.txt Please run tests before committing. (It also breaks check-clang in a local cmake build. The offi

[PATCH] D96638: [Driver][Windows] Support per-target runtimes dir layout for profile instr generate

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

[PATCH] D52386: [Lexer] Add udefined_behavior_sanitizer feature

thakis added a comment. Looks like this landed in b32d40417e25bad79e9b5bc0f0a29d7ba0222ad9 but the review feedback was never addressed. With a more fine-grained thing, we could use this to add a compile-time error to libcxxabi for D97323 . Repository: rL LLV

[PATCH] D97437: Rewrite MSVC toolchain discovery with VFS

thakis added a comment. Re test: Maybe some combination of `-ivfsoverlay` (as a `/clang:` flag I suppose) and `/winsysroot` can be used to test this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97437/new/ https://reviews.llvm.org/D97437 ___

[PATCH] D97437: Rewrite MSVC toolchain discovery with VFS

thakis added a comment. (aside: Do you know if the sysroot vs clangd stuff works on non-win? I happened to see https://bugs.llvm.org/show_bug.cgi?id=27909) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97437/new/ https://reviews.llvm.org/D97437 _

[PATCH] D103771: [clang][msvc] Define _HAS_STATIC_RTTI to 0, when compiling with -fno-rtti

thakis added a comment. Some background https://bugs.chromium.org/p/chromium/issues/detail?id=996675 : Back then I thought that MSVC switched to /Zc:twoPhase , but that turned out to not be true. So we never switched the default. I think this patch here still makes sense, but I admit I don't ha

[PATCH] D103772: [clang-cl] Reenable /Zc:twoPhase by default if targetting MSVC 2017 Update 3 or newer

thakis added a comment. It was reverted due to build failures, but it didn't reland because it's not the default in cl.exe after all. So we shouldn't make it the default in clang-cl either. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103772/new/ https://reviews.llvm.org/D103772 ___

[PATCH] D103773: [clang-cl] Add /permissive and /permissive-

thakis added a comment. I think adding `/permissive-` to make things more conforming is great. The docs say "Starting in Visual Studio 2019 version 16.8, the /std:c++latest option implicitly sets the /permissive- option." so maybe we should do that too (doesn't have to be in this patch). Since

[PATCH] D103773: [clang-cl] Add /permissive and /permissive-

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Since it's motivated by a concrete file, this makes sense to me. Wait maybe a day or so to see if anyone objects, but I think this (and dependencies) are good. Do you have commit access? Re

[PATCH] D103771: [clang][msvc] Define _HAS_STATIC_RTTI to 0, when compiling with -fno-rtti

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. (same comment as on https://reviews.llvm.org/D103773) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103771/new/ https://reviews.llvm.org/D103771 _

[PATCH] D103184: [AArch64] handle -Wa,-march=

thakis added a comment. This breaks tests on arm macs: http://45.33.8.238/macm1/10931/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/D103184/new/ https://reviews.llvm.org/D

[PATCH] D103184: [AArch64] handle -Wa,-march=

thakis added a comment. In D103184#2803691 , @nickdesaulniers wrote: > In D103184#2803513 , @thakis wrote: > >> This breaks tests on arm macs: http://45.33.8.238/macm1/10931/step_7.txt > > > >>> clang: error: the

[PATCH] D103184: [AArch64] handle -Wa,-march=

thakis added a comment. FWIW the failure goes away locally if I revert this change here, so it's definitely due to this change here. Things have been red for a while, probably good to revert while you investigate by this point. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D103564: [NFC][compiler-rt][hwasan] Move allocation functions into their own file

thakis added a comment. this breaks tests: http://45.33.8.238/linux/48401/step_10.txt ` // CHECK: #0 {{.*}} in {{.*}}free{{.*}} {{.*}}hwasan_interceptors.cpp` vs ` 13: #0 0x5647a467e9a0 in free ../../compiler-rt/lib/hwasan/hwasan_allocation_functions.cpp:75:3 ` need to update the p

[PATCH] D103934: clang/darwin: use response files with ld64

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

[PATCH] D103184: [AArch64] handle -Wa,-march=

thakis added a comment. In D103184#2804241 , @jcai19 wrote: > In D103184#2803768 , @thakis wrote: > >> FWIW the failure goes away locally if I revert this change here, so it's >> definitely due to this change her

<    6   7   8   9   10   11   12   13   14   15   >