Hey,
I’ve been looking into a difference in compilation warnings between MSVC and clang with the following modified test code from clang/test/CXX/basic/basic.def/p2.cpp:
1 // RUN: %clang_cc1 -std=c++1z -verify %s -Wdeprecated
2
3 namespace {
4 struct A {
5 static constexpr int n = 0;
6 };
7 const int A::n; // expected-warning {{deprecated}}
8
9 struct B {
10 static constexpr int m = 0;
11 };
12 constexpr int B::m; // expected-warning {{deprecated}}
13 }
Compiler Explorer shows that MSVC emits warnings for both lines 7 and 12, but clang only emits a warning for the first line. Through debugging clang I found the difference is in the return value for New->isThisDeclarationADefinition()
on line 4540 of clang/lib/Sema/SemaDecl.cpp:
4538 // C++ doesn't have tentative definitions, so go right ahead and check here.
4539 if (getLangOpts().CPlusPlus &&
4540 New->isThisDeclarationADefinition() == VarDecl::Definition) {
4541 if (Old->isStaticDataMember() && Old->getCanonicalDecl()->isInline() &&
4542 Old->getCanonicalDecl()->isConstexpr()) {
4543 // This definition won't be a definition any more once it's been merged.
4544 Diag(New->getLocation(),
4545 diag::warn_deprecated_redundant_constexpr_static_def);
4546 } else if (VarDecl *Def = Old->getDefinition()) {
4547 if (checkVarDeclRedefinition(Def, New))
4548 return;
4549 }
4550 }
With A::n
returning VarDecl::Definition
and B::m
returning VarDecl::DeclarationOnly
. The relevant code inside clang/lib/AST/Decl.cpp is below, I’ve added the comment on lines 2184-2185 in my local copy:
2164 VarDecl::DefinitionKind
2165 VarDecl::isThisDeclarationADefinition(ASTContext &C) const {
2166 if (isThisDeclarationADemotedDefinition())
2167 return DeclarationOnly;
2168
2169 // C++ [basic.def]p2:
2170 // A declaration is a definition unless [...] it contains the 'extern'
2171 // specifier or a linkage-specification and neither an initializer [...],
2172 // it declares a non-inline static data member in a class declaration [...],
2173 // it declares a static data member outside a class definition and the variable
2174 // was defined within the class with the constexpr specifier [...],
2175 // C++1y [temp.expl.spec]p15:
2176 // An explicit specialization of a static data member or an explicit
2177 // specialization of a static data member template is a definition if the
2178 // declaration includes an initializer; otherwise, it is a declaration.
2179 //
2180 // FIXME: How do you declare (but not define) a partial specialization of
2181 // a static data member template outside the containing class?
2182 if (isStaticDataMember()) {
2183 if (isOutOfLine() &&
2184 // Both false in 'const' case, so we return "Definition", both true in 'constexpr'
2185 // case, so we return "DeclarationOnly"
2186 !(getCanonicalDecl()->isInline() &&
2187 getCanonicalDecl()->isConstexpr()) &&
2188 (hasInit() ||
2189 // If the first declaration is out-of-line, this may be an
2190 // instantiation of an out-of-line partial specialization of a variable
2191 // template for which we have not yet instantiated the initializer.
2192 (getFirstDecl()->isOutOfLine()
2193 ? getTemplateSpecializationKind() == TSK_Undeclared
2194 : getTemplateSpecializationKind() !=
2195 TSK_ExplicitSpecialization) ||
2196 isa<VarTemplatePartialSpecializationDecl>(this)))
2197 return Definition;
2198 if (!isOutOfLine() && isInline())
2199 return Definition;
2200 return DeclarationOnly;
2201 }
In the A::n
case the getCanonicalDecl()
calls on lines 2186 and 2187 return the declaration on line 7 of the example code, which is neither inline nor a constexpr, so the if expression started on line 2183 ultimately evaluates as true
. In the B::m
input case getCanonicalDecl()
returns the definition on line 10 of the example code, which is both inline and a constexpr, so the if condition is false.
This is where I’m confused. My reading of the standard comment included above is that the intention of this code is that getCanonicalDecl()
in the A::n
case should return the first declaration of A::n
, which is on line 5 of the example input. Therefore the declaration on line 7 is not a definition, because it “declares a static member outside a class definition and the variable was defined within the class with the constexpr specifier.”
If that’s so, then, both getCanonicalDecl()
and the calling code in SemaDecl.cpp are erroneous. But the intention of the warning, IIUC, is to point out redundant definitions that in C++17 are back to declarations. But I also don’t understand how const int A::n
would have been thought of as a definition at any C++ standard.
I’d really appreciate any clarification that folks can provide.
Thanks!