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

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert marked 3 inline comments as done.
aaronpuchert 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.
----------------
aaron.ballman wrote:
> 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. ;-)
> Though I'd spell it parentheses instead. ;-)

There must have been a red squiggly line, but I've apparently decided to ignore it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62470