Re: [Simulavr-devel] [PATCH] use static callbacks instead of template param for IOReg

[panic]

ping? any comments?

- panic

panic:
> Michael Hennebry:
>> Would something like this help:
> [snip]
> 
> To me this looks a bit like reinventing std::function/std::bind.
> 
> But I'd do something similar to your proposal, using std::function that
> was added in C++11. GCC in current Debian stable is 6.3.0. Since GCC/g++
> 6, -std= defaults to c++14, so the feature is available for free:
> 
> http://en.cppreference.com/w/cpp/utility/functional
> 
> 
> Step 1:
> ------
> In IOReg class remove the template parameter and change the
> getter_t/setter_t to:
> 
>   typedef std::function<unsigned char(void)> getter_t;
>   typedef std::function<void(unsigned char)> setter_t;
> 
> 
> Step 2:
> ------
> Peripherals then need to be updated like this:
>     ddr_reg(this, "DDR",
> -           this, &HWPort::GetDdr, &HWPort::SetDdr),
> +           std::bind(&HWPort::GetDdr, this),
> +           std::bind(&HWPort::SetDdr, this, std::placeholders::_1))
> 
> Up to here, I've already implemented it. Testsuite runs through mostly
> fine*. A patch is attached.
> 
> Step 3:
> ------
> Add a set_bit(unsigned char bitpos, bool val) to IOReg, provide RMW
> default implementation if no specific callback for Set_bit was given.
> Then fix the insn decoder in AvrDevice.
> 
> 
> Discussion:
> ----------
> The syntax could be simpified if the IOReg itself (or RWMemoryMember)
> handled their unsigned char value themselves, instead of being a wrapper
> to get/set callbacks. We could then change the implementation of IOReg
> to just trigger "OnWrite" or "OnRead" callbacks that don't need any
> parameters, thus, we could omit the std::placeholders::_1.
> The IOReg/RWMemoryMember would then also make sure that the tracers are
> updated, which should avoid bugs related to wrong/forgotten tracing as
> it already happend multiple times.
> 
> 
> Cheers,
> panic
> 
> 
> *:
> I'm getting a testsuite error with the normal HEAD that also appears
> with my patch. To me, the test case seems to have a one-off issue. When
> I add round() to the testcase, they run fine:
> 
>> ----------------------- regress/modtest/adc_diff_t25.py 
>> -----------------------
>> @@@ -46,7 -46,7 +46,7 @@@ class TestCase(SimTestCase)
>>      else:
>>        rng = 512
>>      v = self.sim.getWordByName(self.dev, "adc_value")
>> -    e = int(((pValue - nValue) / refValue) * rng) & 0x3ff
>> +    e = int(round((pValue - nValue) / refValue)) * rng) & 0x3ff
>>      self.assertEqual(v, e, "expected adc value is 0x%x, got 0x%x" % (e, v))
>>  
>>    def test_00(self):
> 
> Since the upgrade to GCC 6, I see another testsuite error (already
> occurs when run on HEAD, independent of my changes):
> 
>> ======================================================================
>> FAIL: test_00 (eeprom.TestCase)
>> eeprom_atmega16::check read and write eeprom data
>> ----------------------------------------------------------------------
>> Traceback (most recent call last):
>>   File "eeprom.py", line 70, in test_00
>>     self.assertValue(0x66)
>>   File "eeprom.py", line 19, in assertValue
>>     self.assertComplete()
>>   File "eeprom.py", line 16, in assertComplete
>>     self.assertEqual(c, 1, "function isn't complete (complete=%d)" % c)
>> AssertionError: function isn't complete (complete=2)
>>
>> ======================================================================
>> FAIL: test_00 (eeprom.TestCase)
>> eeprom_atmega128::check read and write eeprom data
>> ----------------------------------------------------------------------
>> Traceback (most recent call last):
>>   File "eeprom.py", line 54, in test_00
>>     self.assertValue(0x33)
>>   File "eeprom.py", line 19, in assertValue
>>     self.assertComplete()
>>   File "eeprom.py", line 16, in assertComplete
>>     self.assertEqual(c, 1, "function isn't complete (complete=%d)" % c)
>> AssertionError: function isn't complete (complete=2)
>>
>> ----------------------------------------------------------------------
>>
>>
>> _______________________________________________
>> Simulavr-devel mailing list
>> address@hidden
>> https://lists.nongnu.org/mailman/listinfo/simulavr-devel