Szelethus updated this revision to Diff 172459.
Szelethus marked an inline comment as done.
Szelethus added a comment.
- Fixes according to @xazax.hun's observations
Thanks! I'll commit when I have time watch buildbots.
https://reviews.llvm.org/D52794
Files:
lib/StaticAnalyzer/Core/PlistDiag
Szelethus added a comment.
In https://reviews.llvm.org/D53974#1286434, @ztamas wrote:
> In https://reviews.llvm.org/D53974#1285930, @Szelethus wrote:
>
> > In https://reviews.llvm.org/D53974#1283759, @ZaMaZaN4iK wrote:
> >
> > > Hmm, i thought Clang has some warning for this, but I was wrong... D
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:96
+ // Get the value of the expression to cast.
+ const auto ValueToCastOptional =
+ C.getSVal(CE->getSubExpr()).getAs();
ZaMaZaN4iK wrote:
> lebedev.ri
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:96
+ // Get the value of the expression to cast.
+ const auto ValueToCastOptional =
+ C.getSVal(CE->getSubExpr()).getAs();
Szelethus wrote:
> ZaMaZaN4iK
This revision was automatically updated to reflect the committed changes.
Closed by commit rL346095: [analyzer][PlistMacroExpansion] Part 2.: Retrieving
the macro name and… (authored by Szelethus, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.
Szelethus reopened this revision.
Szelethus added a comment.
This revision is now accepted and ready to land.
Here's to losing a couple more handfuls of hair, tests break on many platforms,
so I reverted it.
Repository:
rL LLVM
https://reviews.llvm.org/D52794
_
Szelethus updated this revision to Diff 172533.
Szelethus added a comment.
- Came to the shocking realization that `Hidden` is also an unused property, so
I removed that too.
- Added comments to `CheckerBase.td`.
- Added missing ` // end "packagename"` comments
Now that `CheckerBase.td` isn't th
Szelethus updated this revision to Diff 172534.
https://reviews.llvm.org/D53995
Files:
include/clang/Driver/CC1Options.td
include/clang/StaticAnalyzer/Checkers/CheckerBase.td
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/ClangCheckers.cpp
lib/StaticAnalyz
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Core/SarifDiagnostics.cpp:237-242
+#define GET_CHECKERS
+#define CHECKER(FULLNAME, CLASS, CXXFILE, HELPTEXT, GROUPINDEX, HIDDEN)
\
+ .Case(FULLNAME, HELPTEXT)
+#include "clang/StaticAnalyzer/Checkers/Checker
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Core/SarifDiagnostics.cpp:237-242
+#define GET_CHECKERS
+#define CHECKER(FULLNAME, CLASS, CXXFILE, HELPTEXT, GROUPINDEX, HIDDEN)
\
+ .Case(FULLNAME, HELPTEXT)
+#include "clang/StaticAnalyzer/Checkers/Checker
Szelethus closed this revision.
Szelethus added a comment.
Woohoo, it seems to be fine! ^-^
I thought the evaluation order in braced initializer lists are from left to
right, but apparently not. Certainly not on windows.
Repository:
rL LLVM
https://reviews.llvm.org/D52794
___
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL346113: [analyzer] Restrict AnalyzerOptions' interface
so that non-checker objects have… (authored by Szelethus, committed
Szelethus marked 6 inline comments as done.
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:677
+/// need to expanded further when it is nested inside another macro.
+class MacroArgMap : public std::map {
+public:
NoQ wro
Szelethus added a comment.
I read through your patch multiple times, have read your mail on cfe-dev, and I
still couldn't find eve minor nits, well done!
However, both in the mail, and in this discussion, there are a lot of
invaluable information about the inner workings of the analyzer, that I
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Core/SarifDiagnostics.cpp:237-242
+#define GET_CHECKERS
+#define CHECKER(FULLNAME, CLASS, CXXFILE, HELPTEXT, GROUPINDEX, HIDDEN)
\
+ .Case(FULLNAME, HELPTEXT)
+#include "clang/StaticAnalyzer/Checkers/Checker
Szelethus added a comment.
Thanks for summarizing the problem so well in the summary! I should start doing
that too.
Comment at: lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:343-346
+ // There should have originally been a symbol. If it was a concrete value
+ // to beg
Szelethus added a comment.
Please reupload with full context.
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2369
ProgramStateRef state = C.getState();
- RegionStateTy RS = state->get();
+ RegionStateTy OldRS = state->get();
RegionStateTy::Factory &F = state-
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2369
ProgramStateRef state = C.getState();
- RegionStateTy RS = state->get();
+ RegionStateTy OldRS = state->get();
RegionStateTy::Factory &F = state->get_context();
---
Szelethus added a comment.
In https://reviews.llvm.org/D52790#1285039, @NoQ wrote:
> Looks great, let's land?
I'll probably land it after part 5, in order to ease on rebasing.
> Not sure if i already asked - am i understanding correctly that this is a
> "poor-man's" support for macro expansio
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2369
ProgramStateRef state = C.getState();
- RegionStateTy RS = state->get();
+ RegionStateTy OldRS = state->get();
RegionStateTy::Factory &F = state->get_context();
---
Szelethus added a comment.
I have no other objections, looks great!
Comment at: test/Analysis/nullability.mm:3-4
// RUN: %clang_analyze_cc1 -fblocks
-analyzer-checker=core,nullability.NullPassedToNonnull,nullability.NullReturnedFromNonnull,nullability.NullablePassedToNonnull,
Szelethus added inline comments.
Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:20
+
+static const char LoopName[] = "forLoopName";
+static const char loopVarName[] = "loopVar";
JonasToth wrote:
> JonasToth wrote:
> > ztamas wrote:
> > > JonasToth
Szelethus added a comment.
We probably should also add an entry about some code conventions we use here,
for example, the use of `auto` was debated recently when used with
`SVal::getAs`, maybe something like this:
- As an LLVM subproject, the code in the Static Analyzer should too follow the
h
Szelethus added a comment.
Also, context is missing :)
https://reviews.llvm.org/D52984
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Szelethus marked 2 inline comments as done.
Szelethus added inline comments.
Comment at: test/Analysis/analyzer-config.c:4-12
void bar() {}
void foo() {
// Call bar 33 times so max-times-inline-large is met and
// min-blocks-for-inline-large is checked
for (int i = 0;
Szelethus updated this revision to Diff 172856.
Szelethus set the repository for this revision to rC Clang.
Szelethus added a comment.
- Moved initializer functions to `CompilerInvocation.cpp`, like every other
Options-like class.
- The fields for options are no longer `Optional`
- Fixed the test
Szelethus added inline comments.
Comment at: test/Analysis/conversion.c:195
+return r;
+ } else if (b>1<<25) {
+float f = b; // expected-warning {{Loss of precision}}
NoQ wrote:
> Szelethus wrote:
> > This too -- how about running clang-format on this fi
Szelethus added a comment.
Herald added a subscriber: donat.nagy.
Bad news, this approach doesn't work too well, and here's why:
https://github.com/llvm-mirror/clang/blob/master/lib/StaticAnalyzer/Core/CheckerRegistry.cpp#L115
void CheckerRegistry::initializeManager(CheckerManager &checkerMgr,
Szelethus added a comment.
I personally find the registration process very error-prone, the current
infrastructure around it makes it super easy to mess up in a way that won't be
detected for months, so we probably need a better approach altogether. It
shouldn't be possible to do any harm withi
Szelethus added a comment.
In https://reviews.llvm.org/D52984#1269850, @NoQ wrote:
> - Warning and note messages should be clear and easy to understand, even if a
> bit long.
> - Messages should start with a capital letter (unlike Clang warnings!) and
> should not end with `.`.
> - Articles
Szelethus added a comment.
Ping
Repository:
rC Clang
https://reviews.llvm.org/D53995
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Szelethus marked an inline comment as done.
Szelethus added a comment.
Thanks! This patch was the last for a while directly related to non-checker
config options, I'm currently stuck on them as I unearthed countless bugs that
I have to fix beforehand. (Did you know that clang unit tests do not r
Szelethus added a comment.
> I agree that it's aesthetically satisfying, but it is (1) inconvenient to
> write because that's the whole new syntax you need to memorize, i.e. all
> those <> and semicolons and (2) not cooperating when i want to look up
> checker name by source file (have to scan
Szelethus added a comment.
Ping, @NoQ, do you insist on a global set?
https://reviews.llvm.org/D51531
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Szelethus added inline comments.
Comment at: www/analyzer/checker_dev_manual.html:719
+
+User facing documentation is important for adoption! Make sure the check
list updated
+at the homepage of the analyzer. Also ensure that the description is good
quality in
-
Szelethus added a comment.
In https://reviews.llvm.org/D52984#1294258, @NoQ wrote:
> In https://reviews.llvm.org/D52984#1294233, @aaron.ballman wrote:
>
> > Personally, I think it's detrimental to the community for subprojects to
> > come up with their own coding guidelines. My preference is for
Szelethus added a comment.
Mainly my problem is that checker files can be large and hard to maneuver, some
forward declare and document everything, some are a big bowl of spaghetti, and
I think having a checklist of how it should be organized to keep uniformity and
readability would be great.
Szelethus added inline comments.
Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:19-22
+struct Entry {
+ SourceLocation Loc;
+ std::string MacroName;
+};
This is a way too general name in my opinion. Either include comments that
describe it,
Szelethus added inline comments.
Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:19-22
+struct Entry {
+ SourceLocation Loc;
+ std::string MacroName;
+};
vmiklos wrote:
> Szelethus wrote:
> > This is a way too general name in my opinion. Eithe
Szelethus created this revision.
Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, rnkovacs, MTC.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho,
a.sidorin, szepet, whisperity.
TL;DR: `CheckerOptInfo` feels very much out of place in
`CheckerRegistration.cpp
Szelethus added inline comments.
Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:56-59
+StringRef SourceText =
+Lexer::getSourceText(CharSourceRange::getTokenRange(ConditionRange),
+ PP.getSourceManager(), PP.getLangOpts()
Szelethus added inline comments.
Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:56-59
+StringRef SourceText =
+Lexer::getSourceText(CharSourceRange::getTokenRange(ConditionRange),
+ PP.getSourceManager(), PP.getLangOpts()
Szelethus created this revision.
Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, MTC.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho,
a.sidorin, rnkovacs, szepet, whisperity.
This bugged me for a long time, so it's time to put an end to it:
`collectChecke
Szelethus updated this revision to Diff 173584.
Szelethus added a comment.
Also, remove `diags::note_suggest_disabling_all_checkers`.
https://reviews.llvm.org/D54401
Files:
include/clang/Basic/DiagnosticFrontendKinds.td
include/clang/StaticAnalyzer/Core/CheckerRegistry.h
include/clang/Sta
Szelethus accepted this revision.
Szelethus added a comment.
This revision is now accepted and ready to land.
LGTM! Let's continue the conversation about coding standards somewhere else.
Can you please mark inlines as done?
https://reviews.llvm.org/D52984
Szelethus added a comment.
Thanks for taking a look! ^-^
Comment at: lib/StaticAnalyzer/Core/CheckerRegistry.cpp:51
-/// Collects the checkers for the supplied \p opt option into \p collected.
-static void collectCheckers(const CheckerRegistry::CheckerInfoList &checkers,
-
Szelethus added a subscriber: xazax.hun.
Szelethus added a comment.
I'll read through this as soon as possible, but a HUGE thank you for this. This
is what the Static Analyzer desperately needed for years. This will trivialize
documenting, so conversations about the inner workings of the analyze
This revision was automatically updated to reflect the committed changes.
Closed by commit rL346680: [analyzer] Drastically simplify the tblgen files
used for checkers (authored by Szelethus, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.
Szelethus updated this revision to Diff 173717.
Szelethus set the repository for this revision to rC Clang.
Szelethus added a comment.
Might as well make it a method.
Repository:
rC Clang
https://reviews.llvm.org/D54401
Files:
include/clang/Basic/DiagnosticFrontendKinds.td
include/clang/
Szelethus created this revision.
Szelethus added reviewers: george.karpenkov, NoQ, xazax.hun.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho,
a.sidorin, rnkovacs, szepet, whisperity, mgorny.
Herald added a reviewer: teemperor.
`ClangCheckerRegistry` is a very non-obvio
Szelethus added a comment.
In https://reviews.llvm.org/D54429#1295838, @george.karpenkov wrote:
> What do you propose we should do with other pages on the existing website?
> (OpenProjects / etc.)
I think we should just move everything here, and forget about the old site, as
there clearly isn
Szelethus added a comment.
In https://reviews.llvm.org/D54401#1295832, @george.karpenkov wrote:
> > Also, remove diags::note_suggest_disabling_all_checkers.
>
> Isn't that a separate revision? Given that removing existing options is often
> questionable, I'd much rather see this patch separated.
Szelethus created this revision.
Szelethus added reviewers: george.karpenkov, NoQ, xazax.hun.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho,
a.sidorin, rnkovacs, szepet, whisperity.
Now that `CheckerRegistry` lies in `Frontend`, we can finally eliminate
`ClangChecker
Szelethus created this revision.
Szelethus added reviewers: george.karpenkov, NoQ, xazax.hun, rnkovacs.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, jfb,
mikhail.ramalho, a.sidorin, mgrang, szepet, whisperity, mgorny.
This patch solves the problem I explained in an earlier revision[
Szelethus added inline comments.
Comment at: include/clang/Basic/DiagnosticFrontendKinds.td:177
"no analyzer checkers are associated with '%0'">;
-def note_suggest_disabling_all_checkers : Note<
-"use -analyzer-disable-all-checks to disable all static analyzer
checkers"
Szelethus updated this revision to Diff 173745.
Szelethus edited the summary of this revision.
Szelethus added a comment.
Restored the discussed note.
https://reviews.llvm.org/D54401
Files:
include/clang/StaticAnalyzer/Core/CheckerRegistry.h
include/clang/StaticAnalyzer/Frontend/FrontendAct
Szelethus added a comment.
In https://reviews.llvm.org/D54438#1296155, @NoQ wrote:
> Can we reduce this patch to simply introducing the dependency item in
> `Checkers.td` and using it in, like, one place (eg.,
> `MallocChecker`/`CStringChecker` inter-op) and discuss the rest of the stuff
> lat
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Core/SarifDiagnostics.cpp:237-242
+#define GET_CHECKERS
+#define CHECKER(FULLNAME, CLASS, CXXFILE, HELPTEXT, GROUPINDEX, HIDDEN)
\
+ .Case(FULLNAME, HELPTEXT)
+#include "clang/StaticAnalyzer/Checkers/Checker
Szelethus added a comment.
In https://reviews.llvm.org/D54438#1296239, @NoQ wrote:
> Mm, i don't understand. I mean, what prevents you from cutting it off even
> earlier and completely omitting that part of the patch? Somebody will get to
> this later in order to see how exactly does the separa
Szelethus added a comment.
Actually, no. The main problem here is that `InnerPointerChecker` doesn't only
want to register `MallocBase`, it needs to modify the checker object. In any
case, either `MallocChecker.cpp` needs the definition of `InnerPointerChecker`,
or vice versa. Sure, I could sep
Szelethus added a comment.
Herald added a subscriber: baloghadamsoftware.
Allow me to disagree again -- the reason why `CheckerRegistry` and the entire
checker registration process is a mess is that suboptimal solutions were added
instead of making the system do well on the long term. This is co
Szelethus planned changes to this revision.
Szelethus added a comment.
Here's how I'm planning to approach this issue:
- Split `MallocChecker` and `CStringChecker` up properly, introduce
`MallocCheckerBase` and `CStringCheckerBase`. I'm a little unsure about how
I'll do this, but I'll share my
Szelethus added a comment.
Hmmm, shouldn't we add this to `MemRegion`'s interface instead?
Repository:
rC Clang
https://reviews.llvm.org/D54466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo
Szelethus added a comment.
Thanks you so much! One of the reasons why I love working on this is because
the great feedback I get. I don't wanna seem annoyingly grateful, but it's been
a very enjoyable experience so far.
> This should not cause a warnings because it should be fine to compile
>
Szelethus added a comment.
Herald added subscribers: gamesh411, baloghadamsoftware.
Unfortunately, I found //yet another// corner case I didn't cover: if the macro
arguments themselves are macros. I already fixed it, but it raises the question
that what other things I may have missed? I genuinel
Szelethus added a comment.
Herald added a subscriber: gamesh411.
Unfortunately, I found //yet another// corner case I didn't cover: if the macro
arguments themselves are macros. I already fixed it, but it raises the question
that what other things I may have missed? I genuinely dislike this proj
Szelethus added a comment.
Herald added subscribers: gamesh411, baloghadamsoftware.
@george.karpenkov Matching macros is a very non-trivial job, how would you feel
if we shipped this patch as-is, and maybe leave a TODO about adding macro
`assert`s down the line?
https://reviews.llvm.org/D51866
Szelethus marked 5 inline comments as done.
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:982
+ int ParenthesesDepth = 1;
+ while (ParenthesesDepth != 0) {
++It;
xazax.hun wrote:
> Is the misspelling already comm
Szelethus accepted this revision.
Szelethus added a comment.
LGTM, both the concept and the patch! I have read your followup patches up to
part 4, I'll leave some thoughts there.
Repository:
rC Clang
https://reviews.llvm.org/D54556
___
cfe-commi
Szelethus added a comment.
I think we should either remove the non-default functionality (which wouldn't
be ideal), or emphasise somewhere (open projects?) that there is still work to
be done, but leaving it to be forgotten and essentially making it an extra
maintenance work would be, in my opt
Szelethus accepted this revision.
Szelethus added a comment.
This revision is now accepted and ready to land.
Awesome! :D
Comment at: lib/StaticAnalyzer/Checkers/MoveChecker.cpp:385-386
+ }
+ // Provide the caller with the classification of the object
+ // we've obtained her
Szelethus accepted this revision.
Szelethus added a comment.
This revision is now accepted and ready to land.
How about `std::list::splice`?
Repository:
rC Clang
https://reviews.llvm.org/D54563
___
cfe-commits mailing list
cfe-commits@lists.llvm.
Szelethus added a comment.
Alpha checkers, at least to developers, are clearly stating "Please finish
me!", but a non-alpha, enabled-by-default checker with a flag that you'd never
know about unless you looked at the source code (at least up until I fix these)
sounds like it'll be forgotten lik
Szelethus added a comment.
Herald added a subscriber: gamesh411.
In https://reviews.llvm.org/D54438#1297602, @NoQ wrote:
> Hmm, makes sense. Maybe `static bool BlahBlahChecker::isApplicable(const
> LangOpts &LO)` or something like that.
>
> Btw, maybe instead of default constructor and `->setup(
Szelethus added a comment.
Herald added a subscriber: gamesh411.
In https://reviews.llvm.org/D52730#1289989, @donat.nagy wrote:
> Could someone with commit rights commit this patch (if it is acceptable)? I
> don't have commit rights myself.
I'll do the honors. Thanks!
Repository:
rC Clang
This revision was automatically updated to reflect the committed changes.
Closed by commit rC347006: [analyzer] ConversionChecker: handle floating point
(authored by Szelethus, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52730?vs=172922&id=174306#toc
Repository:
rC Cla
Szelethus added a comment.
In https://reviews.llvm.org/D54557#1300581, @NoQ wrote:
> In https://reviews.llvm.org/D54557#1299899, @Szelethus wrote:
>
> > Whenever I compile `Rtags`, I get a bunch of move related warnings, I'm
> > curious how this patch behaves on that project. I'll take a look.
>
Szelethus added a comment.
> In https://reviews.llvm.org/D54557#1300653, @NoQ wrote:
>
>> In https://reviews.llvm.org/D54557#1299899, @Szelethus wrote:
>>
>> > I think we should either remove the non-default functionality (which
>> > wouldn't be ideal), or emphasise somewhere (open projects?) th
Szelethus added a comment.
In https://reviews.llvm.org/D54557#1301869, @NoQ wrote:
> In https://reviews.llvm.org/D54557#1300671, @Szelethus wrote:
>
> > Nice catch, would you like to make an issue on their project or shall I?
>
>
> I guess it can be an outright pull request :) I'll see if i have
Szelethus updated this revision to Diff 174495.
Szelethus retitled this revision from "[analyzer] Emit a warning for unknown
-analyzer-config options" to "[analyzer] Emit an error for invalid
-analyzer-config inputs".
Szelethus set the repository for this revision to rC Clang.
Szelethus added a c
Szelethus added a comment.
Herald added subscribers: gamesh411, baloghadamsoftware.
In https://reviews.llvm.org/D51531#1296256, @NoQ wrote:
> In https://reviews.llvm.org/D51531#1286110, @Szelethus wrote:
>
> > Oh, and the reason why I think it would add a lot of complication: since
> > only `Exp
This revision was automatically updated to reflect the committed changes.
Closed by commit rC347153: [analyzer][UninitializedObjectChecker] Uninit
regions are only reported once (authored by Szelethus, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D51531?vs=163542&id=174530#
Szelethus abandoned this revision.
Szelethus added a comment.
Herald added subscribers: gamesh411, baloghadamsoftware.
In https://reviews.llvm.org/D53069#1274554, @george.karpenkov wrote:
> If we want to be serious about this page, it really has to be auto-generated
> (like clang-tidy one), but
This revision was automatically updated to reflect the committed changes.
Closed by commit rL347157: [analyzer][NFC] Move CheckerOptInfo to
CheckerRegistry.cpp, and make it local (authored by Szelethus, committed by ).
Herald added subscribers: llvm-commits, gamesh411, baloghadamsoftware.
Changed
Szelethus added a comment.
Herald added subscribers: gamesh411, baloghadamsoftware.
Ping
https://reviews.llvm.org/D54401
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/MoveChecker.cpp:385-386
+ }
+ // Provide the caller with the classification of the object
+ // we've obtained here accidentally, for later use.
+ return OK;
NoQ wrote:
> Szelethus wrote:
Szelethus added a comment.
In the meanwhile we managed to figure out where the problem lays, so if you're
interested, I'm going to document it here.
The problem this patch attempts to solve is that `ConditionBRVisitor` adds
event pieces to the bugpath when the state has changed from the previou
Szelethus added a comment.
Ping, @xazax.hun, any objections?
https://reviews.llvm.org/D52795
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Szelethus added a comment.
Herald added subscribers: gamesh411, baloghadamsoftware.
Polite ping :)
Repository:
rC Clang
https://reviews.llvm.org/D54436
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/
Szelethus created this revision.
Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, rnkovacs.
Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy,
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, whisperity.
I'm in the process of splitting this checker into multi
Szelethus added a comment.
I also have some very neat ideas how the split up should go, but I'd like to
mature the idea in my head for just a bit longer.
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:24-27
+// It also has a boolean "Optimistic" checker option
Szelethus added inline comments.
Comment at: lib/AST/DeclBase.cpp:1469
assert(Pos != Map->end() && "no lookup entry for decl");
-if (Pos->second.getAsVector() || Pos->second.getAsDecl() == ND)
+// Remove the decl only if it is contained.
+if ((Pos
Szelethus added a comment.
In https://reviews.llvm.org/D54466#1305305, @baloghadamsoftware wrote:
> In https://reviews.llvm.org/D54466#1297887, @NoQ wrote:
>
> > > Hmmm, shouldn't we add this to `MemRegion`'s interface instead?
>
>
> This:
>
> > I wouldn't insist, but this does indeed sound usefu
Szelethus created this revision.
Szelethus added reviewers: george.karpenkov, NoQ, xazax.hun, rnkovacs.
Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy,
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, whisperity.
Repository:
rC Clang
https://reviews.llvm.org/D54834
Szelethus added a comment.
In https://reviews.llvm.org/D54563#1301893, @NoQ wrote:
> In https://reviews.llvm.org/D54563#1299918, @Szelethus wrote:
>
> > How about `std::list::splice`?
>
>
> Hmm, you mean that it leaves its argument empty? Interesting, i guess we may
> need a separate facility fo
Szelethus added a comment.
In https://reviews.llvm.org/D18860#1297742, @NoQ wrote:
> Nope, unit tests aren't quite useful here. I can't unit-test the behavior of
> API that i'm about to //remove//... right?
Hmmm, can't really argue with that, can I? :D
https://reviews.llvm.org/D18860
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2383-2384
+ if (RS == OldRS)
+return;
+
Hmmm, I guess we return here because if `RegionState` is unchanged, so should
be `ReallocPairs` and `FreeReturnValue`, corre
Szelethus added a comment.
Let's also have the link to your most recent mail about this issue here:
http://lists.llvm.org/pipermail/cfe-dev/2018-November/060038.html
I re-read the mailing list conversation from 2016, @szepet's
`MisusedMoveChecker` patch, so I have a better graps on whats happen
Szelethus added a comment.
And thank you for helping me along the way! :D
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:342
+ DefaultBool IsOptimistic;
+ MemFunctionInfoTy MemFunctionInfo;
public:
MTC wrote:
> I can't say that abstracting `MemFun
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:342
+ DefaultBool IsOptimistic;
+ MemFunctionInfoTy MemFunctionInfo;
public:
Szelethus wrote:
> Szelethus wrote:
> > MTC wrote:
> > > I can't say that abstracting `MemFu
301 - 400 of 548 matches
Mail list logo
