[PATCH] D110801: [clang-format] [docs] [NFC] improve clarity in the QualifierAlignment warning

[Simon Giesecke via Phabricator via cfe-commits]

simon.giesecke added inline comments.


================
Comment at: clang/include/clang/Format/Format.h:1902
+  ///  Setting ``QualifierAlignment``  to something other than `Leave`, COULD
+  ///  lead to incorrect code generation due to a lack of semantic information
+  ///  especially in the presense of macros, care should be take to review code
----------------
I don't think `incorrect code generation` is the decisive point here (or at least the wording is confusing if "code generation" refers to the output of clang-format rather than, what I would understand, the binary code generated by the compiler). Rather it's that the semantics of the source code might change. Whether that causes syntax errors, bad code generation, or remains without any visible effect is probably impossible to judge here.


================
Comment at: clang/include/clang/Format/Format.h:1903
+  ///  lead to incorrect code generation due to a lack of semantic information
+  ///  especially in the presense of macros, care should be take to review code
+  ///  changes made by this option.
----------------
curdeius wrote:
> 
As discussed around https://reviews.llvm.org/D69764#3032727, I see how macros could break this.

But what exactly does "especially" mean here, with respect to cases not involving macros?

Does it mean:
* We have some indication that this might break even without macros.
* We have no such indication but want to play safe until this has been tested in the wild.
* We can never know for sure...

At least in the last case, I'd suggest to remove the word "especially". That's true for all options of clang-format...

Similarly in the second case. That's true for all newly introduced options of clang-format.

In the first case, it makes sense to keep the word. But some more specific indication of the potential issues with and without macros seem to be helpful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110801