Skip to content

Conversation

@mandolaerik
Copy link

  • Refactor underscore ban
  • Remove old underscore ban
  • Ensure discard refs work with new API

and compat.discard_ref_shadowing not in dml.globals.enabled_compat):
report(ESYNTAX(site, '_', "reserved identifier"))
@prod_dml14
@lex.TOKEN(ident_rule('ident_or_underscore',
Copy link
Owner

Choose a reason for hiding this comment

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

Prefer ident_or_discard_ref.

Copy link
Author

Choose a reason for hiding this comment

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

Agree

Copy link
Owner

@lwaern-intel lwaern-intel left a comment

Choose a reason for hiding this comment

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

I want to investigate a bit more as to why I ran into reduce/reduce conflicts...

'ELLIPSIS' : '<i>"..."</i>',
'<empty>' : '&lt;empty&gt;'
'<empty>' : '&lt;empty&gt;',
'_' : '<b>_</b>'
Copy link
Owner

Choose a reason for hiding this comment

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

I prefer the token be named DISCARDREF.

Copy link
Author

Choose a reason for hiding this comment

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

Disagree weakly. Other keyword-formed tokens are named as the keyword.

'FOREACH', 'IN', 'IS', 'LAYOUT', 'LOCAL', 'LOG', 'SELECT',
'SIZEOFTYPE', 'TYPEOF', 'UNDEFINED', 'VECT', '_WARNING', 'WHERE',
'EACH', 'SESSION', 'SEQUENCE',
'EACH', 'SESSION', 'SEQUENCE', '_',
Copy link
Owner

Choose a reason for hiding this comment

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

It should be 1.4 exclusive, no? Way I went about it in my WIP work was, in dmllex14:

keywords_dml14['_'] = 'DISCARDREF'
tokens.add('DISCARDREF')

Copy link
Author

Choose a reason for hiding this comment

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

Sure, although somewhat 'tomato tomato'.

@lwaern-intel
Copy link
Owner

lwaern-intel commented Nov 17, 2023

@mandolaerik expression_member was the issue! We want to support _ as the name of a member of a layout or struct. We cant with this approach unless we split cdecl to cdecl and cdecl_for_variable(???) and tweak expression_member. In my WIP I failed to consider the cdecl issue, but did recognize the expression_member issue; when I tinkered with this (ident_decl/ident rather than ident/ident_or_underscore, and ident_decl just leverages the ident), I encountered a reduce/reduce conflict with statement_delay_hook_one_msg_param, as at:

after whatever -> [ident] $: m();

what was previously a shift/reduce conflict that could be resolved via %prec bind now becomes a reduce/reduce conflict, as [ident] can be reduced either to objident or ident_decl.

Your approach where both ident and ident_or_underscore have identical defs except ident_or_underscore also accepts _ blows up even harder, but I think has the same root issue of statement_delay_hook_one_msg_param and expression_member causing a reduce/reduce conflict instead of simply a shift/reduce one.

@lwaern-intel lwaern-intel force-pushed the lw/21584 branch 2 times, most recently from d56cfe8 to 7fe389c Compare November 30, 2023 12:18
@lwaern-intel lwaern-intel force-pushed the lw/21584 branch 5 times, most recently from dd7d5d7 to 9854d98 Compare March 3, 2025 13:48
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