Skip to content

Commit c7ce6d9

Browse files
dtarditiAndrew Ruef
authored and
Andrew Ruef
committed
Add checking of redeclarations of variables with bounds. (#96)
This change adds checking of redeclarations of variables with bounds, ensuring that the redeclarations follow the Checked C rules. This completes the work for issue #30. For now, when bounds are required to match, we require them to be identical syntactically. This will be generalized later. The checking is done when bounds are attached to variable declarations, not during the merging of declarations. Bounds are attached to variable declarations after declarators have been processed (the bounds may refer to the variable being declared, so the variable declaration needs to be built before we build the bounds expression). This means bounds can't be checked during merging of declarations, which operates on just the declarators. - ActOnBoundsDecl does the checking for conflicting bounds expressions on variable declarations. - Generalize the existing code for checking for conflicting bounds expressions on declaration to handle non-parameter variables. Also generalize the check for variables with unchecked types to include unchecked arrays. Declarations of variables with unchecked pointer and unchecked array types and bounds-safe interfaces are compatible with the declarations that omit the bounds-safe interfaces. - Rework the existing error messages for variable redeclarations to use the clang select mechanism for error messages. This lets us use one diagnostic id in place of several diagnostic ids and simplify the code. I discovered an error in the parsing of bounds expressions for function declarators. I believe it was possible to write a bounds expression after a function declarator, which would have triggered an internal compiler assert: `array_ptr<int> f(int len) : count(len) : count(5)`, for example. The fix is to not try to parse a bounds expression after a function declarator. The parsing of the function declarator already handled the bounds expression. Testing: - Added tests for redeclarations of variables to the Checked C regression tests. This will be committed separately to the Checked C repo. - Passes existing Checked C tests. - Passes clang regression test suite.
1 parent a37f835 commit c7ce6d9

File tree

5 files changed

+193
-118
lines changed

5 files changed

+193
-118
lines changed

include/clang/AST/Type.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1713,6 +1713,7 @@ class Type : public ExtQualsTypeCommonBase {
17131713
bool isDependentSizedArrayType() const;
17141714
/// \brief whether this is a Checked C checked array type.
17151715
bool isCheckedArrayType() const;
1716+
bool isUncheckedArrayType() const;
17161717
bool isRecordType() const;
17171718
bool isClassType() const;
17181719
bool isStructureType() const;
@@ -5670,6 +5671,12 @@ inline bool Type::isCheckedArrayType() const {
56705671
else
56715672
return false;
56725673
}
5674+
inline bool Type::isUncheckedArrayType() const {
5675+
if (const ArrayType *T = dyn_cast<ArrayType>(CanonicalType))
5676+
return !T->isChecked();
5677+
else
5678+
return false;
5679+
}
56735680
inline bool Type::isBuiltinType() const {
56745681
return isa<BuiltinType>(CanonicalType);
56755682
}

include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8843,23 +8843,19 @@ def err_bounds_type_annotation_lost_checking : Error<
88438843

88448844
def note_previous_bounds_decl : Note<"previous bounds declaration is here">;
88458845

8846-
def err_conflicting_parameter_bounds : Error<
8847-
"function redeclaration has conflicting parameter bounds">;
8848-
8849-
def err_conflicting_return_bounds : Error<
8850-
"function redeclaration has conflicting return bounds">;
8851-
8852-
def err_added_bounds_for_return : Error<
8853-
"function redeclaration added return bounds">;
8854-
8855-
def err_missing_bounds_for_return : Error <
8856-
"function redeclaration dropped return bounds">;
8857-
8858-
def err_added_bounds_for_parameter : Error<
8859-
"function redeclaration added bounds for parameter">;
8860-
8861-
def err_missing_bounds_for_parameter : Error<
8862-
"function redeclaration dropped bounds for parameter">;
8846+
def err_decl_conflicting_bounds : Error<
8847+
"%select{function redeclaration has conflicting parameter bounds|function "
8848+
"redeclaration has conflicting return bounds|variable redeclaration has "
8849+
"conflicting bounds}0">;
8850+
8851+
def err_decl_added_bounds : Error<
8852+
"%select{function redeclaration added bounds for parameter|function "
8853+
"redeclaration added return bounds|variable redeclaration added bounds}0">;
8854+
8855+
def err_decl_dropped_bounds : Error<
8856+
"%select{function redeclaration dropped bounds for parameter|function "
8857+
"redeclaration dropped return bounds|variable redeclaration dropped "
8858+
"bounds}0">;
88638859

88648860
def err_conflicting_bounds : Error<"conflicting bounds for %0">;
88658861

include/clang/Sema/Sema.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2267,6 +2267,14 @@ class Sema {
22672267
CCT_Union
22682268
};
22692269

2270+
// used for %select in diagnostics for errors involving redeclarations
2271+
// with bounds
2272+
enum class CheckedCBoundsError {
2273+
CCBE_Parameter,
2274+
CCBE_Return,
2275+
CCBE_Variable
2276+
};
2277+
22702278
CheckedTypeClassification classifyForCheckedTypeDiagnostic(QualType qt);
22712279

22722280
// AssignmentAction - This is used by all the assignment diagnostic functions

lib/Parse/ParseDecl.cpp

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2086,22 +2086,24 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
20862086

20872087
bool TypeContainsAuto = D.getDeclSpec().containsPlaceholderType();
20882088

2089-
// Parse the optional Checked C bounds expression or interop type
2090-
// annotation.
2091-
if (getLangOpts().CheckedC && Tok.is(tok::colon)) {
2092-
ConsumeToken();
2093-
ExprResult Bounds = ParseBoundsExpressionOrInteropType(D);
2094-
if (Bounds.isInvalid())
2095-
SkipUntil(tok::comma, tok::equal, StopAtSemi | StopBeforeMatch);
2096-
2089+
// If this is a variable declarator in Checked C, parse the bounds expression
2090+
// (if any) and set the bounds expression. Function declarators are ignored
2091+
// here because return bounds expressions are parsed as part of function
2092+
// declarators already.
2093+
if (getLangOpts().CheckedC && isa<VarDecl>(ThisDecl)) {
20972094
VarDecl *ThisVarDecl = dyn_cast<VarDecl>(ThisDecl);
2098-
if (ThisVarDecl) {
2099-
if (Bounds.isInvalid())
2100-
Actions.ActOnInvalidBoundsDecl(ThisVarDecl);
2101-
else
2102-
Actions.ActOnBoundsDecl(ThisVarDecl, cast<BoundsExpr>(Bounds.get()));
2103-
} else
2104-
llvm_unreachable("Unexpected decl type");
2095+
BoundsExpr *Bounds = nullptr;
2096+
// The optional Checked C bounds expression or interop type annotation.
2097+
if (Tok.is(tok::colon)) {
2098+
ConsumeToken();
2099+
ExprResult ParsedBounds = ParseBoundsExpressionOrInteropType(D);
2100+
if (ParsedBounds.isInvalid()) {
2101+
SkipUntil(tok::comma, tok::equal, StopAtSemi | StopBeforeMatch);
2102+
Bounds = Actions.CreateInvalidBoundsExpr();
2103+
} else
2104+
Bounds = cast<BoundsExpr>(ParsedBounds.get());
2105+
}
2106+
Actions.ActOnBoundsDecl(ThisVarDecl, Bounds);
21052107
}
21062108

21072109
// Parse declarator '=' initializer.

0 commit comments

Comments
 (0)