[PATCH] [WIP] clang-format: Enforce EOL at EOF as required by C standard

Alp Toker alp at nuanti.com
Sun Oct 6 05:10:51 PDT 2013


Hi,

I'm just putting this patch out there for now as it's been useful to us
in its current state. Some tweaks are needed before this can land:

1) It needs a lot of existing unit tests to be fixed. The tests could be
fixed before this patch goes in if anyone has the time to volunteer(!)

2) I'm not convinced it's worth even having a 'AllowNoEOL' format flag
for this. The spec is clear that it's mandatory and many VCS also expect
EOL at EOF to work properly.

3) That means we might do better to add a command line flag to identify
that the code is being formatted for snippets, which uses the
forSnippets() internally, in which case AllowNoEOL would be made true.

4) There are other rules such as trailing slash in 5.5.1.2 that I'm not
sure are handled yet, though these could be worked on subequent to this
patch landing.

Note also that this fixes a bug in clang-format where it was reporting
an error code for an empty (zero-sized) file. There's no benefit to the
special case so we can just remove that check now.

Here's the change:

    clang-format: Enforce EOL at EOF as required by C standard
   
    This enables end-of-line correction as specified for C-family languages.
   
    C11 5.5.1.2p2 "A source file that is not empty shall end in a new-line
    character"
   
    The new setting 'AllowNoEOL' and the fluent
FormatStyle::forSnippets() helper
    are provided for consumers that prefer the old behaviour, which is
preferable
    for inline code snippets in documentation.

Alp.

-- 
http://www.nuanti.com
the browser experts

-------------- next part --------------
commit 885c9f7c64b7c989d22a4f4b39c9df588914f0e4
Author: Alp Toker <alp at nuanti.com>
Date:   Sun Oct 6 12:31:28 2013 +0100

    clang-format: Enforce EOL at EOF as required by C standard
    
    This enables end-of-line correction as specified for C-family languages.
    
    C11 5.5.1.2p2 "A source file that is not empty shall end in a new-line
    character"
    
    The new setting 'AllowNoEOL' and the fluent FormatStyle::forSnippets() helper
    are provided for consumers that prefer the old behaviour, which is preferable
    for inline code snippets in documentation.

diff --git a/include/clang/Format/Format.h b/include/clang/Format/Format.h
index 899087a..20363d8 100644
--- a/include/clang/Format/Format.h
+++ b/include/clang/Format/Format.h
@@ -235,6 +235,17 @@ struct FormatStyle {
   /// \brief If \c false, spaces will be removed before assignment operators.
   bool SpaceBeforeAssignmentOperators;
 
+  /// \brief If \c false, ensure that non-empty sources end in a new-line
+  /// character as required by C-family languages.
+  bool AllowNoEOL;
+
+  /// \brief Adjust the configuration to format single-line code chunks.
+  ///
+  /// The updated format will not enforce trailing EOL at EOF and may also
+  /// apply other adjustments tailored for short inline code snippets appearing
+  /// in technical documentation.
+  FormatStyle forSnippets() const;
+
   bool operator==(const FormatStyle &R) const {
     return AccessModifierOffset == R.AccessModifierOffset &&
            ConstructorInitializerIndentWidth ==
@@ -282,7 +293,8 @@ struct FormatStyle {
            SpacesInCStyleCastParentheses == R.SpacesInCStyleCastParentheses &&
            SpaceAfterControlStatementKeyword ==
                R.SpaceAfterControlStatementKeyword &&
-           SpaceBeforeAssignmentOperators == R.SpaceBeforeAssignmentOperators;
+           SpaceBeforeAssignmentOperators == R.SpaceBeforeAssignmentOperators &&
+           AllowNoEOL == R.AllowNoEOL;
   }
 };
 
diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp
index 8ee770a..4a6f855 100644
--- a/lib/Format/Format.cpp
+++ b/lib/Format/Format.cpp
@@ -163,6 +163,7 @@ template <> struct MappingTraits<clang::format::FormatStyle> {
                    Style.SpaceAfterControlStatementKeyword);
     IO.mapOptional("SpaceBeforeAssignmentOperators",
                    Style.SpaceBeforeAssignmentOperators);
+    IO.mapOptional("AllowNoEOL", Style.AllowNoEOL);
   }
 };
 }
@@ -214,6 +215,7 @@ FormatStyle getLLVMStyle() {
   LLVMStyle.SpacesInCStyleCastParentheses = false;
   LLVMStyle.SpaceAfterControlStatementKeyword = true;
   LLVMStyle.SpaceBeforeAssignmentOperators = true;
+  LLVMStyle.AllowNoEOL = false;
 
   setDefaultPenalties(LLVMStyle);
   LLVMStyle.PenaltyReturnTypeOnItsOwnLine = 60;
@@ -257,6 +259,7 @@ FormatStyle getGoogleStyle() {
   GoogleStyle.SpacesInCStyleCastParentheses = false;
   GoogleStyle.SpaceAfterControlStatementKeyword = true;
   GoogleStyle.SpaceBeforeAssignmentOperators = true;
+  GoogleStyle.AllowNoEOL = false;
 
   setDefaultPenalties(GoogleStyle);
   GoogleStyle.PenaltyReturnTypeOnItsOwnLine = 200;
@@ -318,6 +321,12 @@ bool getPredefinedStyle(StringRef Name, FormatStyle *Style) {
   return true;
 }
 
+FormatStyle FormatStyle::forSnippets() const {
+  FormatStyle Style = *this;
+  Style.AllowNoEOL = true;
+  return Style;
+}
+
 llvm::error_code parseConfiguration(StringRef Text, FormatStyle *Style) {
   if (Text.trim().empty())
     return llvm::make_error_code(llvm::errc::invalid_argument);
@@ -860,7 +869,11 @@ public:
       bool WasMoved = PreviousLineWasTouched && FirstTok->NewlinesBefore == 0;
       if (TheLine.First->is(tok::eof)) {
         if (PreviousLineWasTouched) {
-          unsigned NewLines = std::min(FirstTok->NewlinesBefore, 1u);
+          // C11 5.5.1.2p2 "A source file that is not empty shall end in a
+          // new-line character"
+          unsigned NewLines = WasMoved && !Style.AllowNoEOL
+                                  ? 1
+                                  : std::min(FirstTok->NewlinesBefore, 1u);
           Whitespaces.replaceWhitespace(*TheLine.First, NewLines,
                                         /*IndentLevel=*/0, /*Spaces=*/0,
                                         /*TargetColumn=*/0);
diff --git a/tools/clang-format/ClangFormat.cpp b/tools/clang-format/ClangFormat.cpp
index e08aa2d..3815b8e 100644
--- a/tools/clang-format/ClangFormat.cpp
+++ b/tools/clang-format/ClangFormat.cpp
@@ -149,7 +149,7 @@ static bool fillRanges(SourceManager &Sources, FileID ID,
     return true;
   }
   for (unsigned i = 0, e = Offsets.size(); i != e; ++i) {
-    if (Offsets[i] >= Code->getBufferSize()) {
+    if (Offsets[i] > Code->getBufferSize()) {
       llvm::errs() << "error: offset " << Offsets[i]
                    << " is outside the file\n";
       return true;
@@ -185,8 +185,6 @@ static bool format(std::string FileName) {
     llvm::errs() << ec.message() << "\n";
     return true;
   }
-  if (Code->getBufferSize() == 0)
-    return true; // Empty files are formatted correctly.
   FileID ID = createInMemoryFile(FileName, Code.get(), Sources, Files);
   std::vector<CharSourceRange> Ranges;
   if (fillRanges(Sources, ID, Code.get(), Ranges))
diff --git a/tools/libclang/CXComment.cpp b/tools/libclang/CXComment.cpp
index 4658856..5a97670 100644
--- a/tools/libclang/CXComment.cpp
+++ b/tools/libclang/CXComment.cpp
@@ -970,7 +970,8 @@ void CommentASTToXMLConverter::formatTextOfDeclaration(
   Lexer Lex(ID, FormatRewriterContext.Sources.getBuffer(ID),
             FormatRewriterContext.Sources, LangOpts);
   tooling::Replacements Replace =
-    reformat(format::getLLVMStyle(), Lex, FormatRewriterContext.Sources, Ranges);
+      reformat(format::getLLVMStyle().forSnippets(), Lex,
+               FormatRewriterContext.Sources, Ranges);
   applyAllReplacements(Replace, FormatRewriterContext.Rewrite);
   Declaration = FormatRewriterContext.getRewrittenText(ID);
 }


More information about the cfe-commits mailing list