Skip to content

Commit

Permalink
Explain why the array bound is non-constant in VLA diagnostics.
Browse files Browse the repository at this point in the history 
In passing, also use a more precise diagnostic to explain why an
expression is not an ICE if it's not of integral type.
  • Loading branch information
@zygoloid
zygoloid committed Aug 19, 2020
1 parent 09ca3f4 commit 6f33936
Show file tree
Hide file tree
Showing 121 changed files with 653 additions and 597 deletions.
2 changes: 1 addition & 1 deletion clang/include/clang/Basic/DiagnosticGroups.td
View file
Original file line number Diff line number Diff line change
Expand Up @@ -733,8 +733,8 @@ def OverridingMethodMismatch : DiagGroup<"overriding-method-mismatch">;
def VariadicMacros : DiagGroup<"variadic-macros">;
def VectorConversion : DiagGroup<"vector-conversion">; // clang specific
def VexingParse : DiagGroup<"vexing-parse">;
def VLA : DiagGroup<"vla">;
def VLAExtension : DiagGroup<"vla-extension">;
def VLA : DiagGroup<"vla", [VLAExtension]>;
def VolatileRegisterVar : DiagGroup<"volatile-register-var">;
def Visibility : DiagGroup<"visibility">;
def ZeroLengthArray : DiagGroup<"zero-length-array">;
Expand Down
4 changes: 2 additions & 2 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ def ext_cce_narrowing : ExtWarn<
"evaluates to %2, which cannot be narrowed to type %3}1">,
InGroup<CXX11Narrowing>, DefaultError, SFINAEFailure;
def err_ice_not_integral : Error<
"integral constant expression must have integral or unscoped enumeration "
"type, not %0">;
"%select{integer|integral}1 constant expression must have "
"%select{integer|integral or unscoped enumeration}1 type, not %0">;
def err_ice_incomplete_type : Error<
"integral constant expression has incomplete class type %0">;
def err_ice_explicit_conversion : Error<
Expand Down
9 changes: 6 additions & 3 deletions clang/include/clang/Sema/Sema.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11588,9 +11588,12 @@ class Sema final {

VerifyICEDiagnoser(bool Suppress = false) : Suppress(Suppress) { }

virtual void diagnoseNotICE(Sema &S, SourceLocation Loc, SourceRange SR) =0;
virtual void diagnoseFold(Sema &S, SourceLocation Loc, SourceRange SR);
virtual ~VerifyICEDiagnoser() { }
virtual SemaDiagnosticBuilder
diagnoseNotICEType(Sema &S, SourceLocation Loc, QualType T);
virtual SemaDiagnosticBuilder diagnoseNotICE(Sema &S,
SourceLocation Loc) = 0;
virtual SemaDiagnosticBuilder diagnoseFold(Sema &S, SourceLocation Loc);
virtual ~VerifyICEDiagnoser() {}
};

/// VerifyIntegerConstantExpression - Verifies that an expression is an ICE,
Expand Down
5 changes: 3 additions & 2 deletions clang/lib/Sema/SemaDeclCXX.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1066,8 +1066,9 @@ static IsTupleLike isTupleLike(Sema &S, SourceLocation Loc, QualType T,
TemplateArgumentListInfo &Args;
ICEDiagnoser(LookupResult &R, TemplateArgumentListInfo &Args)
: R(R), Args(Args) {}
void diagnoseNotICE(Sema &S, SourceLocation Loc, SourceRange SR) override {
S.Diag(Loc, diag::err_decomp_decl_std_tuple_size_not_constant)
Sema::SemaDiagnosticBuilder diagnoseNotICE(Sema &S,
SourceLocation Loc) override {
return S.Diag(Loc, diag::err_decomp_decl_std_tuple_size_not_constant)
<< printTemplateArgs(S.Context.getPrintingPolicy(), Args);
}
} Diagnoser(R, Args);
Expand Down
44 changes: 29 additions & 15 deletions clang/lib/Sema/SemaExpr.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15821,8 +15821,13 @@ ExprResult Sema::VerifyIntegerConstantExpression(Expr *E,
llvm::APSInt *Result) {
class SimpleICEDiagnoser : public VerifyICEDiagnoser {
public:
void diagnoseNotICE(Sema &S, SourceLocation Loc, SourceRange SR) override {
S.Diag(Loc, diag::err_expr_not_ice) << S.LangOpts.CPlusPlus << SR;
SemaDiagnosticBuilder diagnoseNotICEType(Sema &S, SourceLocation Loc,
QualType T) override {
return S.Diag(Loc, diag::err_ice_not_integral)
<< T << S.LangOpts.CPlusPlus;
}
SemaDiagnosticBuilder diagnoseNotICE(Sema &S, SourceLocation Loc) override {
return S.Diag(Loc, diag::err_expr_not_ice) << S.LangOpts.CPlusPlus;
}
} Diagnoser;

Expand All @@ -15840,17 +15845,23 @@ ExprResult Sema::VerifyIntegerConstantExpression(Expr *E,
IDDiagnoser(unsigned DiagID)
: VerifyICEDiagnoser(DiagID == 0), DiagID(DiagID) { }

void diagnoseNotICE(Sema &S, SourceLocation Loc, SourceRange SR) override {
S.Diag(Loc, DiagID) << SR;
SemaDiagnosticBuilder diagnoseNotICE(Sema &S, SourceLocation Loc) override {
return S.Diag(Loc, DiagID);
}
} Diagnoser(DiagID);

return VerifyIntegerConstantExpression(E, Result, Diagnoser, AllowFold);
}

void Sema::VerifyICEDiagnoser::diagnoseFold(Sema &S, SourceLocation Loc,
SourceRange SR) {
S.Diag(Loc, diag::ext_expr_not_ice) << SR << S.LangOpts.CPlusPlus;
Sema::SemaDiagnosticBuilder
Sema::VerifyICEDiagnoser::diagnoseNotICEType(Sema &S, SourceLocation Loc,
QualType T) {
return diagnoseNotICE(S, Loc);
}

Sema::SemaDiagnosticBuilder
Sema::VerifyICEDiagnoser::diagnoseFold(Sema &S, SourceLocation Loc) {
return S.Diag(Loc, diag::ext_expr_not_ice) << S.LangOpts.CPlusPlus;
}

ExprResult
Expand All @@ -15867,14 +15878,16 @@ Sema::VerifyIntegerConstantExpression(Expr *E, llvm::APSInt *Result,
// unscoped enumeration type
ExprResult Converted;
class CXX11ConvertDiagnoser : public ICEConvertDiagnoser {
VerifyICEDiagnoser &BaseDiagnoser;
public:
CXX11ConvertDiagnoser(bool Silent)
: ICEConvertDiagnoser(/*AllowScopedEnumerations*/false,
Silent, true) {}
CXX11ConvertDiagnoser(VerifyICEDiagnoser &BaseDiagnoser)
: ICEConvertDiagnoser(/*AllowScopedEnumerations*/ false,
BaseDiagnoser.Suppress, true),
BaseDiagnoser(BaseDiagnoser) {}

SemaDiagnosticBuilder diagnoseNotInt(Sema &S, SourceLocation Loc,
QualType T) override {
return S.Diag(Loc, diag::err_ice_not_integral) << T;
return BaseDiagnoser.diagnoseNotICEType(S, Loc, T);
}

SemaDiagnosticBuilder diagnoseIncomplete(
Expand Down Expand Up @@ -15908,7 +15921,7 @@ Sema::VerifyIntegerConstantExpression(Expr *E, llvm::APSInt *Result,
Sema &S, SourceLocation Loc, QualType T, QualType ConvTy) override {
llvm_unreachable("conversion functions are permitted");
}
} ConvertDiagnoser(Diagnoser.Suppress);
} ConvertDiagnoser(Diagnoser);

Converted = PerformContextualImplicitConversion(DiagLoc, E,
ConvertDiagnoser);
Expand All @@ -15920,7 +15933,8 @@ Sema::VerifyIntegerConstantExpression(Expr *E, llvm::APSInt *Result,
} else if (!E->getType()->isIntegralOrUnscopedEnumerationType()) {
// An ICE must be of integral or unscoped enumeration type.
if (!Diagnoser.Suppress)
Diagnoser.diagnoseNotICE(*this, DiagLoc, E->getSourceRange());
Diagnoser.diagnoseNotICEType(*this, DiagLoc, E->getType())
<< E->getSourceRange();
return ExprError();
}

Expand Down Expand Up @@ -15973,15 +15987,15 @@ Sema::VerifyIntegerConstantExpression(Expr *E, llvm::APSInt *Result,

if (!Folded || !AllowFold) {
if (!Diagnoser.Suppress) {
Diagnoser.diagnoseNotICE(*this, DiagLoc, E->getSourceRange());
Diagnoser.diagnoseNotICE(*this, DiagLoc) << E->getSourceRange();
for (const PartialDiagnosticAt &Note : Notes)
Diag(Note.first, Note.second);
}

return ExprError();
}

Diagnoser.diagnoseFold(*this, DiagLoc, E->getSourceRange());
Diagnoser.diagnoseFold(*this, DiagLoc) << E->getSourceRange();
for (const PartialDiagnosticAt &Note : Notes)
Diag(Note.first, Note.second);

Expand Down
6 changes: 3 additions & 3 deletions clang/lib/Sema/SemaTemplate.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6983,9 +6983,9 @@ ExprResult Sema::CheckTemplateArgument(NonTypeTemplateParmDecl *Param,
public:
TmplArgICEDiagnoser(QualType T) : T(T) { }

void diagnoseNotICE(Sema &S, SourceLocation Loc,
SourceRange SR) override {
S.Diag(Loc, diag::err_template_arg_not_ice) << T << SR;
SemaDiagnosticBuilder diagnoseNotICE(Sema &S,
SourceLocation Loc) override {
return S.Diag(Loc, diag::err_template_arg_not_ice) << T;
}
} Diagnoser(ArgType);

Expand Down
189 changes: 112 additions & 77 deletions clang/lib/Sema/SemaType.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2221,26 +2221,48 @@ QualType Sema::BuildExtIntType(bool IsUnsigned, Expr *BitWidth,
return Context.getExtIntType(IsUnsigned, NumBits);
}

/// Check whether the specified array size makes the array type a VLA. If so,
/// return true, if not, return the size of the array in SizeVal.
static bool isArraySizeVLA(Sema &S, Expr *ArraySize, llvm::APSInt &SizeVal) {
/// Check whether the specified array bound can be evaluated using the relevant
/// language rules. If so, returns the possibly-converted expression and sets
/// SizeVal to the size. If not, but the expression might be a VLA bound,
/// returns ExprResult(). Otherwise, produces a diagnostic and returns
/// ExprError().
static ExprResult checkArraySize(Sema &S, Expr *&ArraySize,
llvm::APSInt &SizeVal, unsigned VLADiag,
bool VLAIsError) {
// If the size is an ICE, it certainly isn't a VLA. If we're in a GNU mode
// (like gnu99, but not c99) accept any evaluatable value as an extension.
class VLADiagnoser : public Sema::VerifyICEDiagnoser {
public:
VLADiagnoser() : Sema::VerifyICEDiagnoser(true) {}
unsigned VLADiag;
bool VLAIsError;
bool IsVLA = false;

void diagnoseNotICE(Sema &S, SourceLocation Loc, SourceRange SR) override {
VLADiagnoser(unsigned VLADiag, bool VLAIsError)
: VLADiag(VLADiag), VLAIsError(VLAIsError) {}

Sema::SemaDiagnosticBuilder diagnoseNotICEType(Sema &S, SourceLocation Loc,
QualType T) override {
return S.Diag(Loc, diag::err_array_size_non_int) << T;
}

Sema::SemaDiagnosticBuilder diagnoseNotICE(Sema &S,
SourceLocation Loc) override {
IsVLA = !VLAIsError;
return S.Diag(Loc, VLADiag);
}

void diagnoseFold(Sema &S, SourceLocation Loc, SourceRange SR) override {
S.Diag(Loc, diag::ext_vla_folded_to_constant) << SR;
Sema::SemaDiagnosticBuilder diagnoseFold(Sema &S,
SourceLocation Loc) override {
return S.Diag(Loc, diag::ext_vla_folded_to_constant);
}
} Diagnoser;
} Diagnoser(VLADiag, VLAIsError);

return S.VerifyIntegerConstantExpression(ArraySize, &SizeVal, Diagnoser,
S.LangOpts.GNUMode ||
S.LangOpts.OpenCL).isInvalid();
ExprResult R = S.VerifyIntegerConstantExpression(
ArraySize, &SizeVal, Diagnoser,
(S.LangOpts.GNUMode || S.LangOpts.OpenCL));
if (Diagnoser.IsVLA)
return ExprResult();
return R;
}

/// Build an array type.
Expand Down Expand Up @@ -2352,68 +2374,95 @@ QualType Sema::BuildArrayType(QualType T, ArrayType::ArraySizeModifier ASM,
return QualType();
}

// VLAs always produce at least a -Wvla diagnostic, sometimes an error.
unsigned VLADiag;
bool VLAIsError;
if (getLangOpts().OpenCL) {
// OpenCL v1.2 s6.9.d: variable length arrays are not supported.
VLADiag = diag::err_opencl_vla;
VLAIsError = true;
} else if (getLangOpts().C99) {
VLADiag = diag::warn_vla_used;
VLAIsError = false;
} else if (isSFINAEContext()) {
VLADiag = diag::err_vla_in_sfinae;
VLAIsError = true;
} else {
VLADiag = diag::ext_vla;
VLAIsError = false;
}

llvm::APSInt ConstVal(Context.getTypeSize(Context.getSizeType()));
if (!ArraySize) {
if (ASM == ArrayType::Star)
if (ASM == ArrayType::Star) {
Diag(Loc, VLADiag);
if (VLAIsError)
return QualType();

T = Context.getVariableArrayType(T, nullptr, ASM, Quals, Brackets);
else
} else {
T = Context.getIncompleteArrayType(T, ASM, Quals);
}
} else if (ArraySize->isTypeDependent() || ArraySize->isValueDependent()) {
T = Context.getDependentSizedArrayType(T, ArraySize, ASM, Quals, Brackets);
} else if ((!T->isDependentType() && !T->isIncompleteType() &&
!T->isConstantSizeType()) ||
isArraySizeVLA(*this, ArraySize, ConstVal)) {
// Even in C++11, don't allow contextual conversions in the array bound
// of a VLA.
if (getLangOpts().CPlusPlus11 &&
!ArraySize->getType()->isIntegralOrUnscopedEnumerationType()) {
Diag(ArraySize->getBeginLoc(), diag::err_array_size_non_int)
<< ArraySize->getType() << ArraySize->getSourceRange();
} else {
ExprResult R =
checkArraySize(*this, ArraySize, ConstVal, VLADiag, VLAIsError);
if (R.isInvalid())
return QualType();
}

// C99: an array with an element type that has a non-constant-size is a VLA.
// C99: an array with a non-ICE size is a VLA. We accept any expression
// that we can fold to a non-zero positive value as an extension.
T = Context.getVariableArrayType(T, ArraySize, ASM, Quals, Brackets);
} else {
// C99 6.7.5.2p1: If the expression is a constant expression, it shall
// have a value greater than zero.
if (ConstVal.isSigned() && ConstVal.isNegative()) {
if (Entity)
Diag(ArraySize->getBeginLoc(), diag::err_decl_negative_array_size)
<< getPrintableNameForEntity(Entity) << ArraySize->getSourceRange();
else
Diag(ArraySize->getBeginLoc(), diag::err_typecheck_negative_array_size)
if (!R.isUsable()) {
// C99: an array with a non-ICE size is a VLA. We accept any expression
// that we can fold to a non-zero positive value as a non-VLA as an
// extension.
T = Context.getVariableArrayType(T, ArraySize, ASM, Quals, Brackets);
} else if (!T->isDependentType() && !T->isIncompleteType() &&
!T->isConstantSizeType()) {
// C99: an array with an element type that has a non-constant-size is a
// VLA.
// FIXME: Add a note to explain why this isn't a VLA.
Diag(Loc, VLADiag);
if (VLAIsError)
return QualType();
T = Context.getVariableArrayType(T, ArraySize, ASM, Quals, Brackets);
} else {
// C99 6.7.5.2p1: If the expression is a constant expression, it shall
// have a value greater than zero.
// In C++, this follows from narrowing conversions being disallowed.
if (ConstVal.isSigned() && ConstVal.isNegative()) {
if (Entity)
Diag(ArraySize->getBeginLoc(), diag::err_decl_negative_array_size)
<< getPrintableNameForEntity(Entity)
<< ArraySize->getSourceRange();
else
Diag(ArraySize->getBeginLoc(),
diag::err_typecheck_negative_array_size)
<< ArraySize->getSourceRange();
return QualType();
}
if (ConstVal == 0) {
// GCC accepts zero sized static arrays. We allow them when
// we're not in a SFINAE context.
Diag(ArraySize->getBeginLoc(),
isSFINAEContext() ? diag::err_typecheck_zero_array_size
: diag::ext_typecheck_zero_array_size)
<< ArraySize->getSourceRange();
return QualType();
}
if (ConstVal == 0) {
// GCC accepts zero sized static arrays. We allow them when
// we're not in a SFINAE context.
Diag(ArraySize->getBeginLoc(), isSFINAEContext()
? diag::err_typecheck_zero_array_size
: diag::ext_typecheck_zero_array_size)
<< ArraySize->getSourceRange();
} else if (!T->isDependentType() && !T->isVariablyModifiedType() &&
!T->isIncompleteType() && !T->isUndeducedType()) {
}

// Is the array too large?
unsigned ActiveSizeBits
= ConstantArrayType::getNumAddressingBits(Context, T, ConstVal);
unsigned ActiveSizeBits =
(!T->isDependentType() && !T->isVariablyModifiedType() &&
!T->isIncompleteType() && !T->isUndeducedType())
? ConstantArrayType::getNumAddressingBits(Context, T, ConstVal)
: ConstVal.getActiveBits();
if (ActiveSizeBits > ConstantArrayType::getMaxSizeBits(Context)) {
Diag(ArraySize->getBeginLoc(), diag::err_array_too_large)
<< ConstVal.toString(10) << ArraySize->getSourceRange();
return QualType();
}
}

T = Context.getConstantArrayType(T, ConstVal, ArraySize, ASM, Quals);
}

// OpenCL v1.2 s6.9.d: variable length arrays are not supported.
if (getLangOpts().OpenCL && T->isVariableArrayType()) {
Diag(Loc, diag::err_opencl_vla);
return QualType();
T = Context.getConstantArrayType(T, ConstVal, ArraySize, ASM, Quals);
}
}

if (T->isVariableArrayType() && !Context.getTargetInfo().isVLASupported()) {
Expand All @@ -2426,26 +2475,12 @@ QualType Sema::BuildArrayType(QualType T, ArrayType::ArraySizeModifier ASM,
: CFT_InvalidTarget);
}

// If this is not C99, extwarn about VLA's and C99 array size modifiers.
if (!getLangOpts().C99) {
if (T->isVariableArrayType()) {
// Prohibit the use of VLAs during template argument deduction.
if (isSFINAEContext()) {
Diag(Loc, diag::err_vla_in_sfinae);
return QualType();
}
// Just extwarn about VLAs.
else
Diag(Loc, diag::ext_vla);
} else if (ASM != ArrayType::Normal || Quals != 0)
Diag(Loc,
getLangOpts().CPlusPlus? diag::err_c99_array_usage_cxx
: diag::ext_c99_array_usage) << ASM;
}

if (T->isVariableArrayType()) {
// Warn about VLAs for -Wvla.
Diag(Loc, diag::warn_vla_used);
// If this is not C99, diagnose array size modifiers on non-VLAs.
if (!getLangOpts().C99 && !T->isVariableArrayType() &&
(ASM != ArrayType::Normal || Quals != 0)) {
Diag(Loc, getLangOpts().CPlusPlus ? diag::err_c99_array_usage_cxx
: diag::ext_c99_array_usage)
<< ASM;
}

// OpenCL v2.0 s6.12.5 - Arrays of blocks are not supported.
Expand Down
Loading

0 comments on commit 6f33936

Please sign in to comment.