| 1 |
On Mon, Jun 27, 2011 at 1:41 PM, Sebastian Pipping <sping@g.o> wrote: |
| 2 |
> On 06/27/2011 07:15 AM, Matt Turner wrote: |
| 3 |
>> class arch_mips(generic): |
| 4 |
>> "MIPS class" |
| 5 |
>> def __init__(self, Olevel, arch, additional_cflags, include_workarounds): |
| 6 |
>> generic.__init__(self) |
| 7 |
>> |
| 8 |
>> self.settings["CFLAGS"] = "-O" + Olevel |
| 9 |
>> self.settings["CFLAGS"] += " -march=" + arch |
| 10 |
>> if additional_cflags != "": |
| 11 |
>> self.settings["CFLAGS"] += " " + additional_cflags |
| 12 |
>> if include_workarounds: |
| 13 |
>> if arch == "mips3": |
| 14 |
>> self.settings["CFLAGS"] += " -mfix-r4000 -mfix-r4400" |
| 15 |
>> elif arch == "r4000" or arch == "r4k": |
| 16 |
>> self.settings["CFLAGS"] += " -mfix-r4000" |
| 17 |
>> elif arch == "r4300": |
| 18 |
>> self.settings["CFLAGS"] += " -mfix-r4300" |
| 19 |
>> elif arch == "r10000" or arch == "r10k": |
| 20 |
>> self.settings["CFLAGS"] += " -mfix-r10000" |
| 21 |
>> self.settings["CFLAGS"] += " -pipe" |
| 22 |
> |
| 23 |
> Thoughts: |
| 24 |
> |
| 25 |
> - How are you going to ensure that such refactoring keeps all |
| 26 |
> ~50 cases working without writing 50 explicit, data-duplicating |
| 27 |
> test cases? Would you be willing to write these? |
| 28 |
|
| 29 |
There seems to be an implicit assumption that the current code has |
| 30 |
some kind of working test cases. :) |
| 31 |
|
| 32 |
This is certainly not the case. Let me be clear, mistakes in the |
| 33 |
current code come from having the same CFLAG, CHOST, etc strings |
| 34 |
duplicated in many places. Refactoring the code would allow us to |
| 35 |
catch mistakes like |
| 36 |
http://git.overlays.gentoo.org/gitweb/?p=proj/catalyst.git;a=commit;h=db4323146ce27362948de6eab57e1dbe28240bde |
| 37 |
much more quickly. |
| 38 |
|
| 39 |
It seems to me that test coverage would be much simpler if the classes |
| 40 |
were refactored, since various combinations would use nearly identical |
| 41 |
code paths. |
| 42 |
|
| 43 |
> - The code above adds flexibility but is less obvious than |
| 44 |
> the current code. So while it improves on one aspect |
| 45 |
> it worsens on another. |
| 46 |
|
| 47 |
Perhaps... I'm not sure I'd agree. I'm not sure 24 classes with only |
| 48 |
slight variations between them lends itself to being obvious. |
| 49 |
|
| 50 |
> - Such refactoring would have to be done on both 2.x and 3.x branches. |
| 51 |
> Better wait until we are clear on their future. |
| 52 |
|
| 53 |
Yes, of course. |
| 54 |
|
| 55 |
Thanks, |
| 56 |
Matt |
Archives