Interest in fixing MSVC Level 4 warnings

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

Interest in fixing MSVC Level 4 warnings

Michael Sharpe
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
[hidden email]
https://lists.sourceforge.net/lists/listinfo/quantlib-dev
Reply | Threaded
Open this post in threaded view
|

Re: Interest in fixing MSVC Level 4 warnings

Luigi Ballabio
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 <[hidden email]> 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
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/quantlib-dev
>

------------------------------------------------------------------------------
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_122712
_______________________________________________
QuantLib-dev mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/quantlib-dev
Reply | Threaded
Open this post in threaded view
|

Re: Interest in fixing MSVC Level 4 warnings

Michael Sharpe
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 <[hidden email]> 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 <[hidden email]> 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
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/quantlib-dev
>


------------------------------------------------------------------------------
Master SQL Server Development, Administration, T-SQL, SSAS, SSIS, SSRS
and more. Get SQL Server skills now (including 2012) with LearnDevNow -
200+ hours of step-by-step video tutorials by Microsoft MVPs and experts.
SALE $99.99 this month only - learn more at:
http://p.sf.net/sfu/learnmore_122512
_______________________________________________
QuantLib-dev mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/quantlib-dev
Reply | Threaded
Open this post in threaded view
|

Re: Interest in fixing MSVC Level 4 warnings

Michael Sharpe
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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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
> [hidden email]
> 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
[hidden email]
https://lists.sourceforge.net/lists/listinfo/quantlib-dev
Reply | Threaded
Open this post in threaded view
|

Re: Interest in fixing MSVC Level 4 warnings

Luigi Ballabio
Mike,
    thanks a lot, there's a lot of useful information in there. I'll
try and get at least some of those changes into next release. I'll get
back to you if I have questions.

Later,
    Luigi


On Thu, Feb 28, 2013 at 5:46 AM, Mike Sharpe <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>
>> 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 <[hidden email]> 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
>>> > [hidden email]
>>> > 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
> [hidden email]
> 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_mar
_______________________________________________
QuantLib-dev mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/quantlib-dev
Reply | Threaded
Open this post in threaded view
|

Re: Interest in fixing MSVC Level 4 warnings

Luigi Ballabio
In reply to this post by Michael Sharpe
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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>
>> 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 <[hidden email]> 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
>>> > [hidden email]
>>> > 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
> [hidden email]
> 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_mar
_______________________________________________
QuantLib-dev mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/quantlib-dev
Reply | Threaded
Open this post in threaded view
|

Re: Interest in fixing MSVC Level 4 warnings

Michael Sharpe
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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>
>> 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 <[hidden email]> 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
>>> > [hidden email]
>>> > 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
> [hidden email]
> 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_mar
_______________________________________________
QuantLib-dev mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/quantlib-dev