[PATCH] D117205: [clang-tidy] Support custom fix hint for cppcoreguidelines-pro-bounds-constant-array-index

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp:84
+
+if (!GslHeader.empty() && (FixHint == "gsl::at()")) {
   Diag << FixItHint::CreateInsertion(BaseRange.getBegin(), "gsl::at(")

njames93 wrote:
> This restriction on only offering a fix if using `gsl::at` seem a little too 
> restrictive, especially when you factor that the GslHeader is defaulted to an 
> empty string and that is already required for a fix to be emitted.
What would this change for the user? Current behavior is kept, and we can't 
really predict a good fix if the hint is user-provided. There's many other ways 
to perform safe indexing than using a free standing function gsl::at-like, and 
those will require a larger, non-automated refactor.

Another alternative I had in mind is to completely remove the "use gsl::at() 
instead" hint from the message, and simply warn about the problem (unsafe 
indexing). CppCoreGuidelines don't actually mention that you need to use 
gsl::at, at least not from what I can see in the link provided in the docs. 

Removing the fix hint from the warning would solve my concerns (confusing 
information), and we don't need to introduce extra configuration options.

What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117205

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

[PATCH] D116514: [clangd] Add code action to generate a constructor for a C++ class

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp:203
+  return CopyRef;
+return Fail;
+  }

If C is default constructible, would it be nice to skip here instead of failing?



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp:211
+if (Fields.size() == 1)
+  OS << "explicit ";
+OS << Class->getName() << "(";

Some codebases may not want these to be explicit, would it be wiser to use 
config to control this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116514

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

[PATCH] D117414: [clang-format] Add return code to git-clang-format

[Marek Kurdej via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117414

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

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

Does it fix https://github.com/llvm/llvm-project/issues/46915?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

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

[PATCH] D117414: [clang-format] Add return code to git-clang-format

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.

So simple but so nice, LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117414

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

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[MyDeveloperDay via Phabricator via cfe-commits]

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



Comment at: .arclint:5
   "type": "script-and-regex",
-  "script-and-regex.script": "bash utils/arcanist/clang-format.sh",
+  "script-and-regex.script": "cmd utils\\arcanist\\clang-format.sh",
   "script-and-regex.regex": 
"/^(?P[[:alpha:]]+)\n(?P[^\n]+)\n(|(?P\\d),(?P\\d)\n(?P.*)\n(?P.*)\n)$/s",

remove this change



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:103
   return 0;
-if (RootToken.isAccessSpecifier(false) ||
+if (RootToken.isAccessSpecifier(Style.Language == FormatStyle::LK_Cpp) ||
+(RootToken.Next &&

Style.isCpp()



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2510
+  if (clang::format::OperatorsFollowingVar.find(FormatTok->Tok.getKind()) ==
+  OperatorsFollowingVar.end()) {
+addUnwrappedLine();

elide `{`



Comment at: clang/unittests/Format/FormatTest.cpp:3034
+  // Var that looks like accessSpecifier
+  verifyFormat("private.p = 1;");
 }

This isn't enough of a test, if you are going to add this 
`CppOperatorsFollowingVar` then you'd better test every case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

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

[PATCH] D111617: [RISCV] Lazily add RVV C intrinsics.

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng updated this revision to Diff 400358.
kito-cheng added a comment.
Herald added subscribers: alextsao1999, hiraditya.

Changes:

- Using different approach to implement to prevent build time explosion.
  - build time for `SemaRVVLookup.cpp` is ~6 sec in my machine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111617

Files:
  clang/include/clang/Basic/CMakeLists.txt
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/CMakeLists.txt
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaRVVLookup.cpp
  clang/utils/TableGen/RISCVVEmitter.cpp
  clang/utils/TableGen/TableGen.cpp
  clang/utils/TableGen/TableGenBackends.h
  llvm/docs/CommandGuide/tblgen.rst
  llvm/include/llvm/Support/RISCVVIntrinsicUtils.h
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/RISCVVIntrinsicUtils.cpp

Index: llvm/lib/Support/RISCVVIntrinsicUtils.cpp
===
--- /dev/null
+++ llvm/lib/Support/RISCVVIntrinsicUtils.cpp
@@ -0,0 +1,748 @@
+//===- RISCVVIntrinsicUtils.cpp - RISC-V Vector Intrinsic Utils -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "llvm/Support/RISCVVIntrinsicUtils.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringSet.h"
+#include "llvm/ADT/StringSwitch.h"
+#include "llvm/ADT/Twine.h"
+#include 
+
+namespace llvm {
+namespace RISCV {
+
+LMULType::LMULType(int NewLog2LMUL) {
+  // Check Log2LMUL is -3, -2, -1, 0, 1, 2, 3
+  assert(NewLog2LMUL <= 3 && NewLog2LMUL >= -3 && "Bad LMUL number!");
+  Log2LMUL = NewLog2LMUL;
+}
+
+std::string LMULType::str() const {
+  if (Log2LMUL < 0)
+return "mf" + utostr(1ULL << (-Log2LMUL));
+  return "m" + utostr(1ULL << Log2LMUL);
+}
+
+VScaleVal LMULType::getScale(unsigned ElementBitwidth) const {
+  int Log2ScaleResult = 0;
+  switch (ElementBitwidth) {
+  default:
+break;
+  case 8:
+Log2ScaleResult = Log2LMUL + 3;
+break;
+  case 16:
+Log2ScaleResult = Log2LMUL + 2;
+break;
+  case 32:
+Log2ScaleResult = Log2LMUL + 1;
+break;
+  case 64:
+Log2ScaleResult = Log2LMUL;
+break;
+  }
+  // Illegal vscale result would be less than 1
+  if (Log2ScaleResult < 0)
+return None;
+  return 1 << Log2ScaleResult;
+}
+
+void LMULType::MulLog2LMUL(int log2LMUL) { Log2LMUL += log2LMUL; }
+
+LMULType &LMULType::operator*=(uint32_t RHS) {
+  assert(isPowerOf2_32(RHS));
+  this->Log2LMUL = this->Log2LMUL + Log2_32(RHS);
+  return *this;
+}
+
+RVVType::RVVType(RVVBasicType BT, int Log2LMUL, StringRef prototype)
+: LMUL(LMULType(Log2LMUL)) {
+  applyBasicType(BT);
+  applyModifier(prototype);
+  Valid = verifyType();
+  if (Valid) {
+initBuiltinStr();
+initTypeStr();
+if (isVector()) {
+  initClangBuiltinStr();
+}
+  }
+}
+
+StringRef RVVType::getMangledStr() {
+  if (MangledStr.empty()) {
+if (!Valid)
+  MangledStr = "invalid";
+else {
+  MangledStr = (Twine(ScalarType) + Twine(IsPointer) + Twine(IsImmediate) +
+Twine(IsConstant) + Twine(ElementBitwidth))
+   .str();
+  if (!isScalar())
+MangledStr += (Twine(Scale.getValue()) + LMUL.str()).str();
+}
+  }
+
+  return MangledStr;
+}
+
+// clang-format off
+// boolean type are encoded the ratio of n (SEW/LMUL)
+// SEW/LMUL | 1 | 2 | 4 | 8| 16| 32| 64
+// c type   | vbool64_t | vbool32_t | vbool16_t | vbool8_t | vbool4_t  | vbool2_t  | vbool1_t
+// IR type  | nxv1i1| nxv2i1| nxv4i1| nxv8i1   | nxv16i1   | nxv32i1   | nxv64i1
+
+// type\lmul | 1/8| 1/4  | 1/2 | 1   | 2| 4| 8
+//   |--  |  | --- | --- |  |  | 
+// i64   | N/A| N/A  | N/A | nxv1i64 | nxv2i64  | nxv4i64  | nxv8i64
+// i32   | N/A| N/A  | nxv1i32 | nxv2i32 | nxv4i32  | nxv8i32  | nxv16i32
+// i16   | N/A| nxv1i16  | nxv2i16 | nxv4i16 | nxv8i16  | nxv16i16 | nxv32i16
+// i8| nxv1i8 | nxv2i8   | nxv4i8  | nxv8i8  | nxv16i8  | nxv32i8  | nxv64i8
+// double| N/A| N/A  | N/A | nxv1f64 | nxv2f64  | nxv4f64  | nxv8f64
+// float | N/A| N/A  | nxv1f32 | nxv2f32 | nxv4f32  | nxv8f32  | nxv16f32
+// half  | N/A| nxv1f16  | nxv2f16 | nxv4f16 | nxv8f16  | nxv16f16 | nxv32f16
+// clang-format on
+bool RVVType::verifyType() const {
+  if (ScalarType == STK_Invalid)
+ret

[PATCH] D111617: [RISCV] Lazily add RVV C intrinsics.

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng added a comment.

TL;DR:
--

- Binary size of clang increase ~200k, which is +0.07%  for debug build and 
+0.13% for release build.
- Single file compilation speed up ~33x speed up for debug build and ~8.5x 
speed up for release build
- Regression time reduce ~10% (`ninja check-all`, enable all targets)

Header size change
--

 |  size | LoC |
  --
  Before | 4,434,725 |  69,749 |
  After  | 6,140 | 162 |



Single File Compilation Time


Testcase:

  #include 
  
  vint32m1_t test_vadd_vv_vfloat32m1_t(vint32m1_t op1, vint32m1_t op2, size_t 
vl) {
return vadd(op1, op2, vl);
  }

Debug build:


Before:

  real0m19.352s
  user0m19.252s
  sys 0m0.092s

After:

  real0m0.576s
  user0m0.552s
  sys 0m0.024s

~33x speed up for debug build

Release build:
--

Before:

  real0m0.773s
  user0m0.741s
  sys 0m0.032s

After:

  real0m0.092s
  user0m0.080s
  sys 0m0.012s

~8.5x speed up for release build

Regression time
---

Note: the failed case is `tools/llvm-debuginfod-find/debuginfod.test` which is 
unrelated to this patch.

Debug build
---

Before:

  Testing Time: 1358.38s
Skipped  :11
Unsupported  :   446
Passed   : 75767
Expectedly Failed:   190
Failed   : 1

After

  Testing Time: 1220.29s
Skipped  :11
Unsupported  :   446
Passed   : 75767
Expectedly Failed:   190
Failed   : 1

Release build
-

Before:

  Testing Time: 381.98s
Skipped  :12
Unsupported  :  1407
Passed   : 74765
Expectedly Failed:   176
Failed   : 1

After:

  Testing Time: 346.25s
Skipped  :12
Unsupported  :  1407
Passed   : 74765
Expectedly Failed:   176
Failed   : 1



Binary size of clang


Debug build
---

Before

 textdata bss dec hex filename
  335261851   12726004 552812 348540667   14c64efb
bin/clang

After

 textdata bss dec hex filename
  335442803   12798708 552940 348794451   14ca2e53
bin/clang

+253K, +0.07% code size

Release build
-

Before

 textdata bss dec hex filename
  144123975   8374648  483140 152981763   91e5103 bin/clang

After

 textdata bss dec hex filename
  144255762   8447296  483268 153186326   9217016 bin/clang

+204K, +0.13%


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111617

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

[PATCH] D112408: [RISCV] Add the zve extension according to the v1.0 spec

[Yueh-Ting Chen via Phabricator via cfe-commits]

eopXD added a comment.

We can land non-macro related code for zve first and continue on proceeding 
patches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112408

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

[PATCH] D117423: [AVR][clang] Reject non assembly source files for the avr1 family

[Ben Shi via Phabricator via cfe-commits]

benshi001 created this revision.
benshi001 added reviewers: aykevl, dylanmckay, MaskRay.
Herald added a subscriber: Jim.
benshi001 requested review of this revision.
Herald added subscribers: cfe-commits, jacquesguan.
Herald added a project: clang.

Devices belongs to the avr1 family do not has SRAM, so
only assembly source files should be accepted. This behaviour
conforms to GCC.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117423

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/AVR.cpp
  clang/lib/Driver/ToolChains/AVR.h
  clang/test/Driver/avr-mmcu.c

Index: clang/test/Driver/avr-mmcu.c
===
--- clang/test/Driver/avr-mmcu.c
+++ clang/test/Driver/avr-mmcu.c
@@ -1,8 +1,7 @@
 // A test for the propagation of the -mmcu option to -cc1 and -cc1as
 
-// RUN: %clang -### -target avr -no-canonical-prefixes -mmcu=attiny11 -save-temps %s 2>&1 | FileCheck -check-prefix=CHECK0 %s
-// CHECK0: clang{{.*}} "-cc1" {{.*}} "-target-cpu" "attiny11"
-// CHECK0: clang{{.*}} "-cc1as" {{.*}} "-target-cpu" "attiny11"
+// RUN: %clang -### -target avr -no-canonical-prefixes -mmcu=attiny11 %s 2>&1 | FileCheck -check-prefix=CHECK0 %s
+// CHECK0: error: CPU 'attiny11' does not support 'c' language mode
 
 // RUN: %clang -### -target avr -no-canonical-prefixes -mmcu=at90s2313 -save-temps %s 2>&1 | FileCheck -check-prefix=CHECK1 %s
 // CHECK1: clang{{.*}} "-cc1" {{.*}} "-target-cpu" "at90s2313"
Index: clang/lib/Driver/ToolChains/AVR.h
===
--- clang/lib/Driver/ToolChains/AVR.h
+++ clang/lib/Driver/ToolChains/AVR.h
@@ -19,6 +19,8 @@
 namespace toolchains {
 
 class LLVM_LIBRARY_VISIBILITY AVRToolChain : public Generic_ELF {
+  std::string AVRMcu;
+
 public:
   AVRToolChain(const Driver &D, const llvm::Triple &Triple,
const llvm::opt::ArgList &Args);
Index: clang/lib/Driver/ToolChains/AVR.cpp
===
--- clang/lib/Driver/ToolChains/AVR.cpp
+++ clang/lib/Driver/ToolChains/AVR.cpp
@@ -12,6 +12,7 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/InputInfo.h"
 #include "clang/Driver/Options.h"
+#include "clang/Driver/Types.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
@@ -19,6 +20,7 @@
 #include "llvm/MC/SubtargetFeature.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
 
 using namespace clang::driver;
 using namespace clang::driver::toolchains;
@@ -311,24 +313,26 @@
 : Generic_ELF(D, Triple, Args), LinkStdlib(false) {
   GCCInstallation.init(Triple, Args);
 
+  // Save the CPU name for further checks.
+  AVRMcu = getCPUName(D, Args, Triple);
+
   // Only add default libraries if the user hasn't explicitly opted out.
   if (!Args.hasArg(options::OPT_nostdlib) &&
   !Args.hasArg(options::OPT_nodefaultlibs) &&
   !Args.hasArg(options::OPT_c /* does not apply when not linking */)) {
-std::string CPU = getCPUName(D, Args, Triple);
 
-if (CPU.empty()) {
+if (AVRMcu.empty()) {
   // We cannot link any standard libraries without an MCU specified.
   D.Diag(diag::warn_drv_avr_mcu_not_specified);
 } else {
-  Optional FamilyName = GetMCUFamilyName(CPU);
+  Optional FamilyName = GetMCUFamilyName(AVRMcu);
   Optional AVRLibcRoot = findAVRLibcInstallation();
 
   if (!FamilyName.hasValue()) {
 // We do not have an entry for this CPU in the family
 // mapping table yet.
 D.Diag(diag::warn_drv_avr_family_linking_stdlibs_not_implemented)
-<< CPU;
+<< AVRMcu;
   } else if (!GCCInstallation.isValid()) {
 // No avr-gcc found and so no runtime linked.
 D.Diag(diag::warn_drv_avr_gcc_not_found);
@@ -339,7 +343,7 @@
 std::string GCCRoot(GCCInstallation.getInstallPath());
 std::string GCCParentPath(GCCInstallation.getParentLibPath());
 std::string LibcRoot = AVRLibcRoot.getValue();
-std::string SubPath = GetMCUSubPath(CPU);
+std::string SubPath = GetMCUSubPath(AVRMcu);
 
 getProgramPaths().push_back(GCCParentPath + "/../bin");
 getFilePaths().push_back(LibcRoot + std::string("/lib/") + SubPath);
@@ -379,6 +383,24 @@
   if (!DriverArgs.hasFlag(options::OPT_fuse_init_array,
   options::OPT_fno_use_init_array, false))
 CC1Args.push_back("-fno-use-init-array");
+
+  // Search for the main file name.
+  const char *MainFileName = nullptr;
+  for (unsigned I = 0; I < CC1Args.size(); I++)
+if (StringRef(CC1Args[I]) == "-main-file-name")
+  MainFileName = CC1Args[I + 1];
+  // Report an error if the main input file is not assembly and the avr mcu
+  // belongs to the AVR1 family.
+  if (MainFileName) {
+llvm::Optional FamilyName = GetMCUFamilyName(AVRMcu);
+   

[PATCH] D117423: [AVR][clang] Reject non assembly source files for the avr1 family

[Ben Shi via Phabricator via cfe-commits]

benshi001 added a comment.

This is GCC's handling on  AVR

https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/config/avr/avr.c;h=62973927fdc30d502e5d225f83cdde958bf2dad0;hb=refs/heads/master#l10424

around line 10433 / function `static void avr_file_start (void)`,

gcc rises an error `architecture %qs supported for assembler only`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117423

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

[PATCH] D117414: [clang-format] Add return code to git-clang-format

[Owen Pan 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 rGedbb8a843c13: [clang-format] Add return code to 
git-clang-format (authored by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117414

Files:
  clang/tools/clang-format/git-clang-format


Index: clang/tools/clang-format/git-clang-format
===
--- clang/tools/clang-format/git-clang-format
+++ clang/tools/clang-format/git-clang-format
@@ -161,10 +161,12 @@
   print('Running clang-format on the following files:')
   for filename in changed_lines:
 print('%s' % filename)
+
   if not changed_lines:
 if opts.verbose >= 0:
   print('no modified files to format')
-return
+return 0
+
   if len(commits) > 1:
 old_tree = commits[1]
 new_tree = run_clang_format_and_save_to_tree(changed_lines,
@@ -179,10 +181,13 @@
   if opts.verbose >= 1:
 print('old tree: %s' % old_tree)
 print('new tree: %s' % new_tree)
+
   if old_tree == new_tree:
 if opts.verbose >= 0:
   print('clang-format did not modify any files')
-  elif opts.diff:
+return 0
+
+  if opts.diff:
 print_diff(old_tree, new_tree)
   elif opts.diffstat:
 print_diffstat(old_tree, new_tree)
@@ -194,6 +199,8 @@
   for filename in changed_files:
 print('%s' % filename)
 
+  return 1
+
 
 def load_git_config(non_string_options=None):
   """Return the git configuration as a dictionary.


Index: clang/tools/clang-format/git-clang-format
===
--- clang/tools/clang-format/git-clang-format
+++ clang/tools/clang-format/git-clang-format
@@ -161,10 +161,12 @@
   print('Running clang-format on the following files:')
   for filename in changed_lines:
 print('%s' % filename)
+
   if not changed_lines:
 if opts.verbose >= 0:
   print('no modified files to format')
-return
+return 0
+
   if len(commits) > 1:
 old_tree = commits[1]
 new_tree = run_clang_format_and_save_to_tree(changed_lines,
@@ -179,10 +181,13 @@
   if opts.verbose >= 1:
 print('old tree: %s' % old_tree)
 print('new tree: %s' % new_tree)
+
   if old_tree == new_tree:
 if opts.verbose >= 0:
   print('clang-format did not modify any files')
-  elif opts.diff:
+return 0
+
+  if opts.diff:
 print_diff(old_tree, new_tree)
   elif opts.diffstat:
 print_diffstat(old_tree, new_tree)
@@ -194,6 +199,8 @@
   for filename in changed_files:
 print('%s' % filename)
 
+  return 1
+
 
 def load_git_config(non_string_options=None):
   """Return the git configuration as a dictionary.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] edbb8a8 - [clang-format] Add return code to git-clang-format

[via cfe-commits]

Author: owenca
Date: 2022-01-16T02:41:10-08:00
New Revision: edbb8a843c130e60d71cb73e56a33d5ba2cc0ec9

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

LOG: [clang-format] Add return code to git-clang-format

https://github.com/llvm/llvm-project/issues/53220

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

Added: 


Modified: 
clang/tools/clang-format/git-clang-format

Removed: 




diff  --git a/clang/tools/clang-format/git-clang-format 
b/clang/tools/clang-format/git-clang-format
index 7838fd82c1dd6..7968c43875744 100755
--- a/clang/tools/clang-format/git-clang-format
+++ b/clang/tools/clang-format/git-clang-format
@@ -161,10 +161,12 @@ def main():
   print('Running clang-format on the following files:')
   for filename in changed_lines:
 print('%s' % filename)
+
   if not changed_lines:
 if opts.verbose >= 0:
   print('no modified files to format')
-return
+return 0
+
   if len(commits) > 1:
 old_tree = commits[1]
 new_tree = run_clang_format_and_save_to_tree(changed_lines,
@@ -179,10 +181,13 @@ def main():
   if opts.verbose >= 1:
 print('old tree: %s' % old_tree)
 print('new tree: %s' % new_tree)
+
   if old_tree == new_tree:
 if opts.verbose >= 0:
   print('clang-format did not modify any files')
-  elif opts.
diff :
+return 0
+
+  if opts.
diff :
 print_
diff (old_tree, new_tree)
   elif opts.
diff stat:
 print_
diff stat(old_tree, new_tree)
@@ -194,6 +199,8 @@ def main():
   for filename in changed_files:
 print('%s' % filename)
 
+  return 1
+
 
 def load_git_config(non_string_options=None):
   """Return the git configuration as a dictionary.



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

[PATCH] D116778: [clang-tidy][clang] Don't trigger unused-parameter warnings on naked functions

[Tommaso Bonvicini via Phabricator via cfe-commits]

MuAlphaOmegaEpsilon added a comment.

Let me know if I should rebase this onto the latest main branch, at the moment 
the Windows build is passing but the Debian build is not, failing at 
`compiler-rt/test/sanitizer_common/tsan-x86_64-Linux/Linux/Output/decorate_proc_maps.cpp`
 for no immediately apparent reason.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116778

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

[clang] 4608b1d - Resolve lit failures in clang-aarch64*

[hyeongyu kim via cfe-commits]

Author: hyeongyu kim
Date: 2022-01-16T23:06:05+09:00
New Revision: 4608b1d726daa808abf08f0f0860636a7b20771f

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

LOG: Resolve lit failures in clang-aarch64*

Added: 


Modified: 
clang/test/Profile/cxx-lambda.cpp

Removed: 




diff  --git a/clang/test/Profile/cxx-lambda.cpp 
b/clang/test/Profile/cxx-lambda.cpp
index 1dda64df915d2..dca076a03c7b1 100644
--- a/clang/test/Profile/cxx-lambda.cpp
+++ b/clang/test/Profile/cxx-lambda.cpp
@@ -19,8 +19,8 @@
 void lambdas() {
   int i = 1;
 
-  // LMBGEN-LABEL: define internal{{( [0-9_a-z]*cc)?( noundef zeroext)?}} i1 
@"_ZZ7lambdasvENK3$_0clEi"(
-  // LMBUSE-LABEL: define internal{{( [0-9_a-z]*cc)?( noundef zeroext)?}} i1 
@"_ZZ7lambdasvENK3$_0clEi"(
+  // LMBGEN-LABEL: define internal{{( [0-9_a-z]*cc)?( noundef)?( zeroext)?}} 
i1 @"_ZZ7lambdasvENK3$_0clEi"(
+  // LMBUSE-LABEL: define internal{{( [0-9_a-z]*cc)?( noundef)?( zeroext)?}} 
i1 @"_ZZ7lambdasvENK3$_0clEi"(
   // LMBGEN: store {{.*}} @[[LFC]], i32 0, i32 0
   auto f = [&i](int k) {
 // LMBGEN: store {{.*}} @[[LFC]], i32 0, i32 1



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

[PATCH] D117407: [clang] Add include path for cppwinrt on Windows SDK 10.0.17134+

[Kagami Sascha Rosylight via Phabricator via cfe-commits]

saschanaz added a comment.

That same check decorate_proc_maps.cpp failed on D117405 
 too 🤔


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117407

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

[PATCH] D117390: [AST] Reformat CastExpr unittest suite

[Kim Gräsman via Phabricator via cfe-commits]

kimgr added a comment.

@daverec I don't have commit access, could you help me land this?

I've rebased on `main` without conflicts, and `ninja check-clang` ran 
successfully after rebase.

Let me know if there's anything else I can do as far as prep work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117390

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

[PATCH] D117428: [docs] Clarify & update JSONCompilationDatabase docs

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: hokein.
sammccall requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

- prefer `arguments` over `command`, and add example
- clarify that there's no shell-unescaping of `arguments`

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117428

Files:
  clang/docs/JSONCompilationDatabase.rst


Index: clang/docs/JSONCompilationDatabase.rst
===
--- clang/docs/JSONCompilationDatabase.rst
+++ clang/docs/JSONCompilationDatabase.rst
@@ -63,8 +63,13 @@
 
 [
   { "directory": "/home/user/llvm/build",
-"command": "/usr/bin/clang++ -Irelative -DSOMEDEF=\"With spaces, 
quotes and \\-es.\" -c -o file.o file.cc",
+"arguments": ["/usr/bin/clang++", "-Irelative", "-DSOMEDEF=With 
spaces, quotes and \\-es.", "-c", "-o", "file.o", "file.cc"],
 "file": "file.cc" },
+
+  { "directory": "/home/user/llvm/build",
+"command": "/usr/bin/clang++ -Irelative -DSOMEDEF=\"With spaces, 
quotes and \\-es.\" -c -o file.o file.cc",
+"file": "file2.cc" },
+
   ...
 ]
 
@@ -78,14 +83,17 @@
compilation database. There can be multiple command objects for the
same file, for example if the same source file is compiled with
different configurations.
--  **command:** The compile command executed. After JSON unescaping,
-   this must be a valid command to rerun the exact compilation step for
-   the translation unit in the environment the build system uses.
-   Parameters use shell quoting and shell escaping of quotes, with '``"``'
-   and '``\``' being the only special characters. Shell expansion is not
-   supported.
--  **arguments:** The compile command executed as list of strings.
-   Either **arguments** or **command** is required.
+-  **arguments:** The compile command argv as list of strings.
+   This should run the compilation step for the translation unit ``file``.
+   ``arguments[0]`` should be the executable name, such as ``clang++``.
+   Arguments should not be escaped, but ready to pass to ``execvp()``.
+-  **command:** The compile command as a single shell-escaped string.
+   Arguments may be shell quoted and escaped following platform conventions,
+   with '``"``' and '``\``' being the only special characters. Shell expansion
+   is not supported.
+
+   Either **arguments** or **command** is required. **arguments** is preferred,
+   as shell (un)escaping is a possible source of errors.
 -  **output:** The name of the output created by this compilation step.
This field is optional. It can be used to distinguish different processing
modes of the same input file.


Index: clang/docs/JSONCompilationDatabase.rst
===
--- clang/docs/JSONCompilationDatabase.rst
+++ clang/docs/JSONCompilationDatabase.rst
@@ -63,8 +63,13 @@
 
 [
   { "directory": "/home/user/llvm/build",
-"command": "/usr/bin/clang++ -Irelative -DSOMEDEF=\"With spaces, quotes and \\-es.\" -c -o file.o file.cc",
+"arguments": ["/usr/bin/clang++", "-Irelative", "-DSOMEDEF=With spaces, quotes and \\-es.", "-c", "-o", "file.o", "file.cc"],
 "file": "file.cc" },
+
+  { "directory": "/home/user/llvm/build",
+"command": "/usr/bin/clang++ -Irelative -DSOMEDEF=\"With spaces, quotes and \\-es.\" -c -o file.o file.cc",
+"file": "file2.cc" },
+
   ...
 ]
 
@@ -78,14 +83,17 @@
compilation database. There can be multiple command objects for the
same file, for example if the same source file is compiled with
different configurations.
--  **command:** The compile command executed. After JSON unescaping,
-   this must be a valid command to rerun the exact compilation step for
-   the translation unit in the environment the build system uses.
-   Parameters use shell quoting and shell escaping of quotes, with '``"``'
-   and '``\``' being the only special characters. Shell expansion is not
-   supported.
--  **arguments:** The compile command executed as list of strings.
-   Either **arguments** or **command** is required.
+-  **arguments:** The compile command argv as list of strings.
+   This should run the compilation step for the translation unit ``file``.
+   ``arguments[0]`` should be the executable name, such as ``clang++``.
+   Arguments should not be escaped, but ready to pass to ``execvp()``.
+-  **command:** The compile command as a single shell-escaped string.
+   Arguments may be shell quoted and escaped following platform conventions,
+   with '``"``' and '``\``' being the only special characters. Shell expansion
+   is not supported.
+
+   Either **arguments** or **command** is required. **arguments** is preferred,
+   as shell (un)escaping is a possible source of errors.
 -  **output:** The name of the output created by t

[PATCH] D117428: [docs] Clarify & update JSONCompilationDatabase docs

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang/docs/JSONCompilationDatabase.rst:95
+
+   Either **arguments** or **command** is required. **arguments** is preferred,
+   as shell (un)escaping is a possible source of errors.

Hopefully I'm not too far out on a limb here.

Today, `command` is generated by CMake & others. The shell unescaping performed 
is underspecified and platform-specific (`#if _WIN32 ... #else`). There are 
non-llvm consumers of this format (e.g. vscode-cpptools).

I think we're in danger of interop problems and gently encouraging migration to 
`arguments` reduces the danger.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117428

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

[PATCH] D117423: [AVR][clang] Reject non assembly source files for the avr1 family

[Ben Shi via Phabricator via cfe-commits]

benshi001 updated this revision to Diff 400375.

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

https://reviews.llvm.org/D117423

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/AVR.cpp
  clang/lib/Driver/ToolChains/AVR.h
  clang/test/Driver/avr-mmcu.S
  clang/test/Driver/avr-mmcu.c

Index: clang/test/Driver/avr-mmcu.c
===
--- clang/test/Driver/avr-mmcu.c
+++ clang/test/Driver/avr-mmcu.c
@@ -1,12 +1,15 @@
 // A test for the propagation of the -mmcu option to -cc1 and -cc1as
 
-// RUN: %clang -### -target avr -no-canonical-prefixes -mmcu=attiny11 -save-temps %s 2>&1 | FileCheck -check-prefix=CHECK0 %s
-// CHECK0: clang{{.*}} "-cc1" {{.*}} "-target-cpu" "attiny11"
-// CHECK0: clang{{.*}} "-cc1as" {{.*}} "-target-cpu" "attiny11"
+// RUN: %clang -### -target avr -no-canonical-prefixes -mmcu=attiny11 %s -c 2>&1 | FileCheck -check-prefix=CHECKA %s
+// CHECKA: error: CPU 'attiny11' does not support {{.*}} language mode
+
+// RUN: %clang -### -target avr -no-canonical-prefixes -mmcu=attiny11 -x assembler-with-cpp %s -c 2>&1 | FileCheck -check-prefix=CHECKB %s
+// CHECKB-NOT: error: CPU 'attiny11' does not support {{.*}} language mode
 
 // RUN: %clang -### -target avr -no-canonical-prefixes -mmcu=at90s2313 -save-temps %s 2>&1 | FileCheck -check-prefix=CHECK1 %s
 // CHECK1: clang{{.*}} "-cc1" {{.*}} "-target-cpu" "at90s2313"
 // CHECK1: clang{{.*}} "-cc1as" {{.*}} "-target-cpu" "at90s2313"
+// CHECK1-NOT: error: CPU 'at90s2313' does not support {{.*}} language mode
 
 // RUN: %clang -### -target avr -no-canonical-prefixes -mmcu=at90s8515 -save-temps %s 2>&1 | FileCheck -check-prefix=CHECK2 %s
 // CHECK2: clang{{.*}} "-cc1" {{.*}} "-target-cpu" "at90s8515"
Index: clang/test/Driver/avr-mmcu.S
===
--- /dev/null
+++ clang/test/Driver/avr-mmcu.S
@@ -0,0 +1,13 @@
+// A test for the supported language mode by different avr families.
+
+// RUN: %clang -### -target avr -no-canonical-prefixes -mmcu=attiny11 %s -c 2>&1 | FileCheck -check-prefix=CHECKA %s
+// CHECKA-NOT: error: CPU 'attiny11' does not support {{.*}} language mode
+
+// RUN: %clang -### -target avr -no-canonical-prefixes -mmcu=attiny11 -x c++ %s -c 2>&1 | FileCheck -check-prefix=CHECKB %s
+// CHECKB: error: CPU 'attiny11' does not support {{.*}} language mode
+
+// RUN: %clang -### -target avr -no-canonical-prefixes -mmcu=at90s8515 %s -c 2>&1 | FileCheck -check-prefix=CHECKC %s
+// CHECKC-NOT: error: CPU 'at90s8515' does not support {{.*}} language mode
+
+// RUN: %clang -### -target avr -no-canonical-prefixes -mmcu=at90s8515 %s -x c++ -c 2>&1 | FileCheck -check-prefix=CHECKD %s
+// CHECKD-NOT: error: CPU 'at90s8515' does not support {{.*}} language mode
Index: clang/lib/Driver/ToolChains/AVR.h
===
--- clang/lib/Driver/ToolChains/AVR.h
+++ clang/lib/Driver/ToolChains/AVR.h
@@ -19,6 +19,8 @@
 namespace toolchains {
 
 class LLVM_LIBRARY_VISIBILITY AVRToolChain : public Generic_ELF {
+  std::string AVRMcu;
+
 public:
   AVRToolChain(const Driver &D, const llvm::Triple &Triple,
const llvm::opt::ArgList &Args);
Index: clang/lib/Driver/ToolChains/AVR.cpp
===
--- clang/lib/Driver/ToolChains/AVR.cpp
+++ clang/lib/Driver/ToolChains/AVR.cpp
@@ -12,6 +12,7 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/InputInfo.h"
 #include "clang/Driver/Options.h"
+#include "clang/Driver/Types.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
@@ -19,6 +20,7 @@
 #include "llvm/MC/SubtargetFeature.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
 
 using namespace clang::driver;
 using namespace clang::driver::toolchains;
@@ -311,24 +313,26 @@
 : Generic_ELF(D, Triple, Args), LinkStdlib(false) {
   GCCInstallation.init(Triple, Args);
 
+  // Save the CPU name for further checks.
+  AVRMcu = getCPUName(D, Args, Triple);
+
   // Only add default libraries if the user hasn't explicitly opted out.
   if (!Args.hasArg(options::OPT_nostdlib) &&
   !Args.hasArg(options::OPT_nodefaultlibs) &&
   !Args.hasArg(options::OPT_c /* does not apply when not linking */)) {
-std::string CPU = getCPUName(D, Args, Triple);
 
-if (CPU.empty()) {
+if (AVRMcu.empty()) {
   // We cannot link any standard libraries without an MCU specified.
   D.Diag(diag::warn_drv_avr_mcu_not_specified);
 } else {
-  Optional FamilyName = GetMCUFamilyName(CPU);
+  Optional FamilyName = GetMCUFamilyName(AVRMcu);
   Optional AVRLibcRoot = findAVRLibcInstallation();
 
   if (!FamilyName.hasValue()) {
 // We do not have an entry for this CPU in the family
 // mapping table yet.
 D.Diag(diag::w

[PATCH] D117423: [AVR][clang] Reject non assembly source files for the avr1 family

[Ben Shi via Phabricator via cfe-commits]

benshi001 added inline comments.



Comment at: clang/lib/Driver/ToolChains/AVR.cpp:419
+StringRef Arg(DriverArgs.getArgString(I));
+if (Arg.equals("assembler-with-cpp"))
+  return;

We need not care about '-x c++' or '-x c', which are excluded by above checks.


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

https://reviews.llvm.org/D117423

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

[PATCH] D117423: [AVR][clang] Reject non assembly source files for the avr1 family

[Ben Shi via Phabricator via cfe-commits]

benshi001 added inline comments.



Comment at: clang/lib/Driver/ToolChains/AVR.cpp:417
+  // '-x assembler-with-cpp' if it has not a '.S' suffix.
+  for (unsigned I = 0; I < DriverArgs.size(); I++) {
+StringRef Arg(DriverArgs.getArgString(I));

In this for loop, the key point is which one is found first.

It is OK that `-x assembler-with-cpp` comes first, otherwise an error should be 
reported.


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

https://reviews.llvm.org/D117423

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

[PATCH] D117391: [AST] Ignore implicit nodes in CastExpr::getConversionFunction

[Kim Gräsman via Phabricator via cfe-commits]

kimgr added inline comments.



Comment at: clang/unittests/Tooling/CastExprTest.cpp:111
+  "const char *f() {\n"
+  "  constexpr X x;\n";
+  "  return x;\n"

Oops, spurious semicolon here. Will post a new version once I've tested it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117391

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

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[Marek Kurdej via Phabricator via cfe-commits]

curdeius requested changes to this revision.
curdeius added a comment.

Thanks for having a try on this.
However, I don't like this approach too much. You add many changes and a single 
test. That's not sufficient.
Also, handling C++ keywords in all cases (e.g. `delete` as a function name) 
*may* need to distinguish whether we format a C file or a C++ file. It's 
probably impossible to do this without user input (.h extension is used in both 
languages for example).
We'd maybe need to add C as language option and let the user specify the 
language (`-x c`?).
That in turn may be painful (because not automatic).
But, you may have a better solution.
My 2 cents.




Comment at: .arclint:5
   "type": "script-and-regex",
-  "script-and-regex.script": "bash utils/arcanist/clang-format.sh",
+  "script-and-regex.script": "cmd utils\\arcanist\\clang-format.sh",
   "script-and-regex.regex": 
"/^(?P[[:alpha:]]+)\n(?P[^\n]+)\n(|(?P\\d),(?P\\d)\n(?P.*)\n(?P.*)\n)$/s",

Unrelated. Please revert.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:103
   return 0;
-if (RootToken.isAccessSpecifier(false) ||
+if (RootToken.isAccessSpecifier(Style.Language == FormatStyle::LK_Cpp) ||
+(RootToken.Next &&

Please refactor to e.g. a lambda to ease the understanding. And use a series of 
`if`s and returns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

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

[PATCH] D117391: [AST] Ignore implicit nodes in CastExpr::getConversionFunction

[Kim Gräsman via Phabricator via cfe-commits]

kimgr updated this revision to Diff 400381.
kimgr added a comment.

Fix spurious semicolon


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117391

Files:
  clang/lib/AST/Expr.cpp
  clang/unittests/Tooling/CastExprTest.cpp


Index: clang/unittests/Tooling/CastExprTest.cpp
===
--- clang/unittests/Tooling/CastExprTest.cpp
+++ clang/unittests/Tooling/CastExprTest.cpp
@@ -14,12 +14,19 @@
 
 struct CastExprVisitor : TestVisitor {
   std::function OnExplicitCast;
+  std::function OnCast;
 
   bool VisitExplicitCastExpr(ExplicitCastExpr *Expr) {
 if (OnExplicitCast)
   OnExplicitCast(Expr);
 return true;
   }
+
+  bool VisitCastExpr(CastExpr *Expr) {
+if (OnCast)
+  OnCast(Expr);
+return true;
+  }
 };
 
 TEST(CastExprTest, GetSubExprAsWrittenThroughMaterializedTemporary) {
@@ -54,4 +61,57 @@
   CastExprVisitor::Lang_CXX2a);
 }
 
+// Verify that getConversionFunction looks through a ConstantExpr for implicit
+// constructor conversions (https://github.com/llvm/llvm-project/issues/53044):
+//
+// `-ImplicitCastExpr 'S' 
+// `-ConstantExpr 'S'
+//   |-value: Struct
+//   `-CXXConstructExpr 'S' 'void (const char *)'
+// `-ImplicitCastExpr 'const char *' 
+//   `-StringLiteral 'const char [7]' lvalue "foobar"
+TEST(CastExprTest, GetCtorConversionFunctionThroughConstantExpr) {
+  CastExprVisitor Visitor;
+  Visitor.OnCast = [](CastExpr *Expr) {
+if (Expr->getCastKind() == CK_ConstructorConversion) {
+  auto *Conv = Expr->getConversionFunction();
+  EXPECT_TRUE(isa(Conv))
+  << "Expected CXXConstructorDecl, but saw " << 
Conv->getDeclKindName();
+}
+  };
+  Visitor.runOver("struct X { consteval X(const char *) {} };\n"
+  "void f() { X x = \"foobar\"; }\n",
+  CastExprVisitor::Lang_CXX2a);
+}
+
+// Verify that getConversionFunction looks through a ConstantExpr for implicit
+// user-defined conversions.
+//
+// `-ImplicitCastExpr 'const char *' 
+//   `-ConstantExpr 'const char *'
+// |-value: LValue
+// `-CXXMemberCallExpr 'const char *'
+//   `-MemberExpr '' .operator const char *
+// `-DeclRefExpr 'const X' lvalue Var 'x' 'const X'
+TEST(CastExprTest, GetUserDefinedConversionFunctionThroughConstantExpr) {
+  CastExprVisitor Visitor;
+  Visitor.OnCast = [](CastExpr *Expr) {
+if (Expr->getCastKind() == CK_UserDefinedConversion) {
+  auto *Conv = Expr->getConversionFunction();
+  EXPECT_TRUE(isa(Conv))
+  << "Expected CXXMethodDecl, but saw " << Conv->getDeclKindName();
+}
+  };
+  Visitor.runOver("struct X {\n"
+  "  consteval operator const char *() const {\n"
+  "return nullptr;\n"
+  "  }\n"
+  "};\n"
+  "const char *f() {\n"
+  "  constexpr X x;\n"
+  "  return x;\n"
+  "}\n",
+  CastExprVisitor::Lang_CXX2a);
+}
+
 } // namespace
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -1944,6 +1944,7 @@
 
   for (const CastExpr *E = this; E; E = dyn_cast(SubExpr)) {
 SubExpr = skipImplicitTemporary(E->getSubExpr());
+SubExpr = SubExpr->IgnoreImplicit();
 
 if (E->getCastKind() == CK_ConstructorConversion)
   return cast(SubExpr)->getConstructor();


Index: clang/unittests/Tooling/CastExprTest.cpp
===
--- clang/unittests/Tooling/CastExprTest.cpp
+++ clang/unittests/Tooling/CastExprTest.cpp
@@ -14,12 +14,19 @@
 
 struct CastExprVisitor : TestVisitor {
   std::function OnExplicitCast;
+  std::function OnCast;
 
   bool VisitExplicitCastExpr(ExplicitCastExpr *Expr) {
 if (OnExplicitCast)
   OnExplicitCast(Expr);
 return true;
   }
+
+  bool VisitCastExpr(CastExpr *Expr) {
+if (OnCast)
+  OnCast(Expr);
+return true;
+  }
 };
 
 TEST(CastExprTest, GetSubExprAsWrittenThroughMaterializedTemporary) {
@@ -54,4 +61,57 @@
   CastExprVisitor::Lang_CXX2a);
 }
 
+// Verify that getConversionFunction looks through a ConstantExpr for implicit
+// constructor conversions (https://github.com/llvm/llvm-project/issues/53044):
+//
+// `-ImplicitCastExpr 'S' 
+// `-ConstantExpr 'S'
+//   |-value: Struct
+//   `-CXXConstructExpr 'S' 'void (const char *)'
+// `-ImplicitCastExpr 'const char *' 
+//   `-StringLiteral 'const char [7]' lvalue "foobar"
+TEST(CastExprTest, GetCtorConversionFunctionThroughConstantExpr) {
+  CastExprVisitor Visitor;
+  Visitor.OnCast = [](CastExpr *Expr) {
+if (Expr->getCastKind() == CK_ConstructorConversion) {
+  auto *Conv = Expr->getConversionFunction();
+  EXPECT_TRUE(isa(Conv))
+  << "Expected CXXConstructorDecl, but saw " << Conv

[PATCH] D117390: [AST] Reformat CastExpr unittest suite

[David Rector via Phabricator via cfe-commits]

davrec added a reviewer: rsmith.
davrec added a subscriber: rsmith.
davrec added a comment.

In D117390#3246799 , @kimgr wrote:

> @daverec I don't have commit access, could you help me land this?
>
> I've rebased on `main` without conflicts, and `ninja check-clang` ran 
> successfully after rebase.
>
> Let me know if there's anything else I can do as far as prep work.

@rsmith is the code owner, and would know best about prep, though he's not 
always available - Richard could you spare a glance and point @kimgr to any 
additional steps needed to land this and D117391 
?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117390

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

[PATCH] D117391: [AST] Ignore implicit nodes in CastExpr::getConversionFunction

[Kim Gräsman via Phabricator via cfe-commits]

kimgr added a comment.

- Update broken test case
- Rebase on latest main (8eb74626f 
)
- Build and run `ninja check-clang`

I don't have commit access, so I would appreciate help landing. Let me know if 
there's anything else I can do before.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117391

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

[PATCH] D117091: [Clang] Add attributes alloc_size and alloc_align to mm_malloc

[Florian Hahn via Phabricator via cfe-commits]

fhahn added a comment.

Not really familiar with testing for clang headers. Is it possible to test the 
new attributes have the desired effect?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117091

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

[PATCH] D117431: [IRBuilder] Migrate and-folding to value-based FoldAnd.

[Florian Hahn via Phabricator via cfe-commits]

fhahn created this revision.
fhahn added reviewers: reames, nikic, lebedev.ri, nhaehnle.
Herald added a subscriber: dexonsmith.
fhahn requested review of this revision.
Herald added projects: clang, LLVM.
Herald added a subscriber: cfe-commits.

Similar to the migration of or-folding to FoldOr, there are a few cases
where the fold in IRBuilder::CreateAnd triggered directly. Those have
been updated.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117431

Files:
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset.c
  clang/test/CodeGen/catch-pointer-overflow.c
  clang/test/CodeGen/cmse-clear-return.c
  llvm/include/llvm/Analysis/InstSimplifyFolder.h
  llvm/include/llvm/Analysis/TargetFolder.h
  llvm/include/llvm/IR/ConstantFolder.h
  llvm/include/llvm/IR/IRBuilder.h
  llvm/include/llvm/IR/IRBuilderFolder.h
  llvm/include/llvm/IR/NoFolder.h
  llvm/test/Transforms/LoopIdiom/X86/left-shift-until-bittest.ll
  llvm/test/Transforms/LoopVersioning/bound-check-partially-known.ll

Index: llvm/test/Transforms/LoopVersioning/bound-check-partially-known.ll
===
--- llvm/test/Transforms/LoopVersioning/bound-check-partially-known.ll
+++ llvm/test/Transforms/LoopVersioning/bound-check-partially-known.ll
@@ -17,12 +17,10 @@
 ; CHECK-NEXT:[[SCEVGEP4:%.*]] = getelementptr [[STRUCT_FOO]], %struct.foo* @global, i64 0, i32 0, i64 [[TMP1]]
 ; CHECK-NEXT:[[SCEVGEP45:%.*]] = bitcast double* [[SCEVGEP4]] to i8*
 ; CHECK-NEXT:[[BOUND1:%.*]] = icmp ult i8* bitcast (%struct.foo* @global to i8*), [[SCEVGEP23]]
-; CHECK-NEXT:[[FOUND_CONFLICT:%.*]] = and i1 false, [[BOUND1]]
 ; CHECK-NEXT:[[BOUND06:%.*]] = icmp ult i8* [[SCEVGEP1]], [[SCEVGEP45]]
 ; CHECK-NEXT:[[BOUND17:%.*]] = icmp ult i8* bitcast (double* getelementptr inbounds ([[STRUCT_FOO]], %struct.foo* @global, i64 0, i32 1, i64 0) to i8*), [[SCEVGEP23]]
 ; CHECK-NEXT:[[FOUND_CONFLICT8:%.*]] = and i1 [[BOUND06]], [[BOUND17]]
-; CHECK-NEXT:[[CONFLICT_RDX:%.*]] = or i1 [[FOUND_CONFLICT]], [[FOUND_CONFLICT8]]
-; CHECK-NEXT:br i1 [[CONFLICT_RDX]], label %loop.ph.lver.orig, label %loop.ph
+; CHECK-NEXT:br i1 [[FOUND_CONFLICT8]], label %loop.ph.lver.orig, label %loop.ph
 ;
 entry:
   %N.ext = zext i32 %N to i64
Index: llvm/test/Transforms/LoopIdiom/X86/left-shift-until-bittest.ll
===
--- llvm/test/Transforms/LoopIdiom/X86/left-shift-until-bittest.ll
+++ llvm/test/Transforms/LoopIdiom/X86/left-shift-until-bittest.ll
@@ -581,10 +581,11 @@
 define void @p8_constant_mask_signbit_noncanonical(i32 %x, i32* %p0, i32* %p1) {
 ; LZCNT-LABEL: @p8_constant_mask_signbit_noncanonical(
 ; LZCNT-NEXT:  entry:
-; LZCNT-NEXT:[[X_NUMLEADINGZEROS:%.*]] = call i32 @llvm.ctlz.i32(i32 [[X:%.*]], i1 true), !dbg [[DBG142:![0-9]+]]
-; LZCNT-NEXT:[[X_NUMACTIVEBITS:%.*]] = sub nuw nsw i32 32, [[X_NUMLEADINGZEROS]], !dbg [[DBG142]]
-; LZCNT-NEXT:[[X_LEADINGONEPOS:%.*]] = add nsw i32 [[X_NUMACTIVEBITS]], -1, !dbg [[DBG142]]
-; LZCNT-NEXT:[[LOOP_BACKEDGETAKENCOUNT:%.*]] = sub nuw nsw i32 31, [[X_LEADINGONEPOS]], !dbg [[DBG142]]
+; LZCNT-NEXT:[[X_MASKED:%.*]] = and i32 [[X:%.*]], -1, !dbg [[DBG142:![0-9]+]]
+; LZCNT-NEXT:[[X_MASKED_NUMLEADINGZEROS:%.*]] = call i32 @llvm.ctlz.i32(i32 [[X_MASKED]], i1 true), !dbg [[DBG142]]
+; LZCNT-NEXT:[[X_MASKED_NUMACTIVEBITS:%.*]] = sub nuw nsw i32 32, [[X_MASKED_NUMLEADINGZEROS]], !dbg [[DBG142]]
+; LZCNT-NEXT:[[X_MASKED_LEADINGONEPOS:%.*]] = add nsw i32 [[X_MASKED_NUMACTIVEBITS]], -1, !dbg [[DBG142]]
+; LZCNT-NEXT:[[LOOP_BACKEDGETAKENCOUNT:%.*]] = sub nuw nsw i32 31, [[X_MASKED_LEADINGONEPOS]], !dbg [[DBG142]]
 ; LZCNT-NEXT:[[LOOP_TRIPCOUNT:%.*]] = add nuw nsw i32 [[LOOP_BACKEDGETAKENCOUNT]], 1, !dbg [[DBG142]]
 ; LZCNT-NEXT:[[X_CURR:%.*]] = shl i32 [[X]], [[LOOP_BACKEDGETAKENCOUNT]], !dbg [[DBG142]]
 ; LZCNT-NEXT:[[X_NEXT:%.*]] = shl i32 [[X_CURR]], 1, !dbg [[DBG142]]
@@ -648,10 +649,11 @@
 define void @p9_constant_mask_signbit_canonical(i32 %x, i32* %p0, i32* %p1) {
 ; LZCNT-LABEL: @p9_constant_mask_signbit_canonical(
 ; LZCNT-NEXT:  entry:
-; LZCNT-NEXT:[[X_NUMLEADINGZEROS:%.*]] = call i32 @llvm.ctlz.i32(i32 [[X:%.*]], i1 true), !dbg [[DBG156:![0-9]+]]
-; LZCNT-NEXT:[[X_NUMACTIVEBITS:%.*]] = sub nuw nsw i32 32, [[X_NUMLEADINGZEROS]], !dbg [[DBG156]]
-; LZCNT-NEXT:[[X_LEADINGONEPOS:%.*]] = add nsw i32 [[X_NUMACTIVEBITS]], -1, !dbg [[DBG156]]
-; LZCNT-NEXT:[[LOOP_BACKEDGETAKENCOUNT:%.*]] = sub nuw nsw i32 31, [[X_LEADINGONEPOS]], !dbg [[DBG156]]
+; LZCNT-NEXT:[[X_MASKED:%.*]] = and i32 [[X:%.*]], -1, !dbg [[DBG156:![0-9]+]]
+; LZCNT-NEXT:[[X_MASKED_NUMLEADINGZEROS:%.*]] = call i32 @llvm.ctlz.i32(i32 [[X_MASKED]], i1 true), !dbg [[DBG156]]
+; LZCNT-NEXT:[[X_MASKED_NUMACTIVEBITS:%.*]] = sub nuw nsw i32 32, [[X_MASKED_NUMLEADINGZEROS]], !dbg [[DBG156]]
+; LZCNT-NEXT:[[X_MASKED_LEADINGONEPOS:%.*]] = add nsw i32 [[X_MASKE

[PATCH] D117431: [IRBuilder] Migrate and-folding to value-based FoldAnd.

[Nikita Popov via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117431

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

[PATCH] D117091: [Clang] Add attributes alloc_size and alloc_align to mm_malloc

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Yes, test for alignment should be possible, something like

  _Bool alig_test(void) {
  // CHECK: ret i1 true
   yvoid *p = _mm_malloc(2014, 16);
  _Bool ret = ((__SIZE_TYPE__)p % 16) == 0;
  _mm_free(p);
  return ret;
  }

For alloc size not sure how..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117091

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

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

psigillito added a comment.

In D117416#3246838 , @curdeius wrote:

> Thanks for having a try on this.
> However, I don't like this approach too much. You add many changes and a 
> single test. That's not sufficient.
> Also, handling C++ keywords in all cases (e.g. `delete` as a function name) 
> *may* need to distinguish whether we format a C file or a C++ file. It's 
> probably impossible to do this without user input (.h extension is used in 
> both languages for example).
> We'd maybe need to add C as language option and let the user specify the 
> language (`-x c`?).
> That in turn may be painful (because not automatic).
> But, you may have a better solution.
> My 2 cents.

Thanks, I am just starting to understand some of the code base so my changes 
should be taken with a grain of salt. The more I think about this, I do not 
think my change is correct. As you pointed out with the 'delete' keyword, my 
changes would not correctly handle a struct named 'delete' i.e.

  struct delete foo = {0};
  delete.val; 

I think adding a cmd line argument specifying you are using C code is more 
correct. If the user does not specify they are using C, clang-format would 
format the code as it does now.

The only thing I don't like about this is that it might be kind of clumsy to 
only be able to specify your language is C. I can take a stab at implementing 
this if this is the route we want to go.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

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

[PATCH] D117421: Fix 31568 (i.e. incorrect alignment of operator= overloads).

[Elliott Maguire via Phabricator via cfe-commits]

glotchimo created this revision.
glotchimo added a comment.
glotchimo updated this revision to Diff 400390.
glotchimo added reviewers: MyDeveloperDay, djasper.
glotchimo added a project: clang-format.
glotchimo added a subscriber: cfe-commits.
glotchimo published this revision for review.
Herald added a project: clang.

I don't know why, but `clang-format` reformatted most if not all of the 
long/block comments in `WhitespaceManager.cpp`. Will it be necessary for me to 
revert the changes to those comments and skip formatting when updating the diff 
so as to keep it minimal?


glotchimo added a comment.

Same change as before but without the changes to long and block comments.


Added a look-ahead for brackets in the operator= context to avoid incorrect 
alignment.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117421

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
@@ -16167,6 +16167,14 @@
   verifyFormat("int oneTwoThree = 123; // comment\n"
"int oneTwo  = 12;  // comment",
Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default; // comment\n"
+   "int &operator() = default; // comment\n"
+   "int &operator=() {",
+   Alignment);
 
   // Bug 25167
   /* Uncomment when fixed
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -731,6 +731,16 @@
 if (&C != &Changes.back() && (&C + 1)->NewlinesBefore > 0)
   return false;
 
+// Do not align operator= overloads.
+if (C.Tok->Previous && C.Tok->Previous->is(tok::kw_operator)) {
+  auto *Next = C.Tok->Next;
+  while (Next && Next->NewlinesBefore == 0) {
+if (Next->Tok.is(tok::l_brace))
+  return false;
+Next = Next->Next;
+  }
+}
+
 return C.Tok->is(tok::equal);
   },
   Changes, /*StartAt=*/0, Style.AlignConsecutiveAssignments);


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16167,6 +16167,14 @@
   verifyFormat("int oneTwoThree = 123; // comment\n"
"int oneTwo  = 12;  // comment",
Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default; // comment\n"
+   "int &operator() = default; // comment\n"
+   "int &operator=() {",
+   Alignment);
 
   // Bug 25167
   /* Uncomment when fixed
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -731,6 +731,16 @@
 if (&C != &Changes.back() && (&C + 1)->NewlinesBefore > 0)
   return false;
 
+// Do not align operator= overloads.
+if (C.Tok->Previous && C.Tok->Previous->is(tok::kw_operator)) {
+  auto *Next = C.Tok->Next;
+  while (Next && Next->NewlinesBefore == 0) {
+if (Next->Tok.is(tok::l_brace))
+  return false;
+Next = Next->Next;
+  }
+}
+
 return C.Tok->is(tok::equal);
   },
   Changes, /*StartAt=*/0, Style.AlignConsecutiveAssignments);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117339: [clang][dataflow] Add transfer functions for bind temporary and static cast

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev updated this revision to Diff 400392.
sgatev marked 2 inline comments as done.
sgatev added a comment.

Address reviewers' comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117339

Files:
  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
@@ -489,6 +489,44 @@
   });
 }
 
+TEST_F(TransferTest, MultipleVarsDecl) {
+  std::string Code = R"(
+void target() {
+  int Foo, Bar;
+  (void)0;
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const StorageLocation *FooLoc =
+Env.getStorageLocation(*FooDecl, SkipPast::None);
+ASSERT_TRUE(isa_and_nonnull(FooLoc));
+
+const StorageLocation *BarLoc =
+Env.getStorageLocation(*BarDecl, SkipPast::None);
+ASSERT_TRUE(isa_and_nonnull(BarLoc));
+
+const Value *FooVal = Env.getValue(*FooLoc);
+EXPECT_TRUE(isa_and_nonnull(FooVal));
+
+const Value *BarVal = Env.getValue(*BarLoc);
+EXPECT_TRUE(isa_and_nonnull(BarVal));
+  });
+}
+
 TEST_F(TransferTest, JoinVarDecl) {
   std::string Code = R"(
 void target(bool B) {
@@ -1589,4 +1627,71 @@
   });
 }
 
+TEST_F(TransferTest, BindTemporary) {
+  std::string Code = R"(
+struct A {
+  virtual ~A() = default;
+
+  int Baz;
+};
+
+void target(A Foo) {
+  int Bar = A(Foo).Baz;
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
+ASSERT_THAT(BazDecl, NotNull());
+
+const auto &FooVal =
+*cast(Env.getValue(*FooDecl, SkipPast::None));
+const auto *BarVal =
+cast(Env.getValue(*BarDecl, SkipPast::None));
+EXPECT_EQ(BarVal, &FooVal.getChild(*BazDecl));
+  });
+}
+
+TEST_F(TransferTest, StaticCast) {
+  std::string Code = R"(
+void target(int Foo) {
+  int Bar = static_cast(Foo);
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *FooVal =
+cast(Env.getValue(*FooDecl, SkipPast::None));
+const auto *BarVal =
+cast(Env.getValue(*BarDecl, SkipPast::None));
+EXPECT_EQ(FooVal, BarVal);
+  });
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -87,11 +87,50 @@
   }
 
   void VisitDeclStmt(const DeclStmt *S) {
-// FIXME: Add support for group decls, e.g: `int a, b;`
-if (S->isSingleDecl()) {
-  if (const auto *D = dyn_cast(S->getSingleDecl())) {
-visitVarDecl(*D);
+// Group decls are converted into single decls in the CFG so the cast below
+// is safe.
+const auto &D = *cast(S->getSingleDecl());
+auto &Loc = Env.createStorageLocation(D);
+Env.setStorageLo

[PATCH] D117339: [clang][dataflow] Add transfer functions for bind temporary and static cast

[Stanislav Gatev 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 rG37e6496c800b: [clang][dataflow] Add transfer functions for 
bind temporary and static cast (authored by sgatev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117339

Files:
  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
@@ -489,6 +489,44 @@
   });
 }
 
+TEST_F(TransferTest, MultipleVarsDecl) {
+  std::string Code = R"(
+void target() {
+  int Foo, Bar;
+  (void)0;
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const StorageLocation *FooLoc =
+Env.getStorageLocation(*FooDecl, SkipPast::None);
+ASSERT_TRUE(isa_and_nonnull(FooLoc));
+
+const StorageLocation *BarLoc =
+Env.getStorageLocation(*BarDecl, SkipPast::None);
+ASSERT_TRUE(isa_and_nonnull(BarLoc));
+
+const Value *FooVal = Env.getValue(*FooLoc);
+EXPECT_TRUE(isa_and_nonnull(FooVal));
+
+const Value *BarVal = Env.getValue(*BarLoc);
+EXPECT_TRUE(isa_and_nonnull(BarVal));
+  });
+}
+
 TEST_F(TransferTest, JoinVarDecl) {
   std::string Code = R"(
 void target(bool B) {
@@ -1589,4 +1627,71 @@
   });
 }
 
+TEST_F(TransferTest, BindTemporary) {
+  std::string Code = R"(
+struct A {
+  virtual ~A() = default;
+
+  int Baz;
+};
+
+void target(A Foo) {
+  int Bar = A(Foo).Baz;
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
+ASSERT_THAT(BazDecl, NotNull());
+
+const auto &FooVal =
+*cast(Env.getValue(*FooDecl, SkipPast::None));
+const auto *BarVal =
+cast(Env.getValue(*BarDecl, SkipPast::None));
+EXPECT_EQ(BarVal, &FooVal.getChild(*BazDecl));
+  });
+}
+
+TEST_F(TransferTest, StaticCast) {
+  std::string Code = R"(
+void target(int Foo) {
+  int Bar = static_cast(Foo);
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *FooVal =
+cast(Env.getValue(*FooDecl, SkipPast::None));
+const auto *BarVal =
+cast(Env.getValue(*BarDecl, SkipPast::None));
+EXPECT_EQ(FooVal, BarVal);
+  });
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -87,11 +87,50 @@
   }
 
   void VisitDeclStmt(const DeclStmt *S) {
-// FIXME: Add support for group decls, e.g: `int a, b;`
-if (S->isSingleDecl()) {
-  if (const auto *D = dyn_cast(S->getSingleDecl())) {
-visitVarDecl(*D);
+// Group decls are converted into single decls in the CFG so the cast below
+// is s

[clang] 37e6496 - [clang][dataflow] Add transfer functions for bind temporary and static cast

[Stanislav Gatev via cfe-commits]

Author: Stanislav Gatev
Date: 2022-01-16T17:41:02Z
New Revision: 37e6496c800b33cbf6f7967d90eab53327147478

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

LOG: [clang][dataflow] Add transfer functions for bind temporary and static cast

This is part of the implementation of the dataflow analysis framework.
See "[RFC] A dataflow analysis framework for Clang AST" on cfe-dev.

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

Added: 


Modified: 
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp 
b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 8b35ec023ecc..cc73fb09ffd9 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -87,11 +87,50 @@ class TransferVisitor : public 
ConstStmtVisitor {
   }
 
   void VisitDeclStmt(const DeclStmt *S) {
-// FIXME: Add support for group decls, e.g: `int a, b;`
-if (S->isSingleDecl()) {
-  if (const auto *D = dyn_cast(S->getSingleDecl())) {
-visitVarDecl(*D);
+// Group decls are converted into single decls in the CFG so the cast below
+// is safe.
+const auto &D = *cast(S->getSingleDecl());
+auto &Loc = Env.createStorageLocation(D);
+Env.setStorageLocation(D, Loc);
+
+const Expr *InitExpr = D.getInit();
+if (InitExpr == nullptr) {
+  // No initializer expression - associate `Loc` with a new value.
+  Env.initValueInStorageLocation(Loc, D.getType());
+  return;
+}
+
+// The CFG does not contain `ParenExpr` as top-level statements in basic
+// blocks, however sub-expressions can still be of that type.
+InitExpr = skipExprWithCleanups(D.getInit()->IgnoreParens());
+assert(InitExpr != nullptr);
+
+if (D.getType()->isReferenceType()) {
+  // Initializing a reference variable - do not create a reference to
+  // reference.
+  if (auto *InitExprLoc =
+  Env.getStorageLocation(*InitExpr, SkipPast::Reference)) {
+auto &Val =
+Env.takeOwnership(std::make_unique(*InitExprLoc));
+Env.setValue(Loc, Val);
+  } else {
+// FIXME: The initializer expression must always be assigned a value.
+// Replace this with an assert when we have sufficient coverage of
+// language features.
+Env.initValueInStorageLocation(Loc, D.getType());
   }
+  return;
+}
+
+if (auto *InitExprVal = Env.getValue(*InitExpr, SkipPast::None)) {
+  Env.setValue(Loc, *InitExprVal);
+} else if (!D.getType()->isStructureOrClassType()) {
+  // FIXME: The initializer expression must always be assigned a value.
+  // Replace this with an assert when we have sufficient coverage of
+  // language features.
+  Env.initValueInStorageLocation(Loc, D.getType());
+} else {
+  llvm_unreachable("structs and classes must always be assigned values");
 }
   }
 
@@ -309,57 +348,34 @@ class TransferVisitor : public 
ConstStmtVisitor {
 Env.setStorageLocation(*S, *SubExprLoc);
   }
 
-  // FIXME: Add support for:
-  // - CXXBindTemporaryExpr
-  // - CXXBoolLiteralExpr
-  // - CXXStaticCastExpr
-
-private:
-  void visitVarDecl(const VarDecl &D) {
-auto &Loc = Env.createStorageLocation(D);
-Env.setStorageLocation(D, Loc);
+  void VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *S) {
+const Expr *SubExpr = S->getSubExpr();
+assert(SubExpr != nullptr);
 
-const Expr *InitExpr = D.getInit();
-if (InitExpr == nullptr) {
-  // No initializer expression - associate `Loc` with a new value.
-  Env.initValueInStorageLocation(Loc, D.getType());
+auto *SubExprLoc = Env.getStorageLocation(*SubExpr, SkipPast::None);
+if (SubExprLoc == nullptr)
   return;
-}
 
-// The CFG does not contain `ParenExpr` as top-level statements in basic
-// blocks, however sub-expressions can still be of that type.
-InitExpr = skipExprWithCleanups(D.getInit()->IgnoreParens());
-assert(InitExpr != nullptr);
+Env.setStorageLocation(*S, *SubExprLoc);
+  }
 
-if (D.getType()->isReferenceType()) {
-  // Initializing a reference variable - do not create a reference to
-  // reference.
-  if (auto *InitExprLoc =
-  Env.getStorageLocation(*InitExpr, SkipPast::Reference)) {
-auto &Val =
-Env.takeOwnership(std::make_unique(*InitExprLoc));
-Env.setValue(Loc, Val);
-  } else {
-// FIXME: The initializer expression must always be assigned a value.
-// Replace this with an assert when we have sufficient coverage of
-// language features.
-Env.initValueInStorageLocation(Loc, D.g

[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

Thanks for working on this!
I'll have a closer look tomorrow or on Tuesday.




Comment at: clang/lib/Format/WhitespaceManager.cpp:735
+// Do not align operator= overloads.
+if (C.Tok->Previous && C.Tok->Previous->is(tok::kw_operator)) {
+  auto *Next = C.Tok->Next;

Maybe `getPreviousNonComment`? Cf. test comment.



Comment at: clang/lib/Format/WhitespaceManager.cpp:736
+if (C.Tok->Previous && C.Tok->Previous->is(tok::kw_operator)) {
+  auto *Next = C.Tok->Next;
+  while (Next && Next->NewlinesBefore == 0) {

Please spell out the type instead of auto.



Comment at: clang/unittests/Format/FormatTest.cpp:16172
+   "int &operator() = default;\n"
+   "int &operator=() {",
+   Alignment);

Are other operators impacted too by this bug? Like ==, <=, !=.



Comment at: clang/unittests/Format/FormatTest.cpp:16174
+   Alignment);
+  verifyFormat("int()   = default; // comment\n"
+   "int &operator() = default; // comment\n"

What happens if there's a block comment between `operator` and `=`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117421

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

[PATCH] D117435: [clang] Warning for inline constexpr functions

[Evgeny Shulgin via Phabricator via cfe-commits]

Izaron created this revision.
Izaron added reviewers: rjmccall, rsmith.
Izaron requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

"inline constexpr" and "inline consteval" function declarations are redundant 
and shall be written rather as "constexpr" and "consteval".


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117435

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Parser/cxx0x-decl.cpp


Index: clang/test/Parser/cxx0x-decl.cpp
===
--- clang/test/Parser/cxx0x-decl.cpp
+++ clang/test/Parser/cxx0x-decl.cpp
@@ -172,6 +172,15 @@
   consteval constinit int huh(); // expected-error {{cannot combine with 
previous 'consteval'}}
 }
 
+namespace RedundantSpecifier {
+  consteval inline int f1(); // expected-warning {{'inline' declaration 
specifier is redundant due to the presence of stricter 'consteval' declaration 
specifier}}
+  inline constexpr int f2(); // expected-warning {{'inline' declaration 
specifier is redundant due to the presence of stricter 'constexpr' declaration 
specifier}}
+  inline constexpr inline int f3(); // expected-warning {{'inline' declaration 
specifier is redundant due to the presence of stricter 'constexpr' declaration 
specifier}} \
+// expected-warning {{duplicate 'inline' 
declaration specifier}}
+  template inline consteval int f4(T t); // expected-warning 
{{'inline' declaration specifier is redundant due to the presence of stricter 
'consteval' declaration specifier}}
+  inline constexpr int i1 = 0;
+}
+
 namespace ColonColonDecltype {
   struct S { struct T {}; };
   ::decltype(S())::T invalid; // expected-error {{expected unqualified-id}}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -6209,6 +6209,20 @@
  diag::err_noreturn_non_function);
 }
 
+/// Diagnose specifiers on a declaration of an identifier that
+/// are redundant due to the presence of stricter specifiers
+void Sema::DiagnoseRedundantSpecifiers(QualType T, const DeclSpec &DS) {
+  if (T->isFunctionType()) {
+// [dcl.constexpr]p1: A function declared with the constexpr or consteval
+// specifier is implicitly an inline function
+if (DS.hasConstexprSpecifier() && DS.isInlineSpecified()) {
+  Diag(DS.getInlineSpecLoc(),
+   diag::warn_redundant_weaker_declspec_constexpr)
+  << "inline" << static_cast(DS.getConstexprSpecifier());
+}
+  }
+}
+
 NamedDecl*
 Sema::ActOnTypedefDeclarator(Scope* S, Declarator& D, DeclContext* DC,
  TypeSourceInfo *TInfo, LookupResult &Previous) {
@@ -9046,6 +9060,8 @@
   if (R.getCanonicalType()->castAs()->getCmseNSCallAttr())
 Diag(D.getIdentifierLoc(), diag::err_function_decl_cmse_ns_call);
 
+  DiagnoseRedundantSpecifiers(R, D.getDeclSpec());
+
   SmallVector TemplateParamLists;
   for (TemplateParameterList *TPL : TemplateParamListsRef)
 TemplateParamLists.push_back(TPL);
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -2707,6 +2707,7 @@
 
   static bool adjustContextForLocalExternDecl(DeclContext *&DC);
   void DiagnoseFunctionSpecifiers(const DeclSpec &DS);
+  void DiagnoseRedundantSpecifiers(QualType T, const DeclSpec &DS);
   NamedDecl *getShadowedDeclaration(const TypedefNameDecl *D,
 const LookupResult &R);
   NamedDecl *getShadowedDeclaration(const VarDecl *D, const LookupResult &R);
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -641,6 +641,7 @@
  TautologicalObjCBoolCompare]>;
 def HeaderHygiene : DiagGroup<"header-hygiene">;
 def DuplicateDeclSpecifier : DiagGroup<"duplicate-decl-specifier">;
+def RedundantDeclSpecifier : DiagGroup<"redundant-decl-specifier">;
 def CompareDistinctPointerType : DiagGroup<"compare-distinct-pointer-types">;
 def GNUUnionCast : DiagGroup<"gnu-union-cast">;
 def GNUVariableSizedTypeNotAtEnd : 
DiagGroup<"gnu-variable-sized-type-not-at-end">;
Index: clang/include/clang/Basic/DiagnosticCommonKinds.td
===
--- clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -169,6 +169,11 @@
 
 def err_duplicate_declspec : Error<"%sub{duplicate_declspec}0">;
 
+def warn_redundant_weaker_declspec_constexpr : Warning<
+  "'%0' declaration specifier is redundant d

[PATCH] D117388: [Driver][FreeBSD] -r: imply -nostdlib like GCC

[Dimitry Andric via Phabricator via cfe-commits]

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

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117388

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

[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

[Elliott Maguire via Phabricator via cfe-commits]

glotchimo added a comment.

Excellent, thank you! I'll get right on these edits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117421

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

[PATCH] D117435: [clang] Warning for inline constexpr functions

[Evgeny Shulgin via Phabricator via cfe-commits]

Izaron added a comment.

I will provide more context in this comment.

**Why this patch?**
Clang currently has a warning for duplicating specifiers (`inline inline`, 
`extern extern`, `consteval consteval`, etc.), but has no warnings for 
redundant weaker specifiers.
I suggest to add such a group of warnings, starting  from `inline 
constexpr`/`inline consteval` functions.

The Standard says (dcl.constexpr 
) that 
`constexpr`/`consteval` functions are implicitly `inline`. But Clang doesn't 
tell programmers that.
The volume of affected code can be seen in GitHub Search: inline constexpr 
, inline consteval 
.

**What about the other meaning of `inline`?**
Some years ago, the `inline` keyword meant not only "there may be multiple 
definitions", but also "there will be `inlinehint` attribute added in LLVM IR".
Fortunately, this is no more since Clang 3.3 release.
`constexpr` and `inline constexpr` functions emit the same LLVM IR attributes. 
(`consteval` function aren't even emitted to LLVM IR, executing exclusively at 
compile-time)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117435

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

[PATCH] D112408: [RISCV][MC] Add the zve extension according to the v1.0 spec

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: llvm/include/llvm/Support/RISCVISAInfo.h:91
   void updateMinVLen();
+  void updateMaxEew();
 };

There's no definition for this



Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:821
 
-let Predicates = [HasStdExtV, IsRV64] in {
-  // Vector Indexed Instructions

Is this deletion correct? I don't see where the VLUXEI64 instructions are 
declared now.



Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:871
+
+def : InstAlias<"vl1r.v $vd, (${rs1})", (VL1RE8_V VR:$vd, GPR:$rs1)>;
+def : InstAlias<"vl2r.v $vd, (${rs1})", (VL2RE8_V VRM2:$vd, GPR:$rs1)>;

Why is there no Predicate on these aliases?



Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:1509
 
-let Predicates = [HasStdExtZvlsseg, IsRV64] in {
+let Predicates = [HasStdExtZvlsseg, HasVInstructionsI64] in {
   foreach nf=2-8 in {

These require RV64 don't they?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112408

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

[PATCH] D112408: [RISCV][MC] Add the zve extension according to the v1.0 spec

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:353
+
+multiclass VIndexLoadStore EEWList> {
+  foreach n = EEWList in {

Why is this class only used for [64]



Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:821
 
-let Predicates = [HasStdExtV, IsRV64] in {
-  // Vector Indexed Instructions

craig.topper wrote:
> Is this deletion correct? I don't see where the VLUXEI64 instructions are 
> declared now.
Oh I see, it's done with `VIndexLoadStore<[64]>`.



Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:893
+// Vector Indexed Instructions
+defm "" : VIndexLoadStore<[64]>;
 

EEW64 indexed loads/stores also require RV64


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112408

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

[PATCH] D117435: [clang] Warning for inline constexpr functions

[Evgeny Shulgin via Phabricator via cfe-commits]

Izaron updated this revision to Diff 400399.
Izaron added a comment.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.

Fix tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117435

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/test/AST/ast-dump-openmp-begin-declare-variant_10.c
  clang/test/Analysis/builtin_bitcast.cpp
  clang/test/CXX/temp/temp.deduct.guide/p1.cpp
  clang/test/Headers/x86-intrinsics-headers-clean.cpp
  clang/test/Parser/cxx0x-decl.cpp
  clang/test/SemaCUDA/function-overload.cu
  clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
  clang/test/SemaCXX/cxx2a-destroying-delete.cpp
  clang/test/SemaCXX/extended-usual-deallocation-functions.cpp
  clang/test/SemaCXX/static-assert-cxx17.cpp

Index: clang/test/SemaCXX/static-assert-cxx17.cpp
===
--- clang/test/SemaCXX/static-assert-cxx17.cpp
+++ clang/test/SemaCXX/static-assert-cxx17.cpp
@@ -15,7 +15,7 @@
 };
 
 template 
-inline constexpr bool constexpr_return_false() {
+constexpr bool constexpr_return_false() {
   return false;
 }
 
Index: clang/test/SemaCXX/extended-usual-deallocation-functions.cpp
===
--- clang/test/SemaCXX/extended-usual-deallocation-functions.cpp
+++ clang/test/SemaCXX/extended-usual-deallocation-functions.cpp
@@ -16,7 +16,7 @@
   explicit destroying_delete_t(__construct) {}
 };
 
-inline constexpr destroying_delete_t destroying_delete(destroying_delete_t::__construct());
+constexpr destroying_delete_t destroying_delete(destroying_delete_t::__construct());
 }
 
 // FIXME: Should destroying delete really be on in all dialects by default?
Index: clang/test/SemaCXX/cxx2a-destroying-delete.cpp
===
--- clang/test/SemaCXX/cxx2a-destroying-delete.cpp
+++ clang/test/SemaCXX/cxx2a-destroying-delete.cpp
@@ -10,7 +10,7 @@
 explicit destroying_delete_t(__construct) {}
   };
 
-  inline constexpr destroying_delete_t destroying_delete(destroying_delete_t::__construct());
+  constexpr destroying_delete_t destroying_delete(destroying_delete_t::__construct());
 }
 
 void operator delete(void*, std::destroying_delete_t); // ok, just a placement delete
Index: clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
===
--- clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
+++ clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
@@ -5,7 +5,7 @@
 using size_t = decltype(sizeof(int));
 
 namespace std {
-inline constexpr bool is_constant_evaluated() noexcept {
+constexpr bool is_constant_evaluated() noexcept {
   return __builtin_is_constant_evaluated();
 }
 } // namespace std
Index: clang/test/SemaCUDA/function-overload.cu
===
--- clang/test/SemaCUDA/function-overload.cu
+++ clang/test/SemaCUDA/function-overload.cu
@@ -693,9 +693,9 @@
 namespace TestImplicitHDWithHAndD {
   namespace X {
 inline double foo(double, double) { return 0;}
-inline constexpr float foo(float, float) { return 1;}
-inline constexpr long double foo(long double, long double) { return 2;}
-template inline constexpr double foo(_Tp, _Up) { return 3;}
+constexpr float foo(float, float) { return 1;}
+constexpr long double foo(long double, long double) { return 2;}
+template constexpr double foo(_Tp, _Up) { return 3;}
   };
   using X::foo;
   inline __device__ double foo(double, double) { return 4;}
Index: clang/test/Parser/cxx0x-decl.cpp
===
--- clang/test/Parser/cxx0x-decl.cpp
+++ clang/test/Parser/cxx0x-decl.cpp
@@ -172,6 +172,15 @@
   consteval constinit int huh(); // expected-error {{cannot combine with previous 'consteval'}}
 }
 
+namespace RedundantSpecifier {
+  consteval inline int f1(); // expected-warning {{'inline' declaration specifier is redundant due to the presence of stricter 'consteval' declaration specifier}}
+  inline constexpr int f2(); // expected-warning {{'inline' declaration specifier is redundant due to the presence of stricter 'constexpr' declaration specifier}}
+  inline constexpr inline int f3(); // expected-warning {{'inline' declaration specifier is redundant due to the presence of stricter 'constexpr' declaration specifier}} \
+// expected-warning {{duplicate 'inline' declaration specifier}}
+  template inline consteval int f4(T t); // expected-warning {{'inline' declaration specifier is redundant due to the presence of stricter 'consteval' declaration specifier}}
+  inline constexpr int i1 = 0;
+}
+
 namespace ColonColonDecltype {
   struct S { struct T {}; 

[PATCH] D117435: [clang] Warning for inline constexpr functions

[John McCall via Phabricator via cfe-commits]

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

I don't remember if this is written up anywhere, but clang has a longstanding 
policy that style enforcement belongs in a linter rather than the compiler.  
One programmer's "redundant" is another programmer's "clear and explicit".  I 
don't think this warning is consistent with that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117435

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

[PATCH] D117129: [clang-tidy] Extract Class IncluderClangTidyCheck

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added a comment.

I added some comments on the other review.

Is constructing an `IncludeInserter` particularly expensive?
I'm just curious.


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

https://reviews.llvm.org/D117129

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

[PATCH] D117435: [clang] Warning for inline constexpr functions

[Evgeny Shulgin via Phabricator via cfe-commits]

Izaron added inline comments.



Comment at: clang/test/Headers/x86-intrinsics-headers-clean.cpp:4-5
 // RUN: %clang_cc1 -ffreestanding -triple x86_64-unknown-unknown 
-Wsystem-headers \
-// RUN:   -Wcast-qual -fsyntax-only -flax-vector-conversions=none -x c++ 
-verify %s
+// RUN:   -Wno-redundant-decl-specifier -Wcast-qual -fsyntax-only \
+// RUN:   -flax-vector-conversions=none -x c++ -verify %s
 

Honestly I have a doubt for this "fix" (just ignoring the new diagnostic), but 
I couldn't change the included file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117435

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

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

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

HazardyKnusperkeks added a comment.

In D117416#3246838 , @curdeius wrote:

> Thanks for having a try on this.
> However, I don't like this approach too much. You add many changes and a 
> single test. That's not sufficient.
> Also, handling C++ keywords in all cases (e.g. `delete` as a function name) 
> *may* need to distinguish whether we format a C file or a C++ file. It's 
> probably impossible to do this without user input (.h extension is used in 
> both languages for example).
> We'd maybe need to add C as language option and let the user specify the 
> language (`-x c`?).
> That in turn may be painful (because not automatic).
> But, you may have a better solution.
> My 2 cents.

I second just adding a new language.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

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

[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

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

HazardyKnusperkeks added a comment.

Thanks for fixing this. It hit my quite some times, but never had the time to 
look into it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117421

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

[PATCH] D117435: [clang] Warning for inline constexpr functions

[Evgeny Shulgin via Phabricator via cfe-commits]

Izaron added a comment.

In D117435#3247137 , @rjmccall wrote:

> I don't remember if this is written up anywhere, but clang has a longstanding 
> policy that style enforcement belongs in a linter rather than the compiler.  
> One programmer's "redundant" is another programmer's "clear and explicit".  I 
> don't think this warning is consistent with that.

Thanks, didn't know this. Hope I'll make it to clang-tidy then.
(There was an abandoned review back in time when "inline" meant "inlinehint" 
for LLVM IR. https://reviews.llvm.org/D18914)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117435

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

[PATCH] D18914: [clang-tidy] new readability-redundant-inline

[Evgeny Shulgin via Phabricator via cfe-commits]

Izaron added a comment.

In D18914#2399705 , @chfast wrote:

> This check can be useful in other case like this:
>
>   inline constexpr const int x = 1;
>
> where `inline` and `const` are redundant.

In your case `inline` is not actually redundant (at least for a global 
variable). `const` (therefore `constexpr`) means internal linkage, so every 
translation unit will just get itself an unique internal copy of this variable. 
We can see it if we print the address of `x` in different translation units. 
The addresses will be different without `inline` and the same with `inline`.


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

https://reviews.llvm.org/D18914

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

[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

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

HazardyKnusperkeks added a comment.

Could you please rebase the patch? I promise I will commit it **very** shortly 
afterwards.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109557

___
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 400400.
HazardyKnusperkeks marked 2 inline comments as done.
HazardyKnusperkeks added a comment.
This revision is now accepted and ready to land.

Updated, now using a pointer (potential null) for PreviousLine.


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,12 @@
 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];
+const auto *PreviousLine = I != AnnotatedLines.begin() ? I[-1] : nullptr;
+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,15 +233,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);
+// Handle empty record blocks where the brace has already been wrapped.
+if (PreviousLine && TheLine->Last->is(tok::l_brace) &&
+TheLine->First == TheLine->Last) {
+  bool EmptyBlock = NextLine.First->is(tok::r_brace);
 
-  const FormatToken *Tok = I[-1]->First;
+  const FormatToken *Tok = PreviousLine->First;
   if (Tok && Tok->is(tok::comment))
 Tok = Tok->getNextNonComment();
 
@@ -263,20 +265,20 @@
 }
 
 auto ShouldMergeShortFunctions =
-[this, B = AnnotatedLines.begin()](
-SmallVectorImpl::const_iterator I) {
+[this, B = AnnotatedLines.begin(), &NextLine,
+ TheLine](SmallVectorImpl::const_iterator I) {
   if (Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_All)
 return true;
   if (Style.AllowShortFunctionsOnASingleLine >=
   FormatStyle::SFS_Empty &&
-  I[1]->First->is(tok::r_brace))
+  NextLine.First->is(tok::r_brace))
 return true;
 
   if (Style.AllowShortFunctionsOnASingleLine &
   FormatStyle::SFS_InlineOnly) {
 // Just checking TheLine->Level != 0 is not enough, because it
 // provokes treating functions inside indented namespaces as short.
-if ((*I)->Level != 0) {
+if (TheLine->Level != 0) {
   if (I == B)
 return false;
 
@@ -324,7 +326,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;
@@ -340,89 +342,90 @@
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
+// Try to merge a control statement block with left brace unwrapped.
 if (TheLine->Last->is(tok::l_brace) && TheLine->First != TheLine->Last &&
 TheLine->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for)) {
   return Style.AllowShortBlocksOnASingleLine != FormatStyle::SBS_Never
  ? tryMergeSimpleBlock(I, E, Limit)
  : 

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

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

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:288
+// 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)) {

owenpan wrote:
> Is `TheLine->First != TheLine->Last` redundant?
This seems to be an error on my side.


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] D117306: [clang-tidy] Add new check 'shared-ptr-array-mismatch'.

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/misc/SharedPtrArrayMismatchCheck.cpp:21-26
+SharedPtrArrayMismatchCheck::SharedPtrArrayMismatchCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context) {}
+
+void SharedPtrArrayMismatchCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {}

If you're not doing anything different in the c'tor I think you can
eliminate this definition and just lift the visibility of the base class
c'tor in the class declaration:
```
using ClangTidyCheck::ClangTidyCheck;
```



Comment at: clang-tools-extra/clang-tidy/misc/SharedPtrArrayMismatchCheck.cpp:39
+  cxxConstructExpr(
+  hasDeclaration(UsedConstructor), argumentCountIs(1),
+  hasArgument(0, cxxNewExpr(isArray(), hasType(pointerType(pointee(

What about the other constructor overloads?
Creating a `shared_ptr` with a deleter is quite common.



Comment at: 
clang-tools-extra/clang-tidy/misc/SharedPtrArrayMismatchCheck.cpp:104
+.getLocalSourceRange();
+D << TemplateArgumentRange;
+

I'm not sure what this is doing?



Comment at: 
clang-tools-extra/clang-tidy/misc/SharedPtrArrayMismatchCheck.cpp:106
+
+if (isInSingleDeclStmt(VarOrField)) {
+  const SourceManager &SM = Ctx.getSourceManager();

Is this to guard against multiple variables being declared at once?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117306

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

[PATCH] D117304: [clang][dataflow] Remove TestingSupport's dependency on gtest

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:144
+  auto *Lattice =
+  llvm::any_cast(&State.Lattice.Value);
+  Results.emplace_back(It->second, StateT{*Lattice, State.Env});

any_cast already performs an assert.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117304

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

[PATCH] D116875: [clang-tidy] Add performance-inefficient-array-traversal check

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added a comment.

Except for a couple nits, LGTM.




Comment at: 
clang-tools-extra/clang-tidy/performance/InefficientArrayTraversalCheck.h:25-26
+public:
+  InefficientArrayTraversalCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;

If you're not doing anything different, I believe you can just do
`using ClangTidyCheck::ClangTidyCheck;`

Also, shouldn't you be explicitly overriding the d'tor?
`~InefficientArrayTraversalCheck() override = default;`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116875

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

[PATCH] D117292: [Driver] Pass the flag -dI to cc1 invocation

[Qichao Gu via Phabricator via cfe-commits]

qichaogu added a comment.

Could someone help commit the patch?
I don't have commit access.


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

https://reviews.llvm.org/D117292

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

[PATCH] D114908: [clang] Don't call inheritDefaultTemplateArguments() on CXXDeductionGuideDecl's template parameters

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D114908#3223981 , @rnk wrote:

> Friendly ping for a modules CTAD bugfix.

Not sure if you met the same problem with me. In our downstream, we did a 
workaround like:

  if (Context.getLangOpts().CPlusPlusModules &&
To->getOwningModule() != From->getOwningModule())
  return false;

We workarounded the problem by inserting the above checking in 
`inheritDefaultTemplateArgument()` in ASTReaderDecl. Hope this helps.
(I understand this is not a good solution)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114908

___
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

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:215
+const auto &NextLine = *I[1];
+const auto *PreviousLine = I != AnnotatedLines.begin() ? I[-1] : nullptr;
+if (NextLine.Type == LT_Invalid || NextLine.First->MustBreakBefore)

I would move this line to just before handling empty record blocks below.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:268-289
+[this, B = AnnotatedLines.begin(), &NextLine,
+ TheLine](SmallVectorImpl::const_iterator I) {
   if (Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_All)
 return true;
   if (Style.AllowShortFunctionsOnASingleLine >=
   FormatStyle::SFS_Empty &&
+  NextLine.First->is(tok::r_brace))

I'd either leave this lambda alone or further simplify it as suggested here.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:318
 
 bool MergeShortFunctions = ShouldMergeShortFunctions(I);
 

Drop the `I` if you decide to further simplify the lambda as suggested above.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:406-417
+if (PreviousLine && TheLine->First->is(tok::l_brace) &&
+PreviousLine->First->is(tok::at) && PreviousLine->First->Next) {
+  tok::ObjCKeywordKind kwId =
+  PreviousLine->First->Next->Tok.getObjCKeywordID();
   if (kwId == clang::tok::objc_autoreleasepool ||
   kwId == clang::tok::objc_synchronized)
 return 0;

If you want, you can factor out `PreviousLine && 
TheLine->First->is(tok::l_brace)` and even combine the two `if` statements as 
they both return 0.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:310
+   : 0;
+  }
+  if (TheLine->First->isOneOf(tok::kw_if, tok::kw_else, tok::kw_while,

owenpan wrote:
> I don't think you can drop `else` here.
> I don't think you can drop `else` here.

I was wrong. For some reason, I didn't see the `return` statement.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:317
+   : 0;
+  }
+  if (TheLine->First->isOneOf(tok::kw_else, tok::kw_catch) &&

HazardyKnusperkeks wrote:
> owenpan wrote:
> > Ditto.
> Same.
I missed the `return` here as well.


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] D117419: [clang][cmake] Use `GNUInstallDirs` to support custom installation dirs

[John Ericson via Phabricator via cfe-commits]

Ericson2314 updated this revision to Diff 400419.
Ericson2314 added a comment.

1. Updating D117419 : [clang][cmake] Use 
`GNUInstallDirs` to support custom installation dirs #
2. Enter a brief description of the changes included in this update.
3. The first line is used as subject, next lines as comment. #
4. If you intended to create a new revision, use:
5. $ arc diff --create

Add missing import


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117419

Files:
  clang/CMakeLists.txt
  clang/cmake/modules/AddClang.cmake
  clang/cmake/modules/CMakeLists.txt
  clang/tools/c-index-test/CMakeLists.txt
  clang/tools/clang-format/CMakeLists.txt
  clang/tools/clang-nvlink-wrapper/CMakeLists.txt
  clang/tools/clang-rename/CMakeLists.txt
  clang/tools/libclang/CMakeLists.txt
  clang/tools/scan-build-py/CMakeLists.txt
  clang/tools/scan-build/CMakeLists.txt
  clang/tools/scan-view/CMakeLists.txt
  clang/utils/hmaptool/CMakeLists.txt
  llvm/cmake/modules/LLVMInstallSymlink.cmake

Index: llvm/cmake/modules/LLVMInstallSymlink.cmake
===
--- llvm/cmake/modules/LLVMInstallSymlink.cmake
+++ llvm/cmake/modules/LLVMInstallSymlink.cmake
@@ -6,7 +6,8 @@
 
 function(install_symlink name target outdir)
   set(DESTDIR $ENV{DESTDIR})
-  set(bindir "${DESTDIR}${CMAKE_INSTALL_PREFIX}/${outdir}")
+  GNUInstallDirs_get_absolute_install_dir(bindir "${outdir}" BINDIR)
+  set(bindir "${DESTDIR}${bindir}")
 
   message(STATUS "Creating ${name}")
 
Index: clang/utils/hmaptool/CMakeLists.txt
===
--- clang/utils/hmaptool/CMakeLists.txt
+++ clang/utils/hmaptool/CMakeLists.txt
@@ -10,7 +10,7 @@
 
 list(APPEND Depends ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/${CLANG_HMAPTOOL})
 install(PROGRAMS ${CLANG_HMAPTOOL}
-DESTINATION bin
+DESTINATION "${CMAKE_INSTALL_BINDIR}"
 COMPONENT hmaptool)
 
 add_custom_target(hmaptool ALL DEPENDS ${Depends})
Index: clang/tools/scan-view/CMakeLists.txt
===
--- clang/tools/scan-view/CMakeLists.txt
+++ clang/tools/scan-view/CMakeLists.txt
@@ -20,7 +20,7 @@
DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/bin/${BinFile})
 list(APPEND Depends ${CMAKE_BINARY_DIR}/bin/${BinFile})
 install(PROGRAMS bin/${BinFile}
-DESTINATION bin
+DESTINATION "${CMAKE_INSTALL_BINDIR}"
 COMPONENT scan-view)
   endforeach()
 
@@ -34,7 +34,7 @@
DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/share/${ShareFile})
 list(APPEND Depends ${CMAKE_BINARY_DIR}/share/scan-view/${ShareFile})
 install(FILES share/${ShareFile}
-DESTINATION share/scan-view
+DESTINATION "${CMAKE_INSTALL_DATADIR}/scan-view"
 COMPONENT scan-view)
   endforeach()
 
Index: clang/tools/scan-build/CMakeLists.txt
===
--- clang/tools/scan-build/CMakeLists.txt
+++ clang/tools/scan-build/CMakeLists.txt
@@ -47,7 +47,7 @@
DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/bin/${BinFile})
 list(APPEND Depends ${CMAKE_BINARY_DIR}/bin/${BinFile})
 install(PROGRAMS bin/${BinFile}
-DESTINATION bin
+DESTINATION "${CMAKE_INSTALL_BINDIR}"
 COMPONENT scan-build)
   endforeach()
 
@@ -61,7 +61,7 @@
DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/libexec/${LibexecFile})
 list(APPEND Depends ${CMAKE_BINARY_DIR}/libexec/${LibexecFile})
 install(PROGRAMS libexec/${LibexecFile}
-DESTINATION libexec
+DESTINATION "${CMAKE_INSTALL_LIBEXECDIR}"
 COMPONENT scan-build)
   endforeach()
 
@@ -89,7 +89,7 @@
DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/share/scan-build/${ShareFile})
 list(APPEND Depends ${CMAKE_BINARY_DIR}/share/scan-build/${ShareFile})
 install(FILES share/scan-build/${ShareFile}
-DESTINATION share/scan-build
+DESTINATION "${CMAKE_INSTALL_DATADIR}/scan-build"
 COMPONENT scan-build)
   endforeach()
 
Index: clang/tools/scan-build-py/CMakeLists.txt
===
--- clang/tools/scan-build-py/CMakeLists.txt
+++ clang/tools/scan-build-py/CMakeLists.txt
@@ -43,7 +43,7 @@
  ${CMAKE_BINARY_DIR}/bin/scan-build-py
DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/bin/scan-build)
 install (PROGRAMS "bin/scan-build"
- DESTINATION bin
+ DESTINATION "${CMAKE_INSTALL_BINDIR}"
  RENAME scan-build-py
  COMPONENT scan-build-py)
 list(APPEND Depends ${CMAKE_BINARY_DIR}/bin/scan-build-py)
@@ -56,7 +56,7 @@
  ${CMAKE_BINARY_DIR}/bin/
DEPENDS ${CMAKE_CUR

[clang] 427d3b9 - [Driver][FreeBSD] -r: imply -nostdlib like GCC

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2022-01-16T19:44:48-08:00
New Revision: 427d3b93eebadb57ef72e1eb167dd41043b87f39

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

LOG: [Driver][FreeBSD] -r: imply -nostdlib like GCC

Similar to D116843 for Gnu.cpp

Reviewed By: dim

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/FreeBSD.cpp
clang/test/Driver/freebsd.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/FreeBSD.cpp 
b/clang/lib/Driver/ToolChains/FreeBSD.cpp
index de635f5816cf..05c58a8f43a8 100644
--- a/clang/lib/Driver/ToolChains/FreeBSD.cpp
+++ b/clang/lib/Driver/ToolChains/FreeBSD.cpp
@@ -247,7 +247,8 @@ void freebsd::Linker::ConstructJob(Compilation &C, const 
JobAction &JA,
 assert(Output.isNothing() && "Invalid output.");
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 const char *crt1 = nullptr;
 if (!Args.hasArg(options::OPT_shared)) {
   if (Args.hasArg(options::OPT_pg))
@@ -295,7 +296,8 @@ void freebsd::Linker::ConstructJob(Compilation &C, const 
JobAction &JA,
 
   unsigned Major = ToolChain.getTriple().getOSMajorVersion();
   bool Profiling = Args.hasArg(options::OPT_pg) && Major != 0 && Major < 14;
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
+   options::OPT_r)) {
 // Use the static OpenMP runtime with -static-openmp
 bool StaticOpenMP = Args.hasArg(options::OPT_static_openmp) &&
 !Args.hasArg(options::OPT_static);
@@ -358,7 +360,8 @@ void freebsd::Linker::ConstructJob(Compilation &C, const 
JobAction &JA,
 }
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (Args.hasArg(options::OPT_shared) || IsPIE)
   
CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtendS.o")));
 else

diff  --git a/clang/test/Driver/freebsd.c b/clang/test/Driver/freebsd.c
index bed1b927898f..06eb8880f577 100644
--- a/clang/test/Driver/freebsd.c
+++ b/clang/test/Driver/freebsd.c
@@ -205,3 +205,10 @@
 // RUN: %clang -target ppc64-unknown-freebsd13.0 -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=PPC64-MUNWIND %s
 // PPC64-MUNWIND: "-funwind-tables=2"
+
+/// -r suppresses default -l and crt*.o like -nostdlib.
+// RUN: %clang -### %s --target=aarch64-pc-freebsd11 -r \
+// RUN:   --sysroot=%S/Inputs/basic_freebsd64_tree 2>&1 | FileCheck %s 
--check-prefix=RELOCATABLE
+// RELOCATABLE: "-r"
+// RELOCATABLE-NOT: "-l
+// RELOCATABLE-NOT: crt{{[^.]+}}.o



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

[PATCH] D117388: [Driver][FreeBSD] -r: imply -nostdlib like GCC

[Fangrui Song 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 rG427d3b93eeba: [Driver][FreeBSD] -r: imply -nostdlib like GCC 
(authored by MaskRay).

Changed prior to commit:
  https://reviews.llvm.org/D117388?vs=400251&id=400424#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117388

Files:
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/test/Driver/freebsd.c


Index: clang/test/Driver/freebsd.c
===
--- clang/test/Driver/freebsd.c
+++ clang/test/Driver/freebsd.c
@@ -205,3 +205,10 @@
 // RUN: %clang -target ppc64-unknown-freebsd13.0 -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=PPC64-MUNWIND %s
 // PPC64-MUNWIND: "-funwind-tables=2"
+
+/// -r suppresses default -l and crt*.o like -nostdlib.
+// RUN: %clang -### %s --target=aarch64-pc-freebsd11 -r \
+// RUN:   --sysroot=%S/Inputs/basic_freebsd64_tree 2>&1 | FileCheck %s 
--check-prefix=RELOCATABLE
+// RELOCATABLE: "-r"
+// RELOCATABLE-NOT: "-l
+// RELOCATABLE-NOT: crt{{[^.]+}}.o
Index: clang/lib/Driver/ToolChains/FreeBSD.cpp
===
--- clang/lib/Driver/ToolChains/FreeBSD.cpp
+++ clang/lib/Driver/ToolChains/FreeBSD.cpp
@@ -247,7 +247,8 @@
 assert(Output.isNothing() && "Invalid output.");
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 const char *crt1 = nullptr;
 if (!Args.hasArg(options::OPT_shared)) {
   if (Args.hasArg(options::OPT_pg))
@@ -295,7 +296,8 @@
 
   unsigned Major = ToolChain.getTriple().getOSMajorVersion();
   bool Profiling = Args.hasArg(options::OPT_pg) && Major != 0 && Major < 14;
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
+   options::OPT_r)) {
 // Use the static OpenMP runtime with -static-openmp
 bool StaticOpenMP = Args.hasArg(options::OPT_static_openmp) &&
 !Args.hasArg(options::OPT_static);
@@ -358,7 +360,8 @@
 }
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (Args.hasArg(options::OPT_shared) || IsPIE)
   
CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtendS.o")));
 else


Index: clang/test/Driver/freebsd.c
===
--- clang/test/Driver/freebsd.c
+++ clang/test/Driver/freebsd.c
@@ -205,3 +205,10 @@
 // RUN: %clang -target ppc64-unknown-freebsd13.0 -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=PPC64-MUNWIND %s
 // PPC64-MUNWIND: "-funwind-tables=2"
+
+/// -r suppresses default -l and crt*.o like -nostdlib.
+// RUN: %clang -### %s --target=aarch64-pc-freebsd11 -r \
+// RUN:   --sysroot=%S/Inputs/basic_freebsd64_tree 2>&1 | FileCheck %s --check-prefix=RELOCATABLE
+// RELOCATABLE: "-r"
+// RELOCATABLE-NOT: "-l
+// RELOCATABLE-NOT: crt{{[^.]+}}.o
Index: clang/lib/Driver/ToolChains/FreeBSD.cpp
===
--- clang/lib/Driver/ToolChains/FreeBSD.cpp
+++ clang/lib/Driver/ToolChains/FreeBSD.cpp
@@ -247,7 +247,8 @@
 assert(Output.isNothing() && "Invalid output.");
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 const char *crt1 = nullptr;
 if (!Args.hasArg(options::OPT_shared)) {
   if (Args.hasArg(options::OPT_pg))
@@ -295,7 +296,8 @@
 
   unsigned Major = ToolChain.getTriple().getOSMajorVersion();
   bool Profiling = Args.hasArg(options::OPT_pg) && Major != 0 && Major < 14;
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
+   options::OPT_r)) {
 // Use the static OpenMP runtime with -static-openmp
 bool StaticOpenMP = Args.hasArg(options::OPT_static_openmp) &&
 !Args.hasArg(options::OPT_static);
@@ -358,7 +360,8 @@
 }
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (Args.hasArg(options::OPT_shared) || IsPIE)
   CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtendS.o")));
 else
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103228: [PoC][RISCV] Using pragma to register vector intrinsic

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng abandoned this revision.
kito-cheng added a comment.
Herald added subscribers: alextsao1999, VincentWu, luke957, 
achieveartificialintelligence.

Further development move to https://reviews.llvm.org/D111617


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103228

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

[PATCH] D117306: [clang-tidy] Add new check 'shared-ptr-array-mismatch'.

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added a comment.

In D117306#3245915 , @njames93 wrote:

> How does this check play with the `modernize-make-shared` check?

Wouldn't it be orthogonal?

This check looks for existing `make_shared` usages, whereas
modernize-make-shared adds new `make_shared` usages from
`new shared_ptr` usages.  I wouldn't expect `modernize-make-shared`
to generate mismatched scalar/array `shared_ptr` instances.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117306

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

[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

[Cameron Mulhern via Phabricator via cfe-commits]

csmulhern updated this revision to Diff 400435.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109557

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/ContinuationIndenter.h
  clang/lib/Format/Format.cpp
  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
@@ -19167,6 +19167,8 @@
   FormatStyle::BAS_DontAlign);
   CHECK_PARSE("AlignAfterOpenBracket: AlwaysBreak", AlignAfterOpenBracket,
   FormatStyle::BAS_AlwaysBreak);
+  CHECK_PARSE("AlignAfterOpenBracket: BlockIndent", AlignAfterOpenBracket,
+  FormatStyle::BAS_BlockIndent);
   // For backward compatibility:
   CHECK_PARSE("AlignAfterOpenBracket: false", AlignAfterOpenBracket,
   FormatStyle::BAS_DontAlign);
@@ -23696,6 +23698,224 @@
Style);
 }
 
+TEST_F(FormatTest, AlignAfterOpenBracketBlockIndent) {
+  auto Style = getLLVMStyle();
+
+  StringRef Short = "functionCall(paramA, paramB, paramC);\n"
+"void functionDecl(int a, int b, int c);";
+
+  StringRef Medium = "functionCall(paramA, paramB, paramC, paramD, paramE, "
+ "paramF, paramG, paramH, paramI);\n"
+ "void functionDecl(int argumentA, int argumentB, int "
+ "argumentC, int argumentD, int argumentE);";
+
+  verifyFormat(Short, Style);
+
+  StringRef NoBreak = "functionCall(paramA, paramB, paramC, paramD, paramE, "
+  "paramF, paramG, paramH,\n"
+  " paramI);\n"
+  "void functionDecl(int argumentA, int argumentB, int "
+  "argumentC, int argumentD,\n"
+  "  int argumentE);";
+
+  verifyFormat(NoBreak, Medium, Style);
+  verifyFormat(NoBreak,
+   "functionCall(\n"
+   "paramA,\n"
+   "paramB,\n"
+   "paramC,\n"
+   "paramD,\n"
+   "paramE,\n"
+   "paramF,\n"
+   "paramG,\n"
+   "paramH,\n"
+   "paramI\n"
+   ");\n"
+   "void functionDecl(\n"
+   "int argumentA,\n"
+   "int argumentB,\n"
+   "int argumentC,\n"
+   "int argumentD,\n"
+   "int argumentE\n"
+   ");",
+   Style);
+
+  verifyFormat("outerFunctionCall(nestedFunctionCall(argument1),\n"
+   "  nestedLongFunctionCall(argument1, "
+   "argument2, argument3,\n"
+   " argument4, "
+   "argument5));",
+   Style);
+
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_BlockIndent;
+
+  verifyFormat(Short, Style);
+  verifyFormat(
+  "functionCall(\n"
+  "paramA, paramB, paramC, paramD, paramE, paramF, paramG, paramH, "
+  "paramI\n"
+  ");\n"
+  "void functionDecl(\n"
+  "int argumentA, int argumentB, int argumentC, int argumentD, int "
+  "argumentE\n"
+  ");",
+  Medium, Style);
+
+  Style.AllowAllArgumentsOnNextLine = false;
+  Style.AllowAllParametersOfDeclarationOnNextLine = false;
+
+  verifyFormat(Short, Style);
+  verifyFormat(
+  "functionCall(\n"
+  "paramA, paramB, paramC, paramD, paramE, paramF, paramG, paramH, "
+  "paramI\n"
+  ");\n"
+  "void functionDecl(\n"
+  "int argumentA, int argumentB, int argumentC, int argumentD, int "
+  "argumentE\n"
+  ");",
+  Medium, Style);
+
+  Style.BinPackArguments = false;
+  Style.BinPackParameters = false;
+
+  verifyFormat(Short, Style);
+
+  verifyFormat("functionCall(\n"
+   "paramA,\n"
+   "paramB,\n"
+   "paramC,\n"
+   "paramD,\n"
+   "paramE,\n"
+   "paramF,\n"
+   "paramG,\n"
+   "paramH,\n"
+   "paramI\n"
+   ");\n"
+   "void functionDecl(\n"
+   "int argumentA,\n"
+   "int argumentB,\n"
+   "int argumentC,\n"
+   "int argumentD,\n"
+   "int argumentE\n"
+   ");",
+   Medium, Style);
+
+  verifyFormat("outerFunctionCall(\n"
+   "nestedFunctionCall(argument1),\n"
+   "nestedLongFunctionCall(\n"
+   "argument1,\n"
+   "argument2,\n"
+   "argument3,\n"
+  

[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

[Cameron Mulhern via Phabricator via cfe-commits]

csmulhern added a comment.

In D109557#3247167 , 
@HazardyKnusperkeks wrote:

> Could you please rebase the patch? I promise I will commit it **very** 
> shortly afterwards.

Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109557

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

[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

[Elliott Maguire via Phabricator via cfe-commits]

glotchimo updated this revision to Diff 400439.
glotchimo added a comment.

Explicitly declare `FormatToken` type instead of using `auto`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117421

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
@@ -16167,6 +16167,14 @@
   verifyFormat("int oneTwoThree = 123; // comment\n"
"int oneTwo  = 12;  // comment",
Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default; // comment\n"
+   "int &operator() = default; // comment\n"
+   "int &operator=() {",
+   Alignment);
 
   // Bug 25167
   /* Uncomment when fixed
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -731,6 +731,16 @@
 if (&C != &Changes.back() && (&C + 1)->NewlinesBefore > 0)
   return false;
 
+// Do not align operator= overloads.
+if (C.Tok->Previous && C.Tok->Previous->is(tok::kw_operator)) {
+  FormatToken *Next = C.Tok->Next;
+  while (Next && Next->NewlinesBefore == 0) {
+if (Next->Tok.is(tok::l_brace))
+  return false;
+Next = Next->Next;
+  }
+}
+
 return C.Tok->is(tok::equal);
   },
   Changes, /*StartAt=*/0, Style.AlignConsecutiveAssignments);


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16167,6 +16167,14 @@
   verifyFormat("int oneTwoThree = 123; // comment\n"
"int oneTwo  = 12;  // comment",
Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default; // comment\n"
+   "int &operator() = default; // comment\n"
+   "int &operator=() {",
+   Alignment);
 
   // Bug 25167
   /* Uncomment when fixed
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -731,6 +731,16 @@
 if (&C != &Changes.back() && (&C + 1)->NewlinesBefore > 0)
   return false;
 
+// Do not align operator= overloads.
+if (C.Tok->Previous && C.Tok->Previous->is(tok::kw_operator)) {
+  FormatToken *Next = C.Tok->Next;
+  while (Next && Next->NewlinesBefore == 0) {
+if (Next->Tok.is(tok::l_brace))
+  return false;
+Next = Next->Next;
+  }
+}
+
 return C.Tok->is(tok::equal);
   },
   Changes, /*StartAt=*/0, Style.AlignConsecutiveAssignments);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116786: [clangd] Add designator inlay hints for initializer lists.

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

Thanks, this is a pretty nice addition!

My only piece of high-level feedback is probing if the array element 
designators (`[0]=`) are useful enough to be worth the space/noise. The rest is 
minor implementation comments.




Comment at: clang-tools-extra/clangd/InlayHints.cpp:71
+if (BasesI != BasesE)
+  return false; // Bases can't be designated. Should we make one up?
+if (FieldsI != FieldsE) {

We could consider using the type name of the base (but I also get the 
impression that aggregate initialization with bases is uncommon enough to not 
worry about right now)



Comment at: clang-tools-extra/clangd/InlayHints.cpp:81
+   // std::array x = {1,2,3}. Designators not strictly valid!
+   (OneField && isReservedName(FieldName
+return true;

Should we try to more specifically restrict this to the case where the one 
field is an array? Otherwise skipping the field name feels a bit arbitrary.



Comment at: clang-tools-extra/clangd/InlayHints.cpp:99
+  unsigned Index = 0;
+  CXXRecordDecl::base_class_const_iterator BasesI;
+  CXXRecordDecl::base_class_const_iterator BasesE;

suggest `Iter` and `End`, `I` and `E` are a bit cryptic



Comment at: clang-tools-extra/clangd/InlayHints.cpp:117
+// It should generate designators '.a:' and '.b.x:'.
+// '.a:' is produced directly without recursing into the written sublist.
+// Recursion with Prefix='.b' and Sem = {3, ImplicitValue} produces '.b.x:'.

Can we add that the written sublist will get its own VisitInitListExpr call by 
RAV, while the implicit sublist will not? (If I've understood that correctly.)



Comment at: clang-tools-extra/clangd/InlayHints.cpp:128
+  // The elements of the semantic form all correspond to direct subobjects of
+  // the type Type. `Fields` iterates over these subobject names.
+  AggregateDesignatorNames Fields(Sem->getType());

"of the aggregate type"? ("the type Type" is a bit confusing, for a moment I 
thought you were talking about `clang::Type`)



Comment at: clang-tools-extra/clangd/InlayHints.cpp:145
+
+if (!Fields.append(Prefix, BraceElidedSubobject != nullptr))
+  continue; // no designator available for this subobject

aside: `Prefix` on this line is a great use case for the 
`UsedAsMutableReference` modifier. Now, if only we could somehow integrate 
clangd's semantic highlighting into Phabricator...



Comment at: clang-tools-extra/clangd/InlayHints.cpp:151
+  // Descend into the semantic list describing the subobject.
+  collectDesignators(BraceElidedSubobject, Out, NestedBraces, Prefix);
+  continue;

It took me a while to think through why it's **not** necessary to add more 
things into `NestedBraces` for the recursive call: since we only recurse in the 
case of elided braces, if a brace-elided subobject has an initializer with an 
explicit brace then **syntactically** that's a direct child of the containing 
explicit-brace initializer (even if **semantically** it's several levels deep), 
and thus its brace is already represented in `NestedBraces`. Might be worth 
pointing that out in a comment.



Comment at: clang-tools-extra/clangd/InlayHints.cpp:165
+
+  // gatherDesignators needs to know which InitListExprs in the semantic tree
+  // were actually written, but InitListExpr::isExplicit() lies.

collectDesignators



Comment at: clang-tools-extra/clangd/InlayHints.cpp:176
+  llvm::DenseMap Designators;
+  std::string Scratch;
+  collectDesignators(Syn->isSemanticForm() ? Syn : Syn->getSemanticForm(),

"EmptyPrefix"? ("Scratch" makes it sound like "scratch space for it to write 
temporary things into")



Comment at: clang-tools-extra/clangd/InlayHints.cpp:283
 
+  bool VisitInitListExpr(InitListExpr *Syn) {
+if (Syn->isIdiomaticZeroInitializer(AST.getLangOpts()))

I take it the parameter name `Syn` is supposed to suggest that the 
`InitListExpr` is the syntactic form (of the two forms described 
[here](https://searchfox.org/llvm/rev/be9eafc71004393363d155dd16ea1af9c663aafe/clang/include/clang/AST/Expr.h#4757)),
 and that we know this because `InlayHintVisitor` does not override 
`shouldVisitImplicitCode()`, which makes RAV [only 
traverse](https://searchfox.org/llvm/rev/be9eafc71004393363d155dd16ea1af9c663aafe/clang/include/clang/AST/RecursiveASTVisitor.h#2412)
 the syntactic form.

I think it would aid understandability if we mentioned these things in a 
comment.



Comment at: clang-tools-extra/clangd/InlayHints.cpp:404
+SourcePrefix = SourcePrefix.rtrim(IgnoreChars);
+ParamName = ParamName.trim(IgnoreChars);
 // Other than that, the comment must contain exactly ParamName.
-

[clang] d771cf2 - [PowerPC] Allow -mfloat128 option for VSX targets

[Qiu Chaofan via cfe-commits]

Author: Qiu Chaofan
Date: 2022-01-17T15:12:33+08:00
New Revision: d771cf277565f579aba24fef522355f4406323c9

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

LOG: [PowerPC] Allow -mfloat128 option for VSX targets

Targets with VSX feature but without native float128 instructions can
also use that type with supplementary libcalls. We don't enable it by
default now because Glibc assumes long double and float128 can be
implicitly converted in between, which is not available under default
'ibmlongdouble' semantics in clang.

This commit partly relands cbd93ce.

Added: 


Modified: 
clang/lib/Basic/Targets/PPC.cpp
clang/test/Driver/ppc-f128-support-check.c

Removed: 




diff  --git a/clang/lib/Basic/Targets/PPC.cpp b/clang/lib/Basic/Targets/PPC.cpp
index 7f7b44b658eba..19b7ded40402a 100644
--- a/clang/lib/Basic/Targets/PPC.cpp
+++ b/clang/lib/Basic/Targets/PPC.cpp
@@ -561,9 +561,9 @@ bool PPCTargetInfo::initFeatureMap(
   if (!ppcUserFeaturesCheck(Diags, FeaturesVec))
 return false;
 
-  if (!(ArchDefs & ArchDefinePwr9) && (ArchDefs & ArchDefinePpcgr) &&
+  if (!(ArchDefs & ArchDefinePwr7) && (ArchDefs & ArchDefinePpcgr) &&
   llvm::is_contained(FeaturesVec, "+float128")) {
-// We have __float128 on PPC but not power 9 and above.
+// We have __float128 on PPC but not pre-VSX targets.
 Diags.Report(diag::err_opt_not_valid_with_opt) << "-mfloat128" << CPU;
 return false;
   }

diff  --git a/clang/test/Driver/ppc-f128-support-check.c 
b/clang/test/Driver/ppc-f128-support-check.c
index 24748905612ff..616d641f54b96 100644
--- a/clang/test/Driver/ppc-f128-support-check.c
+++ b/clang/test/Driver/ppc-f128-support-check.c
@@ -2,13 +2,17 @@
 // RUN:   -mcpu=pwr9 -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=HASF128
 // RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
 // RUN:   -mcpu=power9 -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=HASF128
-
 // RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
-// RUN:   -mcpu=pwr8 -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=NOF128
+// RUN:   -mcpu=pwr8 -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=HASF128
+// RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
+// RUN:   -mcpu=pwr7 -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=HASF128
+// RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
+// RUN:   -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=HASF128
+
 // RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
-// RUN:   -mcpu=pwr7 -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=NOF128
+// RUN:   -mcpu=pwr6 -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=NOF128
 // RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
-// RUN:   -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=NOF128
+// RUN:   -mno-vsx -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=NOF128
 
 #ifdef __FLOAT128__
 static_assert(false, "__float128 enabled");



___
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 marked 5 inline comments as done.
HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:215
+const auto &NextLine = *I[1];
+const auto *PreviousLine = I != AnnotatedLines.begin() ? I[-1] : nullptr;
+if (NextLine.Type == LT_Invalid || NextLine.First->MustBreakBefore)

owenpan wrote:
> I would move this line to just before handling empty record blocks below.
I'd rather keep the definitions close together.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:268-289
+[this, B = AnnotatedLines.begin(), &NextLine,
+ TheLine](SmallVectorImpl::const_iterator I) {
   if (Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_All)
 return true;
   if (Style.AllowShortFunctionsOnASingleLine >=
   FormatStyle::SFS_Empty &&
+  NextLine.First->is(tok::r_brace))

owenpan wrote:
> I'd either leave this lambda alone or further simplify it as suggested here.
I'm no fan of capturing everything, but some things I've applied.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:406-417
+if (PreviousLine && TheLine->First->is(tok::l_brace) &&
+PreviousLine->First->is(tok::at) && PreviousLine->First->Next) {
+  tok::ObjCKeywordKind kwId =
+  PreviousLine->First->Next->Tok.getObjCKeywordID();
   if (kwId == clang::tok::objc_autoreleasepool ||
   kwId == clang::tok::objc_synchronized)
 return 0;

owenpan wrote:
> If you want, you can factor out `PreviousLine && 
> TheLine->First->is(tok::l_brace)` and even combine the two `if` statements as 
> they both return 0.
Kept the two cases apart for their comments.


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] D115060: [clang-format][NFC] Code Tidies in UnwrappedLineFormatter

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

HazardyKnusperkeks updated this revision to Diff 400449.
HazardyKnusperkeks marked an inline comment as done.

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,12 @@
 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];
+const auto *PreviousLine = I != AnnotatedLines.begin() ? I[-1] : nullptr;
+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,15 +233,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);
+// Handle empty record blocks where the brace has already been wrapped.
+if (PreviousLine && TheLine->Last->is(tok::l_brace) &&
+TheLine->First == TheLine->Last) {
+  bool EmptyBlock = NextLine.First->is(tok::r_brace);
 
-  const FormatToken *Tok = I[-1]->First;
+  const FormatToken *Tok = PreviousLine->First;
   if (Tok && Tok->is(tok::comment))
 Tok = Tok->getNextNonComment();
 
@@ -262,58 +264,56 @@
   }
 }
 
-auto ShouldMergeShortFunctions =
-[this, B = AnnotatedLines.begin()](
-SmallVectorImpl::const_iterator I) {
-  if (Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_All)
-return true;
-  if (Style.AllowShortFunctionsOnASingleLine >=
-  FormatStyle::SFS_Empty &&
-  I[1]->First->is(tok::r_brace))
-return true;
-
-  if (Style.AllowShortFunctionsOnASingleLine &
-  FormatStyle::SFS_InlineOnly) {
-// Just checking TheLine->Level != 0 is not enough, because it
-// provokes treating functions inside indented namespaces as short.
-if ((*I)->Level != 0) {
-  if (I == B)
-return false;
-
-  // TODO: Use IndentTracker to avoid loop?
-  // Find the last line with lower level.
-  auto J = I - 1;
-  for (; J != B; --J)
-if ((*J)->Level < (*I)->Level)
-  break;
-
-  // Check if the found line starts a record.
-  auto *RecordTok = (*J)->First;
-  while (RecordTok) {
-// TODO: Refactor to isRecord(RecordTok).
-if (RecordTok->isOneOf(tok::kw_class, tok::kw_struct))
-  return true;
-if (Style.isCpp() && RecordTok->is(tok::kw_union))
-  return true;
-if (Style.isCSharp() && RecordTok->is(Keywords.kw_interface))
-  return true;
-if (Style.Language == FormatStyle::LK_Java &&
-RecordTok->is(tok::kw_enum))
-  return true;
-if (Style.isJavaScript() && RecordTok->is(Keywords.kw_function))
-  return true;
-
-RecordTok = RecordTok->Next;
-  }
-
-  return false;
-}
+auto ShouldMergeShortFunctions = [this, &I, &NextLine, PreviousLine,
+  TheLine]() {
+  if (Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_All)
+return true;
+  if (Style.AllowShortFunctionsOnASingleLine >= FormatStyle::SFS_Empty &&
+  NextLine.First->is(tok::r_brace))
+return true;
+
+  if (Style.AllowShortFunctionsOnASingleLine &
+  FormatStyle::SFS_InlineOnly) {
+// Just checking TheLine->Level != 0 is not enough, because it
+// provokes treating functions inside indented namespaces as short.
+if (TheLine->Level != 0) {
+  if (!PreviousLine)
+return false;
+
+  // TODO: Use IndentTracker to avoid loop?
+  // Find the last line with lower level.
+  auto J = I - 1;
+  for (; J != AnnotatedLines.begin(); --J)
+if ((*J)->Level < TheLine->Level)
+