[PATCH] D147875: [clang][Diagnostics] Show line numbers when printing code snippets

kwk added a comment. @tbaeder I have a small suggestion. Comment at: clang/lib/Frontend/TextDiagnostic.cpp:1121-1138 +static unsigned getNumDisplayWidth(unsigned N) { + if (N < 10) +return 1; + if (N < 100) +return 2; + if (N < 1'000) +return 3;

[PATCH] D149641: [docs] Hide collaboration and include graphs in doxygen docs

kwk accepted this revision. kwk added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149641/new/ https://reviews.llvm.org/D149641 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D138472: clang/cmake: Use installed gtest libraries for stand-alone builds

kwk accepted this revision. kwk added a comment. This revision is now accepted and ready to land. Working for me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138472/new/ https://reviews.llvm.org/D138472 __

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

kwk added a comment. In D141581#4071617 , @thesamesam wrote: > This is currently holding back further testing on our end which is concerning > me a bit, especially as we approach the branch point. Could you revert this > please if a fix isn't imminent?

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

kwk added a comment. @tstellar as you suspected, I had the following problem when building clang in standalone mode and your patch solved it for me. Thank you. -- Configuring done CMake Error at /usr/lib64/cmake/llvm/AddLLVM.cmake:536 (add_dependencies): The dependency target "RISCVTarge

[PATCH] D141050: [standalone-build] outsource, simplify and unify repetitive CMake code

kwk updated this revision to Diff 488582. kwk added a comment. - both, build tree and install method support find_package by setting different LLVM_CMAKE_DIR Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141050/new/ https://reviews.llvm.org/D14105

[PATCH] D141050: [standalone-build] outsource, simplify and unify repetitive CMake code

kwk updated this revision to Diff 487826. kwk added a comment. - Make require_llvm_utility_binary_path usable from a build-tree setup Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141050/new/ https://reviews.llvm.org/D141050 Files: clang/CMakeLi

[PATCH] D141050: [standalone-build] outsource, simplify and unify repetitive CMake code

kwk added a comment. Thank you @phosek for your review! I really appreciate it. I've addressed all of your comments. Could you give it another review? Comment at: clang/CMakeLists.txt:35 + include(StandaloneBuildHelpers) + get_llvm_utility_binary_path("llvm-tblgen" "LLVM_TAB

[PATCH] D141050: [standalone-build] outsource, simplify and unify repetitive CMake code

kwk updated this revision to Diff 487750. kwk marked 2 inline comments as done. kwk added a comment. - Don't quote output variables - Use 2 space indention in CMake files - Make out_var optional for get_llvm_utility_binary_path - Rename get_llvm_utility_binary_path -> require_llvm_utility_binary_p

[PATCH] D141050: [standalone-build] outsource, simplify and unify repetitive CMake code

kwk created this revision. kwk added reviewers: tstellar, phosek. Herald added subscribers: bzcheeseman, rriddle. Herald added a project: All. kwk requested review of this revision. Herald added subscribers: cfe-commits, stephenneuendorffer. Herald added a project: clang. Rationale = This

[PATCH] D138472: clang/cmake: Use installed gtest libraries for stand-alone builds

kwk added inline comments. Comment at: clang/CMakeLists.txt:103 if (LLVM_LIT AND LLVM_UTILS_PROVIDED) - set(UNITTEST_DIR ${LLVM_THIRD_PARTY_DIR}/utils/unittest) - if(EXISTS ${UNITTEST_DIR}/googletest/include/gtest/gtest.h - AND NOT EXISTS ${LLVM_LIBRARY_DI

[PATCH] D138258: clang/cmake: Fix incorrectly disabling tests when LLVM_EXTERNAL_LIT is used

kwk requested changes to this revision. kwk added a comment. This revision now requires changes to proceed. As much as I would like this to be fixed. I vote against this patch because in `lld/CMakeLists.txt` there's an almost (if not entirely) identical piece of code

[PATCH] D134852: [clang-format][NFC] Clean up class HeaderIncludes and Format.cpp

kwk accepted this revision. kwk added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Format/Format.cpp:2774 - -const char CppIncludeRegexPattern[] = -R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))"; owenpan

[PATCH] D134852: [clang-format][NFC] Clean up class HeaderIncludes and Format.cpp

kwk added inline comments. Comment at: clang/lib/Format/Format.cpp:2774 - -const char CppIncludeRegexPattern[] = -R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))"; owenpan wrote: > kwk wrote: > > MyDeveloperDay wrote: > > > Did I miss where this co

[PATCH] D134733: [clang-format][chore] transparent #include name regex

kwk updated this revision to Diff 464196. kwk added a comment. - Make include regex a static member and not a function - Run clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134733/new/ https://reviews.llvm.org/D134733 Files: clang/inc

[PATCH] D134733: [clang-format][chore] transparent #include name regex

kwk added inline comments. Comment at: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp:407 +const llvm::SmallVectorImpl &Matches) { + if (Matches.size() >= 3) { +return Matches[2]; MyDeveloperDay wrote: > kwk wrote: > > MyDeveloperDay wrote: > > > ‘>= 2’

[PATCH] D134852: [clang-format][NFC] Clean up class HeaderIncludes and Format.cpp

kwk added a comment. I didn't see much difference in what this patch does compare to mine but I saw that it removes the need for instantiating multiple `llvm::Regex` objects from a single static pattern. But that's something I've just done in a new revision of my own patch: https://reviews.llvm

[PATCH] D134733: [clang-format][chore] transparent #include name regex

kwk updated this revision to Diff 463786. kwk added a comment. - Remove ad-hoc instantiated IncludeRegex objects Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134733/new/ https://reviews.llvm.org/D134733 Files: clang/include/clang/Tooling/Inclus

[PATCH] D134733: [clang-format][chore] transparent #include name regex

kwk added a comment. @MyDeveloperDay please see my potentially uneducated comments. Comment at: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp:400 +llvm::Regex getCppIncludeRegex() { + static const char CppIncludeRegexPattern[] = + R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk abandoned this revision. kwk added a comment. In D121370#3817753 , @MyDeveloperDay wrote: > @kwk have we bottomed out on this issue? its been stale for a bit. If we are > not going to work on it further can be "Abandon" the review to get it off our

[PATCH] D134733: [clang-format][chore] transparent #include name regex

kwk created this revision. kwk added a reviewer: MyDeveloperDay. Herald added a project: All. kwk requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The foundation for this patch was done in: https://reviews.llvm.org/D121370. I wanted to "resc

[PATCH] D133801: Extraction of a matcher for an unused value from an expression from the bugprone-unused-return-value check

kwk added inline comments. Comment at: clang-tools-extra/unittests/clang-tidy/MatchersTest.cpp:2 +#include "utils/Matchers.h" +#include "../../clang/unittests/ASTMatchers/ASTMatchersTest.h" + njames93 wrote: > @kwk I seem to recall you saying in another patch tha

[PATCH] D56303: [clang-tidy] Recognize labelled statements when simplifying boolean exprs

kwk added subscribers: tstellar, serge-sans-paille, kwk. kwk added inline comments. Herald added a project: All. Comment at: clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp:1 +#include "../../clang/unittests/ASTMatchers/ASTMatchersTest.h" #include "ClangTidyTest

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk added a comment. I found another problem with both regular expressions (pre mine and mine): They cannot find /* a comment before */ #include Is this a known problem @MyDeveloperDay @owenpan @krasimir ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk planned changes to this revision. kwk added a comment. Some thing that doesn't work at the moment is ordering an include like this: #include /*some include*/ "wontwork.h" I wonder if it worked before. Let's see. Yes it worked. Grrr. I guess I need to do more... Repository: rG LLVM Git

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk added a comment. @krasimir could you please test this patch on your side? I've reopened it and tested it with the following file that contains both `//` and `/* ... */` trailing comments on include lines followed by `foo` and alike. The includes use `<...>` and `"..."` notation to locate fi

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk updated this revision to Diff 426701. kwk added a comment. Address trailing comments on include lines by ignoring slashes in characters that lead up to the match groups. ^[\t\ ]*[@#][\t\ ]*(import|include)([^"/]*("[^"]+")|[^]+>)|[\t/\ ]*([^;]+;)) ~

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk added a comment. In D121370#3471183 , @krasimir wrote: > It appears that this regressed include sorting in the following case, where > the contents of `test.h` show the expected include order and the > `clang-format` behavior before this patch: > >

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd46fa023caa2: [clang-format] SortIncludes should support "@import" lines in Objective-C (authored by kwk). Changed prior to commit: https://review

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk updated this revision to Diff 423584. kwk added a comment. - typo: @include bar; -> @import bar; Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121370/new/ https://reviews.llvm.org/D121370 Files: clang/include/clang/Tooling/Inclusions/HeaderI

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk marked 2 inline comments as done. kwk added a comment. In D121370#3456223 , @owenpan wrote: > Should we handle `#import` and `@import` for Object-C only so as to simply > the regex for C++? @owenpan my counter question probably reveals how few thing

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk updated this revision to Diff 423582. kwk added a comment. - Make functions lowercase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121370/new/ https://reviews.llvm.org/D121370 Files: clang/include/clang/Tooling/Inclusions/HeaderIncludes.h

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk marked 3 inline comments as done. kwk added inline comments. Comment at: clang/lib/Format/Format.cpp:2692-2695 + for (int i = Matches.size() - 1; i > 0; i--) { +if (!Matches[i].empty()) + return Matches[i]; + } owenpan wrote: > I think you missed `

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk updated this revision to Diff 423257. kwk marked an inline comment as done. kwk added a comment. - Make function static Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121370/new/ https://reviews.llvm.org/D121370 Files: clang/include/clang/Too

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk updated this revision to Diff 423256. kwk added a comment. - Use proposed reverse loop Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121370/new/ https://reviews.llvm.org/D121370 Files: clang/include/clang/Tooling/Inclusions/HeaderIncludes.h

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk updated this revision to Diff 423255. kwk added a comment. - Added llvm:: namespace Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121370/new/ https://reviews.llvm.org/D121370 Files: clang/include/clang/Tooling/Inclusions/HeaderIncludes.h c

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk updated this revision to Diff 423254. kwk added a comment. - Put shared code for finding include names HeaderIncludes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121370/new/ https://reviews.llvm.org/D121370 Files: clang/include/clang/Tooli

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk marked 2 inline comments as done. kwk added a comment. @HazardyKnusperkeks I've addressed your comments and did an early return together with `llvm_unreachable()` which is used in the `clang/lib/Format` in other places as well. I hope this is to your liking. There's no longer an `assert`. D

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk updated this revision to Diff 417379. kwk added a comment. - Address review comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121370/new/ https://reviews.llvm.org/D121370 Files: clang/lib/Format/Format.cpp clang/lib/Tooling/Inclusions/

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk added a comment. @MyDeveloperDay can you please have another look now that the patch has additional tests and most comments that still apply have been addressed? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121370/new/ https://reviews.llvm.or

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk updated this revision to Diff 417101. kwk added a comment. - Remove duplicate assert Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121370/new/ https://reviews.llvm.org/D121370 Files: clang/lib/Format/Format.cpp clang/lib/Tooling/Inclusions

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk updated this revision to Diff 417099. kwk added a comment. - Remove left-over comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121370/new/ https://reviews.llvm.org/D121370 Files: clang/lib/Format/Format.cpp clang/lib/Tooling/Inclusion

[PATCH] D120884: [format] Use int8_t as the underlying type of all enums in FormatStyle

This revision was automatically updated to reflect the committed changes. Closed by commit rG063bd3b886b2: [format] Use int8_t as the underlying type of all enums in FormatStyle (authored by kwk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120884/

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk marked 3 inline comments as done. kwk added a comment. @HazardyKnusperkeks thank you for the approval. Can you have one last view please? I've introduced a function to get the match instead of repeating the for loop two times. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk updated this revision to Diff 417097. kwk marked an inline comment as done. kwk added a comment. - introduce getIncludeNameFromMatches - Fix format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121370/new/ https://reviews.llvm.org/D121370 File

[PATCH] D120884: [format] Use int8_t as the underlying type of all enums in FormatStyle

kwk added a comment. @HazardyKnusperkeks @owenpan can you please have a quick look again. All I did was rebasing my changes and in the meantime I was interrupted... This patch has been sitting for some time and I'd like to land it as soon one of you approves it. Repository: rG LLVM Github M

[PATCH] D120884: [format] Use int8_t as the underlying type of all enums in FormatStyle

kwk updated this revision to Diff 417044. kwk added a comment. Fixup my work after interruption by screaming children wanting dinner ;) This is how I've tested this patch: ninja clang-format && ninja FormatTests && ./tools/clang/unittests/Format/FormatTests Repository: rG LLVM Github Mono

[PATCH] D120884: [format] Use int8_t as the underlying type of all enums in FormatStyle

kwk updated this revision to Diff 416971. kwk added a comment. Fixup Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120884/new/ https://reviews.llvm.org/D120884 Files: clang/include/clang/Format/Format.h Index: clang/include/clang/Format/Format.

[PATCH] D120884: [format] Use int8_t as the underlying type of all enums in FormatStyle

kwk updated this revision to Diff 416970. kwk added a comment. Rebased Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120884/new/ https://reviews.llvm.org/D120884 Files: clang/include/clang/Format/Format.h Index: clang/include/clang/Format/Forma

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk updated this revision to Diff 416878. kwk added a comment. - Fix test but unsure if correct Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121370/new/ https://reviews.llvm.org/D121370 Files: clang/lib/Format/Format.cpp clang/lib/Tooling/Inc

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk added a comment. In the current state, some tests in `SortIncludesTest.SupportAtImportLines` are failing. This is strange, because I've tested it with this `.clang-format` file: BasedOnStyle: LLVM IncludeBlocks: Regroup NOTE: for each test I've change the `Regroup` to `Preserve` resp

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk added a comment. In D121370#3387175 , @HazardyKnusperkeks wrote: > In D121370#3383574 , > @HazardyKnusperkeks wrote: > >> Some test with a main header and blocks would be nice. > > This one please. :) Done.

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk updated this revision to Diff 416877. kwk added a comment. - Fix: int -> bool Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121370/new/ https://reviews.llvm.org/D121370 Files: clang/lib/Format/Format.cpp clang/lib/Tooling/Inclusions/Header

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk updated this revision to Diff 416876. kwk marked an inline comment as done. kwk added a comment. - Address review comments on grouping and main headers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121370/new/ https://reviews.llvm.org/D121370

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk marked an inline comment as done. kwk added a comment. Addressed review comments. Comment at: clang/lib/Format/Format.cpp:2779 /*CheckMainHeader=*/!MainIncludeFound && FirstIncludeBlock); -int Priority = Categories.getSortIncludePriority( -I

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk updated this revision to Diff 415769. kwk added a comment. - Use std::numeric_limits::max() instead of INT_MAX Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121370/new/ https://reviews.llvm.org/D121370 Files: clang/lib/Format/Format.cpp cl

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk updated this revision to Diff 415489. kwk edited the summary of this revision. kwk added a comment. - Fix formatting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121370/new/ https://reviews.llvm.org/D121370 Files: clang/lib/Format/Format.cp

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk updated this revision to Diff 415487. kwk added a comment. Changed the strategy of how includes are sorted by increasing the priority if an include is of the "new" type: `@import Foundation;`. This will sort these includes after everything else just as requested in https://github.com/llvm/l

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk planned changes to this revision. kwk added a comment. A test if failing. Need to address this first. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121370/new/ https://reviews.llvm.org/D121370 ___ cf

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk added a comment. Thank you for your comments! I've addressed many of them and now address the rest. Comment at: clang/lib/Format/Format.cpp:2759 + IncludeName = + StringRef(Twine("<", IncludeName).concat(Twine(">")).str()); +}

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk updated this revision to Diff 414469. kwk marked 2 inline comments as done. kwk added a comment. - Make @ or # not optional again - Remove [\t\n\ \\]* - Properly concat string Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121370/new/ https://re

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

kwk created this revision. kwk added reviewers: HazardyKnusperkeks, MyDeveloperDay. Herald added a project: All. kwk requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fixes #38995 This is an attempt to modify the regular expression to identi

[PATCH] D120884: [format] Use int8_t as the underlying type of all enums in FormatStyle

kwk added a comment. In D120884#3356746 , @HazardyKnusperkeks wrote: > Please use the clang-format tag. Yes, sorry that I forgot it again. And thank you for approving. Shall I wait for the others to approve this or land the patch? Repository: rG LL

[PATCH] D120398: [format] follow up: Use unsigned char as the base of all enums in FormatStyle

kwk abandoned this revision. kwk added a comment. I abandon this revision in favor of https://reviews.llvm.org/D120884. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120398/new/ https://reviews.llvm.org/D120398

[PATCH] D120884: [format] Use int8_t as the underlying type of all enums in FormatStyle

kwk created this revision. kwk added reviewers: HazardyKnusperkeks, MyDeveloperDay, curdeius. Herald added a project: All. kwk requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. It was requested here (https://reviews.llvm.org/D120398#3353053)

[PATCH] D120398: [format] follow up: Use unsigned char as the base of all enums in FormatStyle

kwk added a comment. In D120398#3352436 , @HazardyKnusperkeks wrote: > > That being said, I don't care that much that we declare the enums with a > type. But I care about consistency. Either land this, or revert D93758 >

[PATCH] D120712: [clang-format][docs] handle explicit enum values

kwk added a comment. In D120712#3352051 , @MyDeveloperDay wrote: > Does it need to be so "cloak and dagger"? ;-) No, you're right it doesn't have to be this way. > We always welcome patches, but please think about logging the idea in github > issues

[PATCH] D120712: [clang-format][docs] handle explicit enum values

kwk added a comment. In D120712#3351889 , @MyDeveloperDay wrote: >> My reasoning was that I'm going to introduce an explicit enum value > > Any reason why? Yes, I but where's the fun if I told you now ;) . Just kidding. I don't want to go into the deta

[PATCH] D120712: [clang-format][docs] handle explicit enum values

kwk added inline comments. Comment at: clang/docs/tools/dump_format_style.py:174-175 return '* ``%s`` (in configuration: ``%s``)\n%s' % ( -self.name, -re.sub('.*_', '', self.config), +self.clean_name, +self.clean_config, doxygen2rst(i

[PATCH] D120712: [clang-format][docs] handle explicit enum values

kwk added a comment. In D120712#3351014 , @MyDeveloperDay wrote: > I'm not quite sure how I feel about adding functionality that doesn't > actually do anything. I hear you. My reasoning was that I'm going to introduce an explicit enum value sometime s

[PATCH] D120712: [clang-format][docs] handle explicit enum values

kwk created this revision. kwk added a reviewer: FederAndInk. kwk requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. If there was this enum definition before: struct FormatStyle { //... /// Different styles for aligning after ope

[PATCH] D120398: [format] follow up: Use unsigned char as the base of all enums in FormatStyle

kwk updated this revision to Diff 411105. kwk added a comment. Rebased on upstream/main Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120398/new/ https://reviews.llvm.org/D120398 Files: clang/include/clang/Format/Format.h Index: clang/include/

[PATCH] D120398: [format] follow up: Use unsigned char as the base of all enums in FormatStyle

kwk created this revision. kwk requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Analogous to https://reviews.llvm.org/D93758, this patch uses `unsigned char` for all enums in `FormatStyle`. Repository: rG LLVM Github Monorepo https://re

[PATCH] D113993: WIP: [clang] customizable build-id style

kwk updated this revision to Diff 387650. kwk added a comment. enabled DEFAULT_LINKER_BUILD_ID_STYLE in linux Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113993/new/ https://reviews.llvm.org/D113993 Files: clang/CMakeLists.txt clang/include/

[PATCH] D113993: WIP: [clang] customizable build-id style

kwk updated this revision to Diff 387610. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113993/new/ https://reviews.llvm.org/D113993 Files: clang/CMakeLists.txt clang/include/clang/Config/config.h.cmake clang/lib/Driver/ToolChains/Hurd.cpp I

[PATCH] D113993: WIP: [clang] customizable build-id style

kwk updated this revision to Diff 387608. kwk added a comment. Use correct CMake variable type: BOOLEAN -> BOOL Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113993/new/ https://reviews.llvm.org/D113993 Files: clang/CMakeLists.txt Index: clang

[PATCH] D113993: WIP: [clang] customizable build-id style

kwk updated this revision to Diff 387606. kwk edited the summary of this revision. kwk added a comment. Renamed: `ENABLE_LINKER_BUILD_ID_STYLE` to `DEFAULT_LINKER_BUILD_ID_STYLE`. Fixed issues with CMake (not done yet) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[PATCH] D113993: WIP: [clang] customizable build-id style

kwk created this revision. kwk added reviewers: tbaeder, tstellar. kwk added projects: clang, lld. Herald added a subscriber: mgorny. kwk requested review of this revision. Herald added a subscriber: cfe-commits. **This is work in progress. I'm evaluating the patch now.** This change introduces t

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

kwk added a comment. You've disabled sized deallocation in some tests by providing `-fno-sized-deallocation `. I admit I haven't looked at those tests in detail but I would like to understand if those tests would otherwise fail. If they fail, is there work that needs to be done later to make th

[PATCH] D106809: [clang-offload-bundler] Make Bundle Entry ID backward compatible

kwk added a comment. @saiislam please have a look into why this happens. Is the `-debug-only=CodeObjectCompatibility` maybe a left-over of some sort? Comment at: clang/test/Driver/clang-offload-bundler.c:405 +// Tests to check compatibility between Bundle Entry ID formats i.e.

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

This revision was automatically updated to reflect the committed changes. Closed by commit rGe636e6b79ac0: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro (authored by kwk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80531/new

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

kwk added a comment. In D80531#2069383 , @njames93 wrote: > LGTM with 2 small nits, but I'd still give a few days see if anyone else > wants to weight in. I'm okay with this but I'd like to understand when it's time to wait for others? Only when a patc

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

kwk added a comment. Done. Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.h:52 + + const std::string getMacroName() const { return MacroName; } + njames93 wrote: > Return by reference here. Yeah, that makes sense. St

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

kwk updated this revision to Diff 267960. kwk marked 4 inline comments as done. kwk added a comment. - Return macro name by reference - Add colon to paragraph showing code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80531/new/ https://reviews.llv

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

kwk marked 4 inline comments as done. kwk added a comment. Marked more comments as "done" after going through the code again. Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.cpp:49 +R"cpp({0}(const {0} &) = delete; +const {0} &

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

kwk updated this revision to Diff 267021. kwk marked 9 inline comments as done. kwk added a comment. - Don't store SourceManager but get it from Preprocessor - Use getName instead of getNameStart - Remove FIXME - change warning message - doxygen: \a -> \p - Make MacroName private and add getter

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

kwk added a comment. @njames93 I've addressed all your comments and hope the patch is good to land now :) Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.cpp:25 + const SourceManager &SM) + : Check(Check), PP(PP), SM(SM) {}

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

kwk added a comment. @njames93 and @Eugene.Zelenko do you guys think this patch is ready to be approved and land? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80531/new/ https://reviews.llvm.org/D80531 _

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

kwk updated this revision to Diff 266438. kwk marked an inline comment as done. kwk added a comment. - fix link in doc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80531/new/ https://reviews.llvm.org/D80531 Files: clang-tools-extra/clang-tidy/m

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

kwk added a comment. @Eugene.Zelenko thank you for your patience with me. I fixed the link. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80531/new/ https://reviews.llvm.org/D80531 ___ cfe-commits mail

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

kwk updated this revision to Diff 266268. kwk added a comment. - Adjust link to documentation Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80531/new/ https://reviews.llvm.org/D80531 Files: clang-tools-extra/clang-tidy/modernize/CMakeLists.txt

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

kwk marked an inline comment as done. kwk added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize-replace-disallow-copy-and-assign-macro.rst:41 + specification untouched. You might want to run the check + `modernize-use-equals-delete `_ to get +

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

kwk marked 2 inline comments as done. kwk added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize-replace-disallow-copy-and-assign-macro.rst:41 + specification untouched. You might want to run the check + `modernize-use-equals-delete `_ to get +

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

kwk updated this revision to Diff 266257. kwk marked 2 inline comments as done. kwk added a comment. - Refer to modernize-use-equals-delete for warning about deleted functions in private sections Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80531/

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

kwk added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize-replace-disallow-copy-and-assign-macro.rst:31 +* Notice that the migration example above leaves the ``private`` access + specification untouched. This opens room for improvement, yes I kno

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

kwk added a comment. Thanks @njames93 and @Eugene.Zelenko for your review. Most of it, I addressed but for some I have comments. See inline. Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.cpp:36 +// be the class name. +const

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

kwk updated this revision to Diff 266251. kwk marked 11 inline comments as done. kwk added a comment. - Avoid constructing string - CamelCase for vars - more readable snippets for documentation - Added empty line after header in documentation - Remove sentence from doc - Remove unimportant tests

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

kwk marked 2 inline comments as done. kwk added a comment. @njames93 I've implemented everything you've mentioned. This simplified a lot. Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize-replace-disallow-copy-and-assign-macro.rst:41 + +.. option:: FinalizeWithSem

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

kwk updated this revision to Diff 266148. kwk marked 4 inline comments as done. kwk added a comment. - Wrap RUN lines - Use CamelCase variable name - Simplify access to options - Make check options const - Use two-line placeholder - Automate placement of semicolon at the end Repository: rG LLV

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

kwk planned changes to this revision. kwk added a comment. Thank you @njames93 for the review. I'm implementing what you wrote. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80531/new/ https://reviews.llvm.org/D80531

  1   2   >