Skip to content

Conversation

@Dunqing
Copy link
Member

@Dunqing Dunqing commented Mar 27, 2025

part of #7278 and #7951

We had assumed Semantic adds a symbol id for symbol_id field that AST has, but declare function and declare class are exceptions, which would cause panicking somewhere. Add symbol id for declare function to fix the factor for panic.

Copy link
Member Author

Dunqing commented Mar 27, 2025


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@codspeed-hq
Copy link

codspeed-hq bot commented Mar 27, 2025

CodSpeed Instrumentation Performance Report

Merging #10078 will not alter performance

Comparing 03-27-feat_semantic_add_symbol_id_for_declare_function_binding (84a3490) with main (c903e42)

Summary

✅ 33 untouched benchmarks

@Dunqing Dunqing force-pushed the 03-27-feat_semantic_check_redeclaration_of_variable_declaration_and_function_declaration_in_the_block_scope branch from 1c741db to e81ae1e Compare March 27, 2025 14:12
@Dunqing Dunqing force-pushed the 03-27-feat_semantic_add_symbol_id_for_declare_function_binding branch 2 times, most recently from faf8870 to 93aeabc Compare March 27, 2025 14:34
@Dunqing Dunqing changed the base branch from 03-27-feat_semantic_check_redeclaration_of_variable_declaration_and_function_declaration_in_the_block_scope to graphite-base/10078 March 28, 2025 00:59
@Dunqing Dunqing force-pushed the 03-27-feat_semantic_add_symbol_id_for_declare_function_binding branch from 93aeabc to ccc8c9d Compare March 28, 2025 00:59
@Dunqing Dunqing changed the base branch from graphite-base/10078 to 03-28-refactor_semantic_align_handling_of_declaring_symbol_for_function_with_typescript March 28, 2025 00:59
@Dunqing Dunqing marked this pull request as ready for review March 28, 2025 01:42
@Dunqing Dunqing requested a review from Boshen as a code owner March 28, 2025 01:42
@graphite-app graphite-app bot force-pushed the 03-28-refactor_semantic_align_handling_of_declaring_symbol_for_function_with_typescript branch from c29f055 to 132631b Compare March 28, 2025 04:30
@graphite-app graphite-app bot requested a review from overlookmotel as a code owner March 28, 2025 04:30
@graphite-app graphite-app bot force-pushed the 03-27-feat_semantic_add_symbol_id_for_declare_function_binding branch from ccc8c9d to ffc5dbc Compare March 28, 2025 04:31
@Dunqing Dunqing force-pushed the 03-28-refactor_semantic_align_handling_of_declaring_symbol_for_function_with_typescript branch from 132631b to 7116b2c Compare March 28, 2025 05:18
@Dunqing Dunqing force-pushed the 03-27-feat_semantic_add_symbol_id_for_declare_function_binding branch from ffc5dbc to a514509 Compare March 28, 2025 05:19
graphite-app bot pushed a commit that referenced this pull request Mar 28, 2025
…ion declaration in the block scope (#10074)

close: #10050
close: #10051
close: #10055
close: #10056

This PR causes a lot of test262 tests to fail, all are expected. The reason is that the test262 test runner has a little incorrect, see #10057. And also produces a lot of duplicate errors, which will be removed in #10078.
@graphite-app graphite-app bot changed the base branch from 03-28-refactor_semantic_align_handling_of_declaring_symbol_for_function_with_typescript to graphite-base/10078 March 28, 2025 07:26
graphite-app bot pushed a commit that referenced this pull request Mar 28, 2025
…ion declaration in the block scope (#10074)

close: #10050
close: #10051
close: #10055
close: #10056

This PR causes a lot of test262 tests to fail, all are expected. The reason is that the test262 test runner has a little incorrect, see #10057. And also produces a lot of duplicate errors, which will be removed in #10078.
graphite-app bot pushed a commit that referenced this pull request Mar 28, 2025
…ion declaration in the block scope (#10074)

close: #10050
close: #10051
close: #10055
close: #10056

This PR causes a lot of test262 tests to fail, all are expected. The reason is that the test262 test runner has a little incorrect, see #10057. And also produces a lot of duplicate errors, which will be removed in #10078.
@graphite-app graphite-app bot force-pushed the graphite-base/10078 branch from 7116b2c to 5d829c2 Compare March 28, 2025 07:44
@graphite-app graphite-app bot force-pushed the 03-27-feat_semantic_add_symbol_id_for_declare_function_binding branch from a514509 to 6e5fca2 Compare March 28, 2025 07:44
@graphite-app graphite-app bot changed the base branch from graphite-base/10078 to main March 28, 2025 07:45
@graphite-app graphite-app bot force-pushed the 03-27-feat_semantic_add_symbol_id_for_declare_function_binding branch from 6e5fca2 to 268a827 Compare March 28, 2025 07:45
@graphite-app graphite-app bot added the 0-merge Merge with Graphite Merge Queue label Mar 29, 2025
@graphite-app
Copy link
Contributor

graphite-app bot commented Mar 29, 2025

Merge activity

part of #7278 and #7951

We had assumed `Semantic` adds a symbol id for `symbol_id` field that AST has, but `declare function` and `declare class` are exceptions, which would cause panicking somewhere. Add symbol id for `declare function` to fix the factor for panic.
@graphite-app graphite-app bot force-pushed the 03-27-feat_semantic_add_symbol_id_for_declare_function_binding branch from 268a827 to 84a3490 Compare March 29, 2025 14:15
graphite-app bot pushed a commit that referenced this pull request Mar 29, 2025
Part of #7278 and #7951, and the same as #10078 but for `declare class`
@graphite-app graphite-app bot merged commit 84a3490 into main Mar 29, 2025
28 checks passed
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Mar 29, 2025
@graphite-app graphite-app bot deleted the 03-27-feat_semantic_add_symbol_id_for_declare_function_binding branch March 29, 2025 14:26
This was referenced Apr 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-linter Area - Linter A-semantic Area - Semantic A-transformer Area - Transformer / Transpiler C-enhancement Category - New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants