================
@@ -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(),
----------------
alejandro-alvarez-sonarsource wrote:
The reasons for the `RecoveryExpr` is, indeed, like @erichkeane said, to be 
able to identify on the AST places where things were missing during parsing.

For this particular use case, the frontend (an analyzers) can easily tell apart 

1. There is no type constraint (`nullptr`)
2. There should be one but we could not parse it (`RecoveryExpr`)

The second point, IIUC [the 
documentation](https://clang.llvm.org/doxygen/classclang_1_1RecoveryExpr.html#details),
 is one of the reasons to use `RecoveryExpr`. Option 1 fixes the crash but 
loses information that can be used for better diagnosis.

> which was found during reduction of an unrelated bug

Not really, and I apologize if I have given this impression.
We have had few crashes and assertions when parsing `mp-units` with missing 
headers. This was not accidentally found because of #118288 (which is my other 
PR based on errors from this project).

>  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.

I agree with you here, and the trailing object should probably be initialized 
to null on the constructor of `NonTypeTemplateParmDecl` to catch any other 
accidental uninitialized value. I am happy to do that too if no one objects.

https://github.com/llvm/llvm-project/pull/121768
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to