GitHub PullRequestReviewCommentEvent at 2014-12-16 22:55:19
Repository: chapel
Actor: lydia-duncan
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/chapel-lang/chapel/pulls/comments/21939614","id":21939614,"diff_hunk":"@@ -1030,7 +1030,35 @@ static void init_typed_var(VarSymbol* var, Expr* type, Expr* init, Expr* stmt, V\n     // the initialization expression if it exists\n     //\n     bool isNoinit = init && init->isNoInitExpr();\n-    if (isNoinit) {\n+","path":"compiler/passes/normalize.cpp","position":5,"original_position":5,"commit_id":"46ef6882e635f133f122d224dc24e9d3e5f92081","original_commit_id":"46ef6882e635f133f122d224dc24e9d3e5f92081","user":{"login":"lydia-duncan","id":2454710,"avatar_url":"https://avatars.githubusercontent.com/u/2454710?v=3","gravatar_id":"","url":"https://api.github.com/users/lydia-duncan","html_url":"https://github.com/lydia-duncan","followers_url":"https://api.github.com/users/lydia-duncan/followers","following_url":"https://api.github.com/users/lydia-duncan/following{/other_user}","gists_url":"https://api.github.com/users/lydia-duncan/gists{/gist_id}","starred_url":"https://api.github.com/users/lydia-duncan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lydia-duncan/subscriptions","organizations_url":"https://api.github.com/users/lydia-duncan/orgs","repos_url":"https://api.github.com/users/lydia-duncan/repos","events_url":"https://api.github.com/users/lydia-duncan/events{/privacy}","received_events_url":"https://api.github.com/users/lydia-duncan/received_events","type":"User","site_admin":false},"body":"Strings themselves may skip noinit, but the default value of strings was moved into module code specifically in preparation for the implementation of strings as records living in module code.","created_at":"2014-12-16T22:55:19Z","updated_at":"2014-12-16T22:55:19Z","html_url":"https://github.com/chapel-lang/chapel/pull/1017#discussion_r21939614","pull_request_url":"https://api.github.com/repos/chapel-lang/chapel/pulls/1017","_links":{"self":{"href":"https://api.github.com/repos/chapel-lang/chapel/pulls/comments/21939614"},"html":{"href":"https://github.com/chapel-lang/chapel/pull/1017#discussion_r21939614"},"pull_request":{"href":"https://api.github.com/repos/chapel-lang/chapel/pulls/1017"}}},"pull_request":{"url":"https://api.github.com/repos/chapel-lang/chapel/pulls/1017","id":26183423,"html_url":"https://github.com/chapel-lang/chapel/pull/1017","diff_url":"https://github.com/chapel-lang/chapel/pull/1017.diff","patch_url":"https://github.com/chapel-lang/chapel/pull/1017.patch","issue_url":"https://api.github.com/repos/chapel-lang/chapel/issues/1017","number":1017,"state":"open","locked":false,"title":"Add flag to ignore/enable noinit","user":{"login":"lydia-duncan","id":2454710,"avatar_url":"https://avatars.githubusercontent.com/u/2454710?v=3","gravatar_id":"","url":"https://api.github.com/users/lydia-duncan","html_url":"https://github.com/lydia-duncan","followers_url":"https://api.github.com/users/lydia-duncan/followers","following_url":"https://api.github.com/users/lydia-duncan/following{/other_user}","gists_url":"https://api.github.com/users/lydia-duncan/gists{/gist_id}","starred_url":"https://api.github.com/users/lydia-duncan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lydia-duncan/subscriptions","organizations_url":"https://api.github.com/users/lydia-duncan/orgs","repos_url":"https://api.github.com/users/lydia-duncan/repos","events_url":"https://api.github.com/users/lydia-duncan/events{/privacy}","received_events_url":"https://api.github.com/users/lydia-duncan/received_events","type":"User","site_admin":false},"body":"Adds the flag --[no-]use-noinit to the language, with a default of --use-noinit.\r\nThis allows the user to control whether the use of noinit within their code\r\n(and module code) is honored at compile time, allowing them to easily determine\r\nthe impact noinit has on the performance of their program.\r\n\r\nImplementation details:\r\n\r\n--use-noinit controls the insertion of the primitive PRIM_NO_INIT in normalize\r\nfor user code.  I could have altered the the functionResolution behavior in\r\nresponse to noinit, but that was more complicated (and also would have resulted\r\nin the unnecessary insertion and replacement of PRIM_NO_INIT wherever it was\r\notherwise needed).  Thus, I felt that making the switch during normalize was\r\nthe better course of action.\r\n\r\nNoinit must still be used with return temps, because our default initialization\r\nlives in the modules - default initializing the return temps in the default\r\ninitialization functions (soon to be methods) would cause infinite recursion to\r\noccur.  Thus, --no-use-noinit does not affect return temps.  This should not be\r\na detail that is apparent to the user, as it is specific to our implementation.\r\nIf we do encounter an error related to this, then I will have a bug to fix.\r\nSimilarly, noinit must still be used within our internal and standard modules.\r\nThere are some _defaultOf functions which must define an instance of\r\nthat type in order to initialize it.  Obviously these cases need to skip default\r\ninitialization for that instance or infinite recursion will occur.  Our\r\nintention moving forward is that types will define their default through the use\r\nof constructors, which would remove the need for noinit in these cases, but\r\nsince we are not there yet, it is necessary to ignore --no-use-noinit in the\r\nprovided module code.  While a bit heavy-handed, this is a reasonable approach\r\nto take because the internal modules and standard modules should be considered\r\nan implementation detail - steps taken to ensure their operability in all\r\ncircumstances should not be breakable by user code and compilation actions, or\r\nour implementation would be incorrect.\r\n\r\nAt this time, there is no use of noinit in the module code solely for\r\nperformance (though as an implementation detail, it would not be unreasonable to\r\nensure noinit was always honored for that use).\r\n\r\nTesting details:\r\nUpdates the test on our -h flag to include the information about the new flag.\r\nAdds a test directory devoted to use of the flag --no-use-noinit, and a case to\r\nverify --use-noinit behaves the same as the default behavior (which we skip when\r\nthe --no-use-noinit flag is thrown for obvious reasons)\r\n\r\nI ran std/ and std + --no-use-noinit.  The former case passed without a hitch.  The\r\nlatter had some hiccups, which resulted in varying solutions.  My noinit test\r\ndirectory is skipped, as the output will be different. (and the important cases\r\nare covered in the compflags directory).  The only remaining failures are\r\nexecflags/sungeun/about (the test requires that no flags be passed to the\r\nfunction, we should really make that skipif more general), whose skipif I did\r\nnot alter because it consisted on modes we run nightly; and some tests\r\nrelated to the redefinition of _defaultOf by the user\r\n(functions/hilde/defaultOf/precedence and\r\nfunctions/lydia/userDefinedDefaultFunction/*) - the behavior these tests are\r\nverifying will change when we fully implement our constructor story, and I\r\nbelieve the final constructor story would allow them to use the flag\r\n--no-use-noinit without error.  If it does not, further investigation should still\r\nbe made, so skipping them would hide that necessity.","created_at":"2014-12-16T21:58:23Z","updated_at":"2014-12-16T22:55:19Z","closed_at":null,"merged_at":null,"merge_commit_sha":"7f1d714e7d82ce99633c788bcf38e156d8753817","assignee":{"login":"lydia-duncan","id":2454710,"avatar_url":"https://avatars.githubusercontent.com/u/2454710?v=3","gravatar_id":"","url":"https://api.github.com/users/lydia-duncan","html_url":"https://github.com/lydia-duncan","followers_url":"https://api.github.com/users/lydia-duncan/followers","following_url":"https://api.github.com/users/lydia-duncan/following{/other_user}","gists_url":"https://api.github.com/users/lydia-duncan/gists{/gist_id}","starred_url":"https://api.github.com/users/lydia-duncan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lydia-duncan/subscriptions","organizations_url":"https://api.github.com/users/lydia-duncan/orgs","repos_url":"https://api.github.com/users/lydia-duncan/repos","events_url":"https://api.github.com/users/lydia-duncan/events{/privacy}","received_events_url":"https://api.github.com/users/lydia-duncan/received_events","type":"User","site_admin":false},"milestone":null,"commits_url":"https://api.github.com/repos/chapel-lang/chapel/pulls/1017/commits","review_comments_url":"https://api.github.com/repos/chapel-lang/chapel/pulls/1017/comments","review_comment_url":"https://api.github.com/repos/chapel-lang/chapel/pulls/comments/{number}","comments_url":"https://api.github.com/repos/chapel-lang/chapel/issues/1017/comments","statuses_url":"https://api.github.com/repos/chapel-lang/chapel/statuses/46ef6882e635f133f122d224dc24e9d3e5f92081","head":{"label":"lydia-duncan:addIgnoreNoinitFlag","ref":"addIgnoreNoinitFlag","sha":"46ef6882e635f133f122d224dc24e9d3e5f92081","user":{"login":"lydia-duncan","id":2454710,"avatar_url":"https://avatars.githubusercontent.com/u/2454710?v=3","gravatar_id":"","url":"https://api.github.com/users/lydia-duncan","html_url":"https://github.com/lydia-duncan","followers_url":"https://api.github.com/users/lydia-duncan/followers","following_url":"https://api.github.com/users/lydia-duncan/following{/other_user}","gists_url":"https://api.github.com/users/lydia-duncan/gists{/gist_id}","starred_url":"https://api.github.com/users/lydia-duncan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lydia-duncan/subscriptions","organizations_url":"https://api.github.com/users/lydia-duncan/orgs","repos_url":"https://api.github.com/users/lydia-duncan/repos","events_url":"https://api.github.com/users/lydia-duncan/events{/privacy}","received_events_url":"https://api.github.com/users/lydia-duncan/received_events","type":"User","site_admin":false},"repo":{"id":21914939,"name":"chapel","full_name":"lydia-duncan/chapel","owner":{"login":"lydia-duncan","id":2454710,"avatar_url":"https://avatars.githubusercontent.com/u/2454710?v=3","gravatar_id":"","url":"https://api.github.com/users/lydia-duncan","html_url":"https://github.com/lydia-duncan","followers_url":"https://api.github.com/users/lydia-duncan/followers","following_url":"https://api.github.com/users/lydia-duncan/following{/other_user}","gists_url":"https://api.github.com/users/lydia-duncan/gists{/gist_id}","starred_url":"https://api.github.com/users/lydia-duncan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lydia-duncan/subscriptions","organizations_url":"https://api.github.com/users/lydia-duncan/orgs","repos_url":"https://api.github.com/users/lydia-duncan/repos","events_url":"https://api.github.com/users/lydia-duncan/events{/privacy}","received_events_url":"https://api.github.com/users/lydia-duncan/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/lydia-duncan/chapel","description":"a Productive Parallel Programming Language","fork":true,"url":"https://api.github.com/repos/lydia-duncan/chapel","forks_url":"https://api.github.com/repos/lydia-duncan/chapel/forks","keys_url":"https://api.github.com/repos/lydia-duncan/chapel/keys{/key_id}","collaborators_url":"https://api.github.com/repos/lydia-duncan/chapel/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/lydia-duncan/chapel/teams","hooks_url":"https://api.github.com/repos/lydia-duncan/chapel/hooks","issue_events_url":"https://api.github.com/repos/lydia-duncan/chapel/issues/events{/number}","events_url":"https://api.github.com/repos/lydia-duncan/chapel/events","assignees_url":"https://api.github.com/repos/lydia-duncan/chapel/assignees{/user}","branches_url":"https://api.github.com/repos/lydia-duncan/chapel/branches{/branch}","tags_url":"https://api.github.com/repos/lydia-duncan/chapel/tags","blobs_url":"https://api.github.com/repos/lydia-duncan/chapel/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/lydia-duncan/chapel/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/lydia-duncan/chapel/git/refs{/sha}","trees_url":"https://api.github.com/repos/lydia-duncan/chapel/git/trees{/sha}","statuses_url":"https://api.github.com/repos/lydia-duncan/chapel/statuses/{sha}","languages_url":"https://api.github.com/repos/lydia-duncan/chapel/languages","stargazers_url":"https://api.github.com/repos/lydia-duncan/chapel/stargazers","contributors_url":"https://api.github.com/repos/lydia-duncan/chapel/contributors","subscribers_url":"https://api.github.com/repos/lydia-duncan/chapel/subscribers","subscription_url":"https://api.github.com/repos/lydia-duncan/chapel/subscription","commits_url":"https://api.github.com/repos/lydia-duncan/chapel/commits{/sha}","git_commits_url":"https://api.github.com/repos/lydia-duncan/chapel/git/commits{/sha}","comments_url":"https://api.github.com/repos/lydia-duncan/chapel/comments{/number}","issue_comment_url":"https://api.github.com/repos/lydia-duncan/chapel/issues/comments/{number}","contents_url":"https://api.github.com/repos/lydia-duncan/chapel/contents/{+path}","compare_url":"https://api.github.com/repos/lydia-duncan/chapel/compare/{base}...{head}","merges_url":"https://api.github.com/repos/lydia-duncan/chapel/merges","archive_url":"https://api.github.com/repos/lydia-duncan/chapel/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/lydia-duncan/chapel/downloads","issues_url":"https://api.github.com/repos/lydia-duncan/chapel/issues{/number}","pulls_url":"https://api.github.com/repos/lydia-duncan/chapel/pulls{/number}","milestones_url":"https://api.github.com/repos/lydia-duncan/chapel/milestones{/number}","notifications_url":"https://api.github.com/repos/lydia-duncan/chapel/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/lydia-duncan/chapel/labels{/name}","releases_url":"https://api.github.com/repos/lydia-duncan/chapel/releases{/id}","created_at":"2014-07-16T19:55:13Z","updated_at":"2014-12-16T16:33:45Z","pushed_at":"2014-12-16T18:33:40Z","git_url":"git://github.com/lydia-duncan/chapel.git","ssh_url":"git@github.com:lydia-duncan/chapel.git","clone_url":"https://github.com/lydia-duncan/chapel.git","svn_url":"https://github.com/lydia-duncan/chapel","homepage":"http://chapel.cray.com/","size":247742,"stargazers_count":1,"watchers_count":1,"language":"Chapel","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":1,"default_branch":"master"}},"base":{"label":"chapel-lang:master","ref":"master","sha":"e935d49e75b4db2268e49adb526bd815ae73c0a3","user":{"login":"chapel-lang","id":7597261,"avatar_url":"https://avatars.githubusercontent.com/u/7597261?v=3","gravatar_id":"","url":"https://api.github.com/users/chapel-lang","html_url":"https://github.com/chapel-lang","followers_url":"https://api.github.com/users/chapel-lang/followers","following_url":"https://api.github.com/users/chapel-lang/following{/other_user}","gists_url":"https://api.github.com/users/chapel-lang/gists{/gist_id}","starred_url":"https://api.github.com/users/chapel-lang/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chapel-lang/subscriptions","organizations_url":"https://api.github.com/users/chapel-lang/orgs","repos_url":"https://api.github.com/users/chapel-lang/repos","events_url":"https://api.github.com/users/chapel-lang/events{/privacy}","received_events_url":"https://api.github.com/users/chapel-lang/received_events","type":"Organization","site_admin":false},"repo":{"id":21663285,"name":"chapel","full_name":"chapel-lang/chapel","owner":{"login":"chapel-lang","id":7597261,"avatar_url":"https://avatars.githubusercontent.com/u/7597261?v=3","gravatar_id":"","url":"https://api.github.com/users/chapel-lang","html_url":"https://github.com/chapel-lang","followers_url":"https://api.github.com/users/chapel-lang/followers","following_url":"https://api.github.com/users/chapel-lang/following{/other_user}","gists_url":"https://api.github.com/users/chapel-lang/gists{/gist_id}","starred_url":"https://api.github.com/users/chapel-lang/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chapel-lang/subscriptions","organizations_url":"https://api.github.com/users/chapel-lang/orgs","repos_url":"https://api.github.com/users/chapel-lang/repos","events_url":"https://api.github.com/users/chapel-lang/events{/privacy}","received_events_url":"https://api.github.com/users/chapel-lang/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/chapel-lang/chapel","description":"a Productive Parallel Programming Language","fork":false,"url":"https://api.github.com/repos/chapel-lang/chapel","forks_url":"https://api.github.com/repos/chapel-lang/chapel/forks","keys_url":"https://api.github.com/repos/chapel-lang/chapel/keys{/key_id}","collaborators_url":"https://api.github.com/repos/chapel-lang/chapel/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/chapel-lang/chapel/teams","hooks_url":"https://api.github.com/repos/chapel-lang/chapel/hooks","issue_events_url":"https://api.github.com/repos/chapel-lang/chapel/issues/events{/number}","events_url":"https://api.github.com/repos/chapel-lang/chapel/events","assignees_url":"https://api.github.com/repos/chapel-lang/chapel/assignees{/user}","branches_url":"https://api.github.com/repos/chapel-lang/chapel/branches{/branch}","tags_url":"https://api.github.com/repos/chapel-lang/chapel/tags","blobs_url":"https://api.github.com/repos/chapel-lang/chapel/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/chapel-lang/chapel/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/chapel-lang/chapel/git/refs{/sha}","trees_url":"https://api.github.com/repos/chapel-lang/chapel/git/trees{/sha}","statuses_url":"https://api.github.com/repos/chapel-lang/chapel/statuses/{sha}","languages_url":"https://api.github.com/repos/chapel-lang/chapel/languages","stargazers_url":"https://api.github.com/repos/chapel-lang/chapel/stargazers","contributors_url":"https://api.github.com/repos/chapel-lang/chapel/contributors","subscribers_url":"https://api.github.com/repos/chapel-lang/chapel/subscribers","subscription_url":"https://api.github.com/repos/chapel-lang/chapel/subscription","commits_url":"https://api.github.com/repos/chapel-lang/chapel/commits{/sha}","git_commits_url":"https://api.github.com/repos/chapel-lang/chapel/git/commits{/sha}","comments_url":"https://api.github.com/repos/chapel-lang/chapel/comments{/number}","issue_comment_url":"https://api.github.com/repos/chapel-lang/chapel/issues/comments/{number}","contents_url":"https://api.github.com/repos/chapel-lang/chapel/contents/{+path}","compare_url":"https://api.github.com/repos/chapel-lang/chapel/compare/{base}...{head}","merges_url":"https://api.github.com/repos/chapel-lang/chapel/merges","archive_url":"https://api.github.com/repos/chapel-lang/chapel/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/chapel-lang/chapel/downloads","issues_url":"https://api.github.com/repos/chapel-lang/chapel/issues{/number}","pulls_url":"https://api.github.com/repos/chapel-lang/chapel/pulls{/number}","milestones_url":"https://api.github.com/repos/chapel-lang/chapel/milestones{/number}","notifications_url":"https://api.github.com/repos/chapel-lang/chapel/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/chapel-lang/chapel/labels{/name}","releases_url":"https://api.github.com/repos/chapel-lang/chapel/releases{/id}","created_at":"2014-07-09T18:15:54Z","updated_at":"2014-12-16T18:22:52Z","pushed_at":"2014-12-16T18:22:52Z","git_url":"git://github.com/chapel-lang/chapel.git","ssh_url":"git@github.com:chapel-lang/chapel.git","clone_url":"https://github.com/chapel-lang/chapel.git","svn_url":"https://github.com/chapel-lang/chapel","homepage":"http://chapel.cray.com/","size":324900,"stargazers_count":53,"watchers_count":53,"language":"Chapel","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":32,"mirror_url":null,"open_issues_count":11,"forks":32,"open_issues":11,"watchers":53,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/chapel-lang/chapel/pulls/1017"},"html":{"href":"https://github.com/chapel-lang/chapel/pull/1017"},"issue":{"href":"https://api.github.com/repos/chapel-lang/chapel/issues/1017"},"comments":{"href":"https://api.github.com/repos/chapel-lang/chapel/issues/1017/comments"},"review_comments":{"href":"https://api.github.com/repos/chapel-lang/chapel/pulls/1017/comments"},"review_comment":{"href":"https://api.github.com/repos/chapel-lang/chapel/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/chapel-lang/chapel/pulls/1017/commits"},"statuses":{"href":"https://api.github.com/repos/chapel-lang/chapel/statuses/46ef6882e635f133f122d224dc24e9d3e5f92081"}}}}