[PATCH] [lld] enable mach-o and native yaml to be intermixed

Rui Ueyama ruiu at google.com
Thu Jan 9 20:20:02 PST 2014



================
Comment at: include/lld/ReaderWriter/Reader.h:25
@@ +24,3 @@
+namespace llvm {
+  namespace yaml {
+    class IO;
----------------
kledzik at apple.com wrote:
> Rui Ueyama wrote:
> > Don't indent namespaces.
> The coding guide says to indent namespaces if they are small.
OK.

================
Comment at: lib/ReaderWriter/MachO/File.h:24
@@ -23,2 +23,3 @@
 
-  void addDefinedAtom(StringRef name, ArrayRef<uint8_t> content) {
+  void addDefinedAtom(StringRef name, ArrayRef<uint8_t> content, bool copyRefs){
+    if (copyRefs) {
----------------
kledzik at apple.com wrote:
> Rui Ueyama wrote:
> > nit: need a space before "{"
> But that would push the line of 80 columns.  This seems more readable than moving the parameter to a new line.
I'd still want to stick to the coding standard, as the benefit of having a coding standard is IMO to avoid discussing about what is better coding style once and for all. In that sense the rule should be applied mechanically rather than cherry picking case by case. The coding standard says that the line should be shorter than 80 cols, and a space is needed before "{", so we don't have any way other than adding a new line after the comma.

================
Comment at: include/lld/ReaderWriter/YamlContext.h:38
@@ -37,5 +37,3 @@
 
-  const LinkingContext *_linkingContext;
-  const Registry *_registry;
-  File *_file;
-  NormalizedFile *_normalizeMachOFile;
+  const LinkingContext   *_linkingContext;
+  const Registry         *_registry;
----------------
kledzik at apple.com wrote:
> Joey Gouly wrote:
> > I wish we could stick with the original code for this. It's what clang-format produces, and while it might be a little harder to skim down and see the types and names of class members, I think it's worth the consistent style.
> > 
> > Nick, I know you like this style, but are you willing to try convert to clang-format style? (You don't even have to format your code, it's brilliant!)
> I think of clang-format as a great tool for the first-pass, bulk cleanup of code.  But there are still lots of areas where a human would format it differently to be even more readable.  That is why I don't like to run clang-format on code that is already hand tuned, because it removes lots of the hand improvements.  I'll write some bugs on clang-format for some of the cases I care about and see what it can do.
I agree with Joey because as the previous comment it's our coding standard. The other reason is this: I think running clang-format on a new patch is generally a good thing, but indenting this code snippet this way prevents other people to do that, because if one does that, only the modified lines would have only one space between type and field name.


http://llvm-reviews.chandlerc.com/D2529

BRANCH
  svn

ARCANIST PROJECT
  lld



More information about the llvm-commits mailing list