Stefan Glienke

Every time I see allocations for comparing strings I have to cry

Of course, the result will be as expected if you name the two functions differently, then it does not matter what order they are getting called - but when it's twice the same function which returns different results then the call order matters

Here is the code to simulate the issue:

var
  callNo: Integer;

const
  PGM_Magic_Number = 'P5';

function ReadChar: Char;
begin
  Inc(callNo);
  Result := PGM_Magic_Number[callNo];
end;

procedure Test;
var
  s: string;
begin
  s := ReadChar + ReadChar;
  Writeln(s);
end;

begin
  Test;
end;

We recently discussed this in https://quality.embarcadero.com/browse/RSP-18259 - even after reading the comment in System.pas I cannot imagine what the idea behind ReleaseExceptionObject was given that AcquireExceptionObject returns a reference and thus also passes ownership to the caller. Thus the parameterless ReleaseExceptionObject cannot work by design given that it does not have any parameter where you might pass the previous result of AcquireExceptionObject.

Even the documentation tells that this routine is just there for backwards compatibility and should not be used (I would have marked it as deprecated long ago so people that might still have it in their code would remove it)

https://docwiki.embarcadero.com/Libraries/Alexandria/en/System.ReleaseExceptionObject

3 hours ago, dummzeuch said:

Isn't there something like {$SomeOption default} to reset an option to the value given in the project options? I seem to remember reading about that at some time in the what's new with Delphi Xxxx.

This would not be of much use given that one can use include files or other means to control their compiler options. You want a way to preserve the effective options at a given location to restore precisely that state.

2 hours ago, Fr0sT.Brutal said:

Yes, language change via restart is OK.

I also found the way to modify constants

const
  src_arr: array[1..3] of string = ('foo', 'bar', 'quz');

  PString(@src_arr[1])^ := 'translated'+Inttostr(2); // imitate dynamically-created string

AFAICT no issues happen in my test project

That "works" (i.e. you wont get a GPF) because typed consts are no real consts but rather read-only variables (as in you cannot directly assign to them regularly).

But I am sure this will cause a memory leak - a static one but ReportMemoryLeaksOnShutdown should complain about it.

13 minutes ago, mytbo said:

Question for the experts. Is there a reason not to write this way?

function BuildDigits(_Digits: UInt16; _Invalids: UInt8): UInt16;
var
  tmp: SmallInt absolute _Digits;
begin
  Dec(tmp, 2048);
  tmp := tmp * 11;
  _Digits := (_Digits and $FFFC) or (_Invalids and $03);
  Result := Swap(_Digits);
end;

With best regards

Thomas

Actually, it's way simpler:

function BuildDigits(_Digits: UInt16; _Invalids: UInt8): UInt16;
begin
  _Digits := UInt16((_Digits - 2048) * 11);
  _Digits := (_Digits and $FFFC) or (_Invalids and $03);
  Result := Swap(_Digits);
end;

IIRC a smaller than register size unsigned ordinal type will not cause an overflow when it wraps around 0 so it's not necessary to turn it into a signed to avoid that. The CPU uses 32bit register math anyway. The UInt16 of the multiplication is necessary though.

Which you obviously didn't which is the raison d'être of this thread

4 minutes ago, dummzeuch said:

The point is that I want to find range check errors and fix them.

And my point was that you cannot assign signed to unsigned and vice versa without potentially getting them.

10 minutes ago, Uwe Raabe said:

I wonder if simply disabling range checks for the whole routine would be the best solution here.

Pretty much because the very first line already is subject to a range check error.

Many people have not noticed many potential range check errors because range and overflow checks were disabled even in debug config and only after they finally changed that they occur in a lot of code that unknowingly implicitly converts between signed and unsigned.

And why exactly would it be a problem to subtract 2048 from 0 in a signed 16-bit integer?

If you don't need language swapping while the application is running then go with the const array using resourcestrings.

I say so because the const array is initialized on startup with the localized values from the resourcestrings. If you change language later they will not change.

  {$IFOPT R+}{$DEFINE RANGECHECKS_ON}{$R-}{$ENDIF}
  TempInt := TempInt * 11;
  {$IFDEF RANGECHECKS_ON}{$UNDEF RANGECHECKS_ON}{$R+}{$ENDIF}

5 minutes ago, PeaShooter_OMO said:

TMethod in System is a record with a Code and Data pointer. Would it be possible to browse Delphi's source code to find any source code to verify Event types' structure or is the knowledge about Event types found in documentaion only?

https://docwiki.embarcadero.com/RADStudio/Alexandria/en/Procedural_Types_(Delphi)#Method_Pointers

Event types and TMethod while binary compatible (both consist of the data and code pointer) they are not assignment compatible.

This routine will work for all event handlers that have the TNotifyEvent signature but not for others (such as mouse or key-related ones that have additional parameters).

For those, you need to write overloads.

procedure SetDefaultEventHandlerIfEventExists2(const AControl: TControl;
  const AEvent: string; const AHandler: TNotifyEvent);
begin
  if IsPublishedProp(AControl, AEvent) then
    SetMethodProp(AControl, AEvent, TMethod(AHandler));
end;

I read "branchless" and I cry - see RSP-21955

Turn the strings into an enum or an index and put the functions into a const array over that enum/index.

8 minutes ago, Anders Melander said:

To mark block 1 as allocated I would set bit 0 of the first byte, block 9 would be bit 0 of the second byte, etc. It could be that I should be using another bit-layout.

Could the documentation be of any help?

12 minutes ago, Attila Kovacs said:

this is a different checkInvariants

No, it's not - it's the call from this line: https://github.com/microsoft/microsoft-pdb/blob/master/PDB/msf/msf.cpp#L1627

It however might look different today given that source on GitHub is from seven years ago but it might still give a clue.

13 minutes ago, Anders Melander said:

I want a sex change operation so I can have your children.

 

(it means "thank you" in case you wondered)

The weirdest way I got a "thank you" ever ngl 

FWIW: https://github.com/microsoft/microsoft-pdb/blob/master/PDB/msf/msf.cpp#L1385

@Anders Melander Might be worth looking into https://github.com/llvm/llvm-project/blob/main/llvm/lib/DebugInfo/PDB/Native/PDBFileBuilder.cpp

This could be some relevant info: https://randomascii.wordpress.com/2023/03/08/when-debug-symbols-get-large/

2 minutes ago, Anders Melander said:

I meant what do you mean by this ^

 

Do you mean 32- vs 64-bit addresses? AFAIR there's no choice or ambiguities in the PDB format with regard to the size of an address value, relative or absolute, but I would have to take a look at the source to make sure.

Yes - because apparently according to the warning, there is a problem with "match the module with the symbol file". It could be very well a VTune bug though - after all the 2023.1.0 we currently get is a "pre-release".

Just now, Anders Melander said:

Please elaborate

I can't - as I confirmed the breaking change is from 2023.0.0 to 2023.1.0 and that explicit mention of large pdb file support was the most obvious clue. I don't know anything about the pdb format.

I just upgraded from 2023.0.0 where it worked to 2023.1.0 where it does not.

What I noticed in the change log of VTune 2023.1.0 is this:

Debug Support 

Support for Large PDB Files 
Starting with the 2023.1 version, on Windows* systems, you can resolve debug information from PDB files larger than 4GB. 

Could it be that the address bitness written by map2pdb is not correct now?

In the collection log of VTune I can see this warning:

Cannot locate debugging information for file `somepath\Tests.exe'. Cannot match the module with the symbol file `somepath\Tests.pdb'. Make sure to specify the correct path to the symbol file in the Binary/Symbol Search list of directories.

21 minutes ago, mikerabat said:

Let me know what you think or if you encounter some errors...

Personally, I would find it way more readable if you would not ifdef DELPHIAVX for every single line but simply make it two blocks, one using the avx instructions and one using db.