On 12.07.2024 15:25, Rick McGuire wrote:
Looking at your CreateTraceObject code, it appears there were several conditional tests where it appears it was possible to not have an available RexxActivation. My suggestion be to make the code that creates the TraceObject into a static method and have add the activity and the activation as arguments, restoring the conditional tests from the original.
+1 Committed [r12851]. ---rony
On Fri, Jul 12, 2024 at 9:05 AM Rick McGuire <object.r...@gmail.com> wrote: On Fri, Jul 12, 2024 at 8:49 AM Rony G. Flatscher <rony.flatsc...@wu.ac.at> wrote: Unfortunately, when doing a global trace (e.g. "::options trace i") there are crashes with the refactored version, hence I uncommitted the changes. The problem is in RexxActivation.cpp where in global trace "this" is NULL. Even if checking for NULL and supplying defaults for that situation there is problem in trying to invoke Activity::traceOutput(...) as there is no activity available if in RexxActivation "this" is NULL. What would be the proper solution for this situation? Figure out why the RexxActivation is NULL, it's called debugging. The first step would be to figure out how you got to that point by examining the stack trace. Ignoring any invocation of processing the trace in such a situation? ---rony On 12.07.2024 13:54, Rony G. Flatscher wrote:Commit [r12849] implements the refactoring (changes got successfully tested using the ooRexx testing framework's TRACE related testgroups). ---rony On 12.07.2024 11:55, Rick McGuire wrote:On Fri, Jul 12, 2024 at 5:52 AM Rony G. Flatscher <rony.flatsc...@wu.ac.at> wrote: On 12.07.2024 01:16, Rick McGuire wrote:I still think the code refactoring I suggested is a good idea, which will make things a bit cleaner.+1However, I just realized that the original problem could have been fixed by just adding enum TracePrefix; where the class definitions at the top of Activity.cpp are located.As the first error occurs with compiling the ArrayClass.cpp Activity.hpp also got that definition added and it makes it compile. However, later when RexxActivation.cpp gets compiled the following error gets raised: [ 17%] Building CXX object CMakeFiles/rexx.dir/interpreter/execution/RexxActivation.cpp.obj RexxActivation.cpp D:\orx.work202312\main_trunk_20240312\trunk\interpreter\execution\RexxActivation.cpp(3674): error C2664: 'void Activity::traceOutput(RexxActivation *,RexxString *,TracePrefix,RexxObject *,RexxObject *)': cannot convert argument 3 from 'RexxActivation::TracePrefix' to 'TracePrefix' Ah, that's because they are different types because of the RexxActivation qualification. Rick ---ronyOn Thu, Jul 11, 2024 at 12:29 PM Rony G. Flatscher <rony.flatsc...@wu.ac.at> wrote: On 11.07.2024 18:26, Rick McGuire wrote:On Thu, Jul 11, 2024 at 12:20 PM Rony G. Flatscher <rony.flatsc...@wu.ac.at> wrote: On 11.07.2024 17:52, Rick McGuire wrote:I'm going to answer this out of line because I want to answer the last question first.Thank you!The problem with the compile error results in how RexxActivation is defined in Activity.hpp. It's declaration is just "class RexxActivation", which tells the compiler that RexxActivation is a class...and nothing else. This allows RexxActivation * to be used in prototypes since there is enough information available for that purpose. To get the full definition of RexxActivation, you would need to replace "class RexxActivation" with "#include "RexxActivation.hpp". However, I suspect doing that will result in some circular dependencies between the two header files.Indeed.Any you would probably need to define Activity as friend to the RexxActivation class to make that enum visible. Given the relationship between RexxActivation and Activity, and that both classes need the information, having the enum defined in RexxActivation feels like the wrong place to for that to be located. It feels like it should belong in Activity. However, those values are really only used for things created for RexxActivation. A better solution would be to refactor the code a bit. Activity::traceOutput() . The first two lines of TraceOutput deal with the creation of the TraceObject, which really should be something that the RexxActivation is responsible for. Move the stuff involved with that to RexxActivation and then only pass the constructed TraceObject to the traceOutput() method which is now just responsible for handling the output. Now there's no need for TracePrefix to be visible outside of RexxActivation.There are the following places in Activity that cause the use of traceOutput() directly and indirectly: * wholenumber_t Activity::error() /** Force error termination on an activity, returning the resulting REXX error code. * @return The resulting Rexx error code. */ * wholenumber_t Activity::error(ActivationBase *activation, DirectoryClass *errorInfo) /** Force error termination on an activity, returning the resulting REXX error code. * @param activation The activation raising the error. * @param errorInfo The directory containing the error condition information. * @return The Rexx error code. */ * wholenumber_t Activity::displayCondition(DirectoryClass *errorInfo) /** Display error information and traceback lines for a Syntax condition. * @param errorInfo The condition object with the error information * @return The major error code for the syntax error, if this is indeed a syntax conditon. */ * void Activity::display(DirectoryClass *exobj) ... /** Display information from an exception object. *@param exobj The exception object. */ * void Activity::displayDebug(DirectoryClass *exobj) /** Display information about an error in interactive debug. @param exobj The exception */+ Only the last two of these are relevant, since the others just call those. If the creation of the trace object is a method of RexxActivation(), that method is just called to get the information needed by traceOutput().ah, ok, thank you! ---rony
_______________________________________________ Oorexx-devel mailing list Oorexx-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/oorexx-devel
