[dcc32 Hint] Foobar.pas(112): H2443 Inline function 'MessageDlg' has not been expanded because unit 'System.UITypes' is not specified in USES list
[dcc32 Hint] Foobar.pas(112): H2443 Inline function 'MessageDlg' has not been expanded because unit 'System.UITypes' is not specified in USES list
You know what Delphi? I DON'T CARE!!!
The problem is I can't just disable this one hint, no, that would be a nice and sensible feature. Instead I have to disable *all* hints, and miss out on the sensible ones, like unused variables and other things which may point to actual code issues.
In fact, the failure to inline is of such little importance there should be a specific command line switch to turn on the failure reporting, ala how VC++ deals with auto-vectorization. Oherwise it should just shut the fuck up!
You know what Delphi? I DON'T CARE!!!
The problem is I can't just disable this one hint, no, that would be a nice and sensible feature. Instead I have to disable *all* hints, and miss out on the sensible ones, like unused variables and other things which may point to actual code issues.
In fact, the failure to inline is of such little importance there should be a specific command line switch to turn on the failure reporting, ala how VC++ deals with auto-vectorization. Oherwise it should just shut the fuck up!
I find this hint valuable, as prevention of inlining can have numerous implications. I do agree that it should have been possible to individually disable hints.
ReplyDeleteLars Fosdal For what other uses than performance sensitive code do you find this useful?
ReplyDeleteOn the other hand the important stuff is often not inlined (like the new helpers for String, and various RTL functions), and the irrelevant ones are inlined (like MessageDlg... where you really don't care about performance).
ReplyDeleteAlso it often ends up messing with cross-platform support when it ends up requiring the Windows units.
I'm very sorry, but why not to include the unit mentioned in the uses list ? :)
ReplyDeleteAsbjørn Heid - First and foremost - inlining decryption routines, to force the need for patching in multiple places, instead of just one.
ReplyDeleteEric Grange - Really!? That's not good - particularly for the string helpers. Is it because it was forgotten, or because it doesn't work, or because it wasn't allowed?
Eric Grange True, part of the problem is the rather weird sprinkling of inline directives in the RTL/VLC source.
ReplyDeleteIgor Schevchenko We have some 2000+ units, granted not all of them has this issue, but a fair amount do. Inlining a message dialog function is completely stupid, it's not time critical at all, and as such is a waste of time adding this unit where "needed" (where the need is to shut the compiler up).
ReplyDeleteAsbjørn Heid - Is it possible that the warning appears because you call a routine in your units which has the inline clause, and that routine calls MessageDlg?
ReplyDeleteLars Fosdal I'd lump that in the "special needs" category.
ReplyDeleteI guess my point is, unless you have special needs, the Delphi should just do the right thing and not whine like a spoiled 2 year old.
For those cases when one has a special need, one should be able to pass a flag to the compiler, or add a compiler directive to the sensitive code.
Lars Fosdal No, the problem is that Vlc.Diaglogs' MessageDlg() function is marked as inline for no good reason.
ReplyDeleteOk, yeah, that's silly.
ReplyDeleteAlternatively one could argue that this is the compiler being stupid, but the chances of getting that fixed is infinitesimal compared to fixing the inline directive mess.
ReplyDeleteLars Fosdal I don't know why the String helpers were not marked as "inlined". When I tested them I noticed that having them inline was sufficient to get normal performance (ie. same as the non-helper variants functions, otherwise the compiler injects the whole shebang for copying strings, implicit parameters, setting up implicit exception frames, etc.)
ReplyDeleteOf course there are several string helpers that got "inventive" algorithms, and for those lack of "inline" is the least of their problems.
Eric Grange I'll add that as another piece of evidence to my theory that they replaced the core Delphi team with interns.
ReplyDelete{$WARN H2443 OFF}
ReplyDeleteReally, CHUA Chee Wee?
ReplyDeleteEven the official docs say it can't be done, and you just proved them wrong?
The reason of inlining MessageDlg is because this method just calls MessageDlgPosHelp which makes sense in theory. Overloading and calling the same method with more or less parameters is common practise. The problem is rather the compiler not being able to handle the redeclared types from System.UITypes that are used (TMsgDlgType and TMsgDlgButtons)
ReplyDeleteAlthough I guess this doesn't solve Asbjørn Heid's issue - as he still has to patch each unit.
ReplyDeleteI meant that inlining said method indeed makes sense and that I would classify the H2443 as a compiler bug in this case because everything is accessable by the calling unit.
ReplyDeleteP.S. Yes, it definetly is a regression since XE. I just made a small project to reproduce the H2443 and in XE it does not occur and successfully inlines whereas in XE5 it shows the hint and does NOT inline.
program Project1;
{$APPTYPE CONSOLE}
uses
Unit1;
begin
Test(one);
end.
----------------------------------------
unit Unit1;
interface
uses
Unit2;
type
TEnum = Unit2.TEnum;
const
one = Unit2.TEnum.one;
two = Unit2.TEnum.two;
procedure Test(value: TEnum); inline;
procedure Test2(value: TEnum);
implementation
procedure Test(value: TEnum); inline;
begin
Test2(value);
end;
procedure Test2(value: TEnum);
begin
Assert(value <> two);
end;
end.
----------------------------------------
unit Unit2;
interface
type
TEnum = (one, two);
implementation
end.
Stefan Glienke Nice find! We upgraded from D2010 straight to XE3, so didn't notice this. Want to add it to QC or shall I do the honor?
ReplyDeleteAsbjørn Heid Done: http://qc.embarcadero.com/wc/qcmain.aspx?d=119557
ReplyDeleteStefan Glienke Thank you, voted!
ReplyDeleteLars Fosdal Now that I'm back at my PC, no, {$WARN H2443 OFF} won't work. Turn off inlining at the project options level.
ReplyDeleteI had such high hopes, CHUA Chee Wee :)
ReplyDeleteThanks for checking!