revert to legacy error position handling

[wlammen]

The __FILE__ and __LINE__ macros define a source code location, which, for example, causes an error. This is a very standard way to mark a position.
The problem is: during development these locations may move because of code insertion or deletion. So this form of report is not stable across program versions, and may cause confusion. Metamath uses #nn markers instead, that can be easily found using text search, and do not have this drawback. Discontinue forced use of this kind of location marks in mmfatl.

[digama0]

I'm not convinced by this argument. Using __FILE__/__LINE__ means less maintenance overhead to allocate these error numbers and ensure they are distinct, and even if the file/line numbers change over versions of the program it is only because they are reflecting the actual location of the error in the code. If the user hits a fatal error in the program we want to make the output useful for follow-up debugging work and giving a line number seems like the best way to do that. (We should not use file/line for anything but fatal errors / bugs / panics though -- stuff that the user should ideally never experience. For errors which are the result of legitimate inputs which simply happen to be incorrect, we should use stable error codes and a good explanatory message. We also don't want these error line numbers to appear in tests, since that would make them more unstable across versions which increases maintenance.)

[wlammen]

"Discontinue" does not mean you cannot use it anymore. Let your message look like "Error location at %s:%u" followed by the position parameters, and you get the location info again. On the other hand, you can trick the former code to suppress the location by submitting the pair NULL:0. So the caller of the error handler is responsible for providing the message it wants the user to see in all details, anyway. This PR just removes a bit of convenience, if you strongly support such location reports in future. The function signature now is less suggestive of using it, and more on the side of the legacy # style, but it does not inhibit development in any direction. I think it should not be me who sets new trends here, this can be solved outside of the core error handler code.

[digama0]

Well the PR title seems to be setting a new trend, it says "discontinue support" and the function which handles it is being removed so it seems like this would actually be worse? I think we should make it easy to write BUG("stuff is borked") without further elaboration at the point of use. BUG should be a macro which picks up the file and line to make this short invocation more useful. I am not too concerned about the signature of the underlying function being called, whether the file/line is added in the macro vs in the function, but the "user code" should not be more onerous than this for a panic message which is being called in conditions that the programmer cannot even envision.

[wlammen]

The reason why I added the __FILE__ __LINE__ support to fatalErrorExit a few PRs before, are similar to your visions. But there is still too much left to decide. First you should encapsulate a program location in a dedicated struct. You can then evaluate whether one should extend the tuple to a triple by adding the program version. And, not lastly, the point where this data is submitted to the error handler need not be the error location. This can best be seen with the outOfMemory function, that is called in various places and need to forward the error location from its caller, that way adding more complexity. While this all is not beyond a limited amount of work, I still like to have this series of PR narrowed down to what needs to be done, and not much more.
I changed the title to something more to the point. Here we reach my personal limits in English. Using discontinue was a quick decision, and no alternatives came to my mind at that time.

[digama0]

I understand what you are saying, I just disagree that we should lose the ability to have file/line on panics. This is a deliverable of the error handling system, so whether it needs to use argument passing or macros or whatever doesn't really matter, it's trying to accomplish a job and if you remove the thing it needs to do that job then there is a feature regression.

Just to be clear, this is not in any way a judgment on your work or even the code in the PR, this is a discussion about the proposed policy change in the PR title/description.

If you have an alternative approach to putting file/line on panics, preserving the high-level behavior from the programmer's perspective, which simplifies the error handling logic, then that seems fine to me.

[wlammen]

It was a premature addition, not needed by the current source code, and serving some future expectations, that in my view could have been addressed in an extra branch. Anyway, it is now added as commented out code, fairly easy to activate in case of need. I hope this is good enough.

[digama0]

To the extent that it is not already serving this purpose, I have a feature suggestion for you... I don't want it to be commented out code, I want it to work. Quite frankly I value error messages with useful debugging information far more highly than error messages that print even in OOM conditions.

[wlammen]

The main reason that I am writing this is, that the architecture of metamath is flawed. It had (to some extent even has, although the amount is reduced now) cyclic dependencies, which makes it hard to

• document
• establish pre/post conditions
• modify the sources for the better in a safe manner

As a positive side effect, Metamath is now (provable) able to present diagnostic messages even under difficult conditions, although there are still some restrictions left. I did NOT work for this side effect alone.

Regarding your request: Why don't you address it yourself? A good starting time is when the error handler got finished, and a starting point the commented out source code.

[digama0]

The main reason that I am writing this is, that the architecture of metamath is flawed.

I don't see what this has to do with anything, nor how this PR makes the architecture any less flawed. Is there something problematic about printing file/line with respect to those cyclic dependencies you mention?

I do wish you would have a bit less of an ultimatum approach to your PRs. It should be possible for me to express disapproval of a PR without you taking it as some kind of judgment on your overall coding style or refactoring roadmap.

Regarding your request: Why don't you address it yourself? A good starting time is when the error handler got finished, and a starting point the commented out source code.

Huh? This is a revert PR, the behavior exists already and you are removing it. I don't want to lose that behavior, and I thought the point of reviewing and merging it in the first place was to assess that this is a good thing that we are interested in having and that the implementation of the behavior is good. The status quo is always to not change the code.

[digama0]

I'm going to close this PR. If I have time to implement file/line passing for outOfMemory, I will do that, but until then the current incomplete behavior is still better than nothing and this PR is a deliberate regression.

[wlammen]

There is nothing reverted that was in place, say, only a few days before. There is nothing reverted that adds a true value to the current usage. There is nothing reverted that I had not introduced myself. I wonder whether this matches the definition of regression at all. It is common development practice (in my world) to focus on one idea, and get it done, without opening a can of worms like "Let's introduce this and that first on the side, before continuing the main goal". I am also used to be respected as an author of a series of PRs under the same topic, and allowed to finish my idea first, before it may get dissected afterwards.

[digama0]

I do not like the idea of merging things and then immediately retracting them, without a very good reason (like a critical bug). Please don't PR things if you aren't ready for them to become permanent parts of the codebase. That is what the review process is about. If you are just playing around do that on your own fork.

[wlammen]

I do not like the idea either. I am on my private branch always ahead of what I submitted here, and the core ideas are developed and known here for almost a year now. This preparation does not prevent me from occasionally pushing something I regret a bit later, its just less probable. This is also tied to the idea of submitting a series of small PRs instead of one big (and possibly overwhelming) one with huge modifications. This strategy puts me into some kind of review mode as well, because I revisit my own code after some time again, and I might spot shortcomings while doing that. There is a slight risk I might be mistaken then, though.
I am not going to excuse me for what I did. This is part of how development and submission happens to be. If this leads to unpleasant results above some threshold, then the process should be adapted. BUT I DONT KNOW HOW. I tried several strategies, and always ended up in endless discussions like this one, or simply being ignored.

[digama0]

I am not going to excuse me for what I did.

I'm not asking for an apology, you didn't do anything wrong and I keep telling you not to take it personally. This PR is just not the direction I want us to take. If you submit a PR which replaces the code with a beautifully architected snake game implementation, it is possible for it to be good code with good style by a good programmer and yet still be rejected as a PR because it's not what this project is about. Submitting a replacement of the code by snake game in small pieces does not make the PRs any more likely to be accepted.

BUT I DONT KNOW HOW. I tried several strategies, and always ended up in endless discussions like this one, or simply being ignored.

There is no need to have a long discussion on each PR. I have said repeatedly ([1], [2]) what I'm looking for, I think you just don't want to do those things and are ignoring the advice. Benoit recently submitted a "small fixes" PR #121 after giving an indication that he was going to do so on a previous thread and getting a thumbs-up. These are the key steps to a successful interaction. If you are doing a refactor and I either don't know where it's going or I think it's going in the wrong direction then I will be asking a lot of questions or rejecting it depending on the situation. We need to be on the same page about the overall direction, and since you haven't submitted any "big-picture" issues which describe what your goals are we are continually misaligned.

[wlammen]

The core idea was to break dependency loops, which caused me headaches (meanwhile) a few years ago, and prevented me from further document the source, because I sometimes wasn't able to estimate the implications, let alone phrase them as reliable post-conditions. I am pretty sure I voiced this before. Some other measures like the refactor.log could have been a mixture of a road map and a progress bar, helping understand what is going on.
OK. If you don't think an upgrade in architecture is benefiting the software, but is best avoided, well then - not my nature, bye-bye then.

[digama0]

OK. If you don't think an upgrade in architecture is benefiting the software, but is best avoided, well then - not my nature, bye-bye then.

See this is very combative. You need to be more specific about what you are doing and how it is taking us in the direction of those goals you mention. I am not against upgrades in architecture, but can you please explain your plans in more detail than that?

The core idea was to break dependency loops, which caused me headaches (meanwhile) a few years ago, and prevented me from further document the source, because I sometimes wasn't able to estimate the implications, let alone phrase them as reliable post-conditions. I am pretty sure I voiced this before.

Indeed you have said so, and at this high level I agree. But (1) I think the main issue has already been fixed and (2) I can't see how e.g. removing file/line printing on errors has anything to do with that goal, it looks like a yak-shaving expedition gone wrong. Would it be possible to give a list of things you plan to do, specifically? That is, remove this function, add this other one, make everyone call through this interface, etc.

[wlammen]

No matter what goal you pursue unsuccessfully in your life (wooing for a girl, selling goods...), there is a common psychological misconception I know as "If you only had tried harder...". If you only had rang the girl's, customer's... door bell just once more, you would have been flooded with success. While not exactly false, life tells me, the opposite is way more likely.
Leaving the bloat like comments and test code aside, I tried to submit around 100 lines of code - from around June 22 on.
A preview of the final outcome was available both as a cancelled PR #86 and on my private GitHub repository. The concepts behind it had been laid open on various occasions, and are readily available on the internet (https://en.wikipedia.org/wiki/Acyclic_dependencies_principle and many more). But for each and every individual commit I need to wait, and invest more private resources than I do for other projects, and this time I am outright rejected. I am not under the impression that this community (or whatever it is) is supportive or at least tolerant to my ideas.
Enough is enough.

[digama0]

Are you asserting that your PRs were rejected despite following the advice I gave? Because I am asserting that they are rejected because they don't. Your time is your own to spend, but if you wish to collaborate and not simply make this into your personal project then you should try to compromise on the direction of your PRs. And please try not to make your PRs a part of your personal identity, that is very unhealthy, and it means that I can't review your code without reviewing you and how could that possibly end well? What should be a straightforward discussion somehow turns into a personal sleight.

I don't know what else to tell you. I've given lots of options and you don't seem interested in them, or at least you aren't taking about considering them and are instead talking about your past contributions (I won't rehash those discussions, leave that in the past).

If you want to know whether we will be supportive of your ideas for change, OPEN AN ISSUE. Don't submit code without a prior discussion unless you are prepared for it to possibly be rejected. And even if it is, so what? That's one bit of information, roll with the punches and try something else.

[wlammen]

I want to end the discussion and my collaboration here. You may have the final word.

[digama0]

If the project still interests you, consider working on a fork instead. You can pull commits from here, or we can pull commits from your fork, without needing to PR things directly. You can even suggest changes in issues which you have already fixed on your fork as a way of drawing attention to it, e.g. an issue "Printing fails in OOM situations" with a link to your fork which contains a fix.