[PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.


================
Comment at: clang/include/clang/AST/ASTContext.h:3036
     if (!std::is_trivially_destructible<T>::value) {
-      auto DestroyPtr = [](void *V) { static_cast<T *>(V)->~T(); };
-      AddDeallocation(DestroyPtr, Ptr);
+      auto DestroyPtr = [](void *V) { ((T *)V)->~T(); };
+      AddDeallocation(DestroyPtr, (void *)Ptr);
----------------
bolshakov-a wrote:
> erichkeane wrote:
> > This change is weird... what is going on here?
> Here is not very beautiful attempt to workaround const-ness of `TemplateArgument::V::Value` pointer passed here from the added `TemplateArgument` constructor. The change in this line isn't acually needed and made only for consistence with the next line, I think. Alternatively, I can
> 1) refactor `addDestruction` and `AddDeallocation` to work with pointers to constants, or
> 2) add `const_cast` to `AddDeallocation` call in the next line, or
> 3) make `TemplateArgument::V::Value` pointer non-const.
> 
> I'm biased to the first variant.
I'd lean towards #3, it ends up being consistent with the rest of the things here.  #1 is interesting, but that results in these functions violating const-correctness.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140996