Skip to content

Conversation

@cirras
Copy link
Collaborator

@cirras cirras commented Oct 8, 2024

In 5072681, we implemented keyword case-insensitivity directly into the grammar. This gave us a small performance boost, but quietly impaired our ability to track directiveNesting while lexing a source file.

We were using the case-sensitive String#startsWith to check for a particular compiler directive while lexing the token, which used to work because the input actually was lowercase before. Now, we need to do a case-insensitive comparison because the input is using the original casing.

This directiveNesting issue introduced some subtle issues in cases where:

  • we're only parsing the interface of a file (skipping the implementation section
  • implementation keywords are nested within conditional directives ($IF(DEF), $SLSE)
  • the conditional directives are not lowercase

Fixes #299.

@cirras cirras requested a review from fourls October 8, 2024 01:50
@cirras cirras force-pushed the skip_implementation_with_directive_nesting branch from 97244a5 to b570e33 Compare October 8, 2024 03:27
Copy link
Collaborator

@fourls fourls left a comment

Choose a reason for hiding this comment

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

I'm always suspicious when I can't find something to nitpick about... but looks good to me!

Copy link
Collaborator

@fourls fourls left a comment

Choose a reason for hiding this comment

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

Aha, found one! I think we need a changelog entry.

@cirras cirras force-pushed the skip_implementation_with_directive_nesting branch from b570e33 to ad83630 Compare October 8, 2024 05:57
@cirras cirras requested a review from fourls October 8, 2024 05:57
Copy link
Collaborator

@fourls fourls left a comment

Choose a reason for hiding this comment

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

Looks good.

@cirras cirras merged commit 0595ded into master Oct 8, 2024
2 checks passed
@cirras cirras deleted the skip_implementation_with_directive_nesting branch October 8, 2024 06:27
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.

Parsing error when interface section has invalid conditional blocks

3 participants