r339803 - clang-format: Change Google style wrt. the formatting of empty messages.
Author: djasper
Date: Wed Aug 15 12:07:55 2018
New Revision: 339803
URL: http://llvm.org/viewvc/llvm-project?rev=339803&view=rev
Log:
clang-format: Change Google style wrt. the formatting of empty messages.
Before:
message Empty {
}
After:
message Empty {}
Modified:
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/unittests/Format/FormatTestProto.cpp
Modified: cfe/trunk/lib/Format/Format.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=339803&r1=339802&r2=339803&view=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Wed Aug 15 12:07:55 2018
@@ -819,7 +819,7 @@ FormatStyle getGoogleStyle(FormatStyle::
GoogleStyle.JavaScriptQuotes = FormatStyle::JSQS_Single;
GoogleStyle.JavaScriptWrapImports = false;
} else if (Language == FormatStyle::LK_Proto) {
-GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
GoogleStyle.SpacesInContainerLiterals = false;
GoogleStyle.Cpp11BracedListStyle = false;
Modified: cfe/trunk/unittests/Format/FormatTestProto.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestProto.cpp?rev=339803&r1=339802&r2=339803&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestProto.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestProto.cpp Wed Aug 15 12:07:55 2018
@@ -397,29 +397,25 @@ TEST_F(FormatTestProto, FormatsService)
}
TEST_F(FormatTestProto, ExtendingMessage) {
- verifyFormat("extend .foo.Bar {\n"
- "}");
+ verifyFormat("extend .foo.Bar {}");
}
TEST_F(FormatTestProto, FormatsImports) {
verifyFormat("import \"a.proto\";\n"
"import \"b.proto\";\n"
"// comment\n"
- "message A {\n"
- "}");
+ "message A {}");
verifyFormat("import public \"a.proto\";\n"
"import \"b.proto\";\n"
"// comment\n"
- "message A {\n"
- "}");
+ "message A {}");
// Missing semicolons should not confuse clang-format.
verifyFormat("import \"a.proto\"\n"
"import \"b.proto\"\n"
"// comment\n"
- "message A {\n"
- "}");
+ "message A {}");
}
TEST_F(FormatTestProto, KeepsLongStringLiteralsOnSameLine) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r315439 - Revert r314955: "Remove PendingBody mechanism for function and ObjC method deserialization."
Author: djasper
Date: Wed Oct 11 00:47:54 2017
New Revision: 315439
URL: http://llvm.org/viewvc/llvm-project?rev=315439&view=rev
Log:
Revert r314955: "Remove PendingBody mechanism for function and ObjC method
deserialization."
This is breaking a build of https://github.com/abseil/abseil-cpp and so
likely not really NFC. Also reverted subsequent r314956/7.
I'll forward reproduction instructions to Richard.
Removed:
cfe/trunk/test/Modules/merge-lambdas.cpp
Modified:
cfe/trunk/include/clang/Serialization/ASTReader.h
cfe/trunk/lib/Serialization/ASTCommon.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=315439&r1=315438&r2=315439&view=diff
==
--- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTReader.h Wed Oct 11 00:47:54 2017
@@ -559,9 +559,13 @@ private:
/// declarations that have not yet been linked to their definitions.
llvm::SmallPtrSet PendingDefinitions;
- /// \brief Functions or methods that are known to already have a definition
- /// (that might not yet be merged into the redeclaration chain).
- llvm::SmallDenseMap FunctionDefinitions;
+ typedef llvm::MapVector,
+ SmallVector, 4> >
+PendingBodiesMap;
+
+ /// \brief Functions or methods that have bodies that will be attached.
+ PendingBodiesMap PendingBodies;
/// \brief Definitions for which we have added merged definitions but not yet
/// performed deduplication.
@@ -987,13 +991,25 @@ private:
/// the last time we loaded information about this identifier.
llvm::DenseMap IdentifierGeneration;
+ class InterestingDecl {
+Decl *D;
+bool DeclHasPendingBody;
+
+ public:
+InterestingDecl(Decl *D, bool HasBody)
+: D(D), DeclHasPendingBody(HasBody) {}
+Decl *getDecl() { return D; }
+/// Whether the declaration has a pending body.
+bool hasPendingBody() { return DeclHasPendingBody; }
+ };
+
/// \brief Contains declarations and definitions that could be
/// "interesting" to the ASTConsumer, when we get that AST consumer.
///
/// "Interesting" declarations are those that have data that may
/// need to be emitted, such as inline function definitions or
/// Objective-C protocols.
- std::deque PotentiallyInterestingDecls;
+ std::deque PotentiallyInterestingDecls;
/// \brief The list of redeclaration chains that still need to be
/// reconstructed, and the local offset to the corresponding list
Modified: cfe/trunk/lib/Serialization/ASTCommon.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTCommon.cpp?rev=315439&r1=315438&r2=315439&view=diff
==
--- cfe/trunk/lib/Serialization/ASTCommon.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTCommon.cpp Wed Oct 11 00:47:54 2017
@@ -344,8 +344,8 @@ bool serialization::needsAnonymousDeclar
return true;
}
- // Otherwise, we only care about anonymous class members / block-scope decls.
- if (D->getDeclName() || D->getLexicalDeclContext()->isFileContext())
+ // Otherwise, we only care about anonymous class members.
+ if (D->getDeclName() || !isa(D->getLexicalDeclContext()))
return false;
return isa(D) || isa(D);
}
Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=315439&r1=315438&r2=315439&view=diff
==
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Wed Oct 11 00:47:54 2017
@@ -9169,6 +9169,30 @@ void ASTReader::finishPendingActions() {
}
PendingDefinitions.clear();
+ // Load the bodies of any functions or methods we've encountered. We do
+ // this now (delayed) so that we can be sure that the declaration chains
+ // have been fully wired up (hasBody relies on this).
+ // FIXME: We shouldn't require complete redeclaration chains here.
+ for (PendingBodiesMap::iterator PB = PendingBodies.begin(),
+ PBEnd = PendingBodies.end();
+ PB != PBEnd; ++PB) {
+if (FunctionDecl *FD = dyn_cast(PB->first)) {
+ // FIXME: Check for =delete/=default?
+ // FIXME: Complain about ODR violations here?
+ const FunctionDecl *Defn = nullptr;
+ if (!getContext().getLangOpts().Modules || !FD->hasBody(Defn)) {
+FD->setLazyBody(PB->second);
+ } else
+mergeDefinitionVisibility(const_cast(Defn), FD);
+ continue;
+}
+
+ObjCMethodDecl *MD = cast(PB->first);
+if (!getContext().getLangOpts().Modules || !MD->hasBody())
+ MD->setLazyBody(
r326023 - Make module use diagnostics refer to the top-level module
Author: djasper
Date: Fri Feb 23 22:54:09 2018
New Revision: 326023
URL: http://llvm.org/viewvc/llvm-project?rev=326023&view=rev
Log:
Make module use diagnostics refer to the top-level module
All use declarations need to be directly placed in the top-level module
anyway, knowing the submodule doesn't really help. The header that has
the offending #include can easily be seen in the diagnostics source
location.
Review: https://reviews.llvm.org/D43673
Modified:
cfe/trunk/lib/Lex/ModuleMap.cpp
cfe/trunk/test/Modules/Inputs/declare-use/h.h
Modified: cfe/trunk/lib/Lex/ModuleMap.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/ModuleMap.cpp?rev=326023&r1=326022&r2=326023&view=diff
==
--- cfe/trunk/lib/Lex/ModuleMap.cpp (original)
+++ cfe/trunk/lib/Lex/ModuleMap.cpp Fri Feb 23 22:54:09 2018
@@ -475,7 +475,7 @@ void ModuleMap::diagnoseHeaderInclusion(
// We have found a module, but we don't use it.
if (NotUsed) {
Diags.Report(FilenameLoc, diag::err_undeclared_use_of_module)
-<< RequestingModule->getFullModuleName() << Filename;
+<< RequestingModule->getTopLevelModule()->Name << Filename;
return;
}
@@ -486,7 +486,7 @@ void ModuleMap::diagnoseHeaderInclusion(
if (LangOpts.ModulesStrictDeclUse) {
Diags.Report(FilenameLoc, diag::err_undeclared_use_of_module)
-<< RequestingModule->getFullModuleName() << Filename;
+<< RequestingModule->getTopLevelModule()->Name << Filename;
} else if (RequestingModule && RequestingModuleIsModuleInterface &&
LangOpts.isCompilingModule()) {
// Do not diagnose when we are not compiling a module.
Modified: cfe/trunk/test/Modules/Inputs/declare-use/h.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/declare-use/h.h?rev=326023&r1=326022&r2=326023&view=diff
==
--- cfe/trunk/test/Modules/Inputs/declare-use/h.h (original)
+++ cfe/trunk/test/Modules/Inputs/declare-use/h.h Fri Feb 23 22:54:09 2018
@@ -1,7 +1,7 @@
#ifndef H_H
#define H_H
#include "c.h"
-#include "d.h" // expected-error {{does not depend on a module exporting}}
+#include "d.h" // expected-error {{module XH does not depend on a module
exporting}}
#include "h1.h"
const int h1 = aux_h*c*7*d;
#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r326024 - Remove unused variable. We should be warning-free.
Author: djasper
Date: Fri Feb 23 22:57:47 2018
New Revision: 326024
URL: http://llvm.org/viewvc/llvm-project?rev=326024&view=rev
Log:
Remove unused variable. We should be warning-free.
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=326024&r1=326023&r2=326024&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Fri Feb 23
22:57:47 2018
@@ -440,7 +440,7 @@ private:
os << Sep;
FR->getDecl()->getDeclName().print(os, PP);
Sep = ".";
- } else if (auto *CXXR = dyn_cast(*I)) {
+ } else if (isa(*I)) {
continue; // Just keep going up to the base region.
} else {
llvm_unreachable("Previous check has missed an unexpected region");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r305182 - Revert r305164/5/7.
Author: djasper
Date: Mon Jun 12 03:08:18 2017
New Revision: 305182
URL: http://llvm.org/viewvc/llvm-project?rev=305182&view=rev
Log:
Revert r305164/5/7.
cc1as does not currently access the "--" version of this flag. At the
very least this needs to be fixed and proper test cases need to be
added.
Simple reproducer:
clang -Wa,--compress-debug-sections /tmp/test.cc
Result:
error: unknown argument: '--compress-debug-sections'
Removed:
cfe/trunk/test/Driver/compress-noias.c
Modified:
cfe/trunk/include/clang/Driver/CC1Options.td
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/Driver/compress.c
cfe/trunk/test/Driver/nozlibcompress.c
cfe/trunk/tools/driver/cc1as_main.cpp
Modified: cfe/trunk/include/clang/Driver/CC1Options.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=305182&r1=305181&r2=305182&view=diff
==
--- cfe/trunk/include/clang/Driver/CC1Options.td (original)
+++ cfe/trunk/include/clang/Driver/CC1Options.td Mon Jun 12 03:08:18 2017
@@ -134,6 +134,7 @@ def migrator_no_finalize_removal : Flag<
//===--===//
let Flags = [CC1Option, CC1AsOption, NoDriverOption] in {
+
def debug_info_kind_EQ : Joined<["-"], "debug-info-kind=">;
def debug_info_macro : Flag<["-"], "debug-info-macro">,
HelpText<"Emit macro debug information">;
@@ -143,16 +144,14 @@ def fdebug_compilation_dir : Separate<["
HelpText<"The compilation directory to embed in the debug info.">;
def dwarf_debug_flags : Separate<["-"], "dwarf-debug-flags">,
HelpText<"The string to embed in the Dwarf debug flags record.">;
-def compress_debug_sections : Flag<["-"], "compress-debug-sections">,
-HelpText<"DWARF debug sections compression">;
-def compress_debug_sections_EQ : Flag<["-"], "compress-debug-sections=">,
-HelpText<"DWARF debug sections compression type">;
def mno_exec_stack : Flag<["-"], "mnoexecstack">,
HelpText<"Mark the file as not needing an executable stack">;
def massembler_fatal_warnings : Flag<["-"], "massembler-fatal-warnings">,
HelpText<"Make assembler warnings fatal">;
def mrelax_relocations : Flag<["--"], "mrelax-relocations">,
HelpText<"Use relaxable elf relocations">;
+def compress_debug_sections : Flag<["-"], "compress-debug-sections">,
+HelpText<"Compress DWARF debug sections using zlib">;
def msave_temp_labels : Flag<["-"], "msave-temp-labels">,
HelpText<"Save temporary labels in the symbol table. "
"Note this may change .s semantics and shouldn't generally be used "
Modified: cfe/trunk/include/clang/Driver/Options.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=305182&r1=305181&r2=305182&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Mon Jun 12 03:08:18 2017
@@ -1563,10 +1563,6 @@ def gdwarf_aranges : Flag<["-"], "gdwarf
def gmodules : Flag <["-"], "gmodules">, Group,
HelpText<"Generate debug info with external references to clang modules"
" or precompiled headers">;
-def gz : Flag<["-"], "gz">, Group,
-HelpText<"DWARF debug sections compression type">;
-def gz_EQ : Joined<["-"], "gz=">, Group,
-HelpText<"DWARF debug sections compression type">;
def headerpad__max__install__names : Joined<["-"],
"headerpad_max_install_names">;
def help : Flag<["-", "--"], "help">, Flags<[CC1Option,CC1AsOption]>,
HelpText<"Display available options">;
Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=305182&r1=305181&r2=305182&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Mon Jun 12 03:08:18 2017
@@ -910,37 +910,6 @@ static void RenderDebugEnablingArgs(cons
}
}
-static void RenderDebugInfoCompressionArgs(const ArgList &Args,
- ArgStringList &CmdArgs,
- const Driver &D) {
- const Arg *A = Args.getLastArg(options::OPT_gz, options::OPT_gz_EQ);
- if (!A)
-return;
-
- if (A->getOption().getID() == options::OPT_gz) {
-if (llvm::zlib::isAvailable())
- CmdArgs.push_back("-compress-debug-sections");
-else
- D.Diag(diag::warn_debug_compression_unavailable);
-return;
- }
-
- StringRef Value = A->getValue();
- if (Value == "none") {
-CmdArgs.push_back("-compress-debug-sections=none");
- } else if (Value == "zlib" || Value == "zlib-gnu") {
-if (llvm::zlib::isAvailabl
r305456 - Revert "Define _GNU_SOURCE for rtems c++"
Author: djasper
Date: Thu Jun 15 04:17:12 2017
New Revision: 305456
URL: http://llvm.org/viewvc/llvm-project?rev=305456&view=rev
Log:
Revert "Define _GNU_SOURCE for rtems c++"
This reverts commit r305399.
This breaks a build in libcxx:
libcxx/src/system_error.cpp:90:16: error: assigning to 'int' from incompatible
type 'char *'
if ((ret = ::strerror_r(ev, buffer, strerror_buff_size)) != 0) {
^~~~
1 error generated.
Which makes sense according to:
https://linux.die.net/man/3/strerror_r
Not entirely sure how this needs to be fixed.
Modified:
cfe/trunk/lib/Basic/Targets.cpp
cfe/trunk/test/Preprocessor/init.c
Modified: cfe/trunk/lib/Basic/Targets.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=305456&r1=305455&r2=305456&view=diff
==
--- cfe/trunk/lib/Basic/Targets.cpp (original)
+++ cfe/trunk/lib/Basic/Targets.cpp Thu Jun 15 04:17:12 2017
@@ -4734,9 +4734,6 @@ protected:
Builder.defineMacro("__rtems__");
Builder.defineMacro("__ELF__");
-// Required by the libc++ locale support.
-if (Opts.CPlusPlus)
- Builder.defineMacro("_GNU_SOURCE");
}
public:
Modified: cfe/trunk/test/Preprocessor/init.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/init.c?rev=305456&r1=305455&r2=305456&view=diff
==
--- cfe/trunk/test/Preprocessor/init.c (original)
+++ cfe/trunk/test/Preprocessor/init.c Thu Jun 15 04:17:12 2017
@@ -8779,7 +8779,6 @@
// KFREEBSDI686-DEFINE:#define __GLIBC__ 1
//
// RUN: %clang_cc1 -x c++ -triple i686-pc-linux-gnu -fobjc-runtime=gcc -E -dM
< /dev/null | FileCheck -match-full-lines -check-prefix GNUSOURCE %s
-// RUN: %clang_cc1 -x c++ -triple sparc-rtems-elf -E -dM < /dev/null |
FileCheck -match-full-lines -check-prefix GNUSOURCE %s
// GNUSOURCE:#define _GNU_SOURCE 1
//
// RUN: %clang_cc1 -x c++ -std=c++98 -fno-rtti -E -dM < /dev/null | FileCheck
-match-full-lines -check-prefix NORTTI %s
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r305574 - Ignore return value in test.
Author: djasper Date: Fri Jun 16 14:29:20 2017 New Revision: 305574 URL: http://llvm.org/viewvc/llvm-project?rev=305574&view=rev Log: Ignore return value in test. Modified: cfe/trunk/test/Driver/m_and_mm.c Modified: cfe/trunk/test/Driver/m_and_mm.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/m_and_mm.c?rev=305574&r1=305573&r2=305574&view=diff == --- cfe/trunk/test/Driver/m_and_mm.c (original) +++ cfe/trunk/test/Driver/m_and_mm.c Fri Jun 16 14:29:20 2017 @@ -5,7 +5,7 @@ // RUN: %clang -M -MM %s 2> %t // RUN: not grep "warning" %t -// RUN: %clang -MMD -MD %s 2> %t +// RUN: %clang -MMD -MD %s 2> %t || true // RUN: grep "warning" %t #warning "This warning shouldn't show up with -M and -MM" ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r305665 - clang-format: Add capability to format the diff on save in vim.
Author: djasper
Date: Mon Jun 19 02:30:04 2017
New Revision: 305665
URL: http://llvm.org/viewvc/llvm-project?rev=305665&view=rev
Log:
clang-format: Add capability to format the diff on save in vim.
With this patch, one can configure a BufWrite hook that will make the
clang-format integration compute a diff of the current buffer with the file
that's on disk and format all changed lines. This should create a
zero-overhead auto-format solution that doesn't require the file to
already be clang-format clean to avoid spurious diffs.
Review: https://reviews.llvm.org/D32429
Modified:
cfe/trunk/docs/ClangFormat.rst
cfe/trunk/tools/clang-format/clang-format.py
Modified: cfe/trunk/docs/ClangFormat.rst
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormat.rst?rev=305665&r1=305664&r2=305665&view=diff
==
--- cfe/trunk/docs/ClangFormat.rst (original)
+++ cfe/trunk/docs/ClangFormat.rst Mon Jun 19 02:30:04 2017
@@ -120,6 +120,18 @@ entity.
It operates on the current, potentially unsaved buffer and does not create
or save any files. To revert a formatting, just undo.
+An alternative option is to format changes when saving a file and thus to
+have a zero-effort integration into the coding workflow. To do this, add this
to
+your `.vimrc`:
+
+.. code-block:: vim
+
+ function! Formatonsave()
+let l:formatdiff = 1
+pyf ~/llvm/tools/clang/tools/clang-format/clang-format.py
+ endfunction
+ autocmd BufWritePre *.h,*.cc,*.cpp call Formatonsave()
+
Emacs Integration
=
Modified: cfe/trunk/tools/clang-format/clang-format.py
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/clang-format.py?rev=305665&r1=305664&r2=305665&view=diff
==
--- cfe/trunk/tools/clang-format/clang-format.py (original)
+++ cfe/trunk/tools/clang-format/clang-format.py Mon Jun 19 02:30:04 2017
@@ -63,8 +63,19 @@ def main():
# Determine range to format.
if vim.eval('exists("l:lines")') == '1':
lines = vim.eval('l:lines')
+ elif vim.eval('exists("l:formatdiff")') == '1':
+with open(vim.current.buffer.name, 'r') as f:
+ ondisk = f.read().splitlines();
+sequence = difflib.SequenceMatcher(None, ondisk, vim.current.buffer)
+lines = []
+for op in reversed(sequence.get_opcodes()):
+ if op[0] not in ['equal', 'delete']:
+lines += ['-lines', '%s:%s' % (op[3] + 1, op[4])]
+if lines == []:
+ return
else:
-lines = '%s:%s' % (vim.current.range.start + 1, vim.current.range.end + 1)
+lines = ['-lines', '%s:%s' % (vim.current.range.start + 1,
+ vim.current.range.end + 1)]
# Determine the cursor position.
cursor = int(vim.eval('line2byte(line("."))+col(".")')) - 2
@@ -82,7 +93,7 @@ def main():
# Call formatter.
command = [binary, '-style', style, '-cursor', str(cursor)]
if lines != 'all':
-command.extend(['-lines', lines])
+command += lines
if fallback_style:
command.extend(['-fallback-style', fallback_style])
if vim.current.buffer.name:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r305666 - clang-format: Handle "if constexpr".
Author: djasper
Date: Mon Jun 19 02:40:49 2017
New Revision: 305666
URL: http://llvm.org/viewvc/llvm-project?rev=305666&view=rev
Log:
clang-format: Handle "if constexpr".
c++1z adds the following constructions to the language:
if constexpr (cond)
statement1;
else if constexpr (cond)
statement2;
else if constexpr (cond)
statement3;
else
statement4;
A first version of this was proposed in reviews.llvm.org/D26953 by
Francis Visoiu Mistrih, but never commited. This patch additionally
fixes the behavior when allowing short if statements on a single line
and was authored by Jacob Bandes-Storch. Thank you to both authors.
Modified:
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=305666&r1=305665&r2=305666&view=diff
==
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Mon Jun 19 02:40:49 2017
@@ -453,7 +453,8 @@ void ContinuationIndenter::addTokenOnCur
State.Column += Spaces;
if (Current.isNot(tok::comment) && Previous.is(tok::l_paren) &&
Previous.Previous &&
- Previous.Previous->isOneOf(tok::kw_if, tok::kw_for)) {
+ (Previous.Previous->isOneOf(tok::kw_if, tok::kw_for) ||
+ Previous.Previous->endsSequence(tok::kw_constexpr, tok::kw_if))) {
// Treat the condition inside an if as if it was a second function
// parameter, i.e. let nested calls have a continuation indent.
State.Stack.back().LastSpace = State.Column;
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=305666&r1=305665&r2=305666&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Mon Jun 19 02:40:49 2017
@@ -145,6 +145,7 @@ private:
(Left->Previous->isOneOf(tok::kw_static_assert, tok::kw_decltype,
tok::kw_if, tok::kw_while, tok::l_paren,
tok::comma) ||
+ Left->Previous->endsSequence(tok::kw_constexpr, tok::kw_if) ||
Left->Previous->is(TT_BinaryOperator))) {
// static_assert, if and while usually contain expressions.
Contexts.back().IsExpression = true;
@@ -572,6 +573,8 @@ private:
break;
case tok::kw_if:
case tok::kw_while:
+ if (Tok->is(tok::kw_if) && CurrentToken &&
CurrentToken->is(tok::kw_constexpr))
+next();
if (CurrentToken && CurrentToken->is(tok::l_paren)) {
next();
if (!parseParens(/*LookForDecls=*/true))
@@ -2060,7 +2063,8 @@ unsigned TokenAnnotator::splitPenalty(co
Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign)
return 100;
if (Left.is(tok::l_paren) && Left.Previous &&
- Left.Previous->isOneOf(tok::kw_if, tok::kw_for))
+ (Left.Previous->isOneOf(tok::kw_if, tok::kw_for)
+ || Left.Previous->endsSequence(tok::kw_constexpr, tok::kw_if)))
return 1000;
if (Left.is(tok::equal) && InFunctionDecl)
return 110;
@@ -2211,6 +2215,7 @@ bool TokenAnnotator::spaceRequiredBetwee
(Left.isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for, tok::kw_while,
tok::kw_switch, tok::kw_case, TT_ForEachMacro,
TT_ObjCForIn) ||
+ Left.endsSequence(tok::kw_constexpr, tok::kw_if) ||
(Left.isOneOf(tok::kw_try, Keywords.kw___except, tok::kw_catch,
tok::kw_new, tok::kw_delete) &&
(!Left.Previous || Left.Previous->isNot(tok::period) ||
Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=305666&r1=305665&r2=305666&view=diff
==
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Mon Jun 19 02:40:49 2017
@@ -1516,6 +1516,8 @@ void UnwrappedLineParser::parseSquare()
void UnwrappedLineParser::parseIfThenElse() {
assert(FormatTok->Tok.is(tok::kw_if) && "'if' expected");
nextToken();
+ if (FormatTok->Tok.is(tok::kw_constexpr))
+nextToken();
if (FormatTok->Tok.is(tok::l_paren))
parseParens();
bool NeedsUnwrappedLine = false;
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=305666&r1=305665&r2=305666&view=diff
==
--- cfe/t
r305667 - clang-format: Improve understanding of combined typedef+record declarations
Author: djasper
Date: Mon Jun 19 02:45:41 2017
New Revision: 305667
URL: http://llvm.org/viewvc/llvm-project?rev=305667&view=rev
Log:
clang-format: Improve understanding of combined typedef+record declarations
Fixes an issue where struct A { int X; }; would be broken onto multiple
lines, but typedef struct A { int X; } A2; was collapsed onto a single
line.
Patch by Jacob Bandes-Storch. Thank you.
Modified:
cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp?rev=305667&r1=305666&r2=305667&view=diff
==
--- cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp Mon Jun 19 02:45:41 2017
@@ -434,8 +434,11 @@ private:
} else if (Limit != 0 && !Line.startsWith(tok::kw_namespace) &&
!startsExternCBlock(Line)) {
// We don't merge short records.
- if (Line.First->isOneOf(tok::kw_class, tok::kw_union, tok::kw_struct,
- Keywords.kw_interface))
+ FormatToken *RecordTok =
+ Line.First->is(tok::kw_typedef) ? Line.First->Next : Line.First;
+ if (RecordTok &&
+ RecordTok->isOneOf(tok::kw_class, tok::kw_union, tok::kw_struct,
+ Keywords.kw_interface))
return 0;
// Check that we still have three lines and they fit into the limit.
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=305667&r1=305666&r2=305667&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Jun 19 02:45:41 2017
@@ -458,6 +458,14 @@ TEST_F(FormatTest, FormatShortBracedStat
"}",
AllowSimpleBracedStatements);
+ verifyFormat("struct A2 {\n"
+ " int X;\n"
+ "};",
+ AllowSimpleBracedStatements);
+ verifyFormat("typedef struct A2 {\n"
+ " int X;\n"
+ "} A2_t;",
+ AllowSimpleBracedStatements);
verifyFormat("template struct A2 {\n"
" struct B {};\n"
"};",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r327253 - Don't re-format raw string literal contents when formatting is disable
Author: djasper
Date: Mon Mar 12 03:11:30 2018
New Revision: 327253
URL: http://llvm.org/viewvc/llvm-project?rev=327253&view=rev
Log:
Don't re-format raw string literal contents when formatting is disable
Not entirely sure this is the best place to put this check, but it fixes
the immediate issue.
Modified:
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/unittests/Format/FormatTestRawStrings.cpp
Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=327253&r1=327252&r2=327253&view=diff
==
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Mon Mar 12 03:11:30 2018
@@ -1480,7 +1480,7 @@ unsigned ContinuationIndenter::handleEnd
// Compute the raw string style to use in case this is a raw string literal
// that can be reformatted.
auto RawStringStyle = getRawStringStyle(Current, State);
- if (RawStringStyle) {
+ if (RawStringStyle && !Current.Finalized) {
Penalty = reformatRawStringLiteral(Current, State, *RawStringStyle,
DryRun);
} else if (Current.IsMultiline && Current.isNot(TT_BlockComment)) {
// Don't break multi-line tokens other than block comments and raw string
Modified: cfe/trunk/unittests/Format/FormatTestRawStrings.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestRawStrings.cpp?rev=327253&r1=327252&r2=327253&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestRawStrings.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestRawStrings.cpp Mon Mar 12 03:11:30 2018
@@ -164,6 +164,20 @@ t = R"pb(item:1)pb";)test",
getRawStringPbStyleWithColumns(40)));
}
+TEST_F(FormatTestRawStrings, RespectsClangFormatOff) {
+ expect_eq(R"test(
+// clang-format off
+s = R"pb(item: 1)pb";
+// clang-format on
+t = R"pb(item: 1)pb";)test",
+format(R"test(
+// clang-format off
+s = R"pb(item: 1)pb";
+// clang-format on
+t = R"pb(item: 1)pb";)test",
+ getRawStringPbStyleWithColumns(40)));
+}
+
TEST_F(FormatTestRawStrings, ReformatsShortRawStringsOnSingleLine) {
expect_eq(
R"test(P p = TP(R"pb()pb");)test",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r327255 - clang-format: Properly handle implicit string concatenation in text protos
Author: djasper
Date: Mon Mar 12 03:32:18 2018
New Revision: 327255
URL: http://llvm.org/viewvc/llvm-project?rev=327255&view=rev
Log:
clang-format: Properly handle implicit string concatenation in text protos
Three issues to fix:
- char_constants weren't properly treated as string literals
- Prevening the break after "label: " does not make sense in concunction
with AlwaysBreakBeforeMultilineStrings. It leads to situations where
clang-format just cannot find a viable format (it must break and yet
it must not break).
- AlwaysBreakBeforeMultilineStrings should not be on for LK_TextProto in
Google style.
Modified:
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/FormatTokenLexer.cpp
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestProto.cpp
cfe/trunk/unittests/Format/FormatTestTextProto.cpp
Modified: cfe/trunk/lib/Format/Format.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=327255&r1=327254&r2=327255&view=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Mon Mar 12 03:32:18 2018
@@ -766,6 +766,7 @@ FormatStyle getGoogleStyle(FormatStyle::
GoogleStyle.JavaScriptWrapImports = false;
} else if (Language == FormatStyle::LK_Proto) {
GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
GoogleStyle.SpacesInContainerLiterals = false;
GoogleStyle.Cpp11BracedListStyle = false;
// This affects protocol buffer options specifications and text protos.
Modified: cfe/trunk/lib/Format/FormatTokenLexer.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatTokenLexer.cpp?rev=327255&r1=327254&r2=327255&view=diff
==
--- cfe/trunk/lib/Format/FormatTokenLexer.cpp (original)
+++ cfe/trunk/lib/Format/FormatTokenLexer.cpp Mon Mar 12 03:32:18 2018
@@ -691,7 +691,9 @@ void FormatTokenLexer::readRawToken(Form
}
}
- if (Style.Language == FormatStyle::LK_JavaScript &&
+ if ((Style.Language == FormatStyle::LK_JavaScript ||
+ Style.Language == FormatStyle::LK_Proto ||
+ Style.Language == FormatStyle::LK_TextProto) &&
Tok.is(tok::char_constant)) {
Tok.Tok.setKind(tok::string_literal);
}
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=327255&r1=327254&r2=327255&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Mon Mar 12 03:32:18 2018
@@ -2912,7 +2912,7 @@ bool TokenAnnotator::canBreakBefore(cons
if (Left.is(tok::colon) && Left.isOneOf(TT_DictLiteral, TT_ObjCMethodExpr)) {
if ((Style.Language == FormatStyle::LK_Proto ||
Style.Language == FormatStyle::LK_TextProto) &&
-Right.isStringLiteral())
+!Style.AlwaysBreakBeforeMultilineStrings && Right.isStringLiteral())
return false;
return true;
}
Modified: cfe/trunk/unittests/Format/FormatTestProto.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestProto.cpp?rev=327255&r1=327254&r2=327255&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestProto.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestProto.cpp Mon Mar 12 03:32:18 2018
@@ -158,9 +158,8 @@ TEST_F(FormatTestProto, MessageFieldAttr
"key: 'a' //\n"
" }\n"
"];");
- verifyFormat("optional string test = 1 [default =\n"
- " \"test\"\n"
- " \"test\"];");
+ verifyFormat("optional string test = 1 [default = \"test\"\n"
+ "\"test\"];");
verifyFormat("optional Aaaa = 12 [\n"
" (aaa) = ,\n"
" (bb) = {\n"
Modified: cfe/trunk/unittests/Format/FormatTestTextProto.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestTextProto.cpp?rev=327255&r1=327254&r2=327255&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestTextProto.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestTextProto.cpp Mon Mar 12 03:32:18 2018
@@ -143,6 +143,23 @@ TEST_F(FormatTestTextProto, AddsNewlines
"}");
}
+TEST_F(FormatTestTextProto, ImplicitStringLiteralConcatenation) {
+ verifyFormat("field_a: 'a'\n"
+ " 'b'");
+ verifyFormat("field_a: \"a\"\n"
+ " \"b\"");
+ FormatStyle Style = getGoogleStyle(
r311792 - [Format] Invert nestingAndIndentLevel pair in WhitespaceManager used for
Author: djasper
Date: Fri Aug 25 12:14:53 2017
New Revision: 311792
URL: http://llvm.org/viewvc/llvm-project?rev=311792&view=rev
Log:
[Format] Invert nestingAndIndentLevel pair in WhitespaceManager used for
alignments
Indent should be compared before nesting level to determine if a token
is on the same scope as the one we align with. Because it was inverted,
clang-format sometimes tried to align tokens with tokens from outer
scopes, causing the assert(Shift >= 0) to fire.
This fixes bug #33507. Patch by Beren Minor, thank you!
Modified:
cfe/trunk/lib/Format/WhitespaceManager.cpp
cfe/trunk/lib/Format/WhitespaceManager.h
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/WhitespaceManager.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.cpp?rev=311792&r1=311791&r2=311792&view=diff
==
--- cfe/trunk/lib/Format/WhitespaceManager.cpp (original)
+++ cfe/trunk/lib/Format/WhitespaceManager.cpp Fri Aug 25 12:14:53 2017
@@ -246,12 +246,12 @@ AlignTokenSequence(unsigned Start, unsig
for (unsigned i = Start; i != End; ++i) {
if (ScopeStack.size() != 0 &&
-Changes[i].nestingAndIndentLevel() <
-Changes[ScopeStack.back()].nestingAndIndentLevel())
+Changes[i].indentAndNestingLevel() <
+Changes[ScopeStack.back()].indentAndNestingLevel())
ScopeStack.pop_back();
-if (i != Start && Changes[i].nestingAndIndentLevel() >
- Changes[i - 1].nestingAndIndentLevel())
+if (i != Start && Changes[i].indentAndNestingLevel() >
+ Changes[i - 1].indentAndNestingLevel())
ScopeStack.push_back(i);
bool InsideNestedScope = ScopeStack.size() != 0;
@@ -327,8 +327,8 @@ static unsigned AlignTokens(const Format
// Measure the scope level (i.e. depth of (), [], {}) of the first token, and
// abort when we hit any token in a higher scope than the starting one.
- auto NestingAndIndentLevel = StartAt < Changes.size()
- ? Changes[StartAt].nestingAndIndentLevel()
+ auto IndentAndNestingLevel = StartAt < Changes.size()
+ ? Changes[StartAt].indentAndNestingLevel()
: std::pair(0, 0);
// Keep track of the number of commas before the matching tokens, we will
only
@@ -359,7 +359,7 @@ static unsigned AlignTokens(const Format
unsigned i = StartAt;
for (unsigned e = Changes.size(); i != e; ++i) {
-if (Changes[i].nestingAndIndentLevel() < NestingAndIndentLevel)
+if (Changes[i].indentAndNestingLevel() < IndentAndNestingLevel)
break;
if (Changes[i].NewlinesBefore != 0) {
@@ -375,7 +375,7 @@ static unsigned AlignTokens(const Format
if (Changes[i].Tok->is(tok::comma)) {
++CommasBeforeMatch;
-} else if (Changes[i].nestingAndIndentLevel() > NestingAndIndentLevel) {
+} else if (Changes[i].indentAndNestingLevel() > IndentAndNestingLevel) {
// Call AlignTokens recursively, skipping over this scope block.
unsigned StoppedAt = AlignTokens(Style, Matches, Changes, i);
i = StoppedAt - 1;
Modified: cfe/trunk/lib/Format/WhitespaceManager.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.h?rev=311792&r1=311791&r2=311792&view=diff
==
--- cfe/trunk/lib/Format/WhitespaceManager.h (original)
+++ cfe/trunk/lib/Format/WhitespaceManager.h Fri Aug 25 12:14:53 2017
@@ -154,12 +154,11 @@ public:
const Change *StartOfBlockComment;
int IndentationOffset;
-// A combination of nesting level and indent level, which are used in
+// A combination of indent level and nesting level, which are used in
// tandem to compute lexical scope, for the purposes of deciding
// when to stop consecutive alignment runs.
-std::pair
-nestingAndIndentLevel() const {
- return std::make_pair(Tok->NestingLevel, Tok->IndentLevel);
+std::pair indentAndNestingLevel() const {
+ return std::make_pair(Tok->IndentLevel, Tok->NestingLevel);
}
};
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=311792&r1=311791&r2=311792&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Fri Aug 25 12:14:53 2017
@@ -8958,6 +8958,16 @@ TEST_F(FormatTest, AlignConsecutiveDecla
Alignment);
Alignment.BinPackParameters = true;
Alignment.ColumnLimit = 80;
+
+ // Bug 33507
+ Alignment.PointerAlignment = FormatStyle::PAS_Middle;
+ verifyFormat(
+ "auto found = range::find_if(vsProducts, [&](auto * aProduct) {\n"
+ " static const Version verVs2017;\n"
+
r312437 - clang-format: Fix formatting of for loops with multiple increments.
Author: djasper
Date: Sun Sep 3 01:56:24 2017
New Revision: 312437
URL: http://llvm.org/viewvc/llvm-project?rev=312437&view=rev
Log:
clang-format: Fix formatting of for loops with multiple increments.
This fixes llvm.org/PR34366.
Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=312437&r1=312436&r2=312437&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Sun Sep 3 01:56:24 2017
@@ -1116,6 +1116,11 @@ private:
(!Line.MightBeFunctionDecl || Current.NestingLevel != 0)) {
Contexts.back().FirstStartOfName = &Current;
Current.Type = TT_StartOfName;
+} else if (Current.is(tok::semi)) {
+ // Reset FirstStartOfName after finding a semicolon so that a for loop
+ // with multiple increment statements is not confused with a for loop
+ // having multiple variable declarations.
+ Contexts.back().FirstStartOfName = nullptr;
} else if (Current.isOneOf(tok::kw_auto, tok::kw___auto_type)) {
AutoFound = true;
} else if (Current.is(tok::arrow) &&
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=312437&r1=312436&r2=312437&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Sun Sep 3 01:56:24 2017
@@ -657,6 +657,10 @@ TEST_F(FormatTest, FormatsForLoop) {
" I != E;\n"
" ++I) {\n}",
NoBinPacking);
+
+ FormatStyle AlignLeft = getLLVMStyle();
+ AlignLeft.PointerAlignment = FormatStyle::PAS_Left;
+ verifyFormat("for (A* a = start; a < end; ++a, ++value) {\n}", AlignLeft);
}
TEST_F(FormatTest, RangeBasedForLoops) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r312484 - clang-format: Fix indentation of macros in include guards (after r312125).
Author: djasper
Date: Mon Sep 4 06:33:52 2017
New Revision: 312484
URL: http://llvm.org/viewvc/llvm-project?rev=312484&view=rev
Log:
clang-format: Fix indentation of macros in include guards (after r312125).
Before:
#ifndef A_H
#define A_H
#define A() \
int i;\
int j;
#endif // A_H
After:
#ifndef A_H
#define A_H
#define A() \
int i;\
int j;
#endif // A_H
Modified:
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=312484&r1=312483&r2=312484&view=diff
==
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Mon Sep 4 06:33:52 2017
@@ -742,12 +742,11 @@ void UnwrappedLineParser::parsePPEndIf()
// preprocessor indent.
unsigned TokenPosition = Tokens->getPosition();
FormatToken *PeekNext = AllTokens[TokenPosition];
- if (FoundIncludeGuardStart && PPBranchLevel == -1 && PeekNext->is(tok::eof))
{
-for (auto &Line : Lines) {
+ if (FoundIncludeGuardStart && PPBranchLevel == -1 && PeekNext->is(tok::eof)
&&
+ Style.IndentPPDirectives != FormatStyle::PPDIS_None)
+for (auto &Line : Lines)
if (Line.InPPDirective && Line.Level > 0)
--Line.Level;
-}
- }
}
void UnwrappedLineParser::parsePPDefine() {
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=312484&r1=312483&r2=312484&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Sep 4 06:33:52 2017
@@ -2332,7 +2332,6 @@ TEST_F(FormatTest, IndentPreprocessorDir
"#define A 1\n"
"#endif",
Style);
-
Style.IndentPPDirectives = FormatStyle::PPDIS_AfterHash;
verifyFormat("#ifdef _WIN32\n"
"# define A 0\n"
@@ -2493,6 +2492,15 @@ TEST_F(FormatTest, IndentPreprocessorDir
"#\tdefine A 1\n"
"#endif",
Style);
+
+ // Regression test: Multiline-macro inside include guards.
+ verifyFormat("#ifndef HEADER_H\n"
+ "#define HEADER_H\n"
+ "#define A()\\\n"
+ " int i; \\\n"
+ " int j;\n"
+ "#endif // HEADER_H",
+ getLLVMStyleWithColumns(20));
}
TEST_F(FormatTest, FormatHashIfNotAtStartOfLine) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r312721 - [clang-format] Fix documentation for AllowAllParametersOfDeclarationOnNextLine
Author: djasper
Date: Thu Sep 7 06:45:41 2017
New Revision: 312721
URL: http://llvm.org/viewvc/llvm-project?rev=312721&view=rev
Log:
[clang-format] Fix documentation for AllowAllParametersOfDeclarationOnNextLine
The current description of AllowAllParametersOfDeclarationOnNextLine in
the Clang-Format Style Options guide suggests that it is possible to
format function declaration, which fits in a single line (what is not
supported in current clang-format version). Also the example was not
reproducible and mades no sense.
Patch by Lucja Mazur, thank you!
Modified:
cfe/trunk/docs/ClangFormatStyleOptions.rst
cfe/trunk/include/clang/Format/Format.h
Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=312721&r1=312720&r2=312721&view=diff
==
--- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst Thu Sep 7 06:45:41 2017
@@ -271,15 +271,22 @@ the configuration (without a prefix: ``A
int b = 2; // comment bint b = 2; // comment about b
**AllowAllParametersOfDeclarationOnNextLine** (``bool``)
- Allow putting all parameters of a function declaration onto
+ If the function declaration doesn't fit on a line,
+ allow putting all parameters of a function declaration onto
the next line even if ``BinPackParameters`` is ``false``.
.. code-block:: c++
-true: false:
-myFunction(foo, vs. myFunction(foo, bar, plop);
- bar,
- plop);
+true:
+void myFunction(
+int a, int b, int c, int d, int e);
+
+false:
+void myFunction(int a,
+int b,
+int c,
+int d,
+int e);
**AllowShortBlocksOnASingleLine** (``bool``)
Allows contracting simple braced statements to a single line.
Modified: cfe/trunk/include/clang/Format/Format.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=312721&r1=312720&r2=312721&view=diff
==
--- cfe/trunk/include/clang/Format/Format.h (original)
+++ cfe/trunk/include/clang/Format/Format.h Thu Sep 7 06:45:41 2017
@@ -151,13 +151,20 @@ struct FormatStyle {
/// \endcode
bool AlignTrailingComments;
- /// \brief Allow putting all parameters of a function declaration onto
+ /// \brief If the function declaration doesn't fit on a line,
+ /// allow putting all parameters of a function declaration onto
/// the next line even if ``BinPackParameters`` is ``false``.
/// \code
- /// true: false:
- /// myFunction(foo, vs. myFunction(foo, bar, plop);
- /// bar,
- /// plop);
+ /// true:
+ /// void myFunction(
+ /// int a, int b, int c, int d, int e);
+ ///
+ /// false:
+ /// void myFunction(int a,
+ /// int b,
+ /// int c,
+ /// int d,
+ /// int e);
/// \endcode
bool AllowAllParametersOfDeclarationOnNextLine;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r312766 - Update for llvm change.
Hi Rafael,
could you write slightly more extensive commit messages? This really
doesn't say anything..
Thank you,
Daniel
On Fri, Sep 8, 2017 at 2:01 AM, Rafael Espindola via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Author: rafael
> Date: Thu Sep 7 17:01:26 2017
> New Revision: 312766
>
> URL: http://llvm.org/viewvc/llvm-project?rev=312766&view=rev
> Log:
> Update for llvm change.
>
> Modified:
> cfe/trunk/tools/clang-format/ClangFormat.cpp
> cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp
>
> Modified: cfe/trunk/tools/clang-format/ClangFormat.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-
> format/ClangFormat.cpp?rev=312766&r1=312765&r2=312766&view=diff
>
> ==
> --- cfe/trunk/tools/clang-format/ClangFormat.cpp (original)
> +++ cfe/trunk/tools/clang-format/ClangFormat.cpp Thu Sep 7 17:01:26 2017
> @@ -351,8 +351,10 @@ int main(int argc, const char **argv) {
>"together with s, the files are edited in-place. Otherwise,
> the\n"
>"result is written to the standard output.\n");
>
> - if (Help)
> + if (Help) {
> cl::PrintHelpMessage();
> +return 0;
> + }
>
>if (DumpConfig) {
> llvm::Expected FormatStyle =
>
> Modified: cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-
> offload-bundler/ClangOffloadBundler.cpp?rev=312766&r1=312765&r2=312766&
> view=diff
>
> ==
> --- cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp
> (original)
> +++ cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp Thu
> Sep 7 17:01:26 2017
> @@ -931,8 +931,10 @@ int main(int argc, const char **argv) {
>"one. The resulting file can also be unbundled into different files
> by \n"
>"this tool if -unbundle is provided.\n");
>
> - if (Help)
> + if (Help) {
> cl::PrintHelpMessage();
> +return 0;
> + }
>
>bool Error = false;
>if (Unbundle) {
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r312897 - Revert r312830: "Reinstall the patch "Use EmitPointerWithAlignment to get alignment information of the pointer used in atomic expr"."
Author: djasper
Date: Mon Sep 11 00:35:01 2017
New Revision: 312897
URL: http://llvm.org/viewvc/llvm-project?rev=312897&view=rev
Log:
Revert r312830: "Reinstall the patch "Use EmitPointerWithAlignment to get
alignment information of the pointer used in atomic expr"."
This triggers llvm.org/PR31620 in several of our internal builds. I'll
forward reproduction instructions to the original author.
Removed:
cfe/trunk/test/CodeGenCXX/atomic-align.cpp
Modified:
cfe/trunk/lib/CodeGen/CGAtomic.cpp
Modified: cfe/trunk/lib/CodeGen/CGAtomic.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGAtomic.cpp?rev=312897&r1=312896&r2=312897&view=diff
==
--- cfe/trunk/lib/CodeGen/CGAtomic.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGAtomic.cpp Mon Sep 11 00:35:01 2017
@@ -745,20 +745,20 @@ RValue CodeGenFunction::EmitAtomicExpr(A
QualType MemTy = AtomicTy;
if (const AtomicType *AT = AtomicTy->getAs())
MemTy = AT->getValueType();
- llvm::Value *IsWeak = nullptr, *OrderFail = nullptr;
-
- Address Val1 = Address::invalid();
- Address Val2 = Address::invalid();
- Address Dest = Address::invalid();
- Address Ptr = EmitPointerWithAlignment(E->getPtr());
-
CharUnits sizeChars, alignChars;
std::tie(sizeChars, alignChars) = getContext().getTypeInfoInChars(AtomicTy);
uint64_t Size = sizeChars.getQuantity();
unsigned MaxInlineWidthInBits = getTarget().getMaxAtomicInlineWidth();
- bool UseLibcall = ((Ptr.getAlignment() % sizeChars) != 0 ||
+ bool UseLibcall = (sizeChars != alignChars ||
getContext().toBits(sizeChars) > MaxInlineWidthInBits);
+ llvm::Value *IsWeak = nullptr, *OrderFail = nullptr;
+
+ Address Val1 = Address::invalid();
+ Address Val2 = Address::invalid();
+ Address Dest = Address::invalid();
+ Address Ptr(EmitScalarExpr(E->getPtr()), alignChars);
+
if (E->getOp() == AtomicExpr::AO__c11_atomic_init ||
E->getOp() == AtomicExpr::AO__opencl_atomic_init) {
LValue lvalue = MakeAddrLValue(Ptr, AtomicTy);
Removed: cfe/trunk/test/CodeGenCXX/atomic-align.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/atomic-align.cpp?rev=312896&view=auto
==
--- cfe/trunk/test/CodeGenCXX/atomic-align.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/atomic-align.cpp (removed)
@@ -1,30 +0,0 @@
-// RUN: %clang_cc1 %s -std=c++11 -emit-llvm -o - -triple=x86_64-linux-gnu |
FileCheck %s
-
-struct AM {
- int f1, f2;
-};
-alignas(8) AM m;
-AM load1() {
- AM am;
- // m is declared to align to 8bytes, so generate load atomic instead
- // of libcall.
- // CHECK-LABEL: @_Z5load1v
- // CHECK: load atomic {{.*}} monotonic
- __atomic_load(&m, &am, 0);
- return am;
-}
-
-struct BM {
- int f1;
- alignas(8) AM f2;
-};
-BM bm;
-AM load2() {
- AM am;
- // BM::f2 is declared to align to 8bytes, so generate load atomic instead
- // of libcall.
- // CHECK-LABEL: @_Z5load2v
- // CHECK: load atomic {{.*}} monotonic
- __atomic_load(&bm.f2, &am, 0);
- return am;
-}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24663: When replacements have the same offset, make replacements with smaller length order first in the set.
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. Comment at: lib/Tooling/Core/Replacement.cpp:407 @@ -409,3 +406,3 @@ bool Result = true; - for (Replacements::const_iterator I = Replaces.begin(), -E = Replaces.end(); + for (Replacements::const_reverse_iterator I = Replaces.rbegin(), +E = Replaces.rend(); Maybe use auto? https://reviews.llvm.org/D24663 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24703: [clang-format] BreakBeforeBinaryOperations and AlignAfterOpenBracket conflict, bug 30304
djasper added a comment. I think, this is the wrong fix. Instead, a || (Left.is(TT_TemplateOpener) && !Right.is(TT_TemplateCloser)) should be added to the last return statement of this function. Also, could you please add a test in unittests/Format/FormatTest.cpp. https://reviews.llvm.org/D24703 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24703: [clang-format] BreakBeforeBinaryOperations and AlignAfterOpenBracket conflict, bug 30304
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. Thank you! https://reviews.llvm.org/D24703 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24708: clang-format: [JS] Fix line breaks before comments when sorting imports.
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks Good. https://reviews.llvm.org/D24708 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24606: Recommit r281457 "Supports adding insertion around non-insertion replacements".
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. https://reviews.llvm.org/D24606 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24717: Merge deletions that are contained in a larger deletion.
djasper added a comment. Thinking about this some more, starting to merge deletions now, but only some of them is a bit suspect. I think we either want to allow even more or continue to be restrictive for now. I think fundamentally, there are two questions that we need to answer: 1. Is this something that the user/tool author would likely want to do? 2. Is add the replacement order-dependent in any way? I have no clue about #1, I'd have to see use cases. E.g. what use case are you trying to solve here? But lets look at #2: I think I have come up with an easy definition of what makes something order-dependent. Lets assume we have two replacements A and B that both refer to the same original code (I am using A and B as single replacements or as sets of a single replacement for simplicity). The question is whether A.add(B) is order-dependent. I think we should define this as (assuming we have a function that shifts a replacement by another replacement like your getReplacementInChangedCode from https://reviews.llvm.org/D24383): A.add(B) is order-dependent (and thus should conflict, if A.merge(getReplacementInChangedCode(B)) != B.merge(getReplacementInChangedCode(A)). I think, this enables exactly the kinds of additions that we have so far enabled, which seems good. It also enables overlapping deletions, e.g. deleting range [0-2] and [1-3] will result in deleting [0-3], not matter in which order. https://reviews.llvm.org/D24717 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24717: Merge deletions that are contained in a larger deletion.
djasper added a comment.
I actually think this is a good example. So lets assume we'd write a tool to
fully quote binary expressions, e.g. that turns
if (a * b + c * d == 10) ...
into
if (((a * b) + (c * d)) == 10) ...
So, here, we would be inserting two "(" and two ")" at the same locations. And,
as you correctly mention, the order doesn't matter because we are inserting the
same string twice. I think this is actually good behavior.
Deduplication is an interesting concern, but I think we probably want to handle
that at a different layer. E.g. in the use case above, deduplicating would be
quite fatal :).
https://reviews.llvm.org/D24717
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24767: clang-format: [JS] do not wrapp @returns tags.
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. https://reviews.llvm.org/D24767 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24804: clang-format: [JS] reserved words in method names.
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. Thank you. https://reviews.llvm.org/D24804 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24829: [clang-format] support header deletion in cleanupAroundReplacemnts.
djasper accepted this revision.
djasper added a comment.
This revision is now accepted and ready to land.
Looks good.
Comment at: lib/Format/Format.cpp:1557
@@ +1556,3 @@
+ const std::set HeadersToDelete) {
+ if (HeadersToDelete.find(HeaderName) != HeadersToDelete.end())
+return true;
return HeaderToDelete.find(HeaderName) != HeadersToDelete.end() ||
HeaderToDelete.find(HeaderName.trim("\"<>")) != HeadersToDelete.end()
Comment at: lib/Format/Format.cpp:1623
@@ -1607,2 +1622,3 @@
if (IncludeRegex.match(Line, &Matches)) {
+ // The header name with quptos or angle brackets.
StringRef IncludeName = Matches[2];
quotes.
Comment at: lib/Format/Format.cpp:1634
@@ +1633,3 @@
+// sure we don't delete across the file boundary.
+unsigned Length = (Offset + Line.size() < Code.size()) ? Line.size() +
1
+ : Line.size();
Maybe:
unsigned Length = std::min(Line.size(), Code.size() - Offset - 1);
https://reviews.llvm.org/D24829
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes
djasper added a comment. Are we talking completely past each other? I specifically think we should *NOT* combine NestingLevel and IndentLevel into one value. Not in ScopeLevel() and not anywhere else. https://reviews.llvm.org/D21279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r282410 - clang-format: Only special-case top-level */& in multivar-declstmts.
Author: djasper
Date: Mon Sep 26 10:14:24 2016
New Revision: 282410
URL: http://llvm.org/viewvc/llvm-project?rev=282410&view=rev
Log:
clang-format: Only special-case top-level */& in multivar-declstmts.
Before (even with PointerAlignment: Left):
vector a, b;
After:
vector a, b;
Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=282410&r1=282409&r2=282410&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Mon Sep 26 10:14:24 2016
@@ -2015,7 +2015,7 @@ bool TokenAnnotator::spaceRequiredBetwee
Left.Previous->is(tok::r_paren)) ||
(!Left.isOneOf(TT_PointerOrReference, tok::l_paren) &&
(Style.PointerAlignment != FormatStyle::PAS_Left ||
- Line.IsMultiVariableDeclStmt)));
+ (Line.IsMultiVariableDeclStmt && Left.NestingLevel == 0;
if (Right.is(TT_FunctionTypeLParen) && Left.isNot(tok::l_paren) &&
(!Left.is(TT_PointerOrReference) ||
(Style.PointerAlignment != FormatStyle::PAS_Right &&
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=282410&r1=282409&r2=282410&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Sep 26 10:14:24 2016
@@ -4802,6 +4802,7 @@ TEST_F(FormatTest, DeclarationsOfMultipl
verifyFormat("a *a = aaa, *b =
bbb,\n"
" *b = bbb, *d = ddd;",
Style);
+ verifyFormat("vector a, b;", Style);
}
TEST_F(FormatTest, ConditionalExpressionsInBrackets) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15643: [clang-format] Don't allow newline after uppercase Obj-C block return types
djasper added a comment. Ah, sorry for dropping this on the floor :(. May I nonetheless ask you to add a test (unittests/Format/FormatTest.cpp)? https://reviews.llvm.org/D15643 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r282448 - [clang-format] Don't allow newline after uppercase Obj-C block return types
Author: djasper
Date: Mon Sep 26 17:19:08 2016
New Revision: 282448
URL: http://llvm.org/viewvc/llvm-project?rev=282448&view=rev
Log:
[clang-format] Don't allow newline after uppercase Obj-C block return types
Fixes the following:
BOOL (^aaa)(void) = ^BOOL {
};
The first BOOL's token was getting set to TT_FunctionAnnotationRParen
incorrectly, which was causing an unexpected newline after (^aaa). This
was introduced in r245846.
Patch by Kent Sutherland, thank you!
Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=282448&r1=282447&r2=282448&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Mon Sep 26 17:19:08 2016
@@ -1037,12 +1037,17 @@ private:
!Current.Next->isBinaryOperator() &&
!Current.Next->isOneOf(tok::semi, tok::colon, tok::l_brace,
tok::period, tok::arrow, tok::coloncolon))
-if (FormatToken *BeforeParen = Current.MatchingParen->Previous)
- if (BeforeParen->is(tok::identifier) &&
- BeforeParen->TokenText == BeforeParen->TokenText.upper() &&
- (!BeforeParen->Previous ||
- BeforeParen->Previous->ClosesTemplateDeclaration))
-Current.Type = TT_FunctionAnnotationRParen;
+if (FormatToken *AfterParen = Current.MatchingParen->Next) {
+ // Make sure this isn't the return type of an Obj-C block declaration
+ if (AfterParen->Tok.isNot(tok::caret)) {
+if (FormatToken *BeforeParen = Current.MatchingParen->Previous)
+ if (BeforeParen->is(tok::identifier) &&
+ BeforeParen->TokenText == BeforeParen->TokenText.upper() &&
+ (!BeforeParen->Previous ||
+ BeforeParen->Previous->ClosesTemplateDeclaration))
+Current.Type = TT_FunctionAnnotationRParen;
+ }
+}
} else if (Current.is(tok::at) && Current.Next) {
if (Current.Next->isStringLiteral()) {
Current.Type = TT_ObjCStringLiteral;
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=282448&r1=282447&r2=282448&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Sep 26 17:19:08 2016
@@ -11156,6 +11156,8 @@ TEST_F(FormatTest, FormatsBlocks) {
" }\n"
"});");
verifyFormat("Block b = ^int *(A *a, B *b) {}");
+ verifyFormat("BOOL (^aaa)(void) = ^BOOL {\n"
+ "};");
FormatStyle FourIndent = getLLVMStyle();
FourIndent.ObjCBlockIndentWidth = 4;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15643: [clang-format] Don't allow newline after uppercase Obj-C block return types
djasper closed this revision. djasper added a comment. Committed as r282448. https://reviews.llvm.org/D15643 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r283246 - clang-format: Fix bad multi-variable for-loop formatting.
Author: djasper
Date: Tue Oct 4 15:18:25 2016
New Revision: 283246
URL: http://llvm.org/viewvc/llvm-project?rev=283246&view=rev
Log:
clang-format: Fix bad multi-variable for-loop formatting.
Before:
for (int*p, *q; p != q; p = p->next) {
After:
for (int *p, *q; p != q; p = p->next) {
Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=283246&r1=283245&r2=283246&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Tue Oct 4 15:18:25 2016
@@ -2020,7 +2020,9 @@ bool TokenAnnotator::spaceRequiredBetwee
Left.Previous->is(tok::r_paren)) ||
(!Left.isOneOf(TT_PointerOrReference, tok::l_paren) &&
(Style.PointerAlignment != FormatStyle::PAS_Left ||
- (Line.IsMultiVariableDeclStmt && Left.NestingLevel == 0;
+ (Line.IsMultiVariableDeclStmt &&
+ (Left.NestingLevel == 0 ||
+(Left.NestingLevel == 1 && Line.First->is(tok::kw_for)));
if (Right.is(TT_FunctionTypeLParen) && Left.isNot(tok::l_paren) &&
(!Left.is(TT_PointerOrReference) ||
(Style.PointerAlignment != FormatStyle::PAS_Right &&
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=283246&r1=283245&r2=283246&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Oct 4 15:18:25 2016
@@ -4803,6 +4803,7 @@ TEST_F(FormatTest, DeclarationsOfMultipl
" *b = bbb, *d = ddd;",
Style);
verifyFormat("vector a, b;", Style);
+ verifyFormat("for (int *p, *q; p != q; p = p->next) {\n}", Style);
}
TEST_F(FormatTest, ConditionalExpressionsInBrackets) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes
djasper added a comment.
So sorry. Seems I forgot to hit "Submit" :(.
If you don't like the ".first" and ".second" of the pair, you could introduce a
struct for it and overload operator<. Might actually be more readable.
> WhitespaceManager.cpp:73
> + Tok.NestingLevel,
>/*Spaces=*/0, Tok.OriginalColumn, Tok.NewlinesBefore, "", "",
> + Tok.Tok.getKind(), Tok.Type, InPPDirective && !Tok.IsFirst,
nit: Move these to the previous line. clang-format won't do that, because of
the comment, but that's actually irrelevant here.
> WhitespaceManager.cpp:183
> +
> +// NestingLevel is raised on the opening paren/square, and remains raised
> +// until AFTER the closing paren/square. This means that with a statement
I don't see why this would be necessary. If I remove it, all tests do still
pass.
> WhitespaceManager.cpp:190
> +//
> +// The "int x = 1" line is going to have the same NestingLevel as
> +// the tokens inside the parentheses of the "for" statement.
Also, this comment seems wrong? The "int x = 1;" actually starts a new (child)
line. If that has the same nesting level, that seems like a bug we need to fix.
> WhitespaceManager.cpp:210
> + // We only run the "Matches" function on tokens from the outer-most scope.
> + // However, we do need to adjust some special tokens within the entire
> + // Start..End range, regardless of their scope. This special rule applies
Make this (and maybe a few others) more concrete. Don't write "some special
tokens", write what they actually are.
> WhitespaceManager.cpp:214
> + // who's function names have been shifted for declaration alignment's sake.
> + // See "split function parameter alignment" in FormatTest.cpp for an
> example.
> + SmallVector ScopeStack;
If the example isn't too long, writing the source code in the comment seems
better than referencing the test.
> WhitespaceManager.cpp:389
>
> - EndOfSequence = Changes.size();
> + unsigned StoppedAt = i;
> + EndOfSequence = i;
Either do:
EndOfSequence = StoppedAt;
or just remove StoppedAt and use i.
> FormatTest.cpp:9364
> + // WhitespaceManager.cpp
> + EXPECT_EQ("double a(int x);\n"
> +"intb(intx,\n"
Can you add a test case where there is a line wrap after the "("?
https://reviews.llvm.org/D21279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25171: clang-format: Add two new formatting options
djasper added a comment. Could you read: http://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options And provide some evidence about the requirements for new style options? https://reviews.llvm.org/D25171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D19063: clang-format: Fixed line merging of more than two lines
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Sorry for the delay. Looks good. https://reviews.llvm.org/D19063 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D19066: clang-format: `SpaceAfterTemplate` and `SpacesInBraces` options
djasper added a comment. Ping? SpaceAfterTemplateKeyword exists by now. https://reviews.llvm.org/D19066 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25162: Make DeletedLines local variables in checkEmptyNamespace.
djasper added a comment.
I think we should add a test for why this was a problem. IIUC, this is caused
by something like:
#if a
#else
#endif
namespace {
}
Because we actually analyze the whole thing twice.
https://reviews.llvm.org/D25162
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21026: [clang-format] append newline after code when inserting new headers at the end of the code which does not end with newline.
djasper added a comment. Ping? https://reviews.llvm.org/D21026 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D19385: [scan-build] fix warnings emitted on Clang Format code base
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. https://reviews.llvm.org/D19385 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21026: [clang-format] append newline after code when inserting new headers at the end of the code which does not end with newline.
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. https://reviews.llvm.org/D21026 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25162: Make DeletedLines local variables in checkEmptyNamespace.
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. https://reviews.llvm.org/D25162 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25171: clang-format: Add two new formatting options
djasper added a comment. Sorry, but that's actually not enough, at least at first sight. With 37 contributors total, bro is still quite small and only 12 of them have more than a handful of commits. And it doesn't have a real style guide. It has: https://www.bro.org/development/contribute.html#coding-guidelines, which basically says "adapt to the existing code structure" and "We don’t have many strict rules". https://reviews.llvm.org/D25171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25171: clang-format: Add two new formatting options
djasper added a comment.
It's not about whether or not we like the patch. It's whether adding these
options is a good trade-off for clang-format overall. If we find that actually
more people would find these styles desirable, we can reconsider.
I have left some comments anyway in case you want to keep the patch around.
> Format.h:603
>
> + /// \brief If ``true``, spaces will be inserted around if/for/while
> conditions.
> + bool SpacesAroundConditions;
It's actually more than if/for/while.
> Format.cpp:355
> IO.mapOptional("SpacesInSquareBrackets", Style.SpacesInSquareBrackets);
> +IO.mapOptional("SpacesAfterNot",
> + Style.SpacesAfterNot);
Unnecessary linebreaks.
> TokenAnnotator.cpp:1989
> +if (Left.is(tok::l_paren) && Left.Previous &&
> + Left.Previous->isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for,
> tok::kw_while,
> + tok::kw_switch, TT_ForEachMacro))
Indent is off.
> TokenAnnotator.cpp:1989-1990
> +if (Left.is(tok::l_paren) && Left.Previous &&
> + Left.Previous->isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for,
> tok::kw_while,
> + tok::kw_switch, TT_ForEachMacro))
> +return true;
This should go into a helper function or lambda so that it can be reused. What
about catch? Also some of these aren't tested, e.g. the preprocessor ones.
> TokenAnnotator.cpp:1993
> +if (Right.is(tok::r_paren) && Right.MatchingParen &&
> Right.MatchingParen->Previous &&
> + Right.MatchingParen->Previous->isOneOf(tok::kw_if, tok::pp_elif,
> tok::kw_for, tok::kw_while,
> + tok::kw_switch,
> TT_ForEachMacro))
Indent is off.
> TokenAnnotator.cpp:2232
>}
> + if (Style.SpacesAfterNot && Left.is(tok::exclaim))
> +return true;
Note that at least JavaScript/TypeScript has a ! postfix operator, i.e. unary
operator that applies to the token before it. You definitely don't want to
always add a space after that.
> TokenAnnotator.cpp:2233
> + if (Style.SpacesAfterNot && Left.is(tok::exclaim))
> +return true;
>if (Left.is(TT_UnaryOperator))
Indent is off.
> TokenAnnotator.cpp:2252
> return false;
> - if (Right.is(tok::coloncolon) && !Left.isOneOf(tok::l_brace, tok::comment))
> + if (Right.is(tok::coloncolon) && !Left.isOneOf(tok::l_brace, tok::comment,
> tok::l_paren))
> return (Left.is(TT_TemplateOpener) &&
Is this related in any way? I don't see a test with ::
> FormatTest.cpp:11508
> + Spaces.SpacesAfterNot = true;
> + verifyFormat("if (! a)\n return;", Spaces);
> + verifyFormat("while (! (x || y))\n return;", Spaces);
While you have added some test here, I think the more debatable ones are
actually where there is also a space before the !, e.g.:
if (a && ! b) ..
return ! c;
bool x = ! y && z;
The last one is especially tricky, because it makes it less clear that ! binds
stronger than &&. Might seem obvious in this case, but IIRC, we fought quite a
few bugs of the form:
if (! a < b) ..
Until a warning was added in Clang.
https://reviews.llvm.org/D25171
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r283743 - Revert "[x86][inline-asm][clang] accept 'v' constraint"
Author: djasper
Date: Mon Oct 10 06:40:28 2016
New Revision: 283743
URL: http://llvm.org/viewvc/llvm-project?rev=283743&view=rev
Log:
Revert "[x86][inline-asm][clang] accept 'v' constraint"
This reverts commit r283716.
Breaks buildbot:
http://lab.llvm.org:8080/green/job/clang-stage2-configure-Rlto_check/9155/testReport/junit/Clang/CodeGen/x86_inline_asm_v_constraint_c/
Removed:
cfe/trunk/test/CodeGen/x86-inline-asm-v-constraint.c
Modified:
cfe/trunk/lib/Basic/Targets.cpp
Modified: cfe/trunk/lib/Basic/Targets.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=283743&r1=283742&r2=283743&view=diff
==
--- cfe/trunk/lib/Basic/Targets.cpp (original)
+++ cfe/trunk/lib/Basic/Targets.cpp Mon Oct 10 06:40:28 2016
@@ -4005,7 +4005,6 @@ X86TargetInfo::validateAsmConstraint(con
case 'u': // Second from top of floating point stack.
case 'q': // Any register accessible as [r]l: a, b, c, and d.
case 'y': // Any MMX register.
- case 'v': // Any {X,Y,Z}MM register (Arch & context dependent)
case 'x': // Any SSE register.
case 'Q': // Any register accessible as [r]h: a, b, c, and d.
case 'R': // "Legacy" registers: ax, bx, cx, dx, di, si, sp, bp.
@@ -4046,7 +4045,6 @@ bool X86TargetInfo::validateOperandSize(
case 't':
case 'u':
return Size <= 128;
- case 'v':
case 'x':
if (SSELevel >= AVX512F)
// 512-bit zmm registers can be used if target supports AVX512F.
Removed: cfe/trunk/test/CodeGen/x86-inline-asm-v-constraint.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/x86-inline-asm-v-constraint.c?rev=283742&view=auto
==
--- cfe/trunk/test/CodeGen/x86-inline-asm-v-constraint.c (original)
+++ cfe/trunk/test/CodeGen/x86-inline-asm-v-constraint.c (removed)
@@ -1,30 +0,0 @@
-// RUN: %clang_cc1 %s -triple x86_64-unknown-linux-gnu -emit-llvm -target-cpu
x86-64 -o - | FileCheck %s --check-prefix SSE
-// RUN: %clang_cc1 %s -triple x86_64-unknown-linux-gnu -emit-llvm -target-cpu
skylake -D AVX -o - | FileCheck %s --check-prefixes AVX,SSE
-// RUN: %clang_cc1 %s -triple x86_64-unknown-linux-gnu -emit-llvm -target-cpu
skylake-avx512 -D AVX512 -D AVX -o - | FileCheck %s --check-prefixes
AVX512,AVX,SSE
-// RUN: %clang_cc1 %s -triple x86_64-unknown-linux-gnu -emit-llvm -target-cpu
knl -D AVX -D AVX512 -o - | FileCheck %s --check-prefixes AVX512,AVX,SSE
-
-typedef float __m128 __attribute__ ((vector_size (16)));
-typedef float __m256 __attribute__ ((vector_size (32)));
-typedef float __m512 __attribute__ ((vector_size (64)));
-
-// SSE: call <4 x float> asm "vmovhlps $1, $2, $0",
"=v,v,v,~{dirflag},~{fpsr},~{flags}"(i64 %0, <4 x float> %1)
-__m128 testXMM(__m128 _xmm0, long _l) {
- __asm__("vmovhlps %1, %2, %0" :"=v"(_xmm0) : "v"(_l), "v"(_xmm0));
- return _xmm0;
-}
-
-// AVX: call <8 x float> asm "vmovsldup $1, $0",
"=v,v,~{dirflag},~{fpsr},~{flags}"(<8 x float> %0)
-__m256 testYMM(__m256 _ymm0) {
-#ifdef AVX
- __asm__("vmovsldup %1, %0" :"=v"(_ymm0) : "v"(_ymm0));
-#endif
- return _ymm0;
-}
-
-// AVX512: call <16 x float> asm "vpternlogd $$0, $1, $2, $0",
"=v,v,v,~{dirflag},~{fpsr},~{flags}"(<16 x float> %0, <16 x float> %1)
-__m512 testZMM(__m512 _zmm0, __m512 _zmm1) {
-#ifdef AVX512
- __asm__("vpternlogd $0, %1, %2, %0" :"=v"(_zmm0) : "v"(_zmm1), "v"(_zmm0));
-#endif
- return _zmm0;
-}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r283716 - [x86][inline-asm][clang] accept 'v' constraint
I have reverted this in r283743 for now.
On Mon, Oct 10, 2016 at 1:43 PM, Ismail Donmez via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Hi,
>
> On Mon, Oct 10, 2016 at 8:45 AM, Michael Zuckerman via cfe-commits
> wrote:
> > Author: mzuckerm
> > Date: Mon Oct 10 00:45:54 2016
> > New Revision: 283716
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=283716&view=rev
> > Log:
> > [x86][inline-asm][clang] accept 'v' constraint
> >
> > Commit in the name of: Coby Tayree
> >
> > 1.'v' constraint for (x86) non-avx arch imitates the already implemented
> 'x' constraint, i.e. allows XMM{0-15} & YMM{0-15} depending on the apparent
> arch & mode (32/64).
> > 2.for the avx512 arch it allows [X,Y,Z]MM{0-31} (mode dependent)
> >
> > This patch applies the needed changes to clang
> > LLVM patch: https://reviews.llvm.org/D25005
> >
> > Differential Revision: D25004
>
> This fails on Linux x86-64:
>
> --
> Exit Code: 1
>
> Command Output (stderr):
> --
> /home/abuild/rpmbuild/BUILD/llvm/tools/clang/test/CodeGen/
> x86-inline-asm-v-constraint.c:10:9:
> error: expected string not found in input
> // SSE: call <4 x float> asm "vmovhlps $1, $2, $0",
> "=v,v,v,~{dirflag},~{fpsr},~{flags}"(i64 %0, <4 x float> %1)
> ^
> :1:1: note: scanning from here
> ; ModuleID = '/home/abuild/rpmbuild/BUILD/llvm/tools/clang/test/CodeGen/
> x86-inline-asm-v-constraint.c'
> ^
> :14:7: note: possible intended match here
> %3 = call <4 x float> asm "vmovhlps $1, $2, $0",
> "=v,v,v,~{dirflag},~{fpsr},~{flags}"(i64 %1, <4 x float> %2) #1,
> !srcloc !1
> ^
>
> --
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25439: Fixed column shift when formatting line containing bit shift operators
djasper added inline comments. Comment at: test/Format/bitshift-operator-width.cpp:1 +// RUN: grep -Ev "// *[A-Z-]+:" %s | clang-format -style=LLVM \ +// RUN: | FileCheck -strict-whitespace %s Could you move this test into unittests/Format/FormatTest.cpp? https://reviews.llvm.org/D25439 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r283853 - Explicitly ignore return code in test for test systems that use pipefail
Author: djasper
Date: Tue Oct 11 01:13:18 2016
New Revision: 283853
URL: http://llvm.org/viewvc/llvm-project?rev=283853&view=rev
Log:
Explicitly ignore return code in test for test systems that use pipefail
Modified:
cfe/trunk/test/Driver/show-option-names.c
Modified: cfe/trunk/test/Driver/show-option-names.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/show-option-names.c?rev=283853&r1=283852&r2=283853&view=diff
==
--- cfe/trunk/test/Driver/show-option-names.c (original)
+++ cfe/trunk/test/Driver/show-option-names.c Tue Oct 11 01:13:18 2016
@@ -1,5 +1,5 @@
-// RUN: %clang -c -target i386-apple-darwin10 -isysroot /FOO %s 2>&1 |
FileCheck --check-prefix=CHECK-SHOW-OPTION-NAMES %s
+// RUN: (%clang -c -target i386-apple-darwin10 -isysroot /FOO %s 2>&1 || true)
| FileCheck --check-prefix=CHECK-SHOW-OPTION-NAMES %s
// CHECK-SHOW-OPTION-NAMES: warning: no such sysroot directory:
'{{([A-Za-z]:.*)?}}/FOO' [-Wmissing-sysroot]
-// RUN: %clang -c -target i386-apple-darwin10 -fno-diagnostics-show-option
-isysroot /FOO %s 2>&1 | FileCheck --check-prefix=CHECK-NO-SHOW-OPTION-NAMES %s
+// RUN: (%clang -c -target i386-apple-darwin10 -fno-diagnostics-show-option
-isysroot /FOO %s 2>&1 || true) | FileCheck
--check-prefix=CHECK-NO-SHOW-OPTION-NAMES %s
// CHECK-NO-SHOW-OPTION-NAMES: warning: no such sysroot directory:
'{{([A-Za-z]:.*)?}}/FOO'{{$}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r283853 - Explicitly ignore return code in test for test systems that use pipefail
On Tue, Oct 11, 2016 at 1:38 PM, Renato Golin wrote: > On 11 October 2016 at 07:13, Daniel Jasper via cfe-commits > wrote: > > Author: djasper > > Date: Tue Oct 11 01:13:18 2016 > > New Revision: 283853 > > > > URL: http://llvm.org/viewvc/llvm-project?rev=283853&view=rev > > Log: > > Explicitly ignore return code in test for test systems that use pipefail > > Hi Daniel, > > I missed this commit and reverted the original patch. > > However, before we reapply, why not use "not" instead of "|| true" ? > Just because I don't know shell very well :). Feel free to use "not" instead. > > cheers, > --renato > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D7842: Make clang-format-diff compatible with Python 3.4
djasper accepted this revision. djasper added a reviewer: djasper. djasper added a comment. This revision is now accepted and ready to land. Looks good. https://reviews.llvm.org/D7842 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D20785: Python 3.5 compatibility for clang-format.py
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. Repository: rL LLVM https://reviews.llvm.org/D20785 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25565: Deduplicate sets of replacements by file names.
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. I'd not rename the function. Otherwise looks good. https://reviews.llvm.org/D25565 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r311070 - Fix undefined behavior that is caused by not always initializing a bool.
Author: djasper
Date: Wed Aug 16 23:33:46 2017
New Revision: 311070
URL: http://llvm.org/viewvc/llvm-project?rev=311070&view=rev
Log:
Fix undefined behavior that is caused by not always initializing a bool.
The fix in r310994 is incomplete, as moveFromAndCancel can set the
pointer without initializing OldIsSpeculativelyEvaluating.
Modified:
cfe/trunk/lib/AST/ExprConstant.cpp
Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=311070&r1=311069&r2=311070&view=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Wed Aug 16 23:33:46 2017
@@ -984,6 +984,7 @@ namespace {
void moveFromAndCancel(SpeculativeEvaluationRAII &&Other) {
Info = Other.Info;
OldStatus = Other.OldStatus;
+ OldIsSpeculativelyEvaluating = Other.OldIsSpeculativelyEvaluating;
Other.Info = nullptr;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r328200 - clang-format: Fix SpacesInParentheses with fully qualified names.
Author: djasper
Date: Thu Mar 22 07:30:28 2018
New Revision: 328200
URL: http://llvm.org/viewvc/llvm-project?rev=328200&view=rev
Log:
clang-format: Fix SpacesInParentheses with fully qualified names.
When SpacesInParentheses is set to true clang-format does not add a
space before fully qualified names. For example:
do_something(::globalVar );
Fix by Darby Payne. Thank you!
Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=328200&r1=328199&r2=328200&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Thu Mar 22 07:30:28 2018
@@ -2692,7 +2692,8 @@ bool TokenAnnotator::spaceRequiredBefore
Style.Standard == FormatStyle::LS_Cpp03) ||
!(Left.isOneOf(tok::l_paren, tok::r_paren, tok::l_square,
tok::kw___super, TT_TemplateCloser,
- TT_TemplateOpener));
+ TT_TemplateOpener)) ||
+ (Left.is(tok ::l_paren) && Style.SpacesInParentheses);
if ((Left.is(TT_TemplateOpener)) != (Right.is(TT_TemplateCloser)))
return Style.SpacesInAngles;
// Space before TT_StructuredBindingLSquare.
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=328200&r1=328199&r2=328200&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Mar 22 07:30:28 2018
@@ -8827,6 +8827,7 @@ TEST_F(FormatTest, ConfigurableSpacesInP
FormatStyle Spaces = getLLVMStyle();
Spaces.SpacesInParentheses = true;
+ verifyFormat("do_something( ::globalVar );", Spaces);
verifyFormat("call( x, y, z );", Spaces);
verifyFormat("call();", Spaces);
verifyFormat("std::function callback;", Spaces);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r328201 - clang-format: Narrow down raw string literal line break exception.
Author: djasper
Date: Thu Mar 22 07:43:54 2018
New Revision: 328201
URL: http://llvm.org/viewvc/llvm-project?rev=328201&view=rev
Log:
clang-format: Narrow down raw string literal line break exception.
For multiline raw string literals, we generally want to respect the
author's choice of linebreak before the 'R"(' as the rest of the raw
string might be aligned to it and we cannot (commonly) modify the
content.
For single-line raw string literals, this doesn't make any sense and so
we should just treat them as regular string literals in this regard.
Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=328201&r1=328200&r2=328201&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Thu Mar 22 07:43:54 2018
@@ -2830,10 +2830,10 @@ bool TokenAnnotator::mustBreakBefore(con
if (Style.BreakBeforeInheritanceComma && Right.is(TT_InheritanceComma))
return true;
if (Right.is(tok::string_literal) && Right.TokenText.startswith("R\""))
-// Raw string literals are special wrt. line breaks. The author has made a
-// deliberate choice and might have aligned the contents of the string
-// literal accordingly. Thus, we try keep existing line breaks.
-return Right.NewlinesBefore > 0;
+// Multiline raw string literals are special wrt. line breaks. The author
+// has made a deliberate choice and might have aligned the contents of the
+// string literal accordingly. Thus, we try keep existing line breaks.
+return Right.IsMultiline && Right.NewlinesBefore > 0;
if ((Right.Previous->is(tok::l_brace) ||
(Right.Previous->is(tok::less) && Right.Previous->Previous &&
Right.Previous->Previous->is(tok::equal))) &&
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=328201&r1=328200&r2=328201&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Mar 22 07:43:54 2018
@@ -8106,6 +8106,9 @@ TEST_F(FormatTest, CountsCharactersInMul
"multiline raw string literal xx\n"
")x\" + bb);",
getGoogleStyleWithColumns(20)));
+ EXPECT_EQ("fff(R\"(single line raw string)\" + bb);",
+format("fff(\n"
+ " R\"(single line raw string)\" + bb);"));
}
TEST_F(FormatTest, SkipsUnknownStringLiterals) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r317473 - [clang-format] Handle unary operator overload with arguments and specifiers
Author: djasper
Date: Mon Nov 6 04:11:51 2017
New Revision: 317473
URL: http://llvm.org/viewvc/llvm-project?rev=317473&view=rev
Log:
[clang-format] Handle unary operator overload with arguments and specifiers
Before:
int operator++(int)noexcept;
After:
int operator++(int) noexcept;
Patch by Igor Sugak. Thank you!
Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=317473&r1=317472&r2=317473&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Mon Nov 6 04:11:51 2017
@@ -696,7 +696,8 @@ private:
CurrentToken->Type = TT_PointerOrReference;
consumeToken();
if (CurrentToken &&
-CurrentToken->Previous->isOneOf(TT_BinaryOperator, tok::comma))
+CurrentToken->Previous->isOneOf(TT_BinaryOperator,
TT_UnaryOperator,
+tok::comma))
CurrentToken->Previous->Type = TT_OverloadedOperator;
}
if (CurrentToken) {
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=317473&r1=317472&r2=317473&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Nov 6 04:11:51 2017
@@ -5591,6 +5591,7 @@ TEST_F(FormatTest, UnderstandsOverloaded
verifyFormat("bool operator!=();");
verifyFormat("int operator+();");
verifyFormat("int operator++();");
+ verifyFormat("int operator++(int) volatile noexcept;");
verifyFormat("bool operator,();");
verifyFormat("bool operator();");
verifyFormat("bool operator()();");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r317901 - [clang-format] Handle leading comments in using declaration
Author: djasper
Date: Fri Nov 10 09:11:18 2017
New Revision: 317901
URL: http://llvm.org/viewvc/llvm-project?rev=317901&view=rev
Log:
[clang-format] Handle leading comments in using declaration
This fixes clang-format internal assertion for the following code:
/* override */ using std::string;
Patch by Igor Sugak. Thank you.
Modified:
cfe/trunk/lib/Format/UsingDeclarationsSorter.cpp
cfe/trunk/unittests/Format/UsingDeclarationsSorterTest.cpp
Modified: cfe/trunk/lib/Format/UsingDeclarationsSorter.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UsingDeclarationsSorter.cpp?rev=317901&r1=317900&r2=317901&view=diff
==
--- cfe/trunk/lib/Format/UsingDeclarationsSorter.cpp (original)
+++ cfe/trunk/lib/Format/UsingDeclarationsSorter.cpp Fri Nov 10 09:11:18 2017
@@ -172,15 +172,17 @@ std::pair UsingDeclarations;
for (size_t I = 0, E = AnnotatedLines.size(); I != E; ++I) {
+const auto *FirstTok = AnnotatedLines[I]->First;
if (AnnotatedLines[I]->InPPDirective ||
-!AnnotatedLines[I]->startsWith(tok::kw_using) ||
-AnnotatedLines[I]->First->Finalized) {
+!AnnotatedLines[I]->startsWith(tok::kw_using) || FirstTok->Finalized) {
endUsingDeclarationBlock(&UsingDeclarations, SourceMgr, &Fixes);
continue;
}
-if (AnnotatedLines[I]->First->NewlinesBefore > 1)
+if (FirstTok->NewlinesBefore > 1)
endUsingDeclarationBlock(&UsingDeclarations, SourceMgr, &Fixes);
-std::string Label = computeUsingDeclarationLabel(AnnotatedLines[I]->First);
+const auto *UsingTok =
+FirstTok->is(tok::comment) ? FirstTok->getNextNonComment() : FirstTok;
+std::string Label = computeUsingDeclarationLabel(UsingTok);
if (Label.empty()) {
endUsingDeclarationBlock(&UsingDeclarations, SourceMgr, &Fixes);
continue;
Modified: cfe/trunk/unittests/Format/UsingDeclarationsSorterTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/UsingDeclarationsSorterTest.cpp?rev=317901&r1=317900&r2=317901&view=diff
==
--- cfe/trunk/unittests/Format/UsingDeclarationsSorterTest.cpp (original)
+++ cfe/trunk/unittests/Format/UsingDeclarationsSorterTest.cpp Fri Nov 10
09:11:18 2017
@@ -348,6 +348,13 @@ TEST_F(UsingDeclarationsSorterTest, Sort
{tooling::Range(19, 1)}));
}
+TEST_F(UsingDeclarationsSorterTest,
SortsUsingDeclarationsWithLeadingkComments) {
+ EXPECT_EQ("/* comment */ using a;\n"
+"/* comment */ using b;",
+sortUsingDeclarations("/* comment */ using b;\n"
+ "/* comment */ using a;"));
+}
+
} // end namespace
} // end namespace format
} // end namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r290084 - clang-format: Allow "single column" list layout even if that violates the
Author: djasper
Date: Mon Dec 19 01:26:11 2016
New Revision: 290084
URL: http://llvm.org/viewvc/llvm-project?rev=290084&view=rev
Log:
clang-format: Allow "single column" list layout even if that violates the
column limit.
Single-column layout basically means that we format the list with one
element per line. Not doing that when there is a column limit violation
doesn't change the fact that there is an item that doesn't fit within
the column limit.
Before (with a column limit of 30):
std::vector a = {
, ,
, ,
aa, ,
aaa};
After:
std::vector a = {
,
,
,
,
aa,
,
aaa};
(and previously we would have formatted like "After" it wasn't for the one
item that is too long)
Modified:
cfe/trunk/lib/Format/FormatToken.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/FormatToken.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatToken.cpp?rev=290084&r1=290083&r2=290084&view=diff
==
--- cfe/trunk/lib/Format/FormatToken.cpp (original)
+++ cfe/trunk/lib/Format/FormatToken.cpp Mon Dec 19 01:26:11 2016
@@ -273,7 +273,7 @@ void CommaSeparatedList::precomputeForma
continue;
// Ignore layouts that are bound to violate the column limit.
-if (Format.TotalWidth > Style.ColumnLimit)
+if (Format.TotalWidth > Style.ColumnLimit && Columns > 1)
continue;
Formats.push_back(Format);
@@ -287,7 +287,7 @@ CommaSeparatedList::getColumnFormat(unsi
I = Formats.rbegin(),
E = Formats.rend();
I != E; ++I) {
-if (I->TotalWidth <= RemainingCharacters) {
+if (I->TotalWidth <= RemainingCharacters || I->Columns == 1) {
if (BestFormat && I->LineCount > BestFormat->LineCount)
break;
BestFormat = &*I;
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=290084&r1=290083&r2=290084&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Dec 19 01:26:11 2016
@@ -6779,6 +6779,18 @@ TEST_F(FormatTest, FormatsBracedListsInC
" 1, 22, 333, , 5, 66, 777,\n"
" 1, 22, 333, , 5, 66, 777,\n"
" 1, 22, 333, , 5, 66, 777});");
+
+ // Allow "single-column" layout even if that violates the column limit. There
+ // isn't going to be a better way.
+ verifyFormat("std::vector a = {\n"
+ ",\n"
+ ",\n"
+ ",\n"
+ ",\n"
+ "aa,\n"
+ ",\n"
+ "aaa};",
+ getLLVMStyleWithColumns(30));
}
TEST_F(FormatTest, PullTrivialFunctionDefinitionsIntoSingleLine) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r290084 - clang-format: Allow "single column" list layout even if that violates the
Yeah, I just saw that when fixing polly format. I'll take a look.
On Mon, Dec 19, 2016 at 9:05 AM, Tobias Grosser wrote:
> Hi Daniel,
>
> this commit introduce an unnecessary string split, which does not seem
> to be an intended result of the formatting style change this commit
> introduced:
>
> BEFORE:
>
> #define SCOP_STAT(NAME, DESC)
>
> llvm::Statistic RejectStatistics[] = {
> SCOP_STAT(CFG, ""),
> SCOP_STAT(InvalidTerminator, "Unsupported terminator instruction"),
> SCOP_STAT(IrreducibleRegion, "Irreducible loops"),
> SCOP_STAT(UndefCond, "Undefined branch condition"),
> SCOP_STAT(NonSimpleMemoryAccess,
> "Compilated access semantics (volatile or atomic)"),
> };
>
> AFTER:
>
> #define SCOP_STAT(NAME, DESC)
>
> llvm::Statistic RejectStatistics[] = {
> SCOP_STAT(CFG, ""),
> SCOP_STAT(InvalidTerminator, "Unsupported terminator instruction"),
> SCOP_STAT(IrreducibleRegion, "Irreducible loops"),
> SCOP_STAT(UndefCond, "Undefined branch condition"),
> SCOP_STAT(NonSimpleMemoryAccess, "Compilated access semantics
> (volatile or "
> "atomic)"
>
> As this worked before, this seems to be a regression.
>
> Best,
> Tobias
>
> On Mon, Dec 19, 2016, at 08:26 AM, Daniel Jasper via cfe-commits wrote:
> > Author: djasper
> > Date: Mon Dec 19 01:26:11 2016
> > New Revision: 290084
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=290084&view=rev
> > Log:
> > clang-format: Allow "single column" list layout even if that violates the
> > column limit.
> >
> > Single-column layout basically means that we format the list with one
> > element per line. Not doing that when there is a column limit violation
> > doesn't change the fact that there is an item that doesn't fit within
> > the column limit.
> >
> > Before (with a column limit of 30):
> > std::vector a = {
> > , ,
> > , ,
> > aa, ,
> > aaa};
> >
> > After:
> > std::vector a = {
> > ,
> > ,
> > ,
> > ,
> > aa,
> > ,
> > aaa};
> >
> > (and previously we would have formatted like "After" it wasn't for the
> > one
> > item that is too long)
> >
> > Modified:
> > cfe/trunk/lib/Format/FormatToken.cpp
> > cfe/trunk/unittests/Format/FormatTest.cpp
> >
> > Modified: cfe/trunk/lib/Format/FormatToken.cpp
> > URL:
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/
> FormatToken.cpp?rev=290084&r1=290083&r2=290084&view=diff
> >
> ==
> > --- cfe/trunk/lib/Format/FormatToken.cpp (original)
> > +++ cfe/trunk/lib/Format/FormatToken.cpp Mon Dec 19 01:26:11 2016
> > @@ -273,7 +273,7 @@ void CommaSeparatedList::precomputeForma
> >continue;
> >
> > // Ignore layouts that are bound to violate the column limit.
> > -if (Format.TotalWidth > Style.ColumnLimit)
> > +if (Format.TotalWidth > Style.ColumnLimit && Columns > 1)
> >continue;
> >
> > Formats.push_back(Format);
> > @@ -287,7 +287,7 @@ CommaSeparatedList::getColumnFormat(unsi
> > I = Formats.rbegin(),
> > E = Formats.rend();
> > I != E; ++I) {
> > -if (I->TotalWidth <= RemainingCharacters) {
> > +if (I->TotalWidth <= RemainingCharacters || I->Columns == 1) {
> >if (BestFormat && I->LineCount > BestFormat->LineCount)
> > break;
> >BestFormat = &*I;
> >
> > Modified: cfe/trunk/unittests/Format/FormatTest.cpp
> > URL:
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/
> Format/FormatTest.cpp?rev=290084&r1=290083&r2=290084&view=diff
> >
> ==
> > --- cfe/trunk/unittests/Format/FormatTest.cpp (original)
> > +++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Dec 19 01:26:11 2016
> > @@ -6779,6 +6779,18 @@ TEST_F(FormatTest, FormatsBracedListsInC
> >" 1, 22, 333, , 5, 66,
> >
r290090 - clang-format: Fix regression introduced in r290084.
Author: djasper
Date: Mon Dec 19 02:40:56 2016
New Revision: 290090
URL: http://llvm.org/viewvc/llvm-project?rev=290090&view=rev
Log:
clang-format: Fix regression introduced in r290084.
We still want to try in linewrap within single elements of a 1-column
list.
After:
Type *Params[] = {PointerType::getUnqual(FunctionType::get(
Builder.getVoidTy(), Builder.getInt8PtrTy(), false)),
Builder.getInt8PtrTy(),
Builder.getInt32Ty(),
LongType,
LongType,
LongType};
Before:
No line break in the first element, so column limit violation.
Modified:
cfe/trunk/lib/Format/FormatToken.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/FormatToken.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatToken.cpp?rev=290090&r1=290089&r2=290090&view=diff
==
--- cfe/trunk/lib/Format/FormatToken.cpp (original)
+++ cfe/trunk/lib/Format/FormatToken.cpp Mon Dec 19 02:40:56 2016
@@ -92,6 +92,14 @@ unsigned CommaSeparatedList::formatAfter
// Find the best ColumnFormat, i.e. the best number of columns to use.
const ColumnFormat *Format = getColumnFormat(RemainingCodePoints);
+
+ // Formatting with 1 Column isn't really a column layout, so we don't need
the
+ // special logic here. We can just avoid bin packing any of the parameters.
+ if (Format && Format->Columns == 1) {
+State.Stack.back().AvoidBinPacking = true;
+return 0;
+ }
+
// If no ColumnFormat can be used, the braced list would generally be
// bin-packed. Add a severe penalty to this so that column layouts are
// preferred if possible.
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=290090&r1=290089&r2=290090&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Dec 19 02:40:56 2016
@@ -6641,7 +6641,7 @@ TEST_F(FormatTest, LayoutCxx11BraceIniti
"std::this_thread::sleep_for(\n"
"std::chrono::nanoseconds{ std::chrono::seconds{ 1 } } / 5);",
ExtraSpaces);
- verifyFormat("std::vector aaa{\n"
+ verifyFormat("std::vector
aa{\n"
"aaa,\n"
"aa,\n"
"a,\n"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r290092 - Revert "[c++1z] P0195R2: Support pack-expansion of using-declarations."
Author: djasper
Date: Mon Dec 19 04:09:25 2016
New Revision: 290092
URL: http://llvm.org/viewvc/llvm-project?rev=290092&view=rev
Log:
Revert "[c++1z] P0195R2: Support pack-expansion of using-declarations."
This reverts commit r290080 as it leads to many Clang crashes, e.g.:
http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/1814
Removed:
cfe/trunk/test/PCH/cxx1z-using-declaration.cpp
cfe/trunk/test/SemaTemplate/cxx1z-using-declaration.cpp
Modified:
cfe/trunk/include/clang/AST/DeclCXX.h
cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
cfe/trunk/include/clang/Basic/DeclNodes.td
cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/include/clang/Sema/Template.h
cfe/trunk/include/clang/Serialization/ASTBitCodes.h
cfe/trunk/lib/AST/DeclBase.cpp
cfe/trunk/lib/AST/DeclCXX.cpp
cfe/trunk/lib/CodeGen/CGDecl.cpp
cfe/trunk/lib/Parse/ParseDeclCXX.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp
cfe/trunk/lib/Sema/TreeTransform.h
cfe/trunk/lib/Serialization/ASTCommon.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
cfe/trunk/test/Parser/cxx1z-using-declaration.cpp
cfe/trunk/tools/libclang/CIndex.cpp
Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=290092&r1=290091&r2=290092&view=diff
==
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Mon Dec 19 04:09:25 2016
@@ -3140,77 +3140,6 @@ public:
friend class ASTDeclWriter;
};
-/// Represents a pack of using declarations that a single
-/// using-declarator pack-expanded into.
-///
-/// \code
-/// template struct X : T... {
-/// using T::operator()...;
-/// using T::operator T...;
-/// };
-/// \endcode
-///
-/// In the second case above, the UsingPackDecl will have the name
-/// 'operator T' (which contains an unexpanded pack), but the individual
-/// UsingDecls and UsingShadowDecls will have more reasonable names.
-class UsingPackDecl final
-: public NamedDecl, public Mergeable,
- private llvm::TrailingObjects {
- void anchor() override;
-
- /// The UnresolvedUsingValueDecl or UnresolvedUsingTypenameDecl from
- /// which this waas instantiated.
- NamedDecl *InstantiatedFrom;
-
- /// The number of using-declarations created by this pack expansion.
- unsigned NumExpansions;
-
- UsingPackDecl(DeclContext *DC, NamedDecl *InstantiatedFrom,
-ArrayRef UsingDecls)
- : NamedDecl(UsingPack, DC,
- InstantiatedFrom ? InstantiatedFrom->getLocation()
- : SourceLocation(),
- InstantiatedFrom ? InstantiatedFrom->getDeclName()
- : DeclarationName()),
-InstantiatedFrom(InstantiatedFrom), NumExpansions(UsingDecls.size()) {
-std::uninitialized_copy(UsingDecls.begin(), UsingDecls.end(),
-getTrailingObjects());
- }
-
-public:
- /// Get the using declaration from which this was instantiated. This will
- /// always be an UnresolvedUsingValueDecl or an UnresolvedUsingTypenameDecl
- /// that is a pack expansion.
- NamedDecl *getInstantiatedFromUsingDecl() { return InstantiatedFrom; }
-
- /// Get the set of using declarations that this pack expanded into. Note that
- /// some of these may still be unresolved.
- ArrayRef expansions() const {
-return llvm::makeArrayRef(getTrailingObjects(),
NumExpansions);
- }
-
- static UsingPackDecl *Create(ASTContext &C, DeclContext *DC,
- NamedDecl *InstantiatedFrom,
- ArrayRef UsingDecls);
-
- static UsingPackDecl *CreateDeserialized(ASTContext &C, unsigned ID,
- unsigned NumExpansions);
-
- SourceRange getSourceRange() const override LLVM_READONLY {
-return InstantiatedFrom->getSourceRange();
- }
-
- UsingPackDecl *getCanonicalDecl() override { return getFirstDecl(); }
- const UsingPackDecl *getCanonicalDecl() const { return getFirstDecl(); }
-
- static bool classof(const Decl *D) { return classofKind(D->getKind()); }
- static bool classofKind(Kind K) { return K == UsingPack; }
-
- friend class ASTDeclReader;
- friend class ASTDeclWriter;
- friend TrailingObjects;
-};
-
/// \brief Represents a dependent using declaration which was not marked with
/// \c typename.
///
@@ -3229,9 +3158,6 @@ class UnresolvedUsingValueDecl : public
/// \brief The source location of the 'using' keyword
Sour
r290094 - clang-format: Slightly tweak the behavior of <<-wrapping.
Author: djasper
Date: Mon Dec 19 05:14:23 2016
New Revision: 290094
URL: http://llvm.org/viewvc/llvm-project?rev=290094&view=rev
Log:
clang-format: Slightly tweak the behavior of <<-wrapping.
Before:
SomeLongLoggingStatementOrMacro() << "Some long text "
<< some_variable << "\n";
Before:
SomeLongLoggingStatementOrMacro()
<< "Some long text " << some_variable << "\n";
Short logging statements are already special cased in a different part
of the code.
Modified:
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=290094&r1=290093&r2=290094&view=diff
==
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Mon Dec 19 05:14:23 2016
@@ -460,7 +460,7 @@ unsigned ContinuationIndenter::addTokenO
Penalty += State.NextToken->SplitPenalty;
// Breaking before the first "<<" is generally not desirable if the LHS is
- // short. Also always add the penalty if the LHS is split over mutliple lines
+ // short. Also always add the penalty if the LHS is split over multiple lines
// to avoid unnecessary line breaks that just work around this penalty.
if (NextNonComment->is(tok::lessless) &&
State.Stack.back().FirstLessLess == 0 &&
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=290094&r1=290093&r2=290094&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Mon Dec 19 05:14:23 2016
@@ -2007,8 +2007,13 @@ unsigned TokenAnnotator::splitPenalty(co
if (Right.isOneOf(tok::lessless, tok::plus) && Left.isLabelString() &&
(Right.NextOperator || Right.OperatorIndex != 1))
return 25;
- if (Right.is(tok::lessless))
-return 1; // Breaking at a << is really cheap.
+ if (Right.is(tok::lessless)) {
+// Breaking at a << is really cheap.
+if (!Left.is(tok::r_paren) || Right.OperatorIndex > 0)
+ // Slightly prefer to break before the first one in log-like statements.
+ return 2;
+return 1;
+ }
if (Left.is(TT_ConditionalExpr))
return prec::Conditional;
prec::Level Level = Left.getPrecedence();
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=290094&r1=290093&r2=290094&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Dec 19 05:14:23 2016
@@ -5092,6 +5092,9 @@ TEST_F(FormatTest, AlignsPipes) {
" << \n"
" << ;");
verifyFormat(
+ "()\n"
+ "<< << ;");
+ verifyFormat(
"llvm::outs() << \"a\"\n"
"\"b\"\n"
" <<
\"c\";");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r290080 - [c++1z] P0195R2: Support pack-expansion of using-declarations.
I don't understand. This *is* a revert of the whole patch. On Mon, Dec 19, 2016 at 1:26 PM, Renato Golin wrote: > On 19 December 2016 at 11:28, Daniel Jasper via cfe-commits > wrote: > > I have reverted this in r290092 as it was leading to Clang crashes on the > > bots and elsewhere, e.g.: > > http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/1814 > > Hi Daniel, Richard, > > This is will red on our LNT bot, which started with this commit: > > http://lab.llvm.org:8011/builders/clang-native-arm-lnt/builds/1354 > > and still has the same failures on the last build: > > http://lab.llvm.org:8011/builders/clang-native-arm-lnt/builds/1360 > > this is one of the 5 different failures we have in all our bots... > After so many fix-patches and reverts, I'm not surprised we got into > this corner of mayhem. > > I'd like to ask people a bit more care and worry about the bots. > > Most of the time, reverting the whole patch and talking to the bot > owners is a much better strategy than push-fix a bunch of trail and > errors. > > cheers, > --renato > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r290080 - [c++1z] P0195R2: Support pack-expansion of using-declarations.
Oh, I completely understand, I am doing the same here :) On Mon, Dec 19, 2016 at 1:38 PM, Renato Golin wrote: > On 19 December 2016 at 12:27, Daniel Jasper wrote: > > I don't understand. This *is* a revert of the whole patch. > > My bad, your revert hadn't gone through: > > http://lab.llvm.org:8011/builders/clang-native-arm-lnt/builds/1361 > > Now it's green. > > Sorry, we're dealing with 4 different cluster-plucks today. > > I hate Mondays. > > --renato > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r290171 - [OpenCL] Enabling the usage of CLK_NULL_QUEUE as compare operand.
This triggers a bunch of warnings:
lib/AST/Expr.cpp:1519:11: error: enumeration value 'CK_ZeroToOCLQueue'
not handled in switch [-Werror,-Wswitch]
lib/CodeGen/CGExprConstant.cpp:654:13: error: enumeration value
'CK_ZeroToOCLQueue' not handled in switch [-Werror,-Wswitch]
lib/Edit/RewriteObjCFoundationAPI.cpp:1000:13: error: enumeration value
'CK_ZeroToOCLQueue' not handled in switch [-Werror,-Wswitch]
lib/StaticAnalyzer/Core/ExprEngineC.cpp:314:13: error: enumeration value
'CK_ZeroToOCLQueue' not handled in switch [-Werror,-Wswitch]
Can you please fix or roll back quickly?
On Tue, Dec 20, 2016 at 10:15 AM, Egor Churaev via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Author: echuraev
> Date: Tue Dec 20 03:15:21 2016
> New Revision: 290171
>
> URL: http://llvm.org/viewvc/llvm-project?rev=290171&view=rev
> Log:
> [OpenCL] Enabling the usage of CLK_NULL_QUEUE as compare operand.
>
> Summary: Enabling the compression of CLK_NULL_QUEUE to variable of type
> queue_t.
>
> Reviewers: Anastasia
>
> Subscribers: cfe-commits, yaxunl, bader
>
> Differential Revision: https://reviews.llvm.org/D27569
>
> Added:
> cfe/trunk/test/CodeGenOpenCL/null_queue.cl
> cfe/trunk/test/SemaOpenCL/null_queue.cl
> cfe/trunk/test/SemaOpenCL/queue_t_overload.cl
> Modified:
> cfe/trunk/include/clang/AST/OperationKinds.def
> cfe/trunk/include/clang/Sema/Initialization.h
> cfe/trunk/include/clang/Sema/Overload.h
> cfe/trunk/lib/AST/ExprConstant.cpp
> cfe/trunk/lib/CodeGen/CGExprAgg.cpp
> cfe/trunk/lib/CodeGen/CGExprComplex.cpp
> cfe/trunk/lib/CodeGen/CGExprScalar.cpp
> cfe/trunk/lib/Sema/SemaExpr.cpp
> cfe/trunk/lib/Sema/SemaExprCXX.cpp
> cfe/trunk/lib/Sema/SemaInit.cpp
> cfe/trunk/lib/Sema/SemaOverload.cpp
>
> Modified: cfe/trunk/include/clang/AST/OperationKinds.def
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/AST/OperationKinds.def?rev=290171&r1=290170&r2=290171&view=diff
>
> ==
> --- cfe/trunk/include/clang/AST/OperationKinds.def (original)
> +++ cfe/trunk/include/clang/AST/OperationKinds.def Tue Dec 20 03:15:21
> 2016
> @@ -321,6 +321,9 @@ CAST_OPERATION(BuiltinFnToFnPtr)
> // Convert a zero value for OpenCL event_t initialization.
> CAST_OPERATION(ZeroToOCLEvent)
>
> +// Convert a zero value for OpenCL queue_t initialization.
> +CAST_OPERATION(ZeroToOCLQueue)
> +
> // Convert a pointer to a different address space.
> CAST_OPERATION(AddressSpaceConversion)
>
>
> Modified: cfe/trunk/include/clang/Sema/Initialization.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/Sema/Initialization.h?rev=290171&r1=290170&r2=290171&view=diff
>
> ==
> --- cfe/trunk/include/clang/Sema/Initialization.h (original)
> +++ cfe/trunk/include/clang/Sema/Initialization.h Tue Dec 20 03:15:21 2016
> @@ -751,6 +751,8 @@ public:
> SK_StdInitializerListConstructorCall,
> /// \brief Initialize an OpenCL sampler from an integer.
> SK_OCLSamplerInit,
> +/// \brief Initialize queue_t from 0.
> +SK_OCLZeroQueue,
> /// \brief Passing zero to a function where OpenCL event_t is
> expected.
> SK_OCLZeroEvent
>};
> @@ -1148,6 +1150,9 @@ public:
>/// constant.
>void AddOCLZeroEventStep(QualType T);
>
> + /// \brief Add a step to initialize an OpenCL queue_t from 0.
> + void AddOCLZeroQueueStep(QualType T);
> +
>/// \brief Add steps to unwrap a initializer list for a reference
> around a
>/// single element and rewrap it at the end.
>void RewrapReferenceInitList(QualType T, InitListExpr *Syntactic);
>
> Modified: cfe/trunk/include/clang/Sema/Overload.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/Sema/Overload.h?rev=290171&r1=290170&r2=290171&view=diff
>
> ==
> --- cfe/trunk/include/clang/Sema/Overload.h (original)
> +++ cfe/trunk/include/clang/Sema/Overload.h Tue Dec 20 03:15:21 2016
> @@ -83,6 +83,7 @@ namespace clang {
> ICK_TransparentUnionConversion, ///< Transparent Union Conversions
> ICK_Writeback_Conversion, ///< Objective-C ARC writeback conversion
> ICK_Zero_Event_Conversion, ///< Zero constant to event (OpenCL1.2
> 6.12.10)
> +ICK_Zero_Queue_Conversion, ///< Zero constant to queue
> ICK_C_Only_Conversion, ///< Conversions allowed in C, but not C++
> ICK_Incompatible_Pointer_Conversion, ///< C-only conversion between
> pointers
> /// with incompatible types
>
> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/
> ExprConstant.cpp?rev=290171&r1=290170&r2=290171&view=diff
>
> ==
> --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
>
r290173 - Revert "[OpenCL] Enabling the usage of CLK_NULL_QUEUE as compare operand."
Author: djasper
Date: Tue Dec 20 04:05:04 2016
New Revision: 290173
URL: http://llvm.org/viewvc/llvm-project?rev=290173&view=rev
Log:
Revert "[OpenCL] Enabling the usage of CLK_NULL_QUEUE as compare operand."
This reverts commit r290171. It triggers a bunch of warnings, because
the new enumerator isn't handled in all switches. We want a warning-free
build.
Replied on the commit with more details.
Removed:
cfe/trunk/test/CodeGenOpenCL/null_queue.cl
cfe/trunk/test/SemaOpenCL/null_queue.cl
cfe/trunk/test/SemaOpenCL/queue_t_overload.cl
Modified:
cfe/trunk/include/clang/AST/OperationKinds.def
cfe/trunk/include/clang/Sema/Initialization.h
cfe/trunk/include/clang/Sema/Overload.h
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/CodeGen/CGExprAgg.cpp
cfe/trunk/lib/CodeGen/CGExprComplex.cpp
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
Modified: cfe/trunk/include/clang/AST/OperationKinds.def
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/OperationKinds.def?rev=290173&r1=290172&r2=290173&view=diff
==
--- cfe/trunk/include/clang/AST/OperationKinds.def (original)
+++ cfe/trunk/include/clang/AST/OperationKinds.def Tue Dec 20 04:05:04 2016
@@ -321,9 +321,6 @@ CAST_OPERATION(BuiltinFnToFnPtr)
// Convert a zero value for OpenCL event_t initialization.
CAST_OPERATION(ZeroToOCLEvent)
-// Convert a zero value for OpenCL queue_t initialization.
-CAST_OPERATION(ZeroToOCLQueue)
-
// Convert a pointer to a different address space.
CAST_OPERATION(AddressSpaceConversion)
Modified: cfe/trunk/include/clang/Sema/Initialization.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Initialization.h?rev=290173&r1=290172&r2=290173&view=diff
==
--- cfe/trunk/include/clang/Sema/Initialization.h (original)
+++ cfe/trunk/include/clang/Sema/Initialization.h Tue Dec 20 04:05:04 2016
@@ -751,8 +751,6 @@ public:
SK_StdInitializerListConstructorCall,
/// \brief Initialize an OpenCL sampler from an integer.
SK_OCLSamplerInit,
-/// \brief Initialize queue_t from 0.
-SK_OCLZeroQueue,
/// \brief Passing zero to a function where OpenCL event_t is expected.
SK_OCLZeroEvent
};
@@ -1150,9 +1148,6 @@ public:
/// constant.
void AddOCLZeroEventStep(QualType T);
- /// \brief Add a step to initialize an OpenCL queue_t from 0.
- void AddOCLZeroQueueStep(QualType T);
-
/// \brief Add steps to unwrap a initializer list for a reference around a
/// single element and rewrap it at the end.
void RewrapReferenceInitList(QualType T, InitListExpr *Syntactic);
Modified: cfe/trunk/include/clang/Sema/Overload.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Overload.h?rev=290173&r1=290172&r2=290173&view=diff
==
--- cfe/trunk/include/clang/Sema/Overload.h (original)
+++ cfe/trunk/include/clang/Sema/Overload.h Tue Dec 20 04:05:04 2016
@@ -83,7 +83,6 @@ namespace clang {
ICK_TransparentUnionConversion, ///< Transparent Union Conversions
ICK_Writeback_Conversion, ///< Objective-C ARC writeback conversion
ICK_Zero_Event_Conversion, ///< Zero constant to event (OpenCL1.2 6.12.10)
-ICK_Zero_Queue_Conversion, ///< Zero constant to queue
ICK_C_Only_Conversion, ///< Conversions allowed in C, but not C++
ICK_Incompatible_Pointer_Conversion, ///< C-only conversion between
pointers
/// with incompatible types
Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=290173&r1=290172&r2=290173&view=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Tue Dec 20 04:05:04 2016
@@ -8340,7 +8340,6 @@ bool IntExprEvaluator::VisitCastExpr(con
case CK_IntegralComplexToFloatingComplex:
case CK_BuiltinFnToFnPtr:
case CK_ZeroToOCLEvent:
- case CK_ZeroToOCLQueue:
case CK_NonAtomicToAtomic:
case CK_AddressSpaceConversion:
case CK_IntToOCLSampler:
@@ -8838,7 +8837,6 @@ bool ComplexExprEvaluator::VisitCastExpr
case CK_CopyAndAutoreleaseBlockObject:
case CK_BuiltinFnToFnPtr:
case CK_ZeroToOCLEvent:
- case CK_ZeroToOCLQueue:
case CK_NonAtomicToAtomic:
case CK_AddressSpaceConversion:
case CK_IntToOCLSampler:
Modified: cfe/trunk/lib/CodeGen/CGExprAgg.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprAgg.cpp?rev=290173&r1=290172&r2=290173&view=diff
==
--- cfe/trunk/lib
Re: r290171 - [OpenCL] Enabling the usage of CLK_NULL_QUEUE as compare operand.
Reverted in rL290173.
On Tue, Dec 20, 2016 at 11:02 AM, Daniel Jasper wrote:
> This triggers a bunch of warnings:
>
> lib/AST/Expr.cpp:1519:11: error: enumeration value 'CK_ZeroToOCLQueue'
> not handled in switch [-Werror,-Wswitch]
> lib/CodeGen/CGExprConstant.cpp:654:13: error: enumeration value
> 'CK_ZeroToOCLQueue' not handled in switch [-Werror,-Wswitch]
> lib/Edit/RewriteObjCFoundationAPI.cpp:1000:13: error: enumeration value
> 'CK_ZeroToOCLQueue' not handled in switch [-Werror,-Wswitch]
> lib/StaticAnalyzer/Core/ExprEngineC.cpp:314:13: error: enumeration
> value 'CK_ZeroToOCLQueue' not handled in switch [-Werror,-Wswitch]
>
> Can you please fix or roll back quickly?
>
> On Tue, Dec 20, 2016 at 10:15 AM, Egor Churaev via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: echuraev
>> Date: Tue Dec 20 03:15:21 2016
>> New Revision: 290171
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=290171&view=rev
>> Log:
>> [OpenCL] Enabling the usage of CLK_NULL_QUEUE as compare operand.
>>
>> Summary: Enabling the compression of CLK_NULL_QUEUE to variable of type
>> queue_t.
>>
>> Reviewers: Anastasia
>>
>> Subscribers: cfe-commits, yaxunl, bader
>>
>> Differential Revision: https://reviews.llvm.org/D27569
>>
>> Added:
>> cfe/trunk/test/CodeGenOpenCL/null_queue.cl
>> cfe/trunk/test/SemaOpenCL/null_queue.cl
>> cfe/trunk/test/SemaOpenCL/queue_t_overload.cl
>> Modified:
>> cfe/trunk/include/clang/AST/OperationKinds.def
>> cfe/trunk/include/clang/Sema/Initialization.h
>> cfe/trunk/include/clang/Sema/Overload.h
>> cfe/trunk/lib/AST/ExprConstant.cpp
>> cfe/trunk/lib/CodeGen/CGExprAgg.cpp
>> cfe/trunk/lib/CodeGen/CGExprComplex.cpp
>> cfe/trunk/lib/CodeGen/CGExprScalar.cpp
>> cfe/trunk/lib/Sema/SemaExpr.cpp
>> cfe/trunk/lib/Sema/SemaExprCXX.cpp
>> cfe/trunk/lib/Sema/SemaInit.cpp
>> cfe/trunk/lib/Sema/SemaOverload.cpp
>>
>> Modified: cfe/trunk/include/clang/AST/OperationKinds.def
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/
>> AST/OperationKinds.def?rev=290171&r1=290170&r2=290171&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/AST/OperationKinds.def (original)
>> +++ cfe/trunk/include/clang/AST/OperationKinds.def Tue Dec 20 03:15:21
>> 2016
>> @@ -321,6 +321,9 @@ CAST_OPERATION(BuiltinFnToFnPtr)
>> // Convert a zero value for OpenCL event_t initialization.
>> CAST_OPERATION(ZeroToOCLEvent)
>>
>> +// Convert a zero value for OpenCL queue_t initialization.
>> +CAST_OPERATION(ZeroToOCLQueue)
>> +
>> // Convert a pointer to a different address space.
>> CAST_OPERATION(AddressSpaceConversion)
>>
>>
>> Modified: cfe/trunk/include/clang/Sema/Initialization.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/
>> Sema/Initialization.h?rev=290171&r1=290170&r2=290171&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Sema/Initialization.h (original)
>> +++ cfe/trunk/include/clang/Sema/Initialization.h Tue Dec 20 03:15:21
>> 2016
>> @@ -751,6 +751,8 @@ public:
>> SK_StdInitializerListConstructorCall,
>> /// \brief Initialize an OpenCL sampler from an integer.
>> SK_OCLSamplerInit,
>> +/// \brief Initialize queue_t from 0.
>> +SK_OCLZeroQueue,
>> /// \brief Passing zero to a function where OpenCL event_t is
>> expected.
>> SK_OCLZeroEvent
>>};
>> @@ -1148,6 +1150,9 @@ public:
>>/// constant.
>>void AddOCLZeroEventStep(QualType T);
>>
>> + /// \brief Add a step to initialize an OpenCL queue_t from 0.
>> + void AddOCLZeroQueueStep(QualType T);
>> +
>>/// \brief Add steps to unwrap a initializer list for a reference
>> around a
>>/// single element and rewrap it at the end.
>>void RewrapReferenceInitList(QualType T, InitListExpr *Syntactic);
>>
>> Modified: cfe/trunk/include/clang/Sema/Overload.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/
>> Sema/Overload.h?rev=290171&r1=290170&r2=290171&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Sema/Overload.h (original)
>> +++ cfe/trunk/include/clang/Sema/Overload.h Tue Dec 20 03:15:21 2016
>> @@ -83,6 +83,7 @@ namespace clang {
>> ICK_TransparentUnionConversion, ///< Transparent Union Conversions
>> ICK_Writeback_Conversion, ///< Objective-C ARC writeback conversion
>> ICK_Zero_Event_Conversion, ///< Zero constant to event (OpenCL1.2
>> 6.12.10)
>> +ICK_Zero_Queue_Conversion, ///< Zero constant to queue
>> ICK_C_Only_Conversion, ///< Conversions allowed in C, but not C++
>> ICK_Incompatible_Pointer_Conversion, ///< C-only conversion between
>> pointers
>> /// with incompatible types
>>
>> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/
r290177 - clang-format: Fix bug in understanding string-label&value analysis.
Author: djasper
Date: Tue Dec 20 09:27:46 2016
New Revision: 290177
URL: http://llvm.org/viewvc/llvm-project?rev=290177&view=rev
Log:
clang-format: Fix bug in understanding string-label&value analysis.
While for <<-operators often used in log statments, a single key value
pair is always on the second operator, e.g.
llvm::errs() << "a=" << a;
It is on the first operator for plus- or comma-concatenated strings:
string s = "aa: " + ;
(the "=" not counting because that's a different operator precedence)
Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=290177&r1=290176&r2=290177&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Tue Dec 20 09:27:46 2016
@@ -2000,11 +2000,14 @@ unsigned TokenAnnotator::splitPenalty(co
if (Left.isOneOf(tok::plus, tok::comma) && Left.Previous &&
Left.Previous->isLabelString() &&
- (Left.NextOperator || Left.OperatorIndex != 1))
+ (Left.NextOperator || Left.OperatorIndex != 0))
return 100;
+ if (Right.is(tok::plus) && Left.isLabelString() &&
+ (Right.NextOperator || Right.OperatorIndex != 0))
+return 25;
if (Left.is(tok::comma))
return 1;
- if (Right.isOneOf(tok::lessless, tok::plus) && Left.isLabelString() &&
+ if (Right.is(tok::lessless) && Left.isLabelString() &&
(Right.NextOperator || Right.OperatorIndex != 1))
return 25;
if (Right.is(tok::lessless)) {
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=290177&r1=290176&r2=290177&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Dec 20 09:27:46 2016
@@ -5214,6 +5214,12 @@ TEST_F(FormatTest, KeepStringLabelValueP
verifyFormat("string v = StrCat(\": \", ,\n"
" \": \", ,\n"
" \": \", );");
+ verifyFormat("string v = \": \" +\n"
+ " ( + );",
+ getLLVMStyleWithColumns(40));
+ verifyFormat("string v = StrCat(\": \" +\n"
+ " (aaa + a));",
+ getLLVMStyleWithColumns(40));
}
TEST_F(FormatTest, UnderstandsEquals) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r290259 - clang-format: Fix bug in handling of single-column lists.
Author: djasper
Date: Wed Dec 21 11:02:06 2016
New Revision: 290259
URL: http://llvm.org/viewvc/llvm-project?rev=290259&view=rev
Log:
clang-format: Fix bug in handling of single-column lists.
Members that are themselves wrapped in fake parentheses would lead to
AvoidBinPacking be set on the wrong ParenState.
After:
vector = {
aa.aaa,
aa.aaa,
aa.aaa,
aa.aaa,
aa.aaa,
aa.aaa,
};
Before we were falling back to bin-packing these.
Modified:
cfe/trunk/lib/Format/FormatToken.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/FormatToken.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatToken.cpp?rev=290259&r1=290258&r2=290259&view=diff
==
--- cfe/trunk/lib/Format/FormatToken.cpp (original)
+++ cfe/trunk/lib/Format/FormatToken.cpp Wed Dec 21 11:02:06 2016
@@ -77,6 +77,9 @@ unsigned CommaSeparatedList::formatAfter
if (State.NextToken == nullptr || !State.NextToken->Previous)
return 0;
+ if (Formats.size() == 1)
+return 0; // Handled by formatFromToken
+
// Ensure that we start on the opening brace.
const FormatToken *LBrace =
State.NextToken->Previous->getPreviousNonComment();
@@ -93,13 +96,6 @@ unsigned CommaSeparatedList::formatAfter
// Find the best ColumnFormat, i.e. the best number of columns to use.
const ColumnFormat *Format = getColumnFormat(RemainingCodePoints);
- // Formatting with 1 Column isn't really a column layout, so we don't need
the
- // special logic here. We can just avoid bin packing any of the parameters.
- if (Format && Format->Columns == 1) {
-State.Stack.back().AvoidBinPacking = true;
-return 0;
- }
-
// If no ColumnFormat can be used, the braced list would generally be
// bin-packed. Add a severe penalty to this so that column layouts are
// preferred if possible.
@@ -137,7 +133,9 @@ unsigned CommaSeparatedList::formatAfter
unsigned CommaSeparatedList::formatFromToken(LineState &State,
ContinuationIndenter *Indenter,
bool DryRun) {
- if (HasNestedBracedList)
+ // Formatting with 1 Column isn't really a column layout, so we don't need
the
+ // special logic here. We can just avoid bin packing any of the parameters.
+ if (Formats.size() == 1 || HasNestedBracedList)
State.Stack.back().AvoidBinPacking = true;
return 0;
}
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=290259&r1=290258&r2=290259&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Dec 21 11:02:06 2016
@@ -6800,6 +6800,14 @@ TEST_F(FormatTest, FormatsBracedListsInC
",\n"
"aaa};",
getLLVMStyleWithColumns(30));
+ verifyFormat("vector = {\n"
+ "aa.aaa,\n"
+ "aa.aaa,\n"
+ "aa.aaa,\n"
+ "aa.aaa,\n"
+ "aa.aaa,\n"
+ "aa.aaa,\n"
+ "};");
}
TEST_F(FormatTest, PullTrivialFunctionDefinitionsIntoSingleLine) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r290337 - clang-format: Less eagerly try to keep label-value pairs on a line.
Author: djasper
Date: Thu Dec 22 06:37:06 2016
New Revision: 290337
URL: http://llvm.org/viewvc/llvm-project?rev=290337&view=rev
Log:
clang-format: Less eagerly try to keep label-value pairs on a line.
Before:
string v =
StrCat("aaa: ", SomeFunction(,
aaa),
bbb);
After:
string v = StrCat("aaa: ",
SomeFunction(, aaa),
bbb);
Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=290337&r1=290336&r2=290337&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Thu Dec 22 06:37:06 2016
@@ -2001,7 +2001,7 @@ unsigned TokenAnnotator::splitPenalty(co
if (Left.isOneOf(tok::plus, tok::comma) && Left.Previous &&
Left.Previous->isLabelString() &&
(Left.NextOperator || Left.OperatorIndex != 0))
-return 100;
+return 45;
if (Right.is(tok::plus) && Left.isLabelString() &&
(Right.NextOperator || Right.OperatorIndex != 0))
return 25;
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=290337&r1=290336&r2=290337&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Dec 22 06:37:06 2016
@@ -5220,6 +5220,10 @@ TEST_F(FormatTest, KeepStringLabelValueP
verifyFormat("string v = StrCat(\": \" +\n"
" (aaa + a));",
getLLVMStyleWithColumns(40));
+ verifyFormat(
+ "string v = StrCat(\"aaa: \",\n"
+ " SomeFunction(, .aaa),\n"
+ " bbb);");
}
TEST_F(FormatTest, UnderstandsEquals) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r290842 - Remove isIgnored()-test that is more expensive than the analysis behind it
Author: djasper
Date: Mon Jan 2 16:55:45 2017
New Revision: 290842
URL: http://llvm.org/viewvc/llvm-project?rev=290842&view=rev
Log:
Remove isIgnored()-test that is more expensive than the analysis behind it
In many translation units I have tried, the calls to isIgnored() removed
in this patch are more expensive than doing the analysis that is behind
it. The speed-up in translation units I have tried is between 10 and
20%.
Review: https://reviews.llvm.org/D28208
Modified:
cfe/trunk/lib/Sema/SemaTemplate.cpp
Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=290842&r1=290841&r2=290842&view=diff
==
--- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp Mon Jan 2 16:55:45 2017
@@ -4244,7 +4244,7 @@ namespace {
UnnamedLocalNoLinkageFinder(Sema &S, SourceRange SR) : S(S), SR(SR) { }
bool Visit(QualType T) {
- return inherited::Visit(T.getTypePtr());
+ return T.isNull() ? false : inherited::Visit(T.getTypePtr());
}
#define TYPE(Class, Parent) \
@@ -4497,17 +4497,7 @@ bool Sema::CheckTemplateArgument(Templat
//
// C++11 allows these, and even in C++03 we allow them as an extension with
// a warning.
- bool NeedsCheck;
- if (LangOpts.CPlusPlus11)
-NeedsCheck =
-!Diags.isIgnored(diag::warn_cxx98_compat_template_arg_unnamed_type,
- SR.getBegin()) ||
-!Diags.isIgnored(diag::warn_cxx98_compat_template_arg_local_type,
- SR.getBegin());
- else
-NeedsCheck = Arg->hasUnnamedOrLocalType();
-
- if (NeedsCheck) {
+ if (LangOpts.CPlusPlus11 || Arg->hasUnnamedOrLocalType()) {
UnnamedLocalNoLinkageFinder Finder(*this, SR);
(void)Finder.Visit(Context.getCanonicalType(Arg));
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r291434 - clang-format: Improve support for override/final as variable names.
Author: djasper
Date: Mon Jan 9 05:04:07 2017
New Revision: 291434
URL: http://llvm.org/viewvc/llvm-project?rev=291434&view=rev
Log:
clang-format: Improve support for override/final as variable names.
Before:
bool a = f() &&override.f();
bool a = f() &&final.f();
void f(const MyOverride & override);
void f(const MyFinal & final);
After:
bool a = f() && override.f();
bool a = f() && final.f();
void f(const MyOverride &override);
void f(const MyFinal &final);
Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=291434&r1=291433&r2=291434&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Mon Jan 9 05:04:07 2017
@@ -1282,9 +1282,7 @@ private:
return TT_UnaryOperator;
const FormatToken *NextToken = Tok.getNextNonComment();
-if (!NextToken ||
-NextToken->isOneOf(tok::arrow, Keywords.kw_final, tok::equal,
- Keywords.kw_override) ||
+if (!NextToken || NextToken->isOneOf(tok::arrow, tok::equal) ||
(NextToken->is(tok::l_brace) && !NextToken->getNextNonComment()))
return TT_PointerOrReference;
@@ -2088,9 +2086,9 @@ bool TokenAnnotator::spaceRequiredBetwee
!Line.IsMultiVariableDeclStmt)))
return true;
if (Left.is(TT_PointerOrReference))
-return Right.Tok.isLiteral() ||
- Right.isOneOf(TT_BlockComment, Keywords.kw_final,
- Keywords.kw_override) ||
+return Right.Tok.isLiteral() || Right.is(TT_BlockComment) ||
+ (Right.isOneOf(Keywords.kw_override, Keywords.kw_final) &&
+!Right.is(TT_StartOfName)) ||
(Right.is(tok::l_brace) && Right.BlockKind == BK_Block) ||
(!Right.isOneOf(TT_PointerOrReference, TT_ArraySubscriptLSquare,
tok::l_paren) &&
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=291434&r1=291433&r2=291434&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Jan 9 05:04:07 2017
@@ -5780,6 +5780,10 @@ TEST_F(FormatTest, UnderstandsUsesOfStar
verifyGoogleFormat("MACRO Constructor(const int& i) : a(a), b(b) {}");
verifyFormat("void f() { f(a, c * d); }");
verifyFormat("void f() { f(new a(), c * d); }");
+ verifyFormat("void f(const MyOverride &override);");
+ verifyFormat("void f(const MyFinal &final);");
+ verifyIndependentOfContext("bool a = f() && override.f();");
+ verifyIndependentOfContext("bool a = f() && final.f();");
verifyIndependentOfContext("InvalidRegions[*R] = 0;");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23602: Port tools/clang-format/git-clang-format to work Python beyond 2.7
djasper accepted this revision. djasper added a comment. Looks good. Repository: rL LLVM https://reviews.llvm.org/D23602 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes
djasper added a comment. I think instead of doing some complex computation with LineLevel and NestingLevel, it might be better to just leave them as the pair and compare them as a pair. The LineLevel should probably always trump the NestingLevel. So, I'd try to just defined ScopeLevel as a pair, put both the LineLevel and the NestingLevel in it and use that for the comparisons. I might be overlooking something, though. https://reviews.llvm.org/D21279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23714: clang-format: [JS] handle object literals with casts.
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. https://reviews.llvm.org/D23714 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23761: clang-format: [JS] supports casts to types starting with punctuation ("{[(").
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Please add a "before" and "after" to the patch description before submitting. Otherwise looks good. https://reviews.llvm.org/D23761 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes
djasper added a comment. I think the IndentLevel in WhitespaceManager (and the nested Change) is a horrible mess and should be cleaned up. It gets set either to 0 or to the "Level" of the AnnotatedLine. To me only the latter makes sense as the line defines the indent level. Everything else, including recomputing this and introducing a ScopeLevel makes the current situation worse. I can try to do this in advance of this patch, if you prefer. https://reviews.llvm.org/D21279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23279: clang-reorder-fields
djasper added a subscriber: djasper. djasper added a comment. Sorry for the long silence. Manuel is currently out on vacation. I am not entirely sure how we want to make progress at this point. There seems to be consensus that in the mid-term, this should go into the clang-refactor tool. However, it might take a bit for that to be there (we have to carefully design that, I think). In the meantime, I don't know whether we should check this in as a standalone-tool and then merge into clang-refactor once ready. Ben, what do you think? Also, I guess we could at the very least start with a proper review of the code. That seems to be useful irrespective of how where the code ends up in the end. My only high-level comment so far (will try to review in more detail later) is: I find it strange to mix ASTMatchers and a RecursiveASTVisitor in the same tool. It seem that if you are using ASTMatchers internally for other things, you should also use ASTMatchers to find all the constructors etc. That should (I hope) make the code a bit simpler. Repository: rL LLVM https://reviews.llvm.org/D23279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23279: clang-reorder-fields
djasper added a comment.
Ben: I am happy to have this as a separate tool until then. But if we go that
route, let's add very explicit comments about this so that people don't start
depending on it and we can actually remove it as easily as possible.
Alex: I don't think there will be a significant performance difference between
implementing this as recursive AST visitor or implementing this with matchers.
Even if you have multiple matchers, a single run over the AST would suffice. If
there is a performance difference, that's probably something we should fix.
It's hard to make an estimate on which version would be more concise without
actually comparing both versions (I would bet on the AST matcher version to be
more concise). However, an equally important point is that a single way to find
something in the AST is easier to understand and maintain then having two ways
to do it in the same tool.
Comment at: test/clang-reorder-fields/CStructAmbiguousName.cpp:6
@@ +5,3 @@
+struct Foo {
+ int x;// CHECK: int x;
+ double y; // CHECK: double y;
Have you thought about how to handle comments that surround these?
Repository:
rL LLVM
https://reviews.llvm.org/D23279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23279: clang-reorder-fields
djasper added a comment. I think quite a bit of the complexity in this patch stems from the two different ways to format the input (defining the field order or alternatively defining the new index of specific fields). Do we really need both? If not, I'd remove one of them for now (likely the index-based one). If we do need both, I'd still remove the index-based version from ReorderFieldsAction and instead provide a helper that converts from one to the other. Or maybe we should even go one step further and move the entire name-to-index-mapping into helper functions and only hand an index-to-index mapping into ReorderFieldsAction. That way, it doesn't need a lot of its error handing. (All of these thoughts are based on the single responsibility principle). Repository: rL LLVM https://reviews.llvm.org/D23279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23279: clang-reorder-fields
djasper added a comment.
Sorry, didn't have much time so far, will give this some more review later
today. One thing those is that the code could do with a bit more comments. It
would help reviewing (and future maintenance) of functions such as
reorderFieldsInConstructor to have brief comment (doesn't have to be more than
1-3 sentences) explaining what they are doing and maybe hint at how they are
doing it (not going into too much detail, there is the code for that).
Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:56
@@ +55,3 @@
+ Finder.matchAST(Context);
+ return Callback.RecordDecl;
+}
I think you can replace most of this function with:
auto Results = match(recordDecl(hasName(RecordName),
isDefinition()).bind("x"),
*Context.getTranslationUnitDecl(), Context));
if (Results.size() != 1) {
// error
}
return selectFirst("x", Result);
Or make that a bit more elaborate with
Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:61
@@ +60,3 @@
+ ArrayRef DesiredFieldsOrder,
+ SmallVector &NewFieldsOrder) {
+ assert(Definition && "Definition is null");
I'd just return the NewFieldsOrder and let it return an empty vector in case of
an error. For boolean values, it is always hard to know whether they mean
"success" or "failure". And like this, it is unclear of what happens if
NewFieldsOrder isn't empty to start with (would run into the assert, but that's
not helpful).
Comment at: clang-reorder-fields/ReorderFieldsAction.h:29
@@ +28,3 @@
+ llvm::ArrayRef DesiredFieldsOrder;
+ std::map &Replacements;
+
It's not obvious what this map is mapping. Add a comment here or at the
constructor.
Or actually just store a single Replacements object and use
groupReplacementsByFile (Replacement.h) when you need these by file.
Repository:
rL LLVM
https://reviews.llvm.org/D23279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23279: clang-reorder-fields
djasper added a comment.
I am serious about writing more comments.
Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:41
@@ +40,3 @@
+ }
+ if (Results.size() != 1) {
+errs() << "The name " << RecordName
Make this "> 1" instead of "!= 1". Not really important, but imagine that at
some point for some reason you wanted to change the order of the two ifs.
Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:137
@@ +136,3 @@
+std::end(NewWrittenInitializersOrder), ByFieldNewPosition);
+ assert(OldWrittenInitializersOrder.size() ==
+ NewWrittenInitializersOrder.size());
This assert seems pretty useless.
Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:139
@@ +138,3 @@
+ NewWrittenInitializersOrder.size());
+ for (unsigned Index = 0; Index < NewWrittenInitializersOrder.size(); ++Index)
+addReplacement(OldWrittenInitializersOrder[Index]->getSourceRange(),
Also use "i" and "e" as below.
And you shouldn't be creating a replacement if new order == old order.
Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:152
@@ +151,3 @@
+ // SourceLocation::isValid is const.
+ if (!const_cast(InitListEx)->isExplicit() ||
+ !InitListEx->getNumInits())
I think it is an oversight that isExplicit() isn't const. Can you send out a
separate patch to fix that instead of using const_cast here? Seems horrible ;)
Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:157
@@ +156,3 @@
+ "Currently only full initialization is supported");
+ for (unsigned Index = 0, NumInits = InitListEx->getNumInits();
+ Index < NumInits; ++Index) {
Use "i" and "e" instead of "Index" and "NumInits". That's a common pattern used
in a lot of places.
Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:197
@@ +196,3 @@
+continue;
+ const auto *D = C->getDefinition();
+ if (!D)
I'd write this as:
if (const auto *D = C->getDefinition())
reorderFieldsInConstructor(...);
Comment at: clang-reorder-fields/ReorderFieldsAction.h:31
@@ +30,3 @@
+
+public:
+ ReorderFieldsAction(
I don't why clang-rename does what it does at the moment. My main point is that
we have already implemented splitting up replacements by file in
groupByReplacementsByFile. No need to reimplement that here or make the
interface of this function more complex than it needs to be.
Comment at: clang-reorder-fields/tool/ClangReorderFields.cpp:44
@@ +43,3 @@
+ bool parse(cl::Option &O, StringRef ArgName, const std::string &ArgValue,
+ SmallVector &Val) {
+Val.clear();
Don't use and output parameter. Just return Parts.
Comment at: clang-reorder-fields/tool/ClangReorderFields.cpp:62
@@ +61,3 @@
+
+static cl::opt Inplace("i", cl::desc("Overwrite edited s."),
+ cl::cat(ClangReorderFieldsCategory));
remove <>, just "files".
Comment at: clang-reorder-fields/tool/ClangReorderFields.cpp:74
@@ +73,3 @@
+
+ reorder_fields::ReorderFieldsAction Action(RecordName, FieldsOrder,
+ Tool.getReplacements());
Should you be checking whether all the commandline flags are given and display
a usage message if not here?
Comment at: clang-reorder-fields/tool/ClangReorderFields.cpp:81
@@ +80,3 @@
+ if (Inplace) {
+ExitCode = Tool.runAndSave(Factory.get());
+ } else {
just:
return Tool.runAndSave(..);
Here and elsewhere.
Repository:
rL LLVM
https://reviews.llvm.org/D23279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23279: clang-reorder-fields
djasper added inline comments.
Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:137
@@ +136,3 @@
+std::end(NewWrittenInitializersOrder), ByFieldNewPosition);
+ assert(OldWrittenInitializersOrder.size() ==
+ NewWrittenInitializersOrder.size());
djasper wrote:
> This assert seems pretty useless.
Actually no, this is fine.
Comment at: clang-reorder-fields/ReorderFieldsAction.h:31
@@ +30,3 @@
+
+public:
+ ReorderFieldsAction(
ioeric wrote:
> alexshap wrote:
> > alexshap wrote:
> > > djasper wrote:
> > > > I don't why clang-rename does what it does at the moment. My main point
> > > > is that we have already implemented splitting up replacements by file
> > > > in groupByReplacementsByFile. No need to reimplement that here or make
> > > > the interface of this function more complex than it needs to be.
> > > sorry, i am not sure i got ur comment - need to think - maybe i am too
> > > sleepy -
> > > the issue is not with clang-rename, but with class RefactoringTool
> > > and more precisely with the method the method getReplacements.
> > pls, take a look at my previous comments (especially regarding the diff
> > https://reviews.llvm.org/D21748?id=64016.)
> >
> > meanwhile:
> > http://clang.llvm.org/doxygen/Replacement_8h_source.html
> >
> > **/// \brief Adds a new replacement \p R to the current set of
> > replacements.
> > 160 /// \p R must have the same file path as all existing replacements.
> > **
> > 161 /// Returns true if the replacement is successfully inserted;
> > otherwise,
> > 162 /// it returns an llvm::Error, i.e. there is a conflict between R
> > and the
> > 163 /// existing replacements or R's file path is different from the
> > filepath of
> > 164 /// existing replacements. Callers must explicitly check the Error
> > returned.
> > 165 /// This prevents users from adding order-dependent replacements.
> > To control
> > 166 /// the order in which order-dependent replacements are applied, use
> > 167 /// merge({R}) with R referring to the changed code after applying
> > all
> > 168 /// existing replacements.
> > 169 /// Replacements with offset UINT_MAX are special - we do not
> > detect conflicts
> > 170 /// for such replacements since users may add them intentionally as
> > a special
> > 171 /// category of replacements.
> > 172 llvm::Error add(const Replacement &R);
> >
> > at this point it seems like groupReplacementsByFile was made redundant by
> > https://reviews.llvm.org/D21748
> >
> > @ioeric, could u comment on this ? I apologize in advance - i might be
> > missing smth / might be to sleepy. Will take a look at this again tomorrow
> >
> > P.S. just in case - ReorderingFieldsAction can change different files
> > (including headers etc) - so i simply can not use a single Replacements
> > object
> We don't have an implementation for grouping replacements by file yet (for
> class implementation of `tooling::Replacements`). The previous
> `groupReplacementsByFile` indeed should have been deprecated. At this point,
> tools which carry replacements for multiple files use a map from file names
> to `tooling::Replacements` (usually called `FileToReplacements`) like what
> you do here.
Ok.. Then nevermind and sorry for the noise..
Defining a typedef as Eric suggests probably makes this a bit easier to read.
Repository:
rL LLVM
https://reviews.llvm.org/D23279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22431: clang-format: [JS] nested and tagged template strings.
djasper accepted this revision.
djasper added a comment.
This revision is now accepted and ready to land.
Looks good. Sorry for the delay!
Comment at: lib/Format/FormatTokenLexer.h:68
@@ +67,3 @@
+ // embedding expressions nested in ${expr-here}. Template strings can be
+ // nested recursively, i.e. expressions can contain template strings in
turns.
+ //
.. in turn.
https://reviews.llvm.org/D22431
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23279: clang-reorder-fields
djasper added inline comments.
Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:29
@@ +28,3 @@
+using namespace llvm;
+using namespace clang;
+using namespace clang::ast_matchers;
Put everything here into the namespace clang and remove "using namespace
clang". Similarly check if you (then) really still need the "using namespace
llvm".
Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:92
@@ +91,3 @@
+
+/// \brief Reorders fields in the definition of a struct/class
+/// At the moment reodering of fields with
You need an empty line in the comment to end the "\brief", I think.
Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:108
@@ +107,3 @@
+ continue;
+assert(Field->getAccess() ==
+ Fields[NewFieldsOrder[FieldIndex]]->getAccess() &&
I think you need to properly handle this in a different way, e.g. returning a
bool value and aborting the replacement operation. Two main reasons:
- Otherwise you cannot test it (well)
- This alone mean that if I build this tool in a release (non-assert-enabled)
build, it will just do the wrong thing.
asserts are supposed to be used only for code paths that you don't expect to
ever get into.
Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:118
@@ +117,3 @@
+
+/// \brief Reorders initializers in a C++ struct/class constructor
+static void reorderFieldsInConstructor(
This is not what I meant be writing a comment. I consider this comment pretty
much useless, it doesn't add any information on top of the function name. You
need to write significantly more comments so people can actually (easily)
understand what your implementation is doing. You can write a sentence or two
up here and/or add comments to the different things that are done within the
function. Without that, this is really hard to follow, e.g. the different
between NewFieldsOrder and NewFieldsPositions is not at all intuitive (at least
to me). You want to write enough comments that other people can at a quick
glance understand a) what this is doing and b) that the implementation is doing
what you are writing.
E.g. something like:
// A constructor can have initializers for an arbitrary subset of the classes
fields. Thus,
// we need to ensure that we reorder just the initializers that a present.
Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:148
@@ +147,3 @@
+ NewWrittenInitializersOrder.size());
+ for (unsigned I = 0, E = NewWrittenInitializersOrder.size(); I < E; ++I)
+if (OldWrittenInitializersOrder[I] != NewWrittenInitializersOrder[I])
By convention we use I and E for iterators, i and e for ints.
Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:165
@@ +164,3 @@
+return;
+ assert(InitListEx->getNumInits() == NewFieldsOrder.size() &&
+ "Currently only full initialization is supported");
Same here, an assert is insufficient.
Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:202
@@ +201,3 @@
+for (const auto *C : RD->ctors()) {
+ if (C->isImplicit() || C->isDelegatingConstructor())
+continue;
Why are you ruling out delegating constructors?
Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:204
@@ +203,3 @@
+continue;
+ if (const auto *D = C->getDefinition())
+reorderFieldsInConstructor(cast(D),
I'd do:
if (const auto *D = dyn_cast(C->getDefinition))
reorderFieldsInConstructor(D, NewFieldsOrder, Context, Replacements);
But both should be fine.
Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:208
@@ +207,3 @@
+}
+if (!RD->isAggregate())
+ return;
Here you could add a comment like:
// We only need to reorder init list expressions for aggregate types. For
other types
// the order of constructor parameters is used, which we don't change.
Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:210
@@ +209,3 @@
+ return;
+for (auto Result : match(initListExpr().bind("initListExpr"), Context)) {
+ const auto *E = Result.getNodeAs("initListExpr");
Something like:
match(initListExpr(hasType(equalsNode(RD)))
should work.
Comment at: clang-reorder-fields/tool/ClangReorderFields.cpp:68
@@ +67,3 @@
+
+ int ExitCode = Tool.run(Factory.get());
+ LangOptions DefaultLangOptions;
Should you continue if the exit code isn't 0 here?
Repository:
rL LLVM
https://reviews.llvm.org/D23279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23279: clang-reorder-fields
djasper added a comment.
Probably the last round ..
Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:202
@@ +201,3 @@
+for (const auto *C : RD->ctors()) {
+ if (C->isImplicit() || C->isDelegatingConstructor())
+continue;
alexshap wrote:
> djasper wrote:
> > Why are you ruling out delegating constructors?
> ./include/clang/Basic/DiagnosticSemaKinds.td:1961: "an initializer for a
> delegating constructor must appear alone";
> So i assumed that right now we don't need to do anything with them.
> Please, correct me if i am wrong. Anyway, good question.
I see. A comment about that wouldn't hurt.
Or, how about excluding all constructors with less than 2 initializers? That's
intuitive, probably more efficient and includes delegating constructors :).
Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:29
@@ +28,3 @@
+namespace clang {
+using namespace ast_matchers;
+
Put
using namespace clang::ast_matchers;
into the namespace reorder_fields. Otherwise you are polluting the namespace
clang.
Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:82
@@ +81,3 @@
+// FIXME: error-handling
+/// \brief Replaces one range of source code by another.
+static void
If this is what it is doing, maybe call it swapSourceRanges?
Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:133
@@ +132,3 @@
+///
+/// A constructor can have initializers for an arbitrary subset of the classes
+/// fields.
- class's fields (or rephrase)
- no need to add a newline after the first sentence
- ".. aRE present"
Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:177
@@ +176,3 @@
+/// \returns true on success
+static bool reorderFieldsInInitListExpr(
+const InitListExpr *InitListEx, ArrayRef NewFieldsOrder,
While it's nice that this has a bool return value now, you aren't using it.
Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:182
@@ +181,3 @@
+ assert(InitListEx && "Init list expression is null");
+ if (!InitListEx->isExplicit() || !InitListEx->getNumInits())
+return true;
Maybe instead getNumInits() <= 1?
Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:231
@@ +230,3 @@
+// We only need to reorder init list expressions for aggregate types.
+// Now (v0) partial initialization is not supported.
+// For other types the order of constructor parameters is used,
I'd move this sentence to the end of the comment.
Repository:
rL LLVM
https://reviews.llvm.org/D23279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r280165 - clang-format: Correctly calculate affected ranges when sorting #includes.
Author: djasper
Date: Tue Aug 30 16:33:41 2016
New Revision: 280165
URL: http://llvm.org/viewvc/llvm-project?rev=280165&view=rev
Log:
clang-format: Correctly calculate affected ranges when sorting #includes.
affectedRanges takes a start and an end offset, not offset and length.
Modified:
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/unittests/Format/SortIncludesTest.cpp
Modified: cfe/trunk/lib/Format/Format.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=280165&r1=280164&r2=280165&view=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Tue Aug 30 16:33:41 2016
@@ -1263,10 +1263,10 @@ static void sortCppIncludes(const Format
ArrayRef Ranges, StringRef
FileName,
tooling::Replacements &Replaces, unsigned *Cursor)
{
unsigned IncludesBeginOffset = Includes.front().Offset;
- unsigned IncludesBlockSize = Includes.back().Offset +
- Includes.back().Text.size() -
- IncludesBeginOffset;
- if (!affectsRange(Ranges, IncludesBeginOffset, IncludesBlockSize))
+ unsigned IncludesEndOffset =
+ Includes.back().Offset + Includes.back().Text.size();
+ unsigned IncludesBlockSize = IncludesEndOffset - IncludesBeginOffset;
+ if (!affectsRange(Ranges, IncludesBeginOffset, IncludesEndOffset))
return;
SmallVector Indices;
for (unsigned i = 0, e = Includes.size(); i != e; ++i)
Modified: cfe/trunk/unittests/Format/SortIncludesTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/SortIncludesTest.cpp?rev=280165&r1=280164&r2=280165&view=diff
==
--- cfe/trunk/unittests/Format/SortIncludesTest.cpp (original)
+++ cfe/trunk/unittests/Format/SortIncludesTest.cpp Tue Aug 30 16:33:41 2016
@@ -24,8 +24,8 @@ protected:
return std::vector(1, tooling::Range(0, Code.size()));
}
- std::string sort(StringRef Code, StringRef FileName = "input.cpp") {
-auto Ranges = GetCodeRange(Code);
+ std::string sort(StringRef Code, std::vector Ranges,
+ StringRef FileName = "input.cc") {
auto Replaces = sortIncludes(Style, Code, Ranges, FileName);
Ranges = tooling::calculateRangesAfterReplacements(Replaces, Ranges);
auto Sorted = applyAllReplacements(Code, Replaces);
@@ -36,6 +36,10 @@ protected:
return *Result;
}
+ std::string sort(StringRef Code, StringRef FileName = "input.cpp") {
+return sort(Code, GetCodeRange(Code), FileName);
+ }
+
unsigned newCursor(llvm::StringRef Code, unsigned Cursor) {
sortIncludes(Style, Code, GetCodeRange(Code), "input.cpp", &Cursor);
return Cursor;
@@ -52,6 +56,14 @@ TEST_F(SortIncludesTest, BasicSorting) {
sort("#include \"a.h\"\n"
"#include \"c.h\"\n"
"#include \"b.h\"\n"));
+
+ EXPECT_EQ("// comment\n"
+"#include \n"
+"#include \n",
+sort("// comment\n"
+ "#include \n"
+ "#include \n",
+ {tooling::Range(25, 1)}));
}
TEST_F(SortIncludesTest, NoReplacementsForValidIncludes) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23279: clang-reorder-fields
djasper added inline comments.
Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:178
@@ +177,3 @@
+const InitListExpr *InitListEx, ArrayRef NewFieldsOrder,
+const ASTContext &Context,
+std::map &Replacements) {
alexshap wrote:
> yeah, i am aware of it. In general - yes, you are right.
> But to me it seems that for now it would be better to leave it as is (don't
> drop all the other replacements because of this one case - now we write a
> message about this issue to llvm::errs()).
> The other options don't look very good to me (or at least better) so decided
> not to make things complicated (for now). Probably i need to provide more
> context / explanations. Right now in my code there are two places where we
> return true/false to signal about an issue and also write a message to
> llvm::errs(). The first one is reorderFieldsInDefinition and the second one
> is reorderFieldsInInitListExpr. The first seems to be critical (if we are
> not able to edit the definition doing anything else doesn't make sense - now
> it works this way - so we should be good), the second
> (reorderFieldsInInitListExpr) doesn't seem to be so critical (partial
> initialization of an aggregate), so i would not like to drop all the other
> replacements because of it. Okay, that was one part of the story. The other
> part of the story:
> A.
>void HandleTranslationUnit(ASTContext &Context) override
> - it doesn't return anything (so i can not propagate the return code) (to be
> honest i can, but not sure if those changes are really worth doing (taking
> into account the context i described above))
> B.
>http://clang.llvm.org/doxygen/CompilerInstance_8cpp_source.html#l00871
>line 871 Act.Execute(); (return value is ignored)
> -- but this seems to be beyond the scope of my diff, and as i said above -
> doing smth more complicated only for partial initialization (which is now not
> supported by this tool anyway) - not sure if it's worth doing right now.
If you don't reorder a partial initializer then I think it is quite likely that
the resulting code doesn't compile or, worse, compiles but has incorrect
behavior. I don't think this is any better or worse than not reordering fields
across different access specifiers.
Also, I don't understand what the problem is. Just create separate local
variable for Replacements and only copy to the class member if everything is
successful. Or clear Replacements if something goes wrong. Am I missing
something?
Repository:
rL LLVM
https://reviews.llvm.org/D23279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23279: clang-reorder-fields
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Now that you can, you should add test cases for the different cases where you cannot reorder fields. Otherwise looks good. Repository: rL LLVM https://reviews.llvm.org/D23279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23279: clang-reorder-fields
djasper added inline comments.
Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:179
@@ +178,3 @@
+const ASTContext &Context,
+std::map &Replacements) {
+ assert(InitListEx && "Init list expression is null");
alexshap wrote:
> >Also, I don't understand what the problem is. Just create separate local
> >variable for >Replacements and only copy to the class member if everything
> >is successful. Or clear >Replacements if something goes wrong. Am I missing
> >something?
> the return code. I assumed that in this case the return code should not be 0,
> but pls take a look at my comments above (regarding the signature of
> HandleTranslationUnit and
> http://clang.llvm.org/doxygen/CompilerInstance_8cpp_source.html#l00871
> line 871 Act.Execute(); (return value is ignored) ) it looks like the
> return code is ignored at several layers - i might be missing smth - pls,
> correct me if i am wrong.
I don't really care about the return code. To me it's important that the tool
doesn't do the wrong thing and that seems fine now. We can sort this out in a
follow-up, I think.
Repository:
rL LLVM
https://reviews.llvm.org/D23279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23279: clang-reorder-fields
djasper added a comment. As per my comment, please add tests for cases where you currently don't do re-ordering (different access specifiers, partial initializers). Other than that, yes, this is fine to commit. Repository: rL LLVM https://reviews.llvm.org/D23279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23279: clang-reorder-fields
djasper added inline comments. Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:181 @@ +180,3 @@ + assert(InitListEx && "Init list expression is null"); + // we care only about brace initializations + // which have valid source locations Write full sentences: // We care only about brace initializations which have valid source locations. Same for all the other comments you just added. Also, it'd be cool if the comment would say why we only care about init list exprs with valid source locations or in which cases the source locations won't be valid. Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:185 @@ +184,3 @@ +return true; + const auto *SyntacticForm = InitListEx->getSyntacticForm(); + // getSyntacticForm() may return nullptr indicating that Can you make this if (const auto *SyntacticForm = InitListEx->getSyntacticForm()) InitListEx = SyntacticForm; ? Repository: rL LLVM https://reviews.llvm.org/D23279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23279: clang-reorder-fields
djasper added a comment. Looks good :). Repository: rL LLVM https://reviews.llvm.org/D23279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r280245 - clang-format: Set default WebKit style to use C++11.
Author: djasper
Date: Wed Aug 31 09:05:56 2016
New Revision: 280245
URL: http://llvm.org/viewvc/llvm-project?rev=280245&view=rev
Log:
clang-format: Set default WebKit style to use C++11.
The WebKit style page says to use nullptr, so this should be fine:
https://webkit.org/code-style-guidelines/
This fixes: llvm.org/PR30220
Modified:
cfe/trunk/lib/Format/Format.cpp
Modified: cfe/trunk/lib/Format/Format.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=280245&r1=280244&r2=280245&view=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Wed Aug 31 09:05:56 2016
@@ -685,7 +685,6 @@ FormatStyle getWebKitStyle() {
Style.ObjCBlockIndentWidth = 4;
Style.ObjCSpaceAfterProperty = true;
Style.PointerAlignment = FormatStyle::PAS_Left;
- Style.Standard = FormatStyle::LS_Cpp03;
return Style;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24155: clang-format: [JS] merge requoting replacements.
djasper accepted this revision.
djasper added a comment.
This revision is now accepted and ready to land.
Basically looks good.
Comment at: lib/Format/Format.cpp:806
@@ -805,2 +805,3 @@
FormatTokenLexer &Tokens, tooling::Replacements &Result) override {
+tooling::Replacements RunResult;
deriveLocalStyle(AnnotatedLines);
Call this "RequoteChanges".
Comment at: lib/Format/Format.cpp:831
@@ -830,1 +830,3 @@
+RunResult = RunResult.merge(Whitespaces.generateReplacements());
+return RunResult;
}
Just
return RequoteChanges.merge(Whitespaces.generateReplacements());
https://reviews.llvm.org/D24155
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23972: clang-format: [JS] Sort all JavaScript imports if any changed.
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. https://reviews.llvm.org/D23972 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23973: clang-format: [JS] handle default bindings in imports.
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. It would be helpful to have a before/after to review these patches. https://reviews.llvm.org/D23973 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24257: clang-format: [JS] ignore comments when wrapping returns.
djasper accepted this revision.
djasper added a comment.
This revision is now accepted and ready to land.
Looks good.
Comment at: lib/Format/TokenAnnotator.cpp:2384
@@ -2383,2 +2383,3 @@
} else if (Style.Language == FormatStyle::LK_JavaScript) {
-if (Left.is(tok::kw_return))
+const FormatToken *NonComment = Right.getPreviousNonComment();
+if (Left.isOneOf(tok::kw_return, tok::kw_continue, tok::kw_break,
mprobst wrote:
> Should the entire method generally consider the previous non-comment, instead
> of just the previous token? Not sure if that makes sense in all cases, and I
> don't think we always have a previous non-comment token either.
Lets leave this as is for now. Yes, some of these should likely look past
comments, but probably not all of them.
https://reviews.llvm.org/D24257
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24272: clang-format: [JS] whitespace required between ! and as.
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. https://reviews.llvm.org/D24272 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
