tinycc-devel
[Top][All Lists]
Advanced

[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.

reply via email to

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