Re: [Tinycc-devel] GAS symbols

[Vladimir Vissoultchev]

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.