uncaught syntax error in classes with sub-functions in Octave 4.2.2

[Remi-Gau]

MISS_HIT Component affected

Please choose one from:

• Style checker
• Code metrics
• Documentation

Your MATLAB/Octave environment

• Octave
• 4.2.2

Your operating system and Python version

• linux ubuntu 18.04

Describe the bug

on this repo: https://github.com/cpp-lln-lab/spm_2_bids.git

on the master branch

• in MATLAB 2017a

>> init_spm_2_bids
Correct matlab/octave verions and added to the path!
>> a = Mapping()

a = 

  Mapping with properties:

          mapping: []
              cfg: [1×1 struct]
              stc: 'a'
          realign: 'r'
           unwarp: 'u'
            coreg: 'r'
         bias_cor: 'm'
             norm: 'w'
           smooth: 's'
    default_value: ''

• in Octave

>> init_spm_2_bids
Correct matlab/octave verions and added to the path!
>> a = Mapping()
parse error near line 371 of file /home/remi/github/spm_2_bids/src/Mapping.m

  syntax error

>>> function bf = prepare_for_printing(spec)
           ^

[Remi-Gau]

not sure if miss_hit should catch this, but opening an issue just in case some other people bump into this down the line.

will try to confirm this on a more recent Octave version and ideally try to provide a MWE.

[Remi-Gau]

confirming the failure in CI:

octave fails: https://github.com/cpp-lln-lab/spm_2_bids/runs/7081438354?check_suite_focus=true#step:6:65

• using moxunit unit github action that runs Octave 4.2.2

matlab passes: https://github.com/cpp-lln-lab/spm_2_bids/runs/7081438195?check_suite_focus=true

[Remi-Gau]

MWE and comparison between octave versions

TL;DR:

sub-functions in the same m file that contains a class seem to throw a syntax error with octave 4.2.2 but not 5.1.0

---

for a m-file containing

classdef Foo

    properties
      closest = 'home'
    end
    
    methods
      
      function obj = Foo()
      end
      
      function next(obj)
        
        bar = obj.closest;
        
        show_me_the_way_to_the_next_whiskey(bar)

      end

    end
    
end


% sub function
function show_me_the_way_to_the_next_whiskey(bar)
  fprintf('next whiskey bar is "%s" \n', bar)
end    

---

octave 4.2.2

>> a = Foo()
parse error near line 24 of file /home/remi/github/tmp/Foo.m

  syntax error

>>> function show_me_the_way_to_the_next_whiskey(bar)
           ^

>>

---

octave 5.1.0

>> a = Foo
a =

<object Foo>

>> a.next
next whiskey bar is "home"
>>

---

matlab 2017a

>> a = Foo()

a = 

  Foo with properties:

    closest: 'home'

>> a.next()
next whiskey bar is "home" 

[florianschanda]

So just to make sure I understand this: there is a grammatical construct that is allowed in MATLAB but not Octave, and the issue here is that MISS_HIT should also reject it, if run in Octave mode?

If so, that seems reasonable.

[Remi-Gau]

thought I had replied to this. sorry.

So the behavior would be to reject this by default for Octave only. But as this is only a problem for older Octave, it would be nice to be able to ignore this (for people who don't want to support backward compatibility with older Octave).

Another option would be to specify in the miss_hit config which version of matlab or octave the config applies to but I don't think you want to open that can of worms.

[Remi-Gau]

forgot to mention that I can provide m-files for the tests: making sure that they fail with octave 4 but not later versions.

I suspect that nested functions in functions and in scripts may also make octave 4 complain.

[florianschanda]

If you can post here short snippets from the files (and containing a comment with expectations) that would be super helpful

[Remi-Gau]

Added some files on a branch on my fork: I can open a PR if you want, or I can let you grab them from there.

https://github.com/Remi-Gau/miss_hit/tree/remi_octave_subfunction/tests/octave

[florianschanda]

OK, so this is more complex. Basically right now MATLAB means "latest MATLAB" and Octave means "latest Octave". Under this view, everything works.

I do want to add a more complex way to deal with language versions, see e.g. #240 that I haven't touched in a while; and the infrastructure work needed to do that has not been done yet.

I will re-classify the ticket, but please do not expect an immediate fix.

[Remi-Gau]

Awesome. Thanks for looking into this.
This is defo not urgent at all: take all the time you need. :-)

[florianschanda]

Was it just subfunctions in classdefs files that octave didn't support, or in general? I have been looking over the NEWS files for Octave and I cannot find any mention of this.

[florianschanda]

@Remi-Gau could you please test the master branch and see if this now works for your project? CHANGELOG.md should explain how to use the new feature.

[Remi-Gau]

Well it seems to affect certain (older) versions of Octave (that is still the default version on some Linux distro). So rejecting this for all Octave version might be too stringent. But I don't think you want to start to also handle different Octave versions. As a developper I may favor backwards compatibility and thus would prefer the stringent way (reject this construct when in Octave mode). But others may not feel the same so it could be useful to also be able to ignore this rule. What to do you think? Also do you want me to provide m files to run the tests on for this? I need to check if nested functions within functions are also affected by this in Octave.

…

Sent from my Galaxy

[florianschanda]

But I don't think you want to start to also handle different Octave versions.

The groundwork for this is done. I just need to know which Octave versions support which constructs.

Also do you want me to provide m files to run the tests on for this?

That will always help.