Hi, > You made it such that most cstr_cat calls (except two and those in > i386-asm.c) are now followed by cstr_ccat(0). Consider adding a > cstr_catz() routine that does both.
It's not obvious that cstr_cat does *not* terminate the target buffer so I stepped on this landmine and out of curiosity checked all the places it was used. That's when I spotted someone missed it too (in cases that seem to be left for debug purposes only). Thought about implementing a benign null-temrination just ahead of last character but the implementation would be ugly as it has to cstr_ccat(0) for the auto-resize to kick in (if needed) and then unget the '\0'. About the TOK_DOTS saga -- yes, the original code was using PEEKC canonically, I'm not sure about the unget hack it implemented though. Will have to write a testcase and fix it as currently it's not ok. As I see it, will try to use github branches and diffs along with the commit messages for list approval before pushing to mob. Thanks for the feedback and your code-review! cheers, </wqw> -----Original Message----- From: [email protected] [mailto:[email protected]] On Behalf Of Michael Matz Sent: Wednesday, March 16, 2016 10:36 PM To: [email protected] Subject: Re: [Tinycc-devel] GAS symbols Hi, welcome to TCC development :) On Mon, 14 Mar 2016, Vladimir Vissoultchev wrote: > > A symbol is one or more characters chosen from the set of all > > letters > (both upper and lower case), digits and the three characters _.$. No > > > symbol may begin with a digit. Case is significant. There is no > > length > limit: all characters are significant. Symbols are delimited by > > > characters not in that set, or by the beginning of a file (since the > source program must end with a newline, the end of a file is not a > > > possible symbol delimiter). See Symbols. > > So it seems labels can indeed start with and contain dots. Am I > correct in understanding this text? Yes, GAS labels. There's no universal convention for assemblers. Being compatible with GAS does make sense (when easily possible), so yeah, such change seems appropriate. > Also, what is the polite way to commit in mob branch? Do you practice > sending patches to the list beforehand so that anyone can chime in > with problems spotted? No formal process exists, but if you're a new developer sending patches beforehand would be appreciated, especially so for new features, because the feature might not even be wanted (or in a different form). > I'm sorry my first commits were out of nowhere and then had to revert > some rogue changes that broke some tests. Now I have the tests working > under MinGW. Some comments on some of those patches (such comments are also easier when the patch was in a mail :) ): case TOK_CLDOUBLE: cstr_cat(&cstr_buf, "<long double>"); + cstr_ccat(&cstr_buf, '\0'); You made it such that most cstr_cat calls (except two and those in i386-asm.c) are now followed by cstr_ccat(0). Consider adding a cstr_catz() routine that does both. + /* keep structs lvalue by transforming `(expr ? a : b)` to `*(expr~ + that `(expr ? a : b).mem` does not error with "lvalue expected~ + islv = (vtop->r & VT_LVAL) && (sv.r & VT_LVAL) && VT_STRUCT + == (ty~ + Please respect a 80 characters per line limit. It's not always currently respected, but we shouldn't introduce new over long lines. Also, this specific change could use a testcase to not regress in the future. - } else if (c == '.') { - PEEKC(c, p); - if (c == '.') { - p++; - tok = TOK_DOTS; - } else { - *--p = '.'; /* may underflow into file->unget[] */ - tok = '.'; - } + } else if ((isidnum_table['.' - CH_EOF] & IS_ID) != 0) { /* asm mode */ + *--p = c = '.'; + goto parse_ident_fast; + } else if (c == '.' && p[1] == '.') { + p += 2; + tok = TOK_DOTS; As you removed the PEEKC call you mishandle quoted line-endings. For instance the following decl is conforming and hence the ..\\n. must be parsed as one token, TOK_DOTS: int f (int a, ..\ .); This feature could also do with a testcase. One more remark about future git commit messages: please follow the usual git convention. From git-commit(1): Though not required, it?s a good idea to begin the commit message with a single short (less than 50 character) line summarizing the change, followed by a blank line and then a more thorough description. The text up to the first blank line in a commit message is treated as the commit title, and that title is used throughout git. Ciao, Michael. _______________________________________________ Tinycc-devel mailing list [email protected] https://lists.nongnu.org/mailman/listinfo/tinycc-devel
