Closed Bug 1127882 Opened 9 years ago Closed 9 years ago

error: invalid use of incomplete type 'class mozilla::dom::Promise' error: invalid use of incomplete type 'class mozilla::dom::Promise'

Categories

(Firefox Build System :: General, defect)

Product:
Firefox Build System
Component:
General
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
blocker

Tracking

(firefox36 unaffected, firefox37 unaffected, firefox38 fixed, firefox-esr31 unaffected)

Status:
RESOLVED FIXED
Milestone:
mozilla38
Tracking Flags:
Tracking Status
firefox36 --- unaffected
firefox37 --- unaffected
firefox38 --- fixed
firefox-esr31 --- unaffected

People

(Reporter: philip.chee, Assigned: jcranmer)

References

Details

(Keywords: intermittent-failure, regression)

Attachments

(2 files, 1 obsolete file)

Fix Promise include
1.68 KB, patch
smaug
: review+
Details | Diff | Splinter Review
With tests
2.04 KB, patch
froydnj
: review+
Details | Diff | Splinter Review 
https://tbpl.mozilla.org/php/getParsedLog.php?id=57836224&tree=Thunderbird-Trunk#error0

> ../../dist/include/nsRefPtr.h:60:7: error: invalid use of incomplete type 'class mozilla::dom::Promise'
> In file included from ../../dist/include/mozilla/dom/PopupBlockedEventBinding.h:12:0,
>                  from /builds/slave/tb-c-cen-lx-d-0000000000000000/build/objdir-tb/dom/bindings/PopupBlockedEventBinding.cpp:5,
>                  from /builds/slave/tb-c-cen-lx-d-0000000000000000/build/objdir-tb/dom/bindings/UnifiedBindings1.cpp:50:
> /builds/slave/tb-c-cen-lx-d-0000000000000000/build/mozilla/dom/base/nsGlobalWindow.h:112:7: error: forward declaration of 'class mozilla::dom::Promise'
> In file included from ../../dist/include/nsAutoPtr.h:11:0,
>                  from /builds/slave/tb-c-cen-lx-d-0000000000000000/build/mozilla/dom/svg/DOMSVGAnimatedLengthList.h:9,
>                  from /builds/slave/tb-c-cen-lx-d-0000000000000000/build/mozilla/dom/svg/DOMSVGLengthList.h:9,
>                  from /builds/slave/tb-c-cen-lx-d-0000000000000000/build/mozilla/dom/svg/DOMSVGLength.h:9,
>                  from /builds/slave/tb-c-cen-lx-d-0000000000000000/build/objdir-tb/dom/bindings/SVGLengthBinding.cpp:3,
>                  from /builds/slave/tb-c-cen-lx-d-0000000000000000/build/objdir-tb/dom/bindings/UnifiedBindings1.cpp:2:
> ../../dist/include/nsRefPtr.h: In instantiation of 'nsRefPtr<T>::nsRefPtr(const nsRefPtr<T>&) [with T = mozilla::dom::Promise]':
> ../../dist/include/mozilla/dom/ServiceWorkerEvents.h:69:27:   required from here
> ../../dist/include/nsRefPtr.h:77:7: error: invalid use of incomplete type 'class mozilla::dom::Promise'
> In file included from ../../dist/include/mozilla/dom/PopupBlockedEventBinding.h:12:0,
>                  from /builds/slave/tb-c-cen-lx-d-0000000000000000/build/objdir-tb/dom/bindings/PopupBlockedEventBinding.cpp:5,
>                  from /builds/slave/tb-c-cen-lx-d-0000000000000000/build/objdir-tb/dom/bindings/UnifiedBindings1.cpp:50:
> /builds/slave/tb-c-cen-lx-d-0000000000000000/build/mozilla/dom/base/nsGlobalWindow.h:112:7: error: forward declaration of 'class mozilla::dom::Promise'
Is this an intermittent, or reliably happening?
Flags: needinfo?(philip.chee)
It's a perma bustage on thunderbird Linux debug (only). x64 and opt builds are fine. https://treeherder.mozilla.org/#/jobs?repo=comm-central
Flags: needinfo?(philip.chee)
(adding intermittent-failure to let treehearder suggest this for starring)
Keywords: intermittent-failure
Severity: normal → blocker
OS: Windows 7 → Linux
This really needs either a bisection or a local reproduction to figure out who's using Promise without including Promise.h.
In file included from ../../dist/include/nsAutoPtr.h:11:0,
                 from /builds/slave/tb-c-cen-lx-d-0000000000000000/build/mozilla/dom/svg/DOMSVGAnimatedLengthList.h:9,
                 from /builds/slave/tb-c-cen-lx-d-0000000000000000/build/mozilla/dom/svg/DOMSVGLengthList.h:9,
                 from /builds/slave/tb-c-cen-lx-d-0000000000000000/build/mozilla/dom/svg/DOMSVGLength.h:9,
                 from /builds/slave/tb-c-cen-lx-d-0000000000000000/build/objdir-tb/dom/bindings/SVGLengthBinding.cpp:3,
                 from /builds/slave/tb-c-cen-lx-d-0000000000000000/build/objdir-tb/dom/bindings/UnifiedBindings1.cpp:2:
../../dist/include/nsRefPtr.h: In instantiation of 'nsRefPtr<T>::~nsRefPtr() [with T = mozilla::dom::Promise]':
../../dist/include/mozilla/dom/ServiceWorkerEvents.h:29:22:   required from here
../../dist/include/nsRefPtr.h:60:7: error: invalid use of incomplete type 'class mozilla::dom::Promise'

Any idea?
Looks like ServiceWorkerEvents.h is trying to forward-declare Promise, but using Promise in a way that requires it to be fully defined.

Perhaps the destructors should be declared out-of-line, or somebody should #include Promise.h?  Either way, I don't see how bug 1108750 would have caused this...
(In reply to Nathan Froyd [:froydnj] [:nfroyd] from comment #12)
> Perhaps the destructors should be declared out-of-line, or somebody should
> #include Promise.h?  Either way, I don't see how bug 1108750 would have
> caused this...

Actually, I think I see how it might have done so. Here's UnifiedBindings0.cpp from my current build [stripping some stuff out]:
#include "SVGGElementBinding.cpp"
#include "SVGFEConvolveMatrixElementBinding.cpp"
#include "SVGFEDiffuseLightingElementBinding.cpp"
#include "HTMLObjectElementBinding.cpp"

I'm guessing that something in bug 1108750 caused the include list to stop getting alphabetically sorted, so every build effectively has a random order of files in unified files. Or, to put it more emphatically, if there are multiple unified files, the subset that is in the files is effectively random and per-person.

Asking from another contributor, I got this in their UnifiedBindings0.cpp:
#include "HTMLBRElementBinding.cpp"
#include "IDBTransactionBinding.cpp"
#include "RTCRtpSenderBinding.cpp"
http://hg.mozilla.org/mozilla-central/rev/d23cbb9c5271#l1.91 appears to be to blame. _add_unified_build_rules enforces that its input was sorted... by bypassing that line, non-sorted lists (like, say, the WebIDL generator, which is effectively returned in the order of a Python set, see <https://dxr.mozilla.org/comm-central/source/mozilla/python/mozbuild/mozbuild/backend/common.py#83>) don't get sorted before being grouped into unified files.

Quite frankly, it's a miracle we haven't seen more problems caused by this. I guess we still have rather good hygiene in our WebIDL binding files?
Blocks: 1108750
Status: NEW → ASSIGNED
Component: DOM → Build Config

        Attachment #8560244 -
        Flags: review?(nfroyd)

        Attachment #8560244 -
        Flags: review?(mshal)

    
  
Assignee: nobody → Pidgeot18

    
    

    
  
> I guess we still have rather good hygiene in our WebIDL binding files?

Yes.  In fact, the only bad things there should be from headers they include, not the bindings themselves...

So ok, the intermittency is explained by the random order, which definitely needs to be fixed.  But the failure means that some header is relying on Promise being defined, not just forward-declared, while not including Promise.h, yes?

Looking at the log earlier in this bug, sounds like ServiceWorkerEvents.h is the offending header, presumably for its mPromise member.
Flags: needinfo?(nsm.nikhil)

    
    

    
      
  

    
    

    
  
Comment on attachment 8560244 [details] [diff] [review]
Thou shalt not use unsorted lists to group Unified files.

Review of attachment 8560244 [details] [diff] [review]:
-----------------------------------------------------------------

And thou shalt know that I am the responsible party, when thou executest the sort() upon mine input.

        Attachment #8560244 -
        Flags: review?(nfroyd) → review+

    
    

    
  
It would be good to add a check for sortedness here, too:

http://mxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/test/test_util.py#669

Maybe with scrambling the initial input:

http://mxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/test/test_util.py#667

so that the test is not trivially true.
Assignee: Pidgeot18 → nsm.nikhil

    
    

    
  
:jcranmer, please remove leave-open after you land your patch. Thanks!

https://hg.mozilla.org/integration/mozilla-inbound/rev/74052b081fed
Assignee: nsm.nikhil → Pidgeot18
Flags: needinfo?(nsm.nikhil)
Keywords: leave-open
Keywords: regression

        Attachment #8560244 -
        Flags: review?(mshal) → review+

    
    

    
  

      
      
      
      

        Attached patch
          
          
        With tests
        — 
          — Splinter Review
      

    


  
The test is just slightly nontrivial enough I wanted to get a re-review.

        Attachment #8560244 -
        Attachment is obsolete: true

        Attachment #8560757 -
        Flags: review?(nfroyd)

        Attachment #8560757 -
        Flags: review?(nfroyd) → review+

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/70a1847bdc20
Status: ASSIGNED → RESOLVED
Closed: 9 years ago

          status-firefox38:
          --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla38

    
  
Product: Core → Firefox Build System

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: