[PATCH] D84147: Use typedef to represent storage type in FPOption and FPOptionsOverride

[Serge Pavlov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG21cd7b72a3d4: Use typedef to represent storage type in 
FPOption and FPOptionsOverride (authored by sepavloff).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84147

Files:
  clang/include/clang/Basic/FPOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTReader.h

Index: clang/include/clang/Serialization/ASTReader.h
===
--- clang/include/clang/Serialization/ASTReader.h
+++ clang/include/clang/Serialization/ASTReader.h
@@ -858,10 +858,10 @@
   SourceLocation PointersToMembersPragmaLocation;
 
   /// The pragma float_control state.
-  Optional FpPragmaCurrentValue;
+  Optional FpPragmaCurrentValue;
   SourceLocation FpPragmaCurrentLocation;
   struct FpPragmaStackEntry {
-unsigned Value;
+FPOptionsOverride::storage_type Value;
 SourceLocation Location;
 SourceLocation PushLocation;
 StringRef SlotLabel;
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -587,7 +587,7 @@
   PragmaStack CodeSegStack;
 
   // This stack tracks the current state of Sema.CurFPFeatures.
-  PragmaStack FpPragmaStack;
+  PragmaStack FpPragmaStack;
   FPOptionsOverride CurFPFeatureOverrides() {
 FPOptionsOverride result;
 if (!FpPragmaStack.hasValue()) {
@@ -1405,12 +1405,12 @@
   S.CurFPFeatures = OldFPFeaturesState;
   S.FpPragmaStack.CurrentValue = OldOverrides;
 }
-unsigned getOverrides() { return OldOverrides; }
+FPOptionsOverride::storage_type getOverrides() { return OldOverrides; }
 
   private:
 Sema& S;
 FPOptions OldFPFeaturesState;
-unsigned OldOverrides;
+FPOptionsOverride::storage_type OldOverrides;
   };
 
   void addImplicitTypedef(StringRef Name, QualType T);
Index: clang/include/clang/Basic/LangOptions.h
===
--- clang/include/clang/Basic/LangOptions.h
+++ clang/include/clang/Basic/LangOptions.h
@@ -374,6 +374,8 @@
 
   using RoundingMode = llvm::RoundingMode;
 
+  static constexpr unsigned StorageBitSize = 8 * sizeof(storage_type);
+
   // Define a fake option named "First" so that we have a PREVIOUS even for the
   // real first option.
   static constexpr storage_type FirstShift = 0, FirstWidth = 0;
@@ -385,6 +387,12 @@
  << NAME##Shift;
 #include "clang/Basic/FPOptions.def"
 
+  static constexpr storage_type TotalWidth = 0
+#define OPTION(NAME, TYPE, WIDTH, PREVIOUS) +WIDTH
+#include "clang/Basic/FPOptions.def"
+  ;
+  static_assert(TotalWidth <= StorageBitSize, "Too short type for FPOptions");
+
 private:
   storage_type Value;
 
@@ -402,8 +410,8 @@
 setFPContractMode(LO.getDefaultFPContractMode());
 setRoundingMode(LO.getFPRoundingMode());
 setFPExceptionMode(LO.getFPExceptionMode());
-setAllowFEnvAccess(LangOptions::FPM_Off),
-setAllowFPReassociate(LO.AllowFPReassoc);
+setAllowFEnvAccess(LangOptions::FPM_Off);
+setAllowFPReassociate(LO.AllowFPReassoc);
 setNoHonorNaNs(LO.NoHonorNaNs);
 setNoHonorInfs(LO.NoHonorInfs);
 setNoSignedZero(LO.NoSignedZero);
@@ -453,18 +461,45 @@
   LLVM_DUMP_METHOD void dump();
 };
 
-/// The FPOptions override type is value of the new FPOptions
-///  plus a mask showing which fields are actually set in it:
+/// Represents difference between two FPOptions values.
+///
+/// The effect of language constructs changing the set of floating point options
+/// is usually a change of some FP properties while leaving others intact. This
+/// class describes such changes by keeping information about what FP options
+/// are overridden.
+///
+/// The integral set of FP options, described by the class FPOptions, may be
+/// represented as a default FP option set, defined by language standard and
+/// command line options, with the overrides introduced by pragmas.
+///
+/// The is implemented as a value of the new FPOptions plus a mask showing which
+/// fields are actually set in it.
 class FPOptionsOverride {
   FPOptions Options;
   FPOptions::storage_type OverrideMask = 0;
 
 public:
   using RoundingMode = llvm::RoundingMode;
+
+  /// The type suitable for storing values of FPOptionsOverride. Must be twice
+  /// as wide as bit size of FPOption.
+  using storage_type = uint32_t;
+  static_assert(sizeof(storage_type) >= 2 * sizeof(FPOptions::storage_type),
+"Too short type for FPOptionsOverride");
+
+  /// Bit mask selecting bits of OverrideMask in serialized representation of
+  /// FPOptionsOverride.
+  static constexpr storage_type OverrideMaskBits =
+  (static_cast(1) << FPOptions::StorageBitSize) - 1;
+
   FPOpti

[clang] 21cd7b7 - Use typedef to represent storage type in FPOption and FPOptionsOverride

[Serge Pavlov via cfe-commits]

Author: Serge Pavlov
Date: 2020-07-21T14:35:50+07:00
New Revision: 21cd7b72a3d4d1d5efaadc09533655090b678e57

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

LOG: Use typedef to represent storage type in FPOption and FPOptionsOverride

Size of FPOption is now 14 bit, which is closed to the current limit
of 16 bits. Adding new properties to FPOption would require change of
the types, which represent storage of FPOption and FPOptionsOverride.
To facilitate this change, the storage types were changed from standard
integer types to typedefs defined inside the proper classes. Checks for
size of these storage types were added.

No functional changes intended.

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

Added: 


Modified: 
clang/include/clang/Basic/FPOptions.def
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Sema/Sema.h
clang/include/clang/Serialization/ASTReader.h

Removed: 




diff  --git a/clang/include/clang/Basic/FPOptions.def 
b/clang/include/clang/Basic/FPOptions.def
index 6b6789b8ecc8..1733689775d4 100644
--- a/clang/include/clang/Basic/FPOptions.def
+++ b/clang/include/clang/Basic/FPOptions.def
@@ -7,7 +7,7 @@
 
//===--===//
 
 // This file defines the Floating Point language options. Users of this file
-//  must define the FPOPT macro to make use of this information.
+//  must define the OPTION macro to make use of this information.
 #ifndef OPTION
 #  error Define the OPTION macro to handle floating point language options
 #endif

diff  --git a/clang/include/clang/Basic/LangOptions.h 
b/clang/include/clang/Basic/LangOptions.h
index a9213b7d8668..31e8af4589b4 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -374,6 +374,8 @@ class FPOptions {
 
   using RoundingMode = llvm::RoundingMode;
 
+  static constexpr unsigned StorageBitSize = 8 * sizeof(storage_type);
+
   // Define a fake option named "First" so that we have a PREVIOUS even for the
   // real first option.
   static constexpr storage_type FirstShift = 0, FirstWidth = 0;
@@ -385,6 +387,12 @@ class FPOptions {
  << NAME##Shift;
 #include "clang/Basic/FPOptions.def"
 
+  static constexpr storage_type TotalWidth = 0
+#define OPTION(NAME, TYPE, WIDTH, PREVIOUS) +WIDTH
+#include "clang/Basic/FPOptions.def"
+  ;
+  static_assert(TotalWidth <= StorageBitSize, "Too short type for FPOptions");
+
 private:
   storage_type Value;
 
@@ -402,8 +410,8 @@ class FPOptions {
 setFPContractMode(LO.getDefaultFPContractMode());
 setRoundingMode(LO.getFPRoundingMode());
 setFPExceptionMode(LO.getFPExceptionMode());
-setAllowFEnvAccess(LangOptions::FPM_Off),
-setAllowFPReassociate(LO.AllowFPReassoc);
+setAllowFEnvAccess(LangOptions::FPM_Off);
+setAllowFPReassociate(LO.AllowFPReassoc);
 setNoHonorNaNs(LO.NoHonorNaNs);
 setNoHonorInfs(LO.NoHonorInfs);
 setNoSignedZero(LO.NoSignedZero);
@@ -453,18 +461,45 @@ class FPOptions {
   LLVM_DUMP_METHOD void dump();
 };
 
-/// The FPOptions override type is value of the new FPOptions
-///  plus a mask showing which fields are actually set in it:
+/// Represents 
diff erence between two FPOptions values.
+///
+/// The effect of language constructs changing the set of floating point 
options
+/// is usually a change of some FP properties while leaving others intact. This
+/// class describes such changes by keeping information about what FP options
+/// are overridden.
+///
+/// The integral set of FP options, described by the class FPOptions, may be
+/// represented as a default FP option set, defined by language standard and
+/// command line options, with the overrides introduced by pragmas.
+///
+/// The is implemented as a value of the new FPOptions plus a mask showing 
which
+/// fields are actually set in it.
 class FPOptionsOverride {
   FPOptions Options;
   FPOptions::storage_type OverrideMask = 0;
 
 public:
   using RoundingMode = llvm::RoundingMode;
+
+  /// The type suitable for storing values of FPOptionsOverride. Must be twice
+  /// as wide as bit size of FPOption.
+  using storage_type = uint32_t;
+  static_assert(sizeof(storage_type) >= 2 * sizeof(FPOptions::storage_type),
+"Too short type for FPOptionsOverride");
+
+  /// Bit mask selecting bits of OverrideMask in serialized representation of
+  /// FPOptionsOverride.
+  static constexpr storage_type OverrideMaskBits =
+  (static_cast(1) << FPOptions::StorageBitSize) - 1;
+
   FPOptionsOverride() {}
+  FPOptionsOverride(FPOptions::storage_type Value, FPOptions::storage_type 
Mask)
+  : Options(Value), OverrideMask(Mask) {}
+  FPOptionsOverride(const LangOption

[clang] 7af852d - [AST][RecoveryExpr] Preserve the invalid "undef_var" initializer.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2020-07-21T09:38:09+02:00
New Revision: 7af852dcbff9a4d5034e6deaadb7b630c548c6fa

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

LOG: [AST][RecoveryExpr] Preserve the invalid "undef_var" initializer.

And don't invalidate the VarDecl if the type is known.

Reviewed By: sammccall

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

Added: 


Modified: 
clang/lib/Sema/SemaDecl.cpp
clang/test/AST/ast-dump-recovery.cpp
clang/test/SemaCXX/typo-correction-delayed.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 3413a420430c..406c9c712564 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -12039,7 +12039,7 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr 
*Init, bool DirectInit) {
 // Try to correct any TypoExprs in the initialization arguments.
 for (size_t Idx = 0; Idx < Args.size(); ++Idx) {
   ExprResult Res = CorrectDelayedTyposInExpr(
-  Args[Idx], VDecl, /*RecoverUncorrectedTypos=*/false,
+  Args[Idx], VDecl, /*RecoverUncorrectedTypos=*/true,
   [this, Entity, Kind](Expr *E) {
 InitializationSequence Init(*this, Entity, Kind, MultiExprArg(E));
 return Init.Failed() ? ExprError() : E;

diff  --git a/clang/test/AST/ast-dump-recovery.cpp 
b/clang/test/AST/ast-dump-recovery.cpp
index 4d11bb29d5bf..fd7c923b7e51 100644
--- a/clang/test/AST/ast-dump-recovery.cpp
+++ b/clang/test/AST/ast-dump-recovery.cpp
@@ -24,8 +24,11 @@ void test_invalid_call(int s) {
   // CHECK-NEXT:   |-UnresolvedLookupExpr {{.*}} 'some_func'
   // CHECK-NEXT:   `-RecoveryExpr {{.*}} contains-errors
   s = some_func(undef1);
-  // CHECK: `-VarDecl {{.*}} invalid var 'int'
-  // FIXME: preserve the broken call.
+
+  // CHECK: VarDecl {{.*}} var 'int'
+  // CHECK-NEXT: `-CallExpr {{.*}} '' contains-errors
+  // CHECK-NEXT:   |-UnresolvedLookupExpr {{.*}} 'some_func'
+  // CHECK-NEXT:   `-RecoveryExpr {{.*}} contains-errors
   int var = some_func(undef1);
 }
 
@@ -178,9 +181,15 @@ void InvalidInitalizer(int x) {
   // CHECK-NEXT: `-UnresolvedLookupExpr {{.*}} 'invalid'
   Bar b6 = Bar{invalid()};
 
-  // CHECK: `-RecoveryExpr {{.*}} 'Bar' contains-errors
+  // CHECK: RecoveryExpr {{.*}} 'Bar' contains-errors
   // CHECK-NEXT:  `-IntegerLiteral {{.*}} 'int' 1
   Bar(1);
+
+  // CHECK: `-VarDecl {{.*}} var1
+  // CHECK-NEXT: `-BinaryOperator {{.*}} '' contains-errors
+  // CHECK-NEXT:   |-RecoveryExpr {{.*}} '' contains-errors
+  // CHECK-NEXT:   `-IntegerLiteral {{.*}} 'int' 1
+  int var1 = undef + 1;
 }
 void InitializerForAuto() {
   // CHECK: `-VarDecl {{.*}} invalid a 'auto'

diff  --git a/clang/test/SemaCXX/typo-correction-delayed.cpp 
b/clang/test/SemaCXX/typo-correction-delayed.cpp
index 610d43971381..66d19daf66fd 100644
--- a/clang/test/SemaCXX/typo-correction-delayed.cpp
+++ b/clang/test/SemaCXX/typo-correction-delayed.cpp
@@ -149,7 +149,8 @@ void test() {
 }
 
 namespace PR21905 {
-int (*a) () = (void)Z;  // expected-error-re {{use of undeclared identifier 
'Z'{{$
+int (*a)() = (void)Z; // expected-error-re {{use of undeclared identifier 
'Z'{{$ \
+  // expected-error {{cannot initialize a variable of type 
'int (*)()' with an rvalue of type 'void'}}
 }
 
 namespace PR21947 {



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

[Differential] D81395: [AST][RecoveryExpr] Preserve the invalid "undef_var" initializer.

[Haojian Wu via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7af852dcbff9: [AST][RecoveryExpr] Preserve the invalid 
"undef_var" initializer. (authored by hokein).

Changed prior to commit:
  https://reviews.llvm.org/D81395?vs=271928&id=279039#toc

Repository:
  rG LLVM Github Monorepo

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


  https://reviews.llvm.org/D81395

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/AST/ast-dump-recovery.cpp
  clang/test/SemaCXX/typo-correction-delayed.cpp



Index: clang/test/SemaCXX/typo-correction-delayed.cpp
===
--- clang/test/SemaCXX/typo-correction-delayed.cpp
+++ clang/test/SemaCXX/typo-correction-delayed.cpp
@@ -149,7 +149,8 @@
 }
 
 namespace PR21905 {
-int (*a) () = (void)Z;  // expected-error-re {{use of undeclared identifier 
'Z'{{$
+int (*a)() = (void)Z; // expected-error-re {{use of undeclared identifier 
'Z'{{$ \
+  // expected-error {{cannot initialize a variable of type 
'int (*)()' with an rvalue of type 'void'}}
 }
 
 namespace PR21947 {
Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -24,8 +24,11 @@
   // CHECK-NEXT:   |-UnresolvedLookupExpr {{.*}} 'some_func'
   // CHECK-NEXT:   `-RecoveryExpr {{.*}} contains-errors
   s = some_func(undef1);
-  // CHECK: `-VarDecl {{.*}} invalid var 'int'
-  // FIXME: preserve the broken call.
+
+  // CHECK: VarDecl {{.*}} var 'int'
+  // CHECK-NEXT: `-CallExpr {{.*}} '' contains-errors
+  // CHECK-NEXT:   |-UnresolvedLookupExpr {{.*}} 'some_func'
+  // CHECK-NEXT:   `-RecoveryExpr {{.*}} contains-errors
   int var = some_func(undef1);
 }
 
@@ -178,9 +181,15 @@
   // CHECK-NEXT: `-UnresolvedLookupExpr {{.*}} 'invalid'
   Bar b6 = Bar{invalid()};
 
-  // CHECK: `-RecoveryExpr {{.*}} 'Bar' contains-errors
+  // CHECK: RecoveryExpr {{.*}} 'Bar' contains-errors
   // CHECK-NEXT:  `-IntegerLiteral {{.*}} 'int' 1
   Bar(1);
+
+  // CHECK: `-VarDecl {{.*}} var1
+  // CHECK-NEXT: `-BinaryOperator {{.*}} '' contains-errors
+  // CHECK-NEXT:   |-RecoveryExpr {{.*}} '' contains-errors
+  // CHECK-NEXT:   `-IntegerLiteral {{.*}} 'int' 1
+  int var1 = undef + 1;
 }
 void InitializerForAuto() {
   // CHECK: `-VarDecl {{.*}} invalid a 'auto'
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12039,7 +12039,7 @@
 // Try to correct any TypoExprs in the initialization arguments.
 for (size_t Idx = 0; Idx < Args.size(); ++Idx) {
   ExprResult Res = CorrectDelayedTyposInExpr(
-  Args[Idx], VDecl, /*RecoverUncorrectedTypos=*/false,
+  Args[Idx], VDecl, /*RecoverUncorrectedTypos=*/true,
   [this, Entity, Kind](Expr *E) {
 InitializationSequence Init(*this, Entity, Kind, MultiExprArg(E));
 return Init.Failed() ? ExprError() : E;


Index: clang/test/SemaCXX/typo-correction-delayed.cpp
===
--- clang/test/SemaCXX/typo-correction-delayed.cpp
+++ clang/test/SemaCXX/typo-correction-delayed.cpp
@@ -149,7 +149,8 @@
 }
 
 namespace PR21905 {
-int (*a) () = (void)Z;  // expected-error-re {{use of undeclared identifier 'Z'{{$
+int (*a)() = (void)Z; // expected-error-re {{use of undeclared identifier 'Z'{{$ \
+  // expected-error {{cannot initialize a variable of type 'int (*)()' with an rvalue of type 'void'}}
 }
 
 namespace PR21947 {
Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -24,8 +24,11 @@
   // CHECK-NEXT:   |-UnresolvedLookupExpr {{.*}} 'some_func'
   // CHECK-NEXT:   `-RecoveryExpr {{.*}} contains-errors
   s = some_func(undef1);
-  // CHECK: `-VarDecl {{.*}} invalid var 'int'
-  // FIXME: preserve the broken call.
+
+  // CHECK: VarDecl {{.*}} var 'int'
+  // CHECK-NEXT: `-CallExpr {{.*}} '' contains-errors
+  // CHECK-NEXT:   |-UnresolvedLookupExpr {{.*}} 'some_func'
+  // CHECK-NEXT:   `-RecoveryExpr {{.*}} contains-errors
   int var = some_func(undef1);
 }
 
@@ -178,9 +181,15 @@
   // CHECK-NEXT: `-UnresolvedLookupExpr {{.*}} 'invalid'
   Bar b6 = Bar{invalid()};
 
-  // CHECK: `-RecoveryExpr {{.*}} 'Bar' contains-errors
+  // CHECK: RecoveryExpr {{.*}} 'Bar' contains-errors
   // CHECK-NEXT:  `-IntegerLiteral {{.*}} 'int' 1
   Bar(1);
+
+  // CHECK: `-VarDecl {{.*}} var1
+  // CHECK-NEXT: `-BinaryOperator {{.*}} '' contains-errors
+  // CHECK-NEXT:   |-RecoveryExpr {{.*}} '' contains-errors
+  // CHECK-NEXT:   `-IntegerLiteral {{.*}} 'int' 1
+  int var1 = undef + 1;
 }
 void InitializerForAut

[PATCH] D81395: [AST][RecoveryExpr] Preserve the invalid "undef_var" initializer.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7af852dcbff9: [AST][RecoveryExpr] Preserve the invalid 
"undef_var" initializer. (authored by hokein).

Changed prior to commit:
  https://reviews.llvm.org/D81395?vs=271928&id=279436#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81395

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/AST/ast-dump-recovery.cpp
  clang/test/SemaCXX/typo-correction-delayed.cpp


Index: clang/test/SemaCXX/typo-correction-delayed.cpp
===
--- clang/test/SemaCXX/typo-correction-delayed.cpp
+++ clang/test/SemaCXX/typo-correction-delayed.cpp
@@ -149,7 +149,8 @@
 }
 
 namespace PR21905 {
-int (*a) () = (void)Z;  // expected-error-re {{use of undeclared identifier 
'Z'{{$
+int (*a)() = (void)Z; // expected-error-re {{use of undeclared identifier 
'Z'{{$ \
+  // expected-error {{cannot initialize a variable of type 
'int (*)()' with an rvalue of type 'void'}}
 }
 
 namespace PR21947 {
Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -24,8 +24,11 @@
   // CHECK-NEXT:   |-UnresolvedLookupExpr {{.*}} 'some_func'
   // CHECK-NEXT:   `-RecoveryExpr {{.*}} contains-errors
   s = some_func(undef1);
-  // CHECK: `-VarDecl {{.*}} invalid var 'int'
-  // FIXME: preserve the broken call.
+
+  // CHECK: VarDecl {{.*}} var 'int'
+  // CHECK-NEXT: `-CallExpr {{.*}} '' contains-errors
+  // CHECK-NEXT:   |-UnresolvedLookupExpr {{.*}} 'some_func'
+  // CHECK-NEXT:   `-RecoveryExpr {{.*}} contains-errors
   int var = some_func(undef1);
 }
 
@@ -178,9 +181,15 @@
   // CHECK-NEXT: `-UnresolvedLookupExpr {{.*}} 'invalid'
   Bar b6 = Bar{invalid()};
 
-  // CHECK: `-RecoveryExpr {{.*}} 'Bar' contains-errors
+  // CHECK: RecoveryExpr {{.*}} 'Bar' contains-errors
   // CHECK-NEXT:  `-IntegerLiteral {{.*}} 'int' 1
   Bar(1);
+
+  // CHECK: `-VarDecl {{.*}} var1
+  // CHECK-NEXT: `-BinaryOperator {{.*}} '' contains-errors
+  // CHECK-NEXT:   |-RecoveryExpr {{.*}} '' contains-errors
+  // CHECK-NEXT:   `-IntegerLiteral {{.*}} 'int' 1
+  int var1 = undef + 1;
 }
 void InitializerForAuto() {
   // CHECK: `-VarDecl {{.*}} invalid a 'auto'
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12039,7 +12039,7 @@
 // Try to correct any TypoExprs in the initialization arguments.
 for (size_t Idx = 0; Idx < Args.size(); ++Idx) {
   ExprResult Res = CorrectDelayedTyposInExpr(
-  Args[Idx], VDecl, /*RecoverUncorrectedTypos=*/false,
+  Args[Idx], VDecl, /*RecoverUncorrectedTypos=*/true,
   [this, Entity, Kind](Expr *E) {
 InitializationSequence Init(*this, Entity, Kind, MultiExprArg(E));
 return Init.Failed() ? ExprError() : E;


Index: clang/test/SemaCXX/typo-correction-delayed.cpp
===
--- clang/test/SemaCXX/typo-correction-delayed.cpp
+++ clang/test/SemaCXX/typo-correction-delayed.cpp
@@ -149,7 +149,8 @@
 }
 
 namespace PR21905 {
-int (*a) () = (void)Z;  // expected-error-re {{use of undeclared identifier 'Z'{{$
+int (*a)() = (void)Z; // expected-error-re {{use of undeclared identifier 'Z'{{$ \
+  // expected-error {{cannot initialize a variable of type 'int (*)()' with an rvalue of type 'void'}}
 }
 
 namespace PR21947 {
Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -24,8 +24,11 @@
   // CHECK-NEXT:   |-UnresolvedLookupExpr {{.*}} 'some_func'
   // CHECK-NEXT:   `-RecoveryExpr {{.*}} contains-errors
   s = some_func(undef1);
-  // CHECK: `-VarDecl {{.*}} invalid var 'int'
-  // FIXME: preserve the broken call.
+
+  // CHECK: VarDecl {{.*}} var 'int'
+  // CHECK-NEXT: `-CallExpr {{.*}} '' contains-errors
+  // CHECK-NEXT:   |-UnresolvedLookupExpr {{.*}} 'some_func'
+  // CHECK-NEXT:   `-RecoveryExpr {{.*}} contains-errors
   int var = some_func(undef1);
 }
 
@@ -178,9 +181,15 @@
   // CHECK-NEXT: `-UnresolvedLookupExpr {{.*}} 'invalid'
   Bar b6 = Bar{invalid()};
 
-  // CHECK: `-RecoveryExpr {{.*}} 'Bar' contains-errors
+  // CHECK: RecoveryExpr {{.*}} 'Bar' contains-errors
   // CHECK-NEXT:  `-IntegerLiteral {{.*}} 'int' 1
   Bar(1);
+
+  // CHECK: `-VarDecl {{.*}} var1
+  // CHECK-NEXT: `-BinaryOperator {{.*}} '' contains-errors
+  // CHECK-NEXT:   |-RecoveryExpr {{.*}} '' contains-errors
+  // CHECK-NEXT:   `-IntegerLiteral {{.*}} 'int' 1
+  int var1 = undef + 1;
 }
 void InitializerForAuto() 

[clang-tools-extra] 566b498 - [clang] Set the error-bit for ill-formed semantic InitListExpr.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2020-07-21T09:39:44+02:00
New Revision: 566b49884d69f88147c1ca18fd4512f73a3c15e3

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

LOG: [clang] Set the error-bit for ill-formed semantic InitListExpr.

When a semantic checking fails on a syntactic InitListExpr, we will
get an ill-formed semantic InitListExpr (e.g. some inits are nullptr),
using this semantic InitListExpr in clang (without setting the err-bits) is 
crashy.

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

Added: 


Modified: 
clang-tools-extra/clangd/unittests/HoverTests.cpp
clang/include/clang/AST/DependenceFlags.h
clang/include/clang/AST/Expr.h
clang/lib/AST/ComputeDependence.cpp
clang/lib/Sema/SemaInit.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp 
b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 19ab6d63947b..636e5d99be52 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -957,6 +957,16 @@ TEST(Hover, NoHover) {
   template  void foo() {
 (void)[[size^of]](T);
   })cpp",
+  R"cpp(// should not crash on invalid semantic form of init-list-expr.
+/*error-ok*/
+struct Foo {
+  int xyz = 0;
+};
+class Bar {};
+constexpr Foo s = ^{
+  .xyz = Bar(),
+};
+  )cpp",
   // literals
   "auto x = t^rue;",
   "auto x = '^A';",

diff  --git a/clang/include/clang/AST/DependenceFlags.h 
b/clang/include/clang/AST/DependenceFlags.h
index 14a7ffaecb2b..ca96b65574bd 100644
--- a/clang/include/clang/AST/DependenceFlags.h
+++ b/clang/include/clang/AST/DependenceFlags.h
@@ -41,6 +41,7 @@ struct ExprDependenceScope {
 TypeInstantiation = Type | Instantiation,
 ValueInstantiation = Value | Instantiation,
 TypeValueInstantiation = Type | Value | Instantiation,
+ErrorDependent = Error | ValueInstantiation,
 
 LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/Error)
   };

diff  --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index c13b97119285..ea8f688452eb 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -4690,6 +4690,13 @@ class InitListExpr : public Expr {
   setDependence(getDependence() | expr->getDependence());
   }
 
+  /// Mark the semantic form of the InitListExpr as error when the semantic
+  /// analysis fails.
+  void markError() {
+assert(isSemanticForm());
+setDependence(getDependence() | ExprDependence::ErrorDependent);
+  }
+
   /// Reserve space for some number of initializers.
   void reserveInits(const ASTContext &C, unsigned NumInits);
 

diff  --git a/clang/lib/AST/ComputeDependence.cpp 
b/clang/lib/AST/ComputeDependence.cpp
index 2333993dbeb4..c3a209026662 100644
--- a/clang/lib/AST/ComputeDependence.cpp
+++ b/clang/lib/AST/ComputeDependence.cpp
@@ -502,7 +502,7 @@ ExprDependence clang::computeDependence(RecoveryExpr *E) {
   // dependent type), or the type is known and dependent, or it has
   // type-dependent subexpressions.
   auto D = toExprDependence(E->getType()->getDependence()) |
-   ExprDependence::ValueInstantiation | ExprDependence::Error;
+   ExprDependence::ErrorDependent;
   // FIXME: remove the type-dependent bit from subexpressions, if the
   // RecoveryExpr has a non-dependent type.
   for (auto *S : E->subExpressions())

diff  --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index eb07de65d266..e2f67e9fd59b 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -962,6 +962,8 @@ InitListChecker::InitListChecker(Sema &S, const 
InitializedEntity &Entity,
   FillInEmptyInitializations(Entity, FullyStructuredList,
  RequiresSecondPass, nullptr, 0);
   }
+  if (hadError && FullyStructuredList)
+FullyStructuredList->markError();
 }
 
 int InitListChecker::numArrayElements(QualType DeclType) {



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

[PATCH] D84140: [clang] Set the error-bit for ill-formed semantic InitListExpr.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG566b49884d69: [clang] Set the error-bit for ill-formed 
semantic InitListExpr. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84140

Files:
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang/include/clang/AST/DependenceFlags.h
  clang/include/clang/AST/Expr.h
  clang/lib/AST/ComputeDependence.cpp
  clang/lib/Sema/SemaInit.cpp


Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -962,6 +962,8 @@
   FillInEmptyInitializations(Entity, FullyStructuredList,
  RequiresSecondPass, nullptr, 0);
   }
+  if (hadError && FullyStructuredList)
+FullyStructuredList->markError();
 }
 
 int InitListChecker::numArrayElements(QualType DeclType) {
Index: clang/lib/AST/ComputeDependence.cpp
===
--- clang/lib/AST/ComputeDependence.cpp
+++ clang/lib/AST/ComputeDependence.cpp
@@ -502,7 +502,7 @@
   // dependent type), or the type is known and dependent, or it has
   // type-dependent subexpressions.
   auto D = toExprDependence(E->getType()->getDependence()) |
-   ExprDependence::ValueInstantiation | ExprDependence::Error;
+   ExprDependence::ErrorDependent;
   // FIXME: remove the type-dependent bit from subexpressions, if the
   // RecoveryExpr has a non-dependent type.
   for (auto *S : E->subExpressions())
Index: clang/include/clang/AST/Expr.h
===
--- clang/include/clang/AST/Expr.h
+++ clang/include/clang/AST/Expr.h
@@ -4690,6 +4690,13 @@
   setDependence(getDependence() | expr->getDependence());
   }
 
+  /// Mark the semantic form of the InitListExpr as error when the semantic
+  /// analysis fails.
+  void markError() {
+assert(isSemanticForm());
+setDependence(getDependence() | ExprDependence::ErrorDependent);
+  }
+
   /// Reserve space for some number of initializers.
   void reserveInits(const ASTContext &C, unsigned NumInits);
 
Index: clang/include/clang/AST/DependenceFlags.h
===
--- clang/include/clang/AST/DependenceFlags.h
+++ clang/include/clang/AST/DependenceFlags.h
@@ -41,6 +41,7 @@
 TypeInstantiation = Type | Instantiation,
 ValueInstantiation = Value | Instantiation,
 TypeValueInstantiation = Type | Value | Instantiation,
+ErrorDependent = Error | ValueInstantiation,
 
 LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/Error)
   };
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -957,6 +957,16 @@
   template  void foo() {
 (void)[[size^of]](T);
   })cpp",
+  R"cpp(// should not crash on invalid semantic form of init-list-expr.
+/*error-ok*/
+struct Foo {
+  int xyz = 0;
+};
+class Bar {};
+constexpr Foo s = ^{
+  .xyz = Bar(),
+};
+  )cpp",
   // literals
   "auto x = t^rue;",
   "auto x = '^A';",


Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -962,6 +962,8 @@
   FillInEmptyInitializations(Entity, FullyStructuredList,
  RequiresSecondPass, nullptr, 0);
   }
+  if (hadError && FullyStructuredList)
+FullyStructuredList->markError();
 }
 
 int InitListChecker::numArrayElements(QualType DeclType) {
Index: clang/lib/AST/ComputeDependence.cpp
===
--- clang/lib/AST/ComputeDependence.cpp
+++ clang/lib/AST/ComputeDependence.cpp
@@ -502,7 +502,7 @@
   // dependent type), or the type is known and dependent, or it has
   // type-dependent subexpressions.
   auto D = toExprDependence(E->getType()->getDependence()) |
-   ExprDependence::ValueInstantiation | ExprDependence::Error;
+   ExprDependence::ErrorDependent;
   // FIXME: remove the type-dependent bit from subexpressions, if the
   // RecoveryExpr has a non-dependent type.
   for (auto *S : E->subExpressions())
Index: clang/include/clang/AST/Expr.h
===
--- clang/include/clang/AST/Expr.h
+++ clang/include/clang/AST/Expr.h
@@ -4690,6 +4690,13 @@
   setDependence(getDependence() | expr->getDependence());
   }
 
+  /// Mark the semantic form of the InitListExpr as error when the semantic
+  /// analysis fails.
+  void markError()

[PATCH] D83652: Merge some of the PCH object support with modular codegen

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

> @llunak Would you be able to test this on anything you've got?

No, but thinking more about this, I think dllexport specifically voids the 
possible problems I listed. If I'm getting it right, dllexport is used only for 
code in the current library, so codegen won't create code referencing private 
symbols in other libraries, and dllexport means the code will be emitted 
anyway, so no unnecessary code generating. So I'm actually fine with the patch 
as it is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83652



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

[PATCH] D80802: [RISCV] Upgrade RVV MC to v0.9.

[Hsiangkai Wang via Phabricator via cfe-commits]

HsiangKai added a comment.

In D80802#2155802 , @fpallares wrote:

> Apologies we didn't identify this earlier but with the change of the mask 
> register layout (`MLEN=1`) the overlap constraints involving the mask 
> register were modified:
>
> //**RVV-0.8, Section 5.3. Vector Masking:**//
>
> > The destination vector register group for a masked vector instruction can 
> > only overlap the source mask register (v0) when LMUL=1. Otherwise, an 
> > illegal instruction exception is raised.
>
> //**RVV-0.9, Section 5.3. Vector Masking:**//
>
> > The destination vector register group for a masked vector instruction 
> > cannot overlap the source mask register (v0), unless the destination vector 
> > register is being written with a mask value (e.g., comparisons) or the 
> > scalar result of a reduction. Otherwise, an illegal instruction exception 
> > is raised.
>
> The change was introduced in this commit 
> .
>
> From my understanding, with this change an instruction such as the following 
> should be rejected in RVV-0.9:
>
>   vadd.vv v0, v1, v2, v0.t
>
>
> Also note that `vadc`/`vsbc` already have this behaviour.


Indeed, I did not take care of the mask register constraint for instructions. I 
will handle it in the next revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80802



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

[PATCH] D83759: [clangd] Port lit tests to Windows

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D83759#2163622 , @ArcsinX wrote:

> What do you think of this patch? I'm not sure if Windows is important OS for 
> developers.


Windows is most certainly an important OS for developers and something the 
whole llvm project has been trying to catch up on where possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83759



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

[PATCH] D83759: [clangd] Port lit tests to Windows

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

hi, sorry for the late replay :/

We discussed this offline with Sam and are both concerned with the duplications 
it introduces, haven't really thought about a nice way to address this yet.

As you mentioned the background-index test in itself looks a lot better do, so 
we can land that one separately and think about the rest in the meantime.




Comment at: clang-tools-extra/clangd/test/background-index.test:18
 # Test that the index is writing files in the expected location.
 # RUN: ls %t/.cache/clangd/index/foo.cpp.*.idx
 # RUN: ls %t/sub_dir/.cache/clangd/index/foo.h.*.idx

are you sure these don't need changes on windows bots ? AFAICT this contains 
both backslashes(coming from %t) and forwards slashes, and command is `ls` 
which might not be available on a default windows prompt.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83759



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

[PATCH] D84176: [CMake][CTE] Add "check-clang-tools-..." targets to test only a particular Clang extra tool

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

@lebedev.ri If you have an idea on who's competent in accepting this change, 
please update the //reviewers// field, I'm in the dark here.

Also, should the commands be called `check-clang-tools-clang-tidy` or 
`check-clang-`**`extra`**`-clang-tidy` instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84176



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

[PATCH] D83826: [clangd] Don't send invalid messages from remote index

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 279447.
kbobyrev marked 5 inline comments as done.
kbobyrev added a comment.

Address post-LGTM comments, rebase on top of HEAD.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83826

Files:
  clang-tools-extra/clangd/index/remote/Client.cpp
  clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
  clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
  clang-tools-extra/clangd/index/remote/server/Server.cpp
  clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp

Index: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
===
--- clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
+++ clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
@@ -13,6 +13,7 @@
 #include "index/Serialization.h"
 #include "index/Symbol.h"
 #include "index/SymbolID.h"
+#include "index/SymbolLocation.h"
 #include "index/remote/marshalling/Marshalling.h"
 #include "clang/Index/IndexSymbol.h"
 #include "llvm/ADT/SmallString.h"
@@ -39,29 +40,35 @@
 TEST(RemoteMarshallingTest, URITranslation) {
   llvm::BumpPtrAllocator Arena;
   llvm::UniqueStringSaver Strings(Arena);
+  Marshaller ProtobufMarshaller(
+  testPath("remote/machine/projects/llvm-project/"),
+  testPath("home/my-projects/llvm-project/"));
   clangd::Ref Original;
   Original.Location.FileURI =
   testPathURI("remote/machine/projects/llvm-project/clang-tools-extra/"
   "clangd/unittests/remote/MarshallingTests.cpp",
   Strings);
-  auto Serialized =
-  toProtobuf(Original, testPath("remote/machine/projects/llvm-project/"));
-  EXPECT_EQ(Serialized.location().file_path(),
+  auto Serialized = ProtobufMarshaller.toProtobuf(Original);
+  EXPECT_TRUE(Serialized);
+  EXPECT_EQ(Serialized->location().file_path(),
 "clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp");
-  const std::string LocalIndexPrefix = testPath("local/machine/project/");
-  auto Deserialized = fromProtobuf(Serialized, &Strings,
-   testPath("home/my-projects/llvm-project/"));
+  auto Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
   EXPECT_TRUE(Deserialized);
-  EXPECT_EQ(Deserialized->Location.FileURI,
-testPathURI("home/my-projects/llvm-project/clang-tools-extra/"
-"clangd/unittests/remote/MarshallingTests.cpp",
-Strings));
+  EXPECT_STREQ(Deserialized->Location.FileURI,
+   testPathURI("home/my-projects/llvm-project/clang-tools-extra/"
+   "clangd/unittests/remote/MarshallingTests.cpp",
+   Strings));
+
+  // Can't have empty paths.
+  *Serialized->mutable_location()->mutable_file_path() = std::string();
+  Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
+  EXPECT_FALSE(Deserialized);
 
   clangd::Ref WithInvalidURI;
-  // Invalid URI results in empty path.
+  // Invalid URI results in serialization failure.
   WithInvalidURI.Location.FileURI = "This is not a URI";
-  Serialized = toProtobuf(WithInvalidURI, testPath("home/"));
-  EXPECT_EQ(Serialized.location().file_path(), "");
+  Serialized = ProtobufMarshaller.toProtobuf(WithInvalidURI);
+  EXPECT_FALSE(Serialized);
 
   // Can not use URIs with scheme different from "file".
   auto UnittestURI =
@@ -69,15 +76,15 @@
   EXPECT_TRUE(bool(UnittestURI));
   WithInvalidURI.Location.FileURI =
   Strings.save(UnittestURI->toString()).begin();
-  Serialized = toProtobuf(WithInvalidURI, testPath("project/lib/"));
-  EXPECT_EQ(Serialized.location().file_path(), "");
+  Serialized = ProtobufMarshaller.toProtobuf(WithInvalidURI);
+  EXPECT_FALSE(Serialized);
 
+  // Paths transmitted over the wire can not be absolute, they have to be
+  // relative.
   Ref WithAbsolutePath;
   *WithAbsolutePath.mutable_location()->mutable_file_path() =
   "/usr/local/user/home/HelloWorld.cpp";
-  Deserialized = fromProtobuf(WithAbsolutePath, &Strings, LocalIndexPrefix);
-  // Paths transmitted over the wire can not be absolute, they have to be
-  // relative.
+  Deserialized = ProtobufMarshaller.fromProtobuf(WithAbsolutePath);
   EXPECT_FALSE(Deserialized);
 }
 
@@ -128,48 +135,63 @@
 
   Sym.Flags = clangd::Symbol::SymbolFlag::IndexedForCodeCompletion;
 
+  Marshaller ProtobufMarshaller(testPath("home/"), testPath("home/"));
+
   // Check that symbols are exactly the same if the path to indexed project is
   // the same on indexing machine and the client.
-  auto Serialized = toProtobuf(Sym, testPath("home/"));
-  auto Deserialized = fromProtobuf(Serialized, &Strings, testPath("home/"));
+  auto Serialized = ProtobufMarshaller.toProtobuf(Sym);
+  EXPECT_TRUE(Serialized);
+  auto Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
   EXPECT_TRUE(Deserialized);
   EXPECT_E

[Differential] D83826: [clangd] Don't send invalid messages from remote index

[Kirill Bobyrev via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGeef162c330b0: [clangd] Don't send invalid messages from 
remote index (authored by kbobyrev).

Changed prior to commit:
  https://reviews.llvm.org/D83826?vs=278683&id=279041#toc

Repository:
  rG LLVM Github Monorepo

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


  https://reviews.llvm.org/D83826

Files:
  clang-tools-extra/clangd/index/remote/Client.cpp
  clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
  clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
  clang-tools-extra/clangd/index/remote/server/Server.cpp
  clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp

Index: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
===
--- clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
+++ clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
@@ -13,6 +13,7 @@
 #include "index/Serialization.h"
 #include "index/Symbol.h"
 #include "index/SymbolID.h"
+#include "index/SymbolLocation.h"
 #include "index/remote/marshalling/Marshalling.h"
 #include "clang/Index/IndexSymbol.h"
 #include "llvm/ADT/SmallString.h"
@@ -39,29 +40,35 @@
 TEST(RemoteMarshallingTest, URITranslation) {
   llvm::BumpPtrAllocator Arena;
   llvm::UniqueStringSaver Strings(Arena);
+  Marshaller ProtobufMarshaller(
+  testPath("remote/machine/projects/llvm-project/"),
+  testPath("home/my-projects/llvm-project/"));
   clangd::Ref Original;
   Original.Location.FileURI =
   testPathURI("remote/machine/projects/llvm-project/clang-tools-extra/"
   "clangd/unittests/remote/MarshallingTests.cpp",
   Strings);
-  auto Serialized =
-  toProtobuf(Original, testPath("remote/machine/projects/llvm-project/"));
-  EXPECT_EQ(Serialized.location().file_path(),
+  auto Serialized = ProtobufMarshaller.toProtobuf(Original);
+  EXPECT_TRUE(Serialized);
+  EXPECT_EQ(Serialized->location().file_path(),
 "clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp");
-  const std::string LocalIndexPrefix = testPath("local/machine/project/");
-  auto Deserialized = fromProtobuf(Serialized, &Strings,
-   testPath("home/my-projects/llvm-project/"));
+  auto Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
   EXPECT_TRUE(Deserialized);
-  EXPECT_EQ(Deserialized->Location.FileURI,
-testPathURI("home/my-projects/llvm-project/clang-tools-extra/"
-"clangd/unittests/remote/MarshallingTests.cpp",
-Strings));
+  EXPECT_STREQ(Deserialized->Location.FileURI,
+   testPathURI("home/my-projects/llvm-project/clang-tools-extra/"
+   "clangd/unittests/remote/MarshallingTests.cpp",
+   Strings));
+
+  // Can't have empty paths.
+  *Serialized->mutable_location()->mutable_file_path() = std::string();
+  Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
+  EXPECT_FALSE(Deserialized);
 
   clangd::Ref WithInvalidURI;
-  // Invalid URI results in empty path.
+  // Invalid URI results in serialization failure.
   WithInvalidURI.Location.FileURI = "This is not a URI";
-  Serialized = toProtobuf(WithInvalidURI, testPath("home/"));
-  EXPECT_EQ(Serialized.location().file_path(), "");
+  Serialized = ProtobufMarshaller.toProtobuf(WithInvalidURI);
+  EXPECT_FALSE(Serialized);
 
   // Can not use URIs with scheme different from "file".
   auto UnittestURI =
@@ -69,15 +76,15 @@
   EXPECT_TRUE(bool(UnittestURI));
   WithInvalidURI.Location.FileURI =
   Strings.save(UnittestURI->toString()).begin();
-  Serialized = toProtobuf(WithInvalidURI, testPath("project/lib/"));
-  EXPECT_EQ(Serialized.location().file_path(), "");
+  Serialized = ProtobufMarshaller.toProtobuf(WithInvalidURI);
+  EXPECT_FALSE(Serialized);
 
+  // Paths transmitted over the wire can not be absolute, they have to be
+  // relative.
   Ref WithAbsolutePath;
   *WithAbsolutePath.mutable_location()->mutable_file_path() =
   "/usr/local/user/home/HelloWorld.cpp";
-  Deserialized = fromProtobuf(WithAbsolutePath, &Strings, LocalIndexPrefix);
-  // Paths transmitted over the wire can not be absolute, they have to be
-  // relative.
+  Deserialized = ProtobufMarshaller.fromProtobuf(WithAbsolutePath);
   EXPECT_FALSE(Deserialized);
 }
 
@@ -128,48 +135,63 @@
 
   Sym.Flags = clangd::Symbol::SymbolFlag::IndexedForCodeCompletion;
 
+  Marshaller ProtobufMarshaller(testPath("home/"), testPath("home/"));
+
   // Check that symbols are exactly the same if the path to indexed project is
   // the same on indexing machine and the client.
-  auto Serialized = toProtobuf(Sym, testPath("home/"));
-  auto Deserialized = fromPr

[PATCH] D83826: [clangd] Don't send invalid messages from remote index

[Kirill Bobyrev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGeef162c330b0: [clangd] Don't send invalid messages from 
remote index (authored by kbobyrev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83826

Files:
  clang-tools-extra/clangd/index/remote/Client.cpp
  clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
  clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
  clang-tools-extra/clangd/index/remote/server/Server.cpp
  clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp

Index: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
===
--- clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
+++ clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
@@ -13,6 +13,7 @@
 #include "index/Serialization.h"
 #include "index/Symbol.h"
 #include "index/SymbolID.h"
+#include "index/SymbolLocation.h"
 #include "index/remote/marshalling/Marshalling.h"
 #include "clang/Index/IndexSymbol.h"
 #include "llvm/ADT/SmallString.h"
@@ -39,29 +40,35 @@
 TEST(RemoteMarshallingTest, URITranslation) {
   llvm::BumpPtrAllocator Arena;
   llvm::UniqueStringSaver Strings(Arena);
+  Marshaller ProtobufMarshaller(
+  testPath("remote/machine/projects/llvm-project/"),
+  testPath("home/my-projects/llvm-project/"));
   clangd::Ref Original;
   Original.Location.FileURI =
   testPathURI("remote/machine/projects/llvm-project/clang-tools-extra/"
   "clangd/unittests/remote/MarshallingTests.cpp",
   Strings);
-  auto Serialized =
-  toProtobuf(Original, testPath("remote/machine/projects/llvm-project/"));
-  EXPECT_EQ(Serialized.location().file_path(),
+  auto Serialized = ProtobufMarshaller.toProtobuf(Original);
+  EXPECT_TRUE(Serialized);
+  EXPECT_EQ(Serialized->location().file_path(),
 "clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp");
-  const std::string LocalIndexPrefix = testPath("local/machine/project/");
-  auto Deserialized = fromProtobuf(Serialized, &Strings,
-   testPath("home/my-projects/llvm-project/"));
+  auto Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
   EXPECT_TRUE(Deserialized);
-  EXPECT_EQ(Deserialized->Location.FileURI,
-testPathURI("home/my-projects/llvm-project/clang-tools-extra/"
-"clangd/unittests/remote/MarshallingTests.cpp",
-Strings));
+  EXPECT_STREQ(Deserialized->Location.FileURI,
+   testPathURI("home/my-projects/llvm-project/clang-tools-extra/"
+   "clangd/unittests/remote/MarshallingTests.cpp",
+   Strings));
+
+  // Can't have empty paths.
+  *Serialized->mutable_location()->mutable_file_path() = std::string();
+  Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
+  EXPECT_FALSE(Deserialized);
 
   clangd::Ref WithInvalidURI;
-  // Invalid URI results in empty path.
+  // Invalid URI results in serialization failure.
   WithInvalidURI.Location.FileURI = "This is not a URI";
-  Serialized = toProtobuf(WithInvalidURI, testPath("home/"));
-  EXPECT_EQ(Serialized.location().file_path(), "");
+  Serialized = ProtobufMarshaller.toProtobuf(WithInvalidURI);
+  EXPECT_FALSE(Serialized);
 
   // Can not use URIs with scheme different from "file".
   auto UnittestURI =
@@ -69,15 +76,15 @@
   EXPECT_TRUE(bool(UnittestURI));
   WithInvalidURI.Location.FileURI =
   Strings.save(UnittestURI->toString()).begin();
-  Serialized = toProtobuf(WithInvalidURI, testPath("project/lib/"));
-  EXPECT_EQ(Serialized.location().file_path(), "");
+  Serialized = ProtobufMarshaller.toProtobuf(WithInvalidURI);
+  EXPECT_FALSE(Serialized);
 
+  // Paths transmitted over the wire can not be absolute, they have to be
+  // relative.
   Ref WithAbsolutePath;
   *WithAbsolutePath.mutable_location()->mutable_file_path() =
   "/usr/local/user/home/HelloWorld.cpp";
-  Deserialized = fromProtobuf(WithAbsolutePath, &Strings, LocalIndexPrefix);
-  // Paths transmitted over the wire can not be absolute, they have to be
-  // relative.
+  Deserialized = ProtobufMarshaller.fromProtobuf(WithAbsolutePath);
   EXPECT_FALSE(Deserialized);
 }
 
@@ -128,48 +135,63 @@
 
   Sym.Flags = clangd::Symbol::SymbolFlag::IndexedForCodeCompletion;
 
+  Marshaller ProtobufMarshaller(testPath("home/"), testPath("home/"));
+
   // Check that symbols are exactly the same if the path to indexed project is
   // the same on indexing machine and the client.
-  auto Serialized = toProtobuf(Sym, testPath("home/"));
-  auto Deserialized = fromProtobuf(Serialized, &Strings, testPath("home/"));
+  auto Serialized = ProtobufMarshaller.toProtobuf(Sym);
+  EXPECT_TRUE(Serialized);
+  auto Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
   EXPECT_TRUE(Deserial

[clang-tools-extra] eef162c - [clangd] Don't send invalid messages from remote index

[Kirill Bobyrev via cfe-commits]

Author: Kirill Bobyrev
Date: 2020-07-21T11:05:03+02:00
New Revision: eef162c330b02fdc53ec33e5549635be5c5911fa

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

LOG: [clangd] Don't send invalid messages from remote index

Summary:
Remote server should not send messages that are invalid and will cause problems
on the client side. The client should not be affected by server's failures
whenever possible.

Also add more error messages and logs.

Reviewers: sammccall

Reviewed By: sammccall

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

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/index/remote/Client.cpp
clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
clang-tools-extra/clangd/index/remote/server/Server.cpp
clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/index/remote/Client.cpp 
b/clang-tools-extra/clangd/index/remote/Client.cpp
index b46883193076..35ce84068f40 100644
--- a/clang-tools-extra/clangd/index/remote/Client.cpp
+++ b/clang-tools-extra/clangd/index/remote/Client.cpp
@@ -29,16 +29,6 @@ class IndexClient : public clangd::SymbolIndex {
   using StreamingCall = std::unique_ptr> (
   remote::SymbolIndex::Stub::*)(grpc::ClientContext *, const RequestT &);
 
-  template 
-  RequestT serializeRequest(ClangdRequestT Request) const {
-return toProtobuf(Request);
-  }
-
-  template <>
-  FuzzyFindRequest serializeRequest(clangd::FuzzyFindRequest Request) const {
-return toProtobuf(Request, ProjectRoot);
-  }
-
   template 
   bool streamRPC(ClangdRequestT Request,
@@ -46,24 +36,23 @@ class IndexClient : public clangd::SymbolIndex {
  CallbackT Callback) const {
 bool FinalResult = false;
 trace::Span Tracer(RequestT::descriptor()->name());
-const auto RPCRequest = serializeRequest(Request);
+const auto RPCRequest = ProtobufMarshaller->toProtobuf(Request);
 grpc::ClientContext Context;
 std::chrono::system_clock::time_point Deadline =
 std::chrono::system_clock::now() + DeadlineWaitingTime;
 Context.set_deadline(Deadline);
 auto Reader = (Stub.get()->*RPCCall)(&Context, RPCRequest);
-llvm::BumpPtrAllocator Arena;
-llvm::UniqueStringSaver Strings(Arena);
 ReplyT Reply;
 while (Reader->Read(&Reply)) {
   if (!Reply.has_stream_result()) {
 FinalResult = Reply.final_result();
 continue;
   }
-  auto Response =
-  fromProtobuf(Reply.stream_result(), &Strings, ProjectRoot);
-  if (!Response)
+  auto Response = ProtobufMarshaller->fromProtobuf(Reply.stream_result());
+  if (!Response) {
 elog("Received invalid {0}", ReplyT::descriptor()->name());
+continue;
+  }
   Callback(*Response);
 }
 SPAN_ATTACH(Tracer, "status", Reader->Finish().ok());
@@ -74,7 +63,9 @@ class IndexClient : public clangd::SymbolIndex {
   IndexClient(
   std::shared_ptr Channel, llvm::StringRef ProjectRoot,
   std::chrono::milliseconds DeadlineTime = std::chrono::milliseconds(1000))
-  : Stub(remote::SymbolIndex::NewStub(Channel)), ProjectRoot(ProjectRoot),
+  : Stub(remote::SymbolIndex::NewStub(Channel)),
+ProtobufMarshaller(new Marshaller(/*RemoteIndexRoot=*/"",
+  /*LocalIndexRoot=*/ProjectRoot)),
 DeadlineWaitingTime(DeadlineTime) {}
 
   void lookup(const clangd::LookupRequest &Request,
@@ -105,7 +96,7 @@ class IndexClient : public clangd::SymbolIndex {
 
 private:
   std::unique_ptr Stub;
-  std::string ProjectRoot;
+  std::unique_ptr ProtobufMarshaller;
   // Each request will be terminated if it takes too long.
   std::chrono::milliseconds DeadlineWaitingTime;
 };

diff  --git a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp 
b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
index db895bf039ba..059c42ee0eaa 100644
--- a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -30,101 +30,28 @@ namespace clang {
 namespace clangd {
 namespace remote {
 
-namespace {
-
-clangd::SymbolLocation::Position fromProtobuf(const Position &Message) {
-  clangd::SymbolLocation::Position Result;
-  Result.setColumn(static_cast(Message.column()));
-  Result.setLine(static_cast(Message.line()));
-  return Result;
-}
-
-Position toProtobuf(const clangd::SymbolLocation::Position &Position) {
-  remote::Position Result;
-  Result.set_column(Position.column());
-  Result.set_line(Position.line());
-  ret

[PATCH] D84090: [clang-format] Add BitFieldColonSpacing option

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a subscriber: djasper.
MyDeveloperDay added a comment.

> Thanks for assistance and review. And sorry about the extra noise.

No worries on my part about the noise, firstly this was a really nice clean, 
quick review but also the noise is part of the learning for us all (especially 
for me!), I'd much rather spend the time helping to build a `clang-formatters` 
community like this with a little noise. In my view it is important for us to 
build a strong community who can help take over the mantel from @djasper and 
@klimek's amazing work and let them move onto other areas of interest hopefully 
with the knowledge we won't undo all their good work!

if its OK with you I'd like to keep you on my list of people who I can use as a 
reviewer, I very much appreciated your analytical approach to determine what 
was the most used options.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84090



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

[PATCH] D84090: [clang-format] Add BitFieldColonSpacing option

[MyDeveloperDay via Phabricator via cfe-commits]

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

just reopening to formally accept


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84090



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

[PATCH] D83759: [clangd] Port lit tests to Windows

[Aleksandr Platonov via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/test/background-index.test:18
 # Test that the index is writing files in the expected location.
 # RUN: ls %t/.cache/clangd/index/foo.cpp.*.idx
 # RUN: ls %t/sub_dir/.cache/clangd/index/foo.h.*.idx

kadircet wrote:
> are you sure these don't need changes on windows bots ? AFAICT this contains 
> both backslashes(coming from %t) and forwards slashes, and command is `ls` 
> which might not be available on a default windows prompt.
I am not sure. How I can check it? The only thing I can say, that I tested this 
on mingw and visual studio builds on my local machine.

>  this contains both backslashes(coming from %t) and forwards slashes,
For me that was OK, but I agree that `ls %t/.cache/clangd/index/foo.cpp.*.idx` 
should be replaced with `ls %/t/.cache/clangd/index/foo.cpp.*.idx`.

As for `ls`, it's a part of mingw, so I think `ls` absence probability is the 
same as for `sed`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83759



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

[PATCH] D83223: [clang-tidy] Header guard check can skip past license comment

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In D83223#2147306 , @aaron.ballman 
wrote:

> In D83223#2147247 , @njames93 wrote:
>
> > In D83223#2147072 , @aaron.ballman 
> > wrote:
> >
> > > >   // This is not identified as a license comment as the
> > > >   // block is followed by code.
> > > >   void foo();
> > >
> > > FWIW: 
> > > https://github.com/GrammaTech/gtirb-pprinter/blob/master/include/gtirb_pprinter/AttPrettyPrinter.hpp
> > >  or 
> > > https://github.com/GrammaTech/gtirb/blob/master/include/gtirb/AuxData.hpp 
> > > (so there are projects which do not put a newline between the license and 
> > > code).
> >
> >
> > Short of creating an AI that understands context it won't be possible to 
> > determine the difference between license and general documentation, in any 
> > case I feel this heuristic is the safest way to ensure good coverage with 
> > minimised risk of inserting the guard in the middle of documentation,
>
>
> My instinct is that we shouldn't be trying to play those games in the first 
> place and should consider *all* leading comments and empty (whitespace-only) 
> lines as part of the "license" and expect the first significant token to be 
> the header guard. e.g., this isn't about the license at all, it's about 
> whether you can have prose before the header guard or not. It's not uncommon 
> for projects to put prose before header guards, nor is it uncommon for it to 
> go after the header guards. tbh, that feels a bit like an option for the 
> feature rather than an automatic behavior because I could also see a project 
> wanting to enforce a consistent style.


A drive-by note: I'm also for treating all leading comments equally (and 
referring to them as just "comments", not "license comments").


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83223



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

[PATCH] D84225: [CFE] Add nomerge function attribute to inline assembly.

[Pengfei Wang via Phabricator via cfe-commits]

pengfei created this revision.
pengfei added reviewers: zequanwu, rnk, asbirlea.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Sometimes we also want to avoid merging inline assembly. This patch add
the nomerge function attribute to inline assembly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84225

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGen/attr-nomerge.cpp


Index: clang/test/CodeGen/attr-nomerge.cpp
===
--- clang/test/CodeGen/attr-nomerge.cpp
+++ clang/test/CodeGen/attr-nomerge.cpp
@@ -10,6 +10,7 @@
   [[clang::nomerge]] f(bar(), bar());
   [[clang::nomerge]] [] { bar(); bar(); }(); // nomerge only applies to the 
anonymous function call
   [[clang::nomerge]] for (bar(); bar(); bar()) {}
+  [[clang::nomerge]] { asm("nop"); }
   bar();
 }
 // CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR:[0-9]+]]
@@ -22,5 +23,7 @@
 // CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
 // CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
 // CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
+// CHECK: call void asm {{.*}} #[[NOMERGEATTR2:[0-9]+]]
 // CHECK: call zeroext i1 @_Z3barv()
 // CHECK: attributes #[[NOMERGEATTR]] = { nomerge }
+// CHECK: attributes #[[NOMERGEATTR2]] = { nomerge nounwind }
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -183,6 +183,7 @@
   bool foundCallExpr() { return FoundCallExpr; }
 
   void VisitCallExpr(const CallExpr *E) { FoundCallExpr = true; }
+  void VisitAsmStmt(const AsmStmt *S) { FoundCallExpr = true; }
 
   void Visit(const Stmt *St) {
 if (!St)
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -1954,12 +1954,16 @@
 }
 
 static void UpdateAsmCallInst(llvm::CallBase &Result, bool HasSideEffect,
-  bool ReadOnly, bool ReadNone, const AsmStmt &S,
+  bool ReadOnly, bool ReadNone, bool NoMerge,
+  const AsmStmt &S,
   const std::vector &ResultRegTypes,
   CodeGenFunction &CGF,
   std::vector &RegResults) {
   Result.addAttribute(llvm::AttributeList::FunctionIndex,
   llvm::Attribute::NoUnwind);
+  if (NoMerge)
+Result.addAttribute(llvm::AttributeList::FunctionIndex,
+llvm::Attribute::NoMerge);
   // Attach readnone and readonly attributes.
   if (!HasSideEffect) {
 if (ReadNone)
@@ -2334,12 +2338,14 @@
 Builder.CreateCallBr(IA, Fallthrough, Transfer, Args);
 EmitBlock(Fallthrough);
 UpdateAsmCallInst(cast(*Result), HasSideEffect, ReadOnly,
-  ReadNone, S, ResultRegTypes, *this, RegResults);
+  ReadNone, InNoMergeAttributedStmt, S, ResultRegTypes,
+  *this, RegResults);
   } else {
 llvm::CallInst *Result =
 Builder.CreateCall(IA, Args, getBundlesForFunclet(IA));
 UpdateAsmCallInst(cast(*Result), HasSideEffect, ReadOnly,
-  ReadNone, S, ResultRegTypes, *this, RegResults);
+  ReadNone, InNoMergeAttributedStmt, S, ResultRegTypes,
+  *this, RegResults);
   }
 
   assert(RegResults.size() == ResultRegTypes.size());


Index: clang/test/CodeGen/attr-nomerge.cpp
===
--- clang/test/CodeGen/attr-nomerge.cpp
+++ clang/test/CodeGen/attr-nomerge.cpp
@@ -10,6 +10,7 @@
   [[clang::nomerge]] f(bar(), bar());
   [[clang::nomerge]] [] { bar(); bar(); }(); // nomerge only applies to the anonymous function call
   [[clang::nomerge]] for (bar(); bar(); bar()) {}
+  [[clang::nomerge]] { asm("nop"); }
   bar();
 }
 // CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR:[0-9]+]]
@@ -22,5 +23,7 @@
 // CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
 // CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
 // CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
+// CHECK: call void asm {{.*}} #[[NOMERGEATTR2:[0-9]+]]
 // CHECK: call zeroext i1 @_Z3barv()
 // CHECK: attributes #[[NOMERGEATTR]] = { nomerge }
+// CHECK: attributes #[[NOMERGEATTR2]] = { nomerge nounwind }
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -183,6 +183,7 @@
   bool foundCallExpr() { return FoundCallExpr; }
 
   void VisitCallExpr(const CallExpr *E) { FoundCallExpr = true; }
+  void VisitAsmStmt(const AsmStmt *S) { FoundCallExpr = true; }
 
   void Visit(const Stmt *St) {
 if (!St)
Index: clang/lib/CodeGen/CGStmt.cpp
==

[clang-tools-extra] 4470b8c - [clangd] Fix assertions for D83826

[Kirill Bobyrev via cfe-commits]

Author: Kirill Bobyrev
Date: 2020-07-21T11:20:31+02:00
New Revision: 4470b8c6a6b16de3b4f1f3c4cf81137a9fe4c8a1

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

LOG: [clangd] Fix assertions for D83826

FuzzyFindRequest's toProtobuf is called on the client side (hence
LocalIndexRoot must be present) and fromProtobuf - on the server.

Added: 


Modified: 
clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp 
b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
index 059c42ee0eaa..e8393d17b01e 100644
--- a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -51,7 +51,7 @@ Marshaller::Marshaller(llvm::StringRef RemoteIndexRoot,
 
 clangd::FuzzyFindRequest
 Marshaller::fromProtobuf(const FuzzyFindRequest *Request) {
-  assert(LocalIndexRoot);
+  assert(RemoteIndexRoot);
   clangd::FuzzyFindRequest Result;
   Result.Query = Request->query();
   for (const auto &Scope : Request->scopes())
@@ -146,7 +146,7 @@ LookupRequest Marshaller::toProtobuf(const 
clangd::LookupRequest &From) {
 }
 
 FuzzyFindRequest Marshaller::toProtobuf(const clangd::FuzzyFindRequest &From) {
-  assert(RemoteIndexRoot);
+  assert(LocalIndexRoot);
   FuzzyFindRequest RPCRequest;
   RPCRequest.set_query(From.Query);
   for (const auto &Scope : From.Scopes)



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

[PATCH] D84222: [AST][RecoveryExpr] Error-dependent expression should not be treat as a nullptr pointer constant.

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/Expr.cpp:3747
 case NPC_ValueDependentIsNull:
-  if (isTypeDependent() || getType()->isIntegralType(Ctx))
+  if ((!containsErrors() && isTypeDependent()) ||
+  getType()->isIntegralType(Ctx))

would it be clearer to say if (containsErrors()) return NPCK_NotNull at the top?

Only difference in behavior is not hitting llvm_unreachable if we have 
NPC_NeverValueDependent... do you think we actually want that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84222



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

[PATCH] D83817: [clangd] Add remote index support for Clangd itself

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 279457.
kbobyrev added a comment.

Add flag description.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83817

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/Features.inc.in
  clang-tools-extra/clangd/tool/ClangdMain.cpp

Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -14,6 +14,7 @@
 #include "Transport.h"
 #include "index/Background.h"
 #include "index/Serialization.h"
+#include "index/remote/Client.h"
 #include "refactor/Rename.h"
 #include "support/Path.h"
 #include "support/Shutdown.h"
@@ -64,6 +65,7 @@
 OptionCategory Features("clangd feature options");
 OptionCategory Misc("clangd miscellaneous options");
 OptionCategory Protocol("clangd protocol and logging options");
+OptionCategory Remote("clangd remote index options");
 const OptionCategory *ClangdCategories[] = {&Features, &Protocol,
 &CompileCommands, &Misc};
 
@@ -449,6 +451,23 @@
 init(false),
 };
 
+#ifdef CLANGD_ENABLE_REMOTE
+opt RemoteIndexAddress{
+"remote-index-address",
+cat(Remote),
+desc("Address of the remote index server"),
+Hidden,
+};
+
+// FIXME(kirillbobyrev): Should this be the location of compile_commands.json?
+opt ProjectPath{
+"project-path",
+cat(Remote),
+desc("Path to the project root. Requires remote-index-address to be set."),
+Hidden,
+};
+#endif
+
 /// Supports a test URI scheme with relaxed constraints for lit tests.
 /// The path in a test URI will be combined with a platform-specific fake
 /// directory to form an absolute path. For example, test:///a.cpp is resolved
@@ -680,6 +699,20 @@
 if (Sync)
   AsyncIndexLoad.wait();
   }
+  if (RemoteIndexAddress.empty() != ProjectPath.empty()) {
+llvm::errs() << "remote-index-address and project-path have to be "
+"specified at the same time.";
+return 1;
+  }
+  if (!RemoteIndexAddress.empty()) {
+if (!IndexFile.empty()) {
+  llvm::errs() << "When remote index is enabled, IndexFile should not be "
+  "specified. Only one can be used at time.";
+  return 1;
+}
+log("Connecting to remote index at {0}", RemoteIndexAddress);
+StaticIdx = remote::getClient(RemoteIndexAddress, ProjectPath);
+  }
   Opts.StaticIndex = StaticIdx.get();
   Opts.AsyncThreadsCount = WorkerThreadsCount;
   Opts.BuildRecoveryAST = RecoveryAST;
Index: clang-tools-extra/clangd/Features.inc.in
===
--- clang-tools-extra/clangd/Features.inc.in
+++ clang-tools-extra/clangd/Features.inc.in
@@ -1 +1,2 @@
 #define CLANGD_BUILD_XPC @CLANGD_BUILD_XPC@
+#define CLANGD_ENABLE_REMOTE @CLANGD_ENABLE_REMTE@
Index: clang-tools-extra/clangd/CMakeLists.txt
===
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -109,6 +109,15 @@
   omp_gen
   )
 
+# FIXME(kirillbobyrev): Document this in the LLVM docs once remote index is stable.
+option(CLANGD_ENABLE_REMOTE "Use gRPC library to enable remote index support for Clangd" OFF)
+set(GRPC_INSTALL_PATH "" CACHE PATH "Path to gRPC library manual installation.")
+
+if (CLANGD_ENABLE_REMOTE)
+  include(FindGRPC)
+endif()
+add_subdirectory(index/remote)
+
 clang_target_link_libraries(clangDaemon
   PRIVATE
   clangAST
@@ -126,6 +135,7 @@
   clangToolingInclusions
   clangToolingRefactoring
   clangToolingSyntax
+  clangdRemoteIndex
   )
 
 add_subdirectory(refactor/tweaks)
@@ -148,12 +158,4 @@
 add_subdirectory(unittests)
 endif()
 
-# FIXME(kirillbobyrev): Document this in the LLVM docs once remote index is stable.
-option(CLANGD_ENABLE_REMOTE "Use gRPC library to enable remote index support for Clangd" OFF)
-set(GRPC_INSTALL_PATH "" CACHE PATH "Path to gRPC library manual installation.")
-
-if (CLANGD_ENABLE_REMOTE)
-  include(FindGRPC)
-endif()
-add_subdirectory(index/remote)
 add_subdirectory(index/dex/dexp)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83551: [PATCH 2/4][Sema][AArch64] Add semantics for arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes updated this revision to Diff 279455.
c-rhodes added a comment.

- Make helpers static in ASTContext.
- Use `getCharWidth`.
- `s/vector-length-sized/vector-length-specific`.


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

https://reviews.llvm.org/D83551

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/Type.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/attr-arm-sve-vector-bits.c

Index: clang/test/Sema/attr-arm-sve-vector-bits.c
===
--- clang/test/Sema/attr-arm-sve-vector-bits.c
+++ clang/test/Sema/attr-arm-sve-vector-bits.c
@@ -60,3 +60,168 @@
 typedef float badtype3 __attribute__((arm_sve_vector_bits(N))); // expected-error {{'arm_sve_vector_bits' attribute applied to non-SVE type 'float'}}
 typedef svint8x2_t badtype4 __attribute__((arm_sve_vector_bits(N)));// expected-error {{'arm_sve_vector_bits' attribute applied to non-SVE type 'svint8x2_t' (aka '__clang_svint8x2_t')}}
 typedef svfloat32x3_t badtype5 __attribute__((arm_sve_vector_bits(N))); // expected-error {{'arm_sve_vector_bits' attribute applied to non-SVE type 'svfloat32x3_t' (aka '__clang_svfloat32x3_t')}}
+
+// Attribute only applies to typedefs.
+svint8_t non_typedef_type __attribute__((arm_sve_vector_bits(N)));  // expected-error {{'arm_sve_vector_bits' attribute only applies to typedefs}}
+
+// Test that we can define non-local fixed-length SVE types (unsupported for
+// sizeless types).
+fixed_int8_t global_int8;
+fixed_bfloat16_t global_bfloat16;
+fixed_bool_t global_bool;
+
+extern fixed_int8_t extern_int8;
+extern fixed_bfloat16_t extern_bfloat16;
+extern fixed_bool_t extern_bool;
+
+static fixed_int8_t static_int8;
+static fixed_bfloat16_t static_bfloat16;
+static fixed_bool_t static_bool;
+
+fixed_int8_t *global_int8_ptr;
+extern fixed_int8_t *extern_int8_ptr;
+static fixed_int8_t *static_int8_ptr;
+__thread fixed_int8_t thread_int8;
+
+typedef fixed_int8_t int8_typedef;
+typedef fixed_int8_t *int8_ptr_typedef;
+
+// Test sized expressions
+int sizeof_int8 = sizeof(global_int8);
+int sizeof_int8_var = sizeof(*global_int8_ptr);
+int sizeof_int8_var_ptr = sizeof(global_int8_ptr);
+
+extern fixed_int8_t *extern_int8_ptr;
+
+int alignof_int8 = __alignof__(extern_int8);
+int alignof_int8_var = __alignof__(*extern_int8_ptr);
+int alignof_int8_var_ptr = __alignof__(extern_int8_ptr);
+
+void f(int c) {
+  fixed_int8_t fs8;
+  svint8_t ss8;
+
+  void *sel __attribute__((unused));
+  sel = c ? ss8 : fs8; // expected-error {{incompatible operand types ('svint8_t' (aka '__SVInt8_t') and 'fixed_int8_t' (aka '__SVInt8_t'))}}
+  sel = c ? fs8 : ss8; // expected-error {{incompatible operand types ('fixed_int8_t' (aka '__SVInt8_t') and 'svint8_t' (aka '__SVInt8_t'))}}
+}
+
+// --//
+// Sizeof
+
+#define VECTOR_SIZE ((N / 8))
+#define PRED_SIZE ((N / 64))
+
+_Static_assert(sizeof(fixed_int8_t) == VECTOR_SIZE, "");
+
+_Static_assert(sizeof(fixed_int16_t) == VECTOR_SIZE, "");
+_Static_assert(sizeof(fixed_int32_t) == VECTOR_SIZE, "");
+_Static_assert(sizeof(fixed_int64_t) == VECTOR_SIZE, "");
+
+_Static_assert(sizeof(fixed_uint8_t) == VECTOR_SIZE, "");
+_Static_assert(sizeof(fixed_uint16_t) == VECTOR_SIZE, "");
+_Static_assert(sizeof(fixed_uint32_t) == VECTOR_SIZE, "");
+_Static_assert(sizeof(fixed_uint64_t) == VECTOR_SIZE, "");
+
+_Static_assert(sizeof(fixed_float16_t) == VECTOR_SIZE, "");
+_Static_assert(sizeof(fixed_float32_t) == VECTOR_SIZE, "");
+_Static_assert(sizeof(fixed_float64_t) == VECTOR_SIZE, "");
+
+_Static_assert(sizeof(fixed_bfloat16_t) == VECTOR_SIZE, "");
+
+_Static_assert(sizeof(fixed_bool_t) == PRED_SIZE, "");
+
+// --//
+// Alignof
+
+#define VECTOR_ALIGN 16
+#define PRED_ALIGN 2
+
+_Static_assert(__alignof__(fixed_int8_t) == VECTOR_ALIGN, "");
+_Static_assert(__alignof__(fixed_int16_t) == VECTOR_ALIGN, "");
+_Static_assert(__alignof__(fixed_int32_t) == VECTOR_ALIGN, "");
+_Static_assert(__alignof__(fixed_int64_t) == VECTOR_ALIGN, "");
+
+_Static_assert(__alignof__(fixed_uint8_t) == VECTOR_ALIGN, "");
+_Static_assert(__alignof__(fixed_uint16_t) == VECTOR_ALIGN, "");
+_Static_assert(__alignof__(fixed_uint32_t) == VECTOR_ALIGN, "");
+_Static_assert(__alignof__(fixed_uint64_t) == VECTOR_ALIGN, "");
+
+_Static_assert(__alignof__(fixed_float16_t) == VECTOR_ALIGN, "");
+_Static_assert(__alignof__(fixed_float32_t) == VECTOR_ALIGN, "");
+_Static_assert(__alignof__(fixed_float64_t) == VECTOR_ALIGN, "");
+
+_Static_assert(__alignof__(fixed_bfloat16_t) == VECTOR_ALIGN, "");
+
+_Static_assert(__alignof__(fixed_bool_t) == PRED_ALIGN, "");
+
+// --//
+/

[PATCH] D84226: [AST][RecoveryExpr] Support dependent binary operator in C for error recovery.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84226

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/AST/ast-dump-recovery.c
  clang/test/Sema/error-dependence.c

Index: clang/test/Sema/error-dependence.c
===
--- /dev/null
+++ clang/test/Sema/error-dependence.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -frecovery-ast -fno-recovery-ast-type %s
+
+int call(int); // expected-note {{'call' declared here}}
+
+void test1(int s) {
+  // verify "assigning to 'int' from incompatible type ''" is
+  // not emitted.
+  s = call(); // expected-error {{too few arguments to function call}}
+}
Index: clang/test/AST/ast-dump-recovery.c
===
--- clang/test/AST/ast-dump-recovery.c
+++ clang/test/AST/ast-dump-recovery.c
@@ -42,11 +42,16 @@
 
 void test2() {
   int* ptr;
-  // FIXME: the top-level expr should be a binary operator.
-  // CHECK:  ImplicitCastExpr {{.*}} contains-errors 
-  // CHECK-NEXT: `-RecoveryExpr {{.*}} contains-errors lvalue
-  // CHECK-NEXT:   |-DeclRefExpr {{.*}} 'ptr' 'int *'
-  // CHECK-NEXT:   `-RecoveryExpr {{.*}}
-  // CHECK-NEXT: `-DeclRefExpr {{.*}} 'some_func'
+  // CHECK: BinaryOperator {{.*}} contains-errors '='
+  // CHECK-NEXT: |-DeclRefExpr {{.*}} 'ptr' 'int *'
+  // CHECK-NEXT: `-RecoveryExpr {{.*}}
+  // CHECK-NEXT:   `-DeclRefExpr {{.*}} 'some_func'
   ptr = some_func(); // should not crash
+
+  int compoundOp;
+  // CHECK: CompoundAssignOperator {{.*}} '+='
+  // CHECK-NEXT: |-DeclRefExpr {{.*}} 'compoundOp'
+  // CHECK-NEXT: `-RecoveryExpr {{.*}} contains-errors
+  // CHECK-NEXT:   `-DeclRefExpr {{.*}} 'some_func'
+  compoundOp += some_func();
 }
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -13288,18 +13288,8 @@
   // If either side is type-dependent, create an appropriate dependent
   // expression.
   if (Args[0]->isTypeDependent() || Args[1]->isTypeDependent()) {
-if (Fns.empty()) {
-  // If there are no functions to store, just build a dependent
-  // BinaryOperator or CompoundAssignment.
-  if (Opc <= BO_Assign || Opc > BO_OrAssign)
-return BinaryOperator::Create(
-Context, Args[0], Args[1], Opc, Context.DependentTy, VK_RValue,
-OK_Ordinary, OpLoc, CurFPFeatureOverrides());
-  return CompoundAssignOperator::Create(
-  Context, Args[0], Args[1], Opc, Context.DependentTy, VK_LValue,
-  OK_Ordinary, OpLoc, CurFPFeatureOverrides(), Context.DependentTy,
-  Context.DependentTy);
-}
+if (Fns.empty())
+  return CreateDependentBinOp(OpLoc, Opc, Args[0], Args[1]);
 
 // FIXME: save results of ADL from here?
 CXXRecordDecl *NamingClass = nullptr; // lookup ignores member operators
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -14220,10 +14220,27 @@
   return S.CreateOverloadedBinOp(OpLoc, Opc, Functions, LHS, RHS);
 }
 
+ExprResult Sema::CreateDependentBinOp(SourceLocation OpLoc,
+  BinaryOperatorKind Opc, Expr *LHS,
+  Expr *RHS) {
+  assert(LHS->isTypeDependent() || RHS->isTypeDependent());
+  // If there are no functions to store, just build a dependent
+  // BinaryOperator or CompoundAssignment.
+  if (Opc <= BO_Assign || Opc > BO_OrAssign)
+return BinaryOperator::Create(Context, LHS, RHS, Opc, Context.DependentTy,
+  VK_RValue, OK_Ordinary, OpLoc,
+  CurFPFeatureOverrides());
+  return CompoundAssignOperator::Create(
+  Context, LHS, RHS, Opc, Context.DependentTy, VK_LValue, OK_Ordinary,
+  OpLoc, CurFPFeatureOverrides(), Context.DependentTy, Context.DependentTy);
+}
+
 ExprResult Sema::BuildBinOp(Scope *S, SourceLocation OpLoc,
 BinaryOperatorKind Opc,
 Expr *LHSExpr, Expr *RHSExpr) {
   ExprResult LHS, RHS;
+  // FIXME: this early typo correction can be removed now, since we support the
+  // dependent binary operator in C.
   std::tie(LHS, RHS) = CorrectDelayedTyposInBinOp(*this, Opc, LHSExpr, RHSExpr);
   if (!LHS.isUsable() || !RHS.isUsable())
 return ExprError();
@@ -14322,6 +14339,13 @@
   return BuildOverloadedBinOp(*this, S, OpLoc, Opc, LHSExpr, RHSExpr);
   }
 
+  if (LHSExpr->isTypeDependent() || RHSExpr->isTypeDependent()) {
+assert(!getLangOpts().CPlusPlus);
+assert((LHSExpr->containsErrors() || RHSExpr->containsErrors()) &&
+   "Should only occur in error-reco

[PATCH] D82948: [Driver][ARM] Disable unsupported features when nofp arch extension is used

[Victor Campos via Phabricator via cfe-commits]

vhscampos added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82948



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

[PATCH] D84226: [AST][RecoveryExpr] Support dependent binary operator in C for error recovery.

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaExpr.cpp:14243
+  // FIXME: this early typo correction can be removed now, since we support the
+  // dependent binary operator in C.
   std::tie(LHS, RHS) = CorrectDelayedTyposInBinOp(*this, Opc, LHSExpr, 
RHSExpr);

I think we probably need a new cc1 flag e.g. `CDependenceType` to control the 
typo-correction and whether we build dependent ast node for C.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84226



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

[PATCH] D83759: [clangd] Port lit tests to Windows

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/test/background-index.test:18
 # Test that the index is writing files in the expected location.
 # RUN: ls %t/.cache/clangd/index/foo.cpp.*.idx
 # RUN: ls %t/sub_dir/.cache/clangd/index/foo.h.*.idx

ArcsinX wrote:
> kadircet wrote:
> > are you sure these don't need changes on windows bots ? AFAICT this 
> > contains both backslashes(coming from %t) and forwards slashes, and command 
> > is `ls` which might not be available on a default windows prompt.
> I am not sure. How I can check it? The only thing I can say, that I tested 
> this on mingw and visual studio builds on my local machine.
> 
> >  this contains both backslashes(coming from %t) and forwards slashes,
> For me that was OK, but I agree that `ls 
> %t/.cache/clangd/index/foo.cpp.*.idx` should be replaced with `ls 
> %/t/.cache/clangd/index/foo.cpp.*.idx`.
> 
> As for `ls`, it's a part of mingw, so I think `ls` absence probability is the 
> same as for `sed`
> I am not sure. How I can check it? The only thing I can say, that I tested 
> this on mingw and visual studio builds on my local machine.

unfortunately there is no good way (that I know of) for checking these, apart 
from landing the patch and monitoring the buildbots(http://lab.llvm.org:8011/) 
for breakages and reverting/fixing forward accordingly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83759



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

[clang-tools-extra] 7d591e1 - [clangd] Complete the fix for (Local|Remote)IndexRoot confusion

[Kirill Bobyrev via cfe-commits]

Author: Kirill Bobyrev
Date: 2020-07-21T11:53:17+02:00
New Revision: 7d591e123e0eb2d3415840de9c2b45c3742f0eaa

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

LOG: [clangd] Complete the fix for (Local|Remote)IndexRoot confusion

Related revision: https://reviews.llvm.org/D83826

Added: 


Modified: 
clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp 
b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
index e8393d17b01e..b6c83c974072 100644
--- a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -61,7 +61,7 @@ Marshaller::fromProtobuf(const FuzzyFindRequest *Request) {
 Result.Limit = Request->limit();
   Result.RestrictForCodeCompletion = Request->restricted_for_code_completion();
   for (const auto &Path : Request->proximity_paths()) {
-llvm::SmallString<256> LocalPath = llvm::StringRef(*LocalIndexRoot);
+llvm::SmallString<256> LocalPath = llvm::StringRef(*RemoteIndexRoot);
 llvm::sys::path::append(LocalPath, Path);
 Result.ProximityPaths.push_back(std::string(LocalPath));
   }
@@ -157,7 +157,7 @@ FuzzyFindRequest Marshaller::toProtobuf(const 
clangd::FuzzyFindRequest &From) {
   
RPCRequest.set_restricted_for_code_completion(From.RestrictForCodeCompletion);
   for (const auto &Path : From.ProximityPaths) {
 llvm::SmallString<256> RelativePath = llvm::StringRef(Path);
-if (llvm::sys::path::replace_path_prefix(RelativePath, *RemoteIndexRoot,
+if (llvm::sys::path::replace_path_prefix(RelativePath, *LocalIndexRoot,
  ""))
   RPCRequest.add_proximity_paths(llvm::sys::path::convert_to_slash(
   RelativePath, llvm::sys::path::Style::posix));

diff  --git a/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp 
b/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
index 6cc08144bef8..7db7c03d61c9 100644
--- a/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
+++ b/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
@@ -282,16 +282,16 @@ TEST(RemoteMarshallingTest, IncludeHeaderURIs) {
 
 TEST(RemoteMarshallingTest, FuzzyFindRequestSerialization) {
   clangd::FuzzyFindRequest Request;
-  Request.ProximityPaths = {testPath("remote/Header.h"),
-testPath("remote/subdir/OtherHeader.h"),
-testPath("notremote/File.h"), "Not a Path."};
-  Marshaller ProtobufMarshaller(testPath("remote/"), testPath("home/"));
+  Request.ProximityPaths = {testPath("local/Header.h"),
+testPath("local/subdir/OtherHeader.h"),
+testPath("remote/File.h"), "Not a Path."};
+  Marshaller ProtobufMarshaller(testPath("remote/"), testPath("local/"));
   auto Serialized = ProtobufMarshaller.toProtobuf(Request);
   EXPECT_EQ(Serialized.proximity_paths_size(), 2);
   auto Deserialized = ProtobufMarshaller.fromProtobuf(&Serialized);
   EXPECT_THAT(Deserialized.ProximityPaths,
-  testing::ElementsAre(testPath("home/Header.h"),
-   testPath("home/subdir/OtherHeader.h")));
+  testing::ElementsAre(testPath("remote/Header.h"),
+   testPath("remote/subdir/OtherHeader.h")));
 }
 
 TEST(RemoteMarshallingTest, RelativePathToURITranslation) {



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

[clang] 76c0577 - [Analyzer] Handle unique_ptr::swap() in SmartPtrModeling

[Nithin Vadukkumchery Rajendrakumar via cfe-commits]

Author: Nithin Vadukkumchery Rajendrakumar
Date: 2020-07-21T12:05:27+02:00
New Revision: 76c0577763505ea3db1017a9aab579c1c2f135d0

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

LOG: [Analyzer] Handle unique_ptr::swap() in SmartPtrModeling

Summary:
Implemented modeling for unique_ptr::swap() SmartPtrModeling

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

Reviewers: NoQ, Szelethus, vsavchenko, xazax.hun
Reviewed By: NoQ, vsavchenko, xazax.hun

Tags: #clang

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
clang/test/Analysis/Inputs/system-header-simulator-cxx.h
clang/test/Analysis/smart-ptr.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp 
b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
index 1b2174501d6e..c36e89c3e3a9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -108,6 +108,17 @@ removeTrackedSubregions(TrackedRegionMapTy RegionMap,
   return RegionMap;
 }
 
+static ProgramStateRef updateSwappedRegion(ProgramStateRef State,
+   const MemRegion *Region,
+   const SVal *RegionInnerPointerVal) {
+  if (RegionInnerPointerVal) {
+State = State->set(Region, *RegionInnerPointerVal);
+  } else {
+State = State->remove(Region);
+  }
+  return State;
+}
+
 bool SmartPtrModeling::isNullAfterMoveMethod(const CallEvent &Call) const {
   // TODO: Update CallDescription to support anonymous calls?
   // TODO: Handle other methods, such as .get() or .release().
@@ -129,7 +140,8 @@ bool SmartPtrModeling::evalCall(const CallEvent &Call,
 cast(&Call)->getCXXThisVal().getAsRegion();
 
 if (!move::isMovedFrom(State, ThisR)) {
-  // TODO: Model this case as well. At least, avoid invalidation of 
globals.
+  // TODO: Model this case as well. At least, avoid invalidation of
+  // globals.
   return false;
 }
 
@@ -204,7 +216,7 @@ void SmartPtrModeling::handleReset(const CallEvent &Call,
 return;
   auto State = updateTrackedRegion(Call, C, ThisValRegion);
   C.addTransition(State);
-  // TODO: Make sure to ivalidate the the region in the Store if we don't have
+  // TODO: Make sure to ivalidate the region in the Store if we don't have
   // time to model all methods.
 }
 
@@ -232,7 +244,30 @@ void SmartPtrModeling::handleRelease(const CallEvent &Call,
 
 void SmartPtrModeling::handleSwap(const CallEvent &Call,
   CheckerContext &C) const {
-  // TODO: Add support to handle swap method.
+  // To model unique_ptr::swap() method.
+  const auto *IC = dyn_cast(&Call);
+  if (!IC)
+return;
+
+  const MemRegion *ThisRegion = IC->getCXXThisVal().getAsRegion();
+  if (!ThisRegion)
+return;
+
+  const auto *ArgRegion = Call.getArgSVal(0).getAsRegion();
+  if (!ArgRegion)
+return;
+
+  auto State = C.getState();
+  const auto *ThisRegionInnerPointerVal =
+  State->get(ThisRegion);
+  const auto *ArgRegionInnerPointerVal =
+  State->get(ArgRegion);
+
+  // Swap the tracked region values.
+  State = updateSwappedRegion(State, ThisRegion, ArgRegionInnerPointerVal);
+  State = updateSwappedRegion(State, ArgRegion, ThisRegionInnerPointerVal);
+
+  C.addTransition(State);
 }
 
 ProgramStateRef

diff  --git a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h 
b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
index d5e7c4c9218d..9010ce2bb9b6 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -971,6 +971,12 @@ class unique_ptr {
   operator bool() const noexcept;
   unique_ptr &operator=(unique_ptr &&p) noexcept;
 };
+
+// TODO :: Once the deleter parameter is added update with additional template 
parameter.
+template 
+void swap(unique_ptr &x, unique_ptr &y) noexcept {
+  x.swap(y);
+}
 } // namespace std
 #endif
 

diff  --git a/clang/test/Analysis/smart-ptr.cpp 
b/clang/test/Analysis/smart-ptr.cpp
index 5645afc9b657..168682ba758f 100644
--- a/clang/test/Analysis/smart-ptr.cpp
+++ b/clang/test/Analysis/smart-ptr.cpp
@@ -201,3 +201,30 @@ void derefAfterAssignment() {
 Q->foo(); // no-warning
   }
 }
+
+void derefOnSwappedNullPtr() {
+  std::unique_ptr P(new A());
+  std::unique_ptr PNull;
+  P.swap(PNull);
+  PNull->foo(); // No warning.
+  (*P).foo();   // expected-warning {{Dereference of null smart pointer 
[alpha.cplusplus.SmartPtr]}}
+}
+
+void derefOnStdSwappedNullPtr() 

[clang-tools-extra] 3ad0181 - [clangd] Fix null check after D82739.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2020-07-21T12:15:17+02:00
New Revision: 3ad0181169dc8efcdec7166a76a4624d73bdad48

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

LOG: [clangd] Fix null check after D82739.

I hit the null-deference crash when opening ASTReaderDecl.cpp.

The BaseType can be a nullptr,

Added: 


Modified: 
clang-tools-extra/clangd/FindTarget.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/FindTarget.cpp 
b/clang-tools-extra/clangd/FindTarget.cpp
index bc4002c42623..c6022b2463e8 100644
--- a/clang-tools-extra/clangd/FindTarget.cpp
+++ b/clang-tools-extra/clangd/FindTarget.cpp
@@ -175,6 +175,8 @@ std::vector 
resolveDependentExprToDecls(const Expr *E) {
 if (ME->isArrow()) {
   BaseType = getPointeeType(BaseType);
 }
+if (!BaseType)
+  return {};
 if (const auto *BT = BaseType->getAs()) {
   // If BaseType is the type of a dependent expression, it's just
   // represented as BultinType::Dependent which gives us no information. We



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

[Differential] D83772: [Windows] Fix limit on command line size

[Serge Pavlov via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd4020ef7c474: [Windows] Fix limit on command line size 
(authored by sepavloff).

Repository:
  rG LLVM Github Monorepo

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


  https://reviews.llvm.org/D83772

Files:
  llvm/include/llvm/Support/Program.h
  llvm/lib/Support/Windows/Program.inc
  llvm/unittests/Support/CommandLineTest.cpp

Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -763,6 +763,18 @@
 TEST(CommandLineTest, ArgumentLimit) {
   std::string args(32 * 4096, 'a');
   EXPECT_FALSE(llvm::sys::commandLineFitsWithinSystemLimits("cl", args.data()));
+  std::string args2(256, 'a');
+  EXPECT_TRUE(llvm::sys::commandLineFitsWithinSystemLimits("cl", args2.data()));
+  if (Triple(sys::getProcessTriple()).isOSWindows()) {
+// We use 32000 as a limit for command line length. Program name ('cl'),
+// separating spaces and termination null character occupy 5 symbols.
+std::string long_arg(32000 - 5, 'b');
+EXPECT_TRUE(
+llvm::sys::commandLineFitsWithinSystemLimits("cl", long_arg.data()));
+long_arg += 'b';
+EXPECT_FALSE(
+llvm::sys::commandLineFitsWithinSystemLimits("cl", long_arg.data()));
+  }
 }
 
 TEST(CommandLineTest, ResponseFileWindows) {
Index: llvm/lib/Support/Windows/Program.inc
===
--- llvm/lib/Support/Windows/Program.inc
+++ llvm/lib/Support/Windows/Program.inc
@@ -189,7 +189,14 @@
   // Windows wants a command line, not an array of args, to pass to the new
   // process.  We have to concatenate them all, while quoting the args that
   // have embedded spaces (or are empty).
-  std::string Command = flattenWindowsCommandLine(Args);
+  auto Result = flattenWindowsCommandLine(Args);
+  if (std::error_code ec = Result.getError()) {
+SetLastError(ec.value());
+MakeErrMsg(ErrMsg,
+   std::string("Unable to convert command-line to UTF-16"));
+return false;
+  }
+  std::wstring Command = *Result;
 
   // The pointer to the environment block for the new process.
   std::vector EnvBlock;
@@ -271,14 +278,8 @@
 return false;
   }
 
-  SmallVector CommandUtf16;
-  if (std::error_code ec = windows::UTF8ToUTF16(Command, CommandUtf16)) {
-SetLastError(ec.value());
-MakeErrMsg(ErrMsg,
-   std::string("Unable to convert command-line to UTF-16"));
-return false;
-  }
-
+  std::vector CommandUtf16(Command.size() + 1, 0);
+  std::copy(Command.begin(), Command.end(), CommandUtf16.begin());
   BOOL rc = CreateProcessW(ProgramUtf16.data(), CommandUtf16.data(), 0, 0,
TRUE, CREATE_UNICODE_ENVIRONMENT,
EnvBlock.empty() ? 0 : EnvBlock.data(), 0, &si,
@@ -376,7 +377,7 @@
 }
 
 namespace llvm {
-std::string sys::flattenWindowsCommandLine(ArrayRef Args) {
+ErrorOr sys::flattenWindowsCommandLine(ArrayRef Args) {
   std::string Command;
   for (StringRef Arg : Args) {
 if (argNeedsQuotes(Arg))
@@ -387,7 +388,11 @@
 Command.push_back(' ');
   }
 
-  return Command;
+  SmallVector CommandUtf16;
+  if (std::error_code ec = windows::UTF8ToUTF16(Command, CommandUtf16))
+return ec;
+
+  return std::wstring(CommandUtf16.begin(), CommandUtf16.end());
 }
 
 ProcessInfo sys::Wait(const ProcessInfo &PI, unsigned SecondsToWait,
@@ -532,12 +537,16 @@
 
 bool llvm::sys::commandLineFitsWithinSystemLimits(StringRef Program,
   ArrayRef Args) {
-  // The documented max length of the command line passed to CreateProcess.
-  static const size_t MaxCommandStringLength = 32768;
+  // The documentation on CreateProcessW states that the size of the argument
+  // lpCommandLine must not be greater than 32767 characters, including the
+  // Unicode terminating null character. We use smaller value to reduce risk
+  // of getting invalid command line due to unaccounted factors.
+  static const size_t MaxCommandStringLength = 32000;
   SmallVector FullArgs;
   FullArgs.push_back(Program);
   FullArgs.append(Args.begin(), Args.end());
-  std::string Result = flattenWindowsCommandLine(FullArgs);
-  return (Result.size() + 1) <= MaxCommandStringLength;
+  auto Result = flattenWindowsCommandLine(FullArgs);
+  assert(!Result.getError());
+  return (Result->size() + 1) <= MaxCommandStringLength;
 }
 }
Index: llvm/include/llvm/Support/Program.h
===
--- llvm/include/llvm/Support/Program.h
+++ llvm/include/llvm/Support/Program.h
@@ -218,7 +218,7 @@
   /// to build a single flat command line appropriate for calling CreateProcess
   /// on
   /// Windows.
-  std::string flattenWindowsCommandLine(ArrayRef Args);
+  ErrorOr flattenWindowsCommandLine(ArrayRef Args);
 #e

[PATCH] D83772: [Windows] Fix limit on command line size

[Serge Pavlov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd4020ef7c474: [Windows] Fix limit on command line size 
(authored by sepavloff).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83772

Files:
  llvm/include/llvm/Support/Program.h
  llvm/lib/Support/Windows/Program.inc
  llvm/unittests/Support/CommandLineTest.cpp

Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -763,6 +763,18 @@
 TEST(CommandLineTest, ArgumentLimit) {
   std::string args(32 * 4096, 'a');
   EXPECT_FALSE(llvm::sys::commandLineFitsWithinSystemLimits("cl", args.data()));
+  std::string args2(256, 'a');
+  EXPECT_TRUE(llvm::sys::commandLineFitsWithinSystemLimits("cl", args2.data()));
+  if (Triple(sys::getProcessTriple()).isOSWindows()) {
+// We use 32000 as a limit for command line length. Program name ('cl'),
+// separating spaces and termination null character occupy 5 symbols.
+std::string long_arg(32000 - 5, 'b');
+EXPECT_TRUE(
+llvm::sys::commandLineFitsWithinSystemLimits("cl", long_arg.data()));
+long_arg += 'b';
+EXPECT_FALSE(
+llvm::sys::commandLineFitsWithinSystemLimits("cl", long_arg.data()));
+  }
 }
 
 TEST(CommandLineTest, ResponseFileWindows) {
Index: llvm/lib/Support/Windows/Program.inc
===
--- llvm/lib/Support/Windows/Program.inc
+++ llvm/lib/Support/Windows/Program.inc
@@ -189,7 +189,14 @@
   // Windows wants a command line, not an array of args, to pass to the new
   // process.  We have to concatenate them all, while quoting the args that
   // have embedded spaces (or are empty).
-  std::string Command = flattenWindowsCommandLine(Args);
+  auto Result = flattenWindowsCommandLine(Args);
+  if (std::error_code ec = Result.getError()) {
+SetLastError(ec.value());
+MakeErrMsg(ErrMsg,
+   std::string("Unable to convert command-line to UTF-16"));
+return false;
+  }
+  std::wstring Command = *Result;
 
   // The pointer to the environment block for the new process.
   std::vector EnvBlock;
@@ -271,14 +278,8 @@
 return false;
   }
 
-  SmallVector CommandUtf16;
-  if (std::error_code ec = windows::UTF8ToUTF16(Command, CommandUtf16)) {
-SetLastError(ec.value());
-MakeErrMsg(ErrMsg,
-   std::string("Unable to convert command-line to UTF-16"));
-return false;
-  }
-
+  std::vector CommandUtf16(Command.size() + 1, 0);
+  std::copy(Command.begin(), Command.end(), CommandUtf16.begin());
   BOOL rc = CreateProcessW(ProgramUtf16.data(), CommandUtf16.data(), 0, 0,
TRUE, CREATE_UNICODE_ENVIRONMENT,
EnvBlock.empty() ? 0 : EnvBlock.data(), 0, &si,
@@ -376,7 +377,7 @@
 }
 
 namespace llvm {
-std::string sys::flattenWindowsCommandLine(ArrayRef Args) {
+ErrorOr sys::flattenWindowsCommandLine(ArrayRef Args) {
   std::string Command;
   for (StringRef Arg : Args) {
 if (argNeedsQuotes(Arg))
@@ -387,7 +388,11 @@
 Command.push_back(' ');
   }
 
-  return Command;
+  SmallVector CommandUtf16;
+  if (std::error_code ec = windows::UTF8ToUTF16(Command, CommandUtf16))
+return ec;
+
+  return std::wstring(CommandUtf16.begin(), CommandUtf16.end());
 }
 
 ProcessInfo sys::Wait(const ProcessInfo &PI, unsigned SecondsToWait,
@@ -532,12 +537,16 @@
 
 bool llvm::sys::commandLineFitsWithinSystemLimits(StringRef Program,
   ArrayRef Args) {
-  // The documented max length of the command line passed to CreateProcess.
-  static const size_t MaxCommandStringLength = 32768;
+  // The documentation on CreateProcessW states that the size of the argument
+  // lpCommandLine must not be greater than 32767 characters, including the
+  // Unicode terminating null character. We use smaller value to reduce risk
+  // of getting invalid command line due to unaccounted factors.
+  static const size_t MaxCommandStringLength = 32000;
   SmallVector FullArgs;
   FullArgs.push_back(Program);
   FullArgs.append(Args.begin(), Args.end());
-  std::string Result = flattenWindowsCommandLine(FullArgs);
-  return (Result.size() + 1) <= MaxCommandStringLength;
+  auto Result = flattenWindowsCommandLine(FullArgs);
+  assert(!Result.getError());
+  return (Result->size() + 1) <= MaxCommandStringLength;
 }
 }
Index: llvm/include/llvm/Support/Program.h
===
--- llvm/include/llvm/Support/Program.h
+++ llvm/include/llvm/Support/Program.h
@@ -218,7 +218,7 @@
   /// to build a single flat command line appropriate for calling CreateProcess
   /// on
   /// Windows.
-  std::string flattenWindowsCommandLine(ArrayRef Args);
+  ErrorOr flattenWindowsCommandLine(ArrayRef Args);
 #endi

[PATCH] D83817: [clangd] Add remote index support for Clangd itself

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 279472.
kbobyrev added a comment.

Rebase on top of master.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83817

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/Features.inc.in
  clang-tools-extra/clangd/tool/ClangdMain.cpp

Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -14,6 +14,7 @@
 #include "Transport.h"
 #include "index/Background.h"
 #include "index/Serialization.h"
+#include "index/remote/Client.h"
 #include "refactor/Rename.h"
 #include "support/Path.h"
 #include "support/Shutdown.h"
@@ -64,6 +65,7 @@
 OptionCategory Features("clangd feature options");
 OptionCategory Misc("clangd miscellaneous options");
 OptionCategory Protocol("clangd protocol and logging options");
+OptionCategory Remote("clangd remote index options");
 const OptionCategory *ClangdCategories[] = {&Features, &Protocol,
 &CompileCommands, &Misc};
 
@@ -449,6 +451,23 @@
 init(true),
 };
 
+#ifdef CLANGD_ENABLE_REMOTE
+opt RemoteIndexAddress{
+"remote-index-address",
+cat(Remote),
+desc("Address of the remote index server"),
+Hidden,
+};
+
+// FIXME(kirillbobyrev): Should this be the location of compile_commands.json?
+opt ProjectPath{
+"project-path",
+cat(Remote),
+desc("Path to the project root. Requires remote-index-address to be set."),
+Hidden,
+};
+#endif
+
 /// Supports a test URI scheme with relaxed constraints for lit tests.
 /// The path in a test URI will be combined with a platform-specific fake
 /// directory to form an absolute path. For example, test:///a.cpp is resolved
@@ -680,6 +699,20 @@
 if (Sync)
   AsyncIndexLoad.wait();
   }
+  if (RemoteIndexAddress.empty() != ProjectPath.empty()) {
+llvm::errs() << "remote-index-address and project-path have to be "
+"specified at the same time.";
+return 1;
+  }
+  if (!RemoteIndexAddress.empty()) {
+if (!IndexFile.empty()) {
+  llvm::errs() << "When remote index is enabled, IndexFile should not be "
+  "specified. Only one can be used at time.";
+  return 1;
+}
+log("Connecting to remote index at {0}", RemoteIndexAddress);
+StaticIdx = remote::getClient(RemoteIndexAddress, ProjectPath);
+  }
   Opts.StaticIndex = StaticIdx.get();
   Opts.AsyncThreadsCount = WorkerThreadsCount;
   Opts.BuildRecoveryAST = RecoveryAST;
Index: clang-tools-extra/clangd/Features.inc.in
===
--- clang-tools-extra/clangd/Features.inc.in
+++ clang-tools-extra/clangd/Features.inc.in
@@ -1 +1,2 @@
 #define CLANGD_BUILD_XPC @CLANGD_BUILD_XPC@
+#define CLANGD_ENABLE_REMOTE @CLANGD_ENABLE_REMTE@
Index: clang-tools-extra/clangd/CMakeLists.txt
===
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -114,6 +114,15 @@
   omp_gen
   )
 
+# FIXME(kirillbobyrev): Document this in the LLVM docs once remote index is stable.
+option(CLANGD_ENABLE_REMOTE "Use gRPC library to enable remote index support for Clangd" OFF)
+set(GRPC_INSTALL_PATH "" CACHE PATH "Path to gRPC library manual installation.")
+
+if (CLANGD_ENABLE_REMOTE)
+  include(FindGRPC)
+endif()
+add_subdirectory(index/remote)
+
 clang_target_link_libraries(clangDaemon
   PRIVATE
   clangAST
@@ -131,6 +140,7 @@
   clangToolingInclusions
   clangToolingRefactoring
   clangToolingSyntax
+  clangdRemoteIndex
   )
 
 add_subdirectory(refactor/tweaks)
@@ -153,12 +163,4 @@
 add_subdirectory(unittests)
 endif()
 
-# FIXME(kirillbobyrev): Document this in the LLVM docs once remote index is stable.
-option(CLANGD_ENABLE_REMOTE "Use gRPC library to enable remote index support for Clangd" OFF)
-set(GRPC_INSTALL_PATH "" CACHE PATH "Path to gRPC library manual installation.")
-
-if (CLANGD_ENABLE_REMOTE)
-  include(FindGRPC)
-endif()
-add_subdirectory(index/remote)
 add_subdirectory(index/dex/dexp)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81728: [InstCombine] Add target-specific inst combining

[Dave Green via Phabricator via cfe-commits]

dmgreen added inline comments.



Comment at: llvm/test/CodeGen/Thumb2/mve-intrinsics/predicates.ll:2
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: opt -instcombine %s | llc -mtriple=thumbv8.1m.main -mattr=+mve.fp 
-verify-machineinstrs -o - | FileCheck %s
+; RUN: opt -instcombine -mtriple=arm %s | llc -mtriple=thumbv8.1m.main 
-mattr=+mve.fp -verify-machineinstrs -o - | FileCheck %s
 

Please use the same triple as llc for any test with "mve" in the title.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81728



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

[PATCH] D81728: [InstCombine] Add target-specific inst combining

[Dave Green via Phabricator via cfe-commits]

dmgreen added a comment.

Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81728



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

[PATCH] D84213: [clang-tools-extra] Disable -Wsuggest-override for unittests/

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman 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/D84213/new/

https://reviews.llvm.org/D84213



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

[PATCH] D80791: [AArch64] Generate .note.gnu.property based on module flags.

[Momchil Velikov via Phabricator via cfe-commits]

chill added a comment.

I don't think this behaviour is correct with regard to the specification 
(AAELF64 2020Q2):

> Static linkers processing ELF relocatable objects must set the feature bit in 
> the output object or image
>  only if all the input objects have the corresponding feature bit set.



> GNU_PROPERTY_AARCH64_FEATURE_1_BTI This indicates that all executable 
> sections are compatible with
>  Branch Target Identification mechanism. An executable or shared object with 
> this bit set is required to
>  generate Custom PLTs (page 35) with BTI instruction.
> 
> GNU_PROPERTY_AARCH64_FEATURE_1_PAC This indicates that all executable 
> sections have Return Address
>  Signing enabled. An executable or shared object with this bit set can 
> generate Custom PLTs (page 35)
>  with a PAC instruction.

Compatibility of executable sections ultimately depends on each individual 
function, therefore
it cannot be inferred from command-line options alone (transitively from module 
flags), which
merely set a default that can be overridden by function attributes.

If any function has the attribute "sign-return-address", then the output note
section should have PAC bit set. The return address signing is completely local
to the function, and functions with or without return address signing can be
freely mixed with each other.

Likewise, if any function has the attribute "branch-target-enforcement", then
the output note section should have the BTI flag set. Even though code compiled
with BTI is not necessarily compatible with non-BTI code:

- the only way to get BTI code is by explicit use of `-mbranch-protection=...` 
command-line option, or the corresponding attribute, which we should consider a 
clear indication about the user's intent to use BTI.
- the only way to get a mix of present/non-present "branch-target-enforcement" 
attributes is by the explicit use of the 
`__attribute__((target("branch-protection=..."))`, in which case we should 
assume the user knows what they are doing.

What do to if there are no functions in the compile unit?

Technically, objects produced from such a unit are fully compatible with both 
PAC and BTI, which
means both flags should be set. But looking at the (non-existent) function 
attributes alone does
not allow us to unambiguously derive a user's intent to use PAC/BTI. In this 
case, I would suggest
setting the ELF note flags, according to the LLVM IR module flags.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80791



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

[PATCH] D83759: [clangd] Port lit tests to Windows

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX updated this revision to Diff 279482.
ArcsinX added a comment.

Revert tests split; Replace '\' with '/' in background-index.test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83759

Files:
  clang-tools-extra/clangd/test/background-index.test
  clang-tools-extra/clangd/test/did-change-configuration-params.test
  clang-tools-extra/clangd/test/test-uri-windows.test


Index: clang-tools-extra/clangd/test/test-uri-windows.test
===
--- clang-tools-extra/clangd/test/test-uri-windows.test
+++ clang-tools-extra/clangd/test/test-uri-windows.test
@@ -1,5 +1,5 @@
 # RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
-# REQUIRES: windows-gnu || windows-msvc
+# UNSUPPORTED: !(windows-gnu || windows-msvc)
 # Test authority-less URI
 
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
 ---
Index: clang-tools-extra/clangd/test/did-change-configuration-params.test
===
--- clang-tools-extra/clangd/test/did-change-configuration-params.test
+++ clang-tools-extra/clangd/test/did-change-configuration-params.test
@@ -1,5 +1,5 @@
 # RUN: clangd -compile_args_from=lsp -lit-test < %s 2> %t | FileCheck 
-strict-whitespace %s
-# RUN: cat %t | FileCheck --check-prefix=ERR %s
+# RUN: FileCheck --check-prefix=ERR --input-file=%t %s
 # UNSUPPORTED: windows-gnu,windows-msvc
 
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
 ---
Index: clang-tools-extra/clangd/test/background-index.test
===
--- clang-tools-extra/clangd/test/background-index.test
+++ clang-tools-extra/clangd/test/background-index.test
@@ -1,23 +1,23 @@
-# We need to splice paths into file:// URIs for this test.
-# UNSUPPORTED: windows-msvc
-
 # Use a copy of inputs, as we'll mutate it (as will the background index).
-# RUN: rm -rf %t
-# RUN: cp -r %S/Inputs/background-index %t
+# RUN: rm -rf %/t
+# RUN: cp -r %/S/Inputs/background-index %/t
 # Need to embed the correct temp path in the actual JSON-RPC requests.
-# RUN: sed -i -e "s|DIRECTORY|%t|" %t/definition.jsonrpc
-# RUN: sed -i -e "s|DIRECTORY|%t|" %t/compile_commands.json
+# RUN: sed -i -e "s|DIRECTORY|%/t|" %/t/definition.jsonrpc
+# RUN: sed -i -e "s|DIRECTORY|%/t|" %/t/compile_commands.json
+# On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
+# (with the extra slash in the front), so we add it here.
+# RUN: sed -i -E -e 's|"file://([A-Z]):/|"file:///\1:/|g' 
%/t/definition.jsonrpc
 
 # We're editing bar.cpp, which includes foo.h.
 # foo() is declared in foo.h and defined in foo.cpp.
 # The background index should allow us to go-to-definition on foo().
 # We should also see indexing progress notifications.
-# RUN: clangd -background-index -lit-test < %t/definition.jsonrpc | FileCheck 
%t/definition.jsonrpc --check-prefixes=CHECK,BUILD
+# RUN: clangd -background-index -lit-test < %/t/definition.jsonrpc | FileCheck 
%/t/definition.jsonrpc --check-prefixes=CHECK,BUILD
 
 # Test that the index is writing files in the expected location.
-# RUN: ls %t/.cache/clangd/index/foo.cpp.*.idx
-# RUN: ls %t/sub_dir/.cache/clangd/index/foo.h.*.idx
+# RUN: ls %/t/.cache/clangd/index/foo.cpp.*.idx
+# RUN: ls %/t/sub_dir/.cache/clangd/index/foo.h.*.idx
 
 # Test the index is read from disk: delete code and restart clangd.
-# RUN: rm %t/foo.cpp
-# RUN: clangd -background-index -lit-test < %t/definition.jsonrpc | FileCheck 
%t/definition.jsonrpc --check-prefixes=CHECK,USE
+# RUN: rm %/t/foo.cpp
+# RUN: clangd -background-index -lit-test < %/t/definition.jsonrpc | FileCheck 
%/t/definition.jsonrpc --check-prefixes=CHECK,USE


Index: clang-tools-extra/clangd/test/test-uri-windows.test
===
--- clang-tools-extra/clangd/test/test-uri-windows.test
+++ clang-tools-extra/clangd/test/test-uri-windows.test
@@ -1,5 +1,5 @@
 # RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
-# REQUIRES: windows-gnu || windows-msvc
+# UNSUPPORTED: !(windows-gnu || windows-msvc)
 # Test authority-less URI
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
 ---
Index: clang-tools-extra/clangd/test/did-change-configuration-params.test
===
--- clang-tools-extra/clangd/test/did-change-configuration-params.test
+++ clang-tools-extra/clangd/test/did-change-configuration-params.test
@@ -1,5 +1,5 @@
 # RUN: clangd -compile_args_from=lsp -lit-test < %s 2> %t | FileCheck -strict-whitespace %s
-# RUN: cat %t | FileCheck --check-prefix=ERR %s
+# RUN: FileCheck --check-prefix=ERR --input-file=%t %s

[PATCH] D83665: [OpenCL] Fixed missing address space for templated copy constructor

[Ole Strohm via Phabricator via cfe-commits]

olestrohm updated this revision to Diff 279485.
olestrohm added a comment.

The code now directly adds the __generic address space to the pointee type.


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

https://reviews.llvm.org/D83665

Files:
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/test/SemaOpenCLCXX/address-space-templates.cl


Index: clang/test/SemaOpenCLCXX/address-space-templates.cl
===
--- clang/test/SemaOpenCLCXX/address-space-templates.cl
+++ clang/test/SemaOpenCLCXX/address-space-templates.cl
@@ -22,10 +22,28 @@
   __private T ii; // expected-error{{conflicting address space qualifiers are 
provided between types '__private T' and '__global int'}}
 }
 
+template  struct remove_reference { typedef _Tp type; };
+template  struct remove_reference<_Tp &>  { typedef _Tp type; };
+template  struct as_pointer {
+typedef typename remove_reference<_Tp>::type* type;
+};
+
+struct rep {
+  // CHECK |-CXXConstructorDecl {{.*}} rep 'void (const __generic rep 
&__private) __generic'
+  template::type>
+  rep(U&& v) {}
+};
+
+struct rep_outer : private rep {
+  rep_outer()
+: rep(0) {}
+};
+
 void bar() {
   S sintgl; // expected-note{{in instantiation of template 
class 'S' requested here}}
 
   foo1<__local int>(1); // expected-error{{no matching function for call to 
'foo1'}}
   foo2<__global int>(0);
   foo3<__global int>(); // expected-note{{in instantiation of function 
template specialization 'foo3<__global int>' requested here}}
+  rep_outer r;
 }
Index: clang/lib/Sema/SemaTemplateDeduction.cpp
===
--- clang/lib/Sema/SemaTemplateDeduction.cpp
+++ clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -3806,8 +3806,11 @@
 //   If P is a forwarding reference and the argument is an lvalue, the type
 //   "lvalue reference to A" is used in place of A for type deduction.
 if (isForwardingReference(QualType(ParamRefType, 0), FirstInnerIndex) &&
-Arg->isLValue())
+Arg->isLValue()) {
+  if (S.getLangOpts().OpenCL)
+ArgType = S.Context.getAddrSpaceQualType(ArgType, 
LangAS::opencl_generic);
   ArgType = S.Context.getLValueReferenceType(ArgType);
+}
   } else {
 // C++ [temp.deduct.call]p2:
 //   If P is not a reference type:


Index: clang/test/SemaOpenCLCXX/address-space-templates.cl
===
--- clang/test/SemaOpenCLCXX/address-space-templates.cl
+++ clang/test/SemaOpenCLCXX/address-space-templates.cl
@@ -22,10 +22,28 @@
   __private T ii; // expected-error{{conflicting address space qualifiers are provided between types '__private T' and '__global int'}}
 }
 
+template  struct remove_reference { typedef _Tp type; };
+template  struct remove_reference<_Tp &>  { typedef _Tp type; };
+template  struct as_pointer {
+typedef typename remove_reference<_Tp>::type* type;
+};
+
+struct rep {
+  // CHECK |-CXXConstructorDecl {{.*}} rep 'void (const __generic rep &__private) __generic'
+  template::type>
+  rep(U&& v) {}
+};
+
+struct rep_outer : private rep {
+  rep_outer()
+: rep(0) {}
+};
+
 void bar() {
   S sintgl; // expected-note{{in instantiation of template class 'S' requested here}}
 
   foo1<__local int>(1); // expected-error{{no matching function for call to 'foo1'}}
   foo2<__global int>(0);
   foo3<__global int>(); // expected-note{{in instantiation of function template specialization 'foo3<__global int>' requested here}}
+  rep_outer r;
 }
Index: clang/lib/Sema/SemaTemplateDeduction.cpp
===
--- clang/lib/Sema/SemaTemplateDeduction.cpp
+++ clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -3806,8 +3806,11 @@
 //   If P is a forwarding reference and the argument is an lvalue, the type
 //   "lvalue reference to A" is used in place of A for type deduction.
 if (isForwardingReference(QualType(ParamRefType, 0), FirstInnerIndex) &&
-Arg->isLValue())
+Arg->isLValue()) {
+  if (S.getLangOpts().OpenCL)
+ArgType = S.Context.getAddrSpaceQualType(ArgType, LangAS::opencl_generic);
   ArgType = S.Context.getLValueReferenceType(ArgType);
+}
   } else {
 // C++ [temp.deduct.call]p2:
 //   If P is not a reference type:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82948: [Driver][ARM] Disable unsupported features when nofp arch extension is used

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:476
+// -mfpu=none, -march=armvX+nofp or -mcpu=X+nofp is *very* similar to
+// -mfloat-abi=soft, only that it should not disable MVE-I.
 Features.insert(Features.end(),

Why not disable MVE-I? I assume because it's integer only but then why does 
-mfloat-abi=soft disable it?

If possible add a regression test for this. In general a test like the bf16 
test below, but for all the listed extensions would help. Perhaps it makes more 
sense to add a driver test that looks for the "-" bits in the -### output 
instead of doing each extension on its own.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82948



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

Re: [clang-tools-extra] 9791416 - Silence a "logical operation on address of string constant" via CMake instead.

[Aaron Ballman via cfe-commits]

On Mon, Jul 20, 2020 at 5:44 PM David Blaikie  wrote:
>
> Should the warning be disabled for LLVM overall, rather than only for
> this subproject? (& be good tohave some details in the commit at least
> of why this warning is being disabled - how it is noisy/unhelpful/etc)

Thank you for pointing out the lack of details in the commit message,
sorry about that! I didn't think it should be disabled for LLVM
overall because the issue was highly local to that particular file.
Effectively, there's a #define/#include/#undef pattern in that file
where some of the macro expansions would expand out to "string
literal" == nullptr, which MSVC would complain about. Replacing the
macro expansion with different code caused other diagnostics, so this
was an expedient solution. I think if we found a second place where we
need to disable the diagnostic, it might make sense to disable it
globally at that point, but I wouldn't be strongly opposed to
silencing it globally if someone felt strongly.

~Aaron

>
> On Sun, Jul 19, 2020 at 8:20 AM Aaron Ballman via cfe-commits
>  wrote:
> >
> >
> > Author: Aaron Ballman
> > Date: 2020-07-19T11:19:48-04:00
> > New Revision: 97914164f8454e745219566d58479b5762cccd51
> >
> > URL: 
> > https://github.com/llvm/llvm-project/commit/97914164f8454e745219566d58479b5762cccd51
> > DIFF: 
> > https://github.com/llvm/llvm-project/commit/97914164f8454e745219566d58479b5762cccd51.diff
> >
> > LOG: Silence a "logical operation on address of string constant" via CMake 
> > instead.
> >
> > Added:
> >
> >
> > Modified:
> > clang-tools-extra/clangd/CMakeLists.txt
> >
> > Removed:
> >
> >
> >
> > 
> > diff  --git a/clang-tools-extra/clangd/CMakeLists.txt 
> > b/clang-tools-extra/clangd/CMakeLists.txt
> > index b3002b1d5698..8db6656e5291 100644
> > --- a/clang-tools-extra/clangd/CMakeLists.txt
> > +++ b/clang-tools-extra/clangd/CMakeLists.txt
> > @@ -28,6 +28,10 @@ set(LLVM_LINK_COMPONENTS
> >Option
> >)
> >
> > +if(MSVC AND NOT CLANG_CL)
> > + set_source_files_properties(CompileCommands.cpp PROPERTIES COMPILE_FLAGS 
> > -wd4130) # disables C4130: logical operation on address of string constant
> > +endif()
> > +
> >  add_clang_library(clangDaemon
> >AST.cpp
> >ClangdLSPServer.cpp
> >
> >
> >
> > ___
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82948: [Driver][ARM] Disable unsupported features when nofp arch extension is used

[Momchil Velikov via Phabricator via cfe-commits]

chill added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:476
+// -mfpu=none, -march=armvX+nofp or -mcpu=X+nofp is *very* similar to
+// -mfloat-abi=soft, only that it should not disable MVE-I.
 Features.insert(Features.end(),

DavidSpickett wrote:
> Why not disable MVE-I? I assume because it's integer only but then why does 
> -mfloat-abi=soft disable it?
> 
> If possible add a regression test for this. In general a test like the bf16 
> test below, but for all the listed extensions would help. Perhaps it makes 
> more sense to add a driver test that looks for the "-" bits in the -### 
> output instead of doing each extension on its own.
> Why not disable MVE-I?

After MVE, "FPU" registers are a separate entity from the FPU.

`-mfpu=none`/`+nofp` disable the FPU. MVE-I does not require an FPU.
`-mfloat-abi=soft` disables both the FPU instructions and the FPU registers.
MVE-I requires "FPU" registers.

It's possible to define different semantics, but this is the one we agreed with 
GCC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82948



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

[PATCH] D84232: [clangd] Set minimum gRPC version to 1.27

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 279497.
kbobyrev added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Check gRPC version for system-installed APT package on Debian-like Linux
distros.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84232

Files:
  clang-tools-extra/clangd/index/remote/README.md
  llvm/cmake/modules/FindGRPC.cmake


Index: llvm/cmake/modules/FindGRPC.cmake
===
--- llvm/cmake/modules/FindGRPC.cmake
+++ llvm/cmake/modules/FindGRPC.cmake
@@ -6,7 +6,7 @@
   set(protobuf_MODULE_COMPATIBLE TRUE)
   find_package(Protobuf CONFIG REQUIRED HINTS ${GRPC_INSTALL_PATH})
   message(STATUS "Using protobuf ${protobuf_VERSION}")
-  find_package(gRPC CONFIG REQUIRED HINTS ${GRPC_INSTALL_PATH})
+  find_package(gRPC 1.27 CONFIG REQUIRED HINTS ${GRPC_INSTALL_PATH})
   message(STATUS "Using gRPC ${gRPC_VERSION}")
 
   include_directories(${Protobuf_INCLUDE_DIRS})
@@ -29,6 +29,7 @@
   # On macOS the libraries are typically installed via Homebrew and are not on
   # the system path.
   if (${APPLE})
+# FIXME(kirillbobyrev): Check gRPC version for macOS, too.
 find_program(HOMEBREW brew)
 # If Homebrew is not found, the user might have installed libraries
 # manually. Fall back to the system path.
@@ -52,6 +53,24 @@
 link_directories(${PROTOBUF_HOMEBREW_PATH}/lib)
   endif()
 endif()
+  elseif(CMAKE_HOST_SYSTEM_NAME STREQUAL "Linux")
+# Try to figure out gRPC version using APT for Debian-like Linux
+# distributions.
+find_program(APT apt)
+if (NOT APT-NOTFOUND)
+  execute_process(COMMAND ${APT} policy libgrpc++-dev
+OUTPUT_VARIABLE APT_GRPC_POLICY
+RESULT_VARIABLE APT_GRPC_POLICY_RETURN_CODE)
+  if (APT_GRPC_POLICY_RETURN_CODE EQUAL "0")
+# Parse MAJOR.MINOR gRPC version.
+string(REGEX MATCH "Installed: ([0-9]+\\.[0-9]+)"
+  gRPC_VERSION ${APT_GRPC_POLICY})
+if (CMAKE_MATCH_1 VERSION_LESS "1.27")
+  message(FATAL_ERROR "gRPC version rerquied: >=1.27, found: 
${CMAKE_MATCH_1}")
+endif()
+message(STATUS "Using gRPC ${gRPC_VERSION}")
+  endif()
+endif()
   endif()
 endif()
 
Index: clang-tools-extra/clangd/index/remote/README.md
===
--- clang-tools-extra/clangd/index/remote/README.md
+++ clang-tools-extra/clangd/index/remote/README.md
@@ -11,8 +11,9 @@
 
 ## Building
 
-This feature uses gRPC and Protobuf libraries, so you will need to install 
them.
-There are two ways of doing that.
+This feature uses gRPC (known to work with versions starting from 1.27.0) and
+Protobuf libraries, so you will need to install them. There are two ways of
+doing that.
 
 However you install dependencies, to enable this feature and build remote index
 tools you will need to set this CMake flag — `-DCLANGD_ENABLE_REMOTE=On`.


Index: llvm/cmake/modules/FindGRPC.cmake
===
--- llvm/cmake/modules/FindGRPC.cmake
+++ llvm/cmake/modules/FindGRPC.cmake
@@ -6,7 +6,7 @@
   set(protobuf_MODULE_COMPATIBLE TRUE)
   find_package(Protobuf CONFIG REQUIRED HINTS ${GRPC_INSTALL_PATH})
   message(STATUS "Using protobuf ${protobuf_VERSION}")
-  find_package(gRPC CONFIG REQUIRED HINTS ${GRPC_INSTALL_PATH})
+  find_package(gRPC 1.27 CONFIG REQUIRED HINTS ${GRPC_INSTALL_PATH})
   message(STATUS "Using gRPC ${gRPC_VERSION}")
 
   include_directories(${Protobuf_INCLUDE_DIRS})
@@ -29,6 +29,7 @@
   # On macOS the libraries are typically installed via Homebrew and are not on
   # the system path.
   if (${APPLE})
+# FIXME(kirillbobyrev): Check gRPC version for macOS, too.
 find_program(HOMEBREW brew)
 # If Homebrew is not found, the user might have installed libraries
 # manually. Fall back to the system path.
@@ -52,6 +53,24 @@
 link_directories(${PROTOBUF_HOMEBREW_PATH}/lib)
   endif()
 endif()
+  elseif(CMAKE_HOST_SYSTEM_NAME STREQUAL "Linux")
+# Try to figure out gRPC version using APT for Debian-like Linux
+# distributions.
+find_program(APT apt)
+if (NOT APT-NOTFOUND)
+  execute_process(COMMAND ${APT} policy libgrpc++-dev
+OUTPUT_VARIABLE APT_GRPC_POLICY
+RESULT_VARIABLE APT_GRPC_POLICY_RETURN_CODE)
+  if (APT_GRPC_POLICY_RETURN_CODE EQUAL "0")
+# Parse MAJOR.MINOR gRPC version.
+string(REGEX MATCH "Installed: ([0-9]+\\.[0-9]+)"
+  gRPC_VERSION ${APT_GRPC_POLICY})
+if (CMAKE_MATCH_1 VERSION_LESS "1.27")
+  message(FATAL_ERROR "gRPC version rerquied: >=1.27, found: ${CMAKE_MATCH_1}")
+endif()
+message(STATUS "Using gRPC ${gRPC_VERSION}")
+  endif()
+endif()
   endif()
 endif()
 
Index: clang-tools-extra/clangd/index/remote/README.md
==

[PATCH] D84090: [clang-format] Add BitFieldColonSpacing option

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

In D84090#2162389 , @MyDeveloperDay 
wrote:

> Nit:clang-format the patch
>
> Here is a Tip as to what I do (In case it helps other):
>
> 1. I generate the diff off the staged items so I've already git added all the 
> files I'm working on.
> 2. then I can do `git clang-format`, this will fix up any files in the diff 
> that need formatting (you'll need to git add them again if they have)
> 3. I then check the documentation builds with `/usr/bin/sphinx-build -n 
> ./docs ./html`  (if the review contains rst files)
> 4. then I do  `git diff --cached -U99 > patch_to_submitt.diff`
> 5. I check the patch to ensure I'm not changing the mode of the files with 
> `grep -A0 -B2 "new mode" patch_to_submitt.diff`
> 6. and this is the patch_tosubmitt.diff I upload to the review
>
>   These steps try to reduce the number of review fails I get based on 
> clang-format issues (all of this is scripted so making a patch is repeatable, 
> quick and easy and all based off whats in my staged area)
>
>   Apart from that this patch LGTM (Before and After are good names)
>
>   But please clang-format before pushing (including the tests)


That's useful, thank you! I was not aware of `git clang-format`, which unlike 
`clang-format-diff` seems to respect `.clang-format`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84090



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

[PATCH] D83970: [ASTImporter] Refactor ASTImporter to support custom downstream tests

[Gabor Marton via Phabricator via cfe-commits]

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

Ok, LGTM! Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83970



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

[clang] 7b5bddf - [clang] Partially revert "Disable a few formatting options for test/"

[Bruno Ricci via cfe-commits]

Author: Bruno Ricci
Date: 2020-07-21T14:53:37+01:00
New Revision: 7b5bddfd034ef42c92c67731743399df844d5f43

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

LOG: [clang] Partially revert "Disable a few formatting options for test/"

The changes to "AlignTrailingComments" and "CommentPragmas" did not
result in what I expected (just leave the special comments alone).

Instead now the following:
  void test() {
int i; // expected-error
   // expected-warning
  }

is formatted into:
  void test() {
int i; // expected-error
// expected-warning
  }

which is even worse.

Added: 


Modified: 
clang/test/.clang-format

Removed: 




diff  --git a/clang/test/.clang-format b/clang/test/.clang-format
index a6176c2e0013..f7fb083d1055 100644
--- a/clang/test/.clang-format
+++ b/clang/test/.clang-format
@@ -1,5 +1,3 @@
 BasedOnStyle: LLVM
 ColumnLimit: 0
-AlignTrailingComments: false
-CommentPragmas: "(^ ?CHECK|^ ?expected-)"
 AlwaysBreakTemplateDeclarations: No



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

[PATCH] D83652: Merge some of the PCH object support with modular codegen

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

> Thanks for checking! So, in summary: This seems to work fine for Chromium, 
> but Chromium isn't really exercising this functionality (only insofar as the 
> functionality is enabled, but essentially a no-op)?

Yup.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83652



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

[PATCH] D81385: Fix libdl linking for libclang in standalone mode

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In D81385#2162911 , @mstorsjo wrote:

> Ping @beanz
>
> @hans - I think this is something that would be wanted to have fixed in the 
> release branch (it's a regression for certain build configurations, afaik) - 
> the fix awaits an ack from @beanz.


Thanks, I've put it on my list. It needs to land first though :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81385



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

[PATCH] D83992: [ASTImporter] Add Visitor for TypedefNameDecl's

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers added a comment.

I discussed with Gabor. We don't feel comfortable landing this without a 
covering negative test case, so I'll work on that a little more, see what I can 
come up with. Thanks Gabor!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83992



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

[PATCH] D81385: Fix libdl linking for libclang in standalone mode

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D81385#2164235 , @hans wrote:

> In D81385#2162911 , @mstorsjo wrote:
>
> > Ping @beanz
> >
> > @hans - I think this is something that would be wanted to have fixed in the 
> > release branch (it's a regression for certain build configurations, afaik) 
> > - the fix awaits an ack from @beanz.
>
>
> Thanks, I've put it on my list. It needs to land first though :)


I think that was enough of a headroom to provide feedback.
Please proceed to commit, there's always post-commit review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81385



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

[PATCH] D82699: [driver][arm64] Set target CPU to A12 for compiler invocations that target Apple Silicon

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

We're seeing some build warnings in ffmpeg due to this not being in yet. Could 
you try to land it soon?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82699



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

[PATCH] D83759: [clangd] Fixes in lit tests

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks, LGTM!

as i mentioned please watch out for buildbot breakages after landing this. (and 
again let me know if I should land this for you)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83759



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

[PATCH] D83190: [analyzer] Model iterator random incrementation symmetrically

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 added a comment.

I experienced 2 crashes with and without this patch using commit 
`1af9fc82132da7c876e8f70c4e986cc9c59010ee` on master:
I have used the clang built on that revision to analyse itself, and also used 
the patched version (with this current revision applied) to do the same.

  clang-12: 
/mnt/ssd/zfulend/llvm-project/clang/lib/Analysis/PathDiagnostic.cpp:399: bool 
compare(const clang::ento::PathDiagnostic&, const 
clang::ento::PathDiagnostic&): Assertion `b.hasValue()' failed.
  PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash 
backtrace, preprocessed source, and associated run script.
  Stack dump:
  0.  Program arguments: /mnt/ssd/zfulend/clang-rwa/bin/clang-12 --analyze 
-Qunused-arguments -Xclang -analyzer-opt-analyze-headers -Xclang 
-analyzer-output=plist-multi-file -o 
/mnt/ssd/zfulend/symm_with_patch/Verifier.cpp_clangsa_5e3a41accb2e401b7b63c8ab1892a17d.plist
 -Xclang -analyzer-config -Xclang expand-macros=true -Xclang 
-analyzer-checker=alpha.cplusplus.InvalidatedIterator -Xclang 
-analyzer-checker=alpha.cplusplus.IteratorModeling -Xclang 
-analyzer-checker=alpha.cplusplus.IteratorRange -Xclang 
-analyzer-checker=alpha.cplusplus.MismatchedIterator -Xclang 
-analyzer-checker=apiModeling.StdCLibraryFunctions -Xclang 
-analyzer-checker=apiModeling.TrustNonnull -Xclang 
-analyzer-checker=apiModeling.google.GTest -Xclang 
-analyzer-checker=apiModeling.llvm.CastValue -Xclang 
-analyzer-checker=apiModeling.llvm.ReturnValue -Xclang 
-analyzer-checker=core.CallAndMessage -Xclang 
-analyzer-checker=core.CallAndMessageModeling -Xclang 
-analyzer-checker=core.DivideZero -Xclang 
-analyzer-checker=core.DynamicTypePropagation -Xclang 
-analyzer-checker=core.NonNullParamChecker -Xclang 
-analyzer-checker=core.NonnilStringConstants -Xclang 
-analyzer-checker=core.NullDereference -Xclang 
-analyzer-checker=core.StackAddrEscapeBase -Xclang 
-analyzer-checker=core.StackAddressEscape -Xclang 
-analyzer-checker=core.UndefinedBinaryOperatorResult -Xclang 
-analyzer-checker=core.VLASize -Xclang 
-analyzer-checker=core.builtin.BuiltinFunctions -Xclang 
-analyzer-checker=core.builtin.NoReturnFunctions -Xclang 
-analyzer-checker=core.uninitialized.ArraySubscript -Xclang 
-analyzer-checker=core.uninitialized.Assign -Xclang 
-analyzer-checker=core.uninitialized.Branch -Xclang 
-analyzer-checker=core.uninitialized.CapturedBlockVariable -Xclang 
-analyzer-checker=core.uninitialized.UndefReturn -Xclang 
-analyzer-checker=cplusplus.InnerPointer -Xclang 
-analyzer-checker=cplusplus.Move -Xclang -analyzer-checker=cplusplus.NewDelete 
-Xclang -analyzer-checker=cplusplus.NewDeleteLeaks -Xclang 
-analyzer-checker=cplusplus.PlacementNew -Xclang 
-analyzer-checker=cplusplus.PureVirtualCall -Xclang 
-analyzer-checker=cplusplus.SelfAssignment -Xclang 
-analyzer-checker=cplusplus.SmartPtrModeling -Xclang 
-analyzer-checker=cplusplus.VirtualCallModeling -Xclang 
-analyzer-checker=deadcode.DeadStores -Xclang 
-analyzer-checker=nullability.NullPassedToNonnull -Xclang 
-analyzer-checker=nullability.NullReturnedFromNonnull -Xclang 
-analyzer-checker=optin.cplusplus.UninitializedObject -Xclang 
-analyzer-checker=optin.cplusplus.VirtualCall -Xclang 
-analyzer-checker=optin.portability.UnixAPI -Xclang 
-analyzer-checker=security.FloatLoopCounter -Xclang 
-analyzer-checker=security.insecureAPI.UncheckedReturn -Xclang 
-analyzer-checker=security.insecureAPI.getpw -Xclang 
-analyzer-checker=security.insecureAPI.gets -Xclang 
-analyzer-checker=security.insecureAPI.mkstemp -Xclang 
-analyzer-checker=security.insecureAPI.mktemp -Xclang 
-analyzer-checker=security.insecureAPI.rand -Xclang 
-analyzer-checker=security.insecureAPI.vfork -Xclang -analyzer-checker=unix.API 
-Xclang -analyzer-checker=unix.DynamicMemoryModeling -Xclang 
-analyzer-checker=unix.Malloc -Xclang -analyzer-checker=unix.MallocSizeof 
-Xclang -analyzer-checker=unix.MismatchedDeallocator -Xclang 
-analyzer-checker=unix.Vfork -Xclang -analyzer-checker=unix.cstring.BadSizeArg 
-Xclang -analyzer-checker=unix.cstring.CStringModeling -Xclang 
-analyzer-checker=unix.cstring.NullArg -Xclang 
-analyzer-checker=valist.CopyToSelf -Xclang 
-analyzer-checker=valist.Uninitialized -Xclang 
-analyzer-checker=valist.Unterminated -Xclang 
-analyzer-checker=valist.ValistBase -Xclang -analyzer-config -Xclang 
aggressive-binary-operation-simplification=true -x c++ 
--target=x86_64-linux-gnu -std=gnu++14 -DGTEST_HAS_RTTI=0 -D_DEBUG 
-D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS 
-D__STDC_LIMIT_MACROS -I/mnt/ssd/zfulend/clang-rwa/lib/IR 
-I/mnt/ssd/zfulend/llvm-project/llvm/lib/IR -I/usr/include/libxml2 
-I/mnt/ssd/zfulend/clang-rwa/include 
-I/mnt/ssd/zfulend/llvm-project/llvm/include -fPIC -fvisibility-inlines-hidden 
-Wno-unused-parameter -Wno-missing-field-initializers -pedantic -Wno-long-long 
-Wno-maybe-uninitialized -Wno-noexcept-type -Wno-comment -fdiagnostics-color 
-ffunction-sections -fdata-sections -O3 -fno-exceptions -fno-r

[PATCH] D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist

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

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

In D82598#2162496 , @xazax.hun wrote:

> In D82598#2162239 , @Szelethus wrote:
>
> > I chased my own tail for weeks before realizing that there is indeed 
> > another instance when a live **statement** is stored, other then 
> > `ObjCForCollectionStmt`...
> >
> >   void clang_analyzer_eval(bool);
> >  
> >   void test_lambda_refcapture() {
> > int a = 6;
> > [&](int &a) { a = 42; }(a);
> > clang_analyzer_eval(a == 42); // expected-warning{{TRUE}}
> >   }
> >
>
>
> Hmm, interesting. I don't really understand why do we need to keep that block 
> live, as we definitely won't use any of the value it provides (since it does 
> not provide a value at all).


Actually, what I said initially is true:

> [...] the only non-expression statements it **queried** are 
> ObjCForCollectionStmts [...]

so I think it'd be okay to simply drop this. Also, its easy to see why this 
popped up, because... (cont. in inline)




Comment at: clang/lib/Analysis/LiveVariables.cpp:317-320
   for (Stmt *Child : S->children()) {
-if (Child)
-  AddLiveStmt(val.liveStmts, LV.SSetFact, Child);
+if (const auto *E = dyn_cast_or_null(Child))
+  AddLiveExpr(val.liveExprs, LV.SSetFact, E);
   }

..this part of the code caused the issue. Looking at the related CFG,
```lang=c++
void test_lambda_refcapture()
 [B2 (ENTRY)]
   Succs (1): B1

 [B1]
   1: 6
   2: int a = 6;
   3: operator()
   4: [B1.3] (ImplicitCastExpr, FunctionToPointerDecay, void (*)(int &) const)
   5: [&](int &a) {
a = 42;
}
   6: [B1.5]
   7: [B1.6] (ImplicitCastExpr, NoOp, const class (lambda at 
/home/szelethus/Documents/llvm-project/clang/test/Analysis/live-stmts.cpp:183:3))
   8: a
   9: [B1.7]([B1.8]) (OperatorCall)
  10: clang_analyzer_eval
  11: [B1.10] (ImplicitCastExpr, FunctionToPointerDecay, void (*)(_Bool))
  12: a
  13: [B1.12] (ImplicitCastExpr, LValueToRValue, int)
  14: 42
  15: [B1.13] == [B1.14]
  16: [B1.11]([B1.15])
   Preds (1): B2
   Succs (1): B0

 [B0 (EXIT)]
   Preds (1): B1
```
its clear that element 5 added the live statement, and I think that that this 
entire CFG just simply isn't right. Shouldn't we have a distinct element for 
the assignment?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598



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

[PATCH] D84182: [OPENMP]Fix PR46012: declare target pointer cannot be accessed in target region.

[Ye Luo via Phabricator via cfe-commits]

ye-luo accepted this revision.
ye-luo added a comment.
This revision is now accepted and ready to land.

I verified that 46012 is fixed with this patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84182



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

[clang] 2015741 - [ASTImporter] Refactor ASTImporter to support custom downstream tests

[via cfe-commits]

Author: Vince Bridgers
Date: 2020-07-21T10:34:17-05:00
New Revision: 20157410862d376c624cc24bffd9730290a16142

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

LOG: [ASTImporter] Refactor ASTImporter to support custom downstream tests

Summary:
The purpose of this change is to do a small refactoring of code in
ASTImporterTest.cpp by moving it to ASTImporterFixtures.h in order to
support tests of downstream custom types and minimize the "living
downstream burden" of frequent integrations from community to a
downstream repo that implements custom AST import tests.

Reviewers: martong, a.sidorin, shafik

Reviewed By: martong

Subscribers: balazske, dkrupp, bjope, rnkovacs, teemperor, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/unittests/AST/ASTImporterFixtures.h
clang/unittests/AST/ASTImporterTest.cpp

Removed: 




diff  --git a/clang/unittests/AST/ASTImporterFixtures.h 
b/clang/unittests/AST/ASTImporterFixtures.h
index 4fbceef39384..a63a98b7654c 100644
--- a/clang/unittests/AST/ASTImporterFixtures.h
+++ b/clang/unittests/AST/ASTImporterFixtures.h
@@ -24,6 +24,7 @@
 #include "llvm/Support/ErrorHandling.h"
 
 #include "DeclMatcher.h"
+#include "MatchVerifier.h"
 
 #include 
 
@@ -202,6 +203,229 @@ class ASTImporterOptionSpecificTestBase
   std::vector getExtraArgs() const override { return GetParam(); }
 };
 
+// Base class for those tests which use the family of `testImport` functions.
+class TestImportBase
+: public CompilerOptionSpecificTest,
+  public ::testing::WithParamInterface> {
+
+  template 
+  llvm::Expected importNode(ASTUnit *From, ASTUnit *To,
+  ASTImporter &Importer, NodeType Node) {
+ASTContext &ToCtx = To->getASTContext();
+
+// Add 'From' file to virtual file system so importer can 'find' it
+// while importing SourceLocations. It is safe to add same file multiple
+// times - it just isn't replaced.
+StringRef FromFileName = From->getMainFileName();
+createVirtualFileIfNeeded(To, FromFileName,
+  From->getBufferForFile(FromFileName));
+
+auto Imported = Importer.Import(Node);
+
+if (Imported) {
+  // This should dump source locations and assert if some source locations
+  // were not imported.
+  SmallString<1024> ImportChecker;
+  llvm::raw_svector_ostream ToNothing(ImportChecker);
+  ToCtx.getTranslationUnitDecl()->print(ToNothing);
+
+  // This traverses the AST to catch certain bugs like poorly or not
+  // implemented subtrees.
+  (*Imported)->dump(ToNothing);
+}
+
+return Imported;
+  }
+
+  template 
+  testing::AssertionResult
+  testImport(const std::string &FromCode,
+ const std::vector &FromArgs,
+ const std::string &ToCode, const std::vector &ToArgs,
+ MatchVerifier &Verifier,
+ const internal::BindableMatcher &SearchMatcher,
+ const internal::BindableMatcher &VerificationMatcher) {
+const char *const InputFileName = "input.cc";
+const char *const OutputFileName = "output.cc";
+
+std::unique_ptr FromAST = tooling::buildASTFromCodeWithArgs(
+ FromCode, FromArgs, InputFileName),
+ ToAST = tooling::buildASTFromCodeWithArgs(
+ ToCode, ToArgs, OutputFileName);
+
+ASTContext &FromCtx = FromAST->getASTContext(),
+   &ToCtx = ToAST->getASTContext();
+
+ASTImporter Importer(ToCtx, ToAST->getFileManager(), FromCtx,
+ FromAST->getFileManager(), false);
+
+auto FoundNodes = match(SearchMatcher, FromCtx);
+if (FoundNodes.size() != 1)
+  return testing::AssertionFailure()
+ << "Multiple potential nodes were found!";
+
+auto ToImport = selectFirst(DeclToImportID, FoundNodes);
+if (!ToImport)
+  return testing::AssertionFailure() << "Node type mismatch!";
+
+// Sanity check: the node being imported should match in the same way as
+// the result node.
+internal::BindableMatcher WrapperMatcher(VerificationMatcher);
+EXPECT_TRUE(Verifier.match(ToImport, WrapperMatcher));
+
+auto Imported = importNode(FromAST.get(), ToAST.get(), Importer, ToImport);
+if (!Imported) {
+  std::string ErrorText;
+  handleAllErrors(
+  Imported.takeError(),
+  [&ErrorText](const ImportError &Err) { ErrorText = Err.message(); });
+  return testing::AssertionFailure()
+ << "Import failed, error: \"" << ErrorText << "\"!";
+}
+
+return Verifier.match(*Imported, WrapperMatcher);
+  }
+
+  template 
+  testing::AssertionResult
+  testImport(const std::string &FromCode,
+

[PATCH] D83970: [ASTImporter] Refactor ASTImporter to support custom downstream tests

[Vince Bridgers via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG20157410862d: [ASTImporter] Refactor ASTImporter to support 
custom downstream tests (authored by vabridgers, committed by einvbri 
).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83970

Files:
  clang/unittests/AST/ASTImporterFixtures.h
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -18,7 +18,6 @@
 #include "gtest/gtest.h"
 
 #include "ASTImporterFixtures.h"
-#include "MatchVerifier.h"
 
 namespace clang {
 namespace ast_matchers {
@@ -27,230 +26,6 @@
 using internal::BindableMatcher;
 using llvm::StringMap;
 
-// Base class for those tests which use the family of `testImport` functions.
-class TestImportBase
-: public CompilerOptionSpecificTest,
-  public ::testing::WithParamInterface> {
-
-  template 
-  llvm::Expected importNode(ASTUnit *From, ASTUnit *To,
-  ASTImporter &Importer, NodeType Node) {
-ASTContext &ToCtx = To->getASTContext();
-
-// Add 'From' file to virtual file system so importer can 'find' it
-// while importing SourceLocations. It is safe to add same file multiple
-// times - it just isn't replaced.
-StringRef FromFileName = From->getMainFileName();
-createVirtualFileIfNeeded(To, FromFileName,
-  From->getBufferForFile(FromFileName));
-
-auto Imported = Importer.Import(Node);
-
-if (Imported) {
-  // This should dump source locations and assert if some source locations
-  // were not imported.
-  SmallString<1024> ImportChecker;
-  llvm::raw_svector_ostream ToNothing(ImportChecker);
-  ToCtx.getTranslationUnitDecl()->print(ToNothing);
-
-  // This traverses the AST to catch certain bugs like poorly or not
-  // implemented subtrees.
-  (*Imported)->dump(ToNothing);
-}
-
-return Imported;
-  }
-
-  template 
-  testing::AssertionResult
-  testImport(const std::string &FromCode,
- const std::vector &FromArgs,
- const std::string &ToCode, const std::vector &ToArgs,
- MatchVerifier &Verifier,
- const BindableMatcher &SearchMatcher,
- const BindableMatcher &VerificationMatcher) {
-const char *const InputFileName = "input.cc";
-const char *const OutputFileName = "output.cc";
-
-std::unique_ptr FromAST = tooling::buildASTFromCodeWithArgs(
- FromCode, FromArgs, InputFileName),
- ToAST = tooling::buildASTFromCodeWithArgs(
- ToCode, ToArgs, OutputFileName);
-
-ASTContext &FromCtx = FromAST->getASTContext(),
-   &ToCtx = ToAST->getASTContext();
-
-ASTImporter Importer(ToCtx, ToAST->getFileManager(), FromCtx,
- FromAST->getFileManager(), false);
-
-auto FoundNodes = match(SearchMatcher, FromCtx);
-if (FoundNodes.size() != 1)
-  return testing::AssertionFailure()
- << "Multiple potential nodes were found!";
-
-auto ToImport = selectFirst(DeclToImportID, FoundNodes);
-if (!ToImport)
-  return testing::AssertionFailure() << "Node type mismatch!";
-
-// Sanity check: the node being imported should match in the same way as
-// the result node.
-BindableMatcher WrapperMatcher(VerificationMatcher);
-EXPECT_TRUE(Verifier.match(ToImport, WrapperMatcher));
-
-auto Imported = importNode(FromAST.get(), ToAST.get(), Importer, ToImport);
-if (!Imported) {
-  std::string ErrorText;
-  handleAllErrors(
-  Imported.takeError(),
-  [&ErrorText](const ImportError &Err) { ErrorText = Err.message(); });
-  return testing::AssertionFailure()
- << "Import failed, error: \"" << ErrorText << "\"!";
-}
-
-return Verifier.match(*Imported, WrapperMatcher);
-  }
-
-  template 
-  testing::AssertionResult
-  testImport(const std::string &FromCode,
- const std::vector &FromArgs,
- const std::string &ToCode, const std::vector &ToArgs,
- MatchVerifier &Verifier,
- const BindableMatcher &VerificationMatcher) {
-return testImport(
-FromCode, FromArgs, ToCode, ToArgs, Verifier,
-translationUnitDecl(
-has(namedDecl(hasName(DeclToImportID)).bind(DeclToImportID))),
-VerificationMatcher);
-  }
-
-protected:
-  std::vector getExtraArgs() const override { return GetParam(); }
-
-public:
-
-  /// Test how AST node named "declToImport" located in the translation unit
-  /// of "FromCode" virtual file is imported to "ToCode" virtual file.
-  /// The verification is done by running AMatcher over the

[Differential] D83970: [ASTImporter] Refactor ASTImporter to support custom downstream tests

[Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG20157410862d: [ASTImporter] Refactor ASTImporter to support 
custom downstream tests (authored by vabridgers, committed by einvbri 
).

Repository:
  rG LLVM Github Monorepo

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


  https://reviews.llvm.org/D83970

Files:
  clang/unittests/AST/ASTImporterFixtures.h
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -18,7 +18,6 @@
 #include "gtest/gtest.h"
 
 #include "ASTImporterFixtures.h"
-#include "MatchVerifier.h"
 
 namespace clang {
 namespace ast_matchers {
@@ -27,230 +26,6 @@
 using internal::BindableMatcher;
 using llvm::StringMap;
 
-// Base class for those tests which use the family of `testImport` functions.
-class TestImportBase
-: public CompilerOptionSpecificTest,
-  public ::testing::WithParamInterface> {
-
-  template 
-  llvm::Expected importNode(ASTUnit *From, ASTUnit *To,
-  ASTImporter &Importer, NodeType Node) {
-ASTContext &ToCtx = To->getASTContext();
-
-// Add 'From' file to virtual file system so importer can 'find' it
-// while importing SourceLocations. It is safe to add same file multiple
-// times - it just isn't replaced.
-StringRef FromFileName = From->getMainFileName();
-createVirtualFileIfNeeded(To, FromFileName,
-  From->getBufferForFile(FromFileName));
-
-auto Imported = Importer.Import(Node);
-
-if (Imported) {
-  // This should dump source locations and assert if some source locations
-  // were not imported.
-  SmallString<1024> ImportChecker;
-  llvm::raw_svector_ostream ToNothing(ImportChecker);
-  ToCtx.getTranslationUnitDecl()->print(ToNothing);
-
-  // This traverses the AST to catch certain bugs like poorly or not
-  // implemented subtrees.
-  (*Imported)->dump(ToNothing);
-}
-
-return Imported;
-  }
-
-  template 
-  testing::AssertionResult
-  testImport(const std::string &FromCode,
- const std::vector &FromArgs,
- const std::string &ToCode, const std::vector &ToArgs,
- MatchVerifier &Verifier,
- const BindableMatcher &SearchMatcher,
- const BindableMatcher &VerificationMatcher) {
-const char *const InputFileName = "input.cc";
-const char *const OutputFileName = "output.cc";
-
-std::unique_ptr FromAST = tooling::buildASTFromCodeWithArgs(
- FromCode, FromArgs, InputFileName),
- ToAST = tooling::buildASTFromCodeWithArgs(
- ToCode, ToArgs, OutputFileName);
-
-ASTContext &FromCtx = FromAST->getASTContext(),
-   &ToCtx = ToAST->getASTContext();
-
-ASTImporter Importer(ToCtx, ToAST->getFileManager(), FromCtx,
- FromAST->getFileManager(), false);
-
-auto FoundNodes = match(SearchMatcher, FromCtx);
-if (FoundNodes.size() != 1)
-  return testing::AssertionFailure()
- << "Multiple potential nodes were found!";
-
-auto ToImport = selectFirst(DeclToImportID, FoundNodes);
-if (!ToImport)
-  return testing::AssertionFailure() << "Node type mismatch!";
-
-// Sanity check: the node being imported should match in the same way as
-// the result node.
-BindableMatcher WrapperMatcher(VerificationMatcher);
-EXPECT_TRUE(Verifier.match(ToImport, WrapperMatcher));
-
-auto Imported = importNode(FromAST.get(), ToAST.get(), Importer, ToImport);
-if (!Imported) {
-  std::string ErrorText;
-  handleAllErrors(
-  Imported.takeError(),
-  [&ErrorText](const ImportError &Err) { ErrorText = Err.message(); });
-  return testing::AssertionFailure()
- << "Import failed, error: \"" << ErrorText << "\"!";
-}
-
-return Verifier.match(*Imported, WrapperMatcher);
-  }
-
-  template 
-  testing::AssertionResult
-  testImport(const std::string &FromCode,
- const std::vector &FromArgs,
- const std::string &ToCode, const std::vector &ToArgs,
- MatchVerifier &Verifier,
- const BindableMatcher &VerificationMatcher) {
-return testImport(
-FromCode, FromArgs, ToCode, ToArgs, Verifier,
-translationUnitDecl(
-has(namedDecl(hasName(DeclToImportID)).bind(DeclToImportID))),
-VerificationMatcher);
-  }
-
-protected:
-  std::vector getExtraArgs() const override { return GetParam(); }
-
-public:
-
-  /// Test how AST node named "declToImport" located in the transla

[PATCH] D84221: [OpenMP] Add missing RUN lines for OpenMP 4.5

[Saiyedul Islam via Phabricator via cfe-commits]

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

Thank you. LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84221



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

[PATCH] D83061: [OpenMP] Implement TR8 `present` map type modifier in Clang (1/2)

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Parse/ParseOpenMP.cpp:3067-3068
+  unsigned Type = getOpenMPSimpleClauseType(OMPC_map, PP.getSpelling(Tok));
+  if (Type < OMPC_MAP_MODIFIER_unknown)
+return OMPC_MAP_MODIFIER_unknown;
+  return static_cast(Type);

jdenny wrote:
> ABataev wrote:
> > jdenny wrote:
> > > ABataev wrote:
> > > > Why do we need this?
> > > When called with `OMPC_map`, `getOpenMPSimpleClauseType` can return 
> > > either an `OpenMPMapModifierKind` or `OpenMPMapClauseKind` depending on 
> > > `Tok`.  Thus, without this change, both `isMapModifier` and `isMapType` 
> > > can return either of those despite the difference in their names and 
> > > documentation.
> > > 
> > > I found that, when `Parser::parseMapTypeModifiers` ignores `present` as 
> > > if it's not a modifier because OpenMP < 5.1, then `isMapType` later 
> > > returns `OMPC_MAP_MODIFIER_present` and causes the following assert to 
> > > fail in `Sema::ActOnOpenMPVarListClause`:
> > > 
> > > ```
> > > assert(0 <= ExtraModifier && ExtraModifier <= OMPC_MAP_unknown &&
> > >"Unexpected map modifier.");
> > > ```
> > > 
> > > To me, the most obvious solution is to fix `isMapType` and 
> > > `isMapModifier`.  Fortunately, looking in `OpenMPKinds.h`, the enumerator 
> > > values in `OpenMPMapModifierKind` and `OpenMPMapClauseKind` are disjoint 
> > > so we can tell which we have.
> > Can we have something like:
> > ```
> > if (LangOpts.OpenMP <= 50 && Type == OMPC_MAP_MODIFIER_present)
> >   return OMPC_MAP_MODIFIER_unknown;
> > ```
> > or extend `getOpenMPSimpleClauseType` function with the version parameter 
> > and check there is modifier is allowed and return `unknown` if it is not 
> > compatible with provided version?
> As far as I can tell, my changes general fix this bug in `isMapType` and 
> `isMapModifier`.  It makes them behave as documented.  The suggestions you're 
> making only fix them for the case of `present`.  Why is that better?
It is too general. I think it may mask some issues in future. That's why I 
would suggest to tweak it for `present` only. Or, even better, extend 
`getOpenMPSimpleClauseType` function to handle this modifiers more correctly 
for each particular version rather than fix it here.



Comment at: clang/test/OpenMP/target_data_codegen.cpp:258-260
+// CK1A: [[MTYPE00:@.+]] = {{.+}}constant [1 x i64] [i64 [[#0x1021]]]
+
+// CK1A: [[MTYPE01:@.+]] = {{.+}}constant [1 x i64] [i64 [[#0x1425]]]

jdenny wrote:
> ABataev wrote:
> > Add a comment with interpretaion of the map flags, like `OMP_TO = 0x1 | 
> > OMP_FROM=0x2 | OMP_PRESENT = 0x1000 = 0x1003`
> Are you asking me to add that comment to every map type encoding appearing in 
> this patch?  Or just this one?
> 
It would be good to have something like this in all codegen tests you added in 
this patch


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

https://reviews.llvm.org/D83061



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

[PATCH] D83759: [clangd] Fixes in lit tests

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX added a comment.

In D83759#2164296 , @kadircet wrote:

> as i mentioned please watch out for buildbot breakages after landing this. 
> (and again let me know if I should land this for you)


Thanks for review.

Yes, could you please land this for me?
Aleksandr Platonov 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83759



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

[PATCH] D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: clang/lib/Analysis/LiveVariables.cpp:317-320
   for (Stmt *Child : S->children()) {
-if (Child)
-  AddLiveStmt(val.liveStmts, LV.SSetFact, Child);
+if (const auto *E = dyn_cast_or_null(Child))
+  AddLiveExpr(val.liveExprs, LV.SSetFact, E);
   }

Szelethus wrote:
> ..this part of the code caused the issue. Looking at the related CFG,
> ```lang=c++
> void test_lambda_refcapture()
>  [B2 (ENTRY)]
>Succs (1): B1
> 
>  [B1]
>1: 6
>2: int a = 6;
>3: operator()
>4: [B1.3] (ImplicitCastExpr, FunctionToPointerDecay, void (*)(int &) const)
>5: [&](int &a) {
> a = 42;
> }
>6: [B1.5]
>7: [B1.6] (ImplicitCastExpr, NoOp, const class (lambda at 
> /home/szelethus/Documents/llvm-project/clang/test/Analysis/live-stmts.cpp:183:3))
>8: a
>9: [B1.7]([B1.8]) (OperatorCall)
>   10: clang_analyzer_eval
>   11: [B1.10] (ImplicitCastExpr, FunctionToPointerDecay, void (*)(_Bool))
>   12: a
>   13: [B1.12] (ImplicitCastExpr, LValueToRValue, int)
>   14: 42
>   15: [B1.13] == [B1.14]
>   16: [B1.11]([B1.15])
>Preds (1): B2
>Succs (1): B0
> 
>  [B0 (EXIT)]
>Preds (1): B1
> ```
> its clear that element 5 added the live statement, and I think that that this 
> entire CFG just simply isn't right. Shouldn't we have a distinct element for 
> the assignment?
> 
>  Shouldn't we have a distinct element for the assignment?

Strictly speaking, we have CFGs for a function. The assignment is **not** in 
this function, it is in the `operator()` of the class representing this lambda 
expression. 

So basically, we do have a `LambdaExpr` to represent the expression, but the 
body of the lambda is in a separate entity.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598



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

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

[Logan Smith via Phabricator via cfe-commits]

logan-5 created this revision.
logan-5 added a reviewer: mgorny.
logan-5 added a project: clang.
Herald added a subscriber: cfe-commits.

Yesterday I enabled `-Wsuggest-override` in the main LLVM build and have been 
fighting with the -Werror bots ever since. The key culprits making this 
difficult are googletest and googlemock, which do not use the `override` 
keyword in their sources, so any files that include them are met with massive 
warning (or error, in the case of -Werror) spam.

I've been going through and playing whack-a-mole by adding 
`-Wno-suggest-override` to directories that have code that uses gtest and/or 
gmock; this approach is feeling increasingly inelegant the more I do it, but 
all the patches I've submitted for review have been LGTM'd so far.

I'm wondering if I should do this a different way, or if it's fine to just 
proceed along this path until the bots are green again.

Thank you for your review.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84244

Files:
  clang/unittests/CMakeLists.txt


Index: clang/unittests/CMakeLists.txt
===
--- clang/unittests/CMakeLists.txt
+++ clang/unittests/CMakeLists.txt
@@ -10,6 +10,10 @@
   endif()
 endif()
 
+if (CXX_SUPPORTS_SUGGEST_OVERRIDE_FLAG)
+  add_definitions("-Wno-suggest-override")
+endif()
+
 # add_clang_unittest(test_dirname file1.cpp file2.cpp)
 #
 # Will compile the list of files together and link against the clang


Index: clang/unittests/CMakeLists.txt
===
--- clang/unittests/CMakeLists.txt
+++ clang/unittests/CMakeLists.txt
@@ -10,6 +10,10 @@
   endif()
 endif()
 
+if (CXX_SUPPORTS_SUGGEST_OVERRIDE_FLAG)
+  add_definitions("-Wno-suggest-override")
+endif()
+
 # add_clang_unittest(test_dirname file1.cpp file2.cpp)
 #
 # Will compile the list of files together and link against the clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] fa42b7c - [clang-tools-extra] Disable -Wsuggest-override for unittests/

[Logan Smith via cfe-commits]

Author: Logan Smith
Date: 2020-07-21T09:11:53-07:00
New Revision: fa42b7cf2949802ff0b8a63a2e111a2a68711067

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

LOG: [clang-tools-extra] Disable -Wsuggest-override for unittests/

This avoids massive warning spam due to the unit tests' use of gtest and gmock, 
which do not use the 'override' keyword in their sources.

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

Added: 


Modified: 
clang-tools-extra/unittests/CMakeLists.txt

Removed: 




diff  --git a/clang-tools-extra/unittests/CMakeLists.txt 
b/clang-tools-extra/unittests/CMakeLists.txt
index 086a68e63830..751827cd2a0f 100644
--- a/clang-tools-extra/unittests/CMakeLists.txt
+++ b/clang-tools-extra/unittests/CMakeLists.txt
@@ -5,6 +5,10 @@ function(add_extra_unittest test_dirname)
   add_unittest(ExtraToolsUnitTests ${test_dirname} ${ARGN})
 endfunction()
 
+if (CXX_SUPPORTS_SUGGEST_OVERRIDE_FLAG)
+  add_definitions("-Wno-suggest-override")
+endif()
+
 add_subdirectory(clang-apply-replacements)
 add_subdirectory(clang-change-namespace)
 add_subdirectory(clang-doc)



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

[PATCH] D84213: [clang-tools-extra] Disable -Wsuggest-override for unittests/

[Logan Smith via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfa42b7cf2949: [clang-tools-extra] Disable -Wsuggest-override 
for unittests/ (authored by logan-5).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84213

Files:
  clang-tools-extra/unittests/CMakeLists.txt


Index: clang-tools-extra/unittests/CMakeLists.txt
===
--- clang-tools-extra/unittests/CMakeLists.txt
+++ clang-tools-extra/unittests/CMakeLists.txt
@@ -5,6 +5,10 @@
   add_unittest(ExtraToolsUnitTests ${test_dirname} ${ARGN})
 endfunction()
 
+if (CXX_SUPPORTS_SUGGEST_OVERRIDE_FLAG)
+  add_definitions("-Wno-suggest-override")
+endif()
+
 add_subdirectory(clang-apply-replacements)
 add_subdirectory(clang-change-namespace)
 add_subdirectory(clang-doc)


Index: clang-tools-extra/unittests/CMakeLists.txt
===
--- clang-tools-extra/unittests/CMakeLists.txt
+++ clang-tools-extra/unittests/CMakeLists.txt
@@ -5,6 +5,10 @@
   add_unittest(ExtraToolsUnitTests ${test_dirname} ${ARGN})
 endfunction()
 
+if (CXX_SUPPORTS_SUGGEST_OVERRIDE_FLAG)
+  add_definitions("-Wno-suggest-override")
+endif()
+
 add_subdirectory(clang-apply-replacements)
 add_subdirectory(clang-change-namespace)
 add_subdirectory(clang-doc)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84246: [clang][LTO] Pass-through remarks options and set auto hotness threshold

[Wei Wang via Phabricator via cfe-commits]

weiwang created this revision.
Herald added subscribers: llvm-commits, cfe-commits, dang, dexonsmith, 
steven_wu, MaskRay, hiraditya, eraman, arichardson, inglorion, emaste.
Herald added a reviewer: espindola.
Herald added a reviewer: MaskRay.
Herald added projects: clang, LLVM.

Expand remarks hotness threshold option -fdiagnostics-hotness-threshold in clang
driver command-line to both filtering remarks by hotness threshold from profile
summary and automatic remarks options pass-through to linker.

Remarks hotness filtering relies on several driver options. Table below lists
how different options are correlated and affect final remarks outputs:

| profile | hotness | threshold | remarks printed |
| --- | --- | - | --- |
| No  | No  | No| All |
| No  | No  | Yes   | None|
| No  | Yes | No| All |
| No  | Yes | Yes   | None|
| Yes | No  | No| All |
| Yes | No  | Yes   | None|
| Yes | Yes | No| All |
| Yes | Yes | Yes   | >=threshold |
|

The new argument value -fdiagnostics-hotness-threshold=auto indicates threshold
will be synced with hotness threshold from profile summary during compilation.
In addition, when the following conditions are met, remarks related options are
passed into linker as well:

1. LTO is enabled;
2. Single arch target is specified;
3. The linker is lld;

The "auto" threshold relies on the availability of profile summary. In case of
missing such information, no remarks will be generated.

This gives novice user a convenient way to collect and filter remarks throughout
a typical toolchain invocation with sample profile and LTO using single switch
from the clang driver.

A typical use of this option from clang command-line:

- Using -Rpass* options to print remarks to screen:

clang -fuse-ld=lld -flto=thin

  -fprofile-sample-use=foo_sample.txt
  -Rpass=inline
  -Rpass-missed=inline -Rpass-analysis=inline
  -fdiagnostics-show-hotness -fdiagnostics-hotness-threshold=auto
  -o foo foo.cpp

Remarks will be dumped to screen from both pre-lto and lto compilation.

- Using serialized remarks options:

clang -fuse-ld=lld -flto=thin

  -fprofile-sample-use=foo_sample.txt -fsave-optimization-record
  -fdiagnostics-show-hotness -fdiagnostics-hotness-threshold=auto
  -o foo foo.cpp

This will produce multiple yaml files containing optimization remarks:

- foo.opt.yaml : remarks from pre-lto
- foo.opt.ld.yaml.thin.1.yaml: remark during lto

Both types of options can be used together.

Given its restricted use cases, this shouldn't be viewed as a complete
replacement of current remarks option handling for the linker.

In order to make the option consistent across various tools, the support for the
new 'auto' argument is also available in the following tools:

- lld:  -opt-remarks-with-hotness=auto
- llvm-lto: -lto-pass-remarks-hotness-threshold=auto
- opt/llc:  --pass-remarks-hotness-threshold=auto


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84246

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/remarks-pass-through.c
  lld/ELF/Config.h
  lld/ELF/Driver.cpp
  lld/ELF/LTO.cpp
  lld/ELF/Options.td
  lld/test/ELF/lto/opt-remarks.ll
  llvm/include/llvm/Analysis/ProfileSummaryInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/IR/LLVMRemarkStreamer.h
  llvm/include/llvm/IR/Module.h
  llvm/include/llvm/LTO/Config.h
  llvm/include/llvm/LTO/LTO.h
  llvm/include/llvm/Remarks/HotnessThresholdParser.h
  llvm/lib/Analysis/OptimizationRemarkEmitter.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/IR/LLVMRemarkStreamer.cpp
  llvm/lib/IR/Module.cpp
  llvm/lib/LTO/LTO.cpp
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/LTO/LTOCodeGenerator.cpp
  llvm/lib/LTO/ThinLTOCodeGenerator.cpp
  llvm/test/LTO/X86/diagnostic-handler-remarks-with-hotness.ll
  llvm/test/Other/optimization-remarks-auto.ll
  llvm/test/Transforms/SampleProfile/Inputs/remarks-hotness.prof
  llvm/test/Transforms/SampleProfile/remarks-hotness.ll
  llvm/tools/llc/llc.cpp
  llvm/tools/opt/opt.cpp

Index: llvm/tools/opt/opt.cpp
===
--- llvm/tools/opt/opt.cpp
+++ llvm/tools/opt/opt.cpp
@@ -40,6 +40,7 @@
 #include "llvm/LinkAllIR.h"
 #include "llvm/LinkAllPasses.h"
 #include "llvm/MC/SubtargetFeature.h"
+#include "llvm/Remarks/HotnessThresholdParser.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Host.h"
@@ -274,11 +275,13 @@

[PATCH] D83190: [analyzer] Model iterator random incrementation symmetrically

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

Maybe the assert `b.hasValue()` is related to the problems in D83115 
  or D83961 ? 
The changes in those affect the same functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83190



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

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

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

Quuxplusone added a comment.

For GTest and GMock specifically, it would be a good medium-term idea to try to 
upstream patches into those libraries. I did eventually have success 
upstreaming fixes for `-Wdeprecated` into GTest, for example: 
https://github.com/google/googletest/pull/2815
Disabling the warning in unittests for now still seems like a good short-term 
fix though.

(Although I'm a bit confused; I thought `-Wsuggest-override` was 
off-by-default? Is it part of `-Wall`, and do unittests add `-Wall` to their 
command line or something?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84244



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

[PATCH] D80791: [AArch64] Generate .note.gnu.property based on module flags.

[Daniel Kiss via Phabricator via cfe-commits]

danielkiss added a comment.

> If any function has the attribute "sign-return-address", then the output note
>  section should have PAC bit set. The return address signing is completely 
> local
>  to the function, and functions with or without return address signing can be
>  freely mixed with each other.

That is true PAC and non-PAC functions can be mixed. 
Does one function makes the "all executable sections" pac-ret enabled?
BTW `GNU_PROPERTY_AARCH64_FEATURE_1_PAC` is not really used for anything.

> Likewise, if any function has the attribute "branch-target-enforcement", then
>  the output note section should have the BTI flag set. Even though code 
> compiled
>  with BTI is not necessarily compatible with non-BTI code:
> 
> - the only way to get BTI code is by explicit use of 
> `-mbranch-protection=...` command-line option, or the corresponding 
> attribute, which we should consider a clear indication about the user's 
> intent to use BTI.
> - the only way to get a mix of present/non-present 
> "branch-target-enforcement" attributes is by the explicit use of the 
> `__attribute__((target("branch-protection=..."))`, in which case we should 
> assume the user knows what they are doing.

`__ARM_FEATURE_PAC_DEFAULT` and `__ARM_FEATURE_BTI_DEFAULT` controlled by the 
`-mbranch-protection=...` 
https://developer.arm.com/documentation/101028/0011/Feature-test-macros?lang=en

One of the reasons of the introduction of these macros is the management of the 
function attributes.
For example:

  #ifdef __ARM_FEATURE_PAC_DEFAULT
  #ifdef __ARM_FEATURE_BTI_DEFAULT
  #define NO_PAC_FUNC __attribute__((target("branch-protection=bti")))
  #else
  #define NO_PAC_FUNC __attribute__((target("branch-protection=none")))
  #endif /* __ARM_FEATURE_BTI_DEFAULT */
  ...

In my humble opinion the function attribute is there to alter global setting.
I considered to propagate the function attribute to the module flags but 
that would lead to inconsistent compilation with the macros that I'd avoid.

> What do to if there are no functions in the compile unit?
> 
> Technically, objects produced from such a unit are fully compatible with both 
> PAC and BTI, which
>  means both flags should be set. But looking at the (non-existent) function 
> attributes alone does
>  not allow us to unambiguously derive a user's intent to use PAC/BTI. In this 
> case, I would suggest
>  setting the ELF note flags, according to the LLVM IR module flags.

I think the only clear indication from the user to use PAC/BTI is the explicit 
use of `-mbranch-protection=...` command-line option.
A few function attributes that would turn PAC/BTI on just on those few 
functions makes no sense for me in any real world application. 
Valid to turn off PAC/BTI on selected functions while the whole application 
compiled with them.

We need to turn PAC off on the code path where we change\manage the keys for 
example.
Exaggerated example for BTI: https://godbolt.org/z/Y9bhe9  Current version of 
llvm issues a warning and won't emit the note while I think it should.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80791



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

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

[Logan Smith via Phabricator via cfe-commits]

logan-5 added a comment.

In D84244#2164537 , @Quuxplusone wrote:

> For GTest and GMock specifically, it would be a good medium-term idea to try 
> to upstream patches into those libraries. I did eventually have success 
> upstreaming fixes for `-Wdeprecated` into GTest, for example: 
> https://github.com/google/googletest/pull/2815


Nice, that's encouraging. That does sound like a good solution for slightly 
further down the road.

> (Although I'm a bit confused; I thought `-Wsuggest-override` was 
> off-by-default? Is it part of `-Wall`, and do unittests add `-Wall` to their 
> command line or something?)

After I implemented the warning and it landed, people wanted it explicitly 
turned on in the LLVM build, so then I did that. This is part of the fallout 
from that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84244



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

[PATCH] D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist

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

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



Comment at: clang/lib/Analysis/LiveVariables.cpp:317-320
   for (Stmt *Child : S->children()) {
-if (Child)
-  AddLiveStmt(val.liveStmts, LV.SSetFact, Child);
+if (const auto *E = dyn_cast_or_null(Child))
+  AddLiveExpr(val.liveExprs, LV.SSetFact, E);
   }

xazax.hun wrote:
> Szelethus wrote:
> > ..this part of the code caused the issue. Looking at the related CFG,
> > ```lang=c++
> > void test_lambda_refcapture()
> >  [B2 (ENTRY)]
> >Succs (1): B1
> > 
> >  [B1]
> >1: 6
> >2: int a = 6;
> >3: operator()
> >4: [B1.3] (ImplicitCastExpr, FunctionToPointerDecay, void (*)(int &) 
> > const)
> >5: [&](int &a) {
> > a = 42;
> > }
> >6: [B1.5]
> >7: [B1.6] (ImplicitCastExpr, NoOp, const class (lambda at 
> > /home/szelethus/Documents/llvm-project/clang/test/Analysis/live-stmts.cpp:183:3))
> >8: a
> >9: [B1.7]([B1.8]) (OperatorCall)
> >   10: clang_analyzer_eval
> >   11: [B1.10] (ImplicitCastExpr, FunctionToPointerDecay, void (*)(_Bool))
> >   12: a
> >   13: [B1.12] (ImplicitCastExpr, LValueToRValue, int)
> >   14: 42
> >   15: [B1.13] == [B1.14]
> >   16: [B1.11]([B1.15])
> >Preds (1): B2
> >Succs (1): B0
> > 
> >  [B0 (EXIT)]
> >Preds (1): B1
> > ```
> > its clear that element 5 added the live statement, and I think that that 
> > this entire CFG just simply isn't right. Shouldn't we have a distinct 
> > element for the assignment?
> > 
> >  Shouldn't we have a distinct element for the assignment?
> 
> Strictly speaking, we have CFGs for a function. The assignment is **not** in 
> this function, it is in the `operator()` of the class representing this 
> lambda expression. 
> 
> So basically, we do have a `LambdaExpr` to represent the expression, but the 
> body of the lambda is in a separate entity.
Well, `debug.DumpCFG` definitely doesn't indulge me with a separate lambda CFG, 
so I figured this is a (rightful) optimization or compression.

My point is, this entire code snippet is a seriously error prone, best-effort 
heuristic. Switch casing every small little corner case might be tedious, 
troublesome in terms of scaling, and I for sure don't want to maintain it 'til 
eternity, but we have to acknowledge that this isn't a perfect solution either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598



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

[PATCH] D83519: [NewPM] Support optnone under new pass manager

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks updated this revision to Diff 279563.
aeubanks added a comment.

Rename OptNoneInstrumentation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83519

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/Passes/StandardInstrumentations.h
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/test/Feature/optnone-opt.ll
  llvm/tools/opt/NewPMDriver.cpp

Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -261,7 +261,7 @@
 }
   }
   PassInstrumentationCallbacks PIC;
-  StandardInstrumentations SI;
+  StandardInstrumentations SI(DebugPM);
   SI.registerCallbacks(PIC);
 
   PipelineTuningOptions PTO;
Index: llvm/test/Feature/optnone-opt.ll
===
--- llvm/test/Feature/optnone-opt.ll
+++ llvm/test/Feature/optnone-opt.ll
@@ -1,9 +1,15 @@
-; RUN: opt -S -debug %s 2>&1 | FileCheck %s --check-prefix=OPT-O0
-; RUN: opt -O1 -S -debug %s 2>&1 | FileCheck %s --check-prefix=OPT-O1
-; RUN: opt -O2 -S -debug %s 2>&1 | FileCheck %s --check-prefix=OPT-O1 --check-prefix=OPT-O2O3
-; RUN: opt -O3 -S -debug %s 2>&1 | FileCheck %s --check-prefix=OPT-O1 --check-prefix=OPT-O2O3
-; RUN: opt -dce -die -gvn-hoist -loweratomic -S -debug %s 2>&1 | FileCheck %s --check-prefix=OPT-MORE
-; RUN: opt -indvars -licm -loop-deletion -loop-extract -loop-idiom -loop-instsimplify -loop-reduce -loop-reroll -loop-rotate -loop-unroll -loop-unswitch -S -debug %s 2>&1 | FileCheck %s --check-prefix=OPT-LOOP
+; RUN: opt -S -debug -enable-new-pm=0 %s 2>&1 | FileCheck %s --check-prefix=O0
+; RUN: opt -O1 -S -debug -enable-new-pm=0 %s 2>&1 | FileCheck %s --check-prefix=O1
+; RUN: opt -O2 -S -debug -enable-new-pm=0 %s 2>&1 | FileCheck %s --check-prefix=O1 --check-prefix=O2O3
+; RUN: opt -O3 -S -debug -enable-new-pm=0 %s 2>&1 | FileCheck %s --check-prefix=O1 --check-prefix=O2O3
+; RUN: opt -dce -die -gvn-hoist -loweratomic -S -debug -enable-new-pm=0 %s 2>&1 | FileCheck %s --check-prefix=MORE
+; RUN: opt -indvars -licm -loop-deletion -loop-extract -loop-idiom -loop-instsimplify -loop-reduce -loop-reroll -loop-rotate -loop-unroll -loop-unswitch -enable-new-pm=0 -S -debug %s 2>&1 | FileCheck %s --check-prefix=LOOP
+; RUN: opt -enable-npm-optnone -S -debug-pass-manager -enable-new-pm %s 2>&1 | FileCheck %s --check-prefix=NPM-O0
+; RUN: opt -enable-npm-optnone -O1 -S -debug-pass-manager -enable-new-pm %s 2>&1 | FileCheck %s --check-prefix=NPM-O1
+; RUN: opt -enable-npm-optnone -O2 -S -debug-pass-manager -enable-new-pm %s 2>&1 | FileCheck %s --check-prefix=NPM-O1 --check-prefix=NPM-O2O3
+; RUN: opt -enable-npm-optnone -O3 -S -debug-pass-manager -enable-new-pm %s 2>&1 | FileCheck %s --check-prefix=NPM-O1 --check-prefix=NPM-O2O3
+; RUN: opt -enable-npm-optnone -dce -gvn-hoist -loweratomic -S -debug-pass-manager -enable-new-pm %s 2>&1 | FileCheck %s --check-prefix=NPM-MORE
+; RUN: opt -enable-npm-optnone -indvars -licm -loop-deletion -loop-idiom -loop-instsimplify -loop-reduce -loop-unroll -simple-loop-unswitch -S -debug-pass-manager -enable-new-pm %s 2>&1 | FileCheck %s --check-prefix=NPM-LOOP
 
 ; REQUIRES: asserts
 
@@ -34,32 +40,53 @@
 attributes #0 = { optnone noinline }
 
 ; Nothing that runs at -O0 gets skipped.
-; OPT-O0-NOT: Skipping pass
+; O0-NOT: Skipping pass
+; NPM-O0-NOT: Skipping pass
 
 ; IR passes run at -O1 and higher.
-; OPT-O1-DAG: Skipping pass 'Aggressive Dead Code Elimination'
-; OPT-O1-DAG: Skipping pass 'Combine redundant instructions'
-; OPT-O1-DAG: Skipping pass 'Early CSE'
-; OPT-O1-DAG: Skipping pass 'Reassociate expressions'
-; OPT-O1-DAG: Skipping pass 'Simplify the CFG'
-; OPT-O1-DAG: Skipping pass 'Sparse Conditional Constant Propagation'
+; O1-DAG: Skipping pass 'Aggressive Dead Code Elimination'
+; O1-DAG: Skipping pass 'Combine redundant instructions'
+; O1-DAG: Skipping pass 'Early CSE'
+; O1-DAG: Skipping pass 'Reassociate expressions'
+; O1-DAG: Skipping pass 'Simplify the CFG'
+; O1-DAG: Skipping pass 'Sparse Conditional Constant Propagation'
+; NPM-O1-DAG: Skipping pass: SimplifyCFGPass
+; NPM-O1-DAG: Skipping pass: SROA
+; NPM-O1-DAG: Skipping pass: EarlyCSEPass
+; NPM-O1-DAG: Skipping pass: LowerExpectIntrinsicPass
+; NPM-O1-DAG: Skipping pass: PromotePass
+; NPM-O1-DAG: Skipping pass: InstCombinePass
 
 ; Additional IR passes run at -O2 and higher.
-; OPT-O2O3-DAG: Skipping pass 'Global Value Numbering'
-; OPT-O2O3-DAG: Skipping pass 'SLP Vectorizer'
+; O2O3-DAG: Skipping pass 'Global Value Numbering'
+; O2O3-DAG: Skipping pass 'SLP Vectorizer'
+; NPM-O2O3-DAG: Skipping pass: GVN
+; NPM-O2O3-DAG: Skipping pass: SLPVectorizerPass
 
 ; Additional IR passes that opt doesn't turn on by default.
-; OPT-MORE-DAG: Skipping pass 'Dead Code Elimination'
-; OPT-MORE-DAG: Skipping pass 'Dead Instruction 

[PATCH] D83519: [NewPM] Support optnone under new pass manager

[Arthur Eubanks via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb13b85818218: [NewPM] Support optnone under new pass manager 
(authored by aeubanks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83519

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/Passes/StandardInstrumentations.h
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/test/Feature/optnone-opt.ll
  llvm/tools/opt/NewPMDriver.cpp

Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -261,7 +261,7 @@
 }
   }
   PassInstrumentationCallbacks PIC;
-  StandardInstrumentations SI;
+  StandardInstrumentations SI(DebugPM);
   SI.registerCallbacks(PIC);
 
   PipelineTuningOptions PTO;
Index: llvm/test/Feature/optnone-opt.ll
===
--- llvm/test/Feature/optnone-opt.ll
+++ llvm/test/Feature/optnone-opt.ll
@@ -1,9 +1,15 @@
-; RUN: opt -S -debug %s 2>&1 | FileCheck %s --check-prefix=OPT-O0
-; RUN: opt -O1 -S -debug %s 2>&1 | FileCheck %s --check-prefix=OPT-O1
-; RUN: opt -O2 -S -debug %s 2>&1 | FileCheck %s --check-prefix=OPT-O1 --check-prefix=OPT-O2O3
-; RUN: opt -O3 -S -debug %s 2>&1 | FileCheck %s --check-prefix=OPT-O1 --check-prefix=OPT-O2O3
-; RUN: opt -dce -die -gvn-hoist -loweratomic -S -debug %s 2>&1 | FileCheck %s --check-prefix=OPT-MORE
-; RUN: opt -indvars -licm -loop-deletion -loop-extract -loop-idiom -loop-instsimplify -loop-reduce -loop-reroll -loop-rotate -loop-unroll -loop-unswitch -S -debug %s 2>&1 | FileCheck %s --check-prefix=OPT-LOOP
+; RUN: opt -S -debug -enable-new-pm=0 %s 2>&1 | FileCheck %s --check-prefix=O0
+; RUN: opt -O1 -S -debug -enable-new-pm=0 %s 2>&1 | FileCheck %s --check-prefix=O1
+; RUN: opt -O2 -S -debug -enable-new-pm=0 %s 2>&1 | FileCheck %s --check-prefix=O1 --check-prefix=O2O3
+; RUN: opt -O3 -S -debug -enable-new-pm=0 %s 2>&1 | FileCheck %s --check-prefix=O1 --check-prefix=O2O3
+; RUN: opt -dce -die -gvn-hoist -loweratomic -S -debug -enable-new-pm=0 %s 2>&1 | FileCheck %s --check-prefix=MORE
+; RUN: opt -indvars -licm -loop-deletion -loop-extract -loop-idiom -loop-instsimplify -loop-reduce -loop-reroll -loop-rotate -loop-unroll -loop-unswitch -enable-new-pm=0 -S -debug %s 2>&1 | FileCheck %s --check-prefix=LOOP
+; RUN: opt -enable-npm-optnone -S -debug-pass-manager -enable-new-pm %s 2>&1 | FileCheck %s --check-prefix=NPM-O0
+; RUN: opt -enable-npm-optnone -O1 -S -debug-pass-manager -enable-new-pm %s 2>&1 | FileCheck %s --check-prefix=NPM-O1
+; RUN: opt -enable-npm-optnone -O2 -S -debug-pass-manager -enable-new-pm %s 2>&1 | FileCheck %s --check-prefix=NPM-O1 --check-prefix=NPM-O2O3
+; RUN: opt -enable-npm-optnone -O3 -S -debug-pass-manager -enable-new-pm %s 2>&1 | FileCheck %s --check-prefix=NPM-O1 --check-prefix=NPM-O2O3
+; RUN: opt -enable-npm-optnone -dce -gvn-hoist -loweratomic -S -debug-pass-manager -enable-new-pm %s 2>&1 | FileCheck %s --check-prefix=NPM-MORE
+; RUN: opt -enable-npm-optnone -indvars -licm -loop-deletion -loop-idiom -loop-instsimplify -loop-reduce -loop-unroll -simple-loop-unswitch -S -debug-pass-manager -enable-new-pm %s 2>&1 | FileCheck %s --check-prefix=NPM-LOOP
 
 ; REQUIRES: asserts
 
@@ -34,32 +40,53 @@
 attributes #0 = { optnone noinline }
 
 ; Nothing that runs at -O0 gets skipped.
-; OPT-O0-NOT: Skipping pass
+; O0-NOT: Skipping pass
+; NPM-O0-NOT: Skipping pass
 
 ; IR passes run at -O1 and higher.
-; OPT-O1-DAG: Skipping pass 'Aggressive Dead Code Elimination'
-; OPT-O1-DAG: Skipping pass 'Combine redundant instructions'
-; OPT-O1-DAG: Skipping pass 'Early CSE'
-; OPT-O1-DAG: Skipping pass 'Reassociate expressions'
-; OPT-O1-DAG: Skipping pass 'Simplify the CFG'
-; OPT-O1-DAG: Skipping pass 'Sparse Conditional Constant Propagation'
+; O1-DAG: Skipping pass 'Aggressive Dead Code Elimination'
+; O1-DAG: Skipping pass 'Combine redundant instructions'
+; O1-DAG: Skipping pass 'Early CSE'
+; O1-DAG: Skipping pass 'Reassociate expressions'
+; O1-DAG: Skipping pass 'Simplify the CFG'
+; O1-DAG: Skipping pass 'Sparse Conditional Constant Propagation'
+; NPM-O1-DAG: Skipping pass: SimplifyCFGPass
+; NPM-O1-DAG: Skipping pass: SROA
+; NPM-O1-DAG: Skipping pass: EarlyCSEPass
+; NPM-O1-DAG: Skipping pass: LowerExpectIntrinsicPass
+; NPM-O1-DAG: Skipping pass: PromotePass
+; NPM-O1-DAG: Skipping pass: InstCombinePass
 
 ; Additional IR passes run at -O2 and higher.
-; OPT-O2O3-DAG: Skipping pass 'Global Value Numbering'
-; OPT-O2O3-DAG: Skipping pass 'SLP Vectorizer'
+; O2O3-DAG: Skipping pass 'Global Value Numbering'
+; O2O3-DAG: Skipping pass 'SLP Vectorizer'
+; NPM-O2O3-DAG: Skipping pass: GVN
+; NPM-O2O3-DAG: Skipping pass: SLPVectorizerPass
 
 ; Additional IR passes that opt doesn't turn on by default.
-; OPT-MORE-DAG: Skipping pass

[clang] b13b858 - [NewPM] Support optnone under new pass manager

[Arthur Eubanks via cfe-commits]

Author: Arthur Eubanks
Date: 2020-07-21T09:53:43-07:00
New Revision: b13b85818218aa17cee4c987cbf835208c06cf10

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

LOG: [NewPM] Support optnone under new pass manager

OptNoneInstrumentation is part of StandardInstrumentations. It skips
functions (or loops) that are marked optnone.

The feature of skipping optional passes for optnone functions under NPM
is gated on a -enable-npm-optnone flag. Currently it is by default
false. That is because we still need to mark all required passes to be
required. Otherwise optnone functions will start having incorrect
semantics.  After that is done in following changes, we can remove the
flag and always enable this.

Reviewed By: ychen

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

Added: 


Modified: 
clang/lib/CodeGen/BackendUtil.cpp
llvm/include/llvm/Passes/StandardInstrumentations.h
llvm/lib/LTO/LTOBackend.cpp
llvm/lib/Passes/StandardInstrumentations.cpp
llvm/test/Feature/optnone-opt.ll
llvm/tools/opt/NewPMDriver.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index dce0940670a2..3405c48bc389 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -1153,7 +1153,7 @@ void EmitAssemblyHelper::EmitAssemblyWithNewPassManager(
   PTO.Coroutines = LangOpts.Coroutines;
 
   PassInstrumentationCallbacks PIC;
-  StandardInstrumentations SI;
+  StandardInstrumentations SI(CodeGenOpts.DebugPassManager);
   SI.registerCallbacks(PIC);
   PassBuilder PB(TM.get(), PTO, PGOOpt, &PIC);
 

diff  --git a/llvm/include/llvm/Passes/StandardInstrumentations.h 
b/llvm/include/llvm/Passes/StandardInstrumentations.h
index 3d3002eecce9..bd8b886a2bb3 100644
--- a/llvm/include/llvm/Passes/StandardInstrumentations.h
+++ b/llvm/include/llvm/Passes/StandardInstrumentations.h
@@ -16,6 +16,7 @@
 #define LLVM_PASSES_STANDARDINSTRUMENTATIONS_H
 
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/IR/PassInstrumentation.h"
 #include "llvm/IR/PassTimingInfo.h"
 
@@ -53,14 +54,27 @@ class PrintIRInstrumentation {
   bool StoreModuleDesc = false;
 };
 
+class OptNoneInstrumentation {
+public:
+  OptNoneInstrumentation(bool DebugLogging) : DebugLogging(DebugLogging) {}
+  void registerCallbacks(PassInstrumentationCallbacks &PIC);
+
+private:
+  bool skip(StringRef PassID, Any IR);
+
+  bool DebugLogging;
+};
+
 /// This class provides an interface to register all the standard pass
 /// instrumentations and manages their state (if any).
 class StandardInstrumentations {
   PrintIRInstrumentation PrintIR;
   TimePassesHandler TimePasses;
+  OptNoneInstrumentation OptNone;
 
 public:
-  StandardInstrumentations() = default;
+  StandardInstrumentations(bool DebugLogging)
+  : PrintIR(), TimePasses(), OptNone(DebugLogging) {}
 
   void registerCallbacks(PassInstrumentationCallbacks &PIC);
 

diff  --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp
index 0c395f9bbf28..ca29548a4d7c 100644
--- a/llvm/lib/LTO/LTOBackend.cpp
+++ b/llvm/lib/LTO/LTOBackend.cpp
@@ -197,7 +197,7 @@ static void runNewPMPasses(const Config &Conf, Module &Mod, 
TargetMachine *TM,
   }
 
   PassInstrumentationCallbacks PIC;
-  StandardInstrumentations SI;
+  StandardInstrumentations SI(Conf.DebugPassManager);
   SI.registerCallbacks(PIC);
   PassBuilder PB(TM, Conf.PTO, PGOOpt, &PIC);
   AAManager AA;

diff  --git a/llvm/lib/Passes/StandardInstrumentations.cpp 
b/llvm/lib/Passes/StandardInstrumentations.cpp
index 1e1a6b98a65a..22a7103715d5 100644
--- a/llvm/lib/Passes/StandardInstrumentations.cpp
+++ b/llvm/lib/Passes/StandardInstrumentations.cpp
@@ -13,6 +13,7 @@
 
//===--===//
 
 #include "llvm/Passes/StandardInstrumentations.h"
+#include "llvm/ADT/Any.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/Analysis/CallGraphSCCPass.h"
 #include "llvm/Analysis/LazyCallGraph.h"
@@ -21,12 +22,19 @@
 #include "llvm/IR/IRPrintingPasses.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/PassInstrumentation.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
 
+// TODO: remove once all required passes are marked as such.
+static cl::opt
+EnableOptnone("enable-npm-optnone", cl::init(false),
+  cl::desc("Enable skipping optional passes optnone functions "
+   "under new pass manager"));
+
 namespace {
 
 /// Extracting Module out of \p IR unit. Also fills a textual description
@@ -243,8 +251,32 @@ void PrintIRInstrumentation::registerCallbacks(
   }
 }
 
+void OptNoneInstrume

[PATCH] D84248: [analyzer][StdLibraryFunctionsChecker] Add POSIX time handling functions

[Gabor Marton via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84248

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-POSIX.c

Index: clang/test/Analysis/std-c-library-functions-POSIX.c
===
--- clang/test/Analysis/std-c-library-functions-POSIX.c
+++ clang/test/Analysis/std-c-library-functions-POSIX.c
@@ -95,6 +95,19 @@
 // CHECK: Loaded summary for: ssize_t send(int sockfd, const void *buf, size_t len, int flags)
 // CHECK: Loaded summary for: int socketpair(int domain, int type, int protocol, int sv[2])
 // CHECK: Loaded summary for: int getnameinfo(const struct sockaddr *restrict sa, socklen_t salen, char *restrict node, socklen_t nodelen, char *restrict service, socklen_t servicelen, int flags)
+// CHECK: Loaded summary for: int utime(const char *filename, struct utimbuf *buf)
+// CHECK: Loaded summary for: int futimens(int fd, const struct timespec times[2])
+// CHECK: Loaded summary for: int utimensat(int dirfd, const char *pathname, const struct timespec times[2], int flags)
+// CHECK: Loaded summary for: int utimes(const char *filename, const struct timeval times[2])
+// CHECK: Loaded summary for: int nanosleep(const struct timespec *rqtp, struct timespec *rmtp)
+// CHECK: Loaded summary for: struct tm *localtime(const time_t *tp)
+// CHECK: Loaded summary for: struct tm *localtime_r(const time_t *restrict timer, struct tm *restrict result)
+// CHECK: Loaded summary for: char *asctime_r(const struct tm *restrict tm, char *restrict buf)
+// CHECK: Loaded summary for: char *ctime_r(const time_t *timep, char *buf)
+// CHECK: Loaded summary for: struct tm *gmtime_r(const time_t *restrict timer, struct tm *restrict result)
+// CHECK: Loaded summary for: struct tm *gmtime(const time_t *tp)
+// CHECK: Loaded summary for: int clock_gettime(clockid_t clock_id, struct timespec *tp)
+// CHECK: Loaded summary for: int getitimer(int which, struct itimerval *curr_value)
 
 long a64l(const char *str64);
 char *l64a(long value);
@@ -226,6 +239,25 @@
 ssize_t send(int sockfd, const void *buf, size_t len, int flags);
 int socketpair(int domain, int type, int protocol, int sv[2]);
 int getnameinfo(const struct sockaddr *restrict sa, socklen_t salen, char *restrict node, socklen_t nodelen, char *restrict service, socklen_t servicelen, int flags);
+struct utimbuf;
+struct timespec { int x; };
+struct timeval { int x; };
+int utime(const char *filename, struct utimbuf *buf);
+int futimens(int fd, const struct timespec times[2]);
+int utimensat(int dirfd, const char *pathname, const struct timespec times[2], int flags);
+int utimes(const char *filename, const struct timeval times[2]);
+int nanosleep(const struct timespec *rqtp, struct timespec *rmtp);
+typedef unsigned long time_t;
+struct tm *localtime(const time_t *tp);
+struct tm *localtime_r(const time_t *restrict timer, struct tm *restrict result);
+char *asctime_r(const struct tm *restrict tm, char *restrict buf);
+char *ctime_r(const time_t *timep, char *buf);
+struct tm *gmtime_r(const time_t *restrict timer, struct tm *restrict result);
+struct tm *gmtime(const time_t *tp);
+typedef unsigned long clockid_t;
+int clock_gettime(clockid_t clock_id, struct timespec *tp);
+struct itimerval;
+int getitimer(int which, struct itimerval *curr_value);
 
 // Must have at least one call expression to initialize the summary map.
 int bar(void);
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -1986,6 +1986,156 @@
   BufferSize(/*Buffer=*/ArgNo(4), /*BufSize=*/ArgNo(5)))
   .ArgConstraint(
   ArgumentCondition(5, WithinRange, Range(0, *Socklen_tMax;
+
+Optional StructUtimbufTy = lookupType("utimbuf", ACtx);
+Optional StructUtimbufPtrTy;
+if (StructUtimbufTy)
+  StructUtimbufPtrTy = ACtx.getPointerType(*StructUtimbufTy);
+
+if (StructUtimbufPtrTy)
+  // int utime(const char *filename, struct utimbuf *buf);
+  addToFunctionSummaryMap(
+  "utime", Summary(ArgTypes{ConstCharPtrTy, *StructUtimbufPtrTy},
+   RetType{IntTy}, NoEvalCall)
+   .ArgConstraint(NotNull(ArgNo(0;
+
+Optional StructTimespecTy = lookupType("timespec", ACtx);
+Optional StructTimespecPtrTy, ConstStructTimespecPtrTy;
+if (StructTimespecTy) {
+  StructTimespecPtrTy = ACtx.getPointerType(*StructTimespecTy);

[PATCH] D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune added a comment.

Made a patch at https://reviews.llvm.org/D84250 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83360



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

[PATCH] D83772: [Windows] Fix limit on command line size

[Max Kudryavtsev via Phabricator via cfe-commits]

max-kudr added a comment.

This commit is now failing LLDB Windows buildbot 
 builds 
http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/17702. Please 
fix or revert.

  
E:\build_slave\lldb-x64-windows-ninja\llvm-project\lldb\source\Host\windows\ProcessLauncherWindows.cpp(53):
 error C2679: binary '=': no operator found which takes a right-hand operand of 
type 'llvm::ErrorOr' (or there is no acceptable conversion)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83772



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

[PATCH] D83812: [clang][RelativeVTablesABI] Do not emit stubs for architectures that support a PLT relocation

[John McCall via Phabricator via cfe-commits]

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

Thanks, LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83812



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

[PATCH] D83820: Change metadata to deferred evalutaion in Clang Transformer.

[Andy Soffer via Phabricator via cfe-commits]

asoffer updated this revision to Diff 279572.
asoffer added a comment.

Construct types for default metadata explicitly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83820

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

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -440,6 +440,12 @@
 }
 
 TEST_F(TransformerTest, WithMetadata) {
+  auto makeMetadata = [](const MatchFinder::MatchResult &R) -> llvm::Any {
+int N =
+R.Nodes.getNodeAs("int")->getValue().getLimitedValue();
+return N;
+  };
+
   std::string Input = R"cc(
 int f() {
   int x = 5;
@@ -448,8 +454,11 @@
   )cc";
 
   Transformer T(
-  makeRule(declStmt().bind("decl"),
-   withMetadata(remove(statement(std::string("decl"))), 17)),
+  makeRule(
+  declStmt(containsDeclaration(0, varDecl(hasInitializer(
+  integerLiteral().bind("int")
+  .bind("decl"),
+  withMetadata(remove(statement(std::string("decl"))), makeMetadata)),
   consumer());
   T.registerMatchers(&MatchFinder);
   auto Factory = newFrontendActionFactory(&MatchFinder);
@@ -459,7 +468,7 @@
   ASSERT_EQ(Changes.size(), 1u);
   const llvm::Any &Metadata = Changes[0].getMetadata();
   ASSERT_TRUE(llvm::any_isa(Metadata));
-  EXPECT_THAT(llvm::any_cast(Metadata), 17);
+  EXPECT_THAT(llvm::any_cast(Metadata), 5);
 }
 
 TEST_F(TransformerTest, MultiChange) {
Index: clang/lib/Tooling/Transformer/RewriteRule.cpp
===
--- clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -44,10 +44,13 @@
 auto Replacement = E.Replacement->eval(Result);
 if (!Replacement)
   return Replacement.takeError();
+auto Metadata = E.Metadata(Result);
+if (!Metadata)
+  return Metadata.takeError();
 transformer::Edit T;
 T.Range = *EditRange;
 T.Replacement = std::move(*Replacement);
-T.Metadata = E.Metadata;
+T.Metadata = std::move(*Metadata);
 Edits.push_back(std::move(T));
   }
   return Edits;
Index: clang/include/clang/Tooling/Transformer/RewriteRule.h
===
--- clang/include/clang/Tooling/Transformer/RewriteRule.h
+++ clang/include/clang/Tooling/Transformer/RewriteRule.h
@@ -47,6 +47,8 @@
 
 using TextGenerator = std::shared_ptr>;
 
+using AnyGenerator = MatchConsumer;
+
 // Description of a source-code edit, expressed in terms of an AST node.
 // Includes: an ID for the (bound) node, a selector for source related to the
 // node, a replacement and, optionally, an explanation for the edit.
@@ -87,7 +89,12 @@
   RangeSelector TargetRange;
   TextGenerator Replacement;
   TextGenerator Note;
-  llvm::Any Metadata;
+  // Not all transformations will want or need to attach metadata and therefore
+  // should not be requierd to do so.
+  AnyGenerator Metadata = [](const ast_matchers::MatchFinder::MatchResult &)
+  -> llvm::Expected {
+return llvm::Expected(llvm::Any());
+  };
 };
 
 /// Lifts a list of `ASTEdit`s into an `EditGenerator`.
@@ -261,9 +268,27 @@
 /// Removes the source selected by \p S.
 ASTEdit remove(RangeSelector S);
 
-inline ASTEdit withMetadata(ASTEdit edit, llvm::Any Metadata) {
-  edit.Metadata = std::move(Metadata);
-  return edit;
+// FIXME: If `Metadata` returns an `llvm::Expected` the `AnyGenerator` will
+// construct an `llvm::Expected` where no error is present but the
+// `llvm::Any` holds the error. This is unlikely but potentially surprising.
+// Perhaps the `llvm::Expected` should be unwrapped, or perhaps this should be a
+// compile-time error. No solution here is perfect.
+//
+// Note: This function template accepts any type callable with a MatchResult
+// rather than a `std::function` because the return-type needs to be deduced. If
+// it accepted a `std::function`, lambdas or other callable types
+// would not be able to deduce `R`, and users would be forced to specify
+// explicitly the type they intended to return by wrapping the lambda at the
+// call-site.
+template 
+inline ASTEdit withMetadata(ASTEdit Edit, Callable Metadata) {
+  Edit.Metadata =
+  [Gen = std::move(Metadata)](
+  const ast_matchers::MatchFinder::MatchResult &R) -> llvm::Any {
+return Gen(R);
+  };
+
+  return Edit;
 }
 
 /// The following three functions are a low-level part of the RewriteRule
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83061: [OpenMP] Implement TR8 `present` map type modifier in Clang (1/2)

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

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



Comment at: clang/lib/Parse/ParseOpenMP.cpp:3067-3068
+  unsigned Type = getOpenMPSimpleClauseType(OMPC_map, PP.getSpelling(Tok));
+  if (Type < OMPC_MAP_MODIFIER_unknown)
+return OMPC_MAP_MODIFIER_unknown;
+  return static_cast(Type);

ABataev wrote:
> jdenny wrote:
> > ABataev wrote:
> > > jdenny wrote:
> > > > ABataev wrote:
> > > > > Why do we need this?
> > > > When called with `OMPC_map`, `getOpenMPSimpleClauseType` can return 
> > > > either an `OpenMPMapModifierKind` or `OpenMPMapClauseKind` depending on 
> > > > `Tok`.  Thus, without this change, both `isMapModifier` and `isMapType` 
> > > > can return either of those despite the difference in their names and 
> > > > documentation.
> > > > 
> > > > I found that, when `Parser::parseMapTypeModifiers` ignores `present` as 
> > > > if it's not a modifier because OpenMP < 5.1, then `isMapType` later 
> > > > returns `OMPC_MAP_MODIFIER_present` and causes the following assert to 
> > > > fail in `Sema::ActOnOpenMPVarListClause`:
> > > > 
> > > > ```
> > > > assert(0 <= ExtraModifier && ExtraModifier <= OMPC_MAP_unknown &&
> > > >"Unexpected map modifier.");
> > > > ```
> > > > 
> > > > To me, the most obvious solution is to fix `isMapType` and 
> > > > `isMapModifier`.  Fortunately, looking in `OpenMPKinds.h`, the 
> > > > enumerator values in `OpenMPMapModifierKind` and `OpenMPMapClauseKind` 
> > > > are disjoint so we can tell which we have.
> > > Can we have something like:
> > > ```
> > > if (LangOpts.OpenMP <= 50 && Type == OMPC_MAP_MODIFIER_present)
> > >   return OMPC_MAP_MODIFIER_unknown;
> > > ```
> > > or extend `getOpenMPSimpleClauseType` function with the version parameter 
> > > and check there is modifier is allowed and return `unknown` if it is not 
> > > compatible with provided version?
> > As far as I can tell, my changes general fix this bug in `isMapType` and 
> > `isMapModifier`.  It makes them behave as documented.  The suggestions 
> > you're making only fix them for the case of `present`.  Why is that better?
> It is too general. I think it may mask some issues in future. That's why I 
> would suggest to tweak it for `present` only. Or, even better, extend 
> `getOpenMPSimpleClauseType` function to handle this modifiers more correctly 
> for each particular version rather than fix it here.
> Or, even better, extend getOpenMPSimpleClauseType function to handle this 
> modifiers more correctly for each particular version rather than fix it here.

One way to implement what I think you mean is to add an additional parameter to 
`getOpenMPSimpleClauseType` to request either the clause's associated type or 
that type's modifiers.  Unless I missed a clause, this parameter would affect 
only map, defaultmap, and schedule clauses.  For other clauses, this parameter 
would be ignored.  Is that what you have in mind?


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

https://reviews.llvm.org/D83061



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

[PATCH] D81728: [InstCombine] Add target-specific inst combining

[Nicolai Hähnle via Phabricator via cfe-commits]

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

This has had a month of good review that has been addressed, I'd say it's good 
to go.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81728



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

[PATCH] D80873: [clang][cmake] Force CMAKE_LINKER for multistage build in case of BOOTSTRAP_LLVM_ENABLE_LLD and MSVC

[Kristina Bessonova via Phabricator via cfe-commits]

krisb added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80873



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

[PATCH] D80873: [clang][cmake] Force CMAKE_LINKER for multistage build in case of BOOTSTRAP_LLVM_ENABLE_LLD and MSVC

[Petr Hosek via Phabricator via cfe-commits]

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

LGTM




Comment at: clang/CMakeLists.txt:751
+  if(BOOTSTRAP_LLVM_ENABLE_LLD)
+if(MSVC AND NOT BOOTSTRAP_CMAKE_SYSTEM_NAME)
+  set(${CLANG_STAGE}_LINKER 
-DCMAKE_LINKER=${LLVM_RUNTIME_OUTPUT_INTDIR}/lld-link.exe)

krisb wrote:
> phosek wrote:
> > I don't understand the second part of this condition, can you elaborate? 
> > Why not set `CMAKE_LINKER` to `lld-link.exe` even if 
> > `BOOTSTRAP_CMAKE_SYSTEM_NAME STREQUAL "Windows"`?
> The only reason that stopped me from adding `BOOTSTRAP_CMAKE_SYSTEM_NAME 
> STREQUAL "Windows"` case here is that there is no way to keep it working in 
> the future (or, at least, I don't know one). Moreover, the build which sets 
> BOOTSTRAP_CMAKE_SYSTEM_NAME equal to "Windows" is currently broken.
> Since the same issue is exposed if to build LLVM with 
> `clang/cmake/caches/DistributionExample.cmake` on Windows, I included only 
> the changes that contribute to making this configuration buildable. I wanted 
> to avoid making the impression that some other configurations are supported 
> (because they are not) and also avoid adding any dead code that nobody would 
> use and that would easily be broken.
> 
I'd prefer to use `WIN32 OR BOOTSTRAP_CMAKE_SYSTEM_NAME STREQUAL "Windows"` 
here even if we don't support cross-compilation to Windows now as I hope it's 
something we're going to support in the future and this will be one less thing 
we need to address. Alternatively, please at least leave a `TODO` so it's 
easier to find later. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80873



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

[PATCH] D80514: [clang-tidy] modernize-use-trailing-return-type support for C++20 concepts and decltype

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:430
+  AT->getKeyword() == AutoTypeKeyword::Auto &&
+  !hasAnyNestedLocalQualifiers(F->getDeclaredReturnType()))
+return;

aaron.ballman wrote:
> Why do we need to check that there aren't any nested local qualifiers?
I'm still wondering about this.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-trailing-return-type.cpp:1
-// RUN: %check_clang_tidy -std=c++14,c++17 %s 
modernize-use-trailing-return-type %t -- -- -fdeclspec -fexceptions
+// RUN: %check_clang_tidy -std=c++14-or-later %s 
modernize-use-trailing-return-type %t -- -- -fdeclspec -fexceptions 
-DCOMMAND_LINE_INT=int
 // FIXME: Fix the checker to work in C++20 mode, it is performing a

The change to the language standard line makes me wonder if the fixme below it 
is now stale, or if the test will fail in C++20 mode.


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

https://reviews.llvm.org/D80514



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

[clang] e5b3202 - [libTooling] In Clang Transformer, change `Metadata` field to deferred evaluation.

[Yitzhak Mandelbaum via cfe-commits]

Author: Andy Soffer
Date: 2020-07-21T18:05:49Z
New Revision: e5b3202b6f9484590c9e70b8bb82d2778d1ca4fe

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

LOG: [libTooling] In Clang Transformer, change `Metadata` field to deferred 
evaluation.

`Metadata` is being changed from an `llvm::Any` to a `MatchConsumer`
so that it's evaluation can be be dependent on on `MatchResult`s passed in.

Reviewed By: ymandel, gribozavr2

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

Added: 


Modified: 
clang/include/clang/Tooling/Transformer/RewriteRule.h
clang/lib/Tooling/Transformer/RewriteRule.cpp
clang/unittests/Tooling/TransformerTest.cpp

Removed: 




diff  --git a/clang/include/clang/Tooling/Transformer/RewriteRule.h 
b/clang/include/clang/Tooling/Transformer/RewriteRule.h
index d9e68717d5c8..1be572736460 100644
--- a/clang/include/clang/Tooling/Transformer/RewriteRule.h
+++ b/clang/include/clang/Tooling/Transformer/RewriteRule.h
@@ -47,6 +47,8 @@ using EditGenerator = MatchConsumer>;
 
 using TextGenerator = std::shared_ptr>;
 
+using AnyGenerator = MatchConsumer;
+
 // Description of a source-code edit, expressed in terms of an AST node.
 // Includes: an ID for the (bound) node, a selector for source related to the
 // node, a replacement and, optionally, an explanation for the edit.
@@ -87,7 +89,12 @@ struct ASTEdit {
   RangeSelector TargetRange;
   TextGenerator Replacement;
   TextGenerator Note;
-  llvm::Any Metadata;
+  // Not all transformations will want or need to attach metadata and therefore
+  // should not be requierd to do so.
+  AnyGenerator Metadata = [](const ast_matchers::MatchFinder::MatchResult &)
+  -> llvm::Expected {
+return llvm::Expected(llvm::Any());
+  };
 };
 
 /// Lifts a list of `ASTEdit`s into an `EditGenerator`.
@@ -261,9 +268,27 @@ inline ASTEdit insertAfter(RangeSelector S, TextGenerator 
Replacement) {
 /// Removes the source selected by \p S.
 ASTEdit remove(RangeSelector S);
 
-inline ASTEdit withMetadata(ASTEdit edit, llvm::Any Metadata) {
-  edit.Metadata = std::move(Metadata);
-  return edit;
+// FIXME: If `Metadata` returns an `llvm::Expected` the `AnyGenerator` will
+// construct an `llvm::Expected` where no error is present but the
+// `llvm::Any` holds the error. This is unlikely but potentially surprising.
+// Perhaps the `llvm::Expected` should be unwrapped, or perhaps this should be 
a
+// compile-time error. No solution here is perfect.
+//
+// Note: This function template accepts any type callable with a MatchResult
+// rather than a `std::function` because the return-type needs to be deduced. 
If
+// it accepted a `std::function`, lambdas or other callable 
types
+// would not be able to deduce `R`, and users would be forced to specify
+// explicitly the type they intended to return by wrapping the lambda at the
+// call-site.
+template 
+inline ASTEdit withMetadata(ASTEdit Edit, Callable Metadata) {
+  Edit.Metadata =
+  [Gen = std::move(Metadata)](
+  const ast_matchers::MatchFinder::MatchResult &R) -> llvm::Any {
+return Gen(R);
+  };
+
+  return Edit;
 }
 
 /// The following three functions are a low-level part of the RewriteRule

diff  --git a/clang/lib/Tooling/Transformer/RewriteRule.cpp 
b/clang/lib/Tooling/Transformer/RewriteRule.cpp
index 995bec03cd66..a212a868c81d 100644
--- a/clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ b/clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -44,10 +44,13 @@ translateEdits(const MatchResult &Result, ArrayRef 
ASTEdits) {
 auto Replacement = E.Replacement->eval(Result);
 if (!Replacement)
   return Replacement.takeError();
+auto Metadata = E.Metadata(Result);
+if (!Metadata)
+  return Metadata.takeError();
 transformer::Edit T;
 T.Range = *EditRange;
 T.Replacement = std::move(*Replacement);
-T.Metadata = E.Metadata;
+T.Metadata = std::move(*Metadata);
 Edits.push_back(std::move(T));
   }
   return Edits;

diff  --git a/clang/unittests/Tooling/TransformerTest.cpp 
b/clang/unittests/Tooling/TransformerTest.cpp
index 7d6b63293748..59b334b0ea5a 100644
--- a/clang/unittests/Tooling/TransformerTest.cpp
+++ b/clang/unittests/Tooling/TransformerTest.cpp
@@ -440,6 +440,12 @@ TEST_F(TransformerTest, RemoveEdit) {
 }
 
 TEST_F(TransformerTest, WithMetadata) {
+  auto makeMetadata = [](const MatchFinder::MatchResult &R) -> llvm::Any {
+int N =
+R.Nodes.getNodeAs("int")->getValue().getLimitedValue();
+return N;
+  };
+
   std::string Input = R"cc(
 int f() {
   int x = 5;
@@ -448,8 +454,11 @@ TEST_F(TransformerTest, WithMetadata) {
   )cc";
 
   Transformer T(
-  makeRule(declStmt().bind("decl"),
-   withMetadata(remove(statement(std::string("decl"))), 17)),
+  makeRule(
+  

[PATCH] D83820: Change metadata to deferred evalutaion in Clang Transformer.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe5b3202b6f94: [libTooling] In Clang Transformer, change 
`Metadata` field to deferred… (authored by asoffer, committed by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83820

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

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -440,6 +440,12 @@
 }
 
 TEST_F(TransformerTest, WithMetadata) {
+  auto makeMetadata = [](const MatchFinder::MatchResult &R) -> llvm::Any {
+int N =
+R.Nodes.getNodeAs("int")->getValue().getLimitedValue();
+return N;
+  };
+
   std::string Input = R"cc(
 int f() {
   int x = 5;
@@ -448,8 +454,11 @@
   )cc";
 
   Transformer T(
-  makeRule(declStmt().bind("decl"),
-   withMetadata(remove(statement(std::string("decl"))), 17)),
+  makeRule(
+  declStmt(containsDeclaration(0, varDecl(hasInitializer(
+  integerLiteral().bind("int")
+  .bind("decl"),
+  withMetadata(remove(statement(std::string("decl"))), makeMetadata)),
   consumer());
   T.registerMatchers(&MatchFinder);
   auto Factory = newFrontendActionFactory(&MatchFinder);
@@ -459,7 +468,7 @@
   ASSERT_EQ(Changes.size(), 1u);
   const llvm::Any &Metadata = Changes[0].getMetadata();
   ASSERT_TRUE(llvm::any_isa(Metadata));
-  EXPECT_THAT(llvm::any_cast(Metadata), 17);
+  EXPECT_THAT(llvm::any_cast(Metadata), 5);
 }
 
 TEST_F(TransformerTest, MultiChange) {
Index: clang/lib/Tooling/Transformer/RewriteRule.cpp
===
--- clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -44,10 +44,13 @@
 auto Replacement = E.Replacement->eval(Result);
 if (!Replacement)
   return Replacement.takeError();
+auto Metadata = E.Metadata(Result);
+if (!Metadata)
+  return Metadata.takeError();
 transformer::Edit T;
 T.Range = *EditRange;
 T.Replacement = std::move(*Replacement);
-T.Metadata = E.Metadata;
+T.Metadata = std::move(*Metadata);
 Edits.push_back(std::move(T));
   }
   return Edits;
Index: clang/include/clang/Tooling/Transformer/RewriteRule.h
===
--- clang/include/clang/Tooling/Transformer/RewriteRule.h
+++ clang/include/clang/Tooling/Transformer/RewriteRule.h
@@ -47,6 +47,8 @@
 
 using TextGenerator = std::shared_ptr>;
 
+using AnyGenerator = MatchConsumer;
+
 // Description of a source-code edit, expressed in terms of an AST node.
 // Includes: an ID for the (bound) node, a selector for source related to the
 // node, a replacement and, optionally, an explanation for the edit.
@@ -87,7 +89,12 @@
   RangeSelector TargetRange;
   TextGenerator Replacement;
   TextGenerator Note;
-  llvm::Any Metadata;
+  // Not all transformations will want or need to attach metadata and therefore
+  // should not be requierd to do so.
+  AnyGenerator Metadata = [](const ast_matchers::MatchFinder::MatchResult &)
+  -> llvm::Expected {
+return llvm::Expected(llvm::Any());
+  };
 };
 
 /// Lifts a list of `ASTEdit`s into an `EditGenerator`.
@@ -261,9 +268,27 @@
 /// Removes the source selected by \p S.
 ASTEdit remove(RangeSelector S);
 
-inline ASTEdit withMetadata(ASTEdit edit, llvm::Any Metadata) {
-  edit.Metadata = std::move(Metadata);
-  return edit;
+// FIXME: If `Metadata` returns an `llvm::Expected` the `AnyGenerator` will
+// construct an `llvm::Expected` where no error is present but the
+// `llvm::Any` holds the error. This is unlikely but potentially surprising.
+// Perhaps the `llvm::Expected` should be unwrapped, or perhaps this should be a
+// compile-time error. No solution here is perfect.
+//
+// Note: This function template accepts any type callable with a MatchResult
+// rather than a `std::function` because the return-type needs to be deduced. If
+// it accepted a `std::function`, lambdas or other callable types
+// would not be able to deduce `R`, and users would be forced to specify
+// explicitly the type they intended to return by wrapping the lambda at the
+// call-site.
+template 
+inline ASTEdit withMetadata(ASTEdit Edit, Callable Metadata) {
+  Edit.Metadata =
+  [Gen = std::move(Metadata)](
+  const ast_matchers::MatchFinder::MatchResult &R) -> llvm::Any {
+return Gen(R);
+  };
+
+  return Edit;
 }
 
 /// The following three functions are a low-level part of the RewriteRule
___
cfe-commits mailing list
cfe-commits@lists.llvm.or

[PATCH] D83061: [OpenMP] Implement TR8 `present` map type modifier in Clang (1/2)

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Parse/ParseOpenMP.cpp:3067-3068
+  unsigned Type = getOpenMPSimpleClauseType(OMPC_map, PP.getSpelling(Tok));
+  if (Type < OMPC_MAP_MODIFIER_unknown)
+return OMPC_MAP_MODIFIER_unknown;
+  return static_cast(Type);

jdenny wrote:
> ABataev wrote:
> > jdenny wrote:
> > > ABataev wrote:
> > > > jdenny wrote:
> > > > > ABataev wrote:
> > > > > > Why do we need this?
> > > > > When called with `OMPC_map`, `getOpenMPSimpleClauseType` can return 
> > > > > either an `OpenMPMapModifierKind` or `OpenMPMapClauseKind` depending 
> > > > > on `Tok`.  Thus, without this change, both `isMapModifier` and 
> > > > > `isMapType` can return either of those despite the difference in 
> > > > > their names and documentation.
> > > > > 
> > > > > I found that, when `Parser::parseMapTypeModifiers` ignores `present` 
> > > > > as if it's not a modifier because OpenMP < 5.1, then `isMapType` 
> > > > > later returns `OMPC_MAP_MODIFIER_present` and causes the following 
> > > > > assert to fail in `Sema::ActOnOpenMPVarListClause`:
> > > > > 
> > > > > ```
> > > > > assert(0 <= ExtraModifier && ExtraModifier <= OMPC_MAP_unknown &&
> > > > >"Unexpected map modifier.");
> > > > > ```
> > > > > 
> > > > > To me, the most obvious solution is to fix `isMapType` and 
> > > > > `isMapModifier`.  Fortunately, looking in `OpenMPKinds.h`, the 
> > > > > enumerator values in `OpenMPMapModifierKind` and 
> > > > > `OpenMPMapClauseKind` are disjoint so we can tell which we have.
> > > > Can we have something like:
> > > > ```
> > > > if (LangOpts.OpenMP <= 50 && Type == OMPC_MAP_MODIFIER_present)
> > > >   return OMPC_MAP_MODIFIER_unknown;
> > > > ```
> > > > or extend `getOpenMPSimpleClauseType` function with the version 
> > > > parameter and check there is modifier is allowed and return `unknown` 
> > > > if it is not compatible with provided version?
> > > As far as I can tell, my changes general fix this bug in `isMapType` and 
> > > `isMapModifier`.  It makes them behave as documented.  The suggestions 
> > > you're making only fix them for the case of `present`.  Why is that 
> > > better?
> > It is too general. I think it may mask some issues in future. That's why I 
> > would suggest to tweak it for `present` only. Or, even better, extend 
> > `getOpenMPSimpleClauseType` function to handle this modifiers more 
> > correctly for each particular version rather than fix it here.
> > Or, even better, extend getOpenMPSimpleClauseType function to handle this 
> > modifiers more correctly for each particular version rather than fix it 
> > here.
> 
> One way to implement what I think you mean is to add an additional parameter 
> to `getOpenMPSimpleClauseType` to request either the clause's associated type 
> or that type's modifiers.  Unless I missed a clause, this parameter would 
> affect only map, defaultmap, and schedule clauses.  For other clauses, this 
> parameter would be ignored.  Is that what you have in mind?
I would also add a check for version compatibility if the modifier is supported 
for the given version. But anyway, I would like to take a look at what you have 
in mind


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

https://reviews.llvm.org/D83061



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

[clang-tools-extra] ff63d6b - [clangd] Fixes in lit tests

[Aleksandr Platonov via cfe-commits]

Author: Aleksandr Platonov
Date: 2020-07-21T21:21:40+03:00
New Revision: ff63d6be93dc5958bf35d92919ce6fafcc611e89

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

LOG: [clangd] Fixes in lit tests

Summary:
Changes:
- `background-index.test` Add Windows support.
- `did-change-configuration-params.test` Replace `cat | FileCheck` with 
`FileCheck --input-file`
- `test-uri-windows.test` This test did not run on Windows displite `REQUIRES: 
windows-gnu || windows-msvc` (replacement: `UNSUPPORTED: !(windows-gnu || 
windows-msvc)`).

Reviewers: sammccall, kadircet

Reviewed By: kadircet

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

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/test/background-index.test
clang-tools-extra/clangd/test/did-change-configuration-params.test
clang-tools-extra/clangd/test/test-uri-windows.test

Removed: 




diff  --git a/clang-tools-extra/clangd/test/background-index.test 
b/clang-tools-extra/clangd/test/background-index.test
index 41184443e947..c17abb43376f 100644
--- a/clang-tools-extra/clangd/test/background-index.test
+++ b/clang-tools-extra/clangd/test/background-index.test
@@ -1,23 +1,23 @@
-# We need to splice paths into file:// URIs for this test.
-# UNSUPPORTED: windows-msvc
-
 # Use a copy of inputs, as we'll mutate it (as will the background index).
-# RUN: rm -rf %t
-# RUN: cp -r %S/Inputs/background-index %t
+# RUN: rm -rf %/t
+# RUN: cp -r %/S/Inputs/background-index %/t
 # Need to embed the correct temp path in the actual JSON-RPC requests.
-# RUN: sed -i -e "s|DIRECTORY|%t|" %t/definition.jsonrpc
-# RUN: sed -i -e "s|DIRECTORY|%t|" %t/compile_commands.json
+# RUN: sed -i -e "s|DIRECTORY|%/t|" %/t/definition.jsonrpc
+# RUN: sed -i -e "s|DIRECTORY|%/t|" %/t/compile_commands.json
+# On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
+# (with the extra slash in the front), so we add it here.
+# RUN: sed -i -E -e 's|"file://([A-Z]):/|"file:///\1:/|g' 
%/t/definition.jsonrpc
 
 # We're editing bar.cpp, which includes foo.h.
 # foo() is declared in foo.h and defined in foo.cpp.
 # The background index should allow us to go-to-definition on foo().
 # We should also see indexing progress notifications.
-# RUN: clangd -background-index -lit-test < %t/definition.jsonrpc | FileCheck 
%t/definition.jsonrpc --check-prefixes=CHECK,BUILD
+# RUN: clangd -background-index -lit-test < %/t/definition.jsonrpc | FileCheck 
%/t/definition.jsonrpc --check-prefixes=CHECK,BUILD
 
 # Test that the index is writing files in the expected location.
-# RUN: ls %t/.cache/clangd/index/foo.cpp.*.idx
-# RUN: ls %t/sub_dir/.cache/clangd/index/foo.h.*.idx
+# RUN: ls %/t/.cache/clangd/index/foo.cpp.*.idx
+# RUN: ls %/t/sub_dir/.cache/clangd/index/foo.h.*.idx
 
 # Test the index is read from disk: delete code and restart clangd.
-# RUN: rm %t/foo.cpp
-# RUN: clangd -background-index -lit-test < %t/definition.jsonrpc | FileCheck 
%t/definition.jsonrpc --check-prefixes=CHECK,USE
+# RUN: rm %/t/foo.cpp
+# RUN: clangd -background-index -lit-test < %/t/definition.jsonrpc | FileCheck 
%/t/definition.jsonrpc --check-prefixes=CHECK,USE

diff  --git 
a/clang-tools-extra/clangd/test/did-change-configuration-params.test 
b/clang-tools-extra/clangd/test/did-change-configuration-params.test
index 4aef1011b370..19d41d0812e2 100644
--- a/clang-tools-extra/clangd/test/did-change-configuration-params.test
+++ b/clang-tools-extra/clangd/test/did-change-configuration-params.test
@@ -1,5 +1,5 @@
 # RUN: clangd -compile_args_from=lsp -lit-test < %s 2> %t | FileCheck 
-strict-whitespace %s
-# RUN: cat %t | FileCheck --check-prefix=ERR %s
+# RUN: FileCheck --check-prefix=ERR --input-file=%t %s
 # UNSUPPORTED: windows-gnu,windows-msvc
 
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
 ---

diff  --git a/clang-tools-extra/clangd/test/test-uri-windows.test 
b/clang-tools-extra/clangd/test/test-uri-windows.test
index 381c48fafc03..3f03b2985a70 100644
--- a/clang-tools-extra/clangd/test/test-uri-windows.test
+++ b/clang-tools-extra/clangd/test/test-uri-windows.test
@@ -1,5 +1,5 @@
 # RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
-# REQUIRES: windows-gnu || windows-msvc
+# UNSUPPORTED: !(windows-gnu || windows-msvc)
 # Test authority-less URI
 
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
 ---



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

[PATCH] D83759: [clangd] Fixes in lit tests

[Aleksandr Platonov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGff63d6be93dc: [clangd] Fixes in lit tests (authored by 
ArcsinX).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83759

Files:
  clang-tools-extra/clangd/test/background-index.test
  clang-tools-extra/clangd/test/did-change-configuration-params.test
  clang-tools-extra/clangd/test/test-uri-windows.test


Index: clang-tools-extra/clangd/test/test-uri-windows.test
===
--- clang-tools-extra/clangd/test/test-uri-windows.test
+++ clang-tools-extra/clangd/test/test-uri-windows.test
@@ -1,5 +1,5 @@
 # RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
-# REQUIRES: windows-gnu || windows-msvc
+# UNSUPPORTED: !(windows-gnu || windows-msvc)
 # Test authority-less URI
 
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
 ---
Index: clang-tools-extra/clangd/test/did-change-configuration-params.test
===
--- clang-tools-extra/clangd/test/did-change-configuration-params.test
+++ clang-tools-extra/clangd/test/did-change-configuration-params.test
@@ -1,5 +1,5 @@
 # RUN: clangd -compile_args_from=lsp -lit-test < %s 2> %t | FileCheck 
-strict-whitespace %s
-# RUN: cat %t | FileCheck --check-prefix=ERR %s
+# RUN: FileCheck --check-prefix=ERR --input-file=%t %s
 # UNSUPPORTED: windows-gnu,windows-msvc
 
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
 ---
Index: clang-tools-extra/clangd/test/background-index.test
===
--- clang-tools-extra/clangd/test/background-index.test
+++ clang-tools-extra/clangd/test/background-index.test
@@ -1,23 +1,23 @@
-# We need to splice paths into file:// URIs for this test.
-# UNSUPPORTED: windows-msvc
-
 # Use a copy of inputs, as we'll mutate it (as will the background index).
-# RUN: rm -rf %t
-# RUN: cp -r %S/Inputs/background-index %t
+# RUN: rm -rf %/t
+# RUN: cp -r %/S/Inputs/background-index %/t
 # Need to embed the correct temp path in the actual JSON-RPC requests.
-# RUN: sed -i -e "s|DIRECTORY|%t|" %t/definition.jsonrpc
-# RUN: sed -i -e "s|DIRECTORY|%t|" %t/compile_commands.json
+# RUN: sed -i -e "s|DIRECTORY|%/t|" %/t/definition.jsonrpc
+# RUN: sed -i -e "s|DIRECTORY|%/t|" %/t/compile_commands.json
+# On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
+# (with the extra slash in the front), so we add it here.
+# RUN: sed -i -E -e 's|"file://([A-Z]):/|"file:///\1:/|g' 
%/t/definition.jsonrpc
 
 # We're editing bar.cpp, which includes foo.h.
 # foo() is declared in foo.h and defined in foo.cpp.
 # The background index should allow us to go-to-definition on foo().
 # We should also see indexing progress notifications.
-# RUN: clangd -background-index -lit-test < %t/definition.jsonrpc | FileCheck 
%t/definition.jsonrpc --check-prefixes=CHECK,BUILD
+# RUN: clangd -background-index -lit-test < %/t/definition.jsonrpc | FileCheck 
%/t/definition.jsonrpc --check-prefixes=CHECK,BUILD
 
 # Test that the index is writing files in the expected location.
-# RUN: ls %t/.cache/clangd/index/foo.cpp.*.idx
-# RUN: ls %t/sub_dir/.cache/clangd/index/foo.h.*.idx
+# RUN: ls %/t/.cache/clangd/index/foo.cpp.*.idx
+# RUN: ls %/t/sub_dir/.cache/clangd/index/foo.h.*.idx
 
 # Test the index is read from disk: delete code and restart clangd.
-# RUN: rm %t/foo.cpp
-# RUN: clangd -background-index -lit-test < %t/definition.jsonrpc | FileCheck 
%t/definition.jsonrpc --check-prefixes=CHECK,USE
+# RUN: rm %/t/foo.cpp
+# RUN: clangd -background-index -lit-test < %/t/definition.jsonrpc | FileCheck 
%/t/definition.jsonrpc --check-prefixes=CHECK,USE


Index: clang-tools-extra/clangd/test/test-uri-windows.test
===
--- clang-tools-extra/clangd/test/test-uri-windows.test
+++ clang-tools-extra/clangd/test/test-uri-windows.test
@@ -1,5 +1,5 @@
 # RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
-# REQUIRES: windows-gnu || windows-msvc
+# UNSUPPORTED: !(windows-gnu || windows-msvc)
 # Test authority-less URI
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
 ---
Index: clang-tools-extra/clangd/test/did-change-configuration-params.test
===
--- clang-tools-extra/clangd/test/did-change-configuration-params.test
+++ clang-tools-extra/clangd/test/did-change-configuration-params.test
@@ -1,5 +1,5 @@
 # RUN: clangd -compile_args_from=lsp -lit-test < %s 2> %t | FileCheck -strict-whitespace %s
-# RUN: cat %t | FileCheck --check-prefix=ERR %s
+# RUN: FileCheck --check-prefix=ERR 

[PATCH] D83592: [Parser] Add comment to skipped regions

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

The testing looks really good. Just a few more requests for documentation 
(inline).




Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:309
 
+  /// Return a new SpellingRegion for the SkippedRange if it's valid.
+  Optional adjustSkippedRange(SourceManager &SM,

This could use some description of what adjustment is done, e.g. "This shrinks 
the skipped range if it spans a line that contains a non-comment token. If 
shrinking the skipped range would make it empty, this returns None."



Comment at: clang/lib/CodeGen/CoverageMappingGen.h:37
+  SourceLocation PrevTokLoc;
+  SourceLocation NextTokLoc;
+

It'd be helpful to leave inline documentation for these fields as well. It's 
clear from context that 'Range' is the skipped source range, but it's not as 
clear what {Prev,Next}TokLoc are.



Comment at: clang/lib/CodeGen/CoverageMappingGen.h:48
+class CoverageSourceInfo : public PPCallbacks, public CommentHandler {
+  // A pair of SkippedRanges and PrevTokLoc with NextTokLoc
+  std::vector SkippedRanges;

Please end sentences with a '.'



Comment at: clang/lib/CodeGen/CoverageMappingGen.h:53
 public:
-  ArrayRef getSkippedRanges() const { return SkippedRanges; }
+  // The location of previously token. It's updated when Preprocessor::Lex
+  // lexed a new token.

How does "Location of the token parsed before HandleComment is called. This is 
updated every time Preprocessor::Lex lexes a new token." sound?


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

https://reviews.llvm.org/D83592



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

[PATCH] D84048: DR2303: Prefer 'nearer' base classes during template deduction.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman removed a reviewer: aaron.ballman.
aaron.ballman added a comment.

While this looks reasonable to me, I don't feel comfortable signing off on it 
because Templates Are Complicated (switching myself to a subscriber instead).


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

https://reviews.llvm.org/D84048



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

[PATCH] D84192: [OpenMP5.0] map item can be non-contiguous for target update

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Also, what about compatibility with declare mapper? Can you add tests for it?




Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7809-7813
+MapValuesArrayTy CurOffsets{llvm::ConstantInt::get(CGF.CGM.Int64Ty, 0)};
+MapValuesArrayTy CurCounts{llvm::ConstantInt::get(CGF.CGM.Int64Ty, 1)};
+MapValuesArrayTy CurStrides;
+SmallVector DimSizes{
+llvm::ConstantInt::get(CGF.CGM.Int64Ty, 1)};

DO not use braced initializer list 
https://llvm.org/docs/CodingStandards.html#id26



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7839-7847
+if (CAT) {
+  ElementType = CAT->getElementType().getTypePtr();
+} else if (VAT) {
+  ElementType = VAT->getElementType().getTypePtr();
+} else {
+  assert(&Component == &*Components.begin() &&
+ "Only expect pointer (non CAT or VAT) when this is the "

No need for braces here per coding standard



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7876
+
+// Skip the dummy dimension since we have already have its information.
+auto DI = DimSizes.begin() + 1;

have already have



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7892
+  const Expr *AssocExpr = Component.getAssociatedExpression();
+  AssocExpr->dump();
+  const auto *OASE = dyn_cast(AssocExpr);

debug code



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7893
+  AssocExpr->dump();
+  const auto *OASE = dyn_cast(AssocExpr);
+  const auto *AE = dyn_cast(AssocExpr);

Move this after the first `if` statement



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7894-7896
+  const auto *AE = dyn_cast(AssocExpr);
+
+  if (AE) {

Better to make it something like:
```
if (const auto *AE = dyn_cast(AssocExpr))
```



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7944-7945
+  : llvm::ConstantInt::get(CGF.Int64Ty, /*V=*/1);
+  Count = CGF.Builder.CreateUDiv(CGF.Builder.CreateNUWSub(*DI, Offset),
+ Stride);
+}

If no stride at all, why need to divide?



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7967
+  DimProd = CGF.Builder.CreateNUWMul(DimProd, *(DI - 1));
+  CurStrides.push_back(CGF.Builder.CreateNUWMul(DimProd, Stride));
+  if (DI != DimSizes.end())

Same, if no stride at all, no need to mul.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:16913-16919
+if (OASE) {
+  if (!OASE->getLength())
+SemaRef.Diag(ELoc, diag::err_array_section_does_not_specify_length)
+<< ERange;
+  else
+break;
+}

Better to transform it to something like:
```
if (!OASE || OASE->getLength())
  break;
SemaRef.Diag(...)
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84192



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

[PATCH] D83592: [Coverage] Add comment to skipped regions

[Zequan Wu via Phabricator via cfe-commits]

zequanwu updated this revision to Diff 279595.
zequanwu marked 2 inline comments as done.
zequanwu retitled this revision from "[Parser] Add comment to skipped regions" 
to "[Coverage] Add comment to skipped regions".
zequanwu edited the summary of this revision.
zequanwu added a comment.

- Address comments
- Update summary.


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

https://reviews.llvm.org/D83592

Files:
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/lib/CodeGen/CoverageMappingGen.h
  clang/lib/Lex/Preprocessor.cpp
  clang/test/CoverageMapping/break.c
  clang/test/CoverageMapping/builtinmacro.c
  clang/test/CoverageMapping/classtemplate.cpp
  clang/test/CoverageMapping/comment-in-macro.c
  clang/test/CoverageMapping/continue.c
  clang/test/CoverageMapping/coroutine.cpp
  clang/test/CoverageMapping/deferred-region.cpp
  clang/test/CoverageMapping/if.cpp
  clang/test/CoverageMapping/includehell.cpp
  clang/test/CoverageMapping/label.cpp
  clang/test/CoverageMapping/logical.cpp
  clang/test/CoverageMapping/loops.cpp
  clang/test/CoverageMapping/macro-expressions.cpp
  clang/test/CoverageMapping/macroparams2.c
  clang/test/CoverageMapping/macros.c
  clang/test/CoverageMapping/macroscopes.cpp
  clang/test/CoverageMapping/moremacros.c
  clang/test/CoverageMapping/objc.m
  clang/test/CoverageMapping/pr32679.cpp
  clang/test/CoverageMapping/preprocessor.c
  clang/test/CoverageMapping/return.c
  clang/test/CoverageMapping/switch.cpp
  clang/test/CoverageMapping/switchmacro.c
  clang/test/CoverageMapping/test.c
  clang/test/CoverageMapping/trycatch.cpp
  clang/test/CoverageMapping/unreachable-macro.c
  clang/test/CoverageMapping/while.c
  clang/test/lit.cfg.py
  compiler-rt/test/profile/Inputs/instrprof-comdat.h
  compiler-rt/test/profile/coverage_comments.cpp

Index: compiler-rt/test/profile/coverage_comments.cpp
===
--- /dev/null
+++ compiler-rt/test/profile/coverage_comments.cpp
@@ -0,0 +1,71 @@
+// RUN: %clangxx_profgen -fcoverage-mapping -Wno-comment -o %t %s
+// RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t
+// RUN: llvm-profdata merge -o %t.profdata %t.profraw
+// RUN: llvm-cov show %t -instr-profile %t.profdata -path-equivalence=/tmp,%S 2>&1 | FileCheck %s
+
+int main() {   // CHECK:  [[@LINE]]| 1|int main() {
+/* comment */ int x = 0;   // CHECK:  [[@LINE]]| 1|  /* comment */ int x = 0;
+int y = 0; /* comment */   // CHECK:  [[@LINE]]| 1|  int y = 0; /* comment */
+int z = 0; // comment  // CHECK:  [[@LINE]]| 1|  int z = 0; // comment
+// comment // CHECK:  [[@LINE]]|  |  // comment
+   // CHECK:  [[@LINE]]|  |
+x = 0; /*  // CHECK:  [[@LINE]]| 1|  x = 0; /*
+comment// CHECK:  [[@LINE]]|  |comment
+*/ // CHECK:  [[@LINE]]|  |*/
+   // CHECK:  [[@LINE]]|  |
+/* // CHECK:  [[@LINE]]|  |/*
+comment// CHECK:  [[@LINE]]|  |comment
+*/ x = 0;  // CHECK:  [[@LINE]]| 1|*/ x = 0;
+   // CHECK:  [[@LINE]]|  |
+/* comment */  // CHECK:  [[@LINE]]|  |/* comment */
+// comment // CHECK:  [[@LINE]]|  |// comment
+/* comment */  // CHECK:  [[@LINE]]|  |/* comment */
+z =// CHECK:  [[@LINE]]| 1|z =
+x // comment   // CHECK:  [[@LINE]]| 1|x // comment
+// comment // CHECK:  [[@LINE]]|  |// comment
++ /*   // CHECK:  [[@LINE]]| 1|+ /*
+comment// CHECK:  [[@LINE]]|  |comment
+*/ // CHECK:  [[@LINE]]|  |*/
+/* // CHECK:  [[@LINE]]|  |/*
+comment// CHECK:  [[@LINE]]|  |comment
+*/y;   // CHECK:  [[@LINE]]| 1|*/y;
+   // CHECK:  [[@LINE]]|  |
+// Comments inside directives. // CHECK:  [[@LINE]]|  |// Comments inside directives.
+#if 0 //comment// CHECK:  [[@LINE]]|  |#if 0 //comment
+/* comment */ x = 0;   // CHECK:  [[@LINE]]|  |/* comment */ x = 0;
+y = 0; /* comment */   // CHECK:  [[@LINE]]|  |y = 0; /* comment */
+z = 0; // comment  // CHECK:  [[@LINE]]|  |z = 0; // comment
+// comment // CHECK:  [[@LINE]]|  |// comment
+   // CHECK:  [[@LI

[PATCH] D83261: [OPENMP]Redesign of OMPExecutableDirective representation.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D83261#2162904 , @Meinersbur wrote:

> In D83261#2162561 , @ABataev wrote:
>
> > 1. OMPChildren class uses standard TrailingObjects harness instead of 
> > manual calculation.
>
>
> I was going to argue that OMPExecutableDirective could derive from 
> TrailingObjects as well, but it requires a template parameter for its final 
> subclass. Good point!
>
> > 2. Child Stmt* based nodes are not related to the AsssociatedStmt* anymore 
> > and can exist independently.
>
> It moved into the OMPChildren class. Not sure whether it is better.


What I mean, that previously child nodes could not exist without 
AssociatedStmt, i.e. you have to have AssociatedStmt to have other children. It 
is solved (though, of course, can be implemented in a different way with the 
current design).

> 
> 
>>> OMPChildren also handles clauses for OMPExecutableDirectives but not for 
>>> declarative directives. Should handling of of clauses also be extracted 
>>> into into its own class? That would make (de-)serialization easier for 
>>> those as well.
>> 
>> This class is only for executable directives.
> 
> Nearly all directives have clauses. At the moment they all implement their 
> own clause list. I don't see why clause management should be centralized for 
> executable statements only.

Sure, we can make `OMPChildren` common for declarative and executable 
directives. Do you want me to do it?

> 
> 
>>> There is no effect on D76342  (except a 
>>> requiring a rebase), since the OMPTileDirective has children and thus does 
>>> not profit from the functionality of `OMPChildren` becoming optional. Since 
>>> I don't see a relation to D76342 , more , 
>>> I am indifferent to whether this should be merged.
>> 
>> There should be an additional patch, which, I hope, should simplify things 
>> for loop-based directives.
> 
> OK. What does this patch do? Are you going to upload it as well?

At first, need to deal with this one, at least come to an agreement with the 
design.

> 
> 
>>> Trailing objects is a technique to ensure that all substmts are consecutive 
>>> in memory (so `StmtIterator` can iterator over them). For 
>>> OMPExeuctableDirectives, only the associated statement is returned by the 
>>> `StmtIterator`, i.e. all the children could be made regular class members 
>>> without the complication of computing the offset. I'd prefer that change 
>>> over OMPChildren.
>> 
>> There are also used_children, which are used by the clang analyzer for, at 
>> least, use of uninitialized variables diagnostics.
> 
> OK, I see.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83261



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

[PATCH] D83188: [clang-tidy] bugprone-bool-pointer-implicit-conversion doesn't handle members

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

LGTM, though I have a possible code simplification if you think it's an 
improvement.




Comment at: 
clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp:78
+
+  if (const auto *DeclRef = Result.Nodes.getNodeAs("expr")) {
+const Decl *D = DeclRef->getDecl();

Can you get away with:
```
if (const auto *E = Result.Nodes.getNodeAs("expr")) {
  const Decl *D = isa ? cast(E)->getDecl() : 
cast(E)->getMemberDecl();
  const auto M = ignoringParenImpCasts(anyOf(declRefExpr(to(equalsNode(D))), 
memberExpr(hasDeclaration(equalsNode(D);
  checkImpl(Result, E, If, M, *this);
}
```
(Totally untested, but it hopefully demonstrates the point.)


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

https://reviews.llvm.org/D83188



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

[PATCH] D84048: DR2303: Prefer 'nearer' base classes during template deduction.

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 279598.
erichkeane added a comment.

Also make sure to update the the cxx_dr_status.html document.

Additionally, I sent out a CWG message on the reflector about this: 
https://godbolt.org/z/bEr61T

My implementation has this as ambiguous, but the wording makes it well-formed I 
think.  I don't think that was the intent of CWG2303, so unless they confirm  
that this was their intent (or that I'm wrong), I'll leave this patch as is.


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

https://reviews.llvm.org/D84048

Files:
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/test/CXX/drs/dr23xx.cpp
  clang/www/cxx_dr_status.html

Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -13633,7 +13633,7 @@
 https://wg21.link/cwg2303";>2303
 DRWP
 Partial ordering and recursive variadic inheritance
-Unknown
+Yes
   
   
 https://wg21.link/cwg2304";>2304
Index: clang/test/CXX/drs/dr23xx.cpp
===
--- clang/test/CXX/drs/dr23xx.cpp
+++ clang/test/CXX/drs/dr23xx.cpp
@@ -113,3 +113,26 @@
   extern template const int d;
 #endif
 }
+
+#if __cplusplus >= 201103L
+namespace dr2303 {
+template 
+struct A;
+template <>
+struct A<> {};
+template 
+struct A : A {};
+struct B : A {};
+
+template 
+void f(const A &);
+template 
+void f2(const A *);
+
+void g() {
+  f(B{}); // This is no longer ambiguous.
+  B b;
+  f2(&b);
+}
+} //namespace dr2303
+#endif
Index: clang/lib/Sema/SemaTemplateDeduction.cpp
===
--- clang/lib/Sema/SemaTemplateDeduction.cpp
+++ clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -1792,7 +1792,10 @@
   //   transformed A can be a derived class of the deduced A. Likewise if
   //   P is a pointer to a class of the form simple-template-id, the
   //   transformed A can be a pointer to a derived class pointed to by the
-  //   deduced A.
+  //   deduced A. However, if there is a class C that is a (direct or
+  //   indirect) base class of D and derived (directly or indirectly) from a
+  //   class B and that would be a valid deduced A, the deduced A cannot be
+  //   B or pointer to B, respectively.
   //
   //   These alternatives are considered only if type deduction would
   //   otherwise fail. If they yield more than one possible deduced A, the
@@ -1812,6 +1815,7 @@
   while (!ToVisit.empty()) {
 // Retrieve the next class in the inheritance hierarchy.
 const RecordType *NextT = ToVisit.pop_back_val();
+bool SkipBases = false;
 
 // If we have already seen this type, skip it.
 if (!Visited.insert(NextT).second)
@@ -1840,17 +1844,28 @@
 Info.Param = BaseInfo.Param;
 Info.FirstArg = BaseInfo.FirstArg;
 Info.SecondArg = BaseInfo.SecondArg;
+
+// In order to implement CWG2303 (added the following to p4b3):
+//   However, if there is a class C that is a (direct or indirect)
+//   base class of D and derived (directly or indirectly) from a
+//   class B and that would be a valid deduced A, the deduced A
+//   cannot be B or pointer to B, respectively.
+// We shouldn't visit the bases of a successful match ('C'), as they
+// could only be 'B' here.
+SkipBases = true;
   }
 
   Deduced = DeducedOrig;
 }
 
 // Visit base classes
-CXXRecordDecl *Next = cast(NextT->getDecl());
-for (const auto &Base : Next->bases()) {
-  assert(Base.getType()->isRecordType() &&
- "Base class that isn't a record?");
-  ToVisit.push_back(Base.getType()->getAs());
+if (!SkipBases) {
+  CXXRecordDecl *Next = cast(NextT->getDecl());
+  for (const auto &Base : Next->bases()) {
+assert(Base.getType()->isRecordType() &&
+   "Base class that isn't a record?");
+ToVisit.push_back(Base.getType()->getAs());
+  }
 }
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83592: [Coverage] Add comment to skipped regions

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

New tests should use `[[#@LINE]]` instead of `[[@LINE]]`

https://llvm.org/docs/CommandGuide/FileCheck.html

> To support legacy uses of @LINE as a special string variable, FileCheck also 
> accepts the following uses of @LINE with string substitution block syntax: 
> [[@LINE]], [[@LINE+]] and [[@LINE-]] without any spaces 
> inside the brackets and where offset is an integer.


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

https://reviews.llvm.org/D83592



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

[PATCH] D84225: [CFE] Add nomerge function attribute to inline assembly.

[Zequan Wu via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84225



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