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

Do IOrigin wrapping around RangeToken, instead of inside its StartToken #5931

Open
wants to merge 88 commits into
base: master
Choose a base branch
from

Conversation

keyboardDrummer
Copy link
Member

@keyboardDrummer keyboardDrummer commented Nov 26, 2024

Description

  • A RangeToken no longer takes two IOrigins, which could be wrapped, but instead takes two Token instances. Wrapping now happens around the RangeToken instead of inside its StartToken.
  • Add the properties Token StartToken, Token EndToken and Token Center to IOrigin.
  • Add the IOrigin Origin property to INode, and remove the getter for the RangeToken property (I'll remove the setter in a follow-up). Any usages of RangeTokens getter are replaced with Origin. INode still has a .Tok as well, but we'll replace that in a follow-up PR with usages of Origin.
  • RangeToken now includes an optional field for containing the "Center". Replace most overrides of Node.Tok with assignments to RangeToken.Centre.

Future

  • Rename RangeToken to SourceOrigin. Rename RangeNode to OriginNode.
  • Replace usages of Node.tok/Tok with Node.Origin
  • Replace usages of TokenNode with OriginNode (which requires computing the start and end of expression in the parser already), making OriginWithComputedRange obsolete, and allowing OriginNode to be merged into Node
  • Remove token like fields from IOrigin, so users must always choose to use Start/End/Center.
  • Get rid of BoogieRangeToken
  • Inline MakeAutoGenerated and MakeRefined

How has this been tested?

There are several classes of small changes to reported locations. I have manually inspected all of these changes. Here's some of the categories:

  • Message starting with "Warning: This ensures clause is part of a bodyless method.". Previously the reported location was the center of the ensure's predicate, like the == in ensures a == b, and now it is the center of the ensures clause itself, which for now is the start token of the predicate. I would rather it be the keyword of the ensures itself, but that would have introduced more changes and I'd prefer to do that in another PR.
  • Message starting with "Warning: XXX with {:only}". Location was previously the start of {:only} but now it's the center, which apparently is the o of only.
  • Warning: unnecessary requires clause and "Warning: unnecessary (or partly unnecessary) assume statement" are now reported at the center of the predicate instead of the start

By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.

@keyboardDrummer keyboardDrummer changed the title Rename IToken to IOrigin and reduce messiness around 'token wrapping' Introduce IOrigin to reduce headache around tokens Nov 26, 2024
@keyboardDrummer keyboardDrummer changed the title Introduce IOrigin to reduce headache around tokens Introduce IOrigin to make the Dafny AST easier to work with Nov 26, 2024
@keyboardDrummer keyboardDrummer enabled auto-merge (squash) December 3, 2024 13:26
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.

1 participant