Incorrect source range for specialized template VarDecl declaration

Hi All,

I’ve run into what looks like a bug (or possibly ambiguous behaviour) when examining specialized static members of a templated class.

template
class TemplatedClass
{
public:
static int Foo;
};

template
int TemplatedClass::Foo = 1;

template <>
int TemplatedClass::Foo = 2;

The specialization generates two VarDecls (as expected), one for the declaration and one for the definition. However if you query getSourceRange() for the declaration it will give you a range extending from the declaration to the definition - rather than one simply covering the declaration. This proves problematic when it comes to rewriting such declarations.

The declaration’s location is set to the definition location by Sema::CheckMemberSpecialization(). In some ways this is correct as the specialization obviously “spawns” the specialized declaration but it does make the SourceRange somewhat useless in this case.

Any thoughts on this? It’s easily resolved by removing the setLocation() call in CheckMemberSpecialization() but my feeling is this may be incorrect (or at least losing information). It’s almost like there should be a “SourceLocation PointOfSpecialization” member in MemberSpecializationInfo - but perhaps I’m completely on the wrong track?

Obviously, I guess I can fall back on querying the non-specialized declaration for the SourceRange but this feels more than a little ugly.

  • Will.

This is a bug. We shouldn't be updating the location of the
declaration when we see the explicit specialization. Patches welcome
:slight_smile:

Righto, thanks for clarifying. Two questions spring to mind:

  1. I assume the same is true for the whole if-else block in CheckMemberSpecialization() that applies TSK_ExplicitSpecialization for various Decl types?

  2. Where should any tests for this change live?

Righto, thanks for clarifying. Two questions spring to mind:

1. I assume the same is true for the whole if-else block in
CheckMemberSpecialization() that applies TSK_ExplicitSpecialization for
various Decl types?

Yes, those calls to setLocation all look wrong. Do you see any test
failures if you remove them?

2. Where should any tests for this change live?

test/SemaCXX/sourceranges.cpp looks like a good candidate.

> 1. I assume the same is true for the whole if-else block in
> CheckMemberSpecialization() that applies TSK_ExplicitSpecialization for
> various Decl types?

Yes, those calls to setLocation all look wrong. Do you see any test
failures if you remove them?

Failing Tests (5):
    Clang :: CXX/temp/temp.spec/temp.expl.spec/p2-0x.cpp
    Clang :: CXX/temp/temp.spec/temp.expl.spec/p2.cpp
    Clang :: CXX/temp/temp.spec/temp.expl.spec/p5.cpp
    Clang :: CXX/temp/temp.spec/temp.explicit/p4.cpp
    Clang :: SemaCXX/warn-unused-filescoped.cpp

It looks like a few diagnostics rely on the location data. All of the fails
seem to be composed of the diagnostics moving from the specialization to
the declaration (from test/CXX/temp/temp.spec/temp.expl.spec/p5.cpp):

error: 'note' diagnostics expected but not seen:
  Line 31: forward declaration
error: 'note' diagnostics seen but not expected:
  Line 15: forward declaration of 'X<IntHolder, long>::Inner'
2 errors generated.

Obviously the tests could be fairly easily updated but it seems there's
more than a little danger of adversely affecting utility of the
diagnostics. I guess the diagnostics would have to be reworked to ensure
they resolved the explicitly specialized definition rather than declaration
location - which sounds like it might be a fair bit of work...

> 2. Where should any tests for this change live?

test/SemaCXX/sourceranges.cpp looks like a good candidate.

Looks good. A few more test cases wouldn't go amiss looking at it :wink:

OK, that diagnostic change in particular looks bad. Seems this will
not be straightforward to fix. :frowning:

I’ll leave it on the back burner. At least there’s a fairly easy workaround for the time being.