[PATCH] D79417: Don't assert about missing profile info in createProfileWeightsForLoop

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Could there be a test for this? Or is that difficult? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79417/new/ https://reviews.llvm.org/D79417 _

[PATCH] D79417: Don't assert about missing profile info in createProfileWeightsForLoop

thakis added a comment. Fair enough :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79417/new/ https://reviews.llvm.org/D79417 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm

[PATCH] D79265: Let normalize() for posix style convert backslash to slash unconditionally.

This revision was automatically updated to reflect the committed changes. Closed by commit rG8fc7a907b93a: Let normalize() for posix style convert backslash to slash unconditionally. (authored by thakis). Herald added a project: clang. Changed prior to commit: https://reviews.llvm.org/D79265?vs

[PATCH] D79531: Make -Wnonportable-include-path ignore drive case on Windows.

thakis created this revision. thakis added a reviewer: hans. Herald added subscribers: llvm-commits, mgorny. Herald added a project: LLVM. See PR45812 for motivation. No explicit test since I couldn't figure out how to get the current disk drive in lower case into a form in lit where I could mkdi

[PATCH] D79531: Make -Wnonportable-include-path ignore drive case on Windows.

thakis marked an inline comment as done. thakis added inline comments. Comment at: clang/lib/Lex/PPDirectives.cpp:2118 +// given drive letter case as correct for the purpose of this warning. +SmallString<128> FixedDriveRealPath; +if (llvm::sys::path::is_absolute(Name)

[PATCH] D79210: Let clang print registered targets for --version

thakis added a comment. We've talked about this 3 years ago in https://reviews.llvm.org/D33900 and back then decided that this should go behind a different flag. Has anything changed since then? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79210/

[PATCH] D79210: Let clang print registered targets for --version

thakis added a comment. In D79210#2025009 , @thakis wrote: > We've talked about this 3 years ago in https://reviews.llvm.org/D33900 and > back then decided that this should go behind a different flag. Has anything > changed since then? To expand a bit:

[PATCH] D79210: Let clang print registered targets for --version

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

[PATCH] D79531: Make -Wnonportable-include-path ignore drive case on Windows.

thakis marked an inline comment as done. thakis added inline comments. Comment at: clang/lib/Lex/PPDirectives.cpp:2123 +isLowercase(Name[0]) != isLowercase(RealPathName[0])) { + assert(Components.size() >= 3 && "should have drive, backslash, name"); + FixedDrive

[PATCH] D79531: Make -Wnonportable-include-path ignore drive case on Windows.

thakis closed this revision. thakis marked an inline comment as done. thakis added a comment. Landed in d03838343f2199580 . Found another bug elsewhere while looking at this, will make a patch for that now.

[PATCH] D79595: Fix bugs when an included file name is typo corrected.

thakis created this revision. thakis added a reviewer: hans. D52774 fixed a bug with typo correction of includes, but didn't add a test. D65907 then broke recovery of typo correction of includes again, because it extracted the

[PATCH] D79599: Add a flag that controls if clang-tidy and clang-include-fixer are built into libclang.

thakis created this revision. thakis added a reviewer: bkramer. Herald added subscribers: llvm-commits, mgorny. Herald added a project: LLVM. Based on the discussion on D55415 , also make the flag default to false. Having libclang depend on clang-tools-extra means

[PATCH] D79599: Add a flag that controls if clang-tidy and clang-include-fixer are built into libclang.

thakis marked 2 inline comments as done. thakis added a comment. Thanks! Added a release note. I had cc'd everyone who had commented on D55415 -- did I miss anyone? Comment at: clang/tools/libclang/CMakeLists.txt:65 + else () +message(FAT

[PATCH] D79599: Add a flag that controls if clang-tidy and clang-include-fixer are built into libclang.

This revision was automatically updated to reflect the committed changes. thakis marked an inline comment as done. Closed by commit rGe9b411390285: Add a flag that controls if clang-tidy and clang-include-fixer are built into… (authored by thakis). Herald added a project: clang. Changed prior to

[PATCH] D79595: Fix bugs when an included file name is typo corrected.

This revision was automatically updated to reflect the committed changes. Closed by commit rG51d938bc9443: Fix bugs when an included file name is typo corrected. (authored by thakis). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D79655: [WebAssembly] Handle exception specifications

thakis added a comment. The revert broke check-clang on several bots, e.g. here http://lab.llvm.org:8011/builders/clang-ppc64be-linux-lnt/builds/38159 Can you take a look please? The tree's been red all of today due to this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D79655: [WebAssembly] Handle exception specifications

thakis added inline comments. Comment at: clang/test/CodeGenCXX/wasm-eh.cpp:387 +// RUN: %clang_cc1 %s -triple wasm32-unknown-unknown -fms-extensions -fexceptions -fcxx-exceptions -fwasm-exceptions -target-feature +exception-handling -emit-llvm -std=c++11 2>&1 | FileCheck %s

[PATCH] D79678: [clangd] Add CSV export for trace metrics

thakis added a comment. This broke tests on Windows: http://45.33.8.238/win/15539/step_9.txt Please take a look, and revert if it takes a while to investigate. (I'll note that the presubmit bot now has a win/msvc bot that failed compile and reported this on this review before it got merged: ht

[PATCH] D78121: [analyzer][DirectIvarAssignment] Turn DirectIvarAssignmentForAnnotatedFunctions into a checker option

thakis added a comment. Sorry, wrong link, I meant http://45.33.8.238/linux/18049/step_7.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78121/new/ https://reviews.llvm.org/D78121 ___ cfe-commits mai

[PATCH] D78121: [analyzer][DirectIvarAssignment] Turn DirectIvarAssignmentForAnnotatedFunctions into a checker option

thakis added a comment. Looks like this breaks check-clang: http://45.33.8.238/ Please take a look. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78121/new/ https://reviews.llvm.org/D78121 ___ cfe-com

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

thakis added a comment. This breaks tests on Mac and windows, see eg http://45.33.8.238/macm1/43791/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131469/new/ https://rev

[PATCH] D131465: C++/ObjC++: switch to gnu++17 as the default standard

thakis added inline comments. Comment at: clang/test/lit.site.cfg.py.in:27 config.clang_enable_opaque_pointers = @CLANG_ENABLE_OPAQUE_POINTERS_INTERNAL@ +config.clang_default_std_cxx = "@CLANG_DEFAULT_STD_CXX@" config.clang_default_cxx_stdlib = "@CLANG_DEFAULT_CXX_STDLIB@"

[PATCH] D133841: [test][clang] run test for lld emitting dwarf-aranages only if lld is presented

thakis added a comment. As far as I can tell, this broke check-clang everywhere. (See e.g.: https://lab.llvm.org/buildbot/#/builders/139/builds/28039 and many of the bots on https://lab.llvm.org/buildbot/#/console Before landing changes, please: - run tests locally - wait for the presubmit bot

[PATCH] D133841: [test][clang] run test for lld emitting dwarf-aranages only if lld is presented

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

[PATCH] D133092: [clang] fix generation of .debug_aranges with LTO

thakis added a comment. Reverted this in 5631d20bfc9f77d15435badf6ce34e1a56c27e1c since the fix for the test failures this change here caused got reverted due to breaking other tests. Let's regroup and make a new patch that

[PATCH] D83015: [Driver] Add --ld-path= and deprecate -fuse-ld=/abs/path and -fuse-ld=rel/path

thakis added a comment. Herald added a subscriber: StephenFan. Herald added a project: All. `LinkerIsLLD` isn't getting set when `--ld-path` is used. (It also isn't getting set when using `-fuse-ld=` with an absolute path.) How do you imagine using an absolute path and telling clang "the linker

[PATCH] D127911: Delete 'llvm.asan.globals' for global metadata.

thakis added a comment. Looks like this breaks check-clang on windows: http://45.33.8.238/win/61067/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127911/new/ https://rev

[PATCH] D128984: [clang-format] Tweak help text a bit

thakis created this revision. thakis added reviewers: curdeius, MyDeveloperDay. Herald added a project: All. thakis requested review of this revision. In particular, make it clear that `--style=file` is the default, since there's some confusion about this, e.g. here: https://stackoverflow.com/ques

[PATCH] D128991: [NFC] Add a missing test for for clang-repl

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

[PATCH] D128984: [clang-format] Tweak help text a bit

This revision was automatically updated to reflect the committed changes. Closed by commit rGb1f0efc06acc: [clang-format] Tweak help text a bit (authored by thakis). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128984/

[PATCH] D129050: [clang-format] Update documentation

thakis created this revision. thakis added reviewers: MyDeveloperDay, curdeius. Herald added a project: All. thakis requested review of this revision. - Update `clang-format --help` output after b1f0efc06acc . - Update `clang-fo

[PATCH] D129050: [clang-format] Update documentation

thakis added inline comments. Comment at: clang/docs/ClangFormat.rst:233 Script for patch reformatting = (As far as I can tell, we don't have `git clang-format` docs anywhere? But that's for a different patch.) CHANGES SINCE LAS

[PATCH] D129050: [clang-format] Update documentation

This revision was automatically updated to reflect the committed changes. Closed by commit rG1ab37d996ce3: [clang-format] Update documentation (authored by thakis). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129050/n

[PATCH] D128715: [clang-tidy] Fix confusable identifiers interaction with DeclContext

thakis added a comment. Looks like this breaks tests on windows: http://45.33.8.238/win/61559/step_8.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128715/new/ https://reviews.llvm.org/D128715 ___ cfe

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

thakis added a comment. As far as I can tell, this breaks check-clang everywhere: http://45.33.8.238/ Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128059/new/ https://reviews.llvm.

[PATCH] D129223: [Clang] Fix invalid utf-8 detection

thakis added a comment. This breaks check-clang: http://45.33.8.238/linux/80462/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129223/new/ https://reviews.llvm.org/D129223

[PATCH] D129226: [clang/mac] Make -mmacos-version-min the canonical spelling over -mmacosx-version-min

thakis created this revision. thakis added reviewers: arphaman, dexonsmith. Herald added a project: All. thakis requested review of this revision. Herald added a subscriber: MaskRay. This was promised 5 years ago in https://reviews.llvm.org/D32796, let's do it. Both flags are still accepted. No b

[PATCH] D129223: [Clang] Fix invalid utf-8 detection

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

[PATCH] D129223: [Clang] Fix invalid utf-8 detection

thakis added a comment. In D129223#3633754 , @thakis wrote: > This breaks check-clang: http://45.33.8.238/linux/80462/step_7.txt (Also on all other bots, e.g. https://lab.llvm.org/buildbot/#/builders/171/builds/17060 – did you run tests locally before

[PATCH] D120619: [clangd] Support include-fixer inside macro arguments.

thakis added a comment. This breaks tests on windows: http://45.33.8.238/win/54910/step_9.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120619/new/ https://reviews.llvm.org/D120

[PATCH] D122344: [clang][driver] Disable non-functional --version option for clang -cc1

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

[PATCH] D120185: [ASTMatchers] Output currently processing match and nodes on crash

thakis added a comment. That bot sets ENABLE_BACKTRACES=1. It seems to work for clang-cl (foo.cc contains `#pragma clang __debug parser_crash`): >out\gn\bin\clang-cl /c foo.cc PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, prepro

[PATCH] D122547: [clang] Make Driver tests pass when running with temp dir containing "crt"

thakis created this revision. thakis added a reviewer: hans. Herald added subscribers: abrachet, emaste. Herald added a project: All. thakis requested review of this revision. In a recent run, temp files got created in /tmp/lit-tmp-2wcrtcx1/foo-xxx.o. Since the tmp path contained "crt", this made

[PATCH] D122547: [clang] Make Driver tests pass when running with temp dir containing "crt"

This revision was automatically updated to reflect the committed changes. Closed by commit rG67aea3b65cc5: [clang] Make Driver tests pass when running with temp dir containing "crt" (authored by thakis). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D122611: [clang][extract-api] Add support for macros

thakis added a comment. Looks like either this or your other commit at the same time broke tests on windows: http://45.33.8.238/win/55345/step_7.txt Please take a look, and revert for now if it takes a while to fix. (Also, please add a "Differential Revision: https://reviews.llvm.org/D122611";

[PATCH] D122611: [clang][extract-api] Add support for macros

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

[PATCH] D122766: [clang] Use forward slash as the path separator for Windows in __FILE__, __builtin_FILE(), and std::source_location

thakis added a comment. As said elsewhere, I think this is too simplistic. It's good to make this dependent on the triple, instead of on the host platform. But we should have a flag that controls which slash direction to use on windows triples, since different people will want different things.

[PATCH] D122766: [clang] Use forward slash as the path separator for Windows in __FILE__, __builtin_FILE(), and std::source_location

thakis added a comment. (I don't think the argument about '\' in include lines applies to this patch (?)) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122766/new/ https://reviews.llvm.org/D122766 ___ c

[PATCH] D120306: [clangd] IncludeCleaner: Add support for IWYU pragma private

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

[PATCH] D122766: [clang] Use forward slash as the path separator for Windows in __FILE__, __builtin_FILE(), and std::source_location

thakis added a comment. Do you know what cl.exe does to paths in pdb files? Does it write mixed-slashiness for foo/bar.h includes, or does it write backslashes throughout? Windows can handle slashes, but several tools can't. I worry that if we do something different than cl, some random corner

[PATCH] D122766: [clang] Use forward slash as the path separator for Windows in __FILE__, __builtin_FILE(), and std::source_location

thakis added a comment. In D122766#3421410 , @mstorsjo wrote: > In D122766#3420925 , @thakis wrote: > >> Windows can handle slashes, but several tools can't. I worry that if we do >> something different than cl,

[PATCH] D122820: [GH54588]Fix ItaniumMangler for NTTP unnamed unions w/ unnamed structs

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

[PATCH] D122820: [GH54588]Fix ItaniumMangler for NTTP unnamed unions w/ unnamed structs

thakis added a comment. Reverted in 50186b63d1807d389f31c515377d94185795ab44 for now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122820/new/ https://reviews.llvm.org/D122820 ___ cfe-commits mailing li

[PATCH] D122820: [GH54588]Fix ItaniumMangler for NTTP unnamed unions w/ unnamed structs

thakis added a comment. In D122820#3426132 , @erichkeane wrote: > In D122820#3423712 , @thakis wrote: > >> Looks like this breaks tests on macOS: >> http://45.33.8.238/macm1/31733/step_7.txt >> >> Please take a

[PATCH] D122820: [GH54588]Fix ItaniumMangler for NTTP unnamed unions w/ unnamed structs

thakis added a comment. > Oh hm, if it's due to the wrong llvm-cxxfilt being called, maybe we just need > to update clang/test/lit.cfg.py to add llvm-cxxfilt to `tools` to make sure > it uses the just-built one instead of the one on PATH (? not sure). Actually, I consistently saw this failing

[PATCH] D123031: [clangd] Use stable keys for CanonicalIncludes mappings

thakis added a comment. Looks like this breaks tests on Windows: http://45.33.8.238/win/55690/step_9.txt please take a look, etc :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123031/new/ https://reviews.llvm.org/D123031 ___

[PATCH] D122766: [clang] Use forward slash as the path separator for Windows in __FILE__, __builtin_FILE(), and std::source_location

thakis added a comment. Normalizing to `\` seems better to me than normalizing to `/` too. I abstractly like changing as little as necessary at every stage, which in this case would mean changing just the slashiness of slashes that clang itself adds, so I'm still weakly in favor of that. But I

[PATCH] D130443: [CGDebugInfo] Access the current working directory from the `VFS`

thakis added a comment. Looks like this doesn't build: http://45.33.8.238/linux/82380/step_4.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130443/new/ https://reviews.llvm.org/D130443 ___ cfe-commits

[PATCH] D129798: [clang-tidy] Rename the make-confusable-table executable

thakis added inline comments. Comment at: llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/misc/ConfusableTable/BUILD.gn:1 -executable("make-confusable-table") { +executable("clang-tidy-confusable-chars-gen") { deps = [ "//llvm/lib/Support" ] Thanks for a

[PATCH] D130689: [LLVM] Update C++ standard to 17

thakis added a comment. It'd be nice if this was landed opt-in behind some cmake variable at first, so that folks could try it out on their bots and see how well things work. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130689/new/ https://review

[PATCH] D130058: [Clang] Diagnose ill-formed constant expression when setting a non fixed enum to a value outside the range of the enumeration values

thakis added a comment. Hello, this breaks a bunch of existing code over here (and I imagine elsewhere). Can we make this a warning that's mapped to an error by default, so that people can turn it off for a while while they fix their code? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D130058: [Clang] Diagnose ill-formed constant expression when setting a non fixed enum to a value outside the range of the enumeration values

thakis added a comment. Here's a reduced repro of one case: % cat test.cc typedef enum VkResult { VK_ERROR_INVALID_OPAQUE_CAPTURE_ADDRESS = -1000257000, VK_RESULT_MAX_ENUM = 0x7FFF } VkResult; constexpr VkResult VK_FAKE_DEVICE_OOM_FOR_TESTING = static_cast(VK_RESULT_MA

[PATCH] D130058: [Clang] Diagnose ill-formed constant expression when setting a non fixed enum to a value outside the range of the enumeration values

thakis added a comment. In D130058#3687868 , @aaron.ballman wrote: > In D130058#3687680 , @thakis wrote: > >> Hello, this breaks a bunch of existing code over here (and I imagine >> elsewhere). > > Do you have a

[PATCH] D130689: [LLVM] Update C++ standard to 17

thakis added a comment. In D130689#3686716 , @thieta wrote: > You can already test this with `-DCMAKE_CXX_STANDARD=17` afaik. I wonder how > many bot owners would actually test this if we made another flag available. Thanks, that works. Our linux and w

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

thakis added a comment. This breaks tests on mac and win: http://45.33.8.238/macm1/41545/step_7.txt http://45.33.8.238/win/63579/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D129048: Rewording the "static_assert" to static assertion

thakis added a comment. This change caused a ton of churn, for what I understand fix the diag in C mode when assert.h is not included. IMHO, it would've been better to change the diag to say either `static_assert` as before in c++ mode or when assert.h is included (i.e. almost always), and `_S

[PATCH] D35379: Add documentation for @available

thakis accepted this revision. thakis marked an inline comment as done. thakis added a comment. This revision is now accepted and ready to land. Herald added a reviewer: aaron.ballman. Herald added a project: All. landed long ago in 11cafc82b72a80fb9a7266bbda7fc7a0cc1b51a0 / 564004ae78c61a9292a15

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

thakis added inline comments. Comment at: clang/lib/Driver/Driver.cpp:1638 +llvm::opt::ArgStringList ArgList = NewLLDInvocation.getArguments(); +ArgList.push_back(Saver.save(Twine{"--reproduce="} + TmpName).data()); +NewLLDInvocation.replaceArguments(std::move(ArgList

[PATCH] D130808: [InstrProf] Add new format for -fprofile-list=

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

[PATCH] D130808: [InstrProf] Add new format for -fprofile-list=

thakis added a comment. Reverted in 0eb7d86f5873ce897894339a3cc5bc69ca507bee for now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130808/new/ https://reviews.llvm.org/D130808 ___ cfe-commits mailing li

[PATCH] D130689: [LLVM] Update C++ standard to 17

thakis added a comment. In D130689#3696186 , @thieta wrote: > @nikic @thakis I fixed this issue in https://reviews.llvm.org/D131063 and it > can be built with CXX_STANDARD=17 and OSX_DEPLOYMENT_TARGET=10.11. Thanks! We took this opportunity to bump our

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

thakis added a comment. It's already an error, but it's a warning default-mapped to an error. You can -Wno-error=name to downgrade it into a warning, but that requires an explicit action. So people are unlikely to miss it. This is how we usually handle these breaking changes. Maybe there could

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

thakis added a comment. Default-mapped-to-error and release notes seems like a pretty strong signal to me. I think this is fine as is. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131307/new/ https://reviews.llvm.org/D131307 ___ cfe-commits

[PATCH] D130689: [LLVM] Update C++ standard to 17

thakis added a comment. Re MSVC: For what it's worth, our Chromium windows bot (which builds trunk llvm and then chromium with that) is happy. It builds with MSVC 2019. Here's the build log (which includes cmake invocations): https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket/88

[PATCH] D131214: [clang][Driver] Pass correct reproduce flag to lld-link

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Thanks! LG with comment. Comment at: clang/lib/Driver/Driver.cpp:1638 llvm::opt::ArgStringList ArgList = NewLLDInvocation.getArguments(); -ArgList.push_back(Saver.sa

[PATCH] D131528: [Clang] Restrict non fixed enum to a value outside the range of the enumeration values warning to context requiring a constant expression

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Lg CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131528/new/ https://reviews.llvm.org/D131528 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D130689: [LLVM] Update C++ standard to 17

thakis added a comment. Re rollout: I suggested further up to put this behind an opt-in variable. That would've allowed people to test this on their setups. I still think that's a good suggestion, and it would've identified the MSVC issue at least. Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D131528: [Clang] Restrict non fixed enum to a value outside the range of the enumeration values warning to context requiring a constant expression

thakis added a comment. Can this land? Our bots have been red since the default-error-mapped-warning patch landed... CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131528/new/ https://reviews.llvm.org/D131528 ___ cfe-commits mailing list cfe-

[PATCH] D104536: [clang][deps] Avoid minimizing PCH input files

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

[PATCH] D104904: [OpenMP][AMDGCN] Initial math headers support

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

[PATCH] D106535: [clangd] Use CommandMangler in TestTU

thakis added a comment. One of your 4 commits in https://github.com/llvm/llvm-project/compare/f86570cd5241...ba5dd945ad91 broke check-clangd on arm macs: http://45.33.8.238/macm1/14648/step_9.txt Please take a look, and revert for now if it takes a while to fix. (And consider spreading out co

[PATCH] D106733: [clang/darwin] Pass libclang_rt.profile last on linker command

thakis added a comment. Whoops, I'm so used to sending lld changes that I put the wrong reviewers and subscribers at first. Fixed now. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106733/new/ https://reviews.llvm.org/D106733 ___ cfe-commits

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

thakis added inline comments. Comment at: clang/tools/libclang/CMakeLists.txt:67 set(LLVM_EXPORTED_SYMBOL_FILE ${CMAKE_CURRENT_SOURCE_DIR}/libclang.exports) +set(LIBCLANG_VERSION_SCRIPT_FILE ${CMAKE_CURRENT_SOURCE_DIR}/libclang.map) This is still set to CURREN

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

thakis added inline comments. Comment at: clang/test/LibClang/symbols.test:2 +# Check that there are no unversioned clang symbols in libclang.so +RUN: llvm-nm -Dj --defined-only %libclang | grep -v -e '@@LLVM_[0-9]\+$' | not grep '^clang' + I think it's possible

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

thakis added inline comments. Comment at: clang/test/LibClang/symbols.test:2 +# Check that there are no unversioned clang symbols in libclang.so +RUN: llvm-nm -Dj --defined-only %libclang | grep -v -e '@@LLVM_[0-9]\+$' | not grep '^clang' + thakis wrote: > I thi

[PATCH] D106733: [clang/darwin] Pass libclang_rt.profile last on linker command

thakis added a comment. @vsk, I'll go ahead and land this. If this does cause problems for you after all (on the bug you said it should be fine), please revert :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106733/new/ https://reviews.llvm.org/D106733 ___

[PATCH] D106733: [clang/darwin] Pass libclang_rt.profile last on linker command

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG452095fe2f58: [clang/darwin] Pass libclang_rt.profile last on link

[PATCH] D106688: [AIX] Pass the -b option to linker on AIX

thakis added a comment. This seems to break tests everywhere, see e.g. https://lab.llvm.org/buildbot/#/builders/109/builds/19680 PTAL. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106688/new/ https://reviews.llvm.org/D106688 ___

[PATCH] D121269: [clang-format] Fix namespace format when the name is followed by a macro

thakis resigned from this revision. thakis added a comment. (deferring to MyDeveloperDay and curdeius) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121269/new/ https://reviews.llvm.org/D121269 ___ cfe-c

[PATCH] D121233: [pseudo] Move pseudoparser from clang to clang-tools-extra

thakis added a comment. Is there a reason why this can't be part of clang-tools-extra/test and check-clang-tools? Why is this in a directory called "pseudo" instead of "clang-pseudo"? This makes everything fairly self-inconsistent (`pseudo/tool` vs `clang-pseudo` in there, but then also `check

[PATCH] D121233: [pseudo] Move pseudoparser from clang to clang-tools-extra

thakis added a comment. In D121233#3384567 , @sammccall wrote: > In D121233#3384495 , @thakis wrote: > >> Is there a reason why this can't be part of clang-tools-extra/test > > This was discussed pretty extensivel

[PATCH] D116385: [clangd] Code action for creating an ObjC initializer

thakis added a comment. Looks like this breaks tests: http://45.33.8.238/linux/71327/step_9.txt Please take a look and revert for now if it takes a while to fix Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116385/new/ https://reviews.llvm.org/D11

[PATCH] D121837: [OpenMP][FIX] Allow device constructors for AMD GPU

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

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

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

[PATCH] D121283: [Clang] Support multiple attributes in a single pragma

thakis added a comment. The commit message got a bit mutilated: 33a9eac6aaa495fce6fd9b17cd48aa57a95461e6 Just FYI in case you didn't notice. In that case, update your commit workflow to make sure this doesn't happen next tim

[PATCH] D120185: [ASTMatchers] Output currently processing match and nodes on crash

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

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

thakis added a comment. We're seeing the same problem here https://bugs.chromium.org/p/chromium/issues/detail?id=1314297&q=linux_upload_clang&can=2 Reverted in e22a60b1c898a760a73417fa225c2fbe0609a69f for now. Repository:

[PATCH] D120989: Support debug info for alias variable

thakis added a comment. This breaks tests on mac: https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket/8817534773179515649/+/u/package_clang/stdout?format=raw Script: -- : 'RUN: at line 1'; /opt/s/w/ir/cache/builder/src/third_party/llvm-bootstrap/bin/clang -cc1 -interna

[PATCH] D121637: [PowerPC] Fix EmitPPCBuiltinExpr to emit arguments once

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

[PATCH] D123405: [dllexport] odr-use constexpr default args for constructor closures

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

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