Warning: Unsafe typecast of 'TSmallPoint' to 'Integer'
Asked Answered
C

2

10

I use this code in my project:

var
  P: TPoint;

MyControl.Perform(WM_LBUTTONDOWN, 0, Longint(PointToSmallPoint(P)));

The compiler gives me a warning:

[Warning]: Unsafe typecast of 'TSmallPoint' to 'Integer'

But, the same code is used in Controls.pas without any warnings - for example in TControl.BeginDrag method:

....
Perform(WM_LBUTTONUP, 0, Longint(PointToSmallPoint(P)));

I don't see any {$warnings off} in Controls.pas unit.

Why is the compiler warning me, but skips the warning for Controls.pas?
Is this code Unsafe?


Edit: in my Project Options -> Compiler Messages -> Unsafe Typecast is checked (which by default is unchecked).
Maybe this is why @David and @Ken could not reproduce the warning.

Choirboy answered 28/12, 2012 at 22:47 Comment(9)
I can't reproduce that warning. Could you supply a complete program that demonstrates the warning.Dither
I can't reproduce it either, using the following: procedure TForm3.FormCreate(Sender: TObject); var P: TPoint; begin P := Mouse.CursorPos; Perform(WM_LBUTTONDOWN, 0, Longint(PointToSmallPoint(P))); end;Bragg
And you are compiling Controls.pas yourself right?Dither
@DavidHeffernan, I dont know what you mean by " compiling Controls.pas yourself", but please see my edit.Choirboy
IIRC, this warning was newly added in Delphi 7 in anticipation of Delphi .NET, where such casts would not work. Not an answer to your question, but unless you're planning to use that, you don't need to worry about the warning anyway.Addington
Your edit is right; I have unsafe type cast unchecked, because it's no longer applicable. (It was added in Delphi 6 or 7 for .net compatibility when they were developing Delphi for .NET, to make it easier to write code that worked for both .NET and Win32; since the Delphi for .NET product was discontinued, that warning (and the two above it) are not applicable any longer).Bragg
If you don't know what I mean, then you likely are not compiling the unit. See my answer.Dither
@KenWhite, you are correct. +1. and I deserve spankings. in a default project this could not be reproduced. in my Project Options -> Compiler Messages -> Unsafe Typecast is checked.Choirboy
I added an answer that explains why the warning is generated (and why it's safe to disable it) for future readers.Bragg
B
11

This is caused because you have unsafe typecast warnings checked in Project->Options->Compiler Messages. This is safe to uncheck (as are unsafe type and unsafe code above it. (See below.)

I couldn't reproduce the warning because I have unsafe type cast unchecked. It's no longer applicable. (It was added in Delphi 6 or 7 for .net compatibility when they were developing Delphi for .NET, to make it easier to write code that worked for both .NET and Win32; since the Delphi for .NET product was discontinued, that warning (and the two above it) are not applicable any longer). The "unsafe" in these three warnings use the .NET meaning of "unsafe", meaning "unmanaged".

From the Delphi 7 help file (search for "Compiler Changes") (emphasis mine):

The Delphi dcc32 compiler now supports three additional compiler warnings: Unsafe_Type, Unsafe_Code, and Unsafe_Cast. These warnings are disabled by default, but can be enabled with the compiler directive {$WARN UNSAFE_CODE ON}, compiler command line switch (dcc32 -W+UNSAFE_CODE), and, in the IDE, on the Project|Options|Compiler Messages page.

This feature is intended to help you port your code to the managed execution environment of Microsoft's .NET platform. In a managed execution environment, "unsafe" means the operation cannot be verified during the static analysis performed by the Just In Time (JIT) compiler. Such code might pose a security risk, since there is not enough information for the JIT compiler to verify its runtime behavior. Examples of unsafe code include pointer operations and memory overwrites.

Bragg answered 28/12, 2012 at 23:28 Comment(13)
@David: I don't know; I haven't checked. There's probably an {$IFDEF} related to .NET, but I was explaining why the poster was getting the warning (and why you and I couldn't reproduce it). I'll take a look, but it really isn't relevant here because it wasn't generating the warning.Bragg
Controls unit aspect seemed to be the meat of the question asked.Dither
What's your source of information that this is .net specific and no longer applicable. Casting a pair of smallints to a longint seems pretty unsafe to meDither
And you answered that portion. I answered the question implied by the subject, which is "why is this warning being generated when the Controls unit doesn't generate it"; I explained why the warning was happening, how to fix it, and why it was safe to fix it by turning off the warning. A quick look at D7's Controls.pas doesn't show an {$IFDEF}, so it's probably not being compiled at all, and Vlad just looked in Controls and saw it; I don't know if he actually compiled Controls or just though it didn't generate the warning, and didn't really think it was relevant. Vlad apparently agreed.Bragg
The documentation for that warning (and the other two "unsafe" warnings), and the postings in the Borland/CodeGear newsgroups at the time the warnings were added. They were added specifically for doing Win32/.NET compatible single-source projects, and the meaning of "unsafe" in this case is specifically the .NET framework meaning of that word.Bragg
@David: If you have D2007 installed, see ms-help://borland.bds5/devcommon/compdirsunsafecode_xml.html in the help file regarding the unsafe code warning, which specifically mentions "Delphi for .NET"; the same meaning of "unsafe" applies to the other two warnings that use it as well.Bragg
The documentation says: You have used a data type or operation for which static code analysis cannot prove that it does not overwrite memory. For example, you might have cased one record to another or one instance to another. The code in the Q is unsafe. And is unsafe in Win32.Dither
@David: See my edit above, with a specific quote from the D7 documentation. If you want to quibble with it, try support.embarcadero.com or one of their forums.Bragg
Yes, your latest update helps. Before that I couldn't work out where your assertions were coming from. Thanks.Dither
@David: I swear I didn't make it up. :-DBragg
It's always good to back claims up with evidence. Kind of weird that they talked about .net compiler in D7. D7 pre-dated .net compiler. No doubt it was being brewed though. Still, this is an excellent warning in my view that is very useful even for native code.Dither
@David: Typecasting is used to tell the compiler "I know you don't think this is right, but I want to treat <x> as something other than it's type, and I'm telling you to ignore it because I know what I'm doing". Every typecast is technically unsafe, because no type checking is done (you're telling the compiler not to do it by the very act of typecasting), and with or without the warning enabled the compiler will not allow you to make totally invalid typecasts.Bragg
let us continue this discussion in chatDither
D
5

If you compile the Controls unit yourself, with the unsafe typecast warning enabled, then you will see the warning. But if you link the pre-built .dcu file then you see no warning. The compiler only emits warnings on units that it compiles.

As a general rule, the RTL and VCL units generate large numbers of hints and warnings. If ever I have to re-compile them, I always have to switch hints and warnings off on those units.


The modern day documentation for the warning says:

You have used a data type or operation for which static code analysis cannot prove that it does not overwrite memory. For example, you might have cased (sic) one record to another or one instance to another.

And that warning does apply to your code. Your code is unsafe. The compiler cannot verify that it is correct to overlay two 16 bit integers onto a 32 bit integer. So the compiler warns you. It's up to you to decide whether or not the code is correct.

Now, it appears that the warning was intended primarily for the .net compiler. All the same, it still has meaning for the Win32 compiler. Overlaying one record on another is suspicious behaviour.

Dither answered 28/12, 2012 at 23:21 Comment(14)
+1. You did answer the question "Why is the compiler warning me, but skips the warning for Controls.pas". but Ken's answer about "unsafe type cast no longer applicable" made it all clear for me. so maybe I deserve a -1.Choirboy
I thought that was the question.Dither
Ken did answer "Is this code Unsafe?". and explained the topic about "Unsafe typecast".Choirboy
This question specifically refers to D7, and I provided totally conflicting information from the D7 documentation (with a citation of where to locate it). Your documentation link specifically refers to XE3, which is not the same.Bragg
The Delphi XE3 Vcl.Controls unit (TControl.BeginDrag at approx. line 6940, as referenced in Vlad's question) still has the same exact line of code he quoted; apparently when dealing with this message the typecast is appropriate.Bragg
We humans can all see that, but the compiler cannot. That's the point. On the .net compiler won't that code be rejected?Dither
Yes, it will, which is why the warnings were added. But we're not writing code for the .NET compiler here. :-) It didn't exist in D7 days (it was D8), but they were obviously working toward it already.Bragg
But the Controls.pas is compiled by Delphi.net. IDE still relies on it. So is that code in Controls inside an IFDEF?Dither
No, not in D7. (The .NET compiler didn't exist yet; the warnings were added so you could start looking at things with the idea of single source.) Actually, I don't think the typecast is unsafe here even if it didn't refer to .NET (eg., in XE3). The call to PointToSmallPoint will return a valid TSmallPoint, and typecasting it to LongInt should be fine (and is pretty typical when working with the WinAPI, even in the API documentation).Bragg
@Ken Clearly no .net IFDEF needed in D7. I mean in modern Delphi of course. Read back a couple of your comments. Line 6940.Dither
And the typecast is unsafe. It would be UB in C or C++. Splicing two 16 bit ints onto a single 32 bit int is the very essence of unsafe. We all know that it does what we intend, but the compiler doesn't. You can't expect the compiler to consult MSDN! Anyway, this has been fun, but now it is sleep time.Dither
There's no {$IFDEF}` in the XE3 Vcl.Controls.pas unit at that point, either, but the VCL is definitely not .NET at that point. However, there are still remaining {$IFDEF}s in the VCL source that weren't removed, so I'd guess there never was one here. ?? I don't know how it worked in .NET; I'll have to look at a version of Delphi that has VCL for .NET, and see what they did there.Bragg
By that sense, almost every single typecast operation would be unsafe, and you and I both know that's not true. And again, the whole purpose of typecasting is to tell the compiler to not pay attention to what you're doing, because you know what you're doing. Consider the times you use an interposer class to get to something that isn't exposed in a descendant - you know that's a safe typecast, even though the compiler doesn't. If you couldn't compile it because of that warning, there would be lots of code that stopped working (which is probably why it's off by default).Bragg
@Ken Agreed. Pretty much every typecast is unsafe. Examples from my code: Int64Rec(), oodles of casts involving Variant, any use of absolute, casting object references to NativeInt and/or Pointer and so on and so on. So, yes, I agree with your perspective now. These casts are unsafe, but that's why the programmer put them there in the first place.Dither

© 2022 - 2024 — McMap. All rights reserved.