[PATCH] D88434: [clangd] Improve PopulateSwitch tweak to work on non-empty switches

[Sam McCall via Phabricator via cfe-commits]

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

Very nice, thank you!
A few nits about comments and asserts.




Comment at: clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp:112
 
-  // If there aren't any enumerators, there's nothing to insert.
-  if (EnumD->enumerator_begin() == EnumD->enumerator_end())
-return false;
+  // Iterate through switch cases and enumerators at the same time, so we can
+  // exit early if we have more cases than enumerators.

Somewhere we should have a high-level comment about what we're doing here:

We trigger if there are fewer cases than enum values (and no case covers 
multiple values).
This guarantees we'll have at least one case to insert.
We don't yet determine what the cases are, as that means evaluating expressions.



Comment at: clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp:119
+   CaseList && I != E; CaseList = CaseList->getNextSwitchCase(), I++) {
+// Switch has a default case.
+if (isa(CaseList))

Code is obvious here, say why instead?

"Default likely intends to cover cases we'd insert."?

(Sometimes this is still interesting in that case, so we could consider 
downgrading from "fix" to "refactoring" or so in this case. But it makes sense 
to be conservative for now)



Comment at: clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp:123
+
+const CaseStmt *CS = dyn_cast(CaseList);
+if (!CS)

nit: just `cast` and don't check for null (it'll automatically 
assert). There are no other options.



Comment at: clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp:127
+
+// Case expression is a GNU range.
+if (CS->caseStmtIsGNURange())

, so our counting argument doesn't work.



Comment at: clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp:131
+
+// Case expression is not a constant expression or is value-dependent.
+const ConstantExpr *CE = dyn_cast(CS->getLHS());

"We may not be able to work out which cases are covered"?



Comment at: clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp:152
+const CaseStmt *CS = dyn_cast(CaseList);
+if (!CS || CS->caseStmtIsGNURange())
+  continue;

this can be an assert (and previous line a cast) - apply() never gets called 
unless prepare() returns true



Comment at: clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp:156
+const ConstantExpr *CE = dyn_cast_or_null(CS->getLHS());
+if (!CE || CE->isValueDependent())
+  continue;

similarly here



Comment at: clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp:183
+
+  if (Text.empty())
+return error("Populate switch: no enumerators to insert.");

this is also an assert, right? I don't think we can get here.



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:2838
+  {
+  // All enumerators already in switch (multiple, unscoped)
+  Function,

is there an interesting difference between the single/multiple case that 
wouldn't be covered by just the multiple case?
(and below)



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:2838
+  {
+  // All enumerators already in switch (multiple, unscoped)
+  Function,

sammccall wrote:
> is there an interesting difference between the single/multiple case that 
> wouldn't be covered by just the multiple case?
> (and below)
can we add a case with `default`?
(And optionally template/gnu range, though those are pretty rare)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88434

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

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

[Raul Tambre via Phabricator via cfe-commits]

tambre added a comment.

In D77491#2299938 , @rsmith wrote:

> We've hit a fairly subtle miscompile caused by this patch.
>
> glibc's setjmp.h looks like this (irrelevant parts removed):
>
>   struct __jmp_buf_tag { /*...*/ };
>   extern int __sigsetjmp(struct __jmp_buf_tag __env[1], int);
>   typedef struct __jmp_buf_tag sigjmp_buf[1];
>   #define sigsetjmp __sigsetjmp
>
> This worked fine with the old approach. But with the new approach, we decide 
> the declaration of `__sigsetjmp` is not a builtin, because at its point of 
> declaration, we can't compute the "proper" type because `sigjmp_buf` has not 
> been declared yet. As a result, we don't add a `BuiltinAttr` to 
> `__sigsetjmp`, but much more critically, we don't add a `ReturnsTwiceAttr`, 
> which results in miscompiles in calls to this function. (I think `sigsetjmp` 
> is the only affected function with glibc. `jmp_buf` is declared prior to 
> `__setjmp` and friends.)
>
> I suppose we don't actually care what the parameter types for `__sigsetjmp` 
> are, and it would be fine (and much safer) to treat any function with that 
> name as a builtin, like we used to. Perhaps we should have a way of marking 
> builtins as "the given type is what we expect / what we will implicitly 
> declare, but it's OK if it doesn't actually match"?

Marking `__sigsetjmp` as having custom typechecking should suffice (`t` 
attribute in Builtins.def), no? Though a case in 
`Sema::CheckBuiltinFunctionCall()` might also then be necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

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

[PATCH] D88469: [clangd] Heuristic resolution for dependent type and template names

[Nathan Ridge via Phabricator via cfe-commits]

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

Fixes https://github.com/clangd/clangd/issues/543


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88469

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -731,6 +731,33 @@
"template  T convert() const");
 }
 
+TEST_F(TargetDeclTest, DependentTypes) {
+  Flags = {"-fno-delayed-template-parsing"};
+
+  // Heuristic resolution of dependent type name
+  Code = R"cpp(
+template 
+struct A { struct B {}; };
+
+template 
+void foo(typename A::[[B]]);
+  )cpp";
+  EXPECT_DECLS("DependentNameTypeLoc", "struct B");
+
+  // Heuristic resolution of dependent template name
+  Code = R"cpp(
+template 
+struct A { 
+  template  struct B {};
+};
+
+template 
+void foo(typename A::template [[B]]);
+  )cpp";
+  EXPECT_DECLS("DependentTemplateSpecializationTypeLoc",
+   "template  struct B");
+}
+
 TEST_F(TargetDeclTest, ObjC) {
   Flags = {"-xobjective-c"};
   Code = R"cpp(
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -125,6 +125,7 @@
   return !D->isCXXInstanceMember();
 };
 const auto ValueFilter = [](const NamedDecl *D) { return isa(D); };
+const auto TypeFilter = [](const NamedDecl *D) { return !isa(D); };
 
 // Given the type T of a dependent expression that appears of the LHS of a
 // "->", heuristically find a corresponding pointee type in whose scope we
@@ -291,10 +292,8 @@
 //and both are lossy. We must know upfront what the caller ultimately 
wants.
 //
 // FIXME: improve common dependent scope using name lookup in primary 
templates.
-// We currently handle DependentScopeDeclRefExpr and
-// CXXDependentScopeMemberExpr, but some other constructs remain to be handled:
-//  - DependentTemplateSpecializationType,
-//  - DependentNameType
+// We currently handle several dependent constructs, but some others remain to
+// be handled:
 //  - UnresolvedUsingTypenameDecl
 struct TargetFinder {
   using RelSet = DeclRelationSet;
@@ -536,6 +535,23 @@
 if (auto *TD = DTST->getTemplateName().getAsTemplateDecl())
   Outer.add(TD->getTemplatedDecl(), Flags | Rel::TemplatePattern);
   }
+  void VisitDependentNameType(const DependentNameType *DNT) {
+for (const NamedDecl *ND : getMembersReferencedViaDependentName(
+ DNT->getQualifier()->getAsType(),
+ [DNT](ASTContext &) { return DNT->getIdentifier(); },
+ TypeFilter)) {
+  Outer.add(ND, Flags);
+}
+  }
+  void VisitDependentTemplateSpecializationType(
+  const DependentTemplateSpecializationType *DTST) {
+for (const NamedDecl *ND : getMembersReferencedViaDependentName(
+ DTST->getQualifier()->getAsType(),
+ [DTST](ASTContext &) { return DTST->getIdentifier(); },
+ TypeFilter)) {
+  Outer.add(ND, Flags);
+}
+  }
   void VisitTypedefType(const TypedefType *TT) {
 Outer.add(TT->getDecl(), Flags);
   }


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -731,6 +731,33 @@
"template  T convert() const");
 }
 
+TEST_F(TargetDeclTest, DependentTypes) {
+  Flags = {"-fno-delayed-template-parsing"};
+
+  // Heuristic resolution of dependent type name
+  Code = R"cpp(
+template 
+struct A { struct B {}; };
+
+template 
+void foo(typename A::[[B]]);
+  )cpp";
+  EXPECT_DECLS("DependentNameTypeLoc", "struct B");
+
+  // Heuristic resolution of dependent template name
+  Code = R"cpp(
+template 
+struct A { 
+  template  struct B {};
+};
+
+template 
+void foo(typename A::template [[B]]);
+  )cpp";
+  EXPECT_DECLS("DependentTemplateSpecializationTypeLoc",
+   "template  struct B");
+}
+
 TEST_F(TargetDeclTest, ObjC) {
   Flags = {"-xobjective-c"};
   Code = R"cpp(
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp

[PATCH] D83296: [clang-format] Add a MacroExpander.

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

In D83296#2299062 , @nridge wrote:

> What does this change mean for users of clang-format -- better formatting of 
> complicated (e.g. multi-line) macro invocations?

In addition to what Sam said, this also attempts to be an improvement in 
maintainability. Given this is a fairly complex change, you might ask how this 
helps :)
The idea is that we bundle the complexity of macro handling in a clearly 
separated part of the code that can be tested and developed ~on its own.
Currently, we have multiple macro regex settings that then lead to random code 
throughout clang-format that tries to handle those identifiers special.
Once this is done, we can delete all those settings, as the more generalized 
macro configuration will supersede them.




Comment at: clang/lib/Format/MacroExpander.cpp:190
+  return true;
+for (const auto &Arg : Args[I->getValue()]) {
+  // A token can be part of a macro argument at multiple levels.

sammccall wrote:
> nit: this is confusingly a const reference to a non-const pointer... `auto *` 
> or `FormatToken *`?
Yikes, thanks for catching!



Comment at: clang/unittests/Format/MacroExpanderTest.cpp:183
+  EXPECT_ATTRIBUTES(Result, Attributes);
+}
+

sammccall wrote:
> may want a test that uses of an arg after the first are not expanded, because 
> that "guards" a bunch of nasty potential bugs
Discussed offline: the above test tests exactly this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83296

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

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Being permissive about recognizing builtins when the expected signature 
requires a type that lookup can't find seems completely reasonable.  We don't 
really want to force library functions to take the custom-typechecking path 
just because we want to infer an attribute for them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

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

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

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 294901.
hokein marked 2 inline comments as done.
hokein added a comment.

address comments:

- reuse the RecoveryAST to control the early typo-correction;
- uses the fixed result type for some operators


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84226

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

Index: clang/test/Sema/error-dependence.c
===
--- /dev/null
+++ clang/test/Sema/error-dependence.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -frecovery-ast -fno-recovery-ast-type %s
+
+int call(int); // expected-note {{'call' declared here}}
+
+void test1(int s) {
+  // verify "assigning to 'int' from incompatible type ''" is
+  // not emitted.
+  s = call(); // expected-error {{too few arguments to function call}}
+}
Index: clang/test/AST/ast-dump-recovery.c
===
--- clang/test/AST/ast-dump-recovery.c
+++ clang/test/AST/ast-dump-recovery.c
@@ -42,11 +42,22 @@
 
 void test2() {
   int* ptr;
-  // FIXME: the top-level expr should be a binary operator.
-  // CHECK:  ImplicitCastExpr {{.*}} contains-errors 
-  // CHECK-NEXT: `-RecoveryExpr {{.*}} contains-errors lvalue
-  // CHECK-NEXT:   |-DeclRefExpr {{.*}} 'ptr' 'int *'
-  // CHECK-NEXT:   `-RecoveryExpr {{.*}}
-  // CHECK-NEXT: `-DeclRefExpr {{.*}} 'some_func'
+  // CHECK: BinaryOperator {{.*}} contains-errors '='
+  // CHECK-NEXT: |-DeclRefExpr {{.*}} 'ptr' 'int *'
+  // CHECK-NEXT: `-RecoveryExpr {{.*}}
+  // CHECK-NEXT:   `-DeclRefExpr {{.*}} 'some_func'
   ptr = some_func(); // should not crash
+
+  int compoundOp;
+  // CHECK: CompoundAssignOperator {{.*}} '+='
+  // CHECK-NEXT: |-DeclRefExpr {{.*}} 'compoundOp'
+  // CHECK-NEXT: `-RecoveryExpr {{.*}} contains-errors
+  // CHECK-NEXT:   `-DeclRefExpr {{.*}} 'some_func'
+  compoundOp += some_func();
+
+  // CHECK:  BinaryOperator {{.*}} 'int' contains-errors '||'
+  // CHECK-NEXT: |-RecoveryExpr {{.*}}
+  // CHECK-NEXT: | `-DeclRefExpr {{.*}} 'some_func'
+  // CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
+  some_func() || 1;
 }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -24,6 +24,7 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/ExprOpenMP.h"
+#include "clang/AST/OperationKinds.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/Builtins.h"
@@ -13676,7 +13677,7 @@
 CorrectDelayedTyposInBinOp(Sema &S, BinaryOperatorKind Opc, Expr *LHSExpr,
Expr *RHSExpr) {
   ExprResult LHS = LHSExpr, RHS = RHSExpr;
-  if (!S.getLangOpts().CPlusPlus) {
+  if (!S.getLangOpts().CPlusPlus && !S.getLangOpts().RecoveryAST) {
 // C cannot handle TypoExpr nodes on either side of a binop because it
 // doesn't handle dependent types properly, so make sure any TypoExprs have
 // been dealt with before checking the operands.
@@ -14356,6 +14357,37 @@
   return BuildOverloadedBinOp(*this, S, OpLoc, Opc, LHSExpr, RHSExpr);
   }
 
+  if (getLangOpts().RecoveryAST &&
+  (LHSExpr->isTypeDependent() || RHSExpr->isTypeDependent())) {
+assert(!getLangOpts().CPlusPlus);
+assert((LHSExpr->containsErrors() || RHSExpr->containsErrors()) &&
+   "Should only occur in error-recovery path.");
+if (BinaryOperator::isCompoundAssignmentOp(Opc))
+  return CompoundAssignOperator::Create(
+  Context, LHSExpr, RHSExpr, Opc,
+  LHSExpr->getType().getUnqualifiedType(), VK_LValue, OK_Ordinary,
+  OpLoc, CurFPFeatureOverrides());
+switch (Opc) {
+case BO_LT:
+case BO_GT:
+case BO_LE:
+case BO_GE:
+case BO_EQ:
+case BO_NE:
+
+case BO_LAnd:
+case BO_LOr:
+  // These operators have a fixed result type regardless of operands.
+  return BinaryOperator::Create(Context, LHSExpr, RHSExpr, Opc,
+Context.IntTy, VK_RValue, OK_Ordinary,
+OpLoc, CurFPFeatureOverrides());
+default:
+  return BinaryOperator::Create(Context, LHSExpr, RHSExpr, Opc,
+Context.DependentTy, VK_RValue, OK_Ordinary,
+OpLoc, CurFPFeatureOverrides());
+}
+  }
+
   // Build a built-in binary operation.
   return CreateBuiltinBinOp(OpLoc, Opc, LHSExpr, RHSExpr);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.def:153
 COMPATIBLE_LANGOPT(RecoveryASTType, 1, 0, "Preserve the type in recovery 
expressions")
+COMPATIBLE_LANGOPT(CDependence, 1, 0, "Build dependent AST nodes in C for 
better error recovery")
 

sammccall wrote:
> Why is this not just the RecoveryAST flag? Is there some use of dependence 
> outside the error-handling path?
> 
> If the language is C, recovery-AST only works if this flag is on too. So they 
> might as well always have the same value.
> If the language is C++, this flag is meaningless, right?
This flag is mainly for turning off early typo-corrections for C.

Thinking more about this, you're right, looks like using the RecoveryAST is 
possible:
- these early typo-corrections applies non-C++ code, and have to be there until 
we fully implemented recovery-expr-on-c;
- the current default mode of RecoveryAST: on for C++, and off for non-C++;
- for recovery-expr-on-c development, we need to turn on the RecoveryAST and 
turn off these early typo-corrections;

so `if (!S.getLangOpts().CPlusPlus && !S.getLangOpts().RecoveryAST) {` works.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84226

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

[PATCH] D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/XRefs.cpp:572
+  // - backward: 2^(N-1) lines.
+  unsigned WordGain = 1U << Word.Text.size();
+  // Line number for SM.translateLineCol() should be one-based, also

std::min(Word.Text.size(), numeric_limits::digits() - 1) to avoid UB 
:-(



Comment at: clang-tools-extra/clangd/XRefs.cpp:572
+  // - backward: 2^(N-1) lines.
+  unsigned WordGain = 1U << Word.Text.size();
+  // Line number for SM.translateLineCol() should be one-based, also

sammccall wrote:
> std::min(Word.Text.size(), numeric_limits::digits() - 1) to avoid 
> UB :-(
name WordGain is unclear to me: MaxDistance?



Comment at: clang-tools-extra/clangd/XRefs.cpp:574
+  // Line number for SM.translateLineCol() should be one-based, also
+  // SM.translateLineCol() cares about line number greater than
+  // number of lines in the file.

cares about -> can handle?



Comment at: clang-tools-extra/clangd/XRefs.cpp:578
+  // - LineMax = WordLine + 1 + 2^(N-1)
+  unsigned LineMin = WordLine + 1 <= WordGain ? 1 : WordLine + 1 - WordGain;
+  unsigned LineMax = WordLine + 1 + WordGain / 2;

I think this is backwards: min should divide wordgain by two, max should not?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87891

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

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

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.def:153
 COMPATIBLE_LANGOPT(RecoveryASTType, 1, 0, "Preserve the type in recovery 
expressions")
+COMPATIBLE_LANGOPT(CDependence, 1, 0, "Build dependent AST nodes in C for 
better error recovery")
 

hokein wrote:
> sammccall wrote:
> > Why is this not just the RecoveryAST flag? Is there some use of dependence 
> > outside the error-handling path?
> > 
> > If the language is C, recovery-AST only works if this flag is on too. So 
> > they might as well always have the same value.
> > If the language is C++, this flag is meaningless, right?
> This flag is mainly for turning off early typo-corrections for C.
> 
> Thinking more about this, you're right, looks like using the RecoveryAST is 
> possible:
> - these early typo-corrections applies non-C++ code, and have to be there 
> until we fully implemented recovery-expr-on-c;
> - the current default mode of RecoveryAST: on for C++, and off for non-C++;
> - for recovery-expr-on-c development, we need to turn on the RecoveryAST and 
> turn off these early typo-corrections;
> 
> so `if (!S.getLangOpts().CPlusPlus && !S.getLangOpts().RecoveryAST) {` works.
> 
> 
If that's going to be a common condition, I'd suggest a helper function like

```
/// (explanation referencing early typo correction here)
isDependenceAllowed(const LangOpts &LO) { return CPlusPlus || RecoveryAST; }
```
and then guard early typo correction with `if (!isDependenceAllowed())`



Comment at: clang/include/clang/Sema/Sema.h:5169
+  /// In C++, we need to perform unqualified lookups when creating a dependent
+  /// binary operator, thus this should be only used when the unqalified lookup
+  /// found nothing.

nit: unqualified
nit: remove "results"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84226

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

Re: [PATCH] D87822: [FPEnv] Evaluate constant expressions under non-default rounding modes

[Serge Pavlov via cfe-commits]

Hi!

This issue must be fixed by: https://reviews.llvm.org/rGf91b9c0f9858
Do you have recent changes from the trunk?

Thanks,
--Serge


On Tue, Sep 29, 2020 at 4:22 AM Leonard Chan via Phabricator <
revi...@reviews.llvm.org> wrote:

> leonardchan added a comment.
>
> Hi! It looks like this is causing a test failure on our aach64 builders (
> https://luci-milo.appspot.com/p/fuchsia/builders/ci/clang-linux-arm64/b8868095822628984976?
> ):
>
>   [1113/1114] Running the Clang regression tests
>   llvm-lit:
> /b/s/w/ir/k/staging/llvm_build/bin/../../../llvm-project/llvm/utils/lit/lit/llvm/config.py:379:
> note: using clang: /b/s/w/ir/k/staging/llvm_build/bin/clang
>   -- Testing: 26708 tests, 256 workers --
>   Testing:
>   FAIL: Clang :: AST/const-fpfeatures-diag.c (269 of 26708)
>    TEST 'Clang :: AST/const-fpfeatures-diag.c' FAILED
> 
>   Script:
>   --
>   : 'RUN: at line 1';   /b/s/w/ir/k/staging/llvm_build/bin/clang -cc1
> -internal-isystem /b/s/w/ir/k/staging/llvm_build/lib/clang/12.0.0/include
> -nostdsysteminc -verify -ffp-exception-behavior=strict -Wno-unknown-pragmas
> /b/s/w/ir/k/llvm-project/clang/test/AST/const-fpfeatures-diag.c
>   --
>   Exit Code: 1
>
>   Command Output (stderr):
>   --
>   error: 'error' diagnostics expected but not seen:
> File /b/s/w/ir/k/llvm-project/clang/test/AST/const-fpfeatures-diag.c
> Line 8: initializer element is not a compile-time constant
>   error: 'warning' diagnostics seen but not expected:
> (frontend): overriding currently unsupported use of floating point
> exceptions on this target
>   2 errors generated.
>
>   --
>
>   
>   Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
>   
>   Failed Tests (1):
> Clang :: AST/const-fpfeatures-diag.c
>
> Would you mind taking a look and sending out a fix? Thanks.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D87822/new/
>
> https://reviews.llvm.org/D87822
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87528: Enable '#pragma STDC FENV_ACCESS' in frontend cf. D69272 - Work in Progress

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

In D87528#2299497 , @mibintc wrote:

> In D87528#2297647 , @sepavloff wrote:
>
>> In D87528#2295015 , @mibintc wrote:
>>
>>> I tried using the 0924 version of the patch on an internal workload SPEC 
>>> "cpu2017" and found that a few files failed to compile because of an error 
>>> message on static initializer, like this: struct s { float f; }; static 
>>> struct s x = {0.63};   Compiled with ffp-model=strict "initializer..is not 
>>> a compile-time constant"
>>
>> Thank you for trying this.
>>
>> The error happens because static variable initializer gets rounding mode 
>> calculated from command-line options (`dynamic`), but this is wrong because 
>> such initializers must be calculated using constant rounding mode 
>> (`tonearest` in this case). Either we must force default FP mode in such 
>> initializers, or we are wrong when using 
>> `FPOptions::defaultWithoutTrailingStorage`. Need to analyze this problem 
>> more.
>
> @sepavloff Please provide precise reference to support the claim "but this is 
> wrong because such initializers must be calculated using constant rounding 
> mode..."  many thanks!

From http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2478.pdf:

**6.7.9 Initialization**
…
4 All the expressions in an initializer for an object that has static or thread 
storage duration shall be
constant expressions or string literals.

**6.6 Constant expressions**
…
2 A constant expression can be evaluated during translation rather than 
runtime, and accordingly
may be used in any place that a constant may be.

**F.8.2 Translation**
1 During translation, constant rounding direction modes (7.6.2) are in effect 
where specified. Elsewhere,
during translation the IEC 60559 default modes are in effect:

- The rounding direction mode is rounding to nearest.

…

**7.6.2 The FENV_ROUND pragma**
…
2 The FENV_ROUND pragma provides a means to specify a constant rounding 
direction for floating point
operations for standard floating types within a translation unit or compound 
statement. …


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

https://reviews.llvm.org/D87528

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

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

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.def:153
 COMPATIBLE_LANGOPT(RecoveryASTType, 1, 0, "Preserve the type in recovery 
expressions")
+COMPATIBLE_LANGOPT(CDependence, 1, 0, "Build dependent AST nodes in C for 
better error recovery")
 

sammccall wrote:
> hokein wrote:
> > sammccall wrote:
> > > Why is this not just the RecoveryAST flag? Is there some use of 
> > > dependence outside the error-handling path?
> > > 
> > > If the language is C, recovery-AST only works if this flag is on too. So 
> > > they might as well always have the same value.
> > > If the language is C++, this flag is meaningless, right?
> > This flag is mainly for turning off early typo-corrections for C.
> > 
> > Thinking more about this, you're right, looks like using the RecoveryAST is 
> > possible:
> > - these early typo-corrections applies non-C++ code, and have to be there 
> > until we fully implemented recovery-expr-on-c;
> > - the current default mode of RecoveryAST: on for C++, and off for non-C++;
> > - for recovery-expr-on-c development, we need to turn on the RecoveryAST 
> > and turn off these early typo-corrections;
> > 
> > so `if (!S.getLangOpts().CPlusPlus && !S.getLangOpts().RecoveryAST) {` 
> > works.
> > 
> > 
> If that's going to be a common condition, I'd suggest a helper function like
> 
> ```
> /// (explanation referencing early typo correction here)
> isDependenceAllowed(const LangOpts &LO) { return CPlusPlus || RecoveryAST; }
> ```
> and then guard early typo correction with `if (!isDependenceAllowed())`
sounds a good idea.



Comment at: clang/include/clang/Sema/Sema.h:5169
+  /// In C++, we need to perform unqualified lookups when creating a dependent
+  /// binary operator, thus this should be only used when the unqalified lookup
+  /// found nothing.

sammccall wrote:
> nit: unqualified
> nit: remove "results"
this was in the first snapshot, we don't need this change now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84226

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

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

[Haojian Wu via Phabricator via cfe-commits]

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

add isDependenceAllowed function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84226

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

Index: clang/test/Sema/error-dependence.c
===
--- /dev/null
+++ clang/test/Sema/error-dependence.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -frecovery-ast -fno-recovery-ast-type %s
+
+int call(int); // expected-note {{'call' declared here}}
+
+void test1(int s) {
+  // verify "assigning to 'int' from incompatible type ''" is
+  // not emitted.
+  s = call(); // expected-error {{too few arguments to function call}}
+}
Index: clang/test/AST/ast-dump-recovery.c
===
--- clang/test/AST/ast-dump-recovery.c
+++ clang/test/AST/ast-dump-recovery.c
@@ -42,11 +42,22 @@
 
 void test2() {
   int* ptr;
-  // FIXME: the top-level expr should be a binary operator.
-  // CHECK:  ImplicitCastExpr {{.*}} contains-errors 
-  // CHECK-NEXT: `-RecoveryExpr {{.*}} contains-errors lvalue
-  // CHECK-NEXT:   |-DeclRefExpr {{.*}} 'ptr' 'int *'
-  // CHECK-NEXT:   `-RecoveryExpr {{.*}}
-  // CHECK-NEXT: `-DeclRefExpr {{.*}} 'some_func'
+  // CHECK: BinaryOperator {{.*}} contains-errors '='
+  // CHECK-NEXT: |-DeclRefExpr {{.*}} 'ptr' 'int *'
+  // CHECK-NEXT: `-RecoveryExpr {{.*}}
+  // CHECK-NEXT:   `-DeclRefExpr {{.*}} 'some_func'
   ptr = some_func(); // should not crash
+
+  int compoundOp;
+  // CHECK: CompoundAssignOperator {{.*}} '+='
+  // CHECK-NEXT: |-DeclRefExpr {{.*}} 'compoundOp'
+  // CHECK-NEXT: `-RecoveryExpr {{.*}} contains-errors
+  // CHECK-NEXT:   `-DeclRefExpr {{.*}} 'some_func'
+  compoundOp += some_func();
+
+  // CHECK:  BinaryOperator {{.*}} 'int' contains-errors '||'
+  // CHECK-NEXT: |-RecoveryExpr {{.*}}
+  // CHECK-NEXT: | `-DeclRefExpr {{.*}} 'some_func'
+  // CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
+  some_func() || 1;
 }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -24,6 +24,7 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/ExprOpenMP.h"
+#include "clang/AST/OperationKinds.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/Builtins.h"
@@ -13676,7 +13677,7 @@
 CorrectDelayedTyposInBinOp(Sema &S, BinaryOperatorKind Opc, Expr *LHSExpr,
Expr *RHSExpr) {
   ExprResult LHS = LHSExpr, RHS = RHSExpr;
-  if (!S.getLangOpts().CPlusPlus) {
+  if (!S.Context.isDependenceAllowed()) {
 // C cannot handle TypoExpr nodes on either side of a binop because it
 // doesn't handle dependent types properly, so make sure any TypoExprs have
 // been dealt with before checking the operands.
@@ -14356,6 +14357,37 @@
   return BuildOverloadedBinOp(*this, S, OpLoc, Opc, LHSExpr, RHSExpr);
   }
 
+  if (getLangOpts().RecoveryAST &&
+  (LHSExpr->isTypeDependent() || RHSExpr->isTypeDependent())) {
+assert(!getLangOpts().CPlusPlus);
+assert((LHSExpr->containsErrors() || RHSExpr->containsErrors()) &&
+   "Should only occur in error-recovery path.");
+if (BinaryOperator::isCompoundAssignmentOp(Opc))
+  return CompoundAssignOperator::Create(
+  Context, LHSExpr, RHSExpr, Opc,
+  LHSExpr->getType().getUnqualifiedType(), VK_LValue, OK_Ordinary,
+  OpLoc, CurFPFeatureOverrides());
+switch (Opc) {
+case BO_LT:
+case BO_GT:
+case BO_LE:
+case BO_GE:
+case BO_EQ:
+case BO_NE:
+
+case BO_LAnd:
+case BO_LOr:
+  // These operators have a fixed result type regardless of operands.
+  return BinaryOperator::Create(Context, LHSExpr, RHSExpr, Opc,
+Context.IntTy, VK_RValue, OK_Ordinary,
+OpLoc, CurFPFeatureOverrides());
+default:
+  return BinaryOperator::Create(Context, LHSExpr, RHSExpr, Opc,
+Context.DependentTy, VK_RValue, OK_Ordinary,
+OpLoc, CurFPFeatureOverrides());
+}
+  }
+
   // Build a built-in binary operation.
   return CreateBuiltinBinOp(OpLoc, Opc, LHSExpr, RHSExpr);
 }
Index: clang/include/clang/AST/ASTContext.h
===
--- clang/include/clang/AST/ASTContext.h
+++ clang/include/clang/AST/ASTContext.h
@@ -665,6 +665,12 @@
 
   const LangOptions& getLangOpts() const { return LangOpts; }
 
+  // clang's C-only codepath doesn't support dependent types yet, it is used to
+  // perform

[PATCH] D88403: Migrate Declarators to use the List API

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas marked 3 inline comments as done.
eduucaldas added inline comments.



Comment at: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp:2932
   |-'void'
-  |-SimpleDeclarator Declarator
-  | |-'foo'
-  | `-ParametersAndQualifiers
-  |   |-'(' OpenParen
-  |   `-')' CloseParen
+  |-DeclaratorList Declarators
+  | `-SimpleDeclarator ListElement

gribozavr2 wrote:
> eduucaldas wrote:
> > This sounds weird and is probably the reason why SO many tests failed.
> > 
> > According to the [[ 
> > https://eel.is/c++draft/dcl.fct.def.general#nt:function-definition | 
> > grammar ]] a function definition/declaration might only have one declarator:
> > ```
> > function-definition:
> > attribute-specifier-seq_opt decl-specifier-seq_opt declarator 
> > virt-specifier-seq_opt function-body
> > ...
> > ```
> > But our implementation calls `processDeclaratorAndDeclaration` for 
> > `DeclaratorDecl`, which englobes `FunctionDecl`.
> > 
> > I also noticed that we seem to have quite extensive support for 
> > declarations even rarer ones:
> > `StaticAssertDeclaration`, `LinkageSpecificationDeclaration` ... Moreover, 
> > their names and definitions seem to adequately follow the grammar. However 
> > we don't have a `FunctionDefinition` as defined in the grammar. Was 
> > grouping `FunctionDefintion` and `FunctionDeclaration` as 
> > `SimpleDeclaration`  a conscious choice? (I looked quickly at commits that 
> > added `processDeclaratorAndDeclaration` but couldn't find any clue)
> It seemed reasonable for uniformity. However, I can definitely see an 
> argument for defining a special kind of tree node for them, especially now 
> that if we use SimpleDeclaration for functions we would always get a 
> 1-element list wrapper.
> 
> On the other hand, the following is a valid declaration:
> 
> ```
> int x, f(int);
> ```
> 
> Wouldn't it be weird if function definitions and declarations were modeled 
> differently? I understand they are different in the C++ grammar, but I think 
> it would be at least a bit surprising for users.
Oh my god... that hurts my eyes...

I don't know what is more weird to be fair. In my view unifying both seems like 
a similar decision as unifying expressions and statements under the same 
ClangAST node because:
1. I think they have other syntactic differences.
2. I think generally people see declarations and definitions as different 
things. Whereas having a list in the definition of a function is completely 
unexpected.

Looking back at the grammar for declarations I think we would also benefit of 
trying to simplify it before mapping it to syntax trees. Although I would love 
to do that, I'm afraid there's too much on my plate already.

Should I leave a `FIXME` regarding this and adapt the other tests? Or do you 
wanna discuss this more deeply? Both options are fine for me :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88403

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

[PATCH] D88403: Migrate Declarators to use the List API

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas updated this revision to Diff 294905.
eduucaldas marked an inline comment as done.
eduucaldas added a comment.

Improve comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88403

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/lib/Tooling/Syntax/Synthesis.cpp
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -2875,21 +2875,23 @@
 TranslationUnit Detached
 |-SimpleDeclaration
 | |-'int'
-| |-SimpleDeclarator Declarator
-| | |-'*'
-| | `-'a'
-| |-','
-| |-SimpleDeclarator Declarator
-| | `-'b'
+| |-DeclaratorList Declarators
+| | |-SimpleDeclarator ListElement
+| | | |-'*'
+| | | `-'a'
+| | |-',' ListDelimiter
+| | `-SimpleDeclarator ListElement
+| |   `-'b'
 | `-';'
 `-SimpleDeclaration
   |-'int'
-  |-SimpleDeclarator Declarator
-  | |-'*'
-  | `-'c'
-  |-','
-  |-SimpleDeclarator Declarator
-  | `-'d'
+  |-DeclaratorList Declarators
+  | |-SimpleDeclarator ListElement
+  | | |-'*'
+  | | `-'c'
+  | |-',' ListDelimiter
+  | `-SimpleDeclarator ListElement
+  |   `-'d'
   `-';'
 )txt"));
 }
@@ -2904,12 +2906,13 @@
 `-SimpleDeclaration
   |-'typedef'
   |-'int'
-  |-SimpleDeclarator Declarator
-  | |-'*'
-  | `-'a'
-  |-','
-  |-SimpleDeclarator Declarator
-  | `-'b'
+  |-DeclaratorList Declarators
+  | |-SimpleDeclarator ListElement
+  | | |-'*'
+  | | `-'a'
+  | |-',' ListDelimiter
+  | `-SimpleDeclarator ListElement
+  |   `-'b'
   `-';'
 )txt"));
 }
@@ -2926,33 +2929,36 @@
 TranslationUnit Detached
 `-SimpleDeclaration
   |-'void'
-  |-SimpleDeclarator Declarator
-  | |-'foo'
-  | `-ParametersAndQualifiers
-  |   |-'(' OpenParen
-  |   `-')' CloseParen
+  |-DeclaratorList Declarators
+  | `-SimpleDeclarator ListElement
+  |   |-'foo'
+  |   `-ParametersAndQualifiers
+  | |-'(' OpenParen
+  | `-')' CloseParen
   `-CompoundStatement
 |-'{' OpenParen
 |-DeclarationStatement Statement
 | |-SimpleDeclaration
 | | |-'int'
-| | |-SimpleDeclarator Declarator
-| | | |-'*'
-| | | `-'a'
-| | |-','
-| | `-SimpleDeclarator Declarator
-| |   `-'b'
+| | `-DeclaratorList Declarators
+| |   |-SimpleDeclarator ListElement
+| |   | |-'*'
+| |   | `-'a'
+| |   |-',' ListDelimiter
+| |   `-SimpleDeclarator ListElement
+| | `-'b'
 | `-';'
 |-DeclarationStatement Statement
 | |-SimpleDeclaration
 | | |-'typedef'
 | | |-'int'
-| | |-SimpleDeclarator Declarator
-| | | |-'*'
-| | | `-'ta'
-| | |-','
-| | `-SimpleDeclarator Declarator
-| |   `-'tb'
+| | `-DeclaratorList Declarators
+| |   |-SimpleDeclarator ListElement
+| |   | |-'*'
+| |   | `-'ta'
+| |   |-',' ListDelimiter
+| |   `-SimpleDeclarator ListElement
+| | `-'tb'
 | `-';'
 `-'}' CloseParen
 )txt"));
Index: clang/lib/Tooling/Syntax/Synthesis.cpp
===
--- clang/lib/Tooling/Syntax/Synthesis.cpp
+++ clang/lib/Tooling/Syntax/Synthesis.cpp
@@ -183,6 +183,8 @@
 return new (A.getAllocator()) syntax::CallArguments;
   case syntax::NodeKind::ParameterDeclarationList:
 return new (A.getAllocator()) syntax::ParameterDeclarationList;
+  case syntax::NodeKind::DeclaratorList:
+return new (A.getAllocator()) syntax::DeclaratorList;
   }
   llvm_unreachable("unknown node kind");
 }
Index: clang/lib/Tooling/Syntax/Nodes.cpp
===
--- clang/lib/Tooling/Syntax/Nodes.cpp
+++ clang/lib/Tooling/Syntax/Nodes.cpp
@@ -136,6 +136,8 @@
 return OS << "CallArguments";
   case NodeKind::ParameterDeclarationList:
 return OS << "ParameterDeclarationList";
+  case NodeKind::DeclaratorList:
+return OS << "DeclaratorList";
   }
   llvm_unreachable("unknown node kind");
 }
@@ -218,6 +220,8 @@
 return OS << "Callee";
   case syntax::NodeRole::Arguments:
 return OS << "Arguments";
+  case syntax::NodeRole::Declarators:
+return OS << "Declarators";
   }
   llvm_unreachable("invalid role");
 }
@@ -291,6 +295,29 @@
   return Children;
 }
 
+std::vector
+syntax::DeclaratorList::getDeclarators() {
+  auto DeclaratorsAsNodes = getElementsAsNodes();
+  std::vector Children;
+  for (const auto &DeclaratorAsNode : DeclaratorsAsNodes) {
+Children.push_back(llvm::cast(DeclaratorAsNode));
+  }
+  return Children;
+}
+
+std::vector>
+syntax::DeclaratorList::getDeclaratorsAndCommas() {
+  auto DeclaratorsAsNodesAndCommas = getElementsAsNodesAndDelimiters();
+  std::vector>
+  Children;
+  for (const auto &DeclaratorAsNodeAndComma : DeclaratorsAsNodesAndCommas) {
+Children.pu

[PATCH] D88470: [clangd] Extract options struct for ClangdLSPServer. NFC

[Sam McCall via Phabricator via cfe-commits]

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

In preparation for making moving TweakFilter from ClangdServer::Options to
a ClangdLSPServer option, and letting it vary per-request.
(In order to implement CodeActionParams.only)

Also a general overdue cleanup.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88470

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp

Index: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -40,9 +40,7 @@
 
   LSPClient &start() {
 EXPECT_FALSE(Server.hasValue()) << "Already initialized";
-Server.emplace(Client.transport(), FS, CCOpts, RenameOpts,
-   /*CompileCommandsDir=*/llvm::None, /*UseDirBasedCDB=*/false,
-   /*ForcedOffsetEncoding=*/llvm::None, Opts);
+Server.emplace(Client.transport(), FS, Opts);
 ServerThread.emplace([&] { EXPECT_TRUE(Server->run()); });
 Client.call("initialize", llvm::json::Object{});
 return Client;
@@ -64,9 +62,7 @@
   }
 
   MockFS FS;
-  CodeCompleteOptions CCOpts;
-  RenameOptions RenameOpts;
-  ClangdServer::Options Opts = ClangdServer::optsForTest();
+  ClangdLSPServer::Options Opts = ClangdLSPServer::optsForTest();
 
 private:
   // Color logs so we can distinguish them from test output.
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -650,9 +650,11 @@
   if (auto EnvFlags = llvm::sys::Process::GetEnv(FlagsEnvVar))
 log("{0}: {1}", FlagsEnvVar, *EnvFlags);
 
+  ClangdLSPServer::Options Opts;
+  Opts.UseDirBasedCDB = (CompileArgsFrom == FilesystemCompileArgs);
+
   // If --compile-commands-dir arg was invoked, check value and override default
   // path.
-  llvm::Optional CompileCommandsDirPath;
   if (!CompileCommandsDir.empty()) {
 if (llvm::sys::fs::exists(CompileCommandsDir)) {
   // We support passing both relative and absolute paths to the
@@ -666,7 +668,7 @@
  "will be ignored.",
  EC.message());
   } else {
-CompileCommandsDirPath = std::string(Path.str());
+Opts.CompileCommandsDir = std::string(Path.str());
   }
 } else {
   elog("Path specified by --compile-commands-dir does not exist. The "
@@ -674,7 +676,6 @@
 }
   }
 
-  ClangdServer::Options Opts;
   switch (PCHStorage) {
   case PCHStorageFlag::Memory:
 Opts.StorePreamblesInMemory = true;
@@ -724,21 +725,20 @@
   Opts.PreserveRecoveryASTType = RecoveryASTType;
   Opts.FoldingRanges = FoldingRanges;
 
-  clangd::CodeCompleteOptions CCOpts;
-  CCOpts.IncludeIneligibleResults = IncludeIneligibleResults;
-  CCOpts.Limit = LimitResults;
+  Opts.CodeCompleteOpts.IncludeIneligibleResults = IncludeIneligibleResults;
+  Opts.CodeCompleteOpts.Limit = LimitResults;
   if (CompletionStyle.getNumOccurrences())
-CCOpts.BundleOverloads = CompletionStyle != Detailed;
-  CCOpts.ShowOrigins = ShowOrigins;
-  CCOpts.InsertIncludes = HeaderInsertion;
+Opts.CodeCompleteOpts.BundleOverloads = CompletionStyle != Detailed;
+  Opts.CodeCompleteOpts.ShowOrigins = ShowOrigins;
+  Opts.CodeCompleteOpts.InsertIncludes = HeaderInsertion;
   if (!HeaderInsertionDecorators) {
-CCOpts.IncludeIndicator.Insert.clear();
-CCOpts.IncludeIndicator.NoInsert.clear();
+Opts.CodeCompleteOpts.IncludeIndicator.Insert.clear();
+Opts.CodeCompleteOpts.IncludeIndicator.NoInsert.clear();
   }
-  CCOpts.SpeculativeIndexRequest = Opts.StaticIndex;
-  CCOpts.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
-  CCOpts.AllScopes = AllScopesCompletion;
-  CCOpts.RunParser = CodeCompletionParse;
+  Opts.CodeCompleteOpts.SpeculativeIndexRequest = Opts.StaticIndex;
+  Opts.CodeCompleteOpts.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
+  Opts.CodeCompleteOpts.AllScopes = AllScopesCompletion;
+  Opts.CodeCompleteOpts.RunParser = CodeCompletionParse;
 
   RealThreadsafeFS TFS;
   std::vector> ProviderStack;
@@ -797,13 +797,11 @@
   return llvm::is_contained(TweakList, T.id());
 return true;
   };
-  llvm::Optional OffsetEncodingFromFlag;
   if (ForceOffsetEncoding != OffsetEncoding::UnsupportedEncoding)
-OffsetEncodingFromFlag = ForceOffsetEncoding;
+Opts.OffsetEncoding = ForceOffsetEncoding;
 
-  clangd::RenameOptions RenameOpts;
   // Shall we allow to customize the file limit?
-  RenameOp

[PATCH] D87282: [SYCL] Assume SYCL device functions are convergent

[Victor Lomuller via Phabricator via cfe-commits]

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

Change make sense to me (same rational as CUDA or OpenCL).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87282

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

[PATCH] D88470: [clangd] Extract options struct for ClangdLSPServer. NFC

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/ClangdLSPServer.h:39
 public:
-  /// If \p CompileCommandsDir has a value, compile_commands.json will be
-  /// loaded only from \p CompileCommandsDir. Otherwise, clangd will look
-  /// for compile_commands.json in all parent directories of each file.
-  /// If UseDirBasedCDB is false, compile commands are not read from disk.
-  // FIXME: Clean up signature around CDBs.
+  struct Options : ClangdServer::Options {
+/// Look for compilation databases, rather than using compile commands

One question here is whether the inheritance is too tricky.

The nice thing about it is that `Opts.CrossFileRename = true` is pretty natural 
at the configuration site, where `Opts.ServerOpts.CrossFileRename` isn't.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88470

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

[PATCH] D88470: [clangd] Extract options struct for ClangdLSPServer. NFC

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 294907.
sammccall added a comment.

Drop redundant 'opts' from member names of per-feature opts structs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88470

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp

Index: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -40,9 +40,7 @@
 
   LSPClient &start() {
 EXPECT_FALSE(Server.hasValue()) << "Already initialized";
-Server.emplace(Client.transport(), FS, CCOpts, RenameOpts,
-   /*CompileCommandsDir=*/llvm::None, /*UseDirBasedCDB=*/false,
-   /*ForcedOffsetEncoding=*/llvm::None, Opts);
+Server.emplace(Client.transport(), FS, Opts);
 ServerThread.emplace([&] { EXPECT_TRUE(Server->run()); });
 Client.call("initialize", llvm::json::Object{});
 return Client;
@@ -64,9 +62,7 @@
   }
 
   MockFS FS;
-  CodeCompleteOptions CCOpts;
-  RenameOptions RenameOpts;
-  ClangdServer::Options Opts = ClangdServer::optsForTest();
+  ClangdLSPServer::Options Opts = ClangdLSPServer::optsForTest();
 
 private:
   // Color logs so we can distinguish them from test output.
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -650,9 +650,11 @@
   if (auto EnvFlags = llvm::sys::Process::GetEnv(FlagsEnvVar))
 log("{0}: {1}", FlagsEnvVar, *EnvFlags);
 
+  ClangdLSPServer::Options Opts;
+  Opts.UseDirBasedCDB = (CompileArgsFrom == FilesystemCompileArgs);
+
   // If --compile-commands-dir arg was invoked, check value and override default
   // path.
-  llvm::Optional CompileCommandsDirPath;
   if (!CompileCommandsDir.empty()) {
 if (llvm::sys::fs::exists(CompileCommandsDir)) {
   // We support passing both relative and absolute paths to the
@@ -666,7 +668,7 @@
  "will be ignored.",
  EC.message());
   } else {
-CompileCommandsDirPath = std::string(Path.str());
+Opts.CompileCommandsDir = std::string(Path.str());
   }
 } else {
   elog("Path specified by --compile-commands-dir does not exist. The "
@@ -674,7 +676,6 @@
 }
   }
 
-  ClangdServer::Options Opts;
   switch (PCHStorage) {
   case PCHStorageFlag::Memory:
 Opts.StorePreamblesInMemory = true;
@@ -724,21 +725,20 @@
   Opts.PreserveRecoveryASTType = RecoveryASTType;
   Opts.FoldingRanges = FoldingRanges;
 
-  clangd::CodeCompleteOptions CCOpts;
-  CCOpts.IncludeIneligibleResults = IncludeIneligibleResults;
-  CCOpts.Limit = LimitResults;
+  Opts.CodeComplete.IncludeIneligibleResults = IncludeIneligibleResults;
+  Opts.CodeComplete.Limit = LimitResults;
   if (CompletionStyle.getNumOccurrences())
-CCOpts.BundleOverloads = CompletionStyle != Detailed;
-  CCOpts.ShowOrigins = ShowOrigins;
-  CCOpts.InsertIncludes = HeaderInsertion;
+Opts.CodeComplete.BundleOverloads = CompletionStyle != Detailed;
+  Opts.CodeComplete.ShowOrigins = ShowOrigins;
+  Opts.CodeComplete.InsertIncludes = HeaderInsertion;
   if (!HeaderInsertionDecorators) {
-CCOpts.IncludeIndicator.Insert.clear();
-CCOpts.IncludeIndicator.NoInsert.clear();
+Opts.CodeComplete.IncludeIndicator.Insert.clear();
+Opts.CodeComplete.IncludeIndicator.NoInsert.clear();
   }
-  CCOpts.SpeculativeIndexRequest = Opts.StaticIndex;
-  CCOpts.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
-  CCOpts.AllScopes = AllScopesCompletion;
-  CCOpts.RunParser = CodeCompletionParse;
+  Opts.CodeComplete.SpeculativeIndexRequest = Opts.StaticIndex;
+  Opts.CodeComplete.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
+  Opts.CodeComplete.AllScopes = AllScopesCompletion;
+  Opts.CodeComplete.RunParser = CodeCompletionParse;
 
   RealThreadsafeFS TFS;
   std::vector> ProviderStack;
@@ -797,13 +797,11 @@
   return llvm::is_contained(TweakList, T.id());
 return true;
   };
-  llvm::Optional OffsetEncodingFromFlag;
   if (ForceOffsetEncoding != OffsetEncoding::UnsupportedEncoding)
-OffsetEncodingFromFlag = ForceOffsetEncoding;
+Opts.OffsetEncoding = ForceOffsetEncoding;
 
-  clangd::RenameOptions RenameOpts;
   // Shall we allow to customize the file limit?
-  RenameOpts.AllowCrossFile = CrossFileRename;
+  Opts.Rename.AllowCrossFile = CrossFileRename;
 
   // Initialize and run ClangdLSPServer.
   // Change stdin to binary to not lose \r\n on windows.
@@ -834,10 +832,7 @@
 std::move(*Mappings));
   }
 
-  ClangdLSPServer LSPServe

[PATCH] D88469: [clangd] Heuristic resolution for dependent type and template names

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/FindTarget.cpp:128
 const auto ValueFilter = [](const NamedDecl *D) { return isa(D); };
+const auto TypeFilter = [](const NamedDecl *D) { return !isa(D); };
 

why not using `isa(D)`?



Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:743
+template 
+void foo(typename A::[[B]]);
+  )cpp";

can you try to add a nested struct C in B, and verify `typename 
A::B::[[C]])` still works?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88469

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

[PATCH] D88472: [WIP][clangd] Support non-renaming alias underlying decls in TargetDecl.

[Haojian Wu via Phabricator via cfe-commits]

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

Extend the TargetDecl API to fix the workaround in 
https://reviews.llvm.org/D87225 and
https://reviews.llvm.org/D74054.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88472

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/FindTarget.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1090,7 +1090,6 @@
   R"cpp(
   template  struct function {};
   template  using [[callback]] = function;
-
   c^allback foo;
 )cpp",
 
@@ -1118,17 +1117,17 @@
   // decls.
   R"cpp(
   namespace ns { class [[Foo]] {}; }
-  using ns::F^oo;
+  using ns::[[F^oo]];
 )cpp",
 
   R"cpp(
   namespace ns { int [[x]](char); int [[x]](double); }
-  using ns::^x;
+  using ns::[[^x]];
 )cpp",
 
   R"cpp(
   namespace ns { int [[x]](char); int x(double); }
-  using ns::x;
+  using ns::[[x]];
   int y = ^x('a');
 )cpp",
 
@@ -1156,7 +1155,7 @@
   };
   template 
   struct Derived : Base {
-using Base::w^aldo;
+using Base::[[w^aldo]];
   };
 )cpp",
   };
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -182,7 +182,7 @@
   )cpp";
   // f(char) is not referenced!
   EXPECT_DECLS("DeclRefExpr", {"using foo::f", Rel::Alias},
-   {"int f(int)", Rel::Underlying});
+   {"int f(int)", Rel::NonAliasUnderlying});
 
   Code = R"cpp(
 namespace foo {
@@ -193,8 +193,8 @@
   )cpp";
   // All overloads are referenced.
   EXPECT_DECLS("UsingDecl", {"using foo::f", Rel::Alias},
-   {"int f(int)", Rel::Underlying},
-   {"int f(char)", Rel::Underlying});
+   {"int f(int)", Rel::NonAliasUnderlying},
+   {"int f(char)", Rel::NonAliasUnderlying});
 
   Code = R"cpp(
 struct X {
@@ -206,7 +206,7 @@
 int x = Y().[[foo]]();
   )cpp";
   EXPECT_DECLS("MemberExpr", {"using X::foo", Rel::Alias},
-   {"int foo()", Rel::Underlying});
+   {"int foo()", Rel::NonAliasUnderlying});
 
   Code = R"cpp(
   template 
@@ -219,7 +219,7 @@
   };
 )cpp";
   EXPECT_DECLS("UnresolvedUsingValueDecl", {"using Base::waldo", Rel::Alias},
-   {"void waldo()", Rel::Underlying});
+   {"void waldo()", Rel::NonAliasUnderlying});
 }
 
 TEST_F(TargetDeclTest, ConstructorInitList) {
@@ -275,7 +275,7 @@
 int y = [[b]]::x;
   )cpp";
   EXPECT_DECLS("NestedNameSpecifierLoc", {"namespace b = a", Rel::Alias},
-   {"namespace a", Rel::Underlying});
+   {"namespace a", Rel::AliasUnderlying});
 }
 
 TEST_F(TargetDeclTest, Types) {
@@ -291,14 +291,14 @@
 [[X]] x;
   )cpp";
   EXPECT_DECLS("TypedefTypeLoc", {"typedef S X", Rel::Alias},
-   {"struct S", Rel::Underlying});
+   {"struct S", Rel::AliasUnderlying});
   Code = R"cpp(
 namespace ns { struct S{}; }
 typedef ns::S X;
 [[X]] x;
   )cpp";
   EXPECT_DECLS("TypedefTypeLoc", {"typedef ns::S X", Rel::Alias},
-   {"struct S", Rel::Underlying});
+   {"struct S", Rel::AliasUnderlying});
 
   // FIXME: Auto-completion in a template requires disabling delayed template
   // parsing.
@@ -325,7 +325,7 @@
 S X;
 [[decltype]](X) Y;
   )cpp";
-  EXPECT_DECLS("DecltypeTypeLoc", {"struct S", Rel::Underlying});
+  EXPECT_DECLS("DecltypeTypeLoc", {"struct S", Rel::NonAliasUnderlying});
 
   Code = R"cpp(
 struct S{};
@@ -534,8 +534,9 @@
   )cpp";
   EXPECT_DECLS("TemplateSpecializationTypeLoc",
{"template<> class SmallVector",
-Rel::TemplateInstantiation | Rel::Underlying},
-   {"class SmallVector", Rel::TemplatePattern | Rel::Underlying},
+Rel::TemplateInstantiation | DeclRelation::AliasUnderlying},
+   {"class SmallVector",
+Rel::TemplatePattern | DeclRelation::AliasUnderlying},
{"using TinyVector = SmallVector",
 Rel::Alias | Rel::TemplatePattern});
 }
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -314,8 +314,9 @@
   };
 
   // Emit all symbol loc

[PATCH] D88472: [WIP][clangd] Support non-renaming alias underlying decls in TargetDecl.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/FindTarget.h:112
+  /// Underlying declarations for renaming alias (typedef decl, type alias 
decl)
+  AliasUnderlying,
+  /// Underlying declarations for non-renaming alias, decltype, etc.

The previous `Alias` vs `Underlying` were pretty nice, but they were not enough 
to support our non-renaming-alias-underlying case. Looking for the feedback on 
the API change here.  



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:1120
   namespace ns { class [[Foo]] {}; }
-  using ns::F^oo;
+  using ns::[[F^oo]];
 )cpp",

this seems a small regression, I think it is fine.



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:1130
   namespace ns { int [[x]](char); int x(double); }
-  using ns::x;
+  using ns::[[x]];
   int y = ^x('a');

I think it is an improvement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88472

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

[PATCH] D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines

[Aleksandr Platonov via Phabricator via cfe-commits]

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

Fix possible UB at bitwise shift.
WordGain => MaxDistance.
Fix LineMin and LineMax values.
Fix comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87891

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1428,6 +1428,11 @@
 
 
   // h^i
+
+
+
+
+  int x = hi;
 )cpp",
   R"cpp(
   // prefer nearest occurrence even if several matched tokens
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -562,19 +562,34 @@
   auto Cost = [&](SourceLocation Loc) -> unsigned {
 assert(SM.getFileID(Loc) == File && "spelled token in wrong file?");
 unsigned Line = SM.getSpellingLineNumber(Loc);
-if (Line > WordLine)
-  return 1 + llvm::Log2_64(Line - WordLine);
-if (Line < WordLine)
-  return 2 + llvm::Log2_64(WordLine - Line);
-return 0;
+return Line >= WordLine ? Line - WordLine : 2 * (WordLine - Line);
   };
   const syntax::Token *BestTok = nullptr;
-  // Search bounds are based on word length: 2^N lines forward.
-  unsigned BestCost = Word.Text.size() + 1;
+  unsigned BestCost = -1;
+  // Search bounds are based on word length:
+  // - forward: 2^N lines
+  // - backward: 2^(N-1) lines.
+  unsigned MaxDistance =
+  1U << std::min(Word.Text.size(),
+   std::numeric_limits::digits - 1);
+  // Line number for SM.translateLineCol() should be one-based, also
+  // SM.translateLineCol() can handle line number greater than
+  // number of lines in the file.
+  // - LineMin = max(1, WordLine + 1 - 2^(N-1))
+  // - LineMax = WordLine + 1 + 2^N
+  unsigned LineMin =
+  WordLine + 1 <= MaxDistance / 2 ? 1 : WordLine + 1 - MaxDistance / 2;
+  unsigned LineMax = WordLine + 1 + MaxDistance;
+  SourceLocation LocMin = SM.translateLineCol(File, LineMin, 1);
+  assert(LocMin.isValid());
+  SourceLocation LocMax = SM.translateLineCol(File, LineMax, 1);
+  assert(LocMax.isValid());
 
   // Updates BestTok and BestCost if Tok is a good candidate.
   // May return true if the cost is too high for this token.
   auto Consider = [&](const syntax::Token &Tok) {
+if (Tok.location() < LocMin || Tok.location() > LocMax)
+  return true; // we are too far from the word, break the outer loop.
 if (!(Tok.kind() == tok::identifier && Tok.text(SM) == Word.Text))
   return false;
 // No point guessing the same location we started with.


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1428,6 +1428,11 @@
 
 
   // h^i
+
+
+
+
+  int x = hi;
 )cpp",
   R"cpp(
   // prefer nearest occurrence even if several matched tokens
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -562,19 +562,34 @@
   auto Cost = [&](SourceLocation Loc) -> unsigned {
 assert(SM.getFileID(Loc) == File && "spelled token in wrong file?");
 unsigned Line = SM.getSpellingLineNumber(Loc);
-if (Line > WordLine)
-  return 1 + llvm::Log2_64(Line - WordLine);
-if (Line < WordLine)
-  return 2 + llvm::Log2_64(WordLine - Line);
-return 0;
+return Line >= WordLine ? Line - WordLine : 2 * (WordLine - Line);
   };
   const syntax::Token *BestTok = nullptr;
-  // Search bounds are based on word length: 2^N lines forward.
-  unsigned BestCost = Word.Text.size() + 1;
+  unsigned BestCost = -1;
+  // Search bounds are based on word length:
+  // - forward: 2^N lines
+  // - backward: 2^(N-1) lines.
+  unsigned MaxDistance =
+  1U << std::min(Word.Text.size(),
+   std::numeric_limits::digits - 1);
+  // Line number for SM.translateLineCol() should be one-based, also
+  // SM.translateLineCol() can handle line number greater than
+  // number of lines in the file.
+  // - LineMin = max(1, WordLine + 1 - 2^(N-1))
+  // - LineMax = WordLine + 1 + 2^N
+  unsigned LineMin =
+  WordLine + 1 <= MaxDistance / 2 ? 1 : WordLine + 1 - MaxDistance / 2;
+  unsigned LineMax = WordLine + 1 + MaxDistance;
+  SourceLocation LocMin = SM.translateLineCol(File, LineMin, 1);
+  assert(LocMin.isValid());
+  SourceLocation LocMax = SM.translateLineCol(File, LineMax, 1);
+  assert(LocMax.isValid());
 
   // Updates BestTok and BestCos

[PATCH] D88338: [clangd] clangd --check: standalone diagnosis of common problems

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

thanks! this mostly looks good, as discussed offline I believe having an infra 
that we can improve over time is better than not having anything until we've 
got the "perfect" solution.

i just got a couple of questions about some pieces, and some nits.




Comment at: clang-tools-extra/clangd/tool/Check.cpp:57
+// Nonfatal failures are logged and tracked in ErrCount.
+class Checker {
+  // from constructor

put into anon namespace?



Comment at: clang-tools-extra/clangd/tool/Check.cpp:87
+auto Mangler = CommandMangler::detect();
+// Check for missing resource dir?
+if (Opts.ResourceDir)

i agree, this would help identifying the case of "clangd binary has been copied 
to some place without the necessary lib directory".

but i think we should check for the final `-resource-dir` in the compile 
command below. as user's compile flags can override whatever default clangd 
comes up with.



Comment at: clang-tools-extra/clangd/tool/Check.cpp:95
+Cmd = CDB->getFallbackCommand(File);
+log("Generic fallback command is: {0}", llvm::join(Cmd.CommandLine, " "));
+if (auto TrueCmd = CDB->getCompileCommand(File)) {

we don't have any custom fallback commands, what's the point of printing this 
always?



Comment at: clang-tools-extra/clangd/tool/Check.cpp:125
+Inputs.Opts.ClangTidyOpts =
+Opts.GetClangTidyOptions(*TFS.view(llvm::None), File);
+log("Parsing command...");

IIRC, option providers don't really log anything about success/failure of 
parsing the config file. what's the point of having this exactly?



Comment at: clang-tools-extra/clangd/tool/Check.cpp:126
+Opts.GetClangTidyOptions(*TFS.view(llvm::None), File);
+log("Parsing command...");
+Invocation =

it is clear that we've crashed while parsing compile commands if we don't see 
`cc1 args are` in the output. so maybe drop this one?



Comment at: clang-tools-extra/clangd/tool/Check.cpp:131
+showErrors(InvocationDiags);
+log("cc1 args are: {0}", llvm::join(CC1Args, " "));
+if (!Invocation) {

maybe we should also note that this doesn't reflect the final result. as we 
turn off pchs or dependency file outputting related flags afterwards.



Comment at: clang-tools-extra/clangd/tool/Check.cpp:137
+
+Style = getFormatStyleForFile(File, Inputs.Contents, TFS);
+

this seems ... surprising :D but I also don't have suggestion for a better 
place.



Comment at: clang-tools-extra/clangd/tool/Check.cpp:202
+  }
+  unsigned Definitions = locateSymbolAt(*AST, Pos, &Index).size();
+  vlog("definition: {0}", Definitions);

maybe we could print errors if the following has no results for "identifiers" ?



Comment at: clang-tools-extra/clangd/tool/Check.cpp:214
+  // Print (and count) the error-level diagnostics (warnings are ignored).
+  void showErrors(llvm::ArrayRef Diags) {
+for (const auto &D : Diags) {

nit: maybe make this a free function and `return ErrCount` ?



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:52
+// Implemented in Check.cpp.
+bool check(const llvm::StringRef File, const ThreadsafeFS &TFS,
+   llvm::Optional CompileCommandsPath,

why not have this in `Check.h` ?



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:833
+   ? 0
+   : (int)ErrorResultCode::CheckFailed;
+  }

nit: `static_cast(Err..)`

maybe do the same for line 846 while here. (line 872 already does that)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88338

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

[PATCH] D88472: [WIP][clangd] Support non-renaming alias underlying decls in TargetDecl.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

We manage to get rid of a little code here, but we add complexity to an 
important API, and make... one test better and a few tests worse.
I'd like to get rid of the wart in the code, but the tradeoff doesn't seem 
completely compelling... going to think about the API here a bit.




Comment at: clang-tools-extra/clangd/FindTarget.h:112
+  /// Underlying declarations for renaming alias (typedef decl, type alias 
decl)
+  AliasUnderlying,
+  /// Underlying declarations for non-renaming alias, decltype, etc.

hokein wrote:
> The previous `Alias` vs `Underlying` were pretty nice, but they were not 
> enough to support our non-renaming-alias-underlying case. Looking for the 
> feedback on the API change here.  
Inevitably this adds some noise, and the names are less clear (we certainly 
need to avoid using "alias" to mean two different things, and we should try to 
avoid negatives that confuse the boolean logic).
We could simplify this at least at query locations by defining Underlying = 
NonAliasUnderlying | AliasUnderlying, but unfortunately we can't put it in 
DeclRelation.

The other thing is this isn't complete either, because it doesn't distinguish 
between renaming and non-renaming `Alias`es. This is why we have the regression 
in the testcases.

That this is so verbose is a property of the original design: the idea was 
"mark this edge as alias->underlying, and apply a bit when that edge is 
traversed" but of course we decided to signal "the edge is not traversed" too. 
So if we introduce a new type of edge, we need *two* new bits - one at each end.

I'm trying to think of ideas for improvements without totally ditching the 
API...



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:1120
   namespace ns { class [[Foo]] {}; }
-  using ns::F^oo;
+  using ns::[[F^oo]];
 )cpp",

hokein wrote:
> this seems a small regression, I think it is fine.
I can't follow why this is different from the second case of

TEST_F(TargetDeclTest, UsingDecl), where the usingdecl is not reported at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88472

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

[PATCH] D87282: [SYCL] Assume SYCL device functions are convergent

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 294931.
bader added a comment.

Applied Ronan's suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87282

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGenSYCL/convergent.cpp


Index: clang/test/CodeGenSYCL/convergent.cpp
===
--- /dev/null
+++ clang/test/CodeGenSYCL/convergent.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -fsycl -fsycl-is-device -emit-llvm -disable-llvm-passes \
+// RUN:  -triple spir64-unknown-unknown-sycldevice -emit-llvm %s -o - | \
+// RUN:   FileCheck %s
+
+// CHECK-DAG: Function Attrs:
+// CHECK-DAG-SAME: convergent
+// CHECK-DAG-NEXT: define void @_Z3foov
+void foo() {
+  int a = 1;
+}
+
+template 
+__attribute__((sycl_kernel)) void kernel_single_task(const Func &kernelFunc) {
+  kernelFunc();
+}
+
+int main() {
+  kernel_single_task([] { foo(); });
+  return 0;
+}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2882,7 +2882,8 @@
   Opts.Coroutines = Opts.CPlusPlus20 || Args.hasArg(OPT_fcoroutines_ts);
 
   Opts.ConvergentFunctions = Opts.OpenCL || (Opts.CUDA && Opts.CUDAIsDevice) ||
-Args.hasArg(OPT_fconvergent_functions);
+ Opts.SYCLIsDevice ||
+ Args.hasArg(OPT_fconvergent_functions);
 
   Opts.DoubleSquareBracketAttributes =
   Args.hasFlag(OPT_fdouble_square_bracket_attributes,


Index: clang/test/CodeGenSYCL/convergent.cpp
===
--- /dev/null
+++ clang/test/CodeGenSYCL/convergent.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -fsycl -fsycl-is-device -emit-llvm -disable-llvm-passes \
+// RUN:  -triple spir64-unknown-unknown-sycldevice -emit-llvm %s -o - | \
+// RUN:   FileCheck %s
+
+// CHECK-DAG: Function Attrs:
+// CHECK-DAG-SAME: convergent
+// CHECK-DAG-NEXT: define void @_Z3foov
+void foo() {
+  int a = 1;
+}
+
+template 
+__attribute__((sycl_kernel)) void kernel_single_task(const Func &kernelFunc) {
+  kernelFunc();
+}
+
+int main() {
+  kernel_single_task([] { foo(); });
+  return 0;
+}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2882,7 +2882,8 @@
   Opts.Coroutines = Opts.CPlusPlus20 || Args.hasArg(OPT_fcoroutines_ts);
 
   Opts.ConvergentFunctions = Opts.OpenCL || (Opts.CUDA && Opts.CUDAIsDevice) ||
-Args.hasArg(OPT_fconvergent_functions);
+ Opts.SYCLIsDevice ||
+ Args.hasArg(OPT_fconvergent_functions);
 
   Opts.DoubleSquareBracketAttributes =
   Args.hasFlag(OPT_fdouble_square_bracket_attributes,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

[Balázs Benics via Phabricator via cfe-commits]

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

When the analyzer loads a value through a given type which was previously 
stored with a different type, it will implicitly cast the associated value to 
the type in which we are trying to access it.
However, sometimes this //implicit// cast done by the `CastRetrievedVal` in 
`RegionStoreManager::getBinding` casted the value to a wrong type.
In this example, it cast to the `unsigned char` (which is the type of the 
stored value of `**b`, stored at `#1`) instead of the static type of the access 
(the type of `**b` is  `char`at `#2`).

If the analyzer wouldn't overwrite the already given non-null type, it would 
cast it to the correct one instead.
This patch simply modifies the code to overwrite the cast type to the 
underlying type of the pointee's region only if the type were `null`.

This also resolves a FIXME in the test suite.

---

This test case crashes the analyzer:

  // RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
  // expected-no-diagnostics
  
  void test(int *a, char ***b, float *c) {
*(unsigned char **)b = (unsigned char *)a; // #1
if (**b == 0) // no-crash, #2
  ;
  
*(unsigned char **)b = (unsigned char *)c;
if (**b == 0) // no-crash
  ;
  }

Thank you @ASDenysPetrov for rising this issue at D77062 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88477

Files:
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/test/Analysis/nonloc-as-loc-crash.c
  clang/test/Analysis/svalbuilder-float-cast.c


Index: clang/test/Analysis/svalbuilder-float-cast.c
===
--- clang/test/Analysis/svalbuilder-float-cast.c
+++ clang/test/Analysis/svalbuilder-float-cast.c
@@ -4,13 +4,9 @@
 
 void SymbolCast_of_float_type_aux(int *p) {
   *p += 0;
-  // FIXME: Ideally, all unknown values should be symbolicated.
-  clang_analyzer_denote(*p, "$x"); // expected-warning{{Not a symbol}}
-
+  clang_analyzer_denote(*p, "$x");
   *p += 1;
-  // This should NOT be (float)$x + 1. Symbol $x was never casted to float.
-  // FIXME: Ideally, this should be $x + 1.
-  clang_analyzer_express(*p); // expected-warning{{Not a symbol}}
+  clang_analyzer_express(*p); // expected-warning{{$x + 1}}
 }
 
 void SymbolCast_of_float_type() {
Index: clang/test/Analysis/nonloc-as-loc-crash.c
===
--- /dev/null
+++ clang/test/Analysis/nonloc-as-loc-crash.c
@@ -0,0 +1,12 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+// expected-no-diagnostics
+
+void test(int *a, char ***b, float *c) {
+  *(unsigned char **)b = (unsigned char *)a;
+  if (**b == 0) // no-crash
+;
+
+  *(unsigned char **)b = (unsigned char *)c;
+  if (**b == 0) // no-crash
+;
+}
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1439,7 +1439,8 @@
 assert(!T->isVoidType() && "Attempting to dereference a void pointer!");
 MR = GetElementZeroRegion(cast(MR), T);
   } else {
-T = cast(MR)->getValueType();
+if (T.isNull())
+  T = cast(MR)->getValueType();
   }
 
   // FIXME: Perhaps this method should just take a 'const MemRegion*' argument


Index: clang/test/Analysis/svalbuilder-float-cast.c
===
--- clang/test/Analysis/svalbuilder-float-cast.c
+++ clang/test/Analysis/svalbuilder-float-cast.c
@@ -4,13 +4,9 @@
 
 void SymbolCast_of_float_type_aux(int *p) {
   *p += 0;
-  // FIXME: Ideally, all unknown values should be symbolicated.
-  clang_analyzer_denote(*p, "$x"); // expected-warning{{Not a symbol}}
-
+  clang_analyzer_denote(*p, "$x");
   *p += 1;
-  // This should NOT be (float)$x + 1. Symbol $x was never casted to float.
-  // FIXME: Ideally, this should be $x + 1.
-  clang_analyzer_express(*p); // expected-warning{{Not a symbol}}
+  clang_analyzer_express(*p); // expected-warning{{$x + 1}}
 }
 
 void SymbolCast_of_float_type() {
Index: clang/test/Analysis/nonloc-as-loc-crash.c
===
--- /dev/null
+++ clang/test/Analysis/nonloc-as-loc-crash.c
@@ -0,0 +1,12 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+// expected-no-diagnostics
+
+void test(int *a, char ***b, float *c) {
+  *(unsigned char **)b = (unsigned char *)a;
+  if (**b == 0) // no-crash
+;
+
+  *(unsigned char **)b = (unsigned char *)c;
+  if (**b == 0) // no-crash
+;
+}
Index: cla

[PATCH] D88174: [Sema] Address-space sensitive check for unbounded arrays (v2)

[Bevin Hansson via Phabricator via cfe-commits]

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

LGTM, but @aaron.ballman should probably give his two cents as well for 
completion's sake.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88174

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

[PATCH] D77062: [analyzer] Improve zero assumption in CStringChecke::assumeZero

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

In D77062#2298663 , @ASDenysPetrov 
wrote:

> @steakhal
> You told that you suppose a potential fix. It would be nice, if you share the 
> patch to review.

I mean, I already told you my suggestion, there was no concrete fix in my mind 
at the time.

In D77062#2298748 , @NoQ wrote:

> Nice, very interesting!
>
> The contract of RegionStore with respect to type punning is that it stores 
> the value //as written//, even if its type doesn't match the storage type, 
> but then it casts the value to the correct type upon reading by invoking 
> `CastRetrievedVal()` on it. That's where the fix should probably be.

Hm, thank you for the pointer.
I'm not sure if I fixed this in the right way - I'm still puzzled by the 
spaghetti code :D
Never the less, here is my proposed fix for this: D88477 


---

@ASDenysPetrov Do you still want to rework the API of the `assumeZero`?


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

https://reviews.llvm.org/D77062

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

[PATCH] D26418: [clang-tidy] Add '-suppress-checks-filter' option to suppress diagnostics from certain files

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

While I agree that there's an issue here that needs to be solved, I don't think 
this patch will be merged as-is -- there are technical issues brought up by 
@alexfh that have not been addressed, and I share the opinion that we should 
extend existing suppression mechanisms rather than try to invent another new 
one. If someone wanted to invest the time and energy into such a patch, I'd be 
happy to be added as a reviewer.


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

https://reviews.llvm.org/D26418

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

[clang] 9263931 - [SYCL] Assume SYCL device functions are convergent

[Alexey Bader via cfe-commits]

Author: Alexey Bader
Date: 2020-09-29T15:23:50+03:00
New Revision: 9263931fcccdc99000c1de668bea330711333729

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

LOG: [SYCL] Assume SYCL device functions are convergent

SYCL device compiler (similar to other SPMD compilers) assumes that
functions are convergent by default to avoid invalid transformations.
This attribute can be removed if compiler can prove that function does
not have convergent operations.

Reviewed By: Naghasan

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

Added: 
clang/test/CodeGenSYCL/convergent.cpp

Modified: 
clang/lib/Frontend/CompilerInvocation.cpp

Removed: 




diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index 2d008d8a3fbe..42224339250d 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -2882,7 +2882,8 @@ static void ParseLangArgs(LangOptions &Opts, ArgList 
&Args, InputKind IK,
   Opts.Coroutines = Opts.CPlusPlus20 || Args.hasArg(OPT_fcoroutines_ts);
 
   Opts.ConvergentFunctions = Opts.OpenCL || (Opts.CUDA && Opts.CUDAIsDevice) ||
-Args.hasArg(OPT_fconvergent_functions);
+ Opts.SYCLIsDevice ||
+ Args.hasArg(OPT_fconvergent_functions);
 
   Opts.DoubleSquareBracketAttributes =
   Args.hasFlag(OPT_fdouble_square_bracket_attributes,

diff  --git a/clang/test/CodeGenSYCL/convergent.cpp 
b/clang/test/CodeGenSYCL/convergent.cpp
new file mode 100644
index ..784fb8976c27
--- /dev/null
+++ b/clang/test/CodeGenSYCL/convergent.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -fsycl -fsycl-is-device -emit-llvm -disable-llvm-passes \
+// RUN:  -triple spir64-unknown-unknown-sycldevice -emit-llvm %s -o - | \
+// RUN:   FileCheck %s
+
+// CHECK-DAG: Function Attrs:
+// CHECK-DAG-SAME: convergent
+// CHECK-DAG-NEXT: define void @_Z3foov
+void foo() {
+  int a = 1;
+}
+
+template 
+__attribute__((sycl_kernel)) void kernel_single_task(const Func &kernelFunc) {
+  kernelFunc();
+}
+
+int main() {
+  kernel_single_task([] { foo(); });
+  return 0;
+}



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

[PATCH] D87282: [SYCL] Assume SYCL device functions are convergent

[Alexey Bader via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
bader marked an inline comment as done.
Closed by commit rG9263931fcccd: [SYCL] Assume SYCL device functions are 
convergent (authored by bader).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87282

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGenSYCL/convergent.cpp


Index: clang/test/CodeGenSYCL/convergent.cpp
===
--- /dev/null
+++ clang/test/CodeGenSYCL/convergent.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -fsycl -fsycl-is-device -emit-llvm -disable-llvm-passes \
+// RUN:  -triple spir64-unknown-unknown-sycldevice -emit-llvm %s -o - | \
+// RUN:   FileCheck %s
+
+// CHECK-DAG: Function Attrs:
+// CHECK-DAG-SAME: convergent
+// CHECK-DAG-NEXT: define void @_Z3foov
+void foo() {
+  int a = 1;
+}
+
+template 
+__attribute__((sycl_kernel)) void kernel_single_task(const Func &kernelFunc) {
+  kernelFunc();
+}
+
+int main() {
+  kernel_single_task([] { foo(); });
+  return 0;
+}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2882,7 +2882,8 @@
   Opts.Coroutines = Opts.CPlusPlus20 || Args.hasArg(OPT_fcoroutines_ts);
 
   Opts.ConvergentFunctions = Opts.OpenCL || (Opts.CUDA && Opts.CUDAIsDevice) ||
-Args.hasArg(OPT_fconvergent_functions);
+ Opts.SYCLIsDevice ||
+ Args.hasArg(OPT_fconvergent_functions);
 
   Opts.DoubleSquareBracketAttributes =
   Args.hasFlag(OPT_fdouble_square_bracket_attributes,


Index: clang/test/CodeGenSYCL/convergent.cpp
===
--- /dev/null
+++ clang/test/CodeGenSYCL/convergent.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -fsycl -fsycl-is-device -emit-llvm -disable-llvm-passes \
+// RUN:  -triple spir64-unknown-unknown-sycldevice -emit-llvm %s -o - | \
+// RUN:   FileCheck %s
+
+// CHECK-DAG: Function Attrs:
+// CHECK-DAG-SAME: convergent
+// CHECK-DAG-NEXT: define void @_Z3foov
+void foo() {
+  int a = 1;
+}
+
+template 
+__attribute__((sycl_kernel)) void kernel_single_task(const Func &kernelFunc) {
+  kernelFunc();
+}
+
+int main() {
+  kernel_single_task([] { foo(); });
+  return 0;
+}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2882,7 +2882,8 @@
   Opts.Coroutines = Opts.CPlusPlus20 || Args.hasArg(OPT_fcoroutines_ts);
 
   Opts.ConvergentFunctions = Opts.OpenCL || (Opts.CUDA && Opts.CUDAIsDevice) ||
-Args.hasArg(OPT_fconvergent_functions);
+ Opts.SYCLIsDevice ||
+ Args.hasArg(OPT_fconvergent_functions);
 
   Opts.DoubleSquareBracketAttributes =
   Args.hasFlag(OPT_fdouble_square_bracket_attributes,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87946: [OpenMP] Add Location Fields to Libomptarget Runtime for Debugging

[George Rokos via Phabricator via cfe-commits]

grokos added inline comments.



Comment at: openmp/libomptarget/include/Ident.h:48-51
+auto removePath = [](const std::string &path) {
+std::size_t pos = path.rfind('/');
+return path.substr(pos + 1);
+};

jhuber6 wrote:
> This will probably break with a Windows file path, but I don't think you can 
> even build most offloading if you're on Windows. Should I just add a 
> processor check?
> 
> ```
> #ifdef _WIN32
> #define PATH_DELIMITER '\\'
> #else
> #define PATH_DELIMITER '/'
> #endif
> ```
You are right, libomptarget does not run on Windows, but some fork which stays 
in sync with upstream libomptarget may do so and this change may break it. Can 
you add the proposed pre-processor check?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87946

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

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

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Just a follow up on this point, dependencies are a little messed up here. 
Running any of the check-clang-extra<...> results in all clang-tools-extra 
targets being built.
Not ideal if you just want to test a small change to one tools and potentially 
having to rebuild all extra-tools
Is this an artefact of how `check-clang-tools` also depends on all 
clang-tools-extra targets.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84176

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

[PATCH] D88445: Use "default member initializer" instead of "in-class initializer" for diagnostics

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 294947.
aaron.ballman added a comment.

Updating based on review feedback.


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

https://reviews.llvm.org/D88445

Files:
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/test/Parser/MicrosoftExtensions.cpp
  clang/test/Parser/cxx-class.cpp
  clang/test/SemaCXX/PR9572.cpp
  clang/test/SemaCXX/class.cpp
  clang/test/SemaCXX/cxx98-compat.cpp
  clang/test/SemaCXX/member-init.cpp

Index: clang/test/SemaCXX/member-init.cpp
===
--- clang/test/SemaCXX/member-init.cpp
+++ clang/test/SemaCXX/member-init.cpp
@@ -21,20 +21,20 @@
 };
 
 struct UnknownBound {
-  int as[] = { 1, 2, 3 }; // expected-error {{array bound cannot be deduced from an in-class initializer}}
+  int as[] = { 1, 2, 3 }; // expected-error {{array bound cannot be deduced from a default member initializer}}
   int bs[4] = { 4, 5, 6, 7 };
-  int cs[] = { 8, 9, 10 }; // expected-error {{array bound cannot be deduced from an in-class initializer}}
+  int cs[] = { 8, 9, 10 }; // expected-error {{array bound cannot be deduced from a default member initializer}}
 };
 
 template struct T { static const int B; };
 template<> struct T<2> { template using B = int; };
 const int C = 0, D = 0;
 struct S {
-  int as[] = { decltype(x)::B(0) }; // expected-error {{array bound cannot be deduced from an in-class initializer}}
+  int as[] = { decltype(x)::B(0) }; // expected-error {{array bound cannot be deduced from a default member initializer}}
   T x;
   // test that we handle invalid array bound deductions without crashing when the declarator name is itself invalid
   operator int[](){}; // expected-error {{'operator int' cannot be the name of a variable or data member}} \
-  // expected-error {{array bound cannot be deduced from an in-class initializer}}
+  // expected-error {{array bound cannot be deduced from a default member initializer}}
 };
 
 struct ThrowCtor { ThrowCtor(int) noexcept(false); };
Index: clang/test/SemaCXX/cxx98-compat.cpp
===
--- clang/test/SemaCXX/cxx98-compat.cpp
+++ clang/test/SemaCXX/cxx98-compat.cpp
@@ -122,7 +122,7 @@
 }
 
 struct InClassInit {
-  int n = 0; // expected-warning {{in-class initialization of non-static data members is incompatible with C++98}}
+  int n = 0; // expected-warning {{default member initializer for non-static data members is incompatible with C++98}}
 };
 
 struct OverrideControlBase {
Index: clang/test/SemaCXX/class.cpp
===
--- clang/test/SemaCXX/class.cpp
+++ clang/test/SemaCXX/class.cpp
@@ -44,7 +44,7 @@
 
   int i = 0;
 #if __cplusplus <= 199711L
-  // expected-warning@-2 {{in-class initialization of non-static data member is a C++11 extension}}
+  // expected-warning@-2 {{default member initializer for non-static data member is a C++11 extension}}
 #endif
   static int si = 0; // expected-error {{non-const static data member must be initialized out of line}}
   static const NestedC ci = 0; // expected-error {{static data member of type 'const C::NestedC' must be initialized out of line}}
@@ -188,7 +188,7 @@
   struct A {
 #if __cplusplus <= 199711L
 static const float x = 5.0f; // expected-warning {{in-class initializer for static data member of type 'const float' is a GNU extension}}
-static const float y = foo(); // expected-warning {{in-class initializer for static data member of type 'const float' is a GNU extension}} expected-error {{in-class initializer for static data member is not a constant expression}}
+static const float y = foo(); // expected-warning {{in-class initializer for static data member of type 'const float' is a GNU extension}} expected-error {{default member initializer for static data member is not a constant expression}}
 #else
 static constexpr float x = 5.0f;
 static constexpr float y = foo(); // expected-error {{constexpr variable 'y' must be initialized by a constant expression}} expected-note {{non-constexpr function 'foo' cannot be used in a constant expression}}
Index: clang/test/SemaCXX/PR9572.cpp
===
--- clang/test/SemaCXX/PR9572.cpp
+++ clang/test/SemaCXX/PR9572.cpp
@@ -21,7 +21,7 @@
 
   const int kBlah = 3;
 #if __cplusplus <= 199711L
-  // expected-warning@-2 {{in-class initialization of non-static data member is a C++11 extension}}
+  // expected-warning@-2 {{default member initializer for non-static data member is a C++11 extension}}
 #endif
 
   Foo();
Index: clang/test/Parser/cxx-class.cpp
===
--- clang/test/Parser/cxx-class.cpp
+++ clang/test/Parser

[PATCH] D88445: Use "default member initializer" instead of "in-class initializer" for diagnostics

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 294951.
aaron.ballman added a comment.

Missed a test case.


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

https://reviews.llvm.org/D88445

Files:
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/test/Parser/MicrosoftExtensions.cpp
  clang/test/Parser/cxx-class.cpp
  clang/test/SemaCXX/PR9572.cpp
  clang/test/SemaCXX/class.cpp
  clang/test/SemaCXX/cxx98-compat.cpp
  clang/test/SemaCXX/member-init.cpp

Index: clang/test/SemaCXX/member-init.cpp
===
--- clang/test/SemaCXX/member-init.cpp
+++ clang/test/SemaCXX/member-init.cpp
@@ -21,20 +21,20 @@
 };
 
 struct UnknownBound {
-  int as[] = { 1, 2, 3 }; // expected-error {{array bound cannot be deduced from an in-class initializer}}
+  int as[] = { 1, 2, 3 }; // expected-error {{array bound cannot be deduced from a default member initializer}}
   int bs[4] = { 4, 5, 6, 7 };
-  int cs[] = { 8, 9, 10 }; // expected-error {{array bound cannot be deduced from an in-class initializer}}
+  int cs[] = { 8, 9, 10 }; // expected-error {{array bound cannot be deduced from a default member initializer}}
 };
 
 template struct T { static const int B; };
 template<> struct T<2> { template using B = int; };
 const int C = 0, D = 0;
 struct S {
-  int as[] = { decltype(x)::B(0) }; // expected-error {{array bound cannot be deduced from an in-class initializer}}
+  int as[] = { decltype(x)::B(0) }; // expected-error {{array bound cannot be deduced from a default member initializer}}
   T x;
   // test that we handle invalid array bound deductions without crashing when the declarator name is itself invalid
   operator int[](){}; // expected-error {{'operator int' cannot be the name of a variable or data member}} \
-  // expected-error {{array bound cannot be deduced from an in-class initializer}}
+  // expected-error {{array bound cannot be deduced from a default member initializer}}
 };
 
 struct ThrowCtor { ThrowCtor(int) noexcept(false); };
Index: clang/test/SemaCXX/cxx98-compat.cpp
===
--- clang/test/SemaCXX/cxx98-compat.cpp
+++ clang/test/SemaCXX/cxx98-compat.cpp
@@ -122,7 +122,7 @@
 }
 
 struct InClassInit {
-  int n = 0; // expected-warning {{in-class initialization of non-static data members is incompatible with C++98}}
+  int n = 0; // expected-warning {{default member initializer for non-static data members is incompatible with C++98}}
 };
 
 struct OverrideControlBase {
Index: clang/test/SemaCXX/class.cpp
===
--- clang/test/SemaCXX/class.cpp
+++ clang/test/SemaCXX/class.cpp
@@ -44,7 +44,7 @@
 
   int i = 0;
 #if __cplusplus <= 199711L
-  // expected-warning@-2 {{in-class initialization of non-static data member is a C++11 extension}}
+  // expected-warning@-2 {{default member initializer for non-static data member is a C++11 extension}}
 #endif
   static int si = 0; // expected-error {{non-const static data member must be initialized out of line}}
   static const NestedC ci = 0; // expected-error {{static data member of type 'const C::NestedC' must be initialized out of line}}
Index: clang/test/SemaCXX/PR9572.cpp
===
--- clang/test/SemaCXX/PR9572.cpp
+++ clang/test/SemaCXX/PR9572.cpp
@@ -21,7 +21,7 @@
 
   const int kBlah = 3;
 #if __cplusplus <= 199711L
-  // expected-warning@-2 {{in-class initialization of non-static data member is a C++11 extension}}
+  // expected-warning@-2 {{default member initializer for non-static data member is a C++11 extension}}
 #endif
 
   Foo();
Index: clang/test/Parser/cxx-class.cpp
===
--- clang/test/Parser/cxx-class.cpp
+++ clang/test/Parser/cxx-class.cpp
@@ -229,34 +229,34 @@
 class PR20760_a {
   int a = ); // expected-error {{expected expression}}
 #if __cplusplus <= 199711L
-  // expected-warning@-2 {{in-class initialization of non-static data member is a C++11 extension}}
+  // expected-warning@-2 {{default member initializer for non-static data member is a C++11 extension}}
 #endif
 
   int b = }; // expected-error {{expected expression}}
 #if __cplusplus <= 199711L
-  // expected-warning@-2 {{in-class initialization of non-static data member is a C++11 extension}}
+  // expected-warning@-2 {{default member initializer for non-static data member is a C++11 extension}}
 #endif
 
   int c = ]; // expected-error {{expected expression}}
 #if __cplusplus <= 199711L
-  // expected-warning@-2 {{in-class initialization of non-static data member is a C++11 extension}}
+  // expected-warning@-2 {{default member initializer for non-static data member is a C++11 extension}}
 #endif
 
 };

[PATCH] D88434: [clangd] Improve PopulateSwitch tweak to work on non-empty switches

[Tadeo Kondrak via Phabricator via cfe-commits]

tdeo updated this revision to Diff 294959.
tdeo added a comment.

- Improve comments
- In apply(), assert on the conditions we established in prepare() instead of 
proceeding.
- Remove redundant tests and add new ones for value-dependent and GNU range 
cases.


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

https://reviews.llvm.org/D88434

Files:
  clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.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
@@ -2829,9 +2829,48 @@
   "unavailable",
   },
   {
-  // Existing enumerators in switch
+  // All enumerators already in switch (unscoped)
   Function,
-  R""(enum Enum {A}; ^switch ((Enum)0) {case A:break;})"",
+  R""(enum Enum {A,B}; ^switch (A) {case A:break;case B:break;})"",
+  "unavailable",
+  },
+  {
+  // All enumerators already in switch (scoped)
+  Function,
+  R""(
+enum class Enum {A,B};
+^switch (Enum::A) {case Enum::A:break;case Enum::B:break;}
+  )"",
+  "unavailable",
+  },
+  {
+  // Default case in switch
+  Function,
+  R""(
+enum class Enum {A,B};
+^switch (Enum::A) {default:break;}
+  )"",
+  "unavailable",
+  },
+  {
+  // GNU range in switch
+  Function,
+  R""(
+enum class Enum {A,B};
+^switch (Enum::A) {case Enum::A ... Enum::B:break;}
+  )"",
+  "unavailable",
+  },
+  {
+  // Value dependent case expression
+  File,
+  R""(
+enum class Enum {A,B};
+template
+void function() {
+^switch (Enum::A) {case Value:break;}
+}
+  )"",
   "unavailable",
   },
   {
@@ -2867,9 +2906,53 @@
   {
   // Scoped enumeration with multiple enumerators
   Function,
-  R""(enum class Enum {A,B}; ^switch (Enum::A) {})"",
-  R""(enum class Enum {A,B}; )""
-  R""(switch (Enum::A) {case Enum::A:case Enum::B:break;})"",
+  R""(
+enum class Enum {A,B};
+^switch (Enum::A) {}
+  )"",
+  R""(
+enum class Enum {A,B};
+switch (Enum::A) {case Enum::A:case Enum::B:break;}
+  )"",
+  },
+  {
+  // Only filling in missing enumerators (unscoped)
+  Function,
+  R""(
+enum Enum {A,B,C};
+^switch (A) {case B:break;}
+  )"",
+  R""(
+enum Enum {A,B,C};
+switch (A) {case B:break;case A:case C:break;}
+  )"",
+  },
+  {
+  // Only filling in missing enumerators,
+  // even when using integer literals
+  Function,
+  R""(
+enum Enum {A,B=1,C};
+^switch (A) {case 1:break;}
+  )"",
+  R""(
+enum Enum {A,B=1,C};
+switch (A) {case 1:break;case A:case C:break;}
+  )"",
+  },
+  {
+  // Only filling in missing enumerators (scoped)
+  Function,
+  R""(
+enum class Enum {A,B,C};
+^switch (Enum::A)
+{case Enum::B:break;}
+  )"",
+  R""(
+enum class Enum {A,B,C};
+switch (Enum::A)
+{case Enum::B:break;case Enum::A:case Enum::C:break;}
+  )"",
   },
   {
   // Scoped enumerations in namespace
Index: clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
@@ -33,12 +33,15 @@
 #include "AST.h"
 #include "Selection.h"
 #include "refactor/Tweak.h"
+#include "support/Logger.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/SmallSet.h"
+#include 
 #include 
 
 namespace clang {
@@ -52,18 +55,16 @@
   Intent intent() const override { return Refactor; }
 
 private:
-  ASTContext *ASTCtx = nullptr;
   const DeclContext *DeclCtx = nullptr;
   const SwitchStmt *Switch = nullptr;
   const CompoundStmt *Body = nullptr;
+  const EnumType *EnumT = nullptr;
   const EnumDecl *EnumD = nullptr;
 };
 
 REGISTER_TWEAK(PopulateSwitch)
 
 bool PopulateSwitch::prepare(const Selection &Sel) {
-  ASTCtx = &Sel.AST->getASTContext();
-
   const SelectionTree::Node *CA = Sel.ASTSelection.commonAncestor();
   if (

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

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

In D84176#2300488 , @njames93 wrote:

> Is this an artefact of how `check-clang-tools` also depends on all 
> clang-tools-extra targets.

Yes, this was not changed in this patch. Fixing that will require a lot more 
CMake magic which I did not have sufficient time to do so. We'd need to 
refactor the code which generates the initial targets for the subprojects, etc.
So yeah, the new build entry-point only gives you the benefit of not running 
the tests. Build list is equal to the normal `check-clang-tools` target.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84176

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

[PATCH] D88434: [clangd] Improve PopulateSwitch tweak to work on non-empty switches

[Tadeo Kondrak via Phabricator via cfe-commits]

tdeo marked 10 inline comments as done.
tdeo added a comment.

Thanks for the review! Please land this if there are no more issues.


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

https://reviews.llvm.org/D88434

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

[libunwind] f34ae1b - [AArch64] Add v8.5 Branch Target Identification support.

[Daniel Kiss via cfe-commits]

Author: Daniel Kiss
Date: 2020-09-29T15:51:01+02:00
New Revision: f34ae1b9de68152de037fd3e394d196b997c4296

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

LOG: [AArch64] Add v8.5 Branch Target Identification support.

The .note.gnu.property must be in the assembly file to indicate the
support for BTI otherwise BTI will be disabled for the whole library.
__unw_getcontext and libunwind::Registers_arm64::jumpto() may be called
indirectly therefore they should start with a landing pad.

Reviewed By: tamas.petz, #libunwind, compnerd

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

Added: 


Modified: 
libunwind/src/assembly.h

Removed: 




diff  --git a/libunwind/src/assembly.h b/libunwind/src/assembly.h
index 4cf179e13edc..3b1e6e6d01d7 100644
--- a/libunwind/src/assembly.h
+++ b/libunwind/src/assembly.h
@@ -48,6 +48,24 @@
 #define PPC64_OPD2
 #endif
 
+#if defined(__ARM_FEATURE_BTI_DEFAULT)
+  .pushsection ".note.gnu.property", "a" SEPARATOR 
\
+  .balign 8 SEPARATOR  
\
+  .long 4 SEPARATOR
\
+  .long 0x10 SEPARATOR 
\
+  .long 0x5 SEPARATOR  
\
+  .asciz "GNU" SEPARATOR   
\
+  .long 0xc000 SEPARATOR /* GNU_PROPERTY_AARCH64_FEATURE_1_AND */  
\
+  .long 4 SEPARATOR
\
+  .long 3 SEPARATOR /* GNU_PROPERTY_AARCH64_FEATURE_1_BTI AND */   
\
+/* GNU_PROPERTY_AARCH64_FEATURE_1_PAC */   
\
+  .long 0 SEPARATOR
\
+  .popsection SEPARATOR
+#define AARCH64_BTI  bti c
+#else
+#define AARCH64_BTI
+#endif
+
 #define GLUE2(a, b) a ## b
 #define GLUE(a, b) GLUE2(a, b)
 #define SYMBOL_NAME(name) GLUE(__USER_LABEL_PREFIX__, name)
@@ -144,7 +162,8 @@
   SYMBOL_IS_FUNC(SYMBOL_NAME(name)) SEPARATOR  
\
   PPC64_OPD1   
\
   SYMBOL_NAME(name):   
\
-  PPC64_OPD2
+  PPC64_OPD2   
\
+  AARCH64_BTI
 
 #if defined(__arm__)
 #if !defined(__ARM_ARCH)



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

[PATCH] D88434: [clangd] Improve PopulateSwitch tweak to work on non-empty switches

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.

Thank you!


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

https://reviews.llvm.org/D88434

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

[PATCH] D78938: Make LLVM build in C++20 mode

[Barry Revzin via Phabricator via cfe-commits]

BRevzin added inline comments.



Comment at: llvm/include/llvm/IR/BasicBlock.h:324-325
+template ::value>>
 phi_iterator_impl(const phi_iterator_impl &Arg)

dblaikie wrote:
> BRevzin wrote:
> > dblaikie wrote:
> > > What tripped over/required this SFINAE?
> > There's somewhere which compared a const iterator to a non-const iterator, 
> > that ends up doing conversions in both directions under C++20 rules, one 
> > direction of which is perfectly fine and the other was a hard error. Need 
> > to make the non-const iterator not constructible from a const iterator.
> Is this true for all iterators? Or some quirk of how this one is written/used 
> (that could be fixed/changed there instead)?
So I undid this change to copy the exact issue that I ran into. But it actually 
ended up still compiling anyway. Part of the issue might be that I keep futzing 
with the cmake configuration since it takes me more than an hour to compile, so 
maybe there's some target that needed this change that I no longer compile.

But the kind of problem I think this had was:

```
template 
struct iterator {
T* p;

template 
iterator(iterator rhs)
: p(rhs.p)
{ } 

bool operator==(iterator const& rhs);
};

bool check(iterator a, iterator b) {
return a == b;
}
```

which compiles fine in C++17 but is ambiguous in C++20 because 
`b.operator==(a)` is also a candidate (even though it's not _really_ a 
candidate, and would be a hard error if selected). the sfinae removes the bad 
candidate from the set. 

It's true for all iterators in general in that you want `const_iterator` 
constructible from `iterator` but not the reverse (unless they're the same 
type).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938

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

[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> In this example, it cast to the `unsigned char` (which is the type of the 
> stored value of `**b`, stored at `#1`) instead of the static type of the 
> access (the type of `**b` is  `char`at `#2`).

Possible typo in the summary. At `#2` the type should be `char *` shouldn't it?




Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1442
   } else {
-T = cast(MR)->getValueType();
+if (T.isNull())
+  T = cast(MR)->getValueType();

Nice catch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88477

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

[PATCH] D88174: [Sema] Address-space sensitive check for unbounded arrays (v2)

[Chris Hamilton via Phabricator via cfe-commits]

chrish_ericsson_atx added a comment.

I'm going to land this as-is, based on Bevin's LGTM and my own confidence that 
the 10 lines I added are correct (enough).  @aaron.ballman , please reopen this 
review if you have additional comments or concerns.  Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88174

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

[clang] d9ee935 - [Sema] Address-space sensitive check for unbounded arrays (v2)

[Chris Hamilton via cfe-commits]

Author: Chris Hamilton
Date: 2020-09-29T16:14:48+02:00
New Revision: d9ee935679e7164d1c47e351bbbcf5c25742b59c

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

LOG: [Sema] Address-space sensitive check for unbounded arrays (v2)

Check applied to unbounded (incomplete) arrays and pointers to spot
cases where the computed address is beyond the largest possible
addressable extent of the array, based on the address space in which the
array is delcared, or which the pointer refers to.

Check helps to avoid cases of nonsense pointer math and array indexing
which could lead to linker failures or runtime exceptions.  Of
particular interest when building for embedded systems with small
address spaces.

This is version 2 of this patch -- version 1 had some testing issues
due to a sign error in existing code.  That error is corrected and
lit test for this chagne is extended to verify the fix.

Originally reviewed/accepted by: aaron.ballman
Original revision: https://reviews.llvm.org/D86796

Reviewed By: ebevhan

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

Added: 
clang/test/Sema/unbounded-array-bounds.c

Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaChecking.cpp
clang/test/Sema/const-eval.c
clang/test/SemaCXX/constant-expression-cxx1y.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 42d50426ccd8..8f6c7b9400fa 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8917,6 +8917,14 @@ def warn_array_index_precedes_bounds : Warning<
 def warn_array_index_exceeds_bounds : Warning<
   "array index %0 is past the end of the array (which contains %1 "
   "element%s2)">, InGroup;
+def warn_ptr_arith_exceeds_max_addressable_bounds : Warning<
+  "the pointer incremented by %0 refers past the last possible element for an 
array in %1-bit "
+  "address space containing %2-bit (%3-byte) elements (max possible %4 
element%s5)">,
+  InGroup;
+def warn_array_index_exceeds_max_addressable_bounds : Warning<
+  "array index %0 refers past the last possible element for an array in %1-bit 
"
+  "address space containing %2-bit (%3-byte) elements (max possible %4 
element%s5)">,
+  InGroup;
 def note_array_declared_here : Note<
   "array %0 declared here">;
 

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index eeb322262400..a5de6a5c88db 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -14057,11 +14057,11 @@ void Sema::CheckArrayAccess(const Expr *BaseExpr, 
const Expr *IndexExpr,
   const ConstantArrayType *ArrayTy =
   Context.getAsConstantArrayType(BaseExpr->getType());
 
-  if (!ArrayTy)
-return;
-
-  const Type *BaseType = ArrayTy->getElementType().getTypePtr();
-  if (EffectiveType->isDependentType() || BaseType->isDependentType())
+  const Type *BaseType =
+  ArrayTy == nullptr ? nullptr : ArrayTy->getElementType().getTypePtr();
+  bool IsUnboundedArray = (BaseType == nullptr);
+  if (EffectiveType->isDependentType() ||
+  (!IsUnboundedArray && BaseType->isDependentType()))
 return;
 
   Expr::EvalResult Result;
@@ -14069,8 +14069,10 @@ void Sema::CheckArrayAccess(const Expr *BaseExpr, 
const Expr *IndexExpr,
 return;
 
   llvm::APSInt index = Result.Val.getInt();
-  if (IndexNegated)
+  if (IndexNegated) {
+index.setIsUnsigned(false);
 index = -index;
+  }
 
   const NamedDecl *ND = nullptr;
   if (const DeclRefExpr *DRE = dyn_cast(BaseExpr))
@@ -14078,6 +14080,69 @@ void Sema::CheckArrayAccess(const Expr *BaseExpr, 
const Expr *IndexExpr,
   if (const MemberExpr *ME = dyn_cast(BaseExpr))
 ND = ME->getMemberDecl();
 
+  if (IsUnboundedArray) {
+if (index.isUnsigned() || !index.isNegative()) {
+  const auto &ASTC = getASTContext();
+  unsigned AddrBits =
+  ASTC.getTargetInfo().getPointerWidth(ASTC.getTargetAddressSpace(
+  EffectiveType->getCanonicalTypeInternal()));
+  if (index.getBitWidth() < AddrBits)
+index = index.zext(AddrBits);
+  CharUnits ElemCharUnits = ASTC.getTypeSizeInChars(EffectiveType);
+  llvm::APInt ElemBytes(index.getBitWidth(), ElemCharUnits.getQuantity());
+  // If index has more active bits than address space, we already know
+  // we have a bounds violation to warn about.  Otherwise, compute
+  // address of (index + 1)th element, and warn about bounds violation
+  // only if that address exceeds address space.
+  if (index.getActiveBits() <= AddrBits) {
+bool Overflow;
+llvm::APInt Product(index);
+Product += 1;
+Product = Product.umul_ov(ElemBytes, Overflow);
+

[PATCH] D78938: Make LLVM build in C++20 mode

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h:167-171
-
-inline bool operator!=(const DWARFExpression::iterator &LHS,
-   const DWARFExpression::iterator &RHS) {
-  return !(LHS == RHS);
-}

BRevzin wrote:
> dblaikie wrote:
> > Why are some being removed? That seems harder to justify. Even if they're 
> > not called, it may be more valuable to have the symmetry to reduce friction 
> > if/when they are needed. (iterators seem pretty common to compare for 
> > inequality - such as in a loop condition testing I != E)
> They're not being removed. These functions still exist - it's just that now 
> they're being injected by the base class template with this exact signature 
> (rather than before where they were slightly different), so that now these 
> are redefinition issues. 
> 
> There's no loss of functionality here. 
Does LLVM still build fine in C++14/C++17 modes afterwards?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938

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

[PATCH] D88174: [Sema] Address-space sensitive check for unbounded arrays (v2)

[Chris Hamilton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd9ee935679e7: [Sema] Address-space sensitive check for 
unbounded arrays (v2) (authored by chrish_ericsson_atx).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88174

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/const-eval.c
  clang/test/Sema/unbounded-array-bounds.c
  clang/test/SemaCXX/constant-expression-cxx1y.cpp

Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -1018,8 +1018,9 @@
 }
 
 constexpr void PR28739(int n) { // expected-error {{never produces a constant}}
-  int *p = &n;
+  int *p = &n;  // expected-note {{declared here}}
   p += (__int128)(unsigned long)-1; // expected-note {{cannot refer to element 18446744073709551615 of non-array object in a constant expression}}
+  // expected-warning@-1 {{the pointer incremented by 18446744073709551615 refers past the last possible element for an array in 64-bit address space containing 32-bit (4-byte) elements (max possible 4611686018427387904 elements)}}
 }
 
 constexpr void Void(int n) {
Index: clang/test/Sema/unbounded-array-bounds.c
===
--- /dev/null
+++ clang/test/Sema/unbounded-array-bounds.c
@@ -0,0 +1,80 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-X86-ADDR64 %s  \
+// RUN:  --implicit-check-not 'past the last possible element'
+// RUN: %clang_cc1 -triple i386-pc-linux-gnu   -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-I386-ADDR32 %s \
+// RUN:  --implicit-check-not 'past the last possible element'
+// RUN: %clang_cc1 -triple avr-pc-linux-gnu-fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-AVR-ADDR16 %s  \
+// RUN:  --implicit-check-not 'past the last possible element'
+
+struct S {
+  long long a;
+  char b;
+  long long c;
+  short d;
+};
+
+struct S s[];
+
+void f1() {
+  ++s[3].a;
+  ++s[7073650413200313099].b;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: array index 7073650413200313099 refers past the last possible element for an array in 64-bit address space containing 256-bit (32-byte) elements (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  ++s[7073650].c;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+long long ll[];
+
+void f2() {
+  ++ll[3];
+  ++ll[2705843009213693952];
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 2305843009213693952 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 536870912 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 8192 elements)
+  ++ll[847073650];
+  // CHECK-I386-ADDR32: :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 536870912 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 8192 elements)
+}
+
+void f3(struct S p[]) {
+  ++p[3].a;
+  ++p[7073650413200313099].b;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  ++p[7073650].c;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+void f4(struct S *p) {
+  p += 3;
+  p += 7073650413200313099;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:3: warning: the pointer incremented by 7073650413200313099 refers past the last possible element for an array in 64-bit address space containing 256-bit (32-byte) elements (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:3: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK

[PATCH] D78938: Make LLVM build in C++20 mode

[Barry Revzin via Phabricator via cfe-commits]

BRevzin added inline comments.



Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h:167-171
-
-inline bool operator!=(const DWARFExpression::iterator &LHS,
-   const DWARFExpression::iterator &RHS) {
-  return !(LHS == RHS);
-}

lebedev.ri wrote:
> BRevzin wrote:
> > dblaikie wrote:
> > > Why are some being removed? That seems harder to justify. Even if they're 
> > > not called, it may be more valuable to have the symmetry to reduce 
> > > friction if/when they are needed. (iterators seem pretty common to 
> > > compare for inequality - such as in a loop condition testing I != E)
> > They're not being removed. These functions still exist - it's just that now 
> > they're being injected by the base class template with this exact signature 
> > (rather than before where they were slightly different), so that now these 
> > are redefinition issues. 
> > 
> > There's no loss of functionality here. 
> Does LLVM still build fine in C++14/C++17 modes afterwards?
Yes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938

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

[PATCH] D88260: [NFC][FE] Replace TypeSize with StorageUnitSize

[Jason Liu via Phabricator via cfe-commits]

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

LGTM.




Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1749-1750
 
-// Otherwise, allocate just the number of bytes required to store
-// the bitfield.
+  // Otherwise, allocate just the number of bytes required to store
+  // the bitfield.
 } else {

nit: drive-by fix? 
I think we should move this block of comments into the else block.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1768-1770
+// Otherwise, bump the data size up to include the bitfield,
+// including padding up to char alignment, and then remember how
+// bits we didn't use.

nit: same as the above driver-by fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88260

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

[PATCH] D88489: [clangd] Mark code action as "preferred" if it's the sole quickfix action

[Sam McCall via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88489

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/test/fixits-embed-in-diagnostic.test


Index: clang-tools-extra/clangd/test/fixits-embed-in-diagnostic.test
===
--- clang-tools-extra/clangd/test/fixits-embed-in-diagnostic.test
+++ clang-tools-extra/clangd/test/fixits-embed-in-diagnostic.test
@@ -28,6 +28,7 @@
 # CHECK-NEXT:]
 # CHECK-NEXT:  }
 # CHECK-NEXT:},
+# CHECK-NEXT:"isPreferred": true,
 # CHECK-NEXT:"kind": "quickfix",
 # CHECK-NEXT:"title": "change 'union' to 'struct'"
 # CHECK-NEXT:  }
Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -952,6 +952,13 @@
   /// The diagnostics that this code action resolves.
   llvm::Optional> diagnostics;
 
+  /// Marks this as a preferred action. Preferred actions are used by the
+  /// `auto fix` command and can be targeted by keybindings.
+  /// A quick fix should be marked preferred if it properly addresses the
+  /// underlying error. A refactoring should be marked preferred if it is the
+  /// most reasonable choice of actions to take.
+  bool isPreferred = false;
+
   /// The workspace edit this code action performs.
   llvm::Optional edit;
 
Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -740,6 +740,8 @@
 CodeAction["kind"] = *CA.kind;
   if (CA.diagnostics)
 CodeAction["diagnostics"] = llvm::json::Array(*CA.diagnostics);
+  if (CA.isPreferred)
+CodeAction["isPreferred"] = true;
   if (CA.edit)
 CodeAction["edit"] = *CA.edit;
   if (CA.command)
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -411,6 +411,8 @@
 Main.codeActions.emplace();
 for (const auto &Fix : D.Fixes)
   Main.codeActions->push_back(toCodeAction(Fix, File));
+if (Main.codeActions->size() == 1)
+  Main.codeActions->front().isPreferred = true;
   }
   if (Opts.SendDiagnosticCategory && !D.Category.empty())
 Main.category = D.Category;
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -1009,6 +1009,20 @@
 for (const auto &T : *Tweaks)
   Actions.push_back(toCodeAction(T, File, Selection));
 
+// If there's exactly one quick-fix, call it "preferred".
+// We never consider refactorings etc as preferred.
+CodeAction *OnlyFix = nullptr;
+for (auto &Action : Actions) {
+  if (Action.kind && *Action.kind == CodeAction::QUICKFIX_KIND) {
+if (OnlyFix) {
+  OnlyFix->isPreferred = false;
+  break;
+}
+Action.isPreferred = true;
+OnlyFix = &Action;
+  }
+}
+
 if (SupportsCodeAction)
   return Reply(llvm::json::Array(Actions));
 std::vector Commands;


Index: clang-tools-extra/clangd/test/fixits-embed-in-diagnostic.test
===
--- clang-tools-extra/clangd/test/fixits-embed-in-diagnostic.test
+++ clang-tools-extra/clangd/test/fixits-embed-in-diagnostic.test
@@ -28,6 +28,7 @@
 # CHECK-NEXT:]
 # CHECK-NEXT:  }
 # CHECK-NEXT:},
+# CHECK-NEXT:"isPreferred": true,
 # CHECK-NEXT:"kind": "quickfix",
 # CHECK-NEXT:"title": "change 'union' to 'struct'"
 # CHECK-NEXT:  }
Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -952,6 +952,13 @@
   /// The diagnostics that this code action resolves.
   llvm::Optional> diagnostics;
 
+  /// Marks this as a preferred action. Preferred actions are used by the
+  /// `auto fix` command and can be targeted by keybindings.
+  /// A quick fix should be marked preferred if it properly addresses the
+  /// underlying error. A refactoring should be marked pref

[clang-tools-extra] 4fb303f - [clangd] Improve PopulateSwitch tweak to work on non-empty switches

[Sam McCall via cfe-commits]

Author: Tadeo Kondrak
Date: 2020-09-29T16:37:51+02:00
New Revision: 4fb303f340e2c55783f9b0f3ed33fa2c36360acf

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

LOG: [clangd] Improve PopulateSwitch tweak to work on non-empty switches

Improve the recently-added PopulateSwitch tweak to work on non-empty switches.

Reviewed By: sammccall

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

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp 
b/clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
index e84a420f6218..753e8b4df826 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
@@ -33,12 +33,15 @@
 #include "AST.h"
 #include "Selection.h"
 #include "refactor/Tweak.h"
+#include "support/Logger.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/SmallSet.h"
+#include 
 #include 
 
 namespace clang {
@@ -52,18 +55,16 @@ class PopulateSwitch : public Tweak {
   Intent intent() const override { return Refactor; }
 
 private:
-  ASTContext *ASTCtx = nullptr;
   const DeclContext *DeclCtx = nullptr;
   const SwitchStmt *Switch = nullptr;
   const CompoundStmt *Body = nullptr;
+  const EnumType *EnumT = nullptr;
   const EnumDecl *EnumD = nullptr;
 };
 
 REGISTER_TWEAK(PopulateSwitch)
 
 bool PopulateSwitch::prepare(const Selection &Sel) {
-  ASTCtx = &Sel.AST->getASTContext();
-
   const SelectionTree::Node *CA = Sel.ASTSelection.commonAncestor();
   if (!CA)
 return false;
@@ -94,11 +95,6 @@ bool PopulateSwitch::prepare(const Selection &Sel) {
   if (!Body)
 return false;
 
-  // Since we currently always insert all enumerators, don't suggest this tweak
-  // if the body is not empty.
-  if (!Body->body_empty())
-return false;
-
   const Expr *Cond = Switch->getCond();
   if (!Cond)
 return false;
@@ -106,7 +102,7 @@ bool PopulateSwitch::prepare(const Selection &Sel) {
   // Ignore implicit casts, since enums implicitly cast to integer types.
   Cond = Cond->IgnoreParenImpCasts();
 
-  const EnumType *EnumT = Cond->getType()->getAsAdjusted();
+  EnumT = Cond->getType()->getAsAdjusted();
   if (!EnumT)
 return false;
 
@@ -114,21 +110,65 @@ bool PopulateSwitch::prepare(const Selection &Sel) {
   if (!EnumD)
 return false;
 
-  // If there aren't any enumerators, there's nothing to insert.
-  if (EnumD->enumerator_begin() == EnumD->enumerator_end())
-return false;
+  // We trigger if there are fewer cases than enum values (and no case covers
+  // multiple values). This guarantees we'll have at least one case to insert.
+  // We don't yet determine what the cases are, as that means evaluating
+  // expressions.
+  auto I = EnumD->enumerator_begin();
+  auto E = EnumD->enumerator_end();
+
+  for (const SwitchCase *CaseList = Switch->getSwitchCaseList();
+   CaseList && I != E; CaseList = CaseList->getNextSwitchCase(), I++) {
+// Default likely intends to cover cases we'd insert.
+if (isa(CaseList))
+  return false;
+
+const CaseStmt *CS = cast(CaseList);
+// Case statement covers multiple values, so just counting doesn't work.
+if (CS->caseStmtIsGNURange())
+  return false;
 
-  return true;
+// Case expression is not a constant expression or is value-dependent,
+// so we may not be able to work out which cases are covered.
+const ConstantExpr *CE = dyn_cast(CS->getLHS());
+if (!CE || CE->isValueDependent())
+  return false;
+  }
+
+  // Only suggest tweak if we have more enumerators than cases.
+  return I != E;
 }
 
 Expected PopulateSwitch::apply(const Selection &Sel) {
-  const SourceManager &SM = ASTCtx->getSourceManager();
+  ASTContext &Ctx = Sel.AST->getASTContext();
+
+  // Get the enum's integer width and signedness, for adjusting case literals.
+  unsigned EnumIntWidth = Ctx.getIntWidth(QualType(EnumT, 0));
+  bool EnumIsSigned = EnumT->isSignedIntegerOrEnumerationType();
+
+  llvm::SmallSet ExistingEnumerators;
+  for (const SwitchCase *CaseList = Switch->getSwitchCaseList(); CaseList;
+   CaseList = CaseList->getNextSwitchCase()) {
+const CaseStmt *CS = cast(CaseList);
+assert(!CS->caseStmtIsGNURange());
+const ConstantExpr *CE = cast(CS->getLHS());
+assert(!CE->isValueDependent());
+llvm::APSInt Val = CE->getResultAsAPSInt();
+Val = Val.extOrTrunc(EnumIntWidth);
+Val.setIsSigned(EnumIsSigned);
+ExistingEnumerators.in

[PATCH] D88434: [clangd] Improve PopulateSwitch tweak to work on non-empty switches

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4fb303f340e2: [clangd] Improve PopulateSwitch tweak to work 
on non-empty switches (authored by tdeo, committed by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88434

Files:
  clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.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
@@ -2829,9 +2829,48 @@
   "unavailable",
   },
   {
-  // Existing enumerators in switch
+  // All enumerators already in switch (unscoped)
   Function,
-  R""(enum Enum {A}; ^switch ((Enum)0) {case A:break;})"",
+  R""(enum Enum {A,B}; ^switch (A) {case A:break;case B:break;})"",
+  "unavailable",
+  },
+  {
+  // All enumerators already in switch (scoped)
+  Function,
+  R""(
+enum class Enum {A,B};
+^switch (Enum::A) {case Enum::A:break;case Enum::B:break;}
+  )"",
+  "unavailable",
+  },
+  {
+  // Default case in switch
+  Function,
+  R""(
+enum class Enum {A,B};
+^switch (Enum::A) {default:break;}
+  )"",
+  "unavailable",
+  },
+  {
+  // GNU range in switch
+  Function,
+  R""(
+enum class Enum {A,B};
+^switch (Enum::A) {case Enum::A ... Enum::B:break;}
+  )"",
+  "unavailable",
+  },
+  {
+  // Value dependent case expression
+  File,
+  R""(
+enum class Enum {A,B};
+template
+void function() {
+^switch (Enum::A) {case Value:break;}
+}
+  )"",
   "unavailable",
   },
   {
@@ -2867,9 +2906,53 @@
   {
   // Scoped enumeration with multiple enumerators
   Function,
-  R""(enum class Enum {A,B}; ^switch (Enum::A) {})"",
-  R""(enum class Enum {A,B}; )""
-  R""(switch (Enum::A) {case Enum::A:case Enum::B:break;})"",
+  R""(
+enum class Enum {A,B};
+^switch (Enum::A) {}
+  )"",
+  R""(
+enum class Enum {A,B};
+switch (Enum::A) {case Enum::A:case Enum::B:break;}
+  )"",
+  },
+  {
+  // Only filling in missing enumerators (unscoped)
+  Function,
+  R""(
+enum Enum {A,B,C};
+^switch (A) {case B:break;}
+  )"",
+  R""(
+enum Enum {A,B,C};
+switch (A) {case B:break;case A:case C:break;}
+  )"",
+  },
+  {
+  // Only filling in missing enumerators,
+  // even when using integer literals
+  Function,
+  R""(
+enum Enum {A,B=1,C};
+^switch (A) {case 1:break;}
+  )"",
+  R""(
+enum Enum {A,B=1,C};
+switch (A) {case 1:break;case A:case C:break;}
+  )"",
+  },
+  {
+  // Only filling in missing enumerators (scoped)
+  Function,
+  R""(
+enum class Enum {A,B,C};
+^switch (Enum::A)
+{case Enum::B:break;}
+  )"",
+  R""(
+enum class Enum {A,B,C};
+switch (Enum::A)
+{case Enum::B:break;case Enum::A:case Enum::C:break;}
+  )"",
   },
   {
   // Scoped enumerations in namespace
Index: clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
@@ -33,12 +33,15 @@
 #include "AST.h"
 #include "Selection.h"
 #include "refactor/Tweak.h"
+#include "support/Logger.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/SmallSet.h"
+#include 
 #include 
 
 namespace clang {
@@ -52,18 +55,16 @@
   Intent intent() const override { return Refactor; }
 
 private:
-  ASTContext *ASTCtx = nullptr;
   const DeclContext *DeclCtx = nullptr;
   const SwitchStmt *Switch = nullptr;
   const CompoundStmt *Body = nullptr;
+  const EnumType *EnumT = nullptr;
   const EnumDecl *EnumD = nullptr;
 };
 
 REGISTER_TWEAK(PopulateSwitch)
 
 bool PopulateSwitch::prepare(const Selection &Sel) {
-  ASTCtx = &Sel.AST->getASTContext();
-
   const SelectionTree::Node *CA = Sel.ASTSelection.commonAncestor();
   if (!C

[PATCH] D88491: [ASTContext] Use AllowCXX in all merge*Type methods, strip references

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert created this revision.
jdoerfert added a reviewer: aaron.ballman.
Herald added a subscriber: bollu.
Herald added a project: clang.
jdoerfert requested review of this revision.

Add and pass AllowCXX not only in mergeFunctionTypes but all merge*Type
methods. Use it to determine if reference types should be stripped or
are invalid in mergeTypes. The reason for this is that
mergeFunctionTypes allows CXX functions with exception specifiers but
crashes on reference types (in the return or argument position). This is
going to be used by D88384 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88491

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp

Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -9179,13 +9179,15 @@
 /// QualType()
 QualType ASTContext::mergeTransparentUnionType(QualType T, QualType SubType,
bool OfBlockPointer,
-   bool Unqualified) {
+   bool Unqualified,
+   bool AllowCXX) {
   if (const RecordType *UT = T->getAsUnionType()) {
 RecordDecl *UD = UT->getDecl();
 if (UD->hasAttr()) {
   for (const auto *I : UD->fields()) {
 QualType ET = I->getType().getUnqualifiedType();
-QualType MT = mergeTypes(ET, SubType, OfBlockPointer, Unqualified);
+QualType MT = mergeTypes(ET, SubType, OfBlockPointer, Unqualified,
+ /* BlockReturnType */ false, AllowCXX);
 if (!MT.isNull())
   return MT;
   }
@@ -9199,21 +9201,23 @@
 /// parameter types
 QualType ASTContext::mergeFunctionParameterTypes(QualType lhs, QualType rhs,
  bool OfBlockPointer,
- bool Unqualified) {
+ bool Unqualified,
+ bool AllowCXX) {
   // GNU extension: two types are compatible if they appear as a function
   // argument, one of the types is a transparent union type and the other
   // type is compatible with a union member
   QualType lmerge = mergeTransparentUnionType(lhs, rhs, OfBlockPointer,
-  Unqualified);
+  Unqualified, AllowCXX);
   if (!lmerge.isNull())
 return lmerge;
 
   QualType rmerge = mergeTransparentUnionType(rhs, lhs, OfBlockPointer,
-  Unqualified);
+  Unqualified, AllowCXX);
   if (!rmerge.isNull())
 return rmerge;
 
-  return mergeTypes(lhs, rhs, OfBlockPointer, Unqualified);
+  return mergeTypes(lhs, rhs, OfBlockPointer, Unqualified,
+/* BlockReturnType */ false, AllowCXX);
 }
 
 QualType ASTContext::mergeFunctionTypes(QualType lhs, QualType rhs,
@@ -9234,11 +9238,11 @@
 bool UnqualifiedResult = Unqualified;
 if (!UnqualifiedResult)
   UnqualifiedResult = (!RHS.hasQualifiers() && LHS.hasQualifiers());
-retType = mergeTypes(LHS, RHS, true, UnqualifiedResult, true);
-  }
-  else
+retType = mergeTypes(LHS, RHS, true, UnqualifiedResult,
+ /* BlockReturnType */ true, AllowCXX);
+  } else
 retType = mergeTypes(lbase->getReturnType(), rbase->getReturnType(), false,
- Unqualified);
+ Unqualified, /* BlockReturnType */ false, AllowCXX);
   if (retType.isNull())
 return {};
 
@@ -9323,7 +9327,7 @@
   QualType lParamType = lproto->getParamType(i).getUnqualifiedType();
   QualType rParamType = rproto->getParamType(i).getUnqualifiedType();
   QualType paramType = mergeFunctionParameterTypes(
-  lParamType, rParamType, OfBlockPointer, Unqualified);
+  lParamType, rParamType, OfBlockPointer, Unqualified, AllowCXX);
   if (paramType.isNull())
 return {};
 
@@ -9416,9 +9420,18 @@
   return {};
 }
 
-QualType ASTContext::mergeTypes(QualType LHS, QualType RHS,
-bool OfBlockPointer,
-bool Unqualified, bool BlockReturnType) {
+QualType ASTContext::mergeTypes(QualType LHS, QualType RHS, bool OfBlockPointer,
+bool Unqualified, bool BlockReturnType,
+bool AllowCXX) {
+  // If AllowCXX is specifically set we strip reference types on demand.
+  assert((AllowCXX ||
+  (!LHS->getAs() && !RHS->getAs())) &&
+ "C++ references should have been stripped");
+  if (const ReferenceType *lRT = LHS->getAs())
+LHS = lRT->getPointeeType();
+  if (const ReferenceType *rRT = RHS->getAs())
+RHS = rRT->getPointee

[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library

[Axel Y. Rivera via Phabricator via cfe-commits]

ayrivera added inline comments.



Comment at: lld/ELF/Driver.cpp:895
   const char *argv[] = {config->progName.data(), opt.data()};
+  cl::ResetAllOptionOccurrences();
   if (cl::ParseCommandLineOptions(2, argv, "", &os))

aganea wrote:
> ayrivera wrote:
> > MaskRay wrote:
> > > ayrivera wrote:
> > > > Hi,
> > > > 
> > > > I built locally lld and came across an issue. The ELF driver isn't 
> > > > recognizing multiple -mllvm options. It only process the last -mllvm 
> > > > option that was entered in the command line. For example, I can add 
> > > > -mllvm -print-after-all -debug-pass=Arguments and it only prints the 
> > > > information from -debug-pass=Arguments, and not the IR and the debug 
> > > > data. If I swap the order, then the IR is printed and not the debug 
> > > > information.
> > > > 
> > > > I noticed that this change set added a call to 
> > > > cl:ResetAllOptionOccurrences() (line 895) inside parseClangOption. The 
> > > > function parseClangOption is called inside the loop that process the 
> > > > -mllvm options. If I comment this line, then everything works correctly 
> > > > and I can pass multiple -mllvm options through the command.
> > > > 
> > > > Is this an intended behavior or an actual issue? Thanks for your time.
> > > Hi,
> > > 
> > > > For example, I can add -mllvm -print-after-all -debug-pass=Arguments
> > > 
> > > You need two -mllvm, i.e. `-mllvm -print-after-all -mllvm 
> > > -debug-pass=Arguments`. And with the option, I see the print-after-all 
> > > dump.
> > Hi,
> > 
> > 
> > > You need two -mllvm, i.e. -mllvm -print-after-all -mllvm 
> > > -debug-pass=Arguments. And with the option, I see the print-after-all 
> > > dump.
> > 
> > 
> > Thanks for the quick reply. Sorry, this is a typo. I'm using both -mllvm 
> > and still got the issue.
> > Is this an intended behavior or an actual issue?
> At first sight, this is an actual issue. Normally, 
> `cl::ResetAllOptionOccurrences()` should be called only once per compiler or 
> linker instance, to reset the internal counters for the `cl::opt`s. I would 
> move it somewhere at the begining of `readConfigs()`. Can you try that see if 
> that solves your issue?
Hi @aganea ,

This patch https://reviews.llvm.org/D88461 , posted by @MaskRay , works. It 
does what you suggested. Thanks for the quick reply.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70378

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

[PATCH] D88329: [objc] Consolidate ObjC name mangle code to AST

[Ellis Hoag via Phabricator via cfe-commits]

ellis updated this revision to Diff 295000.
ellis added a comment.

[objc] Fix memory leak in CGObjCMac.cpp

CGObjCMac.cpp was leaking a MangleContext everytime it mangled an ObjC method. 
We now have an instance variable that allocates and deallocates the context.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88329

Files:
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Mangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/CodeGen/CGObjCMac.cpp

Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -20,6 +20,7 @@
 #include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclObjC.h"
+#include "clang/AST/Mangle.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/Basic/CodeGenOptions.h"
@@ -924,13 +925,6 @@
 
   llvm::StringMap NSConstantStringMap;
 
-  /// GetNameForMethod - Return a name for the given method.
-  /// \param[out] NameOut - The return value.
-  void GetNameForMethod(const ObjCMethodDecl *OMD,
-const ObjCContainerDecl *CD,
-SmallVectorImpl &NameOut,
-bool ignoreCategoryNamespace = false);
-
   /// GetMethodVarName - Return a unique constant for the given
   /// selector's name. The return value has type char *.
   llvm::Constant *GetMethodVarName(Selector Sel);
@@ -1086,7 +1080,7 @@
 
 public:
   CGObjCCommonMac(CodeGen::CodeGenModule &cgm) :
-CGObjCRuntime(cgm), VMContext(cgm.getLLVMContext()) { }
+CGObjCRuntime(cgm), VMContext(cgm.getLLVMContext()), Mangler(cgm.getContext().createMangleContext()) { }
 
   bool isNonFragileABI() const {
 return ObjCABI == 2;
@@ -1127,6 +1121,7 @@
 
 private:
   void fillRunSkipBlockVars(CodeGenModule &CGM, const CGBlockInfo &blockInfo);
+  std::unique_ptr Mangler;
 };
 
 namespace {
@@ -4008,7 +4003,9 @@
 Method = GenerateDirectMethod(OMD, CD);
   } else {
 SmallString<256> Name;
-GetNameForMethod(OMD, CD, Name);
+llvm::raw_svector_ostream OS(Name);
+Mangler->mangleObjCMethodName(OMD, OS, /*includePrefixByte=*/true,
+ /*includeCategoryNamespace=*/true);
 
 CodeGenTypes &Types = CGM.getTypes();
 llvm::FunctionType *MethodTy =
@@ -4061,7 +4058,9 @@
 I->second = Fn;
   } else {
 SmallString<256> Name;
-GetNameForMethod(OMD, CD, Name, /*ignoreCategoryNamespace*/ true);
+llvm::raw_svector_ostream OS(Name);
+Mangler->mangleObjCMethodName(OMD, OS, /*includePrefixByte=*/true,
+ /*includeCategoryNamespace=*/false);
 
 Fn = llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
 Name.str(), &CGM.getModule());
@@ -5715,21 +5714,6 @@
   return GetPropertyName(&CGM.getContext().Idents.get(TypeStr));
 }
 
-void CGObjCCommonMac::GetNameForMethod(const ObjCMethodDecl *D,
-   const ObjCContainerDecl *CD,
-   SmallVectorImpl &Name,
-   bool ignoreCategoryNamespace) {
-  llvm::raw_svector_ostream OS(Name);
-  assert (CD && "Missing container decl in GetNameForMethod");
-  OS << '\01' << (D->isInstanceMethod() ? '-' : '+')
- << '[' << CD->getName();
-  if (!ignoreCategoryNamespace)
-if (const ObjCCategoryImplDecl *CID =
-dyn_cast(D->getDeclContext()))
-  OS << '(' << *CID << ')';
-  OS << ' ' << D->getSelector().getAsString() << ']';
-}
-
 void CGObjCMac::FinishModule() {
   EmitModuleInfo();
 
Index: clang/lib/AST/MicrosoftMangle.cpp
===
--- clang/lib/AST/MicrosoftMangle.cpp
+++ clang/lib/AST/MicrosoftMangle.cpp
@@ -1323,7 +1323,7 @@
 }
 
 void MicrosoftCXXNameMangler::mangleObjCMethodName(const ObjCMethodDecl *MD) {
-  Context.mangleObjCMethodName(MD, Out);
+  Context.mangleObjCMethodNameAsSourceName(MD, Out);
 }
 
 void MicrosoftCXXNameMangler::mangleTemplateInstantiationName(
Index: clang/lib/AST/Mangle.cpp
===
--- clang/lib/AST/Mangle.cpp
+++ clang/lib/AST/Mangle.cpp
@@ -175,7 +175,7 @@
   const TargetInfo &TI = Context.getTargetInfo();
   if (CC == CCM_Other || (MCXX && TI.getCXXABI() == TargetCXXABI::Microsoft)) {
 if (const ObjCMethodDecl *OMD = dyn_cast(D))
-  mangleObjCMethodName(OMD, Out);
+  mangleObjCMethodNameAsSourceName(OMD, Out);
 else
   mangleCXXName(GD, Out);
 return;
@@ -192,7 +192,7 @@
   if (!MCXX)
 Out << D->getIdentifier()->getName();
   else if (const ObjCMethodDecl *OMD = dyn_cast(D))
-mangleObjCMethodName(OMD, Out);
+mangleObjCMethodNameAsSourceName(OMD, Out);
   else
 mangleCXXName(GD, Out);
 
@@ -275,7 +275,7 @@
   

[PATCH] D88329: [objc] Consolidate ObjC name mangle code to AST

[Ellis Hoag via Phabricator via cfe-commits]

ellis added a comment.

Oops, I meant to create a new commit rather than amend to this one


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88329

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

[PATCH] D88495: [clangd] Disable msan instrumentation for generated Evaluate().

[Utkarsh Saxena via Phabricator via cfe-commits]

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

MSAN build times out for generated DecisionForest inference runtime.

A solution worth trying is splitting the function into 300 smaller
functions and then re-enable msan.

For now we are disabling instrumentation for the generated function.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88495

Files:
  clang-tools-extra/clangd/quality/CompletionModelCodegen.py


Index: clang-tools-extra/clangd/quality/CompletionModelCodegen.py
===
--- clang-tools-extra/clangd/quality/CompletionModelCodegen.py
+++ clang-tools-extra/clangd/quality/CompletionModelCodegen.py
@@ -145,6 +145,7 @@
 return """#ifndef %s
 #define %s
 #include 
+#include "llvm/Support/Compiler.h"
 
 %s
 class %s {
@@ -160,6 +161,7 @@
   friend float Evaluate(const %s&);
 };
 
+LLVM_NO_SANITIZE("memory")
 float Evaluate(const %s&);
 %s
 #endif // %s


Index: clang-tools-extra/clangd/quality/CompletionModelCodegen.py
===
--- clang-tools-extra/clangd/quality/CompletionModelCodegen.py
+++ clang-tools-extra/clangd/quality/CompletionModelCodegen.py
@@ -145,6 +145,7 @@
 return """#ifndef %s
 #define %s
 #include 
+#include "llvm/Support/Compiler.h"
 
 %s
 class %s {
@@ -160,6 +161,7 @@
   friend float Evaluate(const %s&);
 };
 
+LLVM_NO_SANITIZE("memory")
 float Evaluate(const %s&);
 %s
 #endif // %s
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88495: [clangd] Disable msan instrumentation for generated Evaluate().

[Adam Czachorowski via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/quality/CompletionModelCodegen.py:164
 
+LLVM_NO_SANITIZE("memory")
 float Evaluate(const %s&);

Please add a comment here explaining why this is disabled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88495

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

[PATCH] D88359: [analyzer][RFC] Complete rewrite of ArrayBoundCheckerV2

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

balazske added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:61
+
+  return NeedsExtraBitToPreserveSigness ? Signed : ExtendedSigned;
+}

Should not the `ExtendedSigned` be in the true branch?



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:87
+/// stored value.
+static APSInt absWithoutWrapping(const llvm::APSInt &Value) {
+  // If unsigned, we might need a sign bit.

I do not like these function names. `getCommonSignedTypeToFit`, 
`isRepresentableBySigned`, `getAbsWithoutWrapping` is better to have the common 
"start verbs" in the names.



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:241
+  const APSInt &ExclusiveUpperBound)
+  : LastValidState(std::move(State)), SVB(SVB), RootSymbol(RootSymbol) {
+assert(LastValidState);

std::move is not needed here



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:269
+  /// Starts the simplification process.
+  SimplificationResult Simplify() {
+LLVM_DEBUG(llvm::dbgs() << __func__ << ": initially: '";

Function name should start with lower case.



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:285
+   LastValidState->printJson(llvm::dbgs()); llvm::dbgs() << '\n';);
+return {std::move(LastValidState), std::move(LowerBound),
+std::move(UpperBound), SymbolVal(RootSymbol), 
SimplificationFailed};

The moves here are not needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88359

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

[PATCH] D88495: [clangd] Disable msan instrumentation for generated Evaluate().

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 updated this revision to Diff 295005.
usaxena95 added a comment.

Addressed comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88495

Files:
  clang-tools-extra/clangd/quality/CompletionModelCodegen.py


Index: clang-tools-extra/clangd/quality/CompletionModelCodegen.py
===
--- clang-tools-extra/clangd/quality/CompletionModelCodegen.py
+++ clang-tools-extra/clangd/quality/CompletionModelCodegen.py
@@ -145,6 +145,7 @@
 return """#ifndef %s
 #define %s
 #include 
+#include "llvm/Support/Compiler.h"
 
 %s
 class %s {
@@ -160,6 +161,9 @@
   friend float Evaluate(const %s&);
 };
 
+// The function may have large number of lines of code. MSAN
+// build times out in such case.
+LLVM_NO_SANITIZE("memory")
 float Evaluate(const %s&);
 %s
 #endif // %s


Index: clang-tools-extra/clangd/quality/CompletionModelCodegen.py
===
--- clang-tools-extra/clangd/quality/CompletionModelCodegen.py
+++ clang-tools-extra/clangd/quality/CompletionModelCodegen.py
@@ -145,6 +145,7 @@
 return """#ifndef %s
 #define %s
 #include 
+#include "llvm/Support/Compiler.h"
 
 %s
 class %s {
@@ -160,6 +161,9 @@
   friend float Evaluate(const %s&);
 };
 
+// The function may have large number of lines of code. MSAN
+// build times out in such case.
+LLVM_NO_SANITIZE("memory")
 float Evaluate(const %s&);
 %s
 #endif // %s
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88497: [objc] Fix memory leak in CGObjCMac.cpp

[Ellis Hoag via Phabricator via cfe-commits]

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

A memory leak was introduced in https://reviews.llvm.org/D88329

CGObjCMac.cpp was leaking a MangleContext everytime it mangled
an ObjC method. We now have an instance variable that allocates
and deallocates the context.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88497

Files:
  clang/lib/CodeGen/CGObjCMac.cpp


Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -1079,8 +1079,9 @@
   void EmitImageInfo();
 
 public:
-  CGObjCCommonMac(CodeGen::CodeGenModule &cgm) :
-CGObjCRuntime(cgm), VMContext(cgm.getLLVMContext()) { }
+  CGObjCCommonMac(CodeGen::CodeGenModule &cgm)
+  : CGObjCRuntime(cgm), VMContext(cgm.getLLVMContext()),
+Mangler(cgm.getContext().createMangleContext()) {}
 
   bool isNonFragileABI() const {
 return ObjCABI == 2;
@@ -1121,6 +1122,7 @@
 
 private:
   void fillRunSkipBlockVars(CodeGenModule &CGM, const CGBlockInfo &blockInfo);
+  std::unique_ptr Mangler;
 };
 
 namespace {
@@ -4003,9 +4005,8 @@
   } else {
 SmallString<256> Name;
 llvm::raw_svector_ostream OS(Name);
-const auto &MC = CGM.getContext().createMangleContext();
-MC->mangleObjCMethodName(OMD, OS, /*includePrefixByte=*/true,
- /*includeCategoryNamespace=*/true);
+Mangler->mangleObjCMethodName(OMD, OS, /*includePrefixByte=*/true,
+  /*includeCategoryNamespace=*/true);
 
 CodeGenTypes &Types = CGM.getTypes();
 llvm::FunctionType *MethodTy =
@@ -4059,9 +4060,8 @@
   } else {
 SmallString<256> Name;
 llvm::raw_svector_ostream OS(Name);
-const auto &MC = CGM.getContext().createMangleContext();
-MC->mangleObjCMethodName(OMD, OS, /*includePrefixByte=*/true,
- /*includeCategoryNamespace=*/false);
+Mangler->mangleObjCMethodName(OMD, OS, /*includePrefixByte=*/true,
+  /*includeCategoryNamespace=*/false);
 
 Fn = llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
 Name.str(), &CGM.getModule());


Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -1079,8 +1079,9 @@
   void EmitImageInfo();
 
 public:
-  CGObjCCommonMac(CodeGen::CodeGenModule &cgm) :
-CGObjCRuntime(cgm), VMContext(cgm.getLLVMContext()) { }
+  CGObjCCommonMac(CodeGen::CodeGenModule &cgm)
+  : CGObjCRuntime(cgm), VMContext(cgm.getLLVMContext()),
+Mangler(cgm.getContext().createMangleContext()) {}
 
   bool isNonFragileABI() const {
 return ObjCABI == 2;
@@ -1121,6 +1122,7 @@
 
 private:
   void fillRunSkipBlockVars(CodeGenModule &CGM, const CGBlockInfo &blockInfo);
+  std::unique_ptr Mangler;
 };
 
 namespace {
@@ -4003,9 +4005,8 @@
   } else {
 SmallString<256> Name;
 llvm::raw_svector_ostream OS(Name);
-const auto &MC = CGM.getContext().createMangleContext();
-MC->mangleObjCMethodName(OMD, OS, /*includePrefixByte=*/true,
- /*includeCategoryNamespace=*/true);
+Mangler->mangleObjCMethodName(OMD, OS, /*includePrefixByte=*/true,
+  /*includeCategoryNamespace=*/true);
 
 CodeGenTypes &Types = CGM.getTypes();
 llvm::FunctionType *MethodTy =
@@ -4059,9 +4060,8 @@
   } else {
 SmallString<256> Name;
 llvm::raw_svector_ostream OS(Name);
-const auto &MC = CGM.getContext().createMangleContext();
-MC->mangleObjCMethodName(OMD, OS, /*includePrefixByte=*/true,
- /*includeCategoryNamespace=*/false);
+Mangler->mangleObjCMethodName(OMD, OS, /*includePrefixByte=*/true,
+  /*includeCategoryNamespace=*/false);
 
 Fn = llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
 Name.str(), &CGM.getModule());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88329: [objc] Consolidate ObjC name mangle code to AST

[Ellis Hoag via Phabricator via cfe-commits]

ellis added a comment.

https://reviews.llvm.org/D88497 will fix the leak


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88329

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

[PATCH] D78938: Make LLVM build in C++20 mode

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: llvm/include/llvm/IR/BasicBlock.h:324-325
+template ::value>>
 phi_iterator_impl(const phi_iterator_impl &Arg)

dblaikie wrote:
> Quuxplusone wrote:
> > BRevzin wrote:
> > > dblaikie wrote:
> > > > BRevzin wrote:
> > > > > dblaikie wrote:
> > > > > > What tripped over/required this SFINAE?
> > > > > There's somewhere which compared a const iterator to a non-const 
> > > > > iterator, that ends up doing conversions in both directions under 
> > > > > C++20 rules, one direction of which is perfectly fine and the other 
> > > > > was a hard error. Need to make the non-const iterator not 
> > > > > constructible from a const iterator.
> > > > Is this true for all iterators? Or some quirk of how this one is 
> > > > written/used (that could be fixed/changed there instead)?
> > > So I undid this change to copy the exact issue that I ran into. But it 
> > > actually ended up still compiling anyway. Part of the issue might be that 
> > > I keep futzing with the cmake configuration since it takes me more than 
> > > an hour to compile, so maybe there's some target that needed this change 
> > > that I no longer compile.
> > > 
> > > But the kind of problem I think this had was:
> > > 
> > > ```
> > > template 
> > > struct iterator {
> > > T* p;
> > > 
> > > template 
> > > iterator(iterator rhs)
> > > : p(rhs.p)
> > > { } 
> > > 
> > > bool operator==(iterator const& rhs);
> > > };
> > > 
> > > bool check(iterator a, iterator b) {
> > > return a == b;
> > > }
> > > ```
> > > 
> > > which compiles fine in C++17 but is ambiguous in C++20 because 
> > > `b.operator==(a)` is also a candidate (even though it's not _really_ a 
> > > candidate, and would be a hard error if selected). the sfinae removes the 
> > > bad candidate from the set. 
> > > 
> > > It's true for all iterators in general in that you want `const_iterator` 
> > > constructible from `iterator` but not the reverse (unless they're the 
> > > same type).
> > IMO there is a (much) bigger task hiding here, which is to audit every type 
> > in the codebase whose name contains the string "Iterator" and compare them 
> > to the C++20 Ranges `std::forward_iterator` concept. My impression is that 
> > the vast majority of real-world "iterator types" are not iterators 
> > according to C++20 Ranges, and that this can have arbitrarily weird effects 
> > when you mix them with the C++20 STL.
> > 
> > However, that is //massive// scope creep re this particular patch. I think 
> > the larger question of "do all our iterators need X / are all our iterators 
> > written wrong" should be scoped-outside-of this patch.
> Sorry, not suggesting that kind of scope creep - but do want to understand 
> whether this is representative of the way code should generally be written, 
> or whether this is working around some other issue/different fix.
Fair enough - don't mind keeping it in then.



Comment at: llvm/include/llvm/Support/BinaryStreamRef.h:124
 
-  bool operator==(const RefType &Other) const {
-if (BorrowedImpl != Other.BorrowedImpl)
+  friend bool operator==(const RefType &Self, const RefType &Other) {
+if (Self.BorrowedImpl != Other.BorrowedImpl)

dblaikie wrote:
> Quuxplusone wrote:
> > Is there a neat rule of thumb for when you were able to preserve the member 
> > `bool Me::operator==(const Me& rhs) const` versus when you were forced to 
> > change it to a hidden friend? It seems like maybe you changed it to a 
> > hidden friend only in cases where `Me` was a base class, is that right?
> Be curious of the answer here - and, honestly, I'd be fine with changing them 
> all to friends. It makes them more consistent - equal rank for implicit 
> conversions on LHS and RHS, etc. (generally considered best practice 
> basically to not define op overloads as members if they can be defined as 
> non-members)
Ping on this (& I'd usually call the parameters LHS and RHS rather than Self 
and Other)



Comment at: llvm/unittests/ADT/STLExtrasTest.cpp:465
   // Check fancy pointer overload for unique_ptr
+  // Parenthesizing to_address to avoid ADL finding std::to_address
   std::unique_ptr V2 = std::make_unique(0);

dblaikie wrote:
> jhenderson wrote:
> > Nit: trailing full stop.
> Probably more suitable to use qualify the name rather than use parens (teh 
> comment's still helpful to explain why either strategy is used) - that's 
> what's done with llvm::make_unique, for instance.
Ping on this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938

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

[PATCH] D88495: [clangd] Disable msan instrumentation for generated Evaluate().

[Utkarsh Saxena via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa9f63d22fafb: [clangd] Disable msan instrumentation for 
generated Evaluate(). (authored by usaxena95).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88495

Files:
  clang-tools-extra/clangd/quality/CompletionModelCodegen.py


Index: clang-tools-extra/clangd/quality/CompletionModelCodegen.py
===
--- clang-tools-extra/clangd/quality/CompletionModelCodegen.py
+++ clang-tools-extra/clangd/quality/CompletionModelCodegen.py
@@ -145,6 +145,7 @@
 return """#ifndef %s
 #define %s
 #include 
+#include "llvm/Support/Compiler.h"
 
 %s
 class %s {
@@ -160,6 +161,9 @@
   friend float Evaluate(const %s&);
 };
 
+// The function may have large number of lines of code. MSAN
+// build times out in such case.
+LLVM_NO_SANITIZE("memory")
 float Evaluate(const %s&);
 %s
 #endif // %s


Index: clang-tools-extra/clangd/quality/CompletionModelCodegen.py
===
--- clang-tools-extra/clangd/quality/CompletionModelCodegen.py
+++ clang-tools-extra/clangd/quality/CompletionModelCodegen.py
@@ -145,6 +145,7 @@
 return """#ifndef %s
 #define %s
 #include 
+#include "llvm/Support/Compiler.h"
 
 %s
 class %s {
@@ -160,6 +161,9 @@
   friend float Evaluate(const %s&);
 };
 
+// The function may have large number of lines of code. MSAN
+// build times out in such case.
+LLVM_NO_SANITIZE("memory")
 float Evaluate(const %s&);
 %s
 #endif // %s
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] a9f63d2 - [clangd] Disable msan instrumentation for generated Evaluate().

[Utkarsh Saxena via cfe-commits]

Author: Utkarsh Saxena
Date: 2020-09-29T17:44:10+02:00
New Revision: a9f63d22fafb0d7de768efc6b7447f8e7f6bb220

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

LOG: [clangd] Disable msan instrumentation for generated Evaluate().

MSAN build times out for generated DecisionForest inference runtime.

A solution worth trying is splitting the function into 300 smaller
functions and then re-enable msan.

For now we are disabling instrumentation for the generated function.

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

Added: 


Modified: 
clang-tools-extra/clangd/quality/CompletionModelCodegen.py

Removed: 




diff  --git a/clang-tools-extra/clangd/quality/CompletionModelCodegen.py 
b/clang-tools-extra/clangd/quality/CompletionModelCodegen.py
index 20bfccd8806f..423e5d14cf52 100644
--- a/clang-tools-extra/clangd/quality/CompletionModelCodegen.py
+++ b/clang-tools-extra/clangd/quality/CompletionModelCodegen.py
@@ -145,6 +145,7 @@ class can be used to represent a code completion candidate.
 return """#ifndef %s
 #define %s
 #include 
+#include "llvm/Support/Compiler.h"
 
 %s
 class %s {
@@ -160,6 +161,9 @@ class %s {
   friend float Evaluate(const %s&);
 };
 
+// The function may have large number of lines of code. MSAN
+// build times out in such case.
+LLVM_NO_SANITIZE("memory")
 float Evaluate(const %s&);
 %s
 #endif // %s



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

[clang] 1192747 - NFC, add a missing stdlib include for the use of abort

[Alex Lorenz via cfe-commits]

Author: Alex Lorenz
Date: 2020-09-29T08:50:51-07:00
New Revision: 119274748bce6d1248aa57cb55d79bfeae8a2f8e

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

LOG: NFC, add a missing stdlib include for the use of abort

The FatalErrorHandler.cpp file uses 'abort', but doesn't include
'stdlib.h'. This causes a build error when modules are used in clang.

Added: 


Modified: 
clang/tools/libclang/FatalErrorHandler.cpp

Removed: 




diff  --git a/clang/tools/libclang/FatalErrorHandler.cpp 
b/clang/tools/libclang/FatalErrorHandler.cpp
index 6b435fcfcc95..ef21569637f0 100644
--- a/clang/tools/libclang/FatalErrorHandler.cpp
+++ b/clang/tools/libclang/FatalErrorHandler.cpp
@@ -9,6 +9,7 @@
 
 #include "clang-c/FatalErrorHandler.h"
 #include "llvm/Support/ErrorHandling.h"
+#include 
 
 static void aborting_fatal_error_handler(void *, const std::string &reason,
  bool) {



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

[PATCH] D88498: [FPEnv] Evaluate initializers in constant rounding mode

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff created this revision.
sepavloff added reviewers: rsmith, rjmccall, mibintc.
Herald added a project: clang.
sepavloff requested review of this revision.

Previously initializers were evaluated using rounding mode currently
specified by Sema. If at the same time FP exception behavior was set
to `strict`, compiler failed to compile the following C code:

  struct S { float f; };
  static struct S x = {0.63};

This happened because setting strict behavior set rounding mode to
`dymanic`. In this case the value of initializer depends on the current
rounding mode and it cannot be evaluated in compile time.

Initializers for some objects must be constants. On the other hand using
`dynamic` as rounding mode make no sense. Even in C++ if the initializer
is evaluated dynamically, it happens before execution of `main`, so
there is no possibility to set rounding mode for such initializers. So
if rounding mode is `dynamic` when an initializer is evaluated in some
cases, compiler replaces it with default `tonearest` rounding.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88498

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDecl.cpp
  clang/test/AST/const-fpfeatures-diag.c
  clang/test/AST/const-fpfeatures-strict.c

Index: clang/test/AST/const-fpfeatures-strict.c
===
--- /dev/null
+++ clang/test/AST/const-fpfeatures-strict.c
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -S -emit-llvm -ffp-exception-behavior=strict -Wno-unknown-pragmas %s -o - | FileCheck %s
+
+// nextUp(1.F) == 0x1.02p0F
+
+struct S {
+  float f;
+};
+
+static struct S var_01 = {0x1.01p0};
+struct S *func_01() {
+  return &var_01;
+}
+
+struct S var_02 = {0x1.01p0};
+
+struct S *func_03() {
+  static struct S var_03 = {0x1.01p0};
+  return &var_03;
+}
+
+// CHECK: @var_01 = {{.*}} %struct.S { float 1.00e+00 }
+// CHECK: @var_02 = {{.*}} %struct.S { float 1.00e+00 }
+// CHECK: @func_03.var_03 = {{.*}} %struct.S { float 1.00e+00 }
+
+#pragma STDC FENV_ROUND FE_UPWARD
+
+static struct S var_04 = {0x1.01p0};
+struct S *func_04() {
+  return &var_04;
+}
+
+struct S var_05 = {0x1.01p0};
+
+struct S *func_06() {
+  static struct S var_06 = {0x1.01p0};
+  return &var_06;
+}
+
+// CHECK: @var_04 = {{.*}} %struct.S { float 0x3FF02000 }
+// CHECK: @var_05 = {{.*}} %struct.S { float 0x3FF02000 }
+// CHECK: @func_06.var_06 = {{.*}} %struct.S { float 0x3FF02000 }
Index: clang/test/AST/const-fpfeatures-diag.c
===
--- clang/test/AST/const-fpfeatures-diag.c
+++ /dev/null
@@ -1,10 +0,0 @@
-// RUN: %clang_cc1 -verify -ffp-exception-behavior=strict -Wno-unknown-pragmas %s
-
-// REQUIRES: x86-registered-target
-
-#pragma STDC FENV_ROUND FE_DYNAMIC
-
-// nextUp(1.F) == 0x1.02p0F
-
-float F1 = 0x1.00p0F + 0x0.02p0F;
-float F2 = 0x1.00p0F + 0x0.01p0F; // expected-error{{initializer element is not a compile-time constant}}
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -2281,6 +2281,26 @@
   }
 
   PreferredType.enterVariableInit(Tok.getLocation(), ThisDecl);
+
+  Sema::FPFeaturesStateRAII FPO(Actions);
+  if (auto VD = dyn_cast_or_null(ThisDecl))
+if (!VD->isInvalidDecl()) {
+  // If variable requires constant initialization, set constant
+  // rounding mode.
+  if (VD->isFileVarDecl() || VD->isConstexpr() ||
+  (!getLangOpts().CPlusPlus && VD->isStaticLocal())) {
+if (Actions.getCurFPFeatures().getRoundingMode() ==
+llvm::RoundingMode::Dynamic) {
+  // If constant rounding mode is 'dynamic', it means that a command
+  // line option line like `-ffpmodel=strict` is in effect. Set
+  // constant rounding to default in this case.
+  FPOptions NewFPO = Actions.getCurFPFeatures();
+  NewFPO.setRoundingMode(llvm::RoundingMode::NearestTiesToEven);
+  Actions.setCurFPFeatures(NewFPO);
+}
+  }
+}
+
   ExprResult Init = ParseInitializer();
 
   // If this is the only decl in (possibly) range based for statement,
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -589,13 +589,7 @@
   // This stack tracks the current state of Sema.CurFPFeatures.
   PragmaStack FpPragmaStack;
   FPOptionsOverride CurFPFeatureOverrides() {
-FPOptionsOverride result;
-if (!FpPragmaStack.hasValue()) {
-  result = FPOptionsOverride();
-} else {
-  result = FpPragmaStack.CurrentValue;
-}
-return result;
+return FpPragmaStack.CurrentValue;
   }

[PATCH] D88462: FP math settings for static duration initialization - work in progress

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

Please look at D88498 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88462

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

[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

In D88477#2300687 , @martong wrote:

>> In this example, it cast to the `unsigned char` (which is the type of the 
>> stored value of `**b`, stored at `#1`) instead of the static type of the 
>> access (the type of `**b` is  `char`at `#2`).
>
> Possible typo in the summary. At `#2` the type should be `char *` shouldn't 
> it?

Yup, thanks. Updated revision summary accordingly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88477

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

[PATCH] D86841: [clang] Add mustprogress and llvm.loop.mustprogress attribute deduction

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.



Comment at: clang/lib/CodeGen/CGStmt.cpp:801
+ getLangOpts().CPlusPlus11 || getLangOpts().CPlusPlus14 ||
+ getLangOpts().CPlusPlus17 || getLangOpts().C2x) {
+MustProgress = true;

Also in C? And C2x in the end is probably CPLusPlus2x?



Comment at: clang/lib/CodeGen/CGStmt.cpp:894
+else if (C->isOne())
+  IsMustProgress = false;
+  } else if (getLangOpts().C11 || getLangOpts().C17 || getLangOpts().C2x ||

Maybe call this FnIsMustProgress or the other one LoopMustProgress.



Comment at: clang/lib/CodeGen/CGStmt.cpp:898
+ getLangOpts().CPlusPlus17 || getLangOpts().C2x)
+MustProgress = true;
+

same as above



Comment at: clang/lib/CodeGen/CGStmt.cpp:946
+   getLangOpts().CPlusPlus17 || getLangOpts().C2x) &&
+  (!S.getCond() || !S.getCond()->EvaluateAsInt(Result, getContext(
+MustProgress = true;

same as above.

Don't we have to update IsMustProgress here too?




Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1165
+IsMustProgress = true;
+  }
+

no braces. why the mustprogress check?



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1172
+
+  if (IsMustProgress)
+CurFn->addFnAttr(llvm::Attribute::MustProgress);

Say that we do this late because we need to see the body.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86841

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

[PATCH] D88359: [analyzer][RFC] Complete rewrite of ArrayBoundCheckerV2

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

Thank you for your time @balazske!
Your catches were really valuable.




Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:61
+
+  return NeedsExtraBitToPreserveSigness ? Signed : ExtendedSigned;
+}

balazske wrote:
> Should not the `ExtendedSigned` be in the true branch?
You are right. Thanks.



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:87
+/// stored value.
+static APSInt absWithoutWrapping(const llvm::APSInt &Value) {
+  // If unsigned, we might need a sign bit.

balazske wrote:
> I do not like these function names. `getCommonSignedTypeToFit`, 
> `isRepresentableBySigned`, `getAbsWithoutWrapping` is better to have the 
> common "start verbs" in the names.
You are probably right about that.



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:241
+  const APSInt &ExclusiveUpperBound)
+  : LastValidState(std::move(State)), SVB(SVB), RootSymbol(RootSymbol) {
+assert(LastValidState);

balazske wrote:
> std::move is not needed here
But definitely not hurt :D
I quite like to use `move` on by-value parameters.
This way the implementation could omit the increment decrement even if the 
implementation currently doesn't exploit this.



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:269
+  /// Starts the simplification process.
+  SimplificationResult Simplify() {
+LLVM_DEBUG(llvm::dbgs() << __func__ << ": initially: '";

balazske wrote:
> Function name should start with lower case.
Thanks.



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:285
+   LastValidState->printJson(llvm::dbgs()); llvm::dbgs() << '\n';);
+return {std::move(LastValidState), std::move(LowerBound),
+std::move(UpperBound), SymbolVal(RootSymbol), 
SimplificationFailed};

balazske wrote:
> The moves here are not needed.
I'm not convinced about that.
I think without move we would do a copy there - which could be expensive in the 
case of `APSInt`s.
Here is an example demonstrating this: https://godbolt.org/z/E5dqWb
I know that currently APSInt does not support move operations, but I already 
plan adding that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88359

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

[PATCH] D88498: [FPEnv] Evaluate initializers in constant rounding mode

[Melanie Blower via Phabricator via cfe-commits]

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

thanks for this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88498

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

[PATCH] D88462: FP math settings for static duration initialization - work in progress

[Melanie Blower via Phabricator via cfe-commits]

mibintc abandoned this revision.
mibintc added a comment.

refer to https://reviews.llvm.org/D88498


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88462

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

[PATCH] D88500: [AIX][Clang][Driver] Link libm along with libc++

[David Tenty via Phabricator via cfe-commits]

daltenty created this revision.
daltenty added reviewers: hubert.reinterpretcast, DiggerLin.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
daltenty requested review of this revision.

since libc++ has dependencies on libm.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88500

Files:
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/test/Driver/aix-ld.c

Index: clang/test/Driver/aix-ld.c
===
--- clang/test/Driver/aix-ld.c
+++ clang/test/Driver/aix-ld.c
@@ -20,6 +20,7 @@
 // CHECK-LD32: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD32-NOT: "-lc++"
 // CHECK-LD32: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-NOT: "-lm"
 // CHECK-LD32: "-lc"
 
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit.
@@ -41,6 +42,7 @@
 // CHECK-LD64: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD64-NOT: "-lc++"
 // CHECK-LD64: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
+// CHECK-LD64-NOT: "-lm"
 // CHECK-LD64: "-lc"
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit. Enable POSIX thread support.
@@ -64,6 +66,7 @@
 // CHECK-LD32-PTHREAD-NOT: "-lc++"
 // CHECK-LD32-PTHREAD: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
 // CHECK-LD32-PTHREAD: "-lpthreads"
+// CHECK-LD32-PTHREAD-NOT: "-lm"
 // CHECK-LD32-PTHREAD: "-lc"
 
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit. POSIX thread alias.
@@ -87,6 +90,7 @@
 // CHECK-LD64-PTHREAD-NOT: "-lc++"
 // CHECK-LD64-PTHREAD: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
 // CHECK-LD64-PTHREAD: "-lpthreads"
+// CHECK-LD64-PTHREAD-NOT: "-lm"
 // CHECK-LD64-PTHREAD: "-lc"
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit. Enable profiling.
@@ -109,6 +113,7 @@
 // CHECK-LD32-PROF: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD32-PROF-NOT: "-lc++"
 // CHECK-LD32-PROF: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-PROF-NOT: "-lm"
 // CHECK-LD32-PROF: "-lc"
 
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit. Enable g-profiling.
@@ -131,6 +136,7 @@
 // CHECK-LD64-GPROF: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD64-GPROF-NOT: "-lc++"
 // CHECK-LD64-GPROF: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
+// CHECK-LD64-GPROF-NOT: "-lm"
 // CHECK-LD64-GPROF: "-lc"
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit. Static linking.
@@ -153,6 +159,7 @@
 // CHECK-LD32-STATIC: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD32-STATIC-NOT: "-lc++"
 // CHECK-LD32-STATIC: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-STATIC-NOT: "-lm"
 // CHECK-LD32-STATIC: "-lc"
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit. Library search path.
@@ -176,6 +183,7 @@
 // CHECK-LD32-LIBP: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD32-LIBP-NOT: "-lc++"
 // CHECK-LD32-LIBP: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-LIBP-NOT: "-lm"
 // CHECK-LD32-LIBP: "-lc"
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit. nostdlib.
@@ -200,6 +208,7 @@
 // CHECK-LD32-NO-STD-LIB-NOT: "-lc++"
 // CHECK-LD32-NO-STD-LIB-NOT: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
 // CHECK-LD32-NO-STD-LIB-NOT: "-lpthreads"
+// CHECK-LD32-NO-STD-LIB-NOT: "-lm"
 // CHECK-LD32-NO-STD-LIB-NOT: "-lc"
 
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit. nodefaultlibs.
@@ -224,6 +233,7 @@
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: "-lc++"
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: "-lpthreads"
+// CHECK-LD64-NO-DEFAULT-LIBS-NOT: "-lm"
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: "-lc"
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit. 'bcdtors' and argument order.
@@ -247,6 +257,7 @@
 // CHECK-LD32-CXX-ARG-ORDER-NOT: "-bcdtors:all:0:s"
 // CHECK-LD32-CXX-ARG-ORDER: "-lc++"
 // CHECK-LD32-CXX-ARG-ORDER: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-CXX-ARG-ORDER: "-lm"
 // CHECK-LD32-CXX-ARG-ORDER: "-lc"
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit. lc++ and lc order.
@@ -266,6 +277,7 @@
 // CHECK-LD32-CXX-ARG-LCXX: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD32-CXX-ARG-LCXX: "-lc++"
 // CHECK-LD32-CXX-ARG-LCXX: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-CXX-ARG-LCXX: "-lm"
 // CHECK-LD32-CXX-ARG-LCXX: "-lc"
 
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit. lc++ and lc order.
@@ -285,6 +297,7 @@
 // CHECK-LD64-CXX-ARG-LCXX: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD64-CXX-ARG-LCXX: "-lc++"
 // CHECK-LD64-CXX-ARG-LCXX: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
+// CHECK-LD64-CXX-ARG-LCXX: "-lm"
 /

[PATCH] D88497: [objc] Fix memory leak in CGObjCMac.cpp

[John McCall via Phabricator via cfe-commits]

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

Oh, oops, I should've caught this in review.  I assume you still need a commit?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88497

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

[PATCH] D88497: [objc] Fix memory leak in CGObjCMac.cpp

[Ellis Hoag via Phabricator via cfe-commits]

ellis added a comment.

Yes please commit for me :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88497

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

[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

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

Szelethus added a comment.

In D72705#2274568 , @balazske wrote:

> I am not sure if this checker is worth to further development. A part of the 
> found bugs can be detected with other checkers too, specially if the 
> preconditions of many standard functions are checked (more than what is done 
> now) and by modeling the function calls with assumed error return value.

We shouldn't throw all the knowledge away, once its in our hands. What you're 
suggesting as an alternative is using state splits, something that is obviously 
expensive, and we can't know whether that state split is always just / 
constructive.

---

Since we had private meetings about this, it would be great to share the 
overall vision going forward. While we shouldn't burden @NoQ with having to 
review this checker line-by-line, he might have valuable input on what we're 
going for.

I'd like to ask you to write a mail on cfe-dev, and bring forward a formal 
proposal:

- Explain ERR33-C, and your interpretation of it. Highlight how you want to see 
very explicit error checking against the concrete return value in some cases, 
and not in others. Provide a list of functions where you think, and why you 
think that vigorous checking is appropriate, and a list for those where a more 
lenient one could be used.
- Explain why you picked symbolic execution to solve this problem.
- Explain how this problem could be solved, and the pros/cons of them:
  - Making state splits in StdLibraryFunctionChecker
  - Expanding on taint analysis
  - Check constraints on a return value in a new checker (this is impossible, 
sure, but why?)
  - GDM tracking of values to be checked in a new checker
- You picked no4 -- justify it.

Since you made considerable progress already, it would be great to have talk 
about how you are implementing this, and why. For instance, "I recognize 
appropriate function in checkPostCall, and mark their return value as unchecked 
in the GDM. Later, in checkLocation, if I find that such a value is read from, 
I'll analyze the surrounding code to find out whether its a rule-compliant 
comparison of it, if it is not, I emit a warning. This is done by ascending the 
AST with a ParentMap, and running a statement visitor on the parent statement". 
Code examples are always amazing, demonstrate on a piece of code how 
`if(someReturnValue != 0)`, or something similar would be analyzed.

---

I hope I didn't sounds condescending or demanding, it was definitely not my 
intent. Its a joy to see your works come to life, there are a lot of smarts to 
marvel in! With that said, the reviewing process has shown some significant 
problems.

D71510  was submitted 10 months ago -- you 
obviously didn't work on it non-stop, but the fact that some pivotal aspects of 
your solution was realized by me only a month ago despite doing my best to 
follow the development is very unfortunate indeed. I'm not sure anyone else 
realized the extent of it either. Changes on StreamChecker followed a very 
similar trend, I felt like I had to understand, explain, and prove the 
correctness of the change for myself. As I made progress on it, sometimes the 
patches were split up, reuploaded and abandoned, like a multi-headed hydra. We 
ought to draw some lessons from this.

As an author, you can make the reviewers job a lot easier by:

- If the change is big enough, have a round on the mailing list //before// 
writing too much code. Scouting ahead is fine, but its not worth going too far.
- Provide a thorough summary with examples, drawings (D70411#1754356 
), whatever that helps to convey the 
message. If the code you're changing took considerable time to understand, it 
will probably take considerable time for reviewers as well. D86874 
, D55566 , 
D86135  and D86736 
 are great examples. D63954 
 keeps it short, but provides a link to more 
discussions.
- For a new checker, upload a very slimmed down version of your prototype. Its 
okay if it has FP/FN problems, or crashes on some weird input, its alpha for a 
reason. Its rare that for demonstration purposes you really need 500+ LOC. This 
allows everyone to discuss followup steps as well (though I don't think you 
ever left me in the dark regarding that).
- If a high level objection/question rose, its not worth adding new features, 
or even updating it much, unless it directly clarifies some trivial 
misunderstanding.
- When you publish results, give your input on them. Are you satisfied with the 
findings? Are there notable false positives? Could they be fixed easily?
- Laugh at how I used to do none of these, again, because its funny 
(D45532#1083670 ).

There are a lot of lessons to be taken on my

[PATCH] D88265: Fix comma with half vectors.

[Alexandre Rames via Phabricator via cfe-commits]

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



Comment at: clang/test/Sema/fp16vec-sema.c:29
   sv0 = hv0 >= hv1;
+  hv0, 1; // expected-warning 2 {{expression result unused}}
   sv0 = hv0 || hv1; // expected-error{{logical expression with vector types 
'half4' (vector of 4 '__fp16' values) and 'half4' is only supported in C++}}

fhahn wrote:
> nit: it might be slightly better to move it 2 lines further down, so all 
> operators that assign the result are grouped together. Also, should we add 
> `1, hv0` as well?
Moved.
I added `1, hv0` for completeness. It would hit the same path as the reverse.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88265

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

[clang-tools-extra] d8ba6b4 - [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines

[Aleksandr Platonov via cfe-commits]

Author: Aleksandr Platonov
Date: 2020-09-29T19:54:55+03:00
New Revision: d8ba6b4ab3eceb6bbcdf4371d4ffaab9d1a5cebe

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

LOG: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines

As @kadircet mentions in D84912#2184144, `findNearbyIdentifier()` traverses the 
whole file if there is no identifier for the word.
This patch ensures give up after 2^N lines in any case.

Reviewed By: sammccall

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

Added: 


Modified: 
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/XRefs.cpp 
b/clang-tools-extra/clangd/XRefs.cpp
index 9e8791c2a765..9532e1418cca 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -562,19 +562,34 @@ const syntax::Token *findNearbyIdentifier(const 
SpelledWord &Word,
   auto Cost = [&](SourceLocation Loc) -> unsigned {
 assert(SM.getFileID(Loc) == File && "spelled token in wrong file?");
 unsigned Line = SM.getSpellingLineNumber(Loc);
-if (Line > WordLine)
-  return 1 + llvm::Log2_64(Line - WordLine);
-if (Line < WordLine)
-  return 2 + llvm::Log2_64(WordLine - Line);
-return 0;
+return Line >= WordLine ? Line - WordLine : 2 * (WordLine - Line);
   };
   const syntax::Token *BestTok = nullptr;
-  // Search bounds are based on word length: 2^N lines forward.
-  unsigned BestCost = Word.Text.size() + 1;
+  unsigned BestCost = -1;
+  // Search bounds are based on word length:
+  // - forward: 2^N lines
+  // - backward: 2^(N-1) lines.
+  unsigned MaxDistance =
+  1U << std::min(Word.Text.size(),
+   std::numeric_limits::digits - 1);
+  // Line number for SM.translateLineCol() should be one-based, also
+  // SM.translateLineCol() can handle line number greater than
+  // number of lines in the file.
+  // - LineMin = max(1, WordLine + 1 - 2^(N-1))
+  // - LineMax = WordLine + 1 + 2^N
+  unsigned LineMin =
+  WordLine + 1 <= MaxDistance / 2 ? 1 : WordLine + 1 - MaxDistance / 2;
+  unsigned LineMax = WordLine + 1 + MaxDistance;
+  SourceLocation LocMin = SM.translateLineCol(File, LineMin, 1);
+  assert(LocMin.isValid());
+  SourceLocation LocMax = SM.translateLineCol(File, LineMax, 1);
+  assert(LocMax.isValid());
 
   // Updates BestTok and BestCost if Tok is a good candidate.
   // May return true if the cost is too high for this token.
   auto Consider = [&](const syntax::Token &Tok) {
+if (Tok.location() < LocMin || Tok.location() > LocMax)
+  return true; // we are too far from the word, break the outer loop.
 if (!(Tok.kind() == tok::identifier && Tok.text(SM) == Word.Text))
   return false;
 // No point guessing the same location we started with.

diff  --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp 
b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index a48bb9c8c182..40637b5fa758 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1428,6 +1428,11 @@ TEST(LocateSymbol, NearbyIdentifier) {
 
 
   // h^i
+
+
+
+
+  int x = hi;
 )cpp",
   R"cpp(
   // prefer nearest occurrence even if several matched tokens



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

[PATCH] D88265: Fix comma with half vectors.

[Alexandre Rames via Phabricator via cfe-commits]

arames updated this revision to Diff 295025.
arames marked an inline comment as done.
arames added a comment.

Addressed review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88265

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/fp16vec-sema.c


Index: clang/test/Sema/fp16vec-sema.c
===
--- clang/test/Sema/fp16vec-sema.c
+++ clang/test/Sema/fp16vec-sema.c
@@ -28,6 +28,8 @@
   sv0 = hv0 >= hv1;
   sv0 = hv0 || hv1; // expected-error{{logical expression with vector types 
'half4' (vector of 4 '__fp16' values) and 'half4' is only supported in C++}}
   sv0 = hv0 && hv1; // expected-error{{logical expression with vector types 
'half4' (vector of 4 '__fp16' values) and 'half4' is only supported in C++}}
+  hv0, 1; // expected-warning 2 {{expression result unused}}
+  1, hv0; // expected-warning 2 {{expression result unused}}
 
   // Implicit conversion between half vectors and float vectors are not 
allowed.
   hv0 = fv0; // expected-error{{assigning to}}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -13933,9 +13933,10 @@
   // float vectors and truncating the result back to half vector. For now, we 
do
   // this only when HalfArgsAndReturn is set (that is, when the target is arm 
or
   // arm64).
-  assert(isVector(RHS.get()->getType(), Context.HalfTy) ==
- isVector(LHS.get()->getType(), Context.HalfTy) &&
- "both sides are half vectors or neither sides are");
+  assert(
+  (Opc == BO_Comma || isVector(RHS.get()->getType(), Context.HalfTy) ==
+  isVector(LHS.get()->getType(), Context.HalfTy)) 
&&
+  "both sides are half vectors or neither sides are");
   ConvertHalfVec =
   needsConversionOfHalfVec(ConvertHalfVec, Context, LHS.get(), RHS.get());
 


Index: clang/test/Sema/fp16vec-sema.c
===
--- clang/test/Sema/fp16vec-sema.c
+++ clang/test/Sema/fp16vec-sema.c
@@ -28,6 +28,8 @@
   sv0 = hv0 >= hv1;
   sv0 = hv0 || hv1; // expected-error{{logical expression with vector types 'half4' (vector of 4 '__fp16' values) and 'half4' is only supported in C++}}
   sv0 = hv0 && hv1; // expected-error{{logical expression with vector types 'half4' (vector of 4 '__fp16' values) and 'half4' is only supported in C++}}
+  hv0, 1; // expected-warning 2 {{expression result unused}}
+  1, hv0; // expected-warning 2 {{expression result unused}}
 
   // Implicit conversion between half vectors and float vectors are not allowed.
   hv0 = fv0; // expected-error{{assigning to}}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -13933,9 +13933,10 @@
   // float vectors and truncating the result back to half vector. For now, we do
   // this only when HalfArgsAndReturn is set (that is, when the target is arm or
   // arm64).
-  assert(isVector(RHS.get()->getType(), Context.HalfTy) ==
- isVector(LHS.get()->getType(), Context.HalfTy) &&
- "both sides are half vectors or neither sides are");
+  assert(
+  (Opc == BO_Comma || isVector(RHS.get()->getType(), Context.HalfTy) ==
+  isVector(LHS.get()->getType(), Context.HalfTy)) &&
+  "both sides are half vectors or neither sides are");
   ConvertHalfVec =
   needsConversionOfHalfVec(ConvertHalfVec, Context, LHS.get(), RHS.get());
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D87822: [FPEnv] Evaluate constant expressions under non-default rounding modes

[Leonard Chan via cfe-commits]

Thanks for looking into it. We have that commit but it still seems to be
failing for us with the same error.

On Tue, Sep 29, 2020 at 12:58 AM Serge Pavlov  wrote:

> Hi!
>
> This issue must be fixed by: https://reviews.llvm.org/rGf91b9c0f9858
> Do you have recent changes from the trunk?
>
> Thanks,
> --Serge
>
>
> On Tue, Sep 29, 2020 at 4:22 AM Leonard Chan via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
>> leonardchan added a comment.
>>
>> Hi! It looks like this is causing a test failure on our aach64 builders (
>> https://luci-milo.appspot.com/p/fuchsia/builders/ci/clang-linux-arm64/b8868095822628984976?
>> ):
>>
>>   [1113/1114] Running the Clang regression tests
>>   llvm-lit:
>> /b/s/w/ir/k/staging/llvm_build/bin/../../../llvm-project/llvm/utils/lit/lit/llvm/config.py:379:
>> note: using clang: /b/s/w/ir/k/staging/llvm_build/bin/clang
>>   -- Testing: 26708 tests, 256 workers --
>>   Testing:
>>   FAIL: Clang :: AST/const-fpfeatures-diag.c (269 of 26708)
>>    TEST 'Clang :: AST/const-fpfeatures-diag.c' FAILED
>> 
>>   Script:
>>   --
>>   : 'RUN: at line 1';   /b/s/w/ir/k/staging/llvm_build/bin/clang -cc1
>> -internal-isystem /b/s/w/ir/k/staging/llvm_build/lib/clang/12.0.0/include
>> -nostdsysteminc -verify -ffp-exception-behavior=strict -Wno-unknown-pragmas
>> /b/s/w/ir/k/llvm-project/clang/test/AST/const-fpfeatures-diag.c
>>   --
>>   Exit Code: 1
>>
>>   Command Output (stderr):
>>   --
>>   error: 'error' diagnostics expected but not seen:
>> File /b/s/w/ir/k/llvm-project/clang/test/AST/const-fpfeatures-diag.c
>> Line 8: initializer element is not a compile-time constant
>>   error: 'warning' diagnostics seen but not expected:
>> (frontend): overriding currently unsupported use of floating point
>> exceptions on this target
>>   2 errors generated.
>>
>>   --
>>
>>   
>>   Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
>>   
>>   Failed Tests (1):
>> Clang :: AST/const-fpfeatures-diag.c
>>
>> Would you mind taking a look and sending out a fix? Thanks.
>>
>>
>> Repository:
>>   rG LLVM Github Monorepo
>>
>> CHANGES SINCE LAST ACTION
>>   https://reviews.llvm.org/D87822/new/
>>
>> https://reviews.llvm.org/D87822
>>
>>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87822: [FPEnv] Evaluate constant expressions under non-default rounding modes

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Commented in that review, but that patch has the wrong fix: it's based on the 
targets LLVM is configured with rather than the current test target.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87822

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

[PATCH] D88498: [FPEnv] Evaluate initializers in constant rounding mode

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

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



Comment at: clang/lib/Parse/ParseDecl.cpp:2290
+  // rounding mode.
+  if (VD->isFileVarDecl() || VD->isConstexpr() ||
+  (!getLangOpts().CPlusPlus && VD->isStaticLocal())) {

It's far from clear to me that this is correct in C++. In principle, for a 
dynamic initializer, the rounding mode could have been set by an earlier 
initializer.

Perhaps we can make an argument that, due to the permission to interleave 
initializers from different TUs, every dynamic initializer must leave the 
program in the default rounding mode, but I don't think even that makes this 
approach correct, because an initializer could do this:

```
double d;
double e = (fesetround(...), d = some calculation, fesetround(...default...), 
d);
```

I think we can only do this in C and will need something different for C++.

(I think this also has problems in C++ due to constexpr functions: it's not the 
case that all floating point operations that will be evaluated as part of the 
initializer lexically appear within it.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88498

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

[PATCH] D87528: Enable '#pragma STDC FENV_ACCESS' in frontend cf. D69272 - Work in Progress

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Richard and I had a long conversation about this further up-thread, if you 
missed it.


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

https://reviews.llvm.org/D87528

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

[PATCH] D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX closed this revision.
ArcsinX added a comment.

Don't know why this didn't close automatically
Commit: https://reviews.llvm.org/rGd8ba6b4ab3eceb6bbcdf4371d4ffaab9d1a5cebe


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87891

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

[PATCH] D88430: [OpenMP] Replace OpenMP RTL Functions With OMPIRBuilder and OMPKinds.def

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 295032.
jhuber6 added a reviewer: erichkeane.
jhuber6 added a comment.

Adding test cases for incompatible architecture messages. Checking the 
architecture is done by checking all combinations of architectures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88430

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/OpenMP/nvptx_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
  clang/test/OpenMP/target_incompatible_architecture_messages.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/test/Transforms/OpenMP/add_attributes.ll

Index: llvm/test/Transforms/OpenMP/add_attributes.ll
===
--- llvm/test/Transforms/OpenMP/add_attributes.ll
+++ llvm/test/Transforms/OpenMP/add_attributes.ll
@@ -888,313 +888,313 @@
 ; CHECK: declare dso_local i32 @omp_pause_resource_all(i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare dso_local i32 @omp_get_supported_active_levels() #0
+; CHECK-NEXT: declare dso_local i32 @omp_get_supported_active_levels()
 
-; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_barrier(%struct.ident_t*, i32) #0
+; CHECK: ; Function Attrs: convergent nounwind
+; CHECK-NEXT: declare void @__kmpc_barrier(%struct.ident_t*, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare i32 @__kmpc_cancel(%struct.ident_t*, i32, i32) #0
+; CHECK-NEXT: declare i32 @__kmpc_cancel(%struct.ident_t*, i32, i32)
 
-; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare i32 @__kmpc_cancel_barrier(%struct.ident_t*, i32) #0
+; CHECK: ; Function Attrs: convergent nounwind
+; CHECK-NEXT: declare i32 @__kmpc_cancel_barrier(%struct.ident_t*, i32)
 
-; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_flush(%struct.ident_t*) #0
+; CHECK: ; Function Attrs: convergent nounwind
+; CHECK-NEXT: declare void @__kmpc_flush(%struct.ident_t*)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare i32 @__kmpc_global_thread_num(%struct.ident_t*) #0
+; CHECK-NEXT: declare i32 @__kmpc_global_thread_num(%struct.ident_t*)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_fork_call(%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) #0
+; CHECK-NEXT: declare void @__kmpc_fork_call(%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...)
 
-; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare i32 @__kmpc_omp_taskwait(%struct.ident_t*, i32) #0
+; CHECK: ; Function Attrs: convergent nounwind
+; CHECK-NEXT: declare i32 @__kmpc_omp_taskwait(%struct.ident_t*, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare i32 @__kmpc_omp_taskyield(%struct.ident_t*, i32, i32) #0
+; CHECK-NEXT: declare i32 @__kmpc_omp_taskyield(%struct.ident_t*, i32, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_push_num_threads(%struct.ident_t*, i32, i32) #0
+; CHECK-NEXT: declare void @__kmpc_push_num_threads(%struct.ident_t*, i32, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_push_proc_bind(%struct.ident_t*, i32, i32) #0
+; CHECK-NEXT: declare void @__kmpc_push_proc_bind(%struct.ident_t*, i32, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_serialized_parallel(%struct.ident_t*, i32) #0
+; CHECK-NEXT: declare void @__kmpc_serialized_parallel(%struct.ident_t*, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_end_serialized_parallel(%struct.ident_t*, i32) #0
+; CHECK-NEXT: declare void @__kmpc_end_serialized_parallel(%struct.ident_t*, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare i32 @__kmpc_master(%struct.ident_t*, i32) #0
+; CHECK-NEXT: declare i32 @__kmpc_master(%struct.ident_t*, i32)
 
 ; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_end_master(%struct.ident_t*, i32) #0
+; CHECK-NEXT: declare void @__kmpc_end_master(%struct.ident_t*, i32)
 
-; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_critical(%struct.ident_t*, i32, [8 x i32]*) #0
+; CHECK: ; Function Attrs: convergent nounwind
+; CHECK-NEXT: declare void @__kmpc_critical(%struct.ident_t*, i32, [8 x i32]*)
 
-; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_critical_with_hint(%struct.ident_t*, i32, [8 x i32]*, i32) #0
+; CHECK: ; Function Attrs: convergent nounwind
+; CHECK-NEXT: declare void @__kmpc_critical_with_hint(%struct.ident_t*, i32, [8 x i32]*, i32)
 
-; CHECK: ; Function Attrs: nounwind
-; CHECK-NEXT: declare void @__kmpc_end_critical(%struct.ident_t*, i32, [8 x i32]*) #0
+; CHECK: ; Function Attrs: convergent nounwind
+; CHECK-NEXT: declare void @__kmp

[PATCH] D88430: [OpenMP] Replace OpenMP RTL Functions With OMPIRBuilder and OMPKinds.def

[Erich Keane via Phabricator via cfe-commits]

erichkeane resigned from this revision.
erichkeane added a comment.

I'm not really a good person to review this, OMP isn't nearly my expertise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88430

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

[PATCH] D87822: [FPEnv] Evaluate constant expressions under non-default rounding modes

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

In D87822#2301194 , @leonardchan wrote:

> Thanks for looking into it. We have that commit but it still seems to be
> failing for us with the same error.

In D88498  this test is removed because use of 
rounding mode changed. So you could just remove this test to fix the bot.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87822

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

[PATCH] D87737: Add -fprofile-update={atomic,prefer-atomic,single}

[Fangrui Song via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3681be876fea: Add 
-fprofile-update={atomic,prefer-atomic,single} (authored by MaskRay).

Changed prior to commit:
  https://reviews.llvm.org/D87737?vs=294781&id=295042#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87737

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/code-coverage-tsan.c
  clang/test/CodeGen/tsan-instrprof-atomic.c
  clang/test/Driver/fprofile-update.c

Index: clang/test/Driver/fprofile-update.c
===
--- /dev/null
+++ clang/test/Driver/fprofile-update.c
@@ -0,0 +1,15 @@
+/// For -fprofile-instr-generate and -fprofile-arcs, increment counters atomically
+/// if -fprofile-update={atomic,prefer-atomic} or -fsanitize=thread is specified.
+// RUN: %clang -### %s -c -target x86_64-linux -fsanitize=thread %s 2>&1 | FileCheck %s
+// RUN: %clang -### %s -c -fprofile-update=atomic 2>&1 | FileCheck %s
+// RUN: %clang -### %s -c -fprofile-update=prefer-atomic 2>&1 | FileCheck %s
+
+// CHECK: "-fprofile-update=atomic"
+
+// RUN: %clang -### %s -c -fprofile-update=atomic -fprofile-update=single 2>&1 | FileCheck %s --check-prefix=SINGLE
+
+// SINGLE-NOT: "-fprofile-update=atomic"
+
+// RUN: not %clang %s -c -fprofile-update=unknown 2>&1 | FileCheck %s --check-prefix=ERROR
+
+// ERROR: error: unsupported argument 'unknown' to option 'fprofile-update='
Index: clang/test/CodeGen/tsan-instrprof-atomic.c
===
--- clang/test/CodeGen/tsan-instrprof-atomic.c
+++ clang/test/CodeGen/tsan-instrprof-atomic.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -emit-llvm -fprofile-instrument=clang -fsanitize=thread -o - | FileCheck %s
+// RUN: %clang_cc1 %s -emit-llvm -fprofile-instrument=clang -fprofile-update=atomic -o - | FileCheck %s
 
 // CHECK: define {{.*}}@foo
 // CHECK-NOT: load {{.*}}foo
Index: clang/test/CodeGen/code-coverage-tsan.c
===
--- clang/test/CodeGen/code-coverage-tsan.c
+++ clang/test/CodeGen/code-coverage-tsan.c
@@ -1,11 +1,12 @@
-/// -fsanitize=thread requires the (potentially concurrent) counter updates to be atomic.
-// RUN: %clang_cc1 %s -triple x86_64 -emit-llvm -fsanitize=thread -femit-coverage-notes -femit-coverage-data \
+/// -fprofile-update=atomic (implied by -fsanitize=thread) requires the
+/// (potentially concurrent) counter updates to be atomic.
+// RUN: %clang_cc1 %s -triple x86_64 -emit-llvm -fprofile-update=atomic -femit-coverage-notes -femit-coverage-data \
 // RUN:   -coverage-notes-file /dev/null -coverage-data-file /dev/null -o - | FileCheck %s
 
 // CHECK-LABEL: void @foo()
 /// Two counters are incremented by __tsan_atomic64_fetch_add.
-// CHECK: call i64 @__tsan_atomic64_fetch_add
-// CHECK-NEXT:call i32 @__tsan_atomic32_fetch_sub
+// CHECK: atomicrmw add i64* {{.*}} @__llvm_gcov_ctr
+// CHECK-NEXT:atomicrmw sub i32*
 
 _Atomic(int) cnt;
 void foo() { cnt--; }
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -884,6 +884,7 @@
   Opts.DebugRangesBaseAddress = Args.hasArg(OPT_fdebug_ranges_base_address);
 
   setPGOInstrumentor(Opts, Args, Diags);
+  Opts.AtomicProfileUpdate = Args.hasArg(OPT_fprofile_update_EQ);
   Opts.InstrProfileOutput =
   std::string(Args.getLastArgValue(OPT_fprofile_instrument_path_EQ));
   Opts.ProfileInstrumentUsePath =
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -868,6 +868,17 @@
 CmdArgs.push_back(Args.MakeArgString(Twine("-fprofile-filter-files=" + v)));
   }
 
+  if (const auto *A = Args.getLastArg(options::OPT_fprofile_update_EQ)) {
+StringRef Val = A->getValue();
+if (Val == "atomic" || Val == "prefer-atomic")
+  CmdArgs.push_back("-fprofile-update=atomic");
+else if (Val != "single")
+  D.Diag(diag::err_drv_unsupported_option_argument)
+  << A->getOption().getName() << Val;
+  } else if (TC.getSanitizerArgs().needsTsanRt()) {
+CmdArgs.push_back("-fprofile-update=atomic");
+  }
+
   // Leave -fprofile-dir= an unused argument unless .gcda emission is
   // enabled. To be polite, with '-fprofile-arcs -fno-profile-arcs' consider
   // the flag used. There is no -fno-profile-dir, so the user has no
Index: clang/lib/CodeGen/BackendUtil.cpp
===

[clang] 3681be8 - Add -fprofile-update={atomic,prefer-atomic,single}

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2020-09-29T10:43:23-07:00
New Revision: 3681be876fea9b270c7a1d2dc41679a399610e06

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

LOG: Add -fprofile-update={atomic,prefer-atomic,single}

GCC 7 introduced -fprofile-update={atomic,prefer-atomic} (prefer-atomic is for
best efforts (some targets do not support atomics)) to increment counters
atomically, which is exactly what we have done with -fprofile-instr-generate
(D50867) and -fprofile-arcs (b5ef137c11b1cc6ae839ee75b49233825772bdd0).
This patch adds the option to clang to surface the internal options at driver 
level.

GCC 7 also turned on -fprofile-update=prefer-atomic when -pthread is specified,
but it has performance regression
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89307). So we don't follow suit.

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

Added: 
clang/test/Driver/fprofile-update.c

Modified: 
clang/docs/UsersManual.rst
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/CodeGen/code-coverage-tsan.c
clang/test/CodeGen/tsan-instrprof-atomic.c

Removed: 




diff  --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index 2d0d71443dfd..ed6c9e3bc341 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -2172,6 +2172,17 @@ programs using the same instrumentation method as 
``-fprofile-generate``.
   profile file, it reads from that file. If ``pathname`` is a directory name,
   it reads from ``pathname/default.profdata``.
 
+.. option:: -fprofile-update[=]
+
+  Unless ``-fsanitize=thread`` is specified, the default is ``single``, which
+  uses non-atomic increments. The counters can be inaccurate under thread
+  contention. ``atomic`` uses atomic increments which is accurate but has
+  overhead. ``prefer-atomic`` will be transformed to ``atomic`` when supported
+  by the target, or ``single`` otherwise.
+
+  This option currently works with ``-fprofile-arcs`` and 
``-fprofile-instr-generate``,
+  but not with ``-fprofile-generate``.
+
 Disabling Instrumentation
 ^
 

diff  --git a/clang/include/clang/Basic/CodeGenOptions.def 
b/clang/include/clang/Basic/CodeGenOptions.def
index a259218b29c6..062a8c3fe64a 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -185,6 +185,7 @@ CODEGENOPT(ObjCConvertMessagesToRuntimeCalls , 1, 1)
 VALUE_CODEGENOPT(OptimizationLevel, 2, 0) ///< The -O[0-3] option specified.
 VALUE_CODEGENOPT(OptimizeSize, 2, 0) ///< If -Os (==1) or -Oz (==2) is 
specified.
 
+CODEGENOPT(AtomicProfileUpdate , 1, 0) ///< Set -fprofile-update=atomic
 /// Choose profile instrumenation kind or no instrumentation.
 ENUM_CODEGENOPT(ProfileInstr, ProfileInstrKind, 2, ProfileNone)
 /// Choose profile kind for PGO use compilation.

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index a1f3d7a4316f..09fdf50b1cb8 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -853,6 +853,9 @@ def fprofile_filter_files_EQ : Joined<["-"], 
"fprofile-filter-files=">,
 def fprofile_exclude_files_EQ : Joined<["-"], "fprofile-exclude-files=">,
 Group, Flags<[CC1Option, CoreOption]>,
 HelpText<"Instrument only functions from files where names don't match all 
the regexes separated by a semi-colon">;
+def fprofile_update_EQ : Joined<["-"], "fprofile-update=">,
+Group, Flags<[CC1Option, CoreOption]>, 
Values<"atomic,prefer-atomic,single">,
+MetaVarName<"">, HelpText<"Set update method of profile counters 
(atomic,prefer-atomic,single)">;
 def forder_file_instrumentation : Flag<["-"], "forder-file-instrumentation">,
 Group, Flags<[CC1Option, CoreOption]>,
 HelpText<"Generate instrumented code to collect order file into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;

diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index f83ec2479652..d77590cc2adf 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -570,7 +570,7 @@ static Optional getGCOVOptions(const 
CodeGenOptions &CodeGenOpts,
   Options.NoRedZone = CodeGenOpts.DisableRedZone;
   Options.Filter = CodeGenOpts.ProfileFilterFiles;
   Options.Exclude = CodeGenOpts.ProfileExcludeFiles;
-  Options.Atomic = LangOpts.Sanitize.has(SanitizerKind::Thread);
+  Options.Atomic = CodeGenOpts.AtomicProfileUpdate;
   return Options;
 }
 
@@ -582,10 +582,7 @@ getInstrProfOptions(const CodeGenOptions &CodeGenOpts,
   InstrProfOptions Options;
   

[PATCH] D88329: [objc] Consolidate ObjC name mangle code to AST

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added inline comments.



Comment at: clang/lib/CodeGen/CGObjCMac.cpp:929
-  /// \param[out] NameOut - The return value.
-  void GetNameForMethod(const ObjCMethodDecl *OMD,
-const ObjCContainerDecl *CD,

Note that the removed method is mentioned in comment docs at 
https://github.com/llvm/llvm-project/blob/3681be876fea9b270c7a1d2dc41679a399610e06/llvm/tools/dsymutil/SymbolMap.cpp#L50


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88329

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

[PATCH] D88403: Migrate Declarators to use the List API

[Eduardo Caldas via Phabricator via cfe-commits]

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

Update tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88403

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/lib/Tooling/Syntax/Synthesis.cpp
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
  clang/unittests/Tooling/Syntax/SynthesisTest.cpp

Index: clang/unittests/Tooling/Syntax/SynthesisTest.cpp
===
--- clang/unittests/Tooling/Syntax/SynthesisTest.cpp
+++ clang/unittests/Tooling/Syntax/SynthesisTest.cpp
@@ -188,8 +188,9 @@
 TranslationUnit Detached synthesized
 `-SimpleDeclaration synthesized
   |-'int' synthesized
-  |-SimpleDeclarator Declarator synthesized
-  | `-'a' synthesized
+  |-DeclaratorList Declarators synthesized
+  | `-SimpleDeclarator ListElement synthesized
+  |   `-'a' synthesized
   `-';' synthesized
   )txt"));
 }
@@ -201,8 +202,9 @@
   EXPECT_TRUE(treeDumpEqual(Copy, R"txt(
 SimpleDeclaration Detached synthesized
 |-'int' synthesized
-|-SimpleDeclarator Declarator synthesized
-| `-'a' synthesized
+|-DeclaratorList Declarators synthesized
+| `-SimpleDeclarator ListElement synthesized
+|   `-'a' synthesized
 `-';' synthesized
   )txt"));
 }
@@ -225,11 +227,12 @@
 TranslationUnit Detached synthesized
 `-SimpleDeclaration synthesized
   |-'void' synthesized
-  |-SimpleDeclarator Declarator synthesized
-  | |-'test' synthesized
-  | `-ParametersAndQualifiers synthesized
-  |   |-'(' OpenParen synthesized
-  |   `-')' CloseParen synthesized
+  |-DeclaratorList Declarators synthesized
+  | `-SimpleDeclarator ListElement synthesized
+  |   |-'test' synthesized
+  |   `-ParametersAndQualifiers synthesized
+  | |-'(' OpenParen synthesized
+  | `-')' CloseParen synthesized
   `-CompoundStatement synthesized
 |-'{' OpenParen synthesized
 |-IfStatement Statement synthesized
Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -92,21 +92,23 @@
 TranslationUnit Detached
 |-SimpleDeclaration
 | |-'int'
-| |-SimpleDeclarator Declarator
-| | |-'main'
-| | `-ParametersAndQualifiers
-| |   |-'(' OpenParen
-| |   `-')' CloseParen
+| |-DeclaratorList Declarators
+| | `-SimpleDeclarator ListElement
+| |   |-'main'
+| |   `-ParametersAndQualifiers
+| | |-'(' OpenParen
+| | `-')' CloseParen
 | `-CompoundStatement
 |   |-'{' OpenParen
 |   `-'}' CloseParen
 `-SimpleDeclaration
   |-'void'
-  |-SimpleDeclarator Declarator
-  | |-'foo'
-  | `-ParametersAndQualifiers
-  |   |-'(' OpenParen
-  |   `-')' CloseParen
+  |-DeclaratorList Declarators
+  | `-SimpleDeclarator ListElement
+  |   |-'foo'
+  |   `-ParametersAndQualifiers
+  | |-'(' OpenParen
+  | `-')' CloseParen
   `-CompoundStatement
 |-'{' OpenParen
 `-'}' CloseParen
@@ -123,16 +125,18 @@
 TranslationUnit Detached
 |-SimpleDeclaration
 | |-'int'
-| |-SimpleDeclarator Declarator
-| | `-'a'
+| |-DeclaratorList Declarators
+| | `-SimpleDeclarator ListElement
+| |   `-'a'
 | `-';'
 `-SimpleDeclaration
   |-'int'
-  |-SimpleDeclarator Declarator
-  | |-'b'
-  | |-'='
-  | `-IntegerLiteralExpression
-  |   `-'42' LiteralToken
+  |-DeclaratorList Declarators
+  | `-SimpleDeclarator ListElement
+  |   |-'b'
+  |   |-'='
+  |   `-IntegerLiteralExpression
+  | `-'42' LiteralToken
   `-';'
 )txt"));
 }
@@ -146,21 +150,24 @@
 TranslationUnit Detached
 `-SimpleDeclaration
   |-'void'
-  |-SimpleDeclarator Declarator
-  | |-'foo'
-  | `-ParametersAndQualifiers
-  |   |-'(' OpenParen
-  |   |-ParameterDeclarationList Parameters
-  |   | |-SimpleDeclaration ListElement
-  |   | | |-'int'
-  |   | | `-SimpleDeclarator Declarator
-  |   | |   `-'a'
-  |   | |-',' ListDelimiter
-  |   | `-SimpleDeclaration ListElement
-  |   |   |-'int'
-  |   |   `-SimpleDeclarator Declarator
-  |   | `-'b'
-  |   `-')' CloseParen
+  |-DeclaratorList Declarators
+  | `-SimpleDeclarator ListElement
+  |   |-'foo'
+  |   `-ParametersAndQualifiers
+  | |-'(' OpenParen
+  | |-ParameterDeclarationList Parameters
+  | | |-SimpleDeclaration ListElement
+  | | | |-'int'
+  | | | `-DeclaratorList Declarators
+  | | |   `-SimpleDeclarator ListElement
+  | | | `-'a'
+  | | |-',' ListDelimiter
+  | | `-SimpleDeclaration ListElement
+  | |   |-'int'
+  | |   `-DeclaratorList Declarators
+  | | `-SimpleDeclarator ListElement
+  | |   `-'b'
+  | `-')' CloseParen
   `-CompoundStatement
 |-'{' OpenParen
 `-'}' CloseParen
@@ -178,8 +185,9 @@
 `-SimpleDeclaration
   |-'in\
 t'
-  |-SimpleDeclarator Declarator
-  | `-'a'
+  |-DeclaratorList Declarators
+  | `-SimpleDeclarator ListElement
+  

[PATCH] D88497: [objc] Fix memory leak in CGObjCMac.cpp

[Ellis Hoag via Phabricator via cfe-commits]

ellis updated this revision to Diff 295055.
ellis added a subscriber: vsapsai.
ellis added a comment.
Herald added a reviewer: JDevlieghere.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Fix a comment to reference the correct method.

Thanks to @vsapsai for pointing this out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88497

Files:
  clang/lib/CodeGen/CGObjCMac.cpp
  llvm/tools/dsymutil/SymbolMap.cpp


Index: llvm/tools/dsymutil/SymbolMap.cpp
===
--- llvm/tools/dsymutil/SymbolMap.cpp
+++ llvm/tools/dsymutil/SymbolMap.cpp
@@ -47,7 +47,7 @@
 return Translation;
 
   // Objective-C symbols for the MachO symbol table start with a \1. Please see
-  // `CGObjCCommonMac::GetNameForMethod` in clang.
+  // `MangleContext::mangleObjCMethodName` in clang.
   if (Translation[0] == 1)
 return StringRef(Translation).drop_front();
 
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -1079,8 +1079,9 @@
   void EmitImageInfo();
 
 public:
-  CGObjCCommonMac(CodeGen::CodeGenModule &cgm) :
-CGObjCRuntime(cgm), VMContext(cgm.getLLVMContext()) { }
+  CGObjCCommonMac(CodeGen::CodeGenModule &cgm)
+  : CGObjCRuntime(cgm), VMContext(cgm.getLLVMContext()),
+Mangler(cgm.getContext().createMangleContext()) {}
 
   bool isNonFragileABI() const {
 return ObjCABI == 2;
@@ -1121,6 +1122,7 @@
 
 private:
   void fillRunSkipBlockVars(CodeGenModule &CGM, const CGBlockInfo &blockInfo);
+  std::unique_ptr Mangler;
 };
 
 namespace {
@@ -4003,9 +4005,8 @@
   } else {
 SmallString<256> Name;
 llvm::raw_svector_ostream OS(Name);
-const auto &MC = CGM.getContext().createMangleContext();
-MC->mangleObjCMethodName(OMD, OS, /*includePrefixByte=*/true,
- /*includeCategoryNamespace=*/true);
+Mangler->mangleObjCMethodName(OMD, OS, /*includePrefixByte=*/true,
+  /*includeCategoryNamespace=*/true);
 
 CodeGenTypes &Types = CGM.getTypes();
 llvm::FunctionType *MethodTy =
@@ -4059,9 +4060,8 @@
   } else {
 SmallString<256> Name;
 llvm::raw_svector_ostream OS(Name);
-const auto &MC = CGM.getContext().createMangleContext();
-MC->mangleObjCMethodName(OMD, OS, /*includePrefixByte=*/true,
- /*includeCategoryNamespace=*/false);
+Mangler->mangleObjCMethodName(OMD, OS, /*includePrefixByte=*/true,
+  /*includeCategoryNamespace=*/false);
 
 Fn = llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
 Name.str(), &CGM.getModule());


Index: llvm/tools/dsymutil/SymbolMap.cpp
===
--- llvm/tools/dsymutil/SymbolMap.cpp
+++ llvm/tools/dsymutil/SymbolMap.cpp
@@ -47,7 +47,7 @@
 return Translation;
 
   // Objective-C symbols for the MachO symbol table start with a \1. Please see
-  // `CGObjCCommonMac::GetNameForMethod` in clang.
+  // `MangleContext::mangleObjCMethodName` in clang.
   if (Translation[0] == 1)
 return StringRef(Translation).drop_front();
 
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -1079,8 +1079,9 @@
   void EmitImageInfo();
 
 public:
-  CGObjCCommonMac(CodeGen::CodeGenModule &cgm) :
-CGObjCRuntime(cgm), VMContext(cgm.getLLVMContext()) { }
+  CGObjCCommonMac(CodeGen::CodeGenModule &cgm)
+  : CGObjCRuntime(cgm), VMContext(cgm.getLLVMContext()),
+Mangler(cgm.getContext().createMangleContext()) {}
 
   bool isNonFragileABI() const {
 return ObjCABI == 2;
@@ -1121,6 +1122,7 @@
 
 private:
   void fillRunSkipBlockVars(CodeGenModule &CGM, const CGBlockInfo &blockInfo);
+  std::unique_ptr Mangler;
 };
 
 namespace {
@@ -4003,9 +4005,8 @@
   } else {
 SmallString<256> Name;
 llvm::raw_svector_ostream OS(Name);
-const auto &MC = CGM.getContext().createMangleContext();
-MC->mangleObjCMethodName(OMD, OS, /*includePrefixByte=*/true,
- /*includeCategoryNamespace=*/true);
+Mangler->mangleObjCMethodName(OMD, OS, /*includePrefixByte=*/true,
+  /*includeCategoryNamespace=*/true);
 
 CodeGenTypes &Types = CGM.getTypes();
 llvm::FunctionType *MethodTy =
@@ -4059,9 +4060,8 @@
   } else {
 SmallString<256> Name;
 llvm::raw_svector_ostream OS(Name);
-const auto &MC = CGM.getContext().createMangleContext();
-MC->mangleObjCMethodName(OMD, OS, /*includePrefixByte=*/true,
- /*includeCategoryNamespace=*/false);
+Mangler->mangleObjCMethodName(OMD, OS, /*includePrefixByte=*/true

[PATCH] D88445: Use "default member initializer" instead of "in-class initializer" for diagnostics

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

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

Thanks!


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

https://reviews.llvm.org/D88445

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

[PATCH] D77598: Integral template argument suffix and cast printing

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: clang/lib/AST/TemplateBase.cpp:71-72
 
   if (T->isBooleanType() && !Policy.MSVCFormatting) {
 Out << (Val.getBoolValue() ? "true" : "false");
   } else if (T->isCharType()) {

rsmith wrote:
> reikdas wrote:
> > rsmith wrote:
> > > rsmith wrote:
> > > > It looks like `MSVCFormatting` wants `bool` values to be printed as `0` 
> > > > and `1`, and this patch presumably changes that (along with the 
> > > > printing of other builtin types). I wonder if this is a problem in 
> > > > practice (eg, if such things are used as keys for debug info or 
> > > > similar)...
> > > Do we need to suppress printing the suffixes below in `MSVCFormatting` 
> > > mode too?
> > @rsmith The tests pass, so that is reassuring at least. Is there any other 
> > way to find out whether we need to suppress printing the suffixes for other 
> > types in MSVCFormatting mode?
> @rnk Can you take a look at this? Per 
> rG60103383f097b6580ecb4519eeb87defdb7c05c9 and PR21528 it seems like there 
> might be specific requirements for how template arguments are formatted for 
> CodeView support; we presumably need to make sure we still satisfy those 
> requirements.
Probably. This change looks like it preserves behavior from before when 
`MSVCFormatting` is set, so I think this is OK. I checked, my version of MSVC 
still uses 1/0 in debug info for boolean template args.


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

https://reviews.llvm.org/D77598

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

[PATCH] D88507: [clangd][remote] Make sure relative paths are absolute with respect to posix style

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Relative paths received from the server are always in posix style. So
we need to ensure they are relative using that style, and not the native one.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88507

Files:
  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
@@ -299,7 +299,7 @@
   assert(RelativePath == llvm::sys::path::convert_to_slash(RelativePath));
   if (RelativePath.empty())
 return error("Empty relative path.");
-  if (llvm::sys::path::is_absolute(RelativePath))
+  if (llvm::sys::path::is_absolute(RelativePath, 
llvm::sys::path::Style::posix))
 return error("RelativePath '{0}' is absolute.", RelativePath);
   llvm::SmallString<256> FullPath = llvm::StringRef(*LocalIndexRoot);
   llvm::sys::path::append(FullPath, RelativePath);


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
@@ -299,7 +299,7 @@
   assert(RelativePath == llvm::sys::path::convert_to_slash(RelativePath));
   if (RelativePath.empty())
 return error("Empty relative path.");
-  if (llvm::sys::path::is_absolute(RelativePath))
+  if (llvm::sys::path::is_absolute(RelativePath, llvm::sys::path::Style::posix))
 return error("RelativePath '{0}' is absolute.", RelativePath);
   llvm::SmallString<256> FullPath = llvm::StringRef(*LocalIndexRoot);
   llvm::sys::path::append(FullPath, RelativePath);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 962a247 - [clangd] Fix assertion in remote-index marshalling

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-09-29T20:40:19+02:00
New Revision: 962a247aebba39bc8f2d6aa901ed512f5c09dc72

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

LOG: [clangd] Fix assertion in remote-index marshalling

convert_to_slash is a no-op on posix style.

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp 
b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
index 839250982a03..31ce4a44ea55 100644
--- a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -296,8 +296,7 @@ llvm::Expected Marshaller::toProtobuf(const 
clangd::SymbolID &Subject,
 llvm::Expected
 Marshaller::relativePathToURI(llvm::StringRef RelativePath) {
   assert(LocalIndexRoot);
-  assert(RelativePath == llvm::sys::path::convert_to_slash(
- RelativePath, llvm::sys::path::Style::posix));
+  assert(RelativePath == llvm::sys::path::convert_to_slash(RelativePath));
   if (RelativePath.empty())
 return error("Empty relative path.");
   if (llvm::sys::path::is_absolute(RelativePath))



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

[PATCH] D88497: [objc] Fix memory leak in CGObjCMac.cpp

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added a comment.

Thanks for updating the comment in dsymutil!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88497

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

[PATCH] D88500: [AIX][Clang][Driver] Link libm along with libc++

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:165
 
+// Since libc++ has dependencies on libm, if we have one then add the 
other.
+if (getToolChain().ShouldLinkCXXStdlib(Args))

Is that right?

My check of the libc++ from XL C/C++ for AIX 16.1.0 shows a dependency on 
`libC.a`, but not `libm.a`:
```
[183]   0xundef  IMP DS EXTref libC.a(shrcore.o) 
uncaught_exception__3stdFv
```

It seems `-lm` is just linked for C++ invocations. Other targets also pick up 
`-lm` for C++ invocations as a stdlib (not specifically a C++ stdlib).



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:168
+  CmdArgs.push_back("-lm");
 CmdArgs.push_back("-lc");
   }

Minor nit: Given the spacing in this block, I think a blank line would be 
appropriate to have before this one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88500

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

[clang] 15fbae8 - Use "default member initializer" instead of "in-class initializer" for diagnostics.

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2020-09-29T15:04:23-04:00
New Revision: 15fbae8ac303d8601ea95418d4818cb50d0765e1

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

LOG: Use "default member initializer" instead of "in-class initializer" for 
diagnostics.

This changes some diagnostics to use terminology from the standard
rather than invented terminology, which improves consistency with other
diagnostics as well. There are no functional changes intended other
than wording and naming.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/test/Parser/MicrosoftExtensions.cpp
clang/test/Parser/cxx-class.cpp
clang/test/SemaCXX/PR9572.cpp
clang/test/SemaCXX/class.cpp
clang/test/SemaCXX/cxx98-compat.cpp
clang/test/SemaCXX/member-init.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 1ac1e9d10a7a..da4e1725269f 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -709,7 +709,7 @@ def err_ms_property_expected_accessor_name : Error<
 def err_ms_property_expected_comma_or_rparen : Error<
   "expected ',' or ')' at end of property accessor list">;
 def err_ms_property_initializer : Error<
-  "property declaration cannot have an in-class initializer">;
+  "property declaration cannot have a default member initializer">;
 
 def warn_cxx20_compat_explicit_bool : Warning<
   "this expression will be parsed as explicit(bool) in C++20">,
@@ -859,13 +859,13 @@ def warn_cxx98_compat_defaulted_deleted_function : 
Warning<
   "%select{defaulted|deleted}0 function definitions are incompatible with 
C++98">,
   InGroup, DefaultIgnore;
 
-// C++11 in-class member initialization
+// C++11 default member initialization
 def ext_nonstatic_member_init : ExtWarn<
-  "in-class initialization of non-static data member is a C++11 extension">,
-  InGroup;
+  "default member initializer for non-static data member is a C++11 "
+  "extension">, InGroup;
 def warn_cxx98_compat_nonstatic_member_init : Warning<
-  "in-class initialization of non-static data members is incompatible with 
C++98">,
-  InGroup, DefaultIgnore;
+  "default member initializer for non-static data members is incompatible with 
"
+  "C++98">, InGroup, DefaultIgnore;
 def ext_bitfield_member_init: ExtWarn<
   "default member initializer for bit-field is a C++20 extension">,
   InGroup;
@@ -873,7 +873,7 @@ def warn_cxx17_compat_bitfield_member_init: Warning<
   "default member initializer for bit-field is incompatible with "
   "C++ standards before C++20">, InGroup, DefaultIgnore;
 def err_incomplete_array_member_init: Error<
-  "array bound cannot be deduced from an in-class initializer">;
+  "array bound cannot be deduced from a default member initializer">;
 
 // C++11 alias-declaration
 def ext_alias_declaration : ExtWarn<

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 8f6c7b9400fa..ed11e0d1ce3c 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1842,7 +1842,7 @@ def note_nontrivial_no_copy : Note<
 def note_nontrivial_user_provided : Note<
   "because %select{base class of |field of |}0type %1 has a user-provided "
   "%sub{select_special_member_kind}2">;
-def note_nontrivial_in_class_init : Note<
+def note_nontrivial_default_member_init : Note<
   "because field %0 has an initializer">;
 def note_nontrivial_param_type : Note<
   "because its parameter is %
diff {of type $, not $|of the wrong type}2,3">;
@@ -8521,12 +8521,12 @@ def err_in_class_initializer_literal_type : Error<
   "'constexpr' specifier">;
 def err_in_class_initializer_non_constant : Error<
   "in-class initializer for static data member is not a constant expression">;
-def err_in_class_initializer_not_yet_parsed : Error<
+def err_default_member_initializer_not_yet_parsed : Error<
   "default member initializer for %1 needed within definition of enclosing "
   "class %0 outside of member functions">;
-def note_in_class_initializer_not_yet_parsed : Note<
+def note_default_member_initializer_not_yet_parsed : Note<
   "default member initializer declared here">;
-def err_in_class_initializer_cycle
+def err_default_member_initializer_cycle
 : Error<"default member initializer for %0 uses itself">;
 
 def ext_in_class_initializer_non_constant : Extension<

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 6558a4f6d8b2..2d2b80573a69 100644
--- a/clang/lib/Sema/SemaDeclC