Skip to content

Making checked-c-convert into a library #837

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 23 commits into from
Jun 6, 2020

Conversation

Machiry
Copy link
Member

@Machiry Machiry commented May 18, 2020

Made the core of checked-c-convert into a library so that it can be used as a standalone tool as well as a part of clangd (Language server).

Abel Nieto and others added 19 commits September 19, 2019 19:17
This large change adds support for existential struct types to Checked C.  These can be used to replace uses of void pointers in callbacks, abstract data types, and closures.  There is a corresponding commit of tests for existential types to the Checked C repo.

This changes adds IR support, parsing support, semantic checking/typechecking support, and code generation support.  Serialization/deserialization support still needs to be implemented.   There are also a number of cleanups to do, which are documented in GitHub issues.   There are three parts to the change:
- Support for existential types.
- The pack operation.
- The unpack operation, which is done as a declaration with an initializer.


* Add a new existential type to the type hierarchy

* Handle existential types in different places

* More handling of new ExistentialType

* Add new _Exists token

* Handle existentials in a few more places

* Handle existentials in additional places (including codegen)

* [wip] Add an existential type to clang

* Desugar type variable in existential

* Fix bug in desugaring of exists

* Add dedicated scopes for existentials

* Rename some variables

* [wip] Instantiate existential types

* Fix bugs

* Better logic for determining whether an Existential is an aggregate

* Disambiguate between expressions and decls in the parser

* [wip] add a pack operation and start handling it all over the compiler

* Add support for pack in additional locations

* Hook up pack operation in parser

* [wip] parse pack expressions and generate ast node

* Can now parse pack expressions

* Codegen pack expressions

* Slightly  better merge function for existential types

* Better caching of type applications and existentials

* Fix typo in comment

* Check witness type in pack expressions

* Add pack expression in the right place

* [wip] parse unpack type specifiers

* [wip] parse unpack specifiers

* Move unpack to custom specifier

* Differentiate between unpack variables in same scope

* Add hacky rule for typing unpack init

* bug fix

* Minor fixes

* Restructure ExistentialType to contain more than a TypeVariableType for the TypeVar

* Fix evaluation kind resolution

* Better canonical types for existentials

* Simplifying generation of canonical types

* Fix bug in creating existential type

* Revert changes to numbering of existential type variables

* Fix bug in allocation logic

* Implement a visitor that computes free variables

* Comment out logging code

* Implement alpha renamer and wip canonical type generation

* Hooked up canonicalization to creation of existential types

* Fix bugs

* handle typedefs in alpha renamer

* handle type applications

* fix bug

* fix another bug

* remoev incorrect canonicalization

* fix bug in insertion to subst map

* fix bug in checking witness type

* Add TODO

* fix codegen of type applications

* Remove unused static variable

* Emit parsing error while parsing existentials

* Improve wording

* Rename ExistTpe to ExistType

* Improve wording

* Improve wording

* Add issue number to TODOs

* Improve wording

* Add explanation

* Fix typo

* Check return type of pack expressions (must be an existential)

* Strengthen assertion

* Tag comments with issue number

* Tag comment

* Improve wording

* Replace assertion by runtime error message

* Check that an unpack initializer has an existential type

* Add comment about pack expressions being r-values

* Remove unneeded comment

* Compute type linkage info in different way

* Tag comment

* Improve comment

* Tag comment

* Check for malformed unpacks

* Fix warning and missing symbol

* Add missing switch case

* Partially disable clang test

* Fix codegen check

* Fix typo

* Improve handling of linkage for existentials

* Use getAs() instead of manually getting canonical type

* Address code review comment
he release build of the Checked C clang compiler fails to compile the new tests for existential types. The iterator functions for PackExpr were incorrect: they need to return the address of the member, not the address of a local variable.

Also add missing classof method for PackExpr.

Testing:
- Passed release with debug info build on local machine.
- Add knowledge of calloc and realloc as allocator functions.
- Add initializers for variables retyped as checked pointers. 
- Add initializers for structure variables containing members that have been retyped as checked pointers.
- Avoid deletion of variable arguments.
* Remove -fno-checkedc-null-ptr-arith flag from array-related dynamic checks tests

* Remove -fno-checkedc-null-ptr-arith flag from struct-related dynamic checks tests
@Machiry Machiry changed the title Cconvlibrary Makingchecked-c-convert into a library May 18, 2020
@Machiry Machiry changed the title Makingchecked-c-convert into a library Making checked-c-convert into a library May 18, 2020
Copy link

@mgrang mgrang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove from your PR the files under clang-tidy-vs and scan-build which you haven't changed.

@Machiry
Copy link
Member Author

Machiry commented May 20, 2020

Thank you, @mgrang. Handled review comments.

@Machiry Machiry requested a review from mgrang May 21, 2020 22:36
@Machiry Machiry requested a review from mgrang May 23, 2020 00:56
@mgrang
Copy link

mgrang commented May 27, 2020

Thanks for addressing the comments. LGTM.

@mgrang mgrang requested a review from dtarditi May 27, 2020 17:55
Copy link
Member

@dtarditi dtarditi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. @mgrang, could you merge this change on @Machiry's behalf?

@mgrang mgrang merged commit fa4435f into checkedc:master Jun 6, 2020
mgrang pushed a commit that referenced this pull request Jun 10, 2020
After #837 merged we see the following three unit tests failing intermittently
only on Windows:

- test/CheckedCRewriter/global.c
- test/CheckedCRewriter/simple_locals.c
- test/CheckedCRewriter/basic_checks.c

In order to unblock our failing ADO builds we are disabling these three tests.
We can enable them when we have these fixed.
mgrang pushed a commit that referenced this pull request Jun 11, 2020
After #837 merged we see the following three unit tests failing intermittently
only on Windows:

test/CheckedCRewriter/global.c
test/CheckedCRewriter/simple_locals.c
test/CheckedCRewriter/basic_checks.c

In order to unblock our failing ADO builds we are disabling these three tests.
We can enable them when we have these fixed.

See issue #851
@mgrang
Copy link

mgrang commented Jun 11, 2020

Hi @Machiry , this PR is breaking some tests on Windows. Please see #851.

mgrang pushed a commit that referenced this pull request Jun 11, 2020
After #837 merged we see the following three unit tests failing intermittently
only on Windows:

test/CheckedCRewriter/global.c
test/CheckedCRewriter/simple_locals.c
test/CheckedCRewriter/basic_checks.c

In order to unblock our failing ADO builds we are disabling these three tests.
We can enable them when we have these fixed.

See issue #851
mattmccutchen-cci added a commit to correctcomputation/checkedc-clang that referenced this pull request Dec 30, 2020
277d84a.

We first submitted them in PR checkedc#837, but Mandeep noticed
(checkedc#837 (review))
and the unintended changes were removed from that PR in
74bfcaf.  However, when the squash of
PR checkedc#837 was merged with the original commits in
cfc998e, the unintended changes were
incorrectly retained.  They got submitted again in the next 3C PR
(checkedc#891), and no one noticed that time.
mgrang pushed a commit that referenced this pull request Jan 19, 2021
)

277d84a.

We first submitted them in PR #837, but Mandeep noticed
(#837 (review))
and the unintended changes were removed from that PR in
74bfcaf.  However, when the squash of
PR #837 was merged with the original commits in
cfc998e, the unintended changes were
incorrectly retained.  They got submitted again in the next 3C PR
(#891), and no one noticed that time.
kkjeer pushed a commit that referenced this pull request Jan 19, 2021
)

277d84a.

We first submitted them in PR #837, but Mandeep noticed
(#837 (review))
and the unintended changes were removed from that PR in
74bfcaf.  However, when the squash of
PR #837 was merged with the original commits in
cfc998e, the unintended changes were
incorrectly retained.  They got submitted again in the next 3C PR
(#891), and no one noticed that time.

(cherry picked from commit cfe00ea)
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.

4 participants