[PATCH] D56661: [clang-tidy] Fix incorrect array name generation in cppcoreguidelines-pro-bounds-constant-array-index

JonasToth added a comment. Sorry for the long delay! I have one question about a slighty different code constellation. std::optional> my_array; for(int i = 0; i < 42; ++i) (*my_array)[i] = 42; Will this be fixed properly? for(int i = 0; i < 42; ++i) gsl::at(*my_array, i) =

[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

JonasToth updated this revision to Diff 142521. JonasToth added a comment. - [Feature] implement array and method access - [Feature] finalize value semantic Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cp

[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

JonasToth updated this revision to Diff 142526. JonasToth added a comment. - [Feature] start working on handle semantic Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/ConstCheck.cpp clan

[PATCH] D41648: [clang-tidy] implement cppcoreguidelines macro rules

JonasToth added a comment. In https://reviews.llvm.org/D41648#1047799, @JonasToth wrote: > I checked several codebases and implemented a little helper script to see > which kind of macros exist. Then I added some filter regex as configuration > and tried again, here are the results: > > For htt

[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

JonasToth updated this revision to Diff 142544. JonasToth added a comment. - [Misc] unify handle and value modification detection - [Misc] found new caveats, maybe take a look at refactoring first an require this check to produce clean compiles on llvm Repository: rCTE Clang Tools Extra http

[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

JonasToth added a comment. @aaron.ballman @lebedev.ri The check is getting really complex. I run it over LLVM and detected some warnings, where iam not sure if they are valid or not. Its already a somewhat usable state, but its hard to determine false positives and false negatives. For me, tha

[PATCH] D45679: [clang-tidy] Add a new check, readability-unmodified-non-const-variable, that finds declarations of non-const variables that never get modified.

JonasToth added a comment. Hi @shuaiwang, i am currently working on the same check and we should definilty coordinate our work. :) What do you think about joining the forces and to work together on such a check? Things i would like to mention: - i differentiate between values and handles. I wa

[PATCH] D45679: [clang-tidy] Add a new check, readability-unmodified-non-const-variable, that finds declarations of non-const variables that never get modified.

JonasToth added inline comments. Comment at: clang-tidy/utils/ASTUtils.cpp:85 + // LHS of any assignment operators. + const auto AsAssignmentLhs = binaryOperator( + anyOf(hasOperatorName("="), hasOperatorName("+="), hasOperatorName("-="), there is a `util:

[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

JonasToth added a comment. > Coming from https://reviews.llvm.org/D45679, which I should have sent out > much earlier to get in front of you :p > > Kidding aside this check is more mature than mine and I'm happy to help > incorporate mine into this one. I would love to, yours is more elegant :

[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

JonasToth added a comment. > You'll also need to check: > > - a member function calling another non-const member function -> *this can't > be const > - *this is passed as non-const reference param to a function -> *this can't > be const Yes. That is similar behaviour to checking if a function

[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

JonasToth added a comment. In https://reviews.llvm.org/D45444#1069488, @shuaiwang wrote: > I've updated https://reviews.llvm.org/D45679 and I think the `isModified()` > function there is now sufficiently covering the cases you've covered here and > can be used as a good starting version if you

[PATCH] D45679: [clang-tidy] Add a helper function isModified, that checks whether an expression is modified within a statement.

JonasToth added a comment. Short note here too: I think having `isModified` return a track record, like a vector for each 1. modifications 2. non-const handle taking 3. (const usages) would be nice. Every user can decide how to react to it. Then the function would be more like `usageRecord` wi

[PATCH] D45697: [clang-tidy] Fix clang-tidy doesn't read .clangtidy configuration file.

JonasToth added inline comments. Comment at: test/clang-tidy/read_file_config.cpp:1 +// RUN: mkdir -p %T/read-file-config/ +// RUN: cp %s %T/read-file-config/test.cpp Will this work on windows? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D456

[PATCH] D45679: [clang-tidy] Add a helper function isModified, that checks whether an expression is modified within a statement.

JonasToth added a comment. *hust* /llvm/tools/clang/lib/Analysis/PseudoConstantAnalysis.cpp I will check this one first, before we get crazy and implemented something twice. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45679 ___

[PATCH] D45679: [clang-tidy] Add a helper function isModified, that checks whether an expression is modified within a statement.

JonasToth added a comment. You are doing a great job and i learn new stuff :) Inspired by the analysis tool in clangs repo: - What do you think about having these functions in a class? Now, we need to recalculate and reanalyze the scope for every variable, multiple times (reference tracking).

[PATCH] D41648: [clang-tidy] implement cppcoreguidelines macro rules

JonasToth added a comment. >> OpenCV isn't clean either, here the results: >> >> Filter: >> `ASSERT*|ALL*|CAL*|CC*|CALC*|calc*|CL*|cl*|CUDA*|CV*|cv*|EXPECT*|GTEST*|FUNCTOR*|HAVE*|ICV*|IPL*|IPP*|ipp*|__itt*|ITT*|JAS*|jas*|MESSAGE*|MAX*|OCL*|opengl*|OPENCV*|TYP*` > > This one worries me a bit mor

[PATCH] D45679: [clang-tidy] Add a helper function isModified, that checks whether an expression is modified within a statement.

JonasToth added a comment. I was thinking about that too. - We can extract all `DeclRefExpr` for the modifiying expressions and add dependencies. Such analysis is definitly interesting, but should maybe be added later. Having an internal `Expr` : `Modified?` mapping would suffice, too. The `isE

[PATCH] D45679: [clang-tidy] Add a helper function isModified, that checks whether an expression is modified within a statement.

JonasToth added inline comments. Comment at: clang-tidy/utils/ASTUtils.h:31 + +enum ExprModificationKind { + EMK_NotModified, /// The Expr is neither modified nor escaped. Maybe you could add an `Unknown` kind, e.g. if the expression is not found or similar.

[PATCH] D45679: [clang-tidy] Add a helper function isModified, that checks whether an expression is modified within a statement.

JonasToth added a comment. Another note: https://github.com/llvm-mirror/clang-tools-extra/blob/master/clang-tidy/utils/DeclRefExprUtils.cpp There is a `isOnlyUsedAsConst` with a slick implementation, using a set difference. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D4567

[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

JonasToth updated this revision to Diff 142988. JonasToth added a comment. - [Feature] refactor check to use a utility function to determine constness Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreg

[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

JonasToth added a comment. @shuaiwang I implemented the check on top of you utility function. It does fail right now (concentrating on values only for now). I added a `FIXME` for all false positives/negatives for the values test cases. Repository: rCTE Clang Tools Extra https://reviews.llvm

[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

JonasToth updated this revision to Diff 142991. JonasToth added a comment. - [Misc] mark false positives Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/ConstCheck.cpp clang-tidy/cppcoreg

[PATCH] D45679: [clang-tidy] Add a helper function isModified, that checks whether an expression is modified within a statement.

JonasToth added inline comments. Comment at: clang-tidy/utils/ASTUtils.cpp:226 + + // If 'Exp' is bound to a non-const reference, check all declRefExpr to that. + const auto Refs = match( What about transitive references and pointers? It seems that only referen

[PATCH] D45679: [clang-tidy] Add a helper function isModified, that checks whether an expression is modified within a statement.

JonasToth added a comment. I like your refactoring very much and i think we have a state that is close to commit. My clang-tidy check is already based on top of your functionality, but i can not work on it this weekend and next week is already busy for me. I decided to analysis values and refe

[PATCH] D45679: [clang-tidy] Add a helper function isModified, that checks whether an expression is modified within a statement.

JonasToth added a comment. Something came to my mind: conversion operators. Do they behave as normal overloaded operators? The tests should reflect both a const conversion and a modifying one. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45679

[PATCH] D45679: [clang-tidy] Add a helper function isModified, that checks whether an expression is modified within a statement.

JonasToth added a comment. There is a false positive with the reference checking. Could you please take a look at it? I could not find the reason from inspecting your code. void false_positive() { // FIXME False positive int np_local0 = 42; // CHECK-MESSAGES: [[@LINE-1]]:3

[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

JonasToth updated this revision to Diff 144458. JonasToth added a comment. - [Feature] use new statefull const checker - [Test] add more tests finding false positives Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt cla

[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

JonasToth added a comment. I will migrate to the new API today evening (european time). Why do you think that looping is required? From my understanding, we need the first usage (DeclRefExpr) to get the analysis started. The analysis itself will check all remaining uses. Is this necessary, becaus

[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

JonasToth updated this revision to Diff 144616. JonasToth added a comment. - migrate to Decl interface - add template metaprogramming test Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/Co

[PATCH] D45679: [clang-tidy] Add a helper function isModified, that checks whether an expression is modified within a statement.

JonasToth added a comment. I migrated to the new `Decl` interface and adjusted my tests, there are no false positives left and I am not aware of more possible code constellation that would require testing. If you have time you could check my tests, maybe you find something I missed. Otherwise t

[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

JonasToth updated this revision to Diff 144617. JonasToth added a comment. - fix bad template instantiation Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/ConstCheck.cpp clang-tidy/cppco

[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

JonasToth added a comment. I see. Thank you for fixing + explaining :) Am 30.04.2018 um 23:39 schrieb Shuai Wang via Phabricator: > shuaiwang added a comment. > >> Why do you think that looping is required? From my understanding, we >> >> need the first usage (DeclRefExpr) to get the analysi

[PATCH] D45679: [clang-tidy] Add ExprMutationAnalyzer, that analyzes whether an expression is mutated within a statement.

JonasToth added inline comments. Comment at: clang-tidy/utils/ExprMutationAnalyzer.cpp:88 + +const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) { + // LHS of any assignment operators. shuaiwang wrote: > aaron.ballman wrote: > > Should this als

[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

JonasToth updated this revision to Diff 153267. JonasToth added a comment. - rebase on commited ExpMutationAnalyzer - clean up, documentation Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

JonasToth updated this revision to Diff 153268. JonasToth marked 4 inline comments as done. JonasToth added a comment. - [Misc] order in release notes Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreg

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

JonasToth added a comment. - fixed some comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly

JonasToth updated this revision to Diff 153280. JonasToth added a comment. - remove bad code snippet which was dead Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48714 Files: clang-tidy/hicpp/ExceptionBaseclassCheck.cpp test/clang-tidy/hicpp-exception-baseclass.cpp Index:

[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly

JonasToth created this revision. JonasToth added reviewers: aaron.ballman, alexfh, hokein, ilya-biryukov. Herald added subscribers: cfe-commits, xazax.hun. JonasToth updated this revision to Diff 153280. JonasToth added a comment. - remove bad code snippet which was dead PR37913 documents wrong

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

JonasToth updated this revision to Diff 153282. JonasToth added a comment. - Merge branch 'master' into check_const Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/ConstCheck.cpp clang-ti

[PATCH] D48717: [clang-tidy] fix PR36489 - respect deduced pointer types from auto as well

JonasToth created this revision. JonasToth added reviewers: alexfh, aaron.ballman, hokein, ilya-biryukov. Herald added subscribers: cfe-commits, kbarton, xazax.hun, nemanjai. The cppcoreguidelines-pro-bounds-pointer-arithmetic warns on all occassion where pointer arithmetic is used, but does not c

[PATCH] D48717: [clang-tidy] fix PR36489 - respect deduced pointer types from auto as well

JonasToth marked 3 inline comments as done. JonasToth added inline comments. Comment at: clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cpp:24 + const auto AllPointerTypes = anyOf( + hasType(pointerType()), hasType(autoType(hasDeducedType(pointerType();

[PATCH] D48717: [clang-tidy] fix PR36489 - respect deduced pointer types from auto as well

JonasToth updated this revision to Diff 153341. JonasToth added a comment. - address review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48717 Files: clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cpp test/clang-tidy/cppcoreguidelines-pro-bounds-poi

[PATCH] D48717: [clang-tidy] fix PR36489 - respect deduced pointer types from auto as well

JonasToth marked an inline comment as not done. JonasToth added inline comments. Comment at: test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic-pr36489.cpp:4 + +// Fix PR36489 and detect auto-deduced value correctly. +char *getPtr(); aaron.ballman w

[PATCH] D48717: [clang-tidy] fix PR36489 - respect deduced pointer types from auto as well

JonasToth updated this revision to Diff 153457. JonasToth added a comment. This revision is now accepted and ready to land. - fix decltype deduction with new matcher I had to introduce a new matcher in clang for `DecltypeType` to reach its underlying type. It would be better to have `hasType` res

[PATCH] D48759: [ASTMatchers] add matcher for decltypeType and its underlyingType

JonasToth created this revision. JonasToth added reviewers: aaron.ballman, alexfh, NoQ, dcoughlin. Herald added a subscriber: cfe-commits. This patch introduces a new matcher for `DecltypeType` and its underlying type in order to fix a bug in clang-tidy, see https://reviews.llvm.org/D48717 for mo

[PATCH] D48854: Use ExprMutationAnalyzer in performance-for-range-copy

JonasToth added inline comments. Comment at: test/clang-tidy/performance-for-range-copy.cpp:120 +struct Point { + ~Point() {} I feel that `const` methods should be added as a test as well. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48854

[PATCH] D49114: Add a clang-tidy check for "magic numbers"

JonasToth added a comment. Related to this check: https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es45-avoid-magic-constants-use-symbolic-constants Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 _

[PATCH] D49114: Add a clang-tidy check for "magic numbers"

JonasToth added a comment. I think some of the logic you have in your check code could be done via matchers. That is usually better for performance, because you analyze less code. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:44 +for (const auto &Parent : Parent

[PATCH] D49158: [clang-tidy] Fixing segfault when there's no IdentifierInfo

JonasToth added inline comments. Comment at: clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:44 bool &isInterface) const { - StringRef Name = Node->getIdentifier()->getName(); - llvm::StringMapConstIterator Pa

[PATCH] D49158: [clang-tidy] Fixing segfault when there's no IdentifierInfo

JonasToth added a comment. Is there a way to add a test, that would trigger the old segfault and show that it does not happen anymore with this fix? https://reviews.llvm.org/D49158 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://list

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

JonasToth added a comment. Time :/ Am 14.07.2018 um 16:34 schrieb Roman Lebedev via Phabricator: > lebedev.ri added a comment. > > Ping, i guess :) > Anything needed to get this going? > > Repository: > > rCTE Clang Tools Extra > > https://reviews.llvm.org/D45444 Repository: rCTE Clan

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

JonasToth added a comment. Yes. https://github.com/JonasToth/clang-tools-extra/tree/check_const This is the branch i work on. I got it up to date with the current master for CTE. :) Am 14.07.2018 um 20:59 schrieb Florin Iucha via Phabricator: > 0x8000- added a comment. > > In https://revi

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

JonasToth added a comment. @0x8000- are you interested in working on the check? If i recall correctly the branch for 7.0 will happen on 1. august. That would be the timewindow in which it makes sense to give you the patch. I will have time in september following, but not right now. I try to

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

JonasToth marked an inline comment as done. JonasToth added a comment. Removed one unresolved comment, probably thats why it was stuck :D Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 ___ cfe-commits mailing list cfe-commits@

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

JonasToth updated this revision to Diff 155821. JonasToth added a comment. - Merge branch 'master' into check_const - [Misc] remove comment and iostream Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcor

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

JonasToth added a comment. What do you mean by "The code is a bit more intimidating", the check itself or the amount of tests? In general the utility piece that was commited before this check should already analyze the constness good, given that this check builds upon it the functionality should

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

JonasToth added a comment. In https://reviews.llvm.org/D49114#1164788, @0x8000- wrote: > @aaron.ballman , @JonasToth , @Eugene.Zelenko Is there anything missing from > this patch? What do I need to do to get it merged? This is my first > contribution to LLVM so I'm not quite sure. Thank you

[PATCH] D68694: [clang-tidy] hicpp-signed-bitwise: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands

JonasToth added a comment. In D68694#1701772 , @aaron.ballman wrote: > > In my opinion this check should be disabled in case of integer literals, > > since there are a lot of existing code (even in system libraries) where > > user can do nothing, e.g.:

[PATCH] D68694: [clang-tidy] hicpp-signed-bitwise: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands

JonasToth added a comment. The new switch needs documentation as well, and maybe even a note in the release notes (as it is publicly discussed as issue?). Otherwise I am fine with the changes! Comment at: clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise-integer-literals

[PATCH] D68694: [clang-tidy] hicpp-signed-bitwise: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands

JonasToth added a comment. > Do you know who is responsible for it? Because i haven't worked with > documentation before and don't know what i need to do to update it. The files reside in `clang-tools-extra/docs/ReleaseNotes.rst` and `clang-tools-extra/docs/clang-tidy/checks/hicpp-signed-bitwis

[PATCH] D68694: [clang-tidy] hicpp-signed-bitwise: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands

JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. LGTM. But i was inactive for a long time, if @aaron.ballman accepts as well you can commit instantly. Otherwise please let the other people that commented some time to react. :) Reposi

[PATCH] D81923: [clang-tidy] Add modernize-use-ranges check.

JonasToth added a comment. great to see such a check! Comment at: clang-tools-extra/clang-tidy/modernize/UseRangesCheck.cpp:77 + MatchCallTo((ID + "Begin").str(), namedDecl().bind(Range), + hasAnyName("::std::begin", "::std::cbegin"

[PATCH] D40463: [analyzer] Fix false negative on post-increment of uninitialized variable.

JonasToth added inline comments. Comment at: docs/ReleaseNotes.rst:261 +- Static Analyzer can now properly detect and diagnose unary pre-/post- + increment/decrement on an uninitialized values. + The end of the sentence should be either 'on uninitialized values'

[PATCH] D40463: [analyzer] Fix false negative on post-increment of uninitialized variable.

JonasToth added inline comments. Comment at: docs/ReleaseNotes.rst:261 +- Static Analyzer can now properly detect and diagnose unary pre-/post- + increment/decrement on an uninitialized values. + lebedev.ri wrote: > JonasToth wrote: > > The end of the sentence s

[PATCH] D40671: [clang-tidy] Support specific categories for NOLINT directive

JonasToth added a comment. Could you please explain what category means? Could i disable all of `cppcoreguidelines` with something like `// NOLINT (cppcoreguidelines-*)`? Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:297 + if (NolintIndex != StringRef::npos) { +a

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

JonasToth added inline comments. Comment at: test/clang-tidy/nolintnextline.cpp:14 + +// NOLINTNEXTLINE(not-closed-bracket-is-treated-as-skip-all +class C2 { C2(int i); }; xgsa wrote: > JonasToth wrote: > > missing `)` > No, it's intentionally: it's a test for ca

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

JonasToth added inline comments. Comment at: test/clang-tidy/nolintnextline.cpp:14 + +// NOLINTNEXTLINE(not-closed-bracket-is-treated-as-skip-all +class C2 { C2(int i); }; xgsa wrote: > JonasToth wrote: > > xgsa wrote: > > > JonasToth wrote: > > > > missing `)` >

[PATCH] D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

JonasToth created this revision. Herald added subscribers: cfe-commits, xazax.hun, mgorny. The original check did break the green buildbot in the sanitizer build. It took a while to redroduce and understand the issue. There occured a stackoverflow while parsing the AST. The testcase with 256 case

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

JonasToth added a comment. Its good that you added code examples to the clang-tidy documentation page. I think that feature was not documented well before. Comment at: test/clang-tidy/nolintnextline.cpp:23 + +// NOLINTNEXTLINE without-brackets-skip-all, another-check +class C5

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

JonasToth added inline comments. Comment at: test/clang-tidy/nolintnextline.cpp:23 + +// NOLINTNEXTLINE without-brackets-skip-all, another-check +class C5 { C5(int i); }; xgsa wrote: > JonasToth wrote: > > Ian confused now. The NOLINTNEXTLINE with incorrect paren

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

JonasToth added a comment. I am happy with everything now. But one of the reviewers must accept. https://reviews.llvm.org/D40671 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

JonasToth added a comment. It does still regress! In a sense this is a WIP that i did not clarify.(which i do now) While debugging the issue it seemed that parsing the AST did result in the stack overflow and not my check specific code. Looping over all case Labels seems to be done in a recursi

[PATCH] D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions

JonasToth added inline comments. Comment at: clang-tidy/modernize/ReplaceUncaughtExceptionCheck.cpp:20 + +static const char *MatchText = "::std::uncaught_exception"; + Could that be a local variable in `registerMatchers`? Even though its static and const it migh

[PATCH] D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions

JonasToth added inline comments. Comment at: clang-tidy/modernize/ReplaceUncaughtExceptionCheck.cpp:32 + Finder->addMatcher( + declRefExpr(to(functionDecl(hasName(MatchText.bind("call_expr"), + this); koldaniel wrote: > JonasToth wrote: > > Interes

[PATCH] D40829: [clang-tidy] adjust cppcoreguidelines-owning-memory documentation

JonasToth created this revision. Herald added subscribers: cfe-commits, kbarton, xazax.hun, nemanjai, klimek. A user of the check opened a bugreport and reported that `std::exchange` triggers a false positive. I adjusted the doc to include a list of known (std) constructs that do trigger the issue

[PATCH] D40737: [clang-tidy] WIP Resubmit hicpp-multiway-paths-covered without breaking test

JonasToth added a comment. @aaron.ballman I further investigated and the issue does not come directly from my code (at least thats what i believe) :) If i change the matcher simply to Finder->addMatcher(switchStmt().bind("switch"), this); The overflow still occurs! I assume that the recursiv

[PATCH] D40829: [clang-tidy] adjust cppcoreguidelines-owning-memory documentation

JonasToth added a comment. I agree. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40829 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40829: [clang-tidy] adjust cppcoreguidelines-owning-memory documentation

JonasToth updated this revision to Diff 125536. JonasToth added a comment. - [Fix] typo Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40829 Files: docs/clang-tidy/checks/cppcoreguidelines-owning-memory.rst Index: docs/clang-tidy/checks/cppcoreguidelines-owning-memory.rst =

[PATCH] D40829: [clang-tidy] adjust cppcoreguidelines-owning-memory documentation

This revision was automatically updated to reflect the committed changes. Closed by commit rL319785: [clang-tidy] adjust cppcoreguidelines-owning-memory documentation (authored by JonasToth). Repository: rL LLVM https://reviews.llvm.org/D40829 Files: clang-tools-extra/trunk/docs/clang-tidy

[PATCH] D40737: [clang-tidy] WIP Resubmit hicpp-multiway-paths-covered without breaking test

JonasToth updated this revision to Diff 125556. JonasToth retitled this revision from "[clang-tidy] WIP Resubmit hicpp-multiway-paths-covered without breaking test " to "[clang-tidy] WIP Resubmit hicpp-multiway-paths-covered without breaking test". JonasToth added a comment. Herald added a s

[PATCH] D40737: [clang-tidy] WIP Resubmit hicpp-multiway-paths-covered without breaking test

JonasToth added a comment. @sbenza and @klimek the issue that occurs here is probably ASTMatchers related, thats why I added you. The problem: This patch introduces a stack overflow when building with `RelWithDebInfo` that is caught by the sanitizers. I isolated the issue in the big switch sta

[PATCH] D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic

JonasToth created this revision. Herald added subscribers: cfe-commits, kbarton, xazax.hun, mgorny, nemanjai, klimek. This check implements rules ES.100 and ES.102 of the CppCoreGuidelines forbidding to mix `signed` and `unsigned` integer types in arithmetic expression. It currently functions

[PATCH] D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic

JonasToth updated this revision to Diff 125614. JonasToth added a comment. - [Misc] remove iostream Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40854 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-t

[PATCH] D41056: [clang-tidy] New check misc-uniqueptr-release-unused-retval

JonasToth added a comment. In https://reviews.llvm.org/D41056#950605, @khuttun wrote: > In https://reviews.llvm.org/D41056#950570, @Eugene.Zelenko wrote: > > > May be //bugprone// is better module then //misc//? > > > Maybe. I can move it if all the reviewers think that it would be better > suit

[PATCH] D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic

JonasToth added a comment. > The "enforcement" listed on the C++ Core Guidelines is very unhelpful, so it > might be worth bringing it up as an issue on their bug tracker. ES.100 > basically says "you know what we mean, wink wink" as enforcement and doesn't > give any guidance as to what is saf

[PATCH] D41056: [clang-tidy] New check misc-uniqueptr-release-unused-retval

JonasToth added a comment. > What do you think about making this check fully configurable through the > check options? The options could list the functions to check. The > documentation could suggest some candidate functions from std lib to > configure checks for. That sound reasonable. A simi

[PATCH] D41102: Setup clang-doc frontend framework

JonasToth added a comment. Hi julie, i like the new approach for doc generator. I added my thoughts on the code :) Comment at: tools/clang-doc/ClangDoc.cpp:81 + llvm::StringRef InFile) { + return std::unique_ptr( + new ClangDocConsumer(&C

[PATCH] D41102: Setup clang-doc frontend framework

JonasToth added a comment. You can erase some namespace prefix in code, e.g. llvm or clang, because you are working in them and/or had `using namespace clang`. Imo this is not required only if you find it ok to shorten the code. Comment at: tools/clang-doc/ClangDoc.cpp:30 +bo

[PATCH] D41102: Setup clang-doc frontend framework

JonasToth added inline comments. Comment at: tools/clang-doc/ClangDocReporter.cpp:157 + if (C->getNumAttrs() > 0) { +for (unsigned i = 0, e = C->getNumAttrs(); i != e; ++i) { + const HTMLStartTagComment::Attribute &Attr = C->getAttr(i); minor nit: the l

[PATCH] D130793: [clang-tidy] adjust treating of array-of-pointers when 'AnalyzePointers' is deactivated

JonasToth added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-values.cpp:27-30 + for (const int *p_local1 : p_local0) { + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: variable 'p_local1' of type 'const int *' can be de

[PATCH] D132244: [docs] improve documentation for misc-const-correctness

This revision was automatically updated to reflect the committed changes. Closed by commit rGb5b750346346: [docs] improve documentation for misc-const-correctness (authored by JonasToth). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132244/new/ htt

[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

JonasToth added a comment. just my 2 cents Comment at: clang-tools-extra/clang-tidy/readability/UseEarlyExitsCheck.cpp:66 + if (needsParensAfterUnaryNegation(Condition)) { +Diag << FixItHint::CreateInsertion(Condition->getBeginLoc(), "!(") + << FixItHint::CreateIns

[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

JonasToth added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/readability/use-early-exits.rst:63 +void Process(bool A, bool B) { + if (A && B) { +// Long processing. njames93 wrote: > JonasToth wrote: > > if this option i

[PATCH] D131386: [clang-tidy] Added `ConstAlignment` option to `misc-const-correctness`

JonasToth added a comment. during the original implementation of the fix-util I had the plan to provide an option for east-vs-west const, but during the discussions we came to the conclusion that `clang-format` should do this (the patches were already pending at that point in time). if this op

[PATCH] D131386: [clang-tidy] Added `ConstAlignment` option to `misc-const-correctness`

JonasToth added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-alignment.cpp:43 + // of the `*`. + int *IPtr = &I; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'IPtr' of type 'int *' can be declared 'const' ---

[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

JonasToth added a comment. IMHO the check looks good, but I do have concerns about correctness of the transformation in specific cases, especially overloaded operators. If the conditions are inverted, it might be the case that the inverted operator is not overloaded, resulting in compilation er

[PATCH] D54943: [clang-tidy] implement new check 'misc-const-correctness' to add 'const' to unmodified variables

JonasToth updated this revision to Diff 443484. JonasToth marked 4 inline comments as done. JonasToth added a comment. Herald added a reviewer: jdoerfert. Herald added subscribers: abrachet, sstefan1, phosek. - test: move tests into group specific directory - refactor: use more idiomatic c++ for s

[PATCH] D54943: [clang-tidy] implement new check 'misc-const-correctness' to add 'const' to unmodified variables

JonasToth added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/misc-const-correctness.rst:10 +`CppCoreGuidelines ES.25

[PATCH] D54943: [clang-tidy] implement new check 'misc-const-correctness' to add 'const' to unmodified variables

JonasToth added a comment. In D54943#3619333 , @LegalizeAdulthood wrote: > Clang-tidy tests and docs have been moved to subdirectories by module, please > rebase to `main:HEAD` I moved the tests and the documentation as well. tests + documentation do bu

[PATCH] D54943: [clang-tidy] implement new check 'misc-const-correctness' to add 'const' to unmodified variables

JonasToth updated this revision to Diff 443485. JonasToth added a comment. - fix: doc list and release-notes reference Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/new/ https://reviews.llvm.org/D54943 Files: clang-tools-extra/clang-tidy/m

<    5   6   7   8   9   10   11   12   13   14   >