Closed Bug 1318110 Opened 3 years ago Closed 3 years ago

Incorrect handling of errors in nested IPDL parser

Categories

(Core :: IPC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: bkelly, Assigned: mccr8)

References

Details

Attachments

(2 files, 1 obsolete file)

I recently tried to add the ClientType enum import to an ipdlh file:

========================
/* This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

namespace mozilla {
namespace dom {

using mozilla::dom::ClientType from "ClientType.h"

struct ClientHandleConstructorArgs
{
  bool todo;
};

struct ClientSourceConstructorArgs
{
  mozilla::dom::ClientType type;
};

} // namespace dom
} // namespace mozilla
=============================

In return the ipdl compiler gave me:

 0:06.32   in file included from `/srv/mozilla-central/dom/clients/PClientManager.ipdl', line 8:
 0:06.32 /srv/mozilla-central/dom/clients/ClientIPCTypes.ipdlh:46: error: can't define a protocol in a header.  Do it in a protocol spec instead.
 0:06.32 Specification could not be parsed.


This error message is bad for a few reasons:

1. There are not 46 lines in ClientIPCTypes.ipdlh.  It only has 21 lines.
2. I'm not trying to introduce a protocol.
3. It doesn't tell me what symbol its choking on.
Andrew, you asked me to file a bug for bad ipdl compiler error messages.  Here is one I just ran into.
Flags: needinfo?(continuation)
The end problem here was that this line:

  using mozilla::dom::ClientType from "ClientType.h"

Referenced a header it couldn't find in other directories.  After I exported the header and changed it to "mozilla/dom/ClientType.h" then it compiled.

The error message really doesn't help, though.
(In reply to Ben Kelly [:bkelly] from comment #2)
> The end problem here was that this line:
> 
>   using mozilla::dom::ClientType from "ClientType.h"
> 
> Referenced a header it couldn't find in other directories.  After I exported
> the header and changed it to "mozilla/dom/ClientType.h" then it compiled.
> 
> The error message really doesn't help, though.

Actually, its still failing.  Not sure what the problem is. :-\
Turns out the problem was that the line:

  using mozilla::dom::ClientType from "ClientType.h"

Was inside the namespace in the ipdlh file.
That's odd. Putting "using" inside "namespace" should be a parser error.
Flags: needinfo?(continuation)
Assignee: nobody → continuation
When I created a new file as given in comment 0, I did get a parser error:
 0:08.27 /home/amccreight/mc/dom/ipc/Foo.ipdlh:8: error: bad syntax near `using'
 0:08.27 Specification could not be parsed.
Attached patch Example files. (obsolete) — Splinter Review
Ben, what were you doing differently from what I have in the patch? I'd like to figure out how it was triggering that error. Thanks.
Flags: needinfo?(bkelly)
I believe the error was from when the ipdlh was being included from an ipdl file.
Flags: needinfo?(bkelly)
Attached patch Example files.Splinter Review
That did the trick. Thanks.
Attachment #8811875 - Attachment is obsolete: true
Summary: ipdl compiler error messages can be unhelpful → Incorrect handling of errors in nested IPDL parser
There's details in the commit message, but basically handling parser errors inside an included file was totally broken, and has been since 2009.
Comment on attachment 8811954 [details]
Bug 1318110 - Correctly handle parse errors in inner IPDL parsers.

https://reviewboard.mozilla.org/r/93846/#review94044
Attachment #8811954 - Flags: review?(wmccloskey) → review+
Pushed by amccreight@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1156c54cdb68
Correctly handle parse errors in inner IPDL parsers. r=billm
https://hg.mozilla.org/mozilla-central/rev/1156c54cdb68
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Blocks: 1318743
You need to log in before you can comment on or make changes to this bug.