[PATCH] D78760: Check a class has a definition before iterating over its base classes

2020-04-28 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D78760#2005874 , @ahatanak wrote: > `TagDecl::isDependentType()` is returning true. Oh, sorry, we do already do the thing I was suggesting. But in your testcase the issue is that the base class `B0` is dependent but is also

[PATCH] D78760: Check a class has a definition before iterating over its base classes

2020-04-27 Thread Jan Korous via Phabricator via cfe-commits
jkorous added a comment. In D78760#2003118 , @rsmith wrote: > If so, we'll need to make sure that all code that iterates over base classes > checks for this condition (I bet there are more cases than the two that you > found). Just a potentially

[PATCH] D78760: Check a class has a definition before iterating over its base classes

2020-04-27 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. `TagDecl::isDependentType()` is returning true. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78760/new/ https://reviews.llvm.org/D78760 ___ cfe-commits mailing list

[PATCH] D78760: Check a class has a definition before iterating over its base classes

2020-04-27 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. I think the non-dependent base class you mention in this case is `B0`, but it seems that the type of `B0` in the AST is dependent as `isDependentType` returns true. Should we fix this? Or does `isDependentType` mean something different? Repository: rG LLVM Github

[PATCH] D78760: Check a class has a definition before iterating over its base classes

2020-04-24 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. We should reject a definition of a class that has a non-dependent base class that's an incomplete type. We need non-dependent bases to be complete in order to be able to do name lookup into them when processing the template definition. (The C++ standard permits us to

[PATCH] D78760: Check a class has a definition before iterating over its base classes

2020-04-23 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. I'm not sure why this shouldn't be caught in `Sema::CheckBaseSpecifier`. But there is a check for the definition of the class before `findCircularInheritance` is called, so I'm guessing there is a reason the code shouldn't be rejected here. Repository: rG LLVM

[PATCH] D78760: Check a class has a definition before iterating over its base classes

2020-04-23 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak created this revision. ahatanak added a reviewer: rsmith. ahatanak added a project: clang. Herald added subscribers: ributzka, dexonsmith, jkorous. ahatanak added a comment. I'm not sure why this shouldn't be caught in `Sema::CheckBaseSpecifier`. But there is a check for the definition