[clang-tools-extra] 702529d - [clang] Fix returning the underlying VarDecl as top-level decl for VarTemplateDecl.

[Hubert Tong via cfe-commits]

On Mon, Oct 12, 2020 at 9:43 AM Haojian Wu <hokein.wu at gmail.com> wrote:

> sorry, I'm looking at it.
>
No problem, thanks!


> On Mon, 12 Oct 2020 at 15:35, Hubert Tong <
> hubert.reinterpretcast at gmail.com> wrote:
>
>> ParsedASTTest.TopLevelDecls has not recovered on clang-ppc64le-rhel since
>> this went in (even when including f1bf41e433e196ecffcc4fb7cd04c58d48445425,
>> which is purported to fix buildbot failures from this commit).
>>
>> http://lab.llvm.org:8011/#/builders/57/builds/81
>>
>> On Mon, Oct 12, 2020 at 5:06 AM Haojian Wu via cfe-commits <
>> cfe-commits at lists.llvm.org> wrote:
>>
>>>
>>> Author: Haojian Wu
>>> Date: 2020-10-12T10:46:18+02:00
>>> New Revision: 702529d899c87e9268bb33d836dbc91b6bce0b16
>>>
>>> URL:
>>> https://github.com/llvm/llvm-project/commit/702529d899c87e9268bb33d836dbc91b6bce0b16
>>> DIFF:
>>> https://github.com/llvm/llvm-project/commit/702529d899c87e9268bb33d836dbc91b6bce0b16.diff
>>>
>>> LOG: [clang] Fix returning the underlying VarDecl as top-level decl for
>>> VarTemplateDecl.
>>>
>>> Given the following VarTemplateDecl AST,
>>>
>>> ```
>>> VarTemplateDecl col:26 X
>>> |-TemplateTypeParmDecl typename depth 0 index 0
>>> `-VarDecl X 'bool' cinit
>>>   `-CXXBoolLiteralExpr 'bool' true
>>> ```
>>>
>>> previously, we returned the VarDecl as the top-level decl, which was not
>>> correct, the top-level decl should be VarTemplateDecl.
>>>
>>> Differential Revision: https://reviews.llvm.org/D89098
>>>
>>> Added:
>>>
>>>
>>> Modified:
>>>     clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
>>>     clang/lib/Parse/ParseDecl.cpp
>>>
>>> Removed:
>>>
>>>
>>>
>>>
>>> ################################################################################
>>> diff  --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
>>> b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
>>> index 65d9cffeedc7..db23438766d2 100644
>>> --- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
>>> +++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
>>> @@ -57,6 +57,12 @@ MATCHER_P(DeclNamed, Name, "") {
>>>    return false;
>>>  }
>>>
>>> +MATCHER_P(DeclKind, Kind, "") {
>>> +  if (NamedDecl *ND = dyn_cast<NamedDecl>(arg))
>>> +    return ND->getDeclKindName() == Kind;
>>> +  return false;
>>> +}
>>> +
>>>  // Matches if the Decl has template args equal to ArgName. If the decl
>>> is a
>>>  // NamedDecl and ArgName is an empty string it also matches.
>>>  MATCHER_P(WithTemplateArgs, ArgName, "") {
>>> @@ -99,9 +105,15 @@ TEST(ParsedASTTest, TopLevelDecls) {
>>>      int header1();
>>>      int header2;
>>>    )";
>>> -  TU.Code = "int main();";
>>> +  TU.Code = R"cpp(
>>> +    int main();
>>> +    template <typename> bool X = true;
>>> +  )cpp";
>>>    auto AST = TU.build();
>>> -  EXPECT_THAT(AST.getLocalTopLevelDecls(),
>>> ElementsAre(DeclNamed("main")));
>>> +  EXPECT_THAT(
>>> +      AST.getLocalTopLevelDecls(),
>>> +      ElementsAreArray({AllOf(DeclNamed("main"), DeclKind("Function")),
>>> +                        AllOf(DeclNamed("X"),
>>> DeclKind("VarTemplate"))}));
>>>  }
>>>
>>>  TEST(ParsedASTTest, DoesNotGetIncludedTopDecls) {
>>>
>>> diff  --git a/clang/lib/Parse/ParseDecl.cpp
>>> b/clang/lib/Parse/ParseDecl.cpp
>>> index 3f314c59ade6..01a16575c239 100644
>>> --- a/clang/lib/Parse/ParseDecl.cpp
>>> +++ b/clang/lib/Parse/ParseDecl.cpp
>>> @@ -2195,6 +2195,7 @@ Decl
>>> *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
>>>
>>>    // Inform the current actions module that we just parsed this
>>> declarator.
>>>    Decl *ThisDecl = nullptr;
>>> +  Decl *OuterDecl = nullptr;
>>>    switch (TemplateInfo.Kind) {
>>>    case ParsedTemplateInfo::NonTemplate:
>>>      ThisDecl = Actions.ActOnDeclarator(getCurScope(), D);
>>> @@ -2205,10 +2206,12 @@ Decl
>>> *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
>>>      ThisDecl = Actions.ActOnTemplateDeclarator(getCurScope(),
>>>
>>> *TemplateInfo.TemplateParams,
>>>                                                 D);
>>> -    if (VarTemplateDecl *VT =
>>> dyn_cast_or_null<VarTemplateDecl>(ThisDecl))
>>> +    if (VarTemplateDecl *VT =
>>> dyn_cast_or_null<VarTemplateDecl>(ThisDecl)) {
>>>        // Re-direct this decl to refer to the templated decl so that we
>>> can
>>>        // initialize it.
>>>        ThisDecl = VT->getTemplatedDecl();
>>> +      OuterDecl = VT;
>>> +    }
>>>      break;
>>>    }
>>>    case ParsedTemplateInfo::ExplicitInstantiation: {
>>> @@ -2385,8 +2388,7 @@ Decl
>>> *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
>>>    }
>>>
>>>    Actions.FinalizeDeclaration(ThisDecl);
>>> -
>>> -  return ThisDecl;
>>> +  return OuterDecl ? OuterDecl : ThisDecl;
>>>  }
>>>
>>>  /// ParseSpecifierQualifierList
>>>
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201012/579cae30/attachment.html>