#3153 closed enhancement (wontfix)
Define OGRErr as enumerated type
Reported by: | Mateusz Łoskot | Owned by: | Mateusz Łoskot |
---|---|---|---|
Priority: | low | Milestone: | |
Component: | OGR_SF | Version: | svn-trunk |
Severity: | normal | Keywords: | |
Cc: |
Description
This ticket is an enhancement request to re-define OGRErr as enumerated type, similarly to CPLErr.
Thanks to strong-typed definition of CPLErr, it is possible to use this type as an exception type in C++ client code. It can be safely thrown and caught as an exception of distinct type. Unfortunately, this technique is not possible with OGRErr because OGRErr is an alias to int.
Perhaps, upcoming 1.7.0 would be a good opportunity to re-define OGRErr. If clients use OGRErr consistently as they should and they don't assign any values that are out of range of OGR error code, nothing should break. IMHO, making OGRErr a strong and unique type would be more beneficial in long term, especially for C++ users.
Change History (16)
follow-up: 9 comment:1 by , 15 years ago
Priority: | normal → low |
---|
comment:2 by , 15 years ago
I understand, though it's a pity.
It confirms my concerns about C++ API development being kept back and blocked by the fact C API uses common elements with C++ API. It would be better to have C API maintained as a separate library on top of C++ library, but it's probably too late for a revolution.
Anyway, you can expect me to attach again when I spot GDAL 2.0 on the horizon :-)
comment:3 by , 14 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:5 by , 12 years ago
Resolution: | wontfix |
---|---|
Status: | closed → reopened |
As promised, I'm reminding this idea for the GDAL 2.0 which seems to be close. It looks more serious changes to the public API are being approved by the way of Call for discussion for "RFC38 - OGR Faster Open", so I imagine replacing the OGRErr as enum could be accepted too.
comment:6 by , 9 years ago
http://trac.osgeo.org/gdal/wiki/rfc38_ogr_faster_open is targeted to GDAL 2.0. Does it address this issue at the same?
comment:7 by , 9 years ago
I don't see anything related to this request in the wiki:rfc38_ogr_faster_open Means, it's still postponed and waiting for Frank/Even's final decision.
comment:8 by , 9 years ago
I think Frank initial comments should be addressed to understand the consequences of the change.
comment:9 by , 9 years ago
Replying to warmerdam:
I'm concerned that this could trigger an C ABI change on windows if there are any functions using OGRErr that are also using stdcall name decoration which embeds some information on the argument (and return?) types in the function symbol.
Yes, replacing #define with enum effectively changes type affecting prototype of any function taking it as a parameter. Types of enum and int have different mangling type codes (http://www.agner.org/optimize/calling_conventions.pdf).
Hence, I targeted this change for 2.0 where, presumably, ABI changes are allowed.
follow-up: 11 comment:10 by , 8 years ago
Resolution: | → wontfix |
---|---|
Status: | reopened → closed |
The idea has not been accepted. Since, GDAL 2 is out, proposed change lost its potential opportunity. Closing.
comment:11 by , 8 years ago
Replying to mloskot:
The idea has not been accepted.
By whom ? Did you propose this change to the community ?
comment:12 by , 8 years ago
ABI concerns and release of GDAL 2.0, plus longevity of this ticket is enough to conlcude with lack of acceptance. IMO, the ticket itself make a sufficient proposal - changes would be trivial and, obviously, I would have implemented if it, if it was accepted.
comment:13 by , 8 years ago
mloskot, I am in favor of such a change. There is r30136 from 4 weeks ago that has OGRErr as an enum if -DSTRICT_OGRERR_TYPE is set.
follow-up: 15 comment:14 by , 8 years ago
@goatbar Clearly, I've missed that changeset. Thanks for pointing me at it.
@rouault Somewhat, request in this ticket has been addressed. Shall change the status to fixed then?
comment:15 by , 8 years ago
@rouault Somewhat, request in this ticket has been addressed. Shall change the status to fixed then?
Lol, apparently I can't even remember what I did 4 weeks ago. Well, that's just available if a define is set, not in the default case.
Are we sure about FrankW's concern about "C ABI change on windows if there are any functions using OGRErr that are also using stdcall name decoration which embeds some information on the argument" ? Would probably be worth checking/testing. If there's no such C ABI issue, then switching by default to the enumeration would be OK for 2.1.0
If there's an ABI issue, that's a bit more tricky. Wondering if there couldn't have some tricks to have the C ABI use the #define and the C++ ABI using the enumeration... Not pretty...
comment:16 by , 8 years ago
Quick grep-based check reveals the following functions use stdcall convention and return OGRErr:
ogr\ogr_fromepsg.cpp(2135):OGRErr CPL_STDCALL OSRImportFromEPSG( OGRSpatialReferenceH hSRS, int nCode ) ogr\ogr_fromepsg.cpp(2294):OGRErr CPL_STDCALL OSRImportFromEPSGA( OGRSpatialReferenceH hSRS, int nCode ) ogr\ogr_srs_proj4.cpp(1441):OGRErr CPL_STDCALL OSRExportToProj4( OGRSpatialReferenceH hSRS, ogr\ogrspatialreference.cpp(631):OGRErr CPL_STDCALL OSRExportToPrettyWkt( OGRSpatialReferenceH hSRS, char ** ppszReturn, ogr\ogrspatialreference.cpp(685):OGRErr CPL_STDCALL OSRExportToWkt( OGRSpatialReferenceH hSRS, ogr\ogrspatialreference.cpp(854):OGRErr CPL_STDCALL OSRSetAttrValue( OGRSpatialReferenceH hSRS, ogr\ogrspatialreference.cpp(2128):OGRErr CPL_STDCALL OSRSetFromUserInput( OGRSpatialReferenceH hSRS,
I'm concerned that this could trigger an C ABI change on windows if there are any functions using OGRErr that are also using stdcall name decoration which embeds some information on the argument (and return?) types in the function symbol. If so, I would be inclined against this change.