[Tinycc-devel] 回复: Request push

[jiang]

I'm sorry! Is I am too impatient. Thank you for your remind and Suggestions. 
I'll take the time to put a bug in greater detail.

Best regards,

jiang


Thomas Preud'homme <address@hidden>编写：

>Le lundi 30 juin 2014, 16:49:02 jiang a écrit :
>> Struct improved algorithms and add struct warnings and other warnings.
>
>Hi Jiang,
>
>> 
>> The following can be compiled:
>> struct st{int a;;;;;;;};
>> 
>> If you do not have advice, tomorrow pushed into the mob.
>
>No Jiang, that's not how it works. There is several problem with this request:
>
>1) You need to realize that we are all volunteer spending some of our free 
>time to improve tcc. We don't necessarily check emails about tcc everyday and 
>anyway 1 day is too short to review. Another consequence of this is that we 
>don't have to answer within a given delay. We will do our best to answer but 
>you just need to wait. Of course if too much time (like 2 weeks) pass without 
>answer you can ping to ask if we saw your email. And finally I think in your 
>case you should not commit without an approval first. Eventually, when your 
>commit will start to improve we might tell you that you don't need to ask for 
>a review anymore.
>
>2) I gave you some comments on another patch. Before asking for another review 
>you could maybe post a new version that fixes all my comments. It gives the 
>feeling that you don't want to address the remarks and discourages people to 
>review your patches.
>
>3) Don't bundle several changes in one patch. It makes it more difficult to 
>review. So you make one patch that fixes the bug you mentions and others (or 
>maybe just one other, it depends how big it is) with the warnings
>
>4) Your patch contains some formatting issues:
>
>+                        if (v == 0 && (type1.t & VT_BTYPE) != VT_STRUCT){
>+                            tcc_warning("declaration does not declare 
>anything");
>+                            break;
>+                        }if (type_size(&type1, &align) < 0) {
>+                            if ((a == TOK_STRUCT) && (type1.t & VT_ARRAY))
>+                                flexible = 1;
>+                            else
>+                                tcc_error("field '%s' has incomplete type", 
>get_tok_str(v, NULL));
>                         }
>
>There should be a space between the closing parenthesis ')' and the opening 
>curly brace '{'. The second "if" should be on a new line or should be preceded 
>by a "else"
>
>5) The commit message is not very informative. For the commit with the bugfix 
>you could say "Allow tcc to parse:" and then the example. See commit 
>82969f045c99b4d1ef833de35117c17b326b46c0 for an example. You can also simply 
>explain what bug you fix "Fix parsing of arbitrary number of semicolons" or 
>something better and that brings me to the next comment.
>
>6) You need to understand how a language is designed.
>
>-                parse_btype(&btype, &ad);
>+                if(!parse_btype(&btype, &ad)){
>+                    if (tok == ';'){
>+                        skip(';');
>+                        continue;
>
>Here you are saying that it's ok to have struct st {int a;;;;;} but not struct 
>st {int a:10;;;;}. A better place to add such a check would be near the end of 
>the function, around the "skip(';'). You need to understand why this is 
>authorized. So as an exercise grab the C99 standard at [1] and tell me why 
>this is authorized. Hint: start to read from section 6.7.2.1. Definitions are 
>recursive, so you need to browse quite a lot to understand things. Try to tell 
>me which lexical elements to follow (sorry, I don't know the proper term, 
>shame on me) to create your testcase. Something like:
>
>struct-or-union-specifier
>  -> struct-or-union
>  -> identifier (no need to detail here)
>  -> struct-declaration-list
>     -> struct-declaration
>
>etc…
>
>[1] http://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf
>
>Best regards,
>
>Thomas