[PATCH] D116485: [clang][MinGW] Explicitly ignore `-fPIC` & friends

[Martin Storsjö via Phabricator via cfe-commits]

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

LGTM, thanks. The fact that GCC used to emit a non-fatal warning for this 
option, but no longer does, probably is the key deciding factor here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116485

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

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

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis updated this revision to Diff 396914.
zinovy.nis added a comment.

Updated release notes.


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

https://reviews.llvm.org/D116478

Files:
  clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-assert-side-effect.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s bugprone-assert-side-effect %t -- -config="{CheckOptions: [{key: bugprone-assert-side-effect.CheckFunctionCalls, value: true}, {key: bugprone-assert-side-effect.AssertMacros, value: 'assert,assert2,my_assert,convoluted_assert,msvc_assert'}]}" -- -fexceptions
+// RUN: %check_clang_tidy %s bugprone-assert-side-effect %t -- -config="{CheckOptions: [{key: bugprone-assert-side-effect.CheckFunctionCalls, value: true}, {key: bugprone-assert-side-effect.AssertMacros, value: 'assert,assert2,my_assert,convoluted_assert,msvc_assert'}, {key: bugprone-assert-side-effect.FunctionExceptions, value: 'badButExceptedFunc'}]}" -- -fexceptions
 
 //===--- assert definition block --===//
 int abort() { return 0; }
@@ -46,6 +46,7 @@
 class MyClass {
 public:
   bool badFunc(int a, int b) { return a * b > 0; }
+  bool badButExceptedFunc(int a, int b) { return a * b > 0; }
   bool goodFunc(int a, int b) const { return a * b > 0; }
 
   MyClass &operator=(const MyClass &rhs) { return *this; }
@@ -87,6 +88,7 @@
   MyClass mc;
   assert(mc.badFunc(0, 1));
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in assert() condition discarded in release builds
+  assert(mc.badButExceptedFunc(0, 1));
   assert(mc.goodFunc(0, 1));
 
   MyClass mc2;
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-assert-side-effect.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone-assert-side-effect.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-assert-side-effect.rst
@@ -21,3 +21,8 @@
Whether to treat non-const member and non-member functions as they produce
side effects. Disabled by default because it can increase the number of false
positive warnings.
+
+.. option:: FunctionExceptions
+
+   A comma-separated list of the names of functions or methods to be
+   considered as not having side-effects.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -138,6 +138,9 @@
 
 Changes in existing checks
 ^^
+- Added a setting ``bugprone-assert-side-effect.FunctionExceptions`` for
+  a comma-separated list of the names of functions or methods to be considered
+  as not having side-effects.
 
 - Removed default setting ``cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors = "true"``,
   to match the current state of the C++ Core Guidelines.
@@ -148,7 +151,7 @@
 
 - Fixed a false positive in :doc:`fuchsia-trailing-return
   ` for C++17 deduction guides.
-  
+
 - Fixed a false positive in :doc:`bugprone-throw-keyword-missing
   ` when creating an exception object
   using placement new
Index: clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.h
===
--- clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_ASSERTSIDEEFFECTCHECK_H
 
 #include "../ClangTidyCheck.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include 
@@ -33,6 +34,7 @@
 /// can increase the number of false positive warnings.
 class AssertSideEffectCheck : public ClangTidyCheck {
 public:
+  using FunctionExceptionType = llvm::SmallSet;
   AssertSideEffectCheck(StringRef Name, ClangTidyContext *Context);
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
@@ -41,7 +43,9 @@
 private:
   const bool CheckFunctionCalls;
   const std::string RawAssertList;
+  const std::string RawFunctionExceptionsList;
   SmallVector AssertMacros;
+  FunctionExceptionType FunctionExceptions;
 };
 
 } // namespace bugprone
Index: clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
===
-

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

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis added a comment.

In D116478#3216082 , @Eugene.Zelenko 
wrote:

> Please mention new option in Release Notes (in changes in existing checks 
> section).

Done


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

https://reviews.llvm.org/D116478

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

[PATCH] D114064: [clang] [MinGW] Pass --no-demangle through to the mingw linker

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D114064#3216147 , @Ericson2314 
wrote:

> Not sure what the exact division of labor is between the GNU and MinGW 
> backends, but assuming there is no way to share this between them the patch 
> looks good.

Yeah. Also at this point, this option should probably rather be handled as 
opt-out for targets that don’t support it, instead of having every single 
target do manual work to pass it through.

Additionally, this probably doesn’t handle sequences of `--no-demangle 
--demangle` correctly, but this just gets this target to the same level as e.g. 
GNU.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114064

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

[clang] dbeeb13 - [clang][MinGW] Explicitly ignore `-fPIC` & friends

[Markus Böck via cfe-commits]

Author: Markus Böck
Date: 2022-01-02T12:06:54+01:00
New Revision: dbeeb136abcb03eaa85e2ee47a5169f5298e8944

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

LOG: [clang][MinGW] Explicitly ignore `-fPIC` & friends

GCC on Windows ignores this flag completely [0] which some build systems sadly 
rely on when compiling for Windows using MinGW. The current behaviour of clang 
however is to error out as -fPIC & friends has no effect on Windows.

This patch instead changes the behaviour for MinGW to ignore the option for the 
sake of compatibility

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

[0] https://gcc.gnu.org/legacy-ml/gcc-patches/2015-08/msg00836.html

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/CommonArgs.cpp
clang/lib/Driver/ToolChains/MinGW.cpp
clang/test/Driver/pic.c
clang/test/Driver/windows-pic.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp 
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index ad50c66cb6c11..f25fe9ba34c46 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1186,10 +1186,9 @@ tools::ParsePICArgs(const ToolChain &ToolChain, const 
ArgList &Args) {
 options::OPT_fpic, options::OPT_fno_pic,
 options::OPT_fPIE, options::OPT_fno_PIE,
 options::OPT_fpie, options::OPT_fno_pie);
-  if (Triple.isOSWindows() && LastPICArg &&
-  LastPICArg ==
-  Args.getLastArg(options::OPT_fPIC, options::OPT_fpic,
-  options::OPT_fPIE, options::OPT_fpie)) {
+  if (Triple.isOSWindows() && !Triple.isOSCygMing() && LastPICArg &&
+  LastPICArg == Args.getLastArg(options::OPT_fPIC, options::OPT_fpic,
+options::OPT_fPIE, options::OPT_fpie)) {
 ToolChain.getDriver().Diag(diag::err_drv_unsupported_opt_for_target)
 << LastPICArg->getSpelling() << Triple.str();
 if (Triple.getArch() == llvm::Triple::x86_64)

diff  --git a/clang/lib/Driver/ToolChains/MinGW.cpp 
b/clang/lib/Driver/ToolChains/MinGW.cpp
index ecce2f062bd79..6d8bfc358dd31 100644
--- a/clang/lib/Driver/ToolChains/MinGW.cpp
+++ b/clang/lib/Driver/ToolChains/MinGW.cpp
@@ -486,10 +486,7 @@ bool toolchains::MinGW::isPIEDefault(const 
llvm::opt::ArgList &Args) const {
   return false;
 }
 
-bool toolchains::MinGW::isPICDefaultForced() const {
-  return getArch() == llvm::Triple::x86_64 ||
- getArch() == llvm::Triple::aarch64;
-}
+bool toolchains::MinGW::isPICDefaultForced() const { return true; }
 
 llvm::ExceptionHandling
 toolchains::MinGW::GetExceptionModel(const ArgList &Args) const {

diff  --git a/clang/test/Driver/pic.c b/clang/test/Driver/pic.c
index acb0bad022f4c..2124bf1b277b6 100644
--- a/clang/test/Driver/pic.c
+++ b/clang/test/Driver/pic.c
@@ -301,3 +301,13 @@
 // RUN:   | FileCheck %s --check-prefix=CHECK-PIC2
 // RUN: %clang -c %s -target aarch64-windows-gnu -### 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-PIC2
+//
+// On MinGW, allow specifying -fPIC & friends but ignore them
+// RUN: %clang -fno-PIC -c %s -target x86_64-pc-windows-gnu -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-PIC2
+// RUN: %clang -fPIC -c %s -target i686-pc-windows-gnu -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-NO-PIC
+// RUN: %clang -fno-PIC -c %s -target aarch64-pc-windows-gnu -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-PIC2
+// RUN: %clang -fPIC -c %s -target armv7-pc-windows-gnu -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-NO-PIC

diff  --git a/clang/test/Driver/windows-pic.cpp 
b/clang/test/Driver/windows-pic.cpp
index 3b77a7cc5a335..00807d663dffb 100644
--- a/clang/test/Driver/windows-pic.cpp
+++ b/clang/test/Driver/windows-pic.cpp
@@ -16,15 +16,6 @@
 // RUN: %clang -### -target i686-windows-itanium -fPIE -fno-pie %s
 // RUN: %clang -### -target i686-windows-itanium -fpie -fno-pie %s
 
-// RUN: %clang -### -target i686-windows-gnu -fPIC %s 2>&1 | FileCheck 
-check-prefix CHECK-PIC-ERROR %s
-// RUN: %clang -### -target i686-windows-gnu -fpic %s 2>&1 | FileCheck 
-check-prefix CHECK-pic-ERROR %s
-// RUN: %clang -### -target i686-windows-gnu -fPIE %s 2>&1 | FileCheck 
-check-prefix CHECK-PIE-ERROR %s
-// RUN: %clang -### -target i686-windows-gnu -fpie %s 2>&1 | FileCheck 
-check-prefix CHECK-pie-ERROR %s
-// RUN: %clang -### -target i686-windows-gnu -fPIC -fno-pic %s
-// RUN: %clang -### -target i686-windows-gnu -Fpic -fno-pic %s
-// RUN: %clang -### -target i686-windows-gnu -fPIE -fno-pie %s
-// RUN: %clang -### -target i686-windows-gnu -fpie -fno-pie %s
-
 // RUN: %clang -### -target x86_64-windows -fPIC %

[PATCH] D116485: [clang][MinGW] Explicitly ignore `-fPIC` & friends

[Markus Böck 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 rGdbeeb136abcb: [clang][MinGW] Explicitly ignore `-fPIC` & 
friends (authored by zero9178).

Changed prior to commit:
  https://reviews.llvm.org/D116485?vs=396884&id=396916#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116485

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/test/Driver/pic.c
  clang/test/Driver/windows-pic.cpp


Index: clang/test/Driver/windows-pic.cpp
===
--- clang/test/Driver/windows-pic.cpp
+++ clang/test/Driver/windows-pic.cpp
@@ -16,15 +16,6 @@
 // RUN: %clang -### -target i686-windows-itanium -fPIE -fno-pie %s
 // RUN: %clang -### -target i686-windows-itanium -fpie -fno-pie %s
 
-// RUN: %clang -### -target i686-windows-gnu -fPIC %s 2>&1 | FileCheck 
-check-prefix CHECK-PIC-ERROR %s
-// RUN: %clang -### -target i686-windows-gnu -fpic %s 2>&1 | FileCheck 
-check-prefix CHECK-pic-ERROR %s
-// RUN: %clang -### -target i686-windows-gnu -fPIE %s 2>&1 | FileCheck 
-check-prefix CHECK-PIE-ERROR %s
-// RUN: %clang -### -target i686-windows-gnu -fpie %s 2>&1 | FileCheck 
-check-prefix CHECK-pie-ERROR %s
-// RUN: %clang -### -target i686-windows-gnu -fPIC -fno-pic %s
-// RUN: %clang -### -target i686-windows-gnu -Fpic -fno-pic %s
-// RUN: %clang -### -target i686-windows-gnu -fPIE -fno-pie %s
-// RUN: %clang -### -target i686-windows-gnu -fpie -fno-pie %s
-
 // RUN: %clang -### -target x86_64-windows -fPIC %s 2>&1 | FileCheck 
-check-prefix CHECK-PIC-ERROR %s
 // RUN: %clang -### -target x86_64-windows -fpic %s 2>&1 | FileCheck 
-check-prefix CHECK-pic-ERROR %s
 // RUN: %clang -### -target x86_64-windows -fPIE %s 2>&1 | FileCheck 
-check-prefix CHECK-PIE-ERROR %s
@@ -43,15 +34,6 @@
 // RUN: %clang -### -target x86_64-windows-itanium -fPIE -fno-pie %s
 // RUN: %clang -### -target x86_64-windows-itanium -fpie -fno-pie %s
 
-// RUN: %clang -### -target x86_64-windows-gnu -fPIC %s 2>&1 | FileCheck 
-check-prefix CHECK-PIC-ERROR %s
-// RUN: %clang -### -target x86_64-windows-gnu -fpic %s 2>&1 | FileCheck 
-check-prefix CHECK-pic-ERROR %s
-// RUN: %clang -### -target x86_64-windows-gnu -fPIE %s 2>&1 | FileCheck 
-check-prefix CHECK-PIE-ERROR %s
-// RUN: %clang -### -target x86_64-windows-gnu -fpie %s 2>&1 | FileCheck 
-check-prefix CHECK-pie-ERROR %s
-// RUN: %clang -### -target x86_64-windows-gnu -fPIC -fno-pic %s
-// RUN: %clang -### -target x86_64-windows-gnu -Fpic -fno-pic %s
-// RUN: %clang -### -target x86_64-windows-gnu -fPIE -fno-pie %s
-// RUN: %clang -### -target x86_64-windows-gnu -fpie -fno-pie %s
-
 // CHECK-PIC-ERROR: unsupported option '-fPIC' for target '{{.*}}
 // CHECK-pic-ERROR: unsupported option '-fpic' for target '{{.*}}
 // CHECK-PIE-ERROR: unsupported option '-fPIE' for target '{{.*}}
Index: clang/test/Driver/pic.c
===
--- clang/test/Driver/pic.c
+++ clang/test/Driver/pic.c
@@ -301,3 +301,13 @@
 // RUN:   | FileCheck %s --check-prefix=CHECK-PIC2
 // RUN: %clang -c %s -target aarch64-windows-gnu -### 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-PIC2
+//
+// On MinGW, allow specifying -fPIC & friends but ignore them
+// RUN: %clang -fno-PIC -c %s -target x86_64-pc-windows-gnu -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-PIC2
+// RUN: %clang -fPIC -c %s -target i686-pc-windows-gnu -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-NO-PIC
+// RUN: %clang -fno-PIC -c %s -target aarch64-pc-windows-gnu -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-PIC2
+// RUN: %clang -fPIC -c %s -target armv7-pc-windows-gnu -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-NO-PIC
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -486,10 +486,7 @@
   return false;
 }
 
-bool toolchains::MinGW::isPICDefaultForced() const {
-  return getArch() == llvm::Triple::x86_64 ||
- getArch() == llvm::Triple::aarch64;
-}
+bool toolchains::MinGW::isPICDefaultForced() const { return true; }
 
 llvm::ExceptionHandling
 toolchains::MinGW::GetExceptionModel(const ArgList &Args) const {
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1186,10 +1186,9 @@
 options::OPT_fpic, options::OPT_fno_pic,
 options::OPT_fPIE, options::OPT_fno_PIE,
 options::OPT_fpie, options::OPT_fno_pie);
-  if (Triple.isOSWindows() && LastPICArg &&
-  La

[PATCH] D116283: [clang-format] Add an option to add a space between operator overloading and opening parentheses

[Rajat Bajpai via Phabricator via cfe-commits]

rajatbajpai updated this revision to Diff 396922.
rajatbajpai added a comment.

Incorporated review comments.

1. Added Check Parse test case for //SpaceBeforeParensOptions//.
2. Renamed the option to //AfterOverloadedOperator//.
3. Added operator overloading instantiation scenario in the unit test as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116283

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.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
@@ -14542,6 +14542,24 @@
   // verifyFormat("X A::operator++ (T);", SomeSpace2);
   verifyFormat("int x = int (y);", SomeSpace2);
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace2);
+
+  FormatStyle SpaceAfterOverloadedOperator = getLLVMStyle();
+  SpaceAfterOverloadedOperator.SpaceBeforeParens = FormatStyle::SBPO_Custom;
+  SpaceAfterOverloadedOperator.SpaceBeforeParensOptions
+  .AfterOverloadedOperator = true;
+
+  verifyFormat("auto operator++ () -> int;", SpaceAfterOverloadedOperator);
+  verifyFormat("X A::operator++ ();", SpaceAfterOverloadedOperator);
+  verifyFormat("some_object.operator++ ();", SpaceAfterOverloadedOperator);
+  verifyFormat("auto func() -> int;", SpaceAfterOverloadedOperator);
+
+  SpaceAfterOverloadedOperator.SpaceBeforeParensOptions
+  .AfterOverloadedOperator = false;
+
+  verifyFormat("auto operator++() -> int;", SpaceAfterOverloadedOperator);
+  verifyFormat("X A::operator++();", SpaceAfterOverloadedOperator);
+  verifyFormat("some_object.operator++();", SpaceAfterOverloadedOperator);
+  verifyFormat("auto func() -> int;", SpaceAfterOverloadedOperator);
 }
 
 TEST_F(FormatTest, SpaceAfterLogicalNot) {
@@ -18719,6 +18737,15 @@
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, SplitEmptyFunction);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, SplitEmptyRecord);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, SplitEmptyNamespace);
+  CHECK_PARSE_NESTED_BOOL(SpaceBeforeParensOptions, AfterControlStatements);
+  CHECK_PARSE_NESTED_BOOL(SpaceBeforeParensOptions, AfterForeachMacros);
+  CHECK_PARSE_NESTED_BOOL(SpaceBeforeParensOptions,
+  AfterFunctionDeclarationName);
+  CHECK_PARSE_NESTED_BOOL(SpaceBeforeParensOptions,
+  AfterFunctionDefinitionName);
+  CHECK_PARSE_NESTED_BOOL(SpaceBeforeParensOptions, AfterIfMacros);
+  CHECK_PARSE_NESTED_BOOL(SpaceBeforeParensOptions, AfterOverloadedOperator);
+  CHECK_PARSE_NESTED_BOOL(SpaceBeforeParensOptions, BeforeNonEmptyParentheses);
 }
 
 #undef CHECK_PARSE_BOOL
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2921,9 +2921,15 @@
 }
 
 bool TokenAnnotator::spaceRequiredBeforeParens(const FormatToken &Right) const {
-  return Style.SpaceBeforeParens == FormatStyle::SBPO_Always ||
- (Style.SpaceBeforeParensOptions.BeforeNonEmptyParentheses &&
-  Right.ParameterCount > 0);
+  if (Style.SpaceBeforeParens == FormatStyle::SBPO_Always)
+return true;
+  if (Right.is(TT_OverloadedOperatorLParen) &&
+  Style.SpaceBeforeParensOptions.AfterOverloadedOperator)
+return true;
+  if (Style.SpaceBeforeParensOptions.BeforeNonEmptyParentheses &&
+  Right.ParameterCount > 0)
+return true;
+  return false;
 }
 
 bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -855,6 +855,7 @@
 IO.mapOptional("AfterFunctionDeclarationName",
Spacing.AfterFunctionDeclarationName);
 IO.mapOptional("AfterIfMacros", Spacing.AfterIfMacros);
+IO.mapOptional("AfterOverloadedOperator", Spacing.AfterOverloadedOperator);
 IO.mapOptional("BeforeNonEmptyParentheses",
Spacing.BeforeNonEmptyParentheses);
   }
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -3368,6 +3368,13 @@
 ///   
 /// \endcode
 bool AfterIfMacros;
+/// If ``true``, put a space between operator overloading and opening
+/// parentheses.
+/// \code
+///true:  false:
+///void operator++ (int a);vs.void operator++(int a);
+/// \endcode
+bool AfterOverloadedOperator;
 /// If ``true``, put a space before opening parentheses only if the
 /// parentheses are not emp

[PATCH] D114425: [clang] Add __builtin_bswap128

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D114425#3209794 , @craig.topper 
wrote:

> What does the builtin due if __int128 isn't supported? Even though the type 
> isn't legal the builtin can still be called with a narrower type that would 
> be implicitly converted. Does that work correctly?

Would the correct behavior be to throw an error in that case? Or what exactly 
do you expect?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114425

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

[PATCH] D114425: [clang] Add __builtin_bswap128

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

xbolva00 added a comment.

int num = …;

__builtin_bswap64(num); // works, no error
__builtin_bswap128(num); // should work as well


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114425

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

[PATCH] D116283: [clang-format] Add an option to add a space between operator overloading and opening parentheses

[MyDeveloperDay via Phabricator via cfe-commits]

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

LGTM, could you add a release note into docs/ReleaseNotes.rst?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116283

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

[PATCH] D100810: [llvm] Use `GNUInstallDirs` to support custom installation dirs

[John Ericson via Phabricator via cfe-commits]

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

Mark some old threads done.




Comment at: llvm/cmake/modules/CMakeLists.txt:3
 
-set(LLVM_INSTALL_PACKAGE_DIR lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm)
+set(LLVM_INSTALL_PACKAGE_DIR lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm CACHE STRING 
"Path for CMake subdirectory (defaults to 'cmake/llvm')")
 set(llvm_cmake_builddir "${LLVM_BINARY_DIR}/${LLVM_INSTALL_PACKAGE_DIR}")

Ericson2314 wrote:
> compnerd wrote:
> > Why is this variable being put into the cache now?
> I wanted all the installation dirs to be in there for consistency (c.f. Did 
> the same for compiler-rt in D105765).
> 
> FWIW I have a yet-unposted commit I want to improve a bit to make it more 
> genuinely user-configurable too (we want the CMake with the headers so it 
> need not be installed runtime)
I see I put this back for now, that's good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100810

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

[PATCH] D100810: [llvm] Use `GNUInstallDirs` to support custom installation dirs

[John Ericson via Phabricator via cfe-commits]

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



1. Updating D100810 : [llvm] 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

Robust


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100810

Files:
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/AddSphinxTarget.cmake
  llvm/cmake/modules/CMakeLists.txt
  llvm/cmake/modules/LLVMInstallSymlink.cmake
  llvm/docs/CMake.rst
  llvm/examples/Bye/CMakeLists.txt
  llvm/tools/llvm-config/BuildVariables.inc.in
  llvm/tools/llvm-config/llvm-config.cpp
  llvm/tools/lto/CMakeLists.txt
  llvm/tools/opt-viewer/CMakeLists.txt
  llvm/tools/remarks-shlib/CMakeLists.txt

Index: llvm/tools/remarks-shlib/CMakeLists.txt
===
--- llvm/tools/remarks-shlib/CMakeLists.txt
+++ llvm/tools/remarks-shlib/CMakeLists.txt
@@ -19,7 +19,7 @@
   endif()
   
   install(FILES ${LLVM_MAIN_INCLUDE_DIR}/llvm-c/Remarks.h
-DESTINATION include/llvm-c
+DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/llvm-c"
 COMPONENT Remarks)
 
   if (APPLE)
Index: llvm/tools/opt-viewer/CMakeLists.txt
===
--- llvm/tools/opt-viewer/CMakeLists.txt
+++ llvm/tools/opt-viewer/CMakeLists.txt
@@ -8,7 +8,7 @@
 
 foreach (file ${files})
   install(PROGRAMS ${file}
-DESTINATION share/opt-viewer
+DESTINATION "${CMAKE_INSTALL_DATADIR}/opt-viewer"
 COMPONENT opt-viewer)
 endforeach (file)
 
Index: llvm/tools/lto/CMakeLists.txt
===
--- llvm/tools/lto/CMakeLists.txt
+++ llvm/tools/lto/CMakeLists.txt
@@ -33,7 +33,7 @@
 ${SOURCES} DEPENDS intrinsics_gen)
 
 install(FILES ${LLVM_MAIN_INCLUDE_DIR}/llvm-c/lto.h
-  DESTINATION include/llvm-c
+  DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/llvm-c"
   COMPONENT LTO)
 
 if (APPLE)
Index: llvm/tools/llvm-config/llvm-config.cpp
===
--- llvm/tools/llvm-config/llvm-config.cpp
+++ llvm/tools/llvm-config/llvm-config.cpp
@@ -357,10 +357,16 @@
 ("-I" + ActiveIncludeDir + " " + "-I" + ActiveObjRoot + "/include");
   } else {
 ActivePrefix = CurrentExecPrefix;
-ActiveIncludeDir = ActivePrefix + "/include";
-SmallString<256> path(LLVM_TOOLS_INSTALL_DIR);
-sys::fs::make_absolute(ActivePrefix, path);
-ActiveBinDir = std::string(path.str());
+{
+  SmallString<256> Path(LLVM_INSTALL_INCLUDEDIR);
+  sys::fs::make_absolute(ActivePrefix, Path);
+  ActiveIncludeDir = std::string(Path.str());
+}
+{
+  SmallString<256> Path(LLVM_TOOLS_INSTALL_DIR);
+  sys::fs::make_absolute(ActivePrefix, Path);
+  ActiveBinDir = std::string(Path.str());
+}
 ActiveLibDir = ActivePrefix + "/lib" + LLVM_LIBDIR_SUFFIX;
 ActiveCMakeDir = ActiveLibDir + "/cmake/llvm";
 ActiveIncludeOption = "-I" + ActiveIncludeDir;
Index: llvm/tools/llvm-config/BuildVariables.inc.in
===
--- llvm/tools/llvm-config/BuildVariables.inc.in
+++ llvm/tools/llvm-config/BuildVariables.inc.in
@@ -23,6 +23,7 @@
 #define LLVM_CXXFLAGS "@LLVM_CXXFLAGS@"
 #define LLVM_BUILDMODE "@LLVM_BUILDMODE@"
 #define LLVM_LIBDIR_SUFFIX "@LLVM_LIBDIR_SUFFIX@"
+#define LLVM_INSTALL_INCLUDEDIR "@CMAKE_INSTALL_INCLUDEDIR@"
 #define LLVM_TARGETS_BUILT "@LLVM_TARGETS_BUILT@"
 #define LLVM_SYSTEM_LIBS "@LLVM_SYSTEM_LIBS@"
 #define LLVM_BUILD_SYSTEM "@LLVM_BUILD_SYSTEM@"
Index: llvm/examples/Bye/CMakeLists.txt
===
--- llvm/examples/Bye/CMakeLists.txt
+++ llvm/examples/Bye/CMakeLists.txt
@@ -14,6 +14,6 @@
 BUILDTREE_ONLY
)
 
-  install(TARGETS ${name} RUNTIME DESTINATION examples)
+  install(TARGETS ${name} RUNTIME DESTINATION "${LLVM_EXAMPLES_INSTALL_DIR}")
   set_target_properties(${name} PROPERTIES FOLDER "Examples")
 endif()
Index: llvm/docs/CMake.rst
===
--- llvm/docs/CMake.rst
+++ llvm/docs/CMake.rst
@@ -252,6 +252,22 @@
   Sets the C++ standard to conform to when building LLVM.  Possible values are
   14, 17, 20.  LLVM Requires C++ 14 or higher.  This defaults to 14.
 
+**CMAKE_INSTALL_BINDIR**:PATH
+  The path to install executables, relative to the *CMAKE_INSTALL_PREFIX*.
+  Defaults to "bin".
+
+**CMAKE_INSTALL_INCLUDEDIR**:PATH
+  The path to install header files, relative to the *CMAKE_INSTALL_PREFIX*.
+  Defaults to "include".
+
+**CMAKE_INSTALL_DOCDIR**:PATH
+  The path to install documentation, relative to the *CMAKE_INSTALL_PREFIX*.
+  

[PATCH] D100810: [llvm] Use `GNUInstallDirs` to support custom installation dirs

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.



Comment at: llvm/CMakeLists.txt:75
 set(LLVM_ENABLE_PROJECTS "" CACHE STRING
-   "Semicolon-separated list of projects to build 
(${LLVM_KNOWN_PROJECTS}), or \"all\".")
+   "Semicolon-separated list of projects to build (${LLVM_KNOWN_PROJECTS}), or 
\"all\".")
 foreach(proj ${LLVM_ENABLE_PROJECTS})

Nit: This looks like a spurious unrelated change to whitespace?



Comment at: llvm/CMakeLists.txt:164
   set(LLVM_CCACHE_MAXSIZE "" CACHE STRING "Size of ccache")
-  set(LLVM_CCACHE_DIR "" CACHE STRING "Directory to keep ccached data")
+  set(LLVM_CCACHE_DIR "" CACHE PATH "Directory to keep ccached data")
   set(LLVM_CCACHE_PARAMS "CCACHE_CPP2=yes CCACHE_HASHDIR=yes"

Could this bit be split out to separate change, to keep this as small as 
possible - unless it's strictly needed and tied to this one?



Comment at: llvm/CMakeLists.txt:294
 
-set(LLVM_UTILS_INSTALL_DIR "${LLVM_TOOLS_INSTALL_DIR}" CACHE STRING
+set(LLVM_UTILS_INSTALL_DIR "${LLVM_TOOLS_INSTALL_DIR}" CACHE PATH
 "Path to install LLVM utilities (enabled by LLVM_INSTALL_UTILS=ON) 
(defaults to LLVM_TOOLS_INSTALL_DIR)")

Nit: Unrelated and can be split out?



Comment at: llvm/CMakeLists.txt:408
 
-set(LLVM_Z3_INSTALL_DIR "" CACHE STRING "Install directory of the Z3 solver.")
+set(LLVM_Z3_INSTALL_DIR "" CACHE PATH "Install directory of the Z3 solver.")
 

Unrelated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100810

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

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

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Can you add a release note to docs/ReleaseNotes.rst


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

https://reviews.llvm.org/D116314

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

[PATCH] D114425: [clang] Add __builtin_bswap128

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D114425#3216233 , @xbolva00 wrote:

> int num = …;
>
> __builtin_bswap64(num); // works, no error
> __builtin_bswap128(num); // should work as well
>
> Highly likely this case works already with ur patch, so it would be great to 
> add a test for this scenario.

This should already be tested, because `0x1234` is an `int` and should be 
implicitly converted to `__int128`.

The question was about the scenario where clang doesn't support `__int128`. 
`__builtin_bswap128()` returns an `unsigned __int128`. This would allow someone 
to do weird stuff like:

  using int128 = decltype(__builtin_bswap128(0));
  static_assert(sizeof(int128) == 16);

This actually works with the current patch and doesn't seem like it should work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114425

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

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

[ksyx via Phabricator via cfe-commits]

ksyx updated this revision to Diff 396926.
ksyx added a comment.

Add release note entry.


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

https://reviews.llvm.org/D116314

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/CMakeLists.txt
  clang/lib/Format/DefinitionBlockSeparator.cpp
  clang/lib/Format/DefinitionBlockSeparator.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/unittests/Format/CMakeLists.txt
  clang/unittests/Format/DefinitionBlockSeparatorTest.cpp

Index: clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
===
--- /dev/null
+++ clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
@@ -0,0 +1,309 @@
+//===- DefinitionBlockSeparatorTest.cpp - Formatting unit tests ---===//
+//
+// 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 "FormatTestUtils.h"
+#include "clang/Format/Format.h"
+
+#include "llvm/Support/Debug.h"
+#include "gtest/gtest.h"
+
+#define DEBUG_TYPE "definition-block-separator-test"
+
+namespace clang {
+namespace format {
+namespace {
+
+class DefinitionBlockSeparatorTest : public ::testing::Test {
+protected:
+  static std::string
+  separateDefinitionBlocks(llvm::StringRef Code,
+   const std::vector &Ranges,
+   const FormatStyle &Style = getLLVMStyle()) {
+LLVM_DEBUG(llvm::errs() << "---\n");
+LLVM_DEBUG(llvm::errs() << Code << "\n\n");
+tooling::Replacements Replaces = reformat(Style, Code, Ranges, "");
+auto Result = applyAllReplacements(Code, Replaces);
+EXPECT_TRUE(static_cast(Result));
+LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+return *Result;
+  }
+
+  static std::string
+  separateDefinitionBlocks(llvm::StringRef Code,
+   const FormatStyle &Style = getLLVMStyle()) {
+return separateDefinitionBlocks(
+Code,
+/*Ranges=*/{1, tooling::Range(0, Code.size())}, Style);
+  }
+
+  static void verifyFormat(llvm::StringRef Code,
+   const FormatStyle &Style = getLLVMStyle(),
+   llvm::StringRef ExpectedCode = "") {
+bool HasOriginalCode = true;
+if (ExpectedCode == "") {
+  ExpectedCode = Code;
+  HasOriginalCode = false;
+}
+
+FormatStyle InverseStyle = Style;
+if (Style.SeparateDefinitionBlocks == FormatStyle::SDS_Always)
+  InverseStyle.SeparateDefinitionBlocks = FormatStyle::SDS_Never;
+else
+  InverseStyle.SeparateDefinitionBlocks = FormatStyle::SDS_Always;
+EXPECT_EQ(ExpectedCode.str(), separateDefinitionBlocks(ExpectedCode, Style))
+<< "Expected code is not stable";
+std::string InverseResult = separateDefinitionBlocks(Code, InverseStyle);
+EXPECT_NE(Code.str(), InverseResult)
+<< "Inverse formatting makes no difference";
+std::string CodeToFormat =
+HasOriginalCode ? Code.str() : removeEmptyLines(Code);
+std::string Result = separateDefinitionBlocks(CodeToFormat, Style);
+EXPECT_EQ(ExpectedCode.str(), Result) << "Test failed. Formatted:\n"
+  << Result;
+  }
+
+  static std::string removeEmptyLines(llvm::StringRef Code) {
+std::string Result = "";
+for (auto Char : Code.str()) {
+  if (Result.size()) {
+auto LastChar = Result.back();
+if ((Char == '\n' && LastChar == '\n') ||
+(Char == '\r' && (LastChar == '\r' || LastChar == '\n')))
+  continue;
+  }
+  Result.push_back(Char);
+}
+return Result;
+  }
+};
+
+TEST_F(DefinitionBlockSeparatorTest, Basic) {
+  FormatStyle Style = getLLVMStyle();
+  Style.SeparateDefinitionBlocks = FormatStyle::SDS_Always;
+  verifyFormat("int foo(int i, int j) {\n"
+   "  int r = i + j;\n"
+   "  return r;\n"
+   "}\n"
+   "\n"
+   "int bar(int j, int k) {\n"
+   "  int r = j + k;\n"
+   "  return r;\n"
+   "}",
+   Style);
+
+  verifyFormat("struct foo {\n"
+   "  int i, j;\n"
+   "};\n"
+   "\n"
+   "struct bar {\n"
+   "  int j, k;\n"
+   "};",
+   Style);
+
+  verifyFormat("class foo {\n"
+   "  int i, j;\n"
+   "};\n"
+   "\n"
+   "class bar {\n"
+   "  int j, k;\n"
+   "};",
+   Style);
+
+  verifyFormat("namespace foo {\n"
+   "int i, j;\n"
+   "}\n"
+   "\n"
+ 

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

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:141
 ^^
+- Added a setting ``bugprone-assert-side-effect.FunctionExceptions`` for
+  a comma-separated list of the names of functions or methods to be considered

Please separate with newline and use single back-ticks for options.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:145
 
 - Removed default setting 
``cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors = "true"``,
   to match the current state of the C++ Core Guidelines.

Ditto for back-ticks.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:150
   ` to diagnose and fix 
functional
   casts, to achieve feature parity with the corresponding ``cpplint.py`` check.
 

Ditto for back-ticks.


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

https://reviews.llvm.org/D116478

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

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

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis updated this revision to Diff 396927.

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

https://reviews.llvm.org/D116478

Files:
  clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-assert-side-effect.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s bugprone-assert-side-effect %t -- -config="{CheckOptions: [{key: bugprone-assert-side-effect.CheckFunctionCalls, value: true}, {key: bugprone-assert-side-effect.AssertMacros, value: 'assert,assert2,my_assert,convoluted_assert,msvc_assert'}]}" -- -fexceptions
+// RUN: %check_clang_tidy %s bugprone-assert-side-effect %t -- -config="{CheckOptions: [{key: bugprone-assert-side-effect.CheckFunctionCalls, value: true}, {key: bugprone-assert-side-effect.AssertMacros, value: 'assert,assert2,my_assert,convoluted_assert,msvc_assert'}, {key: bugprone-assert-side-effect.FunctionExceptions, value: 'badButExceptedFunc'}]}" -- -fexceptions
 
 //===--- assert definition block --===//
 int abort() { return 0; }
@@ -46,6 +46,7 @@
 class MyClass {
 public:
   bool badFunc(int a, int b) { return a * b > 0; }
+  bool badButExceptedFunc(int a, int b) { return a * b > 0; }
   bool goodFunc(int a, int b) const { return a * b > 0; }
 
   MyClass &operator=(const MyClass &rhs) { return *this; }
@@ -87,6 +88,7 @@
   MyClass mc;
   assert(mc.badFunc(0, 1));
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in assert() condition discarded in release builds
+  assert(mc.badButExceptedFunc(0, 1));
   assert(mc.goodFunc(0, 1));
 
   MyClass mc2;
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-assert-side-effect.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone-assert-side-effect.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-assert-side-effect.rst
@@ -21,3 +21,8 @@
Whether to treat non-const member and non-member functions as they produce
side effects. Disabled by default because it can increase the number of false
positive warnings.
+
+.. option:: FunctionExceptions
+
+   A comma-separated list of the names of functions or methods to be
+   considered as not having side-effects.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -139,16 +139,21 @@
 Changes in existing checks
 ^^
 
-- Removed default setting ``cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors = "true"``,
+- :doc:`bugprone-assert-side-effect `
+  check now supports a `FunctionExceptions` option to explicitly consider the specified
+  functions or methods as not any having side-effects.
+
+- Removed default setting `cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors = "true"`,
+  from :doc:`cppcoreguidelines-explicit-virtual-functions `
   to match the current state of the C++ Core Guidelines.
 
 - Updated :doc:`google-readability-casting
   ` to diagnose and fix functional
-  casts, to achieve feature parity with the corresponding ``cpplint.py`` check.
+  casts, to achieve feature parity with the corresponding `cpplint.py` check.
 
 - Fixed a false positive in :doc:`fuchsia-trailing-return
   ` for C++17 deduction guides.
-  
+
 - Fixed a false positive in :doc:`bugprone-throw-keyword-missing
   ` when creating an exception object
   using placement new
Index: clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.h
===
--- clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_ASSERTSIDEEFFECTCHECK_H
 
 #include "../ClangTidyCheck.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include 
@@ -33,6 +34,7 @@
 /// can increase the number of false positive warnings.
 class AssertSideEffectCheck : public ClangTidyCheck {
 public:
+  using FunctionExceptionType = llvm::SmallSet;
   AssertSideEffectCheck(StringRef Name, ClangTidyContext *Context);
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
@@ -41,7 +43,9 @@
 private:
   const bool CheckFunctionCal

[PATCH] D116494: [clang-format] spacesRequiredBetween is not honouring clang-format off/on

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: curdeius, HazardyKnusperkeks, owenpan, lahwaacz.
MyDeveloperDay added projects: clang, clang-format.
MyDeveloperDay requested review of this revision.

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

It seems that clang-format off/on is not being honoured in regard to adding 
spaces.

My understanding of clang-format off/on is that it marks the token as finalized 
based on whether formatting is currently enabled or disabled.

This was causing a space to be added between the `<` and `<<`  in the Cuda 
kernel `foo<<<1, 1>>>();`

This if doesn't solve this actual issue but ensure that clang-format is at 
least honoured.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116494

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -21172,6 +21172,22 @@
   verifyFormat("A< A< int > >();", Spaces);
   verifyFormat("A >();", Spaces);
   verifyFormat("A< A< int>>();", Spaces);
+
+  Spaces.SpacesInAngles = FormatStyle::SIAS_Always;
+  EXPECT_EQ("// clang-format off\n"
+"foo<<<1, 1>>>();\n"
+"// clang-format on\n",
+format("// clang-format off\n"
+   "foo<<<1, 1>>>();\n"
+   "// clang-format on\n",
+   Spaces));
+  EXPECT_EQ("// clang-format off\n"
+"foo< < <1, 1> > >();\n"
+"// clang-format on\n",
+format("// clang-format off\n"
+   "foo< < <1, 1> > >();\n"
+   "// clang-format on\n",
+   Spaces));
 }
 
 TEST_F(FormatTest, SpaceAfterTemplateKeyword) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3294,6 +3294,10 @@
 return Right.WhitespaceRange.getBegin() != Right.WhitespaceRange.getEnd();
   };
 
+  // If the token is finalized don't touch it (as it could be in a 
clang-format-off section).
+  if (Left.Finalized)
+  return HasExistingWhitespace();
+
   if (Right.Tok.getIdentifierInfo() && Left.Tok.getIdentifierInfo())
 return true; // Never ever merge two identifiers.
 


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -21172,6 +21172,22 @@
   verifyFormat("A< A< int > >();", Spaces);
   verifyFormat("A >();", Spaces);
   verifyFormat("A< A< int>>();", Spaces);
+
+  Spaces.SpacesInAngles = FormatStyle::SIAS_Always;
+  EXPECT_EQ("// clang-format off\n"
+"foo<<<1, 1>>>();\n"
+"// clang-format on\n",
+format("// clang-format off\n"
+   "foo<<<1, 1>>>();\n"
+   "// clang-format on\n",
+   Spaces));
+  EXPECT_EQ("// clang-format off\n"
+"foo< < <1, 1> > >();\n"
+"// clang-format on\n",
+format("// clang-format off\n"
+   "foo< < <1, 1> > >();\n"
+   "// clang-format on\n",
+   Spaces));
 }
 
 TEST_F(FormatTest, SpaceAfterTemplateKeyword) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3294,6 +3294,10 @@
 return Right.WhitespaceRange.getBegin() != Right.WhitespaceRange.getEnd();
   };
 
+  // If the token is finalized don't touch it (as it could be in a clang-format-off section).
+  if (Left.Finalized)
+  return HasExistingWhitespace();
+
   if (Right.Tok.getIdentifierInfo() && Left.Tok.getIdentifierInfo())
 return true; // Never ever merge two identifiers.
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116494: [clang-format] spacesRequiredBetween is not honouring clang-format off/on

[MyDeveloperDay via Phabricator via cfe-commits]

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

Switch to verifyFormat in the tests


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

https://reviews.llvm.org/D116494

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -21172,6 +21172,16 @@
   verifyFormat("A< A< int > >();", Spaces);
   verifyFormat("A >();", Spaces);
   verifyFormat("A< A< int>>();", Spaces);
+
+  Spaces.SpacesInAngles = FormatStyle::SIAS_Always;
+  verifyFormat("// clang-format off\n"
+   "foo<<<1, 1>>>();\n"
+   "// clang-format on\n",
+   Spaces);
+  verifyFormat("// clang-format off\n"
+   "foo< < <1, 1> > >();\n"
+   "// clang-format on\n",
+   Spaces);
 }
 
 TEST_F(FormatTest, SpaceAfterTemplateKeyword) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3294,6 +3294,11 @@
 return Right.WhitespaceRange.getBegin() != Right.WhitespaceRange.getEnd();
   };
 
+  // If the token is finalized don't touch it (as it could be in a
+  // clang-format-off section).
+  if (Left.Finalized)
+return HasExistingWhitespace();
+
   if (Right.Tok.getIdentifierInfo() && Left.Tok.getIdentifierInfo())
 return true; // Never ever merge two identifiers.
 


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -21172,6 +21172,16 @@
   verifyFormat("A< A< int > >();", Spaces);
   verifyFormat("A >();", Spaces);
   verifyFormat("A< A< int>>();", Spaces);
+
+  Spaces.SpacesInAngles = FormatStyle::SIAS_Always;
+  verifyFormat("// clang-format off\n"
+   "foo<<<1, 1>>>();\n"
+   "// clang-format on\n",
+   Spaces);
+  verifyFormat("// clang-format off\n"
+   "foo< < <1, 1> > >();\n"
+   "// clang-format on\n",
+   Spaces);
 }
 
 TEST_F(FormatTest, SpaceAfterTemplateKeyword) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3294,6 +3294,11 @@
 return Right.WhitespaceRange.getBegin() != Right.WhitespaceRange.getEnd();
   };
 
+  // If the token is finalized don't touch it (as it could be in a
+  // clang-format-off section).
+  if (Left.Finalized)
+return HasExistingWhitespace();
+
   if (Right.Tok.getIdentifierInfo() && Left.Tok.getIdentifierInfo())
 return true; // Never ever merge two identifiers.
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D7982: Add readability-duplicate-include check to clang-tidy

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood updated this revision to Diff 396932.
LegalizeAdulthood added a comment.
Herald added a subscriber: carlosgalvezp.
Herald added a project: clang-tools-extra.

Revive review with updated diff on top-of-tree


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D7982

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp
  clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.h
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-duplicate-include.rst
  clang-tools-extra/test/clang-tidy/checkers/readability-duplicate-include.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-duplicate-include.h
  clang-tools-extra/test/clang-tidy/checkers/types.h

Index: clang-tools-extra/test/clang-tidy/checkers/types.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/types.h
@@ -0,0 +1 @@
+// empty file used by readability-duplicate-include.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/readability-duplicate-include.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-duplicate-include.h
@@ -0,0 +1 @@
+// empty file used by readability-duplicate-include.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/readability-duplicate-include.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-duplicate-include.cpp
@@ -0,0 +1,73 @@
+// RUN: %check_clang_tidy %s readability-duplicate-include %t -- -std=c++11 -I$(dirname %s)
+// REQUIRES: shell
+
+int a;
+#include 
+int b;
+#include 
+int c;
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: duplicate include [readability-duplicate-include]
+// CHECK-FIXES:  {{^int a;$}}
+// CHECK-FIXES-NEXT: {{^#include $}}
+// CHECK-FIXES-NEXT: {{^int b;$}}
+// CHECK-FIXES-NEXT: {{^int c;$}}
+
+int d;
+#include 
+int e;
+#include  // extra stuff that will also be removed
+int f;
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: {{.*}}
+// CHECK-FIXES:  {{^int d;$}}
+// CHECK-FIXES-NEXT: {{^#include $}}
+// CHECK-FIXES-NEXT: {{^int e;$}}
+// CHECK-FIXES-NEXT: {{^int f;$}}
+
+int g;
+#include "readability-duplicate-include.h"
+int h;
+#include "readability-duplicate-include.h"
+int i;
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: {{.*}}
+// CHECK-FIXES:  {{^int g;$}}
+// CHECK-FIXES-NEXT: {{^#include "readability-duplicate-include.h"$}}
+// CHECK-FIXES-NEXT: {{^int h;$}}
+// CHECK-FIXES-NEXT: {{^int i;$}}
+
+#include "types.h"
+
+int j;
+#include 
+int k;
+#include 
+int l;
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: {{.*}}
+// CHECK-FIXES:  {{^int j;$}}
+// CHECK-FIXES-NEXT: {{^#include $}}
+// CHECK-FIXES-NEXT: {{^int k;$}}
+// CHECK-FIXES-NEXT: {{^int l;$}}
+
+int m;
+#  include   // lots of space
+int n;
+// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: {{.*}}
+// CHECK-FIXES:  {{^int m;$}}
+// CHECK-FIXES-NEXT: {{^int n;$}}
+
+// defining a macro in the main file resets the included file cache
+#define ARBITRARY_MACRO
+int o;
+#include 
+int p;
+// CHECK-FIXES:  {{^int o;$}}
+// CHECK-FIXES-NEXT: {{^#include $}}
+// CHECK-FIXES-NEXT: {{^int p;$}}
+
+// undefining a macro resets the cache
+#undef ARBITRARY_MACRO
+int q;
+#include 
+int r;
+// CHECK-FIXES:  {{^int q;$}}
+// CHECK-FIXES-NEXT: {{^#include $}}
+// CHECK-FIXES-NEXT: {{^int r;$}}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-duplicate-include.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/readability-duplicate-include.rst
@@ -0,0 +1,48 @@
+.. title:: clang-tidy - readability-duplicate-include
+
+readability-duplicate-include
+=
+
+Looks for duplicate includes and removes them.  The check maintains a list of
+included files and looks for duplicates.  If a macro is defined or undefined
+then the list of included files is cleared.
+
+A common LLVM preprocessor tactic is to define a macro that controls an
+included file and then include it multiple times in order to achieve different
+effects.  Resetting the list of seen includes every time a macro is defined
+or undefined prevents a false positive from that use case.
+
+Examples:
+
+.. code-block:: c++
+
+  #include 
+  #include 
+  #include 
+
+becomes
+
+.. code-block:: c++
+
+  #include 
+  #include 
+
+Because of the intervening macro definitions, this code remains unchanged:
+
+.. code-block:: c++
+
+  class LangOptionsBase {
+  public:
+// Define simple language options (with no accessors).
+  #de

[PATCH] D116385: [clangd] Code action for creating an ObjC initializer

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

This looks like a useful feature, I had been thinking about adding the 
equivalent for C++...

High level feedback:

- this should generate both the declaration and definition
- i doubt the interaction around subsetting fields is going to be satisfying
- don't spend complexity on lots of options/micromanaging formatting until 
basic functionality is solid




Comment at: 
clang-tools-extra/clangd/refactor/tweaks/ObjCMemberwiseInitializer.cpp:66
+
+// Returns the effective begin loc for D, including attached leading comments.
+static SourceLocation effectiveBeginLoc(const Decl *D,

If we're going to care about this (which makes sense), this should be a 
rangeIncludingComment() in AST.h or so



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/ObjCMemberwiseInitializer.cpp:85
+if (SM.isBeforeInTranslationUnit(DeclEnd, CommentEnd)) {
+  return CommentEnd;
+}

the two returns are inconsistent, this one is the first position outside the 
range, the other return is the last token (not character) in the range



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/ObjCMemberwiseInitializer.cpp:91
+
+struct LocationWithPadding {
+  SourceLocation Loc;

We try not to spend complexity on formatting beyond what clang-format cares 
about. 

If you're inserting before a member (or @end), always inserting 2 newlines 
seems close enough? (clang-format will strip consecutive blank lines)



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/ObjCMemberwiseInitializer.cpp:113
+if (MD->getMethodFamily() == OMF_init) {
+  Loc = effectiveEndLoc(MD, SM).getLocWithOffset(1);
+} else if (Loc.isValid()) {

This algorithm doesn't make much sense to me. 
You want to insert before the first non-init instance method, falling back to 
the end.
So why do you need to do more than search for the first non-init instance 
method, and find the location before it? In particular why are we looking at 
init methods and locations after them?



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/ObjCMemberwiseInitializer.cpp:141
+Name = ID.getName();
+if (Name.startswith("_"))
+  Name = Name.substr(1);

Name.consume_front("_"); (FWIW I don't think this needs a comment)



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/ObjCMemberwiseInitializer.cpp:199
+
+  // We support the following selected decls:
+  // - ObjCInterfaceDecl/ObjCImplementationDecl only - generate for all

I understand the desire to select the particular fields, but given the 
limitation of the interaction I wonder whether these cases are useful & 
discoverable enough to be worth the complexity.

- how often is an initializer that sets one variable of many useful?
- out of the times when a subset of variables should be initialized, how often 
are they contiguous in the code?

I think it would likely be better to start with initializing all members...



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/ObjCMemberwiseInitializer.cpp:218
+} else if (const auto *ID = dyn_cast(DC)) {
+  Container = ID;
+}

while here, also grab the InterfaceDecl into another field so you don't have to 
go casehunting again in initParams?



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/ObjCMemberwiseInitializer.cpp:224
+
+void ObjCMemberwiseInitializer::initParams(
+const SelectionTree::Node *N, SmallVectorImpl &Params) {

can you rename this method to something clearer?
unclear whether "init" stands for "initialize" or "initializer" here, which 
would refer to pretty different things!



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/ObjCMemberwiseInitializer.cpp:251
+  }
+  // If just the Container itself was selected, fetch all properties and ivars
+  // for the given class.

Inferring the container was selected by looking at HadBadNode/Params seems very 
indirect.
Why not handle this case at the top? `if (Container == 
N->ASTNode.get()) return getAllParams(Iface)`




Comment at: 
clang-tools-extra/clangd/refactor/tweaks/ObjCMemberwiseInitializer.cpp:282
+  const SelectionTree::Node *N = Inputs.ASTSelection.commonAncestor();
+  if (!N || !Container)
+return error("Invalid selection");

!Container isn't possible here



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/ObjCMemberwiseInitializer.cpp:285
+
+  SmallVector Params;
+  initParams(N, Params);

nit: why isn't this a return value?



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/ObjCMemberwiseInitializer.cpp:290
+
+  bool GenerateImpl = isa(Container);
+  llvm::SmallString<256> Text(std::string(LocPadding.PadStart, '\n'));

Wait, so

[PATCH] D7982: Add readability-duplicate-include check to clang-tidy

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Functionality-wise this check is superseded by Include What You Use 
.




Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-duplicate-include.rst:6
+
+Looks for duplicate includes and removes them.  The check maintains a list of
+included files and looks for duplicates.  If a macro is defined or undefined

Please fix double spaces. Same below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D7982

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

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

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood updated this revision to Diff 396933.
LegalizeAdulthood added a comment.

Document the new matcher in the clang release notes


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

https://reviews.llvm.org/D116328

Files:
  clang/docs/LibASTMatchersReference.html
  clang/docs/ReleaseNotes.rst
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -145,6 +145,23 @@
 HasDescendantVariableI));
 }
 
+TEST_P(ASTMatchersTest, HasCaseSubstmt) {
+  EXPECT_TRUE(matches(
+  "void f() { switch (1) { case 1: return; break; default: break; } }",
+  traverse(TK_AsIs, caseStmt(hasSubstatement(returnStmt());
+}
+
+TEST_P(ASTMatchersTest, HasDefaultSubstmt) {
+  EXPECT_TRUE(matches(
+  "void f() { switch (1) { case 1: return; break; default: break; } }",
+  traverse(TK_AsIs, defaultStmt(hasSubstatement(breakStmt());
+}
+
+TEST_P(ASTMatchersTest, HasLabelSubstmt) {
+  EXPECT_TRUE(matches("void f() { while (1) { bar: break; foo: return; } }",
+  traverse(TK_AsIs, labelStmt(hasSubstatement(breakStmt());
+}
+
 TEST(TypeMatcher, MatchesClassType) {
   TypeMatcher TypeA = hasDeclaration(recordDecl(hasName("A")));
 
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -66,7 +66,7 @@
 
 #define REGISTER_MATCHER(name) \
   registerMatcher(#name, internal::makeMatcherAutoMarshall(\
- ::clang::ast_matchers::name, #name));
+ ::clang::ast_matchers::name, #name))
 
 #define REGISTER_MATCHER_OVERLOAD(name)\
   registerMatcher(#name,   \
@@ -143,7 +143,7 @@
   REGISTER_MATCHER(atomicType);
   REGISTER_MATCHER(attr);
   REGISTER_MATCHER(autoType);
-  REGISTER_MATCHER(autoreleasePoolStmt)
+  REGISTER_MATCHER(autoreleasePoolStmt);
   REGISTER_MATCHER(binaryConditionalOperator);
   REGISTER_MATCHER(binaryOperator);
   REGISTER_MATCHER(binaryOperation);
@@ -355,6 +355,7 @@
   REGISTER_MATCHER(hasSpecializedTemplate);
   REGISTER_MATCHER(hasStaticStorageDuration);
   REGISTER_MATCHER(hasStructuredBlock);
+  REGISTER_MATCHER(hasSubstatement);
   REGISTER_MATCHER(hasSyntacticForm);
   REGISTER_MATCHER(hasTargetDecl);
   REGISTER_MATCHER(hasTemplateArgument);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2429,6 +2429,7 @@
 /// Matches co_await expressions where the type of the promise is dependent
 extern const internal::VariadicDynCastAllOfMatcher
 dependentCoawaitExpr;
+
 /// Matches co_yield expressions.
 ///
 /// Given
@@ -7716,6 +7717,29 @@
   return InnerMatcher.matches(*Node.getLHS(), Finder, Builder);
 }
 
+/// Matches the substatement associated with a case, default or label statement.
+///
+/// Given
+/// \code
+///   switch (1) { case 1: break; case 2: return; break; default: return; break;
+///   }
+///   foo: return;
+///   bar: break;
+/// \endcode
+///
+/// caseStmt(hasSubstmt(returnStmt()))
+///   matches "case 2: return;"
+/// defaultStmt(hasSubstmt(returnStmt()))
+///   matches "default: return;"
+/// labelStmt(hasSubstmt(breakStmt()))
+///   matches "bar: break;"
+AST_POLYMORPHIC_MATCHER_P(hasSubstatement,
+  AST_POLYMORPHIC_SUPPORTED_TYPES(CaseStmt, DefaultStmt,
+  LabelStmt),
+  internal::Matcher, InnerMatcher) {
+  return InnerMatcher.matches(*Node.getSubStmt(), Finder, Builder);
+}
+
 /// Matches declaration that has a given attribute.
 ///
 /// Given
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -277,6 +277,9 @@
   and the underlying ``Type`` with ``hasUnderlyingType``.
   ``hasDeclaration`` continues to see through the alias and apply to the
   underlying type.
+- The ``hasSubstatement`` matcher is now available and allows you to match the
+  statement associated with a labelled construct: case statement, default
+  statement or label statement.
 
 clang-format
 
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersRe

[clang] d4f0978 - [clang] More informative mixed namespace diagnostics

[Nathan Sidwell via cfe-commits]

Author: Nathan Sidwell
Date: 2022-01-02T12:23:13-05:00
New Revision: d4f09786e079361eba1ade1e351be8771d016f29

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

LOG: [clang] More informative mixed namespace diagnostics

First, let's check we get a TemplateDecl, before complaining about
where it might have been found.

Second, if it came from an unexpected place, show where that location is.

Reviewed By: ChuanqiXu

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

Added: 


Modified: 
clang/lib/Sema/SemaCoroutine.cpp
clang/test/SemaCXX/co_await-range-for-exp-namespace.cpp
clang/test/SemaCXX/coreturn-eh-exp-namespace.cpp
clang/test/SemaCXX/coreturn-exp-namespace.cpp
clang/test/SemaCXX/coroutine-final-suspend-noexcept-exp-namespace.cpp
clang/test/SemaCXX/coroutine-mixed-exp-namespace.cpp
clang/test/SemaCXX/coroutine-mixed2-exp-namespace.cpp
clang/test/SemaCXX/coroutine-rvo-exp-namespace.cpp
clang/test/SemaCXX/coroutine-seh-exp-namespace.cpp
clang/test/SemaCXX/coroutine-traits-undefined-template-exp-namespace.cpp
clang/test/SemaCXX/coroutine-unhandled_exception-warning-exp-namespace.cpp
clang/test/SemaCXX/coroutine_handle-address-return-type-exp-namespace.cpp
clang/test/SemaCXX/coroutines-exp-namespace.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaCoroutine.cpp 
b/clang/lib/Sema/SemaCoroutine.cpp
index e89cecd08ccae..3a6d9f0b9f265 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -1740,30 +1740,38 @@ ClassTemplateDecl 
*Sema::lookupCoroutineTraits(SourceLocation KwLoc,
   return nullptr;
 }
 
-if (!InStd) {
-  // Found only in std::experimental.
-  Diag(KwLoc, diag::warn_deprecated_coroutine_namespace)
-  << "coroutine_traits";
-} else if (InExp) {
-  // Found in std and std::experimental.
-  Diag(KwLoc,
-   diag::err_mixed_use_std_and_experimental_namespace_for_coroutine);
-  Diag(KwLoc, diag::warn_deprecated_coroutine_namespace)
-  << "coroutine_traits";
-  return nullptr;
-}
-
 // Prefer ::std to std::experimental.
 auto &Result = InStd ? ResStd : ResExp;
 CoroTraitsNamespaceCache = InStd ? StdSpace : ExpSpace;
 
 // coroutine_traits is required to be a class template.
-if (!(StdCoroutineTraitsCache = Result.getAsSingle())) {
+StdCoroutineTraitsCache = Result.getAsSingle();
+if (!StdCoroutineTraitsCache) {
   Result.suppressDiagnostics();
   NamedDecl *Found = *Result.begin();
   Diag(Found->getLocation(), diag::err_malformed_std_coroutine_traits);
   return nullptr;
 }
+
+if (InExp) {
+  // Found in std::experimental
+  Diag(KwLoc, diag::warn_deprecated_coroutine_namespace)
+  << "coroutine_traits";
+  ResExp.suppressDiagnostics();
+  auto *Found = *ResExp.begin();
+  Diag(Found->getLocation(), diag::note_entity_declared_at) << Found;
+
+  if (InStd) {
+// Also found in std
+Diag(KwLoc,
+ diag::err_mixed_use_std_and_experimental_namespace_for_coroutine);
+Diag(StdCoroutineTraitsCache->getLocation(),
+ diag::note_entity_declared_at)
+<< StdCoroutineTraitsCache;
+
+return nullptr;
+  }
+}
   }
   Namespace = CoroTraitsNamespaceCache;
   return StdCoroutineTraitsCache;

diff  --git a/clang/test/SemaCXX/co_await-range-for-exp-namespace.cpp 
b/clang/test/SemaCXX/co_await-range-for-exp-namespace.cpp
index 75568505ab552..df6b8a4e86b36 100644
--- a/clang/test/SemaCXX/co_await-range-for-exp-namespace.cpp
+++ b/clang/test/SemaCXX/co_await-range-for-exp-namespace.cpp
@@ -53,6 +53,7 @@ MyForLoopArrayAwaiter g() {
   for co_await (auto i : arr) {} // expected-warning {{support for 
std::experimental::coroutine_traits will be removed}}
   // expected-error@-1 {{call to deleted member function 'await_transform'}}
   // expected-note@-2 {{'await_transform' implicitly required by 'co_await' 
here}}
+  // expected-note@Inputs/std-coroutine-exp-namespace.h:8 {{'coroutine_traits' 
declared here}}
 }
 
 struct ForLoopAwaiterBadBeginTransform {

diff  --git a/clang/test/SemaCXX/coreturn-eh-exp-namespace.cpp 
b/clang/test/SemaCXX/coreturn-eh-exp-namespace.cpp
index 7d85c924f6690..facdedf14d01d 100644
--- a/clang/test/SemaCXX/coreturn-eh-exp-namespace.cpp
+++ b/clang/test/SemaCXX/coreturn-eh-exp-namespace.cpp
@@ -40,6 +40,7 @@ VoidTagReturnValue test() {
   object x = {};
   try {
 co_return {}; // expected-warning {{support for 
std::experimental::coroutine_traits will be removed}}
+// expected-note@Inputs/std-coroutine-exp-namespace.h:8 
{{'coroutine_traits' declared here}}
   } catch (...) {
 throw;
   }

diff  --git a/clang/test/SemaCXX/coreturn-exp-namespace.cpp 

[PATCH] D116164: [clang] More informative mixed namespace diagnostics

[Nathan Sidwell 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 rGd4f09786e079: [clang] More informative mixed namespace 
diagnostics (authored by urnathan).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116164

Files:
  clang/lib/Sema/SemaCoroutine.cpp
  clang/test/SemaCXX/co_await-range-for-exp-namespace.cpp
  clang/test/SemaCXX/coreturn-eh-exp-namespace.cpp
  clang/test/SemaCXX/coreturn-exp-namespace.cpp
  clang/test/SemaCXX/coroutine-final-suspend-noexcept-exp-namespace.cpp
  clang/test/SemaCXX/coroutine-mixed-exp-namespace.cpp
  clang/test/SemaCXX/coroutine-mixed2-exp-namespace.cpp
  clang/test/SemaCXX/coroutine-rvo-exp-namespace.cpp
  clang/test/SemaCXX/coroutine-seh-exp-namespace.cpp
  clang/test/SemaCXX/coroutine-traits-undefined-template-exp-namespace.cpp
  clang/test/SemaCXX/coroutine-unhandled_exception-warning-exp-namespace.cpp
  clang/test/SemaCXX/coroutine_handle-address-return-type-exp-namespace.cpp
  clang/test/SemaCXX/coroutines-exp-namespace.cpp

Index: clang/test/SemaCXX/coroutines-exp-namespace.cpp
===
--- clang/test/SemaCXX/coroutines-exp-namespace.cpp
+++ clang/test/SemaCXX/coroutines-exp-namespace.cpp
@@ -45,6 +45,7 @@
 
 template 
 struct coroutine_traits : public traits_sfinae_base {};
+// expected-note@-1{{declared here}}
 } // namespace experimental
 } // namespace std
 
Index: clang/test/SemaCXX/coroutine_handle-address-return-type-exp-namespace.cpp
===
--- clang/test/SemaCXX/coroutine_handle-address-return-type-exp-namespace.cpp
+++ clang/test/SemaCXX/coroutine_handle-address-return-type-exp-namespace.cpp
@@ -32,6 +32,7 @@
 
 template 
 struct coroutine_traits : public traits_sfinae_base {};
+// expected-note@-1{{declared here}}
 } // namespace std::experimental
 
 struct suspend_never {
Index: clang/test/SemaCXX/coroutine-unhandled_exception-warning-exp-namespace.cpp
===
--- clang/test/SemaCXX/coroutine-unhandled_exception-warning-exp-namespace.cpp
+++ clang/test/SemaCXX/coroutine-unhandled_exception-warning-exp-namespace.cpp
@@ -33,6 +33,7 @@
 #ifndef DISABLE_WARNING
 void test0() { // expected-warning {{'promise_void' is required to declare the member 'unhandled_exception()' when exceptions are enabled}}
   co_return;   // expected-warning {{support for std::experimental::coroutine_traits will be removed}}
+  // expected-note@Inputs/std-coroutine-exp-namespace.h:8 {{'coroutine_traits' declared here}}
 }
 #else
 void test0() { // expected-no-diagnostics
Index: clang/test/SemaCXX/coroutine-traits-undefined-template-exp-namespace.cpp
===
--- clang/test/SemaCXX/coroutine-traits-undefined-template-exp-namespace.cpp
+++ clang/test/SemaCXX/coroutine-traits-undefined-template-exp-namespace.cpp
@@ -6,7 +6,7 @@
 namespace std {
 namespace experimental {
 template 
-struct coroutine_traits {
+struct coroutine_traits { // expected-note{{declared here}}
   struct promise_type {};
 };
 
Index: clang/test/SemaCXX/coroutine-seh-exp-namespace.cpp
===
--- clang/test/SemaCXX/coroutine-seh-exp-namespace.cpp
+++ clang/test/SemaCXX/coroutine-seh-exp-namespace.cpp
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -std=c++1z -fcoroutines-ts -verify %s -fcxx-exceptions -fexceptions -triple x86_64-windows-msvc -fms-extensions
 namespace std::experimental {
 template  struct coroutine_traits;
+// expected-note@-1{{declared here}}
 
 template  struct coroutine_handle {
   coroutine_handle() = default;
Index: clang/test/SemaCXX/coroutine-rvo-exp-namespace.cpp
===
--- clang/test/SemaCXX/coroutine-rvo-exp-namespace.cpp
+++ clang/test/SemaCXX/coroutine-rvo-exp-namespace.cpp
@@ -30,6 +30,7 @@
 
 template 
 struct coroutine_traits : public traits_sfinae_base {};
+// expected-note@-1{{declared here}}
 } // namespace std::experimental
 
 struct suspend_never {
Index: clang/test/SemaCXX/coroutine-mixed2-exp-namespace.cpp
===
--- clang/test/SemaCXX/coroutine-mixed2-exp-namespace.cpp
+++ clang/test/SemaCXX/coroutine-mixed2-exp-namespace.cpp
@@ -1,10 +1,10 @@
 // This file is to test the mixed use of `std::experimental::coroutine_traits` and `std::coroutine_traits`
-// which is similar to coroutine-mixed-exp-namesapce. This file tests the relative order of
+// which is similar to coroutine-mixed-exp-namespace. This file tests the relative order of
 // included header wouldn't affect the diagnostic messages.
 // RUN: %clang_cc1 -verify -std=c++20 -fsynt

[PATCH] D116494: [clang-format] spacesRequiredBetween is not honouring clang-format off/on

[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.


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

https://reviews.llvm.org/D116494

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

[clang] d677a7c - [clang] Remove redundant member initialization (NFC)

[Kazu Hirata via cfe-commits]

Author: Kazu Hirata
Date: 2022-01-02T10:20:23-08:00
New Revision: d677a7cb056b17145a50ec8ca2ab6d5f4c494749

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

LOG: [clang] Remove redundant member initialization (NFC)

Identified with readability-redundant-member-init.

Added: 


Modified: 
clang/include/clang/APINotes/Types.h
clang/include/clang/AST/ASTConcept.h
clang/include/clang/AST/Comment.h
clang/include/clang/AST/DeclObjC.h
clang/include/clang/AST/Expr.h
clang/include/clang/ASTMatchers/Dynamic/Diagnostics.h
clang/include/clang/Analysis/Analyses/Consumed.h
clang/include/clang/Basic/Diagnostic.h
clang/include/clang/Basic/PartialDiagnostic.h
clang/include/clang/Sema/DeclSpec.h
clang/include/clang/Sema/Overload.h
clang/include/clang/Sema/Sema.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeInfo.h
clang/lib/ARCMigrate/Internals.h
clang/lib/ARCMigrate/TransAutoreleasePool.cpp
clang/lib/AST/DeclCXX.cpp
clang/lib/AST/ExprConcepts.cpp
clang/lib/AST/ExprConstant.cpp
clang/lib/Analysis/CFG.cpp
clang/lib/Basic/TargetInfo.cpp
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/CodeGen/CGCall.h
clang/lib/CodeGen/CGRecordLayout.h
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/lib/CodeGen/CodeGenPGO.cpp
clang/lib/Driver/Driver.cpp
clang/lib/Frontend/MultiplexConsumer.cpp
clang/lib/Frontend/SerializedDiagnosticPrinter.cpp
clang/lib/Sema/AnalysisBasedWarnings.cpp
clang/lib/Sema/SemaChecking.cpp
clang/lib/Sema/SemaCodeComplete.cpp
clang/lib/Sema/SemaTemplateDeduction.cpp
clang/lib/StaticAnalyzer/Core/ProgramState.cpp
clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
clang/tools/libclang/CXIndexDataConsumer.h
clang/tools/libclang/CXLoadedDiagnostic.cpp

Removed: 




diff  --git a/clang/include/clang/APINotes/Types.h 
b/clang/include/clang/APINotes/Types.h
index 0d97e9ad86231..f741d9b91d764 100644
--- a/clang/include/clang/APINotes/Types.h
+++ b/clang/include/clang/APINotes/Types.h
@@ -133,7 +133,7 @@ class CommonTypeInfo : public CommonEntityInfo {
   llvm::Optional NSErrorDomain;
 
 public:
-  CommonTypeInfo() : CommonEntityInfo() {}
+  CommonTypeInfo() {}
 
   const llvm::Optional &getSwiftBridge() const {
 return SwiftBridge;
@@ -208,10 +208,9 @@ class ObjCContextInfo : public CommonTypeInfo {
 
 public:
   ObjCContextInfo()
-  : CommonTypeInfo(), HasDefaultNullability(0), DefaultNullability(0),
-HasDesignatedInits(0), SwiftImportAsNonGenericSpecified(false),
-SwiftImportAsNonGeneric(false), SwiftObjCMembersSpecified(false),
-SwiftObjCMembers(false) {}
+  : HasDefaultNullability(0), DefaultNullability(0), HasDesignatedInits(0),
+SwiftImportAsNonGenericSpecified(false), 
SwiftImportAsNonGeneric(false),
+SwiftObjCMembersSpecified(false), SwiftObjCMembers(false) {}
 
   /// Determine the default nullability for properties and methods of this
   /// class.
@@ -309,7 +308,7 @@ class VariableInfo : public CommonEntityInfo {
   std::string Type;
 
 public:
-  VariableInfo() : CommonEntityInfo(), NullabilityAudited(false), Nullable(0) 
{}
+  VariableInfo() : NullabilityAudited(false), Nullable(0) {}
 
   llvm::Optional getNullability() const {
 return NullabilityAudited ? llvm::Optional(
@@ -358,8 +357,7 @@ class ObjCPropertyInfo : public VariableInfo {
 
 public:
   ObjCPropertyInfo()
-  : VariableInfo(), SwiftImportAsAccessorsSpecified(false),
-SwiftImportAsAccessors(false) {}
+  : SwiftImportAsAccessorsSpecified(false), SwiftImportAsAccessors(false) 
{}
 
   llvm::Optional getSwiftImportAsAccessors() const {
 return SwiftImportAsAccessorsSpecified
@@ -423,8 +421,7 @@ class ParamInfo : public VariableInfo {
 
 public:
   ParamInfo()
-  : VariableInfo(), NoEscapeSpecified(false), NoEscape(false),
-RawRetainCountConvention() {}
+  : NoEscapeSpecified(false), NoEscape(false), RawRetainCountConvention() 
{}
 
   llvm::Optional isNoEscape() const {
 if (!NoEscapeSpecified)
@@ -514,7 +511,7 @@ class FunctionInfo : public CommonEntityInfo {
   std::vector Params;
 
   FunctionInfo()
-  : CommonEntityInfo(), NullabilityAudited(false), NumAdjustedNullable(0),
+  : NullabilityAudited(false), NumAdjustedNullable(0),
 RawRetainCountConvention() {}
 
   static unsigned getMaxNullabilityIndex() {
@@ -607,8 +604,7 @@ class ObjCMethodInfo : public FunctionInfo {
   /// Whether this is a required initializer.
   unsigned RequiredInit : 1;
 
-  ObjCMethodInfo()
-  : FunctionInfo(), DesignatedInit(false), RequiredInit(false) {}
+  ObjCMethodInfo() : DesignatedInit(false), RequiredInit(false) {}
 
   friend bool

[PATCH] D114064: [clang] [MinGW] Pass --no-demangle through to the mingw linker

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114064

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

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood updated this revision to Diff 396941.
LegalizeAdulthood added a comment.

Move included headers to 
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include
Stub out included headers to isolate from system headers
Run test for C++98 or later


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

https://reviews.llvm.org/D7982

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp
  clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.h
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-duplicate-include.rst
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/iostream
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/readability-duplicate-include.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/string.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/sys/types.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/types.h
  clang-tools-extra/test/clang-tidy/checkers/readability-duplicate-include.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-duplicate-include.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-duplicate-include.cpp
@@ -0,0 +1,72 @@
+// RUN: %check_clang_tidy -std=c++98-or-later %s readability-duplicate-include %t -- -- -I%S/Inputs/readability-duplicate-include
+
+int a;
+#include 
+int b;
+#include 
+int c;
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: duplicate include [readability-duplicate-include]
+// CHECK-FIXES:  {{^int a;$}}
+// CHECK-FIXES-NEXT: {{^#include $}}
+// CHECK-FIXES-NEXT: {{^int b;$}}
+// CHECK-FIXES-NEXT: {{^int c;$}}
+
+int d;
+#include 
+int e;
+#include  // extra stuff that will also be removed
+int f;
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: {{.*}}
+// CHECK-FIXES:  {{^int d;$}}
+// CHECK-FIXES-NEXT: {{^#include $}}
+// CHECK-FIXES-NEXT: {{^int e;$}}
+// CHECK-FIXES-NEXT: {{^int f;$}}
+
+int g;
+#include "readability-duplicate-include.h"
+int h;
+#include "readability-duplicate-include.h"
+int i;
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: {{.*}}
+// CHECK-FIXES:  {{^int g;$}}
+// CHECK-FIXES-NEXT: {{^#include "readability-duplicate-include.h"$}}
+// CHECK-FIXES-NEXT: {{^int h;$}}
+// CHECK-FIXES-NEXT: {{^int i;$}}
+
+#include "types.h"
+
+int j;
+#include 
+int k;
+#include 
+int l;
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: {{.*}}
+// CHECK-FIXES:  {{^int j;$}}
+// CHECK-FIXES-NEXT: {{^#include $}}
+// CHECK-FIXES-NEXT: {{^int k;$}}
+// CHECK-FIXES-NEXT: {{^int l;$}}
+
+int m;
+#  include   // lots of space
+int n;
+// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: {{.*}}
+// CHECK-FIXES:  {{^int m;$}}
+// CHECK-FIXES-NEXT: {{^int n;$}}
+
+// defining a macro in the main file resets the included file cache
+#define ARBITRARY_MACRO
+int o;
+#include 
+int p;
+// CHECK-FIXES:  {{^int o;$}}
+// CHECK-FIXES-NEXT: {{^#include $}}
+// CHECK-FIXES-NEXT: {{^int p;$}}
+
+// undefining a macro resets the cache
+#undef ARBITRARY_MACRO
+int q;
+#include 
+int r;
+// CHECK-FIXES:  {{^int q;$}}
+// CHECK-FIXES-NEXT: {{^#include $}}
+// CHECK-FIXES-NEXT: {{^int r;$}}
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/types.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/types.h
@@ -0,0 +1 @@
+// empty file used by readability-duplicate-include.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/sys/types.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/sys/types.h
@@ -0,0 +1 @@
+// empty file used by readability-duplicate-include.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/string.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/string.h
@@ -0,0 +1 @@
+// empty file used by readability-duplicate-include.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/readability-duplicate-include.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/readability-duplicate-include.h
@@ -0,0 +1 @@
+// empty file used by readability-duplicate-in

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added a comment.

In D7982#3216332 , @Eugene.Zelenko 
wrote:

> Functionality-wise this check is superseded by Include What You Use 
> .

That's not part of clang-tidy and has it's own set of problems (false 
positives) when you run it.




Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-duplicate-include.rst:6
+
+Looks for duplicate includes and removes them.  The check maintains a list of
+included files and looks for duplicates.  If a macro is defined or undefined

Eugene.Zelenko wrote:
> Please fix double spaces. Same below.
Please point me to a LLVM coding standards section that requires this.


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

https://reviews.llvm.org/D7982

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

[clang] b50fea4 - [clang] Allow using std::coroutine_traits in std::experimental

[Nathan Sidwell via cfe-commits]

Author: Nathan Sidwell
Date: 2022-01-02T15:48:16-05:00
New Revision: b50fea47b6c454581fce89af359f3afe5154986c

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

LOG: [clang] Allow using std::coroutine_traits in  std::experimental

This is that diff I was aiming for.  When transitioning code from
coroutines-ts to c++20, it can be useful to add a using declaration to
std::experimental pointing to std::coroutine_traits.  This permits
that use by checking whether lookup in std::experimentl finds a
different decl to lookup in std.  You still get a warning about
std::experimental::coroutine_traits being a thing, just not an error.

Reviewed By: ChuanqiXu

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

Added: 
clang/test/SemaCXX/coroutine-mixed3-exp-namespace.cpp
clang/test/SemaCXX/coroutine-mixed4-exp-namespace.cpp

Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaCoroutine.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 8ef9195944d56..afc63d4806272 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11047,7 +11047,7 @@ def warn_deprecated_coroutine_namespace : Warning<
   "use std::%0 instead">,
   InGroup;
 def err_mixed_use_std_and_experimental_namespace_for_coroutine : Error<
-  "mixed use of std and std::experimental namespaces for "
+  "conflicting mixed use of std and std::experimental namespaces for "
   "coroutine components">;
 def err_implicit_coroutine_std_nothrow_type_not_found : Error<
   "std::nothrow was not found; include  before defining a coroutine which 
"

diff  --git a/clang/lib/Sema/SemaCoroutine.cpp 
b/clang/lib/Sema/SemaCoroutine.cpp
index 3a6d9f0b9f265..f5f78c02e3701 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -1761,8 +1761,9 @@ ClassTemplateDecl 
*Sema::lookupCoroutineTraits(SourceLocation KwLoc,
   auto *Found = *ResExp.begin();
   Diag(Found->getLocation(), diag::note_entity_declared_at) << Found;
 
-  if (InStd) {
-// Also found in std
+  if (InStd &&
+  StdCoroutineTraitsCache != ResExp.getAsSingle()) {
+// Also found something 
diff erent in std
 Diag(KwLoc,
  diag::err_mixed_use_std_and_experimental_namespace_for_coroutine);
 Diag(StdCoroutineTraitsCache->getLocation(),

diff  --git a/clang/test/SemaCXX/coroutine-mixed3-exp-namespace.cpp 
b/clang/test/SemaCXX/coroutine-mixed3-exp-namespace.cpp
new file mode 100644
index 0..533f9d78e2784
--- /dev/null
+++ b/clang/test/SemaCXX/coroutine-mixed3-exp-namespace.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -verify -std=c++20 -fsyntax-only %s
+
+#include "Inputs/std-coroutine.h"
+
+namespace std::experimental {
+using std::coroutine_handle;
+using std::coroutine_traits; // expected-note{{declared here}}
+} // namespace std::experimental
+
+struct my_awaitable {
+  bool await_ready() noexcept;
+  void await_suspend(std::coroutine_handle<> coro) noexcept;
+  void await_resume() noexcept;
+};
+
+struct promise_void {
+  void get_return_object();
+  my_awaitable initial_suspend();
+  my_awaitable final_suspend() noexcept;
+  void return_void();
+  void unhandled_exception();
+};
+
+template <>
+struct std::coroutine_traits { using promise_type = promise_void; };
+
+void test() {
+  co_return;
+  // expected-warning@-1{{support for std::experimental::coroutine_traits will 
be removed}}
+}

diff  --git a/clang/test/SemaCXX/coroutine-mixed4-exp-namespace.cpp 
b/clang/test/SemaCXX/coroutine-mixed4-exp-namespace.cpp
new file mode 100644
index 0..715282dd2df83
--- /dev/null
+++ b/clang/test/SemaCXX/coroutine-mixed4-exp-namespace.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -verify -std=c++20 -fsyntax-only %s
+
+#include "Inputs/std-coroutine.h"
+
+namespace std::experimental {
+// expected-note@+1{{declared here}}
+template  using coroutine_traits = std::coroutine_traits;
+using std::coroutine_handle;
+} // namespace std::experimental
+
+struct my_awaitable {
+  bool await_ready() noexcept;
+  void await_suspend(std::experimental::coroutine_handle<> coro) noexcept;
+  void await_resume() noexcept;
+};
+
+struct promise_void {
+  void get_return_object();
+  my_awaitable initial_suspend();
+  my_awaitable final_suspend() noexcept;
+  void return_void();
+  void unhandled_exception();
+};
+
+template <>
+struct std::coroutine_traits { using promise_type = promise_void; };
+
+void test() {
+  co_return; // expected-error {{mixed use of std and std::experimental 
namespaces for coroutine components}}
+  // expected-warning@-1{{support for std::experimental::coroutine_traits will 
be removed}}

[PATCH] D115943: [clang] Mixed coroutine namespaces

[Nathan Sidwell 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 rGb50fea47b6c4: [clang] Allow using std::coroutine_traits in  
std::experimental (authored by urnathan).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115943

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaCoroutine.cpp
  clang/test/SemaCXX/coroutine-mixed3-exp-namespace.cpp
  clang/test/SemaCXX/coroutine-mixed4-exp-namespace.cpp

Index: clang/test/SemaCXX/coroutine-mixed4-exp-namespace.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/coroutine-mixed4-exp-namespace.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -verify -std=c++20 -fsyntax-only %s
+
+#include "Inputs/std-coroutine.h"
+
+namespace std::experimental {
+// expected-note@+1{{declared here}}
+template  using coroutine_traits = std::coroutine_traits;
+using std::coroutine_handle;
+} // namespace std::experimental
+
+struct my_awaitable {
+  bool await_ready() noexcept;
+  void await_suspend(std::experimental::coroutine_handle<> coro) noexcept;
+  void await_resume() noexcept;
+};
+
+struct promise_void {
+  void get_return_object();
+  my_awaitable initial_suspend();
+  my_awaitable final_suspend() noexcept;
+  void return_void();
+  void unhandled_exception();
+};
+
+template <>
+struct std::coroutine_traits { using promise_type = promise_void; };
+
+void test() {
+  co_return; // expected-error {{mixed use of std and std::experimental namespaces for coroutine components}}
+  // expected-warning@-1{{support for std::experimental::coroutine_traits will be removed}}
+  // expected-note@Inputs/std-coroutine.h:8 {{'coroutine_traits' declared here}}
+}
Index: clang/test/SemaCXX/coroutine-mixed3-exp-namespace.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/coroutine-mixed3-exp-namespace.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -verify -std=c++20 -fsyntax-only %s
+
+#include "Inputs/std-coroutine.h"
+
+namespace std::experimental {
+using std::coroutine_handle;
+using std::coroutine_traits; // expected-note{{declared here}}
+} // namespace std::experimental
+
+struct my_awaitable {
+  bool await_ready() noexcept;
+  void await_suspend(std::coroutine_handle<> coro) noexcept;
+  void await_resume() noexcept;
+};
+
+struct promise_void {
+  void get_return_object();
+  my_awaitable initial_suspend();
+  my_awaitable final_suspend() noexcept;
+  void return_void();
+  void unhandled_exception();
+};
+
+template <>
+struct std::coroutine_traits { using promise_type = promise_void; };
+
+void test() {
+  co_return;
+  // expected-warning@-1{{support for std::experimental::coroutine_traits will be removed}}
+}
Index: clang/lib/Sema/SemaCoroutine.cpp
===
--- clang/lib/Sema/SemaCoroutine.cpp
+++ clang/lib/Sema/SemaCoroutine.cpp
@@ -1761,8 +1761,9 @@
   auto *Found = *ResExp.begin();
   Diag(Found->getLocation(), diag::note_entity_declared_at) << Found;
 
-  if (InStd) {
-// Also found in std
+  if (InStd &&
+  StdCoroutineTraitsCache != ResExp.getAsSingle()) {
+// Also found something different in std
 Diag(KwLoc,
  diag::err_mixed_use_std_and_experimental_namespace_for_coroutine);
 Diag(StdCoroutineTraitsCache->getLocation(),
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11047,7 +11047,7 @@
   "use std::%0 instead">,
   InGroup;
 def err_mixed_use_std_and_experimental_namespace_for_coroutine : Error<
-  "mixed use of std and std::experimental namespaces for "
+  "conflicting mixed use of std and std::experimental namespaces for "
   "coroutine components">;
 def err_implicit_coroutine_std_nothrow_type_not_found : Error<
   "std::nothrow was not found; include  before defining a coroutine which "
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116161: [Clang] Extend emitUnaryBuiltin to avoid duplicate logic.

[Florian Hahn via Phabricator via cfe-commits]

fhahn accepted this revision.
fhahn 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/D116161/new/

https://reviews.llvm.org/D116161

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

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

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:141
 ^^
+- Added a setting ``bugprone-assert-side-effect.FunctionExceptions`` for
+  a comma-separated list of the names of functions or methods to be considered

Eugene.Zelenko wrote:
> Please separate with newline and use single back-ticks for options.
I introduced those double backticks due to review comments. As it turns out, 
single backticks are only for links, not for formatted text. Should they be 
brought back?


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

https://reviews.llvm.org/D116478

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

[PATCH] D114425: [clang] Add __builtin_bswap128

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

In D114425#3216231 , @philnik wrote:

> In D114425#3209794 , @craig.topper 
> wrote:
>
>> What does the builtin due if __int128 isn't supported? Even though the type 
>> isn't legal the builtin can still be called with a narrower type that would 
>> be implicitly converted. Does that work correctly?
>
> Would the correct behavior be to throw an error in that case? Or what exactly 
> do you expect?

gcc only defines the builtin if __int128 is a supported type. It doesn't look 
like it generates an error, it just leaves it as call to an unknown function. I 
don't know how easy it is to do the same in clang.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114425

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

[PATCH] D114425: [clang] Add __builtin_bswap128

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

In D114425#3216490 , @craig.topper 
wrote:

> In D114425#3216231 , @philnik wrote:
>
>> In D114425#3209794 , @craig.topper 
>> wrote:
>>
>>> What does the builtin due if __int128 isn't supported? Even though the type 
>>> isn't legal the builtin can still be called with a narrower type that would 
>>> be implicitly converted. Does that work correctly?
>>
>> Would the correct behavior be to throw an error in that case? Or what 
>> exactly do you expect?
>
> gcc only defines the builtin if __int128 is a supported type. It doesn't look 
> like it generates an error, it just leaves it as call to an unknown function. 
> I don't know how easy it is to do the same in clang.

The existing code has some lines like `CGM.ErrorUnsupported(E, 
"__builtin_dwarf_sp_column");` — I would try doing the same kind of thing in 
the case where `__int128` isn't supported. (But I don't know how to make 
`__int128` unsupported! Every place I can see `int128` mentioned in the code, 
it's not conspicuously guarded by any condition.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114425

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

[PATCH] D116490: [clangd] Code action to declare missing move/copy constructor/assignment

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 396949.
sammccall added a comment.

Oops, forgot the implementation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116490

Files:
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/ParsedAST.h
  clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
  clang-tools-extra/clangd/refactor/tweaks/SpecialMembers.cpp

Index: clang-tools-extra/clangd/refactor/tweaks/SpecialMembers.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/refactor/tweaks/SpecialMembers.cpp
@@ -0,0 +1,196 @@
+//===--- SpecialMembers.cpp - Generate C++ special member functions ---===//
+//
+// 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 "ParsedAST.h"
+#include "refactor/Tweak.h"
+#include "support/Logger.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Sema/Sema.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+// Returns code to declare missing copy/move constructors/assignment operators.
+// They will be deleted or defaulted to match the class's current state.
+std::string buildSpecialMemberDeclarations(const CXXRecordDecl &Class) {
+  struct Members {
+const CXXMethodDecl *Copy = nullptr;
+const CXXMethodDecl *Move = nullptr;
+  } Ctor, Assign;
+
+  for (const auto &M : Class.methods()) {
+if (M->isCopyAssignmentOperator())
+  Assign.Copy = M;
+else if (M->isMoveAssignmentOperator())
+  Assign.Move = M;
+if (const auto *C = llvm::dyn_cast(M)) {
+  if (C->isCopyConstructor())
+Ctor.Copy = C;
+  else if (C->isMoveConstructor())
+Ctor.Move = C;
+}
+  }
+
+  std::string S;
+  llvm::raw_string_ostream OS(S);
+
+  auto PrintMember = [&](const CXXMethodDecl *D, const char *MemberPattern,
+ const char *ParmPattern) {
+if (D && !D->isImplicit())
+  return;
+bool Delete = !D || D->isDeleted();
+OS << llvm::formatv(
+"{0} = {1};\n",
+llvm::formatv(MemberPattern, Class.getName(),
+  llvm::formatv(ParmPattern, Class.getName())),
+Delete ? "delete" : "default");
+  };
+  auto PrintMembers = [&](const Members &M, const char *MemberPattern) {
+PrintMember(M.Copy, MemberPattern, /*ParmPattern=*/"const {0}&");
+PrintMember(M.Move, MemberPattern, /*ParmPattern=*/"{0}&&");
+  };
+  PrintMembers(Ctor, /*MemberPattern=*/"{0}({1})");
+  PrintMembers(Assign, /*MemberPattern=*/"{0} &operator=({1})");
+
+  return S;
+}
+
+// Chooses where in the class definition to insert constructors/operators.
+// If there is any public constructor, we insert after the last one.
+// Otherwise we insert at the top of the first specified public section.
+// If no public section is specified in a struct, insert at the top.
+// If no public section is specified in a class, set NeedsPublic.
+SourceLocation chooseSpecialMemberInsertionPoint(const CXXRecordDecl &Class,
+ bool &NeedsPublic) {
+  SourceLocation InFirstPublic;
+  SourceLocation AfterLastPublicCtor;
+  SourceLocation FirstDecl;
+  SourceLocation RBrace = Class.getBraceRange().getEnd();
+
+  // We want a slightly odd loop here, pairing each declaration with
+  // the location *after* it.
+  auto SawDecl = [&](const Decl *D, SourceLocation NextLoc) {
+if (FirstDecl.isInvalid())
+  FirstDecl = D->getSourceRange().getBegin();
+if (InFirstPublic.isInvalid()) {
+  if (auto *ASD = llvm::dyn_cast(D)) {
+if (ASD->getAccess() == AS_public)
+  InFirstPublic = NextLoc;
+  }
+}
+if (llvm::isa(D) && D->getAccess() == AS_public)
+  AfterLastPublicCtor = NextLoc;
+  };
+
+  const Decl *Prev = nullptr;
+  for (const auto *D : Class.decls()) {
+if (D->isImplicit())
+  continue;
+if (Prev)
+  SawDecl(Prev, D->getSourceRange().getBegin());
+Prev = D;
+  }
+  if (Prev)
+SawDecl(Prev, RBrace);
+
+  // We've gathered all the locations, pick the best insertion point.
+  if (AfterLastPublicCtor.isValid())
+return AfterLastPublicCtor;
+  if (InFirstPublic.isValid())
+return InFirstPublic;
+  if (Class.getTagKind() == TTK_Class) {
+NeedsPublic = true;
+// Insert at the end, to avoid inserting a second access specifier.
+return RBrace;
+  }
+  return FirstDecl.isValid() ? FirstDecl : RBrace;
+}
+
+// A tweak that adds missing declarations of copy & move constructors.
+//
+// 

[PATCH] D116490: [clangd] Code action to declare missing move/copy constructor/assignment

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 396950.
sammccall added a comment.

oops, and the test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116490

Files:
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/ParsedAST.h
  clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
  clang-tools-extra/clangd/refactor/tweaks/SpecialMembers.cpp
  clang-tools-extra/clangd/unittests/tweaks/SpecialMembersTests.cpp

Index: clang-tools-extra/clangd/unittests/tweaks/SpecialMembersTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/tweaks/SpecialMembersTests.cpp
@@ -0,0 +1,47 @@
+//===-- SpecialMembersTests.cpp ---===//
+//
+// 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 "TweakTesting.h"
+#include "gmock/gmock-matchers.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TWEAK_TEST(SpecialMembers);
+
+TEST_F(SpecialMembersTest, Test) {
+  EXPECT_AVAILABLE("struct ^S {};");
+  EXPECT_UNAVAILABLE("struct S { ^ };");
+  EXPECT_UNAVAILABLE("union ^U {};");
+  EXPECT_AVAILABLE("struct ^S { S(const S&); S(S&&); };");
+  EXPECT_UNAVAILABLE("struct ^S {"
+ "S(const S&); S(S&&);"
+ "S &operator=(S&&); S &operator=(const S&);"
+ "};");
+
+  const char *Output = R"cpp(struct S{S(const S &) = default;
+  S(S &&) = default;
+  S &operator=(const S &) = default;
+  S &operator=(S &&) = default;
+};)cpp";
+  EXPECT_EQ(apply("struct ^S{};"), Output);
+
+  Output = R"cpp(struct S{S(const S &) = default;
+S(S &&) = default;
+S &operator=(const S &) = delete;
+S &operator=(S &&) = delete;
+int& ref;};)cpp";
+  EXPECT_EQ(apply("struct ^S{int& ref;};"), Output);
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/refactor/tweaks/SpecialMembers.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/refactor/tweaks/SpecialMembers.cpp
@@ -0,0 +1,196 @@
+//===--- SpecialMembers.cpp - Generate C++ special member functions ---===//
+//
+// 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 "ParsedAST.h"
+#include "refactor/Tweak.h"
+#include "support/Logger.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Sema/Sema.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+// Returns code to declare missing copy/move constructors/assignment operators.
+// They will be deleted or defaulted to match the class's current state.
+std::string buildSpecialMemberDeclarations(const CXXRecordDecl &Class) {
+  struct Members {
+const CXXMethodDecl *Copy = nullptr;
+const CXXMethodDecl *Move = nullptr;
+  } Ctor, Assign;
+
+  for (const auto &M : Class.methods()) {
+if (M->isCopyAssignmentOperator())
+  Assign.Copy = M;
+else if (M->isMoveAssignmentOperator())
+  Assign.Move = M;
+if (const auto *C = llvm::dyn_cast(M)) {
+  if (C->isCopyConstructor())
+Ctor.Copy = C;
+  else if (C->isMoveConstructor())
+Ctor.Move = C;
+}
+  }
+
+  std::string S;
+  llvm::raw_string_ostream OS(S);
+
+  auto PrintMember = [&](const CXXMethodDecl *D, const char *MemberPattern,
+ const char *ParmPattern) {
+if (D && !D->isImplicit())
+  return;
+bool Delete = !D || D->isDeleted();
+OS << llvm::formatv(
+"{0} = {1};\n",
+llvm::formatv(MemberPattern, Class.getName(),
+  llvm::formatv(ParmPattern, Class.getName())),
+Delete ? "delete" : "default");
+  };
+  auto PrintMembers = [&](const Members &M, const char *MemberPattern) {
+PrintMember(M.Copy, MemberPattern, /*ParmPattern=*/"const {0}&");
+PrintMember(M.Move, MemberPattern, /*ParmPattern=*/"{0}&&");
+  };
+  PrintMembers(Ctor, /*MemberPattern=*/"{0}({1})");
+  PrintMembers(Assign, /*MemberPattern=*/"{0} &operator=({1})");
+
+  return S;
+}
+
+// Chooses where in the class definition to insert constructors/operators.
+// If there is any public constructor, we insert after the last one.
+// Otherwise we insert at the top of t

[clang] a8877c5 - [clang] [MinGW] Pass --no-demangle through to the mingw linker

[Martin Storsjö via cfe-commits]

Author: Martin Storsjö
Date: 2022-01-03T00:22:40+02:00
New Revision: a8877c5ccc0e05495d60f1669d47826e60f373b8

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

LOG: [clang] [MinGW] Pass --no-demangle through to the mingw linker

Clang has custom handling of --no-demangle, where it is removed
from the input -Wl and -Xlinker options, and readded specifically
by the drivers where it's known to be supported.

Both ld.bfd and lld support the --no-demangle option. This handles
the option in the same way as in ToolChains/Gnu.cpp.

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/MinGW.cpp
clang/test/Driver/Xlinker-args.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/MinGW.cpp 
b/clang/lib/Driver/ToolChains/MinGW.cpp
index 6d8bfc358dd31..0501f97374044 100644
--- a/clang/lib/Driver/ToolChains/MinGW.cpp
+++ b/clang/lib/Driver/ToolChains/MinGW.cpp
@@ -164,6 +164,9 @@ void tools::MinGW::Linker::ConstructJob(Compilation &C, 
const JobAction &JA,
 CmdArgs.push_back("--enable-auto-image-base");
   }
 
+  if (Args.hasArg(options::OPT_Z_Xlinker__no_demangle))
+CmdArgs.push_back("--no-demangle");
+
   CmdArgs.push_back("-o");
   const char *OutputFile = Output.getFilename();
   // GCC implicitly adds an .exe extension if it is given an output file name

diff  --git a/clang/test/Driver/Xlinker-args.c 
b/clang/test/Driver/Xlinker-args.c
index 0fba8e711bd06..ad59e0beee22b 100644
--- a/clang/test/Driver/Xlinker-args.c
+++ b/clang/test/Driver/Xlinker-args.c
@@ -12,6 +12,11 @@
 // RUN:   -Wl,two,--no-demangle,three -Xlinker four -z five -r %s 2> %t
 // RUN: FileCheck -check-prefix=LINUX < %t %s
 
+/// Check that --no-demangle gets forwarded to the mingw linker
+// RUN: %clang -target x86_64-w64-mingw32 -### \
+// RUN:   -Wl,--no-demangle %s 2> %t
+// RUN: FileCheck -check-prefix=MINGW < %t %s
+
 // RUN: %clang -target powerpc-unknown-aix -### \
 // RUN:   -b one -b two %s 2> %t
 // RUN: FileCheck -check-prefix=AIX < %t %s
@@ -23,6 +28,7 @@
 // DARWIN-NOT: --no-demangle
 // DARWIN: "one" "two" "three" "four" "-z" "five" "-r"
 // LINUX: "--no-demangle" "-e" "_start" "one" "two" "three" "four" "-z" "five" 
"-r" {{.*}} "-T" "a.lds"
+// MINGW: "--no-demangle"
 // AIX: "-b" "one" "-b" "two"
 // NOT-AIX: error: unsupported option '-b' for target 'powerpc-unknown-linux'
 



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

[PATCH] D114064: [clang] [MinGW] Pass --no-demangle through to the mingw linker

[Martin Storsjö 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 rGa8877c5ccc0e: [clang] [MinGW] Pass --no-demangle through to 
the mingw linker (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114064

Files:
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/test/Driver/Xlinker-args.c


Index: clang/test/Driver/Xlinker-args.c
===
--- clang/test/Driver/Xlinker-args.c
+++ clang/test/Driver/Xlinker-args.c
@@ -12,6 +12,11 @@
 // RUN:   -Wl,two,--no-demangle,three -Xlinker four -z five -r %s 2> %t
 // RUN: FileCheck -check-prefix=LINUX < %t %s
 
+/// Check that --no-demangle gets forwarded to the mingw linker
+// RUN: %clang -target x86_64-w64-mingw32 -### \
+// RUN:   -Wl,--no-demangle %s 2> %t
+// RUN: FileCheck -check-prefix=MINGW < %t %s
+
 // RUN: %clang -target powerpc-unknown-aix -### \
 // RUN:   -b one -b two %s 2> %t
 // RUN: FileCheck -check-prefix=AIX < %t %s
@@ -23,6 +28,7 @@
 // DARWIN-NOT: --no-demangle
 // DARWIN: "one" "two" "three" "four" "-z" "five" "-r"
 // LINUX: "--no-demangle" "-e" "_start" "one" "two" "three" "four" "-z" "five" 
"-r" {{.*}} "-T" "a.lds"
+// MINGW: "--no-demangle"
 // AIX: "-b" "one" "-b" "two"
 // NOT-AIX: error: unsupported option '-b' for target 'powerpc-unknown-linux'
 
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -164,6 +164,9 @@
 CmdArgs.push_back("--enable-auto-image-base");
   }
 
+  if (Args.hasArg(options::OPT_Z_Xlinker__no_demangle))
+CmdArgs.push_back("--no-demangle");
+
   CmdArgs.push_back("-o");
   const char *OutputFile = Output.getFilename();
   // GCC implicitly adds an .exe extension if it is given an output file name


Index: clang/test/Driver/Xlinker-args.c
===
--- clang/test/Driver/Xlinker-args.c
+++ clang/test/Driver/Xlinker-args.c
@@ -12,6 +12,11 @@
 // RUN:   -Wl,two,--no-demangle,three -Xlinker four -z five -r %s 2> %t
 // RUN: FileCheck -check-prefix=LINUX < %t %s
 
+/// Check that --no-demangle gets forwarded to the mingw linker
+// RUN: %clang -target x86_64-w64-mingw32 -### \
+// RUN:   -Wl,--no-demangle %s 2> %t
+// RUN: FileCheck -check-prefix=MINGW < %t %s
+
 // RUN: %clang -target powerpc-unknown-aix -### \
 // RUN:   -b one -b two %s 2> %t
 // RUN: FileCheck -check-prefix=AIX < %t %s
@@ -23,6 +28,7 @@
 // DARWIN-NOT: --no-demangle
 // DARWIN: "one" "two" "three" "four" "-z" "five" "-r"
 // LINUX: "--no-demangle" "-e" "_start" "one" "two" "three" "four" "-z" "five" "-r" {{.*}} "-T" "a.lds"
+// MINGW: "--no-demangle"
 // AIX: "-b" "one" "-b" "two"
 // NOT-AIX: error: unsupported option '-b' for target 'powerpc-unknown-linux'
 
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -164,6 +164,9 @@
 CmdArgs.push_back("--enable-auto-image-base");
   }
 
+  if (Args.hasArg(options::OPT_Z_Xlinker__no_demangle))
+CmdArgs.push_back("--no-demangle");
+
   CmdArgs.push_back("-o");
   const char *OutputFile = Output.getFilename();
   // GCC implicitly adds an .exe extension if it is given an output file name
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114425: [clang] Add __builtin_bswap128

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

In D114425#3216531 , @Quuxplusone 
wrote:

> In D114425#3216490 , @craig.topper 
> wrote:
>
>> In D114425#3216231 , @philnik 
>> wrote:
>>
>>> In D114425#3209794 , 
>>> @craig.topper wrote:
>>>
 What does the builtin due if __int128 isn't supported? Even though the 
 type isn't legal the builtin can still be called with a narrower type that 
 would be implicitly converted. Does that work correctly?
>>>
>>> Would the correct behavior be to throw an error in that case? Or what 
>>> exactly do you expect?
>>
>> gcc only defines the builtin if __int128 is a supported type. It doesn't 
>> look like it generates an error, it just leaves it as call to an unknown 
>> function. I don't know how easy it is to do the same in clang.
>
> The existing code has some lines like `CGM.ErrorUnsupported(E, 
> "__builtin_dwarf_sp_column");` — I would try doing the same kind of thing in 
> the case where `__int128` isn't supported. (But I don't know how to make 
> `__int128` unsupported! Every place I can see `int128` mentioned in the code, 
> it's not conspicuously guarded by any condition.)

I believe it controlled by hasInt128Type() in include/clang/Basic/TargetInfo.h. 
It should return false for riscv32 or i686 and probably other 32-bit targets.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114425

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

[PATCH] D116502: [clangd] Helper for determining member insertion point.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: dgoldman.
Herald added subscribers: usaxena95, kadircet, arphaman, mgorny.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

To be used in D116490  and D116385 
, and an upcoming patch to generate C++
constructors.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116502

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/refactor/InsertionPoint.cpp
  clang-tools-extra/clangd/refactor/InsertionPoint.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/InsertionPointTests.cpp

Index: clang-tools-extra/clangd/unittests/InsertionPointTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/InsertionPointTests.cpp
@@ -0,0 +1,175 @@
+//===-- InsertionPointTess.cpp  ---===//
+//
+// 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 "Annotations.h"
+#include "Protocol.h"
+#include "SourceCode.h"
+#include "TestTU.h"
+#include "TestWorkspace.h"
+#include "XRefs.h"
+#include "refactor/InsertionPoint.h"
+#include "clang/AST/DeclBase.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(InsertionPointTests, Generic) {
+  Annotations Code(R"cpp(
+  namespace ns {
+$a^int a1;
+$b^// leading comment
+int b;
+$c^int c1; // trailing comment
+int c2;
+$a2^int a2;
+  $end^};
+  )cpp");
+
+  auto StartsWith =
+  [&](llvm::StringLiteral S) -> std::function {
+return [S](const Decl *D) {
+  if (const auto *ND = llvm::dyn_cast(D))
+return llvm::StringRef(ND->getNameAsString()).startswith(S);
+  return false;
+};
+  };
+
+  auto AST = TestTU::withCode(Code.code()).build();
+  auto &NS = cast(findDecl(AST, "ns"));
+
+  // Test single anchors.
+  auto Point = [&](llvm::StringLiteral Prefix, Anchor::Dir Direction) {
+auto Loc = insertionPoint(NS, {Anchor{StartsWith(Prefix), Direction}});
+return sourceLocToPosition(AST.getSourceManager(), Loc);
+  };
+  EXPECT_EQ(Point("a", Anchor::Above), Code.point("a"));
+  EXPECT_EQ(Point("a", Anchor::Below), Code.point("b"));
+  EXPECT_EQ(Point("b", Anchor::Above), Code.point("b"));
+  EXPECT_EQ(Point("b", Anchor::Below), Code.point("c"));
+  EXPECT_EQ(Point("c", Anchor::Above), Code.point("c"));
+  EXPECT_EQ(Point("c", Anchor::Below), Code.point("a2"));
+  EXPECT_EQ(Point("", Anchor::Above), Code.point("a"));
+  EXPECT_EQ(Point("", Anchor::Below), Code.point("end"));
+  EXPECT_EQ(Point("no_match", Anchor::Below), Position{});
+
+  // Test anchor chaining.
+  auto Chain = [&](llvm::StringLiteral P1, llvm::StringLiteral P2) {
+auto Loc = insertionPoint(NS, {Anchor{StartsWith(P1), Anchor::Above},
+   Anchor{StartsWith(P2), Anchor::Above}});
+return sourceLocToPosition(AST.getSourceManager(), Loc);
+  };
+  EXPECT_EQ(Chain("a", "b"), Code.point("a"));
+  EXPECT_EQ(Chain("b", "a"), Code.point("b"));
+  EXPECT_EQ(Chain("no_match", "a"), Code.point("a"));
+
+  // Test edit generation.
+  auto Edit = insertDecl("foo;", NS, {Anchor{StartsWith("a"), Anchor::Below}});
+  ASSERT_THAT_EXPECTED(Edit, llvm::Succeeded());
+  EXPECT_EQ(offsetToPosition(Code.code(), Edit->getOffset()), Code.point("b"));
+  EXPECT_EQ(Edit->getReplacementText(), "foo;");
+  // If no match, the edit is inserted at the end.
+  Edit = insertDecl("x;", NS, {Anchor{StartsWith("no_match"), Anchor::Below}});
+  ASSERT_THAT_EXPECTED(Edit, llvm::Succeeded());
+  EXPECT_EQ(offsetToPosition(Code.code(), Edit->getOffset()),
+Code.point("end"));
+}
+
+// For CXX, we should check:
+// - special handling for access specifiers
+// - unwrapping of template decls
+TEST(InsertionPointTests, CXX) {
+  Annotations Code(R"cpp(
+class C {
+public:
+  $Method^void pubMethod();
+  $Field^int PubField;
+
+$private^private:
+  $field^int PrivField;
+  $method^void privMethod();
+  template  void privTemplateMethod();
+$end^};
+  )cpp");
+
+  auto AST = TestTU::withCode(Code.code()).build();
+  const CXXRecordDecl &C = cast(findDecl(AST, "C"));
+
+  auto IsMethod = [](const Decl *D) { return llvm::isa(D); };
+  auto Any = [](const Decl *D) { return true; };
+
+  // Test single anchors.
+  auto Point = [&](Anchor A, AccessSpecifier Protection) {
+auto Loc = insertionPoint(C, {A}, Protection);
+return sourceLocToPosition(AST.getSourceManager()

[PATCH] D116502: [clangd] Helper for determining member insertion point.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 396953.
sammccall added a comment.

Fix stale comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116502

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/refactor/InsertionPoint.cpp
  clang-tools-extra/clangd/refactor/InsertionPoint.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/InsertionPointTests.cpp

Index: clang-tools-extra/clangd/unittests/InsertionPointTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/InsertionPointTests.cpp
@@ -0,0 +1,175 @@
+//===-- InsertionPointTess.cpp  ---===//
+//
+// 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 "Annotations.h"
+#include "Protocol.h"
+#include "SourceCode.h"
+#include "TestTU.h"
+#include "TestWorkspace.h"
+#include "XRefs.h"
+#include "refactor/InsertionPoint.h"
+#include "clang/AST/DeclBase.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(InsertionPointTests, Generic) {
+  Annotations Code(R"cpp(
+  namespace ns {
+$a^int a1;
+$b^// leading comment
+int b;
+$c^int c1; // trailing comment
+int c2;
+$a2^int a2;
+  $end^};
+  )cpp");
+
+  auto StartsWith =
+  [&](llvm::StringLiteral S) -> std::function {
+return [S](const Decl *D) {
+  if (const auto *ND = llvm::dyn_cast(D))
+return llvm::StringRef(ND->getNameAsString()).startswith(S);
+  return false;
+};
+  };
+
+  auto AST = TestTU::withCode(Code.code()).build();
+  auto &NS = cast(findDecl(AST, "ns"));
+
+  // Test single anchors.
+  auto Point = [&](llvm::StringLiteral Prefix, Anchor::Dir Direction) {
+auto Loc = insertionPoint(NS, {Anchor{StartsWith(Prefix), Direction}});
+return sourceLocToPosition(AST.getSourceManager(), Loc);
+  };
+  EXPECT_EQ(Point("a", Anchor::Above), Code.point("a"));
+  EXPECT_EQ(Point("a", Anchor::Below), Code.point("b"));
+  EXPECT_EQ(Point("b", Anchor::Above), Code.point("b"));
+  EXPECT_EQ(Point("b", Anchor::Below), Code.point("c"));
+  EXPECT_EQ(Point("c", Anchor::Above), Code.point("c"));
+  EXPECT_EQ(Point("c", Anchor::Below), Code.point("a2"));
+  EXPECT_EQ(Point("", Anchor::Above), Code.point("a"));
+  EXPECT_EQ(Point("", Anchor::Below), Code.point("end"));
+  EXPECT_EQ(Point("no_match", Anchor::Below), Position{});
+
+  // Test anchor chaining.
+  auto Chain = [&](llvm::StringLiteral P1, llvm::StringLiteral P2) {
+auto Loc = insertionPoint(NS, {Anchor{StartsWith(P1), Anchor::Above},
+   Anchor{StartsWith(P2), Anchor::Above}});
+return sourceLocToPosition(AST.getSourceManager(), Loc);
+  };
+  EXPECT_EQ(Chain("a", "b"), Code.point("a"));
+  EXPECT_EQ(Chain("b", "a"), Code.point("b"));
+  EXPECT_EQ(Chain("no_match", "a"), Code.point("a"));
+
+  // Test edit generation.
+  auto Edit = insertDecl("foo;", NS, {Anchor{StartsWith("a"), Anchor::Below}});
+  ASSERT_THAT_EXPECTED(Edit, llvm::Succeeded());
+  EXPECT_EQ(offsetToPosition(Code.code(), Edit->getOffset()), Code.point("b"));
+  EXPECT_EQ(Edit->getReplacementText(), "foo;");
+  // If no match, the edit is inserted at the end.
+  Edit = insertDecl("x;", NS, {Anchor{StartsWith("no_match"), Anchor::Below}});
+  ASSERT_THAT_EXPECTED(Edit, llvm::Succeeded());
+  EXPECT_EQ(offsetToPosition(Code.code(), Edit->getOffset()),
+Code.point("end"));
+}
+
+// For CXX, we should check:
+// - special handling for access specifiers
+// - unwrapping of template decls
+TEST(InsertionPointTests, CXX) {
+  Annotations Code(R"cpp(
+class C {
+public:
+  $Method^void pubMethod();
+  $Field^int PubField;
+
+$private^private:
+  $field^int PrivField;
+  $method^void privMethod();
+  template  void privTemplateMethod();
+$end^};
+  )cpp");
+
+  auto AST = TestTU::withCode(Code.code()).build();
+  const CXXRecordDecl &C = cast(findDecl(AST, "C"));
+
+  auto IsMethod = [](const Decl *D) { return llvm::isa(D); };
+  auto Any = [](const Decl *D) { return true; };
+
+  // Test single anchors.
+  auto Point = [&](Anchor A, AccessSpecifier Protection) {
+auto Loc = insertionPoint(C, {A}, Protection);
+return sourceLocToPosition(AST.getSourceManager(), Loc);
+  };
+  EXPECT_EQ(Point({IsMethod, Anchor::Above}, AS_public), Code.point("Method"));
+  EXPECT_EQ(Point({IsMethod, Anchor::Below}, AS_public), Code.point("Field"));
+  EXPECT_EQ(Point({Any, Anchor::Above}, AS_public), Code.point("Method"));
+  EXPECT_EQ(Point({Any, Anchor::B

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

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:141
 ^^
+- Added a setting ``bugprone-assert-side-effect.FunctionExceptions`` for
+  a comma-separated list of the names of functions or methods to be considered

carlosgalvezp wrote:
> Eugene.Zelenko wrote:
> > Please separate with newline and use single back-ticks for options.
> I introduced those double backticks due to review comments. As it turns out, 
> single backticks are only for links, not for formatted text. Should they be 
> brought back?
Double back-ticks are for language constructs, single back-ticks for options, 
tool names, etc.


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

https://reviews.llvm.org/D116478

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

[PATCH] D116490: [clangd] Code action to declare missing move/copy constructor/assignment

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 396954.
sammccall added a comment.

Rebase on D116502 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116490

Files:
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/ParsedAST.h
  clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
  clang-tools-extra/clangd/refactor/tweaks/SpecialMembers.cpp
  clang-tools-extra/clangd/unittests/tweaks/SpecialMembersTests.cpp

Index: clang-tools-extra/clangd/unittests/tweaks/SpecialMembersTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/tweaks/SpecialMembersTests.cpp
@@ -0,0 +1,47 @@
+//===-- SpecialMembersTests.cpp ---===//
+//
+// 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 "TweakTesting.h"
+#include "gmock/gmock-matchers.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TWEAK_TEST(SpecialMembers);
+
+TEST_F(SpecialMembersTest, Test) {
+  EXPECT_AVAILABLE("struct ^S {};");
+  EXPECT_UNAVAILABLE("struct S { ^ };");
+  EXPECT_UNAVAILABLE("union ^U {};");
+  EXPECT_AVAILABLE("struct ^S { S(const S&); S(S&&); };");
+  EXPECT_UNAVAILABLE("struct ^S {"
+ "S(const S&); S(S&&);"
+ "S &operator=(S&&); S &operator=(const S&);"
+ "};");
+
+  const char *Output = R"cpp(struct S{S(const S &) = default;
+  S(S &&) = default;
+  S &operator=(const S &) = default;
+  S &operator=(S &&) = default;
+};)cpp";
+  EXPECT_EQ(apply("struct ^S{};"), Output);
+
+  Output = R"cpp(struct S{S(const S &) = default;
+S(S &&) = default;
+S &operator=(const S &) = delete;
+S &operator=(S &&) = delete;
+int& ref;};)cpp";
+  EXPECT_EQ(apply("struct ^S{int& ref;};"), Output);
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/refactor/tweaks/SpecialMembers.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/refactor/tweaks/SpecialMembers.cpp
@@ -0,0 +1,153 @@
+//===--- SpecialMembers.cpp - Generate C++ special member functions ---===//
+//
+// 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 "ParsedAST.h"
+#include "refactor/InsertionPoint.h"
+#include "refactor/Tweak.h"
+#include "support/Logger.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Sema/Sema.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+// Returns code to declare missing copy/move constructors/assignment operators.
+// They will be deleted or defaulted to match the class's current state.
+std::string buildSpecialMemberDeclarations(const CXXRecordDecl &Class) {
+  struct Members {
+const CXXMethodDecl *Copy = nullptr;
+const CXXMethodDecl *Move = nullptr;
+  } Ctor, Assign;
+
+  for (const auto &M : Class.methods()) {
+if (M->isCopyAssignmentOperator())
+  Assign.Copy = M;
+else if (M->isMoveAssignmentOperator())
+  Assign.Move = M;
+if (const auto *C = llvm::dyn_cast(M)) {
+  if (C->isCopyConstructor())
+Ctor.Copy = C;
+  else if (C->isMoveConstructor())
+Ctor.Move = C;
+}
+  }
+
+  std::string S;
+  llvm::raw_string_ostream OS(S);
+
+  auto PrintMember = [&](const CXXMethodDecl *D, const char *MemberPattern,
+ const char *ParmPattern) {
+if (D && !D->isImplicit())
+  return;
+bool Delete = !D || D->isDeleted();
+OS << llvm::formatv(
+"{0} = {1};\n",
+llvm::formatv(MemberPattern, Class.getName(),
+  llvm::formatv(ParmPattern, Class.getName())),
+Delete ? "delete" : "default");
+  };
+  auto PrintMembers = [&](const Members &M, const char *MemberPattern) {
+PrintMember(M.Copy, MemberPattern, /*ParmPattern=*/"const {0}&");
+PrintMember(M.Move, MemberPattern, /*ParmPattern=*/"{0}&&");
+  };
+  PrintMembers(Ctor, /*MemberPattern=*/"{0}({1})");
+  PrintMembers(Assign, /*MemberPattern=*/"{0} &operator=({1})");
+
+  return S;
+}
+
+// A tweak that adds missing declarations of copy & move constructors.
+//
+// e.g. given `struct S{};`, produce

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

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:141
 ^^
+- Added a setting ``bugprone-assert-side-effect.FunctionExceptions`` for
+  a comma-separated list of the names of functions or methods to be considered

Eugene.Zelenko wrote:
> carlosgalvezp wrote:
> > Eugene.Zelenko wrote:
> > > Please separate with newline and use single back-ticks for options.
> > I introduced those double backticks due to review comments. As it turns 
> > out, single backticks are only for links, not for formatted text. Should 
> > they be brought back?
> Double back-ticks are for language constructs, single back-ticks for options, 
> tool names, etc.
Hm, I see. I think visually it's much more helpful to have options rendered as 
formatted code (just like you'd see them in the .clang-tidy file in a code 
editor) instead of in italic, which is what is rendered with single backticks.


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

https://reviews.llvm.org/D116478

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

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

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: aaron.ballman, sepavloff, phosek, MaskRay.
Herald added a subscriber: dang.
mstorsjo requested review of this revision.
Herald added a project: clang.

When passing a set of flags to configure defaults for a specific
target (similar to the cmake settings `CLANG_DEFAULT_RTLIB`,
`CLANG_DEFAULT_UNWINDLIB`, `CLANG_DEFAULT_CXX_STDLIB` and
`CLANG_DEFAULT_LINKER`, but without hardcoding it in the binary),
some of the flags may cause warnings (e.g. `-stdlib=` when compiling C
code). Allow requesting selectively ignoring unused arguments among
some of the arguments on the command line, without needing to resort
to `-Qunused-arguments` or `-Wno-unused-command-line-argument`.

Fix up the existing diagnostics.c testcase. It was added in
response to PR12181 to fix handling of
`-Werror=unused-command-line-argument`, but the command line option
in the test (`-fzyzzybalubah`) now triggers "error: unknown argument"
instead of the intended warning. Change it into a linker input
(`-lfoo`) which triggers the intended diagnostic. Extend the
existing test case to check more cases and make sure that it keeps
testing the intended case.

Add testing of the new option to this existing test.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116503

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

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

[PATCH] D114425: [clang] Add __builtin_bswap128

[Nikolas Klauser via Phabricator via cfe-commits]

philnik updated this revision to Diff 396960.
philnik added a comment.

- Rebased
- Error if `__int128` is not supported

Should it be tested that the compiler errors when calling `__builtin_bswap128` 
with __int128 not available?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114425

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/Builtins.def
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins.cpp
  clang/test/Sema/constant-builtins-2.c


Index: clang/test/Sema/constant-builtins-2.c
===
--- clang/test/Sema/constant-builtins-2.c
+++ clang/test/Sema/constant-builtins-2.c
@@ -216,6 +216,9 @@
 int h3 = __builtin_bswap16(0x1234) == 0x3412 ? 1 : f();
 int h4 = __builtin_bswap32(0x1234) == 0x3412 ? 1 : f();
 int h5 = __builtin_bswap64(0x1234) == 0x3412 ? 1 : f();
+#if defined(__SIZEOF_INT128__)
+int h6 = __builtin_bswap128(0x1234) == (((__int128)0x3412) << 112) ? 1 : f();
+#endif
 extern long int bi0;
 extern __typeof__(__builtin_expect(0, 0)) bi0;
 
Index: clang/test/CodeGen/builtins.cpp
===
--- clang/test/CodeGen/builtins.cpp
+++ clang/test/CodeGen/builtins.cpp
@@ -20,6 +20,10 @@
 decltype(__builtin_bswap32(0)) bswap32 = 42;
 extern uint64_t bswap64;
 decltype(__builtin_bswap64(0)) bswap64 = 42;
+#ifdef __SIZEOF_INT128__
+extern __uint128_t bswap128;
+decltype(__builtin_bswap128(0)) bswap128 = 42;
+#endif
 
 #ifdef __clang__
 extern uint8_t bitrev8;
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -2923,6 +2923,11 @@
   Builder.CreateArithmeticFence(ArgValue, ConvertType(ArgType)));
 return RValue::get(ArgValue);
   }
+  case Builtin::BI__builtin_bswap128: {
+if (!Target.hasInt128Type())
+  CGM.ErrorUnsupported(E, "__builtin_bswap128");
+  }
+  [[fallthrough]];
   case Builtin::BI__builtin_bswap16:
   case Builtin::BI__builtin_bswap32:
   case Builtin::BI__builtin_bswap64: {
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -11751,7 +11751,8 @@
 
   case Builtin::BI__builtin_bswap16:
   case Builtin::BI__builtin_bswap32:
-  case Builtin::BI__builtin_bswap64: {
+  case Builtin::BI__builtin_bswap64:
+  case Builtin::BI__builtin_bswap128: {
 APSInt Val;
 if (!EvaluateInteger(E->getArg(0), Val, Info))
   return false;
Index: clang/include/clang/Basic/Builtins.def
===
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -512,6 +512,7 @@
 BUILTIN(__builtin_bswap16, "UsUs", "nc")
 BUILTIN(__builtin_bswap32, "UZiUZi", "nc")
 BUILTIN(__builtin_bswap64, "UWiUWi", "nc")
+BUILTIN(__builtin_bswap128, "ULLLiULLLi", "nc")
 
 BUILTIN(__builtin_bitreverse8, "UcUc", "nc")
 BUILTIN(__builtin_bitreverse16, "UsUs", "nc")
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -67,6 +67,8 @@
   the base path of the current config file. See :ref:`configuration-files` for
   details.
 
+- The builtin ``__builtin_bswap128`` was added.
+
 New Compiler Flags
 --
 


Index: clang/test/Sema/constant-builtins-2.c
===
--- clang/test/Sema/constant-builtins-2.c
+++ clang/test/Sema/constant-builtins-2.c
@@ -216,6 +216,9 @@
 int h3 = __builtin_bswap16(0x1234) == 0x3412 ? 1 : f();
 int h4 = __builtin_bswap32(0x1234) == 0x3412 ? 1 : f();
 int h5 = __builtin_bswap64(0x1234) == 0x3412 ? 1 : f();
+#if defined(__SIZEOF_INT128__)
+int h6 = __builtin_bswap128(0x1234) == (((__int128)0x3412) << 112) ? 1 : f();
+#endif
 extern long int bi0;
 extern __typeof__(__builtin_expect(0, 0)) bi0;
 
Index: clang/test/CodeGen/builtins.cpp
===
--- clang/test/CodeGen/builtins.cpp
+++ clang/test/CodeGen/builtins.cpp
@@ -20,6 +20,10 @@
 decltype(__builtin_bswap32(0)) bswap32 = 42;
 extern uint64_t bswap64;
 decltype(__builtin_bswap64(0)) bswap64 = 42;
+#ifdef __SIZEOF_INT128__
+extern __uint128_t bswap128;
+decltype(__builtin_bswap128(0)) bswap128 = 42;
+#endif
 
 #ifdef __clang__
 extern uint8_t bitrev8;
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -2923,6 +2923,11 @@
   Builder.CreateArithmeticFence(ArgValue, ConvertType(ArgType)));
 return RValue::get(ArgValue);
   }
+  case Bu

[PATCH] D116488: Add a misc-unused-parameters.CommentOutUnusedParameters to clang-tidy

[Jacques Pienaar via Phabricator via cfe-commits]

jpienaar added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/misc-unused-parameters.rst:25
+
+  void a(int ) { /*some code that doesn't use `i`*/ }
+

OOC why the extra space after the type?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116488

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

[PATCH] D116488: Add a misc-unused-parameters.CommentOutUnusedParameters to clang-tidy

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/misc-unused-parameters.rst:25
+
+  void a(int ) { /*some code that doesn't use `i`*/ }
+

jpienaar wrote:
> OOC why the extra space after the type?
The space is in the original code, the fix only touches the name (clang-format 
is expected to fix it afterward)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116488

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

[PATCH] D116509: [Builtins] Add missing the macro 'y' description in comments

[Jim Lin via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116509

Files:
  clang/include/clang/Basic/Builtins.def


Index: clang/include/clang/Basic/Builtins.def
===
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -26,6 +26,7 @@
 //  i -> int
 //  h -> half (__fp16, OpenCL)
 //  x -> half (_Float16)
+//  y -> half (__bf16)
 //  f -> float
 //  d -> double
 //  z -> size_t


Index: clang/include/clang/Basic/Builtins.def
===
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -26,6 +26,7 @@
 //  i -> int
 //  h -> half (__fp16, OpenCL)
 //  x -> half (_Float16)
+//  y -> half (__bf16)
 //  f -> float
 //  d -> double
 //  z -> size_t
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116511: [clang-cl] Support the /HOTPATCH flag

[Alexandre Ganea via Phabricator via cfe-commits]

aganea created this revision.
aganea added reviewers: amccarth, craig.topper, hans, rnk, stefan_reinalter.
Herald added subscribers: ormris, dexonsmith, dang, pengfei, hiraditya.
aganea requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This patch adds support for the /HOTPATCH flag: 
https://docs.microsoft.com/sv-se/cpp/build/reference/hotpatch-create-hotpatchable-image?view=msvc-170&viewFallbackFrom=vs-2019

The flag is translated to a new `-fhotpatch` cc1 flag, which in turn adds a 
`patchable-function` attribute for each function in the TU. This is then picked 
up by the `PatchableFunction` pass which would generate a 
`TargetOpcode::PATCHABLE_OP` of minsize = 2 (which means the target instruction 
must resolve to at least two bytes). Currently `TargetOpcode::PATCHABLE_OP` is 
only implemented for x86/x64.

Additionally we generate a 'hot patchable' flag in the CodeView debug stream, 
in the `S_COMPILE3` record. This flag is then picked up by LLD (or link.exe) 
and is used in conjunction with the linker `/FUNCTIONPADMIN` flag to generate 
extraneous space before each function, to accommodate for live patching. Please 
see: 
https://github.com/llvm/llvm-project/blob/d703b922961e0d02a5effdd4bfbb23ad50a3cc9f/lld/COFF/Writer.cpp#L1298

The end result is that one can finally use Live++ 
 or Recode  along 
with clang-cl.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116511

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/patchable-function-entry.c
  clang/test/CodeGenCXX/debug-info-hotpatch.cpp
  clang/test/Driver/cl-options.c
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

Index: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -843,6 +843,8 @@
   if (MMI->getModule()->getProfileSummary(/*IsCS*/ false) != nullptr) {
 Flags |= static_cast(CompileSym3Flags::PGO);
   }
+  if (Asm->TM.Options.Hotpatch)
+Flags |= static_cast(CompileSym3Flags::HotPatch);
 
   OS.AddComment("Flags and language");
   OS.emitInt32(Flags);
Index: llvm/include/llvm/Target/TargetOptions.h
===
--- llvm/include/llvm/Target/TargetOptions.h
+++ llvm/include/llvm/Target/TargetOptions.h
@@ -140,9 +140,9 @@
   EnableMachineFunctionSplitter(false), SupportsDefaultOutlining(false),
   EmitAddrsig(false), EmitCallSiteInfo(false),
   SupportsDebugEntryValues(false), EnableDebugEntryValues(false),
-  ValueTrackingVariableLocations(false),
-  ForceDwarfFrameSection(false), XRayOmitFunctionIndex(false),
-  DebugStrictDwarf(false),
+  ValueTrackingVariableLocations(false), ForceDwarfFrameSection(false),
+  XRayOmitFunctionIndex(false), DebugStrictDwarf(false),
+  Hotpatch(false),
   FPDenormalMode(DenormalMode::IEEE, DenormalMode::IEEE) {}
 
 /// DisableFramePointerElim - This returns true if frame pointer elimination
@@ -342,6 +342,9 @@
 /// By default, it is set to false.
 unsigned DebugStrictDwarf : 1;
 
+/// Emit the hotpatch flag in CodeView debug.
+unsigned Hotpatch : 1;
+
 /// Name of the stack usage file (i.e., .su file) if user passes
 /// -fstack-usage. If empty, it can be implied that -fstack-usage is not
 /// passed on the command line.
Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -118,6 +118,9 @@
 // RUN: %clang_cl /Gw /Gw- -### -- %s 2>&1 | FileCheck -check-prefix=Gw_ %s
 // Gw_-NOT: -fdata-sections
 
+// RUN: %clang_cl /hotpatch -### -- %s 2>&1 | FileCheck -check-prefix=hotpatch %s
+// hotpatch: -fhotpatchable
+
 // RUN: %clang_cl /Imyincludedir -### -- %s 2>&1 | FileCheck -check-prefix=SLASH_I %s
 // RUN: %clang_cl /I myincludedir -### -- %s 2>&1 | FileCheck -check-prefix=SLASH_I %s
 // SLASH_I: "-I" "myincludedir"
Index: clang/test/CodeGenCXX/debug-info-hotpatch.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-hotpatch.cpp
@@ -0,0 +1,13 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang_cl --target=x86_64-windows-msvc /c /hotpatch /Z7 %s -o %t.obj
+// RUN: llvm-pdbutil dump -symbols %t.obj | FileCheck %s
+// RUN: %clang_cl --target=x86_64-windows-msvc /c /Z7 %s -o %t.obj
+// RUN: llvm-pdbutil dump -symbols %t.obj | FileCheck %s --check-prefix=NOHOTPATCH
+
+int main() {
+  return 0;
+}
+
+// CHECK: S_COMPILE

[clang] e27b5f9 - [clang][AST] Fix crash when printing error

[Ellis Hoag via cfe-commits]

Author: Ellis Hoag
Date: 2022-01-02T18:03:42-08:00
New Revision: e27b5f9371382952eb5482ad151bb6fcb4cd0d7c

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

LOG: [clang][AST] Fix crash when printing error

Clang will crash if it tries to compile the following code. This commit
fixes it.
```
$ cat foo.c
void foo(_Nullable int *ptr) {
__auto_type _Nonnull a = ptr;
};
$ clang foo.c -c -Wnullable-to-nonnull-conversion
```

Reviewed By: sammccall

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

Added: 


Modified: 
clang/lib/AST/TypePrinter.cpp
clang/test/Sema/nullability.c

Removed: 




diff  --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp
index 2a33a69f288d4..cf520fcb037ed 100644
--- a/clang/lib/AST/TypePrinter.cpp
+++ b/clang/lib/AST/TypePrinter.cpp
@@ -280,7 +280,7 @@ bool TypePrinter::canPrefixQualifiers(const Type *T,
 case Type::Attributed: {
   // We still want to print the address_space before the type if it is an
   // address_space attribute.
-  const auto *AttrTy = cast(T);
+  const auto *AttrTy = cast(UnderlyingType);
   CanPrefixQualifiers = AttrTy->getAttrKind() == attr::AddressSpace;
 }
   }

diff  --git a/clang/test/Sema/nullability.c b/clang/test/Sema/nullability.c
index d462886de0436..977b29e9bf9dd 100644
--- a/clang/test/Sema/nullability.c
+++ b/clang/test/Sema/nullability.c
@@ -125,6 +125,7 @@ void nullable_to_nonnull(_Nullable int *ptr) {
   int *a = ptr; // okay
   _Nonnull int *b = ptr; // expected-warning{{implicit conversion from 
nullable pointer 'int * _Nullable' to non-nullable pointer type 'int * 
_Nonnull'}}
   b = ptr; // expected-warning{{implicit conversion from nullable pointer 'int 
* _Nullable' to non-nullable pointer type 'int * _Nonnull'}}
+  __auto_type _Nonnull c = ptr; // expected-warning{{implicit conversion from 
nullable pointer 'int * _Nullable' to non-nullable pointer type 'int * 
_Nullable _Nonnull'}}
 
   accepts_nonnull_1(ptr); // expected-warning{{implicit conversion from 
nullable pointer 'int * _Nullable' to non-nullable pointer type 'int * 
_Nonnull'}}
 }



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

[PATCH] D116342: [clang][AST] Fix crash when printing error

[Ellis Hoag 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 rGe27b5f937138: [clang][AST] Fix crash when printing error 
(authored by ellis).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116342

Files:
  clang/lib/AST/TypePrinter.cpp
  clang/test/Sema/nullability.c


Index: clang/test/Sema/nullability.c
===
--- clang/test/Sema/nullability.c
+++ clang/test/Sema/nullability.c
@@ -125,6 +125,7 @@
   int *a = ptr; // okay
   _Nonnull int *b = ptr; // expected-warning{{implicit conversion from 
nullable pointer 'int * _Nullable' to non-nullable pointer type 'int * 
_Nonnull'}}
   b = ptr; // expected-warning{{implicit conversion from nullable pointer 'int 
* _Nullable' to non-nullable pointer type 'int * _Nonnull'}}
+  __auto_type _Nonnull c = ptr; // expected-warning{{implicit conversion from 
nullable pointer 'int * _Nullable' to non-nullable pointer type 'int * 
_Nullable _Nonnull'}}
 
   accepts_nonnull_1(ptr); // expected-warning{{implicit conversion from 
nullable pointer 'int * _Nullable' to non-nullable pointer type 'int * 
_Nonnull'}}
 }
Index: clang/lib/AST/TypePrinter.cpp
===
--- clang/lib/AST/TypePrinter.cpp
+++ clang/lib/AST/TypePrinter.cpp
@@ -280,7 +280,7 @@
 case Type::Attributed: {
   // We still want to print the address_space before the type if it is an
   // address_space attribute.
-  const auto *AttrTy = cast(T);
+  const auto *AttrTy = cast(UnderlyingType);
   CanPrefixQualifiers = AttrTy->getAttrKind() == attr::AddressSpace;
 }
   }


Index: clang/test/Sema/nullability.c
===
--- clang/test/Sema/nullability.c
+++ clang/test/Sema/nullability.c
@@ -125,6 +125,7 @@
   int *a = ptr; // okay
   _Nonnull int *b = ptr; // expected-warning{{implicit conversion from nullable pointer 'int * _Nullable' to non-nullable pointer type 'int * _Nonnull'}}
   b = ptr; // expected-warning{{implicit conversion from nullable pointer 'int * _Nullable' to non-nullable pointer type 'int * _Nonnull'}}
+  __auto_type _Nonnull c = ptr; // expected-warning{{implicit conversion from nullable pointer 'int * _Nullable' to non-nullable pointer type 'int * _Nullable _Nonnull'}}
 
   accepts_nonnull_1(ptr); // expected-warning{{implicit conversion from nullable pointer 'int * _Nullable' to non-nullable pointer type 'int * _Nonnull'}}
 }
Index: clang/lib/AST/TypePrinter.cpp
===
--- clang/lib/AST/TypePrinter.cpp
+++ clang/lib/AST/TypePrinter.cpp
@@ -280,7 +280,7 @@
 case Type::Attributed: {
   // We still want to print the address_space before the type if it is an
   // address_space attribute.
-  const auto *AttrTy = cast(T);
+  const auto *AttrTy = cast(UnderlyingType);
   CanPrefixQualifiers = AttrTy->getAttrKind() == attr::AddressSpace;
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini created this revision.
mehdi_amini added a reviewer: Eugene.Zelenko.
Herald added subscribers: carlosgalvezp, xazax.hun.
mehdi_amini requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

The rational to avoid applying the warning/fix in non-Strict more to
functions with an empty body is that there is "no place for a bug to hide".
However for private functions, the parameters can be entirely eliminated
and all the call sites improved.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116512

Files:
  clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
  clang-tools-extra/docs/clang-tidy/checks/misc-unused-parameters.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
@@ -154,6 +154,10 @@
 namespace {
 class C {
 public:
+// CHECK-FIXES: C() {}
+  C(int i) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning
+
   void f(int i);
 // CHECK-FIXES: void f();
   void g(int i) {;}
@@ -181,7 +185,7 @@
 void useFunction(T t);
 
 void someMoreCallSites() {
-  C c;
+  C c(42);
   c.f(1);
 // CHECK-FIXES: c.f();
   c.g(1);
Index: clang-tools-extra/docs/clang-tidy/checks/misc-unused-parameters.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/misc-unused-parameters.rst
+++ clang-tools-extra/docs/clang-tidy/checks/misc-unused-parameters.rst
@@ -37,6 +37,8 @@
 
When `false` (default value), the check will ignore trivially unused 
parameters,
i.e. when the corresponding function has an empty body (and in case of
-   constructors - no constructor initializers). When the function body is 
empty,
-   an unused parameter is unlikely to be unnoticed by a human reader, and
-   there's basically no place for a bug to hide.
+   constructors - no constructor initializers) and the definition is public. 
When
+   the function body is empty, an unused parameter is unlikely to be unnoticed 
by
+   a human reader, and there's basically no place for a bug to hide. On the 
other
+   hand for non-public functions, all the call-sites are visible and the 
parameter
+   can be eliminated entirely.
Index: clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -183,9 +183,9 @@
 Param->hasAttr())
   continue;
 
-// In non-strict mode ignore function definitions with empty bodies
+// In non-strict mode ignore public function definitions with empty bodies
 // (constructor initializer counts for non-empty body).
-if (StrictMode ||
+if (StrictMode || !Function->isExternallyVisible() ||
 (Function->getBody()->child_begin() !=
  Function->getBody()->child_end()) ||
 (isa(Function) &&


Index: clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
@@ -154,6 +154,10 @@
 namespace {
 class C {
 public:
+// CHECK-FIXES: C() {}
+  C(int i) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning
+
   void f(int i);
 // CHECK-FIXES: void f();
   void g(int i) {;}
@@ -181,7 +185,7 @@
 void useFunction(T t);
 
 void someMoreCallSites() {
-  C c;
+  C c(42);
   c.f(1);
 // CHECK-FIXES: c.f();
   c.g(1);
Index: clang-tools-extra/docs/clang-tidy/checks/misc-unused-parameters.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/misc-unused-parameters.rst
+++ clang-tools-extra/docs/clang-tidy/checks/misc-unused-parameters.rst
@@ -37,6 +37,8 @@
 
When `false` (default value), the check will ignore trivially unused parameters,
i.e. when the corresponding function has an empty body (and in case of
-   constructors - no constructor initializers). When the function body is empty,
-   an unused parameter is unlikely to be unnoticed by a human reader, and
-   there's basically no place for a bug to hide.
+   constructors - no constructor initializers) and the definition is public. When
+   the function body is empty, an unused parameter is unlikely to be unnoticed by
+   a human reader, and there's basically no place for a bug to hide. On the other
+   hand for non-public functions, all the call-sites are visible and the parameter
+   can be eliminated entirely.
Index: clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp

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

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini created this revision.
mehdi_amini added a reviewer: Eugene.Zelenko.
Herald added subscribers: carlosgalvezp, xazax.hun.
mehdi_amini requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

These weren't tracked and so weren't updated when applying fixes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116513

Files:
  clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
@@ -186,6 +186,7 @@
 
 void someMoreCallSites() {
   C c(42);
+// CHECK-FIXES: C c();
   c.f(1);
 // CHECK-FIXES: c.f();
   c.g(1);
Index: clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -64,8 +64,9 @@
: nullptr));
 }
 
+template 
 static FixItHint removeArgument(const MatchFinder::MatchResult &Result,
-const CallExpr *Call, unsigned Index) {
+const CallExprT *Call, unsigned Index) {
   return FixItHint::CreateRemoval(removeNode(
   Result, Index > 0 ? Call->getArg(Index - 1) : nullptr,
   Call->getArg(Index),
@@ -75,13 +76,20 @@
 class UnusedParametersCheck::IndexerVisitor
 : public RecursiveASTVisitor {
 public:
-  IndexerVisitor(ASTContext &Ctx) { TraverseAST(Ctx); }
+  IndexerVisitor(ASTContext &Ctx) : mgr(Ctx.getSourceManager()) {
+TraverseAST(Ctx);
+  }
 
   const std::unordered_set &
   getFnCalls(const FunctionDecl *Fn) {
 return Index[Fn->getCanonicalDecl()].Calls;
   }
 
+  const std::unordered_set &
+  getCtorCalls(const FunctionDecl *Fn) {
+return Index[Fn->getCanonicalDecl()].CtorRefs;
+  }
+
   const std::unordered_set &
   getOtherRefs(const FunctionDecl *Fn) {
 return Index[Fn->getCanonicalDecl()].OtherRefs;
@@ -97,6 +105,15 @@
 return true;
   }
 
+  bool WalkUpFromCXXConstructExpr(CXXConstructExpr *Call) {
+if (const auto *Ctor =
+dyn_cast_or_null(Call->getConstructor())) {
+  Ctor = Ctor->getCanonicalDecl();
+  Index[Ctor].CtorRefs.insert(Call);
+}
+return true;
+  }
+
   bool WalkUpFromCallExpr(CallExpr *Call) {
 if (const auto *Fn =
 dyn_cast_or_null(Call->getCalleeDecl())) {
@@ -114,8 +131,9 @@
   struct IndexEntry {
 std::unordered_set Calls;
 std::unordered_set OtherRefs;
+std::unordered_set CtorRefs;
   };
-
+  SourceManager &mgr;
   std::unordered_map Index;
 };
 
@@ -165,9 +183,14 @@
   MyDiag << removeParameter(Result, FD, ParamIndex);
 
   // Fix all call sites.
-  for (const CallExpr *Call : Indexer->getFnCalls(Function))
+  for (const CallExpr *Call : Indexer->getFnCalls(Function)) {
+if (ParamIndex < Call->getNumArgs()) // See PR38055 for example.
+  MyDiag << removeArgument(Result, Call, ParamIndex);
+  }
+  for (const CXXConstructExpr *Call : Indexer->getCtorCalls(Function)) {
 if (ParamIndex < Call->getNumArgs()) // See PR38055 for example.
   MyDiag << removeArgument(Result, Call, ParamIndex);
+  }
 }
 
 void UnusedParametersCheck::check(const MatchFinder::MatchResult &Result) {


Index: clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
@@ -186,6 +186,7 @@
 
 void someMoreCallSites() {
   C c(42);
+// CHECK-FIXES: C c();
   c.f(1);
 // CHECK-FIXES: c.f();
   c.g(1);
Index: clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -64,8 +64,9 @@
: nullptr));
 }
 
+template 
 static FixItHint removeArgument(const MatchFinder::MatchResult &Result,
-const CallExpr *Call, unsigned Index) {
+const CallExprT *Call, unsigned Index) {
   return FixItHint::CreateRemoval(removeNode(
   Result, Index > 0 ? Call->getArg(Index - 1) : nullptr,
   Call->getArg(Index),
@@ -75,13 +76,20 @@
 class UnusedParametersCheck::IndexerVisitor
 : public RecursiveASTVisitor {
 public:
-  IndexerVisitor(ASTContext &Ctx) { TraverseAST(Ctx); }
+  IndexerVisitor(ASTContext &Ctx) : mgr(Ctx.getSourceManager()) {

[PATCH] D116502: [clangd] Helper for determining member insertion point.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 396977.
sammccall added a comment.

[clangd] Add code action to generate a constructor for a C++ class.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116502

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/refactor/InsertionPoint.cpp
  clang-tools-extra/clangd/refactor/InsertionPoint.h
  clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
  clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/InsertionPointTests.cpp
  clang-tools-extra/clangd/unittests/tweaks/MemberwiseConstructorTests.cpp

Index: clang-tools-extra/clangd/unittests/tweaks/MemberwiseConstructorTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/tweaks/MemberwiseConstructorTests.cpp
@@ -0,0 +1,98 @@
+//===-- MemberwiseConstructorTests.cpp ===//
+//
+// 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 "TweakTesting.h"
+#include "gmock/gmock-matchers.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+using testing::AllOf;
+using testing::AnyOf;
+using testing::Contains;
+using testing::Eq;
+using testing::HasSubstr;
+using testing::Not;
+
+TWEAK_TEST(MemberwiseConstructor);
+
+TEST_F(MemberwiseConstructorTest, Availability) {
+  EXPECT_AVAILABLE("^struct ^S ^{ int x, y; };");
+  EXPECT_UNAVAILABLE("struct S { ^int ^x, y; }; struct ^S;");
+  EXPECT_UNAVAILABLE("struct ^S {};");
+  EXPECT_UNAVAILABLE("union ^S { int x; };");
+  EXPECT_UNAVAILABLE("struct ^S { int x = 0; };");
+  EXPECT_UNAVAILABLE("struct ^S { struct { int x; }; };");
+}
+
+TEST_F(MemberwiseConstructorTest, Edits) {
+  Header = R"cpp(
+struct Move {
+  Move(Move&&) = default;
+  Move(const Move&) = delete;
+};
+struct Copy {
+  Copy(Copy&&) = delete;
+  Copy(const Copy&);
+};
+  )cpp";
+  EXPECT_EQ(apply("struct ^S{Move M; Copy C; int I; int J=4;};"),
+"struct S{"
+"S(Move M, const Copy &C, int I) : M(std::move(M)), C(C), I(I) {}\n"
+"Move M; Copy C; int I; int J=4;};");
+}
+
+TEST_F(MemberwiseConstructorTest, FieldTreatment) {
+  Header = R"cpp(
+struct MoveOnly {
+  MoveOnly(MoveOnly&&) = default;
+  MoveOnly(const MoveOnly&) = delete;
+};
+struct CopyOnly {
+  CopyOnly(CopyOnly&&) = delete;
+  CopyOnly(const CopyOnly&);
+};
+struct CopyTrivial {
+  CopyTrivial(CopyTrivial&&) = default;
+  CopyTrivial(const CopyTrivial&) = default;
+};
+struct Immovable {
+  Immovable(Immovable&&) = delete;
+  Immovable(const Immovable&) = delete;
+};
+template 
+struct Traits { using Type = typename T::Type; };
+  )cpp";
+
+  auto Fail = Eq("unavailable");
+  auto Move = HasSubstr(": Member(std::move(Member))");
+  auto CopyRef = AllOf(HasSubstr("S(const "), HasSubstr(": Member(Member)"));
+  auto Copy = AllOf(Not(HasSubstr("S(const ")), HasSubstr(": Member(Member)"));
+  auto With = [](llvm::StringRef Type) {
+return ("struct ^S { " + Type + " Member; };").str();
+  };
+
+  EXPECT_THAT(apply(With("Immovable")), Fail);
+  EXPECT_THAT(apply(With("MoveOnly")), Move);
+  EXPECT_THAT(apply(With("CopyOnly")), CopyRef);
+  EXPECT_THAT(apply(With("CopyTrivial")), Copy);
+  EXPECT_THAT(apply(With("int")), Copy);
+  EXPECT_THAT(apply(With("Immovable*")), Copy);
+  EXPECT_THAT(apply(With("Immovable&")), Copy);
+
+  EXPECT_THAT(apply("template " + With("T")), Move);
+  EXPECT_THAT(apply("template " + With("typename Traits::Type")),
+  Move);
+  EXPECT_THAT(apply("template " + With("T*")), Copy);
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/unittests/InsertionPointTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/InsertionPointTests.cpp
@@ -0,0 +1,175 @@
+//===-- InsertionPointTess.cpp  ---===//
+//
+// 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 "Annotations.h"
+#include "Protocol.h"
+#include "SourceCode.h"
+#include "TestTU.h"
+#include "TestWorkspace.h"
+#include "XRefs.h"
+#include "refa

[PATCH] D116502: [clangd] Helper for determining member insertion point.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 396978.
sammccall added a comment.

Oops, revert wrong patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116502

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/refactor/InsertionPoint.cpp
  clang-tools-extra/clangd/refactor/InsertionPoint.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/InsertionPointTests.cpp

Index: clang-tools-extra/clangd/unittests/InsertionPointTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/InsertionPointTests.cpp
@@ -0,0 +1,175 @@
+//===-- InsertionPointTess.cpp  ---===//
+//
+// 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 "Annotations.h"
+#include "Protocol.h"
+#include "SourceCode.h"
+#include "TestTU.h"
+#include "TestWorkspace.h"
+#include "XRefs.h"
+#include "refactor/InsertionPoint.h"
+#include "clang/AST/DeclBase.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(InsertionPointTests, Generic) {
+  Annotations Code(R"cpp(
+  namespace ns {
+$a^int a1;
+$b^// leading comment
+int b;
+$c^int c1; // trailing comment
+int c2;
+$a2^int a2;
+  $end^};
+  )cpp");
+
+  auto StartsWith =
+  [&](llvm::StringLiteral S) -> std::function {
+return [S](const Decl *D) {
+  if (const auto *ND = llvm::dyn_cast(D))
+return llvm::StringRef(ND->getNameAsString()).startswith(S);
+  return false;
+};
+  };
+
+  auto AST = TestTU::withCode(Code.code()).build();
+  auto &NS = cast(findDecl(AST, "ns"));
+
+  // Test single anchors.
+  auto Point = [&](llvm::StringLiteral Prefix, Anchor::Dir Direction) {
+auto Loc = insertionPoint(NS, {Anchor{StartsWith(Prefix), Direction}});
+return sourceLocToPosition(AST.getSourceManager(), Loc);
+  };
+  EXPECT_EQ(Point("a", Anchor::Above), Code.point("a"));
+  EXPECT_EQ(Point("a", Anchor::Below), Code.point("b"));
+  EXPECT_EQ(Point("b", Anchor::Above), Code.point("b"));
+  EXPECT_EQ(Point("b", Anchor::Below), Code.point("c"));
+  EXPECT_EQ(Point("c", Anchor::Above), Code.point("c"));
+  EXPECT_EQ(Point("c", Anchor::Below), Code.point("a2"));
+  EXPECT_EQ(Point("", Anchor::Above), Code.point("a"));
+  EXPECT_EQ(Point("", Anchor::Below), Code.point("end"));
+  EXPECT_EQ(Point("no_match", Anchor::Below), Position{});
+
+  // Test anchor chaining.
+  auto Chain = [&](llvm::StringLiteral P1, llvm::StringLiteral P2) {
+auto Loc = insertionPoint(NS, {Anchor{StartsWith(P1), Anchor::Above},
+   Anchor{StartsWith(P2), Anchor::Above}});
+return sourceLocToPosition(AST.getSourceManager(), Loc);
+  };
+  EXPECT_EQ(Chain("a", "b"), Code.point("a"));
+  EXPECT_EQ(Chain("b", "a"), Code.point("b"));
+  EXPECT_EQ(Chain("no_match", "a"), Code.point("a"));
+
+  // Test edit generation.
+  auto Edit = insertDecl("foo;", NS, {Anchor{StartsWith("a"), Anchor::Below}});
+  ASSERT_THAT_EXPECTED(Edit, llvm::Succeeded());
+  EXPECT_EQ(offsetToPosition(Code.code(), Edit->getOffset()), Code.point("b"));
+  EXPECT_EQ(Edit->getReplacementText(), "foo;");
+  // If no match, the edit is inserted at the end.
+  Edit = insertDecl("x;", NS, {Anchor{StartsWith("no_match"), Anchor::Below}});
+  ASSERT_THAT_EXPECTED(Edit, llvm::Succeeded());
+  EXPECT_EQ(offsetToPosition(Code.code(), Edit->getOffset()),
+Code.point("end"));
+}
+
+// For CXX, we should check:
+// - special handling for access specifiers
+// - unwrapping of template decls
+TEST(InsertionPointTests, CXX) {
+  Annotations Code(R"cpp(
+class C {
+public:
+  $Method^void pubMethod();
+  $Field^int PubField;
+
+$private^private:
+  $field^int PrivField;
+  $method^void privMethod();
+  template  void privTemplateMethod();
+$end^};
+  )cpp");
+
+  auto AST = TestTU::withCode(Code.code()).build();
+  const CXXRecordDecl &C = cast(findDecl(AST, "C"));
+
+  auto IsMethod = [](const Decl *D) { return llvm::isa(D); };
+  auto Any = [](const Decl *D) { return true; };
+
+  // Test single anchors.
+  auto Point = [&](Anchor A, AccessSpecifier Protection) {
+auto Loc = insertionPoint(C, {A}, Protection);
+return sourceLocToPosition(AST.getSourceManager(), Loc);
+  };
+  EXPECT_EQ(Point({IsMethod, Anchor::Above}, AS_public), Code.point("Method"));
+  EXPECT_EQ(Point({IsMethod, Anchor::Below}, AS_public), Code.point("Field"));
+  EXPECT_EQ(Point({Any, Anchor::Above}, AS_public), Code.point("Method"));
+  EXPECT_EQ(Point({Any, An

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

[Sam McCall via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116514

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
  clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/tweaks/MemberwiseConstructorTests.cpp

Index: clang-tools-extra/clangd/unittests/tweaks/MemberwiseConstructorTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/tweaks/MemberwiseConstructorTests.cpp
@@ -0,0 +1,98 @@
+//===-- MemberwiseConstructorTests.cpp ===//
+//
+// 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 "TweakTesting.h"
+#include "gmock/gmock-matchers.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+using testing::AllOf;
+using testing::AnyOf;
+using testing::Contains;
+using testing::Eq;
+using testing::HasSubstr;
+using testing::Not;
+
+TWEAK_TEST(MemberwiseConstructor);
+
+TEST_F(MemberwiseConstructorTest, Availability) {
+  EXPECT_AVAILABLE("^struct ^S ^{ int x, y; };");
+  EXPECT_UNAVAILABLE("struct S { ^int ^x, y; }; struct ^S;");
+  EXPECT_UNAVAILABLE("struct ^S {};");
+  EXPECT_UNAVAILABLE("union ^S { int x; };");
+  EXPECT_UNAVAILABLE("struct ^S { int x = 0; };");
+  EXPECT_UNAVAILABLE("struct ^S { struct { int x; }; };");
+}
+
+TEST_F(MemberwiseConstructorTest, Edits) {
+  Header = R"cpp(
+struct Move {
+  Move(Move&&) = default;
+  Move(const Move&) = delete;
+};
+struct Copy {
+  Copy(Copy&&) = delete;
+  Copy(const Copy&);
+};
+  )cpp";
+  EXPECT_EQ(apply("struct ^S{Move M; Copy C; int I; int J=4;};"),
+"struct S{"
+"S(Move M, const Copy &C, int I) : M(std::move(M)), C(C), I(I) {}\n"
+"Move M; Copy C; int I; int J=4;};");
+}
+
+TEST_F(MemberwiseConstructorTest, FieldTreatment) {
+  Header = R"cpp(
+struct MoveOnly {
+  MoveOnly(MoveOnly&&) = default;
+  MoveOnly(const MoveOnly&) = delete;
+};
+struct CopyOnly {
+  CopyOnly(CopyOnly&&) = delete;
+  CopyOnly(const CopyOnly&);
+};
+struct CopyTrivial {
+  CopyTrivial(CopyTrivial&&) = default;
+  CopyTrivial(const CopyTrivial&) = default;
+};
+struct Immovable {
+  Immovable(Immovable&&) = delete;
+  Immovable(const Immovable&) = delete;
+};
+template 
+struct Traits { using Type = typename T::Type; };
+  )cpp";
+
+  auto Fail = Eq("unavailable");
+  auto Move = HasSubstr(": Member(std::move(Member))");
+  auto CopyRef = AllOf(HasSubstr("S(const "), HasSubstr(": Member(Member)"));
+  auto Copy = AllOf(Not(HasSubstr("S(const ")), HasSubstr(": Member(Member)"));
+  auto With = [](llvm::StringRef Type) {
+return ("struct ^S { " + Type + " Member; };").str();
+  };
+
+  EXPECT_THAT(apply(With("Immovable")), Fail);
+  EXPECT_THAT(apply(With("MoveOnly")), Move);
+  EXPECT_THAT(apply(With("CopyOnly")), CopyRef);
+  EXPECT_THAT(apply(With("CopyTrivial")), Copy);
+  EXPECT_THAT(apply(With("int")), Copy);
+  EXPECT_THAT(apply(With("Immovable*")), Copy);
+  EXPECT_THAT(apply(With("Immovable&")), Copy);
+
+  EXPECT_THAT(apply("template " + With("T")), Move);
+  EXPECT_THAT(apply("template " + With("typename Traits::Type")),
+  Move);
+  EXPECT_THAT(apply("template " + With("T*")), Copy);
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -117,6 +117,7 @@
   tweaks/ExpandMacroTests.cpp
   tweaks/ExtractFunctionTests.cpp
   tweaks/ExtractVariableTests.cpp
+  tweaks/MemberwiseConstructorTests.cpp
   tweaks/ObjCLocalizeStringLiteralTests.cpp
   tweaks/PopulateSwitchTests.cpp
   tweaks/RawStringLiteralTests.cpp
Index: clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp
@@ -0,0 +1,256 @@
+//===--- MemberwiseConstructor.cpp - Generate C++ constructor -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 

[PATCH] D114425: [clang] Add __builtin_bswap128

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

OOC, how hard would it be to generalize this builtin a little? It is nice that 
we have builtins like `__builtin_add_overflow` which do the right thing 
regardless of their input.

It seems like it would be nice if we started to expose more intrinsics which 
did the right thing regardless of operand width; another bonus is that it 
composes well with language features like `_BitInt`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114425

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

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

This is looking great! Just a few more questions.

What is the behavior with something like:

  thread_local int x = 2;
  int f() {
return x;
  }

I'm wondering if we need to move this logic 

 into the generic C++ ABI implementation.


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

https://reviews.llvm.org/D115456

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

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

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood created this revision.
LegalizeAdulthood added a reviewer: klimek.
LegalizeAdulthood added a project: clang.
LegalizeAdulthood requested review of this revision.

This allows the matching of two specific statements in sequence within
a compound statement.  For instance `if (x) return true; return false;`
can be matched as

  compoundStmt(hasSubstatementSequence(ifStmt(), returnStmt()))


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116518

Files:
  clang/docs/LibASTMatchersReference.html
  clang/docs/ReleaseNotes.rst
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2604,6 +2604,19 @@
   EXPECT_TRUE(matchesObjC("void f() { ^{}(); }", blockExpr()));
 }
 
+TEST_P(ASTMatchersTest, HasSubstatementSequence) {
+  const char *Text =
+  "int f() { int x = 5; if (x < 0) return 1; return 0; }";
+  EXPECT_TRUE(matches(
+  Text, compoundStmt(hasSubstatementSequence(ifStmt(), returnStmt();
+  EXPECT_FALSE(matches(
+  Text, compoundStmt(hasSubstatementSequence(ifStmt(), labelStmt();
+  EXPECT_FALSE(matches(
+  Text, compoundStmt(hasSubstatementSequence(returnStmt(), ifStmt();
+  EXPECT_FALSE(matches(
+  Text, compoundStmt(hasSubstatementSequence(switchStmt(), labelStmt();
+}
+
 TEST_P(ASTMatchersTest,
StatementCountIs_FindsNoStatementsInAnEmptyCompoundStatement) {
   EXPECT_TRUE(matches("void f() { }", compoundStmt(statementCountIs(0;
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -355,6 +355,7 @@
   REGISTER_MATCHER(hasSpecializedTemplate);
   REGISTER_MATCHER(hasStaticStorageDuration);
   REGISTER_MATCHER(hasStructuredBlock);
+  REGISTER_MATCHER(hasSubstatementSequence);
   REGISTER_MATCHER(hasSyntacticForm);
   REGISTER_MATCHER(hasTargetDecl);
   REGISTER_MATCHER(hasTemplateArgument);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -5432,6 +5432,32 @@
   Builder) != CS->body_end();
 }
 
+/// Matches two consecutive statements within a compound statement.
+///
+/// Given
+/// \code
+///   { if (x > 0) return true; return false; }
+/// \endcode
+/// compoundStmt(hasSubstatementSequence(ifStmt(), returnStmt()))
+///   matches '{ if (x > 0) return true; return false; }'
+AST_POLYMORPHIC_MATCHER_P2(hasSubstatementSequence,
+   AST_POLYMORPHIC_SUPPORTED_TYPES(CompoundStmt,
+   StmtExpr),
+   internal::Matcher, InnerMatcher1,
+   internal::Matcher, InnerMatcher2) {
+  if (const CompoundStmt *CS = CompoundStmtMatcher::get(Node)) {
+auto It = matchesFirstInPointerRange(InnerMatcher1, CS->body_begin(),
+ CS->body_end(), Finder, Builder);
+if (It == CS->body_end())
+  return false;
+++It;
+if (It == CS->body_end())
+  return false;
+return InnerMatcher2.matches(**It, Finder, Builder);
+  }
+  return false;
+}
+
 /// Checks that a compound statement contains a specific number of
 /// child statements.
 ///
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -277,6 +277,8 @@
   and the underlying ``Type`` with ``hasUnderlyingType``.
   ``hasDeclaration`` continues to see through the alias and apply to the
   underlying type.
+- The ``hasSubstatementSequence`` matcher has been added to match two statements
+  appearing sequentially within a compound statement.
 
 clang-format
 
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -3775,6 +3775,16 @@
 
 
 
+MatcherCompoundStmt>hasSubstatementSequenceMatcherStmt> InnerMatcher1, MatcherStmt> InnerMatcher2
+Checks that a compound statement contains two sequential statements as matched by the two matchers.
+
+Example: Given
+  { if (x > 10) return false; return true; }
+compoundStmt(hasSubstatementSequence(ifStmt(), return

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

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood updated this revision to Diff 397004.
LegalizeAdulthood added a comment.

Improve matching when the first matcher matches multiple times,
but the 2nd matcher doesn't match at first.


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

https://reviews.llvm.org/D116518

Files:
  clang/docs/LibASTMatchersReference.html
  clang/docs/ReleaseNotes.rst
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2604,6 +2604,52 @@
   EXPECT_TRUE(matchesObjC("void f() { ^{}(); }", blockExpr()));
 }
 
+TEST_P(ASTMatchersTest, HasSubstatementSequenceSimple) {
+  const char *Text = "int f() { int x = 5; if (x < 0) return 1; return 0; }";
+  EXPECT_TRUE(matches(
+  Text, compoundStmt(hasSubstatementSequence(ifStmt(), returnStmt();
+  EXPECT_FALSE(matches(
+  Text, compoundStmt(hasSubstatementSequence(ifStmt(), labelStmt();
+  EXPECT_FALSE(matches(
+  Text, compoundStmt(hasSubstatementSequence(returnStmt(), ifStmt();
+  EXPECT_FALSE(matches(
+  Text, compoundStmt(hasSubstatementSequence(switchStmt(), labelStmt();
+}
+
+TEST_P(ASTMatchersTest, HasSubstatementSequenceAlmost) {
+  const char *Text = R"code(
+int f() {
+  int x = 5;
+  if (x < 10)
+;
+  if (x < 0)
+return 1;
+  return 0;
+}
+)code";
+  EXPECT_TRUE(matches(
+  Text, compoundStmt(hasSubstatementSequence(ifStmt(), returnStmt();
+  EXPECT_TRUE(
+  matches(Text, compoundStmt(hasSubstatementSequence(ifStmt(), ifStmt();
+}
+
+TEST_P(ASTMatchersTest, HasSubstatementSequenceComplex) {
+  const char *Text = R"code(
+int f() {
+  int x = 5;
+  if (x < 10)
+x -= 10;
+  if (x < 0)
+return 1;
+  return 0;
+}
+)code";
+  EXPECT_TRUE(matches(
+  Text, compoundStmt(hasSubstatementSequence(ifStmt(), returnStmt();
+  EXPECT_FALSE(
+  matches(Text, compoundStmt(hasSubstatementSequence(ifStmt(), expr();
+}
+
 TEST_P(ASTMatchersTest,
StatementCountIs_FindsNoStatementsInAnEmptyCompoundStatement) {
   EXPECT_TRUE(matches("void f() { }", compoundStmt(statementCountIs(0;
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -355,6 +355,7 @@
   REGISTER_MATCHER(hasSpecializedTemplate);
   REGISTER_MATCHER(hasStaticStorageDuration);
   REGISTER_MATCHER(hasStructuredBlock);
+  REGISTER_MATCHER(hasSubstatementSequence);
   REGISTER_MATCHER(hasSyntacticForm);
   REGISTER_MATCHER(hasTargetDecl);
   REGISTER_MATCHER(hasTemplateArgument);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -5432,6 +5432,35 @@
   Builder) != CS->body_end();
 }
 
+/// Matches two consecutive statements within a compound statement.
+///
+/// Given
+/// \code
+///   { if (x > 0) return true; return false; }
+/// \endcode
+/// compoundStmt(hasSubstatementSequence(ifStmt(), returnStmt()))
+///   matches '{ if (x > 0) return true; return false; }'
+AST_POLYMORPHIC_MATCHER_P2(hasSubstatementSequence,
+   AST_POLYMORPHIC_SUPPORTED_TYPES(CompoundStmt,
+   StmtExpr),
+   internal::Matcher, InnerMatcher1,
+   internal::Matcher, InnerMatcher2) {
+  if (const CompoundStmt *CS = CompoundStmtMatcher::get(Node)) {
+auto It = matchesFirstInPointerRange(InnerMatcher1, CS->body_begin(),
+ CS->body_end(), Finder, Builder);
+while (It != CS->body_end()) {
+  ++It;
+  if (It == CS->body_end())
+return false;
+  if (InnerMatcher2.matches(**It, Finder, Builder))
+return true;
+  It = matchesFirstInPointerRange(InnerMatcher1, It, CS->body_end(), Finder,
+  Builder);
+}
+  }
+  return false;
+}
+
 /// Checks that a compound statement contains a specific number of
 /// child statements.
 ///
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -277,6 +277,8 @@
   and the underlying ``Type`` with ``hasUnderlyingType``.
   ``hasDeclaration`` continues to see through the alias and apply to the
   underlying type.
+- The ``hasSubstatementSequence`` matcher has been added to match two statements
+  appearing sequentially within a compound statement

[PATCH] D116521: [llvm][clang][cmake] Factor out config prefix finding logic

[John Ericson via Phabricator via cfe-commits]

Ericson2314 created this revision.
Ericson2314 added reviewers: mstorsjo, beanz.
Herald added a subscriber: mgorny.
Ericson2314 requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

See the docs in the new function for details.

We get the shared, non-installed CMake modules following the pattern
established in D116472 .

It might be good to have LLD and Flang also use this, but that would be
a functional change and so I leave it as future work.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116521

Files:
  clang/CMakeLists.txt
  clang/cmake/modules/CMakeLists.txt
  cmake/Modules/FindPrefixFromConfig.cmake
  llvm/CMakeLists.txt
  llvm/cmake/modules/CMakeLists.txt

Index: llvm/cmake/modules/CMakeLists.txt
===
--- llvm/cmake/modules/CMakeLists.txt
+++ llvm/cmake/modules/CMakeLists.txt
@@ -1,4 +1,5 @@
 include(LLVMDistributionSupport)
+include(FindPrefixFromConfig)
 
 set(LLVM_INSTALL_PACKAGE_DIR lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm)
 set(llvm_cmake_builddir "${LLVM_BINARY_DIR}/${LLVM_INSTALL_PACKAGE_DIR}")
@@ -99,16 +100,7 @@
   )
 
 # Generate LLVMConfig.cmake for the install tree.
-set(LLVM_CONFIG_CODE "
-# Compute the installation prefix from this LLVMConfig.cmake file location.
-get_filename_component(LLVM_INSTALL_PREFIX \"\${CMAKE_CURRENT_LIST_FILE}\" PATH)")
-# Construct the proper number of get_filename_component(... PATH)
-# calls to compute the installation prefix.
-string(REGEX REPLACE "/" ";" _count "${LLVM_INSTALL_PACKAGE_DIR}")
-foreach(p ${_count})
-  set(LLVM_CONFIG_CODE "${LLVM_CONFIG_CODE}
-get_filename_component(LLVM_INSTALL_PREFIX \"\${LLVM_INSTALL_PREFIX}\" PATH)")
-endforeach(p)
+find_prefix_from_config(LLVM_CONFIG_CODE LLVM_INSTALL_PREFIX "${LLVM_INSTALL_PACKAGE_DIR}")
 set(LLVM_CONFIG_INCLUDE_DIRS "\${LLVM_INSTALL_PREFIX}/include")
 set(LLVM_CONFIG_INCLUDE_DIR "${LLVM_CONFIG_INCLUDE_DIRS}")
 set(LLVM_CONFIG_MAIN_INCLUDE_DIR "${LLVM_CONFIG_INCLUDE_DIRS}")
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -196,11 +196,13 @@
   set(LLVM_GISEL_COV_PREFIX "${CMAKE_BINARY_DIR}/gisel-coverage-" CACHE STRING "Provide a filename prefix to collect the GlobalISel rule coverage")
 endif()
 
+set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
+
 # Add path for custom modules
-set(CMAKE_MODULE_PATH
-  ${CMAKE_MODULE_PATH}
+list(INSERT CMAKE_MODULE_PATH 0
   "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
   "${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules"
+  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
   )
 
 # Generate a CompilationDatabase (compile_commands.json file) for our build,
@@ -308,7 +310,6 @@
 set(LLVM_BINARY_DIR   ${CMAKE_CURRENT_BINARY_DIR}  ) # --prefix
 
 set(LLVM_THIRD_PARTY_DIR  ${CMAKE_CURRENT_SOURCE_DIR}/../third-party)
-set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
 
 # Note: LLVM_CMAKE_DIR does not include generated files
 set(LLVM_CMAKE_DIR ${LLVM_MAIN_SRC_DIR}/cmake/modules)
Index: cmake/Modules/FindPrefixFromConfig.cmake
===
--- /dev/null
+++ cmake/Modules/FindPrefixFromConfig.cmake
@@ -0,0 +1,41 @@
+# Find the prefix from the `*Config.cmake` file being generated.
+#
+# When generating an installed `*Config.cmake` file, we often want to be able
+# to refer to the ancestor directory which contains all the installed files.
+#
+# We want to do this without baking in an absolute path when the config file is
+# generated, in order to allow for a "relocatable" binary distribution that
+# doesn't need to know what path it ends up being installed at when it is
+# built.
+#
+# The solution that we know the relative path that the config file will be at
+# within that prefix, like `"${prefix_var}/lib/cmake/${project}"`, so we count
+# the number of components in that path to figure out how many parent dirs we
+# need to traverse from the location of the config file to get to the prefix
+# dir.
+#
+# out_var:
+#   variable to set the "return value" of the function, which is the code to
+#   include in the config file under construction.
+#
+# prefix_var:
+#   Name of the variable to define in the returned code (not directory for the
+#   faller!) that will contain the prefix path.
+#
+# path_to_leave:
+#   Path from the prefix to the config file, a relative path which we wish to
+#   go up and out from to find the prefix directory.
+function(find_prefix_from_config out_var prefix_var path_to_leave)
+  set(config_code
+"# Compute the installation prefix from this LLVMConfig.cmake file location."
+"get_filename_component(${prefix_var} \"\${CMAKE_CURRENT_LIST_FILE}\" PATH)")
+  # Construct the proper number of get_filename_component(... PATH)
+  # calls to compute the installation prefix.
+  string(REGEX REPLACE "/" ";" _cou

[PATCH] D116521: [llvm][clang][cmake] Factor out config prefix finding logic

[John Ericson via Phabricator via cfe-commits]

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

Fix arg order error in Clang


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116521

Files:
  clang/CMakeLists.txt
  clang/cmake/modules/CMakeLists.txt
  cmake/Modules/FindPrefixFromConfig.cmake
  llvm/CMakeLists.txt
  llvm/cmake/modules/CMakeLists.txt

Index: llvm/cmake/modules/CMakeLists.txt
===
--- llvm/cmake/modules/CMakeLists.txt
+++ llvm/cmake/modules/CMakeLists.txt
@@ -1,4 +1,5 @@
 include(LLVMDistributionSupport)
+include(FindPrefixFromConfig)
 
 set(LLVM_INSTALL_PACKAGE_DIR lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm)
 set(llvm_cmake_builddir "${LLVM_BINARY_DIR}/${LLVM_INSTALL_PACKAGE_DIR}")
@@ -99,16 +100,7 @@
   )
 
 # Generate LLVMConfig.cmake for the install tree.
-set(LLVM_CONFIG_CODE "
-# Compute the installation prefix from this LLVMConfig.cmake file location.
-get_filename_component(LLVM_INSTALL_PREFIX \"\${CMAKE_CURRENT_LIST_FILE}\" PATH)")
-# Construct the proper number of get_filename_component(... PATH)
-# calls to compute the installation prefix.
-string(REGEX REPLACE "/" ";" _count "${LLVM_INSTALL_PACKAGE_DIR}")
-foreach(p ${_count})
-  set(LLVM_CONFIG_CODE "${LLVM_CONFIG_CODE}
-get_filename_component(LLVM_INSTALL_PREFIX \"\${LLVM_INSTALL_PREFIX}\" PATH)")
-endforeach(p)
+find_prefix_from_config(LLVM_CONFIG_CODE LLVM_INSTALL_PREFIX "${LLVM_INSTALL_PACKAGE_DIR}")
 set(LLVM_CONFIG_INCLUDE_DIRS "\${LLVM_INSTALL_PREFIX}/include")
 set(LLVM_CONFIG_INCLUDE_DIR "${LLVM_CONFIG_INCLUDE_DIRS}")
 set(LLVM_CONFIG_MAIN_INCLUDE_DIR "${LLVM_CONFIG_INCLUDE_DIRS}")
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -196,11 +196,13 @@
   set(LLVM_GISEL_COV_PREFIX "${CMAKE_BINARY_DIR}/gisel-coverage-" CACHE STRING "Provide a filename prefix to collect the GlobalISel rule coverage")
 endif()
 
+set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
+
 # Add path for custom modules
-set(CMAKE_MODULE_PATH
-  ${CMAKE_MODULE_PATH}
+list(INSERT CMAKE_MODULE_PATH 0
   "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
   "${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules"
+  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
   )
 
 # Generate a CompilationDatabase (compile_commands.json file) for our build,
@@ -308,7 +310,6 @@
 set(LLVM_BINARY_DIR   ${CMAKE_CURRENT_BINARY_DIR}  ) # --prefix
 
 set(LLVM_THIRD_PARTY_DIR  ${CMAKE_CURRENT_SOURCE_DIR}/../third-party)
-set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
 
 # Note: LLVM_CMAKE_DIR does not include generated files
 set(LLVM_CMAKE_DIR ${LLVM_MAIN_SRC_DIR}/cmake/modules)
Index: cmake/Modules/FindPrefixFromConfig.cmake
===
--- /dev/null
+++ cmake/Modules/FindPrefixFromConfig.cmake
@@ -0,0 +1,41 @@
+# Find the prefix from the `*Config.cmake` file being generated.
+#
+# When generating an installed `*Config.cmake` file, we often want to be able
+# to refer to the ancestor directory which contains all the installed files.
+#
+# We want to do this without baking in an absolute path when the config file is
+# generated, in order to allow for a "relocatable" binary distribution that
+# doesn't need to know what path it ends up being installed at when it is
+# built.
+#
+# The solution that we know the relative path that the config file will be at
+# within that prefix, like `"${prefix_var}/lib/cmake/${project}"`, so we count
+# the number of components in that path to figure out how many parent dirs we
+# need to traverse from the location of the config file to get to the prefix
+# dir.
+#
+# out_var:
+#   variable to set the "return value" of the function, which is the code to
+#   include in the config file under construction.
+#
+# prefix_var:
+#   Name of the variable to define in the returned code (not directory for the
+#   faller!) that will contain the prefix path.
+#
+# path_to_leave:
+#   Path from the prefix to the config file, a relative path which we wish to
+#   go up and out from to find the prefix directory.
+function(find_prefix_from_config out_var prefix_var path_to_leave)
+  set(config_code
+"# Compute the installation prefix from this LLVMConfig.cmake file location."
+"get_filename_component(${prefix_var} \"\${CMAKE_CURRENT_LIST_FILE}\" PATH)")
+  # Construct the proper number of get_filename_component(... PATH)
+  # calls to compute the installation prefix.
+  string(REGEX REPLACE "/" ";" _count "${path_to_leave}")
+  foreach(p ${_count})
+list(APPEND config_code
+  "get_filename_component(${prefix_var} \"\${${prefix_var}}\" PATH)")
+  endforeach(p)
+  string(REPLACE ";" "\n" config_code "${config_code}")
+  set("${out_var}" "${config_code}" PARENT_SCOPE)
+endfunction()
Index: clang/cmake/modules/CMakeLists.txt
==

[PATCH] D114077: [clangd] Basic IncludeCleaner support for c/c++ standard library

[Kirill Bobyrev via Phabricator via cfe-commits]

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

LG with a nit, thanks!




Comment at: clang-tools-extra/clangd/Headers.h:330
+  static inline clang::clangd::stdlib::Header getEmptyKey() {
+return clang::clangd::stdlib::Header(-1);
+  }

sammccall wrote:
> kbobyrev wrote:
> > maybe `DenseMapInfo::getEmptyKey()` and 
> > `DenseMapInfo::getTombstoneKey()` just like above?
> empty/tombstone keys are reserved values, and we know what sensible reserved 
> values are better than the traits for unsigned do. 
> 
> I can fix the code above if you like.
Okay, that makes sense! Yes, that would be great if you could fix the code 
above for consistency :)



Comment at: clang-tools-extra/clangd/IncludeCleaner.h:90
+/// FIXME: remove this hack once the implementation is good enough.
+void setIncludeCleanerAnalyzesStdlib(bool B);
+

sammccall wrote:
> kbobyrev wrote:
> > Not sure but: don't we want a config option instead? We can remove it just 
> > as easily since it's all "hidden" right now.
> I think we discussed this offline a bunch?
> 
> A config option is a bunch more plumbing, and we don't actually want to 
> expose this option.
Oh, right, thanks for reminding!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114077

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

[PATCH] D116370: [clang][dataflow] Add multi-variable constant propagation example.

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev accepted this revision.
sgatev added inline comments.



Comment at: 
clang/unittests/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp:171-173
+  Vars[Var] = (E->EvaluateAsInt(R, Context) && R.Val.isInt())
+  ? ValueLattice(R.Val.getInt().getExtValue())
+  : ValueLattice::top();

ymandel wrote:
> sgatev wrote:
> > I think this makes the transfer function non-monotonic. Concretely, 
> > `Vars[var]` can be `top` and the right-hand side can be something else. The 
> > transfer function must be monotonic to guarantee termination. Perhaps this 
> > should be `Vars[Var].join(...)`?
> Keep in mind that CP analysis is not looking at a variable's value *over the 
> course of program*, but only *at each program point*. If we wanted to know 
> whether a variable could be replaced entirely with a constant, then we'd need 
> the former, in which case you'd be right that join would be correct here.
> 
> To the more general point: monotonic means something more subtle. 
> Specifically, it doesn't relate inputs to outputs, but input-output pairs to 
> each other. Specifically, for monotinically increasing functions `f`:
> ```
> if a <= b then f(a) <= f(b)
> ```
> In this case, no matter the inputs to f, the output is a constant (the value 
> of the expression at that point or `top`), which trivially satisfies the 
> requirement `f(a) <= f(b)`.
> 
> Intuitively, transfer functions model program behavior, so indeed it would be 
> odd to enforce that `x <= f(x)`. Assignment is a good example -- at that 
> program point, we know *exactly* the value of the variable, no matter what it 
> held before.
> 
> 
Agreed about the distinction in the type of analysis. What I meant is that 
`ConstantPropagationAnalysis::transfer` isn't monotonic because it could happen 
that `transfer(top, ...) = 1` and `transfer(1, ...) = top`. However, I think 
this isn't important. What's important is that `transferBlock` is monotonic 
which I think is the case for `ConstantPropagationAnalysis`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116370

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