dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Looks good.



================
Comment at: clang/lib/Sema/SemaLookup.cpp:623
 
-  Decls.set_size(N);
+  Decls.truncate(N);
 
----------------
dexonsmith wrote:
> Two things to confirm here.
> 
> First is that the destructors are trivial. From 
> clang/include/clang/AST/DeclAccessPair.h:
> ```
> lang=c++
> class DeclAccessPair {
>   uintptr_t Ptr; // we'd use llvm::PointerUnion, but it isn't trivial
> ```
> (If they hadn't been trivial, then hypothetically there could been other code 
> somewhere that ran the destructors later...)
> 
> Second is that `set_size()` was only used for truncation. I confirmed that 
> that three ways:
> - Looking backward, `N` starts as `Decls.size()` and the only changes are 
> decrement operatoers.
> - Looking forward, there's no code that would initialize / assign to the new 
> member (so if it increased size, it would likely have led to problems 
> elsewhere).
> - Tests pass.
Thanks for the details!


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

https://reviews.llvm.org/D115386

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D115386: A... Duncan P. N. Exon Smith via Phabricator via cfe-commits
    • [PATCH] D1153... Duncan P. N. Exon Smith via Phabricator via cfe-commits
    • [PATCH] D1153... Duncan P. N. Exon Smith via Phabricator via cfe-commits
    • [PATCH] D1153... David Blaikie via Phabricator via cfe-commits

Reply via email to