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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to