Challenge 5 – Dead, unused, predetermined and suspicious code
The package Conductor defines an automated conductor for an orchestra, which is updated both periodically every few micro-seconds (entry Advance of task Concert) and each time a sensor detects that a musician plays a new note (entry Start_Of_Note of task Concert).
Have a look at the following code and see if you can spot the errors. Once you think you’ve got them all, click on the “Go CodePeer” button to see how well you matched up to CodePeer’s comprehensive and rapid analysis of the same code.
Error line 97: “high: conditional check raises exception here: checking that Part >= 1”
Here’s how CodePeer helps you coming to this conclusion: Local variable Part is initialized to zero, whereas array Orchestra starts at index one, hence the mismatch. In fact, the code here is dead, as the test for the loop will fail at the very first iteration. CodePeer is reporting that an error would occur if the execution was able to reach this program point.
Error line 114: “high: conditional check raises exception here: checking that Part >= 1 and Part <= num_parts.all”
Here’s how CodePeer helps you coming to this conclusion: Local variable Part is initialized to zero, whereas array Orchestra starts at index one, hence the mismatch. Contrary to the previous error, the code is not dead here, due to the different form of the loop test. CodePeer reports the same error on the following line 115 when accessing field Dur.
Error line 125: “warning: suspicious constant operation (min(…))/96 always evaluates to 0”
Here’s how CodePeer helps you coming to this conclusion: CodePeer computes a range of Score.Duration’First .. Score.Duration’Last – 1 for local variable Min from its initialization on line 122. Then, dividing Min by Score.Duration’Last can only result in value zero, which is clearly suspicious for an operation on non-constant operands. The problem stems from the inversion of the conversion to fixed-point type Position and the division on line 125, so that the division occurs in integers here instead of fixed-points.
Error line 156: “medium: condition predetermined because (Dur <= Score.Duration'last) is always true”
Here’s how CodePeer helps you coming to this conclusion: Local variable Dur is of type Score.Duration, hence it cannot exceed Score.Duration’Last. The problem here is that the programmer used Dur where Cumul_Dur was expected.
Error line 167: “warning: unused assignment into Dur”
Here’s how CodePeer helps you coming to this conclusion: Dur is a local variable, hence assigning to it is only useful if its value is read afterwards, which is not the case here. Instead of defining Dur as a local variable on line 143, it should be defined as a renaming like Pos on line 140.
-------------------- -- file score.ads -- -------------------- package Score is type Pitch is new Natural range 0 .. 127; -- MIDI note in the MIDI Tuning Standard, only representing semitones here type Long_Duration is new Natural; subtype Duration is Long_Duration range 0 .. 96; -- Fraction of a measure, 96 representing the complete measure, which -- allows representing from thirty-second note to whole note for measures -- with one to four beats type Position is delta 10.0**(-2) range 0.0 .. 10_000.0; -- Distance from the start of the score in number of measures type Instrument is (Flute, Recorder, Violin, Trumpet, Clarinet, Oboe, Cornet, Flugelhorn, Piccolo_Trumpet, Piccolo, Alto_Saxophone, Alto_Flute, Viola, Horn, Alto_Horn, Trombone, Tenor_Saxophone, Bass_Trumpet, Bassoon, English_Horn, Baritone_Saxophone, Baritone_Horn, Bass_Clarinet, Cello, Euphonium, Bass_Trombone, Contrabassoon, Bass_Saxophone, Double_Bass, Tuba, Contrabass_Saxophone, Contrabass_Bugle); type Interpreter is (First, Second, Third, Fourth, Fifth); procedure Get_Note (Inter : Interpreter; Instr : Instrument; Pos : Position; Fwd : Long_Duration; Pit : out Pitch; Dur : out Duration); -- Return the Pitch and Duration of the note starting forward by Fwd from -- position Pos for the Interpreter of the Instrument end Score; ------------------------ -- file conductor.ads -- ------------------------ with Score; use Score; package Conductor is type Positions is array (Positive range <>) of Position; type Durations is array (Positive range <>) of Score.Duration; type Part (Max_Players : Positive) is record Inter : Interpreter; -- Interpreter for this part Instr : Instrument; -- Instrument for this part Num_Players : Positive; -- Number of players in 1 .. Max_Players Pos : Positions (1 .. Max_Players); -- Current position of each player in the score Dur : Durations (1 .. Max_Players); -- Remaining duration of the current note for each player end record; type Parts is array (Positive range <>) of Part (5); task type Concert (Num_Parts : Positive) is -- Initialize the parts to be played entry Initialize (Initial_Parts : Parts); -- Signal that a player starts a note entry Start_Of_Note (Index : Positive; -- Index of the Part being played in the array Num : Positive; -- Number of the player in the Part Pit : Pitch); -- Pitch of the note -- Advance the position of all players by a Step, which is expected to -- be much smaller than the interval between two consecutive notes entry Advance (Step : Score.Duration); -- Start of the concert entry Play; end Concert; end Conductor; ------------------------ -- file conductor.adb -- ------------------------ package body Conductor is task body Concert is Orchestra : Parts (1 .. Num_Parts); begin accept Initialize (Initial_Parts : Parts) do declare Part : Integer := 0; begin while Part in Orchestra'Range loop Orchestra (Part) := Initial_Parts (Part); Part := Part + 1; end loop; end; end Initialize; accept Play; -- Start the concert loop select accept Advance (Step : Score.Duration) do declare Part : Integer := 0; begin while Part < Num_Parts loop declare Pos : Positions renames Orchestra (Part).Pos; Dur : Durations renames Orchestra (Part).Dur; begin for Index in Pos'Range loop declare -- Do not allow to advance beyond a new note as -- it is the role of Start_Of_Note to do so Min : constant Score.Duration := Score.Duration'Min (Step, Dur (Index) - 1); begin Pos (Index) := Pos (Index) + Position (Min / Score.Duration'Last); end; end loop; end; Part := Part + 1; end loop; end; end; or accept Start_Of_Note (Index : Positive; Num : Positive; Pit : Pitch) do declare Pos : Position renames Orchestra (Index).Pos (Num); Inter : Interpreter := Orchestra (Index).Inter; Instr : Instrument := Orchestra (Index).Instr; Dur : Score.Duration := Orchestra (Index).Dur (Num); Expect_Pit : Pitch; Expect_Dur : Score.Duration; Cumul_Dur : Long_Duration := Dur; begin -- Look for the next note to be played Get_Note (Inter, Instr, Pos, Dur, Expect_Pit, Expect_Dur); -- If this note's pitch does not match, suppose that either -- the player or the sensor missed a note and continue while Pit /= Expect_Pit and then -- Look for a note at the appropriate pitch Dur <= Score.Duration'Last -- If it cannot be found one measure in advance, give up loop Cumul_Dur := Cumul_Dur + Expect_Dur; Get_Note (Inter, Instr, Pos, Cumul_Dur, Expect_Pit, Expect_Dur); end loop; if Pit = Expect_Pit then -- A matching note was found, so update the state Pos := Pos + Position (Cumul_Dur / Score.Duration'Last); Dur := Expect_Dur; end if; end; end Start_Of_Note; end select; end loop; end Concert; end Conductor;
See CodePeer in Action!
In this video, AdaCore engineer Yannick Moy walks you through this month’s challenge using CodePeer and GNAT Programming Studio, GNAT Pro IDE.