kcrossen commented 2 years ago

Calc bug report template version: 1.2 2022-02-24

Describe the bug Under different circumstances assigning a value to a variable has different results. If the variable is assigned the value of a list member real integer 2, we get the expected results. But if the same list member is pi, we get a different (wrong) result. Freeing the variable before assignment gives the correct result in both cases.

To Reproduce See attached file

How you started calc In my case calc is embedded in a GUI calculator UI with different "modes", e.g. RPN
Calc commands and their output See attached file

Calc compiles perfectly and nearly always performs perfectly.
Indicate where the problem is See attached file, copy/pasted directly from working code.

Expected behavior See attached file, there are two examples, one working correctly, the other working incorrectly. There's also a work-around that solves the issue under all known circumstances, problem being that according to documentation, work-around should not be required.

Have also brute force work-around, which is approximately: ... user clicks on a visible rpnstack entry, expected behavior is for the internal value of that rpnstack entry to be pushed ... QString rpn_commands = ""; QString clicked_stack_internal = Trim_Calc_Result(Calc_Evaluate("estr(rpnstack[" + stack_idx + "])")); ... code that modifies rpnstack ... rpn_commands += "push(rpnstack, " + clicked_stack_internal + ");";
... execute Calc command in rpn_commands ...

Attach make debug output See attached file.

Screenshots See attached file

Execution environment (please complete the following information):

OS: macOS 11.6.3 and Android version 12, both exhibit same symptoms
OS version: see above
OS distribution: macOS Big Sur, Android 12 UI 4.0 Samsung Galaxy Tab S7
Calc Version Calc 2.12.4.14, searched release notes for 2.14.0.15 without seeing this bug.
Shell and shell version NA

Calc mods Calc source has been modified such that all calls to OS services I/O, file system, etc. have been eliminated. This is so that the Calc engine can be embedded as the "micro code" of other applications, e.g. a GUI calculator. This modification has been working correctly at least since Calc 2.12.4.14 was released. The previous version, released 5 years ago can be found here: https://github.com/kcrossen/APCalculator

What prompted this report is while updating/enhancing APCalculator and porting to Android, more features of Calc were being used that weren't being used previously.

Patch If you have a recommended code patch to address the problem, please attach your file

Additional context Calc 2.12.4.14, searched release notes for 2.14.0.15 without seeing this bug. Error_Report.txt

kcrossen commented 2 years ago

Both work-arounds (one shown in Error_Report.txt the other shown above) work correctly in all tested circumstances.

kcrossen commented 2 years ago

Better answer to question about modifications: The modified files are symbol.c, qmath.c, zio.c, and most importantly codegen.c.

For symbol.c, qmath.c the only change was to substitute my own out_str(...); for your printf(...);

My zio.c directs what would have been C output streams into designated buffers.

The key modification to codegen.c: / @krc@ / EXTERN int calc_use_scanerr_jmpbuf; EXTERN int calc_use_matherr_jmpbuf;

int execute_commands(BOOL toplevel) { calc_use_scanerr_jmpbuf = 1; calc_use_matherr_jmpbuf = 1;

if (setjmp(calc_error_jmpbuf) == 0) {
    inittokens();
    getcommands(toplevel);
    return 0;
}
else { /* There was an error, return gracefully */
    return 1;
}

} / @krc@ / This was derived initially by executing/tracing your code.

Each execution of this function is designed to be effectively executing an independent "file", except that already defined variables and functions retain their values/definitions.

lcn2 commented 2 years ago

Thank you, @kcrossen for your willingness to re-submit your bug report.

Attach make debug output See attached file.

We only see the attached Error_Report.txt file. Did we miss something? If you do attach the output, please consider doing so on your macOS platform using your modified code base.

lcn2 commented 2 years ago

Better answer to question about modifications: The modified files are symbol.c, qmath.c, zio.c, and most importantly codegen.c.

For symbol.c, qmath.c the only change was to substitute my own out_str(...); for your printf(...);

My zio.c directs what would have been C output streams into designated buffers.

The key modification to codegen.c: / @krc@ / EXTERN int calc_use_scanerr_jmpbuf; EXTERN int calc_use_matherr_jmpbuf;

int execute_commands(BOOL toplevel) { calc_use_scanerr_jmpbuf = 1; calc_use_matherr_jmpbuf = 1;
if (setjmp(calc_error_jmpbuf) == 0) {
    inittokens();
    getcommands(toplevel);
  return 0;
}
else { /* There was an error, return gracefully */
    return 1;
}
} / @krc@ / This was derived initially by executing/tracing your code.

Each execution of this function is designed to be effectively executing an independent "file", except that already defined variables and functions retain their values/definitions.

Please consider attaching a patch (diff -u) for symbol.c, qmath.c, zio.c, and codegen.c between the released2.14.0.14 calc code and your code, @kcrossen.

lcn2 commented 2 years ago

If there is a special patch to make your code base easier to develop from, please consider attaching a suggested diff -u, perhaps with some sort of:

#if defined(GUI_CALCULATOR_UI)
...
#endif /* GUI_CALCULATOR_UI */

surrounding the special code. If attached as a patch, we would be happy to consider adding it to a future calc source code release. We would not compile with -DGUI_CALCULATOR_UI as a default, but you could in your code such as in your Makefile.local.

If there is part of the patch that would be beneficial to the wider calc community, then of course such an #if defined(GUI_CALCULATOR_UI) ... #endif /* GUI_CALCULATOR_UI */ would not be used around that part of your patch.

Doing so will help us understand where any " bug fix" is found and where any "patch to make our use case easier" is found.

kcrossen commented 2 years ago

Here's the output as requested Enhanced_Error_Report.txt .

kcrossen commented 2 years ago

The above attached txt file shows a cleaned-up-for-clarity version of the qDebug() output of the executed code. I'm developing in Qt principally because I've found no other reliable way to port code to Android. The Android SDK has defeated me numerous times (e.g. hard-deprecating code before successor code is feature-complete). Kivy is not consistently maintained.

In each of the three run instances, there are effectively three steps. Each step is some variation of: QString rpn_commands = ""; rpn_commands += "global err_value;err_stack=list();"; rpn_commands += "push(err_stack, pi);"; // rpn_commands += "push(err_stack, 2);"; rpn_commands += "push(err_stack, 1i);";

RPN_Commands_Execute(rpn_commands);

qDebug() << "After: " + rpn_commands;
qDebug()  << "err_stack[1] == " + Trim_Calc_Result(Calc_Evaluate("estr(err_stack[1])"));
qDebug()  << "err_stack[0] == " + Trim_Calc_Result(Calc_Evaluate("estr(err_stack[0])"));
qDebug() << "...";
qDebug() << "...";

In other words, the value of the same string that was submitted to calc followed by a listing of the state of the stack (list) that resulted. In the case of the last step, the value of the "transfer" variable is also shown.

I should note also that this is the first/only anomaly I've found over more than five years of intensive use of your code.

kcrossen commented 2 years ago

With regard to the requested diffs, I'm embarrassed to say that I don't know how to do that. I started programming professionally in 1966 (yup) as a better way of earning living expenses than washing dishes in the dorm. Trained in EE to design transistors (obsolete when I graduated) I drifted into manufacturing which is not very unix/etc. command line intensive (VB mostly). I'm using DiffMerge to do diffing.

kcrossen commented 2 years ago

diff_codegen.txt diff_qmath.txt diff_symbol.txt diff_zio.txt

kcrossen commented 2 years ago

Sources are here: https://github.com/kcrossen/APCalculator/tree/master/libcalc_sources

lcn2 commented 2 years ago

Is there s case, with an unmodified release of calc, where entering commands into the terminal will trigger this problem OR perhaps is there calc resource file (some foo.cal file) that when read (executed by calc) will demonstrate this problem using an unmodified release of calc? If so, please provide that.

We are trying to develop, if it is possible, a test via the regress.cal regression suite that would show this problem being triggered in the an unmodified release of calc.

Or does this problem arise with those @ krc @ source code modifications?

We are happy to look into this, if we can, either way. We are just trying to understand, how to trigger the problem, if this is some regression in the main calc code base, or an issue with how calc was modified by someone.

lcn2 commented 2 years ago

You filed:

Attach make debug output See attached file.

We see no output from made debug in this bug report. Please attach it.

When you do, please let us know if that output is from the unmodified release of calc, or one that was modified as indicated in one of your comments above.

lcn2 commented 2 years ago

diff_codegen.txt diff_qmath.txt diff_symbol.txt diff_zio.txt

There appear to be references to other files, such as User.h in your diff_codegen.txt:

#include "User.h"

The User.h is not a part of the calc source code and it is not a known standard C include file.

Are you sure you included all the medications to the calc source code base?

kcrossen commented 2 years ago

Have now identified the problem via tracing. The initialization code does this ""protect(pi, 0);pi=pi();protect(pi, 7);protect(e, 0);e=exp(1);protect(e, 7);". The purpose of these is to set up "convenience" variables such that users "understand" pi and e as constants rather than functions. Obviously it would be counter-productive/confusing to allow the user to change these.

And: "If B is a variable with positive status and assignment of B to A is permitted, execution of the assignment A = B adds to the protections of A all protections of B that A does not already have." (from calc help)

This could be understood as protection "inheritance", a very powerful feature, but not what the user might anticipate. Any assignment of pi or e to a variable "freezes" that variable. My choices would then be: a) forcing the user to understand pi and e as functions, b) remember to "protect(user_variable, 0);" at least after assignment from pi or e, or "define pi() { x = pi; protect(x, 0); return x};" which puts me back to square one.

More intuitive (for my application anyway) would be some default or initializable defense against protection "inheritance", in effect giving some variables the same no-inherited-protection as builtin functions. Like 256 A protections not inherited. In any case, it's a rational, reasonable, potentially useful alternative.

I'm certainly not complaining about protection itself, but protection "inheritance" is too powerful, too "omnipotent", and not very intuitive or usual in typical programming practice.

OR have I missed something?

kcrossen commented 2 years ago

My goal is to make your Calc engine accessible to folks who don't have a command-line background

.

kcrossen commented 2 years ago

These are from macOS but it looks about the same on Android. Just such a pain to do screenshots on Android.

lcn2 commented 2 years ago

Have now identified the problem via tracing. The initialization code does this ""protect(pi, 0);pi=pi();protect(pi, 7);protect(e, 0);e=exp(1);protect(e, 7);". The purpose of these is to set up "convenience" variables such that users "understand" pi and e as constants rather than functions. Obviously it would be counter-productive/confusing to allow the user to change these.

And: "If B is a variable with positive status and assignment of B to A is permitted, execution of the assignment A = B adds to the protections of A all protections of B that A does not already have." (from calc help)

This could be understood as protection "inheritance", a very powerful feature, but not what the user might anticipate. Any assignment of pi or e to a variable "freezes" that variable. My choices would then be: a) forcing the user to understand pi and e as functions, b) remember to "protect(user_variable, 0);" at least after assignment from pi or e, or "define pi() { x = pi; protect(x, 0); return x};" which puts me back to square one.

More intuitive (for my application anyway) would be some default or initializable defense against protection "inheritance", in effect giving some variables the same no-inherited-protection as builtin functions. Like 256 A protections not inherited. In any case, it's a rational, reasonable, potentially useful alternative.

I'm certainly not complaining about protection itself, but protection "inheritance" is too powerful, too "omnipotent", and not very intuitive or usual in typical programming practice.

OR have I missed something?

It is we who have missed a lot of critical info from you:

We have had great difficulty in obtaining the basics of a bug report. We have yet to even determine that you are reporting as bug in the released, unmodified, code base of calc.

We have asked, several times, for information to help us understand your compile environment. While there is a slight chance we missed the supplying of that information, and there is a chance that you may have attempted to attach it and failed to do so, the most likely explanation is that you did not provide the critical information.

The information that we typically ask in the BUGS file (that is found in the calc source code) is missing. True, our initial template for calc issues failed ask for that information: we will give you that. But even when asked you to re-submit with the new template, even then we asked for the data to better understand your Environment. We did receive critical data and do your environment remains a block box to us.

We have also asked for a case where the released code base fails. Even in this comment above, you fail to show a case where the code is failing. Had you given is something like:

$ calc -v
C-style arbitrary precision calculator w/custom functions (version 2.14.0.14)
$ calc -e 'a=2; b=3; c=a+b; if (c != 5) { print "wrong value"; }'
wrong value
$ make debug > debug.out.txt
# see the attached debug.out.txt file

we might have been able to understand the issue.

We have tried to understand the modifications you have made to the code. Even when we detected in a diff the existence of some include file (such as User.h) and asked you about it, we received no information. It was as if you simply ignored our request for information.

A simple:

$ diff -f calc.2.14.0.14 my.modified.calc > calc.diff.txt
# see the attached calc.diff.txt file

Our best guess is that you have a request for some calc feature: that there is NOT a bug in the code base of calc. Another guess is that somehow the modifications you have made to calc is causing problems: problem that other calc users appear to be not reporting.

With all due respect, we are going to close this bug report. We close this bug report reluctantly because of:

A lack of a clear and concise bug report
Unknown mods you or someone else made to calc with possible side effects
This might be a possible enhancement request instead of a bug in the calc source code base
Trying to extract even the basic bug report information from you in order to try and to help has been unsuccessful

Regarding the modification you or someone else has made / wants to make to the code base:

PLEASE look at the custom function interface that is in calc. There are several projects that make effective use of custom functions to support their software and sometimes hardware platforms. From what we can see in those diffs, some of the modifications may have been better handled thru in invocation of custom services. Also, you may be able to minimize the impact in the code base through C pre-processor macros.

In regards all this being some possible calc enhancement:

Calc has a large support based that is spread over a wide set of platforms, including Android. There is a significant super-computer use, and use of calc by hardware accelerators. As such we must be extremely careful with ANY changes to the calc code base. Calc is full of extremely subtle code that has been extensively tested and improved on over many years.

Touching the code as you seem to have done is not easy to get right or even to see how your changes may have caused problems. Support of your GUI application would involve significant testing on the part of many people over an extensive period. To protect the code base, we would have to restrict the mods to non-fault compile options and/or custom functions to enable behavior.

Right now, the primary maintainer of calc is under a significant programming load / pressure relating to critical internet infrastructure as well a major effort on another open-source programming project that people are depending on Landon's efforts. So please understand that a calc enhancement will have to stand a line behind other requests for calc maintainer time. While your GUI use case looks interesting, addressing it right now or int he near future would be difficult.

Sorry to have to close this bug report:

We wish we could have helped you. Just in case it was Landon who missed something, others were asked for their opinion in your report. Multiple people stopped what they were doing and put a fair amount of effort in trying to even understand what you were reporting: all those attempts failed. We really tried to help: but others gave up in frustration. Landon was even asked by one person to stop asking them for help in this bug report.

We are closing this bug report as invalid, sorry.

Best wishes.

kcrossen commented 2 years ago

I think we are speaking different languages & live in different worlds. I don't even understand what you asked for that I could have provided and did not provide to the best of my ability. I live in the world of Qt Creator, for better or worse. I've run into similar problems working with the AWS C++ SDK manager.

I have modified the code base, seemingly successfully, maybe seven years ago. I don't even know anymore how to compile/use the original code base.

You apparently want output in some particular format. I don't know how to generate that form of output, I apparently don't even understand what it is that you want, so I sent the only form of "output" I know.

I humbly apologize for all the difficulties my shortcomings have caused all of you. Sincerely I do.

pmetzger commented 2 years ago

FYI: https://www.chiark.greenend.org.uk/~sgtatham/bugs.html

lcn2 / calc

Bug: Variable value setting depends on RH source values #51

It is we who have missed a lot of critical info from you:

Regarding the modification you or someone else has made / wants to make to the code base:

In regards all this being some possible calc enhancement:

Sorry to have to close this bug report: