Skip to content
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

Revamp implementation of generic functions. #581

Merged
merged 10 commits into from Oct 29, 2018
Merged

Revamp implementation of generic functions. #581

merged 10 commits into from Oct 29, 2018

Conversation

dtarditi
Copy link
Contributor

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). 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 "Address of" operation on generic functions with type parameters #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.
    1. Dumping of itype generic functions differed from generic functions.
    2. 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.
    3. When adding function declarations to scopes, we didn't properly go to the enclosing scope when the function was an itype forall.
    4. 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.

We implemented parsing of generic type applications as part of an intern
project.  The parsing code relied on detecting an occurrence of generic
function (or a function with a generic bounds-safe interface in a checked
scope). If either condition was met, the code assumed that a generic type
argument list must follow.  It tried to parse that type argument list to
create a type application.

Reimplement parsing of generic type applications so that it does not depend on
information from semantic checking.  Relying on information from semantic
checking is a potential sign that that grammer for ambiguous.

We treat generic type applications as postfix expression, with the
grammar production:

postfix-expression:
   postfix-expression < type argument list>

where type argument list consists of a comma separated list of zero
more type arguments.

This isn't ambiguous because the `<` operator cannot currently 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 the name of a type or
the name of a variable.  We haven't made this worse.

Add a new diagnostic message for the case where a non-generic function
is applied to type arguments.  Before, there was just a parsing failure
of the form "expected expression".

This change partly addresses Github issue #559 (address of operator
on function with generic type parameters).  It now allows the address-of
operator to be applied to generic functions with type arguments.

Testing:
- Passes existing tests, except for these where a generic function
  is called without applying it to type arguments.
If a function with a generic bounds-safe interface is called in a
_Checked _bounds_only or an unchecked context, and it hasn't been
applied to type arguments, supply void as the type argument.

Move the code for substituting type arguments for type variables to
ASTContext.cpp.

Testing:
- The substitution code alters the return and parameter type
  when a bounds-safe interface is present, instead of updating
  the bounds-safe interfaces.  This causes a handful of tests to
  fail.
- Passes other local testing.
Type substitution done at type applications is buggy.  Refactor
code before fixing it.  The fix will require access to the AST
tree writing support in TreeTransform.h.

- Move type substitution from ASTContext to the Sema directory.
  Add a new file CheckedCSubst.cpp to hold it.
- Move semantic checking for type applications from Parse
  to Sema.  Place a new function for this in CheckedCSubst.cpp.
  This is where it should have been to start with.

Testing:
- Passes existing tests, with failures due to issues in type
  substitution.
First take at fixing type substitution for type applications so that it
is correct for bounds expression.  Use TreeTransform so that type
variables in bounds expressions are rewritten.

Still need to revise handling of lexixcal
depth so that nested generic function types work properly.
- 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.
 - 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.

Change the way that AST dumps are done for type variables with bounds-safe
interfaces.

Testing:
- Update existing test of AST dumping of type variables.
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.
@dtarditi dtarditi merged commit 946d86f into master Oct 29, 2018
@dtarditi dtarditi deleted the issue572 branch November 1, 2018 00:41
sulekhark pushed a commit that referenced this pull request Jul 8, 2021
…les. (#593)

* Re-enable the canwrite_constraints test except for diagnostic
verification.

Reformat a CHECK comment to match the code reformatting done in
2a6a28e. (This wasn't caught at the
time because the test was XFAILed.) After this fix, the test passes
except for #581.

Also remove some obsolete TODO comments: there are no known problems
with these tests on Windows.

* Don't allow itypes in unwritable files to change.

Also take the opportunity to improve the documentation of
equateWithItype based on what I learned. Some things are probably still
wrong with the code and/or the documentation, but I think this is better
than the status quo.

Fixes #581.

* Add variable to make equateWithItype conditionals easier to understand.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant