New issue
Advanced search Search tips

Issue 729447 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

[blink-gc] Invalid fields error when templating on garbage collected type

Project Member Reported by shend@chromium.org, Jun 5 2017

Issue description

Code snippet to reproduce:

```
// Simple GC'd class
struct Cat : public GarbageCollected<Cat> { // (A)
  DECLARE_TRACE();
};

// Simple class that uses a GC'd class.
template <class T>
struct Ptr { T* ptr; };

// Simple overloaded template functions
template <class T>
void meow(const T*) {}

template <class T>
void meow(const Ptr<T>&) {}

// Calls meow with explicit template parameters.
// Expect to call meow(const T*).
// error: [blink-gc] Class 'Ptr<blink::Cat>' contains invalid fields.
void boom() {
  meow<Cat>(new Cat); // (B)
}
```

For some reason, the above code snippet causes a blink-gc compile error. The code works fine if we remove GarbageCollected from line (A). It also works if we remove the explicit template parameter on line (B).

I feel like the above code snippet should not cause an error because of SFINAE. Also, it's very strange that it works fine without explicit template parameters. Any ideas as to why this doesn't work?
 

Comment 1 by shend@chromium.org, Jun 5 2017

Components: Blink>MemoryAllocator>GarbageCollection
Labels: -BlinkMemoryAllocatorGarbageCollection

Comment 2 by shend@chromium.org, Jun 5 2017

I put the code snippet into a CL so it's easier to repro: https://codereview.chromium.org/2917353002

Comment 3 by shend@chromium.org, Jun 13 2017

Cc: haraken@chromium.org
+haraken (owner in blink_gc_plugin). Hi Haraken, do you know who's the best person to look at this?
Owner: keishi@chromium.org

Comment 5 by keishi@chromium.org, Jun 14 2017

Right now we check type in the clang AST so I think this is up to clang. Do you have a specific CL where this will be a problem?

Comment 6 by shend@chromium.org, Jun 14 2017

Kind of, I have a workaround patch: https://codereview.chromium.org/2922163002
I wanted to add an overload of DataEquivalent with DataRef (which is a GC'd type). However, that broke DataEquivalent<CSSValue>(...) because it was explicitly specifying the template parameter. So I just changed it to use static_cast. If it's too much trouble to fix this, I'll just stick with this workaround.

Comment 7 by keishi@chromium.org, Jun 14 2017

DataRef nor its template T isn't GCed. Which field will blink gc plugin complain about?

Comment 8 by shend@chromium.org, Jun 14 2017

Ah apologies, I meant that DataRef contains a RefPtr as a member. It complains about the DataRef<CSSValue> instantiation:

[blink-gc] Class 'DataRef<blink::CSSValue>' contains invalid fields.
class DataRef {
^
../../third_party/WebKit/Source/core/style/DataRef.h:68:3: note: [blink-gc] RefPtr field 'data_' to a GC managed class declared here:
  RefPtr<T> data_;
  ^
1 error generated.

I can submit a CL for you to repro if you want.

Comment 9 by keishi@chromium.org, Jun 14 2017

Thanks. I understand how it maps to your minimized test case now.
I've looked at how the plugin is handling this but I think it is up to clang.

Comment 10 by shend@chromium.org, Jun 14 2017

Status: WontFix (was: Untriaged)
Alright no worries, I'll just use the workaround then. Thanks for your help. Marking as Wont-Fix because it's up to clang.
I wonder if this is worth filing a bug against Clang if plugin errors can't participate in SFINAE like regular errors can.

Sign in to add a comment