Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP/feedbacks] Add magic constants #482

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

minirop
Copy link
Contributor

@minirop minirop commented Nov 11, 2017

Disclaimer: it's more to have feedbacks/tweaks (e.g. on the syntax) or ideas (e.g. of new constants) than actually get this PR merged "as-is".

Hello there,

Many languages have a way to print the current filename or the current line (like the C preprocessor with __FILE__ and __LINE__). So I told to myself "why not add them to wren?" and went with the same syntax.

Since the filename is not known from the VM, I chose to print the module name. So now you have __LINE__ and __MODULE__.

Here's some code with what is printed when you do wren test.wren:

mymod.wren:

var func = Fn.new {
	System.print("inside module: " + __MODULE__ + " on line " + __LINE__.toString)
}

test.wren:

var x = __LINE__
System.print(x.toString + " => " + __LINE__.toString) // print: "1 => 2"

System.print(__MODULE__) // print: "main"

import "mymod" for func

func.call() // print: "inside module: mymod on line 2"
func.call() // print: "inside module: mymod on line 2"
func.call() // print: "inside module: mymod on line 2"

minirop

__LINE__ for the current line
__MODULE__ for the name of the module
@minirop minirop changed the title Add magic constants [WIP/feedbacks] Add magic constants Nov 11, 2017
@acook
Copy link
Contributor

acook commented Nov 12, 2017

Wouldn't it be better to nest them under a single top level constant instead of adding several?

@minirop
Copy link
Contributor Author

minirop commented Nov 12, 2017

What kind of "single"? __MODULE_LINE__ or Magic.line + Magic.module?

I chose "several" because it's the easiest, it's just a "detect and replace" idiom like the keywords.

@KyleCharters
Copy link
Contributor

I believe it would be best to place it under the existing optional "Meta" class.

@ruby0x1
Copy link
Member

ruby0x1 commented Nov 12, 2017

Another note, in the context of scripting a line/file can be variable. For instance if a "script" is just a loose file, but is merged with several loose files into one wren module, this would report incorrect line numbers. This is a common issue for languages that are like this, see for instance #line directive in glsl.

I'd also note that it would make more sense to me in meta since the module/line is available for a callstack usually, it should be fairly easily available as a regular function. This too has the same concern about module/line.

@minirop
Copy link
Contributor Author

minirop commented Nov 22, 2017

I've tried to put module and line as getter inside a class called "Location" (because why not?).
Location.module: super easy to implement.
Location.line: impossible? (see below).

The line number of each bytecode is inside FnDebug.sourcelines so it's accessible, but the bytecode "index", which is frame->ip - fn->code.data - 1, is not valid for the current frame (the current frame->ip is only updated just before a new callframe is pushed, see STORE_FRAME() in runInterpreter()).

Also, the reason I chose variables is because it's a simple "replace" at compile time, while a function call has cost at runtime.

And what to do with that kind of code? Will it print 1 or 2? (Both can be considered valid, and everybody has a different opinion)

System.print(Location.
             line)

@IngwiePhoenix
Copy link

It would be nice having such a feature! But this kind of makes me wonder where the error-callback, which can be supplied to a VM config, is getting it's line from? Couldn't you use a similiar method to how that stack trace is being built to fill `LINE``?

I can see how this could become a problem with concatenated files, actually. However, it should at least be an optional thing people can include if they would like to. That is at least my opinion. Therefore I support the idea of adding this to Meta or making it an own little module that can be compiled in, if wanted. Or, at least enabled either during compilation or through VM configuration.

config.provideLineVariable = true/false;
config.provideModuleNameVariable = true/false;

@ruby0x1 ruby0x1 closed this Jun 12, 2020
@ruby0x1 ruby0x1 reopened this Jun 13, 2020
@ruby0x1 ruby0x1 changed the base branch from master to main June 13, 2020 21:14
@mhermier
Copy link
Contributor

Maybe this could also be augmented with a 'FUNCTION' variable (or what ever the name) to inform about the current function/method name.

@ChayimFriedman2
Copy link
Contributor

The one thing that I learned from these macros, and similar things in other languages, is that they useful for one thing: tracking the location of an error. For that, we have #868, which will display you the full stack trace.

If for some strange reason you still need that, you can parse the stack trace. This is the worst idea ever because the stack trace isn't guaranteed to be stable to parse but just easy for humans to read.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants