Skip to content

Revamp implementation of generic functions. #581

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Oct 29, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix bugs and add diagnostics for generic types.
1.The calculation of nesting depths of type variables (the number
  of generic quantiifers 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. 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.
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.

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.
- Passed existing Checked C, Checked C clang, and clang tests locally.
  • Loading branch information
dtarditi committed Oct 27, 2018
commit ccfc0f2b1d527ce1ec045553c2676d0be58e8dde
3 changes: 3 additions & 0 deletions include/clang/AST/Type.h
Original file line number Diff line number Diff line change
Expand Up @@ -3564,6 +3564,9 @@ class FunctionProtoType : public FunctionType, public llvm::FoldingSetNode {
unsigned getNumTypeVars() const { return NumTypeVars; }
bool isGenericFunction() const { return GenericFunction; }
bool isItypeGenericFunction() const { return ItypeGenericFunction; }
bool isNonGenericFunction() const {
return !(GenericFunction || ItypeGenericFunction);
}

ArrayRef<QualType> getParamTypes() const {
return llvm::makeArrayRef(param_type_begin(), param_type_end());
Expand Down
6 changes: 6 additions & 0 deletions include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -9806,6 +9806,12 @@ def err_bounds_type_annotation_lost_checking : Error<
def no_prototype_generic_function : Error<
"expected prototype for a generic function">;

def err_decl_conflicting_generic_non_generic_functions : Error<
"conflicting non-generic and generic declarations of %0" >;

def err_decl_conflicting_type_variable_count : Error<
"conflicting numbers of type variables for declarations of %0" >;

def err_expected_type_argument_list_for_generic_call : Error<
"expected a type argument list for a generic function call">;

Expand Down
5 changes: 4 additions & 1 deletion include/clang/Parse/Parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -996,6 +996,10 @@ class Parser : public CodeCompletionHandler {
/// point for skipping past a simple-declaration.
void SkipMalformedDecl();

// If the current scope is a Checked C _Forany or _Itypeforany scope, exit it.
// TODO: this probably doesn't belong in the parser.
void ExitQuantifiedTypeScope(DeclSpec &DS);

private:
//===--------------------------------------------------------------------===//
// Lexing and parsing of C++ inline methods.
Expand Down Expand Up @@ -1776,7 +1780,6 @@ class Parser : public CodeCompletionHandler {

ExprResult ParseReturnValueExpression();


//===--------------------------------------------------------------------===//
// clang Expressions

Expand Down
2 changes: 1 addition & 1 deletion include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -4812,7 +4812,7 @@ class Sema {
ExprResult ActOnTypeApplication(ExprResult TypeFunc, SourceLocation Loc,
ArrayRef<DeclRefExpr::GenericInstInfo::TypeArgument> Args);

QualType SubstituteTypeArgs(QualType QT,
QualType SubstituteTypeArgs(QualType QT,
ArrayRef<DeclRefExpr::GenericInstInfo::TypeArgument> TypeArgs);

bool AbstractForFunctionType(BoundsAnnotations &BA,
Expand Down
49 changes: 34 additions & 15 deletions lib/AST/ASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8075,7 +8075,7 @@ QualType ASTContext::mergeFunctionParameterTypes(QualType lhs, QualType rhs,
/*BlockReturnType=*/false, IgnoreBounds);
}

QualType ASTContext::mergeFunctionTypes(QualType lhs, QualType rhs,
QualType ASTContext::mergeFunctionTypes(QualType lhs, QualType rhs,
bool OfBlockPointer,
bool Unqualified,
bool IgnoreBounds) {
Expand All @@ -8100,7 +8100,7 @@ QualType ASTContext::mergeFunctionTypes(QualType lhs, QualType rhs,
retType = mergeTypes(lbase->getReturnType(), rbase->getReturnType(), false,
Unqualified,/*BlockReturnType=*/false, IgnoreBounds);
if (retType.isNull()) return QualType();

if (Unqualified)
retType = retType.getUnqualifiedType();

Expand All @@ -8110,7 +8110,7 @@ QualType ASTContext::mergeFunctionTypes(QualType lhs, QualType rhs,
LRetType = LRetType.getUnqualifiedType();
RRetType = RRetType.getUnqualifiedType();
}

if (getCanonicalType(retType) != LRetType)
allLTypes = false;
if (getCanonicalType(retType) != RRetType)
Expand Down Expand Up @@ -8148,25 +8148,44 @@ QualType ASTContext::mergeFunctionTypes(QualType lhs, QualType rhs,

FunctionType::ExtInfo einfo = lbaseInfo.withNoReturn(NoReturn);
unsigned NumTypeVars = 0;
bool IsITypeGenericFunction = false;

if (lproto && rproto) { // two C99 style function prototypes
assert(!lproto->hasExceptionSpec() && !rproto->hasExceptionSpec() &&
"C++ shouldn't be here");

// Compatible functions must have the same number of parameters
if (lproto->getNumParams() != rproto->getNumParams())
return QualType();

// Compatible functions must have the same number of type variables.
if (lproto->getNumTypeVars() != rproto->getNumTypeVars()) {
if (lproto->isItypeGenericFunction() && !rproto->isItypeGenericFunction()) {
allRTypes = false;
NumTypeVars = lproto->getNumTypeVars();
} else if (!lproto->isItypeGenericFunction() && rproto->isItypeGenericFunction()) {
allLTypes = false;
NumTypeVars = rproto->getNumTypeVars();
} else {
// Compatible functions must be:
// 1. Both nongeneric, or
// 2. Both generic, or
// 3. Both itype generic, or
// 4. One is non-generic and the other has an itype.
// For cases 1-3, the number of type variables must match.

if ((lproto->isNonGenericFunction() && rproto->isNonGenericFunction()) ||
(lproto->isGenericFunction() && rproto->isGenericFunction()) ||
(lproto->isItypeGenericFunction() && rproto->isItypeGenericFunction())) {
if (lproto->getNumTypeVars() != rproto->getNumTypeVars())
return QualType();
else {
NumTypeVars = lproto->getNumTypeVars();
IsITypeGenericFunction = lproto->isItypeGenericFunction();
}
}
} else if (lproto->isItypeGenericFunction() &&
rproto->isNonGenericFunction()) {
allRTypes = false;
NumTypeVars = lproto->getNumTypeVars();
IsITypeGenericFunction = true;
} else if (lproto->isNonGenericFunction() &&
rproto->isItypeGenericFunction()) {
allLTypes = false;
NumTypeVars = rproto->getNumTypeVars();
IsITypeGenericFunction = true;
} else
return QualType();

// Variadic and non-variadic functions aren't compatible
if (lproto->isVariadic() != rproto->isVariadic())
Expand Down Expand Up @@ -8263,7 +8282,7 @@ QualType ASTContext::mergeFunctionTypes(QualType lhs, QualType rhs,
}
}
}

if (allLTypes) return lhs;
if (allRTypes) return rhs;

Expand All @@ -8275,7 +8294,7 @@ QualType ASTContext::mergeFunctionTypes(QualType lhs, QualType rhs,
EPI.ParamAnnots = bounds.data();
EPI.ReturnAnnots = ReturnAnnots;
EPI.numTypeVars = NumTypeVars;

EPI.ItypeGenericFunction = IsITypeGenericFunction;
return getFunctionType(retType, types, EPI);
}

Expand Down
8 changes: 5 additions & 3 deletions lib/AST/ASTDumper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1221,9 +1221,11 @@ void ASTDumper::VisitFunctionDecl(const FunctionDecl *D) {
}
}

// If the function is generic function, dump information about type variable.
// Type variable is stored as a TypedefDecl.
if (D->isGenericFunction() && D->getNumTypeVars() > 0) {
// If the function is generic function or an itype generic function, dump
// information about type variables. The type variable name is stored as a
// TypedefDecl.
if ((D->isGenericFunction() || D->isItypeGenericFunction()) &&
D->getNumTypeVars() > 0) {
for (const TypedefDecl* Typevar : D->typeVariables()) {
dumpChild([=] {
OS << "TypeVariable";
Expand Down
70 changes: 47 additions & 23 deletions lib/Parse/ParseDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ TypeResult Parser::ParseTypeName(SourceRange *Range,
// Parse the abstract-declarator, if present.
Declarator DeclaratorInfo(DS, Context);
ParseDeclarator(DeclaratorInfo);
ExitQuantifiedTypeScope(DS);

if (Range)
*Range = DeclaratorInfo.getSourceRange();
Expand Down Expand Up @@ -1729,12 +1730,15 @@ Parser::ParseSimpleDeclaration(unsigned Context,
// If we had a free-standing type definition with a missing semicolon, we
// may get this far before the problem becomes obvious.
if (DS.hasTagDefinition() &&
DiagnoseMissingSemiAfterTagDefinition(DS, AS_none, DSContext))
DiagnoseMissingSemiAfterTagDefinition(DS, AS_none, DSContext)) {
ExitQuantifiedTypeScope(DS);
return nullptr;
}

// C99 6.7.2.3p6: Handle "struct-or-union identifier;", "enum { X };"
// declaration-specifiers init-declarator-list[opt] ';'
if (Tok.is(tok::semi)) {
ExitQuantifiedTypeScope(DS);
ProhibitAttributes(Attrs);
DeclEnd = Tok.getLocation();
if (RequireSemi) ConsumeToken();
Expand All @@ -1744,6 +1748,7 @@ Parser::ParseSimpleDeclaration(unsigned Context,
DS.complete(TheDecl);
if (AnonRecord) {
Decl* decls[] = {AnonRecord, TheDecl};

return Actions.BuildDeclaratorGroup(decls);
}
return Actions.ConvertDeclToDeclGroup(TheDecl);
Expand Down Expand Up @@ -1907,6 +1912,19 @@ void Parser::SkipMalformedDecl() {
}
}

void Parser::ExitQuantifiedTypeScope(DeclSpec &DS) {
if (DS.isForanySpecified()) {
assert(getCurScope()->isForanyScope()
&& "Current scope should be created by _For_any specifier.");
ExitScope();
}
if (DS.isItypeforanySpecified()) {
assert(getCurScope()->isItypeforanyScope()
&& "Current scope should be created by _Itype_forany specifier.");
ExitScope();
}
}

/// ParseDeclGroup - Having concluded that this is either a function
/// definition or a group of object declarations, actually parse the
/// result.
Expand All @@ -1921,6 +1939,7 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
// Bail out if the first declarator didn't seem well-formed.
if (!D.hasName() && !D.mayOmitIdentifier()) {
SkipMalformedDecl();
ExitQuantifiedTypeScope(DS);
return nullptr;
}

Expand Down Expand Up @@ -1984,6 +2003,7 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
D.getIdentifier();
// bail out
SkipMalformedDecl();
ExitQuantifiedTypeScope(DS);
return nullptr;
}
}
Expand All @@ -2005,16 +2025,7 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
Decl *TheDecl =
ParseFunctionDefinition(D, ParsedTemplateInfo(), &LateParsedAttrs);
// If we encountered _For_any make sure we're in Forany scope and exit.
if (DS.isForanySpecified()) {
assert(getCurScope()->isForanyScope()
&& "Current scope should be created by _For_any specifier.");
ExitScope();
}
if (DS.isItypeforanySpecified()) {
assert(getCurScope()->isItypeforanyScope()
&& "Current scope should be created by _Itype_forany specifier.");
ExitScope();
}
ExitQuantifiedTypeScope(DS);
return Actions.ConvertDeclToDeclGroup(TheDecl);
}

Expand All @@ -2034,13 +2045,16 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
if (Tok.is(tok::l_brace)) {
Diag(Tok, diag::err_function_definition_not_allowed);
SkipMalformedDecl();
ExitQuantifiedTypeScope(DS);
return nullptr;
}
}
}

if (ParseAsmAttributesAfterDeclarator(D))
if (ParseAsmAttributesAfterDeclarator(D)) {
ExitQuantifiedTypeScope(DS);
return nullptr;
}

// C++0x [stmt.iter]p1: Check if we have a for-range-declarator. If so, we
// must parse and analyze the for-range-initializer before the declaration is
Expand All @@ -2049,6 +2063,7 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
// Handle the Objective-C for-in loop variable similarly, although we
// don't need to parse the container in advance.
if (FRI && (Tok.is(tok::colon) || isTokIdentifier_in())) {
ExitQuantifiedTypeScope(DS);
bool IsForRangeLoop = false;
if (TryConsumeToken(tok::colon, FRI->ColonLoc)) {
IsForRangeLoop = true;
Expand All @@ -2063,6 +2078,7 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
Actions.ActOnCXXForRangeDecl(ThisDecl);
Actions.FinalizeDeclaration(ThisDecl);
D.complete(ThisDecl);

return Actions.FinalizeDeclaratorGroup(getCurScope(), DS, ThisDecl);
}

Expand Down Expand Up @@ -2133,13 +2149,7 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
}
}

// If we encountered _For_any, make sure we're in Forany scope and exit.
if (DS.isForanySpecified()) {
assert(getCurScope()->isForanyScope()
&& "Current scope should be created by _For_any specifier.");
ExitScope();
}

ExitQuantifiedTypeScope(DS);
return Actions.FinalizeDeclaratorGroup(getCurScope(), DS, DeclsInGroup);
}

Expand Down Expand Up @@ -2231,11 +2241,14 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
// Inform the current actions module that we just parsed this declarator.
Decl *ThisDecl = nullptr;
switch (TemplateInfo.Kind) {
case ParsedTemplateInfo::NonTemplate:
case ParsedTemplateInfo::NonTemplate: {
// if _For_any specifier, make sure to add function decl in correct scope.
ThisDecl = Actions.ActOnDeclarator(D.getDeclSpec().isForanySpecified() ?
bool isQuantifiedScope =
D.getDeclSpec().isForanySpecified() || D.getDeclSpec().isItypeforanySpecified();
ThisDecl = Actions.ActOnDeclarator(isQuantifiedScope ?
getCurScope()->getParent() : getCurScope(), D);
break;
}

case ParsedTemplateInfo::Template:
case ParsedTemplateInfo::ExplicitSpecialization: {
Expand Down Expand Up @@ -2934,6 +2947,7 @@ Parser::DiagnoseMissingSemiAfterTagDefinition(DeclSpec &DS, AccessSpecifier AS,
DS.ClearTypeSpecType();
ParsedTemplateInfo NotATemplate;
ParseDeclarationSpecifiers(DS, NotATemplate, AS, DSContext, LateAttrs);
ExitQuantifiedTypeScope(DS);
return false;
}

Expand Down Expand Up @@ -3971,11 +3985,13 @@ void Parser::ParseStructDeclaration(
// If there are no declarators, this is a free-standing declaration
// specifier. Let the actions module cope with it.
if (Tok.is(tok::semi)) {
ExitQuantifiedTypeScope(DS);
RecordDecl *AnonRecord = nullptr;
Decl *TheDecl = Actions.ParsedFreeStandingDeclSpec(getCurScope(), AS_none,
DS, AnonRecord);
assert(!AnonRecord && "Did not expect anonymous struct or union here");
DS.complete(TheDecl);

return;
}

Expand Down Expand Up @@ -4038,6 +4054,8 @@ void Parser::ParseStructDeclaration(

FieldsCallback(DeclaratorInfo);

ExitQuantifiedTypeScope(DS);

// If we don't have a comma, it is either the end of the list (a ';')
// or an error, bail out.
if (!TryConsumeToken(tok::comma, CommaLoc))
Expand Down Expand Up @@ -6656,6 +6674,11 @@ void Parser::ParseParameterDeclarationClause(
Declarator::PrototypeContext);
ParseDeclarator(ParmDeclarator);

// Checked C: exit _Forany or _Itype_forany scope. This is intentionally done here
// because the bounds / interface type for the parameter shouldn't reference
// any bound type variable.
ExitQuantifiedTypeScope(DS);

// Parse GNU attributes, if present.
MaybeParseGNUAttributes(ParmDeclarator);

Expand Down Expand Up @@ -7241,9 +7264,10 @@ void Parser::ParseForanySpecifier(DeclSpec &DS) {
bool Parser::ParseGenericFunctionSpecifierHelper(DeclSpec &DS,
Scope::ScopeFlags S) {
assert((S == Scope::ForanyScope) || (S == Scope::ItypeforanyScope));


EnterScope(Scope::DeclScope | S);
SourceLocation Loc = ConsumeToken();

StringRef Key = (S == Scope::ForanyScope) ? "_For_any" : "Itype_for_any";
if (ExpectAndConsume(tok::l_paren, diag::err_generic_specifier_unexpected_token, Key)) {
ExitScope();
Expand All @@ -7257,7 +7281,7 @@ bool Parser::ParseGenericFunctionSpecifierHelper(DeclSpec &DS,
// Calculate the depth of "for any"/"itype for any" scope for our generic types
unsigned int Depth = 0;
Scope *tempScope = getCurScope()->getParent();
while (!tempScope) {
while (tempScope) {
if (tempScope->isForanyScope() || tempScope->isItypeforanyScope())
Depth++;
tempScope = tempScope->getParent();
Expand Down
Loading