[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)

[Matheus Izvekov via cfe-commits]

Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/121768 at github.com>


================
@@ -1228,35 +1228,45 @@ bool Sema::AttachTypeConstraint(AutoTypeLoc TL,
                                 NonTypeTemplateParmDecl *NewConstrainedParm,
                                 NonTypeTemplateParmDecl *OrigConstrainedParm,
                                 SourceLocation EllipsisLoc) {
-  if (NewConstrainedParm->getType().getNonPackExpansionType() != TL.getType() ||
-      TL.getAutoKeyword() != AutoTypeKeyword::Auto) {
-    Diag(NewConstrainedParm->getTypeSourceInfo()->getTypeLoc().getBeginLoc(),
-         diag::err_unsupported_placeholder_constraint)
-        << NewConstrainedParm->getTypeSourceInfo()
-               ->getTypeLoc()
-               .getSourceRange();
-    return true;
-  }
-  // FIXME: Concepts: This should be the type of the placeholder, but this is
-  // unclear in the wording right now.
-  DeclRefExpr *Ref =
-      BuildDeclRefExpr(OrigConstrainedParm, OrigConstrainedParm->getType(),
-                       VK_PRValue, OrigConstrainedParm->getLocation());
-  if (!Ref)
-    return true;
-  ExprResult ImmediatelyDeclaredConstraint = formImmediatelyDeclaredConstraint(
-      *this, TL.getNestedNameSpecifierLoc(), TL.getConceptNameInfo(),
-      TL.getNamedConcept(), /*FoundDecl=*/TL.getFoundDecl(), TL.getLAngleLoc(),
-      TL.getRAngleLoc(), BuildDecltypeType(Ref),
-      OrigConstrainedParm->getLocation(),
-      [&](TemplateArgumentListInfo &ConstraintArgs) {
-        for (unsigned I = 0, C = TL.getNumArgs(); I != C; ++I)
-          ConstraintArgs.addArgument(TL.getArgLoc(I));
-      },
-      EllipsisLoc);
+  ExprResult ImmediatelyDeclaredConstraint = [&] {
+    if (NewConstrainedParm->getType().getNonPackExpansionType() !=
+            TL.getType() ||
+        TL.getAutoKeyword() != AutoTypeKeyword::Auto) {
+      Diag(NewConstrainedParm->getTypeSourceInfo()->getTypeLoc().getBeginLoc(),
+           diag::err_unsupported_placeholder_constraint)
+          << NewConstrainedParm->getTypeSourceInfo()
+                 ->getTypeLoc()
+                 .getSourceRange();
+      return ExprResult();
+    }
+
+    // FIXME: Concepts: This should be the type of the placeholder, but this is
+    // unclear in the wording right now.
+    DeclRefExpr *Ref =
+        BuildDeclRefExpr(OrigConstrainedParm, OrigConstrainedParm->getType(),
+                         VK_PRValue, OrigConstrainedParm->getLocation());
+    assert(Ref != nullptr && "Unexpected nullptr!");
+
+    return formImmediatelyDeclaredConstraint(
+        *this, TL.getNestedNameSpecifierLoc(), TL.getConceptNameInfo(),
+        TL.getNamedConcept(), /*FoundDecl=*/TL.getFoundDecl(),
+        TL.getLAngleLoc(), TL.getRAngleLoc(), BuildDecltypeType(Ref),
+        OrigConstrainedParm->getLocation(),
+        [&](TemplateArgumentListInfo &ConstraintArgs) {
+          for (unsigned I = 0, C = TL.getNumArgs(); I != C; ++I)
+            ConstraintArgs.addArgument(TL.getArgLoc(I));
+        },
+        EllipsisLoc);
+  }();
+
   if (ImmediatelyDeclaredConstraint.isInvalid() ||
-      !ImmediatelyDeclaredConstraint.isUsable())
+      !ImmediatelyDeclaredConstraint.isUsable()) {
+    NewConstrainedParm->setPlaceholderTypeConstraint(
+        RecoveryExpr::Create(Context, OrigConstrainedParm->getType(),
----------------
mizvekov wrote:

Right, but the original bug was about fixing a crash-on-invalid, which was found during reduction of an unrelated bug. It seems this 'improvement' is being preempted, without an actual motivating case.
The RecoveryExpr doesn't even always have the correct type the expression would have, so it's not clear what kind of compromise would be achieved here.

I mostly say this because always initializing the placeholder pointer to null would have been a much simpler fix, which would potentially cover other instances of a similar bug.

https://github.com/llvm/llvm-project/pull/121768