{"action":"closed","issue":{"url":"https://api.github.com/repos/tensorflow/tensorflow/issues/17360","repository_url":"https://api.github.com/repos/tensorflow/tensorflow","labels_url":"https://api.github.com/repos/tensorflow/tensorflow/issues/17360/labels{/name}","comments_url":"https://api.github.com/repos/tensorflow/tensorflow/issues/17360/comments","events_url":"https://api.github.com/repos/tensorflow/tensorflow/issues/17360/events","html_url":"https://github.com/tensorflow/tensorflow/issues/17360","id":301469529,"number":17360,"title":"C++ api: use of op::Attrs methods in gradients","user":{"login":"kbsriram","id":1495065,"avatar_url":"https://avatars3.githubusercontent.com/u/1495065?v=4","gravatar_id":"","url":"https://api.github.com/users/kbsriram","html_url":"https://github.com/kbsriram","followers_url":"https://api.github.com/users/kbsriram/followers","following_url":"https://api.github.com/users/kbsriram/following{/other_user}","gists_url":"https://api.github.com/users/kbsriram/gists{/gist_id}","starred_url":"https://api.github.com/users/kbsriram/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kbsriram/subscriptions","organizations_url":"https://api.github.com/users/kbsriram/orgs","repos_url":"https://api.github.com/users/kbsriram/repos","events_url":"https://api.github.com/users/kbsriram/events{/privacy}","received_events_url":"https://api.github.com/users/kbsriram/received_events","type":"User","site_admin":false},"labels":[{"id":404586594,"url":"https://api.github.com/repos/tensorflow/tensorflow/labels/stat:awaiting%20tensorflower","name":"stat:awaiting tensorflower","color":"b60205","default":false},{"id":473172988,"url":"https://api.github.com/repos/tensorflow/tensorflow/labels/type:bug/performance","name":"type:bug/performance","color":"fbca04","default":false}],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2018-03-01T16:26:37Z","updated_at":"2018-03-07T01:17:02Z","closed_at":"2018-03-07T01:17:02Z","author_association":"CONTRIBUTOR","body":"The generated op::Attrs struct returns new instances on its chainable methods, and doesn't change the original object.\r\n\r\nhttps://github.com/tensorflow/tensorflow/blob/d7d7f4eea5f3a2e63e12c803d02f56726c0e0513/tensorflow/cc/framework/cc_op_gen.cc#L701\r\n\r\nThere are a few related issues e.g. https://github.com/tensorflow/tensorflow/blob/6fdb9ad1baf7686a75f9e660178f7ac595e7fc2e/tensorflow/cc/gradients/nn_grad.cc#L164 where the code assumes the underlying object is being mutated and the parameters don't actually pass through.\r\n\r\nI guess there might be a couple of ways forward, depending on how Tensorflow prefers the C++ API:\r\n\r\n1. Decide the Attrs chaining methods mutate the underlying object and fix the code generation.\r\n2. Decide the Attrs chaining methods return new instances, and fix the uses.\r\n\r\nSuggestions?\r\n\r\nFwiw if option 2, it might be nice to add TF_MUST_USE_RESULT to the generated API. (Unfortunately a [long-standing bug in gcc](https://gcc.gnu.org/bugzilla/show_bug.cgi?id=38172) means this may be unreliable as an actual error across versions of gcc that contributors may use.)\r\n\r\n/cc @suharshs @keveman "}}
{"actor":{"display_login":"yifeif"}}