Implement __SCOPE__

[Rangi42]

__SCOPE__ expands to the currently scoped label. It is empty if inside the main scope.

Fixes #775

[Rangi42]

checkpatch is complaining "WARNING: Move const after static - use 'static const char'" regarding static char const *Callback__SCOPE__(void), but I copied it from static char const *Callback__FILE__(void) which doesn't get this warning. :/

[ISSOtm]

As I said, I don't like this functionality because I think it caters to too specific a purpose, which natively handling in RGBASM brings little benefit. (Paging @AntonioND to have a third opinion)

Plus, erroring out when no scope is present means there's no way to handle that case, it should rather not be defined then, as the documentation suggests ("undefined").

[Rangi42]

When no scope is present, __SCOPE__ is undefined: DEF(__SCOPE__) returns false, and evaluating __SCOPE__ acts like evaluating any other undefined symbol (i.e. the error message depends on whether it's evaluated as a constant, or interpolated into a string). (Yes, it still has an entry in the symbol hashtable, but the lookup function explicitly checks for it. Compare with _NARG, which counts as defined even outside a macro, but causes an error when evaluated.)

(Edit: The "__SCOPE__ does not make sense in main scope" error message is just a fail-safe. It could be removed, like the labelScope length check.)

[ISSOtm]

When no scope is present, __SCOPE__ is undefined: DEF(__SCOPE__) returns false, and evaluating __SCOPE__ acts like evaluating any other undefined symbol (i.e. the error message depends on whether it's evaluated as a constant, or interpolated into a string).

Then what is this check for? (I didn't look further after seeing it, assuming it was the only one; hence my inaccurate reply)

(Yes, it still has an entry in the symbol hashtable, but the lookup function explicitly checks for it. Compare with _NARG, which counts as defined even outside a macro, but causes an error when evaluated.)

I don't like that it's hardcoded, tbh. _NARG is imo different because there's no reason to try using it outside of a macro; however, a macro may be written in a way to act differently whether it's in a scope or not.

[Rangi42]

The checks if (!labelScope) and if (n > sizeof(buf) - 1) are both checking for things that should never be the case, because they are avoided before calling Callback__SCOPE__. I'll make them assertions.

I agree that hardcoding the !strcmp(name, "__SCOPE__") check is not ideal, but removing and re-adding __SCOPE__ to the hashtable would have the memory issues that you brought up.

[ISSOtm]

No memory issues at all, since the buffer returned is global. Removing and re-inserting the symbol manually into the hash table would be more problematic (repeated allocations and de-allocations), and it would still be hardcoding, which I really really want to avoid. Call it PTSD perhaps.

[Rangi42]

The repeated allocations and de-allocations are what I meant by memory issues.

To technically avoid hardcoding a strcmp, all Symbols could gain a new property: either bool isDefinedInMainScope, or bool (*defCallback)(void). (sym_FindExactSymbol would then do return (!sym->defCallback || sym->defCallback()) ? sym : NULL;.) The former feels like a cheat that's still hardcoding. The latter is a more flexible mechanism, but would still be created just for this one symbol. (Maybe it would have other uses?)

[ISSOtm]

The implications of the latter on the rest of the codebase's assumptions are not something I would want to deal with, certainly not in 0.5.0.

[Rangi42]

Actually, another solution would be to act like PCSymbol: store __SCOPE__Symbol as a static global, and then do return (sym != __SCOPE__Symbol || labelScope) ? sym : NULL. (Like how sym_IsPC just "hardcodes" return sym == PCSymbol.)

[ISSOtm]

The difference, again imo, is that PC is a necessary evil, but that this functionality isn't worth all that trouble.

[Rangi42]

Okay. For now I've updated this to use a sym == __SCOPE__Symbol as the simplest of those ways to arbitrarily check for a particular symbol.

Any idea why the static const char warnings are inconsistent?

[ISSOtm]

Not the slightest.

[aaaaaa123456789]

As I said, I don't like this functionality because I think it caters to too specific a purpose, which natively handling in RGBASM brings little benefit.

Not really. This can be used by all sorts of metaprograming, ranging from good error messages to structure-defining macros.

See: __FUNC__ in C (I think added in C99).

[ISSOtm]

Structure-defining macros exist of all sorts, and never needed this (nor has it been bothering); and what error messages are bad?

[aaaaaa123456789]

what error messages are bad?

foo: MACRO
  ___foo \1, __SCOPE__
ENDM

; I'm sure you could do this with string
; interpolation, but I don't remember how to
___foo: MACRO
  if (\1) > 40
    warn "Foo value for \2 over threshold!"
  endc
  db \1
ENDM

[ISSOtm]

There's never been a need for this, and it's incompatible with a variety of uses, which is why I'm deeming it as "not enough".

[Rangi42]

That would be:

foo: MACRO
  if (\1) > 40
    warn "Foo value for {__SCOPE__} over threshold!"
  endc
  db \1
ENDM

TrainerA:
    db "A-chan"
    foo 38
    bar 200

TrainerB:
    db "B-kun"
    foo 42 ; oops
    bar 180

[ISSOtm]

This would only be beneficial if there were more than a screenful of lines between the last global label (assuming none of the macros in the middle define one) and the erroring macro. Frankly, it's more fragile than anything.

[daid]

In that case the warn will already report line with foo 42 as the origin of the warning, no real need for SCOPE

[aaaaaa123456789]

You're assuming that people know how to read error messages. A week in pret's #pokecrystal will quickly prove you wrong.

[ISSOtm]

Nothing can be done for people who don't want to read error messages.

[Rangi42]

I'll go ahead and close this, but am still interested what @AntonioND thinks of the feature and of this implementation approach.

[AntonioND]

So I'm a bit confused, what's the purpose of this apart from having custom error messages?

[Rangi42]

@ISSOtm Is the update without a hardcoded check in sym_FindExactSymbol acceptable?

[ISSOtm]

I agree that many of RGBASM's features are not strictly necessary, but are convenient and improve the code quality significantly.

The key part is "significantly": balancing the benefits gained with the increased complexity (both in maintaining and, possibly, performance); and it's largely subjective.
I believe that a non-negligible amount of RGBASM features don't match that balance, sometimes because of legacy, sometimes because of decisions that I came to regret later. But I don't think rolling back would be worth it, so I'm keeping them.
(Yes, I think RGBASM is slightly bloated. We all make mistakes, so imo it's acceptable.)

You're welcome to disagree, but I don't think this change is useful enough to warrant native syntax/support.

Further, it severely limits future expansions to scoping (which would be very useful), and I'm afraid about its interactions with other features. (Think #342, for example.)

tl;dr: I don't like this feature as-is, so I'm going to put it on the backburner for now, and will look at it after more important features have been dealt with. (See what's attached to the 0.5.1 milestone, basically.)

[Rangi42]

As long as .local labels exist, "the currently scoped label" should also exist; I don't expect the future plans for more complex scoping (like auto-purged symbols local to a file/macro/rept, or local names that shadow outer ones for macros' or user-defined functions' parameters) to affect that. I agree __SCOPE__ would have limited applications, just not that it would limit future changes.

[ISSOtm]

I'm considering more complex scoping like ca65's Outer.inner.itsDarkInHere.

[Rangi42]

(For Outer.inner.itsDarkInHere I'd expect __SCOPE__ to be Outer.inner, but regardless, that's beyond this particular implementation. I'll see if something like this is useful+practical depending on future scope-related changes.)