[PATCH] D56563: [clang-tidy] add options documentation to readability-identifier-naming checker

MyDeveloperDay updated this revision to Diff 181224. MyDeveloperDay added a comment. Address review comments - clang-format the code examples - replace "ensure" with "check" CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56563/new/ https://reviews.llvm.org/D56563 Files: docs/clang-ti

[PATCH] D56563: [clang-tidy] add options documentation to readability-identifier-naming checker

MyDeveloperDay marked 5 inline comments as done. MyDeveloperDay added a comment. > I think will be good idea to create generic Case/Prefix/Suffix description to > reduce size of documentation. I'm generating this documentation via a script, using the following template, (which is why its quite

[PATCH] D56563: [clang-tidy] add options documentation to readability-identifier-naming checker

MyDeveloperDay updated this revision to Diff 181497. MyDeveloperDay added a comment. Fix review comments s/the checker will check/the check will ensure/ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56563/new/ https://reviews.llvm.org/D56563 Files: docs/clang-tidy/checks/readability

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

MyDeveloperDay added a comment. In D56424#1356959 , @karepker wrote: > Hi all, ping on this patch. I've addressed all comments to the best of my > ability. Is there anything outstanding that needs to be changed? Round about this time of a review we norm

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

MyDeveloperDay added a comment. In D55433#1351707 , @JonasToth wrote: > > I do not have commit rights. I'm not sure what constitutes someone who can > > commit, but let me contribute a little more before taking that step, I > > have another idea for a c

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

MyDeveloperDay added a comment. In D56424#1357481 , @lebedev.ri wrote: > In D56424#1357471 , @MyDeveloperDay > wrote: > > > In D56424#1356959 , @karepker > > wrote: > > > >

[PATCH] D56563: [clang-tidy] add options documentation to readability-identifier-naming checker

MyDeveloperDay added a comment. In D56563#1358149 , @JonasToth wrote: > > Given its length, what do you think about a short link-list at the beginning > that will point to the section in the docs? With that its easier to see whats > all handled by th

[PATCH] D56563: [clang-tidy] add options documentation to readability-identifier-naming checker

MyDeveloperDay updated this revision to Diff 181836. MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added a comment. Addressing review comment CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56563/new/ https://reviews.llvm.org/D56563 Files: docs/clang-tidy/checks/reada

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

MyDeveloperDay added a comment. In D56424#1359227 , @karepker wrote: > In D56424#1357484 , @MyDeveloperDay > wrote: > > > In D56424#1357481 , @lebedev.ri > > wrote: > > > >

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

MyDeveloperDay added a comment. > LLVM is very chatty as well, I don't consider LLVM to be a bad code-base. > Take readability-braces-around-statements for example. Do we need a llvm-elide-braces-for-small-statements? This would make a great pre-review check Repository: rCTE Clang Tools Ext

[PATCH] D56926: [Documentation] Use HTTPS whenever possible in clang-tools-extra

MyDeveloperDay added inline comments. Comment at: llvm-svn.src/tools/clang/tools/extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-goto.rst:11 from the CppCoreGuidelines and -`6.3.1 from High Integrity C++

[PATCH] D56926: [Documentation] Use HTTPS whenever possible in clang-tools-extra

MyDeveloperDay added a comment. the closest I can see is https://www.perforce.com/resources/qac/high-integrity-cpp-coding-standard#statements which take you to section 6 of the standard, but I see no id or name tags to get you to 6.3.1 Repository: rCTE Clang Tools Extra CHANGES SINCE LAST

[PATCH] D56926: [Documentation] Use HTTPS whenever possible in clang-tools-extra

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. LGTM Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56926/new/ https://reviews.llvm.org/D56926 ___ cfe-commits mailing list cfe-comm

[PATCH] D57087: [clang-tidy] add OverrideMacro to modernize-use-override check

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: alexfh, JonasToth, hokein, Eugene.Zelenko, aaron.ballman. MyDeveloperDay added a project: clang-tools-extra. Herald added subscribers: cfe-commits, xazax.hun. The usefulness of **modernize-use-override** can be reduced if you h

[PATCH] D57087: [clang-tidy] add OverrideMacro to modernize-use-override check

MyDeveloperDay updated this revision to Diff 183063. MyDeveloperDay added a comment. Adding release note change CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57087/new/ https://reviews.llvm.org/D57087 Files: clang-tidy/modernize/UseOverrideCheck.cpp clang-tidy/modernize/UseOverride

[PATCH] D57087: [clang-tidy] add OverrideMacro to modernize-use-override check

MyDeveloperDay added a comment. In D57087#1367610 , @alexfh wrote: > I tend to think that a better migration strategy is to change the compiler to > a C++11-compatible one first, and then turn on C++11 mode and migrate the > code (possibly file-by-file o

[PATCH] D57087: [clang-tidy] add OverrideMacro to modernize-use-override check

MyDeveloperDay added inline comments. Comment at: clang-tidy/modernize/UseOverrideCheck.cpp:32 +: ClangTidyCheck(Name, Context), + OverrideMacro(Options.get("OverrideMacro", "override")), + FinalMacro(Options.get("FinalMacro", "final")) {} alexfh wr

[PATCH] D57087: [clang-tidy] add OverrideMacro to modernize-use-override check

MyDeveloperDay updated this revision to Diff 183268. MyDeveloperDay marked 12 inline comments as done. MyDeveloperDay added a comment. Addressing review comments, - reduce changes causing excessive string concats and problems with temporaries - simplify cxx version and macro checks CHANGES SINC

[PATCH] D57380: [clang-tools-extra] add missing .clang-format and .clang-tidy for use with git monorepo

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: JonasToth, alexfh, hokein, aaron.ballman. MyDeveloperDay added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Before git monorepo, clang-tools-extra inherited its .clang-format and .clang-tidy files from

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

MyDeveloperDay added a comment. I checked this out and built this and it works really well. The thing I really like is that it helps to format Windows resource include files e.g. 'Resource.h' better than before. Which changes the current clang-formatted version #define IDD_

[PATCH] D57380: [clang-tools-extra] add missing .clang-format and .clang-tidy for use with git monorepo

MyDeveloperDay abandoned this revision. MyDeveloperDay added a comment. Abandoning as its been superseded by... https://github.com/llvm/llvm-project/commit/149be18dbc4d54328fe33b5ac21dcbbca2b07aa6 Which puts these files at the base of llvm-project Repository: rCTE Clang Tools Extra CHANGES

[PATCH] D57353: [clang-tidy] Add the abseil-duration-double-conversion check

MyDeveloperDay added inline comments. Comment at: docs/clang-tidy/checks/abseil-duration-double-conversion.rst:20 + + + // Original - Conversion to integer and back again hwright wrote: > Eugene.Zelenko wrote: > > Unnecessary empty line. > This is consistent wit

[PATCH] D57435: [clang-tidy] Add abseil-wrap-unique check

MyDeveloperDay added inline comments. Comment at: clang-tidy/abseil/WrapUniqueCheck.cpp:27 + + return (ArgRef.str().length() > 0) ? ArgRef.str() + ")" : "()"; +} couldn't this be ``` return (!ArgRef.empty()) ? ArgRef.str() + ")" : "()"; ``` You know I think t

[PATCH] D57435: [clang-tidy] Add abseil-wrap-unique check

MyDeveloperDay added inline comments. Comment at: clang-tidy/abseil/WrapUniqueCheck.cpp:54 + const auto *facExpr = Result.Nodes.getNodeAs("facCons"); + const auto *callExpr = Result.Nodes.getNodeAs("callExpr"); + I'm not sure how expensive getNodeAs is... or if

[PATCH] D57435: [clang-tidy] Add abseil-wrap-unique check

MyDeveloperDay added inline comments. Comment at: clang-tidy/abseil/WrapUniqueCheck.h:27 +private: + std::string getArgs(const SourceManager *SM, const CallExpr *MemExpr); + Nit: could this method could be static in the cpp file and not in the header? Reposito

[PATCH] D57353: [clang-tidy] Add the abseil-duration-double-conversion check

MyDeveloperDay added inline comments. Comment at: docs/clang-tidy/checks/abseil-duration-double-conversion.rst:20 + + + // Original - Conversion to integer and back again hwright wrote: > MyDeveloperDay wrote: > > hwright wrote: > > > Eugene.Zelenko wrote: > > >

[PATCH] D57108: [clang-tidy] diagnose possibiltiy to add 'noexcept' in modernize-use-noexcept

MyDeveloperDay added inline comments. Comment at: clang-tidy/modernize/UseNoexceptCheck.h:1 //===--- UseNoexceptCheck.h - clang-tidy-*- C++ -*-===// // Nit: sorry OCD kicking in... space after tidy and before first - Repository: rCT

[PATCH] D57435: [clang-tidy] Add abseil-wrap-unique check

MyDeveloperDay added a comment. This is the clang-tidy revision that mgiht have caught the str().length() > 0 case D56644: [clang-tidy] readability-container-size-empty handle std::string length() adding a a cross reference for that work. Repository: rCTE

[PATCH] D56644: [clang-tidy] readability-container-size-empty handle std::string length()

MyDeveloperDay added inline comments. Comment at: test/clang-tidy/readability-container-size-empty.cpp:135 + // CHECK-FIXES: {{^ }}if (str.empty()){{$}} + if (str.length() == 0) +; could you add a test that checks if StringRef.str().length() >0 becomes !

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

MyDeveloperDay added a comment. In D55433#1323106 , @lebedev.ri wrote: > Have you evaluated this on some major C++ projects yet? > I suspect this may have pretty low SNR. Internally yes (on millions of lines), but you are correct high noise level, howe

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

MyDeveloperDay added a comment. I'm trying to generate an example of the modernize-use-nodiscard checker on something open source...as I developed this checker on Windows I struggled a little getting cmake to build me the json file to run clang-tidy over everything and a lot of projects aren't

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

MyDeveloperDay updated this revision to Diff 177295. MyDeveloperDay added a comment. Address some (not all yet) review comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55433/new/ https://reviews.llvm.org/D55433 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/modernize/M

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

MyDeveloperDay marked 12 inline comments as done. MyDeveloperDay added inline comments. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:28-30 +bool isOperator(const FunctionDecl *D) { + return D->getNameAsString().find("operator") != std::string::npos; +}

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

MyDeveloperDay updated this revision to Diff 177311. MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added a comment. Move more of the conditional checks into the matchers CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55433/new/ https://reviews.llvm.org/D55433 Files:

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

MyDeveloperDay updated this revision to Diff 177312. MyDeveloperDay added a comment. Move the conditional checks into matchers CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55433/new/ https://reviews.llvm.org/D55433 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/modernize/Mo

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

MyDeveloperDay updated this revision to Diff 177313. MyDeveloperDay added a comment. Move the conditional statements into AST_MATCHERS CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55433/new/ https://reviews.llvm.org/D55433 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/mode

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

MyDeveloperDay updated this revision to Diff 177327. MyDeveloperDay added a comment. - Move the final conditional statements into AST_MATCHERS - Add additional IgnoreInternalFunctions option to allow adding of ``[[nodiscard]]`` to functions starting with _ (e.g. _Foo()) CHANGES SINCE LAST ACTIO

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

MyDeveloperDay updated this revision to Diff 177330. MyDeveloperDay added a comment. Minor alterations to Release notes and Documentation to address review comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55433/new/ https://reviews.llvm.org/D55433 Files: clang-tidy/modernize/CM

[PATCH] D55482: [clang-tidy] Improve google-objc-function-naming diagnostics 📙

MyDeveloperDay added inline comments. Comment at: test/clang-tidy/google-objc-function-naming.m:8 +// must be in Pascal case as required by Google Objective-C style guide // CHECK-FIXES: static bool Ispositive(int a) { return a > 0; } I realize there are words

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

MyDeveloperDay updated this revision to Diff 177474. MyDeveloperDay marked 11 inline comments as done. MyDeveloperDay added a comment. - Addressing review comments and concerns - Removed internal function logic and option, not really the role of this checker - Fixed grammatical error in documenta

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

MyDeveloperDay added inline comments. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:43 + // function like _Foo() + if (ignore){ + return doesFunctionNameStartWithUnderScore(&Node); curdeius wrote: > If think that you should run clang-format over yo

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

MyDeveloperDay marked an inline comment as done. MyDeveloperDay added inline comments. Comment at: docs/clang-tidy/checks/modernize-use-nodiscard.rst:62 +Users can use :option:`IgnoreInternalFunctions` to turn off the adding of +``[nodiscard]]`` to functions starting with _ e.g.

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

MyDeveloperDay added a comment. In D55433#1325117 , @lebedev.ri wrote: > In D55433#1323779 , @lebedev.ri > wrote: > > > In D55433#1323757 , > > @MyDeveloperDay wrote: > > >

[PATCH] D55508: [clang-tidy] insert release notes for new checkers alphabetically

MyDeveloperDay added a comment. In D55508#1325316 , @Eugene.Zelenko wrote: > Thank you for this fix! Your welcome, I was reviewing other revisions to try and get up to speed, and I saw you giving someone else the same comment you gave mine! I'm no pyt

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

MyDeveloperDay updated this revision to Diff 177496. MyDeveloperDay added a comment. Update the documentation to utilize 80 character lines CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55433/new/ https://reviews.llvm.org/D55433 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy

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

MyDeveloperDay updated this revision to Diff 177532. MyDeveloperDay marked an inline comment as done. MyDeveloperDay added a comment. Addressing review comments, - additional unit tests for no ReplacementString and C++ 11 case - more expressive hasNonConstReferenceOrPointerArguments matcher - min

[PATCH] D55508: [clang-tidy] insert release notes for new checkers alphabetically

MyDeveloperDay updated this revision to Diff 177538. MyDeveloperDay added a comment. Fix review comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55508/new/ https://reviews.llvm.org/D55508 Files: clang-tidy/add_new_check.py Index: clang-tidy/add_new_check.py ==

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

MyDeveloperDay added a comment. @curdeius Thanks, I don't have commit access so I'm happy wait for a CODE_OWNER, they could likely have more input. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55433/new/ https://reviews.llvm.org/D55433 __

[PATCH] D55508: [clang-tidy] insert release notes for new checkers alphabetically

MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added a comment. In D55508#1325670 , @JonasToth wrote: > I think this patch is fine, AFAIK these utility scripts are not tested > directly but are just adjusted if they dont work as expected

[PATCH] D55508: [clang-tidy] insert release notes for new checkers alphabetically

MyDeveloperDay added a comment. In D55508#1325758 , @JonasToth wrote: > LGTM. Do you have commit access? I do not I'm afraid CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55508/new/ https://reviews.llvm.org/D55508 __

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

MyDeveloperDay updated this revision to Diff 177584. MyDeveloperDay marked 11 inline comments as done. MyDeveloperDay added a comment. Addressing review comments - grammatical errors in documentation and comments - prevent adding [[nodiscard]] to macros - clean up list.rst CHANGES SINCE LAST AC

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

MyDeveloperDay updated this revision to Diff 177679. MyDeveloperDay marked 10 inline comments as done. MyDeveloperDay added a comment. Resolving some (not all yet) review comments, and looking for help on template parameter exclusion - add additional template argument tests - add additional clan

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

MyDeveloperDay added a comment. I wanted to leave a comment regarding running the [[nodiscard]] checker over all of clang... as requested by @JonasToth, @lebedev.ri and @Szelethus I'm not 100% sure how to present the results, but let me pick out a few high/low lights... My efforts are somewha

[PATCH] D58404: [clang-format] Add basic support for formatting C# files

MyDeveloperDay updated this revision to Diff 188239. MyDeveloperDay added a subscriber: llvm-commits. MyDeveloperDay added a comment. Fix a crash running clang-format over large C# code base Add support for C# Null Coalescing CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58404/new/ http

[PATCH] D58609: [clang-tidy] bugprone-string-integer-assignment: Reduce false positives.

MyDeveloperDay added inline comments. Comment at: clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp:79 +return; + } + elide {} on small if statements Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58609/new/

[PATCH] D56943: [clang-format][NFC] Allow getLLVMStyle() to take a language

MyDeveloperDay added inline comments. Herald added a subscriber: jdoerfert. Comment at: clang/lib/Format/ContinuationIndenter.cpp:193 RawStringFormat.Language, &PredefinedStyle)) { -PredefinedStyle = getLLVMStyle(); +PredefinedStyle

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

MyDeveloperDay added a comment. Please resubmit this patch with full context git diff --cached -U99 > patch_to_submitt.diff You need to add some documention describing the new option into the check in docs/clang-tidy/checks/modernize .rst Repository: rCTE Clang Tools Extra CHANGE

[PATCH] D56943: [clang-format][NFC] Allow getLLVMStyle() to take a language

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. I think I've seen you need this for another patch, so in the absence of the code owners this LGTM, and thank you for removing all the changes to the tests in the previous diff.

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

MyDeveloperDay added a comment. In D28462#1407239 , @micah-s wrote: > ClamAV recently started using clang-format. We published this blog post > about how we're using it: > https://blog.clamav.net/2019/02/clamav-adopts-clang-format.html One of the > th

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

MyDeveloperDay added a comment. In D58731#1412930 , @lewmpk wrote: > cleaned up documentation Are you planning on landing this anytime soon given that it was accepted? I would like to land D57087: [clang-tidy] add OverrideMacro to modernize-use-overrid

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

MyDeveloperDay added a comment. In D58731#1413476 , @lewmpk wrote: > I'm happy to land this ASAP but I don't have commit rights So one of us could land it for you.. (I've not personally done that before as I'm a bit green too! but I do have commit right

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

MyDeveloperDay added a comment. In D58731#1413695 , @alexfh wrote: > In D58731#1413552 , @JonasToth wrote: > > > In D58731#1413498 , > > @MyDeveloperDay wrote: > > > > > In

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

MyDeveloperDay added a comment. While we wait for code review I've landed this currently unaccepted clang-format revision into https://github.com/mydeveloperday/clang-experimental/releases for those wishing to try/test or provide feedback CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

MyDeveloperDay added a comment. @JonasToth I left a comment in the commit needed to fix the index.rst, which I don't think your later review fixes, sphinx complained about the rst file being an unreferenced octtree https://reviews.llvm.org/rG5fbeff797a9dba504f08f14c4fa59b6f1076fe72#inline-2691

[PATCH] D57087: [clang-tidy] add OverrideMacro to modernize-use-override check

MyDeveloperDay updated this revision to Diff 188752. MyDeveloperDay added a comment. rebase after D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions change as this was previous accepted I will land shortly, but I'd appreciate a quick overv

[PATCH] D55964: [clang-format][TableGen] Don't add spaces around items in square braces.

MyDeveloperDay added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55964/new/ https://reviews.llvm.org/D55964 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

MyDeveloperDay added a comment. Hi @reuk, I'm trying to go over old reviews and see if they are still desired, I stumbled on your review and want to see if its still important. Here is some initial feedback. I know its ironic that the clang-format code isn't clang-formatted itself, but it mig

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

MyDeveloperDay added a comment. sorry to be critical, just trying to help get traction on your review... Comment at: lib/Format/TokenAnnotator.cpp:65 -const FormatToken &Previous = *CurrentToken->Previous; // The '<'. +const FormatToken &Previous = *CurrentToken->Pre

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

MyDeveloperDay added a comment. > ClamAV recently started using clang-format. We published this blog post > about how we're using it: > https://blog.clamav.net/2019/02/clamav-adopts-clang-format.html One of the > things I called out in the blog post is that we really want this feature, and >

[PATCH] D58819: [clang-format] clang-format off/on not respected when using C Style comments

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: djasper, klimek, JonasToth, krasimir. MyDeveloperDay added projects: clang, clang-tools-extra. If the clang-format on/off is in a /* comment */ then the sorting of headers is not ignored PR40901 - https://bugs.llvm.org/show_bu

[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check

MyDeveloperDay added inline comments. Comment at: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp:47 +"cppcoreguidelines-use-raii-locks"); CheckFactories.registerCheck( "cppcoreguidelines-avoid-c-arrays"); Nit: by and large this

[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check

MyDeveloperDay added a comment. This looks good to me but I would wait for one of @JonasToth or @alexfh to perhaps take a look, maybe you should add some nested scope example too using the same name, I know the compiler should warn about a shadow variable anyway but std::mutex m; m.lock();

[PATCH] D58819: [clang-format] clang-format off/on not respected when using C Style comments

MyDeveloperDay marked an inline comment as done. MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/SortIncludesTest.cpp:132 + "#include \n" + "/* clang-format off */\n" + "#include \n" alexfh

[PATCH] D58819: [clang-format] clang-format off/on not respected when using C Style comments

MyDeveloperDay updated this revision to Diff 188931. MyDeveloperDay added a comment. Fix negative test case support the same /*clang-format off*/ in the sort includes that the TokenLexer supports. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58819/new/ https://reviews.llvm.org/D58819

[PATCH] D58819: [clang-format] clang-format off/on not respected when using C Style comments

MyDeveloperDay marked an inline comment as done. MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:1792 +else if (Trimmed == "// clang-format on" || + Trimmed == "/* clang-format on */") FormattingOff = false; Jo

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. Thank you for helping to address one of the many clang-format issues from bugzilla, I'm not the code owner here but this looks good to me. If we want to address the many issues we need to be able to move forwards. CHANGES S

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

MyDeveloperDay added a comment. It might be a good idea to add the nested lambda tests from D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style) into your review to show how this patch c

[PATCH] D58922: [clang-format] broken after lambda with return type template with boolean literal

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: klimek, djasper, JonasToth, alexfh, krasimir. MyDeveloperDay added a project: clang-tools-extra. A Lamdba with a return type template with a boolean literal (true,false) behaves differently to an integer literal https://bugs.l

[PATCH] D58922: [clang-format] broken after lambda with return type template with boolean literal

MyDeveloperDay added a comment. In D58922#1417605 , @jkorous wrote: > BTW I suggest you also add the original test case since the test cases you > added fail the expectation at FormatTest.cpp:74 and the message is a bit > unclear whether the testcase or

[PATCH] D58934: [clang-format] Fix lambdas returning template specialization that contains operator in parameter

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. I'm not sure I personally would ever write code like that ;-) , but if its legal C++ and it compiles we should handle it the same as foo<1>,foo,foo As there are a number of re

[PATCH] D59087: [clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: klimek, djasper, JonasToth, alexfh, krasimir. MyDeveloperDay added a project: clang-tools-extra. Herald added a subscriber: jdoerfert. Addressing: PR25010 - https://bugs.llvm.org/show_bug.cgi?id=25010 Code like: if(true) var

[PATCH] D59103: [clang-tidy] New checker bugprone-incomplete-comparison-operator

MyDeveloperDay added a comment. I definately been burnt by not handling all the cases, but imagine a string class class mystring { const char *ptr; int size; int numallocated; } to compare the strings I wouldn't want to compare all the fields? I think this could lead to a lot of false

[PATCH] D59087: [clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

MyDeveloperDay updated this revision to Diff 189837. MyDeveloperDay added a comment. Add missing Format.h from the review CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59087/new/ https://reviews.llvm.org/D59087 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/clang/Forma

[PATCH] D59087: [clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

MyDeveloperDay updated this revision to Diff 189854. MyDeveloperDay added a comment. Fix spelling typo in documentation and comment CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59087/new/ https://reviews.llvm.org/D59087 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/c

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. I'm not the code owner but this LGTM, Assuming the unit tests remain passing, I'd like to see more items like this in clang-format addressed, at present we seem to lack getting

[PATCH] D33029: [clang-format] add option for dangling parenthesis

MyDeveloperDay added a comment. This review lack unit tests, please add some for FormatTest to show its use cases, otherwise someone else will come along and break this later CHANGES SINCE LAST ACTION https://reviews.llvm.org/D33029/new/ https://reviews.llvm.org/D33029 ___

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

MyDeveloperDay added a comment. In D55170#1423798 , @reuk wrote: > Thanks for the review, and for approving this PR. It's very much appreciated! > > I don't have merge rights - could someone commit this for me please? I'd actively encourage you to go get

[PATCH] D33029: [clang-format] add option for dangling parenthesis

MyDeveloperDay added a comment. Adding the unit tests lets us see how this option will work in various cases, it will let us understand that its not breaking anything else. I personally don't like to see revisions like this sit for 2 years with nothing happening, I don't see anything wrong with

[PATCH] D59087: [clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

MyDeveloperDay planned changes to this revision. MyDeveloperDay added a comment. In D59087#1423942 , @reuk wrote: > Does it make sense to allow `AllowShortIfElseStatementsOnASingleLine` to be > enabled when `AllowShortIfStatementsOnASingleLine` is not? >

[PATCH] D33029: [clang-format] add option for dangling parenthesis

MyDeveloperDay added a comment. In D33029#1423947 , @lebedev.ri wrote: > In D33029#1423944 , @MyDeveloperDay > wrote: > > > Adding the unit tests lets us see how this option will work in various > > cases, it will

[PATCH] D59087: [clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

MyDeveloperDay updated this revision to Diff 190049. MyDeveloperDay added a comment. Address review comments - collapse two options into one CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59087/new/ https://reviews.llvm.org/D59087 Files: clang/docs/ClangFormatStyleOptions.rst clang

[PATCH] D59087: [clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

MyDeveloperDay updated this revision to Diff 190053. MyDeveloperDay marked 8 inline comments as done. MyDeveloperDay added a comment. Address review comment - add document and comment spelling and punctuation - add additional unit tests to show non compound else clause case CHANGES SINCE LAST A

[PATCH] D59210: clang-format: distinguish ObjC call subexpressions after r355434

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59210/new/ https://reviews.llvm.org/D59210 __

[PATCH] D59210: clang-format: distinguish ObjC call subexpressions after r355434

MyDeveloperDay added a comment. most of the cases that we were adding here were for a templated return types e.g. verifyFormat("[]() -> foo<5 + 2> { return {}; };"); verifyFormat("[]() -> foo<5 - 2> { return {}; };"); So we should probably have handle that "-> (return type)", where the retu

[PATCH] D59087: [clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

MyDeveloperDay updated this revision to Diff 190351. MyDeveloperDay added a comment. run git clang-format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59087/new/ https://reviews.llvm.org/D59087 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/clang/Format/Format.h cla

[PATCH] D59292: [clang-format] messes up indentation when using JavaScript private fields and methods

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: djasper, klimek, JonasToth, reuk, krasimir. MyDeveloperDay added a project: clang-tools-extra. Addresses PR40999 https://bugs.llvm.org/show_bug.cgi?id=40999 Private fields and methods in javasceipt would get incorrectly indent

[PATCH] D59309: [clang-format] BreakAfterReturnType ignored on functions with numeric template parameters

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: djasper, klimek, JonasToth, krasimir, reuk. MyDeveloperDay added a project: clang-tools-extra. Addresses PR40696 - https://bugs.llvm.org/show_bug.cgi?id=40696 The BreakAfterReturnType didn't work if it had a single arguments wh

[PATCH] D59332: [clang-format] AlignConsecutiveDeclarations fails with attributes

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: rolandschulz, reuk, djasper, klimek. MyDeveloperDay added a project: clang-tools-extra. Addresses http://llvm.org/PR40418 When using `AlignConsecutiveDeclarations: true` variables with Attributes would not be aligned correctly

[PATCH] D57435: [clang-tidy] Add abseil-wrap-unique check

MyDeveloperDay added inline comments. Comment at: clang-tidy/abseil/WrapUniqueCheck.cpp:7 +// License. See LICENSE.TXT for details. +// +//===--===// License as above Comment a

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

MyDeveloperDay added inline comments. Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:163 + // Conservatively disable for list initializations + if (UseLegacyFunction && New->getInitializationStyle() == CXXNewExpr::ListInit) { +return; elide the bra

  1   2   3   4   5   6   7   8   9   10   >