xazax.hun created this revision.
Right now CStringSytanxChecker assumes that the argument of a sizeof expression
is an expression. The argument can also be a type. In this case an assertion
fail will be triggered when the SubExpression is being queried. I fixed this
issue and did other minor
xazax.hun added a comment.
I think this might be better as a readability checker to find misleading
variable or parameter names.
It would also be great to consider types. Unfortunately it probably means
reimplementing some of the logic from Sema, since that information is not
available at
xazax.hun added a comment.
Hi Benedek, could you do the merge or should anybody commandeer these revisions?
Repository:
rL LLVM
https://reviews.llvm.org/D23423
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
xazax.hun added a comment.
Benedek, do you have time to address the review comments or do you want me to
commandeer this revision?
Repository:
rL LLVM
https://reviews.llvm.org/D23421
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
xazax.hun added a comment.
Great find! Could you transform your examole into a testcasr that fails before
this patch? I think there should be already some tests for call graphs that
you can take a look at.
https://reviews.llvm.org/D29183
___
xazax.hun marked 3 inline comments as done.
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/ValistChecker.cpp:178
+VaListModelledAsArray = Cast->getCastKind() == CK_ArrayToPointerDecay;
+ const MemRegion *Reg = SV.getAsRegion();
+ if (const auto
xazax.hun updated this revision to Diff 89185.
xazax.hun added a comment.
- Address some review comments.
- Add some additional tests.
- Fixed some false positives (checking for symbolic values for va_copy and more
robust detection of which valist model is used by the platform)
- I have run the
xazax.hun updated this revision to Diff 89187.
xazax.hun marked 3 inline comments as done.
xazax.hun added a comment.
- Fixed a comment.
https://reviews.llvm.org/D30157
Files:
lib/StaticAnalyzer/Checkers/ValistChecker.cpp
test/Analysis/valist-uninitialized-no-undef.c
xazax.hun added inline comments.
Comment at: test/Analysis/valist-uninitialized-no-undef.c:19
+ // FIXME: There should be no warning for this.
+ (void)va_arg(*fst, int); // expected-warning{{va_arg() is called on an
uninitialized va_list}} expected-note{{va_arg() is called on
xazax.hun added a comment.
Nice check! Thank you for working on this!
Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:30
+" The old user defined 'RandomFunction' is not usable for 'shuffle'. You "
+"need to "
+"make additional changes if you want a
xazax.hun added inline comments.
Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:81
+ Stream << "shuffle(";
+ FirstArgument->printPretty(Stream, nullptr, Ctx.getPrintingPolicy());
+ Stream << ", ";
madsravn wrote:
> xazax.hun wrote:
> >
xazax.hun added inline comments.
Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:30
+" The old user defined 'RandomFunction' is not usable for 'shuffle'. You "
+"need to "
+"make additional changes if you want a specific random function.";
xazax.hun created this revision.
This patch makes the valist check more robust to the different kind of ASTs
that are generated on different platforms:
Generated on x86_64-pc-linux-gnu:
|-TypedefDecl 0x1d07a40 <> implicit
__builtin_ms_va_list 'char *'
| `-PointerType 0x1d07a00 'char *'
xazax.hun added a comment.
In https://reviews.llvm.org/D15227#681127, @zaks.anna wrote:
> > But as far as I remember, this produced false negatives in the tests not
> > false positives.
>
> Could you double check that? Maybe you still have some notes in your mail box
> or just by looking at
xazax.hun added a comment.
In https://reviews.llvm.org/D6550#663002, @a.sidorin wrote:
> Hi Gabor. One of the bugs fixed in this patch is still present in master,
> other two are already fixed.
Thanks for checking that! Do you think it is ok for me to commit the missing
part?
xazax.hun added a comment.
In https://reviews.llvm.org/D6549#662955, @a.sidorin wrote:
> This should be fixed in r269693.
Indeed, I commandeer than abandon this revision so it is closed.
https://reviews.llvm.org/D6549
___
cfe-commits mailing
xazax.hun updated this revision to Diff 88333.
xazax.hun marked 9 inline comments as done.
xazax.hun added a comment.
- Updated to latest trunk.
- The cert rule was renamed, the patch is updated accordingly.
- Fixes as per review comments.
https://reviews.llvm.org/D23421
Files:
xazax.hun added inline comments.
Comment at: clang-tidy/readability/MisleadingIndentationCheck.cpp:79
+ Finder->addMatcher(
+ compoundStmt(anyOf(has(ifStmt()), has(forStmt()), has(whileStmt(
+ .bind("compound"),
alexfh wrote:
> xazax.hun
xazax.hun updated this revision to Diff 88330.
xazax.hun marked 7 inline comments as done.
xazax.hun added a comment.
- Added a note to make it easier to understand the diagnostics.
- Reworded the error message about dangling else.
- Fixed other review comments.
https://reviews.llvm.org/D19586
xazax.hun added a comment.
Thank you for the review!
Comment at: clang-tidy/readability/MisleadingIndentationCheck.cpp:79
+ Finder->addMatcher(
+ compoundStmt(anyOf(has(ifStmt()), has(forStmt()), has(whileStmt(
+ .bind("compound"),
alexfh
xazax.hun added a comment.
In https://reviews.llvm.org/D15227#674278, @zaks.anna wrote:
> @xazax.hun,
>
> Can we move this out of alpha?
>
> Have this checkers been tested on a large codebase? What are false positive
> rates?
I have tested it on a few ~200k LOC C codebase and I did not see
xazax.hun added a comment.
Shouldn't this be a path sensitive check within the clang static analyzer
instead? So branches are properly handled and interprocedural analysis is done.
https://reviews.llvm.org/D29839
___
cfe-commits mailing list
xazax.hun added a comment.
In https://reviews.llvm.org/D29839#674307, @Prazek wrote:
> In https://reviews.llvm.org/D29839#674301, @xazax.hun wrote:
>
> > Shouldn't this be a path sensitive check within the clang static analyzer
> > instead? So branches are properly handled and interprocedural
xazax.hun updated this revision to Diff 88506.
xazax.hun marked 3 inline comments as done.
xazax.hun retitled this revision from "[Clang-tidy] CERT-MSC53-CPP (checker for
std namespace modification)" to "[Clang-tidy] CERT-DCL58-CPP (checker for std
namespace modification)".
xazax.hun edited the
xazax.hun created this revision.
During the review of https://reviews.llvm.org/D29567 it turned out the caching
in CallDescription is not implemented properly. In case an identifier does not
exist in a translation unit, repeated identifier lookups will be done which
might have bad impact on
xazax.hun abandoned this revision.
xazax.hun added a comment.
For the first case ToT clang compiler gives a warning (-Wstring-compare), for
the second case, it generates a compiler error (error: ordered comparison
between pointer and zero). Note that, older versions of clang did not even give
xazax.hun abandoned this revision.
xazax.hun added a comment.
-Wtautological-pointer-compare already covers this case.
Repository:
rL LLVM
https://reviews.llvm.org/D23423
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
xazax.hun added inline comments.
Comment at:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:73
+ : II(nullptr), IsLookupDone(false), FuncName(FuncName),
+RequiredArgs(RequiredArgs) {}
NoQ wrote:
> Maybe
xazax.hun added a comment.
In https://reviews.llvm.org/D29884#677387, @NoQ wrote:
> Yep, seems that somebody has missed these issues :)
>
> I guess there's no way to test the operator case, because nobody made a
> CallDescription with an empty name for us (maybe we should even assert that).
xazax.hun updated this revision to Diff 88541.
xazax.hun added a comment.
- Do not warn for function specializations within the std namespace.
- Add a test case for swap.
https://reviews.llvm.org/D23421
Files:
clang-tidy/cert/CERTTidyModule.cpp
clang-tidy/cert/CMakeLists.txt
xazax.hun commandeered this revision.
xazax.hun edited reviewers, added: Pajesz; removed: xazax.hun.
xazax.hun added a comment.
Herald added a subscriber: mgorny.
The original author is no longer available.
https://reviews.llvm.org/D19586
___
xazax.hun updated this revision to Diff 88182.
xazax.hun added a comment.
- Updated to latest trunk.
- Mentioned check in the release notes.
- Documented the limitation that tabs and spaces need to be consistent for this
check to work.
- Fixed (hopefully all) review comments.
- Fixed the test
xazax.hun marked 13 inline comments as done.
xazax.hun added inline comments.
Comment at: clang-tidy/readability/MisleadingIndentationCheck.cpp:20
+
+void MisleadingIndentationCheck::danglingElseCheck(
+const MatchFinder::MatchResult ) {
danielmarjamaki
xazax.hun added a comment.
It is not supported to run the analyzer with some of the core checkers turned
off. Maybe we should change the behavior such that turning off core checkers
turn off the warnings from those checkers but not the checkers themselves?
https://reviews.llvm.org/D28765
xazax.hun added a comment.
You might want to give CodeChecker [1] a try as a workaround. It stores the
results in a more compact format and you can do filtering.
[1] https://github.com/Ericsson/codechecker
https://reviews.llvm.org/D28765
___
xazax.hun updated this revision to Diff 89857.
xazax.hun edited the summary of this revision.
xazax.hun added a comment.
- Move the check out of alpha.
https://reviews.llvm.org/D30157
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/ValistChecker.cpp
xazax.hun added a comment.
There is an alternative approach idea:
This is not found by ArrayBoundCheckerV2? If no, an alternative approach would
be to properly set the constraints on the extent of the VLA's memory region.
After that, maybe ArrayBoundCheckerV2 would work automatically on this
xazax.hun added a comment.
LGTM!
https://reviews.llvm.org/D27600
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
xazax.hun added inline comments.
Comment at: lib/Format/Encoding.h:136
+ }
+ while (Left + 1 < Right) {
+assert(ComputeWidth(Left) <= Width && "binary search left invariant");
Was just skimming through this patch. What is the reason to use a hand written
xazax.hun added inline comments.
Comment at: docs/clang-tidy/checks/misc-string-compare.rst:10
+equality or inequality operators. The compare method is intended for sorting
+functions and thus returns ``-1``, ``0`` or ``1`` depending on the
lexicographical
+relationship
xazax.hun added a comment.
Did you experience any problems with the array out of bounds check lately? In
case it was stable on large code-bases and did not give too many false
positives, I think it might be worth to move that check out of alpha at the
same time, so users who do not turn on
xazax.hun added a comment.
Small ping, is this ready to be committed?
https://reviews.llvm.org/D21298
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Core/CoreEngine.cpp:662
+ bool EdgeExists = false;
+ for (auto I = N->pred_begin(), E = N->pred_end(); I != E; ++I)
+if (*I == FromN) {
I prefer having the braces written in this case, but it
xazax.hun added inline comments.
Comment at: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp:125-126
+}
+diag(Ctor->getLocation(), "function %0 can hide copy and move
constructors")
+<< Ctor;
+ }
aaron.ballman wrote:
> aaron.ballman wrote:
xazax.hun added inline comments.
Comment at: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp:125-126
+}
+diag(Ctor->getLocation(), "function %0 can hide copy and move
constructors")
+<< Ctor;
+ }
aaron.ballman wrote:
> xazax.hun wrote:
> >
xazax.hun added a comment.
What is the status of this?
Aleksei, could you upload a new patch with the context available?
(And also with a testcase added for jumps/gotos and VLA.)
You modified the malloc checker but I did not see a test for that.
https://reviews.llvm.org/D19979
xazax.hun added a comment.
So there is no LLVM supported target with different bit width pointer types?
In case we have such a target upstreamed, you can add a test with the
host-triple hardcoded into the test.
Repository:
rL LLVM
https://reviews.llvm.org/D31029
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:60
+// Set timeout to 15000ms = 15s
+Z3_set_param_value(Config, "timeout", "15000");
+ }
Sorry for being a bit late in the party, I was wondering whether this
xazax.hun added a comment.
I wonder whether warning on implicit casts still makes sense for example in
mission critical code. So maybe it is worth to have a configuration option with
the default setting being less strict and chatty. What do you think?
Repository:
rL LLVM
xazax.hun added a comment.
In https://reviews.llvm.org/D31097#704626, @alexfh wrote:
> In https://reviews.llvm.org/D31097#704621, @xazax.hun wrote:
>
> > I wonder whether warning on implicit casts still makes sense for example in
> > mission critical code. So maybe it is worth to have a
xazax.hun added a comment.
In https://reviews.llvm.org/D30489#689947, @zaks.anna wrote:
> Should we just remove ArrayBoundChecker.cpp or is there a value in keeping it
> around?
I think once ArrayBoundCheckerV2 is in production we should only keep
ArrayBoundChecker there as educational
xazax.hun added a comment.
This is a very welcome addition. I hope the performance will be still good :)
Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1023
+
+SVal VisitSymIntExpr(const SymIntExpr *S) {
+ SValBuilder =
xazax.hun marked 6 inline comments as done.
xazax.hun added inline comments.
Comment at: test/Analysis/Inputs/externalFnMap.txt:1
+_Z7h_chaini@x86_64 xtu-chain.cpp.ast
+_ZN4chns4chf2Ei@x86_64 xtu-chain.cpp.ast
NoQ wrote:
> These tests use a pre-made external
xazax.hun updated this revision to Diff 94709.
xazax.hun edited the summary of this revision.
xazax.hun added a comment.
- Fixed some memory leaks.
- Removed some unrelated style changes.
- Simplifications to the python scripts.
- Removed debug prints.
https://reviews.llvm.org/D30691
Files:
xazax.hun updated this revision to Diff 94814.
xazax.hun edited the summary of this revision.
xazax.hun added a comment.
- Removed a clang tool, replaced with python tool functionality.
https://reviews.llvm.org/D30691
Files:
include/clang/AST/ASTContext.h
include/clang/AST/Decl.h
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
LGTM! Thank you for working on this!
https://reviews.llvm.org/D31886
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
xazax.hun added a comment.
One last question: maybe we want to skip this kind of simplification in case of
Z3?
Probably the constraint managers could have a flag like
"wantsSimplifiedConstraints"?
Maybe somehow the checkers that are doing their own simplification could
respect this flag as
xazax.hun added a comment.
Maybe this is a good time to decide ClangD, Clangd, or clangd is the correct
capitalization.
Repository:
rL LLVM
https://reviews.llvm.org/D31887
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
LGTM!
https://reviews.llvm.org/D32291
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
xazax.hun marked 6 inline comments as done and an inline comment as not done.
xazax.hun added a comment.
In https://reviews.llvm.org/D30691#731617, @zaks.anna wrote:
> I agree that scan-build or scan-build-py integration is an important issue to
> resolve here. What I envision is that users
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:975
+ if (Op->getOpcode() == UO_AddrOf)
+if (Op->getSubExpr()->isLValue()) {
+ Ex = Op->getSubExpr()->IgnoreParenCasts();
Maybe you could move this
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
In https://reviews.llvm.org/D30841#698634, @fgross wrote:
> I just assumed it would traverse in the "right" way, is there any
> documentation about AST / matcher traversal?
I do not
xazax.hun added a comment.
Also, maybe the readability module would be a better place for this check.
Repository:
rL LLVM
https://reviews.llvm.org/D30896
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
xazax.hun added a comment.
In https://reviews.llvm.org/D30798#697115, @zaks.anna wrote:
> I've committed the change, but would very much appreciate community feedback
> here if if there is any!
I agree with the change. Users are usually not interested in the results from
the standard
xazax.hun added inline comments.
Comment at: clang-tidy/misc/MiscTidyModule.cpp:70
+CheckFactories.registerCheck(
+"misc-forwarding-reference-overload");
CheckFactories.registerCheck("misc-misplaced-const");
malcolm.parsons wrote:
>
xazax.hun added a comment.
Functionally LGTM!
Note that while the traversal of AST Matchers are not defined in general, in
this particular case of chained ifs, it is guaranteed that parent nodes will be
matched before the child nodes. For this reason I think it is ok to have a
state like
xazax.hun added a comment.
In https://reviews.llvm.org/D30157#689374, @danielmarjamaki wrote:
> I am running this checker right now on various projects. Here are currently
> seen results.. https://drive.google.com/open?id=0BykPmWrCOxt2STZMOXZ5OGlwM3c
>
> Feel free to look at it and see if there
xazax.hun updated this revision to Diff 90676.
xazax.hun marked an inline comment as done.
xazax.hun added a comment.
- Improve the readability of test files.
- Rebased on latest ToT.
https://reviews.llvm.org/D30157
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
xazax.hun added inline comments.
Comment at: test/clang-tidy/modernize-use-nullptr.cpp:252
+ public:
+ explicit TemplateClass(int a, T default_value = 0) {}
+};
It might be great to have a test case for:
```
template
class TemplateClass {
public:
explicit
xazax.hun added inline comments.
Comment at: test/clang-tidy/modernize-use-nullptr.cpp:254
+
+ void h(T *default_value = 0) {}
+
Great! Thanks for adding this test. I have the impression we do want to warn
and fix this case however. What do you think?
xazax.hun added inline comments.
Comment at:
clang-tools-extra/trunk/test/clang-tidy/modernize-use-nullptr.cpp:254
+
+ void h(T *default_value = 0) {}
+
Maybe as a separate patch, but I think it might be worth to warn here. WDYT?
(Sorry for the double post,
xazax.hun created this revision.
Herald added a subscriber: mgorny.
This patch adds support for naive cross translational unit analysis.
The aim of this patch is to be minimal to enable the development of the feature
on the top of tree. This patch should be an NFC in case XTUDir is not provided
xazax.hun added a comment.
Guide to run the two pass analysis:
Process
---
These are the steps of XTU analysis:
1. `xtu-build.py` script uses your compilation database and extracts all
necessary information from files compiled. It puts all its generated data into
a folder (.xtu by
xazax.hun added a comment.
In the meantime CheckBeginFunction has been implemented separately. I think you
should "abandon" this revision so it is shown as closed.
https://reviews.llvm.org/D15090
___
cfe-commits mailing list
xazax.hun updated this revision to Diff 93658.
xazax.hun marked 11 inline comments as done.
xazax.hun added a comment.
- Fixed some of the review comments and some other cleanups.
https://reviews.llvm.org/D30691
Files:
include/clang/AST/ASTContext.h
include/clang/AST/Decl.h
xazax.hun added a comment.
In https://reviews.llvm.org/D34512#806505, @klimek wrote:
> Yep, I want Richard's approval for the name. I think he already expressed a
> pro-pulling-out stance.
Great! Ping for the name approval.
https://reviews.llvm.org/D34512
xazax.hun added a comment.
I like the directions of this patch.
In general, I am in favor of explicitly registering the options from user
defined checkers.
But changing a config option will now break the command line compatibility, so
I wonder how do we want to handle this:
- Have a list of
xazax.hun added a comment.
Maybe instead of a separate list, having this information like yes/no column in
a table in the original is more user-friendly.
https://reviews.llvm.org/D36051
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
xazax.hun updated this revision to Diff 109559.
xazax.hun marked 2 inline comments as done.
xazax.hun added a comment.
- Address further review comments.
https://reviews.llvm.org/D34512
Files:
include/clang/Basic/AllDiagnostics.h
include/clang/Basic/CMakeLists.txt
xazax.hun added a comment.
In https://reviews.llvm.org/D34512#829712, @rsmith wrote:
> Organizationally, this seems fine. Carry on :)
Great news! Thank you!
https://reviews.llvm.org/D34512
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
xazax.hun updated this revision to Diff 109552.
xazax.hun marked 3 inline comments as done.
xazax.hun added a comment.
- Addressed review comments.
https://reviews.llvm.org/D34512
Files:
include/clang/Basic/AllDiagnostics.h
include/clang/Basic/CMakeLists.txt
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
LGTM!
Repository:
rL LLVM
https://reviews.llvm.org/D36670
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
xazax.hun added inline comments.
Comment at: test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp:63
+ enum Type {};
+ static char *m_fn1() { char p = (Type)( - m_fn1()); }
+};
Isn't this testcase a bit overcomplicated to demonstrate the issue?
xazax.hun added a comment.
Good idea!
I think some of your extremely helpful responses from the mailing list would
worth archiving in a form of documentation as well.
Also, as far as I can recall, you are also maintaining a pdf which is a very
good reference. In some form, I would love to see
xazax.hun added a comment.
In https://reviews.llvm.org/D34508#791048, @NoQ wrote:
> Currently, we already highlight the last assignments for the "interesting"
> variables, which is implemented through, for example,
> `bugreporter::trackNullOrUndefValue()` - see how various checkers use it.
>
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/MagentaHandleChecker.cpp:483-498
+void MagentaHandleChecker::checkPreStmt(const ReturnStmt *RS,
+CheckerContext ) const {
+ ProgramStateRef State = Ctx.getState();
+
xazax.hun added a comment.
In https://reviews.llvm.org/D34512#836831, @whisperity wrote:
> Apart from those in the in-line comments, I have a question: how safe is this
> library to `Release` builds? I know this is only a submodule dependency for
> the "real deal" in
xazax.hun updated this revision to Diff 110559.
xazax.hun marked 4 inline comments as done.
xazax.hun added a comment.
- Address review comments
https://reviews.llvm.org/D34512
Files:
include/clang/Basic/AllDiagnostics.h
include/clang/Basic/CMakeLists.txt
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:72
+REGISTER_MAP_WITH_PROGRAMSTATE(CtorMap, const MemRegion *, bool)
+REGISTER_MAP_WITH_PROGRAMSTATE(DtorMap, const MemRegion *, bool)
+
I was wondering if there is
xazax.hun added a comment.
At this point, I am a bit wondering about two questions.
- When should something belong to a checker and when should something belong to
the engine? Sometimes we model library aspects in the engine and model language
constructs in checkers.
- What is the checker
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:500
+ // If the type of A - B is the same as the type of A, then use the type
of
+ // B as the type of B - A. Otherwise keep the type of A - B.
+ SymbolRef negSym =
xazax.hun added a comment.
Are you sure this works as intended when e.g.: `$a+2==$b*3`
So on one of the sides, the ops are not additive?
I would like to see some test cases for that.
Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:572
+ lInt = >getRHS();
xazax.hun added a comment.
Thank you! I think we can start to run this check on real world code bases and
evaluate the results.
Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:41
+ void checkPreCall(const CallEvent , CheckerContext ) const;
+ void
xazax.hun added a comment.
In https://reviews.llvm.org/D34512#803724, @klimek wrote:
> Specifically, ping Richard for new top-level lib in clang.
Richard proposed pulling this out into a separate library in the first place.
Do we need his approval for the name? Or we want him to consider if
xazax.hun added a comment.
You are making a pretty good progress!
I think right now there is some code duplication that could be reduced, but
otherwise, the checker starts to look really good.
Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:43
+private:
+ bool
xazax.hun added a comment.
gentle ping
https://reviews.llvm.org/D34512
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
xazax.hun accepted this revision.
xazax.hun added a comment.
One minor nit, otherwise looks good to me.
Comment at: lib/StaticAnalyzer/Core/BugReporter.cpp:
+})) {
+ // Throw-expressions are currently generating sinks during symbolic
+ // execution:
xazax.hun added inline comments.
Comment at: test/clang-tidy/readability-static-accessed-through-instance.cpp:34
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member accessed through
instance [readability-static-accessed-through-instance]
+ // CHECK-FIXES: {{^}}
xazax.hun added a reviewer: aaron.ballman.
xazax.hun added a comment.
Aaron, could you comment on the applicability of this check to C? Thanks in
advance.
https://reviews.llvm.org/D33672
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
xazax.hun accepted this revision.
xazax.hun added reviewers: dcoughlin, NoQ, a.sidorin.
xazax.hun added a comment.
This revision is now accepted and ready to land.
Looks good to me. Did you run it on a codebase to check the results?
https://reviews.llvm.org/D33672
1 - 100 of 1407 matches
Mail list logo
