iree-org / iree-org/fusilli

[Misc] Make `emitAsm`, `getCompiledArtifact` private

Open
#13 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
25
Forks
16
PR merge metrics
No merged PRs in 30d

Description

Now that there's higher level API (e.g. compile, execute), I wanted to switch the low level ones to private (e.g. emitAsm, getCompiledArtifact etc). But the tests directly call those API on the graph objects. Looking for ideas to refactor them to work on private methods. I didn't like the approach of using friend classes here, especially if we're hard-coding the names of specific test classes in the Graph class.

If there are any tests that can be converted use the public API, that sounds ideal. I'm not sure if we can do that will all of them though. The getCompiledArtifact tests for caching behavior seem very difficult to test as caching should be opaque to the public facing API. For this problem, the solutions I've heard of are friend classes and #ifdefs here.

We could move the cache tests do "direct cache manipulation" i.e. rather than checking an out parameter on createCompiledArtifact we could wipe the cache programmatically then check that it's updated, or check that the contents haven't changed.

There was a long thread on catch2 github issues about #define private public, it was considered bad practice IIRC.
https://github.com/catchorg/Catch2/issues/258

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.