[Clang] Recompute the value category when rebuilding SubstNonTypeTemplateParmExpr

[zyn0217]

In concept checking, we need to transform SubstNTTPExpr when evaluating constraints.

The value category is initially computed during parameter mapping, possibly with a dependent expression. However during instantiation, it wasn't recomputed, and the stale category is propagated into parent expressions. So we may end up with an 'out-of-thin-air' reference type, which breaks the evaluation.

We now call BuildSubstNonTypeTemplateParmExpr in TreeTransform, in which the value category is recomputed.

The issue was brought by both 078e99e and the concept normalization patch, which are not released yet, so no release note.

Fixes #170856

[llvmbot]

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

Changes

In concept checking, we need to transform SubstNTTPExpr when evaluating constraints.

The value category is initially computed during parameter mapping, possibly with a dependent expression. However during instantiation, it wasn't recomputed, and the stale category is propagated into parent expressions. So we may end up with an 'out-of-thin-air' reference type, which breaks the evaluation.

We now call BuildSubstNonTypeTemplateParmExpr in TreeTransform, in which the value category is recomputed.

The issue was brought by both 078e99e and the concept normalization patch, which are not released yet, so no release note.

Fixes #170856

---

Full diff: https://github.com/llvm/llvm-project/pull/172251.diff

2 Files Affected:

• (modified) clang/lib/Sema/TreeTransform.h (+14-5)
• (modified) clang/test/SemaTemplate/concepts.cpp (+45)

diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 8e5dbeb792348..1adb81ad358b7 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -4297,6 +4297,15 @@ class TreeTransform {
     return getSema().OpenACC().ActOnOpenACCAsteriskSizeExpr(AsteriskLoc);
   }
 
+  ExprResult
+  RebuildSubstNonTypeTemplateParmExpr(Decl *AssociatedDecl,
+                                      const NonTypeTemplateParmDecl *NTTP,
+                                      SourceLocation Loc, TemplateArgument Arg,
+                                      UnsignedOrNone PackIndex, bool Final) {
+    return getSema().BuildSubstNonTypeTemplateParmExpr(
+        AssociatedDecl, NTTP, Loc, Arg, PackIndex, Final);
+  }
+
 private:
   QualType TransformTypeInObjectScope(TypeLocBuilder &TLB, TypeLoc TL,
                                       QualType ObjectType,
@@ -16459,7 +16468,7 @@ ExprResult TreeTransform<Derived>::TransformSubstNonTypeTemplateParmPackExpr(
 
   TemplateArgument Pack = E->getArgumentPack();
   TemplateArgument Arg = SemaRef.getPackSubstitutedTemplateArgument(Pack);
-  return SemaRef.BuildSubstNonTypeTemplateParmExpr(
+  return getDerived().RebuildSubstNonTypeTemplateParmExpr(
       E->getAssociatedDecl(), E->getParameterPack(),
       E->getParameterPackLocation(), Arg, SemaRef.getPackIndex(Pack),
       E->getFinal());
@@ -16517,10 +16526,10 @@ ExprResult TreeTransform<Derived>::TransformSubstNonTypeTemplateParmExpr(
     Replacement = E->getReplacement();
   }
 
-  return new (SemaRef.Context) SubstNonTypeTemplateParmExpr(
-      Replacement.get()->getType(), Replacement.get()->getValueKind(),
-      E->getNameLoc(), Replacement.get(), AssociatedDecl, E->getIndex(),
-      E->getPackIndex(), E->isReferenceParameter(), E->getFinal());
+  return getDerived().RebuildSubstNonTypeTemplateParmExpr(
+      AssociatedDecl, E->getParameter(), E->getNameLoc(),
+      TemplateArgument(Replacement.get(), /*IsCanonical=*/false),
+      E->getPackIndex(), E->getFinal());
 }
 
 template<typename Derived>
diff --git a/clang/test/SemaTemplate/concepts.cpp b/clang/test/SemaTemplate/concepts.cpp
index c90af41a09468..eed8b786f9954 100644
--- a/clang/test/SemaTemplate/concepts.cpp
+++ b/clang/test/SemaTemplate/concepts.cpp
@@ -1658,3 +1658,48 @@ struct {
 void foo() { call(""); }
 
 }
+
+namespace GH170856 {
+
+template <unsigned N, unsigned M> struct symbol_text {
+  consteval symbol_text(const char txt[]) {}
+};
+template <unsigned N> symbol_text(const char (&)[N]) -> symbol_text<1, 1>;
+struct quantity_spec_interface_base {};
+template <symbol_text, auto...> struct named_unit;
+struct quantity_spec_interface : quantity_spec_interface_base {};
+struct : quantity_spec_interface {
+} thermodynamic_temperature;
+
+template <typename T>
+concept QuantitySpec = __is_convertible(T*, quantity_spec_interface_base*);
+template <typename, auto QS>
+concept QuantitySpecOf = QuantitySpec<decltype(QS)>;
+template <typename T, auto QS>
+concept PointOriginFor = QuantitySpecOf<decltype(QS), T::_quantity_spec_>;
+
+template <symbol_text Symbol, auto QS, auto PO>
+struct named_unit<Symbol, QS, PO> {
+  static constexpr auto _point_origin_ = PO;
+};
+template <auto QS> struct absolute_point_origin {
+  static constexpr auto _quantity_spec_ = QS;
+};
+template <auto> struct relative_point_origin {};
+template <class R>
+consteval PointOriginFor<R{}> auto default_point_origin(R) {
+  return R{}._point_origin_;
+}
+template <auto> class quantity_point;
+template <class R> struct point_ {
+  quantity_point<default_point_origin(R{})> operator0();
+};
+template <auto R> point_<decltype(R)> point;
+struct absolute_zero : absolute_point_origin<thermodynamic_temperature> {
+} absolute_zero;
+auto zeroth_kelvin = absolute_zero;
+struct : named_unit<"", thermodynamic_temperature, zeroth_kelvin> {
+} kelvin;
+struct ice_point : relative_point_origin<point<kelvin>> {};
+
+}