{"action":"created","comment":{"url":"https://api.github.com/repos/Microsoft/checkedc-clang/pulls/comments/116637516","pull_request_review_id":38271525,"id":116637516,"diff_hunk":"@@ -0,0 +1,35 @@\n+// Checks for crashes during type-checking error.\r\n+// This was caused by an lvalue assert in SemaBounds.cpp\r\n+// The goal of this test is to make sure we're seeing \r\n+// type-checking errors, but not get any crashes.\r\n+// The llvm-lit test will fail the test if the compiler\r\n+// crashes because of assert failure (This has been tested)\r\n+// I wanted to use FileCheck to specifically look for\r","path":"test/CheckedC/type-checking-crash.c","position":7,"original_position":7,"commit_id":"08ad097d5c019d71cc52430e0f30e9491cfc5c34","original_commit_id":"08ad097d5c019d71cc52430e0f30e9491cfc5c34","user":{"login":"dtarditi","id":11841034,"avatar_url":"https://avatars0.githubusercontent.com/u/11841034?v=3","gravatar_id":"","url":"https://api.github.com/users/dtarditi","html_url":"https://github.com/dtarditi","followers_url":"https://api.github.com/users/dtarditi/followers","following_url":"https://api.github.com/users/dtarditi/following{/other_user}","gists_url":"https://api.github.com/users/dtarditi/gists{/gist_id}","starred_url":"https://api.github.com/users/dtarditi/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dtarditi/subscriptions","organizations_url":"https://api.github.com/users/dtarditi/orgs","repos_url":"https://api.github.com/users/dtarditi/repos","events_url":"https://api.github.com/users/dtarditi/events{/privacy}","received_events_url":"https://api.github.com/users/dtarditi/received_events","type":"User","site_admin":false},"body":"The test looks good.    This comment needs to be cleaned up. I generally like tests to have comments with the following form:\r\n\r\n- 1-2 sentence summary of what the test does.\r\n- Blank comment line.\r\n- More detailed explanation.\r\n\r\nComments should be written with the idea that someone you don't know will look at it 2 years from now.  The first sentence needs to be more precise.   I'd suggest removing the comment about trying to use FileCheck.  As you mention, llvm-lit will fail a test if a compiler crashes because of an assert in a compiler, so you can't use FileCheck to check if an assertion was hit.\r\n\r\nIt seems a little strange to me that we get a type checking error after we get a bounds checking error.   I think it would be clearer to users if the type checking message happened first (generally, you'd want to fix typechecking errors first).\r\n","created_at":"2017-05-16T01:31:30Z","updated_at":"2017-05-16T01:31:35Z","html_url":"https://github.com/Microsoft/checkedc-clang/pull/296#discussion_r116637516","pull_request_url":"https://api.github.com/repos/Microsoft/checkedc-clang/pulls/296","_links":{"self":{"href":"https://api.github.com/repos/Microsoft/checkedc-clang/pulls/comments/116637516"},"html":{"href":"https://github.com/Microsoft/checkedc-clang/pull/296#discussion_r116637516"},"pull_request":{"href":"https://api.github.com/repos/Microsoft/checkedc-clang/pulls/296"}}},"pull_request":{"url":"https://api.github.com/repos/Microsoft/checkedc-clang/pulls/296","id":120411133,"html_url":"https://github.com/Microsoft/checkedc-clang/pull/296","diff_url":"https://github.com/Microsoft/checkedc-clang/pull/296.diff","patch_url":"https://github.com/Microsoft/checkedc-clang/pull/296.patch","issue_url":"https://api.github.com/repos/Microsoft/checkedc-clang/issues/296","number":296,"state":"open","locked":false,"title":"Type-checking error leads to compiler crash","user":{"login":"jp1729","id":8700707,"avatar_url":"https://avatars2.githubusercontent.com/u/8700707?v=3","gravatar_id":"","url":"https://api.github.com/users/jp1729","html_url":"https://github.com/jp1729","followers_url":"https://api.github.com/users/jp1729/followers","following_url":"https://api.github.com/users/jp1729/following{/other_user}","gists_url":"https://api.github.com/users/jp1729/gists{/gist_id}","starred_url":"https://api.github.com/users/jp1729/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jp1729/subscriptions","organizations_url":"https://api.github.com/users/jp1729/orgs","repos_url":"https://api.github.com/users/jp1729/repos","events_url":"https://api.github.com/users/jp1729/events{/privacy}","received_events_url":"https://api.github.com/users/jp1729/received_events","type":"User","site_admin":false},"body":"Type-checking error removes lvalue attribute in MemberExpr. This causes assert to throw error. Changed assert to conditional. Although there are numerous lvalue assert checks, it looks like the assert in Line402 is the only assert that it comes across. Fixes #291 ","created_at":"2017-05-12T21:38:12Z","updated_at":"2017-05-16T01:31:35Z","closed_at":null,"merged_at":null,"merge_commit_sha":"cc5a32c3a76199caf1e095701f372d44e095fa75","assignee":null,"assignees":[],"requested_reviewers":[],"milestone":null,"commits_url":"https://api.github.com/repos/Microsoft/checkedc-clang/pulls/296/commits","review_comments_url":"https://api.github.com/repos/Microsoft/checkedc-clang/pulls/296/comments","review_comment_url":"https://api.github.com/repos/Microsoft/checkedc-clang/pulls/comments{/number}","comments_url":"https://api.github.com/repos/Microsoft/checkedc-clang/issues/296/comments","statuses_url":"https://api.github.com/repos/Microsoft/checkedc-clang/statuses/08ad097d5c019d71cc52430e0f30e9491cfc5c34","head":{"label":"Microsoft:Type-checking-error-compiler-crash","ref":"Type-checking-error-compiler-crash","sha":"08ad097d5c019d71cc52430e0f30e9491cfc5c34","user":{"login":"Microsoft","id":6154722,"avatar_url":"https://avatars1.githubusercontent.com/u/6154722?v=3","gravatar_id":"","url":"https://api.github.com/users/Microsoft","html_url":"https://github.com/Microsoft","followers_url":"https://api.github.com/users/Microsoft/followers","following_url":"https://api.github.com/users/Microsoft/following{/other_user}","gists_url":"https://api.github.com/users/Microsoft/gists{/gist_id}","starred_url":"https://api.github.com/users/Microsoft/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Microsoft/subscriptions","organizations_url":"https://api.github.com/users/Microsoft/orgs","repos_url":"https://api.github.com/users/Microsoft/repos","events_url":"https://api.github.com/users/Microsoft/events{/privacy}","received_events_url":"https://api.github.com/users/Microsoft/received_events","type":"Organization","site_admin":false},"repo":{"id":60645050,"name":"checkedc-clang","full_name":"Microsoft/checkedc-clang","owner":{"login":"Microsoft","id":6154722,"avatar_url":"https://avatars1.githubusercontent.com/u/6154722?v=3","gravatar_id":"","url":"https://api.github.com/users/Microsoft","html_url":"https://github.com/Microsoft","followers_url":"https://api.github.com/users/Microsoft/followers","following_url":"https://api.github.com/users/Microsoft/following{/other_user}","gists_url":"https://api.github.com/users/Microsoft/gists{/gist_id}","starred_url":"https://api.github.com/users/Microsoft/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Microsoft/subscriptions","organizations_url":"https://api.github.com/users/Microsoft/orgs","repos_url":"https://api.github.com/users/Microsoft/repos","events_url":"https://api.github.com/users/Microsoft/events{/privacy}","received_events_url":"https://api.github.com/users/Microsoft/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/Microsoft/checkedc-clang","description":"This repo contains a version of clang that is being modified to support Checked C.   Checked C is an extension to C that adds checking to detect or prevent common programming errors such as out-of-bounds memory accesses.","fork":false,"url":"https://api.github.com/repos/Microsoft/checkedc-clang","forks_url":"https://api.github.com/repos/Microsoft/checkedc-clang/forks","keys_url":"https://api.github.com/repos/Microsoft/checkedc-clang/keys{/key_id}","collaborators_url":"https://api.github.com/repos/Microsoft/checkedc-clang/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/Microsoft/checkedc-clang/teams","hooks_url":"https://api.github.com/repos/Microsoft/checkedc-clang/hooks","issue_events_url":"https://api.github.com/repos/Microsoft/checkedc-clang/issues/events{/number}","events_url":"https://api.github.com/repos/Microsoft/checkedc-clang/events","assignees_url":"https://api.github.com/repos/Microsoft/checkedc-clang/assignees{/user}","branches_url":"https://api.github.com/repos/Microsoft/checkedc-clang/branches{/branch}","tags_url":"https://api.github.com/repos/Microsoft/checkedc-clang/tags","blobs_url":"https://api.github.com/repos/Microsoft/checkedc-clang/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/Microsoft/checkedc-clang/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/Microsoft/checkedc-clang/git/refs{/sha}","trees_url":"https://api.github.com/repos/Microsoft/checkedc-clang/git/trees{/sha}","statuses_url":"https://api.github.com/repos/Microsoft/checkedc-clang/statuses/{sha}","languages_url":"https://api.github.com/repos/Microsoft/checkedc-clang/languages","stargazers_url":"https://api.github.com/repos/Microsoft/checkedc-clang/stargazers","contributors_url":"https://api.github.com/repos/Microsoft/checkedc-clang/contributors","subscribers_url":"https://api.github.com/repos/Microsoft/checkedc-clang/subscribers","subscription_url":"https://api.github.com/repos/Microsoft/checkedc-clang/subscription","commits_url":"https://api.github.com/repos/Microsoft/checkedc-clang/commits{/sha}","git_commits_url":"https://api.github.com/repos/Microsoft/checkedc-clang/git/commits{/sha}","comments_url":"https://api.github.com/repos/Microsoft/checkedc-clang/comments{/number}","issue_comment_url":"https://api.github.com/repos/Microsoft/checkedc-clang/issues/comments{/number}","contents_url":"https://api.github.com/repos/Microsoft/checkedc-clang/contents/{+path}","compare_url":"https://api.github.com/repos/Microsoft/checkedc-clang/compare/{base}...{head}","merges_url":"https://api.github.com/repos/Microsoft/checkedc-clang/merges","archive_url":"https://api.github.com/repos/Microsoft/checkedc-clang/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/Microsoft/checkedc-clang/downloads","issues_url":"https://api.github.com/repos/Microsoft/checkedc-clang/issues{/number}","pulls_url":"https://api.github.com/repos/Microsoft/checkedc-clang/pulls{/number}","milestones_url":"https://api.github.com/repos/Microsoft/checkedc-clang/milestones{/number}","notifications_url":"https://api.github.com/repos/Microsoft/checkedc-clang/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/Microsoft/checkedc-clang/labels{/name}","releases_url":"https://api.github.com/repos/Microsoft/checkedc-clang/releases{/id}","deployments_url":"https://api.github.com/repos/Microsoft/checkedc-clang/deployments","created_at":"2016-06-07T20:43:32Z","updated_at":"2017-05-12T02:31:13Z","pushed_at":"2017-05-15T17:23:46Z","git_url":"git://github.com/Microsoft/checkedc-clang.git","ssh_url":"git@github.com:Microsoft/checkedc-clang.git","clone_url":"https://github.com/Microsoft/checkedc-clang.git","svn_url":"https://github.com/Microsoft/checkedc-clang","homepage":"","size":280994,"stargazers_count":118,"watchers_count":118,"language":"C++","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":16,"mirror_url":null,"open_issues_count":67,"forks":16,"open_issues":67,"watchers":118,"default_branch":"master"}},"base":{"label":"Microsoft:master","ref":"master","sha":"a8184c8e8448d5a692ffb0ec1bef2d027d2df086","user":{"login":"Microsoft","id":6154722,"avatar_url":"https://avatars1.githubusercontent.com/u/6154722?v=3","gravatar_id":"","url":"https://api.github.com/users/Microsoft","html_url":"https://github.com/Microsoft","followers_url":"https://api.github.com/users/Microsoft/followers","following_url":"https://api.github.com/users/Microsoft/following{/other_user}","gists_url":"https://api.github.com/users/Microsoft/gists{/gist_id}","starred_url":"https://api.github.com/users/Microsoft/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Microsoft/subscriptions","organizations_url":"https://api.github.com/users/Microsoft/orgs","repos_url":"https://api.github.com/users/Microsoft/repos","events_url":"https://api.github.com/users/Microsoft/events{/privacy}","received_events_url":"https://api.github.com/users/Microsoft/received_events","type":"Organization","site_admin":false},"repo":{"id":60645050,"name":"checkedc-clang","full_name":"Microsoft/checkedc-clang","owner":{"login":"Microsoft","id":6154722,"avatar_url":"https://avatars1.githubusercontent.com/u/6154722?v=3","gravatar_id":"","url":"https://api.github.com/users/Microsoft","html_url":"https://github.com/Microsoft","followers_url":"https://api.github.com/users/Microsoft/followers","following_url":"https://api.github.com/users/Microsoft/following{/other_user}","gists_url":"https://api.github.com/users/Microsoft/gists{/gist_id}","starred_url":"https://api.github.com/users/Microsoft/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Microsoft/subscriptions","organizations_url":"https://api.github.com/users/Microsoft/orgs","repos_url":"https://api.github.com/users/Microsoft/repos","events_url":"https://api.github.com/users/Microsoft/events{/privacy}","received_events_url":"https://api.github.com/users/Microsoft/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/Microsoft/checkedc-clang","description":"This repo contains a version of clang that is being modified to support Checked C.   Checked C is an extension to C that adds checking to detect or prevent common programming errors such as out-of-bounds memory accesses.","fork":false,"url":"https://api.github.com/repos/Microsoft/checkedc-clang","forks_url":"https://api.github.com/repos/Microsoft/checkedc-clang/forks","keys_url":"https://api.github.com/repos/Microsoft/checkedc-clang/keys{/key_id}","collaborators_url":"https://api.github.com/repos/Microsoft/checkedc-clang/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/Microsoft/checkedc-clang/teams","hooks_url":"https://api.github.com/repos/Microsoft/checkedc-clang/hooks","issue_events_url":"https://api.github.com/repos/Microsoft/checkedc-clang/issues/events{/number}","events_url":"https://api.github.com/repos/Microsoft/checkedc-clang/events","assignees_url":"https://api.github.com/repos/Microsoft/checkedc-clang/assignees{/user}","branches_url":"https://api.github.com/repos/Microsoft/checkedc-clang/branches{/branch}","tags_url":"https://api.github.com/repos/Microsoft/checkedc-clang/tags","blobs_url":"https://api.github.com/repos/Microsoft/checkedc-clang/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/Microsoft/checkedc-clang/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/Microsoft/checkedc-clang/git/refs{/sha}","trees_url":"https://api.github.com/repos/Microsoft/checkedc-clang/git/trees{/sha}","statuses_url":"https://api.github.com/repos/Microsoft/checkedc-clang/statuses/{sha}","languages_url":"https://api.github.com/repos/Microsoft/checkedc-clang/languages","stargazers_url":"https://api.github.com/repos/Microsoft/checkedc-clang/stargazers","contributors_url":"https://api.github.com/repos/Microsoft/checkedc-clang/contributors","subscribers_url":"https://api.github.com/repos/Microsoft/checkedc-clang/subscribers","subscription_url":"https://api.github.com/repos/Microsoft/checkedc-clang/subscription","commits_url":"https://api.github.com/repos/Microsoft/checkedc-clang/commits{/sha}","git_commits_url":"https://api.github.com/repos/Microsoft/checkedc-clang/git/commits{/sha}","comments_url":"https://api.github.com/repos/Microsoft/checkedc-clang/comments{/number}","issue_comment_url":"https://api.github.com/repos/Microsoft/checkedc-clang/issues/comments{/number}","contents_url":"https://api.github.com/repos/Microsoft/checkedc-clang/contents/{+path}","compare_url":"https://api.github.com/repos/Microsoft/checkedc-clang/compare/{base}...{head}","merges_url":"https://api.github.com/repos/Microsoft/checkedc-clang/merges","archive_url":"https://api.github.com/repos/Microsoft/checkedc-clang/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/Microsoft/checkedc-clang/downloads","issues_url":"https://api.github.com/repos/Microsoft/checkedc-clang/issues{/number}","pulls_url":"https://api.github.com/repos/Microsoft/checkedc-clang/pulls{/number}","milestones_url":"https://api.github.com/repos/Microsoft/checkedc-clang/milestones{/number}","notifications_url":"https://api.github.com/repos/Microsoft/checkedc-clang/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/Microsoft/checkedc-clang/labels{/name}","releases_url":"https://api.github.com/repos/Microsoft/checkedc-clang/releases{/id}","deployments_url":"https://api.github.com/repos/Microsoft/checkedc-clang/deployments","created_at":"2016-06-07T20:43:32Z","updated_at":"2017-05-12T02:31:13Z","pushed_at":"2017-05-15T17:23:46Z","git_url":"git://github.com/Microsoft/checkedc-clang.git","ssh_url":"git@github.com:Microsoft/checkedc-clang.git","clone_url":"https://github.com/Microsoft/checkedc-clang.git","svn_url":"https://github.com/Microsoft/checkedc-clang","homepage":"","size":280994,"stargazers_count":118,"watchers_count":118,"language":"C++","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":16,"mirror_url":null,"open_issues_count":67,"forks":16,"open_issues":67,"watchers":118,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/Microsoft/checkedc-clang/pulls/296"},"html":{"href":"https://github.com/Microsoft/checkedc-clang/pull/296"},"issue":{"href":"https://api.github.com/repos/Microsoft/checkedc-clang/issues/296"},"comments":{"href":"https://api.github.com/repos/Microsoft/checkedc-clang/issues/296/comments"},"review_comments":{"href":"https://api.github.com/repos/Microsoft/checkedc-clang/pulls/296/comments"},"review_comment":{"href":"https://api.github.com/repos/Microsoft/checkedc-clang/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/Microsoft/checkedc-clang/pulls/296/commits"},"statuses":{"href":"https://api.github.com/repos/Microsoft/checkedc-clang/statuses/08ad097d5c019d71cc52430e0f30e9491cfc5c34"}}}}
{"actor":{"display_login":"dtarditi"}}