[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Tinycc-devel] GAS symbols
From: |
Vladimir Vissoultchev |
Subject: |
Re: [Tinycc-devel] GAS symbols |
Date: |
Thu, 17 Mar 2016 00:25:05 +0200 |
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: address@hidden
[mailto:address@hidden On Behalf Of
Michael Matz
Sent: Wednesday, March 16, 2016 10:36 PM
To: address@hidden
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] GAS symbols, Vladimir Vissoultchev, 2016/03/14
- Re: [Tinycc-devel] GAS symbols, Michael Matz, 2016/03/16
- Re: [Tinycc-devel] GAS symbols,
Vladimir Vissoultchev <=
- Re: [Tinycc-devel] GAS symbols, Michael Matz, 2016/03/24
- Re: [Tinycc-devel] GAS symbols, Vladimir Vissoultchev, 2016/03/24
- Re: [Tinycc-devel] GAS symbols, Michael Matz, 2016/03/24
- Re: [Tinycc-devel] GAS symbols, Sergey Korshunoff, 2016/03/29
- Re: [Tinycc-devel] GAS symbols, Vladimir Vissoultchev, 2016/03/29
- Re: [Tinycc-devel] GAS symbols, Sergey Korshunoff, 2016/03/30
- Re: [Tinycc-devel] GAS symbols, Sergey Korshunoff, 2016/03/30
- Re: [Tinycc-devel] GAS symbols, Sergey Korshunoff, 2016/03/30
- Re: [Tinycc-devel] GAS symbols, Sergey Korshunoff, 2016/03/30
- Re: [Tinycc-devel] GAS symbols, Vladimir Vissoultchev, 2016/03/30