#93 Implement --werror option

closed-accepted
None
5
2007-10-14
2007-10-14
No

Hi.

The attached patch implements the --Werror
option.

Discussion

  • Stas Sergeev

    Stas Sergeev - 2007-10-14

    the patch

     
  • Stas Sergeev

    Stas Sergeev - 2007-10-14
    • summary: Implement --Werror option --> Implement --werror option
     
  • Stas Sergeev

    Stas Sergeev - 2007-10-14

    Logged In: YES
    user_id=501371
    Originator: YES

    File Added: werror.diff

     
  • Maarten Brock

    Maarten Brock - 2007-10-14

    Logged In: YES
    user_id=888171
    Originator: NO

    Stas,

    Thanks for the patch, but a bit more information might have been helpful. The intention seems to be to change all warnings into errors.

    Instead of the variable "options" I would prefer the use of "_SDCCERRG" which currently holds all other options for the error/warning generation.

    This line contains a bug. Too many formatters or too few parameters:
    + fprintf(_SDCCERRG.out, options.werror ?
    + "error %d: " : "warning %d: ", errNum);

    A small language issue here, please remove "an" and change "threat" to "treat":
    + int werror; /* threat the warnings as an errors */
    + { 0, OPTION_WERROR, NULL, "Treat the warnings as an errors" },

    Finally, I think we would prefer --Werror over --werror, but I would very much like to hear from the other developers what they think. (gcc uses -Werror, but SDCC already uses -W... for passing options to the preprocessor, assembler or linker.)

    Maarten

     
  • Stas Sergeev

    Stas Sergeev - 2007-10-14

    Logged In: YES
    user_id=501371
    Originator: YES

    > Thanks for the patch, but a bit more information might have been helpful.
    This just duplicates the -Werror of gcc.

    > Instead of the variable "options" I would prefer the use of "_SDCCERRG"
    Done.

    > This line contains a bug. Too many formatters or too few parameters:
    What is the bug? My gcc doesn't complain...
    OK, I changed that to "if".

    > Finally, I think we would prefer --Werror over --werror
    Actually, my initial patch did that. You can even
    see the "--Werror" in my original message. But a
    few minutes later I changed that and attached the
    patch with "--werror" for the following reasons:
    1. All sdcc long-options starts from the lowercase,
    it seems.
    2. gcc have "-Werror", but sdcc wants "--", not "-"
    in the beginning. So we already deviate from gcc
    syntax, and as such, there seem to be no reason
    for the uppercase.
    But that's not a problem and can be changed.

    A slightly bigger problem is that the "werror"
    field may be uninitialized. This is not a problem
    since gcc zeroes .bss AFAIK, but who knows what
    compiler someone uses. Where would it be better
    to init that value, or is it needed at all?
    File Added: werror1.diff

     
  • Stas Sergeev

    Stas Sergeev - 2007-10-14

    new patch

     
  • Maarten Brock

    Maarten Brock - 2007-10-14

    Logged In: YES
    user_id=888171
    Originator: NO

    Stas,

    Once again thank you.

    Indeed there was no bug, I misread the code as
    "error %d: : warning %d: ", errNum);
    instead of
    "error %d: " : "warning %d: ", errNum);

    Sorry, my fault. Nonetheless, this one is better human-readable :-)

    I also did not realize that to access _SDCCERRG you would need an extra function. This kind of cancels out the advantage of keeping error/warning options local. I think I no longer have a preference.

    You need not worry about the field being uninitialized. The C standard mandates that all uninitialized globals and statics are initialized to zero.

    The reason I prefer the uppercase W in --Werror is because gcc also uses it. Compare it to --funsigned-char (SDCC) vs. -funsigned-char (GCC).

    Now all we have to do is wait for some other developer to give an opinion on the name of this option.

     
  • Borut Ražem

    Borut Ražem - 2007-10-14

    Logged In: YES
    user_id=568035
    Originator: NO

    > The reason I prefer the uppercase W in --Werror is because gcc also uses
    > it. Compare it to --funsigned-char (SDCC) vs. -funsigned-char (GCC).

    I agree with Maarten. My opinion that we should be compatible with gcc as much as possible.

    Borut

     
  • Maarten Brock

    Maarten Brock - 2007-10-14
    • assigned_to: nobody --> maartenbrock
     
  • Stas Sergeev

    Stas Sergeev - 2007-10-14

    Logged In: YES
    user_id=501371
    Originator: YES

    No problems.
    Maarten please do such a change, as I
    am too lazy to create/test a new patch
    for a single letter change. :)

     
  • Maarten Brock

    Maarten Brock - 2007-10-14

    Logged In: YES
    user_id=888171
    Originator: NO

    Applied in SDCC 2.7.4 #4932.

     
  • Maarten Brock

    Maarten Brock - 2007-10-14
    • status: open --> closed-accepted
     

Log in to post a comment.