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

Fix namespacing of hilti IDs in Spicy-side diagnostic output. #1902

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

rsmmr
Copy link
Member

@rsmmr rsmmr commented Oct 18, 2024

Fix namespacing of hilti IDs in Spicy-side diagnostic output.

We now show them with a spicy prefix, which makes more sense for
users.

This is a bit messy because our printer code (1) isn't aware of the
HILTI/Spicy distinction; (2) isn't differentiating between output
that's user-visible vs not; and (3) manages its current
parameterization a bit awkwardly through global state. So this
shoehorns a notion of user-visibility in there, and enables the Spicy
plugin to replace hilti:: with spicy:: when it deems that
necessary. Eventually we should overhaul the printer code to be more
modular and contextual.

  • Add printer plugin hook to customize ID printing.
  • Fix namespacing of hilti IDs in Spicy-side diagnostic output.

@rsmmr rsmmr requested a review from bbannier October 18, 2024 10:46
@bbannier bbannier linked an issue Oct 18, 2024 that may be closed by this pull request
bbannier
bbannier previously approved these changes Oct 18, 2024
hilti/toolchain/include/ast/node.h Show resolved Hide resolved
hilti/toolchain/include/compiler/printer.h Outdated Show resolved Hide resolved
hilti/toolchain/include/compiler/printer.h Outdated Show resolved Hide resolved
hilti/toolchain/src/compiler/printer.cc Outdated Show resolved Hide resolved
spicy/toolchain/src/compiler/printer.cc Show resolved Hide resolved
Because IDs are no longer AST nodes, we did not have a way for a
compiler plugin's printing code to modify how it would like them to be
printed. This adds a corresponding hook. It's not used anywhere yet,
but will be soon.

In addition, we add a notion of "user-visibility" to the printer API
so that printing code knows whether the resulting output is something
that will be shown to the user (e.g., in diagnostics), or remains
internal (e..g, raw code output, debugging output). Likewise not used
yet, but will be soon.

We also clean up the printer API a little bit.
We now show them with a `spicy` prefix, which makes more sense for
users.

Closes #1803.
@rsmmr rsmmr force-pushed the topic/robin/gh-1803-no-hilti branch from 4ebb7c9 to a76f920 Compare October 21, 2024 14:19
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.

Error message around <bytes>.decode(...) mention hilti::Charset
2 participants