[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

[Andrew via Phabricator via cfe-commits]

browneee added a comment.

In D77621#1968437 , @dexonsmith wrote:

> This is thanks to a commit of mine that shaved a word off of `SmallVector`.  
> Some options to consider:
>
> 1. Revert to word-size integers (`size_t`? `uintptr_t`?) for Size and 
> Capacity for small-enough types.  Could be just if `sizeof(T)==1`.  Or maybe 
> just for `char` and `unsigned char`.
> 2. Revert my patch entirely and go back to words (these used to be `void*`).
> 3. (Your patch, stop using `SmallVector`.)
>
>   I think I would prefer some variation of (1) over (3).


Hi Duncan, thanks for raising these alternatives.

Links to your prior commit for context: Review 
, Commit 


I agree any of these options would solve the issue I'm experiencing.

Option 1:

- I think SmallVectorBase would need to become templated.
- The size related code would need support to two sets of edge cases.
- The varying capacity may be surprising, and adds another variation to both 
both small mode and heap mode.

Option 3:

- This patch is somewhat widespread. A more localized fix may be desirable.
- Some inconvenience of an API change for downstream.

Do we want to increase the complexity of SmallVector somewhat? or do we want to 
keep the limit and affirm SmallVector is for small things?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77621



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

[PATCH] D75677: [Analyzer] Only add iterator note tags to the operations of the affected iterators

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:541-542
+BR.markInteresting(It1);
+if (const auto  = It1.getAs()) {
+  BR.markInteresting(LCV1->getRegion());
+}

baloghadamsoftware wrote:
> baloghadamsoftware wrote:
> > baloghadamsoftware wrote:
> > > baloghadamsoftware wrote:
> > > > Szelethus wrote:
> > > > > NoQ wrote:
> > > > > > baloghadamsoftware wrote:
> > > > > > > NoQ wrote:
> > > > > > > > I'm opposed to this code for the same reason that i'm opposed 
> > > > > > > > to it in the debug checker. Parent region is an undocumented 
> > > > > > > > implementation detail of `RegionStore`. It is supposed to be 
> > > > > > > > immaterial to the user. You should not rely on its exact value.
> > > > > > > > 
> > > > > > > > @baloghadamsoftware Can we eliminate all such code from 
> > > > > > > > iterator checkers and instead identify all iterators by regions 
> > > > > > > > in which they're stored? Does my improved C++ support help with 
> > > > > > > > this anyhow whenever it kicks in?
> > > > > > > How to find the region where it is stored? I am open to find 
> > > > > > > better solutions, but it was the only one I could find so far. If 
> > > > > > > we ignore `LazyCompoundVal` then everything falls apart, we can 
> > > > > > > remove all the iterator-related checkers.
> > > > > > It's the region from which you loaded it. If you obtained it as 
> > > > > > `Call.getArgSVal()` then it's the parameter region for the call. If 
> > > > > > you obtained it as `Call.getReturnValue()` then it's the target 
> > > > > > region can be obtained by looking at the //construction context// 
> > > > > > for the call.
> > > > > `LazyCompoundVal` and friends seem to be a constantly emerging 
> > > > > headache for almost everyone. For how long I've spent in the 
> > > > > analyzer, and have religiously studied conversations and your 
> > > > > workbook about it, I still feel anxious whenever it comes up.
> > > > > 
> > > > > It would be great to have this documented in the code sometime.
> > > > Do you mean `CallEvent::getParameterLocation()` for arguments? What is 
> > > > the //construction context// for the call? How can it be obtained?
> > > I do not know exactly how many place `LazyCompoundVal`  appears, but one 
> > > place for sure is in `MaterializeTemporaryExpr::getSubExpr()`. What to 
> > > use there instead?
> > I also get it in the `Val` parameter of `checkBind()`.
> Now I spent a whole day in vain. You probably mean 
> `ExprEngine::getObjectUnderConstruction()`, (which takes 
> `ConstructionContextItem` as its argument) but it turned out that there are 
> no objects under construction in `checkPostCall()`. (Stack dump says 
> `constructing_objects` as `null`.) It seems that the //only working 
> solution// is the current one. I am not opposed to find better working 
> solutions, but we cannot spend months to completely rewrite parts of the 
> analyzer for such a simple patch. And note tags are definitely needed for 
> iterator checkers.
"A whole day"? "One simple patch"? Give me a break.

We've been discussing this problem since your very first implementation of the 
iterator checker dozens of patches ago, and i spent //six months// of my full 
time work trying to make this part of the analyzer operate in an obvious, 
principled manner, even made a dev meeting talk about it in order to explain 
how it works. And all you do is keep insisting that your solution is "working" 
even though literally nobody understands how, even you.

Out of all the contributors who bring patches to me every day, only @Szelethus 
is actively addressing the technical debt. This is not "one simple patch". This 
has to stop at some point, and i expect you, being a fairly senior contributor 
at this point, to put at least a slight effort into good engineering practices, 
apply the necessary amount of critical thinking, take basic responsibility for 
your code.

I don't mind if you address this issue immediately after this patch if you 
urgently need this patch landed. But i wouldn't like this to go on forever.

> dump says `constructing_objects` as `null`

You shouldn't be spending the whole day before noticing it. Whenever something 
isn't working, the first thing you do should be dump the ExplodedGraph and look 
at it. You would have noticed it right away.

Was the object never there or was it removed too early? If there are no objects 
under construction tracked but you need them tracked, make them tracked for as 
long as you need. That's the whole point of the objects-under-construction map.


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

https://reviews.llvm.org/D75677



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

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

[Andrew via Phabricator via cfe-commits]

browneee added inline comments.



Comment at: llvm/include/llvm/Bitstream/BitstreamWriter.h:19
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"

RKSimon wrote:
> Can this be dropped?
It is still used to construct records (line 512).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77621



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

[PATCH] D77290: [OpenMP] Specialize OpenMP calls after template instantiation

[Johannes Doerfert via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGeb5a16efbf59: [OpenMP] Specialize OpenMP calls after 
template instantiation (authored by jdoerfert).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77290

Files:
  clang/lib/Sema/TreeTransform.h
  clang/test/AST/ast-dump-openmp-begin-declare-variant_template_1.cpp

Index: clang/test/AST/ast-dump-openmp-begin-declare-variant_template_1.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-openmp-begin-declare-variant_template_1.cpp
@@ -0,0 +1,170 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fopenmp -verify -ast-dump %s   | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fopenmp -verify -ast-dump %s -x c++| FileCheck %s
+// expected-no-diagnostics
+
+int also_before() {
+  return 1;
+}
+
+#pragma omp begin declare variant match(implementation={vendor(score(100):llvm)})
+int also_after(void) {
+  return 2;
+}
+int also_after(int) {
+  return 3;
+}
+int also_after(double) {
+  return 0;
+}
+#pragma omp end declare variant
+#pragma omp begin declare variant match(implementation={vendor(score(0):llvm)})
+int also_before() {
+  return 0;
+}
+#pragma omp end declare variant
+
+int also_after(void) {
+  return 4;
+}
+int also_after(int) {
+  return 5;
+}
+int also_after(double) {
+  return 6;
+}
+
+template
+int test1() {
+  // Should return 0.
+  return also_after(T(0));
+}
+
+typedef int(*Ty)();
+
+template
+int test2() {
+  // Should return 0.
+  return fn();
+}
+
+int test() {
+  // Should return 0.
+  return test1() + test2();
+}
+
+// CHECK:  |-FunctionDecl [[ADDR_0:0x[a-z0-9]*]] <{{.*}}, line:7:1> line:5:5 used also_before 'int ({{.*}})'
+// CHECK-NEXT: | |-CompoundStmt [[ADDR_1:0x[a-z0-9]*]] 
+// CHECK-NEXT: | | `-ReturnStmt [[ADDR_2:0x[a-z0-9]*]] 
+// CHECK-NEXT: | |   `-IntegerLiteral [[ADDR_3:0x[a-z0-9]*]]  'int' 1
+// CHECK-NEXT: | `-OMPDeclareVariantAttr [[ADDR_4:0x[a-z0-9]*]] <> Implicit implementation={vendor(score(0): llvm)}
+// CHECK-NEXT: |   `-DeclRefExpr [[ADDR_5:0x[a-z0-9]*]]  'int ({{.*}})' Function [[ADDR_6:0x[a-z0-9]*]] 'also_before[implementation={vendor(llvm)}]' 'int ({{.*}})'
+// CHECK-NEXT: |-FunctionDecl [[ADDR_7:0x[a-z0-9]*]]  col:5 implicit also_after 'int ({{.*}})'
+// CHECK-NEXT: | `-OMPDeclareVariantAttr [[ADDR_8:0x[a-z0-9]*]] <> Implicit implementation={vendor(score(100): llvm)}
+// CHECK-NEXT: |   `-DeclRefExpr [[ADDR_9:0x[a-z0-9]*]]  'int ({{.*}})' Function [[ADDR_10:0x[a-z0-9]*]] 'also_after[implementation={vendor(llvm)}]' 'int ({{.*}})'
+// CHECK-NEXT: |-FunctionDecl [[ADDR_10]]  line:10:1 also_after[implementation={vendor(llvm)}] 'int ({{.*}})'
+// CHECK-NEXT: | `-CompoundStmt [[ADDR_11:0x[a-z0-9]*]] 
+// CHECK-NEXT: |   `-ReturnStmt [[ADDR_12:0x[a-z0-9]*]] 
+// CHECK-NEXT: | `-IntegerLiteral [[ADDR_13:0x[a-z0-9]*]]  'int' 2
+// CHECK-NEXT: |-FunctionDecl [[ADDR_14:0x[a-z0-9]*]]  col:5 implicit also_after 'int (int)'
+// CHECK-NEXT: | |-ParmVarDecl [[ADDR_15:0x[a-z0-9]*]]  col:19 'int'
+// CHECK-NEXT: | `-OMPDeclareVariantAttr [[ADDR_16:0x[a-z0-9]*]] <> Implicit implementation={vendor(score(100): llvm)}
+// CHECK-NEXT: |   `-DeclRefExpr [[ADDR_17:0x[a-z0-9]*]]  'int (int)' Function [[ADDR_18:0x[a-z0-9]*]] 'also_after[implementation={vendor(llvm)}]' 'int (int)'
+// CHECK-NEXT: |-FunctionDecl [[ADDR_18]]  line:13:1 also_after[implementation={vendor(llvm)}] 'int (int)'
+// CHECK-NEXT: | |-ParmVarDecl [[ADDR_15]]  col:19 'int'
+// CHECK-NEXT: | `-CompoundStmt [[ADDR_19:0x[a-z0-9]*]] 
+// CHECK-NEXT: |   `-ReturnStmt [[ADDR_20:0x[a-z0-9]*]] 
+// CHECK-NEXT: | `-IntegerLiteral [[ADDR_21:0x[a-z0-9]*]]  'int' 3
+// CHECK-NEXT: |-FunctionDecl [[ADDR_22:0x[a-z0-9]*]]  col:5 implicit used also_after 'int (double)'
+// CHECK-NEXT: | |-ParmVarDecl [[ADDR_23:0x[a-z0-9]*]]  col:22 'double'
+// CHECK-NEXT: | `-OMPDeclareVariantAttr [[ADDR_24:0x[a-z0-9]*]] <> Implicit implementation={vendor(score(100): llvm)}
+// CHECK-NEXT: |   `-DeclRefExpr [[ADDR_25:0x[a-z0-9]*]]  'int (double)' Function [[ADDR_26:0x[a-z0-9]*]] 'also_after[implementation={vendor(llvm)}]' 'int (double)'
+// CHECK-NEXT: |-FunctionDecl [[ADDR_26]]  line:16:1 also_after[implementation={vendor(llvm)}] 'int (double)'
+// CHECK-NEXT: | |-ParmVarDecl [[ADDR_23]]  col:22 'double'
+// CHECK-NEXT: | `-CompoundStmt [[ADDR_27:0x[a-z0-9]*]] 
+// CHECK-NEXT: |   `-ReturnStmt [[ADDR_28:0x[a-z0-9]*]] 
+// CHECK-NEXT: | `-IntegerLiteral [[ADDR_29:0x[a-z0-9]*]]  'int' 0
+// CHECK-NEXT: |-FunctionDecl [[ADDR_6]]  line:21:1 also_before[implementation={vendor(llvm)}] 'int ({{.*}})'
+// CHECK-NEXT: | `-CompoundStmt [[ADDR_30:0x[a-z0-9]*]] 
+// CHECK-NEXT: |   `-ReturnStmt [[ADDR_31:0x[a-z0-9]*]] 
+// CHECK-NEXT: | `-IntegerLiteral [[ADDR_32:0x[a-z0-9]*]]  'int' 0
+// CHECK-NEXT: |-FunctionDecl [[ADDR_33:0x[a-z0-9]*]] prev [[ADDR_7]]  line:26:5 also_after 'int 

[PATCH] D75788: [OpenMP] Provide math functions in OpenMP device code via OpenMP variants

[Johannes Doerfert via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf85ae058f580: [OpenMP] Provide math functions in OpenMP 
device code via OpenMP variants (authored by jdoerfert).

Changed prior to commit:
  https://reviews.llvm.org/D75788?vs=255050=255897#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75788

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/__clang_cuda_cmath.h
  clang/lib/Headers/__clang_cuda_device_functions.h
  clang/lib/Headers/__clang_cuda_math.h
  clang/lib/Headers/__clang_cuda_math_forward_declares.h
  clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h
  clang/lib/Headers/openmp_wrappers/__clang_openmp_math.h
  clang/lib/Headers/openmp_wrappers/__clang_openmp_math_declares.h
  clang/lib/Headers/openmp_wrappers/cmath
  clang/lib/Headers/openmp_wrappers/math.h
  clang/lib/Headers/openmp_wrappers/time.h
  clang/test/Headers/Inputs/include/climits
  clang/test/Headers/Inputs/include/cmath
  clang/test/Headers/Inputs/include/cstdlib
  clang/test/Headers/Inputs/include/math.h
  clang/test/Headers/Inputs/include/stdlib.h
  clang/test/Headers/nvptx_device_cmath_functions.c
  clang/test/Headers/nvptx_device_cmath_functions.cpp
  clang/test/Headers/nvptx_device_cmath_functions_cxx17.cpp
  clang/test/Headers/nvptx_device_math_complex.c
  clang/test/Headers/nvptx_device_math_functions.c
  clang/test/Headers/nvptx_device_math_functions.cpp
  clang/test/Headers/nvptx_device_math_functions_cxx17.cpp
  clang/test/Headers/nvptx_device_math_macro.cpp
  clang/test/Headers/nvptx_device_math_modf.cpp
  clang/test/Headers/nvptx_device_math_sin.c
  clang/test/Headers/nvptx_device_math_sin.cpp
  clang/test/Headers/nvptx_device_math_sin_cos.cpp
  clang/test/Headers/nvptx_device_math_sincos.cpp

Index: clang/test/Headers/nvptx_device_math_sincos.cpp
===
--- /dev/null
+++ clang/test/Headers/nvptx_device_math_sincos.cpp
@@ -0,0 +1,58 @@
+// REQUIRES: nvptx-registered-target
+// RUN: %clang_cc1 -internal-isystem %S/Inputs/include -fopenmp -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
+// RUN: %clang_cc1 -internal-isystem %S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h -internal-isystem %S/Inputs/include -fopenmp -triple nvptx64-nvidia-cuda -aux-triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
+
+#include 
+
+// 4 calls to sincos(f), all translated to __nv_sincos calls:
+
+// CHECK-NOT: _Z.sincos
+// CHECK: call void @__nv_sincos(double
+// CHECK-NOT: _Z.sincos
+// CHECK: call void @__nv_sincosf(float
+// CHECK-NOT: _Z.sincos
+// CHECK: call void @__nv_sincos(double
+// CHECK-NOT: _Z.sincos
+// CHECK: call void @__nv_sincosf(float
+// CHECK-NOT: _Z.sincos
+
+// single precision wrapper
+inline void sincos(float x, float* __restrict__ sin, float* __restrict__ cos)
+{
+  sincosf(x, sin, cos);
+}
+
+template
+void test_sincos(T x)
+{
+  T res_sin, res_cos;
+
+  #pragma omp target map(from: res_sin, res_cos)
+  {
+sincos(x, _sin, _cos);
+  }
+
+}
+
+int main(int argc, char **argv)
+{
+
+#if !defined(C_ONLY)
+  test_sincos(0.0);
+  test_sincos(0.0);
+#endif
+
+  #pragma omp target
+  {
+double s, c;
+sincos(0, , );
+  }
+
+  #pragma omp target
+  {
+float s, c;
+sincosf(0.f, , );
+  }
+
+  return 0;
+}
Index: clang/test/Headers/nvptx_device_math_sin_cos.cpp
===
--- /dev/null
+++ clang/test/Headers/nvptx_device_math_sin_cos.cpp
@@ -0,0 +1,63 @@
+// REQUIRES: nvptx-registered-target
+// RUN: %clang_cc1 -internal-isystem %S/Inputs/include -fopenmp -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
+// RUN: %clang_cc1 -internal-isystem %S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h -internal-isystem %S/Inputs/include -fopenmp -triple nvptx64-nvidia-cuda -aux-triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
+
+#include 
+
+// 6 calls to sin/cos(f), all translated to __nv_sin/__nv_cos calls:
+
+// CHECK-NOT: _Z.sin
+// CHECK-NOT: _Z.cos
+// CHECK: call double @__nv_sin(double
+// CHECK-NOT: _Z.sin
+// CHECK-NOT: _Z.cos
+// CHECK: call float @__nv_sinf(float
+// CHECK-NOT: _Z.sin
+// CHECK-NOT: _Z.cos
+// CHECK: call double @__nv_sin(double
+// CHECK-NOT: _Z.sin
+// CHECK-NOT: _Z.cos
+// CHECK: call double @__nv_cos(double
+// CHECK-NOT: _Z.sin
+// CHECK-NOT: _Z.cos
+// CHECK: call float @__nv_sinf(float
+// CHECK-NOT: _Z.sin
+// CHECK-NOT: _Z.cos
+// CHECK: call float 

[PATCH] D77414: [OpenMP] Add match_{all,any,none} declare variant selector extensions.

[Johannes Doerfert via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa19eb1de726c: [OpenMP] Add match_{all,any,none} declare 
variant selector extensions. (authored by jdoerfert).

Changed prior to commit:
  https://reviews.llvm.org/D77414?vs=255212=255895#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77414

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/AST/ast-dump-openmp-declare-variant-extensions-messages.c
  clang/test/AST/ast-dump-openmp-declare-variant-extensions.c
  clang/test/OpenMP/declare_variant_ast_print.c
  clang/test/OpenMP/declare_variant_messages.c
  llvm/include/llvm/Frontend/OpenMP/OMPContext.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPContext.cpp

Index: llvm/lib/Frontend/OpenMP/OMPContext.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPContext.cpp
+++ llvm/lib/Frontend/OpenMP/OMPContext.cpp
@@ -137,52 +137,121 @@
 
 static int isVariantApplicableInContextHelper(
 const VariantMatchInfo , const OMPContext ,
-SmallVectorImpl *ConstructMatches) {
+SmallVectorImpl *ConstructMatches, bool DeviceSetOnly) {
+
+  // The match kind determines if we need to match all traits, any of the
+  // traits, or none of the traits for it to be an applicable context.
+  enum MatchKind { MK_ALL, MK_ANY, MK_NONE };
+
+  MatchKind MK = MK_ALL;
+  // Determine the match kind the user wants, "all" is the default and provided
+  // to the user only for completeness.
+  if (VMI.RequiredTraits.test(
+  unsigned(TraitProperty::implementation_extension_match_any)))
+MK = MK_ANY;
+  if (VMI.RequiredTraits.test(
+  unsigned(TraitProperty::implementation_extension_match_none)))
+MK = MK_NONE;
+
+  // Helper to deal with a single property that was (not) found in the OpenMP
+  // context based on the match kind selected by the user via
+  // `implementation={extensions(match_[all,any,none])}'
+  auto HandleTrait = [MK](TraitProperty Property,
+  bool WasFound) -> Optional /* Result */ {
+// For kind "any" a single match is enough but we ignore non-matched
+// properties.
+if (MK == MK_ANY) {
+  if (WasFound)
+return true;
+  return None;
+}
+
+// In "all" or "none" mode we accept a matching or non-matching property
+// respectively and move on. We are not done yet!
+if ((WasFound && MK == MK_ALL) || (!WasFound && MK == MK_NONE))
+  return None;
+
+// We missed a property, provide some debug output and indicate failure.
+LLVM_DEBUG({
+  if (MK == MK_ALL)
+dbgs() << "[" << DEBUG_TYPE << "] Property "
+   << getOpenMPContextTraitPropertyName(Property)
+   << " was not in the OpenMP context but match kind is all.\n";
+  if (MK == MK_NONE)
+dbgs() << "[" << DEBUG_TYPE << "] Property "
+   << getOpenMPContextTraitPropertyName(Property)
+   << " was in the OpenMP context but match kind is none.\n";
+});
+return false;
+  };
 
   for (unsigned Bit : VMI.RequiredTraits.set_bits()) {
 TraitProperty Property = TraitProperty(Bit);
+if (DeviceSetOnly &&
+getOpenMPContextTraitSetForProperty(Property) != TraitSet::device)
+  continue;
+
+// So far all extensions are handled elsewhere, we skip them here as they
+// are not part of the OpenMP context.
+if (getOpenMPContextTraitSelectorForProperty(Property) ==
+TraitSelector::implementation_extension)
+  continue;
 
 bool IsActiveTrait = Ctx.ActiveTraits.test(unsigned(Property));
-if (!IsActiveTrait) {
-  LLVM_DEBUG(dbgs() << "[" << DEBUG_TYPE << "] Property "
-<< getOpenMPContextTraitPropertyName(Property)
-<< " was not in the OpenMP context.\n");
-  return false;
-}
+Optional Result = HandleTrait(Property, IsActiveTrait);
+if (Result.hasValue())
+  return Result.getValue();
   }
 
-  // We could use isSubset here but we also want to record the match locations.
-  unsigned ConstructIdx = 0, NoConstructTraits = Ctx.ConstructTraits.size();
-  for (TraitProperty Property : VMI.ConstructTraits) {
-assert(getOpenMPContextTraitSetForProperty(Property) ==
-   TraitSet::construct &&
-   "Variant context is ill-formed!");
-
-// Verify the nesting.
-bool FoundInOrder = false;
-while (!FoundInOrder && ConstructIdx != NoConstructTraits)
-  FoundInOrder = (Ctx.ConstructTraits[ConstructIdx++] == Property);
-if (ConstructMatches)
-  ConstructMatches->push_back(ConstructIdx - 1);
-
-if (!FoundInOrder) {
-  LLVM_DEBUG(dbgs() << 

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

[Andrew via Phabricator via cfe-commits]

browneee updated this revision to Diff 255891.
browneee added a comment.

Fix formatting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77621

Files:
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
  clang/include/clang/Serialization/ASTWriter.h
  clang/include/clang/Serialization/PCHContainerOperations.h
  clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/PrecompiledPreamble.cpp
  clang/lib/Frontend/SerializedDiagnosticPrinter.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/GlobalModuleIndex.cpp
  clang/lib/Serialization/PCHContainerOperations.cpp
  llvm/include/llvm/Bitcode/BitcodeWriter.h
  llvm/include/llvm/Bitstream/BitstreamWriter.h
  llvm/include/llvm/Remarks/BitstreamRemarkSerializer.h
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/ExecutionEngine/Orc/ThreadSafeModule.cpp
  llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
  llvm/tools/llvm-cat/llvm-cat.cpp
  llvm/tools/llvm-modextract/llvm-modextract.cpp
  llvm/unittests/Bitstream/BitstreamReaderTest.cpp
  llvm/unittests/Bitstream/BitstreamWriterTest.cpp

Index: llvm/unittests/Bitstream/BitstreamWriterTest.cpp
===
--- llvm/unittests/Bitstream/BitstreamWriterTest.cpp
+++ llvm/unittests/Bitstream/BitstreamWriterTest.cpp
@@ -8,27 +8,31 @@
 
 #include "llvm/Bitstream/BitstreamWriter.h"
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SmallString.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
+#include 
+
 using namespace llvm;
 
+using ::testing::IsEmpty;
+
 namespace {
 
 TEST(BitstreamWriterTest, emitBlob) {
-  SmallString<64> Buffer;
+  std::vector Buffer;
   BitstreamWriter W(Buffer);
   W.emitBlob("str", /* ShouldEmitSize */ false);
-  EXPECT_EQ(StringRef("str\0", 4), Buffer);
+  EXPECT_EQ(StringRef("str\0", 4), StringRef(Buffer.data(), Buffer.size()));
 }
 
 TEST(BitstreamWriterTest, emitBlobWithSize) {
-  SmallString<64> Buffer;
+  std::vector Buffer;
   {
 BitstreamWriter W(Buffer);
 W.emitBlob("str");
   }
-  SmallString<64> Expected;
+  std::vector Expected;
   {
 BitstreamWriter W(Expected);
 W.EmitVBR(3, 6);
@@ -38,21 +42,21 @@
 W.Emit('r', 8);
 W.Emit(0, 8);
   }
-  EXPECT_EQ(StringRef(Expected), Buffer);
+  EXPECT_EQ(Expected, Buffer);
 }
 
 TEST(BitstreamWriterTest, emitBlobEmpty) {
-  SmallString<64> Buffer;
+  std::vector Buffer;
   BitstreamWriter W(Buffer);
   W.emitBlob("", /* ShouldEmitSize */ false);
-  EXPECT_EQ(StringRef(""), Buffer);
+  EXPECT_THAT(Buffer, IsEmpty());
 }
 
 TEST(BitstreamWriterTest, emitBlob4ByteAligned) {
-  SmallString<64> Buffer;
+  std::vector Buffer;
   BitstreamWriter W(Buffer);
   W.emitBlob("str0", /* ShouldEmitSize */ false);
-  EXPECT_EQ(StringRef("str0"), Buffer);
+  EXPECT_EQ(StringRef("str0"), StringRef(Buffer.data(), Buffer.size()));
 }
 
 } // end namespace
Index: llvm/unittests/Bitstream/BitstreamReaderTest.cpp
===
--- llvm/unittests/Bitstream/BitstreamReaderTest.cpp
+++ llvm/unittests/Bitstream/BitstreamReaderTest.cpp
@@ -11,6 +11,8 @@
 #include "llvm/Bitstream/BitstreamWriter.h"
 #include "gtest/gtest.h"
 
+#include 
+
 using namespace llvm;
 
 namespace {
@@ -96,7 +98,7 @@
 StringRef BlobIn((const char *)BlobData.begin(), BlobSize);
 
 // Write the bitcode.
-SmallVector Buffer;
+std::vector Buffer;
 unsigned AbbrevID;
 {
   BitstreamWriter Stream(Buffer);
@@ -115,7 +117,7 @@
 
 // Stream the buffer into the reader.
 BitstreamCursor Stream(
-ArrayRef((const uint8_t *)Buffer.begin(), Buffer.size()));
+ArrayRef((const uint8_t *)Buffer.data(), Buffer.size()));
 
 // Header.  Included in test so that we can run llvm-bcanalyzer to debug
 // when there are problems.
Index: llvm/tools/llvm-modextract/llvm-modextract.cpp
===
--- llvm/tools/llvm-modextract/llvm-modextract.cpp
+++ llvm/tools/llvm-modextract/llvm-modextract.cpp
@@ -58,12 +58,12 @@
   ExitOnErr(errorCodeToError(EC));
 
   if (BinaryExtract) {
-SmallVector Result;
+std::vector Result;
 BitcodeWriter Writer(Result);
-Result.append(Ms[ModuleIndex].getBuffer().begin(),
+Result.insert(Result.end(), Ms[ModuleIndex].getBuffer().begin(),
   Ms[ModuleIndex].getBuffer().end());
 Writer.copyStrtab(Ms[ModuleIndex].getStrtab());
-Out->os() << Result;
+Out->os().write(Result.data(), Result.size());
 Out->keep();
 return 0;
   }
Index: llvm/tools/llvm-cat/llvm-cat.cpp
===
--- llvm/tools/llvm-cat/llvm-cat.cpp
+++ llvm/tools/llvm-cat/llvm-cat.cpp
@@ -54,7 +54,7 @@
   ExitOnError ExitOnErr("llvm-cat: ");
   LLVMContext Context;
 

[PATCH] D77705: [Driver] Forward pass plugin arguments to gold

[Dominic Chen via Phabricator via cfe-commits]

ddcc created this revision.
ddcc added a reviewer: tejohnson.
Herald added a project: clang.

Support forwarding `-fplugin` and `-fpass-plugin` arguments for loading pass 
plugins

Depends on: D77704 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77705

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -474,10 +474,18 @@
 ProfileUseArg->getNumValues() == 0 ? "" : ProfileUseArg->getValue());
 if (Path.empty() || llvm::sys::fs::is_directory(Path))
   llvm::sys::path::append(Path, "default.profdata");
-CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-opt=cs-profile-path=") 
+
- Path));
+CmdArgs.push_back(
+Args.MakeArgString(Twine("-plugin-opt=cs-profile-path=") + Path));
   }
 
+  // Forward arguments for loading plugins (old/new PM)
+  for (const Arg *A : Args.filtered(options::OPT_fplugin_EQ))
+CmdArgs.push_back(
+Args.MakeArgString(Twine("-plugin-opt=load=") + A->getValue()));
+  for (const Arg *A : Args.filtered(options::OPT_fpass_plugin_EQ))
+CmdArgs.push_back(Args.MakeArgString(
+Twine("-plugin-opt=load-pass-plugin=") + A->getValue()));
+
   // Need this flag to turn on new pass manager via Gold plugin.
   if (Args.hasFlag(options::OPT_fexperimental_new_pass_manager,
options::OPT_fno_experimental_new_pass_manager,


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -474,10 +474,18 @@
 ProfileUseArg->getNumValues() == 0 ? "" : ProfileUseArg->getValue());
 if (Path.empty() || llvm::sys::fs::is_directory(Path))
   llvm::sys::path::append(Path, "default.profdata");
-CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-opt=cs-profile-path=") +
- Path));
+CmdArgs.push_back(
+Args.MakeArgString(Twine("-plugin-opt=cs-profile-path=") + Path));
   }
 
+  // Forward arguments for loading plugins (old/new PM)
+  for (const Arg *A : Args.filtered(options::OPT_fplugin_EQ))
+CmdArgs.push_back(
+Args.MakeArgString(Twine("-plugin-opt=load=") + A->getValue()));
+  for (const Arg *A : Args.filtered(options::OPT_fpass_plugin_EQ))
+CmdArgs.push_back(Args.MakeArgString(
+Twine("-plugin-opt=load-pass-plugin=") + A->getValue()));
+
   // Need this flag to turn on new pass manager via Gold plugin.
   if (Args.hasFlag(options::OPT_fexperimental_new_pass_manager,
options::OPT_fno_experimental_new_pass_manager,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] f85ae05 - [OpenMP] Provide math functions in OpenMP device code via OpenMP variants

[Johannes Doerfert via cfe-commits]

Author: Johannes Doerfert
Date: 2020-04-07T23:33:24-05:00
New Revision: f85ae058f580e9d74c4a8f2f0de168c18da6150f

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

LOG: [OpenMP] Provide math functions in OpenMP device code via OpenMP variants

For OpenMP target regions to piggy back on the CUDA/AMDGPU/... implementation 
of math functions,
we include the appropriate definitions inside of an `omp begin/end declare 
variant match(device={arch(nvptx)})` scope.
This way, the vendor specific math functions will become specialized versions 
of the system math functions.
When a system math function is called and specialized version is available the 
selection logic introduced in D75779
instead call the specialized version. In contrast to the code path we used so 
far, the system header is actually included.
This means functions without specialized versions are available and so are 
macro definitions.

This should address PR42061, PR42798, and PR42799.

Reviewed By: ye-luo

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

Added: 
clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h
clang/lib/Headers/openmp_wrappers/time.h
clang/test/Headers/Inputs/include/climits
clang/test/Headers/nvptx_device_math_complex.c
clang/test/Headers/nvptx_device_math_macro.cpp
clang/test/Headers/nvptx_device_math_modf.cpp
clang/test/Headers/nvptx_device_math_sin.c
clang/test/Headers/nvptx_device_math_sin.cpp
clang/test/Headers/nvptx_device_math_sin_cos.cpp
clang/test/Headers/nvptx_device_math_sincos.cpp

Modified: 
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Headers/CMakeLists.txt
clang/lib/Headers/__clang_cuda_cmath.h
clang/lib/Headers/__clang_cuda_device_functions.h
clang/lib/Headers/__clang_cuda_math.h
clang/lib/Headers/__clang_cuda_math_forward_declares.h
clang/lib/Headers/openmp_wrappers/cmath
clang/lib/Headers/openmp_wrappers/math.h
clang/test/Headers/Inputs/include/cmath
clang/test/Headers/Inputs/include/cstdlib
clang/test/Headers/Inputs/include/math.h
clang/test/Headers/Inputs/include/stdlib.h
clang/test/Headers/nvptx_device_cmath_functions.c
clang/test/Headers/nvptx_device_cmath_functions.cpp
clang/test/Headers/nvptx_device_cmath_functions_cxx17.cpp
clang/test/Headers/nvptx_device_math_functions.c
clang/test/Headers/nvptx_device_math_functions.cpp
clang/test/Headers/nvptx_device_math_functions_cxx17.cpp

Removed: 
clang/lib/Headers/openmp_wrappers/__clang_openmp_math.h
clang/lib/Headers/openmp_wrappers/__clang_openmp_math_declares.h



diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 4d825301be41..2b368131f5cc 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -1216,7 +1216,7 @@ void Clang::AddPreprocessingOptions(Compilation , const 
JobAction ,
 }
 
 CmdArgs.push_back("-include");
-CmdArgs.push_back("__clang_openmp_math_declares.h");
+CmdArgs.push_back("__clang_openmp_device_functions.h");
   }
 
   // Add -i* options, and automatically translate to

diff  --git a/clang/lib/Headers/CMakeLists.txt 
b/clang/lib/Headers/CMakeLists.txt
index 6851957600e0..d6c8ed5e1fc6 100644
--- a/clang/lib/Headers/CMakeLists.txt
+++ b/clang/lib/Headers/CMakeLists.txt
@@ -145,8 +145,7 @@ set(ppc_wrapper_files
 set(openmp_wrapper_files
   openmp_wrappers/math.h
   openmp_wrappers/cmath
-  openmp_wrappers/__clang_openmp_math.h
-  openmp_wrappers/__clang_openmp_math_declares.h
+  openmp_wrappers/__clang_openmp_device_functions.h
   openmp_wrappers/new
 )
 

diff  --git a/clang/lib/Headers/__clang_cuda_cmath.h 
b/clang/lib/Headers/__clang_cuda_cmath.h
index 834a2e3fd134..f406112164e5 100644
--- a/clang/lib/Headers/__clang_cuda_cmath.h
+++ b/clang/lib/Headers/__clang_cuda_cmath.h
@@ -12,7 +12,9 @@
 #error "This file is for CUDA compilation only."
 #endif
 
+#ifndef _OPENMP
 #include 
+#endif
 
 // CUDA lets us use various std math functions on the device side.  This file
 // works in concert with __clang_cuda_math_forward_declares.h to make this 
work.
@@ -31,31 +33,15 @@
 // std covers all of the known knowns.
 
 #ifdef _OPENMP
-#define __DEVICE__ static __attribute__((always_inline))
+#define __DEVICE__ static constexpr __attribute__((always_inline, nothrow))
 #else
 #define __DEVICE__ static __device__ __inline__ __attribute__((always_inline))
 #endif
 
-// For C++ 17 we need to include noexcept attribute to be compatible
-// with the header-defined version. This may be removed once
-// variant is supported.
-#if defined(_OPENMP) && defined(__cplusplus) && __cplusplus >= 201703L
-#define __NOEXCEPT noexcept
-#else
-#define __NOEXCEPT
-#endif
-
-#if !(defined(_OPENMP) && 

[clang] a19eb1d - [OpenMP] Add match_{all,any,none} declare variant selector extensions.

[Johannes Doerfert via cfe-commits]

Author: Johannes Doerfert
Date: 2020-04-07T23:33:24-05:00
New Revision: a19eb1de726c1ccbf60dca6a1fbcd49b3157282f

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

LOG: [OpenMP] Add match_{all,any,none} declare variant selector extensions.

By default, all traits in the OpenMP context selector have to match for
it to be acceptable. Though, we sometimes want a single property out of
multiple to match (=any) or no match at all (=none). We offer these
choices as extensions via
  `implementation={extension(match_{all,any,none})}`
to the user. The choice will affect the entire context selector not only
the traits following the match property.

The first user will be D75788. There we can replace
```
  #pragma omp begin declare variant match(device={arch(nvptx64)})
  #define __CUDA__

  #include <__clang_cuda_cmath.h>

  // TODO: Hack until we support an extension to the match clause that allows 
"or".
  #undef __CLANG_CUDA_CMATH_H__

  #undef __CUDA__
  #pragma omp end declare variant

  #pragma omp begin declare variant match(device={arch(nvptx)})
  #define __CUDA__

  #include <__clang_cuda_cmath.h>

  #undef __CUDA__
  #pragma omp end declare variant
```
with the much simpler
```
  #pragma omp begin declare variant match(device={arch(nvptx, nvptx64)}, 
implementation={extension(match_any)})
  #define __CUDA__

  #include <__clang_cuda_cmath.h>

  #undef __CUDA__
  #pragma omp end declare variant
```

Reviewed By: mikerice

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

Added: 
clang/test/AST/ast-dump-openmp-declare-variant-extensions-messages.c
clang/test/AST/ast-dump-openmp-declare-variant-extensions.c

Modified: 
clang/include/clang/AST/OpenMPClause.h
clang/include/clang/Basic/AttrDocs.td
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/lib/AST/OpenMPClause.cpp
clang/lib/Parse/ParseOpenMP.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/declare_variant_ast_print.c
clang/test/OpenMP/declare_variant_messages.c
llvm/include/llvm/Frontend/OpenMP/OMPContext.h
llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
llvm/lib/Frontend/OpenMP/OMPContext.cpp

Removed: 




diff  --git a/clang/include/clang/AST/OpenMPClause.h 
b/clang/include/clang/AST/OpenMPClause.h
index 9f5ff5a85182..f276611e3d0c 100644
--- a/clang/include/clang/AST/OpenMPClause.h
+++ b/clang/include/clang/AST/OpenMPClause.h
@@ -7229,11 +7229,9 @@ class OMPTraitInfo {
   /// former is a flat representation the actual main 
diff erence is that the
   /// latter uses clang::Expr to store the score/condition while the former is
   /// independent of clang. Thus, expressions and conditions are evaluated in
-  /// this method. If \p DeviceSetOnly is true, only the device selector set, 
if
-  /// present, is put in \p VMI, otherwise all selector sets are put in \p VMI.
+  /// this method.
   void getAsVariantMatchInfo(ASTContext ,
- llvm::omp::VariantMatchInfo ,
- bool DeviceSetOnly) const;
+ llvm::omp::VariantMatchInfo ) const;
 
   /// Return a string representation identifying this context selector.
   std::string getMangledName() const;

diff  --git a/clang/include/clang/Basic/AttrDocs.td 
b/clang/include/clang/Basic/AttrDocs.td
index 36561c04d395..e3308cd74874 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -3394,6 +3394,20 @@ where clause is one of the following:
 and where `variant-func-id` is the name of a function variant that is either a
 base language identifier or, for C++, a template-id.
 
+Clang provides the following context selector extensions, used via 
`implementation={extension(EXTENSION)}`:
+
+  .. code-block:: none
+
+match_all
+match_any
+match_none
+
+The match extensions change when the *entire* context selector is considered a
+match for an OpenMP context. The default is `all`, with `none` no trait in the
+selector is allowed to be in the OpenMP context, with `any` a single trait in
+both the selector and OpenMP context is sufficient. Only a single match
+extension trait is allowed per context selector.
+
   }];
 }
 

diff  --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index f3741531c8b5..a1311d7776c6 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1313,6 +1313,8 @@ def warn_omp_ctx_incompatible_score_for_property
 def warn_omp_more_one_device_type_clause
 : Warning<"more than one 'device_type' clause is specified">,
   InGroup;
+def err_omp_variant_ctx_second_match_extension : Error<
+  "only a single match extension allowed per OpenMP context selector">;
 
 

[clang] eb5a16e - [OpenMP] Specialize OpenMP calls after template instantiation

[Johannes Doerfert via cfe-commits]

Author: Johannes Doerfert
Date: 2020-04-07T23:33:24-05:00
New Revision: eb5a16efbf59150af31bd4e3d37b8ea5976d780b

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

LOG: [OpenMP] Specialize OpenMP calls after template instantiation

As with regular calls, we want to specialize a call that went through
template instantiation if it has an applicable OpenMP declare variant.

Reviewed By: erichkeane, mikerice

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

Added: 
clang/test/AST/ast-dump-openmp-begin-declare-variant_template_1.cpp

Modified: 
clang/lib/Sema/TreeTransform.h

Removed: 




diff  --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index e9f4b11ca7bb..a3103205f2bd 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -2411,8 +2411,8 @@ class TreeTransform {
MultiExprArg Args,
SourceLocation RParenLoc,
Expr *ExecConfig = nullptr) {
-return getSema().BuildCallExpr(/*Scope=*/nullptr, Callee, LParenLoc, Args,
-   RParenLoc, ExecConfig);
+return getSema().ActOnCallExpr(
+/*Scope=*/nullptr, Callee, LParenLoc, Args, RParenLoc, ExecConfig);
   }
 
   /// Build a new member access expression.

diff  --git 
a/clang/test/AST/ast-dump-openmp-begin-declare-variant_template_1.cpp 
b/clang/test/AST/ast-dump-openmp-begin-declare-variant_template_1.cpp
new file mode 100644
index ..6a663d5d75d9
--- /dev/null
+++ b/clang/test/AST/ast-dump-openmp-begin-declare-variant_template_1.cpp
@@ -0,0 +1,170 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fopenmp -verify -ast-dump 
%s   | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fopenmp -verify -ast-dump 
%s -x c++| FileCheck %s
+// expected-no-diagnostics
+
+int also_before() {
+  return 1;
+}
+
+#pragma omp begin declare variant 
match(implementation={vendor(score(100):llvm)})
+int also_after(void) {
+  return 2;
+}
+int also_after(int) {
+  return 3;
+}
+int also_after(double) {
+  return 0;
+}
+#pragma omp end declare variant
+#pragma omp begin declare variant match(implementation={vendor(score(0):llvm)})
+int also_before() {
+  return 0;
+}
+#pragma omp end declare variant
+
+int also_after(void) {
+  return 4;
+}
+int also_after(int) {
+  return 5;
+}
+int also_after(double) {
+  return 6;
+}
+
+template
+int test1() {
+  // Should return 0.
+  return also_after(T(0));
+}
+
+typedef int(*Ty)();
+
+template
+int test2() {
+  // Should return 0.
+  return fn();
+}
+
+int test() {
+  // Should return 0.
+  return test1() + test2();
+}
+
+// CHECK:  |-FunctionDecl [[ADDR_0:0x[a-z0-9]*]] <{{.*}}, line:7:1> 
line:5:5 used also_before 'int ({{.*}})'
+// CHECK-NEXT: | |-CompoundStmt [[ADDR_1:0x[a-z0-9]*]] 
+// CHECK-NEXT: | | `-ReturnStmt [[ADDR_2:0x[a-z0-9]*]] 
+// CHECK-NEXT: | |   `-IntegerLiteral [[ADDR_3:0x[a-z0-9]*]]  'int' 1
+// CHECK-NEXT: | `-OMPDeclareVariantAttr [[ADDR_4:0x[a-z0-9]*]] <> Implicit implementation={vendor(score(0): llvm)}
+// CHECK-NEXT: |   `-DeclRefExpr [[ADDR_5:0x[a-z0-9]*]]  'int 
({{.*}})' Function [[ADDR_6:0x[a-z0-9]*]] 
'also_before[implementation={vendor(llvm)}]' 'int ({{.*}})'
+// CHECK-NEXT: |-FunctionDecl [[ADDR_7:0x[a-z0-9]*]]  col:5 
implicit also_after 'int ({{.*}})'
+// CHECK-NEXT: | `-OMPDeclareVariantAttr [[ADDR_8:0x[a-z0-9]*]] <> Implicit implementation={vendor(score(100): llvm)}
+// CHECK-NEXT: |   `-DeclRefExpr [[ADDR_9:0x[a-z0-9]*]]  'int ({{.*}})' 
Function [[ADDR_10:0x[a-z0-9]*]] 'also_after[implementation={vendor(llvm)}]' 
'int ({{.*}})'
+// CHECK-NEXT: |-FunctionDecl [[ADDR_10]]  line:10:1 
also_after[implementation={vendor(llvm)}] 'int ({{.*}})'
+// CHECK-NEXT: | `-CompoundStmt [[ADDR_11:0x[a-z0-9]*]] 
+// CHECK-NEXT: |   `-ReturnStmt [[ADDR_12:0x[a-z0-9]*]] 
+// CHECK-NEXT: | `-IntegerLiteral [[ADDR_13:0x[a-z0-9]*]]  'int' 2
+// CHECK-NEXT: |-FunctionDecl [[ADDR_14:0x[a-z0-9]*]]  
col:5 implicit also_after 'int (int)'
+// CHECK-NEXT: | |-ParmVarDecl [[ADDR_15:0x[a-z0-9]*]]  col:19 'int'
+// CHECK-NEXT: | `-OMPDeclareVariantAttr [[ADDR_16:0x[a-z0-9]*]] <> Implicit implementation={vendor(score(100): llvm)}
+// CHECK-NEXT: |   `-DeclRefExpr [[ADDR_17:0x[a-z0-9]*]]  'int (int)' 
Function [[ADDR_18:0x[a-z0-9]*]] 'also_after[implementation={vendor(llvm)}]' 
'int (int)'
+// CHECK-NEXT: |-FunctionDecl [[ADDR_18]]  line:13:1 
also_after[implementation={vendor(llvm)}] 'int (int)'
+// CHECK-NEXT: | |-ParmVarDecl [[ADDR_15]]  col:19 'int'
+// CHECK-NEXT: | `-CompoundStmt [[ADDR_19:0x[a-z0-9]*]] 
+// CHECK-NEXT: |   `-ReturnStmt [[ADDR_20:0x[a-z0-9]*]] 
+// CHECK-NEXT: | `-IntegerLiteral [[ADDR_21:0x[a-z0-9]*]]  'int' 3
+// CHECK-NEXT: 

[PATCH] D77688: [CUDA] Improve testing of libdevice detection.

[Yaxun Liu via Phabricator via cfe-commits]

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

LGTM. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77688



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

[clang-tools-extra] 5303770 - [OpenMP] "UnFix" last layering problem with FrontendOpenMP

[Johannes Doerfert via cfe-commits]

Author: Johannes Doerfert
Date: 2020-04-07T22:47:41-05:00
New Revision: 530377018f624eadb8c07650511bbb9ca63608de

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

LOG: [OpenMP] "UnFix" last layering problem with FrontendOpenMP

It seems one target was missed in D77666 which kept some bots red [0].

[0] 
http://lab.llvm.org:8011/builders/clang-ppc64le-linux-multistage/builds/12079/steps/build%20stage%201/logs/stdio

Added: 


Modified: 
clang-tools-extra/clang-tidy/tool/CMakeLists.txt

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/tool/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/tool/CMakeLists.txt
index 0cd15ddb4653..ff9104b661d0 100644
--- a/clang-tools-extra/clang-tidy/tool/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/tool/CMakeLists.txt
@@ -2,6 +2,7 @@ set(LLVM_LINK_COMPONENTS
   AllTargetsAsmParsers
   AllTargetsDescs
   AllTargetsInfos
+  FrontendOpenMP
   support
   )
 



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

[PATCH] D76768: [analyzer] Added support of scan-build and exploded-graph-rewriter regression tests for Windows

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D76768#1968023 , @ASDenysPetrov 
wrote:

> Is there any way to get to buildbot file system to compare test files with 
> what is in the master branch? Particularly 
> //C:\src\llvm-project\clang\test\Analysis\scan-build/Inputs/single_null_dereference.c//


If the source was checked out incorrectly then most likely everything would 
have been broken, not just our test. I'd rather suspect write permissions or 
something, but then again, a lot of tests create directories and then write 
into them. Dunno.

@thakis, do you know what is the setup of this buildbot? Like, what's so 
special about it? 'Cause i weren't seeing any failures from other windows 
buildbots. Can we discriminate between this buildbot and other buildbots with 
any `UNSUPPORTED:` bang (even if we have to add one)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76768



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

[PATCH] D77702: [clangd] Use token modifiers and more token types for semanticTokens

[Nathan Ridge via Phabricator via cfe-commits]

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

The intention here is to allow clients to continue using a different
color for each of our HighlightingKinds, as with the old API.

For some highlighting kinds, this is accomplished by adding more
token types (e.g. splitting "dependent" into "dependentType" and
"dependentName"). For others, it is accomplished by using token
modifiers.

Note that the patch introduces a "member" modifier in place of
the "member" token type. This allows member variables and member
functions to be colored differently, by using the "variable" and
"function" token types, respectively, combined with the "member"
modifier.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77702

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h

Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -88,6 +88,7 @@
 // main AST.
 std::vector getSemanticHighlightings(ParsedAST );
 
+std::vector semanticTokenModifiers();
 std::vector toSemanticTokens(llvm::ArrayRef);
 llvm::StringRef toSemanticTokenType(HighlightingKind Kind);
 std::vector diffTokens(llvm::ArrayRef Before,
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -445,6 +445,43 @@
   return std::tie(L.Line, L.Tokens) == std::tie(R.Line, R.Tokens);
 }
 
+enum class TokenModifiers {
+  None = 0,
+  Local = 1,
+  Static = 2,
+  Member = 4,
+};
+
+TokenModifiers operator|(TokenModifiers A, TokenModifiers B) {
+  return static_cast(static_cast(A) |
+ static_cast(B));
+}
+
+TokenModifiers getTokenModifiers(HighlightingKind Kind) {
+  switch (Kind) {
+  case HighlightingKind::LocalVariable:
+return TokenModifiers::Local;
+  case HighlightingKind::StaticField:
+return TokenModifiers::Static | TokenModifiers::Member;
+  case HighlightingKind::Method:
+return TokenModifiers::Member;
+  case HighlightingKind::StaticMethod:
+return TokenModifiers::Static | TokenModifiers::Member;
+  case HighlightingKind::Field:
+return TokenModifiers::Member;
+  default:
+return TokenModifiers::None;
+  }
+}
+
+std::vector semanticTokenModifiers() {
+  std::vector Modifiers;
+  Modifiers.push_back("local");
+  Modifiers.push_back("static");
+  Modifiers.push_back("member");
+  return Modifiers;
+}
+
 std::vector
 toSemanticTokens(llvm::ArrayRef Tokens) {
   assert(std::is_sorted(Tokens.begin(), Tokens.end()));
@@ -473,6 +510,7 @@
 assert(Tok.R.end.line == Tok.R.start.line);
 Out.length = Tok.R.end.character - Tok.R.start.character;
 Out.tokenType = static_cast(Tok.Kind);
+Out.tokenModifiers = static_cast(getTokenModifiers(Tok.Kind));
 
 Last = 
   }
@@ -489,12 +527,11 @@
   case HighlightingKind::Function:
 return "function";
   case HighlightingKind::Method:
-return "member";
+return "function";
   case HighlightingKind::StaticMethod:
-// FIXME: better function/member with static modifier?
 return "function";
   case HighlightingKind::Field:
-return "member";
+return "variable";
   case HighlightingKind::Class:
 return "class";
   case HighlightingKind::Enum:
@@ -504,9 +541,9 @@
   case HighlightingKind::Typedef:
 return "type";
   case HighlightingKind::DependentType:
-return "dependent"; // nonstandard
+return "dependentType"; // nonstandard
   case HighlightingKind::DependentName:
-return "dependent"; // nonstandard
+return "dependentName"; // nonstandard
   case HighlightingKind::Namespace:
 return "namespace";
   case HighlightingKind::TemplateParameter:
@@ -514,7 +551,7 @@
   case HighlightingKind::Concept:
 return "concept"; // nonstandard
   case HighlightingKind::Primitive:
-return "type";
+return "primitive";
   case HighlightingKind::Macro:
 return "macro";
   case HighlightingKind::InactiveCode:
@@ -537,8 +574,8 @@
 llvm::SmallVector LineByteTokens;
 llvm::raw_svector_ostream OS(LineByteTokens);
 for (const auto  : Line.Tokens) {
-  // Writes the token to LineByteTokens in the byte format specified by the
-  // LSP proposal. Described below.
+  // Writes the token to LineByteTokens in the byte format specified by
+  // the LSP proposal. Described below.
   // |< 4 bytes >|<-- 2 bytes -->|<--- 2 bytes -->|
   // |character  |  length   |index   |
 
@@ -600,9 +637,8 @@
   llvm_unreachable("unhandled HighlightingKind");
 }
 

[PATCH] D76896: Color dependent names based on their heuristic target if they have one

[Nathan Ridge via Phabricator via cfe-commits]

nridge added reviewers: sammccall, kadircet, hokein.
nridge added a comment.

Adding some reviewers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76896



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

[PATCH] D76269: [opaque pointer types] Remove deprecated Instruction/IRBuilder APIs.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76269



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

[PATCH] D76451: [clangd] Enable textual fallback for go-to-definition on dependent names

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

Review ping :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76451



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

[PATCH] D77701: [Sema] refactor static functions into private methods NFC

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
nickdesaulniers added reviewers: aaron.ballman, dblaikie.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

While working in SemaStmt, I noticed a bunch of static functions are
passed *this, which is a code smell. Prefer private methods.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77701

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaStmt.cpp

Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -120,12 +120,7 @@
   }
 }
 
-/// Diagnose unused comparisons, both builtin and overloaded operators.
-/// For '==' and '!=', suggest fixits for '=' or '|='.
-///
-/// Adding a cast to void (or other expression wrappers) will prevent the
-/// warning from firing.
-static bool DiagnoseUnusedComparison(Sema , const Expr *E) {
+bool Sema::DiagnoseUnusedComparison(const Expr *E) {
   SourceLocation Loc;
   bool CanAssign;
   enum { Equality, Inequality, Relational, ThreeWay } Kind;
@@ -176,43 +171,41 @@
 
   // Suppress warnings when the operator, suspicious as it may be, comes from
   // a macro expansion.
-  if (S.SourceMgr.isMacroBodyExpansion(Loc))
+  if (SourceMgr.isMacroBodyExpansion(Loc))
 return false;
 
-  S.Diag(Loc, diag::warn_unused_comparison)
-<< (unsigned)Kind << E->getSourceRange();
+  Diag(Loc, diag::warn_unused_comparison)
+  << (unsigned)Kind << E->getSourceRange();
 
   // If the LHS is a plausible entity to assign to, provide a fixit hint to
   // correct common typos.
   if (CanAssign) {
 if (Kind == Inequality)
-  S.Diag(Loc, diag::note_inequality_comparison_to_or_assign)
-<< FixItHint::CreateReplacement(Loc, "|=");
+  Diag(Loc, diag::note_inequality_comparison_to_or_assign)
+  << FixItHint::CreateReplacement(Loc, "|=");
 else if (Kind == Equality)
-  S.Diag(Loc, diag::note_equality_comparison_to_assign)
-<< FixItHint::CreateReplacement(Loc, "=");
+  Diag(Loc, diag::note_equality_comparison_to_assign)
+  << FixItHint::CreateReplacement(Loc, "=");
   }
 
   return true;
 }
 
-static bool DiagnoseNoDiscard(Sema , const WarnUnusedResultAttr *A,
-  SourceLocation Loc, SourceRange R1,
-  SourceRange R2, bool IsCtor) {
+bool Sema::DiagnoseNoDiscard(const WarnUnusedResultAttr *A, SourceLocation Loc,
+ SourceRange R1, SourceRange R2, bool IsCtor) {
   if (!A)
 return false;
   StringRef Msg = A->getMessage();
 
   if (Msg.empty()) {
 if (IsCtor)
-  return S.Diag(Loc, diag::warn_unused_constructor) << A << R1 << R2;
-return S.Diag(Loc, diag::warn_unused_result) << A << R1 << R2;
+  return Diag(Loc, diag::warn_unused_constructor) << A << R1 << R2;
+return Diag(Loc, diag::warn_unused_result) << A << R1 << R2;
   }
 
   if (IsCtor)
-return S.Diag(Loc, diag::warn_unused_constructor_msg) << A << Msg << R1
-  << R2;
-  return S.Diag(Loc, diag::warn_unused_result_msg) << A << Msg << R1 << R2;
+return Diag(Loc, diag::warn_unused_constructor_msg) << A << Msg << R1 << R2;
+  return Diag(Loc, diag::warn_unused_result_msg) << A << Msg << R1 << R2;
 }
 
 void Sema::DiagnoseUnusedExprResult(const Stmt *S) {
@@ -269,7 +262,7 @@
   if (const CXXBindTemporaryExpr *TempExpr = dyn_cast(E))
 E = TempExpr->getSubExpr();
 
-  if (DiagnoseUnusedComparison(*this, E))
+  if (DiagnoseUnusedComparison(E))
 return;
 
   E = WarnExpr;
@@ -282,8 +275,8 @@
 if (E->getType()->isVoidType())
   return;
 
-if (DiagnoseNoDiscard(*this, cast_or_null(
- CE->getUnusedResultAttr(Context)),
+if (DiagnoseNoDiscard(cast_or_null(
+  CE->getUnusedResultAttr(Context)),
   Loc, R1, R2, /*isCtor=*/false))
   return;
 
@@ -307,14 +300,14 @@
 if (const CXXConstructorDecl *Ctor = CE->getConstructor()) {
   const auto *A = Ctor->getAttr();
   A = A ? A : Ctor->getParent()->getAttr();
-  if (DiagnoseNoDiscard(*this, A, Loc, R1, R2, /*isCtor=*/true))
+  if (DiagnoseNoDiscard(A, Loc, R1, R2, /*isCtor=*/true))
 return;
 }
   } else if (const auto *ILE = dyn_cast(E)) {
 if (const TagDecl *TD = ILE->getType()->getAsTagDecl()) {
 
-  if (DiagnoseNoDiscard(*this, TD->getAttr(), Loc, R1,
-R2, /*isCtor=*/false))
+  if (DiagnoseNoDiscard(TD->getAttr(), Loc, R1, R2,
+/*isCtor=*/false))
 return;
 }
   } else if (ShouldSuppress)
@@ -328,8 +321,8 @@
 }
 const ObjCMethodDecl *MD = ME->getMethodDecl();
 if (MD) {
-  if (DiagnoseNoDiscard(*this, MD->getAttr(), Loc, R1,
-R2, /*isCtor=*/false))
+  if 

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

[Andrew via Phabricator via cfe-commits]

browneee updated this revision to Diff 255875.
browneee marked 2 inline comments as done and an inline comment as not done.
browneee added a comment.

Build fixes in additional projects.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77621

Files:
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
  clang/include/clang/Serialization/ASTWriter.h
  clang/include/clang/Serialization/PCHContainerOperations.h
  clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/PrecompiledPreamble.cpp
  clang/lib/Frontend/SerializedDiagnosticPrinter.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/GlobalModuleIndex.cpp
  clang/lib/Serialization/PCHContainerOperations.cpp
  llvm/include/llvm/Bitcode/BitcodeWriter.h
  llvm/include/llvm/Bitstream/BitstreamWriter.h
  llvm/include/llvm/Remarks/BitstreamRemarkSerializer.h
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/ExecutionEngine/Orc/ThreadSafeModule.cpp
  llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
  llvm/tools/llvm-cat/llvm-cat.cpp
  llvm/tools/llvm-modextract/llvm-modextract.cpp
  llvm/unittests/Bitstream/BitstreamReaderTest.cpp
  llvm/unittests/Bitstream/BitstreamWriterTest.cpp

Index: llvm/unittests/Bitstream/BitstreamWriterTest.cpp
===
--- llvm/unittests/Bitstream/BitstreamWriterTest.cpp
+++ llvm/unittests/Bitstream/BitstreamWriterTest.cpp
@@ -8,27 +8,31 @@
 
 #include "llvm/Bitstream/BitstreamWriter.h"
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SmallString.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
+#include 
+
 using namespace llvm;
 
+using ::testing::IsEmpty;
+
 namespace {
 
 TEST(BitstreamWriterTest, emitBlob) {
-  SmallString<64> Buffer;
+  std::vector Buffer;
   BitstreamWriter W(Buffer);
   W.emitBlob("str", /* ShouldEmitSize */ false);
-  EXPECT_EQ(StringRef("str\0", 4), Buffer);
+  EXPECT_EQ(StringRef("str\0", 4), StringRef(Buffer.data(), Buffer.size()));
 }
 
 TEST(BitstreamWriterTest, emitBlobWithSize) {
-  SmallString<64> Buffer;
+  std::vector Buffer;
   {
 BitstreamWriter W(Buffer);
 W.emitBlob("str");
   }
-  SmallString<64> Expected;
+  std::vector Expected;
   {
 BitstreamWriter W(Expected);
 W.EmitVBR(3, 6);
@@ -38,21 +42,21 @@
 W.Emit('r', 8);
 W.Emit(0, 8);
   }
-  EXPECT_EQ(StringRef(Expected), Buffer);
+  EXPECT_EQ(Expected, Buffer);
 }
 
 TEST(BitstreamWriterTest, emitBlobEmpty) {
-  SmallString<64> Buffer;
+  std::vector Buffer;
   BitstreamWriter W(Buffer);
   W.emitBlob("", /* ShouldEmitSize */ false);
-  EXPECT_EQ(StringRef(""), Buffer);
+  EXPECT_THAT(Buffer, IsEmpty());
 }
 
 TEST(BitstreamWriterTest, emitBlob4ByteAligned) {
-  SmallString<64> Buffer;
+  std::vector Buffer;
   BitstreamWriter W(Buffer);
   W.emitBlob("str0", /* ShouldEmitSize */ false);
-  EXPECT_EQ(StringRef("str0"), Buffer);
+  EXPECT_EQ(StringRef("str0"), StringRef(Buffer.data(), Buffer.size()));
 }
 
 } // end namespace
Index: llvm/unittests/Bitstream/BitstreamReaderTest.cpp
===
--- llvm/unittests/Bitstream/BitstreamReaderTest.cpp
+++ llvm/unittests/Bitstream/BitstreamReaderTest.cpp
@@ -11,6 +11,8 @@
 #include "llvm/Bitstream/BitstreamWriter.h"
 #include "gtest/gtest.h"
 
+#include 
+
 using namespace llvm;
 
 namespace {
@@ -96,7 +98,7 @@
 StringRef BlobIn((const char *)BlobData.begin(), BlobSize);
 
 // Write the bitcode.
-SmallVector Buffer;
+std::vector Buffer;
 unsigned AbbrevID;
 {
   BitstreamWriter Stream(Buffer);
@@ -115,7 +117,7 @@
 
 // Stream the buffer into the reader.
 BitstreamCursor Stream(
-ArrayRef((const uint8_t *)Buffer.begin(), Buffer.size()));
+ArrayRef((const uint8_t *)Buffer.data(), Buffer.size()));
 
 // Header.  Included in test so that we can run llvm-bcanalyzer to debug
 // when there are problems.
Index: llvm/tools/llvm-modextract/llvm-modextract.cpp
===
--- llvm/tools/llvm-modextract/llvm-modextract.cpp
+++ llvm/tools/llvm-modextract/llvm-modextract.cpp
@@ -58,12 +58,12 @@
   ExitOnErr(errorCodeToError(EC));
 
   if (BinaryExtract) {
-SmallVector Result;
+std::vector Result;
 BitcodeWriter Writer(Result);
-Result.append(Ms[ModuleIndex].getBuffer().begin(),
+Result.insert(Result.end(), Ms[ModuleIndex].getBuffer().begin(),
   Ms[ModuleIndex].getBuffer().end());
 Writer.copyStrtab(Ms[ModuleIndex].getStrtab());
-Out->os() << Result;
+Out->os().write(Result.data(), Result.size());
 Out->keep();
 return 0;
   }
Index: llvm/tools/llvm-cat/llvm-cat.cpp
===
--- llvm/tools/llvm-cat/llvm-cat.cpp
+++ 

[PATCH] D73307: Unique Names for Functions with Internal Linkage

[Sriraman Tallam via Phabricator via cfe-commits]

tmsriram added inline comments.



Comment at: clang/docs/UsersManual.rst:1684
+   linkage symbols. The unique name is obtained by appending the hash of the
+   full module name to the original symbol. This option is particularly useful
+   in attributing profile information to the correct function when multiple

hubert.reinterpretcast wrote:
> MaskRay wrote:
> > rnk wrote:
> > > I think it's important to be specific here. I'd go so far as to say:
> > > "When this option is set, compiler hashes the main source file path from 
> > > the command line and appends it to all internal symbols. If a program 
> > > contains multiple objects compiled from the same source file path, the 
> > > symbols are not guaranteed to be unique."
> > Should option like -ffile-prefix-map= affect the hashed source file path? I 
> > suspect it already does this but I haven't verified.
> > 
> > This may need tests for Windows and non-Windows, similar to D77223.
> With respect to @rnk's suggestion, "from the same source file path" implies 
> the same file. I would suggest saying "with the same command-line source file 
> specification".
I have handled this now and added a test.  I saw the reference but don't quite 
understand the need for windows test?  I am not familiar with this.


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

https://reviews.llvm.org/D73307



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

[PATCH] D73307: Unique Names for Functions with Internal Linkage

[Sriraman Tallam via Phabricator via cfe-commits]

tmsriram updated this revision to Diff 255871.
tmsriram marked 4 inline comments as done.
tmsriram added a comment.

Change description and handle -ffile-prefix-map/-fmacro-prefix-map.


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

https://reviews.llvm.org/D73307

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/unique-internal-linkage-names.cpp
  clang/test/CodeGen/unique-internal-linkage-names2.c
  clang/test/Driver/funique-internal-linkage-names.c

Index: clang/test/Driver/funique-internal-linkage-names.c
===
--- /dev/null
+++ clang/test/Driver/funique-internal-linkage-names.c
@@ -0,0 +1,4 @@
+// RUN: %clang -### -funique-internal-linkage-names %s -c 2>&1 | FileCheck -check-prefix=CHECK-OPT %s
+// RUN: %clang -### -funique-internal-linkage-names -fno-unique-internal-linkage-names %s -c 2>&1 | FileCheck -check-prefix=CHECK-NOOPT %s
+// CHECK-OPT: "-funique-internal-linkage-names"
+// CHECK-NOOPT-NOT: "-funique-internal-linkage-names"
Index: clang/test/CodeGen/unique-internal-linkage-names2.c
===
--- /dev/null
+++ clang/test/CodeGen/unique-internal-linkage-names2.c
@@ -0,0 +1,15 @@
+// This test checks if -funique-internal-linkage-names uses the right
+// path when -ffile-prefix-map= (-fmacro-prefix-map=) is enabled.
+// With -fmacro-prefix-map=%p=NEW, this file must be referenced as:
+// NEW/unique-internal-linkage-names2.c
+// MD5 hash of "NEW/unique-internal-linkage-names2.c" :
+// $ echo -n NEW/unique-internal-linkage-names2.c | md5sum
+// bd816b262f03c98ffb082cde0847804c
+// RUN: %clang_cc1 -triple x86_64 -funique-internal-linkage-names -fmacro-prefix-map=%p=NEW -x c -S -emit-llvm -o - %s | FileCheck %s
+static int glob;
+
+int getGlob() {
+  return glob;
+}
+
+// CHECK: glob.bd816b262f03c98ffb082cde0847804c = internal global
Index: clang/test/CodeGen/unique-internal-linkage-names.cpp
===
--- /dev/null
+++ clang/test/CodeGen/unique-internal-linkage-names.cpp
@@ -0,0 +1,68 @@
+// This test checks if internal linkage symbols get unique names with
+// -funique-internal-linkage-names option.
+// RUN: %clang_cc1 -triple x86_64 -x c++ -S -emit-llvm -o - < %s | FileCheck %s --check-prefix=PLAIN
+// RUN: %clang_cc1 -triple x86_64 -x c++ -S -emit-llvm -funique-internal-linkage-names -o - < %s | FileCheck %s --check-prefix=UNIQUE
+
+static int glob;
+static int foo() {
+  return 0;
+}
+
+int (*bar())() {
+  return foo;
+}
+
+int getGlob() {
+  return glob;
+}
+
+// Function local static variable and anonymous namespace namespace variable.
+namespace {
+int anon_m;
+int getM() {
+  return anon_m;
+}
+} // namespace
+
+int retAnonM() {
+  static int fGlob;
+  return getM() + fGlob;
+}
+
+// Multiversioning symbols
+__attribute__((target("default"))) static int mver() {
+  return 0;
+}
+
+__attribute__((target("sse4.2"))) static int mver() {
+  return 1;
+}
+
+int mver_call() {
+  return mver();
+}
+
+// PLAIN: @_ZL4glob = internal global
+// PLAIN: @_ZL3foov()
+// PLAIN: @_ZN12_GLOBAL__N_14getMEv
+// PLAIN: @_ZZ8retAnonMvE5fGlob
+// PLAIN: @_ZN12_GLOBAL__N_16anon_mE
+// PLAIN: @_ZL4mverv.resolver()
+// PLAIN: @_ZL4mverv()
+// PLAIN: @_ZL4mverv.sse4.2()
+// UNIQUE-NOT: @_ZL4glob = internal global
+// UNIQUE-NOT: @_ZL3foov()
+// UNIQUE-NOT: @_ZN12_GLOBAL__N_14getMEv
+// UNIQUE-NOT: @_ZZ8retAnonMvE5fGlob
+// UNIQUE-NOT: @_ZN12_GLOBAL__N_16anon_mE
+// UNIQUE-NOT: @_ZL4mverv.resolver()
+// UNIQUE-NOT: @_ZL4mverv()
+// UNIQUE-NOT: @_ZL4mverv.sse4.2()
+// UNIQUE: @_ZL4glob.{{[0-9a-f]+}} = internal global
+// UNIQUE: @_ZL3foov.{{[0-9a-f]+}}()
+// UNIQUE: @_ZN12_GLOBAL__N_14getMEv.{{[0-9a-f]+}}
+// UNIQUE: @_ZZ8retAnonMvE5fGlob.{{[0-9a-f]+}}
+// UNIQUE: @_ZN12_GLOBAL__N_16anon_mE.{{[0-9a-f]+}}
+// UNIQUE: @_ZL4mverv.{{[0-9a-f]+}}.resolver()
+// UNIQUE: @_ZL4mverv.{{[0-9a-f]+}}()
+// UNIQUE: @_ZL4mverv.{{[0-9a-f]+}}.sse4.2()
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -958,6 +958,8 @@
   Opts.DataSections = Args.hasArg(OPT_fdata_sections);
   Opts.StackSizeSection = Args.hasArg(OPT_fstack_size_section);
   Opts.UniqueSectionNames = !Args.hasArg(OPT_fno_unique_section_names);
+  Opts.UniqueInternalLinkageNames =
+  Args.hasArg(OPT_funique_internal_linkage_names);
 
   Opts.MergeFunctions = Args.hasArg(OPT_fmerge_functions);
 
Index: clang/docs/UsersManual.rst
===
--- clang/docs/UsersManual.rst
+++ clang/docs/UsersManual.rst
@@ 

[PATCH] D77683: [Docs] Make code review policy clearer about requested pre-commit reviews

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.

This seems like both over-fitting the general rule for a specific case and 
fuzzy/imprecise enough that it isn't clear to me how much useful it is in 
practice?

Maybe we could step back: what is the concrete problem that this intends to 
address? Is there an area where it has been problematic at the moment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77683



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

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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

Requesting changes just to be sure we consider the other options.  I don't 
think it's good that `SmallVector` is no longer useful for large byte streams; 
I would prefer to fix that then stop using the type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77621



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

[PATCH] D77697: libc++: adjust modulemap for non-modular C

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

This SGTM in principle, but the patch makes me wonder: which other headers from 
the non-modularized C runtime change ownership?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77697



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

[PATCH] D77669: [clangd] Run TUStatus test in sync mode to make output deterministic

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:849
+  auto Opts = ClangdServer::optsForTest();
+  Opts.AsyncThreadsCount = 0;
+  ClangdServer Server(CDB, FS, Opts, );

This seems unfortunate because it doesn't test the production configuration.

How many possible sequences are there? Are there fewer if we blockuntilidle 
between adddoc + locatesymbolat?

Can we use testing::AnyOf to fudge around the nondeterminism?



Comment at: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:866
+  CaptureTUStatus.allStatus(),
+  ElementsAre(TUState(PreambleAction::Building, ASTAction::Idle),
+  TUState(PreambleAction::Building, ASTAction::Building),

FWIW, I found the comments useful, and don't understand e.g. why we have two 
identical lines anymore.



Comment at: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:869
+  TUState(PreambleAction::Building, ASTAction::Building),
+  TUState(PreambleAction::Idle, ASTAction::Building)));
 }

why do we never go idle at the end?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77669



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

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

This is thanks to a commit of mine that shaved a word off of `SmallVector`.  
Some options to consider:

1. Revert to word-size integers (`size_t`? `uintptr_t`?) for Size and Capacity 
for small-enough types.  Could be just if `sizeof(T)==1`.  Or maybe just for 
`char` and `unsigned char`.
2. Revert my patch entirely and go back to words (these used to be `void*`).
3. (Your patch, stop using `SmallVector`.)

I think I would prefer some variation of (1) over (3).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77621



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

[PATCH] D77656: [clangd] Fix a crash bug in AddUsing tweak around template handling.

[Sam McCall via Phabricator via cfe-commits]

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

Whoops, indeed.
There should be a version of falsehoods programmers believe about names 

 for the clang AST :-)

- Decls have names
- well, NamedDecls have names
- Decls that are not NamedDecls don't have names
- if a NamedDecl has a name, that name is an identifier
- ...




Comment at: clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp:212
+Name =
+
E.getType().getUnqualifiedType().getBaseTypeIdentifier()->getName();
+QualifierToRemove = E.getQualifierLoc();

BaseTypeIdentifier->getName()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77656



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

[PATCH] D77392: [WIP][clangd] Make signatureHelp work with stale preambles

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

This is the perfect feature to start with (doesn't actually need any location 
transforms, lets us drop TUScheduler features), so well done for finding that.

That said, high-level comments mostly about preamble patching in general rather 
than this particular case.




Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1076
+  if (PatchAdditionalIncludes) {
+for (const auto  : newIncludes(
+ Input.Preamble.LexedIncludes,

This is going to be used in other places where we parse too, we'll want some 
abstraction to encapsulate the analysis, the tweaks to CompilerInvocation, and 
(for other uses) mapping of locations from the preamble file and injected 
regions.

As discussed offline, I think injecting by synthesizing a header snippet which 
is mapped into the VFS and then added to PreprocessorOpts.Includes is 
preferable to tweaking the CompilerInvocation directly:
 - it extends to arbitrary directives, e.g. there's no way today to inject an 
`#import` directive via CompilerInvocation, no control over `<>` vs `""`, ...
 - it's easier to debug as the injected region can easily be dumped as text
 - it's easier to for code to recognize locations in our injected region if 
we're not sharing it with e.g. `-D`s coming from the commandline
 - (I think) it lets us use the PresumedLocation mechanism to record the 
association of each injected line with the corresponding line in the mainfile 
(not sure if this is valuable if we also need to translate the other way)

So I'd probably suggest something like

```
class PreamblePatch { // ends up stored in ParsedAST
public:
  PreamblePatch(); // empty, used when no patching is done
  static PreamblePatch compute(const ParseInputs&);

  void apply(CompilerInvocation&); // adjusts PPOptions.{RemappedFileBuffers, 
Includes} to include injected header

  friend operator<<(raw_ostream&, const PreamblePatch &); // probably just 
dumps InjectedHeaderContent

  // later... not sure about signature/names here
  SourceLocation parsedToUserLoc(SourceLocation);
  SourceLocation userToParsedLoc(SourceLocation); // is this also needed?

private:
  std::string InjectedHeaderContent; // with #line directives, or at least 
comments 
  // some data structure to map between locations in the stale preamble and 
their user locations
  // some data structure to map between locations in the injected header and 
their user locations
};
```



Comment at: clang-tools-extra/clangd/Headers.cpp:237
 
+std::vector getPreambleIncludes(llvm::StringRef Contents,
+   const LangOptions ) {

The tradeoff between raw-lexing and running the actual preprocessor in 
SingleFileParseMode seems pretty interesting.
(I'm not suggesting any particular change here, but I think it's worth 
exploring at some point. If the raw lexing is "for now" let's call that out, if 
it's a decision let's justify it. Also just curious what you think at this 
point - I know we've discussed this in the past and I pushed for raw-lexing)

---

The SingleFileParseMode is more accurate:
 - it's probably much easier to handle more directives correctly
 - it will handle ifdefs correctly where possible without reading files
 - it will tell us what includes actually resolve to (I'm not sure if this is 
useful though, when you can inject an #include with the right spelling and 
check that later)
 - ??? maybe we can recognize bad directives that we want to avoid injecting 
for better results ???
 - we don't have to do weird stuff like track the raw-lexed includes in 
preambledata

But potentially expensive with IO:
 - it will stat all the files along the search path.
 - it will ::realpath all the files that were found

Possible workaround for stat along the search path: most of the time, the 
#include directives are going to refer to files that were already included in 
the previous preamble (i.e. the preamble we're trying to patch) so we 
(heuristically) know what they resolve to.
If we used the HeaderSearchInfo's alias map to redirect  to 
 then `#include ` will just cost a single stat. 
And when our heuristic is wrong and  resolves to something else, we'll 
just get an include-not-found, we can still add the #include.

More radical workaround: give it a completely null VFS so it can't do any IO. 
All includes will fail to resolve, but if we don't need their resolved path at 
this point we don't care, right?



Comment at: clang-tools-extra/clangd/Headers.h:198
 
+/// Gets the includes in the preamble section of the file by running lexer over
+/// \p Contents. Returned inclusions are sorted according to written filename.

I'm guessing the APIs we end up with probably belong more in Preamble.h rather 
than here, both thematically and in terms of layering


Repository:
  rG LLVM Github Monorepo

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

[PATCH] D77697: libc++: adjust modulemap for non-modular C

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd created this revision.
compnerd added a reviewer: ldionne.
Herald added subscribers: libcxx-commits, dexonsmith.
Herald added a project: libc++.
Herald added a reviewer: libc++.

When building with a non-modularized C runtime, `csignal` would claim `time.h` 
and thus `timespec` rather than `ctime`.  Adjust the modulemap to ensure that 
`ctime` claims `timespec`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77697

Files:
  libcxx/include/module.modulemap


Index: libcxx/include/module.modulemap
===
--- libcxx/include/module.modulemap
+++ libcxx/include/module.modulemap
@@ -157,6 +157,13 @@
   header "csetjmp"
   export *
 }
+// NOTE: ensure that `ctime` preceeds `csignal` as otherwise, for a
+// non-modular C runtime, you would end up with `csignal` providing time
+// functions.
+module ctime {
+  header "ctime"
+  export *
+}
 module csignal {
   header "csignal"
   export *
@@ -197,10 +204,6 @@
   export cmath
   export *
 }
-module ctime {
-  header "ctime"
-  export *
-}
 // FIXME:  is missing.
 module cwchar {
   header "cwchar"


Index: libcxx/include/module.modulemap
===
--- libcxx/include/module.modulemap
+++ libcxx/include/module.modulemap
@@ -157,6 +157,13 @@
   header "csetjmp"
   export *
 }
+// NOTE: ensure that `ctime` preceeds `csignal` as otherwise, for a
+// non-modular C runtime, you would end up with `csignal` providing time
+// functions.
+module ctime {
+  header "ctime"
+  export *
+}
 module csignal {
   header "csignal"
   export *
@@ -197,10 +204,6 @@
   export cmath
   export *
 }
-module ctime {
-  header "ctime"
-  export *
-}
 // FIXME:  is missing.
 module cwchar {
   header "cwchar"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [clang] a8c8b62 - [ObjC generics] Fix not inheriting type bounds in categories/extensions.

[Volodymyr Sapsai via cfe-commits]

Can reproduce test failure locally, reverted in 
8fb7cfcea97af440830d256cc18ccd978f218e1d

Thanks for noticing the problem and pointing me to it.

> On Apr 6, 2020, at 23:28, Mikael Holmén  wrote:
> 
> We see flakiness in the test in our bots too. Fails one time and then
> passes again.
> 
> /Mikael
> 
> On Mon, 2020-04-06 at 21:03 -0400, Nico Weber via cfe-commits wrote:
>> This isn't bot-dependent, it's been flaking on many different bots
>> over the last few days. Here's one from just now: 
>> http://lab.llvm.org:8011/builders/clang-atom-d525-fedora-rel/builds/34705
>> 
>> On Mon, Apr 6, 2020 at 4:40 PM Volodymyr Sapsai 
>> wrote:
>>> Is there anything special in the builedbot configuration? Are you
>>> still observing intermittent failures?
>>> 
>>> I’m double checking if we see similar failures internally but so
>>> far looks like everything is working fine. I’ve only seen a single
>>> failure 
>>> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-ubsan/builds/18616
>>> 
>>> At the moment I don’t know what might be causing the non-
>>> determinism. The type checking depends on the parsing order. We
>>> rely that
>>> 
>>> @interface ParameterizedContainer (Cat)
>>> 
>>> is parsed before
>>> 
>>> - (ParameterizedContainer *)inCategory;
>>> 
>>> So we check type parameter T is consistent with original @interface
>>> before using it. What is also confusing is that there is only a
>>> single error. I  expect both a category and an extension to have
>>> same errors.
>>> 
 On Apr 5, 2020, at 10:10, Nico Weber  wrote:
 
 The test here flakily fails, maybe 1 in 10 times: 
 http://45.33.8.238/mac/11180/step_7.txt
 
 error: 'error' diagnostics seen but not expected: 
  File /Users/thakis/src/llvm-
 project/clang/test/SemaObjC/parameterized_classes_subst.m Line
 479: type argument 'T' (aka 'id') does not satisfy the bound
 ('id') of type parameter 'T'
 error: 'note' diagnostics seen but not expected: 
  File /Users/thakis/src/llvm-
 project/clang/test/SemaObjC/parameterized_classes_subst.m Line
 475: type parameter 'T' declared here
 2 errors generated.
 
 Maybe if this is emitted depends on the order of something in a
 data structure that has no deterministic order, or similar?
 
 On Fri, Apr 3, 2020 at 7:29 PM Volodymyr Sapsai via cfe-commits <
 cfe-commits@lists.llvm.org> wrote:
> Author: Volodymyr Sapsai
> Date: 2020-04-03T16:29:02-07:00
> New Revision: a8c8b627f23f204fb621bd2a8c495cfc8bc16ae7
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/a8c8b627f23f204fb621bd2a8c495cfc8bc16ae7
> DIFF: 
> https://github.com/llvm/llvm-project/commit/a8c8b627f23f204fb621bd2a8c495cfc8bc16ae7.diff
> 
> LOG: [ObjC generics] Fix not inheriting type bounds in
> categories/extensions.
> 
> When a category/extension doesn't repeat a type bound,
> corresponding
> type parameter is substituted with `id` when used as a type
> argument. As
> a result, in the added test case it was causing errors like
> 
>> type argument 'T' (aka 'id') does not satisfy the bound
> ('id') of type parameter 'T'
> 
> We are already checking that type parameters should be
> consistent
> everywhere (see `checkTypeParamListConsistency`) and update
> `ObjCTypeParamDecl` to have correct underlying type. And when
> we use the
> type parameter as a method return type or a method parameter
> type, it is
> substituted to the bounded type. But when we use the type
> parameter as a
> type argument, we check `ObjCTypeParamType` that wasn't updated
> and
> remains `id`.
> 
> Fix by updating not only `ObjCTypeParamDecl` UnderlyingType but
> also
> TypeForDecl as we use the underlying type to create a canonical
> type for
> `ObjCTypeParamType` (see `ASTContext::getObjCTypeParamType`).
> 
> This is a different approach to fixing the issue. The previous
> one was
> 02c2ab3d8872416589bd1a6ca3dfb96ba373a3b9 which was reverted in
> 4c539e8da1b3de38a53ef3f7497f5c45a3243b61. The problem with the
> previous
> approach was that `ObjCTypeParamType::desugar` was returning
> underlying
> type for `ObjCTypeParamDecl` without applying any protocols
> stored in
> `ObjCTypeParamType`. It caused inconsistencies in comparing
> types before
> and after desugaring.
> 
> rdar://problem/54329242
> 
> Reviewed By: erik.pilkington
> 
> Differential Revision: https://reviews.llvm.org/D72872
> 
> Added: 
> 
> 
> Modified: 
>clang/include/clang/AST/ASTContext.h
>clang/lib/AST/ASTContext.cpp
>clang/lib/AST/Type.cpp
>clang/lib/Sema/SemaDeclObjC.cpp
> 
> clang/test/SemaObjC/parameterized_classes_collection_literal.m
>clang/test/SemaObjC/parameterized_classes_subst.m
> 
> Removed: 
> 

[clang] 8fb7cfc - Revert "[ObjC generics] Fix not inheriting type bounds in categories/extensions."

[Volodymyr Sapsai via cfe-commits]

Author: Volodymyr Sapsai
Date: 2020-04-07T17:41:30-07:00
New Revision: 8fb7cfcea97af440830d256cc18ccd978f218e1d

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

LOG: Revert "[ObjC generics] Fix not inheriting type bounds in 
categories/extensions."

This reverts commit a8c8b627f23f204fb621bd2a8c495cfc8bc16ae7. It causes
intermittent

Clang :: SemaObjC/parameterized_classes_subst.m

test failures on various bots.

Added: 


Modified: 
clang/include/clang/AST/ASTContext.h
clang/lib/AST/ASTContext.cpp
clang/lib/AST/Type.cpp
clang/lib/Sema/SemaDeclObjC.cpp
clang/test/SemaObjC/parameterized_classes_collection_literal.m
clang/test/SemaObjC/parameterized_classes_subst.m

Removed: 




diff  --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index 6360f18217c7..6813ab58874e 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1442,8 +1442,6 @@ class ASTContext : public RefCountedBase {
 
   QualType getObjCTypeParamType(const ObjCTypeParamDecl *Decl,
 ArrayRef protocols) const;
-  void adjustObjCTypeParamBoundType(const ObjCTypeParamDecl *Orig,
-ObjCTypeParamDecl *New) const;
 
   bool ObjCObjectAdoptsQTypeProtocols(QualType QT, ObjCInterfaceDecl *Decl);
 

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 06dcb6fa0580..1e81e0a67b4d 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -4874,17 +4874,6 @@ ASTContext::getObjCTypeParamType(const ObjCTypeParamDecl 
*Decl,
   return QualType(newType, 0);
 }
 
-void ASTContext::adjustObjCTypeParamBoundType(const ObjCTypeParamDecl *Orig,
-  ObjCTypeParamDecl *New) const {
-  New->setTypeSourceInfo(getTrivialTypeSourceInfo(Orig->getUnderlyingType()));
-  // Update TypeForDecl after updating TypeSourceInfo.
-  auto NewTypeParamTy = cast(New->getTypeForDecl());
-  SmallVector protocols;
-  protocols.append(NewTypeParamTy->qual_begin(), NewTypeParamTy->qual_end());
-  QualType UpdatedTy = getObjCTypeParamType(New, protocols);
-  New->setTypeForDecl(UpdatedTy.getTypePtr());
-}
-
 /// ObjCObjectAdoptsQTypeProtocols - Checks that protocols in IC's
 /// protocol list adopt all protocols in QT's qualified-id protocol
 /// list.

diff  --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 7c65378261ad..3428437c3146 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -3534,7 +3534,6 @@ void ObjCTypeParamType::Profile(llvm::FoldingSetNodeID 
,
 const ObjCTypeParamDecl *OTPDecl,
 ArrayRef protocols) {
   ID.AddPointer(OTPDecl);
-  ID.AddPointer(OTPDecl->getUnderlyingType().getAsOpaquePtr());
   ID.AddInteger(protocols.size());
   for (auto proto : protocols)
 ID.AddPointer(proto);

diff  --git a/clang/lib/Sema/SemaDeclObjC.cpp b/clang/lib/Sema/SemaDeclObjC.cpp
index 6db57898e378..934e1a23141c 100644
--- a/clang/lib/Sema/SemaDeclObjC.cpp
+++ b/clang/lib/Sema/SemaDeclObjC.cpp
@@ -938,7 +938,8 @@ static bool checkTypeParamListConsistency(Sema ,
 
   // Override the new type parameter's bound type with the previous type,
   // so that it's consistent.
-  S.Context.adjustObjCTypeParamBoundType(prevTypeParam, newTypeParam);
+  newTypeParam->setTypeSourceInfo(
+
S.Context.getTrivialTypeSourceInfo(prevTypeParam->getUnderlyingType()));
   continue;
 }
 
@@ -965,7 +966,8 @@ static bool checkTypeParamListConsistency(Sema ,
 }
 
 // Update the new type parameter's bound to match the previous one.
-S.Context.adjustObjCTypeParamBoundType(prevTypeParam, newTypeParam);
+newTypeParam->setTypeSourceInfo(
+  S.Context.getTrivialTypeSourceInfo(prevTypeParam->getUnderlyingType()));
   }
 
   return false;

diff  --git a/clang/test/SemaObjC/parameterized_classes_collection_literal.m 
b/clang/test/SemaObjC/parameterized_classes_collection_literal.m
index 034d2e8da217..472746e09db9 100644
--- a/clang/test/SemaObjC/parameterized_classes_collection_literal.m
+++ b/clang/test/SemaObjC/parameterized_classes_collection_literal.m
@@ -29,9 +29,7 @@ + (instancetype)arrayWithObjects:(const T [])objects 
count:(NSUInteger)cnt;
 @end
 
 @interface NSDictionary : NSObject 
-+ (instancetype)dictionaryWithObjects:(const V [])objects
-  forKeys:(const K  [])keys
-count:(NSUInteger)cnt;
++ (instancetype)dictionaryWithObjects:(const V [])objects forKeys:(const K 
[])keys count:(NSUInteger)cnt;
 @end
 
 void testArrayLiteral(void) {
@@ -52,9 +50,3 @@ void testDictionaryLiteral(void) {
 @"world" : @"blah" // 

[PATCH] D77683: [Docs] Make code review policy clearer about requested pre-commit reviews

[Chris Lattner via Phabricator via cfe-commits]

lattner added a comment.

I don't really understand what this is getting at, so I'd recommend rewording 
this a bit for clarity.  Would something along the lines of this capture the 
intended meaning?:

If there is likely to be uncertainty, you should default to getting a patch 
reviewed prior to commit, particularly if someone has asked for extra review of 
a specific area.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77683



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

[PATCH] D77683: [Docs] Make code review policy clearer about requested pre-commit reviews

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: llvm/docs/CodeReview.rst:34
+uncertainty, a patch should be reviewed prior to being committed. If pre-commit
+code reviewes in a particular area have been requested, code should clear a
+significantly higher bar, e.g., fixes, to be commited without review.

arsenm wrote:
> jdoerfert wrote:
> > hubert.reinterpretcast wrote:
> > > If I understand this correctly, this is meant to cover situations where 
> > > reviewers are active in an area and indicated an interest in reviewing 
> > > basically everything. This seems redundant to likely-community-consensus. 
> > > If someone is active in an area of the code, they would already know 
> > > which reviewers are active and the area-specific culture. If someone is 
> > > not active in an area of the code, the likely-community-consensus 
> > > requirement should be enough to cause them to request a review.
> > > 
> > > Perhaps a clarification that "the likely-community-consensus requirement 
> > > may vary depending on the specific area of the code" is enough. Or 
> > > perhaps, "likely-community-consensus is not restricted to the content of 
> > > the patch, but also includes the review process and culture".
> > > If I understand this correctly, this is meant to cover situations where 
> > > reviewers are active in an area and indicated an interest in reviewing 
> > > basically everything. 
> > 
> > Pretty much, yes.
> > 
> > > This seems redundant to likely-community-consensus. If someone is active 
> > > in an area of the code, they would already know which reviewers are 
> > > active and the area-specific culture. If someone is not active in an area 
> > > of the code, the likely-community-consensus requirement should be enough 
> > > to cause them to request a review.
> > 
> > I would have agreed with you but this is in practice unfortunately not true.
> > 
> > > Perhaps a clarification that "the likely-community-consensus requirement 
> > > may vary depending on the specific area of the code" is enough. Or 
> > > perhaps, "likely-community-consensus is not restricted to the content of 
> > > the patch, but also includes the review process and culture".
> > 
> > I'm fine with either. At the end of the day this is supposed to make it 
> > clear that reviews cannot be circumvented while pointing to this rule if it 
> > is clear reviews were requested.
> Typo reviewes, and commited
I'd suggest a process of checking for Herald rules if I knew of an easy way to 
know how Herald would react for a commit without posting a review. I don't know 
of such a way though.

Suggestion for wording:
The likely-community-consensus is not restricted to the content of the patch 
but also includes the review process and culture, which can vary by context. A 
strong pre-commit culture in an area of the code should be respected.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77683



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

[PATCH] D77683: [Docs] Make code review policy clearer about requested pre-commit reviews

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: llvm/docs/CodeReview.rst:34
+uncertainty, a patch should be reviewed prior to being committed. If pre-commit
+code reviewes in a particular area have been requested, code should clear a
+significantly higher bar, e.g., fixes, to be commited without review.

jdoerfert wrote:
> hubert.reinterpretcast wrote:
> > If I understand this correctly, this is meant to cover situations where 
> > reviewers are active in an area and indicated an interest in reviewing 
> > basically everything. This seems redundant to likely-community-consensus. 
> > If someone is active in an area of the code, they would already know which 
> > reviewers are active and the area-specific culture. If someone is not 
> > active in an area of the code, the likely-community-consensus requirement 
> > should be enough to cause them to request a review.
> > 
> > Perhaps a clarification that "the likely-community-consensus requirement 
> > may vary depending on the specific area of the code" is enough. Or perhaps, 
> > "likely-community-consensus is not restricted to the content of the patch, 
> > but also includes the review process and culture".
> > If I understand this correctly, this is meant to cover situations where 
> > reviewers are active in an area and indicated an interest in reviewing 
> > basically everything. 
> 
> Pretty much, yes.
> 
> > This seems redundant to likely-community-consensus. If someone is active in 
> > an area of the code, they would already know which reviewers are active and 
> > the area-specific culture. If someone is not active in an area of the code, 
> > the likely-community-consensus requirement should be enough to cause them 
> > to request a review.
> 
> I would have agreed with you but this is in practice unfortunately not true.
> 
> > Perhaps a clarification that "the likely-community-consensus requirement 
> > may vary depending on the specific area of the code" is enough. Or perhaps, 
> > "likely-community-consensus is not restricted to the content of the patch, 
> > but also includes the review process and culture".
> 
> I'm fine with either. At the end of the day this is supposed to make it clear 
> that reviews cannot be circumvented while pointing to this rule if it is 
> clear reviews were requested.
Typo reviewes, and commited


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77683



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

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

[Andrew via Phabricator via cfe-commits]

browneee updated this revision to Diff 255856.
browneee added a comment.

Fix build errors. Missed -DLLVM_ENABLE_PROJECTS in previous local test builds.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77621

Files:
  clang/include/clang/Serialization/ASTWriter.h
  clang/include/clang/Serialization/PCHContainerOperations.h
  clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/PrecompiledPreamble.cpp
  clang/lib/Frontend/SerializedDiagnosticPrinter.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/GlobalModuleIndex.cpp
  clang/lib/Serialization/PCHContainerOperations.cpp
  llvm/include/llvm/Bitcode/BitcodeWriter.h
  llvm/include/llvm/Bitstream/BitstreamWriter.h
  llvm/include/llvm/Remarks/BitstreamRemarkSerializer.h
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/ExecutionEngine/Orc/ThreadSafeModule.cpp
  llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
  llvm/tools/llvm-cat/llvm-cat.cpp
  llvm/tools/llvm-modextract/llvm-modextract.cpp
  llvm/unittests/Bitstream/BitstreamReaderTest.cpp
  llvm/unittests/Bitstream/BitstreamWriterTest.cpp

Index: llvm/unittests/Bitstream/BitstreamWriterTest.cpp
===
--- llvm/unittests/Bitstream/BitstreamWriterTest.cpp
+++ llvm/unittests/Bitstream/BitstreamWriterTest.cpp
@@ -8,27 +8,31 @@
 
 #include "llvm/Bitstream/BitstreamWriter.h"
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SmallString.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
+#include 
+
 using namespace llvm;
 
+using ::testing::IsEmpty;
+
 namespace {
 
 TEST(BitstreamWriterTest, emitBlob) {
-  SmallString<64> Buffer;
+  std::vector Buffer;
   BitstreamWriter W(Buffer);
   W.emitBlob("str", /* ShouldEmitSize */ false);
-  EXPECT_EQ(StringRef("str\0", 4), Buffer);
+  EXPECT_EQ(StringRef("str\0", 4), StringRef(Buffer.data(), Buffer.size()));
 }
 
 TEST(BitstreamWriterTest, emitBlobWithSize) {
-  SmallString<64> Buffer;
+  std::vector Buffer;
   {
 BitstreamWriter W(Buffer);
 W.emitBlob("str");
   }
-  SmallString<64> Expected;
+  std::vector Expected;
   {
 BitstreamWriter W(Expected);
 W.EmitVBR(3, 6);
@@ -38,21 +42,21 @@
 W.Emit('r', 8);
 W.Emit(0, 8);
   }
-  EXPECT_EQ(StringRef(Expected), Buffer);
+  EXPECT_EQ(Expected, Buffer);
 }
 
 TEST(BitstreamWriterTest, emitBlobEmpty) {
-  SmallString<64> Buffer;
+  std::vector Buffer;
   BitstreamWriter W(Buffer);
   W.emitBlob("", /* ShouldEmitSize */ false);
-  EXPECT_EQ(StringRef(""), Buffer);
+  EXPECT_THAT(Buffer, IsEmpty());
 }
 
 TEST(BitstreamWriterTest, emitBlob4ByteAligned) {
-  SmallString<64> Buffer;
+  std::vector Buffer;
   BitstreamWriter W(Buffer);
   W.emitBlob("str0", /* ShouldEmitSize */ false);
-  EXPECT_EQ(StringRef("str0"), Buffer);
+  EXPECT_EQ(StringRef("str0"), StringRef(Buffer.data(), Buffer.size()));
 }
 
 } // end namespace
Index: llvm/unittests/Bitstream/BitstreamReaderTest.cpp
===
--- llvm/unittests/Bitstream/BitstreamReaderTest.cpp
+++ llvm/unittests/Bitstream/BitstreamReaderTest.cpp
@@ -11,6 +11,8 @@
 #include "llvm/Bitstream/BitstreamWriter.h"
 #include "gtest/gtest.h"
 
+#include 
+
 using namespace llvm;
 
 namespace {
@@ -96,7 +98,7 @@
 StringRef BlobIn((const char *)BlobData.begin(), BlobSize);
 
 // Write the bitcode.
-SmallVector Buffer;
+std::vector Buffer;
 unsigned AbbrevID;
 {
   BitstreamWriter Stream(Buffer);
@@ -115,7 +117,7 @@
 
 // Stream the buffer into the reader.
 BitstreamCursor Stream(
-ArrayRef((const uint8_t *)Buffer.begin(), Buffer.size()));
+ArrayRef((const uint8_t *)Buffer.data(), Buffer.size()));
 
 // Header.  Included in test so that we can run llvm-bcanalyzer to debug
 // when there are problems.
Index: llvm/tools/llvm-modextract/llvm-modextract.cpp
===
--- llvm/tools/llvm-modextract/llvm-modextract.cpp
+++ llvm/tools/llvm-modextract/llvm-modextract.cpp
@@ -58,12 +58,12 @@
   ExitOnErr(errorCodeToError(EC));
 
   if (BinaryExtract) {
-SmallVector Result;
+std::vector Result;
 BitcodeWriter Writer(Result);
-Result.append(Ms[ModuleIndex].getBuffer().begin(),
+Result.insert(Result.end(), Ms[ModuleIndex].getBuffer().begin(),
   Ms[ModuleIndex].getBuffer().end());
 Writer.copyStrtab(Ms[ModuleIndex].getStrtab());
-Out->os() << Result;
+Out->os().write(Result.data(), Result.size());
 Out->keep();
 return 0;
   }
Index: llvm/tools/llvm-cat/llvm-cat.cpp
===
--- llvm/tools/llvm-cat/llvm-cat.cpp
+++ llvm/tools/llvm-cat/llvm-cat.cpp
@@ -54,7 +54,7 @@
   ExitOnError ExitOnErr("llvm-cat: ");
   LLVMContext Context;
 
-  SmallVector Buffer;
+  

[PATCH] D77341: [DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff.

[Alina Sbirlea via Phabricator via cfe-commits]

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



Comment at: llvm/include/llvm/Support/GenericDomTreeConstruction.h:325
+auto  = BUI ? BUI->PreViewCFG : EmptyGD;
+return !empty(children({, N}));
   }

kuhar wrote:
> This pattern repeats a few times. Could it be pushed into a helper function 
> to get something like this?
> 
> ```
> return !empty(children(GetGraphDiffNodePair(BUI, N)));
> ```
> 
My dilemma here is how to not allocate a GraphDiffT instance. There are 4 cases 
where the pattern is inside a static method and once in a class method. I used 
a stack var for the 4 cases and a unique_ptr for the class method.

Sure, I can do:
```
static auto getGDNPair(BUI, EmptyGD, N) {
return std::make_pair<>(BUI ? BUI->PreViewCFG : EmptyGD, N);
}
{
...
GraphDiffT EmptyGD;
return !empty(children(getGDNPair(BUI, , N)));
}
```
But it felt like moving one line at the callsite to a oneline helper is not 
making things much better.


I was hoping for something more along the lines of:
```
return !empty(children({getGD(BUI), N});
```
Or, ensure BUI is always non-null, and simplify to:
```
return !empty(children({BUI->PreViewCFG, N});
```


Thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77341



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

[PATCH] D77341: [DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff.

[Alina Sbirlea via Phabricator via cfe-commits]

asbirlea updated this revision to Diff 255852.
asbirlea marked an inline comment as done.
asbirlea added a comment.

Name anonymous namespace.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77341

Files:
  clang/include/clang/Analysis/Analyses/Dominators.h
  llvm/include/llvm/IR/CFGDiff.h
  llvm/include/llvm/IR/Dominators.h
  llvm/include/llvm/Support/GenericDomTree.h
  llvm/include/llvm/Support/GenericDomTreeConstruction.h
  llvm/lib/IR/Dominators.cpp

Index: llvm/lib/IR/Dominators.cpp
===
--- llvm/lib/IR/Dominators.cpp
+++ llvm/lib/IR/Dominators.cpp
@@ -90,9 +90,10 @@
 DomTreeBuilder::BBPostDomTree , BasicBlock *From, BasicBlock *To);
 
 template void llvm::DomTreeBuilder::ApplyUpdates(
-DomTreeBuilder::BBDomTree , DomTreeBuilder::BBUpdates);
+DomTreeBuilder::BBDomTree , DomTreeBuilder::BBDomTreeGraphDiff &);
 template void llvm::DomTreeBuilder::ApplyUpdates(
-DomTreeBuilder::BBPostDomTree , DomTreeBuilder::BBUpdates);
+DomTreeBuilder::BBPostDomTree ,
+DomTreeBuilder::BBPostDomTreeGraphDiff &);
 
 template bool llvm::DomTreeBuilder::Verify(
 const DomTreeBuilder::BBDomTree ,
Index: llvm/include/llvm/Support/GenericDomTreeConstruction.h
===
--- llvm/include/llvm/Support/GenericDomTreeConstruction.h
+++ llvm/include/llvm/Support/GenericDomTreeConstruction.h
@@ -58,6 +58,13 @@
   using TreeNodePtr = DomTreeNodeBase *;
   using RootsT = decltype(DomTreeT::Roots);
   static constexpr bool IsPostDom = DomTreeT::IsPostDominator;
+  using GraphDiffT = GraphDiff;
+  using GraphDiffNodePair = std::pair;
+  using GraphDiffInvNodePair = std::pair>;
+  using GraphDiffNodePairIfDomT =
+  std::conditional_t;
+  using GraphDiffNodePairIfPDomT =
+  std::conditional_t;
 
   // Information record used by Semi-NCA during tree construction.
   struct InfoRec {
@@ -77,28 +84,27 @@
   using UpdateT = typename DomTreeT::UpdateType;
   using UpdateKind = typename DomTreeT::UpdateKind;
   struct BatchUpdateInfo {
-SmallVector Updates;
-using NodePtrAndKind = PointerIntPair;
-
-// In order to be able to walk a CFG that is out of sync with the CFG
-// DominatorTree last knew about, use the list of updates to reconstruct
-// previous CFG versions of the current CFG. For each node, we store a set
-// of its virtually added/deleted future successors and predecessors.
-// Note that these children are from the future relative to what the
-// DominatorTree knows about -- using them to gets us some snapshot of the
-// CFG from the past (relative to the state of the CFG).
-DenseMap> FutureSuccessors;
-DenseMap> FuturePredecessors;
+// Note: Updates inside PreViewCFG are aleady legalized.
+BatchUpdateInfo(GraphDiffT )
+: PreViewCFG(PreViewCFG),
+  NumLegalized(PreViewCFG.getNumLegalizedUpdates()) {}
+
 // Remembers if the whole tree was recalculated at some point during the
 // current batch update.
 bool IsRecalculated = false;
+GraphDiffT 
+const size_t NumLegalized;
   };
 
   BatchUpdateInfo *BatchUpdates;
   using BatchUpdatePtr = BatchUpdateInfo *;
+  std::unique_ptr EmptyGD;
 
   // If BUI is a nullptr, then there's no batch update in progress.
-  SemiNCAInfo(BatchUpdatePtr BUI) : BatchUpdates(BUI) {}
+  SemiNCAInfo(BatchUpdatePtr BUI) : BatchUpdates(BUI) {
+if (!BatchUpdates)
+  EmptyGD = std::make_unique();
+  }
 
   void clear() {
 NumToNode = {nullptr}; // Restore to initial state with a dummy start node.
@@ -107,67 +113,6 @@
 // in progress, we need this information to continue it.
   }
 
-  template 
-  struct ChildrenGetter {
-using ResultTy = SmallVector;
-
-static ResultTy Get(NodePtr N, std::integral_constant) {
-  auto RChildren = reverse(children(N));
-  return ResultTy(RChildren.begin(), RChildren.end());
-}
-
-static ResultTy Get(NodePtr N, std::integral_constant) {
-  auto IChildren = inverse_children(N);
-  return ResultTy(IChildren.begin(), IChildren.end());
-}
-
-using Tag = std::integral_constant;
-
-// The function below is the core part of the batch updater. It allows the
-// Depth Based Search algorithm to perform incremental updates in lockstep
-// with updates to the CFG. We emulated lockstep CFG updates by getting its
-// next snapshots by reverse-applying future updates.
-static ResultTy Get(NodePtr N, BatchUpdatePtr BUI) {
-  ResultTy Res = Get(N, Tag());
-  // If there's no batch update in progress, simply return node's children.
-  if (!BUI) return Res;
-
-  // CFG children are actually its *most current* children, and we have to
-  // reverse-apply the future updates to get the node's children at the
-  // point in time the update was performed.
-  auto  = (Inverse != 

[PATCH] D77683: [Docs] Make code review policy clearer about requested pre-commit reviews

[Johannes Doerfert via Phabricator via cfe-commits]

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



Comment at: llvm/docs/CodeReview.rst:34
+uncertainty, a patch should be reviewed prior to being committed. If pre-commit
+code reviewes in a particular area have been requested, code should clear a
+significantly higher bar, e.g., fixes, to be commited without review.

hubert.reinterpretcast wrote:
> If I understand this correctly, this is meant to cover situations where 
> reviewers are active in an area and indicated an interest in reviewing 
> basically everything. This seems redundant to likely-community-consensus. If 
> someone is active in an area of the code, they would already know which 
> reviewers are active and the area-specific culture. If someone is not active 
> in an area of the code, the likely-community-consensus requirement should be 
> enough to cause them to request a review.
> 
> Perhaps a clarification that "the likely-community-consensus requirement may 
> vary depending on the specific area of the code" is enough. Or perhaps, 
> "likely-community-consensus is not restricted to the content of the patch, 
> but also includes the review process and culture".
> If I understand this correctly, this is meant to cover situations where 
> reviewers are active in an area and indicated an interest in reviewing 
> basically everything. 

Pretty much, yes.

> This seems redundant to likely-community-consensus. If someone is active in 
> an area of the code, they would already know which reviewers are active and 
> the area-specific culture. If someone is not active in an area of the code, 
> the likely-community-consensus requirement should be enough to cause them to 
> request a review.

I would have agreed with you but this is in practice unfortunately not true.

> Perhaps a clarification that "the likely-community-consensus requirement may 
> vary depending on the specific area of the code" is enough. Or perhaps, 
> "likely-community-consensus is not restricted to the content of the patch, 
> but also includes the review process and culture".

I'm fine with either. At the end of the day this is supposed to make it clear 
that reviews cannot be circumvented while pointing to this rule if it is clear 
reviews were requested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77683



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

[PATCH] D77688: [CUDA] Improve testing of libdevice detection.

[Artem Belevich via Phabricator via cfe-commits]

tra created this revision.
tra added a reviewer: yaxunl.
Herald added subscribers: sanjoy.google, bixia.
Herald added a project: clang.

Added new testcases for libdevice in CUDA-9+ and removed unused checks.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77688

Files:
  clang/test/Driver/cuda-detect.cu


Index: clang/test/Driver/cuda-detect.cu
===
--- clang/test/Driver/cuda-detect.cu
+++ clang/test/Driver/cuda-detect.cu
@@ -51,49 +51,64 @@
 // RUN: %clang -### -v --target=i386-unknown-linux --cuda-gpu-arch=sm_21 \
 // RUN:   --cuda-path=%S/Inputs/CUDA_80/usr/local/cuda %s 2>&1 \
 // RUN:   | FileCheck %s -check-prefix COMMON \
-// RUN: -check-prefix LIBDEVICE -check-prefix LIBDEVICE20
+// RUN: -check-prefixes PTX42,LIBDEVICE,LIBDEVICE20
 // RUN: %clang -### -v --target=i386-unknown-linux --cuda-gpu-arch=sm_32 \
 // RUN:   --cuda-path=%S/Inputs/CUDA_80/usr/local/cuda %s 2>&1 \
 // RUN:   | FileCheck %s -check-prefix COMMON \
-// RUN: -check-prefix LIBDEVICE -check-prefix LIBDEVICE20
+// RUN: -check-prefixes PTX42,LIBDEVICE,LIBDEVICE20
 // sm_30, sm_6x map to compute_30.
 // RUN: %clang -### -v --target=i386-unknown-linux --cuda-gpu-arch=sm_30 \
 // RUN:   --cuda-path=%S/Inputs/CUDA_80/usr/local/cuda %s 2>&1 \
 // RUN:   | FileCheck %s -check-prefix COMMON \
-// RUN: -check-prefix LIBDEVICE -check-prefix LIBDEVICE30
+// RUN: -check-prefixes PTX42,LIBDEVICE,LIBDEVICE30
 // sm_5x is a special case. Maps to compute_30 for cuda-7.x only.
 // RUN: %clang -### -v --target=i386-unknown-linux --cuda-gpu-arch=sm_50 \
 // RUN:   --cuda-path=%S/Inputs/CUDA/usr/local/cuda %s 2>&1 \
 // RUN:   | FileCheck %s -check-prefix COMMON \
-// RUN: -check-prefix LIBDEVICE -check-prefix LIBDEVICE30
+// RUN: -check-prefixes PTX42,LIBDEVICE,LIBDEVICE30
 // RUN: %clang -### -v --target=i386-unknown-linux --cuda-gpu-arch=sm_60 \
 // RUN:   --cuda-path=%S/Inputs/CUDA_80/usr/local/cuda %s 2>&1 \
 // RUN:   | FileCheck %s -check-prefix COMMON \
-// RUN: -check-prefix LIBDEVICE -check-prefix LIBDEVICE30
+// RUN: -check-prefixes PTX42,LIBDEVICE,LIBDEVICE30
 // sm_35 and sm_37 -> compute_35
 // RUN: %clang -### -v --target=i386-unknown-linux --cuda-gpu-arch=sm_35 \
 // RUN:   --cuda-path=%S/Inputs/CUDA_80/usr/local/cuda %s 2>&1 \
 // RUN:   | FileCheck %s -check-prefix COMMON -check-prefix CUDAINC \
-// RUN: -check-prefix LIBDEVICE -check-prefix LIBDEVICE35
+// RUN: -check-prefixes PTX42,LIBDEVICE,LIBDEVICE35
 // RUN: %clang -### -v --target=i386-unknown-linux --cuda-gpu-arch=sm_37 \
 // RUN:   --cuda-path=%S/Inputs/CUDA_80/usr/local/cuda %s 2>&1 \
 // RUN:   | FileCheck %s -check-prefix COMMON -check-prefix CUDAINC \
-// RUN: -check-prefix LIBDEVICE -check-prefix LIBDEVICE35
+// RUN: -check-prefixes PTX42,LIBDEVICE,LIBDEVICE35
 // sm_5x -> compute_50 for CUDA-8.0 and newer.
 // RUN: %clang -### -v --target=i386-unknown-linux --cuda-gpu-arch=sm_50 \
 // RUN:   --cuda-path=%S/Inputs/CUDA_80/usr/local/cuda %s 2>&1 \
 // RUN:   | FileCheck %s -check-prefix COMMON \
-// RUN: -check-prefix LIBDEVICE -check-prefix LIBDEVICE50
+// RUN: -check-prefixes PTX42,LIBDEVICE,LIBDEVICE50
+
+// CUDA-9+ uses the same libdevice for all GPU variants:
+// RUN: %clang -### -v --target=x86_64-unknown-linux --cuda-gpu-arch=sm_30 \
+// RUN:   --cuda-path=%S/Inputs/CUDA_90/usr/local/cuda %s 2>&1 \
+// RUN:   | FileCheck %s -check-prefix COMMON64 \
+// RUN: -check-prefixes PTX60,LIBDEVICE,LIBDEVICE10
+// RUN: %clang -### -v --target=x86_64-unknown-linux --cuda-gpu-arch=sm_50 \
+// RUN:   --cuda-path=%S/Inputs/CUDA_90/usr/local/cuda %s 2>&1 \
+// RUN:   | FileCheck %s -check-prefix COMMON64 \
+// RUN: -check-prefixes PTX60,LIBDEVICE,LIBDEVICE10
+// RUN: %clang -### -v --target=x86_64-unknown-linux --cuda-gpu-arch=sm_60 \
+// RUN:   --cuda-path=%S/Inputs/CUDA_90/usr/local/cuda %s 2>&1 \
+// RUN:   | FileCheck %s -check-prefix COMMON64 \
+// RUN: -check-prefixes PTX60,LIBDEVICE,LIBDEVICE10
+
 
 // Verify that -nocudainc prevents adding include path to CUDA headers.
 // RUN: %clang -### -v --target=i386-unknown-linux --cuda-gpu-arch=sm_35 \
 // RUN:   -nocudainc --cuda-path=%S/Inputs/CUDA/usr/local/cuda %s 2>&1 \
 // RUN:   | FileCheck %s -check-prefix COMMON -check-prefix NOCUDAINC \
-// RUN: -check-prefix LIBDEVICE -check-prefix LIBDEVICE35
+// RUN: -check-prefixes PTX42,LIBDEVICE,LIBDEVICE35
 // RUN: %clang -### -v --target=i386-apple-macosx --cuda-gpu-arch=sm_35 \
 // RUN:   -nocudainc --cuda-path=%S/Inputs/CUDA/usr/local/cuda %s 2>&1 \
 // RUN:   | FileCheck %s -check-prefix COMMON -check-prefix NOCUDAINC \
-// RUN: -check-prefix LIBDEVICE -check-prefix LIBDEVICE35
+// RUN: -check-prefixes PTX42,LIBDEVICE,LIBDEVICE35
 
 // We should not add any CUDA include paths if there's no valid CUDA 
installation
 // RUN: %clang -### -v --target=i386-unknown-linux --cuda-gpu-arch=sm_35 \
@@ -154,16 

[PATCH] D77611: [Sema] Check calls to __attribute__((warn_unused_result)) from StmtExprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D77611#1968227 , @dblaikie wrote:

> Looks like GCC warns on everything but the last unused expression (the last 
> one being the return statement) - only warning on that last one if it's 
> ultimately unused by the statement expression function call, as it were (& is 
> annotated warn_unused_result). Basically, it models it like a function in a 
> way that Clang perhaps does not?


For the case of warn_unused_result, it doesn't matter whether the unused call 
is the first or last statement in the statement expression: 
https://godbolt.org/z/DDgsVC (notice that trunk of LLVM warns now, just not for 
the return value case from my added test).

So GCC treats the return value from statement expressions differently from 
return values from functions in this regard: https://godbolt.org/z/83wP7W


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77611



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

[PATCH] D77498: [Hexagon] Select lld as the default linker for linux-musl target

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

> These are new tests how do you get the generic lld driver to work?

It doesn't, Clang invokes the appropriate driver in 
`ToolChain::GetLinkerPath()`: 
https://github.com/llvm/llvm-project/blob/6011627f5118dd64a0c33694b604c70e766d8c40/clang/lib/Driver/ToolChain.cpp#L537-L541


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77498



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

[PATCH] D77611: [Sema] Check calls to __attribute__((warn_unused_result)) from StmtExprs

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Looks like GCC warns on everything but the last unused expression (the last one 
being the return statement) - only warning on that last one if it's ultimately 
unused by the statement expression function call, as it were (& is annotated 
warn_unused_result). Basically, it models it like a function in a way that 
Clang perhaps does not?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77611



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

[PATCH] D77683: [Docs] Make code review policy clearer about requested pre-commit reviews

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: llvm/docs/CodeReview.rst:34
+uncertainty, a patch should be reviewed prior to being committed. If pre-commit
+code reviewes in a particular area have been requested, code should clear a
+significantly higher bar, e.g., fixes, to be commited without review.

If I understand this correctly, this is meant to cover situations where 
reviewers are active in an area and indicated an interest in reviewing 
basically everything. This seems redundant to likely-community-consensus. If 
someone is active in an area of the code, they would already know which 
reviewers are active and the area-specific culture. If someone is not active in 
an area of the code, the likely-community-consensus requirement should be 
enough to cause them to request a review.

Perhaps a clarification that "the likely-community-consensus requirement may 
vary depending on the specific area of the code" is enough. Or perhaps, 
"likely-community-consensus is not restricted to the content of the patch, but 
also includes the review process and culture".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77683



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

[PATCH] D77498: [Hexagon] Select lld as the default linker for linux-musl target

[Brian Cain via Phabricator via cfe-commits]

bcain added a comment.

In D77498#1968123 , @sidneym wrote:

> Since this isn't something that can be always known in advance I think the 
> testcase should just be removed.


Why not just check if `CLANG_DEFAULT_LINKER` is empty?

  const char *getDefaultLinker() const override {
  StringRef ClangDefault = CLANG_DEFAULT_LINKER;
  return ClangDefault.isEmpty() ? (getTriple().isMusl() ? "ld.lld" : 
"hexagon-link") : ClangDefault.str();
}


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77498



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

[PATCH] D75726: [ConstExprPreter] Updated constant interpreter documentation

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

rsmith added inline comments.



Comment at: clang/docs/ConstantInterpreter.rst:118
 Memory management in the interpreter relies on 3 data structures: ``Block``
-object which store the data and associated inline metadata, ``Pointer`` objects
-which refer to or into blocks, and ``Descriptor`` structures which describe
-blocks and subobjects nested inside blocks.
+object which store the data and associated inline metadata, ``Pointer`` 
+objects which refer to or into blocks, and ``Descriptor`` structures which 

object -> objects



Comment at: clang/docs/ConstantInterpreter.rst:167
 
-Descriptor are generated at bytecode compilation time and contain information 
required to determine if a particular memory access is allowed in constexpr. 
Even though there is a single descriptor object, it encodes information for 
several kinds of objects:
+Descriptor are generated at bytecode compilation time and contain information 
+required to determine if a particular memory access is allowed in constexpr. 

Descriptor -> Descriptors



Comment at: clang/docs/ConstantInterpreter.rst:169-170
+required to determine if a particular memory access is allowed in constexpr. 
+Even though there is a single descriptor object, it encodes information for 
+several kinds of objects:
 

This sounds like this is talking about the memory layout of descriptors, but I 
think the text below is actually talking about the memory layout of blocks?



Comment at: clang/docs/ConstantInterpreter.rst:174
 
   A block containing a primitive reserved storage only for the primitive.
 

reserved -> reserves

How do you distinguish between initialized and in-lifetime-but-uninitialized 
primitives? Eg:

```
constexpr int f() {
  int a;
  return a; // error, a is in-lifetime but uninitialized
}
```



Comment at: clang/docs/ConstantInterpreter.rst:174
 
   A block containing a primitive reserved storage only for the primitive.
 

rsmith wrote:
> reserved -> reserves
> 
> How do you distinguish between initialized and in-lifetime-but-uninitialized 
> primitives? Eg:
> 
> ```
> constexpr int f() {
>   int a;
>   return a; // error, a is in-lifetime but uninitialized
> }
> ```
How do you represent the result of casting a pointer to an integer (which we 
permit only when constant-folding, but nonetheless we do permit)?



Comment at: clang/docs/ConstantInterpreter.rst:182
+  initialised, while a value of ``(InitMap*)-1`` indicates that the object was 
+  fully initialised. when all fields are initialised, the map is deallocated 
+  and replaced with that token.

when -> When

The overwhelmingly common case is the set of initialized elements is [0, 1, 
..., K) for some K. Have you considered instead storing this value as a union 
of an `InitMap*` and an integer, using the bottom bit to indicate which case 
we're in? (We don't need to allocate the map at all except in weird cases where 
someone makes a hole in the array through a pseudo-destructor or allocates 
out-of-order with `construct_at` or similar.)

How do you distinguish between in-lifetime-but-uninitialized elements and 
out-of-lifetime elements? For example:

```
using T = int;
constexpr int f(bool b) {
  int arr[5];
  arr[3].~T();
  arr[0] = 1; // ok, uninitialized -> initialized
  if (!b)
arr[3] = 1; // error, arr[3] is not in lifetime
  else
std::construct_at(arr + 3, 0); // ok, not in lifetime -> in lifetime and 
initialized
  return arr[3];
}
```

Maybe we should use two bits per primitive in the `InitMap` case and store both 
"initialized" and "in-lifetime"?



Comment at: clang/docs/ConstantInterpreter.rst:194-195
+Records are laid out identically to arrays of composites: each field and base 
+class is preceded by an inline descriptor. The ``InlineDescriptor`` 
+has the following field:
 

field -> fields

From the description below, it looks like `sizeof(InlineDescriptor)` is 
currently 16. That seems unnecessary: We could easily get this down to 8 bytes 
by bit-packing the offset and the flags. (Restricting ourselves to 2^59 bytes 
for each record seems unproblematic.) I suspect it might even be OK to pack 
this into 4 bytes; that'd still allow us to support objects whose 
representations are up to 128 MiB -- though perhaps that's getting a little too 
close to territory that programs might actually want to enter.



Comment at: clang/docs/ConstantInterpreter.rst:201
+ * **IsInitialized**: flag indicating whether the field or element was 
+ initialized. For non-primitive fields, this is only relevant for base classes.
+ * **IsBase**: flag indicating whether the record is a base class. In that 
case, 

It's not completely clear to me what this would mean for a base class. Is the 
idea that this tracks whether base classes are 

[PATCH] D75936: Add a Pass to X86 that builds a Condensed CFG for Load Value Injection (LVI) Gadgets [4/6]

[Scott Constable via Phabricator via cfe-commits]

sconstab added inline comments.



Comment at: llvm/lib/Target/X86/ImmutableGraph.h:73
+// The end of this Node's edges is the beginning of the next node's edges.
+const Edge *edges_end() const { return (this + 1)->Edges; }
+ArrayRef edges() const {

mattdr wrote:
> sconstab wrote:
> > mattdr wrote:
> > > Seems like you also want to add a comment here that we know we will never 
> > > be asked for `edges_end` for the last stored node -- that is, we know 
> > > that `this + 1` always refers to a valid Node (which is presumably a 
> > > dummy/sentinel)
> > Not sure I agree. I cannot think of a conventional use of this interface 
> > that would perform an operation on the sentinel.
> > ```
> > G->nodes_end().edges_end(); // invalid use of any end iterator
> > SomeNode.edges_end(); // invalid if SomeNode == G->nodes_end()
> > ```
> > That is, the way that we "know" that we will never be asked for 
> > `edges_end()` for the last stored node is that the ask itself would already 
> > violate C++ conventions.
> I believe any operation on the last `Node` in the array will end up accessing 
> the sentinel:
> 
> ```
> Node* LastNode = G->nodes_begin() + (G->nodes_size() - 1);  // valid 
> reference to the last node
> LastNode->edges_end();  // uses `this+1`, accessing the sentinel value in the 
> Nodes array
> ```
> 
`G->nodes_size()` will return the size without the sentinel node, so your 
example should actually operate on the last data node. Right?


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

https://reviews.llvm.org/D75936



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

[PATCH] D77498: [Hexagon] Select lld as the default linker for linux-musl target

[Sid Manning via Phabricator via cfe-commits]

sidneym added a comment.

Since this isn't something that can be always known in advance I think the 
testcase should just be removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77498



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

[PATCH] D77684: [Hexagon] Remove testcases that check for default linker.

[Sid Manning via Phabricator via cfe-commits]

sidneym created this revision.
sidneym added reviewers: bcain, kparzysz, nathanchance, bcahoon.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

These values are not always known since there is a configuration option to set 
the default linker, CLANG_DEFAULT_LINKER.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77684

Files:
  clang/test/Driver/hexagon-toolchain-elf.c
  clang/test/Driver/hexagon-toolchain-linux.c


Index: clang/test/Driver/hexagon-toolchain-linux.c
===
--- clang/test/Driver/hexagon-toolchain-linux.c
+++ clang/test/Driver/hexagon-toolchain-linux.c
@@ -95,7 +95,3 @@
 // RUN:   %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK007 %s
 // CHECK007:  "-internal-isystem" 
"{{.*}}hexagon{{/|}}include{{/|}}c++{{/|}}v1"
-// 
-
-// RUN: %clang -### -target hexagon-unknown-linux-musl %s 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHECK008 %s
-// CHECK008:  ld.lld
Index: clang/test/Driver/hexagon-toolchain-elf.c
===
--- clang/test/Driver/hexagon-toolchain-elf.c
+++ clang/test/Driver/hexagon-toolchain-elf.c
@@ -597,10 +597,3 @@
 // RUN:   %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK084 %s
 // CHECK084:  "-fno-use-init-array"
-// 
-
-// Check default linker for elf
-// 
-
-// RUN: %clang -### -target hexagon-unknown-elf %s 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHECK092 %s
-// CHECK092:  hexagon-link
-// 
-


Index: clang/test/Driver/hexagon-toolchain-linux.c
===
--- clang/test/Driver/hexagon-toolchain-linux.c
+++ clang/test/Driver/hexagon-toolchain-linux.c
@@ -95,7 +95,3 @@
 // RUN:   %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK007 %s
 // CHECK007:  "-internal-isystem" "{{.*}}hexagon{{/|}}include{{/|}}c++{{/|}}v1"
-// -
-// RUN: %clang -### -target hexagon-unknown-linux-musl %s 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHECK008 %s
-// CHECK008:  ld.lld
Index: clang/test/Driver/hexagon-toolchain-elf.c
===
--- clang/test/Driver/hexagon-toolchain-elf.c
+++ clang/test/Driver/hexagon-toolchain-elf.c
@@ -597,10 +597,3 @@
 // RUN:   %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK084 %s
 // CHECK084:  "-fno-use-init-array"
-// -
-// Check default linker for elf
-// -
-// RUN: %clang -### -target hexagon-unknown-elf %s 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHECK092 %s
-// CHECK092:  hexagon-link
-// -
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77498: [Hexagon] Select lld as the default linker for linux-musl target

[Sid Manning via Phabricator via cfe-commits]

sidneym added a comment.

These are new tests how do you get the generic lld driver to work?  When I 
invoke it using just "lld" I need to add -flavor gnu or I get this error:
Invoke ld.lld (Unix), ld64.lld (macOS), lld-link (Windows), wasm-ld 
(WebAssembly) instead


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77498



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

[PATCH] D75936: Add a Pass to X86 that builds a Condensed CFG for Load Value Injection (LVI) Gadgets [4/6]

[Matthew Riley via Phabricator via cfe-commits]

mattdr added a comment.

I'll wait for current comments to be addressed before doing my next round here.


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

https://reviews.llvm.org/D75936



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

[PATCH] D77598: Integral template argument suffix printing

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGenCXX/debug-info-template.cpp:34
 
-// CHECK: ![[TC]] = distinct !DICompositeType(tag: DW_TAG_structure_type, 
name: "TC"
+// CHECK: ![[TC]] = distinct !DICompositeType(tag: DW_TAG_structure_type, 
name: "TC"
 // CHECK-SAME:  templateParams: [[TCARGS:![0-9]*]]

This is not a diagnostic change.

My experiments with an older GCC seem to indicate that it uses lowercase `u` 
and is capable of distinguishing between 64-bit `long` and `long long`. I also 
observe that no suffix appears with a newer GCC.


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

https://reviews.llvm.org/D77598



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

[PATCH] D75936: Add a Pass to X86 that builds a Condensed CFG for Load Value Injection (LVI) Gadgets [4/6]

[Matthew Riley via Phabricator via cfe-commits]

mattdr added a comment.

Some more comments. FWIW, I'm doing rounds of review as I can in some evening 
quiet or during my son's nap. This is a huge change and it's really hard to get 
any part of it into my head at once in a reasonable amount of time.




Comment at: llvm/lib/Target/X86/ImmutableGraph.h:73
+// The end of this Node's edges is the beginning of the next node's edges.
+const Edge *edges_end() const { return (this + 1)->Edges; }
+ArrayRef edges() const {

sconstab wrote:
> mattdr wrote:
> > Seems like you also want to add a comment here that we know we will never 
> > be asked for `edges_end` for the last stored node -- that is, we know that 
> > `this + 1` always refers to a valid Node (which is presumably a 
> > dummy/sentinel)
> Not sure I agree. I cannot think of a conventional use of this interface that 
> would perform an operation on the sentinel.
> ```
> G->nodes_end().edges_end(); // invalid use of any end iterator
> SomeNode.edges_end(); // invalid if SomeNode == G->nodes_end()
> ```
> That is, the way that we "know" that we will never be asked for `edges_end()` 
> for the last stored node is that the ask itself would already violate C++ 
> conventions.
I believe any operation on the last `Node` in the array will end up accessing 
the sentinel:

```
Node* LastNode = G->nodes_begin() + (G->nodes_size() - 1);  // valid reference 
to the last node
LastNode->edges_end();  // uses `this+1`, accessing the sentinel value in the 
Nodes array
```




Comment at: llvm/lib/Target/X86/ImmutableGraph.h:79
+
+protected:
+  ImmutableGraph(std::unique_ptr Nodes, std::unique_ptr Edges,

sconstab wrote:
> mattdr wrote:
> > Why "protected" rather than "private"? Usually seeing "protected" makes me 
> > think subclassing is expected, but that doesn't seem to be the case here.
> The `MachineGadgetGraph` class actually does subclass `ImmutableGraph` to add 
> some contextual information. I did not want the constructors for 
> `ImmutableGraph` to be public, because the intent is to use the builder. So 
> `protected` seemed like the best option.
Ah, I missed that. I searched through the file for `public ImmutableGraph` and 
didn't find it because `MachineGadgetGraph` uses the default inheritance 
specifier.



Comment at: llvm/lib/Target/X86/X86LoadValueInjectionLoadHardening.cpp:83-85
+#define ARG_NODE nullptr
+#define GADGET_EDGE ((int)(-1))
+#define WEIGHT(EdgeValue) ((double)(2 * (EdgeValue) + 1))

Please replace these with constants or functions.



Comment at: llvm/lib/Target/X86/X86LoadValueInjectionLoadHardening.cpp:113
+  NumFences(NumFences), NumGadgets(NumGadgets) {}
+MachineFunction () { // FIXME: This function should be cleaner
+  for (const Node  : nodes())

sconstab wrote:
> mattdr wrote:
> > Cleaner how?
> Maybe by keeping a member reference to the associated `MachineFunction`?
Let's put that in the comment instead.


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

https://reviews.llvm.org/D75936



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

[PATCH] D77081: [MS] Mark vbase dtors ref'd when ref'ing dtor

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

ptal


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77081



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

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

[Andrew via Phabricator via cfe-commits]

browneee updated this revision to Diff 255831.
browneee added a comment.

Fix more build errors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77621

Files:
  clang/include/clang/Serialization/ASTWriter.h
  clang/include/clang/Serialization/PCHContainerOperations.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/PrecompiledPreamble.cpp
  clang/lib/Frontend/SerializedDiagnosticPrinter.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/GlobalModuleIndex.cpp
  llvm/include/llvm/Bitcode/BitcodeWriter.h
  llvm/include/llvm/Bitstream/BitstreamWriter.h
  llvm/include/llvm/Remarks/BitstreamRemarkSerializer.h
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/ExecutionEngine/Orc/ThreadSafeModule.cpp
  llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
  llvm/tools/llvm-cat/llvm-cat.cpp
  llvm/tools/llvm-modextract/llvm-modextract.cpp
  llvm/unittests/Bitstream/BitstreamReaderTest.cpp
  llvm/unittests/Bitstream/BitstreamWriterTest.cpp

Index: llvm/unittests/Bitstream/BitstreamWriterTest.cpp
===
--- llvm/unittests/Bitstream/BitstreamWriterTest.cpp
+++ llvm/unittests/Bitstream/BitstreamWriterTest.cpp
@@ -8,27 +8,31 @@
 
 #include "llvm/Bitstream/BitstreamWriter.h"
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SmallString.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
+#include 
+
 using namespace llvm;
 
+using ::testing::IsEmpty;
+
 namespace {
 
 TEST(BitstreamWriterTest, emitBlob) {
-  SmallString<64> Buffer;
+  std::vector Buffer;
   BitstreamWriter W(Buffer);
   W.emitBlob("str", /* ShouldEmitSize */ false);
-  EXPECT_EQ(StringRef("str\0", 4), Buffer);
+  EXPECT_EQ(StringRef("str\0", 4), StringRef(Buffer.data(), Buffer.size()));
 }
 
 TEST(BitstreamWriterTest, emitBlobWithSize) {
-  SmallString<64> Buffer;
+  std::vector Buffer;
   {
 BitstreamWriter W(Buffer);
 W.emitBlob("str");
   }
-  SmallString<64> Expected;
+  std::vector Expected;
   {
 BitstreamWriter W(Expected);
 W.EmitVBR(3, 6);
@@ -38,21 +42,21 @@
 W.Emit('r', 8);
 W.Emit(0, 8);
   }
-  EXPECT_EQ(StringRef(Expected), Buffer);
+  EXPECT_EQ(Expected, Buffer);
 }
 
 TEST(BitstreamWriterTest, emitBlobEmpty) {
-  SmallString<64> Buffer;
+  std::vector Buffer;
   BitstreamWriter W(Buffer);
   W.emitBlob("", /* ShouldEmitSize */ false);
-  EXPECT_EQ(StringRef(""), Buffer);
+  EXPECT_THAT(Buffer, IsEmpty());
 }
 
 TEST(BitstreamWriterTest, emitBlob4ByteAligned) {
-  SmallString<64> Buffer;
+  std::vector Buffer;
   BitstreamWriter W(Buffer);
   W.emitBlob("str0", /* ShouldEmitSize */ false);
-  EXPECT_EQ(StringRef("str0"), Buffer);
+  EXPECT_EQ(StringRef("str0"), StringRef(Buffer.data(), Buffer.size()));
 }
 
 } // end namespace
Index: llvm/unittests/Bitstream/BitstreamReaderTest.cpp
===
--- llvm/unittests/Bitstream/BitstreamReaderTest.cpp
+++ llvm/unittests/Bitstream/BitstreamReaderTest.cpp
@@ -11,6 +11,8 @@
 #include "llvm/Bitstream/BitstreamWriter.h"
 #include "gtest/gtest.h"
 
+#include 
+
 using namespace llvm;
 
 namespace {
@@ -96,7 +98,7 @@
 StringRef BlobIn((const char *)BlobData.begin(), BlobSize);
 
 // Write the bitcode.
-SmallVector Buffer;
+std::vector Buffer;
 unsigned AbbrevID;
 {
   BitstreamWriter Stream(Buffer);
@@ -115,7 +117,7 @@
 
 // Stream the buffer into the reader.
 BitstreamCursor Stream(
-ArrayRef((const uint8_t *)Buffer.begin(), Buffer.size()));
+ArrayRef((const uint8_t *)Buffer.data(), Buffer.size()));
 
 // Header.  Included in test so that we can run llvm-bcanalyzer to debug
 // when there are problems.
Index: llvm/tools/llvm-modextract/llvm-modextract.cpp
===
--- llvm/tools/llvm-modextract/llvm-modextract.cpp
+++ llvm/tools/llvm-modextract/llvm-modextract.cpp
@@ -58,12 +58,12 @@
   ExitOnErr(errorCodeToError(EC));
 
   if (BinaryExtract) {
-SmallVector Result;
+std::vector Result;
 BitcodeWriter Writer(Result);
-Result.append(Ms[ModuleIndex].getBuffer().begin(),
+Result.insert(Result.end(), Ms[ModuleIndex].getBuffer().begin(),
   Ms[ModuleIndex].getBuffer().end());
 Writer.copyStrtab(Ms[ModuleIndex].getStrtab());
-Out->os() << Result;
+Out->os().write(Result.data(), Result.size());
 Out->keep();
 return 0;
   }
Index: llvm/tools/llvm-cat/llvm-cat.cpp
===
--- llvm/tools/llvm-cat/llvm-cat.cpp
+++ llvm/tools/llvm-cat/llvm-cat.cpp
@@ -54,7 +54,7 @@
   ExitOnError ExitOnErr("llvm-cat: ");
   LLVMContext Context;
 
-  SmallVector Buffer;
+  std::vector Buffer;
   BitcodeWriter Writer(Buffer);
   if (BinaryCat) {
 for (const auto  : InputFilenames) {
Index: llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp

[PATCH] D77683: [Docs] Make code review policy clearer about requested pre-commit reviews

[Johannes Doerfert via Phabricator via cfe-commits]

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

Add comma


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77683

Files:
  llvm/docs/CodeReview.rst


Index: llvm/docs/CodeReview.rst
===
--- llvm/docs/CodeReview.rst
+++ llvm/docs/CodeReview.rst
@@ -30,7 +30,9 @@
 (or patches where the developer owns the component) that meet
 likely-community-consensus requirements (as apply to all patch approvals) can
 be committed prior to an explicit review. In situations where there is any
-uncertainty, a patch should be reviewed prior to being committed.
+uncertainty, a patch should be reviewed prior to being committed. If pre-commit
+code reviewes in a particular area have been requested, code should clear a
+significantly higher bar, e.g., fixes, to be commited without review.
 
 Please note that the developer responsible for a patch is also
 responsible for making all necessary review-related changes, including


Index: llvm/docs/CodeReview.rst
===
--- llvm/docs/CodeReview.rst
+++ llvm/docs/CodeReview.rst
@@ -30,7 +30,9 @@
 (or patches where the developer owns the component) that meet
 likely-community-consensus requirements (as apply to all patch approvals) can
 be committed prior to an explicit review. In situations where there is any
-uncertainty, a patch should be reviewed prior to being committed.
+uncertainty, a patch should be reviewed prior to being committed. If pre-commit
+code reviewes in a particular area have been requested, code should clear a
+significantly higher bar, e.g., fixes, to be commited without review.
 
 Please note that the developer responsible for a patch is also
 responsible for making all necessary review-related changes, including
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77683: [Docs] Make code review policy clearer about requested pre-commit reviews

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert created this revision.
jdoerfert added reviewers: hfinkel, ABataev, dblaikie, mehdi_amini, 
aaron.ballman, modocache, lattner, MaskRay, hubert.reinterpretcast, jhenderson, 
rengolin, kristina.
Herald added a subscriber: bollu.
Herald added a project: LLVM.
jdoerfert updated this revision to Diff 255830.
jdoerfert added a comment.

Add comma


The current wording already urges for pre-commit code reviews in case of
uncertainty. In addition, we now make it clear that the bar to assume
likely-community-consensus, and to do unreviewed commits in general,
is significantly higher if pre-commit reviews were requested in a
certain are.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77683

Files:
  llvm/docs/CodeReview.rst


Index: llvm/docs/CodeReview.rst
===
--- llvm/docs/CodeReview.rst
+++ llvm/docs/CodeReview.rst
@@ -30,7 +30,9 @@
 (or patches where the developer owns the component) that meet
 likely-community-consensus requirements (as apply to all patch approvals) can
 be committed prior to an explicit review. In situations where there is any
-uncertainty, a patch should be reviewed prior to being committed.
+uncertainty, a patch should be reviewed prior to being committed. If pre-commit
+code reviewes in a particular area have been requested, code should clear a
+significantly higher bar, e.g., fixes, to be commited without review.
 
 Please note that the developer responsible for a patch is also
 responsible for making all necessary review-related changes, including


Index: llvm/docs/CodeReview.rst
===
--- llvm/docs/CodeReview.rst
+++ llvm/docs/CodeReview.rst
@@ -30,7 +30,9 @@
 (or patches where the developer owns the component) that meet
 likely-community-consensus requirements (as apply to all patch approvals) can
 be committed prior to an explicit review. In situations where there is any
-uncertainty, a patch should be reviewed prior to being committed.
+uncertainty, a patch should be reviewed prior to being committed. If pre-commit
+code reviewes in a particular area have been requested, code should clear a
+significantly higher bar, e.g., fixes, to be commited without review.
 
 Please note that the developer responsible for a patch is also
 responsible for making all necessary review-related changes, including
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77085: [clang-tidy] Added support for validating configuration options

[Nathan James via Phabricator via cfe-commits]

njames93 added a subscriber: jdoerfert.
njames93 added a comment.

In D77085#1967883 , @nemanjai wrote:

> Awesome, thanks. Certainly fixes the compile time failures in my local build. 
> There is still a link-time failure (undefined reference) with my shared 
> libraries build:
>
>   
> tools/clang/tools/extra/clang-tidy/tool/CMakeFiles/obj.clangTidyMain.dir/ClangTidyMain.cpp.o:
>  In function 
> `clang::ast_matchers::internal::matcher_isAllowedToContainClauseKind0Matcher::matches(clang::OMPExecutableDirective
>  const&, clang::ast_matchers::internal::ASTMatchFinder*, 
> clang::ast_matchers::internal::BoundNodesTreeBuilder*) const':
>   
> ClangTidyMain.cpp:(.text._ZNK5clang12ast_matchers8internal44matcher_isAllowedToContainClauseKind0Matcher7matchesERKNS_22OMPExecutableDirectiveEPNS1_14ASTMatchFinderEPNS1_21BoundNodesTreeBuilderE[_ZNK5clang12ast_matchers8internal44matcher_isAllowedToContainClauseKind0Matcher7matchesERKNS_22OMPExecutableDirectiveEPNS1_14ASTMatchFinderEPNS1_21BoundNodesTreeBuilderE]+0x50):
>  undefined reference to 
> `llvm::omp::isAllowedClauseForDirective(llvm::omp::Directive, 
> llvm::omp::Clause, unsigned int)'
>   collect2: error: ld returned 1 exit status
>
>
> But that may be unrelated to this patch.


That's unrelated to this patch, something about ASTMatchers and specifically 
OpenMP @jdoerfert has changed a few things there, may be worth hitting him up 
to see.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77085



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

[PATCH] D77498: [Hexagon] Select lld as the default linker for linux-musl target

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

> Weird -- the baseline didn't take CLANG_DEFAULT_LINKER into account either, 
> though, right?

I assume by baseline you mean the test before this change? It does not seem 
like it.

> Is it a regression?

I guess not but the test is still broken when `CLANG_DEFAULT_LINKER` is set.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77498



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

[PATCH] D77682: [clang-format] Always break line after enum opening brace

[Omar Sandoval via Phabricator via cfe-commits]

osandov created this revision.
osandov added reviewers: MyDeveloperDay, krasimir.
osandov added projects: clang-format, clang.
Herald added a subscriber: cfe-commits.

clang-format currently puts the first enumerator on the same line as the
enum keyword and opening brace if it fits (for example, for anonymous
enums if IndentWidth is 8):

  $ echo "enum { A, };" | clang-format -style="{BasedOnStyle: llvm, 
IndentWidth: 8}"
  enum { A,
  };

This doesn't seem to be intentional, as I can't find any style guide that
suggests wrapping enums this way. Always force the enumerator to be on a new
line, which gets us the desired result:

  $ echo "enum { A, };" | ./bin/clang-format -style="{BasedOnStyle: llvm, 
IndentWidth: 8}"
  enum {
  A,
  };

Test Plan:

New test added. Confirmed test failed without change and passed with change by
running:

  $ ninja FormatTests && ./tools/clang/unittests/Format/FormatTests


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77682

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1929,6 +1929,14 @@
"  TWO\n"
"};\n"
"int i;");
+
+  FormatStyle EightIndent = getLLVMStyle();
+  EightIndent.IndentWidth = 8;
+  verifyFormat("enum {\n"
+   "A,\n"
+   "};",
+   EightIndent);
+
   // Not enums.
   verifyFormat("enum X f() {\n"
"  a();\n"
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -423,7 +423,7 @@
   State.Stack.back().BreakBeforeParameter && Current.CanBreakBefore)
 return true;
 
-  if (State.Column <= NewLineColumn)
+  if (!State.Line->First->is(tok::kw_enum) && State.Column <= NewLineColumn)
 return false;
 
   if (Style.AlwaysBreakBeforeMultilineStrings &&


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1929,6 +1929,14 @@
"  TWO\n"
"};\n"
"int i;");
+
+  FormatStyle EightIndent = getLLVMStyle();
+  EightIndent.IndentWidth = 8;
+  verifyFormat("enum {\n"
+   "A,\n"
+   "};",
+   EightIndent);
+
   // Not enums.
   verifyFormat("enum X f() {\n"
"  a();\n"
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -423,7 +423,7 @@
   State.Stack.back().BreakBeforeParameter && Current.CanBreakBefore)
 return true;
 
-  if (State.Column <= NewLineColumn)
+  if (!State.Line->First->is(tok::kw_enum) && State.Column <= NewLineColumn)
 return false;
 
   if (Style.AlwaysBreakBeforeMultilineStrings &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77611: [Sema] Check calls to __attribute__((warn_unused_result)) from StmtExprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 255823.
nickdesaulniers added a comment.

- tighten up condition, eagerly emit diag, test with kernel before pushing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77611

Files:
  clang/lib/Sema/SemaStmt.cpp
  clang/test/Frontend/macros.c


Index: clang/test/Frontend/macros.c
===
--- clang/test/Frontend/macros.c
+++ clang/test/Frontend/macros.c
@@ -1,5 +1,4 @@
-// RUN: %clang_cc1 -DA= -DB=1 -verify -fsyntax-only %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -Wunused-result -DA= -DB=1 -verify -fsyntax-only %s
 
 int a[(B A) == 1 ? 1 : -1];
 
@@ -11,3 +10,15 @@
 void foo(int a, int b, int c) {
   memset(a,b,c);  // No warning!
 }
+
+__attribute__((warn_unused_result)) static inline int bar(void) {
+  return 42;
+}
+
+#define baz() ({ \
+  bar(); \
+})
+
+void quux(void) {
+  baz(); // expected-warning {{ignoring return value of function declared with 
'warn_unused_result' attribute}}
+}
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -247,9 +247,18 @@
   // If this is a GNU statement expression expanded from a macro, it is 
probably
   // unused because it is a function-like macro that can be used as either an
   // expression or statement.  Don't warn, because it is almost certainly a
-  // false positive.
-  if (isa(E) && Loc.isMacroID())
+  // false positive. But we do want to check CallExprs with
+  // WarnUnusedResultAttr.
+  if (isa(E) && Loc.isMacroID()) {
+if (const CallExpr *CE = dyn_cast(WarnExpr))
+  if (const Decl *FD = CE->getCalleeDecl())
+if (FD->hasAttr())
+  DiagnoseNoDiscard(*this,
+cast_or_null(
+CE->getUnusedResultAttr(Context)),
+Loc, R1, R2, /*isCtor=*/false);
 return;
+  }
 
   // Check if this is the UNREFERENCED_PARAMETER from the Microsoft headers.
   // That macro is frequently used to suppress "unused parameter" warnings,


Index: clang/test/Frontend/macros.c
===
--- clang/test/Frontend/macros.c
+++ clang/test/Frontend/macros.c
@@ -1,5 +1,4 @@
-// RUN: %clang_cc1 -DA= -DB=1 -verify -fsyntax-only %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -Wunused-result -DA= -DB=1 -verify -fsyntax-only %s
 
 int a[(B A) == 1 ? 1 : -1];
 
@@ -11,3 +10,15 @@
 void foo(int a, int b, int c) {
   memset(a,b,c);  // No warning!
 }
+
+__attribute__((warn_unused_result)) static inline int bar(void) {
+  return 42;
+}
+
+#define baz() ({ \
+  bar(); \
+})
+
+void quux(void) {
+  baz(); // expected-warning {{ignoring return value of function declared with 'warn_unused_result' attribute}}
+}
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -247,9 +247,18 @@
   // If this is a GNU statement expression expanded from a macro, it is probably
   // unused because it is a function-like macro that can be used as either an
   // expression or statement.  Don't warn, because it is almost certainly a
-  // false positive.
-  if (isa(E) && Loc.isMacroID())
+  // false positive. But we do want to check CallExprs with
+  // WarnUnusedResultAttr.
+  if (isa(E) && Loc.isMacroID()) {
+if (const CallExpr *CE = dyn_cast(WarnExpr))
+  if (const Decl *FD = CE->getCalleeDecl())
+if (FD->hasAttr())
+  DiagnoseNoDiscard(*this,
+cast_or_null(
+CE->getUnusedResultAttr(Context)),
+Loc, R1, R2, /*isCtor=*/false);
 return;
+  }
 
   // Check if this is the UNREFERENCED_PARAMETER from the Microsoft headers.
   // That macro is frequently used to suppress "unused parameter" warnings,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libunwind] 82576d6 - [libunwind] Fix UB in EHHeaderParser::findFDE

[Jorge Gorbe Moya via cfe-commits]

Author: Jorge Gorbe Moya
Date: 2020-04-07T14:44:42-07:00
New Revision: 82576d6fecfec71725eb900111c000d772002449

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

LOG: [libunwind] Fix UB in EHHeaderParser::findFDE

When the EHHeaderInfo object filled by decodeEHHdr has fde_count == 0,
findFDE does the following:

- sets low = 0 and len = hdrInfo.fde_count as a preparation to start a
  binary search
- because len is 0, the binary search loop is skipped
- the code still tries to find a table entry at
  hdrInfo.table + low * tableEntrySize, and decode it.

This is wrong when fde_count is 0, and trying to decode a table entry
that isn't there will lead to reading garbage offsets and can cause
segfaults.

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

Added: 


Modified: 
libunwind/src/EHHeaderParser.hpp

Removed: 




diff  --git a/libunwind/src/EHHeaderParser.hpp 
b/libunwind/src/EHHeaderParser.hpp
index 0101835b8e63..f97cca54825f 100644
--- a/libunwind/src/EHHeaderParser.hpp
+++ b/libunwind/src/EHHeaderParser.hpp
@@ -109,6 +109,8 @@ bool EHHeaderParser::findFDE(A , pint_t pc, 
pint_t ehHdrStart,
   hdrInfo))
 return false;
 
+  if (hdrInfo.fde_count == 0) return false;
+
   size_t tableEntrySize = getTableEntrySize(hdrInfo.table_enc);
   pint_t tableEntry;
 



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

[PATCH] D77680: [clang-tidy] misc-unused-parameters: Don't remove parameter from lambda

[Matthias Gehre via Phabricator via cfe-commits]

mgehre created this revision.
mgehre added reviewers: aaron.ballman, alexfh, hokein, njames93.
Herald added a subscriber: xazax.hun.
Herald added a project: clang.
mgehre added a project: clang-tools-extra.
mgehre edited the summary of this revision.

Previously, the check would fix

  using fn = void(int);
  void f(fn *);
  void test() {
// CHECK-MESSAGES: :[[@LINE+2]]:12: warning: parameter 'I' is unused
// CHECK-FIXES: {{^}}  f([](int  /*I*/) {
f([](int I) { return; });
  }

into
`f([]() { return; });` which breaks compilation. Now the check is disabled for 
lambdas.

The AST is not so easy to use. For

  auto l = [](int) {  return;  };
  f(l);

one gets

  `-CallExpr  'void'
   |-ImplicitCastExpr  'void (*)(fn *)' 
   | `-DeclRefExpr  'void (fn *)' lvalue Function 0x55a91a545e28 'f' 
'void (fn *)'
   `-ImplicitCastExpr  'void (*)(int)' 
 `-CXXMemberCallExpr  'void (*)(int)'
   `-MemberExpr  '' .operator void 
(*)(int) 0x55a91a546850
 `-ImplicitCastExpr  'const (lambda at line:6:14)' lvalue 

   `-DeclRefExpr  '(lambda at line:6:14)':'(lambda at 
line:6:14)' lvalue Var 0x55a91a5461c0 'l' '(lambda at line:6:14)':'(lambda at 
line:6:14)'

There is no direct use of the `operator()(int I)` of the lambda, so the 
`!Indexer->getOtherRefs(Function).empty()`
does not fire. In the future, we might be able to use the conversion operator 
`operator void (*)(int)` to mark
the call operator as having an "other ref".


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77680

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


Index: clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
@@ -276,3 +276,13 @@
 // CHECK-FIXES: {{^}}  B(int  /*i*/) : A() {}{{$}}
 };
 } // namespace strict_mode_off
+
+namespace lambda {
+using fn = void(int);
+void f(fn *);
+void test() {
+  // CHECK-MESSAGES: :[[@LINE+2]]:12: warning: parameter 'I' is unused
+  // CHECK-FIXES: {{^}}  f([](int  /*I*/) {
+  f([](int I) { return; });
+}
+} // namespace lambda
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
@@ -8,6 +8,7 @@
 
 #include "UnusedParametersCheck.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTLambda.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
@@ -141,7 +142,8 @@
   // Cannot remove parameter for non-local functions.
   if (Function->isExternallyVisible() ||
   !Result.SourceManager->isInMainFile(Function->getLocation()) ||
-  !Indexer->getOtherRefs(Function).empty() || isOverrideMethod(Function)) {
+  !Indexer->getOtherRefs(Function).empty() || isOverrideMethod(Function) ||
+  isLambdaCallOperator(Function)) {
 
 // It is illegal to omit parameter name here in C code, so early-out.
 if (!Result.Context->getLangOpts().CPlusPlus)


Index: clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
@@ -276,3 +276,13 @@
 // CHECK-FIXES: {{^}}  B(int  /*i*/) : A() {}{{$}}
 };
 } // namespace strict_mode_off
+
+namespace lambda {
+using fn = void(int);
+void f(fn *);
+void test() {
+  // CHECK-MESSAGES: :[[@LINE+2]]:12: warning: parameter 'I' is unused
+  // CHECK-FIXES: {{^}}  f([](int  /*I*/) {
+  f([](int I) { return; });
+}
+} // namespace lambda
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
@@ -8,6 +8,7 @@
 
 #include "UnusedParametersCheck.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTLambda.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
@@ -141,7 +142,8 @@
   // Cannot remove parameter for non-local functions.
   if (Function->isExternallyVisible() ||
   !Result.SourceManager->isInMainFile(Function->getLocation()) ||
-  !Indexer->getOtherRefs(Function).empty() || isOverrideMethod(Function)) {
+  !Indexer->getOtherRefs(Function).empty() || isOverrideMethod(Function) ||
+  isLambdaCallOperator(Function)) {
 
 // It is illegal to omit parameter name here in C code, 

[PATCH] D77670: [CUDA] Add partial support for recent CUDA versions.

[Yaxun Liu via Phabricator via cfe-commits]

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

LGTM. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77670



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

[PATCH] D76768: [analyzer] Added support of scan-build and exploded-graph-rewriter regression tests for Windows

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

In D76768#1967129 , @Charusso wrote:

> I believe it is very strange on a Windows system to have multiple dots in a 
> file. The other issue could be the wildcard `/*/` in a path full of `\`s. The 
> LLVM `lit` (https://llvm.org/docs/CommandGuide/lit.html) has tons of 
> Windows-related shortcuts, which I have never seen being used, but could be 
> useful.


I've checked. There is no problem with dots and /*/. My command promt (Win10) 
handles them perfectly well. Mixing / and \ in paths is also acceptable and 
handles correctly.
For instance, Visual Studio creates file with multiple dots for projects as 
"Project1.vcxproj.filters". Also you can easely create the same directory.
Wildcard /*/ is a part of input syntax of unix utility **cat**, thus it works 
as well.

Is there any way to get to buildbot file system to compare test files with what 
is in the master branch? Particularly 
//C:\src\llvm-project\clang\test\Analysis\scan-build/Inputs/single_null_dereference.c//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76768



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

[PATCH] D77498: [Hexagon] Select lld as the default linker for linux-musl target

[Brian Cain via Phabricator via cfe-commits]

bcain added a comment.

In D77498#1967870 , @nathanchance 
wrote:

> This does not take into account `CLANG_DEFAULT_LINKER`, resulting in a 
> `check-clang` failure:


Weird -- the baseline didn't take CLANG_DEFAULT_LINKER into account either, 
though, right?  Is it a regression?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77498



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

[PATCH] D77665: [CUDA] Simplify GPU variant handling. NFC.

[Yaxun Liu via Phabricator via cfe-commits]

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

LGTM. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77665



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

[PATCH] D77611: [Sema] fix -Wunused-result in StmtExpr

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:252
+  // CallExprs below.
+  if (isa(E) && Loc.isMacroID() && !isa(WarnExpr))
 return;

sigh, this again produces different from GCC for the kernel.  Let me tighten 
this constraint up further.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77611



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

[PATCH] D77572: [clang-tidy] add new check readability-use-anyofallof

[Matthias Gehre via Phabricator via cfe-commits]

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

Review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77572

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp
  clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-use-anyofallof.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-use-anyofallof-cpp20.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-use-anyofallof.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-use-anyofallof.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-use-anyofallof.cpp
@@ -0,0 +1,158 @@
+// RUN: %check_clang_tidy -std=c++14,c++17 %s readability-use-anyofallof %t
+
+bool good_any_of() {
+  int v[] = {1, 2, 3};
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: Replace loop by std::any_of() from  [readability-use-anyofallof]
+  for (int i : v)
+if (i)
+  return true;
+  return false;
+}
+
+bool cond(int i);
+
+bool good_any_of2() {
+  int v[] = {1, 2, 3};
+  for (int i : v) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Replace loop by std::any_of()
+int k = i / 2;
+if (cond(k))
+  return true;
+  }
+  return false;
+}
+
+bool good_any_of3() {
+  int v[] = {1, 2, 3};
+  for (int i : v) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Replace loop by std::any_of()
+if (i == 3)
+  continue;
+if (i)
+  return true;
+  }
+
+  return false;
+}
+
+bool good_any_of_use_external(int comp) {
+  int v[] = {1, 2, 3};
+  for (int i : v) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Replace loop by std::any_of()
+if (i == comp)
+  return true;
+  }
+
+  return false;
+}
+
+bool good_any_of_no_cond() {
+  int v[] = {1, 2, 3};
+  for (int i : v) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Replace loop by std::any_of()
+return true; // Not a real loop, but technically can become any_of.
+  }
+
+  return false;
+}
+
+bool good_any_of_local_modification() {
+  int v[] = {1, 2, 3};
+  for (int i : v) {
+int j = i;
+j++; // FIXME: Any non-const use disables check.
+if (j > 3)
+  return true;
+  }
+
+  return false;
+}
+
+bool bad_any_of1() {
+  int v[] = {1, 2, 3};
+  for (int i : v) {
+if (i)
+  return false; // bad constant
+  }
+  return false;
+}
+
+bool bad_any_of2() {
+  int v[] = {1, 2, 3};
+  for (int i : v)
+if (i)
+  return true;
+
+  return true; // bad return
+}
+
+bool bad_any_of3() {
+  int v[] = {1, 2, 3};
+  for (int i : v)
+if (i)
+  return true;
+else
+  return i / 2; // bad return
+
+  return false;
+}
+
+bool bad_any_of_control_flow1() {
+  int v[] = {1, 2, 3};
+  for (int i : v) {
+break; // bad control flow
+if (i)
+  return true;
+  }
+
+  return false;
+}
+
+bool bad_any_of4() {
+  return false; // wrong order
+
+  int v[] = {1, 2, 3};
+  for (int i : v) {
+if (i)
+  return true;
+  }
+}
+
+bool bad_any_of5() {
+  int v[] = {1, 2, 3};
+  int j = 0;
+  for (int i : v) {
+j++; // modifications
+if (i)
+  return true;
+  }
+  return false;
+}
+
+bool bad_any_of6() {
+  int v[] = {1, 2, 3};
+  for (int i : v) {
+if (i)
+  return true;
+  }
+  int j = 0; // Statements between loop and return
+  j++;
+  return false;
+}
+
+bool bad_any_of7() {
+  int v[] = {1, 2, 3};
+  for (int i : v) {
+i; // No 'return true' in body.
+  }
+  return false;
+}
+
+bool good_all_of() {
+  int v[] = {1, 2, 3};
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: Replace loop by std::all_of() from  [readability-use-anyofallof]
+  for (int i : v)
+if (i)
+  return false;
+  return true;
+}
Index: clang-tools-extra/test/clang-tidy/checkers/readability-use-anyofallof-cpp20.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-use-anyofallof-cpp20.cpp
@@ -0,0 +1,19 @@
+// RUN: %check_clang_tidy -std=c++2a-or-later %s readability-use-anyofallof %t
+
+bool good_any_of() {
+  int v[] = {1, 2, 3};
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: Replace loop by std::ranges::any_of() from 
+  for (int i : v)
+if (i)
+  return true;
+  return false;
+}
+
+bool good_all_of() {
+  int v[] = {1, 2, 3};
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: Replace loop by std::ranges::all_of() from 
+  for (int i : v)
+if (i)
+  return false;
+  return true;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-use-anyofallof.rst
===
--- /dev/null

[PATCH] D77572: [clang-tidy] add new check readability-use-anyofallof

[Matthias Gehre via Phabricator via cfe-commits]

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

Review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77572

Files:
  clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp
  clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst

Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -132,8 +132,8 @@
 - New :doc:`readability-use-anyofallof
   ` check.
 
-  Finds range-based for loops that can be replaced by a call to std::any_of or
-  std::all_of.
+  Finds range-based for loops that can be replaced by a call to ``std::any_of``
+  or ``std::all_of``.
 
 New check aliases
 ^
Index: clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.h
===
--- clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.h
+++ clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.h
@@ -24,19 +24,14 @@
 /// http://clang.llvm.org/extra/clang-tidy/checks/readability-use-anyofallof.html
 class UseAnyOfAllOfCheck : public ClangTidyCheck {
 public:
-  UseAnyOfAllOfCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context),
-IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
-Options.getLocalOrGlobal("IncludeStyle", "llvm"))) {}
+  using ClangTidyCheck::ClangTidyCheck;
+
+  bool isLanguageVersionSupported(const LangOptions ) const override {
+return LangOpts.CPlusPlus;
+  }
 
-  void registerPPCallbacks(const SourceManager , Preprocessor *PP,
-   Preprocessor *ModuleExpanderPP) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
-
-private:
-  std::unique_ptr IncludeInserter;
-  const utils::IncludeSorter::IncludeStyle IncludeStyle;
 };
 
 } // namespace readability
Index: clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp
@@ -18,12 +18,13 @@
 using namespace clang::ast_matchers;
 
 namespace clang {
-namespace ast_matchers {
+namespace {
 /// Matches a Stmt whose parent is a CompoundStmt,
 /// and which is directly followed by
 /// a Stmt matching the inner matcher.
-AST_MATCHER_P(Stmt, nextStmt, internal::Matcher, InnerMatcher) {
-  const auto  = Finder->getASTContext().getParents(Node);
+AST_MATCHER_P(Stmt, nextStmt, ast_matchers::internal::Matcher,
+  InnerMatcher) {
+  DynTypedNodeList Parents = Finder->getASTContext().getParents(Node);
   if (Parents.size() != 1)
 return false;
 
@@ -31,30 +32,19 @@
   if (!C)
 return false;
 
-  const auto *I = std::find(C->body_begin(), C->body_end(), );
+  const auto *I = llvm::find(C->body(), );
   assert(I != C->body_end()); // C is parent of Node.
   if (++I == C->body_end())
 return false; // Node is last statement.
 
   return InnerMatcher.matches(**I, Finder, Builder);
 }
-} // namespace ast_matchers
+} // namespace
 
 namespace tidy {
 namespace readability {
 
-void UseAnyOfAllOfCheck::registerPPCallbacks(const SourceManager ,
- Preprocessor *PP,
- Preprocessor *ModuleExpanderPP) {
-  IncludeInserter =
-  std::make_unique(SM, getLangOpts(), IncludeStyle);
-  PP->addPPCallbacks(IncludeInserter->CreatePPCallbacks());
-}
-
 void UseAnyOfAllOfCheck::registerMatchers(MatchFinder *Finder) {
-  if (!getLangOpts().CPlusPlus)
-return;
-
   auto returns = [](bool V) {
 return returnStmt(hasReturnValue(cxxBoolLiteral(equals(V;
   };
@@ -107,7 +97,6 @@
 
 diag(S->getForLoc(), "Replace loop by std%0::any_of() from ")
 << Ranges;
-
   } else if (const auto *S =
  Result.Nodes.getNodeAs("all_of_loop")) {
 if (!isViableLoop(*S, *Result.Context))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77598: Integral template argument suffix printing

[Aristotelis Koutsouridis via Phabricator via cfe-commits]

arisKoutsou updated this revision to Diff 255810.

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

https://reviews.llvm.org/D77598

Files:
  clang/lib/AST/TemplateBase.cpp
  clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
  clang/test/CXX/lex/lex.literal/lex.ext/p12.cpp
  clang/test/CodeGenCXX/debug-info-template.cpp
  clang/test/Index/print-type.cpp
  clang/test/Misc/integer-literal-printing.cpp
  clang/test/Modules/lsv-debuginfo.cpp
  clang/test/SemaCXX/builtin-align-cxx.cpp
  clang/test/SemaCXX/cxx11-ast-print.cpp
  clang/test/SemaCXX/cxx11-call-to-deleted-constructor.cpp
  clang/test/SemaCXX/invalid-instantiated-field-decl.cpp
  clang/test/SemaCXX/vector.cpp
  clang/test/SemaTemplate/address_space-dependent.cpp
  clang/test/SemaTemplate/delegating-constructors.cpp
  clang/test/SemaTemplate/dependent-names.cpp
  clang/test/SemaTemplate/temp_arg_nontype.cpp

Index: clang/test/SemaTemplate/temp_arg_nontype.cpp
===
--- clang/test/SemaTemplate/temp_arg_nontype.cpp
+++ clang/test/SemaTemplate/temp_arg_nontype.cpp
@@ -270,6 +270,24 @@
   void test_char_possibly_negative() { enable_if_char<'\x02'>::type i; } // expected-error{{enable_if_char<'\x02'>'; did you mean 'enable_if_char<'a'>::type'?}}
   void test_char_single_quote() { enable_if_char<'\''>::type i; } // expected-error{{enable_if_char<'\''>'; did you mean 'enable_if_char<'a'>::type'?}}
   void test_char_backslash() { enable_if_char<'\\'>::type i; } // expected-error{{enable_if_char<'\\'>'; did you mean 'enable_if_char<'a'>::type'?}}
+  
+  template  struct enable_if_int {};
+  template <> struct enable_if_int<1> { typedef int type; }; // expected-note{{'enable_if_int<1>::type' declared here}}
+  void test_int() { enable_if_int<2>::type i; } // expected-error{{enable_if_int<2>'; did you mean 'enable_if_int<1>::type'?}}
+
+  template  struct enable_if_unsigned_int {};
+  template <> struct enable_if_unsigned_int<1> { typedef int type; }; // expected-note{{'enable_if_unsigned_int<1>::type' declared here}}
+  void test_unsigned_int() { enable_if_unsigned_int<2>::type i; } // expected-error{{enable_if_unsigned_int<2U>'; did you mean 'enable_if_unsigned_int<1>::type'?}}
+
+
+  template  struct enable_if_unsigned_long_long {};
+  template <> struct enable_if_unsigned_long_long<1> { typedef int type; }; // expected-note{{'enable_if_unsigned_long_long<1>::type' declared here}}
+  void test_unsigned_long_long() { enable_if_unsigned_long_long<2>::type i; } // expected-error{{enable_if_unsigned_long_long<2ULL>'; did you mean 'enable_if_unsigned_long_long<1>::type'?}}
+
+  template  struct enable_if_long_long {};
+  template <> struct enable_if_long_long<1> { typedef int type; }; // expected-note{{'enable_if_long_long<1>::type' declared here}}
+  void test_long_long() { enable_if_long_long<2>::type i; } // expected-error{{enable_if_long_long<2LL>'; did you mean 'enable_if_long_long<1>::type'?}}
+
 }
 
 namespace PR10579 {
Index: clang/test/SemaTemplate/dependent-names.cpp
===
--- clang/test/SemaTemplate/dependent-names.cpp
+++ clang/test/SemaTemplate/dependent-names.cpp
@@ -338,7 +338,7 @@
   struct Y: Y { }; // expected-error{{circular inheritance between 'Y' and 'Y'}}
 };
 typedef X<3> X3;
-X3::Y<>::iterator it; // expected-error {{no type named 'iterator' in 'PR11421::X<3>::Y<3>'}}
+X3::Y<>::iterator it; // expected-error {{no type named 'iterator' in 'PR11421::X<3U>::Y<3U>'}}
 }
 
 namespace rdar12629723 {
Index: clang/test/SemaTemplate/delegating-constructors.cpp
===
--- clang/test/SemaTemplate/delegating-constructors.cpp
+++ clang/test/SemaTemplate/delegating-constructors.cpp
@@ -9,7 +9,7 @@
   public:
 template 
 string(const char ()[N])
-  : string(str) {} // expected-error{{constructor for 'string<6>' creates a delegation cycle}}
+  : string(str) {} // expected-error{{constructor for 'string<6U>' creates a delegation cycle}}
   };
 
   void f() {
Index: clang/test/SemaTemplate/address_space-dependent.cpp
===
--- clang/test/SemaTemplate/address_space-dependent.cpp
+++ clang/test/SemaTemplate/address_space-dependent.cpp
@@ -84,8 +84,8 @@
 template  int __attribute__((address_space(B))) *same_template();
 void test_same_template() { (void) same_template<0>(); }
 
-template  int __attribute__((address_space(A))) *different_template(); // expected-note {{candidate function [with A = 0]}}
-template  int __attribute__((address_space(B+1))) *different_template(); // expected-note {{candidate function [with B = 0]}}
+template  int __attribute__((address_space(A))) *different_template(); // expected-note {{candidate function [with A = 0U]}}
+template  int __attribute__((address_space(B+1))) *different_template(); // expected-note {{candidate function [with B = 0U]}}
 void 

[PATCH] D77611: [Sema] fix -Wunused-result in StmtExpr

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 255808.
nickdesaulniers added a comment.

- emit warning just for CallExprs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77611

Files:
  clang/lib/Sema/SemaStmt.cpp
  clang/test/Frontend/macros.c


Index: clang/test/Frontend/macros.c
===
--- clang/test/Frontend/macros.c
+++ clang/test/Frontend/macros.c
@@ -1,5 +1,4 @@
-// RUN: %clang_cc1 -DA= -DB=1 -verify -fsyntax-only %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -Wunused-result -DA= -DB=1 -verify -fsyntax-only %s
 
 int a[(B A) == 1 ? 1 : -1];
 
@@ -11,3 +10,15 @@
 void foo(int a, int b, int c) {
   memset(a,b,c);  // No warning!
 }
+
+__attribute__((warn_unused_result)) static inline int bar(void) {
+  return 42;
+}
+
+#define baz() ({ \
+  bar(); \
+})
+
+void quux(void) {
+  baz(); // expected-warning {{ignoring return value of function declared with 
'warn_unused_result' attribute}}
+}
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -247,8 +247,9 @@
   // If this is a GNU statement expression expanded from a macro, it is 
probably
   // unused because it is a function-like macro that can be used as either an
   // expression or statement.  Don't warn, because it is almost certainly a
-  // false positive.
-  if (isa(E) && Loc.isMacroID())
+  // false positive. But we do want to check for WarnUnusedResultAttr on
+  // CallExprs below.
+  if (isa(E) && Loc.isMacroID() && !isa(WarnExpr))
 return;
 
   // Check if this is the UNREFERENCED_PARAMETER from the Microsoft headers.


Index: clang/test/Frontend/macros.c
===
--- clang/test/Frontend/macros.c
+++ clang/test/Frontend/macros.c
@@ -1,5 +1,4 @@
-// RUN: %clang_cc1 -DA= -DB=1 -verify -fsyntax-only %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -Wunused-result -DA= -DB=1 -verify -fsyntax-only %s
 
 int a[(B A) == 1 ? 1 : -1];
 
@@ -11,3 +10,15 @@
 void foo(int a, int b, int c) {
   memset(a,b,c);  // No warning!
 }
+
+__attribute__((warn_unused_result)) static inline int bar(void) {
+  return 42;
+}
+
+#define baz() ({ \
+  bar(); \
+})
+
+void quux(void) {
+  baz(); // expected-warning {{ignoring return value of function declared with 'warn_unused_result' attribute}}
+}
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -247,8 +247,9 @@
   // If this is a GNU statement expression expanded from a macro, it is probably
   // unused because it is a function-like macro that can be used as either an
   // expression or statement.  Don't warn, because it is almost certainly a
-  // false positive.
-  if (isa(E) && Loc.isMacroID())
+  // false positive. But we do want to check for WarnUnusedResultAttr on
+  // CallExprs below.
+  if (isa(E) && Loc.isMacroID() && !isa(WarnExpr))
 return;
 
   // Check if this is the UNREFERENCED_PARAMETER from the Microsoft headers.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77572: [clang-tidy] add new check readability-use-anyofallof

[Matthias Gehre via Phabricator via cfe-commits]

mgehre marked 6 inline comments as done.
mgehre added a comment.

In D77572#1965143 , @njames93 wrote:

> I'm struggling to see the value of this check though. If it was reworked to 
> check for loop in the middle of a function it would have a lot more value.


This is (just) a first step. The next step is to detect the local variable case 
as you also described it. Note that this also catches functions
that do some preprocessing and end with a any_of-style loop.
I also have a local branch that generates fixits, but they add quite some code, 
so I wanted to put them in a separate PR.

In LLVM & clang, the check in this PR already emits 370 unique warnings. 
F11685854: readability-use-anyofallof.log 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77572



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

[clang] dbe8678 - [OPENMP]Do not capture global marked as shared in OpenMP region.

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2020-04-07T17:33:17-04:00
New Revision: dbe86786f7f19c3f1338437f4275797e08501efd

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

LOG: [OPENMP]Do not capture global marked as shared in OpenMP region.

No need to capture the global variable marked as shared in the OpenMP
region, the original variable can be used.

Added: 


Modified: 
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/parallel_codegen.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index e9b18f6e9307..e969b7044a89 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -2107,6 +2107,9 @@ VarDecl *Sema::isOpenMPCapturedDecl(ValueDecl *D, bool 
CheckScopeInfo,
 // Threadprivate variables must not be captured.
 if (isOpenMPThreadPrivate(DVarPrivate.CKind))
   return nullptr;
+// Global shared must not be captured.
+if (VD && !VD->hasLocalStorage() && DVarPrivate.CKind == OMPC_shared)
+  return nullptr;
 // The variable is not private or it is the variable in the directive with
 // default(none) clause and not used in any clause.
 DVarPrivate = DSAStack->hasDSA(D, isOpenMPPrivate,

diff  --git a/clang/test/OpenMP/parallel_codegen.cpp 
b/clang/test/OpenMP/parallel_codegen.cpp
index 586187f09521..d42312193709 100644
--- a/clang/test/OpenMP/parallel_codegen.cpp
+++ b/clang/test/OpenMP/parallel_codegen.cpp
@@ -21,10 +21,10 @@
 // CHECK-DEBUG-DAG: %struct.ident_t = type { i32, i32, i32, i32, i8* }
 // CHECK-DEBUG-DAG: [[STR:@.+]] = private unnamed_addr constant [23 x i8] 
c";unknown;unknown;0;0;;\00"
 // CHECK-DEBUG-DAG: [[DEF_LOC_2:@.+]] = private unnamed_addr global 
%struct.ident_t { i32 0, i32 2, i32 0, i32 0, i8* getelementptr inbounds ([23 x 
i8], [23 x i8]* [[STR]], i32 0, i32 0) }
-// CHECK-DEBUG-DAG: [[LOC1:@.+]] = private unnamed_addr constant [{{.+}} x i8] 
c";{{.*}}parallel_codegen.cpp;main;[[@LINE+22]];1;;\00"
+// CHECK-DEBUG-DAG: [[LOC1:@.+]] = private unnamed_addr constant [{{.+}} x i8] 
c";{{.*}}parallel_codegen.cpp;main;[[@LINE+23]];1;;\00"
 // CHECK-DEBUG-DAG: [[LOC2:@.+]] = private unnamed_addr constant [{{.+}} x i8] 
c";{{.*}}parallel_codegen.cpp;tmain;[[@LINE+11]];1;;\00"
 // IRBUILDER-DEBUG-DAG: %struct.ident_t = type { i32, i32, i32, i32, i8* }
-// IRBUILDER-DEBUG-DAG: [[LOC1:@.+]] = private unnamed_addr constant [{{.+}} x 
i8] c";{{.*}}parallel_codegen.cpp;main;[[@LINE+19]];0;;\00"
+// IRBUILDER-DEBUG-DAG: [[LOC1:@.+]] = private unnamed_addr constant [{{.+}} x 
i8] c";{{.*}}parallel_codegen.cpp;main;[[@LINE+20]];0;;\00"
 // IRBUILDER-DEBUG-DAG: [[LOC2:@.+]] = private unnamed_addr constant [{{.+}} x 
i8] c";{{.*}}parallel_codegen.cpp;tmain;[[@LINE+8]];0;;\00"
 
 template 
@@ -41,10 +41,11 @@ int tmain(T argc) {
   return 0;
 }
 
+int global;
 int main (int argc, char **argv) {
   int a[argc];
-#pragma omp parallel
-  foo(a[1]);
+#pragma omp parallel shared(global, a) default(none)
+  (void)global, foo(a[1]);
   return tmain(argv);
 }
 



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

[clang] 9e6670b - [Driver] Only pass LTO remark arguments if the driver asks for it

[Francis Visoiu Mistrih via cfe-commits]

Author: Francis Visoiu Mistrih
Date: 2020-04-07T14:11:47-07:00
New Revision: 9e6670b03ceaa5980eccb06e2dd037e6a9584c66

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

LOG: [Driver] Only pass LTO remark arguments if the driver asks for it

Previous fix missed a check to willEmitRemarks, causing remarks to
always be enabled for LTO.

Added: 


Modified: 
clang/lib/Driver/ToolChains/Darwin.cpp
clang/test/Driver/darwin-opt-record-ld.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Darwin.cpp 
b/clang/lib/Driver/ToolChains/Darwin.cpp
index ab984271555b..a113d05cc579 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -533,7 +533,8 @@ void darwin::Linker::ConstructJob(Compilation , const 
JobAction ,
   // we follow suite for ease of comparison.
   AddLinkArgs(C, Args, CmdArgs, Inputs);
 
-  if (checkRemarksOptions(getToolChain().getDriver(), Args,
+  if (willEmitRemarks(Args) &&
+  checkRemarksOptions(getToolChain().getDriver(), Args,
   getToolChain().getTriple()))
 renderRemarksOptions(Args, CmdArgs, getToolChain().getTriple(), Output, 
JA);
 

diff  --git a/clang/test/Driver/darwin-opt-record-ld.c 
b/clang/test/Driver/darwin-opt-record-ld.c
index 0e1e312c493d..83630ed01da8 100644
--- a/clang/test/Driver/darwin-opt-record-ld.c
+++ b/clang/test/Driver/darwin-opt-record-ld.c
@@ -2,6 +2,10 @@
 
 // RUN: touch %t.o
 //
+// Check that we're not passing -lto-pass-remarks-output if not requested
+// RUN: %clang -target x86_64-apple-darwin12 %t.o -### -o foo/bar.out 2> %t.log
+// RUN: FileCheck -check-prefix=NO_PASS_REMARKS_OUTPUT %s < %t.log
+// NO_PASS_REMARKS_OUTPUT-NOT: -lto-pass-remarks
 // Check that we're passing -lto-pass-remarks-output for LTO
 // RUN: %clang -target x86_64-apple-darwin12 %t.o -fsave-optimization-record 
-### -o foo/bar.out 2> %t.log
 // RUN: FileCheck -check-prefix=PASS_REMARKS_OUTPUT %s < %t.log



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

[PATCH] D77085: [clang-tidy] Added support for validating configuration options

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai added a comment.

In D77085#1967864 , @njames93 wrote:

> In D77085#1967807 , @nemanjai wrote:
>
> > A recent commit has taken down a whole bunch of bots. The build error 
> > messages all seem to point to code in this patch. If this is indeed the 
> > cause, please revert.
>
>
> I was aware and hopefully this fixes the issue 
> https://github.com/llvm/llvm-project/commit/0361798dbeb6ead0a79ab7985f02da347fce988e


Awesome, thanks. Certainly fixes the compile time failures in my local build. 
There is still a link-time failure (undefined reference) with my shared 
libraries build:

  
tools/clang/tools/extra/clang-tidy/tool/CMakeFiles/obj.clangTidyMain.dir/ClangTidyMain.cpp.o:
 In function 
`clang::ast_matchers::internal::matcher_isAllowedToContainClauseKind0Matcher::matches(clang::OMPExecutableDirective
 const&, clang::ast_matchers::internal::ASTMatchFinder*, 
clang::ast_matchers::internal::BoundNodesTreeBuilder*) const':
  
ClangTidyMain.cpp:(.text._ZNK5clang12ast_matchers8internal44matcher_isAllowedToContainClauseKind0Matcher7matchesERKNS_22OMPExecutableDirectiveEPNS1_14ASTMatchFinderEPNS1_21BoundNodesTreeBuilderE[_ZNK5clang12ast_matchers8internal44matcher_isAllowedToContainClauseKind0Matcher7matchesERKNS_22OMPExecutableDirectiveEPNS1_14ASTMatchFinderEPNS1_21BoundNodesTreeBuilderE]+0x50):
 undefined reference to 
`llvm::omp::isAllowedClauseForDirective(llvm::omp::Directive, 
llvm::omp::Clause, unsigned int)'
  collect2: error: ld returned 1 exit status

But that may be unrelated to this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77085



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

[PATCH] D77498: [Hexagon] Select lld as the default linker for linux-musl target

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

This does not take into account `CLANG_DEFAULT_LINKER`, resulting in a 
`check-clang` failure:

  $ cmake -GNinja \
-DCMAKE_C_COMPILER=clang \
-DCMAKE_CXX_COMPILER=clang++ \
-DLLVM_ENABLE_PROJECTS=clang \
-DPYTHON_EXECUTABLE=$(command -v python3) \
../llvm && ninja check-clang
  ...
  Testing Time: 163.80s
Expected Passes: 17055
Expected Failures  : 23
Unsupported Tests  : 55
  
  $ cmake -GNinja \
-DCMAKE_C_COMPILER=clang \
-DCMAKE_CXX_COMPILER=clang++ \
-DLLVM_ENABLE_PROJECTS=clang \
-DPYTHON_EXECUTABLE=$(command -v python3) \
-DCLANG_DEFAULT_LINKER=lld \
../llvm && ninja check-clang
  ...
  Testing Time: 176.55s
  
  Failing Tests (1):
  Clang :: Driver/hexagon-toolchain-elf.c
  
Expected Passes: 17050
Expected Failures  : 23
Unsupported Tests  : 59
Unexpected Failures: 1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77498



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

[PATCH] D77085: [clang-tidy] Added support for validating configuration options

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D77085#1967807 , @nemanjai wrote:

> A recent commit has taken down a whole bunch of bots. The build error 
> messages all seem to point to code in this patch. If this is indeed the 
> cause, please revert.


I was aware and hopefully this fixes the issue 
https://github.com/llvm/llvm-project/commit/0361798dbeb6ead0a79ab7985f02da347fce988e


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77085



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

[PATCH] D77085: [clang-tidy] Added support for validating configuration options

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai added a comment.
Herald added a subscriber: wuzish.

A recent commit has taken down a whole bunch of bots. The build error messages 
all seem to point to code in this patch. If this is indeed the cause, please 
revert.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77085



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

[clang-tools-extra] 0361798 - [clang-tidy] Fix buildbot failing with explicit specialization in class scope

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2020-04-07T21:30:29+01:00
New Revision: 0361798dbeb6ead0a79ab7985f02da347fce988e

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

LOG: [clang-tidy] Fix buildbot failing with explicit specialization in class 
scope

Added: 


Modified: 
clang-tools-extra/clang-tidy/ClangTidyCheck.h

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/ClangTidyCheck.h 
b/clang-tools-extra/clang-tidy/ClangTidyCheck.h
index e90e92f6e136..84438c21a30b 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyCheck.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyCheck.h
@@ -312,41 +312,6 @@ class ClangTidyCheck : public 
ast_matchers::MatchFinder::MatchCallback {
   return Default;
 }
 
-/// Read a named option from the ``Context`` and parse it as a bool.
-///
-/// Reads the option with the check-local name \p LocalName from the
-/// ``CheckOptions``. If the corresponding key is not present, returns
-/// a ``MissingOptionError``. If the corresponding key can't be parsed as
-/// a bool, return an ``UnparseableIntegerOptionError``.
-template <> llvm::Expected get(StringRef LocalName) const;
-
-/// Read a named option from the ``Context`` and parse it as a bool.
-///
-/// Reads the option with the check-local name \p LocalName from the
-/// ``CheckOptions``. If the corresponding key is not present or it can't 
be
-/// parsed as a bool, returns \p Default.
-template <> bool get(StringRef LocalName, bool Default) const;
-
-/// Read a named option from the ``Context`` and parse it as a bool.
-///
-/// Reads the option with the check-local name \p LocalName from local or
-/// global ``CheckOptions``. Gets local option first. If local is not
-/// present, falls back to get global option. If global option is not
-/// present either, returns a ``MissingOptionError``. If the corresponding
-/// key can't be parsed as a bool, return an
-/// ``UnparseableIntegerOptionError``.
-template <>
-llvm::Expected getLocalOrGlobal(StringRef LocalName) const;
-
-/// Read a named option from the ``Context`` and parse it as a bool.
-///
-/// Reads the option with the check-local name \p LocalName from local or
-/// global ``CheckOptions``. Gets local option first. If local is not
-/// present, falls back to get global option. If global option is not
-/// present either or it can't be parsed as a bool, returns \p Default.
-template <>
-bool getLocalOrGlobal(StringRef LocalName, bool Default) const;
-
 /// Read a named option from the ``Context`` and parse it as an
 /// enum type ``T`` using the \p Mapping provided. If \p IgnoreCase is set,
 /// it will search the mapping ignoring the case.
@@ -488,6 +453,47 @@ class ClangTidyCheck : public 
ast_matchers::MatchFinder::MatchCallback {
   const LangOptions () const { return Context->getLangOpts(); }
 };
 
+/// Read a named option from the ``Context`` and parse it as a bool.
+///
+/// Reads the option with the check-local name \p LocalName from the
+/// ``CheckOptions``. If the corresponding key is not present, returns
+/// a ``MissingOptionError``. If the corresponding key can't be parsed as
+/// a bool, return an ``UnparseableIntegerOptionError``.
+template <>
+llvm::Expected
+ClangTidyCheck::OptionsView::get(StringRef LocalName) const;
+
+/// Read a named option from the ``Context`` and parse it as a bool.
+///
+/// Reads the option with the check-local name \p LocalName from the
+/// ``CheckOptions``. If the corresponding key is not present or it can't be
+/// parsed as a bool, returns \p Default.
+template <>
+bool ClangTidyCheck::OptionsView::get(StringRef LocalName,
+bool Default) const;
+
+/// Read a named option from the ``Context`` and parse it as a bool.
+///
+/// Reads the option with the check-local name \p LocalName from local or
+/// global ``CheckOptions``. Gets local option first. If local is not
+/// present, falls back to get global option. If global option is not
+/// present either, returns a ``MissingOptionError``. If the corresponding
+/// key can't be parsed as a bool, return an
+/// ``UnparseableIntegerOptionError``.
+template <>
+llvm::Expected
+ClangTidyCheck::OptionsView::getLocalOrGlobal(StringRef LocalName) const;
+
+/// Read a named option from the ``Context`` and parse it as a bool.
+///
+/// Reads the option with the check-local name \p LocalName from local or
+/// global ``CheckOptions``. Gets local option first. If local is not
+/// present, falls back to get global option. If global option is not
+/// present either or it can't be parsed as a bool, returns \p Default.
+template <>
+bool 

[PATCH] D75936: Add a Pass to X86 that builds a Condensed CFG for Load Value Injection (LVI) Gadgets [4/6]

[Scott Constable via Phabricator via cfe-commits]

sconstab added a comment.

Summary points for @craig.topper who has commandeered this diff:

- fix the typo that Matt pointed out
- `SizeT` should not be a template parameter, and `size_type` should be fixed 
to `int`.
- Maybe have a member reference in `MachineGadgetGraph` to the associated 
`MachineFunction`.
- Determine how this pass (and other X86 machine passes) should fail on 
unsupported X86 subtargets.




Comment at: llvm/lib/Target/X86/ImmutableGraph.h:41
+class ImmutableGraph {
+  using Traits = GraphTraits *>;
+  template  friend class ImmutableGraphBuilder;

mattdr wrote:
> I think this self-reference to `ImmutableGraph` dropped the `SizeT` parameter.
Yup. Good catch.



Comment at: llvm/lib/Target/X86/ImmutableGraph.h:73
+// The end of this Node's edges is the beginning of the next node's edges.
+const Edge *edges_end() const { return (this + 1)->Edges; }
+ArrayRef edges() const {

mattdr wrote:
> Seems like you also want to add a comment here that we know we will never be 
> asked for `edges_end` for the last stored node -- that is, we know that `this 
> + 1` always refers to a valid Node (which is presumably a dummy/sentinel)
Not sure I agree. I cannot think of a conventional use of this interface that 
would perform an operation on the sentinel.
```
G->nodes_end().edges_end(); // invalid use of any end iterator
SomeNode.edges_end(); // invalid if SomeNode == G->nodes_end()
```
That is, the way that we "know" that we will never be asked for `edges_end()` 
for the last stored node is that the ask itself would already violate C++ 
conventions.



Comment at: llvm/lib/Target/X86/ImmutableGraph.h:79
+
+protected:
+  ImmutableGraph(std::unique_ptr Nodes, std::unique_ptr Edges,

mattdr wrote:
> Why "protected" rather than "private"? Usually seeing "protected" makes me 
> think subclassing is expected, but that doesn't seem to be the case here.
The `MachineGadgetGraph` class actually does subclass `ImmutableGraph` to add 
some contextual information. I did not want the constructors for 
`ImmutableGraph` to be public, because the intent is to use the builder. So 
`protected` seemed like the best option.



Comment at: llvm/lib/Target/X86/ImmutableGraph.h:117
+NodeSet(const ImmutableGraph , bool ContainsAll = false)
+: G{G}, V{static_cast(G.nodes_size()), ContainsAll} {}
+bool insert(const Node ) {

mattdr wrote:
> How do we know that a value of `size_type` (aka `SizeT`) can be cast to 
> `unsigned` without truncation?
Ah. We do not know that. We could have a static assert here, but maybe the best 
thing to do would be to follow Matt's earlier advice and fix `size_type` to 
`int`, rather than have it as a template parameter. Anything larger would break 
the `BitVectors` and/or waste space.



Comment at: llvm/lib/Target/X86/X86LoadValueInjectionLoadHardening.cpp:113
+  NumFences(NumFences), NumGadgets(NumGadgets) {}
+MachineFunction () { // FIXME: This function should be cleaner
+  for (const Node  : nodes())

mattdr wrote:
> Cleaner how?
Maybe by keeping a member reference to the associated `MachineFunction`?



Comment at: llvm/lib/Target/X86/X86LoadValueInjectionLoadHardening.cpp:233
+  if (!STI->useLVILoadHardening() || !STI->is64Bit())
+return false; // FIXME: support 32-bit
+

mattdr wrote:
> If the user requests hardening and we can't do it, it seems better to fail 
> loudly so they don't accidentally deploy an unmitigated binary.
@craig.topper I think this is related to the discussion we were having about 
what would happen for SLH on unsupported subtargets. I'm not sure what the most 
appropriate solution would be.


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

https://reviews.llvm.org/D75936



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

[PATCH] D77666: [OpenMP] "UnFix" layering problem with FrontendOpenMP

[Johannes Doerfert via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf9d558c87133: [OpenMP] UnFix layering problem 
with FrontendOpenMP (authored by jdoerfert).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77666

Files:
  clang-tools-extra/clang-change-namespace/CMakeLists.txt
  clang-tools-extra/clang-change-namespace/tool/CMakeLists.txt
  clang-tools-extra/clang-doc/CMakeLists.txt
  clang-tools-extra/clang-include-fixer/find-all-symbols/CMakeLists.txt
  clang-tools-extra/clang-move/CMakeLists.txt
  clang-tools-extra/clang-query/CMakeLists.txt
  clang-tools-extra/clang-reorder-fields/CMakeLists.txt
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/abseil/CMakeLists.txt
  clang-tools-extra/clang-tidy/android/CMakeLists.txt
  clang-tools-extra/clang-tidy/boost/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/cert/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/darwin/CMakeLists.txt
  clang-tools-extra/clang-tidy/fuchsia/CMakeLists.txt
  clang-tools-extra/clang-tidy/google/CMakeLists.txt
  clang-tools-extra/clang-tidy/hicpp/CMakeLists.txt
  clang-tools-extra/clang-tidy/linuxkernel/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvmlibc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/mpi/CMakeLists.txt
  clang-tools-extra/clang-tidy/objc/CMakeLists.txt
  clang-tools-extra/clang-tidy/openmp/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/portability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/utils/CMakeLists.txt
  clang-tools-extra/clang-tidy/zircon/CMakeLists.txt
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/tool-template/CMakeLists.txt
  clang-tools-extra/unittests/clang-change-namespace/CMakeLists.txt
  clang-tools-extra/unittests/clang-doc/CMakeLists.txt
  
clang-tools-extra/unittests/clang-include-fixer/find-all-symbols/CMakeLists.txt
  clang-tools-extra/unittests/clang-move/CMakeLists.txt
  clang-tools-extra/unittests/clang-query/CMakeLists.txt
  clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
  clang/lib/ASTMatchers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Core/CMakeLists.txt
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/Transformer/CMakeLists.txt
  clang/unittests/AST/CMakeLists.txt
  clang/unittests/ASTMatchers/CMakeLists.txt
  clang/unittests/ASTMatchers/Dynamic/CMakeLists.txt
  clang/unittests/Analysis/CMakeLists.txt
  clang/unittests/Rename/CMakeLists.txt
  clang/unittests/Sema/CMakeLists.txt
  clang/unittests/StaticAnalyzer/CMakeLists.txt
  clang/unittests/Tooling/CMakeLists.txt

Index: clang/unittests/Tooling/CMakeLists.txt
===
--- clang/unittests/Tooling/CMakeLists.txt
+++ clang/unittests/Tooling/CMakeLists.txt
@@ -1,5 +1,6 @@
 set(LLVM_LINK_COMPONENTS
   ${LLVM_TARGETS_TO_BUILD}
+  FrontendOpenMP
   Support
   )
 
Index: clang/unittests/StaticAnalyzer/CMakeLists.txt
===
--- clang/unittests/StaticAnalyzer/CMakeLists.txt
+++ clang/unittests/StaticAnalyzer/CMakeLists.txt
@@ -1,4 +1,5 @@
 set(LLVM_LINK_COMPONENTS
+  FrontendOpenMP
   Support
   )
 
Index: clang/unittests/Sema/CMakeLists.txt
===
--- clang/unittests/Sema/CMakeLists.txt
+++ clang/unittests/Sema/CMakeLists.txt
@@ -1,4 +1,5 @@
 set(LLVM_LINK_COMPONENTS
+  FrontendOpenMP
   Support
   )
 
Index: clang/unittests/Rename/CMakeLists.txt
===
--- clang/unittests/Rename/CMakeLists.txt
+++ clang/unittests/Rename/CMakeLists.txt
@@ -1,4 +1,5 @@
 set(LLVM_LINK_COMPONENTS
+  FrontendOpenMP
   support
   )
 
Index: clang/unittests/Analysis/CMakeLists.txt
===
--- clang/unittests/Analysis/CMakeLists.txt
+++ clang/unittests/Analysis/CMakeLists.txt
@@ -1,4 +1,5 @@
 set(LLVM_LINK_COMPONENTS
+  FrontendOpenMP
   Support
   )
 
Index: clang/unittests/ASTMatchers/Dynamic/CMakeLists.txt
===
--- clang/unittests/ASTMatchers/Dynamic/CMakeLists.txt
+++ clang/unittests/ASTMatchers/Dynamic/CMakeLists.txt
@@ -1,4 +1,5 @@
 set(LLVM_LINK_COMPONENTS
+  FrontendOpenMP
   Support
   )
 
Index: clang/unittests/ASTMatchers/CMakeLists.txt
===
--- 

[PATCH] D77644: [clangd] Handle additional includes while parsing ASTs

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

this doesn't handle source locations correctly in presence of deleted headers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77644



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

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

[Andrew via Phabricator via cfe-commits]

browneee updated this revision to Diff 255773.
browneee marked an inline comment as done.
browneee added a comment.
Herald added subscribers: cfe-commits, arphaman.
Herald added a project: clang.

Fix build.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77621

Files:
  clang/include/clang/Serialization/ASTWriter.h
  clang/include/clang/Serialization/PCHContainerOperations.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/SerializedDiagnosticPrinter.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/GlobalModuleIndex.cpp
  llvm/include/llvm/Bitcode/BitcodeWriter.h
  llvm/include/llvm/Bitstream/BitstreamWriter.h
  llvm/include/llvm/Remarks/BitstreamRemarkSerializer.h
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/ExecutionEngine/Orc/ThreadSafeModule.cpp
  llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
  llvm/tools/llvm-cat/llvm-cat.cpp
  llvm/tools/llvm-modextract/llvm-modextract.cpp
  llvm/unittests/Bitstream/BitstreamReaderTest.cpp
  llvm/unittests/Bitstream/BitstreamWriterTest.cpp

Index: llvm/unittests/Bitstream/BitstreamWriterTest.cpp
===
--- llvm/unittests/Bitstream/BitstreamWriterTest.cpp
+++ llvm/unittests/Bitstream/BitstreamWriterTest.cpp
@@ -8,27 +8,31 @@
 
 #include "llvm/Bitstream/BitstreamWriter.h"
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SmallString.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
+#include 
+
 using namespace llvm;
 
+using ::testing::IsEmpty;
+
 namespace {
 
 TEST(BitstreamWriterTest, emitBlob) {
-  SmallString<64> Buffer;
+  std::vector Buffer;
   BitstreamWriter W(Buffer);
   W.emitBlob("str", /* ShouldEmitSize */ false);
-  EXPECT_EQ(StringRef("str\0", 4), Buffer);
+  EXPECT_EQ(StringRef("str\0", 4), StringRef(Buffer.data(), Buffer.size()));
 }
 
 TEST(BitstreamWriterTest, emitBlobWithSize) {
-  SmallString<64> Buffer;
+  std::vector Buffer;
   {
 BitstreamWriter W(Buffer);
 W.emitBlob("str");
   }
-  SmallString<64> Expected;
+  std::vector Expected;
   {
 BitstreamWriter W(Expected);
 W.EmitVBR(3, 6);
@@ -38,21 +42,21 @@
 W.Emit('r', 8);
 W.Emit(0, 8);
   }
-  EXPECT_EQ(StringRef(Expected), Buffer);
+  EXPECT_EQ(Expected, Buffer);
 }
 
 TEST(BitstreamWriterTest, emitBlobEmpty) {
-  SmallString<64> Buffer;
+  std::vector Buffer;
   BitstreamWriter W(Buffer);
   W.emitBlob("", /* ShouldEmitSize */ false);
-  EXPECT_EQ(StringRef(""), Buffer);
+  EXPECT_THAT(Buffer, IsEmpty());
 }
 
 TEST(BitstreamWriterTest, emitBlob4ByteAligned) {
-  SmallString<64> Buffer;
+  std::vector Buffer;
   BitstreamWriter W(Buffer);
   W.emitBlob("str0", /* ShouldEmitSize */ false);
-  EXPECT_EQ(StringRef("str0"), Buffer);
+  EXPECT_EQ(StringRef("str0"), StringRef(Buffer.data(), Buffer.size()));
 }
 
 } // end namespace
Index: llvm/unittests/Bitstream/BitstreamReaderTest.cpp
===
--- llvm/unittests/Bitstream/BitstreamReaderTest.cpp
+++ llvm/unittests/Bitstream/BitstreamReaderTest.cpp
@@ -11,6 +11,8 @@
 #include "llvm/Bitstream/BitstreamWriter.h"
 #include "gtest/gtest.h"
 
+#include 
+
 using namespace llvm;
 
 namespace {
@@ -96,7 +98,7 @@
 StringRef BlobIn((const char *)BlobData.begin(), BlobSize);
 
 // Write the bitcode.
-SmallVector Buffer;
+std::vector Buffer;
 unsigned AbbrevID;
 {
   BitstreamWriter Stream(Buffer);
@@ -115,7 +117,7 @@
 
 // Stream the buffer into the reader.
 BitstreamCursor Stream(
-ArrayRef((const uint8_t *)Buffer.begin(), Buffer.size()));
+ArrayRef((const uint8_t *)Buffer.data(), Buffer.size()));
 
 // Header.  Included in test so that we can run llvm-bcanalyzer to debug
 // when there are problems.
Index: llvm/tools/llvm-modextract/llvm-modextract.cpp
===
--- llvm/tools/llvm-modextract/llvm-modextract.cpp
+++ llvm/tools/llvm-modextract/llvm-modextract.cpp
@@ -58,12 +58,12 @@
   ExitOnErr(errorCodeToError(EC));
 
   if (BinaryExtract) {
-SmallVector Result;
+std::vector Result;
 BitcodeWriter Writer(Result);
-Result.append(Ms[ModuleIndex].getBuffer().begin(),
+Result.insert(Result.end(), Ms[ModuleIndex].getBuffer().begin(),
   Ms[ModuleIndex].getBuffer().end());
 Writer.copyStrtab(Ms[ModuleIndex].getStrtab());
-Out->os() << Result;
+Out->os().write(Result.data(), Result.size());
 Out->keep();
 return 0;
   }
Index: llvm/tools/llvm-cat/llvm-cat.cpp
===
--- llvm/tools/llvm-cat/llvm-cat.cpp
+++ llvm/tools/llvm-cat/llvm-cat.cpp
@@ -54,7 +54,7 @@
   ExitOnError ExitOnErr("llvm-cat: ");
   LLVMContext Context;
 
-  SmallVector Buffer;
+  std::vector Buffer;
   BitcodeWriter Writer(Buffer);
   if (BinaryCat) {
 for (const auto  : 

[clang] e0ae907 - [OPENMP][DOCS]Update status of oimplemented constructs, NFC.

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2020-04-07T15:45:08-04:00
New Revision: e0ae907ab5a15fe6d814f4794ab6f4c541149482

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

LOG: [OPENMP][DOCS]Update status of oimplemented constructs, NFC.

Added: 


Modified: 
clang/docs/OpenMPSupport.rst

Removed: 




diff  --git a/clang/docs/OpenMPSupport.rst b/clang/docs/OpenMPSupport.rst
index 7fef9e867885..49cd11e4eeeb 100644
--- a/clang/docs/OpenMPSupport.rst
+++ b/clang/docs/OpenMPSupport.rst
@@ -237,7 +237,7 @@ implementation.
 
+--+--+--+---+
 | misc extension   | conditional modifier for lastprivate clause   
   | :good:`done` | 
  |
 
+--+--+--+---+
-| misc extension   | iterator and multidependences 
   | :part:`claimed`  | 
  |
+| misc extension   | iterator and multidependences 
   | :good:`done` | 
  |
 
+--+--+--+---+
 | misc extension   | depobj directive and depobj dependency kind   
   | :good:`done` | 
  |
 
+--+--+--+---+



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

[clang-tools-extra] f9d558c - [OpenMP] "UnFix" layering problem with FrontendOpenMP

[Johannes Doerfert via cfe-commits]

Author: Johannes Doerfert
Date: 2020-04-07T14:41:18-05:00
New Revision: f9d558c871337699d2815dbf116bae94025f5d90

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

LOG: [OpenMP] "UnFix" layering problem with FrontendOpenMP

This reverts commit 97aa593a8387586095b7eac12974ba2fdd08f4c3 as it
causes problems (PR45453) https://reviews.llvm.org/D77574#1966321.

This additionally adds an explicit reference to FrontendOpenMP to
clang-tidy where ASTMatchers is used.

This is hopefully just a temporary solution. The dependence on
`FrontendOpenMP` from `ASTMatchers` should be handled by CMake
implicitly, not us explicitly.

Reviewed By: aheejin

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

Added: 


Modified: 
clang-tools-extra/clang-change-namespace/CMakeLists.txt
clang-tools-extra/clang-change-namespace/tool/CMakeLists.txt
clang-tools-extra/clang-doc/CMakeLists.txt
clang-tools-extra/clang-include-fixer/find-all-symbols/CMakeLists.txt
clang-tools-extra/clang-move/CMakeLists.txt
clang-tools-extra/clang-query/CMakeLists.txt
clang-tools-extra/clang-reorder-fields/CMakeLists.txt
clang-tools-extra/clang-tidy/CMakeLists.txt
clang-tools-extra/clang-tidy/abseil/CMakeLists.txt
clang-tools-extra/clang-tidy/android/CMakeLists.txt
clang-tools-extra/clang-tidy/boost/CMakeLists.txt
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/clang-tidy/cert/CMakeLists.txt
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tools-extra/clang-tidy/darwin/CMakeLists.txt
clang-tools-extra/clang-tidy/fuchsia/CMakeLists.txt
clang-tools-extra/clang-tidy/google/CMakeLists.txt
clang-tools-extra/clang-tidy/hicpp/CMakeLists.txt
clang-tools-extra/clang-tidy/linuxkernel/CMakeLists.txt
clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
clang-tools-extra/clang-tidy/llvmlibc/CMakeLists.txt
clang-tools-extra/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
clang-tools-extra/clang-tidy/mpi/CMakeLists.txt
clang-tools-extra/clang-tidy/objc/CMakeLists.txt
clang-tools-extra/clang-tidy/openmp/CMakeLists.txt
clang-tools-extra/clang-tidy/performance/CMakeLists.txt
clang-tools-extra/clang-tidy/portability/CMakeLists.txt
clang-tools-extra/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/clang-tidy/utils/CMakeLists.txt
clang-tools-extra/clang-tidy/zircon/CMakeLists.txt
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/unittests/CMakeLists.txt
clang-tools-extra/tool-template/CMakeLists.txt
clang-tools-extra/unittests/clang-change-namespace/CMakeLists.txt
clang-tools-extra/unittests/clang-doc/CMakeLists.txt

clang-tools-extra/unittests/clang-include-fixer/find-all-symbols/CMakeLists.txt
clang-tools-extra/unittests/clang-move/CMakeLists.txt
clang-tools-extra/unittests/clang-query/CMakeLists.txt
clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
clang/lib/ASTMatchers/CMakeLists.txt
clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
clang/lib/StaticAnalyzer/Core/CMakeLists.txt
clang/lib/Tooling/CMakeLists.txt
clang/lib/Tooling/Transformer/CMakeLists.txt
clang/unittests/AST/CMakeLists.txt
clang/unittests/ASTMatchers/CMakeLists.txt
clang/unittests/ASTMatchers/Dynamic/CMakeLists.txt
clang/unittests/Analysis/CMakeLists.txt
clang/unittests/Rename/CMakeLists.txt
clang/unittests/Sema/CMakeLists.txt
clang/unittests/StaticAnalyzer/CMakeLists.txt
clang/unittests/Tooling/CMakeLists.txt

Removed: 




diff  --git a/clang-tools-extra/clang-change-namespace/CMakeLists.txt 
b/clang-tools-extra/clang-change-namespace/CMakeLists.txt
index 178306423eb7..7c0363cd00d0 100644
--- a/clang-tools-extra/clang-change-namespace/CMakeLists.txt
+++ b/clang-tools-extra/clang-change-namespace/CMakeLists.txt
@@ -1,5 +1,6 @@
 set(LLVM_LINK_COMPONENTS
-  support
+  FrontendOpenMP
+  Support
   )
 
 add_clang_library(clangChangeNamespace

diff  --git a/clang-tools-extra/clang-change-namespace/tool/CMakeLists.txt 
b/clang-tools-extra/clang-change-namespace/tool/CMakeLists.txt
index ae48a5e0f798..c168bb4d5794 100644
--- a/clang-tools-extra/clang-change-namespace/tool/CMakeLists.txt
+++ b/clang-tools-extra/clang-change-namespace/tool/CMakeLists.txt
@@ -1,6 +1,7 @@
 include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
 
 set(LLVM_LINK_COMPONENTS
+  FrontendOpenMP
   Support
   )
 

diff  --git a/clang-tools-extra/clang-doc/CMakeLists.txt 
b/clang-tools-extra/clang-doc/CMakeLists.txt
index c301ad5aface..8df7d3ef9098 100644
--- a/clang-tools-extra/clang-doc/CMakeLists.txt
+++ b/clang-tools-extra/clang-doc/CMakeLists.txt
@@ -1,6 +1,7 @@
 set(LLVM_LINK_COMPONENTS
   support
   

[PATCH] D77611: [Sema] fix -Wunused-result in StmtExpr

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D77611#1967488 , @nickdesaulniers 
wrote:

> Note your test case is related to `-Wunused-value`, mine is `-Wunused-result` 
> (via the use of `__attribute__((warn_result_unused))` which we should always 
> warn about.


Right, but the code you're looking at/proposing to change is related to all 
unused expression warnings, by the looks of it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77611



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

[PATCH] D76606: [clang-tidy] Change checks that take enum configurations to use a new access method.

[Nathan James via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9dff9ecdd113: [clang-tidy] Change checks that take enum 
configurations to use a new access… (authored by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76606

Files:
  clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp
  clang-tools-extra/clang-tidy/performance/MoveConstructorInitCheck.cpp
  clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp
  clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
  clang-tools-extra/clang-tidy/utils/IncludeSorter.h
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp
@@ -0,0 +1,15 @@
+// RUN: clang-tidy %s -checks=readability-identifier-naming \
+// RUN:   -config="{CheckOptions: [\
+// RUN:   {key: readability-identifier-naming.FunctionCase, value: camelback}, \
+// RUN:   {key: readability-identifier-naming.VariableCase, value: camelBack}, \
+// RUN:   {key: readability-identifier-naming.ClassCase, value: UUPER_CASE}, \
+// RUN:   {key: readability-identifier-naming.StructCase, value: CAMEL}, \
+// RUN:   {key: readability-identifier-naming.EnumCase, value: AnY_cASe}, \
+// RUN:   ]}" 2>&1 | FileCheck %s --implicit-check-not warning
+
+// CHECK-DAG: warning: invalid configuration value 'camelback' for option 'readability-identifier-naming.FunctionCase'; did you mean 'camelBack'?{{$}}
+// CHECK-DAG: warning: invalid configuration value 'UUPER_CASE' for option 'readability-identifier-naming.ClassCase'; did you mean 'UPPER_CASE'?{{$}}
+// Don't try to suggest an alternative for 'CAMEL'
+// CHECK-DAG: warning: invalid configuration value 'CAMEL' for option 'readability-identifier-naming.StructCase'{{$}}
+// This fails on the EditDistance, but as it matches ignoring case suggest the correct value
+// CHECK-DAG: warning: invalid configuration value 'AnY_cASe' for option 'readability-identifier-naming.EnumCase'; did you mean 'aNy_CasE'?{{$}}
Index: clang-tools-extra/clang-tidy/utils/IncludeSorter.h
===
--- clang-tools-extra/clang-tidy/utils/IncludeSorter.h
+++ clang-tools-extra/clang-tidy/utils/IncludeSorter.h
@@ -25,11 +25,7 @@
   /// Supported include styles.
   enum IncludeStyle { IS_LLVM = 0, IS_Google = 1 };
 
-  /// Converts "llvm" to ``IS_LLVM``, otherwise returns ``IS_Google``.
-  static IncludeStyle parseIncludeStyle(const std::string );
-
-  /// Converts ``IncludeStyle`` to string representation.
-  static StringRef toString(IncludeStyle Style);
+  static ArrayRef> getMapping();
 
   /// The classifications of inclusions, in the order they should be sorted.
   enum IncludeKinds {
Index: clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
===
--- clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
+++ clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
@@ -275,13 +275,11 @@
   return Fixes;
 }
 
-IncludeSorter::IncludeStyle
-IncludeSorter::parseIncludeStyle(const std::string ) {
-  return Value == "llvm" ? IS_LLVM : IS_Google;
-}
-
-StringRef IncludeSorter::toString(IncludeStyle Style) {
-  return Style == IS_LLVM ? "llvm" : "google";
+llvm::ArrayRef>
+IncludeSorter::getMapping() {
+  static constexpr std::pair Mapping[] =
+  {{"llvm", IS_LLVM}, {"google", IS_Google}};
+  return makeArrayRef(Mapping);
 }
 
 } // namespace utils
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -11,8 +11,10 @@
 #include "clang/AST/CXXInheritance.h"
 #include "clang/Lex/PPCallbacks.h"
 #include "clang/Lex/Preprocessor.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/Support/Debug.h"
+#include "llvm/Support/Error.h"
 #include 

[PATCH] D77671: [clangd] Destroy context before resetting CurrentReq

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Our tests stash callbacks into request context and rely on it being
invoked before threads going idle.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77671

Files:
  clang-tools-extra/clangd/TUScheduler.cpp


Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -255,9 +255,11 @@
 NextReq.reset();
   }
 
-  WithContext Guard(std::move(CurrentReq->Ctx));
-  // Build the preamble and let the waiters know about it.
-  build(std::move(*CurrentReq));
+  {
+WithContext Guard(std::move(CurrentReq->Ctx));
+// Build the preamble and let the waiters know about it.
+build(std::move(*CurrentReq));
+  }
   bool IsEmpty = false;
   {
 std::lock_guard Lock(Mutex);


Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -255,9 +255,11 @@
 NextReq.reset();
   }
 
-  WithContext Guard(std::move(CurrentReq->Ctx));
-  // Build the preamble and let the waiters know about it.
-  build(std::move(*CurrentReq));
+  {
+WithContext Guard(std::move(CurrentReq->Ctx));
+// Build the preamble and let the waiters know about it.
+build(std::move(*CurrentReq));
+  }
   bool IsEmpty = false;
   {
 std::lock_guard Lock(Mutex);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77229: [Analyzer][WIP] Avoid handling of LazyCompundVals in IteratorModeling

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware marked an inline comment as done.
baloghadamsoftware added a comment.

In D77229#1967269 , @NoQ wrote:

> In D77229#1966888 , 
> @baloghadamsoftware wrote:
>
> > Not so strange, of course, they are destructed before the `postCall()` as 
> > they should be, but the question still remains: how to keep them alive for 
> > post-checking the call, but not forever.
>
>
> Nope, temporaries are destroyed at the end of the full-expression, which is 
> much later than `PostCall`.
>
> What code are you debugging?


OK, maybe I phrased it incorrectly. It is not destroyed, but not "live" 
anymore. Thus `isLive()` returns false. I am trying to debug 
`vector_insert_begin()` (line 954).




Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:231-235
+if (dyn_cast_or_null(LCtx->getParentMap().getParent(E))) 
{
+  MemRegionManager  = getSValBuilder().getRegionManager();
+  return std::make_pair(
+  State, loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx)));
+}

NoQ wrote:
> baloghadamsoftware wrote:
> > Did you mean this piece of code? It returns `_object{struct 
> > simple_iterator_base, S44016}`. Is this correct? If so, I will factor out 
> > this code and put it into a common function to be used by both this 
> > function and the original one.
> No, this one's for members, we've been talking about base classes.
Oh yes, I see it now. But which one then? Maybe line 585? Or the whole `switch` 
expression? Sorry, I am not sure I fully understand this piece of code.


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

https://reviews.llvm.org/D77229



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

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

[Manoj Gupta via Phabricator via cfe-commits]

manojgupta added a comment.

I opened https://github.com/ClangBuiltLinux/linux/issues/979 to see if we can 
fix this in Linux kernel.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71082



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

[PATCH] D77666: [OpenMP] "UnFix" layering problem with FrontendOpenMP

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D77666#1967506 , @aheejin wrote:

> I don't know much about how this part works, but if this is purely a revert 
> of the previous patch, I don't think you need a review for that.


It is not, as stated in the commit message. A pure revert will cause old 
failures to reappear, this one should not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77666



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

[PATCH] D77665: [CUDA] Simplify GPU variant handling. NFC.

[Artem Belevich via Phabricator via cfe-commits]

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



Comment at: clang/lib/Basic/Cuda.cpp:61
+// clang-format off
+SM2(20, "compute_20"), SM2(21, "compute_20"), // Fermi
+SM(30), SM(32), SM(35), SM(37),  // Kepler

yaxunl wrote:
> Thanks for the efforts. Really appreciate this.
> 
> Can we separate this part to a def file and include it here and also in enum 
> class CudaArch? Then we only need to maintain the def file.
> 
> Thanks.
If we had similar per-GPU code in many places, then it would make a lot of 
sense.
At the moment I'm not sure it would be worth it -- I don't see any other code 
that would benefit, other than this file.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77665



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

[clang-tools-extra] 9dff9ec - [clang-tidy] Change checks that take enum configurations to use a new access method.

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2020-04-07T20:04:31+01:00
New Revision: 9dff9ecdd113ca57b8c548ebcdb14121bfd1840c

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

LOG: [clang-tidy] Change checks that take enum configurations to use a new 
access method.

Summary: Change all checks that take enums as configuration to use enum 
specific methods in `ClangTidyCheck::OptionsView`.

Reviewers: aaron.ballman, alexfh

Reviewed By: aaron.ballman

Subscribers: wuzish, nemanjai, kbarton, arphaman, xazax.hun, cfe-commits

Tags: #clang, #clang-tools-extra

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

Added: 

clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp

Modified: 
clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp

clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp
clang-tools-extra/clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp
clang-tools-extra/clang-tidy/performance/MoveConstructorInitCheck.cpp
clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp
clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
clang-tools-extra/clang-tidy/utils/IncludeSorter.h

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp 
b/clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
index 3f0ca4adb54c..513714e99cb8 100644
--- a/clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
+++ b/clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
@@ -26,8 +26,9 @@ 
StringFindStartswithCheck::StringFindStartswithCheck(StringRef Name,
 : ClangTidyCheck(Name, Context),
   StringLikeClasses(utils::options::parseStringList(
   Options.get("StringLikeClasses", "::std::basic_string"))),
-  IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
-  Options.getLocalOrGlobal("IncludeStyle", "llvm"))),
+  IncludeStyle(Options.getLocalOrGlobal("IncludeStyle",
+utils::IncludeSorter::getMapping(),
+utils::IncludeSorter::IS_LLVM)),
   AbseilStringsMatchHeader(
   Options.get("AbseilStringsMatchHeader", "absl/strings/match.h")) {}
 
@@ -121,8 +122,8 @@ void StringFindStartswithCheck::storeOptions(
 ClangTidyOptions::OptionMap ) {
   Options.store(Opts, "StringLikeClasses",
 utils::options::serializeStringList(StringLikeClasses));
-  Options.store(Opts, "IncludeStyle",
-utils::IncludeSorter::toString(IncludeStyle));
+  Options.store(Opts, "IncludeStyle", IncludeStyle,
+utils::IncludeSorter::getMapping());
   Options.store(Opts, "AbseilStringsMatchHeader", AbseilStringsMatchHeader);
 }
 

diff  --git 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
index f616efb1db6b..a5756ff634bb 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -24,8 +24,9 @@ AST_MATCHER(VarDecl, isLocalVarDecl) { return 
Node.isLocalVarDecl(); }
 InitVariablesCheck::InitVariablesCheck(StringRef Name,
ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
-  IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
-  Options.getLocalOrGlobal("IncludeStyle", "llvm"))),
+  IncludeStyle(Options.getLocalOrGlobal("IncludeStyle",
+utils::IncludeSorter::getMapping(),
+utils::IncludeSorter::IS_LLVM)),
   MathHeader(Options.get("MathHeader", "math.h")) {}
 
 void InitVariablesCheck::registerMatchers(MatchFinder *Finder) {

diff  --git 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
index 56a8a8140271..b48511287f88 100644
--- 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
+++ 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
@@ -21,8 +21,9 @@ 

[PATCH] D77670: [CUDA] Add partial support for recent CUDA versions.

[Artem Belevich via Phabricator via cfe-commits]

tra created this revision.
tra added a reviewer: yaxunl.
Herald added subscribers: sanjoy.google, bixia, hiraditya, jholewinski.
Herald added a project: clang.

Generate PTX using newer versions of PTX and allow using sm_80 with CUDA-11.
None of the new features of CUDA-10.2+ have been implemented yet, so using these
versions will still produce a warning.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77670

Files:
  clang/include/clang/Basic/Cuda.h
  clang/lib/Basic/Cuda.cpp
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  clang/lib/Driver/ToolChains/Cuda.cpp
  llvm/lib/Target/NVPTX/NVPTX.td

Index: llvm/lib/Target/NVPTX/NVPTX.td
===
--- llvm/lib/Target/NVPTX/NVPTX.td
+++ llvm/lib/Target/NVPTX/NVPTX.td
@@ -55,6 +55,8 @@
  "Target SM 7.2">;
 def SM75 : SubtargetFeature<"sm_75", "SmVersion", "75",
  "Target SM 7.5">;
+def SM80 : SubtargetFeature<"sm_80", "SmVersion", "80",
+ "Target SM 8.0">;
 
 // PTX Versions
 def PTX32 : SubtargetFeature<"ptx32", "PTXVersion", "32",
@@ -77,6 +79,10 @@
  "Use PTX version 6.3">;
 def PTX64 : SubtargetFeature<"ptx64", "PTXVersion", "64",
  "Use PTX version 6.4">;
+def PTX65 : SubtargetFeature<"ptx65", "PTXVersion", "65",
+ "Use PTX version 6.5">;
+def PTX70 : SubtargetFeature<"ptx70", "PTXVersion", "70",
+ "Use PTX version 7.0">;
 
 //===--===//
 // NVPTX supported processors.
@@ -100,6 +106,7 @@
 def : Proc<"sm_70", [SM70, PTX60]>;
 def : Proc<"sm_72", [SM72, PTX61]>;
 def : Proc<"sm_75", [SM75, PTX63]>;
+def : Proc<"sm_80", [SM80, PTX70]>;
 
 def NVPTXInstrInfo : InstrInfo {
 }
Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -45,17 +45,22 @@
 return;
   DetectedVersion = join_items(".", VersionParts[0], VersionParts[1]);
   Version = CudaStringToVersion(DetectedVersion);
-  if (Version != CudaVersion::UNKNOWN)
+  if (Version != CudaVersion::UNKNOWN) {
+// TODO(tra): remove the warning once we have all features of 10.2 and 11.0
+// implemented.
+DetectedVersionIsNotSupported = Version > CudaVersion::LATEST_SUPPORTED;
 return;
+  }
 
-  Version = CudaVersion::LATEST;
+  Version = CudaVersion::LATEST_SUPPORTED;
   DetectedVersionIsNotSupported = true;
 }
 
 void CudaInstallationDetector::WarnIfUnsupportedVersion() {
   if (DetectedVersionIsNotSupported)
 D.Diag(diag::warn_drv_unknown_cuda_version)
-<< DetectedVersion << CudaVersionToString(Version);
+<< DetectedVersion
+<< CudaVersionToString(CudaVersion::LATEST_SUPPORTED);
 }
 
 CudaInstallationDetector::CudaInstallationDetector(
@@ -639,24 +644,30 @@
   // by new PTX version, so we need to raise PTX level to enable them in NVPTX
   // back-end.
   const char *PtxFeature = nullptr;
-  switch(CudaInstallation.version()) {
-case CudaVersion::CUDA_101:
-  PtxFeature = "+ptx64";
-  break;
-case CudaVersion::CUDA_100:
-  PtxFeature = "+ptx63";
-  break;
-case CudaVersion::CUDA_92:
-  PtxFeature = "+ptx61";
-  break;
-case CudaVersion::CUDA_91:
-  PtxFeature = "+ptx61";
-  break;
-case CudaVersion::CUDA_90:
-  PtxFeature = "+ptx60";
-  break;
-default:
-  PtxFeature = "+ptx42";
+  switch (CudaInstallation.version()) {
+  case CudaVersion::CUDA_110:
+PtxFeature = "+ptx70";
+break;
+  case CudaVersion::CUDA_102:
+PtxFeature = "+ptx65";
+break;
+  case CudaVersion::CUDA_101:
+PtxFeature = "+ptx64";
+break;
+  case CudaVersion::CUDA_100:
+PtxFeature = "+ptx63";
+break;
+  case CudaVersion::CUDA_92:
+PtxFeature = "+ptx61";
+break;
+  case CudaVersion::CUDA_91:
+PtxFeature = "+ptx61";
+break;
+  case CudaVersion::CUDA_90:
+PtxFeature = "+ptx60";
+break;
+  default:
+PtxFeature = "+ptx42";
   }
   CC1Args.append({"-target-feature", PtxFeature});
   if (DriverArgs.hasFlag(options::OPT_fcuda_short_ptr,
Index: clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -4992,6 +4992,7 @@
   case CudaArch::SM_70:
   case CudaArch::SM_72:
   case CudaArch::SM_75:
+  case CudaArch::SM_80:
   case CudaArch::GFX600:
   case CudaArch::GFX601:
   case CudaArch::GFX700:
@@ -5049,6 +5050,7 @@
   case CudaArch::SM_70:
   case CudaArch::SM_72:
   case CudaArch::SM_75:
+  case CudaArch::SM_80:
 return {84, 32};
   case CudaArch::GFX600:
   case CudaArch::GFX601:
Index: 

[PATCH] D77085: [clang-tidy] Added support for validating configuration options

[Nathan James via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfcf7cc268fe4: [clang-tidy] Added support for validating 
configuration options (authored by njames93).

Changed prior to commit:
  https://reviews.llvm.org/D77085?vs=255023=255764#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77085

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -1,6 +1,8 @@
 #include "ClangTidyOptions.h"
-#include "gtest/gtest.h"
+#include "ClangTidyCheck.h"
+#include "ClangTidyDiagnosticConsumer.h"
 #include "llvm/ADT/StringExtras.h"
+#include "gtest/gtest.h"
 
 namespace clang {
 namespace tidy {
@@ -97,6 +99,172 @@
 llvm::join(Options.ExtraArgsBefore->begin(),
Options.ExtraArgsBefore->end(), ","));
 }
+
+class TestCheck : public ClangTidyCheck {
+public:
+  TestCheck(ClangTidyContext *Context) : ClangTidyCheck("test", Context) {}
+
+  template  auto getLocal(Args &&... Arguments) {
+return Options.get(std::forward(Arguments)...);
+  }
+
+  template  auto getGlobal(Args &&... Arguments) {
+return Options.getLocalOrGlobal(std::forward(Arguments)...);
+  }
+
+  template 
+  auto getIntLocal(Args &&... Arguments) {
+return Options.get(std::forward(Arguments)...);
+  }
+
+  template 
+  auto getIntGlobal(Args &&... Arguments) {
+return Options.getLocalOrGlobal(std::forward(Arguments)...);
+  }
+};
+
+#define CHECK_VAL(Value, Expected) \
+  do { \
+auto Item = Value; \
+ASSERT_TRUE(!!Item);   \
+EXPECT_EQ(*Item, Expected);\
+  } while (false)
+
+#define CHECK_ERROR(Value, ErrorType, ExpectedMessage) \
+  do { \
+auto Item = Value; \
+ASSERT_FALSE(Item);\
+ASSERT_TRUE(Item.errorIsA());   \
+ASSERT_FALSE(llvm::handleErrors(   \
+Item.takeError(), [&](const ErrorType ) -> llvm::Error {   \
+  EXPECT_EQ(Err.message(), ExpectedMessage);   \
+  return llvm::Error::success();   \
+}));   \
+  } while (false)
+
+TEST(CheckOptionsValidation, MissingOptions) {
+  ClangTidyOptions Options;
+  ClangTidyContext Context(std::make_unique(
+  ClangTidyGlobalOptions(), Options));
+  TestCheck TestCheck();
+  CHECK_ERROR(TestCheck.getLocal("Opt"), MissingOptionError,
+  "option not found 'test.Opt'");
+  EXPECT_EQ(TestCheck.getLocal("Opt", "Unknown"), "Unknown");
+}
+
+TEST(CheckOptionsValidation, ValidIntOptions) {
+  ClangTidyOptions Options;
+  auto  = Options.CheckOptions;
+  CheckOptions["test.IntExpected1"] = "1";
+  CheckOptions["test.IntExpected2"] = "1WithMore";
+  CheckOptions["test.IntExpected3"] = "NoInt";
+  CheckOptions["GlobalIntExpected1"] = "1";
+  CheckOptions["GlobalIntExpected2"] = "NoInt";
+  CheckOptions["test.DefaultedIntInvalid"] = "NoInt";
+  CheckOptions["GlobalIntInvalid"] = "NoInt";
+  CheckOptions["test.BoolITrueValue"] = "1";
+  CheckOptions["test.BoolIFalseValue"] = "0";
+  CheckOptions["test.BoolTrueValue"] = "true";
+  CheckOptions["test.BoolFalseValue"] = "false";
+  CheckOptions["test.BoolUnparseable"] = "Nothing";
+  CheckOptions["test.BoolCaseMismatch"] = "True";
+
+  ClangTidyContext Context(std::make_unique(
+  ClangTidyGlobalOptions(), Options));
+  TestCheck TestCheck();
+
+#define CHECK_ERROR_INT(Name, Expected)\
+  CHECK_ERROR(Name, UnparseableIntegerOptionError, Expected)
+
+  CHECK_VAL(TestCheck.getIntLocal("IntExpected1"), 1);
+  CHECK_VAL(TestCheck.getIntGlobal("GlobalIntExpected1"), 1);
+  CHECK_ERROR_INT(TestCheck.getIntLocal("IntExpected2"),
+  "invalid configuration value '1WithMore' for option "
+  "'test.IntExpected2'; expected an integer value");
+  CHECK_ERROR_INT(TestCheck.getIntLocal("IntExpected3"),
+  "invalid configuration value 'NoInt' for option "
+  

[PATCH] D77440: [Hexagon] Update include paths for linux/musl

[Sid Manning via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGaed2fdb1671c: [Hexagon] Update paths for linux/musl 
(authored by sidneym).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77440

Files:
  clang/lib/Driver/ToolChains/Hexagon.cpp
  clang/lib/Driver/ToolChains/Hexagon.h
  
clang/test/Driver/Inputs/hexagon_tree/Tools/target/hexagon/include/c++/v1/readme
  clang/test/Driver/hexagon-toolchain-elf.c
  clang/test/Driver/hexagon-toolchain-linux.c

Index: clang/test/Driver/hexagon-toolchain-linux.c
===
--- /dev/null
+++ clang/test/Driver/hexagon-toolchain-linux.c
@@ -0,0 +1,101 @@
+// -
+// Passing --musl
+// -
+// RUN: %clang -### -target hexagon-unknown-linux-musl \
+// RUN:   -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
+// RUN:   -mcpu=hexagonv60 \
+// RUN:   -fuse-ld=lld \
+// RUN:   --sysroot=%S/Inputs/basic_linux_libcxx_tree \
+// RUN:   %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK000 %s
+// CHECK000-NOT:  {{.*}}basic_linux_libcxx_tree{{/|}}usr{{/|}}lib{{/|}}crti.o
+// CHECK000:  "-dynamic-linker={{/|}}lib{{/|}}ld-musl-hexagon.so.1"
+// CHECK000:  "{{.*}}basic_linux_libcxx_tree{{/|}}usr{{/|}}lib{{/|}}crt1.o"
+// CHECK000:  "-lclang_rt.builtins-hexagon" "-lc"
+// -
+// Passing --musl --shared
+// -
+// RUN: %clang -### -target hexagon-unknown-linux-musl \
+// RUN:   -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
+// RUN:   -mcpu=hexagonv60 \
+// RUN:   --sysroot=%S/Inputs/basic_linux_libcxx_tree -shared \
+// RUN:   %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK001 %s
+// CHECK001-NOT:-dynamic-linker={{/|}}lib{{/|}}ld-musl-hexagon.so.1
+// CHECK001:"{{.*}}basic_linux_libcxx_tree{{/|}}usr{{/|}}lib{{/|}}crti.o"
+// CHECK001:"-lclang_rt.builtins-hexagon" "-lc"
+// CHECK001-NOT:{{.*}}basic_linux_libcxx_tree{{/|}}usr{{/|}}lib{{/|}}crt1.o
+// -
+// Passing --musl -nostdlib
+// -
+// RUN: %clang -### -target hexagon-unknown-linux-musl \
+// RUN:   -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
+// RUN:   -mcpu=hexagonv60 \
+// RUN:   --sysroot=%S/Inputs/basic_linux_libcxx_tree -nostdlib \
+// RUN:   %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK002 %s
+// CHECK002:   "-dynamic-linker={{/|}}lib{{/|}}ld-musl-hexagon.so.1"
+// CHECK002-NOT:   {{.*}}basic_linux_libcxx_tree{{/|}}usr{{/|}}lib{{/|}}crti.o
+// CHECK002-NOT:   {{.*}}basic_linux_libcxx_tree{{/|}}usr{{/|}}lib{{/|}}crt1.o
+// CHECK002-NOT:   -lclang_rt.builtins-hexagon
+// CHECK002-NOT:   -lc
+// -
+// Passing --musl -nostartfiles
+// -
+// RUN: %clang -### -target hexagon-unknown-linux-musl \
+// RUN:   -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
+// RUN:   -mcpu=hexagonv60 \
+// RUN:   --sysroot=%S/Inputs/basic_linux_libcxx_tree -nostartfiles \
+// RUN:   %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK003 %s
+// CHECK003:   "-dynamic-linker={{/|}}lib{{/|}}ld-musl-hexagon.so.1"
+// CHECK003-NOT:   {{.*}}basic_linux_libcxx_tree{{/|}}usr{{/|}}lib{{/|}}Scrt1.o
+// CHECK003-NOT:   {{.*}}basic_linux_libcxx_tree{{/|}}usr{{/|}}lib{{/|}}crt1.o
+// CHECK003:   "-lclang_rt.builtins-hexagon" "-lc"
+// -
+// Passing --musl -nodefaultlibs
+// -
+// RUN: %clang -### -target hexagon-unknown-linux-musl \
+// RUN:   -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
+// RUN:   -mcpu=hexagonv60 \
+// RUN:   --sysroot=%S/Inputs/basic_linux_libcxx_tree -nodefaultlibs \
+// RUN:   %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK004 %s
+// CHECK004:   "-dynamic-linker={{/|}}lib{{/|}}ld-musl-hexagon.so.1"
+// CHECK004:   "{{.*}}basic_linux_libcxx_tree{{/|}}usr{{/|}}lib{{/|}}crt1.o"
+// CHECK004-NOT:   -lclang_rt.builtins-hexagon
+// CHECK004-NOT:   -lc
+// -
+// Not Passing -fno-use-init-array when musl is selected
+// -
+// RUN: %clang -### 

[PATCH] D77669: [clangd] Run TUStatus test in sync mode to make output deterministic

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Currently it doesn't matter because we run PreambleThread in sync mode.
Once we start running it in async, test order will become non-deterministic.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77669

Files:
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp


Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -8,6 +8,7 @@
 
 #include "Annotations.h"
 #include "Cancellation.h"
+#include "ClangdServer.h"
 #include "Context.h"
 #include "Diagnostics.h"
 #include "Matchers.h"
@@ -844,7 +845,9 @@
   } CaptureTUStatus;
   MockFSProvider FS;
   MockCompilationDatabase CDB;
-  ClangdServer Server(CDB, FS, ClangdServer::optsForTest(), );
+  auto Opts = ClangdServer::optsForTest();
+  Opts.AsyncThreadsCount = 0;
+  ClangdServer Server(CDB, FS, Opts, );
   Annotations Code("int m^ain () {}");
 
   // We schedule the following tasks in the queue:
@@ -858,27 +861,12 @@
 
   ASSERT_TRUE(Server.blockUntilIdleForTest());
 
-  EXPECT_THAT(CaptureTUStatus.allStatus(),
-  ElementsAre(
-  // Everything starts with ASTWorker starting to execute an
-  // update
-  TUState(PreambleAction::Idle, ASTAction::RunningAction),
-  // We build the preamble
-  TUState(PreambleAction::Building, ASTAction::RunningAction),
-  // We built the preamble, and issued ast built on ASTWorker
-  // thread. Preambleworker goes idle afterwards.
-  TUState(PreambleAction::Idle, ASTAction::RunningAction),
-  // Start task for building the ast, as a result of building
-  // preamble, on astworker thread.
-  TUState(PreambleAction::Idle, ASTAction::RunningAction),
-  // AST build starts.
-  TUState(PreambleAction::Idle, ASTAction::Building),
-  // AST built finished successfully
-  TUState(PreambleAction::Idle, ASTAction::Building),
-  // Running go to def
-  TUState(PreambleAction::Idle, ASTAction::RunningAction),
-  // ASTWorker goes idle.
-  TUState(PreambleAction::Idle, ASTAction::Idle)));
+  EXPECT_THAT(
+  CaptureTUStatus.allStatus(),
+  ElementsAre(TUState(PreambleAction::Building, ASTAction::Idle),
+  TUState(PreambleAction::Building, ASTAction::Building),
+  TUState(PreambleAction::Building, ASTAction::Building),
+  TUState(PreambleAction::Idle, ASTAction::Building)));
 }
 
 TEST_F(TUSchedulerTests, CommandLineErrors) {


Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -8,6 +8,7 @@
 
 #include "Annotations.h"
 #include "Cancellation.h"
+#include "ClangdServer.h"
 #include "Context.h"
 #include "Diagnostics.h"
 #include "Matchers.h"
@@ -844,7 +845,9 @@
   } CaptureTUStatus;
   MockFSProvider FS;
   MockCompilationDatabase CDB;
-  ClangdServer Server(CDB, FS, ClangdServer::optsForTest(), );
+  auto Opts = ClangdServer::optsForTest();
+  Opts.AsyncThreadsCount = 0;
+  ClangdServer Server(CDB, FS, Opts, );
   Annotations Code("int m^ain () {}");
 
   // We schedule the following tasks in the queue:
@@ -858,27 +861,12 @@
 
   ASSERT_TRUE(Server.blockUntilIdleForTest());
 
-  EXPECT_THAT(CaptureTUStatus.allStatus(),
-  ElementsAre(
-  // Everything starts with ASTWorker starting to execute an
-  // update
-  TUState(PreambleAction::Idle, ASTAction::RunningAction),
-  // We build the preamble
-  TUState(PreambleAction::Building, ASTAction::RunningAction),
-  // We built the preamble, and issued ast built on ASTWorker
-  // thread. Preambleworker goes idle afterwards.
-  TUState(PreambleAction::Idle, ASTAction::RunningAction),
-  // Start task for building the ast, as a result of building
-  // preamble, on astworker thread.
-  TUState(PreambleAction::Idle, ASTAction::RunningAction),
-  // AST build starts.
-  TUState(PreambleAction::Idle, ASTAction::Building),
-  // AST built finished successfully
-  TUState(PreambleAction::Idle, ASTAction::Building),
-  // Running go to def
- 

[PATCH] D74467: [analyzer] Teach scan-build how to rebuild index.html without analyzing.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ closed this revision.
NoQ added a comment.
Herald added a subscriber: ASDenysPetrov.

Re-landed as rG0c27fd82e1e6 
.

I sorted the list of HTML files before deduplicating and the buildbots seem 
silent for now.

Also fxd the uninitialized variable `$Clang` use when `--use-analyzer=` is not 
specified; here's what it was used for:

F11684939: Screen Shot 2020-04-07 at 9.49.46 PM.png 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74467



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

[PATCH] D77586: Allow parameter names to be elided in a function definition in C

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

In D77586#1967052 , @erik.pilkington 
wrote:

> > This also adds the same feature to ObjC blocks under the assumption that 
> > ObjC wishes to follow the C standard in this regard.
>
> SGTM - thats generally been the idea for blocks.


Note that `-fblocks` is a C extension, not just Objective-C.  (SGTM to me too.)


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

https://reviews.llvm.org/D77586



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