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

Remove unused strict field from ParseInModule #3807

Merged
merged 3 commits into from
Nov 13, 2023
Merged

Conversation

Scott-Guest
Copy link
Contributor

@Scott-Guest Scott-Guest commented Nov 11, 2023

This PR removes the strict field in ParseInModule.

The field is safe to remove because, after eliminating some dead code,

  • The only use of strict in ParseInModule is in parseStringTerm
  • strict is (almost) always set to true
  • The only place where strict is set to false is through a few calls to RuleGrammarGenerator.getCombinedGrammar in KPrint. However, these usages only ever call getParsingModule and getExtensionModule on the constructed ParseInModule object, neither of which invokes parseStringTerm, so the value of strict here is actually irrelevant.

Note that no actual functionality is lost either - the only live usage of strict in parseStringTerm is now fully controlled by the inferSortChecks argument rather than strict && inferSortChecks.

@Scott-Guest Scott-Guest self-assigned this Nov 11, 2023
@Scott-Guest Scott-Guest marked this pull request as ready for review November 11, 2023 19:23
@Scott-Guest Scott-Guest changed the title Remove unused strict parameter from ParseInModule Remove unused strict field from ParseInModule Nov 11, 2023
Copy link
Contributor

@Baltoli Baltoli left a comment

Choose a reason for hiding this comment

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

Nice one!

@rv-jenkins rv-jenkins merged commit 9a8e0a7 into develop Nov 13, 2023
8 checks passed
@rv-jenkins rv-jenkins deleted the parse-cleanup branch November 13, 2023 18:03
@Baltoli Baltoli mentioned this pull request Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants