bugThe FreeType Project - Bugs: bug #51331, C4146 Compiler warnings in ftcalc.c

 
 

bug #51331: C4146 Compiler warnings in ftcalc.c

Submitted by:  Dominik Röttsches <drott>
Submitted on:  Wed 28 Jun 2017 10:17:15 AM UTC  
 
Severity: 3 - NormalItem Group: None
Status: FixedPrivacy: Public
Assigned to: Werner LEMBERG <wl>Open/Closed: Closed
Planned Release: 2.8.1

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Fri 30 Jun 2017 06:35:08 AM UTC, comment #9:

IMHO yes – I guess every compiler has an optimization for `0 - something'.

Werner LEMBERG <wl>
Project AdministratorIn charge of this item.
Thu 29 Jun 2017 07:37:42 PM UTC, comment #8:

The premise was that all these macros are purely cosmetic and do not have any performance hits. Are we still in the cosmetic realm? I would rather check the result if it is larger than 0x7FFFFFFF before negating.

Alexei Podtelezhnikov <podtelez>
Project Member
Wed 28 Jun 2017 08:59:22 PM UTC, comment #7:

Thanks a lot! Patch applied to git.

Werner LEMBERG <wl>
Project AdministratorIn charge of this item.
Wed 28 Jun 2017 07:31:06 PM UTC, comment #6:

Now that I have that working, it appears NEG_LONG and NEG_INT32 have similar issues. I am attaching a patch for the full proposed change. With this patch I'm able to build as outlined in comment #5.

(file #41062)

bungeman <bungeman>
Wed 28 Jun 2017 07:14:04 PM UTC, comment #5:

Also, note that this isn't just a Chromium build issue, I can get the same warning as error building just FreeType on Windows with

del config.mk
make visualc
make

bungeman <bungeman>
Wed 28 Jun 2017 05:56:13 PM UTC, comment #4:

Of course, that may be overly clever, I believe

x_unsigned = 0u - x_unsigned;

is always the same as the current code, it probably simpler to understand and optimize, and avoids this (silly, when applied to a non-constant) warning.

bungeman <bungeman>
Wed 28 Jun 2017 04:07:42 PM UTC, comment #3:

Since the top of ftcalc.c states that "Support for 1-complement arithmetic has been totally dropped" is it possible to replace

x_unsigned = -x_unsigned;

with

x_unsigned = ~(x_unsigned) + 1u;

I believe that (at least in non-debug builds) this results in the same code (at least it does here) and avoids the warning.

Part of the issue with pragmas is that (afaik) you can't (in c89) put one inside a macro, so they'd have to go around each block or the whole file to be effective.

bungeman <bungeman>
Wed 28 Jun 2017 11:30:31 AM UTC, comment #2:

I mean a patch that uses #pragma to silence MSVC.

Werner LEMBERG <wl>
Project AdministratorIn charge of this item.
Wed 28 Jun 2017 11:30:06 AM UTC, comment #1:

Well, yes. Casting to unsigned is what clang recommends to do a negation that doesn't overflow...

Can you provide a patch to silence MSVC? While the warning is nice, it is completely useless here :-)

Werner LEMBERG <wl>
Project AdministratorIn charge of this item.
Wed 28 Jun 2017 10:17:15 AM UTC, original submission:

When compiling 2e7bb5e82 in MSVC for Chrome, we get the following warnings for ftcalc.c

Compare
https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Ftryserver.chromium.win%2Fwin_chromium_rel_ng%2F477881%2F%2B%2Frecipes%2Fsteps%2Fcompile__with_patch_%2F0%2Fstdout

and

https://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/477881

e:\b\c\b\win\src\third_party\freetype\src\src\base\ftcalc.c(187): error C2220: warning treated as error - no 'object' file generated
e:\b\c\b\win\src\third_party\freetype\src\src\base\ftcalc.c(187): warning C4146: unary minus operator applied to unsigned type, result still unsigned
e:\b\c\b\win\src\third_party\freetype\src\src\base\ftcalc.c(188): warning C4146: unary minus operator applied to unsigned type, result still unsigned
e:\b\c\b\win\src\third_party\freetype\src\src\base\ftcalc.c(189): warning C4146: unary minus operator applied to unsigned type, result still unsigned
e:\b\c\b\win\src\third_party\freetype\src\src\base\ftcalc.c(196): warning C4146: unary minus operator applied to unsigned type, result still unsigned
e:\b\c\b\win\src\third_party\freetype\src\src\base\ftcalc.c(216): warning C4146: unary minus operator applied to unsigned type, result still unsigned
e:\b\c\b\win\src\third_party\freetype\src\src\base\ftcalc.c(217): warning C4146: unary minus operator applied to unsigned type, result still unsigned
e:\b\c\b\win\src\third_party\freetype\src\src\base\ftcalc.c(218): warning C4146: unary minus operator applied to unsigned type, result still unsigned
e:\b\c\b\win\src\third_party\freetype\src\src\base\ftcalc.c(225): warning C4146: unary minus operator applied to unsigned type, result still unsigned
e:\b\c\b\win\src\third_party\freetype\src\src\base\ftcalc.c(264): warning C4146: unary minus operator applied to unsigned type, result still unsigned
e:\b\c\b\win\src\third_party\freetype\src\src\base\ftcalc.c(265): warning C4146: unary minus operator applied to unsigned type, result still unsigned
e:\b\c\b\win\src\third_party\freetype\src\src\base\ftcalc.c(272): warning C4146: unary minus operator applied to unsigned type, result still unsigned
e:\b\c\b\win\src\third_party\freetype\src\src\base\ftcalc.c(788): warning C4146: unary minus operator applied to unsigned type, result still unsigned
e:\b\c\b\win\src\third_party\freetype\src\src\base\ftcalc.c(789): warning C4146: unary minus operator applied to unsigned type, result still unsigned

Dominik Röttsches <drott>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #41062:  C4146.diff added by bungeman (1KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by podtelez (Posted a comment)
  • -unavailable- added by bungeman (Posted a comment)
  • -unavailable- added by wl (Posted a comment)
  • -unavailable- added by drott (Submitted the item)
  • -unavailable- added by drott
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 7 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 28 Jun 2017 08:59:22 PM UTCwlStatusWorks For Me=>Fixed
      Open/ClosedOpen=>Closed
      Planned ReleaseNone=>2.8.1
    Wed 28 Jun 2017 07:31:06 PM UTCbungemanAttached File-=>Added C4146.diff, #41062
    Wed 28 Jun 2017 11:30:06 AM UTCwlStatusNone=>Works For Me
      Assigned toNone=>wl
    Wed 28 Jun 2017 10:17:15 AM UTCdrottCarbon-Copy-=>Added bungeman

    Back to the top


    Powered by Savane 3.1-cleanup1