IfThen(Assigned(Widget), Widget.Description, 'No Widget') doesn't crash. Should it?

Question

In code that I help maintain, I have found multiple examples of code that looks like this:

Description := IfThen(Assigned(Widget), Widget.Description, 'No Widget');

I expected this to crash when Widget was nil, but when I tested it, it worked perfectly.

If I recompile it with "Code inlining control" turned off in Project - Options - Compiler, I do get an Access Violation.

It seems that, because IfThen is marked as inline, the compiler is normally not evaluating Widget.Description if Widget is nil.

Is there any reason that the code should be "fixed", as it doesn't seem to be broken? They don't want the code changed unnecessarily. Is it likely to bite them?

I have tested it with Delphi XE2 and XE6.

Answer 1

Personally, I hate to rely on a behavior that isn't contractual.

The inline directive is a suggestion to the compiler.

If I understand correctly what I read, your code would also crash if you build using runtime packages.

inlining never occurs across package boundaries

Like Uli Gerhardt commented, it could be considered a bug that it works in the first place. Since the behavior isn't contractual, it can change at any time.

If I was to make any recommendation, I would flag that as a low priority "fix". I'm pretty sure some would argue that if the code works, it doesn't need fixing, there is no bug. At that point, it becomes more of a philosophical question (If a tree falls in a forest and no one is around to hear it, does it make a sound?)

Answer 2

Is there any reason that the code should be "fixed", as it doesn't seem to be broken?

That's really a question that only you can answer. However, to answer it then you need to understand fully the implications of reliance on this behaviour. There are two main issues that I perceive:

1. Inlining of functions is not guaranteed. The compiler may choose not to inline, and in the case of runtime packages or DLLs, a function in another package cannot be inlined.
2. Skipping evaluation of an argument only occurs when the compiler is sure that there are no side effects associated with evaluation of the argument. For instance, if the argument involved a function call, the compiler will ensure that it is always evaluated.

To expand on point 2, consider the statement in your question:

Description := IfThen(Assigned(Widget), Widget.Description, 'No Widget');

Now, if Widget.Description is a field, or is a property with a getter that reads a field, then the compiler decides that evaluation has no side effects. This evaluation can safely be skipped.

On the other hand, if Widget.Description is a function, or property with a getter function, then the compiler determines that there may be side effects. And so it ensures that Widget.Description is evaluated exactly once.

So, armed with this knowledge, here are a couple of ways for your code to fail:

1. You move to runtime packages, or the compiler decides not to inline the function.
2. You change the Description property getter from a field getter to a function getter.

If it were me, I would not like to rely on this behaviour. But as I said right at the top, ultimately it is your decision.

Finally, the behaviour has been changed from XE7. All arguments to inline functions are evaluated exactly once. This is in keeping with other languages and means that observable behaviour is no longer affected by inlining decisions. I would regard the change in XE7 as a bug fix.