Re: [Quantlib-dev] Interest in fixing MSVC Level 4 warnings

[Mike S. <ma...@gm...>]

Hi Luigi,

Thank you for a taking a look and getting the changes applied. I've updated
my repository and found no problems with the modifications.

Maintaining a git repository and branch to track these changes seemed more
appropriate since there would have been a lot of small patches
generated. I'm also not particularly concerned with maintaining authorship
information, though I do appreciate that you track this in quantlib for
everyone involved in development of the library.

I look forward to working with you again in the future.

Best,

Mike




On Fri, Mar 22, 2013 at 3:30 AM, Luigi Ballabio <lui...@gm...>wrote:

> Hi Mike,
>     I just committed a bunch of your changes; basically, all of them
> except those related to warning C4512 and related (4510, 4610).  As
> you said, inheriting from boost::noncopyable seems overkill.
> Personally, I'd rather disable the warning if I wanted to get a clean
> build.
> You might want to get the updated code from the repository and check
> that I didn't botch the thing; I just pushed them to the github
> mirror.
>
> By the way: thanks for using github for the thing. It really made
> easier to merge them into my working copy (the only problem I had was
> that sometimes, your editor had changed all the line endings from Unix
> to Windows, but that was easily solved). Unfortunately, passing
> through subversion has erased your authorship information from the
> commits, so I've mentioned it in the commit message instead.
>
> Thanks,
>     Luigi
>
>
> On Thu, Feb 28, 2013 at 5:46 AM, Mike Sharpe <ma...@gm...> wrote:
> > I apologize for the late follow up to looking into getting these warnings
> > fixed, but I have a VC++ 2012 warning free fork of the github mirror at
> > https://github.com/masrtis/quantlib. I've created a branch,
> > VC11Level4WarningFixes, that contains the changes I've made.
> >
> > The vast majority of warnings were C4512: cannot generate an assignment
> > operator for 'class'. This is because of the use of const members and
> > references in classes. To silence these warnings as I was going through,
> I
> > used boost::noncopyable and that seems to be a bit overkill. Ideally it
> > would be nice to fix these warnings by adding a similar class that just
> > disables the assignment operator for classes with const members and maybe
> > keep using boost::noncopyable if a class has reference members, but I
> think
> > the amount of files affected for the amount of benefit gained is pretty
> > small. Because of this, I recommend not merging down the entire branch.
> >
> > I did, however, find some bugs and do some minor refactoring for other
> > warnings. I've created tags on the branch that mark changes that should
> be
> > made and also changes that would be good to get reviewed. A summary of
> these
> > is below.
> >
> > FIX1: Disables the conditional expression is constant warning for the
> > QL_FAIL macro using Microsoft specific pragmas that work correctly with
> > macro expansion. It's worth considering because this change fixes 1,276
> > warnings on its own.
> >
> > FIX2: Removes unreachable code from CPICouponPricer::optionletPriceImp. I
> > wasn't sure if the code was left in for documentation purposes, but if
> so it
> > should be commented out.
> >
> > FIX3: Disables a bunch of warnings that leaked from uBLAS headers.
> >
> > FIX4: Replaced a runtime check of a numeric template parameter with
> > BOOST_STATIC_ASSERT to catch the programming error at compile time
> instead
> > of runtime.
> >
> > FIX5: Fixed uninitialized variable in CPICapFloorTermPriceSurface.
> >
> > FIX6: Fixed uninitialized variable in CPICapFloor.
> >
> > FIX7: Refactored code in Concentrating1dMesher to improve variable scope.
> >
> > REVIEW1: I wasn't sure why RendistatoCalculator was using private
> > inheritance from LazyObject. I fixed the warning by using
> > boost::noncopyable, but I suspect there's a missing public keyword here.
> >
> > FIX8: Disables some more warnings from uBLAS.
> >
> > REVIEW2: There might need to be some code added to the check a return
> value
> > in Garch11::calibrate_r2().
> >
> > FIX9: Remove a duplicated return statement from
> > OptionletStripper::optionletStrikes().
> >
> > FIX10: Added a static_cast around a toupper() call in
> > PeriodParser::parseOnePeriod() to silence a size conversion warning.
> >
> > REVIEW3, REVIEW4: There is a lot of commented out code in function
> bodies in
> > experimental/credit/distribution.cpp and
> > experimental/credit/lossdistribution.cpp. I ended up commenting out what
> > appeared to be similar code that was actually active and therefore
> > generating unreachable code warnings. It's worth reviewing these files to
> > see if the commented out code is still needed.
> >
> > FIX11: Made sure that a const array declaration in
> > math/randomnumbers/primitivepolynomials.h was always considered extern,
> even
> > when it was included in the companion .c file.
> >
> > FIX12: Disabled a conditional expression is constant warning in
> > ZigguratRng::nextGaussian().
> >
> > FIX13: Added files to the VC11 projects that were checked in after I had
> > created my branch.
> >
> > I hope this information is useful. Let me know if there's any questions
> or
> > if I can do anything to help get these changes integrated into quantlib.
> >
> > Mike
> >
> >
> > On Mon, Jan 14, 2013 at 9:20 PM, Mike Sharpe <ma...@gm...> wrote:
> >>
> >> Hi Luigi,
> >>
> >> Thanks for your reply.
> >>
> >> A quick upper bound on the amount of warnings that would need to be
> fixed
> >> is 1800. This is high because VS reports the same warning multiple
> times if
> >> the code in question is in headers, and also because boost has some
> level 4
> >> warnings in uBLAS headers.
> >>
> >> I'm more than willing to use my spare time to generate patches that
> would
> >> fix all the warnings, especially if they appear in headers so people who
> >> include quantlib headers can do so without needing to resort to
> workarounds
> >> if they want to bump up the warning level. When I submit the patches,
> I'll
> >> describe which warning the patch fixes and we can go from there.
> >>
> >> Look for patches over the next week or two.
> >>
> >> Mike
> >>
> >>
> >> On Thu, Jan 10, 2013 at 9:31 AM, Luigi Ballabio <
> lui...@gm...>
> >> wrote:
> >>>
> >>> Hi Mike,
> >>>     given that we won't be able to go entirely warning-free (due to
> >>> QL_FAIL etc.) I wouldn't want you to invest too much time on this.
> >>> But if you want to have a look at the warnings and check if there's
> >>> anything that should be fixed, by all means go ahead.  A count might
> >>> indeed be useful in order to assess if the thing is worth pursuing.
> >>>
> >>> Later,
> >>>     Luigi
> >>>
> >>>
> >>>
> >>> On Sun, Jan 6, 2013 at 11:35 PM, Mike Sharpe <ma...@gm...>
> wrote:
> >>> > Hi all,
> >>> >
> >>> > First, I'd like to start off by saying thanks for your hard work on
> >>> > quantlib! This is my first post to a quantlib mailing list. I've
> >>> > recently
> >>> > developed an interest in mathematical finance and discovered quantlib
> >>> > over
> >>> > the holidays, which has only piqued my interest more.
> >>> >
> >>> > As I'm still new to the codebase and most of the techniques that are
> >>> > used in
> >>> > the code, I'd like to give back in some other way and one of the
> ways I
> >>> > can
> >>> > do that is by looking into fixing some of the level 4 warnings for
> the
> >>> > Microsoft compilers. Because the warnings could appear anywhere and I
> >>> > don't
> >>> > want to cause any difficulties for other work, I wanted to check here
> >>> > to see
> >>> > if there was any interest in seeing these warnings fixed.
> >>> >
> >>> > I'm not necessarily proposing fixing all of the warnings that I've
> seen
> >>> > in
> >>> > the code (QL_FAIL triggers C4127 "conditional expression is constant"
> >>> > because of the do { } while(false) macro guard, for example), but I
> do
> >>> > think
> >>> > it'd a good idea to look through the list and fix some of them. If a
> >>> > count
> >>> > of warnings would help, I can follow up with that soon.
> >>> >
> >>> > Mike
> >>> >
> >>> >
> >>> >
> >>> >
> ------------------------------------------------------------------------------
> >>> > Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
> >>> > MVC, Windows 8 Apps, JavaScript and much more. Keep your skills
> current
> >>> > with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
> >>> > MVPs and experts. ON SALE this month only -- learn more at:
> >>> > http://p.sf.net/sfu/learnmore_123012
> >>> > _______________________________________________
> >>> > QuantLib-dev mailing list
> >>> > Qua...@li...
> >>> > https://lists.sourceforge.net/lists/listinfo/quantlib-dev
> >>> >
> >>
> >>
> >
> >
> >
> ------------------------------------------------------------------------------
> > Everyone hates slow websites. So do we.
> > Make your web apps faster with AppDynamics
> > Download AppDynamics Lite for free today:
> > http://p.sf.net/sfu/appdyn_d2d_feb
> > _______________________________________________
> > QuantLib-dev mailing list
> > Qua...@li...
> > https://lists.sourceforge.net/lists/listinfo/quantlib-dev
> >
>