senjuhashirama / pugixml

Automatically exported from code.google.com/p/pugixml
0 stars 0 forks source link

A simple improvment to the source code #28

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
The last item in each enum don't have a "," following it, so if we ever
want to add items to the enum it's very error prone.

I'm using a simple static code checker that chokes on this.

Solution: add a comma ',' every any item in the enums. It's C++ standard,
as avoid maintainance bug when adding items to the enums.

Thanks for such good code anyway

Original issue reported on code.google.com by xryl...@gmail.com on 10 Oct 2009 at 7:51

GoogleCodeExporter commented 9 years ago
There are two problems with the change.

First, it's not of much value, since it does not prevent bugs, it merely 
prevents 
compilation errors. However, it's the same to me, so I'd make the change. 
However, there 
is a second problem - pugixml has a no compilation warning policy on highest 
warning 
levels on maximum number of compilers. There are at least three compilers 
(CodeWarrior 
8.0, Intel C++ 8.0, MinGW 3.4) that issue a warning if there is a trailing 
comma in 
enumerator.

I suggest either changing your code checker or skipping either this check or 
all code 
checks on pugixml.

Original comment by arseny.k...@gmail.com on 10 Oct 2009 at 9:28

GoogleCodeExporter commented 9 years ago
The standard state that trailing comma are valid: 
http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#518
so it's a compiler error to report the warning, the code is valid.

Anyway, I'll keep a patch on my side to add them back, as I don't think you'll 
change
the public enum anyway, the patch will still apply in any new version.

Original comment by xryl...@gmail.com on 10 Oct 2009 at 6:27

GoogleCodeExporter commented 9 years ago
Well, this is a fairly recent defect report; any compiler that supports C++03 
does not 
have to support this, I guess that's the reason for the warning.
The public enum is very unlikely to change indeed.

Original comment by arseny.k...@gmail.com on 10 Oct 2009 at 6:31