[PATCH] D85666: [clang-tidy] IncludeInserter: allow <> in header name

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp:31
utils::IncludeSorter::IS_LLVM)),
-  MathHeader(Options.get("MathHeader", "math.h")) {}
+  MathHeader(Options.get("MathHeader", "")) {}
 

this looks like a bug fix, I think the check lit test didn't cover it before, 
maybe add a header verification in 
`test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp`.



Comment at: clang-tools-extra/clang-tidy/utils/IncludeInserter.h:71
   /// Returns ``llvm::None`` on error or if the inclusion directive already
   /// exists.
   llvm::Optional

Add a FIXME saying this would be removed eventually.



Comment at: clang-tools-extra/clang-tidy/utils/IncludeInserter.h:85
   /// Returns``llvm::None`` on error or if the inclusion directive already
   /// exists.
   llvm::Optional

The same here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85666

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

[PATCH] D85645: [AST] Fix the CXXFoldExpr source range when parentheses range is invalid.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 284592.
hokein marked an inline comment as done.
hokein added a comment.

address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85645

Files:
  clang/include/clang/AST/ExprCXX.h
  clang/test/AST/ast-dump-concepts.cpp


Index: clang/test/AST/ast-dump-concepts.cpp
===
--- clang/test/AST/ast-dump-concepts.cpp
+++ clang/test/AST/ast-dump-concepts.cpp
@@ -12,6 +12,9 @@
 template 
 concept binary_concept = true;
 
+template 
+concept variadic_concept = true;
+
 template 
 struct Foo {
   // CHECK:  TemplateTypeParmDecl {{.*}} referenced Concept {{.*}} 
'binary_concept'
@@ -37,4 +40,12 @@
   template 
   Foo(R, char) requires unary_concept {
   }
+
+  // CHECK: CXXFoldExpr {{.*}} 
+  template 
+  Foo();
+
+  // CHECK: CXXFoldExpr {{.*}} 
+  template ... Ts>
+  Foo();
 };
Index: clang/include/clang/AST/ExprCXX.h
===
--- clang/include/clang/AST/ExprCXX.h
+++ clang/include/clang/AST/ExprCXX.h
@@ -4575,9 +4575,21 @@
 return None;
   }
 
-  SourceLocation getBeginLoc() const LLVM_READONLY { return LParenLoc; }
+  SourceLocation getBeginLoc() const LLVM_READONLY {
+if (LParenLoc.isValid())
+  return LParenLoc;
+if (isLeftFold())
+  return getEllipsisLoc();
+return getLHS()->getBeginLoc();
+  }
 
-  SourceLocation getEndLoc() const LLVM_READONLY { return RParenLoc; }
+  SourceLocation getEndLoc() const LLVM_READONLY {
+if (RParenLoc.isValid())
+  return RParenLoc;
+if (isRightFold())
+  return getEllipsisLoc();
+return getRHS()->getEndLoc();
+  }
 
   static bool classof(const Stmt *T) {
 return T->getStmtClass() == CXXFoldExprClass;


Index: clang/test/AST/ast-dump-concepts.cpp
===
--- clang/test/AST/ast-dump-concepts.cpp
+++ clang/test/AST/ast-dump-concepts.cpp
@@ -12,6 +12,9 @@
 template 
 concept binary_concept = true;
 
+template 
+concept variadic_concept = true;
+
 template 
 struct Foo {
   // CHECK:  TemplateTypeParmDecl {{.*}} referenced Concept {{.*}} 'binary_concept'
@@ -37,4 +40,12 @@
   template 
   Foo(R, char) requires unary_concept {
   }
+
+  // CHECK: CXXFoldExpr {{.*}} 
+  template 
+  Foo();
+
+  // CHECK: CXXFoldExpr {{.*}} 
+  template ... Ts>
+  Foo();
 };
Index: clang/include/clang/AST/ExprCXX.h
===
--- clang/include/clang/AST/ExprCXX.h
+++ clang/include/clang/AST/ExprCXX.h
@@ -4575,9 +4575,21 @@
 return None;
   }
 
-  SourceLocation getBeginLoc() const LLVM_READONLY { return LParenLoc; }
+  SourceLocation getBeginLoc() const LLVM_READONLY {
+if (LParenLoc.isValid())
+  return LParenLoc;
+if (isLeftFold())
+  return getEllipsisLoc();
+return getLHS()->getBeginLoc();
+  }
 
-  SourceLocation getEndLoc() const LLVM_READONLY { return RParenLoc; }
+  SourceLocation getEndLoc() const LLVM_READONLY {
+if (RParenLoc.isValid())
+  return RParenLoc;
+if (isRightFold())
+  return getEllipsisLoc();
+return getRHS()->getEndLoc();
+  }
 
   static bool classof(const Stmt *T) {
 return T->getStmtClass() == CXXFoldExprClass;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85645: [AST] Fix the CXXFoldExpr source range when parentheses range is invalid.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang/include/clang/AST/ExprCXX.h:4587
+  return RParenLoc;
+return getPattern()->getEndLoc();
+  }

nridge wrote:
> Wouldn't `EllipsisLoc` be more accurate?
yeah, that sounds more reasonable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85645

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

[PATCH] D85635: [clangd] Compute the inactive code range for semantic highlighting.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D85635#2208325 , @nridge wrote:

> Do you mean "inactive" (instead of "interactive") in the commit message?

oh, right. Fixed the typo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85635

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

Re: [clang] 6170072 - Improve modeling of variable template specializations with dependent

[Martin Storsjö via cfe-commits]

On Sun, 9 Aug 2020, Richard Smith via cfe-commits wrote:



Author: Richard Smith
Date: 2020-08-09T23:22:26-07:00
New Revision: 617007240cbfb97c8ccf6d61b0c4ca0bb62d43c9

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

LOG: Improve modeling of variable template specializations with dependent
arguments.

Don't build a variable template specialization declaration until its
scope and template arguments are non-dependent.

No functionality change intended, but the AST representation is now more
consistent with how we model other templates.


This did turn out to make a functional change, breaking building the dev 
branch of Qt. A halfway reduced example below:



template 
struct integral_constant
{
  static constexpr const _Tp value = __v;
  typedef _Tp value_type;
  typedef integral_constant type;
  __attribute__ ((__exclude_from_explicit_instantiation__))
  constexpr operator value_type() const noexcept {return value;}

  __attribute__ ((__exclude_from_explicit_instantiation__))
  constexpr value_type operator ()() const noexcept {return value;}
};


template 
struct is_nothrow_assignable
: public integral_constant 
{};

template 
inline constexpr bool is_nothrow_assignable_v
= is_nothrow_assignable<_Tp, _Arg>::value;


template 
class QCache
{
struct Value {
T *t = nullptr;
};

struct Node {
Value value;

void replace(const Value &t) noexcept(is_nothrow_assignable_v) {
value = t;
}
};

};



// Martin

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

[PATCH] D85545: [Diagnostics] Diagnose missing comma in string array initialization

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

xbolva00 added a comment.

Thanks for feedback:)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85545

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

[PATCH] D85710: Code generation support for lvalue bit-field conditionals.

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

rsmith created this revision.
rsmith added a reviewer: rjmccall.
Herald added subscribers: cfe-commits, jfb.
Herald added a project: clang.
rsmith requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.

C++ permits use of (cond ? x.a : y.b) as an lvalue even when a and/or b is a
bit-field. We previously failed to generate IR for such constructs.

Handle these cases with a new form of LValue representing a choice between two
other LValues. Whenever we want to generate a primitive operation on a
conditional lvalue, generate a branch to perform the operation.

This would be sufficient to also support lvalue conditionals between other
kinds of non-simple lvalue, such as a conditional between the imaginary
component of a _Complex T and a vector element, but Sema rejects all the other
combinations for the moment.

One case that's not yet supported is atomic compare-exchange operations on
bitfield conditional lvalues. These only arise through `#pragma omp atomic`
constructs, and would probably not be too hard to support, but we need to teach
the OpenMP code to cope with its subexpression being emitted multiple times.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85710

Files:
  clang/lib/CodeGen/CGAtomic.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CGValue.h
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGenCXX/conditional-expr-lvalue.cpp

Index: clang/test/CodeGenCXX/conditional-expr-lvalue.cpp
===
--- clang/test/CodeGenCXX/conditional-expr-lvalue.cpp
+++ clang/test/CodeGenCXX/conditional-expr-lvalue.cpp
@@ -1,8 +1,15 @@
-// RUN: %clang_cc1 -emit-llvm-only %s
-void f(bool flag) {
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -o - -fopenmp > /tmp/tmp.ir
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -o - -fopenmp | FileCheck %s
+
+// CHECK-LABEL: define {{.*}} @{{.*}}simple_scalar_cond
+void simple_scalar_cond(bool flag) {
+  // CHECK: store i32 1, i32* %[[A:.*]],
   int a = 1;
+  // CHECK: store i32 2, i32* %[[B:.*]],
   int b = 2;
   
+  // CHECK: %[[LVAL:.*]] = phi i32* [ %[[A]], {{.*}} ], [ %[[B]], {{.*}} ]
+  // CHECK: store i32 3, i32* %[[LVAL]],
   (flag ? a : b) = 3;
 }
 
@@ -14,7 +21,173 @@
 A sub() const;
 void foo() const;
   };
+  // CHECK-LABEL: define {{.*}} @{{.*}}test0
   void foo(bool cond, const A &a) {
+// CHECK: %[[TMP:.*]] = alloca %"struct.test0::A",
+// CHECK: br i1
+//
+// CHECK: call void @_ZN5test01AC1ERKS0_(%"struct.test0::A"* %[[TMP]],
+// CHECK: br label
+//
+// CHECK: call void @_ZNK5test01A3subEv(%"struct.test0::A"* sret align 1 %[[TMP]],
+// CHECK: br label
+//
+// CHECK: call void @_ZNK5test01A3fooEv(%"struct.test0::A"* %[[TMP]])
 (cond ? a : a.sub()).foo();
   }
 }
+
+namespace bitfields {
+  struct A {
+int m : 3;
+int n : 5;
+  } a1, a2;
+
+  // CHECK-LABEL: define {{.*}} @{{.*}}load
+  int load(bool b) {
+// CHECK: br i1
+//
+// CHECK: load {{.*}}@_ZN9bitfields2a1E, i32 0, i32 0
+// CHECK: shl {{.*}}, 5
+// CHECK: ashr {{.*}}, 5
+// CHECK: br
+//
+// CHECK: load {{.*}}@_ZN9bitfields2a2E, i32 0, i32 0
+// CHECK: ashr {{.*}}, 3
+// CHECK: br
+return b ? a1.m : a2.n;
+  }
+
+  // CHECK-LABEL: define {{.*}} @{{.*}}store
+  void store(bool b, int k) {
+// CHECK: br i1
+// CHECK: %[[SEL:.*]] = phi i1
+// CHECK: %[[A1:.*]] = phi i8* {{.*}} @_ZN9bitfields2a1E, {{.*}} [ undef,
+// CHECK: %[[A2:.*]] = phi i8* {{.*}} @_ZN9bitfields2a2E, {{.*}} [ undef,
+// CHECK: br i1 %[[SEL]]
+//
+// CHECK: load {{.*}}%[[A1]]
+// CHECK: and {{.*}}, 7
+// CHECK: and {{.*}}, -8
+// CHECK: or
+// CHECK: store
+// CHECK: br label
+//
+// CHECK: load {{.*}}%[[A2]]
+// CHECK: and {{.*}}, 31
+// CHECK: shl {{.*}}, 3
+// CHECK: and {{.*}}, 7
+// CHECK: or
+// CHECK: store
+// CHECK: br label
+(b ? a1.m : a2.n) = k;
+  }
+
+  // CHECK-LABEL: define {{.*}} @{{.*}}update
+  void update(bool b, int k) {
+// 1: Branch on condition and evaluate the chosen lvalue.
+//
+// CHECK: br i1
+// CHECK: %[[SEL:.*]] = phi i1
+// CHECK: %[[A1:.*]] = phi i8* {{.*}} @_ZN9bitfields2a1E, {{.*}} [ undef,
+// CHECK: %[[A2:.*]] = phi i8* {{.*}} @_ZN9bitfields2a2E, {{.*}} [ undef,
+// CHECK: br i1 %[[SEL]]
+//
+// 2: Branch on condition and load the relevant value.
+//
+// CHECK: load {{.*}}%[[A1]]
+// CHECK: shl {{.*}}, 5
+// CHECK: ashr {{.*}}, 5
+// CHECK: br
+//
+// CHECK: load {{.*}}%[[A2]]
+// CHECK: ashr {{.*}}, 3
+// CHECK: br
+//
+// 3: Add k.
+//
+// CHECK: add nsw
+// CHECK: br i1
+//
+// 4: Branch on condition and store the new value.
+//
+// CHECK: load {{.*}}%[[A1]]
+// CHECK: and {{.*}}, 7
+// CHECK: and {{.

[PATCH] D85710: Code generation support for lvalue bit-field conditionals.

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

rsmith updated this revision to Diff 284594.
rsmith added a comment.

- Remove development aid from test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85710

Files:
  clang/lib/CodeGen/CGAtomic.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CGValue.h
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGenCXX/conditional-expr-lvalue.cpp

Index: clang/test/CodeGenCXX/conditional-expr-lvalue.cpp
===
--- clang/test/CodeGenCXX/conditional-expr-lvalue.cpp
+++ clang/test/CodeGenCXX/conditional-expr-lvalue.cpp
@@ -1,8 +1,14 @@
-// RUN: %clang_cc1 -emit-llvm-only %s
-void f(bool flag) {
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -o - -fopenmp | FileCheck %s
+
+// CHECK-LABEL: define {{.*}} @{{.*}}simple_scalar_cond
+void simple_scalar_cond(bool flag) {
+  // CHECK: store i32 1, i32* %[[A:.*]],
   int a = 1;
+  // CHECK: store i32 2, i32* %[[B:.*]],
   int b = 2;
   
+  // CHECK: %[[LVAL:.*]] = phi i32* [ %[[A]], {{.*}} ], [ %[[B]], {{.*}} ]
+  // CHECK: store i32 3, i32* %[[LVAL]],
   (flag ? a : b) = 3;
 }
 
@@ -14,7 +20,173 @@
 A sub() const;
 void foo() const;
   };
+  // CHECK-LABEL: define {{.*}} @{{.*}}test0
   void foo(bool cond, const A &a) {
+// CHECK: %[[TMP:.*]] = alloca %"struct.test0::A",
+// CHECK: br i1
+//
+// CHECK: call void @_ZN5test01AC1ERKS0_(%"struct.test0::A"* %[[TMP]],
+// CHECK: br label
+//
+// CHECK: call void @_ZNK5test01A3subEv(%"struct.test0::A"* sret align 1 %[[TMP]],
+// CHECK: br label
+//
+// CHECK: call void @_ZNK5test01A3fooEv(%"struct.test0::A"* %[[TMP]])
 (cond ? a : a.sub()).foo();
   }
 }
+
+namespace bitfields {
+  struct A {
+int m : 3;
+int n : 5;
+  } a1, a2;
+
+  // CHECK-LABEL: define {{.*}} @{{.*}}load
+  int load(bool b) {
+// CHECK: br i1
+//
+// CHECK: load {{.*}}@_ZN9bitfields2a1E, i32 0, i32 0
+// CHECK: shl {{.*}}, 5
+// CHECK: ashr {{.*}}, 5
+// CHECK: br
+//
+// CHECK: load {{.*}}@_ZN9bitfields2a2E, i32 0, i32 0
+// CHECK: ashr {{.*}}, 3
+// CHECK: br
+return b ? a1.m : a2.n;
+  }
+
+  // CHECK-LABEL: define {{.*}} @{{.*}}store
+  void store(bool b, int k) {
+// CHECK: br i1
+// CHECK: %[[SEL:.*]] = phi i1
+// CHECK: %[[A1:.*]] = phi i8* {{.*}} @_ZN9bitfields2a1E, {{.*}} [ undef,
+// CHECK: %[[A2:.*]] = phi i8* {{.*}} @_ZN9bitfields2a2E, {{.*}} [ undef,
+// CHECK: br i1 %[[SEL]]
+//
+// CHECK: load {{.*}}%[[A1]]
+// CHECK: and {{.*}}, 7
+// CHECK: and {{.*}}, -8
+// CHECK: or
+// CHECK: store
+// CHECK: br label
+//
+// CHECK: load {{.*}}%[[A2]]
+// CHECK: and {{.*}}, 31
+// CHECK: shl {{.*}}, 3
+// CHECK: and {{.*}}, 7
+// CHECK: or
+// CHECK: store
+// CHECK: br label
+(b ? a1.m : a2.n) = k;
+  }
+
+  // CHECK-LABEL: define {{.*}} @{{.*}}update
+  void update(bool b, int k) {
+// 1: Branch on condition and evaluate the chosen lvalue.
+//
+// CHECK: br i1
+// CHECK: %[[SEL:.*]] = phi i1
+// CHECK: %[[A1:.*]] = phi i8* {{.*}} @_ZN9bitfields2a1E, {{.*}} [ undef,
+// CHECK: %[[A2:.*]] = phi i8* {{.*}} @_ZN9bitfields2a2E, {{.*}} [ undef,
+// CHECK: br i1 %[[SEL]]
+//
+// 2: Branch on condition and load the relevant value.
+//
+// CHECK: load {{.*}}%[[A1]]
+// CHECK: shl {{.*}}, 5
+// CHECK: ashr {{.*}}, 5
+// CHECK: br
+//
+// CHECK: load {{.*}}%[[A2]]
+// CHECK: ashr {{.*}}, 3
+// CHECK: br
+//
+// 3: Add k.
+//
+// CHECK: add nsw
+// CHECK: br i1
+//
+// 4: Branch on condition and store the new value.
+//
+// CHECK: load {{.*}}%[[A1]]
+// CHECK: and {{.*}}, 7
+// CHECK: and {{.*}}, -8
+// CHECK: or
+// CHECK: store
+// CHECK: br label
+//
+// CHECK: load {{.*}}%[[A2]]
+// CHECK: and {{.*}}, 31
+// CHECK: shl {{.*}}, 3
+// CHECK: and {{.*}}, 7
+// CHECK: or
+// CHECK: store
+// CHECK: br label
+(b ? a1.m : a2.n) += k;
+  }
+
+  // CHECK-LABEL: define {{.*}} @{{.*}}atomic_load
+  int atomic_load(bool b) {
+// CHECK: br i1
+// CHECK: %[[SEL:.*]] = phi i1
+// CHECK: %[[A1:.*]] = phi i8* {{.*}} @_ZN9bitfields2a1E, {{.*}} [ undef,
+// CHECK: %[[A2:.*]] = phi i8* {{.*}} @_ZN9bitfields2a2E, {{.*}} [ undef,
+// CHECK: br i1 %[[SEL]]
+//
+// CHECK: %[[A1b:.*]] = getelementptr i8, i8* %[[A1]], i64 0
+// CHECK: %[[A1c:.*]] = bitcast i8* %[[A1b]] to i32*
+// CHECK: load atomic {{.*}}%[[A1c]]
+// CHECK: shl i32 {{.*}}, 29
+// CHECK: ashr i32 {{.*}}, 29
+// CHECK: br label
+//
+// CHECK: %[[A2b:.*]] = getelementptr i8, i8* %[[A2]], i64 0
+// CHECK: %[[A2c:.*]] = bitcast i8* %[[A2b]] to i32*
+// CHECK: load atomic {{.*}}%[[A2c]]
+// CHECK: shl i32 {{.*}}

Re: [clang] 6170072 - Improve modeling of variable template specializations with dependent

[Richard Smith via cfe-commits]

On Tue, 11 Aug 2020 at 00:29, Martin Storsjö  wrote:

> On Sun, 9 Aug 2020, Richard Smith via cfe-commits wrote:
>
> >
> > Author: Richard Smith
> > Date: 2020-08-09T23:22:26-07:00
> > New Revision: 617007240cbfb97c8ccf6d61b0c4ca0bb62d43c9
> >
> > URL:
> https://github.com/llvm/llvm-project/commit/617007240cbfb97c8ccf6d61b0c4ca0bb62d43c9
> > DIFF:
> https://github.com/llvm/llvm-project/commit/617007240cbfb97c8ccf6d61b0c4ca0bb62d43c9.diff
> >
> > LOG: Improve modeling of variable template specializations with dependent
> > arguments.
> >
> > Don't build a variable template specialization declaration until its
> > scope and template arguments are non-dependent.
> >
> > No functionality change intended, but the AST representation is now more
> > consistent with how we model other templates.
>
> This did turn out to make a functional change, breaking building the dev
> branch of Qt. A halfway reduced example below:
>

Thanks for reporting this! Reduced a bit more:

template
inline constexpr bool is_nothrow_assignable_v = true;

template class QCache {
void replace() noexcept(is_nothrow_assignable_v);
};

We used to allow this and now reject. This code is ill-formed (no
diagnostic required) by [temp.res]/8, because it has no valid
instantiations, because the second template argument of
is_nothrow_assignable_v is missing.

So, assuming the case from which this was reduced was similarly ill-formed
(which it looks like it is:
https://github.com/qt/qtbase/blob/48c8322a613f58d19ad9e0262bbac437ce2598f8/src/corelib/tools/qcache.h#L114),
I think that's a (minor) quality of implementation improvement -- we now
diagnose a bug that we used to miss. ICC also rejects this, although GCC
does not, and all three compilers reject the corresponding case using a
class template instead of a variable template.


> template 
> struct integral_constant
> {
>static constexpr const _Tp value = __v;
>typedef _Tp value_type;
>typedef integral_constant type;
>__attribute__ ((__exclude_from_explicit_instantiation__))
>constexpr operator value_type() const noexcept {return value;}
>
>__attribute__ ((__exclude_from_explicit_instantiation__))
>constexpr value_type operator ()() const noexcept {return value;}
> };
>
>
> template 
> struct is_nothrow_assignable
>  : public integral_constant
> {};
> template 
> inline constexpr bool is_nothrow_assignable_v
>  = is_nothrow_assignable<_Tp, _Arg>::value;
>
>
> template 
> class QCache
> {
>  struct Value {
>  T *t = nullptr;
>  };
>
>  struct Node {
>  Value value;
>
>  void replace(const Value &t) noexcept(is_nothrow_assignable_v)
> {
>  value = t;
>  }
>  };
>
> };
>
>
>
> // Martin
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] c6d2078 - [clangd] Improve diagnostics in dexp interface

[Kirill Bobyrev via cfe-commits]

Author: Kirill Bobyrev
Date: 2020-08-11T09:50:33+02:00
New Revision: c6d2078a35d536c8fa152fa9205924f8f10cbaac

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

LOG: [clangd] Improve diagnostics in dexp interface

When running dexp in remote mode without --project-root it shuts down
with an assertion. This is not the desired behaviour: instruct user on
how to run it properly when the configuration is incorrect.

Added: 


Modified: 
clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp 
b/clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
index ca35f620bba1..7652dbb92d02 100644
--- a/clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
+++ b/clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
@@ -30,10 +30,12 @@ llvm::cl::opt IndexLocation(
 llvm::cl::Positional);
 
 llvm::cl::opt
-ExecCommand("c", llvm::cl::desc("Command to execute and then exit"));
+ExecCommand("c", llvm::cl::desc("Command to execute and then exit."));
 
-llvm::cl::opt ProjectRoot("project-root",
-   llvm::cl::desc("Path to the project"));
+llvm::cl::opt ProjectRoot(
+"project-root",
+llvm::cl::desc(
+"Path to the project. Required when connecting using remote index."));
 
 static constexpr char Overview[] = R"(
 This is an **experimental** interactive tool to process user-provided search
@@ -373,10 +375,14 @@ int main(int argc, const char *argv[]) {
   llvm::cl::ResetCommandLineParser(); // We reuse it for REPL commands.
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
 
+  bool RemoteMode = llvm::StringRef(IndexLocation).startswith("remote:");
+  if (RemoteMode && ProjectRoot.empty()) {
+llvm::errs() << "--project-root is required in remote mode\n";
+return -1;
+  }
+
   std::unique_ptr Index;
-  reportTime(llvm::StringRef(IndexLocation).startswith("remote:")
- ? "Remote index client creation"
- : "Dex build",
+  reportTime(RemoteMode ? "Remote index client creation" : "Dex build",
  [&]() { Index = openIndex(IndexLocation); });
 
   if (!Index) {



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

[PATCH] D85711: [clangd] Enforce trailing slash for remote index's project root

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added a reviewer: hokein.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous.
Herald added a project: clang.
kbobyrev requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85711

Files:
  clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
  clang-tools-extra/clangd/index/remote/Client.cpp
  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
@@ -702,6 +702,10 @@
 return 1;
   }
   if (!RemoteIndexAddress.empty()) {
+if (!ProjectRoot.empty() &&
+!llvm::StringRef(ProjectRoot)
+ .endswith(llvm::sys::path::get_separator()))
+  ProjectRoot += llvm::sys::path::get_separator();
 if (IndexFile.empty()) {
   log("Connecting to remote index at {0}", RemoteIndexAddress);
   StaticIdx = remote::getClient(RemoteIndexAddress, ProjectRoot);
Index: clang-tools-extra/clangd/index/remote/Client.cpp
===
--- clang-tools-extra/clangd/index/remote/Client.cpp
+++ clang-tools-extra/clangd/index/remote/Client.cpp
@@ -76,7 +76,9 @@
   : Stub(remote::SymbolIndex::NewStub(Channel)),
 ProtobufMarshaller(new Marshaller(/*RemoteIndexRoot=*/"",
   /*LocalIndexRoot=*/ProjectRoot)),
-DeadlineWaitingTime(DeadlineTime) {}
+DeadlineWaitingTime(DeadlineTime) {
+assert(!ProjectRoot.empty());
+  }
 
   void lookup(const clangd::LookupRequest &Request,
   llvm::function_ref Callback) const 
{
Index: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
===
--- clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
+++ clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
@@ -19,6 +19,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/LineEditor/LineEditor.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/Signals.h"
 
 namespace clang {
@@ -381,6 +382,10 @@
 return -1;
   }
 
+  if (!ProjectRoot.empty() &&
+  !llvm::StringRef(ProjectRoot).endswith(llvm::sys::path::get_separator()))
+ProjectRoot += llvm::sys::path::get_separator();
+
   std::unique_ptr Index;
   reportTime(RemoteMode ? "Remote index client creation" : "Dex build",
  [&]() { Index = openIndex(IndexLocation); });


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -702,6 +702,10 @@
 return 1;
   }
   if (!RemoteIndexAddress.empty()) {
+if (!ProjectRoot.empty() &&
+!llvm::StringRef(ProjectRoot)
+ .endswith(llvm::sys::path::get_separator()))
+  ProjectRoot += llvm::sys::path::get_separator();
 if (IndexFile.empty()) {
   log("Connecting to remote index at {0}", RemoteIndexAddress);
   StaticIdx = remote::getClient(RemoteIndexAddress, ProjectRoot);
Index: clang-tools-extra/clangd/index/remote/Client.cpp
===
--- clang-tools-extra/clangd/index/remote/Client.cpp
+++ clang-tools-extra/clangd/index/remote/Client.cpp
@@ -76,7 +76,9 @@
   : Stub(remote::SymbolIndex::NewStub(Channel)),
 ProtobufMarshaller(new Marshaller(/*RemoteIndexRoot=*/"",
   /*LocalIndexRoot=*/ProjectRoot)),
-DeadlineWaitingTime(DeadlineTime) {}
+DeadlineWaitingTime(DeadlineTime) {
+assert(!ProjectRoot.empty());
+  }
 
   void lookup(const clangd::LookupRequest &Request,
   llvm::function_ref Callback) const {
Index: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
===
--- clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
+++ clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
@@ -19,6 +19,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/LineEditor/LineEditor.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/Signals.h"
 
 namespace clang {
@@ -381,6 +382,10 @@
 return -1;
   }
 
+  if (!ProjectRoot.empty() &&
+  !llvm::StringRef(ProjectRoot).endswith(llvm::sys::path::get_separator()))
+ProjectRoot += llvm::sys::path::get_separator();
+
   std::unique_ptr Index;
   reportTime(RemoteMode ? "Remote index client creation" : "Dex build",
  [&]() { Index = openIndex(IndexLocation); });
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83501: [clangd][ObjC] Improve xrefs for protocols and classes

[Sam McCall via Phabricator via cfe-commits]

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

Ugh, I forgot to hit submit and went on vacation :-\ Really sorry again.

As much as we can simplify/unify the tests that helps, but let's not block on 
this anymore, up to you.




Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:614
+
+TEST_F(SymbolCollectorTest, ObjCClassExtensions) {
+  Annotations Header(R"(

dgoldman wrote:
> sammccall wrote:
> > dgoldman wrote:
> > > Here's the ClassExtension that I was talking about.
> > > 
> > > Ideally we can map each
> > > 
> > > `Cat ()` --> `@implementation Cat` like I did in XRefs
> > > 
> > > But as you said the `Cat ()` could be in a different file and I think it 
> > > has a different USR.
> > > 
> > > See also 
> > > https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/ProgrammingWithObjectiveC/CustomizingExistingClasses/CustomizingExistingClasses.html#//apple_ref/doc/uid/TP40011210-CH6-SW3
> > > Ideally we can map each
> > > Cat () --> @implementation Cat like I did in XRefs
> > 
> > I'm not sure there's anything that would ideally be done differently here.
> > The logic in xrefs is a special "go-to-definition" action - there's some 
> > ambiguity about what's being *targeted* by the user. But here there's no 
> > targeting going on, and there's no ambiguity about what's being *declared*.
> > 
> > The thing to test would be that we're emitting *refs* from `@interface 
> > [[Cat]] ()` to catdecl.
> > 
> Hmm, it looks like at the moment it either shares the same QName or doesn't 
> have one. This might be good to look into a follow up patch?
Sorry, I don't know what you mean about QName. USR?

Yeah, fine to defer out of this patch. This is just "does find references on an 
@interface find extensions of that interface".



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:803
+  };
+  for (const char *Test : Tests) {
+Annotations T(Test);

dgoldman wrote:
> sammccall wrote:
> > this seems to be copy/pasted from the test above.
> > Is there a reason this can't be part of the test above?
> I could merge them but I figured it would be better to separate tests with 
> multi def/decls from those with just one. WDYT?
I think it would be better to split each decl/def pair into its own testcase, 
and combine with the above (even if it means a bit of duplication between test 
cases)



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:838
+
+TEST(LocateSymbol, MultipleDeclsWithSameDefinition) {
+  // Ranges in tests:

dgoldman wrote:
> sammccall wrote:
> > and again here
> > 
> > Desire to split these tables up into named tests is something we want to 
> > address somehow, but we don't have a good answer right now and it's 
> > important for maintenance that the logic/annotation conventions don't 
> > diverge across different tests that could be the same.
> This one is split because you can't annotate one symbol with multiple 
> annotations. I can instead make this a regular non generic test like the 
> following, WDYT?
> 
>   @interface $interfacedecl[[Cat]]
>   @end
>   @interface $classextensiondecl[[Ca^t]] ()
>   - (void)meow;
>   @end
>   @implementation $implementationdecl[[Cat]]
>   - (void)meow {}
>   @end
> This one is split because you can't annotate one symbol with multiple 
> annotations

Not sure I quite see what you mean here, but `$foo[[$bar[[symbol should 
work`.

Anyway, fine if you want to leave this one separate. I'd avoid the tests 
array+loop if there's just one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83501

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

[clang] 497d060 - [Analyzer] Improve invalid dereference bug reporting in DereferenceChecker.

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

Author: Balázs Kéri
Date: 2020-08-11T10:10:13+02:00
New Revision: 497d060d0a741e13dd5e6218ba7301a7ec96f332

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

LOG: [Analyzer] Improve invalid dereference bug reporting in DereferenceChecker.

Report undefined pointer dereference in similar way as null pointer dereference.

Reviewed By: NoQ

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

Added: 
clang/test/Analysis/invalid-deref.c

Modified: 
clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
clang/test/Analysis/misc-ps-region-store.m

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
index 9a87729de8fd..adfc2f8cb8fe 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
@@ -30,11 +30,14 @@ class DereferenceChecker
 : public Checker< check::Location,
   check::Bind,
   EventDispatcher > {
+  enum DerefKind { NullPointer, UndefinedPointerValue };
+
   BugType BT_Null{this, "Dereference of null pointer", categories::LogicError};
   BugType BT_Undef{this, "Dereference of undefined pointer value",
categories::LogicError};
 
-  void reportBug(ProgramStateRef State, const Stmt *S, CheckerContext &C) 
const;
+  void reportBug(DerefKind K, ProgramStateRef State, const Stmt *S,
+ CheckerContext &C) const;
 
 public:
   void checkLocation(SVal location, bool isLoad, const Stmt* S,
@@ -117,8 +120,24 @@ static bool isDeclRefExprToReference(const Expr *E) {
   return false;
 }
 
-void DereferenceChecker::reportBug(ProgramStateRef State, const Stmt *S,
-   CheckerContext &C) const {
+void DereferenceChecker::reportBug(DerefKind K, ProgramStateRef State,
+   const Stmt *S, CheckerContext &C) const {
+  const BugType *BT = nullptr;
+  llvm::StringRef DerefStr1;
+  llvm::StringRef DerefStr2;
+  switch (K) {
+  case DerefKind::NullPointer:
+BT = &BT_Null;
+DerefStr1 = " results in a null pointer dereference";
+DerefStr2 = " results in a dereference of a null pointer";
+break;
+  case DerefKind::UndefinedPointerValue:
+BT = &BT_Undef;
+DerefStr1 = " results in an undefined pointer dereference";
+DerefStr2 = " results in a dereference of an undefined pointer value";
+break;
+  };
+
   // Generate an error node.
   ExplodedNode *N = C.generateErrorNode(State);
   if (!N)
@@ -135,7 +154,7 @@ void DereferenceChecker::reportBug(ProgramStateRef State, 
const Stmt *S,
 const ArraySubscriptExpr *AE = cast(S);
 AddDerefSource(os, Ranges, AE->getBase()->IgnoreParenCasts(),
State.get(), N->getLocationContext());
-os << " results in a null pointer dereference";
+os << DerefStr1;
 break;
   }
   case Stmt::OMPArraySectionExprClass: {
@@ -143,11 +162,11 @@ void DereferenceChecker::reportBug(ProgramStateRef State, 
const Stmt *S,
 const OMPArraySectionExpr *AE = cast(S);
 AddDerefSource(os, Ranges, AE->getBase()->IgnoreParenCasts(),
State.get(), N->getLocationContext());
-os << " results in a null pointer dereference";
+os << DerefStr1;
 break;
   }
   case Stmt::UnaryOperatorClass: {
-os << "Dereference of null pointer";
+os << BT->getDescription();
 const UnaryOperator *U = cast(S);
 AddDerefSource(os, Ranges, U->getSubExpr()->IgnoreParens(),
State.get(), N->getLocationContext(), true);
@@ -156,8 +175,7 @@ void DereferenceChecker::reportBug(ProgramStateRef State, 
const Stmt *S,
   case Stmt::MemberExprClass: {
 const MemberExpr *M = cast(S);
 if (M->isArrow() || isDeclRefExprToReference(M->getBase())) {
-  os << "Access to field '" << M->getMemberNameInfo()
- << "' results in a dereference of a null pointer";
+  os << "Access to field '" << M->getMemberNameInfo() << "'" << DerefStr2;
   AddDerefSource(os, Ranges, M->getBase()->IgnoreParenCasts(),
  State.get(), N->getLocationContext(), true);
 }
@@ -165,8 +183,7 @@ void DereferenceChecker::reportBug(ProgramStateRef State, 
const Stmt *S,
   }
   case Stmt::ObjCIvarRefExprClass: {
 const ObjCIvarRefExpr *IV = cast(S);
-os << "Access to instance variable '" << *IV->getDecl()
-   << "' results in a dereference of a null pointer";
+os << "Access to instance variable '" << *IV->getDecl() << "'" << 
DerefStr2;
 AddDerefSource(os, Ranges, IV->getBase()->IgnoreParenCasts(),
State.get(), N->getLocationContext(), true);
 break;
@@ -176,7 +193,7 @@ void DereferenceChecker::reportBug(ProgramStateRef State,

[PATCH] D84520: [Analyzer] Improve invalid dereference bug reporting in DereferenceChecker.

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG497d060d0a74: [Analyzer] Improve invalid dereference bug 
reporting in DereferenceChecker. (authored by balazske).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84520

Files:
  clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
  clang/test/Analysis/invalid-deref.c
  clang/test/Analysis/misc-ps-region-store.m

Index: clang/test/Analysis/misc-ps-region-store.m
===
--- clang/test/Analysis/misc-ps-region-store.m
+++ clang/test/Analysis/misc-ps-region-store.m
@@ -1157,7 +1157,7 @@
 struct list_pr8141 *
 pr8141 (void) {
   struct list_pr8141 *items;
-  for (;; items = ({ do { } while (0); items->tail; })) // expected-warning{{Dereference of undefined pointer value}}
+  for (;; items = ({ do { } while (0); items->tail; })) // expected-warning{{dereference of an undefined pointer value}}
 {
 }
 }
Index: clang/test/Analysis/invalid-deref.c
===
--- /dev/null
+++ clang/test/Analysis/invalid-deref.c
@@ -0,0 +1,32 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+
+typedef unsigned uintptr_t;
+
+void f1() {
+  int *p;
+  *p = 0; // expected-warning{{Dereference of undefined pointer value}}
+}
+
+struct foo_struct {
+  int x;
+};
+
+int f2() {
+  struct foo_struct *p;
+
+  return p->x++; // expected-warning{{Access to field 'x' results in a dereference of an undefined pointer value (loaded from variable 'p')}}
+}
+
+int f3() {
+  char *x;
+  int i = 2;
+
+  return x[i + 1]; // expected-warning{{Array access (from variable 'x') results in an undefined pointer dereference}}
+}
+
+int f3_b() {
+  char *x;
+  int i = 2;
+
+  return x[i + 1]++; // expected-warning{{Array access (from variable 'x') results in an undefined pointer dereference}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
@@ -30,11 +30,14 @@
 : public Checker< check::Location,
   check::Bind,
   EventDispatcher > {
+  enum DerefKind { NullPointer, UndefinedPointerValue };
+
   BugType BT_Null{this, "Dereference of null pointer", categories::LogicError};
   BugType BT_Undef{this, "Dereference of undefined pointer value",
categories::LogicError};
 
-  void reportBug(ProgramStateRef State, const Stmt *S, CheckerContext &C) const;
+  void reportBug(DerefKind K, ProgramStateRef State, const Stmt *S,
+ CheckerContext &C) const;
 
 public:
   void checkLocation(SVal location, bool isLoad, const Stmt* S,
@@ -117,8 +120,24 @@
   return false;
 }
 
-void DereferenceChecker::reportBug(ProgramStateRef State, const Stmt *S,
-   CheckerContext &C) const {
+void DereferenceChecker::reportBug(DerefKind K, ProgramStateRef State,
+   const Stmt *S, CheckerContext &C) const {
+  const BugType *BT = nullptr;
+  llvm::StringRef DerefStr1;
+  llvm::StringRef DerefStr2;
+  switch (K) {
+  case DerefKind::NullPointer:
+BT = &BT_Null;
+DerefStr1 = " results in a null pointer dereference";
+DerefStr2 = " results in a dereference of a null pointer";
+break;
+  case DerefKind::UndefinedPointerValue:
+BT = &BT_Undef;
+DerefStr1 = " results in an undefined pointer dereference";
+DerefStr2 = " results in a dereference of an undefined pointer value";
+break;
+  };
+
   // Generate an error node.
   ExplodedNode *N = C.generateErrorNode(State);
   if (!N)
@@ -135,7 +154,7 @@
 const ArraySubscriptExpr *AE = cast(S);
 AddDerefSource(os, Ranges, AE->getBase()->IgnoreParenCasts(),
State.get(), N->getLocationContext());
-os << " results in a null pointer dereference";
+os << DerefStr1;
 break;
   }
   case Stmt::OMPArraySectionExprClass: {
@@ -143,11 +162,11 @@
 const OMPArraySectionExpr *AE = cast(S);
 AddDerefSource(os, Ranges, AE->getBase()->IgnoreParenCasts(),
State.get(), N->getLocationContext());
-os << " results in a null pointer dereference";
+os << DerefStr1;
 break;
   }
   case Stmt::UnaryOperatorClass: {
-os << "Dereference of null pointer";
+os << BT->getDescription();
 const UnaryOperator *U = cast(S);
 AddDerefSource(os, Ranges, U->getSubExpr()->IgnoreParens(),
State.get(), N->getLocationContext(), true);
@@ -156,8 +175,7 @@
   case Stmt::MemberExprClass: {
 const MemberExpr *M = cast(S);
 if (M->isArrow() || isDeclRefExprToReference(M->getBase())) {
-  os << "Access to field '" << M->getMemberNameInfo()
- << "' results in a dereference 

[PATCH] D81272: [clang-tidy] New check `misc-redundant-condition`

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

baloghadamsoftware added a comment.

6 findings in the //LLVM Project//. All of them confirmed as trues positives, 5 
of them already fixed. Fix pending for the last one.

D82555 , D8556 
, D82557 , 
D82558 , D82559 
, D82563 

@aaron.ballman, @gribozavr2 or someone please review this patch.


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

https://reviews.llvm.org/D81272

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

[PATCH] D85099: [UpdateTestChecks] Match unnamed values like "@[0-9]+" and "![0-9]+"

[Alexander Richardson via Phabricator via cfe-commits]

arichardson accepted this revision.
arichardson added a comment.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85099

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

[PATCH] D85713: [SyntaxTree]: Use Annotations in tests to reduce noise

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
eduucaldas requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85713

Files:
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -168,6 +168,35 @@
 return ::testing::AssertionSuccess();
   }
 
+  ::testing::AssertionResult
+  treeDumpEqualOnAnnotations(StringRef CodeWithAnnotations,
+ ArrayRef TreeDumps) {
+SCOPED_TRACE(llvm::join(GetParam().getCommandLineArgs(), " "));
+
+auto AnnotatedCode = llvm::Annotations(CodeWithAnnotations);
+auto *Root = buildTree(AnnotatedCode.code(), GetParam());
+
+if (Diags->getClient()->getNumErrors() != 0) {
+  return ::testing::AssertionFailure()
+ << "Source file has syntax errors, they were printed to the test "
+"log";
+}
+
+auto AnnotatedRanges = AnnotatedCode.ranges();
+assert(AnnotatedRanges.size() == TreeDumps.size());
+for (auto i = 0u; i < AnnotatedRanges.size(); i++) {
+  auto *AnnotatedNode = nodeByRange(AnnotatedRanges[i], Root);
+  assert(AnnotatedNode);
+  auto AnnotatedNodeDump =
+  std::string(StringRef(AnnotatedNode->dump(*Arena)).trim());
+  // EXPECT_EQ shows the diff between the two strings if they are different.
+  EXPECT_EQ(TreeDumps[i].trim().str(), AnnotatedNodeDump);
+  if (AnnotatedNodeDump != TreeDumps[i].trim().str())
+return ::testing::AssertionFailure();
+}
+return ::testing::AssertionSuccess();
+  }
+
   // Adds a file to the test VFS.
   void addFile(StringRef Path, StringRef Contents) {
 if (!FS->addFile(Path, time_t(),
@@ -214,34 +243,23 @@
 };
 
 TEST_P(SyntaxTreeTest, Simple) {
-  EXPECT_TRUE(treeDumpEqual(
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
   R"cpp(
 int main() {}
-void foo() {}
+[[void foo() {}]]
 )cpp",
-  R"txt(
-*: TranslationUnit
-|-SimpleDeclaration
-| |-int
-| |-SimpleDeclarator
-| | |-main
-| | `-ParametersAndQualifiers
-| |   |-(
-| |   `-)
-| `-CompoundStatement
-|   |-{
-|   `-}
-`-SimpleDeclaration
-  |-void
-  |-SimpleDeclarator
-  | |-foo
-  | `-ParametersAndQualifiers
-  |   |-(
-  |   `-)
-  `-CompoundStatement
-|-{
-`-}
-)txt"));
+  {R"txt(
+SimpleDeclaration
+|-void
+|-SimpleDeclarator
+| |-foo
+| `-ParametersAndQualifiers
+|   |-(
+|   `-)
+`-CompoundStatement
+  |-{
+  `-}
+)txt"}));
 }
 
 TEST_P(SyntaxTreeTest, SimpleVariable) {
@@ -871,7 +889,7 @@
   if (!GetParam().isCXX()) {
 return;
   }
-  EXPECT_TRUE(treeDumpEqual(
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
   R"cpp(
 namespace n {
   struct S {
@@ -889,194 +907,71 @@
   };
 };
 void test() {
-  :: // global-namespace-specifier
-  n::// namespace-specifier
-  S::// type-name-specifier
-  template ST:: // type-template-instantiation-specifier
-  f();
-
-  n::// namespace-specifier
-  S::// type-name-specifier
-  ST::  // type-template-instantiation-specifier
-  f();
-
-  ST:: // type-name-specifier
-  S::   // type-name-specifier
-  f();
-
-  ST:: // type-name-specifier
-  S::   // type-name-specifier
-  template f();
+[[::n::S::template ST::]]f();
+
+[[n::S::ST::]]f();
+
+[[ST::S::]]f();
+
+[[ST::S::]]template f();
 }
 )cpp",
-  R"txt(
-*: TranslationUnit
-|-NamespaceDefinition
-| |-namespace
-| |-n
-| |-{
-| |-SimpleDeclaration
-| | |-struct
-| | |-S
-| | |-{
-| | |-TemplateDeclaration
-| | | |-template
-| | | |-<
-| | | |-UnknownDeclaration
-| | | | |-typename
-| | | | `-T
-| | | |->
-| | | `-SimpleDeclaration
-| | |   |-struct
-| | |   |-ST
-| | |   |-{
-| | |   |-SimpleDeclaration
-| | |   | |-static
-| | |   | |-void
-| | |   | |-SimpleDeclarator
-| | |   | | |-f
-| | |   | | `-ParametersAndQualifiers
-| | |   | |   |-(
-| | |   | |   `-)
-| | |   | `-;
-| | |   |-}
-| | |   `-;
-| | |-}
-| | `-;
-| `-}
-|-TemplateDeclaration
+  {R"txt(
+NestedNameSpecifier
+|-::
+|-IdentifierNameSpecifier
+| `-n
+|-::
+|-IdentifierNameSpecifier
+| `-S
+|-::
+|-SimpleTemplateNameSpecifier
 | |-template
+| |-ST
 | |-<
-| |-UnknownDeclaration
-| | |-typename
-| | `-T
-| |->
-| `-SimpleDeclaration
-|   |-struct
-|   |-ST
-|   |-{
-|   |-SimpleDeclaration
-|   | |-struct
-|   | |-S
-|   | |-{
-|   | |-TemplateDeclaration
-|   | | |-template
-|   | | |-<
-|   | | |-UnknownDeclaration
-|   | | | |-typename
-|   | | | `-U
-|   | | |->
-|   | | `-SimpleDeclaration
-|   | |   |-static
-|   | |   |-U
-|   | |   |-SimpleDeclarator
-|   | |   | |-f
-|   | |   | `-ParametersAndQualifiers
-|   | |   |   |-(
-|   | |   |   `-)
-|   | |   `-;
-|   | |-}
-|   | `-;
-|   |-}
-|   `-;
-`-SimpleDeclaration
-  |-void
-

[PATCH] D85711: [clangd] Enforce trailing slash for remote index's project root

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp:387
+  !llvm::StringRef(ProjectRoot).endswith(llvm::sys::path::get_separator()))
+ProjectRoot += llvm::sys::path::get_separator();
+

Instead of handling this in all callers, I think it might be better to move it 
to `Marshaller` and drop the ends-with-slash assertion there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85711

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

[PATCH] D85431: [analyzer] Implement a new checker ThreadPrimitivesChecker

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@NoQ

> Umm, why don't you extend `PthreadLockChecker` instead?

First of all I wanted to try to go through all the steps, since it's my first 
checker. The second is that I considered `PthreadLockChecker` designed for 
C-functions. Another reason is that it better helps me to test the code when it 
is separated from all other.
When I feel more confident and it makes sence I'll try to merge this patch into 
PthreadLockChecker. For now it's more just an idea to discuss, to collect 
expertise.


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

https://reviews.llvm.org/D85431

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

[PATCH] D85256: Add -Wtautological-value-range-compare warning.

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

I think this generates a false positive with `test.cc`

  enum E { E1 = 1, E2 = 2 };
  bool f(E e) { return ((e & E1) ? 1 : 0) + ((e & E2) ? 1 : 0) > 1; }

and `clang++ -fsyntax-only -Wtautological-value-range-compare test.cc`

  test.cc:2:62: warning: result of comparison of 1-bit unsigned value > 1 is 
always false [-Wtautological-value-range-compare]
  bool f(E e) { return ((e & E1) ? 1 : 0) + ((e & E2) ? 1 : 0) > 1; }
   ~~~ ^ ~


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85256

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

[PATCH] D85711: [clangd] Enforce trailing slash for remote index's project root

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 284609.
kbobyrev marked an inline comment as done.
kbobyrev added a comment.

Handle slash appending in Marshaller.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85711

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


Index: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
===
--- clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -59,14 +59,16 @@
 assert(llvm::sys::path::is_absolute(RemoteIndexRoot));
 assert(RemoteIndexRoot ==
llvm::sys::path::convert_to_slash(RemoteIndexRoot));
-assert(RemoteIndexRoot.endswith(llvm::sys::path::get_separator()));
 this->RemoteIndexRoot = RemoteIndexRoot.str();
+if (!RemoteIndexRoot.endswith(llvm::sys::path::get_separator()))
+  *this->RemoteIndexRoot += llvm::sys::path::get_separator().str();
   }
   if (!LocalIndexRoot.empty()) {
 assert(llvm::sys::path::is_absolute(LocalIndexRoot));
 assert(LocalIndexRoot == 
llvm::sys::path::convert_to_slash(LocalIndexRoot));
-assert(LocalIndexRoot.endswith(llvm::sys::path::get_separator()));
 this->LocalIndexRoot = LocalIndexRoot.str();
+if (!LocalIndexRoot.endswith(llvm::sys::path::get_separator()))
+  *this->LocalIndexRoot += llvm::sys::path::get_separator().str();
   }
   assert(!RemoteIndexRoot.empty() || !LocalIndexRoot.empty());
 }
Index: clang-tools-extra/clangd/index/remote/Client.cpp
===
--- clang-tools-extra/clangd/index/remote/Client.cpp
+++ clang-tools-extra/clangd/index/remote/Client.cpp
@@ -76,7 +76,9 @@
   : Stub(remote::SymbolIndex::NewStub(Channel)),
 ProtobufMarshaller(new Marshaller(/*RemoteIndexRoot=*/"",
   /*LocalIndexRoot=*/ProjectRoot)),
-DeadlineWaitingTime(DeadlineTime) {}
+DeadlineWaitingTime(DeadlineTime) {
+assert(!ProjectRoot.empty());
+  }
 
   void lookup(const clangd::LookupRequest &Request,
   llvm::function_ref Callback) const 
{


Index: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
===
--- clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -59,14 +59,16 @@
 assert(llvm::sys::path::is_absolute(RemoteIndexRoot));
 assert(RemoteIndexRoot ==
llvm::sys::path::convert_to_slash(RemoteIndexRoot));
-assert(RemoteIndexRoot.endswith(llvm::sys::path::get_separator()));
 this->RemoteIndexRoot = RemoteIndexRoot.str();
+if (!RemoteIndexRoot.endswith(llvm::sys::path::get_separator()))
+  *this->RemoteIndexRoot += llvm::sys::path::get_separator().str();
   }
   if (!LocalIndexRoot.empty()) {
 assert(llvm::sys::path::is_absolute(LocalIndexRoot));
 assert(LocalIndexRoot == llvm::sys::path::convert_to_slash(LocalIndexRoot));
-assert(LocalIndexRoot.endswith(llvm::sys::path::get_separator()));
 this->LocalIndexRoot = LocalIndexRoot.str();
+if (!LocalIndexRoot.endswith(llvm::sys::path::get_separator()))
+  *this->LocalIndexRoot += llvm::sys::path::get_separator().str();
   }
   assert(!RemoteIndexRoot.empty() || !LocalIndexRoot.empty());
 }
Index: clang-tools-extra/clangd/index/remote/Client.cpp
===
--- clang-tools-extra/clangd/index/remote/Client.cpp
+++ clang-tools-extra/clangd/index/remote/Client.cpp
@@ -76,7 +76,9 @@
   : Stub(remote::SymbolIndex::NewStub(Channel)),
 ProtobufMarshaller(new Marshaller(/*RemoteIndexRoot=*/"",
   /*LocalIndexRoot=*/ProjectRoot)),
-DeadlineWaitingTime(DeadlineTime) {}
+DeadlineWaitingTime(DeadlineTime) {
+assert(!ProjectRoot.empty());
+  }
 
   void lookup(const clangd::LookupRequest &Request,
   llvm::function_ref Callback) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85713: [SyntaxTree]: Use Annotations in tests to reduce noise

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas updated this revision to Diff 284610.
eduucaldas added a comment.

Use unsigned long in the loop index


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85713

Files:
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -168,6 +168,35 @@
 return ::testing::AssertionSuccess();
   }
 
+  ::testing::AssertionResult
+  treeDumpEqualOnAnnotations(StringRef CodeWithAnnotations,
+ ArrayRef TreeDumps) {
+SCOPED_TRACE(llvm::join(GetParam().getCommandLineArgs(), " "));
+
+auto AnnotatedCode = llvm::Annotations(CodeWithAnnotations);
+auto *Root = buildTree(AnnotatedCode.code(), GetParam());
+
+if (Diags->getClient()->getNumErrors() != 0) {
+  return ::testing::AssertionFailure()
+ << "Source file has syntax errors, they were printed to the test "
+"log";
+}
+
+auto AnnotatedRanges = AnnotatedCode.ranges();
+assert(AnnotatedRanges.size() == TreeDumps.size());
+for (auto i = 0ul; i < AnnotatedRanges.size(); i++) {
+  auto *AnnotatedNode = nodeByRange(AnnotatedRanges[i], Root);
+  assert(AnnotatedNode);
+  auto AnnotatedNodeDump =
+  std::string(StringRef(AnnotatedNode->dump(*Arena)).trim());
+  // EXPECT_EQ shows the diff between the two strings if they are different.
+  EXPECT_EQ(TreeDumps[i].trim().str(), AnnotatedNodeDump);
+  if (AnnotatedNodeDump != TreeDumps[i].trim().str())
+return ::testing::AssertionFailure();
+}
+return ::testing::AssertionSuccess();
+  }
+
   // Adds a file to the test VFS.
   void addFile(StringRef Path, StringRef Contents) {
 if (!FS->addFile(Path, time_t(),
@@ -214,34 +243,23 @@
 };
 
 TEST_P(SyntaxTreeTest, Simple) {
-  EXPECT_TRUE(treeDumpEqual(
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
   R"cpp(
 int main() {}
-void foo() {}
+[[void foo() {}]]
 )cpp",
-  R"txt(
-*: TranslationUnit
-|-SimpleDeclaration
-| |-int
-| |-SimpleDeclarator
-| | |-main
-| | `-ParametersAndQualifiers
-| |   |-(
-| |   `-)
-| `-CompoundStatement
-|   |-{
-|   `-}
-`-SimpleDeclaration
-  |-void
-  |-SimpleDeclarator
-  | |-foo
-  | `-ParametersAndQualifiers
-  |   |-(
-  |   `-)
-  `-CompoundStatement
-|-{
-`-}
-)txt"));
+  {R"txt(
+SimpleDeclaration
+|-void
+|-SimpleDeclarator
+| |-foo
+| `-ParametersAndQualifiers
+|   |-(
+|   `-)
+`-CompoundStatement
+  |-{
+  `-}
+)txt"}));
 }
 
 TEST_P(SyntaxTreeTest, SimpleVariable) {
@@ -871,7 +889,7 @@
   if (!GetParam().isCXX()) {
 return;
   }
-  EXPECT_TRUE(treeDumpEqual(
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
   R"cpp(
 namespace n {
   struct S {
@@ -889,194 +907,71 @@
   };
 };
 void test() {
-  :: // global-namespace-specifier
-  n::// namespace-specifier
-  S::// type-name-specifier
-  template ST:: // type-template-instantiation-specifier
-  f();
-
-  n::// namespace-specifier
-  S::// type-name-specifier
-  ST::  // type-template-instantiation-specifier
-  f();
-
-  ST:: // type-name-specifier
-  S::   // type-name-specifier
-  f();
-
-  ST:: // type-name-specifier
-  S::   // type-name-specifier
-  template f();
+[[::n::S::template ST::]]f();
+
+[[n::S::ST::]]f();
+
+[[ST::S::]]f();
+
+[[ST::S::]]template f();
 }
 )cpp",
-  R"txt(
-*: TranslationUnit
-|-NamespaceDefinition
-| |-namespace
-| |-n
-| |-{
-| |-SimpleDeclaration
-| | |-struct
-| | |-S
-| | |-{
-| | |-TemplateDeclaration
-| | | |-template
-| | | |-<
-| | | |-UnknownDeclaration
-| | | | |-typename
-| | | | `-T
-| | | |->
-| | | `-SimpleDeclaration
-| | |   |-struct
-| | |   |-ST
-| | |   |-{
-| | |   |-SimpleDeclaration
-| | |   | |-static
-| | |   | |-void
-| | |   | |-SimpleDeclarator
-| | |   | | |-f
-| | |   | | `-ParametersAndQualifiers
-| | |   | |   |-(
-| | |   | |   `-)
-| | |   | `-;
-| | |   |-}
-| | |   `-;
-| | |-}
-| | `-;
-| `-}
-|-TemplateDeclaration
+  {R"txt(
+NestedNameSpecifier
+|-::
+|-IdentifierNameSpecifier
+| `-n
+|-::
+|-IdentifierNameSpecifier
+| `-S
+|-::
+|-SimpleTemplateNameSpecifier
 | |-template
+| |-ST
 | |-<
-| |-UnknownDeclaration
-| | |-typename
-| | `-T
-| |->
-| `-SimpleDeclaration
-|   |-struct
-|   |-ST
-|   |-{
-|   |-SimpleDeclaration
-|   | |-struct
-|   | |-S
-|   | |-{
-|   | |-TemplateDeclaration
-|   | | |-template
-|   | | |-<
-|   | | |-UnknownDeclaration
-|   | | | |-typename
-|   | | | `-U
-|   | | |->
-|   | | `-SimpleDeclaration
-|   | |   |-static
-|   | |   |-U
-|   | |   |-SimpleDeclarator
-|   | |   | |-f
-|   | |   | `-ParametersAndQualifiers
-|   | |   |   |-(
-|   | |   |   `-)
-|   | |   `-;
-|   | |-}
-|   | `-;
-|   |-}
-|   `-;
-

[PATCH] D85711: [clangd] Enforce trailing slash for remote index's project root

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Remove redundant `.str()` calls.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85711

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


Index: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
===
--- clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -59,14 +59,16 @@
 assert(llvm::sys::path::is_absolute(RemoteIndexRoot));
 assert(RemoteIndexRoot ==
llvm::sys::path::convert_to_slash(RemoteIndexRoot));
-assert(RemoteIndexRoot.endswith(llvm::sys::path::get_separator()));
 this->RemoteIndexRoot = RemoteIndexRoot.str();
+if (!RemoteIndexRoot.endswith(llvm::sys::path::get_separator()))
+  *this->RemoteIndexRoot += llvm::sys::path::get_separator();
   }
   if (!LocalIndexRoot.empty()) {
 assert(llvm::sys::path::is_absolute(LocalIndexRoot));
 assert(LocalIndexRoot == 
llvm::sys::path::convert_to_slash(LocalIndexRoot));
-assert(LocalIndexRoot.endswith(llvm::sys::path::get_separator()));
 this->LocalIndexRoot = LocalIndexRoot.str();
+if (!LocalIndexRoot.endswith(llvm::sys::path::get_separator()))
+  *this->LocalIndexRoot += llvm::sys::path::get_separator();
   }
   assert(!RemoteIndexRoot.empty() || !LocalIndexRoot.empty());
 }
Index: clang-tools-extra/clangd/index/remote/Client.cpp
===
--- clang-tools-extra/clangd/index/remote/Client.cpp
+++ clang-tools-extra/clangd/index/remote/Client.cpp
@@ -76,7 +76,9 @@
   : Stub(remote::SymbolIndex::NewStub(Channel)),
 ProtobufMarshaller(new Marshaller(/*RemoteIndexRoot=*/"",
   /*LocalIndexRoot=*/ProjectRoot)),
-DeadlineWaitingTime(DeadlineTime) {}
+DeadlineWaitingTime(DeadlineTime) {
+assert(!ProjectRoot.empty());
+  }
 
   void lookup(const clangd::LookupRequest &Request,
   llvm::function_ref Callback) const 
{


Index: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
===
--- clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -59,14 +59,16 @@
 assert(llvm::sys::path::is_absolute(RemoteIndexRoot));
 assert(RemoteIndexRoot ==
llvm::sys::path::convert_to_slash(RemoteIndexRoot));
-assert(RemoteIndexRoot.endswith(llvm::sys::path::get_separator()));
 this->RemoteIndexRoot = RemoteIndexRoot.str();
+if (!RemoteIndexRoot.endswith(llvm::sys::path::get_separator()))
+  *this->RemoteIndexRoot += llvm::sys::path::get_separator();
   }
   if (!LocalIndexRoot.empty()) {
 assert(llvm::sys::path::is_absolute(LocalIndexRoot));
 assert(LocalIndexRoot == llvm::sys::path::convert_to_slash(LocalIndexRoot));
-assert(LocalIndexRoot.endswith(llvm::sys::path::get_separator()));
 this->LocalIndexRoot = LocalIndexRoot.str();
+if (!LocalIndexRoot.endswith(llvm::sys::path::get_separator()))
+  *this->LocalIndexRoot += llvm::sys::path::get_separator();
   }
   assert(!RemoteIndexRoot.empty() || !LocalIndexRoot.empty());
 }
Index: clang-tools-extra/clangd/index/remote/Client.cpp
===
--- clang-tools-extra/clangd/index/remote/Client.cpp
+++ clang-tools-extra/clangd/index/remote/Client.cpp
@@ -76,7 +76,9 @@
   : Stub(remote::SymbolIndex::NewStub(Channel)),
 ProtobufMarshaller(new Marshaller(/*RemoteIndexRoot=*/"",
   /*LocalIndexRoot=*/ProjectRoot)),
-DeadlineWaitingTime(DeadlineTime) {}
+DeadlineWaitingTime(DeadlineTime) {
+assert(!ProjectRoot.empty());
+  }
 
   void lookup(const clangd::LookupRequest &Request,
   llvm::function_ref Callback) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85713: [SyntaxTree]: Use Annotations in tests to reduce noise

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas added a reviewer: gribozavr2.
eduucaldas added a comment.

A proposition, upon review I'll change other tests.

One concern is that we might lose coverage while reducing noise. But I'll take 
a look into that with calm when changing the tests.




Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:186-187
+auto AnnotatedRanges = AnnotatedCode.ranges();
+assert(AnnotatedRanges.size() == TreeDumps.size());
+for (auto i = 0u; i < AnnotatedRanges.size(); i++) {
+  auto *AnnotatedNode = nodeByRange(AnnotatedRanges[i], Root);

I just wanted to do a for( auto [range, dump]& : zip(AnnotatedRanges,TreeDumps))

Is indexed loop the way to go in C++? 
And also I used `0u` here because we make a comparison to 
`std::vector::size_type`, is there a less error-prone way of writing those loop 
indexes?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85713

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

[PATCH] D85193: [clang] Do not use an invalid expression to update the initializer.

[Aleksandr Platonov via Phabricator via cfe-commits]

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

Check for `expr == nullptr` inside 
`InitListChecker::UpdateStructuredListElement()`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85193

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/Sema/init-invalid-struct-array.c


Index: clang/test/Sema/init-invalid-struct-array.c
===
--- /dev/null
+++ clang/test/Sema/init-invalid-struct-array.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only
+
+struct S {
+  Unknown u; // expected-error {{unknown type name 'Unknown'}}
+  int i;
+};
+// Should not crash
+struct S s[] = {[0].i = 0, [1].i = 1, {}};
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -1585,10 +1585,7 @@
   IList->setInit(Index, ResultExpr);
 }
   }
-  if (hadError)
-++StructuredIndex;
-  else
-UpdateStructuredListElement(StructuredList, StructuredIndex, ResultExpr);
+  UpdateStructuredListElement(StructuredList, StructuredIndex, ResultExpr);
   ++Index;
 }
 
@@ -1643,10 +1640,7 @@
   if (!VerifyOnly && expr)
 IList->setInit(Index, expr);
 
-  if (hadError)
-++StructuredIndex;
-  else
-UpdateStructuredListElement(StructuredList, StructuredIndex, expr);
+  UpdateStructuredListElement(StructuredList, StructuredIndex, expr);
   ++Index;
 }
 
@@ -1697,11 +1691,7 @@
   IList->setInit(Index, ResultExpr);
 }
   }
-  if (hadError)
-++StructuredIndex;
-  else
-UpdateStructuredListElement(StructuredList, StructuredIndex,
-ResultExpr);
+  UpdateStructuredListElement(StructuredList, StructuredIndex, ResultExpr);
   ++Index;
   return;
 }
@@ -3100,8 +3090,10 @@
 
   if (Expr *PrevInit = StructuredList->updateInit(SemaRef.Context,
   StructuredIndex, expr)) {
-// This initializer overwrites a previous initializer. Warn.
-diagnoseInitOverride(PrevInit, expr->getSourceRange());
+if (expr) {
+  // This initializer overwrites a previous initializer. Warn.
+  diagnoseInitOverride(PrevInit, expr->getSourceRange());
+}
   }
 
   ++StructuredIndex;


Index: clang/test/Sema/init-invalid-struct-array.c
===
--- /dev/null
+++ clang/test/Sema/init-invalid-struct-array.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only
+
+struct S {
+  Unknown u; // expected-error {{unknown type name 'Unknown'}}
+  int i;
+};
+// Should not crash
+struct S s[] = {[0].i = 0, [1].i = 1, {}};
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -1585,10 +1585,7 @@
   IList->setInit(Index, ResultExpr);
 }
   }
-  if (hadError)
-++StructuredIndex;
-  else
-UpdateStructuredListElement(StructuredList, StructuredIndex, ResultExpr);
+  UpdateStructuredListElement(StructuredList, StructuredIndex, ResultExpr);
   ++Index;
 }
 
@@ -1643,10 +1640,7 @@
   if (!VerifyOnly && expr)
 IList->setInit(Index, expr);
 
-  if (hadError)
-++StructuredIndex;
-  else
-UpdateStructuredListElement(StructuredList, StructuredIndex, expr);
+  UpdateStructuredListElement(StructuredList, StructuredIndex, expr);
   ++Index;
 }
 
@@ -1697,11 +1691,7 @@
   IList->setInit(Index, ResultExpr);
 }
   }
-  if (hadError)
-++StructuredIndex;
-  else
-UpdateStructuredListElement(StructuredList, StructuredIndex,
-ResultExpr);
+  UpdateStructuredListElement(StructuredList, StructuredIndex, ResultExpr);
   ++Index;
   return;
 }
@@ -3100,8 +3090,10 @@
 
   if (Expr *PrevInit = StructuredList->updateInit(SemaRef.Context,
   StructuredIndex, expr)) {
-// This initializer overwrites a previous initializer. Warn.
-diagnoseInitOverride(PrevInit, expr->getSourceRange());
+if (expr) {
+  // This initializer overwrites a previous initializer. Warn.
+  diagnoseInitOverride(PrevInit, expr->getSourceRange());
+}
   }
 
   ++StructuredIndex;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85193: [clang] Do not use an invalid expression to update the initializer.

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX added a comment.

In D85193#2207798 , @aaron.ballman 
wrote:

> In D85193#2207052 , @ArcsinX wrote:
>
>> In D85193#2204685 , @aaron.ballman 
>> wrote:
>>
>>> I sort of wonder if the correct change is to make 
>>> `UpdateStructuredListElement()` resilient to being passed a null `Expr *` 
>>> and then removing the manual increments when the expression is an error.
>>
>> Should it be in this patch?
>
> Given that we have at least four uses of this pattern in the file, I'd say it 
> should be this patch. WDYT?

Agree, updated patch.

But I am not sure about correct place for `expr` check. Seems `nullptr` 
dereference possible only here: `diagnoseInitOverride(PrevInit, 
expr->getSourceRange());`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85193

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

Re: [clang] 6170072 - Improve modeling of variable template specializations with dependent

[Martin Storsjö via cfe-commits]

On Tue, 11 Aug 2020, Richard Smith wrote:


On Tue, 11 Aug 2020 at 00:29, Martin Storsjö  wrote:
  On Sun, 9 Aug 2020, Richard Smith via cfe-commits wrote:

  >
  > Author: Richard Smith
  > Date: 2020-08-09T23:22:26-07:00
  > New Revision: 617007240cbfb97c8ccf6d61b0c4ca0bb62d43c9
  >
  > 
URL:https://github.com/llvm/llvm-project/commit/617007240cbfb97c8ccf6d61b0c4ca0
  bb62d43c9
  > 
DIFF:https://github.com/llvm/llvm-project/commit/617007240cbfb97c8ccf6d61b0c4ca0
  bb62d43c9.diff
  >
  > LOG: Improve modeling of variable template specializations
  with dependent
  > arguments.
  >
  > Don't build a variable template specialization declaration
  until its
  > scope and template arguments are non-dependent.
  >
  > No functionality change intended, but the AST representation
  is now more
  > consistent with how we model other templates.

  This did turn out to make a functional change, breaking building
  the dev
  branch of Qt. A halfway reduced example below:


Thanks for reporting this! Reduced a bit more:

template
inline constexpr bool is_nothrow_assignable_v = true;

template class QCache {
    void replace() noexcept(is_nothrow_assignable_v);
};

We used to allow this and now reject. This code is ill-formed (no diagnostic
required) by [temp.res]/8, because it has no valid instantiations, because
the second template argument of is_nothrow_assignable_v is missing.

So, assuming the case from which this was reduced was similarly ill-formed
(which it looks like itis: 
https://github.com/qt/qtbase/blob/48c8322a613f58d19ad9e0262bbac437ce259
8f8/src/corelib/tools/qcache.h#L114), I think that's a (minor) quality of
implementation improvement -- we now diagnose a bug that we used to miss.
ICC also rejects this, although GCC does not, and all three compilers reject
the corresponding case using a class template instead of a variable
template.


Thanks! I sent an attempt at a fix upstream at 
https://codereview.qt-project.org/c/qt/qtbase/+/309878.


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

[PATCH] D85714: [AST][RecoveryExpr] Don't preserve the return type if the FunctionDecl is invalid.

[Haojian Wu via Phabricator via cfe-commits]

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

If a functionDecl is invalid (e.g. return type cannot be formed), int is
use as he fallback type, which may lead to some bogus diagnostics.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85714

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaCXX/recovery-expr-type.cpp


Index: clang/test/SemaCXX/recovery-expr-type.cpp
===
--- clang/test/SemaCXX/recovery-expr-type.cpp
+++ clang/test/SemaCXX/recovery-expr-type.cpp
@@ -68,3 +68,10 @@
 int &&f(int);  // expected-note {{candidate function not viable}}
 int &&k = f(); // expected-error {{no matching function for call}}
 }
+
+// verify that "type 'double' cannot bind to a value of unrelated type 'int'" 
diagnostic is suppressed.
+namespace test5 {
+  template using U = T; // expected-note {{template parameter is 
declared here}}
+  template U& f(); // expected-error {{pack expansion 
used as argument for non-pack parameter of alias template}}
+  double &s1 = f(); // expected-error {{no matching function}}
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -12821,6 +12821,8 @@
   auto ConsiderCandidate = [&](const OverloadCandidate &Candidate) {
 if (!Candidate.Function)
   return;
+if (Candidate.Function->isInvalidDecl())
+  return;
 QualType T = Candidate.Function->getReturnType();
 if (T.isNull())
   return;


Index: clang/test/SemaCXX/recovery-expr-type.cpp
===
--- clang/test/SemaCXX/recovery-expr-type.cpp
+++ clang/test/SemaCXX/recovery-expr-type.cpp
@@ -68,3 +68,10 @@
 int &&f(int);  // expected-note {{candidate function not viable}}
 int &&k = f(); // expected-error {{no matching function for call}}
 }
+
+// verify that "type 'double' cannot bind to a value of unrelated type 'int'" diagnostic is suppressed.
+namespace test5 {
+  template using U = T; // expected-note {{template parameter is declared here}}
+  template U& f(); // expected-error {{pack expansion used as argument for non-pack parameter of alias template}}
+  double &s1 = f(); // expected-error {{no matching function}}
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -12821,6 +12821,8 @@
   auto ConsiderCandidate = [&](const OverloadCandidate &Candidate) {
 if (!Candidate.Function)
   return;
+if (Candidate.Function->isInvalidDecl())
+  return;
 QualType T = Candidate.Function->getReturnType();
 if (T.isNull())
   return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85118: [clang][AArch64] Correct return type of Neon vqmovun intrinsics

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

Thanks for the pointer, I found more mistakes this way, e.g.: 
https://godbolt.org/z/f6n95z
(I don't think I can use  in a test but there is 
clang/test/Headers/x86-64-apple-macosx-types.cpp which uses is_same with it's 
own implementation so I can use/copy that)

There are ~5 more with incorrect return types so I'll add those to this patch 
as well as the return type check.
The arguments check is complicated by some intrinsics being macros but we can 
check the majority, I'll do that in a separate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85118

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

[PATCH] D85713: [SyntaxTree]: Use Annotations in tests to reduce noise

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:186-187
+auto AnnotatedRanges = AnnotatedCode.ranges();
+assert(AnnotatedRanges.size() == TreeDumps.size());
+for (auto i = 0u; i < AnnotatedRanges.size(); i++) {
+  auto *AnnotatedNode = nodeByRange(AnnotatedRanges[i], Root);

eduucaldas wrote:
> I just wanted to do a for( auto [range, dump]& : 
> zip(AnnotatedRanges,TreeDumps))
> 
> Is indexed loop the way to go in C++? 
> And also I used `0u` here because we make a comparison to 
> `std::vector::size_type`, is there a less error-prone way of writing those 
> loop indexes?
Yes, I believe what you have now is indeed the best way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85713

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

[PATCH] D82081: [z/OS] Add binary format goff and operating system zos to the triple

[Kai Nacke via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb3aece05313e: [SystemZ/ZOS] Add binary format goff and 
operating system zos to the triple (authored by Kai).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82081

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  llvm/include/llvm/ADT/Triple.h
  llvm/include/llvm/Support/TargetRegistry.h
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/unittests/ADT/TripleTest.cpp

Index: llvm/unittests/ADT/TripleTest.cpp
===
--- llvm/unittests/ADT/TripleTest.cpp
+++ llvm/unittests/ADT/TripleTest.cpp
@@ -136,6 +136,18 @@
   EXPECT_EQ(Triple::FreeBSD, T.getOS());
   EXPECT_EQ(Triple::UnknownEnvironment, T.getEnvironment());
 
+  T = Triple("s390x-ibm-zos");
+  EXPECT_EQ(Triple::systemz, T.getArch());
+  EXPECT_EQ(Triple::IBM, T.getVendor());
+  EXPECT_EQ(Triple::ZOS, T.getOS());
+  EXPECT_EQ(Triple::UnknownEnvironment, T.getEnvironment());
+
+  T = Triple("systemz-ibm-zos");
+  EXPECT_EQ(Triple::systemz, T.getArch());
+  EXPECT_EQ(Triple::IBM, T.getVendor());
+  EXPECT_EQ(Triple::ZOS, T.getOS());
+  EXPECT_EQ(Triple::UnknownEnvironment, T.getEnvironment());
+
   T = Triple("arm-none-none-eabi");
   EXPECT_EQ(Triple::arm, T.getArch());
   EXPECT_EQ(Triple::UnknownVendor, T.getVendor());
@@ -1314,6 +1326,15 @@
   EXPECT_EQ(Triple::ELF, Triple("i686-pc-windows-msvc-elf").getObjectFormat());
   EXPECT_EQ(Triple::ELF, Triple("i686-pc-cygwin-elf").getObjectFormat());
 
+  EXPECT_EQ(Triple::ELF, Triple("systemz-ibm-linux").getObjectFormat());
+  EXPECT_EQ(Triple::ELF, Triple("systemz-ibm-unknown").getObjectFormat());
+
+  EXPECT_EQ(Triple::GOFF, Triple("s390x-ibm-zos").getObjectFormat());
+  EXPECT_EQ(Triple::GOFF, Triple("systemz-ibm-zos").getObjectFormat());
+  EXPECT_EQ(Triple::GOFF, Triple("s390x-ibm-zos-goff").getObjectFormat());
+  EXPECT_EQ(Triple::GOFF, Triple("s390x-unknown-zos-goff").getObjectFormat());
+  EXPECT_EQ(Triple::GOFF, Triple("s390x---goff").getObjectFormat());
+
   EXPECT_EQ(Triple::Wasm, Triple("wasm32-unknown-unknown").getObjectFormat());
   EXPECT_EQ(Triple::Wasm, Triple("wasm64-unknown-unknown").getObjectFormat());
   EXPECT_EQ(Triple::Wasm, Triple("wasm32-wasi").getObjectFormat());
@@ -1360,6 +1381,9 @@
 
   T.setObjectFormat(Triple::XCOFF);
   EXPECT_EQ(Triple::XCOFF, T.getObjectFormat());
+
+  T.setObjectFormat(Triple::GOFF);
+  EXPECT_EQ(Triple::GOFF, T.getObjectFormat());
 }
 
 TEST(TripleTest, NormalizeWindows) {
Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -1950,9 +1950,10 @@
   case Triple::ELF:   return "asan_globals";
   case Triple::MachO: return "__DATA,__asan_globals,regular";
   case Triple::Wasm:
+  case Triple::GOFF:
   case Triple::XCOFF:
 report_fatal_error(
-"ModuleAddressSanitizer not implemented for object file format.");
+"ModuleAddressSanitizer not implemented for object file format");
   case Triple::UnknownObjectFormat:
 break;
   }
Index: llvm/lib/Support/Triple.cpp
===
--- llvm/lib/Support/Triple.cpp
+++ llvm/lib/Support/Triple.cpp
@@ -215,6 +215,7 @@
   case WASI: return "wasi";
   case WatchOS: return "watchos";
   case Win32: return "windows";
+  case ZOS: return "zos";
   }
 
   llvm_unreachable("Invalid OSType");
@@ -499,6 +500,7 @@
 .StartsWith("solaris", Triple::Solaris)
 .StartsWith("win32", Triple::Win32)
 .StartsWith("windows", Triple::Win32)
+.StartsWith("zos", Triple::ZOS)
 .StartsWith("haiku", Triple::Haiku)
 .StartsWith("minix", Triple::Minix)
 .StartsWith("rtems", Triple::RTEMS)
@@ -552,6 +554,7 @@
 .EndsWith("xcoff", Triple::XCOFF)
 .EndsWith("coff", Triple::COFF)
 .EndsWith("elf", Triple::ELF)
+.EndsWith("goff", Triple::GOFF)
 .EndsWith("macho", Triple::MachO)
 .EndsWith("wasm", Triple::Wasm)
 .Default(Triple::UnknownObjectFormat);
@@ -643,6 +646,7 @@
   case Triple::UnknownObjectFormat: return "";
   case Triple::COFF:  return "coff";
   case Triple::ELF:   return "elf";
+  case Triple::GOFF:  return "goff";
   case Triple::MachO: return "macho";
   case Triple::Wasm:  return "wasm";
   case Triple::XCOFF: return "xcoff";
@@ -700,7 +704,6 @@
   case Triple::sparcv9:
   case Triple::spir64:
   case Triple::spir:
-  case Triple::systemz:
   case Triple::tce:
   case Triple::tcele:
   case Triple::thumbeb:
@@ -714,6 +717,11 @@
   return Triple::XCOFF;
 return Triple::ELF;
 
+  ca

[clang] b3aece0 - [SystemZ/ZOS] Add binary format goff and operating system zos to the triple

[Kai Nacke via cfe-commits]

Author: Kai Nacke
Date: 2020-08-11T05:26:26-04:00
New Revision: b3aece05313e740d26e517a9f32af90818ee4390

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

LOG: [SystemZ/ZOS] Add binary format goff and operating system zos to the triple

Adds the binary format goff and the operating system zos to the triple
class. goff is selected as default binary format if zos is choosen as
operating system. No further functionality is added.

Reviewers: efriedma, tahonermann, hubert.reinterpertcast, MaskRay

Reviewed By: efriedma, tahonermann, hubert.reinterpertcast

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

Added: 


Modified: 
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/CodeGen/CGObjCMac.cpp
clang/lib/CodeGen/CodeGenModule.cpp
llvm/include/llvm/ADT/Triple.h
llvm/include/llvm/Support/TargetRegistry.h
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
llvm/lib/MC/MCObjectFileInfo.cpp
llvm/lib/Support/Triple.cpp
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
llvm/unittests/ADT/TripleTest.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index f0058690e43d..566affa91b76 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -256,6 +256,8 @@ static bool asanUseGlobalsGC(const Triple &T, const 
CodeGenOptions &CGOpts) {
 return true;
   case Triple::ELF:
 return CGOpts.DataSections && !CGOpts.DisableIntegratedAS;
+  case Triple::GOFF:
+llvm::report_fatal_error("ASan not implemented for GOFF");
   case Triple::XCOFF:
 llvm::report_fatal_error("ASan not implemented for XCOFF.");
   case Triple::Wasm:

diff  --git a/clang/lib/CodeGen/CGObjCMac.cpp b/clang/lib/CodeGen/CGObjCMac.cpp
index 1d0379afb4b5..5922c63e0116 100644
--- a/clang/lib/CodeGen/CGObjCMac.cpp
+++ b/clang/lib/CodeGen/CGObjCMac.cpp
@@ -5106,9 +5106,10 @@ std::string CGObjCCommonMac::GetSectionName(StringRef 
Section,
"expected the name to begin with __");
 return ("." + Section.substr(2) + "$B").str();
   case llvm::Triple::Wasm:
+  case llvm::Triple::GOFF:
   case llvm::Triple::XCOFF:
 llvm::report_fatal_error(
-"Objective-C support is unimplemented for object file format.");
+"Objective-C support is unimplemented for object file format");
   }
 
   llvm_unreachable("Unhandled llvm::Triple::ObjectFormatType enum");

diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 67f06ac1c07c..ff35d94626d1 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -4915,6 +4915,8 @@ CodeGenModule::GetAddrOfConstantCFString(const 
StringLiteral *Literal) {
   switch (Triple.getObjectFormat()) {
   case llvm::Triple::UnknownObjectFormat:
 llvm_unreachable("unknown file format");
+  case llvm::Triple::GOFF:
+llvm_unreachable("GOFF is not yet implemented");
   case llvm::Triple::XCOFF:
 llvm_unreachable("XCOFF is not yet implemented");
   case llvm::Triple::COFF:

diff  --git a/llvm/include/llvm/ADT/Triple.h b/llvm/include/llvm/ADT/Triple.h
index c578c097c6f6..7c3a4b2b699a 100644
--- a/llvm/include/llvm/ADT/Triple.h
+++ b/llvm/include/llvm/ADT/Triple.h
@@ -173,6 +173,7 @@ class Triple {
 OpenBSD,
 Solaris,
 Win32,
+ZOS,
 Haiku,
 Minix,
 RTEMS,
@@ -224,6 +225,7 @@ class Triple {
 
 COFF,
 ELF,
+GOFF,
 MachO,
 Wasm,
 XCOFF,
@@ -468,6 +470,8 @@ class Triple {
 return getSubArch() == Triple::ARMSubArch_v7k;
   }
 
+  bool isOSzOS() const { return getOS() == Triple::ZOS; }
+
   /// isOSDarwin - Is this a "Darwin" OS (macOS, iOS, tvOS or watchOS).
   bool isOSDarwin() const {
 return isMacOSX() || isiOS() || isWatchOS();
@@ -620,6 +624,9 @@ class Triple {
 return getObjectFormat() == Triple::COFF;
   }
 
+  /// Tests whether the OS uses the GOFF binary format.
+  bool isOSBinFormatGOFF() const { return getObjectFormat() == Triple::GOFF; }
+
   /// Tests whether the environment is MachO.
   bool isOSBinFormatMachO() const {
 return getObjectFormat() == Triple::MachO;

diff  --git a/llvm/include/llvm/Support/TargetRegistry.h 
b/llvm/include/llvm/Support/TargetRegistry.h
index d91eabae8235..2c65eb60f910 100644
--- a/llvm/include/llvm/Support/TargetRegistry.h
+++ b/llvm/include/llvm/Support/TargetRegistry.h
@@ -510,6 +510,8 @@ class Target {
 S = createWasmStreamer(Ctx, std::move(TAB), std::move(OW),
std::move(Emitter), RelaxAll);
   break;
+case Triple::GOFF:
+  report_fatal_error("GOFF MCObjectStreamer not implemented yet");
 case Triple::XCOFF:
   S = createXCOFFStreamer(Ctx, std::move(TAB), std::move(OW),
   std::move(Emitter), 

[PATCH] D85697: [clang-tidy] Add cppcoreguidelines-prefer-scoped-enums-over-unscoped

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Is 'over-unscoped' really needed in the name, would just 'prefer-scoped-enum' 
be better, WDYT?

For the case of macro, you can check if the location is a macro using 
`SourceLocation::isMacroID()`.

For this to work you would also need to check every single usage of the the 
values in the enum to make sure they are converted to use the scoped access.
You're best bet would be to actually store a map indexed by unscoped enum decls 
with a set of all their locations and maybe a flag to say if the fix can be 
applied or not.
For instance a fix couldn't be applied if any of the usages or decls are in 
macros.
This map could then be checked using the `endOfTranslationUnit` virtual method, 
with all the diags and fixes being spawned there.




Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferScopedEnumsOverUnscopedCheck.cpp:31-32
+  const auto *MatchedDecl = Result.Nodes.getNodeAs("enumDecls");
+  if (MatchedDecl->isScoped())
+return;
+  diag(MatchedDecl->getLocation(), "enumeration %0 is not a scoped 
enumeration")

Please hoist this logic into the matcher.
```
enumDecl(unless(isScoped())).bind("enumDecls")



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-scoped-enums-over-unscoped.rst:20
+B
+  }
+

Semi-colon after the struct definition.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-scoped-enums-over-unscoped.rst:29
+B
+  }
+

Semi-colon after the struct definition.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-scoped-enums-over-unscoped.cpp:27-28
+enum struct ScopedEnumWithStruct {
+  SEWC_FirstValue,
+  SEWC_SecondValue,
+};

nit:


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85697

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

[PATCH] D85666: [clang-tidy] IncludeInserter: allow <> in header name

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Should a line be added to the release notes to explain this behaviour:
'Checks that specify files to include now support wrapping the include in angle 
brackets to create a system include'?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85666

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

[PATCH] D85716: [AST][RecoveryExpr] Fix a bogus unused diagnostic when the type is preserved.

[Haojian Wu via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85716

Files:
  clang/lib/Sema/SemaStmt.cpp
  clang/test/SemaCXX/recovery-expr-type.cpp


Index: clang/test/SemaCXX/recovery-expr-type.cpp
===
--- clang/test/SemaCXX/recovery-expr-type.cpp
+++ clang/test/SemaCXX/recovery-expr-type.cpp
@@ -75,3 +75,17 @@
   template U& f(); // expected-error {{pack expansion 
used as argument for non-pack parameter of alias template}}
   double &s1 = f(); // expected-error {{no matching function}}
 }
+
+namespace test6 {
+struct Base {
+private:
+  ~Base();
+};
+struct Derived : Base { // expected-note {{default constructor of}}
+};
+
+void func() {
+  // verify that no -Wunused-variable diagnostic on the inner Derived 
expression.
+  (Derived(Derived())); // expected-error {{call to implicitly-deleted default 
constructor}}
+}
+}
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -350,6 +350,8 @@
   E = TE->getSubExpr();
 if (isa(E))
   return;
+if (isa(E))
+  return;
 if (const CXXConstructExpr *CE = dyn_cast(E))
   if (const CXXRecordDecl *RD = CE->getType()->getAsCXXRecordDecl())
 if (!RD->getAttr())


Index: clang/test/SemaCXX/recovery-expr-type.cpp
===
--- clang/test/SemaCXX/recovery-expr-type.cpp
+++ clang/test/SemaCXX/recovery-expr-type.cpp
@@ -75,3 +75,17 @@
   template U& f(); // expected-error {{pack expansion used as argument for non-pack parameter of alias template}}
   double &s1 = f(); // expected-error {{no matching function}}
 }
+
+namespace test6 {
+struct Base {
+private:
+  ~Base();
+};
+struct Derived : Base { // expected-note {{default constructor of}}
+};
+
+void func() {
+  // verify that no -Wunused-variable diagnostic on the inner Derived expression.
+  (Derived(Derived())); // expected-error {{call to implicitly-deleted default constructor}}
+}
+}
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -350,6 +350,8 @@
   E = TE->getSubExpr();
 if (isa(E))
   return;
+if (isa(E))
+  return;
 if (const CXXConstructExpr *CE = dyn_cast(E))
   if (const CXXRecordDecl *RD = CE->getType()->getAsCXXRecordDecl())
 if (!RD->getAttr())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85031: [builtins] Unify the softfloat division implementation

[Anatoly Trosinenko via Phabricator via cfe-commits]

atrosinenko updated this revision to Diff 284623.
atrosinenko added a comment.

Refactoring


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85031

Files:
  compiler-rt/lib/builtins/divdf3.c
  compiler-rt/lib/builtins/divsf3.c
  compiler-rt/lib/builtins/divtf3.c
  compiler-rt/lib/builtins/fp_div_impl.inc
  compiler-rt/lib/builtins/fp_lib.h
  compiler-rt/lib/builtins/int_util.h
  compiler-rt/test/builtins/Unit/divdf3_test.c

Index: compiler-rt/test/builtins/Unit/divdf3_test.c
===
--- compiler-rt/test/builtins/Unit/divdf3_test.c
+++ compiler-rt/test/builtins/Unit/divdf3_test.c
@@ -80,5 +80,13 @@
 if (test__divdf3(0x1.0p+0, 0x1.0001p+0, UINT64_C(0x3fefffe0)))
   return 1;
 
+// some misc test cases obtained by fuzzing against h/w implementation
+if (test__divdf3(0x1.fdc239dd64735p-658, -0x1.fff9364c0843fp-948, UINT64_C(0xd20fdc8fc0ceffb1)))
+  return 1;
+if (test__divdf3(-0x1.78abb261d47c8p+794, 0x1.fb01d537cc5aep+266, UINT64_C(0xe0e7c6148ffc23e3)))
+  return 1;
+if (test__divdf3(-0x1.da7dfe6048b8bp-875, 0x1.ffc7ea3ff60a4p-610, UINT64_C(0xaf5dab1fe0269e2a)))
+  return 1;
+
 return 0;
 }
Index: compiler-rt/lib/builtins/int_util.h
===
--- compiler-rt/lib/builtins/int_util.h
+++ compiler-rt/lib/builtins/int_util.h
@@ -28,4 +28,20 @@
 #define COMPILE_TIME_ASSERT2(expr, cnt)\
   typedef char ct_assert_##cnt[(expr) ? 1 : -1] UNUSED
 
+// Force unrolling the code specified to be repeated N times.
+#define REPEAT_0_TIMES(code_to_repeat) /* do nothing */
+#define REPEAT_1_TIMES(code_to_repeat) code_to_repeat
+#define REPEAT_2_TIMES(code_to_repeat) \
+  REPEAT_1_TIMES(code_to_repeat)   \
+  code_to_repeat
+#define REPEAT_3_TIMES(code_to_repeat) \
+  REPEAT_2_TIMES(code_to_repeat)   \
+  code_to_repeat
+#define REPEAT_4_TIMES(code_to_repeat) \
+  REPEAT_3_TIMES(code_to_repeat)   \
+  code_to_repeat
+
+#define REPEAT_N_TIMES_(N, code_to_repeat) REPEAT_##N##_TIMES(code_to_repeat)
+#define REPEAT_N_TIMES(N, code_to_repeat) REPEAT_N_TIMES_(N, code_to_repeat)
+
 #endif // INT_UTIL_H
Index: compiler-rt/lib/builtins/fp_lib.h
===
--- compiler-rt/lib/builtins/fp_lib.h
+++ compiler-rt/lib/builtins/fp_lib.h
@@ -40,9 +40,12 @@
 
 #if defined SINGLE_PRECISION
 
+typedef uint16_t half_rep_t;
 typedef uint32_t rep_t;
+typedef uint64_t twice_rep_t;
 typedef int32_t srep_t;
 typedef float fp_t;
+#define HALF_REP_C UINT16_C
 #define REP_C UINT32_C
 #define significandBits 23
 
@@ -58,9 +61,11 @@
 
 #elif defined DOUBLE_PRECISION
 
+typedef uint32_t half_rep_t;
 typedef uint64_t rep_t;
 typedef int64_t srep_t;
 typedef double fp_t;
+#define HALF_REP_C UINT32_C
 #define REP_C UINT64_C
 #define significandBits 52
 
@@ -102,9 +107,11 @@
 #elif defined QUAD_PRECISION
 #if __LDBL_MANT_DIG__ == 113 && defined(__SIZEOF_INT128__)
 #define CRT_LDBL_128BIT
+typedef uint64_t half_rep_t;
 typedef __uint128_t rep_t;
 typedef __int128_t srep_t;
 typedef long double fp_t;
+#define HALF_REP_C UINT64_C
 #define REP_C (__uint128_t)
 // Note: Since there is no explicit way to tell compiler the constant is a
 // 128-bit integer, we let the constant be casted to 128-bit integer
Index: compiler-rt/lib/builtins/fp_div_impl.inc
===
--- /dev/null
+++ compiler-rt/lib/builtins/fp_div_impl.inc
@@ -0,0 +1,310 @@
+//===-- lib/fp_div_impl.inc - Floating point division -*- C -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file implements soft-float division with the IEEE-754 default
+// rounding (to nearest, ties to even).
+//
+//===--===//
+
+#include "fp_lib.h"
+
+#define HW (typeWidth / 2)
+#define loMask (REP_C(-1) >> HW)
+
+#define NUMBER_OF_ITERATIONS   \
+  (NUMBER_OF_HALF_ITERATIONS + NUMBER_OF_FULL_ITERATIONS)
+
+#if NUMBER_OF_FULL_ITERATIONS < 1
+#error At least one full iteration is required
+#endif
+
+static __inline fp_t __divXf3__(fp_t a, fp_t b) {
+
+  const unsigned int aExponent = toRep(a) >> significandBits & maxExponent;
+  const unsigned int bExponent = toRep(b) >> significandBits & maxExpon

[PATCH] D85032: [builtins] Make divXf3 handle denormal results

[Anatoly Trosinenko via Phabricator via cfe-commits]

atrosinenko updated this revision to Diff 284624.
atrosinenko added a comment.

Re-upload after amending previous commit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85032

Files:
  compiler-rt/lib/builtins/fp_div_impl.inc
  compiler-rt/test/builtins/Unit/divdf3_test.c
  compiler-rt/test/builtins/Unit/divsf3_test.c
  compiler-rt/test/builtins/Unit/divtf3_test.c

Index: compiler-rt/test/builtins/Unit/divtf3_test.c
===
--- compiler-rt/test/builtins/Unit/divtf3_test.c
+++ compiler-rt/test/builtins/Unit/divtf3_test.c
@@ -124,6 +124,13 @@
  UINT64_C(0xfffe)))
 return 1;
 
+// smallest normal value divided by 2.0
+if (test__divtf3(0x1.0p-16382L, 2.L, UINT64_C(0x8000), UINT64_C(0x0)))
+  return 1;
+// smallest subnormal result
+if (test__divtf3(0x1.0p-1022L, 0x1p+52L, UINT64_C(0x0), UINT64_C(0x1)))
+  return 1;
+
 // any / any
 if (test__divtf3(0x1.a23b45362464523375893ab4cdefp+5L,
  0x1.eedcbaba3a94546558237654321fp-1L,
Index: compiler-rt/test/builtins/Unit/divsf3_test.c
===
--- compiler-rt/test/builtins/Unit/divsf3_test.c
+++ compiler-rt/test/builtins/Unit/divsf3_test.c
@@ -80,5 +80,20 @@
 if (test__divsf3(0x1.0p+0F, 0x1.0001p+0F, UINT32_C(0x3f7fff00)))
   return 1;
 
+// smallest normal value divided by 2.0
+if (test__divsf3(0x1.0p-126F, 2.0F, UINT32_C(0x0040)))
+  return 1;
+// smallest subnormal result
+if (test__divsf3(0x1.0p-126F, 0x1p+23F, UINT32_C(0x0001)))
+  return 1;
+
+// some misc test cases obtained by fuzzing against h/w implementation
+if (test__divsf3(-0x1.3e75e6p-108F, -0x1.cf372p+38F, UINT32_C(0x0006)))
+  return 1;
+if (test__divsf3(0x1.e77c54p+81F, -0x1.e77c52p-47F, UINT32_C(0xff80)))
+  return 1;
+if (test__divsf3(0x1.fep-126F, 2.F, UINT32_C(0x0080)))
+  return 1;
+
 return 0;
 }
Index: compiler-rt/test/builtins/Unit/divdf3_test.c
===
--- compiler-rt/test/builtins/Unit/divdf3_test.c
+++ compiler-rt/test/builtins/Unit/divdf3_test.c
@@ -80,6 +80,13 @@
 if (test__divdf3(0x1.0p+0, 0x1.0001p+0, UINT64_C(0x3fefffe0)))
   return 1;
 
+// smallest normal value divided by 2.0
+if (test__divdf3(0x1.0p-1022, 2., UINT64_C(0x0008)))
+  return 1;
+// smallest subnormal result
+if (test__divdf3(0x1.0p-1022, 0x1.0p+52, UINT64_C(0x0001)))
+  return 1;
+
 // some misc test cases obtained by fuzzing against h/w implementation
 if (test__divdf3(0x1.fdc239dd64735p-658, -0x1.fff9364c0843fp-948, UINT64_C(0xd20fdc8fc0ceffb1)))
   return 1;
@@ -87,6 +94,12 @@
   return 1;
 if (test__divdf3(-0x1.da7dfe6048b8bp-875, 0x1.ffc7ea3ff60a4p-610, UINT64_C(0xaf5dab1fe0269e2a)))
   return 1;
+if (test__divdf3(0x1.0p-1022, 0x1.9p+5, UINT64_C(0x51eb851eb852)))
+  return 1;
+if (test__divdf3(0x1.0p-1022, 0x1.0028p+41, UINT64_C(0x07ff)))
+  return 1;
+if (test__divdf3(0x1.0p-1022, 0x1.0028p+52, UINT64_C(0x1)))
+  return 1;
 
 return 0;
 }
Index: compiler-rt/lib/builtins/fp_div_impl.inc
===
--- compiler-rt/lib/builtins/fp_div_impl.inc
+++ compiler-rt/lib/builtins/fp_div_impl.inc
@@ -249,6 +249,7 @@
   if (quotient_UQ1 < (implicitBit << 1)) {
 residualLo = (aSignificand << (significandBits + 1)) - quotient_UQ1 * bSignificand;
 writtenExponent -= 1;
+aSignificand <<= 1;
 
 // the error is doubled
   } else {
@@ -278,19 +279,25 @@
   // Now, quotient_UQ1_SB <= the correctly-rounded result
   // and may need taking NextAfter() up to 3 times (see error estimates above)
   // r = a - b * q
+  rep_t absResult;
+  if (writtenExponent > 0) {
+// Clear the implicit bit
+absResult = quotient_UQ1 & significandMask;
+// Insert the exponent
+absResult |= (rep_t)writtenExponent << significandBits;
+residualLo <<= 1;
+  } else {
+// Prevent shift amount from being negative
+if (significandBits + writtenExponent < 0)
+  return fromRep(quotientSign);
 
-  if (writtenExponent < 0) {
-// Result is definitely subnormal, flushing to zero
-return fromRep(quotientSign);
-  }
+absResult = quotient_UQ1 >> (-writtenExponent + 1);
 
-  // Clear the implicit bit
-  rep_t absResult = quotient_UQ1 & significandMask;
-  // Insert the exponent
-  absResult |= (rep_t)writtenExponent << significandBits;
+// multiplied by two to prevent shift amount to be negative
+residualLo = (aSignificand << (significandBits + writtenExponent)) - (absResult * bSignificand << 1);
+  }
 
   // Round
-  residualLo <<= 1;
   residualLo += absResult & 1; // t

[PATCH] D85429: [OpenCL] Allow for variadic macros in C++ for OpenCL

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Thanks


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

https://reviews.llvm.org/D85429

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

[PATCH] D85697: [clang-tidy] Add cppcoreguidelines-prefer-scoped-enums-over-unscoped

[János Benjamin Antal via Phabricator via cfe-commits]

janosbenjaminantal added a comment.

In D85697#2209272 , @njames93 wrote:

> Is 'over-unscoped' really needed in the name, would just 'prefer-scoped-enum' 
> be better, WDYT?
>
> For the case of macro, you can check if the location is a macro using 
> `SourceLocation::isMacroID()`.
>
> For this to work you would also need to check every single usage of the the 
> values in the enum to make sure they are converted to use the scoped access.
> You're best bet would be to actually store a map indexed by unscoped enum 
> decls with a set of all their locations and maybe a flag to say if the fix 
> can be applied or not.
> For instance a fix couldn't be applied if any of the usages or decls are in 
> macros.
> This map could then be checked using the `endOfTranslationUnit` virtual 
> method, with all the diags and fixes being spawned there.

Yes, it is better without 'over-unscoped'. I will change this.

I checked how other checks are dealing with macros, but I haven't found 
anything useful. Your idea sounds good, I will try to implement it.

- Fix not just the declarations, but also the enum usages
- Fix only the enums that are not declared/used in macros

As I am totally new to llvm, it might take a few days to come with a proper 
solutions, but I will do my best.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85697

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

[clang-tools-extra] 73a6a36 - [clangd] RIFF.cpp - Use logical && instead of bitwise & for padding check

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2020-08-11T11:38:43+01:00
New Revision: 73a6a36469468bb72d409d5179c6244e751545e2

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

LOG: [clangd] RIFF.cpp - Use logical && instead of bitwise & for padding check

Fixes PR47070

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/RIFF.cpp 
b/clang-tools-extra/clangd/RIFF.cpp
index b87c2d56af0c..f59200bd5856 100644
--- a/clang-tools-extra/clangd/RIFF.cpp
+++ b/clang-tools-extra/clangd/RIFF.cpp
@@ -33,7 +33,7 @@ llvm::Expected readChunk(llvm::StringRef &Stream) {
  llvm::Twine(Stream.size()));
   C.Data = Stream.take_front(Len);
   Stream = Stream.drop_front(Len);
-  if (Len % 2 & !Stream.empty()) { // Skip padding byte.
+  if ((Len % 2) && !Stream.empty()) { // Skip padding byte.
 if (Stream.front())
   return makeError("nonzero padding byte");
 Stream = Stream.drop_front();



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

[PATCH] D85600: [clang-format] use spaces for alignment of binary/ternary expressions with UT_AlignWithSpaces

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:1362-1367
+if (Style.AlignOperands != FormatStyle::OAS_DontAlign && Previous &&
+(Previous->getPrecedence() == prec::Assignment ||
+ Previous->is(tok::kw_return) ||
+ (*I == prec::Conditional && Previous->is(tok::question) &&
+  Previous->is(TT_ConditionalExpr))) &&
+!Newline)

You could factor out the part of the condition that doesn't concern 
`AlignOperands` to get rid of the repetition wrt. the previous `if`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85600

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

[PATCH] D85600: [clang-format] use spaces for alignment of binary/ternary expressions with UT_AlignWithSpaces

[Maximilian Fickert via Phabricator via cfe-commits]

fickert updated this revision to Diff 284643.
fickert added a comment.

Resolved repetitive conditions in the two consecutive `if`s and added a small 
comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85600

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11259,6 +11259,23 @@
"\t}\n"
"};",
Tab);
+  Tab.AlignOperands = FormatStyle::OAS_Align;
+  verifyFormat("int aa =  +\n"
+   " ;",
+   Tab);
+  // no alignment
+  verifyFormat("int aa =\n"
+   "\t;",
+   Tab);
+  verifyFormat("return  ? 111\n"
+   "   : bb ? 222\n"
+   ": 333;",
+   Tab);
+  Tab.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
+  Tab.AlignOperands = FormatStyle::OAS_AlignAfterOperator;
+  verifyFormat("int aa = \n"
+   "   + ;",
+   Tab);
 }
 
 TEST_F(FormatTest, ZeroTabWidth) {
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1348,16 +1348,20 @@
State.Stack.back().LastSpace);
 }
 
-// If BreakBeforeBinaryOperators is set, un-indent a bit to account for
-// the operator and keep the operands aligned
-if (Style.AlignOperands == FormatStyle::OAS_AlignAfterOperator &&
-Previous &&
+if (Previous &&
 (Previous->getPrecedence() == prec::Assignment ||
  Previous->is(tok::kw_return) ||
  (*I == prec::Conditional && Previous->is(tok::question) &&
   Previous->is(TT_ConditionalExpr))) &&
-!Newline)
-  NewParenState.UnindentOperator = true;
+!Newline) {
+  // If BreakBeforeBinaryOperators is set, un-indent a bit to account for
+  // the operator and keep the operands aligned
+  if (Style.AlignOperands == FormatStyle::OAS_AlignAfterOperator)
+NewParenState.UnindentOperator = true;
+  // Mark indentation as alignment if the expression is aligned.
+  if (Style.AlignOperands != FormatStyle::OAS_DontAlign)
+NewParenState.IsAligned = true;
+}
 
 // Do not indent relative to the fake parentheses inserted for "." or "->".
 // This is a special case to make the following to statements consistent:


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11259,6 +11259,23 @@
"\t}\n"
"};",
Tab);
+  Tab.AlignOperands = FormatStyle::OAS_Align;
+  verifyFormat("int aa =  +\n"
+   " ;",
+   Tab);
+  // no alignment
+  verifyFormat("int aa =\n"
+   "\t;",
+   Tab);
+  verifyFormat("return  ? 111\n"
+   "   : bb ? 222\n"
+   ": 333;",
+   Tab);
+  Tab.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
+  Tab.AlignOperands = FormatStyle::OAS_AlignAfterOperator;
+  verifyFormat("int aa = \n"
+   "   + ;",
+   Tab);
 }
 
 TEST_F(FormatTest, ZeroTabWidth) {
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1348,16 +1348,20 @@
State.Stack.back().LastSpace);
 }
 
-// If BreakBeforeBinaryOperators is set, un-indent a bit to account for
-// the operator and keep the operands aligned
-if (Style.AlignOperands == FormatStyle::OAS_AlignAfterOperator &&
-Previous &&
+if (Previous &&
 (Previous->getPrecedence() == prec::Assignment ||
  Previous->is(tok::kw_return) ||
  (*I == prec::Conditional && Previous->is(tok::question) &&
   Previous->is(TT_ConditionalExpr))) &&
-!Newline)
-  NewParenState.UnindentOperator = true;
+!Newline) {
+  // If BreakBeforeBinaryOperators is set, un-indent a bit to account for
+  /

[PATCH] D85600: [clang-format] use spaces for alignment of binary/ternary expressions with UT_AlignWithSpaces

[Marek Kurdej via Phabricator via cfe-commits]

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

LGTM. Thank you for the patch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85600

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

[PATCH] D85600: [clang-format] use spaces for alignment of binary/ternary expressions with UT_AlignWithSpaces

[Maximilian Fickert via Phabricator via cfe-commits]

fickert added a comment.

Thanks! I don't have push permissions to the repository so I cannot submit the 
commit myself (Maximilian Fickert ).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85600

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

[clang-tools-extra] bd1013a - [clangd] Enforce trailing slash for remote index's project root

[Kirill Bobyrev via cfe-commits]

Author: Kirill Bobyrev
Date: 2020-08-11T13:24:43+02:00
New Revision: bd1013a4825bf2cf58b3c75320f64b24b244f343

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

LOG: [clangd] Enforce trailing slash for remote index's project root

Reviewed By: hokein

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

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/index/remote/Client.cpp 
b/clang-tools-extra/clangd/index/remote/Client.cpp
index 4c1741e715a5..131e4c0b2fce 100644
--- a/clang-tools-extra/clangd/index/remote/Client.cpp
+++ b/clang-tools-extra/clangd/index/remote/Client.cpp
@@ -76,7 +76,9 @@ class IndexClient : public clangd::SymbolIndex {
   : Stub(remote::SymbolIndex::NewStub(Channel)),
 ProtobufMarshaller(new Marshaller(/*RemoteIndexRoot=*/"",
   /*LocalIndexRoot=*/ProjectRoot)),
-DeadlineWaitingTime(DeadlineTime) {}
+DeadlineWaitingTime(DeadlineTime) {
+assert(!ProjectRoot.empty());
+  }
 
   void lookup(const clangd::LookupRequest &Request,
   llvm::function_ref Callback) const 
{

diff  --git a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp 
b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
index dbda5bf24aa3..cfc72ce87be6 100644
--- a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -59,14 +59,16 @@ Marshaller::Marshaller(llvm::StringRef RemoteIndexRoot,
 assert(llvm::sys::path::is_absolute(RemoteIndexRoot));
 assert(RemoteIndexRoot ==
llvm::sys::path::convert_to_slash(RemoteIndexRoot));
-assert(RemoteIndexRoot.endswith(llvm::sys::path::get_separator()));
 this->RemoteIndexRoot = RemoteIndexRoot.str();
+if (!RemoteIndexRoot.endswith(llvm::sys::path::get_separator()))
+  *this->RemoteIndexRoot += llvm::sys::path::get_separator();
   }
   if (!LocalIndexRoot.empty()) {
 assert(llvm::sys::path::is_absolute(LocalIndexRoot));
 assert(LocalIndexRoot == 
llvm::sys::path::convert_to_slash(LocalIndexRoot));
-assert(LocalIndexRoot.endswith(llvm::sys::path::get_separator()));
 this->LocalIndexRoot = LocalIndexRoot.str();
+if (!LocalIndexRoot.endswith(llvm::sys::path::get_separator()))
+  *this->LocalIndexRoot += llvm::sys::path::get_separator();
   }
   assert(!RemoteIndexRoot.empty() || !LocalIndexRoot.empty());
 }



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

[PATCH] D85711: [clangd] Enforce trailing slash for remote index's project root

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85711

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


Index: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
===
--- clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -59,14 +59,16 @@
 assert(llvm::sys::path::is_absolute(RemoteIndexRoot));
 assert(RemoteIndexRoot ==
llvm::sys::path::convert_to_slash(RemoteIndexRoot));
-assert(RemoteIndexRoot.endswith(llvm::sys::path::get_separator()));
 this->RemoteIndexRoot = RemoteIndexRoot.str();
+if (!RemoteIndexRoot.endswith(llvm::sys::path::get_separator()))
+  *this->RemoteIndexRoot += llvm::sys::path::get_separator();
   }
   if (!LocalIndexRoot.empty()) {
 assert(llvm::sys::path::is_absolute(LocalIndexRoot));
 assert(LocalIndexRoot == 
llvm::sys::path::convert_to_slash(LocalIndexRoot));
-assert(LocalIndexRoot.endswith(llvm::sys::path::get_separator()));
 this->LocalIndexRoot = LocalIndexRoot.str();
+if (!LocalIndexRoot.endswith(llvm::sys::path::get_separator()))
+  *this->LocalIndexRoot += llvm::sys::path::get_separator();
   }
   assert(!RemoteIndexRoot.empty() || !LocalIndexRoot.empty());
 }
Index: clang-tools-extra/clangd/index/remote/Client.cpp
===
--- clang-tools-extra/clangd/index/remote/Client.cpp
+++ clang-tools-extra/clangd/index/remote/Client.cpp
@@ -76,7 +76,9 @@
   : Stub(remote::SymbolIndex::NewStub(Channel)),
 ProtobufMarshaller(new Marshaller(/*RemoteIndexRoot=*/"",
   /*LocalIndexRoot=*/ProjectRoot)),
-DeadlineWaitingTime(DeadlineTime) {}
+DeadlineWaitingTime(DeadlineTime) {
+assert(!ProjectRoot.empty());
+  }
 
   void lookup(const clangd::LookupRequest &Request,
   llvm::function_ref Callback) const 
{


Index: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
===
--- clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -59,14 +59,16 @@
 assert(llvm::sys::path::is_absolute(RemoteIndexRoot));
 assert(RemoteIndexRoot ==
llvm::sys::path::convert_to_slash(RemoteIndexRoot));
-assert(RemoteIndexRoot.endswith(llvm::sys::path::get_separator()));
 this->RemoteIndexRoot = RemoteIndexRoot.str();
+if (!RemoteIndexRoot.endswith(llvm::sys::path::get_separator()))
+  *this->RemoteIndexRoot += llvm::sys::path::get_separator();
   }
   if (!LocalIndexRoot.empty()) {
 assert(llvm::sys::path::is_absolute(LocalIndexRoot));
 assert(LocalIndexRoot == llvm::sys::path::convert_to_slash(LocalIndexRoot));
-assert(LocalIndexRoot.endswith(llvm::sys::path::get_separator()));
 this->LocalIndexRoot = LocalIndexRoot.str();
+if (!LocalIndexRoot.endswith(llvm::sys::path::get_separator()))
+  *this->LocalIndexRoot += llvm::sys::path::get_separator();
   }
   assert(!RemoteIndexRoot.empty() || !LocalIndexRoot.empty());
 }
Index: clang-tools-extra/clangd/index/remote/Client.cpp
===
--- clang-tools-extra/clangd/index/remote/Client.cpp
+++ clang-tools-extra/clangd/index/remote/Client.cpp
@@ -76,7 +76,9 @@
   : Stub(remote::SymbolIndex::NewStub(Channel)),
 ProtobufMarshaller(new Marshaller(/*RemoteIndexRoot=*/"",
   /*LocalIndexRoot=*/ProjectRoot)),
-DeadlineWaitingTime(DeadlineTime) {}
+DeadlineWaitingTime(DeadlineTime) {
+assert(!ProjectRoot.empty());
+  }
 
   void lookup(const clangd::LookupRequest &Request,
   llvm::function_ref Callback) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85711: [clangd] Enforce trailing slash for remote index's project root

[Kirill Bobyrev via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbd1013a4825b: [clangd] Enforce trailing slash for remote 
index's project root (authored by kbobyrev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85711

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


Index: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
===
--- clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -59,14 +59,16 @@
 assert(llvm::sys::path::is_absolute(RemoteIndexRoot));
 assert(RemoteIndexRoot ==
llvm::sys::path::convert_to_slash(RemoteIndexRoot));
-assert(RemoteIndexRoot.endswith(llvm::sys::path::get_separator()));
 this->RemoteIndexRoot = RemoteIndexRoot.str();
+if (!RemoteIndexRoot.endswith(llvm::sys::path::get_separator()))
+  *this->RemoteIndexRoot += llvm::sys::path::get_separator();
   }
   if (!LocalIndexRoot.empty()) {
 assert(llvm::sys::path::is_absolute(LocalIndexRoot));
 assert(LocalIndexRoot == 
llvm::sys::path::convert_to_slash(LocalIndexRoot));
-assert(LocalIndexRoot.endswith(llvm::sys::path::get_separator()));
 this->LocalIndexRoot = LocalIndexRoot.str();
+if (!LocalIndexRoot.endswith(llvm::sys::path::get_separator()))
+  *this->LocalIndexRoot += llvm::sys::path::get_separator();
   }
   assert(!RemoteIndexRoot.empty() || !LocalIndexRoot.empty());
 }
Index: clang-tools-extra/clangd/index/remote/Client.cpp
===
--- clang-tools-extra/clangd/index/remote/Client.cpp
+++ clang-tools-extra/clangd/index/remote/Client.cpp
@@ -76,7 +76,9 @@
   : Stub(remote::SymbolIndex::NewStub(Channel)),
 ProtobufMarshaller(new Marshaller(/*RemoteIndexRoot=*/"",
   /*LocalIndexRoot=*/ProjectRoot)),
-DeadlineWaitingTime(DeadlineTime) {}
+DeadlineWaitingTime(DeadlineTime) {
+assert(!ProjectRoot.empty());
+  }
 
   void lookup(const clangd::LookupRequest &Request,
   llvm::function_ref Callback) const 
{


Index: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
===
--- clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -59,14 +59,16 @@
 assert(llvm::sys::path::is_absolute(RemoteIndexRoot));
 assert(RemoteIndexRoot ==
llvm::sys::path::convert_to_slash(RemoteIndexRoot));
-assert(RemoteIndexRoot.endswith(llvm::sys::path::get_separator()));
 this->RemoteIndexRoot = RemoteIndexRoot.str();
+if (!RemoteIndexRoot.endswith(llvm::sys::path::get_separator()))
+  *this->RemoteIndexRoot += llvm::sys::path::get_separator();
   }
   if (!LocalIndexRoot.empty()) {
 assert(llvm::sys::path::is_absolute(LocalIndexRoot));
 assert(LocalIndexRoot == llvm::sys::path::convert_to_slash(LocalIndexRoot));
-assert(LocalIndexRoot.endswith(llvm::sys::path::get_separator()));
 this->LocalIndexRoot = LocalIndexRoot.str();
+if (!LocalIndexRoot.endswith(llvm::sys::path::get_separator()))
+  *this->LocalIndexRoot += llvm::sys::path::get_separator();
   }
   assert(!RemoteIndexRoot.empty() || !LocalIndexRoot.empty());
 }
Index: clang-tools-extra/clangd/index/remote/Client.cpp
===
--- clang-tools-extra/clangd/index/remote/Client.cpp
+++ clang-tools-extra/clangd/index/remote/Client.cpp
@@ -76,7 +76,9 @@
   : Stub(remote::SymbolIndex::NewStub(Channel)),
 ProtobufMarshaller(new Marshaller(/*RemoteIndexRoot=*/"",
   /*LocalIndexRoot=*/ProjectRoot)),
-DeadlineWaitingTime(DeadlineTime) {}
+DeadlineWaitingTime(DeadlineTime) {
+assert(!ProjectRoot.empty());
+  }
 
   void lookup(const clangd::LookupRequest &Request,
   llvm::function_ref Callback) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85600: [clang-format] use spaces for alignment of binary/ternary expressions with UT_AlignWithSpaces

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

Could this also be causing https://bugs.llvm.org/show_bug.cgi?id=33896 ?

I'll try to see if changes something in that case...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85600

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

[PATCH] D85721: [clangd] Unify macro matching in code completion for AST and Index based macros

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: hokein.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous.
Herald added a project: clang.
kadircet requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

fixes https://github.com/clangd/clangd/issues/489


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85721

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -194,9 +194,14 @@
   EXPECT_THAT(completions(Body + "int main() { S().Foba^ }").Completions,
   AllOf(Has("FooBar"), Has("FooBaz"), Not(Has("Qux";
 
-  // Macros require  prefix match.
-  EXPECT_THAT(completions(Body + "int main() { C^ }").Completions,
-  AllOf(Has("Car"), Not(Has("MotorCar";
+  // Macros require prefix match, either from index or AST.
+  Symbol Sym = var("MotorCarIndex");
+  Sym.SymInfo.Kind = index::SymbolKind::Macro;
+  EXPECT_THAT(
+  completions(Body + "int main() { C^ }", {Sym}).Completions,
+  AllOf(Has("Car"), Not(Has("MotorCar")), Not(Has("MotorCarIndex";
+  EXPECT_THAT(completions(Body + "int main() { M^ }", {Sym}).Completions,
+  AllOf(Has("MotorCar"), Has("MotorCarIndex")));
 }
 
 void testAfterDotCompletion(clangd::CodeCompleteOptions Opts) {
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -1618,7 +1618,10 @@
   llvm::Optional fuzzyScore(const CompletionCandidate &C) {
 // Macros can be very spammy, so we only support prefix completion.
 // We won't end up with underfull index results, as macros are sema-only.
-if (C.SemaResult && C.SemaResult->Kind == CodeCompletionResult::RK_Macro &&
+if (((C.SemaResult &&
+  C.SemaResult->Kind == CodeCompletionResult::RK_Macro) ||
+ (C.IndexResult &&
+  C.IndexResult->SymInfo.Kind == index::SymbolKind::Macro)) &&
 !C.Name.startswith_lower(Filter->pattern()))
   return None;
 return Filter->match(C.Name);


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -194,9 +194,14 @@
   EXPECT_THAT(completions(Body + "int main() { S().Foba^ }").Completions,
   AllOf(Has("FooBar"), Has("FooBaz"), Not(Has("Qux";
 
-  // Macros require  prefix match.
-  EXPECT_THAT(completions(Body + "int main() { C^ }").Completions,
-  AllOf(Has("Car"), Not(Has("MotorCar";
+  // Macros require prefix match, either from index or AST.
+  Symbol Sym = var("MotorCarIndex");
+  Sym.SymInfo.Kind = index::SymbolKind::Macro;
+  EXPECT_THAT(
+  completions(Body + "int main() { C^ }", {Sym}).Completions,
+  AllOf(Has("Car"), Not(Has("MotorCar")), Not(Has("MotorCarIndex";
+  EXPECT_THAT(completions(Body + "int main() { M^ }", {Sym}).Completions,
+  AllOf(Has("MotorCar"), Has("MotorCarIndex")));
 }
 
 void testAfterDotCompletion(clangd::CodeCompleteOptions Opts) {
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -1618,7 +1618,10 @@
   llvm::Optional fuzzyScore(const CompletionCandidate &C) {
 // Macros can be very spammy, so we only support prefix completion.
 // We won't end up with underfull index results, as macros are sema-only.
-if (C.SemaResult && C.SemaResult->Kind == CodeCompletionResult::RK_Macro &&
+if (((C.SemaResult &&
+  C.SemaResult->Kind == CodeCompletionResult::RK_Macro) ||
+ (C.IndexResult &&
+  C.IndexResult->SymInfo.Kind == index::SymbolKind::Macro)) &&
 !C.Name.startswith_lower(Filter->pattern()))
   return None;
 return Filter->match(C.Name);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85600: [clang-format] use spaces for alignment of binary/ternary expressions with UT_AlignWithSpaces

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In D85600#2209545 , @Typz wrote:

> Could this also be causing https://bugs.llvm.org/show_bug.cgi?id=33896 ?
>
> I'll try to see if changes something in that case...

Actually, I can confirm this is unrelated.




Comment at: clang/lib/Format/ContinuationIndenter.cpp:1360
+  if (Style.AlignOperands == FormatStyle::OAS_AlignAfterOperator)
+NewParenState.UnindentOperator = true;
+  // Mark indentation as alignment if the expression is aligned.

bad indent here : should be space-indented.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85600

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

[PATCH] D85600: [clang-format] use spaces for alignment of binary/ternary expressions with UT_AlignWithSpaces

[Maximilian Fickert via Phabricator via cfe-commits]

fickert added a comment.

Yes I think it's unrelated, though this patch should fix the alignment in that 
example when using the `UT_AlignWithSpaces` option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85600

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

[PATCH] D85599: [PowerPC] Remove isTerminator for TRAP instruction

[ChenZheng via Phabricator via cfe-commits]

shchenz added a comment.

With this change, we get verify failure for below case:

  int test_builtin_trap(int num) {
volatile int i = 0;
if (num > 0)
__builtin_unreachable();
else
 goto L1;
  L1:
return i;
  }



  clang t.c -target powerpc-unknown-unknown  -c -mllvm -verify-machineinstrs 
-mllvm -trap-unreachable=true



  *** Bad machine code: MBB exits via unconditional fall-through but ends with 
a barrier instruction! *** 
  - function:test_builtin_trap
  - basic block: %bb.1 if.then (0x1001f837b50)
  fatal error: error in backend: Found 1 machine code errors.

IR `unreachable` instruction is an IR level terminator, and it will be selected 
to `TRAP` in selectiondagbuilder, is it ok to change `TRAP` to non-terminator 
on PowerPC. If above case is valid, I think the answer is no.

There is no `TRAP` in IR level, I guess this is the reason why IR verifier does 
not complain. Setting `llvm.trap` as terminator seems also not right, because 
on some targets, it sets `ISD::TRAP` to non-terminator on purpose. See 
https://reviews.llvm.org/rL278144.

When selected `Intrinsic::trap` to `ISD::TRAP`, updating the BB info according 
to `TRAP` is a terminator or not on specific target? If `TRAP` is terminator, 
add a new BB, if it is not, do nothing? I am not sure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85599

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

[PATCH] D83325: [Sema] Iteratively strip sugar when removing address spaces.

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh accepted this revision.
svenvh added a comment.

LGTM, but just wondering if the test actually belongs to this patch, as it 
doesn't demonstrate the problem without one of your other patches?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83325

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

[PATCH] D85600: [clang-format] use spaces for alignment of binary/ternary expressions with UT_AlignWithSpaces

[Maximilian Fickert via Phabricator via cfe-commits]

fickert added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:1360
+  if (Style.AlignOperands == FormatStyle::OAS_AlignAfterOperator)
+NewParenState.UnindentOperator = true;
+  // Mark indentation as alignment if the expression is aligned.

Typz wrote:
> bad indent here : should be space-indented.
Can you clarify? This is formatted with the LLVM clang-format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85600

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

[clang] 031738a - [AST][RecoveryExpr] Don't preserve the return type if the FunctionDecl is invalid.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2020-08-11T13:49:57+02:00
New Revision: 031738a5611843674d5f739e739e8a386d5e741b

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

LOG: [AST][RecoveryExpr] Don't preserve the return type if the FunctionDecl is 
invalid.

If a functionDecl is invalid (e.g. return type cannot be formed), int is
use as he fallback type, which may lead to some bogus diagnostics.

Reviewed By: sammccall

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

Added: 


Modified: 
clang/lib/Sema/SemaOverload.cpp
clang/test/SemaCXX/recovery-expr-type.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 5a20a7990c3b..bcf6aa7a310e 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -12821,6 +12821,8 @@ static QualType chooseRecoveryType(OverloadCandidateSet 
&CS,
   auto ConsiderCandidate = [&](const OverloadCandidate &Candidate) {
 if (!Candidate.Function)
   return;
+if (Candidate.Function->isInvalidDecl())
+  return;
 QualType T = Candidate.Function->getReturnType();
 if (T.isNull())
   return;

diff  --git a/clang/test/SemaCXX/recovery-expr-type.cpp 
b/clang/test/SemaCXX/recovery-expr-type.cpp
index 6cd79326e8c3..2ce47f2f8efc 100644
--- a/clang/test/SemaCXX/recovery-expr-type.cpp
+++ b/clang/test/SemaCXX/recovery-expr-type.cpp
@@ -68,3 +68,10 @@ namespace test4 {
 int &&f(int);  // expected-note {{candidate function not viable}}
 int &&k = f(); // expected-error {{no matching function for call}}
 }
+
+// verify that "type 'double' cannot bind to a value of unrelated type 'int'" 
diagnostic is suppressed.
+namespace test5 {
+  template using U = T; // expected-note {{template parameter is 
declared here}}
+  template U& f(); // expected-error {{pack expansion 
used as argument for non-pack parameter of alias template}}
+  double &s1 = f(); // expected-error {{no matching function}}
+}



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

[PATCH] D85714: [AST][RecoveryExpr] Don't preserve the return type if the FunctionDecl is invalid.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG031738a56118: [AST][RecoveryExpr] Don't preserve the 
return type if the FunctionDecl is… (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85714

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaCXX/recovery-expr-type.cpp


Index: clang/test/SemaCXX/recovery-expr-type.cpp
===
--- clang/test/SemaCXX/recovery-expr-type.cpp
+++ clang/test/SemaCXX/recovery-expr-type.cpp
@@ -68,3 +68,10 @@
 int &&f(int);  // expected-note {{candidate function not viable}}
 int &&k = f(); // expected-error {{no matching function for call}}
 }
+
+// verify that "type 'double' cannot bind to a value of unrelated type 'int'" 
diagnostic is suppressed.
+namespace test5 {
+  template using U = T; // expected-note {{template parameter is 
declared here}}
+  template U& f(); // expected-error {{pack expansion 
used as argument for non-pack parameter of alias template}}
+  double &s1 = f(); // expected-error {{no matching function}}
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -12821,6 +12821,8 @@
   auto ConsiderCandidate = [&](const OverloadCandidate &Candidate) {
 if (!Candidate.Function)
   return;
+if (Candidate.Function->isInvalidDecl())
+  return;
 QualType T = Candidate.Function->getReturnType();
 if (T.isNull())
   return;


Index: clang/test/SemaCXX/recovery-expr-type.cpp
===
--- clang/test/SemaCXX/recovery-expr-type.cpp
+++ clang/test/SemaCXX/recovery-expr-type.cpp
@@ -68,3 +68,10 @@
 int &&f(int);  // expected-note {{candidate function not viable}}
 int &&k = f(); // expected-error {{no matching function for call}}
 }
+
+// verify that "type 'double' cannot bind to a value of unrelated type 'int'" diagnostic is suppressed.
+namespace test5 {
+  template using U = T; // expected-note {{template parameter is declared here}}
+  template U& f(); // expected-error {{pack expansion used as argument for non-pack parameter of alias template}}
+  double &s1 = f(); // expected-error {{no matching function}}
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -12821,6 +12821,8 @@
   auto ConsiderCandidate = [&](const OverloadCandidate &Candidate) {
 if (!Candidate.Function)
   return;
+if (Candidate.Function->isInvalidDecl())
+  return;
 QualType T = Candidate.Function->getReturnType();
 if (T.isNull())
   return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85600: [clang-format] use spaces for alignment of binary/ternary expressions with UT_AlignWithSpaces

[Francois Ferrand via Phabricator via cfe-commits]

Typz added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:1360
+  if (Style.AlignOperands == FormatStyle::OAS_AlignAfterOperator)
+NewParenState.UnindentOperator = true;
+  // Mark indentation as alignment if the expression is aligned.

fickert wrote:
> Typz wrote:
> > bad indent here : should be space-indented.
> Can you clarify? This is formatted with the LLVM clang-format.
Phabricator shows some kind of `>>` symbol at the beginning of this line, which 
i interpreted as a TAB character, hence my comment.
But indeed no issue after looking at the raw file, sorry about this.
  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85600

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

[clang] ee17f72 - Fix Wdocumentation unknown param warning. NFC.

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2020-08-11T12:52:37+01:00
New Revision: ee17f72e13b839001a6e8a77e767d5b15d2bd4d1

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

LOG: Fix Wdocumentation unknown param warning. NFC.

Added: 


Modified: 
clang/include/clang/AST/DeclOpenMP.h

Removed: 




diff  --git a/clang/include/clang/AST/DeclOpenMP.h 
b/clang/include/clang/AST/DeclOpenMP.h
index a035de1e20e2..e595ec98d914 100644
--- a/clang/include/clang/AST/DeclOpenMP.h
+++ b/clang/include/clang/AST/DeclOpenMP.h
@@ -43,9 +43,6 @@ template  class OMPDeclarativeDirective : public 
U {
   OMPChildren *Data = nullptr;
 
   /// Build instance of directive.
-  ///
-  /// \param StartLoc Starting location of the directive (directive keyword).
-  ///
   template 
   OMPDeclarativeDirective(Params &&... P) : U(std::forward(P)...) {}
 



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

[clang] 49193e1 - Fix Wdocumentation unknown param warnings. NFC.

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2020-08-11T12:52:37+01:00
New Revision: 49193e1fe7e143555d3b740610d6d1d20a553b2f

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

LOG: Fix Wdocumentation unknown param warnings. NFC.

Added: 


Modified: 
clang/include/clang/AST/StmtOpenMP.h

Removed: 




diff  --git a/clang/include/clang/AST/StmtOpenMP.h 
b/clang/include/clang/AST/StmtOpenMP.h
index 0bb91a4ec9ec..b7bbf15949a0 100644
--- a/clang/include/clang/AST/StmtOpenMP.h
+++ b/clang/include/clang/AST/StmtOpenMP.h
@@ -3009,7 +3009,6 @@ class OMPCancellationPointDirective : public 
OMPExecutableDirective {
   ///
   /// \param StartLoc Starting location of the directive kind.
   /// \param EndLoc Ending location of the directive.
-  /// \param Data Data storage, containing info about associated clauses,
   /// statements and child expressions.
   ///
   OMPCancellationPointDirective(SourceLocation StartLoc, SourceLocation EndLoc)
@@ -3018,9 +3017,6 @@ class OMPCancellationPointDirective : public 
OMPExecutableDirective {
EndLoc) {}
 
   /// Build an empty directive.
-  /// \param Data Data storage, containing info about associated clauses,
-  /// statements and child expressions.
-  ///
   explicit OMPCancellationPointDirective()
   : OMPExecutableDirective(OMPCancellationPointDirectiveClass,
llvm::omp::OMPD_cancellation_point,



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

[PATCH] D85635: [clangd] Compute the inactive code range for semantic highlighting.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Yup, this is a nice improvement, though there are further things we could do.

As discussed offline, we could consider mapping "disabled" to an attribute but 
we can't really consume that well in VSCode (or any other editor yet) so let's 
leave it.




Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:238
+}
+size_t LineLength = MainCode.substr(*StartOfLine).find('\n');
+size_t EndOfLine = LineLength == llvm::StringRef::npos

A little more direct and avoiding the special case: 
```
StringRef LineText = MainCode.drop_front(StartOfLine).take_until([](char C) { 
return C == '\n'; });
...
{Position{Line, 0}, Position{Line, lspLength(LineText)}}
```



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:242
+   : *StartOfLine + LineLength;
+NonConflicting.push_back(
+{HighlightingKind::InactiveCode,

this may overlap existing tokens.
The spec is silent on this, it may pose a challenge to clients and makes for a 
complex model for little benefit. I think we should avoid overlapping tokens.

For now this can actually happen, as we consider `#ifndef FOO` to be disabled 
if FOO is defined, but also a usage of the macro FOO.
I'd suggest erasing these (collect their indexes and delete them in a second 
pass).

In future, I think we probably want to reduce the scope of the disabled regions 
to not include the directive itself, then this can become an assert.



Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:83
+ llvm::ArrayRef Tokens,
+ size_t NextChar = 0) {
   assert(std::is_sorted(

AIUI the extra complexity here is to accommodate overlapping ranges, which we 
can likely get rid of.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85635

___
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.

[Szabolcs Nagy via Phabricator via cfe-commits]

nsz added a comment.

In D80791#2207203 , @chill wrote:

> I would prefer to avoid the situation where the markings of two otherwise 
> identical files were different,
> depending on how the files were produced, no matter if it was a common or a 
> special case.

i don't see why it is desirable to silently get marking on an object file if 
function definitions happen to be bti compatible in it:

- compiler cannot reliably do this (e.g. bti incompatible inline asm).
- some users don't want the marking: not all linkers support it so it can cause 
unexpected breakage.
- most users (all?) want the marking reliably (not opportunistically), but 
function annotations are fragile (can depend on optimizations and code outside 
of user control).
- it is not useful to have a bti annotated function unless everything else is 
bti compatible too: it is all or nothing per elf module.
- but a compiler cannot diagnose if only some functions have the annotation (we 
don't have a cflag for it) so even if the compiler tried to add the marking 
silently users cannot rely on it: it's too easy to drop the marking and no way 
to debug such failure.


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] D85722: [z/OS] enable trigraphs by default on z/OS

[Abhina Sreeskantharajan via Phabricator via cfe-commits]

abhina.sreeskantharajan created this revision.
abhina.sreeskantharajan added reviewers: thakis, hubert.reinterpretcast, 
uweigand, yusra.syeda, Kai, zibi, MaskRay, anirudhp.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
abhina.sreeskantharajan requested review of this revision.

This patch enables trigraphs on z/OS.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85722

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Frontend/trigraphs.cpp


Index: clang/test/Frontend/trigraphs.cpp
===
--- clang/test/Frontend/trigraphs.cpp
+++ clang/test/Frontend/trigraphs.cpp
@@ -4,12 +4,15 @@
 // RUN: %clang_cc1 -DSTDCPP17 -std=c++1z -verify -fsyntax-only %s
 // RUN: %clang_cc1 -DSTDCPP17TRI -ftrigraphs -std=c++1z -verify -fsyntax-only 
%s
 // RUN: %clang_cc1 -DMSCOMPAT -fms-compatibility -std=c++11 -verify 
-fsyntax-only %s
+// RUN: %clang_cc1 -DZOS -triple=s390x-none-zos -verify -fsyntax-only %s
+// RUN: %clang_cc1 -DZOSNOTRI -triple=s390x-none-zos -fno-trigraphs -verify 
-fsyntax-only %s
 
 void foo() {
 #if defined(NOFLAGS) || defined(STDCPP11) || defined(STDGNU11TRI) || \
-defined(STDCPP17TRI)
+defined(STDCPP17TRI) || defined(ZOS)
   const char c[] = "??/n"; // expected-warning{{trigraph converted to '\' 
character}}
-#elif defined(STDGNU11) || defined(STDCPP17) || defined(MSCOMPAT)
+#elif defined(STDGNU11) || defined(STDCPP17) || defined(MSCOMPAT) || \
+defined(ZOSNOTRI)
   const char c[] = "??/n"; // expected-warning{{trigraph ignored}}
 #else
 #error Not handled.
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2785,9 +2785,10 @@
   }
 
   // Mimicking gcc's behavior, trigraphs are only enabled if -trigraphs
-  // is specified, or -std is set to a conforming mode.
+  // is specified, -std is set to a conforming mode, or on z/OS.
   // Trigraphs are disabled by default in c++1z onwards.
-  Opts.Trigraphs = !Opts.GNUMode && !Opts.MSVCCompat && !Opts.CPlusPlus17;
+  Opts.Trigraphs =
+  (!Opts.GNUMode && !Opts.MSVCCompat && !Opts.CPlusPlus17) || T.isOSzOS();
   Opts.Trigraphs =
   Args.hasFlag(OPT_ftrigraphs, OPT_fno_trigraphs, Opts.Trigraphs);
 


Index: clang/test/Frontend/trigraphs.cpp
===
--- clang/test/Frontend/trigraphs.cpp
+++ clang/test/Frontend/trigraphs.cpp
@@ -4,12 +4,15 @@
 // RUN: %clang_cc1 -DSTDCPP17 -std=c++1z -verify -fsyntax-only %s
 // RUN: %clang_cc1 -DSTDCPP17TRI -ftrigraphs -std=c++1z -verify -fsyntax-only %s
 // RUN: %clang_cc1 -DMSCOMPAT -fms-compatibility -std=c++11 -verify -fsyntax-only %s
+// RUN: %clang_cc1 -DZOS -triple=s390x-none-zos -verify -fsyntax-only %s
+// RUN: %clang_cc1 -DZOSNOTRI -triple=s390x-none-zos -fno-trigraphs -verify -fsyntax-only %s
 
 void foo() {
 #if defined(NOFLAGS) || defined(STDCPP11) || defined(STDGNU11TRI) || \
-defined(STDCPP17TRI)
+defined(STDCPP17TRI) || defined(ZOS)
   const char c[] = "??/n"; // expected-warning{{trigraph converted to '\' character}}
-#elif defined(STDGNU11) || defined(STDCPP17) || defined(MSCOMPAT)
+#elif defined(STDGNU11) || defined(STDCPP17) || defined(MSCOMPAT) || \
+defined(ZOSNOTRI)
   const char c[] = "??/n"; // expected-warning{{trigraph ignored}}
 #else
 #error Not handled.
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2785,9 +2785,10 @@
   }
 
   // Mimicking gcc's behavior, trigraphs are only enabled if -trigraphs
-  // is specified, or -std is set to a conforming mode.
+  // is specified, -std is set to a conforming mode, or on z/OS.
   // Trigraphs are disabled by default in c++1z onwards.
-  Opts.Trigraphs = !Opts.GNUMode && !Opts.MSVCCompat && !Opts.CPlusPlus17;
+  Opts.Trigraphs =
+  (!Opts.GNUMode && !Opts.MSVCCompat && !Opts.CPlusPlus17) || T.isOSzOS();
   Opts.Trigraphs =
   Args.hasFlag(OPT_ftrigraphs, OPT_fno_trigraphs, Opts.Trigraphs);
 
___
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.

In D80791#2209624 , @nsz wrote:

> In D80791#2207203 , @chill wrote:
>
>> I would prefer to avoid the situation where the markings of two otherwise 
>> identical files were different,
>> depending on how the files were produced, no matter if it was a common or a 
>> special case.
>
> i don't see why it is desirable to silently get marking on an object file if 
> function definitions happen to be bti compatible in it:

It is desirable to get the marking because BTI-compatible functions don't 
appear by accident - they are a result of deliberate user actions, which clearly
express intent to use BTI.

> - compiler cannot reliably do this (e.g. bti incompatible inline asm).

Like for any other case, that's entirely the responsibility of the user if they 
use inline asm; Command-line options are not
special with regard to inline asm, so everything that can break 
attribute-derived marking, breaks command-line derived marking.

> - some users don't want the marking: not all linkers support it so it can 
> cause unexpected breakage.

Those linkers would need to be upgraded if the compiler imposes extra 
requirements on them.  One can't hold the compiler hostage to obsolete linkers. 
If users insist, they can just remove the .note section.

> - most users (all?) want the marking reliably (not opportunistically), but 
> function annotations are fragile (can depend on optimizations and code 
> outside of user control).

The user explicitly marking an object is the least reliable option, because 
it's done without regard what the object in actually contains.

> - it is not useful to have a bti annotated function unless everything else is 
> bti compatible too: it is all or nothing per elf module.

This is false. Some functions in an elf module could be in a guarded region, 
some in a non-guarded region. Some function may always
be called in a "BTI-safe" way, which may be unknown to the compiler.

> - but a compiler cannot diagnose if only some functions have the annotation 
> (we don't have a cflag for it) so even if the compiler tried to add the 
> marking silently users cannot rely on it: it's too easy to drop the marking 
> and no way to debug such failure.

At the time a compiler decides to or decides not to emit instructions which 
implement PAC-RET or BTI is perfectly clear what;s the effective annotation for 
each individual function.

I don't really understand the point of all these objections.

With my proposal to derive marking from function attributes, as well as from 
command-line
everything above will still work in the (arguably) most common case that we 
expect - users just using
command line.

I'm proposing to be strict and cover a few corner case where the command-line 
only approach produces bogus results.


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] D85713: [SyntaxTree]: Use Annotations in tests to reduce noise

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas updated this revision to Diff 284678.
eduucaldas added a comment.

Use annotations to reduce noise of expression tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85713

Files:
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -168,6 +168,35 @@
 return ::testing::AssertionSuccess();
   }
 
+  ::testing::AssertionResult
+  treeDumpEqualOnAnnotations(StringRef CodeWithAnnotations,
+ ArrayRef TreeDumps) {
+SCOPED_TRACE(llvm::join(GetParam().getCommandLineArgs(), " "));
+
+auto AnnotatedCode = llvm::Annotations(CodeWithAnnotations);
+auto *Root = buildTree(AnnotatedCode.code(), GetParam());
+
+if (Diags->getClient()->getNumErrors() != 0) {
+  return ::testing::AssertionFailure()
+ << "Source file has syntax errors, they were printed to the test "
+"log";
+}
+
+auto AnnotatedRanges = AnnotatedCode.ranges();
+assert(AnnotatedRanges.size() == TreeDumps.size());
+for (auto i = 0ul; i < AnnotatedRanges.size(); i++) {
+  auto *AnnotatedNode = nodeByRange(AnnotatedRanges[i], Root);
+  assert(AnnotatedNode);
+  auto AnnotatedNodeDump =
+  std::string(StringRef(AnnotatedNode->dump(*Arena)).trim());
+  // EXPECT_EQ shows the diff between the two strings if they are different.
+  EXPECT_EQ(TreeDumps[i].trim().str(), AnnotatedNodeDump);
+  if (AnnotatedNodeDump != TreeDumps[i].trim().str())
+return ::testing::AssertionFailure();
+}
+return ::testing::AssertionSuccess();
+  }
+
   // Adds a file to the test VFS.
   void addFile(StringRef Path, StringRef Contents) {
 if (!FS->addFile(Path, time_t(),
@@ -632,7 +661,7 @@
 )txt"));
 }
 
-TEST_P(SyntaxTreeTest, UnqualifiedId) {
+TEST_P(SyntaxTreeTest, UnqualifiedIdDecl) {
   if (!GetParam().isCXX()) {
 return;
   }
@@ -645,14 +674,7 @@
 };
 template
 void f(T&);
-void test(X x) {
-  x;  // identifier
-  operator+(x, x);// operator-function-id
-  f(x);// template-id
-  // TODO: Expose `id-expression` from `MemberExpr`
-  x.operator int();   // conversion-funtion-id
-  x.~X(); // ~type-name
-}
+void test(X x);
 )cpp",
   R"txt(
 *: TranslationUnit
@@ -722,72 +744,100 @@
   |   | `-SimpleDeclarator
   |   |   `-x
   |   `-)
-  `-CompoundStatement
-|-{
-|-ExpressionStatement
-| |-IdExpression
-| | `-UnqualifiedId
-| |   `-x
-| `-;
-|-ExpressionStatement
-| |-UnknownExpression
-| | |-IdExpression
-| | | `-UnqualifiedId
-| | |   |-operator
-| | |   `-+
-| | |-(
-| | |-IdExpression
-| | | `-UnqualifiedId
-| | |   `-x
-| | |-,
-| | |-IdExpression
-| | | `-UnqualifiedId
-| | |   `-x
-| | `-)
-| `-;
-|-ExpressionStatement
-| |-UnknownExpression
-| | |-IdExpression
-| | | `-UnqualifiedId
-| | |   |-f
-| | |   |-<
-| | |   |-X
-| | |   `->
-| | |-(
-| | |-IdExpression
-| | | `-UnqualifiedId
-| | |   `-x
-| | `-)
-| `-;
-|-ExpressionStatement
-| |-UnknownExpression
-| | |-UnknownExpression
-| | | |-IdExpression
-| | | | `-UnqualifiedId
-| | | |   `-x
-| | | |-.
-| | | |-operator
-| | | `-int
-| | |-(
-| | `-)
-| `-;
-|-ExpressionStatement
-| |-UnknownExpression
-| | |-UnknownExpression
-| | | |-IdExpression
-| | | | `-UnqualifiedId
-| | | |   `-x
-| | | |-.
-| | | |-~
-| | | `-X
-| | |-(
-| | `-)
-| `-;
-`-}
+  `-;
 )txt"));
 }
 
-TEST_P(SyntaxTreeTest, UnqualifiedIdCxx11OrLater) {
+TEST_P(SyntaxTreeTest, UnqualifiedId) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct X {
+  // TODO: Expose `id-expression` from `Declarator`
+  friend X operator+(const X&, const X&);
+  operator int();
+};
+template
+void f(T&);
+void test(X x) 
+[[{
+  x;  // identifier
+  operator+(x, x);// operator-function-id
+  f(x);// template-id
+  // TODO: Expose `id-expression` from `MemberExpr`
+  x.operator int();   // conversion-funtion-id
+  x.~X(); // ~type-name
+}]]
+)cpp",
+  {R"txt(
+CompoundStatement
+|-{
+|-ExpressionStatement
+| |-IdExpression
+| | `-UnqualifiedId
+| |   `-x
+| `-;
+|-ExpressionStatement
+| |-UnknownExpression
+| | |-IdExpression
+| | | `-UnqualifiedId
+| | |   |-operator
+| | |   `-+
+| | |-(
+| | |-IdExpression
+| | | `-UnqualifiedId
+| | |   `-x
+| | |-,
+| | |-IdExpression
+| | | `-UnqualifiedId
+| | |   `-x
+| | `-)
+| `-;
+|-ExpressionStatement
+| |-UnknownExpression
+| | |-IdExpression

[PATCH] D85600: [clang-format] use spaces for alignment of binary/ternary expressions with UT_AlignWithSpaces

[Marek Kurdej via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb18c63e85aa8: [clang-format] use spaces for alignment of 
binary/ternary expressions with… (authored by fickert, committed by curdeius).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85600

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11259,6 +11259,23 @@
"\t}\n"
"};",
Tab);
+  Tab.AlignOperands = FormatStyle::OAS_Align;
+  verifyFormat("int aa =  +\n"
+   " ;",
+   Tab);
+  // no alignment
+  verifyFormat("int aa =\n"
+   "\t;",
+   Tab);
+  verifyFormat("return  ? 111\n"
+   "   : bb ? 222\n"
+   ": 333;",
+   Tab);
+  Tab.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
+  Tab.AlignOperands = FormatStyle::OAS_AlignAfterOperator;
+  verifyFormat("int aa = \n"
+   "   + ;",
+   Tab);
 }
 
 TEST_F(FormatTest, ZeroTabWidth) {
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1348,16 +1348,20 @@
State.Stack.back().LastSpace);
 }
 
-// If BreakBeforeBinaryOperators is set, un-indent a bit to account for
-// the operator and keep the operands aligned
-if (Style.AlignOperands == FormatStyle::OAS_AlignAfterOperator &&
-Previous &&
+if (Previous &&
 (Previous->getPrecedence() == prec::Assignment ||
  Previous->is(tok::kw_return) ||
  (*I == prec::Conditional && Previous->is(tok::question) &&
   Previous->is(TT_ConditionalExpr))) &&
-!Newline)
-  NewParenState.UnindentOperator = true;
+!Newline) {
+  // If BreakBeforeBinaryOperators is set, un-indent a bit to account for
+  // the operator and keep the operands aligned
+  if (Style.AlignOperands == FormatStyle::OAS_AlignAfterOperator)
+NewParenState.UnindentOperator = true;
+  // Mark indentation as alignment if the expression is aligned.
+  if (Style.AlignOperands != FormatStyle::OAS_DontAlign)
+NewParenState.IsAligned = true;
+}
 
 // Do not indent relative to the fake parentheses inserted for "." or "->".
 // This is a special case to make the following to statements consistent:


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11259,6 +11259,23 @@
"\t}\n"
"};",
Tab);
+  Tab.AlignOperands = FormatStyle::OAS_Align;
+  verifyFormat("int aa =  +\n"
+   " ;",
+   Tab);
+  // no alignment
+  verifyFormat("int aa =\n"
+   "\t;",
+   Tab);
+  verifyFormat("return  ? 111\n"
+   "   : bb ? 222\n"
+   ": 333;",
+   Tab);
+  Tab.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
+  Tab.AlignOperands = FormatStyle::OAS_AlignAfterOperator;
+  verifyFormat("int aa = \n"
+   "   + ;",
+   Tab);
 }
 
 TEST_F(FormatTest, ZeroTabWidth) {
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1348,16 +1348,20 @@
State.Stack.back().LastSpace);
 }
 
-// If BreakBeforeBinaryOperators is set, un-indent a bit to account for
-// the operator and keep the operands aligned
-if (Style.AlignOperands == FormatStyle::OAS_AlignAfterOperator &&
-Previous &&
+if (Previous &&
 (Previous->getPrecedence() == prec::Assignment ||
  Previous->is(tok::kw_return) ||
  (*I == prec::Conditional && Previous->is(tok::question) &&
   Previous->is(TT_ConditionalExpr))) &&
-!Newline)
-  NewParenState.UnindentOperator = true;
+!Newline) {
+  // If Break

[clang] b18c63e - [clang-format] use spaces for alignment of binary/ternary expressions with UT_AlignWithSpaces

[Marek Kurdej via cfe-commits]

Author: Maximilian Fickert
Date: 2020-08-11T14:56:26+02:00
New Revision: b18c63e85aa8191e583cd1014757904c344b45d7

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

LOG: [clang-format] use spaces for alignment of binary/ternary expressions with 
UT_AlignWithSpaces

Use spaces to align binary and ternary expressions when using AlignOperands and 
UT_AlignWithSpaces.

This fixes an oversight in the new UT_AlignWithSpaces option (see D75034), 
which did not correctly identify the alignment of binary/ternary expressions.

Reviewed By: curdeius

Patch by: fickert

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/Format/ContinuationIndenter.cpp
index f3202bcb5bc1..d99107cb8b2c 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -1348,16 +1348,20 @@ void 
ContinuationIndenter::moveStatePastFakeLParens(LineState &State,
State.Stack.back().LastSpace);
 }
 
-// If BreakBeforeBinaryOperators is set, un-indent a bit to account for
-// the operator and keep the operands aligned
-if (Style.AlignOperands == FormatStyle::OAS_AlignAfterOperator &&
-Previous &&
+if (Previous &&
 (Previous->getPrecedence() == prec::Assignment ||
  Previous->is(tok::kw_return) ||
  (*I == prec::Conditional && Previous->is(tok::question) &&
   Previous->is(TT_ConditionalExpr))) &&
-!Newline)
-  NewParenState.UnindentOperator = true;
+!Newline) {
+  // If BreakBeforeBinaryOperators is set, un-indent a bit to account for
+  // the operator and keep the operands aligned
+  if (Style.AlignOperands == FormatStyle::OAS_AlignAfterOperator)
+NewParenState.UnindentOperator = true;
+  // Mark indentation as alignment if the expression is aligned.
+  if (Style.AlignOperands != FormatStyle::OAS_DontAlign)
+NewParenState.IsAligned = true;
+}
 
 // Do not indent relative to the fake parentheses inserted for "." or "->".
 // This is a special case to make the following to statements consistent:

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index d8642062fb6f..dcd9da77a390 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -11259,6 +11259,23 @@ TEST_F(FormatTest, ConfigurableUseOfTab) {
"\t}\n"
"};",
Tab);
+  Tab.AlignOperands = FormatStyle::OAS_Align;
+  verifyFormat("int aa =  +\n"
+   " ;",
+   Tab);
+  // no alignment
+  verifyFormat("int aa =\n"
+   "\t;",
+   Tab);
+  verifyFormat("return  ? 111\n"
+   "   : bb ? 222\n"
+   ": 333;",
+   Tab);
+  Tab.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
+  Tab.AlignOperands = FormatStyle::OAS_AlignAfterOperator;
+  verifyFormat("int aa = \n"
+   "   + ;",
+   Tab);
 }
 
 TEST_F(FormatTest, ZeroTabWidth) {



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

[PATCH] D85727: [clangd] Disable ExtractFunction for C

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: hokein.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous.
Herald added a project: clang.
kadircet requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

This tweak uses constructs like auto and refs, which are not available
in C.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85727

Files:
  clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -605,6 +605,9 @@
   EXPECT_THAT(apply(" if(true) [[{ return; }]] "), HasSubstr("extracted"));
   // Don't extract uncertain return
   EXPECT_THAT(apply(" if(true) [[if (false) return;]] "), StartsWith("fail"));
+
+  FileName = "a.c";
+  EXPECT_THAT(apply(" for([[int i = 0;]];);"), HasSubstr("unavailable"));
 }
 
 TEST_F(ExtractFunctionTest, FileTest) {
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -673,6 +673,8 @@
   const Node *CommonAnc = Inputs.ASTSelection.commonAncestor();
   const SourceManager &SM = Inputs.AST->getSourceManager();
   const LangOptions &LangOpts = Inputs.AST->getLangOpts();
+  if (!LangOpts.CPlusPlus)
+return false;
   if (auto MaybeExtZone = findExtractionZone(CommonAnc, SM, LangOpts)) {
 ExtZone = std::move(*MaybeExtZone);
 return true;


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -605,6 +605,9 @@
   EXPECT_THAT(apply(" if(true) [[{ return; }]] "), HasSubstr("extracted"));
   // Don't extract uncertain return
   EXPECT_THAT(apply(" if(true) [[if (false) return;]] "), StartsWith("fail"));
+
+  FileName = "a.c";
+  EXPECT_THAT(apply(" for([[int i = 0;]];);"), HasSubstr("unavailable"));
 }
 
 TEST_F(ExtractFunctionTest, FileTest) {
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -673,6 +673,8 @@
   const Node *CommonAnc = Inputs.ASTSelection.commonAncestor();
   const SourceManager &SM = Inputs.AST->getSourceManager();
   const LangOptions &LangOpts = Inputs.AST->getLangOpts();
+  if (!LangOpts.CPlusPlus)
+return false;
   if (auto MaybeExtZone = findExtractionZone(CommonAnc, SM, LangOpts)) {
 ExtZone = std::move(*MaybeExtZone);
 return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85727: [clangd] Disable ExtractFunction for C

[Haojian Wu via Phabricator via cfe-commits]

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

> This tweak uses constructs like auto and refs, which are not available in C.

I didn't find this tweak is using `auto`. I think we need to do the same thing 
for extract variable tweak as well (only for C++11).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85727

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

[PATCH] D85721: [clangd] Unify macro matching in code completion for AST and Index based macros

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1620
 // Macros can be very spammy, so we only support prefix completion.
 // We won't end up with underfull index results, as macros are sema-only.
+if (((C.SemaResult &&

this comment seems stale, macros are not sema-only now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85721

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

[PATCH] D85728: [Analyzer][WIP] Support for the new variadic isa<> and isa_and_nod_null<> in CastValueChecker

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

baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: NoQ, Charusso.
baloghadamsoftware added a project: clang.
Herald added subscribers: ASDenysPetrov, martong, steakhal, gamesh411, dkrupp, 
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, xazax.hun, 
whisperity.
Herald added a reviewer: Szelethus.
baloghadamsoftware requested review of this revision.

llvm::isa<>() and llvm::isa_and_not_null<>() template functions recently became 
variadic. Unfortunately this causes crashes in case of isa_and_not_null<>() and 
incorrect behavior in isa<>(). This patch fixes this issue.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85728

Files:
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/test/Analysis/Inputs/llvm.h
  clang/test/Analysis/cast-value-logic.cpp
  clang/test/Analysis/cast-value-notes.cpp

Index: clang/test/Analysis/cast-value-notes.cpp
===
--- clang/test/Analysis/cast-value-notes.cpp
+++ clang/test/Analysis/cast-value-notes.cpp
@@ -13,20 +13,22 @@
   const T *getAs() const;
 };
 class Triangle : public Shape {};
+class Rectangle : public Shape {};
+class Hexagon : public Shape {};
 class Circle : public Shape {};
 } // namespace clang
 
 using namespace llvm;
 using namespace clang;
 
-void evalReferences(const Shape &S) {
+/*void evalReferences(const Shape &S) {
   const auto &C = dyn_cast(S);
   // expected-note@-1 {{Assuming 'S' is not a 'Circle'}}
   // expected-note@-2 {{Dereference of null pointer}}
   // expected-warning@-3 {{Dereference of null pointer}}
-}
+  }*/
 
-void evalNonNullParamNonNullReturnReference(const Shape &S) {
+void evalNonNullParamNonNullReturnReference(const Shape *S) {
   // Unmodeled cast from reference to pointer.
   const auto *C = dyn_cast_or_null(S);
   // expected-note@-1 {{'C' initialized here}}
@@ -43,12 +45,36 @@
 return;
   }
 
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming 'C' is not a 'Rectangle'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming 'C' is not a 'Hexagon'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
   if (isa(C)) {
 // expected-note@-1 {{'C' is not a 'Triangle'}}
 // expected-note@-2 {{Taking false branch}}
 return;
   }
 
+  if (isa(C)) {
+// expected-note@-1 {{'C' is not a 'Triangle'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (isa(C)) {
+// expected-note@-1 {{'C' is not a 'Triangle'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
   if (isa(C)) {
 // expected-note@-1 {{'C' is a 'Circle'}}
 // expected-note@-2 {{Taking true branch}}
@@ -58,9 +84,29 @@
 // expected-note@-2 {{Division by zero}}
 // expected-warning@-3 {{Division by zero}}
   }
+
+  if (isa(C)) {
+// expected-note@-1 {{'C' is a 'Circle'}}
+// expected-note@-2 {{Taking true branch}}
+
+(void)(1 / !C);
+// expected-note@-1 {{'C' is non-null}}
+// expected-note@-2 {{Division by zero}}
+// expected-warning@-3 {{Division by zero}}
+  }
+
+  if (isa(C)) {
+// expected-note@-1 {{'C' is a 'Circle'}}
+// expected-note@-2 {{Taking true branch}}
+
+(void)(1 / !C);
+// expected-note@-1 {{'C' is non-null}}
+// expected-note@-2 {{Division by zero}}
+// expected-warning@-3 {{Division by zero}}
+  }
 }
 
-void evalNonNullParamNonNullReturn(const Shape *S) {
+/*void evalNonNullParamNonNullReturn(const Shape *S) {
   const auto *C = cast(S);
   // expected-note@-1 {{'S' is a 'Circle'}}
   // expected-note@-2 {{'C' initialized here}}
@@ -153,3 +199,4 @@
   // expected-note@-1 {{Division by zero}}
   // expected-warning@-2 {{Division by zero}}
 }
+*/
Index: clang/test/Analysis/cast-value-logic.cpp
===
--- clang/test/Analysis/cast-value-logic.cpp
+++ clang/test/Analysis/cast-value-logic.cpp
@@ -19,6 +19,8 @@
   virtual double area();
 };
 class Triangle : public Shape {};
+class Rectangle : public Shape {};
+class Hexagon : public Shape {};
 class Circle : public Shape {
 public:
   ~Circle();
@@ -39,6 +41,23 @@
 clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
 }
 
+void test_regions_isa_variadic(const Shape *A, const Shape *B) {
+  if (isa(A) &&
+  !isa(B))
+clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
+
+void test_regions_isa_and_nonnull(const Shape *A, const Shape *B) {
+  if (isa_and_nonnull(A) && !isa_and_nonnull(B))
+clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
+
+void test_regions_isa_and_nonnull_variadic(const Shape *A, const Shape *B) {
+  if (isa_and_nonnull(A) &&
+  !isa_and_nonnull(B))
+clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
+
 namespace test_cast {
 void evalLogic(const Shape *S) {
   const Circle *

[PATCH] D85728: [Analyzer][WIP] Support for the new variadic isa<> and isa_and_nod_null<> in CastValueChecker

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

baloghadamsoftware added a comment.

This patch intends to fix bug 47037 
. It is still work in progress 
with lots of debug printouts.

I described the issue with this patch at the bug report:

//The variadic tests fail because the whole Dyanamic Type library is extremely 
poor design: it stores the pointers and references as they are instead of the 
underlying types. Unfortunately, cast<> functions which can take pointers have 
pointers to SubstTemplateTypeParmType but isa<> functions take everything by 
reference. Thus they have a reference to a SubstTemplateTypeParmType which 
contains the pointer. The two will never match even if I unpack the reference 
for the isa<> functions and get to the pointer itself because I cannot create a 
new pointer type in the cast<> functions where SubstTemplateTypeParmType is 
inside the pointer. I did not found any method to remove 
SubstTemplateTypeParmType from a pointer either.

This results that the test evalNonNullParamNonNullReturnReference() passes only 
by chance in the original version. Even if I fix the parameter which should 
have been pointer instead of reference the early exit blocks which use 
dyn_cast<> store a different "from" type for the casts that we retrieve later 
at the isa<> calls. This way both true and false branches for the isa<> blocks 
remain alive. If I extend the tests for variadic templates we are not lucky 
anymore, the tests fail.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85728

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

[PATCH] D84844: [OpenMP] Ensure testing for versions 4.5 and default - Part 1

[Saiyedul Islam via Phabricator via cfe-commits]

saiislam added a comment.

Ping for this (part-1), D85150  (part-2), and 
D85214  (part-3).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84844

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

[PATCH] D85728: [Analyzer][WIP] Support for the new variadic isa<> and isa_and_nod_null<> in CastValueChecker

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

baloghadamsoftware added a comment.

My proposal is the following: do not store the casing info between pointers and 
references but the underlying types themselves. There are two ways to implement 
this: in the checkers using it (currently only this one) or behind the API of 
`DynamicCastInfo`. Maybe the latter is more appropriate.

To be fully correct we should not only change the `DynamicCastInfo` but also 
the `DynamicTypeInfo`: if the dynamic type of an object behind a pointer is 
known, then it remains the same also if we get a reference from the pointer or 
substitute a type for a template type parameter with it. (We can create new 
pointer or reference type using `ASTContext`.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85728

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

[PATCH] D85716: [AST][RecoveryExpr] Fix a bogus unused diagnostic when the type is preserved.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:353
   return;
+if (isa(E))
+  return;

This seems like a very specific patch to a special case of a potentially more 
general problem...



Comment at: clang/test/SemaCXX/recovery-expr-type.cpp:88
+void func() {
+  // verify that no -Wunused-variable diagnostic on the inner Derived 
expression.
+  (Derived(Derived())); // expected-error {{call to implicitly-deleted default 
constructor}}

(nit: I think this is -Wunused but not -Wunused-variable)



Comment at: clang/test/SemaCXX/recovery-expr-type.cpp:88
+void func() {
+  // verify that no -Wunused-variable diagnostic on the inner Derived 
expression.
+  (Derived(Derived())); // expected-error {{call to implicitly-deleted default 
constructor}}

sammccall wrote:
> (nit: I think this is -Wunused but not -Wunused-variable)
what makes you think this is firing on the *inner* expression? The diagnostic 
location is that of the outer expression. I think the existing logic is pretty 
close to correct (and the diagnostic is actually correct here, albeit through 
luck), and if we want to change it, the right fix belongs somewhere else.

The warning doesn't fire for just `Derived();` - isUnusedResultAWarning returns 
false for RecoveryExpr. Nor does it fire for `(Derived());` - 
isUnusedResultAWarning looks inside the paren expr and gets the same result for 
the contents.

For the case in this test, we hit the CXXFunctionalCastExpr/CStyleExprCast case 
of isUnusedResultAWarning. It says:

> If this is a cast to a constructor conversion, check the operand.
> Otherwise, the result of the cast is unused.

But this is a bit misleading: given `Foo(2+2)` the "operand" isn't 2+2, but 
rather a CXXConstructExpr.  And isUnusedResultAWarning returns true for this 
case (usually).

Back to the case at hand: we diverge because the cast type is Dependent instead 
of ConstructorConversion.
This seems like it could be a bug - in this configuration the operand is not 
type-dependent so why can't it resolve the overload?
But in any case, I don't think isUnusedResultAWarning should be assuming a 
dependent cast is safe to warn on - rather either return false or delegate to 
the inner expr (which in this case is a RecoveryExpr).



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85716

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

[PATCH] D85721: [clangd] Unify macro matching in code completion for AST and Index based macros

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 284694.
kadircet added a comment.

- Drop the comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85721

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -194,9 +194,14 @@
   EXPECT_THAT(completions(Body + "int main() { S().Foba^ }").Completions,
   AllOf(Has("FooBar"), Has("FooBaz"), Not(Has("Qux";
 
-  // Macros require  prefix match.
-  EXPECT_THAT(completions(Body + "int main() { C^ }").Completions,
-  AllOf(Has("Car"), Not(Has("MotorCar";
+  // Macros require prefix match, either from index or AST.
+  Symbol Sym = var("MotorCarIndex");
+  Sym.SymInfo.Kind = index::SymbolKind::Macro;
+  EXPECT_THAT(
+  completions(Body + "int main() { C^ }", {Sym}).Completions,
+  AllOf(Has("Car"), Not(Has("MotorCar")), Not(Has("MotorCarIndex";
+  EXPECT_THAT(completions(Body + "int main() { M^ }", {Sym}).Completions,
+  AllOf(Has("MotorCar"), Has("MotorCarIndex")));
 }
 
 void testAfterDotCompletion(clangd::CodeCompleteOptions Opts) {
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -1617,8 +1617,10 @@
 
   llvm::Optional fuzzyScore(const CompletionCandidate &C) {
 // Macros can be very spammy, so we only support prefix completion.
-// We won't end up with underfull index results, as macros are sema-only.
-if (C.SemaResult && C.SemaResult->Kind == CodeCompletionResult::RK_Macro &&
+if (((C.SemaResult &&
+  C.SemaResult->Kind == CodeCompletionResult::RK_Macro) ||
+ (C.IndexResult &&
+  C.IndexResult->SymInfo.Kind == index::SymbolKind::Macro)) &&
 !C.Name.startswith_lower(Filter->pattern()))
   return None;
 return Filter->match(C.Name);


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -194,9 +194,14 @@
   EXPECT_THAT(completions(Body + "int main() { S().Foba^ }").Completions,
   AllOf(Has("FooBar"), Has("FooBaz"), Not(Has("Qux";
 
-  // Macros require  prefix match.
-  EXPECT_THAT(completions(Body + "int main() { C^ }").Completions,
-  AllOf(Has("Car"), Not(Has("MotorCar";
+  // Macros require prefix match, either from index or AST.
+  Symbol Sym = var("MotorCarIndex");
+  Sym.SymInfo.Kind = index::SymbolKind::Macro;
+  EXPECT_THAT(
+  completions(Body + "int main() { C^ }", {Sym}).Completions,
+  AllOf(Has("Car"), Not(Has("MotorCar")), Not(Has("MotorCarIndex";
+  EXPECT_THAT(completions(Body + "int main() { M^ }", {Sym}).Completions,
+  AllOf(Has("MotorCar"), Has("MotorCarIndex")));
 }
 
 void testAfterDotCompletion(clangd::CodeCompleteOptions Opts) {
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -1617,8 +1617,10 @@
 
   llvm::Optional fuzzyScore(const CompletionCandidate &C) {
 // Macros can be very spammy, so we only support prefix completion.
-// We won't end up with underfull index results, as macros are sema-only.
-if (C.SemaResult && C.SemaResult->Kind == CodeCompletionResult::RK_Macro &&
+if (((C.SemaResult &&
+  C.SemaResult->Kind == CodeCompletionResult::RK_Macro) ||
+ (C.IndexResult &&
+  C.IndexResult->SymInfo.Kind == index::SymbolKind::Macro)) &&
 !C.Name.startswith_lower(Filter->pattern()))
   return None;
 return Filter->match(C.Name);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 24a816c - [clangd] Disable ExtractFunction for C

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-08-11T16:00:39+02:00
New Revision: 24a816c7d3925eb21970ea733d7d6bda11088ac7

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

LOG: [clangd] Disable ExtractFunction for C

This tweak uses constructs like auto and refs, which are not available
in C.

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

Added: 


Modified: 
clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
clang-tools-extra/clangd/unittests/TweakTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp 
b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
index 895afbb116f1..d4c723e02eeb 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -673,6 +673,8 @@ bool ExtractFunction::prepare(const Selection &Inputs) {
   const Node *CommonAnc = Inputs.ASTSelection.commonAncestor();
   const SourceManager &SM = Inputs.AST->getSourceManager();
   const LangOptions &LangOpts = Inputs.AST->getLangOpts();
+  if (!LangOpts.CPlusPlus)
+return false;
   if (auto MaybeExtZone = findExtractionZone(CommonAnc, SM, LangOpts)) {
 ExtZone = std::move(*MaybeExtZone);
 return true;

diff  --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp 
b/clang-tools-extra/clangd/unittests/TweakTests.cpp
index 200a53c690b6..b4f135b3efe2 100644
--- a/clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -605,6 +605,9 @@ TEST_F(ExtractFunctionTest, FunctionTest) {
   EXPECT_THAT(apply(" if(true) [[{ return; }]] "), HasSubstr("extracted"));
   // Don't extract uncertain return
   EXPECT_THAT(apply(" if(true) [[if (false) return;]] "), StartsWith("fail"));
+
+  FileName = "a.c";
+  EXPECT_THAT(apply(" for([[int i = 0;]];);"), HasSubstr("unavailable"));
 }
 
 TEST_F(ExtractFunctionTest, FileTest) {



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

[PATCH] D85727: [clangd] Disable ExtractFunction for C

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG24a816c7d392: [clangd] Disable ExtractFunction for C 
(authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85727

Files:
  clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -605,6 +605,9 @@
   EXPECT_THAT(apply(" if(true) [[{ return; }]] "), HasSubstr("extracted"));
   // Don't extract uncertain return
   EXPECT_THAT(apply(" if(true) [[if (false) return;]] "), StartsWith("fail"));
+
+  FileName = "a.c";
+  EXPECT_THAT(apply(" for([[int i = 0;]];);"), HasSubstr("unavailable"));
 }
 
 TEST_F(ExtractFunctionTest, FileTest) {
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -673,6 +673,8 @@
   const Node *CommonAnc = Inputs.ASTSelection.commonAncestor();
   const SourceManager &SM = Inputs.AST->getSourceManager();
   const LangOptions &LangOpts = Inputs.AST->getLangOpts();
+  if (!LangOpts.CPlusPlus)
+return false;
   if (auto MaybeExtZone = findExtractionZone(CommonAnc, SM, LangOpts)) {
 ExtZone = std::move(*MaybeExtZone);
 return true;


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -605,6 +605,9 @@
   EXPECT_THAT(apply(" if(true) [[{ return; }]] "), HasSubstr("extracted"));
   // Don't extract uncertain return
   EXPECT_THAT(apply(" if(true) [[if (false) return;]] "), StartsWith("fail"));
+
+  FileName = "a.c";
+  EXPECT_THAT(apply(" for([[int i = 0;]];);"), HasSubstr("unavailable"));
 }
 
 TEST_F(ExtractFunctionTest, FileTest) {
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -673,6 +673,8 @@
   const Node *CommonAnc = Inputs.ASTSelection.commonAncestor();
   const SourceManager &SM = Inputs.AST->getSourceManager();
   const LangOptions &LangOpts = Inputs.AST->getLangOpts();
+  if (!LangOpts.CPlusPlus)
+return false;
   if (auto MaybeExtZone = findExtractionZone(CommonAnc, SM, LangOpts)) {
 ExtZone = std::move(*MaybeExtZone);
 return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] b626f45 - [clangd] Unify macro matching in code completion for AST and Index based macros

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-08-11T16:00:39+02:00
New Revision: b626f45329e8575f72e6cd444b444942d4913e9c

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

LOG: [clangd] Unify macro matching in code completion for AST and Index based 
macros

fixes https://github.com/clangd/clangd/issues/489

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

Added: 


Modified: 
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/CodeComplete.cpp 
b/clang-tools-extra/clangd/CodeComplete.cpp
index f79033ac9514..92ebc4c39f64 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -1617,8 +1617,10 @@ class CodeCompleteFlow {
 
   llvm::Optional fuzzyScore(const CompletionCandidate &C) {
 // Macros can be very spammy, so we only support prefix completion.
-// We won't end up with underfull index results, as macros are sema-only.
-if (C.SemaResult && C.SemaResult->Kind == CodeCompletionResult::RK_Macro &&
+if (((C.SemaResult &&
+  C.SemaResult->Kind == CodeCompletionResult::RK_Macro) ||
+ (C.IndexResult &&
+  C.IndexResult->SymInfo.Kind == index::SymbolKind::Macro)) &&
 !C.Name.startswith_lower(Filter->pattern()))
   return None;
 return Filter->match(C.Name);

diff  --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp 
b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index 31a1c2d46c09..fd144d9391d3 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -194,9 +194,14 @@ TEST(CompletionTest, Filter) {
   EXPECT_THAT(completions(Body + "int main() { S().Foba^ }").Completions,
   AllOf(Has("FooBar"), Has("FooBaz"), Not(Has("Qux";
 
-  // Macros require  prefix match.
-  EXPECT_THAT(completions(Body + "int main() { C^ }").Completions,
-  AllOf(Has("Car"), Not(Has("MotorCar";
+  // Macros require prefix match, either from index or AST.
+  Symbol Sym = var("MotorCarIndex");
+  Sym.SymInfo.Kind = index::SymbolKind::Macro;
+  EXPECT_THAT(
+  completions(Body + "int main() { C^ }", {Sym}).Completions,
+  AllOf(Has("Car"), Not(Has("MotorCar")), Not(Has("MotorCarIndex";
+  EXPECT_THAT(completions(Body + "int main() { M^ }", {Sym}).Completions,
+  AllOf(Has("MotorCar"), Has("MotorCarIndex")));
 }
 
 void testAfterDotCompletion(clangd::CodeCompleteOptions Opts) {



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

[PATCH] D85721: [clangd] Unify macro matching in code completion for AST and Index based macros

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb626f45329e8: [clangd] Unify macro matching in code 
completion for AST and Index based macros (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85721

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -194,9 +194,14 @@
   EXPECT_THAT(completions(Body + "int main() { S().Foba^ }").Completions,
   AllOf(Has("FooBar"), Has("FooBaz"), Not(Has("Qux";
 
-  // Macros require  prefix match.
-  EXPECT_THAT(completions(Body + "int main() { C^ }").Completions,
-  AllOf(Has("Car"), Not(Has("MotorCar";
+  // Macros require prefix match, either from index or AST.
+  Symbol Sym = var("MotorCarIndex");
+  Sym.SymInfo.Kind = index::SymbolKind::Macro;
+  EXPECT_THAT(
+  completions(Body + "int main() { C^ }", {Sym}).Completions,
+  AllOf(Has("Car"), Not(Has("MotorCar")), Not(Has("MotorCarIndex";
+  EXPECT_THAT(completions(Body + "int main() { M^ }", {Sym}).Completions,
+  AllOf(Has("MotorCar"), Has("MotorCarIndex")));
 }
 
 void testAfterDotCompletion(clangd::CodeCompleteOptions Opts) {
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -1617,8 +1617,10 @@
 
   llvm::Optional fuzzyScore(const CompletionCandidate &C) {
 // Macros can be very spammy, so we only support prefix completion.
-// We won't end up with underfull index results, as macros are sema-only.
-if (C.SemaResult && C.SemaResult->Kind == CodeCompletionResult::RK_Macro &&
+if (((C.SemaResult &&
+  C.SemaResult->Kind == CodeCompletionResult::RK_Macro) ||
+ (C.IndexResult &&
+  C.IndexResult->SymInfo.Kind == index::SymbolKind::Macro)) &&
 !C.Name.startswith_lower(Filter->pattern()))
   return None;
 return Filter->match(C.Name);


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -194,9 +194,14 @@
   EXPECT_THAT(completions(Body + "int main() { S().Foba^ }").Completions,
   AllOf(Has("FooBar"), Has("FooBaz"), Not(Has("Qux";
 
-  // Macros require  prefix match.
-  EXPECT_THAT(completions(Body + "int main() { C^ }").Completions,
-  AllOf(Has("Car"), Not(Has("MotorCar";
+  // Macros require prefix match, either from index or AST.
+  Symbol Sym = var("MotorCarIndex");
+  Sym.SymInfo.Kind = index::SymbolKind::Macro;
+  EXPECT_THAT(
+  completions(Body + "int main() { C^ }", {Sym}).Completions,
+  AllOf(Has("Car"), Not(Has("MotorCar")), Not(Has("MotorCarIndex";
+  EXPECT_THAT(completions(Body + "int main() { M^ }", {Sym}).Completions,
+  AllOf(Has("MotorCar"), Has("MotorCarIndex")));
 }
 
 void testAfterDotCompletion(clangd::CodeCompleteOptions Opts) {
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -1617,8 +1617,10 @@
 
   llvm::Optional fuzzyScore(const CompletionCandidate &C) {
 // Macros can be very spammy, so we only support prefix completion.
-// We won't end up with underfull index results, as macros are sema-only.
-if (C.SemaResult && C.SemaResult->Kind == CodeCompletionResult::RK_Macro &&
+if (((C.SemaResult &&
+  C.SemaResult->Kind == CodeCompletionResult::RK_Macro) ||
+ (C.IndexResult &&
+  C.IndexResult->SymInfo.Kind == index::SymbolKind::Macro)) &&
 !C.Name.startswith_lower(Filter->pattern()))
   return None;
 return Filter->match(C.Name);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85590: [clang][HeaderInsert] Do not treat defines with values as header guards

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 284702.
kadircet added a comment.

- Move new test closer to other FakeHeaderGuard tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85590

Files:
  clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
  clang/unittests/Tooling/HeaderIncludesTest.cpp


Index: clang/unittests/Tooling/HeaderIncludesTest.cpp
===
--- clang/unittests/Tooling/HeaderIncludesTest.cpp
+++ clang/unittests/Tooling/HeaderIncludesTest.cpp
@@ -347,6 +347,18 @@
   EXPECT_EQ(Expected, insert(Code, ""));
 }
 
+TEST_F(HeaderIncludesTest, FakeHeaderGuardIfnDef) {
+  std::string Code = "#ifndef A_H\n"
+ "#define A_H 1\n"
+ "#endif";
+  std::string Expected = "#include \"b.h\"\n"
+ "#ifndef A_H\n"
+ "#define A_H 1\n"
+ "#endif";
+
+  EXPECT_EQ(Expected, insert(Code, "\"b.h\""));
+}
+
 TEST_F(HeaderIncludesTest, HeaderGuardWithComment) {
   std::string Code = "// comment \n"
  "#ifndef X // comment\n"
Index: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
===
--- clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
+++ clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
@@ -100,7 +100,8 @@
   [](const SourceManager &SM, Lexer &Lex, Token Tok) -> unsigned {
 if (checkAndConsumeDirectiveWithName(Lex, "ifndef", Tok)) {
   skipComments(Lex, Tok);
-  if (checkAndConsumeDirectiveWithName(Lex, "define", Tok))
+  if (checkAndConsumeDirectiveWithName(Lex, "define", Tok) &&
+  Tok.isAtStartOfLine())
 return SM.getFileOffset(Tok.getLocation());
 }
 return 0;


Index: clang/unittests/Tooling/HeaderIncludesTest.cpp
===
--- clang/unittests/Tooling/HeaderIncludesTest.cpp
+++ clang/unittests/Tooling/HeaderIncludesTest.cpp
@@ -347,6 +347,18 @@
   EXPECT_EQ(Expected, insert(Code, ""));
 }
 
+TEST_F(HeaderIncludesTest, FakeHeaderGuardIfnDef) {
+  std::string Code = "#ifndef A_H\n"
+ "#define A_H 1\n"
+ "#endif";
+  std::string Expected = "#include \"b.h\"\n"
+ "#ifndef A_H\n"
+ "#define A_H 1\n"
+ "#endif";
+
+  EXPECT_EQ(Expected, insert(Code, "\"b.h\""));
+}
+
 TEST_F(HeaderIncludesTest, HeaderGuardWithComment) {
   std::string Code = "// comment \n"
  "#ifndef X // comment\n"
Index: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
===
--- clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
+++ clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
@@ -100,7 +100,8 @@
   [](const SourceManager &SM, Lexer &Lex, Token Tok) -> unsigned {
 if (checkAndConsumeDirectiveWithName(Lex, "ifndef", Tok)) {
   skipComments(Lex, Tok);
-  if (checkAndConsumeDirectiveWithName(Lex, "define", Tok))
+  if (checkAndConsumeDirectiveWithName(Lex, "define", Tok) &&
+  Tok.isAtStartOfLine())
 return SM.getFileOffset(Tok.getLocation());
 }
 return 0;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85731: [builtins] Make softfloat-related errors less noisy

[Anatoly Trosinenko via Phabricator via cfe-commits]

atrosinenko created this revision.
atrosinenko added reviewers: asl, scanon, howard.hinnant.
Herald added a project: Sanitizers.
Herald added a subscriber: Sanitizers.
atrosinenko requested review of this revision.

Choose SINGLE_PRECISION by default in fp_lib.h after `#error`ing when
incorrectly configured. This eliminates cluttering log with lots of
"unknown type name 'fp_t'" and so on after the descriptive error was
manually generated.

This makes clang-tidy output more relevant for `fp_*_impl.inc` while
checking macroses even stricter (checking that *exactly* one is
defined instead of *at least* one).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85731

Files:
  compiler-rt/lib/builtins/fp_lib.h


Index: compiler-rt/lib/builtins/fp_lib.h
===
--- compiler-rt/lib/builtins/fp_lib.h
+++ compiler-rt/lib/builtins/fp_lib.h
@@ -26,6 +26,15 @@
 #include 
 #include 
 
+#if (defined(SINGLE_PRECISION) + defined(DOUBLE_PRECISION) +   
\
+ defined(QUAD_PRECISION)) != 1
+#error Exactly one of SINGLE_PRECISION, DOUBLE_PRECISION or QUAD_PRECISION 
must be defined.
+// Now, turn on some arbitrary one of the three precision settings (except for
+// QUAD_PRECISION that has some specific requirements), so fp_*_impl.inc files
+// could be validated by clang-tidy in a more meaningful way.
+#define SINGLE_PRECISION
+#endif
+
 // x86_64 FreeBSD prior v9.3 define fixed-width types incorrectly in
 // 32-bit mode.
 #if defined(__FreeBSD__) && defined(__i386__)
@@ -195,6 +204,8 @@
 #undef Word_FullMask
 #endif // __LDBL_MANT_DIG__ == 113 && __SIZEOF_INT128__
 #else
+// Should never be reached due to the check for precison-related macro
+// at the top of this file. Recheck it here just in case...
 #error SINGLE_PRECISION, DOUBLE_PRECISION or QUAD_PRECISION must be defined.
 #endif
 


Index: compiler-rt/lib/builtins/fp_lib.h
===
--- compiler-rt/lib/builtins/fp_lib.h
+++ compiler-rt/lib/builtins/fp_lib.h
@@ -26,6 +26,15 @@
 #include 
 #include 
 
+#if (defined(SINGLE_PRECISION) + defined(DOUBLE_PRECISION) +   \
+ defined(QUAD_PRECISION)) != 1
+#error Exactly one of SINGLE_PRECISION, DOUBLE_PRECISION or QUAD_PRECISION must be defined.
+// Now, turn on some arbitrary one of the three precision settings (except for
+// QUAD_PRECISION that has some specific requirements), so fp_*_impl.inc files
+// could be validated by clang-tidy in a more meaningful way.
+#define SINGLE_PRECISION
+#endif
+
 // x86_64 FreeBSD prior v9.3 define fixed-width types incorrectly in
 // 32-bit mode.
 #if defined(__FreeBSD__) && defined(__i386__)
@@ -195,6 +204,8 @@
 #undef Word_FullMask
 #endif // __LDBL_MANT_DIG__ == 113 && __SIZEOF_INT128__
 #else
+// Should never be reached due to the check for precison-related macro
+// at the top of this file. Recheck it here just in case...
 #error SINGLE_PRECISION, DOUBLE_PRECISION or QUAD_PRECISION must be defined.
 #endif
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85607: CfgTraits: add CfgInstructionRef

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: llvm/include/llvm/CodeGen/MachineCfgTraits.h:100
+// provide one at all. We don't want to lay a subtle performance trap here.
+abort();
+  }

llvm_unreachable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85607

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

[PATCH] D85590: [clang][HeaderInsert] Do not treat defines with values as header guards

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGea8e71c3da56: [clang][HeaderInsert] Do not treat defines 
with values as header guards (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85590

Files:
  clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
  clang/unittests/Tooling/HeaderIncludesTest.cpp


Index: clang/unittests/Tooling/HeaderIncludesTest.cpp
===
--- clang/unittests/Tooling/HeaderIncludesTest.cpp
+++ clang/unittests/Tooling/HeaderIncludesTest.cpp
@@ -347,6 +347,18 @@
   EXPECT_EQ(Expected, insert(Code, ""));
 }
 
+TEST_F(HeaderIncludesTest, FakeHeaderGuardIfnDef) {
+  std::string Code = "#ifndef A_H\n"
+ "#define A_H 1\n"
+ "#endif";
+  std::string Expected = "#include \"b.h\"\n"
+ "#ifndef A_H\n"
+ "#define A_H 1\n"
+ "#endif";
+
+  EXPECT_EQ(Expected, insert(Code, "\"b.h\""));
+}
+
 TEST_F(HeaderIncludesTest, HeaderGuardWithComment) {
   std::string Code = "// comment \n"
  "#ifndef X // comment\n"
Index: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
===
--- clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
+++ clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
@@ -100,7 +100,8 @@
   [](const SourceManager &SM, Lexer &Lex, Token Tok) -> unsigned {
 if (checkAndConsumeDirectiveWithName(Lex, "ifndef", Tok)) {
   skipComments(Lex, Tok);
-  if (checkAndConsumeDirectiveWithName(Lex, "define", Tok))
+  if (checkAndConsumeDirectiveWithName(Lex, "define", Tok) &&
+  Tok.isAtStartOfLine())
 return SM.getFileOffset(Tok.getLocation());
 }
 return 0;


Index: clang/unittests/Tooling/HeaderIncludesTest.cpp
===
--- clang/unittests/Tooling/HeaderIncludesTest.cpp
+++ clang/unittests/Tooling/HeaderIncludesTest.cpp
@@ -347,6 +347,18 @@
   EXPECT_EQ(Expected, insert(Code, ""));
 }
 
+TEST_F(HeaderIncludesTest, FakeHeaderGuardIfnDef) {
+  std::string Code = "#ifndef A_H\n"
+ "#define A_H 1\n"
+ "#endif";
+  std::string Expected = "#include \"b.h\"\n"
+ "#ifndef A_H\n"
+ "#define A_H 1\n"
+ "#endif";
+
+  EXPECT_EQ(Expected, insert(Code, "\"b.h\""));
+}
+
 TEST_F(HeaderIncludesTest, HeaderGuardWithComment) {
   std::string Code = "// comment \n"
  "#ifndef X // comment\n"
Index: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
===
--- clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
+++ clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
@@ -100,7 +100,8 @@
   [](const SourceManager &SM, Lexer &Lex, Token Tok) -> unsigned {
 if (checkAndConsumeDirectiveWithName(Lex, "ifndef", Tok)) {
   skipComments(Lex, Tok);
-  if (checkAndConsumeDirectiveWithName(Lex, "define", Tok))
+  if (checkAndConsumeDirectiveWithName(Lex, "define", Tok) &&
+  Tok.isAtStartOfLine())
 return SM.getFileOffset(Tok.getLocation());
 }
 return 0;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] ea8e71c - [clang][HeaderInsert] Do not treat defines with values as header guards

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-08-11T16:02:11+02:00
New Revision: ea8e71c3da56f76ae2ab2f9ee7afc608ffab

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

LOG: [clang][HeaderInsert] Do not treat defines with values as header guards

This was resulting in inserting headers at bogus locations, see
https://github.com/ycm-core/YouCompleteMe/issues/3736 for an example.

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

Added: 


Modified: 
clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
clang/unittests/Tooling/HeaderIncludesTest.cpp

Removed: 




diff  --git a/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp 
b/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
index 681fcc5c762a..b65d7f0c1a39 100644
--- a/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
+++ b/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
@@ -100,7 +100,8 @@ unsigned getOffsetAfterHeaderGuardsAndComments(StringRef 
FileName,
   [](const SourceManager &SM, Lexer &Lex, Token Tok) -> unsigned {
 if (checkAndConsumeDirectiveWithName(Lex, "ifndef", Tok)) {
   skipComments(Lex, Tok);
-  if (checkAndConsumeDirectiveWithName(Lex, "define", Tok))
+  if (checkAndConsumeDirectiveWithName(Lex, "define", Tok) &&
+  Tok.isAtStartOfLine())
 return SM.getFileOffset(Tok.getLocation());
 }
 return 0;

diff  --git a/clang/unittests/Tooling/HeaderIncludesTest.cpp 
b/clang/unittests/Tooling/HeaderIncludesTest.cpp
index 9f86d2e913a8..d38104fe40ec 100644
--- a/clang/unittests/Tooling/HeaderIncludesTest.cpp
+++ b/clang/unittests/Tooling/HeaderIncludesTest.cpp
@@ -347,6 +347,18 @@ TEST_F(HeaderIncludesTest, FakeHeaderGuard) {
   EXPECT_EQ(Expected, insert(Code, ""));
 }
 
+TEST_F(HeaderIncludesTest, FakeHeaderGuardIfnDef) {
+  std::string Code = "#ifndef A_H\n"
+ "#define A_H 1\n"
+ "#endif";
+  std::string Expected = "#include \"b.h\"\n"
+ "#ifndef A_H\n"
+ "#define A_H 1\n"
+ "#endif";
+
+  EXPECT_EQ(Expected, insert(Code, "\"b.h\""));
+}
+
 TEST_F(HeaderIncludesTest, HeaderGuardWithComment) {
   std::string Code = "// comment \n"
  "#ifndef X // comment\n"



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

[PATCH] D83536: [clangd] Index refs to main-file symbols as well

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

thanks, lgtm. mostly comments arounds tests, sorry for not looking at them 
before.




Comment at: clang-tools-extra/clangd/index/FileIndex.h:156
 /// Exposed to assist in unit tests.
-SlabTuple indexMainDecls(ParsedAST &AST);
+SlabTuple indexMainDecls(ParsedAST &AST, bool CollectMainFileRefs = true);
 

again default to false.



Comment at: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp:194
+  /*ContextProvider=*/nullptr,
+  /*CollectMainFileRefs=*/true);
 

this requires it is own separate test, as this is not only testing for indexing 
two files case now.

I would suggest a minimal test with one header and one cc ref, running 
background index twice to make sure both configurations work as expected.



Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:628
   HaveRanges(Main.ranges("macro");
-  // Symbols *only* in the main file:
-  // - (a, b) externally visible and should have refs.
-  // - (c, FUNC) externally invisible and had no refs collected.
-  auto MainSymbols =
-  TestTU::withHeaderCode(SymbolsOnlyInMainCode.code()).headerSymbols();
-  EXPECT_THAT(Refs, Contains(Pair(findSymbol(MainSymbols, "a").ID, _)));
-  EXPECT_THAT(Refs, Contains(Pair(findSymbol(MainSymbols, "b").ID, _)));
-  EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "c").ID, _;
-  EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "FUNC").ID, 
_;
+  // Symbols *only* in the main file (a, b, c) should have refs collected
+  // as well.

could you instead run twice ? once with mainfilerefs = false, and once with it 
set to true?



Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:710
   } TestCases[] = {
-{
-  R"cpp(
+  {
+  R"cpp(

these seem to be some unrelated formatting changes, please revert before 
landing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83536

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

[PATCH] D84828: [clang] Don't make synthesized accessor stub functions visible twice

[Raphael Isemann via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG442a80292d50: [clang] Don't make synthesized accessor 
stub functions visible twice (authored by teemperor).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84828

Files:
  clang/lib/Sema/SemaDeclObjC.cpp


Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -3922,15 +3922,11 @@
   if (auto *OID = dyn_cast(CurContext)) {
 for (auto PropImpl : OID->property_impls()) {
   if (auto *Getter = PropImpl->getGetterMethodDecl())
-if (Getter->isSynthesizedAccessorStub()) {
-  OID->makeDeclVisibleInContext(Getter);
+if (Getter->isSynthesizedAccessorStub())
   OID->addDecl(Getter);
-}
   if (auto *Setter = PropImpl->getSetterMethodDecl())
-if (Setter->isSynthesizedAccessorStub()) {
-  OID->makeDeclVisibleInContext(Setter);
+if (Setter->isSynthesizedAccessorStub())
   OID->addDecl(Setter);
-}
 }
   }
 


Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -3922,15 +3922,11 @@
   if (auto *OID = dyn_cast(CurContext)) {
 for (auto PropImpl : OID->property_impls()) {
   if (auto *Getter = PropImpl->getGetterMethodDecl())
-if (Getter->isSynthesizedAccessorStub()) {
-  OID->makeDeclVisibleInContext(Getter);
+if (Getter->isSynthesizedAccessorStub())
   OID->addDecl(Getter);
-}
   if (auto *Setter = PropImpl->getSetterMethodDecl())
-if (Setter->isSynthesizedAccessorStub()) {
-  OID->makeDeclVisibleInContext(Setter);
+if (Setter->isSynthesizedAccessorStub())
   OID->addDecl(Setter);
-}
 }
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 442a802 - [clang] Don't make synthesized accessor stub functions visible twice

[Raphael Isemann via cfe-commits]

Author: Raphael Isemann
Date: 2020-08-11T16:23:51+02:00
New Revision: 442a80292d50d895396eb14418bd471e7da68fd0

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

LOG: [clang] Don't make synthesized accessor stub functions visible twice

`addDecl` is making the Decl visible, so there is no need to make it explicitly
visible again. Making it visible twice will also make the lookup storage less
efficient and potentially lead to crashes, see D84827 for that.

Reviewed By: aprantl

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

Added: 


Modified: 
clang/lib/Sema/SemaDeclObjC.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDeclObjC.cpp b/clang/lib/Sema/SemaDeclObjC.cpp
index d376880a40e8..89815b838500 100644
--- a/clang/lib/Sema/SemaDeclObjC.cpp
+++ b/clang/lib/Sema/SemaDeclObjC.cpp
@@ -3922,15 +3922,11 @@ Decl *Sema::ActOnAtEnd(Scope *S, SourceRange AtEnd, 
ArrayRef allMethods,
   if (auto *OID = dyn_cast(CurContext)) {
 for (auto PropImpl : OID->property_impls()) {
   if (auto *Getter = PropImpl->getGetterMethodDecl())
-if (Getter->isSynthesizedAccessorStub()) {
-  OID->makeDeclVisibleInContext(Getter);
+if (Getter->isSynthesizedAccessorStub())
   OID->addDecl(Getter);
-}
   if (auto *Setter = PropImpl->getSetterMethodDecl())
-if (Setter->isSynthesizedAccessorStub()) {
-  OID->makeDeclVisibleInContext(Setter);
+if (Setter->isSynthesizedAccessorStub())
   OID->addDecl(Setter);
-}
 }
   }
 



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

[clang] 02899d7 - [clang] Don't make ObjCIvarDecl visible twice when adding them to an implicit ObjCInterfaceDecl

[Raphael Isemann via cfe-commits]

Author: Raphael Isemann
Date: 2020-08-11T16:24:32+02:00
New Revision: 02899d7f1b9ae7f6da30bd020a714c7b3eb2c59f

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

LOG: [clang] Don't make ObjCIvarDecl visible twice when adding them to an 
implicit ObjCInterfaceDecl

`addDecl` is making the ivar visible in its primary context. The primary context
of the ivar here is in a 'fragile' ABI the ObjCInterfaceDecl and in a
'non-fragile' ABI the current ObjCImplementationDecl. The additional call to
`makeDeclVisibleInContext` to make the ivar visible in the ObjCInterfaceDecl is
only necessary in the 'non-fragile' case (as in the 'fragile' case the Decl
becomes automatically visible in the ObjCInterfaceDecl with the `addDecl` call
as thats its primary context). See `Sema::ActOnIvar` for where the ivar is put
into a different context depending on the ABI.

To put this into an example:

```
lang=c++
@implementation SomeClass
{
  id ivar1;
}
@end

fragile case:
implicit ObjCInterfaceDecl 'SomeClass'
`- ivar1 (in primary context and will be automatically made visible)
ObjCImplementationDecl 'SomeClass'

non-fragile case:
implicit ObjCInterfaceDecl 'SomeClass'
`-<<>>
ObjCImplementationDecl 'SomeClass'
`- ivar1 (in its primary context and will be automatically made visible here)
```

Making a Decl visible multiple times in the same context is inefficient and
potentially can lead to crashes. See D84827 for more info and what this is
breaking.

Reviewed By: aprantl

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

Added: 


Modified: 
clang/lib/Sema/SemaDeclObjC.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDeclObjC.cpp b/clang/lib/Sema/SemaDeclObjC.cpp
index 89815b838500..6ef6fd1d8c1c 100644
--- a/clang/lib/Sema/SemaDeclObjC.cpp
+++ b/clang/lib/Sema/SemaDeclObjC.cpp
@@ -2122,7 +2122,12 @@ void 
Sema::CheckImplementationIvars(ObjCImplementationDecl *ImpDecl,
 // Add ivar's to class's DeclContext.
 for (unsigned i = 0, e = numIvars; i != e; ++i) {
   ivars[i]->setLexicalDeclContext(ImpDecl);
-  IDecl->makeDeclVisibleInContext(ivars[i]);
+  // In a 'fragile' runtime the ivar was added to the implicit
+  // ObjCInterfaceDecl while in a 'non-fragile' runtime the ivar is
+  // only in the ObjCImplementationDecl. In the non-fragile case the ivar
+  // therefore also needs to be propagated to the ObjCInterfaceDecl.
+  if (!LangOpts.ObjCRuntime.isFragile())
+IDecl->makeDeclVisibleInContext(ivars[i]);
   ImpDecl->addDecl(ivars[i]);
 }
 



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

[PATCH] D85733: [libTooling] Cleanup and reorder `RewriteRule.h`.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added a reviewer: gribozavr2.
Herald added a project: clang.
ymandel requested review of this revision.

This patch lifts `RootID` out of the `RewriteRule` class so that constructs
(e.g. inline functions) can that refer to the root id don't need to depend on
the `RewriteRule` class.

With this dependency, the patch is able to collect all `ASTEdit` helper function
declarations together with the class declaration, before the introduction of the
`RewriteRule` class. In the process, we also adjust some of the comments.

This patch is essentially a NFC.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85733

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

Index: clang/lib/Tooling/Transformer/RewriteRule.cpp
===
--- clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -30,6 +30,8 @@
 
 using MatchResult = MatchFinder::MatchResult;
 
+const char transformer::RootID[] = "___root___";
+
 static Expected>
 translateEdits(const MatchResult &Result, ArrayRef ASTEdits) {
   SmallVector Edits;
@@ -329,8 +331,7 @@
 taggedMatchers("Tag", Bucket.second, TK_IgnoreUnlessSpelledInSource));
 M.setAllowBind(true);
 // `tryBind` is guaranteed to succeed, because `AllowBind` was set to true.
-Matchers.push_back(
-M.tryBind(RewriteRule::RootID)->withTraversalKind(TK_AsIs));
+Matchers.push_back(M.tryBind(RootID)->withTraversalKind(TK_AsIs));
   }
   return Matchers;
 }
@@ -343,7 +344,7 @@
 
 SourceLocation transformer::detail::getRuleMatchLoc(const MatchResult &Result) {
   auto &NodesMap = Result.Nodes.getMap();
-  auto Root = NodesMap.find(RewriteRule::RootID);
+  auto Root = NodesMap.find(RootID);
   assert(Root != NodesMap.end() && "Transformation failed: missing root node.");
   llvm::Optional RootRange = tooling::getRangeForEdit(
   CharSourceRange::getTokenRange(Root->second.getSourceRange()),
@@ -373,7 +374,7 @@
   llvm_unreachable("No tag found for this rule.");
 }
 
-constexpr llvm::StringLiteral RewriteRule::RootID;
+const llvm::StringRef RewriteRule::RootID = ::clang::transformer::RootID;
 
 TextGenerator tooling::text(std::string M) {
   return std::make_shared(std::move(M));
Index: clang/include/clang/Tooling/Transformer/RewriteRule.h
===
--- clang/include/clang/Tooling/Transformer/RewriteRule.h
+++ clang/include/clang/Tooling/Transformer/RewriteRule.h
@@ -97,6 +97,9 @@
   };
 };
 
+/// Generates a single (specified) edit.
+EditGenerator edit(ASTEdit E);
+
 /// Lifts a list of `ASTEdit`s into an `EditGenerator`.
 ///
 /// The `EditGenerator` will return an empty vector if any of the edits apply to
@@ -107,21 +110,19 @@
 /// clients.  We recommend use of the \c AtomicChange or \c Replacements classes
 /// for assistance in detecting such conflicts.
 EditGenerator editList(llvm::SmallVector Edits);
-/// Convenience form of `editList` for a single edit.
-EditGenerator edit(ASTEdit);
 
-/// Convenience generator for a no-op edit generator.
+/// Generators no edits.
 inline EditGenerator noEdits() { return editList({}); }
 
-/// Convenience version of `ifBound` specialized to `ASTEdit`.
+/// Version of `ifBound` specialized to `ASTEdit`.
 inline EditGenerator ifBound(std::string ID, ASTEdit TrueEdit,
  ASTEdit FalseEdit) {
   return ifBound(std::move(ID), edit(std::move(TrueEdit)),
  edit(std::move(FalseEdit)));
 }
 
-/// Convenience version of `ifBound` that has no "False" branch. If the node is
-/// not bound, then no edits are produced.
+/// Version of `ifBound` that has no "False" branch. If the node is not bound,
+/// then no edits are produced.
 inline EditGenerator ifBound(std::string ID, ASTEdit TrueEdit) {
   return ifBound(std::move(ID), edit(std::move(TrueEdit)), noEdits());
 }
@@ -131,7 +132,7 @@
 EditGenerator flattenVector(SmallVector Generators);
 
 namespace detail {
-/// Convenience function to construct an \c EditGenerator. Overloaded for common
+/// Helper function to construct an \c EditGenerator. Overloaded for common
 /// cases so that user doesn't need to specify which factory function to
 /// use. This pattern gives benefits similar to implicit constructors, while
 /// maintaing a higher degree of explicitness.
@@ -143,6 +144,78 @@
   return flattenVector({detail::injectEdits(std::forward(Edits))...});
 }
 
+// Every rewrite rules is triggered by a match against some AST node.
+// Transformer guarantees that this ID is bound to the triggering node whenever
+// a rewrite rule is applied.
+extern const char RootID[];
+
+/// Replaces a portion of the source text with \p Replacement.
+ASTEdit changeTo(RangeSelector Target, TextGenerator Replacement);
+/// DEPRECATED: use \c changeTo.
+inline ASTEdit change(RangeSelector Target, TextGen

[PATCH] D84829: [clang] Don't make ObjCIvarDecl visible twice when adding them to an implicit ObjCInterfaceDecl

[Raphael Isemann via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG02899d7f1b9a: [clang] Don't make ObjCIvarDecl visible 
twice when adding them to an implicit… (authored by teemperor).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84829

Files:
  clang/lib/Sema/SemaDeclObjC.cpp


Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -2122,7 +2122,12 @@
 // Add ivar's to class's DeclContext.
 for (unsigned i = 0, e = numIvars; i != e; ++i) {
   ivars[i]->setLexicalDeclContext(ImpDecl);
-  IDecl->makeDeclVisibleInContext(ivars[i]);
+  // In a 'fragile' runtime the ivar was added to the implicit
+  // ObjCInterfaceDecl while in a 'non-fragile' runtime the ivar is
+  // only in the ObjCImplementationDecl. In the non-fragile case the ivar
+  // therefore also needs to be propagated to the ObjCInterfaceDecl.
+  if (!LangOpts.ObjCRuntime.isFragile())
+IDecl->makeDeclVisibleInContext(ivars[i]);
   ImpDecl->addDecl(ivars[i]);
 }
 


Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -2122,7 +2122,12 @@
 // Add ivar's to class's DeclContext.
 for (unsigned i = 0, e = numIvars; i != e; ++i) {
   ivars[i]->setLexicalDeclContext(ImpDecl);
-  IDecl->makeDeclVisibleInContext(ivars[i]);
+  // In a 'fragile' runtime the ivar was added to the implicit
+  // ObjCInterfaceDecl while in a 'non-fragile' runtime the ivar is
+  // only in the ObjCImplementationDecl. In the non-fragile case the ivar
+  // therefore also needs to be propagated to the ObjCInterfaceDecl.
+  if (!LangOpts.ObjCRuntime.isFragile())
+IDecl->makeDeclVisibleInContext(ivars[i]);
   ImpDecl->addDecl(ivars[i]);
 }
 
___
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.

[Szabolcs Nagy via Phabricator via cfe-commits]

nsz added a comment.

In D80791#2209703 , @chill wrote:

> In D80791#2209624 , @nsz wrote:
>
>> - it is not useful to have a bti annotated function unless everything else 
>> is bti compatible too: it is all or nothing per elf module.
>
> This is false. Some functions in an elf module could be in a guarded region, 
> some in a non-guarded region. Some function may always
> be called in a "BTI-safe" way, which may be unknown to the compiler.

the current design is per elf module, so the non-guarded things have to go into 
a different elf module (and thus different tu).

i think the only way the current abi supports mixing bti and non-bti code is if 
all the linker inputs to the elf module are marked as bti compatible and then 
the user explicitly unprotects some region at runtime, i.e. bti is still all or 
nothing per elf module, but a user might want to do some hack and turn bti off 
in some places.

> With my proposal to derive marking from function attributes, as well as from 
> command-line
> everything above will still work in the (arguably) most common case that we 
> expect - users just using
> command line.
>
> I'm proposing to be strict and cover a few corner case where the command-line 
> only approach produces bogus results.

ok i think deriving the marking in the absence of command-line option works, 
but it's not something users can rely on and not what gcc does.


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] D85635: [clangd] Compute the inactive code range for semantic highlighting.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 284715.
hokein marked 3 inline comments as done.
hokein added a comment.

address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85635

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -503,11 +503,11 @@
 
   #define $Macro[[test]]
   #undef $Macro[[test]]
-$InactiveCode[[]]  #ifdef $Macro[[test]]
-$InactiveCode[[]]  #endif
+$InactiveCode[[#ifdef test]]
+$InactiveCode[[#endif]]
 
-$InactiveCode[[]]  #if defined($Macro[[test]])
-$InactiveCode[[]]  #endif
+$InactiveCode[[#if defined(test)]]
+$InactiveCode[[#endif]]
 )cpp",
   R"cpp(
   struct $Class[[S]] {
@@ -614,8 +614,8 @@
   R"cpp(
   // Code in the preamble.
   // Inactive lines get an empty InactiveCode token at the beginning.
-$InactiveCode[[]]  #ifdef $Macro[[test]]
-$InactiveCode[[]]  #endif
+$InactiveCode[[#ifdef test]]
+$InactiveCode[[#endif]]
 
   // A declaration to cause the preamble to end.
   int $Variable[[EndPreamble]];
@@ -623,17 +623,17 @@
   // Code after the preamble.
   // Code inside inactive blocks does not get regular highlightings
   // because it's not part of the AST.
-$InactiveCode[[]]  #ifdef $Macro[[test]]
-$InactiveCode[[]]  int Inactive2;
-$InactiveCode[[]]  #endif
+$InactiveCode[[#ifdef test]]
+$InactiveCode[[int Inactive2;]]
+$InactiveCode[[#endif]]
 
   #ifndef $Macro[[test]]
   int $Variable[[Active1]];
   #endif
 
-$InactiveCode[[]]  #ifdef $Macro[[test]]
-$InactiveCode[[]]  int Inactive3;
-$InactiveCode[[]]  #else
+$InactiveCode[[#ifdef test]]
+$InactiveCode[[int Inactive3;]]
+$InactiveCode[[#else]]
   int $Variable[[Active2]];
   #endif
 )cpp",
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -221,23 +221,51 @@
   // the end of the Tokens).
   TokRef = TokRef.drop_front(Conflicting.size());
 }
+const auto &SM = AST.getSourceManager();
+StringRef MainCode = SM.getBuffer(SM.getMainFileID())->getBuffer();
 // Add tokens indicating lines skipped by the preprocessor.
 for (const Range &R : AST.getMacros().SkippedRanges) {
   // Create one token for each line in the skipped range, so it works
   // with line-based diffing.
   assert(R.start.line <= R.end.line);
   for (int Line = R.start.line; Line <= R.end.line; ++Line) {
-// Don't bother computing the offset for the end of the line, just use
-// zero. The client will treat this highlighting kind specially, and
-// highlight the entire line visually (i.e. not just to where the text
-// on the line ends, but to the end of the screen).
-NonConflicting.push_back({HighlightingKind::InactiveCode,
-  {Position{Line, 0}, Position{Line, 0}}});
+auto StartOfLine = positionToOffset(MainCode, Position{Line, 0});
+if (!StartOfLine) {
+  elog("Failed to convert position to offset: {0}",
+   StartOfLine.takeError());
+  continue;
+}
+StringRef LineText =
+MainCode.drop_front(*StartOfLine).take_until([](char C) {
+  return C == '\n';
+});
+NonConflicting.push_back(
+{HighlightingKind::InactiveCode,
+ {Position{Line, 0},
+  Position{Line, static_cast(lspLength(LineText))}}});
   }
 }
 // Re-sort the tokens because that's what the diffing expects.
 llvm::sort(NonConflicting);
-return NonConflicting;
+std::vector Results;
+// Remove tokens that are overlapped with an inactive region, e.g.
+// `#ifndef Foo` is considered as part of an inactive region when Foo is
+// defined, and there is a Foo macro token.
+// FIXME: we should reduce the scope of the inactive region to not include
+// the directive itself.
+for (ArrayRef TokRef = NonConflicting;
+ !TokRef.empty();) {
+  ArrayRef ContainedTokens =
+  TokRef.take_while([&](const HighlightingToken &T) {
+return TokRef.front().R.contains(T.R);
+  });
+  assert(ContainedTokens.size() == 1 ||
+ (ContainedTokens.size() > 1 &&
+  TokRef.front().Kind == HighlightingKind::InactiveCode));
+  Results.push_back(ContainedTokens.front());
+  TokRef = TokRef.drop_front(ContainedTokens.size()

[PATCH] D85733: [libTooling] Cleanup and reorder `RewriteRule.h`.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

fix typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85733

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

Index: clang/lib/Tooling/Transformer/RewriteRule.cpp
===
--- clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -30,6 +30,8 @@
 
 using MatchResult = MatchFinder::MatchResult;
 
+const char transformer::RootID[] = "___root___";
+
 static Expected>
 translateEdits(const MatchResult &Result, ArrayRef ASTEdits) {
   SmallVector Edits;
@@ -329,8 +331,7 @@
 taggedMatchers("Tag", Bucket.second, TK_IgnoreUnlessSpelledInSource));
 M.setAllowBind(true);
 // `tryBind` is guaranteed to succeed, because `AllowBind` was set to true.
-Matchers.push_back(
-M.tryBind(RewriteRule::RootID)->withTraversalKind(TK_AsIs));
+Matchers.push_back(M.tryBind(RootID)->withTraversalKind(TK_AsIs));
   }
   return Matchers;
 }
@@ -343,7 +344,7 @@
 
 SourceLocation transformer::detail::getRuleMatchLoc(const MatchResult &Result) {
   auto &NodesMap = Result.Nodes.getMap();
-  auto Root = NodesMap.find(RewriteRule::RootID);
+  auto Root = NodesMap.find(RootID);
   assert(Root != NodesMap.end() && "Transformation failed: missing root node.");
   llvm::Optional RootRange = tooling::getRangeForEdit(
   CharSourceRange::getTokenRange(Root->second.getSourceRange()),
@@ -373,7 +374,7 @@
   llvm_unreachable("No tag found for this rule.");
 }
 
-constexpr llvm::StringLiteral RewriteRule::RootID;
+const llvm::StringRef RewriteRule::RootID = ::clang::transformer::RootID;
 
 TextGenerator tooling::text(std::string M) {
   return std::make_shared(std::move(M));
Index: clang/include/clang/Tooling/Transformer/RewriteRule.h
===
--- clang/include/clang/Tooling/Transformer/RewriteRule.h
+++ clang/include/clang/Tooling/Transformer/RewriteRule.h
@@ -97,6 +97,9 @@
   };
 };
 
+/// Generates a single (specified) edit.
+EditGenerator edit(ASTEdit E);
+
 /// Lifts a list of `ASTEdit`s into an `EditGenerator`.
 ///
 /// The `EditGenerator` will return an empty vector if any of the edits apply to
@@ -107,21 +110,19 @@
 /// clients.  We recommend use of the \c AtomicChange or \c Replacements classes
 /// for assistance in detecting such conflicts.
 EditGenerator editList(llvm::SmallVector Edits);
-/// Convenience form of `editList` for a single edit.
-EditGenerator edit(ASTEdit);
 
-/// Convenience generator for a no-op edit generator.
+/// Generates no edits.
 inline EditGenerator noEdits() { return editList({}); }
 
-/// Convenience version of `ifBound` specialized to `ASTEdit`.
+/// Version of `ifBound` specialized to `ASTEdit`.
 inline EditGenerator ifBound(std::string ID, ASTEdit TrueEdit,
  ASTEdit FalseEdit) {
   return ifBound(std::move(ID), edit(std::move(TrueEdit)),
  edit(std::move(FalseEdit)));
 }
 
-/// Convenience version of `ifBound` that has no "False" branch. If the node is
-/// not bound, then no edits are produced.
+/// Version of `ifBound` that has no "False" branch. If the node is not bound,
+/// then no edits are produced.
 inline EditGenerator ifBound(std::string ID, ASTEdit TrueEdit) {
   return ifBound(std::move(ID), edit(std::move(TrueEdit)), noEdits());
 }
@@ -131,7 +132,7 @@
 EditGenerator flattenVector(SmallVector Generators);
 
 namespace detail {
-/// Convenience function to construct an \c EditGenerator. Overloaded for common
+/// Helper function to construct an \c EditGenerator. Overloaded for common
 /// cases so that user doesn't need to specify which factory function to
 /// use. This pattern gives benefits similar to implicit constructors, while
 /// maintaing a higher degree of explicitness.
@@ -143,6 +144,78 @@
   return flattenVector({detail::injectEdits(std::forward(Edits))...});
 }
 
+// Every rewrite rules is triggered by a match against some AST node.
+// Transformer guarantees that this ID is bound to the triggering node whenever
+// a rewrite rule is applied.
+extern const char RootID[];
+
+/// Replaces a portion of the source text with \p Replacement.
+ASTEdit changeTo(RangeSelector Target, TextGenerator Replacement);
+/// DEPRECATED: use \c changeTo.
+inline ASTEdit change(RangeSelector Target, TextGenerator Replacement) {
+  return changeTo(std::move(Target), std::move(Replacement));
+}
+
+/// Replaces the entirety of a RewriteRule's match with \p Replacement.  For
+/// example, to replace a function call, one could write:
+/// \code
+///   makeRule(callExpr(callee(functionDecl(hasName("foo",
+///changeTo(cat("bar()")))
+/// \endcode
+inline ASTEdit changeTo(TextGenerator Replacement) {
+  return changeTo(node(RootID), std::

[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno updated this revision to Diff 284718.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85613

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp

Index: clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
===
--- clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
+++ clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
@@ -1,5 +1,20 @@
 // RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s
 
+namespace std {
+using size_t = decltype(sizeof(int));
+template  struct tuple_size;
+template  struct tuple_element;
+} // namespace std
+using size_t = std::size_t;
+
+struct E {};
+template  int get(E);
+
+namespace std {
+template <> struct tuple_size { static constexpr size_t value = 2; };
+template  struct tuple_element { using type = int; };
+}; // namespace std
+
 void h() {
   int i1 = 0;
   extern void h1(int x = i1);
@@ -24,8 +39,19 @@
   extern void h6(int = i5);
   // expected-error@-1 {{default argument references local variable '' of enclosing function}}
 
-  struct S { int i; };
-  auto [x] = S();
+  struct S {
+int i, j;
+  };
+  auto [x, y] = S();
+
+  extern void h7(int = x); // expected-error {{default argument references local binding 'x'}}
+
+  auto [z, w] = E();
+  extern void h8a(int = sizeof(z)); // ok
+  extern void h8b(int = w); // expected-error {{default argument references local binding 'w'}}
 
-  extern void h7(int = x); // FIXME: reject
+  extern auto get_array()->int(&)[2];
+  auto [a0, a1] = get_array();
+  extern void h9a(int = sizeof(a0));
+  extern void h9b(int = a1); // expected-error {{default argument references local binding 'a1'}}
 }
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -86,6 +86,20 @@
 /// argument expression.
 bool CheckDefaultArgumentVisitor::VisitDeclRefExpr(const DeclRefExpr *DRE) {
   const NamedDecl *Decl = DRE->getDecl();
+
+  auto DiagnoseIfOdrUse = [&](const VarDecl *VD, unsigned DiagID,
+  const auto &... DiagArgs) -> bool {
+if (!DRE->isNonOdrUse()) {
+  auto DB = S.Diag(DRE->getBeginLoc(), DiagID);
+  int dummy[] = {0, (DB << DiagArgs, 0)...};
+  (void)dummy;
+  DB << DefaultArg->getSourceRange();
+  return true;
+}
+
+return false;
+  };
+
   if (const auto *Param = dyn_cast(Decl)) {
 // C++ [dcl.fct.default]p9:
 //   [...] parameters of a function shall not be used in default
@@ -111,10 +125,29 @@
 // C++20 [dcl.fct.default]p7 (DR as part of P0588R1, see also CWG 2346):
 //   Note: A local variable cannot be odr-used (6.3) in a default argument.
 //
-if (VDecl->isLocalVarDecl() && !DRE->isNonOdrUse())
-  return S.Diag(DRE->getBeginLoc(),
-diag::err_param_default_argument_references_local)
- << VDecl->getDeclName() << DefaultArg->getSourceRange();
+if (VDecl->isLocalVarDecl() &&
+DiagnoseIfOdrUse(VDecl,
+ diag::err_param_default_argument_references_local,
+ /*variable*/ 0, VDecl))
+  return true;
+
+  } else if (const auto *Binding = dyn_cast(Decl)) {
+// C++20 [basic.pre]p7:
+//   A local entity is [...] a structured binding whose corresponding
+//   variable is such an entity [...]
+//
+// C++20 [basic.def.odr]p9:
+//   A local entity (6.1) is odr-usable in a declarative region if [...]
+//
+// Note that this was not entirely clear in C++17 since [dcl.fct.default]p7
+// only prohibited local variables (a structured binding declaration
+// introduces identifiers as names).
+//
+const auto *VD = cast(Binding->getDecomposedDecl());
+if (VD->isLocalVarDecl() &&
+DiagnoseIfOdrUse(VD, diag::err_param_default_argument_references_local,
+ /*binding*/ 1, Binding))
+  return true;
   }
 
   return false;
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3994,7 +3994,8 @@
 def err_param_default_argument_references_param : Error<
   "default argument references parameter %0">;
 def err_param_default_argument_references_local : Error<
-  "default argument references local variable %0 of enclosing function">;
+  "default argument references local %select{variable|binding}0 %1 "
+  "of enclosing function">;
 def err_param_default_argument_references_this : Error<
   "default argument references 'this'">;
 def err_param_default_argument_nonfunc : Error<
___
cf

[PATCH] D85734: [libTooling] Move RewriteRule include edits to ASTEdit granularity.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added reviewers: tdl-g, gribozavr2.
Herald added a project: clang.
ymandel requested review of this revision.

Currently, changes to includes are applied to an entire rule. However,
include changes may be specific to particular edits within a rule (for example,
they may apply to one file but not another). Also, include changes may need to
carry metadata, just like other changes. So, we make include changes first-class
edits.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85734

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

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -21,6 +21,7 @@
 using namespace tooling;
 using namespace ast_matchers;
 namespace {
+using ::testing::ElementsAre;
 using ::testing::IsEmpty;
 using transformer::cat;
 using transformer::changeTo;
@@ -194,6 +195,43 @@
 }
 
 TEST_F(TransformerTest, AddIncludeQuoted) {
+  RewriteRule Rule =
+  makeRule(callExpr(callee(functionDecl(hasName("f",
+   {addInclude("clang/OtherLib.h"), changeTo(cat("other()"))});
+
+  std::string Input = R"cc(
+int f(int x);
+int h(int x) { return f(x); }
+  )cc";
+  std::string Expected = R"cc(#include "clang/OtherLib.h"
+
+int f(int x);
+int h(int x) { return other(); }
+  )cc";
+
+  testRule(Rule, Input, Expected);
+}
+
+TEST_F(TransformerTest, AddIncludeAngled) {
+  RewriteRule Rule = makeRule(
+  callExpr(callee(functionDecl(hasName("f",
+  {addInclude("clang/OtherLib.h", transformer::IncludeFormat::Angled),
+   changeTo(cat("other()"))});
+
+  std::string Input = R"cc(
+int f(int x);
+int h(int x) { return f(x); }
+  )cc";
+  std::string Expected = R"cc(#include 
+
+int f(int x);
+int h(int x) { return other(); }
+  )cc";
+
+  testRule(Rule, Input, Expected);
+}
+
+TEST_F(TransformerTest, AddIncludeQuotedForRule) {
   RewriteRule Rule = makeRule(callExpr(callee(functionDecl(hasName("f",
   changeTo(cat("other()")));
   addInclude(Rule, "clang/OtherLib.h");
@@ -211,7 +249,7 @@
   testRule(Rule, Input, Expected);
 }
 
-TEST_F(TransformerTest, AddIncludeAngled) {
+TEST_F(TransformerTest, AddIncludeAngledForRule) {
   RewriteRule Rule = makeRule(callExpr(callee(functionDecl(hasName("f",
   changeTo(cat("other()")));
   addInclude(Rule, "clang/OtherLib.h", transformer::IncludeFormat::Angled);
@@ -1180,4 +1218,32 @@
   EXPECT_EQ(format(*UpdatedCode), format(R"cc(#include "input.h"
 ;)cc"));
 }
+
+TEST_F(TransformerTest, AddIncludeMultipleFiles) {
+  std::string Header = R"cc(void RemoveThisFunction();)cc";
+  std::string Source = R"cc(#include "input.h"
+void Foo() {RemoveThisFunction();})cc";
+  Transformer T(
+  makeRule(callExpr(callee(
+   functionDecl(hasName("RemoveThisFunction")).bind("fun"))),
+   addInclude(node("fun"), "header.h")),
+  consumer());
+  T.registerMatchers(&MatchFinder);
+  auto Factory = newFrontendActionFactory(&MatchFinder);
+  EXPECT_TRUE(runToolOnCodeWithArgs(
+  Factory->create(), Source, std::vector(), "input.cc",
+  "clang-tool", std::make_shared(),
+  {{"input.h", Header}}));
+
+  ASSERT_EQ(Changes.size(), 1U);
+  ASSERT_EQ(Changes[0].getFilePath(), "./input.h");
+  EXPECT_THAT(Changes[0].getInsertedHeaders(), ElementsAre("header.h"));
+  EXPECT_THAT(Changes[0].getRemovedHeaders(), IsEmpty());
+  llvm::Expected UpdatedCode =
+  clang::tooling::applyAllReplacements(Header,
+   Changes[0].getReplacements());
+  ASSERT_TRUE(static_cast(UpdatedCode))
+  << "Could not update code: " << llvm::toString(UpdatedCode.takeError());
+  EXPECT_EQ(format(*UpdatedCode), format(Header));
+}
 } // namespace
Index: clang/lib/Tooling/Transformer/Transformer.cpp
===
--- clang/lib/Tooling/Transformer/Transformer.cpp
+++ clang/lib/Tooling/Transformer/Transformer.cpp
@@ -51,29 +51,20 @@
   T.Range.getBegin(), T.Metadata))
 .first;
 auto &AC = Iter->second;
-if (auto Err = AC.replace(*Result.SourceManager, T.Range, T.Replacement)) {
-  Consumer(std::move(Err));
-  return;
-}
-  }
-
-  for (auto &IDChangePair : ChangesByFileID) {
-auto &AC = IDChangePair.second;
-// FIXME: this will add includes to *all* changed files, which may not be
-// the intent. We should upgrade the representation to allow associating
-// headers with specific edits.
-for (const auto &I : Case.AddedIncludes) {
-

[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added inline comments.



Comment at: clang/include/clang/AST/DeclCXX.h:3843
   /// The declaration that this binding binds to part of.
+  // FIXME: Currently not set during deserialization of the BindingDecl;
+  // only set when the corresponding DecompositionDecl is visited.

rsmith wrote:
> !! This seems pretty bad; would it be hard to fix?
On further examination, I believe that`Decomp` is set but this is subtle, and 
it is likely that I am missing a case/wrong somehow.

The expression for the binding (`Binding`) will be deserialized when visiting 
the `BindingDecl`. This expression when non-null will always (as far as I can 
tell) contain a reference to the decomposition declaration so the decomposition 
will be deserialized, which will set `Decomp`.

The binding expression is null when the initializer is type-dependent. But 
then, since variable template decompositions are not allowed, the decomposition 
must occurs at block scope (`mayHaveDecompositionDeclarator`). This means that 
the `DecompositionDecl` will be read first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85613

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

[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:112
 //
 if (DRE->isNonOdrUse() != NOUR_Unevaluated)
   return S.Diag(DRE->getBeginLoc(),

This can use `DiagnoseIfOdrUse` as soon as the inconsistency between parameters 
and local variables is removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85613

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

[PATCH] D85734: [libTooling] Move RewriteRule include edits to ASTEdit granularity.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

tweak


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85734

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

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -21,6 +21,7 @@
 using namespace tooling;
 using namespace ast_matchers;
 namespace {
+using ::testing::ElementsAre;
 using ::testing::IsEmpty;
 using transformer::cat;
 using transformer::changeTo;
@@ -194,6 +195,43 @@
 }
 
 TEST_F(TransformerTest, AddIncludeQuoted) {
+  RewriteRule Rule =
+  makeRule(callExpr(callee(functionDecl(hasName("f",
+   {addInclude("clang/OtherLib.h"), changeTo(cat("other()"))});
+
+  std::string Input = R"cc(
+int f(int x);
+int h(int x) { return f(x); }
+  )cc";
+  std::string Expected = R"cc(#include "clang/OtherLib.h"
+
+int f(int x);
+int h(int x) { return other(); }
+  )cc";
+
+  testRule(Rule, Input, Expected);
+}
+
+TEST_F(TransformerTest, AddIncludeAngled) {
+  RewriteRule Rule = makeRule(
+  callExpr(callee(functionDecl(hasName("f",
+  {addInclude("clang/OtherLib.h", transformer::IncludeFormat::Angled),
+   changeTo(cat("other()"))});
+
+  std::string Input = R"cc(
+int f(int x);
+int h(int x) { return f(x); }
+  )cc";
+  std::string Expected = R"cc(#include 
+
+int f(int x);
+int h(int x) { return other(); }
+  )cc";
+
+  testRule(Rule, Input, Expected);
+}
+
+TEST_F(TransformerTest, AddIncludeQuotedForRule) {
   RewriteRule Rule = makeRule(callExpr(callee(functionDecl(hasName("f",
   changeTo(cat("other()")));
   addInclude(Rule, "clang/OtherLib.h");
@@ -211,7 +249,7 @@
   testRule(Rule, Input, Expected);
 }
 
-TEST_F(TransformerTest, AddIncludeAngled) {
+TEST_F(TransformerTest, AddIncludeAngledForRule) {
   RewriteRule Rule = makeRule(callExpr(callee(functionDecl(hasName("f",
   changeTo(cat("other()")));
   addInclude(Rule, "clang/OtherLib.h", transformer::IncludeFormat::Angled);
@@ -1180,4 +1218,32 @@
   EXPECT_EQ(format(*UpdatedCode), format(R"cc(#include "input.h"
 ;)cc"));
 }
+
+TEST_F(TransformerTest, AddIncludeMultipleFiles) {
+  std::string Header = R"cc(void RemoveThisFunction();)cc";
+  std::string Source = R"cc(#include "input.h"
+void Foo() {RemoveThisFunction();})cc";
+  Transformer T(
+  makeRule(callExpr(callee(
+   functionDecl(hasName("RemoveThisFunction")).bind("fun"))),
+   addInclude(node("fun"), "header.h")),
+  consumer());
+  T.registerMatchers(&MatchFinder);
+  auto Factory = newFrontendActionFactory(&MatchFinder);
+  EXPECT_TRUE(runToolOnCodeWithArgs(
+  Factory->create(), Source, std::vector(), "input.cc",
+  "clang-tool", std::make_shared(),
+  {{"input.h", Header}}));
+
+  ASSERT_EQ(Changes.size(), 1U);
+  ASSERT_EQ(Changes[0].getFilePath(), "./input.h");
+  EXPECT_THAT(Changes[0].getInsertedHeaders(), ElementsAre("header.h"));
+  EXPECT_THAT(Changes[0].getRemovedHeaders(), IsEmpty());
+  llvm::Expected UpdatedCode =
+  clang::tooling::applyAllReplacements(Header,
+   Changes[0].getReplacements());
+  ASSERT_TRUE(static_cast(UpdatedCode))
+  << "Could not update code: " << llvm::toString(UpdatedCode.takeError());
+  EXPECT_EQ(format(*UpdatedCode), format(Header));
+}
 } // namespace
Index: clang/lib/Tooling/Transformer/Transformer.cpp
===
--- clang/lib/Tooling/Transformer/Transformer.cpp
+++ clang/lib/Tooling/Transformer/Transformer.cpp
@@ -51,29 +51,20 @@
   T.Range.getBegin(), T.Metadata))
 .first;
 auto &AC = Iter->second;
-if (auto Err = AC.replace(*Result.SourceManager, T.Range, T.Replacement)) {
-  Consumer(std::move(Err));
-  return;
-}
-  }
-
-  for (auto &IDChangePair : ChangesByFileID) {
-auto &AC = IDChangePair.second;
-// FIXME: this will add includes to *all* changed files, which may not be
-// the intent. We should upgrade the representation to allow associating
-// headers with specific edits.
-for (const auto &I : Case.AddedIncludes) {
-  auto &Header = I.first;
-  switch (I.second) {
-  case transformer::IncludeFormat::Quoted:
-AC.addHeader(Header);
-break;
-  case transformer::IncludeFormat::Angled:
-AC.addHeader((llvm::Twine("<") + Header + ">").str());
-break;
+switch (T.Kind) {
+case transformer

[PATCH] D85735: [OpenMP] Context selector extensions for template functions

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert created this revision.
jdoerfert added reviewers: JonChesterfield, jhuber6, ABataev.
Herald added subscribers: llvm-commits, guansong, bollu, yaxunl.
Herald added a reviewer: aaron.ballman.
Herald added projects: clang, LLVM.
jdoerfert requested review of this revision.
Herald added a subscriber: sstefan1.

With this extension the effects of `omp begin declare variant` will be
applied to template function declarations. The behavior is opt-in and
controlled by the `extension(allow_templates)` trait. While generally
useful, this will enable us to implement complex math function calls by
overloading the templates of the standard library with the ones in
libc++.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85735

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Headers/openmp_wrappers/cmath
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/ast-dump-openmp-begin-declare-variant_template_2.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def

Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -1114,6 +1114,7 @@
 __OMP_TRAIT_PROPERTY(implementation, extension, match_all)
 __OMP_TRAIT_PROPERTY(implementation, extension, match_any)
 __OMP_TRAIT_PROPERTY(implementation, extension, match_none)
+__OMP_TRAIT_PROPERTY(implementation, extension, allow_templates)
 
 __OMP_TRAIT_SET(user)
 
Index: clang/test/AST/ast-dump-openmp-begin-declare-variant_template_2.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-openmp-begin-declare-variant_template_2.cpp
@@ -0,0 +1,264 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fopenmp -verify -ast-dump %s -x c++ | FileCheck %s
+// expected-no-diagnostics
+
+template 
+int also_before(T) {
+  return 1;
+}
+template 
+int also_before_mismatch(void) {
+  return 0;
+}
+int also_before_non_template(void) {
+  return 0;
+}
+
+#pragma omp begin declare variant match(implementation = {extension(allow_templates)})
+template 
+int also_before(T) {
+  return 0;
+}
+template 
+int also_after(T) {
+  return 0;
+}
+template 
+int also_after_mismatch(T, Q) {
+  return 2;
+}
+template 
+int also_before_mismatch(T) {
+  return 3;
+}
+template 
+int also_before_non_template(T) {
+  return 4;
+}
+template 
+int only_def(void) {
+  return 0;
+}
+#pragma omp end declare variant
+
+template 
+int also_after(T) {
+  return 6;
+}
+template 
+int also_after_mismatch(T) {
+  return 0;
+}
+
+int test() {
+  // Should return 0.
+  return also_before(0.) + also_before_mismatch<0>() + also_before_non_template() + also_after(0) + also_after_mismatch(0) + only_def<0>();
+}
+
+// CHECK:  |-FunctionTemplateDecl [[ADDR_0:0x[a-z0-9]*]] <{{.*}}, line:7:1> line:5:5 also_before
+// CHECK-NEXT: | |-TemplateTypeParmDecl [[ADDR_1:0x[a-z0-9]*]]  col:19 referenced typename depth 0 index 0 T
+// CHECK-NEXT: | |-FunctionDecl [[ADDR_2:0x[a-z0-9]*]]  line:5:5 also_before 'int (T)'
+// CHECK-NEXT: | | |-ParmVarDecl [[ADDR_3:0x[a-z0-9]*]]  col:18 'T'
+// CHECK-NEXT: | | |-CompoundStmt [[ADDR_4:0x[a-z0-9]*]] 
+// CHECK-NEXT: | | | `-ReturnStmt [[ADDR_5:0x[a-z0-9]*]] 
+// CHECK-NEXT: | | |   `-IntegerLiteral [[ADDR_6:0x[a-z0-9]*]]  'int' 1
+// CHECK-NEXT: | | `-OMPDeclareVariantAttr [[ADDR_7:0x[a-z0-9]*]] <> Implicit implementation={extension(allow_templates)}
+// CHECK-NEXT: | |   `-DeclRefExpr [[ADDR_8:0x[a-z0-9]*]]  'int (T)' Function [[ADDR_9:0x[a-z0-9]*]] 'also_before[implementation={extension(allow_templates)}]' 'int (T)'
+// CHECK-NEXT: | `-FunctionDecl [[ADDR_10:0x[a-z0-9]*]]  line:5:5 used also_before 'int (double)'
+// CHECK-NEXT: |   |-TemplateArgument type 'double'
+// CHECK-NEXT: |   | `-BuiltinType [[ADDR_11:0x[a-z0-9]*]] 'double'
+// CHECK-NEXT: |   |-ParmVarDecl [[ADDR_12:0x[a-z0-9]*]]  col:18 'double':'double'
+// CHECK-NEXT: |   |-CompoundStmt [[ADDR_13:0x[a-z0-9]*]] 
+// CHECK-NEXT: |   | `-ReturnStmt [[ADDR_14:0x[a-z0-9]*]] 
+// CHECK-NEXT: |   |   `-IntegerLiteral [[ADDR_6]]  'int' 1
+// CHECK-NEXT: |   `-OMPDeclareVariantAttr [[ADDR_15:0x[a-z0-9]*]] <> Implicit implementation={extension(allow_templates)}
+// CHECK-NEXT: | `-DeclRefExpr [[ADDR_16:0x[a-z0-9]*]]  'int (double)' Function [[ADDR_17:0x[a-z0-9]*]] 'also_before[implementation={extension(allow_templates)}]' 'int (double)'
+// CHECK-NEXT: |-FunctionTemplateDecl [[ADDR_18:0x[a-z0-9]*]]  line:9:5 also_before_mismatch
+// CHECK-NEXT: | |-NonTypeTemplateParmDecl [[ADDR_19:0x[a-z0-9]*]]  col:14 'int' depth 0 index 0 V
+// CHECK-NEXT: | |-FunctionDecl [[ADDR_20:0x[a-z0-9]*]]  line:9:5 also_before_mismatch 'int ({{.*}})'
+// CHECK-NEXT: | | `-CompoundStmt [[ADDR_21:0x[a-z0-9]*]] 
+// CHECK-

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

[Daniel Kiss via Phabricator via cfe-commits]

danielkiss added a comment.

>> it is not useful to have a bti annotated function unless everything else is 
>> bti compatible too: it is all or nothing per elf module.
>
> This is false. Some functions in an elf module could be in a guarded region, 
> some in a non-guarded region. Some function may always
> be called in a "BTI-safe" way, which may be unknown to the compiler.

Right now the elf and all of the `text` sections considered BTI enabled or not. 
The dynamic linkers/loaders can't support this use case without additional 
information to be encoded somewhere (and specified). To support such we need to 
consider grouping/align to page boundaries these functions in the linker 
because BTI could be controlled by flags in PTE.
With the current spec this usecase is not supported in this way. The user have 
to link the BTI protected code into another elf.
Side note: The `force-bti` linker option can't work with half BTI enabled 
objects.


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