User Details
- User Since
- Oct 31 2016, 11:13 AM (220 w, 4 d)
Nov 13 2020
LGTM! thanks for fixing.
Oct 12 2020
Hey Alex, first thanks for the bug fixing! Of course we are interested in improvements in clang-tidy, but i think noone is actually employed to help out here and the normal reviewers have many, many reviews parallel. Sometimes stuff just slips through :/
Oct 11 2020
You can land it. Worst case would be post-commit comments, but this is not a complicated patch and should be fine!
I am fine now, thank you!
The issue is resolved now in the CppCoreGuidelines.
The context thingie was not that important here, but still thanks :)
LGTM.
Short reminder to please use full context patches. Its not an issue right now, but review sometimes just needs the context to understand the changes :)
Oct 10 2020
- fix test RUN line
- no delayed template parsing
- adjust release note issue
Oct 9 2020
- missed one place of decltype
- fix platform dependent warning message for decltype to just match the beginning and not the 'a.k.a'
- address review comments
- update to landed ExprMutAnalyzer changes
- [Feature] add elvis operator detection
Oct 7 2020
- fix imprecisions
Oct 6 2020
rebase to newest expmutanalyzer patch
@aaron.ballman I update the code a bit. This stuff is just a hydra :/
- document sections in the testcases, hopefully little more structure for comprehension
- fix derived-to-base-cast OUTSIDE of an conditional operator
- improve type-dependent callexpr, too
- better canResolveToExpr matcher code
- adjust matcher for comma
I am confused now, but richard knows this stuff very well! The patches you referenced showed only cases where the static was defined in an inline-header definition (and templates). Does this make a difference on the semantics?
This should be clear before we continue.
Oct 5 2020
Sep 26 2020
Sep 25 2020
- fix typo that provided wrong argument to AST building
- address review comments
Sep 24 2020
- adress review comments
@AlexanderLanin @0x8000-0000 i created the branch release-11-const (https://github.com/JonasToth/llvm-project/tree/release-11-const) in my fork.
rebase to current exprmutanalyzer
- improve the expression matcher, to catch the expression in general ternary-operator cases, too
- considered unresolved operator calls to be a modification
- fix method calls with templates in some instances
Sep 23 2020
Sep 22 2020
- remove spurious formatting change
- rebase to revision for https://reviews.llvm.org/D88088
- include ExprMutAnalyzer implementation changes, that got lost somehow with prior rebase
- rebase to master after AST Matchers are extracted
Sep 15 2020
- rebase
Sep 14 2020
- fix compilation error from removed brace
Sep 13 2020
- fix left over merge marker and one formatting problem
Sorry for missing this review, i simply hadn't time :/
All my concerns are addressed and I think this is LGTM after rebasing.
ping.
This checks seems pretty much done. @jranieri-grammatech do you think there is something missing? Otherwise it could be rebased and comitted?
ping.
@njames93 are you revisiting this patch? What is the missing piece? Maybe i can help out a bit?
Sep 10 2020
LGTM
Jun 16 2020
great to see such a check!
Jan 30 2020
Jan 25 2020
We need tests for that. There should be tests for the exporting-feature. Maybe they could provide a good starting point on adding tests. (not sure if there is unit-test code for isolated testing, but some tests do exist!)
Yeah, the libc++ failures are not your fault :)
My 2 cents added.
Jan 20 2020
Jan 19 2020
Jan 13 2020
K. Thank you for fixing it!
LGTM in general. The linked bug report misses a "4" --> PR44440, so please the description for that.
Jan 11 2020
@gkistanova Do you know the toolchain on the power-pc builders? I get an ICE with this patch (see the prior commit) and would like to find out whats the problem. Thanks for your time!
The powerpc buildbots get an internal compiler error and crash. No Idea why, reverted for now.
- remove merge conflict markers in docs
- fix compilation error, missing getType
- add test for K&R functions
Thank you for looking into this; I'll start building diff14 to test locally.
So that is were the CTU question comes from? :)
The other recursion check seems more sophisticated. What is your take on it? Would you consider it better as well, what is your opinion on it?
@0x8000-0000 i did remove dependentTypes from matching. Locally that works with your reported test-case (i was running clang-tidy wrong :().
- fix false positive with ignoring dependentTypes
- remove pointee-analysis artifacts as this will be done later
- remove unnecessary comments
Jan 10 2020
- test if references to pointers are ignored if pointers are ignored as values
- add different possible auto constellations
- document the problematic cases for 'auto' deductions
- address review comments
- fix unit tests for exprmutanalyzer, whitespace was the problem
- Merge branch 'master' into feature_transform_const.patch
Jan 9 2020
- Merge branch 'master' into feature_transform_const.patch
- actually dismiss function references
- work on exclusion of variables that have type, autotype or reference-type to template parameter
- by default analyze the pointers and transform as well, for testing
- fix canResolveToExpr to account for derivedToBaseClass casts
- restore unit tests for expr mut analyzer - there was an oversight in my build that lead them not being running
- short id for matcher
- fix test for fixed false negative
- rip out unused matchers that are now unnecessary
- try to ignore auto-types that are type-dependent
- consider 'static_cast<Type&>()' always a mutation, speculativly fix tests for that
LGTM.
This check already existed in clang-tidy and had a bug. So we should fix that (thank you for that) and probably even backport (release coming soon).
So even though clang itself might be able to do it, clang-tidy should do it properly as well.
Jan 8 2020
I did evaluate the performance of the check on LLVM with the script in the attachement.
Adding a -fno-delayed-template-parsing to the RUN-line should suffice.