Debug informations
Locked
Debug informations
 
|
Hi,
Right now the only way to get the temps, args names from methods and blocks is the use the source code and extract those informations from it. With the proposed patch I've added for each compiled method/block a debug information with the args and the names, the class could be extended to add line number inside it. Cheers, Gwen _______________________________________________ help-smalltalk mailing list [hidden email] https://lists.gnu.org/mailman/listinfo/help-smalltalk |
0001-Add-debug-information.patch (14K)
Re: Debug informations
|
|
Re: Debug informations
|
|
On 27/05/2013 11:02, Paolo Bonzini wrote:
> Il 27/05/2013 10:47, Gwenaël Casaccio ha scritto: >> Hi, >> >> Right now the only way to get the temps, args names from >> methods and blocks is the use the source code and extract >> those informations from it. >> >> With the proposed patch I've added for each compiled method/block >> a debug information with the args and the names, the class could be >> extended to add line number inside it. > You can use a single array with arguments and temporaries concatenated. > Otherwise the patch looks pretty good, thanks! > > Paolo >> Cheers, >> Gwen >> >> >> >> _______________________________________________ >> help-smalltalk mailing list >> [hidden email] >> https://lists.gnu.org/mailman/listinfo/help-smalltalk >> _______________________________________________ help-smalltalk mailing list [hidden email] https://lists.gnu.org/mailman/listinfo/help-smalltalk |
Re: Debug informations
|
|
Re: Debug informations
|
|
On 27/05/2013 17:15, Paolo Bonzini wrote:
> Il 27/05/2013 12:26, Gwenaël Casaccio ha scritto: >> On 27/05/2013 11:02, Paolo Bonzini wrote: >>> Il 27/05/2013 10:47, Gwenaël Casaccio ha scritto: >>>> Hi, >>>> >>>> Right now the only way to get the temps, args names from >>>> methods and blocks is the use the source code and extract >>>> those informations from it. >>>> >>>> With the proposed patch I've added for each compiled method/block >>>> a debug information with the args and the names, the class could be >>>> extended to add line number inside it. >>> You can use a single array with arguments and temporaries concatenated. >>> Otherwise the patch looks pretty good, thanks! >>> >>> Paolo >> Here is a new version with one array > Oops, sorry for not noticing this before: this must go in the > MethodInfo, or you break loading old images. CompiledCode (and > subclasses) have their format set in stone. > > Paolo MethodInfo. I've changed the grow_identity_dic I think that could create a leak (if the object is allocated in old space) Cheers, Gwen _______________________________________________ help-smalltalk mailing list [hidden email] https://lists.gnu.org/mailman/listinfo/help-smalltalk |
Re: Debug informations
|
|
On 07/06/2013 15:56, Gwenaël Casaccio wrote:
> On 27/05/2013 17:15, Paolo Bonzini wrote: >> Il 27/05/2013 12:26, Gwenaël Casaccio ha scritto: >>> On 27/05/2013 11:02, Paolo Bonzini wrote: >>>> Il 27/05/2013 10:47, Gwenaël Casaccio ha scritto: >>>>> Hi, >>>>> >>>>> Right now the only way to get the temps, args names from >>>>> methods and blocks is the use the source code and extract >>>>> those informations from it. >>>>> >>>>> With the proposed patch I've added for each compiled method/block >>>>> a debug information with the args and the names, the class could be >>>>> extended to add line number inside it. >>>> You can use a single array with arguments and temporaries >>>> concatenated. >>>> Otherwise the patch looks pretty good, thanks! >>>> >>>> Paolo >>> Here is a new version with one array >> Oops, sorry for not noticing this before: this must go in the >> MethodInfo, or you break loading old images. CompiledCode (and >> subclasses) have their format set in stone. >> >> Paolo > > Here is the new patch, it does the same but informations are added in > MethodInfo. > I've changed the grow_identity_dic I think that could create a leak > (if the object is > allocated in old space) > > Cheers, > Gwen > Gwen _______________________________________________ help-smalltalk mailing list [hidden email] https://lists.gnu.org/mailman/listinfo/help-smalltalk |
Re: Debug informations
|
|
Re: Debug informations
|
|
Re: Debug informations
|
|
Thanks for the review :-)
You can find the new (and hope last) version of the patch! On 23/06/2013 20:16, Holger Hans Peter Freyther wrote: > On Fri, Jun 07, 2013 at 05:15:15PM +0200, Gwenaël Casaccio wrote: > > Hi, > > so my comments from the 8th of May still apply but here is another > round on the same patch. > >> + arguments: anInteger [ >> + <category: 'accessing'> >> + >> + ^ variables copyFrom: 1 to: anInteger >> + ] > What is the thought behind not storing these in two different > collections from the beginning? > >> diff --git a/libgst/comp.c b/libgst/comp.c >> index 10330e1..b6ce106 100644 >> --- a/libgst/comp.c >> +++ b/libgst/comp.c >> @@ -534,6 +534,9 @@ _gst_execute_statements (OOP receiverOOP, memset (&s, 0, >> + _gst_compiler_state->debugInfo = _gst_identity_dictionary_new (_gst_identity_dictionary_class, 6); >> + INC_ADD_OOP (_gst_compiler_state->debugInfo); >> @@ -719,6 +726,9 @@ _gst_compile_method (tree_node method, >> _gst_push_new_scope (); >> selector = compute_selector (method->v_method.selectorExpr); >> >> + _gst_compiler_state->debugInfo = _gst_identity_dictionary_new (_gst_identity_dictionary_class, 6); >> + INC_ADD_OOP (_gst_compiler_state->debugInfo); > how often will both be created and one of them turns out to be garbage? (ie when it grows), also I thing it's nice to protect object to be GC'ed for a future usage (at least in the compiler). >> - if (_gst_declare_arguments (method->v_method.selectorExpr) == -1) >> + if ((argCount = _gst_declare_arguments (method->v_method.selectorExpr)) == -1) > I have commented this in the previous commit. This is something to get > wrong most of the time and I would prefer to move the assignment out of > the if statement. > >> - if (_gst_declare_temporaries (method->v_method.temporaries) == -1) >> + if ((tempCount = _gst_declare_temporaries (method->v_method.temporaries)) == -1) > same here. Fixed > >> + if (methodOOP != _gst_nil_oop) { >> + INC_ADD_OOP (methodOOP); >> + >> + object = new_instance_with (_gst_array_class, argCount + tempCount, &variablesOOP); >> + INC_ADD_OOP (variablesOOP); >> + >> + args = method->v_method.selectorExpr; >> + i = 0; >> + >> + if (args->nodeType == TREE_BINARY_EXPR) >> + { >> + object->data[0] = _gst_intern_string (args->v_expr.expression->v_list.name); >> + i = i + 1; > use i instead of 0 even if they need to match right now. (and i += 1) >> + >> + new_instance (_gst_debug_information_class, &debugInfo); >> + INC_ADD_OOP (debugInfo); >> + >> + inst_var_at_put (debugInfo, 1, variablesOOP); >> + _gst_identity_dictionary_at_put (_gst_compiler_state->debugInfo, methodOOP, debugInfo); > the naming of debugInfo for both a dictionary and an instance of the debug > information class is a bit unfortunate. Can you think of two different names? it's renamed as debugInfoDict >> + inst_var_at_put (inst_var_at (methodOOP, 3), 5, _gst_compiler_state->debugInfo); >> } >> >> if (methodOOP != _gst_nil_oop) >> { >> - INC_ADD_OOP (methodOOP); > I see that this moved inside into the setjmp block. I know the code too little > to know when EXIT_COMPILATION will be called. What is the guarantee that if > methodOOP != nil that we already INC_ADD_OOPed it? If there is an error the compiler will return nil thus all the object could be GC'ed that's not a problem. > > >> + object = new_instance_with (_gst_array_class, argCount + tempCount, &variablesOOP); >> + INC_ADD_OOP (variablesOOP); >> + >> + for (i = 0, args = blockExpr->v_block.arguments; args != NULL; args = args->v_list.next) { >> + object->data[i] = _gst_intern_string (args->v_list.name); >> + i = i + 1; > i += 1 > > >> OOP >> +_gst_identity_dictionary_new (OOP classOOP, int size) >> +{ >> + return identity_dictionary_new (classOOP, size); >> +} >> + >> +OOP >> identity_dictionary_new (OOP classOOP, int size) > we could just rename it? There are only two callers of it? Either do it now or later > or is there precedence of not doing these renames? >> diff --git a/libgst/files.c b/libgst/files.c >> index a7156f9..913ade1 100644 >> --- a/libgst/files.c >> +++ b/libgst/files.c >> @@ -290,6 +290,8 @@ static const char standard_files[] = { >> "PkgLoader.st\0" >> "DirPackage.st\0" >> "Autoload.st\0" >> + >> + "DebugInformation.st\0" >> }; > oh? that is not too late? I would assume it should be close to > CompiledMethod code? >> + >> +"Test debug informations are generated" >> +Object subclass: Foo [ >> + a_1: i_1 a_2: i_2 [ >> + | i j k | >> + >> + ^ [ :a :b :c | | d e f | ] >> + ] >> +] >> + >> +Eval [ >> + | mth | >> + mth := Foo>>#'a_1:a_2:'. >> + (mth arguments = #(#'i_1' #'i_2')) printNl. >> + (mth temporaries = #(#'i' #'j' #'k')) printNl. >> + ((mth blockAt: 1) arguments = #(#'a' #'b' #'c')) printNl. >> + ((mth blockAt: 1) temporaries = #(#'d' #'e' #'f')) printNl. >> + nil > *yeah* !!! > One more question. Paolo had a comment in regard to compat with older > images. I forgot the details, do you remember them? Did you already > include his comment? I only change the MethodInfo class. > holger Gwen _______________________________________________ help-smalltalk mailing list [hidden email] https://lists.gnu.org/mailman/listinfo/help-smalltalk |
Re: Debug informations
|
|
| Free forum by Nabble | Edit this page |