Make //+build and other similar tags not be a comment

[karl-zylinski]

The problem

Currently you can add tags at the top of files, before the package line. Such as:

//+build darwin, linux

This tag makes the file only build on the platforms listed.

However, the tag is within a comment. This has several problems:

• Some people who see //+build darwin, linux think its just an informational comment.
• Some people who see //+build darwin, linux think the actual tag should be +build darwin, linux but it has been disabled by commenting it out.
• Comments shouldn't instruct the compiler to do anything. It should be possible to run a script that removes all comments. The program should still compile and run like before.
• It has very strong "hack" vibe, and is considered a hack by Bill. But having a core language feature be a confusing hack is quite bad.

There are additional tags such as //+private and //+no-instrumentation that have the same issues.

My suggested solutuion

Drop the comment:

+private
+build darwin, linux
+no-instrumentation
package game

This has problems because the syntax is incompatible with the rest of the language.

Therefore I suggest that anything that goes before the package line has syntax rules of its own. Almost like a mini language.

This might sound bad, but in a sense we already have this:

//+build darwin, linux

This build-tag-in-a-comment is only valid before the package line, but it just doesn't do anything if it's anywhere else in the file. So the current solution is like a mini-language that is hidden in comments, but it doesn't affect the rest of the file because it's in a comment.

So my suggestion is to explicitly introduce separate syntax rules for things above the package line. It is of course not a perfect solution either. But everything here is a compromise, and I think it's a lot better than the current solution.

Migration

It's easy to migrate to the new style: search and replace all of core and base for the //+build tags and the other tags and replace them with variants without the // in front.

During a transition period both can be supported with a warning saying that you should remove the //.

Update

I did some experiments, they can be seen in this draft PR: #4203

[thetarnav]

If the //+ was a distinct token, then it could be highlighted not as a comment, but a tag, which would make it less confusing.
But it is not
Tags like // +private and // +private still work
And nothing stops you from writing //+my comment later in code
Which makes it tricky to add to editor highlighting grammars

[Androthi]

i propose a new distinct token such as /# for such options. this should make it easy for editors and it will prevent confusion by having similar syntax do other things.

[karl-zylinski]

I did some experiments: https://github.com/odin-lang/Odin/compare/master...karl-zylinski:Odin:file-tags-without-comments?expand=1

In that branch I have made it so that !+ can be used instead of //+

It's quite simple changes, it mostly adds a new token !+ and makes sure there can only be comments or such tokens before the package line. Then it uses the same code to process the tags. All seems to work fine!

Just having + like I wrote in the opening of this thread probably won't work because it was tricky with tokenization. Important part is that it no longer looks like a comment.

[karl-zylinski]

Actually I was able to get just +build linux, darwin etc to work as well. Just had to make tokenizer a bit smart about if it had seen the package line or not.

But doing just + is tricky for other reasons, it's hard to make syntax highlighting that works with it, since you must have smart highlighting that reasons about if you're past the package line or not. With !+ it's very simple to make that syntax highlight. So I haven't commited the + style to the branch above, it's still !+

[karl-zylinski]

I made a draft PR with my changes: #4203

[Feoramund]

Interesting thoughts. Shell scripting has this same situation with shebangs.

# is used universally for compile-time statements and expressions throughout the language. We have, for example, #assert(...), which itself may come at the global file scope with no issue. Why not use that as well for this? Such as: #build_on(windows).

[karl-zylinski]

Another concern is migration. If all we do is change the //+ to !+ or similar, then migration becomes trivial search and replace. If we start adding parenthesis etc then users might have a lot of by-hand-fixing to do.

[Feoramund]

Another concern is migration. If all we do is change the //+ to !+ or similar, then migration becomes trivial search and replace. If we start adding parenthesis etc then users might have a lot of by-hand-fixing to do.

:%s/\v^\/\/\s*\+(\S*)\s*(.*)$/#\1(\2)

This is a vim-compatible substitution regex that will take //+build [...] and turn it into #build([...]). I'm sure most editors have similar ways of replacement-by-pattern with capture groups. Replacing #no-instrumentation with #no_instrumentation would take a second pass.

I think to be safe, any form of //+ should raise a compiler error with a deprecation notice at the point this change goes into effect.

[karl-zylinski]

Thanks, however, the main migration problem is not for me but for users with lots of code. They might not be able to figure out the regex, they'll probably do it by hand.

But I guess it's more important to get stuff right than be afraid of annoying migrations, as long as there are good deprecation warnings / errors.

I already put in a deprecation-ish warning in the proof-of-concept PR: #4203

[Feoramund]

If it's a significant issue, we could just write and provide a program in Odin to automatically convert the old format over to the new one. Unlike vim or other editors, if you're writing code in Odin, you're almost guaranteed to have an Odin compiler. :)

It wouldn't even be that large of a program.

[karl-zylinski]

Bill prefers something like #+ instead of just # because these file tags are sort of their own thing, and just using # would as he says "overload the meaning" of the #. If you have any opinions about this, then please add them to the PR: #4203

[karl-zylinski]

This has been fixed since #4203 got merged!