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

Issue 645511 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner: ----
Closed: Feb 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 578344



Sign in to add a comment

rewrite_to_chrome_style: Some export-only template specialization declarations don't get rewritten

Project Member Reported by lukasza@chromium.org, Sep 9 2016

Issue description

Example:

Before the rewrite:

    third_party/WebKit/Source/platform/graphics/skia/SkiaUtils.h:

        ...
        template<typename PrimitiveType>
        void drawPlatformFocusRing(const PrimitiveType&, SkCanvas*, SkColor, int width);
        ...

    third_party/WebKit/Source/platform/graphics/skia/SkiaUtils.cpp:

        ...
        template<typename PrimitiveType>
        void drawPlatformFocusRing(const PrimitiveType& primitive, SkCanvas* canvas, SkColor color, int width)
        {
            ...
        }
        ...
        template void PLATFORM_EXPORT drawPlatformFocusRing<SkRect>(const SkRect&, SkCanvas*, SkColor, int width);
        template void PLATFORM_EXPORT drawPlatformFocusRing<SkPath>(const SkPath&, SkCanvas*, SkColor, int width);

The tool rewrites |drawPlatformFocusRing| to |DrawPlatformFocusRing| (upper-case first character) everywhere except the last two occurences.
 
Summary: rewrite_to_chrome_style: Some export-only template specialization declarations don't get rewritten (was: ewrite_to_chrome_style: Some export-only template specialization declarations don't get rewritten)
Small repro for function-templates-original.cc:

namespace rename_missed_in_export_template_declaration {

template <typename PrimitiveType>
void drawPlatformFocusRing(const PrimitiveType&);

template <typename PrimitiveType>
void drawPlatformFocusRing(const PrimitiveType& primitive) {}

class SkPath;
template void drawPlatformFocusRing<SkPath>(const SkPath&);

}  // namespace rename_missed_in_export_template_declaration
Also - these 2 last declarations are important because of the PLATFORM_EXPORT part.  After deleting the last 2 declarations we would get the following linker error in component build (+ a few more similar errors):

obj/third_party/WebKit/Source/platform/platform/GraphicsContext.o:../../third_party/WebKit/Source/platform/graphics/GraphicsContext.cpp:function blink::GraphicsContext::drawFocusRing(WTF::Vector<blink::IntRect, 0ul, WTF::PartitionAllocator> const&, int, int, blink::Color const&): error: undefined reference to 'void blink::drawPlatformFocusRing<SkPath>(SkPath const&, SkCanvas*, unsigned int, int)'

Smaller test:

template <typename PrimitiveType>
void DrawPlatformFocusRing(const PrimitiveType& primitive) {}

template void DrawPlatformFocusRing<int>(const int&);
AST for the smaller test:

`-FunctionTemplateDecl 0x5be9868 </usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_to_chrome_style/tests/function-templates-actual.cc:1:1, line:2:61> col:6 drawPlatformFocusRing
  |-TemplateTypeParmDecl 0x5be95a0 <line:1:11, col:20> col:20 referenced typename PrimitiveType
  |-FunctionDecl 0x5be97c0 <line:2:1, col:61> col:6 drawPlatformFocusRing 'void (const PrimitiveType &)'
  | |-ParmVarDecl 0x5be96b8 <col:28, col:49> col:49 primitive 'const PrimitiveType &'
  | `-CompoundStmt 0x5be9900 <col:60, col:61>
  `-FunctionDecl 0x5be9cb0 <col:1, col:61> col:6 drawPlatformFocusRing 'void (const int &)'
    |-TemplateArgument type 'int'
    |-ParmVarDecl 0x5be9c28 <col:28, col:49> col:49 primitive 'const int &'
    `-CompoundStmt 0x5be9900 <col:60, col:61>
It seems that both FunctionDecl-s above point at the same line in the input.  I'd have to double-check this, but this seems wrong and seems to be similar to (but not quite the same as) https://llvm.org/bugs/show_bug.cgi?id=29145 trouble we had with unnamed parameters.
Cc: lukasza@chromium.org
Owner: ----
Status: Available (was: Started)
Cc: nasko@chromium.org
 Issue 685357  has been merged into this issue.
 Issue 685695  has been merged into this issue.
Status: WontFix (was: Available)
This can be fixed manually (via the manual fixes stashes in https://crrev.com/2329463004).

Sign in to add a comment