(cpp) grammar confused by complex Template << >> patterns

[sylveon]

This code results in everything after L':' to be highlighted as if it was a string: (Editor: This is now resolved, the second issue still standing).

template<typename T>
class PropertyChangedBase {
public:
    winrt::event_token PropertyChanged(const winrt::Windows::UI::Xaml::Data::PropertyChangedEventHandler &value)
    {
        return m_propertyChanged.add(value);
    }

    void PropertyChanged(const winrt::event_token &token)
    {
        m_propertyChanged.remove(token);
    }

protected:
    template<typename U>
    void compare_assign(U &value, const U &new_value, std::wstring_view name)
    {
        if (value != new_value)
        {
            value = new_value;
            name.remove_prefix(name.find_last_of(L':') + 1);
            m_propertyChanged(*static_cast<T *>(this), winrt::Windows::UI::Xaml::Data::PropertyChangedEventArgs(name));
        }
    }

private:
    winrt::event<winrt::Windows::UI::Xaml::Data::PropertyChangedEventHandler> m_propertyChanged;
};

On this one, it just completely stops highlighting after the second is_streamable

namespace impl {
    template<typename T, typename = void>
    struct is_streamable : std::false_type { };

    template<typename T>
    struct is_streamable<T, std::void_t<decltype(std::declval<std::wostream &>() << std::declval<T>())>> : std::true_type { };

    template<typename T>
    inline constexpr bool is_streamable_v = is_streamable<T>::value;
}

// Disable overload for already valid operands.
template<class T, class = std::enable_if_t<!impl::is_streamable_v<const T &> && std::is_convertible_v<const T &, std::wstring_view>>>
std::wostream &operator <<(std::wostream &stream, const T &thing)
{
    return stream << static_cast<std::wstring_view>(thing);
}

Both pieces above is completely correct code which compiles, assuming the right headers are included

[joshgoebel]

The actual issue: #1412

[sylveon]

This is indeed the first issue, but the second piece of code does not feature any wchar_t character literals.

[joshgoebel]

If i could read that code I might try fixing this, but I've never fully understood the crazier C/C++ syntactic stuff... I'm guessing it gets stuck inside "struct" and doesn't find what it expects so it never escapes, and that breaks the rest of the parsing.

[sylveon]

It's possible that the mess of < and > on that line is what breaks it.

[joshgoebel]

It's possible that the mess of < and > on that line is what breaks it.

It's highly likely. :-) But since I don't understand C++ enough (at this level) [templates] I don't even know what's possible and what isn't, and hence no way I could hope to alter the parser for this without potentially just making it even worse.

I actually don't see any support for templates in the parser right now.

[joshgoebel]

Nevermidn it has it:

      {
        className: 'class',
        beginKeywords: 'class struct', end: /[{;:]/,
        contains: [
          {begin: /</, end: />/, contains: ['self']}, // skip generic stuff
          hljs.TITLE_MODE
        ]
      }

It doesn't look like the < and > pairs match to me (in your code sample)... so that would be a problem.

[joshgoebel]

Is << always guaranteed to be an operator or is there anything in C/C++ where you can have a nested <<something>> expression... if it's ONLY ever an operator this could probably be fixed by adding a check to eat up any extra <<s as such and that would bring the brackets back into balance.

[sylveon]

AFAIK, << is always an operator (if there's cases where it can not be, I have never encountered them)

In this snippet, it is an operator, albeit it's very hidden (basically it evaluates if someiostream << someT is correct, and if it is, gets what type this expression results in)

[sylveon]

Note that >> can not be an operator, mostly in cases where templates are nested, eg std::vector<std::vector<int>>

[joshgoebel]

AFAIK, << is always an operator

Well, as I said if that is a given then this should be fixable. :) Assuming that's the core issue, which seems likely.

[sylveon]

Just checked, << is indeed always an operator

[joshgoebel]

I opened a PR for the string issue: #2156

[egor-rogov]

Fixed

[sylveon]

This hasn't resolved the second snippet

[joshgoebel]

Yeah, this is still open. Someone else might have to look into second issue.

[paul-reilly]

I came across this today, a function with a trailing return type appears to break highlighting too:

auto foo() -> void;

[joshgoebel]

That is likely a separate issue.

[joshgoebel]

@klmr Lets step back and take a second look at this... I worry we've made it too complex... all the dealing with nested ( and < really feels like we've crossed the border and are now parsing the text - knowing far more than we should about the context.

What about:

diff --git a/src/languages/c-like.js b/src/languages/c-like.js
index ece09524..47afb47d 100644
--- a/src/languages/c-like.js
+++ b/src/languages/c-like.js
@@ -132,7 +132,7 @@ export default function(hljs) {
     // definition and shouldn't highlight anything that looks like one:
     // `return some()`, `else if()`, `(x*sum(1, 2))`
     variants: [
-      {begin: /=/, end: /;/},
+      {begin: /=/, end: /[;<>]/},
       {begin: /\(/, end: /\)/},
       {beginKeywords: 'new throw return else', end: /;/}
     ],
@@ -225,10 +225,10 @@ export default function(hljs) {
       },
       {
         className: 'class',
-        beginKeywords: 'class struct', end: /[{;:]/,
+        beginKeywords: 'class struct', end: /[{;:<>,]/,
         contains: [
           { beginKeywords: "final" },
-          {begin: /</, end: />/, contains: ['self']}, // skip generic stuff
           hljs.TITLE_MODE
         ]
       }

IE, just terminating most rules when they see a < or > that could be problematic... falling back into the normal highlighting loop. This does mean class T would be title highlighted everywhere, but I don't see that as a serious issue.

I also see a tiny issue with something like class = std but that could be fixed with a guard.

What would be a few other cases this would not handle so well?

[klmr]

@joshgoebel Despite my radio silence I’be been mulling this over and I’ve reached a similar (though not quite the same) conclusion. It’s definitely a good fix in the first instance, and we can add improved heuristics later. One thing I’d probably do is to collect test cases that currently fail in a separate PR or issue.

If you want to apply your diff as a “quick fix” go ahead, I think it shouldn’t work too too badly. I would add union (and probably also enum and) to the beginKeywords: they both initiate type declarations, and union (but not struct) can even be a template in C++.

Something else that I just thought of: how would this handle C++11’s enum class construct?

enum class Boolean : char {
    True, False, FileNotFound
}

[joshgoebel]

how would this handle C++11’s enum class construct?

Does that require special handling? Is enum just a keyword there and it would just work with the existing rules?

[klmr]

It currently doesn’t require special handling but once added to the beginKeywords, the snippet in my last comment is highlighted as follows:

<class><keyword>enum</keyword> <title>class</title> <title>Boolean</title> :</class> <keyword>unsigned</keyword>…

… but adding class struct to the nested beginKeywords: "final" fixes that (both enum class and enum struct are valid, because of course they are).