NoQ added inline comments.
Comment at: test/Analysis/NewDelete-path-notes.cpp:44
// CHECK-NEXT:
-// CHECK-NEXT:line6
+// CHECK-NEXT:line7
// CHECK-NEXT:col3
a.sidorin wrote:
> NoQ wrote:
> > a.sidorin wrote:
> > >
NoQ added a comment.
See also https://reviews.llvm.org/rC326165.
Repository:
rC Clang
https://reviews.llvm.org/D43798
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.
Final results are still being baked on my end, but by putting this on review
i'm trying to say that i believe i managed to fix the biggest pr
NoQ updated this revision to Diff 136114.
NoQ added a comment.
Add one more comment.
https://reviews.llvm.org/D43791
Files:
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
test/Analysis/NewDelete-atomics.cpp
Index: test/Analysis/NewDelete-atomics.cpp
=
NoQ added inline comments.
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h:70
+/// by binding a smaller object within it to a reference.
+bool IsTemporaryLifetimeExtendedViaSubobject = false;
dcoughlin wrote:
> Would you be will
NoQ updated this revision to Diff 136145.
NoQ marked 2 inline comments as done.
NoQ added a comment.
Use `SmallString`.
Add some test for base-class implicit constructors to see how it currently
works (not ideal but good enough).
https://reviews.llvm.org/D43798
Files:
lib/StaticAnalyzer/Che
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp:60
// Generate a report for this bug.
+ std::string Str;
+ llvm::raw_string_ostream OS(Str);
a.sidorin wrote:
> SmallString<128>?
*recalls how it's done* Ok!
NoQ updated this revision to Diff 136151.
NoQ marked an inline comment as done.
NoQ added a comment.
Mention the implicit constructor in the warning message.
https://reviews.llvm.org/D43798
Files:
lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp
test/Analysis/implicit-ctor-undef-v
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.
While landing https://reviews.llvm.org/D43428 as
https://reviews.llvm.org/rC325966, i caused an internal compiler error of an
MSVC 2015 comp
NoQ added a comment.
That's right, most analyzer-oriented tests were added explicitly by me to
`temporaries.cpp` and `lifetime-extension.cpp` in previous patches, so they
didn't need to be changed in this patch. There didn't seem to be many FIXME
tests specific to my work before i started doing
NoQ updated this revision to Diff 136428.
NoQ added a comment.
Don't inline temporary destructors for now (i.e. keep
`c++-temp-dtor-inlining=false`, previously it was by default irrelevant and
arbitrarily set to true). That's because https://reviews.llvm.org/D43791 wasn't
enough to handle all t
NoQ updated this revision to Diff 136434.
NoQ added a comment.
Add the comment.
https://reviews.llvm.org/D43840
Files:
lib/Analysis/CFG.cpp
test/Analysis/cfg-rich-constructors.cpp
Index: test/Analysis/cfg-rich-constructors.cpp
==
NoQ added a comment.
Sorry, busy days>< Done.
Repository:
rL LLVM
https://reviews.llvm.org/D42645
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:517
if (const auto *Array = dyn_cast(DeclRef->getType())) {
- uint64_t ArraySize = BR.getContext().getTypeSize(Array) / 8;
+ auto ArraySize = BR.getContext().getTypeSizeI
NoQ added a comment.
Nice catch, thanks!
Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:1078-1080
+// The use of an operand of type bool with the ++ operators is deprecated
+// but valid untill C++17. And if the operand of the increment operator is
+// of type
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.
Implicit constructor conversions such as `A a = B()` are represented by
surrounding the constructor for `B()` with an `ImplicitCastExpr` of
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Making `CStringChecker` smaller is an accomplishment on its own :)
Repository:
rC Clang
https://reviews.llvm.org/D44075
___
cfe-commits mailing list
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
LGTM, thank you for the fix!
> I not familiar with Objective-C++, can you provide a appropriate test about
> Objective-C++ for me, thank you!
Now that the explicit check and the respective code pat
NoQ created this revision.
NoQ added reviewers: rsmith, dcoughlin, xazax.hun, a.sidorin, george.karpenkov,
szepet.
Herald added subscribers: cfe-commits, rnkovacs.
https://reviews.llvm.org/D42672 has added extra context to construction calls
in the CFG so that the users, such as the analyzer, di
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.
With https://reviews.llvm.org/D44120 we can find the `CXXBindTemporaryExpr` and
`MaterializeTemporaryExpr` that correspond to the temporary o
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:316
}
}
break;
george.karpenkov wrote:
> 5 closing braces is a lot. What about moving the entire block under
> `CK_Complete` into a static function?
I'm already on
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.
NoQ added a dependency: D44124: [analyzer] Support destruction and
lifetime-extension of inlined function return values..
Because @george.kar
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
baloghadamsoftware.
Herald added subscribers: cfe-commits, rnkovacs.
If a conservatively evaluated function returns a C++ object by value, it no
longer returns a conjured symbol. Instead it
NoQ updated this revision to Diff 137116.
NoQ added a comment.
Agreed to rename into `CFGCXXRecordTypedCall`. Because, yeah, `int` is a value
as well.
https://reviews.llvm.org/D44120
Files:
include/clang/Analysis/CFG.h
lib/Analysis/CFG.cpp
lib/StaticAnalyzer/Core/ExprEngine.cpp
lib/Sta
NoQ updated this revision to Diff 137117.
NoQ added a comment.
Fix test comments and make them start from `0` :) Also rebase.
https://reviews.llvm.org/D44124
Files:
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
test/Analysis/lifetime-extension.cpp
Index: test/Analysis/lifetime-extension.cpp
==
NoQ updated this revision to Diff 137119.
NoQ added a comment.
Rebase.
https://reviews.llvm.org/D44129
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/Core/ExprEngine.cpp
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
Index: lib/StaticAnalyzer/Core/ExprE
NoQ updated this revision to Diff 137120.
NoQ added a comment.
Rebase.
https://reviews.llvm.org/D44131
Files:
lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
test/Analysis/explain-svals.cpp
test/Analysis/temporaries.cpp
Index: test/Analysis/temporaries.cpp
===
NoQ created this revision.
NoQ added reviewers: rsmith, doug.gregor, dcoughlin, xazax.hun, a.sidorin,
george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.
In code like
const int &x = A().x;
the destructor for `A()` was not present in the CFG due to two problems in the
p
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.
NoQ added a dependency: D44238: [CFG] Fix automatic destructors when a member
is bound to a reference..
In code like
const int &x = C().x;
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
I think we should land this and celebrate.
@szepet: Ouch, i was sure i already answered this, sorry, dunno where this went.
> So, LoopExit and ScopeExit would be the same but the underlying TriggerS
NoQ added inline comments.
Comment at: lib/Analysis/CFG.cpp:1700
/// way return valid LocalScope object.
LocalScope* CFGBuilder::createOrReuseLocalScope(LocalScope* Scope) {
if (Scope)
NoQ wrote:
> It seems that something has moved asterisks to a weird spot
NoQ added a comment.
In https://reviews.llvm.org/D16403#1031567, @thakis wrote:
> Since this affects analysis-based warnings, have you checked if this patch
> has any effect on compile times?
Just in case - this is under an analyzer-only flag, so the actual warnings are
not affected. I guess
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.
The type of the call expression and the return value type of the function are
not necessarily the same thing. In the attached example, the fu
NoQ updated this revision to Diff 137662.
NoQ added a comment.
Fix typo in the comments.
https://reviews.llvm.org/D44273
Files:
include/clang/Analysis/CFG.h
lib/Analysis/CFG.cpp
test/Analysis/temporaries.cpp
Index: test/Analysis/temporaries.cpp
==
NoQ updated this revision to Diff 137661.
NoQ added a comment.
Same goes for rvalue references. Hmm, we have a fancy helper method for that
exact purpose. But it's not super easy to use, had to sacrifice a sanity check
assertion to use it.
https://reviews.llvm.org/D44273
Files:
include/clan
NoQ updated this revision to Diff 137675.
NoQ added a comment.
Add the tests to a CFG-oriented test file as well, because it doesn't have much
to do with the analyzer.
FIXME: `temp-obj-dtors-cfg-output.cpp` is a mess. It'd be great to interleave
code and expected output, like other CFG tests do
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.
https://reviews.llvm.org/D43791 wasn't quite enough because we often run out of
inlining stack depth limit and for that reason fail to see th
NoQ edited reviewers, added: NoQ; removed: dergachev.a.
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Looks good, thank you!
Repository:
rC Clang
https://reviews.llvm.org/D44250
___
cfe-commits
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs, baloghadamsoftware.
I've been investigating a false positive that had a pointer-type symbol with a
non-zero range which had its range forgott
NoQ updated this revision to Diff 137892.
NoQ added a comment.
Slightly simplify one of the tests.
https://reviews.llvm.org/D44347
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
test/Analysis/symbol-reaper.c
Index: test/Analysis/symbol-reaper.c
NoQ added a comment.
Leak false-negatives that result from bugs in
`Environment::removeDeadBindings()` and
`RegionStoreManager::removeDeadBindings()` are also only appearing due to the
overall zombie symbol problem we have (https://reviews.llvm.org/D18860). The
bugs are in the code that popula
NoQ added a comment.
> Because string length is for now only composed of `CStringChecker`-tagged
> metadata symbols and constants...
Even if this was not the case, it is stil certain that string length is a
`NonLoc`. And as such it is either a plain `nonloc::SymbolVal` that is
unaffected or a
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.
In C++17 copy elision is mandatory for variable and return value constructors
(as long as it doesn't involve type conversion) which results i
NoQ added inline comments.
Comment at: test/Analysis/cfg-rich-constructors.cpp:481-486
+// CXX11:[B1]
+// CXX11-NEXT: 1: [B4.4].~D() (Implicit destructor)
+// CXX11:[B2]
+// CXX11-NEXT: 1: ~temporary_object_expr_with_dtors::D() (Temporary object
destructo
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Just in case: we indeed do not guarantee that `SymbolConjured` corresponds to a
statement; it is, however, not intended, but rather a bug. Consider:
1void clang_analyzer_eval(bool);
2
3
NoQ added a comment.
Sorry, one moment, i'm seeing a few regressions after the previous refactoring
but i didn't look at them closely yet to provide a reproducer. I'll get back to
this.
Repository:
rC Clang
https://reviews.llvm.org/D44557
___
c
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:33
+public:
+ DanglingInternalBufferChecker() : CStrFn("c_str") {}
+
xbolva00 wrote:
> string.data() support?
Yup, there's a lot of API support improvements plan
NoQ updated this revision to Diff 149205.
NoQ marked 3 inline comments as done.
NoQ added a comment.
Refactor everything to address review comments.
https://reviews.llvm.org/D47350
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/Core/ExprEngine.cpp
l
NoQ added a comment.
Refactor everything to address review comments.
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:123
+/// AST nodes.
+class ConstructedObjectKey : public ConstructedObjectKeyImpl {
+ const void *getAnyASTNodePtr() const {
george.karpenko
NoQ updated this revision to Diff 149337.
NoQ added a comment.
Hmm, actually composition looks very pretty if you use the magic word "impl".
https://reviews.llvm.org/D47350
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/Core/ExprEngine.cpp
lib/Stati
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, baloghadamsoftware.
Copy elision occurs when
1. a temporary object (with its corresponding temporary object construction
context) is constr
NoQ added a comment.
We might as well make a directory for z3-specific tests. Eg.,
`z3/bool-bit-width.c`.
Also does this test need to be z3-specific? We would also not like to crash
here without z3.
Repository:
rC Clang
https://reviews.llvm.org/D47617
___
NoQ added a comment.
In https://reviews.llvm.org/D35110#1117401, @baloghadamsoftware wrote:
> Sorry, Artem, but it does not work this way. Even if the symbolic expressions
> are constrained to `[-MAX/4..MAX/4]`, after rearrangement the difference
> still uses the whole range, thus `m>n` becomes
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, baloghadamsoftware.
I don't remember why lifetime extension for temporaries without destructors was
disabled. It's clearly less important an
NoQ updated this revision to Diff 149566.
NoQ added a comment.
Fix comments in the callback test. The test itself is pretty pointless now,
because the behavior that it was supposed to test was a workaround for lack of
lifetime extension support.
https://reviews.llvm.org/D47616
Files:
lib/St
NoQ added a comment.
Whoops wrong patch.
https://reviews.llvm.org/D47616
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
NoQ updated this revision to Diff 149567.
NoQ added a comment.
Fix comments in the callback test. The test itself is pretty pointless now,
because the behavior that it was supposed to test was a workaround for lack of
lifetime extension support.
https://reviews.llvm.org/D47658
Files:
lib/St
NoQ added inline comments.
Comment at: lib/Analysis/CFG.cpp:1320
+auto *MTE = cast(Child);
+findConstructionContexts(ConstructionContextLayer::create(
+ cfg->getBumpVectorContext(), MTE, Layer),
george.karpenkov
NoQ updated this revision to Diff 149568.
NoQ marked 2 inline comments as done.
NoQ added a comment.
Actually let's do a separate context kind. The difference in semantics they
represent is so drastically different that i think it's worth it.
https://reviews.llvm.org/D47616
Files:
include/cl
NoQ updated this revision to Diff 149571.
NoQ added a comment.
Actually let's use lambdas as well.
https://reviews.llvm.org/D47616
Files:
include/clang/Analysis/ConstructionContext.h
lib/Analysis/CFG.cpp
lib/Analysis/ConstructionContext.cpp
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
te
NoQ added inline comments.
Comment at: lib/Analysis/CFG.cpp:1320
+auto *MTE = cast(Child);
+findConstructionContexts(ConstructionContextLayer::create(
+ cfg->getBumpVectorContext(), MTE, Layer),
NoQ wrote:
> geo
NoQ updated this revision to Diff 149586.
NoQ added a comment.
Remove the outdated FIXME comment.
https://reviews.llvm.org/D47658
Files:
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
test/Analysis/lifetime-extension.cpp
test/Analysis/temporaries-callback-order.cpp
Index: test/Analysis/tempor
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, baloghadamsoftware.
`CXXBindTemporaryExpr ` is used for attaching the destructor of the temporary
object to it.
If the object is lifetime-e
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, baloghadamsoftware.
This patch builds on top of https://reviews.llvm.org/D47616 to elide pre-C++17
elidable constructors during analysis. Th
NoQ added a comment.
In https://reviews.llvm.org/D47671#1122994, @george.karpenkov wrote:
> From my understanding, that's just how c++17 is defined, and that's unrelated
> to what compiler implementation is used.
Yeah, but this patch is specifically about supporting the pre-C++17 AST, where
c
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:92-94
+ if (!SymReaper.hasDeadSymbols())
+return;
+
This optimization is in fact incorrect due to an old bug that i didn't yet get
to fixing: D18860. The pr
NoQ accepted this revision.
NoQ added a comment.
Great, thanks!
https://reviews.llvm.org/D47416
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
NoQ added a comment.
In https://reviews.llvm.org/D47671#1127486, @xazax.hun wrote:
> Just for the record, there is a common example where relying on copy elision
> might bite and google do not recommend relying on it for correctness:
> https://abseil.io/tips/120
>
> The main purpose of sharing
NoQ added a comment.
Thanks, this looks very reasonable!
I agree that the syntax pointed out by @george.karpenkov is much cleaner.
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h:30
ProgramStateRef getWidenedLoopState(ProgramStateRef PrevState,
+
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Core/LoopWidening.cpp:89
+new Callback(LCtx, MRMgr, ITraits));
+ Finder.matchAST(ASTCtx);
+
NoQ wrote:
> george.karpenkov wrote:
> > ormris wrote:
> > > george.karpenkov wrote:
> > > >
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Core/LoopWidening.cpp:89
+new Callback(LCtx, MRMgr, ITraits));
+ Finder.matchAST(ASTCtx);
+
ormris wrote:
> ormris wrote:
> > NoQ wrote:
> > > NoQ wrote:
> > > > george.karpenkov wrote
NoQ accepted this revision.
NoQ added a comment.
I'm still curious whether this also works:
void foo() {
const A &x = B();
bar();
}
void bar() {
for (int i = 0; i < 10; ++i) {}
}
Though we can land this patch and deal with this later.
Repository:
rC Clang
https://revi
NoQ added a comment.
Thanks for adding me!
Hmm, i think a matcher-based solution would be pretty limited. This is
definitely your typical all-path data-flow problem because you want to make
sure that you're looking at the //last// assignment to the variable.
For example:
int *m = new int;
NoQ added a comment.
For now the tests that i proposed may in fact accidentally work because as far
as i understand you're updating the "variable contains a value returned by
operator new" flag when you encounter assignment operators. The real problems
will arise when the order of the assignmen
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:32
check::PostCall> {
- CallDescription CStrFn;
+ const llvm::SmallVector CStrFnFamily = {
+{"std::basic_string::c_str"
NoQ updated this revision to Diff 151276.
NoQ marked an inline comment as done.
NoQ added a comment.
Herald added a subscriber: mikhail.ramalho.
Fxd.
https://reviews.llvm.org/D47658
Files:
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
test/Analysis/lifetime-extension.cpp
test/Analysis/tempora
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:234
+ SVal V = UnknownVal();
+ if (MTE) {
+if (MTE->getStorageDuration() != SD_FullExpression) {
MTC wrote:
> An unrelated question. I want to know, what considerat
NoQ added inline comments.
Comment at: include/clang/Analysis/ConstructionContext.h:351
+
+ explicit SimpleTemporaryObjectConstructionContext(
+ const CXXBindTemporaryExpr *BTE, const MaterializeTemporaryExpr *MTE)
MTC wrote:
> MTC wrote:
> > `explicit` use
NoQ updated this revision to Diff 151278.
NoQ added a comment.
Herald added a subscriber: mikhail.ramalho.
Rebase.
https://reviews.llvm.org/D47667
Files:
lib/Analysis/ConstructionContext.cpp
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
test/Analysis/auto-obj-dtors-cfg-output.cpp
test/Analy
NoQ updated this revision to Diff 151277.
NoQ added a comment.
Add a flag to disable copy elision. It's convenient to have such flag in the
CFG because this way all clients will be able to transparently handle it. When
the option is turned off, elided construction contexts will be replaced with
NoQ updated this revision to Diff 151279.
NoQ added a comment.
I added an option to disable copy elision on CFG side in
https://reviews.llvm.org/D47616. The analyzer makes use of it automagically:
elided constructors are replaced with temporary constructors in the CFG and the
old behavior is re
NoQ added a comment.
> P.S. It seems that one of my currently-on-review patches has introduced a
> performance regression, i'm investigating it.
Never mind, that was an old version of the patch, i.e. i accidentally fixed
that regression before uploading the patch to phabricator.
https://revie
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, mikhail.ramalho, baloghadamsoftware,
eraman.
`ExprWithCleanups` that cleans up function arguments (or any other stuff) at
the end of the fu
NoQ added a comment.
This is supposed to suppress a few Inlined-Defensive-Checks-related false
positives that accidentally spiked up during my testing of copy elision.
Repository:
rC Clang
https://reviews.llvm.org/D48204
___
cfe-commits mailing
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, mikhail.ramalho, baloghadamsoftware.
`nonloc::SymbolVal` that contains a pointer-type or reference-type symbol is
ill-formed; our code isn't
NoQ added inline comments.
Comment at: test/Analysis/inlining/inline-defensive-checks.cpp:93
+S *conjure();
+S *get_conjured(S _) {
+ S *s = conjure();
george.karpenkov wrote:
> what is the argument doing?
Causing cleanups (:
Repository:
rC Clang
https://re
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, mikhail.ramalho, baloghadamsoftware.
The canonical way to represent the result of casting `&SymRegion{$x}` to `bool`
is `($x != 0)`, not `$x
NoQ added a comment.
> In the iterator checkers we do not know anything about the rearranged
> expressions, it has no access to the sum/difference, the whole purpose of
> your proposal was to put in into the infrastructure.
It wasn't. The purpose was merely to move (de-duplicate) the code that
NoQ added a comment.
I still don't think i fully understand your concern? Could you provide an
example and point out what exactly goes wrong?
https://reviews.llvm.org/D35110
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.
NoQ accepted this revision.
NoQ added a comment.
Ok, let's land this one and see how it goes! I'm looking forward to seeing the
follow-up patches.
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:359-372
+ // Checking bases.
+ const auto *CXXRD = dyn_ca
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, mikhail.ramalho, baloghadamsoftware.
This is similar to https://reviews.llvm.org/D45650. Constructors of
pass-by-value arguments aren't simi
NoQ updated this revision to Diff 151584.
NoQ added a comment.
Whoops, that was an old patch.
https://reviews.llvm.org/D48249
Files:
lib/Analysis/CFG.cpp
lib/Analysis/ConstructionContext.cpp
test/Analysis/cfg-rich-constructors.cpp
test/Analysis/cfg-rich-constructors.mm
test/Analysis/t
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:696
void ento::registerUninitializedObjectChecker(CheckerManager &Mgr) {
auto Chk = Mgr.registerChecker();
Chk->IsPedantic = Mgr.getAnalyzerOptions().getBooleanOption(
---
NoQ added a comment.
Also, great, and can i has tests?^^
Like a simple code snippet with two `// RUN: ... -analyzer-output=text` lines
and different expected-warnings/notes under `#if`s.
Repository:
rC Clang
https://reviews.llvm.org/D48285
___
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, mikhail.ramalho, baloghadamsoftware.
`const` methods shouldn't invalidate the object unless mutable fields kick in.
They sometimes were inva
NoQ added a comment.
Aha, yeah, i see. It only invalidates the current stack frame, and additionally
it's impossible to bring the reference into the current stack frame by
reference, because, well, it's already a reference and you can't mutate a
reference.
Ok then!
Repository:
rC Clang
ht
NoQ added a comment.
Ok, code makes sense to me now!
I think we still need a few new tests to cover the corner cases.
In https://reviews.llvm.org/D35110#1135306, @baloghadamsoftware wrote:
> I added extra assertion into the test for the difference. Interestingly, it
> also works if I assert `n
NoQ added a comment.
Well, i guess that's just one of those mildly infuriating aspects of the AST
and/or the standard :)
> If references are resolved, why are pointers not?
It's kinda understandable. References are simple aliases to glvalues. When you
use a reference, you simply get *the* orig
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/AllocationState.h:23-25
+std::unique_ptr
+getDanglingBufferBRVisitor(SymbolRef Sym);
+
I think we should start commenting this stuff up. Like, "This function provides
an additional visitor that a
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Looks good tho!
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:63-64
+ RawPtrMapTy Map = State->get();
+ for (const auto Entry : Map) {
+
401 - 500 of 3498 matches
Mail list logo
