[C23] Implement N3018: The constexpr specifier for object definitions #73099
Conversation
The implementation mostly reuses C++ code paths where possible, including narrowing check in order to provide diagnostic messages in case initializer for constexpr variable is not exactly representable in target type. The following won't work due to lack of support for other features: - Diagnosing of underspecified declarations involving constexpr - Constexpr attached to compound literals Also due to lack of support for char8_t some of examples with utf-8 strings don't work properly.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Mariya Podchishchaeva (Fznamznon) ChangesThe implementation mostly reuses C++ code paths where possible, including narrowing check in order to provide diagnostic messages in case initializer for constexpr variable is not exactly representable in target type. The following won't work due to lack of support for other features:
Also due to lack of support for char8_t some of examples with utf-8 strings don't work properly. Fixes #64742 Patch is 38.06 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/73099.diff 12 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index b65106b9106d4d7..cae1707f3e30feb 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -208,6 +208,7 @@ C23 Feature Support
- Clang now supports ``<stdckdint.h>`` which defines several macros for performing
checked integer arithmetic. It is also exposed in pre-C23 modes.
+- Clang now supports ``N3018 The constexpr specifier for object definitions``.
- Completed the implementation of
`N2508 <https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2508.pdf>`_. We
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 990692c06d7d3a8..11f24583dc55a9b 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2932,6 +2932,22 @@ def warn_private_extern : Warning<
def note_private_extern : Note<
"use __attribute__((visibility(\"hidden\"))) attribute instead">;
+// C23 constexpr
+def err_c23_thread_local_constexpr : Error<
+ "thread-local storage is not allowed with constexpr">;
+def err_c23_extern_constexpr : Error<
+ "extern specifier is not allowed with constexpr">;
+def err_c23_constexpr_not_variable : Error<
+ "constexpr is only allowed in variable declarations">;
+def err_c23_constexpr_invalid_type : Error<
+ "constexpr variable cannot have type %0">;
+def err_c23_constexpr_init_not_representable : Error<
+ "constexpr initializer evaluates to %0 which is not exactly representable in type %1">;
+def err_c23_constexpr_init_type_mismatch : Error<
+ "constexpr initializer for type %0 is of type %1">;
+def err_c23_constexpr_pointer_not_null : Error<
+ "constexpr pointer initializer is not null">;
+
// C++ Concepts
def err_concept_decls_may_only_appear_in_global_namespace_scope : Error<
"concept declarations may only appear in global or namespace scope">;
diff --git a/clang/include/clang/Basic/TokenKinds.def b/clang/include/clang/Basic/TokenKinds.def
index 3ab420821d82bfe..e9e8f59247662ea 100644
--- a/clang/include/clang/Basic/TokenKinds.def
+++ b/clang/include/clang/Basic/TokenKinds.def
@@ -393,7 +393,7 @@ CXX11_KEYWORD(alignas , KEYC23)
CXX11_UNARY_EXPR_OR_TYPE_TRAIT(alignof, AlignOf, KEYC23)
CXX11_KEYWORD(char16_t , KEYNOMS18)
CXX11_KEYWORD(char32_t , KEYNOMS18)
-CXX11_KEYWORD(constexpr , 0)
+CXX11_KEYWORD(constexpr , KEYC23)
CXX11_KEYWORD(decltype , 0)
CXX11_KEYWORD(noexcept , 0)
CXX11_KEYWORD(nullptr , KEYC23)
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index c5c2edf1bfe3aba..678a366ed29ad78 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -2461,7 +2461,7 @@ bool VarDecl::mightBeUsableInConstantExpressions(const ASTContext &C) const {
// OpenCL permits const integral variables to be used in constant
// expressions, like in C++98.
- if (!Lang.CPlusPlus && !Lang.OpenCL)
+ if (!Lang.CPlusPlus && !Lang.OpenCL && !Lang.C23)
return false;
// Function parameters are never usable in constant expressions.
@@ -2485,12 +2485,12 @@ bool VarDecl::mightBeUsableInConstantExpressions(const ASTContext &C) const {
// In C++, const, non-volatile variables of integral or enumeration types
// can be used in constant expressions.
- if (getType()->isIntegralOrEnumerationType())
+ if (getType()->isIntegralOrEnumerationType() && !Lang.C23)
return true;
// Additionally, in C++11, non-volatile constexpr variables can be used in
// constant expressions.
- return Lang.CPlusPlus11 && isConstexpr();
+ return (Lang.CPlusPlus11 || Lang.C23) && isConstexpr();
}
bool VarDecl::isUsableInConstantExpressions(const ASTContext &Context) const {
@@ -2568,11 +2568,11 @@ APValue *VarDecl::evaluateValueImpl(SmallVectorImpl<PartialDiagnosticAt> &Notes,
bool Result = Init->EvaluateAsInitializer(Eval->Evaluated, Ctx, this, Notes,
IsConstantInitialization);
- // In C++, this isn't a constant initializer if we produced notes. In that
+ // In C++/C23, this isn't a constant initializer if we produced notes. In that
// case, we can't keep the result, because it may only be correct under the
// assumption that the initializer is a constant context.
- if (IsConstantInitialization && Ctx.getLangOpts().CPlusPlus &&
- !Notes.empty())
+ if (IsConstantInitialization &&
+ (Ctx.getLangOpts().CPlusPlus || Ctx.getLangOpts().C23) && !Notes.empty())
Result = false;
// Ensure the computed APValue is cleaned up later if evaluation succeeded,
@@ -2630,7 +2630,9 @@ bool VarDecl::checkForConstantInitialization(
// std::is_constant_evaluated()).
assert(!Eval->WasEvaluated &&
"already evaluated var value before checking for constant init");
- assert(getASTContext().getLangOpts().CPlusPlus && "only meaningful in C++");
+ assert((getASTContext().getLangOpts().CPlusPlus ||
+ getASTContext().getLangOpts().C23) &&
+ "only meaningful in C++/C23");
assert(!getInit()->isValueDependent());
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 3a41e9718bb5875..fdd4bbafbb73c22 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2231,6 +2231,13 @@ static bool CheckLValueConstantExpression(EvalInfo &Info, SourceLocation Loc,
return false;
}
+ if (Info.getLangOpts().C23) {
+ auto *VarD = dyn_cast_or_null<VarDecl>(BaseVD);
+ if (VarD && VarD->isConstexpr() && !LVal.isNullPointer()) {
+ Info.report(Loc, diag::err_c23_constexpr_pointer_not_null);
+ }
+ }
+
// Check that the object is a global. Note that the fake 'this' object we
// manufacture when checking potential constant expressions is conservatively
// assumed to be global here.
@@ -4110,6 +4117,10 @@ static CompleteObject findCompleteObject(EvalInfo &Info, const Expr *E,
}
bool IsConstant = BaseType.isConstant(Info.Ctx);
+ bool ConstexprVar = false;
+ if (const auto *VD = dyn_cast_or_null<VarDecl>(
+ Info.EvaluatingDecl.dyn_cast<const ValueDecl *>()))
+ ConstexprVar = VD->isConstexpr();
// Unless we're looking at a local variable or argument in a constexpr call,
// the variable we're reading must be const.
@@ -4129,6 +4140,9 @@ static CompleteObject findCompleteObject(EvalInfo &Info, const Expr *E,
return CompleteObject();
} else if (VD->isConstexpr()) {
// OK, we can read this variable.
+ } else if (Info.getLangOpts().C23 && ConstexprVar) {
+ Info.FFDiag(E);
+ return CompleteObject();
} else if (BaseType->isIntegralOrEnumerationType()) {
if (!IsConstant) {
if (!IsAccess)
@@ -15704,7 +15718,8 @@ bool Expr::EvaluateAsInitializer(APValue &Value, const ASTContext &Ctx,
EStatus.Diag = &Notes;
EvalInfo Info(Ctx, EStatus,
- (IsConstantInitialization && Ctx.getLangOpts().CPlusPlus)
+ (IsConstantInitialization &&
+ (Ctx.getLangOpts().CPlusPlus || Ctx.getLangOpts().C23))
? EvalInfo::EM_ConstantExpression
: EvalInfo::EM_ConstantFold);
Info.setEvaluatingDecl(VD, Value);
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 8cb5b09fd3b0fa6..7059631515c873f 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -4168,6 +4168,8 @@ void Parser::ParseDeclarationSpecifiers(
// constexpr, consteval, constinit specifiers
case tok::kw_constexpr:
+ if (getLangOpts().C23)
+ Diag(Tok, diag::warn_c23_compat_keyword) << Tok.getName();
isInvalid = DS.SetConstexprSpec(ConstexprSpecKind::Constexpr, Loc,
PrevSpec, DiagID);
break;
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 4e1857b931cc868..1f74d937bf97ad1 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -5152,13 +5152,17 @@ Decl *Sema::ParsedFreeStandingDeclSpec(Scope *S, AccessSpecifier AS,
// and definitions of functions and variables.
// C++2a [dcl.constexpr]p1: The consteval specifier shall be applied only to
// the declaration of a function or function template
- if (Tag)
+ if (Tag) {
Diag(DS.getConstexprSpecLoc(), diag::err_constexpr_tag)
<< GetDiagnosticTypeSpecifierID(DS)
<< static_cast<int>(DS.getConstexprSpecifier());
- else
- Diag(DS.getConstexprSpecLoc(), diag::err_constexpr_wrong_decl_kind)
- << static_cast<int>(DS.getConstexprSpecifier());
+ } else {
+ if (getLangOpts().C23)
+ Diag(DS.getConstexprSpecLoc(), diag::err_c23_constexpr_not_variable);
+ else
+ Diag(DS.getConstexprSpecLoc(), diag::err_constexpr_wrong_decl_kind)
+ << static_cast<int>(DS.getConstexprSpecifier());
+ }
// Don't emit warnings after this error.
return TagD;
}
@@ -7894,6 +7898,17 @@ NamedDecl *Sema::ActOnVariableDeclarator(
(getLangOpts().CPlusPlus17 ||
Context.getTargetInfo().getCXXABI().isMicrosoft()))
NewVD->setImplicitlyInline();
+
+ if (getLangOpts().C23) {
+ DeclSpec::TSCS TSC = D.getDeclSpec().getThreadStorageClassSpec();
+ if (TSC != TSCS_unspecified) {
+ Diag(D.getDeclSpec().getThreadStorageClassSpecLoc(),
+ diag::err_c23_thread_local_constexpr);
+ }
+ if (NewVD->hasExternalStorage())
+ Diag(D.getDeclSpec().getThreadStorageClassSpecLoc(),
+ diag::err_c23_extern_constexpr);
+ }
break;
case ConstexprSpecKind::Constinit:
@@ -8605,6 +8620,27 @@ static bool checkForConflictWithNonVisibleExternC(Sema &S, const T *ND,
return false;
}
+static bool CheckC23ConstexprVarTypeQualifiers(Sema &SemaRef,
+ SourceLocation VarLoc, QualType T) {
+ if (const auto *A = SemaRef.Context.getAsArrayType(T)) {
+ T = A->getElementType();
+ }
+
+ if (T->isAtomicType() || T.isVolatileQualified() || T.isRestrictQualified()) {
+ SemaRef.Diag(VarLoc, diag::err_c23_constexpr_invalid_type) << T;
+ return true;
+ }
+
+ if (T->isRecordType()) {
+ RecordDecl *RD = T->getAsRecordDecl();
+ for (const auto &F : RD->fields())
+ if (CheckC23ConstexprVarTypeQualifiers(SemaRef, VarLoc, F->getType()))
+ return true;
+ }
+
+ return false;
+}
+
void Sema::CheckVariableDeclarationType(VarDecl *NewVD) {
// If the decl is already known invalid, don't check it.
if (NewVD->isInvalidDecl())
@@ -8857,7 +8893,9 @@ void Sema::CheckVariableDeclarationType(VarDecl *NewVD) {
if (NewVD->isConstexpr() && !T->isDependentType() &&
RequireLiteralType(NewVD->getLocation(), T,
- diag::err_constexpr_var_non_literal)) {
+ getLangOpts().C23
+ ? diag::err_c23_constexpr_invalid_type
+ : diag::err_constexpr_var_non_literal)) {
NewVD->setInvalidDecl();
return;
}
@@ -8870,6 +8908,12 @@ void Sema::CheckVariableDeclarationType(VarDecl *NewVD) {
return;
}
+ if (getLangOpts().C23 && NewVD->isConstexpr() &&
+ CheckC23ConstexprVarTypeQualifiers(*this, NewVD->getLocation(), T)) {
+ NewVD->setInvalidDecl();
+ return;
+ }
+
// Check that SVE types are only used in functions with SVE available.
if (T->isSVESizelessBuiltinType() && isa<FunctionDecl>(CurContext)) {
const FunctionDecl *FD = cast<FunctionDecl>(CurContext);
@@ -9237,6 +9281,22 @@ static FunctionDecl *CreateNewFunctionDecl(Sema &SemaRef, Declarator &D,
FunctionDecl *NewFD = nullptr;
bool isInline = D.getDeclSpec().isInlineSpecified();
+ ConstexprSpecKind ConstexprKind = D.getDeclSpec().getConstexprSpecifier();
+ if (ConstexprKind == ConstexprSpecKind::Constinit ||
+ (SemaRef.getLangOpts().C23 &&
+ ConstexprKind == ConstexprSpecKind::Constexpr)) {
+
+ if (SemaRef.getLangOpts().C23)
+ SemaRef.Diag(D.getDeclSpec().getConstexprSpecLoc(),
+ diag::err_c23_constexpr_not_variable);
+ else
+ SemaRef.Diag(D.getDeclSpec().getConstexprSpecLoc(),
+ diag::err_constexpr_wrong_decl_kind)
+ << static_cast<int>(ConstexprKind);
+ ConstexprKind = ConstexprSpecKind::Unspecified;
+ D.getMutableDeclSpec().ClearConstexprSpec();
+ }
+
if (!SemaRef.getLangOpts().CPlusPlus) {
// Determine whether the function was written with a prototype. This is
// true when:
@@ -9270,15 +9330,6 @@ static FunctionDecl *CreateNewFunctionDecl(Sema &SemaRef, Declarator &D,
}
ExplicitSpecifier ExplicitSpecifier = D.getDeclSpec().getExplicitSpecifier();
-
- ConstexprSpecKind ConstexprKind = D.getDeclSpec().getConstexprSpecifier();
- if (ConstexprKind == ConstexprSpecKind::Constinit) {
- SemaRef.Diag(D.getDeclSpec().getConstexprSpecLoc(),
- diag::err_constexpr_wrong_decl_kind)
- << static_cast<int>(ConstexprKind);
- ConstexprKind = ConstexprSpecKind::Unspecified;
- D.getMutableDeclSpec().ClearConstexprSpec();
- }
Expr *TrailingRequiresClause = D.getTrailingRequiresClause();
SemaRef.CheckExplicitObjectMemberFunction(DC, D, Name, R);
@@ -13786,7 +13837,9 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
VDecl->setStorageClass(SC_Extern);
// C99 6.7.8p4. All file scoped initializers need to be constant.
- if (!getLangOpts().CPlusPlus && !VDecl->isInvalidDecl())
+ // Avoid double diagnosing for constexpr variables.
+ if (!getLangOpts().CPlusPlus && !VDecl->isInvalidDecl() &&
+ !VDecl->isConstexpr())
CheckForConstantInitializer(Init, DclT);
}
@@ -14240,6 +14293,115 @@ StmtResult Sema::ActOnCXXForRangeIdentifier(Scope *S, SourceLocation IdentLoc,
: IdentLoc);
}
+static ImplicitConversionKind getConversionKind(QualType FromType,
+ QualType ToType) {
+ if (ToType->isIntegerType()) {
+ if (FromType->isComplexType())
+ return ICK_Complex_Real;
+ if (FromType->isFloatingType())
+ return ICK_Floating_Integral;
+ if (FromType->isIntegerType())
+ return ICK_Integral_Conversion;
+ }
+
+ if (ToType->isFloatingType()) {
+ if (FromType->isComplexType())
+ return ICK_Complex_Real;
+ if (FromType->isFloatingType())
+ return ICK_Floating_Conversion;
+ if (FromType->isIntegerType())
+ return ICK_Floating_Integral;
+ }
+
+ return ICK_Identity;
+}
+
+static bool checkC23ConstexprInitConversion(Sema &S, const Expr *Init) {
+ assert(S.getLangOpts().C23);
+ const Expr *InitNoCast = Init->IgnoreImpCasts();
+ StandardConversionSequence SCS;
+ SCS.setAsIdentityConversion();
+ auto FromType = InitNoCast->getType();
+ auto ToType = Init->getType();
+ SCS.setToType(0, FromType);
+ SCS.setToType(1, ToType);
+ SCS.Second = getConversionKind(FromType, ToType);
+
+ APValue Value;
+ QualType PreNarrowingType;
+ // Reuse C++ narrowing check.
+ switch (SCS.getNarrowingKind(S.Context, Init, Value, PreNarrowingType,
+ /*IgnoreFloatToIntegralConversion*/ false)) {
+ // The value doesn't fit.
+ case NK_Constant_Narrowing:
+ S.Diag(Init->getBeginLoc(), diag::err_c23_constexpr_init_not_representable)
+ << Value.getAsString(S.Context, PreNarrowingType) << ToType;
+ return true;
+
+ // Conversion to a narrower type.
+ case NK_Type_Narrowing:
+ S.Diag(Init->getBeginLoc(), diag::err_c23_constexpr_init_type_mismatch)
+ << ToType << FromType;
+ return true;
+
+ // Since we only reuse narrowing check for C23 constexpr variables here, we're
+ // not really interested in these cases.
+ case NK_Dependent_Narrowing:
+ case NK_Variable_Narrowing:
+ case NK_Not_Narrowing:
+ return false;
+
+ }
+ llvm_unreachable("unhandled case in switch");
+}
+
+static bool checkC23ConstexprInitStringLiteral(const StringLiteral *SE,
+ Sema &SemaRef,
+ SourceLocation Loc) {
+ assert(SemaRef.getLangOpts().C23);
+ // String literals have the target type attached but underneath may contain
+ // values that don't really fit into the target type. Check that every
+ // character fits.
+ const ConstantArrayType *CAT =
+ SemaRef.Context.getAsConstantArrayType(SE->getType());
+ QualType CharType = CAT->getElementType();
+ uint32_t BitWidth = SemaRef.Context.getTypeSize(CharType);
+ bool isUnsigned = CharType->isUnsignedIntegerType();
+ llvm::APSInt Value(BitWidth, isUnsigned);
+ const StringRef S = SE->getBytes();
+ for (unsigned I = 0, N = SE->getLength(); I != N; ++I) {
+ Value = S[I];
+ if (Value != S[I]) {
+ SemaRef.Diag(Loc, diag::err_c23_constexpr_init_not_representable)
+ << S[I] << CharType;
+ return true;
+ }
+ }
+ return false;
+}
+
+static bool checkC23ConstexprInitializer(Sema &S, const Expr *Init) {
+ const Expr *InitNoCast = Init->IgnoreImpCasts();
+ if (Init->getType() != InitNoCast->getType())
+ if (checkC23ConstexprInitConversion(S, Init))
+ return true;
+
+ if (auto *SE = dyn_cast<StringLiteral>(Init))
+ if (checkC23ConstexprInitStringLiteral(SE, S, Init->getBeginLoc()))
+ return true;
+
+ for (const Stmt *SubStmt : Init->children()) {
+ const Expr *ChildExpr = dyn_cast_or_null<const Expr>(SubStmt);
+ if (!ChildExpr)
+ continue;
+
+ if (checkC23ConstexprInitializer(S, ChildExpr)) {
+ return true;
+ }
+ }
+ return false;
+}
+
void Sema::CheckCompleteVariableDeclaration(VarDecl *var) {
if (var->isInvalidDecl()) return;
@@ -14397,9 +14559,13 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl *var) {
QualType baseType = Context.getBaseElementType(type);
bool HasConstInit = true;
+ if (getLangOpts().C23 && var->isConstexpr() && !Init)
+ Diag(var->getLocation(), diag::err_constexpr_var_requires_const_init)
+ << var;
+
// Check whether the initializer is sufficiently constant.
- if (getLangOpts().CPlusPlus && !type->isDependentType() && Init &&
- !Init->isValueDependent() &&
+ if ((getLangOpts().CPlusPlus || getLangOpts().C23) &&
+ !type->isDependentType() && Init && !Init->isValueDependent() &&
(GlobalStorage || var->isConstexpr() ||
var->mightBeUsableInConstantExpressions(Context))) {
// If this variable might have a constant initializer or might be usable in
@@ -14407,7 +14573,7 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl *var) {
// do this lazily, because the result might depend on things that change
// later, such as which constexpr functions happen to be defined.
SmallVector<PartialDiagnosticAt, 8> Notes;
- if (!getLangOpts().CPlusPlus11) {
+ if (!getLangOpts().CPlusPlus11 && !getLangOpts().C23) {
// Prior to C++11, in contexts where a constant initializer is required,
// the set of valid constant initializers is described by syntactic rules
// in [expr.const]p2-6.
@@ -14432,6 +14598,8 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl *var) {
if (HasConstInit) {
// FIXME: Consider replacing the initializer with a ConstantExpr.
+ if (getLangOpts().C23 && var->isConstexpr())
+ checkC23ConstexprInitializer(*this, Init);
} else if (var->isConstexpr()) {
SourceLocation DiagLoc = var->getLocation();
// If the note doesn't add any useful information other than a source
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 64607e28b8b35e6..dfb1dfc00c1bc49 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -328,7 +328,8 @@ static const Expr *IgnoreNarrowingConversion(ASTContext &Ctx,
NarrowingKind StandardConversionSequence::getNarrowingKind(
ASTContext &Ctx, const Expr *Converted, APValue &ConstantValue,
QualType &ConstantType, bool IgnoreFloatToIntegralConversion) const {
- assert(Ctx.getLangOpts().CPlusPlus && "narrowing check outside C++");
+ assert((Ctx.getLangOpts().CPlusPlus || Ctx.getLangOpts().C23) &&
+ "narrowing check outside C++");
// C++11 [dcl.init.list]p7:
// A narrowing conversion is an implicit conversion ...
@@ -410,20 +4...
[truncated]
|
|
cc @to268 , GitHub doesn't let me add you to the reviewers list. |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
That's probably because I have still not commit access, but I'll take a look. |
|
Ping. |
| def err_c23_thread_local_constexpr : Error< | ||
| "thread-local storage is not allowed with constexpr">; | ||
| def err_c23_extern_constexpr : Error< | ||
| "extern specifier is not allowed with constexpr">; |
There was a problem hiding this comment.
I think we want to reuse err_invalid_decl_spec_combination for these diagnostics; and probably issue the diagnostic from DeclSpec::Finish() as well (which is where we diagnose other invalid combinations of declaration specifiers).
There was a problem hiding this comment.
I'm not quite sure reusing err_invalid_decl_spec_combination will look clear enough, here is what I got while trying:
t3.c:2:1: error: cannot combine with previous 'extern' declaration specifier
2 | constexpr extern int V79 = 10;
| ^ ~~~~~~
If that is still better, I can update it.
There was a problem hiding this comment.
I ended up updating this.
clang/lib/AST/Decl.cpp
Outdated
| // In C++, const, non-volatile variables of integral or enumeration types | ||
| // can be used in constant expressions. | ||
| if (getType()->isIntegralOrEnumerationType()) | ||
| if (getType()->isIntegralOrEnumerationType() && !Lang.C23) | ||
| return true; | ||
|
|
||
| // Additionally, in C++11, non-volatile constexpr variables can be used in | ||
| // constant expressions. | ||
| return Lang.CPlusPlus11 && isConstexpr(); | ||
| return (Lang.CPlusPlus11 || Lang.C23) && isConstexpr(); |
There was a problem hiding this comment.
Can we update the comments? Maybe with standardese references too
clang/lib/Sema/SemaDecl.cpp
Outdated
| const StringRef S = SE->getBytes(); | ||
| for (unsigned I = 0, N = SE->getLength(); I != N; ++I) { | ||
| Value = S[I]; | ||
| if (Value != S[I]) { | ||
| SemaRef.Diag(Loc, diag::err_c23_constexpr_init_not_representable) | ||
| << S[I] << CharType; |
There was a problem hiding this comment.
Nice catch. Yes, we should use getCodeUnit here - SE->getLength() already gives you the number of code units, something like
| const StringRef S = SE->getBytes(); | |
| for (unsigned I = 0, N = SE->getLength(); I != N; ++I) { | |
| Value = S[I]; | |
| if (Value != S[I]) { | |
| SemaRef.Diag(Loc, diag::err_c23_constexpr_init_not_representable) | |
| << S[I] << CharType; | |
| for (unsigned I = 0, N = SE->getLength(); I != N; ++I) { | |
| uint32_t C = SE->getCodeUnit(I); | |
| Value = C; | |
| if (Value != C) { | |
| SemaRef.Diag(Loc, diag::err_c23_constexpr_init_not_representable) | |
| << C << CharType; |
There was a problem hiding this comment.
I'm not sure? What I'm trying to do is catch an overflowing conversion which already happens in getCodeUnit(). Perhaps if I have to use getCodeUnit() there should be another check then?
There was a problem hiding this comment.
Oh wait.
How would you get into a situation where a string literal contains non representable data?
StringLiterals ought to be representable per constructions
https://eel.is/c++draft/lex.string#10.2.4
https://eel.is/c++draft/lex.string#10.1.sentence-2
Now that i understand what you are trying to do, I'm not sure that whole function is necessary at all.
Is there an example of scenario that concerns you?
There was a problem hiding this comment.
What I'm trying to implement is 6.7.1 p5 of C (looking at N3096 draft):
An object declared with storage-class specifier constexpr or any of its members, even recursively,
shall not have an atomic type, or a variably modified type, or a type that is volatile or restrict
qualified. An initializer of floating type shall be evaluated with the translation-time floating-point
environment. The declaration shall be a definition and shall have an initializer.139) The value of
any constant expressions or of any character in a string literal of the initializer shall be exactly
representable in the corresponding target type; no change of value shall be applied
There is also an illustrating example in p18:
Similarly, implementation-defined behavior related to the char type of the elements of the string literal "\xFF"
may cause constraint violations at translation time:
constexpr char string[] = { "\xFF", }; // ok
constexpr char8_t u8string[] = { u8"\xFF", }; // ok
constexpr unsigned char ucstring[] = { "\xFF", }; // possible constraint
// violation
In both the string and ucstring initializers, the initializer is a (brace-enclosed) string literal of type char. If the type char is
capable of representing negative values and its width is 8, then the code above is equivalent to:
constexpr char string[] = { -1, 0, }; // ok
constexpr char8_t u8string[] = { 255, 0, }; // ok
constexpr unsigned char ucstring[] = { -1, 0, }; // constraint violation
About
StringLiterals ought to be representable per constructions
https://eel.is/c++draft/lex.string#10.2.4
https://eel.is/c++draft/lex.string#10.1.sentence-2
My understanding that it is not the same for C. At least I fail to find somewhat same in C draft.
There was a problem hiding this comment.
Thanks for pointing out the wording.
I started to write a long reply before realizing I did not initially understand what your code was doing,
so I managed to be wrong twice! Sorry for being a bit slow.
Now that I think I get it, I am wondering if we should modify getCodeUnit to return an int64 so that we don't lose the sign information in getCodeUnit. WDYT?
The alternative would be to reimplement a function similar to getCodeUnit that would keep the sign.
There was a problem hiding this comment.
I would prefer adding a new function then, since returning an int64 from getCodeUnit breaks several tests related to printing string literals in diagnostic and ast dumps.
There was a problem hiding this comment.
There is also getStrDataAsUInt16 and getStrDataAsUInt32, do we need to reimplement these as well?
There was a problem hiding this comment.
Yes, because wchar_t can be signed.
Here is something that might work and avoid duplicating half of StringLiteral with dubious casts:
Get the unsigned value, and manually do the two complement to sign cast using APInt.
int64_t V = str.getCodeUnit(I);
if(str.isOrdinary() || str.isWide()) {
Width W = str.getCharByteWidth();
APInt apint(str.getCharByteWidth(), (uint64_t)V);
apint.sext(Width + 1).trunc(Width);
V = apint.getExtValue();
}
the sext idea credit of @erichkeane . I haven't tested it though.
There was a problem hiding this comment.
Ok, I tried to apply the idea, seems to be working and it made more examples from the paper work correctly. Thanks!
There is no APValue::getExtValue, there is either getZExtValue or getSExtValue I used the latter and it seems to be enough even without sext and trunc.
- Minor changes - Don't reuse requireLiteralType - Diagnose interaction with extern and thread_local from DeclSpec::Finish() - Add a couple of test cases
clang/include/clang/AST/Expr.h
Outdated
| @@ -1914,6 +1914,17 @@ class StringLiteral final | |||
| llvm_unreachable("Unsupported character width!"); | |||
| } | |||
|
|
|||
| // Get code unit but preserve sign info. | |||
| int64_t getCodeUnitS(size_t I, uint64_t ByteWidth) const { | |||
There was a problem hiding this comment.
| int64_t getCodeUnitS(size_t I, uint64_t ByteWidth) const { | |
| int64_t getCodeUnitS(size_t I, uint64_t BitWidth) const { |
ByteWidth is actually a number of bits, right?
There was a problem hiding this comment.
It is a number of bits in byte, but ok
clang/lib/Sema/SemaDecl.cpp
Outdated
| int64_t C = SE->getCodeUnitS(I, SemaRef.Context.getCharWidth()); | ||
| Value = C; | ||
| if (Value != C) { | ||
| SemaRef.Diag(Loc, diag::err_c23_constexpr_init_not_representable) |
There was a problem hiding this comment.
Maybe we can use getLocationOfByte instead of Loc here, if it's easy (I'm not insisting)
There was a problem hiding this comment.
Yeah why not. Looks better this way
|
I have found no major issues with the C23 Concerning N3006 (AKA: Underspecified object declarations), I haven't reviewed yet the compatibility with the |
|
Adding more reviewers since Aaron is on vacation... |
|
Ping. |
1 similar comment
|
Ping. |
|
Ping. |
| constexpr void f0() {} // expected-error {{'constexpr' can only be used in variable declarations}} | ||
| constexpr const int f1() { return 0; } // expected-error {{'constexpr' can only be used in variable declarations}} | ||
|
|
||
| constexpr struct S1 { int f; }; //expected-error {{struct cannot be marked constexpr}} |
There was a problem hiding this comment.
This will become an underspecified object declaration; CC @to268 to make sure he's covering this with his patch. Same is true for lines 8-11 as well.
Caused test fail in test/Sema/atomic-expr.c with the new evaluator.
- Move initializer checking to SemaInit - Use TryImplicitSequence - Fix null pointer initializer check, it was all wrong - When doing type check, check canonical type - Add test cases
There was a problem hiding this comment.
I've done another pass over the changes and in general, I think we're really close to done. I spotted these two issues:
#define NAN __builtin_nan("1")
#define FLT_SNAN __builtin_nansf("1")
constexpr float f2 = (long double)NAN; // 6.7.2p21 EXAMPLE 5: should be accepted
constexpr double d3 = FLT_SNAN; // 6.7.2p21 EXAMPLE 5: should be rejected
but would be okay with handling those in a follow-up if they turned out to be complicated. Please let me know your thoughts.
@AaronBallman , implemented in dff61e0 . |
The implementation mostly reuses C++ code paths where possible, including narrowing check in order to provide diagnostic messages in case initializer for constexpr variable is not exactly representable in target type.
The following won't work due to lack of support for other features:
Also due to lack of support for char8_t some of examples with utf-8 strings don't work properly.
Fixes #64742