Discard reference -- SIMICS-21584

lwaern-intel · lwaern-intel · commit dce32e4119cf · 2023-11-30T13:18:05.000+01:00
diff --git a/RELEASENOTES-1.4.docu b/RELEASENOTES-1.4.docu
@@ -523,4 +523,14 @@
       <tt>--no-compat=shared_logs_on_device</tt> to DMLC.</add-note></build-id>
   <build-id _6="next" _7="next"><add-note> Improved the run-time performance
       of accesses to registers with many fields.</add-note></build-id>
+  <build-id _6="next" _7="next"><add-note> Added the <em>discard reference</em>
+      '<tt>_</tt>' &mdash; a non-value expression which may be used as an assign
+      target in order to explictly discard the result of an evaluated expression
+      or return value of a method call <bug number="SIMICS-21584"/>.
+      Example usage:
+      <pre>
+      _ = any_expression;
+      _ = throwing_method();
+      (_, x, _) = method_with_multiple_return_values();
+      </pre></add-note></build-id>
 </rn>
diff --git a/doc/1.4/language.md b/doc/1.4/language.md
@@ -3808,6 +3808,32 @@ independent method callback(int i, void *aux) {
 }
 ```
 
+### The Discard Reference (`_`)
+```
+_
+```
+
+The discard reference *`_`* is an expression without any run-time representation
+that may be used as the target of an assignment in order to explicitly discard
+the result of an evaluated expression or return value of a method call.
+
+For backwards compatibility reasons, `_` is not a keyword, but instead behaves
+more closely as a global identifier. What this means is that declared
+identifiers (e.g. local variables) are allowed to shadow it by being named `_`.
+
+Example usage:
+```
+// Evaluate an expression and explicitly discard its result.
+// Can be relevant to e.g. suppress Coverity's CHECKED_RETURN checker
+_ = nonthrowing_single_return_method();
+
+// Calls to methods that throw or have multiple return values require a target
+// for each return value. `_` can be used to discard return values not of
+// interest.
+_ = throwing_method();
+(_, x, _) = method_with_multiple_return_values();
+```
+
 ### New Expressions
 
 <pre>
diff --git a/py/dml/c_backend.py b/py/dml/c_backend.py
@@ -3116,12 +3116,7 @@ def generate_startup_trait_calls(data, idxvars):
         ref = ObjTraitRef(site, node, trait, indices)
         out(f'_tref = {ref.read()};\n')
         for method in trait_methods:
-            outargs = [mkLit(method.site,
-                             ('*((%s) {0})'
-                              % ((TArray(t, mkIntegerLiteral(method.site, 1))
-                                  .declaration('')),)),
-                             t)
-                       for (_, t) in method.outp]
+            outargs = [mkDiscardRef(method.site) for _ in method.outp]
 
             method_ref = TraitMethodDirect(
                 method.site, mkLit(method.site, '_tref', TTrait(trait)), method)
@@ -3133,12 +3128,7 @@ def generate_startup_trait_calls(data, idxvars):
 def generate_startup_regular_call(method, idxvars):
     site = method.site
     indices = tuple(mkLit(site, idx, TInt(32, False)) for idx in idxvars)
-    outargs = [mkLit(site,
-                     ('*((%s) {0})'
-                      % ((TArray(t, mkIntegerLiteral(site, 1))
-                          .declaration('')),)),
-                     t)
-               for (_, t) in method.outp]
+    outargs = [mkDiscardRef(method.site) for _ in method.outp]
     # startup memoized methods can throw, which is ignored during startup.
     # Memoization of the throw then allows for the user to check whether
     # or not the method did throw during startup by calling the method
diff --git a/py/dml/codegen.py b/py/dml/codegen.py
@@ -1207,6 +1207,13 @@ def expr_variable(tree, location, scope):
         if in_dev_tree:
             e = in_dev_tree
     if e is None:
+        # TODO/HACK: The discard ref is exposed like this to allow it to be as
+        # keyword-like as possible while still allowing it to be shadowed.
+        # Once we remove support for discard_ref_shadowing the discard ref
+        # should become a proper keyword and its codegen be done via dedicated
+        # dispatch
+        if name == '_' and tree.site.dml_version() != (1, 2):
+            return mkDiscardRef(tree.site)
         raise EIDENT(tree.site, name)
     return e
 
@@ -2341,14 +2348,25 @@ def try_codegen_invocation(site, init_ast, outargs, location, scope):
     else:
         return common_inline(site, meth_node, indices, inargs, outargs)
 
+def codegen_src_for_nonvalue_target(site, tgt, src_ast, location, scope):
+    if not tgt.writable:
+        raise EASSIGN(site, tgt)
+    if src_ast.kind != 'initializer_scalar':
+        raise EDATAINIT(tgt.site,
+                        f'{tgt} can only be used as the target '
+                        + 'of an assignment if its initializer is a '
+                        + 'simple expression or a return value of a '
+                        + 'method call')
+    return codegen_expression(src_ast.args[0], location, scope)
+
 @statement_dispatcher
 def stmt_assign(stmt, location, scope):
     (_, site, tgt_ast, src_asts) = stmt
     assert tgt_ast.kind in ('assign_target_chain', 'assign_target_tuple')
-    tgts = [codegen_expression(ast, location, scope)
+    tgts = [codegen_expression_maybe_nonvalue(ast, location, scope)
             for ast in tgt_ast.args[0]]
     for tgt in tgts:
-        if deep_const(tgt.ctype()):
+        if not isinstance(tgt, NonValue) and deep_const(tgt.ctype()):
             raise ECONST(tgt.site)
     if tgt_ast.kind == 'assign_target_chain':
         method_tgts = [tgts[0]]
@@ -2370,14 +2388,21 @@ def stmt_assign(stmt, location, scope):
                            + f'initializer: Expected {src_asts}, got 1'))
             return []
 
-        lscope = Symtab(scope)
+        if isinstance(tgts[-1], NonValue):
+            if len(tgts) != 1:
+                raise tgts[-1].exc()
+            expr = codegen_src_for_nonvalue_target(site, tgts[0], src_asts[0],
+                                                   location, scope)
+            return [mkCopyData(site, expr, tgts[0])]
+
         init_typ = tgts[-1].ctype()
         init = eval_initializer(
             tgts[-1].site, init_typ, src_asts[0], location, scope, False)
 
         if len(tgts) == 1:
             return [mkAssignStatement(tgts[0].site, tgts[0], init)]
 
+        lscope = Symtab(scope)
         sym = lscope.add_variable(
             'tmp', type=init_typ, site=init.site, init=init,
             stmt=True)
@@ -2406,22 +2431,27 @@ def stmt_assign(stmt, location, scope):
 
         stmts = []
         lscope = Symtab(scope)
-        syms = []
+        stmt_pairs = []
         for (i, (tgt, src_ast)) in enumerate(zip(tgts, src_asts)):
-            init = eval_initializer(site, tgt.ctype(), src_ast, location,
-                                    scope, False)
-            name = 'tmp%d' % (i,)
-            sym = lscope.add_variable(
-                    name, type=tgt.ctype(), site=tgt.site, init=init,
-                    stmt=True)
-            syms.append(sym)
-
-        stmts.extend(sym_declaration(sym) for sym in syms)
-        stmts.extend(
-            AssignStatement(
-                tgt.site, tgt,
-                ExpressionInitializer(mkLocalVariable(tgt.site, sym)))
-            for (tgt, sym) in zip(tgts, syms))
+            if isinstance(tgt, NonValue):
+                expr = codegen_src_for_nonvalue_target(site, tgt, src_ast,
+                                                       location, scope)
+                stmt_pairs.append((mkCopyData(tgt.site, expr, tgt), None))
+            else:
+                init = eval_initializer(site, tgt.ctype(), src_ast, location,
+                                        scope, False)
+                name = 'tmp%d' % (i,)
+                sym = lscope.add_variable(
+                        name, type=tgt.ctype(), site=tgt.site, init=init,
+                        stmt=True)
+                write = AssignStatement(
+                    tgt.site, tgt,
+                    ExpressionInitializer(mkLocalVariable(tgt.site, sym)))
+                stmt_pairs.append((sym_declaration(sym), write))
+
+        stmts.extend(first for (first, _) in stmt_pairs)
+        stmts.extend(second for (_, second) in stmt_pairs
+                     if second is not None)
         return [mkCompound(site, stmts)]
 
 @statement_dispatcher
@@ -3616,7 +3646,7 @@ def codegen_inline(site, meth_node, indices, inargs, outargs,
                                 parmtype if parmtype else arg.ctype(),
                                 meth_node.name)
                     for (arg, var, (parmname, parmtype)) in zip(
-                            outargs, outvars, meth_node.outp)] 
+                            outargs, outvars, meth_node.outp)]
             exit_handler = GotoExit_dml12()
             with exit_handler:
                 code = [codegen_statement(meth_node.astcode,
@@ -4040,15 +4070,20 @@ def copy_outarg(arg, var, parmname, parmtype, method_name):
     an exception. We would be able to skip the proxy variable for
     calls to non-throwing methods when arg.ctype() and parmtype are
     equivalent types, but we don't do this today.'''
-    argtype = arg.ctype()
-
-    if not argtype:
-        raise ICE(arg.site, "unknown expression type")
+    if isinstance(arg, NonValue):
+        if not arg.writable:
+            raise arg.exc()
     else:
-        ok, trunc, constviol = realtype(parmtype).canstore(realtype(argtype))
-        if not ok:
-            raise EARGT(arg.site, 'call', method_name,
-                         arg.ctype(), parmname, parmtype, 'output')
+        argtype = arg.ctype()
+
+        if not argtype:
+            raise ICE(arg.site, "unknown expression type")
+        else:
+            ok, trunc, constviol = realtype(parmtype).canstore(
+                realtype(argtype))
+            if not ok:
+                raise EARGT(arg.site, 'call', method_name,
+                             arg.ctype(), parmname, parmtype, 'output')
 
     return mkCopyData(var.site, var, arg)
 
diff --git a/py/dml/ctree.py b/py/dml/ctree.py
@@ -129,6 +129,7 @@
     'mkEachIn', 'EachIn',
     'mkBoolConstant',
     'mkUndefined', 'Undefined',
+    'mkDiscardRef',
     'TraitParameter',
     'TraitSessionRef',
     'TraitHookRef',
@@ -1100,14 +1101,21 @@ def mkAssignStatement(site, target, init):
     if not target.writable:
         raise EASSIGN(site, target)
 
-    target_type = target.ctype()
+    if isinstance(target, NonValue):
+        if not isinstance(init, ExpressionInitializer):
+            raise EDATAINIT(target.site,
+                            f'{target} can only be used as the target of an '
+                            + 'assignment if its initializer is a simple '
+                            + 'expression or a return value of a method call')
+    else:
+        target_type = target.ctype()
 
-    if deep_const(target_type):
-        raise ECONST(site)
+        if deep_const(target_type):
+            raise ECONST(site)
 
-    if isinstance(init, ExpressionInitializer):
-        init = ExpressionInitializer(
-            source_for_assignment(site, target_type, init.expr))
+        if isinstance(init, ExpressionInitializer):
+            init = ExpressionInitializer(
+                source_for_assignment(site, target_type, init.expr))
 
     return AssignStatement(site, target, init)
 
@@ -2514,7 +2522,7 @@ class AssignOp(BinOp):
     def __str__(self):
         return "%s = %s" % (self.lh, self.rh)
 
-    def discard(self):
+    def discard(self, explicit=False):
         return self.lh.write(ExpressionInitializer(self.rh))
 
     def read(self):
@@ -3539,6 +3547,18 @@ def exc(self):
 
 mkUndefined = Undefined
 
+class DiscardRef(NonValue):
+    writable = True
+
+    def __str__(self):
+        return '_'
+
+    def write(self, source):
+        assert isinstance(source, ExpressionInitializer)
+        return source.expr.discard(explicit=True)
+
+mkDiscardRef = DiscardRef
+
 def endian_convert_expr(site, idx, endian, size):
     """Convert a bit index to little-endian (lsb=0) numbering.
 
@@ -4758,8 +4778,8 @@ def ctype(self):
         return self.expr.ctype()
     def read(self):
         return self.expr.read()
-    def discard(self):
-        return self.expr.discard()
+    def discard(self, explicit=False):
+        return self.expr.discard(explicit)
     def incref(self):
         self.expr.incref()
     def decref(self):
@@ -4959,15 +4979,15 @@ def assign_to(self, dest, typ):
         rt = safe_realtype_shallow(typ)
         # There is a reasonable implementation for this case (memcpy), but it
         # never occurs today
-        assert not isinstance(typ, TArray)
+        assert not isinstance(rt, TArray)
         if isinstance(rt, TEndianInt):
             return (f'{rt.dmllib_fun("copy")}((void *)&{dest},'
                     + f' {self.expr.read()})')
         elif deep_const(typ):
             shallow_deconst_typ = safe_realtype_unconst(typ)
             # a const-qualified ExternStruct can be leveraged by the user as a
             # sign that there is some const-qualified member unknown to DMLC
-            if (isinstance(typ, TExternStruct)
+            if (isinstance(shallow_deconst_typ, TExternStruct)
                 or deep_const(shallow_deconst_typ)):
                 # Expression statement to delimit lifetime of compound literal
                 # TODO it's possible to improve the efficiency of this by not
diff --git a/py/dml/expr.py b/py/dml/expr.py
@@ -136,8 +136,16 @@ def read(self):
         raise ICE(self.site, "can't read %r" % self)
 
     # Produce a C expression but don't worry about the value.
-    def discard(self):
-        return self.read()
+    def discard(self, explicit=False):
+        if not explicit or safe_realtype_shallow(self.ctype()).void:
+            return self.read()
+
+        if self.constant:
+            return '(void)0'
+        from .ctree import Cast
+        expr = (f'({self.read()})'
+                if self.priority < Cast.priority else self.read())
+        return f'(void){expr}'
 
     def ctype(self):
         '''The corresponding DML type of this expression'''
diff --git a/test/1.4/expressions/T_discard_ref.dml b/test/1.4/expressions/T_discard_ref.dml
@@ -0,0 +1,46 @@
+/*
+  © 2023 Intel Corporation
+  SPDX-License-Identifier: MPL-2.0
+*/
+dml 1.4;
+device test;
+
+header %{
+    #define FUNCLIKE_MACRO() 4
+    #define VARLIKE_MACRO ++counter
+
+    static int counter = 0;
+%}
+
+extern int FUNCLIKE_MACRO(void);
+extern int VARLIKE_MACRO;
+extern int counter;
+
+method t() -> (int) throws {
+    return 1;
+}
+method m2() -> (int, int) {
+    return (1, 2);
+}
+
+method init() {
+    local int x;
+    // Explicit discard guarantees GCC doesn't emit -Wunused by always
+    // void-casting, unless the expression is already void
+    _ = x;
+    _ = FUNCLIKE_MACRO();
+    // Explicit discard does generate C, which evaluates the initializer
+    assert counter == 0;
+    _ = VARLIKE_MACRO;
+    assert counter == 1;
+    try
+        _ = t();
+    catch assert false;
+    (x, _) = m2();
+    assert x == 1;
+    local int y;
+    // Tuple initializers retain the property of each expression being
+    // evaluated left-to-right
+    (_, y) = (x++, x);
+    assert y == 2;
+}
