[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

[Clement Courbet via Phabricator via cfe-commits]

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

I'm going to remove the `&&` warning from the check and keep only the `const` 
one, as I think these is a consensus on that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70390



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

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

[Clement Courbet via Phabricator via cfe-commits]

courbet added a comment.

In D70390#1751751 , @Quuxplusone wrote:

> that sounds like a big non-local change whose correctness would be very 
> difficult to verify.


Agreed. That's why I did not make this check suggest a fixit, because it's too 
easy to click "apply" without thinking.

>   I think (FWIW) that such a fixit would be a very hard sell

I've seen several places in our codebase where it did make quite a big 
difference performance wise, so they were quite easy to sell :)

> Nobody is currently working on that Clang patch AFAIK. If you're interested 
> in submitting Clang patches in this area, that might be a very high-value (if 
> kind of fuzzy/user-experience/political) patch to work on!
>  Meanwhile, as of November 2019, the C++2a committee draft still contains 
> unfixed corner cases that Clang doesn't warn about. For example:
>  https://godbolt.org/z/fBKM-4 (no warning from Clang; C++2a fixes this 
> behavior)
>  https://godbolt.org/z/EfBCdQ (no warning from Clang; C++2a does NOT fix this 
> behavior)

Super interesting, thanks ! I'll see if I have time to work on that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70390



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

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

[Clement Courbet via Phabricator via cfe-commits]

courbet updated this revision to Diff 230202.
courbet added a comment.

Remove && warnings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70390

Files:
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp
  clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.h
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst
  clang-tools-extra/test/clang-tidy/checkers/performance-no-automatic-move.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-no-automatic-move.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-no-automatic-move.cpp
@@ -0,0 +1,92 @@
+// RUN: %check_clang_tidy -std=c++11,c++14,c++17,c++2a %s performance-no-automatic-move %t
+
+struct Obj {
+  Obj();
+  Obj(const Obj &);
+  Obj(Obj &&);
+  virtual ~Obj();
+};
+
+template 
+struct StatusOr {
+  StatusOr(const T &);
+  StatusOr(T &&);
+};
+
+struct NonTemplate {
+  NonTemplate(const Obj &);
+  NonTemplate(Obj &&);
+};
+
+template 
+T Make();
+
+StatusOr PositiveStatusOrConstValue() {
+  const Obj obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+NonTemplate PositiveNonTemplateConstValue() {
+  const Obj obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+Obj PositiveSelfConstValue() {
+  const Obj obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+// FIXME: Ideally we would warn here too.
+NonTemplate PositiveNonTemplateLifetimeExtension() {
+  const Obj &obj = Make();
+  return obj;
+}
+
+// FIXME: Ideally we would warn here too.
+StatusOr PositiveStatusOrLifetimeExtension() {
+  const Obj &obj = Make();
+  return obj;
+}
+
+// Negatives.
+
+StatusOr Temporary() {
+  return Make();
+}
+
+StatusOr ConstTemporary() {
+  return Make();
+}
+
+StatusOr Nrvo() {
+  Obj obj = Make();
+  return obj;
+}
+
+StatusOr Ref() {
+  Obj &obj = Make();
+  return obj;
+}
+
+StatusOr ConstRef() {
+  const Obj &obj = Make();
+  return obj;
+}
+
+const Obj global;
+
+StatusOr Global() {
+  return global;
+}
+
+struct FromConstRefOnly {
+  FromConstRefOnly(const Obj &);
+};
+
+FromConstRefOnly FromConstRefOnly() {
+  const Obj obj = Make();
+  return obj;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst
@@ -0,0 +1,53 @@
+.. title:: clang-tidy - performance-no-automatic-move
+
+performance-no-automatic-move
+=
+
+Finds local variables that cannot be automatically moved due to constness.
+
+Under
+`certain conditions `_,
+local values are automatically moved out when returning from a function. A
+common mistake is to declared local ``lvalue`` variables ``const``, which
+prevents the move.
+
+Example `[1] `_:
+
+.. code-block:: c++
+
+  StatusOr> Cool() {
+std::vector obj = ...;
+return obj;  // calls StatusOr::StatusOr(std::vector&&)
+  }
+  
+  StatusOr> NotCool() {
+const std::vector obj = ...;
+return obj;  // calls `StatusOr::StatusOr(const std::vector&)`
+  }
+
+The former version (``Cool``) should be preferred over the latter (``Uncool``)
+as it will avoid allocations and potentially large memory copies.
+
+Semantics
+-
+
+In the example above, ``StatusOr::StatusOr(T&&)`` have the same semantics as
+long as the copy and move constructors for ``T`` have the same semantics. Note
+that there is no guarantee that ``S::S(T&&)`` and ``S::S(const T&)`` have the
+same semantics for any single ``S``, so we're not providing automated fixes for
+this check, and judgement should be exerted when making the suggested changes.
+
+-Wreturn-std-move
+-
+
+Another case where the move cannot happen is the following:
+
+.. code-block:: c++
+
+  StatusOr> Uncool() {
+std::vector&& obj = ...;
+return obj;  // calls `StatusOr::StatusOr(const std::vector&)`
+  }
+
+In that case the fix is more consensual: just `return std::move(obj)`.
+This is handled by the `-Wreturn-std-move` warning.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-to

[PATCH] D70320: [Analyzer] [NFC] Iterator Checkers - Separate iterator modeling and the actual checkers

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:1308-1318
+ProgramStateRef removeIteratorPosition(ProgramStateRef State, const SVal &Val) 
{
   if (auto Reg = Val.getAsRegion()) {
 Reg = Reg->getMostDerivedObjectRegion();
-return State->get(Reg);
+return State->remove(Reg);
   } else if (const auto Sym = Val.getAsSymbol()) {
-return State->get(Sym);
+return State->remove(Sym);
   } else if (const auto LCVal = Val.getAs()) {

baloghadamsoftware wrote:
> NoQ wrote:
> > Maybe move this function to `Iterator.cpp` as well, and then move the 
> > definitions for iterator maps from `Iterator.h` to `Iterator.cpp`, which 
> > will allow you to use the usual `REGISTER_MAP_WITH_PROGRAMSTATE` macros, 
> > and additionally guarantee that all access to the maps goes through the 
> > accessor methods that you provide?
> Hmm, I was trying hard to use these macros but failed so I reverted to the 
> manual solution. I will retry now.
Here is a how-to: https://reviews.llvm.org/D69726

You need to add the fully qualified names to the register macro because of the 
global scope. I hope it helps.


Repository:
  rC Clang

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

https://reviews.llvm.org/D70320



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

[PATCH] D67536: [clangd] Inactive regions support as an extension to semantic highlighting

[Haojian Wu via Phabricator via cfe-commits]

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

looks good, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67536



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

[PATCH] D68206: [clang] Remove the DIFlagArgumentNotModified debug info flag

[Djordje Todorovic via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGce1f95a6e077: Reland "[clang] Remove the 
DIFlagArgumentNotModified debug info flag" (authored by djtodoro).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68206

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/test/CodeGen/debug-info-param-modification.c
  
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py

Index: lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py
===
--- lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py
@@ -6,7 +6,8 @@
 supported_platforms.extend(lldbplatformutil.getDarwinOSTriples())
 
 lldbinline.MakeInlineTest(__file__, globals(),
-[decorators.skipUnlessPlatform(supported_platforms),
+[decorators.skipIf(bugnumber="llvm.org/pr44059"),
+ decorators.skipUnlessPlatform(supported_platforms),
  decorators.skipIf(compiler="clang", compiler_version=['<', '10.0']),
  decorators.skipUnlessArch('x86_64'),
  decorators.skipUnlessHasCallSiteInfo,
Index: clang/test/CodeGen/debug-info-param-modification.c
===
--- clang/test/CodeGen/debug-info-param-modification.c
+++ /dev/null
@@ -1,25 +0,0 @@
-// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -Xclang -disable-llvm-passes -S -target x86_64-none-linux-gnu -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK-ENTRY-VAL-OPT
-// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -Xclang -disable-llvm-passes -S -target arm-none-linux-gnu -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK-ENTRY-VAL-OPT
-// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -Xclang -disable-llvm-passes -S -target aarch64-none-linux-gnu -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK-ENTRY-VAL-OPT
-// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -Xclang -disable-llvm-passes -S -target armeb-none-linux-gnu -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK-ENTRY-VAL-OPT
-
-// CHECK-ENTRY-VAL-OPT: !DILocalVariable(name: "a", arg: 1, scope: {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}})
-// CHECK-ENTRY-VAL-OPT: !DILocalVariable(name: "b", arg: 2, scope: {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}}, flags: DIFlagArgumentNotModified)
-//
-// For the os_log_helper:
-// CHECK-ENTRY-VAL-OPT: !DILocalVariable(name: "buffer", arg: 1, {{.*}}, flags: DIFlagArtificial)
-//
-// RUN: %clang -g -O2 -Xclang -disable-llvm-passes -target x86_64-none-linux-gnu -S -emit-llvm %s -o - | FileCheck %s
-// CHECK-NOT: !DILocalVariable(name: "b", arg: 2, scope: {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}}, flags: DIFlagArgumentNotModified)
-//
-// For the os_log_helper:
-// CHECK: !DILocalVariable(name: "buffer", arg: 1, {{.*}}, flags: DIFlagArtificial)
-
-int fn2 (int a, int b) {
-  ++a;
-  return b;
-}
-
-void test_builtin_os_log(void *buf, int i, const char *data) {
-  __builtin_os_log_format(buf, "%d %{public}s %{private}.16P", i, data, data);
-}
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -146,10 +146,6 @@
 
   llvm::DenseMap DIFileCache;
   llvm::DenseMap SPCache;
-  /// Cache function definitions relevant to use for parameters mutation
-  /// analysis.
-  llvm::DenseMap SPDefCache;
-  llvm::DenseMap ParamCache;
   /// Cache declarations relevant to DW_TAG_imported_declarations (C++
   /// using declarations) that aren't covered by other more specific caches.
   llvm::DenseMap DeclCache;
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -18,7 +18,6 @@
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "ConstantEmitter.h"
-#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclFriend.h"
 #include "clang/AST/DeclObjC.h"
@@ -3686,15 +3685,6 @@
   if (HasDecl && isa(D))
 DeclCache[D->getCanonicalDecl()].reset(SP);
 
-  // We use the SPDefCache only in the case when the debug entry values option
-  // is set, in order to speed up parameters modification analysis.
-  //
-  // FIXME: Use AbstractCallee here to support ObjCMethodDecl.
-  if (CGM.getCodeGenOpts().EnableDebugEntryValues && HasDecl)
-if (auto 

[clang] ce1f95a - Reland "[clang] Remove the DIFlagArgumentNotModified debug info flag"

[Djordje Todorovic via cfe-commits]

Author: Djordje Todorovic
Date: 2019-11-20T10:08:07+01:00
New Revision: ce1f95a6e077693f93d8869245f911aff3eb7e4c

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

LOG: Reland "[clang] Remove the DIFlagArgumentNotModified debug info flag"

It turns out that the ExprMutationAnalyzer can be very slow when AST
gets huge in some cases. The idea is to move this analysis to the LLVM
back-end level (more precisely, in the LiveDebugValues pass). The new
approach will remove the performance regression, simplify the
implementation and give us front-end independent implementation.

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

Added: 


Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CGDebugInfo.h

lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py

Removed: 
clang/test/CodeGen/debug-info-param-modification.c



diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index 116517a9cb99..a9b3831aa0b5 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -18,7 +18,6 @@
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "ConstantEmitter.h"
-#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclFriend.h"
 #include "clang/AST/DeclObjC.h"
@@ -3686,15 +3685,6 @@ void CGDebugInfo::EmitFunctionStart(GlobalDecl GD, 
SourceLocation Loc,
   if (HasDecl && isa(D))
 DeclCache[D->getCanonicalDecl()].reset(SP);
 
-  // We use the SPDefCache only in the case when the debug entry values option
-  // is set, in order to speed up parameters modification analysis.
-  //
-  // FIXME: Use AbstractCallee here to support ObjCMethodDecl.
-  if (CGM.getCodeGenOpts().EnableDebugEntryValues && HasDecl)
-if (auto *FD = dyn_cast(D))
-  if (FD->hasBody() && !FD->param_empty())
-SPDefCache[FD].reset(SP);
-
   // Push the function onto the lexical block stack.
   LexicalBlockStack.emplace_back(SP);
 
@@ -4097,11 +4087,6 @@ llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const 
VarDecl *VD,
  llvm::DebugLoc::get(Line, Column, Scope, 
CurInlinedAt),
  Builder.GetInsertBlock());
 
-  if (CGM.getCodeGenOpts().EnableDebugEntryValues && ArgNo) {
-if (auto *PD = dyn_cast(VD))
-  ParamCache[PD].reset(D);
-  }
-
   return D;
 }
 
@@ -4717,29 +4702,6 @@ void CGDebugInfo::setDwoId(uint64_t Signature) {
   TheCU->setDWOId(Signature);
 }
 
-/// Analyzes each function parameter to determine whether it is constant
-/// throughout the function body.
-static void analyzeParametersModification(
-ASTContext &Ctx,
-llvm::DenseMap &SPDefCache,
-llvm::DenseMap &ParamCache) {
-  for (auto &SP : SPDefCache) {
-auto *FD = SP.first;
-assert(FD->hasBody() && "Functions must have body here");
-const Stmt *FuncBody = (*FD).getBody();
-for (auto Parm : FD->parameters()) {
-  ExprMutationAnalyzer FuncAnalyzer(*FuncBody, Ctx);
-  if (FuncAnalyzer.isMutated(Parm))
-continue;
-
-  auto I = ParamCache.find(Parm);
-  assert(I != ParamCache.end() && "Parameters should be already cached");
-  auto *DIParm = cast(I->second);
-  DIParm->setIsNotModified();
-}
-  }
-}
-
 void CGDebugInfo::finalize() {
   // Creating types might create further types - invalidating the current
   // element and the size(), so don't cache/reference them.
@@ -4812,10 +4774,6 @@ void CGDebugInfo::finalize() {
 if (auto MD = TypeCache[RT])
   DBuilder.retainType(cast(MD));
 
-  if (CGM.getCodeGenOpts().EnableDebugEntryValues)
-// This will be used to emit debug entry values.
-analyzeParametersModification(CGM.getContext(), SPDefCache, ParamCache);
-
   DBuilder.finalize();
 }
 

diff  --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h
index 9a097615b4b4..5341bfa7f350 100644
--- a/clang/lib/CodeGen/CGDebugInfo.h
+++ b/clang/lib/CodeGen/CGDebugInfo.h
@@ -146,10 +146,6 @@ class CGDebugInfo {
 
   llvm::DenseMap DIFileCache;
   llvm::DenseMap SPCache;
-  /// Cache function definitions relevant to use for parameters mutation
-  /// analysis.
-  llvm::DenseMap SPDefCache;
-  llvm::DenseMap ParamCache;
   /// Cache declarations relevant to DW_TAG_imported_declarations (C++
   /// using declarations) that aren't covered by other more specific caches.
   llvm::DenseMap DeclCache;

diff  --git a/clang/test/CodeGen/debug-info-param-modification.c 
b/clang/test/CodeGen/debug-info-param-modification.c
deleted file mode 100644
index f0a13a3777db..
--- a/clang/test/CodeGen/debug-info-param-modification.c
+++ /dev/null
@@ -1,25 +0,0 @@
-// RUN: %clang -Xclang -femit

[PATCH] D70441: [clangd] Speed up when building rename edit.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/SourceCode.h:54
+/// Use of UTF-16 may be overridden by kCurrentOffsetEncoding.
+llvm::Expected byteOffset(StringRef LineCode, int LSPCharacter);
+

How this is different from `positionToOffset` with `Pos.line = 0` and 
`Pos.column = LSPCharacter`?
Why do we need an extra function?



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:441
 
+llvm::Expected buildRenameEdit(llvm::StringRef InitialCode,
+ const std::vector &Occurrences,

We are aiming to convert all ranges in `O(n)` instead of `O(n^2)`, right?
I think this could be simpler and also have guaranteed performance even for 
long lines if we do it slightly differently. WDYT?

```
assert(llvm::is_sorted(Occurences));
// These two always correspond to the same position.
Position LastPos = Position{0, 0};
size_t LastOffset = 0;

std::vector> Offsets;
for (auto R : Occurences) {
  Position ShiftedStart = R.start - LastPos;
  size_t ShiftedStartOffset = positionToOffset(ShiftedStart, 
Code.substr(LastOffset);

  LastPos = ShiftedStart;
  LastOffset  = ShiftedStartOffset;
  // Do the same for the end position.
  // ...
}

// Now we can easily replacements.
// ...
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70441



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

[PATCH] D70480: [clangd] Use expansion location when the ref is inside macros.

[Haojian Wu via Phabricator via cfe-commits]

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

Previously, xrefs has inconsistent behavior when the reference is inside
macro body:

- AST-based xrefs (for main file) uses the expansion location;
- our index uses the spelling location;

This patch makes our index use expansion location, which is consistent with
AST-based xrefs, and kythe as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70480

Files:
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp


Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -644,6 +644,24 @@
   HaveRanges(Header.ranges();
 }
 
+TEST_F(SymbolCollectorTest, RefsOnMacros) {
+  CollectorOpts.RefFilter = RefKind::All;
+  CollectorOpts.RefsInHeaders = true;
+  Annotations Header(R"(
+#define TYPE(X) X
+#define FOO Foo
+class [[Foo]] {};
+void test() {
+  TYPE([[Foo]]) foo;
+  [[FOO]] foo2;
+}
+  )");
+  CollectorOpts.RefFilter = RefKind::All;
+  runSymbolCollector(Header.code(), "");
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
+  HaveRanges(Header.ranges();
+}
+
 TEST_F(SymbolCollectorTest, HeaderAsMainFile) {
   CollectorOpts.RefFilter = RefKind::All;
   Annotations Header(R"(
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -275,10 +275,9 @@
   // Mark D as referenced if this is a reference coming from the main file.
   // D may not be an interesting symbol, but it's cheaper to check at the end.
   auto &SM = ASTCtx->getSourceManager();
-  auto SpellingLoc = SM.getSpellingLoc(Loc);
   if (Opts.CountReferences &&
   (Roles & static_cast(index::SymbolRole::Reference)) &&
-  SM.getFileID(SpellingLoc) == SM.getMainFileID())
+  SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainFileID())
 ReferencedDecls.insert(ND);
 
   auto ID = getSymbolID(ND);
@@ -312,8 +311,9 @@
 return true;
   // Do not store references to main-file symbols.
   if (CollectRef && !IsMainFileOnly && !isa(ND) &&
-  (Opts.RefsInHeaders || SM.getFileID(SpellingLoc) == SM.getMainFileID()))
-DeclRefs[ND].emplace_back(SpellingLoc, Roles);
+  (Opts.RefsInHeaders ||
+   SM.getFileID(SM.getFileLoc(Loc)) == SM.getMainFileID()))
+DeclRefs[ND].emplace_back(SM.getFileLoc(Loc), Roles);
   // Don't continue indexing if this is a mere reference.
   if (IsOnlyRef)
 return true;


Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -644,6 +644,24 @@
   HaveRanges(Header.ranges();
 }
 
+TEST_F(SymbolCollectorTest, RefsOnMacros) {
+  CollectorOpts.RefFilter = RefKind::All;
+  CollectorOpts.RefsInHeaders = true;
+  Annotations Header(R"(
+#define TYPE(X) X
+#define FOO Foo
+class [[Foo]] {};
+void test() {
+  TYPE([[Foo]]) foo;
+  [[FOO]] foo2;
+}
+  )");
+  CollectorOpts.RefFilter = RefKind::All;
+  runSymbolCollector(Header.code(), "");
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
+  HaveRanges(Header.ranges();
+}
+
 TEST_F(SymbolCollectorTest, HeaderAsMainFile) {
   CollectorOpts.RefFilter = RefKind::All;
   Annotations Header(R"(
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -275,10 +275,9 @@
   // Mark D as referenced if this is a reference coming from the main file.
   // D may not be an interesting symbol, but it's cheaper to check at the end.
   auto &SM = ASTCtx->getSourceManager();
-  auto SpellingLoc = SM.getSpellingLoc(Loc);
   if (Opts.CountReferences &&
   (Roles & static_cast(index::SymbolRole::Reference)) &&
-  SM.getFileID(SpellingLoc) == SM.getMainFileID())
+  SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainFileID())
 ReferencedDecls.insert(ND);
 
   auto ID = getSymbolID(ND);
@@ -312,8 +311,9 @@
 return true;
   // Do not store references to main-file symbols.
   if (CollectRef && !IsMainFileOnly && !isa(ND) &&
-  (Opts.RefsInHeaders || SM.getFileID(SpellingLoc) == SM.getMainFileID()))
-DeclRefs

[PATCH] D70480: [clangd] Use expansion location when the ref is inside macros.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:280
   (Roles & static_cast(index::SymbolRole::Reference)) &&
-  SM.getFileID(SpellingLoc) == SM.getMainFileID())
+  SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainFileID())
 ReferencedDecls.insert(ND);

We're using `getSpellingLoc` here and `getFileLoc` later. Why not use 
`getFileLoc` everywhere?

Having a variable (similar to the `SpellingLoc` we had before) and calling 
`getFileLoc` only once also seems preferable.



Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:655
+void test() {
+  TYPE([[Foo]]) foo;
+  [[FOO]] foo2;

Could you also check:
1. Multi-level macro arguments `TYPE(TYPE(FOO))`
2. Concatenated tokens as identifiers, e.g. coming from `#define CAT(X, Y) X##Y`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70480



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

[PATCH] D69608: [clangd] Helper for getting nested namespace qualification

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 230211.
kadircet marked 4 inline comments as done.
kadircet added a comment.

- Add support for non-namespace decl contexts.
- Add an overload for "textual namespace" handling.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69608

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/clangd/unittests/ASTTests.cpp

Index: clang-tools-extra/clangd/unittests/ASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ASTTests.cpp
@@ -7,7 +7,17 @@
 //===--===//
 
 #include "AST.h"
+#include "Annotations.h"
+#include "ParsedAST.h"
+#include "TestTU.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
+#include "llvm/ADT/StringRef.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -36,6 +46,103 @@
 "testns1::TestClass", "testns1"));
 }
 
+TEST(ClangdAST, GetQualification) {
+  const struct {
+llvm::StringRef Test;
+std::vector Qualifications;
+std::vector VisibleNamespaces;
+  } Cases[] = {
+  {
+  R"cpp(
+namespace ns1 { namespace ns2 { class Foo {}; } }
+void insert(); // ns1::ns2::Foo
+namespace ns1 {
+  void insert(); // ns2::Foo
+  namespace ns2 {
+void insert(); // Foo
+  }
+  using namespace ns2;
+  void insert(); // Foo
+}
+using namespace ns1;
+void insert(); // ns2::Foo
+using namespace ns2;
+void insert(); // Foo
+  )cpp",
+  {"ns1::ns2::", "ns2::", "", "", "ns2::", ""},
+  {},
+  },
+  {
+  R"cpp(
+namespace ns1 { namespace ns2 { class Bar { void Foo(); }; } }
+void insert(); // ns1::ns2::Bar::Foo
+namespace ns1 {
+  void insert(); // ns2::Bar::Foo
+  namespace ns2 {
+void insert(); // Bar::Foo
+  }
+  using namespace ns2;
+  void insert(); // Bar::Foo
+}
+using namespace ns1;
+void insert(); // ns2::Bar::Foo
+using namespace ns2;
+void insert(); // Bar::Foo
+  )cpp",
+  {"ns1::ns2::Bar::", "ns2::Bar::", "Bar::", "Bar::", "ns2::Bar::",
+   "Bar::"},
+  {},
+  },
+  {
+  R"cpp(
+namespace ns1 { namespace ns2 { void Foo(); } }
+void insert(); // ns2::Foo
+namespace ns1 {
+  void insert(); // ns2::Foo
+  namespace ns2 {
+void insert(); // Foo
+  }
+}
+  )cpp",
+  {"ns2::", "ns2::", ""},
+  {"ns1::"},
+  },
+  };
+  for (const auto &Case : Cases) {
+Annotations Test(Case.Test);
+TestTU TU = TestTU::withCode(Test.code());
+ParsedAST AST = TU.build();
+std::vector InsertionPoints;
+const NamedDecl *TargetDecl;
+findDecl(AST, [&](const NamedDecl &ND) {
+  if (ND.getNameAsString() == "Foo") {
+TargetDecl = &ND;
+return true;
+  }
+
+  if (ND.getNameAsString() == "insert")
+InsertionPoints.push_back(&ND);
+  return false;
+});
+
+ASSERT_EQ(InsertionPoints.size(), Case.Qualifications.size());
+for (size_t I = 0, E = InsertionPoints.size(); I != E; ++I) {
+  const Decl *D = InsertionPoints[I];
+  if (Case.VisibleNamespaces.empty()) {
+EXPECT_EQ(getQualification(AST.getASTContext(),
+   D->getLexicalDeclContext(), D->getBeginLoc(),
+   TargetDecl),
+  Case.Qualifications[I]);
+  } else {
+EXPECT_EQ(getQualification(AST.getASTContext(),
+   D->getLexicalDeclContext(), D->getBeginLoc(),
+   TargetDecl, Case.VisibleNamespaces),
+  Case.Qualifications[I]);
+  }
+}
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/AST.h
===
--- clang-tools-extra/clangd/AST.h
+++ clang-tools-extra/clangd/AST.h
@@ -15,9 +15,13 @@
 
 #include "index/SymbolID.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/NestedNameSpecifier.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Lex/MacroInfo.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/StringRef.h"
+#include 
+#include 
 
 namespace clang {
 class SourceManager;
@@ -111,6 +115,35 @@
 /// void foo() -> returns null
 NestedNameSpecifierLoc getQualifierLoc(const N

[PATCH] D70480: [clangd] Use expansion location when the ref is inside macros.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: pass - 60164 tests passed, 0 failed and 731 were skipped.
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70480



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

[PATCH] D69033: [clangd] Improve symbol qualification in DefineInline code action

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 230213.
kadircet marked 4 inline comments as done.
kadircet added a comment.

- Address comments
- Rebase
- Get rid of string literals inside macros


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69033

Files:
  clang-tools-extra/clangd/refactor/tweaks/DefineInline.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
@@ -1727,6 +1727,146 @@
 template <> inline void foo(){})cpp")));
 }
 
+TEST_F(DefineInlineTest, DropCommonNamespecifiers) {
+  struct {
+llvm::StringRef Test;
+llvm::StringRef Expected;
+  } Cases[] = {
+  {R"cpp(
+namespace a { namespace b { void aux(); } }
+namespace ns1 {
+  void foo();
+  namespace qq { void test(); }
+  namespace ns2 {
+void bar();
+namespace ns3 { void baz(); }
+  }
+}
+
+using namespace a;
+using namespace a::b;
+using namespace ns1::qq;
+void ns1::ns2::ns3::b^az() {
+  foo();
+  bar();
+  baz();
+  ns1::ns2::ns3::baz();
+  aux();
+  test();
+})cpp",
+   R"cpp(
+namespace a { namespace b { void aux(); } }
+namespace ns1 {
+  void foo();
+  namespace qq { void test(); }
+  namespace ns2 {
+void bar();
+namespace ns3 { void baz(){
+  foo();
+  bar();
+  baz();
+  ns1::ns2::ns3::baz();
+  a::b::aux();
+  qq::test();
+} }
+  }
+}
+
+using namespace a;
+using namespace a::b;
+using namespace ns1::qq;
+)cpp"},
+  {R"cpp(
+namespace ns1 {
+  namespace qq { struct Foo { struct Bar {}; }; using B = Foo::Bar; }
+  namespace ns2 { void baz(); }
+}
+
+using namespace ns1::qq;
+void ns1::ns2::b^az() { Foo f; B b; })cpp",
+   R"cpp(
+namespace ns1 {
+  namespace qq { struct Foo { struct Bar {}; }; using B = Foo::Bar; }
+  namespace ns2 { void baz(){ qq::Foo f; qq::B b; } }
+}
+
+using namespace ns1::qq;
+)cpp"},
+  {R"cpp(
+namespace ns1 {
+  namespace qq {
+template struct Foo { template  struct Bar {}; };
+template
+using B = typename Foo::template Bar;
+  }
+  namespace ns2 { void baz(); }
+}
+
+using namespace ns1::qq;
+void ns1::ns2::b^az() { B b; })cpp",
+   R"cpp(
+namespace ns1 {
+  namespace qq {
+template struct Foo { template  struct Bar {}; };
+template
+using B = typename Foo::template Bar;
+  }
+  namespace ns2 { void baz(){ qq::B b; } }
+}
+
+using namespace ns1::qq;
+)cpp"},
+  };
+  for (const auto &Case : Cases)
+EXPECT_EQ(apply(Case.Test), Case.Expected) << Case.Test;
+}
+
+TEST_F(DefineInlineTest, QualifyWithUsingDirectives) {
+  llvm::StringRef Test = R"cpp(
+namespace a {
+  void bar();
+  namespace b { struct Foo{}; void aux(); }
+  namespace c { void cux(); }
+}
+using namespace a;
+using X = b::Foo;
+void foo();
+
+using namespace b;
+using namespace c;
+void ^foo() {
+  cux();
+  bar();
+  X x;
+  aux();
+  using namespace c;
+  cux();
+})cpp";
+  llvm::StringRef Expected = R"cpp(
+namespace a {
+  void bar();
+  namespace b { struct Foo{}; void aux(); }
+  namespace c { void cux(); }
+}
+using namespace a;
+using X = b::Foo;
+void foo(){
+  c::cux();
+  bar();
+  X x;
+  b::aux();
+  using namespace c;
+  c::cux();
+}
+
+using namespace b;
+using namespace c;
+)cpp";
+  // FIXME: The last reference to cux() in body of foo should not be qualified,
+  // since there is a using directive inside the function body.
+  EXPECT_EQ(apply(Test), Expected) << Test;
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
@@ -136,8 +136,10 @@
   return true;
 }
 
-// Rewrites body of FD to fully-qualify all of the decls inside.
-llvm::Expected qualifyAllDecls(const FunctionDecl *FD) {
+// Rewrites body of FD by re-spelling all of the names to make sure they are
+// still valid in context of Target.
+llvm::Expected qualifyAllDecls(const Function

[PATCH] D69033: [clangd] Improve symbol qualification in DefineInline code action

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:196
 //~ -> we need to qualify Bar not x.
-if (!ND->getDeclContext()->isNamespace())
+if (!ND->getLexicalDeclContext()->isNamespace())
   return;

ilya-biryukov wrote:
> Why do we need to change this?
> My understanding is that we want semantic decl context, not the lexical one 
> here.
oopsy, was experimenting.



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:200
+std::string Qualifier;
+// FIXME: Also take using directives and namespace aliases inside function
+// body into account.

ilya-biryukov wrote:
> NIT: it looks like this FIXME belongs to the `getQualification` function 
> itself.
I don't think so, as qualification function has no idea about the function 
body, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69033



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

[PATCH] D70481: [clangd] Fix a crash in expected types

[Ilya Biryukov via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70481

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


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1030,6 +1030,16 @@
 SnippetSuffix("(${1:int A})";
 }
 
+TEST(CompletionTest, NoCrashWithTemplateParamsAndPreferredTypes) {
+  auto Completions = completions(R"cpp(
+template  class TT> int foo() {
+  int a = ^
+}
+)cpp")
+ .Completions;
+  EXPECT_THAT(Completions, Contains(Named("TT")));
+}
+
 SignatureHelp signatures(llvm::StringRef Text, Position Point,
  std::vector IndexSymbols = {}) {
   std::unique_ptr Index;
Index: clang-tools-extra/clangd/ExpectedTypes.cpp
===
--- clang-tools-extra/clangd/ExpectedTypes.cpp
+++ clang-tools-extra/clangd/ExpectedTypes.cpp
@@ -44,12 +44,10 @@
 static llvm::Optional
 typeOfCompletion(const CodeCompletionResult &R) {
   const NamedDecl *D = R.Declaration;
-  if (!D)
-return llvm::None;
   // Templates do not have a type on their own, look at the templated decl.
-  if (auto *Template = dyn_cast(D))
+  if (auto *Template = dyn_cast_or_null(D))
 D = Template->getTemplatedDecl();
-  auto *VD = dyn_cast(D);
+  auto *VD = dyn_cast_or_null(D);
   if (!VD)
 return llvm::None; // We handle only variables and functions below.
   auto T = VD->getType();


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1030,6 +1030,16 @@
 SnippetSuffix("(${1:int A})";
 }
 
+TEST(CompletionTest, NoCrashWithTemplateParamsAndPreferredTypes) {
+  auto Completions = completions(R"cpp(
+template  class TT> int foo() {
+  int a = ^
+}
+)cpp")
+ .Completions;
+  EXPECT_THAT(Completions, Contains(Named("TT")));
+}
+
 SignatureHelp signatures(llvm::StringRef Text, Position Point,
  std::vector IndexSymbols = {}) {
   std::unique_ptr Index;
Index: clang-tools-extra/clangd/ExpectedTypes.cpp
===
--- clang-tools-extra/clangd/ExpectedTypes.cpp
+++ clang-tools-extra/clangd/ExpectedTypes.cpp
@@ -44,12 +44,10 @@
 static llvm::Optional
 typeOfCompletion(const CodeCompletionResult &R) {
   const NamedDecl *D = R.Declaration;
-  if (!D)
-return llvm::None;
   // Templates do not have a type on their own, look at the templated decl.
-  if (auto *Template = dyn_cast(D))
+  if (auto *Template = dyn_cast_or_null(D))
 D = Template->getTemplatedDecl();
-  auto *VD = dyn_cast(D);
+  auto *VD = dyn_cast_or_null(D);
   if (!VD)
 return llvm::None; // We handle only variables and functions below.
   auto T = VD->getType();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69948: [Checkers] Added support for freopen to StreamChecker.

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

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



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:201
+std::tie(StateRetNotNull, StateRetNull) =
+CM.assumeDual(StateStreamNull, RetVal);
+if (StateRetNull) {

NoQ wrote:
> Mmm, wait, this doesn't look right to me. You cannot deduce from the presence 
> of `freopen()` in the code that the argument may be null, so the split over 
> the argument is not well-justified.
> 
> The right thing to do here will be to produce a "Schrödinger file descriptor" 
> that's both `Opened` and `OpenFailed` until we observe the return value to be 
> constrained to null or non-null later during analysis (cf. D32449), otherwise 
> conservatively assume that the open succeeded when queried for the first time 
> (because that's how non-paranoid code under analysis will behave).
> 
> Or you could drop the failed path immediately if the argument isn't known to 
> be zero. That'll drop the coverage slightly but won't cause anything bad to 
> happen.
> 
> 
Where does this comment belong? I got this in the email:
```
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:201
+std::tie(StateRetNotNull, StateRetNull) =
+CM.assumeDual(StateStreamNull, RetVal);
+if (StateRetNull) {
```
Is the problem with the null-check of argument of freopen? Why is this 
different than the other calls where `checkNullStream` is used? 
Or is the problem with the case of NULL return value from `freopen` (in this 
case the argument was non-null, for the null case we assume program crash)? In 
this case we really do not know what happened with the file descriptor argument 
(but the value of it is not changed), so the code assumes now that is will be 
OpenFailed. This is not accurate always (it may remain open or become closed). 
We could have another state split here (for these cases)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69948



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

[PATCH] D70481: [clangd] Fix a crash in expected types

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: pass - 60077 tests passed, 0 failed and 729 were skipped.
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70481



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

[PATCH] D70485: [ARM,MVE] Add intrinsics to deal with predicates.

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham created this revision.
simon_tatham added reviewers: ostannard, MarkMurrayARM, dmgreen.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya, kristof.beyls.
Herald added projects: clang, LLVM.

This commit adds the `vpselq` intrinsics which take an MVE predicate
word and select lanes from two vectors; the `vctp` intrinsics which
create a tail predicate word suitable for processing the first m
elements of a vector (e.g. in the last iteration of a loop); and
`vpnot`, which simply complements a predicate word and is just
syntactic sugar for the `~` operator.

Most sizes of `vctp` are lowered to the existing Arm IR intrinsics
that MVE shares with NEON. We already had isel patterns for converting
those into MVE VCTP, although I've added extra ones to generate the
predicated version. But I've made a special MVE version of the 64-bit
VCTP IR intrinsic, because the NEON version of that is defined to take
a `v2i1`, whereas in MVE that's not a legal type. So the MVE version
does the usual workaround of using `v4i1` instead.

I needed one small tweak in MveEmitter to allow the `unpromoted` type
modifier to apply to predicates as well as integers, so that `vpnot`
doesn't pointlessly convert its input integer to an `` before
complementing it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70485

Files:
  clang/include/clang/Basic/arm_mve.td
  clang/include/clang/Basic/arm_mve_defs.td
  clang/test/CodeGen/arm-mve-intrinsics/predicates.c
  clang/utils/TableGen/MveEmitter.cpp
  llvm/include/llvm/IR/IntrinsicsARM.td
  llvm/lib/Target/ARM/ARMInstrMVE.td
  llvm/test/CodeGen/Thumb2/mve-intrinsics/predicates.ll

Index: llvm/test/CodeGen/Thumb2/mve-intrinsics/predicates.ll
===
--- /dev/null
+++ llvm/test/CodeGen/Thumb2/mve-intrinsics/predicates.ll
@@ -0,0 +1,219 @@
+; RUN: opt -instcombine %s | llc -mtriple=thumbv8.1m.main -mattr=+mve.fp -verify-machineinstrs -o - | FileCheck %s
+
+declare <16 x i1> @llvm.arm.vctp8(i32)
+declare <8 x i1> @llvm.arm.vctp16(i32)
+declare <4 x i1> @llvm.arm.vctp32(i32)
+declare <4 x i1> @llvm.arm.mve.vctp64(i32)
+
+declare i32 @llvm.arm.mve.pred.v2i.v4i1(<4 x i1>)
+declare i32 @llvm.arm.mve.pred.v2i.v8i1(<8 x i1>)
+declare i32 @llvm.arm.mve.pred.v2i.v16i1(<16 x i1>)
+
+declare <4 x i1> @llvm.arm.mve.pred.i2v.v4i1(i32)
+declare <8 x i1> @llvm.arm.mve.pred.i2v.v8i1(i32)
+declare <16 x i1> @llvm.arm.mve.pred.i2v.v16i1(i32)
+
+define arm_aapcs_vfpcc zeroext i16 @test_vctp8q(i32 %a) {
+; CHECK-LABEL: test_vctp8q:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vctp.8 r0
+; CHECK-NEXT:vmrs r0, p0
+; CHECK-NEXT:bx lr
+entry:
+  %0 = call <16 x i1> @llvm.arm.vctp8(i32 %a)
+  %1 = call i32 @llvm.arm.mve.pred.v2i.v16i1(<16 x i1> %0)
+  %2 = trunc i32 %1 to i16
+  ret i16 %2
+}
+
+define arm_aapcs_vfpcc zeroext i16 @test_vctp8q_m(i32 %a, i16 zeroext %p) {
+; CHECK-LABEL: test_vctp8q_m:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vmsr p0, r1
+; CHECK-NEXT:vpst
+; CHECK-NEXT:vctpt.8 r0
+; CHECK-NEXT:vmrs r0, p0
+; CHECK-NEXT:bx lr
+entry:
+  %0 = zext i16 %p to i32
+  %1 = call <16 x i1> @llvm.arm.mve.pred.i2v.v16i1(i32 %0)
+  %2 = call <16 x i1> @llvm.arm.vctp8(i32 %a)
+  %3 = and <16 x i1> %1, %2
+  %4 = call i32 @llvm.arm.mve.pred.v2i.v16i1(<16 x i1> %3)
+  %5 = trunc i32 %4 to i16
+  ret i16 %5
+}
+
+define arm_aapcs_vfpcc zeroext i16 @test_vctp16q(i32 %a) {
+; CHECK-LABEL: test_vctp16q:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vctp.16 r0
+; CHECK-NEXT:vmrs r0, p0
+; CHECK-NEXT:bx lr
+entry:
+  %0 = call <8 x i1> @llvm.arm.vctp16(i32 %a)
+  %1 = call i32 @llvm.arm.mve.pred.v2i.v8i1(<8 x i1> %0)
+  %2 = trunc i32 %1 to i16
+  ret i16 %2
+}
+
+define arm_aapcs_vfpcc zeroext i16 @test_vctp16q_m(i32 %a, i16 zeroext %p) {
+; CHECK-LABEL: test_vctp16q_m:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vmsr p0, r1
+; CHECK-NEXT:vpst
+; CHECK-NEXT:vctpt.16 r0
+; CHECK-NEXT:vmrs r0, p0
+; CHECK-NEXT:bx lr
+entry:
+  %0 = zext i16 %p to i32
+  %1 = call <8 x i1> @llvm.arm.mve.pred.i2v.v8i1(i32 %0)
+  %2 = call <8 x i1> @llvm.arm.vctp16(i32 %a)
+  %3 = and <8 x i1> %1, %2
+  %4 = call i32 @llvm.arm.mve.pred.v2i.v8i1(<8 x i1> %3)
+  %5 = trunc i32 %4 to i16
+  ret i16 %5
+}
+
+define arm_aapcs_vfpcc zeroext i16 @test_vctp32q(i32 %a) {
+; CHECK-LABEL: test_vctp32q:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vctp.32 r0
+; CHECK-NEXT:vmrs r0, p0
+; CHECK-NEXT:bx lr
+entry:
+  %0 = call <4 x i1> @llvm.arm.vctp32(i32 %a)
+  %1 = call i32 @llvm.arm.mve.pred.v2i.v4i1(<4 x i1> %0)
+  %2 = trunc i32 %1 to i16
+  ret i16 %2
+}
+
+define arm_aapcs_vfpcc zeroext i16 @test_vctp32q_m(i32 %a, i16 zeroext %p) {
+; CHECK-LABEL: test_vctp32q_m:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vmsr p0, r1
+; CHECK-NEXT:vpst
+; CHECK-NEXT:vctpt.32 r0
+; CHECK-NEXT:vmrs r0, p0
+; CHECK-NEXT:bx lr
+entry:
+  %0 = zext i16 %p t

[clang-tools-extra] e18ab2a - [clangd] Treat UserDefinedLiteral as a leaf in SelectionTree, sidestepping tokenization issues

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2019-11-20T13:06:57+01:00
New Revision: e18ab2a0b801e75ee39bb8ba30584c69b4c6e577

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

LOG: [clangd] Treat UserDefinedLiteral as a leaf in SelectionTree, sidestepping 
tokenization issues

Summary: Fixes https://github.com/clangd/clangd/issues/203

Reviewers: kadircet

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

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/Selection.cpp
clang-tools-extra/clangd/unittests/SelectionTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Selection.cpp 
b/clang-tools-extra/clangd/Selection.cpp
index 04076df0c39d..c91cd24e2f25 100644
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -12,6 +12,7 @@
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/TypeLoc.h"
@@ -245,6 +246,10 @@ class SelectionVisitor : public 
RecursiveASTVisitor {
 if (canSafelySkipNode(N))
   return false;
 push(std::move(N));
+if (shouldSkipChildren(X)) {
+  pop();
+  return false;
+}
 return true;
   }
   bool dataTraverseStmtPost(Stmt *X) {
@@ -355,6 +360,15 @@ class SelectionVisitor : public 
RecursiveASTVisitor {
 return true;
   }
 
+  // There are certain nodes we want to treat as leaves in the SelectionTree,
+  // although they do have children.
+  bool shouldSkipChildren(const Stmt *X) const {
+// UserDefinedLiteral (e.g. 12_i) has two children (12 and _i).
+// Unfortunately TokenBuffer sees 12_i as one token and can't split it.
+// So we treat UserDefinedLiteral as a leaf node, owning the token.
+return llvm::isa(X);
+  }
+
   // Pushes a node onto the ancestor stack. Pairs with pop().
   // Performs early hit detection for some nodes (on the earlySourceRange).
   void push(DynTypedNode Node) {

diff  --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp 
b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
index 7b8b33d8a25b..2803aaaca1c5 100644
--- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -304,6 +304,16 @@ TEST(SelectionTest, CommonAncestor) {
 }
   )cpp",
   "CallExpr"},
+
+  // User-defined literals are tricky: is 12_i one token or two?
+  // For now we treat it as one, and the UserDefinedLiteral as a leaf.
+  {
+  R"cpp(
+struct Foo{};
+Foo operator""_ud(unsigned long long);
+Foo x = [[^12_ud]];
+  )cpp",
+  "UserDefinedLiteral"},
   };
   for (const Case &C : Cases) {
 Annotations Test(C.Code);



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

[PATCH] D70480: [clangd] Use expansion location when the ref is inside macros.

[Haojian Wu via Phabricator via cfe-commits]

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

add more tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70480

Files:
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp


Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -644,6 +644,27 @@
   HaveRanges(Header.ranges();
 }
 
+TEST_F(SymbolCollectorTest, RefsOnMacros) {
+  CollectorOpts.RefFilter = RefKind::All;
+  CollectorOpts.RefsInHeaders = true;
+  Annotations Header(R"(
+#define TYPE(X) X
+#define FOO Foo
+#define CAT(X, Y) X##Y
+class [[Foo]] {};
+void test() {
+  TYPE([[Foo]]) foo;
+  [[FOO]] foo2;
+  TYPE(TYPE([[Foo]])) foo3;
+  [[CAT]](Fo, o) foo4;
+}
+  )");
+  CollectorOpts.RefFilter = RefKind::All;
+  runSymbolCollector(Header.code(), "");
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
+  HaveRanges(Header.ranges();
+}
+
 TEST_F(SymbolCollectorTest, HeaderAsMainFile) {
   CollectorOpts.RefFilter = RefKind::All;
   Annotations Header(R"(
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -275,10 +275,9 @@
   // Mark D as referenced if this is a reference coming from the main file.
   // D may not be an interesting symbol, but it's cheaper to check at the end.
   auto &SM = ASTCtx->getSourceManager();
-  auto SpellingLoc = SM.getSpellingLoc(Loc);
   if (Opts.CountReferences &&
   (Roles & static_cast(index::SymbolRole::Reference)) &&
-  SM.getFileID(SpellingLoc) == SM.getMainFileID())
+  SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainFileID())
 ReferencedDecls.insert(ND);
 
   auto ID = getSymbolID(ND);
@@ -312,8 +311,9 @@
 return true;
   // Do not store references to main-file symbols.
   if (CollectRef && !IsMainFileOnly && !isa(ND) &&
-  (Opts.RefsInHeaders || SM.getFileID(SpellingLoc) == SM.getMainFileID()))
-DeclRefs[ND].emplace_back(SpellingLoc, Roles);
+  (Opts.RefsInHeaders ||
+   SM.getFileID(SM.getFileLoc(Loc)) == SM.getMainFileID()))
+DeclRefs[ND].emplace_back(SM.getFileLoc(Loc), Roles);
   // Don't continue indexing if this is a mere reference.
   if (IsOnlyRef)
 return true;


Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -644,6 +644,27 @@
   HaveRanges(Header.ranges();
 }
 
+TEST_F(SymbolCollectorTest, RefsOnMacros) {
+  CollectorOpts.RefFilter = RefKind::All;
+  CollectorOpts.RefsInHeaders = true;
+  Annotations Header(R"(
+#define TYPE(X) X
+#define FOO Foo
+#define CAT(X, Y) X##Y
+class [[Foo]] {};
+void test() {
+  TYPE([[Foo]]) foo;
+  [[FOO]] foo2;
+  TYPE(TYPE([[Foo]])) foo3;
+  [[CAT]](Fo, o) foo4;
+}
+  )");
+  CollectorOpts.RefFilter = RefKind::All;
+  runSymbolCollector(Header.code(), "");
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
+  HaveRanges(Header.ranges();
+}
+
 TEST_F(SymbolCollectorTest, HeaderAsMainFile) {
   CollectorOpts.RefFilter = RefKind::All;
   Annotations Header(R"(
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -275,10 +275,9 @@
   // Mark D as referenced if this is a reference coming from the main file.
   // D may not be an interesting symbol, but it's cheaper to check at the end.
   auto &SM = ASTCtx->getSourceManager();
-  auto SpellingLoc = SM.getSpellingLoc(Loc);
   if (Opts.CountReferences &&
   (Roles & static_cast(index::SymbolRole::Reference)) &&
-  SM.getFileID(SpellingLoc) == SM.getMainFileID())
+  SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainFileID())
 ReferencedDecls.insert(ND);
 
   auto ID = getSymbolID(ND);
@@ -312,8 +311,9 @@
 return true;
   // Do not store references to main-file symbols.
   if (CollectRef && !IsMainFileOnly && !isa(ND) &&
-  (Opts.RefsInHeaders || SM.getFileID(SpellingLoc) == SM.getMainFileID()))
-DeclRefs[ND].emplace_back(SpellingLoc, Roles);
+  (Opts.RefsInHeaders ||
+   SM.getFileID(SM.getFileLoc(Loc)

[PATCH] D50147: clang-format: support external styles

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D50147#1751198 , @Typz wrote:

> >> In our case, we actually have more than one "standard" style, a 
> >> combination of various OS (linux, windows, macos), and not a very strong 
> >> control on user computers. So we cannot rely on a specific file or 
> >> variable being setup by an administrator.
> > 
> > In this case my best advice would be in the short term to use .clang-format 
> > files. Longer term, some combination of using well-known styles, 
> > publicising and teaching clang-format about the styles you use, and gaining 
> > the ability to set environment variables would reduce duplication.
>
> what do you mean about "gaining the ability to set environment variables" ?


If you can set a CLANG_FORMAT_STYLE_PATH or so environment variable, then you 
can install your styles there

> - Build option is implemented. This allows turn the feature off if needed, at 
> build time (by specifying empty search path). I would prefer to keep thos
> - Overriding an environment varialbe to change the search path is fine by me. 
> But I would still prefer to have a "working" default, so that it can be used 
> out-of-the-box, with no extra env variable to set

OK, I'm not going to approve turning this feature on by default (specifically: 
searching unknown strings as paths relative to some directory baked into the 
binary). I think it's bad for the ecosystem.
If you want to use absolute paths, or set the path with a build option, 
environment variable, or flag that seems fine to me.

> The goal is indeed that user keep installing clang-format through  LLVM 
> releases or OS distributors; but they would also install the custom styles 
> they need, which would be provided by their organization (not clang/llvm!): 
> e.g. Qt style, or "my-compagny" style.

My concern about the ecosystem is largely about requiring users to know which 
styles they need.


Repository:
  rC Clang

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

https://reviews.llvm.org/D50147



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

[PATCH] D70480: [clangd] Use expansion location when the ref is inside macros.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:280
   (Roles & static_cast(index::SymbolRole::Reference)) &&
-  SM.getFileID(SpellingLoc) == SM.getMainFileID())
+  SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainFileID())
 ReferencedDecls.insert(ND);

ilya-biryukov wrote:
> We're using `getSpellingLoc` here and `getFileLoc` later. Why not use 
> `getFileLoc` everywhere?
> 
> Having a variable (similar to the `SpellingLoc` we had before) and calling 
> `getFileLoc` only once also seems preferable.
> We're using getSpellingLoc here and getFileLoc later. Why not use getFileLoc 
> everywhere?

There are two things in SymbolCollector:
- symbols & ranking signals, we use spelling location for them, the code is 
part of this, `ReferencedDecls` is used to calculate the ranking
- references

this patch only targets the reference part (changing the loc here would break 
many assumptions I think, and there was a failure test).



Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:655
+void test() {
+  TYPE([[Foo]]) foo;
+  [[FOO]] foo2;

ilya-biryukov wrote:
> Could you also check:
> 1. Multi-level macro arguments `TYPE(TYPE(FOO))`
> 2. Concatenated tokens as identifiers, e.g. coming from `#define CAT(X, Y) 
> X##Y`
Added tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70480



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

[PATCH] D60455: [SYCL] Implement SYCL device code outlining

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10076
+def warn_sycl_kernel_num_of_template_params : Warning<
+  "'sycl_kernel' attribute only applies to template functions with at least"
+  " two template parameters">, InGroup;

Do you mean template function or function template? A function template is a 
template used to generate functions and a template function is a function 
produced by a template. I think you probably mean "function template" here.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10079-10080
+def warn_sycl_kernel_invalid_template_param_type : Warning<
+  "template parameter of template functions with 'sycl_kernel' attribute must"
+  " be typename">, InGroup;
+def warn_sycl_kernel_num_of_function_params : Warning<

This diagnostic reads a bit like you cannot do this: `template ` when 
I think the actual restriction is that you cannot do this: `template `. 
Is that correct? If so, I think this could be worded as `template parameter of 
a function template with the 'sycl_kernel' attribute must be a template type 
parameter`.

Just double-checking, but you also intend to prohibit template template 
parameters? e.g., you can't do `template  typename C>`



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10082
+def warn_sycl_kernel_num_of_function_params : Warning<
+  "template function with 'sycl_kernel' attribute must have single parameter">,
+  InGroup;

Probably "function template" here as well.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10085
+def warn_sycl_kernel_return_type : Warning<
+  "template function with 'sycl_kernel' attribute must return void type">,
+  InGroup;

Same here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

[PATCH] D70441: [clangd] Speed up when building rename edit.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 230242.
hokein marked 2 inline comments as done.
hokein added a comment.
Herald added a subscriber: mgrang.

address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70441

Files:
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/refactor/Rename.h
  clang-tools-extra/clangd/unittests/RenameTests.cpp

Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -638,6 +638,33 @@
   UnorderedElementsAre(Pair(Eq(Path), Eq(expectedResult(Code, NewName);
 }
 
+TEST(CrossFileRenameTests, BuildRenameEdits) {
+  Annotations Code("[[😂]]");
+  auto LSPRange = Code.range();
+  auto Edit = buildRenameEdit(Code.code(), {LSPRange}, "abc");
+  ASSERT_TRUE(bool(Edit)) << Edit.takeError();
+  ASSERT_EQ(1UL, Edit->Replacements.size());
+  EXPECT_EQ(4UL, Edit->Replacements.begin()->getLength());
+
+  // Test invalid range.
+  LSPRange.end = {-1, 0}; // out of range
+  Edit = buildRenameEdit(Code.code(), {LSPRange}, "abc");
+  EXPECT_FALSE(Edit);
+  EXPECT_THAT(llvm::toString(Edit.takeError()),
+  testing::HasSubstr("fail to convert"));
+
+  // Normal ascii characters.
+  Annotations T(R"cpp(
+[[range]]
+  [[range]]
+  [[range]]
+  )cpp");
+  Edit = buildRenameEdit(T.code(), T.ranges(), "abc");
+  ASSERT_TRUE(bool(Edit)) << Edit.takeError();
+  EXPECT_EQ(applyEdits(FileEdits{{T.code(), std::move(*Edit)}}).front().second,
+expectedResult(Code, expectedResult(T, "abc")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/refactor/Rename.h
===
--- clang-tools-extra/clangd/refactor/Rename.h
+++ clang-tools-extra/clangd/refactor/Rename.h
@@ -47,6 +47,13 @@
 /// in another file (per the index).
 llvm::Expected rename(const RenameInputs &RInputs);
 
+/// Generates rename edits that replaces all given occurrences with the
+/// NewName.
+/// Exposed for testing only.
+llvm::Expected buildRenameEdit(llvm::StringRef InitialCode,
+ std::vector Occurrences,
+ llvm::StringRef NewName);
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -20,6 +20,7 @@
 #include "clang/Tooling/Refactoring/Rename/USRFindingAction.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/FormatVariadic.h"
 
 namespace clang {
 namespace clangd {
@@ -297,34 +298,6 @@
   return AffectedFiles;
 }
 
-llvm::Expected> toRangeOffset(const clangd::Range &R,
-llvm::StringRef Code) {
-  auto StartOffset = positionToOffset(Code, R.start);
-  if (!StartOffset)
-return StartOffset.takeError();
-  auto EndOffset = positionToOffset(Code, R.end);
-  if (!EndOffset)
-return EndOffset.takeError();
-  return std::make_pair(*StartOffset, *EndOffset);
-};
-
-llvm::Expected buildRenameEdit(llvm::StringRef InitialCode,
- const std::vector &Occurrences,
- llvm::StringRef NewName) {
-  tooling::Replacements RenameEdit;
-  for (const Range &Occurrence : Occurrences) {
-// FIXME: !positionToOffset is O(N), optimize it.
-auto RangeOffset = toRangeOffset(Occurrence, InitialCode);
-if (!RangeOffset)
-  return RangeOffset.takeError();
-auto ByteLength = RangeOffset->second - RangeOffset->first;
-if (auto Err = RenameEdit.add(tooling::Replacement(
-InitialCode, RangeOffset->first, ByteLength, NewName)))
-  return std::move(Err);
-  }
-  return Edit(InitialCode, std::move(RenameEdit));
-}
-
 // Index-based rename, it renames all occurrences outside of the main file.
 //
 // The cross-file rename is purely based on the index, as we don't want to
@@ -358,7 +331,7 @@
 llvm::inconvertibleErrorCode());
 
   FileEdits Results;
-  for (const auto &FileAndOccurrences : AffectedFiles) {
+  for (auto &FileAndOccurrences : AffectedFiles) {
 llvm::StringRef FilePath = FileAndOccurrences.first();
 
 auto AffectedFileCode = GetFileContent(FilePath);
@@ -366,11 +339,14 @@
   elog("Fail to read file content: {0}", AffectedFileCode.takeError());
   continue;
 }
-
-auto RenameEdit = buildRenameEdit(*AffectedFileCode,
-  FileAndOccurrences.getValue(), NewName);
-if (!RenameEdit)
-  return RenameEdit.takeError();
+auto RenameEdit = buildRenameEdit(
+

[PATCH] D70481: [clangd] Fix a crash in expected types

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70481



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

[PATCH] D70320: [Analyzer] [NFC] Iterator Checkers - Separate iterator modeling and the actual checkers

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

baloghadamsoftware marked an inline comment as done and an inline comment as 
not done.
baloghadamsoftware added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:1308-1318
+ProgramStateRef removeIteratorPosition(ProgramStateRef State, const SVal &Val) 
{
   if (auto Reg = Val.getAsRegion()) {
 Reg = Reg->getMostDerivedObjectRegion();
-return State->get(Reg);
+return State->remove(Reg);
   } else if (const auto Sym = Val.getAsSymbol()) {
-return State->get(Sym);
+return State->remove(Sym);
   } else if (const auto LCVal = Val.getAs()) {

Charusso wrote:
> baloghadamsoftware wrote:
> > NoQ wrote:
> > > Maybe move this function to `Iterator.cpp` as well, and then move the 
> > > definitions for iterator maps from `Iterator.h` to `Iterator.cpp`, which 
> > > will allow you to use the usual `REGISTER_MAP_WITH_PROGRAMSTATE` macros, 
> > > and additionally guarantee that all access to the maps goes through the 
> > > accessor methods that you provide?
> > Hmm, I was trying hard to use these macros but failed so I reverted to the 
> > manual solution. I will retry now.
> Here is a how-to: https://reviews.llvm.org/D69726
> 
> You need to add the fully qualified names to the register macro because of 
> the global scope. I hope it helps.
OK, I checked it now. If we want to put the maps into `Iterator.cpp` then we 
also have to move a couple of functions there which are only used by the 
modeling part: the internals of `checkDeadSymbols()` and `checkLiveSymbols()` 
must go there, although no other checker should use them. Also 
`processIteratorPositions()` iterates over these maps, thus it must also go 
there. Should I do it? Generally I like the current solution better, only 
functions used by multiple checker classes are in the library.

On the other hand I do not see why I should move `assumeNoOverflow()` to 
`Iterator.cpp`? This function is only used by the modeling part, and does not 
refer to the maps. You mean that we should ensure this constraint whenever 
setting the position for any iterator? This would mean that we should decompose 
every symblic expression and (re)assume this range on the symbolic part. Or we 
should replace `setPosition()` by at least two different functions (e.g. 
`setAdvancedPosition()` and `setConjuredPosition()`) but this means a total 
reqriting of the modeling.

I plan some refactoring but this first patch is meant to be a single separation 
of code.


Repository:
  rC Clang

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

https://reviews.llvm.org/D70320



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

[PATCH] D70488: [InstCombine] Infer fast math flags on fadd/fsub/fmul/fcmp

[Benjamin Kramer via Phabricator via cfe-commits]

bkramer created this revision.
bkramer added a reviewer: spatel.
Herald added subscribers: cfe-commits, hiraditya.
Herald added projects: clang, LLVM.

Applies nnan, ninf and nsz. This allows more instructions to be folded
away even with no fast math flags, e.g. (int)x * (b ? 1.0 : 0.0) -> b ? x : 0.0

As a side effect this will propagate fast math flags out of inlined fast
math code into surrounding functions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70488

Files:
  clang/test/CodeGen/builtins-systemz-zvector.c
  llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
  llvm/lib/Transforms/InstCombine/InstCombineInternal.h
  llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
  llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
  llvm/test/Transforms/InstCombine/add-sitofp.ll
  llvm/test/Transforms/InstCombine/cast-int-fcmp-eq-0.ll
  llvm/test/Transforms/InstCombine/fadd-fsub-factor.ll
  llvm/test/Transforms/InstCombine/fast-math.ll
  llvm/test/Transforms/InstCombine/fcmp.ll
  llvm/test/Transforms/InstCombine/known-never-nan.ll
  llvm/test/Transforms/InstCombine/minmax-fp.ll
  llvm/test/Transforms/InstCombine/pow_fp_int.ll

Index: llvm/test/Transforms/InstCombine/pow_fp_int.ll
===
--- llvm/test/Transforms/InstCombine/pow_fp_int.ll
+++ llvm/test/Transforms/InstCombine/pow_fp_int.ll
@@ -91,7 +91,7 @@
 define double @pow_uitofp_const_base_power_of_2_fast(i31 %x) {
 ; CHECK-LABEL: @pow_uitofp_const_base_power_of_2_fast(
 ; CHECK-NEXT:[[SUBFP:%.*]] = uitofp i31 [[X:%.*]] to float
-; CHECK-NEXT:[[MUL:%.*]] = fmul afn float [[SUBFP]], 4.00e+00
+; CHECK-NEXT:[[MUL:%.*]] = fmul nnan afn float [[SUBFP]], 4.00e+00
 ; CHECK-NEXT:[[EXP2:%.*]] = call afn float @llvm.exp2.f32(float [[MUL]])
 ; CHECK-NEXT:[[RES:%.*]] = fpext float [[EXP2]] to double
 ; CHECK-NEXT:ret double [[RES]]
@@ -383,7 +383,7 @@
 define double @pow_uitofp_const_base_power_of_2_no_fast(i32 %x) {
 ; CHECK-LABEL: @pow_uitofp_const_base_power_of_2_no_fast(
 ; CHECK-NEXT:[[SUBFP:%.*]] = uitofp i32 [[X:%.*]] to float
-; CHECK-NEXT:[[MUL:%.*]] = fmul float [[SUBFP]], 4.00e+00
+; CHECK-NEXT:[[MUL:%.*]] = fmul nnan float [[SUBFP]], 4.00e+00
 ; CHECK-NEXT:[[EXP2:%.*]] = call float @llvm.exp2.f32(float [[MUL]])
 ; CHECK-NEXT:[[RES:%.*]] = fpext float [[EXP2]] to double
 ; CHECK-NEXT:ret double [[RES]]
Index: llvm/test/Transforms/InstCombine/minmax-fp.ll
===
--- llvm/test/Transforms/InstCombine/minmax-fp.ll
+++ llvm/test/Transforms/InstCombine/minmax-fp.ll
@@ -273,7 +273,7 @@
 ; CHECK-LABEL: define {{[^@]+}}@fsub_fmax(
 ; CHECK-NEXT:[[COND_INV:%.*]] = fcmp nnan nsz ogt <2 x float> [[X:%.*]], [[Y:%.*]]
 ; CHECK-NEXT:[[TMP1:%.*]] = select nnan nsz <2 x i1> [[COND_INV]], <2 x float> [[Y]], <2 x float> [[X]]
-; CHECK-NEXT:[[MAX:%.*]] = fsub <2 x float> , [[TMP1]]
+; CHECK-NEXT:[[MAX:%.*]] = fsub nnan <2 x float> , [[TMP1]]
 ; CHECK-NEXT:ret <2 x float> [[MAX]]
 ;
   %n1 = fsub <2 x float> , %x
Index: llvm/test/Transforms/InstCombine/known-never-nan.ll
===
--- llvm/test/Transforms/InstCombine/known-never-nan.ll
+++ llvm/test/Transforms/InstCombine/known-never-nan.ll
@@ -11,7 +11,7 @@
 ; CHECK-LABEL: @fabs_sqrt_src_maybe_nan(
 ; CHECK-NEXT:[[FABS:%.*]] = call double @llvm.fabs.f64(double [[ARG0:%.*]])
 ; CHECK-NEXT:[[OP:%.*]] = call double @llvm.sqrt.f64(double [[FABS]])
-; CHECK-NEXT:[[TMP:%.*]] = fcmp ord double [[OP]], 0.00e+00
+; CHECK-NEXT:[[TMP:%.*]] = fcmp nsz ord double [[OP]], 0.00e+00
 ; CHECK-NEXT:ret i1 [[TMP]]
 ;
   %fabs = call double @llvm.fabs.f64(double %arg0)
Index: llvm/test/Transforms/InstCombine/fcmp.ll
===
--- llvm/test/Transforms/InstCombine/fcmp.ll
+++ llvm/test/Transforms/InstCombine/fcmp.ll
@@ -535,11 +535,11 @@
 ; Do not fold 1.0 / X > 0.0 when ninf is missing
 define i1 @test24_recipX_noninf_cmp(float %X) {
 ; CHECK-LABEL: @test24_recipX_noninf_cmp(
-; CHECK-NEXT:[[DIV:%.*]] = fdiv ninf float 2.00e+00, [[X:%.*]]
+; CHECK-NEXT:[[DIV:%.*]] = fdiv float 2.00e+00, [[X:%.*]]
 ; CHECK-NEXT:[[CMP:%.*]] = fcmp ogt float [[DIV]], 0.00e+00
 ; CHECK-NEXT:ret i1 [[CMP]]
 ;
-  %div = fdiv ninf float 2.0, %X
+  %div = fdiv float 2.0, %X
   %cmp = fcmp ogt float %div, 0.0
   ret i1 %cmp
 }
Index: llvm/test/Transforms/InstCombine/fast-math.ll
===
--- llvm/test/Transforms/InstCombine/fast-math.ll
+++ llvm/test/Transforms/InstCombine/fast-math.ll
@@ -18,7 +18,7 @@
 define float @notfold(float %a) {
 ; CHECK-LABEL: @notfold(
 ; CHECK-NEXT:[[MUL:%.*]] = fmul fast float [[A:%.*]], 0x3FF34000
-; CHECK-NEXT

[PATCH] D70441: [clangd] Speed up when building rename edit.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/SourceCode.h:54
+/// Use of UTF-16 may be overridden by kCurrentOffsetEncoding.
+llvm::Expected byteOffset(StringRef LineCode, int LSPCharacter);
+

ilya-biryukov wrote:
> How this is different from `positionToOffset` with `Pos.line = 0` and 
> `Pos.column = LSPCharacter`?
> Why do we need an extra function?
removed now, not needed.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:441
 
+llvm::Expected buildRenameEdit(llvm::StringRef InitialCode,
+ const std::vector &Occurrences,

ilya-biryukov wrote:
> We are aiming to convert all ranges in `O(n)` instead of `O(n^2)`, right?
> I think this could be simpler and also have guaranteed performance even for 
> long lines if we do it slightly differently. WDYT?
> 
> ```
> assert(llvm::is_sorted(Occurences));
> // These two always correspond to the same position.
> Position LastPos = Position{0, 0};
> size_t LastOffset = 0;
> 
> std::vector> Offsets;
> for (auto R : Occurences) {
>   Position ShiftedStart = R.start - LastPos;
>   size_t ShiftedStartOffset = positionToOffset(ShiftedStart, 
> Code.substr(LastOffset);
> 
>   LastPos = ShiftedStart;
>   LastOffset  = ShiftedStartOffset;
>   // Do the same for the end position.
>   // ...
> }
> 
> // Now we can easily replacements.
> // ...
> ```
agree, this is smart. note that we need to pay the cost of sorting, but I think 
it is totally ok as the number is relatively small.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70441



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

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

[Hans Wennborg via Phabricator via cfe-commits]

hans added inline comments.



Comment at: clang/lib/Driver/Job.cpp:347
+StringRef DriverExe = llvm::sys::path::stem(D.ClangExecutable);
+if (CommandExe.equals_lower(DriverExe))
+CC1Main = Driver::CC1Main;

aganea wrote:
> hans wrote:
> > Now that we're not calling main() anymore, but cc1 directly -- is this 
> > checking still necessary?
> Yes it is - see my other comment just above.
> 
> The driver builds phases that do not always call the cc1 process. Simply 
> stating `clang a.cpp` would invoke `clang -cc1`, then the linker, say 
> `link.exe`. In this later case (invoking `link.exe`), even if we have 
> `Driver::Main` it doesn't mean we should use it. There are a number of other 
> edge cases of the same kind, such as `/fallback` or building cuda files, 
> where a different executable is invoked along the way."
Okay, but couldn't we find a cleaner way of figuring out that the Command is a 
cc1 invocation? The thing that creates the Command would know.. maybe it could 
set a flag or something? Or maybe instead of creating a Command it would create 
something else?



Comment at: clang/lib/Driver/Job.cpp:337
 
-  if (ResponseFile == nullptr) {
+  Driver::CC1ToolFunc CC1Main = nullptr;
+

I think it would be clearer with a boolean "CallCC1" or something instead of a 
function pointer.


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

https://reviews.llvm.org/D69825



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

[PATCH] D70489: [clangd] Add xref for macro to static index.

[UTKARSH SAXENA via Phabricator via cfe-commits]

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

This adds the references for macros to the SymbolCollector (used for static 
index).
Enabled if `CollectMacro` option is set.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70489

Files:
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/SymbolCollector.h
  clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp

Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -39,6 +39,7 @@
 using ::testing::Each;
 using ::testing::ElementsAre;
 using ::testing::Field;
+using ::testing::IsEmpty;
 using ::testing::Not;
 using ::testing::Pair;
 using ::testing::UnorderedElementsAre;
@@ -577,15 +578,16 @@
 
 TEST_F(SymbolCollectorTest, Refs) {
   Annotations Header(R"(
-  class $foo[[Foo]] {
+  #define $macro[[MACRO]](X) (X + 1)
+  class Foo {
   public:
-$foo[[Foo]]() {}
-$foo[[Foo]](int);
+Foo() {}
+Foo(int);
   };
-  class $bar[[Bar]];
-  void $func[[func]]();
+  class Bar;
+  void func();
 
-  namespace $ns[[NS]] {} // namespace ref is ignored
+  namespace NS {} // namespace ref is ignored
   )");
   Annotations Main(R"(
   class $bar[[Bar]] {};
@@ -598,19 +600,20 @@
 $func[[func]]();
 int abc = 0;
 $foo[[Foo]] foo2 = abc;
+abc = $macro[[MACRO]](1);
   }
   )");
   Annotations SymbolsOnlyInMainCode(R"(
+  #define FUNC(X) (X+1)
   int a;
   void b() {}
-  static const int c = 0;
+  static const int c = FUNC(1);
   class d {};
   )");
   CollectorOpts.RefFilter = RefKind::All;
+  CollectorOpts.CollectMacro = true;
   runSymbolCollector(Header.code(),
  (Main.code() + SymbolsOnlyInMainCode.code()).str());
-  auto HeaderSymbols = TestTU::withHeaderCode(Header.code()).headerSymbols();
-
   EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
   HaveRanges(Main.ranges("foo");
   EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Bar").ID,
@@ -618,12 +621,65 @@
   EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "func").ID,
   HaveRanges(Main.ranges("func");
   EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(Symbols, "NS").ID, _;
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "MACRO").ID,
+  HaveRanges(Main.ranges("macro");
   // Symbols *only* in the main file (a, b, c) had no refs collected.
   auto MainSymbols =
   TestTU::withHeaderCode(SymbolsOnlyInMainCode.code()).headerSymbols();
   EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "a").ID, _;
   EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "b").ID, _;
   EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "c").ID, _;
+  EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "FUNC").ID, _;
+}
+
+TEST_F(SymbolCollectorTest, MacroRef) {
+  Annotations Main(R"(
+  #define $foo[[FOO]](X) (X + 1)
+  #define $bar[[BAR]](X) (X + 2)
+
+  // FIXME: No references for undef'ed macros.
+  #define $ud[[UD]] 1
+  // #undef UD
+
+  // Macros from token concatenations not included.
+  #define $concat[[CONCAT]](X) X##A()
+  #define $prepend[[PREPEND]](X) MACRO##X()
+  #define $macroa[[MACROA]]() 123
+  int B = $concat[[CONCAT]](MACRO);
+  int D = $prepend[[PREPEND]](A);
+
+  void fff() {
+int abc = $foo[[FOO]](1) + $bar[[BAR]]($foo[[FOO]](1));
+  }
+  )");
+  CollectorOpts.RefFilter = RefKind::All;
+  CollectorOpts.CollectMacro = true;
+  CollectorOpts.CollectMainFileSymbols = true;
+  
+  runSymbolCollector("", Main.code());
+
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "FOO").ID,
+  HaveRanges(Main.ranges("foo");
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "BAR").ID,
+  HaveRanges(Main.ranges("bar");
+  EXPECT_THAT(Refs, Contains(Pair(_,
+  HaveRanges(Main.ranges("ud");
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "CONCAT").ID,
+  HaveRanges(Main.ranges("concat");
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "PREPEND").ID,
+  HaveRanges(Main.ranges("prepend");
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "MACROA").ID,
+  HaveRanges(Main.ranges("macroa");
+}
+
+TEST_F(SymbolCollectorTest, RefsWithoutMacros) {
+  Annotations Header("#define $macro[[MACRO]](X) (X + 1)");
+  Annotations Main("void foo() { int x = $macro[[MACRO]](1); }");
+  CollectorOpts.RefFilter = RefKind::All;
+  CollectorOpts.CollectMacro = false;
+  

[PATCH] D70489: [clangd] Add xref for macro to static index.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: pass - 60166 tests passed, 0 failed and 730 were skipped.
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70489



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

[PATCH] D70480: [clangd] Use expansion location when the ref is inside macros.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: pass - 60164 tests passed, 0 failed and 731 were skipped.
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70480



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

[PATCH] D69855: [clang-tidy] Fix llvm-namespace-comment for macro expansions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp:83
+const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) 
{
+  PP->addPPCallbacks(std::make_unique(PP, this));
+}

twardakm wrote:
> aaron.ballman wrote:
> > Rather than making an entire new object for PP callbacks, why not make 
> > `NamespaceCommentCheck` inherit from `PPCallbacks`? It seems like it would 
> > simplify the interface somewhat.
> I think the check hast to inherit from ClangTidyCheck?
> 
> I duplicated the solution from other checks (e.g. IdentifierNamingCheck.cpp). 
> Could you please point to some existing check which implements your idea?
I don't know if we have other checks doing this -- I was thinking of using 
multiple inheritance in this case, because the callbacks are effectively a 
mixin.

That said, I don't insist on this change.



Comment at: 
clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp:101-104
+if (!IsNamespaceMacroExpansion)
+  Fix.append(" ").append(ND->getNameAsString());
+else
+  Fix.append(" ").append(MacroDefinition);

Would this work instead `Fix.append(" ").append(IsNamespaceMacroExpansion ? 
MacroDefinition : ND->getName());`?

(You may have to check the behavioral difference of `getName()` vs 
`getNameAsString()` to be sure.)



Comment at: 
clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp:135
+const StringRef NameSpaceName) {
+  const auto &MacroIt = std::find_if(std::begin(Macros), std::end(Macros),
+ [&NameSpaceName](const auto &Macro) {

`llvm::find_if(Macros, ...);`



Comment at: 
clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp:247
+  // Allow using macro definitions in closing comment.
+  if (isNamespaceMacroDefinition(NamespaceNameInComment)) {
+return;

Can elide braces.



Comment at: 
clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp:287
+
+  if (IsNamespaceMacroExpansion) {
+NestedNamespaceName = MacroDefinition;

Elide braces


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69855



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

[clang] b80e483 - Update tests after change to llvm-cxxfilt's underscore stripping behaviour.

[Tim Northover via cfe-commits]

Author: Tim Northover
Date: 2019-11-20T13:10:55Z
New Revision: b80e483c4205d216f6648d7e217183694fe9a55e

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

LOG: Update tests after change to llvm-cxxfilt's underscore stripping behaviour.

Added: 


Modified: 
clang/test/CodeGen/ppc-emmintrin.c
clang/test/CodeGen/ppc-mmintrin.c
clang/test/CodeGen/ppc-pmmintrin.c
clang/test/CodeGen/ppc-smmintrin.c
clang/test/CodeGen/ppc-tmmintrin.c
clang/test/CodeGen/ppc-xmmintrin.c

Removed: 




diff  --git a/clang/test/CodeGen/ppc-emmintrin.c 
b/clang/test/CodeGen/ppc-emmintrin.c
index 5e0ff67e4dd1..631b6c9d2614 100644
--- a/clang/test/CodeGen/ppc-emmintrin.c
+++ b/clang/test/CodeGen/ppc-emmintrin.c
@@ -2,9 +2,9 @@
 // REQUIRES: powerpc-registered-target
 
 // RUN: %clang -S -emit-llvm -target powerpc64-unknown-linux-gnu -mcpu=pwr8 
-ffreestanding -DNO_WARN_X86_INTRINSICS %s \
-// RUN:  -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | 
llvm-cxxfilt | FileCheck %s --check-prefixes=CHECK,CHECK-BE
+// RUN:  -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | 
llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-BE
 // RUN: %clang -S -emit-llvm -target powerpc64le-unknown-linux-gnu -mcpu=pwr8 
-ffreestanding -DNO_WARN_X86_INTRINSICS %s \
-// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | 
llvm-cxxfilt | FileCheck %s --check-prefixes=CHECK,CHECK-LE
+// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | 
llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE
 
 // CHECK-BE-DAG: @_mm_movemask_pd.perm_mask = internal constant <4 x i32> , align 16
 // CHECK-BE-DAG: @_mm_shuffle_epi32.permute_selectors = internal constant [4 x 
i32] [i32 66051, i32 67438087, i32 134810123, i32 202182159], align 4

diff  --git a/clang/test/CodeGen/ppc-mmintrin.c 
b/clang/test/CodeGen/ppc-mmintrin.c
index 019672863331..0a43f32fb0b0 100644
--- a/clang/test/CodeGen/ppc-mmintrin.c
+++ b/clang/test/CodeGen/ppc-mmintrin.c
@@ -2,13 +2,13 @@
 // REQUIRES: powerpc-registered-target
 
 // RUN: %clang -S -emit-llvm -target powerpc64-unknown-linux-gnu -mcpu=pwr8 
-DNO_WARN_X86_INTRINSICS %s \
-// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | 
llvm-cxxfilt | FileCheck %s --check-prefixes=CHECK-P8,CHECK,CHECK-BE
+// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | 
llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK-P8,CHECK,CHECK-BE
 // RUN: %clang -S -emit-llvm -target powerpc64le-unknown-linux-gnu -mcpu=pwr8 
-DNO_WARN_X86_INTRINSICS %s \
-// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | 
llvm-cxxfilt | FileCheck %s --check-prefixes=CHECK-P8,CHECK,CHECK-LE
+// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | 
llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK-P8,CHECK,CHECK-LE
 // RUN: %clang -S -emit-llvm -target powerpc64-unknown-linux-gnu -mcpu=pwr9 
-DNO_WARN_X86_INTRINSICS %s \
-// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | 
llvm-cxxfilt | FileCheck %s --check-prefixes=CHECK-P9,CHECK,CHECK-BE
+// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | 
llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK-P9,CHECK,CHECK-BE
 // RUN: %clang -S -emit-llvm -target powerpc64le-unknown-linux-gnu -mcpu=pwr9 
-DNO_WARN_X86_INTRINSICS %s \
-// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | 
llvm-cxxfilt | FileCheck %s --check-prefixes=CHECK-P9,CHECK,CHECK-LE
+// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | 
llvm-cxxfilt -n| FileCheck %s --check-prefixes=CHECK-P9,CHECK,CHECK-LE
 
 #include 
 

diff  --git a/clang/test/CodeGen/ppc-pmmintrin.c 
b/clang/test/CodeGen/ppc-pmmintrin.c
index ee4e89837444..f56a6a9993df 100644
--- a/clang/test/CodeGen/ppc-pmmintrin.c
+++ b/clang/test/CodeGen/ppc-pmmintrin.c
@@ -2,9 +2,9 @@
 // REQUIRES: powerpc-registered-target
 
 // RUN: %clang -S -emit-llvm -target powerpc64-gnu-linux -mcpu=pwr8 
-DNO_MM_MALLOC -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
-// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | 
llvm-cxxfilt | FileCheck %s
+// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | 
llvm-cxxfilt -n | FileCheck %s
 // RUN: %clang -S -emit-llvm -target powerpc64le-gnu-linux -mcpu=pwr8 
-DNO_MM_MALLOC -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
-// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | 
llvm-cxxfilt | FileCheck %s
+// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | 
llvm-cxxfilt -n | FileCheck %s
 
 #include 
 

diff  --git a/clang/test/CodeGen/ppc-smmintrin.c 
b/clang/test/CodeGen/ppc-smmintrin.c
index 666bc0fc440c..c3245ab19d1f 100644
--- a/clang/test/CodeGen/ppc-smmintrin.c
+++

[PATCH] D70488: [InstCombine] Infer fast math flags on fadd/fsub/fmul/fcmp

[Benjamin Kramer via Phabricator via cfe-commits]

bkramer updated this revision to Diff 230245.
bkramer added a comment.

Fix condition


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70488

Files:
  clang/test/CodeGen/builtins-systemz-zvector.c
  llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
  llvm/lib/Transforms/InstCombine/InstCombineInternal.h
  llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
  llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
  llvm/test/Transforms/InstCombine/add-sitofp.ll
  llvm/test/Transforms/InstCombine/cast-int-fcmp-eq-0.ll
  llvm/test/Transforms/InstCombine/fadd-fsub-factor.ll
  llvm/test/Transforms/InstCombine/fast-math.ll
  llvm/test/Transforms/InstCombine/fcmp.ll
  llvm/test/Transforms/InstCombine/known-never-nan.ll
  llvm/test/Transforms/InstCombine/minmax-fp.ll
  llvm/test/Transforms/InstCombine/pow_fp_int.ll

Index: llvm/test/Transforms/InstCombine/pow_fp_int.ll
===
--- llvm/test/Transforms/InstCombine/pow_fp_int.ll
+++ llvm/test/Transforms/InstCombine/pow_fp_int.ll
@@ -91,7 +91,7 @@
 define double @pow_uitofp_const_base_power_of_2_fast(i31 %x) {
 ; CHECK-LABEL: @pow_uitofp_const_base_power_of_2_fast(
 ; CHECK-NEXT:[[SUBFP:%.*]] = uitofp i31 [[X:%.*]] to float
-; CHECK-NEXT:[[MUL:%.*]] = fmul afn float [[SUBFP]], 4.00e+00
+; CHECK-NEXT:[[MUL:%.*]] = fmul nnan afn float [[SUBFP]], 4.00e+00
 ; CHECK-NEXT:[[EXP2:%.*]] = call afn float @llvm.exp2.f32(float [[MUL]])
 ; CHECK-NEXT:[[RES:%.*]] = fpext float [[EXP2]] to double
 ; CHECK-NEXT:ret double [[RES]]
@@ -383,7 +383,7 @@
 define double @pow_uitofp_const_base_power_of_2_no_fast(i32 %x) {
 ; CHECK-LABEL: @pow_uitofp_const_base_power_of_2_no_fast(
 ; CHECK-NEXT:[[SUBFP:%.*]] = uitofp i32 [[X:%.*]] to float
-; CHECK-NEXT:[[MUL:%.*]] = fmul float [[SUBFP]], 4.00e+00
+; CHECK-NEXT:[[MUL:%.*]] = fmul nnan float [[SUBFP]], 4.00e+00
 ; CHECK-NEXT:[[EXP2:%.*]] = call float @llvm.exp2.f32(float [[MUL]])
 ; CHECK-NEXT:[[RES:%.*]] = fpext float [[EXP2]] to double
 ; CHECK-NEXT:ret double [[RES]]
Index: llvm/test/Transforms/InstCombine/minmax-fp.ll
===
--- llvm/test/Transforms/InstCombine/minmax-fp.ll
+++ llvm/test/Transforms/InstCombine/minmax-fp.ll
@@ -273,7 +273,7 @@
 ; CHECK-LABEL: define {{[^@]+}}@fsub_fmax(
 ; CHECK-NEXT:[[COND_INV:%.*]] = fcmp nnan nsz ogt <2 x float> [[X:%.*]], [[Y:%.*]]
 ; CHECK-NEXT:[[TMP1:%.*]] = select nnan nsz <2 x i1> [[COND_INV]], <2 x float> [[Y]], <2 x float> [[X]]
-; CHECK-NEXT:[[MAX:%.*]] = fsub <2 x float> , [[TMP1]]
+; CHECK-NEXT:[[MAX:%.*]] = fsub nnan <2 x float> , [[TMP1]]
 ; CHECK-NEXT:ret <2 x float> [[MAX]]
 ;
   %n1 = fsub <2 x float> , %x
Index: llvm/test/Transforms/InstCombine/known-never-nan.ll
===
--- llvm/test/Transforms/InstCombine/known-never-nan.ll
+++ llvm/test/Transforms/InstCombine/known-never-nan.ll
@@ -11,7 +11,7 @@
 ; CHECK-LABEL: @fabs_sqrt_src_maybe_nan(
 ; CHECK-NEXT:[[FABS:%.*]] = call double @llvm.fabs.f64(double [[ARG0:%.*]])
 ; CHECK-NEXT:[[OP:%.*]] = call double @llvm.sqrt.f64(double [[FABS]])
-; CHECK-NEXT:[[TMP:%.*]] = fcmp ord double [[OP]], 0.00e+00
+; CHECK-NEXT:[[TMP:%.*]] = fcmp nsz ord double [[OP]], 0.00e+00
 ; CHECK-NEXT:ret i1 [[TMP]]
 ;
   %fabs = call double @llvm.fabs.f64(double %arg0)
Index: llvm/test/Transforms/InstCombine/fcmp.ll
===
--- llvm/test/Transforms/InstCombine/fcmp.ll
+++ llvm/test/Transforms/InstCombine/fcmp.ll
@@ -535,11 +535,11 @@
 ; Do not fold 1.0 / X > 0.0 when ninf is missing
 define i1 @test24_recipX_noninf_cmp(float %X) {
 ; CHECK-LABEL: @test24_recipX_noninf_cmp(
-; CHECK-NEXT:[[DIV:%.*]] = fdiv ninf float 2.00e+00, [[X:%.*]]
+; CHECK-NEXT:[[DIV:%.*]] = fdiv float 2.00e+00, [[X:%.*]]
 ; CHECK-NEXT:[[CMP:%.*]] = fcmp ogt float [[DIV]], 0.00e+00
 ; CHECK-NEXT:ret i1 [[CMP]]
 ;
-  %div = fdiv ninf float 2.0, %X
+  %div = fdiv float 2.0, %X
   %cmp = fcmp ogt float %div, 0.0
   ret i1 %cmp
 }
Index: llvm/test/Transforms/InstCombine/fast-math.ll
===
--- llvm/test/Transforms/InstCombine/fast-math.ll
+++ llvm/test/Transforms/InstCombine/fast-math.ll
@@ -18,7 +18,7 @@
 define float @notfold(float %a) {
 ; CHECK-LABEL: @notfold(
 ; CHECK-NEXT:[[MUL:%.*]] = fmul fast float [[A:%.*]], 0x3FF34000
-; CHECK-NEXT:[[MUL1:%.*]] = fmul float [[MUL]], 0x40026000
+; CHECK-NEXT:[[MUL1:%.*]] = fmul nnan float [[MUL]], 0x40026000
 ; CHECK-NEXT:ret float [[MUL1]]
 ;
   %mul = fmul fast float %a, 0x3FF34000
Index: llvm/test/Transforms/InstCombin

[PATCH] D68346: [clang-format] Add new option to add spaces around conditions

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Can you rebase it and add a release note in docs/ReleaseNote.rst


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68346



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

[PATCH] D68346: [clang-format] Add new option to add spaces around conditions

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:14382
+  verifyFormat("if ( !a )\n  return;", Spaces);
+  verifyFormat("if ( a )\n  return;", Spaces);
+  verifyFormat("if constexpr ( a )\n  return;", Spaces);

show "else if" example


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68346



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

[clang] e23d6f3 - NeonEmitter: remove special case on casting polymorphic builtins.

[Tim Northover via cfe-commits]

Author: Tim Northover
Date: 2019-11-20T13:20:02Z
New Revision: e23d6f3184d365a9e72a67870d98e80f998d

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

LOG: NeonEmitter: remove special case on casting polymorphic builtins.

For some reason we were not casting a fairly obscure class of builtin calls we
expected to be polymorphic to vectors of char. It worked because the only
affected intrinsics weren't actually polymorphic after all, but is
unnecessarily complicated.

Added: 


Modified: 
clang/lib/CodeGen/CGBuiltin.cpp
clang/utils/TableGen/NeonEmitter.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index d9d0538b9138..ecac9aee5c7c 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -5456,6 +5456,11 @@ Value *CodeGenFunction::EmitCommonNeonBuiltinExpr(
 llvm::Type *Tys[2] = { Ty, GetFloatNeonType(this, Type) };
 return EmitNeonCall(CGM.getIntrinsic(LLVMIntrinsic, Tys), Ops, NameHint);
   }
+  case NEON::BI__builtin_neon_vcvtx_f32_v: {
+llvm::Type *Tys[2] = { VTy->getTruncatedElementVectorType(VTy), Ty};
+return EmitNeonCall(CGM.getIntrinsic(LLVMIntrinsic, Tys), Ops, NameHint);
+
+  }
   case NEON::BI__builtin_neon_vext_v:
   case NEON::BI__builtin_neon_vextq_v: {
 int CV = cast(Ops[2])->getSExtValue();

diff  --git a/clang/utils/TableGen/NeonEmitter.cpp 
b/clang/utils/TableGen/NeonEmitter.cpp
index bb893bc49f63..cdf761b00c61 100644
--- a/clang/utils/TableGen/NeonEmitter.cpp
+++ b/clang/utils/TableGen/NeonEmitter.cpp
@@ -1078,9 +1078,7 @@ std::string Intrinsic::getBuiltinTypeStr() {
 if (!RetT.isScalar() && RetT.isInteger() && !RetT.isSigned())
   RetT.makeSigned();
 
-bool ForcedVectorFloatingType = isFloatingPointProtoModifier(Proto[0]);
-if (LocalCK == ClassB && !RetT.isVoid() && !RetT.isScalar() &&
-!ForcedVectorFloatingType)
+if (LocalCK == ClassB && !RetT.isVoid() && !RetT.isScalar())
   // Cast to vector of 8-bit elements.
   RetT.makeInteger(8, true);
 
@@ -1092,8 +1090,7 @@ std::string Intrinsic::getBuiltinTypeStr() {
 if (T.isPoly())
   T.makeInteger(T.getElementSizeInBits(), false);
 
-bool ForcedFloatingType = isFloatingPointProtoModifier(Proto[I + 1]);
-if (LocalCK == ClassB && !T.isScalar() && !ForcedFloatingType)
+if (LocalCK == ClassB && !T.isScalar())
   T.makeInteger(8, true);
 // Halves always get converted to 8-bit elements.
 if (T.isHalf() && T.isVector() && !T.isScalarForMangling())



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

[PATCH] D50147: clang-format: support external styles

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I tend to agree, I'm not keen on the silent searching for the files.. this 
happens too much as it is, with Microsoft releasing VisualStudio with a 
clang-format.exe (v5.0), I suddenly find that binary is in my path, then all of 
a sudden all the  5.0 bugs we fixed since 5.0 reappear.

Being specific seems much better than searching my %PROFILE%


Repository:
  rC Clang

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

https://reviews.llvm.org/D50147



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

[PATCH] D70441: [clangd] Speed up when building rename edit.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:453
+  auto Offset = [&](const Position &P) -> llvm::Expected {
+Position Shifted = {
+P.line - LastPos.line,

add `assert(LastPos <= P)` to protect against malformed inputs (e.g. if one of 
occurence ranges would have an endpoint that is before the starting point).



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:458
+positionToOffset(InitialCode.substr(LastOffset), Shifted);
+if (!ShiftedOffset) {
+  return llvm::make_error(

NIT: braces are redundant and can be dropped.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:459
+if (!ShiftedOffset) {
+  return llvm::make_error(
+  llvm::formatv("fail to convert the position {0} to offset ({1})", P,

Could we simply propagate error?
I believe we mostly choose simpler code over more detailed error messages in 
clangd.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70441



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

[PATCH] D70489: [clangd] Add xref for macro to static index.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:374
+Roles & static_cast(index::SymbolRole::Definition) ||
+Roles & static_cast(index::SymbolRole::Reference)))
 return true;

I think we should avoid running the code below if this is a mere reference.



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:532
+  const IdentifierInfo *II = MacroRef.first;
+  if (const auto *MI = PP->getMacroDefinition(II).getMacroInfo())
+if (auto ID = getSymbolID(II->getName(), MI, PP->getSourceManager())) {

I'm curious of the behavior `getMacroDefinition` -- from its implementation, if 
`II` doesn't have macro definition, it just returns empty.

could you check whether it works at the below case (or even with a same macro 
name defined/undef multiple times)?

```
#define abc 1
#undef abc

// not at the EOF, I guess, II for `abc` doesn't have macro definition at this 
point because it has been undef?
```



Comment at: clang-tools-extra/clangd/index/SymbolCollector.h:76
 /// macro even if this is true.
 bool CollectMacro = false;
 /// Collect symbols local to main-files, such as static functions

This option is for macro symbols, I'd not use it for collecting macro 
references. I think the whether to collect macro references is judged by 
`RefFilter` and `RefsInHeaders`. 



Comment at: clang-tools-extra/clangd/index/SymbolCollector.h:154
   Options Opts;
   using DeclRef = std::pair;
   // Symbols referenced from the current TU, flushed on finish().

since we now use it for macros, the name is not valid any more, maybe rename it 
to `SymbolRef`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70489



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

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

also could you rename the revision so that it reflects the fact that, this is a 
change to clang-format and has nothing to do with clang-tidy ?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68682



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

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

The changes are not directly related to clang-tidy, so could you get rid of 
that clang-tidy some-checker-specific tests and put some unittests into 
clang/unittests/Format/CleanupTest.cpp ?




Comment at: clang/include/clang/Basic/CharInfo.h:196
 
+/// Requires that BufferPtr point to a newline character ("/n" or "/r").
+/// Returns a pointer past the end of any platform newline, i.e. past

use '\' instead of '/' ?



Comment at: clang/include/clang/Basic/CharInfo.h:200
+LLVM_READONLY inline const char *skipNewlineChars(const char *BufferPtr,
+ const char *BufferEnd) {
+  if (BufferPtr == BufferEnd)

formatting seems to be off, have you run clang-format on your changes?



Comment at: clang/include/clang/Basic/CharInfo.h:216
+/// Returns true if and only if S consists entirely of whitespace.
+LLVM_READONLY inline bool isWhitespaceStringRef(llvm::StringRef S) {
+  for (StringRef::const_iterator I = S.begin(), E = S.end(); I != E; ++I) {

could you make rename this to `isWhitespace` and move it closer to 
`isWhitespace(unsigned char c)`



Comment at: clang/lib/AST/CommentParser.cpp:19
 
-static inline bool isWhitespace(llvm::StringRef S) {
+// Consider moving this useful function to a more general utility location.
+bool isWhitespace(llvm::StringRef S) {

poelmanc wrote:
> gribozavr2 wrote:
> > clang/include/clang/Basic/CharInfo.h ?
> Done. I renamed it to `isWhitespaceStringRef` to avoid making it an overload 
> of the existing `isWhitespace(unsigned char)`, which causes ambiguous 
> overload errors at QueryParser.cpp:42 & CommentSema.cpp:237.
> 
> We could alternatively keep this as an `isWhitespace` overload and instead 
> change those two lines to use a `static_cast char)>(&clang::isWhitespace)` or precede them with a line like:
> ```
> bool (*isWhitespaceOverload)(unsigned char) = &clang::isWhitespace;
> ```
ah that's unfortunate, I believe it makes sense to have this as an overload 
though.

Could you instead make the predicate explicit by putting
```[](char c) { return clang::isWhitespace(c); }```
instead of just `clang::isWhitespace` in those call sites?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68682



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

[clang] db73bcd - ARM-NEON: separate soon-to-be conflicting f16 patterns. NFC.

[Tim Northover via cfe-commits]

Author: Tim Northover
Date: 2019-11-20T13:20:02Z
New Revision: db73bcd98ef4ffbe91405a5adfcfdcd83bc007f4

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

LOG: ARM-NEON: separate soon-to-be conflicting f16 patterns. NFC.

This separates some intrinsic definitions into multiple instantiations because
they use a modifier that forces the float size to a given value. That modifier
won't work in the new NeonEmitter modifier scheme and committing this
separately allows the Python script to be run on the .td files to perform the
conversion automatically.

Added: 


Modified: 
clang/include/clang/Basic/arm_fp16.td

Removed: 




diff  --git a/clang/include/clang/Basic/arm_fp16.td 
b/clang/include/clang/Basic/arm_fp16.td
index ca33a8d2ec0b..bb9873efac85 100644
--- a/clang/include/clang/Basic/arm_fp16.td
+++ b/clang/include/clang/Basic/arm_fp16.td
@@ -43,7 +43,9 @@ let ArchGuard = 
"defined(__ARM_FEATURE_FP16_SCALAR_ARITHMETIC) && defined(__aarc
   def FRINTX_S64H : SInst<"vrndx", "ss", "Sh">;
 
   // Conversion
-  def SCALAR_SCVTFSH  : SInst<"vcvth_f16", "Ys", "silUsUiUl">;
+  def SCALAR_SCVTFSH  : SInst<"vcvth_f16", "Ys", "sUs">;
+  def SCALAR_SCVTFSH1 : SInst<"vcvth_f16", "Ys", "iUi">;
+  def SCALAR_SCVTFSH2 : SInst<"vcvth_f16", "Ys", "lUl">;
   def SCALAR_FCVTZSH  : SInst<"vcvt_s16", "$s", "Sh">;
   def SCALAR_FCVTZSH1 : SInst<"vcvt_s32", "Is", "Sh">;
   def SCALAR_FCVTZSH2 : SInst<"vcvt_s64", "Ls", "Sh">;
@@ -75,7 +77,9 @@ let ArchGuard = 
"defined(__ARM_FEATURE_FP16_SCALAR_ARITHMETIC) && defined(__aarc
   def SCALAR_FCVTPUH1 : SInst<"vcvtp_u32", "Us", "Sh">;
   def SCALAR_FCVTPUH2 : SInst<"vcvtp_u64", "Os", "Sh">;
   let isVCVT_N = 1 in {
-def SCALAR_SCVTFSHO : SInst<"vcvth_n_f16", "Ysi", "silUsUiUl">;
+def SCALAR_SCVTFSHO : SInst<"vcvth_n_f16", "Ysi", "sUs">;
+def SCALAR_SCVTFSH1O: SInst<"vcvth_n_f16", "Ysi", "iUi">;
+def SCALAR_SCVTFSH2O: SInst<"vcvth_n_f16", "Ysi", "lUl">;
 def SCALAR_FCVTZSHO : SInst<"vcvt_n_s16", "$si", "Sh">;
 def SCALAR_FCVTZSH1O: SInst<"vcvt_n_s32", "Isi", "Sh">;
 def SCALAR_FCVTZSH2O: SInst<"vcvt_n_s64", "Lsi", "Sh">;



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

[PATCH] D70446: [clangd] Treat UserDefinedLiteral as a leaf in SelectionTree, sidestepping tokenization issues

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe18ab2a0b801: [clangd] Treat UserDefinedLiteral as a leaf in 
SelectionTree, sidestepping… (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70446

Files:
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/unittests/SelectionTests.cpp


Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -304,6 +304,16 @@
 }
   )cpp",
   "CallExpr"},
+
+  // User-defined literals are tricky: is 12_i one token or two?
+  // For now we treat it as one, and the UserDefinedLiteral as a leaf.
+  {
+  R"cpp(
+struct Foo{};
+Foo operator""_ud(unsigned long long);
+Foo x = [[^12_ud]];
+  )cpp",
+  "UserDefinedLiteral"},
   };
   for (const Case &C : Cases) {
 Annotations Test(C.Code);
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -12,6 +12,7 @@
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/TypeLoc.h"
@@ -245,6 +246,10 @@
 if (canSafelySkipNode(N))
   return false;
 push(std::move(N));
+if (shouldSkipChildren(X)) {
+  pop();
+  return false;
+}
 return true;
   }
   bool dataTraverseStmtPost(Stmt *X) {
@@ -355,6 +360,15 @@
 return true;
   }
 
+  // There are certain nodes we want to treat as leaves in the SelectionTree,
+  // although they do have children.
+  bool shouldSkipChildren(const Stmt *X) const {
+// UserDefinedLiteral (e.g. 12_i) has two children (12 and _i).
+// Unfortunately TokenBuffer sees 12_i as one token and can't split it.
+// So we treat UserDefinedLiteral as a leaf node, owning the token.
+return llvm::isa(X);
+  }
+
   // Pushes a node onto the ancestor stack. Pairs with pop().
   // Performs early hit detection for some nodes (on the earlySourceRange).
   void push(DynTypedNode Node) {


Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -304,6 +304,16 @@
 }
   )cpp",
   "CallExpr"},
+
+  // User-defined literals are tricky: is 12_i one token or two?
+  // For now we treat it as one, and the UserDefinedLiteral as a leaf.
+  {
+  R"cpp(
+struct Foo{};
+Foo operator""_ud(unsigned long long);
+Foo x = [[^12_ud]];
+  )cpp",
+  "UserDefinedLiteral"},
   };
   for (const Case &C : Cases) {
 Annotations Test(C.Code);
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -12,6 +12,7 @@
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/TypeLoc.h"
@@ -245,6 +246,10 @@
 if (canSafelySkipNode(N))
   return false;
 push(std::move(N));
+if (shouldSkipChildren(X)) {
+  pop();
+  return false;
+}
 return true;
   }
   bool dataTraverseStmtPost(Stmt *X) {
@@ -355,6 +360,15 @@
 return true;
   }
 
+  // There are certain nodes we want to treat as leaves in the SelectionTree,
+  // although they do have children.
+  bool shouldSkipChildren(const Stmt *X) const {
+// UserDefinedLiteral (e.g. 12_i) has two children (12 and _i).
+// Unfortunately TokenBuffer sees 12_i as one token and can't split it.
+// So we treat UserDefinedLiteral as a leaf node, owning the token.
+return llvm::isa(X);
+  }
+
   // Pushes a node onto the ancestor stack. Pairs with pop().
   // Performs early hit detection for some nodes (on the earlySourceRange).
   void push(DynTypedNode Node) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70144: clang-tidy: modernize-use-equals-default avoid adding redundant semicolons

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D70144



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

[PATCH] D68346: [clang-format] Add new option to add spaces around conditions

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:14386
+  verifyFormat("while ( a )\n  return;", Spaces);
+  verifyFormat("while ( (a && b) )\n  return;", Spaces);
+  verifyFormat("do {\n} while ( 1 != 0 );", Spaces);

is this what you expect? or should it be `while ( ( a && b ) )`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68346



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

My apologies if the revert was incorrect. I asked for advice after seeing 
breakages and heard that llvm-c should be kept ABI compatible.
But it looks like that's too strong: 
https://llvm.org/docs/DeveloperPolicy.html#c-api-changes says we make a best 
effort to maintain compatibility.

So if we don't think it's worth staying compatible here feel free to roll 
forward, but please do update the go bindings since they're in-tree.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111



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

[PATCH] D69618: NeonEmitter: clean up prototype modifiers

[Tim Northover via Phabricator via cfe-commits]

t.p.northover closed this revision.
t.p.northover marked 2 inline comments as done.
t.p.northover added a comment.

Thanks. Pushed it with those suggestions:

To github.com:llvm/llvm-project.git

  c34478f5f6c7..3f91705ca54b  master -> master


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

https://reviews.llvm.org/D69618



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

[PATCH] D70441: [clangd] Speed up when building rename edit.

[Haojian Wu via Phabricator via cfe-commits]

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

address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70441

Files:
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/refactor/Rename.h
  clang-tools-extra/clangd/unittests/RenameTests.cpp

Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -638,6 +638,33 @@
   UnorderedElementsAre(Pair(Eq(Path), Eq(expectedResult(Code, NewName);
 }
 
+TEST(CrossFileRenameTests, BuildRenameEdits) {
+  Annotations Code("[[😂]]");
+  auto LSPRange = Code.range();
+  auto Edit = buildRenameEdit(Code.code(), {LSPRange}, "abc");
+  ASSERT_TRUE(bool(Edit)) << Edit.takeError();
+  ASSERT_EQ(1UL, Edit->Replacements.size());
+  EXPECT_EQ(4UL, Edit->Replacements.begin()->getLength());
+
+  // Test invalid range.
+  LSPRange.end = {-1, 0}; // out of range
+  Edit = buildRenameEdit(Code.code(), {LSPRange}, "abc");
+  EXPECT_FALSE(Edit);
+  EXPECT_THAT(llvm::toString(Edit.takeError()),
+  testing::HasSubstr("fail to convert"));
+
+  // Normal ascii characters.
+  Annotations T(R"cpp(
+[[range]]
+  [[range]]
+  [[range]]
+  )cpp");
+  Edit = buildRenameEdit(T.code(), T.ranges(), "abc");
+  ASSERT_TRUE(bool(Edit)) << Edit.takeError();
+  EXPECT_EQ(applyEdits(FileEdits{{T.code(), std::move(*Edit)}}).front().second,
+expectedResult(Code, expectedResult(T, "abc")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/refactor/Rename.h
===
--- clang-tools-extra/clangd/refactor/Rename.h
+++ clang-tools-extra/clangd/refactor/Rename.h
@@ -47,6 +47,13 @@
 /// in another file (per the index).
 llvm::Expected rename(const RenameInputs &RInputs);
 
+/// Generates rename edits that replaces all given occurrences with the
+/// NewName.
+/// Exposed for testing only.
+llvm::Expected buildRenameEdit(llvm::StringRef InitialCode,
+ std::vector Occurrences,
+ llvm::StringRef NewName);
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -20,6 +20,7 @@
 #include "clang/Tooling/Refactoring/Rename/USRFindingAction.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/FormatVariadic.h"
 
 namespace clang {
 namespace clangd {
@@ -297,34 +298,6 @@
   return AffectedFiles;
 }
 
-llvm::Expected> toRangeOffset(const clangd::Range &R,
-llvm::StringRef Code) {
-  auto StartOffset = positionToOffset(Code, R.start);
-  if (!StartOffset)
-return StartOffset.takeError();
-  auto EndOffset = positionToOffset(Code, R.end);
-  if (!EndOffset)
-return EndOffset.takeError();
-  return std::make_pair(*StartOffset, *EndOffset);
-};
-
-llvm::Expected buildRenameEdit(llvm::StringRef InitialCode,
- const std::vector &Occurrences,
- llvm::StringRef NewName) {
-  tooling::Replacements RenameEdit;
-  for (const Range &Occurrence : Occurrences) {
-// FIXME: !positionToOffset is O(N), optimize it.
-auto RangeOffset = toRangeOffset(Occurrence, InitialCode);
-if (!RangeOffset)
-  return RangeOffset.takeError();
-auto ByteLength = RangeOffset->second - RangeOffset->first;
-if (auto Err = RenameEdit.add(tooling::Replacement(
-InitialCode, RangeOffset->first, ByteLength, NewName)))
-  return std::move(Err);
-  }
-  return Edit(InitialCode, std::move(RenameEdit));
-}
-
 // Index-based rename, it renames all occurrences outside of the main file.
 //
 // The cross-file rename is purely based on the index, as we don't want to
@@ -358,7 +331,7 @@
 llvm::inconvertibleErrorCode());
 
   FileEdits Results;
-  for (const auto &FileAndOccurrences : AffectedFiles) {
+  for (auto &FileAndOccurrences : AffectedFiles) {
 llvm::StringRef FilePath = FileAndOccurrences.first();
 
 auto AffectedFileCode = GetFileContent(FilePath);
@@ -366,11 +339,14 @@
   elog("Fail to read file content: {0}", AffectedFileCode.takeError());
   continue;
 }
-
-auto RenameEdit = buildRenameEdit(*AffectedFileCode,
-  FileAndOccurrences.getValue(), NewName);
-if (!RenameEdit)
-  return RenameEdit.takeError();
+auto RenameEdit = buildRenameEdit(
+*AffectedFileCode, std::move(Fi

[PATCH] D70368: [clang-tidy] Rewrite modernize-avoid-bind check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp:153
+
+  return HNM.matchesNode(*dyn_cast(D));
+}

zturner wrote:
> aaron.ballman wrote:
> > This should probably use `cast<>` if it's going to assume the returned 
> > value is never null.
> Good point.  Since we're talking about this code anyway, it felt super hacky 
> to instantiate an AST matcher just to check for the qualified name of a Decl. 
>  Is there a better way to do this?
Since you only care about named call declarations, I think you could probably 
get away with:
```
if (const auto *ND = dyn_cast(CE->getCalleeDecl())) {
  const std::string &Str = ND->getQualifiedNameAsString();
  if (Str == "::boost::ref" || Str == "::std::ref") {
...
  }
}
```



Comment at: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp:167
+}
+  } else if (const auto *ThisExpr = dyn_cast(Statement))
+return true;

`isa(Statement)`



Comment at: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp:428-429
+if (const auto *DRE = dyn_cast(CallExpression)) {
+  if (const auto *FD = dyn_cast(DRE->getDecl()))
+return FD;
+}

I think this can be replaced with: `return 
dyn_cast(DRE->getDecl());`


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

https://reviews.llvm.org/D70368



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

[PATCH] D70494: [clangd] Fix diagnostic location for macro expansions

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Diagnostic locations were broken when it was result of a macro
expansion. This patch fixes it by using expansion location instead of location
inside macro body.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70494

Files:
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -951,7 +951,11 @@
   TestTU TU = TestTU::withCode(Main.code());
   TU.AdditionalFiles = {{"a.h", Header.code()}};
   TU.ExtraArgs = {"-DFOO=NOOO"};
-  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  UnorderedElementsAre(AllOf(
+  Diag(Main.range(),
+   "in included file: use of undeclared identifier 
'NOOO'"),
+  WithNote(Diag(Header.range(), "error occurred here");
 }
 
 TEST(IgnoreDiags, FromNonWrittenInclude) {
@@ -963,6 +967,23 @@
   EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
 }
 
+TEST(IgnoreDiags, ErrorFromMacroExpansion) {
+  Annotations Main(R"cpp(
+  void bar() {
+int fo;
+#include [["a.h"]]
+  })cpp");
+  Annotations Header(R"cpp(
+  #define X foo
+  X;)cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.AdditionalFiles = {{"a.h", Header.code()}};
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  UnorderedElementsAre(
+  Diag(Main.range(), "in included file: use of undeclared "
+ "identifier 'foo'; did you mean 'fo'?")));
+}
+
 } // namespace
 
 } // namespace clangd
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -117,8 +117,8 @@
   if (D.Severity < DiagnosticsEngine::Level::Error)
 return false;
 
-  const SourceLocation &DiagLoc = Info.getLocation();
   const SourceManager &SM = Info.getSourceManager();
+  const SourceLocation &DiagLoc = SM.getFileLoc(Info.getLocation());
   SourceLocation IncludeInMainFile;
   auto GetIncludeLoc = [&SM](SourceLocation SLoc) {
 return SM.getIncludeLoc(SM.getFileID(SLoc));


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -951,7 +951,11 @@
   TestTU TU = TestTU::withCode(Main.code());
   TU.AdditionalFiles = {{"a.h", Header.code()}};
   TU.ExtraArgs = {"-DFOO=NOOO"};
-  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  UnorderedElementsAre(AllOf(
+  Diag(Main.range(),
+   "in included file: use of undeclared identifier 'NOOO'"),
+  WithNote(Diag(Header.range(), "error occurred here");
 }
 
 TEST(IgnoreDiags, FromNonWrittenInclude) {
@@ -963,6 +967,23 @@
   EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
 }
 
+TEST(IgnoreDiags, ErrorFromMacroExpansion) {
+  Annotations Main(R"cpp(
+  void bar() {
+int fo;
+#include [["a.h"]]
+  })cpp");
+  Annotations Header(R"cpp(
+  #define X foo
+  X;)cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.AdditionalFiles = {{"a.h", Header.code()}};
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  UnorderedElementsAre(
+  Diag(Main.range(), "in included file: use of undeclared "
+ "identifier 'foo'; did you mean 'fo'?")));
+}
+
 } // namespace
 
 } // namespace clangd
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -117,8 +117,8 @@
   if (D.Severity < DiagnosticsEngine::Level::Error)
 return false;
 
-  const SourceLocation &DiagLoc = Info.getLocation();
   const SourceManager &SM = Info.getSourceManager();
+  const SourceLocation &DiagLoc = SM.getFileLoc(Info.getLocation());
   SourceLocation IncludeInMainFile;
   auto GetIncludeLoc = [&SM](SourceLocation SLoc) {
 return SM.getIncludeLoc(SM.getFileID(SLoc));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68407: [RISCV] Use compiler-rt if no GCC installation detected

[Sam Elliott via Phabricator via cfe-commits]

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

Nice, LGTM! Thanks for fixing this in the presence of `CLANG_DEFAULT_RTLIB`.


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

https://reviews.llvm.org/D68407



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

[PATCH] D70411: [analyzer][WIP] StrChecker: 31.c

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D70411#1750690 , @Charusso wrote:

> Somehow the `MallocBugVisitor` is broken in the wild - it does not catch the 
> allocation, and we need to support a lot more to call it non-alpha (see the 
> `FIXME`s). I do not want to spend time on fixing the visitor, but rather I 
> would make this checker alpha, and move on. After a while when we have enough 
> alarms we could see what could go wrong. The reports are not bad, I tried to 
> make it super false-positive free.
>
> @aaron.ballman the name became `security.cert.str.31.c` and we lack the 
> support to invoke every C checker with `str.*.c` (as I know), but for now it 
> should be fine.


The name seems reasonable enough to me -- it at least has all the relevant 
information in it, which is a great start.


Repository:
  rC Clang

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

https://reviews.llvm.org/D70411



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

[PATCH] D70441: [clangd] Speed up when building rename edit.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 230264.
hokein added a comment.

update the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70441

Files:
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/refactor/Rename.h
  clang-tools-extra/clangd/unittests/RenameTests.cpp

Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -638,6 +638,33 @@
   UnorderedElementsAre(Pair(Eq(Path), Eq(expectedResult(Code, NewName);
 }
 
+TEST(CrossFileRenameTests, BuildRenameEdits) {
+  Annotations Code("[[😂]]");
+  auto LSPRange = Code.range();
+  auto Edit = buildRenameEdit(Code.code(), {LSPRange}, "abc");
+  ASSERT_TRUE(bool(Edit)) << Edit.takeError();
+  ASSERT_EQ(1UL, Edit->Replacements.size());
+  EXPECT_EQ(4UL, Edit->Replacements.begin()->getLength());
+
+  // Test invalid range.
+  LSPRange.end = {10, 0}; // out of range
+  Edit = buildRenameEdit(Code.code(), {LSPRange}, "abc");
+  EXPECT_FALSE(Edit);
+  EXPECT_THAT(llvm::toString(Edit.takeError()),
+  testing::HasSubstr("fail to convert"));
+
+  // Normal ascii characters.
+  Annotations T(R"cpp(
+[[range]]
+  [[range]]
+  [[range]]
+  )cpp");
+  Edit = buildRenameEdit(T.code(), T.ranges(), "abc");
+  ASSERT_TRUE(bool(Edit)) << Edit.takeError();
+  EXPECT_EQ(applyEdits(FileEdits{{T.code(), std::move(*Edit)}}).front().second,
+expectedResult(Code, expectedResult(T, "abc")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/refactor/Rename.h
===
--- clang-tools-extra/clangd/refactor/Rename.h
+++ clang-tools-extra/clangd/refactor/Rename.h
@@ -47,6 +47,13 @@
 /// in another file (per the index).
 llvm::Expected rename(const RenameInputs &RInputs);
 
+/// Generates rename edits that replaces all given occurrences with the
+/// NewName.
+/// Exposed for testing only.
+llvm::Expected buildRenameEdit(llvm::StringRef InitialCode,
+ std::vector Occurrences,
+ llvm::StringRef NewName);
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -20,6 +20,7 @@
 #include "clang/Tooling/Refactoring/Rename/USRFindingAction.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/FormatVariadic.h"
 
 namespace clang {
 namespace clangd {
@@ -297,34 +298,6 @@
   return AffectedFiles;
 }
 
-llvm::Expected> toRangeOffset(const clangd::Range &R,
-llvm::StringRef Code) {
-  auto StartOffset = positionToOffset(Code, R.start);
-  if (!StartOffset)
-return StartOffset.takeError();
-  auto EndOffset = positionToOffset(Code, R.end);
-  if (!EndOffset)
-return EndOffset.takeError();
-  return std::make_pair(*StartOffset, *EndOffset);
-};
-
-llvm::Expected buildRenameEdit(llvm::StringRef InitialCode,
- const std::vector &Occurrences,
- llvm::StringRef NewName) {
-  tooling::Replacements RenameEdit;
-  for (const Range &Occurrence : Occurrences) {
-// FIXME: !positionToOffset is O(N), optimize it.
-auto RangeOffset = toRangeOffset(Occurrence, InitialCode);
-if (!RangeOffset)
-  return RangeOffset.takeError();
-auto ByteLength = RangeOffset->second - RangeOffset->first;
-if (auto Err = RenameEdit.add(tooling::Replacement(
-InitialCode, RangeOffset->first, ByteLength, NewName)))
-  return std::move(Err);
-  }
-  return Edit(InitialCode, std::move(RenameEdit));
-}
-
 // Index-based rename, it renames all occurrences outside of the main file.
 //
 // The cross-file rename is purely based on the index, as we don't want to
@@ -358,7 +331,7 @@
 llvm::inconvertibleErrorCode());
 
   FileEdits Results;
-  for (const auto &FileAndOccurrences : AffectedFiles) {
+  for (auto &FileAndOccurrences : AffectedFiles) {
 llvm::StringRef FilePath = FileAndOccurrences.first();
 
 auto AffectedFileCode = GetFileContent(FilePath);
@@ -366,11 +339,14 @@
   elog("Fail to read file content: {0}", AffectedFileCode.takeError());
   continue;
 }
-
-auto RenameEdit = buildRenameEdit(*AffectedFileCode,
-  FileAndOccurrences.getValue(), NewName);
-if (!RenameEdit)
-  return RenameEdit.takeError();
+auto RenameEdit = buildRenameEdit(
+*AffectedFileCode, std::move(FileAndOccurrences.second), NewName);
+i

[clang-tools-extra] b5135a8 - [clangd] Fix a crash in expected types

[Ilya Biryukov via cfe-commits]

Author: Ilya Biryukov
Date: 2019-11-20T16:40:28+01:00
New Revision: b5135a86e04761577494c70e7c0057136cc90b5b

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

LOG: [clangd] Fix a crash in expected types

Reviewers: kadircet

Reviewed By: kadircet

Subscribers: merge_guards_bot, MaskRay, jkorous, arphaman, usaxena95, 
cfe-commits

Tags: #clang

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

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/ExpectedTypes.cpp 
b/clang-tools-extra/clangd/ExpectedTypes.cpp
index 3b0779ea66bc..a82a64cf14e2 100644
--- a/clang-tools-extra/clangd/ExpectedTypes.cpp
+++ b/clang-tools-extra/clangd/ExpectedTypes.cpp
@@ -44,12 +44,10 @@ static const Type *toEquivClass(ASTContext &Ctx, QualType 
T) {
 static llvm::Optional
 typeOfCompletion(const CodeCompletionResult &R) {
   const NamedDecl *D = R.Declaration;
-  if (!D)
-return llvm::None;
   // Templates do not have a type on their own, look at the templated decl.
-  if (auto *Template = dyn_cast(D))
+  if (auto *Template = dyn_cast_or_null(D))
 D = Template->getTemplatedDecl();
-  auto *VD = dyn_cast(D);
+  auto *VD = dyn_cast_or_null(D);
   if (!VD)
 return llvm::None; // We handle only variables and functions below.
   auto T = VD->getType();

diff  --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp 
b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index 5b50b9fe9f8b..e69b2a6205f6 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1030,6 +1030,16 @@ TEST(CompletionTest, DefaultArgs) {
 SnippetSuffix("(${1:int A})";
 }
 
+TEST(CompletionTest, NoCrashWithTemplateParamsAndPreferredTypes) {
+  auto Completions = completions(R"cpp(
+template  class TT> int foo() {
+  int a = ^
+}
+)cpp")
+ .Completions;
+  EXPECT_THAT(Completions, Contains(Named("TT")));
+}
+
 SignatureHelp signatures(llvm::StringRef Text, Position Point,
  std::vector IndexSymbols = {}) {
   std::unique_ptr Index;



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

[PATCH] D70481: [clangd] Fix a crash in expected types

[Ilya Biryukov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb5135a86e047: [clangd] Fix a crash in expected types 
(authored by ilya-biryukov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70481

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


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1030,6 +1030,16 @@
 SnippetSuffix("(${1:int A})";
 }
 
+TEST(CompletionTest, NoCrashWithTemplateParamsAndPreferredTypes) {
+  auto Completions = completions(R"cpp(
+template  class TT> int foo() {
+  int a = ^
+}
+)cpp")
+ .Completions;
+  EXPECT_THAT(Completions, Contains(Named("TT")));
+}
+
 SignatureHelp signatures(llvm::StringRef Text, Position Point,
  std::vector IndexSymbols = {}) {
   std::unique_ptr Index;
Index: clang-tools-extra/clangd/ExpectedTypes.cpp
===
--- clang-tools-extra/clangd/ExpectedTypes.cpp
+++ clang-tools-extra/clangd/ExpectedTypes.cpp
@@ -44,12 +44,10 @@
 static llvm::Optional
 typeOfCompletion(const CodeCompletionResult &R) {
   const NamedDecl *D = R.Declaration;
-  if (!D)
-return llvm::None;
   // Templates do not have a type on their own, look at the templated decl.
-  if (auto *Template = dyn_cast(D))
+  if (auto *Template = dyn_cast_or_null(D))
 D = Template->getTemplatedDecl();
-  auto *VD = dyn_cast(D);
+  auto *VD = dyn_cast_or_null(D);
   if (!VD)
 return llvm::None; // We handle only variables and functions below.
   auto T = VD->getType();


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1030,6 +1030,16 @@
 SnippetSuffix("(${1:int A})";
 }
 
+TEST(CompletionTest, NoCrashWithTemplateParamsAndPreferredTypes) {
+  auto Completions = completions(R"cpp(
+template  class TT> int foo() {
+  int a = ^
+}
+)cpp")
+ .Completions;
+  EXPECT_THAT(Completions, Contains(Named("TT")));
+}
+
 SignatureHelp signatures(llvm::StringRef Text, Position Point,
  std::vector IndexSymbols = {}) {
   std::unique_ptr Index;
Index: clang-tools-extra/clangd/ExpectedTypes.cpp
===
--- clang-tools-extra/clangd/ExpectedTypes.cpp
+++ clang-tools-extra/clangd/ExpectedTypes.cpp
@@ -44,12 +44,10 @@
 static llvm::Optional
 typeOfCompletion(const CodeCompletionResult &R) {
   const NamedDecl *D = R.Declaration;
-  if (!D)
-return llvm::None;
   // Templates do not have a type on their own, look at the templated decl.
-  if (auto *Template = dyn_cast(D))
+  if (auto *Template = dyn_cast_or_null(D))
 D = Template->getTemplatedDecl();
-  auto *VD = dyn_cast(D);
+  auto *VD = dyn_cast_or_null(D);
   if (!VD)
 return llvm::None; // We handle only variables and functions below.
   auto T = VD->getType();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70494: [clangd] Fix diagnostic location for macro expansions

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: pass - 60059 tests passed, 0 failed and 729 were skipped.
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70494



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

[PATCH] D69330: [AST] Add RecoveryExpr to retain expressions on semantic errors

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 230270.
ilya-biryukov added a comment.

- Do not correct typos when creating recovery expr
- Simplify StmtPrinter::VisitRecoveryExpr
- Add -Wno-used and remove extra expect-warning directives


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69330

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Basic/StmtNodes.td
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprClassification.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/AST/ast-dump-expr-errors.cpp
  clang/test/AST/ast-dump-recovery.cpp
  clang/test/Index/getcursor-recovery.cpp
  clang/test/Parser/cxx-concepts-ambig-constraint-expr.cpp
  clang/test/Parser/objcxx0x-lambda-expressions.mm
  clang/test/Parser/objcxx11-invalid-lambda.cpp
  clang/test/SemaCXX/builtins.cpp
  clang/test/SemaCXX/cast-conversion.cpp
  clang/test/SemaCXX/cxx1z-copy-omission.cpp
  clang/test/SemaCXX/decltype-crash.cpp
  clang/test/SemaCXX/varargs.cpp
  clang/test/SemaOpenCLCXX/address-space-references.cl
  clang/test/SemaTemplate/instantiate-init.cpp
  clang/tools/libclang/CXCursor.cpp

Index: clang/tools/libclang/CXCursor.cpp
===
--- clang/tools/libclang/CXCursor.cpp
+++ clang/tools/libclang/CXCursor.cpp
@@ -291,6 +291,7 @@
   case Stmt::ObjCDictionaryLiteralClass:
   case Stmt::ObjCBoxedExprClass:
   case Stmt::ObjCSubscriptRefExprClass:
+  case Stmt::RecoveryExprClass:
 K = CXCursor_UnexposedExpr;
 break;
 
Index: clang/test/SemaTemplate/instantiate-init.cpp
===
--- clang/test/SemaTemplate/instantiate-init.cpp
+++ clang/test/SemaTemplate/instantiate-init.cpp
@@ -100,7 +100,7 @@
 integral_c<1> ic1 = array_lengthof(Description::data);
 (void)sizeof(array_lengthof(Description::data));
 
-sizeof(array_lengthof( // expected-error{{no matching function for call to 'array_lengthof'}}
+(void)sizeof(array_lengthof( // expected-error{{no matching function for call to 'array_lengthof'}}
   Description::data // expected-note{{in instantiation of static data member 'PR7985::Description::data' requested here}}
   ));
 
Index: clang/test/SemaOpenCLCXX/address-space-references.cl
===
--- clang/test/SemaOpenCLCXX/address-space-references.cl
+++ clang/test/SemaOpenCLCXX/address-space-references.cl
@@ -11,5 +11,5 @@
 int bar(const unsigned int &i);
 
 void foo() {
-  bar(1) // expected-error{{binding reference of type 'const __global unsigned int' to value of type 'int' changes address space}}
+  bar(1); // expected-error{{binding reference of type 'const __global unsigned int' to value of type 'int' changes address space}}
 }
Index: clang/test/SemaCXX/varargs.cpp
===
--- clang/test/SemaCXX/varargs.cpp
+++ clang/test/SemaCXX/varargs.cpp
@@ -22,7 +22,8 @@
 // default ctor.
 void record_context(int a, ...) {
   struct Foo {
-// expected-error@+1 {{'va_start' cannot be used outside a function}}
+// expected-error@+2 {{'va_start' cannot be used outside a function}}
+// expected-error@+1 {{default argument references parameter 'a'}}
 void meth(int a, int b = (__builtin_va_start(ap, a), 0)) {}
   };
 }
Index: clang/test/SemaCXX/decltype-crash.cpp
===
--- clang/test/SemaCXX/decltype-crash.cpp
+++ clang/test/SemaCXX/decltype-crash.cpp
@@ -3,5 +3,8 @@
 int& a();
 
 void f() {
-  decltype(a()) c; // expected-warning {{'decltype' is a keyword in C++11}} expected-error {{use of undeclared identifier 'decltype'}}
+  decltype(a()) c; // expected-warning {{'decltype' is a keyword in C++11}} \
+   // expected-error {{use of undeclared identifier 'decltype'}} \
+   // expected-error {{expected ';' after expression}} \
+   // expected-error {{use of undeclared identifier 'c'}}
 }
Index: clang/test/SemaCXX/cxx1z-copy-omission.cpp
===
--- clang/test/SemaCXX/cxx1z-copy-omission.cpp
+++ clang/test/SemaCXX/cxx1z-copy-omission.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++1z -verify %s
+// RUN: %clang_cc1 -std=c++1z -verify -Wno-unused %s
 
 struct Nonc

Re: [PATCH] D69897: Add #pragma clang loop vectorize_assume_alignment(n)

[HAPPY Mahto via cfe-commits]

Hello Michael,
Very sorry for the late reply, we had exams and assignments this week and I
had to read about _builtin_assume_aligned as I didn't come across this.

#pragma clang loop vectorize_assume_alignment(32)
> for(int i = 0;i < n; i++){
> a[i] = b[i] + i*i;
> }
>
 for this all-access inside the loop will be aligned to 32bit,
ex  IR

> for.cond: ; preds = %for.inc,
> %entry
>   %5 = load i32, i32* %i, align 32, !llvm.access.group !2
>   %6 = load i32, i32* %n, align 32, !llvm.access.group !2
>   %cmp = icmp slt i32 %5, %6
>   br i1 %cmp, label %for.body, label %for.end
>
> for.body: ; preds = %for.cond
>   %7 = load i32, i32* %i, align 32, !llvm.access.group !2
>   %8 = load i32, i32* %i, align 32, !llvm.access.group !2
>   %idxprom = sext i32 %8 to i64
>   %arrayidx = getelementptr inbounds i32, i32* %vla1, i64 %idxprom
>   store i32 %7, i32* %arrayidx, align 32, !llvm.access.group !2
>   br label %for.inc
>
> for.inc:  ; preds = %for.body
>   %9 = load i32, i32* %i, align 32, !llvm.access.group !2
>   %inc = add nsw i32 %9, 1
>   store i32 %inc, i32* %i, align 32, !llvm.access.group !2
>   br label %for.cond, !llvm.loop !3
>
You will not need to create pointers for every array(or operand you want to
perform the operation on).

> void mult(float* x, int size, float factor){
>   float* ax = (float*)__builtin_assume_aligned(x, 64);
>   for (int i = 0; i < size; ++i)
>  ax[i] *= factor;
> }
>
the IR generated for this :

> define void @mult(i32*, i32, float) #0 {
>   %4 = alloca i32*, align 8
>   %5 = alloca i32, align 4
>   %6 = alloca float, align 4
>   %7 = alloca i32*, align 8
>   %8 = alloca i32, align 4
>   store i32* %0, i32** %4, align 8
>   store i32 %1, i32* %5, align 4
>   store float %2, float* %6, align 4
>   %9 = load i32*, i32** %4, align 8
>   %10 = bitcast i32* %9 to i8*
>   %11 = ptrtoint i8* %10 to i64
>   %12 = and i64 %11, 63
>   %13 = icmp eq i64 %12, 0
>   call void @llvm.assume(i1 %13)
>   %14 = bitcast i8* %10 to i32*
>   store i32* %14, i32** %7, align 8
>   store i32 0, i32* %8, align 4
>   br label %15
>
> ; :15: ; preds = %29, %3
>   %16 = load i32, i32* %8, align 4
>   %17 = load i32, i32* %5, align 4
>   %18 = icmp slt i32 %16, %17
>   br i1 %18, label %19, label %32
>
> ; :19: ; preds = %15
>   %20 = load float, float* %6, align 4
>   %21 = load i32*, i32** %7, align 8
>   %22 = load i32, i32* %8, align 4
>   %23 = sext i32 %22 to i64
>   %24 = getelementptr inbounds i32, i32* %21, i64 %23
>   %25 = load i32, i32* %24, align 4
>   %26 = sitofp i32 %25 to float
>   %27 = fmul float %26, %20
>   %28 = fptosi float %27 to i32
>   store i32 %28, i32* %24, align 4
>   br label %29
>
> ; :29: ; preds = %19
>   %30 = load i32, i32* %8, align 4
>   %31 = add nsw i32 %30, 1
>   store i32 %31, i32* %8, align 4
>   br label %15
>
> ; :32: ; preds = %15
>   ret void
> }
>
the alignment is assumed whereas in #pragma it is set to the number
specified.
it'll be easier, and having a pragma for doing this will help as it's
provided in OMP and intel compilers.
Thank you, If I made any mistake please tell me.

Happy Mahto
CSE Undergrad, IIT Hyderabad


On Thu, Nov 14, 2019 at 10:32 PM Michael Kruse via Phabricator <
revi...@reviews.llvm.org> wrote:

> Meinersbur added a comment.
>
> Could you elaborate why this is better than `__builtin_assume_aligned`?
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D69897/new/
>
> https://reviews.llvm.org/D69897
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69330: [AST] Add RecoveryExpr to retain expressions on semantic errors

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.

@rsmith, could you also take a look at D65591 ?
It's really important to have the `containsError()` check in this patch that 
marks decls with undeduced types as invalid. Otherwise, they would have a 
"dependent auto type" and the constant evaluation code will fail when 
attempting to get sizes of those, e.g. when they're used inside `sizeof()`.
There's actually a test in clang that crashes if this is not done 
(`clang/test/SemaCXX/lambda-expressions.cpp`)

I have also realized doing typo-correction in `CreateRecoveryExpr` is a bad 
idea, the contract for typo correction is to run it at particular points (e.g. 
on full expressions) and it's unclear why recovery expressions should be 
another case for this context. All code attempting to create recovery 
expressions should already handle typo correction properly, so there's no need 
to cross these two features.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69330



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

[PATCH] D69810: [OpenCL] Fix address space for base method call (PR43145)

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh updated this revision to Diff 230278.
svenvh added a comment.

Incorporate suggestions from @rjmccall and add a test for the pointer case.


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

https://reviews.llvm.org/D69810

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CodeGenOpenCLCXX/addrspace-derived-base.cl


Index: clang/test/CodeGenOpenCLCXX/addrspace-derived-base.cl
===
--- clang/test/CodeGenOpenCLCXX/addrspace-derived-base.cl
+++ clang/test/CodeGenOpenCLCXX/addrspace-derived-base.cl
@@ -28,6 +28,7 @@
 class B2 {
   public:
 void baseMethod() const {  }
+int &getRef() { return bb; }
 int bb;
 };
 
@@ -46,7 +47,25 @@
 
 void pr43145_2(B *argB) {
   Derived *x = (Derived*)argB;
+  // CHECK-LABEL: @_Z9pr43145_2
+  // CHECK: bitcast %struct.B addrspace(4)* %0 to %class.Derived addrspace(4)*
 }
 
-// CHECK-LABEL: @_Z9pr43145_2
-// CHECK: bitcast %struct.B addrspace(4)* %0 to %class.Derived addrspace(4)*
+// Assigning to reference returned by base class method through derived class.
+
+void pr43145_3(int n) {
+  Derived d;
+  d.getRef() = n;
+
+  // CHECK-LABEL: @_Z9pr43145_3
+  // CHECK: addrspacecast %class.Derived* %d to %class.Derived addrspace(4)*
+  // CHECK: bitcast i8 addrspace(4)* %add.ptr to %class.B2 addrspace(4)*
+  // CHECK: call {{.*}} @_ZNU3AS42B26getRefEv
+
+  private Derived *pd = &d;
+  pd->getRef() = n;
+
+  // CHECK: addrspacecast %class.Derived* %4 to %class.Derived addrspace(4)*
+  // CHECK: bitcast i8 addrspace(4)* %add.ptr1 to %class.B2 addrspace(4)*
+  // CHECK: call {{.*}} @_ZNU3AS42B26getRefEv
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -2711,6 +2711,20 @@
   FromRecordType = FromType;
   DestType = DestRecordType;
 }
+
+LangAS FromAS = FromRecordType.getAddressSpace();
+LangAS DestAS = DestRecordType.getAddressSpace();
+if (FromAS != DestAS) {
+  QualType FromRecordTypeWithoutAS =
+  Context.removeAddrSpaceQualType(FromRecordType);
+  QualType FromTypeWithDestAS =
+  Context.getAddrSpaceQualType(FromRecordTypeWithoutAS, DestAS);
+  if (PointerConversions)
+FromTypeWithDestAS = Context.getPointerType(FromTypeWithDestAS);
+  From = ImpCastExprToType(From, FromTypeWithDestAS,
+   CK_AddressSpaceConversion, From->getValueKind())
+ .get();
+}
   } else {
 // No conversion necessary.
 return From;


Index: clang/test/CodeGenOpenCLCXX/addrspace-derived-base.cl
===
--- clang/test/CodeGenOpenCLCXX/addrspace-derived-base.cl
+++ clang/test/CodeGenOpenCLCXX/addrspace-derived-base.cl
@@ -28,6 +28,7 @@
 class B2 {
   public:
 void baseMethod() const {  }
+int &getRef() { return bb; }
 int bb;
 };
 
@@ -46,7 +47,25 @@
 
 void pr43145_2(B *argB) {
   Derived *x = (Derived*)argB;
+  // CHECK-LABEL: @_Z9pr43145_2
+  // CHECK: bitcast %struct.B addrspace(4)* %0 to %class.Derived addrspace(4)*
 }
 
-// CHECK-LABEL: @_Z9pr43145_2
-// CHECK: bitcast %struct.B addrspace(4)* %0 to %class.Derived addrspace(4)*
+// Assigning to reference returned by base class method through derived class.
+
+void pr43145_3(int n) {
+  Derived d;
+  d.getRef() = n;
+
+  // CHECK-LABEL: @_Z9pr43145_3
+  // CHECK: addrspacecast %class.Derived* %d to %class.Derived addrspace(4)*
+  // CHECK: bitcast i8 addrspace(4)* %add.ptr to %class.B2 addrspace(4)*
+  // CHECK: call {{.*}} @_ZNU3AS42B26getRefEv
+
+  private Derived *pd = &d;
+  pd->getRef() = n;
+
+  // CHECK: addrspacecast %class.Derived* %4 to %class.Derived addrspace(4)*
+  // CHECK: bitcast i8 addrspace(4)* %add.ptr1 to %class.B2 addrspace(4)*
+  // CHECK: call {{.*}} @_ZNU3AS42B26getRefEv
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -2711,6 +2711,20 @@
   FromRecordType = FromType;
   DestType = DestRecordType;
 }
+
+LangAS FromAS = FromRecordType.getAddressSpace();
+LangAS DestAS = DestRecordType.getAddressSpace();
+if (FromAS != DestAS) {
+  QualType FromRecordTypeWithoutAS =
+  Context.removeAddrSpaceQualType(FromRecordType);
+  QualType FromTypeWithDestAS =
+  Context.getAddrSpaceQualType(FromRecordTypeWithoutAS, DestAS);
+  if (PointerConversions)
+FromTypeWithDestAS = Context.getPointerType(FromTypeWithDestAS);
+  From = ImpCastExprToType(From, FromTypeWithDestAS,
+   CK_AddressSpaceConversion, From->getValueKind())
+ .get();
+}
   } else {
 // No conversion necessary.
 return From;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.l

[PATCH] D60455: [SYCL] Implement SYCL device code outlining

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 230281.
bader marked 5 inline comments as done.
bader added a comment.

Applied code review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ASTContext.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenSYCL/device-functions.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
  clang/test/SemaSYCL/device-attributes.cpp

Index: clang/test/SemaSYCL/device-attributes.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl-is-device -verify %s
+
+[[clang::sycl_kernel]] int gv2 = 0; // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+__attribute__((sycl_kernel)) int gv3 = 0; // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+
+__attribute__((sycl_kernel(1))) void foo(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+[[clang::sycl_kernel(1)]] void foo2(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+
+// Only function templates
+__attribute__((sycl_kernel)) void foo(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+[[clang::sycl_kernel]] void foo1(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+
+// At least two template parameters
+template 
+__attribute__((sycl_kernel)) void foo(T P); // expected-warning {{'sycl_kernel' attribute only applies to a function template with at least two template parameters}}
+template 
+[[clang::sycl_kernel]] void foo1(T P); // expected-warning {{'sycl_kernel' attribute only applies to a function template with at least two template parameters}}
+
+// Both first two template parameters must be a template type parameter
+template 
+__attribute__((sycl_kernel)) void foo(T P); // expected-warning {{template parameter of a function template with the 'sycl_kernel' attribute must be a template type parameter}}
+template 
+[[clang::sycl_kernel]] void foo1(T P); // expected-warning {{template parameter of a function template with the 'sycl_kernel' attribute must be a template type parameter}}
+
+// Must return void
+template 
+__attribute__((sycl_kernel)) int foo(T P); // expected-warning {{function template with 'sycl_kernel' attribute must have a 'void' return type}}
+template 
+[[clang::sycl_kernel]] int foo1(T P); // expected-warning {{function template with 'sycl_kernel' attribute must have a 'void' return type}}
+
+// Must take at least one argument
+template 
+__attribute__((sycl_kernel)) void foo(); // expected-warning {{function template with 'sycl_kernel' attribute must have a single parameter}}
+template 
+[[clang::sycl_kernel]] void foo1(T t, A a); // expected-warning {{function template with 'sycl_kernel' attribute must have a single parameter}}
+
+// No diagnostics
+template 
+__attribute__((sycl_kernel)) void foo(T P);
+template 
+[[clang::sycl_kernel]] void foo1(T P);
Index: clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl-is-device -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -x c++ %s
+
+#ifndef __SYCL_DEVICE_ONLY__
+// expected-warning@+7 {{'sycl_kernel' attribute ignored}}
+// expected-warning@+8 {{'sycl_kernel' attribute ignored}}
+#else
+// expected-no-diagnostics
+#endif
+
+template 
+__attribute__((sycl_kernel)) void foo(T P);
+template 
+[[clang::sycl_kernel]] void foo1(T P);
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -131,6 +131,7 @@
 // CHECK-NEXT: ReturnTypestate (SubjectMatchRule_function, SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: ReturnsNonNull (SubjectMatchRule_objc_method, SubjectMatchRule_function)
 // CHECK-NEXT: ReturnsTwice (SubjectMatchRule_function)
+// CHECK-NEXT: SYCLKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)
 // CHECK-NEXT: Section (SubjectMatchRule_function, SubjectMatchRule_variable_is_global, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property)
 // CHECK-NEXT: SetTypestate (SubjectMatchRule_function_is_member)
Index: clang/test/CodeGenSYCL/device-functions.cpp
===

[PATCH] D60455: [SYCL] Implement SYCL device code outlining

[Alexey Bader via Phabricator via cfe-commits]

bader added a subscriber: erichkeane.
bader added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10079-10080
+def warn_sycl_kernel_invalid_template_param_type : Warning<
+  "template parameter of template functions with 'sycl_kernel' attribute must"
+  " be typename">, InGroup;
+def warn_sycl_kernel_num_of_function_params : Warning<

aaron.ballman wrote:
> This diagnostic reads a bit like you cannot do this: `template ` 
> when I think the actual restriction is that you cannot do this: `template 
> `. Is that correct? If so, I think this could be worded as `template 
> parameter of a function template with the 'sycl_kernel' attribute must be a 
> template type parameter`.
> 
> Just double-checking, but you also intend to prohibit template template 
> parameters? e.g., you can't do `template  typename C>`
> This diagnostic reads a bit like you cannot do this: template  when 
> I think the actual restriction is that you cannot do this: template . 
> Is that correct?

Yes. That is correct.

>  If so, I think this could be worded as template parameter of a function 
> template with the 'sycl_kernel' attribute must be a template type parameter.

Thanks! Applied your wording.

> Just double-checking, but you also intend to prohibit template template 
> parameters? e.g., you can't do template  typename C>

Currently we allow following use case: 
https://github.com/intel/llvm/blob/sycl/clang/test/SemaSYCL/mangle-kernel.cpp. 
I assume it qualifies as "template type" and not as "template template" 
parameter. Right?

Quoting SYCL specification $6.2 Naming of kernels 
(https://www.khronos.org/registry/SYCL/specs/sycl-1.2.1.pdf#page=250).

> SYCL kernels are extracted from C++ source files and stored in an 
> implementation- defined format. In the case of
> the shared-source compilation model, the kernels have to be uniquely 
> identified by both host and device compiler.
> This is required in order for the host runtime to be able to load the kernel 
> by using the OpenCL host runtime
> interface.
> From this requirement the following rules apply for naming the kernels:
> • The kernel name is a C++ typename.
> • The kernel needs to have a globally-visible name. In the case of a named 
> function object type, the name can
> be the typename of the function object, as long as it is globally-visible. In 
> the case where it isn’t, a globally visible name has to be provided, as 
> template parameter to the kernel invoking interface, as described in 4.8.5.
> In C++11, lambdas do not have a globally-visible name, so a globally-visible 
> typename has to be provided
> in the kernel invoking interface, as described in 4.8.5.
> • The kernel name has to be a unique identifier in the program.
> 

We also have an extension, which lifts these restrictions/requirements when 
clang is used as host and device compiler. @erichkeane implemented built-in 
function (https://github.com/intel/llvm/pull/250) providing "unique 
identifier", which we use as a kernel name for lambda objects. But this is 
going to be a separate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

Re: [PATCH] D69897: Add #pragma clang loop vectorize_assume_alignment(n)

[Michael Kruse via cfe-commits]

Am Mi., 20. Nov. 2019 um 10:21 Uhr schrieb HAPPY Mahto
:
>> #pragma clang loop vectorize_assume_alignment(32)
>> for(int i = 0;i < n; i++){
>> a[i] = b[i] + i*i;
>> }
>
>  for this all-access inside the loop will be aligned to 32bit,
> ex  IR
>>
>> for.cond: ; preds = %for.inc, %entry
>>   %5 = load i32, i32* %i, align 32, !llvm.access.group !2
>>   %6 = load i32, i32* %n, align 32, !llvm.access.group !2
>>   %cmp = icmp slt i32 %5, %6
>>   br i1 %cmp, label %for.body, label %for.end
>>
>> for.body: ; preds = %for.cond
>>   %7 = load i32, i32* %i, align 32, !llvm.access.group !2
>>   %8 = load i32, i32* %i, align 32, !llvm.access.group !2
>>   %idxprom = sext i32 %8 to i64
>>   %arrayidx = getelementptr inbounds i32, i32* %vla1, i64 %idxprom
>>   store i32 %7, i32* %arrayidx, align 32, !llvm.access.group !2
>>   br label %for.inc
>>
>> for.inc:  ; preds = %for.body
>>   %9 = load i32, i32* %i, align 32, !llvm.access.group !2
>>   %inc = add nsw i32 %9, 1
>>   store i32 %inc, i32* %i, align 32, !llvm.access.group !2
>>   br label %for.cond, !llvm.loop !3
>
> You will not need to create pointers for every array(or operand you want to 
> perform the operation on).

IMHO it is better if the programmer has to. It is not always obvious
which arrays are used in the loop. Also, the information can be used
by other optimzations that the vectorizer.


>>
>> void mult(float* x, int size, float factor){
>>   float* ax = (float*)__builtin_assume_aligned(x, 64);
>>   for (int i = 0; i < size; ++i)
>>  ax[i] *= factor;
>> }

https://godbolt.org/z/Fd6HMe

> the alignment is assumed whereas in #pragma it is set to the number specified.

Semantically, it is the same.

I wonder how you expect the assembly output to change? The
__builtin_assume_aligned, will be picked up by the backend and result
in movaps to be used instead of movups.


> it'll be easier, and having a pragma for doing this will help as it's 
> provided in OMP and intel compilers.

This is a compiler-specific extension. It does not have an influence
on what other compilers do. Even with clang, if you try to do

#pragma clang loop vectorize_assume_alignment(32)
#pragma omp simd
for (int i = 0; i < size; ++i)

clang will silently swallow the vectorize_assume_alignment.


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

[PATCH] D70499: [clang] Fix the path to CrossWinToARMLinux.cmake CMake cache

[Sergej Jaskiewicz via Phabricator via cfe-commits]

broadwaylamb created this revision.
broadwaylamb added reviewers: vvereschaka, andreil99.
broadwaylamb added a project: clang.
Herald added subscribers: cfe-commits, kristof.beyls, mgorny.

The comment was slightly misleading.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70499

Files:
  clang/cmake/caches/CrossWinToARMLinux.cmake


Index: clang/cmake/caches/CrossWinToARMLinux.cmake
===
--- clang/cmake/caches/CrossWinToARMLinux.cmake
+++ clang/cmake/caches/CrossWinToARMLinux.cmake
@@ -14,7 +14,7 @@
 #   -DDEFAULT_SYSROOT= ^
 #   -DLLVM_AR=/bin/llvm-ar[.exe] ^
 #   -DCMAKE_CXX_FLAGS="-D__OPTIMIZE__" ^
-#   -C/llvm-project/clang/caches/CrossWinToARMLinux.cmake ^
+#   
-C/llvm-project/clang/cmake/caches/CrossWinToARMLinux.cmake ^
 #   /llvm-project/llvm
 # Build:
 #  cmake --build . --target install


Index: clang/cmake/caches/CrossWinToARMLinux.cmake
===
--- clang/cmake/caches/CrossWinToARMLinux.cmake
+++ clang/cmake/caches/CrossWinToARMLinux.cmake
@@ -14,7 +14,7 @@
 #   -DDEFAULT_SYSROOT= ^
 #   -DLLVM_AR=/bin/llvm-ar[.exe] ^
 #   -DCMAKE_CXX_FLAGS="-D__OPTIMIZE__" ^
-#   -C/llvm-project/clang/caches/CrossWinToARMLinux.cmake ^
+#   -C/llvm-project/clang/cmake/caches/CrossWinToARMLinux.cmake ^
 #   /llvm-project/llvm
 # Build:
 #  cmake --build . --target install
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69855: [clang-tidy] Fix llvm-namespace-comment for macro expansions

[Marcin Twardak via Phabricator via cfe-commits]

twardakm updated this revision to Diff 230285.
twardakm added a comment.

Fix style issues


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69855

Files:
  clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp
  clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.h
  
clang-tools-extra/test/clang-tidy/checkers/google-readability-namespace-comments.cpp
  clang-tools-extra/test/clang-tidy/checkers/llvm-namespace-comment.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/llvm-namespace-comment.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/llvm-namespace-comment.cpp
@@ -0,0 +1,41 @@
+// RUN: %check_clang_tidy %s llvm-namespace-comment %t
+
+namespace n1 {
+namespace n2 {
+  void f();
+
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:1: warning: namespace 'n2' not terminated with a closing comment [llvm-namespace-comment]
+  // CHECK-MESSAGES: :[[@LINE+1]]:2: warning: namespace 'n1' not terminated with a closing comment [llvm-namespace-comment]
+}}
+// CHECK-FIXES: } // namespace n2
+// CHECK-FIXES: } // namespace n1
+
+#define MACRO macro_expansion
+namespace MACRO {
+  void f();
+  // CHECK-MESSAGES: :[[@LINE+1]]:1: warning: namespace 'MACRO' not terminated with a closing comment [llvm-namespace-comment]
+}
+// CHECK-FIXES: } // namespace MACRO
+
+namespace MACRO {
+  void g();
+} // namespace MACRO
+
+namespace MACRO {
+  void h();
+  // CHECK-MESSAGES: :[[@LINE+1]]:2: warning: namespace 'MACRO' ends with a comment that refers to an expansion of macro [llvm-namespace-comment]
+} // namespace macro_expansion
+// CHECK-FIXES: } // namespace MACRO
+
+namespace n1 {
+namespace MACRO {
+namespace n2 {
+  void f();
+  // CHECK-MESSAGES: :[[@LINE+3]]:1: warning: namespace 'n2' not terminated with a closing comment [llvm-namespace-comment]
+  // CHECK-MESSAGES: :[[@LINE+2]]:2: warning: namespace 'MACRO' not terminated with a closing comment [llvm-namespace-comment]
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: namespace 'n1' not terminated with a closing comment [llvm-namespace-comment]
+}}}
+// CHECK-FIXES: } // namespace n2
+// CHECK-FIXES: } // namespace MACRO
+// CHECK-FIXES: } // namespace n1
Index: clang-tools-extra/test/clang-tidy/checkers/google-readability-namespace-comments.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/google-readability-namespace-comments.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/google-readability-namespace-comments.cpp
@@ -25,10 +25,10 @@
 // 5
 // 6
 // 7
-// CHECK-MESSAGES: :[[@LINE+2]]:1: warning: namespace 'macro_expansion' not terminated with
-// CHECK-MESSAGES: :[[@LINE-10]]:11: note: namespace 'macro_expansion' starts here
+// CHECK-MESSAGES: :[[@LINE+2]]:1: warning: namespace 'MACRO' not terminated with
+// CHECK-MESSAGES: :[[@LINE-10]]:11: note: namespace 'MACRO' starts here
 }
-// CHECK-FIXES: }  // namespace macro_expansion
+// CHECK-FIXES: }  // namespace MACRO
 
 namespace short1 {
 namespace short2 {
Index: clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.h
===
--- clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.h
+++ clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.h
@@ -26,14 +26,29 @@
   NamespaceCommentCheck(StringRef Name, ClangTidyContext *Context);
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override;
+
+  void addMacro(const std::string &Name, const std::string &Value) noexcept;
 
 private:
   void storeOptions(ClangTidyOptions::OptionMap &Options) override;
+  std::string getNamespaceComment(const NamespaceDecl *ND,
+  bool InsertLineBreak);
+  std::string getNamespaceComment(const std::string &NameSpaceName,
+  bool InsertLineBreak);
+  bool isNamespaceMacroDefinition(const StringRef NameSpaceName);
+  std::tuple
+  isNamespaceMacroExpansion(const StringRef NameSpaceName);
 
   llvm::Regex NamespaceCommentPattern;
   const unsigned ShortNamespaceLines;
   const unsigned SpacesBeforeComments;
   llvm::SmallVector Ends;
+
+  // Store macros to verify that warning is not thrown when namespace name is a
+  // preprocessed define.
+  std::vector> Macros;
 };
 
 } // namespace readability
Index: clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp
@@ -19,6 +19,44 @@
 namespace tidy {
 

[PATCH] D69855: [clang-tidy] Fix llvm-namespace-comment for macro expansions

[Marcin Twardak via Phabricator via cfe-commits]

twardakm marked 9 inline comments as done.
twardakm added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp:83
+const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) 
{
+  PP->addPPCallbacks(std::make_unique(PP, this));
+}

aaron.ballman wrote:
> twardakm wrote:
> > aaron.ballman wrote:
> > > Rather than making an entire new object for PP callbacks, why not make 
> > > `NamespaceCommentCheck` inherit from `PPCallbacks`? It seems like it 
> > > would simplify the interface somewhat.
> > I think the check hast to inherit from ClangTidyCheck?
> > 
> > I duplicated the solution from other checks (e.g. 
> > IdentifierNamingCheck.cpp). Could you please point to some existing check 
> > which implements your idea?
> I don't know if we have other checks doing this -- I was thinking of using 
> multiple inheritance in this case, because the callbacks are effectively a 
> mixin.
> 
> That said, I don't insist on this change.
The problem which I see is that addPPCallbacks takes ownership of the object 
passed to it. I couldn't find any other way of invoking PP callbacks, so I'll 
leave it as is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69855



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

[PATCH] D69810: [OpenCL] Fix address space for base method call (PR43145)

[John McCall via Phabricator via cfe-commits]

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

LGTM.


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

https://reviews.llvm.org/D69810



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

[PATCH] D60455: [SYCL] Implement SYCL device code outlining

[Alexey Bader via Phabricator via cfe-commits]

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



Comment at: clang/test/Misc/pragma-attribute-supported-attributes-list.test:134
 // CHECK-NEXT: ReturnsTwice (SubjectMatchRule_function)
+// CHECK-NEXT: SYCLKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)

It looks like this change is not needed anymore. This check fails on my machine 
with the latest version of the patch.

@aaron.ballman, I'm not sure if this is a problem of the implementation or test 
issue.
Do I understand correctly that this test validates the list of the attributes 
which can be applied using `#pragma clang`?
If so, removing this check seems to be okay. We need only 
`[[clang::sycl_kernel]]` or `__attribute__((sycl_kernel))` to work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

[PATCH] D70499: [clang] Fix the path to CrossWinToARMLinux.cmake CMake cache

[Andrei Lebedev via Phabricator via cfe-commits]

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

Thanks for catching and fixing this, Sergej!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70499



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

[PATCH] D70470: [analyzer] Add FuchsiaHandleCheck to catch handle leaks, use after frees and double frees

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

xazax.hun updated this revision to Diff 230287.
xazax.hun marked 10 inline comments as done.
xazax.hun added a comment.

- Explicitly model "maybe" states.
- Fix some escaping issues.
- Address most review comments.


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

https://reviews.llvm.org/D70470

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
  clang/test/Analysis/fuchsia_handle.c

Index: clang/test/Analysis/fuchsia_handle.c
===
--- /dev/null
+++ clang/test/Analysis/fuchsia_handle.c
@@ -0,0 +1,112 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,fuchsia.FuchsiaHandleChecker -verify %s
+
+typedef __typeof__(sizeof(int)) size_t;
+typedef int zx_status_t;
+typedef __typeof__(sizeof(int)) zx_handle_t;
+typedef unsigned int uint32_t;
+#define NULL ((void *)0)
+
+#if defined(__clang__)
+#define XZ_HANDLE_ACQUIRE  __attribute__((acquire_handle))
+#define XZ_HANDLE_RELEASE  __attribute__((release_handle))
+#define XZ_HANDLE_USE  __attribute__((use_handle))
+#else
+#define XZ_HANDLE_ACQUIRE
+#define XZ_HANDLE_RELEASE
+#define XZ_HANDLE_USE
+#endif
+
+zx_status_t zx_channel_create(
+uint32_t options,
+XZ_HANDLE_ACQUIRE zx_handle_t* out0,
+zx_handle_t* out1 XZ_HANDLE_ACQUIRE);
+
+zx_status_t zx_handle_close(
+zx_handle_t handle XZ_HANDLE_RELEASE);
+
+void escape1(zx_handle_t *in);
+void escape2(zx_handle_t in);
+
+void use1(const zx_handle_t *in XZ_HANDLE_USE);
+void use2(zx_handle_t in XZ_HANDLE_USE);
+
+void checkNoLeak01() {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, &sa, &sb);
+  zx_handle_close(sa);
+  zx_handle_close(sb);
+}
+
+void checkNoLeak02() {
+  zx_handle_t ay[2];
+  zx_channel_create(0, &ay[0], &ay[1]);
+  zx_handle_close(ay[0]);
+  zx_handle_close(ay[1]);
+}
+
+void checkNoLeak03() {
+  zx_handle_t ay[2];
+  zx_channel_create(0, &ay[0], &ay[1]);
+  for (int i = 0; i < 2; i++)
+zx_handle_close(ay[i]);
+}
+
+zx_handle_t checkNoLeak04() {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, &sa, &sb);
+  zx_handle_close(sa);
+  return sb; // no warning
+}
+
+zx_handle_t checkNoLeak05(zx_handle_t *out1) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, &sa, &sb);
+  *out1 = sa;
+  return sb; // no warning
+}
+
+void checkNoLeak06() {
+  zx_handle_t sa, sb;
+  if (zx_channel_create(0, &sa, &sb))
+return;
+  zx_handle_close(sa);
+  zx_handle_close(sb);
+} 
+
+void checkNoLeak07(int tag) {
+  zx_handle_t sa, sb;
+  if (zx_channel_create(0, &sa, &sb))
+return;
+  escape1(&sa);
+  escape2(sb);
+}
+
+void checkNoLeak08(int tag) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, &sa, &sb);
+  use1(&sa);
+  if (tag)
+zx_handle_close(sa);
+  use2(sb); // expected-warning {{Potential leak of handle}}
+  zx_handle_close(sb);
+}
+
+void checkDoubleRelease01(int tag) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, &sa, &sb);
+  if (tag)
+zx_handle_close(sa);
+  zx_handle_close(sa); // expected-warning {{Releasing a previously released handle}}
+  zx_handle_close(sb);
+}
+
+void checkUseAfterFree01(int tag) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, &sa, &sb);
+  if (tag) {
+zx_handle_close(sa);
+use1(&sa); // expected-warning {{Using a previously released handle}}
+  }
+  zx_handle_close(sb);
+  use2(sb); // expected-warning {{Using a previously released handle}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
===
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
@@ -0,0 +1,469 @@
+//=== FuchsiaHandleChecker.cpp - Find handle leaks/double closes -*- C++ -*--=//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This checker checks if the handle of Fuchsia is properly used according to
+// following rules.
+//   - If a handle is acquired, it should be released before execution
+//ends.
+//   - If a handle is released, it should not be released again.
+//   - If a handle is released, it should not be used for other purposes
+//such as I/O.
+//
+// In this checker, each tracked handle is associated with a state. When the
+// handle variable is passed to different function calls or syscalls, its state
+// changes. The state changes can be generally represented by following ASCII
+// Art:
+//
+//+---+
+//|   |
+//|release_func failed|
+//| 

[PATCH] D63976: Allow clang -Os and -Oz to work with -flto and lld

[Troy Johnson via Phabricator via cfe-commits]

troyj added a comment.

I ran into this same problem and would like to see this patch or a similar one 
land.  Note that there is also a -Og option to consider, which currently has 
the same problem.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63976



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

[PATCH] D69089: [Parser] #pragma clang transform

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69089



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

sunfish created this revision.
sunfish added reviewers: sbc100, dschuff, aheejin.
Herald added subscribers: dexonsmith, steven_wu, hiraditya, jgravelle-google, 
inglorion, aprantl, mehdi_amini.
Herald added a project: clang.

When the WASM_OPT environment variable is set, run the wasm-opt tool to 
optimize LLVM's output. Note that using wasm-opt currently means invalidating 
all debug info, however we can just tell users this when we tell them about 
WASM_OPT. This fixes PR43796.

Also, add an "llvm-lto" directory to the sysroot library search paths, so that 
sysroots can optionally provide LTO-enabled system libraries.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70500

Files:
  clang/lib/Driver/ToolChains/WebAssembly.cpp
  clang/test/Driver/wasm-toolchain-lto.c


Index: clang/test/Driver/wasm-toolchain-lto.c
===
--- /dev/null
+++ clang/test/Driver/wasm-toolchain-lto.c
@@ -0,0 +1,6 @@
+// A basic C link command-line with optimization with known OS and LTO enabled.
+
+// RUN: %clang -### -O2 -flto -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_OPT_KNOWN %s
+// LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi/llvm-lto" 
"-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" "-lc" 
"{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -90,6 +90,28 @@
   CmdArgs.push_back(Output.getFilename());
 
   C.addCommand(llvm::make_unique(JA, *this, Linker, CmdArgs, Inputs));
+
+  // When optimizing, if WASM_OPT is set, run wasm-opt.
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";
+  if (A->getOption().matches(options::OPT_O4) ||
+  A->getOption().matches(options::OPT_Ofast))
+OOpt = "4";
+  else if (A->getOption().matches(options::OPT_O0))
+OOpt = "0";
+  else if (A->getOption().matches(options::OPT_O))
+OOpt = A->getValue();
+
+  const char *WasmOpt = Args.MakeArgString(WasmOptPath);
+  ArgStringList CmdArgs;
+  CmdArgs.push_back(Output.getFilename());
+  CmdArgs.push_back(Args.MakeArgString(llvm::Twine("-O") + OOpt));
+  CmdArgs.push_back("-o");
+  CmdArgs.push_back(Output.getFilename());
+  C.addCommand(llvm::make_unique(JA, *this, WasmOpt, CmdArgs, 
Inputs));
+}
+  }
 }
 
 WebAssembly::WebAssembly(const Driver &D, const llvm::Triple &Triple,
@@ -109,6 +131,11 @@
   } else {
 const std::string MultiarchTriple =
 getMultiarchTriple(getDriver(), Triple, getDriver().SysRoot);
+if (D.isUsingLTO()) {
+  // For LTO, enable use of lto-enabled sysroot libraries too, if 
available.
+  getFilePaths().push_back(getDriver().SysRoot + "/lib/" + MultiarchTriple 
+
+   "/llvm-lto");
+}
 getFilePaths().push_back(getDriver().SysRoot + "/lib/" + MultiarchTriple);
   }
 }


Index: clang/test/Driver/wasm-toolchain-lto.c
===
--- /dev/null
+++ clang/test/Driver/wasm-toolchain-lto.c
@@ -0,0 +1,6 @@
+// A basic C link command-line with optimization with known OS and LTO enabled.
+
+// RUN: %clang -### -O2 -flto -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_OPT_KNOWN %s
+// LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi/llvm-lto" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -90,6 +90,28 @@
   CmdArgs.push_back(Output.getFilename());
 
   C.addCommand(llvm::make_unique(JA, *this, Linker, CmdArgs, Inputs));
+
+  // When optimizing, if WASM_OPT is set, run wasm-opt.
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";
+  if (A->getOption().matches(options::OPT_O4) ||
+  A->getOption().matches(options::OPT_Ofast))
+OOpt = "4";
+  else if (A->getOption().matches(options::OPT_O0))
+OOpt = "0";
+  else if (A->getOption().matches(options::OPT_O))
+OOpt = A->getValue();
+
+  const char *WasmOpt = Args.MakeArgString(WasmOptPath);
+  ArgStringList CmdArgs;
+  CmdArgs.push_back(Output.getFilename());
+  CmdArgs.push_back(Args.MakeArgString(llvm::Twine("-O") + OOpt));
+  

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

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

xazax.hun marked 3 inline comments as done.
xazax.hun added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:3445
+  let Spellings = [Clang<"acquire_handle">];
+  let Subjects = SubjectList<[Function, ParmVar], ErrorDiag>;
+  let Documentation = [AcquireHandleDocs];

aaron.ballman wrote:
> What about function-like interfaces such as lambdas, blocks, or other 
> callable objects that are not a `FunctionDecl`?
Good point!



Comment at: clang/include/clang/Basic/Attr.td:3450
+def UseHandle : InheritableParamAttr {
+  let Spellings = [Clang<"use_handle">];
+  let Documentation = [UseHandleDocs];

aaron.ballman wrote:
> Should this have a subject limiting it to parameters?
My understanding was that the subject list is "inherited" from 
InheritableParamAttr, which already has it limited to parameters. Is this not 
the case?



Comment at: clang/include/clang/Basic/AttrDocs.td:4474
+  let Content = [{
+If this annotation is on a function it is assumed to return a new handle.
+In case this annotation is on an output parameter, the function is assumed

aaron.ballman wrote:
> What is a "handle"? I think some introduction docs are needed.
Good point. Do you prefer to copy and paste the introduction to all attributes 
or is it enough to only have it for one of them?


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

https://reviews.llvm.org/D70469



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

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

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

xazax.hun updated this revision to Diff 230294.
xazax.hun added a comment.

- Check trivial cases when the acquire_handle attribute is not on an output 
parameter.


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

https://reviews.llvm.org/D70469

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/attr-acquire_handle.c

Index: clang/test/Sema/attr-acquire_handle.c
===
--- /dev/null
+++ clang/test/Sema/attr-acquire_handle.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1  -fsyntax-only -verify %s
+
+void f(int *a __attribute__((acquire_handle)));
+void g(int a __attribute__((acquire_handle))); // expected-error {{attribute only applies to output parameters}}
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -9,6 +9,7 @@
 // CHECK-NEXT: AMDGPUWavesPerEU (SubjectMatchRule_function)
 // CHECK-NEXT: AVRSignal (SubjectMatchRule_function)
 // CHECK-NEXT: AbiTag (SubjectMatchRule_record_not_is_union, SubjectMatchRule_variable, SubjectMatchRule_function, SubjectMatchRule_namespace)
+// CHECK-NEXT: AcquireHandle (SubjectMatchRule_function, SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: Alias (SubjectMatchRule_function, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: AlignValue (SubjectMatchRule_variable, SubjectMatchRule_type_alias)
 // CHECK-NEXT: AllocSize (SubjectMatchRule_function)
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6518,6 +6518,18 @@
   handleSimpleAttribute(S, D, AL);
 }
 
+static void handeAcquireHandleAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  // Warn if the parameter is definitely not an output parameter.
+  if (auto *PVD = dyn_cast(D)) {
+if (PVD->getType()->isIntegerType()) {
+  S.Diag(AL.getLoc(), diag::err_attribute_output_parameter)
+  << AL.getRange();
+  return;
+}
+  }
+  handleSimpleAttribute(S, D, AL);
+}
+
 //===--===//
 // Top Level Sema Entry Points
 //===--===//
@@ -7282,6 +7294,16 @@
   case ParsedAttr::AT_ArmMveAlias:
 handleArmMveAliasAttr(S, D, AL);
 break;
+
+  case ParsedAttr::AT_AcquireHandle:
+handeAcquireHandleAttr(S, D, AL);
+break;
+  case ParsedAttr::AT_ReleaseHandle:
+handleSimpleAttribute(S, D, AL);
+break;
+  case ParsedAttr::AT_UseHandle:
+handleSimpleAttribute(S, D, AL);
+break;
   }
 }
 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3072,6 +3072,8 @@
 def err_cconv_incomplete_param_type : Error<
   "parameter %0 must have a complete type to use function %1 with the %2 "
   "calling convention">;
+def err_attribute_output_parameter : Error<
+  "attribute only applies to output parameters">;
 
 def ext_cannot_use_trivial_abi : ExtWarn<
   "'trivial_abi' cannot be applied to %0">, InGroup;
Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -4467,3 +4467,50 @@
   }
   }];
 }
+
+def AcquireHandleDocs : Documentation {
+  let Category = DocCatDecl;
+  let Content = [{
+If this annotation is on a function it is assumed to return a new handle.
+In case this annotation is on an output parameter, the function is assumed
+to fill the corresponding argument with a new handle.
+
+.. code-block:: c++
+
+  // Output arguments from Zircon.
+  zx_status_t zx_socket_create(uint32_t options,
+   zx_handle_t* out0 [[clang::acquire_handle]],
+   zx_handle_t* out1 [[clang::acquire_handle]]);
+
+
+  // Returned handle.
+  [[clang::acquire_handle]] int open(const char *path, int oflag, ... );
+  }];
+}
+
+def UseHandleDocs : Documentation {
+  let Category = DocCatDecl;
+  let Content = [{
+A function taking a handle by value might close the handle. If a function is
+annotated with `use_handle` it is assumed to not to change the state of the
+handle. It is also assumed to require an open handle to work with.
+
+.. code-block:: c++
+
+  zx_status_t zx_port_wait(zx_handle_t handle [[clang::use_handle]],
+   zx_time_t deadline,
+  

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.
Herald added a subscriber: ormris.



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:96
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";

What would you think about adding a way to pass arguments through to wasm-opt 
on the command line, like we have for the linker, assembler, etc? Something 
like `-Wo,-O2` (or `-Ww` or whatever; analogous to `-Wl` and `-Wa`). That seems 
nicer than an env var, although it doesn't solve the problem of controlling 
whether to run the optimizer in the first place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

This is also missing tests for all the Sema handling (that the attributes only 
appertain to the specified subjects, accept no arguments, etc).




Comment at: clang/include/clang/Basic/Attr.td:3445
+  let Spellings = [Clang<"acquire_handle">];
+  let Subjects = SubjectList<[Function, ParmVar], ErrorDiag>;
+  let Documentation = [AcquireHandleDocs];

What about function-like interfaces such as lambdas, blocks, or other callable 
objects that are not a `FunctionDecl`?



Comment at: clang/include/clang/Basic/Attr.td:3450
+def UseHandle : InheritableParamAttr {
+  let Spellings = [Clang<"use_handle">];
+  let Documentation = [UseHandleDocs];

Should this have a subject limiting it to parameters?



Comment at: clang/include/clang/Basic/Attr.td:3455
+def ReleaseHandle : InheritableParamAttr {
+  let Spellings = [Clang<"release_handle">];
+  let Documentation = [ReleaseHandleDocs];

Should this have a subject limiting it to parameters?



Comment at: clang/include/clang/Basic/AttrDocs.td:4474
+  let Content = [{
+If this annotation is on a function it is assumed to return a new handle.
+In case this annotation is on an output parameter, the function is assumed

What is a "handle"? I think some introduction docs are needed.


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

https://reviews.llvm.org/D70469



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

[PATCH] D70488: [InstCombine] Infer fast math flags on fadd/fsub/fmul/fcmp

[Sanjay Patel via Phabricator via cfe-commits]

spatel added reviewers: cameron.mcinally, mcberg2017, arsenm.
spatel added a comment.
Herald added a subscriber: wdng.

I like the idea, but I'd be more comfortable reviewing the diffs in stages, so 
we know that the test coverage for the value tracking calls is good. So I'd 
prefer if we split this somehow - either by the opcode callers (fadd, fsub, 
fmul...) or the the FMF analysis (nnan, nsz, ninf). That raises a few questions:

1. Why aren't fdiv and frem included?
2. Can we infer FMF for FP intrinsics/libcalls/select/phi? (follow-on patches)
3. We're moving away from FMF on fcmp (recent step: rGebf9bf2cbc8f 
), so is 
it worth including that change, or can we wait for that part to settle? (Side 
question may be if/when we're going to allow FMF on fptrunc/fpextend).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70488



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

[PATCH] D60455: [SYCL] Implement SYCL device code outlining

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: arphaman.
aaron.ballman added a subscriber: arphaman.
aaron.ballman added inline comments.
Herald added a subscriber: dexonsmith.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10079-10080
+def warn_sycl_kernel_invalid_template_param_type : Warning<
+  "template parameter of template functions with 'sycl_kernel' attribute must"
+  " be typename">, InGroup;
+def warn_sycl_kernel_num_of_function_params : Warning<

bader wrote:
> aaron.ballman wrote:
> > This diagnostic reads a bit like you cannot do this: `template ` 
> > when I think the actual restriction is that you cannot do this: `template 
> > `. Is that correct? If so, I think this could be worded as `template 
> > parameter of a function template with the 'sycl_kernel' attribute must be a 
> > template type parameter`.
> > 
> > Just double-checking, but you also intend to prohibit template template 
> > parameters? e.g., you can't do `template  typename C>`
> > This diagnostic reads a bit like you cannot do this: template  
> > when I think the actual restriction is that you cannot do this: template 
> > . Is that correct?
> 
> Yes. That is correct.
> 
> >  If so, I think this could be worded as template parameter of a function 
> > template with the 'sycl_kernel' attribute must be a template type parameter.
> 
> Thanks! Applied your wording.
> 
> > Just double-checking, but you also intend to prohibit template template 
> > parameters? e.g., you can't do template  typename C>
> 
> Currently we allow following use case: 
> https://github.com/intel/llvm/blob/sycl/clang/test/SemaSYCL/mangle-kernel.cpp.
>  I assume it qualifies as "template type" and not as "template template" 
> parameter. Right?
> 
> Quoting SYCL specification $6.2 Naming of kernels 
> (https://www.khronos.org/registry/SYCL/specs/sycl-1.2.1.pdf#page=250).
> 
> > SYCL kernels are extracted from C++ source files and stored in an 
> > implementation- defined format. In the case of
> > the shared-source compilation model, the kernels have to be uniquely 
> > identified by both host and device compiler.
> > This is required in order for the host runtime to be able to load the 
> > kernel by using the OpenCL host runtime
> > interface.
> > From this requirement the following rules apply for naming the kernels:
> > • The kernel name is a C++ typename.
> > • The kernel needs to have a globally-visible name. In the case of a named 
> > function object type, the name can
> > be the typename of the function object, as long as it is globally-visible. 
> > In the case where it isn’t, a globally visible name has to be provided, as 
> > template parameter to the kernel invoking interface, as described in 4.8.5.
> > In C++11, lambdas do not have a globally-visible name, so a 
> > globally-visible typename has to be provided
> > in the kernel invoking interface, as described in 4.8.5.
> > • The kernel name has to be a unique identifier in the program.
> > 
> 
> We also have an extension, which lifts these restrictions/requirements when 
> clang is used as host and device compiler. @erichkeane implemented built-in 
> function (https://github.com/intel/llvm/pull/250) providing "unique 
> identifier", which we use as a kernel name for lambda objects. But this is 
> going to be a separate patch.
> Currently we allow following use case: 
> https://github.com/intel/llvm/blob/sycl/clang/test/SemaSYCL/mangle-kernel.cpp.
>  I assume it qualifies as "template type" and not as "template template" 
> parameter. Right?

Yeah, those are template types. A template template parameter would be: 
https://godbolt.org/z/9kwbW9
In that example, `C` is a template template parameter and `Ty` is a template 
type parameter. The part I'm a bit unclear on is why a template template 
parameter should be disallowed (I believe it names a type, as opposed to a 
non-type template parameter which names a value)?



Comment at: clang/test/Misc/pragma-attribute-supported-attributes-list.test:134
 // CHECK-NEXT: ReturnsTwice (SubjectMatchRule_function)
+// CHECK-NEXT: SYCLKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)

bader wrote:
> It looks like this change is not needed anymore. This check fails on my 
> machine with the latest version of the patch.
> 
> @aaron.ballman, I'm not sure if this is a problem of the implementation or 
> test issue.
> Do I understand correctly that this test validates the list of the attributes 
> which can be applied using `#pragma clang`?
> If so, removing this check seems to be okay. We need only 
> `[[clang::sycl_kernel]]` or `__attribute__((sycl_kernel))` to work.
Your understanding is correct, and I think it's a bug if you don't need to add 
an entry here for `SYCLKernel`. @arphaman, WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



_

[PATCH] D70477: [Clang] Enable RISC-V support for Fuchsia

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.



Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:130
 
+  foreach(target x86_64;aarch64)
 # Set the per-target runtimes options.

I don't know what's preventing you from enabling compiling the runtimes. I made 
a cmake config that built the runtimes, in part based off the fuschia config in 
this file.

I suppose this requires you to have a RISC-V sysroot, which you may not have.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70477



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

[PATCH] D69855: [clang-tidy] Fix llvm-namespace-comment for macro expansions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman marked an inline comment as done.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!




Comment at: 
clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp:83
+const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) 
{
+  PP->addPPCallbacks(std::make_unique(PP, this));
+}

twardakm wrote:
> aaron.ballman wrote:
> > twardakm wrote:
> > > aaron.ballman wrote:
> > > > Rather than making an entire new object for PP callbacks, why not make 
> > > > `NamespaceCommentCheck` inherit from `PPCallbacks`? It seems like it 
> > > > would simplify the interface somewhat.
> > > I think the check hast to inherit from ClangTidyCheck?
> > > 
> > > I duplicated the solution from other checks (e.g. 
> > > IdentifierNamingCheck.cpp). Could you please point to some existing check 
> > > which implements your idea?
> > I don't know if we have other checks doing this -- I was thinking of using 
> > multiple inheritance in this case, because the callbacks are effectively a 
> > mixin.
> > 
> > That said, I don't insist on this change.
> The problem which I see is that addPPCallbacks takes ownership of the object 
> passed to it. I couldn't find any other way of invoking PP callbacks, so I'll 
> leave it as is.
Ah, that makes sense. Thank you for investigating it. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69855



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

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:3450
+def UseHandle : InheritableParamAttr {
+  let Spellings = [Clang<"use_handle">];
+  let Documentation = [UseHandleDocs];

xazax.hun wrote:
> aaron.ballman wrote:
> > Should this have a subject limiting it to parameters?
> My understanding was that the subject list is "inherited" from 
> InheritableParamAttr, which already has it limited to parameters. Is this not 
> the case?
It doesn't seem to be the case looking at Attr.td:557 or thereabouts. Adding 
tests for the attributes should clarify whether the change is needed or not, 
too.



Comment at: clang/include/clang/Basic/AttrDocs.td:4474
+  let Content = [{
+If this annotation is on a function it is assumed to return a new handle.
+In case this annotation is on an output parameter, the function is assumed

xazax.hun wrote:
> aaron.ballman wrote:
> > What is a "handle"? I think some introduction docs are needed.
> Good point. Do you prefer to copy and paste the introduction to all 
> attributes or is it enough to only have it for one of them?
You can set up a documentation category to put all the related attributes 
under, and that category can have the introductory documentation. You can see 
an example of this with the calling convention attributes. There is a 
`DocCatCallingConvs` that is then the `Category` for all the other attributes.


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

https://reviews.llvm.org/D70469



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Please update the go bindings and include a release note that this API is 
changing & why.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111



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

[PATCH] D70477: [Clang] Enable RISC-V support for Fuchsia

[Petr Hosek via Phabricator via cfe-commits]

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



Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:130
 
+  foreach(target x86_64;aarch64)
 # Set the per-target runtimes options.

lenary wrote:
> I don't know what's preventing you from enabling compiling the runtimes. I 
> made a cmake config that built the runtimes, in part based off the fuschia 
> config in this file.
> 
> I suppose this requires you to have a RISC-V sysroot, which you may not have.
Correct, as mentioned in the summary of this change, we don't have a sysroot 
yet. Once the sysroot is ready, we'll enable runtimes as well in a follow up 
change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70477



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:96
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";

dschuff wrote:
> What would you think about adding a way to pass arguments through to wasm-opt 
> on the command line, like we have for the linker, assembler, etc? Something 
> like `-Wo,-O2` (or `-Ww` or whatever; analogous to `-Wl` and `-Wa`). That 
> seems nicer than an env var, although it doesn't solve the problem of 
> controlling whether to run the optimizer in the first place.
My guess here is that we don't need clang to have an option for passing 
additional flags -- if people want to do extra special things with wasm-opt on 
clang's output they can just run wasm-opt directly themselves. Does that sound 
reasonable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70481: [clangd] Fix a crash in expected types

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

This broke clangd tests on Windows: 
http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/12324

We now have clangd tests running on main llvm bots (namely, on 
http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/). Do they not send 
mail?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70481



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

[PATCH] D70488: [InstCombine] Infer fast math flags on fadd/fsub/fmul/fcmp

[Michael Berg via Phabricator via cfe-commits]

mcberg2017 added a comment.

For us this would be an impediment as we have math models that want ieee 
behavior while relaxing precision.  Adding nnan or ninf would obstruct those 
choices.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70488



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

[PATCH] D70041: register cuda language activation event and activate for .cuh files

[Paul Taylor via Phabricator via cfe-commits]

ptaylor updated this revision to Diff 230302.
ptaylor added a comment.

- fix package.json whitespace


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70041

Files:
  clang-tools-extra/clangd/clients/clangd-vscode/package.json
  clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts


Index: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
@@ -83,21 +83,18 @@
   }
   const serverOptions: vscodelc.ServerOptions = clangd;
 
-  // Note that CUDA ('.cu') files are special. When opening files of all other
-  // extensions, VSCode would load clangd automatically. This is achieved by
-  // having a corresponding 'onLanguage:...' activation event in package.json.
-  // However, VSCode does not have CUDA as a supported language yet, so we
-  // cannot add a corresponding activationEvent for CUDA files and clangd will
-  // *not* load itself automatically on '.cu' files.
-  const cudaFilePattern: string = '**/*.{' + [ 'cu' ].join() + '}';
   const clientOptions: vscodelc.LanguageClientOptions = {
 // Register the server for c-family and cuda files.
 documentSelector: [
 { scheme: 'file', language: 'c' },
 { scheme: 'file', language: 'cpp' },
+// Syntax highlighting for the 'cuda' language is
+// provided by the kriegalex.vscode-cudacpp plugin.
+// @see https://github.com/kriegalex/vscode-cuda
+// @see 
https://marketplace.visualstudio.com/items?itemName=kriegalex.vscode-cudacpp
+{ scheme: 'file', language: 'cuda' },
 { scheme: 'file', language: 'objective-c'},
-{ scheme: 'file', language: 'objective-cpp'},
-{ scheme: 'file', pattern: cudaFilePattern },
+{ scheme: 'file', language: 'objective-cpp'}
 ],
 synchronize: !syncFileEvents ? undefined : {
 // FIXME: send sync file events when clangd provides implemenatations.
@@ -111,10 +108,10 @@
 serverOptions, clientOptions);
   if (getConfig('semanticHighlighting')) {
 const semanticHighlightingFeature =
-  new semanticHighlighting.SemanticHighlightingFeature(clangdClient,
-context);
+new semanticHighlighting.SemanticHighlightingFeature(clangdClient,
+ context);
 context.subscriptions.push(
-  vscode.Disposable.from(semanticHighlightingFeature));
+vscode.Disposable.from(semanticHighlightingFeature));
 clangdClient.registerFeature(semanticHighlightingFeature);
   }
   console.log('Clang Language Server is now active!');
Index: clang-tools-extra/clangd/clients/clangd-vscode/package.json
===
--- clang-tools-extra/clangd/clients/clangd-vscode/package.json
+++ clang-tools-extra/clangd/clients/clangd-vscode/package.json
@@ -23,6 +23,7 @@
 "activationEvents": [
 "onLanguage:c",
 "onLanguage:cpp",
+"onLanguage:cuda",
 "onLanguage:objective-c",
 "onLanguage:objective-cpp",
 "onCommand:clangd-vscode.activate"
@@ -64,6 +65,13 @@
 "**/MSVC/*/include/**"
 ],
 "firstLine": "^/[/*].*-\\*-\\s*C\\+\\+\\s*-\\*-.*"
+},
+{
+"id": "cuda",
+"extensions": [
+".cu",
+".cuh"
+]
 }
 ],
 "configuration": {


Index: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
@@ -83,21 +83,18 @@
   }
   const serverOptions: vscodelc.ServerOptions = clangd;
 
-  // Note that CUDA ('.cu') files are special. When opening files of all other
-  // extensions, VSCode would load clangd automatically. This is achieved by
-  // having a corresponding 'onLanguage:...' activation event in package.json.
-  // However, VSCode does not have CUDA as a supported language yet, so we
-  // cannot add a corresponding activationEvent for CUDA files and clangd will
-  // *not* load itself automatically on '.cu' files.
-  const cudaFilePattern: string = '**/*.{' + [ 'cu' ].join() + '}';
   const clientOptions: vscodelc.LanguageClientOptions = {
 // Register the server for c-family and cuda files.
 documentSelector: [
 { scheme: 'file', language: 'c' },
 { scheme: 'file', language: 'cpp' },
+// Syntax highlighting for the 'cuda' language is

[PATCH] D70481: [clangd] Fix a crash in expected types

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Reverted for now in 6de45772e0 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70481



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

[PATCH] D70041: register cuda language activation event and activate for .cuh files

[Paul Taylor via Phabricator via cfe-commits]

ptaylor updated this revision to Diff 230301.
ptaylor added a comment.

- add package.json entry to contribute cuda language id
- remove extra cuda file patterns, update comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70041

Files:
  clang-tools-extra/clangd/clients/clangd-vscode/package.json
  clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts


Index: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
@@ -83,21 +83,18 @@
   }
   const serverOptions: vscodelc.ServerOptions = clangd;
 
-  // Note that CUDA ('.cu') files are special. When opening files of all other
-  // extensions, VSCode would load clangd automatically. This is achieved by
-  // having a corresponding 'onLanguage:...' activation event in package.json.
-  // However, VSCode does not have CUDA as a supported language yet, so we
-  // cannot add a corresponding activationEvent for CUDA files and clangd will
-  // *not* load itself automatically on '.cu' files.
-  const cudaFilePattern: string = '**/*.{' + [ 'cu' ].join() + '}';
   const clientOptions: vscodelc.LanguageClientOptions = {
 // Register the server for c-family and cuda files.
 documentSelector: [
 { scheme: 'file', language: 'c' },
 { scheme: 'file', language: 'cpp' },
+// Syntax highlighting for the 'cuda' language is
+// provided by the kriegalex.vscode-cudacpp plugin.
+// @see https://github.com/kriegalex/vscode-cuda
+// @see 
https://marketplace.visualstudio.com/items?itemName=kriegalex.vscode-cudacpp
+{ scheme: 'file', language: 'cuda' },
 { scheme: 'file', language: 'objective-c'},
-{ scheme: 'file', language: 'objective-cpp'},
-{ scheme: 'file', pattern: cudaFilePattern },
+{ scheme: 'file', language: 'objective-cpp'}
 ],
 synchronize: !syncFileEvents ? undefined : {
 // FIXME: send sync file events when clangd provides implemenatations.
@@ -111,10 +108,10 @@
 serverOptions, clientOptions);
   if (getConfig('semanticHighlighting')) {
 const semanticHighlightingFeature =
-  new semanticHighlighting.SemanticHighlightingFeature(clangdClient,
-context);
+new semanticHighlighting.SemanticHighlightingFeature(clangdClient,
+ context);
 context.subscriptions.push(
-  vscode.Disposable.from(semanticHighlightingFeature));
+vscode.Disposable.from(semanticHighlightingFeature));
 clangdClient.registerFeature(semanticHighlightingFeature);
   }
   console.log('Clang Language Server is now active!');
Index: clang-tools-extra/clangd/clients/clangd-vscode/package.json
===
--- clang-tools-extra/clangd/clients/clangd-vscode/package.json
+++ clang-tools-extra/clangd/clients/clangd-vscode/package.json
@@ -23,6 +23,7 @@
 "activationEvents": [
 "onLanguage:c",
 "onLanguage:cpp",
+"onLanguage:cuda",
 "onLanguage:objective-c",
 "onLanguage:objective-cpp",
 "onCommand:clangd-vscode.activate"
@@ -64,7 +65,14 @@
 "**/MSVC/*/include/**"
 ],
 "firstLine": "^/[/*].*-\\*-\\s*C\\+\\+\\s*-\\*-.*"
-}
+},
+   {
+   "id": "cuda",
+   "extensions": [
+   ".cu",
+   ".cuh"
+   ]
+   }
 ],
 "configuration": {
 "type": "object",


Index: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
@@ -83,21 +83,18 @@
   }
   const serverOptions: vscodelc.ServerOptions = clangd;
 
-  // Note that CUDA ('.cu') files are special. When opening files of all other
-  // extensions, VSCode would load clangd automatically. This is achieved by
-  // having a corresponding 'onLanguage:...' activation event in package.json.
-  // However, VSCode does not have CUDA as a supported language yet, so we
-  // cannot add a corresponding activationEvent for CUDA files and clangd will
-  // *not* load itself automatically on '.cu' files.
-  const cudaFilePattern: string = '**/*.{' + [ 'cu' ].join() + '}';
   const clientOptions: vscodelc.LanguageClientOptions = {
 // Register th

[clang-tools-extra] 6de4577 - Revert "[clangd] Fix a crash in expected types"

[Nico Weber via cfe-commits]

Author: Nico Weber
Date: 2019-11-20T14:38:35-05:00
New Revision: 6de45772e0910bf7fa626e5493a2798b071eb26c

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

LOG: Revert "[clangd] Fix a crash in expected types"

This reverts commit b5135a86e04761577494c70e7c0057136cc90b5b.
Test fails on Windows.

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/ExpectedTypes.cpp 
b/clang-tools-extra/clangd/ExpectedTypes.cpp
index a82a64cf14e2..3b0779ea66bc 100644
--- a/clang-tools-extra/clangd/ExpectedTypes.cpp
+++ b/clang-tools-extra/clangd/ExpectedTypes.cpp
@@ -44,10 +44,12 @@ static const Type *toEquivClass(ASTContext &Ctx, QualType 
T) {
 static llvm::Optional
 typeOfCompletion(const CodeCompletionResult &R) {
   const NamedDecl *D = R.Declaration;
+  if (!D)
+return llvm::None;
   // Templates do not have a type on their own, look at the templated decl.
-  if (auto *Template = dyn_cast_or_null(D))
+  if (auto *Template = dyn_cast(D))
 D = Template->getTemplatedDecl();
-  auto *VD = dyn_cast_or_null(D);
+  auto *VD = dyn_cast(D);
   if (!VD)
 return llvm::None; // We handle only variables and functions below.
   auto T = VD->getType();

diff  --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp 
b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index e69b2a6205f6..5b50b9fe9f8b 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1030,16 +1030,6 @@ TEST(CompletionTest, DefaultArgs) {
 SnippetSuffix("(${1:int A})";
 }
 
-TEST(CompletionTest, NoCrashWithTemplateParamsAndPreferredTypes) {
-  auto Completions = completions(R"cpp(
-template  class TT> int foo() {
-  int a = ^
-}
-)cpp")
- .Completions;
-  EXPECT_THAT(Completions, Contains(Named("TT")));
-}
-
 SignatureHelp signatures(llvm::StringRef Text, Position Point,
  std::vector IndexSymbols = {}) {
   std::unique_ptr Index;



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

[PATCH] D70499: [clang] Fix the path to CrossWinToARMLinux.cmake CMake cache

[Vlad Vereschaka via Phabricator via cfe-commits]

vvereschaka accepted this revision.
vvereschaka added a comment.

Thank you Sergej.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70499



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

[clang] fd8d915 - Fix parser bug that permitted 'private' as a (no-op) decl-specifier even outside OpenCL.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2019-11-20T11:59:58-08:00
New Revision: fd8d9155a997ab0f3ef3d7dff1c56efc9b692bfe

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

LOG: Fix parser bug that permitted 'private' as a (no-op) decl-specifier even 
outside OpenCL.

Added: 


Modified: 
clang/lib/Parse/ParseDecl.cpp
clang/test/Parser/cxx-decl.cpp

Removed: 




diff  --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index e5c17a3131ab..a90147ca4692 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -3949,9 +3949,14 @@ void Parser::ParseDeclarationSpecifiers(DeclSpec &DS,
 PrevSpec = Tok.getIdentifierInfo()->getNameStart();
 isInvalid = true;
 break;
-  };
+  }
   LLVM_FALLTHROUGH;
 case tok::kw_private:
+  // It's fine (but redundant) to check this for __generic on the
+  // fallthrough path; we only form the __generic token in OpenCL mode.
+  if (!getLangOpts().OpenCL)
+goto DoneWithDeclSpec;
+  LLVM_FALLTHROUGH;
 case tok::kw___private:
 case tok::kw___global:
 case tok::kw___local:

diff  --git a/clang/test/Parser/cxx-decl.cpp b/clang/test/Parser/cxx-decl.cpp
index c60e42f28eef..a868904bb36d 100644
--- a/clang/test/Parser/cxx-decl.cpp
+++ b/clang/test/Parser/cxx-decl.cpp
@@ -6,6 +6,8 @@ const char const *x10; // expected-error {{duplicate 'const' 
declaration specifi
 
 int x(*g); // expected-error {{use of undeclared identifier 'g'}}
 
+private int cplusplus_is_not_opencl; // expected-error {{expected 
unqualified-id}}
+
 struct Type {
   int Type;
 };



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

[PATCH] D60455: [SYCL] Implement SYCL device code outlining

[Alexey Bader via Phabricator via cfe-commits]

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

Applied code review comments from Aaron.

Allow template template parameters for function templates marked with 
`sycl_kernel` attribute.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ASTContext.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenSYCL/device-functions.cpp
  clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
  clang/test/SemaSYCL/device-attributes.cpp

Index: clang/test/SemaSYCL/device-attributes.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl-is-device -verify %s
+
+[[clang::sycl_kernel]] int gv2 = 0; // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+__attribute__((sycl_kernel)) int gv3 = 0; // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+
+__attribute__((sycl_kernel(1))) void foo(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+[[clang::sycl_kernel(1)]] void foo2(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+
+// Only function templates
+__attribute__((sycl_kernel)) void foo(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+[[clang::sycl_kernel]] void foo1(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+
+// At least two template parameters
+template 
+__attribute__((sycl_kernel)) void foo(T P); // expected-warning {{'sycl_kernel' attribute only applies to a function template with at least two template parameters}}
+template 
+[[clang::sycl_kernel]] void foo1(T P); // expected-warning {{'sycl_kernel' attribute only applies to a function template with at least two template parameters}}
+
+// First two template parameters can't be non-type template parameters
+template 
+__attribute__((sycl_kernel)) void foo(T P); // expected-warning {{template parameter of a function template with the 'sycl_kernel' attribute can't be a non-type template parameter}}
+template 
+[[clang::sycl_kernel]] void foo1(T P); // expected-warning {{template parameter of a function template with the 'sycl_kernel' attribute can't be a non-type template parameter}}
+
+// Must return void
+template 
+__attribute__((sycl_kernel)) int foo(T P); // expected-warning {{function template with 'sycl_kernel' attribute must have a 'void' return type}}
+template 
+[[clang::sycl_kernel]] int foo1(T P); // expected-warning {{function template with 'sycl_kernel' attribute must have a 'void' return type}}
+
+// Must take at least one argument
+template 
+__attribute__((sycl_kernel)) void foo(); // expected-warning {{function template with 'sycl_kernel' attribute must have a single parameter}}
+template 
+[[clang::sycl_kernel]] void foo1(T t, A a); // expected-warning {{function template with 'sycl_kernel' attribute must have a single parameter}}
+
+// No diagnostics
+template 
+__attribute__((sycl_kernel)) void foo(T P);
+template 
+[[clang::sycl_kernel]] void foo1(T P);
Index: clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl-is-device -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -x c++ %s
+
+#ifndef __SYCL_DEVICE_ONLY__
+// expected-warning@+7 {{'sycl_kernel' attribute ignored}}
+// expected-warning@+8 {{'sycl_kernel' attribute ignored}}
+#else
+// expected-no-diagnostics
+#endif
+
+template 
+__attribute__((sycl_kernel)) void foo(T P);
+template 
+[[clang::sycl_kernel]] void foo1(T P);
Index: clang/test/CodeGenSYCL/device-functions.cpp
===
--- /dev/null
+++ clang/test/CodeGenSYCL/device-functions.cpp
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -triple spir64 -fsycl-is-device -S -emit-llvm %s -o - | FileCheck %s
+
+template 
+T bar(T arg);
+
+void foo() {
+  int a = 1 + 1 + bar(1);
+}
+
+template 
+T bar(T arg) {
+  return arg;
+}
+
+template 
+__attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
+  kernelFunc();
+}
+
+// Make sure that definitions for the types not used in SYCL kernels are not
+// emitted
+// CHECK-NOT: %struct.A
+// CHECK-NOT: @a = {{.*}} %struct.A
+struct A {
+  int x = 10;
+} a;
+
+int main() {
+  a.x = 8;
+  kernel_single_task([]() { foo(); });
+  return 0;
+}
+
+// baz is not called from the SYCL kernel, so it must not be emitted
+// CHECK-NOT: define {{.*}} @{{.*}}baz
+void baz() {}
+

[PATCH] D70499: [clang] Fix the path to CrossWinToARMLinux.cmake CMake cache

[Sergej Jaskiewicz via Phabricator via cfe-commits]

broadwaylamb added a comment.

Can you commit this for me?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70499



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added a comment.

Kinds seems like two different changes here.   But lgtm.




Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:137
+  getFilePaths().push_back(getDriver().SysRoot + "/lib/" + MultiarchTriple 
+
+   "/llvm-lto");
+}

Is there any kind of president here?   i.e. do any other platforms have support 
for this kind thing?  Seems like good idea I'd just like to be sure we follow 
existing practices.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:137
+  getFilePaths().push_back(getDriver().SysRoot + "/lib/" + MultiarchTriple 
+
+   "/llvm-lto");
+}

sbc100 wrote:
> Is there any kind of president here?   i.e. do any other platforms have 
> support for this kind thing?  Seems like good idea I'd just like to be sure 
> we follow existing practices.
I looked around, but didn't see anything similar.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70416: [Driver] Make -static-libgcc imply static libunwind

[Josh Kunz via Phabricator via cfe-commits]

jkz added a comment.

In D70416#1750978 , @joerg wrote:

> This is normally done by using `-Bstatic`/`-Bdynamic` around the library. See 
> `tools::addOpenMPRuntime`.


`-Bstatic`/`-Bdynamic` wrapping does seem to be more common, but that will 
change the linkage for libraries that are added after the run-time libraries 

 (e.g., `-lpthread`). We would need something like `--push-state -Bstatic 
-lunwind --pop-state` to preserve the "mode" of the link. (IMO, the existing 
usages of `-Bstatic`, `-Bdynamic` wrapping are wrong). The current approach is 
simpler.

Additionally, I think saugustine's reasoning makes sense here, and we should 
match the libgcc behavior of requiring a `.a` or `.so` depending on the "mode". 
I'd prefer to keep this patch the way it is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70416



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