Scoping problem?

[msohnius]

Hi all,

25+ years in this trade have taught me never to suspect a compiler bug until lots of others have had a very good look at the code!

So, here we go:

File 1.C:

#include "2.h"

class Local {
    public:
        void access() { cout << "Accessed object of class Local (1)\n"; }
};

int main() {
        Local object;
        object.access();

        Nonlocal object2;
        object2.access();
}

File 2.h:

#include <iostream>
using namespace std;

class Nonlocal {
        public:
                void access();
};

File 2.C:

#include "2.h"

class Local {
    public:
        void access() { cout << "Accessed object of class Local (2)\n"; }
};

void Nonlocal::access() {
        Local object;
        object.access();
}

I compile the two source files using gcc version 4.3.4 [gcc-4_3-branch revision 152973] (SUSE Linux) and link them as in

g++ -C -o 1.o 1.C
g++ -C -o 2.o 2.C
g++ 1.o 2.o

Note that a definition for class Local appears in both source files, but not in the header. Running a.out produces:

Accessed object of class Local (1)
Accessed object of class Local (1)

Not what I expected!

My understanding of what should happen is that the call object.access() from the main() function should invoke the Local::access() method of the class defined in line 3 of file 1.C and indeed generate the output seen. The call to object2.access(), however, should ultimately invoke the member function Local::access() of the class defined on line 3 of source file 2.C and therefore generate

Accessed object of class Local (2)

Of course, as soon as the two classes, currently both named "Local," are given different names, the output is what I would expect.

In my understanding, the scoping rules say that both declarations and definitions for class Local should not be visible outside the source file in which they reside. Is this wrong? Or is there a problem with the compiler doing inlining and doing it wrong? Or what?

Thanks for any clarifications.

Martin

[PanGalactic]

The behavior is correct.

http://en.wikipedia.org/wiki/One_Definition_Rule

The one definition rule requires that there is just one object once the executable is linked. The linker should balk at linking it IMO, but that is certainly not required by the spec (it states "no diagnostic required").

[msohnius]

Thanks!

Ultimately, the "One definition rule," as discussed on that Wikipedia page, explains it, with this line:

If a program contains more than one definition of a type, then each definition must be equivalent

That's because what is repeated here is the definition of a type, namely class Local, not of an object where the one-definition rule only applies within a "translation unit" (AFAIK, in Unix/Linux environments that's a source file with all its #include's).

I think this is unfortunate because, rather against the spirit of encapsulation, it forces programmers to keep track of names of classes, enum's and other types across all files in a project, even if the intention is only to use them locally.

[Disch]

I think this is unfortunate because, rather against the spirit of encapsulation, it forces programmers to keep track of names of classes, enum's and other types across all files in a project, even if the intention is only to use them locally.

Which is why such concepts as namespaces, and minimizing scopes of types are available/recommended.

For example:

file1.cpp (ill formed)

class Local
{
  // ill formed, possible name conflict if another cpp file has a 'Local' clas
};

file1.cpp (better)

namespace file1cpp_local
{
class Local
{
  // better because now the class is no longer polluting the global namespace
};
}

using namespace file1cpp_local;  // but now it will be in the global namespace of this cpp file only 

};

[msohnius]

You guys are brilliant! I think I'll keep posting here. Needless to say, with that enhancement my program now produces the desired output.

I believe I actually came across my original problem earlier, when I tried to define different exception classes all called "OpenErr" for file open errors, with different parameters and messages in different source files. Then, it just didn't work correctly, I gave up and changed the names of the classes. The case that made me really investigate worked originally, but after a fairly trivial change in the code started producing Segmentation violations at runtime.

I'll simply stick local class definitions into local namespaces in future. Makes the code less prone to failures when changing things years down the line!

Thanks again,

Martin

[kempofighter]

Interesting thread. In 14+ years of programming I have never even seen an attempt to declare a class type within a cpp file as shown by the OP. As Disch points out, this problem can easily be solved with namespaces and good coding standards. I've worked in many large scale development environments and oddly enough I have never run into this problem. The only unfortunate thing that I see is that the compiler didn't do anything to flag the error.

I have run into a problem with macros though. In projects where people use macro defined constants you can end up with cryptic errors. Since macros are text substitutions and have no scope you can get some really strange errors if you define a global constant which happens to have the same name as a macro that you aren't aware of. This is another good reason to take advantage of the namespace concept. In other words, the fix is to not define global constants. Always ensure that constants are either class members or defined within a namespace or both (since classes can also be within a namespace). In a model driven environment it is very easy to click a box and ensure that all types below a certain package level are declared within a namespace. That way people working within a different set of packages will not have to worry about these problems and there will be a smaller chance of name conflict.

[msohnius]

In 14+ years of programming I have never even seen an attempt to declare a class type within a cpp file as shown by the OP

I don't suppose the "OP" stands for what in Britain is called an OAP, an Old Age Pensioner ;-).

My problem could of course well be related to my age: trumping your "14+ years," I last seriously looked at the standard for C in the early '90s when there wasn't even a standard for C++, and when the (Unix) C++Compiler had got neither working templates nor exceptions, and nobody had even heard of namespace.

I am, however, surprised that you hadn't come across this before. I felt it was only reasonable in a program that reads and parses data from the standard input and later reads records from a database and parses those, to use loops of the kind

class entry;

main() {
    entry line;
    while (line.read(cin))
          cout << line;
}

and, in a different file,

class entry;

/* ... */
      entry* ep;
      for (ep = mydb.first(); ep; mydb.next()) {
         /* do something with *ep */                
      }

and have class entry be locally defined to implement the details of the format expected from reading stdin or the database, respectively. None of these details are needed outside the respective source files. A classic case for name-spaces, of course, but surely not unusual.

One could, and perhaps should, adopt as a programming standard that all type definitions must be in header files and that at least one of the source files includes all header files as a matter of principle. Then the compiler would indeed issue an error.

[PanGalactic]

If the intention is to keep scope local to the file, one should use the anonymous namespace.

namespace // anonymous
{
class Local
{
    ...
};
}