[PATCH] D62470: Clarify when fix-it hints on warnings are appropriate

Mon May 27 12:21:18 PDT 2019

aaron.ballman added inline comments.

================
Comment at: docs/InternalsManual.rst:426-428
+* Fix-it hints on a warning should only clarify the meaning of the existing
+  code, not change it. Examples for such hints are added parentheses in cases
+  of counter-intuitive precedence, when they clarify the order of operations.
----------------
aaronpuchert wrote:
> aaron.ballman wrote:
> > How about:
> > ```
> > Fix-it hints on  a warning must not change the meaning of code, only clarify it. For example, adding parentheses in case of counterintuitive precedence to clarify the order of operations.
> > ```
> > must not change the meaning of code, only clarify it
> Wasn't sure about the order between “not change”/“only clarify”. If it sounds better to you, I'll take that.
> 
> > For example, adding parentheses in case of counterintuitive precedence to clarify the order of operations.
> Can we add a finite verb, like “For example, it would be fine to add parantheses ...”
> 
> Also, I would probably shorten “in case of counterintuitive precedence to clarify the order of operations” to “to clarify the precedence of operators.” So we have
> 
> //For example, it would be fine to add parantheses to clarify the precedence of operators.//
> Wasn't sure about the order between “not change”/“only clarify”. If it sounds better to you, I'll take that.

I think the most important bit is that it must not change code meaning, so that's why I suggest putting it first.

> For example, it would be fine to add parantheses to clarify the precedence of operators.

I like it! Though I'd spell it parentheses instead. ;-)

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62470/new/

https://reviews.llvm.org/D62470