[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Tinycc-devel] GAS symbols

From: Michael Matz
Subject: Re: [Tinycc-devel] GAS symbols
Date: Wed, 16 Mar 2016 21:36:08 +0100 (CET)
User-agent: Alpine 2.20 (LSU 67 2015-01-07)


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.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]