[cfe-dev] Next steps for D24361 (patch to fix hasDeclaration wrt ElaboratedType)

[Aaron Ballman via cfe-dev]

On Fri, Oct 28, 2016 at 12:17 PM, Manuel Klimek <klimek at google.com> wrote:
> Playing around with the code, we have 2 problems we need to solve:
> a) the confusion between type & qualType weirdness
> That got in by accident, I think, though the getDecl() specialization that
> behaves differently from the partial desugaring we have due to b)
> b) the partial desugaring we do
> This basically was born from the idea that some of the sugaring classes we
> have are really really weird for devs (like TemplateSpecializationType) or
> basically useless to match (like ElaboratedType, where we can match on the
> NNS if we need to).
>
> a) seems fixable by a more fundamental patch in HasDeclarationMatcher on
> which I'm working
> b) seems like something we should discuss

I agree, thank you for looking into this and working on it!

~Aaron

>
> On Fri, Oct 28, 2016 at 4:27 PM Manuel Klimek <klimek at google.com> wrote:
>>
>> Now with the right cfe-dev
>>
>>
>> On Fri, Oct 28, 2016 at 4:14 PM Manuel Klimek <klimek at google.com> wrote:
>>>
>>> Somehow we lost cfe-dev on this mail thread :( :(
>>>
>>> adding Aaron, who has also contributed to that part of the code
>>>
>>> On Fri, Oct 28, 2016 at 4:11 PM Manuel Klimek <klimek at google.com> wrote:
>>>>
>>>> On Fri, Oct 28, 2016 at 3:54 PM Łukasz Anforowicz <lukasza at google.com>
>>>> wrote:
>>>>>
>>>>> Some notes from me (that hopefully will helpful to consider during the
>>>>> meeting):
>>>>>
>>>>> I agree that forcing hasDeclaration to consider all subclasses of Types
>>>>> (i.e. forcing via #include "clang/AST/TypeNodes.def" in the proposal below)
>>>>> seems to be a good thing.  The subset of types supported today seems to be
>>>>> rather arbitrary (I think;  please let me know if there is good reasoning +
>>>>> a way to describe the currently chosen subset of supported types).
>>>>> As a user of hasDeclaration, I have to "manually" drill through *some*
>>>>> can paints today.  For example in
>>>>> https://codereview.chromium.org/2256913002/patch/180001/190001 I am manually
>>>>> jumping over pointers and references (what I want to do is to tell whether a
>>>>> member dereference will end up using one of "my" classdecls where I am
>>>>> renaming methods and fields) - see |blink_qual_type_matcher| in that
>>>>> Chromium code review.  Manually stripping pointers and references is doable
>>>>> but 1) it is easy to get wrong (i.e. strip only one level) and 2) it is
>>>>> questionable whether other required stripping is not missed (i.e. auto
>>>>> type?),
>>>>> Today hasDeclaration doesn't match "the outermost can in our russian
>>>>> doll" as suggested (or proposed) by Manual.  Today desugaring is done in an
>>>>> arbitrary order of types - some (deep) types will be preferred over other
>>>>> (shallow) types as shown in an example in the separate doc.  In the example,
>>>>> even though the typeAliasDecl is the most shallow desugaring result, it will
>>>>> *never* be matched because hasDeclaration will first try (and latch onto)
>>>>> other types first.
>>>>> "has" vs "hasDescendant" is not a perfectly valid analogy.
>>>>> "hasDeclaration" is not like "has" today, because even today it can strip
>>>>> multiple layers of sugar (see the example from the previous bullet item).
>>>>>
>>>>> So:
>>>>>
>>>>> I would vote to make "hasDeclaration" match at every desugaring level
>>>>> (i.e. without splitting it into a "has" and a "hasDescendant"-like variant).
>>>>> Having "hasDeclaration" and "eventuallyDesugarsToDeclaration"
>>>>> ("hasDeepDeclaration"?) sounds like an okay option, but it seems to me that
>>>>> even today 1) hasDeclaration code and 2) users of hasDeclaration, behave
>>>>> like or expect behavior of "eventuallyDesugarsToDeclaration"
>>>>
>>>> I'm still looking into where we made the fundamental error that we're
>>>> running into those discussions now. There should be something simpler so
>>>> that the behavior more clearly matches the design of the clang AST.
>>>>>
>>>>> I hope that the discussion of "hasDeclaration" vs "hasDeepDeclaration"
>>>>> can be independent from, and not block the fixes from
>>>>> https://reviews.llvm.org/D24361 (i.e. being able to land them in the next
>>>>> 1-2 would be great).
>>>>
>>>> As Richard noted on the doc, it really depends on which direction we
>>>> want to go in, and whether the patch is a step in the right direction. Sorry
>>>> :(
>>>> But please continue pinging / making sure we drive this forward - I
>>>> think this is important, and I'm happy to spend some time to make sure we
>>>> get this right, and thanks for all the work you've already put into this, I
>>>> really appreciate it, and I think it's helping us getting to the root of the
>>>> problem.
>>>>
>>>>>
>>>>>
>>>>> On Thu, Oct 27, 2016 at 1:12 AM, Manuel Klimek <klimek at google.com>
>>>>> wrote:
>>>>>>
>>>>>> +ben & sam
>>>>>>
>>>>>> On Wed, Oct 26, 2016 at 6:50 PM Łukasz Anforowicz <lukasza at google.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> On Wed, Oct 26, 2016 at 9:46 AM, Łukasz Anforowicz
>>>>>>> <lukasza at google.com> wrote:
>>>>>>>>
>>>>>>>> So - is it fair to say that the CL under review can proceed as-is
>>>>>>>> (*) given that:
>>>>>>>>
>>>>>>>> The ordering problem has existed *before* the CL under review
>>>>>>>> (admittedly the CL under review adds 2 more cases to the list of problematic
>>>>>>>> ordering scenarios)
>>>>>>>> Below it seems that we have a promising direction for solving the
>>>>>>>> ordering problem (the solution probably requires a slightly bigger CL - one
>>>>>>>> that also tackles the question of what hasDeclaration should do about
>>>>>>>> AutoType, PointerType, etc.)
>>>>>>>>
>>>>>>>> -Lukasz
>>>>>>>>
>>>>>>>> (*) I know that I need to add documentation changes to mention
>>>>>>>> ElaboratedType
>>>>>>>>
>>>>>>>> On Tue, Oct 25, 2016 at 5:02 PM, Richard Smith
>>>>>>>> <richardsmith at google.com> wrote:
>>>>>>>>>
>>>>>>>>> On 25 October 2016 at 16:47, Łukasz Anforowicz <lukasza at google.com>
>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>> I think I understand the concern - the concern is that desugaring
>>>>>>>>>> done in HasDeclarationMatcher::matchesSpecialized(const QualType&, ...)
>>>>>>>>>> *latches* onto desugared types in *arbitrary* order.  I agree that this is a
>>>>>>>>>> problem (most obvious for me for the typeAliasDecl and
>>>>>>>>>> classTemplateSpecializationDecl case).  I agree that the CL adds a new case
>>>>>>>>>> to the set of ordered desugaring attempts done by this method (and therefore
>>>>>>>>>> can be seen as making the old problem slightly worse).
>>>>>>>>>>
>>>>>>>>>> I think this problem can be fixed by getting rid of the *order*
>>>>>>>>>> inside HasDeclarationMatcher::matchesSpecialized(const QualType&, ...) -
>>>>>>>>>> instead of:
>>>>>>>>>>
>>>>>>>>>> if (auto *TD = Node->getAsTagDecl())
>>>>>>>>>>   return matchesDecl(TD, Finder, Builder);
>>>>>>>>>> else if (auto *TT = Node->getAs<TypedefType>())
>>>>>>>>>>   return matchesDecl(TT->getDecl(), Finder, Builder);
>>>>>>>>>>
>>>>>>>>>> /* the CL under review would add ElaboratedType and
>>>>>>>>>>    TemplateSpecializationType somewhere here */
>>>>>>>>>>
>>>>>>>>>> ...
>>>>>>>>>> else if (auto *ICNT = Node->getAs<InjectedClassNameType>())
>>>>>>>>>>   return matchesDecl(ICNT->getDecl(), Finder, Builder);
>>>>>>>>>> return false;
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> have something like this:
>>>>>>>>>>
>>>>>>>>>>     if (auto *TD = Node->getAsTagDecl()) {
>>>>>>>>>>       if (matchesDecl(TD, Finder, Builder))
>>>>>>>>>>         return true;
>>>>>>>>>>     }
>>>>>>>>>>
>>>>>>>>>>     if (auto *TT = Node->getAs<TypedefType>()) {
>>>>>>>>>>       if (matchesDecl(TT->getDecl(), Finder, Builder))
>>>>>>>>>>         return true;
>>>>>>>>>>     }
>>>>>>>>>>
>>>>>>>>>>     if (auto *ET = Node->getAs<ElaboratedType>()) {
>>>>>>>>>>       if (matchesSpecialized(ET->getNamedType(), Finder, Builder))
>>>>>>>>>>         return true;
>>>>>>>>>>     }
>>>>>>>>>>
>>>>>>>>>>     if (auto *TST = Node->getAs<TemplateSpecializationType>()) {
>>>>>>>>>>       if (matchesSpecialized(*TST, Finder, Builder))
>>>>>>>>>>         return true;
>>>>>>>>>>     }
>>>>>>>>>> ...
>>>>>>>>>>     if (auto *ICNT = Node->getAs<InjectedClassNameType>()) {
>>>>>>>>>>       if (matchesDecl(ICNT->getDecl(), Finder, Builder))
>>>>>>>>>>         return true;
>>>>>>>>>>     }
>>>>>>>>>>
>>>>>>>>>>     return false;
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I think this is an interesting idea. It would certainly address my
>>>>>>>>> concerns. (You'd actually want something slightly different from the above,
>>>>>>>>> I think: given
>>>>>>>>>
>>>>>>>>>   typedef int A;
>>>>>>>>>   typedef A B;
>>>>>>>>>   B x;
>>>>>>>>>
>>>>>>>>> ...
>>>>>>>>> varDecl(hasType(qualType(hasDeclaration(typedefNameDecl().bind("TD")))))
>>>>>>>>> should match x /twice/: once with TD pointing at A and once with it pointing
>>>>>>>>> at B. So you probably want to repeatedly single-step desugar the type and
>>>>>>>>> try extracting a declaration from each level of type sugar.)
>>>>>>>>
>>>>>>>>
>>>>>>>> Oh, good point - I haven't thought about this scenario.  I think
>>>>>>>> this direction (based on getAsSuger from Type.cpp) look promising (passes a
>>>>>>>> test with 2 chained typedefs):
>>>>>>>>
>>>>>>>> template <typename T, typename DeclMatcherT>
>>>>>>>> class HasDeclarationMatcher : public WrapperMatcherInterface<T> {
>>>>>>>> ...
>>>>>>>>
>>>>>>>>   /// \brief Extracts the TagDecl of a QualType and returns whether
>>>>>>>> the inner
>>>>>>>>   /// matcher matches on it.
>>>>>>>>   bool matchesSpecialized(const QualType &Node, ASTMatchFinder
>>>>>>>> *Finder,
>>>>>>>>                           BoundNodesTreeBuilder *Builder) const {
>>>>>>>>     if (Node.isNull())
>>>>>>>>       return false;
>>>>>>>>
>>>>>>>>     // Do not use getAs<TemplateTypeParmType> instead of the direct
>>>>>>>> dyn_cast.
>>>>>>>>     // Calling getAs will return the canonical type, but that type
>>>>>>>> does not
>>>>>>>>     // store a TemplateTypeParmDecl. We *need* the uncanonical type,
>>>>>>>> if it is
>>>>>>>>     // available, and using dyn_cast ensures that.
>>>>>>>>     if (auto *TTP =
>>>>>>>> dyn_cast<TemplateTypeParmType>(Node.getTypePtr())) {
>>>>>>>>       if (matchesDecl(TTP->getDecl(), Finder, Builder))
>>>>>>>>         return true;
>>>>>>>>     }
>>>>>>>>
>>>>>>>>     const Type* Cur = Node.getTypePtr();
>>>>>>>>     while (Cur) {
>>>>>>>>       switch (Cur->getTypeClass()) {
>>>>>>>> #define ABSTRACT_TYPE(Class, Parent)
>>>>>>>> #define TYPE(Class, Parent) \
>>>>>>>>       case Type::Class: { \
>>>>>>>>         const Class##Type *Ty = cast<Class##Type>(Cur); \
>>>>>>>>         if (matchesSpecialized(*Ty, Finder, Builder)) \
>>>>>>>>           return true; \
>>>>>>>>         if (!Ty->isSugared()) return false; \
>>>>>>>>         Cur = Ty->desugar().getTypePtr(); \
>>>>>>>>         break; \
>>>>>>>>       }
>>>>>>>> #include "clang/AST/TypeNodes.def"
>>>>>>>>       }
>>>>>>>>     }
>>>>>>>>
>>>>>>>>     return false;
>>>>>>>>   }
>>>>>>>>
>>>>>>>>
>>>>>>> BTW: I have to note that the code above is potentially O(N^2) where N
>>>>>>> is the length of the desugaring chain (because each desugaring step can
>>>>>>> potentially call back into the QualType overload).  This is obviously
>>>>>>> suboptimal.  We probably need to discuss this when we review / talk about
>>>>>>> landing something like this.
>>>>>>
>>>>>>
>>>>>> It's basically the same difference between has() and hasDescendant.
>>>>>> I think
>>>>>> a) if we go that route, we should make it explicit by having a matcher
>>>>>> that has it in its name
>>>>>> b) we need to be able to memoize at each step, so this needs to become
>>>>>> part of the visitor interface of the matchers; given the sheer number of
>>>>>> types this seems like it'll get rather complex
>>>>>>
>>>>>> I'll make sure to discuss this with Richard etc al in person next week
>>>>>> when we're at LLVM conf.
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> but it opens a whole bunch of questions - what to do with other
>>>>>>>> types - for example should hasDeclaration "jump over / drill holes in paint
>>>>>>>> cans" for:
>>>>>>>>
>>>>>>>> AutoType, TypeOfExprType, DecltypeType, TypeOfType
>>>>>>>> LValueReferenceType, ValueReferenceType
>>>>>>>> PointerType, ReferenceType
>>>>>>>> And all the other types where I had to add a matchSpecialized
>>>>>>>> overload for so that things compile (some of these obviously don't have a
>>>>>>>> decl [and therefore shouldn't match hasDeclaration(...)] - like BuiltinType,
>>>>>>>> but I am listing all of them for completeness): AtomicType, PipeType,
>>>>>>>> ObjCObjectPointerType, ObjCObjectType, PackExpansionType,
>>>>>>>> DependentTemplateSpecializationType, DependentNameType, AttributedType,
>>>>>>>> UnaryTransformType, SubstTemplateTypeParmPackType,
>>>>>>>> SubstTemplateTypeParmType, AdjustedType, DecayedType, ParenType,
>>>>>>>> FunctionNoProtoType, FunctionProtoType, ExtVectorType, VectorType,
>>>>>>>> DependentSizedExtVectorType, DependentSizedArrayType, VariableArrayType,
>>>>>>>> IncompleteArrayType, ConstantArrayType, MemberPointerType, BlockPointerType,
>>>>>>>> ComplexType, BuiltinType,
>>>>>>>>
>>>>>>>> Also - I am not quite sure if I understand if special-casing of
>>>>>>>> TemplateTypeParmType is needed.  It probably is (because here we use
>>>>>>>> dyn_cast rather than cast).  At any rate drilling into details here can
>>>>>>>> probably wait until a real review in Phabricator...
>>>>>>>>
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> The code above passes all tests from the clang-test ninja target +
>>>>>>>>>> results in the following behavior for the 2 cases pointed out in an email:
>>>>>>>>>>
>>>>>>>>>> TEST(HasDeclaration, Case1FromEmail) {
>>>>>>>>>>   std::string input =
>>>>>>>>>>       "template<typename T>\n"
>>>>>>>>>>       "class Template {\n"
>>>>>>>>>>       " public:\n"
>>>>>>>>>>       "  void Method() {}\n"
>>>>>>>>>>       "};\n"
>>>>>>>>>>       "template <typename U>\n"
>>>>>>>>>>       "void Function(Template<U> param) {\n"
>>>>>>>>>>       "  param.Method();\n"
>>>>>>>>>>       "};\n";
>>>>>>>>>>
>>>>>>>>>>   // param type has-declaration matches class template decl?
>>>>>>>>>>   EXPECT_TRUE(matches(input, parmVarDecl(
>>>>>>>>>>       hasName("param"),
>>>>>>>>>>       hasType(qualType(
>>>>>>>>>>
>>>>>>>>>> hasDeclaration(decl(classTemplateDecl(hasName("Template")))))))));
>>>>>>>>>>
>>>>>>>>>>   // param type has-declaration matches class decl?
>>>>>>>>>>   EXPECT_FALSE(matches(input, parmVarDecl(
>>>>>>>>>>       hasName("param"),
>>>>>>>>>>       hasType(qualType(
>>>>>>>>>>
>>>>>>>>>> hasDeclaration(decl(cxxRecordDecl(hasName("Template")))))))));
>>>>>>>>>> }
>>>>>>>>>>
>>>>>>>>>> TEST(HasDeclaration, Case2FromEmail) {
>>>>>>>>>>   std::string input =
>>>>>>>>>>       "template<typename T>\n"
>>>>>>>>>>       "class Template {\n"
>>>>>>>>>>       " public:\n"
>>>>>>>>>>       "  void Method() {}\n"
>>>>>>>>>>       "};\n"
>>>>>>>>>>       "void Function(Template<int> param) {\n"
>>>>>>>>>>       "  param.Method();\n"
>>>>>>>>>>       "};\n";
>>>>>>>>>>
>>>>>>>>>>   // param type has-declaration matches class template decl?
>>>>>>>>>>   EXPECT_TRUE(matches(input, parmVarDecl(
>>>>>>>>>>       hasName("param"),
>>>>>>>>>>       hasType(qualType(
>>>>>>>>>>
>>>>>>>>>> hasDeclaration(decl(classTemplateDecl(hasName("Template")))))))));
>>>>>>>>>>
>>>>>>>>>>   // param type has-declaration matches class decl?
>>>>>>>>>>   EXPECT_TRUE(matches(input, parmVarDecl(
>>>>>>>>>>       hasName("param"),
>>>>>>>>>>       hasType(qualType(
>>>>>>>>>>
>>>>>>>>>> hasDeclaration(decl(cxxRecordDecl(hasName("Template")))))))));
>>>>>>>>>> }
>>>>>>>>>>
>>>>>>>>>> If the test expectations above look reasonable, then we can just
>>>>>>>>>> discuss how to land the code changes above (e.g. as a single CL or 2 CLs - I
>>>>>>>>>> would prefer 2 separate CLs).  Otherwise, let's talk about the test
>>>>>>>>>> expectations - what should they look like / how should they be different
>>>>>>>>>> from the test snippet above?
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Tue, Oct 25, 2016 at 1:42 PM, Richard Smith
>>>>>>>>>> <richardsmith at google.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>> On 25 October 2016 at 01:57, Manuel Klimek <klimek at google.com>
>>>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> On Tue, Oct 25, 2016 at 3:25 AM Richard Smith
>>>>>>>>>>>> <richardsmith at google.com> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 24 October 2016 at 16:41, Łukasz Anforowicz
>>>>>>>>>>>>> <lukasza at google.com> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I am not sure if we are making progress, but so far, we have
>>>>>>>>>>>>>> seen 1) test cases that are broken and fixed by the CL under review and 2)
>>>>>>>>>>>>>> test cases that are not changed by the CL under review and 3) no test cases
>>>>>>>>>>>>>> that are made worse by the CL under review.  I've tried to gather the
>>>>>>>>>>>>>> scenarios and the associated test cases in
>>>>>>>>>>>>>> https://docs.google.com/document/d/1y1F6uKdulluYBFch680g6bmHe_zDe8mHFYjfXRoakNI/edit.
>>>>>>>>>>>>>> Could you please take a look and shout if I misrepresented anything (i.e.
>>>>>>>>>>>>>> whether we agree that a specific test scenario shows a bug).
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I've also added some comments below / inline.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Mon, Oct 24, 2016 at 3:54 PM, Richard Smith
>>>>>>>>>>>>>> <richardsmith at google.com> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 24 October 2016 at 15:25, Łukasz Anforowicz
>>>>>>>>>>>>>>> <lukasza at google.com> wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On Mon, Oct 24, 2016 at 2:21 PM, Richard Smith
>>>>>>>>>>>>>>>> <richardsmith at google.com> wrote:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> On 24 October 2016 at 11:23, Łukasz Anforowicz
>>>>>>>>>>>>>>>>> <lukasza at google.com> wrote:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> I've run a few more experiments and I see that before any
>>>>>>>>>>>>>>>>>> changes:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> (1) Presence of ElaboratedType means that hasDeclaration
>>>>>>>>>>>>>>>>>> doesn’t match *anything*.  This is clearly wrong.  I still want to fix that.
>>>>>>>>>>>>>>>>>> (2) Both examples from Richard’s email consistently match
>>>>>>>>>>>>>>>>>> the class template decl (and not the class decl).  If Richard’s expectations
>>>>>>>>>>>>>>>>>> are not met *prior* to my CL, then I don’t think these expectations should
>>>>>>>>>>>>>>>>>> be fixed by my CL.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> OTOH, maybe it is worth to open a bug to investigate this
>>>>>>>>>>>>>>>>>> more - personally I would argue that hasDeclaration should match *any* decl
>>>>>>>>>>>>>>>>>> - for example any of type alias / template class decl / class decl.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> (3) If desugaring can result in both
>>>>>>>>>>>>>>>>>> TemplateSpecializationType and TypedefType, then only one is arbitrarily
>>>>>>>>>>>>>>>>>> picked - see test case TypedefTypeAndTemplateSpecializationTypeAreBothHere.
>>>>>>>>>>>>>>>>>> Again - this problem exists *prior* to my CL.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> So - my plan right now is to just simplify/revert my
>>>>>>>>>>>>>>>>>> patches so they only fix (1).  I don't think discussion of (2) should impact
>>>>>>>>>>>>>>>>>> the CL, because the behavior that disagrees with Richard's expectations is
>>>>>>>>>>>>>>>>>> happening *before* my changes.  I will also not attempt to fix (3), because
>>>>>>>>>>>>>>>>>> this starts to interfere with (2) - sorry :-/.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> QUESTION: Does the plan from the previous paragraph sound
>>>>>>>>>>>>>>>>>> good to you?
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Yes; I agree that failing to look through ElaboratedType
>>>>>>>>>>>>>>>>> here is just a bug.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Ack.  I've marked "elaborated type" section in the doc as
>>>>>>>>>>>>>> something that is improved by the CL under review.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Experiments before my changes were done with the following
>>>>>>>>>>>>>>>>>> unit tests:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> // This testcase shows that Richard’s expectations are not
>>>>>>>>>>>>>>>>>> met *prior* to my CL.
>>>>>>>>>>>>>>>>>> // (i.e. the testcase below passes before my CL;  the
>>>>>>>>>>>>>>>>>> expectations in the testcase
>>>>>>>>>>>>>>>>>> // are the opposite of Richard’s expectations).
>>>>>>>>>>>>>>>>>> TEST(HasDeclaration, Case1FromEmail) {
>>>>>>>>>>>>>>>>>>   std::string input =
>>>>>>>>>>>>>>>>>>       "template<typename T>\n"
>>>>>>>>>>>>>>>>>>       "class Template {\n"
>>>>>>>>>>>>>>>>>>       " public:\n"
>>>>>>>>>>>>>>>>>>       "  void Method() {}\n"
>>>>>>>>>>>>>>>>>>       "};\n"
>>>>>>>>>>>>>>>>>>       "template <typename U>\n"
>>>>>>>>>>>>>>>>>>       "void Function(Template<U> param) {\n"
>>>>>>>>>>>>>>>>>>       "  param.Method();\n"