[PATCH] D62187: Delete default constructors, copy constructors, move constructors, copy assignment, move assignment operators on Expr, Stmt and Decl

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: clang/include/clang/AST/DeclBase.h:374
+  Decl(const Decl&) = delete;
+  Decl(Decl &&) = delete;
+  Decl &operator=(const Decl&) = delete;

NIT: move constructors and assignments won't be generated if copy is deleted, 
so this is redundant.
However, feel free to keep it if you want to be more explicit.



Comment at: clang/include/clang/AST/Stmt.h:1057
 
+  Stmt() = delete;
   Stmt(const Stmt &) = delete;

NIT: Move the deleted declarations to the start of the class?
Or move the deleted declarations in other classes to the first public section?

(For consistency)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62187



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

[PATCH] D61386: [clang-tidy] Add support writing a check as a Transformer rewrite rule.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp:46
+
+  StringRef Message = "no explanation";
+  if (Case.Explanation) {

The users will see this for every case without explanation, right?
I'd expect the rules without explanation to be somewhat common, maybe don't 
show any message at all in that case?



Comment at: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h:1
+//===-- TransformerClangTidyCheck.h - clang-tidy 
--===//
+//

NIT: maybe use `TransformerCheck` for brevity? I'm not a `clang-tidy` 
maintainer, though, so not sure whether that aligns with the rest of the code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61386



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

r361355 - [PPC64] Parse -elfv1 -elfv2 when specified on target triple

[Fangrui Song via cfe-commits]

Author: maskray
Date: Wed May 22 00:29:59 2019
New Revision: 361355

URL: http://llvm.org/viewvc/llvm-project?rev=361355&view=rev
Log:
[PPC64] Parse -elfv1 -elfv2 when specified on target triple

Summary:
For big-endian powerpc64, the default ABI is ELFv1. OpenPower ABI ELFv2 is 
supported when -mabi=elfv2 is specified. FreeBSD support for PowerPC64 ELFv2 
ABI with LLVM is in progress[1]. This patch adds an alternative way to specify 
ELFv2 ABI on target triple [2].

The following results are expected:

ELFv1 when using:
-target powerpc64-unknown-freebsd12.0
-target powerpc64-unknown-freebsd12.0 -mabi=elfv1
-target powerpc64-unknown-freebsd12.0-elfv1

ELFv2 when using:
-target powerpc64-unknown-freebsd12.0 -mabi=elfv2
-target powerpc64-unknown-freebsd12.0-elfv2

[1] https://wiki.freebsd.org/powerpc/llvm-elfv2
[2] https://clang.llvm.org/docs/CrossCompilation.html

Patch by Alfredo Dal'Ava Júnior!

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

Modified:
cfe/trunk/lib/Basic/Targets/PPC.h

Modified: cfe/trunk/lib/Basic/Targets/PPC.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/PPC.h?rev=361355&r1=361354&r2=361355&view=diff
==
--- cfe/trunk/lib/Basic/Targets/PPC.h (original)
+++ cfe/trunk/lib/Basic/Targets/PPC.h Wed May 22 00:29:59 2019
@@ -379,13 +379,11 @@ public:
 
 if ((Triple.getArch() == llvm::Triple::ppc64le)) {
   resetDataLayout("e-m:e-i64:64-n32:64");
-  ABI = "elfv2";
 } else {
   resetDataLayout("E-m:e-i64:64-n32:64");
-  ABI = "elfv1";
 }
 
-switch (getTriple().getOS()) {
+switch (Triple.getOS()) {
 case llvm::Triple::FreeBSD:
   LongDoubleWidth = LongDoubleAlign = 64;
   LongDoubleFormat = &llvm::APFloat::IEEEdouble();


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

[PATCH] D61950: [PowerPC64] adds ABI parsing when specified on target triple

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Reordered the code a bit and committed for you...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61950



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

[PATCH] D61950: [PowerPC64] adds ABI parsing when specified on target triple

[Fangrui Song via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL361355: [PPC64] Parse -elfv1 -elfv2 when specified on target 
triple (authored by MaskRay, committed by ).
Herald added a subscriber: kristina.

Changed prior to commit:
  https://reviews.llvm.org/D61950?vs=199633&id=200657#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61950

Files:
  cfe/trunk/lib/Basic/Targets/PPC.h
  llvm/trunk/include/llvm/ADT/Triple.h
  llvm/trunk/lib/Support/Triple.cpp
  llvm/trunk/lib/Target/PowerPC/PPCTargetMachine.cpp
  llvm/trunk/test/CodeGen/PowerPC/ppc64-elf-abi.ll


Index: llvm/trunk/lib/Target/PowerPC/PPCTargetMachine.cpp
===
--- llvm/trunk/lib/Target/PowerPC/PPCTargetMachine.cpp
+++ llvm/trunk/lib/Target/PowerPC/PPCTargetMachine.cpp
@@ -214,6 +214,8 @@
   case Triple::ppc64le:
 return PPCTargetMachine::PPC_ABI_ELFv2;
   case Triple::ppc64:
+if (TT.getEnvironment() == llvm::Triple::ELFv2)
+  return PPCTargetMachine::PPC_ABI_ELFv2;
 return PPCTargetMachine::PPC_ABI_ELFv1;
   default:
 return PPCTargetMachine::PPC_ABI_UNKNOWN;
Index: llvm/trunk/lib/Support/Triple.cpp
===
--- llvm/trunk/lib/Support/Triple.cpp
+++ llvm/trunk/lib/Support/Triple.cpp
@@ -228,6 +228,8 @@
   case CODE16: return "code16";
   case EABI: return "eabi";
   case EABIHF: return "eabihf";
+  case ELFv1: return "elfv1";
+  case ELFv2: return "elfv2";
   case Android: return "android";
   case Musl: return "musl";
   case MuslEABI: return "musleabi";
@@ -521,6 +523,8 @@
   return StringSwitch(EnvironmentName)
 .StartsWith("eabihf", Triple::EABIHF)
 .StartsWith("eabi", Triple::EABI)
+.StartsWith("elfv1", Triple::ELFv1)
+.StartsWith("elfv2", Triple::ELFv2)
 .StartsWith("gnuabin32", Triple::GNUABIN32)
 .StartsWith("gnuabi64", Triple::GNUABI64)
 .StartsWith("gnueabihf", Triple::GNUEABIHF)
Index: llvm/trunk/include/llvm/ADT/Triple.h
===
--- llvm/trunk/include/llvm/ADT/Triple.h
+++ llvm/trunk/include/llvm/ADT/Triple.h
@@ -202,6 +202,8 @@
 CODE16,
 EABI,
 EABIHF,
+ELFv1,
+ELFv2,
 Android,
 Musl,
 MuslEABI,
Index: llvm/trunk/test/CodeGen/PowerPC/ppc64-elf-abi.ll
===
--- llvm/trunk/test/CodeGen/PowerPC/ppc64-elf-abi.ll
+++ llvm/trunk/test/CodeGen/PowerPC/ppc64-elf-abi.ll
@@ -5,6 +5,11 @@
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu 
-target-abi elfv1 < %s | FileCheck %s -check-prefix=CHECK-ELFv1
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu 
-target-abi elfv2 < %s | FileCheck %s -check-prefix=CHECK-ELFv2
 
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd < %s | 
FileCheck %s -check-prefix=CHECK-ELFv1
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd 
-target-abi elfv1 < %s | FileCheck %s -check-prefix=CHECK-ELFv1
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd 
-target-abi elfv2 < %s | FileCheck %s -check-prefix=CHECK-ELFv2
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd-elfv1  < 
%s | FileCheck %s -check-prefix=CHECK-ELFv1
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd-elfv2  < 
%s | FileCheck %s -check-prefix=CHECK-ELFv2
+
 ; CHECK-ELFv2: .abiversion 2
 ; CHECK-ELFv1-NOT: .abiversion 2
-
Index: cfe/trunk/lib/Basic/Targets/PPC.h
===
--- cfe/trunk/lib/Basic/Targets/PPC.h
+++ cfe/trunk/lib/Basic/Targets/PPC.h
@@ -379,13 +379,11 @@
 
 if ((Triple.getArch() == llvm::Triple::ppc64le)) {
   resetDataLayout("e-m:e-i64:64-n32:64");
-  ABI = "elfv2";
 } else {
   resetDataLayout("E-m:e-i64:64-n32:64");
-  ABI = "elfv1";
 }
 
-switch (getTriple().getOS()) {
+switch (Triple.getOS()) {
 case llvm::Triple::FreeBSD:
   LongDoubleWidth = LongDoubleAlign = 64;
   LongDoubleFormat = &llvm::APFloat::IEEEdouble();


Index: llvm/trunk/lib/Target/PowerPC/PPCTargetMachine.cpp
===
--- llvm/trunk/lib/Target/PowerPC/PPCTargetMachine.cpp
+++ llvm/trunk/lib/Target/PowerPC/PPCTargetMachine.cpp
@@ -214,6 +214,8 @@
   case Triple::ppc64le:
 return PPCTargetMachine::PPC_ABI_ELFv2;
   case Triple::ppc64:
+if (TT.getEnvironment() == llvm::Triple::ELFv2)
+  return PPCTargetMachine::PPC_ABI_ELFv2;
 return PPCTargetMachine::PPC_ABI_ELFv1;
   default:
 return PPCTargetMachine::PPC_ABI_UNKNOWN;
Index: llvm/trunk/lib/Support/Triple.cpp
===
--- llvm/trunk/lib/Support/Triple.cpp
+++ llvm/trunk/lib/Support/Triple.cpp
@@ -228,6 +228,8 @@
   case CODE16: ret

[PATCH] D62187: Delete default constructors, copy constructors, move constructors, copy assignment, move assignment operators on Expr, Stmt and Decl

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr marked 4 inline comments as done.
gribozavr added inline comments.



Comment at: clang/include/clang/AST/DeclBase.h:374
+  Decl(const Decl&) = delete;
+  Decl(Decl &&) = delete;
+  Decl &operator=(const Decl&) = delete;

ilya-biryukov wrote:
> NIT: move constructors and assignments won't be generated if copy is deleted, 
> so this is redundant.
> However, feel free to keep it if you want to be more explicit.
I feel like it is better to have the API explicitly spelled out.



Comment at: clang/include/clang/AST/Stmt.h:1057
 
+  Stmt() = delete;
   Stmt(const Stmt &) = delete;

ilya-biryukov wrote:
> NIT: Move the deleted declarations to the start of the class?
> Or move the deleted declarations in other classes to the first public section?
> 
> (For consistency)
I put these new declarations close to the "primary" constructor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62187



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

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 🔍

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added a comment.

> I prefer Option 2 because it is a cleaner, more understandable design for the 
> matchers.

I agree with Aaron. Clang or Clang Tooling provide no promise of source 
stability. Of course we should not break things just because we can, but API 
coherence and maintainability of Clang Tooling code are good reasons.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60543



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

[PATCH] D60455: [SYCL] Implement SYCL device code outlining

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 200658.
Fznamznon added a comment.

Minor fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Parse/ParseAST.cpp
  clang/lib/Sema/CMakeLists.txt
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaSYCL.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CodeGenSYCL/device-functions.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
  clang/test/SemaSYCL/device-attributes.cpp

Index: clang/test/SemaSYCL/device-attributes.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -fsycl-is-device -verify %s
+
+[[clang::sycl_kernel]] int gv2 = 0; // expected-warning {{'sycl_kernel' attribute only applies to functions}}
+__attribute((sycl_kernel)) int gv3 = 0; // expected-warning {{'sycl_kernel' attribute only applies to functions}}
+
+__attribute((sycl_kernel)) void foo();
+[[clang::sycl_kernel]] void foo1();
+
+__attribute((sycl_kernel(1))) void foo(); // expected-error {{'sycl_kernel' attribute takes no arguments}}
+[[clang::sycl_kernel(1)]] void foo2(); // expected-error {{'sycl_kernel' attribute takes no arguments}}
Index: clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -fsycl-is-device -verify %s
+// Now pretend that we're compiling regular C++ file without SYCL mode enabled.
+// There should be warnings.
+// RUN: %clang_cc1 -fsyntax-only -verify -x c++ %s
+
+#if not defined(__SYCL_DEVICE_ONLY__)
+// expected-warning@+6 {{'sycl_kernel' attribute ignored}}
+// expected-warning@+6 {{'sycl_kernel' attribute ignored}}
+#else
+// expected-no-diagnostics
+#endif
+
+__attribute((sycl_kernel)) void foo();
+[[clang::sycl_kernel]] void foo2();
+
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
@@ -124,6 +124,7 @@
 // CHECK-NEXT: ReturnTypestate (SubjectMatchRule_function, SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: ReturnsNonNull (SubjectMatchRule_objc_method, SubjectMatchRule_function)
 // CHECK-NEXT: ReturnsTwice (SubjectMatchRule_function)
+// CHECK-NEXT: SYCLKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)
 // CHECK-NEXT: Section (SubjectMatchRule_function, SubjectMatchRule_variable_is_global, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property)
 // CHECK-NEXT: SetTypestate (SubjectMatchRule_function_is_member)
Index: clang/test/CodeGenSYCL/device-functions.cpp
===
--- /dev/null
+++ clang/test/CodeGenSYCL/device-functions.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -triple spir64-unknown-unknown -std=c++11 -fsycl-is-device -S -emit-llvm %s -o - | FileCheck %s
+
+template 
+T bar(T arg);
+
+void foo() {
+  int a = 1 + 1 + bar(1);
+}
+
+template 
+T bar(T arg) {
+  return arg;
+}
+
+template 
+__attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
+  kernelFunc();
+}
+
+int main() {
+  kernel_single_task([]() { foo(); });
+  return 0;
+}
+// CHECK: define spir_func void @{{.*}}foo
+// CHECK: define linkonce_odr spir_func i32 @{{.*}}bar
+// CHECK: define internal spir_func void @{{.*}}kernel_single_task
+// FIXME: Next function is lambda () operator. spir_func calling convention
+// is missed for C++ methods.
+// CHECK: define internal void @"_ZZ4mainENK3$_0clEv"(%class.anon* %this)
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5517,14 +5517,30 @@
 Function, [this, Inst, DefinitionRequired](FunctionDecl *CurFD) {
   InstantiateFunctionDefinition(/*FIXME:*/ Inst.second, CurFD, true,
 DefinitionRequired, true);
-  if (CurFD->isDefined())
+  if (CurFD->isDefined()) {
+// Because all SYCL kernel functions are template functions - they
+// have deferred instantination. We need bodies of these functions
+// so we are checking for SYCL kernel attribute after instantination.
+if (getLangOpts().SYCLIsDevice &&
+CurFD->ha

r361363 - [PPC64] Fix PPC64TargetInfo after D61950

[Fangrui Song via cfe-commits]

Author: maskray
Date: Wed May 22 02:17:21 2019
New Revision: 361363

URL: http://llvm.org/viewvc/llvm-project?rev=361363&view=rev
Log:
[PPC64] Fix PPC64TargetInfo after D61950

Modified:
cfe/trunk/lib/Basic/Targets/PPC.h

Modified: cfe/trunk/lib/Basic/Targets/PPC.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/PPC.h?rev=361363&r1=361362&r2=361363&view=diff
==
--- cfe/trunk/lib/Basic/Targets/PPC.h (original)
+++ cfe/trunk/lib/Basic/Targets/PPC.h Wed May 22 02:17:21 2019
@@ -382,6 +382,7 @@ public:
 } else {
   resetDataLayout("E-m:e-i64:64-n32:64");
 }
+ABI = Triple.getEnvironment() == llvm::Triple::ELFv1 ? "elfv1" : "elfv2";
 
 switch (Triple.getOS()) {
 case llvm::Triple::FreeBSD:


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

[PATCH] D59467: [clang] Adding the Likelihood Attribute from C++2a

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

The `ASTImporter.cpp` looks good to me. (Becasue the `BranchHint` is a simple 
an enum, so we don't need to specifically import that as we would in case of 
e.g. an `Expr`.)


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

https://reviews.llvm.org/D59467



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

[PATCH] D62009: [clang] perform semantic checking in constant context

[Tyker via Phabricator via cfe-commits]

Tyker updated this revision to Diff 200662.
Tyker added a comment.

most comments were fixed. but there is a few point on which the direction isn't 
clear.

> I think all of the new warnings in the test cases here are undesirable (they 
> duplicate errors produced by the constant evaluator)

in the last revision `warn_impcast_integer_precision_constant` was always 
diagnosed in constant expression and diagnosed if reachable otherwise. this 
warning was responsible of all new warnings but isn't diagnosed by the constant 
evaluator. which is correct because AFAIK integral casts have implementation 
defined behavior and not undefined behavior.

> For code patterns that are suspicious but defined (eg, we think they might do 
> something other than what the programmer intended), we should typically 
> diagnose using Diag, regardless of whether they appear in a constant context.

this is not currently done even in non constant context. for example 
`warn_impcast_integer_precision_constant` is diagnosed via 
`DiagRuntimeBehavior`.

in this revision

  constexpr int i0 = (long long)__builtin_is_constant_evaluated() * (1ll << 
33); //#1

is diagnosed as it should by the reason it is diagnosed isn't correct. it is 
diagnosed only because `CheckCompletedExpr` doesn't push an 
`ExpressionEvaluationContextRecord` but just set isConstantEvaluatedOverride. 
so `DiagRuntimeBehavior` doesn't see the context as constant context. This 
means that other context that are correctly in constant context like:

  case (long long)__builtin_is_constant_evaluated() * (1ll << 33):; //#1

will not be diagnosed even thought they probably should.


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

https://reviews.llvm.org/D62009

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticIDs.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/SemaCXX/attr-nonnull.cpp
  clang/test/SemaCXX/constant-expression-cxx11.cpp

Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -438,8 +438,8 @@
 
 constexpr char c0 = "nought index"[0];
 constexpr char c1 = "nice index"[10];
-constexpr char c2 = "nasty index"[12]; // expected-error {{must be initialized by a constant expression}} expected-warning {{is past the end}} expected-note {{read of dereferenced one-past-the-end pointer}}
-constexpr char c3 = "negative index"[-1]; // expected-error {{must be initialized by a constant expression}} expected-warning {{is before the beginning}} expected-note {{cannot refer to element -1 of array of 15 elements}}
+constexpr char c2 = "nasty index"[12]; // expected-error {{must be initialized by a constant expression}} expected-note {{read of dereferenced one-past-the-end pointer}}
+constexpr char c3 = "negative index"[-1]; // expected-error {{must be initialized by a constant expression}} expected-note {{cannot refer to element -1 of array of 15 elements}}
 constexpr char c4 = ((char*)(int*)"no reinterpret_casts allowed")[14]; // expected-error {{must be initialized by a constant expression}} expected-note {{cast that performs the conversions of a reinterpret_cast}}
 
 constexpr const char *p = "test" + 2;
@@ -547,7 +547,7 @@
 constexpr int xs0 = p[-3]; // ok
 constexpr int xs_1 = p[-4]; // expected-error {{constant expression}} expected-note {{cannot refer to element -1}}
 
-constexpr int zs[2][2][2][2] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 }; // expected-note {{array 'zs' declared here}}
+constexpr int zs[2][2][2][2] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 };
 static_assert(zs[0][0][0][0] == 1, "");
 static_assert(zs[1][1][1][1] == 16, "");
 static_assert(zs[0][0][0][2] == 3, ""); // expected-error {{constant expression}} expected-note {{read of dereferenced one-past-the-end pointer}}
@@ -557,8 +557,7 @@
 static_assert(*(&(&(*(*&(&zs[2] - 1)[0] + 2 - 2))[2])[-1][2] - 2) == 11, "");
 constexpr int err_zs_1_2_0_0 = zs[1][2][0][0]; // \
 expected-error {{constant expression}} \
-expected-note {{cannot access array element of pointer past the end}} \
-expected-warning {{array index 2 is past the end of the array (which contains 2 elements)}}
+expected-note {{cannot access array element of pointer past the end}}
 
 constexpr int fail(const int &p) {
   return (&p)[64]; // expected-note {{cannot refer to element 64 of array of 2 elements}}
Index: clang/test/SemaCXX/attr-nonnull.cpp
===
--- clang/test/SemaCXX/attr-nonnull.cpp
+++ clang/test/SemaCXX/attr-nonnull.cpp
@@ -52,3 +52,35 @@
   (void)(x != 0);  // expected-warning{{null passed}}
 }
 }
+
+namespace test5 {
+
+constexpr int c = 0;
+
+__attribute__((nonnull))
+c

r361365 - [PPC64] Fix PPC64TargetInfo ABI on clang side after D61950

[Fangrui Song via cfe-commits]

Author: maskray
Date: Wed May 22 02:26:46 2019
New Revision: 361365

URL: http://llvm.org/viewvc/llvm-project?rev=361365&view=rev
Log:
[PPC64] Fix PPC64TargetInfo ABI on clang side after D61950

Modified:
cfe/trunk/lib/Basic/Targets/PPC.h

Modified: cfe/trunk/lib/Basic/Targets/PPC.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/PPC.h?rev=361365&r1=361364&r2=361365&view=diff
==
--- cfe/trunk/lib/Basic/Targets/PPC.h (original)
+++ cfe/trunk/lib/Basic/Targets/PPC.h Wed May 22 02:26:46 2019
@@ -379,10 +379,11 @@ public:
 
 if ((Triple.getArch() == llvm::Triple::ppc64le)) {
   resetDataLayout("e-m:e-i64:64-n32:64");
+  ABI = "elfv2";
 } else {
   resetDataLayout("E-m:e-i64:64-n32:64");
+  ABI = Triple.getEnvironment() == llvm::Triple::ELFv2 ? "elfv2" : "elfv1";
 }
-ABI = Triple.getEnvironment() == llvm::Triple::ELFv1 ? "elfv1" : "elfv2";
 
 switch (Triple.getOS()) {
 case llvm::Triple::FreeBSD:


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

[PATCH] D61950: [PowerPC64] adds ABI parsing when specified on target triple

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

@adalava You need to run both `check-llvm` and `check-clang`, but it was my 
fault that I hadn't done this before committing... 
Two clang tests failed 
(http://lab.llvm.org:8011/builders/clang-atom-d525-fedora-rel/builds/24521) It 
should have been fixed by rC361365 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61950



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

[PATCH] D62238: [CodeComplete] Complete a lambda when preferred type is a function

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: kadircet.
Herald added a project: clang.

Uses a heuristic to detect std::function and friends.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62238

Files:
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/lambdas.cpp

Index: clang/test/CodeCompletion/lambdas.cpp
===
--- /dev/null
+++ clang/test/CodeCompletion/lambdas.cpp
@@ -0,0 +1,53 @@
+template 
+struct function {
+};
+
+
+void test() {
+  void (*x)(int, double) = nullptr;
+
+  function y = {};
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:7:28 %s -o - | FileCheck -check-prefix=CHECK-1 %s
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:9:35 %s -o - | FileCheck -check-prefix=CHECK-1 %s
+  // CHECK-1: COMPLETION: Pattern : [](int <#parameter#>, double <#parameter#>){<#body#>}
+
+  // == Placeholders for suffix types must be placed properly.
+  function z = {};
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:15:36 %s -o - | FileCheck -check-prefix=CHECK-2 %s
+  // CHECK-2: COMPLETION: Pattern : [](void (* <#parameter#>)(int)){<#body#>}
+
+  // == No need for a parameter list if function has no parameters.
+  function a = {};
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:20:24 %s -o - | FileCheck -check-prefix=CHECK-3 %s
+  // CHECK-3: COMPLETION: Pattern : []{<#body#>}
+}
+
+template 
+struct vector {};
+
+void test2() {
+  // == Try to preserve types as written.
+  function)> a = {};
+
+  using function_typedef = function)>;
+  function_typedef b = {};
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:30:35 %s -o - | FileCheck -check-prefix=CHECK-4 %s
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:33:24 %s -o - | FileCheck -check-prefix=CHECK-4 %s
+  // CHECK-4: COMPLETION: Pattern : [](vector <#parameter#>){<#body#>}
+}
+
+// Check another common function wrapper name.
+template  struct unique_function {};
+
+void test3() {
+  unique_function a = {};
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:43:31 %s -o - | FileCheck -check-prefix=CHECK-5 %s
+  // CHECK-5: COMPLETION: Pattern : []{<#body#>}
+}
+
+template  struct some_random_class {};
+void test4() {
+  some_random_class a = {};
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:50:31 %s -o - | FileCheck -check-prefix=CHECK-6 %s
+  // CHECK-6-NOT: COMPLETION: Pattern : []{
+}
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -4108,6 +4108,83 @@
   Results.ExitScope();
 }
 
+/// Try to find a corresponding FunctionProtoType for function-like types (e.g.
+/// function pointers, std::function, etc).
+static const FunctionProtoType *TryDeconstructFunctionLike(QualType T) {
+  assert(!T.isNull());
+  // Try to extract first template argument from std::function<> and similar.
+  DeclarationName PotentialTemplateName;
+  const TemplateArgument *PotentialFunctionArg = nullptr;
+  if (auto *Specialization = T->getAs()) {
+// FIXME: handle other kinds of template names.
+auto *Template = Specialization->getTemplateName().getAsTemplateDecl();
+if (!Template || Specialization->getNumArgs() < 1)
+  return nullptr;
+PotentialTemplateName = Template->getDeclName();
+PotentialFunctionArg = &Specialization->getArg(0);
+  } else if (auto *Class = dyn_cast_or_null(
+ T->getAsCXXRecordDecl())) {
+if (Class->getTemplateArgs().size() < 1)
+  return nullptr;
+PotentialTemplateName = Class->getDeclName();
+PotentialFunctionArg = &Class->getTemplateArgs().get(0);
+  }
+  if (PotentialFunctionArg) {
+if (!PotentialTemplateName.isIdentifier() ||
+!PotentialTemplateName.getAsIdentifierInfo()->getName().contains(
+"function"))
+  return nullptr;
+if (PotentialFunctionArg->getKind() != TemplateArgument::Type)
+  return nullptr;
+return PotentialFunctionArg->getAsType()->getAs();
+  }
+  // Handle other cases.
+  if (T->isPointerType())
+T = T->getPointeeType();
+  return T->getAs();
+}
+
+/// Adds a pattern completion for a lambda expression with the specified
+/// parameter types and placeholders for parameter names.
+static void AddLambdaCompletion(ResultBuilder &Results,
+llvm::ArrayRef Parameters,
+const LangOptions &LangOpts) {
+  CodeCompletionBuilder Completion(Results.getAllocator(),
+   Results.getCodeCompletionTUInfo());
+  // []() {}
+  Completion.AddChunk(CodeCompletionString::CK_LeftBracket);
+  Completion.AddChunk(CodeComp

[PATCH] D62187: Delete default constructors, copy constructors, move constructors, copy assignment, move assignment operators on Expr, Stmt and Decl

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

LG either way, the comments were NITs.




Comment at: clang/include/clang/AST/Stmt.h:1057
 
+  Stmt() = delete;
   Stmt(const Stmt &) = delete;

gribozavr wrote:
> ilya-biryukov wrote:
> > NIT: Move the deleted declarations to the start of the class?
> > Or move the deleted declarations in other classes to the first public 
> > section?
> > 
> > (For consistency)
> I put these new declarations close to the "primary" constructor.
Why not move declarations inside `Stmt` to the primary constructor too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62187



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

[PATCH] D62232: [Clang][Driver] recheck for nullptr

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

According to the coverage report we don't have any coverage of TC == null at 
all - not sure if its (a) possible to add tests or (b) if we can ever have a 
non-null TC - so should we just assert?

http://lab.llvm.org:8080/coverage/coverage-reports/clang/coverage/Users/buildslave/jenkins/workspace/clang-stage2-coverage-R/llvm/tools/clang/lib/Driver/Driver.cpp.html


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62232



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

[PATCH] D62187: Delete default constructors, copy constructors, move constructors, copy assignment, move assignment operators on Expr, Stmt and Decl

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr marked 2 inline comments as done.
gribozavr added inline comments.



Comment at: clang/include/clang/AST/Stmt.h:1057
 
+  Stmt() = delete;
   Stmt(const Stmt &) = delete;

ilya-biryukov wrote:
> gribozavr wrote:
> > ilya-biryukov wrote:
> > > NIT: Move the deleted declarations to the start of the class?
> > > Or move the deleted declarations in other classes to the first public 
> > > section?
> > > 
> > > (For consistency)
> > I put these new declarations close to the "primary" constructor.
> Why not move declarations inside `Stmt` to the primary constructor too?
Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62187



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

[PATCH] D62187: Delete default constructors, copy constructors, move constructors, copy assignment, move assignment operators on Expr, Stmt and Decl

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr updated this revision to Diff 200684.
gribozavr marked 2 inline comments as done.
gribozavr added a comment.

Addressed review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62187

Files:
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/Stmt.h
  clang/lib/CodeGen/CGBuiltin.cpp

Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -1134,9 +1134,10 @@
 return F;
 
   llvm::SmallVector ArgTys;
-  llvm::SmallVector Params;
-  Params.emplace_back(Ctx, nullptr, SourceLocation(), &Ctx.Idents.get("buffer"),
-  Ctx.VoidPtrTy, ImplicitParamDecl::Other);
+  FunctionArgList Args;
+  Args.push_back(ImplicitParamDecl::Create(
+  Ctx, nullptr, SourceLocation(), &Ctx.Idents.get("buffer"), Ctx.VoidPtrTy,
+  ImplicitParamDecl::Other));
   ArgTys.emplace_back(Ctx.VoidPtrTy);
 
   for (unsigned int I = 0, E = Layout.Items.size(); I < E; ++I) {
@@ -1145,17 +1146,13 @@
   continue;
 
 QualType ArgTy = getOSLogArgType(Ctx, Size);
-Params.emplace_back(
+Args.push_back(ImplicitParamDecl::Create(
 Ctx, nullptr, SourceLocation(),
 &Ctx.Idents.get(std::string("arg") + llvm::to_string(I)), ArgTy,
-ImplicitParamDecl::Other);
+ImplicitParamDecl::Other));
 ArgTys.emplace_back(ArgTy);
   }
 
-  FunctionArgList Args;
-  for (auto &P : Params)
-Args.push_back(&P);
-
   QualType ReturnTy = Ctx.VoidTy;
   QualType FuncionTy = Ctx.getFunctionType(ReturnTy, ArgTys, {});
 
@@ -1188,7 +1185,7 @@
   auto AL = ApplyDebugLocation::CreateArtificial(*this);
 
   CharUnits Offset;
-  Address BufAddr(Builder.CreateLoad(GetAddrOfLocalVar(&Params[0]), "buf"),
+  Address BufAddr(Builder.CreateLoad(GetAddrOfLocalVar(Args[0]), "buf"),
   BufferAlignment);
   Builder.CreateStore(Builder.getInt8(Layout.getSummaryByte()),
   Builder.CreateConstByteGEP(BufAddr, Offset++, "summary"));
@@ -1208,7 +1205,7 @@
 if (!Size.getQuantity())
   continue;
 
-Address Arg = GetAddrOfLocalVar(&Params[I]);
+Address Arg = GetAddrOfLocalVar(Args[I]);
 Address Addr = Builder.CreateConstByteGEP(BufAddr, Offset, "argData");
 Addr = Builder.CreateBitCast(Addr, Arg.getPointer()->getType(),
  "argDataCast");
Index: clang/include/clang/AST/Stmt.h
===
--- clang/include/clang/AST/Stmt.h
+++ clang/include/clang/AST/Stmt.h
@@ -1040,6 +1040,12 @@
   explicit Stmt(StmtClass SC, EmptyShell) : Stmt(SC) {}
 
 public:
+  Stmt() = delete;
+  Stmt(const Stmt &) = delete;
+  Stmt(Stmt &&) = delete;
+  Stmt &operator=(const Stmt &) = delete;
+  Stmt &operator=(Stmt &&) = delete;
+
   Stmt(StmtClass SC) {
 static_assert(sizeof(*this) <= 8,
   "changing bitfields changed sizeof(Stmt)");
@@ -1054,11 +1060,6 @@
 return static_cast(StmtBits.sClass);
   }
 
-  Stmt(const Stmt &) = delete;
-  Stmt(Stmt &&) = delete;
-  Stmt &operator=(const Stmt &) = delete;
-  Stmt &operator=(Stmt &&) = delete;
-
   const char *getStmtClassName() const;
 
   bool isOMPStructuredBlock() const { return StmtBits.IsOMPStructuredBlock; }
Index: clang/include/clang/AST/Expr.h
===
--- clang/include/clang/AST/Expr.h
+++ clang/include/clang/AST/Expr.h
@@ -108,6 +108,13 @@
 class Expr : public ValueStmt {
   QualType TR;
 
+public:
+  Expr() = delete;
+  Expr(const Expr&) = delete;
+  Expr(Expr &&) = delete;
+  Expr &operator=(const Expr&) = delete;
+  Expr &operator=(Expr&&) = delete;
+
 protected:
   Expr(StmtClass SC, QualType T, ExprValueKind VK, ExprObjectKind OK,
bool TD, bool VD, bool ID, bool ContainsUnexpandedParameterPack)
Index: clang/include/clang/AST/DeclBase.h
===
--- clang/include/clang/AST/DeclBase.h
+++ clang/include/clang/AST/DeclBase.h
@@ -368,6 +368,13 @@
 return ModuleOwnershipKind::Unowned;
   }
 
+public:
+  Decl() = delete;
+  Decl(const Decl&) = delete;
+  Decl(Decl &&) = delete;
+  Decl &operator=(const Decl&) = delete;
+  Decl &operator=(Decl&&) = delete;
+
 protected:
   Decl(Kind DK, DeclContext *DC, SourceLocation L)
   : NextInContextAndBits(nullptr, getModuleOwnershipKindForChildOf(DC)),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r361368 - [Driver][Windows] Add dependent lib argument for -fprofile-generate and -fcs-profile-generate

[Russell Gallop via cfe-commits]

Author: russell_gallop
Date: Wed May 22 03:06:49 2019
New Revision: 361368

URL: http://llvm.org/viewvc/llvm-project?rev=361368&view=rev
Log:
[Driver][Windows] Add dependent lib argument for -fprofile-generate and 
-fcs-profile-generate

Follows on from r360674 which added it for -fprofile-instr-generate.

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

Modified:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/test/Driver/cl-options.c

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=361368&r1=361367&r2=361368&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Wed May 22 03:06:49 2019
@@ -803,6 +803,10 @@ static void addPGOAndCoverageFlags(const
 CmdArgs.push_back("-fprofile-instrument=csllvm");
   }
   if (PGOGenArg) {
+if (TC.getTriple().isWindowsMSVCEnvironment()) {
+  CmdArgs.push_back(Args.MakeArgString("--dependent-lib=" +
+   TC.getCompilerRT(Args, "profile")));
+}
 if (PGOGenArg->getOption().matches(
 PGOGenerateArg ? options::OPT_fprofile_generate_EQ
: options::OPT_fcs_profile_generate_EQ)) {

Modified: cfe/trunk/test/Driver/cl-options.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-options.c?rev=361368&r1=361367&r2=361368&view=diff
==
--- cfe/trunk/test/Driver/cl-options.c (original)
+++ cfe/trunk/test/Driver/cl-options.c Wed May 22 03:06:49 2019
@@ -62,12 +62,16 @@
 // RUN: %clang_cl /Z7 -### -- %s 2>&1 | FileCheck -check-prefix=gdefcolumn %s
 // gdefcolumn-NOT: -dwarf-column-info
 
-// RUN: %clang_cl -### /FA -fprofile-instr-generate -- %s 2>&1 | FileCheck 
-check-prefix=CHECK-PROFILE-GENERATE %s
-// RUN: %clang_cl -### /FA -fprofile-instr-generate=/tmp/somefile.profraw -- 
%s 2>&1 | FileCheck -check-prefix=CHECK-PROFILE-GENERATE-FILE %s
+// RUN: %clang_cl -### /FA -fprofile-instr-generate -- %s 2>&1 | FileCheck 
-check-prefix=CHECK-PROFILE-INSTR-GENERATE %s
+// RUN: %clang_cl -### /FA -fprofile-instr-generate=/tmp/somefile.profraw -- 
%s 2>&1 | FileCheck -check-prefix=CHECK-PROFILE-INSTR-GENERATE-FILE %s
+// CHECK-PROFILE-INSTR-GENERATE: "-fprofile-instrument=clang" 
"--dependent-lib={{[^"]*}}clang_rt.profile-{{[^"]*}}.lib"
+// CHECK-PROFILE-INSTR-GENERATE-FILE: 
"-fprofile-instrument-path=/tmp/somefile.profraw"
+
+// RUN: %clang_cl -### /FA -fprofile-generate -- %s 2>&1 | FileCheck 
-check-prefix=CHECK-PROFILE-GENERATE %s
+// CHECK-PROFILE-GENERATE: "-fprofile-instrument=llvm" 
"--dependent-lib={{[^"]*}}clang_rt.profile-{{[^"]*}}.lib"
+
 // RUN: %clang_cl -### /FA -fprofile-instr-generate -fprofile-instr-use -- %s 
2>&1 | FileCheck -check-prefix=CHECK-NO-MIX-GEN-USE %s
 // RUN: %clang_cl -### /FA -fprofile-instr-generate -fprofile-instr-use=file 
-- %s 2>&1 | FileCheck -check-prefix=CHECK-NO-MIX-GEN-USE %s
-// CHECK-PROFILE-GENERATE: "-fprofile-instrument=clang" 
"--dependent-lib={{[^"]*}}clang_rt.profile-{{[^"]*}}.lib"
-// CHECK-PROFILE-GENERATE-FILE: 
"-fprofile-instrument-path=/tmp/somefile.profraw"
 // CHECK-NO-MIX-GEN-USE: '{{[a-z=-]*}}' not allowed with '{{[a-z=-]*}}'
 
 // RUN: %clang_cl -### /FA -fprofile-instr-use -- %s 2>&1 | FileCheck 
-check-prefix=CHECK-PROFILE-USE %s


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

[PATCH] D62200: [Driver][Windows] Add dependent lib argument for -fprofile-generate and -fcs-profile-generate

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL361368: [Driver][Windows] Add dependent lib argument for 
-fprofile-generate and -fcs… (authored by russell_gallop, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62200?vs=200512&id=200685#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62200

Files:
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/test/Driver/cl-options.c


Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -803,6 +803,10 @@
 CmdArgs.push_back("-fprofile-instrument=csllvm");
   }
   if (PGOGenArg) {
+if (TC.getTriple().isWindowsMSVCEnvironment()) {
+  CmdArgs.push_back(Args.MakeArgString("--dependent-lib=" +
+   TC.getCompilerRT(Args, "profile")));
+}
 if (PGOGenArg->getOption().matches(
 PGOGenerateArg ? options::OPT_fprofile_generate_EQ
: options::OPT_fcs_profile_generate_EQ)) {
Index: cfe/trunk/test/Driver/cl-options.c
===
--- cfe/trunk/test/Driver/cl-options.c
+++ cfe/trunk/test/Driver/cl-options.c
@@ -62,12 +62,16 @@
 // RUN: %clang_cl /Z7 -### -- %s 2>&1 | FileCheck -check-prefix=gdefcolumn %s
 // gdefcolumn-NOT: -dwarf-column-info
 
-// RUN: %clang_cl -### /FA -fprofile-instr-generate -- %s 2>&1 | FileCheck 
-check-prefix=CHECK-PROFILE-GENERATE %s
-// RUN: %clang_cl -### /FA -fprofile-instr-generate=/tmp/somefile.profraw -- 
%s 2>&1 | FileCheck -check-prefix=CHECK-PROFILE-GENERATE-FILE %s
+// RUN: %clang_cl -### /FA -fprofile-instr-generate -- %s 2>&1 | FileCheck 
-check-prefix=CHECK-PROFILE-INSTR-GENERATE %s
+// RUN: %clang_cl -### /FA -fprofile-instr-generate=/tmp/somefile.profraw -- 
%s 2>&1 | FileCheck -check-prefix=CHECK-PROFILE-INSTR-GENERATE-FILE %s
+// CHECK-PROFILE-INSTR-GENERATE: "-fprofile-instrument=clang" 
"--dependent-lib={{[^"]*}}clang_rt.profile-{{[^"]*}}.lib"
+// CHECK-PROFILE-INSTR-GENERATE-FILE: 
"-fprofile-instrument-path=/tmp/somefile.profraw"
+
+// RUN: %clang_cl -### /FA -fprofile-generate -- %s 2>&1 | FileCheck 
-check-prefix=CHECK-PROFILE-GENERATE %s
+// CHECK-PROFILE-GENERATE: "-fprofile-instrument=llvm" 
"--dependent-lib={{[^"]*}}clang_rt.profile-{{[^"]*}}.lib"
+
 // RUN: %clang_cl -### /FA -fprofile-instr-generate -fprofile-instr-use -- %s 
2>&1 | FileCheck -check-prefix=CHECK-NO-MIX-GEN-USE %s
 // RUN: %clang_cl -### /FA -fprofile-instr-generate -fprofile-instr-use=file 
-- %s 2>&1 | FileCheck -check-prefix=CHECK-NO-MIX-GEN-USE %s
-// CHECK-PROFILE-GENERATE: "-fprofile-instrument=clang" 
"--dependent-lib={{[^"]*}}clang_rt.profile-{{[^"]*}}.lib"
-// CHECK-PROFILE-GENERATE-FILE: 
"-fprofile-instrument-path=/tmp/somefile.profraw"
 // CHECK-NO-MIX-GEN-USE: '{{[a-z=-]*}}' not allowed with '{{[a-z=-]*}}'
 
 // RUN: %clang_cl -### /FA -fprofile-instr-use -- %s 2>&1 | FileCheck 
-check-prefix=CHECK-PROFILE-USE %s


Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -803,6 +803,10 @@
 CmdArgs.push_back("-fprofile-instrument=csllvm");
   }
   if (PGOGenArg) {
+if (TC.getTriple().isWindowsMSVCEnvironment()) {
+  CmdArgs.push_back(Args.MakeArgString("--dependent-lib=" +
+   TC.getCompilerRT(Args, "profile")));
+}
 if (PGOGenArg->getOption().matches(
 PGOGenerateArg ? options::OPT_fprofile_generate_EQ
: options::OPT_fcs_profile_generate_EQ)) {
Index: cfe/trunk/test/Driver/cl-options.c
===
--- cfe/trunk/test/Driver/cl-options.c
+++ cfe/trunk/test/Driver/cl-options.c
@@ -62,12 +62,16 @@
 // RUN: %clang_cl /Z7 -### -- %s 2>&1 | FileCheck -check-prefix=gdefcolumn %s
 // gdefcolumn-NOT: -dwarf-column-info
 
-// RUN: %clang_cl -### /FA -fprofile-instr-generate -- %s 2>&1 | FileCheck -check-prefix=CHECK-PROFILE-GENERATE %s
-// RUN: %clang_cl -### /FA -fprofile-instr-generate=/tmp/somefile.profraw -- %s 2>&1 | FileCheck -check-prefix=CHECK-PROFILE-GENERATE-FILE %s
+// RUN: %clang_cl -### /FA -fprofile-instr-generate -- %s 2>&1 | FileCheck -check-prefix=CHECK-PROFILE-INSTR-GENERATE %s
+// RUN: %clang_cl -### /FA -fprofile-instr-generate=/tmp/somefile.profraw -- %s 2>&1 | FileCheck -check-prefix=CHECK-PROFILE-INSTR-GENERATE-FILE %s
+// CHECK-PROFILE-INSTR-GENERATE: "-fprofile-instrument=clang" "--dependent-lib={{[^"]*}}clang_rt.profile-{{[^"]*}}.lib"
+// CHECK-PROFILE-INSTR-GENERATE-FILE: "-fprofile-instrument-path=/tmp/somefil

[PATCH] D62149: [LibTooling] Update Transformer to use RangeSelector instead of NodePart enum.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM. See the nit about the comment, might be a good idea to fix it before 
landing.




Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:186
+/// where a \c TextGenerator, \c RangeSelector are otherwise expected.
+inline ASTEdit change(RangeSelector Target, std::string Replacement) {
+  return change(std::move(Target), text(std::move(Replacement)));

ymandel wrote:
> ilya-biryukov wrote:
> > The overloads create quite a lot of noise (and we are already starting to 
> > see a combinatorial explosion with two classes).
> > 
> > Could we try to overcome this? I see two options:
> > 1. Turn `TextGenerator` and `RangeSelector` into classes, which are 
> > constructible from `string` or `function`.
> > 2. Remove the string overloads, force users to explicitly construct the 
> > parameters.
> > 
> > Option (2) actually is simple and would add a bit of type safety, but will 
> > make some client code a bit less readable. Do you think the readability hit 
> > is significant?
> Agreed. I wasn't loving all the overloads myself.  I removed all the 
> string-related overloads. left the one overload that lets the user implicitly 
> specify the entirety of the match.
> 
> I'm ok with the result.  Certainly for RangeSelector, it forces a consistency 
> which I like. For the use of text() -- I could go both ways, but i think pure 
> text replacements will be rare outside of tests, so I'm not sure users will 
> be terribly inconvenienced by this.
> 
> The only overload i'm unsure about is the one I left, because I think a) it 
> will be a common case and b) it will confuse users to learn to use 
> `RewriteRule::RootID`.  That said, if you have an elegant, but explicit, 
> alternative I'm interested.  We could, for example, add combinators 
> `matchedNode()` and `matchedStatement()`, defined respectively as 
> `node(RewriteRule::RootID)` and `statement(RewriteRule::RootID)`.  Or, add 
> a`makeRule` overload for the `node()` case (and let users be explicit when 
> they want the `statement()` case.).
This one overload actually looks ok and I also agree it'll probably be somewhat 
common.
Thanks!



Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:184
+
+/// Replaces the matched portion of a RewriteRule with \p Replacement
+inline ASTEdit change(TextGenerator Replacement) {

NIT: maybe clarify that "matched portion" is the whole match of a rewrite rule. 
It looks like "portion" in the previous overload means "range defined by the 
selector", while in this overload it means "the whole matched node". It'd be 
nice to use different words there to avoid confusion and make the comment more 
useful


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62149



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

[PATCH] D61487: [clang-tidy] Make the plugin honor NOLINT

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:457
+  forwardDiagnostic(Info);
+  return;
+  }

Indentation is 2 spaces.



Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:472
   if (Info.hasSourceManager())
 checkFilters(Info.getLocation(), Info.getSourceManager());
 }

It seems like the `checkFilters` call should not be skipped even if we have 
another diagnostic engine.



Comment at: clang-tidy/ClangTidyDiagnosticConsumer.h:197
+DiagnosticIDs::Level L;
+std::string string;
+  };

Sorry, it is unclear what this struct is, and what its members are, especially 
given that the member names are `L` and `string`...


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61487



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

[PATCH] D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc added a comment.

Sorry I've been a bit slow to respond here...

In D61634#1503089 , @hfinkel wrote:

> In D61634#1502201 , @tejohnson wrote:
>
> > In D61634#1502138 , @hfinkel wrote:
> >
> > > In D61634#1502043 , @efriedma 
> > > wrote:
> > >
> > > > > I have a related patch that turns -fno-builtin* options into module 
> > > > > flags
> > > >
> > > > Do you have any opinion on representing -fno-builtin using a module 
> > > > flag vs. a function attribute in IR?  It seems generally more flexible 
> > > > and easier to reason about a function attribute from my perspective.  
> > > > But I might be missing something about the semantics of -fno-builtin 
> > > > that would make that representation awkward.  Or I guess it might just 
> > > > be more work to implement, given we have some IPO passes that use 
> > > > TargetLibraryInfo?
> > >
> > >
> > > I think that a function attribute would be better. We generally use these 
> > > flags only in the context of certain translation units, and when we use 
> > > LTO, it would be sad if we had to take the most-conservative settings 
> > > across the entire application. When we insert new function call to a 
> > > standard library, we always do it in the context of some function. We'd 
> > > probably need to block inlining in some cases, but that's better than a 
> > > global conservative setting.
> >
>


FWIW, I definitely agree here. This really is the end state we're going to find 
ourselves in and we should probably go directly there.

>> Using function level attributes instead of module flags does provide finer 
>> grained control and avoids the conservativeness when merging IR for LTO. The 
>> downsides I see, mostly just in terms of the engineering effort to get this 
>> to work, are:
>> 
>> - need to prevent inlining with different attributes
> 
> I think that this should be relatively straightforward. You just need to 
> update `AttributeFuncs::areInlineCompatible` and/or 
> `AttributeFuncs::mergeAttributesForInlining` by adding a new MergeRule in 
> include/llvm/IR/Attributes.td and writing a function like 
> adjustCallerStackProbeSize.

+1

> 
> 
>> - currently the TargetLibraryInfo is constructed on a per-module basis. 
>> Presumably it would instead need to be created per Function - this one in 
>> particular seems like it would require fairly extensive changes.
> 
> Interesting point. The largest issue I see is that we need TLI available from 
> loop passes, etc., and we can't automatically recompute a function-level 
> analysis there. We need to make sure that it's always available and not 
> invalidated. TLI is one of those analysis passes, being derived only from 
> things which don't change (i.e., the target triple), or things that change 
> very rarely (e.g., function attributes), that we probably don't want to 
> require all passes to explicitly say that they preserve it (not that the 
> mechanical change to all existing passes is hard, but it's easy to forget), 
> so I think we'd like something like the CFG-only concept in the current 
> passes, but stronger and perhaps turned on by default, for this kind of 
> "attributes-only" pass. (@chandlerc , thoughts on this?).

Yep, this makes sense.

The new PM makes this quite easy. The analysis itself gets to implement the 
invalidation hook, and say "nope, I'm not invalidated". In fact, in the new PM, 
`TargetLibraryInfo` already works this way. We build an instance per function 
and say it is never invalidated.

However, they are just trivial wrappers around shared implementations, so it 
will still require some non-trivial changes. Will need to remove the 
module-based access and move clients over to provide a function when they query 
it, etc.

IIRC, `TargetTransformInfo` already basically works exactly this way in both 
old and new PMs and we should be able to look at exactly the techniques it uses 
in both pass managers to build an effective way to manage these per-function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61634



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

[PATCH] D62244: [AMDGPU] Enable the implicit arguments for HIP (CLANG)

[Christudasan Devadasan via Phabricator via cfe-commits]

cdevadas created this revision.
cdevadas added reviewers: b-sumner, yaxunl.
Herald added subscribers: cfe-commits, t-tye, Anastasia, tpr, dstuttard, wdng, 
kzhuravl.
Herald added a project: clang.

Enable 48-bytes of implicit arguments for HIP as well. Earlier it was enabled 
for OpenCL. This code is specific to AMDGPU target.


Repository:
  rC Clang

https://reviews.llvm.org/D62244

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGenHIP/Inputs/hip.h
  test/CodeGenHIP/implicit-kernarg.cpp


Index: test/CodeGenHIP/implicit-kernarg.cpp
===
--- /dev/null
+++ test/CodeGenHIP/implicit-kernarg.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -emit-llvm -x hip -o - %s | 
FileCheck %s
+#include "Inputs/hip.h"
+
+__global__ void hip_kernel_temp() {
+}
+
+// CHECK-DAG: attributes #0 = { noinline nounwind optnone 
"amdgpu-implicitarg-num-bytes"="48"
Index: test/CodeGenHIP/Inputs/hip.h
===
--- /dev/null
+++ test/CodeGenHIP/Inputs/hip.h
@@ -0,0 +1,3 @@
+/* Minimal declarations for HIP support.  Testing purposes only. */
+
+#define __global__ __attribute__((global))
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -7853,7 +7853,8 @@
   const auto *ReqdWGS = M.getLangOpts().OpenCL ?
 FD->getAttr() : nullptr;
 
-  if (M.getLangOpts().OpenCL && FD->hasAttr() &&
+  if (((M.getLangOpts().OpenCL && FD->hasAttr()) ||
+  (M.getLangOpts().HIP && FD->hasAttr())) &&
   (M.getTriple().getOS() == llvm::Triple::AMDHSA))
 F->addFnAttr("amdgpu-implicitarg-num-bytes", "48");
 


Index: test/CodeGenHIP/implicit-kernarg.cpp
===
--- /dev/null
+++ test/CodeGenHIP/implicit-kernarg.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -emit-llvm -x hip -o - %s | FileCheck %s
+#include "Inputs/hip.h"
+
+__global__ void hip_kernel_temp() {
+}
+
+// CHECK-DAG: attributes #0 = { noinline nounwind optnone "amdgpu-implicitarg-num-bytes"="48"
Index: test/CodeGenHIP/Inputs/hip.h
===
--- /dev/null
+++ test/CodeGenHIP/Inputs/hip.h
@@ -0,0 +1,3 @@
+/* Minimal declarations for HIP support.  Testing purposes only. */
+
+#define __global__ __attribute__((global))
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -7853,7 +7853,8 @@
   const auto *ReqdWGS = M.getLangOpts().OpenCL ?
 FD->getAttr() : nullptr;
 
-  if (M.getLangOpts().OpenCL && FD->hasAttr() &&
+  if (((M.getLangOpts().OpenCL && FD->hasAttr()) ||
+  (M.getLangOpts().HIP && FD->hasAttr())) &&
   (M.getTriple().getOS() == llvm::Triple::AMDHSA))
 F->addFnAttr("amdgpu-implicitarg-num-bytes", "48");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59415: Do not resolve directory junctions for `-fdiagnostics-absolute-paths` on Windows.

[Hans Wennborg via Phabricator via cfe-commits]

hans added inline comments.



Comment at: lib/Frontend/TextDiagnostic.cpp:768
 if (Dir) {
+#ifdef _WIN32
+  SmallString<4096> DirName = Dir->getName();

I think this requires a comment explaining why Windows is handled differently.


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

https://reviews.llvm.org/D59415



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

[PATCH] D62115: fix a issue that clang is incompatible with gcc with -H option.

[Kan Shengchen via Phabricator via cfe-commits]

skan added a comment.

In D62115#1511407 , @kimgr wrote:

> Also, consider `././Inputs/empty.h`.


Firstly, on linux
write the `clang_H_opt.c` file as

  #include "../Index/Inputs/empty.h"
  #include "Inputs/empty.h"
  #include "./Inputs/empty.h"
  #include "././Inputs/empty.h"

Then compile it with command `gcc -H -fsyntax-only clang_H_opt.c 2>&1`, 
output:

  . ../Index/Inputs/empty.h
  . Inputs/empty.h
  . ./Inputs/empty.h
  . ././Inputs/empty.h

compile it with command `clang -H -fsyntax-only clang_H_opt.c 2>&1` will get 
the same result . So i think there is no need to simplify the existing pathname 
in `#include` directive.

Secondly, on Win2019
write the `clang_H_opt.c` file as

  #include "..\Index\Inputs\empty.h"
  #include "Inputs/empty.h"
  #include ".\Inputs/empty.h"
  #include ".\.\Inputs/empty.h"

compile it with clang,
output:

  . ..\\Index\\Inputs\\empty.h
  . Inputs\\empty.h
  . .\\Inputs\\empty.h
  . .\\.\\Inputs\\empty.h
  . .\\.\\Inputs\\empty.h

i think the output is appropriate.


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

https://reviews.llvm.org/D62115



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

r361372 - [ARM][AArch64] Fix incorrect handling of alignment in va_arg code generation

[John Brawn via cfe-commits]

Author: john.brawn
Date: Wed May 22 04:42:54 2019
New Revision: 361372

URL: http://llvm.org/viewvc/llvm-project?rev=361372&view=rev
Log:
[ARM][AArch64] Fix incorrect handling of alignment in va_arg code generation

Overaligned and underaligned types (i.e. types where the alignment has been
increased or decreased using the aligned and packed attributes) weren't being
correctly handled in all cases, as the unadjusted alignment should be used.

This patch also adjusts getTypeUnadjustedAlign to correctly handle typedefs of
non-aggregate types, which it appears it never had to handle before.

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

Added:
cfe/trunk/test/CodeGen/arm-varargs.c
Modified:
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/CodeGen/TargetInfo.cpp
cfe/trunk/test/CodeGen/aarch64-varargs.c

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=361372&r1=361371&r2=361372&view=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Wed May 22 04:42:54 2019
@@ -2130,7 +2130,7 @@ unsigned ASTContext::getTypeUnadjustedAl
 const ASTRecordLayout &Layout = 
getASTObjCInterfaceLayout(ObjCI->getDecl());
 UnadjustedAlign = toBits(Layout.getUnadjustedAlignment());
   } else {
-UnadjustedAlign = getTypeAlign(T);
+UnadjustedAlign = getTypeAlign(T->getUnqualifiedDesugaredType());
   }
 
   MemoizedUnadjustedAlign[T] = UnadjustedAlign;

Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=361372&r1=361371&r2=361372&view=diff
==
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Wed May 22 04:42:54 2019
@@ -5278,13 +5278,13 @@ Address AArch64ABIInfo::EmitAAPCSVAArg(A
   llvm::BasicBlock *OnStackBlock = CGF.createBasicBlock("vaarg.on_stack");
   llvm::BasicBlock *ContBlock = CGF.createBasicBlock("vaarg.end");
 
-  auto TyInfo = getContext().getTypeInfoInChars(Ty);
-  CharUnits TyAlign = TyInfo.second;
+  CharUnits TySize = getContext().getTypeSizeInChars(Ty);
+  CharUnits TyAlign = getContext().getTypeUnadjustedAlignInChars(Ty);
 
   Address reg_offs_p = Address::invalid();
   llvm::Value *reg_offs = nullptr;
   int reg_top_index;
-  int RegSize = IsIndirect ? 8 : TyInfo.first.getQuantity();
+  int RegSize = IsIndirect ? 8 : TySize.getQuantity();
   if (!IsFPR) {
 // 3 is the field number of __gr_offs
 reg_offs_p = CGF.Builder.CreateStructGEP(VAListAddr, 3, "gr_offs_p");
@@ -5412,8 +5412,8 @@ Address AArch64ABIInfo::EmitAAPCSVAArg(A
 CharUnits SlotSize = BaseAddr.getAlignment();
 if (CGF.CGM.getDataLayout().isBigEndian() && !IsIndirect &&
 (IsHFA || !isAggregateTypeForABI(Ty)) &&
-TyInfo.first < SlotSize) {
-  CharUnits Offset = SlotSize - TyInfo.first;
+TySize < SlotSize) {
+  CharUnits Offset = SlotSize - TySize;
   BaseAddr = CGF.Builder.CreateConstInBoundsByteGEP(BaseAddr, Offset);
 }
 
@@ -5455,7 +5455,7 @@ Address AArch64ABIInfo::EmitAAPCSVAArg(A
   if (IsIndirect)
 StackSize = StackSlotSize;
   else
-StackSize = TyInfo.first.alignTo(StackSlotSize);
+StackSize = TySize.alignTo(StackSlotSize);
 
   llvm::Value *StackSizeC = CGF.Builder.getSize(StackSize);
   llvm::Value *NewStack =
@@ -5465,8 +5465,8 @@ Address AArch64ABIInfo::EmitAAPCSVAArg(A
   CGF.Builder.CreateStore(NewStack, stack_p);
 
   if (CGF.CGM.getDataLayout().isBigEndian() && !isAggregateTypeForABI(Ty) &&
-  TyInfo.first < StackSlotSize) {
-CharUnits Offset = StackSlotSize - TyInfo.first;
+  TySize < StackSlotSize) {
+CharUnits Offset = StackSlotSize - TySize;
 OnStackAddr = CGF.Builder.CreateConstInBoundsByteGEP(OnStackAddr, Offset);
   }
 
@@ -5484,7 +5484,7 @@ Address AArch64ABIInfo::EmitAAPCSVAArg(A
 
   if (IsIndirect)
 return Address(CGF.Builder.CreateLoad(ResAddr, "vaarg.addr"),
-   TyInfo.second);
+   TyAlign);
 
   return ResAddr;
 }
@@ -6210,19 +6210,19 @@ Address ARMABIInfo::EmitVAArg(CodeGenFun
 return Addr;
   }
 
-  auto TyInfo = getContext().getTypeInfoInChars(Ty);
-  CharUnits TyAlignForABI = TyInfo.second;
+  CharUnits TySize = getContext().getTypeSizeInChars(Ty);
+  CharUnits TyAlignForABI = getContext().getTypeUnadjustedAlignInChars(Ty);
 
   // Use indirect if size of the illegal vector is bigger than 16 bytes.
   bool IsIndirect = false;
   const Type *Base = nullptr;
   uint64_t Members = 0;
-  if (TyInfo.first > CharUnits::fromQuantity(16) && isIllegalVectorType(Ty)) {
+  if (TySize > CharUnits::fromQuantity(16) && isIllegalVectorType(Ty)) {
 IsIndirect = true;
 
   // ARMv7k passes structs bigger than 16 bytes indirectly, in space
   // allocated by the caller.
-  } else if (TyInfo.first

[PATCH] D62238: [CodeComplete] Complete a lambda when preferred type is a function

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4135
+!PotentialTemplateName.getAsIdentifierInfo()->getName().contains(
+"function"))
+  return nullptr;

This looks cheesy, do we really want to perform this operation only for 
templates with `"function"` in their names?

As discussed offline maybe perform this for any template with a 
single(non-defaulted?) argument, or maybe even better perform a type-check as 
suggested by you. But I believe there would be too many false positives with 
the current state.



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4154
+   Results.getCodeCompletionTUInfo());
+  // []() {}
+  Completion.AddChunk(CodeCompletionString::CK_LeftBracket);

maybe also add a placeholder for captures?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62238



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

[PATCH] D62152: [ARM][AArch64] Fix incorrect handling of alignment in va_arg code generation

[John Brawn via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC361372: [ARM][AArch64] Fix incorrect handling of alignment 
in va_arg code generation (authored by john.brawn, committed by ).

Repository:
  rC Clang

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

https://reviews.llvm.org/D62152

Files:
  lib/AST/ASTContext.cpp
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/aarch64-varargs.c
  test/CodeGen/arm-varargs.c

Index: test/CodeGen/aarch64-varargs.c
===
--- test/CodeGen/aarch64-varargs.c
+++ test/CodeGen/aarch64-varargs.c
@@ -235,6 +235,653 @@
 // CHECK: [[ADDR:%[a-z._0-9]+]] = phi %struct.hfa* [ [[FROMREG_ADDR]], %[[VAARG_IN_REG]] ], [ [[FROMSTACK_ADDR]], %[[VAARG_ON_STACK]] ]
 }
 
+// Over and under alignment on fundamental types has no effect on parameter
+// passing, so the code generated for va_arg should be the same as for
+// non-aligned fundamental types.
+
+typedef int underaligned_int __attribute__((packed,aligned(2)));
+underaligned_int underaligned_int_test() {
+// CHECK-LABEL: define i32 @underaligned_int_test()
+  return va_arg(the_list, underaligned_int);
+// CHECK: [[GR_OFFS:%[a-z_0-9]+]] = load i32, i32* getelementptr inbounds (%struct.__va_list, %struct.__va_list* @the_list, i32 0, i32 3)
+// CHECK: [[EARLY_ONSTACK:%[a-z_0-9]+]] = icmp sge i32 [[GR_OFFS]], 0
+// CHECK: br i1 [[EARLY_ONSTACK]], label %[[VAARG_ON_STACK:[a-z_.0-9]+]], label %[[VAARG_MAYBE_REG:[a-z_.0-9]+]]
+
+// CHECK: [[VAARG_MAYBE_REG]]
+// CHECK: [[NEW_REG_OFFS:%[a-z_0-9]+]] = add i32 [[GR_OFFS]], 8
+// CHECK: store i32 [[NEW_REG_OFFS]], i32* getelementptr inbounds (%struct.__va_list, %struct.__va_list* @the_list, i32 0, i32 3)
+// CHECK: [[INREG:%[a-z_0-9]+]] = icmp sle i32 [[NEW_REG_OFFS]], 0
+// CHECK: br i1 [[INREG]], label %[[VAARG_IN_REG:[a-z_.0-9]+]], label %[[VAARG_ON_STACK]]
+
+// CHECK: [[VAARG_IN_REG]]
+// CHECK: [[REG_TOP:%[a-z_0-9]+]] = load i8*, i8** getelementptr inbounds (%struct.__va_list, %struct.__va_list* @the_list, i32 0, i32 1)
+// CHECK: [[REG_ADDR:%[a-z_0-9]+]] = getelementptr inbounds i8, i8* [[REG_TOP]], i32 [[GR_OFFS]]
+// CHECK-BE: [[REG_ADDR_ALIGNED:%[0-9]+]] = getelementptr inbounds i8, i8* [[REG_ADDR]], i64 4
+// CHECK-BE: [[FROMREG_ADDR:%[a-z_0-9]+]] = bitcast i8* [[REG_ADDR_ALIGNED]] to i32*
+// CHECK-LE: [[FROMREG_ADDR:%[a-z_0-9]+]] = bitcast i8* [[REG_ADDR]] to i32*
+// CHECK: br label %[[VAARG_END:[a-z._0-9]+]]
+
+// CHECK: [[VAARG_ON_STACK]]
+// CHECK: [[STACK:%[a-z_0-9]+]] = load i8*, i8** getelementptr inbounds (%struct.__va_list, %struct.__va_list* @the_list, i32 0, i32 0)
+// CHECK: [[NEW_STACK:%[a-z_0-9]+]] = getelementptr inbounds i8, i8* [[STACK]], i64 8
+// CHECK: store i8* [[NEW_STACK]], i8** getelementptr inbounds (%struct.__va_list, %struct.__va_list* @the_list, i32 0, i32 0)
+// CHECK-BE: [[STACK_ALIGNED:%[a-z_0-9]*]] = getelementptr inbounds i8, i8* [[STACK]], i64 4
+// CHECK-BE: [[FROMSTACK_ADDR:%[a-z_0-9]+]] = bitcast i8* [[STACK_ALIGNED]] to i32*
+// CHECK-LE: [[FROMSTACK_ADDR:%[a-z_0-9]+]] = bitcast i8* [[STACK]] to i32*
+// CHECK: br label %[[VAARG_END]]
+
+// CHECK: [[VAARG_END]]
+// CHECK: [[ADDR:%[a-z._0-9]+]] = phi i32* [ [[FROMREG_ADDR]], %[[VAARG_IN_REG]] ], [ [[FROMSTACK_ADDR]], %[[VAARG_ON_STACK]] ]
+// CHECK: [[RESULT:%[a-z_0-9]+]] = load i32, i32* [[ADDR]]
+// CHECK: ret i32 [[RESULT]]
+}
+
+typedef int overaligned_int __attribute__((aligned(32)));
+overaligned_int overaligned_int_test() {
+// CHECK-LABEL: define i32 @overaligned_int_test()
+  return va_arg(the_list, overaligned_int);
+// CHECK: [[GR_OFFS:%[a-z_0-9]+]] = load i32, i32* getelementptr inbounds (%struct.__va_list, %struct.__va_list* @the_list, i32 0, i32 3)
+// CHECK: [[EARLY_ONSTACK:%[a-z_0-9]+]] = icmp sge i32 [[GR_OFFS]], 0
+// CHECK: br i1 [[EARLY_ONSTACK]], label %[[VAARG_ON_STACK:[a-z_.0-9]+]], label %[[VAARG_MAYBE_REG:[a-z_.0-9]+]]
+
+// CHECK: [[VAARG_MAYBE_REG]]
+// CHECK: [[NEW_REG_OFFS:%[a-z_0-9]+]] = add i32 [[GR_OFFS]], 8
+// CHECK: store i32 [[NEW_REG_OFFS]], i32* getelementptr inbounds (%struct.__va_list, %struct.__va_list* @the_list, i32 0, i32 3)
+// CHECK: [[INREG:%[a-z_0-9]+]] = icmp sle i32 [[NEW_REG_OFFS]], 0
+// CHECK: br i1 [[INREG]], label %[[VAARG_IN_REG:[a-z_.0-9]+]], label %[[VAARG_ON_STACK]]
+
+// CHECK: [[VAARG_IN_REG]]
+// CHECK: [[REG_TOP:%[a-z_0-9]+]] = load i8*, i8** getelementptr inbounds (%struct.__va_list, %struct.__va_list* @the_list, i32 0, i32 1)
+// CHECK: [[REG_ADDR:%[a-z_0-9]+]] = getelementptr inbounds i8, i8* [[REG_TOP]], i32 [[GR_OFFS]]
+// CHECK-BE: [[REG_ADDR_ALIGNED:%[0-9]+]] = getelementptr inbounds i8, i8* [[REG_ADDR]], i64 4
+// CHECK-BE: [[FROMREG_ADDR:%[a-z_0-9]+]] = bitcast i8* [[REG_ADDR_ALIGNED]] to i32*
+// CHECK-LE: [[FROMREG_ADDR:%[a-z_0-9]+]] = bitcast i8* [[REG_ADDR]] to i32*
+// CHECK: br label %[[VAARG_END:[a-z._0-9]+]]
+
+// CHECK: [[VAARG_ON_STACK]]
+// CHECK: [[STACK:%[a-z_0-9]+]] = load i8*, i8** getelementptr inbo

Re: [PATCH] D61861: DeleteNullPointerCheck now deletes until the end brace of the condition

[Jonathan Camilleri via cfe-commits]

Request to commit patch.

On Tue, May 14, 2019 at 10:56 AM Mads Ravn via Phabricator <
revi...@reviews.llvm.org> wrote:

> madsravn added a comment.
>
> In D61861#1500900 , @J-Camilleri
> wrote:
>
> > In D61861#1500868 , @madsravn
> wrote:
> >
> > > Looks good to me :)
> >
> >
> > Thanks for the review and suggestion, who should I contact now in order
> to push the patch?
>
>
> I asked around and I was told that we should just leave a comment and then
> somebody will come around. I was also told that after a few patches you can
> obtain commit access if you want to push stuff yourself.
>
> someone please commit this.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D61861/new/
>
> https://reviews.llvm.org/D61861
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r361374 - Mark tests from r361278 as unsupported on Windows.

[Russell Gallop via cfe-commits]

Author: russell_gallop
Date: Wed May 22 05:07:52 2019
New Revision: 361374

URL: http://llvm.org/viewvc/llvm-project?rev=361374&view=rev
Log:
Mark tests from r361278 as unsupported on Windows.

Modified:
cfe/trunk/test/Driver/darwin-header-search-libcxx.cpp
cfe/trunk/test/Driver/darwin-header-search-libstdcxx.cpp
cfe/trunk/test/Driver/darwin-header-search-system.cpp

Modified: cfe/trunk/test/Driver/darwin-header-search-libcxx.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/darwin-header-search-libcxx.cpp?rev=361374&r1=361373&r2=361374&view=diff
==
--- cfe/trunk/test/Driver/darwin-header-search-libcxx.cpp (original)
+++ cfe/trunk/test/Driver/darwin-header-search-libcxx.cpp Wed May 22 05:07:52 
2019
@@ -1,3 +1,5 @@
+// UNSUPPORTED: system-windows
+
 // General tests that the header search paths for libc++ detected by the driver
 // and passed to CC1 are correct on Darwin platforms.
 

Modified: cfe/trunk/test/Driver/darwin-header-search-libstdcxx.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/darwin-header-search-libstdcxx.cpp?rev=361374&r1=361373&r2=361374&view=diff
==
--- cfe/trunk/test/Driver/darwin-header-search-libstdcxx.cpp (original)
+++ cfe/trunk/test/Driver/darwin-header-search-libstdcxx.cpp Wed May 22 
05:07:52 2019
@@ -1,3 +1,5 @@
+// UNSUPPORTED: system-windows
+
 // General tests that the header search paths for libstdc++ detected by the
 // driver and passed to CC1 are correct on Darwin platforms.
 

Modified: cfe/trunk/test/Driver/darwin-header-search-system.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/darwin-header-search-system.cpp?rev=361374&r1=361373&r2=361374&view=diff
==
--- cfe/trunk/test/Driver/darwin-header-search-system.cpp (original)
+++ cfe/trunk/test/Driver/darwin-header-search-system.cpp Wed May 22 05:07:52 
2019
@@ -1,3 +1,5 @@
+// UNSUPPORTED: system-windows
+
 // General tests that the system header search paths detected by the driver
 // and passed to CC1 are correct on Darwin platforms.
 


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

[PATCH] D61939: AArch64: add support for arm64_23 (ILP32) IR generation

[Tim Northover via Phabricator via cfe-commits]

t.p.northover updated this revision to Diff 200707.
t.p.northover added a comment.

During upstreaming we've changed from detecting an "arm64_32" ArchName to using 
a Triple::aarch64_32 Arch. We recently discovered a bug that meant only AArch32 
NEON types were permitted, which is fixed in this new revision.

Also, ping.


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

https://reviews.llvm.org/D61939

Files:
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/arm64_32-vaarg.c
  clang/test/CodeGen/arm64_32.c
  clang/test/CodeGen/builtins-arm64.c
  clang/test/CodeGen/target-data.c
  clang/test/CodeGenCXX/armv7k.cpp
  clang/test/Driver/aarch64-cpus.c
  clang/test/Driver/arm64_32-link.c
  clang/test/Preprocessor/aarch64-target-features.c
  clang/test/Preprocessor/arm64_32.c
  clang/test/Preprocessor/init-v7k-compat.c
  clang/test/Preprocessor/stdint.c
  clang/test/Sema/aarch64-neon-vector-types.c
  clang/test/Sema/types.c

Index: clang/test/Sema/types.c
===
--- clang/test/Sema/types.c
+++ clang/test/Sema/types.c
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 %s -fblocks -pedantic -verify -triple=mips64-linux-gnu
 // RUN: %clang_cc1 %s -fblocks -pedantic -verify -triple=x86_64-unknown-linux
 // RUN: %clang_cc1 %s -fblocks -pedantic -verify -triple=x86_64-unknown-linux-gnux32
+// RUN: %clang_cc1 %s -fblocks -pedantic -pedantic -verify -triple=arm64_32-apple-ios7.0
 
 // rdar://6097662
 typedef int (*T)[2];
Index: clang/test/Sema/aarch64-neon-vector-types.c
===
--- clang/test/Sema/aarch64-neon-vector-types.c
+++ clang/test/Sema/aarch64-neon-vector-types.c
@@ -3,6 +3,8 @@
 // RUN: %clang_cc1 %s -triple arm64-none-linux-gnu -target-feature +neon -fsyntax-only -verify
 // RUN: %clang_cc1 %s -triple arm64-none-linux-gnu -target-feature +neon -DUSE_LONG -fsyntax-only -verify
 
+// RUN: %clang_cc1 %s -triple arm64_32-apple-ios -target-feature +neon -fsyntax-only -verify
+
 typedef float float32_t;
 typedef unsigned char poly8_t;
 typedef unsigned short poly16_t;
Index: clang/test/Preprocessor/stdint.c
===
--- clang/test/Preprocessor/stdint.c
+++ clang/test/Preprocessor/stdint.c
@@ -105,6 +105,113 @@
 // ARM:INTMAX_C_(0) 0LL
 // ARM:UINTMAX_C_(0) 0ULL
 //
+// RUN: %clang_cc1 -E -ffreestanding -triple=arm64_32-apple-ios7.0 %s | FileCheck -check-prefix ARM64_32 %s
+//
+// ARM64_32:typedef long long int int64_t;
+// ARM64_32:typedef long long unsigned int uint64_t;
+// ARM64_32:typedef int64_t int_least64_t;
+// ARM64_32:typedef uint64_t uint_least64_t;
+// ARM64_32:typedef int64_t int_fast64_t;
+// ARM64_32:typedef uint64_t uint_fast64_t;
+//
+// ARM64_32:typedef int int32_t;
+// ARM64_32:typedef unsigned int uint32_t;
+// ARM64_32:typedef int32_t int_least32_t;
+// ARM64_32:typedef uint32_t uint_least32_t;
+// ARM64_32:typedef int32_t int_fast32_t;
+// ARM64_32:typedef uint32_t uint_fast32_t;
+// 
+// ARM64_32:typedef short int16_t;
+// ARM64_32:typedef unsigned short uint16_t;
+// ARM64_32:typedef int16_t int_least16_t;
+// ARM64_32:typedef uint16_t uint_least16_t;
+// ARM64_32:typedef int16_t int_fast16_t;
+// ARM64_32:typedef uint16_t uint_fast16_t;
+//
+// ARM64_32:typedef signed char int8_t;
+// ARM64_32:typedef unsigned char uint8_t;
+// ARM64_32:typedef int8_t int_least8_t;
+// ARM64_32:typedef uint8_t uint_least8_t;
+// ARM64_32:typedef int8_t int_fast8_t;
+// ARM64_32:typedef uint8_t uint_fast8_t;
+//
+// ARM64_32:typedef long int intptr_t;
+// ARM64_32:typedef long unsigned int uintptr_t;
+// 
+// ARM64_32:typedef long long int intmax_t;
+// ARM64_32:typedef long long unsigned int uintmax_t;
+//
+// ARM64_32:INT8_MAX_ 127
+// ARM64_32:INT8_MIN_ (-127 -1)
+// ARM64_32:UINT8_MAX_ 255
+// ARM64_32:INT_LEAST8_MIN_ (-127 -1)
+// ARM64_32:INT_LEAST8_MAX_ 127
+// ARM64_32:UINT_LEAST8_MAX_ 255
+// ARM64_32:INT_FAST8_MIN_ (-127 -1)
+// ARM64_32:INT_FAST8_MAX_ 127
+// ARM64_32:UINT_FAST8_MAX_ 255
+//
+// ARM64_32:INT16_MAX_ 32767
+// ARM64_32:INT16_MIN_ (-32767 -1)
+// ARM64_32:UINT16_MAX_ 65535
+// ARM64_32:INT_LEAST16_MIN_ (-32767 -1)
+// ARM64_32:INT_LEAST16_MAX_ 32767
+// ARM64_32:UINT_LEAST16_MAX_ 65535
+// ARM64_32:INT_FAST16_MIN_ (-32767 -1)
+// ARM64_32:INT_FAST16_MAX_ 32767
+// ARM64_32:UINT_FAST16_MAX_ 65535
+//
+// ARM64_32:INT32_MAX_ 2147483647
+// ARM64_32:INT32_MIN_ (-2147483647 -1)
+// ARM64_32:UINT32_MAX_ 4294967295U
+// ARM64_32:INT_LEAST32_MIN_ (-2147483647 -1)
+// ARM64_32:INT_LEAST32_MAX_ 2147483647
+// ARM64_32:UINT_LEAST32_MAX_ 4294967295U
+// A

[PATCH] D61939: AArch64: add support for arm64_23 (ILP32) IR generation

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

The subject has a typo, 23 instead of 32 ?


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

https://reviews.llvm.org/D61939



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

[PATCH] D61386: [clang-tidy] Add support writing a check as a Transformer rewrite rule.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel marked 4 inline comments as done.
ymandel added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp:46
+
+  StringRef Message = "no explanation";
+  if (Case.Explanation) {

ilya-biryukov wrote:
> The users will see this for every case without explanation, right?
> I'd expect the rules without explanation to be somewhat common, maybe don't 
> show any message at all in that case?
There's no option to call `diag()` without a message.  We could pass an empty 
string , but that may be confusing given the way the message is concatenated 
here:
https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp#L204

So, no matter what, there will be some message to go w/ the diagnostic. I 
figure that being explicit about the lack of explanation is better than an 
empty string, but don't feel strongly about this.



Comment at: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h:1
+//===-- TransformerClangTidyCheck.h - clang-tidy 
--===//
+//

ilya-biryukov wrote:
> NIT: maybe use `TransformerCheck` for brevity? I'm not a `clang-tidy` 
> maintainer, though, so not sure whether that aligns with the rest of the code.
It was TransformerTidy and Dmitri suggested I be explicit. I think 
TransformerCheck is better than TransformerTidy, but I also see the argument in 
spelling it out.  WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61386



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

[PATCH] D62174: [Analysis] Link library dependencies to Analysis plugins

[David Zarzycki via Phabricator via cfe-commits]

davezarzycki added a comment.

This breaks non-PIC builds. Was this planned or expected? Can we revert this 
until a fix is found?


Repository:
  rC Clang

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

https://reviews.llvm.org/D62174



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

[PATCH] D62149: [LibTooling] Update Transformer to use RangeSelector instead of NodePart enum.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 200712.
ymandel marked an inline comment as done.
ymandel added a comment.

Updated comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62149

Files:
  clang/include/clang/Tooling/Refactoring/Transformer.h
  clang/lib/Tooling/Refactoring/Transformer.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -7,8 +7,8 @@
 //===--===//
 
 #include "clang/Tooling/Refactoring/Transformer.h"
-
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/Refactoring/RangeSelector.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
@@ -147,7 +147,7 @@
   on(expr(hasType(isOrPointsTo(StringType)))
  .bind(StringExpr)),
   callee(cxxMethodDecl(hasName("c_str")),
-  change("REPLACED"));
+  change(text("REPLACED")));
   R.Cases[0].Explanation = text("Use size() method directly on string.");
   return R;
 }
@@ -183,7 +183,7 @@
 hasName("proto::ProtoCommandLineFlag"
.bind(Flag)),
 unless(callee(cxxMethodDecl(hasName("GetProto"),
-  change(Flag, "EXPR"));
+  change(node(Flag), text("EXPR")));
 
   std::string Input = R"cc(
 proto::ProtoCommandLineFlag flag;
@@ -201,9 +201,8 @@
 
 TEST_F(TransformerTest, NodePartNameNamedDecl) {
   StringRef Fun = "fun";
-  RewriteRule Rule =
-  makeRule(functionDecl(hasName("bad")).bind(Fun),
-   change(Fun, NodePart::Name, "good"));
+  RewriteRule Rule = makeRule(functionDecl(hasName("bad")).bind(Fun),
+  change(name(Fun), text("good")));
 
   std::string Input = R"cc(
 int bad(int x);
@@ -235,7 +234,7 @@
 
   StringRef Ref = "ref";
   testRule(makeRule(declRefExpr(to(functionDecl(hasName("bad".bind(Ref),
-change(Ref, NodePart::Name, "good")),
+change(name(Ref), text("good"))),
Input, Expected);
 }
 
@@ -253,7 +252,7 @@
 
   StringRef Ref = "ref";
   Transformer T(makeRule(declRefExpr(to(functionDecl())).bind(Ref),
- change(Ref, NodePart::Name, "good")),
+ change(name(Ref), text("good"))),
 consumer());
   T.registerMatchers(&MatchFinder);
   EXPECT_FALSE(rewrite(Input));
@@ -262,7 +261,7 @@
 TEST_F(TransformerTest, NodePartMember) {
   StringRef E = "expr";
   RewriteRule Rule = makeRule(memberExpr(member(hasName("bad"))).bind(E),
-  change(E, NodePart::Member, "good"));
+  change(member(E), text("good")));
 
   std::string Input = R"cc(
 struct S {
@@ -315,8 +314,7 @@
   )cc";
 
   StringRef E = "expr";
-  testRule(makeRule(memberExpr().bind(E),
-change(E, NodePart::Member, "good")),
+  testRule(makeRule(memberExpr().bind(E), change(member(E), text("good"))),
Input, Expected);
 }
 
@@ -348,7 +346,7 @@
 
   StringRef MemExpr = "member";
   testRule(makeRule(memberExpr().bind(MemExpr),
-change(MemExpr, NodePart::Member, "good")),
+change(member(MemExpr), text("good"))),
Input, Expected);
 }
 
@@ -371,8 +369,9 @@
   StringRef C = "C", T = "T", E = "E";
   testRule(makeRule(ifStmt(hasCondition(expr().bind(C)),
hasThen(stmt().bind(T)), hasElse(stmt().bind(E))),
-{change(C, "true"), change(T, "{ /* then */ }"),
- change(E, "{ /* else */ }")}),
+{change(node(C), text("true")),
+ change(statement(T), text("{ /* then */ }")),
+ change(statement(E), text("{ /* else */ }"))}),
Input, Expected);
 }
 
@@ -383,7 +382,7 @@
 hasName("proto::ProtoCommandLineFlag"
.bind(Flag)),
 unless(callee(cxxMethodDecl(hasName("GetProto"),
-  change(Flag, "PROTO"));
+  change(node(Flag), text("PROTO")));
 
   std::string Input = R"cc(
 proto::ProtoCommandLineFlag flag;
@@ -410,7 +409,7 @@
hasArgument(0, cxxMemberCallExpr(
   on(expr().bind(S)),
   callee(cxxMethodDecl(hasName("c_str")),
-  change("DISTINCT"));
+  change(text("DISTINCT")));
 }
 
 TEST_F(TransformerTest, OrderedRuleRelated) {
@@ -475,7 +474,7 @@
   -> llvm::Expected {
 return llvm::createStringError(llvm::errc::inval

r361376 - [Frontend] Return an error on bad inputs to PrecompiledPreabmle

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Wed May 22 05:50:01 2019
New Revision: 361376

URL: http://llvm.org/viewvc/llvm-project?rev=361376&view=rev
Log:
[Frontend] Return an error on bad inputs to PrecompiledPreabmle

Summary:
Instead of failing with assertions. Fixes a crash found by oss-fuzz:
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=12865

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h
cfe/trunk/lib/Frontend/ASTUnit.cpp
cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp

Modified: cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h?rev=361376&r1=361375&r2=361376&view=diff
==
--- cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h (original)
+++ cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h Wed May 22 05:50:01 
2019
@@ -291,7 +291,8 @@ enum class BuildPreambleError {
   CouldntCreateTempFile = 1,
   CouldntCreateTargetInfo,
   BeginSourceFileFailed,
-  CouldntEmitPCH
+  CouldntEmitPCH,
+  BadInputs
 };
 
 class BuildPreambleErrorCategory final : public std::error_category {

Modified: cfe/trunk/lib/Frontend/ASTUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTUnit.cpp?rev=361376&r1=361375&r2=361376&view=diff
==
--- cfe/trunk/lib/Frontend/ASTUnit.cpp (original)
+++ cfe/trunk/lib/Frontend/ASTUnit.cpp Wed May 22 05:50:01 2019
@@ -1368,6 +1368,7 @@ ASTUnit::getMainBufferWithPrecompiledPre
   case BuildPreambleError::CouldntCreateTargetInfo:
   case BuildPreambleError::BeginSourceFileFailed:
   case BuildPreambleError::CouldntEmitPCH:
+  case BuildPreambleError::BadInputs:
 // These erros are more likely to repeat, retry after some period.
 PreambleRebuildCountdown = DefaultPreambleRebuildInterval;
 return nullptr;

Modified: cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp?rev=361376&r1=361375&r2=361376&view=diff
==
--- cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp (original)
+++ cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp Wed May 22 05:50:01 2019
@@ -299,14 +299,13 @@ llvm::ErrorOr Preco
   // created. This complexity should be lifted elsewhere.
   Clang->getTarget().adjust(Clang->getLangOpts());
 
-  assert(Clang->getFrontendOpts().Inputs.size() == 1 &&
- "Invocation must have exactly one source file!");
-  assert(Clang->getFrontendOpts().Inputs[0].getKind().getFormat() ==
- InputKind::Source &&
- "FIXME: AST inputs not yet supported here!");
-  assert(Clang->getFrontendOpts().Inputs[0].getKind().getLanguage() !=
- InputKind::LLVM_IR &&
- "IR inputs not support here!");
+  if (Clang->getFrontendOpts().Inputs.size() != 1 ||
+  Clang->getFrontendOpts().Inputs[0].getKind().getFormat() !=
+  InputKind::Source ||
+  Clang->getFrontendOpts().Inputs[0].getKind().getLanguage() ==
+  InputKind::LLVM_IR) {
+return BuildPreambleError::BadInputs;
+  }
 
   // Clear out old caches and data.
   Diagnostics.Reset();
@@ -784,6 +783,8 @@ std::string BuildPreambleErrorCategory::
 return "BeginSourceFile() return an error";
   case BuildPreambleError::CouldntEmitPCH:
 return "Could not emit PCH";
+  case BuildPreambleError::BadInputs:
+return "Command line arguments must contain exactly one source file";
   }
   llvm_unreachable("unexpected BuildPreambleError");
 }


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

r361377 - Revert r361148 "[Syntax] Introduce TokenBuffer, start clangToolingSyntax library"

[Russell Gallop via cfe-commits]

Author: russell_gallop
Date: Wed May 22 05:50:52 2019
New Revision: 361377

URL: http://llvm.org/viewvc/llvm-project?rev=361377&view=rev
Log:
Revert r361148 "[Syntax] Introduce TokenBuffer, start clangToolingSyntax 
library"

Also reverted r361264 "[Syntax] Rename TokensTest to SyntaxTests. NFC"
which built on it. This is because there were hitting an assert on bots

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast

Removed:
cfe/trunk/include/clang/Tooling/Syntax/
cfe/trunk/lib/Tooling/Syntax/
cfe/trunk/unittests/Tooling/Syntax/
Modified:
cfe/trunk/lib/Tooling/CMakeLists.txt
cfe/trunk/unittests/Tooling/CMakeLists.txt

Modified: cfe/trunk/lib/Tooling/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/CMakeLists.txt?rev=361377&r1=361376&r2=361377&view=diff
==
--- cfe/trunk/lib/Tooling/CMakeLists.txt (original)
+++ cfe/trunk/lib/Tooling/CMakeLists.txt Wed May 22 05:50:52 2019
@@ -7,7 +7,6 @@ add_subdirectory(Core)
 add_subdirectory(Inclusions)
 add_subdirectory(Refactoring)
 add_subdirectory(ASTDiff)
-add_subdirectory(Syntax)
 
 add_clang_library(clangTooling
   AllTUsExecution.cpp

Modified: cfe/trunk/unittests/Tooling/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/CMakeLists.txt?rev=361377&r1=361376&r2=361377&view=diff
==
--- cfe/trunk/unittests/Tooling/CMakeLists.txt (original)
+++ cfe/trunk/unittests/Tooling/CMakeLists.txt Wed May 22 05:50:52 2019
@@ -72,6 +72,3 @@ target_link_libraries(ToolingTests
   clangToolingInclusions
   clangToolingRefactor
   )
-
-
-add_subdirectory(Syntax)


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

[PATCH] D62137: [Frontend] Return an error on bad inputs to PrecompiledPreabmle

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC361376: [Frontend] Return an error on bad inputs to 
PrecompiledPreabmle (authored by ibiryukov, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D62137?vs=200253&id=200713#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D62137

Files:
  include/clang/Frontend/PrecompiledPreamble.h
  lib/Frontend/ASTUnit.cpp
  lib/Frontend/PrecompiledPreamble.cpp


Index: include/clang/Frontend/PrecompiledPreamble.h
===
--- include/clang/Frontend/PrecompiledPreamble.h
+++ include/clang/Frontend/PrecompiledPreamble.h
@@ -291,7 +291,8 @@
   CouldntCreateTempFile = 1,
   CouldntCreateTargetInfo,
   BeginSourceFileFailed,
-  CouldntEmitPCH
+  CouldntEmitPCH,
+  BadInputs
 };
 
 class BuildPreambleErrorCategory final : public std::error_category {
Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -1368,6 +1368,7 @@
   case BuildPreambleError::CouldntCreateTargetInfo:
   case BuildPreambleError::BeginSourceFileFailed:
   case BuildPreambleError::CouldntEmitPCH:
+  case BuildPreambleError::BadInputs:
 // These erros are more likely to repeat, retry after some period.
 PreambleRebuildCountdown = DefaultPreambleRebuildInterval;
 return nullptr;
Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -299,14 +299,13 @@
   // created. This complexity should be lifted elsewhere.
   Clang->getTarget().adjust(Clang->getLangOpts());
 
-  assert(Clang->getFrontendOpts().Inputs.size() == 1 &&
- "Invocation must have exactly one source file!");
-  assert(Clang->getFrontendOpts().Inputs[0].getKind().getFormat() ==
- InputKind::Source &&
- "FIXME: AST inputs not yet supported here!");
-  assert(Clang->getFrontendOpts().Inputs[0].getKind().getLanguage() !=
- InputKind::LLVM_IR &&
- "IR inputs not support here!");
+  if (Clang->getFrontendOpts().Inputs.size() != 1 ||
+  Clang->getFrontendOpts().Inputs[0].getKind().getFormat() !=
+  InputKind::Source ||
+  Clang->getFrontendOpts().Inputs[0].getKind().getLanguage() ==
+  InputKind::LLVM_IR) {
+return BuildPreambleError::BadInputs;
+  }
 
   // Clear out old caches and data.
   Diagnostics.Reset();
@@ -784,6 +783,8 @@
 return "BeginSourceFile() return an error";
   case BuildPreambleError::CouldntEmitPCH:
 return "Could not emit PCH";
+  case BuildPreambleError::BadInputs:
+return "Command line arguments must contain exactly one source file";
   }
   llvm_unreachable("unexpected BuildPreambleError");
 }


Index: include/clang/Frontend/PrecompiledPreamble.h
===
--- include/clang/Frontend/PrecompiledPreamble.h
+++ include/clang/Frontend/PrecompiledPreamble.h
@@ -291,7 +291,8 @@
   CouldntCreateTempFile = 1,
   CouldntCreateTargetInfo,
   BeginSourceFileFailed,
-  CouldntEmitPCH
+  CouldntEmitPCH,
+  BadInputs
 };
 
 class BuildPreambleErrorCategory final : public std::error_category {
Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -1368,6 +1368,7 @@
   case BuildPreambleError::CouldntCreateTargetInfo:
   case BuildPreambleError::BeginSourceFileFailed:
   case BuildPreambleError::CouldntEmitPCH:
+  case BuildPreambleError::BadInputs:
 // These erros are more likely to repeat, retry after some period.
 PreambleRebuildCountdown = DefaultPreambleRebuildInterval;
 return nullptr;
Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -299,14 +299,13 @@
   // created. This complexity should be lifted elsewhere.
   Clang->getTarget().adjust(Clang->getLangOpts());
 
-  assert(Clang->getFrontendOpts().Inputs.size() == 1 &&
- "Invocation must have exactly one source file!");
-  assert(Clang->getFrontendOpts().Inputs[0].getKind().getFormat() ==
- InputKind::Source &&
- "FIXME: AST inputs not yet supported here!");
-  assert(Clang->getFrontendOpts().Inputs[0].getKind().getLanguage() !=
- InputKind::LLVM_IR &&
- "IR inputs not support here!");
+  if (Clang->getFrontendOpts().Inputs.size() != 1 ||
+  Clang->getFrontendOpts().Inputs[0].getKind().getFormat() !=
+  InputKind::Source ||
+  Clang->getFrontendOpts().Inputs[0].getKind().getLanguage() ==
+  InputKind::LLVM_IR) {
+

[PATCH] D62192: [clang-tidy]: Add cert-oop54-cpp alias for bugprone-unhandled-self-assignment

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas updated this revision to Diff 200714.
ztamas added a comment.

Fixed docs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62192

Files:
  clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.h
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-oop54-cpp.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment-warn-only-if-this-has-suspicious-field.cpp
  clang-tools-extra/test/clang-tidy/cert-oop54-cpp.cpp

Index: clang-tools-extra/test/clang-tidy/cert-oop54-cpp.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/cert-oop54-cpp.cpp
@@ -0,0 +1,16 @@
+// RUN: %check_clang_tidy %s cert-oop54-cpp %t
+
+// Test whether bugprone-unhandled-self-assignment.WarnOnlyIfThisHasSuspiciousField option is set correctly.
+class TrivialFields {
+public:
+  TrivialFields &operator=(const TrivialFields &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:18: warning: operator=() does not handle self-assignment properly [cert-oop54-cpp]
+return *this;
+  }
+
+private:
+  int m;
+  float f;
+  double d;
+  bool b;
+};
Index: clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment-warn-only-if-this-has-suspicious-field.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment-warn-only-if-this-has-suspicious-field.cpp
@@ -0,0 +1,41 @@
+// RUN: %check_clang_tidy %s bugprone-unhandled-self-assignment %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: bugprone-unhandled-self-assignment.WarnOnlyIfThisHasSuspiciousField, \
+// RUN:   value: 0}]}"
+
+// Classes with pointer field are still caught.
+class PtrField {
+public:
+  PtrField &operator=(const PtrField &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:13: warning: operator=() does not handle self-assignment properly [bugprone-unhandled-self-assignment]
+return *this;
+  }
+
+private:
+  int *p;
+};
+
+// With the option, check catches classes with trivial fields.
+class TrivialFields {
+public:
+  TrivialFields &operator=(const TrivialFields &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:18: warning: operator=() does not handle self-assignment properly [bugprone-unhandled-self-assignment]
+return *this;
+  }
+
+private:
+  int m;
+  float f;
+  double d;
+  bool b;
+};
+
+// The check warns also when there is no field at all.
+// In this case, user-defined copy assignment operator is useless anyway.
+class ClassWithoutFields {
+public:
+  ClassWithoutFields &operator=(const ClassWithoutFields &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:23: warning: operator=() does not handle self-assignment properly [bugprone-unhandled-self-assignment]
+return *this;
+  }
+};
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -98,6 +98,7 @@
cert-msc50-cpp
cert-msc51-cpp
cert-oop11-cpp (redirects to performance-move-constructor-init) 
+   cert-oop54-cpp (redirects to bugprone-unhandled-self-assignment) 
cppcoreguidelines-avoid-c-arrays (redirects to modernize-avoid-c-arrays) 
cppcoreguidelines-avoid-goto
cppcoreguidelines-avoid-magic-numbers (redirects to readability-magic-numbers) 
Index: clang-tools-extra/docs/clang-tidy/checks/cert-oop54-cpp.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cert-oop54-cpp.rst
@@ -0,0 +1,10 @@
+.. title:: clang-tidy - cert-oop54-cpp
+.. meta::
+   :http-equiv=refresh: 5;URL=bugprone-unhandled-self-assignment.html
+
+cert-oop54-cpp
+==
+
+The cert-oop54-cpp check is an alias, please see
+`bugprone-unhandled-self-assignment `_
+for more information.
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst
@@ -3,11 +3,14 @@
 bugprone-unhandled-self-assignment
 ==
 
+`cert-oop54-cpp` redirects here as an alias for this check. For the CERT alias,
+the `WarnOnlyIfThisHasSuspiciousField` option is set to `0`.
+
 Finds user-defined copy assignment operators which do not protect the code
 against self-assignment either by checking s

r361378 - Clang-formatting the header in advance of other planned changes; NFC.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Wed May 22 06:01:28 2019
New Revision: 361378

URL: http://llvm.org/viewvc/llvm-project?rev=361378&view=rev
Log:
Clang-formatting the header in advance of other planned changes; NFC.

Modified:
cfe/trunk/utils/TableGen/TableGenBackends.h

Modified: cfe/trunk/utils/TableGen/TableGenBackends.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/TableGenBackends.h?rev=361378&r1=361377&r2=361378&view=diff
==
--- cfe/trunk/utils/TableGen/TableGenBackends.h (original)
+++ cfe/trunk/utils/TableGen/TableGenBackends.h Wed May 22 06:01:28 2019
@@ -18,9 +18,9 @@
 #include 
 
 namespace llvm {
-  class raw_ostream;
-  class RecordKeeper;
-}
+class raw_ostream;
+class RecordKeeper;
+} // namespace llvm
 
 namespace clang {
 
@@ -28,22 +28,31 @@ void EmitClangDeclContext(llvm::RecordKe
 void EmitClangASTNodes(llvm::RecordKeeper &RK, llvm::raw_ostream &OS,
const std::string &N, const std::string &S);
 
-void EmitClangAttrParserStringSwitches(llvm::RecordKeeper &Records, 
llvm::raw_ostream &OS);
-void EmitClangAttrSubjectMatchRulesParserStringSwitches(llvm::RecordKeeper 
&Records,
-llvm::raw_ostream &OS);
+void EmitClangAttrParserStringSwitches(llvm::RecordKeeper &Records,
+   llvm::raw_ostream &OS);
+void EmitClangAttrSubjectMatchRulesParserStringSwitches(
+llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 void EmitClangAttrClass(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 void EmitClangAttrImpl(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 void EmitClangAttrList(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
-void EmitClangAttrSubjectMatchRuleList(llvm::RecordKeeper &Records, 
llvm::raw_ostream &OS);
+void EmitClangAttrSubjectMatchRuleList(llvm::RecordKeeper &Records,
+   llvm::raw_ostream &OS);
 void EmitClangAttrPCHRead(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 void EmitClangAttrPCHWrite(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
-void EmitClangAttrHasAttrImpl(llvm::RecordKeeper &Records, llvm::raw_ostream 
&OS);
-void EmitClangAttrSpellingListIndex(llvm::RecordKeeper &Records, 
llvm::raw_ostream &OS);
-void EmitClangAttrASTVisitor(llvm::RecordKeeper &Records, llvm::raw_ostream 
&OS);
-void EmitClangAttrTemplateInstantiate(llvm::RecordKeeper &Records, 
llvm::raw_ostream &OS);
-void EmitClangAttrParsedAttrList(llvm::RecordKeeper &Records, 
llvm::raw_ostream &OS);
-void EmitClangAttrParsedAttrImpl(llvm::RecordKeeper &Records, 
llvm::raw_ostream &OS);
-void EmitClangAttrParsedAttrKinds(llvm::RecordKeeper &Records, 
llvm::raw_ostream &OS);
+void EmitClangAttrHasAttrImpl(llvm::RecordKeeper &Records,
+  llvm::raw_ostream &OS);
+void EmitClangAttrSpellingListIndex(llvm::RecordKeeper &Records,
+llvm::raw_ostream &OS);
+void EmitClangAttrASTVisitor(llvm::RecordKeeper &Records,
+ llvm::raw_ostream &OS);
+void EmitClangAttrTemplateInstantiate(llvm::RecordKeeper &Records,
+  llvm::raw_ostream &OS);
+void EmitClangAttrParsedAttrList(llvm::RecordKeeper &Records,
+ llvm::raw_ostream &OS);
+void EmitClangAttrParsedAttrImpl(llvm::RecordKeeper &Records,
+ llvm::raw_ostream &OS);
+void EmitClangAttrParsedAttrKinds(llvm::RecordKeeper &Records,
+  llvm::raw_ostream &OS);
 void EmitClangAttrTextNodeDump(llvm::RecordKeeper &Records,
llvm::raw_ostream &OS);
 void EmitClangAttrNodeTraverse(llvm::RecordKeeper &Records,
@@ -52,16 +61,22 @@ void EmitClangAttrNodeTraverse(llvm::Rec
 void EmitClangDiagsDefs(llvm::RecordKeeper &Records, llvm::raw_ostream &OS,
 const std::string &Component);
 void EmitClangDiagGroups(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
-void EmitClangDiagsIndexName(llvm::RecordKeeper &Records, llvm::raw_ostream 
&OS);
+void EmitClangDiagsIndexName(llvm::RecordKeeper &Records,
+ llvm::raw_ostream &OS);
 
 void EmitClangSACheckers(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 
-void EmitClangCommentHTMLTags(llvm::RecordKeeper &Records, llvm::raw_ostream 
&OS);
-void EmitClangCommentHTMLTagsProperties(llvm::RecordKeeper &Records, 
llvm::raw_ostream &OS);
-void EmitClangCommentHTMLNamedCharacterReferences(llvm::RecordKeeper &Records, 
llvm::raw_ostream &OS);
-
-void EmitClangCommentCommandInfo(llvm::RecordKeeper &Records, 
llvm::raw_ostream &OS);
-void EmitClangCommentCommandList(llvm::RecordKeeper &Records, 
llvm::raw_ostream &OS);
+void EmitClangCommentHTMLTags(llvm::RecordKeeper &Records,
+  llvm::raw_ostream &OS);
+void EmitClangCommentHTMLTagsProperties(ll

r361379 - [CGOpenMPRuntime] emitX86DeclareSimdFunction - assert simdlen/cdtsize is not zero. NFCI.

[Simon Pilgrim via cfe-commits]

Author: rksimon
Date: Wed May 22 06:02:19 2019
New Revision: 361379

URL: http://llvm.org/viewvc/llvm-project?rev=361379&view=rev
Log:
[CGOpenMPRuntime] emitX86DeclareSimdFunction - assert simdlen/cdtsize is not 
zero. NFCI.

Fixes scan-build division by zero warning.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=361379&r1=361378&r2=361379&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Wed May 22 06:02:19 2019
@@ -9774,8 +9774,9 @@ emitX86DeclareSimdFunction(const Functio
   llvm::raw_svector_ostream Out(Buffer);
   Out << "_ZGV" << Data.ISA << Mask;
   if (!VLENVal) {
-Out << llvm::APSInt::getUnsigned(Data.VecRegSize /
- evaluateCDTSize(FD, ParamAttrs));
+unsigned NumElts = evaluateCDTSize(FD, ParamAttrs);
+assert(NumElts && "Non-zero simdlen/cdtsize expected");
+Out << llvm::APSInt::getUnsigned(Data.VecRegSize / NumElts);
   } else {
 Out << VLENVal;
   }


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

r361382 - [OpenCL] Support pipe keyword in C++ mode

[Sven van Haastregt via cfe-commits]

Author: svenvh
Date: Wed May 22 06:12:20 2019
New Revision: 361382

URL: http://llvm.org/viewvc/llvm-project?rev=361382&view=rev
Log:
[OpenCL] Support pipe keyword in C++ mode

Support the OpenCL C pipe feature in C++ for OpenCL mode, to preserve
backwards compatibility with OpenCL C.

Various changes had to be made in Parse and Sema to enable
pipe-specific diagnostics, so enable a SemaOpenCL test for C++.

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

Modified:
cfe/trunk/include/clang/Basic/TokenKinds.def
cfe/trunk/lib/Basic/Builtins.cpp
cfe/trunk/lib/Parse/ParseDecl.cpp
cfe/trunk/lib/Parse/ParseTentative.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/test/SemaOpenCL/invalid-pipes-cl2.0.cl

Modified: cfe/trunk/include/clang/Basic/TokenKinds.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/TokenKinds.def?rev=361382&r1=361381&r2=361382&view=diff
==
--- cfe/trunk/include/clang/Basic/TokenKinds.def (original)
+++ cfe/trunk/include/clang/Basic/TokenKinds.def Wed May 22 06:12:20 2019
@@ -568,7 +568,7 @@ KEYWORD(vec_step, KE
 // OpenMP Type Traits
 KEYWORD(__builtin_omp_required_simd_align, KEYALL)
 
-KEYWORD(pipe, KEYOPENCLC)
+KEYWORD(pipe, KEYOPENCLC | KEYOPENCLCXX)
 
 // Borland Extensions.
 KEYWORD(__pascal, KEYALL)

Modified: cfe/trunk/lib/Basic/Builtins.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Builtins.cpp?rev=361382&r1=361381&r2=361382&view=diff
==
--- cfe/trunk/lib/Basic/Builtins.cpp (original)
+++ cfe/trunk/lib/Basic/Builtins.cpp Wed May 22 06:12:20 2019
@@ -70,8 +70,9 @@ bool Builtin::Context::builtinIsSupporte
   bool ObjCUnsupported = !LangOpts.ObjC && BuiltinInfo.Langs == OBJC_LANG;
   bool OclC1Unsupported = (LangOpts.OpenCLVersion / 100) != 1 &&
   (BuiltinInfo.Langs & ALL_OCLC_LANGUAGES ) ==  
OCLC1X_LANG;
-  bool OclC2Unsupported = LangOpts.OpenCLVersion != 200 &&
-  (BuiltinInfo.Langs & ALL_OCLC_LANGUAGES) == 
OCLC20_LANG;
+  bool OclC2Unsupported =
+  (LangOpts.OpenCLVersion != 200 && !LangOpts.OpenCLCPlusPlus) &&
+  (BuiltinInfo.Langs & ALL_OCLC_LANGUAGES) == OCLC20_LANG;
   bool OclCUnsupported = !LangOpts.OpenCL &&
  (BuiltinInfo.Langs & ALL_OCLC_LANGUAGES);
   bool OpenMPUnsupported = !LangOpts.OpenMP && BuiltinInfo.Langs == OMP_LANG;

Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=361382&r1=361381&r2=361382&view=diff
==
--- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDecl.cpp Wed May 22 06:12:20 2019
@@ -2560,6 +2560,11 @@ bool Parser::ParseImplicitInt(DeclSpec &
 return false;
   }
 
+  // Early exit as Sema has a dedicated missing_actual_pipe_type diagnostic
+  // for incomplete declarations such as `pipe p`.
+  if (getLangOpts().OpenCLCPlusPlus && DS.isTypeSpecPipe())
+return false;
+
   if (getLangOpts().CPlusPlus &&
   DS.getStorageClassSpec() == DeclSpec::SCS_auto) {
 // Don't require a type specifier if we have the 'auto' storage class
@@ -3769,7 +3774,8 @@ void Parser::ParseDeclarationSpecifiers(
   isInvalid = DS.SetTypeAltiVecBool(true, Loc, PrevSpec, DiagID, Policy);
   break;
 case tok::kw_pipe:
-  if (!getLangOpts().OpenCL || (getLangOpts().OpenCLVersion < 200)) {
+  if (!getLangOpts().OpenCL || (getLangOpts().OpenCLVersion < 200 &&
+!getLangOpts().OpenCLCPlusPlus)) {
 // OpenCL 2.0 defined this keyword. OpenCL 1.2 and earlier should
 // support the "pipe" word as identifier.
 Tok.getIdentifierInfo()->revertTokenIDToIdentifier();
@@ -4896,7 +4902,8 @@ bool Parser::isDeclarationSpecifier(bool
   default: return false;
 
   case tok::kw_pipe:
-return getLangOpts().OpenCL && (getLangOpts().OpenCLVersion >= 200);
+return (getLangOpts().OpenCL && getLangOpts().OpenCLVersion >= 200) ||
+   getLangOpts().OpenCLCPlusPlus;
 
   case tok::identifier:   // foo::bar
 // Unfortunate hack to support "Class.factoryMethod" notation.
@@ -5384,7 +5391,8 @@ static bool isPtrOperatorToken(tok::Toke
   if (Kind == tok::star || Kind == tok::caret)
 return true;
 
-  if ((Kind == tok::kw_pipe) && Lang.OpenCL && (Lang.OpenCLVersion >= 200))
+  if (Kind == tok::kw_pipe &&
+  ((Lang.OpenCL && Lang.OpenCLVersion >= 200) || Lang.OpenCLCPlusPlus))
 return true;
 
   if (!Lang.CPlusPlus)

Modified: cfe/trunk/lib/Parse/ParseTentative.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseTentative.cpp?rev=361382&r1=361381&r2=361382&view=diff
===

[PATCH] D62181: [OpenCL] Support pipe keyword in C++ mode

[Sven van Haastregt via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC361382: [OpenCL] Support pipe keyword in C++ mode (authored 
by svenvh, committed by ).

Repository:
  rC Clang

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

https://reviews.llvm.org/D62181

Files:
  include/clang/Basic/TokenKinds.def
  lib/Basic/Builtins.cpp
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseTentative.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaType.cpp
  test/SemaOpenCL/invalid-pipes-cl2.0.cl

Index: include/clang/Basic/TokenKinds.def
===
--- include/clang/Basic/TokenKinds.def
+++ include/clang/Basic/TokenKinds.def
@@ -568,7 +568,7 @@
 // OpenMP Type Traits
 KEYWORD(__builtin_omp_required_simd_align, KEYALL)
 
-KEYWORD(pipe, KEYOPENCLC)
+KEYWORD(pipe, KEYOPENCLC | KEYOPENCLCXX)
 
 // Borland Extensions.
 KEYWORD(__pascal, KEYALL)
Index: test/SemaOpenCL/invalid-pipes-cl2.0.cl
===
--- test/SemaOpenCL/invalid-pipes-cl2.0.cl
+++ test/SemaOpenCL/invalid-pipes-cl2.0.cl
@@ -1,9 +1,10 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=c++
 
 global pipe int gp;// expected-error {{type '__global read_only pipe int' can only be used as a function parameter in OpenCL}}
 global reserve_id_t rid;  // expected-error {{the '__global reserve_id_t' type cannot be used to declare a program scope variable}}
 
-extern pipe write_only int get_pipe(); // expected-error {{type '__global write_only pipe int (void)' can only be used as a function parameter in OpenCL}}
+extern pipe write_only int get_pipe(); // expected-error-re{{type '__global write_only pipe int ({{(void)?}})' can only be used as a function parameter in OpenCL}}
 
 kernel void test_invalid_reserved_id(reserve_id_t ID) { // expected-error {{'reserve_id_t' cannot be used as the type of a kernel parameter}}
 }
@@ -35,6 +36,7 @@
 }
 
 // Tests ASTContext::mergeTypes rejects this.
+#ifndef __OPENCL_CPP_VERSION__
 int f(pipe int x, int y); // expected-note {{previous declaration is here}}
 int f(x, y) // expected-error {{conflicting types for 'f}}
 pipe short x;
@@ -42,3 +44,4 @@
 {
 return y;
 }
+#endif
Index: lib/Basic/Builtins.cpp
===
--- lib/Basic/Builtins.cpp
+++ lib/Basic/Builtins.cpp
@@ -70,8 +70,9 @@
   bool ObjCUnsupported = !LangOpts.ObjC && BuiltinInfo.Langs == OBJC_LANG;
   bool OclC1Unsupported = (LangOpts.OpenCLVersion / 100) != 1 &&
   (BuiltinInfo.Langs & ALL_OCLC_LANGUAGES ) ==  OCLC1X_LANG;
-  bool OclC2Unsupported = LangOpts.OpenCLVersion != 200 &&
-  (BuiltinInfo.Langs & ALL_OCLC_LANGUAGES) == OCLC20_LANG;
+  bool OclC2Unsupported =
+  (LangOpts.OpenCLVersion != 200 && !LangOpts.OpenCLCPlusPlus) &&
+  (BuiltinInfo.Langs & ALL_OCLC_LANGUAGES) == OCLC20_LANG;
   bool OclCUnsupported = !LangOpts.OpenCL &&
  (BuiltinInfo.Langs & ALL_OCLC_LANGUAGES);
   bool OpenMPUnsupported = !LangOpts.OpenMP && BuiltinInfo.Langs == OMP_LANG;
Index: lib/Parse/ParseDecl.cpp
===
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -2560,6 +2560,11 @@
 return false;
   }
 
+  // Early exit as Sema has a dedicated missing_actual_pipe_type diagnostic
+  // for incomplete declarations such as `pipe p`.
+  if (getLangOpts().OpenCLCPlusPlus && DS.isTypeSpecPipe())
+return false;
+
   if (getLangOpts().CPlusPlus &&
   DS.getStorageClassSpec() == DeclSpec::SCS_auto) {
 // Don't require a type specifier if we have the 'auto' storage class
@@ -3769,7 +3774,8 @@
   isInvalid = DS.SetTypeAltiVecBool(true, Loc, PrevSpec, DiagID, Policy);
   break;
 case tok::kw_pipe:
-  if (!getLangOpts().OpenCL || (getLangOpts().OpenCLVersion < 200)) {
+  if (!getLangOpts().OpenCL || (getLangOpts().OpenCLVersion < 200 &&
+!getLangOpts().OpenCLCPlusPlus)) {
 // OpenCL 2.0 defined this keyword. OpenCL 1.2 and earlier should
 // support the "pipe" word as identifier.
 Tok.getIdentifierInfo()->revertTokenIDToIdentifier();
@@ -4896,7 +4902,8 @@
   default: return false;
 
   case tok::kw_pipe:
-return getLangOpts().OpenCL && (getLangOpts().OpenCLVersion >= 200);
+return (getLangOpts().OpenCL && getLangOpts().OpenCLVersion >= 200) ||
+   getLangOpts().OpenCLCPlusPlus;
 
   case tok::identifier:   // foo::bar
 // Unfortunate hack to support "Class.factoryMethod" notation.
@@ -5384,7 +5391,8 @@
   if (Kind == tok::star || Kind == tok::caret)
 return true;
 
-  if ((Kind == tok::kw_pipe) && Lang.OpenCL && (Lang.OpenCLVersion >= 200))
+  if (Kind == tok::kw_pipe

[PATCH] D62238: [CodeComplete] Complete a lambda when preferred type is a function

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 200717.
ilya-biryukov marked 3 inline comments as done.
ilya-biryukov added a comment.

- Add placeholder for captures.
- Only accept classes that have exactly one template argument.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62238

Files:
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/lambdas.cpp

Index: clang/test/CodeCompletion/lambdas.cpp
===
--- /dev/null
+++ clang/test/CodeCompletion/lambdas.cpp
@@ -0,0 +1,56 @@
+template 
+struct function {
+};
+
+
+void test() {
+  void (*x)(int, double) = nullptr;
+
+  function y = {};
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:7:28 %s -o - | FileCheck -check-prefix=CHECK-1 %s
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:9:35 %s -o - | FileCheck -check-prefix=CHECK-1 %s
+  // CHECK-1: COMPLETION: Pattern : [<#=#>](int <#parameter#>, double <#parameter#>){<#body#>}
+
+  // == Placeholders for suffix types must be placed properly.
+  function z = {};
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:15:36 %s -o - | FileCheck -check-prefix=CHECK-2 %s
+  // CHECK-2: COMPLETION: Pattern : [<#=#>](void (* <#parameter#>)(int)){<#body#>}
+
+  // == No need for a parameter list if function has no parameters.
+  function a = {};
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:20:24 %s -o - | FileCheck -check-prefix=CHECK-3 %s
+  // CHECK-3: COMPLETION: Pattern : [<#=#>]{<#body#>}
+}
+
+template 
+struct vector {};
+
+void test2() {
+  // == Try to preserve types as written.
+  function)> a = {};
+
+  using function_typedef = function)>;
+  function_typedef b = {};
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:30:35 %s -o - | FileCheck -check-prefix=CHECK-4 %s
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:33:24 %s -o - | FileCheck -check-prefix=CHECK-4 %s
+  // CHECK-4: COMPLETION: Pattern : [<#=#>](vector <#parameter#>){<#body#>}
+}
+
+// Check another common function wrapper name.
+template  struct unique_function {};
+
+void test3() {
+  unique_function a = {};
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:43:31 %s -o - | FileCheck -check-prefix=CHECK-5 %s
+  // CHECK-5: COMPLETION: Pattern : [<#=#>]{<#body#>}
+}
+
+template  struct some_random_class {};
+template  struct weird_function {};
+void test4() {
+  some_random_class a = {};
+  weird_function b = {};
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:51:31 %s -o - | FileCheck -check-prefix=CHECK-6 %s
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:52:35 %s -o - | FileCheck -check-prefix=CHECK-6 %s
+  // CHECK-6-NOT: COMPLETION: Pattern : [<#=#>]{
+}
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -4108,6 +4108,84 @@
   Results.ExitScope();
 }
 
+/// Try to find a corresponding FunctionProtoType for function-like types (e.g.
+/// function pointers, std::function, etc).
+static const FunctionProtoType *TryDeconstructFunctionLike(QualType T) {
+  assert(!T.isNull());
+  // Try to extract first template argument from std::function<> and similar.
+  DeclarationName PotentialTemplateName;
+  const TemplateArgument *PotentialFunctionArg = nullptr;
+  if (auto *Specialization = T->getAs()) {
+// FIXME: handle other kinds of template names.
+auto *Template = Specialization->getTemplateName().getAsTemplateDecl();
+if (!Template || Specialization->getNumArgs() != 1)
+  return nullptr;
+PotentialTemplateName = Template->getDeclName();
+PotentialFunctionArg = &Specialization->getArg(0);
+  } else if (auto *Class = dyn_cast_or_null(
+ T->getAsCXXRecordDecl())) {
+if (Class->getTemplateArgs().size() != 1)
+  return nullptr;
+PotentialTemplateName = Class->getDeclName();
+PotentialFunctionArg = &Class->getTemplateArgs().get(0);
+  }
+  if (PotentialFunctionArg) {
+if (!PotentialTemplateName.isIdentifier() ||
+!PotentialTemplateName.getAsIdentifierInfo()->getName().contains(
+"function"))
+  return nullptr;
+if (PotentialFunctionArg->getKind() != TemplateArgument::Type)
+  return nullptr;
+return PotentialFunctionArg->getAsType()->getAs();
+  }
+  // Handle other cases.
+  if (T->isPointerType())
+T = T->getPointeeType();
+  return T->getAs();
+}
+
+/// Adds a pattern completion for a lambda expression with the specified
+/// parameter types and placeholders for parameter names.
+static void AddLambdaCompletion(ResultBuilder &Results,
+   

[PATCH] D62238: [CodeComplete] Complete a lambda when preferred type is a function

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked an inline comment as done and an inline comment as not 
done.
ilya-biryukov added inline comments.



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4135
+!PotentialTemplateName.getAsIdentifierInfo()->getName().contains(
+"function"))
+  return nullptr;

kadircet wrote:
> This looks cheesy, do we really want to perform this operation only for 
> templates with `"function"` in their names?
> 
> As discussed offline maybe perform this for any template with a 
> single(non-defaulted?) argument, or maybe even better perform a type-check as 
> suggested by you. But I believe there would be too many false positives with 
> the current state.
WDYT about `"function"` + only template with a single arg?
FWIW, I think function in template arguments are very rare, so I'm more 
optimistic about false-positives even in the current form.

We have an option of making it super-restrictive and only firing on whitelisted 
things like `std::function`, `boost::function`, etc.
But we don't have a mechanism to configure those from the outside, so that will 
probably turn out too restrictive.



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4154
+   Results.getCodeCompletionTUInfo());
+  // []() {}
+  Completion.AddChunk(CodeCompletionString::CK_LeftBracket);

kadircet wrote:
> maybe also add a placeholder for captures?
Done. With `=` as a default.
I'd personally rather have less placeholders, but I guess that might be useful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62238



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

[PATCH] D61508: [clang-tidy] bugprone-header-guard : a simple version of llvm-header-guard

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman marked an inline comment as done.
aaron.ballman added inline comments.



Comment at: clang-tidy/bugprone/HeaderGuardCheck.cpp:30
+}
+std::string BugproneHeaderGuardCheck::getHeaderGuard(StringRef Filename,
+ StringRef OldGuard) {

trixirt wrote:
> aaron.ballman wrote:
> > Can you add a newline for some visual separation?
> I did not think ment a empty newline in the param-list, so  I assumed you 
> ment the function above and this function could use a new line.  
You guessed correctly -- sorry for the lack of clarity. :-)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61508



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

[PATCH] D61939: AArch64: add support for arm64_23 (ILP32) IR generation

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

Oops, yes. I'll leave it wrong though, the best that could come out of any 
attempt to change it would be to split the thread on llvm-commits.


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

https://reviews.llvm.org/D61939



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

[PATCH] D61497: [clangd] Introduce a structured hover response

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 200732.
kadircet marked 5 inline comments as done.
kadircet added a comment.

- Added tests, and went over implementation.

I am looking for input on:

- Behavior on lambdas, currently it just keeps the old behaviour. I believe 
they should look more like functions. Do you think it is necessary for first 
patch?
- Template classes/functions, what we have in definition is not great but I 
don't think it is too much of a problem since semantic representation carries 
all the useful information.
- Declared in #SOME_KIND# #SOME_PLACE#, currently we drop SOME_KIND and it 
needs additional info to be propogated since it is not the type of the symbol 
but rather it is immediate container. I don't think it is necessary for initial 
version.
- Discrepencies between hovering over auto/decltype and explicitly spelled 
types, again this keeps the existing behaviour. You can see details in 
testcases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61497

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/test/hover.test
  clang-tools-extra/clangd/unittests/TestTU.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -9,6 +9,7 @@
 #include "ClangdUnit.h"
 #include "Compiler.h"
 #include "Matchers.h"
+#include "Protocol.h"
 #include "SyncAPI.h"
 #include "TestFS.h"
 #include "TestTU.h"
@@ -16,6 +17,7 @@
 #include "index/FileIndex.h"
 #include "index/SymbolCollector.h"
 #include "clang/Index/IndexingAction.h"
+#include "llvm/ADT/None.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "gmock/gmock.h"
@@ -558,6 +560,350 @@
   HeaderNotInPreambleAnnotations.range(;
 }
 
+TEST(Hover, Structured) {
+  struct {
+const char *const Code;
+const HoverInfo Expected;
+  } // namespace
+  Cases[] = {
+  // Global scope.
+  {R"cpp(
+// Best foo ever.
+void [[fo^o]]() {}
+)cpp",
+   {/*NamespaceScope=*/std::string(""),
+/*LocalScope=*/std::string(""),
+/*Name=*/"foo",
+/*SymRange=*/llvm::None,
+/*Kind=*/SymbolKind::Function,
+/*Documentation=*/"Best foo ever.",
+/*Definition=*/"void foo()",
+/*Type=*/llvm::None,
+/*ReturnType=*/std::string("void"),
+/*Parameters=*/std::vector{},
+/*TemplateParameters=*/llvm::None}},
+  // Inside namespace
+  {R"cpp(
+namespace ns1 { namespace ns2 {
+  /// Best foo ever.
+  void [[fo^o]]() {}
+}}
+)cpp",
+   {/*NamespaceScope=*/std::string("ns1::ns2"),
+/*LocalScope=*/std::string(""),
+/*Name=*/"foo",
+/*SymRange=*/llvm::None,
+/*Kind=*/SymbolKind::Function,
+/*Documentation=*/"Best foo ever.",
+/*Definition=*/"void foo()",
+/*Type=*/llvm::None,
+/*ReturnType=*/std::string("void"),
+/*Parameters=*/std::vector{},
+/*TemplateParameters=*/llvm::None}},
+  // Field
+  {R"cpp(
+namespace ns1 { namespace ns2 {
+  struct Foo {
+int [[b^ar]];
+  };
+}}
+)cpp",
+   {/*NamespaceScope=*/std::string("ns1::ns2"),
+/*LocalScope=*/std::string("Foo"),
+/*Name=*/"bar",
+/*SymRange=*/llvm::None,
+/*Kind=*/SymbolKind::Field,
+/*Documentation=*/"",
+/*Definition=*/"int bar",
+/*Type=*/std::string("int"),
+/*ReturnType=*/llvm::None,
+/*Parameters=*/llvm::None,
+/*TemplateParameters=*/llvm::None}},
+  // Local to class method.
+  {R"cpp(
+namespace ns1 { namespace ns2 {
+  struct Foo {
+void foo() {
+  int [[b^ar]];
+}
+  };
+}}
+)cpp",
+   {/*NamespaceScope=*/std::string("ns1::ns2"),
+/*LocalScope=*/std::string("Foo::foo"),
+/*Name=*/"bar",
+/*SymRange=*/llvm::None,
+/*Kind=*/SymbolKind::Variable,
+/*Documentation=*/"",
+/*Definition=*/"int bar",
+/*Type=*/std::string("int"),
+/*ReturnType=*/llvm::None,
+/*Parameters=*/llvm::None,
+/*TemplateParameters=*/llvm::None}},
+  // Anon namespace and local scope.
+  {R"cpp(
+namespace ns1 { namespace {
+  struct {
+int [[b^ar]];
+  } T;
+}}
+)cpp",
+   {/*NamespaceScope=*/std::string("ns1::(anonymous)"),
+/*

[PATCH] D61497: [clangd] Introduce a structured hover response

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:940
   )cpp",
-  "class std::initializer_list",
+  "template<> class initializer_list {}",
   },

kadircet wrote:
> sammccall wrote:
> > hmm, this is a bit weird - this uses specialization syntax but there's no 
> > actual specialization here right?
> > I think the old output without `template<>` is probably better if possible.
> The old behavior was inconsistent in the case of auto. We print the decl in 
> all cases, but print the type in the case of auto. For example, if you had 
> `initializer_list i = {1,2}` instead of `auto i = {1,2}` you would get 
> the new response I've provided in this test.
> 
> I agree this looks weird in the case of instantiations but I believe it is 
> more important to give a consistent look.
as discussed offline, leaving this as it is in the initial version and adding 
tests with both auto and non-auto cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61497



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

[PATCH] D61865: [clangd] improve help message for limit-results

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

I can land it for you, you can learn more about commit access from 
https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61865



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

[clang-tools-extra] r361388 - [clangd] improve help message for limit-results

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Wed May 22 07:19:40 2019
New Revision: 361388

URL: http://llvm.org/viewvc/llvm-project?rev=361388&view=rev
Log:
[clangd] improve help message for limit-results

Summary: Make it clear that the default is 100.

Patch by Brennan Vincent(@umanwizard)!

Reviewers: #clang-tools-extra, kadircet

Reviewed By: kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp

Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=361388&r1=361387&r2=361388&view=diff
==
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Wed May 22 07:19:40 2019
@@ -119,7 +119,7 @@ static llvm::cl::opt PCH
 static llvm::cl::opt LimitResults(
 "limit-results",
 llvm::cl::desc("Limit the number of results returned by clangd. "
-   "0 means no limit."),
+   "0 means no limit. (default=100)"),
 llvm::cl::init(100));
 
 static llvm::cl::opt RunSynchronously(


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

[PATCH] D61865: [clangd] improve help message for limit-results

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL361388: [clangd] improve help message for limit-results 
(authored by kadircet, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61865?vs=199295&id=200734#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61865

Files:
  clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
@@ -119,7 +119,7 @@
 static llvm::cl::opt LimitResults(
 "limit-results",
 llvm::cl::desc("Limit the number of results returned by clangd. "
-   "0 means no limit."),
+   "0 means no limit. (default=100)"),
 llvm::cl::init(100));
 
 static llvm::cl::opt RunSynchronously(


Index: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
@@ -119,7 +119,7 @@
 static llvm::cl::opt LimitResults(
 "limit-results",
 llvm::cl::desc("Limit the number of results returned by clangd. "
-   "0 means no limit."),
+   "0 means no limit. (default=100)"),
 llvm::cl::init(100));
 
 static llvm::cl::opt RunSynchronously(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet added a comment.

AFAIU here is a coarse plan of what needs to happen

1. Add a `no-builtin` clang function attribute that has the same semantic as 
the `no-builtin` cmd line argument 

2. Propagate it to the IR.
  - In the light of recent discussions and as @theraven suggested it seems more 
appropriate to encode them as individual IR attributes (e.g. 
`"no-builtin-memcpy"`, `"no-builtin-sqrt"`, etc..)
3. Propagate/merge the `no-builtin` IR attribute for LTO by "updating 
`AttributeFuncs::areInlineCompatible` and/or 
`AttributeFuncs::mergeAttributesForInlining` and adding a new MergeRule in 
`include/llvm/IR/Attributes.td` and writing a function like 
`adjustCallerStackProbeSize`."
4. Get inspiration from `TargetTransformInfo` to get `TargetLibraryInfo` on a 
per function basis for all passes and respect the IR attribute.

I'm not familiar with 3 and 4 but I can definitely have a look.
I'll update this patch to do 1 and 2 in the meantime. @tejohnson let me know 
how you want to proceed for your related patch 
. I'm happy to help if I can.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61634



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

[PATCH] D61914: [Support][Test] Time profiler: add regression test

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

Could you please move the test to a more approriate location? (ie. 
clang/trunk/test/Driver/)




Comment at: clang/tools/driver/cc1_main.cpp:245
+
+llvm::errs() << "Time trace json-file dumped to " << Path.str() << "\n";
+llvm::errs()

This seems a bit too chatty. Suround these two lines with `if 
(Config->Verbose)` ?



Comment at: llvm/test/Support/check-time-trace.cxx:4
+
+// CHECK: "args":{"name":"clang"}
+

I don't see any other `-ftime-trace` tests, I would add a few more exhaustive 
file format checks here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61914



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

[PATCH] D59407: [clangd] Add RelationSlab

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 200735.
nridge added a comment.
Herald added a subscriber: mgrang.

Implemented discussed design approach ("Add a RelationSlab storing (subject, 
predicate, object) triples, intended for sparse relations")


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59407

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/Index.h
  clang-tools-extra/clangd/index/Relation.cpp
  clang-tools-extra/clangd/index/Relation.h
  clang-tools-extra/clangd/unittests/IndexTests.cpp

Index: clang-tools-extra/clangd/unittests/IndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/IndexTests.cpp
+++ clang-tools-extra/clangd/unittests/IndexTests.cpp
@@ -76,6 +76,29 @@
 EXPECT_THAT(*S.find(SymbolID(Sym)), Named(Sym));
 }
 
+TEST(RelationSlab, Lookup) {
+  SymbolID A{"A"};
+  SymbolID B{"B"};
+  SymbolID C{"C"};
+  SymbolID D{"D"};
+
+  RelationSlab::Builder Builder;
+  Builder.push_back(Relation{A, index::SymbolRole::RelationBaseOf, B});
+  Builder.push_back(Relation{A, index::SymbolRole::RelationBaseOf, C});
+  Builder.push_back(Relation{B, index::SymbolRole::RelationBaseOf, D});
+  Builder.push_back(Relation{C, index::SymbolRole::RelationBaseOf, D});
+  Builder.push_back(Relation{B, index::SymbolRole::RelationChildOf, A});
+  Builder.push_back(Relation{C, index::SymbolRole::RelationChildOf, A});
+  Builder.push_back(Relation{D, index::SymbolRole::RelationChildOf, B});
+  Builder.push_back(Relation{D, index::SymbolRole::RelationChildOf, C});
+
+  RelationSlab Slab = std::move(Builder).build();
+  EXPECT_THAT(
+  Slab.lookup(A, index::SymbolRole::RelationBaseOf),
+  UnorderedElementsAre(Relation{A, index::SymbolRole::RelationBaseOf, B},
+   Relation{A, index::SymbolRole::RelationBaseOf, C}));
+}
+
 TEST(SwapIndexTest, OldIndexRecycled) {
   auto Token = std::make_shared();
   std::weak_ptr WeakToken = Token;
Index: clang-tools-extra/clangd/index/Relation.h
===
--- /dev/null
+++ clang-tools-extra/clangd/index/Relation.h
@@ -0,0 +1,98 @@
+//===--- Relation.h --*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_RELATION_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_RELATION_H
+
+#include "SymbolID.h"
+#include "SymbolLocation.h"
+#include "clang/Index/IndexSymbol.h"
+#include "llvm/ADT/iterator_range.h"
+#include 
+#include 
+
+namespace clang {
+namespace clangd {
+
+/// Represents a relation between two symbols.
+/// For an example "A is a base class of B" may be represented
+/// as { Subject = A, Predicate = RelationBaseOf, Object = B }.
+struct Relation {
+  SymbolID Subject;
+  index::SymbolRole Predicate;
+  SymbolID Object;
+
+  bool operator==(const Relation &Other) const {
+return Subject == Other.Subject && Predicate == Other.Predicate &&
+   Object == Other.Object;
+  }
+  // SPO order
+  bool operator<(const Relation &Other) const {
+if (Subject < Other.Subject) {
+  return true;
+}
+if (Subject == Other.Subject) {
+  if (Predicate < Other.Predicate) {
+return true;
+  }
+
+  if (Predicate == Other.Predicate) {
+return Object < Other.Object;
+  }
+}
+return false;
+  }
+};
+
+class RelationSlab {
+public:
+  using value_type = Relation;
+  using const_iterator = std::vector::const_iterator;
+  using iterator = const_iterator;
+
+  // We don't need a separate builder type for this, but reserve
+  // that possibility for the future. Having a build() method is
+  // also useful so when know when to sort the relations.
+  using Builder = RelationSlab;
+
+  RelationSlab() = default;
+  RelationSlab(RelationSlab &&Slab) = default;
+  RelationSlab &operator=(RelationSlab &&RHS) = default;
+
+  const_iterator begin() const { return Relations.begin(); }
+  const_iterator end() const { return Relations.end(); }
+  size_t size() const { return Relations.size(); }
+  bool empty() const { return Relations.empty(); }
+
+  void push_back(const Relation &R) { Relations.push_back(R); }
+
+  size_t bytes() const {
+return sizeof(*this) + sizeof(value_type) * Relations.capacity();
+  }
+
+  /// Sort relations in SPO order.
+  void sort();
+
+  /// Lookup all relations matching the given subject and predicate.
+  /// Assumes the slab is sorted in SPO order.
+  llvm::iterator_range lookup(const SymbolID &Subject,
+index::SymbolRole Predicate) const;
+
+  RelationSlab build() &&;
+

[PATCH] D62005: [libunwind] [test] Fix inferring source paths

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

Ping.


Repository:
  rUNW libunwind

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

https://reviews.llvm.org/D62005



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

[PATCH] D61487: [clang-tidy] Make the plugin honor NOLINT

[Nikolai Kosjar via Phabricator via cfe-commits]

nik marked 4 inline comments as done.
nik added a comment.

As I've commented on, this change is not finished. However, I've addressed the 
inline comments nevertheless.

There is one TODO left for which I would like to have an opinion.




Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:472
   if (Info.hasSourceManager())
 checkFilters(Info.getLocation(), Info.getSourceManager());
 }

gribozavr wrote:
> It seems like the `checkFilters` call should not be skipped even if we have 
> another diagnostic engine.
checkFilters() sets some state so that later finalizeLastError() can remove 
diagnostics/errors from ClangTidyDiagnosticConsumer::Errors and also track some 
statistics.

Statistics are not relevant for the pluginc case as they should not be printed 
out for that case.
The filtering happening in finalizeLastError() does not look relevant as the 
plugin currently only supports the "-checks=..." argument but not the e.g. 
header and line filter. When checks are created in 
ClangTidyCheckFactories::createChecks, the "-checks=..." argument is honored, 
so that the !Context.isCheckEnabled(...) in finalizeLastError() is also not 
relevant.

I agree that checkFilters()/finalizeLastError() will be needed once the plugin 
case should support the other filtering options (header + line filter), but 
note that this goes beyond the scope of this change here, which is NOLINT 
filtering.

This is all new to me, so if I miss something regarding the 
checkFilters()/finalizeLastError() thingy, please tell me, preferably with an 
example if possible :)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61487



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

[PATCH] D61487: [clang-tidy] Make the plugin honor NOLINT

[Nikolai Kosjar via Phabricator via cfe-commits]

nik updated this revision to Diff 200736.
nik marked an inline comment as done.
nik added a comment.

Addressed comments.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61487

Files:
  clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tidy/plugin/ClangTidyPlugin.cpp
  test/clang-tidy/basic.cpp
  test/clang-tidy/nolint-plugin.cpp
  test/clang-tidy/nolintnextline-plugin.cpp

Index: test/clang-tidy/nolintnextline-plugin.cpp
===
--- /dev/null
+++ test/clang-tidy/nolintnextline-plugin.cpp
@@ -0,0 +1,47 @@
+class A { A(int i); };
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+// NOLINTNEXTLINE
+class B { B(int i); };
+
+// NOLINTNEXTLINE(for-some-other-check)
+class C { C(int i); };
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+// NOLINTNEXTLINE(*)
+class C1 { C1(int i); };
+
+// NOLINTNEXTLINE(not-closed-bracket-is-treated-as-skip-all
+class C2 { C2(int i); };
+
+// NOLINTNEXTLINE(google-explicit-constructor)
+class C3 { C3(int i); };
+
+// NOLINTNEXTLINE(some-check, google-explicit-constructor)
+class C4 { C4(int i); };
+
+// NOLINTNEXTLINE without-brackets-skip-all, another-check
+class C5 { C5(int i); };
+
+
+// NOLINTNEXTLINE
+
+class D { D(int i); };
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+// NOLINTNEXTLINE
+//
+class E { E(int i); };
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+#define MACRO(X) class X { X(int i); };
+MACRO(F)
+// CHECK: :[[@LINE-1]]:7: warning: single-argument constructors must be marked explicit
+// NOLINTNEXTLINE
+MACRO(G)
+
+#define MACRO_NOARG class H { H(int i); };
+// NOLINTNEXTLINE
+MACRO_NOARG
+
+// RUN: c-index-test -test-load-source-reparse 2 all %s -Xclang -add-plugin -Xclang clang-tidy -Xclang -plugin-arg-clang-tidy -Xclang -checks='-*,google-explicit-constructor' 2>&1 | FileCheck %s
Index: test/clang-tidy/nolint-plugin.cpp
===
--- /dev/null
+++ test/clang-tidy/nolint-plugin.cpp
@@ -0,0 +1,50 @@
+// REQUIRES: static-analyzer
+// RUN: c-index-test -test-load-source-reparse 2 all %s -Xclang -add-plugin -Xclang clang-tidy -Xclang -plugin-arg-clang-tidy -Xclang -checks='-*,google-explicit-constructor,clang-diagnostic-unused-variable,clang-analyzer-core.UndefinedBinaryOperatorResult' -Wunused-variable -I%S/Inputs/nolint 2>&1 | FileCheck %s
+
+#include "trigger_warning.h"
+void I(int& Out) {
+  int In;
+  A1(In, Out);
+}
+// CHECK-NOT: trigger_warning.h:{{.*}} warning
+// CHECK-NOT: :[[@LINE-4]]:{{.*}} note
+
+class A { A(int i); };
+// CHECK-DAG: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+class B { B(int i); }; // NOLINT
+
+class C { C(int i); }; // NOLINT(for-some-other-check)
+// CHECK-DAG: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+class C1 { C1(int i); }; // NOLINT(*)
+
+class C2 { C2(int i); }; // NOLINT(not-closed-bracket-is-treated-as-skip-all
+
+class C3 { C3(int i); }; // NOLINT(google-explicit-constructor)
+
+class C4 { C4(int i); }; // NOLINT(some-check, google-explicit-constructor)
+
+class C5 { C5(int i); }; // NOLINT without-brackets-skip-all, another-check
+
+void f() {
+  int i;
+// CHECK-DAG: :[[@LINE-1]]:7: warning: unused variable 'i' [-Wunused-variable]
+//  31:7: warning: unused variable 'i' [-Wunused-variable]
+//  int j; // NOLINT
+//  int k; // NOLINT(clang-diagnostic-unused-variable)
+}
+
+#define MACRO(X) class X { X(int i); };
+MACRO(D)
+// CHECK-DAG: :[[@LINE-1]]:7: warning: single-argument constructors must be marked explicit
+MACRO(E) // NOLINT
+
+#define MACRO_NOARG class F { F(int i); };
+MACRO_NOARG // NOLINT
+
+#define MACRO_NOLINT class G { G(int i); }; // NOLINT
+MACRO_NOLINT
+
+#define DOUBLE_MACRO MACRO(H) // NOLINT
+DOUBLE_MACRO
Index: test/clang-tidy/basic.cpp
===
--- test/clang-tidy/basic.cpp
+++ test/clang-tidy/basic.cpp
@@ -1,4 +1,5 @@
 // RUN: clang-tidy %s -checks='-*,llvm-namespace-comment' -- | FileCheck %s
+// RUN: c-index-test -test-load-source-reparse 2 all %s -Xclang -add-plugin -Xclang clang-tidy -Xclang -plugin-arg-clang-tidy -Xclang -checks='-*,llvm-namespace-comment' 2>&1 | FileCheck %s
 
 namespace i {
 }
Index: clang-tidy/plugin/ClangTidyPlugin.cpp
===
--- clang-tidy/plugin/ClangTidyPlugin.cpp
+++ clang-tidy/plugin/ClangTidyPlugin.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "../ClangTidy.h"
+#include "../ClangTidyDiagnosticConsumer.h"
 #include "../ClangTidyForceLinker.h"
 #include "../Cl

r361391 - Reland r361148 with a fix to the buildbot failure.

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Wed May 22 07:44:45 2019
New Revision: 361391

URL: http://llvm.org/viewvc/llvm-project?rev=361391&view=rev
Log:
Reland r361148 with a fix to the buildbot failure.

Reverted in r361377.
Also reland the '.gn' files (reverted in r361389).

Added:
cfe/trunk/include/clang/Tooling/Syntax/
cfe/trunk/include/clang/Tooling/Syntax/Tokens.h
cfe/trunk/lib/Tooling/Syntax/
cfe/trunk/lib/Tooling/Syntax/CMakeLists.txt
cfe/trunk/lib/Tooling/Syntax/Tokens.cpp
cfe/trunk/unittests/Tooling/Syntax/
cfe/trunk/unittests/Tooling/Syntax/CMakeLists.txt
cfe/trunk/unittests/Tooling/Syntax/TokensTest.cpp
Modified:
cfe/trunk/lib/Tooling/CMakeLists.txt
cfe/trunk/unittests/Tooling/CMakeLists.txt

Added: cfe/trunk/include/clang/Tooling/Syntax/Tokens.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Syntax/Tokens.h?rev=361391&view=auto
==
--- cfe/trunk/include/clang/Tooling/Syntax/Tokens.h (added)
+++ cfe/trunk/include/clang/Tooling/Syntax/Tokens.h Wed May 22 07:44:45 2019
@@ -0,0 +1,302 @@
+//===- Tokens.h - collect tokens from preprocessing --*- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+// Record tokens that a preprocessor emits and define operations to map between
+// the tokens written in a file and tokens produced by the preprocessor.
+//
+// When running the compiler, there are two token streams we are interested in:
+//   - "spelled" tokens directly correspond to a substring written in some
+// source file.
+//   - "expanded" tokens represent the result of preprocessing, parses consumes
+// this token stream to produce the AST.
+//
+// Expanded tokens correspond directly to locations found in the AST, allowing
+// to find subranges of the token stream covered by various AST nodes. Spelled
+// tokens correspond directly to the source code written by the user.
+//
+// To allow composing these two use-cases, we also define operations that map
+// between expanded and spelled tokens that produced them (macro calls,
+// directives, etc).
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLING_SYNTAX_TOKENS_H
+#define LLVM_CLANG_TOOLING_SYNTAX_TOKENS_H
+
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.h"
+#include "clang/Lex/Token.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Compiler.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+
+namespace clang {
+class Preprocessor;
+
+namespace syntax {
+
+/// A half-open character range inside a particular file, the start offset is
+/// included and the end offset is excluded from the range.
+struct FileRange {
+  /// EXPECTS: File.isValid() && Begin <= End.
+  FileRange(FileID File, unsigned BeginOffset, unsigned EndOffset);
+  /// EXPECTS: BeginLoc.isValid() && BeginLoc.isFileID().
+  FileRange(const SourceManager &SM, SourceLocation BeginLoc, unsigned Length);
+  /// EXPECTS: BeginLoc.isValid() && BeginLoc.isFileID(), Begin <= End and 
files
+  ///  are the same.
+  FileRange(const SourceManager &SM, SourceLocation BeginLoc,
+SourceLocation EndLoc);
+
+  FileID file() const { return File; }
+  /// Start is a start offset (inclusive) in the corresponding file.
+  unsigned beginOffset() const { return Begin; }
+  /// End offset (exclusive) in the corresponding file.
+  unsigned endOffset() const { return End; }
+
+  unsigned length() const { return End - Begin; }
+
+  /// Gets the substring that this FileRange refers to.
+  llvm::StringRef text(const SourceManager &SM) const;
+
+  friend bool operator==(const FileRange &L, const FileRange &R) {
+return std::tie(L.File, L.Begin, L.End) == std::tie(R.File, R.Begin, 
R.End);
+  }
+  friend bool operator!=(const FileRange &L, const FileRange &R) {
+return !(L == R);
+  }
+
+private:
+  FileID File;
+  unsigned Begin;
+  unsigned End;
+};
+
+/// For debugging purposes.
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const FileRange &R);
+
+/// A token coming directly from a file or from a macro invocation. Has just
+/// enough information to locate the token in the source code.
+/// Can represent both expanded and spelled tokens.
+class Token {
+public:
+  Token(SourceLocation Location, unsigned Length, tok::TokenKind Kind)
+  : Location(Location), Length(Length), Kind(Kind) {}
+  /// EXPECTS: clang::Token is not an annotation token.
+  explicit Token(const clang::Token &T);
+

[PATCH] D62238: [CodeComplete] Complete a lambda when preferred type is a function

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4135
+!PotentialTemplateName.getAsIdentifierInfo()->getName().contains(
+"function"))
+  return nullptr;

ilya-biryukov wrote:
> kadircet wrote:
> > This looks cheesy, do we really want to perform this operation only for 
> > templates with `"function"` in their names?
> > 
> > As discussed offline maybe perform this for any template with a 
> > single(non-defaulted?) argument, or maybe even better perform a type-check 
> > as suggested by you. But I believe there would be too many false positives 
> > with the current state.
> WDYT about `"function"` + only template with a single arg?
> FWIW, I think function in template arguments are very rare, so I'm more 
> optimistic about false-positives even in the current form.
> 
> We have an option of making it super-restrictive and only firing on 
> whitelisted things like `std::function`, `boost::function`, etc.
> But we don't have a mechanism to configure those from the outside, so that 
> will probably turn out too restrictive.
I don't think adding "function" as a restriction is helping much on reducing 
false positives. I would rather get rid of that check to increase usability.

The real problem is rather checking constructors to make sure there is at least 
one for which we can provide a callable with the signature we found. Anything 
else just seems cheesy and might result in people adding more and more cases 
over time.

It is up to you, I am OK if you choose to keep this restriction as well.



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4125
+PotentialFunctionArg = &Specialization->getArg(0);
+  } else if (auto *Class = dyn_cast_or_null(
+ T->getAsCXXRecordDecl())) {

I thought you decided to get rid of this branch after the offline discussions, 
sorry for not mentioning in earlier revision.



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4142
+  // Handle other cases.
+  if (T->isPointerType())
+T = T->getPointeeType();

what about referencetype?

```
void test();
void (&y)() = test;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62238



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

r361392 - [LibTooling] Update Transformer to use RangeSelector instead of NodePart enum.

[Yitzhak Mandelbaum via cfe-commits]

Author: ymandel
Date: Wed May 22 07:48:19 2019
New Revision: 361392

URL: http://llvm.org/viewvc/llvm-project?rev=361392&view=rev
Log:
[LibTooling] Update Transformer to use RangeSelector instead of NodePart enum.

Transformer provides an enum to indicate the range of source text to be edited.
That support is now redundant with the new (and more general) RangeSelector
library, so we remove the custom enum support in favor of supporting any
RangeSelector.

Reviewers: ilya-biryukov

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
cfe/trunk/unittests/Tooling/TransformerTest.cpp

Modified: cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h?rev=361392&r1=361391&r2=361392&view=diff
==
--- cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h (original)
+++ cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h Wed May 22 
07:48:19 2019
@@ -19,6 +19,7 @@
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/ASTMatchers/ASTMatchersInternal.h"
 #include "clang/Tooling/Refactoring/AtomicChange.h"
+#include "clang/Tooling/Refactoring/RangeSelector.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Error.h"
@@ -30,19 +31,6 @@
 
 namespace clang {
 namespace tooling {
-/// Determines the part of the AST node to replace.  We support this to work
-/// around the fact that the AST does not differentiate various syntactic
-/// elements into their own nodes, so users can specify them relative to a 
node,
-/// instead.
-enum class NodePart {
-  /// The node itself.
-  Node,
-  /// Given a \c MemberExpr, selects the member's token.
-  Member,
-  /// Given a \c NamedDecl or \c CxxCtorInitializer, selects that token of the
-  /// relevant name, not including qualifiers.
-  Name,
-};
 
 // Note that \p TextGenerator is allowed to fail, e.g. when trying to access a
 // matched node that was not bound.  Allowing this to fail simplifies error
@@ -76,73 +64,28 @@ inline TextGenerator text(std::string M)
 //   (`RewriteRule::Explanation`) instead.  Notes serve the rare cases wherein
 //   edit-specific diagnostics are required.
 //
-// `ASTEdit` should be built using the `change` convenience fucntions. For
+// `ASTEdit` should be built using the `change` convenience functions. For
 // example,
 // \code
-//   change(fun, NodePart::Name, "Frodo")
+//   change(name(fun), text("Frodo"))
 // \endcode
 // Or, if we use Stencil for the TextGenerator:
 // \code
-//   change(thenNode, stencil::cat("{", thenNode, "}"))
-//   change(call, NodePart::Args, stencil::cat(x, ",", y))
-// .note("argument order changed.")
+//   using stencil::cat;
+//   change(statement(thenNode), cat("{", thenNode, "}"))
+//   change(callArgs(call), cat(x, ",", y))
 // \endcode
 // Or, if you are changing the node corresponding to the rule's matcher, you 
can
 // use the single-argument override of \c change:
 // \code
-//   change("different_expr")
+//   change(cat("different_expr"))
 // \endcode
 struct ASTEdit {
-  // The (bound) id of the node whose source will be replaced.  This id should
-  // never be the empty string.
-  std::string Target;
-  ast_type_traits::ASTNodeKind Kind;
-  NodePart Part;
+  RangeSelector TargetRange;
   TextGenerator Replacement;
   TextGenerator Note;
 };
 
-// Convenience functions for creating \c ASTEdits.  They all must be explicitly
-// instantiated with the desired AST type.  Each overload includes both \c
-// std::string and \c TextGenerator versions.
-
-// FIXME: For overloads taking a \c NodePart, constrain the valid values of \c
-// Part based on the type \c T.
-template 
-ASTEdit change(StringRef Target, NodePart Part, TextGenerator Replacement) {
-  ASTEdit E;
-  E.Target = Target.str();
-  E.Kind = ast_type_traits::ASTNodeKind::getFromNodeKind();
-  E.Part = Part;
-  E.Replacement = std::move(Replacement);
-  return E;
-}
-
-template 
-ASTEdit change(StringRef Target, NodePart Part, std::string Replacement) {
-  return change(Target, Part, text(std::move(Replacement)));
-}
-
-/// Variant of \c change for which the NodePart defaults to the whole node.
-template 
-ASTEdit change(StringRef Target, TextGenerator Replacement) {
-  return change(Target, NodePart::Node, std::move(Replacement));
-}
-
-/// Variant of \c change for which the NodePart defaults to the whole node.
-template 
-ASTEdit change(StringRef Target, std::string Replacement) {
-  return change(Target, text(std::move(Replacement)));
-}
-
-/// Variant of \c change that selects the node of the entire match.
-template  ASTEdit change(TextGenerator Replacement);
-
-/// Variant of \c change that selects the node of the entire match.
-template  ASTEdi

[PATCH] D62149: [LibTooling] Update Transformer to use RangeSelector instead of NodePart enum.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL361392: [LibTooling] Update Transformer to use RangeSelector 
instead of NodePart enum. (authored by ymandel, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62149?vs=200712&id=200741#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62149

Files:
  cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
  cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
  cfe/trunk/unittests/Tooling/TransformerTest.cpp

Index: cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
===
--- cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
+++ cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
@@ -32,11 +32,7 @@
 using ast_type_traits::ASTNodeKind;
 using ast_type_traits::DynTypedNode;
 using llvm::Error;
-using llvm::Expected;
-using llvm::Optional;
 using llvm::StringError;
-using llvm::StringRef;
-using llvm::Twine;
 
 using MatchResult = MatchFinder::MatchResult;
 
@@ -71,91 +67,12 @@
   return false;
 }
 
-static llvm::Error invalidArgumentError(Twine Message) {
-  return llvm::make_error(llvm::errc::invalid_argument, Message);
-}
-
-static llvm::Error typeError(StringRef Id, const ASTNodeKind &Kind,
- Twine Message) {
-  return invalidArgumentError(
-  Message + " (node id=" + Id + " kind=" + Kind.asStringRef() + ")");
-}
-
-static llvm::Error missingPropertyError(StringRef Id, Twine Description,
-StringRef Property) {
-  return invalidArgumentError(Description + " requires property '" + Property +
-  "' (node id=" + Id + ")");
-}
-
-static Expected
-getTargetRange(StringRef Target, const DynTypedNode &Node, ASTNodeKind Kind,
-   NodePart TargetPart, ASTContext &Context) {
-  switch (TargetPart) {
-  case NodePart::Node: {
-// For non-expression statements, associate any trailing semicolon with the
-// statement text.  However, if the target was intended as an expression (as
-// indicated by its kind) then we do not associate any trailing semicolon
-// with it.  We only associate the exact expression text.
-if (Node.get() != nullptr) {
-  auto ExprKind = ASTNodeKind::getFromNodeKind();
-  if (!ExprKind.isBaseOf(Kind))
-return getExtendedRange(Node, tok::TokenKind::semi, Context);
-}
-return CharSourceRange::getTokenRange(Node.getSourceRange());
-  }
-  case NodePart::Member:
-if (auto *M = Node.get())
-  return CharSourceRange::getTokenRange(
-  M->getMemberNameInfo().getSourceRange());
-return typeError(Target, Node.getNodeKind(),
- "NodePart::Member applied to non-MemberExpr");
-  case NodePart::Name:
-if (const auto *D = Node.get()) {
-  if (!D->getDeclName().isIdentifier())
-return missingPropertyError(Target, "NodePart::Name", "identifier");
-  SourceLocation L = D->getLocation();
-  auto R = CharSourceRange::getTokenRange(L, L);
-  // Verify that the range covers exactly the name.
-  // FIXME: extend this code to support cases like `operator +` or
-  // `foo` for which this range will be too short.  Doing so will
-  // require subcasing `NamedDecl`, because it doesn't provide virtual
-  // access to the \c DeclarationNameInfo.
-  if (getText(R, Context) != D->getName())
-return CharSourceRange();
-  return R;
-}
-if (const auto *E = Node.get()) {
-  if (!E->getNameInfo().getName().isIdentifier())
-return missingPropertyError(Target, "NodePart::Name", "identifier");
-  SourceLocation L = E->getLocation();
-  return CharSourceRange::getTokenRange(L, L);
-}
-if (const auto *I = Node.get()) {
-  if (!I->isMemberInitializer() && I->isWritten())
-return missingPropertyError(Target, "NodePart::Name",
-"explicit member initializer");
-  SourceLocation L = I->getMemberLocation();
-  return CharSourceRange::getTokenRange(L, L);
-}
-return typeError(
-Target, Node.getNodeKind(),
-"NodePart::Name applied to neither DeclRefExpr, NamedDecl nor "
-"CXXCtorInitializer");
-  }
-  llvm_unreachable("Unexpected case in NodePart type.");
-}
-
 Expected>
 tooling::detail::translateEdits(const MatchResult &Result,
 llvm::ArrayRef Edits) {
-  SmallVector Transformations;
-  auto &NodesMap = Result.Nodes.getMap();
+  SmallVector Transformations;
   for (const auto &Edit : Edits) {
-auto It = NodesMap.find(Edit.Target);
-assert(It != NodesMap.end() && "Edit target must be bound in the match.");
-
-Expected Range = getTargetRange(
-Edit.Target, It->second, Edit.Kind, Edit.Part, *Result.Context);
+Exp

[PATCH] D61509: [OpenMP] Set pragma start loc to `#pragma` loc

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a subscriber: Meinersbur.
jdenny added a comment.

Now that D61643  is pushed, we're back to this 
patch.  My recollection is there are two remaining issues:

1. Should we store both the `#pragma` location and the `omp` location in the 
AST, or is it fine to just replace the latter location with the former?  If we 
choose to store both, we still haven't settled on an implementation, which 
likely will involve non-trivial changes in the parser and the AST.  For OpenMP, 
@ABataev said replacing should be fine.

2. Should we adjust all non-OpenMP pragmas in the same way immediately, or 
should we adjust them gradually as the need arises?

If the answers to the above questions are "replace" and "gradually", then I 
believe this patch is ready.  (It's just a tweak in 
`clang/lib/Parse/ParsePragma.cpp` and a ton of test updates.)

By the way, in D61643 , @Meinersbur pointed 
out that @rsmith also wanted to see diagnostics point at a `#pragma`:  
https://bugs.llvm.org/show_bug.cgi?id=41514#c1

If @rsmith and others don't get a chance to chime in, then I suppose an RFC 
should be next.


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

https://reviews.llvm.org/D61509



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

[PATCH] D62238: [CodeComplete] Complete a lambda when preferred type is a function

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 200745.
ilya-biryukov marked 4 inline comments as done.
ilya-biryukov added a comment.

- Only work on sugared types
- Do not check for 'function' in the name


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62238

Files:
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/lambdas.cpp

Index: clang/test/CodeCompletion/lambdas.cpp
===
--- /dev/null
+++ clang/test/CodeCompletion/lambdas.cpp
@@ -0,0 +1,53 @@
+template 
+struct function {
+};
+
+
+void test() {
+  void (*x)(int, double) = nullptr;
+
+  function y = {};
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:7:28 %s -o - | FileCheck -check-prefix=CHECK-1 %s
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:9:35 %s -o - | FileCheck -check-prefix=CHECK-1 %s
+  // CHECK-1: COMPLETION: Pattern : [<#=#>](int <#parameter#>, double <#parameter#>){<#body#>}
+
+  // == Placeholders for suffix types must be placed properly.
+  function z = {};
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:15:36 %s -o - | FileCheck -check-prefix=CHECK-2 %s
+  // CHECK-2: COMPLETION: Pattern : [<#=#>](void (* <#parameter#>)(int)){<#body#>}
+
+  // == No need for a parameter list if function has no parameters.
+  function a = {};
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:20:24 %s -o - | FileCheck -check-prefix=CHECK-3 %s
+  // CHECK-3: COMPLETION: Pattern : [<#=#>]{<#body#>}
+}
+
+template 
+struct vector {};
+
+void test2() {
+  // == Try to preserve types as written.
+  function)> a = {};
+
+  using function_typedef = function)>;
+  function_typedef b = {};
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:30:35 %s -o - | FileCheck -check-prefix=CHECK-4 %s
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:33:24 %s -o - | FileCheck -check-prefix=CHECK-4 %s
+  // CHECK-4: COMPLETION: Pattern : [<#=#>](vector <#parameter#>){<#body#>}
+}
+
+// Check another common function wrapper name.
+template  struct unique_function {};
+
+void test3() {
+  unique_function a = {};
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:43:31 %s -o - | FileCheck -check-prefix=CHECK-5 %s
+  // CHECK-5: COMPLETION: Pattern : [<#=#>]{<#body#>}
+}
+
+template  struct weird_function {};
+void test4() {
+  weird_function b = {};
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:50:35 %s -o - | FileCheck -check-prefix=CHECK-6 %s
+  // CHECK-6-NOT: COMPLETION: Pattern : [<#=
+}
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -4108,6 +4108,71 @@
   Results.ExitScope();
 }
 
+/// Try to find a corresponding FunctionProtoType for function-like types (e.g.
+/// function pointers, std::function, etc).
+static const FunctionProtoType *TryDeconstructFunctionLike(QualType T) {
+  assert(!T.isNull());
+  // Try to extract first template argument from std::function<> and similar.
+  // Note we only handle the sugared types, they closely match what users wrote.
+  // We explicitly choose to not handle ClassTemplateSpecializationDecl.
+  if (auto *Specialization = T->getAs()) {
+// FIXME: handle other kinds of template names.
+auto *Template = Specialization->getTemplateName().getAsTemplateDecl();
+if (!Template || Specialization->getNumArgs() != 1)
+  return nullptr;
+const TemplateArgument &Argument = Specialization->getArg(0);
+if (Argument.getKind() != TemplateArgument::Type)
+  return nullptr;
+return Argument.getAsType()->getAs();
+  }
+  // Handle other cases.
+  if (T->isPointerType())
+T = T->getPointeeType();
+  return T->getAs();
+}
+
+/// Adds a pattern completion for a lambda expression with the specified
+/// parameter types and placeholders for parameter names.
+static void AddLambdaCompletion(ResultBuilder &Results,
+llvm::ArrayRef Parameters,
+const LangOptions &LangOpts) {
+  CodeCompletionBuilder Completion(Results.getAllocator(),
+   Results.getCodeCompletionTUInfo());
+  // []() {}
+  Completion.AddChunk(CodeCompletionString::CK_LeftBracket);
+  Completion.AddPlaceholderChunk("=");
+  Completion.AddChunk(CodeCompletionString::CK_RightBracket);
+  if (!Parameters.empty()) {
+Completion.AddChunk(CodeCompletionString::CK_LeftParen);
+bool First = true;
+for (auto Parameter : Parameters) {
+  if (!First)
+Completion.AddChunk(CodeCompletionString::ChunkKind::CK_Comma);
+  else
+First = false;
+
+  constex

[PATCH] D62238: [CodeComplete] Complete a lambda when preferred type is a function

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4135
+!PotentialTemplateName.getAsIdentifierInfo()->getName().contains(
+"function"))
+  return nullptr;

kadircet wrote:
> ilya-biryukov wrote:
> > kadircet wrote:
> > > This looks cheesy, do we really want to perform this operation only for 
> > > templates with `"function"` in their names?
> > > 
> > > As discussed offline maybe perform this for any template with a 
> > > single(non-defaulted?) argument, or maybe even better perform a 
> > > type-check as suggested by you. But I believe there would be too many 
> > > false positives with the current state.
> > WDYT about `"function"` + only template with a single arg?
> > FWIW, I think function in template arguments are very rare, so I'm more 
> > optimistic about false-positives even in the current form.
> > 
> > We have an option of making it super-restrictive and only firing on 
> > whitelisted things like `std::function`, `boost::function`, etc.
> > But we don't have a mechanism to configure those from the outside, so that 
> > will probably turn out too restrictive.
> I don't think adding "function" as a restriction is helping much on reducing 
> false positives. I would rather get rid of that check to increase usability.
> 
> The real problem is rather checking constructors to make sure there is at 
> least one for which we can provide a callable with the signature we found. 
> Anything else just seems cheesy and might result in people adding more and 
> more cases over time.
> 
> It is up to you, I am OK if you choose to keep this restriction as well.
Removed the restriction on the function way.
I don't think there's an easy way to check for conversions, though, I hope the 
false-positive rate would super-small.
If not, I'll find a way to check conversions or whitelist only known function 
names.



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4125
+PotentialFunctionArg = &Specialization->getArg(0);
+  } else if (auto *Class = dyn_cast_or_null(
+ T->getAsCXXRecordDecl())) {

kadircet wrote:
> I thought you decided to get rid of this branch after the offline 
> discussions, sorry for not mentioning in earlier revision.
Sorry, my bad, forgot about it.



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4142
+  // Handle other cases.
+  if (T->isPointerType())
+T = T->getPointeeType();

kadircet wrote:
> what about referencetype?
> 
> ```
> void test();
> void (&y)() = test;
> ```
Lambdas are r-values, so suggesting them for l-value references would lead to 
an error.
R-value references to functions are so rare that I feel handling those is not 
worth it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62238



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

[PATCH] D62160: [LibTooling] Update Stencil to use RangeSelector

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 200747.
ymandel added a comment.

Add back `node` and `sNode`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62160

Files:
  clang/include/clang/Tooling/Refactoring/Stencil.h
  clang/lib/Tooling/Refactoring/Stencil.cpp
  clang/unittests/Tooling/StencilTest.cpp

Index: clang/unittests/Tooling/StencilTest.cpp
===
--- clang/unittests/Tooling/StencilTest.cpp
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -22,8 +22,6 @@
 using ::testing::Eq;
 using ::testing::HasSubstr;
 using MatchResult = MatchFinder::MatchResult;
-using tooling::stencil::node;
-using tooling::stencil::sNode;
 using tooling::stencil::text;
 
 // In tests, we can't directly match on llvm::Expected since its accessors
@@ -141,8 +139,8 @@
   hasThen(stmt().bind(Then)), hasElse(stmt().bind(Else;
   ASSERT_TRUE(StmtMatch);
   // Invert the if-then-else.
-  auto Stencil = Stencil::cat("if (!", node(Condition), ") ", sNode(Else),
-  " else ", sNode(Then));
+  auto Stencil = Stencil::cat("if (!", node(Condition), ") ", statement(Else),
+  " else ", statement(Then));
   EXPECT_THAT(toOptional(Stencil.eval(StmtMatch->Result)),
   IsSomething(Eq("if (!true) return 0; else return 1;")));
 }
@@ -160,8 +158,8 @@
   hasThen(stmt().bind(Then)), hasElse(stmt().bind(Else;
   ASSERT_TRUE(StmtMatch);
   // Invert the if-then-else.
-  Stencil S = Stencil::cat("if (!", node(Condition), ") ", sNode(Else),
-  " else ", sNode(Then));
+  Stencil S = Stencil::cat("if (!", node(Condition), ") ", statement(Else),
+   " else ", statement(Then));
   EXPECT_THAT(toOptional(S(StmtMatch->Result)),
   IsSomething(Eq("if (!true) return 0; else return 1;")));
 }
@@ -176,7 +174,7 @@
   auto StmtMatch = matchStmt(Snippet, ifStmt(hasCondition(stmt().bind("a1")),
  hasThen(stmt().bind("a2";
   ASSERT_TRUE(StmtMatch);
-  auto Stencil = Stencil::cat("if(!", sNode("a1"), ") ", node("UNBOUND"), ";");
+  auto Stencil = Stencil::cat("if(!", node("a1"), ") ", node("UNBOUND"), ";");
   auto ResultOrErr = Stencil.eval(StmtMatch->Result);
   EXPECT_TRUE(llvm::errorToBool(ResultOrErr.takeError()))
   << "Expected unbound node, got " << *ResultOrErr;
@@ -192,32 +190,36 @@
   IsSomething(Expected));
 }
 
-TEST_F(StencilTest, NodeOp) {
+TEST_F(StencilTest, SelectionOp) {
   StringRef Id = "id";
   testExpr(Id, "3;", Stencil::cat(node(Id)), "3");
 }
 
-TEST_F(StencilTest, SNodeOp) {
-  StringRef Id = "id";
-  testExpr(Id, "3;", Stencil::cat(sNode(Id)), "3;");
-}
-
 TEST(StencilEqualityTest, Equality) {
   using stencil::dPrint;
-  auto Lhs = Stencil::cat("foo", node("node"), dPrint("dprint_id"));
+  auto Lhs = Stencil::cat("foo", dPrint("dprint_id"));
   auto Rhs = Lhs;
   EXPECT_EQ(Lhs, Rhs);
 }
 
 TEST(StencilEqualityTest, InEqualityDifferentOrdering) {
-  auto Lhs = Stencil::cat("foo", node("node"));
-  auto Rhs = Stencil::cat(node("node"), "foo");
+  using stencil::dPrint;
+  auto Lhs = Stencil::cat("foo", dPrint("node"));
+  auto Rhs = Stencil::cat(dPrint("node"), "foo");
   EXPECT_NE(Lhs, Rhs);
 }
 
 TEST(StencilEqualityTest, InEqualityDifferentSizes) {
-  auto Lhs = Stencil::cat("foo", node("node"), "bar", "baz");
-  auto Rhs = Stencil::cat("foo", node("node"), "bar");
+  using stencil::dPrint;
+  auto Lhs = Stencil::cat("foo", dPrint("node"), "bar", "baz");
+  auto Rhs = Stencil::cat("foo", dPrint("node"), "bar");
   EXPECT_NE(Lhs, Rhs);
 }
+
+// node is opaque and therefore cannot be examined for equality.
+TEST(StencilEqualityTest, InEqualitySelection) {
+  using stencil::dPrint;
+  auto S = Stencil::cat(node("node"));
+  EXPECT_NE(S, S);
+}
 } // namespace
Index: clang/lib/Tooling/Refactoring/Stencil.cpp
===
--- clang/lib/Tooling/Refactoring/Stencil.cpp
+++ clang/lib/Tooling/Refactoring/Stencil.cpp
@@ -55,22 +55,11 @@
   explicit DebugPrintNodeOpData(std::string S) : Id(std::move(S)) {}
   std::string Id;
 };
-// Whether to associate a trailing semicolon with a node when identifying it's
-// text.  This flag is needed for expressions (clang::Expr), because their role
-// is ambiguous when they are also complete statements.  When this flag is
-// `Always`, an expression node will be treated like a statement, and will
-// therefore be associated with any trailing semicolon.
-enum class SemiAssociation : bool {
-  Always,
-  Inferred,
-};
 
-// A reference to a particular (bound) AST node.
-struct NodeRefData {
-  explicit NodeRefData(std::string S, SemiAssociation SA)
-  : Id(std::move(S)), SemiAssoc(SA) {}
-  std::string Id;
-  SemiAssociation SemiAssoc;
+// The fragment of code corresponding to 

[PATCH] D61386: [clang-tidy] Add support writing a check as a Transformer rewrite rule.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked an inline comment as done.
ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp:46
+
+  StringRef Message = "no explanation";
+  if (Case.Explanation) {

ymandel wrote:
> ilya-biryukov wrote:
> > The users will see this for every case without explanation, right?
> > I'd expect the rules without explanation to be somewhat common, maybe don't 
> > show any message at all in that case?
> There's no option to call `diag()` without a message.  We could pass an empty 
> string , but that may be confusing given the way the message is concatenated 
> here:
> https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp#L204
> 
> So, no matter what, there will be some message to go w/ the diagnostic. I 
> figure that being explicit about the lack of explanation is better than an 
> empty string, but don't feel strongly about this.
Ah, so all the options are bad. I can see why you had this design in 
transformers in the first place.
I wonder if we should check the explanations are always set for rewrite rules 
inside the clang-tidy transformation?



Comment at: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h:1
+//===-- TransformerClangTidyCheck.h - clang-tidy 
--===//
+//

ymandel wrote:
> ilya-biryukov wrote:
> > NIT: maybe use `TransformerCheck` for brevity? I'm not a `clang-tidy` 
> > maintainer, though, so not sure whether that aligns with the rest of the 
> > code.
> It was TransformerTidy and Dmitri suggested I be explicit. I think 
> TransformerCheck is better than TransformerTidy, but I also see the argument 
> in spelling it out.  WDYT?
Let's stick with Dmitri's suggestion. He has much more experience with LLVM 
than I do :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61386



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

[PATCH] D59407: [clangd] Add RelationSlab

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

Mostly LG from my side, thanks!




Comment at: clang-tools-extra/clangd/index/Relation.cpp:19
+llvm::iterator_range
+RelationSlab::lookup(const SymbolID &Subject,
+ index::SymbolRole Predicate) const {

I would suggest adding another version which returns all the relations a given 
`Subject` participates, but I suppose currently we don't have any use cases.

Feel free to add or skip, that can be added when necessary.



Comment at: clang-tools-extra/clangd/index/Relation.cpp:24
+   [](const Relation &A, const Relation &B) {
+ return (A.Subject < B.Subject) ||
+(A.Subject == B.Subject &&

`std::tie`



Comment at: clang-tools-extra/clangd/index/Relation.h:31
+  bool operator==(const Relation &Other) const {
+return Subject == Other.Subject && Predicate == Other.Predicate &&
+   Object == Other.Object;

nit: could you use `std::tie`



Comment at: clang-tools-extra/clangd/index/Relation.h:35
+  // SPO order
+  bool operator<(const Relation &Other) const {
+if (Subject < Other.Subject) {

again `std::tie`



Comment at: clang-tools-extra/clangd/index/Relation.h:52
+
+class RelationSlab {
+public:

I believe `RelationSlab` should still be immutable, even after the `build` 
operation below `RelationSlab` is mutable.
Let's introduce the builder class and move mutating operations and build into 
it. So that it would be more symmetric to other slabs we have.

Also are there any use-cases requiring RelationSlab to be mutable?



Comment at: clang-tools-extra/clangd/index/Relation.h:60
+  // that possibility for the future. Having a build() method is
+  // also useful so when know when to sort the relations.
+  using Builder = RelationSlab;

there seems to be some typos in the comment



Comment at: clang-tools-extra/clangd/index/Relation.h:72
+
+  void push_back(const Relation &R) { Relations.push_back(R); }
+

maybe rename to `insert` to keep symmetry with other slabs?
also it suggests a semantics on the operation that we don't care about.



Comment at: clang-tools-extra/clangd/index/Relation.h:79
+  /// Sort relations in SPO order.
+  void sort();
+

maybe even get rid of sort by inlining it into build?



Comment at: clang-tools-extra/clangd/index/Relation.h:82
+  /// Lookup all relations matching the given subject and predicate.
+  /// Assumes the slab is sorted in SPO order.
+  llvm::iterator_range lookup(const SymbolID &Subject,

again, we could get rid of this comment if slab was immutable



Comment at: clang-tools-extra/clangd/index/Relation.h:89
+private:
+  RelationSlab(std::vector Relations)
+  : Relations(std::move(Relations)) {}

Do we really need this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59407



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

[PATCH] D62238: [CodeComplete] Complete a lambda when preferred type is a function

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet accepted this revision.
kadircet marked an inline comment as done.
kadircet added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!




Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4142
+  // Handle other cases.
+  if (T->isPointerType())
+T = T->getPointeeType();

ilya-biryukov wrote:
> kadircet wrote:
> > what about referencetype?
> > 
> > ```
> > void test();
> > void (&y)() = test;
> > ```
> Lambdas are r-values, so suggesting them for l-value references would lead to 
> an error.
> R-value references to functions are so rare that I feel handling those is not 
> worth it.
ah right


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62238



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

[PATCH] D59415: Do not resolve directory junctions for `-fdiagnostics-absolute-paths` on Windows.

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin updated this revision to Diff 200749.
ikudrin edited the summary of this revision.
ikudrin added a comment.

Added a comment explaining the differences in the implementations. I would 
really appreciate any corrections.


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

https://reviews.llvm.org/D59415

Files:
  lib/Frontend/TextDiagnostic.cpp
  test/Frontend/absolute-paths-windows.test
  test/Frontend/lit.local.cfg


Index: test/Frontend/lit.local.cfg
===
--- test/Frontend/lit.local.cfg
+++ test/Frontend/lit.local.cfg
@@ -1 +1 @@
-config.suffixes = ['.c', '.cpp', '.m', '.mm', '.ll', '.cl']
+config.suffixes = ['.c', '.cpp', '.m', '.mm', '.ll', '.cl', '.test']
Index: test/Frontend/absolute-paths-windows.test
===
--- /dev/null
+++ test/Frontend/absolute-paths-windows.test
@@ -0,0 +1,9 @@
+// REQUIRES: system-windows
+// RUN: rm -rf %t.dir
+// RUN: mkdir -p %t.dir\real
+// RUN: cmd /c mklink /j %t.dir\junc %t.dir\real
+// RUN: echo "wrong code" > %t.dir\real\foo.cpp
+// RUN: not %clang_cc1 -fsyntax-only -fdiagnostics-absolute-paths 
%t.dir\junc\foo.cpp 2>&1 | FileCheck %s
+
+// CHECK-NOT: .dir\real\foo.cpp
+// CHECK: .dir\junc\foo.cpp
Index: lib/Frontend/TextDiagnostic.cpp
===
--- lib/Frontend/TextDiagnostic.cpp
+++ lib/Frontend/TextDiagnostic.cpp
@@ -765,7 +765,28 @@
 const DirectoryEntry *Dir = SM.getFileManager().getDirectory(
 llvm::sys::path::parent_path(Filename));
 if (Dir) {
+  // We want to print a simplified absolute path, i. e. without "dots".
+  //
+  // The hardest part here are the paths like "//../".
+  // On Unix-like systems, we cannot just collapse "/..", because
+  // paths are resolved sequentially, and, thereby, the path
+  // "/" may point to a different location. That is why
+  // we use FileManager::getCanonicalName(), which expands all indirections
+  // with llvm::sys::fs::real_path() and caches the result.
+  //
+  // On the other hand, it would be better to preserve as much of the
+  // original path as possible, because that helps a user to recognize it.
+  // real_path() expands all links, which sometimes too much. Luckily,
+  // on Windows we can just use llvm::sys::path::remove_dots(), because,
+  // on that system, both aforementioned paths point to the same place.
+#ifdef _WIN32
+  SmallString<4096> DirName = Dir->getName();
+  llvm::sys::fs::make_absolute(DirName);
+  llvm::sys::path::native(DirName);
+  llvm::sys::path::remove_dots(DirName, /* remove_dot_dot */ true);
+#else
   StringRef DirName = SM.getFileManager().getCanonicalName(Dir);
+#endif
   llvm::sys::path::append(AbsoluteFilename, DirName,
   llvm::sys::path::filename(Filename));
   Filename = StringRef(AbsoluteFilename.data(), AbsoluteFilename.size());


Index: test/Frontend/lit.local.cfg
===
--- test/Frontend/lit.local.cfg
+++ test/Frontend/lit.local.cfg
@@ -1 +1 @@
-config.suffixes = ['.c', '.cpp', '.m', '.mm', '.ll', '.cl']
+config.suffixes = ['.c', '.cpp', '.m', '.mm', '.ll', '.cl', '.test']
Index: test/Frontend/absolute-paths-windows.test
===
--- /dev/null
+++ test/Frontend/absolute-paths-windows.test
@@ -0,0 +1,9 @@
+// REQUIRES: system-windows
+// RUN: rm -rf %t.dir
+// RUN: mkdir -p %t.dir\real
+// RUN: cmd /c mklink /j %t.dir\junc %t.dir\real
+// RUN: echo "wrong code" > %t.dir\real\foo.cpp
+// RUN: not %clang_cc1 -fsyntax-only -fdiagnostics-absolute-paths %t.dir\junc\foo.cpp 2>&1 | FileCheck %s
+
+// CHECK-NOT: .dir\real\foo.cpp
+// CHECK: .dir\junc\foo.cpp
Index: lib/Frontend/TextDiagnostic.cpp
===
--- lib/Frontend/TextDiagnostic.cpp
+++ lib/Frontend/TextDiagnostic.cpp
@@ -765,7 +765,28 @@
 const DirectoryEntry *Dir = SM.getFileManager().getDirectory(
 llvm::sys::path::parent_path(Filename));
 if (Dir) {
+  // We want to print a simplified absolute path, i. e. without "dots".
+  //
+  // The hardest part here are the paths like "//../".
+  // On Unix-like systems, we cannot just collapse "/..", because
+  // paths are resolved sequentially, and, thereby, the path
+  // "/" may point to a different location. That is why
+  // we use FileManager::getCanonicalName(), which expands all indirections
+  // with llvm::sys::fs::real_path() and caches the result.
+  //
+  // On the other hand, it would be better to preserve as much of the
+  // original path as possible, because that helps a user to recognize it.
+  // real_path() expands all links, which sometimes too much. Luckily,
+  // on Windows we can just use l

[PATCH] D62238: [CodeComplete] Complete a lambda when preferred type is a function

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Thanks for a quick review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62238



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

[PATCH] D61386: [clang-tidy] Add support writing a check as a Transformer rewrite rule.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel marked 4 inline comments as done.
ymandel added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp:46
+
+  StringRef Message = "no explanation";
+  if (Case.Explanation) {

ilya-biryukov wrote:
> ymandel wrote:
> > ilya-biryukov wrote:
> > > The users will see this for every case without explanation, right?
> > > I'd expect the rules without explanation to be somewhat common, maybe 
> > > don't show any message at all in that case?
> > There's no option to call `diag()` without a message.  We could pass an 
> > empty string , but that may be confusing given the way the message is 
> > concatenated here:
> > https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp#L204
> > 
> > So, no matter what, there will be some message to go w/ the diagnostic. I 
> > figure that being explicit about the lack of explanation is better than an 
> > empty string, but don't feel strongly about this.
> Ah, so all the options are bad. I can see why you had this design in 
> transformers in the first place.
> I wonder if we should check the explanations are always set for rewrite rules 
> inside the clang-tidy transformation?
> Ah, so all the options are bad. I can see why you had this design in 
> transformers in the first place.
Heh. indeed.

> I wonder if we should check the explanations are always set for rewrite rules 
> inside the clang-tidy transformation?> Quoted Text

I would have thought so, but AFAIK, most folks who write one-off 
transformations use clang-tidy, rather than writing a standalone tool. They 
just ignore the diagnostics, i gather.  Transformer may shift that somewhat if 
we improve the experience of writing a (throwaway) standalone tool, but for the 
time being I think we can't assume that.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61386



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

[PATCH] D62244: [AMDGPU] Enable the implicit arguments for HIP (CLANG)

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

Currently HIP and CUDA share the same test directories, so better put the test 
in CodeGenCUDA.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62244



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

[PATCH] D62208: [OpenCL] Enable queue_t and clk_event_t comparisons in C++ mode

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D62208



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

[PATCH] D61923: [GWP-ASan] Mutex implementation [2].

[Mitch Phillips via Phabricator via cfe-commits]

hctim added inline comments.



Comment at: compiler-rt/lib/gwp_asan/platform_specific/mutex_posix.cpp:43
+
+Mutex::Mutex() : PImpl(new Impl) {}
+Mutex::~Mutex() { delete PImpl; }

eugenis wrote:
> This is a dependency on libc++ / libstdc++.
> 
> I'm not sure about using malloc() inside mutex constructor, either.
> 
> We will probably want the mutex to be linker-initialized, too. AFAIK, 
> gwp-asan would not have any clear initialization entry point, and would need 
> to do this lazily in malloc, which can there be several of, concurrently.
> 
As per offline discussion, have ifdef-d the platform-specific headers into this 
file. Allows us to have linker initialisation (at least for POSIX).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61923



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

[PATCH] D61923: [GWP-ASan] Mutex implementation [2].

[Mitch Phillips via Phabricator via cfe-commits]

hctim updated this revision to Diff 200760.
hctim marked 2 inline comments as done.
hctim added a comment.

- Ifdef-d headers into mutex.h


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61923

Files:
  clang/runtime/CMakeLists.txt
  compiler-rt/lib/gwp_asan/CMakeLists.txt
  compiler-rt/lib/gwp_asan/mutex.h
  compiler-rt/lib/gwp_asan/platform_specific/mutex_posix.cpp
  compiler-rt/lib/gwp_asan/tests/CMakeLists.txt
  compiler-rt/lib/gwp_asan/tests/driver.cpp
  compiler-rt/lib/gwp_asan/tests/mutex_test.cpp
  compiler-rt/test/gwp_asan/CMakeLists.txt
  compiler-rt/test/gwp_asan/dummy_test.cc
  compiler-rt/test/gwp_asan/lit.cfg
  compiler-rt/test/gwp_asan/lit.site.cfg.in
  compiler-rt/test/gwp_asan/unit/lit.site.cfg.in

Index: compiler-rt/test/gwp_asan/unit/lit.site.cfg.in
===
--- /dev/null
+++ compiler-rt/test/gwp_asan/unit/lit.site.cfg.in
@@ -0,0 +1,9 @@
+@LIT_SITE_CFG_IN_HEADER@
+
+config.name = "GwpAsan-Unittest"
+# Load common config for all compiler-rt unit tests.
+lit_config.load_config(config, "@COMPILER_RT_BINARY_DIR@/unittests/lit.common.unit.configured")
+
+config.test_exec_root = os.path.join("@COMPILER_RT_BINARY_DIR@",
+ "lib", "gwp_asan", "tests")
+config.test_source_root = config.test_exec_root
Index: compiler-rt/test/gwp_asan/lit.site.cfg.in
===
--- /dev/null
+++ compiler-rt/test/gwp_asan/lit.site.cfg.in
@@ -0,0 +1,11 @@
+@LIT_SITE_CFG_IN_HEADER@
+
+config.name_suffix = "@GWP_ASAN_TEST_CONFIG_SUFFIX@"
+config.target_arch = "@GWP_ASAN_TEST_TARGET_ARCH@"
+config.target_cflags = "@GWP_ASAN_TEST_TARGET_CFLAGS@"
+
+# Load common config for all compiler-rt lit tests.
+lit_config.load_config(config, "@COMPILER_RT_BINARY_DIR@/test/lit.common.configured")
+
+# Load tool-specific config that would do the real work.
+lit_config.load_config(config, "@GWP_ASAN_LIT_SOURCE_DIR@/lit.cfg")
Index: compiler-rt/test/gwp_asan/lit.cfg
===
--- /dev/null
+++ compiler-rt/test/gwp_asan/lit.cfg
@@ -0,0 +1,31 @@
+# -*- Python -*-
+
+import os
+
+# Setup config name.
+config.name = 'GWP-ASan' + config.name_suffix
+
+# Setup source root.
+config.test_source_root = os.path.dirname(__file__)
+
+# Test suffixes.
+config.suffixes = ['.c', '.cc', '.cpp', '.test']
+
+# C & CXX flags.
+c_flags = ([config.target_cflags])
+
+# Android doesn't want -lrt.
+if not config.android:
+  c_flags += ["-lrt"]
+
+cxx_flags = (c_flags + config.cxx_mode_flags + ["-std=c++11"])
+
+def build_invocation(compile_flags):
+  return " " + " ".join([config.clang] + compile_flags) + " "
+
+# Add substitutions.
+config.substitutions.append(("%clang ", build_invocation(c_flags)))
+
+# GWP-ASan tests are currently supported on Linux only.
+if config.host_os not in ['Linux']:
+   config.unsupported = True
Index: compiler-rt/test/gwp_asan/dummy_test.cc
===
--- /dev/null
+++ compiler-rt/test/gwp_asan/dummy_test.cc
@@ -0,0 +1,4 @@
+// Exists to simply stop warnings about lit not discovering any tests here.
+// RUN: %clang %s
+
+int main() { return 0; }
Index: compiler-rt/test/gwp_asan/CMakeLists.txt
===
--- compiler-rt/test/gwp_asan/CMakeLists.txt
+++ compiler-rt/test/gwp_asan/CMakeLists.txt
@@ -0,0 +1,45 @@
+set(GWP_ASAN_LIT_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
+set(GWP_ASAN_LIT_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
+
+set(GWP_ASAN_TESTSUITES)
+
+set(GWP_ASAN_UNITTEST_DEPS)
+set(GWP_ASAN_TEST_DEPS
+  ${SANITIZER_COMMON_LIT_TEST_DEPS}
+  gwp_asan)
+
+if (COMPILER_RT_INCLUDE_TESTS)
+  list(APPEND GWP_ASAN_TEST_DEPS GwpAsanUnitTests)
+  configure_lit_site_cfg(
+${CMAKE_CURRENT_SOURCE_DIR}/unit/lit.site.cfg.in
+${CMAKE_CURRENT_BINARY_DIR}/unit/lit.site.cfg)
+  add_lit_testsuite(check-gwp_asan-unit "Running GWP-ASan unit tests"
+${CMAKE_CURRENT_BINARY_DIR}/unit
+DEPENDS ${GWP_ASAN_TEST_DEPS})
+  set_target_properties(check-gwp_asan-unit PROPERTIES FOLDER
+"Compiler-RT Tests")
+list(APPEND GWP_ASAN_TEST_DEPS check-gwp_asan-unit)
+endif()
+
+configure_lit_site_cfg(
+  ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.in
+  ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg
+  )
+
+foreach(arch ${GWP_ASAN_SUPPORTED_ARCH})
+  set(GWP_ASAN_TEST_TARGET_ARCH ${arch})
+  string(TOLOWER "-${arch}" GWP_ASAN_TEST_CONFIG_SUFFIX)
+  get_test_cc_for_arch(${arch} GWP_ASAN_TEST_TARGET_CC GWP_ASAN_TEST_TARGET_CFLAGS)
+  string(TOUPPER ${arch} ARCH_UPPER_CASE)
+  set(CONFIG_NAME ${ARCH_UPPER_CASE}${OS_NAME}Config)
+
+  configure_lit_site_cfg(
+${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.in
+${CMAKE_CURRENT_BINARY_DIR}/${CONFIG_NAME}/lit.site.cfg)
+  list(APPEND GWP_ASAN_TESTSUITES ${CMAKE_CURRENT_BINARY_DIR}/${CONFIG_NAME})
+endforeach(

r361400 - Combine two if cases because the second one is never reached.

[Amy Huang via cfe-commits]

Author: akhuang
Date: Wed May 22 08:48:59 2019
New Revision: 361400

URL: http://llvm.org/viewvc/llvm-project?rev=361400&view=rev
Log:
Combine two if cases because the second one is never reached.

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=361400&r1=361399&r2=361400&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Wed May 22 08:48:59 2019
@@ -4352,16 +4352,14 @@ void CGDebugInfo::EmitGlobalVariable(con
   llvm::DIFile *Unit = getOrCreateFile(VD->getLocation());
   StringRef Name = VD->getName();
   llvm::DIType *Ty = getOrCreateType(VD->getType(), Unit);
+
+  // Do not use global variables for enums.
   if (const auto *ECD = dyn_cast(VD)) {
 const auto *ED = cast(ECD->getDeclContext());
 assert(isa(ED->getTypeForDecl()) && "Enum without EnumType?");
-Ty = getOrCreateType(QualType(ED->getTypeForDecl(), 0), Unit);
-  }
-  // Do not use global variables for enums.
-  //
-  // FIXME: why not?
-  if (Ty->getTag() == llvm::dwarf::DW_TAG_enumeration_type)
 return;
+  }
+
   // Do not emit separate definitions for function local const/statics.
   if (isa(VD->getDeclContext()))
 return;


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

[PATCH] D62214: Remove extra if case.

[Amy Huang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL361400: Combine two if cases because the second one is never 
reached. (authored by akhuang, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62214?vs=200558&id=200765#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62214

Files:
  cfe/trunk/lib/CodeGen/CGDebugInfo.cpp


Index: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
===
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
@@ -4352,16 +4352,14 @@
   llvm::DIFile *Unit = getOrCreateFile(VD->getLocation());
   StringRef Name = VD->getName();
   llvm::DIType *Ty = getOrCreateType(VD->getType(), Unit);
+
+  // Do not use global variables for enums.
   if (const auto *ECD = dyn_cast(VD)) {
 const auto *ED = cast(ECD->getDeclContext());
 assert(isa(ED->getTypeForDecl()) && "Enum without EnumType?");
-Ty = getOrCreateType(QualType(ED->getTypeForDecl(), 0), Unit);
-  }
-  // Do not use global variables for enums.
-  //
-  // FIXME: why not?
-  if (Ty->getTag() == llvm::dwarf::DW_TAG_enumeration_type)
 return;
+  }
+
   // Do not emit separate definitions for function local const/statics.
   if (isa(VD->getDeclContext()))
 return;


Index: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
===
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
@@ -4352,16 +4352,14 @@
   llvm::DIFile *Unit = getOrCreateFile(VD->getLocation());
   StringRef Name = VD->getName();
   llvm::DIType *Ty = getOrCreateType(VD->getType(), Unit);
+
+  // Do not use global variables for enums.
   if (const auto *ECD = dyn_cast(VD)) {
 const auto *ED = cast(ECD->getDeclContext());
 assert(isa(ED->getTypeForDecl()) && "Enum without EnumType?");
-Ty = getOrCreateType(QualType(ED->getTypeForDecl(), 0), Unit);
-  }
-  // Do not use global variables for enums.
-  //
-  // FIXME: why not?
-  if (Ty->getTag() == llvm::dwarf::DW_TAG_enumeration_type)
 return;
+  }
+
   // Do not emit separate definitions for function local const/statics.
   if (isa(VD->getDeclContext()))
 return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62174: [Analysis] Link library dependencies to Analysis plugins

[David Zarzycki via Phabricator via cfe-commits]

davezarzycki added a comment.

Fixed: r361399


Repository:
  rC Clang

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

https://reviews.llvm.org/D62174



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

[PATCH] D53847: [C++2a] P0634r3: Down with typename!

[Nicolas Lesser via Phabricator via cfe-commits]

Rakete updated this revision to Diff 200767.
Rakete marked 11 inline comments as done.
Rakete added a comment.

- rebased
- addressed review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D53847

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Parse/ParseTentative.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/CXX/basic/basic.lookup/basic.lookup.qual/class.qual/p2.cpp
  clang/test/CXX/drs/dr1xx.cpp
  clang/test/CXX/drs/dr2xx.cpp
  clang/test/CXX/drs/dr4xx.cpp
  clang/test/CXX/drs/dr5xx.cpp
  clang/test/CXX/temp/temp.res/p3.cpp
  clang/test/CXX/temp/temp.res/p5.cpp
  clang/test/CXX/temp/temp.res/temp.dep/temp.dep.type/p1.cpp
  clang/test/FixIt/fixit.cpp
  clang/test/Parser/cxx-member-initializers.cpp
  clang/test/SemaCXX/MicrosoftCompatibility.cpp
  clang/test/SemaCXX/MicrosoftExtensions.cpp
  clang/test/SemaCXX/MicrosoftSuper.cpp
  clang/test/SemaCXX/unknown-type-name.cpp
  clang/test/SemaTemplate/alias-templates.cpp
  clang/test/SemaTemplate/typename-specifier-3.cpp

Index: clang/test/SemaTemplate/typename-specifier-3.cpp
===
--- clang/test/SemaTemplate/typename-specifier-3.cpp
+++ clang/test/SemaTemplate/typename-specifier-3.cpp
@@ -27,7 +27,7 @@
   typedef int arg;
 };
 struct C {
-  typedef B::X x; // expected-error {{missing 'typename'}}
+  typedef B::X x; // expected-warning {{missing 'typename'}}
 };
   };
 
Index: clang/test/SemaTemplate/alias-templates.cpp
===
--- clang/test/SemaTemplate/alias-templates.cpp
+++ clang/test/SemaTemplate/alias-templates.cpp
@@ -195,11 +195,10 @@
   struct base {
 template  struct derived;
   };
-  // FIXME: The diagnostics here are terrible.
   template 
-  using derived = base::template derived; // expected-error {{expected a type}} expected-error {{expected ';'}}
+  using derived = base::template derived; // expected-warning {{missing 'typename'}}
   template 
-  using derived2 = ::PR16904::base::template derived; // expected-error {{expected a type}} expected-error {{expected ';'}}
+  using derived2 = ::PR16904::base::template derived; // expected-warning {{missing 'typename'}}
 }
 
 namespace PR14858 {
Index: clang/test/SemaCXX/unknown-type-name.cpp
===
--- clang/test/SemaCXX/unknown-type-name.cpp
+++ clang/test/SemaCXX/unknown-type-name.cpp
@@ -36,15 +36,15 @@
 
   static int n;
   static type m;
-  static int h(T::type, int); // expected-error{{missing 'typename'}}
-  static int h(T::type x, char); // expected-error{{missing 'typename'}}
+  static int h(T::type, int); // expected-warning{{implicit 'typename' is a C++2a extension}}
+  static int h(T::type x, char); // expected-warning{{implicit 'typename' is a C++2a extension}}
 };
 
 template
-A::type g(T t) { return t; } // expected-error{{missing 'typename'}}
+A::type g(T t) { return t; } // expected-warning{{implicit 'typename' is a C++2a extension}}
 
 template
-A::type A::f() { return type(); } // expected-error{{missing 'typename'}}
+A::type A::f() { return type(); } // expected-warning{{implicit 'typename' is a C++2a extension}}
 
 template
 void f(T::type) { } // expected-error{{missing 'typename'}}
@@ -84,11 +84,11 @@
 
 template int A::n(T::value); // ok
 template
-A::type // expected-error{{missing 'typename'}}
+A::type // expected-warning {{implicit 'typename' is a C++2a extension}}
 A::m(T::value, 0); // ok
 
-template int A::h(T::type, int) {} // expected-error{{missing 'typename'}}
-template int A::h(T::type x, char) {} // expected-error{{missing 'typename'}}
+template int A::h(T::type, int) {} // expected-warning{{implicit 'typename' is a C++2a extension}}
+template int A::h(T::type x, char) {} // expected-warning{{implicit 'typename' is a C++2a extension}}
 
 template int h(T::type, int); // expected-error{{missing 'typename'}}
 template int h(T::type x, char); // expected-error{{missing 'typename'}}
@@ -116,4 +116,5 @@
 // FIXME: We know which type specifier should have been specified here. Provide
 //a fix-it to add 'typename A::type'
 template
-A::g() { } // expected-error{{requires a type specifier}}
+A::g() { } // expected-error{{expected unqualified-id}}
+// expected-warning@-1{{implicit 'typename' is a C++2a extension}}
Index: clang/test/SemaCXX/MicrosoftSuper.cpp
===
--- clang/test/Se

[PATCH] D53847: [C++2a] P0634r3: Down with typename!

[Nicolas Lesser via Phabricator via cfe-commits]

Rakete added inline comments.



Comment at: clang/lib/Parse/ParseDecl.cpp:4321
+ isCXXDeclarationSpecifier(ITC_Never, TPResult::True) !=
+ TPResult::True) ||
+(!getLangOpts().CPlusPlus && !isDeclarationSpecifier(ITC_Yes))) {

rsmith wrote:
> It seems like a wording oversight that we don't assume `typename` in an 
> //enum-base//. Probably would be good to raise this on the core reflector.
Do you know why this isn't allowed in `operator` ids?



Comment at: clang/lib/Parse/ParseDecl.cpp:5100-5101
   bool IsConstructor = false;
-  if (isDeclarationSpecifier())
+  if (isDeclarationSpecifier(ITC_Never))
 IsConstructor = true;
   else if (Tok.is(tok::identifier) ||

rsmith wrote:
> Oh, this could be a problem.
> 
> If this *is* a constructor declaration, then this is implicit typename 
> context: this is either a "//parameter-declaration// in a 
> //member-declaration//" ([temp.res]/5.2.3) or a "//parameter-declaration// in 
> a //declarator// of a function or function template declaration whose 
> //declarator-id// is qualified". But if it's *not* a constructor declaration, 
> then this is either the //declarator-id// of a declaration or the 
> //nested-name-specifier// of a pointer-to-member declarator:
> 
> ```
> template
> struct C {
>   C(T::type); // implicit typename context
>   friend C (T::fn)(); // not implicit typename context, declarator-id of 
> friend declaration
>   C(T::type::*x)[3]; // not implicit typename context, pointer-to-member type
> };
> ```
> 
> I think we need to use `ITC_Yes` here, in order to correctly disambiguate the 
> first example above. Please add tests for the other two cases to make sure 
> this doesn't break them -- but I'm worried this **will** break the second 
> case, because it will incorrectly annotate `T::fn` as a type.
Yeah it does the break the second. Would just checking whether a `friend` is 
there be good enough? clang doesn't seem to actively propose to add  a friend 
if there's one missing, so if we add a `IsFriend` parameter and then check if 
it is set than always return `false`, it should work. Is that acceptable? It 
would break the nice error message you get if you write `friend C(int);`, but 
if we restrict it when `isDeclarationSpecifier` return false (with `Never`) 
would that be better (that's what I'm doing right now)?

Another solution would be to tentatively parse the whole thing, but that can be 
pretty expensive I believe.



Comment at: clang/lib/Sema/SemaTemplate.cpp:3377-3379
   // FIXME: This is not quite correct recovery as we don't transform SS
   // into the corresponding dependent form (and we don't diagnose missing
   // 'template' keywords within SS as a result).

rsmith wrote:
> This FIXME is concerning. Is this a problem with this patch? (Is the FIXME 
> wrong now?)
Yes you're right, I believe it not longer applies due to the change in 
ParseDecl.cpp in ParseDeclarationSpecifiers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D53847



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

[PATCH] D53847: [C++2a] P0634r3: Down with typename!

[Nicolas Lesser via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaTemplate.cpp:3377-3379
   // FIXME: This is not quite correct recovery as we don't transform SS
   // into the corresponding dependent form (and we don't diagnose missing
   // 'template' keywords within SS as a result).

Rakete wrote:
> rsmith wrote:
> > This FIXME is concerning. Is this a problem with this patch? (Is the FIXME 
> > wrong now?)
> Yes you're right, I believe it not longer applies due to the change in 
> ParseDecl.cpp in ParseDeclarationSpecifiers.
Although I'm not that sure I fully understand what that comments alludes to.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D53847



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

[PATCH] D61509: [OpenMP] Set pragma start loc to `#pragma` loc

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.

1. Is there a diagnostic that would point to the `omp` token? As much as I like 
complete info (such as SourceLoc of semicolons), I cannot think of a use case 
for it.
2. I would like to see all of them all adjusted. There is an immediate 
improvement in that improves the diagnostic output.


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

https://reviews.llvm.org/D61509



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

[PATCH] D61509: [OpenMP] Set pragma start loc to `#pragma` loc

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D61509#1512293 , @Meinersbur wrote:

> 1. Is there a diagnostic that would point to the `omp` token? As much as I 
> like complete info (such as SourceLoc of semicolons), I cannot think of a use 
> case for it.
> 2. I would like to see all of them all adjusted. There is an immediate 
> improvement in that improves the diagnostic output.




1. Nope, there is no `omp` specific diagnostics


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

https://reviews.llvm.org/D61509



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

[PATCH] D62225: [clang][NewPM] Fixing -O0 tests that are broken under new PM

[Leonard Chan via Phabricator via cfe-commits]

leonardchan marked 4 inline comments as done.
leonardchan added inline comments.



Comment at: clang/lib/CodeGen/BackendUtil.cpp:1104-1105
   // which is just that always inlining occurs.
-  MPM.addPass(AlwaysInlinerPass());
+  // We always pass false here since according to the legacy PM logic for
+  // enabling lifetime intrinsics, we should not be compiling with O0.
+  MPM.addPass(AlwaysInlinerPass(/*InsertLifetimeIntrinsics=*/false));

serge-sans-paille wrote:
> echristo wrote:
> > Can you elaborate more here? We do turn on the always inliner at O0 which 
> > makes this comment a bit confusing.
> I guess he means 
> 
> We always pass false here since according to the legacy PM logic for 
> enabling lifetime intrinsics, they are not required with O0
> 
Yup, my bad. This is what I meant with this comment. Always inlining is used. 
It's the lifetime intrinsics that aren't always used.



Comment at: clang/test/CodeGen/aarch64-neon-fma.c:235
 // CHECK: attributes #1 ={{.*}}"min-legal-vector-width"="128"
+// CHECK: attributes [[NOUNWIND_ATTR]] = { nounwind }

echristo wrote:
> Do we really need to check for it or does the autogeneration of testcases do 
> some of this?
I do not know if this is actually an autogenerated test, but if so then 
probably not. Is there a way to check this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62225



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

[PATCH] D62192: [clang-tidy]: Add cert-oop54-cpp alias for bugprone-unhandled-self-assignment

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas updated this revision to Diff 200775.
ztamas added a comment.
Herald added a subscriber: mgorny.

Link burprone module to cert module


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62192

Files:
  clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.h
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/clang-tidy/cert/CMakeLists.txt
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-oop54-cpp.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment-warn-only-if-this-has-suspicious-field.cpp
  clang-tools-extra/test/clang-tidy/cert-oop54-cpp.cpp

Index: clang-tools-extra/test/clang-tidy/cert-oop54-cpp.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/cert-oop54-cpp.cpp
@@ -0,0 +1,16 @@
+// RUN: %check_clang_tidy %s cert-oop54-cpp %t
+
+// Test whether bugprone-unhandled-self-assignment.WarnOnlyIfThisHasSuspiciousField option is set correctly.
+class TrivialFields {
+public:
+  TrivialFields &operator=(const TrivialFields &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:18: warning: operator=() does not handle self-assignment properly [cert-oop54-cpp]
+return *this;
+  }
+
+private:
+  int m;
+  float f;
+  double d;
+  bool b;
+};
Index: clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment-warn-only-if-this-has-suspicious-field.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment-warn-only-if-this-has-suspicious-field.cpp
@@ -0,0 +1,41 @@
+// RUN: %check_clang_tidy %s bugprone-unhandled-self-assignment %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: bugprone-unhandled-self-assignment.WarnOnlyIfThisHasSuspiciousField, \
+// RUN:   value: 0}]}"
+
+// Classes with pointer field are still caught.
+class PtrField {
+public:
+  PtrField &operator=(const PtrField &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:13: warning: operator=() does not handle self-assignment properly [bugprone-unhandled-self-assignment]
+return *this;
+  }
+
+private:
+  int *p;
+};
+
+// With the option, check catches classes with trivial fields.
+class TrivialFields {
+public:
+  TrivialFields &operator=(const TrivialFields &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:18: warning: operator=() does not handle self-assignment properly [bugprone-unhandled-self-assignment]
+return *this;
+  }
+
+private:
+  int m;
+  float f;
+  double d;
+  bool b;
+};
+
+// The check warns also when there is no field at all.
+// In this case, user-defined copy assignment operator is useless anyway.
+class ClassWithoutFields {
+public:
+  ClassWithoutFields &operator=(const ClassWithoutFields &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:23: warning: operator=() does not handle self-assignment properly [bugprone-unhandled-self-assignment]
+return *this;
+  }
+};
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -98,6 +98,7 @@
cert-msc50-cpp
cert-msc51-cpp
cert-oop11-cpp (redirects to performance-move-constructor-init) 
+   cert-oop54-cpp (redirects to bugprone-unhandled-self-assignment) 
cppcoreguidelines-avoid-c-arrays (redirects to modernize-avoid-c-arrays) 
cppcoreguidelines-avoid-goto
cppcoreguidelines-avoid-magic-numbers (redirects to readability-magic-numbers) 
Index: clang-tools-extra/docs/clang-tidy/checks/cert-oop54-cpp.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cert-oop54-cpp.rst
@@ -0,0 +1,10 @@
+.. title:: clang-tidy - cert-oop54-cpp
+.. meta::
+   :http-equiv=refresh: 5;URL=bugprone-unhandled-self-assignment.html
+
+cert-oop54-cpp
+==
+
+The cert-oop54-cpp check is an alias, please see
+`bugprone-unhandled-self-assignment `_
+for more information.
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst
@@ -3,11 +3,14 @@
 bugprone-unhandled-self-assignment
 ==
 
+`cert-oop54-cpp` redirects here as an alias for this check. For the CERT alias,
+the `WarnOnlyIfThisHasSuspiciousField` option is set to `0`.
+
 Finds user

[PATCH] D61509: [OpenMP] Set pragma start loc to `#pragma` loc

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D61509#1512294 , @ABataev wrote:

> In D61509#1512293 , @Meinersbur 
> wrote:
>
> > 1. Is there a diagnostic that would point to the `omp` token? As much as I 
> > like complete info (such as SourceLoc of semicolons), I cannot think of a 
> > use case for it.
> > 2. I would like to see all of them all adjusted. There is an immediate 
> > improvement in that improves the diagnostic output.
>
>
>
>
> 1. Nope, there is no `omp` specific diagnostics




1. Based on the test suite changes this patch makes, I agree for OpenMP.  I 
haven't explored other pragmas.

2. I too think it likely makes sense to adjust them all eventually.  But do 
people think it's important to write patches adjusting all pragmas before 
pushing the adjustment for any of them?


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

https://reviews.llvm.org/D61509



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

[PATCH] D37813: clang-format: better handle namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

The patch goal is indeed to indent the content of namespace-macro blocks like 
the content of any 'regular' namespace. So it should look like the content of a 
namespace, possibly depending on the choose style options. To sumarize, here is 
a detailed summary of the observable differences between macros vs normal 
namespace.

**Without this patch**, clang-format does not understand the macro call 
actually defines a namespace, thus:

- the content of the namespace-macro block is indented, even when 
`Style.NamespaceIndentation = FormatStyle::NI_None`

  TESTSUITE(A) {
int i;  // <--- should not be indented
  }
  namespace B {
  int j;
  }



- similarly for nested namespaces, when  `Style.NamespaceIndentation = 
FormatStyle::NI_Inner` :

  TESTSUITE(A) {
  TESTSUITE(B) {
  int i;// <--- should be indented 2-chars only
  }
  }
  namespace C {
  namespace D {
int j;
  }
  }

- There is no automatic fix of namespace end comment when 
`Style.FixNamespaceComments = true`

  TESTSUITE(A) {
int i;
  } // <--- should have a namespace end comment
  namespace B {
int j;
  } // namespace B

- Multiple nested namespaces are not compacted when `Style.CompactNamespaces = 
true`

  TESTSUITE(A) {
  TESTSUITE(B) { //<--- should be merged with previous line
  int i;
  }
  } // <--- should be merged with previous line (and 
have a "combined" namespace end comment)
  namespace A { namespace B {
  int j;
  } // namespace A::B



---

**This patch fixes all these points**, which hopefully leads to the exact same 
formatting when using the namespace keyword or the namespace-macros:

  // Style.NamespaceIndentation = FormatStyle::NI_None
  TESTSUITE(A) {
  int i;
  }
  namespace B {
  int j;
  }
  
  // Style.NamespaceIndentation = FormatStyle::NI_Inner
  TESTSUITE(A) {
  TESTSUITE(B) {
int i;
  }
  }
  namespace C {
  namespace D {
int j;
  }
  }
  
  // Style.FixNamespaceComments = true
  TESTSUITE(A) {
int i;
  } // TESTSUITE(A)
  namespace B {
int j;
  } // namespace B
  
  // Style.CompactNamespaces = true
  TESTSUITE(A) { TESTSUITE(B) {
int i;
  }} // TESTSUITE(A::B)
  namespace A { namespace B {
int j;
  }} // namespace A::B

I did not see any issue in my testing or reviewing the code, but if you see a 
place in the tests where it does not match this, please point it out !


Repository:
  rC Clang

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

https://reviews.llvm.org/D37813



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

[PATCH] D61509: [OpenMP] Set pragma start loc to `#pragma` loc

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.

In D61509#1512311 , @jdenny wrote:

> 2. I too think it likely makes sense to adjust them all eventually.  But do 
> people think it's important to write patches adjusting all pragmas before 
> pushing the adjustment for any of them?


I am not sure I understand. Do you mean whether you need all patches for each 
pragma to be accepted before you can commit the first? This is not that case. 
IMHO you can even put all of it into a single patch as it should be very 
straightforward. The most work is adapting the tests.


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

https://reviews.llvm.org/D61509



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

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.
Herald added a project: clang.

ping ?


Repository:
  rC Clang

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

https://reviews.llvm.org/D50078



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

[PATCH] D61509: [OpenMP] Set pragma start loc to `#pragma` loc

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D61509#1512321 , @Meinersbur wrote:

> In D61509#1512311 , @jdenny wrote:
>
> > 2. I too think it likely makes sense to adjust them all eventually.  But do 
> > people think it's important to write patches adjusting all pragmas before 
> > pushing the adjustment for any of them?
>
>
> I am not sure I understand. Do you mean whether you need all patches for each 
> pragma to be accepted before you can commit the first? This is not that case.


@lebedev.ri expressed concern that it might not be acceptable to migrate all 
pragmas in the same way.  That would suggest we must handle them all before 
committing any.

> IMHO you can even put all of it into a single patch as it should be very 
> straightforward. The most work is adapting the tests.

I would think different people would want to review different pragmas, so 
separate patches would be better, but I'm happy to be corrected as I haven't 
explored who owns what here.


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

https://reviews.llvm.org/D61509



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

[PATCH] D59673: [Clang] Harmonize Split DWARF options with llc

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

Ping. I'm sorry that the change turned out so big, but note that it doesn't 
change the behavior of any non-cc1 flags.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59673



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

[PATCH] D59402: Fix-it hints for -Wmissing-{prototypes,variable-declarations}

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a reviewer: ed.
aaronpuchert added a comment.

Another ping. I've tried to add the original authors as reviewers, but both 
warnings are several years old and you might not remember.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59402



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

[PATCH] D52395: Thread safety analysis: Require exclusive lock for passing by non-const reference

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert planned changes to this revision.
aaronpuchert added a comment.
Herald added a project: clang.

I don't actually want to change anything, but remove this from your review 
queues until I have an idea how often the warning fires and how many false 
positives we have.


Repository:
  rC Clang

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

https://reviews.llvm.org/D52395



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

[PATCH] D60883: [OpenMP] Avoid emitting maps for target link variables when unified memory is used

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea updated this revision to Diff 200782.
gtbercea added a comment.

- Fix function call.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60883

Files:
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGOpenMPRuntime.h


Index: lib/CodeGen/CGOpenMPRuntime.h
===
--- lib/CodeGen/CGOpenMPRuntime.h
+++ lib/CodeGen/CGOpenMPRuntime.h
@@ -1623,6 +1623,9 @@
   /// the predefined allocator and translates it into the corresponding address
   /// space.
   virtual bool hasAllocateAttributeForGlobalVar(const VarDecl *VD, LangAS &AS);
+
+  /// Return whether the unified_shared_memory has been specified.
+  virtual bool hasRequiresUnifiedSharedMemory();
 };
 
 /// Class supports emissionof SIMD-only code.
Index: lib/CodeGen/CGOpenMPRuntime.cpp
===
--- lib/CodeGen/CGOpenMPRuntime.cpp
+++ lib/CodeGen/CGOpenMPRuntime.cpp
@@ -8255,6 +8255,10 @@
 MapValuesArrayTy &Pointers,
 MapValuesArrayTy &Sizes,
 MapFlagsArrayTy &Types) const {
+// If using unified memory, no need to do the mappings.
+if (CGF.CGM.getOpenMPRuntime().hasRequiresUnifiedSharedMemory())
+  return;
+
 // Map other list items in the map clause which are not captured variables
 // but "declare target link" global variables.
 for (const auto *C : this->CurDir.getClausesOfKind()) {
@@ -9251,6 +9255,10 @@
   return false;
 }
 
+bool CGOpenMPRuntime::hasRequiresUnifiedSharedMemory() {
+  return HasRequiresUnifiedSharedMemory;
+}
+
 CGOpenMPRuntime::DisableAutoDeclareTargetRAII::DisableAutoDeclareTargetRAII(
 CodeGenModule &CGM)
 : CGM(CGM) {


Index: lib/CodeGen/CGOpenMPRuntime.h
===
--- lib/CodeGen/CGOpenMPRuntime.h
+++ lib/CodeGen/CGOpenMPRuntime.h
@@ -1623,6 +1623,9 @@
   /// the predefined allocator and translates it into the corresponding address
   /// space.
   virtual bool hasAllocateAttributeForGlobalVar(const VarDecl *VD, LangAS &AS);
+
+  /// Return whether the unified_shared_memory has been specified.
+  virtual bool hasRequiresUnifiedSharedMemory();
 };
 
 /// Class supports emissionof SIMD-only code.
Index: lib/CodeGen/CGOpenMPRuntime.cpp
===
--- lib/CodeGen/CGOpenMPRuntime.cpp
+++ lib/CodeGen/CGOpenMPRuntime.cpp
@@ -8255,6 +8255,10 @@
 MapValuesArrayTy &Pointers,
 MapValuesArrayTy &Sizes,
 MapFlagsArrayTy &Types) const {
+// If using unified memory, no need to do the mappings.
+if (CGF.CGM.getOpenMPRuntime().hasRequiresUnifiedSharedMemory())
+  return;
+
 // Map other list items in the map clause which are not captured variables
 // but "declare target link" global variables.
 for (const auto *C : this->CurDir.getClausesOfKind()) {
@@ -9251,6 +9255,10 @@
   return false;
 }
 
+bool CGOpenMPRuntime::hasRequiresUnifiedSharedMemory() {
+  return HasRequiresUnifiedSharedMemory;
+}
+
 CGOpenMPRuntime::DisableAutoDeclareTargetRAII::DisableAutoDeclareTargetRAII(
 CodeGenModule &CGM)
 : CGM(CGM) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60455: [SYCL] Implement SYCL device code outlining

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1017
+  let LangOpts = [SYCL];
+  let Documentation = [Undocumented];
+}

Ok, I thought the earlier request was not to add undocumented attributes with 
the spelling?

Also did `__kernel` attribute not work at the end?

I can't quite get where the current disconnect comes from but I find it 
extremely unhelpful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

[PATCH] D60883: [OpenMP] Avoid emitting maps for target link variables when unified memory is used

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7956
+// If using unified memory, no need to do the mappings.
+if (CGF.CGM.getOpenMPRuntime().hasUnifiedAddressingSupport())
+  return;

ABataev wrote:
> Hmm, what about regular declare target declarations? Also, this is not 
> enough. You also should not capture such variables in the captured regions.
For now, only variables under the link clause are considered for unified memory 
usage.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60883



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

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:48
+  return llvm::None;
+} else {
+  DirectoryWatcher::Event Front = Events.front();

nit: get rid of the else



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:156
+
+for (char *p = Buf; p < Buf + NumRead;) {
+  if (p + sizeof(struct inotify_event) > Buf + NumRead) {

naming, it should be capital `P`



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:211
+  // ... inotify-originated events processing ever after.
+  while (true) {
+bool GotEvent = false;

`while (!Stop)` ?



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:211
+  // ... inotify-originated events processing ever after.
+  while (true) {
+bool GotEvent = false;

kadircet wrote:
> `while (!Stop)` ?
why the loop always tries to empty the queue? what would be broken if we simply 
stopped if `Stop` was set?

Also if that is important current implementation doesn't seem to be achiving 
that, since some events can be pushed to the queue after while loop exits.



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:223
+break;
+  std::this_thread::sleep_for(std::chrono::milliseconds(1));
+}

maybe add a condition variable for queue state and waint on it?



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:244
+  ReceivingThread([this]() { EventReceivingLoop(); }) {
+  while (WaitForInitialSync && !Stop && !FinishedInitScan) {
+std::this_thread::sleep_for(std::chrono::milliseconds(1));

why not use two condition variables for stop and finishedinitscan?


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

https://reviews.llvm.org/D58418



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

[PATCH] D58375: [Clang] Disable new PM for tests that use optimization level -O1, -O2 and -O3

[Petr Hosek via Phabricator via cfe-commits]

phosek updated this revision to Diff 200784.
phosek retitled this revision from "[Clang][NewPM] Disable tests that are 
broken under new PM" to "[Clang] Disable new PM for tests that use optimization 
level -O1, -O2 and -O3".
phosek edited the summary of this revision.

Repository:
  rC Clang

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

https://reviews.llvm.org/D58375

Files:
  clang/test/CodeGen/callback_annotated.c
  clang/test/CodeGen/cfi-icall-cross-dso.c
  clang/test/CodeGen/complex-math.c
  clang/test/CodeGen/dllimport.c
  clang/test/CodeGen/inline2.c
  clang/test/CodeGen/lifetime.c
  clang/test/CodeGen/sanitize-address-field-padding.cpp
  clang/test/CodeGen/tbaa-for-vptr.cpp
  clang/test/CodeGenCXX/atomicinit.cpp
  clang/test/CodeGenCXX/cfi-speculative-vtable.cpp
  clang/test/CodeGenCXX/debug-info-class-optzns.cpp
  clang/test/CodeGenCXX/dllimport-members.cpp
  clang/test/CodeGenCXX/dllimport.cpp
  clang/test/CodeGenCXX/dso-local-executable.cpp
  clang/test/CodeGenCXX/init-invariant.cpp
  clang/test/CodeGenCXX/merge-functions.cpp
  clang/test/CodeGenCXX/nrvo.cpp
  clang/test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
  clang/test/CodeGenCXX/visibility-hidden-extern-templates.cpp
  clang/test/CodeGenObjCXX/nrvo.mm
  clang/test/Driver/asan.c
  clang/test/Driver/msan.c
  clang/test/Driver/tsan.c

Index: clang/test/Driver/tsan.c
===
--- clang/test/Driver/tsan.c
+++ clang/test/Driver/tsan.c
@@ -1,9 +1,9 @@
 // REQUIRES: x86-registered-target
 
 // RUN: %clang -target x86_64-unknown-linux -fsanitize=thread %s -S -emit-llvm -o - | FileCheck %s
-// RUN: %clang -O1 -target x86_64-unknown-linux -fsanitize=thread %s -S -emit-llvm -o - | FileCheck %s
-// RUN: %clang -O2 -target x86_64-unknown-linux -fsanitize=thread %s -S -emit-llvm -o - | FileCheck %s
-// RUN: %clang -O3 -target x86_64-unknown-linux -fsanitize=thread %s -S -emit-llvm -o - | FileCheck %s
+// RUN: %clang -O1 -fno-experimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=thread %s -S -emit-llvm -o - | FileCheck %s
+// RUN: %clang -O2 -fno-experimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=thread %s -S -emit-llvm -o - | FileCheck %s
+// RUN: %clang -O3 -fno-experimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=thread %s -S -emit-llvm -o - | FileCheck %s
 // RUN: %clang -target x86_64-unknown-linux -fsanitize=thread  %s -S -emit-llvm -o - | FileCheck %s
 // Verify that -fsanitize=thread invokes tsan instrumentation.
 
Index: clang/test/Driver/msan.c
===
--- clang/test/Driver/msan.c
+++ clang/test/Driver/msan.c
@@ -1,14 +1,14 @@
 // REQUIRES: x86-registered-target
 
 // RUN: %clang -target x86_64-unknown-linux -fsanitize=memory %s -S -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-MSAN
-// RUN: %clang -O1 -target x86_64-unknown-linux -fsanitize=memory %s -S -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-MSAN
-// RUN: %clang -O2 -target x86_64-unknown-linux -fsanitize=memory %s -S -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-MSAN
-// RUN: %clang -O3 -target x86_64-unknown-linux -fsanitize=memory %s -S -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-MSAN
+// RUN: %clang -O1 -fno-experimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=memory %s -S -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-MSAN
+// RUN: %clang -O2 -fno-experimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=memory %s -S -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-MSAN
+// RUN: %clang -O3 -fno-experimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=memory %s -S -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-MSAN
 
 // RUN: %clang -target x86_64-unknown-linux -fsanitize=kernel-memory %s -S -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-KMSAN
-// RUN: %clang -O1 -target x86_64-unknown-linux -fsanitize=kernel-memory %s -S -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-KMSAN
-// RUN: %clang -O2 -target x86_64-unknown-linux -fsanitize=kernel-memory %s -S -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-KMSAN
-// RUN: %clang -O3 -target x86_64-unknown-linux -fsanitize=kernel-memory %s -S -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-KMSAN
+// RUN: %clang -O1 -fno-experimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=kernel-memory %s -S -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-KMSAN
+// RUN: %clang -O2 -fno-experimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=kernel-memory %s -S -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-KMSAN
+// RUN: %clang -O3 -fno-experimental-new-pass-manager -target x86_64-unknown-linux -fsanitize=kernel-memory %s -S -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-KMSAN
 
 // RUN: %clang -target mips64-linux-gnu -fsanitize=memory %s -S -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-MSAN
 //