Support C++/CLI

[Bertk]

We have native and managed C++ code and until now C++/CLI is not supported from any plug-in. It should be possible to support the C++/CLI extension within the C++ community plugin. What is your opinion on this feature? I created a branch with a first limited support of C++/CLI (c0853b265d444be4878dd7ce3942b75bdb1330d2)

See also: https://msdn.microsoft.com/en-us/library/xey702bw.aspx http://www.ecma-international.org/publications/files/ECMA-ST/Ecma-372.pdf

[jmecosta]

Great, saw it and was already trying in our code base. Works OK, some parser errors still

On Sun, Jun 28, 2015, 11:07 Bert notifications@github.com wrote:

We have native and managed C++ code and until now C++/CLI is not supported from any plug-in. It should be possible to support the C++/CLI extension within the C++ community plugin. What is your opinion on this feature? I created a branch with a first limited support of C++/CLI (c0853b2 https://github.com/wenns/sonar-cxx/commit/c0853b265d444be4878dd7ce3942b75bdb1330d2 )

See also: https://msdn.microsoft.com/en-us/library/xey702bw.aspx http://www.ecma-international.org/publications/files/ECMA-ST/Ecma-372.pdf

— Reply to this email directly or view it on GitHub https://github.com/wenns/sonar-cxx/issues/539.

[guwirth]

Hi,

I'm not sure if we should add one more 'dialect' the parser is already complex enough. On the other hand C++\CLI is not a complete separate language there are also mixed projects possible even in one source file. My concerns are always that we end up in a contrary grammar.

Regards,

Am 28.06.2015 um 10:07 schrieb Bert notifications@github.com:

We have native and managed C++ code and until now C++/CLI is not supported from any plug-in. It should be possible to support the C++/CLI extension within the C++ community plugin. What is your opinion on this feature? I created a branch with a first limited support of C++/CLI (c0853b2)

See also: https://msdn.microsoft.com/en-us/library/xey702bw.aspx http://www.ecma-international.org/publications/files/ECMA-ST/Ecma-372.pdf

— Reply to this email directly or view it on GitHub.

[jmecosta]

can it be enabled via a configuration property? as Bert said we dont find this kind of support in any other plugin. so this adds loads of value

On Sun, 28 Jun 2015 at 11:50 Günter Wirth notifications@github.com wrote:

Hi,

I'm not sure if we should add one more 'dialect' the parser is already complex enough. On the other hand C++\CLI is not a complete separate language there are also mixed projects possible even in one source file. My concerns are always that we end up in a contrary grammar.

Regards,

Am 28.06.2015 um 10:07 schrieb Bert notifications@github.com:

We have native and managed C++ code and until now C++/CLI is not supported from any plug-in. It should be possible to support the C++/CLI extension within the C++ community plugin. What is your opinion on this feature? I created a branch with a first limited support of C++/CLI (c0853b2)

See also: https://msdn.microsoft.com/en-us/library/xey702bw.aspx

http://www.ecma-international.org/publications/files/ECMA-ST/Ecma-372.pdf

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub https://github.com/wenns/sonar-cxx/issues/539#issuecomment-116228227.

[Bertk]

I am fine to separate this extension until it is complete and we can assess the impact on the grammar. Is it possible to have a feature branch for this in CI? I do not want to go for a separate fork.

[guwirth]

Question is how much work is left for an initial version? How long do you need to finish the feature?

Am 28.06.2015 um 11:06 schrieb Bert notifications@github.com:

I am fine to separate this extension until it is complete and we can assess the impact on the grammar. Is it possible to have a feature branch for this in CI? I do not want to go for a separate fork.

— Reply to this email directly or view it on GitHub.

[Bertk]

I am not sure but we should mark this as "experimental" feature and go for a time-boxed approach. I can only spent time on weekends for the C++/CLI feature and propose 13th July for the initial implementation. I am looking for feedback from my colleagues on the existing C++/CLI support early next week and plan to implement the next parts based on the parser error ratio.

@jmecosta: Can you please send me your feedback on the remaining parser errors as well.

[jmecosta]

Yep will do. It will run today night in all code base. I will present my findings then

On Sun, Jun 28, 2015, 19:59 Bert notifications@github.com wrote:

I am not sure and but we should mark this as "experimental" feature and go for a time-boxed approach. I can only spent time on weekends for the C++/CLI feature and propose 13th July for the initial implementation. I am looking for feedback from my colleagues on the existing C++/CLI support early next week and plan to implement the next parts based on the parser error ratio.

@jmecosta https://github.com/jmecosta: Can you please send me your feedback on the remaining parser errors as well.

— Reply to this email directly or view it on GitHub https://github.com/wenns/sonar-cxx/issues/539#issuecomment-116298479.

[chuchu]

I would really love to see some C++/CLI features. As guwirth has already mentioned the CLI extension is not a separate dialect. Most times it comes together with ordinary C++ and it has the same file extensions. It would be very helpful if this plugin would (at least) ignore the CLI specialties. Thanks for the work!

[Bertk]

@guwirth, @jmecosta : I have some problems with the property declaration. int get() { return MyInt; } creates a parser error.

    property int Property_Block {   // context-sensitive keyword
      int get() { return MyInt; }
    }

It do not see a parser error if I remove { return MyInt; }

    property int Property_Block {   // context-sensitive keyword
      int get() ;
    }

I hesitate to touch the grammar "function-definition". Any advice is appreciated (ea65fb760c52d44993ce2f9fa7da963a2e2f19e7).

[jmecosta]

sorry @Bertk i am really not comfortable touching those either

[guwirth]

@Bertk: For my understanding this setter/getter belongs to keyword property. Means this set and get are always only inside a property definition? Would create a new grammar rule starting with property? From C++ point of view this is a variable declaration followed by a block something like a class with member functions. Makes this sense?

---

8.8.4 Properties A property is a member that behaves as if it were a field. There are two kinds of properties: scalar and indexed. A scalar property enables field-like access to a class or CLI object. Examples of scalar properties include the length of a string, the size of a font, the caption of a window, and the name of a customer. An indexed property enables array-like access to a CLI object. An example of an index property is a bit-array class.

Properties are an evolutionary extension of fields—both are named members with associated types, and the syntax for accessing scalar fields and scalar properties is the same, as is that for accessing CLI arrays and indexed properties. However, unlike fields, properties do not denote storage locations. Instead, properties have accessor functions that specify the statements to be executed when their values are read or written.

Properties are defined with property definitions. The first part of a property definition looks quite similar to a field definition. The second part includes a get accessor function and/or a set accessor function. Properties that can be both read and written include both get and set accessor functions. In the example below, the Point class defines two read-write properties, X and Y.

public value class Point {
   int Xor;
   int Yor;
   public:
   property int X {
      int get() { return Xor; }
      void set(int value) { Xor = value; }
   }
   property int Y {
      int get() { return Yor; }
      void set(int value) { Yor = value; }
   }
...

A default-indexed property allows array-like access directly on an instance. [Note: Other languages refer to default-indexed properties as “indexers”. end note]

Default-indexed property definitions are similar to property definitions, with the main differences being that default-indexed properties are nameless and that they include indexing parameters. The indexing parameters are provided between square brackets.

property Object^ default[int] { // default-indexed property
   Object^ get(int index) {
      if (!ValidIndex(index))
         throw gcnew Exception("Index out of range.");
      else
         return GetNode(index)->Value;
   }
   void set(int index, Object^ value) {
      if (!ValidIndex(index))
         throw gcnew Exception("Index out of range.");
      else
         GetNode(index)->Value = value;
   } 
...

[Bertk]

Thanks for the details. For now I tried to ignore cliAccessorSpecification and cliAccessorDeclaration for the cliPropertyDefinition rule and drop all statements within the block but the parser error still occurs. Meanwhile I modified a small part which can be successfully parsed

    property int Property_Block {   // context-sensitive keyword
      int get() {} ;
    }

but without the ; parser fails

    property int Property_Block {   // context-sensitive keyword
      int get() {} 
    }

I need to take a closer look on this. By the way, solving the issue #476 would be very helpful for this task.

[Bertk]

@jmecosta, @guwirth : Do you know the reason why we use variations of rule decl-specifier-seq in CxxGrammarImpl(). The standard defines this:

decl-specifier-seq:
    decl-specifier attribute-specifier-seqopt
    decl-specifier decl-specifier-seq 

The property rule is never applied because "property" is handled as a class-name identifier.

There is another rule for memberDeclSpecifierSeq which implements a workaround ...

    b.rule(memberDeclSpecifierSeq).is(
        b.oneOrMore(
            b.nextNot(b.sequence(b.optional(memberDeclaratorList), ";")),
            declSpecifier
        ),
        b.optional(attributeSpecifierSeq)
        );

Looks like some clean-up is necessary before C++/CLI support can be introduced.

104: public Rule conditionDeclSpecifierSeq;  
105: public Rule forRangeDeclSpecifierSeq;  
106: public Rule parameterDeclSpecifierSeq;  
107: public Rule functionDeclSpecifierSeq;  
109: public Rule memberDeclSpecifierSeq;  

Some hints are very welcome :smirk:

And this workaround is the reason why "properties" rule is not applied.

    b.rule(simpleTypeSpecifier).is(
      b.firstOf(
        "char", "char16_t", "char32_t", "wchar_t", "bool", "short", "int", "long", "signed", "unsigned", "float", "double", "void", "auto",
        decltypeSpecifier,
        b.sequence(nestedNameSpecifier, CxxKeyword.TEMPLATE, simpleTemplateId),

            // TODO: the "::"-Alternative to nested-name-specifier is because of need to parse
            // stuff like "void foo(::A a);". Figure out if there is another way

        b.sequence(b.optional(b.firstOf(nestedNameSpecifier,"::")), typeName)
        )
      );

[guwirth]

@Bertk I'm sorry I'm not familiar with this. I think @wenns wrote it?

[Bertk]

C++/CLI property support is now working and I learned some GrammarRuleKey magic

[guwirth]

What is your lessons learned? Can you share it....

Am 06.07.2015 um 22:02 schrieb Bert notifications@github.com:

C++/CLI property support is now working and I learned some GrammarRuleKey magic

— Reply to this email directly or view it on GitHub.

[guwirth]

@Bertk we both working on the grammar (see PR). In which order should we merge it?

[Bertk]

@guwirth I will create the PR this Weekend. The first tests of C++/CLI support are positive but I do not have any performance feedback. I am not sure whether your grammar updates and the CLI will create conflicts. Did you check the CLI updates? @jmecosta Did you test the latest updates for the CLI attributes?

[guwirth]

@Bertk will have a look after there is a PR available. Can't test because we have no C++\CLI code in our projects.

[jmecosta]

Just build it now and its running during the weekend I will come back with news results tomorrow

On Fri, Jul 10, 2015, 22:31 Günter Wirth notifications@github.com wrote:

@Bertk https://github.com/Bertk will have a look after there is a PR available. Can't test because we have no C++\CLI code in our projects.

— Reply to this email directly or view it on GitHub https://github.com/wenns/sonar-cxx/issues/539#issuecomment-120502904.

[Bertk]

created PR #582

[jmecosta]

some results, with last pr: fixed: --> public enum class ProfileType

issues still present in this version: --> public ref class Plugin01 : public --> std::set GetAssociatedIds(Point ^ gcPoint)

no regressions on this check. only checked 2 projects. i will add more soon as they finish

[jmecosta]

new? [10:48:05] : [Step 1/1] --> property System::String^ Name [10:48:05] : [Step 1/1] 58: {

[10:48:15] : [Step 1/1] 10:48:15.331 ERROR - Parse error at line 94 column 28: [10:48:15] : [Step 1/1] [10:48:15] : [Step 1/1] 86: ^, Object^>^>^ gcPlugin); [10:48:15] : [Step 1/1] 87: [10:48:15] : [Step 1/1] 88: [10:48:15] : [Step 1/1] 89: String ^ GetPluginData(String ^ gcGuid); [10:48:15] : [Step 1/1] 90: [10:48:15] : [Step 1/1] 91: [10:48:15] : [Step 1/1] 92: bool CheckExistByObjectType(grprObjectType_e ObjectType); [10:48:15] : [Step 1/1] 93: [10:48:15] : [Step 1/1] --> void DefineData()

this was fixed: [04:50:26] : [Step 1/1] --> [ImportMany(IPlugin::typeid)]

this was fixed, for this run, altought it fails in some other project [04:50:21] : [Step 1/1] --> public ref class PresentationModelServer : public IService

for this project, minus 1 parse error. There is another big project analysing now, i will check in a bit later

[guwirth]

@jmecosta Child spinales have an eye on pure c++ projects please. Are there any changes adding grammar extension?

[jmecosta]

@guwirth not sure what you mean

[Bertk]

@jmecosta I tried to find the issue and used the following code which failed:

public ref class S {
  System::String^ Name;
public:
  S() : Name("test") {}

  property  String^ Property_Block {   // context-sensitive keyword
    String^ get() { return Name; };
  }
};

I need to compile to check whether this is correct code and identify the problem with String^. You can use the sslr-cxx-toolkit to locate the problematic line and eliminate line by line until the exception disappears. By the way, I identified a problem with attributes and delegate definition: [delegate:Attr] delegate void Test2(); The parser fails and I need to find the source.

[guwirth]

Sorry my auto correction 😁. Like to ask you to have also an eye on pure C++ code. If C++\CLI grammar extensions have a negative impact on pure C++ code.

[jmecosta]

@guwirth i am checking both, so far i havent identified any regressions... actually the parser has improved quite a bit... i will run a few analysis with trunck and then with the c++/cli to be sure

[jmecosta]

@Bertk i am not totally sure what causes it. removing the getter fixes it. ive place a System::String^ variable = "asdas"; that works fine. and your code snippet is compiling just fine

[francoisferrand]

I have not had the time to check in any way, but the "new" reserved identifiers seem like a big issue...

is there any way to specify if a file should be parsed as plain c++ or c++/cli, and change the list of keywords accordingly?

On 11 juil. 2015, at 18:12, Jorge Costa notifications@github.com wrote:

@guwirth i am checking both, so far i havent identified any regressions...

— Reply to this email directly or view it on GitHub.

[guwirth]

Difficult. Within one file there could be managed and unmanaged code (#pragma managed/unmanaged). Mostly there is also a using namespace System in the C++\CLI files. The project files contain the /clr switch.

Am 11.07.2015 um 22:12 schrieb Typz notifications@github.com:

I have not had the time to check in any way, but the "new" reserved identifiers seem like a big issue...

is there any way to specify if a file should be parsed as plain c++ or c++/cli, and change the list of keywords accordingly?

On 11 juil. 2015, at 18:12, Jorge Costa notifications@github.com wrote:

@guwirth i am checking both, so far i havent identified any regressions...

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub.

[jmecosta]

Ok, so ive checked now the results with the c++/cli and without:

project 1 - 30K LOC

• no regressions, we have 1 less parser error in c++/cli. current nbm parse errors = 4

project 2 - 285K LOC

• no regressions, we have 5 less parser error in c++/cli. current nbm parse errors = 12

project 3 - 30K LOC

• no regressions, we have 5 less parser error in c++/cli. curren cout = 4

project 4 - 420K LOC

• no regressions, no c++/cli. curren cout = 21

project 5 - 1M LOC

• 6 regressions, we have 10 less parser error in c++/cli. curren cout = 76

[13:54:02] : [Step 1/1] 13:54:02.662 ERROR - Parse error at line 291 column 22: - regression event used as identifier

[13:54:03] : [Step 1/1] 13:54:03.582 ERROR - Parse error at line 174 column 17: - regression event used as identifier

[13:54:03] : [Step 1/1] 13:54:03.730 ERROR - Parse error at line 1231 column 26: - regression event used as identifier

[13:54:04] : [Step 1/1] 13:54:04.148 ERROR - Parse error at line 633 column 17: - regression event used as identifier

[13:54:38] : [Step 1/1] 13:54:38.759 ERROR - Parse error at line 456 column 36: - regression event used as identifier

[13:54:38] : [Step 1/1] 13:54:38.780 ERROR - Parse error at line 223 column 35: - regression event used as identifier

This last project contains some ui logic, thats the reason why the event keyword is used in some places.. however i think i can modify those to more sensible names. some of those were like

void print_event(type event)

But i can understand @Typz concern about this know limitation.

[francoisferrand]

At the very least there must be an option to disable the feature completely for the project/analysis. I especially fear the event and delegate keywords, which can easily be used in standard code.

I don't remember exactly if this is possible with sslr, but do we really need to define new keywords? It would be better if these were more like the new "contextual" keywords in c++11, which are not reserved in general...

So we can include the strings directly in each of new CLI rule, without defining new keywords? If not possible, another option would be to play tricks with the preprocessor, and replace the keywords if in the right/wrong section of the file...

On 12 juil. 2015, at 10:00, Jorge Costa notifications@github.com wrote:

Ok, so ive checked now the results with the c++/cli and without:

project 1 - 30K LOC

no regressions, we have 1 less parser error in c++/cli. current nbm parse errors = 4 project 2 - 285K LOC

no regressions, we have 5 less parser error in c++/cli. current nbm parse errors = 12 project 3 - 30K LOC

no regressions, we have 5 less parser error in c++/cli. curren cout = 4 project 4 - 420K LOC

no regressions, no c++/cli. curren cout = 21 project 5 - 1M LOC

6 regressions, we have 10 less parser error in c++/cli. curren cout = 76 [13:54:02] : [Step 1/1] 13:54:02.662 ERROR - Parse error at line 291 column 22: - regression event used as identifier [13:54:03] : [Step 1/1] 13:54:03.582 ERROR - Parse error at line 174 column 17: - regression event used as identifier [13:54:03] : [Step 1/1] 13:54:03.730 ERROR - Parse error at line 1231 column 26: - regression event used as identifier [13:54:04] : [Step 1/1] 13:54:04.148 ERROR - Parse error at line 633 column 17: - regression event used as identifier [13:54:38] : [Step 1/1] 13:54:38.759 ERROR - Parse error at line 456 column 36: - regression event used as identifier [13:54:38] : [Step 1/1] 13:54:38.780 ERROR - Parse error at line 223 column 35: - regression event used as identifier This last project contains some ui logic, thats the reason why the event keyword is used in some places.. however i think i can modify those to more sensible names. some of those were like

void print_event(type event)

But i can understand @Typz concern about this know limitation.

— Reply to this email directly or view it on GitHub.

[Bertk]

The syntax high-lighting within SonarQube will not work without the declaration as a keyword and I started using strings within the rules. Using strings for the rules is of course an Option (but is not DRY) if the CLI keywords generate a lot of parser errors for existing code. I will rework the PR and add also fixes for the property definition.

[Bertk]

@jmecosta Can you please excute your tests with the latest updates (bf8ffd8 )

[jmecosta]

Yep I will, any chance you can rebase master to that branch. There are some merge conflicts and I might brake the grammar

On Sun, Jul 12, 2015, 18:28 Bert notifications@github.com wrote:

@jmecosta https://github.com/jmecosta Can you please excute your tests with the latest updates (bf8ffd8 https://github.com/wenns/sonar-cxx/commit/bf8ffd85b49f7e7abda0f5f444c2b911610dd5dd )

— Reply to this email directly or view it on GitHub https://github.com/wenns/sonar-cxx/issues/539#issuecomment-120730603.

[Bertk]

@jmecosta I will check and ping you again.

[Bertk]

@jmecosta I rebased the PR #582 but there is an issue with CxxPublicApiVisitorTest.java and could build everything after commenting out the failing tests of CxxPublicApiVisitorTest.java. I expect less parser errors for CLI property extension :wink:

[jmecosta]

OK thanks. I will let this running during the nigth and get back to you tomorrow

On Sun, Jul 12, 2015, 21:00 Bert notifications@github.com wrote:

@jmecosta https://github.com/jmecosta I rebased the PR #582 https://github.com/wenns/sonar-cxx/pull/582 but there is an issue with CxxPublicApiVisitorTest.java and could build everything after commenting out the failing tests of CxxPublicApiVisitorTest.java. I expect less parser errors for CLI property extension [image: :wink:]

— Reply to this email directly or view it on GitHub https://github.com/wenns/sonar-cxx/issues/539#issuecomment-120751143.

[jmecosta]

@Bertk results are now fine, in fact great results. @Typz perhaps you should also try to run this in your codebase.

On the big project the had the most problems. now there is only 40 parse errors. and c++cli 2.

[09:17:34] : [Step 1/1] --> IEnumerable<LayerVisibilityInfo^>^ LayerVisibilities)

[09:17:51] : [Step 1/1] 127: for each(App::Feature^ feature in App::Current->Features) [09:17:51] : [Step 1/1] 128: { [09:17:51] : [Step 1/1] --> if(feature->GetType() == IConnectivity::typeid)

[Bertk]

@jmecosta Thanks for the feedback. Can you please check with the sslr toolkit this code snipped?

main()
{

  IEnumerable^ LayerVisibilities;

  for each(App::Feature^ feature in App::Current->Features)
  {
    if (feature->GetType())
    {
    }

    if (feature->GetType() == true)
    {
    }

    if (feature->GetType() == test)
    {
    }

    if (feature->GetType() == myname::test)
    {
    }

  }
}

This work fine for me and I cannot reproduce the 2 issues. I guess the problem are different Statements and the parser message indicate the wrong line.

[jmecosta]

@Bertk just tested with the latest upstream, this is the faulty snippet that fails

static IInterface^ Connect()
{
    for each(App::Feature^ feature in App::Current->Features)
    {
        if(feature->GetType() == IInterface::typeid)
            return (IInterface^)feature;
    }
    return nullptr;
}

if instead of this if(feature->GetType() == IInterface::typeid)

we have this if(feature->GetType() == true)

then it does not fail

[Bertk]

@jmecosta This issue is related to CxxKeyword.TYPEID. The parser is able to handle the following:

static IInterface^ Connect()
{
    for each(App::Feature^ feature in App::Current->Features)
    {
        if(feature->GetType() == interface::typed)
            return (IInterface^)feature;
    }
    return nullptr;
}

If CxxKeyword.TYPEID is removed and strings are used for the postfixExpression rule everything will work OK but I am not sure whether this is an acceptable solution.

[guwirth]

In C++ typeid must follow an argument in parenthesis. Maybe this can be used to distinguish between C++ typeid and C++\CLI? http://en.cppreference.com/w/cpp/language/typeid https://msdn.microsoft.com/en-us/library/ms235260.aspx

[Bertk]

@guwirth Sorry, this is not related to C++/CLI support but a general problem of the keyword handling and identifier detection. We should create a separate issue for a discussion how this can be improved.

[guwirth]

@Bertk yes create an extra issue for describing the keyword issue makes sense.

[Bertk]

Initial C++/CLI support is available with PR #582.

[zadigus]

Are you supporting references in c++/cli code to .net dlls e.g. with #using statement?

[guwirth]

Hi @zadigus,

the parser is supporting C++/CLI grammar. #using is not really supported because the plugin is doing only a syntax analysis and creating an AST out of it. What do you expect/missing reading this directive?

https://docs.microsoft.com/en-us/cpp/preprocessor/hash-using-directive-cpp?view=msvc-160

Regards,

[zadigus]

Hi @guwirth,

I have made a little test on a very small c++ codebase with resharper. The c++ code compiles fine and executes perfectly. In that codebase, I have a #using statement with which I load a .net dll. Then, in the c++ code, I use classes out of that dll, with statement like e.g.

someObject.someFunc<My::DotNet::Namespace::TheClass^>();

and resharper cannot resolve the My::DotNet::Namespace. I was wondering how different the situation would be with sonarqube. Probably the same, if I understand your comment properly?