Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[lldb] Make module-ownership.mm test more robust against AST node ord… #1197

Conversation

Teemperor
Copy link

…ering

The current test is checking both the anonymous structs and the template
specializations in one FileCheck run, but the anonymous struct line can
partially match the AST dump of a template specialization, causing that
FileCheck won't match that same line later against the template specialization
check and incorrectly fails on that check. This only happens when the
template specialization node somehow ends up before the anonymous struct node.

This patch just puts the checks for the anonymous structs in their own FileCheck
run to prevent them from partially matching any other record decl.

Fixes rdar://62997926

(cherry picked from commit 710fa2c)

…ering

The current test is checking both the anonymous structs and the template
specializations in one FileCheck run, but the anonymous struct line can
partially match the AST dump of a template specialization, causing that
FileCheck won't match that same line later against the template specialization
check and incorrectly fails on that check. This only happens when the
template specialization node somehow ends up before the anonymous struct node.

This patch just puts the checks for the anonymous structs in their own FileCheck
run to prevent them from partially matching any other record decl.

Fixes rdar://62997926

(cherry picked from commit 710fa2c)
@Teemperor Teemperor requested a review from adrian-prantl May 8, 2020 11:45
@Teemperor
Copy link
Author

@swift-ci test

…test running again

The relevant output FileCheck is scanning in this test is as follows:

    CXXRecordDecl 0x7f96cf8239c8 <<invalid sloc>> <invalid sloc> imported in A.B <undeserialized declarations> struct definition
    <<DefinitionData boilerplate>>
    `-FieldDecl 0x7f96cf823b90 <<invalid sloc>> <invalid sloc> imported in A.B anon_field_b 'int'
    (anonymous struct)
    CXXRecordDecl 0x7f96cf823be8 <<invalid sloc>> <invalid sloc> imported in A.B struct

Before 710fa2c this test was passing by
accident as it had a -DAG suffix in the checks changed by this patch,
causing FileCheck to first match the last line of the output above
(instead of the first one), and then finding the FieldDecl above.
When I removed the -DAG suffix, FileCheck actually enforced the ordering
and started failing as the FieldDecl comes before the CXXRecordDecl match
we get.

This patch fixes the CXXRecordDecl check to find the first line of the output
above which caused FileCheck to also find the FieldDecl that follows. Also
gives the FieldDecl a more unique name to make name collisions less likely.

(cherry picked from commit 7283ec0)
@Teemperor
Copy link
Author

Teemperor commented May 8, 2020

@swift-ci test

I also cherry-picked a fix for the test that make it pass upstream, but weirdly this test passes on the stable branch even without as our lldb-test symbols -dump-clang-ast output has this weird twist in it:

Downstream output:

CXXRecordDecl 0x7fc6030319e8 <<invalid sloc>> <invalid sloc> imported in A.B struct
StructB
CXXRecordDecl 0x7fc6030317c8 <<invalid sloc>> <invalid sloc> imported in A.B <undeserialized declarations> struct definition
|-DefinitionData pass_in_registers aggregate standard_layout trivially_copyable pod trivial literal
| |-DefaultConstructor exists trivial needs_implicit
| |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
| |-MoveConstructor exists simple trivial needs_implicit
| |-CopyAssignment trivial has_const_param needs_implicit implicit_has_const_param
| |-MoveAssignment exists simple trivial needs_implicit
| `-Destructor simple irrelevant trivial needs_implicit
`-FieldDecl 0x7fc603031990 <<invalid sloc>> <invalid sloc> imported in A.B anon_field_b 'int'

Upstream output:

StructB
CXXRecordDecl 0x7f96cf8239c8 <<invalid sloc>> <invalid sloc> imported in A.B <undeserialized declarations> struct definition
|-DefinitionData pass_in_registers aggregate standard_layout trivially_copyable pod trivial literal
| |-DefaultConstructor exists trivial needs_implicit
| |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
| |-MoveConstructor exists simple trivial needs_implicit
| |-CopyAssignment trivial has_const_param needs_implicit implicit_has_const_param
| |-MoveAssignment exists simple trivial needs_implicit
| `-Destructor simple irrelevant trivial needs_implicit
`-FieldDecl 0x7f96cf823b90 <<invalid sloc>> <invalid sloc> imported in A.B anon_field_b 'int'
(anonymous struct)
CXXRecordDecl 0x7f96cf823be8 <<invalid sloc>> <invalid sloc> imported in A.B struct
AlsoInNamespace<int>

@Teemperor
Copy link
Author

@swift-ci test

@Teemperor Teemperor merged commit c30e8a5 into swiftlang:apple/stable/20200108 May 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants