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 |
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 |
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, ------------------------------------------------------------------------------ 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 |
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:
------------------------------------------------------------------------------ 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 |
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 |
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 |
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, ------------------------------------------------------------------------------ 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 |
Free forum by Nabble | Edit this page |