Has function initialization code changed from Seattle to Tokyo?
Asked Answered
R

1

4

I am in the process of upgrading code from Delphi 10 Seattle to Delphi 10.2 Tokyo and get a lot of H2077 hints Value assigned to ... never used on assignments.
(Even in places where these were explicitly added in the past to get rid of 'may not have a value' warnings).

These are all function initialized like:

Result := 0;
...

Or:

Result := ftType1; // where ftType1 is an enumerated type
...

Did the compiler get smarter in detecting these or has something changed regarding the initial return values of functions?

We have always had these hints 'on', and I always build (not compile).

Example function (1) that builds without hints in Seattle,
but gives the hint H2077 Value assigned to 'GetDatabaseDialect' not used on the first Result := 0 line in Tokyo.

function GetDatabaseDialect(DBName, User, Pswd: string) : integer;
var
  status: array[1..19] of longint;
  szDbName, szDbParam: PANSIChar;
  dbHandle : pointer;
  rslt: longint;

  lDPBBuffer : ANSIString;
  lDPBLength : integer;

  cItem: ANSIChar;
  szRslt: PANSIChar;      //array[0..IBResultBufferSize-1] of ANSIChar;
begin
   Result := 0;
   dbHandle := nil;
   // init database parameter block with version number
   lDPBBuffer := '';
   SetLength(lDPBBuffer, 1);
   lDPBBuffer[1] := ANSIChar(isc_dpb_version1);
   lDPBLength := 1;

   // fill Database Parameter Buffer with user name/password
   lDPBBuffer := lDPBBuffer +
             ANSIChar(isc_dpb_user_name) +
             ANSIChar(Length(User)) +
             ANSIString( User );
   Inc(lDPBLength, 2 + Length(User));

   lDPBBuffer := lDPBBuffer +
             ANSIChar(isc_dpb_password) +
             ANSIChar(Length(Pswd)) +
             ANSIString( Pswd );
   Inc(lDPBLength, 2 + Length(Pswd));

   //Pointers naar naam + buffer
   szDbName  := PANSIChar(ANSISTring(DBName));
   szDbParam := PANSIChar( lDPBBuffer );

   // attach to the database and set dialect
   rslt := isc_attach_database(@status, 0, szDbName, @dbHandle, lDPBLength, szDbParam);
   if rslt <> 0 then
      raise EDatabaseError.Create('Error attaching database!  ISC# ' + IntToStr(rslt));

   //Haal sql dialect op
   szRslt := AllocMem(1000);
   try
      FillChar( szRslt^, 1000, 0);
      cItem := ANSIChar( isc_info_db_SQL_dialect );
      rslt := isc_database_info(@status, @DBHandle, 1, @cItem, 1000, szRslt);
      if rslt <> 0 then
         raise EDatabaseError.Create('Error retrieving database info !  ISC# ' + IntToStr(rslt));


      Result := Ord(szRslt[3]); //3e positie is dialect
   finally
      FreeMem(szRslt);
   end;

   // Drop the connection to the database
   rslt :=  isc_detach_database(@status, @dbHandle);
   if rslt <> 0 then
      raise EDatabaseError.Create('Error detaching database!  ISC# ' + IntToStr(rslt));
end;

Example (2) from a third party library that does not seem to be optimized for Tokyo,
illustrating the case with enumerated types:
H2077 Value assigned to 'TppTemplate.StreamType' not used
Note that changing the assignment to Result := ftASCII; does not make the hint go away (my initial assumption that it was associated with the first enumeration value was incorrect).

type TppFormatType = (ftBinary, ftASCII);

function TppTemplate.StreamType(aStream: TStream): TppFormatType;
var
  lSavePos: Integer;
begin
  {save stream position}
  lSavePos := aStream.Position;
  Result   := ftBinary;  

  try
    ComputeOffsetFromStream(aStream);

    aStream.Seek(FOffset, soBeginning);

    if IsValidASCIISignature(aStream) then
      Result := ftASCII

    else if IsValidBinarySignature(aStream) then
      Result := ftBinary

    else
      raise EInvalidTemplateError.Create(ppLoadStr(49));

  finally
    {restore stream position}
    aStream.Seek(lSavePos, soBeginning);
  end;
end; {function, StreamType}

The common denominator seems to be the Result assignments being in try/finally blocks.

Rodeo answered 18/7, 2017 at 14:58 Comment(4)
FWIW, that would be a smarter compiler, not function initialization code that changed.Maguire
There were cases where the compiler did not properly recognize that a variable or Result was being set in every code path. Now it does and thus recognizes that the values assigned before are not necessary anymore.Rejoice
@David, yeah and you build that MCVE in both versions and say, yeah they differ, one generates hint whilst the other doesn't, I don't know why but there must be some changes. This is something that EMBT should respond to a bug report.Bottommost
After the update, with the two examples, it seems that the compiler is simply right. Previous versions did -- sometimes -- complain about a variable not being initilialized even if there was no code path that used or returned an uninitialized value. So it seems the compiler got smarter. People who got the hint in the previous versions would often add an initial value at the start of a procedure or function. Now the compiler sees that this is useless and hints about that. That seems to be all.Maguire
D
7

Consider this code with a minimal reproduction of your scenario:

function Bar: Boolean;
begin
  Result := Random<0.5;
end;

function Foo: Integer;
begin
  Result := 0;
  if Bar then
    Result := 1
  else
    raise Exception.Create('');
end;

The compiler, even older versions, emits the following hint:

[dcc32 Hint]: H2077 Value assigned to 'Foo' never used

This is reasonable. The first assignment to Result is pointless and can be removed.

Now consider this variation:

function Foo: Integer;
begin
  Result := 0;
  try
    if Bar then
      Result := 1
    else
      raise Exception.Create('');
  finally
  end;
end;

Older versions of the compiler no longer emit the hint, but the latest version of the compiler does. This should be considered a compiler defect, for older versions. The two variants of Foo shown above are semantically identical. The compiler would be justified in generating identical code.

As you surmise, the assignment being inside the try/finally block is necessary to trigger the defect in previous versions.

We can conclude that the Embarcadero developers have fixed a defect in Tokyo. You can resolve the hints by removing the spurious initial assignments.

Of course, if your code is to be compiled by older versions of the compiler, as well as by new versions, then you are in a bind. With the code as it stands now, a hint is emitted by new versions of the compiler. Remove the initial assignment and a hint is emitted by old versions of the compiler.

Dorita answered 19/7, 2017 at 8:38 Comment(11)
I would simply switch off this particular hint (is that possible?) in the older versions. If it compiles without hint in the newest version, Or I would simply ignore the hints in the previous versions, as long as there is none in the newest.Maguire
@RudyVelthuis Individual warnings can be suppressed, but hints are all or nothing.Dorita
Am I missing something here? In my second example with the enumerated type, suppose the initialization was Result := ftAscii;. Now I get the compiler hint, remove that line. What if an exception is raised in ComputeOffsetFromStream? Execution would jump to the finally and Result would be undefined. But the compiler does not warn me as it did in the past. I'm not sure that is an improvement...Rodeo
@JanDoggen, in case of exception the result value should not be used. why do you care about the result in case of exception?Edmead
@Jan The finally does not catch the exception. If an exception is raised, and not caught in the function, then the function does not return a value at all. So yes, I think you are missing something. Rather than looking at your examples which are very far from minimal, look at the examples in the answer. These are minimal. But are completely analogous to your code.Dorita
@DavidHeffernan, "If an exception is raised, and not caught in the function, then the function does not return a value at all" - on the other hand if the exception is caught by the caller you get a valid result.Edmead
@Edmead No you don't. A function does not return anything to the caller if its execution is ended by an exception. Perhaps you are getting confused by the implementation detail that some return values are actually var parameters. But, for instance, an integer return value comes back in a register. The caller then has to store away that register value, code that won't execute because of the exception. So, no, what you state is incorrect.Dorita
@DavidHeffernan, function Foo: string; begin Result := 'Foo'; raise Exception.Create('Boo'); end; procedure Main; var s: string; begin try s := Foo; except ShowMessage(s); end; end;Edmead
@DavidHeffernan, But you are correct about functions returning integers. did not know that.Edmead
@Edmead Your first example relies on implementation detail. Don't ever write code that relies on that.Dorita
@David: Nah, no NIHS. I just hadn't completely read your comment. <g>Maguire

© 2022 - 2024 — McMap. All rights reserved.