in reply to Re (tilly) 3: Supersplit
in thread Supersplit
Everything that I said about BEGIN is true. But my promise that Perl 5.8 will have a version of Carp which fixes the begin-time Exporter issue is incorrect. Here are some details on the situation.
Suppose that you have 3 packages, A, B, and C. Suppose that B and C both inherit from A but C does not inherit from B. Today when C makes a call into B and then B makes a call into A which generates an error, Carp will not label the error as coming from A, B, or C, but instead will report it from whoever called C. In 5.8 the error will in this case be reported from C because Carp has not been told that B trusts C.
This is good because the fact that B inherits from A is an internal implementation detail which C should not have to know about. What is not good at the moment is that the trust relationship is synonymous with "inherits". But the code for Carp has been written in a way where this can be fixed by just replacing the trusts_directly() function in Carp::Heavy with something smarter. This is all intended.
What I had not thought through, though, is the situation where C calls a method in B, which actually is inherited from A. Now when Carp looks at the information, what it sees from caller is that C called a function in A. But C trusts A, so that call is trusted. In fact if the call generates an error, then C should be fingered because B does not trust C, and where the error is reported from should not depend on that implementation detail. But the critical piece of information required is not reported by caller, on a method call we don't know what call was intended, we only know what function was called. If we had a way to get that information, then set $called to the appropriate thing in the short_error_loc() function in Carp::Heavy, and the underlying problem would go away in Perl 5.8. But unless someone feels motivated to champion this on p5p, that won't happen and in this situation the error will continue to be incorrectly reported.
This last situation is exactly what we get with Exporter and playing games with @ISA during compile time. If you do not play the games, then while your code is being compiled you do not inherit from Exporter during your use invocations, so you can be correctly fingered as causing errors. If you do set @ISA bright and early, your use of other packages will go to Exporter's import() function, and then when Carp tries to figure out the error it will decide not to finger you because what it sees is you calling a function in a package you inherit from.
An incidental note. Capturing information about method calls in caller would allow that information to be properly propagated in Perl's SUPER mechanism. This is a necessary step towards having Perl's SUPER mechanism play nicely with multiple inheritance. Of course making that change in Perl 5 raises backwards compatibility issues, so likely a new keyword would need to be created. That would require even more championining. Since I am personally of the opinion that multiple inheritance leads to overly fragile and complex design issues, I won't be seen caring much either way, but some others might...
|
---|
Replies are listed 'Best First'. | |
---|---|
(tye)Re2: Supersplit
by tye (Sage) on Jan 01, 2001 at 11:20 UTC | |
First, my comment about using qr// was specific about when writing modules. If you are writing modules then you should consider other peoples' environments. So I think the minor effort to support what are still quite common versions of Perl to be good manners. After all of the above, I am even more convinced that: The documentation for Carp.pm is quite simple, saying that the error will be reported as coming from the caller (not from the caller of the caller of the caller etc. for as long as it takes to get out of your inheritance heirachy). I think this is a much better idea. My first run-in with Carp.pm skipping more than one level of the call stack kinda made sense since it allowed your module to have a helper routine that didn't check all of its arguments closely and could pass on bad arguments to an underlying routine and in that specific case, it makes sense to have the error report two levels up. Well, sorta. At least it doesn't help much to have an error reported in your helper routine. I say "sorta" because I'd better be pretty careful in my underlying routine to word my error message clearly. It doesn't do much good to have code fred("bar") report "_fetchField() called with undefined field name". So I think that in most cases, even the best case I that can come up with, skipping more than one level of call stack doesn't really give a better message than just simply always skipping exactly one level would do. I think it makes much more sense to let those specific error messages (that are very carefully worded and carefully chosen such that they really were caused more than one level up and will be understood when reported more than one level up) should call carp() in such a way that carp() knows how far up the stack to climb. Now skipping the whole inheritance heirarchy is just madness. So everyone who uses HTML::ParseTree (or whatever some module is that expects the user to use inheritance in order to make use of the module) should be getting their programming errors popped up to the next higher package instance? I can see wanting carp() to skip a couple levels of inheritance heirarchy in very rare cases where I have several packages that are developed together, probably in a single source file, and some specific arguments are not checked until we get a few levels deep. Then only errors about those specific arguments should be allowed to bubble up the appropriate level. But just because I inherit from a class certainly doesn't mean that I should avoid checking my arguments for validity before passing them along to that class. But this seems to be what the designers of Carp assume, no? If I inherit from someone else's package, I really do want to be told when I use their package wrong. I will write my code such that errors passed in by users of my code will be detected by my code, not blindly passed on to the other module. So the fact that Exporter has a "bug" in that it doesn't take into account the bad design flaw of Carp and this bug can be worked around by making the inheritance tree inconsistant over time, isn't really a good argument for encouraging such inconsistancy. I think you may be too caught up in the whole Carp mess. (: And all of this just enforces my commitment that lots of things go on between declaring @ISA and run-time initializing @ISA and it doesn't make sense to leave your inheritance in an inconsistant state during all of this processing. Sorry, I don't have time right now to dig up the easy way to break things when you don't use BEGIN or use base. But several other people got convinced enough that patches were issued such that BEGIN blocks were actually documented as the proper way to initialize your package variables like $VERSION and @ISA. - tye (but my friends call me "Tye") | [reply] [d/l] [select] |
by tilly (Archbishop) on Jan 02, 2001 at 06:46 UTC | |
So now on to the more substantial BEGIN issue. Having read and thought about what you wrote, the following points summarize my current position fairly well: More on those points.
OK, so this is far, far more than I intended to write, but it hopefully is interesting. I don't think that the concept of Carp is broken. I am emphatically unconvinced that BEGIN games are justified. And I have just said far more on that than I should. :-) | [reply] |
by tye (Sage) on Jan 04, 2001 at 12:05 UTC | |
I prefer to write good error messages and then depend on them. You prefer to use the debugger. ...over writing good error messages? No. And I never said that. That is quite a big jump you've made ): I just said that I prefer using a debugger over adding temporary trace. But I did figure out some of our disagreement. I'm really only talking about Carp::croak(), which I feel was meant for what I call "precondition failures". In many other languages, the simple preconditions for a function are that you specify the right number of arguments and that they each have the correct type. These are checked at compile time. Perl doesn't offer such compile-time preconditions most of the time (rarely you can reasonably use prototypes for this) so a Perl programmer who includes lots of error checks is going to generate fatal errors when a function gets the wrong number of arguments, for example. This type of error is extremely likely to just be a simple coding error in the immediate caller of the function (more on this later). So it makes sense to have Carp::croak() report the error as having come from the immediate caller and including a stack back trace is not likely needed. From the documentation, this seems to be what Carp::croak() does and what it was intended for. But it appears that the documentation is wrong and Carp::croak() looks at inheritance to decide how far up the call stack to go. Now, I can see some use for generating warnings that seem to come from the user code and can see using inheritance to figure that out (imperfect but not too unreasonably so)1. But a fatal error is a different matter. For the module to try to kill the script, there must be a serious problem. These usually amount to precondition failures or assertion failures. Simple precondition failures can be pinned on the immediate caller (modulo wrappers to be discussed shortly) because the source of the error is almost always right there. Complex preconditions should include a stack trace (which can skip the current function context if you like). Assertion failures should also include a stack trace. That is because we may suspect that the user code is at fault but we shouldn't delete useful information about the state of the module even though we don't think that this is the source of the problem. If we know that the user code is at fault, then we should be returning a failure indication (or throwing an exception), but not trying to kill the script. I have this tiny suspicion that Carp::croak() used to work this way but then had patches applied that didn't include documentation changes. But that is just a tiny hunch. Now, Perl doesn't have a preprocessor so you often end up with simple wrappers in your module (that don't check their arguments before passing them on) so the simple preconditions might be due to code more than one level up the call stack. Since a module should be tested before it is released, there shouldn't be any precondition failures that are due to code in the same module. So it makes sense to bubble up within the module. Now, for some huge packages of modules like LWP or CGI, there are lots of modules developed together. Well, CGI never croak()s. LWP croaks a whole 6 times. Either force it to specify that each or all of its calls to Carp::croak() should skip more than one package or have Carp.pm use inheritance along with the fact that all of the package names fall under the same hierarchy. The few big packages of modules shouldn't be setting the behavior for Carp.pm when it is easy (like it is) to find cases that this behavior breaks for. I didn't look at LWP long enough to see whether any of the croak()s could have originated from user code and passed through a different module/package of LWP (thus requiring an @ISA scan to find the user code and the proper place to report the error). But I did notice that several of the error messages would be pretty confusing if this did happen. In some ways, I don't think what Carp.pm is doing is horribly wrong. But it is starting to sound to me like you are "fixing" Carp.pm without a design document and that is part of our problem. So Exporter complaining about %EXPORT_TAGS being set up incorrectly should be throwing a precondition failure which shouldn't (by default) look at (just) inheritance. "Fixing" the problem by lying about your inheritance during a window of time is what sounds like "playing games" to me. What you keep calling "playing cute games" is simply initializing variables close to when you declare them and before you might use them. This is often easy in Perl. But for "static" variables, doing so currently often requires the very simple step of putting the initialization into a BEGIN block. Nothing fancy nor tricky nor in the slightest way "cute". (: And in this case in particular you will guaranteed get wrong behaviour from both old and current versions of Perl Nope. Never have and probably never will. I'd only get wrong behavior if I'd also made stupid mistakes about setting up %EXPORT_TAGS or such. And then I'd have to do that where I have multiple packages inheriting from each other. Yep, I pretty much guarentee that I won't ever run into that. Even if I did, I'd find those during module testing where the error being reported in the wrong place is pretty minor. Not initializing statics at compile time has bitten me several times, for different reasons each time. The only specific cause that I recall is circular module dependancy which isn't something you run into often but then that only accounts for one of my bites. I've seen this advocated by several people, so I'm sure they've been bitten by it too. I'm not telling you to trust their decision about whether or not to do this, I'm telling you that several people have been bitten by it enough that they decided to change. You have one case of the reverse that I know I don't care about. I have one specific case that I suspect you don't care about. But there are quite a few cases out there on my side. Plus, your case really seems to me to be a case of two wrongs making a right. Carp.pm gets the inheritance tree wrong which prevents its other bug from firing. I consider it evidence that you should initialize statics in BEGIN blocks, because look at all that can happen during that window! Well you know my opinion about the stupid misuses of $Carp::CarpLevel in the past. First of all when proper coding depends on programmers always getting things right, that is guaranteed to go wrong often. Um, so if programmers have to get things right then sometimes they won't and then things will go wrong. Is that a tautology? Yeah, $Carp::CarpLevel was a terrible design choice. But having a per-module equivalent of $Carp::CarpLevel as well as a perl-call equivalent would be useful and fairly easy to do. And if it designed well and documented, then programmers will very often get it right. Somehow you think that you setting the one true behavior, that you will get it right more often than programmers will? Only if the programmers are given a horrid, undocumented way to do it. Don't let how bad $Carp::CarpLevel was prevent you from designing and implementing a good solution. Now, you can certainly choose to let this one obscure bug prevent you from coding defensively when it comes to uninitialized static variables; but I think that would be a mistake. :) 1 I'm not a big fan of warnings from modules. Warnings from a module can be quite useful to the script writer (aka module user) but are usually delivered to the script user (non-programmer). The script writer can trap them with $SIG{__WARN__}, for example, but that will also catch warnings that are meant for the script user. So if I want to send a programmer warning from a module, I'll provide a delivery mechanism other than warn. - tye (but my friends call me "Tye") | [reply] |
by tilly (Archbishop) on Jan 07, 2001 at 23:30 UTC | |
Re: Re (tilly) 4 (gotcha): Supersplit
by Anonymous Monk on Jun 13, 2001 at 23:33 UTC | |
| [reply] |