[PATCH] D149088: [clang-format] Add run-clang-format.py script.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

I guess I need to add a test in a similar that run-clang-tidy is tested, but 
I'd like to gather some initial review still.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149088/new/

https://reviews.llvm.org/D149088

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149088: [clang-format] Add run-clang-format.py script.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius created this revision.
Herald added projects: All, clang, clang-format.
Herald added a subscriber: cfe-commits.
Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay.
curdeius requested review of this revision.
Herald added a comment.

NOTE: Clang-Format Team Automated Review Comment

It looks like your clang-format review does not contain any unit tests, please 
try to ensure all code changes have a unit test (unless this is an `NFC` or 
refactoring, adding documentation etc..)

Add your unit tests in `clang/unittests/Format` and you can build with `ninja 
FormatTests`.  We recommend using the `verifyFormat(xxx)` format of unit tests 
rather than `EXPECT_EQ` as this will ensure you change is tolerant to random 
whitespace changes (see FormatTest.cpp as an example)

For situations where your change is altering the TokenAnnotator.cpp which can 
happen if you are trying to improve the annotation phase to ensure we are 
correctly identifying the type of a token, please add a token annotator test in 
`TokenAnnotatorTest.cpp`


Fixes https://github.com/llvm/llvm-project/issues/62108.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149088

Files:
  clang/tools/CMakeLists.txt
  clang/tools/run-clang-format.py

Index: clang/tools/run-clang-format.py
===
--- /dev/null
+++ clang/tools/run-clang-format.py
@@ -0,0 +1,167 @@
+#!/usr/bin/env python3
+#
+#===- run-clang-format.py - Parallel clang-format runner *- python -*--===#
+#
+# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+# See https://llvm.org/LICENSE.txt for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+#
+#===---===#
+
+"""
+Parallel clang-format runner
+==
+
+Runs clang-format over all files in given directories. Requires clang-format in $PATH.
+
+Example invocations.
+- Run clang-format on all files in the current working directory.
+run-clang-format.py
+
+- Run clang-format on all files in the chosen directories.
+run-clang-format.py dir1 dir2 dir3
+"""
+
+
+from __future__ import print_function
+import argparse
+import fnmatch
+import os
+import multiprocessing
+import queue
+import subprocess
+import sys
+import threading
+
+
+def glob_files(args):
+files = []
+
+extensions = args.extensions.split(',')
+
+for directory in args.directories:
+for root, _, filenames in os.walk(directory):
+for ext in extensions:
+for filename in fnmatch.filter(filenames, '*.' + ext):
+files.append(os.path.join(root, filename))
+
+return files
+
+
+def parse_args(argv=None):
+if argv is None:
+argv = sys.argv
+parser = argparse.ArgumentParser(
+description='Runs clang-format over all files in given directories.'
+' Requires clang-format in PATH.')
+parser.add_argument('--clang-format-binary', metavar='PATH',
+default='clang-format',
+help='path to clang-format binary')
+parser.add_argument('-e', '--extensions', dest='extensions',
+help='comma-delimited list of extensions used to glob source files',
+default="c,cc,cpp,cxx,c++,h,hh,hpp,hxx,h++")
+parser.add_argument('-style',
+help='formatting style',
+default="file")
+parser.add_argument('--no-inplace', dest='inplace', action='store_false',
+help='do not format files inplace, but write output to the console'
+' (useful for debugging)',
+default=True)
+parser.add_argument('-j', metavar='THREAD_COUNT', type=int, default=0,
+help='number of clang-format instances to be run in parallel')
+parser.add_argument('-v', '--verbose', action='store_true',
+help='output verbose comments')
+parser.add_argument(metavar='DIRPATH', dest='directories', nargs='*',
+help='path(s) used to glob source files')
+
+args = parser.parse_args(argv[1:])
+
+if not args.directories:
+args.directories = [os.getcwd()]
+
+check_clang_format_binary(args)
+
+return args
+
+
+def _get_format_invocation(args, filename):
+invocation = [args.clang_format_binary]
+invocation.append('-style=' + args.style)
+if args.inplace:
+invocation.append('-i')
+
+invocation.append(filename)
+return invocation
+
+
+def check_clang_format_binary(args):
+"""Checks if invoking supplied clang-format binary works."""
+try:
+subprocess.check_output([args.clang_format_binary, '--version'])
+except OSError:
+print('Unable to run clang-format. Is clang-format '
+  'binary correctly specified?', 

[PATCH] D137338: Fix dupe word typos

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/lib/CodeGen/CodeGenTBAA.cpp:341
 if (const CXXRecordDecl *CXXRD = dyn_cast(RD)) {
-  // Handle C++ base classes. Non-virtual bases can treated a a kind of
+  // Handle C++ base classes. Non-virtual bases can treated a kind of
   // field. Virtual bases are more complex and omitted, but avoid an

aaron.ballman wrote:
> I think this should read:
> ```
>  // Handle C++ base classes. Non-virtual bases can treated as a kind of
> ```
A mistake: can *be* treated.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137338/new/

https://reviews.llvm.org/D137338

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133589: [clang-format] JSON formatting add new option for controlling newlines in json arrays

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:3115
 
+**JsonMultilineArrays** (``Boolean``) :versionbadge:`clang-format 16`
+  If ``true``, clang-format will always break after a Json array `[`

Why limiting to JSON only?
Could we name it in a general fashion (we comment that it's JSON only for the 
time being). I believe it may be an interesting option for various languages.

How about BreakMultilineArrays, or just BreakArrays to follow the naming of 
existing options a bit?



Comment at: clang/lib/Format/TokenAnnotator.cpp:4413
+  while (Tok) {
+if (Tok->isOneOf(tok::l_brace, tok::l_square)) {
+  return true;

Please elide braces in the ifs.



Comment at: clang/lib/Format/TokenAnnotator.cpp:4427
+if (Left.is(tok::comma)) {
+  if (Right.is(tok::l_brace)) {
+return true;

You can elide braces here.



Comment at: clang/lib/Format/TokenAnnotator.cpp:4434
+  while (Tok) {
+if (Tok->isOneOf(tok::l_brace, tok::l_square)) {
+  return true;

You might elide braces in both ifs.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133589/new/

https://reviews.llvm.org/D133589

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133571: [clang-format] Introduce NoFallThrough option into AllowShortCaseLabelsOnASingleLine

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM after fixing the last comment.




Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:650
+  // Don't merge if the last thing on the line before was an attribute.
+  if (PreviousLine->Last->Previous && PreviousLine->Last->Previous) {
+auto *PrevPrev = PreviousLine->Last->Previous->Previous;

Haven't seen this before.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133571/new/

https://reviews.llvm.org/D133571

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133571: [clang-format] Introduce NoFallThrough option into AllowShortCaseLabelsOnASingleLine

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:935
+**AllowShortCaseLabelsOnASingleLine** (``ShortCaseLabelStyle``) 
:versionbadge:`clang-format 3.6`
+  Determine if Short case labels will be contracted to a single line.
+





Comment at: clang/docs/ClangFormatStyleOptions.rst:3596
 
-  QualifierOrder: ['inline', 'static', 'type', 'const']
+  QualifierOrder: ['inline', 'static' , 'type', 'const']
 

Unrelated change.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:643-657
+bool NoFallThrough = Style.AllowShortCaseLabelsOnASingleLine ==
+ FormatStyle::SCLS_NoFallThrough;
+// If the last thing on the line before was just case X:  then don't merge.
+if (NoFallThrough && PreviousLine && PreviousLine->Last &&
+PreviousLine->Last->is(tok::colon))
+  return 0;
+// Check if the last thing on the line before was an attribute and if so

Can we merge the conditions like this?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133571/new/

https://reviews.llvm.org/D133571

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132762: [clang-format] Allow `throw` to be a keyword in front of casts

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.

@rymiel, please provide your name and email address for the commit message, so 
that we can land it for you.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132762/new/

https://reviews.llvm.org/D132762

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132295: [clang-format] Change heuristic for locating lambda template arguments

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.

@rymiel, please provide your name and email address for the commit message, so 
that we can land it for you.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132295/new/

https://reviews.llvm.org/D132295

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132189: [clang-format] Don't put `noexcept` on empty line following constructor

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.

@rymiel, please provide your name and email address for the commit message, so 
that we can land it for you.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132189/new/

https://reviews.llvm.org/D132189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126365: [git-clang-format] Stop ignoring changes for files with space in path

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

@Eitot, please provide your name and email address for the commit message.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126365/new/

https://reviews.llvm.org/D126365

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131978: [clang-format] Concepts: allow identifiers after negation

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.

@rymiel, please provide your name and email address for the commit message, so 
that it can land it for you.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131978/new/

https://reviews.llvm.org/D131978

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129926: [clang-format] Handle constructor invocations after new operator in C# correct

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

@eoanermine, please provide your name and email address for the commit message.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129926/new/

https://reviews.llvm.org/D129926

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130299: [clang-format] FIX: Misannotation 'auto' as trailing return type in lambdas

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added inline comments.



Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:752
 
+TEST_F(TokenAnnotatorTest, UnderstandsTrailingReturnTypeAutoInLabmdas) {
+  auto Tokens = annotate("[]() -> auto {}");

owenpan wrote:
> HazardyKnusperkeks wrote:
> > I'd just merge it with the test above.
> > I'd just merge it with the test above.
> 
> +1.



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130299/new/

https://reviews.llvm.org/D130299

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130417: [clang-format] Missing space between trailing return type 'auto' and left brace

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:23555-23556
+  FormatStyle Style = getLLVMStyle();
+  verifyFormat("[]() -> auto { return Val; }", Style);
+  verifyFormat("auto foo() -> auto { return Val; }", Style);
+}

owenpan wrote:
> Should we add test cases with an `&` between `auto` and `{`?
:+1:


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130417/new/

https://reviews.llvm.org/D130417

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130411: [clang-format] Fix a hang when formatting C# $@ string literals

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added inline comments.



Comment at: clang/lib/Format/FormatTokenLexer.cpp:536
+  // $"{x ?? "null"}"
+  // should not be split into $"{x ?? ", null, "}" but should treated as a
+  // single string-literal.




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130411/new/

https://reviews.llvm.org/D130411

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130299: [clang-format] FIX: Misformatting lambdas with trailing return type 'auto' in braced lists

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

Could you please add full git context?
Was the problem due to misannotation of `auto`? If so, could you add an 
annotator test?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130299/new/

https://reviews.llvm.org/D130299

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130136: [clang-format] Indent tokens after hash only if it starts a line

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130136/new/

https://reviews.llvm.org/D130136

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129940: [clang-format] Fix misannotation of colon in presence of requires clause

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:820-833
+  ASSERT_EQ(BaseTokens.size(), NumberOfBaseTokens) << BaseTokens;
+  ASSERT_EQ(ConstrainedTokens.size(),
+NumberOfBaseTokens + NumberOfAdditionalRequiresClauseTokens)
+  << ConstrainedTokens;
+
+  for (auto I = 0u; I < NumberOfBaseTokens; ++I) {
+if (I < NumberOfTokensBeforeRequires) {

HazardyKnusperkeks wrote:
> curdeius wrote:
> > owenpan wrote:
> > > Can you make it a function or lambda?
> > :+1:
> Often thought about that. But as @MyDeveloperDay mentioned in different other 
> reviews, we would loose the line where the EXPECT failed, since it would 
> always be the same line.
> 
> One step to mitigate that would be to return a `bool`, then one would loose 
> the "subexpect", only knows which subtest failed.
> 
> But an idea I have right now would be to add a StringRef parameter which is 
> then fed into the expect/assert to identify the subtest.
You can then add a function taking a source location maybe (+ optionally a 
macro passing `__FILE__` and `__LINE__`). WDYT?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129940/new/

https://reviews.llvm.org/D129940

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129926: [clang-format] Handle constructor invocations after new operator in C# correct

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.

LGTM.




Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2864
 do {
+  // Handle constructor invocation
+  if (FormatTok->is(tok::l_paren))

Nit: comments should be full phrases, ending with full stops 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129926/new/

https://reviews.llvm.org/D129926

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129940: [clang-format] Fix misannotation of colon in presence of requires clause

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.

Ok for me if it's OK for Owen.




Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:820-833
+  ASSERT_EQ(BaseTokens.size(), NumberOfBaseTokens) << BaseTokens;
+  ASSERT_EQ(ConstrainedTokens.size(),
+NumberOfBaseTokens + NumberOfAdditionalRequiresClauseTokens)
+  << ConstrainedTokens;
+
+  for (auto I = 0u; I < NumberOfBaseTokens; ++I) {
+if (I < NumberOfTokensBeforeRequires) {

owenpan wrote:
> Can you make it a function or lambda?
:+1:


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129940/new/

https://reviews.llvm.org/D129940

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129921: [clang-format] Never remove braces in macro definitions

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

Nice finding! LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129921/new/

https://reviews.llvm.org/D129921

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129845: [clang-format] Allow custom attributes in access specifiers

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:3298
"  void f6() {}\n"
"signals:\n"
"  void g1();\n"

In the same vein, could you add test for `slots:` and `Q_SLOTS:` please?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129845/new/

https://reviews.llvm.org/D129845

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129845: Allow custom attributes in access specifiers

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:3304
+  verifyFormat("class A {\n"
+   "public ATTR:\n"
+   "  void f1() {}\n"

How about multiple macros?
E.g. `public ATTR1 ATTR2`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129845/new/

https://reviews.llvm.org/D129845

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129466: [clang-format][NFC] Replace most of std::vector with SmallVector

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:40
 static unsigned getLengthToMatchingParen(const FormatToken ,
- const std::vector ) 
{
+ const SmallVector ) 
{
   // Normally whether or not a break before T is possible is calculated and

owenpan wrote:
> curdeius wrote:
> > Sorry for a late comment. I think that in a parameter list, we should use 
> > `SmallVectorBase` to avoid changing the type if the inplace element count 
> > changes. That's not important now though.
> Hmm. I can't find an example of `SmallVectorBase` in llvm-project sources, 
> though.
Typo, it's Impl: https://llvm.org/doxygen/classllvm_1_1SmallVectorImpl.html


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129466/new/

https://reviews.llvm.org/D129466

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127270: [clang-format] Add space in placement new expression

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added inline comments.



Comment at: clang/include/clang/Format/Format.h:3504
+enum AfterPlacementOperatorStyle : int8_t {
+  /// Removes space after ``new/delete`` operators and before ``(``.
+  /// \code

We have verbs: removes, add, leave. Si I think it should be "remove" here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127270/new/

https://reviews.llvm.org/D127270

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129466: [clang-format][NFC] Replace most of std::vector with SmallVector

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:40
 static unsigned getLengthToMatchingParen(const FormatToken ,
- const std::vector ) 
{
+ const SmallVector ) 
{
   // Normally whether or not a break before T is possible is calculated and

Sorry for a late comment. I think that in a parameter list, we should use 
`SmallVectorBase` to avoid changing the type if the inplace element count 
changes. That's not important now though.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129466/new/

https://reviews.llvm.org/D129466

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129563: [docs] Document git-clang-format

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

In D129563#3645343 , @thakis wrote:

> (does anyone know why git-clang-format isn't implemented in terms of 
> clang-format-diff.py? I suppose it's easier to use if it's standalone?)

I don't know unfortunately.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129563/new/

https://reviews.llvm.org/D129563

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129443: [clang-format] Add option for aligning requires clause body

[Marek Kurdej via Phabricator via cfe-commits]

curdeius requested changes to this revision.
curdeius added a comment.
This revision now requires changes to proceed.

Haven't you forgotten to add formatting tests? :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129443/new/

https://reviews.llvm.org/D129443

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129348: [clang-format] Fix an assertion failure on -lines=0:n

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129348/new/

https://reviews.llvm.org/D129348

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129064: [clang-format] Avoid crash in LevelIndentTracker.

[Marek Kurdej via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG14c30c70c459: [clang-format] Avoid crash in 
LevelIndentTracker. (authored by curdeius).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129064/new/

https://reviews.llvm.org/D129064

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTestSelective.cpp


Index: clang/unittests/Format/FormatTestSelective.cpp
===
--- clang/unittests/Format/FormatTestSelective.cpp
+++ clang/unittests/Format/FormatTestSelective.cpp
@@ -609,6 +609,14 @@
  "  return a == 8 ? 32 : 16;\n"
  "}\n";
   EXPECT_EQ(Code, format(Code, 40, 0));
+
+  // https://llvm.org/PR56352
+  Style.CompactNamespaces = true;
+  Style.NamespaceIndentation = FormatStyle::NI_All;
+  Code = "\n"
+ "namespace ns1 { namespace ns2 {\n"
+ "}}";
+  EXPECT_EQ(Code, format(Code, 0, 0));
 }
 
 } // end namespace
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -66,7 +66,6 @@
   (Style.PPIndentWidth >= 0) ? Style.PPIndentWidth : Style.IndentWidth;
   Indent = Line.Level * IndentWidth + AdditionalIndent;
 } else {
-  IndentForLevel.resize(Line.Level + 1);
   Indent = getIndent(Line.Level);
 }
 if (static_cast(Indent) + Offset >= 0)
@@ -91,6 +90,7 @@
 unsigned LevelIndent = Line.First->OriginalColumn;
 if (static_cast(LevelIndent) - Offset >= 0)
   LevelIndent -= Offset;
+assert(Line.Level < IndentForLevel.size());
 if ((!Line.First->is(tok::comment) || IndentForLevel[Line.Level] == -1) &&
 !Line.InPPDirective) {
   IndentForLevel[Line.Level] = LevelIndent;


Index: clang/unittests/Format/FormatTestSelective.cpp
===
--- clang/unittests/Format/FormatTestSelective.cpp
+++ clang/unittests/Format/FormatTestSelective.cpp
@@ -609,6 +609,14 @@
  "  return a == 8 ? 32 : 16;\n"
  "}\n";
   EXPECT_EQ(Code, format(Code, 40, 0));
+
+  // https://llvm.org/PR56352
+  Style.CompactNamespaces = true;
+  Style.NamespaceIndentation = FormatStyle::NI_All;
+  Code = "\n"
+ "namespace ns1 { namespace ns2 {\n"
+ "}}";
+  EXPECT_EQ(Code, format(Code, 0, 0));
 }
 
 } // end namespace
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -66,7 +66,6 @@
   (Style.PPIndentWidth >= 0) ? Style.PPIndentWidth : Style.IndentWidth;
   Indent = Line.Level * IndentWidth + AdditionalIndent;
 } else {
-  IndentForLevel.resize(Line.Level + 1);
   Indent = getIndent(Line.Level);
 }
 if (static_cast(Indent) + Offset >= 0)
@@ -91,6 +90,7 @@
 unsigned LevelIndent = Line.First->OriginalColumn;
 if (static_cast(LevelIndent) - Offset >= 0)
   LevelIndent -= Offset;
+assert(Line.Level < IndentForLevel.size());
 if ((!Line.First->is(tok::comment) || IndentForLevel[Line.Level] == -1) &&
 !Line.InPPDirective) {
   IndentForLevel[Line.Level] = LevelIndent;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129064: [clang-format] Avoid crash in LevelIndentTracker.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius updated this revision to Diff 442482.
curdeius marked an inline comment as done.
curdeius added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129064/new/

https://reviews.llvm.org/D129064

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTestSelective.cpp


Index: clang/unittests/Format/FormatTestSelective.cpp
===
--- clang/unittests/Format/FormatTestSelective.cpp
+++ clang/unittests/Format/FormatTestSelective.cpp
@@ -609,6 +609,14 @@
  "  return a == 8 ? 32 : 16;\n"
  "}\n";
   EXPECT_EQ(Code, format(Code, 40, 0));
+
+  // https://llvm.org/PR56352
+  Style.CompactNamespaces = true;
+  Style.NamespaceIndentation = FormatStyle::NI_All;
+  Code = "\n"
+ "namespace ns1 { namespace ns2 {\n"
+ "}}";
+  EXPECT_EQ(Code, format(Code, 0, 0));
 }
 
 } // end namespace
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -66,7 +66,6 @@
   (Style.PPIndentWidth >= 0) ? Style.PPIndentWidth : Style.IndentWidth;
   Indent = Line.Level * IndentWidth + AdditionalIndent;
 } else {
-  IndentForLevel.resize(Line.Level + 1);
   Indent = getIndent(Line.Level);
 }
 if (static_cast(Indent) + Offset >= 0)
@@ -91,6 +90,7 @@
 unsigned LevelIndent = Line.First->OriginalColumn;
 if (static_cast(LevelIndent) - Offset >= 0)
   LevelIndent -= Offset;
+assert(Line.Level < IndentForLevel.size());
 if ((!Line.First->is(tok::comment) || IndentForLevel[Line.Level] == -1) &&
 !Line.InPPDirective) {
   IndentForLevel[Line.Level] = LevelIndent;


Index: clang/unittests/Format/FormatTestSelective.cpp
===
--- clang/unittests/Format/FormatTestSelective.cpp
+++ clang/unittests/Format/FormatTestSelective.cpp
@@ -609,6 +609,14 @@
  "  return a == 8 ? 32 : 16;\n"
  "}\n";
   EXPECT_EQ(Code, format(Code, 40, 0));
+
+  // https://llvm.org/PR56352
+  Style.CompactNamespaces = true;
+  Style.NamespaceIndentation = FormatStyle::NI_All;
+  Code = "\n"
+ "namespace ns1 { namespace ns2 {\n"
+ "}}";
+  EXPECT_EQ(Code, format(Code, 0, 0));
 }
 
 } // end namespace
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -66,7 +66,6 @@
   (Style.PPIndentWidth >= 0) ? Style.PPIndentWidth : Style.IndentWidth;
   Indent = Line.Level * IndentWidth + AdditionalIndent;
 } else {
-  IndentForLevel.resize(Line.Level + 1);
   Indent = getIndent(Line.Level);
 }
 if (static_cast(Indent) + Offset >= 0)
@@ -91,6 +90,7 @@
 unsigned LevelIndent = Line.First->OriginalColumn;
 if (static_cast(LevelIndent) - Offset >= 0)
   LevelIndent -= Offset;
+assert(Line.Level < IndentForLevel.size());
 if ((!Line.First->is(tok::comment) || IndentForLevel[Line.Level] == -1) &&
 !Line.InPPDirective) {
   IndentForLevel[Line.Level] = LevelIndent;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129064: [clang-format] Avoid crash in LevelIndentTracker.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius created this revision.
curdeius added reviewers: HazardyKnusperkeks, owenpan, MyDeveloperDay.
Herald added a project: All.
curdeius requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes https://github.com/llvm/llvm-project/issues/56352.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129064

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTestSelective.cpp


Index: clang/unittests/Format/FormatTestSelective.cpp
===
--- clang/unittests/Format/FormatTestSelective.cpp
+++ clang/unittests/Format/FormatTestSelective.cpp
@@ -609,6 +609,14 @@
  "  return a == 8 ? 32 : 16;\n"
  "}\n";
   EXPECT_EQ(Code, format(Code, 40, 0));
+
+  // https://llvm.org/PR56352
+  Style.CompactNamespaces = true;
+  Style.NamespaceIndentation = FormatStyle::NI_All;
+  Code = "\n"
+ "namespace ns1 { namespace ns2 {\n"
+ "}}";
+  EXPECT_EQ(Code, format(Code, 0, 0));
 }
 
 } // end namespace
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -66,7 +66,8 @@
   (Style.PPIndentWidth >= 0) ? Style.PPIndentWidth : Style.IndentWidth;
   Indent = Line.Level * IndentWidth + AdditionalIndent;
 } else {
-  IndentForLevel.resize(Line.Level + 1);
+  if (IndentForLevel.size() < Line.Level + 1)
+IndentForLevel.resize(Line.Level + 1);
   Indent = getIndent(Line.Level);
 }
 if (static_cast(Indent) + Offset >= 0)
@@ -91,6 +92,7 @@
 unsigned LevelIndent = Line.First->OriginalColumn;
 if (static_cast(LevelIndent) - Offset >= 0)
   LevelIndent -= Offset;
+assert(Line.Level < IndentForLevel.size());
 if ((!Line.First->is(tok::comment) || IndentForLevel[Line.Level] == -1) &&
 !Line.InPPDirective) {
   IndentForLevel[Line.Level] = LevelIndent;


Index: clang/unittests/Format/FormatTestSelective.cpp
===
--- clang/unittests/Format/FormatTestSelective.cpp
+++ clang/unittests/Format/FormatTestSelective.cpp
@@ -609,6 +609,14 @@
  "  return a == 8 ? 32 : 16;\n"
  "}\n";
   EXPECT_EQ(Code, format(Code, 40, 0));
+
+  // https://llvm.org/PR56352
+  Style.CompactNamespaces = true;
+  Style.NamespaceIndentation = FormatStyle::NI_All;
+  Code = "\n"
+ "namespace ns1 { namespace ns2 {\n"
+ "}}";
+  EXPECT_EQ(Code, format(Code, 0, 0));
 }
 
 } // end namespace
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -66,7 +66,8 @@
   (Style.PPIndentWidth >= 0) ? Style.PPIndentWidth : Style.IndentWidth;
   Indent = Line.Level * IndentWidth + AdditionalIndent;
 } else {
-  IndentForLevel.resize(Line.Level + 1);
+  if (IndentForLevel.size() < Line.Level + 1)
+IndentForLevel.resize(Line.Level + 1);
   Indent = getIndent(Line.Level);
 }
 if (static_cast(Indent) + Offset >= 0)
@@ -91,6 +92,7 @@
 unsigned LevelIndent = Line.First->OriginalColumn;
 if (static_cast(LevelIndent) - Offset >= 0)
   LevelIndent -= Offset;
+assert(Line.Level < IndentForLevel.size());
 if ((!Line.First->is(tok::comment) || IndentForLevel[Line.Level] == -1) &&
 !Line.InPPDirective) {
   IndentForLevel[Line.Level] = LevelIndent;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129057: [clang-format] Break on AfterColon only if not followed by comment

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks for addressing my comments.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129057/new/

https://reviews.llvm.org/D129057

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129057: [clang-format] Break on AfterColon only if not followed by comment

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:7136
Style);
+  verifyFormat("Constructor() : // NOLINT\n"
+   "() {}",

How about very long comments? They don't get split now? Please add a test case


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129057/new/

https://reviews.llvm.org/D129057

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129057: [clang-format] Break on AfterColon only if not followed by comment

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:7099
OnePerLine);
-  verifyFormat("SomeClass::Constructor() :\n"
+  verifyFormat("SomeClass::Constructor() : // NOLINT\n"
"a(aa), // Some comment\n"

You should add a test without modifying an existing one.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129057/new/

https://reviews.llvm.org/D129057

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128496: [clang-format] Further improve requires clause detection

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM with Owen's suggestion.




Comment at: clang/lib/Format/UnwrappedLineParser.cpp:3185-3188
-  default:
-// It's an expression.
-parseRequiresExpression(RequiresToken);
-return false;

owenpan wrote:
> How about this instead?
> 
:+1:



Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:429
+  Tokens =
+  annotate("auto bar() -> Tempalte requires(is_integral_v) {}");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128496/new/

https://reviews.llvm.org/D128496

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127873: [clang-format] Fix misplacemnt of `*` in declaration of pointer to struct

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2313
 
+if (PrevToken->is(tok::r_brace) && Tok.isOneOf(tok::amp, tok::ampamp))
+  return TT_BinaryOperator;

Instead of checking for `r_brace`, maybe it would be possible to check 
`PrevToken->MatchingParen` for being `TT_RecordLBrace`?
This way, we'd be precise.
WDYT?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127873/new/

https://reviews.llvm.org/D127873

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127270: [clang-format] Add space in placement new expression

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

Does this patch really fix https://github.com/llvm/llvm-project/issues/54703?
If so, please add test for it. Otherwise remove the link from the summary (and 
if possible handle it in another review).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127270/new/

https://reviews.llvm.org/D127270

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127484: [clang-format] Use tabs on GNU style

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

It seems like a breaking change that may be painful for users of GNU style. 
@mydeveloperday, wdyt?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127484/new/

https://reviews.llvm.org/D127484

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127390: [clang-format][NFC] Remove unused FormatStyle members

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

Good finding!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127390/new/

https://reviews.llvm.org/D127390

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127366: [clang-format][NFC] Format lib/Format and unittests/Format in clang

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM.




Comment at: clang/unittests/Format/FormatTest.cpp:25442
"} while (hasMore() && Filter(*I));",
-   "do { ++I; } while (hasMore() && Filter(*I));",
-   Style);
+   "do { ++I; } while (hasMore() && Filter(*I));", Style);
 

This line was just left unformatted, right? Or there was a recent change in the 
behaviour?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127366/new/

https://reviews.llvm.org/D127366

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127260: [clang-format] Remove braces of else blocks that embody an if block

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

Still looks good. Was there a particular case where the previous version didn't 
work?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127260/new/

https://reviews.llvm.org/D127260

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126365: [git-clang-format] Stop ignoring changes for files with space in path

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

@Eitot, do you need help landing this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126365/new/

https://reviews.llvm.org/D126365

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127260: [clang-format] Remove braces of else blocks that embody an if block

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks!




Comment at: clang/unittests/Format/FormatTest.cpp:25570-25575
+   "  else {\n"
+   "if (d) {\n"
+   "  e;\n"
+   "  f;\n"
+   "}\n"
+   "  }\n"

owenpan wrote:
> curdeius wrote:
> > Why isn't this block changed like this?
> > I might be missing something...
> It would cause a dangling `else` error because the last `else` would be 
> paired with the inner merged `else if`.
True. There are indeed no braces in the outer if.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127260/new/

https://reviews.llvm.org/D127260

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127270: [clang-format] Add space in placement new expression

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

Apart from some missing tests, looks promising!




Comment at: clang/unittests/Format/FormatTest.cpp:15276-15286
+  FormatStyle SpacePlacementNew = getLLVMStyle();
+  SpacePlacementNew.SpaceBeforeParens = FormatStyle::SBPO_Custom;
+  SpacePlacementNew.SpaceBeforeParensOptions.AfterPlacementNew = true;
+  verifyFormat("new (buf) T;", SpacePlacementNew);
+  verifyFormat("T *p = new (buf) T;", SpacePlacementNew);
+  verifyFormat("T *p = new (buf) T(3);", SpacePlacementNew);
+  verifyFormat("T *new() {}", SpacePlacementNew);

Are there any tests with `AfterPlacementNew = false;`? Could you add those 
please?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127270/new/

https://reviews.llvm.org/D127270

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127260: [clang-format] Remove braces of else blocks that embody an if block

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:25570-25575
+   "  else {\n"
+   "if (d) {\n"
+   "  e;\n"
+   "  f;\n"
+   "}\n"
+   "  }\n"

Why isn't this block changed like this?
I might be missing something...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127260/new/

https://reviews.llvm.org/D127260

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127054: [clang-format] Handle attributes for for/while loops

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

Great!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127054/new/

https://reviews.llvm.org/D127054

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126365: [git-clang-format] Stop ignoring changes for files with space in path

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.

Ok, I'm not blocking this patch. I'll take a look to see whether we can add 
some tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126365/new/

https://reviews.llvm.org/D126365

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126758: [clang-format] Handle do-while loops for RemoveBracesLLVM

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/unittests/Format/FormatTest.cpp:25103
 
-  // The following eight test cases are fully-braced versions of the examples 
at
+  // The following nine test cases are fully-braced versions of the examples at
   // "llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-

Nit. To avoid updating it every time we add an example.



Comment at: clang/unittests/Format/FormatTest.cpp:25407-25411
+  verifyFormat("do {\n"
+   "  ++I;\n"
+   "} while (hasMore() && Filter(*I));",
+   "do { ++I; } while (hasMore() && Filter(*I));",
+   Style);

What's the value of testing that newlines are added? That's not really the job 
of Insert/RemoveBraces.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126758/new/

https://reviews.llvm.org/D126758

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125848: [clang-format] Handle attributes in enum declaration.

[Marek Kurdej via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8f70d16c9ab2: [clang-format] Handle attributes in enum 
declaration. (authored by tchatow, committed by curdeius).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125848/new/

https://reviews.llvm.org/D125848

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3573,6 +3573,7 @@
   verifyFormat("enum X E {} d;");
   verifyFormat("enum __attribute__((...)) E {} d;");
   verifyFormat("enum __declspec__((...)) E {} d;");
+  verifyFormat("enum [[nodiscard]] E {} d;");
   verifyFormat("enum {\n"
"  Bar = Foo::value\n"
"};",
@@ -3619,6 +3620,17 @@
"};",
EightIndent);
 
+  verifyFormat("enum [[nodiscard]] E {\n"
+   "  ONE,\n"
+   "  TWO,\n"
+   "};");
+  verifyFormat("enum [[nodiscard]] E {\n"
+   "  // Comment 1\n"
+   "  ONE,\n"
+   "  // Comment 2\n"
+   "  TWO,\n"
+   "};");
+
   // Not enums.
   verifyFormat("enum X f() {\n"
"  a();\n"
@@ -3666,7 +3678,19 @@
   verifyFormat("enum struct X E {} d;");
   verifyFormat("enum struct __attribute__((...)) E {} d;");
   verifyFormat("enum struct __declspec__((...)) E {} d;");
+  verifyFormat("enum struct [[nodiscard]] E {} d;");
   verifyFormat("enum struct X f() {\n  a();\n  return 42;\n}");
+
+  verifyFormat("enum struct [[nodiscard]] E {\n"
+   "  ONE,\n"
+   "  TWO,\n"
+   "};");
+  verifyFormat("enum struct [[nodiscard]] E {\n"
+   "  // Comment 1\n"
+   "  ONE,\n"
+   "  // Comment 2\n"
+   "  TWO,\n"
+   "};");
 }
 
 TEST_F(FormatTest, FormatsEnumClass) {
@@ -3683,7 +3707,19 @@
   verifyFormat("enum class X E {} d;");
   verifyFormat("enum class __attribute__((...)) E {} d;");
   verifyFormat("enum class __declspec__((...)) E {} d;");
+  verifyFormat("enum class [[nodiscard]] E {} d;");
   verifyFormat("enum class X f() {\n  a();\n  return 42;\n}");
+
+  verifyFormat("enum class [[nodiscard]] E {\n"
+   "  ONE,\n"
+   "  TWO,\n"
+   "};");
+  verifyFormat("enum class [[nodiscard]] E {\n"
+   "  // Comment 1\n"
+   "  ONE,\n"
+   "  // Comment 2\n"
+   "  TWO,\n"
+   "};");
 }
 
 TEST_F(FormatTest, FormatsEnumTypes) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -3419,11 +3419,18 @@
 
   while (FormatTok->Tok.getIdentifierInfo() ||
  FormatTok->isOneOf(tok::colon, tok::coloncolon, tok::less,
-tok::greater, tok::comma, tok::question)) {
+tok::greater, tok::comma, tok::question,
+tok::l_square, tok::r_square)) {
 nextToken();
 // We can have macros or attributes in between 'enum' and the enum name.
 if (FormatTok->is(tok::l_paren))
   parseParens();
+if (FormatTok->is(TT_AttributeSquare)) {
+  parseSquare();
+  // Consume the closing TT_AttributeSquare.
+  if (FormatTok->Next && FormatTok->is(TT_AttributeSquare))
+nextToken();
+}
 if (FormatTok->is(tok::identifier)) {
   nextToken();
   // If there are two identifiers in a row, this is likely an elaborate


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3573,6 +3573,7 @@
   verifyFormat("enum X E {} d;");
   verifyFormat("enum __attribute__((...)) E {} d;");
   verifyFormat("enum __declspec__((...)) E {} d;");
+  verifyFormat("enum [[nodiscard]] E {} d;");
   verifyFormat("enum {\n"
"  Bar = Foo::value\n"
"};",
@@ -3619,6 +3620,17 @@
"};",
EightIndent);
 
+  verifyFormat("enum [[nodiscard]] E {\n"
+   "  ONE,\n"
+   "  TWO,\n"
+   "};");
+  verifyFormat("enum [[nodiscard]] E {\n"
+   "  // Comment 1\n"
+   "  ONE,\n"
+   "  // Comment 2\n"
+   "  TWO,\n"
+   "};");
+
   // Not enums.
   verifyFormat("enum X f() {\n"
"  a();\n"
@@ -3666,7 +3678,19 @@
   verifyFormat("enum struct X E {} d;");
   verifyFormat("enum struct __attribute__((...)) E {} d;");
   verifyFormat("enum struct __declspec__((...)) E {} d;");
+  verifyFormat("enum struct 

[PATCH] D126096: [clang-format] Fix QualifierAlignment with global namespace qualified types.

[Marek Kurdej via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd4d28f2ace76: [clang-format] Fix QualifierAlignment with 
global namespace qualified types. (authored by curdeius).

Changed prior to commit:
  https://reviews.llvm.org/D126096?vs=431074=432258#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126096/new/

https://reviews.llvm.org/D126096

Files:
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/unittests/Format/QualifierFixerTest.cpp

Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -318,6 +318,8 @@
   verifyFormat("Foo const ", "const Foo ", Style);
   verifyFormat("Foo::iterator const ", "const Foo::iterator ",
Style);
+  verifyFormat("::Foo::iterator const ", "const ::Foo::iterator ",
+   Style);
 
   verifyFormat("Foo(int a, "
"unsigned b, // c-style args\n"
@@ -355,6 +357,8 @@
 
   verifyFormat("void fn(Foo const );", "void fn(const Foo );", Style);
   verifyFormat("void fns(ns::S const );", "void fns(const ns::S );", Style);
+  verifyFormat("void fns(::ns::S const );", "void fns(const ::ns::S );",
+   Style);
   verifyFormat("void fn(ns::Foo const );", "void fn(const ns::Foo );",
Style);
   verifyFormat("void fns(ns::ns2::S const );",
@@ -445,6 +449,8 @@
   verifyFormat("const Foo ", "Foo const ", Style);
   verifyFormat("const Foo::iterator ", "Foo::iterator const ",
Style);
+  verifyFormat("const ::Foo::iterator ", "::Foo::iterator const ",
+   Style);
 
   verifyFormat("const int a;", "int const a;", Style);
   verifyFormat("const int *a;", "int const *a;", Style);
@@ -508,6 +514,8 @@
 
   verifyFormat("void fn(const Foo );", "void fn(Foo const );", Style);
   verifyFormat("void fns(const ns::S );", "void fns(ns::S const );", Style);
+  verifyFormat("void fns(const ::ns::S );", "void fns(::ns::S const );",
+   Style);
   verifyFormat("void fn(const ns::Foo );", "void fn(ns::Foo const );",
Style);
   verifyFormat("void fns(const ns::ns2::S );",
Index: clang/lib/Format/QualifierAlignmentFixer.cpp
===
--- clang/lib/Format/QualifierAlignmentFixer.cpp
+++ clang/lib/Format/QualifierAlignmentFixer.cpp
@@ -216,6 +216,29 @@
   if (LeftRightQualifierAlignmentFixer::isPossibleMacro(Tok->Next))
 return Tok;
 
+  auto AnalyzeTemplate =
+  [&](const FormatToken *Tok,
+  const FormatToken *StartTemplate) -> const FormatToken * {
+// Read from the TemplateOpener to TemplateCloser.
+FormatToken *EndTemplate = StartTemplate->MatchingParen;
+if (EndTemplate) {
+  // Move to the end of any template class members e.g.
+  // `Foo::iterator`.
+  if (EndTemplate->startsSequence(TT_TemplateCloser, tok::coloncolon,
+  tok::identifier)) {
+EndTemplate = EndTemplate->Next->Next;
+  }
+}
+if (EndTemplate && EndTemplate->Next &&
+!EndTemplate->Next->isOneOf(tok::equal, tok::l_paren)) {
+  insertQualifierAfter(SourceMgr, Fixes, EndTemplate, Qualifier);
+  // Remove the qualifier.
+  removeToken(SourceMgr, Fixes, Tok);
+  return Tok;
+}
+return nullptr;
+  };
+
   FormatToken *Qual = Tok->Next;
   FormatToken *LastQual = Qual;
   while (Qual && isQualifierOrType(Qual, ConfiguredQualifierTokens)) {
@@ -233,27 +256,24 @@
 return Tok;
   } else if (Tok->startsSequence(QualifierType, tok::identifier,
  TT_TemplateOpener)) {
-// Read from the TemplateOpener to
-// TemplateCloser as in const ArrayRef a; const ArrayRef 
-FormatToken *EndTemplate = Tok->Next->Next->MatchingParen;
-if (EndTemplate) {
-  // Move to the end of any template class members e.g.
-  // `Foo::iterator`.
-  if (EndTemplate->startsSequence(TT_TemplateCloser, tok::coloncolon,
-  tok::identifier)) {
-EndTemplate = EndTemplate->Next->Next;
-  }
-}
-if (EndTemplate && EndTemplate->Next &&
-!EndTemplate->Next->isOneOf(tok::equal, tok::l_paren)) {
-  insertQualifierAfter(SourceMgr, Fixes, EndTemplate, Qualifier);
-  // Remove the qualifier.
-  removeToken(SourceMgr, Fixes, Tok);
-  return Tok;
-}
-  } else if (Tok->startsSequence(QualifierType, tok::identifier)) {
+// `const ArrayRef a;`
+// `const ArrayRef `
+const FormatToken *NewTok = AnalyzeTemplate(Tok, Tok->Next->Next);
+if (NewTok)
+  return NewTok;
+  } else if (Tok->startsSequence(QualifierType, tok::coloncolon,
+ tok::identifier, TT_TemplateOpener)) {
+// `const ::ArrayRef a;`
+// `const ::ArrayRef `
+const FormatToken 

[PATCH] D126438: [clang-format] Fix an invalid code generation in RemoveBracesLLVM

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM. Good catch for this bug!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126438/new/

https://reviews.llvm.org/D126438

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125848: [clang-format] Handle attributes in enum declaration.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.

LGTM. Thanks a lot!
Do you have commit rights or need some help landing this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125848/new/

https://reviews.llvm.org/D125848

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126358: clang-format][NFC] Refactor UnwrappedLineParser::parseBlock()

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM.




Comment at: clang/lib/Format/UnwrappedLineParser.cpp:874
 
-  if (SimpleBlock && !KeepBraces) {
+  auto RemoveBraces = [=]() mutable {
+if (KeepBraces || !SimpleBlock)

owenpan wrote:
> curdeius wrote:
> > Are there many captures here? Wouldn't it be better to be explicit and/or 
> > capture by ref? Do we need a mutable lambda?
> Yes, too many to be explicit for me. They are all integrals/pointers, a 
> couple of which are modified in the lambda, hence by copy and mutable.
Ok. Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126358/new/

https://reviews.llvm.org/D126358

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126358: clang-format][NFC] Refactor UnwrappedLineParser::parseBlock()

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:874
 
-  if (SimpleBlock && !KeepBraces) {
+  auto RemoveBraces = [=]() mutable {
+if (KeepBraces || !SimpleBlock)

Are there many captures here? Wouldn't it be better to be explicit and/or 
capture by ref? Do we need a mutable lambda?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126358/new/

https://reviews.llvm.org/D126358

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126365: [clang-format] Stop ignoring changes for files with space in path

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

Would it be possible to add a test please?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126365/new/

https://reviews.llvm.org/D126365

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126157: [clang-format][NFC] Insert/remove braces in clang/lib/Format/

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2769
 
-  if (Style.isCSharp()) {
 do {

owenpan wrote:
> From 
> https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements,
>  it's unclear whether the braces should be removed when a block has a 
> do-while loop as its single statement. Should I put the braces back and add 
> handling of do-while for `RemoveBracesLLVM`?
It's indeed not clear, I remember having paused over this code for a moment.
I believe that the best course of action is to grep for both cases in the 
codebase and see what's more frequent (and whether there's some sort of 
consensus).



Comment at: clang/lib/Format/WhitespaceManager.cpp:231-239
+  if (Change.Tok->is(TT_LineComment) || !Change.IsInsideToken) {
 LastBlockComment = 
-  else {
-if ((Change.StartOfBlockComment = LastBlockComment))
+  } else {
+if ((Change.StartOfBlockComment = LastBlockComment)) {
   Change.IndentationOffset =
   Change.StartOfTokenColumn -
   Change.StartOfBlockComment->StartOfTokenColumn;

owenpan wrote:
> It would be nice to remove the braces of the `else` here. See 
> https://github.com/llvm/llvm-project/issues/55663.
+1. Would you fix it manually here then and fix it later?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126157/new/

https://reviews.llvm.org/D126157

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125848: [clang-format] Handle attributes in enum declaration.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

Could you please add test cases with non-empty enums both with and without 
comments please like in the bug report?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125848/new/

https://reviews.llvm.org/D125848

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126132: [clang-format] Fix a crash on lambda trailing return type

[Marek Kurdej via Phabricator via cfe-commits]

curdeius closed this revision.
curdeius added a comment.

Closing as it landed in 
https://github.com/llvm/llvm-project/commit/130a9cc0a5e25e3be8ff3738518e86ae3ae0b5ba.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126132/new/

https://reviews.llvm.org/D126132

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126157: [clang-format][NFC] Insert/remove braces in clang/lib/Format/

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

Ok. So we mainly missed braces on complex conditionals. LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126157/new/

https://reviews.llvm.org/D126157

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126132: [clang-format] Fix a crash on lambda trailing return type

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM. It seems it has landed already.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126132/new/

https://reviews.llvm.org/D126132

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125959: [clang-format] Fix a bug in "AfterControlStatement: MultiLine"

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:3213
   Style.BraceWrapping.AfterFunction = true;
-  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_MultiLine;
   Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;

This seems unrelated, is it so really?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125959/new/

https://reviews.llvm.org/D125959

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126096: [clang-format] Fix QualifierAlignment with global namespace qualified types.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

If you can think of other cases that may misbehave, I'll be happy to test (and 
fix) these.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126096/new/

https://reviews.llvm.org/D126096

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126096: [clang-format] Fix QualifierAlignment with global namespace qualified types.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius created this revision.
curdeius added reviewers: HazardyKnusperkeks, owenpan, MyDeveloperDay.
Herald added a project: All.
curdeius requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes https://github.com/llvm/llvm-project/issues/55610.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126096

Files:
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/unittests/Format/QualifierFixerTest.cpp

Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -318,6 +318,8 @@
   verifyFormat("Foo const ", "const Foo ", Style);
   verifyFormat("Foo::iterator const ", "const Foo::iterator ",
Style);
+  verifyFormat("::Foo::iterator const ", "const ::Foo::iterator ",
+   Style);
 
   verifyFormat("Foo(int a, "
"unsigned b, // c-style args\n"
@@ -355,6 +357,8 @@
 
   verifyFormat("void fn(Foo const );", "void fn(const Foo );", Style);
   verifyFormat("void fns(ns::S const );", "void fns(const ns::S );", Style);
+  verifyFormat("void fns(::ns::S const );", "void fns(const ::ns::S );",
+   Style);
   verifyFormat("void fn(ns::Foo const );", "void fn(const ns::Foo );",
Style);
   verifyFormat("void fns(ns::ns2::S const );",
@@ -445,6 +449,8 @@
   verifyFormat("const Foo ", "Foo const ", Style);
   verifyFormat("const Foo::iterator ", "Foo::iterator const ",
Style);
+  verifyFormat("const ::Foo::iterator ", "::Foo::iterator const ",
+   Style);
 
   verifyFormat("const int a;", "int const a;", Style);
   verifyFormat("const int *a;", "int const *a;", Style);
@@ -508,6 +514,8 @@
 
   verifyFormat("void fn(const Foo );", "void fn(Foo const );", Style);
   verifyFormat("void fns(const ns::S );", "void fns(ns::S const );", Style);
+  verifyFormat("void fns(const ::ns::S );", "void fns(::ns::S const );",
+   Style);
   verifyFormat("void fn(const ns::Foo );", "void fn(ns::Foo const );",
Style);
   verifyFormat("void fns(const ns::ns2::S );",
Index: clang/lib/Format/QualifierAlignmentFixer.cpp
===
--- clang/lib/Format/QualifierAlignmentFixer.cpp
+++ clang/lib/Format/QualifierAlignmentFixer.cpp
@@ -214,6 +214,28 @@
   if (LeftRightQualifierAlignmentFixer::isPossibleMacro(Tok->Next))
 return Tok;
 
+  auto AnalyzeTemplate =
+  [&](const FormatToken *Tok,
+  const FormatToken *StartTemplate) -> const FormatToken * {
+// Read from the TemplateOpener to TemplateCloser.
+FormatToken *EndTemplate = StartTemplate->MatchingParen;
+if (EndTemplate) {
+  // Move to the end of any template class members e.g.
+  // `Foo::iterator`.
+  if (EndTemplate->startsSequence(TT_TemplateCloser, tok::coloncolon,
+  tok::identifier))
+EndTemplate = EndTemplate->Next->Next;
+}
+if (EndTemplate && EndTemplate->Next &&
+!EndTemplate->Next->isOneOf(tok::equal, tok::l_paren)) {
+  insertQualifierAfter(SourceMgr, Fixes, EndTemplate, Qualifier);
+  // Remove the qualifier.
+  removeToken(SourceMgr, Fixes, Tok);
+  return Tok;
+}
+return nullptr;
+  };
+
   FormatToken *Qual = Tok->Next;
   FormatToken *LastQual = Qual;
   while (Qual && isQualifierOrType(Qual, ConfiguredQualifierTokens)) {
@@ -231,26 +253,24 @@
 return Tok;
   } else if (Tok->startsSequence(QualifierType, tok::identifier,
  TT_TemplateOpener)) {
-// Read from the TemplateOpener to
-// TemplateCloser as in const ArrayRef a; const ArrayRef 
-FormatToken *EndTemplate = Tok->Next->Next->MatchingParen;
-if (EndTemplate) {
-  // Move to the end of any template class members e.g.
-  // `Foo::iterator`.
-  if (EndTemplate->startsSequence(TT_TemplateCloser, tok::coloncolon,
-  tok::identifier))
-EndTemplate = EndTemplate->Next->Next;
-}
-if (EndTemplate && EndTemplate->Next &&
-!EndTemplate->Next->isOneOf(tok::equal, tok::l_paren)) {
-  insertQualifierAfter(SourceMgr, Fixes, EndTemplate, Qualifier);
-  // Remove the qualifier.
-  removeToken(SourceMgr, Fixes, Tok);
-  return Tok;
-}
-  } else if (Tok->startsSequence(QualifierType, tok::identifier)) {
+// `const ArrayRef a;`
+// `const ArrayRef `
+const FormatToken *NewTok = AnalyzeTemplate(Tok, Tok->Next->Next);
+if (NewTok)
+  return NewTok;
+  } else if (Tok->startsSequence(QualifierType, tok::coloncolon,
+ tok::identifier, TT_TemplateOpener)) {
+// `const ::ArrayRef a;`
+// `const ::ArrayRef `
+const FormatToken *NewTok = AnalyzeTemplate(Tok, Tok->Next->Next->Next);
+if (NewTok)
+

[PATCH] D125961: [clang-format] Don't break lines after pragma region

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

Thanks! LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125961/new/

https://reviews.llvm.org/D125961

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125961: [clang-format] Don't break lines after pragma region

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

In D125961#3527452 , @thieta wrote:

> In D125961#3527447 , @curdeius 
> wrote:
>
>> Thanks for the patch!
>> Could you please add a unit test in unittest/Format/FormatTest.cpp instead 
>> of in lit-based test/?
>> Is there a bug report that your patch fixes?
>
> Sure - I didn't know clang-format used unittests instead.

Yes, non-unit tests serve meeting for the config tests IIRC.

> Maybe there should be some documentation around this for new people used to 
> adding lit tests?

That would certainly help. Feel free to do it if you
Otherwise I'll do it but only later (in ten days or so).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125961/new/

https://reviews.llvm.org/D125961

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125961: [clang-format] Don't break lines after pragma region

[Marek Kurdej via Phabricator via cfe-commits]

curdeius requested changes to this revision.
curdeius added a comment.
This revision now requires changes to proceed.

Thanks for the patch!
Could you please add a unit test in unittest/Format/FormatTest.cpp instead of 
in lit-based test/?
Is there a bug report that your patch fixes?




Comment at: clang/lib/Format/TokenAnnotator.cpp:1228
   bool IsMark = CurrentToken->is(Keywords.kw_mark);
   next(); // Consume "mark".
   next(); // Consume first token (so we fix leading whitespace).

Please fix (remove) this comment that consumes not only mark.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125961/new/

https://reviews.llvm.org/D125961

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126052: [clang-format] Handle "complex" conditionals in RemoveBracesLLVM

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126052/new/

https://reviews.llvm.org/D126052

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123676: [clang-format] Fix WhitespaceSensitiveMacros not being honoured when macro closing parenthesis is followed by a newline.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius reopened this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

Reverted for now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123676/new/

https://reviews.llvm.org/D123676

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123676: [clang-format] Fix WhitespaceSensitiveMacros not being honoured when macro closing parenthesis is followed by a newline.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

In D123676#3515949 , @krasimir wrote:

> It looks like this regressed the following example by adding an unwanted 
> level of indentation to the `#elif B` branch:

Sure, I'll have a look.
It seems that even this:

  MACRO_BEGIN
  #if A
  int f();
  #else
  int f();
  #endif

gets misindented:

  MACRO_BEGIN
  #if A
  int f();
  #else
  int
  f();
  #endif


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123676/new/

https://reviews.llvm.org/D123676

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125162: [clang-format] fix alignment w/o binpacked args

[Marek Kurdej via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe57f57841fbb: [clang-format] fix alignment w/o binpacked 
args (authored by cha5on, committed by curdeius).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125162/new/

https://reviews.llvm.org/D125162

Files:
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -17318,6 +17318,23 @@
   //  "ccc ? a : b,\n"
   //  "dd);",
   //  Alignment);
+
+  // Confirm proper handling of AlignConsecutiveAssignments with
+  // BinPackArguments.
+  // See https://llvm.org/PR55360
+  Alignment = getLLVMStyleWithColumns(50);
+  Alignment.AlignConsecutiveAssignments.Enabled = true;
+  Alignment.BinPackArguments = false;
+  verifyFormat("int a_long_name = 1;\n"
+   "auto b  = B({a_long_name, a_long_name},\n"
+   "{a_longer_name_for_wrap,\n"
+   " a_longer_name_for_wrap});",
+   Alignment);
+  verifyFormat("int a_long_name = 1;\n"
+   "auto b  = B{{a_long_name, a_long_name},\n"
+   "{a_longer_name_for_wrap,\n"
+   " a_longer_name_for_wrap}};",
+   Alignment);
 }
 
 TEST_F(FormatTest, AlignConsecutiveBitFields) {
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -371,6 +371,9 @@
 return false;
   if (Changes[ScopeStart].NewlinesBefore > 0)
 return false;
+  if (Changes[i].Tok->is(tok::l_brace) &&
+  Changes[i].Tok->is(BK_BracedInit))
+return true;
   return Style.BinPackArguments;
 }
 
@@ -387,6 +390,14 @@
 Changes[i].Tok->Previous->is(TT_ConditionalExpr))
   return true;
 
+// Continued direct-list-initialization using braced list.
+if (ScopeStart > Start + 1 &&
+Changes[ScopeStart - 2].Tok->is(tok::identifier) &&
+Changes[ScopeStart - 1].Tok->is(tok::l_brace) &&
+Changes[i].Tok->is(tok::l_brace) &&
+Changes[i].Tok->is(BK_BracedInit))
+  return true;
+
 // Continued braced list.
 if (ScopeStart > Start + 1 &&
 Changes[ScopeStart - 2].Tok->isNot(tok::identifier) &&


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -17318,6 +17318,23 @@
   //  "ccc ? a : b,\n"
   //  "dd);",
   //  Alignment);
+
+  // Confirm proper handling of AlignConsecutiveAssignments with
+  // BinPackArguments.
+  // See https://llvm.org/PR55360
+  Alignment = getLLVMStyleWithColumns(50);
+  Alignment.AlignConsecutiveAssignments.Enabled = true;
+  Alignment.BinPackArguments = false;
+  verifyFormat("int a_long_name = 1;\n"
+   "auto b  = B({a_long_name, a_long_name},\n"
+   "{a_longer_name_for_wrap,\n"
+   " a_longer_name_for_wrap});",
+   Alignment);
+  verifyFormat("int a_long_name = 1;\n"
+   "auto b  = B{{a_long_name, a_long_name},\n"
+   "{a_longer_name_for_wrap,\n"
+   " a_longer_name_for_wrap}};",
+   Alignment);
 }
 
 TEST_F(FormatTest, AlignConsecutiveBitFields) {
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -371,6 +371,9 @@
 return false;
   if (Changes[ScopeStart].NewlinesBefore > 0)
 return false;
+  if (Changes[i].Tok->is(tok::l_brace) &&
+  Changes[i].Tok->is(BK_BracedInit))
+return true;
   return Style.BinPackArguments;
 }
 
@@ -387,6 +390,14 @@
 Changes[i].Tok->Previous->is(TT_ConditionalExpr))
   return true;
 
+// Continued direct-list-initialization using braced list.
+if (ScopeStart > Start + 1 &&
+Changes[ScopeStart - 2].Tok->is(tok::identifier) &&
+Changes[ScopeStart - 1].Tok->is(tok::l_brace) &&
+Changes[i].Tok->is(tok::l_brace) &&
+Changes[i].Tok->is(BK_BracedInit))
+  return true;
+
 // Continued braced list.
  

[PATCH] D125528: [clang-format] Fix PointerAlignment: Right not working with tab indentation.

[Marek Kurdej via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
curdeius marked an inline comment as done.
Closed by commit rGe20bc892b6fa: [clang-format] Fix PointerAlignment: Right not 
working with tab indentation. (authored by curdeius).

Changed prior to commit:
  https://reviews.llvm.org/D125528?vs=429178=429629#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125528/new/

https://reviews.llvm.org/D125528

Files:
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14161,6 +14161,21 @@
"int ; // x\n",
Tab);
 
+  FormatStyle TabAlignment = Tab;
+  TabAlignment.AlignConsecutiveDeclarations.Enabled = true;
+  TabAlignment.PointerAlignment = FormatStyle::PAS_Left;
+  verifyFormat("unsigned long long big;\n"
+   "char*\t\t   ptr;",
+   TabAlignment);
+  TabAlignment.PointerAlignment = FormatStyle::PAS_Middle;
+  verifyFormat("unsigned long long big;\n"
+   "char *\t\t   ptr;",
+   TabAlignment);
+  TabAlignment.PointerAlignment = FormatStyle::PAS_Right;
+  verifyFormat("unsigned long long big;\n"
+   "char\t\t  *ptr;",
+   TabAlignment);
+
   Tab.TabWidth = 4;
   Tab.IndentWidth = 8;
   verifyFormat("class TabWidth4Indent8 {\n"
@@ -14203,6 +14218,26 @@
" \t  */",
Tab));
 
+  TabAlignment.UseTab = FormatStyle::UT_ForIndentation;
+  TabAlignment.PointerAlignment = FormatStyle::PAS_Left;
+  verifyFormat("void f() {\n"
+   "\tunsigned long long big;\n"
+   "\tchar*  ptr;\n"
+   "}",
+   TabAlignment);
+  TabAlignment.PointerAlignment = FormatStyle::PAS_Middle;
+  verifyFormat("void f() {\n"
+   "\tunsigned long long big;\n"
+   "\tchar * ptr;\n"
+   "}",
+   TabAlignment);
+  TabAlignment.PointerAlignment = FormatStyle::PAS_Right;
+  verifyFormat("void f() {\n"
+   "\tunsigned long long big;\n"
+   "\tchar  *ptr;\n"
+   "}",
+   TabAlignment);
+
   Tab.UseTab = FormatStyle::UT_ForIndentation;
   verifyFormat("{\n"
"\t();\n"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -432,6 +432,7 @@
--Previous) {
 Changes[Previous + 1].Spaces -= Shift;
 Changes[Previous].Spaces += Shift;
+Changes[Previous].StartOfTokenColumn += Shift;
   }
 }
   }


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14161,6 +14161,21 @@
"int ; // x\n",
Tab);
 
+  FormatStyle TabAlignment = Tab;
+  TabAlignment.AlignConsecutiveDeclarations.Enabled = true;
+  TabAlignment.PointerAlignment = FormatStyle::PAS_Left;
+  verifyFormat("unsigned long long big;\n"
+   "char*\t\t   ptr;",
+   TabAlignment);
+  TabAlignment.PointerAlignment = FormatStyle::PAS_Middle;
+  verifyFormat("unsigned long long big;\n"
+   "char *\t\t   ptr;",
+   TabAlignment);
+  TabAlignment.PointerAlignment = FormatStyle::PAS_Right;
+  verifyFormat("unsigned long long big;\n"
+   "char\t\t  *ptr;",
+   TabAlignment);
+
   Tab.TabWidth = 4;
   Tab.IndentWidth = 8;
   verifyFormat("class TabWidth4Indent8 {\n"
@@ -14203,6 +14218,26 @@
" \t  */",
Tab));
 
+  TabAlignment.UseTab = FormatStyle::UT_ForIndentation;
+  TabAlignment.PointerAlignment = FormatStyle::PAS_Left;
+  verifyFormat("void f() {\n"
+   "\tunsigned long long big;\n"
+   "\tchar*  ptr;\n"
+   "}",
+   TabAlignment);
+  TabAlignment.PointerAlignment = FormatStyle::PAS_Middle;
+  verifyFormat("void f() {\n"
+   "\tunsigned long long big;\n"
+   "\tchar * ptr;\n"
+   "}",
+   TabAlignment);
+  TabAlignment.PointerAlignment = FormatStyle::PAS_Right;
+  verifyFormat("void f() {\n"
+   "\tunsigned long long big;\n"
+   "\tchar  *ptr;\n"
+   "}",
+   TabAlignment);
+
   Tab.UseTab = FormatStyle::UT_ForIndentation;
   verifyFormat("{\n"
"\t();\n"
Index: clang/lib/Format/WhitespaceManager.cpp

[PATCH] D125626: [clang-format][NFC] Don't call mightFitOnOneLine() unnecessarily

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM. I like it! Thanks!




Comment at: clang/lib/Format/UnwrappedLineParser.cpp:523
+ /*MunchSemi=*/true, /*KeepBraces=*/true,
+ /*UnindentWhitesmithBraces=*/false, CanContainBracedList,
+ NextLBracesType);





Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2832
 
-  parseLoopBody(false, Style.BraceWrapping.BeforeWhile);
+  parseLoopBody(true, Style.BraceWrapping.BeforeWhile);
 




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125626/new/

https://reviews.llvm.org/D125626

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125593: [clang-format] Handle "if consteval { ... }" for RemoveBracesLLVM

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM. Good finding!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125593/new/

https://reviews.llvm.org/D125593

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125528: [clang-format] Fix PointerAlignment: Right not working with tab indentation.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:14222
+  TabAlignment.UseTab = FormatStyle::UT_ForIndentation;
+  TabAlignment.AlignConsecutiveDeclarations.Enabled = true;
+  TabAlignment.PointerAlignment = FormatStyle::PAS_Left;

owenpan wrote:
> Redundant.
Good catch. Will remove when landing (after the weekend probably).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125528/new/

https://reviews.llvm.org/D125528

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125528: [clang-format] Fix PointerAlignment: Right not working with tab indentation.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius created this revision.
curdeius added reviewers: HazardyKnusperkeks, owenpan, MyDeveloperDay.
Herald added a project: All.
curdeius requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes https://github.com/llvm/llvm-project/issues/55407.

Given configuration:

  UseTab: Always
  PointerAlignment: Right
  AlignConsecutiveDeclarations: true

Before, the pointer was misaligned in this code:

  void f() {
unsigned long long big;
char  *ptr; // misaligned
inti;
  }

That was due to the fact that when handling right-aligned pointers, the Spaces 
were changed but StartOfTokenColumn was not.

Also, a tab was used not only for indentation but for spacing too when using 
`UseTab: ForIndentation` config option:

  

void f() {
unsigned long long big;
char  *ptr; // \t after char
inti;
}

  


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125528

Files:
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14161,6 +14161,21 @@
"int ; // x\n",
Tab);
 
+  FormatStyle TabAlignment = Tab;
+  TabAlignment.AlignConsecutiveDeclarations.Enabled = true;
+  TabAlignment.PointerAlignment = FormatStyle::PAS_Left;
+  verifyFormat("unsigned long long big;\n"
+   "char*\t\t   ptr;",
+   TabAlignment);
+  TabAlignment.PointerAlignment = FormatStyle::PAS_Middle;
+  verifyFormat("unsigned long long big;\n"
+   "char *\t\t   ptr;",
+   TabAlignment);
+  TabAlignment.PointerAlignment = FormatStyle::PAS_Right;
+  verifyFormat("unsigned long long big;\n"
+   "char\t\t  *ptr;",
+   TabAlignment);
+
   Tab.TabWidth = 4;
   Tab.IndentWidth = 8;
   verifyFormat("class TabWidth4Indent8 {\n"
@@ -14203,6 +14218,27 @@
" \t  */",
Tab));
 
+  TabAlignment.UseTab = FormatStyle::UT_ForIndentation;
+  TabAlignment.AlignConsecutiveDeclarations.Enabled = true;
+  TabAlignment.PointerAlignment = FormatStyle::PAS_Left;
+  verifyFormat("void f() {\n"
+   "\tunsigned long long big;\n"
+   "\tchar*  ptr;\n"
+   "}",
+   TabAlignment);
+  TabAlignment.PointerAlignment = FormatStyle::PAS_Middle;
+  verifyFormat("void f() {\n"
+   "\tunsigned long long big;\n"
+   "\tchar * ptr;\n"
+   "}",
+   TabAlignment);
+  TabAlignment.PointerAlignment = FormatStyle::PAS_Right;
+  verifyFormat("void f() {\n"
+   "\tunsigned long long big;\n"
+   "\tchar  *ptr;\n"
+   "}",
+   TabAlignment);
+
   Tab.UseTab = FormatStyle::UT_ForIndentation;
   verifyFormat("{\n"
"\t();\n"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -432,6 +432,7 @@
--Previous) {
 Changes[Previous + 1].Spaces -= Shift;
 Changes[Previous].Spaces += Shift;
+Changes[Previous].StartOfTokenColumn += Shift;
   }
 }
   }


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14161,6 +14161,21 @@
"int ; // x\n",
Tab);
 
+  FormatStyle TabAlignment = Tab;
+  TabAlignment.AlignConsecutiveDeclarations.Enabled = true;
+  TabAlignment.PointerAlignment = FormatStyle::PAS_Left;
+  verifyFormat("unsigned long long big;\n"
+   "char*\t\t   ptr;",
+   TabAlignment);
+  TabAlignment.PointerAlignment = FormatStyle::PAS_Middle;
+  verifyFormat("unsigned long long big;\n"
+   "char *\t\t   ptr;",
+   TabAlignment);
+  TabAlignment.PointerAlignment = FormatStyle::PAS_Right;
+  verifyFormat("unsigned long long big;\n"
+   "char\t\t  *ptr;",
+   TabAlignment);
+
   Tab.TabWidth = 4;
   Tab.IndentWidth = 8;
   verifyFormat("class TabWidth4Indent8 {\n"
@@ -14203,6 +14218,27 @@
" \t  */",
Tab));
 
+  TabAlignment.UseTab = FormatStyle::UT_ForIndentation;
+  TabAlignment.AlignConsecutiveDeclarations.Enabled = true;
+  TabAlignment.PointerAlignment = FormatStyle::PAS_Left;
+  verifyFormat("void f() {\n"
+   "\tunsigned long long big;\n"
+   "\tchar*  ptr;\n"
+   "}",
+   TabAlignment);
+  

[PATCH] D125162: [clang-format] fix alignment w/o binpacked args

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM. Tell us if you need help landing this.
Thanks a lot for your contribution!




Comment at: clang/lib/Format/WhitespaceManager.cpp:374-375
 return false;
+  if (Changes[i].Tok->is(tok::l_brace) &&
+  Changes[i].Tok->is(BK_BracedInit))
+return true;

cha5on wrote:
> curdeius wrote:
> > It seems that we set `BK_BracedInit` only on `l_brace`, so no need for a 
> > redundant check.
> `BK_BracedInit` also gets set on `r_brace`, assuming I'm reading 
> `clang/lib/Format/UnwrappedLineParser.cpp` correctly.  I think we need both 
> to be true to be sure that this is the intended token.
Ok.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125162/new/

https://reviews.llvm.org/D125162

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125137: [clang-format] Don't remove braces if a 1-statement body would wrap

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:787
+
+  const int Length = LastToken->TotalLength;
+

owenpan wrote:
> curdeius wrote:
> > Why not like this?
> > Why not like this?
> 
> See the assertion on line 781 above. We are computing the `TotalLength` of 
> `LastToken` via `Line`. Either way works, but I prefer the simpler 
> expression. I can change it though if you insist.
Yeah, what I meant is to just get rid of `LastToken` variable and write the 
suggested code.
But well, both ways work. No strong feelings.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125137/new/

https://reviews.llvm.org/D125137

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125137: [clang-format] Don't remove braces if a 1-statement body would wrap

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM with nits.




Comment at: clang/lib/Format/UnwrappedLineParser.cpp:766
+  assert(!Tokens.empty());
+  const auto *LastToken = Tokens.back().Tok;
+  assert(LastToken);

It might be a matter of taste but adding this variable makes the code harder to 
read to me.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:777
+SavedToken.Tok->copyFrom(*Token.Tok);
+SavedToken.Children = std::move(Token.Children);
+  }

So the token's children are modified (moved)? Is it done so that children be 
not considered by the annotator?



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:787
+
+  const int Length = LastToken->TotalLength;
+

Why not like this?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125137/new/

https://reviews.llvm.org/D125137

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123896: [clang-format] fix nested angle brackets parse inside concept definition

[Marek Kurdej via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdab5e10ea5db: [clang-format] fix nested angle brackets parse 
inside concept definition (authored by predelnik, committed by curdeius).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123896/new/

https://reviews.llvm.org/D123896

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -390,6 +390,18 @@
   EXPECT_TOKEN(Tokens[21], tok::r_brace, TT_Unknown);
   EXPECT_EQ(Tokens[21]->MatchingParen, Tokens[15]);
   EXPECT_TRUE(Tokens[21]->ClosesRequiresClause);
+
+  Tokens =
+  annotate("template  concept C ="
+   "std::same_as, std::iter_value_t>;");
+  ASSERT_EQ(Tokens.size(), 31u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::kw_concept, TT_Unknown);
+  EXPECT_TOKEN(Tokens[14], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[18], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[20], tok::greater, TT_TemplateCloser);
+  EXPECT_TOKEN(Tokens[25], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[27], tok::greater, TT_TemplateCloser);
+  EXPECT_TOKEN(Tokens[28], tok::greater, TT_TemplateCloser);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsRequiresExpressions) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -24114,6 +24114,12 @@
   verifyFormat("template \n"
"concept Same = __is_same_as;");
 
+  verifyFormat(
+  "template \n"
+  "concept _Can_reread_dest =\n"
+  "std::forward_iterator<_OutIt> &&\n"
+  "std::same_as, 
std::iter_value_t<_OutIt>>;");
+
   auto Style = getLLVMStyle();
   Style.BreakBeforeConceptDeclarations = FormatStyle::BBCDS_Allowed;
 
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2180,7 +2180,8 @@
   parseBracedList();
   break;
 case tok::less:
-  if (Style.Language == FormatStyle::LK_Proto) {
+  if (Style.Language == FormatStyle::LK_Proto ||
+  ClosingBraceKind == tok::greater) {
 nextToken();
 parseBracedList(/*ContinueOnSemicolons=*/false, /*IsEnum=*/false,
 /*ClosingBraceKind=*/tok::greater);
@@ -3220,6 +3221,7 @@
   if (!FormatTok->is(tok::less))
 return;
 
+  nextToken();
   parseBracedList(/*ContinueOnSemicolons=*/false, /*IsEnum=*/false,
   /*ClosingBraceKind=*/tok::greater);
   break;
@@ -3260,9 +3262,11 @@
 
   // Read identifier with optional template declaration.
   nextToken();
-  if (FormatTok->is(tok::less))
+  if (FormatTok->is(tok::less)) {
+nextToken();
 parseBracedList(/*ContinueOnSemicolons=*/false, /*IsEnum=*/false,
 /*ClosingBraceKind=*/tok::greater);
+  }
   break;
 }
   } while (!eof());


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -390,6 +390,18 @@
   EXPECT_TOKEN(Tokens[21], tok::r_brace, TT_Unknown);
   EXPECT_EQ(Tokens[21]->MatchingParen, Tokens[15]);
   EXPECT_TRUE(Tokens[21]->ClosesRequiresClause);
+
+  Tokens =
+  annotate("template  concept C ="
+   "std::same_as, std::iter_value_t>;");
+  ASSERT_EQ(Tokens.size(), 31u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::kw_concept, TT_Unknown);
+  EXPECT_TOKEN(Tokens[14], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[18], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[20], tok::greater, TT_TemplateCloser);
+  EXPECT_TOKEN(Tokens[25], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[27], tok::greater, TT_TemplateCloser);
+  EXPECT_TOKEN(Tokens[28], tok::greater, TT_TemplateCloser);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsRequiresExpressions) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -24114,6 +24114,12 @@
   verifyFormat("template \n"
"concept Same = __is_same_as;");
 
+  verifyFormat(
+  "template \n"
+  "concept _Can_reread_dest =\n"
+  "std::forward_iterator<_OutIt> &&\n"
+  "std::same_as, std::iter_value_t<_OutIt>>;");
+
   auto Style = getLLVMStyle();
   

[PATCH] D123896: [clang-format] fix nested angle brackets parse inside concept definition

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

Do you need help landing this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123896/new/

https://reviews.llvm.org/D123896

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125162: [clang-format] fix alignment w/o binpacked args

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

Thanks for creating the bug report. A few more comments.




Comment at: clang/lib/Format/WhitespaceManager.cpp:374-375
 return false;
+  if (Changes[i].Tok->is(tok::l_brace) &&
+  Changes[i].Tok->is(BK_BracedInit))
+return true;

It seems that we set `BK_BracedInit` only on `l_brace`, so no need for a 
redundant check.



Comment at: clang/lib/Format/WhitespaceManager.cpp:394-398
+if (ScopeStart > Start + 1 &&
+Changes[ScopeStart - 2].Tok->is(tok::identifier) &&
+Changes[ScopeStart - 1].Tok->is(tok::l_brace) &&
+Changes[i].Tok->is(tok::l_brace) &&
+Changes[i].Tok->is(BK_BracedInit))

Ditto.



Comment at: clang/unittests/Format/FormatTest.cpp:17290-17299
+  verifyFormat("int a_long_name = 1;\n"
+   "auto b  = B({a_long_name, a_long_name},\n"
+   "{a_longer_name_for_wrap,\n"
+   " a_longer_name_for_wrap});",
+   "int a_long_name = 1;\n"
+   "auto b  = B({a_long_name,\n"
+   " a_long_name},\n"

Why not just this?



Comment at: clang/unittests/Format/FormatTest.cpp:17300-17309
+  verifyFormat("int a_long_name = 1;\n"
+   "auto b  = B{{a_long_name, a_long_name},\n"
+   "{a_longer_name_for_wrap,\n"
+   " a_longer_name_for_wrap}};",
+   "int a_long_name = 1;\n"
+   "auto b  = B{{a_long_name,\n"
+   " a_long_name},\n"

And here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125162/new/

https://reviews.llvm.org/D125162

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125085: [clang-format] Correctly handle SpaceBeforeParens for builtins.

[Marek Kurdej via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG85ec8a9ac141: [clang-format] Correctly handle 
SpaceBeforeParens for builtins. (authored by curdeius).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125085/new/

https://reviews.llvm.org/D125085

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -15082,6 +15082,9 @@
   // verifyFormat("X A::operator++ (T);", Space);
   verifyFormat("auto lambda = [] () { return 0; };", Space);
   verifyFormat("int x = int (y);", Space);
+  verifyFormat("#define F(...) __VA_OPT__ (__VA_ARGS__)", Space);
+  verifyFormat("__builtin_LINE ()", Space);
+  verifyFormat("__builtin_UNKNOWN ()", Space);
 
   FormatStyle SomeSpace = getLLVMStyle();
   SomeSpace.SpaceBeforeParens = FormatStyle::SBPO_NonEmptyParentheses;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3436,9 +3436,9 @@
 return (Style.SpaceBeforeParens != FormatStyle::SBPO_Never) ||
spaceRequiredBeforeParens(Right);
 }
+// Handle builtins like identifiers.
 if (Line.Type != LT_PreprocessorDirective &&
-(Left.is(tok::identifier) || Left.isFunctionLikeKeyword() ||
- Left.is(tok::r_paren) || Left.isSimpleTypeSpecifier()))
+(Left.Tok.getIdentifierInfo() || Left.is(tok::r_paren)))
   return spaceRequiredBeforeParens(Right);
 return false;
   }


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -15082,6 +15082,9 @@
   // verifyFormat("X A::operator++ (T);", Space);
   verifyFormat("auto lambda = [] () { return 0; };", Space);
   verifyFormat("int x = int (y);", Space);
+  verifyFormat("#define F(...) __VA_OPT__ (__VA_ARGS__)", Space);
+  verifyFormat("__builtin_LINE ()", Space);
+  verifyFormat("__builtin_UNKNOWN ()", Space);
 
   FormatStyle SomeSpace = getLLVMStyle();
   SomeSpace.SpaceBeforeParens = FormatStyle::SBPO_NonEmptyParentheses;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3436,9 +3436,9 @@
 return (Style.SpaceBeforeParens != FormatStyle::SBPO_Never) ||
spaceRequiredBeforeParens(Right);
 }
+// Handle builtins like identifiers.
 if (Line.Type != LT_PreprocessorDirective &&
-(Left.is(tok::identifier) || Left.isFunctionLikeKeyword() ||
- Left.is(tok::r_paren) || Left.isSimpleTypeSpecifier()))
+(Left.Tok.getIdentifierInfo() || Left.is(tok::r_paren)))
   return spaceRequiredBeforeParens(Right);
 return false;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125085: [clang-format] Correctly handle SpaceBeforeParens for builtins.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius updated this revision to Diff 428009.
curdeius marked an inline comment as done.
curdeius added a comment.

Address comment: simplify.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125085/new/

https://reviews.llvm.org/D125085

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -15082,6 +15082,9 @@
   // verifyFormat("X A::operator++ (T);", Space);
   verifyFormat("auto lambda = [] () { return 0; };", Space);
   verifyFormat("int x = int (y);", Space);
+  verifyFormat("#define F(...) __VA_OPT__ (__VA_ARGS__)", Space);
+  verifyFormat("__builtin_LINE ()", Space);
+  verifyFormat("__builtin_UNKNOWN ()", Space);
 
   FormatStyle SomeSpace = getLLVMStyle();
   SomeSpace.SpaceBeforeParens = FormatStyle::SBPO_NonEmptyParentheses;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3436,9 +3436,9 @@
 return (Style.SpaceBeforeParens != FormatStyle::SBPO_Never) ||
spaceRequiredBeforeParens(Right);
 }
+// Handle builtins like identifiers.
 if (Line.Type != LT_PreprocessorDirective &&
-(Left.is(tok::identifier) || Left.isFunctionLikeKeyword() ||
- Left.is(tok::r_paren) || Left.isSimpleTypeSpecifier()))
+(Left.Tok.getIdentifierInfo() || Left.is(tok::r_paren)))
   return spaceRequiredBeforeParens(Right);
 return false;
   }


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -15082,6 +15082,9 @@
   // verifyFormat("X A::operator++ (T);", Space);
   verifyFormat("auto lambda = [] () { return 0; };", Space);
   verifyFormat("int x = int (y);", Space);
+  verifyFormat("#define F(...) __VA_OPT__ (__VA_ARGS__)", Space);
+  verifyFormat("__builtin_LINE ()", Space);
+  verifyFormat("__builtin_UNKNOWN ()", Space);
 
   FormatStyle SomeSpace = getLLVMStyle();
   SomeSpace.SpaceBeforeParens = FormatStyle::SBPO_NonEmptyParentheses;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3436,9 +3436,9 @@
 return (Style.SpaceBeforeParens != FormatStyle::SBPO_Never) ||
spaceRequiredBeforeParens(Right);
 }
+// Handle builtins like identifiers.
 if (Line.Type != LT_PreprocessorDirective &&
-(Left.is(tok::identifier) || Left.isFunctionLikeKeyword() ||
- Left.is(tok::r_paren) || Left.isSimpleTypeSpecifier()))
+(Left.Tok.getIdentifierInfo() || Left.is(tok::r_paren)))
   return spaceRequiredBeforeParens(Right);
 return false;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123676: [clang-format] Fix WhitespaceSensitiveMacros not being honoured when macro closing parenthesis is followed by a newline.

[Marek Kurdej via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG50cd52d93572: [clang-format] Fix WhitespaceSensitiveMacros 
not being honoured when macro… (authored by curdeius).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123676/new/

https://reviews.llvm.org/D123676

Files:
  clang/lib/Format/FormatTokenLexer.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -23606,6 +23606,11 @@
 
   // Don't use the helpers here, since 'mess up' will change the whitespace
   // and these are all whitespace sensitive by definition
+
+  // Newlines are important here.
+  EXPECT_EQ("FOO(1+2  );\n", format("FOO(1+2  );\n", Style));
+  EXPECT_EQ("FOO(1+2  )\n", format("FOO(1+2  )\n", Style));
+
   EXPECT_EQ("FOO(String-ized+But(: :Still)=Intentional);",
 format("FOO(String-ized+But(: :Still)=Intentional);", 
Style));
   EXPECT_EQ(
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1787,7 +1787,8 @@
 : CommentsBeforeNextToken.front()->NewlinesBefore > 0;
 
 if (FollowedByNewline && (Text.size() >= 5 || FunctionLike) &&
-tokenCanStartNewLine(*FormatTok) && Text == Text.upper()) {
+tokenCanStartNewLine(*FormatTok) && Text == Text.upper() &&
+!PreviousToken->isTypeFinalized()) {
   PreviousToken->setFinalizedType(TT_FunctionLikeOrFreestandingMacro);
   addUnwrappedLine();
   return;
Index: clang/lib/Format/FormatTokenLexer.cpp
===
--- clang/lib/Format/FormatTokenLexer.cpp
+++ clang/lib/Format/FormatTokenLexer.cpp
@@ -1027,7 +1027,10 @@
   Tokens.back()->Tok.getIdentifierInfo()->getPPKeywordID() ==
   tok::pp_define) &&
 it != Macros.end()) {
-  FormatTok->setType(it->second);
+  if (it->second == TT_UntouchableMacroFunc)
+FormatTok->setFinalizedType(TT_UntouchableMacroFunc);
+  else
+FormatTok->setType(it->second);
   if (it->second == TT_IfMacro) {
 // The lexer token currently has type tok::kw_unknown. However, for 
this
 // substitution to be treated correctly in the TokenAnnotator, faking


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -23606,6 +23606,11 @@
 
   // Don't use the helpers here, since 'mess up' will change the whitespace
   // and these are all whitespace sensitive by definition
+
+  // Newlines are important here.
+  EXPECT_EQ("FOO(1+2  );\n", format("FOO(1+2  );\n", Style));
+  EXPECT_EQ("FOO(1+2  )\n", format("FOO(1+2  )\n", Style));
+
   EXPECT_EQ("FOO(String-ized+But(: :Still)=Intentional);",
 format("FOO(String-ized+But(: :Still)=Intentional);", Style));
   EXPECT_EQ(
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1787,7 +1787,8 @@
 : CommentsBeforeNextToken.front()->NewlinesBefore > 0;
 
 if (FollowedByNewline && (Text.size() >= 5 || FunctionLike) &&
-tokenCanStartNewLine(*FormatTok) && Text == Text.upper()) {
+tokenCanStartNewLine(*FormatTok) && Text == Text.upper() &&
+!PreviousToken->isTypeFinalized()) {
   PreviousToken->setFinalizedType(TT_FunctionLikeOrFreestandingMacro);
   addUnwrappedLine();
   return;
Index: clang/lib/Format/FormatTokenLexer.cpp
===
--- clang/lib/Format/FormatTokenLexer.cpp
+++ clang/lib/Format/FormatTokenLexer.cpp
@@ -1027,7 +1027,10 @@
   Tokens.back()->Tok.getIdentifierInfo()->getPPKeywordID() ==
   tok::pp_define) &&
 it != Macros.end()) {
-  FormatTok->setType(it->second);
+  if (it->second == TT_UntouchableMacroFunc)
+FormatTok->setFinalizedType(TT_UntouchableMacroFunc);
+  else
+FormatTok->setType(it->second);
   if (it->second == TT_IfMacro) {
 // The lexer token currently has type tok::kw_unknown. However, for this
 // substitution to be treated correctly in the TokenAnnotator, faking
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125162: [clang-format] fix alignment w/o binpacked args

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

FYI, I like to have a bug report because people that encounter the same problem 
(in an older release) can find it easily, and see if/when it was fixed. It also 
avoid working on the same thing by different people.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125162/new/

https://reviews.llvm.org/D125162

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125162: [clang-format] fix alignment w/o binpacked args

[Marek Kurdej via Phabricator via cfe-commits]

curdeius requested changes to this revision.
curdeius added a comment.
This revision now requires changes to proceed.

Thanks for working on this!
Is there a bug report somewhere? If not, may you create one please?




Comment at: clang/lib/Format/WhitespaceManager.cpp:398
+Changes[i].Tok->is(tok::l_brace) &&
+Changes[i].Tok->is(BK_BracedInit)) {
+  return true;

Elide braces please.



Comment at: clang/unittests/Format/FormatTest.cpp:17321
+
+verifyFormat(Expected, ToFormat, Alignment);
+  }

I don't think you need to create variables, just online the strings.

Also, could you minimize (and possibly split) as much as possible these test 
cases?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125162/new/

https://reviews.llvm.org/D125162

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125085: [clang-format] Correctly handle SpaceBeforeParens for builtins.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius created this revision.
curdeius added reviewers: MyDeveloperDay, HazardyKnusperkeks, owenpan.
Herald added a project: All.
curdeius requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

That's a partial fix for https://github.com/llvm/llvm-project/issues/55292.

Before, known builtins behaved differently from other identifiers:

  void f () { return F (__builtin_LINE() + __builtin_FOO ()); }

After:

  void f () { return F (__builtin_LINE () + __builtin_FOO ()); }


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125085

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -15082,6 +15082,9 @@
   // verifyFormat("X A::operator++ (T);", Space);
   verifyFormat("auto lambda = [] () { return 0; };", Space);
   verifyFormat("int x = int (y);", Space);
+  verifyFormat("#define F(...) __VA_OPT__ (__VA_ARGS__)", Space);
+  verifyFormat("__builtin_LINE ()", Space);
+  verifyFormat("__builtin_UNKNOWN ()", Space);
 
   FormatStyle SomeSpace = getLLVMStyle();
   SomeSpace.SpaceBeforeParens = FormatStyle::SBPO_NonEmptyParentheses;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3434,9 +3434,11 @@
 return (Style.SpaceBeforeParens != FormatStyle::SBPO_Never) ||
spaceRequiredBeforeParens(Right);
 }
+// Handle builtins like identifiers.
 if (Line.Type != LT_PreprocessorDirective &&
-(Left.is(tok::identifier) || Left.isFunctionLikeKeyword() ||
- Left.is(tok::r_paren) || Left.isSimpleTypeSpecifier()))
+(Left.is(tok::identifier) || Left.Tok.getIdentifierInfo() ||
+ Left.isFunctionLikeKeyword() || Left.is(tok::r_paren) ||
+ Left.isSimpleTypeSpecifier()))
   return spaceRequiredBeforeParens(Right);
 return false;
   }


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -15082,6 +15082,9 @@
   // verifyFormat("X A::operator++ (T);", Space);
   verifyFormat("auto lambda = [] () { return 0; };", Space);
   verifyFormat("int x = int (y);", Space);
+  verifyFormat("#define F(...) __VA_OPT__ (__VA_ARGS__)", Space);
+  verifyFormat("__builtin_LINE ()", Space);
+  verifyFormat("__builtin_UNKNOWN ()", Space);
 
   FormatStyle SomeSpace = getLLVMStyle();
   SomeSpace.SpaceBeforeParens = FormatStyle::SBPO_NonEmptyParentheses;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3434,9 +3434,11 @@
 return (Style.SpaceBeforeParens != FormatStyle::SBPO_Never) ||
spaceRequiredBeforeParens(Right);
 }
+// Handle builtins like identifiers.
 if (Line.Type != LT_PreprocessorDirective &&
-(Left.is(tok::identifier) || Left.isFunctionLikeKeyword() ||
- Left.is(tok::r_paren) || Left.isSimpleTypeSpecifier()))
+(Left.is(tok::identifier) || Left.Tok.getIdentifierInfo() ||
+ Left.isFunctionLikeKeyword() || Left.is(tok::r_paren) ||
+ Left.isSimpleTypeSpecifier()))
   return spaceRequiredBeforeParens(Right);
 return false;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123676: [clang-format] Fix WhitespaceSensitiveMacros not being honoured when macro closing parenthesis is followed by a newline.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius marked 5 inline comments as done.
curdeius added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:23545
+  // Newlines are important here.
+  EXPECT_EQ("FOO(1+2  );\n", format("FOO(1+2  );\n", Style));
+  EXPECT_EQ("FOO(1+2  )\n", format("FOO(1+2  )\n", Style));

curdeius wrote:
> owenpan wrote:
> > Do we really need this test case?
> Not really. I just wrote it to cover both cases but it's covered by existing 
> cases indeed. Will remove.
On a second thought, we don't have any other test with a semicolon and a 
newline, so I'd rather keep this test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123676/new/

https://reviews.llvm.org/D123676

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123676: [clang-format] Fix WhitespaceSensitiveMacros not being honoured when macro closing parenthesis is followed by a newline.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius updated this revision to Diff 427566.
curdeius added a comment.

Simplify. Address comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123676/new/

https://reviews.llvm.org/D123676

Files:
  clang/lib/Format/FormatTokenLexer.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -23606,6 +23606,11 @@
 
   // Don't use the helpers here, since 'mess up' will change the whitespace
   // and these are all whitespace sensitive by definition
+
+  // Newlines are important here.
+  EXPECT_EQ("FOO(1+2  );\n", format("FOO(1+2  );\n", Style));
+  EXPECT_EQ("FOO(1+2  )\n", format("FOO(1+2  )\n", Style));
+
   EXPECT_EQ("FOO(String-ized+But(: :Still)=Intentional);",
 format("FOO(String-ized+But(: :Still)=Intentional);", 
Style));
   EXPECT_EQ(
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1787,7 +1787,8 @@
 : CommentsBeforeNextToken.front()->NewlinesBefore > 0;
 
 if (FollowedByNewline && (Text.size() >= 5 || FunctionLike) &&
-tokenCanStartNewLine(*FormatTok) && Text == Text.upper()) {
+tokenCanStartNewLine(*FormatTok) && Text == Text.upper() &&
+!PreviousToken->isTypeFinalized()) {
   PreviousToken->setFinalizedType(TT_FunctionLikeOrFreestandingMacro);
   addUnwrappedLine();
   return;
Index: clang/lib/Format/FormatTokenLexer.cpp
===
--- clang/lib/Format/FormatTokenLexer.cpp
+++ clang/lib/Format/FormatTokenLexer.cpp
@@ -1027,7 +1027,10 @@
   Tokens.back()->Tok.getIdentifierInfo()->getPPKeywordID() ==
   tok::pp_define) &&
 it != Macros.end()) {
-  FormatTok->setType(it->second);
+  if (it->second == TT_UntouchableMacroFunc)
+FormatTok->setFinalizedType(TT_UntouchableMacroFunc);
+  else
+FormatTok->setType(it->second);
   if (it->second == TT_IfMacro) {
 // The lexer token currently has type tok::kw_unknown. However, for 
this
 // substitution to be treated correctly in the TokenAnnotator, faking


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -23606,6 +23606,11 @@
 
   // Don't use the helpers here, since 'mess up' will change the whitespace
   // and these are all whitespace sensitive by definition
+
+  // Newlines are important here.
+  EXPECT_EQ("FOO(1+2  );\n", format("FOO(1+2  );\n", Style));
+  EXPECT_EQ("FOO(1+2  )\n", format("FOO(1+2  )\n", Style));
+
   EXPECT_EQ("FOO(String-ized+But(: :Still)=Intentional);",
 format("FOO(String-ized+But(: :Still)=Intentional);", Style));
   EXPECT_EQ(
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1787,7 +1787,8 @@
 : CommentsBeforeNextToken.front()->NewlinesBefore > 0;
 
 if (FollowedByNewline && (Text.size() >= 5 || FunctionLike) &&
-tokenCanStartNewLine(*FormatTok) && Text == Text.upper()) {
+tokenCanStartNewLine(*FormatTok) && Text == Text.upper() &&
+!PreviousToken->isTypeFinalized()) {
   PreviousToken->setFinalizedType(TT_FunctionLikeOrFreestandingMacro);
   addUnwrappedLine();
   return;
Index: clang/lib/Format/FormatTokenLexer.cpp
===
--- clang/lib/Format/FormatTokenLexer.cpp
+++ clang/lib/Format/FormatTokenLexer.cpp
@@ -1027,7 +1027,10 @@
   Tokens.back()->Tok.getIdentifierInfo()->getPPKeywordID() ==
   tok::pp_define) &&
 it != Macros.end()) {
-  FormatTok->setType(it->second);
+  if (it->second == TT_UntouchableMacroFunc)
+FormatTok->setFinalizedType(TT_UntouchableMacroFunc);
+  else
+FormatTok->setType(it->second);
   if (it->second == TT_IfMacro) {
 // The lexer token currently has type tok::kw_unknown. However, for this
 // substitution to be treated correctly in the TokenAnnotator, faking
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125064: [clang-format][NFC] Make all TokenAnnotator member functions const

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125064/new/

https://reviews.llvm.org/D125064

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124956: [clang-format] Fix another bug in AlignConsecutiveAssignments

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

Just one last thought, shouldn't we have a test case close to what was reported 
in the issue? I.e. with PP directives.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124956/new/

https://reviews.llvm.org/D124956

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124956: [clang-format] Fix another bug in AlignConsecutiveAssignments

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.

LGTM!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124956/new/

https://reviews.llvm.org/D124956

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124868: [clang-format] Fix a bug in AlignConsecutiveAssignments

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

Looks ok, but please let someone else have a look too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124868/new/

https://reviews.llvm.org/D124868

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124818: [clang-format][NFC] Clean up tryToParseLambdaIntroducer()

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124818/new/

https://reviews.llvm.org/D124818

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116527: [clang-format] Fix indentation for array variables with alignment of consecutive assignments and declarations.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

Feel free to revert and add a regression test please. I'll take a look.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116527/new/

https://reviews.llvm.org/D116527

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124589: [clang-format] Fix a bug that misformats Access Specifier after *[]

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:3342
+   "public:\n"
+   "  std::unique_ptr b() { return nullptr; }\n"
+   "\n"

How about `int const *`, `const int*`? Is `const` & co. a simple type specifier?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124589/new/

https://reviews.llvm.org/D124589

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124589: [clang-format] Fix a bug that misformats Access Specifier after *[]

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

Looks more or less correct, but I am afraid that this change may misbehave in 
some rare cases.
Consider `*[](auto *x) { return x; }();`, so an immediately invoked lambda 
returning a pointer.
It seems to be correctly formatted currently, but I don't see any test on it.
Could you add a test for this please?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124589/new/

https://reviews.llvm.org/D124589

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124452: [clang-format] Adjust editor cursor position past #include blocks

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

Looks ok, but I'm not very familiar with this part of clang-format, so please 
leave some time for others to chime in.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124452/new/

https://reviews.llvm.org/D124452

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124280: [git-clang-format] Change run line from python to python3

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM. Especially that we already use python3 in clang-format-diff and other 
scripts, and it's used in many places in LLVM.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124280/new/

https://reviews.llvm.org/D124280

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits