[Rd] Better error message in loadNamespace

[Thomas Lin Pedersen]

> On 22 Jan 2018, at 16.21, Martin Maechler <maechler at stat.math.ethz.ch> wrote:
> 
>>>>>> Thomas Lin Pedersen <thomasp85 at gmail.com <mailto:thomasp85 at gmail.com>>
>>>>>>    on Mon, 22 Jan 2018 14:32:27 +0100 writes:
> 
>> Hi I’ve just spend a bit of time debugging an error
>> arising in `loadNamespace`. The bottom line is that the
>> `vI` object is assigned within an `if` block but expected
>> to exist for all of the remaining code. In some cases
>> where the package library has been corrupted or when it
>> resides on a network drive with bad connection this can
>> lead to error messages complaining about `vI` object not
>> existing. Debugging through the error is difficult, both
>> because `loadNamespace` is called recursively through the
>> dependency graph and the error can arise at any depth. And
>> because the recursive calls are wrapped in `try` so the
>> code breaks some distance from the point where the error
>> occurred.
> 
>> I will suggest mitigating this by adding an `else` clause
>> to the `if` block where `vI` gets assigned that warns
>> about potential corruption of the library and names the
>> package that caused the error.
> 
> Not sure this is desirable... in general even though it may well
> be desirable in your use case...
> 
> You will be aware that this an important function that maybe
> called many times, e.g., notably even at R startup time and so
> must be very robust [hence the many try* settings] and must use
> messages/warnings that are suppressable etc etc.

I absolutely agree that even small changes in execution speed 
would be bad in such a low-level function. Still, the else clause 
would only get called in the event an error should be thrown so I
can’t envision any performance regression.

> 
> On reading the source, I tend to agree with you that it looks
> odd there is no  else  clause to that if(), but then there may
> be subtle good reasons for that we don't see now.

If there are reasons for the current construct, then that should of
course be taken into account. As far as I can parse every code
branch that follows the if statement ends up referencing vI, but
lazy evaluation might result in those expression to never be 
evaluated so it might be valid calls in some circumstances..?

Anyway, I can accept the argument that changing it might break
things in unexpected ways :-)

> 
>> I can open a bug report if you wish, but I would require a
>> bugzilla account for that. Otherwise you’re also welcome
>> to take it from here.
> 
> I'll do that for you in any case.

thanks

best
Thomas

> 
> Martin Maechler
> ETH Zurich
> 
> 
>> With best wishes Thomas Lin Pedersen


	[[alternative HTML version deleted]]