NoQ added a comment.
Np, post-commit review is always welcome (:
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65361/new/
https://reviews.llvm.org/D65361
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https:/
Szelethus added a comment.
Sorry for not accepting this -- I actually read the code multiple times and
didn't see anything that stood out! I didn't have the time to dig too deep, but
if the tests are anything to go by, its gotta be alright.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
ht
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 rL370244: [analyzer] Trust global initializers when analyzing
main(). (authored by dergachev, committed by ).
Herald added a
NoQ updated this revision to Diff 215274.
NoQ marked 3 inline comments as done.
NoQ added a comment.
Fxd.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65361/new/
https://reviews.llvm.org/D65361
Files:
clang/lib/StaticAnalyzer/Core/RegionStore.cpp
clang/test/Analysis/main.c
clang
Szelethus added a comment.
I really like the high level idea proposed by this patch, and the test files
make me believe that its correct as well! I'm really not familiar around this
part of the code, so if its okay, I'll take my time to do the usual find
references, inserting unreachables/asser
NoQ added inline comments.
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:630
+(const RegionBindings::TreeTy *)((uintptr_t)store & ~(uintptr_t)1),
+RBFactory.getTreeFactory(), (bool)((uintptr_t)store & (uintptr_t)1));
}
xazax.hun wro
NoQ updated this revision to Diff 214470.
NoQ marked 8 inline comments as done.
NoQ added a comment.
Fxd!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65361/new/
https://reviews.llvm.org/D65361
Files:
clang/lib/StaticAnalyzer/Core/RegionStore.cpp
clang/test/Analysis/main.c
clang
xazax.hun added a comment.
I like the change.
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:630
+(const RegionBindings::TreeTy *)((uintptr_t)store & ~(uintptr_t)1),
+RBFactory.getTreeFactory(), (bool)((uintptr_t)store & (uintptr_t)1));
}
-
baloghadamsoftware added a comment.
Thank you for working on this! I agree, we should trust global initializers in
`main()` in C. Can we maybe detect whether GNU global constructors are enabled
or better: used?
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:395
NoQ updated this revision to Diff 212035.
NoQ added a comment.
Actually add the logic for C++.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65361/new/
https://reviews.llvm.org/D65361
Files:
clang/lib/StaticAnalyzer/Core/RegionStore.cpp
clang/test/Analysis/main.c
clang/test/Analy
NoQ marked an inline comment as done.
NoQ added inline comments.
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:157
ClusterBindings::Factory *CBFactory;
+ bool IsMainAnalysis;
We could have put this flag into `RegionStoreManager` instead - after
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus,
baloghadamsoftware, Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho,
a.sidorin, JDevlieghere, szepet.
Herald added a project: clang.
This is inspired by http
12 matches
Mail list logo