Page MenuHomePhabricator

[YAMLParser] Add multi-line literal folding support
Needs ReviewPublic

Authored by HassanElDesouky on Sun, May 16, 7:57 PM.

Details

Reviewers
njames93
silvas
Summary

Last year I was working at Swift to add support for Localization of Compiler Diagnostic Messages. We are currently using YAML as the new diagnostic format. The LLVM::YAMLParser didn't have a support for multiline string literal folding and it's crucial to have that for the diagnostic message to help us keep up with the 80 columns rule. Therefore, I decided to add a multiline string literal folding support to the YAML parser.

Diff Detail

Event Timeline

HassanElDesouky requested review of this revision.Sun, May 16, 7:57 PM
Herald added a project: Restricted Project. · View Herald TranscriptSun, May 16, 7:57 PM

Fixed clang-tidy warning

Not too deep into the YAML world, so just my two cents re the isLineBreak function.

llvm/lib/Support/YAMLParser.cpp
1049

shouldn't this rather be a check whether the line is _empty_ -> whether it contains anything else than \n\r\t ?
Currently, this does not support empty lines with spaces, multiple tabs or windows style line feeds \r\n.

This would be easily solvable by using something like:

for(auto Position = Line.begin(); Position != Line.end(); ++Position)
  if(!isBlankOrBreak(Position))
     return false;
return true;

Renaming the function to isLineEmpty might sense then, too.

HassanElDesouky marked an inline comment as done.Wed, Jun 2, 11:00 PM

Not too deep into the YAML world, so just my two cents re the isLineBreak function.

Thank you for the review! I changed the function as suggested.