This is an archive of the discontinued LLVM Phabricator instance.

[Clang] Diagnose invalid member variable with template parameters
ClosedPublic

Authored by cor3ntin on Mar 3 2022, 2:34 AM.

Diff Detail

Event Timeline

cor3ntin created this revision.Mar 3 2022, 2:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 3 2022, 2:34 AM
cor3ntin requested review of this revision.Mar 3 2022, 2:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 3 2022, 2:34 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
cor3ntin updated this revision to Diff 412654.Mar 3 2022, 3:10 AM

Fix Formatting.

Thanks for looking into this, that's a rather amusing bug.

clang/lib/Sema/SemaDeclCXX.cpp
3398–3399

No need to look at isInstField, it's already tested above on line 3365.

clang/test/SemaCXX/class.cpp
219 ↗(On Diff #412654)

Can you also add test cases for:

static int k<12>;
void f<12>();

To make sure we diagnose it the same way, both in a non-template class and within a templated class.

cor3ntin updated this revision to Diff 412689.Mar 3 2022, 5:41 AM
  • Fix redundant check
  • move test to p17.cpp which seems more approriate
  • add a couple of tests
erichkeane accepted this revision.Mar 3 2022, 6:12 AM

Do we have a test somewhere to check the variable template specialization case to make sure that didn't break? Otherwise LGTM.

This revision is now accepted and ready to land.Mar 3 2022, 6:12 AM

Do we have a test somewhere to check the variable template specialization case to make sure that didn't break? Otherwise LGTM.

Thanks for the review.
It seems fairly well covered in clang/test/SemaCXX/cxx1y-variable-templates_in_class.cpp

aaron.ballman accepted this revision.Mar 3 2022, 6:46 AM

LGTM (I'm a bit sad about the diagnostic differences, but I understand why they're different a bit better now, so this is fine.)

This revision was landed with ongoing or failed builds.Mar 3 2022, 6:51 AM
This revision was automatically updated to reflect the committed changes.