jeff-hykin / better-cpp-syntax

💾 The source of VS Code's C++ syntax highlighting
GNU General Public License v3.0
154 stars 29 forks source link

Scope Labels are the same at Function Definition and Invocation #470

Open Redoubts opened 4 years ago

Redoubts commented 4 years ago

Routed here from https://github.com/microsoft/vscode-cpptools/issues/5331

Checklist

In C and C++, function names at definition and invocation are marked with the same scope labels. I would expect these scope labels to be different because

1) In outside documentation, this is suggested:

Function and method names should be scoped using the following [variable.function], but only when they are being invoked. When defined, they should use entity.name.function

2) I see other languages and editors make this distinction. Indeed, GitHub formatting gives different colors, as seen in snippets below.

For example, using some zmq code, the C version has

static void *
worker_task(void *args)
{
    ...
    s_set_id(worker);
    ...
}

The defined function name "worker_task" is marked with the scopes

entity.name.function.c
meta.function.definition.parameters.c
meta.function.c
source.c

and "s_set_id" is marked with scopes

entity.name.function.c
meta.function-call.c
meta.block.c
source.c

Similarly, the C++ version has

static void *
worker_thread(void *arg) {
    ...
    s_set_id(worker);
    ...
}

with the defined function "worker_thread" marked with the scopes

entity.name.function.call.cpp
source.cpp

and "s_set_id" with the scopes

entity.name.function.call.cpp
source.cpp
jeff-hykin commented 4 years ago

Sorry I must've missed this issue. I agree definitions should be tagged different than calls, and that meta isn't good for theming.

The C grammar needs a re-write, which is being worked on. That will fix the meta tag issue.

However, in C++ situation is more complicated due to the parsing engine itself (TextMate). AFAIK GitHub gets around this issue because they're using the much more powerful TreeSitter parser.

The following code is currently parsed correctly for me:

static void * worker_thread(void *arg) {
    s_set_id(worker);
}

entity.name.function.definition for worker_thread entity.name.function.call for s_set_id The issue arises when the return value is on the previous line. The current parser (TextMate parser) can only ever "look" at one line at a time. It can switch contexts based on that line (e.g. switch into or out-of a I'm-in-a-function-body context or an I'm-in-a-struct-definition context) however it can't actually look ahead to see what is on the next line.

Right now the grammar basically looks for [return type] [func name] ( using a 4,000-character-long regular expression in order to activate the function-definition context. However, [func name] ( basically activates the function-call pattern.

This isn't to say nothing can be done, however it does mean that assumptions need to be made. For example, [return type] could activate the function definition context, and then we could match the function name after that. The problem is that [return type] can also overlap with other things, like variable declarations.

int 
a;
// vs
int
main() {
}

Or for a more realistic example

std::unordered_map<
    std::string, std::pair<std::string, std::map<std::string, std::map<std::string, std::map<std::string, std::pair<std::string, std::map<std::string, std::map<std::string, std::map<std::string, std::vector<std::string>>>>>>>>>
>
d;

std::unordered_map<
std::string, std::pair<std::string, std::map<std::string, std::map<std::string, std::map<std::string, std::pair<std::string, std::map<std::string, std::map<std::string, std::map<std::string, std::vector<std::string>>>>>>>>>
>
aFunction() {
    // code
}

Even beyond that, std::unordered_map< cannot even be confirmed to be a less than operator.

thing1<
  int
> a;

thing2 <
  100 + 0x5368 && std::cout << "thing 2 is small I guess";

However, we can at least tell if it is in the root context or not. However (unlike C AFAIK), function calls can occur in the root context.

So there probably are complex assumptions that could be made to improve this. Sadly the grammar isn't quite there yet, so it might be awhile before this is improved.

matter123 commented 4 years ago

thing2 < 100 + 0x5368 && std::cout << "thing 2 is small I guess";

That actually isn't valid c++

jeff-hykin commented 4 years ago

Screen Shot 2020-06-20 at 4 59 31 PM