NoQ added a comment.
Last time i was running on WebKit; i already lost my results, so i'd try to
reproduce the results on the fixed checker and follow up. Apart from
https://reviews.llvm.org/D31538, i've seen a few cases where a method was safe
to be called on a moved-from object (which led me
NoQ created this revision.
Herald added a subscriber: szepet.
In https://bugs.llvm.org/show_bug.cgi?id=34460 CStringChecker tries to
`evalCast()` a memory region from `void *` to `char *` for the purposes of
modeling `mempcpy()`. The memory region turned out to be an element region of
type `uns
NoQ added inline comments.
Comment at: test/Analysis/casts.c:134-139
+ clang_analyzer_eval(y1 == y2); // expected-warning{{TRUE}}
+
+ // FIXME: should be FALSE (i.e. equal pointers).
+ clang_analyzer_eval(y1 - y2); // expected-warning{{UNKNOWN}}
+ // FIXME: should be TRUE (i.
NoQ created this revision.
Herald added a subscriber: szepet.
In https://reviews.llvm.org/D38358, we ended up believing that reading the
first byte of the void pointer is not the intended behavior of
`ProgramState::getSVal(Loc)`. Hence the fix.
Additionally, allow specifying the type in the `Pr
NoQ updated this revision to Diff 118625.
NoQ added a comment.
Herald added a subscriber: szepet.
Because i didn't get back to this in a while, and similar crashes keep coming,
i decided to leave this refactoring as a FIXME.
https://reviews.llvm.org/D23963
Files:
lib/StaticAnalyzer/Core/Regi
NoQ added a comment.
Ideas behind both hashing change and new testing mechanism look great to me.
I think it would be great to split them into two different patches, to be able
to easily see how the change in the hashing affects the tests (and maybe revert
easily if something goes wrong).
==
NoQ added a comment.
The other way round, i guess. I like the test change, it's easier to
understand, so it's better to have it before starting to understand :)
https://reviews.llvm.org/D38728
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
NoQ created this revision.
Herald added subscribers: szepet, xazax.hun.
CoreFoundation users often look for a "safe" wrapper around `CFRetain` and
`CFRelease` that would gracefully accept (and ignore) a null CF reference.
Users are trying to annotate it with `CF_RETURNS_RETAINED`, which misleads
NoQ added a comment.
Neat! Tests?
https://reviews.llvm.org/D38921
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Great. Probably one more reason to turn all `Loc`s into regions.
Repository:
rL LLVM
https://reviews.llvm.org/D39174
___
cfe-commits mailing list
cf
NoQ added a comment.
In https://reviews.llvm.org/D31868#904814, @MTC wrote:
> > One of the possible improvements for future work here would be to actually
> > bind the second argument value to the buffer instead of just invalidating
> > it. Like, after `memset(buf, 0, sizeof(buf))` the analyzer
NoQ accepted this revision.
NoQ added inline comments.
This revision is now accepted and ready to land.
Comment at:
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp:187-191
// If FR is a pointer pointing to a non-primitive type.
if (Optional RecordV
NoQ updated this revision to Diff 162755.
NoQ added a comment.
Address comments.
https://reviews.llvm.org/D50855
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/Core/ExprEngine.cpp
test/Analysis/temporaries.cpp
Index: test/Analysis/temporaries.cpp
=
NoQ added inline comments.
Comment at: test/Analysis/cxx-uninitialized-object-inheritance.cpp:802
+struct DynTBase2 {
+ int x; // expected-note{{uninitialized field 'static_cast(this->bptr)->DynTBase2::x'}}
+};
Mmm, what's the value of casting to derived type an
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Yup, looks correct to me!
Comment at:
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:448-449
Loc ThisLoc = Context.getSValBuilder().getCXXThis
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, Szelethus, mikhail.ramalho,
baloghadamsoftware.
Return value of `dyn_cast_or_null` should be checked before use. Otherwise we
may put a nul
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, Szelethus, mikhail.ramalho,
baloghadamsoftware.
When a checker maintains a program state trait that isn't a simple
list/set/map, but is a c
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, Szelethus, mikhail.ramalho,
baloghadamsoftware.
`CallDescription` constructor now accepts an `ArrayRef`, instead
of two different construct
NoQ added inline comments.
Comment at: test/Analysis/cxx-uninitialized-object-inheritance.cpp:802
+struct DynTBase2 {
+ int x; // expected-note{{uninitialized field 'static_cast(this->bptr)->DynTBase2::x'}}
+};
Szelethus wrote:
> NoQ wrote:
> > Mmm, what's the v
NoQ accepted this revision.
NoQ added a comment.
Let's commit then?
https://reviews.llvm.org/D50892
___
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/D51393#1217644, @aprantl wrote:
> Just for consideration: The raw pointers in dumps are sometimes useful while
> in a debugger session, because you can cast a pointer and dump the object in
> the debugger.
Yup, i use that as well from time to t
NoQ added a comment.
Yeah, i guess i should've split those up.
Well, i had to fix the other code when i fixed the first code, because it
contains a hardcoded duplicate of the vector type. So i decided to fix it in a
slightly more intelligent manner.
Repository:
rC Clang
https://reviews.llv
NoQ added inline comments.
Comment at:
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:448-449
Loc ThisLoc = Context.getSValBuilder().getCXXThis(CtorDecl->getParent(),
Context.getStackFrame(
NoQ added a comment.
In https://reviews.llvm.org/D51393#1218544, @george.karpenkov wrote:
> In fact, generating a single "long" seems even better.
`ptrdiff_t` seems to express what we're trying to say pretty accurately.
https://reviews.llvm.org/D51393
__
NoQ added a comment.
Ok, i'll get to the rest of the stuff a bit later (unless you pick it up).
Repository:
rC Clang
https://reviews.llvm.org/D51385
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/lis
NoQ added a comment.
In https://reviews.llvm.org/D51300#1220537, @Szelethus wrote:
> Would you be comfortable me commiting this without that assert
Yup sure.
In https://reviews.llvm.org/D51300#1223042, @Szelethus wrote:
> I'm not even sure how this is possible -- and unfortunately I've been u
NoQ added inline comments.
Comment at: llvm/include/llvm/Support/Allocator.h:304
+// Use negative index to denote custom sized slabs.
+int64_t CustomSlabOffset = 0;
+for (size_t Idx = 0; Idx < CustomSizedSlabs.size(); Idx++) {
We should start with -1
NoQ added a comment.
Very easy:
1. Put preprocessed file that crashes into `repro.cpp` (`.c`, `.m`, whatever).
2. Write `repro.sh` as follows:
#!/bin/bash
blah/blah/path/to/clang -cc1 -analyze
-blah-blah-arguments-you-need-to-cause-a-crash repro.cpp 2>&1 | grep "Any
assertion message or wh
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
I actually thought it's just a different name for graphviz :/
I've definitely never heard of anybody using ubigraph for this purpose. And
it's an analyzer-specific feature. Let's remove.
https://r
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
I very very much appreciate this.
https://reviews.llvm.org/D51395
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
I'm happy with the code.
Comment at: llvm/include/llvm/Support/Allocator.h:298
+ if (P >= S && P < S + computeSlabSize(Idx))
+return InSlabIdx + static_cast(P - S);
+
NoQ added a comment.
Herald added a subscriber: Szelethus.
Dunno, i guess you can just commit it.
Comment at: lib/Sema/SemaStmtAsm.cpp:57
+ }
+Parent = Child;
+ }
Like father, like son :D
Repository:
rC Clang
https://reviews.llvm.org/D45416
__
NoQ added inline comments.
Comment at: test/Analysis/cstring-syntax.c:49
+ strlcat(dest, "0123456789", badlen / 2);
+ strlcat(dest, "0123456789", badlen); // expected-warning {{The third
argument allows to potentially copy more bytes than it should. Replace with the
value 'ba
NoQ added a comment.
I'm generally fine with landing the patch as long as the overall direction is
chosen (and be moved towards) for how to safely identify the non-deterministic
iterators (my previous inline question). This has to be addressed before we
move out of alpha, but i believe we shoul
NoQ added inline comments.
Comment at:
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp:126-127
if (V.isUndef()) {
+assert(!FR->getDecl()->getType()->isReferenceType() &&
+ "References must be initialized!");
return addFieldToUninits
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Comments always welcome!
Comment at:
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:363-364
+
+ // ImmutableList::getHead() isn't a const method,
NoQ added a comment.
What do you think about the following approach:
1. Define a `lit` substitution `%diff_plist` that resolves to `diff -u -w -I
"/" -I ".:" -I "version" -`.
2. Update these flags in your downstream fork.
It's kinda cool because it'll allow us to test exactly what we want in th
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Looks great to me and pretty trivial, should somebody who's more familiar with
this code take a look?
Comment at: clang/lib/AST/StmtPrinter.cpp:72
PrintingPolicy Policy;
+
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Looks as expected. I hope that assertion does actually hold. Should AST folks
have a look as well?
Comment at: clang/lib/AST/Stmt.cpp:310
+ return *Out / alignof(Stmt);
+
+}
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Looks good i guess. I'm glad this is sorted out^^
https://reviews.llvm.org/D51057
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.
NoQ added a comment.
Hmm, so we're reporting the same region twice in the same bug report through
different field chains, right? Why is a state trait necessary here? Maybe just
de-duplicate them locally before throwing the report?
https://reviews.llvm.org/D51531
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Hmm, i guess it's necessary to de-duplicate across multiple reports as well.
https://reviews.llvm.org/D51531
___
cfe-commits mailing list
cfe-commits@l
NoQ requested changes to this revision.
NoQ added a comment.
This revision now requires changes to proceed.
Or, wait, hmm, no, we can still de-duplicate the reports with a global set
stored in the checker object. Because we would probably like to de-duplicate
across different paths as well.
ht
NoQ accepted this revision.
NoQ added inline comments.
This revision is now accepted and ready to land.
Comment at:
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h:61-63
+ bool IsPedantic;
+ bool ShouldConvertNotesToWarnings;
+ bool CheckPointeeInitializ
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
I hope we'll be able to come up with a reasonable default regex.
Comment at: test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp:1-4
+// RUN: %clang_analyze_cc1
-analy
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
I was thinking of two flags, but that'll work too.
https://reviews.llvm.org/D49536
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:253
+ allocation_state::getContainerObjRegion(N->getState(), PtrToBuf);
+ const auto *TypedRegion = dyn_cast(ObjRegion);
+ QualType ObjTy = TypedRegion->getValueType();
---
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
LGTM!
Repository:
rC Clang
https://reviews.llvm.org/D49553
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mai
NoQ added a comment.
Oh, this FIXME, i almost forgot about that. Not sure if we should focus on this
now because it's kinda premature optimization, especially after
@george.karpenkov has fixed a large performance problem that caused `VisitNode`
to be called like ~30 times more often than necess
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs, baloghadamsoftware.
Herald added subscribers: cfe-commits, mikhail.ramalho.
Because `CXXOperatorCallExpr`'s argument 0 is the `this`-argument of the
operator if the operator is a
NoQ added a comment.
I've just one thing to add.
Comment at: lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp:149
+C.addTransition(State);
return;
+ }
xazax.hun wrote:
> Nit: This return is redundant.
Because of how easy it is to accidentally split
NoQ updated this revision to Diff 156827.
NoQ marked an inline comment as done.
NoQ added a comment.
In https://reviews.llvm.org/D49627#1172023, @george.karpenkov wrote:
> I take it we are crashing otherwise?
Not yet, but i'm about to commit code that would
(https://reviews.llvm.org/D49443).
NoQ added a comment.
What i'm fixing here is a false detection of construction context of a certain
kind. These are very bad in general, because they cause us to inline the
constructor without knowing what object we're constructing or how to handle
this object.
https://reviews.llvm.org/D49627
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs, mikhail.ramalho.
Herald added subscribers: cfe-commits, baloghadamsoftware.
This patch fixes the crash caused by https://reviews.llvm.org/D48650 and
reported as https://bugs.llvm.
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2934
+} else if (const auto *CallE = dyn_cast(S)) {
+ OS << CallE->getDirectCallee()->getNameAsString();
}
rnkovacs wrote:
> xazax.hun wrote
NoQ added inline comments.
Comment at: test/Analysis/inner-pointer.cpp:41-42
+template< class T >
+void func_ref(T& a);
+
Without a definition for this thing, we won't be able to test much. I suggest
you to take a pointer to a function directly, i.e. define a
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp:207-208
+
+for (unsigned I = 0, E = FD->getNumParams(); I != E; ++I) {
+ QualType ParamTy = FD->getParamDecl(I)->getType();
+ if (!ParamTy->isReferenceType() ||
--
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp:207-208
+
+for (unsigned I = 0, E = FD->getNumParams(); I != E; ++I) {
+ QualType ParamTy = FD->getParamDecl(I)->getType();
+ if (!ParamTy->isReferenceType() ||
--
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, mikhail.ramalho, baloghadamsoftware.
These methods allow reasoning about the stack frame that will be (or was) used
when the function will b
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs, mikhail.ramalho.
Herald added subscribers: cfe-commits, baloghadamsoftware.
Add one more defensive check to prevent crashes on trying to simplify a
`SymSymExpr` with different `Lo
NoQ accepted this revision.
NoQ added a comment.
Looks great!
Comment at: lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp:192
- if (Call.isCalled(CStrFn) || Call.isCalled(DataFn)) {
-SVal RawPtr = Call.getReturnValue();
-if (SymbolRef Sym = RawPtr.getAsSymbol(/*I
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Yet another great catch!
I guess you could write a test with `debug.AnalysisOrder` (by making its
`checkEndFunction` callback (that you'll have to define) print different things
depending on the re
NoQ added a comment.
Devin has recently pointed out that we might have as well reordered CFG
elements to have return statement kick in after automatic destructors, so that
callbacks were called in a different order. I don't see any problems with that
solution, but let's stick to the current sol
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Yup, tests for temporaries are great to have as well.
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2475-2477
// Check if we are returning freed memory.
if (Sym)
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, mikhail.ramalho, baloghadamsoftware.
Satisfies a review comment in https://reviews.llvm.org/D49749.
Repository:
rC Clang
https://reviews
NoQ requested review of this revision.
NoQ marked an inline comment as done.
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1300
+ Loc::isLocType(S->getRHS()->getType())) {
+SVal V = SVB.makeSymbolVal(S);
+Cached[S] =
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Yep, that'll do, thanks! Sorry for not keeping up with all the incoming reviews.
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:536
if (!SR.isLiveRegion(Cont.firs
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:605
+ if (Pos && !Pos->isValid()) {
+// If I do not put a tag here, some invalidation tests will fail
+static CheckerProgramPointTag Tag("InvalidatedIteratorChecker",
NoQ added a comment.
In https://reviews.llvm.org/D49536#1176128, @mikhail.ramalho wrote:
> I getting the following error when analyzing
> `test/Analysis/plist-macros.cpp`, usign z3 as constraint manager
> (`-analyzer-constraints=z3 -DANALYZER_CM_Z3`):
>
> /home/mgadelha/llvm/tools/clang/test/
NoQ added inline comments.
Comment at: test/Analysis/cxx-uninitialized-object-ptr-ref.cpp:290
struct IntDynTypedVoidPointerTest1 {
- void *vptr; // expected-note{{uninitialized pointee 'this->vptr'}}
+ void *vptr; // expected-note{{uninitialized pointee 'this->static_cast(vptr
NoQ added a comment.
Sorry, i'm still lagging with my reviews because there are a lot of them and i
have to balance it with doing actual work. I'll hopefully get to this soon.
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:681-689
+Optional OtherObj
NoQ added a comment.
The "Assuming..." diagnostic piece isn't the same as "Taking true/false branch"
diagnostic piece. The former indicates that a constraint is added. The latter
indicate how the path flows between program points.
The absence of "Assuming..." indicates that Z3 didn't need to ma
NoQ updated this revision to Diff 157416.
NoQ marked 4 inline comments as done.
NoQ added a comment.
Address comments by editing comments.
https://reviews.llvm.org/D49715
Files:
include/clang/Analysis/ConstructionContext.h
include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
includ
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:197
+
+ // Recover CFG block via reverse lookup. Maybe it should just be a part of
the
+ // CallEvent object? That would have been convenient.
george.karpenkov wrote:
> Can we remove
NoQ updated this revision to Diff 157550.
NoQ added a comment.
Add one more handy method, `getAdjustedParameterIndex()`, which helps using
`getParameterLocation()`.
https://reviews.llvm.org/D49715
Files:
include/clang/Analysis/ConstructionContext.h
include/clang/StaticAnalyzer/Core/PathSen
NoQ updated this revision to Diff 157789.
NoQ added a comment.
A better `ConstructedObjectKey::print()`.
https://reviews.llvm.org/D49210
Files:
include/clang/Analysis/ConstructionContext.h
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/Analysis/CFG.cpp
lib/Analysis/Con
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp:192
- if (Call.isCalled(CStrFn) || Call.isCalled(DataFn)) {
-SVal RawPtr = Call.getReturnValue();
-if (SymbolRef Sym = RawPtr.getAsSymbol(/*IncludeBaseRegions=*/true)) {
- /
NoQ accepted this revision.
NoQ added a comment.
Ok, cool! Let's get this done.
https://reviews.llvm.org/D49058
___
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/MallocChecker.cpp:2488
+
+ checkPreStmt(S, C);
+}
Let's do a common sub-function, similarly to how
`MallocChecker::checkPointerEscape` and
`MallocChecker::checkConstPointerEscape` both call
`M
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Cool! Always bothered me.
In https://reviews.llvm.org/D49986#1180403, @george.karpenkov wrote:
> I'm a bit confused: does it mean these methods are never called in Clang?
This patch *is* for clang
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, mikhail.ramalho, baloghadamsoftware.
Sometimes it's important to do `BugReport->addRange()` in order to know which
part of the expression is
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:681-689
+Optional OtherObject =
+getObjectVal(OtherCtor, Context);
+if (!OtherObject)
+ continue;
+
+// If the CurrentObject is a field of OtherObject, it wi
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:605
+ if (Pos && !Pos->isValid()) {
+// If I do not put a tag here, some invalidation tests will fail
+static CheckerProgramPointTag Tag("InvalidatedIteratorChecker",
NoQ closed this revision.
NoQ marked an inline comment as done.
NoQ added a comment.
Committed in https://reviews.llvm.org/rC338420 but i accidentally screwed the
revision link.
Repository:
rC Clang
https://reviews.llvm.org/D49749
___
cfe-commit
NoQ added a comment.
Uhm, sry, https://reviews.llvm.org/rC338425 is not relevant, i screwed up the
revision link again.
Repository:
rL LLVM
https://reviews.llvm.org/D49826
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm
NoQ closed this revision.
NoQ added a comment.
Committed in https://reviews.llvm.org/rC338425 but i accidentally screwed up
the revision link.
https://reviews.llvm.org/D48249
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm
NoQ added a comment.
In https://reviews.llvm.org/D49627#1176326, @baloghadamsoftware wrote:
> How much different is a correct `this`-argument construction context from
> real argument construction contexts?
It's identified in a similar manner; this patch pretty much already shows how
to ident
NoQ closed this revision.
NoQ added a comment.
Committed as https://reviews.llvm.org/rC338439 but Phabricator didn't pick it
up because i put a `.` after the phabricator link.
https://reviews.llvm.org/D49210
___
cfe-commits mailing list
cfe-commits
NoQ closed this revision.
NoQ added a comment.
Committed as https://reviews.llvm.org/rC338436 but Phabricator didn't pick it
up because i put a `.` after the phabricator link.
https://reviews.llvm.org/D48681
___
cfe-commits mailing list
cfe-commits
NoQ added a comment.
Hmm, AST has just changed in https://reviews.llvm.org/rC338135. While the
current patch is still relevant, it might be that we'll need to treat this
construction context as if it's a simple temporary now.
https://reviews.llvm.org/D49627
_
NoQ updated this revision to Diff 158668.
NoQ added a comment.
Ok, so with https://reviews.llvm.org/rC338135 operator this-argument
constructors do indeed work exactly like temporaries. This patch is not
necessary anymore. I'd still prefer to add tests and i've added a
path-sensitive test as we
NoQ added a comment.
I suspect that with https://reviews.llvm.org/rC338135 your operator
this-argument re-assignment mechanism is no longer necessary. Due to the
combination of support for materializing temporaries that i added previously
and the change in the AST that turns the operator this-a
NoQ added inline comments.
Comment at:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:431
+return ExprEngine::getObjectUnderConstruction(
+getState(), {getOriginExpr(), Index},
getCalleeStackFrame()).hasValue();
+ }
Whoops, t
NoQ accepted this revision.
NoQ added a comment.
All this stuff looks great! Please commit.
https://reviews.llvm.org/D49361
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
I see, so that's how it's done!
I also noticed that checker name was weird in exploded graph dumps, i.e. it was
showing regular new/delete stuff as if it was done by InnerPointer checker.
I'll chec
NoQ added a comment.
Yay thx! Yeah, that's the best behavior i can imagine here.
Repository:
rC Clang
https://reviews.llvm.org/D50211
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi
NoQ added a comment.
Oh, i see what you did here. I thought you're evaluating the argument, but
you're evaluating the whole builtin function call. In this case you don't need
the else-branch: `EvaluateAsInt` will always succeed. Moreover, you can merge
your code with the `BI__builtin_object_siz
NoQ updated this revision to Diff 132908.
NoQ marked an inline comment as done.
NoQ added a comment.
Fix uninitialized variables.
https://reviews.llvm.org/D42672
Files:
include/clang/Analysis/AnalysisDeclContext.h
include/clang/Analysis/CFG.h
include/clang/StaticAnalyzer/Core/AnalyzerOpti
NoQ added inline comments.
Comment at: include/clang/Analysis/CFG.h:153
+
+ ConstructionContext() = default;
+ ConstructionContext(CXXConstructExpr *Constructor, Stmt *Trigger)
xazax.hun wrote:
> Maybe I am getting this wrong, but I think in this case the membe
NoQ planned changes to this revision.
NoQ added a comment.
Destructor for the returned temporary is still evaluated conservatively:
class C {
public:
int &x, &y;
C(int &_x, int &_y) : x(_x), y(_y) { ++x; }
C(const C &c) : x(c.x), y(c.y) { ++x; }
~C() { ++y; }
};
C make(
101 - 200 of 3498 matches
Mail list logo
