C++: support alternative operators

[moriarty]

Fix for #1569

This is branched off of #1571 because I wanted the tests to be run... I'm not really familiar with the project or syntax... Setting up the testing etc was a bit of work, so I aim relaying on Travis and the code reviewers.

I found in #1570 that the alternative operators already existed, in two locations:
sonar/cxx/api/CxxKeyword.java
sonar/cxx/api/CppPunctuator.java

I used the ones from CxxKeywords and I just added them after where the regular ones were found.

---

This change is 

[ivangalkin]

Please split the change into the

1. fixing the binary repository (already done #1571 )
2. fixing the parsing (started with #1570, but tests and the grammar adjustment is missing)

After that I will merge the #1571 (normally @guwirth does it, but this is an important and blocking PR) and we will rebase remaining PRs on top of that. Thank you in advance.

[moriarty]

From: #1570 (comment)

Do you want this re-based after #1571 gets merged? For the tests to be run?

please merge #1570 and #1572 + rebase on upstream HEAD; thank you

This branch now contains both fixes.

But, from the comment above:

2. fixing the parsing (started with BUG? [sonar/cxx/api] NOT_EQ: "not_eq " to "not_eq" #1570, but tests and the grammar adjustment is missing)

If you're able to give me a hint where to change the tests and the grammar adjustments that would be great.

Thanks

[ivangalkin]

Reviewed 2 of 3 files at r1.
Reviewable status: 2 of 3 files reviewed, 1 unresolved discussion (waiting on @moriarty, @jmecosta, @Bertk, and @guwirth)

---

cxx-squid/src/main/java/org/sonar/cxx/parser/CxxGrammarImpl.java, line 579 at r1 (raw file):

AND_EQ

copy-n-paste error; must be OR_EQ

[ivangalkin]

Hi @moriarty

thank you once again for your PR;

If you're able to give me a hint where to change the tests and the grammar adjustments that would be great.

I believe, that the grammar change is just fine (except for one copy-n-paste error).

@ALL please review

[moriarty]

@ivangalkin I've fixed the copy-and-paste error. I amended the commit and did a force push out of habit... I meant to check the CONTRIBUTING.md first... I don't know how that works with this reviawable app, but here is the diff.

index 70c57d22..25c23559 100644
--- a/cxx-squid/src/main/java/org/sonar/cxx/parser/CxxGrammarImpl.java
+++ b/cxx-squid/src/main/java/org/sonar/cxx/parser/CxxGrammarImpl.java
@@ -576,7 +576,7 @@ public enum CxxGrammarImpl implements GrammarRuleKey {
     b.rule(foldOperator).is(
       b.firstOf( // C++
         "+", "-", "*", "/", "%", "ˆ", CxxKeyword.XOR, "&", CxxKeyword.BITAND, "|", CxxKeyword.BITOR, "<<", ">>",
-        "+=", "-=", "*=", "/=", "%=", "ˆ=", CxxKeyword.XOR_EQ, "&=", CxxKeyword.AND_EQ, "|=", CxxKeyword.AND_EQ, "<<=", ">>=", "=",
+        "+=", "-=", "*=", "/=", "%=", "ˆ=", CxxKeyword.XOR_EQ, "&=", CxxKeyword.AND_EQ, "|=", CxxKeyword.OR_EQ, "<<=", ">>=", "=",
         "==", "!=", CxxKeyword.NOT_EQ, "<", ">", "<=", ">=", "&&", CxxKeyword.AND, "||", CxxKeyword.OR, ",", ".*", "->*"
       )
     );

[moriarty]

Reviewable status: 2 of 3 files reviewed, 1 unresolved discussion (waiting on @ivangalkin, @jmecosta, @Bertk, and @guwirth)

---

cxx-squid/src/main/java/org/sonar/cxx/parser/CxxGrammarImpl.java, line 579 at r1 (raw file):

Previously, ivangalkin wrote…

AND_EQ

copy-n-paste error; must be OR_EQ

Done.

[ivangalkin]

Reviewed 1 of 3 files at r1.
Reviewable status: 2 of 3 files reviewed, all discussions resolved (waiting on @ivangalkin, @jmecosta, @Bertk, and @guwirth)

[ivangalkin]

@moriarty thank you!
@guwirth I couldn't find any explanation, why alternative operators are not mentioned in the original BNF w.r.t. binary/unary/assignment operator etc... but still are listed as tokens. I believe their role in the language grammar is covered by the following statement:

In all respects of the language, each alternative token behaves exactly the same as its primary token, except for its spelling

So at the first glance the solution looks valid to me. There are at least 2 cases where according to the clang parser the tokens of alternative operators are considered to be identifiers (see below), but probably (?) we can ignore them.

• C++ https://github.com/llvm-mirror/clang/blob/e3de7bb263271008c55cc8bf3186a3a08999fbaa/lib/Parse/ParseDeclCXX.cpp#L3781

/// C++11 [dcl.attr.grammar]p3:
///   If a keyword or an alternative token that satisfies the syntactic
///   requirements of an identifier is contained in an attribute-token,
///   it is considered an identifier.

• Object-C https://github.com/llvm-mirror/clang/blob/e3de7bb263271008c55cc8bf3186a3a08999fbaa/lib/Parse/ParseExpr.cpp#L323

    // In Objective-C++, alternative operator tokens can be used as keyword args
    // in message expressions. Unconsume the token so that it can reinterpreted
    // as an identifier in ParseObjCMessageExpressionBody. i.e., we support:
    //   [foo meth:0 and:0];
    //   [foo not_eq];

[guwirth]

@moriarty thx for providing this. Can’t remember why this was not supported in the past? Only thing I’m wondering: why is it not enough to do that in the preprocessor? Using header file iso646.h is also working?

[moriarty]

Hi,

I'm not sure about including iso646.h or doing this with the preprocessor... this is a large existing code base with a lot of mixed C++ and Python, so they reason the alternative operators were being used, was more to write python-like code... I just tried to run Sonar and it gave a lot of error output as in #1569 and skipped whatever was in the blocks behind the alternative operator.

I tried switching them from not to ! etc, and sonar was able to continue... but there are several hundred locations in multiple repositories which I thought it would be easier to try and make sonar-cxx support them without changing the code.

[guwirth]

Hi @moriarty the idea is to use
https://github.com/SonarOpenCommunity/sonar-cxx/wiki/Force-Include with iso646.h. Not too change the original code.

[moriarty]

@guwirth

I tried with

https://github.com/SonarOpenCommunity/sonar-cxx/wiki/Force-Include with iso646.h. Not too change the original code.

--- a/sonar-project.properties
+++ b/sonar-project.properties
@@ -2,6 +2,7 @@ sonar.projectKey=some-project
 sonar.sources=src
 sonar.tests=test
 sonar.language=c++
+sonar.cxx.forceIncludes=iso646.h
 sonar.cxx.defines=__GNUC__ 7 \n\ __GNUC_MINOR__ 3 \n\ BOOST_VERSION 106501 \n\ TEST_MODE 0 \n\ SILENT 0

And still get the errors as reported in #1569

WARN: [.../consumer.cpp:66]: skip declaration: while ( not cancelled_ ) {

But if I change that line to while ( !cancelled_) { it works, or at least that line does not show up as skip declaration, there are still a few hundred others and I only tried changing a handful and checking that they don't show up

[moriarty]

I just saw your link to #778... If that solution worked for others, then maybe the include didn't work due to ccache? I've cleaned that and my build directory and will try again...

Update: now I've added add_definitions(-include iso646.h) ... to my cmake (still waiting for it to build) but this all seems like work arounds.

[guwirth]

@moriarty thx for testing. Let’s wait for other opinions. I always like to keep the grammar as small as possible.

[moriarty]

The solutions proposed did not work.

I tried to include iso646.h first via the sonar-project.properties file, and then manually in a few files.
This didn't work.

Here is the contents of the file:

/* Copyright (C) 1997-2017 Free Software Foundation, Inc.

This file is part of GCC.

GCC is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 3, or (at your option)
any later version.

GCC is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
GNU General Public License for more details.

Under Section 7 of GPL version 3, you are granted additional
permissions described in the GCC Runtime Library Exception, version
3.1, as published by the Free Software Foundation.

You should have received a copy of the GNU General Public License and
a copy of the GCC Runtime Library Exception along with this program;
see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
<http://www.gnu.org/licenses/>.  */

/*
 * ISO C Standard:  7.9  Alternative spellings  <iso646.h>
 */

#ifndef _ISO646_H
#define _ISO646_H

#ifndef __cplusplus
#define and	&&
#define and_eq	&=
#define bitand	&
#define bitor	|
#define compl	~
#define not	!
#define not_eq	!=
#define or	||
#define or_eq	|=
#define xor	^
#define xor_eq	^=
#endif

#endif

and the file came from:

$ dpkg -S /usr/lib/gcc/x86_64-linux-gnu/7/include/iso646.h 
libgcc-7-dev:amd64: /usr/lib/gcc/x86_64-linux-gnu/7/include/iso646.h
$ apt show libgcc-7-dev
Package: libgcc-7-dev
Version: 7.3.0-27ubuntu1~18.04
Priority: optional
Build-Essential: yes
Section: libdevel
Source: gcc-7

A workaround, which changed the warnings shown to the terminal, was to add them all as defines to the sonar-project.properties file..

 sonar.sources=src
 sonar.tests=test
 sonar.language=c++
-sonar.cxx.defines=__GNUC__ 7 \n\ __GNUC_MINOR__ 3 \n\ BOOST_VERSION 106501 \n\ TEST_MODE 0 \n\ SILENT 0
+sonar.cxx.defines=and && \n\ and_eq &= \n\ bitand & \n\ bitor | \n\ compl ~ \n\ not ! \n\ not_eq != \n\ or || \n\ or_eq |= \n\ xor ^ \n\ xor_eq ^= \n\ __GNUC__ 7 \n\ __GNUC_MINOR__ 3 \n\ BOOST_VERSION 106501 \n\ TEST_MODE 0 \n\ SILENT 0

But now I get a lot of new warnings, instead of the old one.

WARN: [/usr/include/boost/mpl/aux_/include_preprocessed.hpp:37]: cannot find the sources for '#   include BOOST_PP_STRINGIZE(boost/mpl/aux_/preprocessed/AUX778076_PREPROCESSED_HEADER)'
WARN: Error evaluating expression '!  ' for AstExp 'IDENTIFIER: not', assuming 0

I don't mind a few sonar.cxx.defines. As a developer I know why the GNU 7 and BOOST_VERSION 106501 defines are there, they're to support two APIs (but it's unfortunately only possible to cover one using this sonar-cxx plugin, so they're scoped to just one line around the call which changed it's API.

But it feels wrong to need to add all of the alternative representations as sonar.cxx.defines to make the parser not skip valid c++.

[guwirth]

@moriarty & @ivangalkin see two possibilities:

• use your approach as is
  • "bigger" grammar
• set the macros from iso646.h automatically in the preprocessor (without the need for sonar.cxx.defines)
  • grammar unchanged
  • maybe other side effects

What do you think?

[ivangalkin]

@guwirth I would personally vote for the grammar extension. In my judgement one cannot relate on our preprocessor implementation. The patch doesn't bloat our grammar significantly and solves the problem with alternative operators once and forever (hopefully).