{"action":"created","comment":{"url":"https://api.github.com/repos/mono/mono/pulls/comments/168248502","pull_request_review_id":96585290,"id":168248502,"diff_hunk":"@@ -309,6 +309,11 @@ typedef struct {\n \tunsigned int token    : 31;\n } MonoCustomMod;\n \n+typedef struct {\n+\tMonoCustomMod exported;\n+\tvoid *image;\n+} MonoCustomModInternal;","path":"mono/metadata/metadata.h","position":7,"original_position":7,"commit_id":"70ae9f7a3760c82e71814b12eaf1ca7acbf091fb","original_commit_id":"24db715ffa3118f9711c755f5668b797e99f595a","user":{"login":"kumpera","id":5738,"avatar_url":"https://avatars0.githubusercontent.com/u/5738?v=4","gravatar_id":"","url":"https://api.github.com/users/kumpera","html_url":"https://github.com/kumpera","followers_url":"https://api.github.com/users/kumpera/followers","following_url":"https://api.github.com/users/kumpera/following{/other_user}","gists_url":"https://api.github.com/users/kumpera/gists{/gist_id}","starred_url":"https://api.github.com/users/kumpera/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kumpera/subscriptions","organizations_url":"https://api.github.com/users/kumpera/orgs","repos_url":"https://api.github.com/users/kumpera/repos","events_url":"https://api.github.com/users/kumpera/events{/privacy}","received_events_url":"https://api.github.com/users/kumpera/received_events","type":"User","site_admin":false},"body":"We shouldn't store then on each cmod, but change MonoType to have an optional cmod container.\r\n\r\nIE, change MonoType to this:\r\n```c \r\nstruct _MonoType {\r\n\tunion {\r\n\t\tMonoClass *klass; /* for VALUETYPE and CLASS */\r\n\t\tMonoType *type;   /* for PTR */\r\n\t\tMonoArrayType *array; /* for ARRAY */\r\n\t\tMonoMethodSignature *method;\r\n\t\tMonoGenericParam *generic_param; /* for VAR and MVAR */\r\n\t\tMonoGenericClass *generic_class; /* for GENERICINST */\r\n\t} data;\r\n\tunsigned int attrs    : 16; /* param attributes or field flags */\r\n\tMonoTypeEnum type     : 8;\r\n\tunsigned int has_cmods : 1;  /* has a cmod appended at the end */\r\n\tunsigned int byref    : 1;\r\n\tunsigned int pinned   : 1;  /* valid when included in a local var signature */\r\n};\r\n```\r\n\r\nThen have this:\r\n```c\r\ntypedef struct CustomModContainer {\r\n   int count;\r\n   MonoImage *image;\r\n   MonoCustomMod modifiers [1]; /*the actual size of this array is count*/\r\n}\r\n```\r\n\r\nYou'd be able to fetch it using something like this:\r\n```c\r\n\r\nCustomModContainer*\r\nmono_type_get_cmods (MonoType *t) {\r\n  if (!t->has_cmods)\r\n    return null;\r\n  return (CustomModContainer*)(((char*)t) + sizeof (MonoType));\r\n\r\n}\r\n```\r\n","created_at":"2018-02-14T17:21:10Z","updated_at":"2018-02-14T17:22:16Z","html_url":"https://github.com/mono/mono/pull/7012#discussion_r168248502","pull_request_url":"https://api.github.com/repos/mono/mono/pulls/7012","author_association":"MEMBER","_links":{"self":{"href":"https://api.github.com/repos/mono/mono/pulls/comments/168248502"},"html":{"href":"https://github.com/mono/mono/pull/7012#discussion_r168248502"},"pull_request":{"href":"https://api.github.com/repos/mono/mono/pulls/7012"}},"in_reply_to_id":168022485},"pull_request":{"url":"https://api.github.com/repos/mono/mono/pulls/7012","id":168973190,"html_url":"https://github.com/mono/mono/pull/7012","diff_url":"https://github.com/mono/mono/pull/7012.diff","patch_url":"https://github.com/mono/mono/pull/7012.patch","issue_url":"https://api.github.com/repos/mono/mono/issues/7012","number":7012,"state":"open","locked":false,"title":"[runtime] Compare custom modifiers in type equality","user":{"login":"alexanderkyte","id":700530,"avatar_url":"https://avatars3.githubusercontent.com/u/700530?v=4","gravatar_id":"","url":"https://api.github.com/users/alexanderkyte","html_url":"https://github.com/alexanderkyte","followers_url":"https://api.github.com/users/alexanderkyte/followers","following_url":"https://api.github.com/users/alexanderkyte/following{/other_user}","gists_url":"https://api.github.com/users/alexanderkyte/gists{/gist_id}","starred_url":"https://api.github.com/users/alexanderkyte/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alexanderkyte/subscriptions","organizations_url":"https://api.github.com/users/alexanderkyte/orgs","repos_url":"https://api.github.com/users/alexanderkyte/repos","events_url":"https://api.github.com/users/alexanderkyte/events{/privacy}","received_events_url":"https://api.github.com/users/alexanderkyte/received_events","type":"User","site_admin":false},"body":"Custom modifiers are an odd feature of .net which allow types to\r\nbe tagged. These tags are semantically relevant. A method with a\r\nmodifier should not override a method without a modifier, for\r\ninstance (github bug #6936).\r\n\r\nWe don't put enough information into the modifier to uniquely find the\r\nImage associated with the token we have in the type. Modifiers from\r\ndifferent referring assemblies will have different tokens to refer to\r\nthe same modifier class, making it important to resolve the tokens to\r\nMonoClass pointers at runtime. Pointer equality of these classes is the\r\nway to determine two custom modifiers the same, or unique.\r\n\r\nWe cannot resolve the image just from the MonoType because it is\r\npossible to put modifiers on primitive, builtin types. These builtin\r\ntypes lack any filled-out pointers in the data block, meaning we have\r\nzero metadata associated with our floating token.\r\n\r\nOther places in the codebase fixed this by passing around MonoImages,\r\nbut adding a MonoImage argument to our type comparison function for\r\nthis single bug would be a very poor decision with much code churn all\r\nthroughout the runtime.\r\n\r\nThe bug above resolves to two I4 types, one of which has a modifier, one\r\nof which does not.\r\n\r\nIn order to pass this MonoImage around, we need to put a reference to it\r\ninto something reachable from the MonoType. We can't put it in the\r\ncustom module type because that is exported. It is part of the public\r\nAPI.\r\n\r\nMonoTypes do not adhere to pointer equality, and custom modules are structs\r\nbaked directly into the MonoType. We need therefore to make MonoType\r\npoint to a MonoCustomModInternal, a new type. This type embeds the\r\npublic type as a struct and carries around the image as another element\r\nin the struct.\r\n\r\nAll public interfaces still see the same metadata endpoints for parsing the\r\nmodules, including the icalls. It is only our type comparison code that\r\naccesses our new field.\r\n\r\n\r\n\r\n<!--\r\nThank you for your Pull Request!\r\n\r\nIf you are new to contributing to Mono, please try to do your best at conforming to our coding guidelines http://www.mono-project.com/community/contributing/coding-guidelines/ but don't worry if you get something wrong. One of the project members will help you to get things landed.\r\n\r\nDoes your pull request fix any of the existing issues? Please use the following format: Fixes #issue-number\r\n-->\r\n","created_at":"2018-02-13T22:25:11Z","updated_at":"2018-02-14T17:22:16Z","closed_at":null,"merged_at":null,"merge_commit_sha":"028e603ae5e3e1b09738d935d929e146ca5b5516","assignee":null,"assignees":[],"requested_reviewers":[{"login":"vargaz","id":186301,"avatar_url":"https://avatars3.githubusercontent.com/u/186301?v=4","gravatar_id":"","url":"https://api.github.com/users/vargaz","html_url":"https://github.com/vargaz","followers_url":"https://api.github.com/users/vargaz/followers","following_url":"https://api.github.com/users/vargaz/following{/other_user}","gists_url":"https://api.github.com/users/vargaz/gists{/gist_id}","starred_url":"https://api.github.com/users/vargaz/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/vargaz/subscriptions","organizations_url":"https://api.github.com/users/vargaz/orgs","repos_url":"https://api.github.com/users/vargaz/repos","events_url":"https://api.github.com/users/vargaz/events{/privacy}","received_events_url":"https://api.github.com/users/vargaz/received_events","type":"User","site_admin":false}],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/mono/mono/pulls/7012/commits","review_comments_url":"https://api.github.com/repos/mono/mono/pulls/7012/comments","review_comment_url":"https://api.github.com/repos/mono/mono/pulls/comments{/number}","comments_url":"https://api.github.com/repos/mono/mono/issues/7012/comments","statuses_url":"https://api.github.com/repos/mono/mono/statuses/70ae9f7a3760c82e71814b12eaf1ca7acbf091fb","head":{"label":"alexanderkyte:custommod_type_equality","ref":"custommod_type_equality","sha":"70ae9f7a3760c82e71814b12eaf1ca7acbf091fb","user":{"login":"alexanderkyte","id":700530,"avatar_url":"https://avatars3.githubusercontent.com/u/700530?v=4","gravatar_id":"","url":"https://api.github.com/users/alexanderkyte","html_url":"https://github.com/alexanderkyte","followers_url":"https://api.github.com/users/alexanderkyte/followers","following_url":"https://api.github.com/users/alexanderkyte/following{/other_user}","gists_url":"https://api.github.com/users/alexanderkyte/gists{/gist_id}","starred_url":"https://api.github.com/users/alexanderkyte/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alexanderkyte/subscriptions","organizations_url":"https://api.github.com/users/alexanderkyte/orgs","repos_url":"https://api.github.com/users/alexanderkyte/repos","events_url":"https://api.github.com/users/alexanderkyte/events{/privacy}","received_events_url":"https://api.github.com/users/alexanderkyte/received_events","type":"User","site_admin":false},"repo":{"id":30037802,"name":"mono","full_name":"alexanderkyte/mono","owner":{"login":"alexanderkyte","id":700530,"avatar_url":"https://avatars3.githubusercontent.com/u/700530?v=4","gravatar_id":"","url":"https://api.github.com/users/alexanderkyte","html_url":"https://github.com/alexanderkyte","followers_url":"https://api.github.com/users/alexanderkyte/followers","following_url":"https://api.github.com/users/alexanderkyte/following{/other_user}","gists_url":"https://api.github.com/users/alexanderkyte/gists{/gist_id}","starred_url":"https://api.github.com/users/alexanderkyte/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alexanderkyte/subscriptions","organizations_url":"https://api.github.com/users/alexanderkyte/orgs","repos_url":"https://api.github.com/users/alexanderkyte/repos","events_url":"https://api.github.com/users/alexanderkyte/events{/privacy}","received_events_url":"https://api.github.com/users/alexanderkyte/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/alexanderkyte/mono","description":"Mono open source ECMA CLI, C# and .NET implementation.","fork":true,"url":"https://api.github.com/repos/alexanderkyte/mono","forks_url":"https://api.github.com/repos/alexanderkyte/mono/forks","keys_url":"https://api.github.com/repos/alexanderkyte/mono/keys{/key_id}","collaborators_url":"https://api.github.com/repos/alexanderkyte/mono/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/alexanderkyte/mono/teams","hooks_url":"https://api.github.com/repos/alexanderkyte/mono/hooks","issue_events_url":"https://api.github.com/repos/alexanderkyte/mono/issues/events{/number}","events_url":"https://api.github.com/repos/alexanderkyte/mono/events","assignees_url":"https://api.github.com/repos/alexanderkyte/mono/assignees{/user}","branches_url":"https://api.github.com/repos/alexanderkyte/mono/branches{/branch}","tags_url":"https://api.github.com/repos/alexanderkyte/mono/tags","blobs_url":"https://api.github.com/repos/alexanderkyte/mono/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/alexanderkyte/mono/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/alexanderkyte/mono/git/refs{/sha}","trees_url":"https://api.github.com/repos/alexanderkyte/mono/git/trees{/sha}","statuses_url":"https://api.github.com/repos/alexanderkyte/mono/statuses/{sha}","languages_url":"https://api.github.com/repos/alexanderkyte/mono/languages","stargazers_url":"https://api.github.com/repos/alexanderkyte/mono/stargazers","contributors_url":"https://api.github.com/repos/alexanderkyte/mono/contributors","subscribers_url":"https://api.github.com/repos/alexanderkyte/mono/subscribers","subscription_url":"https://api.github.com/repos/alexanderkyte/mono/subscription","commits_url":"https://api.github.com/repos/alexanderkyte/mono/commits{/sha}","git_commits_url":"https://api.github.com/repos/alexanderkyte/mono/git/commits{/sha}","comments_url":"https://api.github.com/repos/alexanderkyte/mono/comments{/number}","issue_comment_url":"https://api.github.com/repos/alexanderkyte/mono/issues/comments{/number}","contents_url":"https://api.github.com/repos/alexanderkyte/mono/contents/{+path}","compare_url":"https://api.github.com/repos/alexanderkyte/mono/compare/{base}...{head}","merges_url":"https://api.github.com/repos/alexanderkyte/mono/merges","archive_url":"https://api.github.com/repos/alexanderkyte/mono/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/alexanderkyte/mono/downloads","issues_url":"https://api.github.com/repos/alexanderkyte/mono/issues{/number}","pulls_url":"https://api.github.com/repos/alexanderkyte/mono/pulls{/number}","milestones_url":"https://api.github.com/repos/alexanderkyte/mono/milestones{/number}","notifications_url":"https://api.github.com/repos/alexanderkyte/mono/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/alexanderkyte/mono/labels{/name}","releases_url":"https://api.github.com/repos/alexanderkyte/mono/releases{/id}","deployments_url":"https://api.github.com/repos/alexanderkyte/mono/deployments","created_at":"2015-01-29T19:43:22Z","updated_at":"2016-11-07T18:52:43Z","pushed_at":"2018-02-13T22:32:01Z","git_url":"git://github.com/alexanderkyte/mono.git","ssh_url":"git@github.com:alexanderkyte/mono.git","clone_url":"https://github.com/alexanderkyte/mono.git","svn_url":"https://github.com/alexanderkyte/mono","homepage":"http://www.mono-project.com/","size":519250,"stargazers_count":1,"watchers_count":1,"language":"C#","has_issues":false,"has_projects":true,"has_downloads":false,"has_wiki":false,"has_pages":false,"forks_count":1,"mirror_url":null,"archived":false,"open_issues_count":0,"license":{"key":"other","name":"Other","spdx_id":null,"url":null},"forks":1,"open_issues":0,"watchers":1,"default_branch":"master"}},"base":{"label":"mono:master","ref":"master","sha":"3fdbbe5e44b3228d4714a0a6329ecc88f9f725e0","user":{"login":"mono","id":53395,"avatar_url":"https://avatars3.githubusercontent.com/u/53395?v=4","gravatar_id":"","url":"https://api.github.com/users/mono","html_url":"https://github.com/mono","followers_url":"https://api.github.com/users/mono/followers","following_url":"https://api.github.com/users/mono/following{/other_user}","gists_url":"https://api.github.com/users/mono/gists{/gist_id}","starred_url":"https://api.github.com/users/mono/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mono/subscriptions","organizations_url":"https://api.github.com/users/mono/orgs","repos_url":"https://api.github.com/users/mono/repos","events_url":"https://api.github.com/users/mono/events{/privacy}","received_events_url":"https://api.github.com/users/mono/received_events","type":"Organization","site_admin":false},"repo":{"id":791611,"name":"mono","full_name":"mono/mono","owner":{"login":"mono","id":53395,"avatar_url":"https://avatars3.githubusercontent.com/u/53395?v=4","gravatar_id":"","url":"https://api.github.com/users/mono","html_url":"https://github.com/mono","followers_url":"https://api.github.com/users/mono/followers","following_url":"https://api.github.com/users/mono/following{/other_user}","gists_url":"https://api.github.com/users/mono/gists{/gist_id}","starred_url":"https://api.github.com/users/mono/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mono/subscriptions","organizations_url":"https://api.github.com/users/mono/orgs","repos_url":"https://api.github.com/users/mono/repos","events_url":"https://api.github.com/users/mono/events{/privacy}","received_events_url":"https://api.github.com/users/mono/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/mono/mono","description":"Mono open source ECMA CLI, C# and .NET implementation.","fork":false,"url":"https://api.github.com/repos/mono/mono","forks_url":"https://api.github.com/repos/mono/mono/forks","keys_url":"https://api.github.com/repos/mono/mono/keys{/key_id}","collaborators_url":"https://api.github.com/repos/mono/mono/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/mono/mono/teams","hooks_url":"https://api.github.com/repos/mono/mono/hooks","issue_events_url":"https://api.github.com/repos/mono/mono/issues/events{/number}","events_url":"https://api.github.com/repos/mono/mono/events","assignees_url":"https://api.github.com/repos/mono/mono/assignees{/user}","branches_url":"https://api.github.com/repos/mono/mono/branches{/branch}","tags_url":"https://api.github.com/repos/mono/mono/tags","blobs_url":"https://api.github.com/repos/mono/mono/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/mono/mono/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/mono/mono/git/refs{/sha}","trees_url":"https://api.github.com/repos/mono/mono/git/trees{/sha}","statuses_url":"https://api.github.com/repos/mono/mono/statuses/{sha}","languages_url":"https://api.github.com/repos/mono/mono/languages","stargazers_url":"https://api.github.com/repos/mono/mono/stargazers","contributors_url":"https://api.github.com/repos/mono/mono/contributors","subscribers_url":"https://api.github.com/repos/mono/mono/subscribers","subscription_url":"https://api.github.com/repos/mono/mono/subscription","commits_url":"https://api.github.com/repos/mono/mono/commits{/sha}","git_commits_url":"https://api.github.com/repos/mono/mono/git/commits{/sha}","comments_url":"https://api.github.com/repos/mono/mono/comments{/number}","issue_comment_url":"https://api.github.com/repos/mono/mono/issues/comments{/number}","contents_url":"https://api.github.com/repos/mono/mono/contents/{+path}","compare_url":"https://api.github.com/repos/mono/mono/compare/{base}...{head}","merges_url":"https://api.github.com/repos/mono/mono/merges","archive_url":"https://api.github.com/repos/mono/mono/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/mono/mono/downloads","issues_url":"https://api.github.com/repos/mono/mono/issues{/number}","pulls_url":"https://api.github.com/repos/mono/mono/pulls{/number}","milestones_url":"https://api.github.com/repos/mono/mono/milestones{/number}","notifications_url":"https://api.github.com/repos/mono/mono/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/mono/mono/labels{/name}","releases_url":"https://api.github.com/repos/mono/mono/releases{/id}","deployments_url":"https://api.github.com/repos/mono/mono/deployments","created_at":"2010-07-22T18:06:36Z","updated_at":"2018-02-14T17:21:33Z","pushed_at":"2018-02-14T17:15:07Z","git_url":"git://github.com/mono/mono.git","ssh_url":"git@github.com:mono/mono.git","clone_url":"https://github.com/mono/mono.git","svn_url":"https://github.com/mono/mono","homepage":"http://www.mono-project.com/","size":466410,"stargazers_count":6182,"watchers_count":6182,"language":"C#","has_issues":true,"has_projects":true,"has_downloads":false,"has_wiki":true,"has_pages":false,"forks_count":2786,"mirror_url":null,"archived":false,"open_issues_count":355,"license":null,"forks":2786,"open_issues":355,"watchers":6182,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/mono/mono/pulls/7012"},"html":{"href":"https://github.com/mono/mono/pull/7012"},"issue":{"href":"https://api.github.com/repos/mono/mono/issues/7012"},"comments":{"href":"https://api.github.com/repos/mono/mono/issues/7012/comments"},"review_comments":{"href":"https://api.github.com/repos/mono/mono/pulls/7012/comments"},"review_comment":{"href":"https://api.github.com/repos/mono/mono/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/mono/mono/pulls/7012/commits"},"statuses":{"href":"https://api.github.com/repos/mono/mono/statuses/70ae9f7a3760c82e71814b12eaf1ca7acbf091fb"}},"author_association":"CONTRIBUTOR"}}
{"actor":{"display_login":"kumpera"}}