On Thu, Sep 23, 2010 at 4:04 AM, Michael Spencer <bigcheesegs at gmail.com>wrote:> On Thu, Sep 23, 2010 at 2:40 AM, Nathan Jeffords > <blunted2night at gmail.com> wrote: > > here are a couple of patches to address some warnings > > in Microsoft compilers, and a build error in vs2010 in particular > > vs2010-errors.patch: > > In SelectionDAGISel, std::pairs or pointers are being constructed using > a 0 > > as an initializer causes an error inside the std::pair constructor. I > think > > changes to language in support of the nullptr keyword caused this break. > > Yep, people keep doing this. I've been behind on the trunk for a bit, > but I was surprised to see this come up again. This is the correct > fix, but please make sure you stick within 80 cols. > > > vs2010-warnings.patch: > > This patch fixes a couple of annoying warnings that show up all over the > > place because they are in headers. At least for Microsoft compilers, > > conversion of a int to bool requires the compiler to convert non-zero to > one > > and so the compiler warns of the performance issue. My fix is to > explicitly > > perform the conversion so the compiler knows I'm aware and doesn't warn > me. > > I don't know the behavior of other compilers in this respect, so I don't > > know if it should be committed. The other solution would be to change the > > return type of the accessors function to return ints, though this could > move > > the warning elsewhere. > > -Nathan > > This fix looks like a case of 3am programming. I think using > static_cast<bool> is clearer. > > - Michael Spencer >here are updated versions - Nathan -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100923/6d665e3f/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: vs2010-errors.patch Type: application/octet-stream Size: 1754 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100923/6d665e3f/attachment.obj> -------------- next part -------------- A non-text attachment was scrubbed... Name: vs2010-warnings.patch Type: application/octet-stream Size: 1067 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100923/6d665e3f/attachment-0001.obj>
Nathan Jeffords <blunted2night at gmail.com> writes:> here are updated versionsApplied as 114662 and 114661. Thanks!
Note: static_cast<bool> doesn't make the warning go away or did you check that? I just compiled a small test and it didn't work. The recommended solution is: (expression) != 0 http://msdn.microsoft.com/en-us/library/b6801kcy.aspx It even says: Casting the expression to type bool will not disable the warning, which is by design. On Thu, Sep 23, 2010 at 9:18 AM, Nathan Jeffords <blunted2night at gmail.com>wrote:> > > On Thu, Sep 23, 2010 at 4:04 AM, Michael Spencer <bigcheesegs at gmail.com>wrote: > >> On Thu, Sep 23, 2010 at 2:40 AM, Nathan Jeffords >> <blunted2night at gmail.com> wrote: >> > here are a couple of patches to address some warnings >> > in Microsoft compilers, and a build error in vs2010 in particular >> > vs2010-errors.patch: >> > In SelectionDAGISel, std::pairs or pointers are being constructed using >> a 0 >> > as an initializer causes an error inside the std::pair constructor. I >> think >> > changes to language in support of the nullptr keyword caused this break. >> >> Yep, people keep doing this. I've been behind on the trunk for a bit, >> but I was surprised to see this come up again. This is the correct >> fix, but please make sure you stick within 80 cols. >> >> > vs2010-warnings.patch: >> > This patch fixes a couple of annoying warnings that show up all over the >> > place because they are in headers. At least for Microsoft compilers, >> > conversion of a int to bool requires the compiler to convert non-zero to >> one >> > and so the compiler warns of the performance issue. My fix is to >> explicitly >> > perform the conversion so the compiler knows I'm aware and doesn't warn >> me. >> > I don't know the behavior of other compilers in this respect, so I don't >> > know if it should be committed. The other solution would be to change >> the >> > return type of the accessors function to return ints, though this could >> move >> > the warning elsewhere. >> > -Nathan >> >> This fix looks like a case of 3am programming. I think using >> static_cast<bool> is clearer. >> >> - Michael Spencer >> > > here are updated versions > > - Nathan > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >-- Ahmed Charles http://www.ahmedcharles.com -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100925/50fe257a/attachment.html>
On Sat, Sep 25, 2010 at 10:53 AM, Ahmed Charles <ahmedcharles at gmail.com> wrote:> Note: static_cast<bool> doesn't make the warning go away or did you check > that? I just compiled a small test and it didn't work. The recommended > solution is: > > (expression) != 0 > > http://msdn.microsoft.com/en-us/library/b6801kcy.aspx > > It even says: Casting the expression to type bool will not disable the > warning, which is by design.I did not check that, sorry (I've not seen the warning). Thanks for the proper fix :P. - Michael Spencer> On Thu, Sep 23, 2010 at 9:18 AM, Nathan Jeffords <blunted2night at gmail.com> > wrote: >> >> >> On Thu, Sep 23, 2010 at 4:04 AM, Michael Spencer <bigcheesegs at gmail.com> >> wrote: >>> >>> On Thu, Sep 23, 2010 at 2:40 AM, Nathan Jeffords >>> <blunted2night at gmail.com> wrote: >>> > here are a couple of patches to address some warnings >>> > in Microsoft compilers, and a build error in vs2010 in particular >>> > vs2010-errors.patch: >>> > In SelectionDAGISel, std::pairs or pointers are being constructed >>> > using a 0 >>> > as an initializer causes an error inside the std::pair constructor. I >>> > think >>> > changes to language in support of the nullptr keyword caused this >>> > break. >>> >>> Yep, people keep doing this. I've been behind on the trunk for a bit, >>> but I was surprised to see this come up again. This is the correct >>> fix, but please make sure you stick within 80 cols. >>> >>> > vs2010-warnings.patch: >>> > This patch fixes a couple of annoying warnings that show up all over >>> > the >>> > place because they are in headers. At least for Microsoft compilers, >>> > conversion of a int to bool requires the compiler to convert non-zero >>> > to one >>> > and so the compiler warns of the performance issue. My fix is to >>> > explicitly >>> > perform the conversion so the compiler knows I'm aware and doesn't warn >>> > me. >>> > I don't know the behavior of other compilers in this respect, so I >>> > don't >>> > know if it should be committed. The other solution would be to change >>> > the >>> > return type of the accessors function to return ints, though this could >>> > move >>> > the warning elsewhere. >>> > -Nathan >>> >>> This fix looks like a case of 3am programming. I think using >>> static_cast<bool> is clearer. >>> >>> - Michael Spencer >> >> here are updated versions >> - Nathan >> >> _______________________________________________ >> LLVM Developers mailing list >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> > > > > -- > Ahmed Charles > http://www.ahmedcharles.com >
Sorry, I should have checked the change to the patch. Here is a new patch that fixes it. I found a couple of more places with the same issue. -Nathan On Sat, Sep 25, 2010 at 7:53 AM, Ahmed Charles <ahmedcharles at gmail.com>wrote:> Note: static_cast<bool> doesn't make the warning go away or did you check > that? I just compiled a small test and it didn't work. The recommended > solution is: > > (expression) != 0 > > http://msdn.microsoft.com/en-us/library/b6801kcy.aspx > > It even says: Casting the expression to type bool will not disable the > warning, which is by design. > > On Thu, Sep 23, 2010 at 9:18 AM, Nathan Jeffords <blunted2night at gmail.com>wrote: > >> >> >> On Thu, Sep 23, 2010 at 4:04 AM, Michael Spencer <bigcheesegs at gmail.com>wrote: >> >>> On Thu, Sep 23, 2010 at 2:40 AM, Nathan Jeffords >>> <blunted2night at gmail.com> wrote: >>> > here are a couple of patches to address some warnings >>> > in Microsoft compilers, and a build error in vs2010 in particular >>> > vs2010-errors.patch: >>> > In SelectionDAGISel, std::pairs or pointers are being constructed >>> using a 0 >>> > as an initializer causes an error inside the std::pair constructor. I >>> think >>> > changes to language in support of the nullptr keyword caused this >>> break. >>> >>> Yep, people keep doing this. I've been behind on the trunk for a bit, >>> but I was surprised to see this come up again. This is the correct >>> fix, but please make sure you stick within 80 cols. >>> >>> > vs2010-warnings.patch: >>> > This patch fixes a couple of annoying warnings that show up all over >>> the >>> > place because they are in headers. At least for Microsoft compilers, >>> > conversion of a int to bool requires the compiler to convert non-zero >>> to one >>> > and so the compiler warns of the performance issue. My fix is to >>> explicitly >>> > perform the conversion so the compiler knows I'm aware and doesn't warn >>> me. >>> > I don't know the behavior of other compilers in this respect, so I >>> don't >>> > know if it should be committed. The other solution would be to change >>> the >>> > return type of the accessors function to return ints, though this could >>> move >>> > the warning elsewhere. >>> > -Nathan >>> >>> This fix looks like a case of 3am programming. I think using >>> static_cast<bool> is clearer. >>> >>> - Michael Spencer >>> >> >> here are updated versions >> >> - Nathan >> >> >> _______________________________________________ >> LLVM Developers mailing list >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> >> > > > -- > Ahmed Charles > http://www.ahmedcharles.com >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100925/9554a701/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: vs-warnings.patch Type: application/octet-stream Size: 1873 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100925/9554a701/attachment.obj>