chipsalliance / chipsalliance/Surelog

Set of exposed internal API is very wide. Impossible to guarantee stable API long term.

Open
#3,568 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
475
Forks
90
Avg merge
1h 39m
Merged PRs (30d)
37

Description

On `make install`, a very large amount of headers is exported. This makes it impossible for internal refactorings to ever be possible as we've spilled all the implementation details externally, and Hyrum's law will make it impossible to change.

We have a single header that should specify the API for Surelog: `Surelog/API/Surelog.h`. However, now there are many headers that with there mere existence in an installation all give the impression to provide a stable API. We should strive to provide any user-visible stuff only in there or have a very limited set of includes that we actually export and are comfortable to keep stable long-term.

There is a kitchen-sink include that is exported (Surelog/surelog.h) which should only include the limited set of files we need to export (and eventually, we should eliminate this header as it is hard to reason what the users will actually use. IWYU!).

I suggest the following steps

1. figure out what actually needs to be visible (I suspect something like error reporting, symbolId, PathId, symbol table, command line parser, maybe a few more)
2. Look through these headers and make sure they don't accidentally rely on other things. Possibly cleanup.
3. Add a `#warning` in the surelog. h header to encourage people to include what they need (IWYU).
4. In a first step, reduce the amount of things included in surelog.h as determined in 1 and possibly user feedback.
5. reduce the number of header exported in an `make install` to the relevant ones and pledge to keep them backward compatible long-term

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with Surelog/API/Surelog.h and the exported Surelog/surelog.h header, then inspect the make install rules that determine which headers are installed. Identify the minimal public header set, reduce the kitchen-sink includes, add the requested warning, and verify that installation exposes only the intended stable API.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
api, build-system
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.