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

[Christudasan Devadasan via Phabricator via cfe-commits]

cdevadas updated this revision to Diff 203975.
cdevadas added a comment.

simplified the check in the test case.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62244

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGenCUDA/amdgpu-hip-implicit-kernarg.cu


Index: test/CodeGenCUDA/amdgpu-hip-implicit-kernarg.cu
===
--- /dev/null
+++ test/CodeGenCUDA/amdgpu-hip-implicit-kernarg.cu
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -emit-llvm -x hip -o - %s | 
FileCheck %s
+#include "Inputs/cuda.h"
+
+__global__ void hip_kernel_temp() {
+}
+
+// CHECK: attributes #0 = { noinline nounwind optnone 
"amdgpu-implicitarg-num-bytes"="48"
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/CodeGenCUDA/amdgpu-hip-implicit-kernarg.cu
===
--- /dev/null
+++ test/CodeGenCUDA/amdgpu-hip-implicit-kernarg.cu
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -emit-llvm -x hip -o - %s | FileCheck %s
+#include "Inputs/cuda.h"
+
+__global__ void hip_kernel_temp() {
+}
+
+// CHECK: attributes #0 = { noinline nounwind optnone "amdgpu-implicitarg-num-bytes"="48"
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

[clang-tools-extra] r363034 - [clangd] Fix gcc warning by removing extra "; "

[Mikael Holmen via cfe-commits]

Author: uabelho
Date: Mon Jun 10 23:02:01 2019
New Revision: 363034

URL: http://llvm.org/viewvc/llvm-project?rev=363034&view=rev
Log:
[clangd] Fix gcc warning by removing extra ";"

Modified:
clang-tools-extra/trunk/clangd/Format.cpp

Modified: clang-tools-extra/trunk/clangd/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Format.cpp?rev=363034&r1=363033&r2=363034&view=diff
==
--- clang-tools-extra/trunk/clangd/Format.cpp (original)
+++ clang-tools-extra/trunk/clangd/Format.cpp Mon Jun 10 23:02:01 2019
@@ -91,7 +91,7 @@ tooling::Replacement replacement(llvm::S
   // The filename is required but ignored.
   return tooling::Replacement(Filename, From.data() - Code.data(),
   From.size(), To);
-};
+}
 
 // High-level representation of incremental formatting changes.
 // The changes are made in two steps.


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

[PATCH] D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403)

[Mateusz Maćkowski via Phabricator via cfe-commits]

m4tx added a comment.

@JonasToth thanks for asking! Yes, since I do not have commit access, I need 
someone to do the commit for me.


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

https://reviews.llvm.org/D55793



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

[PATCH] D61256: [clang-format][docs] Fix the Google C++ and Chromium style guide URLs

[Mateusz Maćkowski via Phabricator via cfe-commits]

m4tx added a comment.

@MyDeveloperDay thanks for accepting the revision! Since I do not have commit 
access, could you please make the commit for me?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61256



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

[PATCH] D62962: Clang implementation of sizeless types

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm marked 4 inline comments as done.
rsandifo-arm added a comment.

Thanks for the review.

In D62962#1534528 , @jfb wrote:

> How do these types mangle?


At the moment, the mangling is only defined for the Itanium ABI, which uses 
`str(len(name)) + name` for the built-in type name (e.g. `__SVInt8_t`.  This is 
implemented in https://reviews.llvm.org/D62960.




Comment at: test/Sema/sizeless-1.c:60
+  (void)__atomic_is_lock_free(1, &local_int8);
+  (void)__atomic_always_lock_free(1, &local_int8);
+

jfb wrote:
> What do you expect these to return?
Fixed to make this clearer.  The types provide no compile-time alignment 
guarantees, so constant evaluation needs to make the most pessimistic 
assumption.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62962



___
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 203785.
Fznamznon added a comment.

Applied comments from @Anastasia

- Added link to documentation for `sycl_device` attribute
- Removed redundant comment from test

@Anastasia, do you have additional comments?


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/Basic/AttrDocs.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 -std=c++11 -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,13 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl-is-device -verify %s
+// RUN: %clang_cc1 -std=c++11 -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
@@ -125,6 +125,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 SY

[PATCH] D63018: [X86] Attempt to make the Intel core CPU inheritance a little more readable and maintainable

[Simon Pilgrim via Phabricator via cfe-commits]

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

LGTM - the goto is unfortunate but more maintainable and less confusing then 
the current state of things




Comment at: clang/lib/Basic/Targets/X86.cpp:157
+setFeatureEnabledImpl(Features, "pku", true);
+goto SkylakeCommon;
+

Worth adding a comment explaining the AVX512 ISA divergence?


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

https://reviews.llvm.org/D63018



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

r362924 - Revert "Revert "[CodeComplete] Improve overload handling for C++ qualified and ref-qualified methods.""

[Sam McCall via cfe-commits]

Author: sammccall
Date: Mon Jun 10 02:52:09 2019
New Revision: 362924

URL: http://llvm.org/viewvc/llvm-project?rev=362924&view=rev
Log:
Revert "Revert "[CodeComplete] Improve overload handling for C++ qualified and 
ref-qualified methods.""

This reverts commit r362830, and relands r362785 with the leak fixed.

Modified:
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/test/CodeCompletion/member-access.cpp

Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=362924&r1=362923&r2=362924&view=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Mon Jun 10 02:52:09 2019
@@ -16,7 +16,9 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/QualTypeNames.h"
+#include "clang/AST/Type.h"
 #include "clang/Basic/CharInfo.h"
+#include "clang/Basic/Specifiers.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/Preprocessor.h"
@@ -105,7 +107,7 @@ private:
   DeclIndexPair(ND, Index));
 }
 
-void Destroy() {
+~ShadowMapEntry() {
   if (DeclIndexPairVector *Vec =
   DeclOrVector.dyn_cast()) {
 delete Vec;
@@ -152,9 +154,16 @@ private:
   /// different levels of, e.g., the inheritance hierarchy.
   std::list ShadowMaps;
 
+  /// Overloaded C++ member functions found by SemaLookup.
+  /// Used to determine when one overload is dominated by another.
+  llvm::DenseMap, ShadowMapEntry>
+  OverloadMap;
+
   /// If we're potentially referring to a C++ member function, the set
   /// of qualifiers applied to the object type.
   Qualifiers ObjectTypeQualifiers;
+  /// The kind of the object expression, for rvalue/lvalue overloads.
+  ExprValueKind ObjectKind;
 
   /// Whether the \p ObjectTypeQualifiers field is active.
   bool HasObjectTypeQualifiers;
@@ -230,8 +239,9 @@ public:
   /// out member functions that aren't available (because there will be a
   /// cv-qualifier mismatch) or prefer functions with an exact qualifier
   /// match.
-  void setObjectTypeQualifiers(Qualifiers Quals) {
+  void setObjectTypeQualifiers(Qualifiers Quals, ExprValueKind Kind) {
 ObjectTypeQualifiers = Quals;
+ObjectKind = Kind;
 HasObjectTypeQualifiers = true;
   }
 
@@ -1157,6 +1167,53 @@ static void setInBaseClass(ResultBuilder
   R.InBaseClass = true;
 }
 
+enum class OverloadCompare { BothViable, Dominates, Dominated };
+// Will Candidate ever be called on the object, when overloaded with Incumbent?
+// Returns Dominates if Candidate is always called, Dominated if Incumbent is
+// always called, BothViable if either may be called dependending on arguments.
+// Precondition: must actually be overloads!
+static OverloadCompare compareOverloads(const CXXMethodDecl &Candidate,
+const CXXMethodDecl &Incumbent,
+const Qualifiers &ObjectQuals,
+ExprValueKind ObjectKind) {
+  if (Candidate.isVariadic() != Incumbent.isVariadic() ||
+  Candidate.getNumParams() != Incumbent.getNumParams() ||
+  Candidate.getMinRequiredArguments() !=
+  Incumbent.getMinRequiredArguments())
+return OverloadCompare::BothViable;
+  for (unsigned I = 0, E = Candidate.getNumParams(); I != E; ++I)
+if (Candidate.parameters()[I]->getType().getCanonicalType() !=
+Incumbent.parameters()[I]->getType().getCanonicalType())
+  return OverloadCompare::BothViable;
+  if (!llvm::empty(Candidate.specific_attrs()) ||
+  !llvm::empty(Incumbent.specific_attrs()))
+return OverloadCompare::BothViable;
+  // At this point, we know calls can't pick one or the other based on
+  // arguments, so one of the two must win. (Or both fail, handled elsewhere).
+  RefQualifierKind CandidateRef = Candidate.getRefQualifier();
+  RefQualifierKind IncumbentRef = Incumbent.getRefQualifier();
+  if (CandidateRef != IncumbentRef) {
+// If the object kind is LValue/RValue, there's one acceptable 
ref-qualifier
+// and it can't be mixed with ref-unqualified overloads (in valid code).
+
+// For xvalue objects, we prefer the rvalue overload even if we have to
+// add qualifiers (which is rare, because const&& is rare).
+if (ObjectKind == clang::VK_XValue)
+  return CandidateRef == RQ_RValue ? OverloadCompare::Dominates
+   : OverloadCompare::Dominated;
+  }
+  // Now the ref qualifiers are the same (or we're in some invalid state).
+  // So make some decision based on the qualifiers.
+  Qualifiers CandidateQual = Candidate.getMethodQualifiers();
+  Qualifiers IncumbentQual = Incumbent.getMethodQualifiers();
+  bool CandidateSuperset = CandidateQual.compatiblyIncludes(IncumbentQual);
+  bool IncumbentSuperset = IncumbentQual.compatiblyInclud

Re: r362785 - [CodeComplete] Improve overload handling for C++ qualified and ref-qualified methods.

[Sam McCall via cfe-commits]

Thanks! Re-landed in r362924.

On Fri, Jun 7, 2019 at 9:15 PM Vlad Tsyrklevich via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> This change caused LSan failures and has been reverted in r362830:
> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/32809
>
> On Fri, Jun 7, 2019 at 2:42 AM Sam McCall via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: sammccall
>> Date: Fri Jun  7 02:45:17 2019
>> New Revision: 362785
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=362785&view=rev
>> Log:
>> [CodeComplete] Improve overload handling for C++ qualified and
>> ref-qualified methods.
>>
>> Summary:
>> - when a method is not available because of the target value kind (e.g.
>> an &&
>>   method on a Foo& variable), then don't offer it.
>> - when a method is effectively shadowed by another method from the same
>> class
>>   with a) an identical argument list and b) superior qualifiers, then
>> don't
>>   offer it.
>>
>> Reviewers: ilya-biryukov
>>
>> Subscribers: cfe-commits
>>
>> Tags: #clang
>>
>> Differential Revision: https://reviews.llvm.org/D62582
>>
>> Modified:
>> cfe/trunk/lib/Sema/SemaCodeComplete.cpp
>> cfe/trunk/test/CodeCompletion/member-access.cpp
>>
>> Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=362785&r1=362784&r2=362785&view=diff
>>
>> ==
>> --- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Fri Jun  7 02:45:17 2019
>> @@ -16,7 +16,9 @@
>>  #include "clang/AST/ExprCXX.h"
>>  #include "clang/AST/ExprObjC.h"
>>  #include "clang/AST/QualTypeNames.h"
>> +#include "clang/AST/Type.h"
>>  #include "clang/Basic/CharInfo.h"
>> +#include "clang/Basic/Specifiers.h"
>>  #include "clang/Lex/HeaderSearch.h"
>>  #include "clang/Lex/MacroInfo.h"
>>  #include "clang/Lex/Preprocessor.h"
>> @@ -152,9 +154,16 @@ private:
>>/// different levels of, e.g., the inheritance hierarchy.
>>std::list ShadowMaps;
>>
>> +  /// Overloaded C++ member functions found by SemaLookup.
>> +  /// Used to determine when one overload is dominated by another.
>> +  llvm::DenseMap,
>> ShadowMapEntry>
>> +  OverloadMap;
>> +
>>/// If we're potentially referring to a C++ member function, the set
>>/// of qualifiers applied to the object type.
>>Qualifiers ObjectTypeQualifiers;
>> +  /// The kind of the object expression, for rvalue/lvalue overloads.
>> +  ExprValueKind ObjectKind;
>>
>>/// Whether the \p ObjectTypeQualifiers field is active.
>>bool HasObjectTypeQualifiers;
>> @@ -230,8 +239,9 @@ public:
>>/// out member functions that aren't available (because there will be a
>>/// cv-qualifier mismatch) or prefer functions with an exact qualifier
>>/// match.
>> -  void setObjectTypeQualifiers(Qualifiers Quals) {
>> +  void setObjectTypeQualifiers(Qualifiers Quals, ExprValueKind Kind) {
>>  ObjectTypeQualifiers = Quals;
>> +ObjectKind = Kind;
>>  HasObjectTypeQualifiers = true;
>>}
>>
>> @@ -1157,6 +1167,53 @@ static void setInBaseClass(ResultBuilder
>>R.InBaseClass = true;
>>  }
>>
>> +enum class OverloadCompare { BothViable, Dominates, Dominated };
>> +// Will Candidate ever be called on the object, when overloaded with
>> Incumbent?
>> +// Returns Dominates if Candidate is always called, Dominated if
>> Incumbent is
>> +// always called, BothViable if either may be called dependending on
>> arguments.
>> +// Precondition: must actually be overloads!
>> +static OverloadCompare compareOverloads(const CXXMethodDecl &Candidate,
>> +const CXXMethodDecl &Incumbent,
>> +const Qualifiers &ObjectQuals,
>> +ExprValueKind ObjectKind) {
>> +  if (Candidate.isVariadic() != Incumbent.isVariadic() ||
>> +  Candidate.getNumParams() != Incumbent.getNumParams() ||
>> +  Candidate.getMinRequiredArguments() !=
>> +  Incumbent.getMinRequiredArguments())
>> +return OverloadCompare::BothViable;
>> +  for (unsigned I = 0, E = Candidate.getNumParams(); I != E; ++I)
>> +if (Candidate.parameters()[I]->getType().getCanonicalType() !=
>> +Incumbent.parameters()[I]->getType().getCanonicalType())
>> +  return OverloadCompare::BothViable;
>> +  if (!llvm::empty(Candidate.specific_attrs()) ||
>> +  !llvm::empty(Incumbent.specific_attrs()))
>> +return OverloadCompare::BothViable;
>> +  // At this point, we know calls can't pick one or the other based on
>> +  // arguments, so one of the two must win. (Or both fail, handled
>> elsewhere).
>> +  RefQualifierKind CandidateRef = Candidate.getRefQualifier();
>> +  RefQualifierKind IncumbentRef = Incumbent.getRefQualifier();
>> +  if (CandidateRef != IncumbentRef) {
>> +// If the object kind is LValue/RValue, there's one acceptabl

[PATCH] D63072: [clang] Fixing incorrect implicit deduction guides (PR41549)

[Tyker via Phabricator via cfe-commits]

Tyker created this revision.
Tyker added a reviewer: rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

bug report 

Before this patch, implicit deduction guides were generated from the first 
declaration found by lookup.
With this patch implicit deduction guides are generated from the definition of 
the class template.
Also added test that was previously failing.


Repository:
  rC Clang

https://reviews.llvm.org/D63072

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp


Index: clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
===
--- clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
+++ clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
@@ -489,6 +489,21 @@
 }
 #pragma clang diagnostic pop
 
+namespace PR41549 {
+
+template  struct umm;
+
+template 
+struct umm {
+  umm(H h = 0, P p = 0);
+};
+
+template  struct umm;
+
+umm<> m(1);
+
+}
+
 #else
 
 // expected-no-diagnostics
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -2052,6 +2052,12 @@
 
 void Sema::DeclareImplicitDeductionGuides(TemplateDecl *Template,
   SourceLocation Loc) {
+  if (CXXRecordDecl *DefRecord =
+  cast(Template->getTemplatedDecl())->getDefinition()) {
+TemplateDecl *DescribedTemplate = DefRecord->getDescribedClassTemplate();
+Template = DescribedTemplate ? DescribedTemplate : Template;
+  }
+
   DeclContext *DC = Template->getDeclContext();
   if (DC->isDependentContext())
 return;


Index: clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
===
--- clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
+++ clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
@@ -489,6 +489,21 @@
 }
 #pragma clang diagnostic pop
 
+namespace PR41549 {
+
+template  struct umm;
+
+template 
+struct umm {
+  umm(H h = 0, P p = 0);
+};
+
+template  struct umm;
+
+umm<> m(1);
+
+}
+
 #else
 
 // expected-no-diagnostics
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -2052,6 +2052,12 @@
 
 void Sema::DeclareImplicitDeductionGuides(TemplateDecl *Template,
   SourceLocation Loc) {
+  if (CXXRecordDecl *DefRecord =
+  cast(Template->getTemplatedDecl())->getDefinition()) {
+TemplateDecl *DescribedTemplate = DefRecord->getDescribedClassTemplate();
+Template = DescribedTemplate ? DescribedTemplate : Template;
+  }
+
   DeclContext *DC = Template->getDeclContext();
   if (DC->isDependentContext())
 return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63009: [OpenMP] Add target task alloc function with device ID

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

As for D63010 , the new function isn't used 
anywhere (and hence cannot be tested). What's the advantage of splitting 
changes at such granularity?


Repository:
  rC Clang

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

https://reviews.llvm.org/D63009



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

[clang-tools-extra] r362934 - [clangd] Stop marshalling/requiring FormattingOptions. We never did anything with them.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Mon Jun 10 06:01:49 2019
New Revision: 362934

URL: http://llvm.org/viewvc/llvm-project?rev=362934&view=rev
Log:
[clangd] Stop marshalling/requiring FormattingOptions. We never did anything 
with them.

Modified:
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/Protocol.h
clang-tools-extra/trunk/clangd/test/formatting.test

Modified: clang-tools-extra/trunk/clangd/Protocol.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.cpp?rev=362934&r1=362933&r2=362934&view=diff
==
--- clang-tools-extra/trunk/clangd/Protocol.cpp (original)
+++ clang-tools-extra/trunk/clangd/Protocol.cpp Mon Jun 10 06:01:49 2019
@@ -402,38 +402,23 @@ bool fromJSON(const llvm::json::Value &P
  O.map("text", R.text);
 }
 
-bool fromJSON(const llvm::json::Value &Params, FormattingOptions &R) {
-  llvm::json::ObjectMapper O(Params);
-  return O && O.map("tabSize", R.tabSize) &&
- O.map("insertSpaces", R.insertSpaces);
-}
-
-llvm::json::Value toJSON(const FormattingOptions &P) {
-  return llvm::json::Object{
-  {"tabSize", P.tabSize},
-  {"insertSpaces", P.insertSpaces},
-  };
-}
-
 bool fromJSON(const llvm::json::Value &Params,
   DocumentRangeFormattingParams &R) {
   llvm::json::ObjectMapper O(Params);
   return O && O.map("textDocument", R.textDocument) &&
- O.map("range", R.range) && O.map("options", R.options);
+ O.map("range", R.range);
 }
 
 bool fromJSON(const llvm::json::Value &Params,
   DocumentOnTypeFormattingParams &R) {
   llvm::json::ObjectMapper O(Params);
   return O && O.map("textDocument", R.textDocument) &&
- O.map("position", R.position) && O.map("ch", R.ch) &&
- O.map("options", R.options);
+ O.map("position", R.position) && O.map("ch", R.ch);
 }
 
 bool fromJSON(const llvm::json::Value &Params, DocumentFormattingParams &R) {
   llvm::json::ObjectMapper O(Params);
-  return O && O.map("textDocument", R.textDocument) &&
- O.map("options", R.options);
+  return O && O.map("textDocument", R.textDocument);
 }
 
 bool fromJSON(const llvm::json::Value &Params, DocumentSymbolParams &R) {

Modified: clang-tools-extra/trunk/clangd/Protocol.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.h?rev=362934&r1=362933&r2=362934&view=diff
==
--- clang-tools-extra/trunk/clangd/Protocol.h (original)
+++ clang-tools-extra/trunk/clangd/Protocol.h Mon Jun 10 06:01:49 2019
@@ -548,15 +548,13 @@ struct DidChangeConfigurationParams {
 };
 bool fromJSON(const llvm::json::Value &, DidChangeConfigurationParams &);
 
-struct FormattingOptions {
-  /// Size of a tab in spaces.
-  int tabSize = 0;
-
-  /// Prefer spaces over tabs.
-  bool insertSpaces = false;
-};
-bool fromJSON(const llvm::json::Value &, FormattingOptions &);
-llvm::json::Value toJSON(const FormattingOptions &);
+// Note: we do not parse FormattingOptions for *FormattingParams.
+// In general, we use a clang-format style detected from common mechanisms
+// (.clang-format files and the -fallback-style flag).
+// It would be possible to override these with FormatOptions, but:
+//  - the protocol makes FormatOptions mandatory, so many clients set them to
+//useless values, and we can't tell when to respect them
+// - we also format in other places, where FormatOptions aren't available.
 
 struct DocumentRangeFormattingParams {
   /// The document to format.
@@ -564,9 +562,6 @@ struct DocumentRangeFormattingParams {
 
   /// The range to format
   Range range;
-
-  /// The format options
-  FormattingOptions options;
 };
 bool fromJSON(const llvm::json::Value &, DocumentRangeFormattingParams &);
 
@@ -579,18 +574,12 @@ struct DocumentOnTypeFormattingParams {
 
   /// The character that has been typed.
   std::string ch;
-
-  /// The format options.
-  FormattingOptions options;
 };
 bool fromJSON(const llvm::json::Value &, DocumentOnTypeFormattingParams &);
 
 struct DocumentFormattingParams {
   /// The document to format.
   TextDocumentIdentifier textDocument;
-
-  /// The format options
-  FormattingOptions options;
 };
 bool fromJSON(const llvm::json::Value &, DocumentFormattingParams &);
 

Modified: clang-tools-extra/trunk/clangd/test/formatting.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/formatting.test?rev=362934&r1=362933&r2=362934&view=diff
==
--- clang-tools-extra/trunk/clangd/test/formatting.test (original)
+++ clang-tools-extra/trunk/clangd/test/formatting.test Mon Jun 10 06:01:49 2019
@@ -3,7 +3,7 @@
 ---
 
{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"int
 foo ( int x ) {\nx = x+1;\nret

[PATCH] D59919: [Attributor] Deduce "returned" argument attribute

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert updated this revision to Diff 203818.
jdoerfert added a comment.

Use worklist instead of recursion, add tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59919

Files:
  clang/test/CodeGenOpenCL/as_type.cl
  llvm/include/llvm/Transforms/IPO/Attributor.h
  llvm/lib/Transforms/IPO/Attributor.cpp
  llvm/test/Transforms/FunctionAttrs/arg_nocapture.ll
  llvm/test/Transforms/FunctionAttrs/arg_returned.ll
  llvm/test/Transforms/FunctionAttrs/read_write_returned_arguments_scc.ll

Index: llvm/test/Transforms/FunctionAttrs/read_write_returned_arguments_scc.ll
===
--- llvm/test/Transforms/FunctionAttrs/read_write_returned_arguments_scc.ll
+++ llvm/test/Transforms/FunctionAttrs/read_write_returned_arguments_scc.ll
@@ -31,7 +31,7 @@
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 
 ; CHECK: Function Attrs: nounwind
-; CHECK-NEXT: define i32* @external_ret2_nrw(i32* %n0, i32* %r0, i32* %w0)
+; CHECK-NEXT: define i32* @external_ret2_nrw(i32* %n0, i32* %r0, i32* returned %w0)
 define i32* @external_ret2_nrw(i32* %n0, i32* %r0, i32* %w0) {
 entry:
   %call = call i32* @internal_ret0_nw(i32* %n0, i32* %w0)
@@ -42,7 +42,7 @@
 }
 
 ; CHECK: Function Attrs: nounwind
-; CHECK-NEXT: define internal i32* @internal_ret0_nw(i32* %n0, i32* %w0)
+; CHECK-NEXT: define internal i32* @internal_ret0_nw(i32* returned %n0, i32* %w0)
 define internal i32* @internal_ret0_nw(i32* %n0, i32* %w0) {
 entry:
   %r0 = alloca i32, align 4
@@ -71,7 +71,7 @@
 }
 
 ; CHECK: Function Attrs: nounwind
-; CHECK-NEXT: define internal i32* @internal_ret1_rrw(i32* %r0, i32* %r1, i32* %w0)
+; CHECK-NEXT: define internal i32* @internal_ret1_rrw(i32* %r0, i32* returned %r1, i32* %w0)
 define internal i32* @internal_ret1_rrw(i32* %r0, i32* %r1, i32* %w0) {
 entry:
   %0 = load i32, i32* %r0, align 4
@@ -122,7 +122,7 @@
 }
 
 ; CHECK: Function Attrs: nounwind
-; CHECK-NEXT: define internal i32* @internal_ret1_rw(i32* %r0, i32* %w0)
+; CHECK-NEXT: define internal i32* @internal_ret1_rw(i32* %r0, i32* returned %w0)
 define internal i32* @internal_ret1_rw(i32* %r0, i32* %w0) {
 entry:
   %0 = load i32, i32* %r0, align 4
@@ -148,7 +148,7 @@
 }
 
 ; CHECK: Function Attrs: nounwind
-; CHECK-NEXT: define i32* @external_source_ret2_nrw(i32* %n0, i32* %r0, i32* %w0)
+; CHECK-NEXT: define i32* @external_source_ret2_nrw(i32* %n0, i32* %r0, i32* returned %w0)
 define i32* @external_source_ret2_nrw(i32* %n0, i32* %r0, i32* %w0) {
 entry:
   %call = call i32* @external_sink_ret2_nrw(i32* %n0, i32* %r0, i32* %w0)
Index: llvm/test/Transforms/FunctionAttrs/arg_returned.ll
===
--- llvm/test/Transforms/FunctionAttrs/arg_returned.ll
+++ llvm/test/Transforms/FunctionAttrs/arg_returned.ll
@@ -1,5 +1,6 @@
-; RUN: opt -functionattrs -attributor -attributor-disable=false -S < %s | FileCheck %s
-; RUN: opt -functionattrs -attributor -attributor-disable=false -attributor-verify=true -S < %s | FileCheck %s
+; RUN: opt -functionattrs -S < %s | FileCheck %s --check-prefix=FNATTR
+; RUN: opt -attributor -attributor-disable=false -S < %s | FileCheck %s --check-prefix=ATTRIBUTOR
+; RUN: opt -attributor -attributor-disable=false -functionattrs -S < %s | FileCheck %s --check-prefix=BOTH
 ;
 ; Test cases specifically designed for the "returned" argument attribute.
 ; We use FIXME's to indicate problems and missing attributes.
@@ -7,16 +8,24 @@
 
 ; TEST SCC test returning an integer value argument
 ;
-; CHECK: Function Attrs: noinline norecurse nounwind readnone uwtable
-; CHECK: define i32 @sink_r0(i32 returned %r)
-;
-; FIXME: returned on %r missing:
-; CHECK: Function Attrs: noinline nounwind readnone uwtable
-; CHECK: define i32 @scc_r1(i32 %a, i32 %r, i32 %b)
-;
-; FIXME: returned on %r missing:
-; CHECK: Function Attrs: noinline nounwind readnone uwtable
-; CHECK: define i32 @scc_r2(i32 %a, i32 %b, i32 %r)
+; BOTH: Function Attrs: noinline norecurse nounwind readnone uwtable
+; BOTH: define i32 @sink_r0(i32 returned %r)
+; BOTH: Function Attrs: noinline nounwind readnone uwtable
+; BOTH: define i32 @scc_r1(i32 %a, i32 returned %r, i32 %b)
+; BOTH: Function Attrs: noinline nounwind readnone uwtable
+; BOTH: define i32 @scc_r2(i32 %a, i32 %b, i32 returned %r)
+; BOTH: Function Attrs: noinline nounwind readnone uwtable
+; BOTH: define i32 @scc_rX(i32 %a, i32 %b, i32 %r)
+;
+; FNATTR: define i32 @sink_r0(i32 returned %r)
+; FNATTR: define i32 @scc_r1(i32 %a, i32 %r, i32 %b)
+; FNATTR: define i32 @scc_r2(i32 %a, i32 %b, i32 %r)
+; FNATTR: define i32 @scc_rX(i32 %a, i32 %b, i32 %r)
+;
+; ATTRIBUTOR: define i32 @sink_r0(i32 returned %r)
+; ATTRIBUTOR: define i32 @scc_r1(i32 %a, i32 returned %r, i32 %b)
+; ATTRIBUTOR: define i32 @scc_r2(i32 %a, i32 %b, i32 returned %r)
+; ATTRIBUTOR: define i32 @scc_rX(i32 %a, i32 %b, i32 %r)
 ;
 ; int scc_r1(int a, int b, int r)

[PATCH] D60605: [clangd] Revamp textDocument/onTypeFormatting.

[Sam McCall via Phabricator via cfe-commits]

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

OK, this has been stuck for a while and (as discussed a bit offline) I haven't 
been able to make the alternative approaches work.

In D60605#1495268 , @yvvan wrote:

> @ilya-biryukov 
>  What do you think about D53072 ? It can be 
> polished and combined with this change removing some code from here (which I 
> assume is a good thing).
>  The idea there is that clang-format knows that it's not allowed to remove 
> new lines and it always marks them with MustBreakBefore.
>  I'm not sure if anybody needs an executable but I think it's not a big deal 
> to have an extra reformat() function.
>
> It is also safe because it was the only way I found which does not break the 
> code style like, for instance, adding comment block to the end of the 
> previous line.


I'm not sure D53072  helps in its current 
form. Preserving newlines with content on the line isn't always the (IMO) right 
formatting: see e.g. FormatBrace test.
(I have other concerns about the details of that patch, but they don't really 
relate to this one).

In D60605#1518031 , @ilya-biryukov 
wrote:

> LGTM with a few NITs and a few questions about the possibility of moving this 
> to `clang-format` at some point in the future.
>  From what I can to simplify the calling, we need to:
>
> 1. teach `clang-format` about a cursor (by making it a special token or 
> somehow else), this would allow to avoid some book-keeping;


There's at least one wrinkle with making it a special token: it may be within a 
token. E.g. after splitting a line comment, the cursor is at offset 3 within 
`// the new line`. Not sure how deep this problem goes.

> 2. teach `clang-format` to format in presence of unmatched parentheses. 
> Again, would simplify things. Although not sure it's easy to do in 
> `clang-format` either;

Note this needs to be a style or other option, as the fact that clang-format 
bails out on broken code in general is well-established desired behavior.
I attempted this (by having the parser return "true" rather than "false" on 
reaching the end of a paren/brace list, and got lost in the details. I'm not 
sure whether it's likely to work, or whether clang-format would just do the 
same source transformation as we do here. (Obviously sharing it would be nice).

> 3. teach `clang-format` to respect indentation before the cursor (added by 
> the editor in our case).

I'm not sure what you mean by this: I don't think we want that indentation to 
be respected, but rather reformatted away.

> After that we don't seem to need the complicated multi-pass replacements.

We currently have pre-formatting changes (e.g. comment splitting). I'm not sure 
if you expect these to belong to clang-format, but I wouldn't. (They're more 
like a "respond to newline" helper for editors than they are like formatting a 
range). So you'd go from {pre-formatting changes, insert placeholder, 
clang-format, remove placeholder} to {pre-formatting changes, clang-format}.
So the logic to implement multi-pass replacements is still needed I think, 
though you'd have fewer actual passes.

> - Am I missing something that we also need from `clang-format` here?

The comment splitting and {} splitting, I think.

> - How would you estimate the amount of work needed to move this functionality 
> to `clang-format`?
> - In general, do you think it's worth moving it there or not?

I don't know, I put maybe 8 hours into this approach and didn't make any 
progress.
If someone finds a clean way to do it, I'd be in favour. But a 
hacky/brute-force way would do more harm than good in my opinion.

> In the meantime, I've found another case where newline gets eaten:

Yes :-(
I've added a test to document this bad behavior. It's very hard to fix with 
clang-format as a black-box, because it hard-codes erasure of blank lines 
adjacent to a formatted region.
Probably the simplest approach is to add a clang-format option to always 
preserve blank lines. Although the motivation isn't an actual style guide, it 
makes sense to think of this as a style rule, it's very simple, and it's 
related to existing options.




Comment at: clangd/Format.h:25
+/// Applies limited formatting around new \p InsertedText.
+/// The \p Code already contains the updated text near \p Cursor, and may have
+/// had additional / characters (such as indentation) inserted by the editor.

ilya-biryukov wrote:
> NIT: maybe be more concrete? `near \p Cursor` means `right before \p Cursor`? 
> Or can it have other characters (e.g. whitespace in your example)?
s/near/before/.
Yes, it can have other characters, these are already mentioned in the comment.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60605

[clang-tools-extra] r362939 - [clangd] Revamp textDocument/onTypeFormatting.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Mon Jun 10 07:26:21 2019
New Revision: 362939

URL: http://llvm.org/viewvc/llvm-project?rev=362939&view=rev
Log:
[clangd] Revamp textDocument/onTypeFormatting.

Summary:
The existing implementation (which triggers on }) is fairly simple and
has flaws:
 - doesn't trigger frequently/regularly enough (particularly in editors that 
type the }
 for you)
 - often reformats too much code around the edit
 - has jarring cases that I don't have clear ideas for fixing

This implementation is designed to trigger on newline, which feels to me more
intuitive than } or ;.
It does have allow for reformatting after other characters - it has a
basic behavior and a model for adding specialized behavior for
particular characters. But at least initially I'd stick to advertising
\n in the capabilities.

This also handles comment splitting: when you insert a line break inside
a line comment, it will make the new line into an aligned line comment.

Working on tests, but want people to patch it in and try it - it's hard to
see if "feel" is right purely by looking at a test.

Reviewers: ilya-biryukov, hokein

Subscribers: mgorny, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

Added:
clang-tools-extra/trunk/clangd/Format.cpp
clang-tools-extra/trunk/clangd/Format.h
clang-tools-extra/trunk/clangd/unittests/FormatTests.cpp
Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/test/formatting.test
clang-tools-extra/trunk/clangd/test/initialize-params.test
clang-tools-extra/trunk/clangd/unittests/CMakeLists.txt

Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=362939&r1=362938&r2=362939&view=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Mon Jun 10 07:26:21 2019
@@ -48,6 +48,7 @@ add_clang_library(clangDaemon
   ExpectedTypes.cpp
   FindSymbols.cpp
   FileDistance.cpp
+  Format.cpp
   FS.cpp
   FSProvider.cpp
   FormattedString.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=362939&r1=362938&r2=362939&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Mon Jun 10 07:26:21 2019
@@ -359,7 +359,7 @@ void ClangdLSPServer::onInitialize(const
 {"documentRangeFormattingProvider", true},
 {"documentOnTypeFormattingProvider",
  llvm::json::Object{
- {"firstTriggerCharacter", "}"},
+ {"firstTriggerCharacter", "\n"},
  {"moreTriggerCharacter", {}},
  }},
 {"codeActionProvider", true},
@@ -576,11 +576,7 @@ void ClangdLSPServer::onDocumentOnTypeFo
 "onDocumentOnTypeFormatting called for non-added file",
 ErrorCode::InvalidParams));
 
-  auto ReplacementsOrError = Server->formatOnType(*Code, File, 
Params.position);
-  if (ReplacementsOrError)
-Reply(replacementsToEdits(*Code, ReplacementsOrError.get()));
-  else
-Reply(ReplacementsOrError.takeError());
+  Reply(Server->formatOnType(*Code, File, Params.position, Params.ch));
 }
 
 void ClangdLSPServer::onDocumentRangeFormatting(

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=362939&r1=362938&r2=362939&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Mon Jun 10 07:26:21 2019
@@ -10,6 +10,7 @@
 #include "ClangdUnit.h"
 #include "CodeComplete.h"
 #include "FindSymbols.h"
+#include "Format.h"
 #include "FormattedString.h"
 #include "Headers.h"
 #include "Protocol.h"
@@ -250,20 +251,23 @@ ClangdServer::formatFile(llvm::StringRef
   return formatCode(Code, File, {tooling::Range(0, Code.size())});
 }
 
-llvm::Expected
-ClangdServer::formatOnType(llvm::StringRef Code, PathRef File, Position Pos) {
-  // Look for the previous opening brace from the character position and
-  // format starting from there.
+llvm::Expected>
+ClangdServer::formatOnType(llvm::StringRef Code, PathRef File, Position Pos,
+   StringRef TriggerText) {
   llvm::Expected CursorPos = positionToOffset(Code, Pos);
   if (!CursorPos)
 return CursorPos.takeError();
-  size_t PreviousLBracePos =
-  

[PATCH] D59919: [Attributor] Deduce "returned" argument attribute

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert updated this revision to Diff 203819.
jdoerfert marked 2 inline comments as done.
jdoerfert added a comment.

Fix Typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59919

Files:
  clang/test/CodeGenOpenCL/as_type.cl
  llvm/include/llvm/Transforms/IPO/Attributor.h
  llvm/lib/Transforms/IPO/Attributor.cpp
  llvm/test/Transforms/FunctionAttrs/arg_nocapture.ll
  llvm/test/Transforms/FunctionAttrs/arg_returned.ll
  llvm/test/Transforms/FunctionAttrs/read_write_returned_arguments_scc.ll

Index: llvm/test/Transforms/FunctionAttrs/read_write_returned_arguments_scc.ll
===
--- llvm/test/Transforms/FunctionAttrs/read_write_returned_arguments_scc.ll
+++ llvm/test/Transforms/FunctionAttrs/read_write_returned_arguments_scc.ll
@@ -31,7 +31,7 @@
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 
 ; CHECK: Function Attrs: nounwind
-; CHECK-NEXT: define i32* @external_ret2_nrw(i32* %n0, i32* %r0, i32* %w0)
+; CHECK-NEXT: define i32* @external_ret2_nrw(i32* %n0, i32* %r0, i32* returned %w0)
 define i32* @external_ret2_nrw(i32* %n0, i32* %r0, i32* %w0) {
 entry:
   %call = call i32* @internal_ret0_nw(i32* %n0, i32* %w0)
@@ -42,7 +42,7 @@
 }
 
 ; CHECK: Function Attrs: nounwind
-; CHECK-NEXT: define internal i32* @internal_ret0_nw(i32* %n0, i32* %w0)
+; CHECK-NEXT: define internal i32* @internal_ret0_nw(i32* returned %n0, i32* %w0)
 define internal i32* @internal_ret0_nw(i32* %n0, i32* %w0) {
 entry:
   %r0 = alloca i32, align 4
@@ -71,7 +71,7 @@
 }
 
 ; CHECK: Function Attrs: nounwind
-; CHECK-NEXT: define internal i32* @internal_ret1_rrw(i32* %r0, i32* %r1, i32* %w0)
+; CHECK-NEXT: define internal i32* @internal_ret1_rrw(i32* %r0, i32* returned %r1, i32* %w0)
 define internal i32* @internal_ret1_rrw(i32* %r0, i32* %r1, i32* %w0) {
 entry:
   %0 = load i32, i32* %r0, align 4
@@ -122,7 +122,7 @@
 }
 
 ; CHECK: Function Attrs: nounwind
-; CHECK-NEXT: define internal i32* @internal_ret1_rw(i32* %r0, i32* %w0)
+; CHECK-NEXT: define internal i32* @internal_ret1_rw(i32* %r0, i32* returned %w0)
 define internal i32* @internal_ret1_rw(i32* %r0, i32* %w0) {
 entry:
   %0 = load i32, i32* %r0, align 4
@@ -148,7 +148,7 @@
 }
 
 ; CHECK: Function Attrs: nounwind
-; CHECK-NEXT: define i32* @external_source_ret2_nrw(i32* %n0, i32* %r0, i32* %w0)
+; CHECK-NEXT: define i32* @external_source_ret2_nrw(i32* %n0, i32* %r0, i32* returned %w0)
 define i32* @external_source_ret2_nrw(i32* %n0, i32* %r0, i32* %w0) {
 entry:
   %call = call i32* @external_sink_ret2_nrw(i32* %n0, i32* %r0, i32* %w0)
Index: llvm/test/Transforms/FunctionAttrs/arg_returned.ll
===
--- llvm/test/Transforms/FunctionAttrs/arg_returned.ll
+++ llvm/test/Transforms/FunctionAttrs/arg_returned.ll
@@ -1,5 +1,6 @@
-; RUN: opt -functionattrs -attributor -attributor-disable=false -S < %s | FileCheck %s
-; RUN: opt -functionattrs -attributor -attributor-disable=false -attributor-verify=true -S < %s | FileCheck %s
+; RUN: opt -functionattrs -S < %s | FileCheck %s --check-prefix=FNATTR
+; RUN: opt -attributor -attributor-disable=false -S < %s | FileCheck %s --check-prefix=ATTRIBUTOR
+; RUN: opt -attributor -attributor-disable=false -functionattrs -S < %s | FileCheck %s --check-prefix=BOTH
 ;
 ; Test cases specifically designed for the "returned" argument attribute.
 ; We use FIXME's to indicate problems and missing attributes.
@@ -7,16 +8,24 @@
 
 ; TEST SCC test returning an integer value argument
 ;
-; CHECK: Function Attrs: noinline norecurse nounwind readnone uwtable
-; CHECK: define i32 @sink_r0(i32 returned %r)
-;
-; FIXME: returned on %r missing:
-; CHECK: Function Attrs: noinline nounwind readnone uwtable
-; CHECK: define i32 @scc_r1(i32 %a, i32 %r, i32 %b)
-;
-; FIXME: returned on %r missing:
-; CHECK: Function Attrs: noinline nounwind readnone uwtable
-; CHECK: define i32 @scc_r2(i32 %a, i32 %b, i32 %r)
+; BOTH: Function Attrs: noinline norecurse nounwind readnone uwtable
+; BOTH: define i32 @sink_r0(i32 returned %r)
+; BOTH: Function Attrs: noinline nounwind readnone uwtable
+; BOTH: define i32 @scc_r1(i32 %a, i32 returned %r, i32 %b)
+; BOTH: Function Attrs: noinline nounwind readnone uwtable
+; BOTH: define i32 @scc_r2(i32 %a, i32 %b, i32 returned %r)
+; BOTH: Function Attrs: noinline nounwind readnone uwtable
+; BOTH: define i32 @scc_rX(i32 %a, i32 %b, i32 %r)
+;
+; FNATTR: define i32 @sink_r0(i32 returned %r)
+; FNATTR: define i32 @scc_r1(i32 %a, i32 %r, i32 %b)
+; FNATTR: define i32 @scc_r2(i32 %a, i32 %b, i32 %r)
+; FNATTR: define i32 @scc_rX(i32 %a, i32 %b, i32 %r)
+;
+; ATTRIBUTOR: define i32 @sink_r0(i32 returned %r)
+; ATTRIBUTOR: define i32 @scc_r1(i32 %a, i32 returned %r, i32 %b)
+; ATTRIBUTOR: define i32 @scc_r2(i32 %a, i32 %b, i32 returned %r)
+; ATTRIBUTOR: define i32 @scc_rX(i32 %a, i32 %b, i32 %r)
 ;
 ; int scc_r1(int a, int b

[PATCH] D60605: [clangd] Revamp textDocument/onTypeFormatting.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.
Closed by commit rL362939: [clangd] Revamp textDocument/onTypeFormatting. 
(authored by sammccall, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60605?vs=197928&id=203821#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60605

Files:
  clang-tools-extra/trunk/clangd/CMakeLists.txt
  clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
  clang-tools-extra/trunk/clangd/ClangdServer.cpp
  clang-tools-extra/trunk/clangd/ClangdServer.h
  clang-tools-extra/trunk/clangd/Format.cpp
  clang-tools-extra/trunk/clangd/Format.h
  clang-tools-extra/trunk/clangd/test/formatting.test
  clang-tools-extra/trunk/clangd/test/initialize-params.test
  clang-tools-extra/trunk/clangd/unittests/CMakeLists.txt
  clang-tools-extra/trunk/clangd/unittests/FormatTests.cpp

Index: clang-tools-extra/trunk/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/trunk/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/trunk/clangd/unittests/CMakeLists.txt
@@ -39,6 +39,7 @@
   FileIndexTests.cpp
   FindSymbolsTests.cpp
   FormattedStringTests.cpp
+  FormatTests.cpp
   FSTests.cpp
   FunctionTests.cpp
   FuzzyMatchTests.cpp
Index: clang-tools-extra/trunk/clangd/unittests/FormatTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/FormatTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/FormatTests.cpp
@@ -0,0 +1,308 @@
+//===-- FormatTests.cpp - Automatic code formatting tests -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Format.h"
+#include "Annotations.h"
+#include "SourceCode.h"
+#include "TestFS.h"
+#include "clang/Format/Format.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/Support/Error.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+std::string afterTyped(llvm::StringRef CodeWithCursor,
+   llvm::StringRef Typed) {
+  Annotations Code(CodeWithCursor);
+  unsigned Cursor = llvm::cantFail(positionToOffset(Code.code(), Code.point()));
+  auto Changes =
+  formatIncremental(Code.code(), Cursor, Typed,
+format::getGoogleStyle(format::FormatStyle::LK_Cpp));
+  tooling::Replacements Merged;
+  for (const auto& R : Changes)
+if (llvm::Error E = Merged.add(R))
+  ADD_FAILURE() << llvm::toString(std::move(E));
+  auto NewCode = tooling::applyAllReplacements(Code.code(), Merged);
+  EXPECT_TRUE(bool(NewCode))
+  << "Bad replacements: " << llvm::toString(NewCode.takeError());
+  NewCode->insert(transformCursorPosition(Cursor, Changes), "^");
+  return *NewCode;
+}
+
+// We can't pass raw strings directly to EXPECT_EQ because of gcc bugs.
+void expectAfterNewline(const char *Before, const char *After) {
+  EXPECT_EQ(After, afterTyped(Before, "\n")) << Before;
+}
+void expectAfter(const char *Typed, const char *Before, const char *After) {
+  EXPECT_EQ(After, afterTyped(Before, Typed)) << Before;
+}
+
+TEST(FormatIncremental, SplitComment) {
+  expectAfterNewline(R"cpp(
+// this comment was
+^split
+)cpp",
+   R"cpp(
+// this comment was
+// ^split
+)cpp");
+
+  expectAfterNewline(R"cpp(
+// trailing whitespace is not a split
+^   
+)cpp",
+   R"cpp(
+// trailing whitespace is not a split
+^
+)cpp");
+
+  expectAfterNewline(R"cpp(
+// splitting a
+^
+// multiline comment
+)cpp",
+ R"cpp(
+// splitting a
+// ^
+// multiline comment
+)cpp");
+
+  expectAfterNewline(R"cpp(
+// extra   
+^ whitespace
+)cpp",
+   R"cpp(
+// extra
+// ^whitespace
+)cpp");
+
+  expectAfterNewline(R"cpp(
+/// triple
+^slash
+)cpp",
+   R"cpp(
+/// triple
+/// ^slash
+)cpp");
+
+  expectAfterNewline(R"cpp(
+/// editor continuation
+//^
+)cpp",
+   R"cpp(
+/// editor continuation
+/// ^
+)cpp");
+
+  expectAfterNewline(R"cpp(
+// break before
+^ // slashes
+)cpp",
+   R"cpp(
+// break before
+^// slashes
+)cpp");
+
+
+  expectAfterNewline(R"cpp(
+int x;  // aligned
+^comment
+)cpp",
+   R"cpp(
+int x;  // aligned
+// ^comment
+)cpp");
+
+  // Fixed bug: the second line of the aligned comment shouldn't be "attached"
+  // to the cursor and outdented.
+  expectAfterNewline(R"cpp(
+void foo() {
+  if (x)
+return; // All spelled tokens are accounted for.
+// that takes two lines
+^
+}
+)cpp",
+   R"cpp(
+void foo() {
+  if (x)
+return;  // All spelled tokens are accounted for

r362944 - Revert "[CodeComplete] Improve overload handling for C++ qualified and ref-qualified methods."

[Sam McCall via cfe-commits]

Author: sammccall
Date: Mon Jun 10 07:55:57 2019
New Revision: 362944

URL: http://llvm.org/viewvc/llvm-project?rev=362944&view=rev
Log:
Revert "[CodeComplete] Improve overload handling for C++ qualified and 
ref-qualified methods."

This reverts commit r362924, which causes a double-free of ShadowMapEntry.

Modified:
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/test/CodeCompletion/member-access.cpp

Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=362944&r1=362943&r2=362944&view=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Mon Jun 10 07:55:57 2019
@@ -16,9 +16,7 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/QualTypeNames.h"
-#include "clang/AST/Type.h"
 #include "clang/Basic/CharInfo.h"
-#include "clang/Basic/Specifiers.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/Preprocessor.h"
@@ -107,7 +105,7 @@ private:
   DeclIndexPair(ND, Index));
 }
 
-~ShadowMapEntry() {
+void Destroy() {
   if (DeclIndexPairVector *Vec =
   DeclOrVector.dyn_cast()) {
 delete Vec;
@@ -154,16 +152,9 @@ private:
   /// different levels of, e.g., the inheritance hierarchy.
   std::list ShadowMaps;
 
-  /// Overloaded C++ member functions found by SemaLookup.
-  /// Used to determine when one overload is dominated by another.
-  llvm::DenseMap, ShadowMapEntry>
-  OverloadMap;
-
   /// If we're potentially referring to a C++ member function, the set
   /// of qualifiers applied to the object type.
   Qualifiers ObjectTypeQualifiers;
-  /// The kind of the object expression, for rvalue/lvalue overloads.
-  ExprValueKind ObjectKind;
 
   /// Whether the \p ObjectTypeQualifiers field is active.
   bool HasObjectTypeQualifiers;
@@ -239,9 +230,8 @@ public:
   /// out member functions that aren't available (because there will be a
   /// cv-qualifier mismatch) or prefer functions with an exact qualifier
   /// match.
-  void setObjectTypeQualifiers(Qualifiers Quals, ExprValueKind Kind) {
+  void setObjectTypeQualifiers(Qualifiers Quals) {
 ObjectTypeQualifiers = Quals;
-ObjectKind = Kind;
 HasObjectTypeQualifiers = true;
   }
 
@@ -1167,53 +1157,6 @@ static void setInBaseClass(ResultBuilder
   R.InBaseClass = true;
 }
 
-enum class OverloadCompare { BothViable, Dominates, Dominated };
-// Will Candidate ever be called on the object, when overloaded with Incumbent?
-// Returns Dominates if Candidate is always called, Dominated if Incumbent is
-// always called, BothViable if either may be called dependending on arguments.
-// Precondition: must actually be overloads!
-static OverloadCompare compareOverloads(const CXXMethodDecl &Candidate,
-const CXXMethodDecl &Incumbent,
-const Qualifiers &ObjectQuals,
-ExprValueKind ObjectKind) {
-  if (Candidate.isVariadic() != Incumbent.isVariadic() ||
-  Candidate.getNumParams() != Incumbent.getNumParams() ||
-  Candidate.getMinRequiredArguments() !=
-  Incumbent.getMinRequiredArguments())
-return OverloadCompare::BothViable;
-  for (unsigned I = 0, E = Candidate.getNumParams(); I != E; ++I)
-if (Candidate.parameters()[I]->getType().getCanonicalType() !=
-Incumbent.parameters()[I]->getType().getCanonicalType())
-  return OverloadCompare::BothViable;
-  if (!llvm::empty(Candidate.specific_attrs()) ||
-  !llvm::empty(Incumbent.specific_attrs()))
-return OverloadCompare::BothViable;
-  // At this point, we know calls can't pick one or the other based on
-  // arguments, so one of the two must win. (Or both fail, handled elsewhere).
-  RefQualifierKind CandidateRef = Candidate.getRefQualifier();
-  RefQualifierKind IncumbentRef = Incumbent.getRefQualifier();
-  if (CandidateRef != IncumbentRef) {
-// If the object kind is LValue/RValue, there's one acceptable 
ref-qualifier
-// and it can't be mixed with ref-unqualified overloads (in valid code).
-
-// For xvalue objects, we prefer the rvalue overload even if we have to
-// add qualifiers (which is rare, because const&& is rare).
-if (ObjectKind == clang::VK_XValue)
-  return CandidateRef == RQ_RValue ? OverloadCompare::Dominates
-   : OverloadCompare::Dominated;
-  }
-  // Now the ref qualifiers are the same (or we're in some invalid state).
-  // So make some decision based on the qualifiers.
-  Qualifiers CandidateQual = Candidate.getMethodQualifiers();
-  Qualifiers IncumbentQual = Incumbent.getMethodQualifiers();
-  bool CandidateSuperset = CandidateQual.compatiblyIncludes(IncumbentQual);
-  bool IncumbentSuperset = IncumbentQual.compatiblyIncludes(C

[PATCH] D62739: AMDGPU: Always emit amdgpu-flat-work-group-size

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

ping


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

https://reviews.llvm.org/D62739



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

r362950 - Re-land "[CodeComplete] Improve overload handling for C++ qualified and ref-qualified methods."

[Sam McCall via cfe-commits]

Author: sammccall
Date: Mon Jun 10 08:17:52 2019
New Revision: 362950

URL: http://llvm.org/viewvc/llvm-project?rev=362950&view=rev
Log:
Re-land "[CodeComplete] Improve overload handling for C++ qualified and 
ref-qualified methods."

ShadowMapEntry is now really, truly a normal class.

Modified:
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/test/CodeCompletion/member-access.cpp

Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=362950&r1=362949&r2=362950&view=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Mon Jun 10 08:17:52 2019
@@ -16,7 +16,9 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/QualTypeNames.h"
+#include "clang/AST/Type.h"
 #include "clang/Basic/CharInfo.h"
+#include "clang/Basic/Specifiers.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/Preprocessor.h"
@@ -82,6 +84,15 @@ private:
 
   public:
 ShadowMapEntry() : DeclOrVector(), SingleDeclIndex(0) {}
+ShadowMapEntry(const ShadowMapEntry &) = delete;
+ShadowMapEntry(ShadowMapEntry &&Move) { *this = std::move(Move); }
+ShadowMapEntry &operator=(const ShadowMapEntry &) = delete;
+ShadowMapEntry &operator=(ShadowMapEntry &&Move) {
+  SingleDeclIndex = Move.SingleDeclIndex;
+  DeclOrVector = Move.DeclOrVector;
+  Move.DeclOrVector = nullptr;
+  return *this;
+}
 
 void Add(const NamedDecl *ND, unsigned Index) {
   if (DeclOrVector.isNull()) {
@@ -105,7 +116,7 @@ private:
   DeclIndexPair(ND, Index));
 }
 
-void Destroy() {
+~ShadowMapEntry() {
   if (DeclIndexPairVector *Vec =
   DeclOrVector.dyn_cast()) {
 delete Vec;
@@ -152,9 +163,16 @@ private:
   /// different levels of, e.g., the inheritance hierarchy.
   std::list ShadowMaps;
 
+  /// Overloaded C++ member functions found by SemaLookup.
+  /// Used to determine when one overload is dominated by another.
+  llvm::DenseMap, ShadowMapEntry>
+  OverloadMap;
+
   /// If we're potentially referring to a C++ member function, the set
   /// of qualifiers applied to the object type.
   Qualifiers ObjectTypeQualifiers;
+  /// The kind of the object expression, for rvalue/lvalue overloads.
+  ExprValueKind ObjectKind;
 
   /// Whether the \p ObjectTypeQualifiers field is active.
   bool HasObjectTypeQualifiers;
@@ -230,8 +248,9 @@ public:
   /// out member functions that aren't available (because there will be a
   /// cv-qualifier mismatch) or prefer functions with an exact qualifier
   /// match.
-  void setObjectTypeQualifiers(Qualifiers Quals) {
+  void setObjectTypeQualifiers(Qualifiers Quals, ExprValueKind Kind) {
 ObjectTypeQualifiers = Quals;
+ObjectKind = Kind;
 HasObjectTypeQualifiers = true;
   }
 
@@ -1157,6 +1176,53 @@ static void setInBaseClass(ResultBuilder
   R.InBaseClass = true;
 }
 
+enum class OverloadCompare { BothViable, Dominates, Dominated };
+// Will Candidate ever be called on the object, when overloaded with Incumbent?
+// Returns Dominates if Candidate is always called, Dominated if Incumbent is
+// always called, BothViable if either may be called dependending on arguments.
+// Precondition: must actually be overloads!
+static OverloadCompare compareOverloads(const CXXMethodDecl &Candidate,
+const CXXMethodDecl &Incumbent,
+const Qualifiers &ObjectQuals,
+ExprValueKind ObjectKind) {
+  if (Candidate.isVariadic() != Incumbent.isVariadic() ||
+  Candidate.getNumParams() != Incumbent.getNumParams() ||
+  Candidate.getMinRequiredArguments() !=
+  Incumbent.getMinRequiredArguments())
+return OverloadCompare::BothViable;
+  for (unsigned I = 0, E = Candidate.getNumParams(); I != E; ++I)
+if (Candidate.parameters()[I]->getType().getCanonicalType() !=
+Incumbent.parameters()[I]->getType().getCanonicalType())
+  return OverloadCompare::BothViable;
+  if (!llvm::empty(Candidate.specific_attrs()) ||
+  !llvm::empty(Incumbent.specific_attrs()))
+return OverloadCompare::BothViable;
+  // At this point, we know calls can't pick one or the other based on
+  // arguments, so one of the two must win. (Or both fail, handled elsewhere).
+  RefQualifierKind CandidateRef = Candidate.getRefQualifier();
+  RefQualifierKind IncumbentRef = Incumbent.getRefQualifier();
+  if (CandidateRef != IncumbentRef) {
+// If the object kind is LValue/RValue, there's one acceptable 
ref-qualifier
+// and it can't be mixed with ref-unqualified overloads (in valid code).
+
+// For xvalue objects, we prefer the rvalue overload even if we have to
+// add qualifiers (which is rare, because const&& is rare).
+if 

[PATCH] D63080: [analyzer] Track indices of arrays

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus created this revision.
Szelethus added reviewers: NoQ, dcoughlin, xazax.hun, rnkovacs, 
baloghadamsoftware, Charusso.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, szepet, whisperity.

Observe the test file before this patch:
F9155603: image.png 

I think this patch really improves on this case. The problem however, that some 
subexpressions of expressions should be tracked (like `n` in `arr[n]` here) is 
a broader one, and there may be other some cases I could add.


Repository:
  rC Clang

https://reviews.llvm.org/D63080

Files:
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/test/Analysis/diagnostics/track_subexpressions.cpp


Index: clang/test/Analysis/diagnostics/track_subexpressions.cpp
===
--- clang/test/Analysis/diagnostics/track_subexpressions.cpp
+++ clang/test/Analysis/diagnostics/track_subexpressions.cpp
@@ -17,3 +17,35 @@
   (void)(TCP_MAXWIN << shift_amount); // expected-warning{{The result of the 
left shift is undefined due to shifting by '255', which is greater or equal to 
the width of type 'int'}}
   // expected-note@-1{{The result of the 
left shift is undefined due to shifting by '255', which is greater or equal to 
the width of type 'int'}}
 }
+
+namespace array_index_tracking {
+void consume(int);
+
+int getIndex(int x) {
+  int a;
+  if (x > 0) // expected-note {{Assuming 'x' is > 0}}
+ // expected-note@-1 {{Taking true branch}}
+a = 3; // expected-note {{The value 3 is assigned to 'a'}}
+  else
+a = 2;
+  return a; // expected-note {{Returning the value 3 (loaded from 'a')}}
+}
+
+int getInt();
+
+void testArrayIndexTracking() {
+  int arr[10];
+
+  for (int i = 0; i < 3; ++i)
+// expected-note@-1 3{{Loop condition is true.  Entering loop body}}
+// expected-note@-2 {{Loop condition is false. Execution continues on line 
43}}
+arr[i] = 0;
+  int x = getInt();
+  int n = getIndex(x); // expected-note {{Calling 'getIndex'}}
+   // expected-note@-1 {{Returning from 'getIndex'}}
+   // expected-note@-2 {{'n' initialized to 3}}
+  consume(arr[n]);
+  // expected-note@-1 {{1st function call argument is an uninitialized value}}
+  // expected-warning@-2{{1st function call argument is an uninitialized 
value}}
+}
+} // end of namespace array_index_tracking
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1676,6 +1676,10 @@
   if (const Expr *Receiver = NilReceiverBRVisitor::getNilReceiver(Inner, 
LVNode))
 trackExpressionValue(LVNode, Receiver, report, EnableNullFPSuppression);
 
+  if (const auto *Arr = dyn_cast(Inner))
+trackExpressionValue(
+LVNode, Arr->getIdx(), report, EnableNullFPSuppression);
+
   // See if the expression we're interested refers to a variable.
   // If so, we can track both its contents and constraints on its value.
   if (ExplodedGraph::isInterestingLValueExpr(Inner)) {


Index: clang/test/Analysis/diagnostics/track_subexpressions.cpp
===
--- clang/test/Analysis/diagnostics/track_subexpressions.cpp
+++ clang/test/Analysis/diagnostics/track_subexpressions.cpp
@@ -17,3 +17,35 @@
   (void)(TCP_MAXWIN << shift_amount); // expected-warning{{The result of the left shift is undefined due to shifting by '255', which is greater or equal to the width of type 'int'}}
   // expected-note@-1{{The result of the left shift is undefined due to shifting by '255', which is greater or equal to the width of type 'int'}}
 }
+
+namespace array_index_tracking {
+void consume(int);
+
+int getIndex(int x) {
+  int a;
+  if (x > 0) // expected-note {{Assuming 'x' is > 0}}
+ // expected-note@-1 {{Taking true branch}}
+a = 3; // expected-note {{The value 3 is assigned to 'a'}}
+  else
+a = 2;
+  return a; // expected-note {{Returning the value 3 (loaded from 'a')}}
+}
+
+int getInt();
+
+void testArrayIndexTracking() {
+  int arr[10];
+
+  for (int i = 0; i < 3; ++i)
+// expected-note@-1 3{{Loop condition is true.  Entering loop body}}
+// expected-note@-2 {{Loop condition is false. Execution continues on line 43}}
+arr[i] = 0;
+  int x = getInt();
+  int n = getIndex(x); // expected-note {{Calling 'getIndex'}}
+   // expected-note@-1 {{Returning from 'getIndex'}}
+   // expected-note@-2 {{'n' initialized to 3}}
+  consume(arr[n]);
+  // expected-note@-1 {{1st function call argument is an uninitialized value}}
+  // expected-warning@-2{{1st function call argument is an uninitialized value}}
+}
+} 

[PATCH] D63009: [OpenMP] Add target task alloc function with device ID

[Alexandre Eichenberger via Phabricator via cfe-commits]

AlexEichenberger added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:610
+  // size_t sizeof_shareds, kmp_routine_entry_t *task_entry,
+  // size_t device_id);
+  OMPRTL__kmpc_omp_target_task_alloc,

device_id is int64_t



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:1928
+CGM.SizeTy, CGM.SizeTy, KmpRoutineEntryPtrTy,
+CGM.SizeTy};
+// Return void * and then cast to particular kmp_task_t type.

device_id must be a int64_t type, not a machine dependent quantity.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63009



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

[PATCH] D63072: [clang] Fixing incorrect implicit deduction guides (PR41549)

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp:503
+
+umm<> m(1);
+

Should this be using CTAD?


Repository:
  rC Clang

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

https://reviews.llvm.org/D63072



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

[PATCH] D63062: [clang-format] Added New Style Rule: BitFieldDeclsOnSeparateLines

[Manikishan Ghantasala via Phabricator via cfe-commits]

Manikishan updated this revision to Diff 203833.
Manikishan added a subscriber: mgorny.

Repository:
  rC Clang

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

https://reviews.llvm.org/D63062

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

Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2916,6 +2916,9 @@
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
  const FormatToken &Right) {
   const FormatToken &Left = *Right.Previous;
+  if (Right.Previous->is(tok::comma) && Style.BitFieldDeclsOnSeparateLines &&
+  Right.is(tok::identifier) && (Right.Next->is(tok::colon)))
+  return true;
   if (Right.NewlinesBefore > 1 && Style.MaxEmptyLinesToKeep > 0)
 return true;
 
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -451,6 +451,7 @@
 IO.mapOptional("JavaScriptWrapImports", Style.JavaScriptWrapImports);
 IO.mapOptional("KeepEmptyLinesAtTheStartOfBlocks",
Style.KeepEmptyLinesAtTheStartOfBlocks);
+IO.mapOptional("BitFieldDeclsOnSeparateLines", Style.BitFieldDeclsOnSeparateLines);
 IO.mapOptional("MacroBlockBegin", Style.MacroBlockBegin);
 IO.mapOptional("MacroBlockEnd", Style.MacroBlockEnd);
 IO.mapOptional("MaxEmptyLinesToKeep", Style.MaxEmptyLinesToKeep);
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -278,6 +278,10 @@
   const FormatToken &Current = *State.NextToken;
   const FormatToken &Previous = *Current.Previous;
   assert(&Previous == Current.Previous);
+  if(Previous.is(tok::comma) && Style.BitFieldDeclsOnSeparateLines && Current.is(tok::identifier)){
+  if(Current.Next->is(tok::colon))
+return true;
+  }
   if (!Current.CanBreakBefore && !(State.Stack.back().BreakBeforeClosingBrace &&
Current.closesBlockOrBlockTypeList(Style)))
 return false;
@@ -329,6 +333,11 @@
 bool ContinuationIndenter::mustBreak(const LineState &State) {
   const FormatToken &Current = *State.NextToken;
   const FormatToken &Previous = *Current.Previous;
+  if(Previous.is(tok::comma) && Style.BitFieldDeclsOnSeparateLines && Current.is(tok::identifier)){
+  if(Current.Next->is(tok::colon)){
+return true;
+  }
+  }
   if (Current.MustBreakBefore || Current.is(TT_InlineASMColon))
 return true;
   if (State.Stack.back().BreakBeforeClosingBrace &&
@@ -541,7 +550,7 @@
  unsigned ExtraSpaces) {
   FormatToken &Current = *State.NextToken;
   const FormatToken &Previous = *State.NextToken->Previous;
-  if (Current.is(tok::equal) &&
+  if (Current.isOneOf(tok::equal, tok::colon) &&
   (State.Line->First->is(tok::kw_for) || Current.NestingLevel == 0) &&
   State.Stack.back().VariablePos == 0) {
 State.Stack.back().VariablePos = State.Column;
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -101,6 +101,16 @@
   /// \endcode
   bool AlignConsecutiveDeclarations;
 
+  /// If ``true``, Linesup Bitfield Declarations.
+  /// This will lineup Bitfield declarations on consecutive lines. This
+  /// will result in formatting like
+  /// \code
+  ///  unsigned int  baz : 1, /* Bitfield; line up entries if desire*/
+  ///fuz : 5,
+  ///zap : 2;
+  /// \endcode
+  bool BitFieldDeclsOnSeparateLines;
+
   /// Different styles for aligning escaped newlines.
   enum EscapedNewlineAlignmentStyle {
 /// Don't align escaped newlines.
@@ -1950,6 +1960,7 @@
JavaScriptWrapImports == R.JavaScriptWrapImports &&
KeepEmptyLinesAtTheStartOfBlocks ==
R.KeepEmptyLinesAtTheStartOfBlocks &&
+   BitFieldDeclsOnSeparateLines == R.BitFieldDeclsOnSeparateLines &&
MacroBlockBegin == R.MacroBlockBegin &&
MacroBlockEnd == R.MacroBlockEnd &&
MaxEmptyLinesToKeep == R.MaxEmptyLinesToKeep &&
Index: docs/ClangFormatStyleOptions.rst
===
--- docs/ClangFormatStyleOptions.rst
+++ docs/ClangFormatStyleOptions.rst
@@ -191,6 +191,18 @@
   argument1, argument2);
 
 
+**BitFieldDeclsOnSeparateLines** (``bool``)
+  If ``true``, Align Bitfield Declarations on seperate lines.
+
+  This will align Bitfield declarations on consecutive lines. This
+  will result in formatting like:
+
+  .. code-block:: c++
+
+unsigned int  baz : 1,
+  fuz : 5,
+  

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8729
+/// \code
+/// void .omp_mapper...(void *rt_mapper_handle,
+///   void *base, void *begin,

This function looks like the universal one, regardless of the type 
`` specifics. Do we really need to generate it for each particular 
type and mapper? Or we could use the same function for all types/mappers?


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

https://reviews.llvm.org/D59474



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

[PATCH] D63009: [OpenMP] Add target task alloc function with device ID

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

This patches does not add any functionality. I thought, it was published by an 
accident. The patch should be abandoned or reworked to add a functional part of 
the code.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63009



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

[PATCH] D63062: [clang-format] Added New Style Rule: BitFieldDeclsOnSeparateLines

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

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



Comment at: docs/ClangFormatStyleOptions.rst:195
+**BitFieldDeclsOnSeparateLines** (``bool``)
+  If ``true``, Align Bitfield Declarations on seperate lines.
+

'separate'



Comment at: include/clang/Format/Format.h:104
 
+  /// If ``true``, Linesup Bitfield Declarations.
+  /// This will lineup Bitfield declarations on consecutive lines. This

Also update this to match docs.



Comment at: lib/Format/ContinuationIndenter.cpp:281
   assert(&Previous == Current.Previous);
+  if(Previous.is(tok::comma) && Style.BitFieldDeclsOnSeparateLines && 
Current.is(tok::identifier)){
+  if(Current.Next->is(tok::colon))

Space after 'if' and before '{'. Also below.



Comment at: lib/Format/TokenAnnotator.cpp:2921
+  Right.is(tok::identifier) && (Right.Next->is(tok::colon)))
+  return true;
   if (Right.NewlinesBefore > 1 && Style.MaxEmptyLinesToKeep > 0)

Misindent.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63062



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

[PATCH] D63081: [WebAssembly] Cleanup toolchain test files. NFC.

[Sam Clegg via Phabricator via cfe-commits]

sbc100 created this revision.
Herald added subscribers: cfe-commits, jfb, sunfish, aheejin, jgravelle-google, 
dschuff.
Herald added a project: clang.
sbc100 added a reviewer: dschuff.
Herald added a subscriber: ormris.

Split up long lines to improve test readability.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63081

Files:
  clang/test/Driver/wasm-toolchain.c
  clang/test/Driver/wasm-toolchain.cpp

Index: clang/test/Driver/wasm-toolchain.cpp
===
--- clang/test/Driver/wasm-toolchain.cpp
+++ clang/test/Driver/wasm-toolchain.cpp
@@ -1,40 +1,52 @@
 // A basic clang -cc1 command-line. WebAssembly is somewhat special in
 // enabling -fvisibility=hidden by default.
 
-// RUN: %clangxx %s -### -no-canonical-prefixes -target wasm32-unknown-unknown 2>&1 | FileCheck -check-prefix=CC1 %s
+// RUN: %clangxx %s -### -no-canonical-prefixes -target wasm32-unknown-unknown 2>&1 \
+// RUN:   | FileCheck -check-prefix=CC1 %s
 // CC1: clang{{.*}} "-cc1" "-triple" "wasm32-unknown-unknown" {{.*}} "-fvisibility" "hidden" {{.*}}
 
 // Ditto, but ensure that a user -fvisibility=default disables the default
 // -fvisibility=hidden.
 
-// RUN: %clangxx %s -### -target wasm32-unknown-unknown -fvisibility=default 2>&1 | FileCheck -check-prefix=FVISIBILITY_DEFAULT %s
+// RUN: %clangxx %s -### -target wasm32-unknown-unknown -fvisibility=default 2>&1 \
+// RUN:   | FileCheck -check-prefix=FVISIBILITY_DEFAULT %s
 // FVISIBILITY_DEFAULT-NOT: hidden
 
 // A basic C++ link command-line with unknown OS.
 
-// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=LINK %s
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo --stdlib=c++ %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK %s
 // LINK: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ link command-line with optimization with unknown OS.
 
-// RUN: %clangxx -### -O2 -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s --stdlib=c++ 2>&1 | FileCheck -check-prefix=LINK_OPT %s
+// RUN: %clangxx -### -O2 -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s --stdlib=c++ 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_OPT %s
 // LINK_OPT: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_OPT: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ link command-line with known OS.
 
-// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=LINK_KNOWN %s
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo --stdlib=c++ %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_KNOWN %s
 // LINK_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ link command-line with optimization with known OS.
 
-// RUN: %clangxx -### -O2 -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo %s --stdlib=c++ 2>&1 | FileCheck -check-prefix=LINK_OPT_KNOWN %s
+// RUN: %clangxx -### -O2 -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo %s --stdlib=c++ 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_OPT_KNOWN %s
 // LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ compile command-line with known OS.
 
-// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=COMPILE %s
-// COMPILE: clang{{.*}}" "-cc1" {{.*}} "-internal-isystem" "/foo/include/wasm32-wasi/c++/v1" "-internal-isystem" "/foo/include/c++/v1" "-internal-isystem" "/foo/include/wasm32-wasi" "-internal-isystem" "/foo/include"
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo --stdlib=c++ %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=COMPILE %s
+// COMPILE: clang{{.*}}" "-cc1"
+// COMPILE: "-isysroot" "/foo"
+// COMPILE: "-internal-isystem" "/foo/include/wasm32-wasi/c++/v1"
+// COMPILE: "-internal-isystem" "/foo/include/c++/v1"
+// COMPILE: "-internal-isystem" "/foo/include/wasm32-wasi"
+// COMPILE: "-internal-isystem" "/foo/include"
Index: clang/test/Driver/wasm-toolchain.c
===
--- clang/test/Driver/wasm-toolchain.c
+++ clang/test/Driver/wasm-toolchain.c
@@ -1,51 +1,62 @@
 // A basic clang -cc1 command-line. WebAssembly is somewh

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

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

xbolva00 created this revision.
xbolva00 added reviewers: rsmith, rnk.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

I consider -Wint-in-bool-context as quite useful warning so I ported it to 
Clang.

-Wint-in-bool-context is on by default, it is part of GCC's -Wall so I make it 
same here.

Current patch passes all tests for -Wint-in-bool-context in GCC's test-suite.


Repository:
  rC Clang

https://reviews.llvm.org/D63082

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/Analysis/misc-ps.c
  test/Sema/integer-overflow.c
  test/Sema/parentheses.c
  test/Sema/parentheses.cpp
  test/Sema/warn-unreachable.c
  test/Sema/warn_int_in_bool_context.c
  test/SemaCXX/constexpr-printing.cpp
  test/SemaCXX/integer-overflow.cpp
  test/SemaCXX/nested-name-spec.cpp
  test/SemaCXX/warn-unreachable.cpp
  test/SemaCXX/warn_int_in_bool_context.cpp

Index: test/SemaCXX/warn_int_in_bool_context.cpp
===
--- test/SemaCXX/warn_int_in_bool_context.cpp
+++ test/SemaCXX/warn_int_in_bool_context.cpp
@@ -0,0 +1,170 @@
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wint-in-bool-context %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wall %s
+
+#define ONE 1
+#define TWO 2
+
+enum answer {
+  foo,
+  bar,
+  yes,
+  no,
+  unknown,
+};
+
+enum fruit {
+  orange = -3,
+  lemon,
+  banana,
+  apple,
+};
+
+void g(bool b);
+
+template 
+void h() {}
+template 
+void h() {}
+
+bool test(int a, int b, enum answer ans, enum fruit f) {
+  bool r = TWO * 7; // expected-warning {{'*' in bool context, maybe you mean '&&'?}}
+  r = 3 * 7;// expected-warning {{'*' in bool context, maybe you mean '&&'?}}
+  r = a * 7;// expected-warning {{'*' in bool context, maybe you mean '&&'?}}
+  r = a * b;// expected-warning {{'*' in bool context, maybe you mean '&&'?}}
+  g(3 * 9); // expected-warning {{'*' in bool context, maybe you mean '&&'?}}
+  h<3 * 9>();
+
+  r = TWO << 7; // expected-warning {{'<<' in bool context, maybe you mean '<'?}}
+  r = a << 7;   // expected-warning {{'<<' in bool context, maybe you mean '<'?}}
+  r = ONE << b; // expected-warning {{'<<' in bool context, maybe you mean '<'?}}
+
+  r = a ? ONE : TWO; // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+  r = a ? (1) : TWO;
+  r = a ? 3 : TWO; // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+  r = a ? -2 : 0;
+  r = a ? 3 : -2;
+  r = a ? 0 : TWO; // expected-warning {{'?:' with integer constants in boolean context}}
+  r = a ? 3 : ONE; // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+  r = a ? ONE : 0;
+  r = a ? 0 : 0;
+  r = a ? ONE : 0;
+  r = a ? ONE : ONE;
+  g(a ? 3 : 9); // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+
+  r = a ? yes   // expected-warning {{enum constant in bool context}}
+: no;   // expected-warning {{enum constant in bool context}}
+  r = ans == yes || no; // expected-warning {{enum constant in bool context}}
+  r = yes || ans == no; // expected-warning {{enum constant in bool context}}
+  r = ans == yes || ans == no;
+  r = !foo;
+  r = !bar;
+  r = !yes;// expected-warning {{enum constant in bool context}}
+  g(ans == yes || no); // expected-warning {{enum constant in bool context}}
+
+  if (8 * 4) // expected-warning {{'*' in bool context, maybe you mean '&&'?}}
+return a;
+
+  if (a * 4) // expected-warning {{'*' in bool context, maybe you mean '&&'?}}
+return a;
+
+  if (-TWO * b) // expected-warning {{'*' in bool context, maybe you mean '&&'?}}
+return a;
+
+  if (TWO << 4) // expected-warning {{'<<' in bool context, maybe you mean '<'?}}
+return a;
+
+  if (a << TWO) // expected-warning {{'<<' in bool context, maybe you mean '<'?}}
+return a;
+
+  if (ONE << b) // expected-warning {{'<<' in bool context, maybe you mean '<'?}}
+return a;
+
+  if (a ? ONE : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+return a;
+
+  if (a ? 32 : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+return a;
+
+  if (a ? 7 : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+return a;
+
+  if (ans == yes || foo)
+return a;
+
+  if (f == apple || orange) // expected-warning {{enum constant in bool context}}
+return a;
+
+  if (ans == yes || no) // expected-warning {{enum constant in bool context}}
+return a;
+
+  if (yes || ans == no) // expected-warning {{enum constant in bool context}}

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

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

xbolva00 added a subscriber: RKSimon.
xbolva00 added a comment.

Adding @RKSimon as a reviewer since he recently fixed some bugs which GCC's 
-Wint-in-bool-context caught.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63082



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

[PATCH] D63062: [clang-format] Added New Style Rule: BitFieldDeclsOnSeparateLines

[Manikishan Ghantasala via Phabricator via cfe-commits]

Manikishan updated this revision to Diff 203838.
Manikishan marked 4 inline comments as done.
Manikishan added a comment.

Added unittests and made the changes suggested by @mgorny


Repository:
  rC Clang

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

https://reviews.llvm.org/D63062

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -3656,6 +3656,19 @@
"#define A Just forcing a new line\n"
"ddd);");
 }
+TEST_F(FormatTest, AlignBitFieldDeclarationsOnConsecutiveLines){
+  FormatStyle Style = {};
+  Style.BitFieldDeclsOnSeparateLines = true;
+  verifyFormat(
+"unsigned int baz : 11,
+  aaa : 2,
+  foo : 3"
+  );
+  Style.BitFieldDeclsOnSeparateLines = false;
+  verifyFormat(
+"unsigned int baz : 11, aaa : 2, foo : 3"
+  );
+} 
 
 TEST_F(FormatTest, LineBreakingInBinaryExpressions) {
   verifyFormat(
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2916,6 +2916,9 @@
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
  const FormatToken &Right) {
   const FormatToken &Left = *Right.Previous;
+  if (Right.Previous->is(tok::comma) && Style.BitFieldDeclsOnSeparateLines &&
+  Right.is(tok::identifier) && (Right.Next->is(tok::colon)))
+return true;
   if (Right.NewlinesBefore > 1 && Style.MaxEmptyLinesToKeep > 0)
 return true;
 
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -451,6 +451,7 @@
 IO.mapOptional("JavaScriptWrapImports", Style.JavaScriptWrapImports);
 IO.mapOptional("KeepEmptyLinesAtTheStartOfBlocks",
Style.KeepEmptyLinesAtTheStartOfBlocks);
+IO.mapOptional("BitFieldDeclsOnSeparateLines", Style.BitFieldDeclsOnSeparateLines);
 IO.mapOptional("MacroBlockBegin", Style.MacroBlockBegin);
 IO.mapOptional("MacroBlockEnd", Style.MacroBlockEnd);
 IO.mapOptional("MaxEmptyLinesToKeep", Style.MaxEmptyLinesToKeep);
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -278,6 +278,10 @@
   const FormatToken &Current = *State.NextToken;
   const FormatToken &Previous = *Current.Previous;
   assert(&Previous == Current.Previous);
+  if(Previous.is(tok::comma) && Style.BitFieldDeclsOnSeparateLines && Current.is(tok::identifier)){
+  if(Current.Next->is(tok::colon))
+return true;
+  }
   if (!Current.CanBreakBefore && !(State.Stack.back().BreakBeforeClosingBrace &&
Current.closesBlockOrBlockTypeList(Style)))
 return false;
@@ -329,6 +333,11 @@
 bool ContinuationIndenter::mustBreak(const LineState &State) {
   const FormatToken &Current = *State.NextToken;
   const FormatToken &Previous = *Current.Previous;
+  if (Previous.is(tok::comma) && Style.BitFieldDeclsOnSeparateLines && Current.is(tok::identifier)) {
+  if (Current.Next->is(tok::colon)){
+return true;
+  }
+  }
   if (Current.MustBreakBefore || Current.is(TT_InlineASMColon))
 return true;
   if (State.Stack.back().BreakBeforeClosingBrace &&
@@ -541,7 +550,7 @@
  unsigned ExtraSpaces) {
   FormatToken &Current = *State.NextToken;
   const FormatToken &Previous = *State.NextToken->Previous;
-  if (Current.is(tok::equal) &&
+  if (Current.isOneOf(tok::equal, tok::colon) &&
   (State.Line->First->is(tok::kw_for) || Current.NestingLevel == 0) &&
   State.Stack.back().VariablePos == 0) {
 State.Stack.back().VariablePos = State.Column;
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -101,6 +101,16 @@
   /// \endcode
   bool AlignConsecutiveDeclarations;
 
+  /// If ``true``, Linesup Bitfield Declarations.
+  /// This will lineup Bitfield declarations on consecutive lines. This
+  /// will result in formatting like
+  /// \code
+  ///  unsigned int  baz : 1, /* Bitfield; line up entries if desire*/
+  ///fuz : 5,
+  ///zap : 2;
+  /// \endcode
+  bool BitFieldDeclsOnSeparateLines;
+
   /// Different styles for aligning escaped newlines.
   enum EscapedNewlineAlignmentStyle {
 /// Don't align escaped newlines.
@@ -1950,6 +1960,7 @@
JavaScriptWrapImports == R.JavaScriptWrapImports &&

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

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

xbolva00 updated this revision to Diff 203839.
xbolva00 added a comment.

Removed unrelated change


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

https://reviews.llvm.org/D63082

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/Analysis/misc-ps.c
  test/Sema/integer-overflow.c
  test/Sema/parentheses.c
  test/Sema/parentheses.cpp
  test/Sema/warn-unreachable.c
  test/Sema/warn_int_in_bool_context.c
  test/SemaCXX/constexpr-printing.cpp
  test/SemaCXX/integer-overflow.cpp
  test/SemaCXX/nested-name-spec.cpp
  test/SemaCXX/warn-unreachable.cpp
  test/SemaCXX/warn_int_in_bool_context.cpp

Index: test/SemaCXX/warn_int_in_bool_context.cpp
===
--- test/SemaCXX/warn_int_in_bool_context.cpp
+++ test/SemaCXX/warn_int_in_bool_context.cpp
@@ -0,0 +1,170 @@
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wint-in-bool-context %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wall %s
+
+#define ONE 1
+#define TWO 2
+
+enum answer {
+  foo,
+  bar,
+  yes,
+  no,
+  unknown,
+};
+
+enum fruit {
+  orange = -3,
+  lemon,
+  banana,
+  apple,
+};
+
+void g(bool b);
+
+template 
+void h() {}
+template 
+void h() {}
+
+bool test(int a, int b, enum answer ans, enum fruit f) {
+  bool r = TWO * 7; // expected-warning {{'*' in bool context, maybe you mean '&&'?}}
+  r = 3 * 7;// expected-warning {{'*' in bool context, maybe you mean '&&'?}}
+  r = a * 7;// expected-warning {{'*' in bool context, maybe you mean '&&'?}}
+  r = a * b;// expected-warning {{'*' in bool context, maybe you mean '&&'?}}
+  g(3 * 9); // expected-warning {{'*' in bool context, maybe you mean '&&'?}}
+  h<3 * 9>();
+
+  r = TWO << 7; // expected-warning {{'<<' in bool context, maybe you mean '<'?}}
+  r = a << 7;   // expected-warning {{'<<' in bool context, maybe you mean '<'?}}
+  r = ONE << b; // expected-warning {{'<<' in bool context, maybe you mean '<'?}}
+
+  r = a ? ONE : TWO; // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+  r = a ? (1) : TWO;
+  r = a ? 3 : TWO; // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+  r = a ? -2 : 0;
+  r = a ? 3 : -2;
+  r = a ? 0 : TWO; // expected-warning {{'?:' with integer constants in boolean context}}
+  r = a ? 3 : ONE; // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+  r = a ? ONE : 0;
+  r = a ? 0 : 0;
+  r = a ? ONE : 0;
+  r = a ? ONE : ONE;
+  g(a ? 3 : 9); // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+
+  r = a ? yes   // expected-warning {{enum constant in bool context}}
+: no;   // expected-warning {{enum constant in bool context}}
+  r = ans == yes || no; // expected-warning {{enum constant in bool context}}
+  r = yes || ans == no; // expected-warning {{enum constant in bool context}}
+  r = ans == yes || ans == no;
+  r = !foo;
+  r = !bar;
+  r = !yes;// expected-warning {{enum constant in bool context}}
+  g(ans == yes || no); // expected-warning {{enum constant in bool context}}
+
+  if (8 * 4) // expected-warning {{'*' in bool context, maybe you mean '&&'?}}
+return a;
+
+  if (a * 4) // expected-warning {{'*' in bool context, maybe you mean '&&'?}}
+return a;
+
+  if (-TWO * b) // expected-warning {{'*' in bool context, maybe you mean '&&'?}}
+return a;
+
+  if (TWO << 4) // expected-warning {{'<<' in bool context, maybe you mean '<'?}}
+return a;
+
+  if (a << TWO) // expected-warning {{'<<' in bool context, maybe you mean '<'?}}
+return a;
+
+  if (ONE << b) // expected-warning {{'<<' in bool context, maybe you mean '<'?}}
+return a;
+
+  if (a ? ONE : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+return a;
+
+  if (a ? 32 : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+return a;
+
+  if (a ? 7 : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+return a;
+
+  if (ans == yes || foo)
+return a;
+
+  if (f == apple || orange) // expected-warning {{enum constant in bool context}}
+return a;
+
+  if (ans == yes || no) // expected-warning {{enum constant in bool context}}
+return a;
+
+  if (yes || ans == no) // expected-warning {{enum constant in bool context}}
+return a;
+
+  if (r || a ? 7 : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+return a;
+
+  if (b && a ? 7 : TWO) // expected-warning {{'?:' with integer constants in 

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

[Michael Liao via Phabricator via cfe-commits]

hliao added inline comments.



Comment at: test/CodeGenCUDA/amdgpu-hip-implicit-kernarg.cu:7
+
+// CHECK-DAG: attributes #0 = { noinline nounwind optnone 
"amdgpu-implicitarg-num-bytes"="48"

For a single check, you don't need CHECK-DAG.


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

r362959 - [WebAssembly] Cleanup toolchain test files. NFC.

[Sam Clegg via cfe-commits]

Author: sbc
Date: Mon Jun 10 09:18:04 2019
New Revision: 362959

URL: http://llvm.org/viewvc/llvm-project?rev=362959&view=rev
Log:
[WebAssembly] Cleanup toolchain test files. NFC.

Summary: Split up long lines to improve test readability.

Subscribers: dschuff, jgravelle-google, aheejin, sunfish, jfb, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/test/Driver/wasm-toolchain.c
cfe/trunk/test/Driver/wasm-toolchain.cpp

Modified: cfe/trunk/test/Driver/wasm-toolchain.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/wasm-toolchain.c?rev=362959&r1=362958&r2=362959&view=diff
==
--- cfe/trunk/test/Driver/wasm-toolchain.c (original)
+++ cfe/trunk/test/Driver/wasm-toolchain.c Mon Jun 10 09:18:04 2019
@@ -1,51 +1,62 @@
 // A basic clang -cc1 command-line. WebAssembly is somewhat special in
 // enabling -fvisibility=hidden by default.
 
-// RUN: %clang %s -### -no-canonical-prefixes -target wasm32-unknown-unknown 
2>&1 | FileCheck -check-prefix=CC1 %s
+// RUN: %clang %s -### -no-canonical-prefixes -target wasm32-unknown-unknown 
2>&1 \
+// RUN:   | FileCheck -check-prefix=CC1 %s
 // CC1: clang{{.*}} "-cc1" "-triple" "wasm32-unknown-unknown" {{.*}} 
"-fvisibility" "hidden" {{.*}}
 
 // Ditto, but ensure that a user -fvisibility=default disables the default
 // -fvisibility=hidden.
 
-// RUN: %clang %s -### -target wasm32-unknown-unknown -fvisibility=default 
2>&1 | FileCheck -check-prefix=FVISIBILITY_DEFAULT %s
+// RUN: %clang %s -### -target wasm32-unknown-unknown -fvisibility=default 
2>&1 \
+// RUN:   | FileCheck -check-prefix=FVISIBILITY_DEFAULT %s
 // FVISIBILITY_DEFAULT-NOT: hidden
 
 // A basic C link command-line with unknown OS.
 
-// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown 
--sysroot=/foo %s 2>&1 | FileCheck -check-prefix=LINK %s
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown 
--sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK %s
 // LINK: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc" 
"{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C link command-line with optimization with unknown OS.
 
-// RUN: %clang -### -O2 -no-canonical-prefixes -target wasm32-unknown-unknown 
--sysroot=/foo %s 2>&1 | FileCheck -check-prefix=LINK_OPT %s
+// RUN: %clang -### -O2 -no-canonical-prefixes -target wasm32-unknown-unknown 
--sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_OPT %s
 // LINK_OPT: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_OPT: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc" 
"{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C link command-line with known OS.
 
-// RUN: %clang -### -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo 
%s 2>&1 | FileCheck -check-prefix=LINK_KNOWN %s
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo 
%s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_KNOWN %s
 // LINK_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" 
"-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C link command-line with optimization with known OS.
 
-// RUN: %clang -### -O2 -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo %s 2>&1 | FileCheck -check-prefix=LINK_OPT_KNOWN %s
+// RUN: %clang -### -O2 -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_OPT_KNOWN %s
 // LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" 
"-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C compile command-line with known OS.
 
-// RUN: %clang -### -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo 
%s 2>&1 | FileCheck -check-prefix=COMPILE %s
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo 
%s 2>&1 \
+// RUN:   | FileCheck -check-prefix=COMPILE %s
 // COMPILE: clang{{.*}}" "-cc1" {{.*}} "-internal-isystem" 
"/foo/include/wasm32-wasi" "-internal-isystem" "/foo/include"
 
 // Thread-related command line tests.
 
 // '-pthread' sets '-target-feature +atomics' and '--shared-memory'
-// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown 
--sysroot=/foo %s -fuse-ld=wasm-ld -pthread 2>&1 | FileCheck 
-check-prefix=PTHREAD %s
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown \
+// RUN:--sysroot=/foo %s -fuse-ld=wasm-ld -pthread 2>&1 \
+// RUN:  | FileCheck -check-prefix=PTHREAD %s
 // PTHREAD: clang{{.*}}" "-cc1" {{.*}} "-target-feature" "+atomics"
 // PTHREAD: wasm-ld{{.*}}" "-lpthread" "--shared-memory"
 
 // '-pthread' not allowed with '-mno-atomics'
-// RU

[PATCH] D62363: [X86] Enable intrinsics that convert float and bf16 data to each other

[Craig Topper via Phabricator via cfe-commits]

craig.topper accepted this revision.
craig.topper added a comment.

LGTM


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

https://reviews.llvm.org/D62363



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

[PATCH] D63062: [clang-format] Added New Style Rule: BitFieldDeclsOnSeparateLines

[Manikishan Ghantasala via Phabricator via cfe-commits]

Manikishan updated this revision to Diff 203841.
Manikishan added a comment.

Made some missing style modifications in the last revision


Repository:
  rC Clang

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

https://reviews.llvm.org/D63062

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -3656,6 +3656,19 @@
"#define A Just forcing a new line\n"
"ddd);");
 }
+TEST_F(FormatTest, AlignBitFieldDeclarationsOnConsecutiveLines){
+  FormatStyle Style = {};
+  Style.BitFieldDeclsOnSeparateLines = true;
+  verifyFormat(
+"unsigned int baz : 11,
+  aaa : 2,
+  foo : 3"
+  );
+  Style.BitFieldDeclsOnSeparateLines = false;
+  verifyFormat(
+"unsigned int baz : 11, aaa : 2, foo : 3"
+  );
+} 
 
 TEST_F(FormatTest, LineBreakingInBinaryExpressions) {
   verifyFormat(
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2916,6 +2916,9 @@
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
  const FormatToken &Right) {
   const FormatToken &Left = *Right.Previous;
+  if (Right.Previous->is(tok::comma) && Style.BitFieldDeclsOnSeparateLines &&
+  Right.is(tok::identifier) && (Right.Next->is(tok::colon)))
+return true;
   if (Right.NewlinesBefore > 1 && Style.MaxEmptyLinesToKeep > 0)
 return true;
 
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -451,6 +451,7 @@
 IO.mapOptional("JavaScriptWrapImports", Style.JavaScriptWrapImports);
 IO.mapOptional("KeepEmptyLinesAtTheStartOfBlocks",
Style.KeepEmptyLinesAtTheStartOfBlocks);
+IO.mapOptional("BitFieldDeclsOnSeparateLines", Style.BitFieldDeclsOnSeparateLines);
 IO.mapOptional("MacroBlockBegin", Style.MacroBlockBegin);
 IO.mapOptional("MacroBlockEnd", Style.MacroBlockEnd);
 IO.mapOptional("MaxEmptyLinesToKeep", Style.MaxEmptyLinesToKeep);
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -278,6 +278,9 @@
   const FormatToken &Current = *State.NextToken;
   const FormatToken &Previous = *Current.Previous;
   assert(&Previous == Current.Previous);
+  if (Previous.is(tok::comma) && Style.BitFieldDeclsOnSeparateLines &&
+  Current.is(tok::identifier) && Current.Next->is(tok::colon))
+return true;
   if (!Current.CanBreakBefore && !(State.Stack.back().BreakBeforeClosingBrace &&
Current.closesBlockOrBlockTypeList(Style)))
 return false;
@@ -329,6 +332,9 @@
 bool ContinuationIndenter::mustBreak(const LineState &State) {
   const FormatToken &Current = *State.NextToken;
   const FormatToken &Previous = *Current.Previous;
+  if (Previous.is(tok::comma) && Style.BitFieldDeclsOnSeparateLines &&
+  Current.is(tok::identifier) && Current.Next->is(tok::colon))
+return true; 
   if (Current.MustBreakBefore || Current.is(TT_InlineASMColon))
 return true;
   if (State.Stack.back().BreakBeforeClosingBrace &&
@@ -541,7 +547,7 @@
  unsigned ExtraSpaces) {
   FormatToken &Current = *State.NextToken;
   const FormatToken &Previous = *State.NextToken->Previous;
-  if (Current.is(tok::equal) &&
+  if (Current.isOneOf(tok::equal, tok::colon) &&
   (State.Line->First->is(tok::kw_for) || Current.NestingLevel == 0) &&
   State.Stack.back().VariablePos == 0) {
 State.Stack.back().VariablePos = State.Column;
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -101,6 +101,16 @@
   /// \endcode
   bool AlignConsecutiveDeclarations;
 
+  /// If ``true``, Linesup Bitfield Declarations.
+  /// This will lineup Bitfield declarations on consecutive lines. This
+  /// will result in formatting like
+  /// \code
+  ///  unsigned int  baz : 1, /* Bitfield; line up entries if desire*/
+  ///fuz : 5,
+  ///zap : 2;
+  /// \endcode
+  bool BitFieldDeclsOnSeparateLines;
+
   /// Different styles for aligning escaped newlines.
   enum EscapedNewlineAlignmentStyle {
 /// Don't align escaped newlines.
@@ -1950,6 +1960,7 @@
JavaScriptWrapImports == R.JavaScriptWrapImports &&
KeepEmptyLinesAtTheStartOfBlocks ==
R.KeepEmptyLinesAtTheS

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Lingda Li via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8729
+/// \code
+/// void .omp_mapper...(void *rt_mapper_handle,
+///   void *base, void *begin,

ABataev wrote:
> This function looks like the universal one, regardless of the type 
> `` specifics. Do we really need to generate it for each particular 
> type and mapper? Or we could use the same function for all types/mappers?
I think we need a particular mapper function for each type and mapper, because 
the code generated within the mapper function depends on what type and what 
mapper it is.


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

https://reviews.llvm.org/D59474



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

[PATCH] D62739: AMDGPU: Always emit amdgpu-flat-work-group-size

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

My concern is that this essentially forcing user to add 
amdgpu_flat_work_group_size attribute to all kernels that are executed outside 
of (128,256). Potentially this can cause lots of regressions for existing 
OpenCL apps. I am not sure if it is feasible to force all OpenCL apps to make 
this change. Should we do some tests before making this change?


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

https://reviews.llvm.org/D62739



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

[PATCH] D62739: AMDGPU: Always emit amdgpu-flat-work-group-size

[Brian Sumner via Phabricator via cfe-commits]

b-sumner added a comment.

We need to communicate with anyone generating IR to ensure this is being 
generated before we change the default.  clang is only one of those generators. 
 This change will also need to be documented in the usage document.


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

https://reviews.llvm.org/D62739



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

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8729
+/// \code
+/// void .omp_mapper...(void *rt_mapper_handle,
+///   void *base, void *begin,

lildmh wrote:
> ABataev wrote:
> > This function looks like the universal one, regardless of the type 
> > `` specifics. Do we really need to generate it for each 
> > particular type and mapper? Or we could use the same function for all 
> > types/mappers?
> I think we need a particular mapper function for each type and mapper, 
> because the code generated within the mapper function depends on what type 
> and what mapper it is.
Hmm, maybe I'm wrong but I don't see significant mapper or type-specific 
dependencies in this mapper function. It uses the pointer to type and size of 
the type, but this information can be generalized, I think. Could you point the 
lines of code that are type and mapper specific?


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

https://reviews.llvm.org/D59474



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

[PATCH] D63081: [WebAssembly] Cleanup toolchain test files. NFC.

[Phabricator via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL362959: [WebAssembly] Cleanup toolchain test files. NFC. 
(authored by sbc, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63081?vs=203836&id=203842#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63081

Files:
  cfe/trunk/test/Driver/wasm-toolchain.c
  cfe/trunk/test/Driver/wasm-toolchain.cpp

Index: cfe/trunk/test/Driver/wasm-toolchain.cpp
===
--- cfe/trunk/test/Driver/wasm-toolchain.cpp
+++ cfe/trunk/test/Driver/wasm-toolchain.cpp
@@ -1,40 +1,52 @@
 // A basic clang -cc1 command-line. WebAssembly is somewhat special in
 // enabling -fvisibility=hidden by default.
 
-// RUN: %clangxx %s -### -no-canonical-prefixes -target wasm32-unknown-unknown 2>&1 | FileCheck -check-prefix=CC1 %s
+// RUN: %clangxx %s -### -no-canonical-prefixes -target wasm32-unknown-unknown 2>&1 \
+// RUN:   | FileCheck -check-prefix=CC1 %s
 // CC1: clang{{.*}} "-cc1" "-triple" "wasm32-unknown-unknown" {{.*}} "-fvisibility" "hidden" {{.*}}
 
 // Ditto, but ensure that a user -fvisibility=default disables the default
 // -fvisibility=hidden.
 
-// RUN: %clangxx %s -### -target wasm32-unknown-unknown -fvisibility=default 2>&1 | FileCheck -check-prefix=FVISIBILITY_DEFAULT %s
+// RUN: %clangxx %s -### -target wasm32-unknown-unknown -fvisibility=default 2>&1 \
+// RUN:   | FileCheck -check-prefix=FVISIBILITY_DEFAULT %s
 // FVISIBILITY_DEFAULT-NOT: hidden
 
 // A basic C++ link command-line with unknown OS.
 
-// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=LINK %s
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo --stdlib=c++ %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK %s
 // LINK: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ link command-line with optimization with unknown OS.
 
-// RUN: %clangxx -### -O2 -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s --stdlib=c++ 2>&1 | FileCheck -check-prefix=LINK_OPT %s
+// RUN: %clangxx -### -O2 -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s --stdlib=c++ 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_OPT %s
 // LINK_OPT: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_OPT: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ link command-line with known OS.
 
-// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=LINK_KNOWN %s
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo --stdlib=c++ %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_KNOWN %s
 // LINK_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ link command-line with optimization with known OS.
 
-// RUN: %clangxx -### -O2 -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo %s --stdlib=c++ 2>&1 | FileCheck -check-prefix=LINK_OPT_KNOWN %s
+// RUN: %clangxx -### -O2 -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo %s --stdlib=c++ 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_OPT_KNOWN %s
 // LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ compile command-line with known OS.
 
-// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=COMPILE %s
-// COMPILE: clang{{.*}}" "-cc1" {{.*}} "-internal-isystem" "/foo/include/wasm32-wasi/c++/v1" "-internal-isystem" "/foo/include/c++/v1" "-internal-isystem" "/foo/include/wasm32-wasi" "-internal-isystem" "/foo/include"
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo --stdlib=c++ %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=COMPILE %s
+// COMPILE: clang{{.*}}" "-cc1"
+// COMPILE: "-isysroot" "/foo"
+// COMPILE: "-internal-isystem" "/foo/include/wasm32-wasi/c++/v1"
+// COMPILE: "-internal-isystem" "/foo/include/c++/v1"
+// COMPILE: "-internal-isystem" "/foo/include/wasm32-wasi"
+// COMPILE: "-internal-isystem" "/foo/include"
Index: cfe/trunk/test/Driver/wasm-toolchai

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Lingda Li via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8729
+/// \code
+/// void .omp_mapper...(void *rt_mapper_handle,
+///   void *base, void *begin,

ABataev wrote:
> lildmh wrote:
> > ABataev wrote:
> > > This function looks like the universal one, regardless of the type 
> > > `` specifics. Do we really need to generate it for each 
> > > particular type and mapper? Or we could use the same function for all 
> > > types/mappers?
> > I think we need a particular mapper function for each type and mapper, 
> > because the code generated within the mapper function depends on what type 
> > and what mapper it is.
> Hmm, maybe I'm wrong but I don't see significant mapper or type-specific 
> dependencies in this mapper function. It uses the pointer to type and size of 
> the type, but this information can be generalized, I think. Could you point 
> the lines of code that are type and mapper specific?
Code between line 8857-8965 is type and mapper specific. For instance, 
`generateAllInforForMapper` depends on the map clauses associated with the 
mapper and the internal structure of struct/class type, and generates 
difference code as a result. `BasePointers.size()` also depends on the above 
things.


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

https://reviews.llvm.org/D59474



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

r362965 - [X86] Attempt to make the Intel core CPU inheritance a little more readable and maintainable

[Craig Topper via cfe-commits]

Author: ctopper
Date: Mon Jun 10 09:59:28 2019
New Revision: 362965

URL: http://llvm.org/viewvc/llvm-project?rev=362965&view=rev
Log:
[X86] Attempt to make the Intel core CPU inheritance a little more readable and 
maintainable

The recently added cooperlake CPU has made our already ugly switch statement 
even worse. There's a CPU exclusion list around the bf16 feature in the cooper 
lake block. I worry that we'll have to keep adding new CPUs to that until bf16 
intercepts a client space CPU. We have several other exclusion lists in other 
parts of the switch due to skylakeserver, cascadelake, and cooperlake not 
having sgx. Another for cannonlake not having clwb but having all other 
features from skx.

This removes all these special ifs at the cost of some duplication of features 
and a goto. I've copied all of the skx features into either cannonlake or 
icelakeclient(for clwb). And pulled sklyakeserver, cascadelake, and cooperlake 
out of the main inheritance chain into their own chain. At the end of 
skylakeserver we merge back into the main chain at skylakeclient but below sgx. 
I think this is at least easier to follow.

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

Modified:
cfe/trunk/lib/Basic/Targets/X86.cpp

Modified: cfe/trunk/lib/Basic/Targets/X86.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.cpp?rev=362965&r1=362964&r2=362965&view=diff
==
--- cfe/trunk/lib/Basic/Targets/X86.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/X86.cpp Mon Jun 10 09:59:28 2019
@@ -138,6 +138,25 @@ bool X86TargetInfo::initFeatureMap(
 setFeatureEnabledImpl(Features, "mmx", true);
 break;
 
+  case CK_Cooperlake:
+// CPX inherits all CLX features plus AVX512BF16
+setFeatureEnabledImpl(Features, "avx512bf16", true);
+LLVM_FALLTHROUGH;
+  case CK_Cascadelake:
+// CLX inherits all SKX features plus AVX512VNNI
+setFeatureEnabledImpl(Features, "avx512vnni", true);
+LLVM_FALLTHROUGH;
+  case CK_SkylakeServer:
+setFeatureEnabledImpl(Features, "avx512f", true);
+setFeatureEnabledImpl(Features, "avx512cd", true);
+setFeatureEnabledImpl(Features, "avx512dq", true);
+setFeatureEnabledImpl(Features, "avx512bw", true);
+setFeatureEnabledImpl(Features, "avx512vl", true);
+setFeatureEnabledImpl(Features, "clwb", true);
+setFeatureEnabledImpl(Features, "pku", true);
+// SkylakeServer cores inherits all SKL features, except SGX
+goto SkylakeCommon;
+
   case CK_IcelakeServer:
 setFeatureEnabledImpl(Features, "pconfig", true);
 setFeatureEnabledImpl(Features, "wbnoinvd", true);
@@ -148,45 +167,29 @@ bool X86TargetInfo::initFeatureMap(
 setFeatureEnabledImpl(Features, "vpclmulqdq", true);
 setFeatureEnabledImpl(Features, "avx512bitalg", true);
 setFeatureEnabledImpl(Features, "avx512vbmi2", true);
+setFeatureEnabledImpl(Features, "avx512vnni", true);
 setFeatureEnabledImpl(Features, "avx512vpopcntdq", true);
 setFeatureEnabledImpl(Features, "rdpid", true);
+setFeatureEnabledImpl(Features, "clwb", true);
 LLVM_FALLTHROUGH;
   case CK_Cannonlake:
-setFeatureEnabledImpl(Features, "avx512ifma", true);
-setFeatureEnabledImpl(Features, "avx512vbmi", true);
-setFeatureEnabledImpl(Features, "sha", true);
-LLVM_FALLTHROUGH;
-  case CK_Cooperlake:
-// Cannonlake, IcelakeClient and IcelakeServer have no AVX512BF16 feature
-if (Kind != CK_Cannonlake && Kind != CK_IcelakeClient &&
-Kind != CK_IcelakeServer)
-  // CPX inherits all CLX features plus AVX512BF16
-  setFeatureEnabledImpl(Features, "avx512bf16", true);
-LLVM_FALLTHROUGH;
-  case CK_Cascadelake:
-//Cannonlake has no VNNI feature inside while Icelake has
-if (Kind != CK_Cannonlake)
-  // CLK inherits all SKX features plus AVX512_VNNI
-  setFeatureEnabledImpl(Features, "avx512vnni", true);
-LLVM_FALLTHROUGH;
-  case CK_SkylakeServer:
 setFeatureEnabledImpl(Features, "avx512f", true);
 setFeatureEnabledImpl(Features, "avx512cd", true);
 setFeatureEnabledImpl(Features, "avx512dq", true);
 setFeatureEnabledImpl(Features, "avx512bw", true);
 setFeatureEnabledImpl(Features, "avx512vl", true);
+setFeatureEnabledImpl(Features, "avx512ifma", true);
+setFeatureEnabledImpl(Features, "avx512vbmi", true);
 setFeatureEnabledImpl(Features, "pku", true);
-if (Kind != CK_Cannonlake) // CNL inherits all SKX features, except CLWB
-  setFeatureEnabledImpl(Features, "clwb", true);
+setFeatureEnabledImpl(Features, "sha", true);
 LLVM_FALLTHROUGH;
   case CK_SkylakeClient:
+setFeatureEnabledImpl(Features, "sgx", true);
+// SkylakeServer cores inherits all SKL features, except SGX
+SkylakeCommon:
 setFeatureEnabledImpl(Features, "xsavec", true);
 setFeatureEnabledImpl(Features, "xsaves", true);
 setFeatureEnabledImpl(Features, "mpx", true);
-if (Kind != CK

[PATCH] D63018: [X86] Attempt to make the Intel core CPU inheritance a little more readable and maintainable

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362965: [X86] Attempt to make the Intel core CPU inheritance 
a little more readable and… (authored by ctopper, committed by ).
Herald added a project: LLVM.

Changed prior to commit:
  https://reviews.llvm.org/D63018?vs=203597&id=203852#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63018

Files:
  cfe/trunk/lib/Basic/Targets/X86.cpp


Index: cfe/trunk/lib/Basic/Targets/X86.cpp
===
--- cfe/trunk/lib/Basic/Targets/X86.cpp
+++ cfe/trunk/lib/Basic/Targets/X86.cpp
@@ -138,6 +138,25 @@
 setFeatureEnabledImpl(Features, "mmx", true);
 break;
 
+  case CK_Cooperlake:
+// CPX inherits all CLX features plus AVX512BF16
+setFeatureEnabledImpl(Features, "avx512bf16", true);
+LLVM_FALLTHROUGH;
+  case CK_Cascadelake:
+// CLX inherits all SKX features plus AVX512VNNI
+setFeatureEnabledImpl(Features, "avx512vnni", true);
+LLVM_FALLTHROUGH;
+  case CK_SkylakeServer:
+setFeatureEnabledImpl(Features, "avx512f", true);
+setFeatureEnabledImpl(Features, "avx512cd", true);
+setFeatureEnabledImpl(Features, "avx512dq", true);
+setFeatureEnabledImpl(Features, "avx512bw", true);
+setFeatureEnabledImpl(Features, "avx512vl", true);
+setFeatureEnabledImpl(Features, "clwb", true);
+setFeatureEnabledImpl(Features, "pku", true);
+// SkylakeServer cores inherits all SKL features, except SGX
+goto SkylakeCommon;
+
   case CK_IcelakeServer:
 setFeatureEnabledImpl(Features, "pconfig", true);
 setFeatureEnabledImpl(Features, "wbnoinvd", true);
@@ -148,45 +167,29 @@
 setFeatureEnabledImpl(Features, "vpclmulqdq", true);
 setFeatureEnabledImpl(Features, "avx512bitalg", true);
 setFeatureEnabledImpl(Features, "avx512vbmi2", true);
+setFeatureEnabledImpl(Features, "avx512vnni", true);
 setFeatureEnabledImpl(Features, "avx512vpopcntdq", true);
 setFeatureEnabledImpl(Features, "rdpid", true);
+setFeatureEnabledImpl(Features, "clwb", true);
 LLVM_FALLTHROUGH;
   case CK_Cannonlake:
-setFeatureEnabledImpl(Features, "avx512ifma", true);
-setFeatureEnabledImpl(Features, "avx512vbmi", true);
-setFeatureEnabledImpl(Features, "sha", true);
-LLVM_FALLTHROUGH;
-  case CK_Cooperlake:
-// Cannonlake, IcelakeClient and IcelakeServer have no AVX512BF16 feature
-if (Kind != CK_Cannonlake && Kind != CK_IcelakeClient &&
-Kind != CK_IcelakeServer)
-  // CPX inherits all CLX features plus AVX512BF16
-  setFeatureEnabledImpl(Features, "avx512bf16", true);
-LLVM_FALLTHROUGH;
-  case CK_Cascadelake:
-//Cannonlake has no VNNI feature inside while Icelake has
-if (Kind != CK_Cannonlake)
-  // CLK inherits all SKX features plus AVX512_VNNI
-  setFeatureEnabledImpl(Features, "avx512vnni", true);
-LLVM_FALLTHROUGH;
-  case CK_SkylakeServer:
 setFeatureEnabledImpl(Features, "avx512f", true);
 setFeatureEnabledImpl(Features, "avx512cd", true);
 setFeatureEnabledImpl(Features, "avx512dq", true);
 setFeatureEnabledImpl(Features, "avx512bw", true);
 setFeatureEnabledImpl(Features, "avx512vl", true);
+setFeatureEnabledImpl(Features, "avx512ifma", true);
+setFeatureEnabledImpl(Features, "avx512vbmi", true);
 setFeatureEnabledImpl(Features, "pku", true);
-if (Kind != CK_Cannonlake) // CNL inherits all SKX features, except CLWB
-  setFeatureEnabledImpl(Features, "clwb", true);
+setFeatureEnabledImpl(Features, "sha", true);
 LLVM_FALLTHROUGH;
   case CK_SkylakeClient:
+setFeatureEnabledImpl(Features, "sgx", true);
+// SkylakeServer cores inherits all SKL features, except SGX
+SkylakeCommon:
 setFeatureEnabledImpl(Features, "xsavec", true);
 setFeatureEnabledImpl(Features, "xsaves", true);
 setFeatureEnabledImpl(Features, "mpx", true);
-if (Kind != CK_SkylakeServer && Kind != CK_Cascadelake &&
-Kind != CK_Cooperlake)
-  // SKX/CLX/CPX inherits all SKL features, except SGX
-  setFeatureEnabledImpl(Features, "sgx", true);
 setFeatureEnabledImpl(Features, "clflushopt", true);
 setFeatureEnabledImpl(Features, "aes", true);
 LLVM_FALLTHROUGH;


Index: cfe/trunk/lib/Basic/Targets/X86.cpp
===
--- cfe/trunk/lib/Basic/Targets/X86.cpp
+++ cfe/trunk/lib/Basic/Targets/X86.cpp
@@ -138,6 +138,25 @@
 setFeatureEnabledImpl(Features, "mmx", true);
 break;
 
+  case CK_Cooperlake:
+// CPX inherits all CLX features plus AVX512BF16
+setFeatureEnabledImpl(Features, "avx512bf16", true);
+LLVM_FALLTHROUGH;
+  case CK_Cascadelake:
+// CLX inherits all SKX features plus AVX512VNNI
+setFeatureEnabledImpl(Features, "avx512vnni", true);
+LLVM_FALLTHROUGH;
+  case CK_SkylakeServer:
+setFeatureEnabledImpl(Features, "avx512f"

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8729
+/// \code
+/// void .omp_mapper...(void *rt_mapper_handle,
+///   void *base, void *begin,

lildmh wrote:
> ABataev wrote:
> > lildmh wrote:
> > > ABataev wrote:
> > > > This function looks like the universal one, regardless of the type 
> > > > `` specifics. Do we really need to generate it for each 
> > > > particular type and mapper? Or we could use the same function for all 
> > > > types/mappers?
> > > I think we need a particular mapper function for each type and mapper, 
> > > because the code generated within the mapper function depends on what 
> > > type and what mapper it is.
> > Hmm, maybe I'm wrong but I don't see significant mapper or type-specific 
> > dependencies in this mapper function. It uses the pointer to type and size 
> > of the type, but this information can be generalized, I think. Could you 
> > point the lines of code that are type and mapper specific?
> Code between line 8857-8965 is type and mapper specific. For instance, 
> `generateAllInforForMapper` depends on the map clauses associated with the 
> mapper and the internal structure of struct/class type, and generates 
> difference code as a result. `BasePointers.size()` also depends on the above 
> things.
Most of these data can be passed as parameters to the function. It would be 
good, if we could move this function to the libomptaret library and reduce the 
number of changes (and, thus, complexity) of the compiler itself. It is always 
easier to review and to maintain the source code written in C/C++ rather than 
the changes in the compiler codegen. Plus, it may reduce the size of the final 
code significantly, I assume. I would appreciate it if you would try to move 
this function to libomptarget.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8857
+  MappableExprsHandler MEHandler(*D, MapperCGF);
+  MEHandler.generateAllInfoForMapper(BasePointers, Pointers, Sizes, MapTypes);
+

These data can be passed as parameters to the function, no?


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

https://reviews.llvm.org/D59474



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

[PATCH] D61027: Fix crash on switch conditions of non-integer types in templates

[Elizabeth Andrews via Phabricator via cfe-commits]

eandrews updated this revision to Diff 203854.
eandrews edited the summary of this revision.
eandrews added a reviewer: rsmith.

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

https://reviews.llvm.org/D61027

Files:
  lib/AST/Expr.cpp
  lib/Sema/SemaChecking.cpp
  test/SemaTemplate/dependent-names.cpp
  test/SemaTemplate/enum-argument.cpp
  test/SemaTemplate/member-access-expr.cpp
  test/SemaTemplate/non-integral-switch-cond.cpp


Index: test/SemaTemplate/non-integral-switch-cond.cpp
===
--- /dev/null
+++ test/SemaTemplate/non-integral-switch-cond.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct NOT_AN_INTEGRAL_TYPE {};
+
+template 
+struct foo {
+  NOT_AN_INTEGRAL_TYPE Bad;
+  void run() {
+switch (Bad) { // expected-error {{statement requires expression of 
integer type ('NOT_AN_INTEGRAL_TYPE' invalid)}}
+case 0:
+  break;
+}
+  }
+};
Index: test/SemaTemplate/member-access-expr.cpp
===
--- test/SemaTemplate/member-access-expr.cpp
+++ test/SemaTemplate/member-access-expr.cpp
@@ -156,7 +156,7 @@
 void get(B **ptr) {
   // It's okay if at some point we figure out how to diagnose this
   // at instantiation time.
-  *ptr = field;
+  *ptr = field; // expected-error {{assigning to 'test6::B *' from 
incompatible type 'test6::A *}}
 }
   };
 }
Index: test/SemaTemplate/enum-argument.cpp
===
--- test/SemaTemplate/enum-argument.cpp
+++ test/SemaTemplate/enum-argument.cpp
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
-// expected-no-diagnostics
 
 enum Enum { val = 1 };
 template  struct C {
@@ -31,7 +30,7 @@
 unsigned long long bitfield : e0;
 
 void f(int j) {
-  bitfield + j;
+  bitfield + j; // expected-warning {{expression result unused}}
 }
   };
 }
Index: test/SemaTemplate/dependent-names.cpp
===
--- test/SemaTemplate/dependent-names.cpp
+++ test/SemaTemplate/dependent-names.cpp
@@ -273,9 +273,6 @@
   }
   int e[10];
 };
-void g() {
-  S().f(); // expected-note {{here}}
-}
   }
 
   namespace A2 {
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -14125,6 +14125,8 @@
   bool AnyIsPacked = false;
   do {
 QualType BaseType = ME->getBase()->getType();
+if (BaseType->isDependentType())
+  return;
 if (ME->isArrow())
   BaseType = BaseType->getPointeeType();
 RecordDecl *RD = BaseType->getAs()->getDecl();
Index: lib/AST/Expr.cpp
===
--- lib/AST/Expr.cpp
+++ lib/AST/Expr.cpp
@@ -1560,6 +1560,13 @@
   MemberExpr *E = new (Mem)
   MemberExpr(base, isarrow, OperatorLoc, memberdecl, nameinfo, ty, vk, ok);
 
+  if (!isa(memberdecl)) {
+DeclContext *DC = memberdecl->getDeclContext();
+CXXRecordDecl *RD = dyn_cast(DC);
+if (RD && RD->isDependentContext() && RD->isCurrentInstantiation(DC))
+  E->setTypeDependent(ty->isDependentType());
+  }
+
   if (hasQualOrFound) {
 // FIXME: Wrong. We should be looking at the member declaration we found.
 if (QualifierLoc && QualifierLoc.getNestedNameSpecifier()->isDependent()) {


Index: test/SemaTemplate/non-integral-switch-cond.cpp
===
--- /dev/null
+++ test/SemaTemplate/non-integral-switch-cond.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct NOT_AN_INTEGRAL_TYPE {};
+
+template 
+struct foo {
+  NOT_AN_INTEGRAL_TYPE Bad;
+  void run() {
+switch (Bad) { // expected-error {{statement requires expression of integer type ('NOT_AN_INTEGRAL_TYPE' invalid)}}
+case 0:
+  break;
+}
+  }
+};
Index: test/SemaTemplate/member-access-expr.cpp
===
--- test/SemaTemplate/member-access-expr.cpp
+++ test/SemaTemplate/member-access-expr.cpp
@@ -156,7 +156,7 @@
 void get(B **ptr) {
   // It's okay if at some point we figure out how to diagnose this
   // at instantiation time.
-  *ptr = field;
+  *ptr = field; // expected-error {{assigning to 'test6::B *' from incompatible type 'test6::A *}}
 }
   };
 }
Index: test/SemaTemplate/enum-argument.cpp
===
--- test/SemaTemplate/enum-argument.cpp
+++ test/SemaTemplate/enum-argument.cpp
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
-// expected-no-diagnostics
 
 enum Enum { val = 1 };
 template  struct C {
@@ -31,7 +30,7 @@
 unsigned long long bitfield : e0;
 
 void f(int j) {
-  bitfield + j;
+  bitfield + j; // expected-warning {{expression result unused

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Lingda Li via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8729
+/// \code
+/// void .omp_mapper...(void *rt_mapper_handle,
+///   void *base, void *begin,

ABataev wrote:
> lildmh wrote:
> > ABataev wrote:
> > > lildmh wrote:
> > > > ABataev wrote:
> > > > > This function looks like the universal one, regardless of the type 
> > > > > `` specifics. Do we really need to generate it for each 
> > > > > particular type and mapper? Or we could use the same function for all 
> > > > > types/mappers?
> > > > I think we need a particular mapper function for each type and mapper, 
> > > > because the code generated within the mapper function depends on what 
> > > > type and what mapper it is.
> > > Hmm, maybe I'm wrong but I don't see significant mapper or type-specific 
> > > dependencies in this mapper function. It uses the pointer to type and 
> > > size of the type, but this information can be generalized, I think. Could 
> > > you point the lines of code that are type and mapper specific?
> > Code between line 8857-8965 is type and mapper specific. For instance, 
> > `generateAllInforForMapper` depends on the map clauses associated with the 
> > mapper and the internal structure of struct/class type, and generates 
> > difference code as a result. `BasePointers.size()` also depends on the 
> > above things.
> Most of these data can be passed as parameters to the function. It would be 
> good, if we could move this function to the libomptaret library and reduce 
> the number of changes (and, thus, complexity) of the compiler itself. It is 
> always easier to review and to maintain the source code written in C/C++ 
> rather than the changes in the compiler codegen. Plus, it may reduce the size 
> of the final code significantly, I assume. I would appreciate it if you would 
> try to move this function to libomptarget.
Hi Alexey, I don't think libomptarget can do this efficiently. For example,

```
class C {
  int a;
  double *b
}

#pragma omp declare mapper(C c) map(c.a, c.b[0:1])
```

The codegen can directly know there are 2 components (c.a, c.b[0]) in this 
mapper function (3 actually when we count the pointer), and it can also know 
the size, starting address, map type, etc. about these components. Passing all 
these information to libomptarget seems to be a bad idea. Or did I get your 
idea wrong?



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

https://reviews.llvm.org/D59474



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

[PATCH] D62962: Clang implementation of sizeless types

[JF Bastien via Phabricator via cfe-commits]

jfb added inline comments.



Comment at: test/Sema/sizeless-1.c:66
+  _Static_assert(__atomic_always_lock_free(1, &local_int8) == 
__atomic_always_lock_free(1, incomplete_ptr), "");
+  _Static_assert(__atomic_always_lock_free(2, &local_int8) == 
__atomic_always_lock_free(2, incomplete_ptr), "");
+

I expect sizeless types are never lock free.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62962



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

[PATCH] D62960: SVE opaque type for C intrinsics demo

[JF Bastien via Phabricator via cfe-commits]

jfb added subscribers: rjmccall, jfb.
jfb added a comment.

Tests?




Comment at: lib/AST/ItaniumMangle.cpp:2680
+break;
+#include "clang/Basic/AArch64SVEACLETypes.def"
   }

@rjmccall you probably should review this part.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62960



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

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8729
+/// \code
+/// void .omp_mapper...(void *rt_mapper_handle,
+///   void *base, void *begin,

lildmh wrote:
> ABataev wrote:
> > lildmh wrote:
> > > ABataev wrote:
> > > > lildmh wrote:
> > > > > ABataev wrote:
> > > > > > This function looks like the universal one, regardless of the type 
> > > > > > `` specifics. Do we really need to generate it for each 
> > > > > > particular type and mapper? Or we could use the same function for 
> > > > > > all types/mappers?
> > > > > I think we need a particular mapper function for each type and 
> > > > > mapper, because the code generated within the mapper function depends 
> > > > > on what type and what mapper it is.
> > > > Hmm, maybe I'm wrong but I don't see significant mapper or 
> > > > type-specific dependencies in this mapper function. It uses the pointer 
> > > > to type and size of the type, but this information can be generalized, 
> > > > I think. Could you point the lines of code that are type and mapper 
> > > > specific?
> > > Code between line 8857-8965 is type and mapper specific. For instance, 
> > > `generateAllInforForMapper` depends on the map clauses associated with 
> > > the mapper and the internal structure of struct/class type, and generates 
> > > difference code as a result. `BasePointers.size()` also depends on the 
> > > above things.
> > Most of these data can be passed as parameters to the function. It would be 
> > good, if we could move this function to the libomptaret library and reduce 
> > the number of changes (and, thus, complexity) of the compiler itself. It is 
> > always easier to review and to maintain the source code written in C/C++ 
> > rather than the changes in the compiler codegen. Plus, it may reduce the 
> > size of the final code significantly, I assume. I would appreciate it if 
> > you would try to move this function to libomptarget.
> Hi Alexey, I don't think libomptarget can do this efficiently. For example,
> 
> ```
> class C {
>   int a;
>   double *b
> }
> 
> #pragma omp declare mapper(C c) map(c.a, c.b[0:1])
> ```
> 
> The codegen can directly know there are 2 components (c.a, c.b[0]) in this 
> mapper function (3 actually when we count the pointer), and it can also know 
> the size, starting address, map type, etc. about these components. Passing 
> all these information to libomptarget seems to be a bad idea. Or did I get 
> your idea wrong?
> 
Yes, I understand this. But can we pass some additional parameters to this 
function so we don't need to generate a unique copy of almost the same function 
for all types/mappers? 


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

https://reviews.llvm.org/D59474



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

[PATCH] D63041: [PlistSupport] Produce a newline to end plist output files

[Xing Xue via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63041



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

[PATCH] D62960: SVE opaque type for C intrinsics demo

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added inline comments.



Comment at: lib/AST/ItaniumMangle.cpp:2680
+break;
+#include "clang/Basic/AArch64SVEACLETypes.def"
   }

jfb wrote:
> @rjmccall you probably should review this part.
Sorry for the drive by comment, but: All of these mangling should really be 
using the "vendor extension" production IMO:

` ::= u `

As is, these manglings intrude on the users's namespace, (i.e. if they had a 
type named `objc_selector` or something), and confuse demanglers which 
incorrectly assume these are substitutable (vendor extension builtin types are 
substitutable too though, but that should be handled here).


Repository:
  rC Clang

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

https://reviews.llvm.org/D62960



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

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Lingda Li via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8729
+/// \code
+/// void .omp_mapper...(void *rt_mapper_handle,
+///   void *base, void *begin,

ABataev wrote:
> lildmh wrote:
> > ABataev wrote:
> > > lildmh wrote:
> > > > ABataev wrote:
> > > > > lildmh wrote:
> > > > > > ABataev wrote:
> > > > > > > This function looks like the universal one, regardless of the 
> > > > > > > type `` specifics. Do we really need to generate it 
> > > > > > > for each particular type and mapper? Or we could use the same 
> > > > > > > function for all types/mappers?
> > > > > > I think we need a particular mapper function for each type and 
> > > > > > mapper, because the code generated within the mapper function 
> > > > > > depends on what type and what mapper it is.
> > > > > Hmm, maybe I'm wrong but I don't see significant mapper or 
> > > > > type-specific dependencies in this mapper function. It uses the 
> > > > > pointer to type and size of the type, but this information can be 
> > > > > generalized, I think. Could you point the lines of code that are type 
> > > > > and mapper specific?
> > > > Code between line 8857-8965 is type and mapper specific. For instance, 
> > > > `generateAllInforForMapper` depends on the map clauses associated with 
> > > > the mapper and the internal structure of struct/class type, and 
> > > > generates difference code as a result. `BasePointers.size()` also 
> > > > depends on the above things.
> > > Most of these data can be passed as parameters to the function. It would 
> > > be good, if we could move this function to the libomptaret library and 
> > > reduce the number of changes (and, thus, complexity) of the compiler 
> > > itself. It is always easier to review and to maintain the source code 
> > > written in C/C++ rather than the changes in the compiler codegen. Plus, 
> > > it may reduce the size of the final code significantly, I assume. I would 
> > > appreciate it if you would try to move this function to libomptarget.
> > Hi Alexey, I don't think libomptarget can do this efficiently. For example,
> > 
> > ```
> > class C {
> >   int a;
> >   double *b
> > }
> > 
> > #pragma omp declare mapper(C c) map(c.a, c.b[0:1])
> > ```
> > 
> > The codegen can directly know there are 2 components (c.a, c.b[0]) in this 
> > mapper function (3 actually when we count the pointer), and it can also 
> > know the size, starting address, map type, etc. about these components. 
> > Passing all these information to libomptarget seems to be a bad idea. Or 
> > did I get your idea wrong?
> > 
> Yes, I understand this. But can we pass some additional parameters to this 
> function so we don't need to generate a unique copy of almost the same 
> function for all types/mappers? 
For different types/mappers, the skeleton of mapper functions are similar 
(i.e., the things outlined in the comment here). I would say most other code is 
unique, for instance, the code to prepare parameters of call to 
`__tgt_push_mapper_component`. These code should be much more compared with the 
skeleton shown here. I cannot think of a way to reduce the code by passing more 
parameters to this function. Please let me know if you have some suggestions.


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

https://reviews.llvm.org/D59474



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

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

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: llvm/cmake/modules/AddLLVM.cmake:812
+#   llvm::PassPluginLibraryInfo ${entry_point}();
+add_custom_target(LLVM_PLUGINS)  # target used to hold global properties 
referencable from generator-expression
+function(register_llvm_extension llvm_extension entry_point)

Change this to `llvm-plugins` to match our convention and wrap it in `if (NOT 
TARGET...)` so it doesn't error if AddLLVM is included twice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446



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

[PATCH] D63062: [clang-format] Added New Style Rule: BitFieldDeclsOnSeparateLines

[Manikishan Ghantasala via Phabricator via cfe-commits]

Manikishan updated this revision to Diff 203860.
Manikishan marked 4 inline comments as done.
Manikishan added a comment.

Updated unittest


Repository:
  rC Clang

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

https://reviews.llvm.org/D63062

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -3656,6 +3656,19 @@
"#define A Just forcing a new line\n"
"ddd);");
 }
+TEST_F(FormatTest, AlignBitFieldDeclarationsOnConsecutiveLines){
+  FormatStyle Style = {};
+  Style.BitFieldDeclsOnSeparateLines = true;
+  verifyFormat(
+"unsigned int baz : 11,
+  aaa : 2,
+  foo : 3"
+  );
+  Style.BitFieldDeclsOnSeparateLines = false;
+  verifyFormat(
+"unsigned int baz : 11, aaa : 2, foo : 3"
+  );
+} 
 
 TEST_F(FormatTest, LineBreakingInBinaryExpressions) {
   verifyFormat(
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2916,6 +2916,9 @@
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
  const FormatToken &Right) {
   const FormatToken &Left = *Right.Previous;
+  if (Right.Previous->is(tok::comma) && Style.BitFieldDeclsOnSeparateLines &&
+  Right.is(tok::identifier) && (Right.Next->is(tok::colon)))
+return true;
   if (Right.NewlinesBefore > 1 && Style.MaxEmptyLinesToKeep > 0)
 return true;
 
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -451,6 +451,7 @@
 IO.mapOptional("JavaScriptWrapImports", Style.JavaScriptWrapImports);
 IO.mapOptional("KeepEmptyLinesAtTheStartOfBlocks",
Style.KeepEmptyLinesAtTheStartOfBlocks);
+IO.mapOptional("BitFieldDeclsOnSeparateLines", Style.BitFieldDeclsOnSeparateLines);
 IO.mapOptional("MacroBlockBegin", Style.MacroBlockBegin);
 IO.mapOptional("MacroBlockEnd", Style.MacroBlockEnd);
 IO.mapOptional("MaxEmptyLinesToKeep", Style.MaxEmptyLinesToKeep);
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -278,6 +278,9 @@
   const FormatToken &Current = *State.NextToken;
   const FormatToken &Previous = *Current.Previous;
   assert(&Previous == Current.Previous);
+  if (Previous.is(tok::comma) && Style.BitFieldDeclsOnSeparateLines &&
+  Current.is(tok::identifier) && Current.Next->is(tok::colon))
+return true;
   if (!Current.CanBreakBefore && !(State.Stack.back().BreakBeforeClosingBrace &&
Current.closesBlockOrBlockTypeList(Style)))
 return false;
@@ -329,6 +332,9 @@
 bool ContinuationIndenter::mustBreak(const LineState &State) {
   const FormatToken &Current = *State.NextToken;
   const FormatToken &Previous = *Current.Previous;
+  if (Previous.is(tok::comma) && Style.BitFieldDeclsOnSeparateLines &&
+  Current.is(tok::identifier) && Current.Next->is(tok::colon))
+return true; 
   if (Current.MustBreakBefore || Current.is(TT_InlineASMColon))
 return true;
   if (State.Stack.back().BreakBeforeClosingBrace &&
@@ -541,7 +547,7 @@
  unsigned ExtraSpaces) {
   FormatToken &Current = *State.NextToken;
   const FormatToken &Previous = *State.NextToken->Previous;
-  if (Current.is(tok::equal) &&
+  if (Current.isOneOf(tok::equal, tok::colon) &&
   (State.Line->First->is(tok::kw_for) || Current.NestingLevel == 0) &&
   State.Stack.back().VariablePos == 0) {
 State.Stack.back().VariablePos = State.Column;
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -101,6 +101,16 @@
   /// \endcode
   bool AlignConsecutiveDeclarations;
 
+  /// If ``true``, Linesup Bitfield Declarations.
+  /// This will lineup Bitfield declarations on consecutive lines. This
+  /// will result in formatting like
+  /// \code
+  ///  unsigned int  baz : 1, /* Bitfield; line up entries if desire*/
+  ///fuz : 5,
+  ///zap : 2;
+  /// \endcode
+  bool BitFieldDeclsOnSeparateLines;
+
   /// Different styles for aligning escaped newlines.
   enum EscapedNewlineAlignmentStyle {
 /// Don't align escaped newlines.
@@ -1950,6 +1960,7 @@
JavaScriptWrapImports == R.JavaScriptWrapImports &&
KeepEmptyLinesAtTheStartOfBlocks ==
R.KeepEmptyLinesAtT

[PATCH] D62962: Clang implementation of sizeless types

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm marked 2 inline comments as done.
rsandifo-arm added inline comments.



Comment at: test/Sema/sizeless-1.c:66
+  _Static_assert(__atomic_always_lock_free(1, &local_int8) == 
__atomic_always_lock_free(1, incomplete_ptr), "");
+  _Static_assert(__atomic_always_lock_free(2, &local_int8) == 
__atomic_always_lock_free(2, incomplete_ptr), "");
+

jfb wrote:
> I expect sizeless types are never lock free.
Yeah, that's right.  But AIUI `__atomic_always_lock_free(N, P)` asks whether an 
N-byte access at P is lock-free (where P can be null to query standard 
alignment).  So the question isn't whether sizeless types are lock-free, but 
whether an N-byte access is lock-free given the alignment guarantees of P.  For 
this line the answer would be yes if `&local_int8` was aligned to a 2-byte 
boundary.

The query isn't really that interesting for sizeless types.  The reason for 
having it is that `IntExprEvaluator::VisitBuiltinCallExpr` says that 1-byte 
accesses are lock-free if the target has lock-free accesses, whatever P happens 
to be.  But for larger N it punts when P is a pointer to incomplete type, 
because in that case it knows nothing about the alignment of P.  The test is 
enforcing this behaviour for sizeless types too.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62962



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

[PATCH] D63072: [clang] Fixing incorrect implicit deduction guides (PR41549)

[Tyker via Phabricator via cfe-commits]

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

fixed the issue.


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

https://reviews.llvm.org/D63072

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp


Index: clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
===
--- clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
+++ clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
@@ -489,6 +489,21 @@
 }
 #pragma clang diagnostic pop
 
+namespace PR41549 {
+
+template  struct umm;
+
+template 
+struct umm {
+  umm(H h = 0, P p = 0);
+};
+
+template  struct umm;
+
+umm m(1);
+
+}
+
 #else
 
 // expected-no-diagnostics
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -2052,6 +2052,12 @@
 
 void Sema::DeclareImplicitDeductionGuides(TemplateDecl *Template,
   SourceLocation Loc) {
+  if (CXXRecordDecl *DefRecord =
+  cast(Template->getTemplatedDecl())->getDefinition()) {
+TemplateDecl *DescribedTemplate = DefRecord->getDescribedClassTemplate();
+Template = DescribedTemplate ? DescribedTemplate : Template;
+  }
+
   DeclContext *DC = Template->getDeclContext();
   if (DC->isDependentContext())
 return;


Index: clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
===
--- clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
+++ clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
@@ -489,6 +489,21 @@
 }
 #pragma clang diagnostic pop
 
+namespace PR41549 {
+
+template  struct umm;
+
+template 
+struct umm {
+  umm(H h = 0, P p = 0);
+};
+
+template  struct umm;
+
+umm m(1);
+
+}
+
 #else
 
 // expected-no-diagnostics
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -2052,6 +2052,12 @@
 
 void Sema::DeclareImplicitDeductionGuides(TemplateDecl *Template,
   SourceLocation Loc) {
+  if (CXXRecordDecl *DefRecord =
+  cast(Template->getTemplatedDecl())->getDefinition()) {
+TemplateDecl *DescribedTemplate = DefRecord->getDescribedClassTemplate();
+Template = DescribedTemplate ? DescribedTemplate : Template;
+  }
+
   DeclContext *DC = Template->getDeclContext();
   if (DC->isDependentContext())
 return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62962: Clang implementation of sizeless types

[JF Bastien via Phabricator via cfe-commits]

jfb added inline comments.



Comment at: test/Sema/sizeless-1.c:66
+  _Static_assert(__atomic_always_lock_free(1, &local_int8) == 
__atomic_always_lock_free(1, incomplete_ptr), "");
+  _Static_assert(__atomic_always_lock_free(2, &local_int8) == 
__atomic_always_lock_free(2, incomplete_ptr), "");
+

rsandifo-arm wrote:
> jfb wrote:
> > I expect sizeless types are never lock free.
> Yeah, that's right.  But AIUI `__atomic_always_lock_free(N, P)` asks whether 
> an N-byte access at P is lock-free (where P can be null to query standard 
> alignment).  So the question isn't whether sizeless types are lock-free, but 
> whether an N-byte access is lock-free given the alignment guarantees of P.  
> For this line the answer would be yes if `&local_int8` was aligned to a 
> 2-byte boundary.
> 
> The query isn't really that interesting for sizeless types.  The reason for 
> having it is that `IntExprEvaluator::VisitBuiltinCallExpr` says that 1-byte 
> accesses are lock-free if the target has lock-free accesses, whatever P 
> happens to be.  But for larger N it punts when P is a pointer to incomplete 
> type, because in that case it knows nothing about the alignment of P.  The 
> test is enforcing this behaviour for sizeless types too.
Ah yes, that makes sense. Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D62962



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

[PATCH] D63048: Update __VERSION__ to remove the hardcoded 4.2.1 version

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a subscriber: dexonsmith.
rnk added a comment.

Personally, I think it's safe to do this. I believe this "4.2.1" compat thing 
was something Apple added as part of their switch from GCC, and then llvm-gcc, 
over to Clang, and I think most of the code they care about probably doesn't 
rely on `__VERSION__` anymore. In any case, we should let @dexonsmith know.

As an alternative, maybe we should just stop defining the `__VERSION__` macro. 
It's ambiguous. What is it the version of? The compiler, or the standard 
library? What gives the compiler the right to take some identifier, even in the 
implementer's namespace, and use it without putting it in their namespace by 
adding `clang` or `GNUC` to it? We already define `__clang_version__`, which 
has the same information.




Comment at: lib/Frontend/InitPreprocessor.cpp:610
+  Builder.defineMacro("__VERSION__", "\"" CLANG_VERSION_STRING
+  " Compatible " + Twine(getClangFullCPPVersion()) +
+  "\"");

sylvestre.ledru wrote:
> lebedev.ri wrote:
> > So how does the entire `__VERSION__` macro looks like now?
> ```
> #define __VERSION__ "9.0.0 Compatible Clang 9.0.0 (trunk 362877)"
> 
> ```
> instead of
> 
> ```
> #define __VERSION__ "4.2.1 Compatible Clang 9.0.0 (trunk 362877)"
> ```
> 
> 
> 
> ```
> clang -dM -E -xc - < /dev/null 
> ```
> to see it
If we're going to make this change, I don't think we need to say `$VER 
Compatible Clang $VER`, we can just say `Clang $VER (trunk $REVISION)`, so this 
can be simplified to `getClangFullCPPVersion()`. For a vendor like Apple, it 
will become `Apple Clang N.M.Q`, which is probably fine. @dexonsmith 


Repository:
  rC Clang

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

https://reviews.llvm.org/D63048



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

[PATCH] D63085: Provide a fix-it hint for -Wswitch, which adds missing cases. If there are >3 cases, the inserted text will contain newlines so it will not be shown in console output (but will be appl

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

Lifted the helper for spelling one scope from another from
SemaCodeComplete -> AST, to reuse it. It should be useful in other contexts too.


Repository:
  rC Clang

https://reviews.llvm.org/D63085

Files:
  include/clang/AST/DeclBase.h
  lib/AST/DeclBase.cpp
  lib/Sema/SemaCodeComplete.cpp
  lib/Sema/SemaLookup.cpp
  lib/Sema/SemaStmt.cpp
  test/FixIt/fixit-enum.cpp

Index: test/FixIt/fixit-enum.cpp
===
--- /dev/null
+++ test/FixIt/fixit-enum.cpp
@@ -0,0 +1,19 @@
+// RUN: cp %s %t.cpp
+// RUN: not %clang_cc1 -fixit %t.cpp -Werror
+// RUN: %clang_cc1 %t.cpp -Werror
+
+namespace ns {
+inline namespace q { namespace { enum { A, B, C }; } }
+} // namespace ns
+
+namespace ns2 {
+enum class F { X, Y, Z };
+
+void test(F f, decltype(ns::A) g) {
+  switch (f) {}
+  // CHECK: YYY
+
+  switch (g) {}
+  // CHECK: ZZZ
+}
+} // namespace ns2
Index: lib/Sema/SemaStmt.cpp
===
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -1193,14 +1193,31 @@
 
   // Produce a nice diagnostic if multiple values aren't handled.
   if (!UnhandledNames.empty()) {
-DiagnosticBuilder DB = Diag(CondExpr->getExprLoc(),
-TheDefaultStmt ? diag::warn_def_missing_case
-   : diag::warn_missing_case)
-   << (int)UnhandledNames.size();
+DiagnosticBuilder DB =
+Diag(CondExpr->getExprLoc(), TheDefaultStmt
+ ? diag::warn_def_missing_case
+ : diag::warn_missing_case)
+<< (int)UnhandledNames.size();
 
 for (size_t I = 0, E = std::min(UnhandledNames.size(), (size_t)3);
  I != E; ++I)
   DB << UnhandledNames[I];
+
+std::string NewCases;
+llvm::raw_string_ostream NewCasesOS(NewCases);
+// Include \n to separate new cases added if there are many.
+// This suppresses printing the (long) insert text.
+char Sep = UnhandledNames.size() > 3 ? '\n' : ' ';
+const NestedNameSpecifier *Qual =
+CurContext->getQualifierFor(ED->isScoped() ? ED : ED->getParent());
+for (const auto& Name : UnhandledNames) {
+  NewCasesOS << "case ";
+  if (Qual)
+Qual->print(NewCasesOS, Context.getPrintingPolicy());
+  NewCasesOS << Name << ':' << Sep;
+}
+NewCasesOS << "break;";
+DB << FixItHint::CreateInsertion(SS->getEndLoc(), NewCasesOS.str());
   }
 
   if (!hasCasesNotInSwitch)
Index: lib/Sema/SemaLookup.cpp
===
--- lib/Sema/SemaLookup.cpp
+++ lib/Sema/SemaLookup.cpp
@@ -20,6 +20,7 @@
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/NestedNameSpecifier.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Lex/HeaderSearch.h"
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -648,50 +648,6 @@
   return iterator(DeclOrVector.get()->end());
 }
 
-/// Compute the qualification required to get from the current context
-/// (\p CurContext) to the target context (\p TargetContext).
-///
-/// \param Context the AST context in which the qualification will be used.
-///
-/// \param CurContext the context where an entity is being named, which is
-/// typically based on the current scope.
-///
-/// \param TargetContext the context in which the named entity actually
-/// resides.
-///
-/// \returns a nested name specifier that refers into the target context, or
-/// NULL if no qualification is needed.
-static NestedNameSpecifier *
-getRequiredQualification(ASTContext &Context, const DeclContext *CurContext,
- const DeclContext *TargetContext) {
-  SmallVector TargetParents;
-
-  for (const DeclContext *CommonAncestor = TargetContext;
-   CommonAncestor && !CommonAncestor->Encloses(CurContext);
-   CommonAncestor = CommonAncestor->getLookupParent()) {
-if (CommonAncestor->isTransparentContext() ||
-CommonAncestor->isFunctionOrMethod())
-  continue;
-
-TargetParents.push_back(CommonAncestor);
-  }
-
-  NestedNameSpecifier *Result = nullptr;
-  while (!TargetParents.empty()) {
-const DeclContext *Parent = TargetParents.pop_back_val();
-
-if (const auto *Namespace = dyn_cast(Parent)) {
-  if (!Namespace->getIdentifier())
-continue;
-
-  Result = NestedNameSpecifier::Create(Context, Result, Namespace);
-} else

[PATCH] D61749: [clang-tidy] initial version of readability-static-const-method

[Matthias Gehre via Phabricator via cfe-commits]

mgehre added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61749



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

[PATCH] D63086: [analyzer][NoStoreFuncVisitor][NFC] Move methods out-of-line, turn some to static functions

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus created this revision.
Szelethus added reviewers: NoQ, dcoughlin, xazax.hun, rnkovacs, Charusso, 
baloghadamsoftware.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, szepet, whisperity.

I personally found NoStoreFuncVisitor very hard to read, and since I'm planning 
to make other changes to it later on, I think this time is as good as any to 
make it somewhat more readable.

- Make several methods static functions
- Move non-trivial methods out-of-line
- Add a divider
- Turn non-obvious `auto`s into `Optional`
- clang-format affected lines


Repository:
  rC Clang

https://reviews.llvm.org/D63086

Files:
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -196,37 +196,6 @@
   return {};
 }
 
-//===--===//
-// Implementation of BugReporterVisitor.
-//===--===//
-
-std::shared_ptr
-BugReporterVisitor::getEndPath(BugReporterContext &,
-   const ExplodedNode *, BugReport &) {
-  return nullptr;
-}
-
-void
-BugReporterVisitor::finalizeVisitor(BugReporterContext &,
-const ExplodedNode *, BugReport &) {}
-
-std::shared_ptr BugReporterVisitor::getDefaultEndPath(
-BugReporterContext &BRC, const ExplodedNode *EndPathNode, BugReport &BR) {
-  PathDiagnosticLocation L =
-PathDiagnosticLocation::createEndOfPath(EndPathNode,BRC.getSourceManager());
-
-  const auto &Ranges = BR.getRanges();
-
-  // Only add the statement itself as a range if we didn't specify any
-  // special ranges for this report.
-  auto P = std::make_shared(
-  L, BR.getDescription(), Ranges.begin() == Ranges.end());
-  for (SourceRange Range : Ranges)
-P->addRange(Range);
-
-  return P;
-}
-
 /// \return name of the macro inside the location \p Loc.
 static StringRef getMacroName(SourceLocation Loc,
 BugReporterContext &BRC) {
@@ -251,36 +220,70 @@
 }
 
 /// \return Whether \c RegionOfInterest was modified at \p N,
-/// where \p ReturnState is a state associated with the return
-/// from the current frame.
-static bool wasRegionOfInterestModifiedAt(
-const SubRegion *RegionOfInterest,
-const ExplodedNode *N,
-SVal ValueAfter) {
+/// where \p ValueAfter is \c RegionOfInterest's value at the end of the
+/// stack frame.
+static bool wasRegionOfInterestModifiedAt(const SubRegion *RegionOfInterest,
+  const ExplodedNode *N,
+  SVal ValueAfter) {
   ProgramStateRef State = N->getState();
   ProgramStateManager &Mgr = N->getState()->getStateManager();
 
-  if (!N->getLocationAs()
-  && !N->getLocationAs()
-  && !N->getLocationAs())
+  if (!N->getLocationAs() && !N->getLocationAs() &&
+  !N->getLocationAs())
 return false;
 
   // Writing into region of interest.
   if (auto PS = N->getLocationAs())
 if (auto *BO = PS->getStmtAs())
   if (BO->isAssignmentOp() && RegionOfInterest->isSubRegionOf(
-N->getSVal(BO->getLHS()).getAsRegion()))
+  N->getSVal(BO->getLHS()).getAsRegion()))
 return true;
 
   // SVal after the state is possibly different.
   SVal ValueAtN = N->getState()->getSVal(RegionOfInterest);
-  if (!Mgr.getSValBuilder().areEqual(State, ValueAtN, ValueAfter).isConstrainedTrue() &&
+  if (!Mgr.getSValBuilder()
+   .areEqual(State, ValueAtN, ValueAfter)
+   .isConstrainedTrue() &&
   (!ValueAtN.isUndef() || !ValueAfter.isUndef()))
 return true;
 
   return false;
 }
 
+//===--===//
+// Implementation of BugReporterVisitor.
+//===--===//
+
+std::shared_ptr
+BugReporterVisitor::getEndPath(BugReporterContext &,
+   const ExplodedNode *, BugReport &) {
+  return nullptr;
+}
+
+void
+BugReporterVisitor::finalizeVisitor(BugReporterContext &,
+const ExplodedNode *, BugReport &) {}
+
+std::shared_ptr BugReporterVisitor::getDefaultEndPath(
+BugReporterContext &BRC, const ExplodedNode *EndPathNode, BugReport &BR) {
+  PathDiagnosticLocation L =
+PathDiagnosticLocation::createEndOfPath(EndPathNode,BRC.getSourceManager());
+
+  const auto &Ranges = BR.getRanges();
+
+  // Only add the statement itself as a range if we didn't specify any
+  // special ranges for this report.
+  auto P = std::make_shared(
+  L, BR.getDescription(), Ranges.begin() == Ranges.end());
+  for (SourceRange Range 

[PATCH] D63088: [clang-tidy] misc-unused-parameters: don't comment out parameter name for C code

[Matthias Gehre via Phabricator via cfe-commits]

mgehre created this revision.
mgehre added reviewers: klimek, ilya-biryukov, lebedev.ri, aaron.ballman.
Herald added a subscriber: xazax.hun.
Herald added a project: clang.

The fixit `int square(int /*num*/)` yields `error: parameter name omitted` for 
C code. Enable it only for C++ code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63088

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


Index: clang-tools-extra/test/clang-tidy/misc-unused-parameters.c
===
--- clang-tools-extra/test/clang-tidy/misc-unused-parameters.c
+++ clang-tools-extra/test/clang-tidy/misc-unused-parameters.c
@@ -4,7 +4,7 @@
 // =
 void a(int i) {;}
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: parameter 'i' is unused 
[misc-unused-parameters]
-// CHECK-FIXES: {{^}}void a(int  /*i*/) {;}{{$}}
+// CHECK-FIXES: {{^}}void a(int i) {;}{{$}}
 
 static void b(); // In C, forward declarations can leave out parameters.
 static void b(int i) {;}
Index: clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -138,16 +138,21 @@
 Indexer = llvm::make_unique(*Result.Context);
   }
 
-  // Comment out parameter name for non-local functions.
+  // Cannot remove parameter for non-local functions.
   if (Function->isExternallyVisible() ||
   !Result.SourceManager->isInMainFile(Function->getLocation()) ||
   !Indexer->getOtherRefs(Function).empty() || isOverrideMethod(Function)) {
-SourceRange RemovalRange(Param->getLocation());
-// Note: We always add a space before the '/*' to not accidentally create a
-// '*/*' for pointer types, which doesn't start a comment. clang-format 
will
-// clean this up afterwards.
-MyDiag << FixItHint::CreateReplacement(
-RemovalRange, (Twine(" /*") + Param->getName() + "*/").str());
+
+// Comment out parameter name.
+if (Result.Context->getLangOpts().CPlusPlus) {
+  SourceRange RemovalRange(Param->getLocation());
+  // Note: We always add a space before the '/*' to not accidentally create
+  // a
+  // '*/*' for pointer types, which doesn't start a comment. clang-format
+  // will clean this up afterwards.
+  MyDiag << FixItHint::CreateReplacement(
+  RemovalRange, (Twine(" /*") + Param->getName() + "*/").str());
+}
 return;
   }
 


Index: clang-tools-extra/test/clang-tidy/misc-unused-parameters.c
===
--- clang-tools-extra/test/clang-tidy/misc-unused-parameters.c
+++ clang-tools-extra/test/clang-tidy/misc-unused-parameters.c
@@ -4,7 +4,7 @@
 // =
 void a(int i) {;}
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: parameter 'i' is unused [misc-unused-parameters]
-// CHECK-FIXES: {{^}}void a(int  /*i*/) {;}{{$}}
+// CHECK-FIXES: {{^}}void a(int i) {;}{{$}}
 
 static void b(); // In C, forward declarations can leave out parameters.
 static void b(int i) {;}
Index: clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -138,16 +138,21 @@
 Indexer = llvm::make_unique(*Result.Context);
   }
 
-  // Comment out parameter name for non-local functions.
+  // Cannot remove parameter for non-local functions.
   if (Function->isExternallyVisible() ||
   !Result.SourceManager->isInMainFile(Function->getLocation()) ||
   !Indexer->getOtherRefs(Function).empty() || isOverrideMethod(Function)) {
-SourceRange RemovalRange(Param->getLocation());
-// Note: We always add a space before the '/*' to not accidentally create a
-// '*/*' for pointer types, which doesn't start a comment. clang-format will
-// clean this up afterwards.
-MyDiag << FixItHint::CreateReplacement(
-RemovalRange, (Twine(" /*") + Param->getName() + "*/").str());
+
+// Comment out parameter name.
+if (Result.Context->getLangOpts().CPlusPlus) {
+  SourceRange RemovalRange(Param->getLocation());
+  // Note: We always add a space before the '/*' to not accidentally create
+  // a
+  // '*/*' for pointer types, which doesn't start a comment. clang-format
+  // will clean this up afterwards.
+  MyDiag << FixItHint::CreateReplacement(
+  RemovalRange, (Twine(" /*") + Param->getName() + "*/").str());
+}
 return;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63088: [clang-tidy] misc-unused-parameters: don't comment out parameter name for C code

[Matthias Gehre via Phabricator via cfe-commits]

mgehre updated this revision to Diff 203873.
mgehre added a comment.

Fix rewrapped comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63088

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


Index: clang-tools-extra/test/clang-tidy/misc-unused-parameters.c
===
--- clang-tools-extra/test/clang-tidy/misc-unused-parameters.c
+++ clang-tools-extra/test/clang-tidy/misc-unused-parameters.c
@@ -4,7 +4,7 @@
 // =
 void a(int i) {;}
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: parameter 'i' is unused 
[misc-unused-parameters]
-// CHECK-FIXES: {{^}}void a(int  /*i*/) {;}{{$}}
+// CHECK-FIXES: {{^}}void a(int i) {;}{{$}}
 
 static void b(); // In C, forward declarations can leave out parameters.
 static void b(int i) {;}
Index: clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -138,16 +138,20 @@
 Indexer = llvm::make_unique(*Result.Context);
   }
 
-  // Comment out parameter name for non-local functions.
+  // Cannot remove parameter for non-local functions.
   if (Function->isExternallyVisible() ||
   !Result.SourceManager->isInMainFile(Function->getLocation()) ||
   !Indexer->getOtherRefs(Function).empty() || isOverrideMethod(Function)) {
-SourceRange RemovalRange(Param->getLocation());
-// Note: We always add a space before the '/*' to not accidentally create a
-// '*/*' for pointer types, which doesn't start a comment. clang-format 
will
-// clean this up afterwards.
-MyDiag << FixItHint::CreateReplacement(
-RemovalRange, (Twine(" /*") + Param->getName() + "*/").str());
+
+// Comment out parameter name.
+if (Result.Context->getLangOpts().CPlusPlus) {
+  SourceRange RemovalRange(Param->getLocation());
+  // Note: We always add a space before the '/*' to not accidentally create
+  // a '*/*' for pointer types, which doesn't start a comment. clang-format
+  // will clean this up afterwards.
+  MyDiag << FixItHint::CreateReplacement(
+  RemovalRange, (Twine(" /*") + Param->getName() + "*/").str());
+}
 return;
   }
 


Index: clang-tools-extra/test/clang-tidy/misc-unused-parameters.c
===
--- clang-tools-extra/test/clang-tidy/misc-unused-parameters.c
+++ clang-tools-extra/test/clang-tidy/misc-unused-parameters.c
@@ -4,7 +4,7 @@
 // =
 void a(int i) {;}
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: parameter 'i' is unused [misc-unused-parameters]
-// CHECK-FIXES: {{^}}void a(int  /*i*/) {;}{{$}}
+// CHECK-FIXES: {{^}}void a(int i) {;}{{$}}
 
 static void b(); // In C, forward declarations can leave out parameters.
 static void b(int i) {;}
Index: clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -138,16 +138,20 @@
 Indexer = llvm::make_unique(*Result.Context);
   }
 
-  // Comment out parameter name for non-local functions.
+  // Cannot remove parameter for non-local functions.
   if (Function->isExternallyVisible() ||
   !Result.SourceManager->isInMainFile(Function->getLocation()) ||
   !Indexer->getOtherRefs(Function).empty() || isOverrideMethod(Function)) {
-SourceRange RemovalRange(Param->getLocation());
-// Note: We always add a space before the '/*' to not accidentally create a
-// '*/*' for pointer types, which doesn't start a comment. clang-format will
-// clean this up afterwards.
-MyDiag << FixItHint::CreateReplacement(
-RemovalRange, (Twine(" /*") + Param->getName() + "*/").str());
+
+// Comment out parameter name.
+if (Result.Context->getLangOpts().CPlusPlus) {
+  SourceRange RemovalRange(Param->getLocation());
+  // Note: We always add a space before the '/*' to not accidentally create
+  // a '*/*' for pointer types, which doesn't start a comment. clang-format
+  // will clean this up afterwards.
+  MyDiag << FixItHint::CreateReplacement(
+  RemovalRange, (Twine(" /*") + Param->getName() + "*/").str());
+}
 return;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63089: [clang] Warn on implicit boolean casts in more contexts (PR34180)

[Mateusz Maćkowski via Phabricator via cfe-commits]

m4tx created this revision.
m4tx added reviewers: lebedev.ri, rsmith, klimek.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

  int x;
  return x = 5;

For a code like above, GCC produces a warning suggesting using parentheses 
about the assignment. This change makes clang produce similar warning to, 
suggesting either changing the operator to `==`, or wrap the expression inside 
parentheses.

This is based off D45401 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63089

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/Sema/parentheses.c
  clang/test/Sema/parentheses.cpp


Index: clang/test/Sema/parentheses.cpp
===
--- clang/test/Sema/parentheses.cpp
+++ clang/test/Sema/parentheses.cpp
@@ -215,3 +215,15 @@
 // fix-it:"{{.*}}":{[[@LINE-9]]:20-[[@LINE-9]]:20}:")"
   }
 }
+
+bool return_assign() {
+  int i;
+  return i = 4; // expected-warning {{assignment as a condition}} \
+// expected-note{{place parentheses around the assignment to 
silence this warning}} \
+// expected-note{{use '==' to turn this assignment into an 
equality comparison}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:10}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:15-[[@LINE-4]]:15}:")"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:12-[[@LINE-5]]:13}:"=="
+
+  return (i = 4);
+}
Index: clang/test/Sema/parentheses.c
===
--- clang/test/Sema/parentheses.c
+++ clang/test/Sema/parentheses.c
@@ -14,6 +14,18 @@
   if ((i = 4)) {}
 }
 
+_Bool return_assign(void) {
+  int i;
+  return i = 4; // expected-warning {{assignment as a condition}} \
+// expected-note{{place parentheses around the assignment to 
silence this warning}} \
+// expected-note{{use '==' to turn this assignment into an 
equality comparison}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:10}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:15-[[@LINE-4]]:15}:")"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:12-[[@LINE-5]]:13}:"=="
+
+  return (i = 4);
+}
+
 void bitwise_rel(unsigned i) {
   (void)(i & 0x2 == 0); // expected-warning {{& has lower precedence than ==}} 
\
 // expected-note{{place parentheses around the '==' 
expression to silence this warning}} \
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -3926,6 +3926,11 @@
 FromType = From->getType();
   }
 
+  // Warn on assignments inside implicit casts (= instead of ==)
+  if (SCS.Second == ICK_Boolean_Conversion || FromType == Context.BoolTy) {
+DiagnoseAssignmentAsCondition(From->IgnoreImpCasts());
+  }
+
   // If we're converting to an atomic type, first convert to the corresponding
   // non-atomic type.
   QualType ToAtomicType;
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -16375,7 +16375,10 @@
 
 ExprResult Sema::CheckBooleanCondition(SourceLocation Loc, Expr *E,
bool IsConstexpr) {
-  DiagnoseAssignmentAsCondition(E);
+  // C++ implicit casts are checked inside PerformImplicitConversion
+  if (!getLangOpts().CPlusPlus) {
+DiagnoseAssignmentAsCondition(E);
+  }
   if (ParenExpr *parenE = dyn_cast(E))
 DiagnoseEqualityWithExtraParens(parenE);
 
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -9700,6 +9700,13 @@
   << FD << getLangOpts().CPlusPlus11;
 }
   }
+
+  // C++ implicit casts are checked inside PerformImplicitConversion
+  if (!getLangOpts().CPlusPlus) {
+if (ImplicitCastExpr *CastExpr = dyn_cast(RetValExp)) {
+  DiagnoseAssignmentAsCondition(CastExpr->IgnoreImpCasts());
+}
+  }
 }
 
 //===--- CHECK: Floating-Point comparisons (-Wfloat-equal) ---===//


Index: clang/test/Sema/parentheses.cpp
===
--- clang/test/Sema/parentheses.cpp
+++ clang/test/Sema/parentheses.cpp
@@ -215,3 +215,15 @@
 // fix-it:"{{.*}}":{[[@LINE-9]]:20-[[@LINE-9]]:20}:")"
   }
 }
+
+bool return_assign() {
+  int i;
+  return i = 4; // expected-warning {{assignment as a condition}} \
+// expected-note{{place parentheses around the assignment to silence this warning}} \
+// expected-note{{use '==' to turn this assignment into an equality comparison}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:10}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:15-[[@LINE-4]]:15}:")"
+  // CHECK: fix-it:"

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

[Michael Kruse via Phabricator via cfe-commits]

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

In D61446#1533076 , @serge-sans-paille 
wrote:

> That's what I tried to do when always adding Polly.cpp to PollyCore, but it 
> doesn't seem to be enough, I still need to invstigate why (it is actually 
> enough when using BUILD_SHARED_LIBS=ON)


With static libraries, you had the following structure:

  PollyCore.a:
RegisterPasses.o
Polly.o <-- static initializer here, but never used
  
  LLVMPolly.so (for use with -load mechanism)
RegisterPasses.o
Polly.o <-- static initializer here, executed when loading the .so
  
  opt:
main.o <-- call to initializePollyPass removed

With static linking, there is no reason for the linker to add Polly.o to the 
`opt` executable because no symbol from Polly.o (or any of PollyCore) was 
required to resolve any symbol in `opt`. Building a shared library using the 
object library files will include all object files, since it does not know 
which symbols will be used at runtime. I recommend reading 
http://www.lurklurk.org/linkers/linkers.html#staticlibs and 
https://www.bfilipek.com/2018/02/static-vars-static-lib.html.

What I proposed was

  PollyCore.a:
RegisterPasses.o
  
  LLVMPolly.so (for use with -load mechanism)
RegisterPasses.o
Polly.o
  
  opt: // or any ENABLE_PLUGINS tool
main.o
Polly.o // via add_executable(opt Polly.cpp) or target_sources

Adding the object file explicitly to the add_executable will add it 
unconditionally, even if no none of its symbol is required, like for 
LLVMPolly.so.

In the latest update you changed the location of the static initalizer to 
`RegisterPasses.o`, which contains the symbol `llvmGetPassPluginInfo` which is 
pulled-in by the new pass manager plugin system. This makes an unfortunate 
dependence of the static initializer on the `llvmGetPassPluginInfo` mechanism 
(which I think only works for `opt` in the current patch).

There is a reason why pass loading in Polly is organized as it is.




Comment at: llvm/cmake/modules/AddLLVM.cmake:812
+#   llvm::PassPluginLibraryInfo ${entry_point}();
+add_custom_target(LLVM_PLUGINS)  # target used to hold global properties 
referencable from generator-expression
+function(register_llvm_extension llvm_extension entry_point)

beanz wrote:
> Change this to `llvm-plugins` to match our convention and wrap it in `if (NOT 
> TARGET...)` so it doesn't error if AddLLVM is included twice.
[serious] I get multiple of these errors by cmake:
```
CMake Error at cmake/modules/AddLLVM.cmake:812 (add_custom_target):
  add_custom_target cannot create target "LLVM_PLUGINS" because another
  target with the same name already exists.  The existing target is a custom
  target created in source directory "/home/meinersbur/src/llvm".  See
  documentation for policy CMP0002 for more details.
Call Stack (most recent call first):
  projects/compiler-rt/unittests/CMakeLists.txt:2 (include)
```



Comment at: llvm/tools/opt/NewPMDriver.cpp:292
+#define HANDLE_EXTENSION(Ext)  
\
+  get##Ext##PluginInfo().RegisterPassBuilderCallbacks(PB);
+#include "llvm/Support/Extension.def"

[serious] This will pull-in the symbol `llvmGetPassPluginInfo` from the plugin 
for `opt`, but what about the other tools (bugpoint/clang)?



Comment at: polly/lib/Support/RegisterPasses.cpp:727
+extern "C" ::llvm::PassPluginLibraryInfo LLVM_ATTRIBUTE_WEAK
+llvmGetPassPluginInfo() {
+  return getPassPluginInfo();

[serious] Unfortunately, the new pass manager's plugin system relies on the 
function name to be `llvmGetPassPluginInfo` in each plugin. This works with 
multiple dynamic libraries all declaring the same name using the 
`PassPlugin::Load` mechanism, but linking them all statically will violate the 
one-definition-rule.

IMHO, Polly.cpp would have been a better place for this function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446



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

[PATCH] D62926: [analyzer] ReturnVisitor: Bypass everything to see inlined calls

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 203874.
Charusso retitled this revision from "[analyzer] ReturnVisitor: Bypass 
constructing objects to see inlined calls" to "[analyzer] ReturnVisitor: Bypass 
everything to see inlined calls".
Charusso added a comment.

- The most generic approach.


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

https://reviews.llvm.org/D62926

Files:
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/test/Analysis/new-ctor-null-throw.cpp
  clang/test/Analysis/new-ctor-null.cpp


Index: clang/test/Analysis/new-ctor-null.cpp
===
--- clang/test/Analysis/new-ctor-null.cpp
+++ clang/test/Analysis/new-ctor-null.cpp
@@ -1,4 +1,6 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection 
-analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,debug.ExprInspection \
+// RUN:  -verify %s
 
 void clang_analyzer_eval(bool);
 void clang_analyzer_warnIfReached();
@@ -24,7 +26,8 @@
 
 void testArrays() {
   S *s = new S[10]; // no-crash
-  s[0].x = 2; // expected-warning{{Dereference of null pointer}}
+  s[0].x = 2;
+  // no-warning: 'Dereference of null pointer' suppressed by ReturnVisitor.
 }
 
 int global;
Index: clang/test/Analysis/new-ctor-null-throw.cpp
===
--- clang/test/Analysis/new-ctor-null-throw.cpp
+++ clang/test/Analysis/new-ctor-null-throw.cpp
@@ -1,4 +1,9 @@
-// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,debug.ExprInspection 
-analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:  -analyzer-config suppress-null-return-paths=false \
+// RUN:  -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:  -DSUPPRESSED \
+// RUN:  -verify %s
 
 void clang_analyzer_eval(bool);
 
@@ -9,18 +14,41 @@
 // operator new.
 void *operator new(size_t size) {
   return nullptr;
+  // expected-warning@-1 {{'operator new' should not return a null pointer 
unless it is declared 'throw()' or 'noexcept'}}
 }
 void *operator new[](size_t size) {
   return nullptr;
+  // expected-warning@-1 {{'operator new[]' should not return a null pointer 
unless it is declared 'throw()' or 'noexcept'}}
 }
 
 struct S {
   int x;
   S() : x(1) {}
   ~S() {}
+  int getX() const { return x; }
 };
 
 void testArrays() {
   S *s = new S[10]; // no-crash
-  s[0].x = 2; // expected-warning{{Dereference of null pointer}}
+  s[0].x = 2;
+#ifndef SUPPRESSED
+  // expected-warning@-2 {{Dereference of null pointer}}
+#endif
 }
+
+void testCtor() {
+  S *s = new S();
+  s->x = 13;
+#ifndef SUPPRESSED
+  // expected-warning@-2 {{Access to field 'x' results in a dereference of a 
null pointer (loaded from variable 's')}}
+#endif
+}
+
+void testMethod() {
+  S *s = new S();
+  const int X = s->getX();
+#ifndef SUPPRESSED
+  // expected-warning@-2 {{Called C++ object pointer is null}}
+#endif
+}
+
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -834,12 +834,9 @@
 
 // First, find when we processed the statement.
 do {
-  if (auto CEE = Node->getLocationAs())
+  if (Optional CEE = Node->getLocationAs())
 if (CEE->getCalleeContext()->getCallSite() == S)
   break;
-  if (auto SP = Node->getLocationAs())
-if (SP->getStmt() == S)
-  break;
 
   Node = Node->getFirstPred();
 } while (Node);


Index: clang/test/Analysis/new-ctor-null.cpp
===
--- clang/test/Analysis/new-ctor-null.cpp
+++ clang/test/Analysis/new-ctor-null.cpp
@@ -1,4 +1,6 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,debug.ExprInspection \
+// RUN:  -verify %s
 
 void clang_analyzer_eval(bool);
 void clang_analyzer_warnIfReached();
@@ -24,7 +26,8 @@
 
 void testArrays() {
   S *s = new S[10]; // no-crash
-  s[0].x = 2; // expected-warning{{Dereference of null pointer}}
+  s[0].x = 2;
+  // no-warning: 'Dereference of null pointer' suppressed by ReturnVisitor.
 }
 
 int global;
Index: clang/test/Analysis/new-ctor-null-throw.cpp
===
--- clang/test/Analysis/new-ctor-null-throw.cpp
+++ clang/test/Analysis/new-ctor-null-throw.cpp
@@ -1,4 +1,9 @@
-// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:  -analyzer-config suppress-null-return-paths=false \
+// RUN

[PATCH] D63089: [clang] Warn on implicit boolean casts in more contexts (PR34180)

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

xbolva00 added a comment.

Thanks !

You can check and take tests from my older patch https://reviews.llvm.org/D58878


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63089



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

[PATCH] D62926: [analyzer] ReturnVisitor: Bypass everything to see inlined calls

[Csaba Dabis via Phabricator via cfe-commits]

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



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:842-849
+  if (Optional CEE = Node->getLocationAs())
 if (CEE->getCalleeContext()->getCallSite() == S)
   break;
-  if (auto SP = Node->getLocationAs())
-if (SP->getStmt() == S)
-  break;
+
+  if (!IsBypass)
+if (Optional SP = Node->getLocationAs())
+  if (SP->getStmt() == S)

NoQ wrote:
> Comparing statements is usually insufficient because the same statement may 
> appear multiple times due to recursion. When recursion occurs, you may reach 
> the same statement in a different location context. You should think in terms 
> of (statement, location context) pairs to avoid these problems. Your aim here 
> is to find the `CallExitEnd` node that corresponds to returning from an 
> inlined operator new to the current location context. You should stop 
> searching when you find an unrelated statement in the current location 
> context or when you exit the current location context entirely.
I have made a little test when we have a 25-second long Static Analyzer run 
with predefined names and checker. The loop ran 500 times in summary and we 
have some serious performance impacts at other places.

We exit the current context to see inlined calls, so that could not work sadly. 
If you remove that nonsense second condition we run at the same time, so if we 
have not got any problem since 7 years ago I think it is good to go.


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

https://reviews.llvm.org/D62926



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

[PATCH] D59919: [Attributor] Deduce "returned" argument attribute

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D59919#1535643 , @nicholas wrote:

> > CHANGED: build-libcalls   NumNoUnwind   
> >   4526 ->   3382 (   -25.276%)
>
> Why did the number of nounwinds drop?


I rerun the experiment with and without this commit. The numbers are the same 
as before except that there is no difference in NumNoUnwind. I don't know if I 
used a different baseline or if there was another problem (e.g., concurrency 
related) but I don't think this patch does affect the NumNoUnwind number at all.

Btw. I only report statistics that changed more than 1% as there seem to be 
some minimal variations all the time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59919



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

[PATCH] D63062: [clang-format] Added New Style Rule: BitFieldDeclsOnSeparateLines

[MyDeveloperDay via Phabricator via cfe-commits]

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



Comment at: docs/ClangFormatStyleOptions.rst:194
 
+**BitFieldDeclsOnSeparateLines** (``bool``)
+  If ``true``, Align Bitfield Declarations on separate lines.

Isn't the documentation normally alphabetric? shouldn't it be after 
AlignConsecutiveAssignments



Comment at: include/clang/Format/Format.h:104
 
+  /// If ``true``, Linesup Bitfield Declarations.
+  /// This will lineup Bitfield declarations on consecutive lines. This

Align



Comment at: include/clang/Format/Format.h:112
+  /// \endcode
+  bool BitFieldDeclsOnSeparateLines;
+

I think this should be alphabetic in this file (not sure though check the rest 
of it)

are you happy with the name?  BitFieldDeclsOnSeparateLines? 

1) people often spell Separate incorrectly (didn't you?), this could lead to 
misconfigured
2) isn't this really a ''Break'' rule

I want to say this might better as  something like "BreakAfterBitFieldDecl"





Comment at: lib/Format/ContinuationIndenter.cpp:284
+return true;
   if (!Current.CanBreakBefore && !(State.Stack.back().BreakBeforeClosingBrace 
&&
Current.closesBlockOrBlockTypeList(Style)))

something here doesn't feel quite right,, without trying the code change myself 
I cannot tell, did you ever try this code without having the same clause in 
canBreak() and mustBreak()? (i.e. just put it in mustBreak)

The reason I ask is I'm unclear as to why the other mustBreak() rules aren't 
here in canBreak() if thats the case



Comment at: lib/Format/TokenAnnotator.cpp:2921
+  Right.is(tok::identifier) && (Right.Next->is(tok::colon)))
+  return true;
   if (Right.NewlinesBefore > 1 && Style.MaxEmptyLinesToKeep > 0)

mgorny wrote:
> Misindent.
This code appears 3 times (does it need to appear 3 times?), do we need some 
sort of 

```
bool isBitField(FormatToken)
{
...
}
```

Should a bit field check for the existence of a number after the colon? I can't 
think of other C++ constructs that appear as

```
comma identifier colon
```

but given that clang-format is used for ObjC,ProtoBuf,Java,JavaScript,C# I'm 
pretty sure something odd is going to happen with JavaScript named parameters, 
to be honest I think this is going to cause the following to get reformatted 

MyFunctionCall({ xPosition: 20**, yPosition: 50,** width: 100, height: 5, 
drawingNow: true });


```
MyFunctionCall({ xPosition: 20**, yPosition: 50,**
width: 100, 
height: 5, 
drawingNow: true });
```

or something like that



Comment at: unittests/Format/FormatTest.cpp:3671
+  );
+} 
 

please add a test with comments (it will get logged)


```
unsigned int baz : 11, /*motor control flags*/
 add: 2/* control code for turning the lights on */ ,
 foo: 3 /* (unused */
```



Repository:
  rC Clang

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

https://reviews.llvm.org/D63062



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

[PATCH] D62978: [analyzer] ReturnVisitor: Handle unknown ReturnStmts better

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 203876.
Charusso retitled this revision from "[analyzer] ReturnVisitor: Handle non-null 
ReturnStmts" to "[analyzer] ReturnVisitor: Handle unknown ReturnStmts better".
Charusso edited the summary of this revision.
Charusso added a comment.

- The report was too misleading.
- It turns out we have to keep non-nulls.
- Now we do not treat unknown values as known.


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

https://reviews.llvm.org/D62978

Files:
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/test/Analysis/diagnostics/find_last_store.c
  clang/test/Analysis/uninit-vals.c

Index: clang/test/Analysis/uninit-vals.c
===
--- clang/test/Analysis/uninit-vals.c
+++ clang/test/Analysis/uninit-vals.c
@@ -149,8 +149,6 @@
   RetVoidFuncType f = foo_radar12278788_fp;
   return ((RetIntFuncType)f)(); //expected-warning {{Undefined or garbage value returned to caller}}
 //expected-note@-1 {{Undefined or garbage value returned to caller}}
-//expected-note@-2 {{Calling 'foo_radar12278788_fp'}}
-//expected-note@-3 {{Returning from 'foo_radar12278788_fp'}}
 }
 
 void rdar13665798() {
@@ -164,8 +162,6 @@
 RetVoidFuncType f = foo_radar12278788_fp;
 return ((RetIntFuncType)f)(); //expected-warning {{Undefined or garbage value returned to caller}}
   //expected-note@-1 {{Undefined or garbage value returned to caller}}
-  //expected-note@-2 {{Calling 'foo_radar12278788_fp'}}
-  //expected-note@-3 {{Returning from 'foo_radar12278788_fp'}}
   }();
 }
 
@@ -182,18 +178,14 @@
 void use(struct Point p); 
 
 void testUseHalfPoint() {
-  struct Point p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}}
-   // expected-note@-1{{Returning from 'getHalfPoint'}}
-   // expected-note@-2{{'p' initialized here}}
+  struct Point p = getHalfPoint(); // expected-note{{'p' initialized here}}
   use(p); // expected-warning{{uninitialized}}
   // expected-note@-1{{uninitialized}}
 }
 
 void testUseHalfPoint2() {
   struct Point p;
-  p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}}
-  // expected-note@-1{{Returning from 'getHalfPoint'}}
-  // expected-note@-2{{Value assigned to 'p'}}
+  p = getHalfPoint(); // expected-note{{Value assigned to 'p'}}
   use(p); // expected-warning{{uninitialized}}
   // expected-note@-1{{uninitialized}}
 }
Index: clang/test/Analysis/diagnostics/find_last_store.c
===
--- clang/test/Analysis/diagnostics/find_last_store.c
+++ clang/test/Analysis/diagnostics/find_last_store.c
@@ -2,13 +2,11 @@
 typedef struct { float b; } c;
 void *a();
 void *d() {
-  return a(); // expected-note{{Returning pointer}}
+  return a();
 }
 
 void no_find_last_store() {
-  c *e = d(); // expected-note{{Calling 'd'}}
-  // expected-note@-1{{Returning from 'd'}}
-  // expected-note@-2{{'e' initialized here}}
+  c *e = d(); // expected-note{{'e' initialized here}}
 
   (void)(e || e->b); // expected-note{{Assuming 'e' is null}}
   // expected-note@-1{{Left side of '||' is false}}
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -874,7 +874,6 @@
   if (Optional RetLoc = RetVal.getAs())
 EnableNullFPSuppression = State->isNull(*RetLoc).isConstrainedTrue();
 
-BR.markInteresting(CalleeContext);
 BR.addVisitor(llvm::make_unique(CalleeContext,
EnableNullFPSuppression,
Options));
@@ -925,16 +924,13 @@
 
 RetE = RetE->IgnoreParenCasts();
 
-// If we're returning 0, we should track where that 0 came from.
-bugreporter::trackExpressionValue(N, RetE, BR, EnableNullFPSuppression);
-
 // Build an appropriate message based on the return value.
 SmallString<64> Msg;
 llvm::raw_svector_ostream Out(Msg);
 
+// Known to be null.
 if (State->isNull(V).isConstrainedTrue()) {
   if (V.getAs()) {
-
 // If we have counter-suppression enabled, make sure we keep visiting
 // future nodes. We want to emit a path note as well, in case
 // the report is resurrected as valid later on.
@@ -950,8 +946,8 @@
   } else {
 Out << "Returning zero";
   }
-
-} else {
+// Known to be non-null.
+} else if (State->isNonNull(V).isConstrainedTrue()) {
   if (auto CI = V.getAs()) {
 Out << "Returning the valu

[PATCH] D63091: [clangd] Add a capability to enable completions with fixes.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D63091

Files:
  clangd/ClangdLSPServer.cpp
  clangd/Protocol.cpp
  clangd/Protocol.h


Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -387,6 +387,11 @@
   /// textDocument.completion.completionItem.snippetSupport
   bool CompletionSnippets = false;
 
+  /// Client supports completions with additionalTextEdit near the cursor.
+  /// This is a clangd extension. (LSP says this is for unrelated text only).
+  /// textDocument.completion.editsNearCursor
+  bool CompletionFixes = false;
+
   /// Client supports hierarchical document symbols.
   bool HierarchicalDocumentSymbol = false;
 
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -293,6 +293,8 @@
 return false;
 }
   }
+  if (auto EditsNearCursor = Completion->getBoolean("editsNearCursor"))
+R.CompletionFixes = *EditsNearCursor;
 }
 if (auto *CodeAction = TextDocument->getObject("codeAction")) {
   if (CodeAction->getObject("codeActionLiteralSupport"))
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -337,6 +337,7 @@
   applyConfiguration(Params.initializationOptions.ConfigSettings);
 
   CCOpts.EnableSnippets = Params.capabilities.CompletionSnippets;
+  CCOpts.IncludeFixIts = Params.capabilities.CompletionFixes;
   DiagOpts.EmbedFixesInDiagnostics = Params.capabilities.DiagnosticFixes;
   DiagOpts.SendDiagnosticCategory = Params.capabilities.DiagnosticCategory;
   DiagOpts.EmitRelatedLocations =


Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -387,6 +387,11 @@
   /// textDocument.completion.completionItem.snippetSupport
   bool CompletionSnippets = false;
 
+  /// Client supports completions with additionalTextEdit near the cursor.
+  /// This is a clangd extension. (LSP says this is for unrelated text only).
+  /// textDocument.completion.editsNearCursor
+  bool CompletionFixes = false;
+
   /// Client supports hierarchical document symbols.
   bool HierarchicalDocumentSymbol = false;
 
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -293,6 +293,8 @@
 return false;
 }
   }
+  if (auto EditsNearCursor = Completion->getBoolean("editsNearCursor"))
+R.CompletionFixes = *EditsNearCursor;
 }
 if (auto *CodeAction = TextDocument->getObject("codeAction")) {
   if (CodeAction->getObject("codeActionLiteralSupport"))
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -337,6 +337,7 @@
   applyConfiguration(Params.initializationOptions.ConfigSettings);
 
   CCOpts.EnableSnippets = Params.capabilities.CompletionSnippets;
+  CCOpts.IncludeFixIts = Params.capabilities.CompletionFixes;
   DiagOpts.EmbedFixesInDiagnostics = Params.capabilities.DiagnosticFixes;
   DiagOpts.SendDiagnosticCategory = Params.capabilities.DiagnosticCategory;
   DiagOpts.EmitRelatedLocations =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62978: [analyzer] ReturnVisitor: Handle unknown ReturnStmts better

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

In D62978#1533558 , @NoQ wrote:

> Ah, that positive!


Positive == true positive, not false positive, I got it.

> No, i don't think this is a valid way to suppress it.

Bought me, they are worth to report. The misleading reports made me think I 
have to suppress them.

Example (difference starts at note 33, at line 410):
F9159208: before.html 
F9159209: after.html 


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

https://reviews.llvm.org/D62978



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

[PATCH] D63041: [PlistSupport] Produce a newline to end plist output files

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.

Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D63041



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

[PATCH] D63092: [Frontend] Use executable path when creating invocation from cmdline

[Petr Hosek via Phabricator via cfe-commits]

phosek created this revision.
phosek added reviewers: ilya-biryukov, sammccall, bkramer, echristo.
Herald added subscribers: cfe-commits, kadircet.
Herald added a project: clang.

Args[0] isn't necessarily a correct path, it may not even be the path
to the compiler, for example when using tools like ccache, distcc or
goma the first argument in command recorded inside the compilation
database would be this tool, not the compiler. This path is then used
as installation dir inside the driver and is then used to derive other
toolchain specific paths, such as path to libc++ headers (which would
be ../include/c++/v1 relative to the compiler binary). When the first
argument is another tool, or even if it's Clang but the compilation
database contains a relative path, the installation dir is going to be
incorrect and the header resolution will fail which breaks tools like
clangd. So rather than relying blindly on the first argument, we'll use
the executable path (when available) instead.


Repository:
  rC Clang

https://reviews.llvm.org/D63092

Files:
  clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
  clang/unittests/libclang/LibclangTest.cpp


Index: clang/unittests/libclang/LibclangTest.cpp
===
--- clang/unittests/libclang/LibclangTest.cpp
+++ clang/unittests/libclang/LibclangTest.cpp
@@ -621,7 +621,10 @@
   std::string Clang = "bin/clang";
   WriteFile(Clang, "");
 
+  std::string InstalledDir = TestDir + "/bin";
+
   const char *Argv[] = {Clang.c_str(), "-target", "arm-linux-gnueabi",
+"-ccc-install-dir", InstalledDir.c_str(),
 "-stdlib=libstdc++", "--gcc-toolchain="};
 
   EXPECT_EQ(CXError_Success,
Index: clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
===
--- clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
+++ clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
@@ -43,8 +43,12 @@
   // FIXME: Find a cleaner way to force the driver into restricted modes.
   Args.push_back("-fsyntax-only");
 
+  void *P = reinterpret_cast(createInvocationFromCommandLine);
+  std::string ClangExecutable =
+  llvm::sys::fs::getMainExecutable(Args[0], P);
+
   // FIXME: We shouldn't have to pass in the path info.
-  driver::Driver TheDriver(Args[0], llvm::sys::getDefaultTargetTriple(),
+  driver::Driver TheDriver(ClangExecutable, 
llvm::sys::getDefaultTargetTriple(),
*Diags, VFS);
 
   // Don't check that inputs exist, they may have been remapped.


Index: clang/unittests/libclang/LibclangTest.cpp
===
--- clang/unittests/libclang/LibclangTest.cpp
+++ clang/unittests/libclang/LibclangTest.cpp
@@ -621,7 +621,10 @@
   std::string Clang = "bin/clang";
   WriteFile(Clang, "");
 
+  std::string InstalledDir = TestDir + "/bin";
+
   const char *Argv[] = {Clang.c_str(), "-target", "arm-linux-gnueabi",
+"-ccc-install-dir", InstalledDir.c_str(),
 "-stdlib=libstdc++", "--gcc-toolchain="};
 
   EXPECT_EQ(CXError_Success,
Index: clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
===
--- clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
+++ clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
@@ -43,8 +43,12 @@
   // FIXME: Find a cleaner way to force the driver into restricted modes.
   Args.push_back("-fsyntax-only");
 
+  void *P = reinterpret_cast(createInvocationFromCommandLine);
+  std::string ClangExecutable =
+  llvm::sys::fs::getMainExecutable(Args[0], P);
+
   // FIXME: We shouldn't have to pass in the path info.
-  driver::Driver TheDriver(Args[0], llvm::sys::getDefaultTargetTriple(),
+  driver::Driver TheDriver(ClangExecutable, llvm::sys::getDefaultTargetTriple(),
*Diags, VFS);
 
   // Don't check that inputs exist, they may have been remapped.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63093: [analyzer] WIP: MallocChecker: Release temporary CXXNewExpr

[Csaba Dabis via Phabricator via cfe-commits]

Charusso created this revision.
Charusso added reviewers: NoQ, xazax.hun, ravikandhadai, baloghadamsoftware, 
Szelethus.
Charusso added a project: clang.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, rnkovacs, szepet.

-


Repository:
  rC Clang

https://reviews.llvm.org/D63093

Files:
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp


Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -156,21 +156,14 @@
 
 typedef std::pair LeakInfo;
 
-class MallocChecker : public Checker,
- check::EndFunction,
- check::PreCall,
- check::PostStmt,
- check::PostStmt,
- check::NewAllocator,
- check::PreStmt,
- check::PostStmt,
- check::PostObjCMessage,
- check::Location,
- eval::Assume>
-{
+class MallocChecker
+: public Checker,
+ check::EndFunction, check::PreCall,
+ check::PostStmt, check::PostStmt,
+ check::PostStmt, 
check::NewAllocator,
+ check::PreStmt, check::PostStmt,
+ check::PostObjCMessage, check::Location, eval::Assume> {
 public:
   MallocChecker()
   : II_alloca(nullptr), II_win_alloca(nullptr), II_malloc(nullptr),
@@ -211,6 +204,7 @@
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   void checkPostStmt(const CallExpr *CE, CheckerContext &C) const;
   void checkPostStmt(const CXXNewExpr *NE, CheckerContext &C) const;
+  void checkPostStmt(const CXXBindTemporaryExpr *BTE, CheckerContext &C) const;
   void checkNewAllocator(const CXXNewExpr *NE, SVal Target,
  CheckerContext &C) const;
   void checkPreStmt(const CXXDeleteExpr *DE, CheckerContext &C) const;
@@ -489,7 +483,8 @@
   bool IsReleased = (S && S->isReleased()) &&
 (!SPrev || !SPrev->isReleased());
   assert(!IsReleased ||
- (Stmt && (isa(Stmt) || isa(Stmt))) ||
+ (Stmt && (isa(Stmt) || isa(Stmt) ||
+   isa(Stmt))) ||
  (!Stmt && S->getAllocationFamily() == AF_InnerBuffer));
   return IsReleased;
 }
@@ -1103,6 +1098,34 @@
 processNewAllocation(NE, C, Target);
 }
 
+void MallocChecker::checkPostStmt(const CXXBindTemporaryExpr *BTE,
+  CheckerContext &C) const {
+  if (const auto *CE = dyn_cast(BTE->getSubExpr())) {
+if (CE->getNumArgs() == 0)
+  return;
+
+// If we catch a 'CXXNewExpr' set it is released as it is temporary.
+if (const auto *NE = dyn_cast(CE->getArg(0))) {
+  ProgramStateRef State = C.getState();
+  Optional RetVal = C.getSVal(NE);
+
+  SymbolRef Sym = RetVal->getAsLocSymbol();
+  assert(Sym);
+
+  if (const RefState *RS = State->get(Sym))
+if (RS->isReleased())
+  return;
+
+  // Set the symbol's state to Released.
+  State = State->set(
+  Sym, RefState::getReleased(NE->isArray() ? AF_CXXNewArray : 
AF_CXXNew,
+ NE));
+
+  C.addTransition(State);
+}
+  }
+}
+
 // Sets the extent value of the MemRegion allocated by
 // new expression NE to its size in Bytes.
 //


Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -156,21 +156,14 @@
 
 typedef std::pair LeakInfo;
 
-class MallocChecker : public Checker,
- check::EndFunction,
- check::PreCall,
- check::PostStmt,
- check::PostStmt,
- check::NewAllocator,
- check::PreStmt,
- check::PostStmt,
- check::PostObjCMessage,
- check::Location,
- eval::Assume>
-{
+class MallocChecker
+: public Checker,
+ check::EndFunction, check::PreCall,
+ check::PostStmt, check::PostStmt,
+ check::PostStmt, check::NewAllocator,
+ check::PreStmt, check::PostStmt,
+ check::PostObjCMessage, check::Location, eval::Assume> {
 public:
   MallocChecker()
   : II_alloca(nullptr), II_win_alloca(null

[PATCH] D63093: [analyzer] WIP: MallocChecker: Release temporary CXXNewExpr

[Csaba Dabis via Phabricator via cfe-commits]

Charusso planned changes to this revision.
Charusso added a comment.

This is heavily WIP as sometimes we have to release a `new` after we return it 
or a constructor did something with that. The direction is okay?


Repository:
  rC Clang

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

https://reviews.llvm.org/D63093



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

[PATCH] D60233: [clang-scan-deps] initial outline of the tool that runs preprocessor to find dependencies over a JSON compilation database

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 203887.
arphaman marked 3 inline comments as done.
arphaman added a comment.

- Add test for `-j2` to exercise multi threading.
- Add `InitLLVM`.


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

https://reviews.llvm.org/D60233

Files:
  clang/test/ClangScanDeps/Inputs/header.h
  clang/test/ClangScanDeps/Inputs/header2.h
  clang/test/ClangScanDeps/Inputs/regular_cdb.json
  clang/test/ClangScanDeps/regular_cdb.cpp
  clang/tools/CMakeLists.txt
  clang/tools/clang-scan-deps/CMakeLists.txt
  clang/tools/clang-scan-deps/ClangScanDeps.cpp

Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- /dev/null
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -0,0 +1,218 @@
+//===-- ClangScanDeps.cpp - Implementation of clang-scan-deps -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Frontend/PCHContainerOperations.h"
+#include "clang/FrontendTool/Utils.h"
+#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/JSONCompilationDatabase.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/InitLLVM.h"
+#include "llvm/Support/JSON.h"
+#include "llvm/Support/Options.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
+#include "llvm/Support/Signals.h"
+#include "llvm/Support/Threading.h"
+#include 
+
+using namespace clang;
+
+namespace {
+
+/// A clang tool that runs the preprocessor only for the given compiler
+/// invocation.
+class PreprocessorOnlyTool : public tooling::ToolAction {
+public:
+  PreprocessorOnlyTool(StringRef WorkingDirectory)
+  : WorkingDirectory(WorkingDirectory) {}
+
+  bool runInvocation(std::shared_ptr Invocation,
+ FileManager *FileMgr,
+ std::shared_ptr PCHContainerOps,
+ DiagnosticConsumer *DiagConsumer) override {
+// Create a compiler instance to handle the actual work.
+CompilerInstance Compiler(std::move(PCHContainerOps));
+Compiler.setInvocation(std::move(Invocation));
+FileMgr->getFileSystemOpts().WorkingDir = WorkingDirectory;
+Compiler.setFileManager(FileMgr);
+
+// Create the compiler's actual diagnostics engine.
+Compiler.createDiagnostics(DiagConsumer, /*ShouldOwnClient=*/false);
+if (!Compiler.hasDiagnostics())
+  return false;
+
+Compiler.createSourceManager(*FileMgr);
+
+auto Action = llvm::make_unique();
+const bool Result = Compiler.ExecuteAction(*Action);
+FileMgr->clearStatCache();
+return Result;
+  }
+
+private:
+  StringRef WorkingDirectory;
+};
+
+/// A proxy file system that doesn't call `chdir` when changing the working
+/// directory of a clang tool.
+class ProxyFileSystemWithoutChdir : public llvm::vfs::ProxyFileSystem {
+public:
+  ProxyFileSystemWithoutChdir(
+  llvm::IntrusiveRefCntPtr FS)
+  : ProxyFileSystem(std::move(FS)) {}
+
+  llvm::ErrorOr getCurrentWorkingDirectory() const override {
+assert(!CWD.empty() && "empty CWD");
+return CWD;
+  }
+
+  std::error_code setCurrentWorkingDirectory(const Twine &Path) override {
+CWD = Path.str();
+return {};
+  }
+
+private:
+  std::string CWD;
+};
+
+/// The high-level implementation of the dependency discovery tool that runs on
+/// an individual worker thread.
+class DependencyScanningTool {
+public:
+  /// Construct a dependency scanning tool.
+  ///
+  /// \param Compilations The reference to the compilation database that's
+  /// used by the clang tool.
+  DependencyScanningTool(const tooling::CompilationDatabase &Compilations)
+  : Compilations(Compilations) {
+PCHContainerOps = std::make_shared();
+BaseFS = new ProxyFileSystemWithoutChdir(llvm::vfs::getRealFileSystem());
+  }
+
+  /// Computes the dependencies for the given file.
+  ///
+  /// \returns True on error.
+  bool runOnFile(const std::string &Input, StringRef CWD) {
+BaseFS->setCurrentWorkingDirectory(CWD);
+tooling::ClangTool Tool(Compilations, Input, PCHContainerOps, BaseFS);
+Tool.clearArgumentsAdjusters();
+Tool.setRestoreWorkingDir(false);
+PreprocessorOnlyTool Action(CWD);
+return Tool.run(&Action);
+  }
+
+private:
+  const tooling::CompilationDatabase &Compilations;
+  std::shared_ptr PCHContainerOps;
+  /// The real filesystem used as a base for all the operations performed by the
+  /// tool.
+  llvm::IntrusiveRefCntPtr BaseFS;
+};
+
+llvm::cl::opt Help("h", llvm::cl::desc("Alias for -help"),
+ llvm::cl::Hidden);
+
+llvm::cl::OptionCategory De

[PATCH] D60233: [clang-scan-deps] initial outline of the tool that runs preprocessor to find dependencies over a JSON compilation database

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: clang/test/ClangScanDeps/Inputs/regular_cdb.json:4
+  "directory": "DIR",
+  "command": "clang -c DIR/regular_cdb.cpp -IInputs -MD -MF DIR/regular_cdb.d",
+  "file": "DIR/regular_cdb.cpp"

aganea wrote:
> Is `-MD -MF` required for clang-scan-deps to work? Can't we append those 
> arguments automatically, so that only include paths are needed in the CDB?
> 
> Would it possible for the tool to produce a collated file? A real-world usage 
> would otherwise produce tens of thousands of files. Which then would be 
> opened/parsed by the calling application.
I agree, the tool shouldn't rely on those options being there. `-MD -MF` in 
this test right now a crutch to get the dependencies printed somewhere.

I think the tool by default should print out the collated dependencies to 
STDOUT instead of writing them to files that were specified for the build. The 
`-MD -MF` options will not be required as well. This implementation requires 
some changes in the dependency collector, which I am planning to do as part of 
clang-scan-deps in the follow-up patches. Will it be ok to keep this patch as 
it is right now, and transition to printing out the collated dependencies 
without requiring the options as a follow-up?




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

https://reviews.llvm.org/D60233



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

[PATCH] D63092: [Frontend] Use executable path when creating invocation from cmdline

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.
Herald added a subscriber: ormris.

`argv[0]` does carry important information though, I think this will break a 
lot of things. It's... concerning that no tests broke.

For example, if it's `clang` or `g++` or `clang-cl` then that affects how 
command lines are parsed (regardless of whether the path actually exists).

Additionally I don't think all tools that call createInvocationFromCommandLine 
carry around all the files you want.

The wrapper tool case is a problem though, I see some options:

- change the process that generates the compilation database to emit a path to 
the underlying compiler instead, as tooling currently expects. This may be 
painful if e.g. goma pretends to be a regular toolchain. An argument can be 
made it should pretend harder (e.g. symlinking directories around it so 
discovery works as usual)
- change the process that generates the CDB to inject the relevant directory 
paths explicitly as flags
- make driver aware of the distinction between "invoked-as" and "current 
binary" and use the right path in the right places

(Incidentally if Driver is going to use the current binary, we shouldn't need 
to pass it in as a parameter, right?)

There are some potentially-unsolvable cases here: e.g. if your wrapper is 
`magic-remote-build` then maybe there's no standard library locally, or no way 
to find it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63092



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

[clang-tools-extra] r362978 - [clangd] Remove old hidden -use-dex-index flag

[Sam McCall via cfe-commits]

Author: sammccall
Date: Mon Jun 10 13:38:16 2019
New Revision: 362978

URL: http://llvm.org/viewvc/llvm-project?rev=362978&view=rev
Log:
[clangd] Remove old hidden -use-dex-index flag

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

Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=362978&r1=362977&r2=362978&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Mon Jun 10 13:38:16 2019
@@ -88,8 +88,7 @@ public:
 /// opened files and uses the index to augment code completion results.
 bool BuildDynamicSymbolIndex = false;
 /// Use a heavier and faster in-memory index implementation.
-/// FIXME: we should make this true if it isn't too slow to build!.
-bool HeavyweightDynamicSymbolIndex = false;
+bool HeavyweightDynamicSymbolIndex = true;
 /// If true, ClangdServer automatically indexes files in the current 
project
 /// on background threads. The index is stored in the project root.
 bool BackgroundIndex = false;

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=362978&r1=362977&r2=362978&view=diff
==
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Mon Jun 10 13:38:16 2019
@@ -34,11 +34,6 @@
 
 namespace clang {
 namespace clangd {
-// FIXME: remove this option when Dex is cheap enough.
-static llvm::cl::opt
-UseDex("use-dex-index",
-   llvm::cl::desc("Use experimental Dex dynamic index"),
-   llvm::cl::init(true), llvm::cl::Hidden);
 
 static llvm::cl::opt CompileCommandsDir(
 "compile-commands-dir",
@@ -447,7 +442,6 @@ int main(int argc, char *argv[]) {
   if (!ResourceDir.empty())
 Opts.ResourceDir = ResourceDir;
   Opts.BuildDynamicSymbolIndex = EnableIndex;
-  Opts.HeavyweightDynamicSymbolIndex = UseDex;
   Opts.BackgroundIndex = EnableBackgroundIndex;
   Opts.BackgroundIndexRebuildPeriodMs = BackgroundIndexRebuildPeriod;
   std::unique_ptr StaticIdx;


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

[PATCH] D62888: [NewPM] Port Sancov

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

*ping*


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62888



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

[PATCH] D63098: [CodeComplete] Allow completing enum values within case statements, and insert 'case' as a fixit.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

There are a few alternative ways to expose this.
The most obvious is a RK_Pattern, but it seems to have some triggering problems.
Consider a completion "case Color::Red"

- we want the user input to match Red
- we *don't* want the user input to match Color::
- if "case" is displayed, users will expect to be able to match it.

It's not possible to satisfy all of these:

- [TypedText:case] [Text:Color::][TypedText:Red] has a bad getTypedText()
- [Text:case] [Text:Color::][TypedText:Red] shows "case" but you can't type it
- [TypedText:case Color::TypedText:Red] will match against the qualifier

(I suspect the third is the least bad here)

The option explored here is to provide a regular Declaration completion, but
with "case" inserted as a fix-it. This results in "case" being neither displayed
or matched, but inserted. (If the user types "case", the existing completions
kick in.)

Curious what you think. I'm biased as I'm working on an integration that
supports fixits but not patterns :-)


Repository:
  rC Clang

https://reviews.llvm.org/D63098

Files:
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/enum-switch-case.cpp

Index: test/CodeCompletion/enum-switch-case.cpp
===
--- test/CodeCompletion/enum-switch-case.cpp
+++ test/CodeCompletion/enum-switch-case.cpp
@@ -18,11 +18,13 @@
   case N::Yellow:
 break;
 
-  case 
+  case N::Green:
 // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:21:8 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
 // CHECK-CC1: Blue : [#N::Color#]N::Blue
 // CHECK-CC1-NEXT: Green : [#N::Color#]N::Green
 // CHECK-CC1-NEXT: Indigo : [#N::Color#]N::Indigo
 // CHECK-CC1-NEXT: Orange : [#N::Color#]N::Orange
 // CHECK-CC1-NEXT: Violet : [#N::Color#]N::Violet
-
+  V
+// RUN: %clang_cc1 -fsyntax-only -code-completion-with-fixits -code-completion-at=%s:28:4 %s -o - | FileCheck -check-prefix=CHECK-UNQUAL %s
+// CHECK-UNQUAL: Violet : [#N::Color#]N::Violet (requires fix-it: {28:3-28:3} to "case ")
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -18,6 +18,7 @@
 #include "clang/AST/QualTypeNames.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/CharInfo.h"
+#include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/MacroInfo.h"
@@ -1984,6 +1985,81 @@
   }
 }
 
+namespace {
+/// Information that allows to avoid completing redundant enumerators.
+struct CoveredEnumerators {
+  llvm::SmallPtrSet Seen;
+  NestedNameSpecifier *SuggestedQualifier = nullptr;
+
+  // Determine which enumerators we have already seen in the switch statement.
+  // FIXME: Ideally, we would also be able to look *past* the code-completion
+  // token, in case we are code-completing in the middle of the switch and not
+  // at the end. However, we aren't able to do so at the moment.
+  static CoveredEnumerators gather(SwitchStmt *Switch) {
+CoveredEnumerators Enumerators;
+for (SwitchCase *SC = Switch->getSwitchCaseList(); SC;
+ SC = SC->getNextSwitchCase()) {
+  CaseStmt *Case = dyn_cast(SC);
+  if (!Case)
+continue;
+
+  Expr *CaseVal = Case->getLHS()->IgnoreParenCasts();
+  if (auto *DRE = dyn_cast(CaseVal))
+if (auto *Enumerator = dyn_cast(DRE->getDecl())) {
+  // We look into the AST of the case statement to determine which
+  // enumerator was named. Alternatively, we could compute the value of
+  // the integral constant expression, then compare it against the
+  // values of each enumerator. However, value-based approach would not
+  // work as well with C++ templates where enumerators declared within a
+  // template are type- and value-dependent.
+  Enumerators.Seen.insert(Enumerator);
+
+  // If this is a qualified-id, keep track of the nested-name-specifier
+  // so that we can reproduce it as part of code completion, e.g.,
+  //
+  //   switch (TagD.getKind()) {
+  // case TagDecl::TK_enum:
+  //   break;
+  // case XXX
+  //
+  // At the XXX, our completions are TagDecl::TK_union,
+  // TagDecl::TK_struct, and TagDecl::TK_class, rather than TK_union,
+  // TK_struct, and TK_class.
+  Enumerators.SuggestedQualifier = DRE->getQualifier();
+}
+}
+return Enumerators;
+  }
+};
+} // namespace
+
+static void AddEnumerators(ResultBuilder &Results, ASTContext &Context,
+   EnumDecl *Enum, DeclContext *CurContext,
+   const CoveredEnumerators &Enumerators,
+   llvm:

[PATCH] D62883: [analyzer] Track conditions of terminator statements on which the reported node depends on

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

I implemented the fixes you commented on, but during evaluation it turned out 
that my visitor eats ram for breakfast, and then goes for seconds. I mean, like 
5-30x the normal memory consumption, and the same for analysis speed. I counted 
the number of concurrent instances (incrementing a static counter in ctor, 
decrementing in dtor), and it seems to go up into the hundreds of thousands, 
even millions before restarting the counter. I killed the process before 
crippling the server.

I'm not sure how long it'll take for me to figure out what's wrong, but these 
numbers are so ridiculous, I suspect a programming error rather then an 
algorithmic issue.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62883



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

[PATCH] D63092: [Frontend] Use executable path when creating invocation from cmdline

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

In D63092#1536774 , @sammccall wrote:

> `argv[0]` does carry important information though, I think this will break a 
> lot of things. It's... concerning that no tests broke.
>
> For example, if it's `clang` or `g++` or `clang-cl` then that affects how 
> command lines are parsed (regardless of whether the path actually exists).


You're right, I can see how this change can break the driver mode detection.

> Additionally I don't think all tools that call 
> createInvocationFromCommandLine carry around all the files you want.
> 
> The wrapper tool case is a problem though, I see some options:
> 
> - change the process that generates the compilation database to emit a path 
> to the underlying compiler instead, as tooling currently expects. This may be 
> painful if e.g. goma pretends to be a regular toolchain. An argument can be 
> made it should pretend harder (e.g. symlinking directories around it so 
> discovery works as usual)

I think this is the right solution for compiler wrappers like goma, the problem 
is that this requires changing any tool that generates compilation database to 
have explicit notion of compiler launcher and exclude that from the command 
(today we just prepend the tool to the command 
).
 This includes tool like GN and potentially even Ninja which is a non-trivial 
task. Until that happens, `clangd` is unusable for projects like Fuchsia 
(`clangd` cannot find standard headers and we're getting too many warnings for 
those).

> - change the process that generates the CDB to inject the relevant directory 
> paths explicitly as flags

This would mean either duplicating the Clang driver logic inside the build 
system, which is something I'd like to avoid (why use the driver at all in that 
case?), or have some mechanism through which the driver would have to export 
all the necessary information (e.g. where to find standard library headers) for 
the build system to use.

> - make driver aware of the distinction between "invoked-as" and "current 
> binary" and use the right path in the right places

I'll look into this.

> (Incidentally if Driver is going to use the current binary, we shouldn't need 
> to pass it in as a parameter, right?)
> 
> There are some potentially-unsolvable cases here: e.g. if your wrapper is 
> `magic-remote-build` then maybe there's no standard library locally, or no 
> way to find it.

Do you think it's worth updating the compilation database documentation 
 to explicitly say 
that the command has to start with the compiler invocation? Currently it says 
that "The compile command executed." which is IMHO not sufficiently precise.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63092



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

[PATCH] D60233: [clang-scan-deps] initial outline of the tool that runs preprocessor to find dependencies over a JSON compilation database

[Alexandre Ganea via Phabricator via cfe-commits]

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



Comment at: clang/test/ClangScanDeps/Inputs/regular_cdb.json:4
+  "directory": "DIR",
+  "command": "clang -c DIR/regular_cdb.cpp -IInputs -MD -MF DIR/regular_cdb.d",
+  "file": "DIR/regular_cdb.cpp"

arphaman wrote:
> aganea wrote:
> > Is `-MD -MF` required for clang-scan-deps to work? Can't we append those 
> > arguments automatically, so that only include paths are needed in the CDB?
> > 
> > Would it possible for the tool to produce a collated file? A real-world 
> > usage would otherwise produce tens of thousands of files. Which then would 
> > be opened/parsed by the calling application.
> I agree, the tool shouldn't rely on those options being there. `-MD -MF` in 
> this test right now a crutch to get the dependencies printed somewhere.
> 
> I think the tool by default should print out the collated dependencies to 
> STDOUT instead of writing them to files that were specified for the build. 
> The `-MD -MF` options will not be required as well. This implementation 
> requires some changes in the dependency collector, which I am planning to do 
> as part of clang-scan-deps in the follow-up patches. Will it be ok to keep 
> this patch as it is right now, and transition to printing out the collated 
> dependencies without requiring the options as a follow-up?
> 
> 
Sounds good! Please cc me on the upcoming reviews, we have a good use-case for 
clang-scan-deps in [[ http://www.fastbuild.org/docs/home.html | Fastbuild ]]!



Comment at: clang/tools/clang-scan-deps/CMakeLists.txt:3
+Core
+Support
+)

Two space indent.


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

https://reviews.llvm.org/D60233



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

[PATCH] D44865: [libc++] Implement P0608R3 - A sane variant converting constructor

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

In D44865#1051228 , @EricWF wrote:

> This LGTM.
>
> Also I would like @mclow.lists input about applying this DR early since LWG
>  hasn't commented on it yet.


Even though LWG didn't vote it out as a DR; that's what it is, and I think that 
we would be doing our users a disservice by not providing this in C++17.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D44865



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

[PATCH] D63092: [Frontend] Use executable path when creating invocation from cmdline

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

One more thing, do you think it's reasonable to use 
`llvm::sys::findProgramByName(Args[0])`instead of `Args[0]` when creating the 
driver instance? One of the failure modes I ran into is the case where the 
generated compilation database would contain just the executable name, e.g. 
`clang++`. If you invoke that command, everything works as expected because 
driver resolves the binary to a full path 
,
 but when used with `clangd`, it'll fail because that resolution will never 
happen, the `Dir`/`InstalledDir` will be an empty path and attempt to resolve 
C++ library headers will end up with `/../include/c++/v1` which is invalid.

Alternative would be to make the compilation database specification even more 
strict and require that the compiler command is not only the first argument, 
but it's also a (full or relative) path to the compiler executable.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63092



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

[PATCH] D62156: [Sema][PR41730] Diagnose addr space mismatch while constructing objects

[John McCall via Phabricator via cfe-commits]

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

Okay, LGTM.




Comment at: test/SemaCXX/address-space-ctor.cpp:11
+//expected-note@-6{{candidate constructor (the implicit move constructor) not 
viable: no known conversion from 'int' to 'MyType &&' for 1st argument}}
+//expected-note@-6{{candidate constructor ignored: cannot be used to construct 
an object in address space '__attribute__((address_space(10)))'}}
+//expected-note@-8{{candidate constructor ignored: cannot be used to construct 
an object in address space '__attribute__((address_space(10)))'}}

Anastasia wrote:
> Not sure if we should change to:
>   cannot be used to construct an object with 
> '__attribute__((address_space(10)))'
> 
> Although for OpenCL it would be ok as is.
> 
> Or may be it's better to add custom printing of addr spaces?
Some sort of custom printing of address spaces would probably be best, yeah.  
Users probably think of them as whatever custom syntax/macro they normally 
write, not as an application of some weird attribute.  We do a similar sort of 
thing with vector types where we generally just print them as `float4` or 
whatever without an `aka` clause because we don't expect anyone to know about 
those attributes.


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

https://reviews.llvm.org/D62156



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

r362991 - [CUDA] Fix grep pattern in cuda-types.cu

[Hubert Tong via cfe-commits]

Author: hubert.reinterpretcast
Date: Mon Jun 10 15:28:20 2019
New Revision: 362991

URL: http://llvm.org/viewvc/llvm-project?rev=362991&view=rev
Log:
[CUDA] Fix grep pattern in cuda-types.cu

Summary:
vertical-line is not a BRE special character.

POSIX.1-2017 XBD Section 9.3.2 indicates that the interpretation of `\|`
is undefined. This patch uses EREs instead.

Additionally, the pattern is further fixed so that `SIZEOF` and `WIDTH`
macros are checked.

Reviewers: jlebar, daltenty, xingxue, jasonliu, tra

Reviewed By: tra

Subscribers: jfb, jsji, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/test/Preprocessor/cuda-types.cu

Modified: cfe/trunk/test/Preprocessor/cuda-types.cu
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/cuda-types.cu?rev=362991&r1=362990&r2=362991&view=diff
==
--- cfe/trunk/test/Preprocessor/cuda-types.cu (original)
+++ cfe/trunk/test/Preprocessor/cuda-types.cu Mon Jun 10 15:28:20 2019
@@ -8,41 +8,41 @@
 // RUN: mkdir -p %t
 
 // RUN: %clang --cuda-host-only -nocudainc -target i386-unknown-linux-gnu -x 
cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/i386-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
i386-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/i386-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-device-defines-filtered
 // RUN: diff %t/i386-host-defines-filtered %t/i386-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target x86_64-unknown-linux-gnu -x 
cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/x86_64-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/x86_64-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
x86_64-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/x86_64-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/x86_64-device-defines-filtered
 // RUN: diff %t/x86_64-host-defines-filtered %t/x86_64-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target powerpc64-unknown-linux-gnu 
-x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/powerpc64-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/powerpc64-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
powerpc64-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > 
%t/powerpc64-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > 
%t/powerpc64-device-defines-filtered
 // RUN: diff %t/powerpc64-host-defines-filtered 
%t/powerpc64-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target i386-windows-msvc -x cuda 
-E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/i386-msvc-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-msvc-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
i386-windows-msvc -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > 
%t/i386-msvc-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > 
%t/i386-msvc-device-defines-filtered
 // RUN: diff %t/i386-msvc-host-defines-filtered 
%t/i386-msvc-devi

[PATCH] D63029: [CUDA] Fix grep pattern in cuda-types.cu

[Hubert Tong via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
hubert.reinterpretcast marked an inline comment as done.
Closed by commit rL362991: [CUDA] Fix grep pattern in cuda-types.cu (authored 
by hubert.reinterpretcast, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63029

Files:
  cfe/trunk/test/Preprocessor/cuda-types.cu


Index: cfe/trunk/test/Preprocessor/cuda-types.cu
===
--- cfe/trunk/test/Preprocessor/cuda-types.cu
+++ cfe/trunk/test/Preprocessor/cuda-types.cu
@@ -8,41 +8,41 @@
 // RUN: mkdir -p %t
 
 // RUN: %clang --cuda-host-only -nocudainc -target i386-unknown-linux-gnu -x 
cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/i386-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
i386-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/i386-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-device-defines-filtered
 // RUN: diff %t/i386-host-defines-filtered %t/i386-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target x86_64-unknown-linux-gnu -x 
cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/x86_64-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/x86_64-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
x86_64-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/x86_64-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/x86_64-device-defines-filtered
 // RUN: diff %t/x86_64-host-defines-filtered %t/x86_64-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target powerpc64-unknown-linux-gnu 
-x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/powerpc64-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/powerpc64-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
powerpc64-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > 
%t/powerpc64-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > 
%t/powerpc64-device-defines-filtered
 // RUN: diff %t/powerpc64-host-defines-filtered 
%t/powerpc64-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target i386-windows-msvc -x cuda 
-E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/i386-msvc-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-msvc-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
i386-windows-msvc -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > 
%t/i386-msvc-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > 
%t/i386-msvc-device-defines-filtered
 // RUN: diff %t/i386-msvc-host-defines-filtered 
%t/i386-msvc-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target x86_64-windows-msvc -x cuda 
-E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > 
%t/x86_64-msvc-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GC

[PATCH] D44865: [libc++] Implement P0608R3 - A sane variant converting constructor

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

The patch looks fine to me, but I think you should consider making a couple of 
`T.fail.cpp` tests, and check to make sure you get the "right error".




Comment at: 
test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp:130
   {
 using V = std::variant;
 static_assert(!std::is_assignable::value, "ambiguous");

If you really want to check that these are "ambiguous" , or "no matching 
operator=", etc, the way to do that is to define a fail.cpp test, and check the 
error messages.
 


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D44865



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

r362992 - [PlistSupport] Produce a newline to end plist output files

[Hubert Tong via cfe-commits]

Author: hubert.reinterpretcast
Date: Mon Jun 10 15:30:57 2019
New Revision: 362992

URL: http://llvm.org/viewvc/llvm-project?rev=362992&view=rev
Log:
[PlistSupport] Produce a newline to end plist output files

Summary:
As suggested in the review of D62949, this patch updates the plist
output to have a newline at the end of the file. This makes it so that
the plist output file qualifies as a POSIX text file, which increases
the consumability of the generated plist file in relation to various
tools.

Reviewers: NoQ, sfertile, xingxue, jasonliu, daltenty

Reviewed By: NoQ, xingxue

Subscribers: jsji, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/ARCMigrate/PlistReporter.cpp
cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp

Modified: cfe/trunk/lib/ARCMigrate/PlistReporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ARCMigrate/PlistReporter.cpp?rev=362992&r1=362991&r2=362992&view=diff
==
--- cfe/trunk/lib/ARCMigrate/PlistReporter.cpp (original)
+++ cfe/trunk/lib/ARCMigrate/PlistReporter.cpp Mon Jun 10 15:30:57 2019
@@ -120,5 +120,5 @@ void arcmt::writeARCDiagsToPlist(const s
   o << " \n";
 
   // Finish.
-  o << "\n";
+  o << "\n\n";
 }

Modified: cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp?rev=362992&r1=362991&r2=362992&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp Mon Jun 10 15:30:57 
2019
@@ -748,7 +748,7 @@ void PlistDiagnostics::FlushDiagnosticsI
   }
 
   // Finish.
-  o << "\n";
+  o << "\n\n";
 }
 
 
//===--===//


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

[PATCH] D63041: [PlistSupport] Produce a newline to end plist output files

[Hubert Tong via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362992: [PlistSupport] Produce a newline to end plist output 
files (authored by hubert.reinterpretcast, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63041

Files:
  cfe/trunk/lib/ARCMigrate/PlistReporter.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp


Index: cfe/trunk/lib/ARCMigrate/PlistReporter.cpp
===
--- cfe/trunk/lib/ARCMigrate/PlistReporter.cpp
+++ cfe/trunk/lib/ARCMigrate/PlistReporter.cpp
@@ -120,5 +120,5 @@
   o << " \n";
 
   // Finish.
-  o << "\n";
+  o << "\n\n";
 }
Index: cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -748,7 +748,7 @@
   }
 
   // Finish.
-  o << "\n";
+  o << "\n\n";
 }
 
 
//===--===//


Index: cfe/trunk/lib/ARCMigrate/PlistReporter.cpp
===
--- cfe/trunk/lib/ARCMigrate/PlistReporter.cpp
+++ cfe/trunk/lib/ARCMigrate/PlistReporter.cpp
@@ -120,5 +120,5 @@
   o << " \n";
 
   // Finish.
-  o << "\n";
+  o << "\n\n";
 }
Index: cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -748,7 +748,7 @@
   }
 
   // Finish.
-  o << "\n";
+  o << "\n\n";
 }
 
 //===--===//
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r362994 - [analyzer][tests] Add normalize_plist to replace diff_plist

[Hubert Tong via cfe-commits]

Author: hubert.reinterpretcast
Date: Mon Jun 10 15:33:34 2019
New Revision: 362994

URL: http://llvm.org/viewvc/llvm-project?rev=362994&view=rev
Log:
[analyzer][tests] Add normalize_plist to replace diff_plist

Summary:
The `%diff_plist` lit substitution invokes `diff` with a non-portable
`-I` option. The intended effect can be achieved by normalizing the
inputs to `diff` beforehand. Such normalization can be done with
`grep -Ev`, which is also used by other tests.

This patch applies the change (adjusted for review comments) described
in http://lists.llvm.org/pipermail/cfe-dev/2019-April/061904.html to the
specific case shown in the list message. Mechanical changes to the other
affected files will follow in later patches.

Reviewers: NoQ, sfertile, xingxue, jasonliu, daltenty

Reviewed By: NoQ

Subscribers: xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, 
Szelethus, donat.nagy, dkrupp, Charusso, jsji, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/test/Analysis/lit.local.cfg
cfe/trunk/test/Analysis/unix-fns.c

Modified: cfe/trunk/test/Analysis/lit.local.cfg
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/lit.local.cfg?rev=362994&r1=362993&r2=362994&view=diff
==
--- cfe/trunk/test/Analysis/lit.local.cfg (original)
+++ cfe/trunk/test/Analysis/lit.local.cfg Mon Jun 10 15:33:34 2019
@@ -14,6 +14,14 @@ config.test_format = analyzer_test.Analy
 config.substitutions.append(('%diff_plist',
 'diff -u -w -I "/" -I ".:" -I "version"'))
 
+# Filtering command used by Clang Analyzer tests (when comparing .plist files
+# with reference output)
+config.substitutions.append(('%normalize_plist',
+"grep -Ev '%s|%s|%s'" %
+('^[[:space:]]*.* version .*$',
+ '^[[:space:]]*/.*$',
+ '^[[:space:]]*.:.*$')))
+
 # Diff command for testing SARIF output to reference output.
 config.substitutions.append(('%diff_sarif',
 '''diff -U1 -w -I ".*file:.*%basename_t" -I '"version":' -I 
"2\.0\.0\-csd\.[0-9]*\.beta\."'''))

Modified: cfe/trunk/test/Analysis/unix-fns.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/unix-fns.c?rev=362994&r1=362993&r2=362994&view=diff
==
--- cfe/trunk/test/Analysis/unix-fns.c (original)
+++ cfe/trunk/test/Analysis/unix-fns.c Mon Jun 10 15:33:34 2019
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 
-analyzer-checker=core,unix.API,osx.API,optin.portability %s 
-analyzer-store=region -analyzer-output=plist -analyzer-config faux-bodies=true 
 -fblocks -verify -o %t.plist
-// RUN: cat %t.plist | %diff_plist %S/Inputs/expected-plists/unix-fns.c.plist -
+// RUN: %normalize_plist <%t.plist | diff -u 
%S/Inputs/expected-plists/unix-fns.c.plist -
 // RUN: mkdir -p %t.dir
 // RUN: %clang_analyze_cc1 
-analyzer-checker=core,unix.API,osx.API,optin.portability -analyzer-output=html 
-analyzer-config faux-bodies=true -fblocks -o %t.dir %s
 // RUN: rm -fR %t.dir


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

[PATCH] D62949: [analyzer][tests] Add normalize_plist to replace diff_plist

[Hubert Tong via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362994: [analyzer][tests] Add normalize_plist to replace 
diff_plist (authored by hubert.reinterpretcast, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62949

Files:
  cfe/trunk/test/Analysis/lit.local.cfg
  cfe/trunk/test/Analysis/unix-fns.c


Index: cfe/trunk/test/Analysis/unix-fns.c
===
--- cfe/trunk/test/Analysis/unix-fns.c
+++ cfe/trunk/test/Analysis/unix-fns.c
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 
-analyzer-checker=core,unix.API,osx.API,optin.portability %s 
-analyzer-store=region -analyzer-output=plist -analyzer-config faux-bodies=true 
 -fblocks -verify -o %t.plist
-// RUN: cat %t.plist | %diff_plist %S/Inputs/expected-plists/unix-fns.c.plist -
+// RUN: %normalize_plist <%t.plist | diff -u 
%S/Inputs/expected-plists/unix-fns.c.plist -
 // RUN: mkdir -p %t.dir
 // RUN: %clang_analyze_cc1 
-analyzer-checker=core,unix.API,osx.API,optin.portability -analyzer-output=html 
-analyzer-config faux-bodies=true -fblocks -o %t.dir %s
 // RUN: rm -fR %t.dir
Index: cfe/trunk/test/Analysis/lit.local.cfg
===
--- cfe/trunk/test/Analysis/lit.local.cfg
+++ cfe/trunk/test/Analysis/lit.local.cfg
@@ -14,6 +14,14 @@
 config.substitutions.append(('%diff_plist',
 'diff -u -w -I "/" -I ".:" -I "version"'))
 
+# Filtering command used by Clang Analyzer tests (when comparing .plist files
+# with reference output)
+config.substitutions.append(('%normalize_plist',
+"grep -Ev '%s|%s|%s'" %
+('^[[:space:]]*.* version .*$',
+ '^[[:space:]]*/.*$',
+ '^[[:space:]]*.:.*$')))
+
 # Diff command for testing SARIF output to reference output.
 config.substitutions.append(('%diff_sarif',
 '''diff -U1 -w -I ".*file:.*%basename_t" -I '"version":' -I 
"2\.0\.0\-csd\.[0-9]*\.beta\."'''))


Index: cfe/trunk/test/Analysis/unix-fns.c
===
--- cfe/trunk/test/Analysis/unix-fns.c
+++ cfe/trunk/test/Analysis/unix-fns.c
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,unix.API,osx.API,optin.portability %s -analyzer-store=region -analyzer-output=plist -analyzer-config faux-bodies=true  -fblocks -verify -o %t.plist
-// RUN: cat %t.plist | %diff_plist %S/Inputs/expected-plists/unix-fns.c.plist -
+// RUN: %normalize_plist <%t.plist | diff -u %S/Inputs/expected-plists/unix-fns.c.plist -
 // RUN: mkdir -p %t.dir
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.API,osx.API,optin.portability -analyzer-output=html -analyzer-config faux-bodies=true -fblocks -o %t.dir %s
 // RUN: rm -fR %t.dir
Index: cfe/trunk/test/Analysis/lit.local.cfg
===
--- cfe/trunk/test/Analysis/lit.local.cfg
+++ cfe/trunk/test/Analysis/lit.local.cfg
@@ -14,6 +14,14 @@
 config.substitutions.append(('%diff_plist',
 'diff -u -w -I "/" -I ".:" -I "version"'))
 
+# Filtering command used by Clang Analyzer tests (when comparing .plist files
+# with reference output)
+config.substitutions.append(('%normalize_plist',
+"grep -Ev '%s|%s|%s'" %
+('^[[:space:]]*.* version .*$',
+ '^[[:space:]]*/.*$',
+ '^[[:space:]]*.:.*$')))
+
 # Diff command for testing SARIF output to reference output.
 config.substitutions.append(('%diff_sarif',
 '''diff -U1 -w -I ".*file:.*%basename_t" -I '"version":' -I "2\.0\.0\-csd\.[0-9]*\.beta\."'''))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63101: [Frontend] SetUpDiagnosticLog should handle unowned diagnostic consumer in the compiler

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a subscriber: cfe-commits.
arphaman added a comment.

+ cue-commits


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

https://reviews.llvm.org/D63101



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

[PATCH] D63101: [Frontend] SetUpDiagnosticLog should handle unowned diagnostic consumer in the compiler

[Michael Spencer via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D63101



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

r362996 - [analyzer][tests] Use normalize_plist in place of diff_plist (`cat` cases)

[Hubert Tong via cfe-commits]

Author: hubert.reinterpretcast
Date: Mon Jun 10 15:37:31 2019
New Revision: 362996

URL: http://llvm.org/viewvc/llvm-project?rev=362996&view=rev
Log:
[analyzer][tests] Use normalize_plist in place of diff_plist (`cat` cases)

Summary:
The `%diff_plist` lit substitution invokes `diff` with a non-portable
`-I` option. The intended effect can be achieved by normalizing the
inputs to `diff` beforehand. Such normalization can be done with
`grep -Ev`, which is also used by other tests.

This patch applies the change (adjusted for review comments) described
in http://lists.llvm.org/pipermail/cfe-dev/2019-April/061904.html
mechanically to the cases where the output file is piped to
`%diff_plist` via `cat`.

The changes were applied via a script, except that
`clang/test/Analysis/NewDelete-path-notes.cpp` and
`clang/test/Analysis/plist-macros-with-expansion.cpp` were each adjusted
for the line-continuation on the relevant `RUN` step.

Reviewers: NoQ, sfertile, xingxue, jasonliu, daltenty

Subscribers: xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, 
Szelethus, donat.nagy, dkrupp, Charusso, jsji, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/test/Analysis/NewDelete-path-notes.cpp
cfe/trunk/test/Analysis/conditional-path-notes.c
cfe/trunk/test/Analysis/copypaste/plist-diagnostics-notes-as-events.cpp
cfe/trunk/test/Analysis/copypaste/plist-diagnostics.cpp
cfe/trunk/test/Analysis/cxx-for-range.cpp
cfe/trunk/test/Analysis/diagnostics/deref-track-symbolic-region.c
cfe/trunk/test/Analysis/diagnostics/report-issues-within-main-file.cpp
cfe/trunk/test/Analysis/diagnostics/undef-value-caller.c
cfe/trunk/test/Analysis/diagnostics/undef-value-param.c
cfe/trunk/test/Analysis/diagnostics/undef-value-param.m
cfe/trunk/test/Analysis/edges-new.mm
cfe/trunk/test/Analysis/generics.m
cfe/trunk/test/Analysis/inline-plist.c
cfe/trunk/test/Analysis/inline-unique-reports.c
cfe/trunk/test/Analysis/inlining/eager-reclamation-path-notes.c
cfe/trunk/test/Analysis/inlining/eager-reclamation-path-notes.cpp
cfe/trunk/test/Analysis/inlining/path-notes.c
cfe/trunk/test/Analysis/inlining/path-notes.cpp
cfe/trunk/test/Analysis/inlining/path-notes.m
cfe/trunk/test/Analysis/method-call-path-notes.cpp
cfe/trunk/test/Analysis/model-file.cpp
cfe/trunk/test/Analysis/null-deref-path-notes.m
cfe/trunk/test/Analysis/nullability-notes.m
cfe/trunk/test/Analysis/objc-arc.m
cfe/trunk/test/Analysis/objc-radar17039661.m
cfe/trunk/test/Analysis/plist-macros-with-expansion.cpp
cfe/trunk/test/Analysis/plist-macros.cpp
cfe/trunk/test/Analysis/plist-output-alternate.m
cfe/trunk/test/Analysis/plist-output.m
cfe/trunk/test/Analysis/retain-release-path-notes.m
cfe/trunk/test/Analysis/retain-release.m

Modified: cfe/trunk/test/Analysis/NewDelete-path-notes.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/NewDelete-path-notes.cpp?rev=362996&r1=362995&r2=362996&view=diff
==
--- cfe/trunk/test/Analysis/NewDelete-path-notes.cpp (original)
+++ cfe/trunk/test/Analysis/NewDelete-path-notes.cpp Mon Jun 10 15:37:31 2019
@@ -11,7 +11,7 @@
 // RUN:   -analyzer-checker=cplusplus.NewDelete,unix.Malloc \
 // RUN:   -analyzer-config add-pop-up-notes=false \
 // RUN:   -analyzer-output=plist %s -o %t.plist
-// RUN: cat %t.plist | %diff_plist \
+// RUN: %normalize_plist <%t.plist | diff -u \
 // RUN:   %S/Inputs/expected-plists/NewDelete-path-notes.cpp.plist -
 
 void test() {

Modified: cfe/trunk/test/Analysis/conditional-path-notes.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/conditional-path-notes.c?rev=362996&r1=362995&r2=362996&view=diff
==
--- cfe/trunk/test/Analysis/conditional-path-notes.c (original)
+++ cfe/trunk/test/Analysis/conditional-path-notes.c Mon Jun 10 15:37:31 2019
@@ -1,6 +1,6 @@
 // RUN: %clang_analyze_cc1 %s -analyzer-checker=core.NullDereference 
-analyzer-output=text -verify
 // RUN: %clang_analyze_cc1 %s -analyzer-checker=core.NullDereference 
-analyzer-output=plist -o %t
-// RUN: cat %t | %diff_plist 
%S/Inputs/expected-plists/conditional-path-notes.c.plist -
+// RUN: %normalize_plist <%t | diff -u 
%S/Inputs/expected-plists/conditional-path-notes.c.plist -
 
 void testCondOp(int *p) {
   int *x = p ? p : p;

Modified: 
cfe/trunk/test/Analysis/copypaste/plist-diagnostics-notes-as-events.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/copypaste/plist-diagnostics-notes-as-events.cpp?rev=362996&r1=362995&r2=362996&view=diff
==
--- cfe/trunk/test/Analysis/copypaste/plist-diagnostics-notes-as-events.cpp 
(original)
+++ cfe/trunk/test/Analysis/copypaste/plist-diagnostics-notes-as-event