Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang-format] Separate License text and include blocks #77918

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
52 changes: 52 additions & 0 deletions clang/docs/ClangFormatStyleOptions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3220,6 +3220,58 @@ the configuration (without a prefix: ``Auto``).



.. _EmptyLinesAfterIncludes:

**EmptyLinesAfterIncludes** (``Unsigned``) :versionbadge:`clang-format 1` :ref:`¶ <EmptyLinesAfterIncludes>`
Number of lines after includes.
If set, determines the number of lines to insert after includes.
Limited by MaxEmptyLinesToKeep.
Example:
EmptyLinesAfterIncludes = 1

.. code-block:: c++

#include <string>
#include <map>

class Test {};

vs EmptyLinesAfterIncludes = 2

.. code-block:: c++

#include <string>
#include <map>


class Test {};

.. _EmptyLinesAfterTopLevelComment:

**EmptyLinesAfterTopLevelComment** (``Unsigned``) :versionbadge:`clang-format 1` :ref:`¶ <EmptyLinesAfterTopLevelComment>`
Number of empty lines after top level comment.
If set, determines the number of empty lines to insert/keep after the top
level comment. Limited by MaxEmptyLinesToKeep.
Example:
EmptyLinesAfterTopLevelComment = 1

.. code-block:: c++

/* LICENSE TEXT */

#include <string>
class Test {};

vs EmptyLinesAfterTopLevelComment = 2

.. code-block:: c++

/* License Text */


#include <string>
class Test {};

.. _ExperimentalAutoDetectBinPacking:

**ExperimentalAutoDetectBinPacking** (``Boolean``) :versionbadge:`clang-format 3.7` :ref:`¶ <ExperimentalAutoDetectBinPacking>`
Expand Down
48 changes: 48 additions & 0 deletions clang/include/clang/Format/Format.h
Original file line number Diff line number Diff line change
Expand Up @@ -2459,6 +2459,52 @@ struct FormatStyle {
/// \version 12
EmptyLineBeforeAccessModifierStyle EmptyLineBeforeAccessModifier;

/// \brief Number of lines after includes.
/// If set, determines the number of lines to insert after includes.
/// Limited by MaxEmptyLinesToKeep.
/// Example:
/// EmptyLinesAfterIncludes = 1
/// \code
/// #include <string>
/// #include <map>
///
/// class Test {};
///
/// \endcode
/// vs EmptyLinesAfterIncludes = 2
/// \code
/// #include <string>
/// #include <map>
///
///
/// class Test {};
/// \endcode
/// \version 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// \version 1
/// \version 18

You're a bit late for LLVM 1.0 ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somehow thought this was the version of the option.

std::optional<unsigned> EmptyLinesAfterIncludes;

/// \brief Number of empty lines after top level comment.
/// If set, determines the number of empty lines to insert/keep after the top
/// level comment. Limited by MaxEmptyLinesToKeep.
/// Example:
/// EmptyLinesAfterTopLevelComment = 1
/// \code
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer both examples in the same code block side by side. It is more concise.

/// /* LICENSE TEXT */
///
/// #include <string>
/// class Test {};
///
/// \endcode
/// vs EmptyLinesAfterTopLevelComment = 2
/// \code
/// /* License Text */
///
///
/// #include <string>
/// class Test {};
/// \endcode
/// \version 1
std::optional<unsigned> EmptyLinesAfterTopLevelComment;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name has to be improved.

// Top level comment in your definition

#include <foo>

// But this is also top level, or not?


/// If ``true``, clang-format detects whether function calls and
/// definitions are formatted with one parameter per line.
///
Expand Down Expand Up @@ -4827,6 +4873,8 @@ struct FormatStyle {
DerivePointerAlignment == R.DerivePointerAlignment &&
DisableFormat == R.DisableFormat &&
EmptyLineAfterAccessModifier == R.EmptyLineAfterAccessModifier &&
EmptyLinesAfterIncludes == R.EmptyLinesAfterIncludes &&
EmptyLinesAfterTopLevelComment == R.EmptyLinesAfterTopLevelComment &&
EmptyLineBeforeAccessModifier == R.EmptyLineBeforeAccessModifier &&
Comment on lines +4876 to 4878
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
EmptyLinesAfterIncludes == R.EmptyLinesAfterIncludes &&
EmptyLinesAfterTopLevelComment == R.EmptyLinesAfterTopLevelComment &&
EmptyLineBeforeAccessModifier == R.EmptyLineBeforeAccessModifier &&
EmptyLineBeforeAccessModifier == R.EmptyLineBeforeAccessModifier &&
EmptyLinesAfterIncludes == R.EmptyLinesAfterIncludes &&
EmptyLinesAfterTopLevelComment == R.EmptyLinesAfterTopLevelComment &&

ExperimentalAutoDetectBinPacking ==
R.ExperimentalAutoDetectBinPacking &&
Expand Down
1 change: 1 addition & 0 deletions clang/lib/Format/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ add_clang_library(clangFormat
SortJavaScriptImports.cpp
TokenAnalyzer.cpp
TokenAnnotator.cpp
TopLevelCommentSeparator.cpp
UnwrappedLineFormatter.cpp
UnwrappedLineParser.cpp
UsingDeclarationsSorter.cpp
Expand Down
14 changes: 14 additions & 0 deletions clang/lib/Format/Format.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "SortJavaScriptImports.h"
#include "TokenAnalyzer.h"
#include "TokenAnnotator.h"
#include "TopLevelCommentSeparator.h"
#include "UnwrappedLineFormatter.h"
#include "UnwrappedLineParser.h"
#include "UsingDeclarationsSorter.h"
Expand Down Expand Up @@ -995,6 +996,9 @@ template <> struct MappingTraits<FormatStyle> {
IO.mapOptional("DisableFormat", Style.DisableFormat);
IO.mapOptional("EmptyLineAfterAccessModifier",
Style.EmptyLineAfterAccessModifier);
IO.mapOptional("EmptyLinesAfterIncludes", Style.EmptyLinesAfterIncludes);
IO.mapOptional("EmptyLinesAfterTopLevelComment",
Style.EmptyLinesAfterTopLevelComment);
IO.mapOptional("EmptyLineBeforeAccessModifier",
Style.EmptyLineBeforeAccessModifier);
IO.mapOptional("ExperimentalAutoDetectBinPacking",
Expand Down Expand Up @@ -1035,6 +1039,8 @@ template <> struct MappingTraits<FormatStyle> {
IO.mapOptional("MaxEmptyLinesToKeep", Style.MaxEmptyLinesToKeep);
IO.mapOptional("NamespaceIndentation", Style.NamespaceIndentation);
IO.mapOptional("NamespaceMacros", Style.NamespaceMacros);
IO.mapOptional("EmptyLinesAfterTopLevelComment",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove

Style.EmptyLinesAfterTopLevelComment);
IO.mapOptional("ObjCBinPackProtocolList", Style.ObjCBinPackProtocolList);
IO.mapOptional("ObjCBlockIndentWidth", Style.ObjCBlockIndentWidth);
IO.mapOptional("ObjCBreakBeforeNestedBlockParam",
Expand Down Expand Up @@ -1501,6 +1507,8 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
LLVMStyle.DerivePointerAlignment = false;
LLVMStyle.DisableFormat = false;
LLVMStyle.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Never;
LLVMStyle.EmptyLinesAfterIncludes = std::nullopt;
LLVMStyle.EmptyLinesAfterTopLevelComment = std::nullopt;
LLVMStyle.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock;
LLVMStyle.ExperimentalAutoDetectBinPacking = false;
LLVMStyle.FixNamespaceComments = true;
Expand Down Expand Up @@ -3713,6 +3721,12 @@ reformat(const FormatStyle &Style, StringRef Code,
});
}

if (Style.EmptyLinesAfterTopLevelComment.has_value()) {
Passes.emplace_back([&](const Environment &Env) {
return TopLevelCommentSeparator(Env, Expanded).process();
});
}

if (Style.Language == FormatStyle::LK_ObjC &&
!Style.ObjCPropertyAttributeOrder.empty()) {
Passes.emplace_back([&](const Environment &Env) {
Expand Down
8 changes: 8 additions & 0 deletions clang/lib/Format/TokenAnnotator.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,14 @@ class AnnotatedLine {
return First && First->is(tok::comment) && !First->getNextNonComment();
}

bool isInclude() const {
if (!First)
return false;

const auto *NextToken = First->getNextNonComment();
return First->is(tok::hash) && NextToken && NextToken->is(tok::pp_include);
}

/// \c true if this line starts with the given tokens in order, ignoring
/// comments.
template <typename... Ts> bool startsWith(Ts... Tokens) const {
Expand Down
70 changes: 70 additions & 0 deletions clang/lib/Format/TopLevelCommentSeparator.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
//===--- TopLevelCommentSeparator.cpp ---------------------------*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
///
/// \file
/// This file implements TopLevelCommentSeparator, a TokenAnalyzer that inserts
/// new lines or removes empty lines after the top level comment (i.e. comment
/// block at the top of the source file), usually license text or documentation.
///
//===----------------------------------------------------------------------===//

#include "TopLevelCommentSeparator.h"
#define DEBUG_TYPE "top-level-comment-separator"

namespace clang {
namespace format {
std::pair<tooling::Replacements, unsigned> TopLevelCommentSeparator::analyze(
TokenAnnotator &Annotator, SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
FormatTokenLexer &Tokens) {
assert(Style.EmptyLinesAfterTopLevelComment.has_value());
AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
tooling::Replacements Result;
separateTopLevelComment(AnnotatedLines, Result, Tokens);
return {Result, 0};
}

void TopLevelCommentSeparator::separateTopLevelComment(
SmallVectorImpl<AnnotatedLine *> &Lines, tooling::Replacements &Result,
FormatTokenLexer &Tokens) {
unsigned NewlineCount = std::min(Style.MaxEmptyLinesToKeep,
*Style.EmptyLinesAfterTopLevelComment) +
1;
WhitespaceManager Whitespaces(
Env.getSourceManager(), Style,
Style.LineEnding > FormatStyle::LE_CRLF
? WhitespaceManager::inputUsesCRLF(
Env.getSourceManager().getBufferData(Env.getFileID()),
Style.LineEnding == FormatStyle::LE_DeriveCRLF)
: Style.LineEnding == FormatStyle::LE_CRLF);

bool InTopLevelComment = false;
for (unsigned I = 0; I < Lines.size(); ++I) {
const auto &CurrentLine = Lines[I];
if (CurrentLine->isComment()) {
InTopLevelComment = true;
} else if (InTopLevelComment) {
// Do not handle EOF newlines.
if (!CurrentLine->First->is(tok::eof) && CurrentLine->Affected) {
Whitespaces.replaceWhitespace(*CurrentLine->First, NewlineCount,
CurrentLine->First->OriginalColumn,
CurrentLine->First->OriginalColumn);
}
break;
}
Comment on lines +48 to +58
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#include <something>

// My comment
int foo();

Is My comment now the "Top level" comment and new lines will be set accordingly? That is not what you intent to do, or am I mistaken?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another one

int foo() {
  // Comment
  return 5;
}

}

for (const auto &R : Whitespaces.generateReplacements()) {
// The add method returns an Error instance which simulates program exit
// code through overloading boolean operator, thus false here indicates
// success.
if (Result.add(R))
return;
}
}
} // namespace format
} // namespace clang
42 changes: 42 additions & 0 deletions clang/lib/Format/TopLevelCommentSeparator.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
//===--- TopLevelCommentSeparator.h -----------------------------*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
///
/// \file
/// This file declares TopLevelCommentSeparator, a TokenAnalyzer that inserts
/// new lines or removes empty lines after the top level comment (i.e. comment
/// block at the top of the source file), usually license text or documentation.
///
//===----------------------------------------------------------------------===//

#ifndef LLVM_CLANG_LIB_FORMAT_TOPLEVELCOMMENTSEPARATOR_H
#define LLVM_CLANG_LIB_FORMAT_TOPLEVELCOMMENTSEPARATOR_H

#include "TokenAnalyzer.h"
#include "WhitespaceManager.h"

namespace clang {
namespace format {
class TopLevelCommentSeparator : public TokenAnalyzer {
public:
TopLevelCommentSeparator(const Environment &Env, const FormatStyle &Style)
: TokenAnalyzer(Env, Style) {}

std::pair<tooling::Replacements, unsigned>
analyze(TokenAnnotator &Annotator,
SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
FormatTokenLexer &Tokens) override;

private:
void separateTopLevelComment(SmallVectorImpl<AnnotatedLine *> &Lines,
tooling::Replacements &Result,
FormatTokenLexer &Tokens);
};
} // namespace format
} // namespace clang

#endif
6 changes: 6 additions & 0 deletions clang/lib/Format/UnwrappedLineFormatter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1535,6 +1535,12 @@ static auto computeNewlines(const AnnotatedLine &Line,
}
}

if (Style.EmptyLinesAfterIncludes.has_value() && !Line.InMacroBody &&
PreviousLine && PreviousLine->isInclude() && !Line.isInclude()) {
Newlines =
1 + std::min(Style.MaxEmptyLinesToKeep, *Style.EmptyLinesAfterIncludes);
}

return Newlines;
}

Expand Down
1 change: 1 addition & 0 deletions clang/unittests/Format/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ add_clang_unittest(FormatTests
SortIncludesTest.cpp
UsingDeclarationsSorterTest.cpp
TokenAnnotatorTest.cpp
TopLevelCommentSeparatorTest.cpp
)

clang_target_link_libraries(FormatTests
Expand Down
4 changes: 4 additions & 0 deletions clang/unittests/Format/ConfigParseTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1004,6 +1004,10 @@ TEST(ConfigParseTest, ParsesConfiguration) {
FormatStyle::SDS_Leave);
CHECK_PARSE("SeparateDefinitionBlocks: Never", SeparateDefinitionBlocks,
FormatStyle::SDS_Never);

CHECK_PARSE("EmptyLinesAfterIncludes: 2", EmptyLinesAfterIncludes, 2);
CHECK_PARSE("EmptyLinesAfterTopLevelComment: 2",
EmptyLinesAfterTopLevelComment, 2);
}

TEST(ConfigParseTest, ParsesConfigurationWithLanguages) {
Expand Down
20 changes: 20 additions & 0 deletions clang/unittests/Format/FormatTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26846,6 +26846,26 @@ TEST_F(FormatTest, BreakAdjacentStringLiterals) {
Style.BreakAdjacentStringLiterals = false;
verifyFormat(Code, Style);
}

TEST_F(FormatTest, EmptyLinesAfterInclude) {
auto Style = getLLVMStyle();
Style.EmptyLinesAfterIncludes = 2;
Style.MaxEmptyLinesToKeep = 2;
verifyFormat("#include <string>\n\n\n"
"class Test {};",
Comment on lines +26854 to +26855
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
verifyFormat("#include <string>\n\n\n"
"class Test {};",
verifyFormat("#include <string>\n"
"\n"
"\n"
"class Test {};",

So one does see the new lines directly.

Style);

Style.EmptyLinesAfterIncludes = 1;
verifyFormat("#include <string>\n\n"
"class Test {};",
Style);

Style.EmptyLinesAfterIncludes = 2;
Style.MaxEmptyLinesToKeep = 1;
verifyFormat("#include <string>\n\n"
"class Test {};",
Style);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add tests with multiple includes and in conjunction with include sorting/grouping.

} // namespace
} // namespace test
} // namespace format
Expand Down