[PATCH] D70342: [Diagnostics] Put "deprecated copy" warnings into -Wdeprecated-copy

thakis added a comment. Why do we emit this before C++20? It's only deprecated in C++20 onwards, no? Shouldn't this be default-enabled based off language version instead of being in -Wextra? No other language deprecation warning so far (register etc) has worked like this warning does. (Also, t

[PATCH] D70342: [Diagnostics] Put "deprecated copy" warnings into -Wdeprecated-copy

thakis added a comment. Ah, Hans points out that the paper says that it's been deprecated in C++11, so this should either be on by default (and mention "C++11" somewhere), or it should be in -pedantic given that no other compiler diagnoses this and given that it fires heavily on existing code (

[PATCH] D70615: Add an -fno-temp-file flag for compilation

thakis added a comment. In D70615#1769751 , @rnk wrote: > +more reviewers > > This doesn't add any code complexity, we already have the boolean UseTempFile > flag, so I think we should do this. I would also point out that right now, in > my LLVM build di

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

thakis added a comment. (sorry for the slow response here) >> If you need to run an external program, look for it next to the compiler, >> like we do for gas with -fno-integrated-as, linker, etc. People can use the >> existing -B flag to make clang look elsewhere if they need that. > > Unfortu

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

thakis added a comment. (ps: Thanks for the two changes landed so far!) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70500/new/ https://reviews.llvm.org/D70500 ___ cfe-commits mailing list cfe-commits

[PATCH] D70342: [Diagnostics] Put "deprecated copy" warnings into -Wdeprecated-copy

thakis added a comment. Re-re-reading http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0619r4.html#3.2, the recommendation is "We will consider again for C++23". So I think this should be enabled with -std=c++20 at the earliest, since that's where the idea is from. But even there it's

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

thakis added a comment. Hey, cool! I gave this a shot myself many years ago at http://llvm.org/bugs/show_bug.cgi?id=18938 That bug has a list of interesting test cases I had collected back then. Maybe you want to check and see how this does on those cases? Repository: rG LLVM Github Monorep

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

thakis added a comment. Also, did you check if this has a measurable impact on compilation speed? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70638/new/ https://reviews.llvm.org/D70638 ___ cfe-commit

[PATCH] D71029: [clangd] (take 2) Try harder to find a plausible `clang` as argv0, particularly on Mac.

thakis added a comment. Breaks Win: http://45.33.8.238/win/3549/step_7.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71029/new/ https://reviews.llvm.org/D71029 ___ cfe-commits mailing list cfe-comm

[PATCH] D48921: NFC - Typo fix in test/CodeGenCXX/runtime-dllstorage.cpp

thakis added a comment. This breaks the test everywhere, e.g. http://45.33.8.238/linux/5543/step_7.txt Can you revert this while you investigate if the test was broken since it landed or if something broke it later, while it wasn't really testing what it was supposed to test? Repository: rG

[PATCH] D71320: [IR] Split out target specific intrinsic enums into separate headers

thakis added a comment. Any reason these are called .h? All our other tablegen outputs are called .inc. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71320/new/ https://reviews.llvm.org/D71320 ___ cfe-

[PATCH] D71240: [clangd] Heuristically resolve dependent method calls

thakis added a comment. Tests fail on Windows: http://45.33.8.238/win/3976/step_9.txt Probably needs the usual -fno-delayed-template-parsing workaround, see other clangd tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71240/new/ https://revi

[PATCH] D71466: [ARM][MVE][Intrinsics] remove extraneous intrinsics.

thakis added a comment. This breaks tests: http://45.33.8.238/mac/4132/step_7.txt Please take a look and revert if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71466/new/ https://reviews.llvm.org/D71466 ___

[PATCH] D71345: [clangd] Fall back to selecting token-before-cursor if token-after-cursor fails.

thakis added a comment. Reverted in f0604e73a4daa35a10eb17a998657d6c4bd0e971 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71345/new/ https://reviews.llvm.org/D71345

[PATCH] D71345: [clangd] Fall back to selecting token-before-cursor if token-after-cursor fails.

thakis added a comment. This breaks compile on most bots in the "clang" section on http://lab.llvm.org:8011/console , probably the ones that use gcc as host compiler. Here's an example: http://lab.llvm.org:8011/builders/clang-cmake-armv7-selfhost-neon/builds/2714/steps/build%20stage%201/logs/st

[PATCH] D71320: [IR] Split out target specific intrinsic enums into separate headers

thakis added a comment. I think this broke the reverse-iteration bot: http://lab.llvm.org:8011/builders/reverse-iteration/builds/15619 In D71320#1782204 , @rnk wrote: > In D71320#1780805 , @thakis wrote: > > > Any

[PATCH] D71414: [clangd] Introduce codeblocks

thakis added inline comments. Comment at: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp:126 +test +``` +bar)md"); Older (but still supported) gccs can't handle multiline raw strings in macro arguments, see e.g. http://lab.llvm.org:8011/builders/cl

[PATCH] D68913: Adds fixit hints to the Wrange-loop-analysis

thakis added a comment. Mordante, do you need someone to land this for you? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68913/new/ https://reviews.llvm.org/D68913 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.

[PATCH] D71020: [ASTImporter] Friend class decl should not be visible in its context

thakis added a comment. Looks like this breaks building on Windows in general: http://45.33.8.238/win/4247/step_4.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71020/new/ https://reviews.llvm.org/D71020 _

[PATCH] D71020: [ASTImporter] Friend class decl should not be visible in its context

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

[PATCH] D41910: [Concepts] Constrained partial specializations and function overloads.

thakis added a comment. This makes clang crash when running tests: http://45.33.8.238/linux/5976/step_7.txt Please use "Differential Revision: https://reviews.llvm.org/D41910"; instead of "Phabricator: D41910 " so that phab auto-closes the review and other auto

[PATCH] D71680: Customize simplified dumping and matching of LambdaExpr

thakis added a comment. Looks like this might break tests: http://45.33.8.238/linux/6150/step_7.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71680/new/ https://reviews.llvm.org/D71680 ___ cfe-comm

[PATCH] D71080: [NFC] Separate getLastArgIntValue to Basic

thakis added a comment. Why move this to Basic instead of to Driver if you want to use it in Driver? (Frontend depends on Driver so it could then still use it.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71080/new/ https://reviews.llvm.org/D710

[PATCH] D71802: [NFC] Move OptionUtils from Basic to Driver

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Thank you! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71802/new/ https://reviews.llvm.org/D71802 ___ cfe-commits mailing list cfe-com

[PATCH] D71080: [NFC] Separate getLastArgIntValue to Basic

thakis added a comment. (The follow-up is at D71802 . Thanks!) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71080/new/ https://reviews.llvm.org/D71080 ___ cfe-commits

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

thakis added a comment. Test falls in Windows: http://45.33.8.238/win/10088/step_8.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74669/new/ https://reviews.llvm.org/D74669 ___ cfe-commits mailing l

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

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

[PATCH] D75890: [libunwind] Remove __FILE__ and __LINE__ from error reporting

thakis added a comment. `__FILE__` only expands to an absolute path if you pass an absolute path to clang (ctrl-f "__FILE__" on http://blog.llvm.org/2019/11/deterministic-builds-with-clang-and-lld.html). It feels like this is working around a deficiency in the compiler. Maybe fixing whatever's

[PATCH] D76099: [Clang][Driver] In -fintegrated-cc1 mode, avoid crashing on exit after a compiler crash

thakis added a comment. Do you think this could cause errors like in https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8885934379148608192/+/steps/compile/0/stdout?format=raw ? (see also https://bugs.chromium.org/p/chromium/issues/detail?id=1061535) Repository: r

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

thakis added a comment. Reduced repro at https://bugs.chromium.org/p/chromium/issues/detail?id=1061533#c7 Scrolling up, it looks like this was reverted yesterday already. I'm confused why this wasn't reverted yesterday; all work I did in bisecting and creducing was a duplicate of what happened

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

thakis added a comment. This makes clang crash, repro at https://bugs.chromium.org/p/chromium/issues/detail?id=1061533#c4 I'll revert for now. I'm also seeing timeouts and use-after-frees on other bots; maybe they're related (or maybe not). CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D75470: [SVE] Auto-generate builtins and header for svld1.

thakis added inline comments. Comment at: clang/include/clang/Basic/CMakeLists.txt:44 # ARM NEON and MVE clang_tablegen(arm_neon.inc -gen-arm-neon-sema Update comment to also say "SVE" and "CDE" (or just say "# ARM builtin headers") Comment

[PATCH] D75470: [SVE] Auto-generate builtins and header for svld1.

thakis added inline comments. Comment at: clang/utils/TableGen/SveEmitter.cpp:32 #include "llvm/TableGen/Error.h" +#include "clang/Basic/AArch64SVETypeFlags.h" #include Including stuff from `clang/Basic` in clang/utils/TableGen is conceptually a layering viol

[PATCH] D75470: [SVE] Auto-generate builtins and header for svld1.

thakis added inline comments. Comment at: clang/utils/TableGen/SveEmitter.cpp:32 #include "llvm/TableGen/Error.h" +#include "clang/Basic/AArch64SVETypeFlags.h" #include sdesmalen wrote: > thakis wrote: > > Including stuff from `clang/Basic` in clang/utils/Tabl

[PATCH] D74361: [Clang] Undef attribute for global variables

thakis added a comment. This breaks tests on Windows: http://45.33.8.238/win/10664/step_7.txt Please take a look, and if it takes some time please revert while you investigate. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74361/new/ https://revi

[PATCH] D75470: [SVE] Auto-generate builtins and header for svld1.

thakis added inline comments. Comment at: clang/utils/TableGen/TableGen.cpp:196 +clEnumValN(GenArmSveCodeGenMap, "gen-arm-sve-codegenmap", + "Generate arm_sve_codegenmap.inc for clang"), clEnumValN(GenArmMveHeader, "gen-arm-mve-header",

[PATCH] D76346: [Syntax] Build template declaration nodes

thakis added a comment. Breaks tests on Windows: http://45.33.8.238/win/10716/step_7.txt Please take a look and revert if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76346/new/ https://reviews.llvm.org/D76346

[PATCH] D76346: [Syntax] Build template declaration nodes

thakis added a comment. Reverted in 881f5b5a7b2 for now. Usually the fix is to add -fno-delayed-template-parsing somewhere, but in this case it seems like the whole feature might not expect delayed template parsing, so maybe

[PATCH] D76262: [NFC] Add UsedDeclVisitor

thakis added a comment. Could you add a clang-side test for the check-mlir fix? Having to run mlir tests to verify clang changes seems suboptimal, so we should try to improve coverage of clang's tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D76433: [Syntax] Test both the default and windows target platforms in unittests

thakis added a comment. Breaks tests on Windows: http://45.33.8.238/win/10881/step_7.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76433/new/ https://reviews.llvm.org/D76433 ___ cfe-commits mailing

[PATCH] D73873: [clangd] Mechanism to make update debounce responsive to rebuild speed.

thakis added a comment. This seems to break tests: http://45.33.8.238/linux/9296/step_9.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73873/new/ https://reviews.llvm.org/D73873 ___ cfe-commits mail

[PATCH] D73873: [clangd] Mechanism to make update debounce responsive to rebuild speed.

thakis added a comment. I agree it's a bit strange. The linux bot didn't recover after the revert either, so something's amiss. My Win bot has been hanging since this landed and I just CRD'd in and it's hanging in check-clangd and there are tens of ClangdTests processes hanging around and doin

[PATCH] D72189: [SystemZ] Support -msoft-float

thakis added a comment. Hello, this breaks tests on all platforms (e.g.: http://45.33.8.238/mac/7170/step_11.txt) Please revert immediately and please run tests locally before committing next time. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D721

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

thakis added a comment. This caused a regression when using `-fsanitize=memory -D_FORTIFY_SOURCE=2`. MemorySanitizer requires all code, including system libraries, to be instrumented. Else it reports false positives. For glibc, it has lots of interceptors to make sure glibc doesn't have to be i

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

thakis added a comment. In D69825#1837243 , @hctim wrote: > For tracking purposes. > > The ASan leaks have been worked around in two separate patches in order to > make the bots green again: > > - c38e42527b21acee8d01a016d5bfa2fb83202e29 >

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

thakis added a comment. This makes lld crash when linking chromium's base_unittests and probably does the same for most other binaries that use both thinlto and cfi: https://bugs.chromium.org/p/chromium/issues/detail?id=1049434 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D68720: Support -fstack-clash-protection for x86

thakis added a comment. This breaks check-clang on mac and win: http://45.33.8.238/mac/7485/step_7.txt http://45.33.8.238/win/7753/step_7.txt Please revert and then investigate asynchronously, unless the fix is obvious. Comment at: clang/test/CodeGen/stack-clash-protection.c:

[PATCH] D68720: Support -fstack-clash-protection for x86

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

[PATCH] D73852: [clang] detect switch fallthrough marked by a comment (PR43465)

thakis added a comment. I think giving comments a semantic meaning is a bad idea. Do we really have to do this? In addition to this making comments have semantic meaning, the meaning is different from gcc. I don't think we should support this. Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D73852: [clang] detect switch fallthrough marked by a comment (PR43465)

thakis added inline comments. Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:1239 + llvm::Regex("(/\\*[ \\t]*fall(s | |-)?thr(ough|u)\\.?[ \\t]*\\*/)" + "|(//[ \\t]*fall(s | |-)?thr(ough|u)\\.?[ \\t]*)", + llvm

[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

thakis added a comment. Isn't a better approach to only do in-process cc1 if there's just one TU? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74447/new/ https://reviews.llvm.org/D74447 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D73852: [clang] detect switch fallthrough marked by a comment (PR43465)

thakis added a comment. In D73852#1872044 , @aaron.ballman wrote: > In D73852#1872000 , @thakis wrote: > > > I think giving comments a semantic meaning is a bad idea. Do we really have > > to do this? > > > I thin

[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

thakis added a comment. I'd think that everyone debugging clang always passes a single TU to it, so I'm not sure debuggability does much here :) The `-disable-free` code has never been used in normal compilations, so we didn't have to worry about this path. This patch here brings us to 3 modes

[PATCH] D74384: Use std::foo_t rather than std::foo in LLVM.

thakis added a comment. Looks like this broke building on windows with clang-cl as host compiler in some situations: http://45.33.8.238/win/8160/step_4.txt I'm not sure if that's a bug in the version of the host clang I'm using yet. I'll poke around a bit and let you know. Repository: rG LL

[PATCH] D74384: Use std::foo_t rather than std::foo in LLVM.

thakis added a comment. That landed in 17b77418121139c4e8cfb050d82ead3f29db7132 . Sounds like there's more SFINAE to be fixed here, but my build is happy again. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D74411: [clangd] Query constructors in the index during rename.

thakis added a comment. Doesn't build on Mac: http://45.33.8.238/mac/7949/step_4.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74411/new/ https://reviews.llvm.org/D74411 ___ cfe-commits mailing lis

[PATCH] D73904: [clang] stop baremetal driver to append .a to lib

thakis added a comment. This breaks tests on Windows: http://45.33.8.238/win/8249/step_7.txt Please take a look and if it takes a while to fix, please revert while you investigate. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73904/new/ https://r

[PATCH] D73904: [clang] stop baremetal driver to append .a to lib

thakis added a comment. I fixed windows tests in a41550cff91b7fb2b56bf0e19ccb341bfd3e37b4 . Please watch bots after landing patches next time. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

thakis added a comment. I forgot another reason why "only do cc1 with a single TU" might be a good idea: In case crash recovery turns out to not work in all cases with in-process cc1, we could add a signal handler that on crash make the driver exec() itself with -fno-integrated-cc1 added, so th

[PATCH] D74436: Change clang option -ffp-model=precise to select ffp-contract=on

thakis added a comment. The revert of this breaks tests everywhere, as far as I can tell. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74436/new/ https://reviews.llvm.org/D74436 ___ cfe-commits mailin

[PATCH] D74051: Move update_cc_test_checks.py tests to clang

thakis added a comment. Fails on windows, looks like line endings: http://45.33.8.238/win/8350/step_7.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74051/new/ https://reviews.llvm.org/D74051 ___ c

[PATCH] D74051: Move update_cc_test_checks.py tests to clang

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

[PATCH] D73835: [IRBuilder] Virtualize IRBuilder

thakis added a comment. This causes this build warning: ../../llvm/include/llvm/IR/IRBuilder.h:77:16: warning: 'anchor' overrides a member function but is not marked 'override' [-Winconsistent-missing-override] virtual void anchor(); ^ ../../llvm/include/llvm/IR/IRBuilde

[PATCH] D72848: Remove some SVN-specific code.

thakis created this revision. thakis added a reviewer: hans. $URL$ is an SVN keyword substitution enabled via `svn propset svn:keywords "URL" tools/clang/lib/Basic/Version.cpp`. Now that we no longer use SVN, it's no longer being replaced by anything, and we no longer offer svn exports. So remove

[PATCH] D72848: Remove some SVN-specific code.

This revision was automatically updated to reflect the committed changes. Closed by commit rGfb9413cb84cc: Remove some SVN-specific code. (authored by thakis). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72848/new/ h

[PATCH] D72855: Make LLVM_APPEND_VC_REV=OFF affect clang, lld, and lldb as well.

thakis added a comment. A very late follow-up to my question in D37272 :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72855/new/ https://reviews.llvm.org/D72855 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D72855: Make LLVM_APPEND_VC_REV=OFF affect clang, lld, and lldb as well.

thakis created this revision. thakis added a reviewer: rnk. Herald added a subscriber: mgorny. Herald added a project: LLVM. thakis added a comment. A very late follow-up to my question in D37272 :) When LLVM_APPEND_VC_REV=OFF is set, the current git hash is no

[PATCH] D72855: Make LLVM_APPEND_VC_REV=OFF affect clang, lld, and lldb as well.

This revision was automatically updated to reflect the committed changes. Closed by commit rGfb5fafb23cc2: Make LLVM_APPEND_VC_REV=OFF affect clang, lld, and lldb as well. (authored by thakis). Herald added projects: clang, LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D72378: [clang-tidy] Add `bugprone-reserved-identifier`

thakis added a comment. Looks like this isn't building on Windows: http://45.33.8.238/win/6039/step_4.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72378/new/ https://reviews.llvm.org/D72378 ___ c

[PATCH] D72378: [clang-tidy] Add `bugprone-reserved-identifier`

thakis added a comment. cl.exe prints a different error: http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/13716/steps/stage%201%20build/logs/stdio Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72378/new/ https://reviews.llvm.org/D72

[PATCH] D50360: [Concepts] Requires Expressions

thakis added a comment. This breaks tests on non-Win, see e.g. http://45.33.8.238/linux/7846/step_7.txt http://45.33.8.238/mac/6120/step_7.txt Please watch http://lab.llvm.org:8011/console for a bit after landing changes. Please take a look, and if the fix isn't obvious please revert while you

[PATCH] D50360: [Concepts] Requires Expressions

thakis added a comment. Doesn't this still break check-clang everywhere? See e.g. http://45.33.8.238/linux/7860/step_7.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D50360/new/ https://reviews.llvm.org/D50360

[PATCH] D50360: [Concepts] Requires Expressions

thakis added a comment. In D50360#1828329 , @thakis wrote: > Doesn't this still break check-clang everywhere? See e.g. > http://45.33.8.238/linux/7860/step_7.txt It looks like this is some incremental build issue; after a clean build everything's happy

[PATCH] D72373: [clang-tidy] extend misc-misplaced-const to detect using besides typedef

thakis added a comment. Looks like this breaks tests: http://45.33.8.238/linux/8102/step_8.txt CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72373/new/ https://reviews.llvm.org/D72373 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D50360: [Concepts] Requires Expressions

thakis added a comment. I figured out the incremental build test problem, see D73202 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D50360/new/ https://reviews.llvm.org/D50360 ___

[PATCH] D73202: Make AST reading work better with LLVM_APPEND_VC_REV=NO

thakis marked an inline comment as done. thakis added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3645 // CityHash, but this will do for now. hash_code code = hash_value(getClangFullRepositoryVersion()); Arguably, we should omit

[PATCH] D73202: Make AST reading work better with LLVM_APPEND_VC_REV=NO

thakis created this revision. thakis added reviewers: saar.raz, rsmith. thakis marked an inline comment as done. thakis added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3645 // CityHash, but this will do for now. hash_code code = hash_value(getCl

[PATCH] D73300: [clang-tidy] Add library for clang-tidy main function

thakis added inline comments. Comment at: clang-tools-extra/clang-tidy/tool/CMakeLists.txt:22 + clangToolingCore + ) + Normally we have a single cmake target per CMakeLists.txt. Maybe the library could be in clang-tidy/lib and clang-tidy/tool could use it? The

[PATCH] D73202: Make AST reading work better with LLVM_APPEND_VC_REV=NO

thakis added a comment. rnk, could you maybe take a look? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73202/new/ https://reviews.llvm.org/D73202 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailma

[PATCH] D73202: Make AST reading work better with LLVM_APPEND_VC_REV=NO

This revision was automatically updated to reflect the committed changes. Closed by commit rG49532137d087: Make AST reading work better with LLVM_APPEND_VC_REV=NO (authored by thakis). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[PATCH] D73457: [Clang] Warn about 'z' printf modifier in old MSVC.

thakis added a comment. This breaks tests on Windows: http://45.33.8.238/win/6812/step_7.txt Since we auto-detect -fmsc-version if it's not explicitly installed and since this warning is on by default, it makes the test suite depend on the environment a good bit. Given how old 2015 is by now, I

[PATCH] D72723: Built-in functions for AMDGPU MFMA instructions.

thakis added a comment. This seems to break check-clang everywhere. If the fix isn't obvious, please revert and analyze asynchronously. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72723/new/ https://reviews.llvm.org/D72723 ___

[PATCH] D72723: Built-in functions for AMDGPU MFMA instructions.

thakis added a comment. Things have been broken for 1.5 hours now. Time to revert? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72723/new/ https://reviews.llvm.org/D72723 ___ cfe-commits mailing list

[PATCH] D73299: [HIP] Fix environment variable HIP_DEVICE_LIB_PATH

thakis added a comment. This breaks check-clang on Windows: http://45.33.8.238/win/6843/step_7.txt You probably want "env FOO=bar" instead of "FOO=bar" in the test file. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73299/new/ https://reviews.llvm

[PATCH] D44906: [clang-tidy] Define __clang_analyzer__ macro for clang-tidy for compatibility with clang static analyzer

thakis added a comment. The test added here doesn't pass on Windows, and the change breaks another test on Windows: http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/9794 Repository: rL LLVM https://reviews.llvm.org/D44906 ___ cfe-co

[PATCH] D44906: [clang-tidy] Define __clang_analyzer__ macro for clang-tidy for compatibility with clang static analyzer

thakis added a comment. Actually, it doesn't pass on non-Windows either: http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/27665/steps/test/logs/stdio Repository: rL LLVM https://reviews.llvm.org/D44906 ___ cfe-

[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

thakis added a comment. In https://reviews.llvm.org/D44883#1048751, @dblaikie wrote: > Historically Clang's policy on warnings was, I think, much more > conservative than it seems to be today. There was a strong desire not to > implement off-by-default warnings, and to have warnings with an >

[PATCH] D45165: Use llvm::sys::fs::real_path() in clang.

thakis created this revision. thakis added a reviewer: bruno. No expected behavior change. https://reviews.llvm.org/D45165 Files: lib/Basic/FileManager.cpp lib/Frontend/ModuleDependencyCollector.cpp Index: lib/Frontend/ModuleDependencyCollector.cpp

[PATCH] D45163: [Sema] -Wunused-value: diagnose unused std::move() call results.

thakis added a comment. (See also https://bugs.llvm.org/show_bug.cgi?id=10011 for a somewhat related discussion.) Repository: rC Clang https://reviews.llvm.org/D45163 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/

[PATCH] D45165: Use llvm::sys::fs::real_path() in clang.

thakis added a comment. bruno: ping https://reviews.llvm.org/D45165 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45485: [CMake] Unbreak BUILD_SHARED_LIBS workflow

thakis added a comment. Can you link to the recent timer changes? Repository: rC Clang https://reviews.llvm.org/D45485 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45485: [CMake] Unbreak BUILD_SHARED_LIBS workflow

thakis added a comment. Thanks for the pointer! I commented on https://reviews.llvm.org/D43578; I'm hoping there's a fix that doesn't require making so many libs depend on clang's IR library. Repository: rC Clang https://reviews.llvm.org/D45485 ___

[PATCH] D45165: Use llvm::sys::fs::real_path() in clang.

thakis added a comment. r329698, thanks! https://reviews.llvm.org/D45165 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43578: -ftime-report switch support in Clang

thakis added a comment. Also, please add cfe-commits to clang changes. Since this wasn't here and the patch wasn't seen by clang folks, since ftime-report-template-decl.cppwarnings is still failing on the bots (e.g. http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-f

[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

thakis added a comment. This landing made our clang trunk bots do an evaluation of this warning :-P It fired 8 times, all false positives, and all from unit tests testing that operator= works for self-assignment. (https://chromium-review.googlesource.com/c/chromium/src/+/1000856 has the exact

[PATCH] D43578: -ftime-report switch support in Clang

thakis added a comment. Reverted: $ svn merge -c -329714 . - Reverse-merging r329714 into '.': Atest/Frontend/ftime-report-template-decl.cpp - Recording mergeinfo for reverse merge of r329714 into '.': U . $ svn merge -c -329693 . - Reverse-merging r329693 into '.': Utest/Frontend

[PATCH] D38954: [Sema] -Wzero-as-null-pointer-constant: don't warn for system macros other than NULL.

thakis added a comment. Can you build some large-ish codebase (say, LLVM) with and without this patch and make sure that this doesn't measurably add to build perf? (With the warning turned on, obviously.) Other than that, this looks good to me. Comment at: test/SemaCXX/warn-

[PATCH] D39301: Ignore implicity casts for zero-as-null-pointer-constant warning

thakis accepted this revision. thakis added a comment. Awesome, thanks much! Like lebedev.ri says, adding a test for the "Parens" part would be good. https://reviews.llvm.org/D39301 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lis

[PATCH] D38954: [Sema] -Wzero-as-null-pointer-constant: don't warn for system macros other than NULL.

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. In general, "I don't see any way to prevent $slow_thing" just means the patch can't move ahead. But in this case, this looks like fairly small overhead for a worst-case input, so this won't be

[PATCH] D39317: Use -fuse-init-array if no gcc installation is found.

thakis created this revision. Herald added a subscriber: srhines. clang currently uses .init_array instead of .ctors on Linux if it detects gcc 4.7+. Make it so that it also uses .init_array if no gcc installation is found at all – if there's no old gcc, there's nothing we need to be compatible

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