mboehme marked an inline comment as done. mboehme added inline comments.
================ Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:100 + // used `StorageLocation` subclasses and make them use a `BumpPtrAllocator`. + Locs.push_back(std::make_unique<T>(std::forward<Args>(args)...)); + return *cast<T>(Locs.back().get()); ---------------- xazax.hun wrote: > Would emplace back work? That returns a reference to the just emplaced > element saving us the call to `back` making the code a bit more concise. Nice -- thanks for the suggestion! Done. ================ Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:328 + template <typename T, typename... Args> + std::enable_if_t<std::is_base_of<StorageLocation, T>::value, T &> + create(Args &&...args) { ---------------- xazax.hun wrote: > Just curious, what is the reason for repeating the `enable_if` here in > addition to the one in the called function? Do we get better error messages? Yes, the idea is: - We get error messages from the interface of `create()` rather than the implementation - The `enable_if` documents intent. (The comment above states this intent too, but it's always nicer to have the code itself state the intent if possible.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147302/new/ https://reviews.llvm.org/D147302 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits