Prefer short circuit and membership test #11
Conversation
| if | ||
| ((ND.TCB (Task_Entered).TS = WaitRendzv) | ||
| and then | ||
| (ND.TCB (Task_Entered).SUSPEND = Ident_Index_of_Called)) or |
There was a problem hiding this comment.
Didn't the tool miss here an or else opportunity?
There was a problem hiding this comment.
My suspicion is that the tool (conservatively) assumes that ND.TCB (Task_Entered).TS has a side effect,
and hence not always executing it (in case of "or else") will change the behaviour.
If I have time, I will double check!
| if | ||
| ((ND.TCB (Task_Entered).TS = WaitRendzv) | ||
| and then | ||
| (ND.TCB (Task_Entered).SUSPEND = Ident_Index_of_Called)) or |
There was a problem hiding this comment.
Again a lost or else, seemingly...
| if | ||
| ((ND.TCB (Task_Entered).TS = WaitRendzv) | ||
| and then | ||
| (ND.TCB (Task_Entered).SUSPEND = Ident_Index_of_Called)) or |
There was a problem hiding this comment.
Same pattern, same missed or else...
| @@ -238,12 +238,13 @@ package body HAC_Sys.PCode.Interpreter.Tasking is | |||
| Pop (ND); | |||
| H2 := CD.Tasks_Definitions_Count + 1; | |||
| H3 := Integer (Random (ND.Gen) * Float (H2)); | |||
| while H2 >= 0 and ND.TCB (H3).TS /= WaitSem and ND.TCB (H3).SUSPEND /= H1 | |||
| while H2 >= 0 and ND.TCB (H3).TS /= WaitSem | |||
There was a problem hiding this comment.
Here an and then could have been added, seemingly.
There was a problem hiding this comment.
When ND.TCB (H3) is not a function call (with possible side effects) but just an array access, it can indeed be changed.
|
Integrated a good part of the suggest changes. Thanks! Commit: 8318810. For cases like |
Dear HAC developers,
Dear Gautier de Montmollin,
Thinking about you request to change
into
I realized why we didn't have this find and replace pattern.
On my codebases, we always started with preferring short circuit operators.
See for more info adaic
When the code is changed and uses short circuit operators,
we apply the find-and-replace patterns to prefer membership tests.
So I exactly applied these two steps in succession on the hac archive.
And e.g. src/apps/hac_pkg.adb benefits from both!
Hopes this helps in making hac even better and easier to maintain!
Greetings,
Pierre
P.S. A pretty printer is applied to every rewrite to ensure that no warnings are introduced (such as line too long).
However not all your code seems to adhere to the pretty print settings as specified in your project file.
Problem detected and solved by Rejuvenation-Ada crate
