Review: Spring Cleaning - namespaces, old version removal, etc. #127

Merged
merged 4 commits into from Apr 18, 2012

Conversation

Projects
None yet
2 participants
Owner

lgritz commented Apr 18, 2012

  • Make OSL_NAMESPACE_ENTER/EXIT macros and extensively replace clunky idioms with these.
  • More cleanup related to the recent OIIO namespace alias.
  • Remove support for very old OIIO < 0.10
  • Remove support for LLVM 2.x (3.0 is current and 3.1 is going to release soon)

@fpsunflower fpsunflower commented on the diff Apr 18, 2012

src/include/oslversion.h.in
@@ -68,5 +68,14 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#define OSO_FILE_VERSION_MINOR @OSO_FILE_VERSION_MINOR@
+#ifdef OSL_NAMESPACE
+// Macros to use in each file to enter and exit the right name spaces.
+#define OSL_NAMESPACE_ENTER namespace OSL_NAMESPACE { namespace OSL {
@fpsunflower

fpsunflower Apr 18, 2012

Member

This isn't quite the same as how OIIO does it. Seems fine for the sake of backwards compatibility, but do you have a preferred style going forward?

@lgritz

lgritz Apr 18, 2012

Owner

I was trying to get the "use the macro" part out of the way (I'm bleary-eyed from that) first, then think about whether the definition of that macro is precisely what we want. Once the macro is used instead of doing the idiom separately in each file, it's really easy to change exactly what the namespace nesting is.

But also, I'm not sure why it isn't ok as it is, at least for now. We don't have a good reason to introduce a "version number" level of namespace at this time. So it's always "OSL::", with an optional app-specific wrapper. Is that ok?

@fpsunflower

fpsunflower Apr 18, 2012

Member

The actual symbols in the binary are:

OSL_NAMESPACE::OSL::func
we "use" away the first namespace to get at OSL in the source.

In OIIO the symbols are:

OIIO_NAMESPACE::v123::func
we alias the top level to OIIO and "use" away the second namespace.

In both cases the two levels seem unnecessary. We could get away with just:

PROJECT_NAMESPACE::func

and then the alias:
namespace PRJ = PROJECT_NAMESPACE;

to keep source level compatibility.

As you said, after this cleanup it will be easy to change these details though.

Member

fpsunflower commented Apr 18, 2012

LGTM

@lgritz lgritz merged commit f61741c into imageworks:master Apr 18, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment