[PATCH] D63473: Support -fclang-abi-compat=8.0 to keep old ABI behavior

[Wei Xiao via Phabricator via cfe-commits]

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

Yes, there is a test to ensure that Darwin defaults to the old behaviour in 
"test/CodeGen/x86_32-m64.c".


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

https://reviews.llvm.org/D63473



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

[PATCH] D62115: fix a issue that clang is incompatible with gcc with -H option.

[Kan Shengchen via Phabricator via cfe-commits]

skan added a comment.

In D62115#1547698 , @kimgr wrote:

> This looks good to me, thanks!


Could you help accept this patch?


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

https://reviews.llvm.org/D62115



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

[PATCH] D63473: Support -fclang-abi-compat=8.0 to keep old ABI behavior

[Wei Xiao via Phabricator via cfe-commits]

wxiao3 updated this revision to Diff 205261.
wxiao3 added a reviewer: compnerd.

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

https://reviews.llvm.org/D63473

Files:
  docs/ReleaseNotes.rst
  include/clang/Basic/LangOptions.h
  lib/CodeGen/TargetInfo.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/x86_32-m64.c


Index: test/CodeGen/x86_32-m64.c
===
--- test/CodeGen/x86_32-m64.c
+++ test/CodeGen/x86_32-m64.c
@@ -3,6 +3,7 @@
 // RUN: %clang_cc1 -w -O2 -fblocks -triple i386-apple-darwin9 -target-cpu 
yonah -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,DARWIN
 // RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-elfiamcu -mfloat-abi soft 
-emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,IAMCU
 // RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-win32 -emit-llvm -o - %s | 
FileCheck %s --check-prefixes=CHECK,WIN32
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-linux-gnu -target-cpu 
pentium4 -emit-llvm -fclang-abi-compat=8.0 -o - %s | FileCheck %s 
--check-prefixes=CHECK,OLDABI
 
 #include 
 __m64 m64;
@@ -24,6 +25,8 @@
 // WIN32-LABEL: define dso_local <1 x i64> @caller(i64 %__m1.coerce, i64 
%__m2.coerce)
 // WIN32: call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
 // WIN32: ret <1 x i64>
+// OLDABI: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// OLDABI: ret <1 x i64>
   callee(__m2, __m1);
   return m64;
 }
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -3104,6 +3104,8 @@
 Opts.setClangABICompat(LangOptions::ClangABI::Ver6);
   else if (Major <= 7)
 Opts.setClangABICompat(LangOptions::ClangABI::Ver7);
+  else if (Major <= 8)
+Opts.setClangABICompat(LangOptions::ClangABI::Ver8);
 } else if (Ver != "latest") {
   Diags.Report(diag::err_drv_invalid_value)
   << A->getAsString(Args) << A->getValue();
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -1088,6 +1088,11 @@
   }
 
   bool isPassInMMXRegABI() const {
+// Clang <= 8.0 did not do this for compatiblity with old behavior.
+if (getContext().getLangOpts().getClangABICompat() <=
+LangOptions::ClangABI::Ver8)
+  return false;
+
 // The System V i386 psABI requires __m64 to be passed in MMX registers.
 // Clang historically had a bug where it failed to apply this rule, and
 // some platforms (e.g. Darwin, PS4, and FreeBSD) have opted to maintain
Index: include/clang/Basic/LangOptions.h
===
--- include/clang/Basic/LangOptions.h
+++ include/clang/Basic/LangOptions.h
@@ -138,6 +138,11 @@
 /// rather than returning the required alignment.
 Ver7,
 
+/// Attempt to be ABI-compatible with code generated by Clang 8.0.x
+/// (SVN r363116). This causes __m64 to be passed in MMX register 
+/// instead of integer register.
+Ver8,
+
 /// Conform to the underlying platform's C and C++ ABIs as closely
 /// as we can.
 Latest
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -142,6 +142,14 @@
 ABI Changes in Clang
 
 
+- The System V i386 psABI requires __m64 to be passed in MMX registers.
+  Clang historically had a bug where it failed to apply this rule, and
+  some platforms (e.g. Darwin, PS4, and FreeBSD) have opted to maintain
+  compatibility with the old Clang behavior, so we only apply it on
+  platforms that have specifically requested it (currently just Linux and
+  NetBSD). You can switch back to old API behavior with flag:
+  -fclang-abi-compat=8.0.
+
 - ...
 
 OpenMP Support in Clang


Index: test/CodeGen/x86_32-m64.c
===
--- test/CodeGen/x86_32-m64.c
+++ test/CodeGen/x86_32-m64.c
@@ -3,6 +3,7 @@
 // RUN: %clang_cc1 -w -O2 -fblocks -triple i386-apple-darwin9 -target-cpu yonah -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,DARWIN
 // RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-elfiamcu -mfloat-abi soft -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,IAMCU
 // RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-win32 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,WIN32
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-linux-gnu -target-cpu pentium4 -emit-llvm -fclang-abi-compat=8.0 -o - %s | FileCheck %s --check-prefixes=CHECK,OLDABI
 
 #include 
 __m64 m64;
@@ -24,6 +25,8 @@
 // WIN32-LABEL: define dso_local <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
 // WIN32: call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
 // WIN32: ret <1 x i64>
+// OLDABI: tail call void @calle

[PATCH] D62115: fix a issue that clang is incompatible with gcc with -H option.

[Kim Gräsman via Phabricator via cfe-commits]

kimgr added a comment.

In D62115#1547705 , @skan wrote:

> In D62115#1547698 , @kimgr wrote:
>
> > This looks good to me, thanks!
>
>
> Could you help accept this patch?


I'm not active in the project, so I don't feel comfortable accepting. See if 
you can find a contributor to sign it off.


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

https://reviews.llvm.org/D62115



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

[PATCH] D63473: Support -fclang-abi-compat=8.0 to keep old ABI behavior

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added inline comments.



Comment at: test/CodeGen/x86_32-m64.c:6
 // RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-win32 -emit-llvm -o - %s | 
FileCheck %s --check-prefixes=CHECK,WIN32
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-linux-gnu -target-cpu 
pentium4 -emit-llvm -fclang-abi-compat=8.0 -o - %s | FileCheck %s 
--check-prefixes=CHECK,OLDABI
 

Use ABI80 instead of OLDABI?


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

https://reviews.llvm.org/D63473



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

[PATCH] D63426: [clangd] Narrow rename to local symbols.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:79
+
+  if (Index) {
+// Consult the index to determine whether the symbol is used outside of

pull out a function for this?



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:83
+// FIXME: we may skip querying the index if D is function-local.
+const NamedDecl *D =
+clang::tooling::getNamedDeclAt(ASTCtx, SourceLocationBeg);

it's unfortunate that our "what's under the cursor" logic here has to duplicate 
what's done by RenameOccurrences::initiate().

Can we have RenameOccurrences expose the NamedDecl instead?



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:83
+// FIXME: we may skip querying the index if D is function-local.
+const NamedDecl *D =
+clang::tooling::getNamedDeclAt(ASTCtx, SourceLocationBeg);

sammccall wrote:
> it's unfortunate that our "what's under the cursor" logic here has to 
> duplicate what's done by RenameOccurrences::initiate().
> 
> Can we have RenameOccurrences expose the NamedDecl instead?
does this not work for macros?



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:85
+clang::tooling::getNamedDeclAt(ASTCtx, SourceLocationBeg);
+if (!D)
+  return llvm::make_error(

We're effectively doing this test twice (see `if (!Rename)` below) and emitting 
different error mesages

maybe we should just skip the rest of the check in this case?
(Exposing the ND from the RenameOccurrences would help make this duplication 
more obvious)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63426



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

[PATCH] D62538: [clangd] Add hidden tweaks to dump AST/selection.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

i mean ping


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D62538



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

[PATCH] D62538: [clangd] Add hidden tweaks to dump AST/selection.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

pind


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D62538



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

[PATCH] D62953: [Syntax] Do not glue multiple empty PP expansions to a single mapping

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Can you explain why this is important?
(in the code)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62953



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

[PATCH] D59744: Fix i386 ABI "__m64" type bug

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In D59744#1547445 , @wxiao3 wrote:

> @hans
>
> Please make sure all Chromium for 32-bit Linux libraries are following System 
> V ABI (i.e., m64 is passed on mmx register). I suspect that there are some 
> hand written assembly code in your libraries which is not following the ABI.


We still don't have the root cause, but the library in question (Skia) doesn't 
have much assembly code. After your patch, %st0 (which aliases with %mm0) gets 
clobbered if a function returns a 4 x u16 vector. Skia tries to work around 
this by force-inlining such functions, but we're still seeing functions where 
%mm0 gets used. We believe this is the cause, but I'm still trying to figure 
out where the remaining %mm0 uses come from.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59744



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

[PATCH] D62954: [Syntax] Add a helper to find expansion by its first spelled token

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:204
+  /// An expansion produced by the preprocessor, includes macro expansions and
+  /// macro directives. Preprocessor always maps a non-empty range of spelled
+  /// tokens to a (possibly empty) range of expanded tokens.

macro directives -> preprocessor directives?



Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:207
+  /// Here is a few examples of expansions:
+  ///#pragme once  // Expansion from "#pragma once" to an empty range.
+  ///#define FOO 1 2 3 // Expansion from "#define FOO 1" to an empty range.

nit: "pragma"



Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:207
+  /// Here is a few examples of expansions:
+  ///#pragme once  // Expansion from "#pragma once" to an empty range.
+  ///#define FOO 1 2 3 // Expansion from "#define FOO 1" to an empty range.

sammccall wrote:
> nit: "pragma"
these would be clearer without repeating the text, e.g. "Expands to an empty 
range"



Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:209
+  ///#define FOO 1 2 3 // Expansion from "#define FOO 1" to an empty range.
+  ///FOO   // Expansion from "FOO" to "1 2 3".
+  struct Expansion {

also mention #include?



Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:214
+  };
+  /// If \p Spelled starts a mapping (e.g. if it's a macro name) return the
+  /// subrange of expanded tokens that the macro expands to.

maybe mention `#` as it's the other common case



Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:216
+  /// subrange of expanded tokens that the macro expands to.
+  llvm::Optional findExpansion(const syntax::Token *Spelled) const;
+

I think expansionStartingAt would be a clearer name. Given the current name, I 
would expect to be able to pass any of the tokens within the spelled range


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62954



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

[PATCH] D62954: [Syntax] Add a helper to find expansion by its first spelled token

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

LG but needs tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62954



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

[PATCH] D62956: [clangd] Collect tokens of main files when building the AST

[Sam McCall via Phabricator via cfe-commits]

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

Can we add a test using TestTU that does a very basic verification of 
expanded/spelled tokens (first after preamble, last token in file)?




Comment at: clang-tools-extra/clangd/ClangdUnit.h:140
   std::unique_ptr Action;
+  /// Expanded tokens for the main file. Does not contain tokens for the file
+  /// preamble.

Not sure "expanded" is accurate here - it has both.
(And it'd be worth spelling out if expanded tokens or spelled tokens or both 
are missing for the preamble)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62956



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

[PATCH] D62538: [clangd] Add hidden tweaks to dump AST/selection.

[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 with a few NITs




Comment at: clangd/refactor/Tweak.h:104
+  /// Is this a 'hidden' tweak, which are off by default.
+  virtual bool hidden() const { return false; }
 };

I wonder whether this should be a static method. WDYT?

That would allow to even prevent calling `prepare()` on those tweaks.
OTOH, we want `prepare()` should be fast and it shouldn't matter if that's the 
case.



Comment at: clangd/unittests/TweakTests.cpp:19
 #include "llvm/Testing/Support/Error.h"
+#include "gmock/gmock-matchers.h"
 #include "gmock/gmock.h"

NIT: the include is redundant. Maybe remove? (probably added by clangd)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D62538



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

[PATCH] D63473: Support -fclang-abi-compat=8.0 to keep old ABI behavior

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

> The System V i386 bug fix (https://reviews.llvm.org/D59744) makes it 
> impossible

Please refer to the svn revision instead of the code review.

> for 32-bit Linux Chromium to write an assembly function that works with both
>  trunk clang and clang 8.0.0, which makes it difficult to update compilers
>  independent of changing the code (more details:
>  https://bugs.chromium.org/p/chromium/issues/detail?id=974542#c5).

We still don't understand the Chromium problem so we should probably wait until 
that's done, and then the commit message should be more general. (For Chromium, 
we will not want to lock to an old ABI version but rather do whatever fixing is 
required.)

> This patch aims to provide a solution for such situation.




Comment at: docs/ReleaseNotes.rst:146
+- The System V i386 psABI requires __m64 to be passed in MMX registers.
+  Clang historically had a bug where it failed to apply this rule, and
+  some platforms (e.g. Darwin, PS4, and FreeBSD) have opted to maintain

Instead of "historically", please spell out exactly what versions, i.e. 
versions prior to Clang 9.



Comment at: docs/ReleaseNotes.rst:151
+  NetBSD). You can switch back to old API behavior with flag:
+  -fclang-abi-compat=8.0.
+

Just refer to the major version: ``-fclang-abi-compat=8``



Comment at: include/clang/Basic/LangOptions.h:142
+/// Attempt to be ABI-compatible with code generated by Clang 8.0.x
+/// (SVN r363116). This causes __m64 to be passed in MMX register 
+/// instead of integer register.

Isn't the comment backwards? Setting Ver8 causes __m64 *not* to be passed in an 
MMX register?



Comment at: lib/CodeGen/TargetInfo.cpp:1091
   bool isPassInMMXRegABI() const {
+// Clang <= 8.0 did not do this for compatiblity with old behavior.
+if (getContext().getLangOpts().getClangABICompat() <=

What about Clang 8.0.1? :-) Probably better to say "Clang 8 and previous 
versions did not do this."



Comment at: test/CodeGen/x86_32-m64.c:6
 // RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-win32 -emit-llvm -o - %s | 
FileCheck %s --check-prefixes=CHECK,WIN32
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-linux-gnu -target-cpu 
pentium4 -emit-llvm -fclang-abi-compat=8.0 -o - %s | FileCheck %s 
--check-prefixes=CHECK,OLDABI
 

RKSimon wrote:
> Use ABI80 instead of OLDABI?
Please pass -fclang-abi-compat=8 instead to match how the flag is used 
otherwhere.


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

https://reviews.llvm.org/D63473



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

[PATCH] D63479: Added SemanticSymbolASTCollector for collecting semantic symbolsCurrently collects variable and function declarations

[Johan Vikström via Phabricator via cfe-commits]

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

[clangd] Added SemanticSymbolASTCollector that collects variable and function 
declarations


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63479

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/SemanticSymbolASTCollector.cpp
  clang-tools-extra/clangd/SemanticSymbolASTCollector.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/SemanticSymbolASTCollectorTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticSymbolASTCollectorTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/SemanticSymbolASTCollectorTests.cpp
@@ -0,0 +1,64 @@
+//===-- SemanticSymbolASTCollectorTests.cpp - SemanticSymbolASTCollector tests
+//--*- 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
+//
+//===--===//
+
+#include "Annotations.h"
+#include "ClangdUnit.h"
+#include "Protocol.h"
+#include "SemanticSymbolASTCollector.h"
+#include "SourceCode.h"
+#include "TestTU.h"
+#include "llvm/Support/ScopedPrinter.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+using ::testing::ElementsAreArray;
+
+Position createPosition(int Line, int Character) {
+  Position Pos;
+  Pos.character = Character;
+  Pos.line = Line;
+  return Pos;
+}
+
+TEST(SemanticSymbolASTCollector, GetBeginningOfIdentifier) {
+  std::string Preamble = R"cpp(
+struct A {
+  double SomeMember;
+};
+void foo(int a) {
+  auto VeryLongVariableName = 12312;
+  A aa;
+}
+  )cpp";
+
+  Annotations TestCase(Preamble);
+  std::vector CorrectSymbols = std::vector{
+  SemanticSymbol(SemanticScope::FunctionDeclaration, createPosition(4, 9),
+ 3),
+  SemanticSymbol(SemanticScope::VariableDeclaration, createPosition(4, 17),
+ 1),
+  SemanticSymbol(SemanticScope::VariableDeclaration, createPosition(5, 11),
+ 20),
+  SemanticSymbol(SemanticScope::VariableDeclaration, createPosition(6, 12),
+ 2)};
+
+  auto AST = TestTU::withCode(TestCase.code()).build();
+  SemanticSymbolASTCollector Collector(AST.getASTContext());
+  Collector.TraverseAST(AST.getASTContext());
+  auto Symbols = Collector.getSymbols();
+  EXPECT_THAT(Symbols, ElementsAreArray(CorrectSymbols));
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -53,6 +53,7 @@
   RenameTests.cpp
   RIFFTests.cpp
   SelectionTests.cpp
+  SemanticSymbolASTCollectorTests.cpp
   SerializationTests.cpp
   SourceCodeTests.cpp
   SymbolCollectorTests.cpp
Index: clang-tools-extra/clangd/SemanticSymbolASTCollector.h
===
--- /dev/null
+++ clang-tools-extra/clangd/SemanticSymbolASTCollector.h
@@ -0,0 +1,91 @@
+//===--- SemanticSymbolASTCollector.h - Manipulating source code as strings
+//-*- 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
+//
+//===--===//
+//
+// Code for collecting semantic symbols from the C++ AST using the
+// RecursiveASTVisitor
+//
+//===--===//
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SEMANTICSYMBOLASTCOLLECTOR_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SEMANTICSYMBOLASTCOLLECTOR_H
+
+#include "AST.h"
+#include "Headers.h"
+#include "Protocol.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Lex/Lexer.h"
+
+namespace clang {
+namespace clangd {
+
+// ScopeIndex represents the mapping from the scopes list to a type of
+// expression
+enum class SemanticScope : int {
+  VariableDeclaration = 0,
+  FunctionDeclaration = 1,
+};
+
+// Holds all information needed for the highlighting
+struct SemanticSymbol {
+  SemanticSymbol() {}
+  SemanticSymbol(SemanticScope Scope, Position StartPosition, unsigned int Len)
+  : Scope(Scope), StartPosition(StartPosition), Len(Len) {}
+  SemanticScope Scope;
+  Position StartPosition;
+  unsigned int Len;
+};
+
+bool operator==(const SemanticSymbol &Lhs, const SemanticSymb

[PATCH] D63479: Added SemanticSymbolASTCollector for collecting semantic symbolsCurrently collects variable and function declarations

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom updated this revision to Diff 205275.
jvikstrom added a comment.

Added dots at the end of comments

Updating D63479: Added SemanticSymbolASTCollector for collecting semantic 
symbols
=

Currently collects variable and function declarations


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63479

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/SemanticSymbolASTCollector.cpp
  clang-tools-extra/clangd/SemanticSymbolASTCollector.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/SemanticSymbolASTCollectorTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticSymbolASTCollectorTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/SemanticSymbolASTCollectorTests.cpp
@@ -0,0 +1,64 @@
+//===-- SemanticSymbolASTCollectorTests.cpp - SemanticSymbolASTCollector tests
+//--*- 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
+//
+//===--===//
+
+#include "Annotations.h"
+#include "ClangdUnit.h"
+#include "Protocol.h"
+#include "SemanticSymbolASTCollector.h"
+#include "SourceCode.h"
+#include "TestTU.h"
+#include "llvm/Support/ScopedPrinter.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+using ::testing::ElementsAreArray;
+
+Position createPosition(int Line, int Character) {
+  Position Pos;
+  Pos.character = Character;
+  Pos.line = Line;
+  return Pos;
+}
+
+TEST(SemanticSymbolASTCollector, GetBeginningOfIdentifier) {
+  std::string Preamble = R"cpp(
+struct A {
+  double SomeMember;
+};
+void foo(int a) {
+  auto VeryLongVariableName = 12312;
+  A aa;
+}
+  )cpp";
+
+  Annotations TestCase(Preamble);
+  std::vector CorrectSymbols = std::vector{
+  SemanticSymbol(SemanticScope::FunctionDeclaration, createPosition(4, 9),
+ 3),
+  SemanticSymbol(SemanticScope::VariableDeclaration, createPosition(4, 17),
+ 1),
+  SemanticSymbol(SemanticScope::VariableDeclaration, createPosition(5, 11),
+ 20),
+  SemanticSymbol(SemanticScope::VariableDeclaration, createPosition(6, 12),
+ 2)};
+
+  auto AST = TestTU::withCode(TestCase.code()).build();
+  SemanticSymbolASTCollector Collector(AST.getASTContext());
+  Collector.TraverseAST(AST.getASTContext());
+  auto Symbols = Collector.getSymbols();
+  EXPECT_THAT(Symbols, ElementsAreArray(CorrectSymbols));
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -53,6 +53,7 @@
   RenameTests.cpp
   RIFFTests.cpp
   SelectionTests.cpp
+  SemanticSymbolASTCollectorTests.cpp
   SerializationTests.cpp
   SourceCodeTests.cpp
   SymbolCollectorTests.cpp
Index: clang-tools-extra/clangd/SemanticSymbolASTCollector.h
===
--- /dev/null
+++ clang-tools-extra/clangd/SemanticSymbolASTCollector.h
@@ -0,0 +1,91 @@
+//===--- SemanticSymbolASTCollector.h - Manipulating source code as strings
+//-*- 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
+//
+//===--===//
+//
+// Code for collecting semantic symbols from the C++ AST using the
+// RecursiveASTVisitor
+//
+//===--===//
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SEMANTICSYMBOLASTCOLLECTOR_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SEMANTICSYMBOLASTCOLLECTOR_H
+
+#include "AST.h"
+#include "Headers.h"
+#include "Protocol.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Lex/Lexer.h"
+
+namespace clang {
+namespace clangd {
+
+// ScopeIndex represents the mapping from the scopes list to a type of
+// expression.
+enum class SemanticScope : int {
+  VariableDeclaration = 0,
+  FunctionDeclaration = 1,
+};
+
+// Holds all information needed for the highlighting.
+struct SemanticSymbol {
+  SemanticSymbol() {}
+  SemanticSymbol(SemanticScope Scope, Position StartPosition, unsigned int Len)
+  : Scope(Scope), StartPosition(StartPosition), Len(Len) {}
+  SemanticScope Scope;
+ 

[PATCH] D63481: [clangd] Parse files without extensions if we don't have a compile command.

[Haojian Wu via Phabricator via cfe-commits]

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

This would enable clangd for C++ standard library files.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63481

Files:
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp


Index: clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
===
--- clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
+++ clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
@@ -36,6 +36,10 @@
   EXPECT_THAT(Cmd.CommandLine,
   ElementsAre(EndsWith("clang"), "-xobjective-c++-header",
   testPath("foo/bar.h")));
+  Cmd = DB.getFallbackCommand(testPath("foo/bar"));
+  EXPECT_THAT(Cmd.CommandLine,
+  ElementsAre(EndsWith("clang"), "-xobjective-c++-header",
+  testPath("foo/bar")));
 }
 
 static tooling::CompileCommand cmd(llvm::StringRef File, llvm::StringRef Arg) {
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -58,9 +58,11 @@
 tooling::CompileCommand
 GlobalCompilationDatabase::getFallbackCommand(PathRef File) const {
   std::vector Argv = {getFallbackClangPath()};
-  // Clang treats .h files as C by default, resulting in unhelpful diagnostics.
+  // Clang treats .h files as C by default and files without extension as 
linker
+  // input, resulting in unhelpful diagnostics.
   // Parsing as Objective C++ is friendly to more cases.
-  if (llvm::sys::path::extension(File) == ".h")
+  auto FileExtension = llvm::sys::path::extension(File);
+  if (FileExtension.empty() || FileExtension == ".h")
 Argv.push_back("-xobjective-c++-header");
   Argv.push_back(File);
   tooling::CompileCommand Cmd(llvm::sys::path::parent_path(File),


Index: clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
===
--- clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
+++ clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
@@ -36,6 +36,10 @@
   EXPECT_THAT(Cmd.CommandLine,
   ElementsAre(EndsWith("clang"), "-xobjective-c++-header",
   testPath("foo/bar.h")));
+  Cmd = DB.getFallbackCommand(testPath("foo/bar"));
+  EXPECT_THAT(Cmd.CommandLine,
+  ElementsAre(EndsWith("clang"), "-xobjective-c++-header",
+  testPath("foo/bar")));
 }
 
 static tooling::CompileCommand cmd(llvm::StringRef File, llvm::StringRef Arg) {
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -58,9 +58,11 @@
 tooling::CompileCommand
 GlobalCompilationDatabase::getFallbackCommand(PathRef File) const {
   std::vector Argv = {getFallbackClangPath()};
-  // Clang treats .h files as C by default, resulting in unhelpful diagnostics.
+  // Clang treats .h files as C by default and files without extension as linker
+  // input, resulting in unhelpful diagnostics.
   // Parsing as Objective C++ is friendly to more cases.
-  if (llvm::sys::path::extension(File) == ".h")
+  auto FileExtension = llvm::sys::path::extension(File);
+  if (FileExtension.empty() || FileExtension == ".h")
 Argv.push_back("-xobjective-c++-header");
   Argv.push_back(File);
   tooling::CompileCommand Cmd(llvm::sys::path::parent_path(File),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan created this revision.
yvvan added reviewers: gribozavr, nik.
Herald added a subscriber: xazax.hun.

Currently this check generates the replacement with the newline in the end. The 
proper way to export it to YAML is to have two \n\n instead of one.

Without this fix clients should reinterpret the replacement as "#include 
 " instead of "#include \n"


https://reviews.llvm.org/D63482

Files:
  clang/include/clang/Tooling/ReplacementsYaml.h


Index: clang/include/clang/Tooling/ReplacementsYaml.h
===
--- clang/include/clang/Tooling/ReplacementsYaml.h
+++ clang/include/clang/Tooling/ReplacementsYaml.h
@@ -35,7 +35,15 @@
 
 NormalizedReplacement(const IO &, const clang::tooling::Replacement &R)
 : FilePath(R.getFilePath()), Offset(R.getOffset()),
-  Length(R.getLength()), ReplacementText(R.getReplacementText()) {}
+  Length(R.getLength()), ReplacementText(R.getReplacementText()) {
+  size_t lineBreakPos = ReplacementText.find('\n');
+  while (lineBreakPos != std::string::npos) {
+// Replace this occurrence of Sub String
+ReplacementText.replace(lineBreakPos, 1, "\n\n");
+// Get the next occurrence from the current position
+lineBreakPos = ReplacementText.find('\n', lineBreakPos + 2);
+  }
+}
 
 clang::tooling::Replacement denormalize(const IO &) {
   return clang::tooling::Replacement(FilePath, Offset, Length,


Index: clang/include/clang/Tooling/ReplacementsYaml.h
===
--- clang/include/clang/Tooling/ReplacementsYaml.h
+++ clang/include/clang/Tooling/ReplacementsYaml.h
@@ -35,7 +35,15 @@
 
 NormalizedReplacement(const IO &, const clang::tooling::Replacement &R)
 : FilePath(R.getFilePath()), Offset(R.getOffset()),
-  Length(R.getLength()), ReplacementText(R.getReplacementText()) {}
+  Length(R.getLength()), ReplacementText(R.getReplacementText()) {
+  size_t lineBreakPos = ReplacementText.find('\n');
+  while (lineBreakPos != std::string::npos) {
+// Replace this occurrence of Sub String
+ReplacementText.replace(lineBreakPos, 1, "\n\n");
+// Get the next occurrence from the current position
+lineBreakPos = ReplacementText.find('\n', lineBreakPos + 2);
+  }
+}
 
 clang::tooling::Replacement denormalize(const IO &) {
   return clang::tooling::Replacement(FilePath, Offset, Length,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63139: [Diagnostics] Implement -Wswitch-unreachable

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

xbolva00 added a comment.

There is a bug in Clang AST. @rsmith

Check testcase test/Sema/switch-1.c. In C++ mode we have AST:
CompoundStmt 0x55f5b7d68460 

|  |   | -ReturnStmt 
0x55f5b7d68060|
|  |   | `-IntegerLiteral 
0x55f5b7d68040  'int' 1 |
|  |   | -ReturnStmt 
0x55f5b7d68120|
|  |   | `-IntegerLiteral 
0x55f5b7d68100  'int' 2 |
|  |   | -ReturnStmt 
0x55f5b7d68210|
|  |   | `-IntegerLiteral 
0x55f5b7d681f0  'int' 3 |
|  |   | -ReturnStmt 
0x55f5b7d683c0|
|  |   | `-IntegerLiteral 
0x55f5b7d683a0  'int' 4 |
|  | `-CaseStmt 0x55f5b7d68408  |
|  |   | -ConstantExpr 
0x55f5b7d683f0  'int'  |
|  |   | `-IntegerLiteral 
0x55f5b7d683d0  'int' 2147483647 |
|  | `-ReturnStmt 0x55f5b7d68450|
|  | `-IntegerLiteral 0x55f5b7d68430  'int' 0  |
|

This is bad and blocks this patch since it creates weird warnings :/


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

https://reviews.llvm.org/D63139



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

[PATCH] D63256: [OpenCL] Split type and macro definitions into opencl-c-base.h

[Andrew Savonichev via Phabricator via cfe-commits]

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

Thanks. LGTM.


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

https://reviews.llvm.org/D63256



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

[PATCH] D63139: [Diagnostics] Implement -Wswitch-unreachable

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

xbolva00 updated this revision to Diff 205295.
xbolva00 added a comment.

Addressed some notes. Thanks.


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

https://reviews.llvm.org/D63139

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaStmt.cpp
  test/Sema/implicit-decl-c90.c
  test/SemaCXX/array-bounds.cpp
  test/SemaCXX/warn-unreachable-stmt-switch.cpp

Index: test/SemaCXX/warn-unreachable-stmt-switch.cpp
===
--- test/SemaCXX/warn-unreachable-stmt-switch.cpp
+++ test/SemaCXX/warn-unreachable-stmt-switch.cpp
@@ -0,0 +1,152 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wswitch-unreachable %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wswitch-unreachable %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify  %s
+
+void g(int x);
+
+void foo(int x) {
+  int b = 0;
+  switch (x) {
+  label:
+  case 4:
+break;
+  default:
+return;
+  }
+
+  switch (x) {
+x++; // expected-warning {{statement will be never executed}}
+  label2:
+  case 4:
+break;
+  default:
+return;
+  }
+
+  switch (x) {
+  label3:
+x++;
+  case 4:
+break;
+  default:
+return;
+  }
+
+  switch (x) {
+  case 4:
+return;
+  }
+
+  switch (x) {
+b = x; // expected-warning {{statement will be never executed}}
+  case 7:
+g(b);
+break;
+  }
+
+  switch (x) {
+;
+  case 7:
+g(b);
+break;
+  }
+
+  switch (x) {
+break; // expected-warning {{statement will be never executed}}
+  case 7:
+break;
+  }
+
+  switch (x) {
+return; // expected-warning {{statement will be never executed}}
+  case 7:
+break;
+  }
+
+  switch (x) {
+x++;  // expected-warning {{statement will be never executed}}
+g(x); // expected-warning {{statement will be never executed}}
+  case 7:
+break;
+  }
+
+  switch (x) {
+x++; // expected-warning {{statement will be never executed}}
+  label4:
+g(x);
+  case 7:
+break;
+  }
+
+  switch (x) {
+g(x); // expected-warning {{statement will be never executed}}
+  case 1:
+break;
+  case 2:
+break;
+  }
+
+  switch (x) {
+g(x); // expected-warning {{statement will be never executed}}
+  case 1:
+  case 2:
+  case 3:
+break;
+  }
+
+  switch (x) {
+  case 1:
+break;
+  case 2:
+break;
+  }
+
+  switch (x) {
+  case 1:
+break;
+  default:
+break;
+  }
+
+  switch (x) {
+b = x; // expected-warning {{statement will be never executed}}
+  }
+
+  switch (x) {
+g(x); // expected-warning {{statement will be never executed}}
+  }
+
+  switch (x)
+b = x; // expected-warning {{statement will be never executed}}
+
+  switch (x)
+g(x); // expected-warning {{statement will be never executed}}
+
+  switch (x)
+  label5:
+g(x);
+
+  switch (x) {
+  label6:
+g(x);
+  }
+
+  switch (x)
+  case 5:
+g(x);
+
+  switch (x) {
+  case 5:
+g(x);
+  }
+
+  switch (x)
+  default:
+g(x);
+
+  switch (x) {
+  default:
+g(x);
+  }
+}
Index: test/SemaCXX/array-bounds.cpp
===
--- test/SemaCXX/array-bounds.cpp
+++ test/SemaCXX/array-bounds.cpp
@@ -164,6 +164,7 @@
 static enum enumB myVal = enumB_X;
 void test_nested_switch() {
   switch (enumA_E) { // expected-warning {{no case matching constant}}
+// expected-warning@+1 {{statement will be never executed}}
 switch (myVal) { // expected-warning {{enumeration values 'enumB_X' and 'enumB_Z' not handled in switch}}
   case enumB_Y: ;
 }
Index: test/Sema/implicit-decl-c90.c
===
--- test/Sema/implicit-decl-c90.c
+++ test/Sema/implicit-decl-c90.c
@@ -15,7 +15,7 @@
 void t1(int x) {
   int (*p)();
   switch (x) {
-g();
+g(); // expected-warning {{statement will be never executed}}
   case 0:
 x = h() + 1;
 break;
Index: lib/Sema/SemaStmt.cpp
===
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -723,7 +723,8 @@
 /// Check the specified case value is in range for the given unpromoted switch
 /// type.
 static void checkCaseValue(Sema &S, SourceLocation Loc, const llvm::APSInt &Val,
-   unsigned UnpromotedWidth, bool UnpromotedSign) {
+   unsigned UnpromotedWidth, bool UnpromotedSign,
+   bool &CaseListIsErroneous) {
   // In C++11 onwards, this is checked by the language rules.
   if (S.getLangOpts().CPlusPlus11)
 return;
@@ -738,9 +739,11 @@
 // FIXME: Use different diagnostics for overflow  in conversion to promoted
 // type versus "switch expression cannot have this value". Use proper
 // IntRange checking rather than just looking at the unpromoted type here.
-if (ConvVal != Val)
-  S.Diag(Loc, diag::warn_case_value_overflow) << Val.toString(10)
-

[PATCH] D63331: [clangd] WIP/RFC: Prototype for semantic highlighting proposal

[Nikolai Kosjar via Phabricator via cfe-commits]

nik added a comment.

In D63331#1543441 , @hokein wrote:

> Yeah, we definitely have interest in this feature, and our intern @jvikstrom 
> will work on this feature this summer.


"this summer" depends on the location, so can you concretize this? :)

> You probably missed the discussion 
>  on 
> clangd-dev mailing list, @nridge also has an unfinished prototype 
>  for semantic highlighting.

Yes, I wasn't aware that there is a clangd-dev mailing list and thus was not 
subscribed to it. Subscribed now.

I'm glad that I haven't put more work into this ;)

> To avoid multiple people working on the same feature, I think the plan is 
> that @jvikstrom will pick up the current stuff, and continue 
> working/improving it.

Great. The patch from @nridge is more advanced so I'll abandon this one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63331



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

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev updated this revision to Diff 205297.
anton-afanasyev marked 2 inline comments as done.
anton-afanasyev added a comment.

Updated


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63325

Files:
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Parse/ParseAST.cpp
  clang/test/Driver/check-time-trace-sections.cpp
  clang/test/Driver/check-time-trace-sections.py
  llvm/lib/Support/TimeProfiler.cpp


Index: llvm/lib/Support/TimeProfiler.cpp
===
--- llvm/lib/Support/TimeProfiler.cpp
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -65,7 +65,7 @@
 E.Duration = steady_clock::now() - E.Start;
 
 // Only include sections longer than TimeTraceGranularity msec.
-if (duration_cast(E.Duration).count() > TimeTraceGranularity)
+if (duration_cast(E.Duration).count() >= 
TimeTraceGranularity)
   Entries.emplace_back(E);
 
 // Track total time taken by each "name", but only the topmost levels of
Index: clang/test/Driver/check-time-trace-sections.py
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-sections.py
@@ -0,0 +1,20 @@
+#!/usr/bin/env python
+
+import json, sys
+
+def is_inside(range1, range2):
+a = range1["ts"]; b = a + range1["dur"]
+c = range2["ts"]; d = c + range2["dur"]
+return (a >= c and a <= d) and (b >= c and b <= d)
+
+events = json.loads(sys.stdin.read())["traceEvents"]
+codegens = filter(lambda x: x["name"] == "CodeGen Function", events)
+frontends = filter(lambda x: x["name"] == "Frontend", events)
+
+if not len(frontends) == 1:
+sys.exit("There should be exactly one Frontend section!")
+
+frontend = frontends[0]
+
+if not all([is_inside(codegen, frontend) for codegen in codegens]):
+sys.exit("Not all CodeGen sections are inside Frontend section!")
Index: clang/test/Driver/check-time-trace-sections.cpp
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-sections.cpp
@@ -0,0 +1,7 @@
+// REQUIRES: shell
+// RUN: %clangxx -S -ftime-trace -mllvm --time-trace-granularity=0 -o 
%T/check-time-trace-sections %s
+// RUN: cat %T/check-time-trace-sections.json | %python 
%S/check-time-trace-sections.py
+
+template 
+void foo(T) {}
+void bar() { foo(0); }
Index: clang/lib/Parse/ParseAST.cpp
===
--- clang/lib/Parse/ParseAST.cpp
+++ clang/lib/Parse/ParseAST.cpp
@@ -150,8 +150,11 @@
   // after the pragma, there won't be any tokens or a Lexer.
   bool HaveLexer = S.getPreprocessor().getCurrentLexer();
 
+  // Start "Frontend" section finishing inside clang::HandleTranslationUnit()
+  if (llvm::timeTraceProfilerEnabled())
+llvm::timeTraceProfilerBegin("Frontend", StringRef(""));
+
   if (HaveLexer) {
-llvm::TimeTraceScope TimeScope("Frontend", StringRef(""));
 P.Initialize();
 Parser::DeclGroupPtrTy ADecl;
 for (bool AtEOF = P.ParseFirstTopLevelDecl(ADecl); !AtEOF;
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -37,6 +37,7 @@
 #include "llvm/Pass.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/Timer.h"
 #include "llvm/Support/ToolOutputFile.h"
 #include "llvm/Support/YAMLTraits.h"
@@ -246,6 +247,10 @@
 IRGenFinished = true;
   }
 
+  // Finish "Frontend" section starting inside clang::ParseAST()
+  if (llvm::timeTraceProfilerEnabled())
+llvm::timeTraceProfilerEnd();
+
   // Silently ignore if we weren't initialized for some reason.
   if (!getModule())
 return;


Index: llvm/lib/Support/TimeProfiler.cpp
===
--- llvm/lib/Support/TimeProfiler.cpp
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -65,7 +65,7 @@
 E.Duration = steady_clock::now() - E.Start;
 
 // Only include sections longer than TimeTraceGranularity msec.
-if (duration_cast(E.Duration).count() > TimeTraceGranularity)
+if (duration_cast(E.Duration).count() >= TimeTraceGranularity)
   Entries.emplace_back(E);
 
 // Track total time taken by each "name", but only the topmost levels of
Index: clang/test/Driver/check-time-trace-sections.py
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-sections.py
@@ -0,0 +1,20 @@
+#!/usr/bin/env python
+
+import json, sys
+
+def is_inside(range1, range2):
+a = range1["ts"]; b = a + range1["dur"]
+c = range2["ts"]; d = c + range2["dur"]
+return (a >= c and a <= d) and (b >= c and b <= d)
+
+events = json.loads(sys.stdin.read())["traceEvents"]
+codegens = filter(lambda x:

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added inline comments.



Comment at: clang/lib/CodeGen/CodeGenAction.cpp:232
   {
+llvm::TimeTraceScope TimeScope("Frontend", StringRef(""));
 PrettyStackTraceString CrashInfo("Per-file LLVM IR generation");

lebedev.ri wrote:
> This looks more like `Frontend Codegen` to me?
> (With front-end itself being the entirety of clang time up to this point 
> including this step)
`Codegen` sections are not inside `HandleTranslationUnit()` function only. So 
I've changed it other way: started `Frontend` section inside `ParseAST()` and 
finished this section inside `HandleTranslationUnit()`. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63325



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

[PATCH] Add support for openSUSE RISC-V triple

[Andreas Schwab via cfe-commits]

---
 clang/lib/Driver/ToolChains/Gnu.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp 
b/clang/lib/Driver/ToolChains/Gnu.cpp
index c8520968e45..64f4f9e1e26 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2021,7 +2021,8 @@ void 
Generic_GCC::GCCInstallationDetector::AddDefaultGCCPrefixes(
   static const char *const RISCV64LibDirs[] = {"/lib64", "/lib"};
   static const char *const RISCV64Triples[] = {"riscv64-unknown-linux-gnu",
"riscv64-linux-gnu",
-   "riscv64-unknown-elf"};
+   "riscv64-unknown-elf",
+   "riscv64-suse-linux"};
 
   static const char *const SPARCv8LibDirs[] = {"/lib32", "/lib"};
   static const char *const SPARCv8Triples[] = {"sparc-linux-gnu",
-- 
2.22.0


-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] Add support for openSUSE RISC-V triple

[Roman Lebedev via cfe-commits]

Missing a test, and if this is a patch and not a commit you want to
submit it via https://llvm.org/docs/Phabricator.html

On Tue, Jun 18, 2019 at 2:20 PM Andreas Schwab via cfe-commits
 wrote:
>
> ---
>  clang/lib/Driver/ToolChains/Gnu.cpp | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp 
> b/clang/lib/Driver/ToolChains/Gnu.cpp
> index c8520968e45..64f4f9e1e26 100644
> --- a/clang/lib/Driver/ToolChains/Gnu.cpp
> +++ b/clang/lib/Driver/ToolChains/Gnu.cpp
> @@ -2021,7 +2021,8 @@ void 
> Generic_GCC::GCCInstallationDetector::AddDefaultGCCPrefixes(
>static const char *const RISCV64LibDirs[] = {"/lib64", "/lib"};
>static const char *const RISCV64Triples[] = {"riscv64-unknown-linux-gnu",
> "riscv64-linux-gnu",
> -   "riscv64-unknown-elf"};
> +   "riscv64-unknown-elf",
> +   "riscv64-suse-linux"};
>
>static const char *const SPARCv8LibDirs[] = {"/lib32", "/lib"};
>static const char *const SPARCv8Triples[] = {"sparc-linux-gnu",
> --
> 2.22.0
>
>
> --
> Andreas Schwab, SUSE Labs, sch...@suse.de
> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
> "And now for something completely different."
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63299: [Clang] Parse GNU fallthrough attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/Parse/ParseStmt.cpp:102
   ParsedAttributesWithRange Attrs(AttrFactory);
+  MaybeParseGNUAttributes(Attrs);
   MaybeParseCXX11Attributes(Attrs, nullptr, /*MightBeObjCMessageSend*/ true);

xbolva00 wrote:
> xbolva00 wrote:
> > Maybe we check if Tok is kw__attribute and look ahead a few tokens to check 
> > if attribute name is fallthough in 
> > ParseStatementOrDeclarationAfterAttributes.
> > 
> > Now, we always fall into
> > 
> > if ((getLangOpts().CPlusPlus || getLangOpts().MicrosoftExt ||
> >  (StmtCtx & ParsedStmtContext::AllowDeclarationsInC) !=
> >  ParsedStmtContext()) &&
> > isDeclarationStatement()) {
> >   SourceLocation DeclStart = Tok.getLocation(), DeclEnd;
> >   DeclGroupPtrTy Decl = 
> > ParseDeclaration(DeclaratorContext::BlockContext,
> >  DeclEnd, Attrs);
> >   return Actions.ActOnDeclStmt(Decl, DeclStart, DeclEnd);
> >   }
> Then we go to "ParseSimpleDeclaration" -> "ParseDeclarationSpecifier". What 
> is quite strange for me, we do not set "Attrs" in ParseSimpleDeclaration from 
> DS.getAttributes()..
> Maybe we check if Tok is kw__attribute and look ahead a few tokens to check 
> if attribute name is fallthough in ParseStatementOrDeclarationAfterAttributes.

Why not check whether the attribute is a statement attribute or a declaration 
attribute, rather than tying it to fallthrough specifically?

But the result then would be: if it's a statement attribute, we should be 
trying to parse it as a statement rather than declaration statement; if it's a 
declaration attribute, we should try to parse as a declaration. This seems very 
much like spooky action at a distance -- attributes should not dictate whether 
something is parsed as a decl or a stmt.

That said, this really is the right place for that code to go, I think. We've 
never had a GNU null attribute statement before, so the parser may be written 
in an inconvenient way to support that (IIRC, we have similar deficiencies with 
things like attributes appertaining to declaration specifiers -- we don't have 
attributes that need it yet, so support was never added for it).

This may require a bit more surgery to fix up, unfortunately.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63299



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

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 205298.
serge-sans-paille added a comment.

Mostly documentation update + helper function renaming.

@meinersbur, I've tested the setup with static and dynamic builds on Linux, 
please let me know your thoughts on this.


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

https://reviews.llvm.org/D61446

Files:
  clang/tools/driver/CMakeLists.txt
  clang/tools/driver/cc1_main.cpp
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/docs/WritingAnLLVMPass.rst
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/tools/CMakeLists.txt
  llvm/tools/bugpoint/CMakeLists.txt
  llvm/tools/bugpoint/bugpoint.cpp
  llvm/tools/opt/CMakeLists.txt
  llvm/tools/opt/NewPMDriver.cpp
  llvm/tools/opt/opt.cpp
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  polly/include/polly/RegisterPasses.h
  polly/lib/CMakeLists.txt
  polly/lib/Polly.cpp
  polly/lib/Support/RegisterPasses.cpp
  polly/test/Unit/lit.site.cfg.in
  polly/test/lit.site.cfg.in
  polly/test/update_check.py

Index: polly/test/update_check.py
===
--- polly/test/update_check.py
+++ polly/test/update_check.py
@@ -15,7 +15,7 @@
 polly_lib_dir = '''@POLLY_LIB_DIR@'''
 shlibext = '''@LLVM_SHLIBEXT@'''
 llvm_tools_dir = '''@LLVM_TOOLS_DIR@'''
-link_polly_into_tools = not '''@LINK_POLLY_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','link_polly_into_tools-notfound'}
+llvm_polly_link_into_tools = not '''@LLVM_POLLY_LINK_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','llvm_polly_link_into_tools-notfound'}
 
 runre = re.compile(r'\s*\;\s*RUN\s*\:(?P.*)')
 filecheckre = re.compile(r'\s*(?P.*)\|\s*(?PFileCheck\s[^|]*)')
@@ -298,7 +298,7 @@
 toolarg = toolarg.replace('%s', filename)
 toolarg = toolarg.replace('%S', os.path.dirname(filename))
 if toolarg == '%loadPolly':
-if not link_polly_into_tools:
+if not llvm_polly_link_into_tools:
 newtool += ['-load',os.path.join(polly_lib_dir,'LLVMPolly' + shlibext)]
 newtool.append('-polly-process-unprofitable')
 newtool.append('-polly-remarks-minimal')
Index: polly/test/lit.site.cfg.in
===
--- polly/test/lit.site.cfg.in
+++ polly/test/lit.site.cfg.in
@@ -8,7 +8,7 @@
 config.polly_lib_dir = "@POLLY_LIB_DIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.targets_to_build = "@TARGETS_TO_BUILD@"
 config.extra_paths = "@POLLY_TEST_EXTRA_PATHS@".split(";")
 
@@ -36,14 +36,14 @@
 # directories.
 config.excludes = ['Inputs']
 
-if config.link_polly_into_tools == '' or \
-   config.link_polly_into_tools.lower() == '0' or \
-   config.link_polly_into_tools.lower() == 'n' or \
-   config.link_polly_into_tools.lower() == 'no' or \
-   config.link_polly_into_tools.lower() == 'off' or \
-   config.link_polly_into_tools.lower() == 'false' or \
-   config.link_polly_into_tools.lower() == 'notfound' or \
-   config.link_polly_into_tools.lower() == 'link_polly_into_tools-notfound':
+if config.llvm_polly_link_into_tools == '' or \
+   config.llvm_polly_link_into_tools.lower() == '0' or \
+   config.llvm_polly_link_into_tools.lower() == 'n' or \
+   config.llvm_polly_link_into_tools.lower() == 'no' or \
+   config.llvm_polly_link_into_tools.lower() == 'off' or \
+   config.llvm_polly_link_into_tools.lower() == 'false' or \
+   config.llvm_polly_link_into_tools.lower() == 'notfound' or \
+   config.llvm_polly_link_into_tools.lower() == 'llvm_polly_link_into_tools-notfound':
 config.substitutions.append(('%loadPolly', '-load '
  + config.polly_lib_dir + '/LLVMPolly@LLVM_SHLIBEXT@'
  + ' -load-pass-plugin '
Index: polly/test/Unit/lit.site.cfg.in
===
--- polly/test/Unit/lit.site.cfg.in
+++ polly/test/Unit/lit.site.cfg.in
@@ -13,7 +13,7 @@
 config.shlibdir = "@SHLIBDIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.has_unittests = @POLLY_GTEST_AVAIL@
 
 # Support substitution of the tools_dir, libs_dirs, and build_mode with user
Index: polly/lib/Support/RegisterPasses.cpp
===
--- polly/lib/Support/RegisterPasses.cpp
+++ polly/lib/Support/RegisterPasses.cpp
@@ -701,9 +701,24 @@
 }
 } // namespace polly
 
-// Plugin Entrypoint:
-extern "C" ::llvm::PassPluginLibraryInfo LLVM_ATTRIBUTE_WEAK
-llvmGetPassPluginInfo() {
+/// Initialize Polly passes when l

[PATCH] D63483: [clangd] Detect C++ language based on well-known file path in vscode extension

[Haojian Wu via Phabricator via cfe-commits]

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

Matching the "C++" pattern on the first line of the file doesn't cover
all cases, MSVC C++ headers doesn't have such pattern. This patch
introduce a new heuristic to detect language based on the file path.

MSVC C++ standard headers are in the directory like
"c:\Program Files (x86)\Microsoft Visual 
Studio\2017\BuildTools\VC\Tools\MSVC\14.15.26726\include"


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63483

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


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
@@ -52,6 +52,10 @@
 "contributes": {
 "languages": [{
 "id": "cpp",
+"filenamePatterns": [
+"**/include/c++/**",
+"**/MSVC/*/include/**"
+],
 "firstLine": "^\/[/*].*-\\*-\\s*C\\+\\+\\s*-\\*-.*"
 }],
 "configuration": {


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
@@ -52,6 +52,10 @@
 "contributes": {
 "languages": [{
 "id": "cpp",
+"filenamePatterns": [
+"**/include/c++/**",
+"**/MSVC/*/include/**"
+],
 "firstLine": "^\/[/*].*-\\*-\\s*C\\+\\+\\s*-\\*-.*"
 }],
 "configuration": {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63479: Added SemanticSymbolASTCollector for collecting semantic symbolsCurrently collects variable and function declarations

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom updated this revision to Diff 205301.
jvikstrom added a comment.

Moved SemanticSymbolASTVisitor to an anonymous namespace and added a function 
for getting highlights that returns highlights line by line

Updating D63479: Added SemanticSymbolASTCollector for collecting semantic 
symbols
=

Currently collects variable and function declarations


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63479

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/SemanticHighlight.cpp
  clang-tools-extra/clangd/SemanticHighlight.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/SemanticHighlightTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/SemanticHighlightTests.cpp
@@ -0,0 +1,64 @@
+//===-- SemanticSymbolASTCollectorTests.cpp - SemanticSymbolASTCollector tests
+//--*- 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
+//
+//===--===//
+
+#include "Annotations.h"
+#include "ClangdUnit.h"
+#include "Protocol.h"
+#include "SemanticHighlight.h"
+#include "SourceCode.h"
+#include "TestTU.h"
+#include "llvm/Support/ScopedPrinter.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+using ::testing::ElementsAreArray;
+
+Position createPosition(int Line, int Character) {
+  Position Pos;
+  Pos.character = Character;
+  Pos.line = Line;
+  return Pos;
+}
+
+TEST(SemanticSymbolASTCollector, GetBeginningOfIdentifier) {
+  std::string Preamble = R"cpp(
+struct A {
+  double SomeMember;
+};
+void foo(int a) {
+  auto VeryLongVariableName = 12312;
+  A aa;
+}
+  )cpp";
+
+  Annotations TestCase(Preamble);
+  std::vector CorrectLines = std::vector{
+  LineHighlight(4, {SemanticSymbol(SemanticScope::FunctionDeclaration,
+   createPosition(4, 9), 3),
+SemanticSymbol(SemanticScope::VariableDeclaration,
+   createPosition(4, 17), 1)}),
+  LineHighlight(5, {SemanticSymbol(SemanticScope::VariableDeclaration,
+   createPosition(5, 11), 20)}),
+  LineHighlight(6, {SemanticSymbol(SemanticScope::VariableDeclaration,
+   createPosition(6, 12), 2)})
+
+  };
+
+  auto AST = TestTU::withCode(TestCase.code()).build();
+  auto Lines = getASTHighlights(AST.getASTContext());
+  EXPECT_THAT(Lines, ElementsAreArray(CorrectLines));
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -53,6 +53,7 @@
   RenameTests.cpp
   RIFFTests.cpp
   SelectionTests.cpp
+  SemanticHighlightTests.cpp
   SerializationTests.cpp
   SourceCodeTests.cpp
   SymbolCollectorTests.cpp
Index: clang-tools-extra/clangd/SemanticHighlight.h
===
--- /dev/null
+++ clang-tools-extra/clangd/SemanticHighlight.h
@@ -0,0 +1,62 @@
+//===--- SemanticSymbolASTCollector.h - Manipulating source code as strings
+//-*- 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
+//
+//===--===//
+//
+// Code for collecting semantic symbols from the C++ AST using the
+// RecursiveASTVisitor
+//
+//===--===//
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SEMANTICSYMBOLASTCOLLECTOR_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SEMANTICSYMBOLASTCOLLECTOR_H
+
+#include "AST.h"
+#include "Headers.h"
+#include "Protocol.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Lex/Lexer.h"
+
+namespace clang {
+namespace clangd {
+
+// ScopeIndex represents the mapping from the scopes list to a type of
+// expression.
+enum class SemanticScope : int {
+  VariableDeclaration = 0,
+  FunctionDeclaration = 1,
+};
+
+// Contains all information needed for the highlighting a symbol.
+struct SemanticSymbol {
+  SemanticSymbol() {}
+  SemanticSymbol(SemanticScope Scope, Position StartPosition, unsigned int Len)
+

[PATCH] D63260: [Attr] Support _attribute__ ((fallthrough))

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

xbolva00 added a comment.

As noted in D63299 , seems like this would 
require more surgery - I cannot continue D63299 
 since I have no enough knowledge to rework it 
fully.

I will update this patch, in case somebody picks D63299 
.


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

https://reviews.llvm.org/D63260



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

[PATCH] D63139: [Diagnostics] Implement -Wswitch-unreachable

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

xbolva00 added a comment.

In D63139#1547968 , @xbolva00 wrote:

> There is a bug in Clang AST. @rsmith
>
> Check test/Sema/switch-1.c. In C++ mode we have really bad AST:
>
>   CompoundStmt 0x55f5b7d68460 
>   |   |   |-ReturnStmt 0x55f5b7d68060 
>   |   |   | `-IntegerLiteral 0x55f5b7d68040  'int' 1
>   |   |   |-ReturnStmt 0x55f5b7d68120 
>   |   |   | `-IntegerLiteral 0x55f5b7d68100  'int' 2
>   |   |   |-ReturnStmt 0x55f5b7d68210 
>   |   |   | `-IntegerLiteral 0x55f5b7d681f0  'int' 3
>   |   |   |-ReturnStmt 0x55f5b7d683c0 
>   |   |   | `-IntegerLiteral 0x55f5b7d683a0  'int' 4
>   |   |   `-CaseStmt 0x55f5b7d68408 
>   |   | |-ConstantExpr 0x55f5b7d683f0  'int'
>   |   | | `-IntegerLiteral 0x55f5b7d683d0  'int' 2147483647
>   |   | `-ReturnStmt 0x55f5b7d68450 
>   |   |   `-IntegerLiteral 0x55f5b7d68430  'int' 0
>  
>  
>
>
> This is bad and blocks this patch since it creates weird warnings :/


Or... is it acceptable to produce unreachable stmt warning for this cases?


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

https://reviews.llvm.org/D63139



___
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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

Ping.


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

[clang-tools-extra] r363662 - [clangd] Detect C++ language based on well-known file path in vscode extension

[Haojian Wu via cfe-commits]

Author: hokein
Date: Tue Jun 18 04:53:14 2019
New Revision: 363662

URL: http://llvm.org/viewvc/llvm-project?rev=363662&view=rev
Log:
[clangd] Detect C++ language based on well-known file path in vscode extension

Summary:
Matching the "C++" pattern on the first line of the file doesn't cover
all cases, MSVC C++ headers doesn't have such pattern. This patch
introduce a new heuristic to detect language based on the file path.

MSVC C++ standard headers are in the directory like
"c:\Program Files (x86)\Microsoft Visual 
Studio\2017\BuildTools\VC\Tools\MSVC\14.15.26726\include"

Reviewers: sammccall

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json

Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json?rev=363662&r1=363661&r2=363662&view=diff
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json (original)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json Tue Jun 
18 04:53:14 2019
@@ -52,6 +52,10 @@
 "contributes": {
 "languages": [{
 "id": "cpp",
+"filenamePatterns": [
+"**/include/c++/**",
+"**/MSVC/*/include/**"
+],
 "firstLine": "^\/[/*].*-\\*-\\s*C\\+\\+\\s*-\\*-.*"
 }],
 "configuration": {


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

[clang-tools-extra] r363663 - [clangd] Parse files without extensions if we don't have a compile command.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Tue Jun 18 04:54:17 2019
New Revision: 363663

URL: http://llvm.org/viewvc/llvm-project?rev=363663&view=rev
Log:
[clangd] Parse files without extensions if we don't have a compile command.

Summary: This would enable clangd for C++ standard library files.

Reviewers: sammccall

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
clang-tools-extra/trunk/clangd/unittests/GlobalCompilationDatabaseTests.cpp

Modified: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp?rev=363663&r1=363662&r2=363663&view=diff
==
--- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp (original)
+++ clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp Tue Jun 18 
04:54:17 2019
@@ -58,9 +58,11 @@ static std::string getFallbackClangPath(
 tooling::CompileCommand
 GlobalCompilationDatabase::getFallbackCommand(PathRef File) const {
   std::vector Argv = {getFallbackClangPath()};
-  // Clang treats .h files as C by default, resulting in unhelpful diagnostics.
+  // Clang treats .h files as C by default and files without extension as 
linker
+  // input, resulting in unhelpful diagnostics.
   // Parsing as Objective C++ is friendly to more cases.
-  if (llvm::sys::path::extension(File) == ".h")
+  auto FileExtension = llvm::sys::path::extension(File);
+  if (FileExtension.empty() || FileExtension == ".h")
 Argv.push_back("-xobjective-c++-header");
   Argv.push_back(File);
   tooling::CompileCommand Cmd(llvm::sys::path::parent_path(File),

Modified: 
clang-tools-extra/trunk/clangd/unittests/GlobalCompilationDatabaseTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/GlobalCompilationDatabaseTests.cpp?rev=363663&r1=363662&r2=363663&view=diff
==
--- clang-tools-extra/trunk/clangd/unittests/GlobalCompilationDatabaseTests.cpp 
(original)
+++ clang-tools-extra/trunk/clangd/unittests/GlobalCompilationDatabaseTests.cpp 
Tue Jun 18 04:54:17 2019
@@ -36,6 +36,10 @@ TEST(GlobalCompilationDatabaseTest, Fall
   EXPECT_THAT(Cmd.CommandLine,
   ElementsAre(EndsWith("clang"), "-xobjective-c++-header",
   testPath("foo/bar.h")));
+  Cmd = DB.getFallbackCommand(testPath("foo/bar"));
+  EXPECT_THAT(Cmd.CommandLine,
+  ElementsAre(EndsWith("clang"), "-xobjective-c++-header",
+  testPath("foo/bar")));
 }
 
 static tooling::CompileCommand cmd(llvm::StringRef File, llvm::StringRef Arg) {


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

[PATCH] D63483: [clangd] Detect C++ language based on well-known file path in vscode extension

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363662: [clangd] Detect C++ language based on well-known 
file path in vscode extension (authored by hokein, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63483?vs=205300&id=205306#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63483

Files:
  clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json


Index: clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
===
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
@@ -52,6 +52,10 @@
 "contributes": {
 "languages": [{
 "id": "cpp",
+"filenamePatterns": [
+"**/include/c++/**",
+"**/MSVC/*/include/**"
+],
 "firstLine": "^\/[/*].*-\\*-\\s*C\\+\\+\\s*-\\*-.*"
 }],
 "configuration": {


Index: clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
===
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
@@ -52,6 +52,10 @@
 "contributes": {
 "languages": [{
 "id": "cpp",
+"filenamePatterns": [
+"**/include/c++/**",
+"**/MSVC/*/include/**"
+],
 "firstLine": "^\/[/*].*-\\*-\\s*C\\+\\+\\s*-\\*-.*"
 }],
 "configuration": {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63481: [clangd] Parse files without extensions if we don't have a compile command.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363663: [clangd] Parse files without extensions if we 
don't have a compile command. (authored by hokein, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63481?vs=205290&id=205307#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63481

Files:
  clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/trunk/clangd/unittests/GlobalCompilationDatabaseTests.cpp


Index: 
clang-tools-extra/trunk/clangd/unittests/GlobalCompilationDatabaseTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/GlobalCompilationDatabaseTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/GlobalCompilationDatabaseTests.cpp
@@ -36,6 +36,10 @@
   EXPECT_THAT(Cmd.CommandLine,
   ElementsAre(EndsWith("clang"), "-xobjective-c++-header",
   testPath("foo/bar.h")));
+  Cmd = DB.getFallbackCommand(testPath("foo/bar"));
+  EXPECT_THAT(Cmd.CommandLine,
+  ElementsAre(EndsWith("clang"), "-xobjective-c++-header",
+  testPath("foo/bar")));
 }
 
 static tooling::CompileCommand cmd(llvm::StringRef File, llvm::StringRef Arg) {
Index: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
@@ -58,9 +58,11 @@
 tooling::CompileCommand
 GlobalCompilationDatabase::getFallbackCommand(PathRef File) const {
   std::vector Argv = {getFallbackClangPath()};
-  // Clang treats .h files as C by default, resulting in unhelpful diagnostics.
+  // Clang treats .h files as C by default and files without extension as 
linker
+  // input, resulting in unhelpful diagnostics.
   // Parsing as Objective C++ is friendly to more cases.
-  if (llvm::sys::path::extension(File) == ".h")
+  auto FileExtension = llvm::sys::path::extension(File);
+  if (FileExtension.empty() || FileExtension == ".h")
 Argv.push_back("-xobjective-c++-header");
   Argv.push_back(File);
   tooling::CompileCommand Cmd(llvm::sys::path::parent_path(File),


Index: clang-tools-extra/trunk/clangd/unittests/GlobalCompilationDatabaseTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/GlobalCompilationDatabaseTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/GlobalCompilationDatabaseTests.cpp
@@ -36,6 +36,10 @@
   EXPECT_THAT(Cmd.CommandLine,
   ElementsAre(EndsWith("clang"), "-xobjective-c++-header",
   testPath("foo/bar.h")));
+  Cmd = DB.getFallbackCommand(testPath("foo/bar"));
+  EXPECT_THAT(Cmd.CommandLine,
+  ElementsAre(EndsWith("clang"), "-xobjective-c++-header",
+  testPath("foo/bar")));
 }
 
 static tooling::CompileCommand cmd(llvm::StringRef File, llvm::StringRef Arg) {
Index: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
@@ -58,9 +58,11 @@
 tooling::CompileCommand
 GlobalCompilationDatabase::getFallbackCommand(PathRef File) const {
   std::vector Argv = {getFallbackClangPath()};
-  // Clang treats .h files as C by default, resulting in unhelpful diagnostics.
+  // Clang treats .h files as C by default and files without extension as linker
+  // input, resulting in unhelpful diagnostics.
   // Parsing as Objective C++ is friendly to more cases.
-  if (llvm::sys::path::extension(File) == ".h")
+  auto FileExtension = llvm::sys::path::extension(File);
+  if (FileExtension.empty() || FileExtension == ".h")
 Argv.push_back("-xobjective-c++-header");
   Argv.push_back(File);
   tooling::CompileCommand Cmd(llvm::sys::path::parent_path(File),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r363664 - [clangd] Add a capability to enable completions with fixes.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Tue Jun 18 04:57:26 2019
New Revision: 363664

URL: http://llvm.org/viewvc/llvm-project?rev=363664&view=rev
Log:
[clangd] Add a capability to enable completions with fixes.

Reviewers: ilya-biryukov

Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/Protocol.h

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=363664&r1=363663&r2=363664&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Tue Jun 18 04:57:26 2019
@@ -337,6 +337,7 @@ void ClangdLSPServer::onInitialize(const
   applyConfiguration(Params.initializationOptions.ConfigSettings);
 
   CCOpts.EnableSnippets = Params.capabilities.CompletionSnippets;
+  CCOpts.IncludeFixIts = Params.capabilities.CompletionFixes;
   DiagOpts.EmbedFixesInDiagnostics = Params.capabilities.DiagnosticFixes;
   DiagOpts.SendDiagnosticCategory = Params.capabilities.DiagnosticCategory;
   DiagOpts.EmitRelatedLocations =

Modified: clang-tools-extra/trunk/clangd/Protocol.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.cpp?rev=363664&r1=363663&r2=363664&view=diff
==
--- clang-tools-extra/trunk/clangd/Protocol.cpp (original)
+++ clang-tools-extra/trunk/clangd/Protocol.cpp Tue Jun 18 04:57:26 2019
@@ -293,6 +293,8 @@ bool fromJSON(const llvm::json::Value &P
 return false;
 }
   }
+  if (auto EditsNearCursor = Completion->getBoolean("editsNearCursor"))
+R.CompletionFixes = *EditsNearCursor;
 }
 if (auto *CodeAction = TextDocument->getObject("codeAction")) {
   if (CodeAction->getObject("codeActionLiteralSupport"))

Modified: clang-tools-extra/trunk/clangd/Protocol.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.h?rev=363664&r1=363663&r2=363664&view=diff
==
--- clang-tools-extra/trunk/clangd/Protocol.h (original)
+++ clang-tools-extra/trunk/clangd/Protocol.h Tue Jun 18 04:57:26 2019
@@ -387,6 +387,11 @@ struct ClientCapabilities {
   /// textDocument.completion.completionItem.snippetSupport
   bool CompletionSnippets = false;
 
+  /// Client supports completions with additionalTextEdit near the cursor.
+  /// This is a clangd extension. (LSP says this is for unrelated text only).
+  /// textDocument.completion.editsNearCursor
+  bool CompletionFixes = false;
+
   /// Client supports hierarchical document symbols.
   bool HierarchicalDocumentSymbol = false;
 


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

[PATCH] D63091: [clangd] Add a capability to enable completions with fixes.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363664: [clangd] Add a capability to enable completions with 
fixes. (authored by sammccall, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63091

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


Index: clang-tools-extra/trunk/clangd/Protocol.h
===
--- clang-tools-extra/trunk/clangd/Protocol.h
+++ clang-tools-extra/trunk/clangd/Protocol.h
@@ -387,6 +387,11 @@
   /// textDocument.completion.completionItem.snippetSupport
   bool CompletionSnippets = false;
 
+  /// Client supports completions with additionalTextEdit near the cursor.
+  /// This is a clangd extension. (LSP says this is for unrelated text only).
+  /// textDocument.completion.editsNearCursor
+  bool CompletionFixes = false;
+
   /// Client supports hierarchical document symbols.
   bool HierarchicalDocumentSymbol = false;
 
Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
@@ -337,6 +337,7 @@
   applyConfiguration(Params.initializationOptions.ConfigSettings);
 
   CCOpts.EnableSnippets = Params.capabilities.CompletionSnippets;
+  CCOpts.IncludeFixIts = Params.capabilities.CompletionFixes;
   DiagOpts.EmbedFixesInDiagnostics = Params.capabilities.DiagnosticFixes;
   DiagOpts.SendDiagnosticCategory = Params.capabilities.DiagnosticCategory;
   DiagOpts.EmitRelatedLocations =
Index: clang-tools-extra/trunk/clangd/Protocol.cpp
===
--- clang-tools-extra/trunk/clangd/Protocol.cpp
+++ clang-tools-extra/trunk/clangd/Protocol.cpp
@@ -293,6 +293,8 @@
 return false;
 }
   }
+  if (auto EditsNearCursor = Completion->getBoolean("editsNearCursor"))
+R.CompletionFixes = *EditsNearCursor;
 }
 if (auto *CodeAction = TextDocument->getObject("codeAction")) {
   if (CodeAction->getObject("codeActionLiteralSupport"))


Index: clang-tools-extra/trunk/clangd/Protocol.h
===
--- clang-tools-extra/trunk/clangd/Protocol.h
+++ clang-tools-extra/trunk/clangd/Protocol.h
@@ -387,6 +387,11 @@
   /// textDocument.completion.completionItem.snippetSupport
   bool CompletionSnippets = false;
 
+  /// Client supports completions with additionalTextEdit near the cursor.
+  /// This is a clangd extension. (LSP says this is for unrelated text only).
+  /// textDocument.completion.editsNearCursor
+  bool CompletionFixes = false;
+
   /// Client supports hierarchical document symbols.
   bool HierarchicalDocumentSymbol = false;
 
Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
@@ -337,6 +337,7 @@
   applyConfiguration(Params.initializationOptions.ConfigSettings);
 
   CCOpts.EnableSnippets = Params.capabilities.CompletionSnippets;
+  CCOpts.IncludeFixIts = Params.capabilities.CompletionFixes;
   DiagOpts.EmbedFixesInDiagnostics = Params.capabilities.DiagnosticFixes;
   DiagOpts.SendDiagnosticCategory = Params.capabilities.DiagnosticCategory;
   DiagOpts.EmitRelatedLocations =
Index: clang-tools-extra/trunk/clangd/Protocol.cpp
===
--- clang-tools-extra/trunk/clangd/Protocol.cpp
+++ clang-tools-extra/trunk/clangd/Protocol.cpp
@@ -293,6 +293,8 @@
 return false;
 }
   }
+  if (auto EditsNearCursor = Completion->getBoolean("editsNearCursor"))
+R.CompletionFixes = *EditsNearCursor;
 }
 if (auto *CodeAction = TextDocument->getObject("codeAction")) {
   if (CodeAction->getObject("codeActionLiteralSupport"))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63331: [clangd] WIP/RFC: Prototype for semantic highlighting proposal

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D63331#1548020 , @nik wrote:

> In D63331#1543441 , @hokein wrote:
>
> > Yeah, we definitely have interest in this feature, and our intern 
> > @jvikstrom will work on this feature this summer.
>
>
> "this summer" depends on the location, so can you concretize this? :)


oops, sorry about that. I meant summer in Germany (June ~ the end of August).

>> You probably missed the discussion 
>>  on 
>> clangd-dev mailing list, @nridge also has an unfinished prototype 
>>  for semantic highlighting.
> 
> Yes, I wasn't aware that there is a clangd-dev mailing list and thus was not 
> subscribed to it. Subscribed now.
> 
> I'm glad that I haven't put more work into this ;)
> 
>> To avoid multiple people working on the same feature, I think the plan is 
>> that @jvikstrom will pick up the current stuff, and continue 
>> working/improving it.
> 
> Great. The patch from @nridge is more advanced so I'll abandon this one.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63331



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

[PATCH] D63381: Allow copy/move assignment operator to be coroutine as per N4775

[Vivek Pandya via Phabricator via cfe-commits]

vivekvpandya added a comment.

Ping!


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

https://reviews.llvm.org/D63381



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

[PATCH] D62538: [clangd] Add hidden tweaks to dump AST/selection.

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clangd/refactor/Tweak.h:104
+  /// Is this a 'hidden' tweak, which are off by default.
+  virtual bool hidden() const { return false; }
 };

ilya-biryukov wrote:
> I wonder whether this should be a static method. WDYT?
> 
> That would allow to even prevent calling `prepare()` on those tweaks.
> OTOH, we want `prepare()` should be fast and it shouldn't matter if that's 
> the case.
We'd need to add criteria to prepareTweaks() as hidden() couldn't be checked 
polymorphically on the results.

I think this is a good idea but should probably happen if/when we have a more 
important reason to touch that API.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D62538



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

Re: [PATCH] Add support for openSUSE RISC-V triple

[Andreas Schwab via cfe-commits]

On Jun 18 2019, Roman Lebedev  wrote:

> Missing a test, and if this is a patch and not a commit you want to
> submit it via https://llvm.org/docs/Phabricator.html

http://llvm.org/docs/DeveloperPolicy.html says I should send it here.

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62722: [Driver] Simplify Assemble and Backend action collapsing

[Kévin Petit via Phabricator via cfe-commits]

kpet added a comment.

Friendly ping :).


Repository:
  rC Clang

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

https://reviews.llvm.org/D62722



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

Re: [PATCH] Add support for openSUSE RISC-V triple

[Roman Lebedev via cfe-commits]

On Tue, Jun 18, 2019 at 3:06 PM Andreas Schwab  wrote:
>
> On Jun 18 2019, Roman Lebedev  wrote:
>
> > Missing a test, and if this is a patch and not a commit you want to
> > submit it via https://llvm.org/docs/Phabricator.html
>
> http://llvm.org/docs/DeveloperPolicy.html says I should send it here.
99.99% of patches go to phabricator, that page is simply misleading,
(and not for the first time).
Let's try to change that, https://reviews.llvm.org/D63488

If the patch only goes to -commits, it will most likely will
simply be lost in all the traffic.

> Andreas.
Roman.

> --
> Andreas Schwab, SUSE Labs, sch...@suse.de
> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
> "And now for something completely different."
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r363676 - Require commas to separate multiple GNU-style attributes in the same attribute list.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Tue Jun 18 05:57:05 2019
New Revision: 363676

URL: http://llvm.org/viewvc/llvm-project?rev=363676&view=rev
Log:
Require commas to separate multiple GNU-style attributes in the same attribute 
list.

Fixes PR38352.

Modified:
cfe/trunk/lib/Parse/ParseDecl.cpp
cfe/trunk/test/Parser/attributes.c

Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=363676&r1=363675&r2=363676&view=diff
==
--- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDecl.cpp Tue Jun 18 05:57:05 2019
@@ -164,10 +164,10 @@ void Parser::ParseGNUAttributes(ParsedAt
   return;
 }
 // Parse the attribute-list. e.g. __attribute__(( weak, alias("__f") ))
-while (true) {
-  // Allow empty/non-empty attributes. ((__vector_size__(16)))
-  if (TryConsumeToken(tok::comma))
-continue;
+do {
+  // Eat preceeding commas to allow __attribute__((,,,foo))
+  while (TryConsumeToken(tok::comma))
+;
 
   // Expect an identifier or declaration specifier (const, int, etc.)
   if (Tok.isAnnotation())
@@ -212,7 +212,7 @@ void Parser::ParseGNUAttributes(ParsedAt
   Eof.startToken();
   Eof.setLocation(Tok.getLocation());
   LA->Toks.push_back(Eof);
-}
+} while (Tok.is(tok::comma));
 
 if (ExpectAndConsume(tok::r_paren))
   SkipUntil(tok::r_paren, StopAtSemi);

Modified: cfe/trunk/test/Parser/attributes.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/attributes.c?rev=363676&r1=363675&r2=363676&view=diff
==
--- cfe/trunk/test/Parser/attributes.c (original)
+++ cfe/trunk/test/Parser/attributes.c Tue Jun 18 05:57:05 2019
@@ -59,8 +59,8 @@ void __attribute__((returns_twice)) retu
 
 int aligned(int);
 int __attribute__((vec_type_hint(char, aligned(16) )) missing_rparen_1; // 
expected-error 2{{expected ')'}} expected-note {{to match}} expected-warning 
{{does not declare anything}}
-int __attribute__((mode(x aligned(16) )) missing_rparen_2; // expected-error 
{{expected ')'}}
-int __attribute__((format(printf, 0 aligned(16) )) missing_rparen_3; // 
expected-error {{expected ')'}}
+int __attribute__((mode(x aligned(16) )) missing_rparen_2; // expected-error 
2{{expected ')'}}
+int __attribute__((format(printf, 0 aligned(16) )) missing_rparen_3; // 
expected-error 2{{expected ')'}}
 
 
 
@@ -105,3 +105,11 @@ struct s {
 // specifier.
 struct s
 __attribute__((used)) bar;
+
+// Ensure that attributes must be separated by a comma (PR38352).
+__attribute__((const const)) int PR38352(void); // expected-error {{expected 
')'}}
+// Also ensure that we accept spurious commas.
+__attribute__((,,,const)) int PR38352_1(void);
+__attribute__((const,,,)) int PR38352_2(void);
+__attribute__((const,,,const)) int PR38352_3(void);
+__attribute__((,,,const,,,const,,,)) int PR38352_4(void);


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

[PATCH] D62635: Add enums as global variables in the IR metadata.

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

We did things this way to track which **enumerators** were used, not which 
enums were used. Size data showed it was worth doing (6%). The existing format 
doesn't support tracking usage of individual enumerators, so we pretended they 
were const integers to avoid changing the schema.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62635



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

[PATCH] D63490: Script for generating AST JSON dump test cases

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added a reviewer: rsmith.

Trying to write or maintain test code for AST dumping of JSON information can 
be onerous due to small changes in the test requiring a large amount of line 
numbers or other information to be updated in the expected test output. To help 
with this, we devised a simple python script that automatically generates the 
expected output and appends it to the end of a test file.

The script allows you to specify the clang instance to run, clang command line 
options used within test file, the source file to generate the test output 
from, and a filter in case you wish to only generate test output for specific 
AST JSON nodes.

For instance, you can execute `python gen_ast_dump_json_test.py --clang 
D:\trunk_build\build\x64-Debug\bin\clang.exe --opts "-triple 
x86_64-unknown-unknown -std=gnu11" --source ast-dump-expr-json.c --filters 
FunctionDecl` to generate the test output seen in `ast-dump-expr-json.c`. The 
script also works when passing `-ast-dump-filter` in the options to generate 
the output.


https://reviews.llvm.org/D63490

Files:
  test/AST/gen_ast_dump_json_test.py

Index: test/AST/gen_ast_dump_json_test.py
===
--- test/AST/gen_ast_dump_json_test.py
+++ test/AST/gen_ast_dump_json_test.py
@@ -0,0 +1,137 @@
+#!/usr/bin/env python
+
+from collections import OrderedDict
+from sets import Set
+from shutil import copyfile
+import argparse
+import json
+import os
+import pprint
+import re
+import subprocess
+
+def normalize(dict_var):
+for k, v in dict_var.items():
+if isinstance(v, OrderedDict):
+normalize(v)
+elif isinstance(v, list):
+for e in v:
+if isinstance(e, OrderedDict):
+normalize(e)
+elif type(v) is unicode:
+st = v.encode('utf-8')
+if re.match(r"0x[0-9A-Fa-f]+", v):
+dict_var[k] = u'0x{{.*}}'
+elif os.path.isfile(v):
+dict_var[k] = u'{{.*}}'
+else:
+splits = (v.split(u' '))
+out_splits = []
+for split in splits:
+inner_splits = split.rsplit(u':',2)
+if os.path.isfile(inner_splits[0]):
+out_splits.append(
+u'{{.*}}:%s:%s'
+%(inner_splits[1],
+  inner_splits[2]))
+continue
+out_splits.append(split)
+
+dict_var[k] = ' '.join(out_splits)
+
+def filter_json(dict_var, filters, out):
+for k, v in dict_var.items():
+if type(v) is unicode:
+st = v.encode('utf-8')
+if st in filters:
+out.append(dict_var)
+break
+elif isinstance(v, OrderedDict):
+filter_json(v, filters, out)
+elif isinstance(v, list):
+for e in v:
+if isinstance(e, OrderedDict):
+filter_json(e, filters, out)
+
+def main():
+parser = argparse.ArgumentParser()
+parser.add_argument("--clang", help="The clang binary (could be a relative or absolute path)",
+action="store", required=True)
+parser.add_argument("--opts", help="other options",
+action="store", default='', type=str)
+parser.add_argument("--source", help="the source file. Command used to generate the json will be of the format  -cc1 -ast-dump=json  ",
+action="store", required=True)
+parser.add_argument("--filters", help="comma separated list of AST filters. Ex: --filters=TypedefDecl,BuiltinType",
+action="store", default='')
+
+args = parser.parse_args()
+
+if not args.source:
+print("Specify the source file to give to clang.")
+return -1
+
+clang_binary = os.path.abspath(args.clang)
+if not os.path.isfile(clang_binary):
+print("clang binary specified not present.")
+return -1
+
+options = args.opts.split(' ')
+filters = Set(args.filters.split(',')) if args.filters else Set([])
+
+cmd = [clang_binary, "-cc1"]
+cmd.extend(options)
+
+using_ast_dump_filter = 'ast-dump-filter' in args.opts
+
+cmd.extend(["-ast-dump=json", args.source])
+
+try:
+json_str = subprocess.check_output(cmd)
+except Exception as ex:
+print("The clang command failed with %s" % ex)
+return -1
+
+out_asts = []
+if using_ast_dump_filter:
+splits = re.split('Dumping .*:\n', json_str)
+if len(splits) > 1:
+for split in splits[1:]:
+j = json.loads(split.decode('utf-8'), object_pairs_hook=OrderedDict)
+normalize(j)
+out_asts.append(j)
+else:
+j = json.loads(json_str.decode('u

[PATCH] D62952: [analyzer][tests] Use normalize_sarif in place of diff_sarif

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

Ping.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62952



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

[clang-tools-extra] r363680 - [clangd] Add hidden tweaks to dump AST/selection.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Tue Jun 18 06:37:54 2019
New Revision: 363680

URL: http://llvm.org/viewvc/llvm-project?rev=363680&view=rev
Log:
[clangd] Add hidden tweaks to dump AST/selection.

Summary:
This introduces a few new concepts:
 - tweaks have an Intent (they don't all advertise as refactorings)
 - tweaks may produce messages (for ShowMessage notification). Generalized
   Replacements -> Effect.
 - tweaks (and other features) may be hidden (clangd -hidden-features flag).
   We may choose to promote these one day. I'm not sure they're worth their own
   feature flags though.

Verified it in vim-clangd (not yet open source), curious if the UI is ok in 
VSCode.

Reviewers: ilya-biryukov

Subscribers: mgorny, MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

Added:
clang-tools-extra/trunk/clangd/refactor/tweaks/DumpAST.cpp
Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/Protocol.h
clang-tools-extra/trunk/clangd/Selection.cpp
clang-tools-extra/trunk/clangd/refactor/Tweak.h
clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt
clang-tools-extra/trunk/clangd/refactor/tweaks/RawStringLiteral.cpp
clang-tools-extra/trunk/clangd/refactor/tweaks/SwapIfBranches.cpp
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
clang-tools-extra/trunk/clangd/unittests/SelectionTests.cpp
clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=363680&r1=363679&r2=363680&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Tue Jun 18 06:37:54 2019
@@ -13,6 +13,7 @@
 #include "SourceCode.h"
 #include "Trace.h"
 #include "URI.h"
+#include "refactor/Tweak.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/ScopeExit.h"
@@ -31,7 +32,14 @@ CodeAction toCodeAction(const ClangdServ
 Range Selection) {
   CodeAction CA;
   CA.title = T.Title;
-  CA.kind = CodeAction::REFACTOR_KIND;
+  switch (T.Intent) {
+  case Tweak::Refactor:
+CA.kind = CodeAction::REFACTOR_KIND;
+break;
+  case Tweak::Info:
+CA.kind = CodeAction::INFO_KIND;
+break;
+  }
   // This tweak may have an expensive second stage, we only run it if the user
   // actually chooses it in the UI. We reply with a command that would run the
   // corresponding tweak.
@@ -481,18 +489,25 @@ void ClangdLSPServer::onCommand(const Ex
   llvm::inconvertibleErrorCode(),
   "trying to apply a code action for a non-added file"));
 
-auto Action = [ApplyEdit](decltype(Reply) Reply, URIForFile File,
-  std::string Code,
-  llvm::Expected> R) {
+auto Action = [this, ApplyEdit](decltype(Reply) Reply, URIForFile File,
+std::string Code,
+llvm::Expected R) {
   if (!R)
 return Reply(R.takeError());
 
-  WorkspaceEdit WE;
-  WE.changes.emplace();
-  (*WE.changes)[File.uri()] = std::move(*R);
-
-  Reply("Fix applied.");
-  ApplyEdit(std::move(WE));
+  if (R->ApplyEdit) {
+WorkspaceEdit WE;
+WE.changes.emplace();
+(*WE.changes)[File.uri()] = replacementsToEdits(Code, *R->ApplyEdit);
+ApplyEdit(std::move(WE));
+  }
+  if (R->ShowMessage) {
+ShowMessageParams Msg;
+Msg.message = *R->ShowMessage;
+Msg.type = MessageType::Info;
+notify("window/showMessage", Msg);
+  }
+  Reply("Tweak applied.");
 };
 Server->applyTweak(Params.tweakArgs->file.file(),
Params.tweakArgs->selection, Params.tweakArgs->tweakID,

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=363680&r1=363679&r2=363680&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Jun 18 06:37:54 2019
@@ -95,6 +95,7 @@ ClangdServer::ClangdServer(const GlobalC
  : nullptr),
   GetClangTidyOptions(Opts.GetClangTidyOptions),
   SuggestMissingIncludes(Opts.SuggestMissingIncludes),
+  EnableHiddenFeatures(Opts.HiddenFeatures),
   WorkspaceRoot(Opts.WorkspaceRoot),
   // Pass a callback into `WorkScheduler` to extract symbols from a newly
   // parsed file and 

[PATCH] D63479: Added SemanticSymbolASTCollector for collecting semantic symbolsCurrently collects variable and function declarations

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom abandoned this revision.
jvikstrom added a comment.

Resubmitting later with "complete" prototype in CL


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63479



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

[PATCH] D62538: [clangd] Add hidden tweaks to dump AST/selection.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363680: [clangd] Add hidden tweaks to dump AST/selection. 
(authored by sammccall, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62538?vs=201765&id=205322#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62538

Files:
  clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
  clang-tools-extra/trunk/clangd/ClangdServer.cpp
  clang-tools-extra/trunk/clangd/ClangdServer.h
  clang-tools-extra/trunk/clangd/Protocol.cpp
  clang-tools-extra/trunk/clangd/Protocol.h
  clang-tools-extra/trunk/clangd/Selection.cpp
  clang-tools-extra/trunk/clangd/refactor/Tweak.h
  clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt
  clang-tools-extra/trunk/clangd/refactor/tweaks/DumpAST.cpp
  clang-tools-extra/trunk/clangd/refactor/tweaks/RawStringLiteral.cpp
  clang-tools-extra/trunk/clangd/refactor/tweaks/SwapIfBranches.cpp
  clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
  clang-tools-extra/trunk/clangd/unittests/SelectionTests.cpp
  clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/trunk/clangd/Protocol.h
===
--- clang-tools-extra/trunk/clangd/Protocol.h
+++ clang-tools-extra/trunk/clangd/Protocol.h
@@ -483,6 +483,28 @@
 };
 bool fromJSON(const llvm::json::Value &, InitializeParams &);
 
+enum class MessageType {
+  /// An error message.
+  Error = 1,
+  /// A warning message.
+  Warning = 1,
+  /// An information message.
+  Info = 1,
+  /// A log message.
+  Log = 1,
+};
+llvm::json::Value toJSON(const MessageType &);
+
+/// The show message notification is sent from a server to a client to ask the
+/// client to display a particular message in the user interface.
+struct ShowMessageParams {
+  /// The message type.
+  MessageType type = MessageType::Info;
+  /// The actual message.
+  std::string message;
+};
+llvm::json::Value toJSON(const ShowMessageParams &);
+
 struct DidOpenTextDocumentParams {
   /// The document that was opened.
   TextDocumentItem textDocument;
@@ -740,6 +762,7 @@
   llvm::Optional kind;
   const static llvm::StringLiteral QUICKFIX_KIND;
   const static llvm::StringLiteral REFACTOR_KIND;
+  const static llvm::StringLiteral INFO_KIND;
 
   /// The diagnostics that this code action resolves.
   llvm::Optional> diagnostics;
Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
@@ -13,6 +13,7 @@
 #include "SourceCode.h"
 #include "Trace.h"
 #include "URI.h"
+#include "refactor/Tweak.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/ScopeExit.h"
@@ -31,7 +32,14 @@
 Range Selection) {
   CodeAction CA;
   CA.title = T.Title;
-  CA.kind = CodeAction::REFACTOR_KIND;
+  switch (T.Intent) {
+  case Tweak::Refactor:
+CA.kind = CodeAction::REFACTOR_KIND;
+break;
+  case Tweak::Info:
+CA.kind = CodeAction::INFO_KIND;
+break;
+  }
   // This tweak may have an expensive second stage, we only run it if the user
   // actually chooses it in the UI. We reply with a command that would run the
   // corresponding tweak.
@@ -481,18 +489,25 @@
   llvm::inconvertibleErrorCode(),
   "trying to apply a code action for a non-added file"));
 
-auto Action = [ApplyEdit](decltype(Reply) Reply, URIForFile File,
-  std::string Code,
-  llvm::Expected> R) {
+auto Action = [this, ApplyEdit](decltype(Reply) Reply, URIForFile File,
+std::string Code,
+llvm::Expected R) {
   if (!R)
 return Reply(R.takeError());
 
-  WorkspaceEdit WE;
-  WE.changes.emplace();
-  (*WE.changes)[File.uri()] = std::move(*R);
-
-  Reply("Fix applied.");
-  ApplyEdit(std::move(WE));
+  if (R->ApplyEdit) {
+WorkspaceEdit WE;
+WE.changes.emplace();
+(*WE.changes)[File.uri()] = replacementsToEdits(Code, *R->ApplyEdit);
+ApplyEdit(std::move(WE));
+  }
+  if (R->ShowMessage) {
+ShowMessageParams Msg;
+Msg.message = *R->ShowMessage;
+Msg.type = MessageType::Info;
+notify("window/showMessage", Msg);
+  }
+  Reply("Tweak applied.");
 };
 Server->applyTweak(Params.tweakArgs->file.file(),
Params.tweakArgs->selection, Params.tweakArgs->tweakID,
Index: clang-tools-extra/trunk/clangd/Selection.cpp
===
--- clang-tools-extra/trunk/clangd/Selection.cpp
+++ clang-tools-extra/trunk/clangd/Sele

[PATCH] D63330: [clangd] Add Value field to HoverInfo

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.cpp:722
 
+  // Fill in value with initializer. Puts evaluated version if possible.
+  if (const auto *Var = dyn_cast(D)) {

do we want the initializer both here and in Definition?

I suspect we'd like to turn off one or the other when both are present.

Looking at the examples, I think it's clearer to keep a non-evaluated 
initializer as part of the Definition (which is mostly as-written), and only 
include Value if it's evaluated.



Comment at: clang-tools-extra/clangd/XRefs.cpp:733
+else
+  Init->printPretty(ValueOS, nullptr, Policy);
+  }

why not print the non-evaluated init if it's dependent?



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:920
+  {R"cpp(
+constexpr int add(int a, int b) { return a + b; }
+int [[b^ar]] = add(1, 2);

constexpr may not be required here, I think?
At least the docs suggest evaluation ignores language semantics like this.



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:956
+   }},
+  // FIXME: We should use TypeLoc instead of Decl to extract the concrete
+  // value.

Hmm, actually I'm not sure whether this is TypeLoc vs Decl here. Rather this 
should be a DeclRefExpr to the VarDecl (result) in the *expanded* CXXRecordDecl.

This suggests that maybe the isValueDependent check isn't quite right - the 
initializer expression is value dependent as spelled, but in the instantiation 
it's resolved. Not sure if this is fixable. What happens if you comment out the 
check?



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:981
+ HI.NamespaceScope = "";
+ HI.Value = "&\"1234\"[0]";
+   }},

whoa, that's... weird. But fine, I think.

(It would be nice to elide the `&[]` if the index is zero, or maybe print as 
`"1234" + 2` etc)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63330



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

[PATCH] D44865: [libc++] Implement P0608R3 - A sane variant converting constructor

[Marshall Clow via Phabricator via cfe-commits]

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

This looks good to me. Thanks!


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D44865



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

[clang-tools-extra] r363681 - [clangd] Remove the extra ";", NFC

[Haojian Wu via cfe-commits]

Author: hokein
Date: Tue Jun 18 06:52:00 2019
New Revision: 363681

URL: http://llvm.org/viewvc/llvm-project?rev=363681&view=rev
Log:
[clangd] Remove the extra ";", NFC

Modified:
clang-tools-extra/trunk/clangd/refactor/tweaks/DumpAST.cpp

Modified: clang-tools-extra/trunk/clangd/refactor/tweaks/DumpAST.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/tweaks/DumpAST.cpp?rev=363681&r1=363680&r2=363681&view=diff
==
--- clang-tools-extra/trunk/clangd/refactor/tweaks/DumpAST.cpp (original)
+++ clang-tools-extra/trunk/clangd/refactor/tweaks/DumpAST.cpp Tue Jun 18 
06:52:00 2019
@@ -94,7 +94,7 @@ public:
   Intent intent() const override { return Info; }
   bool hidden() const override { return true; }
 };
-REGISTER_TWEAK(ShowSelectionTree);
+REGISTER_TWEAK(ShowSelectionTree)
 
 /// Shows the layout of the RecordDecl under the cursor.
 /// Input:
@@ -132,7 +132,7 @@ public:
 private:
   const RecordDecl *Record = nullptr;
 };
-REGISTER_TWEAK(DumpRecordLayout);
+REGISTER_TWEAK(DumpRecordLayout)
 
 } // namespace
 } // namespace clangd


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

r363682 - AMDGPU: Disable errno by default

[Matt Arsenault via cfe-commits]

Author: arsenm
Date: Tue Jun 18 06:59:32 2019
New Revision: 363682

URL: http://llvm.org/viewvc/llvm-project?rev=363682&view=rev
Log:
AMDGPU: Disable errno by default

Modified:
cfe/trunk/lib/Driver/ToolChains/AMDGPU.h
cfe/trunk/test/Driver/fast-math.c

Modified: cfe/trunk/lib/Driver/ToolChains/AMDGPU.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/AMDGPU.h?rev=363682&r1=363681&r2=363682&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/AMDGPU.h (original)
+++ cfe/trunk/lib/Driver/ToolChains/AMDGPU.h Tue Jun 18 06:59:32 2019
@@ -57,6 +57,8 @@ public:
   const llvm::opt::ArgList &Args);
   unsigned GetDefaultDwarfVersion() const override { return 5; }
   bool IsIntegratedAssemblerDefault() const override { return true; }
+  bool IsMathErrnoDefault() const override { return false; }
+
   llvm::opt::DerivedArgList *
   TranslateArgs(const llvm::opt::DerivedArgList &Args, StringRef BoundArch,
 Action::OffloadKind DeviceOffloadKind) const override;

Modified: cfe/trunk/test/Driver/fast-math.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fast-math.c?rev=363682&r1=363681&r2=363682&view=diff
==
--- cfe/trunk/test/Driver/fast-math.c (original)
+++ cfe/trunk/test/Driver/fast-math.c Tue Jun 18 06:59:32 2019
@@ -97,6 +97,12 @@
 // RUN:   | FileCheck --check-prefix=CHECK-NO-MATH-ERRNO %s
 // RUN: %clang -### -target x86_64-linux-android -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-MATH-ERRNO %s
+// RUN: %clang -### -target amdgcn-amd-amdhsa -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-NO-MATH-ERRNO %s
+// RUN: %clang -### -target amdgcn-amd-amdpal -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-NO-MATH-ERRNO %s
+// RUN: %clang -### -target amdgcn-mesa-mesa3d -c %s 2>&1   \
+// RUN:   | FileCheck --check-prefix=CHECK-NO-MATH-ERRNO %s
 //
 // Check that -ffast-math disables -fmath-errno, and -fno-fast-math merely
 // preserves the target default. Also check various flag set operations between


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

[PATCH] D62697: AMDGPU: Disable errno by default

[Matt Arsenault via Phabricator via cfe-commits]

arsenm closed this revision.
arsenm added a comment.

r363682


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

https://reviews.llvm.org/D62697



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

[PATCH] D63299: [Clang] Parse GNU fallthrough attributes

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

xbolva00 reclaimed this revision.
xbolva00 added a comment.

Ok, If we peek a few tokens ahead and check attribute if it is stmt attribute 
and then we call MaybeParseGNUAttr - this probably would work but are you fine 
with it as a workaround ?


Repository:
  rC Clang

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

https://reviews.llvm.org/D63299



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

Most of the comments are about minor nits like grammar and coding conventions, 
but I did have some questions regarding what kinds of functions the sycl_kernel 
attribute gets applied to. Also, I'd like to see some additional tests that 
demonstrate the sycl device attribute is being implicitly created on the proper 
declarations as expected (can probably do that using -ast-dump and checking to 
see if the right functions have the attribute attached).




Comment at: clang/include/clang/Basic/AttrDocs.td:259
+  let Content = [{
+The ``sycl_kernel`` attribute specifies that a function is SYCL "kernel
+function". SYCL "kernel function" defines an entry point to a kernel.

is SYCL "kernel function" -> is a SYCL "kernel function"



Comment at: clang/include/clang/Basic/AttrDocs.td:260
+The ``sycl_kernel`` attribute specifies that a function is SYCL "kernel
+function". SYCL "kernel function" defines an entry point to a kernel.
+Kernel is a function which will be compiled for the device and defines some

SYCL -> A SYCL



Comment at: clang/include/clang/Basic/AttrDocs.td:261
+function". SYCL "kernel function" defines an entry point to a kernel.
+Kernel is a function which will be compiled for the device and defines some
+entry point to device code i.e. will be called by host in run time.

Kernel is a -> A kernel is a



Comment at: clang/include/clang/Basic/AttrDocs.td:263-264
+entry point to device code i.e. will be called by host in run time.
+Here is a code example of the SYCL program, which demonstrates the need for
+this attribute:
+.. code-block:: c++

This doesn't really demonstrate the need for the attribute -- the attribute is 
never shown in the code example. I'd prefer an example that shows when and how 
a user would write this attribute.



Comment at: clang/include/clang/Basic/AttrDocs.td:278
+  }
+The lambda that is passed to the ``parallel_for`` is called SYCL
+"kernel function".

called SYLC -> called a SYLC



Comment at: clang/include/clang/Basic/AttrDocs.td:280
+"kernel function".
+The SYCL Runtime implementation will use ``sycl_kernel`` attribute to mark this
+lambda as SYCL "kernel function". Compiler is supposed to construct a kernel

use sycl_kernel -> use the sycl_kernel



Comment at: clang/include/clang/Basic/AttrDocs.td:281
+The SYCL Runtime implementation will use ``sycl_kernel`` attribute to mark this
+lambda as SYCL "kernel function". Compiler is supposed to construct a kernel
+from "kernel function", add it to the "device part" of code and traverse all

as SYCL -> as a SYCL
Compiler is supposed to -> The compiler will



Comment at: clang/include/clang/Basic/AttrDocs.td:284
+symbols accessible from "kernel function" and add them to the "device part" of
+the code. In this code example compiler is supposed to add "foo" function to 
the
+"device part" of the code.

In this code example compiler is supposed to add "foo" function -> In this code 
example, the compiler will add the "foo" function



Comment at: clang/include/clang/Sema/Sema.h:11182
+private:
+  /// Contains Function declarations to be added to the SYCL device code.
+  /// In SYCL when we generate device code we don't know which functions we 
will

Function -> function



Comment at: clang/include/clang/Sema/Sema.h:11183
+  /// Contains Function declarations to be added to the SYCL device code.
+  /// In SYCL when we generate device code we don't know which functions we 
will
+  /// emit before we emit sycl kernels so we add device functions to this array

In SYCL, when we generate device code, we don't know 



Comment at: clang/include/clang/Sema/Sema.h:11184
+  /// In SYCL when we generate device code we don't know which functions we 
will
+  /// emit before we emit sycl kernels so we add device functions to this array
+  /// and handle it in separate way.

we emit sycl kernels, so we add device 



Comment at: clang/include/clang/Sema/Sema.h:11189
+public:
+  /// This function adds function declaration to the SYCL device code.
+  void AddSyclDeviceFunc(Decl *D) {

adds the function declaration



Comment at: clang/include/clang/Sema/Sema.h:11190
+  /// This function adds function declaration to the SYCL device code.
+  void AddSyclDeviceFunc(Decl *D) {
+D->addAttr(SYCLDeviceAttr::CreateImplicit(Context));

Should be named `addSyclDeviceFunc()` per coding standards. Similar for the 
other new functions.



Comment at: clang/include/clang/Sema/Sema.h:11194
+  }
+  /// SyclDeviceFuncs - access to SYCL device function decls.
+  SmallVector &SyclDeviceFuncs() { r

r363684 - AMDGPU: Add GWS instruction builtins

[Matt Arsenault via cfe-commits]

Author: arsenm
Date: Tue Jun 18 07:10:01 2019
New Revision: 363684

URL: http://llvm.org/viewvc/llvm-project?rev=363684&view=rev
Log:
AMDGPU: Add GWS instruction builtins

Modified:
cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def
cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn.cl

Modified: cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def?rev=363684&r1=363683&r2=363684&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def Tue Jun 18 07:10:01 2019
@@ -45,6 +45,8 @@ BUILTIN(__builtin_amdgcn_s_barrier, "v",
 BUILTIN(__builtin_amdgcn_wave_barrier, "v", "n")
 BUILTIN(__builtin_amdgcn_s_dcache_inv, "v", "n")
 BUILTIN(__builtin_amdgcn_buffer_wbinvl1, "v", "n")
+BUILTIN(__builtin_amdgcn_ds_gws_init, "vUiUi", "n")
+BUILTIN(__builtin_amdgcn_ds_gws_barrier, "vUiUi", "n")
 
 // FIXME: Need to disallow constant address space.
 BUILTIN(__builtin_amdgcn_div_scale, "dddbb*", "n")

Modified: cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn.cl?rev=363684&r1=363683&r2=363684&view=diff
==
--- cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn.cl Tue Jun 18 07:10:01 2019
@@ -548,6 +548,18 @@ kernel void test_ds_consume_lds(global i
   *out = __builtin_amdgcn_ds_consume(ptr);
 }
 
+// CHECK-LABEL: @test_gws_init(
+// CHECK: call void @llvm.amdgcn.ds.gws.init(i32 %value, i32 %id)
+kernel void test_gws_init(uint value, uint id) {
+  __builtin_amdgcn_ds_gws_init(value, id);
+}
+
+// CHECK-LABEL: @test_gws_barrier(
+// CHECK: call void @llvm.amdgcn.ds.gws.barrier(i32 %value, i32 %id)
+kernel void test_gws_barrier(uint value, uint id) {
+  __builtin_amdgcn_ds_gws_barrier(value, id);
+}
+
 // CHECK-DAG: [[$WI_RANGE]] = !{i32 0, i32 1024}
 // CHECK-DAG: attributes #[[$NOUNWIND_READONLY:[0-9]+]] = { nounwind readonly }
 // CHECK-DAG: attributes #[[$READ_EXEC_ATTRS]] = { convergent }


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

[PATCH] D63366: AMDGPU: Add GWS instruction builtins

[Matt Arsenault via Phabricator via cfe-commits]

arsenm closed this revision.
arsenm added a comment.

r363684


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

https://reviews.llvm.org/D63366



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

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

[Thomas Manceau via Phabricator via cfe-commits]

Blackhart updated this revision to Diff 205331.
Blackhart added a comment.

- Reorder release note changes
- Update documentation




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

https://reviews.llvm.org/D63324

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/modernize-replace-memcpy-by-stdcopy.rst

Index: clang-tools-extra/docs/clang-tidy/checks/modernize-replace-memcpy-by-stdcopy.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/modernize-replace-memcpy-by-stdcopy.rst
@@ -0,0 +1,47 @@
+.. title:: clang-tidy - modernize-replace-memcpy-by-stdcopy
+
+modernize-replace-memcpy-by-stdcopy
+===
+
+This check replaces all occurrences of the C ``memcpy`` function by ``std::copy``
+
+Example:
+
+.. code-block:: c++
+
+  /*!
+   * \param destination Pointer to the destination array where the content is to be copied
+   * \param source Pointer to the source of data to be copied
+   * \param num Number of bytes to copy
+   */
+  memcpy(destination, source, num);
+
+becomes
+
+.. code-block:: c++
+
+  /*!
+   * \param destination Pointer to the destination array where the content is to be copied
+   * \param source Pointer to the source of data to be copied
+   * \param num Number of bytes to copy
+   */
+  std::copy(source, source + (num / sizeof *source), destination);
+
+Bytes to iterator conversion
+
+
+Unlike ``std::copy`` that take an iterator on the last element of the source array, ``memcpy`` request the number of bytes to copy.
+In order to make the check working, it will convert the size parameter to an iterator by replacing it by ``source + (num / sizeof *source)``
+
+Include
+---
+
+``std::copy`` being provided by the ``algorithm`` include file, this check will include it if needed.
+  
+Options
+---
+
+.. option:: IncludeStyle
+
+   A string specifying which include-style is used, `llvm` or `google`. Default
+   is `llvm`.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -211,6 +211,7 @@
modernize-raw-string-literal
modernize-redundant-void-arg
modernize-replace-auto-ptr
+   modernize-replace-memcpy-by-stdcopy
modernize-replace-random-shuffle
modernize-return-braced-init-list
modernize-shrink-to-fit
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -186,6 +186,11 @@
   finds and replaces cases that match the pattern ``var &&
   isa(var)``, where ``var`` is evaluated twice.
 
+- New :doc:`modernize-replace-memcpy-by-stdcopy
+  ` check.
+  
+  This check replaces all occurrences of the C ``memcpy`` function by ``std::copy``.
+
 - New :doc:`modernize-use-trailing-return-type
   ` check.
 
Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
@@ -0,0 +1,48 @@
+//===--- ReplaceMemcpyByStdCopy.h - clang-tidy*- 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
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Replace the C memcpy function by std::copy
+class ReplaceMemcpyByStdCopy : public ClangTidyCheck {
+public:
+  ReplaceMemcpyByStdCopy(StringRef Name, ClangTidyContext *Context);
+  ~ReplaceMemcpyByStdCopy() override = default;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Options) override;
+
+private:
+  void renameFunction(Diagn

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

[Thomas Manceau via Phabricator via cfe-commits]

Blackhart added a comment.

Hello,

I've updated the documentation with more informations about the transformations 
the check do.

I'm not very confident in writing documentation in english.
Please review it and submit me your change requests.


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

https://reviews.llvm.org/D63324



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

[PATCH] D62952: [analyzer][tests] Use normalize_sarif in place of diff_sarif

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D62952#1535088 , 
@hubert.reinterpretcast wrote:

> @aaron.ballman, for similar cases in the plist output, it has been proposed
>
> - that the reference expected file be committed into the tree pre-normalized, 
> and
> - that tool be modified such that the output file has a newline at the end of 
> the file.
>
>   Does that sound good for this format?


In general, that seems reasonable, but I would prefer to take care of more of 
the work in lit.local.cfg than have to deal with that atrocious RUN line in 
every test case. Is there a way to retain a similarly succinct solution as 
diff_sarif?


Repository:
  rC Clang

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

https://reviews.llvm.org/D62952



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

[PATCH] D63437: [CodeGen][ARM] Fix FP16 vector coercion

[Oliver Stannard (Linaro) via Phabricator via cfe-commits]

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

Ah, right. In that case, this LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63437



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

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:192
+  
+  This check replaces all occurrences of the C ``memcpy`` function by 
``std::copy``.
+

Please remove //This check//. Same in first statement in documentation.


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

https://reviews.llvm.org/D63324



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

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Still missing tests




Comment at: 
clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp:34-35
+  callExpr(hasDeclaration(functionDecl(hasName("memcpy"),
+   isExpansionInSystemHeader())),
+   isExpansionInMainFile())
+  .bind("memcpy_function");

Why these restrictions?



Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h:26
+  ReplaceMemcpyByStdCopy(StringRef Name, ClangTidyContext *Context);
+  ~ReplaceMemcpyByStdCopy() override = default;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;

I don't think you need this one


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

https://reviews.llvm.org/D63324



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

[PATCH] D59402: Suggestions to fix -Wmissing-{prototypes,variable-declarations}

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM!




Comment at: lib/Sema/SemaDecl.cpp:13345-13346
+<< (FD->getStorageClass() == SC_None
+? FixItHint::CreateInsertion(FD->getTypeSpecStartLoc(),
+ "static ")
+: FixItHint{});

aaronpuchert wrote:
> aaron.ballman wrote:
> > aaronpuchert wrote:
> > > aaron.ballman wrote:
> > > > We may not want to produce the fixit if there's a macro involved in the 
> > > > declaration. Consider:
> > > > ```
> > > > #ifdef SOMETHING
> > > > #define FROBBLE static
> > > > #else
> > > > #define FROBBLE
> > > > #endif
> > > > 
> > > > FROBBLE void foo(void);
> > > > ```
> > > > We probably don't want the fixit in the case `SOMETHING` is not defined.
> > > I think that's generally an issue with fix-its, there could always be a 
> > > macro that turns the code into something entirely different. If we look 
> > > at the other fix-it above, we can construct
> > > 
> > > ```
> > > #define VOID
> > > int f(VOID);
> > > int f() { return 0; }
> > > ```
> > > 
> > > Then we get:
> > > ```
> > > :3:5: warning: no previous prototype for function 'f' 
> > > [-Wmissing-prototypes]
> > > int f() { return 0; }
> > > ^
> > > :2:5: note: this declaration is not a prototype; add 'void' to 
> > > make it a prototype for a zero-parameter function
> > > int f(VOID);
> > > ^
> > >   void
> > > ```
> > > 
> > > Then the fix-it doesn't even work in the original configuration, because 
> > > it produces `int f(VOIDvoid)`. If we make it work by adding a space, we 
> > > still have the problem that you mentioned: if someone defines `VOID` as 
> > > `void`, we then have `int f(void void)` after applying the fix-it in the 
> > > original setting.
> > > 
> > > Trying to make fix-its work with macros is probably a hopeless endeavor.
> > > Trying to make fix-its work with macros is probably a hopeless endeavor.
> > 
> > That's what I was getting at -- I think you need to see if there's a macro 
> > at the insertion location and not generate a fix-it in that case. The above 
> > suffers from the same issue (and can be corrected in a subsequent patch, if 
> > desired).
> How can I know that? The AST seems to contain no such information, for the 
> example I only see the following:
> 
> ```
> TranslationUnitDecl 0x12574e8 <> 
> |-[...]
> |-FunctionDecl 0x12b5340 <:2:1, col:11> col:5 f 'int ()'
> `-FunctionDecl 0x12b5440 prev 0x12b5340  col:5 f 'int ()'
>   `-CompoundStmt 0x12b5508 
> `-ReturnStmt 0x12b54f8 
>   `-IntegerLiteral 0x12b54d8  'int' 0
> ```
> 
> It seems fix-its are automatically suppressed if they would land in a macro 
> definition, like
> 
> ```
> #define X int f()
> X;
> int f() { return 0; }
> ```
> 
> produces
> 
> ```
> :3:5: warning: no previous prototype for function 'f' 
> [-Wmissing-prototypes]
> int f() { return 0; }
> ^
> :2:1: note: this declaration is not a prototype; add 'void' to make it 
> a prototype for a zero-parameter function
> X;
> ^
> :1:15: note: expanded from macro 'X'
> #define X int f()
>   ^
> ```
> 
> So there is no fix-it suggested, although we produce one. I guess some 
> intermediate layer that knows about the connection to the original source 
> detects that and throws it away.
> 
> But what you want is that we also suppress it if there is any macro (it 
> wouldn't need to be at the beginning, it could also be `void FROBBLE 
> foo(void)` with the same meaning). That would also mean I can't apply the 
> fix-it even if the macros is unrelated, say if someone has
> 
> ```
> #define ATTR_PURE __attribute__((pure))
> ATTR_PURE int f() { return 0; }
> ```
> 
> Other fix-its (that I have looked at) just seem to ignore macros, like we do, 
> relying on fix-its to be discarded if they would land inside of a macro.
> 
> I think there is a case to be made for avoiding `int f(VOIDvoid)`, but not 
> suggesting the fix-it because a macro could have another meaning in another 
> configuration seems unusual to me.
Okay, good point about `void FOO blah(void);` being just as problematic. I was 
thinking you would look at `FD->getTypeSpecStartLoc().isMacroID()`, but that 
would be insufficient and may be handled by an intermediate layer anyway.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59402



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

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

[Thomas Manceau via Phabricator via cfe-commits]

Blackhart updated this revision to Diff 205341.
Blackhart marked 3 inline comments as done.
Blackhart added a comment.

Remove "//this check//" from documentation and release note


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

https://reviews.llvm.org/D63324

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/modernize-replace-memcpy-by-stdcopy.rst

Index: clang-tools-extra/docs/clang-tidy/checks/modernize-replace-memcpy-by-stdcopy.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/modernize-replace-memcpy-by-stdcopy.rst
@@ -0,0 +1,47 @@
+.. title:: clang-tidy - modernize-replace-memcpy-by-stdcopy
+
+modernize-replace-memcpy-by-stdcopy
+===
+
+Replaces all occurrences of the C ``memcpy`` function by ``std::copy``
+
+Example:
+
+.. code-block:: c++
+
+  /*!
+   * \param destination Pointer to the destination array where the content is to be copied
+   * \param source Pointer to the source of data to be copied
+   * \param num Number of bytes to copy
+   */
+  memcpy(destination, source, num);
+
+becomes
+
+.. code-block:: c++
+
+  /*!
+   * \param destination Pointer to the destination array where the content is to be copied
+   * \param source Pointer to the source of data to be copied
+   * \param num Number of bytes to copy
+   */
+  std::copy(source, source + (num / sizeof *source), destination);
+
+Bytes to iterator conversion
+
+
+Unlike ``std::copy`` that take an iterator on the last element of the source array, ``memcpy`` request the number of bytes to copy.
+In order to make the check working, it will convert the size parameter to an iterator by replacing it by ``source + (num / sizeof *source)``
+
+Include
+---
+
+``std::copy`` being provided by the ``algorithm`` include file, this check will include it if needed.
+  
+Options
+---
+
+.. option:: IncludeStyle
+
+   A string specifying which include-style is used, `llvm` or `google`. Default
+   is `llvm`.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -211,6 +211,7 @@
modernize-raw-string-literal
modernize-redundant-void-arg
modernize-replace-auto-ptr
+   modernize-replace-memcpy-by-stdcopy
modernize-replace-random-shuffle
modernize-return-braced-init-list
modernize-shrink-to-fit
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -186,6 +186,11 @@
   finds and replaces cases that match the pattern ``var &&
   isa(var)``, where ``var`` is evaluated twice.
 
+- New :doc:`modernize-replace-memcpy-by-stdcopy
+  ` check.
+  
+  Replaces all occurrences of the C ``memcpy`` function by ``std::copy``.
+
 - New :doc:`modernize-use-trailing-return-type
   ` check.
 
Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
@@ -0,0 +1,48 @@
+//===--- ReplaceMemcpyByStdCopy.h - clang-tidy*- 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
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Replace the C memcpy function by std::copy
+class ReplaceMemcpyByStdCopy : public ClangTidyCheck {
+public:
+  ReplaceMemcpyByStdCopy(StringRef Name, ClangTidyContext *Context);
+  ~ReplaceMemcpyByStdCopy() override = default;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Options) override;
+
+private:
+ 

r363687 - [CodeGen][ARM] Fix FP16 vector coercion

[Mikhail Maltsev via cfe-commits]

Author: miyuki
Date: Tue Jun 18 07:34:27 2019
New Revision: 363687

URL: http://llvm.org/viewvc/llvm-project?rev=363687&view=rev
Log:
[CodeGen][ARM] Fix FP16 vector coercion

Summary:
When a function argument or return type is a homogeneous aggregate
which contains an FP16 vector but the target does not support FP16
operations natively, the type must be converted into an array of
integer vectors by then front end (otherwise LLVM will handle FP16
vectors incorrectly by scalarizing them and promoting FP16 to float,
see https://reviews.llvm.org/D50507).

Currently the logic for checking whether or not a given homogeneous
aggregate contains FP16 vectors is incorrect: it only looks at the
type of the first vector.

This patch fixes the issue by adding a new method
ARMABIInfo::containsAnyFP16Vectors and using it. The traversal logic
of this method is largely the same as in
ABIInfo::isHomogeneousAggregate.

Reviewers: eli.friedman, olista01, ostannard

Reviewed By: ostannard

Subscribers: ostannard, john.brawn, javed.absar, kristof.beyls, pbarrio, 
cfe-commits

Tags: #clang

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

Added:
cfe/trunk/test/CodeGen/arm-vfp16-arguments2.cpp
Modified:
cfe/trunk/lib/CodeGen/TargetInfo.cpp

Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=363687&r1=363686&r2=363687&view=diff
==
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Tue Jun 18 07:34:27 2019
@@ -5607,6 +5607,7 @@ private:
   uint64_t Members) const;
   ABIArgInfo coerceIllegalVector(QualType Ty) const;
   bool isIllegalVectorType(QualType Ty) const;
+  bool containsAnyFP16Vectors(QualType Ty) const;
 
   bool isHomogeneousAggregateBaseType(QualType Ty) const override;
   bool isHomogeneousAggregateSmallEnough(const Type *Ty,
@@ -5806,9 +5807,7 @@ ABIArgInfo ARMABIInfo::classifyHomogeneo
   // Base can be a floating-point or a vector.
   if (const VectorType *VT = Base->getAs()) {
 // FP16 vectors should be converted to integer vectors
-if (!getTarget().hasLegalHalfType() &&
-(VT->getElementType()->isFloat16Type() ||
-  VT->getElementType()->isHalfType())) {
+if (!getTarget().hasLegalHalfType() && containsAnyFP16Vectors(Ty)) {
   uint64_t Size = getContext().getTypeSize(VT);
   llvm::Type *NewVecTy = llvm::VectorType::get(
   llvm::Type::getInt32Ty(getVMContext()), Size / 32);
@@ -6169,6 +6168,37 @@ bool ARMABIInfo::isIllegalVectorType(Qua
   return false;
 }
 
+/// Return true if a type contains any 16-bit floating point vectors
+bool ARMABIInfo::containsAnyFP16Vectors(QualType Ty) const {
+  if (const ConstantArrayType *AT = getContext().getAsConstantArrayType(Ty)) {
+uint64_t NElements = AT->getSize().getZExtValue();
+if (NElements == 0)
+  return false;
+return containsAnyFP16Vectors(AT->getElementType());
+  } else if (const RecordType *RT = Ty->getAs()) {
+const RecordDecl *RD = RT->getDecl();
+
+// If this is a C++ record, check the bases first.
+if (const CXXRecordDecl *CXXRD = dyn_cast(RD))
+  if (llvm::any_of(CXXRD->bases(), [this](const CXXBaseSpecifier &B) {
+return containsAnyFP16Vectors(B.getType());
+  }))
+return true;
+
+if (llvm::any_of(RD->fields(), [this](FieldDecl *FD) {
+  return FD && containsAnyFP16Vectors(FD->getType());
+}))
+  return true;
+
+return false;
+  } else {
+if (const VectorType *VT = Ty->getAs())
+  return (VT->getElementType()->isFloat16Type() ||
+  VT->getElementType()->isHalfType());
+return false;
+  }
+}
+
 bool ARMABIInfo::isLegalVectorTypeForSwift(CharUnits vectorSize,
llvm::Type *eltTy,
unsigned numElts) const {

Added: cfe/trunk/test/CodeGen/arm-vfp16-arguments2.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/arm-vfp16-arguments2.cpp?rev=363687&view=auto
==
--- cfe/trunk/test/CodeGen/arm-vfp16-arguments2.cpp (added)
+++ cfe/trunk/test/CodeGen/arm-vfp16-arguments2.cpp Tue Jun 18 07:34:27 2019
@@ -0,0 +1,63 @@
+// RUN: %clang_cc1 -triple armv7a--none-eabi -target-abi aapcs \
+// RUN:   -mfloat-abi soft -target-feature +neon -emit-llvm -o - -O1 %s \
+// RUN:   | FileCheck %s --check-prefix=CHECK-SOFT
+// RUN: %clang_cc1 -triple armv7a--none-eabi -target-abi aapcs \
+// RUN:   -mfloat-abi hard -target-feature +neon -emit-llvm -o - -O1 %s \
+// RUN:   | FileCheck %s --check-prefix=CHECK-HARD
+// RUN: %clang_cc1 -triple armv7a--none-eabi -target-abi aapcs \
+// RUN:   -mfloat-abi hard -target-feature +neon -target-feature +fullfp16 \
+// RUN:   -emit-llvm -o - -O1 %s \
+// RUN:   | FileCheck %s 

[PATCH] D63437: [CodeGen][ARM] Fix FP16 vector coercion

[Mikhail Maltsev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363687: [CodeGen][ARM] Fix FP16 vector coercion (authored by 
miyuki, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63437?vs=205088&id=205342#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63437

Files:
  cfe/trunk/lib/CodeGen/TargetInfo.cpp
  cfe/trunk/test/CodeGen/arm-vfp16-arguments2.cpp

Index: cfe/trunk/lib/CodeGen/TargetInfo.cpp
===
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp
@@ -5607,6 +5607,7 @@
   uint64_t Members) const;
   ABIArgInfo coerceIllegalVector(QualType Ty) const;
   bool isIllegalVectorType(QualType Ty) const;
+  bool containsAnyFP16Vectors(QualType Ty) const;
 
   bool isHomogeneousAggregateBaseType(QualType Ty) const override;
   bool isHomogeneousAggregateSmallEnough(const Type *Ty,
@@ -5806,9 +5807,7 @@
   // Base can be a floating-point or a vector.
   if (const VectorType *VT = Base->getAs()) {
 // FP16 vectors should be converted to integer vectors
-if (!getTarget().hasLegalHalfType() &&
-(VT->getElementType()->isFloat16Type() ||
-  VT->getElementType()->isHalfType())) {
+if (!getTarget().hasLegalHalfType() && containsAnyFP16Vectors(Ty)) {
   uint64_t Size = getContext().getTypeSize(VT);
   llvm::Type *NewVecTy = llvm::VectorType::get(
   llvm::Type::getInt32Ty(getVMContext()), Size / 32);
@@ -6169,6 +6168,37 @@
   return false;
 }
 
+/// Return true if a type contains any 16-bit floating point vectors
+bool ARMABIInfo::containsAnyFP16Vectors(QualType Ty) const {
+  if (const ConstantArrayType *AT = getContext().getAsConstantArrayType(Ty)) {
+uint64_t NElements = AT->getSize().getZExtValue();
+if (NElements == 0)
+  return false;
+return containsAnyFP16Vectors(AT->getElementType());
+  } else if (const RecordType *RT = Ty->getAs()) {
+const RecordDecl *RD = RT->getDecl();
+
+// If this is a C++ record, check the bases first.
+if (const CXXRecordDecl *CXXRD = dyn_cast(RD))
+  if (llvm::any_of(CXXRD->bases(), [this](const CXXBaseSpecifier &B) {
+return containsAnyFP16Vectors(B.getType());
+  }))
+return true;
+
+if (llvm::any_of(RD->fields(), [this](FieldDecl *FD) {
+  return FD && containsAnyFP16Vectors(FD->getType());
+}))
+  return true;
+
+return false;
+  } else {
+if (const VectorType *VT = Ty->getAs())
+  return (VT->getElementType()->isFloat16Type() ||
+  VT->getElementType()->isHalfType());
+return false;
+  }
+}
+
 bool ARMABIInfo::isLegalVectorTypeForSwift(CharUnits vectorSize,
llvm::Type *eltTy,
unsigned numElts) const {
Index: cfe/trunk/test/CodeGen/arm-vfp16-arguments2.cpp
===
--- cfe/trunk/test/CodeGen/arm-vfp16-arguments2.cpp
+++ cfe/trunk/test/CodeGen/arm-vfp16-arguments2.cpp
@@ -0,0 +1,63 @@
+// RUN: %clang_cc1 -triple armv7a--none-eabi -target-abi aapcs \
+// RUN:   -mfloat-abi soft -target-feature +neon -emit-llvm -o - -O1 %s \
+// RUN:   | FileCheck %s --check-prefix=CHECK-SOFT
+// RUN: %clang_cc1 -triple armv7a--none-eabi -target-abi aapcs \
+// RUN:   -mfloat-abi hard -target-feature +neon -emit-llvm -o - -O1 %s \
+// RUN:   | FileCheck %s --check-prefix=CHECK-HARD
+// RUN: %clang_cc1 -triple armv7a--none-eabi -target-abi aapcs \
+// RUN:   -mfloat-abi hard -target-feature +neon -target-feature +fullfp16 \
+// RUN:   -emit-llvm -o - -O1 %s \
+// RUN:   | FileCheck %s --check-prefix=CHECK-FULL
+
+typedef float float32_t;
+typedef __fp16 float16_t;
+typedef __attribute__((neon_vector_type(2))) float32_t float32x2_t;
+typedef __attribute__((neon_vector_type(4))) float16_t float16x4_t;
+
+struct S1 {
+  float32x2_t M1;
+  float16x4_t M2;
+};
+
+struct B1 { float32x2_t M; };
+struct B2 { float16x4_t M; };
+
+struct S2 : B1, B2 {};
+
+struct S3 : B1 {
+  float16x4_t M;
+};
+
+struct S4 : B1 {
+  B2 M[1];
+};
+
+// S5 does not contain any FP16 vectors
+struct S5 : B1 {
+  B1 M[1];
+};
+
+// CHECK-SOFT: define void @_Z2f12S1(%struct.S1* noalias nocapture sret %agg.result, [2 x i64] %s1.coerce)
+// CHECK-HARD: define arm_aapcs_vfpcc [2 x <2 x i32>] @_Z2f12S1([2 x <2 x i32>] returned %s1.coerce)
+// CHECK-FULL: define arm_aapcs_vfpcc %struct.S1 @_Z2f12S1(%struct.S1 returned %s1.coerce)
+struct S1 f1(struct S1 s1) { return s1; }
+
+// CHECK-SOFT: define void @_Z2f22S2(%struct.S2* noalias nocapture sret %agg.result, [4 x i32] %s2.coerce)
+// CHECK-HARD: define arm_aapcs_vfpcc [2 x <2 x i32>] @_Z2f22S2([2 x <2 x i32>] returned %s2.coerce)
+// CHECK-FULL: define arm_aapcs_vf

[PATCH] D63299: [Clang] Parse GNU fallthrough attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D63299#1548306 , @xbolva00 wrote:

> Ok, If we peek a few tokens ahead and check attribute if it is stmt attribute 
> and then we call MaybeParseGNUAttr - this probably would work but are you 
> fine with it as a workaround ?


That still seems like the same spooky action at a distance because it means a 
statement vs declaration attribute will change the parsing behavior.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63299



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

[PATCH] D63088: [clang-tidy] misc-unused-parameters: don't comment out parameter name for C code

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM!

Funny enough, I've got a paper out for WG14 to try to relax this for C2x: 
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2381.pdf


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63088



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

[PATCH] D63260: [Attr] Support _attribute__ ((fallthrough))

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

xbolva00 updated this revision to Diff 205349.
xbolva00 added a comment.

Addressed notes. Thanks.


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

https://reviews.llvm.org/D63260

Files:
  include/clang/Basic/Attr.td
  lib/Sema/AnalysisBasedWarnings.cpp
  test/Sema/fallthrough-attr.c
  test/SemaCXX/switch-implicit-fallthrough.cpp

Index: test/SemaCXX/switch-implicit-fallthrough.cpp
===
--- test/SemaCXX/switch-implicit-fallthrough.cpp
+++ test/SemaCXX/switch-implicit-fallthrough.cpp
@@ -329,3 +329,15 @@
   }
   return n;
 }
+
+int fallthrough_attribute_spelling(int n) {
+  switch (n) {
+  case 0:
+n++;
+__attribute__((fallthrough));
+  case 1:
+n++;
+break;
+  }
+  return n;
+}
Index: test/Sema/fallthrough-attr.c
===
--- test/Sema/fallthrough-attr.c
+++ test/Sema/fallthrough-attr.c
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -fsyntax-only -std=gnu89 -verify -Wimplicit-fallthrough %s
+// RUN: %clang_cc1 -fsyntax-only -std=gnu99 -verify -Wimplicit-fallthrough %s
+// RUN: %clang_cc1 -fsyntax-only -std=c99 -verify -Wimplicit-fallthrough %s
+// RUN: %clang_cc1 -fsyntax-only -std=c11 -verify -Wimplicit-fallthrough %s
+// RUN: %clang_cc1 -fsyntax-only -std=c2x -DC2X -verify -Wimplicit-fallthrough %s
+
+int fallthrough_attribute_spelling(int n) {
+  switch (n) {
+  case 0:
+n++;
+  case 1: 
+#if defined(C2X)
+// expected-warning@-2{{unannotated fall-through between switch labels}} expected-note@-2{{insert '[[fallthrough]];' to silence this warning}} expected-note@-2{{insert 'break;' to avoid fall-through}}
+#else
+// expected-warning@-4{{unannotated fall-through between switch labels}} expected-note@-4{{insert '__attribute__((fallthrough));' to silence this warning}} expected-note@-4{{insert 'break;' to avoid fall-through}}
+#endif
+n++;
+__attribute__((fallthrough));
+  case 2:
+n++;
+break;
+  }
+  return n;
+}
Index: lib/Sema/AnalysisBasedWarnings.cpp
===
--- lib/Sema/AnalysisBasedWarnings.cpp
+++ lib/Sema/AnalysisBasedWarnings.cpp
@@ -1215,7 +1215,7 @@
 tok::r_square, tok::r_square
   };
 
-  bool PreferClangAttr = !PP.getLangOpts().CPlusPlus17;
+  bool PreferClangAttr = !PP.getLangOpts().CPlusPlus17 && !PP.getLangOpts().C2x;
 
   StringRef MacroName;
   if (PreferClangAttr)
@@ -1224,24 +1224,19 @@
 MacroName = PP.getLastMacroWithSpelling(Loc, FallthroughTokens);
   if (MacroName.empty() && !PreferClangAttr)
 MacroName = PP.getLastMacroWithSpelling(Loc, ClangFallthroughTokens);
-  if (MacroName.empty())
-MacroName = PreferClangAttr ? "[[clang::fallthrough]]" : "[[fallthrough]]";
+  if (MacroName.empty()) {
+if (!PreferClangAttr)
+  MacroName = "[[fallthrough]]";
+else if (PP.getLangOpts().CPlusPlus)
+  MacroName = "[[clang::fallthrough]]";
+else
+  MacroName = "__attribute__((fallthrough))";
+  }
   return MacroName;
 }
 
 static void DiagnoseSwitchLabelsFallthrough(Sema &S, AnalysisDeclContext &AC,
 bool PerFunction) {
-  // Only perform this analysis when using [[]] attributes. There is no good
-  // workflow for this warning when not using C++11. There is no good way to
-  // silence the warning (no attribute is available) unless we are using
-  // [[]] attributes. One could use pragmas to silence the warning, but as a
-  // general solution that is gross and not in the spirit of this warning.
-  //
-  // NOTE: This an intermediate solution. There are on-going discussions on
-  // how to properly support this warning outside of C++11 with an annotation.
-  if (!AC.getASTContext().getLangOpts().DoubleSquareBracketAttributes)
-return;
-
   FallthroughMapper FM(S);
   FM.TraverseStmt(AC.getBody());
 
@@ -1281,25 +1276,24 @@
   SourceLocation L = Label->getBeginLoc();
   if (L.isMacroID())
 continue;
-  if (S.getLangOpts().CPlusPlus11) {
-const Stmt *Term = B->getTerminatorStmt();
-// Skip empty cases.
-while (B->empty() && !Term && B->succ_size() == 1) {
-  B = *B->succ_begin();
-  Term = B->getTerminatorStmt();
-}
-if (!(B->empty() && Term && isa(Term))) {
-  Preprocessor &PP = S.getPreprocessor();
-  StringRef AnnotationSpelling = getFallthroughAttrSpelling(PP, L);
-  SmallString<64> TextToInsert(AnnotationSpelling);
-  TextToInsert += "; ";
-  S.Diag(L, diag::note_insert_fallthrough_fixit) <<
-  AnnotationSpelling <<
-  FixItHint::CreateInsertion(L, TextToInsert);
-}
+  const Stmt *Term = B->getTerminatorStmt();
+  // Skip empty cases.
+  while (B->empty() && !Term && B->succ_size() == 1) {
+B = *B->succ_begin();
+Term = B->getTerminatorStmt();
+  }
+  if (!(B->empty() && Term && isa(T

[PATCH] D62954: [Syntax] Add a helper to find expansion by its first spelled token

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 205352.
ilya-biryukov marked 7 inline comments as done.
ilya-biryukov added a comment.

- Address comments.
- s/findExpansion/expansionStartingAt.
- Add tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62954

Files:
  clang/include/clang/Tooling/Syntax/Tokens.h
  clang/lib/Tooling/Syntax/Tokens.cpp
  clang/unittests/Tooling/Syntax/TokensTest.cpp

Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- clang/unittests/Tooling/Syntax/TokensTest.cpp
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -55,6 +55,7 @@
 using ::testing::AllOf;
 using ::testing::Contains;
 using ::testing::ElementsAre;
+using ::testing::Field;
 using ::testing::Matcher;
 using ::testing::Not;
 using ::testing::StartsWith;
@@ -65,6 +66,13 @@
 MATCHER_P(SameRange, A, "") {
   return A.begin() == arg.begin() && A.end() == arg.end();
 }
+
+Matcher
+IsExpansion(Matcher> Spelled,
+Matcher> Expanded) {
+  return AllOf(Field(&TokenBuffer::Expansion::Spelled, Spelled),
+   Field(&TokenBuffer::Expansion::Expanded, Expanded));
+}
 // Matchers for syntax::Token.
 MATCHER_P(Kind, K, "") { return arg.kind() == K; }
 MATCHER_P2(HasText, Text, SourceMgr, "") {
@@ -629,6 +637,76 @@
   ValueIs(SameRange(findSpelled("not_mapped";
 }
 
+TEST_F(TokenBufferTest, ExpansionStartingAt) {
+  // Object-like macro expansions.
+  recordTokens(R"cpp(
+#define FOO 3+4
+int a = FOO 1;
+int b = FOO 2;
+  )cpp");
+
+  llvm::ArrayRef Foo1 = findSpelled("FOO 1").drop_back();
+  EXPECT_THAT(
+  Buffer.expansionStartingAt(Foo1.data()),
+  ValueIs(IsExpansion(SameRange(Foo1),
+  SameRange(findExpanded("3 + 4 1").drop_back();
+
+  llvm::ArrayRef Foo2 = findSpelled("FOO 2").drop_back();
+  EXPECT_THAT(
+  Buffer.expansionStartingAt(Foo2.data()),
+  ValueIs(IsExpansion(SameRange(Foo2),
+  SameRange(findExpanded("3 + 4 2").drop_back();
+
+  // Function-like macro expansions.
+  recordTokens(R"cpp(
+#define ID(X) X
+int a = ID(1+2+3);
+int b = ID(ID(2+3+4));
+  )cpp");
+
+  llvm::ArrayRef ID1 = findSpelled("ID ( 1 + 2 + 3 )");
+  EXPECT_THAT(Buffer.expansionStartingAt(&ID1.front()),
+  ValueIs(IsExpansion(SameRange(ID1),
+  SameRange(findExpanded("1 + 2 + 3");
+  // Only the first spelled token should be found.
+  for (const auto &T : ID1.drop_front())
+EXPECT_EQ(Buffer.expansionStartingAt(&T), llvm::None);
+
+  llvm::ArrayRef ID2 = findSpelled("ID ( ID ( 2 + 3 + 4 ) )");
+  EXPECT_THAT(Buffer.expansionStartingAt(&ID2.front()),
+  ValueIs(IsExpansion(SameRange(ID2),
+  SameRange(findExpanded("2 + 3 + 4");
+  // Only the first spelled token should be found.
+  for (const auto &T : ID2.drop_front())
+EXPECT_EQ(Buffer.expansionStartingAt(&T), llvm::None);
+
+  // PP directives.
+  recordTokens(R"cpp(
+#define FOO 1
+int a = FOO;
+#pragma once
+int b = 1;
+  )cpp");
+
+  llvm::ArrayRef DefineFoo = findSpelled("# define FOO 1");
+  EXPECT_THAT(
+  Buffer.expansionStartingAt(&DefineFoo.front()),
+  ValueIs(IsExpansion(SameRange(DefineFoo),
+  SameRange(findExpanded("int a").take_front(0);
+  // Only the first spelled token should be found.
+  for (const auto &T : DefineFoo.drop_front())
+EXPECT_EQ(Buffer.expansionStartingAt(&T), llvm::None);
+
+  llvm::ArrayRef PragmaOnce = findSpelled("# pragma once");
+  EXPECT_THAT(
+  Buffer.expansionStartingAt(&PragmaOnce.front()),
+  ValueIs(IsExpansion(SameRange(PragmaOnce),
+  SameRange(findExpanded("int b").take_front(0);
+  // Only the first spelled token should be found.
+  for (const auto &T : PragmaOnce.drop_front())
+EXPECT_EQ(Buffer.expansionStartingAt(&T), llvm::None);
+}
+
 TEST_F(TokenBufferTest, TokensToFileRange) {
   addFile("./foo.h", "token_from_header");
   llvm::Annotations Code(R"cpp(
Index: clang/lib/Tooling/Syntax/Tokens.cpp
===
--- clang/lib/Tooling/Syntax/Tokens.cpp
+++ clang/lib/Tooling/Syntax/Tokens.cpp
@@ -199,6 +199,32 @@
   : LastSpelled + 1);
 }
 
+llvm::Optional
+TokenBuffer::expansionStartingAt(const syntax::Token *Spelled) const {
+  assert(Spelled);
+  assert(Spelled->location().isFileID() && "not a spelled token");
+  auto FileIt = Files.find(SourceMgr->getFileID(Spelled->location()));
+  assert(FileIt != Files.end());
+
+  auto &File = FileIt->second;
+  assert(File.SpelledTokens.data() <= Spelled &&
+ Spelled < (File.SpelledTokens.data() + File.SpelledTokens.size()));
+
+  unsigned SpelledIndex = Spelled - File.SpelledTokens.data();
+  auto M = llvm::bsearch(File.Mappings, [&](const Mapping &M) {
+r

[PATCH] D62954: [Syntax] Add a helper to find expansion by its first spelled token

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:209
+  ///#define FOO 1 2 3 // Expansion from "#define FOO 1" to an empty range.
+  ///FOO   // Expansion from "FOO" to "1 2 3".
+  struct Expansion {

sammccall wrote:
> also mention #include?
Added an example and a FIXME mentioning this does not work yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62954



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

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

[Thomas Manceau via Phabricator via cfe-commits]

Blackhart updated this revision to Diff 205354.
Blackhart marked 4 inline comments as done.

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

https://reviews.llvm.org/D63324

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/modernize-replace-memcpy-by-stdcopy.rst

Index: clang-tools-extra/docs/clang-tidy/checks/modernize-replace-memcpy-by-stdcopy.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/modernize-replace-memcpy-by-stdcopy.rst
@@ -0,0 +1,47 @@
+.. title:: clang-tidy - modernize-replace-memcpy-by-stdcopy
+
+modernize-replace-memcpy-by-stdcopy
+===
+
+Replaces all occurrences of the C ``memcpy`` function by ``std::copy``
+
+Example:
+
+.. code-block:: c++
+
+  /*!
+   * \param destination Pointer to the destination array where the content is to be copied
+   * \param source Pointer to the source of data to be copied
+   * \param num Number of bytes to copy
+   */
+  memcpy(destination, source, num);
+
+becomes
+
+.. code-block:: c++
+
+  /*!
+   * \param destination Pointer to the destination array where the content is to be copied
+   * \param source Pointer to the source of data to be copied
+   * \param num Number of bytes to copy
+   */
+  std::copy(source, source + (num / sizeof *source), destination);
+
+Bytes to iterator conversion
+
+
+Unlike ``std::copy`` that take an iterator on the last element of the source array, ``memcpy`` request the number of bytes to copy.
+In order to make the check working, it will convert the size parameter to an iterator by replacing it by ``source + (num / sizeof *source)``
+
+Header inclusion
+
+
+``std::copy`` being provided by the ``algorithm`` header file, this check will include it if needed.
+  
+Options
+---
+
+.. option:: IncludeStyle
+
+   A string specifying which include-style is used, `llvm` or `google`. Default
+   is `llvm`.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -211,6 +211,7 @@
modernize-raw-string-literal
modernize-redundant-void-arg
modernize-replace-auto-ptr
+   modernize-replace-memcpy-by-stdcopy
modernize-replace-random-shuffle
modernize-return-braced-init-list
modernize-shrink-to-fit
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -186,6 +186,11 @@
   finds and replaces cases that match the pattern ``var &&
   isa(var)``, where ``var`` is evaluated twice.
 
+- New :doc:`modernize-replace-memcpy-by-stdcopy
+  ` check.
+  
+  Replaces all occurrences of the C ``memcpy`` function by ``std::copy``.
+
 - New :doc:`modernize-use-trailing-return-type
   ` check.
 
Index: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h
@@ -0,0 +1,48 @@
+//===--- ReplaceMemcpyByStdCopy.h - clang-tidy*- 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
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACE_MEMCPY_BY_STDCOPY_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Replace the C memcpy function by std::copy
+class ReplaceMemcpyByStdCopy : public ClangTidyCheck {
+public:
+  ReplaceMemcpyByStdCopy(StringRef Name, ClangTidyContext *Context);
+  ~ReplaceMemcpyByStdCopy() override = default;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Options) override;
+
+private:
+  void renameFunction(DiagnosticBuilder &Diag, const CallExpr *MemcpyNode

[PATCH] D63426: [clangd] Narrow rename to local symbols.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 205355.
hokein marked 5 inline comments as done.
hokein added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63426

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/refactor/Rename.h
  clang-tools-extra/clangd/refactor/tweaks/DumpAST.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp
  clang/include/clang/Tooling/Refactoring/Rename/RenamingAction.h
  clang/lib/Tooling/Refactoring/Rename/RenamingAction.cpp

Index: clang/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
===
--- clang/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
+++ clang/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
@@ -74,6 +74,8 @@
std::move(NewName));
 }
 
+const NamedDecl *RenameOccurrences::getRenameDecl() const { return ND; }
+
 Expected
 RenameOccurrences::createSourceReplacements(RefactoringRuleContext &Context) {
   Expected Occurrences = findSymbolOccurrences(ND, Context);
Index: clang/include/clang/Tooling/Refactoring/Rename/RenamingAction.h
===
--- clang/include/clang/Tooling/Refactoring/Rename/RenamingAction.h
+++ clang/include/clang/Tooling/Refactoring/Rename/RenamingAction.h
@@ -54,6 +54,8 @@
 
   static const RefactoringDescriptor &describe();
 
+  const NamedDecl *getRenameDecl() const;
+
 private:
   RenameOccurrences(const NamedDecl *ND, std::string NewName)
   : ND(ND), NewName(std::move(NewName)) {}
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -18,6 +18,10 @@
 namespace clangd {
 namespace {
 
+MATCHER_P2(RenameRange, Code, Range, "") {
+  return replacementToEdit(Code, arg).range == Range;
+}
+
 TEST(RenameTest, SingleFile) {
   struct Test {
 const char* Before;
@@ -87,6 +91,69 @@
   }
 }
 
+TEST(RenameTest, WithIndex) {
+  const char *Tests[] = {
+  R"cpp(
+class [[Foo]] {};
+void f() {
+  [[Fo^o]] b;
+}
+ )cpp",
+
+  R"cpp(
+void f(int [[Lo^cal]]) {
+  [[Local]] = 2;
+}
+  )cpp",
+
+  // Cases where we reject to do the rename.
+  R"cpp( // no symbol at the cursor
+// This is in comme^nt.
+   )cpp",
+
+  R"cpp(
+void f() { // Outside main file.
+  Out^side s;
+}
+  )cpp",
+  };
+  const char *Header = "class Outside {};";
+
+  TestTU OtherFile = TestTU::withCode("Outside s;");
+  OtherFile.HeaderCode = Header;
+  OtherFile.Filename = "other.cc";
+
+  for (const char *Test : Tests) {
+Annotations T(Test);
+TestTU MainFile;
+MainFile.Code = T.code();
+MainFile.HeaderCode = Header;
+auto AST = MainFile.build();
+// Build a fake index containing refs of MainFile and OtherFile
+auto OtherFileIndex = OtherFile.index();
+auto MainFileIndex = MainFile.index();
+auto Index = MergedIndex(MainFileIndex.get(), OtherFileIndex.get());
+
+auto Results = renameWithinFile(AST, testPath(MainFile.Filename), T.point(),
+"dummyNewName", &Index);
+bool WantRename = true;
+if (T.ranges().empty())
+  WantRename = false;
+if (!WantRename) {
+  EXPECT_FALSE(Results) << "expected renameWithinFile returned an error: "
+<< T.code();
+  llvm::consumeError(Results.takeError());
+} else {
+  EXPECT_TRUE(bool(Results)) << "renameWithinFile returned an error: "
+ << llvm::toString(Results.takeError());
+  std::vector> Expected;
+  for (const auto &R : T.ranges())
+Expected.push_back(RenameRange(MainFile.Code, R));
+  EXPECT_THAT(*Results, UnorderedElementsAreArray(Expected));
+}
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/refactor/tweaks/DumpAST.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DumpAST.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DumpAST.cpp
@@ -94,7 +94,7 @@
   Intent intent() const override { return Info; }
   bool hidden() const override { return true; }
 };
-REGISTER_TWEAK(ShowSelectionTree);
+REGISTER_TWEAK(ShowSelectionTree)
 
 /// Shows the layout of the RecordDecl under the cursor.
 /// Input:
@@ -132,7 +132,7 @@
 private:
   const RecordDecl *Record = nullptr;
 };
-REGISTER_TWEAK(DumpRecordLayout);
+REGISTER_TWEAK(DumpRecordLayout)
 
 } // namespace
 } // namespace clangd
Index: clang-tools-extra/clangd/refactor/Rename.h
===
--- 

[PATCH] D63426: [clangd] Narrow rename to local symbols.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:83
+// FIXME: we may skip querying the index if D is function-local.
+const NamedDecl *D =
+clang::tooling::getNamedDeclAt(ASTCtx, SourceLocationBeg);

sammccall wrote:
> sammccall wrote:
> > it's unfortunate that our "what's under the cursor" logic here has to 
> > duplicate what's done by RenameOccurrences::initiate().
> > 
> > Can we have RenameOccurrences expose the NamedDecl instead?
> does this not work for macros?
unfortunately, the rename library doesn't rename marcos.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:85
+clang::tooling::getNamedDeclAt(ASTCtx, SourceLocationBeg);
+if (!D)
+  return llvm::make_error(

sammccall wrote:
> We're effectively doing this test twice (see `if (!Rename)` below) and 
> emitting different error mesages
> 
> maybe we should just skip the rest of the check in this case?
> (Exposing the ND from the RenameOccurrences would help make this duplication 
> more obvious)
The duplication doesn't exist if we expose the ND from the RenameOccurrence.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63426



___
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

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.



Comment at: clang/test/SemaSYCL/device-attributes.cpp:3
+
+[[clang::sycl_kernel]] int gv2 = 0; // expected-warning {{'sycl_kernel' 
attribute only applies to functions}}
+__attribute((sycl_kernel)) int gv3 = 0; // expected-warning {{'sycl_kernel' 
attribute only applies to functions}}

aaron.ballman wrote:
> I'd like to see some more tests covering less obvious scenarios. Can I add 
> this attribute to a lambda? What about a member function? How does it work 
> with virtual functions? That sort of thing.
Actually there is no restrictions for adding this attribute to any function to 
outline device code so I just checked the simplest variant.

But I'm working on new patch which will put some requirements on function which 
is marked with `sycl_kernel` attribute. 
This new patch will add generation of OpenCL kernel from function marked with 
`sycl_kernel` attribute. The main idea of this approach is described in this [[ 
https://github.com/intel/llvm/blob/sycl/sycl/doc/SYCL_compiler_and_runtime_design.md#lowering-of-lambda-function-objects-and-named-function-objects
 | document ]] (in this document generated kernel is called "kernel wrapper").
And to be able to generate OpenCL kernel using function marked with 
`sycl_kernel` attribute we put some requirements on this function, for example 
it must be a template function. You can find these requirements and example of 
proper function which can be marked with `sycl_kernel` in this [[ 
https://github.com/intel/llvm/pull/177#discussion_r290451286 | comment ]] .




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

[clang-tools-extra] r363691 - [clangd] Return vector from applyTweak. NFC

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Tue Jun 18 08:15:41 2019
New Revision: 363691

URL: http://llvm.org/viewvc/llvm-project?rev=363691&view=rev
Log:
[clangd] Return vector from applyTweak. NFC

For the same reasons as r363150, which got overwritten by changes in
r363680.

Sending without review to unbreak our integrate.

Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=363691&r1=363690&r2=363691&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Tue Jun 18 08:15:41 2019
@@ -491,14 +491,14 @@ void ClangdLSPServer::onCommand(const Ex
 
 auto Action = [this, ApplyEdit](decltype(Reply) Reply, URIForFile File,
 std::string Code,
-llvm::Expected R) {
+llvm::Expected R) {
   if (!R)
 return Reply(R.takeError());
 
   if (R->ApplyEdit) {
 WorkspaceEdit WE;
 WE.changes.emplace();
-(*WE.changes)[File.uri()] = replacementsToEdits(Code, *R->ApplyEdit);
+(*WE.changes)[File.uri()] = *R->ApplyEdit;
 ApplyEdit(std::move(WE));
   }
   if (R->ShowMessage) {

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=363691&r1=363690&r2=363691&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Jun 18 08:15:41 2019
@@ -325,7 +325,7 @@ void ClangdServer::enumerateTweaks(PathR
 }
 
 void ClangdServer::applyTweak(PathRef File, Range Sel, StringRef TweakID,
-  Callback CB) {
+  Callback CB) {
   auto Action = [Sel](decltype(CB) CB, std::string File, std::string TweakID,
   Expected InpAST) {
 if (!InpAST)
@@ -348,7 +348,13 @@ void ClangdServer::applyTweak(PathRef Fi
 *Effect->ApplyEdit, Style))
 Effect->ApplyEdit = std::move(*Formatted);
 }
-return CB(std::move(*Effect));
+
+ResolvedEffect R;
+R.ShowMessage = std::move(Effect->ShowMessage);
+if (Effect->ApplyEdit)
+  R.ApplyEdit =
+  replacementsToEdits(InpAST->Inputs.Contents, *Effect->ApplyEdit);
+return CB(std::move(R));
   };
   WorkScheduler.runWithAST(
   "ApplyTweak", File,

Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=363691&r1=363690&r2=363691&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Tue Jun 18 08:15:41 2019
@@ -57,6 +57,14 @@ public:
 using ClangTidyOptionsBuilder = std::function;
 
+/// Like Tweak::Effect, but stores TextEdits instead of tooling::Replacements.
+/// Slightly nicer to embedders of ClangdServer.
+/// FIXME: figure out how to remove this duplication.
+struct ResolvedEffect {
+  llvm::Optional ShowMessage;
+  llvm::Optional> ApplyEdit;
+};
+
 /// Manages a collection of source files and derived data (ASTs, indexes),
 /// and provides language-aware features such as code completion.
 ///
@@ -239,7 +247,7 @@ public:
 
   /// Apply the code tweak with a specified \p ID.
   void applyTweak(PathRef File, Range Sel, StringRef ID,
-  Callback CB);
+  Callback CB);
 
   /// Only for testing purposes.
   /// Waits until all requests to worker thread are finished and dumps AST for


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

[PATCH] D63451: P0840R2: support for [[no_unique_address]] attribute

[Aaron Ballman via Phabricator via cfe-commits]

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

Can you also add some SemaCXX tests that ensure the attribute is properly 
diagnosed when written on a bit-field, a static data member, a function, is 
given an argument, etc?




Comment at: include/clang/AST/DeclCXX.h:337-341
 /// true when this class is empty for traits purposes,
-/// i.e. has no data members other than 0-width bit-fields, has no
+/// i.e. has no data members other than 0-width bit-fields and empty
+/// fields marked [[no_unique_address]], has no
 /// virtual function/base, and doesn't inherit from a non-empty
 /// class. Doesn't take union-ness into account.

Do you mind re-flowing this entire comment to 80 cols?



Comment at: lib/AST/Decl.cpp:3927
+  // -- is not of class type, or
+  auto *RT = getType()->getAs();
+  if (!RT)

`const auto *` (elsewhere in the function as well)



Comment at: lib/AST/Decl.cpp:3930
+return false;
+  auto *RD = RT->getDecl()->getDefinition();
+  if (!RD) {

I'd prefer to see a concrete type here rather than `auto`.



Comment at: lib/AST/RecordLayoutBuilder.cpp:1785
+if (PotentiallyOverlapping) {
+  auto &Layout = Context.getASTRecordLayout(FieldClass);
+  EffectiveFieldSize =

Don't use `auto` here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63451



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

[PATCH] D44865: [libc++] Implement P0608R3 - A sane variant converting constructor

[Zhihao Yuan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
lichray marked an inline comment as done.
Closed by commit rL363692: [libc++] Implement P0608R3 - A sane variant 
converting constructor (authored by lichray, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D44865

Files:
  libcxx/trunk/include/variant
  
libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp
  
libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/conv.fail.cpp
  
libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/T.pass.cpp
  
libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/conv.fail.cpp
  libcxx/trunk/www/cxx2a_status.html

Index: libcxx/trunk/www/cxx2a_status.html
===
--- libcxx/trunk/www/cxx2a_status.html
+++ libcxx/trunk/www/cxx2a_status.html
@@ -115,7 +115,7 @@
 	https://wg21.link/P0591R4";>P0591R4LWGUtility functions to implement uses-allocator constructionSan Diego 
 	https://wg21.link/P0595R2";>P0595R2CWGP0595R2 std::is_constant_evaluated()San DiegoComplete9.0
 	https://wg21.link/P0602R4";>P0602R4LWGvariant and optional should propagate copy/move trivialitySan DiegoComplete8.0
-	https://wg21.link/P0608R3";>P0608R3LWGA sane variant converting constructorSan Diego 
+	https://wg21.link/P0608R3";>P0608R3LWGA sane variant converting constructorSan DiegoComplete9.0
 	https://wg21.link/P0655R1";>P0655R1LWGvisit: Explicit Return Type for visitSan Diego 
 	https://wg21.link/P0771R1";>P0771R1LWGstd::function move constructor should be noexceptSan DiegoComplete6.0
 	https://wg21.link/P0896R4";>P0896R4LWGThe One Ranges ProposalSan Diego 
Index: libcxx/trunk/include/variant
===
--- libcxx/trunk/include/variant
+++ libcxx/trunk/include/variant
@@ -1098,11 +1098,39 @@
 template 
 struct __overload<_Tp, _Types...> : __overload<_Types...> {
   using __overload<_Types...>::operator();
-  __identity<_Tp> operator()(_Tp) const;
+
+  static auto __test(_Tp (&&)[1]) -> __identity<_Tp>;
+
+  template 
+  auto operator()(_Tp, _Up&& __t) const
+  -> decltype(__test({ _VSTD::forward<_Up>(__t) }));
 };
 
+template 
+struct __overload_bool : _Base {
+  using _Base::operator();
+
+  template >
+  auto operator()(bool, _Up&&) const
+  -> enable_if_t, __identity<_Tp>>;
+};
+
+template 
+struct __overload
+: __overload_bool<__overload<_Types...>, bool> {};
+template 
+struct __overload
+: __overload_bool<__overload<_Types...>, bool const> {};
+template 
+struct __overload
+: __overload_bool<__overload<_Types...>, bool volatile> {};
+template 
+struct __overload
+: __overload_bool<__overload<_Types...>, bool const volatile> {};
+
 template 
-using __best_match_t = typename result_of_t<__overload<_Types...>(_Tp&&)>::type;
+using __best_match_t =
+typename invoke_result_t<__overload<_Types...>, _Tp, _Tp>::type;
 
 } // __variant_detail
 
Index: libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/conv.fail.cpp
===
--- libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/conv.fail.cpp
+++ libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/conv.fail.cpp
@@ -0,0 +1,39 @@
+// -*- 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
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03, c++11, c++14
+
+// 
+
+// template  class variant;
+
+// template  constexpr variant(T&&) noexcept(see below);
+
+#include 
+#include 
+#include 
+
+int main(int, char**)
+{
+  std::variant v1 = 1; // expected-error {{no viable conversion}}
+  std::variant v2 = 1; // expected-error {{no viable conversion}}
+  std::variant v3 = 1; // expected-error {{no viable conversion}}
+
+  std::variant v4 = 1; // expected-error {{no viable conversion}}
+  std::variant v5 = 1; // expected-error {{no viable conversion}}
+  std::variant v6 = 1; // expected-error {{no viable conversion}}
+
+  std::variant v7 = "meow"; // expected-error {{no viable conversion}}
+  std::variant v8 = "meow"; // expected-error {{no viable conversion}}
+  std::variant v9 = "meow"; // expected-error {{no viable conversion}}
+
+  std::variant v10 = std::true_type(); // expected-error {{no viable conversion}}
+  std::variant v11 = std::unique_ptr(); // expected-error {{no viable conversion}}
+  std::variant v12 = nullptr; // expected-error {{no viable conversion}}
+}
Index: libcxx/trunk/test/std/utilit

[PATCH] D59744: Fix i386 ABI "__m64" type bug

[Mike Klein via Phabricator via cfe-commits]

mtklein added a comment.

Hey folks, I'm the Skia point of contact on this, and "luckily" the person who 
wrote all the code that got us into this mess.  Let me cross post a couple 
questions I've had from the Chromium bug over here where folks might know the 
answer...

Now that Clang's decided to match GCC's behavior of using mm0 to pass around 
8-byte vectors on x86-32, is there any way to use 8-byte vector types safely 
any more?  I don't really have the full context of this Clang change, but is it 
maybe a good idea applied to too many types?  I notice the change mentions 
__m64, but here I'm using uint16_t ext_vector_type(4) exclusively, never __m64 
or even an 8x8 vector... can we just squint and say u16x4 and __m64 aren't the 
same, passing __m64 according to the ABI but vector extensions however we were 
doing it before?  Or can we work out some sort of ABI that preserves st0/mm0?  
I think we're finding that even with forced-inlining, at -O0 we still end up 
getting u16x4 values stored in mm0 briefly (kind of roundabout through xmm 
registers and the stack once or twice too).

In short, should working with 4x u16 be safe on x86-32 and there's a bug / 
undefined behavior in my code leading to this  mm0/st0 clobber, or is this just 
actually not really spec'd to work?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59744



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

[PATCH] D54295: [RISCV] Add inline asm constraint A for RISC-V

[Alex Bradbury via Phabricator via cfe-commits]

asb accepted this revision.
asb added a comment.

LGTM, thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D54295



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

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

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

xbolva00 updated this revision to Diff 205364.
xbolva00 added a comment.

Addressed review notes. Thanks @aaron.ballman !


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

https://reviews.llvm.org/D63082

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/Analysis/misc-ps.c
  test/Sema/integer-overflow.c
  test/Sema/parentheses.c
  test/Sema/parentheses.cpp
  test/Sema/warn-int-in-bool-context.c
  test/Sema/warn-unreachable.c
  test/SemaCXX/constexpr-printing.cpp
  test/SemaCXX/integer-overflow.cpp
  test/SemaCXX/nested-name-spec.cpp
  test/SemaCXX/warn-int-in-bool-context.cpp
  test/SemaCXX/warn-unreachable.cpp

Index: test/SemaCXX/warn-unreachable.cpp
===
--- test/SemaCXX/warn-unreachable.cpp
+++ test/SemaCXX/warn-unreachable.cpp
@@ -186,13 +186,13 @@
 
 
 int test_enum_sizeof_arithmetic() {
-  if (BrownCow)
+  if (BrownCow) // expected-warning {{enum constant in boolean context}}
 return 1;
   return 2;
 }
 
 int test_enum_arithmetic() {
-  if (CowBrown)
+  if (CowBrown) // expected-warning {{enum constant in boolean context}}
 return 1;
   return 2; // expected-warning {{never be executed}}
 }
Index: test/SemaCXX/warn-int-in-bool-context.cpp
===
--- test/SemaCXX/warn-int-in-bool-context.cpp
+++ test/SemaCXX/warn-int-in-bool-context.cpp
@@ -0,0 +1,170 @@
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wint-in-bool-context %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wall %s
+
+#define ONE 1
+#define TWO 2
+
+enum answer {
+  foo,
+  bar,
+  yes,
+  no,
+  unknown,
+};
+
+enum fruit {
+  orange = -3,
+  lemon,
+  banana,
+  apple,
+};
+
+void g(bool b);
+
+template 
+void h() {}
+template 
+void h() {}
+
+bool test(int a, int b, enum answer ans, enum fruit f) {
+  bool r = TWO * 7; // expected-warning {{'*' in boolean context}}
+  r = 3 * 7;// expected-warning {{'*' in boolean context}}
+  r = a * 7;// expected-warning {{'*' in boolean context}}
+  r = a * b;// expected-warning {{'*' in boolean context}}
+  g(3 * 9); // expected-warning {{'*' in boolean context}}
+  h<3 * 9>();
+
+  r = TWO << 7; // expected-warning {{'<<' in boolean context, maybe you mean '<'?}}
+  r = a << 7;   // expected-warning {{'<<' in boolean context, maybe you mean '<'?}}
+  r = ONE << b; // expected-warning {{'<<' in boolean context, maybe you mean '<'?}}
+
+  r = a ? ONE : TWO; // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+  r = a ? (1) : TWO;
+  r = a ? 3 : TWO; // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+  r = a ? -2 : 0;
+  r = a ? 3 : -2;
+  r = a ? 0 : TWO; // expected-warning {{'?:' with integer constants in boolean context}}
+  r = a ? 3 : ONE; // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+  r = a ? ONE : 0;
+  r = a ? 0 : 0;
+  r = a ? ONE : 0;
+  r = a ? ONE : ONE;
+  g(a ? 3 : 9); // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+
+  r = a ? yes   // expected-warning {{enum constant in boolean context}}
+: no;   // expected-warning {{enum constant in boolean context}}
+  r = ans == yes || no; // expected-warning {{enum constant in boolean context}}
+  r = yes || ans == no; // expected-warning {{enum constant in boolean context}}
+  r = ans == yes || ans == no;
+  r = !foo;
+  r = !bar;
+  r = !yes;// expected-warning {{enum constant in boolean context}}
+  g(ans == yes || no); // expected-warning {{enum constant in boolean context}}
+
+  if (8 * 4) // expected-warning {{'*' in boolean context}}
+return a;
+
+  if (a * 4) // expected-warning {{'*' in boolean context}}
+return a;
+
+  if (-TWO * b) // expected-warning {{'*' in boolean context}}
+return a;
+
+  if (TWO << 4) // expected-warning {{'<<' in boolean context, maybe you mean '<'?}}
+return a;
+
+  if (a << TWO) // expected-warning {{'<<' in boolean context, maybe you mean '<'?}}
+return a;
+
+  if (ONE << b) // expected-warning {{'<<' in boolean context, maybe you mean '<'?}}
+return a;
+
+  if (a ? ONE : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+return a;
+
+  if (a ? 32 : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+return a;
+
+  if (a ? 7 : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+return a;
+
+  if (ans == yes || foo)
+return a;
+
+  if (f == apple || orange) // expected-warning {{enum c

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

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

xbolva00 updated this revision to Diff 205365.
xbolva00 added a comment.

Updated forgotten test file.


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

https://reviews.llvm.org/D63082

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/Analysis/misc-ps.c
  test/Sema/integer-overflow.c
  test/Sema/parentheses.c
  test/Sema/parentheses.cpp
  test/Sema/warn-int-in-bool-context.c
  test/Sema/warn-unreachable.c
  test/SemaCXX/constexpr-printing.cpp
  test/SemaCXX/integer-overflow.cpp
  test/SemaCXX/nested-name-spec.cpp
  test/SemaCXX/warn-int-in-bool-context.cpp
  test/SemaCXX/warn-unreachable.cpp

Index: test/SemaCXX/warn-unreachable.cpp
===
--- test/SemaCXX/warn-unreachable.cpp
+++ test/SemaCXX/warn-unreachable.cpp
@@ -186,13 +186,13 @@
 
 
 int test_enum_sizeof_arithmetic() {
-  if (BrownCow)
+  if (BrownCow) // expected-warning {{enum constant in boolean context}}
 return 1;
   return 2;
 }
 
 int test_enum_arithmetic() {
-  if (CowBrown)
+  if (CowBrown) // expected-warning {{enum constant in boolean context}}
 return 1;
   return 2; // expected-warning {{never be executed}}
 }
Index: test/SemaCXX/warn-int-in-bool-context.cpp
===
--- test/SemaCXX/warn-int-in-bool-context.cpp
+++ test/SemaCXX/warn-int-in-bool-context.cpp
@@ -0,0 +1,170 @@
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wint-in-bool-context %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wall %s
+
+#define ONE 1
+#define TWO 2
+
+enum answer {
+  foo,
+  bar,
+  yes,
+  no,
+  unknown,
+};
+
+enum fruit {
+  orange = -3,
+  lemon,
+  banana,
+  apple,
+};
+
+void g(bool b);
+
+template 
+void h() {}
+template 
+void h() {}
+
+bool test(int a, int b, enum answer ans, enum fruit f) {
+  bool r = TWO * 7; // expected-warning {{'*' in boolean context}}
+  r = 3 * 7;// expected-warning {{'*' in boolean context}}
+  r = a * 7;// expected-warning {{'*' in boolean context}}
+  r = a * b;// expected-warning {{'*' in boolean context}}
+  g(3 * 9); // expected-warning {{'*' in boolean context}}
+  h<3 * 9>();
+
+  r = TWO << 7; // expected-warning {{'<<' in boolean context, maybe you mean '<'?}}
+  r = a << 7;   // expected-warning {{'<<' in boolean context, maybe you mean '<'?}}
+  r = ONE << b; // expected-warning {{'<<' in boolean context, maybe you mean '<'?}}
+
+  r = a ? ONE : TWO; // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+  r = a ? (1) : TWO;
+  r = a ? 3 : TWO; // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+  r = a ? -2 : 0;
+  r = a ? 3 : -2;
+  r = a ? 0 : TWO; // expected-warning {{'?:' with integer constants in boolean context}}
+  r = a ? 3 : ONE; // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+  r = a ? ONE : 0;
+  r = a ? 0 : 0;
+  r = a ? ONE : 0;
+  r = a ? ONE : ONE;
+  g(a ? 3 : 9); // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+
+  r = a ? yes   // expected-warning {{enum constant in boolean context}}
+: no;   // expected-warning {{enum constant in boolean context}}
+  r = ans == yes || no; // expected-warning {{enum constant in boolean context}}
+  r = yes || ans == no; // expected-warning {{enum constant in boolean context}}
+  r = ans == yes || ans == no;
+  r = !foo;
+  r = !bar;
+  r = !yes;// expected-warning {{enum constant in boolean context}}
+  g(ans == yes || no); // expected-warning {{enum constant in boolean context}}
+
+  if (8 * 4) // expected-warning {{'*' in boolean context}}
+return a;
+
+  if (a * 4) // expected-warning {{'*' in boolean context}}
+return a;
+
+  if (-TWO * b) // expected-warning {{'*' in boolean context}}
+return a;
+
+  if (TWO << 4) // expected-warning {{'<<' in boolean context, maybe you mean '<'?}}
+return a;
+
+  if (a << TWO) // expected-warning {{'<<' in boolean context, maybe you mean '<'?}}
+return a;
+
+  if (ONE << b) // expected-warning {{'<<' in boolean context, maybe you mean '<'?}}
+return a;
+
+  if (a ? ONE : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+return a;
+
+  if (a ? 32 : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+return a;
+
+  if (a ? 7 : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+return a;
+
+  if (ans == yes || foo)
+return a;
+
+  if (f == apple || orange) // expected-warning {{enum constant in boolean 

[PATCH] D63299: [Clang] Parse GNU fallthrough attributes

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

xbolva00 abandoned this revision.
xbolva00 added a comment.

Right :(


Repository:
  rC Clang

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

https://reviews.llvm.org/D63299



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

[PATCH] D62952: [analyzer][tests] Use normalize_sarif in place of diff_sarif

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D62952#1548377 , @aaron.ballman 
wrote:

> In general, that seems reasonable, but I would prefer to take care of more of 
> the work in lit.local.cfg than have to deal with that atrocious RUN line in 
> every test case. Is there a way to retain a similarly succinct solution as 
> diff_sarif?


There'd be no atrocious RUN line if we went with modifying the expected files 
beforehand and having the tool output a newline.

The unchanged:

  // RUN: %clang_analyze_cc1 
-analyzer-checker=alpha.security.taint,debug.TaintTest %s -verify 
-analyzer-output=sarif -o - | %diff_sarif 
%S/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif -

becomes:

  // RUN: %clang_analyze_cc1 
-analyzer-checker=alpha.security.taint,debug.TaintTest %s -verify 
-analyzer-output=sarif -o - | %normalize_sarif | diff -U1 -b 
%S/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif -

As in, `%diff_sarif` gets replaced with `%normalize_sarif | diff -U1 -b` and 
that's it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62952



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

[PATCH] D62952: [analyzer][tests] Use normalize_sarif in place of diff_sarif

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D62952#1548580 , 
@hubert.reinterpretcast wrote:

> In D62952#1548377 , @aaron.ballman 
> wrote:
>
> > In general, that seems reasonable, but I would prefer to take care of more 
> > of the work in lit.local.cfg than have to deal with that atrocious RUN line 
> > in every test case. Is there a way to retain a similarly succinct solution 
> > as diff_sarif?
>
>
> There'd be no atrocious RUN line if we went with modifying the expected files 
> beforehand and having the tool output a newline.
>
> The unchanged:
>
>   // RUN: %clang_analyze_cc1 
> -analyzer-checker=alpha.security.taint,debug.TaintTest %s -verify 
> -analyzer-output=sarif -o - | %diff_sarif 
> %S/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif -
>
>
> becomes:
>
>   // RUN: %clang_analyze_cc1 
> -analyzer-checker=alpha.security.taint,debug.TaintTest %s -verify 
> -analyzer-output=sarif -o - | %normalize_sarif | diff -U1 -b 
> %S/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif -
>
>
> As in, `%diff_sarif` gets replaced with `%normalize_sarif | diff -U1 -b` and 
> that's it.


But is there a reason to not keep `%diff_sarif` and define it in terms of 
`%normalize_sarif | diff -U1 -b` within lit.local.cfg? I guess I don't see the 
benefit to exposing the call to diff (I don't anticipate anyone needing to 
change the options passed to diff).


Repository:
  rC Clang

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

https://reviews.llvm.org/D62952



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

[PATCH] D63497: Add support for openSUSE RISC-V triple

[Andreas Schwab via Phabricator via cfe-commits]

schwab created this revision.
Herald added subscribers: llvm-commits, cfe-commits, rkruppe, dexonsmith, 
rogfer01, shiva0217, kito-cheng.
Herald added projects: clang, LLVM.

Repository:
  rC Clang

https://reviews.llvm.org/D63497

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  llvm/unittests/ADT/TripleTest.cpp


Index: llvm/unittests/ADT/TripleTest.cpp
===
--- llvm/unittests/ADT/TripleTest.cpp
+++ llvm/unittests/ADT/TripleTest.cpp
@@ -330,6 +330,12 @@
   EXPECT_EQ(Triple::FreeBSD, T.getOS());
   EXPECT_EQ(Triple::UnknownEnvironment, T.getEnvironment());
 
+  T = Triple("riscv64-suse-linux");
+  EXPECT_EQ(Triple::riscv64, T.getArch());
+  EXPECT_EQ(Triple::SUSE, T.getVendor());
+  EXPECT_EQ(Triple::Linux, T.getOS());
+  EXPECT_EQ(Triple::UnknownEnvironment, T.getEnvironment());
+
   T = Triple("armv7hl-suse-linux-gnueabi");
   EXPECT_EQ(Triple::arm, T.getArch());
   EXPECT_EQ(Triple::SUSE, T.getVendor());
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2021,7 +2021,8 @@
   static const char *const RISCV64LibDirs[] = {"/lib64", "/lib"};
   static const char *const RISCV64Triples[] = {"riscv64-unknown-linux-gnu",
"riscv64-linux-gnu",
-   "riscv64-unknown-elf"};
+   "riscv64-unknown-elf",
+   "riscv64-suse-linux"};
 
   static const char *const SPARCv8LibDirs[] = {"/lib32", "/lib"};
   static const char *const SPARCv8Triples[] = {"sparc-linux-gnu",


Index: llvm/unittests/ADT/TripleTest.cpp
===
--- llvm/unittests/ADT/TripleTest.cpp
+++ llvm/unittests/ADT/TripleTest.cpp
@@ -330,6 +330,12 @@
   EXPECT_EQ(Triple::FreeBSD, T.getOS());
   EXPECT_EQ(Triple::UnknownEnvironment, T.getEnvironment());
 
+  T = Triple("riscv64-suse-linux");
+  EXPECT_EQ(Triple::riscv64, T.getArch());
+  EXPECT_EQ(Triple::SUSE, T.getVendor());
+  EXPECT_EQ(Triple::Linux, T.getOS());
+  EXPECT_EQ(Triple::UnknownEnvironment, T.getEnvironment());
+
   T = Triple("armv7hl-suse-linux-gnueabi");
   EXPECT_EQ(Triple::arm, T.getArch());
   EXPECT_EQ(Triple::SUSE, T.getVendor());
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2021,7 +2021,8 @@
   static const char *const RISCV64LibDirs[] = {"/lib64", "/lib"};
   static const char *const RISCV64Triples[] = {"riscv64-unknown-linux-gnu",
"riscv64-linux-gnu",
-   "riscv64-unknown-elf"};
+   "riscv64-unknown-elf",
+   "riscv64-suse-linux"};
 
   static const char *const SPARCv8LibDirs[] = {"/lib32", "/lib"};
   static const char *const SPARCv8Triples[] = {"sparc-linux-gnu",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

[Kevin P. Neal via Phabricator via cfe-commits]

kpn updated this revision to Diff 205371.
kpn added a comment.

Add static methods to convert between a StringRef and the enums for 
RoundingMode or ExceptionBehavior.


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

https://reviews.llvm.org/D53157

Files:
  include/llvm/IR/IRBuilder.h
  include/llvm/IR/IntrinsicInst.h
  lib/IR/IntrinsicInst.cpp
  unittests/IR/IRBuilderTest.cpp

Index: unittests/IR/IRBuilderTest.cpp
===
--- unittests/IR/IRBuilderTest.cpp
+++ unittests/IR/IRBuilderTest.cpp
@@ -122,6 +122,70 @@
   EXPECT_FALSE(II->hasNoNaNs());
 }
 
+TEST_F(IRBuilderTest, ConstrainedFP) {
+  IRBuilder<> Builder(BB);
+  Value *V;
+  CallInst *Call;
+  IntrinsicInst *II;
+
+  V = Builder.CreateLoad(GV);
+
+  // See if we get constrained intrinsics instead of non-constrained
+  // instructions.
+  Builder.setIsFPConstrained(true);
+
+  V = Builder.CreateFAdd(V, V);
+  ASSERT_TRUE(isa(V));
+  II = cast(V);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_fadd);
+
+  V = Builder.CreateFSub(V, V);
+  ASSERT_TRUE(isa(V));
+  II = cast(V);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_fsub);
+
+  V = Builder.CreateFMul(V, V);
+  ASSERT_TRUE(isa(V));
+  II = cast(V);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_fmul);
+  
+  V = Builder.CreateFDiv(V, V);
+  ASSERT_TRUE(isa(V));
+  II = cast(V);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_fdiv);
+  
+  V = Builder.CreateFRem(V, V);
+  ASSERT_TRUE(isa(V));
+  II = cast(V);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_frem);
+
+  // Verify the codepaths for setting and overriding the default metadata.
+  V = Builder.CreateFAdd(V, V);
+  ASSERT_TRUE(isa(V));
+  auto *CII = cast(V);
+  ASSERT_TRUE(CII->getExceptionBehavior() == ConstrainedFPIntrinsic::ebStrict);
+  ASSERT_TRUE(CII->getRoundingMode() == ConstrainedFPIntrinsic::rmDynamic);
+
+  Builder.setDefaultConstrainedExcept(ConstrainedFPIntrinsic::ebIgnore);
+  Builder.setDefaultConstrainedRounding(ConstrainedFPIntrinsic::rmUpward);
+  V = Builder.CreateFAdd(V, V);
+  CII = cast(V);
+  ASSERT_TRUE(CII->getExceptionBehavior() == ConstrainedFPIntrinsic::ebIgnore);
+  ASSERT_TRUE(CII->getRoundingMode() == ConstrainedFPIntrinsic::rmUpward);
+
+  // Now override the defaults.
+  Call = Builder.CreateConstrainedFPBinOp(
+Intrinsic::experimental_constrained_fadd, V, V, nullptr, "",
+ConstrainedFPIntrinsic::rmDownward, ConstrainedFPIntrinsic::ebMayTrap);
+  CII = cast(Call);
+  EXPECT_EQ(CII->getIntrinsicID(), Intrinsic::experimental_constrained_fadd);
+  ASSERT_TRUE(CII->getExceptionBehavior() == ConstrainedFPIntrinsic::ebMayTrap);
+  ASSERT_TRUE(CII->getRoundingMode() == ConstrainedFPIntrinsic::rmDownward);
+
+  Builder.CreateRetVoid();
+  EXPECT_FALSE(verifyModule(*M));
+}
+
 TEST_F(IRBuilderTest, Lifetime) {
   IRBuilder<> Builder(BB);
   AllocaInst *Var1 = Builder.CreateAlloca(Builder.getInt8Ty());
Index: lib/IR/IntrinsicInst.cpp
===
--- lib/IR/IntrinsicInst.cpp
+++ lib/IR/IntrinsicInst.cpp
@@ -110,8 +110,11 @@
   dyn_cast(getArgOperand(NumOperands - 2))->getMetadata();
   if (!MD || !isa(MD))
 return rmInvalid;
-  StringRef RoundingArg = cast(MD)->getString();
+  return StrToRoundingMode(cast(MD)->getString());
+}
 
+ConstrainedFPIntrinsic::RoundingMode
+ConstrainedFPIntrinsic::StrToRoundingMode(StringRef RoundingArg) {
   // For dynamic rounding mode, we use round to nearest but we will set the
   // 'exact' SDNodeFlag so that the value will not be rounded.
   return StringSwitch(RoundingArg)
@@ -123,6 +126,30 @@
 .Default(rmInvalid);
 }
 
+StringRef
+ConstrainedFPIntrinsic::RoundingModeToStr(RoundingMode UseRounding) {
+  StringRef RoundingStr;
+  switch (UseRounding) {
+  case ConstrainedFPIntrinsic::rmUnspecified:
+  case ConstrainedFPIntrinsic::rmDynamic:
+RoundingStr = "round.dynamic";
+break;
+  case ConstrainedFPIntrinsic::rmToNearest:
+RoundingStr = "round.tonearest";
+break;
+  case ConstrainedFPIntrinsic::rmDownward:
+RoundingStr = "round.downward";
+break;
+  case ConstrainedFPIntrinsic::rmUpward:
+RoundingStr = "round.upward";
+break;
+  case ConstrainedFPIntrinsic::rmTowardZero:
+RoundingStr = "round.tozero";
+break;
+  }
+  return RoundingStr;
+}
+
 ConstrainedFPIntrinsic::ExceptionBehavior
 ConstrainedFPIntrinsic::getExceptionBehavior() const {
   unsigned NumOperands = getNumArgOperands();
@@ -130,7 +157,11 @@
   dyn_cast(getArgOperand(NumOperands - 1))->getMetadata();
   if (!MD || !isa(MD))
 return ebInvalid;
-  StringRef ExceptionArg = cast(MD)->getString();
+  return StrToExceptionBehavior(cast(MD)->getString());
+}
+
+ConstrainedFPIntrinsic::ExceptionBehavior
+ConstrainedFPIntrinsic::StrToExceptionBehavior(StringRef ExceptionArg) {
   return StringSwitch(E

[PATCH] D62952: [analyzer][tests] Use normalize_sarif in place of diff_sarif

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D62952#1548593 , @aaron.ballman 
wrote:

> But is there a reason to not keep `%diff_sarif` and define it in terms of 
> `%normalize_sarif | diff -U1 -b` within lit.local.cfg? I guess I don't see 
> the benefit to exposing the call to diff (I don't anticipate anyone needing 
> to change the options passed to diff).


Yes: The normalization runs on stdin. `%diff_sarif` would then only make sense 
if stdin is one of the inputs, and I think the only input we can hardcode to be 
stdin is the one conventionally considered to be the reference input (which is 
not what we want).


Repository:
  rC Clang

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

https://reviews.llvm.org/D62952



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

[PATCH] D62954: [Syntax] Add a helper to find expansion by its first spelled token

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:207
+  auto FileIt = Files.find(SourceMgr->getFileID(Spelled->location()));
+  assert(FileIt != Files.end());
+

This assert could use a message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62954



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

[PATCH] D63497: Add support for openSUSE RISC-V triple

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a reviewer: asb.
lebedev.ri added a comment.

Nice :)
It is also a good idea to upload patches with full context (`-U9`)




Comment at: llvm/unittests/ADT/TripleTest.cpp:333
 
+  T = Triple("riscv64-suse-linux");
+  EXPECT_EQ(Triple::riscv64, T.getArch());

Without context i can't tell, in general is the grouping here based on the 
triple, or the vendor?


Repository:
  rC Clang

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

https://reviews.llvm.org/D63497



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

[PATCH] D62954: [Syntax] Add a helper to find expansion by its first spelled token

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 205374.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.

- Added a message to the assertion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62954

Files:
  clang/include/clang/Tooling/Syntax/Tokens.h
  clang/lib/Tooling/Syntax/Tokens.cpp
  clang/unittests/Tooling/Syntax/TokensTest.cpp

Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- clang/unittests/Tooling/Syntax/TokensTest.cpp
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -55,6 +55,7 @@
 using ::testing::AllOf;
 using ::testing::Contains;
 using ::testing::ElementsAre;
+using ::testing::Field;
 using ::testing::Matcher;
 using ::testing::Not;
 using ::testing::StartsWith;
@@ -65,6 +66,13 @@
 MATCHER_P(SameRange, A, "") {
   return A.begin() == arg.begin() && A.end() == arg.end();
 }
+
+Matcher
+IsExpansion(Matcher> Spelled,
+Matcher> Expanded) {
+  return AllOf(Field(&TokenBuffer::Expansion::Spelled, Spelled),
+   Field(&TokenBuffer::Expansion::Expanded, Expanded));
+}
 // Matchers for syntax::Token.
 MATCHER_P(Kind, K, "") { return arg.kind() == K; }
 MATCHER_P2(HasText, Text, SourceMgr, "") {
@@ -629,6 +637,76 @@
   ValueIs(SameRange(findSpelled("not_mapped";
 }
 
+TEST_F(TokenBufferTest, ExpansionStartingAt) {
+  // Object-like macro expansions.
+  recordTokens(R"cpp(
+#define FOO 3+4
+int a = FOO 1;
+int b = FOO 2;
+  )cpp");
+
+  llvm::ArrayRef Foo1 = findSpelled("FOO 1").drop_back();
+  EXPECT_THAT(
+  Buffer.expansionStartingAt(Foo1.data()),
+  ValueIs(IsExpansion(SameRange(Foo1),
+  SameRange(findExpanded("3 + 4 1").drop_back();
+
+  llvm::ArrayRef Foo2 = findSpelled("FOO 2").drop_back();
+  EXPECT_THAT(
+  Buffer.expansionStartingAt(Foo2.data()),
+  ValueIs(IsExpansion(SameRange(Foo2),
+  SameRange(findExpanded("3 + 4 2").drop_back();
+
+  // Function-like macro expansions.
+  recordTokens(R"cpp(
+#define ID(X) X
+int a = ID(1+2+3);
+int b = ID(ID(2+3+4));
+  )cpp");
+
+  llvm::ArrayRef ID1 = findSpelled("ID ( 1 + 2 + 3 )");
+  EXPECT_THAT(Buffer.expansionStartingAt(&ID1.front()),
+  ValueIs(IsExpansion(SameRange(ID1),
+  SameRange(findExpanded("1 + 2 + 3");
+  // Only the first spelled token should be found.
+  for (const auto &T : ID1.drop_front())
+EXPECT_EQ(Buffer.expansionStartingAt(&T), llvm::None);
+
+  llvm::ArrayRef ID2 = findSpelled("ID ( ID ( 2 + 3 + 4 ) )");
+  EXPECT_THAT(Buffer.expansionStartingAt(&ID2.front()),
+  ValueIs(IsExpansion(SameRange(ID2),
+  SameRange(findExpanded("2 + 3 + 4");
+  // Only the first spelled token should be found.
+  for (const auto &T : ID2.drop_front())
+EXPECT_EQ(Buffer.expansionStartingAt(&T), llvm::None);
+
+  // PP directives.
+  recordTokens(R"cpp(
+#define FOO 1
+int a = FOO;
+#pragma once
+int b = 1;
+  )cpp");
+
+  llvm::ArrayRef DefineFoo = findSpelled("# define FOO 1");
+  EXPECT_THAT(
+  Buffer.expansionStartingAt(&DefineFoo.front()),
+  ValueIs(IsExpansion(SameRange(DefineFoo),
+  SameRange(findExpanded("int a").take_front(0);
+  // Only the first spelled token should be found.
+  for (const auto &T : DefineFoo.drop_front())
+EXPECT_EQ(Buffer.expansionStartingAt(&T), llvm::None);
+
+  llvm::ArrayRef PragmaOnce = findSpelled("# pragma once");
+  EXPECT_THAT(
+  Buffer.expansionStartingAt(&PragmaOnce.front()),
+  ValueIs(IsExpansion(SameRange(PragmaOnce),
+  SameRange(findExpanded("int b").take_front(0);
+  // Only the first spelled token should be found.
+  for (const auto &T : PragmaOnce.drop_front())
+EXPECT_EQ(Buffer.expansionStartingAt(&T), llvm::None);
+}
+
 TEST_F(TokenBufferTest, TokensToFileRange) {
   addFile("./foo.h", "token_from_header");
   llvm::Annotations Code(R"cpp(
Index: clang/lib/Tooling/Syntax/Tokens.cpp
===
--- clang/lib/Tooling/Syntax/Tokens.cpp
+++ clang/lib/Tooling/Syntax/Tokens.cpp
@@ -199,6 +199,32 @@
   : LastSpelled + 1);
 }
 
+llvm::Optional
+TokenBuffer::expansionStartingAt(const syntax::Token *Spelled) const {
+  assert(Spelled);
+  assert(Spelled->location().isFileID() && "not a spelled token");
+  auto FileIt = Files.find(SourceMgr->getFileID(Spelled->location()));
+  assert(FileIt != Files.end() && "file not tracked by token buffer");
+
+  auto &File = FileIt->second;
+  assert(File.SpelledTokens.data() <= Spelled &&
+ Spelled < (File.SpelledTokens.data() + File.SpelledTokens.size()));
+
+  unsigned SpelledIndex = Spelled - File.SpelledTokens.data();
+  auto M = llvm::bsearch(File.Mappings, [&](const Mapping &M) {
+

[PATCH] D62952: [analyzer][tests] Use normalize_sarif in place of diff_sarif

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D62952#1548620 , 
@hubert.reinterpretcast wrote:

> In D62952#1548593 , @aaron.ballman 
> wrote:
>
> > But is there a reason to not keep `%diff_sarif` and define it in terms of 
> > `%normalize_sarif | diff -U1 -b` within lit.local.cfg? I guess I don't see 
> > the benefit to exposing the call to diff (I don't anticipate anyone needing 
> > to change the options passed to diff).
>
>
> Yes: The normalization runs on stdin. `%diff_sarif` would then only make 
> sense if stdin is one of the inputs, and I think the only input we can 
> hardcode to be stdin is the one conventionally considered to be the reference 
> input (which is not what we want).


Ah drat, I think you're right. Oh well! I think your approach makes sense to me 
then. Thank you for working on this!


Repository:
  rC Clang

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

https://reviews.llvm.org/D62952



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

r363698 - [Syntax] Add a helper to find expansion by its first spelled token

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Tue Jun 18 09:27:27 2019
New Revision: 363698

URL: http://llvm.org/viewvc/llvm-project?rev=363698&view=rev
Log:
[Syntax] Add a helper to find expansion by its first spelled token

Summary: Used in clangd for a code tweak that expands a macro.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: kadircet, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/Tooling/Syntax/Tokens.h
cfe/trunk/lib/Tooling/Syntax/Tokens.cpp
cfe/trunk/unittests/Tooling/Syntax/TokensTest.cpp

Modified: cfe/trunk/include/clang/Tooling/Syntax/Tokens.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Syntax/Tokens.h?rev=363698&r1=363697&r2=363698&view=diff
==
--- cfe/trunk/include/clang/Tooling/Syntax/Tokens.h (original)
+++ cfe/trunk/include/clang/Tooling/Syntax/Tokens.h Tue Jun 18 09:27:27 2019
@@ -200,6 +200,25 @@ public:
   llvm::Optional>
   spelledForExpanded(llvm::ArrayRef Expanded) const;
 
+  /// An expansion produced by the preprocessor, includes macro expansions and
+  /// preprocessor directives. Preprocessor always maps a non-empty range of
+  /// spelled tokens to a (possibly empty) range of expanded tokens. Here is a
+  /// few examples of expansions:
+  ///#pragma once  // Expands to an empty range.
+  ///#define FOO 1 2 3 // Expands an empty range.
+  ///FOO   // Expands to "1 2 3".
+  /// FIXME(ibiryukov): implement this, currently #include expansions are 
empty.
+  ///#include  // Expands to tokens produced by the include.
+  struct Expansion {
+llvm::ArrayRef Spelled;
+llvm::ArrayRef Expanded;
+  };
+  /// If \p Spelled starts a mapping (e.g. if it's a macro name or '#' starting
+  /// a preprocessor directive) return the subrange of expanded tokens that the
+  /// macro expands to.
+  llvm::Optional
+  expansionStartingAt(const syntax::Token *Spelled) const;
+
   /// Lexed tokens of a file before preprocessing. E.g. for the following input
   /// #define DECL(name) int name = 10
   /// DECL(a);

Modified: cfe/trunk/lib/Tooling/Syntax/Tokens.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Syntax/Tokens.cpp?rev=363698&r1=363697&r2=363698&view=diff
==
--- cfe/trunk/lib/Tooling/Syntax/Tokens.cpp (original)
+++ cfe/trunk/lib/Tooling/Syntax/Tokens.cpp Tue Jun 18 09:27:27 2019
@@ -199,6 +199,32 @@ TokenBuffer::spelledForExpanded(llvm::Ar
   : LastSpelled + 1);
 }
 
+llvm::Optional
+TokenBuffer::expansionStartingAt(const syntax::Token *Spelled) const {
+  assert(Spelled);
+  assert(Spelled->location().isFileID() && "not a spelled token");
+  auto FileIt = Files.find(SourceMgr->getFileID(Spelled->location()));
+  assert(FileIt != Files.end() && "file not tracked by token buffer");
+
+  auto &File = FileIt->second;
+  assert(File.SpelledTokens.data() <= Spelled &&
+ Spelled < (File.SpelledTokens.data() + File.SpelledTokens.size()));
+
+  unsigned SpelledIndex = Spelled - File.SpelledTokens.data();
+  auto M = llvm::bsearch(File.Mappings, [&](const Mapping &M) {
+return SpelledIndex <= M.BeginSpelled;
+  });
+  if (M == File.Mappings.end() || M->BeginSpelled != SpelledIndex)
+return llvm::None;
+
+  Expansion E;
+  E.Spelled = llvm::makeArrayRef(File.SpelledTokens.data() + M->BeginSpelled,
+ File.SpelledTokens.data() + M->EndSpelled);
+  E.Expanded = llvm::makeArrayRef(ExpandedTokens.data() + M->BeginExpanded,
+  ExpandedTokens.data() + M->EndExpanded);
+  return E;
+}
+
 std::vector syntax::tokenize(FileID FID, const SourceManager 
&SM,
 const LangOptions &LO) {
   std::vector Tokens;

Modified: cfe/trunk/unittests/Tooling/Syntax/TokensTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/Syntax/TokensTest.cpp?rev=363698&r1=363697&r2=363698&view=diff
==
--- cfe/trunk/unittests/Tooling/Syntax/TokensTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/Syntax/TokensTest.cpp Tue Jun 18 09:27:27 2019
@@ -55,6 +55,7 @@ using llvm::ValueIs;
 using ::testing::AllOf;
 using ::testing::Contains;
 using ::testing::ElementsAre;
+using ::testing::Field;
 using ::testing::Matcher;
 using ::testing::Not;
 using ::testing::StartsWith;
@@ -65,6 +66,13 @@ namespace {
 MATCHER_P(SameRange, A, "") {
   return A.begin() == arg.begin() && A.end() == arg.end();
 }
+
+Matcher
+IsExpansion(Matcher> Spelled,
+Matcher> Expanded) {
+  return AllOf(Field(&TokenBuffer::Expansion::Spelled, Spelled),
+   Field(&TokenBuffer::Expansion::Expanded, Expanded));
+}
 // Matchers for syntax::Token.
 MATCHER_P(Kind, K, "") { return arg.kind() == K; }
 MATCH

[PATCH] D63426: [clangd] Narrow rename to local symbols.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks for exposing the ND, cleaner.
Found another problem :-( not in your code but about this whole approach to 
using the index.
Let's talk tomorrow




Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:81
+  Req.Limit = 100;
+  if (auto ID = getSymbolID(&Decl))
+Req.IDs.insert(*ID);

What about cases where the symbol has a USR but is not indexed? Non-toplevel, 
template bits we ignore, etc?

Not sure what we should do honestly. The "real" solution is probably to use 
QNames instead of symbol id here, and index then all. Silently allowing the 
rename seems pretty bad and needs at least to be explicit



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:113
+  assert(RenameDecl && "symbol must be found at this point");
+  // FIXME: we may skip querying the index if RenameDecl is function-local.
+  if (auto OtherFile = getOtherRefFile(*RenameDecl, File, Index))

This would be fixed in the helper function, move the fixme there


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63426



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