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

[Aaron Ballman via Phabricator via lldb-commits]

aaron.ballman edited reviewers, added: erichkeane, royjacobson, clang-language-wg; removed: mizvekov.
aaron.ballman added a subscriber: rjmccall.
aaron.ballman added a comment.

In D140996#4125177 <https://reviews.llvm.org/D140996#4125177>, @bolshakov-a wrote:

> @aaron.ballman, @rsmith, @mizvekov, @shafik,  has the mankind any chance to get this reviewed and merged?

Sorry for the delay in review! I've changed the reviewer list a bit to get more visibility on this. Also, don't forget to add a release note for the changes. Should this also update the status in `clang/www/cxx_status.html`?



================
Comment at: clang/lib/AST/ItaniumMangle.cpp:4397
+    // argument.
+    // As proposed in https://github.com/itanium-cxx-abi/cxx-abi/issues/111.
+    auto *SNTTPE = cast<SubstNonTypeTemplateParmExpr>(E);
----------------
We should get this nailed down. It was proposed in Nov 2020 and the issue is still open. CC @rjmccall 


================
Comment at: clang/lib/AST/TemplateBase.cpp:204-211
+  if (Type->isIntegralOrEnumerationType() && V.isInt())
+    *this = TemplateArgument(Ctx, V.getInt(), Type);
+  else if ((V.isLValue() && V.isNullPointer()) ||
+           (V.isMemberPointer() && !V.getMemberPointerDecl()))
+    *this = TemplateArgument(Type, /*isNullPtr=*/true);
+  else if (const ValueDecl *VD = getAsSimpleValueDeclRef(Ctx, Type, V))
+    // FIXME: The Declaration form should expose a const ValueDecl*.
----------------
Well this is certainly a unique approach...  Personally, I think it'd be nice to not assign to `this` within a constructor by calling other constructor; that falls squarely into "wait, what, can you even DO that?" kind of questions for me.


================
Comment at: clang/lib/AST/TemplateBase.cpp:619
+  case TemplateArgument::UncommonValue: {
+    // FIXME: We're guessing at LangOptions!
+    SmallString<32> Str;
----------------
It's probably okay enough, but this is now the third instance of adding the same bug to this helper method -- maybe we should fix that instead?


================
Comment at: clang/lib/Index/USRGeneration.cpp:1032
+  case TemplateArgument::UncommonValue:
+    // FIXME: Visit value.
+    break;
----------------
Any particular reason this isn't being handled now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140996