Skip to content

Conversation

@Muxianesty
Copy link

What are the reasons/motivation for this change?

By installing Yosys includes with install-target we ultimately install frontends/ast/ast.h, which now includes frontends/verilog/verilog_location.h, yet verilog_location.h is not installed along with ast.h.

Explain how this is achieved.

frontends/verilog/verilog_location.h is specified as an explicit header file in Makefile to be installed at some prefix.

Make sure your change comes with tests. If not possible, share how a reviewer might evaluate it.

Just try to compile some C-code including frontends/ast/ast.h.

@widlarizer widlarizer self-assigned this Dec 3, 2025
@widlarizer
Copy link
Collaborator

Sure, but are you including ast.h in your plugin? If so, why? Ideally, I'd make sure ast.h is never installed instead. In 72a0380 I've removed the comment telling people they should use it in their own HDL frontends, because it's a terrible idea. It now instead tells you it's just support code for the existing read_verilog frontend

@Muxianesty
Copy link
Author

Muxianesty commented Dec 4, 2025

Sure, but are you including ast.h in your plugin? If so, why? Ideally, I'd make sure ast.h is never installed instead. In 72a0380 I've removed the comment telling people they should use it in their own HDL frontends, because it's a terrible idea. It now instead tells you it's just support code for the existing read_verilog frontend

Initially I used Yosys as a shared object-library in the context where it was required to proceed with the execution on Yosys failures - including ast.h was needed in order to manually clean up global objects (Yosys 0.44 btw) in a convoluted manner.

I did not know about the thing with the advice against including ast.h. If so, I suppose the MR is obsolete then.

@widlarizer
Copy link
Collaborator

I'd be interested to hear more about your use case and what makes failures (errors? exceptions?) problematic. Either as a thread on Discourse or as a feature request GitHub issue.

The fact it took so long until anybody complained about an unusable ast.h (missing verilog_location.h include) suggests we're fine not even installing ast.h but I'll get more eyes on this topic until I make any decisions about what to do here

@widlarizer widlarizer added the discuss needs further discussion on the YosysHQ discourse (https://yosyshq.discourse.group) label Dec 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

discuss needs further discussion on the YosysHQ discourse (https://yosyshq.discourse.group)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants