Enhancement request for preprocessors support

[ghost]

Here is issue that I've posted on lslint repo few days ago: https://github.com/Makopo/lslint/issues/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! :1st_place_medal:

[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! :+1:

[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?

[XenHat]

@Sei-Lisa How do you deal with comments? I started testing on a larger script and I'm experiencing issues 😳

Even 5ce0632 doesn't appear to fully help

[Sei-Lisa]

I have no idea how you got that, but I haven't checked your code. The preprocessor removes comments and inserts #line to sync:

$ echo "x(){}" > x.h

$ cat test.lsl
//
//
//
//
//
//
#include "x.h"
default{timer(){error1;
error2;
error3;}}

$ mcpp < test.lsl # deliberately input through stdin
#line 1 "<stdin>"
#line 7 "<stdin>"
#line 1 "x.h"
x(){}
#line 8 "<stdin>"
default{timer(){error1;
error2;
error3;}}

$ mcpp < test.lsl | lslint -i
ERROR:: (  6, 17): `error1' is undeclared.
ERROR:: (  7,  1): `error2' is undeclared.
ERROR:: (  8,  1): `error3' is undeclared.
 WARN:: (  4,  1): function `x' declared but never used.
TOTAL:: Errors: 3  Warnings: 1

Note the #line 8 "<stdin>" directive at line 5 of the output. That indicates that the line following it (the default{timer(){error1; line) is line 8 in the original file. Therefore, when lslint finds an error at line 6 of the mcpp output, you have to translate it to line 8 of the original file. When at line 7, you have to translate it to line 9, and when at line 8, to line 10. When at line 4, you have to output line 1 of x.h. I guess the linter plugin must have some way to open a different file when the error isn't in the current one (very common in C, for example).

So, to do the transformations, you look at the last line seen before the line of lslint output that you're considering. Let's take the first error, ERROR:: ( 6, 17). The line is 6, so you have to check which one is the last #line directive seen before line 6. It's the one at line 5 and it says that the next line should be 8. So you do errorline - lastlinedirectiveline - 1 + lastlinedirectiveparameter = 6-5-1+8 = 8. That's the line you have to replace the 6 with.

For the next error, the calculation is 7-5-1+8 = 9. For the next error, the calculation is 8-5-1+8 = 10. For the last warning, things change.

It's reported at line 4. The last #line directive before line 4 is at line 3, and says #line 1 "x.h". Therefore, you have to output line=4-3-1+1=1 and file="x.h".

Doing that, you don't have to deal with comments: they will sort themselves out.

[XenHat]

Hmmm It was my understanding that the mcpp line would always be higher than the original file, which may be causing this behaviour on some scripts and not all. I will revise the code tomorrow and try to find a way to make this smarter.

my very naive function is the following:

    def getLastOffset(T, inlined_line):
        """Yeah."""
        result = 0
        for rest in T:
            for line in rest.pline:
                if int(rest.pline) >= inlined_line:
                    # Woah, use last result
                    break
                result = rest.pline
        return result

The parameters that are given to it are a named tuple of your suggested format, and a number, which is the line number that came from lslint. (the first number in the pair)

# EDIT:
#getLastOffset(namedtuple("name",param,param,param),line_num)
getLastOffset(list(namedtuple(name,param,param,param),namedtuple(name,param,param,param),...),line_num)

[Sei-Lisa]

I don't understand the second loop, probably because I don't understand what the type of rest.pline is.

Edit: also, you're doing nothing with line.

Edit2: I don't understand that you're passing it a tuple either. I figured you'd be passing it a list with all the #line directive tuples: (location, parameter, filename) meaning that at line location there's a #line directive with parameters parameter and filename.

[XenHat]

Uh, you are correct, I am passing the list of tuples. I confused myself with my own code.

I'm not actually using line, no, simply iterating through them. it's kind of hacky? And probably incomplete. This is pretty complicated for me, I don't have much experience with nested loops and that kind of code, and I barely know python.

rest is to T what line is to rest.pline, it's just the current element of the loop, here, a single tuple

        for this_tuple in T:
            for line in this_tuple.pline:
                if int(this_tuple.pline) >= inlined_line:
                    # Woah, use last result
                    break
                result = this_tuple.pline
        return result

[XenHat]

I added some more debug for when I am more rested:

ORIGINAL_CODE:
0    |//
1    |//
2    |//
3    |//
4    |//
5    |//
6    |#include "x.h"
7    |default{timer(){error1;
8    |error2; 
9    |error3;}}
10   | 
MCPP Output:
0    |#line 1 "<stdin>"
1    |//
2    |//
3    |//
4    |//
5    |//
6    |//
7    |
8    |
9    |#line 7 "<stdin>"
10   |
11   |#line 1 "x.h"
12   |x(){}
13   |
14   |#line 8 "<stdin>"
15   |default{timer(){error1;
16   |error2;
17   |error3;}}
18   |
DEBUG:: LINTER_OUT output:
ERROR:: ( 16, 17): `error1' is undeclared.
ERROR:: ( 17,  1): `error2' is undeclared.
ERROR:: ( 18,  1): `error3' is undeclared.
 WARN:: ( 13,  1): function `x' declared but never used.
TOTAL:: Errors: 3  Warnings: 1

[Sei-Lisa]

I'm still scratching my head about the second loop.

After all this discussion, I've now made a quick and dirty implementation for the optimizer. I've added an assertion that the error can never be in the #line directive, because that makes no sense and it would be ambiguous. See Sei-Lisa/LSL-PyOptimizer@1071941

[Sei-Lisa]

One suggestion: why not work on a branch until you have it complete, so you don't affect master?

[XenHat]

I dislike branching in git, I tend to screw it up (this might be SourceTree's fault too). So I hadn't thought about it. I shall branch out.

[XenHat]

@Sei-Lisa Yeah, that second loop is probably not needed. This is likely a vestigial loop from a refactored example.

I'm more a "one feature a day" kind of person, I typically don't draw implementation over several days so my thought process is a bit hectic at the moment

[ghost]

@XenHat Today I reinstalled my OS and noticed that your plugin is not working anymore. Is it I broke something or you?

[XenHat]

@Winterwolf I removed my local package and installed the copy served by package control and the preprocessor-free behavior works as intended. Preprocessor is going to need some more work, I didn't notice some things broke pretty badly so I reverted it

[Sei-Lisa]

There's one problem I forgot about. LSL accepts multiline strings, but the preprocessor doesn't. I solved it in my optimizer and in Firestorm by doing a "pre-pre-processor" pass that converted multiline strings to single-line using \n.

[XenHat]

That's a good thing to be aware of. Even though I by principle never use them, I usually just += them if I have to carriage return

EDIT: Day off for me today, I will resume work on this tomorrow

[ghost]

@XenHat so, current version should work (without preprocessor support)? I don't understand what's wrong. I have your plugin, it is enabled, I have lslint in system path, but there is no errors:

...But lua linter works fine. Maybe you mean that current version is temporary disabled? My english is not so well, sorry for stupid question :<

[Sei-Lisa]

I have now taken a look at your code. The problem seems to be that you don't take into account orig_line at all (the line number parameter of the #line directive). Not sure how to embed it within your code structure. Perhaps making getLastOffset set result = rest.mcpp_in_ine - rest.orig_line instead.

Note also that the lines reported by lslint are 1-based, so you need to subtract 1 when passing the error line number to getLastOffset.

After all that, you will probably need to subtract 1 from the line, to compensate for the fact that #line refers to the line number of the line after itself.

[Sei-Lisa]

I've added my comments to your code here, here and here.

[XenHat]

EDIT: After reading your comments, this is improving

[XenHat]

@Winterwolf I have not tested the changes on Linux. Try enabling SublimeLinter's debug mode, and looking at the Sublime Text console ( CTRL+ ` key)

Also make sure you are using the latest lslint release from Makopo/lslint, not the LSL package

[XenHat]

The offset math/logic appears to be wrong still, but it's.. close?

Adding more meaningful debug is getting somewhere

String attempt:307
Offset: 298
Token - offset: 9
New Line: ERROR:: (9,  9): syntax error, unexpected '}'
SublimeLinter: lslint output:
ERROR:: (9,  9): syntax error, unexpected '}'
TOTAL:: Errors: 1  Warnings: 0 

[XenHat]

@Winterwolf Hopefully the latest commit helps (Try adding my repository to package control to automatically get the latest commit)

reloading plugin SublimeLinter-contrib-lslint.linter
SublimeLinter: lslint version query: c:\windows\lslint.exe -V 
SublimeLinter: lslint version: 1.0.6 
SublimeLinter: lslint: (>= 1.0.4) satisfied by 1.0.6 
SublimeLinter: lslint activated: c:\windows\lslint.exe 
SublimeLinter: lslint linter reloaded 

[Sei-Lisa]

Yes, making it integer chokes in the loop that I said would choke :)

I think you misunderstood something. There are two independent issues related to off-by-one problems.

1. The line number coming from lslint is 1-based. This implies that you need to subtract 1 from it when passing it to getLastOffset, because the lines in your program are 0-based. That's unrelated to the other offset. The line should look like this:

offset = getLastOffset(preproc_bank, n_int - 1)

2. Due to the fact that the numeric parameter of the #line directive refers to the line after the #line directive, instead of itself, you need to apply an additional offset when doing the #line calculations in order for the positions to match. I think that for things to look fine, that offset should be applied when calculating result within getLastOffset. The exact value is probably +1, but I'm not sure. Therefore, that line should look like this:

result = this_tuple.mcpp_in_line - this_tuple.orig_line + 1

[Sei-Lisa]

It seems you've actually already done that. Is there anything left that is not working? What happens when the error is in an include file?

[XenHat]

@Sei-Lisa

ORIGINAL_CODE:
0    |//
1    |//
2    |//
3    |//
4    |//
5    |//
6    |#include "x.h"
7    |default{timer(){error1;
8    |error2;
9    |error3;}}
MCPP Output:
0    |#line 1 "<stdin>"
1    |#line 7 "<stdin>"
2    |#line 1 "x.h"
3    |echo "x(){}"
4    |#line 8 "<stdin>"
5    |default{timer(){error1;
6    |error2;
7    |error3;}}
DEBUG:: LINTER_OUT output:
ERROR:: (  4,  6): syntax error, unexpected STRING_CONSTANT, expecting '('

TOTAL:: Errors: 1  Warnings: 0

number: 4
Offset: 2
Token + offset: 6
New Line: ERROR:: (  6,  6): syntax error, unexpected STRING_CONSTANT, expecting '('
SublimeLinter: lslint output:
ERROR:: (  6,  6): syntax error, unexpected STRING_CONSTANT, expecting '('
TOTAL:: Errors: 1  Warnings: 0