Rebase tool gets confused by functions returning bools |
||
Issue descriptionWe have some logic to try to avoid incorrectly rewriting certain template parameters. However, bool is a macro on some platforms, and this confuses the logic to ignore unnamed template parameters.
,
Apr 13 2017
../../base/bind_unittest.cc:21:9: error: 'bool' macro redefined [-Werror,-Wmacro-redefined]
#define bool MOO
^
/usr/local/google/home/dcheng/src/chrome/src/third_party/llvm-build/Release+Asserts/lib/clang/5.0.0$
include/stdbool.h:37:9: note: previous definition is here
#define bool bool
Apparently it's a GNU extension?
,
Apr 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b029f9994f46b5f178b43740b3599832dcb763fa commit b029f9994f46b5f178b43740b3599832dcb763fa Author: Daniel Cheng <dcheng@chromium.org> Date: Fri Apr 14 22:15:34 2017 rewrite_to_chrome_style: rewrite parameter names of functions that return bool This function deletes code that was originally added to work around https://llvm.org/bugs/show_bug.cgi?id=29145, since it incorrectly rejected rewrites to function parameters if the function returned bool: this is because bool is actually a macro on Linux (!!). However, this exposes another bug for macro arguments that expanded in multiple contexts: if the macro argument is expanded into the name of a function and the name of a variable, the original text is incorrectly rewritten twice: once to rewrite the name to be treated as a variable name, and the second to uppercase the first letter. While apply_edits.py should have caught this as a conflicting edit, it does not for some reason--and moreover, in this case, it is preferable to preserve function-style naming, as the effects are visible outside the macro itself. The fix for this is to create a global table of SourceLocations with replacements, and avoid emitting subsequent replacements for the same SourceLocation. This works because MatchFinder guarantees that match ordering is equivalent to doing a pre-order traversal on the AST and applying the matchers in the order in which they were added: in the AST, the FunctionDecl always comes before the ParmVarDecl, allowing the FunctionDecl naming to override the ParmVarDecl naming. Bug: 711128 Change-Id: I4333c782526ffd4abb62b0d8f881b11a24b0c7e7 Reviewed-on: https://chromium-review.googlesource.com/478471 Commit-Queue: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Lukasz Anforowicz <lukasza@chromium.org> Cr-Commit-Position: refs/heads/master@{#464807} [modify] https://crrev.com/b029f9994f46b5f178b43740b3599832dcb763fa/tools/blink_rename_merge_helper/COMPONENTS [modify] https://crrev.com/b029f9994f46b5f178b43740b3599832dcb763fa/tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp [add] https://crrev.com/b029f9994f46b5f178b43740b3599832dcb763fa/tools/clang/rewrite_to_chrome_style/tests/bool-is-macro-expected.cc [add] https://crrev.com/b029f9994f46b5f178b43740b3599832dcb763fa/tools/clang/rewrite_to_chrome_style/tests/bool-is-macro-original.cc [modify] https://crrev.com/b029f9994f46b5f178b43740b3599832dcb763fa/tools/clang/rewrite_to_chrome_style/tests/macros-expected.cc [modify] https://crrev.com/b029f9994f46b5f178b43740b3599832dcb763fa/tools/clang/rewrite_to_chrome_style/tests/macros-original.cc [modify] https://crrev.com/b029f9994f46b5f178b43740b3599832dcb763fa/tools/clang/rewrite_to_chrome_style/tests/variables-expected.cc
,
Apr 14 2017
|
||
►
Sign in to add a comment |
||
Comment 1 by thakis@chromium.org
, Apr 13 2017