[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat/CreateShuffleVector use poison as placeholder

[Allen zhong via Phabricator via cfe-commits]

Allen added a comment.

I have a babyism question, why poison is preferred to the undef in the pattern 
ConstantVector::getSplat ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93793

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

[clang] 491984c - Document __builtin_trap and __builtin_debugtrap

[via cfe-commits]

Author: serge-sans-paille
Date: 2022-01-05T03:10:17-05:00
New Revision: 491984c4e60c0f0de9dd00dc1a19e90a7b3535eb

URL: 
https://github.com/llvm/llvm-project/commit/491984c4e60c0f0de9dd00dc1a19e90a7b3535eb
DIFF: 
https://github.com/llvm/llvm-project/commit/491984c4e60c0f0de9dd00dc1a19e90a7b3535eb.diff

LOG: Document __builtin_trap and __builtin_debugtrap

Differential Revision: https://reviews.llvm.org/D116598

Added: 


Modified: 
clang/docs/LanguageExtensions.rst

Removed: 




diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index a47625e4a8dd2..a46edf6ca5061 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -2620,6 +2620,42 @@ argument.
   int *pb =__builtin_preserve_access_index(&v->c[3].b);
   __builtin_preserve_access_index(v->j);
 
+``__builtin_debugtrap``
+---
+
+``__builtin_debugtrap`` causes the program to stop its execution in such a way 
that a debugger can catch it.
+
+**Syntax**:
+
+.. code-block:: c++
+
+__builtin_debugtrap()
+
+**Description**
+
+``__builtin_debugtrap`` is lowered to the ` ``llvm.debugtrap`` 
`_ builtin. It 
should have the same effect as setting a breakpoint on the line where the 
builtin is called.
+
+Query for this feature with ``__has_builtin(__builtin_debugtrap)``.
+
+
+``__builtin_trap``
+--
+
+``__builtin_trap`` causes the program to stop its execution abnormally.
+
+**Syntax**:
+
+.. code-block:: c++
+
+__builtin_trap()
+
+**Description**
+
+``__builtin_trap`` is lowered to the ` ``llvm.trap`` 
`_ builtin.
+
+Query for this feature with ``__has_builtin(__builtin_trap)``.
+
+
 ``__builtin_sycl_unique_stable_name``
 -
 



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

[PATCH] D116598: Document __builtin_trap and __builtin_debugtrap

[serge 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 rG491984c4e60c: Document __builtin_trap and 
__builtin_debugtrap (authored by serge-sans-paille).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116598

Files:
  clang/docs/LanguageExtensions.rst


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -2620,6 +2620,42 @@
   int *pb =__builtin_preserve_access_index(&v->c[3].b);
   __builtin_preserve_access_index(v->j);
 
+``__builtin_debugtrap``
+---
+
+``__builtin_debugtrap`` causes the program to stop its execution in such a way 
that a debugger can catch it.
+
+**Syntax**:
+
+.. code-block:: c++
+
+__builtin_debugtrap()
+
+**Description**
+
+``__builtin_debugtrap`` is lowered to the ` ``llvm.debugtrap`` 
`_ builtin. It 
should have the same effect as setting a breakpoint on the line where the 
builtin is called.
+
+Query for this feature with ``__has_builtin(__builtin_debugtrap)``.
+
+
+``__builtin_trap``
+--
+
+``__builtin_trap`` causes the program to stop its execution abnormally.
+
+**Syntax**:
+
+.. code-block:: c++
+
+__builtin_trap()
+
+**Description**
+
+``__builtin_trap`` is lowered to the ` ``llvm.trap`` 
`_ builtin.
+
+Query for this feature with ``__has_builtin(__builtin_trap)``.
+
+
 ``__builtin_sycl_unique_stable_name``
 -
 


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -2620,6 +2620,42 @@
   int *pb =__builtin_preserve_access_index(&v->c[3].b);
   __builtin_preserve_access_index(v->j);
 
+``__builtin_debugtrap``
+---
+
+``__builtin_debugtrap`` causes the program to stop its execution in such a way that a debugger can catch it.
+
+**Syntax**:
+
+.. code-block:: c++
+
+__builtin_debugtrap()
+
+**Description**
+
+``__builtin_debugtrap`` is lowered to the ` ``llvm.debugtrap`` `_ builtin. It should have the same effect as setting a breakpoint on the line where the builtin is called.
+
+Query for this feature with ``__has_builtin(__builtin_debugtrap)``.
+
+
+``__builtin_trap``
+--
+
+``__builtin_trap`` causes the program to stop its execution abnormally.
+
+**Syntax**:
+
+.. code-block:: c++
+
+__builtin_trap()
+
+**Description**
+
+``__builtin_trap`` is lowered to the ` ``llvm.trap`` `_ builtin.
+
+Query for this feature with ``__has_builtin(__builtin_trap)``.
+
+
 ``__builtin_sycl_unique_stable_name``
 -
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93298: [RISCV] add the part of MC layer support of Zfinx extension

[Shao-Ce SUN via Phabricator via cfe-commits]

achieveartificialintelligence updated this revision to Diff 397469.
achieveartificialintelligence marked an inline comment as done.
achieveartificialintelligence added a comment.

Fix issue in RISCVISAInfo.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93298

Files:
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVInstrInfoD.td
  llvm/lib/Target/RISCV/RISCVInstrInfoF.td
  llvm/lib/Target/RISCV/RISCVInstrInfoZfh.td
  llvm/lib/Target/RISCV/RISCVRegisterInfo.td
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/rv32i-invalid.s
  llvm/test/MC/RISCV/rv32zdinx-invalid.s
  llvm/test/MC/RISCV/rv32zdinx-valid.s
  llvm/test/MC/RISCV/rv32zfinx-invalid.s
  llvm/test/MC/RISCV/rv32zfinx-valid.s
  llvm/test/MC/RISCV/rv32zhinx-invalid.s
  llvm/test/MC/RISCV/rv32zhinx-valid.s
  llvm/test/MC/RISCV/rv32zhinxmin-invalid.s
  llvm/test/MC/RISCV/rvzdinx-aliases-valid.s
  llvm/test/MC/RISCV/rvzfinx-aliases-valid.s
  llvm/test/MC/RISCV/rvzhinx-aliases-valid.s

Index: llvm/test/MC/RISCV/rvzhinx-aliases-valid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rvzhinx-aliases-valid.s
@@ -0,0 +1,49 @@
+# RUN: llvm-mc %s -triple=riscv32 -mattr=+experimental-zhinx -riscv-no-aliases \
+# RUN: | FileCheck -check-prefix=CHECK-INST %s
+# RUN: llvm-mc %s -triple=riscv32 -mattr=+experimental-zhinx \
+# RUN: | FileCheck -check-prefix=CHECK-ALIAS %s
+# RUN: llvm-mc %s -triple=riscv64 -mattr=+experimental-zhinx -riscv-no-aliases \
+# RUN: | FileCheck -check-prefix=CHECK-INST %s
+# RUN: llvm-mc %s -triple=riscv64 -mattr=+experimental-zhinx \
+# RUN: | FileCheck -check-prefix=CHECK-ALIAS %s
+# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+experimental-zhinx %s \
+# RUN: | llvm-objdump -d --mattr=+experimental-zhinx -M no-aliases - \
+# RUN: | FileCheck -check-prefix=CHECK-INST %s
+# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+experimental-zhinx %s \
+# RUN: | llvm-objdump -d --mattr=+experimental-zhinx - \
+# RUN: | FileCheck -check-prefix=CHECK-ALIAS %s
+# RUN: llvm-mc -filetype=obj -triple riscv64 -mattr=+experimental-zhinx %s \
+# RUN: | llvm-objdump -d --mattr=+experimental-zhinx -M no-aliases - \
+# RUN: | FileCheck -check-prefix=CHECK-INST %s
+# RUN: llvm-mc -filetype=obj -triple riscv64 -mattr=+experimental-zhinx %s \
+# RUN: | llvm-objdump -d --mattr=+experimental-zhinx - \
+# RUN: | FileCheck -check-prefix=CHECK-ALIAS %s
+
+##===--===##
+## Aliases which omit the rounding mode.
+##===--===##
+
+# CHECK-INST: fmadd.h a0, a1, a2, a3, dyn
+# CHECK-ALIAS: fmadd.h a0, a1, a2, a3
+fmadd.h x10, x11, x12, x13
+# CHECK-INST: fmsub.h a4, a5, a6, a7, dyn
+# CHECK-ALIAS: fmsub.h a4, a5, a6, a7
+fmsub.h x14, x15, x16, x17
+# CHECK-INST: fnmsub.h s2, s3, s4, s5, dyn
+# CHECK-ALIAS: fnmsub.h s2, s3, s4, s5
+fnmsub.h x18, x19, x20, x21
+# CHECK-INST: fnmadd.h s6, s7, s8, s9, dyn
+# CHECK-ALIAS: fnmadd.h s6, s7, s8, s9
+fnmadd.h x22, x23, x24, x25
+# CHECK-INST: fadd.h s10, s11, t3, dyn
+# CHECK-ALIAS: fadd.h s10, s11, t3
+fadd.h x26, x27, x28
+# CHECK-INST: fsub.h t4, t5, t6, dyn
+# CHECK-ALIAS: fsub.h t4, t5, t6
+fsub.h x29, x30, x31
+# CHECK-INST: fmul.h s0, s1, s2, dyn
+# CHECK-ALIAS: fmul.h s0, s1, s2
+fmul.h s0, s1, s2
+# CHECK-INST: fdiv.h s3, s4, s5, dyn
+# CHECK-ALIAS: fdiv.h s3, s4, s5
+fdiv.h s3, s4, s5
Index: llvm/test/MC/RISCV/rvzfinx-aliases-valid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rvzfinx-aliases-valid.s
@@ -0,0 +1,49 @@
+# RUN: llvm-mc %s -triple=riscv32 -mattr=+experimental-zfinx -riscv-no-aliases \
+# RUN: | FileCheck -check-prefix=CHECK-INST %s
+# RUN: llvm-mc %s -triple=riscv32 -mattr=+experimental-zfinx \
+# RUN: | FileCheck -check-prefix=CHECK-ALIAS %s
+# RUN: llvm-mc %s -triple=riscv64 -mattr=+experimental-zfinx -riscv-no-aliases \
+# RUN: | FileCheck -check-prefix=CHECK-INST %s
+# RUN: llvm-mc %s -triple=riscv64 -mattr=+experimental-zfinx \
+# RUN: | FileCheck -check-prefix=CHECK-ALIAS %s
+# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+experimental-zfinx %s \
+# RUN: | llvm-objdump -d --mattr=+experimental-zfinx -M no-aliases - \
+# RUN: | FileCheck -check-prefix=CHECK-INST %s
+# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+experimental-zfinx %s \
+# RUN: | llvm-objdump -d --mattr=+experimental-zfinx - \
+# RUN: | FileCheck -check-prefix=CHECK-ALIAS %s
+# RUN: llvm-mc -filetype=obj -triple riscv64 -mattr=+experimental-zfinx %s \
+# RUN: | llvm-objdump -d --mattr=+experimen

[PATCH] D116638: [ClangFormat] Fix ignoring JavaScriptWrapImport when ColumnWidth: 0

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Thank you for the patch, could you just handle the documentation, but otherwise 
pretty good




Comment at: clang/docs/ClangFormatStyleOptions.rst:2820
 
+**JavaScriptWrapImports** (``Boolean``) :versionbadge:`clang-format 3.9`
+  Whether to wrap JavaScript import/export statements. If ``false``, then

This file is generated from Format.h you need to make the changes there and 
regenerate using clang/docs/tools/dump_format_style.py



Comment at: clang/unittests/Format/FormatTestJS.cpp:1948
+  Style.JavaScriptWrapImports = false;
   verifyFormat("import {VeryLongImportsAreAnnoying, 
VeryLongImportsAreAnnoying,"
" VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying"

you are no longer testing the LLVM Case, please don't remove that, but feel 
free to ensure they are doing the same for google!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116638

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

[PATCH] D116638: [clang-format] Fix ignoring JavaScriptWrapImport when ColumnWidth: 0

[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!
Apart from other reviewer's comments, it looks pretty much ok, but the tests 
are a bit messy IMO.
I'd like to see something that tests 4 cases (false + no column limit, false + 
column limit, true + no column limit, true + column limit). In each case you 
should have the same imports formatted (possibly) differently.
It would be the best to have at least: single short import, single long import, 
multiple short import, multiple long import. And please keep what's currently 
tested.




Comment at: clang/docs/ClangFormatStyleOptions.rst:2836
 
- true:
+ // Original
+ import {VeryLongImport, AnotherLongImport, LongImportsAreAnnoying} from 
'some/module.js'

What does "Original" mean here? It's unclear whether the option is true or 
false nor what the ColumnWidth is.

I think you want to have 3 examples:
1) JavaScriptWrapImports: false, independent of ColumnWidth
2) JavaScriptWrapImports: true, ColumnWidth: 0
3) JavaScriptWrapImports: true, ColumnWidth: non-zero
In each case you want to have short and long import lines.



Comment at: clang/unittests/Format/FormatTestJS.cpp:1990
   Style.ColumnLimit = 40;
-  // Using this version of verifyFormat because test::messUp hides the issue.
+  Style.JavaScriptWrapImports = true;
   verifyFormat("import {\n"

It's already true, cf. line 1977. Remove this line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116638

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

[PATCH] D116592: [clang-format] Missing space after cast in a macro

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 397473.
MyDeveloperDay added a comment.

Fix nit and add a few more tests just to be sure


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

https://reviews.llvm.org/D116592

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
@@ -10150,6 +10150,15 @@
"(aa *)(aa +\n"
"   bb);");
 
+  verifyFormat("#define CONF_BOOL(x) (bool *)(void *)(x)");
+  verifyFormat("#define CONF_BOOL(x) (bool *)(x)");
+  verifyFormat("#define CONF_BOOL(x) (bool)(x)");
+  verifyFormat("bool *y = (bool *)(void *)(x);");
+  verifyFormat("#define CONF_BOOL(x) (bool *)(void *)(int)(x)");
+  verifyFormat("bool *y = (bool *)(void *)(int)(x);");
+  verifyFormat("#define CONF_BOOL(x) (bool *)(void *)(int)foo(x)");
+  verifyFormat("bool *y = (bool *)(void *)(int)foo(x);");
+
   // These are not casts.
   verifyFormat("void f(int *) {}");
   verifyFormat("f(foo)->b;");
@@ -14661,6 +14670,11 @@
"  break;\n"
"}",
Spaces);
+  verifyFormat("#define CONF_BOOL(x) ( bool * ) ( void * ) (x)", Spaces);
+  verifyFormat("#define CONF_BOOL(x) ( bool * ) (x)", Spaces);
+  verifyFormat("#define CONF_BOOL(x) ( bool ) (x)", Spaces);
+  verifyFormat("bool *y = ( bool * ) ( void * ) (x);", Spaces);
+  verifyFormat("bool *y = ( bool * ) (x);", Spaces);
 
   // Run subset of tests again with:
   Spaces.SpacesInCStyleCastParentheses = false;
@@ -14680,6 +14694,11 @@
   verifyFormat("size_t idx = (a->foo)(a - 1);", Spaces);
   verifyFormat("size_t idx = (*foo)(a - 1);", Spaces);
   verifyFormat("size_t idx = (*(foo))(a - 1);", Spaces);
+  verifyFormat("#define CONF_BOOL(x) (bool *) (void *) (x)", Spaces);
+  verifyFormat("#define CONF_BOOL(x) (bool *) (void *) (int) (x)", Spaces);
+  verifyFormat("bool *y = (bool *) (void *) (x);", Spaces);
+  verifyFormat("bool *y = (bool *) (void *) (int) (x);", Spaces);
+  verifyFormat("bool *y = (bool *) (void *) (int) foo(x);", Spaces);
   Spaces.ColumnLimit = 80;
   Spaces.IndentWidth = 4;
   Spaces.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1882,9 +1882,10 @@
 
 FormatToken *LeftOfParens = Tok.MatchingParen->getPreviousNonComment();
 if (LeftOfParens) {
-  // If there is a closing parenthesis left of the current parentheses,
-  // look past it as these might be chained casts.
-  if (LeftOfParens->is(tok::r_paren)) {
+  // If there is a closing parenthesis left of the current
+  // parentheses, look past it as these might be chained casts.
+  if (LeftOfParens->is(tok::r_paren) &&
+  LeftOfParens->isNot(TT_CastRParen)) {
 if (!LeftOfParens->MatchingParen ||
 !LeftOfParens->MatchingParen->Previous)
   return false;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10150,6 +10150,15 @@
"(aa *)(aa +\n"
"   bb);");
 
+  verifyFormat("#define CONF_BOOL(x) (bool *)(void *)(x)");
+  verifyFormat("#define CONF_BOOL(x) (bool *)(x)");
+  verifyFormat("#define CONF_BOOL(x) (bool)(x)");
+  verifyFormat("bool *y = (bool *)(void *)(x);");
+  verifyFormat("#define CONF_BOOL(x) (bool *)(void *)(int)(x)");
+  verifyFormat("bool *y = (bool *)(void *)(int)(x);");
+  verifyFormat("#define CONF_BOOL(x) (bool *)(void *)(int)foo(x)");
+  verifyFormat("bool *y = (bool *)(void *)(int)foo(x);");
+
   // These are not casts.
   verifyFormat("void f(int *) {}");
   verifyFormat("f(foo)->b;");
@@ -14661,6 +14670,11 @@
"  break;\n"
"}",
Spaces);
+  verifyFormat("#define CONF_BOOL(x) ( bool * ) ( void * ) (x)", Spaces);
+  verifyFormat("#define CONF_BOOL(x) ( bool * ) (x)", Spaces);
+  verifyFormat("#define CONF_BOOL(x) ( bool ) (x)", Spaces);
+  verifyFormat("bool *y = ( bool * ) ( void * ) (x);", Spaces);
+  verifyFormat("bool *y = ( bool * ) (x);", Spaces);
 
   // Run subset of tests again with:
   Spaces.SpacesInCStyleCastParentheses = false;
@@ -14680,6 +14694,11 @@
   verifyFormat("size_t idx = (a->foo)(a - 1);", Spaces);
   verifyFormat("size_t idx = (*foo)(a - 1);", Spaces);
   verifyFormat("size_t idx = (*(foo))(a - 1);", Spaces);
+  verifyFormat("#define CONF_BOOL(x) (bool *) (void *)

[PATCH] D116638: [clang-format] Fix ignoring JavaScriptWrapImport when ColumnWidth: 0

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTestJS.cpp:1948
+  Style.JavaScriptWrapImports = false;
   verifyFormat("import {VeryLongImportsAreAnnoying, 
VeryLongImportsAreAnnoying,"
" VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying"

MyDeveloperDay wrote:
> you are no longer testing the LLVM Case, please don't remove that, but feel 
> free to ensure they are doing the same for google!
Its kind of our golden rule, don't change existing tests, subtle changes can 
have huge implications on large code bases


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116638

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

[PATCH] D116314: [clang-format] Add style to separate definition blocks

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

@ksyx

Did you see this issue https://github.com/llvm/llvm-project/issues/52976

In its current form, this patch causes huge amounts of flux in projects
that document code like (anyone using doxygen likely, + most normal people
;-) )

...
}

// My function
  extra newline added
void
foo()
{
}

as it will place and undesirable additional newline between the function
and the return type.

We really need this to be resolved before we branch out v14 (which may be
imminent)

MyDeveloperDay


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116314

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

[PATCH] D116503: [clang] Add arguments for silencing unused argument warnings for some but not all arguments

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D116503#3221083 , @MaskRay wrote:

> A more searchable subject (mentioning the new option in the subject is useful 
> for archaeology) may be `Add 
> --start-no-unused-arguments/--end-no-unused-arguments to silence some unused 
> argument warnings`

Thanks, that's indeed a better summary (although the subject line ends up even 
longer than now). Will change it to that form.




Comment at: clang/docs/ClangCommandLineReference.rst:1342
+
+Don't warn for unused arguments between these arguments.
+

MaskRay wrote:
> The sentence will be scrubbed.
> 
> Use `path/to/clang-tblgen -gen-opt-docs -I llvm/include -I 
> clang/include/clang/Driver clang/include/clang/Driver/ClangOptionDocs.td > 
> clang/docs/ClangCommandLineReference.rst` to regenerate this rst.
Ok, will do. There's lots of other changes in the file though, from other 
options that haven't been added here, but I'm just including the changes for 
the options I'm adding in this commit.



Comment at: clang/include/clang/Driver/Options.td:3918
 def specs : Separate<["-", "--"], "specs">, Flags<[Unsupported]>;
+def start_no_unused_arguments : Flag<["--"], "start-no-unused-arguments">, 
Flags<[NoXarchOption, CoreOption]>,
+  HelpText<"Don't emit warnings about unused arguments for the following 
arguments">;

MaskRay wrote:
> Drop `NoXarchOption`. The option can be used with Darwin -Xarch 
> (`test/Driver/Xarch.c`) though it is unnecessary to test it.
Ok, will do.



Comment at: clang/test/Driver/diagnostics.c:21
+// RUN:   -fsyntax-only -lfoo -Werror %s 2>&1 | FileCheck %s
 
+// With a specific -Wno-..., no diagnostic should be printed.

MaskRay wrote:
> Looks like no command has an output. In case there is one, make sure `-o 
> /dev/null`
As all commands use `-fsyntax-only`, I presume there should be no output. So do 
I read your comment correctly that we should have `-o /dev/null` specifically 
_if_ we'd have a test that lack `-fsyntax-only`, or do you want me to add it 
just in case? (The preexisting test doesn't have that and just use 
`-fsyntax-only`.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116503

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

[PATCH] D116503: [clang] Add arguments for silencing unused argument warnings for some but not all arguments

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo updated this revision to Diff 397476.
mstorsjo added a comment.

Regenerated ClangCommandLineReference.rst (with the relevant changes), removed 
NoXarchOption.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116503

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/diagnostics.c

Index: clang/test/Driver/diagnostics.c
===
--- clang/test/Driver/diagnostics.c
+++ clang/test/Driver/diagnostics.c
@@ -1,9 +1,44 @@
 // Parse diagnostic arguments in the driver
 // PR12181
 
+// Exactly which arguments are warned about and which aren't differ based
+// on what target is selected. -stdlib= and -fuse-ld= emit diagnostics when
+// compiling C code, for e.g. *-linux-gnu. Linker inputs, like -lfoo, emit
+// diagnostics when only compiling for all targets.
+
+// This is normally a non-fatal warning:
+// RUN: %clang -target x86_64-apple-darwin10 \
+// RUN:   -fsyntax-only -lfoo %s 2>&1 | FileCheck %s
+
+// Either with a specific -Werror=unused.. or a blanket -Werror, this
+// causes the command to fail.
+// RUN: not %clang -target x86_64-apple-darwin10 \
+// RUN:   -fsyntax-only -lfoo \
+// RUN:   -Werror=unused-command-line-argument %s 2>&1 | FileCheck %s
+
 // RUN: not %clang -target x86_64-apple-darwin10 \
-// RUN:   -fsyntax-only -fzyzzybalubah \
-// RUN:   -Werror=unused-command-line-argument %s
+// RUN:   -fsyntax-only -lfoo -Werror %s 2>&1 | FileCheck %s
 
+// With a specific -Wno-..., no diagnostic should be printed.
+// RUN: %clang -target x86_64-apple-darwin10 \
+// RUN:   -fsyntax-only -lfoo -Werror \
+// RUN:   -Wno-unused-command-line-argument %s 2>&1 | count 0
+
+// With -Qunused-arguments, no diagnostic should be printed.
+// RUN: %clang -target x86_64-apple-darwin10 \
+// RUN:   -fsyntax-only -lfoo -Werror \
+// RUN:   -Qunused-arguments %s 2>&1 | count 0
+
+// With the argument enclosed in --{start,end}-no-unused-arguments,
+// there's no diagnostic.
+// RUN: %clang -target x86_64-apple-darwin10 -fsyntax-only \
+// RUN:   --start-no-unused-arguments -lfoo --end-no-unused-arguments \
+// RUN:   -Werror %s 2>&1 | count 0
+
+// With --{start,end}-no-unused-argument around a different argument, it
+// still warns about the unused argument.
 // RUN: not %clang -target x86_64-apple-darwin10 \
-// RUN:   -fsyntax-only -fzyzzybalubah -Werror %s
+// RUN:   --start-no-unused-arguments -fsyntax-only --end-no-unused-arguments \
+// RUN:   -lfoo -Werror %s 2>&1 | FileCheck %s
+
+// CHECK: -lfoo: 'linker' input unused
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -367,7 +367,20 @@
   bool HasNostdlib = Args.hasArg(options::OPT_nostdlib);
   bool HasNostdlibxx = Args.hasArg(options::OPT_nostdlibxx);
   bool HasNodefaultlib = Args.hasArg(options::OPT_nodefaultlibs);
+  bool IgnoreUnused = false;
   for (Arg *A : Args) {
+if (IgnoreUnused)
+  A->claim();
+
+if (A->getOption().matches(options::OPT_start_no_unused_arguments)) {
+  IgnoreUnused = true;
+  continue;
+}
+if (A->getOption().matches(options::OPT_end_no_unused_arguments)) {
+  IgnoreUnused = false;
+  continue;
+}
+
 // Unfortunately, we have to parse some forwarding options (-Xassembler,
 // -Xlinker, -Xpreprocessor) because we either integrate their functionality
 // (assembler and preprocessor), or bypass a previous driver ('collect2').
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1076,6 +1076,8 @@
 def emit_merged_ifs : Flag<["-"], "emit-merged-ifs">,
   Flags<[CC1Option]>, Group,
   HelpText<"Generate Interface Stub Files, emit merged text not binary.">;
+def end_no_unused_arguments : Flag<["--"], "end-no-unused-arguments">, Flags<[CoreOption]>,
+  HelpText<"Start emitting warnings for unused driver arguments">;
 def interface_stub_version_EQ : JoinedOrSeparate<["-"], "interface-stub-version=">, Flags<[CC1Option]>;
 def exported__symbols__list : Separate<["-"], "exported_symbols_list">;
 def e : JoinedOrSeparate<["-"], "e">, Flags<[LinkerInput]>, Group;
@@ -3913,6 +3915,8 @@
 def single__module : Flag<["-"], "single_module">;
 def specs_EQ : Joined<["-", "--"], "specs=">, Group;
 def specs : Separate<["-", "--"], "specs">, Flags<[Unsupported]>;
+def start_no_unused_arguments : Flag<["--"], "start-no-unused-arguments">, Flags<[CoreOption]>,
+  HelpText<"Don't emit warnings about unused arguments for the following arguments">;
 def static_libgcc : Flag<["-"], "static-libgcc">;
 def static_libstdcxx : Flag<["-"], "static-libstdc++">;
 def static : Flag<["-", "--"], "static">, Group, Flags<[

[PATCH] D116643: [clangd] Don't rename on symbols from system headers.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: kbobyrev.
Herald added subscribers: usaxena95, kadircet, arphaman.
hokein requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Fixes https://github.com/clangd/clangd/issues/963.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116643

Files:
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp


Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -1198,6 +1198,29 @@
 expectedResult(Code, NewName));
 }
 
+TEST(RenameTest, NoRenameOnSymbolsFromSystemHeaders) {
+  // filter out references not from main file.
+  llvm::StringRef Test =
+  R"cpp(
+#include 
+SystemSym^bol abc;
+)cpp";
+
+  Annotations Code(Test);
+  auto TU = TestTU::withCode(Code.code());
+  TU.AdditionalFiles["system"] = R"cpp(
+class SystemSymbol {};
+)cpp";
+  TU.ExtraArgs = {"-isystem", testRoot()};
+  auto AST = TU.build();
+  llvm::StringRef NewName = "abcde";
+
+  auto Results = rename({Code.point(), NewName, AST, testPath(TU.Filename)});
+  EXPECT_FALSE(Results) << "expected rename returned an error: " << 
Code.code();
+  auto ActualMessage = llvm::toString(Results.takeError());
+  EXPECT_THAT(ActualMessage, testing::HasSubstr("not a supported kind"));
+}
+
 TEST(RenameTest, ProtobufSymbolIsExcluded) {
   Annotations Code("Prot^obuf buf;");
   auto TU = TestTU::withCode(Code.code());
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -159,13 +159,17 @@
   return Result;
 }
 
-// By default, we exclude C++ standard symbols and protobuf symbols as rename
-// these symbols would change system/generated files which are unlikely to be
-// modified.
+// By default, we exclude symbols from system headers and protobuf symbols as
+// rename these symbols would change system/generated files which are unlikely
+// to be modified.
 bool isExcluded(const NamedDecl &RenameDecl) {
-  if (isProtoFile(RenameDecl.getLocation(),
-  RenameDecl.getASTContext().getSourceManager()))
+  const auto &SM = RenameDecl.getASTContext().getSourceManager();
+  if (SM.isInSystemHeader(RenameDecl.getLocation()))
 return true;
+  if (isProtoFile(RenameDecl.getLocation(), SM))
+return true;
+  // FIXME: Remove this std symbol list, as they should be covered by the
+  // above isInSystemHeader check.
   static const auto *StdSymbols = new llvm::DenseSet({
 #define SYMBOL(Name, NameSpace, Header) {#NameSpace #Name},
 #include "StdSymbolMap.inc"


Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -1198,6 +1198,29 @@
 expectedResult(Code, NewName));
 }
 
+TEST(RenameTest, NoRenameOnSymbolsFromSystemHeaders) {
+  // filter out references not from main file.
+  llvm::StringRef Test =
+  R"cpp(
+#include 
+SystemSym^bol abc;
+)cpp";
+
+  Annotations Code(Test);
+  auto TU = TestTU::withCode(Code.code());
+  TU.AdditionalFiles["system"] = R"cpp(
+class SystemSymbol {};
+)cpp";
+  TU.ExtraArgs = {"-isystem", testRoot()};
+  auto AST = TU.build();
+  llvm::StringRef NewName = "abcde";
+
+  auto Results = rename({Code.point(), NewName, AST, testPath(TU.Filename)});
+  EXPECT_FALSE(Results) << "expected rename returned an error: " << Code.code();
+  auto ActualMessage = llvm::toString(Results.takeError());
+  EXPECT_THAT(ActualMessage, testing::HasSubstr("not a supported kind"));
+}
+
 TEST(RenameTest, ProtobufSymbolIsExcluded) {
   Annotations Code("Prot^obuf buf;");
   auto TU = TestTU::withCode(Code.code());
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -159,13 +159,17 @@
   return Result;
 }
 
-// By default, we exclude C++ standard symbols and protobuf symbols as rename
-// these symbols would change system/generated files which are unlikely to be
-// modified.
+// By default, we exclude symbols from system headers and protobuf symbols as
+// rename these symbols would change system/generated files which are unlikely
+// to be modified.
 bool isExcluded(const NamedDecl &RenameDecl) {
-  if (isProtoFile(RenameDecl.getLocation(),
-  RenameDecl.getASTContext().getSourceManager()))
+  const auto &SM = RenameDecl.getASTContext().getSourceManager();
+ 

[PATCH] D116155: [clang][AST][ASTImporter] Set record to complete during import of its members.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:2021
+  // fields imported) at that time without multiple AST import passes.
+  To->setCompleteDefinition(true);
   // Complete the definition even if error is returned.

shafik wrote:
> So `DefinitionCompleterScopeExit` will run 
> `To->setCompleteDefinition(false);`  after this function exits but this will 
> be in effect during the import the base classes. I don't see how the tests 
> you added hit that code.
Should the test `CompleteRecordBeforeImporting` not do the  import in minimal 
mode? The comment says minimal but in `ASTImporter` minimal mode is not set. 
The test will fail because this added line. But I think it should work to call 
the `To->setCompleteDefinition` here only in non-minimal mode.



Comment at: clang/lib/AST/ASTImporter.cpp:2088
+// Set to false here to force "completion" of the record.
+To->setCompleteDefinition(false);
 if (Error Err = ImportDeclContext(From, /*ForceImport=*/true))

My fix was not correct. This line was added to make a test 
`CompleteRecordBeforeImporting` not fail but it makes the original fix not 
work. Something other is needed.



Comment at: clang/unittests/AST/ASTImporterTest.cpp:7485
+static void foo(A x) {
+  (void)&"text"[x.idx];
+}

shafik wrote:
> The member function body should be considered `complete-class context` so the 
> correct thing to do would be have all the fields laid out by this point.
My initial plan was to import first all fields, then everything else. But it is 
possible that a field has reference to the same record before the import of all 
fields finishes (like in the second test) so this can not work in all cases 
(and it caused other test failures).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116155

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

[PATCH] D116155: [clang][AST][ASTImporter] Set record to complete during import of its members.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:2021
+  // fields imported) at that time without multiple AST import passes.
+  To->setCompleteDefinition(true);
   // Complete the definition even if error is returned.

balazske wrote:
> shafik wrote:
> > So `DefinitionCompleterScopeExit` will run 
> > `To->setCompleteDefinition(false);`  after this function exits but this 
> > will be in effect during the import the base classes. I don't see how the 
> > tests you added hit that code.
> Should the test `CompleteRecordBeforeImporting` not do the  import in minimal 
> mode? The comment says minimal but in `ASTImporter` minimal mode is not set. 
> The test will fail because this added line. But I think it should work to 
> call the `To->setCompleteDefinition` here only in non-minimal mode.
And remove the later added line 2088 `To->setCompleteDefinition(false);`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116155

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

[PATCH] D116583: Change the default optimisation level of PTXAS from -O0 to -O3. This makes the optimisation levels of PTXAS and the ptxjitcompiler equal (ptxjitcompiler defaults to -O3).

[Hugh Delaney via Phabricator via cfe-commits]

hdelan added inline comments.



Comment at: clang/lib/Driver/ToolChains/Cuda.cpp:433
   } else {
-// If no -O was passed, pass -O0 to ptxas -- no opt flag should correspond
-// to no optimizations, but ptxas's default is -O3.
-CmdArgs.push_back("-O0");
+// If no -O was passed, pass -O3 to ptxas -- this makes ptxas's
+// optimization level the same as the ptxjitcompiler.

tra wrote:
> I think this would be contrary to the expectation that lack of `-O` in clang 
> means - `do not optimize` and it generally implies the whole compilation 
> chain, including assembler. Matching whatever nvidia tools do is an 
> insufficient reason for breaking this assumption, IMO. 
> 
> If you do want do run optimized ptxas on unoptimized PTX, you can use 
> `-Xcuda-ptxas -O3`.
I think for the average user, consistency across the `ptxjitcompiler` and 
`ptxas` is far more important than assuming that no `-O` means no optimization. 
I think most users will assume that no `-O` will assume that whatever tools 
being used will take their default optimization level, which in the case of 
clang is `-O0` and in the case of `ptxas` is `-O3`. 

We have had a few bugs with `ptxas`/`ptxjitcompiler` at higher optimization 
levels, which were quite hard to pin down since offline `ptxas` and 
`ptxjitcompiler` were using different optimisation levels, making bugs appear 
in one and not the other. Of course we are aware of this now but this 
inconsistency can result in bugs that are difficult to diagnose. Having 
consistency between the `ptxjitcompiler` and `ptxas` is therefore of practical 
benefit. Whereas if we are to leave it as is, with `ptxas` defaulting to `-O0`, 
the benefit is purely semantic and not practical.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116583

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

[clang] 015e08c - [clang][scandeps] Update Module Cache Path in Test

[Archibald Elliott via cfe-commits]

Author: Archibald Elliott
Date: 2022-01-05T10:42:38Z
New Revision: 015e08c6badad6b27404d6f94569e25c18d79049

URL: 
https://github.com/llvm/llvm-project/commit/015e08c6badad6b27404d6f94569e25c18d79049
DIFF: 
https://github.com/llvm/llvm-project/commit/015e08c6badad6b27404d6f94569e25c18d79049.diff

LOG: [clang][scandeps] Update Module Cache Path in Test

This fixes the test introduced in D114206 so it no longer writes to the current 
working directory.

Reviewed By: simon_tatham

Differential Revision: https://reviews.llvm.org/D116611

Added: 


Modified: 
clang/test/ClangScanDeps/modulemap-via-vfs.m

Removed: 




diff  --git a/clang/test/ClangScanDeps/modulemap-via-vfs.m 
b/clang/test/ClangScanDeps/modulemap-via-vfs.m
index cc152b63f4831..b239450a752ec 100644
--- a/clang/test/ClangScanDeps/modulemap-via-vfs.m
+++ b/clang/test/ClangScanDeps/modulemap-via-vfs.m
@@ -15,7 +15,7 @@
 [
 {
   "directory": "DIR",
-  "command": "clang DIR/main.m -Imodules/A -fmodules 
-fmodules-cache-path=module-cache -fimplicit-modules -fimplicit-module-maps 
-ivfsoverlay build/vfs.yaml",
+  "command": "clang DIR/main.m -Imodules/A -fmodules 
-fmodules-cache-path=DIR/module-cache -fimplicit-modules -fimplicit-module-maps 
-ivfsoverlay build/vfs.yaml",
   "file": "DIR/main.m"
 }
 ]



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

[PATCH] D116611: [clang][scandeps] Update Module Cache Path in Test

[Sam Elliott 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 rG015e08c6bada: [clang][scandeps] Update Module Cache Path in 
Test (authored by lenary).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116611

Files:
  clang/test/ClangScanDeps/modulemap-via-vfs.m


Index: clang/test/ClangScanDeps/modulemap-via-vfs.m
===
--- clang/test/ClangScanDeps/modulemap-via-vfs.m
+++ clang/test/ClangScanDeps/modulemap-via-vfs.m
@@ -15,7 +15,7 @@
 [
 {
   "directory": "DIR",
-  "command": "clang DIR/main.m -Imodules/A -fmodules 
-fmodules-cache-path=module-cache -fimplicit-modules -fimplicit-module-maps 
-ivfsoverlay build/vfs.yaml",
+  "command": "clang DIR/main.m -Imodules/A -fmodules 
-fmodules-cache-path=DIR/module-cache -fimplicit-modules -fimplicit-module-maps 
-ivfsoverlay build/vfs.yaml",
   "file": "DIR/main.m"
 }
 ]


Index: clang/test/ClangScanDeps/modulemap-via-vfs.m
===
--- clang/test/ClangScanDeps/modulemap-via-vfs.m
+++ clang/test/ClangScanDeps/modulemap-via-vfs.m
@@ -15,7 +15,7 @@
 [
 {
   "directory": "DIR",
-  "command": "clang DIR/main.m -Imodules/A -fmodules -fmodules-cache-path=module-cache -fimplicit-modules -fimplicit-module-maps -ivfsoverlay build/vfs.yaml",
+  "command": "clang DIR/main.m -Imodules/A -fmodules -fmodules-cache-path=DIR/module-cache -fimplicit-modules -fimplicit-module-maps -ivfsoverlay build/vfs.yaml",
   "file": "DIR/main.m"
 }
 ]
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116647: [clang-format] Simplify raw string regex. NFC.

[Marek Kurdej via Phabricator via cfe-commits]

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

Introduced in https://reviews.llvm.org/D115168.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116647

Files:
  clang/lib/Format/Format.cpp


Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2596,8 +2596,9 @@
   bool MainIncludeFound = false;
   bool FormattingOff = false;
 
+  // '[' must be the first and '-' the last character inside [...].
   llvm::Regex RawStringRegex(
-  "R\"(([\\[A-Za-z0-9_{}#<>%:;.?*+/^&\\$|~!=,'\\-]|])*)\\(");
+  "R\"([][A-Za-z0-9_{}#<>%:;.?*+/^&\\$|~!=,'-]*)\\(");
   SmallVector RawStringMatches;
   std::string RawStringTermination = ")\"";
 


Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2596,8 +2596,9 @@
   bool MainIncludeFound = false;
   bool FormattingOff = false;
 
+  // '[' must be the first and '-' the last character inside [...].
   llvm::Regex RawStringRegex(
-  "R\"(([\\[A-Za-z0-9_{}#<>%:;.?*+/^&\\$|~!=,'\\-]|])*)\\(");
+  "R\"([][A-Za-z0-9_{}#<>%:;.?*+/^&\\$|~!=,'-]*)\\(");
   SmallVector RawStringMatches;
   std::string RawStringTermination = ")\"";
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116643: [clangd] Don't rename on symbols from system headers.

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:163
+// By default, we exclude symbols from system headers and protobuf symbols as
+// rename these symbols would change system/generated files which are unlikely
+// to be modified.

nit: "as renaming these symbols..."



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:164
+// rename these symbols would change system/generated files which are unlikely
+// to be modified.
 bool isExcluded(const NamedDecl &RenameDecl) {

nit: "unlikely to be good candidates for modification"

Maybe my wording is bad, too, but what I want to convey is that the problem 
isn't that they can't be modified but that we don't __want__ them to be 
modified!



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:171
+return true;
+  // FIXME: Remove this std symbol list, as they should be covered by the
+  // above isInSystemHeader check.

Any reason not to do this right now? (if we keep the comment, then it's 
probably better as "Remove this check because it is redundant in the presence 
of isInSystemHeader")


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116643

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

[PATCH] D116643: [clangd] Don't rename on symbols from system headers.

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:1202
+TEST(RenameTest, NoRenameOnSymbolsFromSystemHeaders) {
+  // filter out references not from main file.
+  llvm::StringRef Test =

Nit: capitalization.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116643

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

[clang] 35493b4 - [clang-format][NFC] Replace deque with vector

[Björn Schäpers via cfe-commits]

Author: Björn Schäpers
Date: 2022-01-05T12:31:34+01:00
New Revision: 35493b45603fc897280cfba60866075888d9a790

URL: 
https://github.com/llvm/llvm-project/commit/35493b45603fc897280cfba60866075888d9a790
DIFF: 
https://github.com/llvm/llvm-project/commit/35493b45603fc897280cfba60866075888d9a790.diff

LOG: [clang-format][NFC] Replace deque with vector

I think the deque was chosen because of a better push_front, but in
combination with llvm::reverse the push_back'ed vector should be the
better choice.

Differential Revision: https://reviews.llvm.org/D115064

Added: 


Modified: 
clang/lib/Format/UnwrappedLineFormatter.cpp

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineFormatter.cpp 
b/clang/lib/Format/UnwrappedLineFormatter.cpp
index 5ba5958fbd53d..18a9685ce431a 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1108,22 +1108,22 @@ class OptimizingLineFormatter : public LineFormatter {
   /// Applies the best formatting by reconstructing the path in the
   /// solution space that leads to \c Best.
   void reconstructPath(LineState &State, StateNode *Best) {
-std::deque Path;
+llvm::SmallVector Path;
 // We do not need a break before the initial token.
 while (Best->Previous) {
-  Path.push_front(Best);
+  Path.push_back(Best);
   Best = Best->Previous;
 }
-for (auto I = Path.begin(), E = Path.end(); I != E; ++I) {
+for (const auto &Node : llvm::reverse(Path)) {
   unsigned Penalty = 0;
-  formatChildren(State, (*I)->NewLine, /*DryRun=*/false, Penalty);
-  Penalty += Indenter->addTokenToState(State, (*I)->NewLine, false);
+  formatChildren(State, Node->NewLine, /*DryRun=*/false, Penalty);
+  Penalty += Indenter->addTokenToState(State, Node->NewLine, false);
 
   LLVM_DEBUG({
-printLineState((*I)->Previous->State);
-if ((*I)->NewLine) {
+printLineState(Node->Previous->State);
+if (Node->NewLine) {
   llvm::dbgs() << "Penalty for placing "
-   << (*I)->Previous->State.NextToken->Tok.getName()
+   << Node->Previous->State.NextToken->Tok.getName()
<< " on a new line: " << Penalty << "\n";
 }
   });



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

[clang] c2257fe - [clang-format] Fix SeparateDefinitionBlocks docs and ...

[Björn Schäpers via cfe-commits]

Author: Björn Schäpers
Date: 2022-01-05T12:31:34+01:00
New Revision: c2257fe236726ed46f1ec3d68633f1b6663d2513

URL: 
https://github.com/llvm/llvm-project/commit/c2257fe236726ed46f1ec3d68633f1b6663d2513
DIFF: 
https://github.com/llvm/llvm-project/commit/c2257fe236726ed46f1ec3d68633f1b6663d2513.diff

LOG: [clang-format] Fix SeparateDefinitionBlocks docs and ...

the Style's equality operator.

This amends 6f6f88ffdae1e12e5f950ef418827a77a55c09c7

Differential Revision: https://reviews.llvm.org/D116557

Added: 


Modified: 
clang/docs/ClangFormatStyleOptions.rst
clang/include/clang/Format/Format.h

Removed: 




diff  --git a/clang/docs/ClangFormatStyleOptions.rst 
b/clang/docs/ClangFormatStyleOptions.rst
index 5a52916acc559..9bc62587c8fb1 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -3192,15 +3192,15 @@ the configuration (without a prefix: ``Auto``).
 **PenaltyBreakFirstLessLess** (``Unsigned``) :versionbadge:`clang-format 3.7`
   The penalty for breaking before the first ``<<``.
 
+**PenaltyBreakOpenParenthesis** (``Unsigned``) :versionbadge:`clang-format 14`
+  The penalty for breaking after ``(``.
+
 **PenaltyBreakString** (``Unsigned``) :versionbadge:`clang-format 3.7`
   The penalty for each line break introduced inside a string literal.
 
 **PenaltyBreakTemplateDeclaration** (``Unsigned``) :versionbadge:`clang-format 
7`
   The penalty for breaking after template declaration.
 
-**PenaltyBreakOpenParenthesis** (``Unsigned``) :versionbadge:`clang-format 14`
-  The penalty for breaking after ``(``.
-
 **PenaltyExcessCharacter** (``Unsigned``) :versionbadge:`clang-format 3.7`
   The penalty for each character outside of the column limit.
 
@@ -3403,19 +3403,8 @@ the configuration (without a prefix: ``Auto``).
   * information */
 
 **SeparateDefinitionBlocks** (``SeparateDefinitionStyle``) 
:versionbadge:`clang-format 14`
-  Specifies the use of empty lines to separate definition blocks, including 
classes,
-  structs, enums, and functions.
-
-  Possible values:
-
-  * ``SDS_Leave`` (in configuration: ``Leave``)
-Leave definition blocks as they are.
-
-  * ``SDS_Always`` (in configuration: ``Always``)
-Insert an empty line between definition blocks.
-
-  * ``SDS_Never`` (in configuration: ``Never``)
-Remove any empty line between definition blocks.
+  Specifies the use of empty lines to separate definition blocks, including
+  classes, structs, enums, and functions.
 
   .. code-block:: c++
 
@@ -3461,6 +3450,19 @@ the configuration (without a prefix: ``Auto``).
  class C {};
  }
 
+  Possible values:
+
+  * ``SDS_Leave`` (in configuration: ``Leave``)
+Leave definition blocks as they are.
+
+  * ``SDS_Always`` (in configuration: ``Always``)
+Insert an empty line between definition blocks.
+
+  * ``SDS_Never`` (in configuration: ``Never``)
+Remove any empty line between definition blocks.
+
+
+
 **ShortNamespaceLines** (``Unsigned``) :versionbadge:`clang-format 14`
   The maximal number of unwrapped lines that a short namespace spans.
   Defaults to 1.

diff  --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index d3113a5fdba41..2ee6e26bde55e 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -3054,6 +3054,7 @@ struct FormatStyle {
   bool ReflowComments;
   // clang-format on
 
+  /// \brief The style if definition blocks should be separated.
   enum SeparateDefinitionStyle {
 /// Leave definition blocks as they are.
 SDS_Leave,
@@ -3862,6 +3863,7 @@ struct FormatStyle {
QualifierOrder == R.QualifierOrder &&
RawStringFormats == R.RawStringFormats &&
ReferenceAlignment == R.ReferenceAlignment &&
+   SeparateDefinitionBlocks == R.SeparateDefinitionBlocks &&
ShortNamespaceLines == R.ShortNamespaceLines &&
SortIncludes == R.SortIncludes &&
SortJavaStaticImport == R.SortJavaStaticImport &&



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

[clang] a1db435 - [clang-format][NFC] Don't pass member by argument

[Björn Schäpers via cfe-commits]

Author: Björn Schäpers
Date: 2022-01-05T12:31:34+01:00
New Revision: a1db43539027898cbd200798c4549115d261ed86

URL: 
https://github.com/llvm/llvm-project/commit/a1db43539027898cbd200798c4549115d261ed86
DIFF: 
https://github.com/llvm/llvm-project/commit/a1db43539027898cbd200798c4549115d261ed86.diff

LOG: [clang-format][NFC] Don't pass member by argument

And then use the argument and member.

Differential Revision: https://reviews.llvm.org/D116558

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 5b3a450e31f2..54c2a62542b9 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -75,7 +75,7 @@ class AnnotatingParser {
   : Style(Style), Line(Line), CurrentToken(Line.First), AutoFound(false),
 Keywords(Keywords) {
 Contexts.push_back(Context(tok::unknown, 1, /*IsExpression=*/false));
-resetTokenMetadata(CurrentToken);
+resetTokenMetadata();
   }
 
 private:
@@ -1409,8 +1409,8 @@ class AnnotatingParser {
Tok.Next->Next->Next && Tok.Next->Next->Next->is(tok::l_paren);
   }
 
-  void resetTokenMetadata(FormatToken *Token) {
-if (!Token)
+  void resetTokenMetadata() {
+if (!CurrentToken)
   return;
 
 // Reset token type in case we have already looked at it and then
@@ -1439,7 +1439,7 @@ class AnnotatingParser {
   CurrentToken = CurrentToken->Next;
 }
 
-resetTokenMetadata(CurrentToken);
+resetTokenMetadata();
   }
 
   /// A struct to hold information valid in a specific context, e.g.



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

[PATCH] D115064: [clang-format][NFC] Replace deque with vector

[Björn Schäpers 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 rG35493b45603f: [clang-format][NFC] Replace deque with vector 
(authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115064

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1108,22 +1108,22 @@
   /// Applies the best formatting by reconstructing the path in the
   /// solution space that leads to \c Best.
   void reconstructPath(LineState &State, StateNode *Best) {
-std::deque Path;
+llvm::SmallVector Path;
 // We do not need a break before the initial token.
 while (Best->Previous) {
-  Path.push_front(Best);
+  Path.push_back(Best);
   Best = Best->Previous;
 }
-for (auto I = Path.begin(), E = Path.end(); I != E; ++I) {
+for (const auto &Node : llvm::reverse(Path)) {
   unsigned Penalty = 0;
-  formatChildren(State, (*I)->NewLine, /*DryRun=*/false, Penalty);
-  Penalty += Indenter->addTokenToState(State, (*I)->NewLine, false);
+  formatChildren(State, Node->NewLine, /*DryRun=*/false, Penalty);
+  Penalty += Indenter->addTokenToState(State, Node->NewLine, false);
 
   LLVM_DEBUG({
-printLineState((*I)->Previous->State);
-if ((*I)->NewLine) {
+printLineState(Node->Previous->State);
+if (Node->NewLine) {
   llvm::dbgs() << "Penalty for placing "
-   << (*I)->Previous->State.NextToken->Tok.getName()
+   << Node->Previous->State.NextToken->Tok.getName()
<< " on a new line: " << Penalty << "\n";
 }
   });


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1108,22 +1108,22 @@
   /// Applies the best formatting by reconstructing the path in the
   /// solution space that leads to \c Best.
   void reconstructPath(LineState &State, StateNode *Best) {
-std::deque Path;
+llvm::SmallVector Path;
 // We do not need a break before the initial token.
 while (Best->Previous) {
-  Path.push_front(Best);
+  Path.push_back(Best);
   Best = Best->Previous;
 }
-for (auto I = Path.begin(), E = Path.end(); I != E; ++I) {
+for (const auto &Node : llvm::reverse(Path)) {
   unsigned Penalty = 0;
-  formatChildren(State, (*I)->NewLine, /*DryRun=*/false, Penalty);
-  Penalty += Indenter->addTokenToState(State, (*I)->NewLine, false);
+  formatChildren(State, Node->NewLine, /*DryRun=*/false, Penalty);
+  Penalty += Indenter->addTokenToState(State, Node->NewLine, false);
 
   LLVM_DEBUG({
-printLineState((*I)->Previous->State);
-if ((*I)->NewLine) {
+printLineState(Node->Previous->State);
+if (Node->NewLine) {
   llvm::dbgs() << "Penalty for placing "
-   << (*I)->Previous->State.NextToken->Tok.getName()
+   << Node->Previous->State.NextToken->Tok.getName()
<< " on a new line: " << Penalty << "\n";
 }
   });
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 918c977 - [clang-format][NFC] Early return in TokenAnnotator::next

[Björn Schäpers via cfe-commits]

Author: Björn Schäpers
Date: 2022-01-05T12:31:35+01:00
New Revision: 918c977dc1c8905086443e13c172a492247d4709

URL: 
https://github.com/llvm/llvm-project/commit/918c977dc1c8905086443e13c172a492247d4709
DIFF: 
https://github.com/llvm/llvm-project/commit/918c977dc1c8905086443e13c172a492247d4709.diff

LOG: [clang-format][NFC] Early return in TokenAnnotator::next

Differential Revision: https://reviews.llvm.org/D116559

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 54c2a62542b9..3cd89127348c 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -1431,13 +1431,14 @@ class AnnotatingParser {
   }
 
   void next() {
-if (CurrentToken) {
-  CurrentToken->NestingLevel = Contexts.size() - 1;
-  CurrentToken->BindingStrength = Contexts.back().BindingStrength;
-  modifyContext(*CurrentToken);
-  determineTokenType(*CurrentToken);
-  CurrentToken = CurrentToken->Next;
-}
+if (!CurrentToken)
+  return;
+
+CurrentToken->NestingLevel = Contexts.size() - 1;
+CurrentToken->BindingStrength = Contexts.back().BindingStrength;
+modifyContext(*CurrentToken);
+determineTokenType(*CurrentToken);
+CurrentToken = CurrentToken->Next;
 
 resetTokenMetadata();
   }



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

[clang] 29d8535 - [clang-format][NFC] TokenAnnotator: Use range based for

[Björn Schäpers via cfe-commits]

Author: Björn Schäpers
Date: 2022-01-05T12:31:35+01:00
New Revision: 29d8535e2b86d27a3c1363646f04ddf1835c30d2

URL: 
https://github.com/llvm/llvm-project/commit/29d8535e2b86d27a3c1363646f04ddf1835c30d2
DIFF: 
https://github.com/llvm/llvm-project/commit/29d8535e2b86d27a3c1363646f04ddf1835c30d2.diff

LOG: [clang-format][NFC] TokenAnnotator: Use range based for

Differential Revision: https://reviews.llvm.org/D116560

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 3cd89127348c..b501c4d37d40 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -2550,11 +2550,8 @@ bool TokenAnnotator::mustBreakForReturnType(const 
AnnotatedLine &Line) const {
 }
 
 void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) {
-  for (SmallVectorImpl::iterator I = Line.Children.begin(),
-  E = Line.Children.end();
-   I != E; ++I) {
-calculateFormattingInformation(**I);
-  }
+  for (AnnotatedLine *ChildLine : Line.Children)
+calculateFormattingInformation(*ChildLine);
 
   Line.First->TotalLength =
   Line.First->IsMultiline ? Style.ColumnLimit



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

[clang] 2ab5d29 - [clang-format][NFC] Use Prev instead of Current->Previous

[Björn Schäpers via cfe-commits]

Author: Björn Schäpers
Date: 2022-01-05T12:31:35+01:00
New Revision: 2ab5d29f556bc3b5510ae495e5bb54b8b5921d1f

URL: 
https://github.com/llvm/llvm-project/commit/2ab5d29f556bc3b5510ae495e5bb54b8b5921d1f
DIFF: 
https://github.com/llvm/llvm-project/commit/2ab5d29f556bc3b5510ae495e5bb54b8b5921d1f.diff

LOG: [clang-format][NFC] Use Prev instead of Current->Previous

Differential Revision: https://reviews.llvm.org/D116561

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index b501c4d37d40..cbe7c096fda8 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -2567,9 +2567,9 @@ void 
TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) {
   while (Current) {
 if (isFunctionDeclarationName(Style.isCpp(), *Current, Line))
   Current->setType(TT_FunctionDeclarationName);
+const FormatToken *Prev = Current->Previous;
 if (Current->is(TT_LineComment)) {
-  if (Current->Previous->is(BK_BracedInit) &&
-  Current->Previous->opensScope())
+  if (Prev->is(BK_BracedInit) && Prev->opensScope())
 Current->SpacesRequiredBefore =
 (Style.Cpp11BracedListStyle && !Style.SpacesInParentheses) ? 0 : 1;
   else
@@ -2610,12 +2610,11 @@ void 
TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) {
 Current->CanBreakBefore =
 Current->MustBreakBefore || canBreakBefore(Line, *Current);
 unsigned ChildSize = 0;
-if (Current->Previous->Children.size() == 1) {
-  FormatToken &LastOfChild = *Current->Previous->Children[0]->Last;
+if (Prev->Children.size() == 1) {
+  FormatToken &LastOfChild = *Prev->Children[0]->Last;
   ChildSize = LastOfChild.isTrailingComment() ? Style.ColumnLimit
   : LastOfChild.TotalLength + 
1;
 }
-const FormatToken *Prev = Current->Previous;
 if (Current->MustBreakBefore || Prev->Children.size() > 1 ||
 (Prev->Children.size() == 1 &&
  Prev->Children[0]->First->MustBreakBefore) ||



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

[clang] 8f6af1d - [clang-format][NFC] Put all state change into the for statement

[Björn Schäpers via cfe-commits]

Author: Björn Schäpers
Date: 2022-01-05T12:31:36+01:00
New Revision: 8f6af1d4688904fda730d0fea78d2df11252bf40

URL: 
https://github.com/llvm/llvm-project/commit/8f6af1d4688904fda730d0fea78d2df11252bf40
DIFF: 
https://github.com/llvm/llvm-project/commit/8f6af1d4688904fda730d0fea78d2df11252bf40.diff

LOG: [clang-format][NFC] Put all state change into the for statement

Differential Revision: https://reviews.llvm.org/D116563

Added: 


Modified: 
clang/lib/Format/UnwrappedLineFormatter.cpp

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineFormatter.cpp 
b/clang/lib/Format/UnwrappedLineFormatter.cpp
index 18a9685ce431..18d61b483952 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1162,7 +1162,8 @@ unsigned UnwrappedLineFormatter::format(
   bool FirstLine = true;
   for (const AnnotatedLine *Line =
Joiner.getNextMergedLine(DryRun, IndentTracker);
-   Line; Line = NextLine, FirstLine = false) {
+   Line; PrevPrevLine = PreviousLine, PreviousLine = Line, Line = NextLine,
+   FirstLine = false) {
 const AnnotatedLine &TheLine = *Line;
 unsigned Indent = IndentTracker.getIndent();
 
@@ -1252,8 +1253,6 @@ unsigned UnwrappedLineFormatter::format(
 }
 if (!DryRun)
   markFinalized(TheLine.First);
-PrevPrevLine = PreviousLine;
-PreviousLine = &TheLine;
   }
   PenaltyCache[CacheKey] = Penalty;
   return Penalty;



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

[clang] 1da96f7 - [clang-format][NFC] Right.Previous is Left

[Björn Schäpers via cfe-commits]

Author: Björn Schäpers
Date: 2022-01-05T12:31:36+01:00
New Revision: 1da96f744951b54f7a3f9bf799fb8e75a31d291b

URL: 
https://github.com/llvm/llvm-project/commit/1da96f744951b54f7a3f9bf799fb8e75a31d291b
DIFF: 
https://github.com/llvm/llvm-project/commit/1da96f744951b54f7a3f9bf799fb8e75a31d291b.diff

LOG: [clang-format][NFC] Right.Previous is Left

Use that name. Also remove the one check for its existence, that is
given.

Differential Revision: https://reviews.llvm.org/D116562

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index cbe7c096fda8..ec9bfdb0b2a7 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -3866,16 +3866,14 @@ bool TokenAnnotator::mustBreakBefore(const 
AnnotatedLine &Line,
(Right.NewlinesBefore > 0 && Right.HasUnescapedNewline);
   if (Left.isTrailingComment())
 return true;
-  if (Right.Previous->IsUnterminatedLiteral)
+  if (Left.IsUnterminatedLiteral)
 return true;
-  if (Right.is(tok::lessless) && Right.Next &&
-  Right.Previous->is(tok::string_literal) &&
+  if (Right.is(tok::lessless) && Right.Next && Left.is(tok::string_literal) &&
   Right.Next->is(tok::string_literal))
 return true;
   // Can break after template<> declaration
-  if (Right.Previous->ClosesTemplateDeclaration &&
-  Right.Previous->MatchingParen &&
-  Right.Previous->MatchingParen->NestingLevel == 0) {
+  if (Left.ClosesTemplateDeclaration && Left.MatchingParen &&
+  Left.MatchingParen->NestingLevel == 0) {
 // Put concepts on the next line e.g.
 // template
 // concept ...
@@ -3908,9 +3906,8 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine 
&Line,
 // has made a deliberate choice and might have aligned the contents of the
 // string literal accordingly. Thus, we try keep existing line breaks.
 return Right.IsMultiline && Right.NewlinesBefore > 0;
-  if ((Right.Previous->is(tok::l_brace) ||
-   (Right.Previous->is(tok::less) && Right.Previous->Previous &&
-Right.Previous->Previous->is(tok::equal))) &&
+  if ((Left.is(tok::l_brace) || (Left.is(tok::less) && Left.Previous &&
+ Left.Previous->is(tok::equal))) &&
   Right.NestingLevel == 1 && Style.Language == FormatStyle::LK_Proto) {
 // Don't put enums or option definitions onto single lines in protocol
 // buffers.
@@ -4014,7 +4011,7 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine 
&Line,
   Right.is(TT_SelectorName) && !Right.is(tok::r_square) && Right.Next) {
 // Keep `@submessage` together in:
 // @submessage { key: value }
-if (Right.Previous && Right.Previous->is(tok::at))
+if (Left.is(tok::at))
   return false;
 // Look for the scope opener after selector in cases like:
 // selector { ...



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

[PATCH] D116557: [clang-format] Fix SeparateDefinitionBlocks docs and ...

[Björn Schäpers 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 rGc2257fe23672: [clang-format] Fix SeparateDefinitionBlocks 
docs and ... (authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116557

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -3054,6 +3054,7 @@
   bool ReflowComments;
   // clang-format on
 
+  /// \brief The style if definition blocks should be separated.
   enum SeparateDefinitionStyle {
 /// Leave definition blocks as they are.
 SDS_Leave,
@@ -3862,6 +3863,7 @@
QualifierOrder == R.QualifierOrder &&
RawStringFormats == R.RawStringFormats &&
ReferenceAlignment == R.ReferenceAlignment &&
+   SeparateDefinitionBlocks == R.SeparateDefinitionBlocks &&
ShortNamespaceLines == R.ShortNamespaceLines &&
SortIncludes == R.SortIncludes &&
SortJavaStaticImport == R.SortJavaStaticImport &&
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -3192,15 +3192,15 @@
 **PenaltyBreakFirstLessLess** (``Unsigned``) :versionbadge:`clang-format 3.7`
   The penalty for breaking before the first ``<<``.
 
+**PenaltyBreakOpenParenthesis** (``Unsigned``) :versionbadge:`clang-format 14`
+  The penalty for breaking after ``(``.
+
 **PenaltyBreakString** (``Unsigned``) :versionbadge:`clang-format 3.7`
   The penalty for each line break introduced inside a string literal.
 
 **PenaltyBreakTemplateDeclaration** (``Unsigned``) :versionbadge:`clang-format 
7`
   The penalty for breaking after template declaration.
 
-**PenaltyBreakOpenParenthesis** (``Unsigned``) :versionbadge:`clang-format 14`
-  The penalty for breaking after ``(``.
-
 **PenaltyExcessCharacter** (``Unsigned``) :versionbadge:`clang-format 3.7`
   The penalty for each character outside of the column limit.
 
@@ -3403,19 +3403,8 @@
   * information */
 
 **SeparateDefinitionBlocks** (``SeparateDefinitionStyle``) 
:versionbadge:`clang-format 14`
-  Specifies the use of empty lines to separate definition blocks, including 
classes,
-  structs, enums, and functions.
-
-  Possible values:
-
-  * ``SDS_Leave`` (in configuration: ``Leave``)
-Leave definition blocks as they are.
-
-  * ``SDS_Always`` (in configuration: ``Always``)
-Insert an empty line between definition blocks.
-
-  * ``SDS_Never`` (in configuration: ``Never``)
-Remove any empty line between definition blocks.
+  Specifies the use of empty lines to separate definition blocks, including
+  classes, structs, enums, and functions.
 
   .. code-block:: c++
 
@@ -3461,6 +3450,19 @@
  class C {};
  }
 
+  Possible values:
+
+  * ``SDS_Leave`` (in configuration: ``Leave``)
+Leave definition blocks as they are.
+
+  * ``SDS_Always`` (in configuration: ``Always``)
+Insert an empty line between definition blocks.
+
+  * ``SDS_Never`` (in configuration: ``Never``)
+Remove any empty line between definition blocks.
+
+
+
 **ShortNamespaceLines** (``Unsigned``) :versionbadge:`clang-format 14`
   The maximal number of unwrapped lines that a short namespace spans.
   Defaults to 1.


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -3054,6 +3054,7 @@
   bool ReflowComments;
   // clang-format on
 
+  /// \brief The style if definition blocks should be separated.
   enum SeparateDefinitionStyle {
 /// Leave definition blocks as they are.
 SDS_Leave,
@@ -3862,6 +3863,7 @@
QualifierOrder == R.QualifierOrder &&
RawStringFormats == R.RawStringFormats &&
ReferenceAlignment == R.ReferenceAlignment &&
+   SeparateDefinitionBlocks == R.SeparateDefinitionBlocks &&
ShortNamespaceLines == R.ShortNamespaceLines &&
SortIncludes == R.SortIncludes &&
SortJavaStaticImport == R.SortJavaStaticImport &&
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -3192,15 +3192,15 @@
 **PenaltyBreakFirstLessLess** (``Unsigned``) :versionbadge:`clang-format 3.7`
   The penalty for breaking before the first ``<<``.
 
+**PenaltyBreakOpenParenthesis** (``Unsigned``) :versionbadge:`clang-format 14`
+  The pena

[PATCH] D116558: [clang-format][NFC] Don't pass member by argument

[Björn Schäpers 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 rGa1db43539027: [clang-format][NFC] Don't pass member by 
argument (authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116558

Files:
  clang/lib/Format/TokenAnnotator.cpp


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -75,7 +75,7 @@
   : Style(Style), Line(Line), CurrentToken(Line.First), AutoFound(false),
 Keywords(Keywords) {
 Contexts.push_back(Context(tok::unknown, 1, /*IsExpression=*/false));
-resetTokenMetadata(CurrentToken);
+resetTokenMetadata();
   }
 
 private:
@@ -1409,8 +1409,8 @@
Tok.Next->Next->Next && Tok.Next->Next->Next->is(tok::l_paren);
   }
 
-  void resetTokenMetadata(FormatToken *Token) {
-if (!Token)
+  void resetTokenMetadata() {
+if (!CurrentToken)
   return;
 
 // Reset token type in case we have already looked at it and then
@@ -1439,7 +1439,7 @@
   CurrentToken = CurrentToken->Next;
 }
 
-resetTokenMetadata(CurrentToken);
+resetTokenMetadata();
   }
 
   /// A struct to hold information valid in a specific context, e.g.


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -75,7 +75,7 @@
   : Style(Style), Line(Line), CurrentToken(Line.First), AutoFound(false),
 Keywords(Keywords) {
 Contexts.push_back(Context(tok::unknown, 1, /*IsExpression=*/false));
-resetTokenMetadata(CurrentToken);
+resetTokenMetadata();
   }
 
 private:
@@ -1409,8 +1409,8 @@
Tok.Next->Next->Next && Tok.Next->Next->Next->is(tok::l_paren);
   }
 
-  void resetTokenMetadata(FormatToken *Token) {
-if (!Token)
+  void resetTokenMetadata() {
+if (!CurrentToken)
   return;
 
 // Reset token type in case we have already looked at it and then
@@ -1439,7 +1439,7 @@
   CurrentToken = CurrentToken->Next;
 }
 
-resetTokenMetadata(CurrentToken);
+resetTokenMetadata();
   }
 
   /// A struct to hold information valid in a specific context, e.g.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116559: [clang-format][NFC] Early return in TokenAnnotator::next

[Björn Schäpers 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 rG918c977dc1c8: [clang-format][NFC] Early return in 
TokenAnnotator::next (authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116559

Files:
  clang/lib/Format/TokenAnnotator.cpp


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1431,13 +1431,14 @@
   }
 
   void next() {
-if (CurrentToken) {
-  CurrentToken->NestingLevel = Contexts.size() - 1;
-  CurrentToken->BindingStrength = Contexts.back().BindingStrength;
-  modifyContext(*CurrentToken);
-  determineTokenType(*CurrentToken);
-  CurrentToken = CurrentToken->Next;
-}
+if (!CurrentToken)
+  return;
+
+CurrentToken->NestingLevel = Contexts.size() - 1;
+CurrentToken->BindingStrength = Contexts.back().BindingStrength;
+modifyContext(*CurrentToken);
+determineTokenType(*CurrentToken);
+CurrentToken = CurrentToken->Next;
 
 resetTokenMetadata();
   }


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1431,13 +1431,14 @@
   }
 
   void next() {
-if (CurrentToken) {
-  CurrentToken->NestingLevel = Contexts.size() - 1;
-  CurrentToken->BindingStrength = Contexts.back().BindingStrength;
-  modifyContext(*CurrentToken);
-  determineTokenType(*CurrentToken);
-  CurrentToken = CurrentToken->Next;
-}
+if (!CurrentToken)
+  return;
+
+CurrentToken->NestingLevel = Contexts.size() - 1;
+CurrentToken->BindingStrength = Contexts.back().BindingStrength;
+modifyContext(*CurrentToken);
+determineTokenType(*CurrentToken);
+CurrentToken = CurrentToken->Next;
 
 resetTokenMetadata();
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116560: [clang-format][NFC] TokenAnnotator: Use range based for

[Björn Schäpers 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 rG29d8535e2b86: [clang-format][NFC] TokenAnnotator: Use range 
based for (authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116560

Files:
  clang/lib/Format/TokenAnnotator.cpp


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2550,11 +2550,8 @@
 }
 
 void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) {
-  for (SmallVectorImpl::iterator I = Line.Children.begin(),
-  E = Line.Children.end();
-   I != E; ++I) {
-calculateFormattingInformation(**I);
-  }
+  for (AnnotatedLine *ChildLine : Line.Children)
+calculateFormattingInformation(*ChildLine);
 
   Line.First->TotalLength =
   Line.First->IsMultiline ? Style.ColumnLimit


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2550,11 +2550,8 @@
 }
 
 void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) {
-  for (SmallVectorImpl::iterator I = Line.Children.begin(),
-  E = Line.Children.end();
-   I != E; ++I) {
-calculateFormattingInformation(**I);
-  }
+  for (AnnotatedLine *ChildLine : Line.Children)
+calculateFormattingInformation(*ChildLine);
 
   Line.First->TotalLength =
   Line.First->IsMultiline ? Style.ColumnLimit
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116561: [clang-format][NFC] Use Prev instead of Current->Previous

[Björn Schäpers 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 rG2ab5d29f556b: [clang-format][NFC] Use Prev instead of 
Current->Previous (authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116561

Files:
  clang/lib/Format/TokenAnnotator.cpp


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2567,9 +2567,9 @@
   while (Current) {
 if (isFunctionDeclarationName(Style.isCpp(), *Current, Line))
   Current->setType(TT_FunctionDeclarationName);
+const FormatToken *Prev = Current->Previous;
 if (Current->is(TT_LineComment)) {
-  if (Current->Previous->is(BK_BracedInit) &&
-  Current->Previous->opensScope())
+  if (Prev->is(BK_BracedInit) && Prev->opensScope())
 Current->SpacesRequiredBefore =
 (Style.Cpp11BracedListStyle && !Style.SpacesInParentheses) ? 0 : 1;
   else
@@ -2610,12 +2610,11 @@
 Current->CanBreakBefore =
 Current->MustBreakBefore || canBreakBefore(Line, *Current);
 unsigned ChildSize = 0;
-if (Current->Previous->Children.size() == 1) {
-  FormatToken &LastOfChild = *Current->Previous->Children[0]->Last;
+if (Prev->Children.size() == 1) {
+  FormatToken &LastOfChild = *Prev->Children[0]->Last;
   ChildSize = LastOfChild.isTrailingComment() ? Style.ColumnLimit
   : LastOfChild.TotalLength + 
1;
 }
-const FormatToken *Prev = Current->Previous;
 if (Current->MustBreakBefore || Prev->Children.size() > 1 ||
 (Prev->Children.size() == 1 &&
  Prev->Children[0]->First->MustBreakBefore) ||


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2567,9 +2567,9 @@
   while (Current) {
 if (isFunctionDeclarationName(Style.isCpp(), *Current, Line))
   Current->setType(TT_FunctionDeclarationName);
+const FormatToken *Prev = Current->Previous;
 if (Current->is(TT_LineComment)) {
-  if (Current->Previous->is(BK_BracedInit) &&
-  Current->Previous->opensScope())
+  if (Prev->is(BK_BracedInit) && Prev->opensScope())
 Current->SpacesRequiredBefore =
 (Style.Cpp11BracedListStyle && !Style.SpacesInParentheses) ? 0 : 1;
   else
@@ -2610,12 +2610,11 @@
 Current->CanBreakBefore =
 Current->MustBreakBefore || canBreakBefore(Line, *Current);
 unsigned ChildSize = 0;
-if (Current->Previous->Children.size() == 1) {
-  FormatToken &LastOfChild = *Current->Previous->Children[0]->Last;
+if (Prev->Children.size() == 1) {
+  FormatToken &LastOfChild = *Prev->Children[0]->Last;
   ChildSize = LastOfChild.isTrailingComment() ? Style.ColumnLimit
   : LastOfChild.TotalLength + 1;
 }
-const FormatToken *Prev = Current->Previous;
 if (Current->MustBreakBefore || Prev->Children.size() > 1 ||
 (Prev->Children.size() == 1 &&
  Prev->Children[0]->First->MustBreakBefore) ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116562: [clang-format][NFC] Right.Previous is Left

[Björn Schäpers 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 rG1da96f744951: [clang-format][NFC] Right.Previous is Left 
(authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116562

Files:
  clang/lib/Format/TokenAnnotator.cpp


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3866,16 +3866,14 @@
(Right.NewlinesBefore > 0 && Right.HasUnescapedNewline);
   if (Left.isTrailingComment())
 return true;
-  if (Right.Previous->IsUnterminatedLiteral)
+  if (Left.IsUnterminatedLiteral)
 return true;
-  if (Right.is(tok::lessless) && Right.Next &&
-  Right.Previous->is(tok::string_literal) &&
+  if (Right.is(tok::lessless) && Right.Next && Left.is(tok::string_literal) &&
   Right.Next->is(tok::string_literal))
 return true;
   // Can break after template<> declaration
-  if (Right.Previous->ClosesTemplateDeclaration &&
-  Right.Previous->MatchingParen &&
-  Right.Previous->MatchingParen->NestingLevel == 0) {
+  if (Left.ClosesTemplateDeclaration && Left.MatchingParen &&
+  Left.MatchingParen->NestingLevel == 0) {
 // Put concepts on the next line e.g.
 // template
 // concept ...
@@ -3908,9 +3906,8 @@
 // has made a deliberate choice and might have aligned the contents of the
 // string literal accordingly. Thus, we try keep existing line breaks.
 return Right.IsMultiline && Right.NewlinesBefore > 0;
-  if ((Right.Previous->is(tok::l_brace) ||
-   (Right.Previous->is(tok::less) && Right.Previous->Previous &&
-Right.Previous->Previous->is(tok::equal))) &&
+  if ((Left.is(tok::l_brace) || (Left.is(tok::less) && Left.Previous &&
+ Left.Previous->is(tok::equal))) &&
   Right.NestingLevel == 1 && Style.Language == FormatStyle::LK_Proto) {
 // Don't put enums or option definitions onto single lines in protocol
 // buffers.
@@ -4014,7 +4011,7 @@
   Right.is(TT_SelectorName) && !Right.is(tok::r_square) && Right.Next) {
 // Keep `@submessage` together in:
 // @submessage { key: value }
-if (Right.Previous && Right.Previous->is(tok::at))
+if (Left.is(tok::at))
   return false;
 // Look for the scope opener after selector in cases like:
 // selector { ...


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3866,16 +3866,14 @@
(Right.NewlinesBefore > 0 && Right.HasUnescapedNewline);
   if (Left.isTrailingComment())
 return true;
-  if (Right.Previous->IsUnterminatedLiteral)
+  if (Left.IsUnterminatedLiteral)
 return true;
-  if (Right.is(tok::lessless) && Right.Next &&
-  Right.Previous->is(tok::string_literal) &&
+  if (Right.is(tok::lessless) && Right.Next && Left.is(tok::string_literal) &&
   Right.Next->is(tok::string_literal))
 return true;
   // Can break after template<> declaration
-  if (Right.Previous->ClosesTemplateDeclaration &&
-  Right.Previous->MatchingParen &&
-  Right.Previous->MatchingParen->NestingLevel == 0) {
+  if (Left.ClosesTemplateDeclaration && Left.MatchingParen &&
+  Left.MatchingParen->NestingLevel == 0) {
 // Put concepts on the next line e.g.
 // template
 // concept ...
@@ -3908,9 +3906,8 @@
 // has made a deliberate choice and might have aligned the contents of the
 // string literal accordingly. Thus, we try keep existing line breaks.
 return Right.IsMultiline && Right.NewlinesBefore > 0;
-  if ((Right.Previous->is(tok::l_brace) ||
-   (Right.Previous->is(tok::less) && Right.Previous->Previous &&
-Right.Previous->Previous->is(tok::equal))) &&
+  if ((Left.is(tok::l_brace) || (Left.is(tok::less) && Left.Previous &&
+ Left.Previous->is(tok::equal))) &&
   Right.NestingLevel == 1 && Style.Language == FormatStyle::LK_Proto) {
 // Don't put enums or option definitions onto single lines in protocol
 // buffers.
@@ -4014,7 +4011,7 @@
   Right.is(TT_SelectorName) && !Right.is(tok::r_square) && Right.Next) {
 // Keep `@submessage` together in:
 // @submessage { key: value }
-if (Right.Previous && Right.Previous->is(tok::at))
+if (Left.is(tok::at))
   return false;
 // Look for the scope opener after selector in cases like:
 // selector { ...
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116563: [clang-format][NFC] Put all state change into the for statement

[Björn Schäpers 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 rG8f6af1d46889: [clang-format][NFC] Put all state change into 
the for statement (authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116563

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1162,7 +1162,8 @@
   bool FirstLine = true;
   for (const AnnotatedLine *Line =
Joiner.getNextMergedLine(DryRun, IndentTracker);
-   Line; Line = NextLine, FirstLine = false) {
+   Line; PrevPrevLine = PreviousLine, PreviousLine = Line, Line = NextLine,
+   FirstLine = false) {
 const AnnotatedLine &TheLine = *Line;
 unsigned Indent = IndentTracker.getIndent();
 
@@ -1252,8 +1253,6 @@
 }
 if (!DryRun)
   markFinalized(TheLine.First);
-PrevPrevLine = PreviousLine;
-PreviousLine = &TheLine;
   }
   PenaltyCache[CacheKey] = Penalty;
   return Penalty;


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1162,7 +1162,8 @@
   bool FirstLine = true;
   for (const AnnotatedLine *Line =
Joiner.getNextMergedLine(DryRun, IndentTracker);
-   Line; Line = NextLine, FirstLine = false) {
+   Line; PrevPrevLine = PreviousLine, PreviousLine = Line, Line = NextLine,
+   FirstLine = false) {
 const AnnotatedLine &TheLine = *Line;
 unsigned Indent = IndentTracker.getIndent();
 
@@ -1252,8 +1253,6 @@
 }
 if (!DryRun)
   markFinalized(TheLine.First);
-PrevPrevLine = PreviousLine;
-PreviousLine = &TheLine;
   }
   PenaltyCache[CacheKey] = Penalty;
   return Penalty;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116596: [clang][dataflow] Add transfer functions for assignment

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev updated this revision to Diff 397508.
sgatev marked 3 inline comments as done.
sgatev added a comment.

Address reviewers' comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116596

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -82,10 +82,11 @@
 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "foo");
 ASSERT_THAT(FooDecl, NotNull());
 
-const StorageLocation *FooLoc = Env.getStorageLocation(*FooDecl);
+const StorageLocation *FooLoc =
+Env.getStorageLocation(*FooDecl, SkipPast::None);
 ASSERT_TRUE(isa_and_nonnull(FooLoc));
 
-const Value *FooVal = Env.getValue(*FooLoc);
+const Value *FooVal = Env.getValue(*FooLoc, SkipPast::None);
 ASSERT_TRUE(isa_and_nonnull(FooVal));
   });
 }
@@ -125,14 +126,15 @@
 }
 ASSERT_THAT(BarDecl, NotNull());
 
-const auto *FooLoc =
-cast(Env.getStorageLocation(*FooDecl));
+const auto *FooLoc = cast(
+Env.getStorageLocation(*FooDecl, SkipPast::None));
 const auto *BarLoc =
 cast(&FooLoc->getChild(*BarDecl));
 
-const auto *FooVal = cast(Env.getValue(*FooLoc));
+const auto *FooVal =
+cast(Env.getValue(*FooLoc, SkipPast::None));
 const auto *BarVal = cast(&FooVal->getChild(*BarDecl));
-ASSERT_EQ(Env.getValue(*BarLoc), BarVal);
+ASSERT_EQ(Env.getValue(*BarLoc, SkipPast::None), BarVal);
   });
 }
 
@@ -171,14 +173,15 @@
 }
 ASSERT_THAT(BarDecl, NotNull());
 
-const auto *FooLoc =
-cast(Env.getStorageLocation(*FooDecl));
+const auto *FooLoc = cast(
+Env.getStorageLocation(*FooDecl, SkipPast::None));
 const auto *BarLoc =
 cast(&FooLoc->getChild(*BarDecl));
 
-const auto *FooVal = cast(Env.getValue(*FooLoc));
+const auto *FooVal =
+cast(Env.getValue(*FooLoc, SkipPast::None));
 const auto *BarVal = cast(&FooVal->getChild(*BarDecl));
-ASSERT_EQ(Env.getValue(*BarLoc), BarVal);
+ASSERT_EQ(Env.getValue(*BarLoc, SkipPast::None), BarVal);
   });
 }
 
@@ -204,15 +207,17 @@
 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "foo");
 ASSERT_THAT(FooDecl, NotNull());
 
-const StorageLocation *FooLoc = Env.getStorageLocation(*FooDecl);
+const StorageLocation *FooLoc =
+Env.getStorageLocation(*FooDecl, SkipPast::None);
 ASSERT_TRUE(isa_and_nonnull(FooLoc));
 
 const ReferenceValue *FooVal =
-cast(Env.getValue(*FooLoc));
+cast(Env.getValue(*FooLoc, SkipPast::None));
 const StorageLocation &FooPointeeLoc = FooVal->getPointeeLoc();
 ASSERT_TRUE(isa(&FooPointeeLoc));
 
-const Value *FooPointeeVal = Env.getValue(FooPointeeLoc);
+const Value *FooPointeeVal =
+Env.getValue(FooPointeeLoc, SkipPast::None);
 ASSERT_TRUE(isa_and_nonnull(FooPointeeVal));
   });
 }
@@ -293,36 +298,37 @@
 ASSERT_THAT(BazRefDecl, NotNull());
 ASSERT_THAT(BazPtrDecl, NotNull());
 
-const auto *FooLoc =
-cast(Env.getStorageLocation(*FooDecl));
-const auto *FooVal = cast(Env.getValue(*FooLoc));
-const auto *FooPointeeVal =
-cast(Env.getValue(FooVal->getPointeeLoc()));
+const auto *FooLoc = cast(
+Env.getStorageLocation(*FooDecl, SkipPast::None));
+const auto *FooVal =
+cast(Env.getValue(*FooLoc, SkipPast::None));
+const auto *FooPointeeVal = cast(
+Env.getValue(FooVal->getPointeeLoc(), SkipPast::None));
 
 const auto *BarVal =
 cast(&FooPointeeVal->getChild(*BarDecl));
-const auto *BarPointeeVal =
-cast(Env.getValue(BarVal->getPointeeLoc()));
+const auto *BarPointeeVal = cast(
+Env.getValue(BarVal->getPointeeLoc(), SkipPast::None));
 
 const auto *FooRefVal =
 cast(&BarPointeeVal->getChild(*FooRefDecl));
 const StorageLocation &FooRefPointeeLoc = FooRefVal->getPointeeLoc();
-ASSERT_THAT(Env.getValue(FooRefPointeeLoc), IsNull());
+ASSERT_THAT(Env.getValue(FooRefPointeeLoc, SkipPast::None), IsNull());
 
 const auto *FooPtrVal =
 cast(&BarPointeeVal->getChild(*FooPtrDecl));
 const StorageLocation &FooPtrPointeeLoc = FooPtrVal->getPointeeLoc();
-ASSERT_THAT(Env.getValue(FooPtrPointeeLoc), IsNull());
+A

[PATCH] D116596: [clang][dataflow] Add transfer functions for assignment

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:34
+/// storage locations or values.
+enum class SkipPast {
+  /// No indirections should be skipped past.

xazax.hun wrote:
> I am just wondering if this is the right level of abstraction. Maybe users do 
> think of this in terms of skipping references? 
> Or would they think in terms of value categories? E.g., having `getLValue` vs 
> `getRValue`. I think we can leave this as is for now, I just like the idea of 
> exploring alternatives.
Absolutely! It makes sense to me to consider this (and other) alternatives so I 
added a FIXME as a reminder.



Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:95
+StorageLocation &Environment::createStorageLocation(const Expr &E) {
+  // Evaluated expressions are always assigned the same storage locations to
+  // ensure that the environment stabilizes across loop iterations. Storage

xazax.hun wrote:
> What is the model for expressions that evaluate to different locations in 
> every loop iterations, e.g.:
> ```
> for(int *p = array; p != array + size; ++p)
>   *p; // The dereference expression
> ```
> 
> Here, having `*p` always evaluate to the same location is not correct, we'd 
> probably need a way to widen this. 
In the current model, the storage location for `p` is stable and the value that 
is assigned to it changes. So, in one iteration the value assigned to the 
storage location for `p` is `val1` which is a `PointerValue` that points to 
`loc1` and in another iteration the value assigned to the storage location for 
`p` is `val2` which is a `PointerValue` that points to `loc2`. The storage 
location for `*p` is also stable and the value that is assigned to it is a 
`ReferenceValue` that points to either `loc1` or `loc2`. I implemented this and 
added a test.



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:46
+
+  Env.setValue(*LHSLoc, *RHSVal);
+}

xazax.hun wrote:
> I think you probably also want to set the correct location for the whole 
> expression. Or is that handled somewhere else?
You're right. I updated the code and added a test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116596

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

[clang] 3dc1907 - [ConstantFold] Use ConstantFoldLoadFromUniformValue() in more places

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2022-01-05T12:47:50+01:00
New Revision: 3dc1907d063c1fb1617a0043d5fdb89104e7f7a3

URL: 
https://github.com/llvm/llvm-project/commit/3dc1907d063c1fb1617a0043d5fdb89104e7f7a3
DIFF: 
https://github.com/llvm/llvm-project/commit/3dc1907d063c1fb1617a0043d5fdb89104e7f7a3.diff

LOG: [ConstantFold] Use ConstantFoldLoadFromUniformValue() in more places

In particular, this also preserves undef when loading from padding,
rather than converting it to zero through a different codepath.

This is the remaining part of D115924.

Added: 


Modified: 
clang/test/CodeGen/aapcs-align.cpp
llvm/lib/Analysis/ConstantFolding.cpp
llvm/test/Transforms/InstSimplify/ConstProp/loads.ll

Removed: 




diff  --git a/clang/test/CodeGen/aapcs-align.cpp 
b/clang/test/CodeGen/aapcs-align.cpp
index 8950908183efc..8543081caf233 100644
--- a/clang/test/CodeGen/aapcs-align.cpp
+++ b/clang/test/CodeGen/aapcs-align.cpp
@@ -134,8 +134,8 @@ void g6() {
   f6m(1, 2, 3, 4, 5, s);
 }
 // CHECK: define{{.*}} void @g6
-// CHECK: call void @f6(i32 1, [4 x i32] [i32 6, i32 7, i32 0, i32 0])
-// CHECK: call void @f6m(i32 1, i32 2, i32 3, i32 4, i32 5, [4 x i32] [i32 6, 
i32 7, i32 0, i32 0])
+// CHECK: call void @f6(i32 1, [4 x i32] [i32 6, i32 7, i32 0, i32 undef])
+// CHECK: call void @f6m(i32 1, i32 2, i32 3, i32 4, i32 5, [4 x i32] [i32 6, 
i32 7, i32 0, i32 undef])
 // CHECK: declare void @f6(i32, [4 x i32])
 // CHECK: declare void @f6m(i32, i32, i32, i32, i32, [4 x i32])
 }

diff  --git a/llvm/lib/Analysis/ConstantFolding.cpp 
b/llvm/lib/Analysis/ConstantFolding.cpp
index e475f5e713db5..a373e69c35559 100644
--- a/llvm/lib/Analysis/ConstantFolding.cpp
+++ b/llvm/lib/Analysis/ConstantFolding.cpp
@@ -106,11 +106,8 @@ Constant *FoldBitCast(Constant *C, Type *DestTy, const 
DataLayout &DL) {
  "Invalid constantexpr bitcast!");
 
   // Catch the obvious splat cases.
-  if (C->isNullValue() && !DestTy->isX86_MMXTy() && !DestTy->isX86_AMXTy())
-return Constant::getNullValue(DestTy);
-  if (C->isAllOnesValue() && !DestTy->isX86_MMXTy() && !DestTy->isX86_AMXTy() 
&&
-  !DestTy->isPtrOrPtrVectorTy()) // Don't get ones for ptr types!
-return Constant::getAllOnesValue(DestTy);
+  if (Constant *Res = ConstantFoldLoadFromUniformValue(C, DestTy))
+return Res;
 
   if (auto *VTy = dyn_cast(C->getType())) {
 // Handle a vector->scalar integer/fp cast.
@@ -362,16 +359,8 @@ Constant *llvm::ConstantFoldLoadThroughBitcast(Constant 
*C, Type *DestTy,
 
 // Catch the obvious splat cases (since all-zeros can coerce non-integral
 // pointers legally).
-if (C->isNullValue() && !DestTy->isX86_MMXTy() && !DestTy->isX86_AMXTy())
-  return Constant::getNullValue(DestTy);
-if (C->isAllOnesValue() &&
-(DestTy->isIntegerTy() || DestTy->isFloatingPointTy() ||
- DestTy->isVectorTy()) &&
-!DestTy->isX86_AMXTy() && !DestTy->isX86_MMXTy() &&
-!DestTy->isPtrOrPtrVectorTy())
-  // Get ones when the input is trivial, but
-  // only for supported types inside getAllOnesValue.
-  return Constant::getAllOnesValue(DestTy);
+if (Constant *Res = ConstantFoldLoadFromUniformValue(C, DestTy))
+  return Res;
 
 // If the type sizes are the same and a cast is legal, just directly
 // cast the constant.

diff  --git a/llvm/test/Transforms/InstSimplify/ConstProp/loads.ll 
b/llvm/test/Transforms/InstSimplify/ConstProp/loads.ll
index 4ba3b6dcfb9a2..ddb95868f531e 100644
--- a/llvm/test/Transforms/InstSimplify/ConstProp/loads.ll
+++ b/llvm/test/Transforms/InstSimplify/ConstProp/loads.ll
@@ -320,7 +320,7 @@ define i8 @load_neg_one_at_unknown_offset() {
 
 define i32 @load_padding() {
 ; CHECK-LABEL: @load_padding(
-; CHECK-NEXT:ret i32 0
+; CHECK-NEXT:ret i32 undef
 ;
   %v = load i32, i32* getelementptr (i32, i32* bitcast ({ i32, [4 x i8] }* 
@g_with_padding to i32*), i64 1)
   ret i32 %v



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

[PATCH] D115060: [clang-format][NFC] Code Tidies in UnwrappedLineFormatter

[Björn Schäpers via Phabricator via cfe-commits]

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

In D115060#3219116 , @curdeius wrote:

> @HazardyKnusperkeks, it *seems* as if this commit (or one of others indicated 
> on Changes tab of the link below) provoked failures in ASan. But it could be 
> something else.
> https://lab.llvm.org/buildbot/#/builders/5/builds/16734
> Could you have a look?

Okay, I don't have access to ASAN or so, I'm using MinGW on Windows. But I 
suspect it is the Assignment of the `PreviousLine` since this is not existent 
every time. So I see the following solutions:

1. Only name `NextLine`, and use `I[-1]`.
2. `const auto HasPreviousLine = I != AnnotatedLines.begin(); const auto 
&PreviousLine = HasPreviousLine ? *I[-1] : *I;` which is safe, since 
`PreviousLine` is only used if `HasPreviousLine` is true, but is a bit 
confusing. It would get an explaining comment.
3. Rearrange the statements so that we can have only one check if there is a 
previous line and define `PreviousLine` inside that `if`. It remains to be seen 
if that's NFC.

I would prefer option 3, but if that would change the behavior would go for 
option 2.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115060

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

[PATCH] D114206: [Clang][ScanDeps] Use the virtual path for module maps

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

@Bigcheese have you got a fix for the vfs issue with `.` and `..`?

Looking closely at your comment, I'm surprised you landed this without an 
llvm-lit xfail for windows, given your comment above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114206

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

[PATCH] D116351: Update Bug report URL to Github Issues

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

@asl hi, do you feel good with this revision?


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

https://reviews.llvm.org/D116351

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

[PATCH] D115790: [Coroutines] Set presplit attribute in Clang and mlir

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Early heads-up: we see a failure with 
`llvm/lib/Transforms/Coroutines/CoroEarly.cpp:186 in bool (anonymous 
namespace)::Lowerer::lowerEarlyIntrinsics(llvm::Function &): 
F.hasFnAttribute(CORO_PRESPLIT_ATTR) && 
F.getFnAttribute(CORO_PRESPLIT_ATTR).getValueAsString() == UNPREPARED_FOR_SPLIT 
&& "The frontend uses Swtich-Resumed ABI should emit " "\"coroutine.presplit\" 
attribute with value \"0\" for the " "coroutine."`

A colleague may follow up with the issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115790

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

[PATCH] D115790: [Coroutines] Set presplit attribute in Clang and mlir

[Chuanqi Xu 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 rGc75cedc237f9: [Coroutines] Set presplit attribute in Clang 
and mlir (authored by ChuanqiXu).

Changed prior to commit:
  https://reviews.llvm.org/D115790?vs=397217&id=397437#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115790

Files:
  clang/lib/CodeGen/CGCoroutine.cpp
  clang/test/CodeGenCoroutines/coro-always-inline.cpp
  clang/test/CodeGenCoroutines/coro-attributes.cpp
  llvm/docs/Coroutines.rst
  llvm/lib/Transforms/Coroutines/CoroEarly.cpp
  llvm/lib/Transforms/Coroutines/CoroInternal.h
  llvm/test/Transforms/Coroutines/coro-async.ll
  llvm/test/Transforms/Coroutines/coro-debug-O2.ll
  llvm/test/Transforms/Coroutines/coro-debug-coro-frame.ll
  llvm/test/Transforms/Coroutines/coro-debug-dbg.values-not_used_in_frame.ll
  llvm/test/Transforms/Coroutines/coro-debug-dbg.values.ll
  llvm/test/Transforms/Coroutines/coro-debug-frame-variable.ll
  llvm/test/Transforms/Coroutines/coro-noalias-param.ll
  llvm/test/Transforms/Coroutines/coro-split-01.ll
  llvm/test/Transforms/Coroutines/coro-split-recursive.ll
  llvm/test/Transforms/Coroutines/ex0.ll
  llvm/test/Transforms/Coroutines/ex1.ll
  llvm/test/Transforms/Coroutines/ex2.ll
  llvm/test/Transforms/Coroutines/ex3.ll
  llvm/test/Transforms/Coroutines/ex4.ll
  llvm/test/Transforms/Coroutines/ex5.ll
  llvm/test/Transforms/Coroutines/phi-coro-end.ll
  llvm/test/Transforms/Coroutines/restart-trigger.ll
  mlir/lib/Dialect/Async/Transforms/AsyncToAsyncRuntime.cpp
  mlir/test/mlir-opt/async.mlir

Index: mlir/test/mlir-opt/async.mlir
===
--- /dev/null
+++ mlir/test/mlir-opt/async.mlir
@@ -0,0 +1,80 @@
+// Check if mlir marks the corresponding function with required coroutine attribute.
+//
+// RUN:   mlir-opt %s -async-to-async-runtime  \
+// RUN:   -async-runtime-ref-counting  \
+// RUN:   -async-runtime-ref-counting-opt  \
+// RUN:   -convert-async-to-llvm   \
+// RUN:   -convert-linalg-to-loops \
+// RUN:   -convert-scf-to-std  \
+// RUN:   -convert-linalg-to-llvm  \
+// RUN:   -convert-memref-to-llvm  \
+// RUN:   -convert-arith-to-llvm   \
+// RUN:   -convert-std-to-llvm \
+// RUN:   -reconcile-unrealized-casts  \
+// RUN: | FileCheck %s
+
+// CHECK: llvm.func @async_execute_fn{{.*}}attributes{{.*}}"coroutine.presplit", "0"
+// CHECK: llvm.func @async_execute_fn_0{{.*}}attributes{{.*}}"coroutine.presplit", "0"
+// CHECK: llvm.func @async_execute_fn_1{{.*}}attributes{{.*}}"coroutine.presplit", "0"
+
+func @main() {
+  %i0 = arith.constant 0 : index
+  %i1 = arith.constant 1 : index
+  %i2 = arith.constant 2 : index
+  %i3 = arith.constant 3 : index
+
+  %c0 = arith.constant 0.0 : f32
+  %c1 = arith.constant 1.0 : f32
+  %c2 = arith.constant 2.0 : f32
+  %c3 = arith.constant 3.0 : f32
+  %c4 = arith.constant 4.0 : f32
+
+  %A = memref.alloc() : memref<4xf32>
+  linalg.fill(%c0, %A) : f32, memref<4xf32>
+
+  %U = memref.cast %A :  memref<4xf32> to memref<*xf32>
+  call @print_memref_f32(%U): (memref<*xf32>) -> ()
+
+  memref.store %c1, %A[%i0]: memref<4xf32>
+  call @mlirAsyncRuntimePrintCurrentThreadId(): () -> ()
+  call @print_memref_f32(%U): (memref<*xf32>) -> ()
+
+  %outer = async.execute {
+memref.store %c2, %A[%i1]: memref<4xf32>
+call @mlirAsyncRuntimePrintCurrentThreadId(): () -> ()
+call @print_memref_f32(%U): (memref<*xf32>) -> ()
+
+// No op async region to create a token for testing async dependency.
+%noop = async.execute {
+  call @mlirAsyncRuntimePrintCurrentThreadId(): () -> ()
+  async.yield
+}
+
+%inner = async.execute [%noop] {
+  memref.store %c3, %A[%i2]: memref<4xf32>
+  call @mlirAsyncRuntimePrintCurrentThreadId(): () -> ()
+  call @print_memref_f32(%U): (memref<*xf32>) -> ()
+
+  async.yield
+}
+async.await %inner : !async.token
+
+memref.store %c4, %A[%i3]: memref<4xf32>
+call @mlirAsyncRuntimePrintCurrentThreadId(): () -> ()
+call @print_memref_f32(%U): (memref<*xf32>) -> ()
+
+async.yield
+  }
+  async.await %outer : !async.token
+
+  call @mlirAsyncRuntimePrintCurrentThreadId(): () -> ()
+  call @print_memref_f32(%U): (memref<*xf32>) -> ()
+
+  memref.dealloc %A : memref<4xf32>
+
+  return
+}
+
+func private @mlirAsyncRuntimePrintCurrentThreadId() -> ()
+
+func private @print_memref_f32(memref<*xf32>) attr

[PATCH] D115790: [Coroutines] Set presplit attribute in Clang and mlir

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D115790#3221307 , @MaskRay wrote:

> Early heads-up: we see a failure with 
> `llvm/lib/Transforms/Coroutines/CoroEarly.cpp:186 in bool (anonymous 
> namespace)::Lowerer::lowerEarlyIntrinsics(llvm::Function &): 
> F.hasFnAttribute(CORO_PRESPLIT_ATTR) && 
> F.getFnAttribute(CORO_PRESPLIT_ATTR).getValueAsString() == 
> UNPREPARED_FOR_SPLIT && "The frontend uses Swtich-Resumed ABI should emit " 
> "\"coroutine.presplit\" attribute with value \"0\" for the " "coroutine."`
>
> A colleague may follow up with the issue or request a revert.

Thanks for reporting this. I have tested this locally with our workloads. I 
guess there are 3 reasons to hit this:
(1) The frontend (clang or mlir) is updated.
(2) The input IR is generated by an old frontend.
(3) The tested frontend uses switched-resume ABI coroutine intrinsics and the 
frontend isn't clang nor mlir.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115790

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

[PATCH] D116599: Simplify AttrBuilder storage for target dependent attributes

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 397486.
serge-sans-paille added a comment.

Take advice into account


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

https://reviews.llvm.org/D116599

Files:
  clang/lib/CodeGen/CGAtomic.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  llvm/include/llvm/CodeGen/IndirectThunks.h
  llvm/include/llvm/IR/Attributes.h
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/CodeGen/Analysis.cpp
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
  llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/lib/IR/Function.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
  llvm/lib/Transforms/Coroutines/CoroSplit.cpp
  llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
  llvm/lib/Transforms/Utils/CallPromotionUtils.cpp
  llvm/lib/Transforms/Utils/InlineFunction.cpp
  llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
  llvm/tools/bugpoint/CrashDebugger.cpp
  llvm/tools/llvm-reduce/deltas/ReduceAttributes.cpp
  llvm/unittests/ExecutionEngine/Orc/IndirectionUtilsTest.cpp
  llvm/unittests/IR/AttributesTest.cpp
  llvm/unittests/IR/InstructionsTest.cpp
  mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

Index: mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
===
--- mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -746,7 +746,7 @@
 return func.emitError(
 "llvm.align attribute attached to LLVM non-pointer argument");
   llvmArg.addAttrs(
-  llvm::AttrBuilder().addAlignmentAttr(llvm::Align(attr.getInt(;
+  llvm::AttrBuilder(llvmArg.getContext()).addAlignmentAttr(llvm::Align(attr.getInt(;
 }
 
 if (auto attr = func.getArgAttrOfType(argIdx, "llvm.sret")) {
@@ -754,7 +754,7 @@
   if (!argTy.isa())
 return func.emitError(
 "llvm.sret attribute attached to LLVM non-pointer argument");
-  llvmArg.addAttrs(llvm::AttrBuilder().addStructRetAttr(
+  llvmArg.addAttrs(llvm::AttrBuilder(llvmArg.getContext()).addStructRetAttr(
   llvmArg.getType()->getPointerElementType()));
 }
 
@@ -763,7 +763,7 @@
   if (!argTy.isa())
 return func.emitError(
 "llvm.byval attribute attached to LLVM non-pointer argument");
-  llvmArg.addAttrs(llvm::AttrBuilder().addByValAttr(
+  llvmArg.addAttrs(llvm::AttrBuilder(llvmArg.getContext()).addByValAttr(
   llvmArg.getType()->getPointerElementType()));
 }
 
Index: llvm/unittests/IR/InstructionsTest.cpp
===
--- llvm/unittests/IR/InstructionsTest.cpp
+++ llvm/unittests/IR/InstructionsTest.cpp
@@ -614,7 +614,7 @@
 
   // Test cloning an attribute.
   {
-AttrBuilder AB;
+AttrBuilder AB(C);
 AB.addAttribute(Attribute::ReadOnly);
 Call->setAttributes(
 AttributeList::get(C, AttributeList::FunctionIndex, AB));
@@ -633,7 +633,7 @@
   std::unique_ptr Call(
   CallInst::Create(FnTy, Callee, Args, OldBundle, "result"));
   Call->setTailCallKind(CallInst::TailCallKind::TCK_NoTail);
-  AttrBuilder AB;
+  AttrBuilder AB(C);
   AB.addAttribute(Attribute::Cold);
   Call->setAttributes(AttributeList::get(C, AttributeList::FunctionIndex, AB));
   Call->setDebugLoc(DebugLoc(MDNode::get(C, None)));
@@ -662,7 +662,7 @@
   std::unique_ptr Invoke(
   InvokeInst::Create(FnTy, Callee, NormalDest.get(), UnwindDest.get(), Args,
  OldBundle, "result"));
-  AttrBuilder AB;
+  AttrBuilder AB(C);
   AB.addAttribute(Attribute::Cold);
   Invoke->setAttributes(
   AttributeList::get(C, AttributeList::FunctionIndex, AB));
Index: llvm/unittests/IR/AttributesTest.cpp
===
--- llvm/unittests/IR/AttributesTest.cpp
+++ llvm/unittests/IR/AttributesTest.cpp
@@ -62,9 +62,9 @@
 TEST(Attributes, AddAttributes) {
   LLVMContext C;
   AttributeList AL;
-  AttrBuilder B;
+  AttrBuilder B(C);
   B.addAttribute(Attribute::NoReturn);
-  AL = AL.addFnAttributes(C, AttributeSet::get(C, B));
+  AL = AL.addFnAttributes(C, AttrBuilder(C, AttributeSet::get(C, B)));
   EXPECT_TRUE(AL.hasFnAttr(Attribute::NoReturn));
   B.clear();
   B.addAttribute(Attribute::SExt);
@@ -78,12 +78,12 @@
 
   Attribute AlignAttr = Attribute::getWithAlignment(C, Align(8));
   Attribute StackAlignAttr = Attribute::getWithStackAlignment(C, Align(32));
-  AttrBuilder B_align_readonly;
+  AttrBuilder B_align_readonly(C);
   B_align_readonly.addAttribute(AlignAttr);
   B_align_readonly.addAttrib

[PATCH] D116599: Simplify AttrBuilder storage for target dependent attributes

[Nikita Popov via Phabricator via cfe-commits]

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

LG from my side, but I'd like a second opinion for the "require LLVMContext in 
AttrBuilder" part of the change, as that's the main API impact.




Comment at: llvm/lib/AsmParser/LLParser.cpp:136
 const std::vector &Attrs = RAG.second;
-AttrBuilder B;
+AttrBuilder B(M->getContext());
 

These `M->getContext()` can be replaced by just `Context`, as LLParser stores a 
reference to that.



Comment at: llvm/lib/AsmParser/LLParser.cpp:1246
+  if (R == NumberedAttrBuilders.end())
+R = NumberedAttrBuilders.emplace(VarID, 
AttrBuilder(M->getContext())).first;
+

Is the separate find call here to avoid constructing AttrBuilder if it's 
already in the map?

I was going to suggest that you can write this as `R = 
NumberedAttrBuilders.emplace(VarID, M->getContext()).first;` and leave the 
construction of `AttrBuilder` to emplace(), but it turns out that emplace() 
still unconditionally constructs the object (because C++) and the sensible 
replacement for this (`try_emplace`) has only been introduced in C++17.



Comment at: llvm/lib/IR/Attributes.cpp:1791
   for (const auto &I : AM.td_attrs())
-TargetDepAttrs.erase(I);
+removeAttribute(I);
 

We might as well directly resolve this TODO, I think it should be as simple as:
```
erase_if(TargetDepAttrs, [&](Attribute A) { return AM.contains(A); });
```


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

https://reviews.llvm.org/D116599

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

[clang] ea83517 - Revert "[Clang][ScanDeps] Use the virtual path for module maps"

[Archibald Elliott via cfe-commits]

Author: Archibald Elliott
Date: 2022-01-05T12:17:06Z
New Revision: ea835171389aa356b865bf9cb72ca8f4f84b64fd

URL: 
https://github.com/llvm/llvm-project/commit/ea835171389aa356b865bf9cb72ca8f4f84b64fd
DIFF: 
https://github.com/llvm/llvm-project/commit/ea835171389aa356b865bf9cb72ca8f4f84b64fd.diff

LOG: Revert "[Clang][ScanDeps] Use the virtual path for module maps"

This reverts commits:
- 04192422c4e3b730c580498b8e948088cb15580b.
- 015e08c6badad6b27404d6f94569e25c18d79049

D114206 was landed before it was approved - and was landed knowing that
the test crashed on windows, without an xfail. The promised follow-up
commit with fixes has not appeared since it was promised on December 14th.

Added: 


Modified: 
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

Removed: 
clang/test/ClangScanDeps/modulemap-via-vfs.m



diff  --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp 
b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 086215e7a573d..9229c67c41787 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -237,13 +237,7 @@ ModuleID ModuleDepCollectorPP::handleTopLevelModule(const 
Module *M) {
.getHeaderSearchInfo()
.getModuleMap()
.getModuleMapFileForUniquing(M);
-
-  if (ModuleMap) {
-StringRef Path = ModuleMap->tryGetRealPathName();
-if (Path.empty())
-  Path = ModuleMap->getName();
-MD.ClangModuleMapFile = std::string(Path);
-  }
+  MD.ClangModuleMapFile = std::string(ModuleMap ? ModuleMap->getName() : "");
 
   serialization::ModuleFile *MF =
   MDC.ScanInstance.getASTReader()->getModuleManager().lookup(

diff  --git a/clang/test/ClangScanDeps/modulemap-via-vfs.m 
b/clang/test/ClangScanDeps/modulemap-via-vfs.m
deleted file mode 100644
index b239450a752ec..0
--- a/clang/test/ClangScanDeps/modulemap-via-vfs.m
+++ /dev/null
@@ -1,56 +0,0 @@
-// RUN: rm -rf %t.dir
-// RUN: split-file %s %t.dir
-// RUN: sed -e "s|DIR|%/t.dir|g" %t.dir/build/compile-commands.json.in > 
%t.dir/build/compile-commands.json
-// RUN: sed -e "s|DIR|%/t.dir|g" %t.dir/build/vfs.yaml.in > 
%t.dir/build/vfs.yaml
-// RUN: clang-scan-deps -compilation-database 
%t.dir/build/compile-commands.json -j 1 -format experimental-full \
-// RUN:   -mode preprocess-minimized-sources -generate-modules-path-args > 
%t.db
-// RUN: %python %S/../../utils/module-deps-to-rsp.py %t.db --module-name=A > 
%t.A.cc1.rsp
-// RUN: cat %t.A.cc1.rsp | sed 's:\?:/:g' | FileCheck %s
-
-// CHECK-NOT: build/module.modulemap
-// CHECK: A/module.modulemap
-
-//--- build/compile-commands.json.in
-
-[
-{
-  "directory": "DIR",
-  "command": "clang DIR/main.m -Imodules/A -fmodules 
-fmodules-cache-path=DIR/module-cache -fimplicit-modules -fimplicit-module-maps 
-ivfsoverlay build/vfs.yaml",
-  "file": "DIR/main.m"
-}
-]
-
-//--- build/module.modulemap
-
-module A {
-  umbrella header "A.h"
-}
-
-//--- modules/A/A.h
-
-typedef int A_t;
-
-//--- build/vfs.yaml.in
-
-{
-  "version": 0,
-  "case-sensitive": "false",
-  "roots": [
-  {
- "contents": [
- {
-"external-contents": "DIR/build/module.modulemap",
-"name": "module.modulemap",
-"type": "file"
- }],
- "name": "DIR/modules/A",
- "type": "directory"
-  }
-  ]
-}
-
-//--- main.m
-
-@import A;
-
-A_t a = 0;



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

[PATCH] D114206: [Clang][ScanDeps] Use the virtual path for module maps

[Sam Elliott via Phabricator via cfe-commits]

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

Update: I'm going to revert this on main. This patch was accepted conditionally 
on you fixing the windows crash, which you have not done, so this was landed 
without being accepted.

Revert is here: https://reviews.llvm.org/rGea835171389a

When you re-land this, don't forget to include the changes from D116611 
, please.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114206

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

[PATCH] D116638: [clang-format] Fix ignoring JavaScriptWrapImport when ColumnWidth: 0

[Martin Probst via Phabricator via cfe-commits]

mprobst added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:2827
 
-**JavaScriptWrapImports** (``Boolean``) :versionbadge:`clang-format 3.9`
-  Whether to wrap JavaScript import/export statements.
+  * If ``ColumnWidth`` is 0 (no limit on the number of columns), then import
+statements will keep the number of lines they start with.

this seems odd to me: my understanding is that clang-format always reflows the 
entire document, there's no logic to ever "keep" whitespace.

Are you sure you are seeing this behaviour? The logic change below sounds more 
as if the ColumnWidth: 0, import lines might not break (mustBreak returns 
false), but might still break?



Comment at: clang/unittests/Format/FormatTestJS.cpp:1975
+   Style);
+  // Using the input_code/expected version of verifyFormat because we can't
+  // indiscriminately test::messUp on these tests.

I'm not sure this test case really repros the doc changes you made (keeps any 
line wraps with ColW: 0).

I think if you wanted to demonstrate that, you'd need to add a test case where 
clang-format would normally make a change, and show that it does not with ColW: 
0.

However I think that's not feasible: by design, clang-format cannot not make a 
change, it always reformats all code. You might need to rethink the intent here.



Comment at: clang/unittests/Format/FormatTestJS.cpp:1990
   Style.ColumnLimit = 40;
-  // Using this version of verifyFormat because test::messUp hides the issue.
+  Style.JavaScriptWrapImports = true;
   verifyFormat("import {\n"

curdeius wrote:
> It's already true, cf. line 1977. Remove this line.
FWIW, I think the tests might be more readable if each configuration ({true, 
false} x {col: 0, col: 40}) explicitly set all the options, even if it's a bit 
redundant.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116638

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

[PATCH] D116518: [ast-matchers] Add hasSubstatementSequence matcher

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:5435-5442
+/// Matches two consecutive statements within a compound statement.
+///
+/// Given
+/// \code
+///   { if (x > 0) return true; return false; }
+/// \endcode
+/// compoundStmt(hasSubstatementSequence(ifStmt(), returnStmt()))

LegalizeAdulthood wrote:
> aaron.ballman wrote:
> > LegalizeAdulthood wrote:
> > > aaron.ballman wrote:
> > > > How do we extend this to support testing arbitrary sequences of 
> > > > statements? (If this supports two statements, someone will find a need 
> > > > for three, etc).
> > > Yeah, I was wondering that too.  I didn't see (but didn't look 
> > > extensively) any support for variadic matchers taking a parameter pack.
> > > 
> > > I stopped at 2 because this suits my immediate needs with 
> > > `readability-simplify-boolean-expr` where I have to manually loop over 
> > > `CompoundStmt` matches in order to verify that the `if (x) return true; 
> > > return false;` constructs consist of two adjacent statements.
> > I don't think we have any variadic matchers yet to model on, but I think if 
> > this is only needed for one check, we can add it in the current form as a 
> > local matcher for that check. Whenever we figure out how to give the better 
> > interface through the static and dynamic matchers, then we can figure out 
> > how to lift this to the general matcher interface.
> > 
> > WDYT?
> I don't think it is harmful to make it visible to all and I think it is 
> helpful.
> Defining it in ASTMatchers, enables using it in `clang-query`, for instance.
> 
I contend this is not a generally useful matcher without supporting an 
arbitrary number of statements. Even then, to be honest, it's questionable 
whether there's sufficient need for this to be a public matcher. Typically, we 
don't expose a new public matcher unless there's a general need for it, and 
this one is already pretty borderline even if it's fully generalized. This file 
is *very* expensive to instantiate and it gets used in a lot of places, so 
that's one of the primary reasons we don't expose matchers from here unless 
they're generally useful.

Unless @klimek or another AST matcher code owner thinks this is useful in 
general (to multiple checks people are likely to want to write even if they're 
not contributing the checks to the community), I'm opposed to exposing this 
as-is. However, adding it as a private matcher for the check that needs the 
limited functionality would get no opposition from me (and this implementation 
looks correct as well).


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

https://reviews.llvm.org/D116518

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

[PATCH] D116020: [clang][#52782] Bail on incomplete parameter type in stdcall name mangling

[Markus Böck via Phabricator via cfe-commits]

zero9178 updated this revision to Diff 397524.
zero9178 added a comment.

Update test according to reviewers comment


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

https://reviews.llvm.org/D116020

Files:
  clang/lib/AST/Mangle.cpp
  clang/test/CodeGen/pr52782-stdcall-func-decl.cpp


Index: clang/test/CodeGen/pr52782-stdcall-func-decl.cpp
===
--- /dev/null
+++ clang/test/CodeGen/pr52782-stdcall-func-decl.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -triple i686-w64-windows-gnu -o - -emit-llvm 
-debug-info-kind=constructor %s | FileCheck %s
+
+enum nsresult {};
+
+class NotNull;
+
+class nsICanvasRenderingContextInternal {
+  // CHECK: !DISubprogram(name: "InitializeWithDrawTarget", linkageName: 
"\01__ZN33nsICanvasRenderingContextInternal24InitializeWithDrawTargetE7NotNull@4"
+  nsresult __stdcall InitializeWithDrawTarget(NotNull);
+} nsTBaseHashSet;
Index: clang/lib/AST/Mangle.cpp
===
--- clang/lib/AST/Mangle.cpp
+++ clang/lib/AST/Mangle.cpp
@@ -225,11 +225,17 @@
   if (const CXXMethodDecl *MD = dyn_cast(FD))
 if (!MD->isStatic())
   ++ArgWords;
-  for (const auto &AT : Proto->param_types())
+  for (const auto &AT : Proto->param_types()) {
+// If an argument type is incomplete there is no way to get its size to
+// correctly encode into the mangling scheme.
+// Follow GCCs behaviour by simply breaking out of the loop.
+if (AT->isIncompleteType())
+  break;
 // Size should be aligned to pointer size.
 ArgWords +=
 llvm::alignTo(ASTContext.getTypeSize(AT), TI.getPointerWidth(0)) /
 TI.getPointerWidth(0);
+  }
   Out << ((TI.getPointerWidth(0) / 8) * ArgWords);
 }
 


Index: clang/test/CodeGen/pr52782-stdcall-func-decl.cpp
===
--- /dev/null
+++ clang/test/CodeGen/pr52782-stdcall-func-decl.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -triple i686-w64-windows-gnu -o - -emit-llvm -debug-info-kind=constructor %s | FileCheck %s
+
+enum nsresult {};
+
+class NotNull;
+
+class nsICanvasRenderingContextInternal {
+  // CHECK: !DISubprogram(name: "InitializeWithDrawTarget", linkageName: "\01__ZN33nsICanvasRenderingContextInternal24InitializeWithDrawTargetE7NotNull@4"
+  nsresult __stdcall InitializeWithDrawTarget(NotNull);
+} nsTBaseHashSet;
Index: clang/lib/AST/Mangle.cpp
===
--- clang/lib/AST/Mangle.cpp
+++ clang/lib/AST/Mangle.cpp
@@ -225,11 +225,17 @@
   if (const CXXMethodDecl *MD = dyn_cast(FD))
 if (!MD->isStatic())
   ++ArgWords;
-  for (const auto &AT : Proto->param_types())
+  for (const auto &AT : Proto->param_types()) {
+// If an argument type is incomplete there is no way to get its size to
+// correctly encode into the mangling scheme.
+// Follow GCCs behaviour by simply breaking out of the loop.
+if (AT->isIncompleteType())
+  break;
 // Size should be aligned to pointer size.
 ArgWords +=
 llvm::alignTo(ASTContext.getTypeSize(AT), TI.getPointerWidth(0)) /
 TI.getPointerWidth(0);
+  }
   Out << ((TI.getPointerWidth(0) / 8) * ArgWords);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114206: [Clang][ScanDeps] Use the virtual path for module maps

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese added a comment.

In D114206#3221893 , @lenary wrote:

> Update: I'm going to revert this on main. This patch was accepted 
> conditionally on you fixing the windows crash, which you have not done, so 
> this was landed without being accepted.
>
> Revert is here: https://reviews.llvm.org/rGea835171389a
>
> When you re-land this, don't forget to include the changes from D116611 
> , please.

There is no crash on Windows with this patch. The issue is if you pass `main.m` 
instead of `DIR/main.m`. The commit that landed had both fixes I mentioned. Did 
you try this on Windows or look at the bots? In the future please be more 
careful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114206

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

[clang] 46db030 - [clang-format] Simplify raw string regex. NFC.

[Marek Kurdej via cfe-commits]

Author: Marek Kurdej
Date: 2022-01-05T13:39:28+01:00
New Revision: 46db030188e562c02a3ef2bb83360691bad26caf

URL: 
https://github.com/llvm/llvm-project/commit/46db030188e562c02a3ef2bb83360691bad26caf
DIFF: 
https://github.com/llvm/llvm-project/commit/46db030188e562c02a3ef2bb83360691bad26caf.diff

LOG: [clang-format] Simplify raw string regex. NFC.

Introduced in https://reviews.llvm.org/D115168.

Reviewed By: MyDeveloperDay, HazardyKnusperkeks

Differential Revision: https://reviews.llvm.org/D116647

Added: 


Modified: 
clang/lib/Format/Format.cpp

Removed: 




diff  --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index a4ce8a20a940..d1bc378766cd 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -2596,8 +2596,9 @@ tooling::Replacements sortCppIncludes(const FormatStyle 
&Style, StringRef Code,
   bool MainIncludeFound = false;
   bool FormattingOff = false;
 
+  // '[' must be the first and '-' the last character inside [...].
   llvm::Regex RawStringRegex(
-  "R\"(([\\[A-Za-z0-9_{}#<>%:;.?*+/^&\\$|~!=,'\\-]|])*)\\(");
+  "R\"([][A-Za-z0-9_{}#<>%:;.?*+/^&\\$|~!=,'-]*)\\(");
   SmallVector RawStringMatches;
   std::string RawStringTermination = ")\"";
 



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

[PATCH] D116647: [clang-format] Simplify raw string regex. NFC.

[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 rG46db030188e5: [clang-format] Simplify raw string regex. NFC. 
(authored by curdeius).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116647

Files:
  clang/lib/Format/Format.cpp


Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2596,8 +2596,9 @@
   bool MainIncludeFound = false;
   bool FormattingOff = false;
 
+  // '[' must be the first and '-' the last character inside [...].
   llvm::Regex RawStringRegex(
-  "R\"(([\\[A-Za-z0-9_{}#<>%:;.?*+/^&\\$|~!=,'\\-]|])*)\\(");
+  "R\"([][A-Za-z0-9_{}#<>%:;.?*+/^&\\$|~!=,'-]*)\\(");
   SmallVector RawStringMatches;
   std::string RawStringTermination = ")\"";
 


Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2596,8 +2596,9 @@
   bool MainIncludeFound = false;
   bool FormattingOff = false;
 
+  // '[' must be the first and '-' the last character inside [...].
   llvm::Regex RawStringRegex(
-  "R\"(([\\[A-Za-z0-9_{}#<>%:;.?*+/^&\\$|~!=,'\\-]|])*)\\(");
+  "R\"([][A-Za-z0-9_{}#<>%:;.?*+/^&\\$|~!=,'-]*)\\(");
   SmallVector RawStringMatches;
   std::string RawStringTermination = ")\"";
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115060: [clang-format][NFC] Code Tidies in UnwrappedLineFormatter

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks updated this revision to Diff 397531.
HazardyKnusperkeks added a comment.

Option 3


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

https://reviews.llvm.org/D115060

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp

Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -211,10 +211,11 @@
 const AnnotatedLine *TheLine = *I;
 if (TheLine->Last->is(TT_LineComment))
   return 0;
-if (I[1]->Type == LT_Invalid || I[1]->First->MustBreakBefore)
+const auto &NextLine = *I[1];
+if (NextLine.Type == LT_Invalid || NextLine.First->MustBreakBefore)
   return 0;
 if (TheLine->InPPDirective &&
-(!I[1]->InPPDirective || I[1]->First->HasUnescapedNewline))
+(!NextLine.InPPDirective || NextLine.First->HasUnescapedNewline))
   return 0;
 
 if (Style.ColumnLimit > 0 && Indent > Style.ColumnLimit)
@@ -231,43 +232,15 @@
 if (TheLine->Last->is(TT_FunctionLBrace) &&
 TheLine->First == TheLine->Last &&
 !Style.BraceWrapping.SplitEmptyFunction &&
-I[1]->First->is(tok::r_brace))
+NextLine.First->is(tok::r_brace))
   return tryMergeSimpleBlock(I, E, Limit);
 
-// Handle empty record blocks where the brace has already been wrapped
-if (TheLine->Last->is(tok::l_brace) && TheLine->First == TheLine->Last &&
-I != AnnotatedLines.begin()) {
-  bool EmptyBlock = I[1]->First->is(tok::r_brace);
-
-  const FormatToken *Tok = I[-1]->First;
-  if (Tok && Tok->is(tok::comment))
-Tok = Tok->getNextNonComment();
-
-  if (Tok && Tok->getNamespaceToken())
-return !Style.BraceWrapping.SplitEmptyNamespace && EmptyBlock
-   ? tryMergeSimpleBlock(I, E, Limit)
-   : 0;
-
-  if (Tok && Tok->is(tok::kw_typedef))
-Tok = Tok->getNextNonComment();
-  if (Tok && Tok->isOneOf(tok::kw_class, tok::kw_struct, tok::kw_union,
-  tok::kw_extern, Keywords.kw_interface))
-return !Style.BraceWrapping.SplitEmptyRecord && EmptyBlock
-   ? tryMergeSimpleBlock(I, E, Limit)
-   : 0;
-
-  if (Tok && Tok->is(tok::kw_template) &&
-  Style.BraceWrapping.SplitEmptyRecord && EmptyBlock) {
-return 0;
-  }
-}
-
 // FIXME: TheLine->Level != 0 might or might not be the right check to do.
 // If necessary, change to something smarter.
 bool MergeShortFunctions =
 Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_All ||
 (Style.AllowShortFunctionsOnASingleLine >= FormatStyle::SFS_Empty &&
- I[1]->First->is(tok::r_brace)) ||
+ NextLine.First->is(tok::r_brace)) ||
 (Style.AllowShortFunctionsOnASingleLine & FormatStyle::SFS_InlineOnly &&
  TheLine->Level != 0);
 
@@ -280,7 +253,7 @@
closingLine == I[i + 1]->MatchingClosingBlockLineIndex &&
I[i + 1]->Last->TotalLength < Limit;
  i++, closingLine--) {
-  // No extra indent for compacted namespaces
+  // No extra indent for compacted namespaces.
   IndentTracker.skipLine(*I[i + 1]);
 
   Limit -= I[i + 1]->Last->TotalLength;
@@ -296,102 +269,134 @@
getMatchingNamespaceTokenText(I[i + 1], AnnotatedLines) &&
openingLine == I[i + 1]->MatchingOpeningBlockLineIndex;
  i++, openingLine--) {
-  // No space between consecutive braces
+  // No space between consecutive braces.
   I[i + 1]->First->SpacesRequiredBefore = !I[i]->Last->is(tok::r_brace);
 
-  // Indent like the outer-most namespace
+  // Indent like the outer-most namespace.
   IndentTracker.nextLine(*I[i + 1]);
 }
 return i;
   }
 }
 
-// Try to merge a function block with left brace unwrapped
+// Try to merge a function block with left brace unwrapped.
 if (TheLine->Last->is(TT_FunctionLBrace) &&
 TheLine->First != TheLine->Last) {
   return MergeShortFunctions ? tryMergeSimpleBlock(I, E, Limit) : 0;
 }
-// Try to merge a control statement block with left brace unwrapped
-if (TheLine->Last->is(tok::l_brace) && TheLine->First != TheLine->Last &&
+// Try to merge a control statement block with left brace unwrapped.
+if (TheLine->Last->is(tok::l_brace) &&
 TheLine->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for)) {
   return Style.AllowShortBlocksOnASingleLine != FormatStyle::SBS_Never
  ? tryMergeSimpleBlock(I, E, Limit)
  : 0;
 }
-// Try to merge a control statement block with left brace wrapped
-if (I[1]->First->is(tok::l_brace) &&
-(TheLine->First->isOneOf(tok::kw_if, tok::kw_else, tok::kw_while,
- 

[PATCH] D114206: [Clang][ScanDeps] Use the virtual path for module maps

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

In D114206#3221950 , @Bigcheese wrote:

> In D114206#3221893 , @lenary wrote:
>
>> Update: I'm going to revert this on main. This patch was accepted 
>> conditionally on you fixing the windows crash, which you have not done, so 
>> this was landed without being accepted.
>>
>> Revert is here: https://reviews.llvm.org/rGea835171389a
>>
>> When you re-land this, don't forget to include the changes from D116611 
>> , please.
>
> There is no crash on Windows with this patch. The issue is if you pass 
> `main.m` instead of `DIR/main.m`. The commit that landed had both fixes I 
> mentioned. Did you try this on Windows or look at the bots? In the future 
> please be more careful.

I did try this on windows, and I'm still seeing a crash about traversal 
segments, I'll continue investigating that crash.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114206

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

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

[Marek Kurdej via cfe-commits]

Author: Marek Kurdej
Date: 2022-01-05T13:52:45+01:00
New Revision: 5109737c924d68323b1982949a3e28ef26bc289e

URL: 
https://github.com/llvm/llvm-project/commit/5109737c924d68323b1982949a3e28ef26bc289e
DIFF: 
https://github.com/llvm/llvm-project/commit/5109737c924d68323b1982949a3e28ef26bc289e.diff

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

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

Reviewed By: HazardyKnusperkeks, owenpan

Differential Revision: https://reviews.llvm.org/D116527

Added: 


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

Removed: 




diff  --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index f0e0247ce33e..3f87e70fb90c 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -368,6 +368,13 @@ AlignTokenSequence(const FormatStyle &Style, unsigned 
Start, unsigned End,
 Changes[i].Tok->Previous->is(TT_ConditionalExpr))
   return true;
 
+// Continued braced list.
+if (ScopeStart > Start + 1 &&
+Changes[ScopeStart - 2].Tok->isNot(tok::identifier) &&
+Changes[ScopeStart - 1].Tok->is(tok::l_brace) &&
+Changes[i].Tok->isNot(tok::r_brace))
+  return true;
+
 return false;
   };
 

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 1d622e80ed12..d90c3d3a291f 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -16427,6 +16427,11 @@ TEST_F(FormatTest, AlignConsecutiveDeclarations) {
"  doublebar();\n"
"};\n",
Alignment);
+  // http://llvm.org/PR52914
+  verifyFormat("char *a[] = {\"a\", // comment\n"
+   " \"bb\"};\n"
+   "int   bbb = 0;",
+   Alignment);
 
   // PAS_Right
   EXPECT_EQ("void SomeFunction(int parameter = 0) {\n"



___
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]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5109737c924d: [clang-format] Fix indentation for array 
variables with alignment of… (authored by curdeius).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116527

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
@@ -16427,6 +16427,11 @@
"  doublebar();\n"
"};\n",
Alignment);
+  // http://llvm.org/PR52914
+  verifyFormat("char *a[] = {\"a\", // comment\n"
+   " \"bb\"};\n"
+   "int   bbb = 0;",
+   Alignment);
 
   // PAS_Right
   EXPECT_EQ("void SomeFunction(int parameter = 0) {\n"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -368,6 +368,13 @@
 Changes[i].Tok->Previous->is(TT_ConditionalExpr))
   return true;
 
+// Continued braced list.
+if (ScopeStart > Start + 1 &&
+Changes[ScopeStart - 2].Tok->isNot(tok::identifier) &&
+Changes[ScopeStart - 1].Tok->is(tok::l_brace) &&
+Changes[i].Tok->isNot(tok::r_brace))
+  return true;
+
 return false;
   };
 


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16427,6 +16427,11 @@
"  doublebar();\n"
"};\n",
Alignment);
+  // http://llvm.org/PR52914
+  verifyFormat("char *a[] = {\"a\", // comment\n"
+   " \"bb\"};\n"
+   "int   bbb = 0;",
+   Alignment);
 
   // PAS_Right
   EXPECT_EQ("void SomeFunction(int parameter = 0) {\n"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -368,6 +368,13 @@
 Changes[i].Tok->Previous->is(TT_ConditionalExpr))
   return true;
 
+// Continued braced list.
+if (ScopeStart > Start + 1 &&
+Changes[ScopeStart - 2].Tok->isNot(tok::identifier) &&
+Changes[ScopeStart - 1].Tok->is(tok::l_brace) &&
+Changes[i].Tok->isNot(tok::r_brace))
+  return true;
+
 return false;
   };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116328: [ast-matchers] Add hasSubstatement() traversal matcher for caseStmt(), defaultStmt(), labelStmt()

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

> Previously if you wanted to match the statement associated with a case, 
> default, or labelled statement, you had to use hasDescendant.

This isn't true. You can use `has()` to do direct substatemematching, and I'm 
wondering whether we need this new matcher at all. e.g., 
https://godbolt.org/z/K5sYP757r




Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2432
 dependentCoawaitExpr;
+
 /// Matches co_yield expressions.

Spurious newline?



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:7720
 
+/// Matches the substatement associated with a case, default or label 
statement.
+///

May need to reformat as well, but the intent is to make it clear that this does 
not behave like `hasAnySubstatement()`.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:7727
+///   foo: return;
+///   bar: break;
+/// \endcode

This is invalid code (there's nothing to break out of), so you may want to 
tweak the example a bit.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:7730-7734
+/// caseStmt(hasSubstmt(returnStmt()))
+///   matches "case 2: return;"
+/// defaultStmt(hasSubstmt(returnStmt()))
+///   matches "default: return;"
+/// labelStmt(hasSubstmt(breakStmt()))





Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:7737-7738
+AST_POLYMORPHIC_MATCHER_P(hasSubstatement,
+  AST_POLYMORPHIC_SUPPORTED_TYPES(CaseStmt, 
DefaultStmt,
+  LabelStmt),
+  internal::Matcher, InnerMatcher) {

Pretty sure you can use the base class here as a simplification.



Comment at: clang/lib/ASTMatchers/Dynamic/Registry.cpp:69
   registerMatcher(#name, internal::makeMatcherAutoMarshall(
\
- ::clang::ast_matchers::name, #name));
+ ::clang::ast_matchers::name, #name))
 

Good catch; feel free to land this and related changes as an NFC commit.



Comment at: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp:161
+TEST_P(ASTMatchersTest, HasLabelSubstmt) {
+  EXPECT_TRUE(matches("void f() { while (1) { bar: break; foo: return; } }",
+  traverse(TK_AsIs, 
labelStmt(hasSubstatement(breakStmt());

Should fix the formatting.


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

https://reviews.llvm.org/D116328

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

[PATCH] D116638: [clang-format] Fix ignoring JavaScriptWrapImport when ColumnWidth: 0

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:2827
 
-**JavaScriptWrapImports** (``Boolean``) :versionbadge:`clang-format 3.9`
-  Whether to wrap JavaScript import/export statements.
+  * If ``ColumnWidth`` is 0 (no limit on the number of columns), then import
+statements will keep the number of lines they start with.

mprobst wrote:
> this seems odd to me: my understanding is that clang-format always reflows 
> the entire document, there's no logic to ever "keep" whitespace.
> 
> Are you sure you are seeing this behaviour? The logic change below sounds 
> more as if the ColumnWidth: 0, import lines might not break (mustBreak 
> returns false), but might still break?
From what I can tell with `ColumnLimit` (and not `ColumnWidth`) 0, then 
`clang-format` does many things differently. If by design or by accident I'm 
not sure. I can see that often, since I format my code with a limit, but my 
tests without.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116638

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

[clang] afc14a0 - Retire llvm::make_reverse_iterator in favor of std::make_reverse_iterator

[Benjamin Kramer via cfe-commits]

Author: Benjamin Kramer
Date: 2022-01-05T14:07:08+01:00
New Revision: afc14a0d1767d9f9dc1b9d81a2f76916ba2bab61

URL: 
https://github.com/llvm/llvm-project/commit/afc14a0d1767d9f9dc1b9d81a2f76916ba2bab61
DIFF: 
https://github.com/llvm/llvm-project/commit/afc14a0d1767d9f9dc1b9d81a2f76916ba2bab61.diff

LOG: Retire llvm::make_reverse_iterator in favor of std::make_reverse_iterator

std::make_reverse_iterator is a C++14 feature, gcc has it since GCC 5.1.

Added: 


Modified: 
clang/include/clang/Analysis/CFG.h
clang/tools/clang-scan-deps/ClangScanDeps.cpp
lld/ELF/Writer.cpp
llvm/include/llvm/ADT/STLExtras.h
llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h

Removed: 




diff  --git a/clang/include/clang/Analysis/CFG.h 
b/clang/include/clang/Analysis/CFG.h
index b8e453fcc235a..c5512a7e14998 100644
--- a/clang/include/clang/Analysis/CFG.h
+++ b/clang/include/clang/Analysis/CFG.h
@@ -707,7 +707,7 @@ class CFGBlock {
 
 template 
 ElementRefIterator(ElementRefIterator E)
-: ElementRefIterator(E.Parent, llvm::make_reverse_iterator(E.Pos)) {}
+: ElementRefIterator(E.Parent, std::make_reverse_iterator(E.Pos)) {}
 
 bool operator<(ElementRefIterator Other) const {
   assert(Parent == Other.Parent);

diff  --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp 
b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index 4d61e37db29bd..49cc97b27046f 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -480,7 +480,7 @@ int main(int argc, const char **argv) {
   llvm::is_contained(Args, "--driver-mode=cl");
 
   // Reverse scan, starting at the end or at the element before "--".
-  auto R = llvm::make_reverse_iterator(FlagsEnd);
+  auto R = std::make_reverse_iterator(FlagsEnd);
   for (auto I = R, E = Args.rend(); I != E; ++I) {
 StringRef Arg = *I;
 if (ClangCLMode) {

diff  --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index 6527493f1b1fd..9db997cddfbe3 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -1212,9 +1212,9 @@ findOrphanPos(SmallVectorImpl::iterator 
b,
 auto *os = dyn_cast(cmd);
 return os && os->hasInputSections;
   };
-  auto j = std::find_if(llvm::make_reverse_iterator(i),
-llvm::make_reverse_iterator(b),
-isOutputSecWithInputSections);
+  auto j =
+  std::find_if(std::make_reverse_iterator(i), 
std::make_reverse_iterator(b),
+   isOutputSecWithInputSections);
   i = j.base();
 
   // As a special case, if the orphan section is the last section, put

diff  --git a/llvm/include/llvm/ADT/STLExtras.h 
b/llvm/include/llvm/ADT/STLExtras.h
index 2d38e153c79e0..ed5321821a3a2 100644
--- a/llvm/include/llvm/ADT/STLExtras.h
+++ b/llvm/include/llvm/ADT/STLExtras.h
@@ -416,20 +416,14 @@ auto reverse(ContainerTy &&C,
   return make_range(C.rbegin(), C.rend());
 }
 
-// Returns a std::reverse_iterator wrapped around the given iterator.
-template 
-std::reverse_iterator make_reverse_iterator(IteratorTy It) {
-  return std::reverse_iterator(It);
-}
-
 // Returns an iterator_range over the given container which iterates in 
reverse.
 // Note that the container must have begin()/end() methods which return
 // bidirectional iterators for this to work.
 template 
 auto reverse(ContainerTy &&C,
  std::enable_if_t::value> * = nullptr) {
-  return make_range(llvm::make_reverse_iterator(std::end(C)),
-llvm::make_reverse_iterator(std::begin(C)));
+  return make_range(std::make_reverse_iterator(std::end(C)),
+std::make_reverse_iterator(std::begin(C)));
 }
 
 /// An iterator adaptor that filters the elements of given inner iterators.

diff  --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h 
b/llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h
index 8f93ebc4ebc0b..0d4fe9aec8d03 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h
@@ -463,11 +463,11 @@ inline bool operator!=(const 
std::reverse_iterator &LHS,
 }
 
 inline std::reverse_iterator DWARFDie::rbegin() const {
-  return llvm::make_reverse_iterator(end());
+  return std::make_reverse_iterator(end());
 }
 
 inline std::reverse_iterator DWARFDie::rend() const {
-  return llvm::make_reverse_iterator(begin());
+  return std::make_reverse_iterator(begin());
 }
 
 } // end namespace llvm



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

[PATCH] D116615: [Clang] Extract availability mapping from VersionMap for watchOS/tvOS

[Egor Zhdan via Phabricator via cfe-commits]

egorzhdan updated this revision to Diff 397541.
egorzhdan added a comment.

Add a test to verify that `iOS_tvOS` mapping is parsed correctly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116615

Files:
  clang/lib/Basic/DarwinSDKInfo.cpp
  clang/unittests/Basic/DarwinSDKInfoTest.cpp

Index: clang/unittests/Basic/DarwinSDKInfoTest.cpp
===
--- clang/unittests/Basic/DarwinSDKInfoTest.cpp
+++ clang/unittests/Basic/DarwinSDKInfoTest.cpp
@@ -13,7 +13,68 @@
 using namespace llvm;
 using namespace clang;
 
-TEST(DarwinSDKInfoTest, ParseAndTestMapping) {
+// Check the version mapping logic in DarwinSDKInfo.
+TEST(DarwinSDKInfo, VersionMapping) {
+  llvm::json::Object Obj({{"3.0", "1.0"}, {"3.1", "1.2"}});
+  Optional Mapping =
+  DarwinSDKInfo::RelatedTargetVersionMapping::parseJSON(Obj,
+VersionTuple());
+  EXPECT_TRUE(Mapping.hasValue());
+  EXPECT_EQ(Mapping->getMinimumValue(), VersionTuple(1));
+
+  // Exact mapping.
+  EXPECT_EQ(Mapping->map(VersionTuple(3), VersionTuple(0, 1), None),
+VersionTuple(1));
+  EXPECT_EQ(Mapping->map(VersionTuple(3, 0), VersionTuple(0, 1), None),
+VersionTuple(1));
+  EXPECT_EQ(Mapping->map(VersionTuple(3, 0, 0), VersionTuple(0, 1), None),
+VersionTuple(1));
+  EXPECT_EQ(Mapping->map(VersionTuple(3, 1), VersionTuple(0, 1), None),
+VersionTuple(1, 2));
+  EXPECT_EQ(Mapping->map(VersionTuple(3, 1, 0), VersionTuple(0, 1), None),
+VersionTuple(1, 2));
+
+  // Missing mapping - fallback to major.
+  EXPECT_EQ(Mapping->map(VersionTuple(3, 0, 1), VersionTuple(0, 1), None),
+VersionTuple(1));
+
+  // Minimum
+  EXPECT_EQ(Mapping->map(VersionTuple(2), VersionTuple(0, 1), None),
+VersionTuple(0, 1));
+
+  // Maximum
+  EXPECT_EQ(
+  Mapping->map(VersionTuple(4), VersionTuple(0, 1), VersionTuple(100)),
+  VersionTuple(100));
+}
+
+// Check the version mapping logic in DarwinSDKInfo.
+TEST(DarwinSDKInfo, VersionMappingMissingKey) {
+  llvm::json::Object Obj({{"3.0", "1.0"}, {"5.0", "1.2"}});
+  Optional Mapping =
+  DarwinSDKInfo::RelatedTargetVersionMapping::parseJSON(Obj,
+VersionTuple());
+  EXPECT_TRUE(Mapping.hasValue());
+  EXPECT_EQ(
+  Mapping->map(VersionTuple(4), VersionTuple(0, 1), VersionTuple(100)),
+  None);
+}
+
+TEST(DarwinSDKInfo, VersionMappingParseEmpty) {
+  llvm::json::Object Obj({});
+  EXPECT_FALSE(
+  DarwinSDKInfo::RelatedTargetVersionMapping::parseJSON(Obj, VersionTuple())
+  .hasValue());
+}
+
+TEST(DarwinSDKInfo, VersionMappingParseError) {
+  llvm::json::Object Obj({{"test", "1.2"}});
+  EXPECT_FALSE(
+  DarwinSDKInfo::RelatedTargetVersionMapping::parseJSON(Obj, VersionTuple())
+  .hasValue());
+}
+
+TEST(DarwinSDKInfoTest, ParseAndTestMappingMacCatalyst) {
   llvm::json::Object Obj;
   Obj["Version"] = "11.0";
   Obj["MaximumDeploymentTarget"] = "11.99";
@@ -58,6 +119,53 @@
 VersionTuple(99, 99));
 }
 
+TEST(DarwinSDKInfoTest, ParseAndTestMappingIOSDerived) {
+  llvm::json::Object Obj;
+  Obj["Version"] = "15.0";
+  Obj["MaximumDeploymentTarget"] = "15.0.99";
+  llvm::json::Object VersionMap;
+  VersionMap["10.0"] = "10.0";
+  VersionMap["10.3.1"] = "10.2";
+  VersionMap["11.0"] = "11.0";
+  llvm::json::Object IOSToTvOS;
+  IOSToTvOS["iOS_tvOS"] = std::move(VersionMap);
+  Obj["VersionMap"] = std::move(IOSToTvOS);
+
+  auto SDKInfo = DarwinSDKInfo::parseDarwinSDKSettingsJSON(&Obj);
+  ASSERT_TRUE(SDKInfo);
+  EXPECT_EQ(SDKInfo->getVersion(), VersionTuple(15, 0));
+
+  // Verify that mapping is present for platforms that derive from iOS.
+  const auto *Mapping = SDKInfo->getVersionMapping(DarwinSDKInfo::OSEnvPair(
+  llvm::Triple::IOS,
+  llvm::Triple::UnknownEnvironment,
+  llvm::Triple::TvOS,
+  llvm::Triple::UnknownEnvironment));
+  ASSERT_TRUE(Mapping);
+
+  // Verify that the iOS versions that are present in the map are translated
+  // directly to their corresponding tvOS versions.
+  EXPECT_EQ(*Mapping->map(VersionTuple(10, 0), VersionTuple(), None),
+VersionTuple(10, 0));
+  EXPECT_EQ(*Mapping->map(VersionTuple(10, 3, 1), VersionTuple(), None),
+VersionTuple(10, 2));
+  EXPECT_EQ(*Mapping->map(VersionTuple(11, 0), VersionTuple(), None),
+VersionTuple(11, 0));
+
+  // Verify that an iOS version that's not present in the map is translated
+  // like the nearest major OS version.
+  EXPECT_EQ(*Mapping->map(VersionTuple(10, 1), VersionTuple(), None),
+VersionTuple(10, 0));
+
+  // Verify that the iOS versions that are outside of the mapped version
+  // range map to the min/max values passed to the `map` call.
+  EXPECT_EQ(*Mapping->map(VersionTuple(9, 0), VersionTuple(99, 99), None),
+   

[PATCH] D116630: [clangd] Handle declarators more consistently in Selection.

[Haojian Wu via Phabricator via cfe-commits]

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

Thanks, this looks better!




Comment at: clang-tools-extra/clangd/Selection.cpp:756
+claimRange(
+SourceRange(FTL.getParensRange().getBegin(), FTL.getEndLoc()),
+Result);

nit: FTL.getLParenLoc().



Comment at: clang-tools-extra/clangd/Selection.cpp:757
 if (const auto *TL = N.get()) {
-  // e.g. EltType Foo[OuterSize][InnerSize];
-  //  ~ ArrayTypeLoc (Outer)

I'd prefer to keep this comment, while we have some high-level and abstract 
comment in the new patch, I think it is still useful to give us some details 
(otherwise, I'd probably forget everything when coming back to read the code a 
few months later).


I also want to add the example `int (*Fun(OuterType))(InnerType);` from D116618 
here, I understand it might be too verbose (personally, I find it useful to 
understand this part of code), up to you.



Comment at: clang-tools-extra/clangd/unittests/SelectionTests.cpp:209
   )cpp",
-  "CXXConstructExpr",
+  "VarDecl",
   },

Maybe a FIXME for "CXXConstructExpr is better", but I don't come up with a good 
fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116630

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

[PATCH] D115429: [Clang] Implement the rest of __builtin_elementwise_* functions.

[Jun Zhang via Phabricator via cfe-commits]

junaire added a comment.

In D115429#3219563 , @fhahn wrote:

> LGTM, thanks!

May I ask you to help me to land this patch? I plan to request commit access 
after this one. Thanks a lot! ;D


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115429

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

[PATCH] D116535: [clang-tidy] Fix false positive in modernize-pass-by-value

[Clement Courbet via Phabricator via cfe-commits]

courbet marked 2 inline comments as done.
courbet added a comment.

Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116535

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

[PATCH] D116535: [clang-tidy] Fix false positive in modernize-pass-by-value

[Clement Courbet via Phabricator via cfe-commits]

courbet updated this revision to Diff 397543.
courbet added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116535

Files:
  clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp
@@ -246,3 +246,20 @@
 V::V(const Movable &M) : M(M) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: pass by value and use std::move
 // CHECK-FIXES: V::V(const Movable &M) : M(M) {}
+
+// Test with paired lvalue/rvalue overloads.
+struct W1 {
+  W1(const Movable &M) : M(M) {}
+  W1(Movable &&M);
+  Movable M;
+};
+struct W2 {
+  W2(const Movable &M, int) : M(M) {}
+  W2(Movable &&M, int);
+  Movable M;
+};
+struct W3 {
+  W3(const W1 &, const Movable &M) : M(M) {}
+  W3(W1 &&, Movable &&M);
+  Movable M;
+};
Index: clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
@@ -105,6 +105,75 @@
   return ExactlyOneUsageVisitor(ParamDecl).hasExactlyOneUsageIn(Ctor);
 }
 
+/// Returns true if the given constructor is part of a lvalue/rvalue reference
+/// pair, i.e. `Param` is of lvalue reference type, and there exists another
+/// constructor such that:
+///  - it has the same number of parameters as `Ctor`.
+///  - the parameter at the same index as `Param` is an rvalue reference
+///of the same pointee type
+///  - all other parameters have the same type as the corresponding parameter in
+///`Ctor` or are rvalue references with the same pointee type.
+/// Examples:
+///  A::A(const B& Param)
+///  A::A(B&&)
+///
+///  A::A(const B& Param, const C&)
+///  A::A(B&& Param, C&&)
+///
+///  A::A(const B&, const C& Param)
+///  A::A(B&&, C&& Param)
+///
+///  A::A(const B&, const C& Param)
+///  A::A(const B&, C&& Param)
+///
+///  A::A(const B& Param, int)
+///  A::A(B&& Param, int)
+static bool hasRValueOverload(const CXXConstructorDecl *Ctor,
+  const ParmVarDecl *Param) {
+  if (!Param->getType().getCanonicalType()->isLValueReferenceType()) {
+// The parameter is passed by value.
+return false;
+  }
+  const int ParamIdx = Param->getFunctionScopeIndex();
+  const CXXRecordDecl *Record = Ctor->getParent();
+
+  // Check whether a ctor `C` forms a pair with `Ctor` under the aforementionned
+  // rules.
+  const auto IsRValueOverload = [&Ctor, ParamIdx](const CXXConstructorDecl *C) {
+if (C == Ctor || C->isDeleted() ||
+C->getNumParams() != Ctor->getNumParams())
+  return false;
+for (int I = 0, E = C->getNumParams(); I < E; ++I) {
+  const clang::QualType CandidateParamType =
+  C->parameters()[I]->getType().getCanonicalType();
+  const clang::QualType CtorParamType =
+  Ctor->parameters()[I]->getType().getCanonicalType();
+  const bool IsLValueRValuePair =
+  CtorParamType->isLValueReferenceType() &&
+  CandidateParamType->isRValueReferenceType() &&
+  CandidateParamType->getPointeeType()->getUnqualifiedDesugaredType() ==
+  CtorParamType->getPointeeType()->getUnqualifiedDesugaredType();
+  if (I == ParamIdx) {
+// The parameter of interest must be paired.
+if (!IsLValueRValuePair)
+  return false;
+  } else {
+// All other parameters can be similar or paired.
+if (!(CandidateParamType == CtorParamType || IsLValueRValuePair))
+  return false;
+  }
+}
+return true;
+  };
+
+  for (const auto *Candidate : Record->ctors()) {
+if (IsRValueOverload(Candidate)) {
+  return true;
+}
+  }
+  return false;
+}
+
 /// Find all references to \p ParamDecl across all of the
 /// redeclarations of \p Ctor.
 static SmallVector
@@ -188,6 +257,10 @@
   *Result.Context))
 return;
 
+  // Do not trigger if we find a paired constructor with an rvalue.
+  if (hasRValueOverload(Ctor, ParamDecl))
+return;
+
   auto Diag = diag(ParamDecl->getBeginLoc(), "pass by value and use std::move");
 
   // If we received a `const&` type, we need to rewrite the function
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115060: [clang-format][NFC] Code Tidies in UnwrappedLineFormatter

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.

That's look good. And it's a nice clean up with all those repeated checks 
removed. Don't forget to reformat before landing.


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

https://reviews.llvm.org/D115060

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

[clang-tools-extra] ed8ff29 - [clang-tidy] Fix false positive in modernize-pass-by-value

[Clement Courbet via cfe-commits]

Author: Clement Courbet
Date: 2022-01-05T14:33:40+01:00
New Revision: ed8ff29aa6835187f3d5e7b64de022fe6b33a131

URL: 
https://github.com/llvm/llvm-project/commit/ed8ff29aa6835187f3d5e7b64de022fe6b33a131
DIFF: 
https://github.com/llvm/llvm-project/commit/ed8ff29aa6835187f3d5e7b64de022fe6b33a131.diff

LOG: [clang-tidy] Fix false positive in modernize-pass-by-value

The check should not trigger on lvalue/rvalue overload pairs:

```
struct S {
  S(const A& a) : a(a) {}
  S(A&& a) : a(std::move(a)) {}

  A a;
}
```

Differential Revision: https://reviews.llvm.org/D116535

Added: 


Modified: 
clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
index 34079c65b68cb..2e1f2143947e9 100644
--- a/clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
@@ -105,6 +105,75 @@ static bool paramReferredExactlyOnce(const 
CXXConstructorDecl *Ctor,
   return ExactlyOneUsageVisitor(ParamDecl).hasExactlyOneUsageIn(Ctor);
 }
 
+/// Returns true if the given constructor is part of a lvalue/rvalue reference
+/// pair, i.e. `Param` is of lvalue reference type, and there exists another
+/// constructor such that:
+///  - it has the same number of parameters as `Ctor`.
+///  - the parameter at the same index as `Param` is an rvalue reference
+///of the same pointee type
+///  - all other parameters have the same type as the corresponding parameter 
in
+///`Ctor` or are rvalue references with the same pointee type.
+/// Examples:
+///  A::A(const B& Param)
+///  A::A(B&&)
+///
+///  A::A(const B& Param, const C&)
+///  A::A(B&& Param, C&&)
+///
+///  A::A(const B&, const C& Param)
+///  A::A(B&&, C&& Param)
+///
+///  A::A(const B&, const C& Param)
+///  A::A(const B&, C&& Param)
+///
+///  A::A(const B& Param, int)
+///  A::A(B&& Param, int)
+static bool hasRValueOverload(const CXXConstructorDecl *Ctor,
+  const ParmVarDecl *Param) {
+  if (!Param->getType().getCanonicalType()->isLValueReferenceType()) {
+// The parameter is passed by value.
+return false;
+  }
+  const int ParamIdx = Param->getFunctionScopeIndex();
+  const CXXRecordDecl *Record = Ctor->getParent();
+
+  // Check whether a ctor `C` forms a pair with `Ctor` under the 
aforementionned
+  // rules.
+  const auto IsRValueOverload = [&Ctor, ParamIdx](const CXXConstructorDecl *C) 
{
+if (C == Ctor || C->isDeleted() ||
+C->getNumParams() != Ctor->getNumParams())
+  return false;
+for (int I = 0, E = C->getNumParams(); I < E; ++I) {
+  const clang::QualType CandidateParamType =
+  C->parameters()[I]->getType().getCanonicalType();
+  const clang::QualType CtorParamType =
+  Ctor->parameters()[I]->getType().getCanonicalType();
+  const bool IsLValueRValuePair =
+  CtorParamType->isLValueReferenceType() &&
+  CandidateParamType->isRValueReferenceType() &&
+  CandidateParamType->getPointeeType()->getUnqualifiedDesugaredType() 
==
+  CtorParamType->getPointeeType()->getUnqualifiedDesugaredType();
+  if (I == ParamIdx) {
+// The parameter of interest must be paired.
+if (!IsLValueRValuePair)
+  return false;
+  } else {
+// All other parameters can be similar or paired.
+if (!(CandidateParamType == CtorParamType || IsLValueRValuePair))
+  return false;
+  }
+}
+return true;
+  };
+
+  for (const auto *Candidate : Record->ctors()) {
+if (IsRValueOverload(Candidate)) {
+  return true;
+}
+  }
+  return false;
+}
+
 /// Find all references to \p ParamDecl across all of the
 /// redeclarations of \p Ctor.
 static SmallVector
@@ -188,6 +257,10 @@ void PassByValueCheck::check(const 
MatchFinder::MatchResult &Result) {
   *Result.Context))
 return;
 
+  // Do not trigger if we find a paired constructor with an rvalue.
+  if (hasRValueOverload(Ctor, ParamDecl))
+return;
+
   auto Diag = diag(ParamDecl->getBeginLoc(), "pass by value and use 
std::move");
 
   // If we received a `const&` type, we need to rewrite the function

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp
index 28e4014c43d36..2aacbdd1c7a6a 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp
@@ -246,3 +246,20 @@ struct V {
 V::V(const Movable &M) : M(M) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: pass by value and use std::move
 // CHECK-FIXES: V::V(const Movable &M) : M(M) {}
+
+// Test wi

[PATCH] D116535: [clang-tidy] Fix false positive in modernize-pass-by-value

[Clement Courbet 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 rGed8ff29aa683: [clang-tidy] Fix false positive in 
modernize-pass-by-value (authored by courbet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116535

Files:
  clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp
@@ -246,3 +246,20 @@
 V::V(const Movable &M) : M(M) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: pass by value and use std::move
 // CHECK-FIXES: V::V(const Movable &M) : M(M) {}
+
+// Test with paired lvalue/rvalue overloads.
+struct W1 {
+  W1(const Movable &M) : M(M) {}
+  W1(Movable &&M);
+  Movable M;
+};
+struct W2 {
+  W2(const Movable &M, int) : M(M) {}
+  W2(Movable &&M, int);
+  Movable M;
+};
+struct W3 {
+  W3(const W1 &, const Movable &M) : M(M) {}
+  W3(W1 &&, Movable &&M);
+  Movable M;
+};
Index: clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
@@ -105,6 +105,75 @@
   return ExactlyOneUsageVisitor(ParamDecl).hasExactlyOneUsageIn(Ctor);
 }
 
+/// Returns true if the given constructor is part of a lvalue/rvalue reference
+/// pair, i.e. `Param` is of lvalue reference type, and there exists another
+/// constructor such that:
+///  - it has the same number of parameters as `Ctor`.
+///  - the parameter at the same index as `Param` is an rvalue reference
+///of the same pointee type
+///  - all other parameters have the same type as the corresponding parameter in
+///`Ctor` or are rvalue references with the same pointee type.
+/// Examples:
+///  A::A(const B& Param)
+///  A::A(B&&)
+///
+///  A::A(const B& Param, const C&)
+///  A::A(B&& Param, C&&)
+///
+///  A::A(const B&, const C& Param)
+///  A::A(B&&, C&& Param)
+///
+///  A::A(const B&, const C& Param)
+///  A::A(const B&, C&& Param)
+///
+///  A::A(const B& Param, int)
+///  A::A(B&& Param, int)
+static bool hasRValueOverload(const CXXConstructorDecl *Ctor,
+  const ParmVarDecl *Param) {
+  if (!Param->getType().getCanonicalType()->isLValueReferenceType()) {
+// The parameter is passed by value.
+return false;
+  }
+  const int ParamIdx = Param->getFunctionScopeIndex();
+  const CXXRecordDecl *Record = Ctor->getParent();
+
+  // Check whether a ctor `C` forms a pair with `Ctor` under the aforementionned
+  // rules.
+  const auto IsRValueOverload = [&Ctor, ParamIdx](const CXXConstructorDecl *C) {
+if (C == Ctor || C->isDeleted() ||
+C->getNumParams() != Ctor->getNumParams())
+  return false;
+for (int I = 0, E = C->getNumParams(); I < E; ++I) {
+  const clang::QualType CandidateParamType =
+  C->parameters()[I]->getType().getCanonicalType();
+  const clang::QualType CtorParamType =
+  Ctor->parameters()[I]->getType().getCanonicalType();
+  const bool IsLValueRValuePair =
+  CtorParamType->isLValueReferenceType() &&
+  CandidateParamType->isRValueReferenceType() &&
+  CandidateParamType->getPointeeType()->getUnqualifiedDesugaredType() ==
+  CtorParamType->getPointeeType()->getUnqualifiedDesugaredType();
+  if (I == ParamIdx) {
+// The parameter of interest must be paired.
+if (!IsLValueRValuePair)
+  return false;
+  } else {
+// All other parameters can be similar or paired.
+if (!(CandidateParamType == CtorParamType || IsLValueRValuePair))
+  return false;
+  }
+}
+return true;
+  };
+
+  for (const auto *Candidate : Record->ctors()) {
+if (IsRValueOverload(Candidate)) {
+  return true;
+}
+  }
+  return false;
+}
+
 /// Find all references to \p ParamDecl across all of the
 /// redeclarations of \p Ctor.
 static SmallVector
@@ -188,6 +257,10 @@
   *Result.Context))
 return;
 
+  // Do not trigger if we find a paired constructor with an rvalue.
+  if (hasRValueOverload(Ctor, ParamDecl))
+return;
+
   auto Diag = diag(ParamDecl->getBeginLoc(), "pass by value and use std::move");
 
   // If we received a `const&` type, we need to rewrite the function
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116351: Update Bug report URL to Github Issues

[James Henderson via Phabricator via cfe-commits]

jhenderson added inline comments.



Comment at: llvm/docs/CommandGuide/llvm-install-name-tool.rst:79
 
-To report bugs, please visit .
+To report bugs, please visit .
 

I believe llvm-install-name-tool is a variation of llvm-objcopy, so should 
probably reference the llvm-objcopy URL.



Comment at: llvm/docs/CommandGuide/llvm-otool.rst:135
 
-To report bugs, please visit .
+To report bugs, please visit .
 

I believe llvm-otool is a variation of llvm-objdump, so should reference the 
llvm-objdump URL.


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

https://reviews.llvm.org/D116351

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

[clang] 32c2ea5 - [clang][lex] NFC: Simplify loop

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2022-01-05T14:40:47+01:00
New Revision: 32c2ea5c33a710cb1497bbd903039d8a9641e98a

URL: 
https://github.com/llvm/llvm-project/commit/32c2ea5c33a710cb1497bbd903039d8a9641e98a
DIFF: 
https://github.com/llvm/llvm-project/commit/32c2ea5c33a710cb1497bbd903039d8a9641e98a.diff

LOG: [clang][lex] NFC: Simplify loop

Added: 


Modified: 
clang/lib/Lex/HeaderSearch.cpp

Removed: 




diff  --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index a0b60118a1a81..5cb009dd68d1d 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -29,6 +29,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Capacity.h"
 #include "llvm/Support/Errc.h"
@@ -1779,11 +1780,8 @@ void 
HeaderSearch::collectAllModules(SmallVectorImpl &Modules) {
   }
 
   // Populate the list of modules.
-  for (ModuleMap::module_iterator M = ModMap.module_begin(),
-   MEnd = ModMap.module_end();
-   M != MEnd; ++M) {
-Modules.push_back(M->getValue());
-  }
+  llvm::transform(ModMap.modules(), std::back_inserter(Modules),
+  [](const auto &NameAndMod) { return NameAndMod.second; });
 }
 
 void HeaderSearch::loadTopLevelSystemModules() {



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

[PATCH] D116659: [llvm][clang][vfs] NFC: Extract directory iteration boilerplate into macro

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith, arphaman.
Herald added subscribers: kerbowa, nhaehnle, jvesely.
jansvoboda11 requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

The iteration over directory entries of a VFS is a bit unweildy, since it 
requires using a pair of `llvm::vfs::directory_iterator`, calling 
`llvm::vfs::directory_iterator::increment(std::error_code &)` and checking 
`std::error_code`. Currently, there are 13 instances of this pattern in the 
Clang codebase.

This patch simplifies iteration over directory entries by extracting the 
boilerplate into a macro.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116659

Files:
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/Cuda.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Lex/ModuleMap.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  llvm/include/llvm/Support/VirtualFileSystem.h

Index: llvm/include/llvm/Support/VirtualFileSystem.h
===
--- llvm/include/llvm/Support/VirtualFileSystem.h
+++ llvm/include/llvm/Support/VirtualFileSystem.h
@@ -319,6 +319,13 @@
 /// that of the process.
 std::unique_ptr createPhysicalFileSystem();
 
+/// Expands to a loop header that uses iterator named IT for iterating over
+/// entries of the directory DIR within the VFS.
+#define LLVM_VFS_DIR_FOREACH(VFS, DIR, IT) \
+  std::error_code EC;  \
+  for (llvm::vfs::directory_iterator IT = VFS.dir_begin(DIR, EC), End; \
+   !EC && IT != End; IT = IT.increment(EC))
+
 /// A file system that allows overlaying one \p AbstractFileSystem on top
 /// of another.
 ///
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -9888,10 +9888,8 @@
 const bool isQt = Dirname.startswith("Qt") || Dirname == "ActiveQt";
 const bool ExtensionlessHeaders =
 IsSystem || isQt || Dir.endswith(".framework/Headers");
-std::error_code EC;
 unsigned Count = 0;
-for (auto It = FS.dir_begin(Dir, EC);
- !EC && It != llvm::vfs::directory_iterator(); It.increment(EC)) {
+LLVM_VFS_DIR_FOREACH(FS, Dir, It) {
   if (++Count == 2500) // If we happen to hit a huge directory,
 break; // bail out early so we're not too slow.
   StringRef Filename = llvm::sys::path::filename(It->path());
Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -1057,16 +1057,12 @@
   Result->InferExportWildcard = true;
 
   // Look for subframeworks.
-  std::error_code EC;
   SmallString<128> SubframeworksDirName
 = StringRef(FrameworkDir->getName());
   llvm::sys::path::append(SubframeworksDirName, "Frameworks");
   llvm::sys::path::native(SubframeworksDirName);
   llvm::vfs::FileSystem &FS = FileMgr.getVirtualFileSystem();
-  for (llvm::vfs::directory_iterator
-   Dir = FS.dir_begin(SubframeworksDirName, EC),
-   DirEnd;
-   Dir != DirEnd && !EC; Dir.increment(EC)) {
+  LLVM_VFS_DIR_FOREACH(FS, SubframeworksDirName, Dir) {
 if (!StringRef(Dir->path()).endswith(".framework"))
   continue;
 
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1740,16 +1740,13 @@
 for (unsigned Idx = 0, N = SearchDirs.size(); Idx != N; ++Idx) {
   bool IsSystem = SearchDirs[Idx].isSystemHeaderDirectory();
   if (SearchDirs[Idx].isFramework()) {
-std::error_code EC;
 SmallString<128> DirNative;
 llvm::sys::path::native(SearchDirs[Idx].getFrameworkDir()->getName(),
 DirNative);
 
 // Search each of the ".framework" directories to load them as modules.
 llvm::vfs::FileSystem &FS = FileMgr.getVirtualFileSystem();
-for (llvm::vfs::directory_iterator Dir = FS.dir_begin(DirNative, EC),
-   DirEnd;
- Dir != DirEnd && !EC; Dir.increment(EC)) {
+LLVM_VFS_DIR_FOREACH(FS, DirNative, Dir) {
   if (llvm::sys::path::extension(Dir->path()) != ".framework")
 continue;
 
@@ -1809,14 +1806,12 @@
   if (SearchDir.haveSearchedAllModuleMaps())
 return;
 
-  std::error_code EC;
   SmallString<128> Dir = SearchDir.getDir()->getName();
   FileMgr.makeAbsolutePath(Dir);
   SmallString<128> DirNative;
   

[PATCH] D116550: [clang-tidy] Recognize transformer checks as providing fixits

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116550

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

[PATCH] D116596: [clang][dataflow] Add transfer functions for assignment

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:36
+/// FIXME: Consider replacing this with a model that is more aligned with C++
+/// value categories.
+enum class SkipPast {

I'm not sure that value categories are right here.  The key is that skipping is 
optional, unlike value categories which are fixed.  I think the problem is just 
that C++ references are weird (for lack of a technical term). A reference 
variable exists simultaneously in two different categories -- a pointer and the 
value it points to. That's what this is trying to capture.

So, I'd recommend just changing the name to reflect that. WDYT of something 
like this?

```
/// Indicates how to treat references, if encountered -- as a reference or the 
value referred to -- when retrieving
/// storage locations or values.
enum class RefTreatment {
   /// Consider the reference itself.
  AsSelf,
  /// Consider the referent.
  AsReferent,
};
```



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:111
+  /// value in the environment.
+  Value *getValue(const StorageLocation &Loc, SkipPast SP) const;
+

Why have a SkipPast argument here? I understand the concept for considering 
var-decls - it corresponds to the weirdness of lvalue-refs themselves. But 
StorageLocation is a lower-level (and simpler) concept, to which this doesn't 
seem to map. I'm fine with resolving indirections, but what is the value of 
only doing so conditionally?



Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:242-247
+if (auto *Val =
+dyn_cast_or_null(getValue(Loc, SkipPast::None))) {
+  return Val->getPointeeLoc();
+} else {
+  return Loc;
+}





Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:37
+  assert(S->getLHS() != nullptr);
+  const Expr *LHS = S->getLHS()->IgnoreParens();
+  assert(LHS != nullptr);

maybe comment as to why this is necessary? I'd have thought that CFG's 
flattening of expressions would handle this (why, in general, we don't need to 
look deep into expressions).



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:135
+
+if (const Expr *InitExpr = D.getInit()) {
+  if (D.getType()->isReferenceType()) {

nit: might be easier to read if you invert this:
```
const Expr *InitExpr = D.getInit();
if (InitExpr == nullptr) return;
```



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:144-145
+}
+  } else {
+if (auto *InitExprVal = Env.getValue(*InitExpr, SkipPast::None)) {
+  Env.setValue(Loc, *InitExprVal);

nit: else-if (one line)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116596

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

[PATCH] D116478: [clang-tidy] A comma-separated list of the names of functions or methods to be considered as not having side-effects

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.h:37-48
+  using FunctionExceptionType = llvm::SmallSet;
   AssertSideEffectCheck(StringRef Name, ClangTidyContext *Context);
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
 private:

Instead of using "Exception" here, can we rename slightly? Functions can have 
lists of exceptions (dynamic exception specifications are a thing in C++), so 
the use of "exception" can be somewhat confusing, but I think something like 
`IgnoredFunctionList` and `IgnoredFunctions` would be an improvement.


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

https://reviews.llvm.org/D116478

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

[PATCH] D113676: WIP: [clang][lex] Fix search path usage remark with modules

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

In D113676#3168219 , @Bigcheese wrote:

> Can we not consider a modulemap used when we load an AST that depends on that 
> modulemap? It's possible this breaks if the module only includes textual 
> headers though.

I don't think that would be 100% correct. I think it's possible for `Module` 
instances (created by parsing a modulemap file) to have semantic meaning even 
without their AST being loaded. One example (that I just made up but sounds 
plausible enough to me) is that we could have a fixit for an `@import` of 
non-existent module that suggests importing another available module 
(discovered through header search) with the most similar name. We don't need 
the AST file for that, but removing search paths could change the diagnostic 
output.

> It really feels like we should have a single place where we actually know if 
> a module is used or not. Long term I would really like to separate modulemap 
> parsing from `Module` creation, which would be a great place to actually do 
> this.

Agreed. But I'm afraid that's a big undertaking. Do you have any other ideas on 
how to detect module usage with the current state of things?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113676

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

[PATCH] D114787: [clang][PR51931] Enable `-Wdeclaration-after-statement` for all C versions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D114787#3218382 , @zero9178 wrote:

> In D114787#3188735 , @aaron.ballman 
> wrote:
>
>> https://reviews.llvm.org/D115094 is a review for doing effectively the same 
>> fix. Can you coordinate with the other patch author to determine who will 
>> drive this fix?
>
> I sent an email to the author on December 13th but have yet to hear back from 
> them. Should we move forward with this version of the patch then?

I'd like to give a little bit more time (and another ping) only because of the 
recent holiday season, but I think we don't need to wait too much longer. I'm 
still digging out from under my pile of reviews, so I'd not be surprised if 
others are in a similar boat.

How about we proceed with this patch if there's no response by Mon Jan 10?

In the meantime, I have some minor feedback.




Comment at: clang/test/Sema/warn-mixed-decls.c:1-4
+/* RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -pedantic %s
+ */
+/* RUN: %clang_cc1 -fsyntax-only -verify -std=c99 
-Wdeclaration-after-statement %s
+ */

I'd also like to see RUN lines for when we expect the diagnostic to not be 
enabled:
```
/* RUN: %clang_cc1 -fsyntax-only -verify=none -std=c99 %s */
/* RUN: %clang_cc1 -fsyntax-only -verify=none -x c++ %s */
/* RUN: %clang_cc1 -fsyntax-only -verify=none -x c++ 
-Wdeclaration-after-statement %s */

/* none-no-diagnostics */
```
I should note that the last RUN line will give different behavior between Clang 
and GCC: https://godbolt.org/z/o1PKo7dhM, but I think that's a more general 
issue that doesn't need to be addressed in this patch. (We don't have a way to 
flag a diagnostic as requiring a particular language mode.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114787

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

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat/CreateShuffleVector use poison as placeholder

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune added a comment.

In D93793#3221414 , @Allen wrote:

> I have a babyism question, why poison is preferred to the undef in the 
> pattern ConstantVector::getSplat ?

Hi Allen,
It is because folding poison is easier than folding undef. :)
For example, according to the poison propagating rule, 'and i32 poison, 1' can 
be simplified into 'i32 poison'. However, 'and undef, 1' cannot be folded into 
'undef' since the mask enforces all bits but LSB 0. 
This patch helps simplifications happen more easily.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93793

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

[PATCH] D116630: [clangd] Handle declarators more consistently in Selection.

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 3 inline comments as done.
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/Selection.cpp:757
 if (const auto *TL = N.get()) {
-  // e.g. EltType Foo[OuterSize][InnerSize];
-  //  ~ ArrayTypeLoc (Outer)

hokein wrote:
> I'd prefer to keep this comment, while we have some high-level and abstract 
> comment in the new patch, I think it is still useful to give us some details 
> (otherwise, I'd probably forget everything when coming back to read the code 
> a few months later).
> 
> 
> I also want to add the example `int (*Fun(OuterType))(InnerType);` from 
> D116618 here, I understand it might be too verbose (personally, I find it 
> useful to understand this part of code), up to you.
Yeah, the example is useful.
My problem was that the examples were showing how each declarator type combined 
with itself, which was both confusing (as you have to describe which you're 
talking about) and didn't cover the general case.

I've added a new example, which uses multiple different types of declarators 
and also shows nontrivial non-declarator types for comparison.



Comment at: clang-tools-extra/clangd/unittests/SelectionTests.cpp:209
   )cpp",
-  "CXXConstructExpr",
+  "VarDecl",
   },

hokein wrote:
> Maybe a FIXME for "CXXConstructExpr is better", but I don't come up with a 
> good fix.
I actually like the way we treat CXXConstructExpr, which is that it owns the 
brackets and nothing else.
And having VarDecl consistently own the = is nice too.

This is a divergence from the AST so I've left a comment, but I think it's 
actually good.

(As mentioned offline: the CXXConstructExpr in this case isn't actually the 
interesting constructor, it's the elidable move construction, so this doesn't 
really hurt go-to-definition etc)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116630

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

[clang-tools-extra] 96f5cc1 - [clangd] Handle declarators more consistently in Selection.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-01-05T15:40:47+01:00
New Revision: 96f5cc1ee417f863f85756d1e56b1bed1bd76a7e

URL: 
https://github.com/llvm/llvm-project/commit/96f5cc1ee417f863f85756d1e56b1bed1bd76a7e
DIFF: 
https://github.com/llvm/llvm-project/commit/96f5cc1ee417f863f85756d1e56b1bed1bd76a7e.diff

LOG: [clangd] Handle declarators more consistently in Selection.

Because declarators nest inside-out, we logically need to claim tokens for
parent declarators logically before child ones.
This is the ultimate reason we had problems with DeclaratorDecl, ArrayType etc.

However actually changing the order of traversal is hard, especially for nodes
that have both declarator and non-declarator children.
Since there's only a few TypeLocs corresponding to declarators, we just
have them claim the exact tokens rather than rely on nesting.

This fixes handling of complex declarators, like
`int (*Fun(OuterT^ype))(InnerType);`.

This avoids the need for the DeclaratorDecl early-claim hack, which is
removed.
Unfortunately the DeclaratorDecl early-claims were covering up an AST
anomaly around CXXConstructExpr, so we need to fix that up too.

Based on D116623 and D116618

Differential Revision: https://reviews.llvm.org/D116630

Added: 


Modified: 
clang-tools-extra/clangd/Selection.cpp
clang-tools-extra/clangd/unittests/SelectionTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Selection.cpp 
b/clang-tools-extra/clangd/Selection.cpp
index 40021c62d9e2e..c7a3aacf0f1e8 100644
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -639,11 +639,10 @@ class SelectionVisitor : public 
RecursiveASTVisitor {
   // Nodes *usually* nest nicely: a child's getSourceRange() lies within the
   // parent's, and a node (transitively) owns all tokens in its range.
   //
-  // Exception 1: child range claims tokens that should be owned by the parent.
-  //  e.g. in `void foo(int);`, the FunctionTypeLoc should own
-  //  `void (int)` but the parent FunctionDecl should own `foo`.
-  // To handle this case, certain nodes claim small token ranges *before*
-  // their children are traversed. (see earlySourceRange).
+  // Exception 1: when declarators nest, *inner* declarator is the *outer* 
type.
+  //  e.g. void foo[5](int) is an array of functions.
+  // To handle this case, declarators are careful to only claim the tokens they
+  // own, rather than claim a range and rely on claim ordering.
   //
   // Exception 2: siblings both claim the same node.
   //  e.g. `int x, y;` produces two sibling VarDecls.
@@ -690,16 +689,13 @@ class SelectionVisitor : public 
RecursiveASTVisitor {
   }
 
   // Pushes a node onto the ancestor stack. Pairs with pop().
-  // Performs early hit detection for some nodes (on the earlySourceRange).
   void push(DynTypedNode Node) {
-SourceRange Early = earlySourceRange(Node);
 dlog("{1}push: {0}", printNodeToString(Node, PrintPolicy), indent());
 Nodes.emplace_back();
 Nodes.back().ASTNode = std::move(Node);
 Nodes.back().Parent = Stack.top();
 Nodes.back().Selected = NoTokens;
 Stack.push(&Nodes.back());
-claimRange(Early, Nodes.back().Selected);
   }
 
   // Pops a node off the ancestor stack, and finalizes it. Pairs with push().
@@ -721,52 +717,65 @@ class SelectionVisitor : public 
RecursiveASTVisitor {
 Stack.pop();
   }
 
-  // Returns the range of tokens that this node will claim directly, and
-  // is not available to the node's children.
-  // Usually empty, but sometimes children cover tokens but shouldn't own them.
-  SourceRange earlySourceRange(const DynTypedNode &N) {
-if (const Decl *D = N.get()) {
-  // We want constructor name to be claimed by TypeLoc not the constructor
-  // itself. Similar for deduction guides, we rather want to select the
-  // underlying TypeLoc.
-  // FIXME: Unfortunately this doesn't work, even though 
RecursiveASTVisitor
-  // traverses the underlying TypeLoc inside DeclarationName, it is null 
for
-  // constructors.
-  if (isa(D) || isa(D))
-return SourceRange();
-  // This will capture Field, Function, MSProperty, NonTypeTemplateParm and
-  // VarDecls. We want the name in the declarator to be claimed by the decl
-  // and not by any children. For example:
-  // void [[foo]]();
-  // int (*[[s]])();
-  // struct X { int [[hash]] [32]; [[operator]] int();}
-  if (const auto *DD = llvm::dyn_cast(D))
-return DD->getLocation();
-} else if (const auto *CCI = N.get()) {
-  // : [[b_]](42)
-  return CCI->getMemberLocation();
-}
-return SourceRange();
-  }
-
   // Claim tokens for N, after processing its children.
   // By default this claims all unclaimed tokens in getSourceRange().
   // We override this if we want to claim fewer tokens (e.g. there are gaps).
  

[PATCH] D116630: [clangd] Handle declarators more consistently in Selection.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
sammccall marked 2 inline comments as done.
Closed by commit rG96f5cc1ee417: [clangd] Handle declarators more consistently 
in Selection. (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D116630?vs=397416&id=397555#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116630

Files:
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/unittests/SelectionTests.cpp

Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -204,9 +204,12 @@
   {
   R"cpp(
 struct S { S(const char*); };
-S [[s ^= "foo"]];
+[[S s ^= "foo"]];
   )cpp",
-  "CXXConstructExpr",
+  // The AST says a CXXConstructExpr covers the = sign in C++14.
+  // But we consider CXXConstructExpr to only own brackets.
+  // (It's not the interesting constructor anyway, just S(&&)).
+  "VarDecl",
   },
   {
   R"cpp(
@@ -231,7 +234,7 @@
   R"cpp(
 [[void (^*S)(int)]] = nullptr;
   )cpp",
-  "FunctionProtoTypeLoc",
+  "PointerTypeLoc",
   },
   {
   R"cpp(
@@ -243,7 +246,7 @@
   R"cpp(
 [[void ^(*S)(int)]] = nullptr;
   )cpp",
-  "FunctionProtoTypeLoc",
+  "ParenTypeLoc",
   },
   {
   R"cpp(
@@ -326,6 +329,8 @@
   {"const int x = 1, y = 2; [[i^nt]] array[x][10][y];", "BuiltinTypeLoc"},
   {"void func(int x) { int v_array[^[[x]]][10]; }", "DeclRefExpr"},
 
+  {"int (*getFunc([[do^uble]]))(int);", "BuiltinTypeLoc"},
+
   // FIXME: the AST has no location info for qualifiers.
   {"const [[a^uto]] x = 42;", "AutoTypeLoc"},
   {"[[co^nst auto x = 42]];", "VarDecl"},
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -639,11 +639,10 @@
   // Nodes *usually* nest nicely: a child's getSourceRange() lies within the
   // parent's, and a node (transitively) owns all tokens in its range.
   //
-  // Exception 1: child range claims tokens that should be owned by the parent.
-  //  e.g. in `void foo(int);`, the FunctionTypeLoc should own
-  //  `void (int)` but the parent FunctionDecl should own `foo`.
-  // To handle this case, certain nodes claim small token ranges *before*
-  // their children are traversed. (see earlySourceRange).
+  // Exception 1: when declarators nest, *inner* declarator is the *outer* type.
+  //  e.g. void foo[5](int) is an array of functions.
+  // To handle this case, declarators are careful to only claim the tokens they
+  // own, rather than claim a range and rely on claim ordering.
   //
   // Exception 2: siblings both claim the same node.
   //  e.g. `int x, y;` produces two sibling VarDecls.
@@ -690,16 +689,13 @@
   }
 
   // Pushes a node onto the ancestor stack. Pairs with pop().
-  // Performs early hit detection for some nodes (on the earlySourceRange).
   void push(DynTypedNode Node) {
-SourceRange Early = earlySourceRange(Node);
 dlog("{1}push: {0}", printNodeToString(Node, PrintPolicy), indent());
 Nodes.emplace_back();
 Nodes.back().ASTNode = std::move(Node);
 Nodes.back().Parent = Stack.top();
 Nodes.back().Selected = NoTokens;
 Stack.push(&Nodes.back());
-claimRange(Early, Nodes.back().Selected);
   }
 
   // Pops a node off the ancestor stack, and finalizes it. Pairs with push().
@@ -721,52 +717,65 @@
 Stack.pop();
   }
 
-  // Returns the range of tokens that this node will claim directly, and
-  // is not available to the node's children.
-  // Usually empty, but sometimes children cover tokens but shouldn't own them.
-  SourceRange earlySourceRange(const DynTypedNode &N) {
-if (const Decl *D = N.get()) {
-  // We want constructor name to be claimed by TypeLoc not the constructor
-  // itself. Similar for deduction guides, we rather want to select the
-  // underlying TypeLoc.
-  // FIXME: Unfortunately this doesn't work, even though RecursiveASTVisitor
-  // traverses the underlying TypeLoc inside DeclarationName, it is null for
-  // constructors.
-  if (isa(D) || isa(D))
-return SourceRange();
-  // This will capture Field, Function, MSProperty, NonTypeTemplateParm and
-  // VarDecls. We want the name in the declarator to be claimed by the decl
-  // and not by any children. For example:
-  // void [[foo]]();
-  // int (*[[s]])();
-  // struct X { int [[hash]] [32]; [[operator]] int

[PATCH] D116512: [clang-tidy] Limit non-Strict mode to public functions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman requested changes to this revision.
aaron.ballman added inline comments.
This revision now requires changes to proceed.



Comment at: clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp:188
 // (constructor initializer counts for non-empty body).
-if (StrictMode ||
+if (StrictMode || !Function->isExternallyVisible() ||
 (Function->getBody()->child_begin() !=

This is looking at the linkage of the function, not at its access control; is 
that intended?



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/misc-unused-parameters.rst:43
+   a human reader, and there's basically no place for a bug to hide. On the 
other
+   hand for non-public functions, all the call-sites are visible and the 
parameter
+   can be eliminated entirely.

Call sites are not always visible for protected functions, so this seems a bit 
suspicious. The changes are missing test coverage for that situation.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp:157-159
+// CHECK-FIXES: C() {}
+  C(int i) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning

I think this demonstrates a bad fix -- this changes code meaning from being a 
converting constructor to being a default constructor, which are very different 
things.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116512

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

[clang-tools-extra] 7632d19 - [clangd] Fix typos in the SelectionTree comment.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2022-01-05T15:50:07+01:00
New Revision: 7632d19ada4a1feebc4a06067490ee1c77cd4dc1

URL: 
https://github.com/llvm/llvm-project/commit/7632d19ada4a1feebc4a06067490ee1c77cd4dc1
DIFF: 
https://github.com/llvm/llvm-project/commit/7632d19ada4a1feebc4a06067490ee1c77cd4dc1.diff

LOG: [clangd] Fix typos in the SelectionTree comment.

Added: 


Modified: 
clang-tools-extra/clangd/Selection.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Selection.cpp 
b/clang-tools-extra/clangd/Selection.cpp
index c7a3aacf0f1e..ef964da7b28d 100644
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -742,7 +742,7 @@ class SelectionVisitor : public 
RecursiveASTVisitor {
 //
 // Example:
 //   Vec(*[2])(A)> is a Vec of arrays of pointers to 
functions,
-//  which accept A and return R.
+//  which accept A and return R.
 // The TypeLoc hierarchy:
 //   Vec(*[2])(A)> m;
 //   Vec<->  TemplateSpecialization Vec
@@ -753,7 +753,7 @@ class SelectionVisitor : public 
RecursiveASTVisitor {
 //   R<--->  |-TemplateSpecialization R
 // int   | `-Builtin int
 //A<>`-TemplateSpecialization A
-//  char   `-Builtin int
+//  char   `-Builtin char
 //
 // In each row, --- represents unclaimed parts of the SourceRange.
 // For declarator types, we are careful never to claim these.



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

[PATCH] D116513: [clang-tidy] Fix bugs in misc-unused-parameters for Constructors calls site

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp:157-159
 // CHECK-FIXES: C() {}
   C(int i) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:9: warning

I think this fix is incorrect and should not be applied; it changes the meaning 
of the interface from having a converting constructor to having a default 
constructor. I think that needs to be optional behavior because it's a pretty 
invasive change to apply automatically. This patch builds on top of the 
existing incorrect behavior, but would be fine if it was only applied when the 
option to modify constructors is enabled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116513

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

[clang-tools-extra] bb10e03 - [clangd] Refine comment on declarator ranges

[via cfe-commits]

Author: Sam McCall
Date: 2022-01-05T16:00:13+01:00
New Revision: bb10e03fba7106e51d2e64269d10c3ba95055b26

URL: 
https://github.com/llvm/llvm-project/commit/bb10e03fba7106e51d2e64269d10c3ba95055b26
DIFF: 
https://github.com/llvm/llvm-project/commit/bb10e03fba7106e51d2e64269d10c3ba95055b26.diff

LOG: [clangd] Refine comment on declarator ranges

Added: 


Modified: 
clang-tools-extra/clangd/Selection.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Selection.cpp 
b/clang-tools-extra/clangd/Selection.cpp
index ef964da7b28d..7dc8a868ea00 100644
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -745,19 +745,19 @@ class SelectionVisitor : public 
RecursiveASTVisitor {
 //  which accept A and return R.
 // The TypeLoc hierarchy:
 //   Vec(*[2])(A)> m;
-//   Vec<->  TemplateSpecialization Vec
+//   Vec<#>  TemplateSpecialization Vec
 //   [2]--   `-Array
 //   ---*- `-Pointer
 //   --()-   `-Paren
-//   (---) `-Function
-//   R<--->  |-TemplateSpecialization R
+//   (###) `-Function
+//   R<###>  |-TemplateSpecialization R
 // int   | `-Builtin int
-//A<>`-TemplateSpecialization A
+//A<>`-TemplateSpecialization A
 //  char   `-Builtin char
 //
-// In each row, --- represents unclaimed parts of the SourceRange.
-// For declarator types, we are careful never to claim these.
-// For non-declarator types, children are guaranteed to claim them first.
+// In each row
+//   --- represents unclaimed parts of the SourceRange.
+//   ### represents parts that children already claimed.
 if (const auto *TL = N.get()) {
   if (auto PTL = TL->getAs()) {
 claimRange(PTL.getLParenLoc(), Result);



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

[PATCH] D116615: [Clang] Extract availability mapping from VersionMap for watchOS/tvOS

[Egor Zhdan via Phabricator via cfe-commits]

egorzhdan updated this revision to Diff 397565.
egorzhdan added a comment.

Fix lint warning


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116615

Files:
  clang/lib/Basic/DarwinSDKInfo.cpp
  clang/unittests/Basic/DarwinSDKInfoTest.cpp

Index: clang/unittests/Basic/DarwinSDKInfoTest.cpp
===
--- clang/unittests/Basic/DarwinSDKInfoTest.cpp
+++ clang/unittests/Basic/DarwinSDKInfoTest.cpp
@@ -13,7 +13,68 @@
 using namespace llvm;
 using namespace clang;
 
-TEST(DarwinSDKInfoTest, ParseAndTestMapping) {
+// Check the version mapping logic in DarwinSDKInfo.
+TEST(DarwinSDKInfo, VersionMapping) {
+  llvm::json::Object Obj({{"3.0", "1.0"}, {"3.1", "1.2"}});
+  Optional Mapping =
+  DarwinSDKInfo::RelatedTargetVersionMapping::parseJSON(Obj,
+VersionTuple());
+  EXPECT_TRUE(Mapping.hasValue());
+  EXPECT_EQ(Mapping->getMinimumValue(), VersionTuple(1));
+
+  // Exact mapping.
+  EXPECT_EQ(Mapping->map(VersionTuple(3), VersionTuple(0, 1), None),
+VersionTuple(1));
+  EXPECT_EQ(Mapping->map(VersionTuple(3, 0), VersionTuple(0, 1), None),
+VersionTuple(1));
+  EXPECT_EQ(Mapping->map(VersionTuple(3, 0, 0), VersionTuple(0, 1), None),
+VersionTuple(1));
+  EXPECT_EQ(Mapping->map(VersionTuple(3, 1), VersionTuple(0, 1), None),
+VersionTuple(1, 2));
+  EXPECT_EQ(Mapping->map(VersionTuple(3, 1, 0), VersionTuple(0, 1), None),
+VersionTuple(1, 2));
+
+  // Missing mapping - fallback to major.
+  EXPECT_EQ(Mapping->map(VersionTuple(3, 0, 1), VersionTuple(0, 1), None),
+VersionTuple(1));
+
+  // Minimum
+  EXPECT_EQ(Mapping->map(VersionTuple(2), VersionTuple(0, 1), None),
+VersionTuple(0, 1));
+
+  // Maximum
+  EXPECT_EQ(
+  Mapping->map(VersionTuple(4), VersionTuple(0, 1), VersionTuple(100)),
+  VersionTuple(100));
+}
+
+// Check the version mapping logic in DarwinSDKInfo.
+TEST(DarwinSDKInfo, VersionMappingMissingKey) {
+  llvm::json::Object Obj({{"3.0", "1.0"}, {"5.0", "1.2"}});
+  Optional Mapping =
+  DarwinSDKInfo::RelatedTargetVersionMapping::parseJSON(Obj,
+VersionTuple());
+  EXPECT_TRUE(Mapping.hasValue());
+  EXPECT_EQ(
+  Mapping->map(VersionTuple(4), VersionTuple(0, 1), VersionTuple(100)),
+  None);
+}
+
+TEST(DarwinSDKInfo, VersionMappingParseEmpty) {
+  llvm::json::Object Obj({});
+  EXPECT_FALSE(
+  DarwinSDKInfo::RelatedTargetVersionMapping::parseJSON(Obj, VersionTuple())
+  .hasValue());
+}
+
+TEST(DarwinSDKInfo, VersionMappingParseError) {
+  llvm::json::Object Obj({{"test", "1.2"}});
+  EXPECT_FALSE(
+  DarwinSDKInfo::RelatedTargetVersionMapping::parseJSON(Obj, VersionTuple())
+  .hasValue());
+}
+
+TEST(DarwinSDKInfoTest, ParseAndTestMappingMacCatalyst) {
   llvm::json::Object Obj;
   Obj["Version"] = "11.0";
   Obj["MaximumDeploymentTarget"] = "11.99";
@@ -58,6 +119,51 @@
 VersionTuple(99, 99));
 }
 
+TEST(DarwinSDKInfoTest, ParseAndTestMappingIOSDerived) {
+  llvm::json::Object Obj;
+  Obj["Version"] = "15.0";
+  Obj["MaximumDeploymentTarget"] = "15.0.99";
+  llvm::json::Object VersionMap;
+  VersionMap["10.0"] = "10.0";
+  VersionMap["10.3.1"] = "10.2";
+  VersionMap["11.0"] = "11.0";
+  llvm::json::Object IOSToTvOS;
+  IOSToTvOS["iOS_tvOS"] = std::move(VersionMap);
+  Obj["VersionMap"] = std::move(IOSToTvOS);
+
+  auto SDKInfo = DarwinSDKInfo::parseDarwinSDKSettingsJSON(&Obj);
+  ASSERT_TRUE(SDKInfo);
+  EXPECT_EQ(SDKInfo->getVersion(), VersionTuple(15, 0));
+
+  // Verify that mapping is present for platforms that derive from iOS.
+  const auto *Mapping = SDKInfo->getVersionMapping(DarwinSDKInfo::OSEnvPair(
+  llvm::Triple::IOS, llvm::Triple::UnknownEnvironment, llvm::Triple::TvOS,
+  llvm::Triple::UnknownEnvironment));
+  ASSERT_TRUE(Mapping);
+
+  // Verify that the iOS versions that are present in the map are translated
+  // directly to their corresponding tvOS versions.
+  EXPECT_EQ(*Mapping->map(VersionTuple(10, 0), VersionTuple(), None),
+VersionTuple(10, 0));
+  EXPECT_EQ(*Mapping->map(VersionTuple(10, 3, 1), VersionTuple(), None),
+VersionTuple(10, 2));
+  EXPECT_EQ(*Mapping->map(VersionTuple(11, 0), VersionTuple(), None),
+VersionTuple(11, 0));
+
+  // Verify that an iOS version that's not present in the map is translated
+  // like the nearest major OS version.
+  EXPECT_EQ(*Mapping->map(VersionTuple(10, 1), VersionTuple(), None),
+VersionTuple(10, 0));
+
+  // Verify that the iOS versions that are outside of the mapped version
+  // range map to the min/max values passed to the `map` call.
+  EXPECT_EQ(*Mapping->map(VersionTuple(9, 0), VersionTuple(99, 99), None),
+VersionTuple(99, 99));
+  EXPECT_EQ(
+  *Mapping->

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

[Elvis Stansvik via Phabricator via cfe-commits]

estan added a comment.

@lebedev.ri @aaron.ballman I think the opt-in sounds good too. Are we waiting 
for @fiesh to implement this, or is the patch abandoned? Would be sad if it was 
stranded the way https://reviews.llvm.org/D31130 was, since I think many people 
are skipping this check due to diagnostics that are for all intents and 
purposes "out of their control".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88833

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

[PATCH] D116314: [clang-format] Add style to separate definition blocks

[ksyx via Phabricator via cfe-commits]

ksyx added a comment.

In D116314#3221488 , @MyDeveloperDay 
wrote:

> @ksyx
>
> Did you see this issue https://github.com/llvm/llvm-project/issues/52976
>
> In its current form, this patch causes huge amounts of flux in projects
> that document code like (anyone using doxygen likely, + most normal people
> ;-) )
>
> ...
> }
>
> // My function
>   extra newline added
> void
> foo()
> {
> }
>
> as it will place and undesirable additional newline between the function
> and the return type.
>
> We really need this to be resolved before we branch out v14 (which may be
> imminent)
>
> MyDeveloperDay

Thanks for the feedback! I have proposed some fix in D116663 
, and please see if this works.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116314

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

[PATCH] D115301: [clangd] Don't index __reserved_names in headers.

[Sam McCall via Phabricator via cfe-commits]

sammccall marked an inline comment as done.
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/SourceCode.h:334
+/// Returns true if Name is reserved, like _Foo or __Vector_base.
+inline bool isReservedName(llvm::StringRef Name) {
+  // This doesn't catch all cases, but the most common.

kadircet wrote:
> nit: maybe move definition out-of-line.
This "just a few instructions", I'd rather let the compiler make that call...



Comment at: clang-tools-extra/clangd/unittests/QualityTests.cpp:60
-  EXPECT_FALSE(Quality.ImplementationDetail);
-  EXPECT_TRUE(Quality.ReservedName);
-  EXPECT_EQ(Quality.References, SymbolQualitySignals().References);

kadircet wrote:
> can we still have a test that indexes main file symbols and ensures 
> `ReservedName` is set correctly?
Whoops, of course.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115301

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

[clang-tools-extra] 055d809 - [clangd] Don't index __reserved_names in headers.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-01-05T16:34:04+01:00
New Revision: 055d8090d1d5137dab88533995e0c5d9b5390c28

URL: 
https://github.com/llvm/llvm-project/commit/055d8090d1d5137dab88533995e0c5d9b5390c28
DIFF: 
https://github.com/llvm/llvm-project/commit/055d8090d1d5137dab88533995e0c5d9b5390c28.diff

LOG: [clangd] Don't index __reserved_names in headers.

Main use of these is in the standard library, where they generally clutter up
the index.

Certain macros are also common, we don't touch indexing of macros in this patch.

Differential Revision: https://reviews.llvm.org/D115301

Added: 


Modified: 
clang-tools-extra/clangd/AST.cpp
clang-tools-extra/clangd/AST.h
clang-tools-extra/clangd/Quality.cpp
clang-tools-extra/clangd/SourceCode.h
clang-tools-extra/clangd/index/FileIndex.cpp
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/clangd/index/SymbolCollector.h
clang-tools-extra/clangd/unittests/ASTTests.cpp
clang-tools-extra/clangd/unittests/QualityTests.cpp
clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/AST.cpp 
b/clang-tools-extra/clangd/AST.cpp
index 3b5956037746..53b55e1579ec 100644
--- a/clang-tools-extra/clangd/AST.cpp
+++ b/clang-tools-extra/clangd/AST.cpp
@@ -376,6 +376,24 @@ std::string printType(const QualType QT, const DeclContext 
&CurContext) {
   return OS.str();
 }
 
+bool hasReservedName(const Decl &D) {
+  if (const auto *ND = llvm::dyn_cast(&D))
+if (const auto *II = ND->getIdentifier())
+  return isReservedName(II->getName());
+  return false;
+}
+
+bool hasReservedScope(const DeclContext &DC) {
+  for (const DeclContext *D = &DC; D; D = D->getParent()) {
+if (D->isTransparentContext() || D->isInlineNamespace())
+  continue;
+if (const auto *ND = llvm::dyn_cast(D))
+  if (hasReservedName(*ND))
+return true;
+  }
+  return false;
+}
+
 QualType declaredType(const TypeDecl *D) {
   if (const auto *CTSD = llvm::dyn_cast(D))
 if (const auto *TSI = CTSD->getTypeAsWritten())

diff  --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h
index 53d9f16a4262..afd591f0f4b4 100644
--- a/clang-tools-extra/clangd/AST.h
+++ b/clang-tools-extra/clangd/AST.h
@@ -74,6 +74,12 @@ std::string printObjCMethod(const ObjCMethodDecl &Method);
 // `MyClass()`, `MyClass(Category)`, and `MyProtocol`.
 std::string printObjCContainer(const ObjCContainerDecl &C);
 
+/// Returns true if this is a NamedDecl with a reserved name.
+bool hasReservedName(const Decl &);
+/// Returns true if this scope would be written with a reserved name.
+/// This does not include unwritten scope elements like __1 in 
std::__1::vector.
+bool hasReservedScope(const DeclContext &);
+
 /// Gets the symbol ID for a declaration. Returned SymbolID might be null.
 SymbolID getSymbolID(const Decl *D);
 

diff  --git a/clang-tools-extra/clangd/Quality.cpp 
b/clang-tools-extra/clangd/Quality.cpp
index dc9c75b57670..900db4913f80 100644
--- a/clang-tools-extra/clangd/Quality.cpp
+++ b/clang-tools-extra/clangd/Quality.cpp
@@ -24,7 +24,6 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
-#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/FormatVariadic.h"
@@ -35,11 +34,6 @@
 
 namespace clang {
 namespace clangd {
-static bool isReserved(llvm::StringRef Name) {
-  // FIXME: Should we exclude _Bool and others recognized by the standard?
-  return Name.size() >= 2 && Name[0] == '_' &&
- (isUppercase(Name[1]) || Name[1] == '_');
-}
 
 static bool hasDeclInMainFile(const Decl &D) {
   auto &SourceMgr = D.getASTContext().getSourceManager();
@@ -188,9 +182,10 @@ void SymbolQualitySignals::merge(const 
CodeCompletionResult &SemaCCResult) {
   if (SemaCCResult.Declaration) {
 ImplementationDetail |= isImplementationDetail(SemaCCResult.Declaration);
 if (auto *ID = SemaCCResult.Declaration->getIdentifier())
-  ReservedName = ReservedName || isReserved(ID->getName());
+  ReservedName = ReservedName || isReservedName(ID->getName());
   } else if (SemaCCResult.Kind == CodeCompletionResult::RK_Macro)
-ReservedName = ReservedName || isReserved(SemaCCResult.Macro->getName());
+ReservedName =
+ReservedName || isReservedName(SemaCCResult.Macro->getName());
 }
 
 void SymbolQualitySignals::merge(const Symbol &IndexResult) {
@@ -198,7 +193,7 @@ void SymbolQualitySignals::merge(const Symbol &IndexResult) 
{
   ImplementationDetail |= (IndexResult.Flags & Symbol::ImplementationDetail);
   References = std::max(IndexResult.References, References);
   Category = categorize(IndexResult.SymInfo);
-  ReservedName = ReservedName || isReserved(IndexResult.Name);
+  ReservedName = ReservedName || is

[PATCH] D115301: [clangd] Don't index __reserved_names in headers.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.
Closed by commit rG055d8090d1d5: [clangd] Don't index __reserved_names in 
headers. (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D115301?vs=392579&id=397582#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115301

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/clangd/Quality.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/index/FileIndex.cpp
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/SymbolCollector.h
  clang-tools-extra/clangd/unittests/ASTTests.cpp
  clang-tools-extra/clangd/unittests/QualityTests.cpp
  clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
  clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp

Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -17,7 +17,6 @@
 #include "clang/Index/IndexingOptions.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
-#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/VirtualFileSystem.h"
@@ -1849,6 +1848,22 @@
   UnorderedElementsAre(QName("Foo"), QName("Foo::fun:")));
 }
 
+TEST_F(SymbolCollectorTest, Reserved) {
+  const char *Header = R"cpp(
+void __foo();
+namespace _X { int secret; }
+  )cpp";
+
+  CollectorOpts.CollectReserved = true;
+  runSymbolCollector("", Header);
+  EXPECT_THAT(Symbols, UnorderedElementsAre(QName("__foo"), QName("_X"),
+QName("_X::secret")));
+
+  CollectorOpts.CollectReserved = false;
+  runSymbolCollector("", Header); //
+  EXPECT_THAT(Symbols, IsEmpty());
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
===
--- clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -315,6 +315,16 @@
   }
 }
 
+TEST(SourceCodeTests, isReservedName) {
+  EXPECT_FALSE(isReservedName(""));
+  EXPECT_FALSE(isReservedName("_"));
+  EXPECT_FALSE(isReservedName("foo"));
+  EXPECT_FALSE(isReservedName("_foo"));
+  EXPECT_TRUE(isReservedName("__foo"));
+  EXPECT_TRUE(isReservedName("_Foo"));
+  EXPECT_FALSE(isReservedName("foo__bar")) << "FIXME";
+}
+
 TEST(SourceCodeTests, CollectIdentifiers) {
   auto Style = format::getLLVMStyle();
   auto IDs = collectIdentifiers(R"cpp(
Index: clang-tools-extra/clangd/unittests/QualityTests.cpp
===
--- clang-tools-extra/clangd/unittests/QualityTests.cpp
+++ clang-tools-extra/clangd/unittests/QualityTests.cpp
@@ -54,13 +54,6 @@
   auto AST = Header.build();
 
   SymbolQualitySignals Quality;
-  Quality.merge(findSymbol(Symbols, "_X"));
-  EXPECT_FALSE(Quality.Deprecated);
-  EXPECT_FALSE(Quality.ImplementationDetail);
-  EXPECT_TRUE(Quality.ReservedName);
-  EXPECT_EQ(Quality.References, SymbolQualitySignals().References);
-  EXPECT_EQ(Quality.Category, SymbolQualitySignals::Variable);
-
   Quality.merge(findSymbol(Symbols, "X_Y_Decl"));
   EXPECT_TRUE(Quality.ImplementationDetail);
 
@@ -83,6 +76,16 @@
   Quality = {};
   Quality.merge(CodeCompletionResult("if"));
   EXPECT_EQ(Quality.Category, SymbolQualitySignals::Keyword);
+
+  // Testing ReservedName in main file, we don't index those symbols in headers.
+  auto MainAST = TestTU::withCode("int _X;").build();
+  SymbolSlab MainSymbols = std::get<0>(indexMainDecls(MainAST));
+
+  Quality = {};
+  Quality.merge(findSymbol(MainSymbols, "_X"));
+  EXPECT_FALSE(Quality.Deprecated);
+  EXPECT_FALSE(Quality.ImplementationDetail);
+  EXPECT_TRUE(Quality.ReservedName);
 }
 
 TEST(QualityTests, SymbolRelevanceSignalExtraction) {
Index: clang-tools-extra/clangd/unittests/ASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ASTTests.cpp
@@ -427,6 +427,29 @@
   Contains(attrKind(attr::Unlikely)));
 }
 
+TEST(ClangdAST, HasReservedName) {
+  ParsedAST AST = TestTU::withCode(R"cpp(
+void __foo();
+namespace std {
+  inline namespace __1 { class error_code; }
+  namespace __detail { int secret; }
+}
+  )cpp")
+  .build();
+
+  EXPECT_TRUE(hasReservedName(findUnqualifiedDecl(AST, "__foo")));
+  EXPECT_FALSE(
+  hasReservedScope(*findUnqualifiedDecl(AST, "__foo").getDeclContext()));
+
+  EXPECT_FALSE(hasReserv

[PATCH] D116020: [clang][#52782] Bail on incomplete parameter type in stdcall name mangling

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a comment.

lgtm




Comment at: clang/test/CodeGen/pr52782-stdcall-func-decl.cpp:10
+class nsICanvasRenderingContextInternal {
+  NS_IMETHOD_(nsresult) InitializeWithDrawTarget(NotNull);
+} nsTBaseHashSet;

rnk wrote:
> Please check for the declaration with the mangled name. In this case, we 
> expect to see a `@0` suffix.
> 
> Also, the `NS_IMETHOD_` macro isn't necessary for the reduction, it can just 
> be `void __stdcall InitializeWithDrawTarget(NotNull)`.
I see, the `@4` suffix is for `this`.


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

https://reviews.llvm.org/D116020

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

[PATCH] D116638: [clang-format] Fix ignoring JavaScriptWrapImport when ColumnWidth: 0

[Andrey Mishchenko via Phabricator via cfe-commits]

andmis updated this revision to Diff 397579.
andmis added a comment.

- Move source code for option documentation to `Format.h`, from which the RST 
is autogenerated.
- Clean up tests in response to review feedback.


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

https://reviews.llvm.org/D116638

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/unittests/Format/FormatTestJS.cpp

Index: clang/unittests/Format/FormatTestJS.cpp
===
--- clang/unittests/Format/FormatTestJS.cpp
+++ clang/unittests/Format/FormatTestJS.cpp
@@ -1946,6 +1946,7 @@
   verifyFormat("import {VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying,"
" VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying"
"} from 'some/module.js';");
+
   FormatStyle Style = getGoogleJSStyleWithColumns(80);
   Style.JavaScriptWrapImports = true;
   verifyFormat("import {\n"
@@ -1954,18 +1955,76 @@
"  VeryLongImportsAreAnnoying,\n"
"} from 'some/module.js';",
Style);
+  verifyFormat("import {A, B} from 'some/module.js';", Style);
+  Style.JavaScriptWrapImports = false;
+  verifyFormat("import {VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying,"
+   " VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying"
+   "} from 'some/module.js';",
+   Style);
+  verifyFormat("import {A, B} from 'some/module.js';", Style);
+
+  // For ColumnLimit: 0 tests, we use the code/expected_output form of
+  // verifyFormat because the output format depends on the input format, so we
+  // can't let test::messUp modify the input indiscriminately.
+  Style.ColumnLimit = 0;
+  Style.JavaScriptWrapImports = true;
+  verifyFormat("import {\n"
+   "  VeryLongImportsAreAnnoying,\n"
+   "  VeryLongImportsAreAnnoying,\n"
+   "  VeryLongImportsAreAnnoying,\n"
+   "  VeryLongImportsAreAnnoying,\n"
+   "} from 'some/module.js';",
+   "import {\n"
+   "  VeryLongImportsAreAnnoying,\n"
+   "  VeryLongImportsAreAnnoying,\n"
+   "  VeryLongImportsAreAnnoying,\n"
+   "  VeryLongImportsAreAnnoying,\n"
+   "} from 'some/module.js';",
+   Style);
+  verifyFormat("import {VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying,"
+   " VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying"
+   "} from 'some/module.js';",
+   "import {VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying,"
+   " VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying"
+   "} from 'some/module.js';",
+   Style);
   verifyFormat("import {\n"
"  A,\n"
+   "  B\n"
+   "} from 'some/module.js';",
+   "import {\n"
"  A,\n"
+   "  B\n"
"} from 'some/module.js';",
Style);
-  verifyFormat("export {\n"
-   "  A,\n"
+  verifyFormat("import {A, B} from 'some/module.js';",
+   "import {A, B} from 'some/module.js';",
+   Style);
+  Style.JavaScriptWrapImports = false;
+  verifyFormat("import {VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying,"
+   " VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying"
+   "} from 'some/module.js';",
+   "import {\n"
+   "  VeryLongImportsAreAnnoying,\n"
+   "  VeryLongImportsAreAnnoying,\n"
+   "  VeryLongImportsAreAnnoying,\n"
+   "  VeryLongImportsAreAnnoying\n"
+   "} from 'some/module.js';",
+   Style);
+  verifyFormat("import {A, B} from 'some/module.js';",
+   "import {\n"
"  A,\n"
+   "  B\n"
"} from 'some/module.js';",
Style);
+  verifyFormat("import {A, B} from 'some/module.js';",
+   "import {A, B} from 'some/module.js';",
+   Style);
+
+  // Here we use the code/expected_output form of verifyFormat because
+  // test::messUp would mess up the case being tested.
   Style.ColumnLimit = 40;
-  // Using this version of verifyFormat because test::messUp hides the issue.
+  Style.JavaScriptWrapImports = true;
   verifyFormat("import {\n"
"  A,\n"
"} from\n"
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -325,12 +325,19 @@
   if (Previous.is(tok::l_square) && Previous.is(TT_ObjCMethodExpr))
 return false;
 
+  if (Style.isJavaScript() && State.Line->Type == LT_ImportStatement &&
+  !Style.JavaScriptWrapImports)
+return false;
+
   r

[PATCH] D116638: [clang-format] Fix ignoring JavaScriptWrapImport when ColumnWidth: 0

[Andrey Mishchenko via Phabricator via cfe-commits]

andmis added a comment.

Thanks all for the reviews. I've updated the patch and added responses to your 
comments.




Comment at: clang/docs/ClangFormatStyleOptions.rst:2820
 
+**JavaScriptWrapImports** (``Boolean``) :versionbadge:`clang-format 3.9`
+  Whether to wrap JavaScript import/export statements. If ``false``, then

MyDeveloperDay wrote:
> This file is generated from Format.h you need to make the changes there and 
> regenerate using clang/docs/tools/dump_format_style.py
Alright. I've added the source to `Format.h` and run `dump_format_style.py`.



Comment at: clang/docs/ClangFormatStyleOptions.rst:2827
 
-**JavaScriptWrapImports** (``Boolean``) :versionbadge:`clang-format 3.9`
-  Whether to wrap JavaScript import/export statements.
+  * If ``ColumnWidth`` is 0 (no limit on the number of columns), then import
+statements will keep the number of lines they start with.

HazardyKnusperkeks wrote:
> mprobst wrote:
> > this seems odd to me: my understanding is that clang-format always reflows 
> > the entire document, there's no logic to ever "keep" whitespace.
> > 
> > Are you sure you are seeing this behaviour? The logic change below sounds 
> > more as if the ColumnWidth: 0, import lines might not break (mustBreak 
> > returns false), but might still break?
> From what I can tell with `ColumnLimit` (and not `ColumnWidth`) 0, then 
> `clang-format` does many things differently. If by design or by accident I'm 
> not sure. I can see that often, since I format my code with a limit, but my 
> tests without.
I just double-checked and yes, as-implemented, with `ColumnLimit: 0` and 
`JavaScriptWrapImports: true`, import statements retain the number of lines 
they started with.

It is explicit in the code that with `ColumnLimit: 0`, we decide whether to 
break on a token based on whether there was a preexisting line break. (Check 
out `NoColumnLimitLineFormatter`.) I think you're right that usually 
clang-format reflows everything and I was also confused about this point.

There are other options that do similar things, for example 
`EmptyLineAfterAccessModifier`.



Comment at: clang/docs/ClangFormatStyleOptions.rst:2836
 
- true:
+ // Original
+ import {VeryLongImport, AnotherLongImport, LongImportsAreAnnoying} from 
'some/module.js'

curdeius wrote:
> What does "Original" mean here? It's unclear whether the option is true or 
> false nor what the ColumnWidth is.
> 
> I think you want to have 3 examples:
> 1) JavaScriptWrapImports: false, independent of ColumnWidth
> 2) JavaScriptWrapImports: true, ColumnWidth: 0
> 3) JavaScriptWrapImports: true, ColumnWidth: non-zero
> In each case you want to have short and long import lines.
"Original" means "Before running clang-format", I've changed the comment to the 
latter.

The reason this is here is that with `ColumnLimit: 0`, the output format 
depends on the input, and I don't see how to make that clear without having a 
before and after view.



Comment at: clang/unittests/Format/FormatTestJS.cpp:1948
+  Style.JavaScriptWrapImports = false;
   verifyFormat("import {VeryLongImportsAreAnnoying, 
VeryLongImportsAreAnnoying,"
" VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying"

MyDeveloperDay wrote:
> MyDeveloperDay wrote:
> > you are no longer testing the LLVM Case, please don't remove that, but feel 
> > free to ensure they are doing the same for google!
> Its kind of our golden rule, don't change existing tests, subtle changes can 
> have huge implications on large code bases
Thanks, this was an oversight on my part.



Comment at: clang/unittests/Format/FormatTestJS.cpp:1975
+   Style);
+  // Using the input_code/expected version of verifyFormat because we can't
+  // indiscriminately test::messUp on these tests.

mprobst wrote:
> I'm not sure this test case really repros the doc changes you made (keeps any 
> line wraps with ColW: 0).
> 
> I think if you wanted to demonstrate that, you'd need to add a test case 
> where clang-format would normally make a change, and show that it does not 
> with ColW: 0.
> 
> However I think that's not feasible: by design, clang-format cannot not make 
> a change, it always reformats all code. You might need to rethink the intent 
> here.
I think the tests are doing what's claimed. I've rearranged and rewritten them, 
PTAL.



Comment at: clang/unittests/Format/FormatTestJS.cpp:1990
   Style.ColumnLimit = 40;
-  // Using this version of verifyFormat because test::messUp hides the issue.
+  Style.JavaScriptWrapImports = true;
   verifyFormat("import {\n"

mprobst wrote:
> curdeius wrote:
> > It's already true, cf. line 1977. Remove this line.
> FWIW, I think the tests might be more readable if each configuration ({true, 
> false} x {col: 0, col: 40}) explicitly set all the options, even 

[PATCH] D116638: [clang-format] Fix ignoring JavaScriptWrapImport when ColumnWidth: 0

[Martin Probst via Phabricator via cfe-commits]

mprobst added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:2827
 
-**JavaScriptWrapImports** (``Boolean``) :versionbadge:`clang-format 3.9`
-  Whether to wrap JavaScript import/export statements.
+  * If ``ColumnWidth`` is 0 (no limit on the number of columns), then import
+statements will keep the number of lines they start with.

andmis wrote:
> HazardyKnusperkeks wrote:
> > mprobst wrote:
> > > this seems odd to me: my understanding is that clang-format always 
> > > reflows the entire document, there's no logic to ever "keep" whitespace.
> > > 
> > > Are you sure you are seeing this behaviour? The logic change below sounds 
> > > more as if the ColumnWidth: 0, import lines might not break (mustBreak 
> > > returns false), but might still break?
> > From what I can tell with `ColumnLimit` (and not `ColumnWidth`) 0, then 
> > `clang-format` does many things differently. If by design or by accident 
> > I'm not sure. I can see that often, since I format my code with a limit, 
> > but my tests without.
> I just double-checked and yes, as-implemented, with `ColumnLimit: 0` and 
> `JavaScriptWrapImports: true`, import statements retain the number of lines 
> they started with.
> 
> It is explicit in the code that with `ColumnLimit: 0`, we decide whether to 
> break on a token based on whether there was a preexisting line break. (Check 
> out `NoColumnLimitLineFormatter`.) I think you're right that usually 
> clang-format reflows everything and I was also confused about this point.
> 
> There are other options that do similar things, for example 
> `EmptyLineAfterAccessModifier`.
Can you check with code that would normally cause reformatting? E.g.

```
import {
  A, B,
  C,
} from 'url';
```



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

https://reviews.llvm.org/D116638

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

[PATCH] D116658: [clang-format][NFC] Fix typo in 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.
You need someone to land it for you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116658

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

[PATCH] D116627: [Clang] Initial support for linking offloading code in tool

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 397589.
jhuber6 added a comment.

Small changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116627

Files:
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp

Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
===
--- clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -28,6 +28,7 @@
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/FileOutputBuffer.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Host.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
@@ -59,22 +60,26 @@
 
 // Do not parse linker options.
 static cl::list
-LinkerArgs(cl::Sink, cl::desc("..."));
+HostLinkerArgs(cl::Sink, cl::desc("..."));
 
 /// Path of the current binary.
 static std::string LinkerExecutable;
 
+static SmallVector TempFiles;
 /// Magic section string that marks the existence of offloading data. The
 /// section string will be formatted as `.llvm.offloading..`.
-#define OFFLOAD_SECTION_MAGIC_STR ".llvm.offloading"
+#define OFFLOAD_SECTION_MAGIC_STR ".llvm.offloading."
 
+/// Information for a device offloading file extracted from the host.
 struct DeviceFile {
   DeviceFile(StringRef TheTriple, StringRef Arch, StringRef Filename)
   : TheTriple(TheTriple), Arch(Arch), Filename(Filename) {}
 
-  const Triple TheTriple;
+  const std::string TheTriple;
   const std::string Arch;
   const std::string Filename;
+
+  std::string str() const { return TheTriple + "-" + Arch; }
 };
 
 namespace {
@@ -83,6 +88,16 @@
 extractFromBuffer(std::unique_ptr Buffer,
   SmallVectorImpl &DeviceFiles);
 
+static StringRef getDeviceFileExtension(StringRef DeviceTriple,
+bool IsBitcode = false) {
+  Triple TheTriple(DeviceTriple);
+  if (TheTriple.isAMDGPU() || IsBitcode)
+return "bc";
+  if (TheTriple.isNVPTX())
+return "cubin";
+  return "o";
+}
+
 Error runLinker(std::string &LinkerPath, SmallVectorImpl &Args) {
   std::vector LinkerArgs;
   LinkerArgs.push_back(LinkerPath);
@@ -154,9 +169,12 @@
 
 if (Expected Contents = Sec.getContents()) {
   SmallString<128> TempFile;
+  StringRef DeviceExtension = getDeviceFileExtension(
+  DeviceTriple, identify_magic(*Contents) == file_magic::bitcode);
   if (std::error_code EC = sys::fs::createTemporaryFile(
-  Prefix + "-device-" + DeviceTriple, Extension, TempFile))
+  Prefix + "-device-" + DeviceTriple, DeviceExtension, TempFile))
 return createFileError(TempFile, EC);
+  TempFiles.push_back(static_cast(TempFile));
 
   Expected> OutputOrErr =
   FileOutputBuffer::create(TempFile, Sec.getSize());
@@ -189,6 +207,7 @@
   if (std::error_code EC =
   sys::fs::createTemporaryFile(Prefix + "-host", Extension, TempFile))
 return createFileError(TempFile, EC);
+  TempFiles.push_back(static_cast(TempFile));
 
   SmallVector StripArgs;
   StripArgs.push_back(*StripPath);
@@ -245,9 +264,12 @@
 
 StringRef Contents = CDS->getAsString();
 SmallString<128> TempFile;
+StringRef DeviceExtension = getDeviceFileExtension(
+DeviceTriple, identify_magic(Contents) == file_magic::bitcode);
 if (std::error_code EC = sys::fs::createTemporaryFile(
-Prefix + "-device-" + DeviceTriple, Extension, TempFile))
+Prefix + "-device-" + DeviceTriple, DeviceExtension, TempFile))
   return createFileError(TempFile, EC);
+TempFiles.push_back(static_cast(TempFile));
 
 Expected> OutputOrErr =
 FileOutputBuffer::create(TempFile, Contents.size());
@@ -279,6 +301,8 @@
   if (std::error_code EC =
   sys::fs::createTemporaryFile(Prefix + "-host", Extension, TempFile))
 return createFileError(TempFile, EC);
+  TempFiles.push_back(static_cast(TempFile));
+
   std::error_code EC;
   raw_fd_ostream HostOutput(TempFile, EC, sys::fs::OF_None);
   if (EC)
@@ -349,6 +373,7 @@
   if (std::error_code EC =
   sys::fs::createTemporaryFile(Prefix + "-host", Extension, TempFile))
 return createFileError(TempFile, EC);
+  TempFiles.push_back(static_cast(TempFile));
 
   std::unique_ptr Buffer =
   MemoryBuffer::getMemBuffer(Library.getMemoryBufferRef(), false);
@@ -388,8 +413,160 @@
   default:
 return errorCodeToError(object_error::invalid_file_type);
   }
+}
+
+// TODO: Move these to a separate file.
+namespace nvptx {
+Expected link(ArrayRef InputFiles,
+   ArrayRef LinkerArgs, Triple TheTriple,
+   StringRef Arch) {
+  // NVPTX uses the nvlink binary to link device object files.
+  ErrorOr NvlinkPath = sys::findProgramByName(
+  "nvlink", sys::path::parent_path(LinkerExecutable));
+  if (!Nv

[PATCH] D116542: [OpenMP] Add a flag for embedding a file into the module

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 397590.
jhuber6 added a comment.

Clang format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116542

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/CodeGen/BackendUtil.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/test/Frontend/embed-object.ll
  llvm/include/llvm/Bitcode/BitcodeWriter.h
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -4972,3 +4972,42 @@
   llvm::ConstantArray::get(ATy, UsedArray), "llvm.compiler.used");
   NewUsed->setSection("llvm.metadata");
 }
+
+void llvm::EmbedObjectInModule(llvm::Module &M, llvm::MemoryBufferRef Buf,
+   StringRef SectionName) {
+  // Save llvm.compiler.used and remove it.
+  SmallVector UsedArray;
+  SmallVector UsedGlobals;
+  Type *UsedElementType = Type::getInt8Ty(M.getContext())->getPointerTo(0);
+  GlobalVariable *Used = collectUsedGlobalVariables(M, UsedGlobals, true);
+  for (auto *GV : UsedGlobals) {
+if (!GV->getName().startswith("llvm.embedded.object"))
+  UsedArray.push_back(
+  ConstantExpr::getPointerBitCastOrAddrSpaceCast(GV, UsedElementType));
+  }
+  if (Used)
+Used->eraseFromParent();
+
+  ArrayRef ModuleData = ArrayRef(
+  (const uint8_t *)Buf.getBufferStart(), Buf.getBufferSize());
+
+  // Embed the data in the
+  llvm::Constant *ModuleConstant =
+  llvm::ConstantDataArray::get(M.getContext(), ModuleData);
+  llvm::GlobalVariable *GV = new llvm::GlobalVariable(
+  M, ModuleConstant->getType(), true, llvm::GlobalValue::PrivateLinkage,
+  ModuleConstant, "llvm.embedded.object");
+  GV->setSection(SectionName);
+  // Set alignment to 1 to prevent padding between two contributions from input
+  // sections after linking.
+  GV->setAlignment(Align(1));
+  UsedArray.push_back(
+  ConstantExpr::getPointerBitCastOrAddrSpaceCast(GV, UsedElementType));
+
+  // Recreate llvm.compiler.used.
+  ArrayType *ATy = ArrayType::get(UsedElementType, UsedArray.size());
+  auto *NewUsed = new GlobalVariable(
+  M, ATy, false, llvm::GlobalValue::AppendingLinkage,
+  llvm::ConstantArray::get(ATy, UsedArray), "llvm.compiler.used");
+  NewUsed->setSection("llvm.metadata");
+}
Index: llvm/include/llvm/Bitcode/BitcodeWriter.h
===
--- llvm/include/llvm/Bitcode/BitcodeWriter.h
+++ llvm/include/llvm/Bitcode/BitcodeWriter.h
@@ -165,6 +165,11 @@
 bool EmbedCmdline,
 const std::vector &CmdArgs);
 
+  /// Embeds the memory buffer \p Buf into the module \p M as a global using the
+  /// section name \p SectionName.
+  void EmbedObjectInModule(Module &M, MemoryBufferRef Buf,
+   StringRef SectionName);
+
 } // end namespace llvm
 
 #endif // LLVM_BITCODE_BITCODEWRITER_H
Index: clang/test/Frontend/embed-object.ll
===
--- /dev/null
+++ clang/test/Frontend/embed-object.ll
@@ -0,0 +1,13 @@
+; RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm \
+; RUN:-fembed-offload-binary=%S/Inputs/empty.h -fembed-offload-section=section -x ir %s -o - \
+; RUN:| FileCheck %s -check-prefix=CHECK
+
+; CHECK: @llvm.embedded.object = private constant [0 x i8] zeroinitializer, section ".llvm.offloading.section", align 1
+; CHECK: @llvm.compiler.used = appending global [2 x i8*] [i8* @x, i8* getelementptr inbounds ([0 x i8], [0 x i8]* @llvm.embedded.object, i32 0, i32 0)], section "llvm.metadata"
+
+@x = private constant i8 1
+@llvm.compiler.used = appending global [1 x i8*] [i8* @x], section "llvm.metadata"
+
+define i32 @foo() {
+  ret i32 0
+}
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -1134,6 +1134,7 @@
 TheModule->setTargetTriple(TargetOpts.Triple);
   }
 
+  EmbedBinary(TheModule.get(), CodeGenOpts, Diagnostics);
   EmbedBitcode(TheModule.get(), CodeGenOpts, *MainFile);
 
   LLVMContext &Ctx = TheModule->getContext();
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -1737,8 +1737,43 @@
  llvm::MemoryBufferRef Buf) {
   if (CGOpts.getEmbedBitcode() == CodeGenOptions::Embed_Off)
 return;
+
   llvm::EmbedBitcodeInModule(
   *M, Buf, CGOpts.getEmbedBitcode() != CodeGenOptions::Embed_Marker,
   CGOpts.getEmbedBitcode() != CodeGenOptions::Embed_Bitcode,

[PATCH] D116658: [clang-format][NFC] Fix typo in comment

[Jino Park via Phabricator via cfe-commits]

pjessesco added a comment.

Yes please, this is my first contribution in this project.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116658

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