[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Tinycc-devel] Fixes to bugs in `type_to_str` and `compare_types`; p
From: |
Michael Matz |
Subject: |
Re: [Tinycc-devel] Fixes to bugs in `type_to_str` and `compare_types`; patches to CLI options |
Date: |
Mon, 12 Nov 2018 19:10:48 +0000 (UTC) |
User-agent: |
Alpine 2.21 (LSU 202 2017-01-01) |
Hello,
On Mon, 12 Nov 2018, Petr Skočík wrote:
> My shot at the fixes is at https://github.com/pskocik/tinycc. It's
> separated into addition of failing tests and fixes to those tests.
The fixes seem sensible, but I have some stylistic nits:
* please add testcases and fixes for them in the same commit, or the
testcases after the fixes (so that there are no revisions that have
known fails, which eases bisecting), in this case probably in the same
commit
* if you're using <TAB> characters in your changes, please make sure your
editor has set tabstops at eight characters; as is one of your changes
looks like this (tabs replaced by spaces to demonstrate):
}else if(bt1 & VT_ARRAY){
return type1->ref->c < 0 || type2->ref->c < 0
|| type1->ref->c == type2->ref->c;
} else if (bt1 == VT_STRUCT) {
return (type1->ref == type2->ref);
Also respect the surrounding style: space between '}' and 'else' (and
not a <tab> or nothing).
* one change you had was:
if(varstr && '*'== *varstr){
Surrounding style has spaces around comparisons. (the '42 == x' style
is peculiar as well; I know the reasoning behind it, and in that
function there's precedent so I won't _really_ complain ;) )
> I've also thrown in a separate patch that allows `tcc -o -` to be
> treated as "write output to stdout".
That seems fine.
> (I had problems with -o /dev/stdout on some Cygwin IIRC) and another one
> that makes libc accept `-l lib` in addition
> to `-llib` (because gcc/clang do and POSIX specifically wants it).
And that as well, even though my first reaction was "sigh, POSIX" ;)
So only really minor things, but at least a bit of consistency is nice to
have. I'd say commit to mob, after fixing the above.
Ciao,
Michael.