TritModel.cc uses abc kissat. Include the necessary header.

[hzeller]

No description provided.

[gemini-code-assist]

Code Review

This pull request adds the #include "sat/kissat/kissat.h" header to src/syn/src/ir/TritModel.cc. I have no feedback to provide as there are no review comments.

[povik]

Strictly speaking it's not necessary as we declare what we use here, and it's unlikely kissat's interface will ever change. By going the include way we'll pick up abc's global header (abc_global.h) with certain pragmas and macro definitions which might cause conflict, though we are exposed to abc's header in other parts of openroad anyway.

@hzeller what's motivating the include?

[hzeller]

Include what you use, essentially (also commonly known as IWYU).

It is dangerous to assume (and forward declare) a particular prototype (as it can change any time, and then you get weird issues), the best is to actually include the header, so we always include the corresponding headers to symbols. If there are issues with pragmas and macros, then we undef them or fix things upstream.

I stumbled upon it, as I ran the bant build cleaner and it suggested to remove @abc, as there is no header from abc included in any of the sources of //src/syn/src/ir, thus concluded that the abc dependency must be accidental and can be removed.

$ bant dwyu //src/syn/src/ir
buildozer 'remove deps @abc' //src/syn/src/ir

[povik]

I agree in general but here I think it's more likely we will run into some kind of issue with the abc headers than the kissat interface changing.

[hzeller]

What issue do you expect ? Things seem to compile nicely.

[povik]

to resolve bant false positive

[hzeller]

Note, it is not a false positive, it is an actual positive: no abc header is included anywhere, so no dependency is added.

(you can tell bant to ignore that and not issue a removal, but that is only really for messed up projects; I prefer to fix the project instead :) ).