[PATCH] D30567: [clang-tidy] Fix treating non-space whitespaces in checks list.

curdeius updated this revision to Diff 92630. curdeius added a comment. Trim only spaces. https://reviews.llvm.org/D30567 Files: clang-tidy/ClangTidyDiagnosticConsumer.cpp unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp Index:

[PATCH] D30567: [clang-tidy] Fix treating non-space whitespaces in checks list.

curdeius added a comment. Hi Alex, is it OK now? https://reviews.llvm.org/D30567 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30567: [clang-tidy] Fix treating non-space whitespaces in checks list.

curdeius updated this revision to Diff 92772. curdeius added a comment. Trim spaces only everywhere. Fix test. https://reviews.llvm.org/D30567 Files: clang-tidy/ClangTidyDiagnosticConsumer.cpp unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp Index:

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

curdeius added a comment. Thanks Daniel. I'll give it a try using the proposed approach and update you soon(ish). https://reviews.llvm.org/D31334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D30567: [clang-tidy] Fix treating non-space whitespaces in checks list.

curdeius added a comment. In https://reviews.llvm.org/D30567#708797, @alexfh wrote: > Do you have commit rights? Yes, I will commit this ASAP. https://reviews.llvm.org/D30567 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D30567: [clang-tidy] Fix treating non-space whitespaces in checks list.

curdeius added a comment. Hi Alex and sorry for the late reply. The main use case is a more readable `.clang-tidy` configuration checks. Before this correction one can use something like this: --- Checks: ' ,*, ,-cert-dcl03-c, ' ... It works, but is hardly comprehensible to

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

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

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

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

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

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

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

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.

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:

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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] D38357: [Support/Regex] Handle tabulators and escaped chars in square brackets.

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

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

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.

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.

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.

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] D53372: [clang-tidy] Resolve readability-else-after-return false positive for constexpr if.

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:

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

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

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

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.: //

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

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 /* ... */;

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

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:

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

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/

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

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

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

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] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

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

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

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

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

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 + //

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

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

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

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 =

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

curdeius added a comment. Minor comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:1354 + static FormatToken *untilMatchingParen(FormatToken *Current) { +// for when MatchingParen is not yet established +int ParenLevel = 0; Please write

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM. Just a small typo in comment. Otherwise, great job! Comment at: clang/lib/Format/TokenAnnotator.cpp:1369 + static bool isDeductionGuide(FormatToken ) { +//

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

curdeius added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:4995 + verifyFormat("c()->f();"); + verifyFormat("x()->foo<1>;"); + verifyFormat("x = p->foo<3>();"); MyDeveloperDay wrote: > curdeius wrote: > > What about: > > > > ``` > >

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

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

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

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

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

curdeius added inline comments. Comment at: clang/lib/Format/NamespaceEndCommentsFixer.cpp:129 + // } // namespace + // // verylongnamespacenamethatdidnotfitonthepreviouscommenline + if (!(Comment->Next && Comment->Next->is(TT_LineComment))) Nit: typo

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

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

[PATCH] D88296: [clang-format] De-duplicate includes with leading or trailing whitespace.

curdeius created this revision. curdeius added a reviewer: MyDeveloperDay. Herald added a project: clang. Herald added a subscriber: cfe-commits. curdeius requested review of this revision. This fixes PR46555. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D88296 Files:

[PATCH] D88296: [clang-format] De-duplicate includes with leading or trailing whitespace.

curdeius updated this revision to Diff 294291. curdeius added a comment. - Ooops. Revert unwanted test changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88296/new/ https://reviews.llvm.org/D88296 Files: clang/lib/Format/Format.cpp

[PATCH] D88296: [clang-format] De-duplicate includes with leading or trailing whitespace.

curdeius added a comment. Finally I've opted for creating a small revision just fixing this particular bug report. Mind however that the problem is a bit more complex and to solve all possible cases we would need to first reformat the source code, then sort the includes and then again,

[PATCH] D52676: [clang-format] tweaked another case of lambda formatting

curdeius added subscribers: MyDeveloperDay, curdeius. curdeius added a comment. Hi, I know it's an old revision, but I confirm that it provoked the bug https://bugs.llvm.org/show_bug.cgi?id=45141. The problem is in the `TokenAnnotator::mustBreakBefore` as @jaafar pointed out: if

[PATCH] D52676: [clang-format] tweaked another case of lambda formatting

curdeius added a comment. I agree with both of you. I shouldn't have used the word "regression" indeed. I just meant a change in behaviour. Sorry for that. I'll try to play around and propose a patch to enhance this feature :). If you have any pointers about how to check if everything fits on a

[PATCH] D88296: [clang-format] De-duplicate includes with leading or trailing whitespace.

curdeius added a comment. Thanks for the review. I agree with you that a clang-format user should not expect a perfectly formatted code after a single iteration. Maybe it's actually a documentation issue and we should be clear about it? Just a guarantee of 1) stability of formatting and 2)

[PATCH] D87291: [clang-format][regression][PR47461] ifdef causes catch to be seen as a function

curdeius added a comment. LGTM. Don't we risk any other side effects? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87291/new/ https://reviews.llvm.org/D87291 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

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

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

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

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] D80627: [clang-format] Create a python documentation tool to generate a summary of the clang-format status for the whole of the LLVM project

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

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

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

[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

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

[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

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

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

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

[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

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

[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

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

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?

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

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

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

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] D85600: [clang-format] use spaces for alignment of binary/ternary expressions with UT_AlignWithSpaces

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

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

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

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

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);

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

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] D82016: [clang-format] [PR462254] fix indentation of default and break correctly in whitesmiths style

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

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

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

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

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

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] D80950: [clang-format] [PR44542,38872] String << String always get a forced newline.

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

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

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 = *(Tokens.end() - 2); + auto = *(Tokens.end() - 1); + if (!Try->is(tok::kw_try))

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

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

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

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] D80950: [clang-format] [PR44542,38872] String << String always get a forced newline.

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

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

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

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

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

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

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

[PATCH] D93806: [clang-format] PR48569 clang-format fails to align case label with `switch` with Whitesmith Indentation

curdeius added a comment. In D93806#2471738 , @MyDeveloperDay wrote: > This is the closest I could find thus far > http://bitsavers.informatik.uni-stuttgart.de/pdf/chromatics/CGC_7900_C_Programmers_Manual_Mar82.pdf > > and the very few examples from

[PATCH] D94206: [clang-format] turn on formatting after "clang-format on" while sorting includes

curdeius accepted this revision. curdeius added a comment. LGTM from my perspective. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94206/new/ https://reviews.llvm.org/D94206 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

curdeius added a comment. LGTM if you add a test for hdrstop(filename) and possibly with LF newline (as the test you've already added tests CRLF). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94217/new/ https://reviews.llvm.org/D94217 ___

[PATCH] D43159: [libc++] Replace several uses of 0 by nullptr

curdeius accepted this revision. curdeius added a comment. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43159/new/ https://reviews.llvm.org/D43159 ___ cfe-commits mailing list

[PATCH] D43159: [libc++] Replace several uses of 0 by nullptr

curdeius added inline comments. Comment at: libcxx/include/functional:1759 typedef __base<_Rp(_ArgTypes...)> __func; __func* __f_; All uses of `__f_` should also use `nullptr`. If my search counted correctly, there are 15 of them in `__value_func`.

[PATCH] D43159: [libc++] Replace several uses of 0 by nullptr

curdeius added inline comments. Comment at: libcxx/include/memory:4371 bool expired() const _NOEXCEPT -{return __cntrl_ == 0 || __cntrl_->use_count() == 0;} +{return __cntrl_ == nullptr || __cntrl_->use_count() == nullptr;} shared_ptr<_Tp> lock() const

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

curdeius added a comment. It would be maybe possible to: - address @klimek 's comment and do any other necessary cleanup - create a bug ticket for what @JohelEGP found with ctors - mark clang-format's support of concept as experimental? This patch is getting big IMO and I really think that

[PATCH] D92753: [clang-format] Add IndentPragma style to eliminate common clang-format off scenario

curdeius added a comment. This LGTM in general. I just have a doubt. You've added `messUp` parameter to `verifyFormat`, because, IIUC, pragmas wouldn't be at the desired scope level indentation otherwise. Shouldn't clang-format find out what the correct indentation level for pragmas should be

[PATCH] D92753: [clang-format] Add IndentPragma style to eliminate common clang-format off scenario

curdeius added a comment. Maybe it would be judicious to add a FIXME to `test::messUp` for the pragmas? Or fix it in this patch? Comment at: clang/unittests/Format/FormatTest.cpp:17641-17651 + verifyFormat("void foo() {\n" + " #pragma omp simd\n" +

[PATCH] D92257: [clang-format] Add option to control the space at the front of a line comment

curdeius accepted this revision. curdeius added a comment. LGTM Comment at: clang/lib/Format/Format.cpp:963 LLVMStyle.SpacesInCStyleCastParentheses = false; + LLVMStyle.SpacesInLineComments = {/*Minimum=*/1, /*Maximum=*/-1u}; LLVMStyle.SpaceAfterCStyleCast = false;

[PATCH] D92257: [clang-format] Add option to control the space at the front of a line comment

curdeius added inline comments. Comment at: clang/lib/Format/Format.cpp:963 LLVMStyle.SpacesInCStyleCastParentheses = false; + LLVMStyle.SpacesInLineComments = {/*Minimum=*/1, /*Maximum=*/-1u}; LLVMStyle.SpaceAfterCStyleCast = false; I don't know

[PATCH] D92753: [clang-format] Add IndentPragma style to eliminate common clang-format off scenario

curdeius added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:17676-17684 +format("void foo() {\n" + "#if 1\n" + " #pragma omp simd\n" + " for (int k = 0; k < 10; k++) {\n" +

[PATCH] D90232: [clang-format] Formatting constructor initializer lists by putting them always on different lines (update to D14484)

curdeius requested changes to this revision. curdeius added a comment. This revision now requires changes to proceed. I agree with @MyDeveloperDay that the existing tests should not be altered. Comment at: clang/docs/ClangFormatStyleOptions.rst:1473

[PATCH] D92822: [clang-format] [NFC] Fix spelling and grammatical errors in IncludeBlocks text

curdeius accepted this revision. curdeius added a comment. In D92822#2439069 , @HazardyKnusperkeks wrote: > I have only one question, what stands NFC for? I figured something like "no > functional change"? But I can't find a definition anywhere.

[PATCH] D92753: [clang-format] Add IndentPragma style to eliminate common clang-format off scenario

curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM with a single comment on the test. Comment at: clang/unittests/Format/FormatTest.cpp:17676-17684 +format("void foo() {\n" + "#if 1\n"

[PATCH] D92922: [clang-format] PR42434 Remove preprocessor and pragma lines from ObjectiveC guess

curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. Great! LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92922/new/ https://reviews.llvm.org/D92922 ___ cfe-commits mailing list

[PATCH] D92922: [clang-format] PR42434 Remove preprocessor and pragma lines from ObjectiveC guess

curdeius added inline comments. Comment at: clang/lib/Format/Format.cpp:2037 +if (Line->First->TokenText.startswith("#") || +Line->First->TokenText == "__pragma") { + continue; Maybe you should also handle MS-specific `_Pragma`?

[PATCH] D92547: [c++2b] Add option -std=c++2b to enable support for potential C++2b features.

curdeius created this revision. curdeius added reviewers: rsmith, aaron.ballman, hubert.reinterpretcast, faisalv. Herald added subscribers: cfe-commits, dexonsmith. Herald added a project: clang. curdeius requested review of this revision. Repository: rG LLVM Github Monorepo

  1   2   3   4   5   6   7   8   9   >