steakhal added a comment.
Fixed remarks, cleaned up residual includes, rebased.
Should be read to land.
Final thoughts?
================
Comment at:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h:501
public:
+ const iterator &operator*() const { return *this; }
+
----------------
steakhal wrote:
> xazax.hun wrote:
> > Is this just a dummy to make sure this satisfies some concepts? I think I
> > comment might be helpful in that case, people might find a noop dereference
> > operator confusing. Same for some other places.
> Yes. I'll add a comment.
> This falls into the "weird" iterators category.
Added comments to both places.
================
Comment at:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:749-750
referenced_vars_iterator referenced_vars_begin() const;
referenced_vars_iterator referenced_vars_end() const;
+ llvm::iterator_range<referenced_vars_iterator> referenced_vars() const;
----------------
steakhal wrote:
> xazax.hun wrote:
> > Do we need these with the addition of `referenced_vars`?
> I no longer remember where, but yes. But for sure I went through these to see
> if I could remove them.
Now I remember.
The `referenced_vars` is implemented as
`llvm::make_range(referenced_vars_begin(), referenced_vars_end())`, thus I need
those functions as they do quite a bit of work if you check.
So, I kept them.
================
Comment at:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h:637-638
region_iterator region_begin() const { return LiveRegionRoots.begin(); }
region_iterator region_end() const { return LiveRegionRoots.end(); }
+ llvm::iterator_range<region_iterator> regions() const {
----------------
steakhal wrote:
> xazax.hun wrote:
> > Should we remove these?
> I'm pretty sure we need these somewhere.
> One prominent place is where we dump these into JSONs. We do some fancy
> iteration there. I'm not sure that is the reason for this particular case.
> I'll give it another look to confirm.
This was just an oversight. Removed. Thanks!
================
Comment at: clang/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp:72
- for (auto B=CondS->symbol_begin(), E=CondS->symbol_end(); B != E; ++B) {
- const SymbolRef Antecedent = *B;
+ for (SymbolRef Antecedent : CondS->symbols()) {
State = addImplication(Antecedent, State, true);
----------------
Oh, I didn't know this is a word :D
================
Comment at: clang/lib/StaticAnalyzer/Core/Environment.cpp:29
#include "llvm/ADT/ImmutableMap.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallPtrSet.h"
----------------
Removed this extra include.
================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp:173-177
+ } else {
// The left-hand side may bind to a different value then the
// computation type.
LHSVal = svalBuilder.evalCast(Result, LTy, CTy);
+ }
----------------
steakhal wrote:
> xazax.hun wrote:
> > Might be an issue with Phab, but indent looks strange to me here.
> Thanks for catching this. Indeed. I'll have a look. Probably I messed
> something up.
Clang-format went crazy, and I'm using clang-format on save, thus messed this
up.
Fixed up manually. Should be good now.
================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:1190
CXXRecordDecl::field_iterator CurField = LE->getLambdaClass()->field_begin();
- for (LambdaExpr::const_capture_init_iterator i = LE->capture_init_begin(),
- e = LE->capture_init_end();
- i != e; ++i, ++CurField, ++Idx) {
+ for (auto i = LE->capture_init_begin(), e = LE->capture_init_end(); i != e;
+ ++i, ++CurField, ++Idx) {
----------------
steakhal wrote:
> xazax.hun wrote:
> > Ha about using `capture_inits`?
> I would count this as a "fancy" iteration as we increment/advance more
> logical sequences together.
I had a better idea and llvm::zip all these together.
Check it out, and tell me if you like it more ;)
I certainly do.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154325/new/
https://reviews.llvm.org/D154325
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
