Skip to content

Conversation

@Fznamznon
Copy link
Contributor

No description provided.

@Fznamznon
Copy link
Contributor Author

This is the case when patch itself is very simple, but it is almost impossible to cover all test cases. I was able to figure some test cases, but I would appreciate suggestions.

Copy link
Contributor

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now seeing how simple the patch is, I think you've covered most of the cases I can think of. I didn't realize an invalid field/base made the parent invalid.

It might be a different bug, but I seem to remember seeing a problem with a member function passing something of its own type to a kernel (since it is incomplete!) and causing trouble.

Obviously the same problem exists for in the definition of the base class:

struct Base {
  struct Derived : Base { void use() const{}
  };
  void foo() {
    Derived d;
    kernel<class Whatever>([=](){d.use();});
  }
}; // End of Base

If that is sufficiently different of a bug, feel free to do that somewhere else.

@Fznamznon
Copy link
Contributor Author

Now seeing how simple the patch is, I think you've covered most of the cases I can think of. I didn't realize an invalid field/base made the parent invalid.

It might be a different bug, but I seem to remember seeing a problem with a member function passing something of its own type to a kernel (since it is incomplete!) and causing trouble.

Obviously the same problem exists for in the definition of the base class:

struct Base {
  struct Derived : Base { void use() const{}
  };
  void foo() {
    Derived d;
    kernel<class Whatever>([=](){d.use();});
  }
}; // End of Base

If that is sufficiently different of a bug, feel free to do that somewhere else.

I've just tried this test case and the output seems valid for me:

test.cpp:3:20: error: base class has incomplete type
  struct Derived : Base { void use() const{}
                   ^~~~
test.cpp:2:8: note: definition of 'Base' is not complete until the closing '}'
struct Base {
       ^
1 error generated.

Looking into AST dump, it seems it is also the case when kernel object becomes invalid:
-LambdaExpr 0x561b64cdeec8 <col:50, col:64> '(lambda at test.cpp:7:50)
|-CXXRecordDecl 0x561b64cdea60 col:50 col:50 implicit invalid class definition
... etc

@erichkeane
Copy link
Contributor

Now seeing how simple the patch is, I think you've covered most of the cases I can think of. I didn't realize an invalid field/base made the parent invalid.
It might be a different bug, but I seem to remember seeing a problem with a member function passing something of its own type to a kernel (since it is incomplete!) and causing trouble.
Obviously the same problem exists for in the definition of the base class:

struct Base {
  struct Derived : Base { void use() const{}
  };
  void foo() {
    Derived d;
    kernel<class Whatever>([=](){d.use();});
  }
}; // End of Base

If that is sufficiently different of a bug, feel free to do that somewhere else.

I've just tried this test case and the output seems valid for me:

test.cpp:3:20: error: base class has incomplete type
  struct Derived : Base { void use() const{}
                   ^~~~
test.cpp:2:8: note: definition of 'Base' is not complete until the closing '}'
struct Base {
       ^
1 error generated.

Looking into AST dump, it seems it is also the case when kernel object becomes invalid:
-LambdaExpr 0x561b64cdeec8 <col:50, col:64> '(lambda at test.cpp:7:50)
|-CXXRecordDecl 0x561b64cdea60 col:50 col:50 implicit invalid class definition
... etc

Perfect, thanks!

@bader bader merged commit 0c220ca into intel:sycl Sep 2, 2020
Chenyang-L pushed a commit that referenced this pull request Feb 18, 2025
[Benchmarks] fix label for comput benchmark
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants