[PATCH] D125889: clang-cl: Move /Zc: tests from cl-options.c to cl-zc.cpp

thakis created this revision. thakis added a reviewer: hans. Herald added a project: All. thakis requested review of this revision. Since we already have a dedicated file for testing the /Zc: flags, let's be consistent about putting /Zc: tests there. No behavior change. https://reviews.llvm.org

[PATCH] D125889: clang-cl: Move /Zc: tests from cl-options.c to cl-zc.cpp

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd39928e2cc39: clang-cl: Move /Zc: tests from cl-options.c to cl-zc.cpp (authored by thakis). Herald added a project: clang. Repository: rG LLVM Gi

[PATCH] D115232: [clangd] Indexing of standard library

thakis added a comment. In D115232#3520461 , @sammccall wrote: > Hmm, the test keeps crashing on the GN bot: > http://45.33.8.238/win/58316/step_9.txt > Unfortunately the stacktrace is not symbolized, and I'm not seeing this > elsewhere (e.g. premerge b

[PATCH] D115232: [clangd] Indexing of standard library

thakis added a comment. In D115232#3522571 , @sammccall wrote: > In D115232#3522514 , @thakis wrote: > >> In D115232#3520461 , @sammccall >> wrote: >> >>> Hmm, the test k

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

thakis added a comment. In D66397#1635792 , @aaron.ballman wrote: > In D66397#1635745 , @xbolva00 wrote: > > > I think it is too soon to jump and disable all macros. We still catch all > > motivating cases, it fou

[PATCH] D63648: [Preprocessor] Honor absolute paths in diagnostics

thakis added a comment. Maybe we should just implement full /FC mode that affects paths everywhere (diagnostics, `__FILE__`, etc). Then people who need this more than deterministic, build-directory-independent builds can use that. (But it does break distributed build caching, so it's not that g

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

thakis added a comment. >> I think I missed the workaround. I only saw the suggestion to write 0x2 >> instead of 2 which doesn't seem more clear at all to me. Was there another >> fix suggestion? > > Nope, you didn't miss it -- that was the suggestion. Using a hex notation is > the way we let

[PATCH] D66179: [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)

thakis added a comment. Similar to Comment at: cfe/trunk/unittests/Sema/CMakeLists.txt:14 clangAST + clangASTMatchers clangBasic Is it necessary to use ASTMachers to test this? It'd be good if SemaTests wouldn't have to depend on ASTMatchers (for linkin

[PATCH] D66179: [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)

thakis added inline comments. Comment at: cfe/trunk/unittests/Sema/GslOwnerPointerInference.cpp:9 + +#include "../ASTMatchers/ASTMatchersTest.h" +#include "clang/ASTMatchers/ASTMatchers.h" mgehre wrote: > thakis wrote: > > This weird relative include path is a hi

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

thakis added a comment. It looks like this renamed DynamicTypeMap.h but didn't update all users, see all the files in Checkers at http://llvm-cs.pcc.me.uk/?q=include.*dynamictypemap.h for example. Which targets did you try to build locally? Repository: rL LLVM CHANGES SINCE LAST ACTION h

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

thakis added a comment. See e.g. http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-gn/builds/4344/steps/ninja%20/logs/stdio for errors; I'm guessing most other bots on http://lab.llvm.org:8011/console will turn red in a bit too. Repository: rL LLVM CHANGES SINCE LAST ACTION https:

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

thakis added a comment. Thanks! Looks like it builds fine now, but the tests are failing: http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/19297/steps/test-check-all/logs/stdio FAIL: Clang :: Analysis/cast-value-state-dump.cpp (4352 of 48515) **

[PATCH] D66179: [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)

thakis added a comment. And, unrelatedly, for libc++, shouldn't we just add the attribute to the libc++ source instead of trying to infer it in the compiler? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66179/new/ https://reviews.llvm.org/D66179 _

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

thakis added a comment. > I don't understand how you draw that conclusion, but the reason behind why we > went with that as a way to silence the diagnostic is because using the prefix > acts as a signal that the developer wants to do bit manipulation more than > just a decimal literal does. It'

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

thakis added a comment. In D66397#1640685 , @xbolva00 wrote: > >> I think adding parens and casts are fairly well-understood to suppress > >> warnings. > > It should work here as well. #define ALPHA_OFFSET (3). Did anobody from > Chromium try it? > > >>

[PATCH] D66584: [clang-format] Support .mjs module javascript extension in clang-format

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Does clang-format format import and export lines in mjs files correctly? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66584/new/ https://reviews.llvm.org/D665

[PATCH] D66650: clang-format: Disable across-block include reordering.

thakis created this revision. thakis added reviewers: sammccall, krasimir, ioeric. https://reviews.llvm.org/D66650 Files: clang/lib/Format/Format.cpp Index: clang/lib/Format/Format.cpp === --- clang/lib/Format/Format.cpp +++ clan

[PATCH] D66650: clang-format: Disable across-block include reordering.

thakis added a comment. How's the work going to undo r357695 btw? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66650/new/ https://reviews.llvm.org/D66650 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bi

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

thakis added a comment. In D66397#1642012 , @Quuxplusone wrote: > @thakis wrote: > > > What was the motivation for firing on more than bare literals? > > Well, fundamentally, there is no difference in code quality between any of > these snippets: > > #d

[PATCH] D66653: [clang-format] Turn include regrouping on for Google ObjC style

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66653/new/ https://reviews.llvm.org/D66653 ___ cfe-commits

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

thakis added a comment. We're getting a bunch of errors looking like `../../AppsListViewController.m:11:37: error: incompatible block pointer types assigning to 'void (^)(__strong id)' from 'void (^)(AppCollectionViewCell *__strong)'` on code that looks fairly harmless to me. It looks somethin

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

thakis added a comment. Yes, that makes sense, thanks. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66831/new/ https://reviews.llvm.org/D66831 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llv

[PATCH] D66964: Sort Java imports without newline

thakis added a comment. Making the tests in this patch pass sounds good to me too :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66964/new/ https://reviews.llvm.org/D66964 ___ cfe-commits mailing lis

[PATCH] D66711: [clang] Warning for non-final classes with final destructors

thakis added a comment. This is a cool warning. Could the note have a fixit attached to it, so that it's easy to mass-clean-up the warning automatically? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66711/new/ https://reviews.llvm.org/D66711 _

[PATCH] D67202: Implement Microsoft-compatible mangling for decomposition declarations.

thakis added a comment. ps: Thanks for the patch, this is great :) pps: Since this uses struct mangling, `llvm-undname` can already demangle these names, right? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67202/new/ https://reviews.llvm.org/D67

[PATCH] D67202: Implement Microsoft-compatible mangling for decomposition declarations.

thakis added a comment. Does this pass `ninja check-clang`? If so, can you add a test for the new mangling, based on the manglings in the godbolt example? See `clang/test/CodeGenCXX/mangle-ms-*` for many examples; a new file mangle-ms-cxx20.cpp might be appropriate here. You can run `bin/llvm-l

[PATCH] D67202: Implement Microsoft-compatible mangling for decomposition declarations.

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Thanks! Re llvm-undname: That seems to match behavior of MSVC's undname, so I'd say that's all good :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

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

thakis created this revision. thakis added a reviewer: krasimir. Ports r359967 to clang-format. https://reviews.llvm.org/D67246 Files: clang/lib/Format/TokenAnnotator.cpp clang/lib/Format/UnwrappedLineParser.cpp clang/unittests/Format/FormatTest.cpp Index: clang/unittests/Format/FormatT

[PATCH] D33900: Print registered targets in clang's version information

thakis added a comment. dim: Does putting the target listing behind a different flag work for you? Which problem are you trying to solve here? https://reviews.llvm.org/D33900 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm

[PATCH] D36191: [CodeGen] Don't make availability attributes imply default visibility on macos

thakis added a comment. Awesome, thanks! Maybe mention the PR# for this in the commit message. https://reviews.llvm.org/D36191 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36238: Use "foo-12345.o" instead of "foo.o-12345" as temporary file name.

thakis created this revision. This helps some tools that do things based on the output's extension. For example, we got reports from users on Windows that have a tool that scan a build output dir (but skip .obj files). The tool would keep the "foo.obj-12345" file open, and then when clang tried

[PATCH] D27827: [ObjC] CodeGen support for @available on macOS

thakis added a comment. We just noticed that if you call __builtin_available() for the first time after activating your app's sandbox, the function will fail: SandboxViolation: crdmg(15489) deny file-read-data /System/Library/CoreServices/SystemVersion.plist Violation: deny file-read-data

[PATCH] D27827: [ObjC] CodeGen support for @available on macOS

thakis added a comment. Looks like the email reply didn't make it to phab, so here it is again: It's in this program, which is pretty stand-alone: https://cs.chromium.org/chromium/src/chrome/utility/safe_browsing/mac/crdmg.cc?q=crdmg&sq=package:chromium&l=95 EnableSandbox() is on line 134. clan

[PATCH] D27827: [ObjC] CodeGen support for @available on macOS

thakis added a comment. (Our workaround is to call __builtin_available() once before engaging the sandbox, which isn't so bad. Just thought I'd let you know about it; this isn't a serious bug for us.) Repository: rL LLVM https://reviews.llvm.org/D27827 ___

[PATCH] D33900: Print registered targets in clang's version information

thakis added a comment. I've reverted this in 309960, as discussed. https://reviews.llvm.org/D33900 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36238: Use "foo-12345.o" instead of "foo.o-12345" as temporary file name.

thakis added a comment. I'm going to land this. It's early in the 6.0 cycle, so if this causes issues, we should have time to find them and then follow up in case we run into any. https://reviews.llvm.org/D36238 ___ cfe-commits mailing list cfe-com

[PATCH] D36238: Use "foo-12345.o" instead of "foo.o-12345" as temporary file name.

thakis closed this revision. thakis added a comment. 309984. https://reviews.llvm.org/D36238 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36238: Use "foo-12345.o" instead of "foo.o-12345" as temporary file name.

thakis added a comment. Hm, that's unfortunate. I don't see a good way to rescue this patch. We could keep the lhs of this and always append ".tmp", then tools at least had a way of filtering out the temp files. Hm. https://reviews.llvm.org/D36238 ___

[PATCH] D36191: [CodeGen] Don't make availability attributes imply default visibility on macos

thakis added a comment. Is landing this blocked on internal testing? https://reviews.llvm.org/D36191 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36324: Integrate Kostya's clang-proto-fuzzer with LLVM.

thakis added a comment. Why should this be part of llvm? This seems to come with very heavy dependencies (protobuf), and LLVM has historically tried to minimize the number of things it depends on. https://reviews.llvm.org/D36324 ___ cfe-commits ma

[PATCH] D36413: Use "foo-12345.o.tmp" instead of "foo.o-12345" as temporary file name.

thakis created this revision. This helps some tools that do things based on the output's extension. For example, we got reports from users on Windows that have a tool that scan a build output dir (but skip .obj files). The tool would keep the "foo.obj-12345" file open, and then when clang tried

[PATCH] D36413: Use "foo-12345.o.tmp" instead of "foo.o-12345" as temporary file name.

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

[PATCH] D36526: [Sema] Assign new flag -Wenum-compare-switch to switch-related parts of -Wenum-compare

thakis accepted this revision. thakis added a comment. Thanks! https://reviews.llvm.org/D36526 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36857: [Driver] Set linkPath and MSVT version when cl.exe is detected, plus STL's hacking.

thakis added inline comments. Comment at: lib/Driver/ToolChains/MSVC.cpp:135 + llvm::SmallString<256> TestPath = PathEntry; + while(!TestPath.empty()) { TestPath = llvm::sys::path::parent_path(TestPath); Having a while loop going up the directo

[PATCH] D36857: [Driver] Set linkPath and MSVT version when cl.exe is detected, plus STL's hacking.

thakis added a comment. Oh, I guess this is superseded by https://reviews.llvm.org/D36860 ? https://reviews.llvm.org/D36857 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36860: [Driver] Recognize DevDiv internal builds of MSVC, with a different directory structure

thakis added a comment. This approach looks good to me. Is there any chance we could have a test for this? See `test/Driver` for examples; use `python bin\llvm-lit.py ../llvm/tools/clang/test/Driver/my_test.cpp` to run a single test, `ninja check-clang` to run all of them. You need some unix b

[PATCH] D36860: [Driver] Recognize DevDiv internal builds of MSVC, with a different directory structure

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Many driver tests check in a basic representative directory structure (e.g. test/Driver/Inputs/basic_freebsd_tree/ and its many siblings). But if you're happy with others breaking this code, I

[PATCH] D36914: Implement CFG construction for __try / __except / __leave.

thakis created this revision. This makes -Wunreachable-code work for programs containing SEH (except for __finally, which is still missing for now). __try is modeled like try (but simpler since it can only have a single __except or __finally), __except is fairly similar to catch (but simpler,

[PATCH] D36914: Implement CFG construction for __try / __except / __leave.

thakis added inline comments. Comment at: lib/Analysis/CFG.cpp:448 +BuildOpts(buildOpts), switchExclusivelyCovered(false), +switchCond(nullptr), cachedEntry(nullptr), lastLookup(nullptr) {} (this is now a no-op and only a clang-formatting of the

[PATCH] D36914: Implement CFG construction for __try / __except / __leave.

thakis updated this revision to Diff 112015. thakis edited the summary of this revision. thakis added a comment. Just use TryTerminatedBlock https://reviews.llvm.org/D36914 Files: lib/Analysis/CFG.cpp test/Sema/warn-unreachable-ms.c Index: test/Sema/warn-unreachable-ms.c ==

[PATCH] D36914: Implement CFG construction for __try / __except / __leave.

thakis updated this revision to Diff 112261. thakis added a comment. add test from rnk https://reviews.llvm.org/D36914 Files: lib/Analysis/CFG.cpp test/Sema/warn-unreachable-ms.c Index: test/Sema/warn-unreachable-ms.c === ---

[PATCH] D37090: Implement CFG construction for __finally.

thakis created this revision. This completes CFG construction for SEH AST nodes. Also tweak test `g3` to explicitly check that `__leave` in an `__except` leaves the outer `__try`. https://reviews.llvm.org/D37090 Files: lib/Analysis/CFG.cpp test/Sema/warn-unreachable-ms.c Index: test/Sema

[PATCH] D71554: [llvm-ranlib] Handle -D and -U command line flag

thakis added a comment. The test fails on Windows: http://45.33.8.238/win/4948/step_11.txt Please take a look, and if takes a while to fix, revert in the meantime. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71554/new/ https://reviews.llvm.org/D

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

thakis added inline comments. Comment at: llvm/test/lit.site.cfg.py.in:6 +def cmake_bool(val): +return val.lower() in (1, "on", "yes", "true", "y",) + FYI, this is a now-discouraged pattern. It's better to use llvm_canonicalize_cmake_booleans in the cmake fi

[PATCH] D71554: [llvm-ranlib] Handle -D and -U command line flag

thakis added a comment. In D71554#1801246 , @arichardson wrote: > Should be fixed in > https://github.com/llvm/llvm-project/commit/a4f3847f3d5742cfab7acdc614e7ca54643e0c85 Thanks! Now it fails with a different error: http://45.33.8.238/win/4962/step_11

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

thakis added a comment. This breaks tests on Windows: http://45.33.8.238/win/5061/step_8.txt Please take a look, and if takes a while to investigate, please revert in the meantime. (Probably needs the fno-delayed-template-instantiation kludge that's in many other tests.) Repository: rG LLVM

[PATCH] D72333: [clang-tidy] Disable match on `if constexpr` statements in template instantiation for `readability-misleading-indentation` check.

thakis added a comment. This broke tests on Windows: http://45.33.8.238/win/5297/step_8.txt (probably due to windows using -fdelayed-template-parsing by default). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72333/new/ https://reviews.llvm.org/D7

[PATCH] D72333: [clang-tidy] Disable match on `if constexpr` statements in template instantiation for `readability-misleading-indentation` check.

thakis added a comment. In D72333#1810142 , @Abpostelnicu wrote: > I think the best approach here is to remove the tests that don’t have > template instantiation. In any case, if it takes a while to come to some decision, please revert in the meantime

[PATCH] D72333: [clang-tidy] Disable match on `if constexpr` statements in template instantiation for `readability-misleading-indentation` check.

thakis added a comment. Any luck? The bot's been broken for close to 4h now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72333/new/ https://reviews.llvm.org/D72333 ___ cfe-commits mailing list cfe-co

[PATCH] D72405: Allow /D flags absent during PCH creation under msvc-compat

thakis added a comment. Why do you want this? Isn't it always easy to fix your build instead? In general, stricter is better when we can get away with it, and since we've had this behavior for a while… Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D72438: [clang-tidy] For checker `readability-misleading-indentation` update tests.

thakis added a comment. I'll point out that with this approach we lose all test coverage of the checker with delayed template parsing on, even though most of the test works in that mode and delayed template parsing is the default when targeting windows. Repository: rG LLVM Github Monorepo C

[PATCH] D43357: [Concepts] Function trailing requires clauses

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

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

thakis added a comment. Thanks for seeing this through! > env var or flag Our build setup can easily add commandline flags, but it can't easily add env vars. So I think this should have a driver mode flag. For people who asked for an env var: Why do you need an env var for this? Almost all co

[PATCH] D71301: [clang][IFS] Prevent Clang-IFS from Leaking symbols from inside a block.

thakis added a comment. This breaks tests on macOS: http://45.33.8.238/mac/5672/step_7.txt (macOS prefixes functions with an extra `_`.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71301/new/ https://reviews.llvm.org/D71301 __

[PATCH] D71301: [clang][IFS] Prevent Clang-IFS from Leaking symbols from inside a block.

thakis added a comment. This test fails on Windows: http://45.33.8.238/win/5658/step_7.txt Looks like this expects itanium mangling; maybe you want `-triple %itanium_abi_triple`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71301/new/ https://re

[PATCH] D71301: [clang][IFS] Prevent Clang-IFS from Leaking symbols from inside a block.

thakis added a comment. (And thanks for the mac fix!) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71301/new/ https://reviews.llvm.org/D71301 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D71301: [clang][IFS] Prevent Clang-IFS from Leaking symbols from inside a block.

thakis added a comment. In D71301#1818233 , @thakis wrote: > This test fails on Windows: http://45.33.8.238/win/5658/step_7.txt > > Looks like this expects itanium mangling; maybe you want `-triple > %itanium_abi_triple`? Since the bot's been red for a

[PATCH] D72500: [clangd] Show hover info for expressions

thakis added a comment. This breaks tests on Windows: http://45.33.8.238/win/5837/step_9.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72500/new/ https://reviews.llvm.org/D72500 ___ cfe-commits mai

[PATCH] D72769: Replace CLANG_SPAWN_CC1 env var with a driver mode flag

thakis added a comment. cc cfe-commits for completeness CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72769/new/ https://reviews.llvm.org/D72769 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/

[PATCH] D87163: [DSE] Switch to MemorySSA-backed DSE by default.

thakis added a comment. The fix-up breaks tests: http://45.33.8.238/linux/27687/step_12.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87163/new/ https://reviews.llvm.org/D87163 ___ cfe-commits mailin

[PATCH] D87163: [DSE] Switch to MemorySSA-backed DSE by default.

thakis added a comment. Heads-up: We're seeing fairly widespread test failures with this in Chromium that go away when we force this flag off. It's possible that it's due to only a small number of issues and they might be previously-benign UB (no idea), but I figured I'd give an early note. We'

[PATCH] D63697: clang-format: Fix error return when using inplace with stdin

thakis requested changes to this revision. thakis added a comment. This revision now requires changes to proceed. (marking this "request changes" to get it off my dashboard. please ping if you do add a test :) ) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63697

[PATCH] D87163: [DSE] Switch to MemorySSA-backed DSE by default.

thakis added a comment. It's not blocking us because we added an explicit flag to force this off to our build config for "normal" builds. (We still get the default-on behavior on our bots that build with trunk clang.) But if others are seeing problems with this too, I think it makes sense to re

[PATCH] D86156: [BFI] Make BFI information available through loop passes inside LoopStandardAnalysisResults

thakis added a comment. Looks like this breaks tests: http://45.33.8.238/linux/27942/step_12.txt Ptal, and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86156/new/ https://reviews.llvm.org/D86156 ___

[PATCH] D83004: [UpdateCCTestChecks] Include generated functions if asked

thakis added inline comments. Comment at: llvm/utils/UpdateTestChecks/common.py:645 + added = set() + for prefixes, tool_args, *rest in prefix_list: +for prefix in prefixes: LLVM still supports Python 2.7. This breaks devs and bots still using 2.7. Please

[PATCH] D86290: Move all fields of '-cc1' option related classes into def file databases

thakis added a comment. Looks like this broke tests on windows: http://45.33.8.238/win/23171/step_7.txt Please take a look, and if it takes a while to fix, please revert for now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86290/new/ https://rev

[PATCH] D87118: Add an explicit toggle for the static analyzer in clang-tidy

thakis created this revision. thakis added a reviewer: hans. Herald added subscribers: llvm-commits, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, a.sidorin, baloghadamsoftware, mgorny. Herald added a project: LLVM. thakis requested review of this revision. Instead of using CLANG_ENABLE_STATIC_AN

[PATCH] D87118: Add an explicit toggle for the static analyzer in clang-tidy

thakis added inline comments. Comment at: clang-tools-extra/CMakeLists.txt:4 +option(CLANG_TIDY_ENABLE_STATIC_ANALYZER + "Include static analyzer checks in clang-tidy" ON) + hans wrote: > Should this default to CLANG_ENABLE_STATIC_ANALYZER instead of ON? I had t

[PATCH] D87118: Add an explicit toggle for the static analyzer in clang-tidy

thakis added inline comments. Comment at: llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/BUILD.gn:18 + } else { +values += [ "CLANG_TIDY_ENABLE_STATIC_ANALYZER=" ] + } thakis wrote: > hans wrote: > > Why not =0? > Because it's a #cmakedefine01, and th

[PATCH] D87118: Add an explicit toggle for the static analyzer in clang-tidy

thakis added inline comments. Comment at: llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/BUILD.gn:18 + } else { +values += [ "CLANG_TIDY_ENABLE_STATIC_ANALYZER=" ] + } thakis wrote: > thakis wrote: > > hans wrote: > > > Why not =0? > > Because it's a

[PATCH] D92381: [clangd] Extract per-dir CDB cache to its own threadsafe class. NFC

thakis added a comment. Looks like this breaks tests on Windows: http://45.33.8.238/win/29513/step_9.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92381/new/ https://reviews.llvm.org/D92381 ___ cfe-c

[PATCH] D92915: [Driver] Add -f[no-]legacy-pass-manager to supersede -f[no-]experimental-new-pass-manager

thakis added a comment. Seems to break tests: http://45.33.8.238/linux/34907/step_7.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92915/new/ https://reviews.llvm.org/D92915 ___ cfe-commits mailing li

[PATCH] D92381: [clangd] Extract per-dir CDB cache to its own threadsafe class. NFC

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

[PATCH] D92955: [openmp] Remove clause from OMPKinds.def and use OMP.td info

thakis added inline comments. Comment at: clang/include/clang/AST/ASTTypeTraits.h:152 +#define CLAUSE_CLASS(Enum, Str, Class) NKI_##Class, +#include "llvm/Frontend/OpenMP/OMP.cpp.inc" NKI_NumberOfKinds Now we're even including the `cpp.inc` file in .h files.

[PATCH] D92955: [openmp] Remove clause from OMPKinds.def and use OMP.td info

thakis added a comment. And a less philosophical comment: This causes a bunch of Wcovered-switch-default warnings: ../../clang/include/clang/AST/OpenMPClause.h::5: warning: default label in switch which covers all enumeration values [-Wcovered-switch-default] ../../clang/include/clang/AST/R

[PATCH] D92971: clang-import-test: Clean up error output for files that cannot be found

thakis added a comment. This breaks tests on Windows: http://45.33.8.238/win/29686/step_7.txt PTAL, and please revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92971/new/ https://reviews.llvm.org/D92971 ___

[PATCH] D92971: clang-import-test: Clean up error output for files that cannot be found

thakis added a comment. Thanks for the quick fix! The bot is happy again. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92971/new/ https://reviews.llvm.org/D92971 ___ cfe-commits mailing list cfe-commits

[PATCH] D92191: [clang-scan-deps] Add support for clang-cl

thakis added a comment. Looks like this breaks tests on mac: http://45.33.8.238/macm1/174/step_6.txt Ptal, and revert for now if it takes a while to fix Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92191/new/ https://reviews.llvm.org/D92191

[PATCH] D92975: Lex: Migrate HeaderSearch::LoadedModuleMaps to FileEntryRef

thakis added a comment. One of the changes in https://github.com/llvm/llvm-project/compare/6af2c4ca9bd...2733a5a5b44 broke check-clang on windows. This change looks the most likely to me. Can you take a look? http://45.33.8.238/win/29852/step_7.txt Repository: rG LLVM Github Monorepo CHAN

[PATCH] D93258: [amdgpu] Default to code object v3

thakis added a comment. This seems to break check-clang everywhere: http://45.33.8.238/linux/35295/step_7.txt PTAL, and if it takes a while to investigate, please revert for now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93258/new/ https://re

[PATCH] D93258: [amdgpu] Default to code object v3

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

[PATCH] D92975: Lex: Migrate HeaderSearch::LoadedModuleMaps to FileEntryRef

thakis added a comment. Reverted in 7799ef7121aa7d59f4bd95cdf70035de724ead6f for now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92975/new/ https://reviews.llvm.org/D92975 ___ cfe-commits mailing list

[PATCH] D92975: Lex: Migrate HeaderSearch::LoadedModuleMaps to FileEntryRef

thakis added a comment. The revert helped, check-clang is no longer failing on Windows. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92975/new/ https://reviews.llvm.org/D92975 ___ cfe-commits mailing li

[PATCH] D93134: [FPEnv] Teach the IRBuilder about invoke's correct use of the strictfp attribute.

thakis added a comment. This breaks check-clang: http://45.33.8.238/linux/35362/step_7.txt PTAL, 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/D93134/new/ https://reviews.llvm.org/

[PATCH] D91980: [OpenMP] Add initial support for `omp [begin/end] assumes`

thakis added a comment. In addition to the build error, it also broke check-clang everywhere as far as I can tell. mac: http://45.33.8.238/macm1/292/step_6.txt linux: http://45.33.8.238/linux/35399/step_7.txt win: http://45.33.8.238/win/29934/step_7.txt Repository: rG LLVM Github Monorepo CH

[PATCH] D91980: [OpenMP] Add initial support for `omp [begin/end] assumes`

thakis added a comment. In D91980#2456637 , @jdoerfert wrote: > In D91980#2456600 , @thakis wrote: > >> In addition to the build error, it also broke check-clang everywhere as far >> as I can tell. >> mac: http://4

[PATCH] D93458: clang-cl: Remove /Zd flag

thakis created this revision. thakis added a reviewer: aeubanks. Herald added a subscriber: dang. thakis requested review of this revision. cl.exe doesn't understand Zd (in either MSVC 2017 or 2019), so neiter should we. It used to do the same as `-gline-tables-only` which is exposed as clang-cl f

[PATCH] D93458: clang-cl: Remove /Zd flag

This revision was automatically updated to reflect the committed changes. Closed by commit rG49c248bd62a3: clang-cl: Remove /Zd flag (authored by thakis). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93458/new/ https:/

[PATCH] D92663: [clangd] Add hot-reload of compile_commands.json and compile_flags.txt

thakis added a comment. Seems to break tests on windows: http://45.33.8.238/win/30129/step_9.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92663/new/ https://reviews.llvm.org/D92663 ___ cfe-commits m

[PATCH] D92663: [clangd] Add hot-reload of compile_commands.json and compile_flags.txt

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

<    4   5   6   7   8   9   10   11   12   13   >