[PATCH] D72217: [clang-tidy] Added readability-qualified-auto check

2020-01-04 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

Oh, thank you for working on this one, i've always missed this particular check.

Given that there's finally progress on const-correctness check,
should this only handle adding `*`/`&`, and leave `const` alone?

In D72217#1804628 , @Eugene.Zelenko 
wrote:

> May be check belong to LLVM module?


I wouldn't say this is really llvm-specific.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72217



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


[PATCH] D72221: Support function attribute patchable_function_entry

2020-01-04 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61253 tests passed, 0 failed 
and 736 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72221



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


[PATCH] D72217: [clang-tidy] Added readability-qualified-auto check

2020-01-04 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp:46
+}
+CharSourceRange getFixitRange(const VarDecl *Var) {
+  return CharSourceRange::getTokenRange(Var->getBeginLoc(),

Please separate with empty line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72217



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


[PATCH] D72222: [Driver][CodeGen] Add -fpatchable-function-entry=N[,M]

2020-01-04 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon question-circle color=gray} Unit tests: unknown.

{icon question-circle color=gray} clang-tidy: unknown.

{icon question-circle color=gray} clang-format: unknown.

Build artifacts 
: 
diff.json 
,
 console-log.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D7



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


[PATCH] D72218: [clang-tidy] new altera kernel name restriction check

2020-01-04 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments.



Comment at: clang-tidy/altera/KernelNameRestrictionCheck.cpp:13
+#include "clang/Lex/Preprocessor.h"
+
+using namespace clang::ast_matchers;

Please include string, vector



Comment at: clang-tidy/altera/KernelNameRestrictionCheck.cpp:69
+  for (IncludeDirective  : IncludeDirectives) {
+auto FilePath = StringRef(ID.Filename);
+auto FileName = FilePath.substr(FilePath.find_last_of("/\\") + 1);

Please don't use auto unless type is not spelled in same statement or iterator.



Comment at: clang-tidy/altera/KernelNameRestrictionCheck.cpp:70
+auto FilePath = StringRef(ID.Filename);
+auto FileName = FilePath.substr(FilePath.find_last_of("/\\") + 1);
+if (FileName.equals_lower("kernel.cl") ||

Please don't use auto unless type is not spelled in same statement or iterator.



Comment at: clang-tidy/altera/KernelNameRestrictionCheck.cpp:82
+  // Check main file for restricted names.
+  auto Entry = SM.getFileEntryForID(SM.getMainFileID());
+  StringRef FilePath = Entry->getName();

Please don't use auto unless type is not spelled in same statement or iterator.



Comment at: clang-tidy/altera/KernelNameRestrictionCheck.cpp:84
+  StringRef FilePath = Entry->getName();
+  auto FileName = FilePath.substr(FilePath.find_last_of("/\\") + 1);
+  if (FileName.equals_lower("kernel.cl") ||

Please don't use auto unless type is not spelled in same statement or iterator.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D72218



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


[PATCH] D72221: Support function attribute patchable_function_entry

2020-01-04 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61253 tests passed, 0 failed 
and 736 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72221



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


[PATCH] D72218: [clang-tidy] new altera kernel name restriction check

2020-01-04 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments.



Comment at: clang-tidy/altera/KernelNameRestrictionCheck.cpp:21
+namespace {
+class KernelNameRestrictionPPCallbacks : public PPCallbacks {
+public:

Please separate with empty line.



Comment at: clang-tidy/altera/KernelNameRestrictionCheck.cpp:46
+};
+} // namespace
+

Please separate with empty line.



Comment at: docs/ReleaseNotes.rst:79
+
+Checks for cases where the kernel source file is named "kernel.cl",
+  "Verilog.cl", or "VHDL.cl".

Please fix indentation and use single back-ticks to highlight file names. Same 
in documentation.



Comment at: docs/clang-tidy/checks/altera-kernel-name-restriction.rst:13
+
+As per the "Guidelines for Naming the Kernel" section in the "Intel FPGA SDK 
+for OpenCL Pro Edition: Programming Guide."

May be link is better?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D72218



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


[PATCH] D72217: [clang-tidy] Added readability-qualified-auto check

2020-01-04 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment.

May be check belong to LLVM module?




Comment at: clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp:69
+  if (auto Var = Result.Nodes.getNodeAs("auto_ptr")) {
+
+if (!Var->getType().getTypePtr()->getPointeeType().isConstQualified()) {

Unnecessary empty line.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst:6
+
+`LLVM Coding Standards `_ advises 
to
+make it obvious if a auto typed variable is a pointer, constant pointer or 

Please make first statement same as Release Notes.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst:13
+.. code-block:: c++
+  for (auto  : MutatableContainer) {
+change(Data);

Please separate with empty line.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst:25
+  }
+
+

Unnecessary empty line.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst:30
+.. code-block:: c++
+  for (auto  : MutatableContainer) {
+change(Data);

Please separate with empty line.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst:43
+
+
+This check helps to enforce this `LLVM Coding Standards recommendation

Unnecessary empty line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72217



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


[PATCH] D72221: Support function attribute patchable_function_entry

2020-01-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 236225.
MaskRay added a comment.

clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72221

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/patchable-function-entry.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/patchable-function-entry-attr.c
  clang/test/Sema/patchable-function-entry-attr.cpp

Index: clang/test/Sema/patchable-function-entry-attr.cpp
===
--- /dev/null
+++ clang/test/Sema/patchable-function-entry-attr.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple aarch64 -fsyntax-only -verify=silence %s
+// RUN: %clang_cc1 -triple i386 -fsyntax-only -verify=silence %s
+// RUN: %clang_cc1 -triple x86_64 -fsyntax-only -verify=silence %s
+// RUN: %clang_cc1 -triple ppc64le -fsyntax-only -verify %s
+
+// silence-no-diagnostics
+
+// expected-error@+1 {{'patchable_function_entry' attribute is not supported for this target}}
+[[gnu::patchable_function_entry(0)]] void f();
Index: clang/test/Sema/patchable-function-entry-attr.c
===
--- /dev/null
+++ clang/test/Sema/patchable-function-entry-attr.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple aarch64 -fsyntax-only -verify %s
+
+// expected-error@+1 {{'patchable_function_entry' attribute takes at least 1 argument}}
+__attribute__((patchable_function_entry)) void f();
+
+// expected-error@+1 {{'patchable_function_entry' attribute takes no more than 2 arguments}}
+__attribute__((patchable_function_entry(0, 0, 0))) void f();
+
+// expected-error@+1 {{'patchable_function_entry' attribute requires a non-negative integral compile time constant expression}}
+__attribute__((patchable_function_entry(-1))) void f();
+
+int i;
+// expected-error@+1 {{'patchable_function_entry' attribute requires parameter 0 to be an integer constant}}
+__attribute__((patchable_function_entry(i))) void f();
+
+// expected-error@+1 {{'patchable_function_entry' attribute requires integer constant between 0 and 0 inclusive}}
+__attribute__((patchable_function_entry(1, 1))) void f();
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -128,6 +128,7 @@
 // CHECK-NEXT: Owner (SubjectMatchRule_record_not_is_union)
 // CHECK-NEXT: ParamTypestate (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: PassObjectSize (SubjectMatchRule_variable_is_parameter)
+// CHECK-NEXT: PatchableFunctionEntry (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: Pointer (SubjectMatchRule_record_not_is_union)
 // CHECK-NEXT: ReleaseHandle (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: RenderScriptKernel (SubjectMatchRule_function)
Index: clang/test/CodeGen/patchable-function-entry.c
===
--- /dev/null
+++ clang/test/CodeGen/patchable-function-entry.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple aarch64 -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: define void @f0() #0
+__attribute__((patchable_function_entry(0))) void f0() {}
+
+// CHECK: define void @f00() #0
+__attribute__((patchable_function_entry(0, 0))) void f00() {}
+
+// CHECK: define void @f2() #1
+__attribute__((patchable_function_entry(2))) void f2() {}
+
+// CHECK: define void @f20() #1
+__attribute__((patchable_function_entry(2, 0))) void f20() {}
+
+// CHECK: define void @f20decl() #1
+__attribute__((patchable_function_entry(2, 0))) void f20decl();
+__attribute__((noinline)) void f20decl() {}
+
+// CHECK: attributes #0 = { {{.*}} "patchable-function-entry"="0,0"
+// CHECK: attributes #1 = { {{.*}} "patchable-function-entry"="2,0"
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4917,6 +4917,34 @@
  XRayLogArgsAttr(S.Context, AL, ArgCount.getSourceIndex()));
 }
 
+static void handlePatchableFunctionEntryAttr(Sema , Decl *D,
+ const ParsedAttr ) {
+  const llvm::Triple  = S.Context.getTargetInfo().getTriple();
+  if (!T.isAArch64() && T.getArch() != llvm::Triple::x86 &&
+  T.getArch() != llvm::Triple::x86_64) {
+S.Diag(getAttrLoc(AL), diag::err_attribute_unsupported) << AL;
+return;
+  }
+
+  uint32_t Size = 0, Start = 0;
+  if (AL.getNumArgs()) {
+if (!checkUInt32Argument(S, AL, AL.getArgAsExpr(0), Size, 0, true))
+  return;
+if (AL.getNumArgs() == 2) {
+  Expr *Arg = AL.getArgAsExpr(1);
+  if 

[PATCH] D72222: [Driver][CodeGen] Add -fpatchable-function-entry=N[,M]

2020-01-04 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61253 tests passed, 0 failed 
and 736 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D7



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


[PATCH] D72221: Support function attribute patchable_function_entry

2020-01-04 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61253 tests passed, 0 failed 
and 736 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72221



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


[PATCH] D72222: [Driver][CodeGen] Add -fpatchable-function-entry=N[,M]

2020-01-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 236222.
MaskRay added a comment.

Forgot to add test/Driver/fpatchable-function-entry.c


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D7

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/XRayArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/patchable-function-entry.c
  clang/test/Driver/fpatchable-function-entry.c

Index: clang/test/Driver/fpatchable-function-entry.c
===
--- /dev/null
+++ clang/test/Driver/fpatchable-function-entry.c
@@ -0,0 +1,17 @@
+// RUN: %clang -target i386 %s -fpatchable-function-entry=1 -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64 %s -fpatchable-function-entry=1 -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target aarch64 %s -fpatchable-function-entry=1 -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target aarch64 %s -fpatchable-function-entry=1,0 -c -### 2>&1 | FileCheck %s
+// CHECK: "-fpatchable-function-entry=1"
+
+// RUN: not %clang -target ppc64 -fsyntax-only %s -fpatchable-function-entry=1 2>&1 | FileCheck --check-prefix=TARGET %s
+// TARGET: error: unsupported option '-fpatchable-function-entry=1' for target 'ppc64'
+
+// RUN: not %clang -target i386 -fsyntax-only %s -fpatchable-function-entry=1,1 2>&1 | FileCheck --check-prefix=NONZERO %s
+// NONZERO: error: the clang compiler does not support '1'
+
+// RUN: not %clang -target x86_64 -fsyntax-only %s -fpatchable-function-entry=1,0, 2>&1 | FileCheck --check-prefix=EXCESS %s
+// EXCESS: error: invalid argument '1,0,' to -fpatchable-function-entry=
+
+// RUN: not %clang -target aarch64-linux -fsyntax-only %s -fxray-instrument -fpatchable-function-entry=1 2>&1 | FileCheck --check-prefix=XRAY %s
+// XRAY: error: '-fxray-instrument' and '-fpatchable-function-entry=' cannot be used together
Index: clang/test/CodeGen/patchable-function-entry.c
===
--- clang/test/CodeGen/patchable-function-entry.c
+++ clang/test/CodeGen/patchable-function-entry.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple aarch64 -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64 -emit-llvm %s -fpatchable-function-entry=1 -o - | FileCheck --check-prefixes=CHECK,OPT %s
 
 // CHECK: define void @f0() #0
 __attribute__((patchable_function_entry(0))) void f0() {}
@@ -16,5 +17,9 @@
 __attribute__((patchable_function_entry(2,0))) void f20decl();
 __attribute__((noinline)) void f20decl() {}
 
+// OPT: define void @f() #2
+void f() {}
+
 // CHECK: attributes #0 = { {{.*}} "patchable-function-entry"="0,0"
 // CHECK: attributes #1 = { {{.*}} "patchable-function-entry"="2,0"
+// OPT:   attributes #2 = { {{.*}} "patchable-function-entry"="1,0"
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1101,6 +1101,8 @@
   parseXRayInstrumentationBundle("-fxray-instrumentation-bundle=", A, Args,
  Diags, Opts.XRayInstrumentationBundle);
 
+  Opts.PatchableFunctionEntrySize =
+  getLastArgIntValue(Args, OPT_fpatchable_function_entry, 0, Diags);
   Opts.InstrumentForProfiling = Args.hasArg(OPT_pg);
   Opts.CallFEntry = Args.hasArg(OPT_mfentry);
   Opts.MNopMCount = Args.hasArg(OPT_mnop_mcount);
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -70,6 +70,13 @@
   D.Diag(diag::err_drv_clang_unsupported)
   << (std::string(XRayInstrumentOption) + " on " + Triple.str());
 }
+
+// Both XRay and -fpatchable-function-entry use
+// TargetOpcode::PATCHABLE_FUNCTION_ENTER.
+if (Arg *A = Args.getLastArg(options::OPT_fpatchable_function_entry))
+  D.Diag(diag::err_drv_incompatible_options)
+  << "-fxray-instrument" << A->getSpelling();
+
 XRayInstrument = true;
 if (const Arg *A =
 Args.getLastArg(options::OPT_fxray_instruction_threshold_,
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4998,6 +4998,24 @@
   const XRayArgs  = TC.getXRayArgs();
   XRay.addArgs(TC, Args, CmdArgs, InputType);
 
+  if (Arg *A = Args.getLastArg(options::OPT_fpatchable_function_entry)) {
+StringRef S0 = A->getValue(), S = S0;
+unsigned Size, Start = 0;
+if (!Triple.isAArch64() && Triple.getArch() != llvm::Triple::x86 &&
+Triple.getArch() != llvm::Triple::x86_64)
+  

[PATCH] D72221: Support function attribute patchable_function_entry

2020-01-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 236221.
MaskRay added a comment.

Move a line from D7  to D72221 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72221

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/patchable-function-entry.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/patchable-function-entry-attr.c
  clang/test/Sema/patchable-function-entry-attr.cpp

Index: clang/test/Sema/patchable-function-entry-attr.cpp
===
--- /dev/null
+++ clang/test/Sema/patchable-function-entry-attr.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple aarch64 -fsyntax-only -verify=silence %s
+// RUN: %clang_cc1 -triple i386 -fsyntax-only -verify=silence %s
+// RUN: %clang_cc1 -triple x86_64 -fsyntax-only -verify=silence %s
+// RUN: %clang_cc1 -triple ppc64le -fsyntax-only -verify %s
+
+// silence-no-diagnostics
+
+// expected-error@+1 {{'patchable_function_entry' attribute is not supported for this target}}
+[[gnu::patchable_function_entry(0)]] void f();
Index: clang/test/Sema/patchable-function-entry-attr.c
===
--- /dev/null
+++ clang/test/Sema/patchable-function-entry-attr.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple aarch64 -fsyntax-only -verify %s
+
+// expected-error@+1 {{'patchable_function_entry' attribute takes at least 1 argument}}
+__attribute__((patchable_function_entry)) void f();
+
+// expected-error@+1 {{'patchable_function_entry' attribute takes no more than 2 arguments}}
+__attribute__((patchable_function_entry(0,0,0))) void f();
+
+// expected-error@+1 {{'patchable_function_entry' attribute requires a non-negative integral compile time constant expression}}
+__attribute__((patchable_function_entry(-1))) void f();
+
+int i;
+// expected-error@+1 {{'patchable_function_entry' attribute requires parameter 0 to be an integer constant}}
+__attribute__((patchable_function_entry(i))) void f();
+
+// expected-error@+1 {{'patchable_function_entry' attribute requires integer constant between 0 and 0 inclusive}}
+__attribute__((patchable_function_entry(1,1))) void f();
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -128,6 +128,7 @@
 // CHECK-NEXT: Owner (SubjectMatchRule_record_not_is_union)
 // CHECK-NEXT: ParamTypestate (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: PassObjectSize (SubjectMatchRule_variable_is_parameter)
+// CHECK-NEXT: PatchableFunctionEntry (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: Pointer (SubjectMatchRule_record_not_is_union)
 // CHECK-NEXT: ReleaseHandle (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: RenderScriptKernel (SubjectMatchRule_function)
Index: clang/test/CodeGen/patchable-function-entry.c
===
--- /dev/null
+++ clang/test/CodeGen/patchable-function-entry.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple aarch64 -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: define void @f0() #0
+__attribute__((patchable_function_entry(0))) void f0() {}
+
+// CHECK: define void @f00() #0
+__attribute__((patchable_function_entry(0,0))) void f00() {}
+
+// CHECK: define void @f2() #1
+__attribute__((patchable_function_entry(2))) void f2() {}
+
+// CHECK: define void @f20() #1
+__attribute__((patchable_function_entry(2,0))) void f20() {}
+
+// CHECK: define void @f20decl() #1
+__attribute__((patchable_function_entry(2,0))) void f20decl();
+__attribute__((noinline)) void f20decl() {}
+
+// CHECK: attributes #0 = { {{.*}} "patchable-function-entry"="0,0"
+// CHECK: attributes #1 = { {{.*}} "patchable-function-entry"="2,0"
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4917,6 +4917,34 @@
  XRayLogArgsAttr(S.Context, AL, ArgCount.getSourceIndex()));
 }
 
+static void handlePatchableFunctionEntryAttr(Sema , Decl *D,
+ const ParsedAttr ) {
+  const llvm::Triple  = S.Context.getTargetInfo().getTriple();
+  if (!T.isAArch64() && T.getArch() != llvm::Triple::x86 &&
+  T.getArch() != llvm::Triple::x86_64) {
+S.Diag(getAttrLoc(AL), diag::err_attribute_unsupported) << AL;
+return;
+  }
+
+  uint32_t Size = 0, Start = 0;
+  if (AL.getNumArgs()) {
+if (!checkUInt32Argument(S, AL, AL.getArgAsExpr(0), Size, 0, true))
+  return;
+ 

[PATCH] D72222: [Driver][CodeGen] Add -fpatchable-function-entry=N[,M]

2020-01-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision.
MaskRay added reviewers: dberris, kristof.beyls, nickdesaulniers, rnk, rsmith, 
void.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
MaskRay added a parent revision: D72221: Support function attribute 
patchable_function_entry.
MaskRay added subscribers: uweigand, jonpa.

In the backend, this feature is implemented with the function attribute
"patchable-function-entry". Both the attribute and XRay use
TargetOpcode::PATCHABLE_FUNCTION_ENTER, so the two features are
incompatible.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D7

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/XRayArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/patchable-function-entry.c
  clang/test/Sema/patchable-function-entry-attr.cpp

Index: clang/test/Sema/patchable-function-entry-attr.cpp
===
--- clang/test/Sema/patchable-function-entry-attr.cpp
+++ clang/test/Sema/patchable-function-entry-attr.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple aarch64 -fsyntax-only -verify=silence %s
+// RUN: %clang_cc1 -triple i386 -fsyntax-only -verify=silence %s
 // RUN: %clang_cc1 -triple x86_64 -fsyntax-only -verify=silence %s
 // RUN: %clang_cc1 -triple ppc64le -fsyntax-only -verify %s
 
Index: clang/test/CodeGen/patchable-function-entry.c
===
--- clang/test/CodeGen/patchable-function-entry.c
+++ clang/test/CodeGen/patchable-function-entry.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple aarch64 -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64 -emit-llvm %s -fpatchable-function-entry=1 -o - | FileCheck --check-prefixes=CHECK,OPT %s
 
 // CHECK: define void @f0() #0
 __attribute__((patchable_function_entry(0))) void f0() {}
@@ -16,5 +17,9 @@
 __attribute__((patchable_function_entry(2,0))) void f20decl();
 __attribute__((noinline)) void f20decl() {}
 
+// OPT: define void @f() #2
+void f() {}
+
 // CHECK: attributes #0 = { {{.*}} "patchable-function-entry"="0,0"
 // CHECK: attributes #1 = { {{.*}} "patchable-function-entry"="2,0"
+// OPT:   attributes #2 = { {{.*}} "patchable-function-entry"="1,0"
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1101,6 +1101,8 @@
   parseXRayInstrumentationBundle("-fxray-instrumentation-bundle=", A, Args,
  Diags, Opts.XRayInstrumentationBundle);
 
+  Opts.PatchableFunctionEntrySize =
+  getLastArgIntValue(Args, OPT_fpatchable_function_entry, 0, Diags);
   Opts.InstrumentForProfiling = Args.hasArg(OPT_pg);
   Opts.CallFEntry = Args.hasArg(OPT_mfentry);
   Opts.MNopMCount = Args.hasArg(OPT_mnop_mcount);
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -70,6 +70,13 @@
   D.Diag(diag::err_drv_clang_unsupported)
   << (std::string(XRayInstrumentOption) + " on " + Triple.str());
 }
+
+// Both XRay and -fpatchable-function-entry use
+// TargetOpcode::PATCHABLE_FUNCTION_ENTER.
+if (Arg *A = Args.getLastArg(options::OPT_fpatchable_function_entry))
+  D.Diag(diag::err_drv_incompatible_options)
+  << "-fxray-instrument" << A->getSpelling();
+
 XRayInstrument = true;
 if (const Arg *A =
 Args.getLastArg(options::OPT_fxray_instruction_threshold_,
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4998,6 +4998,24 @@
   const XRayArgs  = TC.getXRayArgs();
   XRay.addArgs(TC, Args, CmdArgs, InputType);
 
+  if (Arg *A = Args.getLastArg(options::OPT_fpatchable_function_entry)) {
+StringRef S0 = A->getValue(), S = S0;
+unsigned Size, Start = 0;
+if (!Triple.isAArch64() && Triple.getArch() != llvm::Triple::x86 &&
+Triple.getArch() != llvm::Triple::x86_64)
+  D.Diag(diag::err_drv_unsupported_opt_for_target)
+  << A->getAsString(Args) << TripleStr;
+else if (S.consumeInteger(10, Size) ||
+ (!S.empty() && (!S.consume_front(",") ||
+ S.consumeInteger(10, Start) || !S.empty(
+  D.Diag(diag::err_drv_invalid_argument_to_option)
+  << S0 << A->getOption().getName();
+else if (Start)
+  D.Diag(diag::err_drv_clang_unsupported) << Start;
+else
+  CmdArgs.push_back(Args.MakeArgString(A->getSpelling() + Twine(Size)));
+  }
+
   if (TC.SupportsProfiling()) {
 

[PATCH] D72221: Support function attribute patchable_function_entry

2020-01-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision.
MaskRay added reviewers: dberris, kristof.beyls, nickdesaulniers, rnk, rsmith, 
void.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
MaskRay added a parent revision: D72220: [X86] Support function attribute 
"patchable-function-entry".
MaskRay added a child revision: D7: [Driver][CodeGen] Add 
-fpatchable-function-entry=N[,M].

This feature is generic. Make it applicable for AArch64 and X86 because
the backend has only implemented NOP insertion for AArch64 and X86.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72221

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/patchable-function-entry.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/patchable-function-entry-attr.c
  clang/test/Sema/patchable-function-entry-attr.cpp

Index: clang/test/Sema/patchable-function-entry-attr.cpp
===
--- /dev/null
+++ clang/test/Sema/patchable-function-entry-attr.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple aarch64 -fsyntax-only -verify=silence %s
+// RUN: %clang_cc1 -triple x86_64 -fsyntax-only -verify=silence %s
+// RUN: %clang_cc1 -triple ppc64le -fsyntax-only -verify %s
+
+// silence-no-diagnostics
+
+// expected-error@+1 {{'patchable_function_entry' attribute is not supported for this target}}
+[[gnu::patchable_function_entry(0)]] void f();
Index: clang/test/Sema/patchable-function-entry-attr.c
===
--- /dev/null
+++ clang/test/Sema/patchable-function-entry-attr.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple aarch64 -fsyntax-only -verify %s
+
+// expected-error@+1 {{'patchable_function_entry' attribute takes at least 1 argument}}
+__attribute__((patchable_function_entry)) void f();
+
+// expected-error@+1 {{'patchable_function_entry' attribute takes no more than 2 arguments}}
+__attribute__((patchable_function_entry(0,0,0))) void f();
+
+// expected-error@+1 {{'patchable_function_entry' attribute requires a non-negative integral compile time constant expression}}
+__attribute__((patchable_function_entry(-1))) void f();
+
+int i;
+// expected-error@+1 {{'patchable_function_entry' attribute requires parameter 0 to be an integer constant}}
+__attribute__((patchable_function_entry(i))) void f();
+
+// expected-error@+1 {{'patchable_function_entry' attribute requires integer constant between 0 and 0 inclusive}}
+__attribute__((patchable_function_entry(1,1))) void f();
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -128,6 +128,7 @@
 // CHECK-NEXT: Owner (SubjectMatchRule_record_not_is_union)
 // CHECK-NEXT: ParamTypestate (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: PassObjectSize (SubjectMatchRule_variable_is_parameter)
+// CHECK-NEXT: PatchableFunctionEntry (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: Pointer (SubjectMatchRule_record_not_is_union)
 // CHECK-NEXT: ReleaseHandle (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: RenderScriptKernel (SubjectMatchRule_function)
Index: clang/test/CodeGen/patchable-function-entry.c
===
--- /dev/null
+++ clang/test/CodeGen/patchable-function-entry.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple aarch64 -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: define void @f0() #0
+__attribute__((patchable_function_entry(0))) void f0() {}
+
+// CHECK: define void @f00() #0
+__attribute__((patchable_function_entry(0,0))) void f00() {}
+
+// CHECK: define void @f2() #1
+__attribute__((patchable_function_entry(2))) void f2() {}
+
+// CHECK: define void @f20() #1
+__attribute__((patchable_function_entry(2,0))) void f20() {}
+
+// CHECK: define void @f20decl() #1
+__attribute__((patchable_function_entry(2,0))) void f20decl();
+__attribute__((noinline)) void f20decl() {}
+
+// CHECK: attributes #0 = { {{.*}} "patchable-function-entry"="0,0"
+// CHECK: attributes #1 = { {{.*}} "patchable-function-entry"="2,0"
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4917,6 +4917,34 @@
  XRayLogArgsAttr(S.Context, AL, ArgCount.getSourceIndex()));
 }
 
+static void handlePatchableFunctionEntryAttr(Sema , Decl *D,
+ const ParsedAttr ) {
+  const llvm::Triple  = S.Context.getTargetInfo().getTriple();
+  if (!T.isAArch64() && T.getArch() != llvm::Triple::x86 &&
+  T.getArch() != llvm::Triple::x86_64) {
+

[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

2020-01-04 Thread Logan Smith via Phabricator via cfe-commits
logan-5 updated this revision to Diff 236216.
logan-5 added a comment.

Change double-underscore fix-it finding algorithm to correctly collapse any 
number of >=2 underscores in a row, not just exactly 2 (or multiples of 2).


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

https://reviews.llvm.org/D72213

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/clang-tidy/utils/CMakeLists.txt
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-reserved-identifier.rst
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-reserved-identifier/system/system-header.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-reserved-identifier/user-header.h
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier-invert.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp
@@ -0,0 +1,183 @@
+// RUN: %check_clang_tidy %s bugprone-reserved-identifier %t -- -- \
+// RUN:   -I%S/Inputs/bugprone-reserved-identifier \
+// RUN:   -isystem %S/Inputs/bugprone-reserved-identifier/system
+
+// no warnings expected without -header-filter=
+#include "user-header.h"
+#include 
+
+#define _MACRO(m) int m = 0
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration uses reserved identifier '_MACRO', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}#define MACRO(m) int m = 0{{$}}
+
+namespace _Ns {
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: declaration uses reserved identifier '_Ns', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}namespace Ns {{{$}}
+
+class _Object {
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '_Object', which causes undefined behavior [bugprone-reserved-identifier]
+  // CHECK-FIXES: {{^}}class Object {{{$}}
+  int _Member;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '_Member', which causes undefined behavior [bugprone-reserved-identifier]
+  // CHECK-FIXES: {{^}}  int Member;{{$}}
+};
+
+float _Global;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '_Global', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}float Global;{{$}}
+
+void _Function() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration uses reserved identifier '_Function', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}void Function() {}{{$}}
+
+using _Alias = int;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '_Alias', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}using Alias = int;{{$}}
+
+} // namespace _Ns
+
+//
+
+#define __macro(m) int m = 0
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration uses reserved identifier '__macro', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}#define macro(m) int m = 0{{$}}
+
+namespace __ns {
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: declaration uses reserved identifier '__ns', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}namespace ns {{{$}}
+class __object {
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '__object', which causes undefined behavior [bugprone-reserved-identifier]
+  // CHECK-FIXES: {{^}}class object {{{$}}
+  int __member;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '__member', which causes undefined behavior [bugprone-reserved-identifier]
+  // CHECK-FIXES: {{^}}  int member;{{$}}
+};
+
+float __global;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '__global', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}float global;{{$}}
+
+void __function() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration uses reserved identifier '__function', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}void function() {}{{$}}
+
+using __alias = int;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '__alias', which causes 

[PATCH] D72218: [clang-tidy] new altera kernel name restriction check

2020-01-04 Thread Frank Derry Wanye via Phabricator via cfe-commits
ffrankies created this revision.
ffrankies added reviewers: alexfh, hokein, aaron.ballman.
ffrankies added projects: clang-tools-extra, clang, LLVM.
Herald added subscribers: mgehre, ormris, arphaman, xazax.hun, Anastasia, 
mgorny.

This lint check is part of the FLOCL (FPGA Linters for OpenCL) project out of 
the Synergy Lab at Virginia Tech.

FLOCL is a set of lint checks aimed at FPGA developers who code in OpenCL.

The altera kernel name restriction check finds kernel files and include 
directives whose filename is "kernel.cl", "Verilog.cl", or "VHDL.cl". Such 
kernel file names cause the Altera Offline Compiler to generate intermediate 
design files that have the same names as certain internal files, which leads to 
a compilation error.

As per the "Guidelines for Naming the Kernel" section in the "Intel FPGA SDK 
for OpenCL Pro Edition: Programming Guide."


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D72218

Files:
  clang-tidy/CMakeLists.txt
  clang-tidy/ClangTidyForceLinker.h
  clang-tidy/altera/AlteraTidyModule.cpp
  clang-tidy/altera/CMakeLists.txt
  clang-tidy/altera/KernelNameRestrictionCheck.cpp
  clang-tidy/altera/KernelNameRestrictionCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/altera-kernel-name-restriction.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/index.rst
  test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/KERNEL.cl
  test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/VHDL.cl
  test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/Verilog.cl
  test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/kernel.cl
  test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/kernel.h
  
test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/other_Verilog.cl
  
test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/otherdir/vhdl.cl
  test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/otherthing.cl
  
test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/dir/kernel.cl
  test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some_kernel.cl
  
test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/somedir/verilog.cl
  test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/thing.h
  test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vERILOG.cl
  test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/verilog.h
  test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl.CL
  test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl.h
  
test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl_number_two.cl
  test/clang-tidy/checkers/altera-kernel-name-restriction.cpp

Index: test/clang-tidy/checkers/altera-kernel-name-restriction.cpp
===
--- /dev/null
+++ test/clang-tidy/checkers/altera-kernel-name-restriction.cpp
@@ -0,0 +1,42 @@
+// RUN: %check_clang_tidy %s altera-kernel-name-restriction %t -- -- -I%S/Inputs/altera-kernel-name-restriction
+
+// These are the banned kernel filenames, and should trigger warnings
+#include "kernel.cl"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: The imported kernel source file is named 'kernel.cl','Verilog.cl', or 'VHDL.cl', which could cause compilation errors. [altera-kernel-name-restriction]
+#include "Verilog.cl"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: The imported kernel source file is named 'kernel.cl','Verilog.cl', or 'VHDL.cl', which could cause compilation errors. [altera-kernel-name-restriction]
+#include "VHDL.cl"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: The imported kernel source file is named 'kernel.cl','Verilog.cl', or 'VHDL.cl', which could cause compilation errors. [altera-kernel-name-restriction]
+
+// The warning should be triggered regardless of capitalization
+#include "KERNEL.cl"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: The imported kernel source file is named 'kernel.cl','Verilog.cl', or 'VHDL.cl', which could cause compilation errors. [altera-kernel-name-restriction]
+#include "vERILOG.cl"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: The imported kernel source file is named 'kernel.cl','Verilog.cl', or 'VHDL.cl', which could cause compilation errors. [altera-kernel-name-restriction]
+#include "vhdl.CL"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: The imported kernel source file is named 'kernel.cl','Verilog.cl', or 'VHDL.cl', which could cause compilation errors. [altera-kernel-name-restriction]
+
+// The warning should be triggered if the names are within a directory
+#include "some/dir/kernel.cl"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: The imported kernel source file is named 'kernel.cl','Verilog.cl', or 'VHDL.cl', which could cause compilation errors. [altera-kernel-name-restriction]
+#include "somedir/verilog.cl"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: The imported kernel source file is named 'kernel.cl','Verilog.cl', or 'VHDL.cl', which could cause 

[PATCH] D72217: [clang-tidy] Added readability-qualified-auto check

2020-01-04 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision.
njames93 added a reviewer: aaron.ballman.
njames93 added projects: clang, clang-tools-extra.
Herald added subscribers: xazax.hun, whisperity, mgorny.
njames93 added a comment.

side note when creating this, the add_new_check.py file hasn't been updated in 
relation to this commit 
https://github.com/llvm/llvm-project/commit/d2c9c9157b0528436d3b9f5e22c872f0ee6509a2.
 This results in a malformed rst file. In this patch I have just manually set 
it up correctly, but a fix will need to be made for the add_new_check.py


Adds a check that detects any auto variables that are deduced to a pointer or a 
const pointer then adds in the const and asterisk according. Will also check 
auto L value references that could be written as const. This relates to the 
coding standard 
https://llvm.org/docs/CodingStandards.html#beware-unnecessary-copies-with-auto


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72217

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp
  clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.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-qualified-auto.rst
  clang-tools-extra/test/clang-tidy/checkers/readability-qualified-auto.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-qualified-auto.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-qualified-auto.cpp
@@ -0,0 +1,100 @@
+// RUN: %check_clang_tidy %s readability-qualified-auto %t -- -- -std=c++17
+
+int getInt();
+int *getIntPtr();
+const int *getCIntPtr();
+
+void foo() {
+  // make sure check disregards named types
+  int TypedInt = getInt();
+  int *TypedPtr = getIntPtr();
+  const int *TypedConstPtr = getCIntPtr();
+  int  = *getIntPtr();
+  const int  = *getCIntPtr();
+
+  // make sure check disregards auto types that aren't pointers or references
+  auto AutoInt = getInt();
+
+  auto NakedPtr = getIntPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto variable' can be declared as an 'auto pointer'
+  // CHECK-FIXES: {{^}}  auto * NakedPtr = getIntPtr();
+  auto NakedCPtr = getCIntPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto variable' can be declared as a 'const auto pointer'
+  // CHECK-FIXES: {{^}}  const auto * NakedCPtr = getCIntPtr();
+
+  const auto ConstPtr = getIntPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto variable' can be declared as an 'auto pointer'
+  // CHECK-FIXES: {{^}}  auto *const  ConstPtr = getIntPtr();
+  const auto ConstCPtr = getCIntPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto variable' can be declared as a 'const auto pointer'
+  // CHECK-FIXES: {{^}}  const auto *const  ConstCPtr = getCIntPtr();
+
+  auto *QualPtr = getIntPtr();
+  auto *QualCPtr = getCIntPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto pointer' can be declared as a 'const auto pointer'
+  // CHECK-FIXES: {{^}}  const auto *QualCPtr = getCIntPtr();
+  const auto *ConstQualCPtr = getCIntPtr();
+
+  auto  = *getIntPtr();
+  auto  = *getCIntPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto reference' can be declared as a 'const auto reference'
+  // CHECK-FIXES: {{^}}  const auto  = *getCIntPtr();
+  const auto  = *getCIntPtr();
+
+  if (auto X = getCIntPtr()) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'auto variable' can be declared as a 'const auto pointer'
+// CHECK-FIXES: {{^}}  if (const auto * X = getCIntPtr()) {
+  }
+  if (auto X = getIntPtr(); X != nullptr) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'auto variable' can be declared as an 'auto pointer'
+// CHECK-FIXES: {{^}}  if (auto * X = getIntPtr(); X != nullptr) {
+  }
+}
+
+namespace std {
+template 
+class vector { // dummy impl
+  T _data[1];
+
+public:
+  T *begin() { return _data; }
+  const T *begin() const { return _data; }
+  T *end() { return &_data[1]; }
+  const T *end() const { return &_data[1]; }
+};
+} // namespace std
+
+void change(int &);
+void observe(const int &);
+
+void loopRef(std::vector , const std::vector ) {
+  for (auto  : Mutate) {
+change(Data);
+  }
+  for (auto  : Constant) {
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'auto reference' can be declared as a 'const auto reference'
+// CHECK-FIXES: {{^}}  for (const auto  : Constant) {
+observe(Data);
+  }
+}
+
+void loopPtr(const std::vector , const std::vector ) {
+  for (auto Data : Mutate) {
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'auto variable' can be declared as an 'auto pointer'
+// CHECK-FIXES: {{^}}  for (auto * Data : Mutate) {
+change(*Data);
+  }
+  for (auto Data : Constant) {
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'auto variable' can be 

[PATCH] D72217: [clang-tidy] Added readability-qualified-auto check

2020-01-04 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

side note when creating this, the add_new_check.py file hasn't been updated in 
relation to this commit 
https://github.com/llvm/llvm-project/commit/d2c9c9157b0528436d3b9f5e22c872f0ee6509a2.
 This results in a malformed rst file. In this patch I have just manually set 
it up correctly, but a fix will need to be made for the add_new_check.py


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72217



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


[PATCH] D67224: [clangd] Enable completions with fixes in VSCode

2020-01-04 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment.

Just throwing a wild idea out there: what if we used 
`textDocument/onTypeFormatting` to replace the `.` with `->` as soon as it's 
typed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67224



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


[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

2020-01-04 Thread Logan Smith via Phabricator via cfe-commits
logan-5 marked an inline comment as done.
logan-5 added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h:59
+  struct FailureInfo {
+std::string KindName; // Tag or misc info to be used as derived classes 
need
+std::string Fixup;// The name that will be proposed as a fix-it hint

JonasToth wrote:
> I think the `kind` should be an enum instead of stringly typing.
I can't disagree that the string is a little gross, but since the 
interpretation of the string is for use by the derived classes, I don't think 
an enum is flexible enough.


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

https://reviews.llvm.org/D72213



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


[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

2020-01-04 Thread Logan Smith via Phabricator via cfe-commits
logan-5 updated this revision to Diff 236210.
logan-5 marked 39 inline comments as done.
logan-5 added a comment.

Addressed formatting issues, and tweaked handling of the names `__` and `::_`: 
the check now flags these but doesn't suggest a fix.


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

https://reviews.llvm.org/D72213

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/clang-tidy/utils/CMakeLists.txt
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-reserved-identifier.rst
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-reserved-identifier/system/system-header.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-reserved-identifier/user-header.h
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier-invert.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp
@@ -0,0 +1,179 @@
+// RUN: %check_clang_tidy %s bugprone-reserved-identifier %t -- -- \
+// RUN:   -I%S/Inputs/bugprone-reserved-identifier \
+// RUN:   -isystem %S/Inputs/bugprone-reserved-identifier/system
+
+// no warnings expected without -header-filter=
+#include "user-header.h"
+#include 
+
+#define _MACRO(m) int m = 0
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration uses reserved identifier '_MACRO', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}#define MACRO(m) int m = 0{{$}}
+
+namespace _Ns {
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: declaration uses reserved identifier '_Ns', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}namespace Ns {{{$}}
+
+class _Object {
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '_Object', which causes undefined behavior [bugprone-reserved-identifier]
+  // CHECK-FIXES: {{^}}class Object {{{$}}
+  int _Member;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '_Member', which causes undefined behavior [bugprone-reserved-identifier]
+  // CHECK-FIXES: {{^}}  int Member;{{$}}
+};
+
+float _Global;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '_Global', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}float Global;{{$}}
+
+void _Function() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration uses reserved identifier '_Function', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}void Function() {}{{$}}
+
+using _Alias = int;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '_Alias', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}using Alias = int;{{$}}
+
+} // namespace _Ns
+
+//
+
+#define __macro(m) int m = 0
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration uses reserved identifier '__macro', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}#define macro(m) int m = 0{{$}}
+
+namespace __ns {
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: declaration uses reserved identifier '__ns', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}namespace ns {{{$}}
+class __object {
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '__object', which causes undefined behavior [bugprone-reserved-identifier]
+  // CHECK-FIXES: {{^}}class object {{{$}}
+  int __member;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '__member', which causes undefined behavior [bugprone-reserved-identifier]
+  // CHECK-FIXES: {{^}}  int member;{{$}}
+};
+
+float __global;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '__global', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}float global;{{$}}
+
+void __function() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration uses reserved identifier '__function', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}void function() {}{{$}}
+
+using __alias = int;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier 

[PATCH] D72212: [Sema] Improve -Wrange-loop-analysis warnings

2020-01-04 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:2806
-  // constructors.
-  if (VariableType.isPODType(SemaRef.Context))
 return;

Mordante wrote:
> xbolva00 wrote:
> > assert it?
> Sorry I don't understand what you mean. What do you want to assert?
if (isTriviallyCopyableType) {
 assert(isPOD);
}

But probably not very useful to have an assert here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72212



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


[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

2020-01-04 Thread Logan Smith via Phabricator via cfe-commits
logan-5 marked an inline comment as done.
logan-5 added a comment.

@Eugene.Zelenko never mind about pushing back on the nits; I had misread some 
of them. They all sound good, will address.




Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:70
+  const bool IsInGlobalNamespace) {
+  return IsInGlobalNamespace && Name.size() > 1 && Name[0] == '_';
+}

Mordante wrote:
> `Name.size() > 1` doesn't catch `_` in the global namespace. Catching `_` 
> will probably also cause issues with the automatic renaming.
> What about renaming `__`?
Good point. Perhaps the check should catch these but simply not propose a fix.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp:68
+
+} // namespace __ns
+

Mordante wrote:
> Should it not suggest to change this to `namespace ns` ?
That's a fair point. I'll point out (again) that this is an existing case 
missed by the logic factored out of readability-identifier-naming, so it might 
be better suited for a separate patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72213



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


[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

2020-01-04 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

i think it would be easier to review if there are two patches, one refactoring 
and one new check.




Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h:59
+  struct FailureInfo {
+std::string KindName; // Tag or misc info to be used as derived classes 
need
+std::string Fixup;// The name that will be proposed as a fix-it hint

I think the `kind` should be an enum instead of stringly typing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72213



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


[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

2020-01-04 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment.

In D72213#1804447 , @Mordante wrote:

> @eugene `Please don't use auto when type is spelled in same statement or 
> iterator.` do you mean `type is not spelled` ?


Yes, you are correct. Sorry for mistake.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72213



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


[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

2020-01-04 Thread Logan Smith via Phabricator via cfe-commits
logan-5 added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier-invert.cpp:21
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: declaration uses identifier 
'Helper', which is not a reserved identifier [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}struct __Helper {};{{$}}
+struct _helper2 {};

Mordante wrote:
> Why suggesting `__Helper` instead of  `_Helper` ?
I mostly didn't see a reason to make the check opinionated on //how// to uglify 
names, though I suppose now that you mention it, a smaller change to the name 
when possible is arguably better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72213



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


[PATCH] D56644: [clang-tidy] readability-container-size-empty handle std::string length()

2020-01-04 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.
Herald added a subscriber: mgehre.

any news here? do you plan to finish this patch @Quolyk ?
Do you mind if someone comandeers this patch?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56644



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


[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-04 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth marked an inline comment as done.
JonasToth added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp:608
+}
+
+template 

0x8000- wrote:
> JonasToth wrote:
> > 0x8000- wrote:
> > > 0x8000- wrote:
> > > > JonasToth wrote:
> > > > > JonasToth wrote:
> > > > > > 0x8000- wrote:
> > > > > > > Please insert the this test code here:
> > > > > > > 
> > > > > > > ```
> > > > > > > struct IntWrapper {
> > > > > > > 
> > > > > > >unsigned low;
> > > > > > >unsigned high;
> > > > > > > 
> > > > > > >IntWrapper& operator=(unsigned value) {
> > > > > > >   low = value & 0x;
> > > > > > >   high = (value >> 16) & 0x;
> > > > > > >}
> > > > > > > 
> > > > > > >template
> > > > > > >friend Istream& operator>>(Istream& is, IntWrapper& rhs) {
> > > > > > >   unsigned someValue = 0;   // false positive now
> > > > > > >   if (is >> someValue) {
> > > > > > >  rhs = someValue;
> > > > > > >   }
> > > > > > >   return is;
> > > > > > >}
> > > > > > > };
> > > > > > > 
> > > > > > > unsigned TestHiddenFriend(IntMaker& im) {
> > > > > > >IntWrapper iw;
> > > > > > > 
> > > > > > >im >> iw;
> > > > > > > 
> > > > > > >return iw.low;
> > > > > > > }
> > > > > > > ```
> > > > > > clang gives me no error when I add the const there. sure it does 
> > > > > > reproduce properly?
> > > > > Here for reference: https://godbolt.org/z/DXKMYh
> > > > Probably I wasn't clear - I suggested adding my test code at line 608, 
> > > > because it needs the "IntMaker" definition at line 594.
> > > > 
> > > > The false positive from this const check is reported on the "unsigned 
> > > > someValue = 0;" line inside the template extraction operator.
> > > Oh, I got it - don't think "shift" think "overloaded extraction operator".
> > > 
> > > In my code above, you don't know what ">>" does, and it clearly takes a 
> > > mutable reference as the right side.
> > > 
> > > ```
> > > const int foo;
> > > std::cin >> foo;
> > > ```
> > > 
> > > should not compile, either :)
> > no. something else is going on.
> > https://godbolt.org/z/xm8eVC
> > ```
> > | |   |-CXXOperatorCallExpr  ''
> > | |   | |-UnresolvedLookupExpr  '' lvalue 
> > (ADL) = 'operator>>' 0x55a26b885938 0x55a26b857238
> > | |   | |-DeclRefExpr  'Istream' lvalue ParmVar 0x55a26b885748 'is' 
> > 'Istream &'
> > | |   | `-DeclRefExpr  'const unsigned int' lvalue Var 
> > 0x55a26b885a38 'someValue' 'const unsigned int'
> > ```
> > This code is only a false positive in the sense, that the "we can not know 
> > if something bad happens" is not detected. The operator>> is not resolved. 
> > That is because the template is not instantiated and the expressions can 
> > therefore not be resolved yet.
> > There seems to be no instantiation of this template function.
> > 
> > Somehow the `im >> iw;` does not instantiate the `friend operator<<`. I 
> > reduced it to something i think is minimal and that shows the same 
> > behaviour. (https://godbolt.org/z/MMG_4q)
> https://godbolt.org/z/7QEdHJ
> 
> ```
> struct IntMaker {
>   operator bool() const;
> 
>   friend IntMaker >>(IntMaker &, unsigned &);
>   //friend IntMaker >>(IntMaker &, const unsigned &) = delete;
> };
> ```
> 
> If you uncomment the deleted overload then
> 
> ```
> template 
> Istream& operator>>(Istream& is, IntWrapper& rhs)  {
> unsigned const someValue = 0;
> if (is >> someValue) {
> rhs = someValue;
> }
> return is;
> }
> ```
> 
> Fails to compile.
> 
> Depending on what else is around, it seems that somehow the compiler would 
> try to use the (bool) conversion to obtain an integral then use it as an 
> argument to the built-in integral left shift.
> 
> https://godbolt.org/z/-JFL5o - this does not compile, as expected:
> 
> ```
> #include 
> 
> int readInt() {
> const int foo = 0;
> std::cin >> foo;
> return foo;
> }
> ```
> 
> so this check should not recommend making foo constant.
I see. Implicit conversions are great... :)

I will recheck that. And the `std::cin` example is analyzed correctly. I added 
a test for that, too.
Thank you for researching the issue!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943



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


[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

2020-01-04 Thread Logan Smith via Phabricator via cfe-commits
logan-5 added a comment.

@Eugene.Zelenko +1 to @Mordante 's question. Otherwise, happy to address most 
of those nits, though I think I will gently push back on a couple of them (in 
the inline comments).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72213



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


[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

2020-01-04 Thread Logan Smith via Phabricator via cfe-commits
logan-5 added a comment.

In D72213#1804394 , @Mordante wrote:

> I wonder what happens if the project already contains a suggested fix, for 
> example:
>
>   #define __FOO(X) ...
>   #define _FOO(X) __FOO(X)
>   #define FOO(X) _FOO(X)
>
>
> will it suggest:
>
>   #define FOO(X) ...
>   #define FOO(X) FOO(X)
>   #define FOO(X) FOO(X)
>
>
> ?


Yes, it will. This issue was already present in the logic I factored out of 
readability-identifier-naming, so I chose not to address it for this PR. A 
future patch should add logic to ensure that the renaming checks don't 
recommend correcting multiple identifiers to the same thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72213



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


[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

2020-01-04 Thread Mark de Wever via Phabricator via cfe-commits
Mordante added a subscriber: eugene.
Mordante added a comment.

@eugene `Please don't use auto when type is spelled in same statement or 
iterator.` do you mean `type is not spelled` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72213



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


[PATCH] D72053: [RFC] Handling implementation limits

2020-01-04 Thread Mark de Wever via Phabricator via cfe-commits
Mordante marked 2 inline comments as done.
Mordante added inline comments.



Comment at: clang/docs/ImplementationQuantities.rst:70
+
+These limits are defined in the standards, but are imposed in Clang.
+

craig.topper wrote:
> should this say ‘not defined’?
Yes, thanks for catching it!


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

https://reviews.llvm.org/D72053



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


[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

2020-01-04 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:49
+}
+static Optional getDoubleUnderscoreFixup(StringRef Name) {
+  if (hasDoubleUnderscore(Name)) {

Please separate with empty line.



Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:60
+}
+static Optional getUnderscoreCapitalFixup(const StringRef Name) {
+  if (startsWithUnderscoreCapital(Name)) {

Please separate with empty line.



Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:72
+}
+static Optional
+getUnderscoreGlobalNamespaceFixup(const StringRef Name,

Please separate with empty line.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:114
+};
+if (auto Fixup = getDoubleUnderscoreFixup(inProgressFixup())) {
+  appendFailure("du", *std::move(Fixup));

Please don't use auto when type is spelled in same statement or iterator.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:117
+}
+if (auto Fixup = getUnderscoreCapitalFixup(inProgressFixup())) {
+  appendFailure("uc", *std::move(Fixup));

Please don't use auto when type is spelled in same statement or iterator.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:120
+}
+if (auto Fixup = getUnderscoreGlobalNamespaceFixup(inProgressFixup(),
+   IsInGlobalNamespace)) {

Please don't use auto when type is spelled in same statement or iterator.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:146
+}
+Optional
+ReservedIdentifierCheck::GetMacroFailureInfo(const Token ,

Please separate with empty line.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:152
+}
+RenamerClangTidyCheck::DiagInfo
+ReservedIdentifierCheck::GetDiagInfo(const NamingCheckId ,

Please separate with empty line.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:168
+};
+} // namespace bugprone
+

Please separate with empty line.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:169
+} // namespace bugprone
+
+} // namespace tidy

Unnecessary empty line.



Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h:13
+#include "../utils/RenamerClangTidyCheck.h"
+
+namespace clang {

Please include vector and llvm/Optional.h.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:645
 }
-
-void IdentifierNamingCheck::checkMacro(SourceManager ,
-   const Token ,
-   const MacroInfo *MI) {
+llvm::Optional
+IdentifierNamingCheck::GetMacroFailureInfo(const Token ,

Please separate with empty line.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:672
 }
-
-void IdentifierNamingCheck::expandMacro(const Token ,
-const MacroInfo *MI) {
-  StringRef Name = MacroNameTok.getIdentifierInfo()->getName();
-  NamingCheckId ID(MI->getDefinitionLoc(), Name);
-
-  auto Failure = NamingCheckFailures.find(ID);
-  if (Failure == NamingCheckFailures.end())
-return;
-
-  SourceRange Range(MacroNameTok.getLocation(), MacroNameTok.getEndLoc());
-  addUsage(NamingCheckFailures, ID, Range);
-}
-
-void IdentifierNamingCheck::onEndOfTranslationUnit() {
-  for (const auto  : NamingCheckFailures) {
-const NamingCheckId  = Pair.first;
-const NamingCheckFailure  = Pair.second;
-
-if (Failure.KindName.empty())
-  continue;
-
-if (Failure.ShouldNotify()) {
-  auto Diag =
-  diag(Decl.first,
-   "invalid case style for %0 '%1'%select{|" // Case 0 is empty on
- // purpose, because we
- // intent to provide a
- // fix
-   "; cannot be fixed because '%3' would conflict with a keyword|"
-   "; cannot be fixed because '%3' would conflict with a macro "
-   "definition}2")
-  << Failure.KindName << Decl.second
-  << static_cast(Failure.FixStatus) << Failure.Fixup;
-
-  if (Failure.ShouldFix()) {
-for (const auto  : Failure.RawUsageLocs) {
-  // We assume that the identifier name is made of one token only. This
-  // is always the case as we ignore usages in macros that could 

[PATCH] D72053: [RFC] Handling implementation limits

2020-01-04 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added inline comments.



Comment at: clang/docs/ImplementationQuantities.rst:70
+
+These limits are defined in the standards, but are imposed in Clang.
+

should this say ‘not defined’?


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

https://reviews.llvm.org/D72053



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


[PATCH] D72212: [Sema] Improve -Wrange-loop-analysis warnings

2020-01-04 Thread Mark de Wever via Phabricator via cfe-commits
Mordante marked an inline comment as done.
Mordante added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:2806
-  // constructors.
-  if (VariableType.isPODType(SemaRef.Context))
 return;

xbolva00 wrote:
> assert it?
Sorry I don't understand what you mean. What do you want to assert?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72212



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


[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

2020-01-04 Thread Mark de Wever via Phabricator via cfe-commits
Mordante added a comment.

I like this change, but I don't feel qualified to fully review the patch.

I wonder what happens if the project already contains a suggested fix, for 
example:

  #define __FOO(X) ...
  #define _FOO(X) __FOO(X)
  #define FOO(X) _FOO(X)

will it suggest:

  #define FOO(X) ...
  #define FOO(X) FOO(X)
  #define FOO(X) FOO(X)

?




Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:70
+  const bool IsInGlobalNamespace) {
+  return IsInGlobalNamespace && Name.size() > 1 && Name[0] == '_';
+}

`Name.size() > 1` doesn't catch `_` in the global namespace. Catching `_` will 
probably also cause issues with the automatic renaming.
What about renaming `__`?



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier-invert.cpp:21
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: declaration uses identifier 
'Helper', which is not a reserved identifier [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}struct __Helper {};{{$}}
+struct _helper2 {};

Why suggesting `__Helper` instead of  `_Helper` ?



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier-invert.cpp:56
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: declaration uses identifier 'Up', 
which is not a reserved identifier [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}template {{$}}
+inline reference_wrapper

Likewise why not `_Up` ?



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp:68
+
+} // namespace __ns
+

Should it not suggest to change this to `namespace ns` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72213



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


[PATCH] D72212: [Sema] Improve -Wrange-loop-analysis warnings

2020-01-04 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:2806
-  // constructors.
-  if (VariableType.isPODType(SemaRef.Context))
 return;

assert it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72212



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


[PATCH] D72212: [Sema] Improve -Wrange-loop-analysis warnings

2020-01-04 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61248 tests passed, 0 failed 
and 736 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72212



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


[PATCH] D72212: [Sema] Improve -Wrange-loop-analysis warnings

2020-01-04 Thread Mark de Wever via Phabricator via cfe-commits
Mordante created this revision.
Mordante added reviewers: aaron.ballman, xbolva00, MaskRay.
Mordante added a project: clang.

No longer generate a diagnostic when a small trivially copyable type is used 
without a reference. Before the test looked for a POD type and had no size 
restriction. Since the range-based for loop is only available in C++11 and POD 
types are trivially copyable in C++11 it's not required to test for a POD type.

Since copying a large object will be expensive its size has been restricted. 64 
bytes is a common size of a cache line and if the object is aligned the copy 
will be cheap. No performance impact testing has been done.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72212

Files:
  clang/lib/Sema/SemaStmt.cpp
  clang/test/SemaCXX/warn-range-loop-analysis-trivially-copyable.cpp
  clang/test/SemaCXX/warn-range-loop-analysis.cpp

Index: clang/test/SemaCXX/warn-range-loop-analysis.cpp
===
--- clang/test/SemaCXX/warn-range-loop-analysis.cpp
+++ clang/test/SemaCXX/warn-range-loop-analysis.cpp
@@ -19,6 +19,8 @@
 
 struct Foo {};
 struct Bar {
+  // The type is too large to suppress the warning for trivially copyable types.
+  char s[128];
   Bar(Foo);
   Bar(int);
   operator int();
Index: clang/test/SemaCXX/warn-range-loop-analysis-trivially-copyable.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-range-loop-analysis-trivially-copyable.cpp
@@ -0,0 +1,125 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wloop-analysis -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wrange-loop-analysis -verify %s
+
+void test_POD() {
+  struct Record {
+volatile int a;
+int b;
+  };
+
+  Record records[8];
+  for (const auto r : records)
+(void)r;
+}
+
+void test_POD_64_bytes() {
+  struct Record {
+char a[64];
+  };
+
+  Record records[8];
+  for (const auto r : records)
+(void)r;
+}
+
+void test_POD_65_bytes() {
+  struct Record {
+char a[65];
+  };
+
+  // expected-warning@+3 {{loop variable 'r' of type 'const Record' creates a copy from type 'const Record'}}
+  // expected-note@+2 {{use reference type 'const Record &' to prevent copying}}
+  Record records[8];
+  for (const auto r : records)
+(void)r;
+}
+
+void test_TriviallyCopyable() {
+  struct Record {
+Record() {}
+volatile int a;
+int b;
+  };
+
+  Record records[8];
+  for (const auto r : records)
+(void)r;
+}
+
+void test_TriviallyCopyable_64_bytes() {
+  struct Record {
+Record() {}
+char a[64];
+  };
+
+  Record records[8];
+  for (const auto r : records)
+(void)r;
+}
+
+void test_TriviallyCopyable_65_bytes() {
+  struct Record {
+Record() {}
+char a[65];
+  };
+
+  // expected-warning@+3 {{loop variable 'r' of type 'const Record' creates a copy from type 'const Record'}}
+  // expected-note@+2 {{use reference type 'const Record &' to prevent copying}}
+  Record records[8];
+  for (const auto r : records)
+(void)r;
+}
+
+void test_NonTriviallyCopyable() {
+  struct Record {
+Record() {}
+~Record() {}
+volatile int a;
+int b;
+  };
+
+  // expected-warning@+3 {{loop variable 'r' of type 'const Record' creates a copy from type 'const Record'}}
+  // expected-note@+2 {{use reference type 'const Record &' to prevent copying}}
+  Record records[8];
+  for (const auto r : records)
+(void)r;
+}
+
+void test_TrivialABI() {
+  struct [[clang::trivial_abi]] Record {
+Record() {}
+~Record() {}
+volatile int a;
+int b;
+  };
+
+  Record records[8];
+  for (const auto r : records)
+(void)r;
+}
+
+void test_TrivialABI_64_bytes() {
+  struct [[clang::trivial_abi]] Record {
+Record() {}
+~Record() {}
+char a[64];
+  };
+
+  Record records[8];
+  for (const auto r : records)
+(void)r;
+}
+
+void test_TrivialABI_65_bytes() {
+  struct [[clang::trivial_abi]] Record {
+Record() {}
+~Record() {}
+char a[65];
+  };
+
+  // expected-warning@+3 {{loop variable 'r' of type 'const Record' creates a copy from type 'const Record'}}
+  // expected-note@+2 {{use reference type 'const Record &' to prevent copying}}
+  Record records[8];
+  for (const auto r : records)
+(void)r;
+}
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -2779,6 +2779,15 @@
   }
 }
 
+/// Determines whether the @p VariableType's declaration is a record with the
+/// clang::trivial_abi attribute.
+static bool hasTrivialABIAttr(QualType VariableType) {
+  if (CXXRecordDecl *RD = VariableType->getAsCXXRecordDecl())
+return RD->hasAttr();
+
+  return false;
+}
+
 // Warns when the loop variable can be changed to a reference type to
 // prevent a copy.  For instance, if given "for (const Foo x : Range)" suggest
 // "for (const Foo  : Range)" if this form does not make a copy.

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-04 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp:608
+}
+
+template 

JonasToth wrote:
> 0x8000- wrote:
> > 0x8000- wrote:
> > > JonasToth wrote:
> > > > JonasToth wrote:
> > > > > 0x8000- wrote:
> > > > > > Please insert the this test code here:
> > > > > > 
> > > > > > ```
> > > > > > struct IntWrapper {
> > > > > > 
> > > > > >unsigned low;
> > > > > >unsigned high;
> > > > > > 
> > > > > >IntWrapper& operator=(unsigned value) {
> > > > > >   low = value & 0x;
> > > > > >   high = (value >> 16) & 0x;
> > > > > >}
> > > > > > 
> > > > > >template
> > > > > >friend Istream& operator>>(Istream& is, IntWrapper& rhs) {
> > > > > >   unsigned someValue = 0;   // false positive now
> > > > > >   if (is >> someValue) {
> > > > > >  rhs = someValue;
> > > > > >   }
> > > > > >   return is;
> > > > > >}
> > > > > > };
> > > > > > 
> > > > > > unsigned TestHiddenFriend(IntMaker& im) {
> > > > > >IntWrapper iw;
> > > > > > 
> > > > > >im >> iw;
> > > > > > 
> > > > > >return iw.low;
> > > > > > }
> > > > > > ```
> > > > > clang gives me no error when I add the const there. sure it does 
> > > > > reproduce properly?
> > > > Here for reference: https://godbolt.org/z/DXKMYh
> > > Probably I wasn't clear - I suggested adding my test code at line 608, 
> > > because it needs the "IntMaker" definition at line 594.
> > > 
> > > The false positive from this const check is reported on the "unsigned 
> > > someValue = 0;" line inside the template extraction operator.
> > Oh, I got it - don't think "shift" think "overloaded extraction operator".
> > 
> > In my code above, you don't know what ">>" does, and it clearly takes a 
> > mutable reference as the right side.
> > 
> > ```
> > const int foo;
> > std::cin >> foo;
> > ```
> > 
> > should not compile, either :)
> no. something else is going on.
> https://godbolt.org/z/xm8eVC
> ```
> | |   |-CXXOperatorCallExpr  ''
> | |   | |-UnresolvedLookupExpr  '' lvalue 
> (ADL) = 'operator>>' 0x55a26b885938 0x55a26b857238
> | |   | |-DeclRefExpr  'Istream' lvalue ParmVar 0x55a26b885748 'is' 
> 'Istream &'
> | |   | `-DeclRefExpr  'const unsigned int' lvalue Var 0x55a26b885a38 
> 'someValue' 'const unsigned int'
> ```
> This code is only a false positive in the sense, that the "we can not know if 
> something bad happens" is not detected. The operator>> is not resolved. That 
> is because the template is not instantiated and the expressions can therefore 
> not be resolved yet.
> There seems to be no instantiation of this template function.
> 
> Somehow the `im >> iw;` does not instantiate the `friend operator<<`. I 
> reduced it to something i think is minimal and that shows the same behaviour. 
> (https://godbolt.org/z/MMG_4q)
https://godbolt.org/z/7QEdHJ

```
struct IntMaker {
  operator bool() const;

  friend IntMaker >>(IntMaker &, unsigned &);
  //friend IntMaker >>(IntMaker &, const unsigned &) = delete;
};
```

If you uncomment the deleted overload then

```
template 
Istream& operator>>(Istream& is, IntWrapper& rhs)  {
unsigned const someValue = 0;
if (is >> someValue) {
rhs = someValue;
}
return is;
}
```

Fails to compile.

Depending on what else is around, it seems that somehow the compiler would try 
to use the (bool) conversion to obtain an integral then use it as an argument 
to the built-in integral left shift.

https://godbolt.org/z/-JFL5o - this does not compile, as expected:

```
#include 

int readInt() {
const int foo = 0;
std::cin >> foo;
return foo;
}
```

so this check should not recommend making foo constant.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943



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


[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-04 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment.

In D54943#1804291 , @JonasToth wrote:

> In D54943#1804183 , @0x8000- 
> wrote:
>
> > This patch does not build on top of current tree:
> >
> >   /usr/bin/ld: 
> > lib/libclangTidyCppCoreGuidelinesModule.a(ConstCorrectnessCheck.cpp.o): in 
> > function 
> > `clang::tidy::cppcoreguidelines::ConstCorrectnessCheck::registerMatchers(clang::ast_matchers::MatchFinder*)':
> >   
> > /home/florin/tools/llvm-project/clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp:65:
> >  undefined reference to `clang::ast_matchers::decompositionDecl'
> >   clang: error: linker command failed with exit code 1 (use -v to see 
> > invocation)
> >   [2771/4997] Building CXX object 
> > tools/clang/tools/extra/clang-include-fixer/find-all-symbols/CMakeFiles/obj.findAllSymbols.dir/PragmaCommentHandler.cpp.o
> >   ninja: build stopped: subcommand failed.
> >
> >
> > This is my top of the tree right now:
> >
> >   b7ecf1c1c373c53183ef6ef66efbe4237ff7b96d (origin/master, origin/HEAD, 
> > master) NFC: Fix trivial typos in comments
> >
>
>
> did the changes to `clang/include/clang/ASTMatchers/ASTMatchers.h` and 
> `clang/lib/ASTMatchers/Dynamic/Registry.cpp` apply? They provide the 
> `decompositionDecl` matcher. It seems odd, they are not available :/


This is the result of applying the patch as e-mailed:

  ~/tools/llvm-project$ patch -p0 < /tmp/D54943.236101.patch


  patching file clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp   


  patching file clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp


  patching file clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp


  patching file clang/lib/Analysis/ExprMutationAnalyzer.cpp 


  patching file clang/lib/ASTMatchers/Dynamic/Registry.cpp  


  patching file clang/include/clang/ASTMatchers/ASTMatchers.h   


  patching file 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp
   
  patching file 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-values.cpp
 
  patching file 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-pointer-as-values.cpp
  
  patching file 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-pointer-as-values.cpp

  patching file 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-cxx17.cpp

  patching file 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-const-correctness.rst

  patching file clang-tools-extra/docs/ReleaseNotes.rst 


  patching file 
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp  

  patching file 
clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.h  

  patching file 
clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp

  patching file clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt   


  patching file clang-tools-extra/CMakeLists.txt

No rejection that I can see.

  ~/tools/llvm-project$ ag decompositionDecl
  clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp
  67: unless(has(decompositionDecl()

[PATCH] D71857: [NFC] Fixes -Wrange-loop-analysis warnings

2020-01-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

In D71857#1804307 , @Mordante wrote:

> In D71857#1800663 , @MaskRay wrote:
>
> > I think there is a false positive.
> >
> > https://github.com/llvm/llvm-project/tree/master/lld/ELF/Relocations.cpp#L1622
> >
> >   for (const std::pair ts : isd->thunkSections)
> >   
>
>
> Removing the `const` like L1648 will also solve the issue.


Yes, both `std::pair` and `auto` work, but `const std::pair` doesn't. std::pair 
has an unfortunate user-defined copy assignment operator 
(https://eel.is/c++draft/pairs#pair-16 "Assigns p.first to first and p.second 
to second." Does this wording mandate a user-defined function?) I will drop 
`const` for this lld file.

For other user-defined types. I think loosening the diagnostic to allow 
trivially copyable types will be nice. Types that take only a few words (say. 
1~3) are likely faster with a copy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71857



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


[PATCH] D71857: [NFC] Fixes -Wrange-loop-analysis warnings

2020-01-04 Thread Mark de Wever via Phabricator via cfe-commits
Mordante added a comment.

In D71857#1800663 , @MaskRay wrote:

> I think there is a false positive.
>
> https://github.com/llvm/llvm-project/tree/master/lld/ELF/Relocations.cpp#L1622
>
>   for (const std::pair ts : isd->thunkSections)
>   


Removing the `const` like L1648 will also solve the issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71857



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


[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-04 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth marked an inline comment as done.
JonasToth added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp:608
+}
+
+template 

0x8000- wrote:
> 0x8000- wrote:
> > JonasToth wrote:
> > > JonasToth wrote:
> > > > 0x8000- wrote:
> > > > > Please insert the this test code here:
> > > > > 
> > > > > ```
> > > > > struct IntWrapper {
> > > > > 
> > > > >unsigned low;
> > > > >unsigned high;
> > > > > 
> > > > >IntWrapper& operator=(unsigned value) {
> > > > >   low = value & 0x;
> > > > >   high = (value >> 16) & 0x;
> > > > >}
> > > > > 
> > > > >template
> > > > >friend Istream& operator>>(Istream& is, IntWrapper& rhs) {
> > > > >   unsigned someValue = 0;   // false positive now
> > > > >   if (is >> someValue) {
> > > > >  rhs = someValue;
> > > > >   }
> > > > >   return is;
> > > > >}
> > > > > };
> > > > > 
> > > > > unsigned TestHiddenFriend(IntMaker& im) {
> > > > >IntWrapper iw;
> > > > > 
> > > > >im >> iw;
> > > > > 
> > > > >return iw.low;
> > > > > }
> > > > > ```
> > > > clang gives me no error when I add the const there. sure it does 
> > > > reproduce properly?
> > > Here for reference: https://godbolt.org/z/DXKMYh
> > Probably I wasn't clear - I suggested adding my test code at line 608, 
> > because it needs the "IntMaker" definition at line 594.
> > 
> > The false positive from this const check is reported on the "unsigned 
> > someValue = 0;" line inside the template extraction operator.
> Oh, I got it - don't think "shift" think "overloaded extraction operator".
> 
> In my code above, you don't know what ">>" does, and it clearly takes a 
> mutable reference as the right side.
> 
> ```
> const int foo;
> std::cin >> foo;
> ```
> 
> should not compile, either :)
no. something else is going on.
https://godbolt.org/z/xm8eVC
```
| |   |-CXXOperatorCallExpr  ''
| |   | |-UnresolvedLookupExpr  '' lvalue 
(ADL) = 'operator>>' 0x55a26b885938 0x55a26b857238
| |   | |-DeclRefExpr  'Istream' lvalue ParmVar 0x55a26b885748 'is' 
'Istream &'
| |   | `-DeclRefExpr  'const unsigned int' lvalue Var 0x55a26b885a38 
'someValue' 'const unsigned int'
```
This code is only a false positive in the sense, that the "we can not know if 
something bad happens" is not detected. The operator>> is not resolved. That is 
because the template is not instantiated and the expressions can therefore not 
be resolved yet.
There seems to be no instantiation of this template function.

Somehow the `im >> iw;` does not instantiate the `friend operator<<`. I reduced 
it to something i think is minimal and that shows the same behaviour. 
(https://godbolt.org/z/MMG_4q)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943



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


[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

2020-01-04 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment.

Is building `check-all` immediately after configuration expected to be clean? I 
have a build configured to build LLVM with just `clang` and just with the 
`PowerPC` target, and it seems `check-all` (under such a configuration) does 
not trigger the building of the library added in this patch; which, in turn, 
leads to test failures.

  /src/llvm/test/tools/llvm-config/booleans.test:27:24: error: 
CHECK-SHARED-MODE-NOT: excluded string found in input
  CHECK-SHARED-MODE-NOT: error:
 ^
  :3:14: note: found here
  llvm-config: error: missing: /build/lib/libLLVMCFGuard.a
   ^~

I have confirmed that the case I mentioned fails with rGd157a9b 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65761



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


[PATCH] D72053: [RFC] Handling implementation limits

2020-01-04 Thread Mark de Wever via Phabricator via cfe-commits
Mordante updated this revision to Diff 236190.
Mordante added a comment.

Added support for some additional features:

- Allow limits from the C standard, either sharing the C++ constants or 
separately.
- Allow to document the design choices for the limit.
- Allow to track the status of the limit. (This may become obsolete ones all 
limits are implemented, but that can take a while.)

Added some additional fields to test the C support.

Removed the .inc file from the patch, this should not be committed.


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

https://reviews.llvm.org/D72053

Files:
  clang/docs/ImplementationQuantities.rst
  clang/docs/UsersManual.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/CMakeLists.txt
  clang/include/clang/Basic/ImplementationQuantities.h
  clang/include/clang/Basic/ImplementationQuantities.td
  clang/lib/CodeGen/CGRecordLayout.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/utils/TableGen/CMakeLists.txt
  clang/utils/TableGen/ClangImplementationQuantitiesEmitter.cpp
  clang/utils/TableGen/TableGen.cpp
  clang/utils/TableGen/TableGenBackends.h

Index: clang/utils/TableGen/TableGenBackends.h
===
--- clang/utils/TableGen/TableGenBackends.h
+++ clang/utils/TableGen/TableGenBackends.h
@@ -81,8 +81,14 @@
  llvm::raw_ostream );
 void EmitClangCommentCommandList(llvm::RecordKeeper ,
  llvm::raw_ostream );
+
 void EmitClangOpcodes(llvm::RecordKeeper , llvm::raw_ostream );
 
+void EmitClangImplementationQuantitiesDocs(llvm::RecordKeeper ,
+   llvm::raw_ostream );
+void EmitClangImplementationQuantities(llvm::RecordKeeper ,
+   llvm::raw_ostream );
+
 void EmitNeon(llvm::RecordKeeper , llvm::raw_ostream );
 void EmitFP16(llvm::RecordKeeper , llvm::raw_ostream );
 void EmitNeonSema(llvm::RecordKeeper , llvm::raw_ostream );
Index: clang/utils/TableGen/TableGen.cpp
===
--- clang/utils/TableGen/TableGen.cpp
+++ clang/utils/TableGen/TableGen.cpp
@@ -60,6 +60,8 @@
   GenClangCommentHTMLNamedCharacterReferences,
   GenClangCommentCommandInfo,
   GenClangCommentCommandList,
+  GenClangImplementationQuantitiesDocs,
+  GenClangImplementationQuantities,
   GenClangOpenCLBuiltins,
   GenArmNeon,
   GenArmFP16,
@@ -172,6 +174,14 @@
 clEnumValN(GenClangCommentCommandList, "gen-clang-comment-command-list",
"Generate list of commands that are used in "
"documentation comments"),
+clEnumValN(GenClangImplementationQuantitiesDocs,
+   "gen-clang-implementation-quantities-docs",
+   "Generate documentation of the implementation quantities "
+   "as defined in the C++ standard"),
+clEnumValN(GenClangImplementationQuantities,
+   "gen-clang-implementation-quantities",
+   "Generate of the implementation quantities "
+   "as defined in the C++ standard"),
 clEnumValN(GenClangOpenCLBuiltins, "gen-clang-opencl-builtins",
"Generate OpenCL builtin declaration handlers"),
 clEnumValN(GenArmNeon, "gen-arm-neon", "Generate arm_neon.h for clang"),
@@ -321,6 +331,12 @@
   case GenClangCommentCommandList:
 EmitClangCommentCommandList(Records, OS);
 break;
+  case GenClangImplementationQuantitiesDocs:
+EmitClangImplementationQuantitiesDocs(Records, OS);
+break;
+  case GenClangImplementationQuantities:
+EmitClangImplementationQuantities(Records, OS);
+break;
   case GenClangOpenCLBuiltins:
 EmitClangOpenCLBuiltins(Records, OS);
 break;
Index: clang/utils/TableGen/ClangImplementationQuantitiesEmitter.cpp
===
--- /dev/null
+++ clang/utils/TableGen/ClangImplementationQuantitiesEmitter.cpp
@@ -0,0 +1,131 @@
+//===--- ClangCommentCommandInfoEmitter.cpp - Generate command lists -//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "llvm/TableGen/Error.h"
+#include "llvm/TableGen/Record.h"
+#include "llvm/TableGen/StringMatcher.h"
+#include "llvm/TableGen/TableGenBackend.h"
+#include 
+
+using namespace llvm;
+
+namespace clang {
+
+void EmitClangImplementationQuantitiesDocs(RecordKeeper ,
+   raw_ostream ) {
+  // Get the documentation introduction paragraph.
+  const Record *Documentation = Records.getDef("GlobalDocumentation");
+  if (!Documentation) {
+llvm::PrintFatalError("The Documentation 

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-04 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

In D54943#1804183 , @0x8000- wrote:

> This patch does not build on top of current tree:
>
>   /usr/bin/ld: 
> lib/libclangTidyCppCoreGuidelinesModule.a(ConstCorrectnessCheck.cpp.o): in 
> function 
> `clang::tidy::cppcoreguidelines::ConstCorrectnessCheck::registerMatchers(clang::ast_matchers::MatchFinder*)':
>   
> /home/florin/tools/llvm-project/clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp:65:
>  undefined reference to `clang::ast_matchers::decompositionDecl'
>   clang: error: linker command failed with exit code 1 (use -v to see 
> invocation)
>   [2771/4997] Building CXX object 
> tools/clang/tools/extra/clang-include-fixer/find-all-symbols/CMakeFiles/obj.findAllSymbols.dir/PragmaCommentHandler.cpp.o
>   ninja: build stopped: subcommand failed.
>
>
> This is my top of the tree right now:
>
>   b7ecf1c1c373c53183ef6ef66efbe4237ff7b96d (origin/master, origin/HEAD, 
> master) NFC: Fix trivial typos in comments
>


did the changes to `clang/include/clang/ASTMatchers/ASTMatchers.h` and 
`clang/lib/ASTMatchers/Dynamic/Registry.cpp` apply? They provide the 
`decompositionDecl` matcher. It seems odd, they are not available :/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943



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


[PATCH] D71512: [clang-format] Fix short block when braking after control statement

2020-01-04 Thread Pablo Martin-Gomez via Phabricator via cfe-commits
Bouska updated this revision to Diff 236188.
Bouska added a comment.

Updated unittest to check under the column limit and over the column limit.


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

https://reviews.llvm.org/D71512

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -582,8 +582,10 @@
   verifyFormat("if CONSTEXPR (true) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("while (true) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("for (;;) { f(); }", AllowSimpleBracedStatements);
+  verifyFormat("if (true) { fff(); }",
+   AllowSimpleBracedStatements);
   verifyFormat("if (true) {\n"
-   "  ff();\n"
+   "  ();\n"
"}",
AllowSimpleBracedStatements);
   verifyFormat("if (true) { //\n"
@@ -657,9 +659,11 @@
   verifyFormat("if CONSTEXPR (true) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("while (true) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("for (;;) { f(); }", AllowSimpleBracedStatements);
+  verifyFormat("if (true) { fff(); }",
+   AllowSimpleBracedStatements);
   verifyFormat("if (true)\n"
"{\n"
-   "  ff();\n"
+   "  ();\n"
"}",
AllowSimpleBracedStatements);
   verifyFormat("if (true)\n"
@@ -721,7 +725,9 @@
   Style.BreakBeforeBraces = FormatStyle::BS_Allman;
   EXPECT_EQ("#define A  \\\n"
 "  if (HANDLEwernufrnuLwrmviferuvnierv) \\\n"
-"  { RET_ERR1_ANUIREUINERUIFNIOAerwfwrvnuier; }\n"
+"  {\\\n"
+"RET_ERR1_ANUIREUINERUIFNIOAerwfwrvnuier;   \\\n"
+"  }\n"
 "X;",
 format("#define A \\\n"
"   if (HANDLEwernufrnuLwrmviferuvnierv) { \\\n"
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -327,21 +327,6 @@
  ? tryMergeSimpleBlock(I, E, Limit)
  : 0;
 }
-// Try to merge either empty or one-line block if is precedeed by control
-// statement token
-if (TheLine->First->is(tok::l_brace) && TheLine->First == TheLine->Last &&
-I != AnnotatedLines.begin() &&
-I[-1]->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for)) {
-  unsigned MergedLines = 0;
-  if (Style.AllowShortBlocksOnASingleLine != FormatStyle::SBS_Never) {
-MergedLines = tryMergeSimpleBlock(I - 1, E, Limit);
-// If we managed to merge the block, discard the first merged line
-// since we are merging starting from I.
-if (MergedLines > 0)
-  --MergedLines;
-  }
-  return MergedLines;
-}
 // Don't merge block with left brace wrapped after ObjC special blocks
 if (TheLine->First->is(tok::l_brace) && I != AnnotatedLines.begin() &&
 I[-1]->First->is(tok::at) && I[-1]->First->Next) {


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -582,8 +582,10 @@
   verifyFormat("if CONSTEXPR (true) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("while (true) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("for (;;) { f(); }", AllowSimpleBracedStatements);
+  verifyFormat("if (true) { fff(); }",
+   AllowSimpleBracedStatements);
   verifyFormat("if (true) {\n"
-   "  ff();\n"
+   "  ();\n"
"}",
AllowSimpleBracedStatements);
   verifyFormat("if (true) { //\n"
@@ -657,9 +659,11 @@
   verifyFormat("if CONSTEXPR (true) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("while (true) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("for (;;) { f(); }", AllowSimpleBracedStatements);
+  verifyFormat("if (true) { fff(); }",
+   AllowSimpleBracedStatements);
   verifyFormat("if (true)\n"
"{\n"
-   "  ff();\n"
+   "  ();\n"
"}",
AllowSimpleBracedStatements);
   

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

2020-01-04 Thread Florian Hahn via Phabricator via cfe-commits
fhahn added a comment.

There seems to be a problem with this patch on macOS. I've XFAIL'd it in 
rGdb82fc5dd80ff14798e7f1c35dd7e593f6409ba3 
. This 
should unblock the macOS builders, but please take a look at the problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446



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


[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

2020-01-04 Thread Tyker via Phabricator via cfe-commits
Tyker added a comment.

In D70638#1804138 , @aaron.ballman 
wrote:

> In D70638#1803364 , @xbolva00 wrote:
>
> > (re-ping; I think this false positive for goto label case is important to 
> > be fixed before 10 release)
>
>
> I agree -- @Tyker, are you planning to work on that fix?


i made a patch. https://reviews.llvm.org/D72202


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70638



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


[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-04 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment.

This patch does not build on top of current tree:

  /usr/bin/ld: 
lib/libclangTidyCppCoreGuidelinesModule.a(ConstCorrectnessCheck.cpp.o): in 
function 
`clang::tidy::cppcoreguidelines::ConstCorrectnessCheck::registerMatchers(clang::ast_matchers::MatchFinder*)':
  
/home/florin/tools/llvm-project/clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp:65:
 undefined reference to `clang::ast_matchers::decompositionDecl'
  clang: error: linker command failed with exit code 1 (use -v to see 
invocation)
  [2771/4997] Building CXX object 
tools/clang/tools/extra/clang-include-fixer/find-all-symbols/CMakeFiles/obj.findAllSymbols.dir/PragmaCommentHandler.cpp.o
  ninja: build stopped: subcommand failed.

This is my top of the tree right now:

  b7ecf1c1c373c53183ef6ef66efbe4237ff7b96d (origin/master, origin/HEAD, master) 
NFC: Fix trivial typos in comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943



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


[PATCH] D71659: [clang-format] Added new option to allow setting spaces before and after the operator

2020-01-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay requested changes to this revision.
MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3359
+  // nuanced as aggressive line breaks are placed when the lambda is not the
+  // last arg.
   if ((Style.Language == FormatStyle::LK_Cpp ||

these are all unrelated changes please remove them from patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71659



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


[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-04 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp:608
+}
+
+template 

0x8000- wrote:
> JonasToth wrote:
> > JonasToth wrote:
> > > 0x8000- wrote:
> > > > Please insert the this test code here:
> > > > 
> > > > ```
> > > > struct IntWrapper {
> > > > 
> > > >unsigned low;
> > > >unsigned high;
> > > > 
> > > >IntWrapper& operator=(unsigned value) {
> > > >   low = value & 0x;
> > > >   high = (value >> 16) & 0x;
> > > >}
> > > > 
> > > >template
> > > >friend Istream& operator>>(Istream& is, IntWrapper& rhs) {
> > > >   unsigned someValue = 0;   // false positive now
> > > >   if (is >> someValue) {
> > > >  rhs = someValue;
> > > >   }
> > > >   return is;
> > > >}
> > > > };
> > > > 
> > > > unsigned TestHiddenFriend(IntMaker& im) {
> > > >IntWrapper iw;
> > > > 
> > > >im >> iw;
> > > > 
> > > >return iw.low;
> > > > }
> > > > ```
> > > clang gives me no error when I add the const there. sure it does 
> > > reproduce properly?
> > Here for reference: https://godbolt.org/z/DXKMYh
> Probably I wasn't clear - I suggested adding my test code at line 608, 
> because it needs the "IntMaker" definition at line 594.
> 
> The false positive from this const check is reported on the "unsigned 
> someValue = 0;" line inside the template extraction operator.
Oh, I got it - don't think "shift" think "overloaded extraction operator".

In my code above, you don't know what ">>" does, and it clearly takes a mutable 
reference as the right side.

```
const int foo;
std::cin >> foo;
```

should not compile, either :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943



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


[PATCH] D72140: [clang-tools-extra] NFC: Fix trivial typos in comments

2020-01-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision.
aaron.ballman added a comment.

Thank you for the patch! I've committed on your behalf in 
b7ecf1c1c373c53183ef6ef66efbe4237ff7b96d 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72140



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


[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-04 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth marked 2 inline comments as done.
JonasToth added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp:608
+}
+
+template 

JonasToth wrote:
> 0x8000- wrote:
> > Please insert the this test code here:
> > 
> > ```
> > struct IntWrapper {
> > 
> >unsigned low;
> >unsigned high;
> > 
> >IntWrapper& operator=(unsigned value) {
> >   low = value & 0x;
> >   high = (value >> 16) & 0x;
> >}
> > 
> >template
> >friend Istream& operator>>(Istream& is, IntWrapper& rhs) {
> >   unsigned someValue = 0;   // false positive now
> >   if (is >> someValue) {
> >  rhs = someValue;
> >   }
> >   return is;
> >}
> > };
> > 
> > unsigned TestHiddenFriend(IntMaker& im) {
> >IntWrapper iw;
> > 
> >im >> iw;
> > 
> >return iw.low;
> > }
> > ```
> clang gives me no error when I add the const there. sure it does reproduce 
> properly?
Here for reference: https://godbolt.org/z/DXKMYh


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943



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


[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-04 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp:608
+}
+
+template 

JonasToth wrote:
> JonasToth wrote:
> > 0x8000- wrote:
> > > Please insert the this test code here:
> > > 
> > > ```
> > > struct IntWrapper {
> > > 
> > >unsigned low;
> > >unsigned high;
> > > 
> > >IntWrapper& operator=(unsigned value) {
> > >   low = value & 0x;
> > >   high = (value >> 16) & 0x;
> > >}
> > > 
> > >template
> > >friend Istream& operator>>(Istream& is, IntWrapper& rhs) {
> > >   unsigned someValue = 0;   // false positive now
> > >   if (is >> someValue) {
> > >  rhs = someValue;
> > >   }
> > >   return is;
> > >}
> > > };
> > > 
> > > unsigned TestHiddenFriend(IntMaker& im) {
> > >IntWrapper iw;
> > > 
> > >im >> iw;
> > > 
> > >return iw.low;
> > > }
> > > ```
> > clang gives me no error when I add the const there. sure it does reproduce 
> > properly?
> Here for reference: https://godbolt.org/z/DXKMYh
Probably I wasn't clear - I suggested adding my test code at line 608, because 
it needs the "IntMaker" definition at line 594.

The false positive from this const check is reported on the "unsigned someValue 
= 0;" line inside the template extraction operator.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943



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


[clang-tools-extra] b7ecf1c - NFC: Fix trivial typos in comments

2020-01-04 Thread Aaron Ballman via cfe-commits

Author: Kazuaki Ishizaki
Date: 2020-01-04T10:28:41-05:00
New Revision: b7ecf1c1c373c53183ef6ef66efbe4237ff7b96d

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

LOG: NFC: Fix trivial typos in comments

Added: 


Modified: 
clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
clang-tools-extra/clang-doc/BitcodeReader.h
clang-tools-extra/clang-doc/Representation.h
clang-tools-extra/clang-doc/Serialize.cpp
clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
clang-tools-extra/clang-include-fixer/IncludeFixerContext.cpp
clang-tools-extra/clang-include-fixer/tool/clang-include-fixer.el
clang-tools-extra/clang-move/tool/ClangMove.cpp
clang-tools-extra/clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp
clang-tools-extra/clang-tidy/abseil/TimeSubtractionCheck.cpp
clang-tools-extra/clang-tidy/bugprone/ForwardDeclarationNamespaceCheck.cpp
clang-tools-extra/clang-tidy/bugprone/StringLiteralWithEmbeddedNulCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.cpp
clang-tools-extra/clang-tidy/google/IntegerTypesCheck.h
clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp
clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp
clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
clang-tools-extra/clang-tidy/performance/InefficientAlgorithmCheck.h
clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h

clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h
clang-tools-extra/clang-tidy/utils/NamespaceAliaser.cpp
clang-tools-extra/clangd/AST.h
clang-tools-extra/clangd/ClangdLSPServer.h
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/Context.h
clang-tools-extra/clangd/FindTarget.cpp
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/IncludeFixer.cpp
clang-tools-extra/clangd/ParsedAST.h
clang-tools-extra/clangd/Protocol.h
clang-tools-extra/clangd/SemanticHighlighting.h
clang-tools-extra/clangd/TUScheduler.h
clang-tools-extra/clangd/Trace.h
clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
clang-tools-extra/clangd/index/CanonicalIncludes.cpp
clang-tools-extra/clangd/index/Symbol.h
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp
clang-tools-extra/clangd/unittests/RenameTests.cpp
clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
clang-tools-extra/clangd/unittests/SyncAPI.cpp
clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
clang-tools-extra/clangd/unittests/TweakTests.cpp
clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp
clang-tools-extra/docs/clang-tidy/checks/bugprone-branch-clone.rst
clang-tools-extra/docs/clang-tidy/checks/cert-mem57-cpp.rst
clang-tools-extra/docs/clang-tidy/checks/hicpp-undelegated-constructor.rst
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
clang-tools-extra/modularize/Modularize.cpp
clang-tools-extra/modularize/PreprocessorTracker.cpp
clang-tools-extra/pp-trace/PPCallbacksTracker.cpp

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-no-malloc-custom.cpp
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-no-malloc.cpp

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-owning-memory-containers.cpp
clang-tools-extra/test/clang-tidy/checkers/modernize-make-unique.cpp
clang-tools-extra/test/clang-tidy/checkers/modernize-use-nullptr-basic.cpp

clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-value-param-arc.m

clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-value-param-arc.mm

clang-tools-extra/test/clang-tidy/checkers/readability-redundant-declaration.cpp

clang-tools-extra/unittests/clang-include-fixer/find-all-symbols/FindAllSymbolsTests.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp 
b/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
index 8d98ba01d1c7..33ece7f1b4e0 100644
--- 

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

2020-01-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D70638#1803364 , @xbolva00 wrote:

> (re-ping; I think this false positive for goto label case is important to be 
> fixed before 10 release)


I agree -- @Tyker, are you planning to work on that fix?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70638



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


[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-04 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth marked 2 inline comments as done.
JonasToth added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp:608
+}
+
+template 

0x8000- wrote:
> Please insert the this test code here:
> 
> ```
> struct IntWrapper {
> 
>unsigned low;
>unsigned high;
> 
>IntWrapper& operator=(unsigned value) {
>   low = value & 0x;
>   high = (value >> 16) & 0x;
>}
> 
>template
>friend Istream& operator>>(Istream& is, IntWrapper& rhs) {
>   unsigned someValue = 0;   // false positive now
>   if (is >> someValue) {
>  rhs = someValue;
>   }
>   return is;
>}
> };
> 
> unsigned TestHiddenFriend(IntMaker& im) {
>IntWrapper iw;
> 
>im >> iw;
> 
>return iw.low;
> }
> ```
clang gives me no error when I add the const there. sure it does reproduce 
properly?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943



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


[PATCH] D71174: [clang-tidy] new check: bugprone-signed-char-misuse

2020-01-04 Thread Tamás Zolnai via Phabricator via cfe-commits
ztamas added a comment.

Thanks for the review!
I applied for the renewal of my commit access. I had commit access only for the 
SVN repo. After I get access to the GitHub repo I'll push this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71174



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


[PATCH] D63960: [C++20] Add consteval-specific semantic for functions

2020-01-04 Thread Tyker via Phabricator via cfe-commits
Tyker added a comment.

ping @rsmith


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

https://reviews.llvm.org/D63960



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


[PATCH] D71903: [Coroutines][6/6] Clang schedules new passes

2020-01-04 Thread JunMa via Phabricator via cfe-commits
junparser added a comment.

In D71903#1804016 , @modocache wrote:

> I'm currently working on ensuring that CGSCC optimizations are rerun to 
> optimize coroutine funclets -- the primary feedback I received on this and on 
> D71899  -- but I just realized I didn't 
> respond to one comment on this set of reviews, from @junparser:
>
> > There is another issue we should consider: clang is crashed when compile 
> > coroutine with -disable-llvm-passes and output an object file.
>
> It's always been the case, since the coroutine intrinsics and passes were 
> first added to LLVM, that attempting to codegen without first running 
> coroutine passes would cause a crash during instruction selection. So `clang 
> -Xclang -disable-llvm-passes -c` has always crashed Clang during LLVM ISel, 
> as it does in this example that uses Clang 9.0.0 and the legacy pass manager: 
> https://godbolt.org/z/Mj2R5G
>
> Personally I'm of the opinion that this is less than ideal... I may be wrong, 
> but I don't think there are very many other C++ features that *require* Clang 
> to run LLVM passes (perhaps the `always_inline` attribute requires LLVM 
> passes to be run for correctness? I'm not sure). So I would like to see this 
> eventually addressed somehow.
>
> > Is it reasonable to run coroutine passes even -disable-llvm-passes is 
> > enabled?
>
> My personal opinion is that this would not be reasonable. The option 
> `-disable-llvm-passes` should, from my point of view, prevent any and all 
> LLVM passes from being run. I also frequently make use of this option when 
> debugging the LLVM IR being output for C++ coroutines code, so if 
> `-disable-llvm-passes` didn't disable coroutines passes, I'd need another 
> option that did 
> (`-disable-llvm-passes-no-really-even-coroutine-passes-them-too` ).
>
> All this being said, considering this behavior has existed in the legacy PM 
> since day one, I think we should start a separate discussion on if/how to 
> change that behavior. I'm working on an update for these patches to address 
> funclet optimization, but the update will not change the fact that coroutine 
> passes are not run when `-disable-llvm-passes` is specified. I think that's 
> an orthogonal issue.


make sense to me, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71903



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


[PATCH] D71939: clang-format: fix conflict between FormatStyle::BWACS_MultiLine and BeforeCatch/BeforeElse

2020-01-04 Thread Pavlo Shkrabliuk via Phabricator via cfe-commits
pastey added a comment.

Guys, sorry for a dumb question – so the fix can be pushed now? If yes, then 
there's one issue – I don't think I have write access to push the fix – may I 
ask one of you to push it?

ERROR: Permission to llvm/llvm-project.git denied to pastey12.


Repository:
  rC Clang

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

https://reviews.llvm.org/D71939



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


[PATCH] D72144: Treat C# `using` as a control statement

2020-01-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D72144



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


[PATCH] D72150: Allow space after C-style cast in C# code

2020-01-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.
This revision is now accepted and ready to land.

Thanks for the patch, really good to have others working on C# support. LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72150



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