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

Reply via email to