C++: Improve compileTimeConstantInt

It is possible for the frontend to make
    (bool)e
where e has a constant value 0, but the (implicit) cast has no constant
value. This was causing us to not understand assume(0) correctly.

Now compileTimeConstantInt will handle casts itself if necessary.

cpp/ql/src/semmle/code/cpp/controlflow/internal/ConstantExprs.qll

@@ -279,20 +279,62 @@ private predicate reachableRecursive(ControlFlowNode n) {
   reachableRecursive(n.getAPredecessor())
 }
 
+/** Holds if `e` is a compile time constant with integer value `val`. */
 private predicate compileTimeConstantInt(Expr e, int val) {
-  val = e.getFullyConverted().getValue().toInt() and
-  not e instanceof StringLiteral and
-  not exists(Expr e1 | e1.getConversion() = e) // only values for fully converted expressions
+  (
+    // If we have an integer value then we are done.
+    if exists(e.getValue().toInt())
+    then val = e.getValue().toInt()
+    else
+      // Otherwise, if we are a conversion of another expression with an
+      // integer value, and that value can be converted into our type,
+      // then we have that value.
+      exists(Expr x, int valx |
+        x.getConversion() = e and
+        compileTimeConstantInt(x, valx) and
+        val = convertIntToType(valx, e.getType().getUnspecifiedType())
+      )
+  ) and
+  // If our unconverted expression is a string literal `"123"`, then we
+  // do not have integer value `123`.
+  not e.getUnconverted() instanceof StringLiteral
 }
 
-library class CompileTimeConstantInt extends Expr {
-  CompileTimeConstantInt() { compileTimeConstantInt(this, _) }
+/**
+ * Get `val` represented as type `t`, if that is possible without
+ * overflow or underflows.
+ */
+bindingset[val, t]
+private int convertIntToType(int val, IntegralType t) {
+  if t instanceof BoolType
+  then if val = 0 then result = 0 else result = 1
+  else
+    if t.isUnsigned()
+    then if val >= 0 and val.bitShiftRight(t.getSize() * 8) = 0 then result = val else none()
+    else
+      if val >= 0 and val.bitShiftRight(t.getSize() * 8 - 1) = 0
+      then result = val
+      else
+        if (-(val + 1)).bitShiftRight(t.getSize() * 8 - 1) = 0
+        then result = val
+        else none()
+}
 
-  int getIntValue() { compileTimeConstantInt(this, result) }
+/**
+ * INTERNAL: Do not use.
+ * An expression that has been found to have an integer value at compile
+ * time.
+ */
+class CompileTimeConstantInt extends Expr {
+  int val;
+
+  CompileTimeConstantInt() { compileTimeConstantInt(this.getFullyConverted(), val) }
+
+  int getIntValue() { result = val }
 }
 
 library class CompileTimeVariableExpr extends Expr {
-  CompileTimeVariableExpr() { not compileTimeConstantInt(this, _) }
+  CompileTimeVariableExpr() { not this instanceof CompileTimeConstantInt }
 }
 
 /** A helper class for evaluation of expressions. */

cpp/ql/test/library-tests/controlflow/assume/cfg.expected

@@ -1,29 +1,29 @@
 | simple | f | 2 | 1 | simple.cpp:2:6:2:6 | f | <none> |
 | simple | g | 3 | 1 | simple.cpp:3:6:3:6 | g | <none> |
-| simple | h | 4 | 4 | simple.cpp:4:6:4:6 | h | <none> |
+| simple | h | 4 | 9 | simple.cpp:4:6:4:6 | h | <none> |
 | simple | h | 5 | 1 | simple.cpp:5:1:11:1 | { ... } | ExprStmt |
 | simple | h | 6 | 2 | simple.cpp:6:2:6:5 | ExprStmt | call to f |
 | simple | h | 6 | 3 | simple.cpp:6:2:6:2 | call to f | ExprStmt |
 | simple | h | 8 | 1 | simple.cpp:8:11:8:11 | 0 | <none> |
 | simple | h | 8 | 1 | simple.cpp:8:11:8:11 | (bool)... | <none> |
 | simple | h | 8 | 4 | simple.cpp:8:2:8:13 | ExprStmt | __assume(...) |
-| simple | h | 8 | 5 | simple.cpp:8:2:8:12 | __assume(...) | <none> |
-| simple | h | 10 | 1 | simple.cpp:10:2:10:5 | ExprStmt | call to g |
-| simple | h | 10 | 2 | simple.cpp:10:2:10:2 | call to g | return ... |
-| simple | h | 11 | 3 | simple.cpp:11:1:11:1 | return ... | h |
+| simple | h | 8 | 5 | simple.cpp:8:2:8:12 | __assume(...) | ExprStmt |
+| simple | h | 10 | 6 | simple.cpp:10:2:10:5 | ExprStmt | call to g |
+| simple | h | 10 | 7 | simple.cpp:10:2:10:2 | call to g | return ... |
+| simple | h | 11 | 8 | simple.cpp:11:1:11:1 | return ... | h |
 | simple | i | 12 | 1 | simple.cpp:12:6:12:6 | i | <none> |
 | simple | j | 13 | 1 | simple.cpp:13:6:13:6 | j | <none> |
-| simple | k | 14 | 4 | simple.cpp:14:6:14:6 | k | <none> |
+| simple | k | 14 | 9 | simple.cpp:14:6:14:6 | k | <none> |
 | simple | k | 15 | 1 | simple.cpp:15:1:19:1 | { ... } | ExprStmt |
 | simple | k | 16 | 2 | simple.cpp:16:2:16:5 | ExprStmt | call to i |
 | simple | k | 16 | 3 | simple.cpp:16:2:16:2 | call to i | ExprStmt |
 | simple | k | 17 | 4 | simple.cpp:17:2:17:5 | ExprStmt | call to h |
-| simple | k | 17 | 5 | simple.cpp:17:2:17:2 | call to h | <none> |
-| simple | k | 18 | 1 | simple.cpp:18:2:18:5 | ExprStmt | call to j |
-| simple | k | 18 | 2 | simple.cpp:18:2:18:2 | call to j | return ... |
-| simple | k | 19 | 3 | simple.cpp:19:1:19:1 | return ... | k |
+| simple | k | 17 | 5 | simple.cpp:17:2:17:2 | call to h | ExprStmt |
+| simple | k | 18 | 6 | simple.cpp:18:2:18:5 | ExprStmt | call to j |
+| simple | k | 18 | 7 | simple.cpp:18:2:18:2 | call to j | return ... |
+| simple | k | 19 | 8 | simple.cpp:19:1:19:1 | return ... | k |
 | switch | doThing | 2 | 1 | switch.cpp:2:6:2:12 | doThing | <none> |
-| switch | switchTest1 | 4 | 37 | switch.cpp:4:6:4:16 | switchTest1 | <none> |
+| switch | switchTest1 | 4 | 40 | switch.cpp:4:6:4:16 | switchTest1 | <none> |
 | switch | switchTest1 | 5 | 1 | switch.cpp:5:1:30:1 | { ... } | declaration |
 | switch | switchTest1 | 6 | 2 | switch.cpp:6:2:6:10 | declaration | declaration |
 | switch | switchTest1 | 7 | 3 | switch.cpp:7:2:7:10 | declaration | switch (...) ...  |
@@ -63,11 +63,11 @@
 | switch | switchTest1 | 21 | 1 | switch.cpp:21:13:21:13 | 0 | <none> |
 | switch | switchTest1 | 21 | 1 | switch.cpp:21:13:21:13 | (bool)... | <none> |
 | switch | switchTest1 | 21 | 8 | switch.cpp:21:4:21:15 | ExprStmt | __assume(...) |
-| switch | switchTest1 | 21 | 9 | switch.cpp:21:4:21:14 | __assume(...) | <none> |
-| switch | switchTest1 | 29 | 1 | switch.cpp:29:2:29:14 | ExprStmt | ptr |
-| switch | switchTest1 | 29 | 2 | switch.cpp:29:10:29:12 | ptr | call to doThing |
-| switch | switchTest1 | 29 | 3 | switch.cpp:29:2:29:8 | call to doThing | return ... |
-| switch | switchTest1 | 30 | 4 | switch.cpp:30:1:30:1 | return ... | switchTest1 |
+| switch | switchTest1 | 21 | 9 | switch.cpp:21:4:21:14 | __assume(...) | ExprStmt |
+| switch | switchTest1 | 29 | 10 | switch.cpp:29:2:29:14 | ExprStmt | ptr |
+| switch | switchTest1 | 29 | 11 | switch.cpp:29:10:29:12 | ptr | call to doThing |
+| switch | switchTest1 | 29 | 12 | switch.cpp:29:2:29:8 | call to doThing | return ... |
+| switch | switchTest1 | 30 | 13 | switch.cpp:30:1:30:1 | return ... | switchTest1 |
 | switch | switchTest2 | 32 | 37 | switch.cpp:32:6:32:16 | switchTest2 | <none> |
 | switch | switchTest2 | 33 | 1 | switch.cpp:33:1:51:1 | { ... } | declaration |
 | switch | switchTest2 | 34 | 2 | switch.cpp:34:2:34:10 | declaration | declaration |

cpp/ql/test/library-tests/controlflow/assume/interesting.expected

@@ -1,15 +1,15 @@
 | simple.cpp:6:2:6:2 | call to f | 1 | 1 | reachable |  |
-| simple.cpp:8:2:8:12 | __assume(...) | 1 | 0 | reachable |  |
-| simple.cpp:10:2:10:2 | call to g | 1 | 1 |  |  |
+| simple.cpp:8:2:8:12 | __assume(...) | 1 | 1 | reachable |  |
+| simple.cpp:10:2:10:2 | call to g | 1 | 1 | reachable |  |
 | simple.cpp:16:2:16:2 | call to i | 1 | 1 | reachable |  |
-| simple.cpp:17:2:17:2 | call to h | 1 | 0 | reachable |  |
-| simple.cpp:18:2:18:2 | call to j | 1 | 1 |  |  |
+| simple.cpp:17:2:17:2 | call to h | 1 | 1 | reachable |  |
+| simple.cpp:18:2:18:2 | call to j | 1 | 1 | reachable |  |
 | switch.cpp:11:4:11:10 | call to doThing | 1 | 1 | reachable | unassigned ptr |
 | switch.cpp:13:4:13:10 | call to doThing | 1 | 1 | reachable |  |
 | switch.cpp:16:4:16:10 | call to doThing | 1 | 1 | reachable | unassigned ptr |
 | switch.cpp:18:4:18:10 | call to doThing | 1 | 1 | reachable |  |
-| switch.cpp:21:4:21:14 | __assume(...) | 1 | 0 | reachable | unassigned ptr |
-| switch.cpp:29:2:29:8 | call to doThing | 1 | 1 |  |  |
+| switch.cpp:21:4:21:14 | __assume(...) | 1 | 1 | reachable | unassigned ptr |
+| switch.cpp:29:2:29:8 | call to doThing | 1 | 1 | reachable | unassigned ptr |
 | switch.cpp:39:4:39:10 | call to doThing | 1 | 1 | reachable | unassigned ptr |
 | switch.cpp:41:4:41:10 | call to doThing | 1 | 1 | reachable |  |
 | switch.cpp:44:4:44:10 | call to doThing | 1 | 1 | reachable | unassigned ptr |

cpp/ql/test/library-tests/opts/options.expected

@@ -3,6 +3,5 @@
 | main.cpp:12:13:12:19 | call to Xstrdup | overrideReturnsNull |
 | main.cpp:13:13:13:19 | call to Xstrdup | overrideReturnsNull |
 | main.cpp:13:13:13:19 | call to Xstrdup | returnsNull |
-| main.cpp:19:3:19:13 | __assume(...) | exprExits |
 | main.cpp:30:2:30:22 | call to ignore_return_value | okToIgnoreReturnValue |
 | main.cpp:32:2:32:20 | call to myIgnoreReturnValue | okToIgnoreReturnValue |

cpp/ql/test/library-tests/special_members/generated_copy/functions.expected

@@ -36,7 +36,6 @@
 | copy.cpp:59:9:59:9 | operator= |  |  |
 | copy.cpp:67:9:67:9 | Wrapper |  |  |
 | copy.cpp:67:9:67:9 | Wrapper | deleted |  |
-| copy.cpp:67:9:67:9 | Wrapper | deleted |  |
 | copy.cpp:67:9:67:9 | operator= |  |  |
 | copy.cpp:67:9:67:9 | operator= |  |  |
 | copy.cpp:67:9:67:9 | operator= | deleted |  |
@@ -44,19 +43,16 @@
 | copy.cpp:71:9:71:9 | operator= |  |  |
 | copy.cpp:72:9:72:9 | NotCopyable |  |  |
 | copy.cpp:72:9:72:9 | NotCopyable | deleted |  |
-| copy.cpp:72:9:72:9 | NotCopyable | deleted |  |
 | copy.cpp:72:9:72:9 | operator= | deleted |  |
 | copy.cpp:76:9:76:9 | operator= |  |  |
 | copy.cpp:76:9:76:9 | operator= |  |  |
 | copy.cpp:80:9:80:9 | NotCopyableComposition |  |  |
 | copy.cpp:80:9:80:9 | NotCopyableComposition | deleted |  |
-| copy.cpp:80:9:80:9 | NotCopyableComposition | deleted |  |
 | copy.cpp:80:9:80:9 | operator= | deleted |  |
 | copy.cpp:84:9:84:9 | operator= |  |  |
 | copy.cpp:84:9:84:9 | operator= |  |  |
 | copy.cpp:86:9:86:9 | NotCopyableInheritance |  |  |
 | copy.cpp:86:9:86:9 | NotCopyableInheritance | deleted |  |
-| copy.cpp:86:9:86:9 | NotCopyableInheritance | deleted |  |
 | copy.cpp:86:9:86:9 | operator= | deleted |  |
 | copy.cpp:90:9:90:9 | operator= |  |  |
 | copy.cpp:90:9:90:9 | operator= |  |  |
@@ -72,9 +68,6 @@
 | copy.cpp:111:9:111:9 | MoveAssign | deleted |  |
 | copy.cpp:111:9:111:9 | operator= | deleted |  |
 | copy.cpp:113:17:113:25 | operator= |  |  |
-| copy.cpp:120:9:120:9 | OnlyCtor |  |  |
-| copy.cpp:120:9:120:9 | OnlyCtor |  |  |
-| copy.cpp:120:9:120:9 | OnlyCtor | deleted |  |
 | copy.cpp:120:9:120:9 | operator= | deleted |  |
 | copy.cpp:126:11:126:19 | operator= |  |  |
 | copy.cpp:128:5:128:8 | Base |  |  |

cpp/ql/test/library-tests/syntax-zoo/dataflow-ir-consistency.expected

@@ -539,8 +539,6 @@ uniqueNodeLocation
 | file://:0:0:0:0 | p#0 | Node should have one location but has 0. |
 | file://:0:0:0:0 | p#0 | Node should have one location but has 0. |
 | file://:0:0:0:0 | p#0 | Node should have one location but has 0. |
-| file://:0:0:0:0 | p#0 | Node should have one location but has 0. |
-| file://:0:0:0:0 | p#0 | Node should have one location but has 0. |
 | file://:0:0:0:0 | p#1 | Node should have one location but has 0. |
 | file://:0:0:0:0 | p#1 | Node should have one location but has 0. |
 | file://:0:0:0:0 | p#1 | Node should have one location but has 0. |
@@ -1418,7 +1416,7 @@ uniqueNodeLocation
 | whilestmt.c:39:6:39:11 | ReturnVoid | Node should have one location but has 4. |
 | whilestmt.c:39:6:39:11 | SideEffect | Node should have one location but has 4. |
 missingLocation
-| Nodes without location: 36 |
+| Nodes without location: 34 |
 uniqueNodeToString
 | break_labels.c:2:11:2:11 | i | Node should have one toString but has 2. |
 | break_labels.c:2:11:2:11 | i | Node should have one toString but has 2. |