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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
Eugene.Zelenko added a comment.
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 basically checking
> CxxThisExpr, being a handle, is not modified within a me
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 :
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
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
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
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
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
lebedev.ri added a comment.
Thank you for working on this check!
Comment at: clang-tidy/cppcoreguidelines/ConstCheck.h:60
+
+ std::unordered_map ValueCanBeConst;
+ std::unordered_map HandleCanBeConst;
I'm guessing these should be `llvm::DenseMap`.
Repositor
Eugene.Zelenko added a comment.
In https://reviews.llvm.org/D45444#1063291, @JonasToth wrote:
> > It'll be good idea to have option to apply this check for
> > pointer/references only, or include built-in types/enums.
>
> Agreed. I aim at a `mark handles const`(default on), `mark values
> const
JonasToth added a comment.
> It'll be good idea to have option to apply this check for pointer/references
> only, or include built-in types/enums.
Agreed. I aim at a `mark handles const`(default on), `mark values
const`(default on for objects), `mark pointer (const int * >> const <<) const`
(d
Eugene.Zelenko added a comment.
Will be good idea to add HICPP alias.
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-co
Eugene.Zelenko added a comment.
It'll be good idea to have option to apply this check for pointer/references
only, or include built-in types/enums.
Comment at: clang-tidy/cppcoreguidelines/ConstCheck.cpp:13
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+#include
--
JonasToth added a comment.
@aaron.ballman, @lebedev.ri, @alexfh and everyone else, i would love to hear
your opinion on my approach. i never implemented a similar check and i might
miss some cases, that need to be considered.
The current state is just a proof-of-concept to see how such a check
JonasToth created this revision.
JonasToth added reviewers: alexfh, aaron.ballman, lebedev.ri, hokein.
Herald added subscribers: cfe-commits, kbarton, xazax.hun, mgorny, nemanjai,
klimek.
This check aims to determine values and references that could be declared
as const, but are not.
The first v
30 matches
Mail list logo
