[PATCH] D52219: [analyzer] (1/n) Support pointee mutation analysis in ExprMutationAnalyzer.

shuaiwang updated this revision to Diff 166068. shuaiwang marked 7 inline comments as done. shuaiwang added a comment. [WIP] Addressed some of review comments. Repository: rC Clang https://reviews.llvm.org/D52219 Files: include/clang/Analysis/Analyses/ExprMutationAnalyzer.h lib/Analysis/

[PATCH] D52219: [analyzer] (1/n) Support pointee mutation analysis in ExprMutationAnalyzer.

shuaiwang marked 5 inline comments as done. shuaiwang added inline comments. Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:67 +if (const auto *DRE = dyn_cast(E)) { + if (DRE->getNameInfo().getAsString()[0] == 'p') +Finder = PointeeMutationFinder; --

[PATCH] D52219: [analyzer] (1/n) Support pointee mutation analysis in ExprMutationAnalyzer.

shuaiwang added inline comments. Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:67 +if (const auto *DRE = dyn_cast(E)) { + if (DRE->getNameInfo().getAsString()[0] == 'p') +Finder = PointeeMutationFinder; JonasToth wrote: > shuaiwang

[PATCH] D52219: [analyzer] (1/n) Support pointee mutation analysis in ExprMutationAnalyzer.

shuaiwang marked 10 inline comments as done. shuaiwang added inline comments. Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:86 +// - Pointer to non-const +// - Pointer-like type with `operator*` returning non-const reference +bool isPointeeMutable(const Expr *Exp, const ASTCo

[PATCH] D52219: [analyzer] (1/n) Support pointee mutation analysis in ExprMutationAnalyzer.

shuaiwang updated this revision to Diff 166595. shuaiwang marked 5 inline comments as done. shuaiwang added a comment. Addresses review comments. Repository: rC Clang https://reviews.llvm.org/D52219 Files: include/clang/Analysis/Analyses/ExprMutationAnalyzer.h lib/Analysis/ExprMutationAn

[PATCH] D52219: [analyzer] (1/n) Support pointee mutation analysis in ExprMutationAnalyzer.

shuaiwang added inline comments. Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:156 EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()")); + + AST = tooling::buildASTFromCode( JonasToth wrote: > shuaiwang wrote: > > JonasToth wrote: >

[PATCH] D52219: [analyzer] (1/n) Support pointee mutation analysis in ExprMutationAnalyzer.

shuaiwang updated this revision to Diff 166710. shuaiwang added a comment. Added test case place holder for cases that should be supported in later patches. Repository: rC Clang https://reviews.llvm.org/D52219 Files: include/clang/Analysis/Analyses/ExprMutationAnalyzer.h lib/Analysis/Ex

[PATCH] D52219: [analyzer] (1/n) Support pointee mutation analysis in ExprMutationAnalyzer.

shuaiwang updated this revision to Diff 166947. shuaiwang marked 6 inline comments as done. shuaiwang added a comment. Addressed review comments. Repository: rC Clang https://reviews.llvm.org/D52219 Files: include/clang/Analysis/Analyses/ExprMutationAnalyzer.h lib/Analysis/ExprMutationAn

[PATCH] D52219: [analyzer] (1/n) Support pointee mutation analysis in ExprMutationAnalyzer.

shuaiwang updated this revision to Diff 168578. shuaiwang marked 5 inline comments as done. shuaiwang added a comment. Resolved review comments. Repository: rC Clang https://reviews.llvm.org/D52219 Files: include/clang/Analysis/Analyses/ExprMutationAnalyzer.h lib/Analysis/ExprMutationAna

[PATCH] D54399: Move ExprMutationAnalyzer to Tooling/Analysis (1/3)

shuaiwang added a comment. Herald added a project: clang. Hi @rsmith what do you think of just get this in since Eric is not responding? Or do you think I should run certain test to verify? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54399/new/ https://reviews

[PATCH] D54399: Move ExprMutationAnalyzer to Tooling/Analysis (1/3)

shuaiwang added a comment. In D54399#1391975 , @lebedev.ri wrote: > In D54399#1391935 , @shuaiwang wrote: > > > Hi @rsmith what do you think of just get this in since Eric is not > > responding? Or do you think I s

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

shuaiwang created this revision. Herald added subscribers: cfe-commits, mgorny, klimek. shuaiwang added a reviewer: hokein. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45679 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ReadabilityTidyModule.cpp cla

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

shuaiwang 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 do have a strong opinion that requires non-trivi

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

shuaiwang added a comment. >> I would imagine things could get messier if this check expands to also >> check for turning member functions const: it's basically checking >> CxxThisExpr, being a handle, is not modified within a member function, but >> note there's no VarDecl for "this". > > U

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

shuaiwang added a comment. In https://reviews.llvm.org/D45444#1068967, @Eugene.Zelenko wrote: > In https://reviews.llvm.org/D45444#1068496, @shuaiwang wrote: > > > - I would imagine things could get messier if this check expands to also > > check for turning member functions const: it's basicall

[PATCH] D45702: [clang-tidy] Add a new check, readability-redundant-data-call, that finds and removes redundant calls to .data().

shuaiwang created this revision. Herald added subscribers: cfe-commits, xazax.hun, mgorny, klimek. shuaiwang added a reviewer: sbenza. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45702 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ReadabilityTidyModul

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

shuaiwang updated this revision to Diff 142730. shuaiwang added a comment. Updated mostly `isModified()`. I'd like to mostly demonstrate that `isModified()` works given that there's https://reviews.llvm.org/D45444 and we'd like to merge. Repository: rCTE Clang Tools Extra https://reviews.llv

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

shuaiwang added a comment. 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 plan to use it here. I copied your const-values test cases and it now

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

shuaiwang updated this revision to Diff 142739. shuaiwang added a comment. Change to just add a helper function `isModified` Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45679 Files: clang-tidy/utils/ASTUtils.cpp clang-tidy/utils/ASTUtils.h unittests/clang-tidy/CMakeLis

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

shuaiwang marked an inline comment as done. shuaiwang added a comment. Hi @alexfh, @hokein, @JonasToth, I've updated this diff to be just adding the helper function `isModified()` with a set of dedicated unit tests. What do you think of the approach of getting this change committed and then @Jon

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

shuaiwang added a comment. In https://reviews.llvm.org/D45679#1069754, @Eugene.Zelenko wrote: > In https://reviews.llvm.org/D45679#1069638, @JonasToth wrote: > > > *hust* /llvm/tools/clang/lib/Analysis/PseudoConstantAnalysis.cpp > > > > I will check this one first, before we get crazy and impleme

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

shuaiwang updated this revision to Diff 142870. shuaiwang edited the summary of this revision. shuaiwang added a comment. Handle casts, ternary operator and lambdas. Also optionally returns a chain of Stmt giving more details about how the conclusion is reached. Repository: rCTE Clang Tools E

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

shuaiwang updated this revision to Diff 142878. shuaiwang added a comment. Handle range-based for loop Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45679 Files: clang-tidy/utils/ASTUtils.cpp clang-tidy/utils/ASTUtils.h unittests/clang-tidy/CMakeLists.txt unittests/cla

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

shuaiwang updated this revision to Diff 142882. shuaiwang added a comment. Better range for loop handling. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45679 Files: clang-tidy/utils/ASTUtils.cpp clang-tidy/utils/ASTUtils.h unittests/clang-tidy/CMakeLists.txt unittests

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

shuaiwang updated this revision to Diff 143197. shuaiwang marked 13 inline comments as done. shuaiwang added a comment. - Moved out of ASTUtils and becomes a separte class ExprMutationAnalyzer (bikeshedding on name a bit.) - Reverted back to return bool instead of tri-valued enum, the meanings of

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

shuaiwang added a comment. Regarding full dependency analysis, I think it's out of the scope of this change at least for now. We're only trying to find //one// possible point where the given `Expr` is (assumed to be) mutated. This is known to be useful at this point for use cases like "check wh

[PATCH] D45702: [clang-tidy] Add a new check, readability-redundant-data-call, that finds and removes redundant calls to .data().

shuaiwang updated this revision to Diff 143204. shuaiwang marked 7 inline comments as done. shuaiwang added a comment. Addressed review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45702 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/Readabi

[PATCH] D45702: [clang-tidy] Add a new check, readability-redundant-data-call, that finds and removes redundant calls to .data().

shuaiwang updated this revision to Diff 143207. shuaiwang marked 7 inline comments as done. shuaiwang added a comment. Addressed more comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45702 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/Readabili

[PATCH] D45702: [clang-tidy] Add a new check, readability-redundant-data-call, that finds and removes redundant calls to .data().

shuaiwang added inline comments. Comment at: clang-tidy/readability/RedundantDataCallCheck.cpp:22 + +const char kDefaultTypes[] = +"::std::basic_string;::std::basic_string_view;::std::vector;::std::array"; Eugene.Zelenko wrote: > Actually you should use stati

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

shuaiwang added inline comments. Comment at: clang-tidy/cppcoreguidelines/ConstCheck.cpp:229-237 + const auto *UseExpr = selectFirst("use", Usage); + + // The declared variables was used in non-const conserving way and can not + // be declared as const. + if (UseExpr && Scope

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

shuaiwang updated this revision to Diff 144490. shuaiwang marked 11 inline comments as done. shuaiwang added a comment. Added an isMutated overload for Decl. A few more test cases. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45679 Files: clang-tidy/utils/CMakeLists.txt c

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

shuaiwang added a comment. > 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, > because we analysis on a `Expr` basis? Yes. the analyzer

[PATCH] D45702: [clang-tidy] Add a new check, readability-redundant-data-call, that finds and removes redundant calls to .data().

shuaiwang added inline comments. Comment at: clang-tidy/readability/RedundantDataCallCheck.cpp:45 + anyOf(TypesMatcher, pointerType(pointee(TypesMatcher)), + callee(namedDecl(hasName("data" + .bind("call", aa

[PATCH] D45702: [clang-tidy] Add a new check, readability-redundant-data-call, that finds and removes redundant calls to .data().

shuaiwang added a comment. In https://reviews.llvm.org/D45702#1085224, @aaron.ballman wrote: > > Have you run this over any large code bases to see whether the check > > triggers in practice? > > I'm still curious about this, btw. Yes it triggers in Google's code base. Repository: rCTE Cla

[PATCH] D45702: [clang-tidy] Add a new check, readability-redundant-data-call, that finds and removes redundant calls to .data().

shuaiwang added a comment. In https://reviews.llvm.org/D45702#1086250, @aaron.ballman wrote: > In https://reviews.llvm.org/D45702#1085890, @shuaiwang wrote: > > > In https://reviews.llvm.org/D45702#1085224, @aaron.ballman wrote: > > > > > > Have you run this over any large code bases to see wheth

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

shuaiwang updated this revision to Diff 145121. shuaiwang marked 4 inline comments as done. shuaiwang added a comment. Addressed review comments, notably added check for DeclRefExpr to volatile variables. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45679 Files: clang-tidy

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

shuaiwang added inline comments. Comment at: clang-tidy/utils/ExprMutationAnalyzer.cpp:82-83 +const auto *E = RefNodes.getNodeAs("expr"); +if (findMutation(E)) + return E; + } aaron.ballman wrote: > Why does this not return the result of `findMutati

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

shuaiwang updated this revision to Diff 145313. shuaiwang added a comment. Revert "DeclRefExpr to volatile handling" part of last update. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45679 Files: clang-tidy/utils/CMakeLists.txt clang-tidy/utils/ExprMutationAnalyzer.cpp

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

shuaiwang marked 5 inline comments as done. shuaiwang added a comment. I've reverted the handling of DeclRefExpr to volatile after rethinking about it. The purpose of this analyzer is to find whether the given Expr is mutated within a scope, but doesn't really care about external changes. In oth

[PATCH] D45702: [clang-tidy] Add a new check, readability-redundant-data-call, that finds and removes redundant calls to .data().

shuaiwang added inline comments. Comment at: clang-tidy/readability/RedundantDataCallCheck.cpp:45 + anyOf(TypesMatcher, pointerType(pointee(TypesMatcher)), + callee(namedDecl(hasName("data" + .bind("call", aa

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

shuaiwang added a comment. Could someone help commit this now that the build issue with header include is fixed? Thanks a lot! (meanwhile I'm requesting commit access) In https://reviews.llvm.org/D45679#1132327, @alexfh wrote: > In https://reviews.llvm.org/D45679#1132086, @JonasToth wrote: > >

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

shuaiwang created this revision. Herald added a reviewer: george.karpenkov. Herald added subscribers: cfe-commits, a.sidorin. This gives better coverage to the check as ExprMutationAnalyzer is more accurate comparing to isOnlyUsedAsConst. Majority of wins come from const usage of member field, e.

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

shuaiwang marked an inline comment as done. shuaiwang added inline comments. Comment at: test/clang-tidy/performance-for-range-copy.cpp:120 +struct Point { + ~Point() {} JonasToth wrote: > I feel that `const` methods should be added as a test as well. const me

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

This revision was automatically updated to reflect the committed changes. shuaiwang marked an inline comment as done. Closed by commit rL336737: Use ExprMutationAnalyzer in performance-for-range-copy (authored by shuaiwang, committed by ). Herald added a subscriber: llvm-commits. Repository: rL

[PATCH] D45153: test

shuaiwang created this revision. Herald added subscribers: cfe-commits, klimek. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45153 Files: clang-tidy/utils/ASTUtils.cpp clang-tidy/utils/ASTUtils.h Index: clang-tidy/utils/ASTUtils.h ==

<    1   2