tinycc-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Tinycc-devel] [ RFC] About packing two infos in a long


From: Thomas Preud'homme
Subject: Re: [Tinycc-devel] [ RFC] About packing two infos in a long
Date: Tue, 6 Nov 2012 16:37:21 +0100
User-agent: KMail/1.13.7 (Linux/3.2.0-4-amd64; KDE/4.8.4; x86_64; ; )

Le mardi 6 novembre 2012 16:22:00, Milutin Jovanović a écrit :
> The primary thing I don't like is that you put additional work on the user
> of got_offsets, to always have to mask the main value to both get and set
> it. If this could be encapsulated through accessors functions, or
> bitfields, then this would be much less of an issue.

Sure, I agree I don't like it. It create a risk of forgetting it for the 
future. But accessors or bitfield doesn't change anything. Bitfield needs to 
shift the value to the left for each use and accessors need to be used for 
each case. Someone reading tcc.h will be tempted to use the field immediately. 
At least bitfield makes it more clear to the compiler what's going on but it 
will not catch a mistake such has reading the value directly from got_offset.

> 
> Small stylistic issue is that I (personally) don't like '& -2' mask. I
> think this obfuscates the intent, and the simple '& ~1' is more usual, and
> it should not be any slower. -2 also assumes two's complement
> implementations...

Right, changed. I did it because of too much time spent reading arm-gen.c 
recently. Also, I tried to be consistant.

> 
> I can only speak as someone who is trying to get into the project. tinycc
> came from 'obfusticated' competition, and is already quite complicated and
> not particularly well commented/documented. This complexity has already
> stopped me from contributing when I wanted to add mach-o executable
> support. I would personally prefer the project to go in the
> 'un-obfusticated' direction...

Yeah, my own pet peeve is all the use of hex value to output code.

> 
> Miki.

Thanks for your review.

Attachment: signature.asc
Description: This is a digitally signed message part.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]