Improved AST:

- Fixed TODO for ternary operator. Add parantheses between ? and : where necessary as a preparation for createAst()
- Improved AST validation:
-- Better message for binary operator with only one operand
-- Ensure ? has : as second operand (#7035)

lib/tokenize.cpp

@@ -1713,6 +1713,8 @@ bool Tokenizer::tokenize(std::istream &code,
 
     if (simplifyTokenList1(FileName)) {
         if (!noSymbolDB_AST) {
+            prepareTernaryOpForAST();
+
             createSymbolDatabase();
 
             // Use symbol database to identify rvalue references. Split && to & &. This is safe, since it doesn't delete any tokens (which might be referenced by symbol database)
@@ -1729,10 +1731,13 @@ bool Tokenizer::tokenize(std::istream &code,
 
             // Verify that ast looks ok
             for (const Token *tok = list.front(); tok; tok = tok->next()) {
-                // internal error / syntax error if binary operator only has 1 operand
+                // Syntax error if binary operator only has 1 operand
-                if ((tok->isAssignmentOp() || tok->isComparisonOp() || Token::Match(tok,"[|^/%]")) && tok->astOperand1() && !tok->astOperand2()) {
+                if ((tok->isAssignmentOp() || tok->isComparisonOp() || Token::Match(tok,"[|^/%]")) && tok->astOperand1() && !tok->astOperand2())
-                    throw InternalError(tok, "ast", InternalError::INTERNAL);
+                    throw InternalError(tok, "Syntax Error: AST broken, binary operator has only one operand.", InternalError::SYNTAX);
-                }
+
+                // Syntax error if we encounter "?" with operand2 that is not ":"
+                if (tok->astOperand2() && tok->str() == "?" && tok->astOperand2()->str() != ":")
+                    throw InternalError(tok, "Syntax Error: AST broken, ternary operator lacks ':'.", InternalError::SYNTAX);
             }
 
             SymbolDatabase::setValueTypeInTokenList(list.front());
@@ -10140,6 +10145,41 @@ bool Tokenizer::simplifyStrlen()
     return modified;
 }
 
+void Tokenizer::prepareTernaryOpForAST()
+{
+    // http://en.cppreference.com/w/cpp/language/operator_precedence says about ternary operator:
+    //       "The expression in the middle of the conditional operator (between ? and :) is parsed as if parenthesized: its precedence relative to ?: is ignored."
+    // The AST parser relies on this function to add such parantheses where necessary.
+    for (Token* tok = list.front(); tok; tok = tok->next()) {
+        if (tok->str() == "?") {
+            bool paranthesesNeeded = false;
+            unsigned int depth = 0;
+            Token* tok2 = tok->next();
+            for (; tok2; tok2 = tok2->next()) {
+                if (tok2->link() && Token::Match(tok2, "{|[|("))
+                    tok2 = tok2->link();
+                else if (tok2->str() == ":") {
+                    if (depth == 0)
+                        break;
+                    depth--;
+                } else if (tok2->str() == ";" || tok2->link())
+                    break;
+                else if (tok2->str() == ",")
+                    paranthesesNeeded = true;
+                else if (tok2->str() == "?") {
+                    depth++;
+                    paranthesesNeeded = true;
+                }
+            }
+            if (paranthesesNeeded && tok2 && tok2->str() == ":") {
+                tok->insertToken("(");
+                tok2->insertToken(")", true);
+                Token::createMutualLinks(tok->next(), tok2->previous());
+            }
+        }
+    }
+}
+
 void Tokenizer::reportError(const Token* tok, const Severity::SeverityType severity, const std::string& id, const std::string& msg, bool inconclusive) const
 {
     const std::list<const Token*> callstack(1, tok);

lib/tokenize.h

@@ -357,11 +357,6 @@ public:
      */
     void simplifyTypedef();
 
-    /**
-     * Simplify float casts (float)1 => 1.0
-     */
-    void simplifyFloatCasts();
-
     /**
      * Simplify casts
      */
@@ -695,6 +690,11 @@ private:
      * */
     bool simplifyStrlen();
 
+    /**
+    * Prepare ternary operators with parantheses so that the AST can be created
+    * */
+    void prepareTernaryOpForAST();
+
     /**
      * check for duplicate enum definition
      */

lib/tokenlist.cpp

@@ -852,14 +852,15 @@ static void compileAssignTernary(Token *&tok, AST_state& state)
 {
     compileLogicOr(tok, state);
     while (tok) {
-        // TODO: http://en.cppreference.com/w/cpp/language/operator_precedence says:
-        //       "The expression in the middle of the conditional operator (between ? and :) is parsed as if parenthesized: its precedence relative to ?: is ignored."
         if (tok->isAssignmentOp()) {
             state.assign++;
             compileBinOp(tok, state, compileAssignTernary);
             if (state.assign > 0U)
                 state.assign--;
         } else if (tok->str() == "?") {
+            // http://en.cppreference.com/w/cpp/language/operator_precedence says about ternary operator:
+            //       "The expression in the middle of the conditional operator (between ? and :) is parsed as if parenthesized: its precedence relative to ?: is ignored."
+            // Hence, we rely on Tokenizer::prepareTernaryOpForAST() to add such parantheses where necessary.
             if (tok->strAt(1) == ":") {
                 state.op.push(0);
             }
@@ -895,7 +896,7 @@ static void compileExpression(Token *&tok, AST_state& state)
 
 static Token * createAstAtToken(Token *tok, bool cpp)
 {
-    if (Token::simpleMatch(tok,"for (")) {
+    if (Token::simpleMatch(tok, "for (")) {
         Token *tok2 = tok->tokAt(2);
         Token *init1 = nullptr;
         const Token * const endPar = tok->next()->link();

test/testgarbage.cpp

@@ -187,6 +187,9 @@ private:
         TEST_CASE(garbageCode135); // #4994
         TEST_CASE(garbageCode136); // #7033
         TEST_CASE(garbageCode137); // #7034
+        TEST_CASE(garbageCode138); // #6660
+        TEST_CASE(garbageCode139); // #6659
+        TEST_CASE(garbageCode140); // #7035
 
         TEST_CASE(garbageValueFlow);
         TEST_CASE(garbageSymbolDatabase);
@@ -361,8 +364,8 @@ private:
     }
 
     void garbageCode6() { // #5214
-        checkCode("int b = ( 0 ? ? ) 1 : 0 ;");
+        ASSERT_THROW(checkCode("int b = ( 0 ? ? ) 1 : 0 ;"), InternalError);
-        checkCode("int a = int b = ( 0 ? ? ) 1 : 0 ;");
+        ASSERT_THROW(checkCode("int a = int b = ( 0 ? ? ) 1 : 0 ;"), InternalError);
     }
 
     void garbageCode7() {
@@ -640,7 +643,7 @@ private:
     }
 
     void garbageCode56() { // #6713
-        checkCode("void foo() { int a = 0; int b = ???; }");
+        ASSERT_THROW(checkCode("void foo() { int a = 0; int b = ???; }"), InternalError);
     }
 
     void garbageCode57() { // #6731
@@ -923,10 +926,9 @@ private:
     }
 
     void garbageCode121() { // #2585
-        checkCode("abcdef?""?<"
+        ASSERT_THROW(checkCode("abcdef?""?<"
-                  "123456?""?>"
+                               "123456?""?>"
-                  "+?""?=");
+                               "+?""?="), InternalError);
-        ASSERT_EQUALS("", errout.str());
     }
 
     void garbageCode122() { // #6303
@@ -1083,6 +1085,30 @@ private:
         checkCode("\" \" typedef signed char f; \" \"; void a() { f * s = () &[]; (; ) (; ) }");
     }
 
+    void garbageCode138() { // #6660
+        checkCode("CS_PLUGIN_NAMESPACE_BEGIN(csparser)\n"
+                  "{\n"
+                  "    struct foo\n"
+                  "    {\n"
+                  "      union\n"
+                  "      {};\n"
+                  "    } halo;\n"
+                  "}\n"
+                  "CS_PLUGIN_NAMESPACE_END(csparser)");
+    }
+
+    void garbageCode139() { // #6659 heap user after free: kernel: sm750_accel.c
+        ASSERT_THROW(checkCode("void hw_copyarea() {\n"
+                               "   de_ctrl = (nDirection == RIGHT_TO_LEFT) ?\n"
+                               "    ( (0 & ~(((1 << (1 - (0 ? DE_CONTROL_DIRECTION))) - 1) << (0 ? DE_CONTROL_DIRECTION))) )\n"
+                               "    : 42;\n"
+                               "}"), InternalError);
+    }
+
+    void garbageCode140() { // #7035
+        ASSERT_THROW(checkCode("int foo(int align) { int off(= 0 % align;  return off) ? \\ align - off  :  0;  \\ }"), InternalError);
+    }
+
 
     void garbageValueFlow() {
         // #6089

test/testsimplifytokens.cpp

@@ -1881,13 +1881,11 @@ private:
         }
 
         {
-            const char code[] = "a ? b = c , d : e ;"; // do nothing
+            ASSERT_EQUALS("a ? ( b = c , d ) : e ;", tok("a ? b = c , d : e ;")); // Keep comma
-            ASSERT_EQUALS(code, tok(code));
         }
 
         {
-            const char code[] = "; return a ? b = c , d : e ;"; // do nothing
+            ASSERT_EQUALS("; return a ? ( b = c , d ) : e ;", tok("; return a ? b = c , d : e ;")); // Keep comma
-            ASSERT_EQUALS(code, tok(code));
         }
 
         {
@@ -1983,7 +1981,7 @@ private:
 
         {
             const char code[] = "void f () { switch(n) { case 1?0?1:0:foo(): break; }}";
-            ASSERT_EQUALS("void f ( ) { switch ( n ) { case 0 : ; break ; } }", tok(code));
+            TODO_ASSERT_EQUALS("void f ( ) { switch ( n ) { case 0 : ; break ; } }", "void f ( ) { switch ( n ) { case ( 0 ) : ; break ; } }", tok(code));
         }
 
         {

test/testtokenize.cpp

@@ -385,8 +385,6 @@ private:
         // a = b = 0;
         TEST_CASE(multipleAssignment);
 
-        TEST_CASE(setVarId) // #6660 - crash
-
         TEST_CASE(platformWin);
         TEST_CASE(platformWin32);
         TEST_CASE(platformWin32A);
@@ -442,6 +440,8 @@ private:
 
         TEST_CASE(compileLimits); // #5592 crash: gcc: testsuit: gcc.c-torture/compile/limits-declparen.c
 
+        TEST_CASE(prepareTernaryOpForAST);
+
         // AST data
         TEST_CASE(astexpr);
         TEST_CASE(astpar);
@@ -458,7 +458,6 @@ private:
         TEST_CASE(removeMacroInClassDef); // #6058
 
         TEST_CASE(sizeofAddParentheses);
-        TEST_CASE(incompleteTernary); // #6659
         TEST_CASE(noreturn); // #5783
     }
 
@@ -7923,6 +7922,15 @@ private:
                       tokenizeAndStringify("[[deprecated]] int f();", false, true, Settings::Unspecified, "test.c", true));
     }
 
+    void prepareTernaryOpForAST() {
+        ASSERT_EQUALS("a ? b : c ;", tokenizeAndStringify("a ? b : c;"));
+
+        ASSERT_EQUALS("a ? ( b , c ) : d ;", tokenizeAndStringify("a ? b , c : d;"));
+        ASSERT_EQUALS("a ? ( b , c ) : d ;", tokenizeAndStringify("a ? (b , c) : d;"));
+
+        ASSERT_EQUALS("a ? ( 1 ? ( a , b ) : 3 ) : d ;", tokenizeAndStringify("a ? 1 ? a, b : 3 : d;"));
+    }
+
     std::string testAst(const char code[],bool verbose=false) {
         // tokenize given code..
         Tokenizer tokenList(&settings0, nullptr);
@@ -7935,6 +7943,7 @@ private:
         tokenList.createLinks2();
 
         // Create AST..
+        tokenList.prepareTernaryOpForAST();
         tokenList.list.createAst();
 
         // Basic AST validation
@@ -7996,8 +8005,12 @@ private:
         // assignments are executed from right to left
         ASSERT_EQUALS("abc==", testAst("a=b=c;"));
 
-        // assignment in ternary operator
+        // ternary operator
         ASSERT_EQUALS("ab0=c1=:?", testAst("a?b=0:c=1;"));
+        ASSERT_EQUALS("fabc,d:?=e,", testAst("f = a ? b, c : d, e;"));
+        ASSERT_EQUALS("fabc,de,:?=", testAst("f = (a ? (b, c) : (d, e));"));
+        ASSERT_EQUALS("fabc,de,:?=", testAst("f = (a ? b, c : (d, e));"));
+        ASSERT_EQUALS("ab35,4:?foo(:?return", testAst("return (a ? b ? (3,5) : 4 : foo());"));
 
         ASSERT_EQUALS("a\"\"=", testAst("a=\"\""));
         ASSERT_EQUALS("a\'\'=", testAst("a=\'\'"));
@@ -8293,30 +8306,6 @@ private:
         ASSERT_EQUALS("f ( 0 , sizeof ( ptr . bar ) ) ;", tokenizeAndStringify("f(0, sizeof ptr->bar );"));
     }
 
-    void setVarId() {
-        const char * code = "CS_PLUGIN_NAMESPACE_BEGIN(csparser)\n"
-                            "{\n"
-                            "    struct foo\n"
-                            "    {\n"
-                            "      union\n"
-                            "      {};\n"
-                            "    } halo;\n"
-                            "}\n"
-                            "CS_PLUGIN_NAMESPACE_END(csparser)\n";
-        tokenizeAndStringify(code, true);
-    }
-
-    // #6659 heap user after free: kernel: sm750_accel.c
-    void incompleteTernary() {
-        const char * code = "void hw_copyarea() {\n"
-                            "   de_ctrl = (nDirection == RIGHT_TO_LEFT) ?\n"
-                            "    ( (0 & ~(((1 << (1 - (0 ? DE_CONTROL_DIRECTION))) - 1) << (0 ? DE_CONTROL_DIRECTION))) )\n"
-                            "    : 42;\n"
-                            "}";
-
-        tokenizeAndStringify(code, true);
-    }
-
     // see #5783
     void noreturn() {
         const char code[] = "void myassert() {\n"