Avoid nullptr dereferencing of 'Constraint'

[schittir]

Summary

Klocwork static code analysis exposed this bug:
Pointer 'Constraint' returned from call to function
'cast_or_null<clang::ConceptSpecializationExpr,clang::Expr>' may
be NULL and will be dereferenced in the statement following it

Replace 'cast_or_null' with 'cast' so that the latter can assert
when it encounters a NULL

[aaron.ballman]

Thanks for the cleanup! Btw, it's helpful if you add more context to the patch when generating the diff. I typically use git diff -U9999 when generating a diff as that gives plenty of context for the patch review within Phabricator.

clang/lib/Sema/SemaConcept.cpp

1065–1068

If we're going to be touching this code, there's more suspect code here that needs to be cleaned up a bit. Directly above this is:

const TypeConstraint *TC =
    cast<TemplateTypeParmDecl>(TPL->getParam(0))->getTypeConstraint();
assert(TC &&
       "TPL must have a template type parameter with a type constraint");

That assertion can be removed entirely -- if the cast<> fails, it doesn't return null, it asserts.

[schittir]

1. Removed the useless assert on TC.
2. Retained the change from cast_or_null to cast

[aaron.ballman]

LGTM, thanks for the cleanup! Do you need someone to commit on your behalf? If so, what name and email address would you like me to use for patch attribution?

[schittir]

In D108481#2960875, @aaron.ballman wrote:

LGTM, thanks for the cleanup! Do you need someone to commit on your behalf? If so, what name and email address would you like me to use for patch attribution?

Thank you, Aaron.
Yes, please! Sindhu Chittireddy; [e-mail]

[aaron.ballman]

In D108481#2960883, @schittir wrote:

In D108481#2960875, @aaron.ballman wrote:

LGTM, thanks for the cleanup! Do you need someone to commit on your behalf? If so, what name and email address would you like me to use for patch attribution?

Thank you, Aaron.
Yes, please! Sindhu Chittireddy; [e-mail]

Thanks! I've committed on your behalf in 98339f14a0420cdfbe4215d8d1bc0a01165e0495.

[mlychkov]

clang/lib/Sema/SemaConcept.cpp

1065–1068

Actually, cast return nonnull value, but

TC = "cast_result"->getTypeConstraint();

and TC may become nullptr, I suppose.

[aaron.ballman]

clang/lib/Sema/SemaConcept.cpp

1065–1068

Good catch! I think we should add the assertion on TC being nonnull back. I'll take care of that.

[erichkeane]

clang/lib/Sema/SemaConcept.cpp

1065–1068

Ooh, good catch! I definitely missed that when suggesting removing the assert. @aaron.ballman can you re-add the assert on 1065?

[aaron.ballman]

clang/lib/Sema/SemaConcept.cpp

1065–1068

I've put the assert back in 0cf4f81082e9fa052e60450b8cbb10007e59931c, thanks for letting us know @mlychkov!

clang/lib/Sema/SemaConcept.cpp

 }
 
 concepts::ExprRequirement::ReturnTypeRequirement::
 ReturnTypeRequirement(TemplateParameterList *TPL) :
     TypeConstraintInfo(TPL, 0) {
   assert(TPL->size() == 1);
   const TypeConstraint *TC =
       cast<TemplateTypeParmDecl>(TPL->getParam(0))->getTypeConstraint();
-  assert(TC &&
-         "TPL must have a template type parameter with a type constraint");
   auto *Constraint =
-      cast_or_null<ConceptSpecializationExpr>(
-          TC->getImmediatelyDeclaredConstraint());
+      cast<ConceptSpecializationExpr>(TC->getImmediatelyDeclaredConstraint());
   bool Dependent =
       Constraint->getTemplateArgsAsWritten() &&
       TemplateSpecializationType::anyInstantiationDependentTemplateArguments(
           Constraint->getTemplateArgsAsWritten()->arguments().drop_front(1));
   TypeConstraintInfo.setInt(Dependent ? 1 : 0);
 }
 
 concepts::TypeRequirement::TypeRequirement(TypeSourceInfo *T) :
     Requirement(RK_Type, T->getType()->isInstantiationDependentType(),
                 T->getType()->containsUnexpandedParameterPack(),
                 // We reach this ctor with either dependent types (in which
                 // IsSatisfied doesn't matter) or with non-dependent type in
                 // which the existence of the type indicates satisfaction.
                 /*IsSatisfied=*/true),
     Value(T),
     Status(T->getType()->isInstantiationDependentType() ? SS_Dependent
                                                         : SS_Satisfied) {}