Re: [Tinycc-devel] GAS symbols

[Michael Matz]

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.