[PATCH] D138914: Make evaluation of nested requirement consistent with requires expr.

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.


================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:2339
+            Req->getConstraintExpr()->getSourceRange(), Satisfaction))
+      TransConstraint = Result[0];
+    assert(!Trap.hasErrorOccurred() && "Substitution failures must be handled "
----------------
usaxena95 wrote:
> rtrieu wrote:
> > I have found a crash here when it access vector `Result` without checking the size first, leading to out of bounds memory access.  CReduce gave the following testcase:
> > 
> > ```
> > template <class a, a> struct b;
> > template <bool c> using d = b<bool, c>;
> > template <class a, class e> using f = d<__is_same(a, e)>;
> > template <class a, class e>
> > concept g = f<a, e>::h;
> > template <class a, class e>
> > concept i = g<e, a>;
> > template <typename> class j {
> >   template <typename k>
> >   requires requires { requires i<j, k>; }
> >   j();
> > };
> > template <> j();
> > ```
> > 
> > `clang reduce.ii --std=c++20`
> > 
> > ```
> > assertion failed at llvm/include/llvm/ADT/SmallVector.h:294 in reference llvm::SmallVectorTemplateCommon<clang::Expr *>::operator[](size_type) [T = clang::Expr *]: idx < size()
> > ...
> > ...
> > (anonymous namespace)::TemplateInstantiator::TransformNestedRequirement(clang::concepts::NestedRequirement*) clang/lib/Sema/SemaTemplateInstantiate.cpp:2339:25
> > ...
> > ...
> > ```
> Thanks for noticing and sorry for the trouble. I have fixed this forward in https://github.com/llvm/llvm-project/commit/8c0aa53b07caa604d58a0d83dc571d8fcb004972.
Can you add a test case as well so if we regress we will catch it, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138914