Repeated scopes and incorrect macro scope

[alexr00]

Sample file
testh.txt

Checkout the { on line 564. It is incorrectly scoped as a macro, and has duplicate scopes.

Scopes:

punctuation.section.block.begin.bracket.curly.namespace.cpp
meta.head.namespace.cpp
meta.block.namespace.cpp
meta.head.function.definition.cpp
meta.function.definition.cpp
meta.preprocessor.macro.cpp
meta.head.function.definition.cpp
meta.function.definition.cpp
meta.preprocessor.macro.cpp
meta.head.function.definition.cpp
meta.function.definition.cpp
meta.preprocessor.macro.cpp
source.cpp

[alexr00]

This also exists in the stable branch. I don't want to cause you to have to scramble to get a fix. I'll roll back this time, and we can use the stable branch for the next release.

[matter123]

#if DOCTEST_CLANG
#ifdef __has_warning
#define DOCTEST_CLANG_HAS_WARNING(x) __has_warning(x)
#endif // __has_warning
#ifdef __has_feature
#define DOCTEST_CLANG_HAS_FEATURE(x) __has_feature(x)
#endif // __has_feature
#define DOCTEST_PRAGMA_TO_STR(x) _Pragma(#x)
#define DOCTEST_CLANG_SUPPRESS_WARNING_PUSH _Pragma("clang diagnostic push")
#define DOCTEST_MSVC_SUPPRESS_WARNING_PUSH
#define DOCTEST_GCC_SUPPRESS_WARNING_PUSH
#define DOCTEST_CLANG_SUPPRESS_WARNING(w) DOCTEST_PRAGMA_TO_STR(clang diagnostic ignored w)
#define DOCTEST_MSVC_SUPPRESS_WARNING(w)
#define DOCTEST_GCC_SUPPRESS_WARNING(w)
#define DOCTEST_CLANG_SUPPRESS_WARNING_POP _Pragma("clang diagnostic pop")
#define DOCTEST_MSVC_SUPPRESS_WARNING_POP
#define DOCTEST_GCC_SUPPRESS_WARNING_POP
#define DOCTEST_CLANG_SUPPRESS_WARNING_WITH_PUSH(w)                                                \
    DOCTEST_CLANG_SUPPRESS_WARNING_PUSH DOCTEST_CLANG_SUPPRESS_WARNING(w)
#define DOCTEST_MSVC_SUPPRESS_WARNING_WITH_PUSH(w)
#define DOCTEST_GCC_SUPPRESS_WARNING_WITH_PUSH(w)
#elif DOCTEST_GCC
#define DOCTEST_PRAGMA_TO_STR(x) _Pragma(#x)
#define DOCTEST_CLANG_SUPPRESS_WARNING_PUSH
#define DOCTEST_MSVC_SUPPRESS_WARNING_PUSH
#if DOCTEST_GCC >= DOCTEST_COMPILER(4, 7, 0)
#define DOCTEST_GCC_SUPPRESS_WARNING_PUSH _Pragma("GCC diagnostic push")
#else // GCC 4.7+
#define DOCTEST_GCC_SUPPRESS_WARNING_PUSH
#endif // GCC 4.7+
#define DOCTEST_CLANG_SUPPRESS_WARNING(w)
#define DOCTEST_MSVC_SUPPRESS_WARNING(w)
#define DOCTEST_GCC_SUPPRESS_WARNING(w) DOCTEST_PRAGMA_TO_STR(GCC diagnostic ignored w)
#define DOCTEST_CLANG_SUPPRESS_WARNING_POP
#define DOCTEST_MSVC_SUPPRESS_WARNING_POP
#if DOCTEST_GCC >= DOCTEST_COMPILER(4, 7, 0)
#define DOCTEST_GCC_SUPPRESS_WARNING_POP _Pragma("GCC diagnostic pop")
#else // GCC 4.7+
#define DOCTEST_GCC_SUPPRESS_WARNING_POP
#endif // GCC 4.7+
#define DOCTEST_CLANG_SUPPRESS_WARNING_WITH_PUSH(w)
#define DOCTEST_MSVC_SUPPRESS_WARNING_WITH_PUSH(w)
#define DOCTEST_GCC_SUPPRESS_WARNING_WITH_PUSH(w)                                                  \
    DOCTEST_GCC_SUPPRESS_WARNING_PUSH DOCTEST_GCC_SUPPRESS_WARNING(w)
#elif DOCTEST_MSVC
#define DOCTEST_PRAGMA_TO_STR(x)
#define DOCTEST_CLANG_SUPPRESS_WARNING_PUSH
#define DOCTEST_MSVC_SUPPRESS_WARNING_PUSH __pragma(warning(push))
#define DOCTEST_GCC_SUPPRESS_WARNING_PUSH
#define DOCTEST_CLANG_SUPPRESS_WARNING(w)
#define DOCTEST_MSVC_SUPPRESS_WARNING(w) __pragma(warning(disable : w))
#define DOCTEST_GCC_SUPPRESS_WARNING(w)
#define DOCTEST_CLANG_SUPPRESS_WARNING_POP
#define DOCTEST_MSVC_SUPPRESS_WARNING_POP __pragma(warning(pop))
#define DOCTEST_GCC_SUPPRESS_WARNING_POP
#define DOCTEST_CLANG_SUPPRESS_WARNING_WITH_PUSH(w)
#define DOCTEST_MSVC_SUPPRESS_WARNING_WITH_PUSH(w)                                                 \
    DOCTEST_MSVC_SUPPRESS_WARNING_PUSH DOCTEST_MSVC_SUPPRESS_WARNING(w)
#define DOCTEST_GCC_SUPPRESS_WARNING_WITH_PUSH(w)
#else
#define DOCTEST_PRAGMA_TO_STR(x)
#define DOCTEST_CLANG_SUPPRESS_WARNING_PUSH
#define DOCTEST_MSVC_SUPPRESS_WARNING_PUSH 
#define DOCTEST_GCC_SUPPRESS_WARNING_PUSH
#define DOCTEST_CLANG_SUPPRESS_WARNING(w)
#define DOCTEST_MSVC_SUPPRESS_WARNING(w) 
#define DOCTEST_GCC_SUPPRESS_WARNING(w)
#define DOCTEST_CLANG_SUPPRESS_WARNING_POP
#define DOCTEST_MSVC_SUPPRESS_WARNING_POP 
#define DOCTEST_GCC_SUPPRESS_WARNING_POP
#define DOCTEST_CLANG_SUPPRESS_WARNING_WITH_PUSH(w)
#define DOCTEST_MSVC_SUPPRESS_WARNING_WITH_PUSH(w)
#define DOCTEST_GCC_SUPPRESS_WARNING_WITH_PUSH(w)
#endif // different compilers - warning suppression macros

This section does not close its ranges.
Scope of line above this block:

source.cpp

Scope of line below this block:

meta.head.function.definition.cpp
meta.function.definition.cpp
meta.preprocessor.macro.cpp
meta.head.function.definition.cpp
meta.function.definition.cpp
meta.preprocessor.macro.cpp
meta.head.function.definition.cpp
meta.function.definition.cpp
meta.preprocessor.macro.cpp
source.cpp

[jeff-hykin]

created cpp/fix/#350 and added an indented version of your example @matter123 to the issues.

Problem 1

Ah I see one of several problems happening here.

#if THING1
    #if THING2
        // stuff
    #endif // BOTH if statements are closed here
// this is a straggling endif
#endif

This means the while pattern might not be viable, which would destroy a lot of the theoretical near-perfect macro behavior :/

Problem 2

    #define DOCTEST_CLANG_SUPPRESS_WARNING_WITH_PUSH(w)                                                \
        DOCTEST_CLANG_SUPPRESS_WARNING_PUSH DOCTEST_CLANG_SUPPRESS_WARNING(w)
// function definition doesn't close
// it thinks  DOCTEST_CLANG_SUPPRESS_WARNING_PUSH DOCTEST_CLANG_SUPPRESS_WARNING(w) is the start of a function definition

This problem will likely be present since function definition was added (many updates ago). It would also get fixed by the bailout pattern.

@alexr00 The only guaranteed full fix for this will probably be the bailout pattern, which is going to be a complex fix, so a rollback would've likely been needed anyways.

Summary of changes to be made (this will get edited/updated)

• Fix for nested if statements (requires bailout)
• Fix for function definition overflow inside of macros
• meta.preprocessor.macro matches the whitespace in front of the #define
• consider finding a common tag instead of entity.name.other.preprocessor.macro.predefined for pre-defined and entity.name.function.preprocessor for normal macros
• Macro calls (with ()'s) are not correctly highlighted, see DOCTEST_COMPILER(4, 7, 0)

[matter123]

If non-preprocessor stuff closes on a lookahead for #endif but preprocessor stuff consumes the #endif wouldn't that prevent double closing?

[jeff-hykin]

If non-preprocessor stuff closes on a lookahead for #endif but preprocessor stuff consumes the #endif wouldn't that prevent double closing?

Yeah, the bailout should work. We'll have to switch from while back to end pattern for the #if statement to properly close though

[matter123]

I think part of the issue is that the behavior of while is underspecified. It looks like vscode-textmate tests while patterns going up the stack rather than down the stack.

This results in the outer pattern closing itself and any nested patterns at the first #endif. This effectively means that while patterns cannot recurse.

[matter123]

I think I have a solution to this, hopefully also provides correct near-perfect syntax highlighting.

The only time that you can be sure that there is not another alternate highlighting is with #endif, therefore, the conditional ranges do not close on #endif,

So #if, #ifdef, #ifndef, #else, and #elseif all open a new range and a lookahead for #else, or #endif close those ranges. This should result in all except the last alternate for preprocessor conditionals to be closed. Since preprocessor conditional contexts do not add any scopes to the stack, it should be transparent to any themes

Caveats:

• This still requires the fix for nested whiles
• This doesn't fix #endif incorrectly closes comments. #359 and in theory, makes it worse
  • #endif incorrectly closes comments. #359 only struck when inside a conditional range, but with this proposal almost all code is inside one (from the first preprocessor conditional)

[alexr00]

It sounds like there is still discussion on what needs to be done. VS Code is in endgame this week so I'll change the version we use to be the version we last shipped with.

[jeff-hykin]

Sorry things have been going slower on my end, school is picking back up for me.

@alexr00 pushed a solid fix for problem #2 just now, and I basically reverted the code for problem #1. Its okay if this change doesn't make it into VS Code though, this is a complex fix. The downside to this solution is that it adds an entire duplicate grammar. There's no performance implications as far as I can tell, and the user shouldn't ever notice the difference (other than the lack of bugs), the only reason I mention it is because it means the ~19,000 line grammar has effectively become ~40,000 lines. There are still discussions to be had, but its very likely this is the only foolproof fix due to the limitations of TextMate. LaTeX is going to need its own copy of the C++ grammar for effectively the same fundamental reason.

@matter123, I used the npm textmate-bailout for the macros, and I switched from a while back to an end_pattern to allow for nested if/else even though its going to undo our recent progress with conditionals. I think we can use a bailout system to simulate a while pattern and have it handle nesting correctly. To do it though I think the bailout will need to be a non-lookahead though so its going to be a little bit of work to implement.

[matter123]

pushed a solid fix for problem #2 just now, and I basically reverted the code for problem #1. Its okay if this change doesn't make it into VS Code though, this is a complex fix. The downside to this solution is that it adds an entire duplicate grammar.

In particular, https://github.com/microsoft/vscode/blob/master/extensions/cpp/package.json needs to be modified to contribute the grammar, source.cpp.embedded.macro. See https://github.com/jeff-hykin/cpp-textmate-grammar/blob/master/source/languages/cpp/package.json#L51 for that.

[jeff-hykin]

Good call @matter123 I forgot that needs to be a manual change on the VS Code end.