New issue
Advanced search Search tips

Issue 740524 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug

Blocking:
issue 617899



Sign in to add a comment

bindings: idl_parser's parser tests are broken

Project Member Reported by raphael....@intel.com, Jul 10 2017

Issue description

Sorry for filing a new bug; I wasn't sure if issue 617899 is being regularly checked for updates.

https://chromium-review.googlesource.com/544424 ("IDL parser: Ignore comments") had the curious side-effect of always making the IDL parser tests pass, as

  def _TestNode(self, node):
    comments = node.GetListOf('Comment')

now always returns an empty list. Our pass rate is now always 100%, but I guess that wasn't part of the plan :-)
 
Labels: -Pri-3 Pri-1
Cc: haraken@chromium.org yukishiino@chromium.org peria@chromium.org
Owner: bashi@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f2f3c9210fe321b1418bf22b5bfbcf2d5bba1332

commit f2f3c9210fe321b1418bf22b5bfbcf2d5bba1332
Author: Kenichi Ishibashi <bashi@chromium.org>
Date: Fri Jul 14 02:02:40 2017

IDL parser: Parse special comments as AST nodes

Tests in tools/idl_parser are broken as of [1] because the current IDL
parser discards comments but tests assumed that there are comment nodes
which contain test expectations. This CL is basically a revert of [1]
with following modifications:
- Introduce SpecialComments instead of Comments
- SpecialComments are Javadoc-style comment (/** ... */) and they are
  parsed to AST nodes
- Normal comments (C/C++ style; /* ... */ and // ...) are still ignored

The reason to introduce SpecialComments: If we treat normal comments as
tokens and parse them as AST nodes, we have to add a lot of custom rules
to the parser, which will mess up the parser.

[1] https://chromium-review.googlesource.com/c/544424

BUG= 740524 

Change-Id: I872b6c19e6b844859bbcaa48c828decda36cb300
Reviewed-on: https://chromium-review.googlesource.com/569940
Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486629}
[modify] https://crrev.com/f2f3c9210fe321b1418bf22b5bfbcf2d5bba1332/tools/idl_parser/idl_lexer.py
[modify] https://crrev.com/f2f3c9210fe321b1418bf22b5bfbcf2d5bba1332/tools/idl_parser/idl_node.py
[modify] https://crrev.com/f2f3c9210fe321b1418bf22b5bfbcf2d5bba1332/tools/idl_parser/idl_parser.py
[modify] https://crrev.com/f2f3c9210fe321b1418bf22b5bfbcf2d5bba1332/tools/idl_parser/idl_parser_test.py
[modify] https://crrev.com/f2f3c9210fe321b1418bf22b5bfbcf2d5bba1332/tools/idl_parser/test_parser/callback_web.idl
[modify] https://crrev.com/f2f3c9210fe321b1418bf22b5bfbcf2d5bba1332/tools/idl_parser/test_parser/dictionary_web.idl
[modify] https://crrev.com/f2f3c9210fe321b1418bf22b5bfbcf2d5bba1332/tools/idl_parser/test_parser/enum_web.idl
[modify] https://crrev.com/f2f3c9210fe321b1418bf22b5bfbcf2d5bba1332/tools/idl_parser/test_parser/implements_web.idl
[modify] https://crrev.com/f2f3c9210fe321b1418bf22b5bfbcf2d5bba1332/tools/idl_parser/test_parser/interface_web.idl
[modify] https://crrev.com/f2f3c9210fe321b1418bf22b5bfbcf2d5bba1332/tools/idl_parser/test_parser/typedef_web.idl

Status: Fixed (was: Assigned)

Sign in to add a comment