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

[Daniel Jasper]

I think the problem is that to me this is not a formatting decision
(although this might be irrational). To me, it is much closer to a file
encoding decision. Also, I don't think this decision is part of any of the
coding styles you have mentioned (at least not to a point where somebody
has bothered to write it into the style guide).

However, I don't disagree with the fundamental concept of clang-format
inserting these newlines. What I somewhat doubt is that it is useful as
part of the clang-format library. If I write a tool that refactors a
certain piece of code (e.g. as done in the unit tests), I do not want
clang-format (more precisely libFormat as "clang-format" is the binary) to
mess with my line-end/file-end decisions. I can very well use the library
to format small bits and pieces of code that aren't even meant to be full
files.

Therefore, I think this particular piece of logic belongs in
clang/tools/clang-format/ClangFormat.cpp. Right before saving a file (or
printing to stdout), we could ensure that there is a newline at the end
(possibly guarded on a style option or command line flag).

On Sun, Oct 6, 2013 at 3:44 PM, Alp Toker <alp at nuanti.com> wrote:

> On 06/10/2013 14:13, Daniel Jasper wrote:
> > Actually, the C++11 standard says:
> > "A source file that is not empty and that does not end in a new-line
> > character, or that ends in a new-line character immediately preceded
> > by a backslash character before any such splicing takes place, shall
> > be processed as if an additional new-line character were appended to
> > the file."
>
> You're right that it's not a hard error in C++, only C, and the remarks
> should be updated to reflect that.
>
> >
> > I think this pretty much implies that source files are allowed to not
> > end in a newline. Also, I think it is fine for clang-format to not
> > alter its input file in this regard. If your VCS requires this,
> > configure your editor appropriately ..
>
> If my editor got formatting right, I wouldn't need clang-format :-)
>
> Newline at EOF is certainly part of the coding style on the WebKit
> project and checked for during code review. It's always a pain to miss
> this and have to go back and fix after the fact.
>
> I can't speak for LLVM but Benjamin Kramer's commit r191857 (Add newline
> at eof) suggests it's also part of the coding style here.
>
> Then there's clang r189110 (Respect -Wnewline-eof even in C++11 mode,
> <rdar://problem/12922063>) showing that Apple customers also consider
> this important with C++ code.
>
> I'm guessing from your response that it's not part of the Google coding
> style, so I'll make sure that this check is disabled for C++ sources in
> GoogleStyle in the next revision of the patch.
>
> That leaves ChromiumStyle which I don't know enough about. Anyone?
>
> Thanks for the feedback.
>
> Alp.
>
>
> >
> >
> > On Sun, Oct 6, 2013 at 2:10 PM, Alp Toker <alp at nuanti.com
> > <mailto:alp at nuanti.com>> wrote:
> >
> >     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
> >
> >
>
> --
> http://www.nuanti.com
> the browser experts
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131006/a909f440/attachment.html>