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.

Sign up for GitHub

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

Jump to bottom

internal: nkError semIdentVis/IdentWithPragma #767

Draft
wants to merge 5 commits into
base: devel
Choose a base branch
from
Draft

internal: nkError semIdentVis/IdentWithPragma #767

wants to merge 5 commits into from

Conversation

saem
Copy link
Collaborator

@saem saem commented Jun 18, 2023 •
edited
Loading

Summary

  • TBD: rework semIdentVis and friends to produce nkError instead or use localReport
  • what changed and how?
  • why are we changing it?

Details

  • info that couldn't fit into the summary
  • relevant details; tricky parts
  • anything else

Notes for Reviewers

  • been spinning my wheels on this for a while, might abandon this approach
  • clean-up the design/API, especially wrt symbol construction/operations and analysed AST
  • add tests for the various errors
  • write a better commit message

TODO/Thinking List

  • semIdentVis muddles AST sem, and evaling a symbol constructor
  • symbol construction wants to succeed regardless (for nimsuggest/check)
  • AST needs to capture the errors (for sempass2 output)
  • how to keep sem idempotent with pragma/export AST sticking around?
    • maybe semAfterMacroCall can wrap output in nkError?
  • the getSym/Recover API/procedures feel off

saem added 3 commits June 17, 2023 12:02
@saem
part way through
d4df5ac 
going to refactor:
- generate one `nkError` node
- contains a set of issue markers
- can generate more than one diagnostic message
@saem
new versions of semIdentVis/IdentWithPragma
76b6595 
Still not happy with the API and a few things are vague, to think about:
- `semIdentVis` muddles AST sem, and evaling a symbol constructor
- symbol construction wants to succeed regardless (for nimsuggest/check)
- AST needs to capture the errors (for `sempass2` output)
- how to keep sem idempotent with pragma/export AST sticking around?
  - maybe `semAfterMacroCall` can wrap output in `nkError`?
- the getSym/Recover proc feel off

Teasing this apart feels unnecessarily difficult, but perhaps it'll have
large knockon effects to various parts of sem. Especially when it comes
to clarifying semanticizing AST, symbolic, and type parts of sem.
@saem
silly formating clean-up; it was annoying
70d5c78
@saem saem added refactor Implementation refactor compiler/sem Related to semantic-analysis system of the compiler labels Jun 18, 2023
saem added 2 commits June 20, 2023 12:01
@saem
Merge remote-tracking branch 'origin/devel' into saem-sem-nkerror-sem…
6ec2b98 
…identvis
@saem
draft attempts at vairous approaches
92ffc05 
will clean-up later, but one takeaway is symbol producing `newSymG` that
places errors in the `.ast` field. That seems potentially cleaner.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
compiler/sem Related to semantic-analysis system of the compiler refactor Implementation refactor
Projects
None yet
Milestone
Semantic analysis phase refactoring
Development

Successfully merging this pull request may close these issues.
1 participant
@saem