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

Parser: do not define typedefs with invalid types #647

Merged
merged 1 commit into from
Mar 22, 2024

Conversation

ehaas
Copy link
Collaborator

@ehaas ehaas commented Mar 12, 2024

Putting invalid typedefs into the symbol table is problematic because if they get redefined, we try to render the original type (not possible for invalid types) for the error message.

Closes #645

@ehaas
Copy link
Collaborator Author

ehaas commented Mar 13, 2024

Looks like the zig compiler is crashing; happens on master for me as well:

* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x7feaf030a217)
  * frame #0: 0x0000000107873d4a zig`InternPool.indexToKey + 58
    frame #1: 0x00000001079abcc9 zig`type.Type.comptimeOnlyAdvanced + 89
    frame #2: 0x00000001079ac1ea zig`type.Type.comptimeOnlyAdvanced + 1402
    frame #3: 0x00000001079abd2f zig`type.Type.comptimeOnlyAdvanced + 191
    frame #4: 0x00000001079b20e4 zig`Sema.resolveStructAlignment + 292
    frame #5: 0x00000001078ca0ba zig`type.Type.abiAlignmentAdvanced + 3402
    frame #6: 0x00000001078c8fd1 zig`Value.getUnsignedIntAdvanced + 913
    frame #7: 0x0000000107cb26c6 zig`Sema.zirPtrType + 1846
    frame #8: 0x0000000107ab98b4 zig`Sema.analyzeBodyInner + 27220
    frame #9: 0x00000001079a975e zig`Sema.analyzeInlineBody + 30
    frame #10: 0x0000000107b3792b zig`Sema.resolveTypeFieldsStruct + 3643
    frame #11: 0x00000001079ac103 zig`type.Type.comptimeOnlyAdvanced + 1171
    frame #12: 0x00000001079abd2f zig`type.Type.comptimeOnlyAdvanced + 191
    frame #13: 0x0000000107ce0c27 zig`Sema.zirParam + 871
    frame #14: 0x0000000107acf610 zig`Sema.analyzeBodyInner + 116656
    frame #15: 0x00000001078ce608 zig`Module.semaDecl + 2904
    frame #16: 0x00000001077c8563 zig`Module.ensureDeclAnalyzed + 14195
    frame #17: 0x0000000107d223d2 zig`Sema.ensureDeclAnalyzed + 258
    frame #18: 0x0000000107d25d8d zig`Sema.analyzeDeclRefInner + 45
    frame #19: 0x0000000107ab602d zig`Sema.analyzeBodyInner + 12749
    frame #20: 0x0000000107ad0c4a zig`Sema.analyzeBodyInner + 122346
    frame #21: 0x0000000107ab32c5 zig`Sema.analyzeBodyInner + 1125
    frame #22: 0x00000001078ce608 zig`Module.semaDecl + 2904
    frame #23: 0x00000001077c8563 zig`Module.ensureDeclAnalyzed + 14195
    frame #24: 0x00000001078a6724 zig`Compilation.processOneJob + 2996
    frame #25: 0x00000001077e43f4 zig`Compilation.update + 15908
    frame #26: 0x00000001077f91c1 zig`main.updateModule + 417
    frame #27: 0x000000010783b41e zig`main.buildOutputType + 70830
    frame #28: 0x0000000107745fa5 zig`main.main + 6885
    frame #29: 0x0000000107744466 zig`main + 102

@Vexu
Copy link
Owner

Vexu commented Mar 14, 2024

Putting invalid typedefs into the symbol table is problematic because if they get redefined, we try to render the original type (not possible for invalid types) for the error message.

Wouldn't it be better to allow redefinitions if the previous type is invalid? Since there was already a hit it should be in the slow path as well.

Also I'm going to look into the crash now.

@Vexu
Copy link
Owner

Vexu commented Mar 14, 2024

Master branch is green so this should be unblocked.

Do not try to combine invalid types into typedefs or render error messages
with invalid types.

Closes Vexu#645
@ehaas ehaas force-pushed the invalid-typedef branch from 61f3c85 to c24a2ff Compare March 15, 2024 07:12
@ehaas
Copy link
Collaborator Author

ehaas commented Mar 19, 2024

@Vexu I forced pushed - is this what you had in mind by allowing redefinitions of invalid types? I believe they were always allowed; it's just that we issue an error diagnostic. So now I skip the diagnostic if the previous typedef was an invalid type.

Copy link
Owner

@Vexu Vexu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this what you had in mind by allowing redefinitions of invalid types? I believe they were always allowed; it's just that we issue an error diagnostic. So now I skip the diagnostic if the previous typedef was an invalid type.

It is.

src/aro/Type.zig Show resolved Hide resolved
@ehaas ehaas merged commit 2966281 into Vexu:master Mar 22, 2024
3 checks passed
@ehaas ehaas deleted the invalid-typedef branch March 22, 2024 06:34
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.

Typedef crash with duplicated invalid type
2 participants