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

Preprocessor conditional logic #31

Open
jeff-hykin opened this issue Mar 16, 2019 · 5 comments
Open

Preprocessor conditional logic #31

jeff-hykin opened this issue Mar 16, 2019 · 5 comments
Labels
🐛 Bug Something isn't working Hard Major An issue that summarizes a large general problem

Comments

@jeff-hykin
Copy link
Owner

jeff-hykin commented Mar 16, 2019

atom/language-c#269
atom/language-c#290
atom/language-c#280

Almost all of these involve code that is highlighted differently because of preprocessor statements.
Most of them are impossible because the TextMate parser needs to find the end of something, but the preprocessor makes it possible for there to be two ends, and TextMate can't handle conditional endings.

However, one improvement that probably can be made, is that the macros should not include any of the TextMate range patterns. This is because if they do, they can screw up the entire rest of the file.

@jeff-hykin jeff-hykin added 🐛 Bug Something isn't working Nearly Impossible :( Things that likely need more than just Textmate matching to fix ☠️ provably impossible Things that are actually impossible with Textmate labels Mar 16, 2019
@jeff-hykin
Copy link
Owner Author

jeff-hykin commented Apr 11, 2019

In terms of cleaning up the syntax, I think we should agree on what behavior should considered "cant fix wont fix" and what should be considered a bug.

I think it's straightfoward that, all ranges inside of macros should end when the macro ends (that can be considered part of this issue).

The part I'm unsure about is the case for opening/closing across if statements.

#ifndef blah
    void func(
#else 
    int func(
#end
);

Stragety 1: My initial response is to say that all ranges be closed whenever there is a preprocessor conditional. This would leave the final ); in the code unmatched/untagged.

Strategy 2: However, there might be a better option where the first blocks if-to-else-if, if-to-else, or else-if-to-else always close all ranges, but the final block else-to-endif always fails to close the range so that the range can overflow and find its actual ending.

There's also the potential for nested macro if statements that could affect which ranges are being closed (and that could be a mess or impossible).

@matter123
Copy link
Collaborator

matter123 commented Apr 11, 2019

Strategy 2 sounds like it better to how the preprocessor is used.

@jeff-hykin jeff-hykin added the Major An issue that summarizes a large general problem label Apr 12, 2019
@jeff-hykin jeff-hykin removed ☠️ provably impossible Things that are actually impossible with Textmate Nearly Impossible :( Things that likely need more than just Textmate matching to fix labels Jun 25, 2019
@matter123 matter123 added the Hard label Oct 21, 2019
@jeff-hykin
Copy link
Owner Author

jeff-hykin commented Jul 10, 2020

Retouching on this, this should be the most ideal behavior given textmate.

(b1 is bailout1)

int main() {
    #if              
        {
            #if
                {
            #else   // b1-range start
                {
            #endif  // b1-range end
                }
           
            #if 
            #endif
    #else           // b1-range start
        {
            #if
                {
            #else      // b2-range start
                {
            #endif     // b2-range end
                }
           
            #if 
            #endif
    #endif         // b1-range end
        }
}
grammar[ :preprocessorConditional ] = PatternRange.new(
    start: /#else|#elif/,
    # have it capture something upon ending
    end: lookBehindFor(/#els|#eli|#endi/).then(/e|f/),
    includes: [
          # itself, allowing for nesting
          :preprocessorConditional,  

          # bailed out ending by capturing /#els|#eli|#endi/.lookAheadFor(/e|f/)
          :bailedOut, 
    ]
)

100% Lookarounds won't work since they're zero-length. So, instead, have the bailedOut pattern work with the preprocessor to 'use-up' the ending pattern so that the outside bailout/preprocessor can't match it.

This should only need 1 bailed out grammar, so it should less than double the size of the repo.

@matter123
Copy link
Collaborator

As I discussed in #485, while this is probably the best we can do, the context at the start of the #else range is actually the same context as write before the #else and not the context right before the #if. In the context of #485. The example will still be broken unless the context inside the #else range allows for function definitions.

@jeff-hykin
Copy link
Owner Author

Yeah it's just extending the previous comment I made here on the nesting issue.

As for the problem with the context continuation: Yeah I don't think there's a way to stop the overflow from the #if part without some crazy generated code. Even if it meets size requirements, the code complexity would be extreme. We could maybe handle it well in the non-nested situation and then switch back to the #else for nested.

Either way I'm probably not implementing it any time soon haha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Bug Something isn't working Hard Major An issue that summarizes a large general problem
Projects
None yet
Development

No branches or pull requests

2 participants