On Wed, Jul 29, 2015 at 8:50 AM, Michael Matz matz@frakked.de wrote:
Hello gus or Augustin,
while I appreciate more people working on tinycc, why do you think the
best thing to do as the very first commits would be source code
reformattings and reorganizations? Look at what damage you've done:
The reformattings were to attempt to make everything conform to the
CodingGuidelines document that was already in the tree (4 spaces not
tabs, etc.) which a few files didn't conform to.
@@ -204,7 +204,7 @@ void C67_g(int c)
#endif
ind1 = ind + 4;
if (ind1 (int) cur_text_section-data_allocated)
- section_realloc(cur_text_section, ind1);
+section_realloc(cur_text_section, ind1);
Grand, so now the conditioned statement isn't indended anymore. Or:
while (t) {
- ptr = (int *) (cur_text_section-data + t);
- {
- Sym *sym;
+ptr = (int *) (cur_text_section-data + t);
+{
+Sym *sym;
The 'ptr =' assignment has to be indended. Or:
} else {
- qrel-r_info = ELFW(R_INFO)(0, R_X86_64_RELATIVE);
- qrel-r_addend = *(long long *)ptr + val;
+qrel-r_info = ELFW(R_INFO)(0, R_X86_64_RELATIVE);
+qrel-r_addend = *(long long *)ptr + val;
qrel++;
}
What the hell? Parts of the conditional block are now indended completely
wrong. Or such changes:
Yes, I'm sorry, this is a mistake. I was trying to convert the tabs to
spaces, but it appears that there was a variety of styles (some people
used 8 spaces per tab, some used 4...) so my automated conversion
methods didn't work so well.
-#define RC_INT 0x0001 /* generic integer register */
-#define RC_FLOAT 0x0002 /* generic float register */
-#define RC_R0 0x0004
-#define RC_R1 0x0008
-#define RC_R2 0x0010
-#define RC_R3 0x0020
-#define RC_R12 0x0040
-#define RC_F0 0x0080
-#define RC_F1 0x0100
-#define RC_F2 0x0200
-#define RC_F3 0x0400
+#define RC_INT 0x0001 /* generic integer register */
+#define RC_FLOAT 0x0002 /* generic float register */
+#define RC_R0 0x0004
+#define RC_R1 0x0008
+#define RC_R2 0x0010
+#define RC_R3 0x0020
+#define RC_R12 0x0040
+#define RC_F0 0x0080
+#define RC_F1 0x0100
+#define RC_F2 0x0200
+#define RC_F3 0x0400
Well, obviously the author of this wanted to align the numbers like in a
table, now it looks ugly. Or this:
static unsigned long put_got_entry(TCCState *s1,
- int reloc_type, unsigned long size, int
info,
- int sym_index)
+ int reloc_type, unsigned long size, int info,
+ int sym_index)
{
The arguments on second and third line were meant to align with the first
argument. Or just a few lines down:
if (need_plt_entry !s1-plt) {
- /* add PLT */
- s1-plt = new_section(s1, .plt, SHT_PROGBITS,
- SHF_ALLOC | SHF_EXECINSTR);
- s1-plt-sh_entsize = 4;
+/* add PLT */
+s1-plt = new_section(s1, .plt, SHT_PROGBITS,
+ SHF_ALLOC | SHF_EXECINSTR);
+s1-plt-sh_entsize = 4;
}
Gnah! First the whole conditional statements aren't indended anymore, and
second the arguments to the new_section call aren't aligned.
This is all quite obvious and terrible, and I could go on and on just
citing from the diffs. Have you even _looked_ at your patches before
committing them? Fix all of this right away, otherwise we have to revert
the whole series.
Yes, I will. Apologies, I should've looked more closely (or run the
whole thing through clang-format, but that probably would've done
damage in a different way...)
Also there's another argument against generally doing such code
reformattings: git blame is disturbed now, all lines that you've
reindended now show your commit as the one to blame, which is useless
information. Unfortunately that's a damage that can't be undone now
anymore.
I seem to recall there's some way to have git blame ignore
whitespace-only commits? I could be mistaken...
Next time you want to do whole-sale code changes please discuss on the
mailing list before doing so, there might be reasons for the status quo
that you're unaware of, like in this case.
Noted.
P.S: some of the reindendation changes look like as if you've replaced
leading tabs with four spaces. That would have been wrong, tabs are eight
spaces. If this is the case, fix your editor settings.
In some of the cases, it's obvious that whoever wrote the code was
using 4 spaces per tab, but in others it's obvious they were using 8.
It was kind of hard to determine which, and I had to second-guess
about the surrounding code.
-gus
___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel