Re: [Tinycc-devel] tiny bit of lint
Hi Larry,
I see your efforts to make tcc more happy when compiled with clang.
For printf("<- %s (%d symbol%s)\n", buf, sym_count, "s"[sym_count < 2]);
It's even better if you use:
printf("<- %s (%d symbol%s)\n", buf, sym_count, ((sym_count > 1) ? "s" :
""));
-Original Message-
From: Tinycc-devel [mailto:tinycc-devel-bounces+eligis=orange...@nongnu.org]
On Behalf Of Larry Doolittle
Sent: lundi 8 mai 2017 18:48
To: tinycc-devel@nongnu.org
Subject: Re: [Tinycc-devel] tiny bit of lint
grischka -
On Mon, May 08, 2017 at 07:27:08PM +0200, grischka wrote:
> Larry Doolittle wrote:
> >I committed (to mob) one patch that should count as progress.
> >Remove some unused-parameter lint
>
> In my book that counts as visual clutter with no practical benefit.
> The opposite of "tinycc avoids the unnecessary". Suggesting
> particular care that feels alien in contrast to what the code really cares
about.
I respect that point of view. Yes, the code is a bit more cluttered.
In this case I added 11 lines of cruft to 9841 existing lines of code.
Also note that this is source-only cruft, with no effect on the resulting
binary.
The potential upside, that I do value, is that (with a bit more work like
this) "we" get to turn on -Wextra for general development. That _should_
lead to _real_ flaws getting caught earlier in the development cycle.
If unused parameters in general are considered OK in this code base, CFLAGS
could be set to -Wextra -Wno-unused-argument. The trick there would be in
the configure/Makefile step, adding that flag set only on compatible
compilers.
I'll hold off doing anything more like this until and unless I hear a net
positive from the mailing list.
- Larry
___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Re: [Tinycc-devel] tiny bit of lint
Larry Doolittle wrote:
@@ -965,7 +965,7 @@ static void pe_build_exports(struct pe_info *pe)
} else {
fprintf(op, "LIBRARY %s\n\nEXPORTS\n", dllname);
if (pe->s1->verbose)
-printf("<- %s (%d symbol%s)\n", buf, sym_count, "s" + (sym_count <
2));
+printf("<- %s (%d symbol%s)\n", buf, sym_count, "s"[sym_count <
2]);
}
#endif
And that is wrong of course.
$ tcc examples/dll.c -shared -v
---> crash
-- gr
___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Re: [Tinycc-devel] tiny bit of lint
Arf! You're right.
That why I never fix warning in a code which is not mine.
My fix proposal is definitively better.
-Original Message-
From: Tinycc-devel [mailto:tinycc-devel-bounces+eligis=orange...@nongnu.org]
On Behalf Of grischka
Sent: mardi 9 mai 2017 10:26
To: tinycc-devel@nongnu.org
Subject: Re: [Tinycc-devel] tiny bit of lint
Larry Doolittle wrote:
@@ -965,7 +965,7 @@ static void pe_build_exports(struct pe_info *pe)
} else {
fprintf(op, "LIBRARY %s\n\nEXPORTS\n", dllname);
if (pe->s1->verbose)
-printf("<- %s (%d symbol%s)\n", buf, sym_count, "s" +
(sym_count < 2));
+printf("<- %s (%d symbol%s)\n", buf, sym_count, "s"[sym_count <
2]);
}
#endif
And that is wrong of course.
$ tcc examples/dll.c -shared -v
---> crash
-- gr
___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Re: [Tinycc-devel] tiny bit of lint
Friends -
On Tue, May 09, 2017 at 11:49:23AM +0200, Christian Jullien wrote:
> Arf! You're right.
> That why I never fix warning in a code which is not mine.
> My fix proposal is definitively better.
Sorry, everyone, testing should have caught my original mistake.
I committed a fix.
The real equivalence is between the original "s" + (i<2)
and &"s"[i<2].
((i > 1) ? "s" : "") as suggested by Christian is perhaps easier
to for humans to understand, but (at least when compiling with tcc)
produces larger (by 20 bytes) and slower object code. With gcc -O2
the object code size penalty is 11 bytes. This is on x86_64, YMMV.
With that extra &, I can't really say the looks are improved over
the original. But testing with gcc (4.9.2), tcc, and clang (3.5.0)
they all get the right answer on both styles, and clang complains
about the original form.
- Larry
> -Original Message-
> From: Tinycc-devel [mailto:tinycc-devel-bounces+eligis=orange...@nongnu.org]
> On Behalf Of grischka
> Sent: mardi 9 mai 2017 10:26
> To: tinycc-devel@nongnu.org
> Subject: Re: [Tinycc-devel] tiny bit of lint
>
> Larry Doolittle wrote:
>
> @@ -965,7 +965,7 @@ static void pe_build_exports(struct pe_info *pe)
> } else {
> fprintf(op, "LIBRARY %s\n\nEXPORTS\n", dllname);
> if (pe->s1->verbose)
> -printf("<- %s (%d symbol%s)\n", buf, sym_count, "s" +
> (sym_count < 2));
> +printf("<- %s (%d symbol%s)\n", buf, sym_count, "s"[sym_count <
> 2]);
> }
> #endif
>
> And that is wrong of course.
>
> $ tcc examples/dll.c -shared -v
> ---> crash
>
> -- gr
>
> ___
> Tinycc-devel mailing list
> Tinycc-devel@nongnu.org
> https://lists.nongnu.org/mailman/listinfo/tinycc-devel
>
>
> ___
> Tinycc-devel mailing list
> Tinycc-devel@nongnu.org
> https://lists.nongnu.org/mailman/listinfo/tinycc-devel
___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Re: [Tinycc-devel] tiny bit of lint
Larry,
Don't you think we can afford 11/20 bytes and increase readability instead
of this obfuscated code?
printf("<- %s (%d symbol%s)\n", buf, sym_count, &"s"[sym_count < 2]);
If you want to save bytes, you can just
printf("<- %s (%d symbol[s])\n", buf, sym_count);
Which is both faster and shorter.
-Original Message-
From: Tinycc-devel [mailto:tinycc-devel-bounces+eligis=orange...@nongnu.org]
On Behalf Of Larry Doolittle
Sent: mardi 9 mai 2017 16:24
To: tinycc-devel@nongnu.org
Subject: Re: [Tinycc-devel] tiny bit of lint
Friends -
On Tue, May 09, 2017 at 11:49:23AM +0200, Christian Jullien wrote:
> Arf! You're right.
> That why I never fix warning in a code which is not mine.
> My fix proposal is definitively better.
Sorry, everyone, testing should have caught my original mistake.
I committed a fix.
The real equivalence is between the original "s" + (i<2) and &"s"[i<2].
((i > 1) ? "s" : "") as suggested by Christian is perhaps easier to for
humans to understand, but (at least when compiling with tcc) produces larger
(by 20 bytes) and slower object code. With gcc -O2 the object code size
penalty is 11 bytes. This is on x86_64, YMMV.
With that extra &, I can't really say the looks are improved over the
original. But testing with gcc (4.9.2), tcc, and clang (3.5.0) they all get
the right answer on both styles, and clang complains about the original
form.
- Larry
> -Original Message-
> From: Tinycc-devel
> [mailto:tinycc-devel-bounces+eligis=orange...@nongnu.org]
> On Behalf Of grischka
> Sent: mardi 9 mai 2017 10:26
> To: tinycc-devel@nongnu.org
> Subject: Re: [Tinycc-devel] tiny bit of lint
>
> Larry Doolittle wrote:
>
> @@ -965,7 +965,7 @@ static void pe_build_exports(struct pe_info *pe)
> } else {
> fprintf(op, "LIBRARY %s\n\nEXPORTS\n", dllname);
> if (pe->s1->verbose)
> -printf("<- %s (%d symbol%s)\n", buf, sym_count, "s" +
> (sym_count < 2));
> +printf("<- %s (%d symbol%s)\n", buf, sym_count,
> + "s"[sym_count <
> 2]);
> }
> #endif
>
> And that is wrong of course.
>
> $ tcc examples/dll.c -shared -v
> ---> crash
>
> -- gr
>
> ___
> Tinycc-devel mailing list
> Tinycc-devel@nongnu.org
> https://lists.nongnu.org/mailman/listinfo/tinycc-devel
>
>
> ___
> Tinycc-devel mailing list
> Tinycc-devel@nongnu.org
> https://lists.nongnu.org/mailman/listinfo/tinycc-devel
___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Re: [Tinycc-devel] tiny bit of lint
Larry Doolittle wrote: Friends - On Tue, May 09, 2017 at 11:49:23AM +0200, Christian Jullien wrote: Arf! You're right. That why I never fix warning in a code which is not mine. My fix proposal is definitively better. Sorry, everyone, testing should have caught my original mistake. I committed a fix. The real equivalence is between the original "s" + (i<2) and &"s"[i<2]. If people are going to use clang more likely, can't you add something to configure for clang to turn off some silly warnings? That would be USEFUL, for example ;) --- gr ___ Tinycc-devel mailing list Tinycc-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Re: [Tinycc-devel] tiny bit of lint
Even with clang, you can write without warnings:
if (pe->s1->verbose) {
static const char* plural = "s";
printf("<- %s (%d symbol%s)\n",
buf, sym_count, plural + (int)(sym_count < 2));
}
But clearly, I prefer to waste 11/20 bytes. Now, it's up to you!
-Original Message-
From: Tinycc-devel [mailto:tinycc-devel-bounces+eligis=orange...@nongnu.org]
On Behalf Of grischka
Sent: mardi 9 mai 2017 17:44
To: tinycc-devel@nongnu.org
Subject: Re: [Tinycc-devel] tiny bit of lint
Larry Doolittle wrote:
> Friends -
>
> On Tue, May 09, 2017 at 11:49:23AM +0200, Christian Jullien wrote:
>> Arf! You're right.
>> That why I never fix warning in a code which is not mine.
>> My fix proposal is definitively better.
>
> Sorry, everyone, testing should have caught my original mistake.
>
> I committed a fix.
>
> The real equivalence is between the original "s" + (i<2) and
> &"s"[i<2].
If people are going to use clang more likely, can't you add something to
configure for clang to turn off some silly warnings?
That would be USEFUL, for example ;)
--- gr
___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Re: [Tinycc-devel] tiny bit of lint
grischka - On Tue, May 09, 2017 at 05:44:28PM +0200, grischka wrote: > If people are going to use clang more likely, can't you add something > to configure for clang to turn off some silly warnings? > That would be USEFUL, for example ;) The clang flag to turn off this warning is -Wno-string-plus-int That brings up another issue, which is that tinycc's cute little configure script that figures out which warning (or lack of warning) flags are compatible with $CC doesn't work with clang. I have no opinion on whether adding the nice plural to two tinycc messages is worth the extra space and time. I don't think that ((i > 1) ? "s" : "") is a good choice. Any of "s" + (i<2), &"s"[i<2], and just a universal plural "symbols" (like it was before 2d3b9559 from Feb 2017) is fine with me. This is a lot of discussion for two lines of code. I guess that's mostly my fault, too. - Larry ___ Tinycc-devel mailing list Tinycc-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Re: [Tinycc-devel] tiny bit of lint
Larry Doolittle wrote: grischka - On Tue, May 09, 2017 at 05:44:28PM +0200, grischka wrote: If people are going to use clang more likely, can't you add something to configure for clang to turn off some silly warnings? That would be USEFUL, for example ;) The clang flag to turn off this warning is -Wno-string-plus-int That brings up another issue, which is that tinycc's cute little configure script that figures out which warning (or lack of warning) flags are compatible with $CC doesn't work with clang. Yes, the configure warning magic doesn't work with clang. And hence my question was: Can you fix this? Means: recognize if $CC is clang and implement some method to disable its warnings in configure. Can you do this? That could be useful in future for similar problems. Otherwise I see no point in fixing people or fixing peoples' style, in a multi-author project. There is a property of original authorship on each line that you did not write and that property should be respected. This includes diverging personal style and maybe even technical flaws. --- grischka I have no opinion on whether adding the nice plural to two tinycc messages is worth the extra space and time. I don't think that ((i > 1) ? "s" : "") is a good choice. Any of "s" + (i<2), &"s"[i<2], and just a universal plural "symbols" (like it was before 2d3b9559 from Feb 2017) is fine with me. This is a lot of discussion for two lines of code. I guess that's mostly my fault, too. - Larry ___ Tinycc-devel mailing list Tinycc-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Re: [Tinycc-devel] tiny bit of lint
Grischka, I know rather well the magic of configure.ac. In numerus projects I own, I use code like: AX_CHECK_COMPILE_FLAG([-Wall], [CXXFLAGS="$CXXFLAGS -Wall"]) AX_CHECK_COMPILE_FLAG([-Wextra], [CXXFLAGS="$CXXFLAGS -Wextra"]) AX_CHECK_COMPILE_FLAG([-Wshadow], [CXXFLAGS="$CXXFLAGS -Wshadow"]) AX_CHECK_COMPILE_FLAG([-Wunused], [CXXFLAGS="$CXXFLAGS -Wunused"]) To detect what options CC supports. With it, clang gives me false positive. i.e. configure pretends clang supports an option which is then refused when actually used. In case of tcc, we'll have to deal with scripts that have to test the exact version of clang used and supported flags. -Original Message- From: Tinycc-devel [mailto:tinycc-devel-bounces+eligis=orange...@nongnu.org] On Behalf Of grischka Sent: mardi 9 mai 2017 19:33 To: tinycc-devel@nongnu.org Subject: Re: [Tinycc-devel] tiny bit of lint Larry Doolittle wrote: > grischka - > > On Tue, May 09, 2017 at 05:44:28PM +0200, grischka wrote: >> If people are going to use clang more likely, can't you add something >> to configure for clang to turn off some silly warnings? >> That would be USEFUL, for example ;) > > The clang flag to turn off this warning is -Wno-string-plus-int > > That brings up another issue, which is that tinycc's cute little > configure script that figures out which warning (or lack of warning) > flags are compatible with $CC doesn't work with clang. Yes, the configure warning magic doesn't work with clang. And hence my question was: Can you fix this? Means: recognize if $CC is clang and implement some method to disable its warnings in configure. Can you do this? That could be useful in future for similar problems. Otherwise I see no point in fixing people or fixing peoples' style, in a multi-author project. There is a property of original authorship on each line that you did not write and that property should be respected. This includes diverging personal style and maybe even technical flaws. --- grischka > I have no opinion on whether adding the nice plural to two tinycc > messages is worth the extra space and time. I don't think that ((i > > 1) ? "s" : "") is a good choice. Any of "s" + (i<2), &"s"[i<2], and > just a universal plural "symbols" (like it was before 2d3b9559 from > Feb 2017) is fine with me. > > This is a lot of discussion for two lines of code. I guess that's > mostly my fault, too. > > - Larry ___ Tinycc-devel mailing list Tinycc-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/tinycc-devel ___ Tinycc-devel mailing list Tinycc-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Re: [Tinycc-devel] tiny bit of lint
grischka -
On Tue, May 09, 2017 at 07:33:28PM +0200, grischka wrote:
> Yes, the configure warning magic doesn't work with clang. And
> hence my question was:
> Can you fix this?
> Means: recognize if $CC is clang and implement some method to
> disable its warnings in configure. Can you do this?
> That could be useful in future for similar problems.
Proof of concept, where dummy.c is something simple and correct like
int dummy(void) { return 0; }
CFLAGS="-Wall -g -O2"
cc=clang
W_OPTIONS="extra declaration-after-statement undef strict-prototypes
write-strings lskdjfsldfkj bogus no-pointer-sign no-sign-compare
no-unused-parameter no-string-plus-int"
$cc -c `for i in $W_OPTIONS; do echo -W$i; done` dummy.c > cc_msg.txt 2>&1
for i in $W_OPTIONS; do
O_INVALID=`grep -- -W$i cc_msg.txt)`
if test -z "$O_INVALID"; then CFLAGS="$CFLAGS -W$i"; fi
done
echo $CFLAGS
tcc has the interesting property that it accepts and ignores any -W flags
not on its (short) list, so even -Wlskdjfsldfkj -Wbogus are accepted by
the above process. gcc and clang reject them.
Tested, works with bash and dash.
- Larry
___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Re: [Tinycc-devel] tiny bit of lint
Larry Doolittle wrote:
Proof of concept, where dummy.c is something simple and correct like
int dummy(void) { return 0; }
CFLAGS="-Wall -g -O2"
cc=clang
W_OPTIONS="extra declaration-after-statement undef strict-prototypes write-strings
lskdjfsldfkj bogus no-pointer-sign no-sign-compare no-unused-parameter
no-string-plus-int"
$cc -c `for i in $W_OPTIONS; do echo -W$i; done` dummy.c > cc_msg.txt 2>&1
for i in $W_OPTIONS; do
O_INVALID=`grep -- -W$i cc_msg.txt)`
if test -z "$O_INVALID"; then CFLAGS="$CFLAGS -W$i"; fi
done
echo $CFLAGS
What about
W_OPTIONS="\
-Wdeclaration-after-statement\
-Wno-pointer-sign -Wno-sign-compare -Wno-unused-result\
-fno-strict-aliasing\
"
if echo "$cc" | grep -q "clang"; then
W_OPTIONS="$W_OPTIONS -Wno-string-plus-int"
fi
$cc -Wunsupported $W_OPTIONS -o a.out -c -xc - < /dev/null > cc_msg.txt 2>&1
for i in $W_OPTIONS; do
grep -q -- $i cc_msg.txt || CFLAGS="$CFLAGS $i"
done
rm -f cc_msg.txt a.out
The -Wunsupported is to make it work with --cc=tcc.
Policy is basically:
- turn on -Wall plus what we really need
- turn off what gets in the way
- otherwise short command-lines and readable build logs
If it works you could push it and people can try it out.
-- gr
___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
