1462794 - Build error: cannot convert ‘nsPresState*’ to ‘mozilla::PresState*’ caused by left-over nsILayoutHistoryState.h in source tree

Status: REOPENED

(Firefox Build System :: General, defect)

Description

[Ben Bucksch (:BenB)]

Environment:
* Ubuntu 16.04 with updates
* Updated gcc from launchpad
  $ gcc --version
  gcc (Ubuntu 6.4.0-17ubuntu1~16.04) 6.4.0 20180424

Code:
* Latest mozilla-central
* Latest comm-central for Thunderbird, in a subdirectory. But the failure is in compiling Mozilla, and Mozilla source is at the top level, so should be irrelevant.
* I did a clobber, and the error persists.

errors:

no matching function for call to ‘nsILayoutHistoryState::AddState()’
   aState->AddState(stateKey, Move(frameState));

cannot convert ‘nsPresState*’ to ‘mozilla::PresState*’ in initialization

‘NewPresState’ was not declared in this scope

prototype for ‘void nsLayoutHistoryState::AddState(const nsCString&, mozilla::UniquePtr<mozilla::PresState>)’ does not match any in class ‘nsLayoutHistoryState’
candidate is: virtual void nsLayoutHistoryState::AddState(const nsCString&, nsPresState*)

prototype for ‘mozilla::PresState* nsLayoutHistoryState::GetState(const nsCString&)’ does not match any in class ‘nsLayoutHistoryState’
candidate is: virtual nsPresState* nsLayoutHistoryState::GetState(const nsCString&)


-------

It looks like a namespace problem, esp. the last error. Compiler?


------


11:13.71 In file included from .../opt/layout/base/Unified_cpp_layout_base1.cpp:56:0:
11:13.71 .../source/layout/base/nsFrameManager.cpp: In member function ‘void nsFrameManager::CaptureFrameStateFor(nsIFrame*, nsILayoutHistoryState*)’:
11:13.71 .../source/layout/base/nsFrameManager.cpp:166:46: error: no matching function for call to ‘nsILayoutHistoryState::AddState(nsAutoCString&, mozilla::RemoveReference<mozilla::UniquePtr<mozilla::PresState>&>::Type)’
11:13.71    aState->AddState(stateKey, Move(frameState));
11:13.71                                               ^
11:13.72 In file included from .../source/layout/base/nsCSSFrameConstructor.h:21:0,
11:13.72                  from .../source/layout/base/nsBidiPresUtils.cpp:18,
11:13.72                  from .../opt/layout/base/Unified_cpp_layout_base1.cpp:2:
11:13.72 .../source/layout/base/nsILayoutHistoryState.h:47:16: note: candidate: virtual void nsILayoutHistoryState::AddState(const nsCString&, nsPresState*)
11:13.72    virtual void AddState(const nsCString & aKey, nsPresState *aState) = 0;
11:13.72                 ^~~~~~~~
11:13.72 .../source/layout/base/nsILayoutHistoryState.h:47:16: note:   no known conversion for argument 2 from ‘mozilla::RemoveReference<mozilla::UniquePtr<mozilla::PresState>&>::Type {aka mozilla::UniquePtr<mozilla::PresState>}’ to ‘nsPresState*’
11:13.73 In file included from .../opt/layout/base/Unified_cpp_layout_base1.cpp:56:0:
11:13.73 .../source/layout/base/nsFrameManager.cpp: In member function ‘void nsFrameManager::RestoreFrameStateFor(nsIFrame*, nsILayoutHistoryState*)’:
11:13.73 .../source/layout/base/nsFrameManager.cpp:229:52: error: cannot convert ‘nsPresState*’ to ‘mozilla::PresState*’ in initialization
11:13.73    PresState* frameState = aState->GetState(stateKey);
11:13.73                                                     ^
11:13.73 In file included from .../source/layout/base/nsCSSFrameConstructor.h:21:0,
11:13.73                  from .../source/layout/base/nsBidiPresUtils.cpp:18,
11:13.73                  from .../opt/layout/base/Unified_cpp_layout_base1.cpp:2:
11:13.73 .../source/layout/base/nsILayoutHistoryState.h:17:7: note: class type ‘nsPresState’ is incomplete
11:13.73  class nsPresState; /* forward declaration */
11:13.73        ^~~~~~~~~~~
11:14.94 In file included from .../opt/layout/base/Unified_cpp_layout_base1.cpp:92:0:
11:14.94 .../source/layout/base/nsLayoutHistoryState.cpp: In member function ‘virtual nsresult nsLayoutHistoryState::GetPresState(const nsACString&, float*, float*, bool*, float*, bool*)’:
11:14.94 .../source/layout/base/nsLayoutHistoryState.cpp:85:46: error: cannot convert ‘nsPresState*’ to ‘mozilla::PresState*’ in initialization
11:14.94    PresState* state = GetState(nsCString(aKey));
11:14.94                                               ^
11:14.94 In file included from .../source/layout/base/nsCSSFrameConstructor.h:21:0,
11:14.94                  from .../source/layout/base/nsBidiPresUtils.cpp:18,
11:14.94                  from .../opt/layout/base/Unified_cpp_layout_base1.cpp:2:
11:14.94 .../source/layout/base/nsILayoutHistoryState.h:17:7: note: class type ‘nsPresState’ is incomplete
11:14.94  class nsPresState; /* forward declaration */
11:14.94        ^~~~~~~~~~~
11:14.94 In file included from .../opt/layout/base/Unified_cpp_layout_base1.cpp:92:0:
11:14.94 .../source/layout/base/nsLayoutHistoryState.cpp: In member function ‘virtual nsresult nsLayoutHistoryState::AddNewPresState(const nsACString&, float, float, bool, float, bool)’:
11:14.95 .../source/layout/base/nsLayoutHistoryState.cpp:106:48: error: ‘NewPresState’ was not declared in this scope
11:14.95    UniquePtr<PresState> newState = NewPresState();
11:14.95                                                 ^
11:14.95 .../source/layout/base/nsLayoutHistoryState.cpp: At global scope:
11:14.95 .../source/layout/base/nsLayoutHistoryState.cpp:118:1: error: prototype for ‘void nsLayoutHistoryState::AddState(const nsCString&, mozilla::UniquePtr<mozilla::PresState>)’ does not match any in class ‘nsLayoutHistoryState’
11:14.95  nsLayoutHistoryState::AddState(const nsCString& aStateKey, UniquePtr<PresState> aState)
11:14.95  ^~~~~~~~~~~~~~~~~~~~
11:14.95 In file included from .../source/layout/base/nsCSSFrameConstructor.h:21:0,
11:14.95                  from .../source/layout/base/nsBidiPresUtils.cpp:18,
11:14.95                  from .../opt/layout/base/Unified_cpp_layout_base1.cpp:2:
11:14.95 .../source/layout/base/nsILayoutHistoryState.h:74:16: error: candidate is: virtual void nsLayoutHistoryState::AddState(const nsCString&, nsPresState*)
11:14.95    virtual void AddState(const nsCString & aKey, nsPresState *aState) override; \
11:14.95                 ^
11:14.95 .../source/layout/base/nsILayoutHistoryState.h:74:16: note: in definition of macro ‘NS_DECL_NSILAYOUTHISTORYSTATE’
11:14.95    virtual void AddState(const nsCString & aKey, nsPresState *aState) override; \
11:14.95                 ^~~~~~~~
11:14.95 In file included from .../opt/layout/base/Unified_cpp_layout_base1.cpp:92:0:
11:14.95 .../source/layout/base/nsLayoutHistoryState.cpp:124:1: error: prototype for ‘mozilla::PresState* nsLayoutHistoryState::GetState(const nsCString&)’ does not match any in class ‘nsLayoutHistoryState’
11:14.95  nsLayoutHistoryState::GetState(const nsCString& aKey)
11:14.95  ^~~~~~~~~~~~~~~~~~~~
11:14.95 In file included from .../source/layout/base/nsCSSFrameConstructor.h:21:0,
11:14.95                  from .../source/layout/base/nsBidiPresUtils.cpp:18,
11:14.95                  from .../opt/layout/base/Unified_cpp_layout_base1.cpp:2:
11:14.95 .../source/layout/base/nsILayoutHistoryState.h:75:25: error: candidate is: virtual nsPresState* nsLayoutHistoryState::GetState(const nsCString&)
11:14.95    virtual nsPresState * GetState(const nsCString & aKey) override; \
11:14.95                          ^
11:14.95 .../source/layout/base/nsILayoutHistoryState.h:75:25: note: in definition of macro ‘NS_DECL_NSILAYOUTHISTORYSTATE’
11:14.95    virtual nsPresState * GetState(const nsCString & aKey) override; \
11:14.95                          ^~~~~~~~
11:17.62 In file included from .../source/layout/generic/ReflowInput.cpp:23:0,
11:17.62                  from .../opt/layout/generic/Unified_cpp_layout_generic0.cpp:65:
11:17.62 .../source/layout/tables/nsTableFrame.h: In member function ‘void mozilla::ReflowInput::InitConstraints(nsPresContext*, const mozilla::LogicalSize&, const nsMargin*, const nsMargin*, mozilla::LayoutFrameType)’:
11:17.62 .../source/layout/tables/nsTableFrame.h:45:20: warning: ‘fType’ may be used uninitialized in this function [-Wmaybe-uninitialized]
11:17.62           frameType == mozilla::LayoutFrameType::BCTableCell;
11:17.62           ~~~~~~~~~~^~~~~~~~~~
11:17.62 In file included from .../opt/layout/generic/Unified_cpp_layout_generic0.cpp:65:0:
11:17.62 .../source/layout/generic/ReflowInput.cpp:2226:21: note: ‘fType’ was declared here
11:17.62      LayoutFrameType fType;
11:17.62                      ^~~~~
11:23.38 In file included from .../opt/layout/generic/Unified_cpp_layout_generic2.cpp:29:0:
11:23.38 .../source/layout/generic/nsGfxScrollFrame.cpp: In member function ‘mozilla::UniquePtr<mozilla::PresState> mozilla::ScrollFrameHelper::SaveState() const’:
11:23.38 .../source/layout/generic/nsGfxScrollFrame.cpp:6233:45: error: ‘NewPresState’ was not declared in this scope
11:23.38    UniquePtr<PresState> state = NewPresState();

Comment 1

[Jorg K (CEST = GMT+2)]

That's an old hat. Delete nsILayoutHistoryState.h from your source. Read bug 1438026 comment #9 and down.

Comment 2

[Ben Bucksch (:BenB)]

Thanks, Jörg, for the help. That helped.

However, this is still a valid build system bug.

> Looks like people had nsILayoutHistoryState.h in the source tree(!??).

I have no idea how it got there. If the build system ever left dirt like this around, it should clean up automatically. This has wasted a lot of my time. It should be worth the 5 minutes for the guy who broke this to add a delete command to the make file.

Comment 3

[Jorg K (CEST = GMT+2)]

Ben, if you think it's a bug, why don't you NI the people from bug 1438026. If you think it's a *build* system bug, you should move it to a different product. Otherwise it will just end up on the pile and receive no attention.

What happened is this: nsILayoutHistoryState.h *was* a valid file in the source tree until recently, see:
https://dxr.mozilla.org/mozilla-esr52/source/layout/base/nsILayoutHistoryState.h

At some stage it got removed and replaced by a dynamically generated file and the one left over in the source caused the problem. Both Aceman on Linux and Philipp on Mac had that problem.

I can't see where it got removed:
https://hg.mozilla.org/mozilla-central/log/tip/layout/base/nsILayoutHistoryState.h

So yes, there appears to be something fishy here.

Ryan, you are the build tools expert, could you please take a look.

Comment 4

[Ryan VanderMeulen (PTO, back July 27)]

I...don't know why I'm getting NIed here. I'm not a build system peer.

Comment 5

[Jorg K (CEST = GMT+2)]

No, but you could tell us when nsILayoutHistoryState.h disappeared. I can't see it here:
https://hg.mozilla.org/mozilla-central/log/tip/layout/base/nsILayoutHistoryState.h

Comment 6

[Emilio Cobos Álvarez [:emilio]]

nsILayoutHistoryState.h is an autogenerated file from nsILayoutHistoryState.idl, fwiw, and it still exists.

Probably wasn't regenerated properly in the incremental build? IDK.

Comment 7

[Jorg K (CEST = GMT+2)]

Hi Emilio, thanks for the comment, but I don't think it's right, or am I missing something? nsILayoutHistoryState.h *was* a version controlled file, see:
https://dxr.mozilla.org/mozilla-esr52/source/layout/base/nsILayoutHistoryState.h
https://hg.mozilla.org/mozilla-central/log/tip/layout/base/nsILayoutHistoryState.h

People had it left in their source tree, not the object directory, and ran into trouble since it had some stale content. Removing it from the source directory fixed the problem for the three people I heard about.

Comment 8

[Emilio Cobos Álvarez [:emilio]]

Oh, apologies. So we're both right. It was converted to IDL in bug 1265818. I thought we were talking about more recent events :)

Comment 9

[Ben Bucksch (:BenB)]

> Probably wasn't regenerated properly in the incremental build? IDK.

@Emilio, this was a clobber, see initial description.

Comment 10

[Daniel Holbert [:dholbert]]

(In reply to Jorg K (GMT+2) (bustage-fix only, NI for urgent reviews) from comment #7)
> People had it left in their source tree, not the object directory, and ran
> into trouble since it had some stale content. Removing it from the source
> directory fixed the problem for the three people I heard about.

That sounds like a mercurial bug, as best I can tell.

This change *literally renamed the file*:
  https://hg.mozilla.org/mozilla-central/rev/113a43981ef1#l2.1

I don't know how/why it could be left behind in the source directory after that happens.  If anyone can come up with reproducible steps that can cause this condition (the file to be left behind after a commit that renames it), that would be great, and maybe we can proceed with fixing Mercurial to prevent this happening in the future (if the bug hasn't already been fixed upstream).  But otherwise I don't know what can be done here.

In any case, I guess it makes sense to tie this to bug 1265818, though I don't think there's anything that Core|Layout developers can really do with this bug at this point.

Comment 11

[ISHIKAWA, Chiaki]

This still is not fixed, or rather the stale old header file was not removed on my local PC and I got hit, too.

Comment 12

[ISHIKAWA, Chiaki]

(In reply to ISHIKAWA, Chiaki from comment #11)
> This still is not fixed, or rather the stale old header file was not removed
> on my local PC and I got hit, too.

I got hit after I changed the M-C / C-C layout in a new manner. C-C is now a subdirectory of M-C. So this layout change might have uncovered this issue or something. But not sure.