dblaikie added a comment.

Thanks for the fix! Totally within the realm of post-commit review so far as I 
can see. The paired assertions could maybe be rewritten as: 
"assert(MRMgr.getVarRegion(P, SFC) ->getKind()== (SFC->inTopFrame() ? 
NonParamVarRegionKind : ParamVarRegionKind));" though it's not necessarily 
better (bit verbose, but avoids the textual duplication of the getVarRegion 
calls). Oh, and that first assert could potentially be an 
"assert(llvm::all_of... " to avoid D2 <https://reviews.llvm.org/D2> being an 
unused variable in non-asserts builds).

There's no need to send things for review that don't need pre-commit review. 
Post-commit review can be done in replies to the commit email on llvm-commits. 
(sending things like this can muddy the waters about post/pre-commit review - 
especially around code sent for pre-commit review but is then committed without 
that review concluding)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81522/new/

https://reviews.llvm.org/D81522



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to