#132 Port remaining grammar tests to C++

[vityaman]

• Ported Whitebox and CPP14 grammar tests.
• Found serious behavior divergence at CPP14Test.cpp.
• Edited GitHub Workflows to run only when required.
• Downgraded C++ from 23 to 20.

[vityaman]

Current collectCandidates is overloaded with parameters. I have thoughts, that it would be better to have something like this (just thoughts):

class CodeCompletionCore final {
  struct Params {
    antlr4::Parser* parser;
    std::chrono::milliseconds timeout = 0ms;
    std::atomic<bool>* is_cancelled = nullptr;
  }

  explicit CodeCompletionCore(Params params);

  CandidatesCollection collectCandidates(std::size_t caretTokenIndex);
  CandidatesCollection collectCandidates(
    std::size_t caretTokenIndex, antlr4::ParserRuleContext* context);
}

auto Usage(antlr4::Parser* parser) {
  CodeCompletionCode completion({
    .parser = parser,
    .timeout = 100ms,
  });
  return completion;
}

[mike-lischke]

Not sure what your point here is. You don't like default parameters?

[vityaman]

The problem with current signature

CandidatesCollection collectCandidates(
  size_t caretTokenIndex, 
  antlr4::ParserRuleContext * context = nullptr, 
  size_t timeoutMS = 0, 
  std::atomic<bool> * cancel = nullptr
);

It is that, it seems, that timeoutMS and cancel will not change from call to call, and their presence generates many combinations of arguments sets.

collectCandidates(0);      // OK, readable
collectCandidates(0, ctx); // OK, readable
collectCandidates(0, nullptr, /* timeoutMS = */ 100); // Forces me to put nullptr
collectCandidates(0, nullptr, /* timeoutMS = */ 0, &cancel); // Forces me to add even zero timeout
collectCandidates(0, nullptr, 0, &cancel); // What?

It makes reading caller code harder. Also, it forces putting this comments like /* timeoutMS = */ because there are no named arguments in C++.

In addition, in examples I showed hard-coded values, but in real code there will be variable names, which will make this line of code longer.

But in constructor we can provide user with named arguments with such Params struct.

I think that a default argument is okay if there are no more than one of such argument (talking about languages without named arguments).

Also, I agree that overloading the method is redundant here.

[vityaman]

Added default parameters and removed that functions.

[mike-lischke]

An alternative would be a simple struct as parameter (object/interface in TypeScript), which allows to specify parameters in any order, have names for each and leave out any that are marked as being optional. I use this often, as it comes as close to named parameters (e.g. like in Swift) as you can get.

[vityaman]

Will create an issue to analyze these results.

[mike-lischke]

Not sure what your point here is. You don't like default parameters?

[mike-lischke]

Thanks!