Crystal: Replace ifdef with flag? macro method

[jhass]

I would like to see ifdef gone, as it feels like a construct not consistent with the language. Let me explain why I think it's inconsistent.

Crystal has two stages at which program code is run. Macro level code runs at the compile time, everything else runs at runtime. Following that it of course has two places where decisions are made. Macro code is clearly marked with the {%, {{, }} and %} tokens, they give a clear visual indicator that the contained code is not run at runtime, but at compile time. This looks as follows:

if condition
  code
else
  code
end

{% if condition %}
  code
{% else %}
  code
{% end %}

It's fairly obvious which code is run when. But more importantly it's also fairly obvious which code is included into the resulting executable as a result of that.

Now let's compare that to ifdef:

if condition
  code
else
  code
end

ifdef condition
   code
else
  code
end

Now that visual distinction is gone, worse else, the same keyword in both places, suddenly has differing meanings, depending on what came prior it.

Okay, now we could just reintroduce the visual distinction and make the macro tokens mandatory for ifdef:

{% ifdef condition %}
  code
{% else %}
  code
{% end %}

But now we have essentially two constructs that do the same thing, branching, in the macro level. Let's go a step further and allow retrieving compile time flags via a macro method:

{% if flag?(:darwin) || !flag?(:x86_64 %}
  workaround_32bit_darwin_bug
{% elsif flag?(:linux) %}
  linux_has_shortcut
{% else %}
  regular_way_for_everybody_else
{% end %}

Yes, this is substantially more noisy, but I think it's more consistent, leads to more obvious code and should only affect low level code for the most part.

[asterite]

I agree, but first we should support macros inside lib. The main issue is that inside a lib the program is parsed in a different way, so expanding a macro inside it must parse it in that way too (for example one can use union and type inside a lib but not outside it).

I think what you propose is the way to do it, even if it ends up being more verbose, but it does makes things more consistent, and removes one redundant concept from the language.

[asterite]

I also think that the formatter could easily upgrade the code, once we make the change :-)

[asterite]

One problem with this change is that this stops compiling:

{% if flag?(:darwin) %}
  x = 1
{% else %}
  x = 2
{% end %}
puts x # undefined method 'x'

The "beauty" of ifdef was that code inside it had to be valid, and was replaced before semantic analysis. Now the parser never gets to see those x.

The same applies for detecting initialized instance vars.

Not a big deal, workaround is:

x =
  {% if flag?(:darwin) %}
    1
  {% else %}
    2
  {% end %}

Maybe it's even better to force this, because it's easier to see if x was initialized in all branches.

[jhass]

Maybe we can introduce a macro ternary for simple cases like that

x = {{flag?(:darwin) ? 1 : 2 }}

[asterite]

@jhass That works already :-P

The problem is that if @x is an instance variable the type won't be inferred.

Not a big deal anyway.

[jhass]

Hah, I never noticed.

[asterite]

Another issue:

puts "start"

{% if true %}
  puts "before openssl 1"
  require "openssl"
  puts "after openssl 1"
{% end %}

puts "before openssl 2"
require "openssl"
puts "after openssl 2"

Assuming "openssl" is this:

# openssl.cr
puts "In openssl"

The output is this:

before openssl 1
after openssl 1
before openssl 2
in openssl
after openssl 2

The problem is that requires are resolved before semantic analysis and macro expansion, and so the first require that's expanded is the second one, not the first one (in order).

This breaks HTTP::Client here, when that line is replaced with if flag?.

The solution I'm thinking to apply is to expand these macros in the same place where require is expanded, as long as they are simple (for now just a flag? check and boolean combination of those). We can later see if other macros should be evaluated as well, but maybe some require some type declarations to exist, but that happens later.

This is another reason to keep ifdef in the language, but with these small new rules we should be able to preserve the old behaviour but with a unified syntax.

[jhass]

Can't we just expand toplevel macro code before require? I think that would lead to the most intuitive behavior. For the, I assume very rare, cases where you do need to require something before/at toplevel macro expansion, we could offer a {{require ...}}.

[asterite]

@jhass I think I'll just place the require logic where everything else is, so it "executes" in the correct order. I'm doing some tests and that seems to work fine