core: always print unregistered ops

[alexarice]

scf.for prints it's blocks without terminators if there are no iter_args. Unfortunately the check for whether an operation is a terminator returns true by default for unregistered operations (presumably so that they don't break verification), and so they don't get printed.

This change should make it so that these ops always get printed.

[alexarice]

@AntonLydike feel free to add your example as a test case

[alexarice]

If not I can make one

[superlopuh]

Code change looks good and a simple "unregistered_op"() : () -> () should work as a test, right?

[alexarice]

I added a small test, but in the process found that the implicit terminator stuff in scf.for doesn't roundtrip. It seems to me that the cause is the ensure_terminator function, which from my understanding of the docstring should add a terminator if it is not present, but it instead just throws a verify error

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.02%. Comparing base (a7d8ebe) to head (299dc70).
Report is 15 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3303   +/-   ##
=======================================
  Coverage   90.01%   90.02%           
=======================================
  Files         445      445           
  Lines       55926    55979   +53     
  Branches     5358     5365    +7     
=======================================
+ Hits        50341    50394   +53     
  Misses       4177     4177           
  Partials     1408     1408           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[alexarice]

@compor I believe you last touched this code, do you know what the intended behaviour is supposed to be?

[superlopuh]

Am I missing something?

Suggested change

	"unregistered_op"() : () -> ()
	scf.yield
	"unregistered_op"() : () -> ()

[alexarice]

You're missing that I believe this doesn't verify as the implicit terminator isn't added, due to an unrelated problem this pr was trying to solve. (scf.for expects to be terminated by an scf.yield)

[superlopuh]

I'm confused, does this test not pass on main?

[superlopuh]

OK I just tested it out myself and became more confused :) I think a part of the problem is that the code in the printer is a little backwards, and would benefit from a comment. It might also benefit from a bit of a refactor, it feels a little wasteful to check for terminator trait for every operation that we print, when they can only be the last operation in the block. That's probably out of scope for this PR though.

[superlopuh]

OK I just tested it out myself and became more confused :) I think a part of the problem is that the code in the printer is a little backwards, and would benefit from a comment. It might also benefit from a bit of a refactor, it feels a little wasteful to check for terminator trait for every operation that we print, when they can only be the last operation in the block. That's probably out of scope for this PR though.

[math-fehr]

Thanks for the fix!