Stefan Seefeld wrote:

Yuval Ronen wrote:

...

1. option 'Og' has been deprecated and will be removed in a future release

That one can probably be fixed in the build system.

I hope it will be.

...

2. You have used a std:: construct that is not safe. See documentation on how to use the Safe Standard C++ Library 3. The POSIX name for this item is deprecated. Instead, use the ISO C++ conformant name: ... 4. This function or variable may be unsafe. Consider using ... instead.

I'm quite sure that they don't indicate any bug (am I right?), but I still don't think they should show up like this and intimidate users...

Seriously, it appears Microsoft set up a list of standard (ISO C++, POSIX, ...) functions that they consider unsafe, and they issue warnings whenever one of them is used. Some workarounds for these were discussed here before, such as pragma guards to mask the warnings inside boost code. No change has been committed to that effect so far.

Yes, I've heard of Microsoft's "deprecation campaign". I think that compiling Boost (as opposed to compiling user code that uses Boost) should not emit any of these warning. I also think it should be quite easy to do. Well, I hope it will be fixed for the next version... Yuval

Douglas Gregor wrote:

On Dec 7, 2005, at 8:35 AM, Yuval Ronen wrote:

...

2. You have used a std:: construct that is not safe. See documentation on how to use the Safe Standard C++ Library 3. The POSIX name for this item is deprecated. Instead, use the ISO C++ conformant name: ... 4. This function or variable may be unsafe. Consider using ... instead.

These are bogus warnings from the compiler. Please see:

http://www.boost.org/tools/build/v1/vc-8_0-tools.html

Then why not disable them? Note that I'm talking about building Boost itself, not building something that uses Boost.

Alexander Ivanov wrote:

defining symbol _CRT_SECURE_NO_DEPRECATE should suppress most of these bogus warnings.

Yes, I know. What I'm saying is that I can define this in *my* project files. I can't do it in *Boost* project files. My point is that the Boost build system should define it for compiling Boost. I can handle my compilations, I'd like to see Boost handle its own compilation... No flames intended, of course, and I sincerely apologize if it looks like it...

Bronek Kozicki wrote:

Yes, we (or personally me) will try to handle this in boost 1.34 , but I it will require tweaks in jamfiles that I wanted to avoid in 1.33.1

Ok. Thank you very much!

Stefan Seefeld wrote:

Yuval Ronen wrote:

...

Alexander Ivanov wrote:

...

defining symbol _CRT_SECURE_NO_DEPRECATE should suppress most of these bogus warnings.

Yes, I know. What I'm saying is that I can define this in *my* project files. I can't do it in *Boost* project files. My point is that the Boost build system should define it for compiling Boost. I can handle my compilations, I'd like to see Boost handle its own compilation... No flames intended, of course, and I sincerely apologize if it looks like it...

Given that most of boost code is in headers, and thus is processed with your own compilation, I don't think the distinction between 'theirs' and 'yours' is as clear as you make it sound.

I think the distinction in this context is very clear. Whatever is done with 'bjam' to compile the non-header-only parts of Boost is 'theirs'. All the rest is 'mine'.

In fact, one solution that has been discussed here before was to wrap all boost code with pragma guards to disable the warning before boost code is parsed, and re-enables them when the parser gets back to your code.

That, however, is quite intrusive and requires a lot of work.

This is a whole new ball game. Wrapping all Boost code in pragma guard has its advantages and disadvantages. I won't argue here in favour or against it. All I'm saying that invoking bjam to compile Boost itself, shouldn't emit any warnings. This is something that I can't imagine anyone saying is wrong. Regard, Yuval

David Abrahams wrote:

Yuval Ronen <ronen_yuval@yahoo.com> writes:

...

This is a whole new ball game. Wrapping all Boost code in pragma guard has its advantages and disadvantages. I won't argue here in favour or against it. All I'm saying that invoking bjam to compile Boost itself, shouldn't emit any warnings. This is something that I can't imagine anyone saying is wrong.

No disagreement with that. However, you missed this part of the discussion *before* the release:

http://lists.boost.org/Archives/boost/2005/11/96241.php

where arguments like this one apparently won out:

http://lists.boost.org/Archives/boost/2005/11/96212.php

Nothing in what I said contradicts these winning arguments. Defining _SCL_SECURE_NO_DEPRECATE (or whatever the macro is) in a Boost config file, as was suggested and rejected in the second link, is far far more drastic then my simple suggestion. All I said was that building Boost itself should define this macro (by default!) to supress these examined-by-us-and-decided-to-be-harmless warnings. If anyone thinks of another way to supress these warning, then by all means, go for it. I don't care. All I preach for is a clean, welcoming Boost installation. Something that won't give users the (extremely) wrong feeling of sloppy code. There is nothing here that affects users code, getting involved in politics, or taking a stand in a controversial matter. On the other, I feel this discussion has grown much more than I anticipated. I have no wish to become a nagging burden on this mailing list, so I'll just shut up. If I couldn't convince you by now, then I guess I just can't do it, or maybe even worse, God forbid, that I'm wrong... ;-) Regards, Yuval

Not all of the warnings are benign. For example I get the following when compiling regex under vc8 with /analyze:

h:\source code\boost\boost_1_33_1\libs\regex\src\wide_posix_api.cpp(159) : warning C6200: Index '20' is out of valid index range '0' to '15' for non-stack buffer 'wchar_t const * * boost::`anonymous namespace'::wnames' h:\source

That's a bug in [W]regerror: you won't hit it unless you're using the C API's, but it's still very worrying, thanks for reporting that.

code\boost\boost_1_33_1\boost\regex\v4\basic_regex_creator.hpp(1183) : warning C6297: Arithmetic overflow: 32-bit value is shifted, then cast to 64-bit value. Results might not be an expected value

Borderline bug, I'll fix it anyway.

h:\source code\boost\boost_1_33_1\boost\regex\v4\basic_regex_creator.hpp(1203) : warning C6297: Arithmetic overflow: 32-bit value is shifted, then cast to 64-bit value. Results might not be an expected value

Ditto.

h:\source code\boost\boost_1_33_1\libs\regex\src\posix_api.cpp(146) : warning C6200: Index '20' is out of valid index range '0' to '15' for non-stack buffer 'char const * * boost::`anonymous namespace'::names' h:\source code\boost\boost_1_33_1\libs\regex\src\posix_api.cpp(132) : warning C6385: Invalid data: accessing 'char const * * boost::`anonymous namespace'::names', the readable size is '64' bytes, but '84' bytes might be read: Lines: 126, 127, 129, 130, 132 h:\source

Same bug as above, will fix.

code\boost\boost_1_33_1\boost\regex\v4\w32_regex_traits.hpp(537) : warning C6385: Invalid data: accessing 'unsigned long const * const `private: unsigned long __thiscall boost::re_detail::w32_regex_traits_implementation<char>::lookup_classname_imp(char const *,char const *)const '::`2'::masks', the readable size is '80' bytes, but '84' bytes might be read: Lines: 505, 528, 530, 531, 532, 535, 536, 537 h:\source

I can't see the issue there at all - and I've verified that the code is fully covered by the test cases.

code\boost\boost_1_33_1\boost\regex\v4\basic_regex_creator.hpp(1203) : warning C6297: Arithmetic overflow: 32-bit value is shifted, then cast to 64-bit value. Results might not be an expected value h:\source code\boost\boost_1_33_1\boost\regex\v4\basic_regex_creator.hpp(576) : warning C6246: Local declaration of 'c3' hides declaration of the same name in outer scope. For additional information, see previous declaration at line '565' of 'h:\source code\boost\boost_1_33_1\boost\regex\v4\basic_regex_creator.hpp':

Harmless, but it's worth fixing anyway.

Lines: 565 Generally I have found these warnings to be right on with my own code. :-) While some of the warnings represent MS design decisions (e.g. deprecating certain SL functions), code analysis as a whole is very helpful and boost might benefit from it.

Yep, it would be worthwhile running one of our regression tests with these turned on if any of our testers are able to do that. Is it possible to turn analyse warnings into errors, without turning all the others into errors? Thanks for the report. John.

This is mainly for the benefit of Google, but here's the full run-down on the /analyse warnings and their fixes: h:\source code\boost\boost_1_33_1\libs\regex\src\wide_posix_api.cpp(159) : warning C6200: Index '20' is out of valid index range '0' to '15' for non-stack buffer 'wchar_t const * * boost::`anonymous namespace'::wnames' It's a bug, it only strikes if you use regerr with the flags REG_ITOA or REG_ATOI set: these are debugging helpers added for compatibility with the BSD regex package, POSIX conforming code is not effected. The bug appears to have been present for several releases without anyone reporting it, so maybe I should have just removed that code (!), however fixes and enhanced test cases are in the diffs below. Frankly this one is embarrassing. h:\source code\boost\boost_1_33_1\boost\regex\v4\basic_regex_creator.hpp(1183) : warning C6297: Arithmetic overflow: 32-bit value is shifted, then cast to 64-bit value. Results might not be an expected value Another bug, but only strikes in certain rare situations when you have more than 32 repetition operators in the expression (that's not the only requirement though). Test cases and fix in the diffs below. h:\source code\boost\boost_1_33_1\boost\regex\v4\basic_regex_creator.hpp(1203) : warning C6297: Arithmetic overflow: 32-bit value is shifted, then cast to 64-bit value. Results might not be an expected value As above. h:\source code\boost\boost_1_33_1\boost\regex\v4\w32_regex_traits.hpp(537) : warning C6385: Invalid data: accessing 'unsigned long const * const `private: unsigned long __thiscall boost::re_detail::w32_regex_traits_implementation<char>::lookup_classname_imp(char const *,char const *)const '::`2'::masks', the readable size is '80' bytes, but '84' bytes might be read: Lines: 505, 528, 530, 531, 532, 535, 536, 537 I can't understand this one, in fact I'm certain it's not a bug :-) George: if you add an extra 0 to the data-array does that suppress the warning, or just shift it? I have a suspicion the analyser is getting confused by the +1 added to the index (this is correct the original value is in the range [-1, max-1), so does need to be shifted by one). h:\source code\boost\boost_1_33_1\boost\regex\v4\basic_regex_creator.hpp(576) : warning C6246: Local declaration of 'c3' hides declaration of the same name in outer scope. For additional information, see previous declaration at line '565' of 'h:\source code\boost\boost_1_33_1\boost\regex\v4\basic_regex_creator.hpp': Lines: 565 Basically harmless, but fixed anyway. HTH, John.

I think that the problem is at regex_traits_defaults.hpp line 191:

static const character_pointer_range<charT>* ranges_end = ranges + (sizeof(ranges)/sizeof(ranges[0]));

Your algorithm doesn't allow for compiler data alignment. With 8-byte data alignment (now the default with VC8) sizeof(ranges) == ((19*4)/8+1)*8 == 80. The compiler warning is correct get_default_class_id() may return a value between -1 and 20. Add 1 to this return value and you get a range of 0-21 which exceeds the size of masks in lookup_classname_imp().

I don't believe that's correct: for one thing the trick: #define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0])) is very well known and heavily used (in the Linux kernal for one thing), I can't believe that MS would break such a well used idiom. For another adding a static_assert in there *does* pass, and in any case each element in the array is 8-bytes (32-bit pointers) or 16-bytes (64-bit pointers), so the alignment and size must be a multiple of 8 even if there are an odd number of elements.

Note that similar code at win32_regex_traits.hpp line 536 is not affected because the size of masks is 8-byte aligned (for now at least). 64-bit compilers require 8-byte alignment so I assume that this code will not compile correctly on those platforms as well.

If you must use this construct ("sizeof(ranges)/sizeof(ranges[0]") then static assert that this formula yields the same size as your array. I see that there are a few other non-trivial warnings in my build of the regex library. My examples were not intended to be exhaustive but to illustrate the utility of incorporating this into boost's QA.

Understood, but what other warnings were you seeing? I'll ask about testing with /analyse, but I do wonder whether the compile times may be too long for it to be practical? Thanks, John.

You sure about that, John? - last I heard, 32-bit pointers were 4 bytes & 64-bit pointers were 8 bytes - or is this the Intel 4004 cross-compiler :-)

:-) I wasn't very clear was I? Each element in the array is a *pair* of pointers, so yes the *pair* is 8 bytes if the pointers are 32-bits (2 x 4 byte pointers). John.

The only time that I get warning 6385 is when id is equal to or exceeds the size of the array. It is a little strange in that the warning only occurs when the line beginning with _ASSERTE is added. There should be an error even without the debug assertion. Nevertheless this suggests that there may be a case where re_detail::get_default_class_id(() returns a value in excess of 18 leading to the warning. The following pseudo-code compiles without warning or error and has the slight advantage if protecting release as well as debug builds: std::size_t id = (1 + re_detail::get_default_class_id(p1, p2)); if(id < sizeof(masks) / sizeof(masks[0])) return masks[id]; return masks[0];

I still think that's redundant code, but I'll make the change anyway.

...

Understood, but what other warnings were you seeing? <

Here is the build log which already incorporates the above code fragment: http://users.erols.com/gmgarner/boost/BuildLog_regex.txt.

OK thanks, there's only one warning in there I hadn't seen before (and isn't fixed now), and that's this one: h:\source code\boost\boost_1_33_1\boost\type_traits\is_member_function_pointer.hpp(67) : error C6334: sizeof operator applied to an expression with an operator might yield unexpected results Looking up the warning on MSDN it says: "This warning indicates a misuse of the sizeof operator. The sizeof operator, when applied to an expression, yields the size of the type of the resulting expression. " But that's exactly what we want it to do! So no fixes possible there. Thanks for all the feedback on this, John.

...

Would you kindly post the diffs to save me some download time?

Attached are all the changes to current cvs HEAD compared to the last release. John.

sorry, i wasn't clear on the first place. you can use following to control boost builds: bjam "-sTOOLS=vc-8_0" ... "-sBUILD=<define>_CRT_SECURE_NO_DEPRECAT" ... On 12/7/05, Yuval Ronen <ronen_yuval@yahoo.com> wrote:

Alexander Ivanov wrote:

...

defining symbol _CRT_SECURE_NO_DEPRECATE should suppress most of these bogus warnings.

Yes, I know. What I'm saying is that I can define this in *my* project files. I can't do it in *Boost* project files. My point is that the Boost build system should define it for compiling Boost. I can handle my compilations, I'd like to see Boost handle its own compilation... No flames intended, of course, and I sincerely apologize if it looks like it...

_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

yes, this one can not be fixed without modifying tool-set/.jam. so it will have to wait until next release. On 12/7/05, Bronek Kozicki <brok@rubikon.pl> wrote:

Alexander Ivanov wrote:

...

sorry, i wasn't clear on the first place. you can use following to control boost builds:

bjam "-sTOOLS=vc-8_0" ... "-sBUILD=<define>_CRT_SECURE_NO_DEPRECAT" ...

There is still a problem with deprecated option /Og . I will look into it

B. _______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost