From: John P. <pet...@ta...> - 2010-06-21 16:24:42
|
Hi, Builds of PETSc with "Sieve" enabled (configure with --with-sieve, sieve is their unstructured mesh data structure) contain a Mesh struct which apparently conflicts with ours. (I haven't seen this but a TACC colleague recently came across the issue.) Compiling C++ (in optimized mode) src/numerics/petsc_matrix.C... In file included from src/numerics/petsc_matrix.C:28: .../workspace/libmesh/include/base/dof_map.h:49: error: using typedef-name ‘Mesh’ after ‘class’ .../workspace/petsc-dev/include/petscmesh.h:26: error: ‘Mesh’ has a previous declaration here make: *** [src/numerics/petsc_matrix.i386-apple-darwin10.3.1.opt.o] Error 1 This isn't a widespread problem now since Sieve is an optional package, but it seems like it could easily become one in the future, if not for Mesh then for other names in PETSc. I was hoping we could discuss what options are available to us... one seems to be namespacing the entire library. I think this might be the "right" thing to do, but it obviously represents a relatively big, probably API-breaking change to user code. Thoughts? -- John |
From: Roy S. <roy...@ic...> - 2010-06-21 17:02:49
|
On Mon, 21 Jun 2010, John Peterson wrote: > In file included from src/numerics/petsc_matrix.C:28: > .../workspace/libmesh/include/base/dof_map.h:49: error: > using typedef-name ‘Mesh’ after ‘class’ > .../workspace/petsc-dev/include/petscmesh.h:26: error: > ‘Mesh’ has a previous declaration here > make: *** [src/numerics/petsc_matrix.i386-apple-darwin10.3.1.opt.o] Error 1 > > This isn't a widespread problem now since Sieve is an optional > package, but it seems like it could easily become one in the future, > if not for Mesh then for other names in PETSc. I was hoping we could > discuss what options are available to us... one seems to be > namespacing the entire library. I think this might be the "right" > thing to do, but it obviously represents a relatively big, probably > API-breaking change to user code. Thoughts? My first reaction: "What kind of jerks monopolize a common word like Mesh in the global namespace!?! Other than us, of course!" So yeah, my vote is to namespace the entire library. We should have done that a long time ago, except as you point out it will be an API-breaking change. Not too nasty an API-breaking change, though, since non-Sieve users will be able to get away with a single "using namespace libMesh;" as the fix. I'd been kind of hoping that we'd be able to get away with the same in library code (in our .C files, albeit not our .h files), but I guess if even PETSc might have new encroaching identifiers then we've got to be careful - anything that includes a third party header (even indirectly...) shouldn't use any non-explicitly-namespaced symbols. We should be just about ready to put out an 0.6.5, yes? Biggest current obstacle is testing of that libHilbert patch? In that case let's make 0.6.5 non-namespaced and release a namespaced 0.6.6 soon after? --- Roy |
From: Jed B. <je...@59...> - 2010-06-21 17:25:49
|
There are quite few Sieve users, the next release will fix this overly public namespace. But don't let that disuade you from adopting a more precise libMesh namespace. Jed |
From: John P. <pet...@ta...> - 2010-06-21 17:49:41
|
On Mon, Jun 21, 2010 at 11:35 AM, Roy Stogner <roy...@ic...> wrote: > > So yeah, my vote is to namespace the entire library. We should have > done that a long time ago, except as you point out it will be an > API-breaking change. Not too nasty an API-breaking change, though, > since non-Sieve users will be able to get away with a single > "using namespace libMesh;" as the fix. In source files yes, but if they've written their own headers (as you mention below) I think the general rule of thumb is you don't put "using" declarations there. So, the more extensive the user project, the more extensive the change will be...hopefully Derek will chime in about how much the proposed change would affect his work. > I'd been kind of hoping that we'd be able to get away with the same in > library code (in our .C files, albeit not our .h files), but I guess > if even PETSc might have new encroaching identifiers then we've got to > be careful - anything that includes a third party header (even > indirectly...) shouldn't use any non-explicitly-namespaced symbols. Right, and what if someone wants to write Sieve code (if this makes sense?) within an otherwise libmesh source file, foo.C? <code> #include <petscmesh.h> #include "mesh.h" // LibMesh's Mesh, assume properly namespaced using namespace LibMesh; Mesh mesh; // How to access Sieve's Mesh? </code> With the using declaration, they'd have no way to access it, right? > We should be just about ready to put out an 0.6.5, yes? Biggest > current obstacle is testing of that libHilbert patch? In that case > let's make 0.6.5 non-namespaced and release a namespaced 0.6.6 soon > after? OK, sounds like a good plan to me. -- John |
From: Boyce G. <gri...@ci...> - 2010-06-21 18:27:23
|
On 6/21/10 1:49 PM, John Peterson wrote: > On Mon, Jun 21, 2010 at 11:35 AM, Roy Stogner<roy...@ic...> wrote: >> I'd been kind of hoping that we'd be able to get away with the same in >> library code (in our .C files, albeit not our .h files), but I guess >> if even PETSc might have new encroaching identifiers then we've got to >> be careful - anything that includes a third party header (even >> indirectly...) shouldn't use any non-explicitly-namespaced symbols. > > Right, and what if someone wants to write Sieve code (if this makes > sense?) within an otherwise libmesh source file, foo.C? > > <code> > #include<petscmesh.h> > #include "mesh.h" // LibMesh's Mesh, assume properly namespaced > using namespace LibMesh; > > Mesh mesh; // How to access Sieve's Mesh? > </code> > > With the using declaration, they'd have no way to access it, right? Assuming it is in the global namespace, I believe that one could do: Mesh libmesh_mesh; ::Mesh sieve_mesh; -- Boyce |
From: John P. <pet...@ta...> - 2010-06-21 18:08:37
|
On Mon, Jun 21, 2010 at 12:54 PM, Boyce Griffith <gri...@ci...> wrote: > > Assuming it is in the global namespace, I believe that one could do: > > Mesh libmesh_mesh; > ::Mesh sieve_mesh; Oh yeah, duh! Sorry about the dumb question forgot about naked double-colon... -- John |
From: Derek G. <fri...@gm...> - 2010-06-21 21:10:51
|
On Jun 21, 2010, at 11:49 AM, John Peterson wrote: > In source files yes, but if they've written their own headers (as you > mention below) I think the general rule of thumb is you don't put > "using" declarations there. So, the more extensive the user project, > the more extensive the change will be...hopefully Derek will chime in > about how much the proposed change would affect his work. Well... on the one hand I agree that it might be the right thing to do. On the other hand libMesh is just TOO pervasive in it's users codes. What I mean by that is that just about every line would either have to have a libmesh:: somewhere in it... or every file would have to have "using" in it. I'm not against it... but we do have on the order of hundreds of .C files (not to mention all of OUR users would also have to put this in all of their implementation files). I'm not sold on either direction... but I do enjoy not having to deal with a libmesh namespace now. But laziness doesn't justify anything. I guess my initial vote will be to wait for Petsc to implement a namespace for Sieve and then reevaluate. Derek |
From: Roy S. <roy...@ic...> - 2010-06-21 21:29:32
|
On Mon, 21 Jun 2010, Derek Gaston wrote: > I'm not sold on either direction... but I do enjoy not having to > deal with a libmesh namespace now. But laziness doesn't justify > anything. How about we decide not to decide? "using namespace libMesh;" in libmesh_common.h, wrapped in something like #ifndef LIBMESH_USE_NAMESPACE which gets defined by something like ./configure --disable-separate-namespace Then for the next release or two, users' code works as-is, but they have the option of easily making the libMesh namespace explicit if they want to integrate with another library which might clash. For releases after that, we'd make the namespace explicit by default, but users could still configure with that disabled for backward compatibility. This would seem to be the ideal solution... but the trouble with compile-time options is that you can only test one at a time, so they're easier to miss until you see the bright red angry box in a BuildBot regression report. It's too easy for a developer to inadvertently check in a revision that works with their own configure options but breaks with others, and I could see "prepend libMesh:: in .h file" becoming a common bugfix if we have major developers relying on --disable-separate-namespace for too long. --- Roy |
From: Derek G. <fri...@gm...> - 2010-06-21 22:20:02
|
On Jun 21, 2010, at 3:29 PM, Roy Stogner wrote: > > On Mon, 21 Jun 2010, Derek Gaston wrote: > >> I'm not sold on either direction... but I do enjoy not having to >> deal with a libmesh namespace now. But laziness doesn't justify >> anything. > > How about we decide not to decide? > > "using namespace libMesh;" in libmesh_common.h, wrapped in > something like > #ifndef LIBMESH_USE_NAMESPACE > which gets defined by something like > ./configure --disable-separate-namespace If this works then users (like myself) who like not needing to use libmesh:: can put "using namespace libMesh" in our own header files that get included. I have one root header file for my library (much like libmesh_common.h) where I can put this and all of my code will be fine (as will my user's code). That's not an objection to your suggestion... that sounds fine to me too. But if the fix is that trivial... it might not require it. Derek |
From: John P. <pet...@ta...> - 2010-06-21 22:04:44
|
On Mon, Jun 21, 2010 at 4:29 PM, Roy Stogner <roy...@ic...> wrote: > > On Mon, 21 Jun 2010, Derek Gaston wrote: > >> I'm not sold on either direction... but I do enjoy not having to >> deal with a libmesh namespace now. But laziness doesn't justify >> anything. > > How about we decide not to decide? Then, according to the awesome band known as Rush, "you still have made a choice" ;-) > "using namespace libMesh;" in libmesh_common.h, wrapped in > something like > #ifndef LIBMESH_USE_NAMESPACE > which gets defined by something like > ./configure --disable-separate-namespace Would it really be that simple? That doesn't put things like our Mesh into a libMesh namespace. It just pulls the libMesh namespace into the existing compilation unit, that namespace will be empty (except for the few things we have in it now.) Perhaps I misunderstood, but I don't think we can configure on/off namespacing the entire library in a couple lines... -- John |
From: Roy S. <roy...@ic...> - 2010-06-21 22:10:08
|
On Mon, 21 Jun 2010, John Peterson wrote: >> "using namespace libMesh;" in libmesh_common.h, wrapped in >> something like >> #ifndef LIBMESH_USE_NAMESPACE >> which gets defined by something like >> ./configure --disable-separate-namespace > > Would it really be that simple? That doesn't put things like our Mesh > into a libMesh namespace. Oh, not on your life. We'd have to do that by hand, in every header file (and many source files), changing tens of thousands of lines of code. All the optional "using namespace libMesh;" would do is bring those bits back into the global namespace for backwards compatibility. --- Roy |
From: Derek G. <fri...@gm...> - 2010-06-21 22:11:28
|
On Jun 21, 2010, at 4:10 PM, Roy Stogner wrote: > > On Mon, 21 Jun 2010, John Peterson wrote: > >>> "using namespace libMesh;" in libmesh_common.h, wrapped in >>> something like >>> #ifndef LIBMESH_USE_NAMESPACE >>> which gets defined by something like >>> ./configure --disable-separate-namespace >> >> Would it really be that simple? That doesn't put things like our Mesh >> into a libMesh namespace. > > Oh, not on your life. We'd have to do that by hand, in every header > file (and many source files), changing tens of thousands of lines of > code. Yikes! I'm seriously doubting the usefulness of this decision... Derek |
From: John P. <pet...@ta...> - 2010-06-21 22:16:02
|
On Mon, Jun 21, 2010 at 5:11 PM, Derek Gaston <fri...@gm...> wrote: > On Jun 21, 2010, at 4:10 PM, Roy Stogner wrote: > >> >> On Mon, 21 Jun 2010, John Peterson wrote: >> >>>> "using namespace libMesh;" in libmesh_common.h, wrapped in >>>> something like >>>> #ifndef LIBMESH_USE_NAMESPACE >>>> which gets defined by something like >>>> ./configure --disable-separate-namespace >>> >>> Would it really be that simple? That doesn't put things like our Mesh >>> into a libMesh namespace. >> >> Oh, not on your life. We'd have to do that by hand, in every header >> file (and many source files), changing tens of thousands of lines of >> code. > > Yikes! I'm seriously doubting the usefulness of this decision... Yeah, but there's no two ways about it if we want to start being "good" computer library citizens, which I think is a laudable goal. The explicitly-namespaced code will be pretty ugly and verbose though. What I'd really like is some kind of GCC plugin that, on an undefined type error, you could have it modify the source and try Foo::Type instead, and if that worked, to carry on. -- John |
From: Kirk, B. (JSC-EG311) <ben...@na...> - 2010-06-21 22:48:49
|
> Yeah, but there's no two ways about it if we want to start being > "good" computer library citizens, which I think is a laudable goal. > The explicitly-namespaced code will be pretty ugly and verbose though. > > What I'd really like is some kind of GCC plugin that, on an undefined > type error, you could have it modify the source and try Foo::Type > instead, and if that worked, to carry on. How about renaming the offending class 'TheClassFormerlyKnownAsMesh' and rely on the old C-style namespace-through-obfuscation? We can then add a Mesh wrapper class which yells deprecated in the constructor... Just a (not very useful) thought. |
From: Roy S. <roy...@ic...> - 2010-06-21 22:35:54
|
On Mon, 21 Jun 2010, Kirk, Benjamin (JSC-EG311) wrote: > How about renaming the offending class 'TheClassFormerlyKnownAsMesh' > > and rely on the old C-style namespace-through-obfuscation? No way. With "libMesh::Mesh" we can use the namespace features and probably get away with a few lines of code in every file (deep breath in, Derek, deep breath out...); with "libMesh_Mesh" there really wouldn't be a way to avoid a 20K line patch. --- Roy |
From: Kirk, B. (JSC-EG311) <ben...@na...> - 2010-06-21 22:51:20
|
> No way. With "libMesh::Mesh" we can use the namespace features and > probably get away with a few lines of code in every file (deep breath > in, Derek, deep breath out...); with "libMesh_Mesh" there really > wouldn't be a way to avoid a 20K line patch. I seem to recall that *even in a header file* int foo () { using namespace Bar; return 0; } only imports Bar into the scope of the function, not everything that includes the header file. Way back some compilers didn't do the right thing and instead imported the namespace globally. Not that I don't like 20K patches, but this option might reduce it to 5K? ;-) |
From: Derek G. <fri...@gm...> - 2010-06-21 22:59:38
|
On Jun 21, 2010, at 4:35 PM, Roy Stogner wrote: > No way. With "libMesh::Mesh" we can use the namespace features and > probably get away with a few lines of code in every file (deep breath > in, Derek, deep breath out...); with "libMesh_Mesh" there really > wouldn't be a way to avoid a 20K line patch. Honestly, I'm not that worried about Mesh. In fact, if you just want to stick that guy into the libMesh namespace and leave everything else alone... that's fine with me. I'm more worried about things like TypeVector. Think about code that could look like this: std::vector<std::vector<libMesh::TypeVector> > > triple_vec; ... libMesh::TypeVector temp = triple_vec[0][1]; Well... now that I write it out it doesn't look too horrible.... hmmm. In fact, it kind of looks like the "right thing" to do. Double hmmmm. I'll leave it up to you guys. I might still stick "using namespace libMesh" in one of my main header files though. Mostly to shield my users from needing lbMesh:: all over the place (or "using"). But you guys should do what's right for the library. Derek |
From: Roy S. <roy...@ic...> - 2010-06-28 22:55:54
|
On Mon, 21 Jun 2010, Roy Stogner wrote: > No way. With "libMesh::Mesh" we can use the namespace features and > probably get away with a few lines of code in every file (deep breath > in, Derek, deep breath out...); with "libMesh_Mesh" there really > wouldn't be a way to avoid a 20K line patch. Okay, let's see if my current patch passes review: It's over 3000 lines, but all but a handful of lines are either whitespace, braces, "namespace libMesh {", "}" I've added "using namespace libMesh;" to the example files. That way future apps based on them will survive when (either as a near-term option or a long-term default) we stop importing the namespace by default. There's a "using namespace libMesh;" in the library itself, which is enough that nearly all code works unchanged. There are at least a couple exceptions, which require ugly workarounds if you want to be compatible with both pre-patch and post-patch library versions: // forward declarations of libMesh classes: #include "libmesh_base.h" #ifdef LIBMESH_USE_SEPARATE_NAMESPACE namespace libMesh { class Point; class Elem; } #else class Point; class Elem; #endif // template specializations of libMesh classes: #ifdef LIBMESH_USE_SEPARATE_NAMESPACE libMesh::FactoryImp<HeatTransfer, Physics> heat_transfer_factory ("Heat Transfer"); #else FactoryImp<HeatTransfer, Physics> heat_transfer_factory ("Heat Transfer"); #endif Comments? This isn't as bad as Derek was fearing, but neither is it the "everything works without changes" situation I'd hoped for. --- Roy |
From: Derek G. <fri...@gm...> - 2010-07-07 23:26:47
|
Well... my transition to the libMesh namespace wasn't perfectly smooth... but wasn't terrible. The largest hangup? We do a lot of forward declaring of libMesh classes in header files (so that they aren't included everywhere)... and all of those forward declarations had to be wrapped in a "namespace libMesh {}". That's not too bad though. The other main gotcha was for template specializations of Paramters::print()... which we do because there are types for which the default print() function doesn't work. These too had to be wrapped in the libMesh namespace. The GOOD part is that all of these changes took place in our library code... and NO user code had to be changed at all. That was the main thing I was worried about... but it appears that we have sufficiently insulated our users from libMesh to the point where this change didn't matter ;-) Just thought I would report back in case anyone else ran into these types of issues. Derek On Mon, Jun 28, 2010 at 4:55 PM, Roy Stogner <roy...@ic...>wrote: > > On Mon, 21 Jun 2010, Roy Stogner wrote: > > No way. With "libMesh::Mesh" we can use the namespace features and >> probably get away with a few lines of code in every file (deep breath >> in, Derek, deep breath out...); with "libMesh_Mesh" there really >> wouldn't be a way to avoid a 20K line patch. >> > > Okay, let's see if my current patch passes review: > > It's over 3000 lines, but all but a handful of lines are either > whitespace, braces, "namespace libMesh {", "}" > > I've added "using namespace libMesh;" to the example files. That way > future apps based on them will survive when (either as a near-term > option or a long-term default) we stop importing the namespace by > default. > > There's a "using namespace libMesh;" in the library itself, which is > enough that nearly all code works unchanged. > > There are at least a couple exceptions, which require ugly workarounds > if you want to be compatible with both pre-patch and post-patch > library versions: > > // forward declarations of libMesh classes: > #include "libmesh_base.h" > #ifdef LIBMESH_USE_SEPARATE_NAMESPACE > namespace libMesh { > class Point; > class Elem; > } > #else > class Point; > class Elem; > #endif > > // template specializations of libMesh classes: > #ifdef LIBMESH_USE_SEPARATE_NAMESPACE > libMesh::FactoryImp<HeatTransfer, Physics> heat_transfer_factory ("Heat > Transfer"); > #else > FactoryImp<HeatTransfer, Physics> heat_transfer_factory ("Heat Transfer"); > #endif > > > Comments? This isn't as bad as Derek was fearing, but neither is it > the "everything works without changes" situation I'd hoped for. > --- > Roy > |
From: Roy S. <roy...@ic...> - 2010-08-24 18:45:22
|
I just ran across a third exception to the "all code works unchanged" rule; I figured I might as well post it to the list where search engines can find it to help people upgrading in the future. With that "using namespace libMesh;" active, some identifiers that were previously wrapped safely in the libMesh namespace can be brought into conflict with the same identifiers in the global namespace. I just needed to change an old application's "bool initialized;" to "bool ICs_initialized;" to work around a conflict with the library's libMesh::initialized() function. --- Roy On Mon, 28 Jun 2010, Roy Stogner wrote: > There's a "using namespace libMesh;" in the library itself, which is > enough that nearly all code works unchanged. > > There are at least a couple exceptions, which require ugly workarounds > if you want to be compatible with both pre-patch and post-patch > library versions: > > // forward declarations of libMesh classes: > #include "libmesh_base.h" > #ifdef LIBMESH_USE_SEPARATE_NAMESPACE > namespace libMesh { > class Point; > class Elem; > } > #else > class Point; > class Elem; > #endif > > // template specializations of libMesh classes: > #ifdef LIBMESH_USE_SEPARATE_NAMESPACE > libMesh::FactoryImp<HeatTransfer, Physics> heat_transfer_factory ("Heat > Transfer"); > #else > FactoryImp<HeatTransfer, Physics> heat_transfer_factory ("Heat Transfer"); > #endif |