Skip to content

[checked-c-convert] Porting Tool Updates #642

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 90 commits into from
Aug 14, 2019

Conversation

rchyena
Copy link
Contributor

@rchyena rchyena commented Jul 25, 2019

This PR adds several new features to checked-c-convert, including NT_array detection.

@Machiry
Copy link
Member

Machiry commented Aug 12, 2019

Overall, this looks great! Great work!

I have some changes that I've requested:

  • There's a space appearing after the itype constraint when it is added: `f(int * : itype(_Ptr) ). Could you remove it? The output of the tool is much nicer to read.

✅ Fixed.

  • The new code often uses maybe 30 characters a line for comments, spacing the comment across multiple lines. That is hard to read. The clang line limit is 80 characters. Could you take advantage of that and have longer lines?

✅ Fixed.

  • There were a number of typos in the comments that I pointed.

✅ Fixed. I used a spell checker this time :)

  • The formatting for if statements is still off in the new code that you've written. There are a number of places where you wrote if(, and a space needs to be placed after the if . I pointed a few out, and left many more for you to fix.

✅ Fixed. I used a format checker this time.

There are markdown files for Checked C in clang\docs\checkedc. Could you create one that capture the content that you posted about the algorithm in the conversion tool? That was really helpful for me. If you want, you can post it later as a separate commit. I just want to make it gets documented).

📚 Created one.

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.

Looks great! Thank you!

@dtarditi dtarditi merged commit 53fa759 into checkedc:master Aug 14, 2019
@Machiry Machiry deleted the PR/aravind-all branch September 3, 2019 18:59
mgrang pushed a commit that referenced this pull request Sep 27, 2019
Cherry-picked from commit 53fa759

    This is a major overhaul of the Checked C conversion tool.  It adds Nt_array_ptr detection, maintains modifiers and qualifiers on declarations properly, handles cases more consistently, and adds more test cases.
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.

3 participants