[PATCH] D37904: [clang-format] Fix FixNamespaceComments when BraceWrapping AfterNamespace is true.

2017-09-26 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. Any thoughts on this? https://reviews.llvm.org/D37904 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35743: [clang-format] Adjust space around &/&& of structured bindings

2017-09-26 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. Thanks for the changes. LGTM. https://reviews.llvm.org/D35743 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/c

[PATCH] D38357: [Support/Regex] Handle tabulators and escaped chars in square brackets.

2017-09-28 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius created this revision. This patch is motivated by bug #34739 (https://bugs.llvm.org/show_bug.cgi?id=34739). Clang-format misbehaves because escape sequence '\t' in regex is treated as literal 't'. This patch handles escaped characters inside square brackets as well (so that somebody ca

[PATCH] D53372: [clang-tidy] Resolve readability-else-after-return false positive for constexpr if.

2018-10-17 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius created this revision. curdeius added reviewers: alexfh, aaron.ballman. Herald added subscribers: cfe-commits, xazax.hun. It fixes the false positive when using constexpr if and where else cannot be removed: Example: if constexpr (sizeof(int) > 4) // ... return /* ... */; e

[PATCH] D53372: [clang-tidy] Resolve readability-else-after-return false positive for constexpr if.

2018-10-18 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. In https://reviews.llvm.org/D53372#1267728, @lebedev.ri wrote: > I think it would be good to add some more explanation as to *why* that `else` > has to be kept. Do you mean add a comment in the code or just an explanation for the review? For the latter, e.g.: // u

[PATCH] D53372: [clang-tidy] Resolve readability-else-after-return false positive for constexpr if.

2018-10-18 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. Actually, I can make it an option for this check to skip or not constexpr ifs, WDYT? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53372 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://list

[PATCH] D53372: [clang-tidy] Resolve readability-else-after-return false positive for constexpr if.

2018-10-18 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius updated this revision to Diff 170101. curdeius added a comment. Applied changes as per comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53372 Files: test/clang-tidy/readability-else-after-return-if-constexpr.cpp Index: test/clang-tidy/readability-else-after

[PATCH] D53372: [clang-tidy] Resolve readability-else-after-return false positive for constexpr if.

2018-10-19 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius updated this revision to Diff 170152. curdeius added a comment. Fixed diff. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53372 Files: clang-tidy/readability/ElseAfterReturnCheck.cpp test/clang-tidy/readability-else-after-return-if-constexpr.cpp Index: test/clan

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

2018-12-10 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius requested changes to this revision. curdeius added a comment. This revision now requires changes to proceed. Just a few minor remarks and a possible workaround for testing `CHECK-FIXES: [[nodiscard]]`. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:43 + // fu

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

2018-12-10 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. Some more minor remarks. I'll give this check a try to see how it behaves on some of my projects. I agree that a high rate of false positives is possible (and is a bit of spoiler) but I wouldn't reject this IMO useful check because of that. Anyway, everything looks pret

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

2018-12-10 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius requested changes to this revision. curdeius added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:45 + // + for (const auto *Par : Node.parameters()) { +const Type *ParType = Par->getType().get

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

2018-12-10 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM. But I'm not a code owner here and I don't know if you need an acceptance of one of them. Great job. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55433/new/ https://reviews

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

2018-12-11 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. A few more ideas for enhancements. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:41 + //bool foo(A*) + // then they may not care about the return value, because of passing data + // via the arguments however functions with no arguments

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

2018-12-11 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:65 +void UseNodiscardCheck::registerMatchers(MatchFinder *Finder) { + // If we are using C++17 attributes we are going to need c++17 + if (NoDiscardMacro == "[[nodiscard]]") { ---

[PATCH] D37132: [clang-format] Add support for C++17 structured bindings.

2017-08-29 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius updated this revision to Diff 113073. curdeius added a comment. Fix line endings. https://reviews.llvm.org/D37132 Files: lib/Format/FormatToken.h lib/Format/TokenAnnotator.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp

[PATCH] D37132: [clang-format] Add support for C++17 structured bindings.

2017-08-29 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius updated this revision to Diff 113077. curdeius added a comment. Fix line endings again. https://reviews.llvm.org/D37132 Files: lib/Format/FormatToken.h lib/Format/TokenAnnotator.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp ==

[PATCH] D37132: [clang-format] Add support for C++17 structured bindings.

2017-08-29 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius updated this revision to Diff 113079. curdeius added a comment. Extract method. https://reviews.llvm.org/D37132 Files: lib/Format/FormatToken.h lib/Format/TokenAnnotator.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp ==

[PATCH] D37132: [clang-format] Add support for C++17 structured bindings.

2017-08-29 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius updated this revision to Diff 113084. curdeius added a comment. Fix: use do-while loop. https://reviews.llvm.org/D37132 Files: lib/Format/FormatToken.h lib/Format/TokenAnnotator.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp ==

[PATCH] D31334: [clang-format] Add options for indenting preprocessor directives

2017-09-07 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius abandoned this revision. curdeius added a comment. Superseded by https://reviews.llvm.org/rL312125. https://reviews.llvm.org/D31334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c

[PATCH] D37904: [clang-format] Fix FixNamespaceComments when BraceWrapping AfterNamespace is true.

2017-09-15 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius created this revision. Herald added a subscriber: klimek. NamespaceEndCommentsFixer did not fix namespace comments when the brace opening the namespace was not on the same line as the "namespace" keyword. It occurs in Allman, GNU and Linux styles and whenever BraceWrapping.AfterNamespac

[PATCH] D37980: [clang-format] Better parsing of lambda captures with initializer expressions.

2017-09-18 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius created this revision. This fixes bug #19986 (https://bugs.llvm.org/show_bug.cgi?id=19986). The code was incorrectly formatted to (mind additional spaces inside brackets) when lambda capture contained an initializer expression. This patch does not handle all possible initializers, but th

[PATCH] D37904: [clang-format] Fix FixNamespaceComments when BraceWrapping AfterNamespace is true.

2017-09-18 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. In https://reviews.llvm.org/D37904#873860, @krasimir wrote: > Could you please move the test that adds a namespace end comment to > `unittests/Format/NamespaceEndCommentsFixerTest.cpp`? That's what I wanted to do initially, but the very same tests there passed surpri

[PATCH] D35743: [clang-format] Adjust space around &/&& of structured bindings

2017-09-19 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. There is one big missing thing here: `PointerAlignment`. Actually only `PAS_Left` is taken into account. There are 3 possible options: Left: `auto& [a, b] = f();` Middle: `auto & [a, b] = f();` Right: `auto &[a, b] = f();` https://reviews.llvm.org/D35743 ___

[PATCH] D37980: [clang-format] Better parsing of lambda captures with initializer expressions.

2017-09-19 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius updated this revision to Diff 115814. curdeius added a comment. Minor: use `FormatToken::isNot` instead of `!FormatToken::is`. https://reviews.llvm.org/D37980 Files: lib/Format/UnwrappedLineParser.cpp lib/Format/UnwrappedLineParser.h unittests/Format/FormatTest.cpp Index: unitte

[PATCH] D37980: [clang-format] Better parsing of lambda captures with initializer expressions.

2017-09-19 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius abandoned this revision. curdeius added a comment. Ok. Nice patch. You can close https://bugs.llvm.org/show_bug.cgi?id=19986 now. https://reviews.llvm.org/D37980 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/

[PATCH] D37904: [clang-format] Fix FixNamespaceComments when BraceWrapping AfterNamespace is true.

2017-09-19 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. I confirm what I observed before: when invoking tests in `unittests/Format/NamespaceEndCommentsFixerTest.cpp`, the `const AnnotatedLine *line` parameter in `getNamespaceToken` gets one big line that includes both `namespace` and `{` (something like `namespace\n{\n...`,

[PATCH] D13811: [clang-format] AllowShortFunctionsOnASingleLine: true/Empty didn't work with BreakBeforeBraces: Linux/Allman.

2017-09-20 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius abandoned this revision. curdeius added a comment. This was fixed by https://reviews.llvm.org/rL312904 and other commits. https://reviews.llvm.org/D13811 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/

[PATCH] D37904: [clang-format] Fix FixNamespaceComments when BraceWrapping AfterNamespace is true.

2017-09-20 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. That's precisely what I've written, but, as I'd said before, such tests pass already without any modification in `NamespaceEndCommentsFixer`. https://reviews.llvm.org/D37904 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D35743: [clang-format] Adjust space around &/&& of structured bindings

2017-09-21 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. Could you add just one more test for `PAS_Middle` please? A single line will be just enough. Otherwise, LGTM. https://reviews.llvm.org/D35743 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

2018-12-14 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:22 + +// Find a better way of detecting const-reference-template type +// parameters via using alias not detected by ``isTemplateTypeParamType()``. You can write `TODO: ` or `F

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

2018-12-18 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. LGTM. Any ideas for improvement, @JonasToth? I'd rather have it merged and improve on it later if there are ideas on how to do it better. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:32 + // Try to catch both std::function and boost::functi

[PATCH] D80309: [clang-format][docfix] Update predefined styles in docs

2020-05-20 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. Nice! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80309/new/ https://reviews.llvm.org/D80309 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D80079: [clang-format] [NFC] isCpp() is inconsistently used to mean both C++ and Objective C, add language specific isXXX() functions

2020-05-21 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. I like these changes. I have mixed feelings about `isCpp()` & co. As @MyDeveloperDay said, I'd like it mean C++ only. I find it confusing that it means C++ or ObjC (even if the latter is a superset of the former). I'd rather see it spelt as `isCppOrObjC()` even if it's

[PATCH] D80547: [clang-format] Fix an ObjC regression introduced with new [[likely]][[unlikely]] support in if/else clauses

2020-05-26 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. Minor remarks. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2346 +// look to see if we have [[ by looking ahead if +// its not then rewind to the original position Nit: shouldn't comments be "Full phrases."? ===

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

2020-05-27 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/docs/ReleaseNotes.rst:359 +- Option ``ConstPlacement`` has been added auto-arrange the positioning of const + in variable and parameter declarations to be ``Right/East`` const or ``Left/West`` It sounds stra

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

2020-05-27 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. One last thought, how about making the config a bit more future-proof and instead of `ConstPlacement` accept what was discussed some time ago: QualifierStyle: - const: Right and restrict it to `const` for the moment? Maybe renaming to `QualifierPlacement`

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

2020-05-28 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius marked an inline comment as done. curdeius added inline comments. Comment at: clang/lib/Format/EastWestConstFixer.cpp:195 +FormatToken *Tok) { + // We only need to think about streams that begin with const. + if (!Tok->is(tok::kw_const))

[PATCH] D80627: [clang-format] Create a python documentation tool to generate a summary of the clang-format status for the whole of the LLVM project

2020-05-28 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/docs/tools/generate_formatted_state.py:17 +with open(DOC_FILE, 'wb') as output: +output.write(".. raw:: html\n") +output.write("\n") It will still not work with Python 3, you need to pass `bytes` to `write

[PATCH] D80627: [clang-format] Create a python documentation tool to generate a summary of the clang-format status for the whole of the LLVM project

2020-05-28 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius requested changes to this revision. curdeius added a comment. This revision now requires changes to proceed. First of all, very nice idea. :) Second, could you please make sure that the script is compatible with Python 3? Also, in order to clean up a bit the generation of the RST (to avoi

[PATCH] D80627: [clang-format] Create a python documentation tool to generate a summary of the clang-format status for the whole of the LLVM project

2020-05-29 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. @MyDeveloperDay , I know it's a strange request, but could you change (or remove) the background color for 100% case. I'm partially color-blind and having red and green background in the same table is really hard to distinguish. I guess I'm not alone. I'd suggest using

[PATCH] D80627: [clang-format] Create a python documentation tool to generate a summary of the clang-format status for the whole of the LLVM project

2020-05-29 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. @MyDeveloperDay, I've played around with the script, you can take as much as you judge useful from here: https://github.com/mkurdej/llvm-project/tree/arcpatch-D80627. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80627/new/ https://reviews.llvm.org/D80627

[PATCH] D80627: [clang-format] Create a python documentation tool to generate a summary of the clang-format status for the whole of the LLVM project

2020-05-29 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. That looks nicer indeed. Thanks. Just some minor nitty-gritty comments. Comment at: clang/docs/tools/generate_formatted_state.py:52 +path = os.path.relpath(root, LLVM_DIR) +if "/test/" in path: +continue MyD

[PATCH] D80627: [clang-format] Create a python documentation tool to generate a summary of the clang-format status for the whole of the LLVM project

2020-05-29 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/docs/tools/generate_formatted_state.py:54 + +table_row = """ * - {path} + - {count} Another nit: I prefer writing `"""\` as it nicely aligns with subsequent lines. CHANGES SINCE LAST ACTION https://revie

[PATCH] D80627: [clang-format] Create a python documentation tool to generate a summary of the clang-format status for the whole of the LLVM project

2020-05-29 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM! Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80627/new/ https://reviews.llvm.org/D80627 ___ cfe-commits mailing list

[PATCH] D80830: [clang-format] [PR46130] When editing a file with unbalance {} the namespace comment fixer can incorrectly comment the wrong closing brace

2020-05-29 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM. It may be silly, but the fact that this code runs over all the tokens makes me think: do we have any performance-related non-regression tests for clang-format? Com

[PATCH] D80830: [clang-format] [PR46130] When editing a file with unbalance {} the namespace comment fixer can incorrectly comment the wrong closing brace

2020-05-30 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. OK, great. Thanks for all the information. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80830/new/ https://reviews.llvm.org/D80830 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D80950: [clang-format] [PR44542,38872] String << String always get a forced newline.

2020-06-02 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. The change seems to me technically sound, but I'm not sure of the scope of its effects. There might be users that rely on this behaviour. On the other hand, adding an option to keep the old behaviour doesn't seem appropriate, and personally I consider the old behaviour

[PATCH] D80933: [clang-format] [PR46157] Wrong spacing of negative literals with use of operator

2020-06-02 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:981 consumeToken(); +if (CurrentToken->is(tok::comma) && +CurrentToken->Previous->isNot(tok::kw_operator)) Shouldn't you check `CurrentToken` for not bein

[PATCH] D80950: [clang-format] [PR44542,38872] String << String always get a forced newline.

2020-06-03 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. I would have nothing against if you'd added this option and we kept current behaviour by default. The only drawback is the (bit of) complexity it adds bit that seems justified to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D80940: [clang-format] [PR46159] Linux kernel 'C' code uses 'try' as a variable name, allow clang-format to handle such cases

2020-06-03 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. Nice. Should we test other non-C keywords as well? Out of curiosity, where does "@try" come from? Comment at: clang/lib/Format/FormatTokenLexer.cpp:392 + auto &Try = *(Tokens.end() - 2); + auto &Brace = *(Tokens.end() - 1); + if (!Try->is(tok::kw_tr

[PATCH] D80933: [clang-format] [PR46157] Wrong spacing of negative literals with use of operator

2020-06-03 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM. Keep in mind I'm not the code owner, I don't know if another approval is required. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80933/new/ https://reviews.llvm.org/D80933

[PATCH] D84090: [clang-format] Add SpaceAroundBitFieldColon option

2020-07-18 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. The changes look good to me in general. I share your doubt though about whether a book flag is sufficient here. We've seen in the past a few times that at some time a false/true flag is not enough. I'd rather go for a Before/After/Both/None flag (or similar, naming pro

[PATCH] D83296: [clang-format] Add a MacroExpander.

2020-07-22 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/lib/Format/MacroExpander.cpp:46 + + // Parse the token stream and return the corresonding Defintion object. + // Returns an empty definition object with a null-Name on error. Nit: typo "corresponding Definition"

[PATCH] D80940: [clang-format] [PR46159] Linux kernel 'C' code uses 'try' as a variable name, allow clang-format to handle such cases

2020-06-03 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM. For a second, I thought that you could simplify the code by removing this @try condition (and calling the function `FormatTokenLexer::tryTransformTryUsageForC()` only if `isCppOnly`

[PATCH] D82016: [clang-format] [PR462254] fix indentation of default and break correctly in whitesmiths style

2020-06-17 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. Great! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82016/new/ https://reviews.llvm.org/D82016 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

2020-07-02 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius marked an inline comment as done. curdeius added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:13540 CHECK_PARSE_BOOL(AlignConsecutiveMacros); + CHECK_PARSE_BOOL(AlwaysBreakBeforeConceptDeclarations); CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLi

[PATCH] D83564: [clang-format] PR46609 clang-format does not obey `PointerAlignment: Right` for ellipsis in declarator for pack

2020-07-12 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM! Thanks for fixing this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83564/new/ https://reviews.llvm.org/D83564 __

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

2020-06-26 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/include/clang/Format/Format.h:533 + /// If ``true``, always break before concept declarations + bool AlwaysBreakBeforeConceptDeclarations; It would be nice to have an example here in the doc. ===

[PATCH] D82620: [clang-format] Preserve whitespace in selected macros

2020-06-26 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:2706 + + For example: STRINGIZE + Shouldn't there be a configuration example like what's in `ForEachMacros` doc? ``` In the .clang-format configuration file, this can be configu

[PATCH] D82620: [clang-format] Preserve whitespace in selected macros

2020-06-26 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. LGTM. Thank you for taking my comments into account. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82620/new/ https://reviews.llvm.org/D82620

[PATCH] D78982: [clang-format] Fix Microsoft style for enums

2020-04-30 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. In D78982#2013221 , @RKSimon wrote: > @asmith I'm seeing MSVC warnings from this patch: > > E:\llvm\llvm-project\clang\lib\Format\UnwrappedLineParser.cpp(1475): warning > C4305: 'argument': truncation from 'clang::tok::TokenKind'

[PATCH] D79293: [clang-format] [PR45218] Fix an issue where < and > and >> in a for loop gets incorrectly interpreted at a TemplateOpener/Closer

2020-05-03 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. > This fix tries to avoid that issue by determining that a ";" between the < > and > would not be a template (I couldn't think of an example where that > would be the case, but I'm sure there are.. For what is worth, with lambdas in unevaluated context (C++20), you can

[PATCH] D85600: [clang-format] use spaces for alignment of binary/ternary expressions with UT_AlignWithSpaces

2020-08-11 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:1362-1367 +if (Style.AlignOperands != FormatStyle::OAS_DontAlign && Previous && +(Previous->getPrecedence() == prec::Assignment || + Previous->is(tok::kw_return) || +

[PATCH] D85600: [clang-format] use spaces for alignment of binary/ternary expressions with UT_AlignWithSpaces

2020-08-11 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM. Thank you for the patch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85600/new/ https://reviews.llvm.org/D85600 ___

[PATCH] D85600: [clang-format] use spaces for alignment of binary/ternary expressions with UT_AlignWithSpaces

2020-08-11 Thread Marek Kurdej via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGb18c63e85aa8: [clang-format] use spaces for alignment of binary/ternary expressions with… (authored by fickert, committed by curdeius). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D79935: [clang-format] [PR44345] Long namespace closing comment is duplicated endlessly

2020-05-15 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/lib/Format/NamespaceEndCommentsFixer.cpp:129 + // } // namespace + // // verylongnamespacenamethatdidnotfitonthepreviouscommenline + if (!(Comment->Next && Comment->Next->is(TT_LineComment))) Nit: typo commen

[PATCH] D129926: [clang-format] Handle constructor invocations after new operator in C# correct

2022-09-05 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. @eoanermine, please provide your name and email address for the commit message. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129926/new/ https://reviews.llvm.org/D129926 ___ cf

[PATCH] D131978: [clang-format] Concepts: allow identifiers after negation

2022-09-05 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. @rymiel, please provide your name and email address for the commit message, so that it can land it for you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131978/new/ https://reviews.llvm.o

[PATCH] D126365: [git-clang-format] Stop ignoring changes for files with space in path

2022-09-05 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. @Eitot, please provide your name and email address for the commit message. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126365/new/ https://reviews.llvm.org/D126365 ___ cfe-com

[PATCH] D132189: [clang-format] Don't put `noexcept` on empty line following constructor

2022-09-05 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. @rymiel, please provide your name and email address for the commit message, so that we can land it for you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132189/new/ https://reviews.llvm.o

[PATCH] D132295: [clang-format] Change heuristic for locating lambda template arguments

2022-09-05 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. @rymiel, please provide your name and email address for the commit message, so that we can land it for you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132295/new/ https://reviews.llvm.o

[PATCH] D132762: [clang-format] Allow `throw` to be a keyword in front of casts

2022-09-05 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. @rymiel, please provide your name and email address for the commit message, so that we can land it for you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132762/new/ https://reviews.llvm.o

[PATCH] D133571: [clang-format] Introduce NoFallThrough option into AllowShortCaseLabelsOnASingleLine

2022-09-09 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:935 +**AllowShortCaseLabelsOnASingleLine** (``ShortCaseLabelStyle``) :versionbadge:`clang-format 3.6` + Determine if Short case labels will be contracted to a single line. + =

[PATCH] D133571: [clang-format] Introduce NoFallThrough option into AllowShortCaseLabelsOnASingleLine

2022-09-09 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM after fixing the last comment. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:650 + // Don't merge if the last thing on the line before was an attribut

[PATCH] D133589: [clang-format] JSON formatting add new option for controlling newlines in json arrays

2022-09-09 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:3115 +**JsonMultilineArrays** (``Boolean``) :versionbadge:`clang-format 16` + If ``true``, clang-format will always break after a Json array `[` Why limiting to JSON only? Could

[PATCH] D129057: [clang-format] Break on AfterColon only if not followed by comment

2022-07-03 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:7099 OnePerLine); - verifyFormat("SomeClass::Constructor() :\n" + verifyFormat("SomeClass::Constructor() : // NOLINT\n" "a(aa),

[PATCH] D129057: [clang-format] Break on AfterColon only if not followed by comment

2022-07-03 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:7136 Style); + verifyFormat("Constructor() : // NOLINT\n" + "() {}", How about very long comments? They don't get split now? Please add a

[PATCH] D129057: [clang-format] Break on AfterColon only if not followed by comment

2022-07-04 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM. Thanks for addressing my comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129057/new/ https://reviews.llvm.org/D129057 __

[PATCH] D129064: [clang-format] Avoid crash in LevelIndentTracker.

2022-07-04 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius created this revision. curdeius added reviewers: HazardyKnusperkeks, owenpan, MyDeveloperDay. Herald added a project: All. curdeius requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fixes https://github.com/llvm/llvm-project/issues/5

[PATCH] D129064: [clang-format] Avoid crash in LevelIndentTracker.

2022-07-06 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius updated this revision to Diff 442482. curdeius marked an inline comment as done. curdeius added a comment. Address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129064/new/ https://reviews.llvm.org/D129064 Files: clang/lib/For

[PATCH] D129064: [clang-format] Avoid crash in LevelIndentTracker.

2022-07-07 Thread Marek Kurdej via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG14c30c70c459: [clang-format] Avoid crash in LevelIndentTracker. (authored by curdeius). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129064/new/ https://re

[PATCH] D129348: [clang-format] Fix an assertion failure on -lines=0:n

2022-07-08 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. 👍 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129348/new/ https://reviews.llvm.org/D129348 __

[PATCH] D121584: [clang-format] Correctly recognize arrays in template parameter list.

2022-03-24 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. Yes, let's revert this. I should have a fix soon though. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121584/new/ https://reviews.llvm.org/D121584 ___ cfe-commits mailing list

[PATCH] D122468: [clang-format] Fix SeparateDefinitionBlocks breaking up function-try-block.

2022-03-25 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius created this revision. curdeius added reviewers: MyDeveloperDay, HazardyKnusperkeks, owenpan. Herald added a project: All. curdeius requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fixes https://github.com/llvm/llvm-project/issues/5

[PATCH] D106112: [clang-format] Break an unwrapped line at a K&R C parameter decl

2021-07-16 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. Looks okay, but I was wondering if we don't want to guard all K&R-related changes behind e.g. ```Standard: Cpp78``, so as not to possibly introduce strange bugs in newer modes. It may be an overkill if there are 2 patches like this, but if there are more, that might be

[PATCH] D106112: [clang-format] Break an unwrapped line at a K&R C parameter decl

2021-07-19 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. In D106112#2885604 , @owenpan wrote: > I just reviewed the differences between K&R C (circa 1978) and ANSI/ISO C > again and didn't see anything else that would impact clang-format, so a new > S

[PATCH] D106773: [clang-format] Fix aligning with linebreaks #2

2021-07-26 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM! That was quick! Thanks. Please wait for @baramin to validate the fix before landing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1067

[PATCH] D99840: [clang-format] Correctly attach enum braces with ShortEnums disabled

2021-07-28 Thread Marek Kurdej via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG71616722d409: [clang-format] Correctly attach enum braces with ShortEnums disabled (authored by lunasorcery, committed by curdeius). Changed prior to commit: https://reviews.llvm.org/D99840?vs=362220&id

[PATCH] D106773: [clang-format] Fix aligning with linebreaks #2

2021-07-29 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. This bug fix should probably be cherry-picked into 13.x branch. WDYT? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106773/new/ https://reviews.llvm.org/D106773 ___ cfe-commits

[PATCH] D106773: [clang-format] Fix aligning with linebreaks #2

2021-07-29 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a subscriber: tstellar. curdeius added a comment. In D106773#2912925 , @HazardyKnusperkeks wrote: > In D106773#2912732 , @curdeius > wrote: > >> This bug fix should probably be cherry-picked into

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

2022-03-09 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:192-211 + EXPECT_EQ("#define M(x) x##x\n" +"namespace A M(x) {\n" +"int i;\n" +"int j;\n" +"}// namespace A M(x)", +fi

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

2022-03-09 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/lib/Format/NamespaceEndCommentsFixer.cpp:72 -Tok = FirstNSTok; -while (Tok && !Tok->is(tok::l_brace)) { +bool IsPrevColoncolon = false; +bool HasColoncolon = false; Nit: Personally, I'd put these

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

2022-03-09 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:192 "}")); + EXPECT_EQ("#define M(x) x##x\n" +"namespace A M(x) {\n" curdeius wrote: > zequanwu wrote: > > zequanwu wrote: > > > cur

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

2022-03-10 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. LGTM % nits. Thanks for working on this! Comment at: clang/lib/Format/NamespaceEndCommentsFixer.cpp:54-55 +Tok = processTokens(Tok, tok::l_paren, tok::r_paren, nullptr); + } else if (Tok->is(tok::l_square)) +To

[PATCH] D119599: [clang-format] Add option to align compound assignments like `+=`

2022-03-10 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. LGTM. Thanks a lot! Comment at: clang/lib/Format/Format.cpp:1181-1184 + LLVMStyle.AlignConsecutiveAssignments = { + /*Enabled=*/false, /*AcrossEmptyLines=*/false, + /*AcrossComments=*/false, /*AlignCompound=*

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

2022-03-10 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/lib/Format/Format.cpp:2685 const char CppIncludeRegexPattern[] = -R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))"; +R"(^[\t\ ]*[@#]?[\t\ ]*(import|include)[^"<]*[\t\n\ \\]*("[^"]+"|<[^>]+>|[^"<>;]+;))"; ---

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

2022-03-10 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/lib/Format/Format.cpp:2685 const char CppIncludeRegexPattern[] = -R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))"; +R"(^[\t\ ]*[@#]?[\t\ ]*(import|include)[^"<]*[\t\n\ \\]*("[^"]+"|<[^>]+>|[^"<>;]+;))"; ---

[PATCH] D121450: [clang-format] Handle attributes before case label.

2022-03-11 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius created this revision. curdeius added reviewers: MyDeveloperDay, HazardyKnusperkeks, owenpan. Herald added a project: All. curdeius requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fixes https://github.com/llvm/llvm-project/issues/5

[PATCH] D121451: [clang-format] Add space to comments starting with '#'.

2022-03-11 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius created this revision. curdeius added reviewers: MyDeveloperDay, HazardyKnusperkeks, owenpan. Herald added a project: All. curdeius requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fixes https://github.com/llvm/llvm-project/issues/3

[PATCH] D121451: [clang-format] Add space to comments starting with '#'.

2022-03-11 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. I'm not sure whether we should consider this a breaking change, there were no tests at all verifying this behaviour. I think that not adding a space in comments starting with a punctuation is a mistake unless for some special comments like `///`, `//!` etc. (which are

  1   2   3   4   5   6   7   8   9   >