[SOLVED] Anything wrong with this code?

[Joubarbe]

EDIT: Problem found: https://www.purebasic.fr/english/viewtopic.php?t=86390

I'm trying to switch from ASM to C, but I now have crashes that never happened before (no error log). The following procedure does not run independently, but maybe you'll see something obviously wrong in my code:

Code: Select all

Procedure$ ReplaceReplacers(line$)
    Define regex
    
    Debug "RR1" ; DEBUG
    
    regex = CreateRegularExpression(#PB_Any, "\*\*(.*?)\*\*")
    ExamineRegularExpression(regex, line$)
    While NextRegularExpressionMatch(regex)
      Define match$ = RegularExpressionMatchString(regex)
      match$ = Trim(ReplaceString(match$, "**", ""))
      Define func_line$ = StringField(match$, 1, "(")
      Define args_line$ = StringField(StringField(match$, 2, "("), 1, ")")
      args_line$ = Trim(ReplaceString(args_line$, ", ", ","))
      args_line$ = ReplaceReplacers(args_line$)
      Define func.__RuntimeReplacer = GetRuntimeInteger("RH_Encounter::" + func_line$ + "()")
      If func = 0
        xyError::FatalError("Error at replacer " + match$ + ": '" + func_line$ + "' is not a valid function.")
      EndIf
      line$ = ReplaceString(line$, RegularExpressionMatchString(regex), func(args_line$), #PB_String_CaseSensitive, 1, 1)
      
      Debug match$ ; DEBUG
      
    Wend
    FreeRegularExpression(regex)
    
    Debug "RR2" ; DEBUG
    
    ProcedureReturn line$
  EndProcedure
  

And when it does not crash, there is ~2 seconds lag for no obvious reason. I can also see "RR2" printed twice in a row, which to me should be impossible without printing "RR1" before?

Debug log looks like this:
RR1
RR2 (lag/crash here)
match$
RR2

[benubi]

The code you posted can't run, it lacks used structures definitions, it uses Define inside Procedure code blocks (you should use Protected).

[Joubarbe]

Thanks for reading this.

it uses Define inside Procedure code blocks (you should use Protected).

Is it really a problem? I've been using Define in procedures and loops for years. I don't use any Global, so I don't use Protected.

[Joubarbe]

Problem found: https://www.purebasic.fr/english/viewtopic.php?t=86390

[benubi]

Well of course, that's bad coding practice/style.

Defines need to be done in the Main program body (like declares) and then when used in Procedures you use the Shared keyword.

Code: Select all

; Main body
Define a.s, b, c
Define.f fx,fy,fz

Procedure MyProc()
Shared fx,fy,fz
Shared a,b,c
; ...
EndProcedure

Every time you declare a variable with it's type you may also re-initialize it, depending how the compiler reacts; in very old times it was considered a more robust practice for strings, and to pre-allocate space for them before working with them like NewString.s=Space(1024) : NewString.s="" and so on, but this isn't necessary anymore.

In short you only need to declare the variable once with it's type in Global, Define, Thread or Protected blocks. Then you never re-set the type on code; this also allows you to change the type at one place without having to go through many lines later, in case you have to, and it may help with readability.

Code: Select all

; "Bad" style
Define a.s,b.s,c.s
a.s = b.s + c.s
Debug a.s
;
;
; Better
Define a.s, b.s, c.s
a = b + c
Debug a

Use "EnableExplicit" keyword to make sure all your variables are well defined and accessible at the right place. This will avoid you to have miss-reads, ambiguous types/multiple declarations. "Why is my float not working?" because it's not declared at the right place, it's not a float at in the procedure's scope and then it gets auto-declared as an integer (32/64 bits system default type). There's also nothing wrong with using Globals, especially when you do it right. When many variables are always used in groups, you may want to use a structure/pointer perhaps; big apps often do this with a "GlobalsStruct" that holds all configuration and important states.

Code: Select all

Structure MyAppGlobals
  a.s
  b.s
  c.s
EndStructure

Define Glob.MyAppGlobals

Glob\a = "ABC"
Glob\b = "Banana"
Glob\c = "Cesar"

Global *MyGlobals.MyAppGlobals = @Glob;


Debug *MyGlobals\a
Debug *MyGlobals\b
Debug *MyGlobals\c

Procedure UseManyThingsAtOnce()
  ; Use *MyGlobals  
EndProcedure  

This may better order & readability once your project grows and you have many "global" and "shared" variables.

[SMaag]

Define or Protected inside a Procedure

I checked this some weeks ago!
In the ASM-Backend it results in the same ASM code and creates the same Stack based varibale.

From my view Protected and Define inside a Procedure is identical.

[Joubarbe]

Out of topic, but Protected can detect duplicates, Define cannot (hence my previous topic: https://www.purebasic.fr/english/viewtopic.php?t=86357)

Code: Select all

Procedure Func()
  Define a
  Define a
EndProcedure

Works.

Code: Select all

Procedure Func()
  Protected a
  Protected a
EndProcedure

Doesn't work.

[benubi]

Define or Protected inside a Procedure

I checked this some weeks ago!
In the ASM-Backend it results in the same ASM code and creates the same Stack based varibale.

From my view Protected and Define inside a Procedure is identical.

That may be the case, but inside the compiler it may be flagged/listed differently in the symbol tables, even though the output is identical for specific micro-operations. There are two keywords for a reason and it's just better to keep to the convention. When the output seems OK there could still be something that is being messed up or ambiguous in the logic because of the "trick", and this may only appear once the project grows a lot. When I get Heisenbugs it always starts around 5000-10000 lines of code. While IDK if it's the cause I then never use simple one line operations on structures and nested structures/pointers, because that may cause problems

Code: Select all

; May cause problems when called via Interface\Procedure()
;
*MyStruct\Ptr2\Ptr3\Counter + 1
;
; to be replaced with
*MyStruct\Ptr2\Ptr3\Counter = *MyStruct\Ptr2\pTr3\Counter + 1

;or
Protected *Ptr3.xyz = *MyStruct\Ptr2\Ptr3
*Ptr3\Counter = *Ptr3\Counter + 1

I might be paranoid about that, and looking at the wrong place, but it helped a little in the past to remove IMA's. But here, I haven't checked the asm output (isn't my strength) I can only imagine that a pointer got incremented instead of the counter because of some confusion.

Also calling an interface function, from an interface that is nested in that way, could cause such problems, especially when you combine ALL those things - like I did - so sorry if it's not precise, but I believe the problem is just because of that - that it is ambiguous on some level.

Code: Select all

; Go on like this for 10,000 lines and you should get Heisenbugs eventually
Procedure method_X(*self, a,b,c)
*MyStruct\Ptr2\Ptr3\Counter + 1
*MyStruct\SuperInterface\Dosomething(12345); 
With *Self\ComplicatedStruct
 ; ... more of the same
EndWith
EndProcedure

So long story short, when I don't use pure primitives, I don't do "one-liner operations" on them, and when I access interfaces and nested/pointed structures, I always use a local variable to store and work with them; in most of the cases that's what the optimizer would also do, and it may shorten pointer paths especially in loops (because it's accessed via the local variable, and not going from root pointer down to the element).

Code: Select all

Protected Cool.SuperInterface = *MyStruct\SuperInterface

Cool\Dosomething(12345) ; "better safe than sorry"

[Joubarbe]

Hmm, I would like to have Fred's opinion on this one. And if there is some kind of convention, it should be documented (where PB is powerful is that it let you do whatever you want - with possible vulnerabilities).

Even though I know that some nested operations are known to cause problems but the extent of the problem, to me, remains unknown.

I have a 30k lines program with rather complex structures, and I don't seem to have a lot of random bugs. I always use Define, and do a lot of nesting, as well as "myVar + 1" operations.

"There are two keywords for a reason and it's just better to keep to the convention."
Yes, and it's to avoid messing with Global variables inside a procedure. Not to define any variable inside a procedure.