Skip to content

Commit 946d86f

Browse files
authored
Revamp implementation of generic functions. (#581)
This change revises parsing and type substitution for generic functions. Fix a number of bugs in the existing implementation. The implementation of generic bounds-safe interfaces required that all calls involving checked arguments be supplied type arguments. This increased the number of code changes required. In unchecked or _Bounds_only contexts, allow the type arguments to be omitted and supply void as the omitted type arguments. This allows existing code to work "as is" (i.e. keep implicit casts to _Array_ptr<void>). We may revisit this at some point Details: 1. Parsing of generic type applications depended on detecting an occurrence of a generic function to parse a type argument list. Reimplement parsing of generic type applications so that it does not depend on information from semantic checking. Treat generic type applications as postfix expressions, with the grammar production: postfix-expression: postfix-expression < type argument list> where type argument list consists of a comma separated list of zero or more type arguments. In C, this isn't ambiguous because the `<` operator cannot be followed by a type name or '>'. We just need to check the first token after '<' to see if it is a type name or '>'. Of course, there is an ambiguity in the C grammar that an identifier could either by a type name or variable name. We haven't made this worse, I don't believe this is ambiguous for C++ assuming that recognizing a template application has higher precedence. A handful of clang tests that check compiler diagnostics for C++ templates broke, so I've enabled this production only for Checked C for now. The parsing change partly addresses Github issue #559 (address of operator on function with generic type parameters). We now allow the address-of operator to be applied to generic functions with type arguments. 2. Refactor the code for applying a generic function type to type arguments. The implementation of type substitution was buggy and didn't substitute type arguments into bounds expressions. Move the substitution code from Parse into Sema so that we have access to TreeTransform. Also move the semantic checking for type applications to Sema. Use TreeTransform-based transformation to do type substitution for type applications so that it is correct for bounds expression (so that bounds expressions are rewritten). When checking call expressions, we expand count/byte_count bounds expressions to standard forms after doing substitution. In the case where the interface type for a parameter is used to construct the parameter bounds expression, we need to cast the argument to the interface type before expansion. Otherwise we end up with arithmetic on void pointers when generic bounds-safe interfaces are used. 3. Improve diagnostics: Add diagnostics for the different ways redeclarations of functions can be incompatible due generic quantifiers. - The quantifiers can be mismatched. - The number of type variables can be mismatched. - One type can be declared as generic and the other type as non-generic. Add a new diagnostic message for the case where a non-generic function is applied to type arguments. 4. Fix bugs in the existing implementation: 1. The calculation of nesting depths of type variables (the number of generic quantifiers enclosing a type variable) was wrong. All type variables were being assigned a depth of 0, which would cause substitution in types involving higher-order polymorphic functions to go wrong. 2. Fix type compatibility for generic, itype generic, and non-generic function types. - Generic types are compatible with generic types. - Non-generic types are compatible with non-generic types. - Itype generic types are compatible with other itype generic types or non-generic types. - All other combinations of generic, itype generics, and non-generic functions are incompatible. Also, merging of function types wasn't working properly for itype generic functions. 3. Dumping of itype generic functions differed from generic functions. 4. Scope handling of scopes created for generic and itype generic functions was often wrong. In particular, we weren't always exiting the scope in some cases. I fixed all the spots I could find, but the handling of this remains brittle and bug prone. The issue is that we treat the quantifier as being part of the decl spec, so we introduce the new scope during parsing of decl specifiers. That means we have to be sure to exit the scope when we are done with the declaration. 5. When adding function declarations to scopes, we didn't properly go to the enclosing scope when the function was an `itype forall`. 6. The Profile function for type variables wasn't including whether the type variable is for a bounds-safe interface. This caused `_Itype_for_any` type variables and `_For_any type` variables to become confused with each other, which led to test failures. Testing: - Add more tests of AST construction for generic functions, including new tests for higher-order generic functions (generic functions that take other generic functions as arguments). - Add tests of AST construction for type applications of generic functions, where type arguments are substited for type parameters. - In the Checked C repo, add tests of new diagnostics for types that are incompatible due properties of generic types. - Update existing tests of AST dumping of type variables. - Passed automated testing for update Checked C, Checked C clang, and and clang tests on Linux. Passed testing on Windows x64 locally.
1 parent 970cba4 commit 946d86f

22 files changed

+737
-373
lines changed

include/clang/AST/Type.h

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1762,6 +1762,8 @@ class Type : public ExtQualsTypeCommonBase {
17621762
bool isFunctionType() const;
17631763
bool isFunctionNoProtoType() const { return getAs<FunctionNoProtoType>(); }
17641764
bool isFunctionProtoType() const { return getAs<FunctionProtoType>(); }
1765+
bool isGenericFunctionType() const;
1766+
bool isItypeGenericFunctionType() const;
17651767
bool isPointerType() const;
17661768
bool isCheckedPointerType() const;
17671769
bool isUncheckedPointerType() const;
@@ -3562,6 +3564,9 @@ class FunctionProtoType : public FunctionType, public llvm::FoldingSetNode {
35623564
unsigned getNumTypeVars() const { return NumTypeVars; }
35633565
bool isGenericFunction() const { return GenericFunction; }
35643566
bool isItypeGenericFunction() const { return ItypeGenericFunction; }
3567+
bool isNonGenericFunction() const {
3568+
return !(GenericFunction || ItypeGenericFunction);
3569+
}
35653570

35663571
ArrayRef<QualType> getParamTypes() const {
35673572
return llvm::makeArrayRef(param_type_begin(), param_type_end());
@@ -3847,7 +3852,7 @@ class TypeVariableType : public Type, public llvm::FoldingSetNode {
38473852
// prototype scope depth, this keeps track of the depth of forany scope.
38483853
unsigned int depth;
38493854
unsigned int index;
3850-
bool isBoundsInterfaceType;
3855+
bool isBoundsInterfaceType; // TODO: pack this into a bitfield.
38513856
protected:
38523857
TypeVariableType(unsigned int inDepth, unsigned int inIndex, bool inBoundsInterface)
38533858
: Type(TypeVariable, QualType(), false, false, false, false),
@@ -3868,11 +3873,13 @@ class TypeVariableType : public Type, public llvm::FoldingSetNode {
38683873
}
38693874

38703875
void Profile(llvm::FoldingSetNodeID &ID) {
3871-
Profile(ID, depth, index);
3876+
Profile(ID, depth, index, isBoundsInterfaceType);
38723877
}
3873-
static void Profile(llvm::FoldingSetNodeID &ID, unsigned int inDepth, unsigned int inIndex) {
3878+
static void Profile(llvm::FoldingSetNodeID &ID, unsigned int inDepth, unsigned int inIndex,
3879+
bool isBoundsInterfaceType) {
38743880
ID.AddInteger(inDepth);
38753881
ID.AddInteger(inIndex);
3882+
ID.AddBoolean(isBoundsInterfaceType);
38763883
}
38773884

38783885
static bool classof(const Type *T) { return T->getTypeClass() == TypeVariable; }
@@ -6011,6 +6018,16 @@ inline bool Type::isCompoundType() const {
60116018
inline bool Type::isFunctionType() const {
60126019
return isa<FunctionType>(CanonicalType);
60136020
}
6021+
inline bool Type::isGenericFunctionType() const {
6022+
if (const FunctionProtoType *FPT = getAs<FunctionProtoType>())
6023+
return FPT->isGenericFunction();
6024+
return false;
6025+
}
6026+
inline bool Type::isItypeGenericFunctionType() const {
6027+
if (const FunctionProtoType *FPT = getAs<FunctionProtoType>())
6028+
return FPT->isItypeGenericFunction();
6029+
return false;
6030+
}
60146031
inline bool Type::isPointerType() const {
60156032
return isa<PointerType>(CanonicalType);
60166033
}

include/clang/Basic/DiagnosticParseKinds.td

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1210,12 +1210,6 @@ def err_expected_list_of_types_expr_for_generic_function : Error<
12101210
def err_type_function_comma_or_greater_expected : Error<
12111211
"expected , or >">;
12121212

1213-
def err_type_list_and_type_variable_num_mismatch : Error<
1214-
"mismatch between the number of types listed and number of type variables">;
1215-
1216-
def note_type_variables_declared_at : Note<
1217-
"type variable(s) declared here">;
1218-
12191213
// Checked C pragmas
12201214
def err_pragma_checked_scope_invalid_argument : Error<
12211215
"unexpected argument '%0' to '#pragma CHECKED_SCOPE'; "

include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9806,6 +9806,30 @@ def err_bounds_type_annotation_lost_checking : Error<
98069806
def no_prototype_generic_function : Error<
98079807
"expected prototype for a generic function">;
98089808

9809+
def err_decl_conflicting_generic_non_generic_functions : Error<
9810+
"conflicting non-generic and generic declarations of %0" >;
9811+
9812+
def err_decl_conflicting_type_variable_count : Error<
9813+
"conflicting numbers of type variables for declarations of %0" >;
9814+
9815+
def err_expected_type_argument_list_for_generic_call : Error<
9816+
"expected a type argument list for a generic function call">;
9817+
9818+
def err_expected_type_argument_list_for_itype_generic_call : Error<
9819+
"expected a type argument list for a bounds-safe interface call in a checked scope">;
9820+
9821+
def err_type_list_and_type_variable_num_mismatch : Error<
9822+
"mismatch between the number of types listed and number of type variables">;
9823+
9824+
def note_type_variables_declared_at : Note<
9825+
"type variable(s) declared here">;
9826+
9827+
def err_type_args_for_non_generic_expression : Error<
9828+
"type arguments supplied for non-generic function or expression">;
9829+
9830+
def err_type_args_limited : Error<
9831+
"type arguments only supported for variables and function names right now">;
9832+
98099833
} // end of Checked C Category
98109834

98119835
} // end of sema component.

include/clang/Parse/Parser.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -996,6 +996,10 @@ class Parser : public CodeCompletionHandler {
996996
/// point for skipping past a simple-declaration.
997997
void SkipMalformedDecl();
998998

999+
// If the current scope is a Checked C _Forany or _Itypeforany scope, exit it.
1000+
// TODO: this probably doesn't belong in the parser.
1001+
void ExitQuantifiedTypeScope(DeclSpec &DS);
1002+
9991003
private:
10001004
//===--------------------------------------------------------------------===//
10011005
// Lexing and parsing of C++ inline methods.
@@ -1749,7 +1753,7 @@ class Parser : public CodeCompletionHandler {
17491753
ExprResult ParseBoundsCastExpression();
17501754

17511755
ExprResult ParseBoundsExpression();
1752-
bool ParseItypeAndGenericFunctionExpression(ExprResult &Res);
1756+
ExprResult ParseGenericTypeArgumentList(ExprResult TypeFunc, SourceLocation Loc);
17531757

17541758
QualType SubstituteTypeVariable(QualType QT,
17551759
SmallVector<DeclRefExpr::GenericInstInfo::TypeArgument, 4> &typeNames);
@@ -1776,7 +1780,6 @@ class Parser : public CodeCompletionHandler {
17761780

17771781
ExprResult ParseReturnValueExpression();
17781782

1779-
17801783
//===--------------------------------------------------------------------===//
17811784
// clang Expressions
17821785

include/clang/Sema/Sema.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4809,6 +4809,12 @@ class Sema {
48094809

48104810
BoundsExpr *CheckNonModifyingBounds(BoundsExpr *Bounds, Expr *E);
48114811

4812+
ExprResult ActOnTypeApplication(ExprResult TypeFunc, SourceLocation Loc,
4813+
ArrayRef<DeclRefExpr::GenericInstInfo::TypeArgument> Args);
4814+
4815+
QualType SubstituteTypeArgs(QualType QT,
4816+
ArrayRef<DeclRefExpr::GenericInstInfo::TypeArgument> TypeArgs);
4817+
48124818
bool AbstractForFunctionType(BoundsAnnotations &BA,
48134819
ArrayRef<DeclaratorChunk::ParamInfo> Params);
48144820
/// \brief Take a bounds expression with positional parameters from a function

lib/AST/ASTContext.cpp

Lines changed: 35 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2519,7 +2519,7 @@ QualType ASTContext::getPointerType(QualType T, CheckedPointerKind kind) const {
25192519
QualType ASTContext::getTypeVariableType(unsigned int inDepth, unsigned int inIndex,
25202520
const bool isInBoundsSafeInterface) const {
25212521
llvm::FoldingSetNodeID ID;
2522-
TypeVariableType::Profile(ID, inDepth, inIndex);
2522+
TypeVariableType::Profile(ID, inDepth, inIndex, isInBoundsSafeInterface);
25232523

25242524
void *InsertPos = nullptr;
25252525
if (TypeVariableType *PT = TypeVariableTypes.FindNodeOrInsertPos(ID, InsertPos))
@@ -8075,7 +8075,7 @@ QualType ASTContext::mergeFunctionParameterTypes(QualType lhs, QualType rhs,
80758075
/*BlockReturnType=*/false, IgnoreBounds);
80768076
}
80778077

8078-
QualType ASTContext::mergeFunctionTypes(QualType lhs, QualType rhs,
8078+
QualType ASTContext::mergeFunctionTypes(QualType lhs, QualType rhs,
80798079
bool OfBlockPointer,
80808080
bool Unqualified,
80818081
bool IgnoreBounds) {
@@ -8100,7 +8100,7 @@ QualType ASTContext::mergeFunctionTypes(QualType lhs, QualType rhs,
81008100
retType = mergeTypes(lbase->getReturnType(), rbase->getReturnType(), false,
81018101
Unqualified,/*BlockReturnType=*/false, IgnoreBounds);
81028102
if (retType.isNull()) return QualType();
8103-
8103+
81048104
if (Unqualified)
81058105
retType = retType.getUnqualifiedType();
81068106

@@ -8110,7 +8110,7 @@ QualType ASTContext::mergeFunctionTypes(QualType lhs, QualType rhs,
81108110
LRetType = LRetType.getUnqualifiedType();
81118111
RRetType = RRetType.getUnqualifiedType();
81128112
}
8113-
8113+
81148114
if (getCanonicalType(retType) != LRetType)
81158115
allLTypes = false;
81168116
if (getCanonicalType(retType) != RRetType)
@@ -8148,25 +8148,44 @@ QualType ASTContext::mergeFunctionTypes(QualType lhs, QualType rhs,
81488148

81498149
FunctionType::ExtInfo einfo = lbaseInfo.withNoReturn(NoReturn);
81508150
unsigned NumTypeVars = 0;
8151+
bool IsITypeGenericFunction = false;
8152+
81518153
if (lproto && rproto) { // two C99 style function prototypes
81528154
assert(!lproto->hasExceptionSpec() && !rproto->hasExceptionSpec() &&
81538155
"C++ shouldn't be here");
8156+
81548157
// Compatible functions must have the same number of parameters
81558158
if (lproto->getNumParams() != rproto->getNumParams())
81568159
return QualType();
81578160

8158-
// Compatible functions must have the same number of type variables.
8159-
if (lproto->getNumTypeVars() != rproto->getNumTypeVars()) {
8160-
if (lproto->isItypeGenericFunction() && !rproto->isItypeGenericFunction()) {
8161-
allRTypes = false;
8162-
NumTypeVars = lproto->getNumTypeVars();
8163-
} else if (!lproto->isItypeGenericFunction() && rproto->isItypeGenericFunction()) {
8164-
allLTypes = false;
8165-
NumTypeVars = rproto->getNumTypeVars();
8166-
} else {
8161+
// Compatible functions must be:
8162+
// 1. Both nongeneric, or
8163+
// 2. Both generic, or
8164+
// 3. Both itype generic, or
8165+
// 4. One is non-generic and the other has an itype.
8166+
// For cases 1-3, the number of type variables must match.
8167+
8168+
if ((lproto->isNonGenericFunction() && rproto->isNonGenericFunction()) ||
8169+
(lproto->isGenericFunction() && rproto->isGenericFunction()) ||
8170+
(lproto->isItypeGenericFunction() && rproto->isItypeGenericFunction())) {
8171+
if (lproto->getNumTypeVars() != rproto->getNumTypeVars())
81678172
return QualType();
8173+
else {
8174+
NumTypeVars = lproto->getNumTypeVars();
8175+
IsITypeGenericFunction = lproto->isItypeGenericFunction();
81688176
}
8169-
}
8177+
} else if (lproto->isItypeGenericFunction() &&
8178+
rproto->isNonGenericFunction()) {
8179+
allRTypes = false;
8180+
NumTypeVars = lproto->getNumTypeVars();
8181+
IsITypeGenericFunction = true;
8182+
} else if (lproto->isNonGenericFunction() &&
8183+
rproto->isItypeGenericFunction()) {
8184+
allLTypes = false;
8185+
NumTypeVars = rproto->getNumTypeVars();
8186+
IsITypeGenericFunction = true;
8187+
} else
8188+
return QualType();
81708189

81718190
// Variadic and non-variadic functions aren't compatible
81728191
if (lproto->isVariadic() != rproto->isVariadic())
@@ -8263,7 +8282,7 @@ QualType ASTContext::mergeFunctionTypes(QualType lhs, QualType rhs,
82638282
}
82648283
}
82658284
}
8266-
8285+
82678286
if (allLTypes) return lhs;
82688287
if (allRTypes) return rhs;
82698288

@@ -8275,7 +8294,7 @@ QualType ASTContext::mergeFunctionTypes(QualType lhs, QualType rhs,
82758294
EPI.ParamAnnots = bounds.data();
82768295
EPI.ReturnAnnots = ReturnAnnots;
82778296
EPI.numTypeVars = NumTypeVars;
8278-
8297+
EPI.ItypeGenericFunction = IsITypeGenericFunction;
82798298
return getFunctionType(retType, types, EPI);
82808299
}
82818300

lib/AST/ASTDumper.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1221,9 +1221,11 @@ void ASTDumper::VisitFunctionDecl(const FunctionDecl *D) {
12211221
}
12221222
}
12231223

1224-
// If the function is generic function, dump information about type variable.
1225-
// Type variable is stored as a TypedefDecl.
1226-
if (D->isGenericFunction() && D->getNumTypeVars() > 0) {
1224+
// If the function is generic function or an itype generic function, dump
1225+
// information about type variables. The type variable name is stored as a
1226+
// TypedefDecl.
1227+
if ((D->isGenericFunction() || D->isItypeGenericFunction()) &&
1228+
D->getNumTypeVars() > 0) {
12271229
for (const TypedefDecl* Typevar : D->typeVariables()) {
12281230
dumpChild([=] {
12291231
OS << "TypeVariable";

lib/AST/TypePrinter.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -943,11 +943,10 @@ void TypePrinter::printUnresolvedUsingAfter(const UnresolvedUsingType *T,
943943

944944
void TypePrinter::printTypeVariableBefore(const TypeVariableType *T,
945945
raw_ostream &OS) {
946-
OS << "(" << T->GetDepth() << ", " << T->GetIndex();
946+
OS << "(" << T->GetDepth() << ", " << T->GetIndex() << ")";
947947
if (T->IsBoundsInterfaceType()) {
948-
OS << " __BoundsInterfaceScope(true)";
948+
OS << " __BoundsInterface";
949949
}
950-
OS << ")";
951950
}
952951

953952
void TypePrinter::printTypeVariableAfter(const TypeVariableType *T, raw_ostream &OS) { }

0 commit comments

Comments
 (0)