43311 - AIX xlC 5.0 compiler issue: friend 'prototype'

Status: RESOLVED FIXED

(SeaMonkey :: Build Config, defect, P3)

Description

[Jim Dunn]

The AIX xlC 5.0 compiler is having issues with having 'friend' declarations
that also double for 'prototyping'.  In this case I get the following error:

"friend.cpp", line 10.7: 1540-0040 (S) The text "function" is unexpected.  "B"
may be undeclared or ambiguous.

Since this is a compiler issue, this bug is aplace holder.

Comment 1

[Jim Dunn]

Attachment: to compile testcase: xlC_r -c friend.cpp

Comment 2

[Jim Dunn]

adding mike

Comment 3

[Bill Buros]

Problem recreated and submitted to the IBM compiler team.

Comment 4

[Bill Buros]

For tracking purposes:  IBM PMR #79946 -519-000

Comment 5

[Bill Buros]

Changed to PMR 85012 -519-000

Comment 6

[Bill Buros]

Not seen with gcc or IBM 3.6.6 compiler.   The problem is still being worked.


A work-around is to pre-declare B.   Is this acceptable for the time being?



--------
#include <stdio.h>

class B;   // pre-declare B

class A {
  public:
    A() { a=1; }

  private:
    friend class B;
  
    B function();
    int a;
};

class B {
  public:
    B() { b = 0; aptr = '\0'; }

  private:
    A *aptr;
    int b;
};

B A::function() {
   B *temp = new B;

   return *temp;
}
---------

Comment 7

[Bill Buros]

Defect #160229.

Comment 8

[Scott Collins]

Hmmm.  I can see how your compiler could legitimately complain that at
the point of

  B function();

|B| is still an incomplete type, and hence can't be used in this way.
However, for a compliant compiler, the name |B| should now be in
scope because of the earlier |friend| declaration.  Many compilers
get this wrong, and don't treat a local |friend| declaration as a
forward declaration.  This is probably your trouble.  Declaring
|class B;| above the entire class that wishes it to be a |friend|
might fix the error you're currently seeing, but would most likely
resolve to the incomplete type error I would have expected in the
first place.

Your example can easily be made to compile by re-ordering the
declarations ... so I don't know if this is a good case for your real
problem.  E.g.,

  class A;

  class B
    {
      public:
        B() : b(0), aptr(0) { }

      private:
        A* aptr; // pointer to an incomplete type is OK
        int b;
    };

  class A
    {
      public:
        A() : a(1) { }

      private:
        B function(); // B not incomplete, OK to use

        int a;
    };

  B
  A::function()
    {
      B* temp = new B;
      return *temp;
    }

I know your example was just quick and dirty, but I don't understand
why you would initialize a pointer to be the conversion of a
character.  How does your example differ from the real case that would
prevent you from avoiding the trouble entirely by re-ordering the
declarations?

Comment 9

[Scott Collins]

oops, I left out the |friend| decl; which I don't think is actually needed in
your example anyway (even though it's the point of your example).

Comment 10

[Jim Dunn]

The work around suggested has been used to get around this.
However, it is annoying and since I had opened other VA5.0
(xlC) bugs, I figured I would open this one too.

The example is down and dirty that mimics what I have seen
in mozilla code.  The example itself is silly and is not meant
to do anything other than show the compiler bug.

The key point to this and the other VA5.0 bugs is too show that
the compiler is not currently up to task to build mozilla and
I wanted to list all of the issues I found.  I have had problems
in the past getting permission to go in and re-order statements
just to fix a 'AIX' issue.  It would be much easier to have a
compiler that didn't exhibit these problems.

Comment 11

[Mike Kaply [:mkaply]]

scc wasn't cced on this so he didn't see jdunn's latest comment.

scc - apparently, this problem exists in some Mozilla files, so if you believe 
the code is incorrect, then mozilla does need to be fixed.

jdunn, can you give specific examples of mozilla files with this problem?

Comment 12

[Jim Dunn]

Not without doing a 5.0 build and right now I am not setup to
do it...

One key is that we can workaround this.
The other key is that my original attachment, shows the problem
so that the compiler people can fix this.
So if we ever decide to move to the new compiler (there are lots
of other bigger issues then just this... we can deal with the
workarounds...

Comment 13

[Jeff George]

I have engaged the compiler people on this issue as well as the two other
AIX VisualAge 5 C++ issues, bugs 43315 and 43306.

A compiler defect (160229) was opened for this problem and was subsequently
closed by the compiler team as "user error", since the friend declaration
does not implicitly declare the class.

I am following up on this, because my understanding was that, in Bjarne
Stroustrup's words, "A friend class must be previously declared in an
enclosing scope or defined in the non-class scope immediately enclosing
the class that is declaring it a friend." (Quote taken from the 2000
edition of "The C++ Programming Language".) This implies that the code in
the testcase should work, since the friend class is defined within "the
non-class scope immediately enclosing the class that is declaring it a
friend".

It is possible that the standard varies slightly from this or that there
is some subtlety which has eluded me, but I am asking for a clarification
from the compiler team, which I will copy into the text of this bug.

Comment 14

[Jim Dunn]

Thanks!  If the compiler team still say no, they are
right, then please ask them to cite where in the standard
that they are quoting from.

With the quote, I can go to engineers and say, HERE, your
code is wrong, here is the solution, let me check it in.
Without a quote from the standard... I can't do squat.

Comment 15

[Jeff George]

I am currently discussing this with our compiler team, who have pointed to
the relevant reference in the standard, section 7.3.1.2, paragraph 3:

"If a friend declaration in a non-local class first declares a class or
function the friend class or function is a member of the innermost
enclosing namespace.  The name of the friend is not found by simple name
lookup until a matching declaration is provided in that namespace scope
(either before or after the class declaration granting friendship)."

Comment 16

[Jeff George]

Attachment: This is the first mozilla file where I see the friendliness problem.

Comment 17

[Jeff George]

This compile error occurs when the compile of xpcom/base/nsWeakReference.cpp
is attempted. The error message is:

  "nsWeakReference.h", line 53.22: 1540-0040 (S) The text "*" is unexpected.
  "nsWeakReference" may be undeclared or ambiguous.

Comment 18

[Jeff George]

It seems that the VisualAge 5 C++ compiler is behaving correctly here
according to the standard. We should definitely resolve this with a code
change in Mozilla.

I was a little fuzzy on exactly how the construct in question violated the
standard, so the VisualAge C++ compiler team provided me with the following
more precise explanation of why this usage is incorrect in terms of the
standard:

The particular example being discussed is included below.  The error
given is that function is unexpected.  I will try to explain the history
behind this and why this is an error.

#include <stdio.h>

class A {
  public:
    A() { a=1; }

  private:
    friend class B;

    B function();
    int a;
};

class B {
  public:
    B() { b = 0; aptr = '\0'; }

  private:
    A *aptr;
    int b;
};

B A::function() {
   B *temp = new B;

   return *temp;
}

First, the history.  In the working drafts of the standard, it used to be that
a friend declaration of a class also implicitly declared a class in global
scope if it did not already exist in global scope.  Version 3.6.6 of IBM's
compilers worked this way, as do many current versions of C++ compilers offered
by other vendors.  When a friend declaration was encountered, the name of the
class being nominated for friendship was looked up and matched if it existed.
If it did not, it would be declared in the global scope.  Thus, the name would
be found in the next declaration because the lookup for B would find the name
in global scope.

This was changed in the final standard.  The reference in the standard is
7.3.1.2, paragraph 3.

"If a friend declaration in a non-local class first declares a class or
function the friend class or function is a member of the innermost enclosing
namespace.  The name of the friend is not found by simple name lookup until a
matching declaration is provided in that namespace scope (either before or
after the class declaration granting friendship)."

What this means is that when a class is nominated for friendship, the name of
the class is looked up in the current scope.  If it is not found, it is looked
up in the enclosing scopes, until it is found or the global namespace is
reached.  Essentially, normal name lookup is performed.  If it is not found (as
is the case here), the rules in the quotation given apply.  The name is
declared in the innermost enclosing namespace.  The name is not visible until a
matching declaration is provided.  Thus, the name B is not found, resulting in
the error.  Note that the error even suggests that B may be undeclared.

Comment 19

[Scott Collins]

The class |nsWeakReference| needs to be forward declared before the class
|nsSupportsWeakReference|, that's all.  The compiler is right to refuse this
code.  Most compilers get this wrong, and find |nsWeakReference| in spite of the
standard, as this was the draft standard defined behavior, as pointed out above.

Comment 20

[Jim Dunn]

Attachment: New patch to fix all the offending "friend" issues

This patch will fix all the friend issues.
Declaring something friend doesn't mean that it has been
declared or prototyped.  The new xlC compiler is just
very strict in this case, so in order for something to be
used it needs to be "declared/prototyped" in addition to
being made a "friend".

Additionally, if it is "declared/prototyped" the default args
need to be set in the prototype and NOT in the friend accessor

Comment 21

[Jim Dunn]

updating bug, adding a bunch of people on the CC list.

I am looking for a r= and hopefully a sr= (but since this
is a "build" issue and shouldn't change any code... I don't
think it is needed, but I cc'd a bunch of people to get there
thoughts)

Comment 22

[Brendan Eich [:brendan]]

Cc'ing dbaron, who is a C++ language guru (in my book, anyway).  David, could
you look at the latest patch and jdunn's comments this year, and bless or curse?
 Thanks,

/be

Comment 23

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 64132 [details] [diff] [review]
New patch to fix all the offending "friend" issues

IIRC (all the online copies of the C++ standard have disappeared) this compiler
is wrong, but the workaround won't really hurt anything, so I don't object
(although people will probably re-break this a bit).

I think the example used in comment #6 led scc to think this was a different
problem from what it really is.

r=dbaron

Comment 24

[jag (Peter Annema)]

From "The C++ Programming Language, Third Edition", section 11.5.1:
<quote>
11.5.1 Finding Friends

  Like a member declaration, a friend declaration does not introduce a name into
an enclosing scope. For example:

  class Matrix {
    friend class Xform;
    friend Matrix invert(const Matrix&);
    //..
  }

  Xform x;                              // error: no Xform in scope
  Matrix (*p)(const Matrix&) = &invert; // error: no invert() in scope

  For large programs and large classes, it is nice that a class doesn't
``quietly'' add names to its enclosing scope. For a template class that can be
instantiated in many different contexts (Chapter 13), this is very important.
  A friend class must be previously declared in an enclosing scope or defined in
the non-class scope immediately enclosing the class that is declaring it a
friend. For example:

  class X { /* ... */ };         // Y's friend
  namespace N {
    class Y {
      friend class X;
      friend class Z;
      friend class AE;
    };
    class Z { /* ... */ };       // Y's friend
  }

  class AE { /* ... */ };        // not a friend of Y
</quote>

This seems to suggest that the following is also the case, since B is declared
in the non-class scope immediately enclosing A:

  class A {
    friend class B;
  };

  class B { /* ... */ };   // friend of A

Comment 25

[Jim Dunn]

*** Bug 72190 has been marked as a duplicate of this bug. ***

Comment 26

[Jim Dunn]

fix checked in

Comment 27

[Mary Hoetzel]

Fixes for several files have been checked in, but the problem has reoccurred in 
other files. Therefore, the standard question should really be settled to make 
it easier to handle new similar problems.

In an earlier note in comment #18, the standard was quoted that explains why the 
 behavior of the new AIX compiler is standard conform. Scott Collins agreed in 
comment #19. David Baron states in comment #23 that the compiler is wrong.

David, if you think the compiler is wrong, could you please explain why the 
quoted standard does not apply, and why the example in comment #6 does not 
address the real problem.

I would like to get the opinion of the compiler group that develops the 
compiler, but to do this I need more specifics.

Thank you very much.

Comment 28

[jag (Peter Annema)]

To quote from comment 18:

"If a friend declaration in a non-local class first declares a class or
function the friend class or function is a member of the innermost enclosing
namespace.  The name of the friend is not found by simple name lookup until a
matching declaration is provided in that namespace scope (either before or
after the class declaration granting friendship)."

// innermost enclosing namespace is the global scope

class A // non-local class
{
  friend class B; // friend declaration in a non-local class, first declaration
}

class B // matching declaration for friend class B, provided in "that namespace"
{
}

So the way I see it, the compiler is wrong and this code is fine, but perhaps
I'm misinterpreting something (for I am not a c++ language lawyer).

Comment 29

[Jim Dunn]

I am attaching a very simple testcase that shows the
current set of problems.

---- friend.cpp : xlC -c friend.cpp ----
class A {
  public:
    A(void);

  private:
    friend int NS_SomeFunc(int a);
};

A::A(void) {
  int rc = NS_SomeFunc(10);
  return;
}

int NS_SomeFunc(int a) {
    return 1;
}
----
Error message
"friend.cpp", line 11.12: 1540-0274 (S) The name lookup for "NS_SomeFunc" did 
not find a declaration.
"friend.cpp", line 15.5: 1540-1298 (I) "int NS_SomeFunc(int)" needs to be 
declared in the containing scope to be found by name lookup.

Comment 30

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I can't clearly find a normative passage in the C++ spec that states this
behavior is wrong (or states the contrary), but there are *examples* in the C++
spec that I don't think would compile on this compiler, and I think the examples
at least make the intentions of the spec authors clear.  Does the compiler
compile this example from 11.4?

          class X {
              int a;
              friend void friend_set(X*, int);
          public:
              void member_set(int);
          };
          void friend_set(X* p, int i) { p->a = i; }
          void X::member_set(int i) { a = i; }

          void f()
          {
              X obj;
              friend_set(&obj,10);
              obj.member_set(10);
          }

Comment 31

[Jim Dunn]

Your example does compile... 

Comment 32

[jag (Peter Annema)]

class A {
  public:
    A(void);

  private:
    friend int NS_SomeFunc(int a);
};

int NS_SomeFunc(int a) {
    return 1;
}

A::A(void) {
  int rc = NS_SomeFunc(10);
  return;
}

How about the above? (So your code, but the actual call and the definition
switched).

Comment 33

[Jim Dunn]

Your latest test does compile... since you are
declaring " int NS_SomeFunc(int a) " before it
is being used.

Comment 34

[jag (Peter Annema)]

Ah, so the issue is that |friend class foopy;| doesn't declare |foopy| as a class.

Time to re-read all comments here with that in mind.

So, it does seem that the new compiler is correct.

Comment 35

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Section 11.4, clause 9 of the C++ standard says this would be the correct
behavior for *local* classes, but the fact that it restricts that statement to
local classes suggests that it's not correct for other ones.  However, I can't
find any explicit statement to the contrary.

attachment 64132 [details] [diff] [review] shows that there are problems with both friend functions and
friend classes.

Comment 36

[Jim Dunn]

From my #29 comment:

I put my "friend.cpp" into Comeau's online compiler
http://www.comeaucomputing.com/tryitout/  after it was
suggested to me from someone else.  

-----------------------------------------------------------------
Comeau C/C++ 4.2.45.2 (Apr 12 2001 10:06:52) for ONLINE_EVALUATION
Copyright 1988-2001 Comeau Computing.  All rights reserved.
MODE:strict errors C++

"14100.c", line 10: error: identifier "NS_SomeFunc" is undefined
    int rc = NS_SomeFunc(10);
             ^

Comment 37

[Brendan Eich [:brendan]]

Waldemar, can you shed any light here? Thanks,

/be

Comment 38

[Waldemar Horwat]

I looked over all of the comments above and looked at the relevant
sections of the standard.  It's clear to me that the compiler is
correct, given the quote mentioned in comment 15 and comment 18.  I
couldn't say it better than as stated in comment 18.

The one remaining dissenting opinion mentions a clause that does not
apply here and covers a different issue:

Comment 35 mentions section 11.4, clause 9.  This clause is not
relevant here because it deals only with what happens when you declare
a class *inside a function*.  This is a really bizarre thing to do --
it's a holdover from old C -- and C++ restricts what you can do there.
In particular, 11.4:9 states that when looking for functions or
classes to be made into friends, you *cannot* look outside the
enclosing function into the namespace or global scope.  See the
example in 11.4:9.

Outside a local class (i.e. a class defined inside a function), the
friend declaration *can* look into global and namespace scopes and
either find or declare things there.  As stated in 7.3.1.2, paragraph
3, these declarations are not visible until they are declared outside
a friend clause.

Comment 39

[Philip K. Warren]

Attachment: Two more cases not picked up in previous diff

There are two more places in the code where the friend function needs to be
declared (as of Mozilla 0.9.8). Here is a patch fixing the last two remaining
problems.

Comment 40

[Philip K. Warren]

Attachment: Friend fixes for latest trunk build

Here is the most recent fix that cleans up all the remaining friend issues with
the AIX v5 compiler.

Comment 41

[jag (Peter Annema)]

Comment on attachment 72806 [details] [diff] [review]
Friend fixes for latest trunk build

sr=jag

Comment 42

[Asa Dotzler [:asa]]

Comment on attachment 72806 [details] [diff] [review]
Friend fixes for latest trunk build

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 43

[Jim Dunn]

Fix checked in

Comment 44

[Philip K. Warren]

Attachment: Fix for latest string changes

One more fix needed after the checkin for Bug 127097.

Comment 45

[Jim Dunn]

reopening to fix latest issue (see #44)

Comment 46

[Jim Dunn]

Attachment: alternative patch to 73249

Jag mentioned that no one should include nsString2.h instead
they should include nsString.h.  Switching this fixes the
compile issue.

Comment 47

[Asa Dotzler [:asa]]

Comment on attachment 75600 [details] [diff] [review]
alternative patch to 73249

with jag's r= you have a=asa for checkin to the 1.0 trunk. scc says this is a
good thing so I'm 'pre-approving'.

Comment 48

[jag (Peter Annema)]

Comment on attachment 75600 [details] [diff] [review]
alternative patch to 73249

r=/sr=jag

Comment 49

[Jim Dunn]

dbaron checked in the fix...
marking fixed (may this bug never be re-opened again...)