m-labs / artiq

A leading-edge control system for quantum information experiments
https://m-labs.hk/artiq
GNU Lesser General Public License v3.0
422 stars 193 forks source link

'With' statement with multiple items has incorrect behavior #1478

Open lriesebos opened 4 years ago

lriesebos commented 4 years ago

Bug Report

One-Line Summary

When using a with statement with multiple items, the behavior is incorrect.

Issue Details

Steps to Reproduce

from artiq.experiment import *

class MyContext:
    def __init__(self):
        self.count = 0

    @portable
    def __enter__(self):
        print('enter')
        self.count += 1

    @portable
    def __exit__(self, exc_type, exc_val, exc_tb):
        print('exit')
        self.count -= 1

class MinimalContextExperiment(EnvExperiment):

    def build(self):
        self.setattr_device('core')
        self.context_a = MyContext()
        self.context_b = MyContext()

    @kernel
    def run(self):
        with self.context_a:
            with self.context_b:
                print('in both context, multiple with statements')
        print(self.context_a.count)
        print(self.context_b.count)

        with self.context_a, self.context_b:
            print('in both context, single with statement')
        print(self.context_a.count)
        print(self.context_b.count)

Expected Behavior

The first two nested with statements should be semantically equivalent to the last with statement with two items. Therefore, the printed results should also be the same (two times a 0).

Actual (undesired) Behavior

Exit functions are called incorrectly:

INFO:worker(2472,minimal_context_experiment.py):print:enter
INFO:worker(2472,minimal_context_experiment.py):print:enter
INFO:worker(2472,minimal_context_experiment.py):print:in both context, multiple with statements
INFO:worker(2472,minimal_context_experiment.py):print:exit
INFO:worker(2472,minimal_context_experiment.py):print:exit
INFO:worker(2472,minimal_context_experiment.py):print:0
INFO:worker(2472,minimal_context_experiment.py):print:0
INFO:worker(2472,minimal_context_experiment.py):print:enter
INFO:worker(2472,minimal_context_experiment.py):print:enter
INFO:worker(2472,minimal_context_experiment.py):print:in both context, single with statement
INFO:worker(2472,minimal_context_experiment.py):print:exit
INFO:worker(2472,minimal_context_experiment.py):print:exit
INFO:worker(2472,minimal_context_experiment.py):print:1
INFO:worker(2472,minimal_context_experiment.py):print:-1

Your System (omit irrelevant parts)

sbourdeauducq commented 3 years ago

Reverted due to #1547. We should also have regression tests for all these things...

lriesebos commented 3 years ago

I am not sure if a fix is planned, but an alternative could be to throw a compiler exception when using a with statement with multiple contexts, basically not supporting the syntax. That will at least prevent silent incorrect behaviour. Unfortunately I am not familiar enough with the ARTIQ compiler at this moment to propose a coded solution myself.