luke-gru / riml

Riml is a subset of VimL with some nice added features. It compiles to plain VimL.
MIT License
224 stars 6 forks source link

Riml compiler produces malformed code with duplicate riml_include #14

Closed dsawardekar closed 11 years ago

dsawardekar commented 11 years ago

This code is a little more involved. It's part of a plugin I'm working on. Consider a class TemplateContext with the code.

class TemplateContext
  def initialize(data)
    self.data = data
  end

  defm lookup(key)
    if has_key(self, key)
      return self[key]
    elseif has_key(self.data, key)
      return self.data[key]
    elseif key == 'context'
      return self.data
    elseif has_key(self.data, 'lookup')
      return self.data.lookup(key)
    else
      return ''
    end
  end
end

I am including this class in another file. Which includes another and so on. I've narrowed down the issue is to do with duplicate include like below.

riml_include 'template_context.riml'
riml_include 'template_context.riml'

With such a duplicate riml_include the compiler get's very confused. See the output below. Further this output viml does not run, giving a endfunction missing error.

function! s:SID()
  return matchstr(expand('<sfile>'), '<SNR>\zs\d\+\ze_SID$')
endfunction
" included: 'template_context.riml'
function! s:TemplateContextConstructor(data)
  let templateContextObj = {}
  let templateContextObj.data = a:data
  let templateContextObj.lookup = function('<SNR>' . s:SID() . '_s:TemplateContext_lookup')
  return templateContextObj
endfunction
function! <SID>s:TemplateContext_lookup(key) dict
  if has_key(self, a:key)
    return self[a:key]
  elseif has_key(self.data, a:key)
    return self.data[a:key]
  elseif a:key ==# 'context'
    return self.data
  elseif has_key(self.data, 'lookup')
    return self.data.lookup(a:key)
  else
    return ''
  endif
endfunction
" included: 'template_context.riml'
function! s:TemplateContextConstructor(data)
  let templateContextObj = {}
  let templateContextObj.data = a:data
  let templateContextObj.lookup = function('<SNR>' . s:SID() . '_s:TemplateContext_lookup')
  return templateContextObj
endfunction
function! <SID>s:TemplateContext_lookup(key) dict
  if has_key(self, a:key)
    return self[a:key]
  elseif has_key(self.data, a:key)
    return self.data[a:key]
  elseif a:key ==# 'context'
    return self.data
  elseif has_key(self.data, 'lookup')
    return self.data.lookup(a:key)
  else
    return ''
  endif
endfunction
function! s:TemplateContextConstructor(data)
  let templateContextObj = {}
  let templateContextObj.data = a:data
  let templateContextObj.lookup = function('<SNR>' . s:SID() . '_s:TemplateContext_lookup')
  return templateContextObj
endfunction
function! <SID>s:TemplateContext_lookup(key) dict
  if has_key(self, a:key)
    return self[a:key]
  elseif has_key(self.data, a:key)
    return self.data[a:key]
  elseif a:key ==# 'context'
    return self.data
  elseif has_key(self.data, 'lookup')
    return self.data.lookup(a:key)
  else
    return ''
  endif
endfunction
function! s:TemplateContextConstructor(data)
  let templateContextObj = {}
  let templateContextObj.data = a:data
  let templateContextObj.lookup = function('<SNR>' . s:SID() . '_s:TemplateContext_lookup')
  return templateContextObj
endfunction
function! <SID>s:TemplateContext_lookup(key) dict
  if has_key(self, a:key)
    return self[a:key]
  elseif has_key(self.data, a:key)
    return self.data[a:key]
  elseif a:key ==# 'context'
    return self.data
  elseif has_key(self.data, 'lookup')
    return self.data.lookup(a:key)
  else
    return ''
  endif
endfunction
function! s:TemplateContextConstructor(data)
  let templateContextObj = {}
  let templateContextObj.data = a:data
  let templateContextObj.lookup = function('<SNR>' . s:SID() . '_s:TemplateContext_lookup')
  return templateContextObj
  let templateContextObj = {}
  let templateContextObj.datatemplateContextObj.data = a:data
  let templateContextObj.lookuptemplateContextObj.lookup = function('<SNR>' . s:SID() . '_s:TemplateContext_lookup''<SNR>' . s:SID()'<SNR>' . s:SID() . '_s:TemplateContext_lookup')
  return templateContextObj
endfunction
function! <SID>s:TemplateContext_lookup(key) dict
  if has_key(self, a:key)
    return self[a:key]
  elseif has_key(self.data, a:key)
    return self.data[a:key]
  elseif a:key ==# 'context'
    return self.data
  elseif has_key(self.data, 'lookup')
    return self.data.lookup(a:key)
  else
    return ''
  endif
  if has_key(self, a:key)
    return self[a:key]
  elseif has_key(self.data, a:key)
    return self.data[a:key]
  elseif a:key ==# 'context'
    return self.data
  elseif has_key(self.data, 'lookup')
    return self.data.lookup(a:key)
  else
    return ''
    return self[a:key]self[a:key]
  elseif has_key(self.data, a:key)
    return self.data[a:key]
  elseif a:key ==# 'context'
    return self.data
  elseif has_key(self.data, 'lookup')
    return self.data.lookup(a:key)
  elseif has_key(self.dataself.data, a:key)
    return self.data[a:key]
    return self.data[a:key]self.dataself.data[a:key]
  elseif a:key ==# 'context'
    a:key ==# 'context'
    return self.data
    return self.dataself.data
  elseif has_key(self.dataself.data, 'lookup')
    return self.data.lookup(a:key)
    return self.data.lookup(a:key)
    self.data.lookupself.data.lookup(a:key)
    self.data.lookup(a:key)
  else
    return ''
    return ''
  endif
endfunction

The compiler appears to be stuck inside an internal loop. The output is concatenated in places, blocks are misaligned, etc.

I think the easiest way to deal with this is to not output anything for the second riml_include.

Or if you are going to include the second time then memoize/cache the result of a riml_include. So any secondary includes use the cached output instead.

luke-gru commented 11 years ago

Thanks for the ticket. I'll take a look on Saturday, but this should be a quick fix.

dsawardekar commented 11 years ago

@luke-gru Can you clarify the approach you will be taking to fix this bug?

Would it be possible to implement some form of a DAG to let the compiler figure out the right order of the includes?

Currently the compiler checks for presence of classnames, so the order of the include is crucial, else you get ClassNotFound errors. Not needing to worry about include order is handy.

I guess what I'm asking is for require to work more like import. :smile:

luke-gru commented 11 years ago

Well, I already have the easy solution to this implemented in a branch, and it fixes this issue. It just keeps track of the filenames that were already included and only includes new filenames. The dependency resolver would be a bit more work, but it shouldn't be too bad. I'll experiment a bit with it and report back.

I think riml also needs a way to either import classes or ignore undefined classes altogether, because if I'm making a library mylib that has as a dependency some other library written in riml, I should be able to use those classes in my library without having to compile or link with the other library during each compile of 'mylib'. What are your thoughts on this?

dsawardekar commented 11 years ago

The simpler solution sounds good for now.

Regarding dependency between libraries, A solution I've seen work to an extent is separate definition files. Eg:- Typescript has a Type Definition File that can be used to define the interfaces of a 3rd party library, so that it is able to compile against that library correctly. If my project used say jQuery, without a definition file a method call like $.foo would be fine, whereas with the definition file it would throw a compile time error, like foo not found on $.

Having said that this isn't an ideal solution, definition files tend to go out of sync as new revisions of the target library are released.

luke-gru commented 11 years ago

Hey @dsawardekar, yeah definition files are probably useful for statically typed languages that need to be compiled separately from the dependent libraries, but for Riml I think a good enough solution for now is to maybe have something like

riml_import g:ThirdPartyClass to register the class with Riml, and then you can use it elsewhere without any errors. Also, for the lazy, I'll offer a command-line option to turn off compile-time class checking, I think.

I'll open an issue for this, and gather thoughts there.

As for this ticket, I'm going to close it, as the fix for the original issue is in master. I'll add another ticket for the DAG class registration implementation.