Are local TGUIDS preinitialized?

[Attila Kovacs 629]

since I switched to x64, I've been encountering a problem that I can only describe if it's not.

[Dalija Prasnikar 1396]

TGUID is a regular record type, so it is not initialized. Only managed types are initialized, including custom managed records.

[Attila Kovacs 629]

@Dalija Prasnikar Thanks! So, I was just lucky all the time <o>, checking it against TGUID.Empty and it bit me under x64. Did a code check and found one more occurence in my code...

[Attila Kovacs 629]

Ahm, isn't there a compiler hint missing then?

[Dalija Prasnikar 1396]

26 minutes ago, Attila Kovacs said:

Ahm, isn't there a compiler hint missing then?

From https://quality.embarcadero.com/browse/RSP-24383 Warning/Error on record fields used without initialization

 

Quote

Sync status from internal system, internal issue closed on Feb 11, 2023 by Marco Cantù with comment:
Because records can be initialized and allocated in different ways, this ir much more complex than a local var. Also methods have optional initialization now.

 

[Attila Kovacs 629]

Does this 'more complex' mean that we don't care, or that we sweated hard and got it done? 🙂

[Dalija Prasnikar 1396]

1 hour ago, Attila Kovacs said:

Does this 'more complex' mean that we don't care, or that we sweated hard and got it done? 🙂

It means that it can be hard to tell whether record is initialized or not. For instance you could have TGuid.Clear method that would initialize record, but there is not way to tell what that method does comparing to TGuid.IsEmpty or any other that would expect initialized record.

 

It is not as much that warnings cannot be emitted, but that there would be a lot of bogus warnings in such case, which defeats the purpose of a warning.

Edited August 16, 2023 by Dalija Prasnikar

[Attila Kovacs 629]

var
  i: integer;
begin
  i.ToSingle;
  if i > 0 then
  Exit;

I understand but I think those excuses are made up.

 

ToSingle supresses the hint.

 

Edited August 16, 2023 by Attila Kovacs

[Dalija Prasnikar 1396]

13 minutes ago, Attila Kovacs said:

I understand but I think those excuses are made up.

I think there was some article about C# covering similar area that explained the problems rather well. I cannot find it now.

[Brandon Staggs 278]

7 hours ago, Attila Kovacs said:

I understand but I think those excuses are made up.

Hardly. If you create your own record that has an initialization function, and so you call that initialization function to init the record, how does the compiler know if you are using the record improperly or not? It does not know what the function does, or there could be a bug in your init function that misses a field, etc.  There could be other functions in the record that require the record to be valid. The compiler cannot just guess at these things.

[Attila Kovacs 629]

@Brandon Staggs The compiler makes educated guesses even with integers. Consider my example. The same could be mimicked for records. I know it won't cover everything, but neither does it for other types.

[Roger Cigol 103]

Writing a compiler is VERY complex. Speaking as someone who doesn't write compilers, I myself would be cautious about criticising my colleagues in the industry who do, particularly when given an explanation that does make some kind of sense (as in this discussion).

 

I certainly agree with @Dalija Prasnikar there is nothing worse than a compiler warning that is wrong. It is hard to make them "go away" and they definitely can "hide" other useful warnings.

[Stefan Glienke 2002]

The explanation was given by the pm and not some compiler dev fwiw and it is nonsense tbh.

The compiler already knows if a record is managed or not because if its managed then it inserts code to call to System._InitializeRecord.

Now the case can happen that you access some field on a record that is a managed record but that field is of a non managed type - these fields are not touched inside that routine.

The compiler can still issue a W1036 when accessing a field on a record that is not managed - would there still be cases where a warning would be missing? Yes, but it would be better than it is currently.

[Brandon Staggs 278]

3 hours ago, Stefan Glienke said:

The compiler already knows if a record is managed or not because if its managed then it inserts code to call to System._InitializeRecord.

But you can have unmanaged records that have initialization functions. I don't understand how the compiler would know if I have properly initialized my record or not. Giving me warnings that I may be accessing an uninitialized record field when in fact I know full well it has been initialized is going to force me to disable the warning.

[Stefan Glienke 2002]

We already have the situation that calling any method on a record disables the warning - I did not mention disabling that behavior, did I?

Also, don't you think a compiler could easily see if the access to a field happens before or after some method call?

I explicitly stated that there would still be cases where a W1036 would be appropriate but would not be seen due to some reasons but in the places where it is hundred percent certain that the access is to an uninitialized field the compiler should raise it

Edited August 17, 2023 by Stefan Glienke

[Uwe Raabe 2057]

17 minutes ago, Stefan Glienke said:

We already have the situation that calling any method on a record disables the warning

Could it be that using the equality operator qualifies for calling any method?

[Stefan Glienke 2002]

26 minutes ago, Uwe Raabe said:

Could it be that using the equality operator qualifies for calling any method?

If you are referring to TGUID specifically then I would assume that to be the case as that is basically what happens because it has operator overloading.

 

To be more precise though because my previous comment might be misleading:

- currently, we don't get any W1036 on using a field on some local record variable (see Test4)

- we get a W1036 on local intrinsic non-managed type variables such as Integer (see Test1)

- we *don't* get a W1036 on local intrinsic non-managed type variables such as Integer when they get passed by ref (see Test2) or a helper method is being called on them (see Test3) anywhere in the routine.

 

type
  TIntHelper = record helper for Integer
    procedure Foo;
  end;

procedure TIntHelper.Foo;
begin
end;

procedure PassRef(var ref);
begin
end;

procedure Test1;
var
  i: Integer;
begin
  if i = 0 then Writeln;
end;

procedure Test2;
var
  i: Integer;
begin
  if i = 0 then Writeln;
  PassRef(i);
end;

procedure Test3;
var
  i: Integer;
begin
  if i = 0 then Writeln;
  i.Foo;
end;

procedure Test4;
var
  g: TGUID;
begin
  if g.D1 = 0 then Writeln;
end;

end.

My point is - yes I can craft some version of Test4 where something might happen with g that causes it to be initialized or not - but the point is: if I access any field on it *before* that it is a hundred percent certain that I am accessing an uninitialized field.

Now in practice the compiler source code and its design might be in a state where doing this is hard to achieve but the point is: it's not impossible.

Edited August 17, 2023 by Stefan Glienke

[Attila Kovacs 629]

16 minutes ago, Uwe Raabe said:

Could it be that using the equality operator qualifies for calling any method?

as both paramerters for Equal() are const it could yell before calling it
 

[Dalija Prasnikar 1396]

23 hours ago, Roger Cigol said:

I certainly agree with @Dalija Prasnikar there is nothing worse than a compiler warning that is wrong. It is hard to make them "go away" and they definitely can "hide" other useful warnings.

I will have to change my position. I was focused on behavior where compiler would catch all cases where record would be uninitialized - by all I don't mean having different code paths where compiler cannot easily determine whether something is initialized or not even for types it currently shows warnings.

 

To catch all such cases, calling record method would also need to show a warning, but there is no way compiler can differentiate between method that initializes a record and method that expects already initialized method - the difference between imagined TGuid.Clear and TGUID.IsEmpty.

 

However, for other value types calling a method does not trigger a warning and simply disables further warnings, so such behavior could be easily extended to non-managed fields in records without causing any bogus warnings and would not be any more difficult to achieve than existing warnings for other value types. 

 

It is likely that variant parts in records would have to be excluded as accessing and initializing one field does not mean that another field that shares same memory would be fully initialized (memory overlap can be only partial) and it is possible that tracking sizes and what is initialized and what not would be too much in such scenario.

 

There is a value in showing at least some warnings comparing to none.

Edited August 18, 2023 by Dalija Prasnikar