Enhancement request for preprocessors support

[ghost]

Here is issue that I've posted on lslint repo few days ago: Makopo/lslint#47.
My problem is that your plugin ignores files icluded by preprocessor and produces errors about undeclared variables.

[XenHat]

Very interesting. I'll look into it. Thanks for using my plugin!

P.S. I was aware of the issue but I did not know it was even fixable. @Sei-Lisa is very knowledgeable 🥇

[ghost]

@XenHat Thank you very much! Your plugin is awesome! 🥇

[Sei-Lisa]

In case it helps, I only see two possibilities around this.

One is to invoke the preprocessor on a file and open the output in a dedicated window if that is possible with this editor, then run lslint on that (the -i option of lslint will skip over the #line directives that mcpp generates; GNU cpp generates # lines instead with identical function). The user will have to match manually every line in the preprocessed text to a line in the input file, to find the offending spot. That's a short-term solution.

The other is to make lslint's -i option understand #line directives, to sync the output lines with the original input files. That's a pending issue for my optimizer, which supports calling a preprocessor internally, but suffers the same problem of a mismatch in the reported lines in case of error. The optimizer comes before lslint in my priorities list, and this is a long-standing issue, which should be a hint that this solution will take quite a while.

That second solution will require adding the file name to the output of lslint, which isn't currently there, so that when #line switches to a different file (for example an include), the error position can still be tracked. I suppose that such a change will imply major changes for this project as well.

I may be missing some other solution. Perhaps there is some other language supported by this editor that already supports preprocessor, and it uses tricks that can be borrowed for this application. Ironically, I don't think C or C++ are among them; most compilers integrate the preprocessor pass internally.

[XenHat]

Right now, I am attempting to preprocess the file in-place by calling mcpp using the linter class, so far I got... About halfway, I'd say:

[Sei-Lisa]

Wow! 👍

[XenHat]

Well, so far I suffer from the same issue with #line and I cannot seem to be able to regex it out.. So I believe that the best thing to do for the time being is to do as you suggested and make -i ignore line, until it can be implemented properly.

Having an issue with default parameters not working, will report back

[Sei-Lisa]

what should that regex do? catch #line (number) (file)?

[XenHat]

Yeah, I was hoping to be able to "ignore" that specific error, but I was being tired, I think.
I could always remove the #line lines from the mcpp output but I'm not sure what would break.. probably nothing?

[XenHat]

Progress:

DEBUG:: sanitized:
integer xlOwnerAttentive()
{
    list landmasters = [llGetOwner(), "1d7465fc-27e4-4386-a965-ad92b670832c"];
    integer lm_index = llGetListLength(landmasters) - 1;
    integer mute = 1;
    while (mute && lm_index > -1)
    {
        mute = llGetAgentInfo(llList2Key(landmasters, lm_index));
        mute = mute && ((mute & AGENT_BUSY) || !(mute & AGENT_AWAY));
        lm_index--;
    }
    return mute;
}
default
{
    touch_start(integer total_number)dd
    {
        if(xlOwnerAttentive())
        {
            llSay(0, "Yes");
        }
        else
        {
            llSay(0, "No");
        }
    }
}

[Sei-Lisa]

Well, if you remove #line from the preproc output you're doing the work that -i does.

The trick here is to remember the #line directives and the lines where they appear. When an error is emitted in a certain line, you have to look up which is the last #line directive that was last seen before that line number, and replace the line in the report with the content of the #line directive, plus the difference between where the #line directive is located and where the error is reported.

For example, if this is the entire preprocessor output (with line numbers added):

1. integer
2. #line 4 "<stdin>"
3. string;

then line 3 has a syntax error which will be reported as ERROR:: (3, 1): Syntax error or similar.

You have to replace 3 with 4+(3-2), where 4 is the line that the #line directive specifies, 3 is the line where the error occurs, and 2 is the line where #line directive actually is. So the actual line would be 5.

EDIT: The above explanation is off by one. The line number indicated in #line actually applies to the next line. So you have to subtract one from the result, and report line 4 in the example instead of 5.

[XenHat]

Yeah, I can't figure a way to make this happen with the facilities SublimeLinter provides. That might be better to leave this to lslint indeed.

[Sei-Lisa]

Hm, but the snapshot above suggests that you have access to the preprocessor output and to lslint's output. Can't you then filter lslint's output to make lines match as required, before giving them back to the editor? That sounds like basic piping to me.

[XenHat]

That is correct, and definitely possible, but beyond my mental gymnastic abilities right now.

[Sei-Lisa]

Would it help if I give you a Python function that takes two strings, one the preprocessor output and the other the lslint output, and gives you another string with the filtered lslint output? Perhaps in a different format, with file path information?

[XenHat]

I'm not sure... I'm pretty tired, I probably need to rest on this. String manipulation with offsets really isn't my part of the things I'm good at, and I don't think I've done something close enough to something like this to know what to do.

[XenHat]

I pushed what I have done so far, be aware that it breaks the linter due to mcpp forcefully inserting at least one #line. I'll resume work tomorrow

[Sei-Lisa]

This is my take on the process:

• Call mcpp on the input text and capture the output.
• Process it with Python, parsing #line and remembering which line number in the file every #line directive is. I suggest to '\n'.split() the output into a list for this purpose, and loop over it with a numeric index. I suggest using a list of tuples (preprocessor line, target line, file name) but up to you.
• Call lslint -i on the preprocessor output, and capture the output in turn. This should take care of #line directives by itself, and it's the main purpose this switch is designed for. (I'm not sure why you're having problems with lslint choking on #line directives at this step. Just passing the -i switch will take care of them)
• Go over every line of the lslint output, replacing each line number with the new line, according to the method outlined earlier, and adding file name info as well.
• Feed the result to the editor.

[XenHat]

More progress

[XenHat]

@Sei-Lisa is there a way to make mcpp not include the <stdin> library? my version doesn't have the -nostdinc switch :(

EDIT3: After adding additional lines to my script, I see how lslint handles #line (I was getting index out of range errors).
Using the standalone lslint appears to have fixed my issue with it not handling -i

[Sei-Lisa]

"stdin" is not a library. It has nothing to do with libraries (you may be confusing it with "stdio.h", a standard C library). "stdin" stands for "standard input" and it is the default pipe programs read from. When mcpp reads a file from standard input as opposed to a file you specify, the file parameter in #line is "<stdin>".

[XenHat]

Ah, right. I pretty much have to use standard input right now, because I can't figure how to "get" the file name without creating a new temporary one... Not something I feel like doing.

I'm currently hitting a wall with getting the values out of a namedtuple, python is being silly... This might take a few more hours. :/
(Python seems to flatten my tuple into a long array, so i'm having issues getting the right value I want out of it without doing index modulus magic, which... well, math...)

Python being so flexible was the problem 😳 I was using += when I should have been using .append()

[Sei-Lisa]

Yeah, so just replacing <stdin> with the input filename should work.

Edit: been there :)

[XenHat]

well, getting the file name is difficult, as mentioned. So whatever (currently hidden/unknown) functionality that relies on that will be broken until I find how..

[buildersbrewery]

@XenHat http://www.sublimetext.com/docs/3/api_reference.html#sublime.View and there file_name()

[XenHat]

Having absolutely no luck getting the API to return me file name, giving up for now. view just doesn't appear to work in SublimeLinter... Best I can get is a pointer, then undefined references or missing required positional arguments... on example code that should work.

Plus, it works, so who caresthat can be left for later when something requires it and I manage to make this work. The API isn't friendly to UI-less plugins

[XenHat]

@winterwolf Could you please test the latest version (just update via package control) and see if this works for you?

[Sei-Lisa]

Yay! 👍

What can be done about an error in a different file? Can it notify the main linter module, in order to automatically open the right file where the error is and report it? I really hope so.

If not, then I can only think of two alternatives. The ugliest is to just filter out the errors that correspond to a different file. The least bad is to try to guess where the #include was, and report the error in it. I have ideas for how to implement the latter, but they will complicate the code.

Edit: Sorry that I was so stubborn. 😊

[XenHat]

RE:edit: No worries. I was too. 😊

About complexity: I'm not afraid of complexity with logic, but I'm pretty trash at math.

About the file thing... well, it would be easier if I could get the name of the file opened in the view, but no matter how I look at it, it seems impossible to get code that works fine in the editor's built-in console to behave properly in my linter plugin. I'm not sure why, either.
Calls to file_name() from any of the two supported classes (windowcommand and textcommand just fail. I can't get anything "deeper" than getView then it just breaks. So I guess that's a nope.

However, we can probably treat "none" as "this file" and then do something with the lines coming from another file such as putting the marker at the position of the #include?

EDIT: if you would prefer to chat about this outside the bug tracker, you can hit me up in-world: https://my.secondlife.com/xenhat.liamano (or secondlife:///app/agent/f1a73716-4ad2-4548-9f0e-634c7a98fe86/im)

[Sei-Lisa]

Well, if anything, to not spam Winterwolf with technical details. But since I didn't find you online...

The question was not if you can access the filename. It's assumed that the main file is open in the editor, therefore you don't need to find its name in principle; you can assume that "<stdin>" means the main file and anything else means the file specified in the #line directive.

The question was if you can force the linter plugin to open a filename you give, in order to report errors in it. I presume that a C linter, for example, must be able to do exactly this, in order to report problems in the headers. Or any other language that supports includes, for the matter.

If it doesn't support it, maybe you can file a feature request asking to implement that.

But in case it's not supported, here's how I imagine the automatic detection of #include.

The basis of idea is that the preprocessor will insert a #line directive with a different filename at the exact point where it found the #include. Therefore, when an error is found in a different filename, your aim is to find the last #line directive which had "<stdin>" as a file before that line (call that L1), keeping track of where the first #line directive which changed the filename was (call that L2).

Then you replace the error line with L2, and do the same conversion math you're doing now using L1, and that's the line number that will appear in the final output.

Example:

top.lsl

   1 | string s; // just to add something
   2 | #include "level1.lsl"
   3 | default{state_entry(){llOwnerSay(s);}}

level1.lsl

   1 | #include "level2.lsl"

level2.lsl

   1 | integer x;

The goal is to report the unused identifier at line 2 of our file, which is the line with the include. The mcpp output is:

   1 | #line 1 "<stdin>"
   2 | string s;
   3 | #line 2 "<stdin>"
   4 | #line 1 "level1.lsl"
   5 | #line 1 "level2.lsl"
   6 | integer x;
   7 | #line 2 "level1.lsl"
   8 | #line 3 "<stdin>"
   9 | default{state_entry(){}}

The error lines reported by lslint are 2 and 6. For line 2, the last #line had "<stdin>" as file, therefore you do your usual thing.

For line 6, the last file was not "<stdin>", so you have to make a replacement. The last #line directive that had "<stdin>" is line 3, and the first #line directive after that one is line 4. Therefore, you replace the error line number as if it actually happened in line 4, because that's where the #include was (which is why the filename has changed). Then you do the usual thing, but using 4 as the error line number.

If all goes well, the output should be:

 WARN:: (  1,  8): variable `s' declared but never used.
 WARN:: (  2,  8): variable `t' declared but never used.
TOTAL:: Errors: 0  Warnings: 2

[buildersbrewery]

Reading through linter.py at f01e33f thinking out loud:

• SublimeLinter-contrib-lslint/linter.py

  Lines 109 to 112 in f01e33f

  	def remove_line_directives(my_string):
  	"""Not a good solution."""
  	# return re.sub("^#line.*\n","",my_string)
  	return re.sub(r'(?m)^\#line.*\n?', '', my_string)

  currently not used

• SublimeLinter-contrib-lslint/linter.py

  Lines 150 to 152 in f01e33f

  	version_args = '-V'
  	version_re = r'(?P<version>\d+\.\d+\.\d+)'
  	version_requirement = '>= 1.0.4'

  You can require v1.0.6 and use --version now...

• SublimeLinter-contrib-lslint/linter.py

  Lines 189 to 191 in f01e33f

  	lslint_binary_path = fullpath(lslint_binary_name)
  	if lslint_binary_path is None:
  	lslint_binary_path = find_linter(lslint_binary_name)

  The preference here changed comparing to a few days ago. Maybe eventually it would make sense to compare the version for each and decide based upon that?

• Some weird chars at EOL when viewing the file via Github online, perhaps some copied Windows line-endings?

  • SublimeLinter-contrib-lslint/linter.py

    Line 213 in f01e33f

    OutputTuple = namedtuple('OutputTuple', 'mcpp_in_line\

  • SublimeLinter-contrib-lslint/linter.py

    Line 215 in f01e33f

    file')

• SublimeLinter-contrib-lslint/linter.py

  Line 55 in f01e33f

  st_pf = sublime.platform().strip()

  no need to .strip()

• Afaik using '...%s' % ... or '...%s...%s' % (..., ...) will format the output to string as well '...{}...{}'.format(..., ...). Haven't found a place where usages are compared in a way that would give a reason for either of these. Currently Sublime Text uses Python 3.3, would be nice to be able to use formatted string literals from Python 3.6.
• Not sure why you're casting to an int?

  • SublimeLinter-contrib-lslint/linter.py

    Lines 253 to 255 in f01e33f

    	offset = getLastOffset(preproc_bank, number)
    	# print("Offset: {0}".format(offset))
    	tokminoff = str(number - int(offset))

  • SublimeLinter-contrib-lslint/linter.py

    Lines 115 to 128 in f01e33f

    	def getLastOffset(tuples_list, inlined_line):
    	"""Yeah."""
    	result = 0 # Fallback if there is no directives.
    	for this_tuple in tuples_list:
    	if int(this_tuple.mcpp_in_line) >= inlined_line - 1:
    	# We reached a #line directive further than the one
    	# we are looking for; Do not store this instance and
    	# return the previous one instead. This assumes a few things.
    	break
    	# The offset ends up being negative in some cases, even if right.
    	# Let's forcefully remove the negative part of the number.
    	result = this_tuple.mcpp_in_line - this_tuple.orig_line + 2
    	# This will return 0 if there is no #line found
    	return result

[XenHat]

1-3: Fixed
4: git is being a git
5: I like to prepare for the worst
6: read the docs, % is deprecated
7: I had to.

[Sei-Lisa]

Here's one (lazy) idea for solving the appending of info:

                    ...
                    # print("Offset: {0}".format(offset))
                    # tokminoff = str(number - int(offset))
                    tokminoff = str(number - int(offset))
                    # moved from below
                    # print("Token - offset: {0}".format(tokminoff))
                    new_line = re.sub(str(number), tokminoff, iter_line)
                    if result[1] != '"<stdin>"':
                        index = getLastStdin(preproc_bank, number)
                        # assert index != -1
                        new_number = preproc_bank[index + 1].mcpp_in_line + 1
                        offset = getLastOffset(preproc_bank, new_number)[0]
                        tokminoff = str(new_number - int(offset))
                        # the next line is best moved up to avoid calling p.match() twice
                        token_match = p.match(token)
                        new_line = '{0}:: ({1:>3},  1): in file {2}: {3}'.format(token_match.group(1), tokminoff, result[1], new_line)
                    # print("New Line: {0}".format(new_line))
                    fixed_output_lines.append(new_line)
                    continue

[ghost]

Sorry if I'm distracting, there is errors on linux again:

SublimeLinter: ERROR: could not launch '/usr/bin/mcpp -W0' 
SublimeLinter: reason: [Errno 2] No such file or directory: '/usr/bin/mcpp -W0' 
SublimeLinter: PATH: /usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/home/winterwolf/Dropbox/bin:/snap/bin:/usr/lib/jvm/java-8-oracle/bin:/usr/lib/jvm/java-8-oracle/db/bin:/usr/lib/jvm/java-8-oracle/jre/bin:/snap/bin:/usr/lib/jvm/java-8-oracle/bin:/usr/lib/jvm/java-8-oracle/db/bin:/usr/lib/jvm/java-8-oracle/jre/bin 
SublimeLinter: ERROR: could not launch ['/home/winterwolf/Dropbox/bin/lslint', '-m', '-i', <property object at 0x7f61fcf2a8e8>] 
SublimeLinter: reason: Can't convert 'property' object to str implicitly 
SublimeLinter: PATH: /usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/home/winterwolf/Dropbox/bin:/snap/bin:/usr/lib/jvm/java-8-oracle/bin:/usr/lib/jvm/java-8-oracle/db/bin:/usr/lib/jvm/java-8-oracle/jre/bin:/snap/bin:/usr/lib/jvm/java-8-oracle/bin:/usr/lib/jvm/java-8-oracle/db/bin:/usr/lib/jvm/java-8-oracle/jre/bin 
reloading settings Packages/User/lsl.sublime-settings
reloading settings Packages/User/ossl.sublime-settings
Package Control: Skipping automatic upgrade, last run at 2017-10-20 09:10:39, next run at 2017-10-20 10:10:39 or after

/usr/bin/mcpp exists! It is correct path, but not working :/

[XenHat]

Looks like Linux treats the executable path correctly and my hack doesn't work there. My OS broke and I had to reinstall it, so I lost the Linux VM.. this might take a few days :(

[XenHat]

Or not ;)

[Sei-Lisa]

So, if everything goes right, the output of the top.lsl example should look like this:

 WARN:: (  1,  8): variable `s' declared but never used.
WARN:: (  2,  1): in file "level2.lsl": WARN:: (  1,  8): variable `t' declared but never used.
TOTAL:: Errors: 0  Warnings: 2

(The lack of a space at the beginning of the second line is because the regex doesn't capture leading space, but that's only cosmetic.)

This would create two warnings, one at line 1 and the other at line 2 of the main file (top.lsl). The first warning would look just as usual. The second warning would point to column 1 of the #include line that included the file, even if indirectly, because the warning is inside it. The text of the warning would be:

in file "level2.lsl": WARN:: (  1,  8): variable `t' declared but never used.

which allows the user to go to level2.lsl and check line 1 column 8 manually.

Lacking any support for file arguments in SublimeLinter, I think this is the best that can be done and we can declare this issue as fixed.

[XenHat]

@Sei-Lisa I'm only getting one warning.. :(

SublimeLinter: lslint output:
WARN:: (  1,  1): in file "level2.lsl":  WARN:: (  1,  9): variable `x' declared but never used.
TOTAL:: Errors: 0  Warnings: 1 

EDIT: Figured something

[Sei-Lisa]

My bad, apologies. I changed my top.lsl like this, and forgot to update the message:

   1 | string s; // just to add something
   2 | #include "level1.lsl"
   3 | default{state_entry(){}}

Before that change, there was nothing wrong with line 1, so no warning was emitted for it.

[XenHat]

Nah, it was something else ^^ my top already looks like this, cleaning up and commiting

[XenHat]

Forgot to tag 9802e10

[XenHat]

Implemented the best we could given SublimeLinter's limitations.

[ghost]

@XenHat Thank you very much for your work! But I have a noob problem - don't know how to add parameters to lslint (I want to enable lazylists). Tried to edit linter settings like this:

"lslint": {
    "@disable": false,
    "args": ["-z"],
     "excludes": []
 }

it did not work.

[XenHat]

@winterwolf see #7 :)