230118 - Mozilla depends on private copy constructors being accessible

Status: RESOLVED FIXED

(SeaMonkey :: General, defect, P1)

Description

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

Current trunk builds of gcc 3.4 (after the fix to
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12226 , I believe) don't compile
Mozilla.  The first error is:

../../dist/include/gfx/nsRegion.h: In member function `PRBool
nsViewManager::CanScrollWithBitBlt(nsView*)':
../../dist/include/gfx/nsRegion.h:237: error: `nsRegion::nsRegion(const
nsRegion&)' is private
/builds/tinderbox/SeaMonkey-gcc3.4/Linux_2.4.7-10_Depend/mozilla/view/src/nsViewManager.cpp:2882:
error: within this context
../../dist/include/gfx/nsRegion.h: In member function `virtual nsresult
nsViewManager::Display(nsIView*, nscoord, nscoord, const nsRect&)':
../../dist/include/gfx/nsRegion.h:237: error: `nsRegion::nsRegion(const
nsRegion&)' is private
/builds/tinderbox/SeaMonkey-gcc3.4/Linux_2.4.7-10_Depend/mozilla/view/src/nsViewManager.cpp:3288:
error: within this context


A simplified testcase for what causes the error is the following:
-----
class C {
  public:  C() {}
  private: C(const C&) {}
};
                                                                                
void f(const C& c) { }
                                                                                
int main()
{
  f(C());
  return 0;
}
-----

which causes gcc to emit the error:

priv.cpp: In function `int main()':
priv.cpp:3: error: `C::C(const C&)' is private
priv.cpp:10: error: within this context


I believe this error is correct (although I'm not sure), based on the following
quotes from the C++ standard (note in particular the two lines with "*" at the
right edge):

section 3.10, clause 2, footnote 47:
  Expressions such as invocations of constructors and of functions that
  return a class type refer to objects, and the implementation can
  invoke a member function upon such objects, but the expressions are
  not lvalues.
                                                                                
section 8.5.3, clause 5:                                                       
                        
  A reference to type "cv1 T1" is initialized by an expression of type
  "cv2 T2" as follows:
  - If the initializer expression
    - is an lvalue (but is not a bit-field), and "cv1 T1" is
      reference-compatible with "cv2 T2", or
    - has a class type (i.e, T2 is a class type) and can be implicitly
      converted to an lvalue of type "cv3 T3," where "cv1 T1" is
      reference-compatible with "cv3 T3" [footnote 92: This requires a
      conversion function (12.3.2) returning a reference type.]  (this
      conversion is selected by enumerating the applicatble conversion
      functions (13.3.1.6) and choosing the best one through overload
      resolution (13.3)),
    then [...]
  - Otherwise, the reference shall be to a non-volatile const type (i.e.,
    cv1 shall be const)
      - If the initializer expression is an rvalue, with T2 a class type,
        and "cv1 T1" is reference-compatible with "cv2 T2," the reference
        is bound in one of the following ways (the choice is
        implementation-defined):
        - The reference is bound to the object represented by the rvalue
          (see 3.10) or to a sub-object within that object.
        - a temporary of type "cv1 T2" [sic] is created, and a constructor
          is called to copy the entire rvalue object into the temporary.
          The reference is bound to the temporary or to a sub-object
          within the temporary.
        The constructor that would be used to make the copy shall be       *
        callable whether or not the copy is actually done.                 *
      - Otherwise, [...]

Comment 1

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

Attachment: patch

Comment 2

[Bradley Baetz (:bbaetz)]

In the construction case, what constructor is being called? Why isn't that being
used in the temp case?

Also, why not use .Copy, if thats what that code is rying to do? Or does Copy
not work that way?

Comment 3

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

to do:

  f(C())

the |C()| constructs a |C| object that is not an lvalue.  Thus, to pass a |const
C&| to |f|, it needs to be converted according to the rules in comment 0.  The
copy constructor isn't actually called, but it needs to be accessible according
to the two starred lines (*).  In the case of an explicit temporary variable,
the temporary is an lvalue.

(We could ask the gcc folks if this is actually correct, I guess...)

Comment 4

[Bradley Baetz (:bbaetz)]

Comment on attachment 138440 [details] [diff] [review]
patch

Oh, right. r is an nsRect, not an nsRegion.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 138440 [details] [diff] [review]
patch

That kinda sucks, but oh well.

Comment 6

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

Fix checked in to trunk, 2003-01-08 13:06 -0800.

Comment 7

[Dainis Jonitis]

I made a change that makes copy constructors public in bug 228378.
If I understand right then with these changes this fix would not be required.
Probably it is better to check in 228378. Roc already reviewed it, but have not
checked in. I can not check in because I do not have access rights.

Comment 8

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

Since bug 228378 landed, I backed out the change here since the copy constructor
is no longer private.

Comment 9

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

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