mzeren-vmw marked 2 inline comments as done.
mzeren-vmw added inline comments.


================
Comment at: lib/Format/TokenAnnotator.cpp:1756
+      if (Alignment == CA_Preprocessor)
+        (*I)->LevelOffset = 1;
     } else {
----------------
krasimir wrote:
> This feels a bit awkward: we're adding code that implicitly assumes the exact 
> style the preprocessor directives and comments around them are handled. Maybe 
> if this could become part of the level itself, it would feel less awkward.
I agree that the "long distance coupling" is awkward. Perhaps the new 
enumerator names make this a bit more palatable?


================
Comment at: lib/Format/TokenAnnotator.h:41
   AnnotatedLine(const UnwrappedLine &Line)
-      : First(Line.Tokens.front().Tok), Level(Line.Level),
+      : First(Line.Tokens.front().Tok), Level(Line.Level), LevelOffset(0),
         MatchingOpeningBlockLineIndex(Line.MatchingOpeningBlockLineIndex),
----------------
krasimir wrote:
> Is there a way to not introduce `LevelOffset`, but have it part of `Level`?
`Level` is an abstract indentation level, while `LevelOffset` is "columns". 
They have different units. Maybe it would be feasible to change the units of 
"Level" to columns in order to merge these two variables, but doing so would 
throw away information. It also seems like a much larger change. We could 
create a composite type `class AnnotatedLevel { private: unsigned Level, 
unsigned Offset public: <strongly typed operations>  }` but that seems 
over-engineered. Any other ideas?


Repository:
  rC Clang

https://reviews.llvm.org/D42036



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

Reply via email to