{"action":"created","issue":{"url":"https://api.github.com/repos/isocpp/CppCoreGuidelines/issues/1206","repository_url":"https://api.github.com/repos/isocpp/CppCoreGuidelines","labels_url":"https://api.github.com/repos/isocpp/CppCoreGuidelines/issues/1206/labels{/name}","comments_url":"https://api.github.com/repos/isocpp/CppCoreGuidelines/issues/1206/comments","events_url":"https://api.github.com/repos/isocpp/CppCoreGuidelines/issues/1206/events","html_url":"https://github.com/isocpp/CppCoreGuidelines/issues/1206","id":318980902,"number":1206,"title":"Approach for narrow<T> is not correct","user":{"login":"dcleblanc","id":32500783,"avatar_url":"https://avatars2.githubusercontent.com/u/32500783?v=4","gravatar_id":"","url":"https://api.github.com/users/dcleblanc","html_url":"https://github.com/dcleblanc","followers_url":"https://api.github.com/users/dcleblanc/followers","following_url":"https://api.github.com/users/dcleblanc/following{/other_user}","gists_url":"https://api.github.com/users/dcleblanc/gists{/gist_id}","starred_url":"https://api.github.com/users/dcleblanc/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dcleblanc/subscriptions","organizations_url":"https://api.github.com/users/dcleblanc/orgs","repos_url":"https://api.github.com/users/dcleblanc/repos","events_url":"https://api.github.com/users/dcleblanc/events{/privacy}","received_events_url":"https://api.github.com/users/dcleblanc/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"neilmacintosh","id":10985489,"avatar_url":"https://avatars2.githubusercontent.com/u/10985489?v=4","gravatar_id":"","url":"https://api.github.com/users/neilmacintosh","html_url":"https://github.com/neilmacintosh","followers_url":"https://api.github.com/users/neilmacintosh/followers","following_url":"https://api.github.com/users/neilmacintosh/following{/other_user}","gists_url":"https://api.github.com/users/neilmacintosh/gists{/gist_id}","starred_url":"https://api.github.com/users/neilmacintosh/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/neilmacintosh/subscriptions","organizations_url":"https://api.github.com/users/neilmacintosh/orgs","repos_url":"https://api.github.com/users/neilmacintosh/repos","events_url":"https://api.github.com/users/neilmacintosh/events{/privacy}","received_events_url":"https://api.github.com/users/neilmacintosh/received_events","type":"User","site_admin":false},"assignees":[{"login":"neilmacintosh","id":10985489,"avatar_url":"https://avatars2.githubusercontent.com/u/10985489?v=4","gravatar_id":"","url":"https://api.github.com/users/neilmacintosh","html_url":"https://github.com/neilmacintosh","followers_url":"https://api.github.com/users/neilmacintosh/followers","following_url":"https://api.github.com/users/neilmacintosh/following{/other_user}","gists_url":"https://api.github.com/users/neilmacintosh/gists{/gist_id}","starred_url":"https://api.github.com/users/neilmacintosh/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/neilmacintosh/subscriptions","organizations_url":"https://api.github.com/users/neilmacintosh/orgs","repos_url":"https://api.github.com/users/neilmacintosh/repos","events_url":"https://api.github.com/users/neilmacintosh/events{/privacy}","received_events_url":"https://api.github.com/users/neilmacintosh/received_events","type":"User","site_admin":false}],"milestone":null,"comments":2,"created_at":"2018-04-30T18:03:50Z","updated_at":"2018-05-14T22:41:06Z","closed_at":null,"author_association":"NONE","body":"In section ES.49: If you must use a cast, use a named cast, there is the following statement:\r\n\r\ngsl::narrow // narrow<T>(x) is static_cast<T>(x) if static_cast<T>(x) == x or it throws narrowing_error\r\n\r\nThe following sample illustrates the problem - \r\n\r\nunsigned int x = 0xffffffff;\r\nif(static_cast< int >(x) == x) \r\n  cout << \"This is safe\";\r\n\r\nThe problem here is that in order to do the comparison, we cast the int back to unsigned int, and because we didn't change the bit pattern, this will evaluate as true. I haven't had the time today to apply this code to the SafeInt test suite (https://github.com/dcleblanc/SafeInt), but other conversions should also fail.\r\n\r\nIn addition, this would throw a false negative in the case of a cast to bool for all but inputs of 0 or 1.\r\n\r\nAnother problem in the code is this:\r\n\r\nsize_t y = something();\r\nunsigned int x = narrow<unsigned int>(y);\r\n\r\nAssuming a 32-bit int and a 32-bit memory space, this check would still have a comparison, when it should optimize away. In an x64 compile, we do need the check.\r\n\r\nAlso, there seems to be a design issue here - this seems to assume that the only problem is narrowing. Signed-unsigned mismatch can also cause problems, so while converting a negative signed short to an unsigned long may not be narrowing, it can be problematic.\r\n\r\nI'd be happy to extract the casting portion of SafeInt to fix this.\r\n"},"comment":{"url":"https://api.github.com/repos/isocpp/CppCoreGuidelines/issues/comments/388985731","html_url":"https://github.com/isocpp/CppCoreGuidelines/issues/1206#issuecomment-388985731","issue_url":"https://api.github.com/repos/isocpp/CppCoreGuidelines/issues/1206","id":388985731,"user":{"login":"gdr-at-ms","id":11031650,"avatar_url":"https://avatars1.githubusercontent.com/u/11031650?v=4","gravatar_id":"","url":"https://api.github.com/users/gdr-at-ms","html_url":"https://github.com/gdr-at-ms","followers_url":"https://api.github.com/users/gdr-at-ms/followers","following_url":"https://api.github.com/users/gdr-at-ms/following{/other_user}","gists_url":"https://api.github.com/users/gdr-at-ms/gists{/gist_id}","starred_url":"https://api.github.com/users/gdr-at-ms/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gdr-at-ms/subscriptions","organizations_url":"https://api.github.com/users/gdr-at-ms/orgs","repos_url":"https://api.github.com/users/gdr-at-ms/repos","events_url":"https://api.github.com/users/gdr-at-ms/events{/privacy}","received_events_url":"https://api.github.com/users/gdr-at-ms/received_events","type":"User","site_admin":false},"created_at":"2018-05-14T22:41:06Z","updated_at":"2018-05-14T22:41:06Z","author_association":"CONTRIBUTOR","body":"I suspect `narrow` and `narrow_cast` are the names we would like to retain. To be fair, if we need something that requires a full fledged library, maybe we would need to refine what it is we mean by 'narrow cast' :-).  The definition of narrowing (as defined by the language spec) indeed prohibits, among other things, implicit conversion 'signed T' to 'unsigned T' and 'unsigned T' to 'signed T'.\r\n\r\nWe are looking forward to your suggestion of better implementation."}}
{"actor":{"display_login":"gdr-at-ms"}}