CODEWITHSUNDEEP
delphicompiler-warningsdelphi-10-seattle
Jan Doggen
Jan Doggen

Reputation: 9106

'Value never used' changes to 'variable not initialized'

If I compile the below code it gives a warning on line 1:

value assigned to lTime never used*

But if I remove that line, I get a different warning on line 2:

variable lTime might not have been initialized

Is the compiler missing something, or am I?

procedure TFormWebServices.RemoveOldReports;
var
  TSR       : TSearchRec;
  I         : Integer;
  lCutOff,
  lTime     : Int64;
  TSToDelete: TStringList;
  S,Msg     : String;
  E         : Exception;
begin
  lCutOff := DelphiToJavaDateTime(Now - cDefReportLifeMins/1440);
  I := FindFirst(FReportDir + '*.pdf',0,TSR);
  TSToDelete := TStringList.Create;
  while I = 0 do
  begin
    if (TSR.Attr and faDirectory) = 0 then 
    begin
      lTime := lCutOff;          // Line 1
      try
        lTime := StrToInt64(Copy(TSR.Name,1,pos('.',TSR.Name)-1));
      except
        on E:Exception do lTime := lCutOff;
      end;
      if lTime < lCutOff then    // Line 2
        TSToDelete.Add(TSR.Name);
    end;
    I := FindNext(TSR);
  end;

This is not a dupe of Why is the Compiler warning that variable may not be initialized?, because I assign lTime in the exception as well.

Upvotes: 4

Views: 257

Answers (2)

David Heffernan
David Heffernan

Reputation: 613481

lTime := lCutOff;       
try
  lTime := StrToInt64(Copy(TSR.Name,1,pos('.',TSR.Name)-1));
except
  on E:Exception do lTime := lCutOff;
end;
if lTime < lCutOff then
  TSToDelete.Add(TSR.Name);

The compiler is correct to warn about this code. When you assign lTime := lCutOff on the first line, the value written in that assignment is never read.

However, when you remove the code things are less clear cut.

try
  lTime := StrToInt64(Copy(TSR.Name,1,pos('.',TSR.Name)-1));
except
  on E:Exception do lTime := lCutOff;
end;
if lTime < lCutOff then
  TSToDelete.Add(TSR.Name);

There are two scenarios to consider: an exception is not raised inside the try/except block, or an exception is raised there.

If no exception is raised then it is simple, lTime is assigned the result of StrToInt64 and is therefore initialized before being read.

In an exception is raised, then lTime is not initialized inside the block. What happens next?

  • If the exception derives from Exception (not compulsory) then it is caught and lTime is initialized.
  • If the exception does not derive from Exception then it is not caught and lTime is never read.

Therefore, the compiler could infer all of that and therefore not issue an uninitialized variable warning. However, the compiler does not do flow analysis of that complexity, sadly. I believe that its logic runs like this:

  1. An exception is raised before lTime is initialized, then
  2. The exception could be caught, in which case
  3. The variable lTime is then read, and still might be uninitialized.

In step 2 it ought to be able to realise that lTime is initialized but it simply does not perform such analysis. So whilst one could argue that the compiler could do better, you just have to accept this as a limitation of its analysis algorithms.

Having understood that we are left with the task of finding a way to write the code and avoid the warnings. We don't want to suppress the warnings, we just need to find a way to write the code so that it is both correct and free of warnings.

The way forward, in my view, is to recognise that exceptions are the wrong tool to use here. It is normal behaviour for this conversion to fail. You should write the code using conversion functions that don't raise an exception when they fail. For instance you could use one of the following options:

if TryStrToInt64(..., lTime) then
  if lTime < lCutOff then
    ....
else
  lTime := lCutoff;

Or:

lTime := StrToInt64Def(..., lCutoff);
if lTime < lCutOff then
  ....

Upvotes: 7

Paul Michael
Paul Michael

Reputation: 216

This is correct behaviour. The line 1 value will never be used since in the try/except you assign a new value without using the previous one, hence the warning.

if you remove the line you try to use the lTime variable outside a try/except block which may fail without actually setting a value to lTime

procedure TFormWebServices.RemoveOldReports;
var
  TSR       : TSearchRec;
  I         : Integer;
  lCutOff,
  lTime     : Int64;
  TSToDelete: TStringList;
  S,Msg     : String;
  E         : Exception;
   begin
   lCutOff := DelphiToJavaDateTime(Now - cDefReportLifeMins/1440);
   I := FindFirst(FReportDir + '*.pdf',0,TSR);
   TSToDelete := TStringList.Create;
   while I = 0 do
      begin
         if (TSR.Attr and faDirectory) = 0 then 
            begin
               lTime := lCutOff;          // Line 1
               try
                  lTime :=     StrToInt64(Copy(TSR.Name,1,pos('.',TSR.Name)-1));
               except
                  on E:Exception do lTime := lCutOff;
               end;
               if lTime < lCutOff then    // Line 2
                  TSToDelete.Add(TSR.Name);
            end;
         I := FindNext(TSR);
      end;

You could do this

procedure TFormWebServices.RemoveOldReports;
var
  TSR       : TSearchRec;
  I         : Integer;
  lCutOff,
  lTime     : Int64;
  TSToDelete: TStringList;
  S,Msg     : String;
  E         : Exception;
   begin
   lCutOff := DelphiToJavaDateTime(Now - cDefReportLifeMins/1440);
   I := FindFirst(FReportDir + '*.pdf',0,TSR);
   TSToDelete := TStringList.Create;
   while I = 0 do
      begin
         if (TSR.Attr and faDirectory) = 0 then 
            begin
               lTime := StrToInt64Def(Copy(TSR.Name,1,pos('.',TSR.Name)-1), lCutOff);
               if lTime < lCutOff then    
                  TSToDelete.Add(TSR.Name);
            end;
         I := FindNext(TSR);
      end;

Upvotes: -1

Related Questions