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

Rule label cannot contain ` or whitespace #3596

Merged
merged 7 commits into from
Aug 28, 2023
Merged

Rule label cannot contain ` or whitespace #3596

merged 7 commits into from
Aug 28, 2023

Conversation

dwightguth
Copy link
Collaborator

Adds a check to the compiler that reports an error if a sentence label contains the backtick character or whitespace. This ought to allow the Maude backend to compile rule labels directly to Maude identifiers without some clumsy encoding system.

I am reasonably certain that it should not break anything to add this restriction.

@rv-jenkins rv-jenkins changed the base branch from master to develop August 23, 2023 18:28
@dwightguth dwightguth marked this pull request as ready for review August 25, 2023 16:05
Copy link
Collaborator

@Robertorosmaninho Robertorosmaninho left a comment

Choose a reason for hiding this comment

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

LGTM!

@dwightguth dwightguth enabled auto-merge (squash) August 25, 2023 16:15
checkLabel(sentence);
}

private static final Pattern whitespace = Pattern.compile("\\s");
Copy link
Contributor

Choose a reason for hiding this comment

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

Why didn't you include the backtick here?
You have to traverse the label twice now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it's still linear time, the string is unlikely to be long, and I doubt the impact on performance will be substantial. We can always address this later if need be. I suspect the amdahl's law on this will be pretty minimal though.

@dwightguth dwightguth disabled auto-merge August 28, 2023 17:23
@dwightguth dwightguth enabled auto-merge (squash) August 28, 2023 17:23
@dwightguth dwightguth merged commit 6c6f7f4 into develop Aug 28, 2023
@dwightguth dwightguth deleted the rulelabel branch August 28, 2023 17:31
@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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants