clang: concept checking bug in out-of-line definitions of inner class member functions

[davidtgoldblatt]

This code:

template<typename Param>
concept TrivialConcept =
  requires(Param param) {
    (void)param;
  };

template <typename T>
struct Base {
    class InnerClass;
};

template <typename T>
class Base<T>::InnerClass {
    template <typename Param>
    requires TrivialConcept<Param> 
    int func(Param param) const;
};

template <typename T>
template <typename Param>
requires TrivialConcept<Param>
int Base<T>::InnerClass::func(Param param) const {
    return 0;
}

Works on clang-16 and gcc trunk, fails on clang trunk. I believe it should be allowed per C++20 and above.

Bisecting points to 6db007a as the likely culprit (this got reverted a couple of times previously for similar issues; this one slipped past).

Tagging a few interested people from the diff and from #63782, which points to the same commit:
@alexander-shaposhnikov @erichkeane @shafik

[llvmbot]

@llvm/issue-subscribers-clang-frontend

[llvmbot]

@llvm/issue-subscribers-c-20

[shafik]

@erichkeane do you have any advice on what to do here?

[alexander-shaposhnikov]

I think it's a bug in Clang, we need to fix it.
p.s. unfortunately, bisecting to 6db007a is not of much help here (that commit switched the approach to how we compare constraints). The symptoms are similar to what we've seen before.

[erichkeane]

So this is another part of comparing constraints that I don't particularly well understand, all handled by AreConstraintExpressionsEqual. We end up needing to substitute each constraint expression with the details from the current declaration, however this is a case of the 'new' Expression not finding any levels to substitute, which seems incorrect to me? So I think the problem is in line 758's call to getTemplateInstantiationArgs.

@alexander-shaposhnikov 's 6db007a ends up adding this line, so I am concerned that the changes he made to the collection of arguments is at least partially to blame here, but I don't particularly have the ability to debug much more.

[erichkeane]

Actually, I think when 6db007a's HandleFunctionTemplateDecl is trying to find the template arguments, it ends up not handling it right. I suspect instead of the 'if' statement there, it should be 'looping' upwards through the NestedNameSpecifiers. In this case, it ends up finding JUST the InnerClass which doesn't add template arguments, but probably should ALSO loop up a step for Base<T>.

[erichkeane]

Bah, I got nerd-sniped. I have a patch up for review here: #65993