generate_packets.py: Self-comparisons in the generated code
alienvalkyrie Yeah, I'd noticed that (relevant Python code is in Field.get_cmp(), toward the bottom where arrays are handled); main reason I haven't looked at changing it yet is that it will probably be less work after everything else is restructured. But it should be relatively easy to fix; would probably end up very similar to how Field.get_get_real() only conditionally emits the code that checks for array truncation.
One thing to think about is that these comparisons are immediately followed by if (!differ), so when replacing them with differ = FALSE; (which must not be omitted), it would be possible (and maybe necessary, to avoid warnings?) to get rid of that if as well (but its block must remain a block, since it declares variables).
alienvalkyrie alienvalkyrie Decided to do it now, since it's a very small change and doesn't actually require any serious kludge (get_cmp() has only one array case).
Reply To alienvalkyrie
Decided to do it now, since it's a very small change and doesn't actually require any serious kludge (get_cmp() has only one array case).
Is that only after all the refactoring you have already done, or could we have it also for older branches? (would potentially save some effort on whitelisting / filtering out / silencing errors from the generated file on all kind of build test environments)
alienvalkyrie Reply To cazfi
Is that only after all the refactoring you have already done, or could we have it also for older branches? (would potentially save some effort on whitelisting / filtering out / silencing errors from the generated file on all kind of build test environments)
It would certainly require a separate patch, but it shouldn't be difficult to whip one up. A bit ugly perhaps, but not difficult.
alienvalkyrie Reply To alienvalkyrie
it shouldn't be difficult to whip one up. A bit ugly perhaps, but not difficult.
Nevermind; it is more difficult, and the best solution I can come up with is pretty damn ugly. What scale of "some effort ... on all kind of build test environments" are we talking about here?
alienvalkyrie Found a less ugly way to do it by basically backporting one of the intermediate cleanup steps along with it.
Reply To alienvalkyrie
What scale of "some effort ... on all kind of build test environments" are we talking about here?
Predicting future to answer that ... isn't easy. At the moment the only issue I have is that on github runner mac/homebrew/meson/-Ddebug=true (-> -Werror) build fails, which I've resolved by building -Ddebug=false for now (obviously meaning that *all* the warnings would get unnoticed if it goes to CI like that). However, this is likely just indicative that newer/upcoming toolchains are going to trip on that part of generated code, so we may need to be implementing workarounds for many of them before EOL of S3_1 or even S3_0.
alienvalkyrie Reply To cazfi
we may need to be implementing workarounds for many of them before EOL of S3_1 or even S3_0.
Right, so probably even the ugly solution would've been worth it (not that this ended up being relevant, since the ugly solution wasn't necessary).
packets_gen.c has a lot of self-comparisons. Haven't looked at how hard it would be to generate the code without them (+ not introducing other compiler warnings, like 'unused variable', in the process).
First one that comes up with current master:
packets_gen.c:6145:20: warning: self-comparison always evaluates to false [-Wtautological-compare]
differ = (O_LAST != O_LAST);