Skip to content

Remove unnecessary .(string) casting and $$ = $1 from parser.y#825

Merged
kennytm merged 3 commits intomasterfrom
kennytm/use-ident-for-strings
Apr 27, 2020
Merged

Remove unnecessary .(string) casting and $$ = $1 from parser.y#825
kennytm merged 3 commits intomasterfrom
kennytm/use-ident-for-strings

Conversation

@kennytm
Copy link
Contributor

@kennytm kennytm commented Apr 22, 2020

What problem does this PR solve?

What is changed and how it works?

  1. If a rule always produces a string, make their type <ident> so we don't need to do $x.(string).
  2. Implicitly every expression without an action gets $$ = $1. So we remove all such actions.

parser.go is slightly reduced from 4,084,324 bytes to 4,079,780 bytes. No observable performance gain or loss.

Check List

Tests

  • Unit test

Code changes

Side effects

Related changes

@kennytm kennytm requested a review from a team April 22, 2020 16:47
@ghost ghost requested review from tangenta and removed request for a team April 22, 2020 16:47
@kennytm kennytm added status/PTAL type/enhancement New feature or request labels Apr 22, 2020
@codecov
Copy link

codecov bot commented Apr 22, 2020

Codecov Report

Merging #825 into master will decrease coverage by 0.17%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #825      +/-   ##
==========================================
- Coverage   78.36%   78.18%   -0.18%     
==========================================
  Files          40       40              
  Lines       14757    14638     -119     
==========================================
- Hits        11564    11445     -119     
  Misses       2512     2512              
  Partials      681      681              

@tiancaiamao
Copy link
Collaborator

Good job!

If a rule always produces a string, make their type so we don't need to do $x.(string).

This could avoid the interface{} => string conversion and it's beneficial for the performance.

LGTM

tangenta
tangenta previously approved these changes Apr 27, 2020
Copy link
Contributor

@tangenta tangenta left a comment

Choose a reason for hiding this comment

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

LGTM

@kennytm kennytm merged commit cd0d9fe into master Apr 27, 2020
@kennytm kennytm deleted the kennytm/use-ident-for-strings branch April 27, 2020 11:51
tiancaiamao pushed a commit to tiancaiamao/parser that referenced this pull request Apr 27, 2021
…ingcap#825)

* parser: use <ident> rather than <item> for string-only rules

This get rid of many .(string) casts.

* parser: get rid of unnecessary $$ = $1.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status/LGT2 LGT2 type/enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants