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

feat: Cache accessed source files #551

Merged
merged 1 commit into from
Oct 10, 2024
Merged

Conversation

mark-koch
Copy link
Collaborator

We used to store the entire source code that was used to build an AST as a field within every node of the AST. This was a hack so that we can render the source line from an AST node when an error occurs. With our new diagnostics system, we should do better than that!

With this PR, we cache the source of every file accessed by the compiler in a new SourceMap class. Then, we can look up the source associated with a span when rendering a diagnostic.

Note that we can't rely on Pythons linecache since we also need to keep track of cells in jupyter notebooks as individual "files".

@mark-koch mark-koch requested a review from a team as a code owner October 9, 2024 13:32
@mark-koch mark-koch requested review from doug-q and removed request for a team October 9, 2024 13:32
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 91.48936% with 4 lines in your changes missing coverage. Please review.

Project coverage is 90.22%. Comparing base (d671bb5) to head (40e8cf2).

Files with missing lines Patch % Lines
guppylang/definition/function.py 75.00% 2 Missing ⚠️
guppylang/span.py 83.33% 2 Missing ⚠️
Additional details and impacted files
@@                 Coverage Diff                  @@
##           feat/diagnostics     #551      +/-   ##
====================================================
+ Coverage             89.90%   90.22%   +0.32%     
====================================================
  Files                    61       61              
  Lines                  6347     6376      +29     
====================================================
+ Hits                   5706     5753      +47     
+ Misses                  641      623      -18     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@doug-q doug-q 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. I think the various parse methods that take and never use a SourceMap are various constructs that don't track their source location yet?

guppylang/definition/function.py Show resolved Hide resolved
@mark-koch
Copy link
Collaborator Author

I think the various parse methods that take and never use a SourceMap are various constructs that don't track their source location yet?

Yes, exactly!

@mark-koch mark-koch merged commit 7f07cc7 into feat/diagnostics Oct 10, 2024
2 checks passed
@mark-koch mark-koch deleted the diag/sources branch October 10, 2024 09:07
github-merge-queue bot pushed a commit that referenced this pull request Nov 11, 2024
This is the development branch for our new diagnostics infrastructure.
Tracked by #535.

*  #548
* #551
* #552 
* #553
* #586
* #588
* #587
* #589 
* #590
* #600
* #601
* #604 
* #605 
* #606
github-merge-queue bot pushed a commit that referenced this pull request Nov 12, 2024
🤖 I have created a release *beep* *boop*
---


## [0.13.0](v0.12.2...v0.13.0)
(2024-11-12)


### ⚠ BREAKING CHANGES

* `prelude` module renamed to `std`

### Features

* add `qubit` discard/measure methods
([#580](#580))
([242fa44](242fa44))
* Add `SizedIter` wrapper type
([#611](#611))
([2e9da6b](2e9da6b))
* conventional results post processing
([#593](#593))
([db96224](db96224))
* Improve compiler diagnostics
([#547](#547))
([90d465d](90d465d)),
closes [#551](#551)
[#553](#553)
[#586](#586)
[#588](#588)
[#587](#587)
[#590](#590)
[#600](#600)
[#601](#601)
[#606](#606)
* restrict result tag sizes to 256 bytes
([#596](#596))
([4e8e00f](4e8e00f)),
closes [#595](#595)


### Bug Fixes

* Mock guppy decorator during sphinx builds
([#622](#622))
([1cccc04](1cccc04))


### Documentation

* Add DEVELOPMENT.md
([#584](#584))
([1d29d39](1d29d39))
* Fix docs build ([#639](#639))
([bd6011c](bd6011c))


### Miscellaneous Chores

* Manually set last release commit
([#643](#643))
([b2d569b](b2d569b))


### Code Refactoring

* rename prelude to std
([#642](#642))
([1a68e8e](1a68e8e))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: Agustín Borgna <agustin.borgna@quantinuum.com>
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