Remove const qualifier from function return type

Remove const qualifier from function return type

BUG=393155
Committed: https://crrev.com/815e62e824debc9193ef7a96fdc8803b2224ff99
Cr-Commit-Position: refs/heads/master@{#364629}

[tanay.c, 2015-12-08 11:35:42]

tanay.c@samsung.com changed reviewers:
+ rkc@chromium.org

[tanay.c, 2015-12-08 11:35:42]

PTAL.

[tanay.c, 2015-12-10 05:25:20]

PTAL

[rkc, 2015-12-10 21:44:48]

rkc@chromium.org changed reviewers:
+ ckehoe@chromium.org

[rkc, 2015-12-10 21:44:49]

Charlie is a more appropriate review for these. Though my opinion is that we
shouldn't be removing const correctness. If these should be const ref returns
instead, that is what we should do.

[Charlie, 2015-12-11 00:48:47]

On 2015/12/10 21:44:49, Rahul Chaturvedi wrote:
> Charlie is a more appropriate review for these. Though my opinion is that we
> shouldn't be removing const correctness. If these should be const ref returns
> instead, that is what we should do.

Since these are not references, there is no issue of const correctness. The only
issue would be one of object copying and turnover, but the compiler should be
smart enough to optimize this correctly. I agree that the const return types are
probably overkill.

LGTM

[tanay.c, 2015-12-11 05:27:41]

The CQ bit was checked by tanay.c@samsung.com

[commit-bot: I haz the power, 2015-12-11 05:28:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1507893005/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1507893005/20001

[commit-bot: I haz the power, 2015-12-11 06:15:46]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-12-11 06:16:58]

Description was changed from

==========
Remove const qualifier from function return type

BUG=393155
==========

to

==========
Remove const qualifier from function return type

BUG=393155

Committed: https://crrev.com/815e62e824debc9193ef7a96fdc8803b2224ff99
Cr-Commit-Position: refs/heads/master@{#364629}
==========

[commit-bot: I haz the power, 2015-12-11 06:16:58]

Patchset 2 (id:??) landed as
https://crrev.com/815e62e824debc9193ef7a96fdc8803b2224ff99
Cr-Commit-Position: refs/heads/master@{#364629}

[vabr (Chromium), 2015-12-11 09:28:22]

On 2015/12/11 06:16:58, commit-bot: I haz the power wrote:
> Patchset 2 (id:??) landed as
> https://crrev.com/815e62e824debc9193ef7a96fdc8803b2224ff99
> Cr-Commit-Position: refs/heads/master@{#364629}

Note that this change is not beneficial: http://crbug.com/393155#c4. :(

[tanay.c, 2015-12-11 09:33:20]

On 2015/12/11 09:28:22, vabr (Chromium) wrote:
> On 2015/12/11 06:16:58, commit-bot: I haz the power wrote:
> > Patchset 2 (id:??) landed as
> > https://crrev.com/815e62e824debc9193ef7a96fdc8803b2224ff99
> > Cr-Commit-Position: refs/heads/master@{#364629}
> 
> Note that this change is not beneficial: http://crbug.com/393155#c4. :(

Just had a look at the previous comments regarding particular conversions of
|const std::string| to |std::string|. I agree with comment #4 , it might be more
appropriate to maintain the |const| to enable the compiler to throw error even
for non user defined return types.  However, AFAIK the removal of const does not
add any risk since the values are being copied and there are no references
involved and can be removed for a cleaner code. That being said, I will keep
this in mind for any further conversions of this type.

[vabr (Chromium), 2015-12-11 10:58:03]

On 2015/12/11 09:33:20, tanay.c wrote:
> On 2015/12/11 09:28:22, vabr (Chromium) wrote:
> > On 2015/12/11 06:16:58, commit-bot: I haz the power wrote:
> > > Patchset 2 (id:??) landed as
> > > https://crrev.com/815e62e824debc9193ef7a96fdc8803b2224ff99
> > > Cr-Commit-Position: refs/heads/master@{#364629}
> > 
> > Note that this change is not beneficial: http://crbug.com/393155#c4. :(
> 
> Just had a look at the previous comments regarding particular conversions of
> |const std::string| to |std::string|. I agree with comment #4 , it might be
more
> appropriate to maintain the |const| to enable the compiler to throw error even
> for non user defined return types.  However, AFAIK the removal of const does
not
> add any risk since the values are being copied and there are no references
> involved and can be removed for a cleaner code. That being said, I will keep
> this in mind for any further conversions of this type.

Thank you.

Indeed, your change does not create a bad code by itself. But it makes it easier
for other people to introduce errors:
foo->StringValue() = "asdf";  // While not corrupting memory, this code does not
do what it looks like doing (altering the "string value" of foo).