Re: [Tinycc-devel] [PATCH] Check for duplicate definitions of variables

[Marc Andre Tanner]

Hi,

Joshua Phillips wrote:
> oops, slight mistake, sorry. Here is replacement patch.
> ----------
> I was going to send this as a high-tech Mercurial bundle, but then I
> realized I'd transplanted the parent, so it was a bit difficult.
> Here's a low-tech patch file instead.

I think it is generally a good idea to send patches to the mailing list
so other can take a look at it.

Furthermore while i think it is worthwhile to have some error checking,
this shouldn't affect compile time if possible. I think we should focus
on getting working code to compile with tcc as opposed to reject *all*
possible wrong constructs. But this is of course only my personal opinion.

>     Check for duplicate definitions of variables
>         Added global "Sym *local_compound" which is a pointer into 
the local

>     symbol stack to mark the start of the current compound statement.
>         The following are valid:
>     int a, a; /* in a global context */
>     int a; { int a; } /* in a function */
>         The following now throw errors:
>     int a, a; /* in a function */
>     void foo(int a, int a);
>     enum { a, a };
>     union { int a; int a; };
>     struct { int a; struct { int a; }; };

The last thing is a bit problematic if you ask me because the following
compiles with gcc and prints 5.

struct foo {
    int a;
    struct {
        int a;
    };
};

static struct foo f = {
        5 , { .a = 10 }
};

int main(){
        printf("%d\n",f.a);
        return 0;
}

It is however not correct standard C if you try to compile it with -ansi
-std=c99 you will get an error. Note that tcc currently has a bug with
this kind of unnamed struct initialization[1,2].

Back to your patch, is there a special reason behind returning -1
instead of 1 in check_field if something is found?

Also a cosmetic remark, is_global could be written as below, no?

+static inline int is_global(Sym *s)
+{
+    return ((s->r & (VT_VALMASK | VT_LVAL)) == (VT_CONST | VT_LVAL))
+}

A quick test showed that there are some false positives. I didn't
investigate further. Below is a test case.

cat > demo.c << EOF
#include <gconv.h>
EOF
tcc -c demo.c
In file included from demo.c:1
/usr/include/gconv.h:70 redefinition of parameter ''

One last thing, against which version did you create your patch? Rob
split post_type() into parse_function_parameters() and
parse_array_dimensions()[3] which causes your patch to fail.

Regards,
Marc

[1] http://lists.gnu.org/archive/html/tinycc-devel/2007-09/msg00165.html
[2] http://lists.gnu.org/archive/html/tinycc-devel/2007-10/msg00046.html
[3] http://www.landley.net/hg/tinycc/rev/41841f0acc48

--
 Marc Andre Tanner >< http://www.brain-dump.org/ >< GPG key: CF7D56C0