New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 688127 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 674196



Sign in to add a comment

rewrite_to_chrome_style: windows: clang assert: Name is not a simple identifier

Project Member Reported by lukasza@chromium.org, Feb 2 2017

Issue description

When trying to use rewrite_to_chrome_style on Windows, I get the following assert:

Assertion failed: Name.isIdentifier() && "Name is not a simple identifier", file D:\src\chromium\src\third_party\llvm\tools\clang\include\clang/AST/Decl.h, line 238
 
Owner: lukasza@chromium.org
Status: Started (was: Available)
This feels like something that might be out fault, related to how/when we call NamedDecl->getName from the clang tool.  I'll try to see if the problem repros after sprinkling RewriteToChromeStyle.cpp with:

  if (!Node.getDeclName().isIdentifier())
    return false;

BTW: I have trouble rebuilding rewrite_to_chrome_style.exe on Windows.  I've run 
D:\src\chromium\src>python tools/clang/scripts/update.py --force-local-build --without-android --extra-tools rewrite_to_chrome_style
and I expected that after this I would be able to edit RewriteToChromeStyle.cpp and rebuild with
D:\src\chromium\src>ninja -C third_party/llvm-build/Release+Asserts rewrite_to_chrome_style
but this ninja command fails complaining about not being able to find headers:
ninja: Entering directory `third_party/llvm-build/Release+Asserts'
[1/1188] Building CXX object utils\TableGen\CMakeFiles\obj.llvm-tblgen.dir\DAGISelEmitter.cpp.obj
...
D:\src\chromium\src\third_party\llvm\include\llvm/ADT/iterator_range.h(22): fatal error C1083: Cannot open include file: 'utility': No such file or directory

Any suggestions on how to more efficiently rebuild rewrite_to_chrome_style.exe on Windows would be appreciated :-).


I think I've figured out how to rebuild the renaming clang tool - I just need to ensure appropriate environment variables are set, by invoking ninja like this:

D:\src\chromium\src>D:\src\depot_tools\win_toolchain\vs_files\d3cb0e37bdd120ad0ac4650b674b09e81be45616\win_sdk\Bin\SetEnv.Cmd /x64 && ninja -C third_party/llvm-build/Release+Asserts rewrite_to_chrome_style
Protecting getName() calls made by our tool helped avoid this assert.  Fix is in review @ https://codereview.chromium.org/2671713005.
Project Member

Comment 4 by bugdroid1@chromium.org, Feb 2 2017

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

commit 5fa41bde4dca431f3a6843bf20bebf90c2229a08
Author: lukasza <lukasza@chromium.org>
Date: Thu Feb 02 23:41:33 2017

Avoid calling getName() on declarations that don't have an associated identifier.

BUG= 688127 

Review-Url: https://codereview.chromium.org/2671713005
Cr-Commit-Position: refs/heads/master@{#447880}

[modify] https://crrev.com/5fa41bde4dca431f3a6843bf20bebf90c2229a08/tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp

Status: Fixed (was: Started)

Sign in to add a comment