Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ILVerify check improvements #18090

Open
Martin521 opened this issue Nov 29, 2024 · 2 comments
Open

ILVerify check improvements #18090

Martin521 opened this issue Nov 29, 2024 · 2 comments

Comments

@Martin521
Copy link
Contributor

The new ILVerify check in CI certainly makes sense.

Here are just some inconveniences I met.

  • In most cases, there are just changes in line numbers.
  • In this case, there was probably some whitespace change at the end of the line, caused by a merge editor.

Can we assume "success" in these cases?

Or, more specific, implement this logic:

if baseline does not exist then
  run ilverify
  if TEST_UPDATE_BSL=1 then
    save output as .bsl
  else
    save output as .actual
  success
else
  run ilverify
  if output = baseline then
    success
  else
    show differences
    if TEST_UPDATE_BSL=1 then
      save output as .bsl
    else
      save output as .actual
    if output differs from baseline only in line numbers or line-end whitespace then
      success
    else
      failure

If that makes sense, I can support implementing it.

@vzarytovskii
Copy link
Member

I think it instead should be fixed. This particular issue, which is causing all those unexpected stack errors is fixable in one place in the compiler.

I considered ignoring line numbers, but decided not to. I'll leave it to the team to decide what to do.

@T-Gro
Copy link
Member

T-Gro commented Dec 2, 2024

@Martin521:

It would be definitely better to address the warnings it produces.

The line numbers are coming from compiler generated code , which includes the LOC number in the name. If anything code is added above, the name also changes.
If baseline differs only in a single number, but all other parts of the generated closure will be the same and also the amount of same named generated members will be the same (the numbers are also there to prevent duplicates) => this could work.

i.e. this part would be a good improvement if output differs from baseline only in line numbers or line-end whitespace then, at least until the underlying reason for ilverification failure is fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: New
Development

No branches or pull requests

4 participants