steakhal added a comment. This isn't an NFC change. It's more than a refactor/cosmetic change. Please run some benchmarks to validate the assumed performance characteristics.
================ Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h:156 + private: + llvm::SmallSet<const ProgramState *, 4> Aux; + }; ---------------- Have you considered [[ https://www.llvm.org/docs/ProgrammersManual.html#llvm-adt-smallptrset-h | llvm::SmallPtrSet]] ? ================ Comment at: clang/lib/StaticAnalyzer/Core/ConstraintManager.cpp:56-59 + const ProgramState *RawSt = State.get(); + if (AssumeStack.hasCycle(RawSt)) + return {State, State}; + AssumeStack.push(RawSt); ---------------- Why don't you return `RawSt` on the early return path instead? That seems to contain more information than `State`, right? You lookup twice the location of the `RawSt` entry; once in the `hasCycle()` and again in the `push()`. I would recommend doing an unconditional `insert()` directly and reusing the returned bool part of the pair to observe if it did actually insert. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126560/new/ https://reviews.llvm.org/D126560 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
