dialects: (hw) Fix printing of module visibility

[Moxinilian]

While the visibility test was working, the practical roundtripping of private modules did not work properly. The reason for this is that there was a typo in the definition of the visibility attribute, which was cancelled out by the fact that roundtriping goes through generic format!

This seems to be a more fundamental issue of the testing infrastructure, so I am not sure how to add more tests that would cover this. @math-fehr @superlopuh

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.84%. Comparing base (ba1bdd4) to head (0b9490a).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2512   +/-   ##
=======================================
  Coverage   89.84%   89.84%           
=======================================
  Files         351      351           
  Lines       43769    43769           
  Branches     6529     6529           
=======================================
  Hits        39323    39323           
  Misses       3486     3486           
  Partials      960      960           

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

[PapyChacal]

Nice catch!
The only way to test generic syntax is to test it explicitly IMO.
I see those filechecks are using // RUN: XDSL_ROUNDTRIP and // CHECK:.
There are complementary helpers; you can just add a // RUN: XDSL_GENERIC_ROUNDTRIP and use // CHECK-GENERIC: wherever in the same file to test the generic syntax output specifically 🙂

So I don't see anything fundamental about the testing infrastructure rather than the test itself here; if you do, could you expand? I'm always curious about ways to improve it

[Moxinilian]

No I think you misunderstood the problem.

XDSL_ROUNDTRIP does xdsl-opt %s --print-op-generic --split-input-file | xdsl-opt --split-input-file. While xdsl-opt %s | xdsl-opt would have spotted the problem, this specific problem is such that doing xdsl-opt --print-op-generic | xdsl-opt hides it, because xDSL internals happen to not trigger the bug when the operation is newly parsed, and the generic format does not run the buggy printer.

For what it's worth, the hw tests already run XDSL_GENERIC_ROUNDTRIP as well.

[superlopuh]

I'm confused, which tests are broken, and what do they miss?

[superlopuh]

Ah, indeed, I'd expect it to not print the generic syntax. When I change the macro locally one test fails, but I assume it's for a different reason. Maybe we can change the macro?