[PATCH] D56273: Validate -add-plugin arguments.

thakis marked an inline comment as done. thakis added inline comments. Comment at: clang/test/Frontend/plugin-unknown.c:3 +// RUN: not %clang_cc1 -add-plugin asdf %s 2>&1 | FileCheck --check-prefix=ADD %s +// REQUIRES: plugins + Probably doesn't even require plu

[PATCH] D56469: [ObjC] Allow the use of implemented unavailable methods from within the @implementation context

thakis added a comment. (This would also help with https://crbug.com/917351) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56469/new/ https://reviews.llvm.org/D56469 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D56446: [Driver] Fix libcxx detection on Darwin with clang run as ./clang

thakis added a comment. …can you land this? :-) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56446/new/ https://reviews.llvm.org/D56446 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/c

[PATCH] D56488: clang-cl: Align help texts for /O1 and O2

thakis created this revision. thakis added a reviewer: hans. Makes it a bit easier to see what exactly the difference is. Also use "same as" instead of "equivalent to", because that's faster to read. https://reviews.llvm.org/D56488 Files: clang/include/clang/Driver/CLCompatOptions.td Index

[PATCH] D56489: clang-cl: Fix help text for /O: '/O2y-' means '/O2 /Oy-', not '/O2 /y-'

thakis created this revision. thakis added a reviewer: hans. https://reviews.llvm.org/D56489 Files: clang/include/clang/Driver/CLCompatOptions.td Index: clang/include/clang/Driver/CLCompatOptions.td === --- clang/include/clang/Dr

[PATCH] D67304: Emit -Wmicrosoft-enum-value warning instead of error in MS ABI

thakis added a comment. We have the old TODO of changing this warning to be emitted at enum use time (e.g. when Foo_Val is compared to 0) instead of declaration time. Maybe that's a better fix. Or is implementing that very involved? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D67246: clang-format: Add support for formatting lambdas with explicit template parameters.

thakis marked 3 inline comments as done. thakis added a comment. Thanks for the thorough review! Indeed, this still gets `[](A &&a){}();` wrong, for the reason you mention. Comment at: clang/lib/Format/TokenAnnotator.cpp:50 + // Skip <...> if present. + if (Left->Previous &&

[PATCH] D67246: clang-format: Add support for formatting lambdas with explicit template parameters.

thakis updated this revision to Diff 219589. thakis added a comment. fix nits CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67246/new/ https://reviews.llvm.org/D67246 Files: clang/lib/Format/TokenAnnotator.cpp clang/lib/Format/UnwrappedLineParser.cpp clang/unittests/Format/Format

[PATCH] D59754: [Sema] Add c++2a designated initializer warnings

thakis added a comment. Another question about this, sorry. Do you know _why_ C++20 is more restrictive than C99 wrt "mixture of designated and non-designated initializers in the same initializer list is a C99 extension"? Is there some interaction with other C++ features that makes the C99 beha

[PATCH] D67246: clang-format: Add support for formatting lambdas with explicit template parameters.

thakis added a comment. Do you think this should land with the comment FIXME for now? It improves formatting of this language feature when that heuristic is not used, and changing the heuristic is independent of the rest of this patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D6724

[PATCH] D67463: [MS] Warn when shadowing template parameters under -fms-compatibility

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Nice! Hopefully this is rare enough that putting it under an existing warning flag won't be too inconvenient. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D67246: clang-format: Add support for formatting lambdas with explicit template parameters.

This revision was automatically updated to reflect the committed changes. Closed by commit rG41f4d68a50be: clang-format: Add support for formatting (some) lambdas with explicit template… (authored by thakis). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D67542: Fix depfile name construction

thakis added inline comments. Comment at: cfe/trunk/lib/Driver/ToolChains/Clang.cpp:6076 + + return Args.MakeArgString(std::string(getBaseInputStem(Args, Inputs)) + ".d"); } Can you use llvm::Twine() instead of std::string() here? Repository: rL LLVM CHAN

[PATCH] D67590: Properly ignore mismatched exception specifiers in MSVC Compat mode.

thakis added a comment. In D67590#1672691 , @zahen wrote: > Is there any interest in supporting the cl.exe flag `/permissive-`? I > considered a hard error on mismatched exception specifier in clang-cl a > feature, not a bug. If msvc compat mode respec

[PATCH] D67843: DisableFormat also now disables SortIncludes

thakis added a comment. Makes sense to me, but krasimir should probably approve this. Please upload patches with lots of context (`git diff -U master`); that makes reviewing on phab easier. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67843/new/ https://re

[PATCH] D67843: DisableFormat also now disables SortIncludes

thakis added a comment. 4. Make it so that if DisableFormat is explicitly set to true and SortIncludes isn't explicitly set, then it disables SortIncludes. Or, put a different way, when DisableFormat is set, set SortIncludes to false at that point. Then an explicit `DisableFormat: true; SortInc

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

thakis added a comment. It looks like bits of this landed recently. I don't know which of the commits it was, so I'm commenting here: The -Wint-in-bool context that clang emits are very useful and have found several bugs in Chromium, with just one false positive (which was in confusing code, so

[PATCH] D67567: [clang-tidy] New check to warn when storing dispatch_once_t in non-static, non-global storage

thakis added a comment. I had a fix, but I was too slow. Here's what you need for the reland: http://codepad.org/3klmw1JV The commit also broke sphinx with this: ` 6.290 [0/1/1] Generating html Sphinx documentation for clang-tools into "/home/buildbot/llvm-build-dir/clang-tools-sphinx-docs

[PATCH] D67567: [clang-tidy] New check to warn when storing dispatch_once_t in non-static, non-global storage

thakis added a comment. In D67567#1685077 , @mwyman wrote: > In D67567#1685036 , @gribozavr wrote: > > > Sorry, I reverted it in r373032 because the test fails on Linux: > > http://lab.llvm.org:8011/builders/clang-

[PATCH] D68132: clang-tidy: Don't repeat list of all checks in three places.

thakis created this revision. thakis added reviewers: beanz, mwyman, gribozavr. Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, ilya-biryukov, mgorny, srhines. Herald added a reviewer: jdoerfert. Instead, put all checks in a cmake variable and reference this. Also, make clangd

[PATCH] D68132: clang-tidy: Don't repeat list of all checks in three places.

thakis added a comment. Thanks! I'll land this, but beanz, if the cmake setup here is weird please shout and I'll try to make it less weird. We have a few other instances of PARENT_SCOPE variables, so maybe this is how cmake likes to do "group of libraries". CHANGES SINCE LAST ACTION https:

[PATCH] D68185: [Diagnostics] Warn when class implements a copy constructor/copy assignment operator, but missing the copy assignment operator/copy constructor

thakis added a comment. Since https://en.cppreference.com/w/cpp/language/rule_of_three says "almost always" (note "almost") I'd say this probably belongs more in clang-tidy territory – I'm guessing false positive rate is very high for this. Can you collect some true / false positive statistics

[PATCH] D65543: [Windows] Autolink with basenames and add libdir to libpath

thakis added a comment. If you strongly feel that this is the right direction, go for it, you're code owner here :) To me, this feels like a regression. This change has no benefit that I can see (at least none that anybody's asked for that I'm aware of – then again I read llvm's bugzilla less

[PATCH] D67592: [Clang] Use -main-file-name for source filename if not set

thakis added a comment. This is failing on my mac like so: FAIL: Clang :: Frontend/stdin-input.c (1 of 1) TEST 'Clang :: Frontend/stdin-input.c' FAILED Script: -- : 'RUN: at line 1'; cat /Users/thakis/src/llvm-project/clang/test/Frontend/std

[PATCH] D67592: [Clang] Use -main-file-name for source filename if not set

thakis added a comment. Yup, test is happy now, thanks! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67592/new/ https://reviews.llvm.org/D67592 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.ll

[PATCH] D65543: [Windows] Autolink with basenames and add libdir to libpath

thakis added a comment. /Brepro seems orthogonal to me. If you only pass relative paths and -no-canonical-prefixes then the embedded path is relative and doesn't impeded determinism. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65543/new/ https:

[PATCH] D67541: [ClangFormat] Future-proof Standard option, allow floating or pinning to arbitrary lang version

thakis added a comment. This broke the sphinx bot: http://lab.llvm.org:8011/builders/clang-sphinx-docs/builds/48207 Warning, treated as error: /home/buildbot/llvm-build-dir/clang-sphinx-docs/llvm/src/tools/clang/docs/ClangFormatStyleOptions.rst:2295: WARNING: Explicit markup ends without a

[PATCH] D45601: Warn on bool* to bool conversion

thakis added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:7806 + "comparing %0 as a boolean">, + InGroup; def warn_format_argument_needs_cast : Warning< lebedev.ri wrote: > Also, this really really should be under it's own flag, whic

[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

thakis added a comment. +1 to just adding a dedicated Wno flag for the new warning instead of coming up with this new spelling. Repository: rC Clang https://reviews.llvm.org/D45685 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://li

[PATCH] D45777: [UnitTests] NFC/build-perf: Break up nontrivial compile jobs

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Makes sense to me. Repository: rC Clang https://reviews.llvm.org/D45777 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

[PATCH] D45877: clang-cl: Accept (and ignore) /Zc:__cplusplus.

thakis closed this revision. thakis added a comment. r330427, thanks! https://reviews.llvm.org/D45877 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45877: clang-cl: Accept (and ignore) /Zc:__cplusplus.

thakis created this revision. thakis added a reviewer: hans. thakis edited the summary of this revision. hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. lgtm, thanks! See https://blogs.msdn.microsoft.com/vcblog/2018/04/09/msvc-now-correctly-re

[PATCH] D45966: Remove LLVM_INSTALL_CCTOOLS_SYMLINKS

thakis created this revision. thakis added a reviewer: JDevlieghere. Herald added a subscriber: mgorny. It used to symlink `dsymutil` to `llvm-dsymutil`, but after r327790 llvm's dsymutil binary is now called dsymutil without prefix. r327792 then reversed the direction of the symlink if LLVM_IN

[PATCH] D43322: Diagnose cases of "return x" that should be "return std::move(x)" for efficiency

thakis added a comment. This is a cool warning, thanks for adding it. We ran into one thing while enabling this in Chromium that I'd like to mention here. We have code that basically does: struct Foo { using passwords_iterator = std::map, ReverseStr

[PATCH] D46050: [Frontend] Avoid running plugins during code completion parse

thakis added a comment. Seems reasonable; can you add a test for this (maybe somewhere in clang/test/Frontend/plugin*)? Repository: rC Clang https://reviews.llvm.org/D46050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

thakis added a comment. > because the headers that are part of Clang toolchain are incompatible with > the system library Do you have details on this? This isn't supposed to be the case as far as I know. We link chrome/mac against system libc++ with the bundled headers, and it at least seems t

[PATCH] D46056: Move _LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS macro to build system

thakis closed this revision. thakis added a comment. Landed in r331150: http://llvm.org/viewvc/llvm-project?view=revision&revision=331150 Repository: rCXXA libc++abi https://reviews.llvm.org/D46056 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D46056: Move _LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS macro to build system

thakis added a comment. Apologies for misspelling your last name :-( Repository: rCXXA libc++abi https://reviews.llvm.org/D46056 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46385: Fix test failure for missing _LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS

thakis closed this revision. thakis added a comment. r331450, thanks! Repository: rCXXA libc++abi https://reviews.llvm.org/D46385 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46050: [Frontend] Avoid running plugins during code completion parse

thakis added inline comments. Comment at: test/Frontend/plugins.c:7 + +// RUN: c-index-test -code-completion-at=%s:6:1 -load %llvmshlibdir/PrintFunctionNames%pluginext -add-plugin print-fns %s | FileCheck -check-prefix=CHECK-COMPLETION-WITHOUT-PLUGINS %s +// REQUIRES: plugins,

[PATCH] D48426: [clang-cl] Don't emit dllexport inline functions etc. from pch files (PR37801)

thakis added a comment. Still looks good, ship it! One more suggestion about additional test coverage (but maybe it's already there and I'm just missing it). Comment at: test/CodeGen/pch-dllexport.cpp:55 +template void __declspec(dllexport) explicitInstantiationDefAfterDecl(

[PATCH] D48426: [clang-cl] Don't emit dllexport inline functions etc. from pch files (PR37801)

thakis added inline comments. Comment at: test/CodeGen/pch-dllexport.cpp:55 +template void __declspec(dllexport) explicitInstantiationDefAfterDecl(T) {} +extern template void explicitInstantiationDefAfterDecl(int); + hans wrote: > thakis wrote: > > This has two

[PATCH] D48626: New option -fwindows-filesystem, affecting #include paths.

thakis added a comment. There are 2 other patches out there for the case sensitivity. Neither landed, because the performance hit form this approach is pretty big, and it's not necessary: You can either put the Win SDK into a ciopfs mount (example: https://cs.chromium.org/chromium/src/build/vs_

[PATCH] D48626: New option -fwindows-filesystem, affecting #include paths.

thakis added a comment. The things I pointed at only do case-insensitivity. I haven't seen anything using backslashes in the builds I worked on. (Is that for -I flags?) I'd think that the backslash bits can probably be implemented with less overhead. Repository: rC Clang https://reviews.llv

[PATCH] D48781: [ms] Fix mangling of char16_t and char32_t to be compatible with MSVC.

thakis created this revision. thakis added a reviewer: majnemer. MSVC limits char16_t and char32_t string literal names to 32 bytes of character data, not to 32 characters. wchar_t string literal names on the other hand can get up to 64 bytes of character data. https://reviews.llvm.org/D48781

[PATCH] D48781: [ms] Fix mangling of char16_t and char32_t to be compatible with MSVC.

thakis added a comment. For your convenience: https://godbolt.org/g/KXxbKb https://reviews.llvm.org/D48781 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48781: [ms] Fix mangling of char16_t and char32_t to be compatible with MSVC.

thakis added a comment. Thanks, landed with nit addressed in r336097. https://reviews.llvm.org/D48781 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48928: [ms] Fix mangling of string literals used to initialize arrays larger or smaller than the literal

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Nice! Comment at: lib/AST/MicrosoftMangle.cpp:3198 + ->getSize() + .getZExtValue(); + nit: Also do

[PATCH] D49112: [Sema] Implement -Wmemset-transposed-args

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. lgtm assuming you ran this on some large code base to make sure it doesn't have false positives. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:659 +def warn_s

[PATCH] D46652: [clang-cl, PCH] Implement support for MS-style PCH through headers

thakis added a comment. Sorry about missing this. Looks great, thanks for doing this. This was a pretty big omission, glad to see it's done now :-) I always imagined we might reuse the precompiled preamble stuff (Lexer::ComputePreamble() etc), but it's pretty clear as-is. Repository: rC Cla

[PATCH] D46652: [clang-cl, PCH] Implement support for MS-style PCH through headers

thakis added a comment. Also, were you planning on also adding support for the (filename-less version of) hdrstop pragma? After this change, that should probably be fairly straightforward. Repository: rC Clang https://reviews.llvm.org/D46652 __

[PATCH] D46652: [clang-cl, PCH] Implement support for MS-style PCH through headers

thakis added a comment. And finally (sorry about all the mails), this should probably be mentioned in the release notes (docs/ReleaseNotes.rst in the clang repo) since it's a notable new feature :-) Repository: rC Clang https://reviews.llvm.org/D46652

[PATCH] D49398: clang-cl: Postpone Wmsvc-not-found emission until link.exe gets used.

thakis created this revision. thakis added a reviewer: zturner. Wmsvc-not-found was added in r297851 to help diagnose why link.exe can't be executed. However, it's emitted even when using -fuse-ld=lld, and in cross builds there's no way to get rid of the warning other than disabling it. Instead

[PATCH] D49398: clang-cl: Postpone Wmsvc-not-found emission until link.exe gets used.

thakis added inline comments. Comment at: lib/Driver/ToolChains/MSVC.cpp:467-468 if (Linker.equals_lower("link")) { +if (!TC.FoundMSVCInstall()) + C.getDriver().Diag(clang::diag::warn_drv_msvc_not_found); + zturner wrote: > It looks like it's possible

[PATCH] D49398: clang-cl: Postpone Wmsvc-not-found emission until link.exe gets used.

thakis updated this revision to Diff 155885. thakis added a comment. address comment https://reviews.llvm.org/D49398 Files: lib/Driver/ToolChains/MSVC.cpp lib/Driver/ToolChains/MSVC.h Index: lib/Driver/ToolChains/MSVC.h === -

[PATCH] D49398: clang-cl: Postpone Wmsvc-not-found emission until link.exe gets used.

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. r337290, thanks! https://reviews.llvm.org/D49398 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/li

[PATCH] D46652: [clang-cl, PCH] Implement support for MS-style PCH through headers

thakis added a comment. In https://reviews.llvm.org/D46652#1164016, @thakis wrote: > And finally (sorry about all the mails), this should probably be mentioned in > the release notes (docs/ReleaseNotes.rst in the clang repo) since it's a > notable new feature :-) I added this to releasenotes

[PATCH] D68526: [Diagnostics] Silence -Wsizeof-array-div for character buffers

thakis accepted this revision. thakis added a comment. Thanks, I think this is a good change. It fixes a false positive in wayland and one somewhere in Chromium's windows sandbox. See the commit thread of r371605 for examples. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D68640: Try to get readability-deleted-default.cpp to pass on Windows.

thakis added a comment. There's just one other use of `-fno-ms-compatibility` in clang-tidy's test suite, so I'm not 100% sure this is the way to go. It's better than disabling the test completely on Windows :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68640/new/ https://reviews.l

[PATCH] D68640: Try to get readability-deleted-default.cpp to pass on Windows.

thakis created this revision. thakis added a reviewer: rnk. thakis added a comment. There's just one other use of `-fno-ms-compatibility` in clang-tidy's test suite, so I'm not 100% sure this is the way to go. It's better than disabling the test completely on Windows :) In MS compatibility mod

[PATCH] D68640: Try to get readability-deleted-default.cpp to pass on Windows.

thakis added a comment. In D68640#1699563 , @gribozavr wrote: > It looks to me that a better fix is to fix the checker to not emit this > warning in MS compatibility mode. The warning is about emitting "here's a redundant declaration", and the test exp

[PATCH] D68640: Try to get readability-deleted-default.cpp to pass on Windows.

thakis added a comment. In D68640#1699628 , @aaron.ballman wrote: > In D68640#1699605 , @thakis wrote: > > > In D68640#1699563 , @gribozavr > > wrote: > > > > > It looks to

[PATCH] D68099: [MS ABI]: Fix mangling function arguments for template types to be compatible with MSVC

thakis added a comment. (It regressed in D62780 ; I had bisected that bit.) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68099/new/ https://reviews.llvm.org/D68099 ___ cfe-commits m

[PATCH] D68099: [MS ABI]: Fix mangling function arguments for template types to be compatible with MSVC

thakis accepted this revision. thakis added a comment. lgtm. I finally got around to understanding the patch. Sorry for the delay, thanks for the fix! Next time you upload a patch, please upload it with more context (see https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-int

[PATCH] D68640: Try to get readability-deleted-default.cpp to pass on Windows.

thakis added a comment. > Can you clarify what exactly the TODO is? As-is, the check suggests removing > the redeclaration where it's a no-op (non-ms) but not where it isn't (C, ms > compat). If I understand your reply correctly, this is desired behavior. Is > the TODO then to have test coverag

[PATCH] D68640: Try to get readability-deleted-default.cpp to pass on Windows.

thakis updated this revision to Diff 223917. thakis edited the summary of this revision. thakis added a comment. make test more explicit CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68640/new/ https://reviews.llvm.org/D68640 Files: clang-tools-extra/test/clang-tidy/readability-redun

[PATCH] D68640: Try to get readability-deleted-default.cpp to pass on Windows.

This revision was automatically updated to reflect the committed changes. Closed by commit rG8cb804a3c9ce: Try to get readability-deleted-default.cpp to pass on Windows. (authored by thakis). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[PATCH] D68702: [clangd] Make sure ReplyCallbacks are destroyed before RequestCancelersMutex

thakis added a comment. Maybe inline the error from the bot. Right now stuff on 45.33.8.238 is very in flux. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68702/new/ https://reviews.llvm.org/D68702 __

[PATCH] D68166: [Clang][OpenMP Offload] Add new tool for wrapping offload device binaries

thakis added a comment. Out of interest (or ignorance :) ), why is this a separate binary instead of just part of the normal `clang` driver? C, C++, Objective-C, and assembly all can do with a single driver, yet the offload stuff now has both clang-offload-wrapper and clang-offload-bundler. Why

[PATCH] D68637: [libTooling] Move Transformer files to their own directory/library.

thakis added inline comments. Comment at: clang/lib/Tooling/Transformer/CMakeLists.txt:3 + +add_clang_library(clangTransformer + RangeSelector.cpp All the other libs in lib/Tooling/Foo are called clangToolingFoo, not clangFoo. Can you please rename this library

[PATCH] D68824: Fix __builtin_assume_aligned with too large values.

thakis added a comment. The new test fails on Windows: -- Testing: 16032 tests, 32 workers -- Testing: 0.. 10.. 20.. 30.. 40.. 50 FAIL: Clang :: Sema/builtin-assume-aligned.c (8862 of 16032) TEST 'Clang :: Sema/builtin-assume-aligned.c' FAILED

[PATCH] D68824: Fix __builtin_assume_aligned with too large values.

thakis added a comment. Since you just left IRC, I reverted this in 374456 for now. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68824/new/ https://reviews.llvm.org/D68824 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lis

[PATCH] D68660: [tooling] Teach Tooling to understand compilation with offloading.

thakis added a comment. This makes tests assert on Mac: http://45.33.8.238/mac/1415/step_6.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68660/new/ https://reviews.llvm.org/D68660 ___ cfe-commits m

[PATCH] D68832: [tsan,msan] Insert module constructors in a module pass

thakis added a comment. This fails on Mac and windows http://45.33.8.238/win/247/step_6.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68832/new/ https://reviews.llvm.org/D68832 ___ cfe-commits mail

[PATCH] D68832: [tsan,msan] Insert module constructors in a module pass

thakis added a comment. Reverted in r374503. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68832/new/ https://reviews.llvm.org/D68832 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lis

[PATCH] D68099: [MS ABI]: Fix mangling function arguments for template types to be compatible with MSVC

This revision was automatically updated to reflect the committed changes. Closed by commit rGb95713784a3c: [MS ABI]: Fix mangling function arguments for template types to be compatible… (authored by thakis). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

thakis added a comment. This fails on macOS: : 'RUN: at line 2'; grep -E "*code should be clang-formatted*" /Users/thakis/src/llvm-project/out/gn/obj/clang/test/Format/Output/dry-run.cpp.tmp.stderr -- Exit Code: 2 Command Output (stderr): -- grep: repetition-operator operand inv

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

thakis added a comment. This fails on Windows: http://45.33.8.238/win/386/step_7.txt Please take a look, and if it's not immediately clear how to fix, b please revert while you investigate. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D45050/new/

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

thakis added a comment. > Thanks for doing this, I am struggling to find the MacOS bot log that failed, > are any available via Buildbot? (I notice the log looks like your own machine) The mac bots are on a different system for some reason: http://green.lab.llvm.org/green/ (see also http://lis

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

thakis added a comment. > Could you or anyone else point me to a good example, I'd definitely like to > take a look. http://llvm-cs.pcc.me.uk/include/llvm/Support/SourceMgr.h/rSMDiagnostic -- > `via clang-format?` did you mean via clang.exe or clang-check.exe? I did > think of this when

[PATCH] D65387: [clangd] Add a callback mechanism for handling responses from client.

thakis added inline comments. Herald added a subscriber: usaxena95. Comment at: clang-tools-extra/trunk/clangd/test/request-reply.test:6 +--- +{"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"file:///clangd-

[PATCH] D62855: [clangd] Implementation of auto type expansion.

thakis added inline comments. Herald added a subscriber: usaxena95. Comment at: clang-tools-extra/trunk/clangd/test/code-action-request.test:54 +--- +{"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"file:///

[PATCH] D68915: [clang][IFS] Escape mangled name in-order to not break llvm-ifs with names mangled using MS ABI

thakis added a comment. On macOS symbols are prefixed with an underscore: Command Output (stderr): -- /Users/thakis/src/llvm-project/clang/test/InterfaceStubs/object.c:5:16: error: CHECK-TAPI: expected string not found in input // CHECK-TAPI: "data" : { Type: Object, Size: 4 }

[PATCH] D41569: [Concepts] Constraint enforcement and diagnostics

thakis added a comment. The mangling test fails on Windows: http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/15944 It also fails on ppc64le: http://lab.llvm.org:8011/builders/clang-ppc64le-linux-lnt/builds/21092 Please watch http://lab.llvm.org:8011/console for a bit after landing c

[PATCH] D41569: [Concepts] Constraint enforcement and diagnostics

thakis added a comment. reverted in r374985, it wasn't obvious to me how to unbreak the test. Sorry! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D41569/new/ https://reviews.llvm.org/D41569 ___ cfe-commits mailing li

[PATCH] D68528: [Implicit Modules] Add -cc1 option -fmodules-strict-hash which includes search paths and diagnostics.

thakis added a comment. Looks like this fails on win: http://45.33.8.238/win/841/step_6.txt Ptal! Maybe just cat'ing all files instead of echoing the first and piping into cat works? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68528/new/ https

[PATCH] D66046: Add new tautological compare warning for bitwise-or with a non-zero constant

thakis added a comment. Any reason why this is off by default? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66046/new/ https://reviews.llvm.org/D66046 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D66046: Add new tautological compare warning for bitwise-or with a non-zero constant

thakis added a comment. Ah, that's a good reason. Are they in -Wall? We've sometimes reconned "-Wall == needs CFG" :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66046/new/ https://reviews.llvm.org/D66046 _

[PATCH] D66046: Add new tautological compare warning for bitwise-or with a non-zero constant

thakis added a comment. Mr Trieu, what do you think about adding some or all of the Wtautological-compare warnings to Wall Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66046/new/ https://reviews.llvm.org/D66046

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

thakis added a comment. klimek: ping :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68969/new/ https://reviews.llvm.org/D68969 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[PATCH] D66046: Add new tautological compare warning for bitwise-or with a non-zero constant

thakis added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8161 + "bitwise or with non-zero value always evaluates to true">, + InGroup, DefaultIgnore; def warn_tautological_overlap_comparison : Warning< rtrieu wrote: > jfb wrote: > >

[PATCH] D69292: Proposal to add -Wtautological-compare to -Wall

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Abstractly this lgtm. Do you have any data on true / false positive rates? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69292/new/ https://reviews.llvm.org/D69292

[PATCH] D67161: [clang,ARM] Initial ACLE intrinsics for MVE.

thakis added inline comments. Comment at: clang/utils/TableGen/CMakeLists.txt:17 NeonEmitter.cpp + MveEmitter.cpp TableGen.cpp nit: These files are listed alphabetically. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D65433: [clangd] DefineInline action availability checks

thakis added a comment. The test fails in Windows: http://45.33.8.238/win/1112/step_7.txt Ptal, and if it takes a while to investigate please revert while you look. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65433/new/ https://reviews.llvm.org/

[PATCH] D69292: Proposal to add -Wtautological-compare to -Wall

thakis added a comment. This is imho basic enough that it doesn't need a test. A test for this doesn't add any value that I can see. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69292/new/ https://reviews.llvm.org/D69292 ___ cfe-commits m

[PATCH] D67983: [ObjC] Diagnose implicit type coercion from ObjC 'Class' to object pointer types.

thakis added a comment. After this, Class can no longer be used as a key type in an Obj-C dictionary literal. Is that intentional? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67983/new/ https://reviews.llvm.org/D67983

[PATCH] D67983: [ObjC] Diagnose implicit type coercion from ObjC 'Class' to object pointer types.

thakis added a comment. > While the default metaclass does in fact implement the one method NSCopying > declares, it's not possible within the language to declare that the Class -- > itself, as an instance -- implements the instance methods from the NSCopying > protocol. Should the compiler ju

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

thakis added a comment. All new files added in this patch had dos line endings. I fixed that in e59f7488 but going forward please configure your environment to create new files for LLVM with unix line endings. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D67983: [ObjC] Diagnose implicit type coercion from ObjC 'Class' to object pointer types.

thakis added a comment. Just doing nothing is fine with me. I'll note that this change required several changes in our Obj-C++ codebase, and we don't have all that much Obj-C++ code in Chromium. Most of the changes made the code better, but I'd imagine that folks with more Obj-C++ code might n

[PATCH] D75323: Support relative dest paths in headermap files

thakis added a comment. I don't know why we (chromium) want to use header maps, but assuming we do want to use them: The problem with absolute paths is that they're machine-dependent and can't be run on other machines and then cached globally. See also http://blog.llvm.org/2019/11/deterministic

<    1   2   3   4   5   6   7   8   9   10   >