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

The policy on availability of ASTNode methods in macros #3274

Open
oprypin opened this issue Sep 7, 2016 · 8 comments
Open

The policy on availability of ASTNode methods in macros #3274

oprypin opened this issue Sep 7, 2016 · 8 comments

Comments

@oprypin
Copy link
Member

oprypin commented Sep 7, 2016

It seems that the set of methods available on type nodes in macros is inconsistent.

  • and and or can be split to their subexpressions, but with unary ! you can't get the expression it wraps.
  • .as is readable but .is_a? isn't.
  • There are random omissions, like block and splat arguments not being available on def.
  • And what's so special about case? Why is it available when many smaller and more frequently used nodes aren't?

I think it is important to define a policy on what is available and what isn't, and then proceed to fill the gaps (I can help with that).
I don't see a reason to have arbitrary limits in exploring the structure of expressions passed to macros. Sure, not everything is applicable, at least it's clear that not being able to modify the nodes is part of the current policy, but not much else seems to be well-defined.

So, what is the policy on the availability of these methods?

@asterite
Copy link
Member

asterite commented Sep 7, 2016

The policy is: all nodes should be able to be traversed. Currently they are not but only because we didn't have time to implement them all. I added the methods that I thought would be most convenient, or that I needed at that moment for some stuff, but all of them should be available.

Adding these methods and tests for them should be relatively easy.

@jhass
Copy link
Member

jhass commented Apr 18, 2020

How is the status now four years later? Anybody wants to reevaluate?

@HertzDevil
Copy link
Contributor

ClassDef and ModuleDef are still missing (#6679 only attempted the API docs), as are lib-related nodes (#5244).

@HertzDevil
Copy link
Contributor

HertzDevil commented Jun 11, 2021

At the minimum it should be possible to reconstruct every AST node from macro interpolation of that node's direct constituents. For example,

  • Reconstructing a TypeDeclaration is simple:
    macro type_decl(node)
      {{ node.var }} : {{ node.type }} {% unless node.value.is_a?(Nop) %} = {{ node.value }} {% end %}
    end
  • Reconstructing a Call is impossible; global calls (e.g. ::raise) are indistinguishable from local ones.
  • Reconstructing a Def is impossible; the abstract declarator and free variables are missing.

@HertzDevil
Copy link
Contributor

HertzDevil commented Jun 12, 2021

So I went through the AST node types with crystal tool hierarchy, and here is my full review:

  • Nodes that have no inner components and can be documented already:
    • Self
    • Underscore
  • Control expressions:
    • ControlExpression
      • Break, Next, Return < ControlExpression
    • ExceptionHandler
    • Rescue
    • Select
      • Crystal::Select::When represents the when branches of a select expression, but it does not inherit from Crystal::ASTNode for some reason.
    • Yield
  • Other expressions:
    • Asm
    • AsmOperand
    • MagicConstant (as default arguments of def nodes only, elsewhere they are expanded by the parser)
    • Metaclass
    • Primitive (e.g. Int32.methods.select(&.name.== "to_i64!").first.body)
    • TypeOf
  • Type definitions:
    • AnnotationDef
    • ClassDef
    • CStructOrUnionDef
    • EnumDef
    • LibDef
    • ModuleDef
  • Declarations:
    • Alias
    • Extend
    • External < Def (funs accessed via TypeNodes)
    • ExternalVar
    • FunDef (funs passed directly as macro arguments)
    • Include
    • TypeDef
  • Macro-related nodes:
    • MacroExpression
    • MacroFor
    • MacroIf
    • MacroLiteral
    • MacroVar
    • MacroVerbatim < UnaryExpression

The following subclasses of Crystal::ASTNode cannot possibly appear in the macro language:

  • Nodes eliminated during syntactic normalization:
    • OpAssign
    • Unless
    • Until
  • Fictitious nodes generated during semantic analysis:
    • AssignWithRestriction
    • FileNode
    • MetaTypeVar (this is what a MetaMacroVar wraps; MetaMacroVar in turn is what we call Crystal::Macros::MetaVar, distinct from Crystal::MetaVar)
    • TupleIndexer
    • TypeFilteredNode
    • Unreachable
    • YieldBlockBinder

@Blacksmoke16
Copy link
Member

Another thing worth bringing up is the mutability of certain kinds of nodes. This was touched on a bit within #7109 (comment) but I think it could use some more discussion given that was quite a few years ago.

I agree on the sentiment for not wanting to enable additional mutability, and am totally in favor of increasing the functionality of alternate approaches, e.g. annotations. But on the flip side I think their usage is no longer solely used as a means of a registry, which was the original primary concern.

Macro logic can get quite complex/involved but also can support very powerful features. In my experience it's not uncommon to work with local hash/namedtuple/array literals as temporary data stores within the scope of a (or multiple) {% ... %}. Some examples of this include:

In such contexts as these the primary reasons for preventing mutability are no longer relevant as there isn't a side affect of it messing with dependency order; not having them only makes the code even more complex due to needing to workaround the lack of some methods , such as #dig, #merge, #delete, etc, that are missing due to this past point of view, or just lack of a general policy on what methods should be available.

#8835 is a related topic that depending on the exact implementation could possibly render this issue moot.

@HertzDevil
Copy link
Contributor

My current opinion on mutability is: the macro language lacks macro-exclusive sequential and associative collections, or rather, doing so wouldn't bring any substantial benefits over simply reusing real AST nodes, so ArrayLiteral and HashLiteral would probably be fine, plus their tuple sublings.

Now even if #8835 were implemented, there would still be "primitive" methods that must be offered by the compiler, or whose performance would severely degrade without that. For example, unless we get rid of HashLiteral#clear, the mutating #delete(key) would have been equivalent to:

macro hash_delete(hash, key)
  {%
    copy = hash.map { |k, v| {k, v} }
    hash.clear
    value = nil
    copy.each do |(k, v)|
      if k == key
        value = v
      else
        hash[k] = v # quadratic complexity (see note below)
      end
    end
    hash  # => {1 => 2, 3 => 4}
    value # => 6
  %}
end

hash_delete({1 => 2, 3 => 4, 5 => 6}, 5)

But of course copy can be avoided altogether if a built-in method implements this instead, so I would allow HashLiteral#delete. Note that a linear scan somewhere is unavoidable as a HashLiteral's entries are stored in an Array, thus it might turn out a lot of mutable methods can be added in order to reduce their time complexities on HashLiteral.

@Blacksmoke16
Copy link
Member

so I would allow HashLiteral#delete

Related: #8849

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

No branches or pull requests

5 participants