Add a warning for large unrolled loops

[lwaern-intel]

This is both a proper PR and also just a Jenkins Pretest target to figure out how bad the issue is on VP

[syssimics]

Verification #12467586: pass

[syssimics]

Verification #13501484: ✅ pass

[mandolaerik]

enabled by

[mandolaerik]

space around binop

[lwaern-intel]

This is a stylistic choice we can quibble about: when combining + and *, I actually prefer omitting spaces around *; I feel that increases readability re. operator precedence. I see it as similar to implicit multiplication in math via concatenation (2(3)).

[mandolaerik]

Here we don't combine + and *. but we do combine * and !=.

And I agree that PEP-8 is not always optimal; in particular, I find f(foo=bar == 2) a bit weird. But I just accept and obey the style for consistency, unless there's strong reasons not to.

[lwaern-intel]

Here we don't combine + and *

I mean... we are, though the + is on a new line. That's good enough for readability, I admit. Though that doesn't apply to the below '#'*(site.dml_version() != (1, 2)) + 'foreach',

we do combine * and !=`.

Not on the same syntactic level, the != is within an (); I'm talking about * and +, which is on the same syntactic level, and where operator precedence determines how associations are made. This is where I'm arguing that omitting spaces around * such that such parts are more obviously whole terms helps readability. To exemplify, compare

   bla_bla_blau * blib * blub_blub + glabradahadaba + blara * dabara
+ canyoutellthat * ihavegivenup + on * variable * naming

vs.

   bla_bla_blau*blib*blub_blub + glabradahadaba + blara*dabara
+ canyoutellthat*ihavegivenupon + on*variable*naming

And that does apply to '#'*(site.dml_version() != (1, 2)) + 'foreach', but not as strongly. If you write out:

'#' * (site.dml_version() != (1, 2)) + 'foreach'

It's not as obviously clear at a glance that the (site.dml_version() != (1, 2)) is used for controlling a term of the string concatenation sequence, as opposed to being a term in of itself. Of course, the counterargument to that would be parentheses...:

('#' * (site.dml_version() != (1, 2))) + 'foreach'

If you insist on spacing, I would do the above. even though i kinda hate it.

[mandolaerik]

Sorry I didn't see the + because of how GH crops things. Still, I find it strange to break the spacing rule when you have a binop within an operand where you do obey the spacing rule.

If you ask me what I prefer, then it's the normal python idiom for expressing when a value depends on a condition:

('' if version == (1, 2) else '#') + 'foreach'

I quite strongly dislike the use of bools for arithmetic on iterables. But I tolerate it since it doesn't break any agreed-upon style.

[mandolaerik]

space around binop

[mandolaerik]

better with a direct link, add <a id= on the destination if needed

[mandolaerik]

Oh, seems that messages.html#dt:wbigunroll would work out-of-the-box

[lwaern-intel]

whaaat
That's a thing? Wow.

[lwaern-intel]

Though validate_md_links.py hates it. I guess we could patch it to tolerate #dt:blabla if any line in the target file starts with * blabla?

[lwaern-intel]

Oh, seems that messages.html#dt:wbigunroll would work out-of-the-box

wait. What experiment did you do, that made you say this?
If we look at what messages_to_md.py does...

        f.write(f"  <dt><b>\n\n{fmt_message(m)} [{m.__name__}]</b></dt>\n")

Looking at this, I don't understand why the hyperlink syntax would pick out and work specifically with m.__name__.

If this doesn't work, maybe the better approach is to just tweak messages_to_md.py to generate those anchors, so we don't have to hand-write them.

[mandolaerik]

What experiment did you do, that made you say this?

I looked at the generated messages.html, and also typed file:///path/to/messages.html#dt:wsystemc into my web browser

[lwaern-intel]

... huh!

[mandolaerik]

that's somewhat weird, when you pass --X --no-X to a compiler, you expect that the one that appears last takes precedence.

In any case, I think we can drop this paragraph entirely; there is no essential difference and the main reason for a compat feature is for consistency with other things that change with API 8.

[lwaern-intel]

I agree with dropping the explanation of the difference; that was inherited from the description of suppress_WLOGMIXUP, but it doesn't bear repeating. Though, I think I should keep the mention that you can either use --warn=WBIGUNROLL or --no-compat=suppress_WBIGUNROLL.

[mandolaerik]

I should keep the mention that you can either use --warn=WBIGUNROLL or --no-compat=suppress_WBIGUNROLL.

Agreed