From System.JSON:
From System.JSON:
function TJSONValue.Cast: T;
var
Size: Integer;
LTypeInfo: PTypeInfo;
LValue: TValue;
begin
LTypeInfo := System.TypeInfo(T);
if not AsTValue(LTypeInfo, LValue) then
raise EJSONException.CreateFmt(sCannotConvertJSONValueToType, [Self.ClassName, LTypeInfo.Name]);
Result := LValue.AsType;
end;
Size variable is not used. Should we report it to QC?
function TJSONValue.Cast
var
Size: Integer;
LTypeInfo: PTypeInfo;
LValue: TValue;
begin
LTypeInfo := System.TypeInfo(T);
if not AsTValue(LTypeInfo, LValue) then
raise EJSONException.CreateFmt(sCannotConvertJSONValueToType, [Self.ClassName, LTypeInfo.Name]);
Result := LValue.AsType
end;
Size variable is not used. Should we report it to QC?
I wonder if 'size' isn't removed automagically by the compiler
ReplyDeleteThat's not really the point here
ReplyDeleteTell Roman Yankovsky to run FixInsight over it. ;-)
ReplyDeleteFixing compiler hints/warnings isn't a high enough priority, IMNSHO. It's a poor reflection on the products codebase if it is shipped with them; especially since they're the ones providing the compiler in the first place. It'd be a lesser problem for 3rd party code, however it still projects a poor image. It's like saying: "yeah, we don't care about those things".
ReplyDeletealso the raise ... should be moved to a local subproc, to avoid expensive internal finalization (see asm cpu)
ReplyDeleteDavid Nottage This is not how raise should be used?
ReplyDeleteJohn Kouraklis I'm not talking about the raise; I'm talking about the unused variable, Size.
ReplyDeleteDavid Nottage Yes I know; Sorry I meant to include David Berneda in my comment :-)
ReplyDeleteJohn Kouraklis when calling raise with managed types like strings, even if the exception does not happen, the compiler adds a finalization code at the end of the proc
ReplyDeleteDavid Berneda Actually that has nothing to do with the raising itself but rather with the fact that the compiler generates variables for all that managed stuff - in this case for the resourcestring (more precisely for the result of the implicitly called LoadResString function).
ReplyDeleteSuch cases happen more often than you think and often in routines that don't have exceptions at all but call functions that return managed types (like string). Worst case I had was a case statement and calling more or less the same function in different cases which resulted in the compiler generating implicit variables for each of them. That caused a huge overhead in prologue and epilogue of the routine.
FWIW the entire code inside of System.JSON dealing with generic type conversion using TValue is completely ridiculous.
ReplyDeletePersonally I think one should fix all hints and warnings. Not doing so is disrespectful. I would feel bad about myself if releasing code in such state. If there is not even a hint/warning review and cleanup, what can I suspect about quality? If you don't care about hints and warnings, why would you care more about the code being correct? Why would you even care about performance?
ReplyDeleteAgustin Ortu This method has a generic parameter and the compiler fails to raise some warnings and hints in such methods.
ReplyDeleteStefan Glienke I didn't know that! Thanks for the clarification. Clearly that should be reported to QC
ReplyDeleteReported: https://quality.embarcadero.com/browse/RSP-16553
ReplyDeleteI really think this is critical
Marco Cantù David Millington Nick Hodges (I'm tagging you guys in case you missed the post)
ReplyDeleteAgain, I insist this is very important
Agustin -- thanks for reporting this.
ReplyDelete