This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Please, take '-Wmisleading-indentation' out of -Wall


David Malcolm wrote:
Does that bug fully capture the kind of issue you're seeing, or are
there other kinds of false positives?  (I only see that single false
positive in ed-1.13, but you mentioned four projects in total).

The four projects are:
http://ftp.gnu.org/gnu/ed/
http://ftp.gnu.org/gnu/moe/
http://ftp.gnu.org/gnu/ocrad/
http://download.savannah.gnu.org/releases/lzip/lziprecover/

The kind of issue I am seeing is about the final break of each 'case' in a switch statement. This fragment of code from ocrad (ucs.cc, line 98) illustrates the problem:

  switch( letter )
    {
    case 'A': if( accent == '\'') return CAACUTE;
              if( accent == '`' ) return CAGRAVE;
              if( accent == '^' ) return CACIRCU;
              if( accent == ':' ) return CADIAER; break;
    case 'E': if( accent == '\'') return CEACUTE;
              if( accent == '`' ) return CEGRAVE;
              if( accent == '^' ) return CECIRCU;
              if( accent == ':' ) return CEDIAER; break;
    case 'G': return CGBREVE;

Notice how prominently the break terminates each case without increasing the vertical size, so that as much of the switch as possible fits in one screen. I do not want to move the break to the next line.


Warning about indentation is gcc *is* optional, that's why you have to
enable -Wall or -Wmisleading-indentation to get it.

-Wall is optional only in theory. Try 'configure && make' in gcc; it enables by default -Wall and -W. Just as every one of my own projects do.


Re "-Woverlength-strings", I don't think that there's a single scale of
"optionalness" against which warnings can be considered.

-Wall = warnings that everybody enables.
-W (now -Wextra) = warnings that most people enable.
-W<not-in-Wall-nor-W> = warnings that some people enable.

Adding -Wmisleading-indentation to -Wall changes the de-facto default behavior of gcc in a way that puts a burden on me both as an user (slower compilation, annoying warnings), and as maintainer of software that is usually compiled with gcc by other people (spurious bug reports).


FWIW I think the issue here is that the warning was confused by the
"case '#':" on the same line as the "while"); I can suppress the
warning (in ed-1.13) via the following addition of a comment:

You are suggesting to uglify the code (adding an useless comment) to placate gcc. Just what the Standards say we should not do.


I also think that -Wmisleading-indentation should not be enabled by -Wall nor -Wextra because:
   - -Wall is defined in the gcc manual as enabling all the warnings
     about constructions that are easy to avoid (or modify to prevent
     the warning), even in conjunction with macros. This is not true
     of -Wmisleading-indentation.

It's trivial to work around, by changing the indentation.

The indentation in the ed example is correct. Changing it makes it incorrect. Changing something from correct to incorrect may be trivial, but I find insulting the mere suggestion of doing it. The next (fixed) version of gcc could then complain that my changed indentation is misleading, and it would be right!


   - If there are no macros involved, it is independent of code
     generation; once tested in the developer's machine there is no
     need to test it again on every user's machine.

I don't understand this objection.

In the following code:

  unsigned a = 1;
  size_t b = 2;
  if( a < b )

it is useful to enable -Wsign-compare on every user's machine because size_t may be an unsigned type on my machine but a signed type on some other user's machine.

On the contrary, if I check that the indentation is correct in the source before packaging it, there is no way for it to become incorrect when the user untars it. -Wmisleading-indentation is a lint option, not a compiler one. Enabling it with -Wall just wastes everybody's time.


   - It makes compilation a 0.5-1% slower for every user for no reason.

Where did these numbers come from?

From the compilation times of the four projects on my machine with and without -Wmisleading-indentation:
('make clean' is run first. Default CFLAGS/CXXFLAGS are '-Wall -W -O2').
with = time make
without = time make CFLAGS='-Wall -W -O2 -Wno-misleading-indentation'
or
          time make CXXFLAGS='-Wall -W -O2 -Wno-misleading-indentation'

                  with          without
ed               2.039s          2.011s         1.3% slower
ocrad           30.502s         30.295s         0.6% slower
moe             18.423s         18.346s         0.4% slower
lziprecover     12.127s         12.005s         1  % slower


Thanks, I hope this is a constructive response.

Well, the main point (that -Wmisleading-indentation does not belong in -Wall) has not been addressed.


Best regards,
Antonio.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]