[PATCH] D66647: [clangd] DefineInline action apply logic with fully qualified names

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:334
+
+const tooling::Replacement SemiColonToFuncBody(SM, *SemiColon, 1,
+   *QualifiedBody);

ilya-biryukov wrote:
> Could we add a test when the semicolon comes from a macro expansion? I 
> believe the action won't be available in that case, which is ok. Just to make 
> sure we cover this corner-case.
> 
> ```
> #define SEMI ;
> void foo() SEMI
> ```
> 
> Also interesting cases:
> - source function is in a macro expansion
> - target function is in a macro expansion.
> 
> I believe they might catch a few bugs here, as we seem to assume all 
> locations are file locations...
see macro tests for the behavior in such cases.



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:1332
+  template <>
+  void fo^o(char p) {
+return;

ilya-biryukov wrote:
> How is this test different from the previous one?
> Is it just trying to make sure we don't always move to the first function 
> body?
yes that's the point, adding a comment.



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:1374
+  )cpp");
+}
+

ilya-biryukov wrote:
> Do we test the following case anywhere?
> ```
> template  void foo();
> template <> void ^foo() { // should not be available
>   return;
> }
> ```
Yes, there is once check in availability tests for:
```
  EXPECT_UNAVAILABLE(R"cpp(
template  void foo();

template<> void f^oo() {
})cpp");
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66647



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

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 226380.
Typz added a comment.

Implement a fallback to regularly indented alignment when the question mark is 
wrapped.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50078

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5798,6 +5798,113 @@
" // comment\n"
" ? a = b\n"
" : a;");
+
+  // Chained conditionals
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 70;
+  Style.AlignOperands = true;
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return aa ? \n"
+   "   :  ? \n"
+   "  : ;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? 22\n"
+   ": 33;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   "   : cc ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? (aaa ? bbb : ccc)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": (aaa ? bbb : ccc);",
+   Style);
+  verifyFormat("return  ? (a ? bb\n"
+   " : cc)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? (a ? bb\n"
+   " : cc)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? a = (a ? bb\n"
+   " : dd)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? a + (a ? bb\n"
+   " : dd)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? \n"
+   "   : bb ? \n"
+   ": a + (a ? bb\n"
+   " : dd)\n",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": (a ? bb\n"
+   " : cc);",
+   Style);
+  verifyFormat("return  ? (a ? bb\n"
+   "   : ccc ? dd\n"
+   " : ee)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? (aa? bb\n"
+   "   

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/ContinuationIndenter.cpp:1125
+  if (Current.is(TT_ConditionalExpr) && Current.is(tok::question) &&
+  ((Current.MustBreakBefore) ||
+   (Current.getNextNonComment() && 
Current.getNextNonComment()->MustBreakBefore)))

Maybe it would be better to go through here whenever this is wrapped (e.g. 
`Newline == true`) : but this implies akso introducing a penalty for "breaking" 
the nested conditional alignment. May ultimately be better, though probably 
somewhat more complex still.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50078



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

[PATCH] D66647: [clangd] DefineInline action apply logic with fully qualified names

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:339
+
+const tooling::Replacement SemiColonToFuncBody(SM, *SemiColon, 1,
+   *QualifiedBody);

s/SemiColon/Semicolon



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:323
+
+auto SemiColon = getSemicolonForDecl(Target);
+if (!SemiColon) {

ilya-biryukov wrote:
> NIT: s/SemiColon/Semicolon
This one is still there.



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:1332
+  template <>
+  void fo^o(char p) {
+return;

kadircet wrote:
> ilya-biryukov wrote:
> > How is this test different from the previous one?
> > Is it just trying to make sure we don't always move to the first function 
> > body?
> yes that's the point, adding a comment.
Thanks!



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:1374
+  )cpp");
+}
+

kadircet wrote:
> ilya-biryukov wrote:
> > Do we test the following case anywhere?
> > ```
> > template  void foo();
> > template <> void ^foo() { // should not be available
> >   return;
> > }
> > ```
> Yes, there is once check in availability tests for:
> ```
>   EXPECT_UNAVAILABLE(R"cpp(
> template  void foo();
> 
> template<> void f^oo() {
> })cpp");
> ```
Ah, nice, I missed it. Thanks for pointing it out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66647



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

[PATCH] D69266: [clangd] Define out-of-line availability checks

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet marked 6 inline comments as done.
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:1548
+
+  ExtraFiles["Test.cpp"] = "";
+  // Basic check for function body and signature.

hokein wrote:
> Is this needed? It is unclear to me why we need it.
yeah not anymore, I was checking for existence of implementation file in the 
first version. but moved it from prepare to apply, forgot to update the test.



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:1556
+
+[[void [[Bar::[[b^a^z() [[{
+  return;

hokein wrote:
> Sorry for not spotting it earlier.
> 
> Moving out-of-line methods is different than general functions, `void 
> Bar::bar();` is illegal in C++ (C++ requires the out-of-line member 
> declaration must be a definition). I think for this case, we could 1) delete 
> the original method definition 2) disable the tweak. I slightly prefer 2) as 
> out-of-line member definitions are rare in header files. WDYT?
oopsy thanks for spotting this. yeah I agree with 2.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69266



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

[PATCH] D69266: [clangd] Define out-of-line availability checks

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 226381.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69266

Files:
  clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
  clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.h
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1519,6 +1519,62 @@
 using namespace a;
 )cpp");
 }
+
+TWEAK_TEST(DefineOutline);
+TEST_F(DefineOutlineTest, TriggersOnFunctionDecl) {
+  FileName = "Test.cpp";
+  // Not available unless in a header file.
+  EXPECT_UNAVAILABLE(R"cpp(
+[[void [[f^o^o]]() [[{
+  return;
+})cpp");
+
+  FileName = "Test.hpp";
+  // Not available unless function name or fully body is selected.
+  EXPECT_UNAVAILABLE(R"cpp(
+// Not a definition
+vo^i[[d^ ^f]]^oo();
+
+[[vo^id ]]foo[[()]] {[[
+  [[(void)(5+3);
+  return;]]
+}]])cpp");
+
+  // Available even if there are no implementation files.
+  EXPECT_AVAILABLE(R"cpp(
+[[void [[f^o^o]]() [[{
+  return;
+})cpp");
+
+  // Not available for out-of-line metohds.
+  EXPECT_UNAVAILABLE(R"cpp(
+class Bar {
+  void baz();
+};
+
+[[void [[Bar::[[b^a^z() [[{
+  return;
+})cpp");
+
+  // Basic check for function body and signature.
+  EXPECT_AVAILABLE(R"cpp(
+class Bar {
+  [[void [[f^o^o]]() [[{ return; }
+};
+
+void foo();
+[[void [[f^o^o]]() [[{
+  return;
+})cpp");
+
+  // Not available on defaulted/deleted members.
+  EXPECT_UNAVAILABLE(R"cpp(
+class Foo {
+  Fo^o() = default;
+  F^oo(const Foo&) = delete;
+};)cpp");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/TweakTesting.h
===
--- clang-tools-extra/clangd/unittests/TweakTesting.h
+++ clang-tools-extra/clangd/unittests/TweakTesting.h
@@ -12,6 +12,7 @@
 #include "TestTU.h"
 #include "index/Index.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
@@ -62,6 +63,8 @@
   // testcases.
   std::string Header;
 
+  llvm::StringRef FileName = "TestTU.cpp";
+
   // Extra flags passed to the compilation in apply().
   std::vector ExtraArgs;
 
Index: clang-tools-extra/clangd/unittests/TweakTesting.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTesting.cpp
+++ clang-tools-extra/clangd/unittests/TweakTesting.cpp
@@ -62,12 +62,13 @@
   cantFail(positionToOffset(A.code(), SelectionRng.end))};
 }
 
-MATCHER_P5(TweakIsAvailable, TweakID, Ctx, Header, ExtraFiles, Index,
+MATCHER_P6(TweakIsAvailable, TweakID, Ctx, Header, ExtraFiles, Index, FileName,
(TweakID + (negation ? " is unavailable" : " is available")).str()) {
   std::string WrappedCode = wrap(Ctx, arg);
   Annotations Input(WrappedCode);
   auto Selection = rangeOrPoint(Input);
   TestTU TU;
+  TU.Filename = FileName;
   TU.HeaderCode = Header;
   TU.Code = Input.code();
   TU.AdditionalFiles = std::move(ExtraFiles);
@@ -89,6 +90,7 @@
   auto Selection = rangeOrPoint(Input);
 
   TestTU TU;
+  TU.Filename = FileName;
   TU.HeaderCode = Header;
   TU.AdditionalFiles = std::move(ExtraFiles);
   TU.Code = Input.code();
@@ -125,7 +127,7 @@
 
 ::testing::Matcher TweakTest::isAvailable() const {
   return TweakIsAvailable(llvm::StringRef(TweakID), Context, Header, ExtraFiles,
-  Index.get());
+  Index.get(), FileName);
 }
 
 std::vector TweakTest::expandCases(llvm::StringRef MarkedCode) {
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -0,0 +1,100 @@
+//===--- DefineOutline.cpp ---*- 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 "HeaderSourceSwitch.h"
+#include "Path.h"
+#include "Selection.h"
+#include "refactor/Tweak.h"
+#include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Stmt.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/Supp

[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

[Ilya Biryukov via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/XRefs.cpp:139
+// that constructor. FIXME(nridge): this should probably be handled in
+// targetDecl() itself.
+const CXXConstructorDecl *findCalledConstructor(const SelectionTree::Node *N) {

nridge wrote:
> ilya-biryukov wrote:
> > nridge wrote:
> > > ilya-biryukov wrote:
> > > > sammccall wrote:
> > > > > nridge wrote:
> > > > > > I would appreciate some tips on how we could handle this in 
> > > > > > `targetDecl()`. Please see more specific comments below.
> > > > > My thinking is basically:
> > > > >  - C++ syntax is vague and doesn't give us great ways of referring 
> > > > > directly to constructors.
> > > > >  - there's value to simplicity, and I've found go-to-definition 
> > > > > additionally finding implicit nodes to be confusing as often as 
> > > > > useful. I think on balance and given the constraints of LSP, we 
> > > > > should consider dropping this and having implicit references be 
> > > > > returned by find-refs but not targetable by go-to-def/hover/etc. It's 
> > > > > OK for simplicity of implementation to be a concern here.
> > > > >  - the node that targets the constructor is the CXXConstructExpr. 
> > > > > Treating the VarDecl conditionally as a reference to the constructor, 
> > > > > while clever, seems like a hack. Ideally the fix is to make 
> > > > > SelectionTree yield the CXXConstructExpr, not to change TargetDecl.
> > > > >  - CXXConstructExpr is only sometimes implicit. SelectionTree should 
> > > > > return it for the parens in `Foo()` or `Foo x()`. And ideally for the 
> > > > > equals in `Foo x = {}`, though I think there's no CXXConstructExpr in 
> > > > > the AST in that case :-(
> > > > >  - When the AST modelling is bad (as it seems to be for some aspects 
> > > > > CXXConstructExpr) we should consider accepting some glitches and 
> > > > > trying to improve things in clang if they're important. Hacking 
> > > > > around them will lead to inconsistencies between different clangd 
> > > > > features.
> > > > > 
> > > > > The TL;DR is I think I'd be happy to drop this special case and try 
> > > > > to make SelectionTree surface CXXConstructExpr more often, even if it 
> > > > > changes some behavior.
> > > > +1 to the idea of landing this and changing behavior.
> > > > I don't think we're loosing much in terms of functionality and I 
> > > > personally like the new code much more.
> > > FWIW, I do find being able to get from a variable declaration to the 
> > > invoked constructor to be very useful.
> > > 
> > > If the class has several constructors, there's no other easy way to 
> > > determine which one is being invoked (only other thing I can think of is 
> > > to perform "find references" on each constructor and see which one 
> > > includes the variable declaration as a reference, which is a lot more 
> > > tedious), and I think that's an important thing to be able to do (just 
> > > like for named functions).
> > > 
> > > So I'd definitely like to keep this case working. However, I'd be fine 
> > > with only having the parens or braces target the constructor. (That's 
> > > still slightly annoying, as you have to place  the cursor more precisely, 
> > > and it also may not be obvious to users, but at least there's a way to do 
> > > it.) I'm also fine with changing the behaviour for now, and deferring 
> > > constructor targeting to a follow-up, as there are other benefits this 
> > > patch brings.
> > We discussed this offline: finding constructor references is useful, but 
> > current approach does not generalize very well, e.g. for implicit 
> > constructor calls:
> > ```
> > struct Foo {
> >   Foo(int);
> > };
> > 
> > int func(Foo a, Foo b);
> > int test() {
> >   func(1, 2); // no way to find Foo(1) and Foo(2) calls.
> > }
> > ```
> > 
> > So we'd probably want some other mechanism to expose *all* possible 
> > references.
> > In any case, changing behavior now and tweaking it with a follow-up patch 
> > is probably the best way to proceed with this change.
> > So we'd probably want some other mechanism to expose *all* possible 
> > references.
> 
> Any ideas for what such a mechanism might look like?
We were discussing this offline, a few options that came up are:

Expose tree view of the AST (that links to the source code) with implicit 
nodes, allow to navigate from the conversion nodes to the references.
Code lens in VSCode, i.e. in-editor annotations right in the middle of the code



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69237



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

[clang] 24ef631 - Fix file-ordering nit in D67161.

[Simon Tatham via cfe-commits]

Author: Simon Tatham
Date: 2019-10-25T09:22:07+01:00
New Revision: 24ef631f4333120abd6b66c1e8466a582b60779f

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

LOG: Fix file-ordering nit in D67161.

Re-sorted the module names in clang/utils/TableGen/CMakeLists.txt back
into alphabetical order.

Added: 


Modified: 
clang/utils/TableGen/CMakeLists.txt

Removed: 




diff  --git a/clang/utils/TableGen/CMakeLists.txt 
b/clang/utils/TableGen/CMakeLists.txt
index 407cf8a57f9a..c685a2c0c076 100644
--- a/clang/utils/TableGen/CMakeLists.txt
+++ b/clang/utils/TableGen/CMakeLists.txt
@@ -13,8 +13,8 @@ add_tablegen(clang-tblgen CLANG
   ClangOptionDocEmitter.cpp
   ClangSACheckersEmitter.cpp
   ClangTypeNodesEmitter.cpp
-  NeonEmitter.cpp
   MveEmitter.cpp
+  NeonEmitter.cpp
   TableGen.cpp
   )
 set_target_properties(clang-tblgen PROPERTIES FOLDER "Clang tablegenning")



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

[PATCH] D67161: [clang,ARM] Initial ACLE intrinsics for MVE.

[Simon Tatham via Phabricator via cfe-commits]

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



Comment at: clang/utils/TableGen/CMakeLists.txt:17
   NeonEmitter.cpp
+  MveEmitter.cpp
   TableGen.cpp

thakis wrote:
> nit: These files are listed alphabetically.
Sorry about that. Fixed in rG24ef631f4333120abd6b66c1e8466a582b60779f


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67161



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

[PATCH] D69383: [RISCV] Match GCC `-march`/`-mabi` driver defaults

[Simon Cook via Phabricator via cfe-commits]

simoncook added a comment.

I have a question about backwards compatibility with this patch. Clang 9 has 
shipped with rvXXg/etc defaulting to ilp32/lp64 ABI, and no march meaning 
rvXXi, with users having built objects with those defaults. When Clang 10 
ships, users they now need to always use a mabi/march flag to keep the same 
compatibility with their Clang 9 flows, the enabled extensions and ABI will be 
changed under their feet without warning (or at least until they either hit a 
linker error in the ABI case, or potentially an invalid instruction trap in the 
march case).

If we're going to change the defaults, this patch should at least contain an 
update to the release notes file, this way this change would be documented for 
users.




Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:476
+if (MArch.startswith_lower("rv32")) {
+  if (MArch.substr(4).contains_lower("d") ||
+  MArch.startswith_lower("rv32g"))

Won’t this break if the user specifies a X/Z extension that has a d in the 
name, so eg rv32iXd will try to use the ilp32d abi by default? For future 
proofing, I think we may need to do a full parse of the isa string to verify 
that d does actually mean the standard D-extension


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69383



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

[PATCH] D66647: [clangd] DefineInline action apply logic with fully qualified names

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 226388.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- s/SemiColon/Semicolon/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66647

Files:
  clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.h
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -25,6 +25,7 @@
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -40,11 +41,16 @@
 using ::testing::AllOf;
 using ::testing::HasSubstr;
 using ::testing::StartsWith;
+using ::testing::ElementsAre;
 
 namespace clang {
 namespace clangd {
 namespace {
 
+MATCHER_P2(FileWithContents, FileName, Contents, "") {
+  return arg.first() == FileName && arg.second == Contents;
+}
+
 TEST(FileEdits, AbsolutePath) {
   auto RelPaths = {"a.h", "foo.cpp", "test/test.cpp"};
 
@@ -1047,6 +1053,512 @@
   })cpp");
 }
 
+TEST_F(DefineInlineTest, TransformNestedNamespaces) {
+  EXPECT_EQ(apply(R"cpp(
+  namespace a {
+void bar();
+namespace b {
+  void baz();
+  namespace c {
+void aux();
+  }
+}
+  }
+
+  void foo();
+  using namespace a;
+  using namespace b;
+  using namespace c;
+  void f^oo() {
+bar();
+a::bar();
+
+baz();
+b::baz();
+a::b::baz();
+
+aux();
+c::aux();
+b::c::aux();
+a::b::c::aux();
+  })cpp"), R"cpp(
+  namespace a {
+void bar();
+namespace b {
+  void baz();
+  namespace c {
+void aux();
+  }
+}
+  }
+
+  void foo(){
+a::bar();
+a::bar();
+
+a::b::baz();
+a::b::baz();
+a::b::baz();
+
+a::b::c::aux();
+a::b::c::aux();
+a::b::c::aux();
+a::b::c::aux();
+  }
+  using namespace a;
+  using namespace b;
+  using namespace c;
+  )cpp");
+}
+
+TEST_F(DefineInlineTest, TransformUsings) {
+  EXPECT_EQ(apply(R"cpp(
+  namespace a { namespace b { namespace c { void aux(); } } }
+
+  void foo();
+  void f^oo() {
+using namespace a;
+using namespace b;
+using namespace c;
+using c::aux;
+namespace d = c;
+  })cpp"),
+R"cpp(
+  namespace a { namespace b { namespace c { void aux(); } } }
+
+  void foo(){
+using namespace a;
+using namespace a::b;
+using namespace a::b::c;
+using a::b::c::aux;
+namespace d = a::b::c;
+  }
+  )cpp");
+}
+
+TEST_F(DefineInlineTest, TransformDecls) {
+  EXPECT_EQ(apply(R"cpp(
+  void foo();
+  void f^oo() {
+class Foo {
+public:
+  void foo();
+  int x;
+  static int y;
+};
+Foo::y = 0;
+
+enum En { Zero, One };
+En x = Zero;
+
+enum class EnClass { Zero, One };
+EnClass y = EnClass::Zero;
+  })cpp"),
+R"cpp(
+  void foo(){
+class Foo {
+public:
+  void foo();
+  int x;
+  static int y;
+};
+Foo::y = 0;
+
+enum En { Zero, One };
+En x = Zero;
+
+enum class EnClass { Zero, One };
+EnClass y = EnClass::Zero;
+  }
+  )cpp");
+}
+
+TEST_F(DefineInlineTest, TransformTemplDecls) {
+  EXPECT_EQ(apply(R"cpp(
+  namespace a {
+template  class Bar {
+public:
+  void bar();
+};
+template  T bar;
+template  void aux() {}
+  }
+
+  void foo();
+
+  using namespace a;
+  void f^oo() {
+bar>.bar();
+aux>();
+  })cpp"),
+R"cpp(
+  namespace a {
+template  class Bar {
+public:
+  void bar();
+};
+template  T bar;
+template  void aux() {}
+  }
+
+  void foo(){
+a::bar>.bar();
+a::aux>();
+  }
+
+  using namespace a;
+  )cpp");
+}
+
+TEST_F(DefineInlineTest, TransformMembers) {
+  EXPECT_EQ(apply(R"cpp(
+  class Foo {
+void foo();
+  };
+
+  void Foo::f^oo() {
+return;
+  })cpp"),
+R"cpp(
+  class Foo {
+void foo(){
+return;
+  }
+  };
+
+  )cpp");
+
+  ExtraFiles["a.h"] = R"cpp(
+  class Foo {
+void foo();
+  };)cpp";
+
+  llvm::StringMap EditedFiles;
+  EXPECT_EQ(apply(R"cpp(
+  #include "a.h"
+  void Foo::f^oo() {
+return;
+  })cpp",
+  &EditedFiles),
+R"cpp(
+  #include "a.h"
+  )cpp");
+  EXPECT_THAT(EditedFiles,
+  ElementsAre(FileWithContents(testPath("a.h"),
+R"cpp(
+  class Foo {
+void foo(){
+return;
+  }
+  };)cpp")));
+}
+
+TEST_F(DefineInlineTest, TransformDependentTypes) {
+  EXPECT_EQ(apply(R"cpp(
+  namespace a {
+template  class Bar {};
+  }
+
+  template 

[PATCH] D68937: [clangd] Add parameter renaming to define-inline code action

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68937

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

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1469,6 +1469,44 @@
 )cpp");
 }
 
+TEST_F(DefineInlineTest, TransformParamNames) {
+  EXPECT_EQ(apply(R"cpp(
+void foo(int, bool b);
+
+void ^foo(int f, bool x) {})cpp"), R"cpp(
+void foo(int f, bool x){}
+
+)cpp");
+}
+
+TEST_F(DefineInlineTest, TransformTemplParamNames) {
+  EXPECT_EQ(apply(R"cpp(
+struct Foo {
+  struct Bar {
+template  class, template class Y,
+  int, int Z>
+void foo(X, Y, int W = 5 * Z + 2);
+  };
+};
+
+template  class V, template class W,
+  int X, int Y>
+void Foo::Bar::f^oo(U, W, int Q) {})cpp"),
+R"cpp(
+struct Foo {
+  struct Bar {
+template  class V, template class W,
+  int X, int Y>
+void foo(U, W, int Q = 5 * Y + 2){}
+  };
+};
+
+)cpp");
+}
+
 TEST_F(DefineInlineTest, TransformInlineNamespaces) {
   EXPECT_EQ(apply(R"cpp(
 namespace a { inline namespace b { namespace { struct Foo{}; } } }
Index: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
@@ -226,6 +226,176 @@
   return QualifiedFunc->substr(BodyBegin, BodyEnd - BodyBegin + 1);
 }
 
+/// Generates a replacement that renames \p Dest to \p Source.
+llvm::Error rewriteParameterName(tooling::Replacements &Replacements,
+ const NamedDecl *Dest,
+ llvm::StringRef SourceName) {
+  auto &SM = Dest->getASTContext().getSourceManager();
+  llvm::StringRef DestName = Dest->getName();
+  if (DestName == SourceName)
+return llvm::Error::success();
+  if (auto Err = Replacements.add(tooling::Replacement(
+  SM, Dest->getLocation(), DestName.size(),
+  // If \p Dest is unnamed, we need to insert a space before current
+  // name.
+  ((DestName.empty() ? " " : "") + SourceName).str( {
+return Err;
+  }
+  return llvm::Error::success();
+}
+
+/// Returns a mapping from template names in \p Dest to \p Source, so that they
+/// can be updated in other places like parameter types and default arguments.
+llvm::Expected>
+renameTemplateParameters(tooling::Replacements &Replacements,
+ const FunctionDecl *Dest, const FunctionDecl *Source) {
+  llvm::DenseMap TemplParamNameDestToSource;
+
+  auto *DestTempl = Dest->getDescribedFunctionTemplate();
+  auto *SourceTempl = Source->getDescribedFunctionTemplate();
+  assert(bool(DestTempl) == bool(SourceTempl));
+  if (DestTempl) {
+const auto *DestTPL = DestTempl->getTemplateParameters();
+const auto *SourceTPL = SourceTempl->getTemplateParameters();
+assert(DestTPL->size() == SourceTPL->size());
+
+for (size_t I = 0, EP = DestTPL->size(); I != EP; ++I) {
+  if (auto Err = rewriteParameterName(Replacements, DestTPL->getParam(I),
+  SourceTPL->getParam(I)->getName()))
+return std::move(Err);
+  TemplParamNameDestToSource[DestTPL->getParam(I)->getName()] =
+  SourceTPL->getParam(I)->getName();
+}
+  }
+
+  return TemplParamNameDestToSource;
+}
+
+/// Generates replacements for rewriting dependent types in \p DestParam to the
+/// matching template name in \p TemplParamNameDestToSource.
+llvm::Error
+rewriteParameterType(tooling::Replacements &Replacements,
+ const ParmVarDecl *DestParam,
+ const llvm::DenseMap
+ &TemplParamNameDestToSource) {
+  auto *DestTSI = DestParam->getTypeSourceInfo();
+  if (!DestTSI)
+return llvm::Error::success();
+  const SourceManager &SM = DestParam->getASTContext().getSourceManager();
+
+  // Update paramater types if they are template template or type template.
+  auto DestTL = DestTSI->getTypeLoc();
+  if (auto DestTPTL = DestTL.getAs()) {
+if (auto Err = Replacements.add(tooling::Replacement(
+SM, CharSourceRange(DestTPTL.getSourceRange(), /*ITR=*/true),
+TemplParamNameDestToSource.lookup(
+DestTPTL.getDecl()->getDeclName().getAsString() {
+  return Err;
+}
+  } else if (auto DestTTSL = DestTL.getAs()) {
+std::string TemplName;
+llvm::raw_string_ostream OS(TemplName);
+DestTTSL.getTypePtr()->getTemplate

[PATCH] D65433: [clangd] DefineInline action availability checks

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG74d39a42f109: [clangd] DefineInline action availability 
checks (authored by kadircet).

Changed prior to commit:
  https://reviews.llvm.org/D65433?vs=222158&id=226394#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65433

Files:
  clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
  clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.h
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -24,6 +24,7 @@
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -33,6 +34,8 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+#include 
+#include 
 
 using ::testing::AllOf;
 using ::testing::HasSubstr;
@@ -880,6 +883,170 @@
 EXPECT_EQ(C.second, apply(C.first)) << C.first;
 }
 
+TWEAK_TEST(DefineInline);
+TEST_F(DefineInlineTest, TriggersOnFunctionDecl) {
+  // Basic check for function body and signature.
+  EXPECT_AVAILABLE(R"cpp(
+  class Bar {
+void baz();
+  };
+
+  [[void [[Bar::[[b^a^z() [[{
+return;
+  }
+
+  void foo();
+  [[void [[f^o^o]]() [[{
+return;
+  }
+  )cpp");
+
+  EXPECT_UNAVAILABLE(R"cpp(
+  // Not a definition
+  vo^i[[d^ ^f]]^oo();
+
+  [[vo^id ]]foo[[()]] {[[
+[[(void)(5+3);
+return;]]
+  }]]
+  )cpp");
+}
+
+TEST_F(DefineInlineTest, NoForwardDecl) {
+  Header = "void bar();";
+  EXPECT_UNAVAILABLE(R"cpp(
+  void bar() {
+return;
+  }
+  // FIXME: Generate a decl in the header.
+  void fo^o() {
+return;
+  })cpp");
+}
+
+TEST_F(DefineInlineTest, ReferencedDecls) {
+  EXPECT_AVAILABLE(R"cpp(
+void bar();
+void foo(int test);
+
+void fo^o(int baz) {
+  int x = 10;
+  bar();
+})cpp");
+
+  // Internal symbol usage.
+  Header = "void foo(int test);";
+  EXPECT_UNAVAILABLE(R"cpp(
+void bar();
+void fo^o(int baz) {
+  int x = 10;
+  bar();
+})cpp");
+
+  // Becomes available after making symbol visible.
+  Header = "void bar();" + Header;
+  EXPECT_AVAILABLE(R"cpp(
+void fo^o(int baz) {
+  int x = 10;
+  bar();
+})cpp");
+
+  // FIXME: Move declaration below bar to make it visible.
+  Header.clear();
+  EXPECT_UNAVAILABLE(R"cpp(
+void foo();
+void bar();
+
+void fo^o() {
+  bar();
+})cpp");
+
+  // Order doesn't matter within a class.
+  EXPECT_AVAILABLE(R"cpp(
+class Bar {
+  void foo();
+  void bar();
+};
+
+void Bar::fo^o() {
+  bar();
+})cpp");
+
+  // FIXME: Perform include insertion to make symbol visible.
+  ExtraFiles["a.h"] = "void bar();";
+  Header = "void foo(int test);";
+  EXPECT_UNAVAILABLE(R"cpp(
+#include "a.h"
+void fo^o(int baz) {
+  int x = 10;
+  bar();
+})cpp");
+}
+
+TEST_F(DefineInlineTest, TemplateSpec) {
+  EXPECT_UNAVAILABLE(R"cpp(
+template  void foo();
+template<> void foo();
+
+template<> void f^oo() {
+})cpp");
+  EXPECT_UNAVAILABLE(R"cpp(
+template  void foo();
+
+template<> void f^oo() {
+})cpp");
+  EXPECT_UNAVAILABLE(R"cpp(
+template  struct Foo { void foo(); };
+
+template  void Foo::f^oo() {
+})cpp");
+  EXPECT_AVAILABLE(R"cpp(
+template  void foo();
+void bar();
+template <> void foo();
+
+template<> void f^oo() {
+  bar();
+})cpp");
+}
+
+TEST_F(DefineInlineTest, CheckForCanonDecl) {
+  EXPECT_UNAVAILABLE(R"cpp(
+void foo();
+
+void bar() {}
+void f^oo() {
+  // This bar normally refers to the definition just above, but it is not
+  // visible from the forward declaration of foo.
+  bar();
+})cpp");
+  // Make it available with a forward decl.
+  EXPECT_AVAILABLE(R"cpp(
+void bar();
+void foo();
+
+void bar() {}
+void f^oo() {
+  bar();
+})cpp");
+}
+
+TEST_F(DefineInlineTest, UsingShadowDecls) {
+  EXPECT_UNAVAILABLE(R"cpp(
+  namespace ns1 { void foo(int); }
+  namespace ns2 { void foo(int*); }
+  template 
+  void bar();
+
+  using ns1::foo;
+  using ns2::foo;
+
+  template 
+  void b^ar() {
+foo(T());
+  })cpp");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/TweakTesting.h
===
--- clang-tools-extra/clangd/unittests/TweakTesting.h
+++ clang-tools-extra/clangd/unittests/TweakTesting.h
@@ -10,8 +1

[PATCH] D66647: [clangd] DefineInline action apply logic with fully qualified names

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdfd6374c784f: [clangd] DefineInline action apply logic with 
fully qualified names (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66647

Files:
  clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.h
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -25,6 +25,7 @@
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -40,11 +41,16 @@
 using ::testing::AllOf;
 using ::testing::HasSubstr;
 using ::testing::StartsWith;
+using ::testing::ElementsAre;
 
 namespace clang {
 namespace clangd {
 namespace {
 
+MATCHER_P2(FileWithContents, FileName, Contents, "") {
+  return arg.first() == FileName && arg.second == Contents;
+}
+
 TEST(FileEdits, AbsolutePath) {
   auto RelPaths = {"a.h", "foo.cpp", "test/test.cpp"};
 
@@ -1047,6 +1053,512 @@
   })cpp");
 }
 
+TEST_F(DefineInlineTest, TransformNestedNamespaces) {
+  EXPECT_EQ(apply(R"cpp(
+  namespace a {
+void bar();
+namespace b {
+  void baz();
+  namespace c {
+void aux();
+  }
+}
+  }
+
+  void foo();
+  using namespace a;
+  using namespace b;
+  using namespace c;
+  void f^oo() {
+bar();
+a::bar();
+
+baz();
+b::baz();
+a::b::baz();
+
+aux();
+c::aux();
+b::c::aux();
+a::b::c::aux();
+  })cpp"), R"cpp(
+  namespace a {
+void bar();
+namespace b {
+  void baz();
+  namespace c {
+void aux();
+  }
+}
+  }
+
+  void foo(){
+a::bar();
+a::bar();
+
+a::b::baz();
+a::b::baz();
+a::b::baz();
+
+a::b::c::aux();
+a::b::c::aux();
+a::b::c::aux();
+a::b::c::aux();
+  }
+  using namespace a;
+  using namespace b;
+  using namespace c;
+  )cpp");
+}
+
+TEST_F(DefineInlineTest, TransformUsings) {
+  EXPECT_EQ(apply(R"cpp(
+  namespace a { namespace b { namespace c { void aux(); } } }
+
+  void foo();
+  void f^oo() {
+using namespace a;
+using namespace b;
+using namespace c;
+using c::aux;
+namespace d = c;
+  })cpp"),
+R"cpp(
+  namespace a { namespace b { namespace c { void aux(); } } }
+
+  void foo(){
+using namespace a;
+using namespace a::b;
+using namespace a::b::c;
+using a::b::c::aux;
+namespace d = a::b::c;
+  }
+  )cpp");
+}
+
+TEST_F(DefineInlineTest, TransformDecls) {
+  EXPECT_EQ(apply(R"cpp(
+  void foo();
+  void f^oo() {
+class Foo {
+public:
+  void foo();
+  int x;
+  static int y;
+};
+Foo::y = 0;
+
+enum En { Zero, One };
+En x = Zero;
+
+enum class EnClass { Zero, One };
+EnClass y = EnClass::Zero;
+  })cpp"),
+R"cpp(
+  void foo(){
+class Foo {
+public:
+  void foo();
+  int x;
+  static int y;
+};
+Foo::y = 0;
+
+enum En { Zero, One };
+En x = Zero;
+
+enum class EnClass { Zero, One };
+EnClass y = EnClass::Zero;
+  }
+  )cpp");
+}
+
+TEST_F(DefineInlineTest, TransformTemplDecls) {
+  EXPECT_EQ(apply(R"cpp(
+  namespace a {
+template  class Bar {
+public:
+  void bar();
+};
+template  T bar;
+template  void aux() {}
+  }
+
+  void foo();
+
+  using namespace a;
+  void f^oo() {
+bar>.bar();
+aux>();
+  })cpp"),
+R"cpp(
+  namespace a {
+template  class Bar {
+public:
+  void bar();
+};
+template  T bar;
+template  void aux() {}
+  }
+
+  void foo(){
+a::bar>.bar();
+a::aux>();
+  }
+
+  using namespace a;
+  )cpp");
+}
+
+TEST_F(DefineInlineTest, TransformMembers) {
+  EXPECT_EQ(apply(R"cpp(
+  class Foo {
+void foo();
+  };
+
+  void Foo::f^oo() {
+return;
+  })cpp"),
+R"cpp(
+  class Foo {
+void foo(){
+return;
+  }
+  };
+
+  )cpp");
+
+  ExtraFiles["a.h"] = R"cpp(
+  class Foo {
+void foo();
+  };)cpp";
+
+  llvm::StringMap EditedFiles;
+  EXPECT_EQ(apply(R"cpp(
+  #include "a.h"
+  void Foo::f^oo() {
+return;
+  })cpp",
+  &EditedFiles),
+R"cpp(
+  #include "a.h"
+  )cpp");
+  EXPECT_THAT(EditedFiles,
+  ElementsAre(FileWithContents(testPath("a.h"),
+R"cpp(
+  class Foo {
+void foo(){
+return;
+  }
+  };)cpp")));
+}
+
+TEST_F(DefineInlineTest, TransformDependentTypes) {
+  EXPECT_EQ(apply(R"cpp(
+  names

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 226393.
Typz added a comment.

Rebase on top of D50078 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D32478

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/ContinuationIndenter.h
  clang/lib/Format/Format.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestJS.cpp

Index: clang/unittests/Format/FormatTestJS.cpp
===
--- clang/unittests/Format/FormatTestJS.cpp
+++ clang/unittests/Format/FormatTestJS.cpp
@@ -277,7 +277,7 @@
   verifyFormat("var x = a() in\n"
".aa.aa;");
   FormatStyle Style = getGoogleJSStyleWithColumns(80);
-  Style.AlignOperands = true;
+  Style.AlignOperands = FormatStyle::OAS_Align;
   verifyFormat("var x = a() in\n"
".aa.aa;",
Style);
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4111,6 +4111,9 @@
"  > c) {\n"
"}",
Style);
+  verifyFormat("return a\n"
+   "   && bbb;",
+   Style);
   verifyFormat("return (a)\n"
"   // comment\n"
"   + b;",
@@ -4139,7 +4142,7 @@
 
   Style.ColumnLimit = 60;
   verifyFormat("zz\n"
-   "= b\n"
+   "= \n"
"  >> (aa);",
Style);
 
@@ -4148,7 +4151,7 @@
   Style.TabWidth = 4;
   Style.UseTab = FormatStyle::UT_Always;
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
-  Style.AlignOperands = false;
+  Style.AlignOperands = FormatStyle::OAS_DontAlign;
   EXPECT_EQ("return someVeryVeryLongConditionThatBarelyFitsOnALine\n"
 "\t&& (someOtherLongishConditionPart1\n"
 "\t\t|| someOtherEvenLongerNestedConditionPart2);",
@@ -4158,6 +4161,107 @@
Style));
 }
 
+TEST_F(FormatTest, ExpressionIndentationStrictAlign) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
+  Style.AlignOperands = FormatStyle::OAS_AlignAfterOperator;
+
+  verifyFormat("bool value = a\n"
+   "   + a\n"
+   "   + a\n"
+   "  == a\n"
+   " * b\n"
+   " + b\n"
+   "  && a\n"
+   " * a\n"
+   " > c;",
+   Style);
+  verifyFormat("if (a\n"
+   "* \n"
+   "+ aa\n"
+   "== bbb) {\n}",
+   Style);
+  verifyFormat("if (a\n"
+   "+ \n"
+   "  * aa\n"
+   "== bbb) {\n}",
+   Style);
+  verifyFormat("if (a\n"
+   "== \n"
+   "   * aa\n"
+   "   + bbb) {\n}",
+   Style);
+  verifyFormat("if () {\n"
+   "} else if (a\n"
+   "   && b // break\n"
+   "  > c) {\n"
+   "}",
+   Style);
+  verifyFormat("return a\n"
+   "&& bbb;",
+   Style);
+  verifyFormat("return (a)\n"
+   " // comment\n"
+   " + b;",
+   Style);
+  verif

[PATCH] D68937: [clangd] Add parameter renaming to define-inline code action

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D68937#1710915 , @kadircet wrote:

> I totally agree that the solution you proposed would also work, but don't 
> think it would be any less code. Since one needs to correlate
>  parameters between two different declarations, and `findExplicitReferences` 
> doesn't really provide a nice way to achieve that. One
>  would have to rely on `SourceLocation` ordering or in the order callback was 
> issued(which implicitly relies on AST traversal order).
>
> It would be nice to unify the rewrite parameter name/type/defaultarg logics, 
> but not sure if it is worth.


I believe it should be less and much simpler code. In particular, I propose 
something like this:

  DenseSet ParametersToRename;
  // Fill ParametersToRename with template and function parameters.
  
  DenseMap> References;
  findExplicitReferences(Func, [&](ReferenceLoc R) {
if (!ParametersToRename.count(R.Target))
  return;
References[R.Target].push_back(R.NameLoc);
  });
  
  for (auto [Param, Locations] : References) {
auto NewName = NewNameForParam(Param);
for (auto L : Locations)
  Replacements.add(replaceToken(L, NewName));
  }

That should also handle various interesting cases like `try-catch` blocks and 
constructor initializer lists.




Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:229
 
+/// Generates a replacement that renames \p Dest to \p Source.
+llvm::Error rewriteParameterName(tooling::Replacements &Replacements,

NIT: `that renames \p Dest to \p SourceName`



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:232
+ const NamedDecl *Dest,
+ llvm::StringRef SourceName) {
+  auto &SM = Dest->getASTContext().getSourceManager();

NIT: call it `NewName`?
`Source` and `Target` are very define-inline-specific. This function actually 
does a more general thing and using the define-inline terminology hurt the 
redability a little.



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:239
+  SM, Dest->getLocation(), DestName.size(),
+  // If \p Dest is unnamed, we need to insert a space before current
+  // name.

Why do we need to insert a space here? could you add an example?
I guess it's 
```
void foo(int^) // <-- avoid gluing 'int' and the new name here
```



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:241
+  // name.
+  ((DestName.empty() ? " " : "") + SourceName).str( {
+return Err;

NIT: could you move this expression and a comment to a separate variable?
should simplify the if condition and improve readability


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68937



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

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/include/clang/Format/Format.h:187
   /// expressions.
-  ///
-  /// Specifically, this aligns operands of a single expression that needs to 
be
-  /// split over multiple lines, e.g.:
-  /// \code
-  ///   int aaa = bbb +
-  /// ccc;
-  /// \endcode
-  bool AlignOperands;
+  OperandAlignmentStyle AlignOperands;
 

I think you are missing an entry in the operator== in Format.h


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D32478



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

[PATCH] D68024: [clangd] Implement GetEligiblePoints

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd62e3ed3f4b9: [clangd] Implement GetEligiblePoints (authored 
by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68024

Files:
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/unittests/SourceCodeTests.cpp

Index: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
===
--- clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -19,6 +19,7 @@
 #include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 
 namespace clang {
 namespace clangd {
@@ -618,6 +619,67 @@
 Test.llvm::Annotations::point("bar"));
 }
 
+TEST(SourceCodeTests, GetEligiblePoints) {
+  constexpr struct {
+const char *Code;
+const char *FullyQualifiedName;
+const char *EnclosingNamespace;
+  } Cases[] = {
+  {R"cpp(// FIXME: We should also mark positions before and after
+ //declarations/definitions as eligible.
+  namespace ns1 {
+  namespace a { namespace ns2 {} }
+  namespace ns2 {^
+  void foo();
+  namespace {}
+  void bar() {}
+  namespace ns3 {}
+  class T {};
+  ^}
+  using namespace ns2;
+  })cpp",
+   "ns1::ns2::symbol", "ns1::ns2::"},
+  {R"cpp(
+  namespace ns1 {^
+  namespace a { namespace ns2 {} }
+  namespace b {}
+  namespace ns {}
+  ^})cpp",
+   "ns1::ns2::symbol", "ns1::"},
+  {R"cpp(
+  namespace x {
+  namespace a { namespace ns2 {} }
+  namespace b {}
+  namespace ns {}
+  }^)cpp",
+   "ns1::ns2::symbol", ""},
+  {R"cpp(
+  namespace ns1 {
+  namespace ns2 {^^}
+  namespace b {}
+  namespace ns2 {^^}
+  }
+  namespace ns1 {namespace ns2 {^^}})cpp",
+   "ns1::ns2::symbol", "ns1::ns2::"},
+  {R"cpp(
+  namespace ns1 {^
+  namespace ns {}
+  namespace b {}
+  namespace ns {}
+  ^}
+  namespace ns1 {^namespace ns {}^})cpp",
+   "ns1::ns2::symbol", "ns1::"},
+  };
+  for (auto Case : Cases) {
+Annotations Test(Case.Code);
+
+auto Res = getEligiblePoints(Test.code(), Case.FullyQualifiedName,
+ format::getLLVMStyle());
+EXPECT_THAT(Res.EligiblePoints, testing::ElementsAreArray(Test.points()))
+<< Test.code();
+EXPECT_EQ(Res.EnclosingNamespace, Case.EnclosingNamespace) << Test.code();
+  }
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/SourceCode.h
===
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -262,6 +262,27 @@
 std::vector visibleNamespaces(llvm::StringRef Code,
const format::FormatStyle &Style);
 
+/// Represents locations that can accept a definition.
+struct EligibleRegion {
+  /// Namespace that owns all of the EligiblePoints, e.g.
+  /// namespace a{ namespace b {^ void foo();^} }
+  /// It will be “a::b” for both carrot locations.
+  std::string EnclosingNamespace;
+  /// Offsets into the code marking eligible points to insert a function
+  /// definition.
+  std::vector EligiblePoints;
+};
+
+/// Returns most eligible region to insert a definition for \p
+/// FullyQualifiedName in the \p Code.
+/// Pseudo parses \pCode under the hood to determine namespace decls and
+/// possible insertion points. Choses the region that matches the longest prefix
+/// of \p FullyQualifiedName. Returns EOF if there are no shared namespaces.
+/// \p FullyQualifiedName should not contain anonymous namespaces.
+EligibleRegion getEligiblePoints(llvm::StringRef Code,
+ llvm::StringRef FullyQualifiedName,
+ const format::FormatStyle &Style);
+
 struct DefinedMacro {
   llvm::StringRef Name;
   const MacroInfo *Info;
Index: clang-tools-extra/clangd/SourceCode.cpp
===
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -20,9 +20,11 @@
 #include "clang/Format/Format.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Preprocessor.h"
+#include "clang/Lex/Token.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/None.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm

[PATCH] D68937: [clangd] Add parameter renaming to define-inline code action

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Ah, we're actually trying to rename parameters in the declaration to match the 
ones in the definition... So the try-catch blocks are not a problem, actually


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68937



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

[clang-tools-extra] 8aa84ad - [clangd] Store Index in Tweak::Selection

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2019-10-25T12:15:20+02:00
New Revision: 8aa84ad37db7ddbff5c1a2e4ef8ff2a616f1da57

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

LOG: [clangd] Store Index in Tweak::Selection

Summary:
Incoming define out-of-line tweak requires access to index.

This patch simply propogates the index in ClangdServer to Tweak::Selection while
passing the AST. Also updates TweakTest to accommodate this change.

Reviewers: ilya-biryukov

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

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/refactor/Tweak.cpp
clang-tools-extra/clangd/refactor/Tweak.h
clang-tools-extra/clangd/unittests/TweakTesting.cpp
clang-tools-extra/clangd/unittests/TweakTesting.h

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index 4f1fe8f5b08b..64ccbe417c24 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -367,7 +367,7 @@ tweakSelection(const Range &Sel, const InputsAndAST &AST) {
   auto End = positionToOffset(AST.Inputs.Contents, Sel.end);
   if (!End)
 return End.takeError();
-  return Tweak::Selection(AST.AST, *Begin, *End);
+  return Tweak::Selection(AST.Inputs.Index, AST.AST, *Begin, *End);
 }
 
 void ClangdServer::enumerateTweaks(PathRef File, Range Sel,

diff  --git a/clang-tools-extra/clangd/refactor/Tweak.cpp 
b/clang-tools-extra/clangd/refactor/Tweak.cpp
index 4f3c40d1eb13..2dc091ed762a 100644
--- a/clang-tools-extra/clangd/refactor/Tweak.cpp
+++ b/clang-tools-extra/clangd/refactor/Tweak.cpp
@@ -9,6 +9,7 @@
 #include "Logger.h"
 #include "Path.h"
 #include "SourceCode.h"
+#include "index/Index.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
@@ -44,9 +45,10 @@ void validateRegistry() {
 }
 } // namespace
 
-Tweak::Selection::Selection(ParsedAST &AST, unsigned RangeBegin,
-unsigned RangeEnd)
-: AST(AST), SelectionBegin(RangeBegin), SelectionEnd(RangeEnd),
+Tweak::Selection::Selection(const SymbolIndex *Index, ParsedAST &AST,
+unsigned RangeBegin, unsigned RangeEnd)
+: Index(Index), AST(AST), SelectionBegin(RangeBegin),
+  SelectionEnd(RangeEnd),
   ASTSelection(AST.getASTContext(), AST.getTokens(), RangeBegin, RangeEnd) 
{
   auto &SM = AST.getSourceManager();
   Code = SM.getBufferData(SM.getMainFileID());

diff  --git a/clang-tools-extra/clangd/refactor/Tweak.h 
b/clang-tools-extra/clangd/refactor/Tweak.h
index 42b7fb0684e8..de655abd98c7 100644
--- a/clang-tools-extra/clangd/refactor/Tweak.h
+++ b/clang-tools-extra/clangd/refactor/Tweak.h
@@ -24,6 +24,7 @@
 #include "Protocol.h"
 #include "Selection.h"
 #include "SourceCode.h"
+#include "index/Index.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringMap.h"
@@ -46,9 +47,12 @@ class Tweak {
 public:
   /// Input to prepare and apply tweaks.
   struct Selection {
-Selection(ParsedAST &AST, unsigned RangeBegin, unsigned RangeEnd);
+Selection(const SymbolIndex *Index, ParsedAST &AST, unsigned RangeBegin,
+  unsigned RangeEnd);
 /// The text of the active document.
 llvm::StringRef Code;
+/// The Index for handling codebase related queries.
+const SymbolIndex *Index = nullptr;
 /// Parsed AST of the active file.
 ParsedAST *
 /// A location of the cursor in the editor.

diff  --git a/clang-tools-extra/clangd/unittests/TweakTesting.cpp 
b/clang-tools-extra/clangd/unittests/TweakTesting.cpp
index 4e7c14de6f0a..e5b619639a5d 100644
--- a/clang-tools-extra/clangd/unittests/TweakTesting.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTesting.cpp
@@ -14,6 +14,7 @@
 #include "refactor/Tweak.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/Support/Error.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
 
@@ -62,7 +63,7 @@ std::pair rangeOrPoint(const Annotations 
&A) {
   cantFail(positionToOffset(A.code(), SelectionRng.end))};
 }
 
-MATCHER_P4(TweakIsAvailable, TweakID, Ctx, Header, ExtraFiles,
+MATCHER_P5(TweakIsAvailable, TweakID, Ctx, Header, ExtraFiles, Index,
(TweakID + (negation ? " is unavailable" : " is available")).str()) 
{
   std::string WrappedCode = wrap(Ctx, arg);
   Annotations Input(WrappedCode);
@@ -72,7 +73,7 @@ MATCHER_P4(TweakIsAvailable, TweakID, Ctx, Header, ExtraFiles,
   TU.Code = Input.code();
   TU.AdditionalFiles = std::move(ExtraFiles);
   ParsedAST AST = TU.build();
-  Tweak::Selection S(AST, Selection.first, Selection.second);
+  Twea

[PATCH] D69383: [RISCV] Match GCC `-march`/`-mabi` driver defaults

[Sam Elliott via Phabricator via cfe-commits]

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

I agree backwards compatibility is hard here.

- This method was introduced to have a single place to choose a default `march` 
string if none was chosen before. I think this change is useful (saves defaults 
being calculated in a multitude of other places, which means they may not 
agree).
- The choice of a default march string was based, as closely as possible, on 
`config.gcc`. This may not be what we want (and is the source of backwards 
compatibility issues).
- I 100% agree that a release notes entry is needed. I will write one just as 
soon as we finalize the default behaviour.
- The elf multilib changes in D67508  seem to 
be more complex, as `config.gcc` chooses a march/mabi default that is not built 
by `t-elf-multilib`.

I think I would understand updating the logic to use fewer architecture 
extensions for a given ABI (i.e., default to `rv32i` unless someone asks for 
`ilp32f/d`) rather than more (`rv32gc` when someone asks for `ilp32`). This 
should be more backwards compatible, but also requires carefully ensuring the 
logic is not circular.

At the very least, I do not plan to merge this patch until we have had a chance 
to discuss it next Thursday (31st Oct) on the RISC-V backend call.




Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:476
+if (MArch.startswith_lower("rv32")) {
+  if (MArch.substr(4).contains_lower("d") ||
+  MArch.startswith_lower("rv32g"))

simoncook wrote:
> Won’t this break if the user specifies a X/Z extension that has a d in the 
> name, so eg rv32iXd will try to use the ilp32d abi by default? For future 
> proofing, I think we may need to do a full parse of the isa string to verify 
> that d does actually mean the standard D-extension
You're right that having "d" in any extension name will cause issues. The same 
is true for any gcc build today (9.2.0).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69383



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

[PATCH] D69165: [clangd] Store Index in Tweak::Selection

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8aa84ad37db7: [clangd] Store Index in Tweak::Selection 
(authored by kadircet).

Changed prior to commit:
  https://reviews.llvm.org/D69165?vs=225871&id=226399#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69165

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/refactor/Tweak.cpp
  clang-tools-extra/clangd/refactor/Tweak.h
  clang-tools-extra/clangd/unittests/TweakTesting.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.h

Index: clang-tools-extra/clangd/unittests/TweakTesting.h
===
--- clang-tools-extra/clangd/unittests/TweakTesting.h
+++ clang-tools-extra/clangd/unittests/TweakTesting.h
@@ -10,9 +10,11 @@
 #define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TWEAKTESTING_H
 
 #include "TestTU.h"
+#include "index/Index.h"
 #include "llvm/ADT/StringMap.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 #include 
 
 namespace clang {
@@ -66,6 +68,9 @@
   // Context in which snippets of code should be placed to run tweaks.
   CodeContext Context = File;
 
+  // Index to be passed into Tweak::Selection.
+  std::unique_ptr Index = nullptr;
+
   // Apply the current tweak to the range (or point) in MarkedCode.
   // MarkedCode will be wrapped according to the Context.
   //  - if the tweak produces edits, returns the edited code (without markings)
Index: clang-tools-extra/clangd/unittests/TweakTesting.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTesting.cpp
+++ clang-tools-extra/clangd/unittests/TweakTesting.cpp
@@ -14,6 +14,7 @@
 #include "refactor/Tweak.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/Support/Error.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
 
@@ -62,7 +63,7 @@
   cantFail(positionToOffset(A.code(), SelectionRng.end))};
 }
 
-MATCHER_P4(TweakIsAvailable, TweakID, Ctx, Header, ExtraFiles,
+MATCHER_P5(TweakIsAvailable, TweakID, Ctx, Header, ExtraFiles, Index,
(TweakID + (negation ? " is unavailable" : " is available")).str()) {
   std::string WrappedCode = wrap(Ctx, arg);
   Annotations Input(WrappedCode);
@@ -72,7 +73,7 @@
   TU.Code = Input.code();
   TU.AdditionalFiles = std::move(ExtraFiles);
   ParsedAST AST = TU.build();
-  Tweak::Selection S(AST, Selection.first, Selection.second);
+  Tweak::Selection S(Index, AST, Selection.first, Selection.second);
   auto PrepareResult = prepareTweak(TweakID, S);
   if (PrepareResult)
 return true;
@@ -94,7 +95,7 @@
   TU.Code = Input.code();
   TU.ExtraArgs = ExtraArgs;
   ParsedAST AST = TU.build();
-  Tweak::Selection S(AST, Selection.first, Selection.second);
+  Tweak::Selection S(Index.get(), AST, Selection.first, Selection.second);
 
   auto T = prepareTweak(TweakID, S);
   if (!T) {
@@ -129,8 +130,8 @@
 }
 
 ::testing::Matcher TweakTest::isAvailable() const {
-  return TweakIsAvailable(llvm::StringRef(TweakID), Context, Header,
-  ExtraFiles);
+  return TweakIsAvailable(llvm::StringRef(TweakID), Context, Header, ExtraFiles,
+  Index.get());
 }
 
 std::vector TweakTest::expandCases(llvm::StringRef MarkedCode) {
Index: clang-tools-extra/clangd/refactor/Tweak.h
===
--- clang-tools-extra/clangd/refactor/Tweak.h
+++ clang-tools-extra/clangd/refactor/Tweak.h
@@ -24,6 +24,7 @@
 #include "Protocol.h"
 #include "Selection.h"
 #include "SourceCode.h"
+#include "index/Index.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringMap.h"
@@ -46,9 +47,12 @@
 public:
   /// Input to prepare and apply tweaks.
   struct Selection {
-Selection(ParsedAST &AST, unsigned RangeBegin, unsigned RangeEnd);
+Selection(const SymbolIndex *Index, ParsedAST &AST, unsigned RangeBegin,
+  unsigned RangeEnd);
 /// The text of the active document.
 llvm::StringRef Code;
+/// The Index for handling codebase related queries.
+const SymbolIndex *Index = nullptr;
 /// Parsed AST of the active file.
 ParsedAST *
 /// A location of the cursor in the editor.
Index: clang-tools-extra/clangd/refactor/Tweak.cpp
===
--- clang-tools-extra/clangd/refactor/Tweak.cpp
+++ clang-tools-extra/clangd/refactor/Tweak.cpp
@@ -9,6 +9,7 @@
 #include "Logger.h"
 #include "Path.h"
 #include "SourceCode.h"
+#include "index/Index.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
@@ -44,9 +45,10 @@
 }
 } // namespace
 
-Tweak::Selection::Selection(ParsedAST &AST, unsigned RangeBegin,
-unsigned RangeEnd)
-: AST(AST), SelectionBegin(RangeBegin)

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Format/Format.h:187
   /// expressions.
-  ///
-  /// Specifically, this aligns operands of a single expression that needs to 
be
-  /// split over multiple lines, e.g.:
-  /// \code
-  ///   int aaa = bbb +
-  /// ccc;
-  /// \endcode
-  bool AlignOperands;
+  OperandAlignmentStyle AlignOperands;
 

MyDeveloperDay wrote:
> I think you are missing an entry in the operator== in Format.h
It is there already, since this is not a new field, but just changing the type 
of an existing field.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D32478



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

[PATCH] D69426: [clang] Switch arm-mve-intrinsics tests to use %clang_cc1.

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham created this revision.
simon_tatham added a reviewer: dmgreen.
Herald added subscribers: cfe-commits, kristof.beyls.
Herald added a project: clang.
simon_tatham edited the summary of this revision.

It isn't really necessary for them to run the clang driver, and it's
more efficient not to (and also more stable against driver changes).
Now they invoke cc1 directly, more like the analogous NEON tests.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69426

Files:
  clang/test/CodeGen/arm-mve-intrinsics/scalar-shifts.c
  clang/test/CodeGen/arm-mve-intrinsics/vadc.c
  clang/test/CodeGen/arm-mve-intrinsics/vaddq.c
  clang/test/CodeGen/arm-mve-intrinsics/vcvt.c
  clang/test/CodeGen/arm-mve-intrinsics/vld24.c
  clang/test/CodeGen/arm-mve-intrinsics/vldr.c
  clang/test/CodeGen/arm-mve-intrinsics/vminvq.c


Index: clang/test/CodeGen/arm-mve-intrinsics/vminvq.c
===
--- clang/test/CodeGen/arm-mve-intrinsics/vminvq.c
+++ clang/test/CodeGen/arm-mve-intrinsics/vminvq.c
@@ -1,6 +1,6 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
-// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main+mve.fp 
-mfloat-abi=hard -O0 -Xclang -disable-O0-optnone -fno-discard-value-names -S 
-emit-llvm -o - %s | opt -S -mem2reg | FileCheck %s
-// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main+mve.fp 
-mfloat-abi=hard -DPOLYMORPHIC -O0 -Xclang -disable-O0-optnone 
-fno-discard-value-names -S -emit-llvm -o - %s | opt -S -mem2reg | FileCheck %s
+// RUN: %clang_cc1 -triple thumbv8.1m.main-arm-none-eabi -target-feature 
+mve.fp -mfloat-abi hard -O0 -disable-O0-optnone -S -emit-llvm -o - %s | opt -S 
-mem2reg | FileCheck %s
+// RUN: %clang_cc1 -triple thumbv8.1m.main-arm-none-eabi -target-feature 
+mve.fp -mfloat-abi hard -O0 -disable-O0-optnone -DPOLYMORPHIC -S -emit-llvm -o 
- %s | opt -S -mem2reg | FileCheck %s
 
 #include 
 
Index: clang/test/CodeGen/arm-mve-intrinsics/vldr.c
===
--- clang/test/CodeGen/arm-mve-intrinsics/vldr.c
+++ clang/test/CodeGen/arm-mve-intrinsics/vldr.c
@@ -1,5 +1,5 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
-// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main+mve.fp 
-mfloat-abi=hard -O0 -Xclang -disable-O0-optnone -fno-discard-value-names -S 
-emit-llvm -o - %s | opt -S -mem2reg | FileCheck %s
+// RUN: %clang_cc1 -triple thumbv8.1m.main-arm-none-eabi -target-feature 
+mve.fp -mfloat-abi hard -O0 -disable-O0-optnone -S -emit-llvm -o - %s | opt -S 
-mem2reg | FileCheck %s
 
 #include 
 
Index: clang/test/CodeGen/arm-mve-intrinsics/vld24.c
===
--- clang/test/CodeGen/arm-mve-intrinsics/vld24.c
+++ clang/test/CodeGen/arm-mve-intrinsics/vld24.c
@@ -1,6 +1,6 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
-// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main+mve.fp 
-mfloat-abi=hard -O0 -Xclang -disable-O0-optnone -fno-discard-value-names -S 
-emit-llvm -o - %s | opt -S -mem2reg -sroa -early-cse | FileCheck %s
-// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main+mve.fp 
-mfloat-abi=hard -DPOLYMORPHIC -O0 -Xclang -disable-O0-optnone 
-fno-discard-value-names -S -emit-llvm -o - %s | opt -S -mem2reg -sroa 
-early-cse | FileCheck %s
+// RUN: %clang_cc1 -triple thumbv8.1m.main-arm-none-eabi -target-feature 
+mve.fp -mfloat-abi hard -O0 -disable-O0-optnone -S -emit-llvm -o - %s | opt -S 
-mem2reg -sroa -early-cse | FileCheck %s
+// RUN: %clang_cc1 -triple thumbv8.1m.main-arm-none-eabi -target-feature 
+mve.fp -mfloat-abi hard -O0 -disable-O0-optnone -DPOLYMORPHIC -S -emit-llvm -o 
- %s | opt -S -mem2reg -sroa -early-cse | FileCheck %s
 
 #include 
 
Index: clang/test/CodeGen/arm-mve-intrinsics/vcvt.c
===
--- clang/test/CodeGen/arm-mve-intrinsics/vcvt.c
+++ clang/test/CodeGen/arm-mve-intrinsics/vcvt.c
@@ -1,5 +1,5 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
-// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main+mve.fp 
-mfloat-abi=hard -O0 -Xclang -disable-O0-optnone -fno-discard-value-names -S 
-emit-llvm -o - %s | opt -S -mem2reg | FileCheck %s
+// RUN: %clang_cc1 -triple thumbv8.1m.main-arm-none-eabi -target-feature 
+mve.fp -mfloat-abi hard -O0 -disable-O0-optnone -S -emit-llvm -o - %s | opt -S 
-mem2reg | FileCheck %s
 
 #include 
 
Index: clang/test/CodeGen/arm-mve-intrinsics/vaddq.c
===
--- clang/test/CodeGen/arm-mve-intrinsics/vaddq.c
+++ clang/test/CodeGen/arm-mve-intrinsics/vaddq.c
@@ -1,6 +1,6 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
-// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main+mve.fp 
-mfloat-abi=hard -O0 -Xclang -di

[PATCH] D69426: [clang] Switch arm-mve-intrinsics tests to use %clang_cc1.

[Dave Green via Phabricator via cfe-commits]

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

Sounds good.

But make sure that  -fno-discard-value-names is still included. Otherwise the 
names like "entry" will not be output, so the tests fail in "non-assert" builds.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69426



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

[PATCH] D69382: [clangd] Do not insert parentheses when completing a using declaration

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1466
   Output.Completions.back().Score = C.second;
   Output.Completions.back().CompletionTokenRange = ReplacedRange;
 }

why not handle `SnippetSuffix` in here ?

instead of propagating `IsUsingDeclaration`, we can just drop 
`Output.Completions.back().SnippetSuffix` in here, which sounds like a more 
appropriate layering.
considering we don't really have context specific knowledge in 
`CodeCompletionBuilder` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69382



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

[PATCH] D68539: [clang-tidy] fix for readability-identifier-naming incorrectly fixes variables which become keywords

[Daniel via Phabricator via cfe-commits]

Daniel599 updated this revision to Diff 226403.
Daniel599 added a comment.

removed curly braces


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68539

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  
clang-tools-extra/test/clang-tidy/readability-identifier-naming-bugfix-name-conflicts.cpp
  clang/include/clang/Basic/IdentifierTable.h

Index: clang/include/clang/Basic/IdentifierTable.h
===
--- clang/include/clang/Basic/IdentifierTable.h
+++ clang/include/clang/Basic/IdentifierTable.h
@@ -581,6 +581,8 @@
   iterator end() const   { return HashTable.end(); }
   unsigned size() const  { return HashTable.size(); }
 
+  iterator find(StringRef Name) const { return HashTable.find(Name); }
+
   /// Print some statistics to stderr that indicate how well the
   /// hashing is doing.
   void PrintStats() const;
Index: clang-tools-extra/test/clang-tidy/readability-identifier-naming-bugfix-name-conflicts.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/readability-identifier-naming-bugfix-name-conflicts.cpp
@@ -0,0 +1,27 @@
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
+// RUN:   -config='{CheckOptions: [ \
+// RUN: {key: readability-identifier-naming.ParameterCase, value: lower_case} \
+// RUN:   ]}'
+
+int func(int Break) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: invalid case style for parameter 'Break'; cannot be fixed because 'break' would conflict with a keyword
+  // CHECK-FIXES: {{^}}int func(int Break) {{{$}}
+  if (Break == 1) {
+// CHECK-FIXES: {{^}}  if (Break == 1) {{{$}}
+return 2;
+  }
+
+  return 0;
+}
+
+#define foo 3
+int func2(int Foo) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: invalid case style for parameter 'Foo'; cannot be fixed because 'foo' would conflict with a macro definition
+  // CHECK-FIXES: {{^}}int func2(int Foo) {{{$}}
+  if (Foo == 1) {
+// CHECK-FIXES: {{^}}  if (Foo == 1) {{{$}}
+return 2;
+  }
+
+  return 0;
+}
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
===
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
@@ -65,6 +65,24 @@
 std::string Suffix;
   };
 
+  /// This enum will be used in %select of the diagnostic message.
+  /// Each value below IgnoreFailureThreshold should have an error message.
+  enum class ShouldFixStatus {
+ShouldFix,
+ConflictsWithKeyword, /// The fixup will conflict with a language keyword,
+  /// so we can't fix it automatically.
+ConflictsWithMacroDefinition, /// The fixup will conflict with a macro
+  /// definition, so we can't fix it
+  /// automatically.
+
+/// Values pass this threshold will be ignored completely
+/// i.e no message, no fixup.
+IgnoreFailureThreshold,
+
+InsideMacro, /// If the identifier was used or declared within a macro we
+ /// won't offer a fixup for safety reasons.
+  };
+
   /// Holds an identifier name check failure, tracking the kind of the
   /// identifer, its possible fixup and the starting locations of all the
   /// identifier usages.
@@ -76,13 +94,19 @@
 ///
 /// ie: if the identifier was used or declared within a macro we won't offer
 /// a fixup for safety reasons.
-bool ShouldFix;
+bool ShouldFix() const { return FixStatus == ShouldFixStatus::ShouldFix; }
+
+bool ShouldNotify() const {
+  return FixStatus < ShouldFixStatus::IgnoreFailureThreshold;
+}
+
+ShouldFixStatus FixStatus = ShouldFixStatus::ShouldFix;
 
 /// A set of all the identifier usages starting SourceLocation, in
 /// their encoded form.
 llvm::DenseSet RawUsageLocs;
 
-NamingCheckFailure() : ShouldFix(true) {}
+NamingCheckFailure() = default;
   };
 
   typedef std::pair NamingCheckId;
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -691,10 +691,11 @@
   if (!Failure.RawUsageLocs.insert(FixLocation.getRawEncoding()).second)
 return;
 
-  if (!Failure.ShouldFix)
+  if (!Failure.ShouldFix())
 return;
 
-  Failure.ShouldFix = utils::rangeCanBeFixed(Range, SourceMgr);
+  if (!utils::rangeCanBeFixed(Range, SourceMgr))
+Failure.FixStatus = IdentifierNamingCheck::ShouldFixStatus::InsideMacro;
 }
 
 /// Convenience method when the usage to be added is a NamedDecl
@@ -873,6 +874

[PATCH] D69426: [clang] Switch arm-mve-intrinsics tests to use %clang_cc1.

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham added a comment.

There's no need, as far as I can see. `-fno-discard-value-names` is a driver 
option only. On the cc1 command line the option is `-discard-value-names`, and 
it has no negative form: you turn it off by not specifying it in the first 
place, which indeed these command lines don't.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69426



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

[PATCH] D69427: Fix compilation error in clangd/refactor/tweaks/ExpandAutoType.cpp

[Pavel Samolysov via Phabricator via cfe-commits]

psamolysov created this revision.
psamolysov added projects: clang, clang-tools-extra.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, 
MaskRay, ilya-biryukov.

During the compilation of the `clangd/refactor/tweaks/ExpandAutoType.cpp`, MSVC 
returns the following error:

llvm-monorepo\llvm\tools\clang\tools\extra\clangd\refactor\tweaks\ExpandAutoType.cpp(85):
 error C2146: syntax error: missing ')' before identifier 'and'
llvm-monorepo\llvm\tools\clang\tools\extra\clangd\refactor\tweaks\ExpandAutoType.cpp(85):
 error C2065: 'and': undeclared identifier
llvm-monorepo\llvm\tools\clang\tools\extra\clangd\refactor\tweaks\ExpandAutoType.cpp(86):
 error C2143: syntax error: missing ';' before ''
llvm-monorepo\llvm\tools\clang\tools\extra\clangd\refactor\tweaks\ExpandAutoType.cpp(73):
 fatal error C1075: '{': no matching token found

So, && must be used instead of `and`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69427

Files:
  clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp


Index: clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
@@ -82,7 +82,7 @@
   }
 
   // if it's a lambda expression, return an error message
-  if (isa(*DeducedType) and
+  if (isa(*DeducedType) &&
   dyn_cast(*DeducedType)->getDecl()->isLambda()) {
 return createErrorMessage("Could not expand type of lambda expression",
   Inputs);


Index: clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
@@ -82,7 +82,7 @@
   }
 
   // if it's a lambda expression, return an error message
-  if (isa(*DeducedType) and
+  if (isa(*DeducedType) &&
   dyn_cast(*DeducedType)->getDecl()->isLambda()) {
 return createErrorMessage("Could not expand type of lambda expression",
   Inputs);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69426: [clang] Switch arm-mve-intrinsics tests to use %clang_cc1.

[Dave Green via Phabricator via cfe-commits]

dmgreen added a comment.

Ah, I see, that's why it was needed. That makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69426



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

[PATCH] D69426: [clang] Switch arm-mve-intrinsics tests to use %clang_cc1.

[Simon Tatham via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG11ce19d2119e: [clang] Switch arm-mve-intrinsics tests to use 
%clang_cc1. (authored by simon_tatham).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69426

Files:
  clang/test/CodeGen/arm-mve-intrinsics/scalar-shifts.c
  clang/test/CodeGen/arm-mve-intrinsics/vadc.c
  clang/test/CodeGen/arm-mve-intrinsics/vaddq.c
  clang/test/CodeGen/arm-mve-intrinsics/vcvt.c
  clang/test/CodeGen/arm-mve-intrinsics/vld24.c
  clang/test/CodeGen/arm-mve-intrinsics/vldr.c
  clang/test/CodeGen/arm-mve-intrinsics/vminvq.c


Index: clang/test/CodeGen/arm-mve-intrinsics/vminvq.c
===
--- clang/test/CodeGen/arm-mve-intrinsics/vminvq.c
+++ clang/test/CodeGen/arm-mve-intrinsics/vminvq.c
@@ -1,6 +1,6 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
-// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main+mve.fp 
-mfloat-abi=hard -O0 -Xclang -disable-O0-optnone -fno-discard-value-names -S 
-emit-llvm -o - %s | opt -S -mem2reg | FileCheck %s
-// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main+mve.fp 
-mfloat-abi=hard -DPOLYMORPHIC -O0 -Xclang -disable-O0-optnone 
-fno-discard-value-names -S -emit-llvm -o - %s | opt -S -mem2reg | FileCheck %s
+// RUN: %clang_cc1 -triple thumbv8.1m.main-arm-none-eabi -target-feature 
+mve.fp -mfloat-abi hard -O0 -disable-O0-optnone -S -emit-llvm -o - %s | opt -S 
-mem2reg | FileCheck %s
+// RUN: %clang_cc1 -triple thumbv8.1m.main-arm-none-eabi -target-feature 
+mve.fp -mfloat-abi hard -O0 -disable-O0-optnone -DPOLYMORPHIC -S -emit-llvm -o 
- %s | opt -S -mem2reg | FileCheck %s
 
 #include 
 
Index: clang/test/CodeGen/arm-mve-intrinsics/vldr.c
===
--- clang/test/CodeGen/arm-mve-intrinsics/vldr.c
+++ clang/test/CodeGen/arm-mve-intrinsics/vldr.c
@@ -1,5 +1,5 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
-// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main+mve.fp 
-mfloat-abi=hard -O0 -Xclang -disable-O0-optnone -fno-discard-value-names -S 
-emit-llvm -o - %s | opt -S -mem2reg | FileCheck %s
+// RUN: %clang_cc1 -triple thumbv8.1m.main-arm-none-eabi -target-feature 
+mve.fp -mfloat-abi hard -O0 -disable-O0-optnone -S -emit-llvm -o - %s | opt -S 
-mem2reg | FileCheck %s
 
 #include 
 
Index: clang/test/CodeGen/arm-mve-intrinsics/vld24.c
===
--- clang/test/CodeGen/arm-mve-intrinsics/vld24.c
+++ clang/test/CodeGen/arm-mve-intrinsics/vld24.c
@@ -1,6 +1,6 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
-// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main+mve.fp 
-mfloat-abi=hard -O0 -Xclang -disable-O0-optnone -fno-discard-value-names -S 
-emit-llvm -o - %s | opt -S -mem2reg -sroa -early-cse | FileCheck %s
-// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main+mve.fp 
-mfloat-abi=hard -DPOLYMORPHIC -O0 -Xclang -disable-O0-optnone 
-fno-discard-value-names -S -emit-llvm -o - %s | opt -S -mem2reg -sroa 
-early-cse | FileCheck %s
+// RUN: %clang_cc1 -triple thumbv8.1m.main-arm-none-eabi -target-feature 
+mve.fp -mfloat-abi hard -O0 -disable-O0-optnone -S -emit-llvm -o - %s | opt -S 
-mem2reg -sroa -early-cse | FileCheck %s
+// RUN: %clang_cc1 -triple thumbv8.1m.main-arm-none-eabi -target-feature 
+mve.fp -mfloat-abi hard -O0 -disable-O0-optnone -DPOLYMORPHIC -S -emit-llvm -o 
- %s | opt -S -mem2reg -sroa -early-cse | FileCheck %s
 
 #include 
 
Index: clang/test/CodeGen/arm-mve-intrinsics/vcvt.c
===
--- clang/test/CodeGen/arm-mve-intrinsics/vcvt.c
+++ clang/test/CodeGen/arm-mve-intrinsics/vcvt.c
@@ -1,5 +1,5 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
-// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main+mve.fp 
-mfloat-abi=hard -O0 -Xclang -disable-O0-optnone -fno-discard-value-names -S 
-emit-llvm -o - %s | opt -S -mem2reg | FileCheck %s
+// RUN: %clang_cc1 -triple thumbv8.1m.main-arm-none-eabi -target-feature 
+mve.fp -mfloat-abi hard -O0 -disable-O0-optnone -S -emit-llvm -o - %s | opt -S 
-mem2reg | FileCheck %s
 
 #include 
 
Index: clang/test/CodeGen/arm-mve-intrinsics/vaddq.c
===
--- clang/test/CodeGen/arm-mve-intrinsics/vaddq.c
+++ clang/test/CodeGen/arm-mve-intrinsics/vaddq.c
@@ -1,6 +1,6 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
-// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main+mve.fp 
-mfloat-abi=hard -O0 -Xclang -disable-O0-optnone -fno-discard-value-names -S 
-emit-llvm -o - %s | opt -S -mem2reg | FileCheck %s
-// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m

[clang] 11ce19d - [clang] Switch arm-mve-intrinsics tests to use %clang_cc1.

[Simon Tatham via cfe-commits]

Author: Simon Tatham
Date: 2019-10-25T12:00:38+01:00
New Revision: 11ce19d2119e0870b2bf53eb23d215aa83cd5540

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

LOG: [clang] Switch arm-mve-intrinsics tests to use %clang_cc1.

It isn't really necessary for them to run the clang driver, and it's
more efficient not to (and also more stable against driver changes).
Now they invoke cc1 directly, more like the analogous NEON tests.

Reviewers: dmgreen

Subscribers: kristof.beyls, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/test/CodeGen/arm-mve-intrinsics/scalar-shifts.c
clang/test/CodeGen/arm-mve-intrinsics/vadc.c
clang/test/CodeGen/arm-mve-intrinsics/vaddq.c
clang/test/CodeGen/arm-mve-intrinsics/vcvt.c
clang/test/CodeGen/arm-mve-intrinsics/vld24.c
clang/test/CodeGen/arm-mve-intrinsics/vldr.c
clang/test/CodeGen/arm-mve-intrinsics/vminvq.c

Removed: 




diff  --git a/clang/test/CodeGen/arm-mve-intrinsics/scalar-shifts.c 
b/clang/test/CodeGen/arm-mve-intrinsics/scalar-shifts.c
index ec9a47f18eb9..0eead7a973f0 100644
--- a/clang/test/CodeGen/arm-mve-intrinsics/scalar-shifts.c
+++ b/clang/test/CodeGen/arm-mve-intrinsics/scalar-shifts.c
@@ -1,5 +1,5 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
-// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main+mve.fp 
-mfloat-abi=hard -O0 -Xclang -disable-O0-optnone -fno-discard-value-names -S 
-emit-llvm -o - %s | opt -S -mem2reg | FileCheck %s
+// RUN: %clang_cc1 -triple thumbv8.1m.main-arm-none-eabi -target-feature 
+mve.fp -mfloat-abi hard -O0 -disable-O0-optnone -S -emit-llvm -o - %s | opt -S 
-mem2reg | FileCheck %s
 
 #include 
 

diff  --git a/clang/test/CodeGen/arm-mve-intrinsics/vadc.c 
b/clang/test/CodeGen/arm-mve-intrinsics/vadc.c
index 6b77eac9ca54..58a47fc42bcb 100644
--- a/clang/test/CodeGen/arm-mve-intrinsics/vadc.c
+++ b/clang/test/CodeGen/arm-mve-intrinsics/vadc.c
@@ -1,6 +1,6 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
-// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main+mve.fp 
-mfloat-abi=hard -O0 -Xclang -disable-O0-optnone -fno-discard-value-names -S 
-emit-llvm -o - %s | opt -S -mem2reg | FileCheck %s
-// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main+mve.fp 
-mfloat-abi=hard -DPOLYMORPHIC -O0 -Xclang -disable-O0-optnone 
-fno-discard-value-names -S -emit-llvm -o - %s | opt -S -mem2reg | FileCheck %s
+// RUN: %clang_cc1 -triple thumbv8.1m.main-arm-none-eabi -target-feature 
+mve.fp -mfloat-abi hard -O0 -disable-O0-optnone -S -emit-llvm -o - %s | opt -S 
-mem2reg | FileCheck %s
+// RUN: %clang_cc1 -triple thumbv8.1m.main-arm-none-eabi -target-feature 
+mve.fp -mfloat-abi hard -O0 -disable-O0-optnone -DPOLYMORPHIC -S -emit-llvm -o 
- %s | opt -S -mem2reg | FileCheck %s
 
 #include 
 

diff  --git a/clang/test/CodeGen/arm-mve-intrinsics/vaddq.c 
b/clang/test/CodeGen/arm-mve-intrinsics/vaddq.c
index 970ac53cefc6..1f18d5b57880 100644
--- a/clang/test/CodeGen/arm-mve-intrinsics/vaddq.c
+++ b/clang/test/CodeGen/arm-mve-intrinsics/vaddq.c
@@ -1,6 +1,6 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
-// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main+mve.fp 
-mfloat-abi=hard -O0 -Xclang -disable-O0-optnone -fno-discard-value-names -S 
-emit-llvm -o - %s | opt -S -mem2reg | FileCheck %s
-// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main+mve.fp 
-mfloat-abi=hard -DPOLYMORPHIC -O0 -Xclang -disable-O0-optnone 
-fno-discard-value-names -S -emit-llvm -o - %s | opt -S -mem2reg | FileCheck %s
+// RUN: %clang_cc1 -triple thumbv8.1m.main-arm-none-eabi -target-feature 
+mve.fp -mfloat-abi hard -O0 -disable-O0-optnone -S -emit-llvm -o - %s | opt -S 
-mem2reg | FileCheck %s
+// RUN: %clang_cc1 -triple thumbv8.1m.main-arm-none-eabi -target-feature 
+mve.fp -mfloat-abi hard -O0 -disable-O0-optnone -DPOLYMORPHIC -S -emit-llvm -o 
- %s | opt -S -mem2reg | FileCheck %s
 
 #include 
 

diff  --git a/clang/test/CodeGen/arm-mve-intrinsics/vcvt.c 
b/clang/test/CodeGen/arm-mve-intrinsics/vcvt.c
index 1aae36619dfa..ed3ecd3ee62e 100644
--- a/clang/test/CodeGen/arm-mve-intrinsics/vcvt.c
+++ b/clang/test/CodeGen/arm-mve-intrinsics/vcvt.c
@@ -1,5 +1,5 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
-// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main+mve.fp 
-mfloat-abi=hard -O0 -Xclang -disable-O0-optnone -fno-discard-value-names -S 
-emit-llvm -o - %s | opt -S -mem2reg | FileCheck %s
+// RUN: %clang_cc1 -triple thumbv8.1m.main-arm-none-eabi -target-feature 
+mve.fp -mfloat-abi hard -O0 -disable-O0-optnone -S -emit-llvm -o - %s | opt -S 
-mem2reg | FileCheck %s
 
 #inc

[PATCH] D69297: [ARM][AArch64] Implement __arm_rsrf, __arm_rsrf64, __arm_wsrf & __arm_wsrf64

[Victor Campos via Phabricator via cfe-commits]

vhscampos updated this revision to Diff 226406.
vhscampos added a comment.

Use __builtin_bit_cast to perform the relevant bitcasts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69297

Files:
  clang/lib/Headers/arm_acle.h
  clang/test/CodeGen/arm_acle.c


Index: clang/test/CodeGen/arm_acle.c
===
--- clang/test/CodeGen/arm_acle.c
+++ clang/test/CodeGen/arm_acle.c
@@ -822,6 +822,55 @@
   __arm_wsrp("sysreg", v);
 }
 
+// ARM-LABEL: test_rsrf
+// AArch64: call i64 @llvm.read_register.i64(metadata ![[M0:[0-9]]])
+// AArch32: call i32 @llvm.read_register.i32(metadata ![[M2:[0-9]]])
+// ARM-NOT: uitofp
+// ARM: bitcast
+float test_rsrf() {
+#ifdef __ARM_32BIT_STATE
+  return __arm_rsrf("cp1:2:c3:c4:5");
+#else
+  return __arm_rsrf("1:2:3:4:5");
+#endif
+}
+// ARM-LABEL: test_rsrf64
+// AArch64: call i64 @llvm.read_register.i64(metadata ![[M0:[0-9]]])
+// AArch32: call i64 @llvm.read_register.i64(metadata ![[M3:[0-9]]])
+// ARM-NOT: uitofp
+// ARM: bitcast
+double test_rsrf64() {
+#ifdef __ARM_32BIT_STATE
+  return __arm_rsrf64("cp1:2:c3");
+#else
+  return __arm_rsrf64("1:2:3:4:5");
+#endif
+}
+// ARM-LABEL: test_wsrf
+// ARM-NOT: fptoui
+// ARM: bitcast
+// AArch64: call void @llvm.write_register.i64(metadata ![[M0:[0-9]]], i64 
%{{.*}})
+// AArch32: call void @llvm.write_register.i32(metadata ![[M2:[0-9]]], i32 
%{{.*}})
+void test_wsrf(float v) {
+#ifdef __ARM_32BIT_STATE
+  __arm_wsrf("cp1:2:c3:c4:5", v);
+#else
+  __arm_wsrf("1:2:3:4:5", v);
+#endif
+}
+// ARM-LABEL: test_wsrf64
+// ARM-NOT: fptoui
+// ARM: bitcast
+// AArch64: call void @llvm.write_register.i64(metadata ![[M0:[0-9]]], i64 
%{{.*}})
+// AArch32: call void @llvm.write_register.i64(metadata ![[M3:[0-9]]], i64 
%{{.*}})
+void test_wsrf64(double v) {
+#ifdef __ARM_32BIT_STATE
+  __arm_wsrf64("cp1:2:c3", v);
+#else
+  __arm_wsrf64("1:2:3:4:5", v);
+#endif
+}
+
 // AArch32: ![[M2]] = !{!"cp1:2:c3:c4:5"}
 // AArch32: ![[M3]] = !{!"cp1:2:c3"}
 // AArch32: ![[M4]] = !{!"sysreg"}
Index: clang/lib/Headers/arm_acle.h
===
--- clang/lib/Headers/arm_acle.h
+++ clang/lib/Headers/arm_acle.h
@@ -609,9 +609,13 @@
 #define __arm_rsr(sysreg) __builtin_arm_rsr(sysreg)
 #define __arm_rsr64(sysreg) __builtin_arm_rsr64(sysreg)
 #define __arm_rsrp(sysreg) __builtin_arm_rsrp(sysreg)
+#define __arm_rsrf(sysreg) __builtin_bit_cast(float, __arm_rsr(sysreg))
+#define __arm_rsrf64(sysreg) __builtin_bit_cast(double, __arm_rsr64(sysreg))
 #define __arm_wsr(sysreg, v) __builtin_arm_wsr(sysreg, v)
 #define __arm_wsr64(sysreg, v) __builtin_arm_wsr64(sysreg, v)
 #define __arm_wsrp(sysreg, v) __builtin_arm_wsrp(sysreg, v)
+#define __arm_wsrf(sysreg, v) __arm_wsr(sysreg, __builtin_bit_cast(uint32_t, 
v))
+#define __arm_wsrf64(sysreg, v) __arm_wsr64(sysreg, 
__builtin_bit_cast(uint64_t, v))
 
 /* Memory Tagging Extensions (MTE) Intrinsics */
 #if __ARM_FEATURE_MEMORY_TAGGING


Index: clang/test/CodeGen/arm_acle.c
===
--- clang/test/CodeGen/arm_acle.c
+++ clang/test/CodeGen/arm_acle.c
@@ -822,6 +822,55 @@
   __arm_wsrp("sysreg", v);
 }
 
+// ARM-LABEL: test_rsrf
+// AArch64: call i64 @llvm.read_register.i64(metadata ![[M0:[0-9]]])
+// AArch32: call i32 @llvm.read_register.i32(metadata ![[M2:[0-9]]])
+// ARM-NOT: uitofp
+// ARM: bitcast
+float test_rsrf() {
+#ifdef __ARM_32BIT_STATE
+  return __arm_rsrf("cp1:2:c3:c4:5");
+#else
+  return __arm_rsrf("1:2:3:4:5");
+#endif
+}
+// ARM-LABEL: test_rsrf64
+// AArch64: call i64 @llvm.read_register.i64(metadata ![[M0:[0-9]]])
+// AArch32: call i64 @llvm.read_register.i64(metadata ![[M3:[0-9]]])
+// ARM-NOT: uitofp
+// ARM: bitcast
+double test_rsrf64() {
+#ifdef __ARM_32BIT_STATE
+  return __arm_rsrf64("cp1:2:c3");
+#else
+  return __arm_rsrf64("1:2:3:4:5");
+#endif
+}
+// ARM-LABEL: test_wsrf
+// ARM-NOT: fptoui
+// ARM: bitcast
+// AArch64: call void @llvm.write_register.i64(metadata ![[M0:[0-9]]], i64 %{{.*}})
+// AArch32: call void @llvm.write_register.i32(metadata ![[M2:[0-9]]], i32 %{{.*}})
+void test_wsrf(float v) {
+#ifdef __ARM_32BIT_STATE
+  __arm_wsrf("cp1:2:c3:c4:5", v);
+#else
+  __arm_wsrf("1:2:3:4:5", v);
+#endif
+}
+// ARM-LABEL: test_wsrf64
+// ARM-NOT: fptoui
+// ARM: bitcast
+// AArch64: call void @llvm.write_register.i64(metadata ![[M0:[0-9]]], i64 %{{.*}})
+// AArch32: call void @llvm.write_register.i64(metadata ![[M3:[0-9]]], i64 %{{.*}})
+void test_wsrf64(double v) {
+#ifdef __ARM_32BIT_STATE
+  __arm_wsrf64("cp1:2:c3", v);
+#else
+  __arm_wsrf64("1:2:3:4:5", v);
+#endif
+}
+
 // AArch32: ![[M2]] = !{!"cp1:2:c3:c4:5"}
 // AArch32: ![[M3]] = !{!"cp1:2:c3"}
 // AArch32: ![[M4]] = !{!"sysreg"}
Index: clang/lib/Headers/arm_acle.h
===
--- clang/lib/Headers/arm_acle.h
+++ clang/lib/Hea

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[MyDeveloperDay via Phabricator via cfe-commits]

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

I've read back through the previous comments, and I'm slightly struggling to 
understand the reason for the objections. (other than the normal public style)

I'd tend to be wary of the "if its not in a public style guide, its not coming 
in" rule despite me understanding the rationale for that stance many people 
have non-public projects but still have millions of lines of code to support:

Many public projects have often already adopted a .clang-format file anyway and 
as such their style is defined by only what clang-format currently supports and 
hence it cannot by definition then define a style that clang-format keeps 
altering the code away from.

So I baulk at the idea that a rule that introduces a new style can't come in, 
My rule of thumb is don't mess up anyone's existing code, don't make others pay 
too much for your feature, help us look after this excellent tool by helping 
the project (bugs, reviews).

With that in mind, I've gone back to the original documentation for 
AlignOperands and BreakBeforeBinaryOperators, and I'm afraid I cannot see the 
logic in the current capabilities that align code nicely with 
BreakBeforeBinaryOperators turned off

F10398828: image.png 

but not nicely with BreakBeforeBinaryOperators turned on

F10398823: image.png 

If I understand correctly, this new setting (which will be off by default) will 
allow the following for those that choose it.

F10398841: image.png 

and it may be the OCD in me, but that re-alignment makes me feel happy, for 
this reason alone I'd give this a LGTM, as long as your willing to help us 
support this behaviour going forward.

That's my rationale for accepting, plus the authors 2.5 years of rebasing a 
feature, shows a dedication which I am assuming extends to continued support.

my 2c worth.

(images from https://zed0.co.uk/clang-format-configurator/) - 10.0.0+b452de0




Comment at: clang/include/clang/Format/Format.h:187
   /// expressions.
-  ///
-  /// Specifically, this aligns operands of a single expression that needs to 
be
-  /// split over multiple lines, e.g.:
-  /// \code
-  ///   int aaa = bbb +
-  /// ccc;
-  /// \endcode
-  bool AlignOperands;
+  OperandAlignmentStyle AlignOperands;
 

Typz wrote:
> MyDeveloperDay wrote:
> > I think you are missing an entry in the operator== in Format.h
> It is there already, since this is not a new field, but just changing the 
> type of an existing field.
sound good.



Comment at: clang/unittests/Format/FormatTest.cpp:4147
"  >> (aa);",
Style);
 

Nit: I get nervous when we change tests, can we simply add a new duplicated line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D32478



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

[PATCH] D69427: Fix compilation error in clangd/refactor/tweaks/ExpandAutoType.cpp

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

LGTM, thanks !


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69427



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

[PATCH] D63960: [C++20] Add consteval-specific semantic for functions

[Tyker via Phabricator via cfe-commits]

Tyker added a comment.

ping @rsmith


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

https://reviews.llvm.org/D63960



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

[PATCH] D69427: Fix compilation error in clangd/refactor/tweaks/ExpandAutoType.cpp

[Pavel Samolysov via Phabricator via cfe-commits]

psamolysov added a comment.

I have no commit access, can anyone land this small fix, please?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69427



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

[PATCH] D69263: [clangd] Implement cross-file rename.

[Haojian Wu via Phabricator via cfe-commits]

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

- simplify the code, addressing comments;
- add unittests;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69263

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/refactor/Rename.h
  clang-tools-extra/clangd/refactor/Tweak.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp
  clang-tools-extra/clangd/unittests/SyncAPI.cpp
  clang-tools-extra/clangd/unittests/SyncAPI.h

Index: clang-tools-extra/clangd/unittests/SyncAPI.h
===
--- clang-tools-extra/clangd/unittests/SyncAPI.h
+++ clang-tools-extra/clangd/unittests/SyncAPI.h
@@ -38,8 +38,8 @@
 llvm::Expected>
 runFindDocumentHighlights(ClangdServer &Server, PathRef File, Position Pos);
 
-llvm::Expected>
-runRename(ClangdServer &Server, PathRef File, Position Pos, StringRef NewName);
+llvm::Expected runRename(ClangdServer &Server, PathRef File,
+Position Pos, StringRef NewName);
 
 std::string runDumpAST(ClangdServer &Server, PathRef File);
 
Index: clang-tools-extra/clangd/unittests/SyncAPI.cpp
===
--- clang-tools-extra/clangd/unittests/SyncAPI.cpp
+++ clang-tools-extra/clangd/unittests/SyncAPI.cpp
@@ -96,11 +96,11 @@
   return std::move(*Result);
 }
 
-llvm::Expected> runRename(ClangdServer &Server,
-PathRef File, Position Pos,
-llvm::StringRef NewName) {
-  llvm::Optional>> Result;
-  Server.rename(File, Pos, NewName, /*WantFormat=*/true, capture(Result));
+llvm::Expected runRename(ClangdServer &Server, PathRef File,
+Position Pos, llvm::StringRef NewName) {
+  llvm::Optional> Result;
+  Server.rename(File, Pos, NewName, /*WantFormat=*/true,
+/*DirtyBuffaer*/ nullptr, capture(Result));
   return std::move(*Result);
 }
 
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -9,8 +9,10 @@
 #include "Annotations.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "index/Ref.h"
 #include "refactor/Rename.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "llvm/Support/MemoryBuffer.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -18,9 +20,61 @@
 namespace clangd {
 namespace {
 
+using testing::Eq;
+using testing::Matches;
+MATCHER_P2(PairMatcher, KeyMatcher, ValueMatcher, "") {
+  return Matches(KeyMatcher)(arg.first()) && Matches(ValueMatcher)(arg.second);
+}
 MATCHER_P2(RenameRange, Code, Range, "") {
   return replacementToEdit(Code, arg).range == Range;
 }
+MATCHER_P2(EqualFileEdit, Code, Ranges, "") {
+  using ReplacementMatcher = testing::Matcher;
+  std::vector Expected;
+  for (const auto &R : Ranges)
+Expected.push_back(RenameRange(Code, R));
+  auto Matcher =
+  testing::internal::UnorderedElementsAreArrayMatcher(
+  Expected.begin(), Expected.end());
+  return arg.InitialCode == Code && Matches(Matcher)(arg.Replacements);
+}
+
+std::unique_ptr buildRefSlab(const Annotations &Code,
+  llvm::StringRef SymbolName,
+  llvm::StringRef Path) {
+  RefSlab::Builder Builder;
+  TestTU TU;
+  TU.HeaderCode = Code.code();
+  auto Symbols = TU.headerSymbols();
+  const auto &SymbolID = findSymbol(Symbols, SymbolName).ID;
+  for (const auto &Range : Code.ranges()) {
+Ref R;
+R.Kind = RefKind::Reference;
+R.Location.Start.setLine(Range.start.line);
+R.Location.Start.setColumn(Range.start.character);
+R.Location.End.setLine(Range.end.line);
+R.Location.End.setColumn(Range.end.character);
+auto U = URI::create(Path).toString();
+R.Location.FileURI = U.c_str();
+Builder.insert(SymbolID, R);
+  }
+
+  return std::make_unique(std::move(Builder).build());
+}
+
+RenameInputs renameInputs(const Annotations &Code, llvm::StringRef NewName,
+  llvm::StringRef Path,
+  const SymbolIndex *Index = nullptr,
+  bool CrossFile = false) {
+  RenameInputs Inputs;
+  Inputs.Pos = Code.point();
+  Inputs.MainFileCode = Code.code();
+  Inputs.MainFilePath = Path;
+  Inputs.NewName = NewName;
+  Inputs.Index = Index;
+  Inputs.AllowCrossFile = CrossFile;
+  return Inputs;
+}
 
 TEST(RenameTes

[PATCH] D69263: [clangd] Implement cross-file rename.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D69263#1718525 , @ilya-biryukov 
wrote:

> Not sure that holds. What if the file in question is being rebuilt right now? 
> We do not wait until all ASTs are built (and the dynamic index gets the new 
> results).
>  Open files actually pose an interesting problem: their contents do not 
> correspond to the file contents on disk.
>  We should choose which of those we update on rename: dirty buffers or file 
> contents? (I believe we should do dirty buffers, but I believe @sammccall has 
> the opposite opinion, so we should sync on this)


Based on the offline discussion, we decide to use dirty buffers for opened 
files.




Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:121
+renameableOutsideFile(const NamedDecl &RenameDecl, const SymbolIndex *Index) {
+  if (RenameDecl.getParentFunctionOrMethod())
+return None; // function-local symbols

ilya-biryukov wrote:
> hokein wrote:
> > ilya-biryukov wrote:
> > > This function resembles `renamableWithinFile`.
> > > We should probably share implementation and pass an extra flag. Something 
> > > like `isRenamable(..., bool IsCrossFile)`.
> > > 
> > > WDYT?
> > though they look similar,  the logic is quite different, for example, we 
> > query the index in within-file rename to detect a symbol is used outside of 
> > the file which is not needed for cross-file rename, and cross-file rename 
> > allows fewer symbols (as our index doesn't support them), so I don't think 
> > we can share a lot of implementation.
> Can this be handled with a special flag:
> ```
> bool renameable(NamedDecl &Decl, const SymbolIndex *Index, bool CrossFile) {
>   if (CrossFileRename) {
> // check something special...
>   }
> }
> ```
> 
> Sharing implementation in the same function makes the differences between 
> cross-file and single-file case obvious and also ensures any things that need 
> to be updated for both are shared.
Done. I tried my best to unify them, please take a look on the new code.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:317
+  std::string OldName = RenameDecl->getNameAsString();
+  for (const auto &FileAndOccurrences : AffectedFiles) {
+llvm::StringRef FilePath = FileAndOccurrences.first();

ilya-biryukov wrote:
> hokein wrote:
> > ilya-biryukov wrote:
> > > I feel this code is fundamental to the trade-offs we made for rename.
> > > Could we move this to a separate function and add unit-tests for it?
> > > 
> > > You probably want to have something that handles just a single file 
> > > rather than all edits in all files, to avoid mocking VFS in tests, etc.
> > > 
> > > 
> > Agree, especially when we start implementing complicated stuff, e.g. range 
> > patching heuristics.
> > 
> >  Moved it out, but note that I don't plan to add more stuff in this initial 
> > patch, so the logic is pretty straightforward, just assembling rename 
> > replacement from occurrences.
> > 
> > but note that I don't plan to add more stuff in this initial patch
> Should we also check whether the replaced text matches the expected 
> identifier and add unit-tests for this?
> Or would you prefer to move all changes to this function to a separate patch?
I prefer to do it afterwards as the patch is relatively big now.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:380
+auto MainFileRenameEdit =
+renameWithinFile(AST, RenameDecl, RInputs.NewName);
+if (!MainFileRenameEdit)

ilya-biryukov wrote:
> hokein wrote:
> > ilya-biryukov wrote:
> > > Can we rely on the fact that dynamic index should not be stale for the 
> > > current file instead?
> > > Or don't we have this invariant?
> > > 
> > > We end up having two implementations now: index-based and AST-based. It'd 
> > > be nice to have just one.
> > > If that's not possible in the first revision, could you explain why?
> > > 
> > > Note that moving the current-file rename to index-based implementation is 
> > > independent of doing cross-file renames. Maybe we should start with it?
> > I think we can't get rid of the AST-based rename -- mainly for renaming 
> > local symbols (e.g. local variable in function body), as we don't index 
> > these symbols...
> Thanks, I believe you're right... We can't unify these implementations, at 
> least not in the short term.
> 
> So `renameOutsideFile` fails on local symbols and `renameWithinFile` should 
> handle that, right?
> Also worth noting that `renameWithinFile` is AST-based and 
> `renameOutsideFile` is index-based.
> Could we document that?
yes, exactly. I have simplified the code in this function, and added 
documentations.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:357
+// Handle within-file rename.
+auto Reject = renamableWithinFile(*RenameDecl->getCanonicalDecl(),
+  RInputs.MainFilePath, RInpu

[clang-tools-extra] ce1e249 - Fix compilation error in clangd/refactor/tweaks/ExpandAutoType.cpp

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2019-10-25T14:16:59+02:00
New Revision: ce1e249a688dced25735e38242df561e387b8e2b

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

LOG: Fix compilation error in clangd/refactor/tweaks/ExpandAutoType.cpp

Summary:
During the compilation of the `clangd/refactor/tweaks/ExpandAutoType.cpp`, MSVC 
returns the following error:

llvm-monorepo\llvm\tools\clang\tools\extra\clangd\refactor\tweaks\ExpandAutoType.cpp(85):
 error C2146: syntax error: missing ')' before identifier 'and'
llvm-monorepo\llvm\tools\clang\tools\extra\clangd\refactor\tweaks\ExpandAutoType.cpp(85):
 error C2065: 'and': undeclared identifier
llvm-monorepo\llvm\tools\clang\tools\extra\clangd\refactor\tweaks\ExpandAutoType.cpp(86):
 error C2143: syntax error: missing ';' before ''
llvm-monorepo\llvm\tools\clang\tools\extra\clangd\refactor\tweaks\ExpandAutoType.cpp(73):
 fatal error C1075: '{': no matching token found

So, && must be used instead of `and`.

Patch By Pavel Samolysov (@psamolysov) !

Reviewers: kadircet

Reviewed By: kadircet

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

Tags: #clang, #clang-tools-extra

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

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp 
b/clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
index 76c14ae723d7..02c9cdb8280f 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
@@ -82,7 +82,7 @@ Expected ExpandAutoType::apply(const 
Selection& Inputs) {
   }
 
   // if it's a lambda expression, return an error message
-  if (isa(*DeducedType) and
+  if (isa(*DeducedType) &&
   dyn_cast(*DeducedType)->getDecl()->isLambda()) {
 return createErrorMessage("Could not expand type of lambda expression",
   Inputs);



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

[PATCH] D69427: Fix compilation error in clangd/refactor/tweaks/ExpandAutoType.cpp

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGce1e249a688d: Fix compilation error in 
clangd/refactor/tweaks/ExpandAutoType.cpp (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69427

Files:
  clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp


Index: clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
@@ -82,7 +82,7 @@
   }
 
   // if it's a lambda expression, return an error message
-  if (isa(*DeducedType) and
+  if (isa(*DeducedType) &&
   dyn_cast(*DeducedType)->getDecl()->isLambda()) {
 return createErrorMessage("Could not expand type of lambda expression",
   Inputs);


Index: clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
@@ -82,7 +82,7 @@
   }
 
   // if it's a lambda expression, return an error message
-  if (isa(*DeducedType) and
+  if (isa(*DeducedType) &&
   dyn_cast(*DeducedType)->getDecl()->isLambda()) {
 return createErrorMessage("Could not expand type of lambda expression",
   Inputs);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69263: [clangd] Implement cross-file rename.

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

merge_guards_bot added a comment.

Build result: fail - 59520 tests passed, 1 failed and 763 were skipped.

  failed: Clangd.Clangd/rename.test

Log files: cmake-log.txt 
, 
ninja_check_all-log.txt 
, 
CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69263



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

[clang] 3d9632a - [clang-rename] NFC, make getCanonicalSymbolDeclaration robust on nullptr input.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2019-10-25T14:33:04+02:00
New Revision: 3d9632a997fb3d59f0740bb00817b5c6115674cb

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

LOG: [clang-rename] NFC, make getCanonicalSymbolDeclaration robust on nullptr 
input.

Added: 


Modified: 
clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp

Removed: 




diff  --git a/clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp 
b/clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
index e26248f50c29..966833137c26 100644
--- a/clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
+++ b/clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
@@ -39,6 +39,8 @@ namespace clang {
 namespace tooling {
 
 const NamedDecl *getCanonicalSymbolDeclaration(const NamedDecl *FoundDecl) {
+  if (!FoundDecl)
+return nullptr;
   // If FoundDecl is a constructor or destructor, we want to instead take
   // the Decl of the corresponding class.
   if (const auto *CtorDecl = dyn_cast(FoundDecl))



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

[PATCH] D69431: [clangd] Do not highlight keywords in semantic highlighting

[Ilya Biryukov via Phabricator via cfe-commits]

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

Editors are good at highlightings the keywords themselves.
Note that this only affects highlightings of builtin types spelled out
as keywords in the source code. Highlightings of typedefs to builtin
types are unchanged.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69431

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

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -432,20 +432,16 @@
 TEST_F(AnnotateHighlightingsTest, Test) {
   EXPECT_AVAILABLE("^vo^id^ ^f(^) {^}^"); // available everywhere.
   EXPECT_AVAILABLE("[[int a; int b;]]");
-  EXPECT_EQ("/* storage.type.primitive.cpp */void "
-"/* entity.name.function.cpp */f() {}",
+  EXPECT_EQ("void /* entity.name.function.cpp */f() {}",
 apply("void ^f() {}"));
 
   EXPECT_EQ(apply("[[void f1(); void f2();]]"),
-"/* storage.type.primitive.cpp */void "
-"/* entity.name.function.cpp */f1(); "
-"/* storage.type.primitive.cpp */void "
-"/* entity.name.function.cpp */f2();");
+"void /* entity.name.function.cpp */f1(); "
+"void /* entity.name.function.cpp */f2();");
 
   EXPECT_EQ(apply("void f1(); void f2() {^}"),
 "void f1(); "
-"/* storage.type.primitive.cpp */void "
-"/* entity.name.function.cpp */f2() {}");
+"void /* entity.name.function.cpp */f2() {}");
 }
 
 TWEAK_TEST(ExpandMacro);
Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -153,26 +153,26 @@
   const char *TestCases[] = {
   R"cpp(
   struct $Class[[AS]] {
-$Primitive[[double]] $Field[[SomeMember]];
+double $Field[[SomeMember]];
   };
   struct {
   } $Variable[[S]];
-  $Primitive[[void]] $Function[[foo]]($Primitive[[int]] $Parameter[[A]], $Class[[AS]] $Parameter[[As]]) {
+  void $Function[[foo]](int $Parameter[[A]], $Class[[AS]] $Parameter[[As]]) {
 $Primitive[[auto]] $LocalVariable[[VeryLongVariableName]] = 12312;
 $Class[[AS]] $LocalVariable[[AA]];
 $Primitive[[auto]] $LocalVariable[[L]] = $LocalVariable[[AA]].$Field[[SomeMember]] + $Parameter[[A]];
-auto $LocalVariable[[FN]] = [ $LocalVariable[[AA]]]($Primitive[[int]] $Parameter[[A]]) -> $Primitive[[void]] {};
+auto $LocalVariable[[FN]] = [ $LocalVariable[[AA]]](int $Parameter[[A]]) -> void {};
 $LocalVariable[[FN]](12312);
   }
 )cpp",
   R"cpp(
-  $Primitive[[void]] $Function[[foo]]($Primitive[[int]]);
-  $Primitive[[void]] $Function[[Gah]]();
-  $Primitive[[void]] $Function[[foo]]() {
+  void $Function[[foo]](int);
+  void $Function[[Gah]]();
+  void $Function[[foo]]() {
 auto $LocalVariable[[Bou]] = $Function[[Gah]];
   }
   struct $Class[[A]] {
-$Primitive[[void]] $Method[[abc]]();
+void $Method[[abc]]();
   };
 )cpp",
   R"cpp(
@@ -186,17 +186,17 @@
   struct $Class[[C]] : $Namespace[[abc]]::$Class[[A]]<$TemplateParameter[[T]]> {
 typename $TemplateParameter[[T]]::$DependentType[[A]]* $Field[[D]];
   };
-  $Namespace[[abc]]::$Class[[A]]<$Primitive[[int]]> $Variable[[AA]];
-  typedef $Namespace[[abc]]::$Class[[A]]<$Primitive[[int]]> $Class[[AAA]];
+  $Namespace[[abc]]::$Class[[A]] $Variable[[AA]];
+  typedef $Namespace[[abc]]::$Class[[A]] $Class[[AAA]];
   struct $Class[[B]] {
 $Class[[B]]();
 ~$Class[[B]]();
-$Primitive[[void]] operator<<($Class[[B]]);
+void operator<<($Class[[B]]);
 $Class[[AAA]] $Field[[AA]];
   };
   $Class[[B]]::$Class[[B]]() {}
   $Class[[B]]::~$Class[[B]]() {}
-  $Primitive[[void]] $Function[[f]] () {
+  void $Function[[f]] () {
 $Class[[B]] $LocalVariable[[BB]] = $Class[[B]]();
 $LocalVariable[[BB]].~$Class[[B]]();
 $Class[[B]]();
@@ -214,7 +214,7 @@
 $Enum[[E]] $Field[[EEE]];
 $Enum[[EE]] $Field[[]];
   };
-  $Primitive[[int]] $Variable[[I]] = $EnumConstant[[Hi]];
+  int $Variable[[I]] = $EnumConstant[[Hi]];
   $Enum[[E]] $Variable[[L]] = $Enum[[E]]::$EnumConstant[[B]];
 )cpp",
   R"cpp(
@@ -242,14 +242,14 @@
 )cpp",
   R"cpp(
   struct $Class[[D]] {
-$Primitive[[double]] $Field[[

[clang-tools-extra] 43e931c - [clangd][NFC] Get rid of raw string literals in macros to make stage1 compiler happy

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2019-10-25T15:01:28+02:00
New Revision: 43e931cb5fc1830f6b9250f35d29e1377a66eee6

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

LOG: [clangd][NFC] Get rid of raw string literals in macros to make stage1 
compiler happy

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp 
b/clang-tools-extra/clangd/unittests/TweakTests.cpp
index e1e7f5b63bc8..2a6744b81d94 100644
--- a/clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -36,6 +36,7 @@
 #include "gtest/gtest.h"
 #include 
 #include 
+#include 
 #include 
 
 using ::testing::AllOf;
@@ -1054,455 +1055,475 @@ TEST_F(DefineInlineTest, UsingShadowDecls) {
 }
 
 TEST_F(DefineInlineTest, TransformNestedNamespaces) {
-  EXPECT_EQ(apply(R"cpp(
-  namespace a {
-void bar();
-namespace b {
-  void baz();
-  namespace c {
-void aux();
+  auto Test = R"cpp(
+namespace a {
+  void bar();
+  namespace b {
+void baz();
+namespace c {
+  void aux();
+}
   }
 }
-  }
 
-  void foo();
-  using namespace a;
-  using namespace b;
-  using namespace c;
-  void f^oo() {
-bar();
-a::bar();
-
-baz();
-b::baz();
-a::b::baz();
-
-aux();
-c::aux();
-b::c::aux();
-a::b::c::aux();
-  })cpp"), R"cpp(
-  namespace a {
-void bar();
-namespace b {
-  void baz();
-  namespace c {
-void aux();
+void foo();
+using namespace a;
+using namespace b;
+using namespace c;
+void f^oo() {
+  bar();
+  a::bar();
+
+  baz();
+  b::baz();
+  a::b::baz();
+
+  aux();
+  c::aux();
+  b::c::aux();
+  a::b::c::aux();
+})cpp";
+  auto Expected = R"cpp(
+namespace a {
+  void bar();
+  namespace b {
+void baz();
+namespace c {
+  void aux();
+}
   }
 }
-  }
 
-  void foo(){
-a::bar();
-a::bar();
+void foo(){
+  a::bar();
+  a::bar();
 
-a::b::baz();
-a::b::baz();
-a::b::baz();
+  a::b::baz();
+  a::b::baz();
+  a::b::baz();
 
-a::b::c::aux();
-a::b::c::aux();
-a::b::c::aux();
-a::b::c::aux();
-  }
-  using namespace a;
-  using namespace b;
-  using namespace c;
-  )cpp");
+  a::b::c::aux();
+  a::b::c::aux();
+  a::b::c::aux();
+  a::b::c::aux();
+}
+using namespace a;
+using namespace b;
+using namespace c;
+)cpp";
+  EXPECT_EQ(apply(Test), Expected);
 }
 
 TEST_F(DefineInlineTest, TransformUsings) {
-  EXPECT_EQ(apply(R"cpp(
-  namespace a { namespace b { namespace c { void aux(); } } }
+  auto Test = R"cpp(
+namespace a { namespace b { namespace c { void aux(); } } }
 
-  void foo();
-  void f^oo() {
-using namespace a;
-using namespace b;
-using namespace c;
-using c::aux;
-namespace d = c;
-  })cpp"),
-R"cpp(
-  namespace a { namespace b { namespace c { void aux(); } } }
+void foo();
+void f^oo() {
+  using namespace a;
+  using namespace b;
+  using namespace c;
+  using c::aux;
+  namespace d = c;
+})cpp";
+  auto Expected = R"cpp(
+namespace a { namespace b { namespace c { void aux(); } } }
 
-  void foo(){
-using namespace a;
-using namespace a::b;
-using namespace a::b::c;
-using a::b::c::aux;
-namespace d = a::b::c;
-  }
-  )cpp");
+void foo(){
+  using namespace a;
+  using namespace a::b;
+  using namespace a::b::c;
+  using a::b::c::aux;
+  namespace d = a::b::c;
+}
+)cpp";
+  EXPECT_EQ(apply(Test), Expected);
 }
 
 TEST_F(DefineInlineTest, TransformDecls) {
-  EXPECT_EQ(apply(R"cpp(
-  void foo();
-  void f^oo() {
-class Foo {
-public:
-  void foo();
-  int x;
-  static int y;
-};
-Foo::y = 0;
+  auto Test = R"cpp(
+void foo();
+void f^oo() {
+  class Foo {
+  public:
+void foo();
+int x;
+static int y;
+  };
+  Foo::y = 0;
 
-enum En { Zero, One };
-En x = Zero;
+  enum En { Zero, One };
+  En x = Zero;
 
-enum class EnClass { Zero, One };
-EnClass y = EnClass::Zero;
-  })cpp"),
-R"cpp(
-  void foo(){
-class Foo {
-public:
-  void foo();
-  int x;
-  static int y;
-};
-Foo::y = 0;
+  enum class EnClass { Zero, One };
+  EnClass y = EnClass::Zero;
+})cpp";
+  auto Expected = R"cpp(
+void foo(){
+  class Foo {
+  public:
+void foo();
+int x;
+static int y;
+  };
+  Foo::y = 0;
 
-enum En { Zero, One };
-En x = Zero;
+  en

Applying fixes in clang 8.0?

[Martijn Otto via cfe-commits]

Hi all,

I am currently running into issues with bug 41810. Now, this is fixed
in 9.0, but I need to also support older versions of clang (up to 7.0,
which luckily is unaffected).

Now, looking through the commit list, it seems that it's commit
08d0c133ccb1b530ed743a021dc5995fbcdaf012 which fixes the issue in 9.0.
I have locally tested and built the release_80 branch with that commit
cherry-picked on top and can confirm that this indeed fixes the bug.

Since 8.0 is still in wide use, would it be possible to release a
patched 8.0.2 version?


Relevant links:
https://bugs.llvm.org/show_bug.cgi?id=41810
https://github.com/llvm-mirror/clang/commit/08d0c133ccb1b530ed743a021dc5995fbcdaf012

With regards,
Martijn Otto


signature.asc
Description: This is a digitally signed message part
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69382: [clangd] Do not insert parentheses when completing a using declaration

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1466
   Output.Completions.back().Score = C.second;
   Output.Completions.back().CompletionTokenRange = ReplacedRange;
 }

kadircet wrote:
> why not handle `SnippetSuffix` in here ?
> 
> instead of propagating `IsUsingDeclaration`, we can just drop 
> `Output.Completions.back().SnippetSuffix` in here, which sounds like a more 
> appropriate layering.
> considering we don't really have context specific knowledge in 
> `CodeCompletionBuilder` ?
I was trying to keep all processing of snippets in one place. 
Code completion code is hard enough to navigate as it stands today...

Although I agree doing this when summarizing items in a bundles looks like the 
wrong layer, but this place is also after bundling, so I'm not sure if it's 
actually winning us much.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69382



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

[PATCH] D69431: [clangd] Do not highlight keywords in semantic highlighting

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

merge_guards_bot added a comment.

Build result: fail - 59585 tests passed, 2 failed and 805 were skipped.

  failed: Clangd.Clangd/semantic-highlighting.test
  failed: LLVM.tools/llvm-ar/mri-utf8.test

Log files: cmake-log.txt 
, 
ninja_check_all-log.txt 
, 
CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69431



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

[PATCH] D69404: [clang-format] [NFC] update the documentation in Format.h to allow dump_format_style.py to get a little closer to being correct.

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar accepted this revision.
mitchell-stellar added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69404



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

[clang] 11c2a85 - [NFC] Rename LLVM_NO_DEAD_STRIP

[David Tenty via cfe-commits]

Author: David Tenty
Date: 2019-10-25T09:32:00-04:00
New Revision: 11c2a85db8849db1a5907e80d9966592248ef825

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

LOG: [NFC] Rename LLVM_NO_DEAD_STRIP

Summary:
The variable LLVM_NO_DEAD_STRIP is set in LLVM cmake files when building 
executables that might make use of plugins .The name of the variable does not 
convey the actual intended usage (i.e. for use with tools that have plugins), 
just what the eventual effect of setting in on some (i.e. not garbage 
collecting unused symbols).

This patch renames it to LLVM_SUPPORT_PLUGINS to convey the intended usage, 
which will allow subsequent patches to add behavior to support that in 
different ways without confusion about whether it will do on, for example, 
non-gnu platforms.

Reviewers: hubert.reinterpretcast, stevewan

Reviewed By: stevewan

Subscribers: cfe-commits, mgorny, llvm-commits

Tags: #llvm, #clang

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

Added: 


Modified: 
clang/tools/driver/CMakeLists.txt
llvm/cmake/modules/AddLLVM.cmake
llvm/cmake/modules/HandleLLVMOptions.cmake
llvm/tools/bugpoint/CMakeLists.txt
llvm/tools/llc/CMakeLists.txt
llvm/tools/opt/CMakeLists.txt

Removed: 




diff  --git a/clang/tools/driver/CMakeLists.txt 
b/clang/tools/driver/CMakeLists.txt
index 590d708d837c..5a8f57eb4668 100644
--- a/clang/tools/driver/CMakeLists.txt
+++ b/clang/tools/driver/CMakeLists.txt
@@ -20,9 +20,9 @@ set( LLVM_LINK_COMPONENTS
 option(CLANG_PLUGIN_SUPPORT "Build clang with plugin support" ON)
 
 # Support plugins. This must be before add_clang_executable as it reads
-# LLVM_NO_DEAD_STRIP.
+# LLVM_SUPPORT_PLUGINS.
 if(CLANG_PLUGIN_SUPPORT)
-  set(LLVM_NO_DEAD_STRIP 1)
+  set(LLVM_SUPPORT_PLUGINS 1)
 endif()
 
 if(NOT CLANG_BUILT_STANDALONE)

diff  --git a/llvm/cmake/modules/AddLLVM.cmake 
b/llvm/cmake/modules/AddLLVM.cmake
index b5f612469ff9..41e72c7e9bcc 100644
--- a/llvm/cmake/modules/AddLLVM.cmake
+++ b/llvm/cmake/modules/AddLLVM.cmake
@@ -228,7 +228,7 @@ function(add_link_opts target_name)
   # to enable. See https://sourceware.org/bugzilla/show_bug.cgi?id=17704.
 endif()
 
-if(NOT LLVM_NO_DEAD_STRIP)
+if(NOT LLVM_SUPPORT_PLUGINS)
   if(${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
 # ld64's implementation of -dead_strip breaks tools that use plugins.
 set_property(TARGET ${target_name} APPEND_STRING PROPERTY
@@ -245,7 +245,7 @@ function(add_link_opts target_name)
 set_property(TARGET ${target_name} APPEND_STRING PROPERTY
  LINK_FLAGS " -Wl,--gc-sections")
   endif()
-else() #LLVM_NO_DEAD_STRIP
+else() #LLVM_SUPPORT_PLUGINS
   if(${CMAKE_SYSTEM_NAME} MATCHES "AIX")
 set_property(TARGET ${target_name} APPEND_STRING PROPERTY
  LINK_FLAGS " -Wl,-bnogc")

diff  --git a/llvm/cmake/modules/HandleLLVMOptions.cmake 
b/llvm/cmake/modules/HandleLLVMOptions.cmake
index a84aff104a44..75c2df9eb559 100644
--- a/llvm/cmake/modules/HandleLLVMOptions.cmake
+++ b/llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -772,7 +772,7 @@ endif()
 # Add flags for add_dead_strip().
 # FIXME: With MSVS, consider compiling with /Gy and linking with /OPT:REF?
 # But MinSizeRel seems to add that automatically, so maybe disable these
-# flags instead if LLVM_NO_DEAD_STRIP is set.
+# flags instead if LLVM_SUPPORT_PLUGINS is set.
 if(NOT CYGWIN AND NOT WIN32)
   if(NOT ${CMAKE_SYSTEM_NAME} MATCHES "Darwin" AND
  NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG")

diff  --git a/llvm/tools/bugpoint/CMakeLists.txt 
b/llvm/tools/bugpoint/CMakeLists.txt
index 031f51480cce..8b468691fa6b 100644
--- a/llvm/tools/bugpoint/CMakeLists.txt
+++ b/llvm/tools/bugpoint/CMakeLists.txt
@@ -22,7 +22,7 @@ set(LLVM_LINK_COMPONENTS
   )
 
 # Support plugins.
-set(LLVM_NO_DEAD_STRIP 1)
+set(LLVM_SUPPORT_PLUGINS 1)
 
 add_llvm_tool(bugpoint
   BugDriver.cpp

diff  --git a/llvm/tools/llc/CMakeLists.txt b/llvm/tools/llc/CMakeLists.txt
index 300efdab10d0..8435b9de6fb4 100644
--- a/llvm/tools/llc/CMakeLists.txt
+++ b/llvm/tools/llc/CMakeLists.txt
@@ -20,7 +20,7 @@ set(LLVM_LINK_COMPONENTS
   )
 
 # Support plugins.
-set(LLVM_NO_DEAD_STRIP 1)
+set(LLVM_SUPPORT_PLUGINS 1)
 
 add_llvm_tool(llc
   llc.cpp

diff  --git a/llvm/tools/opt/CMakeLists.txt b/llvm/tools/opt/CMakeLists.txt
index cb4ba5cfbced..4f74a0571203 100644
--- a/llvm/tools/opt/CMakeLists.txt
+++ b/llvm/tools/opt/CMakeLists.txt
@@ -25,7 +25,7 @@ set(LLVM_LINK_COMPONENTS
   )
 
 # Support plugins.
-set(LLVM_NO_DEAD_STRIP 1)
+set(LLVM_SUPPORT_PLUGINS 1)
 
 add_llvm_tool(opt
   AnalysisWrappers.cpp



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

[PATCH] D69382: [clangd] Do not insert parentheses when completing a using declaration

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:483
   bool EnableFunctionArgSnippets;
+  bool CompleteArgumentList;
 };

maybe rather use `GenerateSnippets`? Since this doesn't generate completions 
for the snippets, but rather disables generation of any snippets at all.

also I think it makes sense to document this one, because of the field just 
above it.



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1466
   Output.Completions.back().Score = C.second;
   Output.Completions.back().CompletionTokenRange = ReplacedRange;
 }

ilya-biryukov wrote:
> kadircet wrote:
> > why not handle `SnippetSuffix` in here ?
> > 
> > instead of propagating `IsUsingDeclaration`, we can just drop 
> > `Output.Completions.back().SnippetSuffix` in here, which sounds like a more 
> > appropriate layering.
> > considering we don't really have context specific knowledge in 
> > `CodeCompletionBuilder` ?
> I was trying to keep all processing of snippets in one place. 
> Code completion code is hard enough to navigate as it stands today...
> 
> Although I agree doing this when summarizing items in a bundles looks like 
> the wrong layer, but this place is also after bundling, so I'm not sure if 
> it's actually winning us much.
> 
ah ok, that's also a good concern. feel free to choose one or the other then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69382



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

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

[David Tenty via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG11c2a85db884: [NFC] Rename LLVM_NO_DEAD_STRIP (authored by 
daltenty).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69356

Files:
  clang/tools/driver/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/HandleLLVMOptions.cmake
  llvm/tools/bugpoint/CMakeLists.txt
  llvm/tools/llc/CMakeLists.txt
  llvm/tools/opt/CMakeLists.txt


Index: llvm/tools/opt/CMakeLists.txt
===
--- llvm/tools/opt/CMakeLists.txt
+++ llvm/tools/opt/CMakeLists.txt
@@ -25,7 +25,7 @@
   )
 
 # Support plugins.
-set(LLVM_NO_DEAD_STRIP 1)
+set(LLVM_SUPPORT_PLUGINS 1)
 
 add_llvm_tool(opt
   AnalysisWrappers.cpp
Index: llvm/tools/llc/CMakeLists.txt
===
--- llvm/tools/llc/CMakeLists.txt
+++ llvm/tools/llc/CMakeLists.txt
@@ -20,7 +20,7 @@
   )
 
 # Support plugins.
-set(LLVM_NO_DEAD_STRIP 1)
+set(LLVM_SUPPORT_PLUGINS 1)
 
 add_llvm_tool(llc
   llc.cpp
Index: llvm/tools/bugpoint/CMakeLists.txt
===
--- llvm/tools/bugpoint/CMakeLists.txt
+++ llvm/tools/bugpoint/CMakeLists.txt
@@ -22,7 +22,7 @@
   )
 
 # Support plugins.
-set(LLVM_NO_DEAD_STRIP 1)
+set(LLVM_SUPPORT_PLUGINS 1)
 
 add_llvm_tool(bugpoint
   BugDriver.cpp
Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -772,7 +772,7 @@
 # Add flags for add_dead_strip().
 # FIXME: With MSVS, consider compiling with /Gy and linking with /OPT:REF?
 # But MinSizeRel seems to add that automatically, so maybe disable these
-# flags instead if LLVM_NO_DEAD_STRIP is set.
+# flags instead if LLVM_SUPPORT_PLUGINS is set.
 if(NOT CYGWIN AND NOT WIN32)
   if(NOT ${CMAKE_SYSTEM_NAME} MATCHES "Darwin" AND
  NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG")
Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -228,7 +228,7 @@
   # to enable. See https://sourceware.org/bugzilla/show_bug.cgi?id=17704.
 endif()
 
-if(NOT LLVM_NO_DEAD_STRIP)
+if(NOT LLVM_SUPPORT_PLUGINS)
   if(${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
 # ld64's implementation of -dead_strip breaks tools that use plugins.
 set_property(TARGET ${target_name} APPEND_STRING PROPERTY
@@ -245,7 +245,7 @@
 set_property(TARGET ${target_name} APPEND_STRING PROPERTY
  LINK_FLAGS " -Wl,--gc-sections")
   endif()
-else() #LLVM_NO_DEAD_STRIP
+else() #LLVM_SUPPORT_PLUGINS
   if(${CMAKE_SYSTEM_NAME} MATCHES "AIX")
 set_property(TARGET ${target_name} APPEND_STRING PROPERTY
  LINK_FLAGS " -Wl,-bnogc")
Index: clang/tools/driver/CMakeLists.txt
===
--- clang/tools/driver/CMakeLists.txt
+++ clang/tools/driver/CMakeLists.txt
@@ -20,9 +20,9 @@
 option(CLANG_PLUGIN_SUPPORT "Build clang with plugin support" ON)
 
 # Support plugins. This must be before add_clang_executable as it reads
-# LLVM_NO_DEAD_STRIP.
+# LLVM_SUPPORT_PLUGINS.
 if(CLANG_PLUGIN_SUPPORT)
-  set(LLVM_NO_DEAD_STRIP 1)
+  set(LLVM_SUPPORT_PLUGINS 1)
 endif()
 
 if(NOT CLANG_BUILT_STANDALONE)


Index: llvm/tools/opt/CMakeLists.txt
===
--- llvm/tools/opt/CMakeLists.txt
+++ llvm/tools/opt/CMakeLists.txt
@@ -25,7 +25,7 @@
   )
 
 # Support plugins.
-set(LLVM_NO_DEAD_STRIP 1)
+set(LLVM_SUPPORT_PLUGINS 1)
 
 add_llvm_tool(opt
   AnalysisWrappers.cpp
Index: llvm/tools/llc/CMakeLists.txt
===
--- llvm/tools/llc/CMakeLists.txt
+++ llvm/tools/llc/CMakeLists.txt
@@ -20,7 +20,7 @@
   )
 
 # Support plugins.
-set(LLVM_NO_DEAD_STRIP 1)
+set(LLVM_SUPPORT_PLUGINS 1)
 
 add_llvm_tool(llc
   llc.cpp
Index: llvm/tools/bugpoint/CMakeLists.txt
===
--- llvm/tools/bugpoint/CMakeLists.txt
+++ llvm/tools/bugpoint/CMakeLists.txt
@@ -22,7 +22,7 @@
   )
 
 # Support plugins.
-set(LLVM_NO_DEAD_STRIP 1)
+set(LLVM_SUPPORT_PLUGINS 1)
 
 add_llvm_tool(bugpoint
   BugDriver.cpp
Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -772,7 +772,7 @@
 # Add flags for add_dead_strip().
 # FIXME: With MSVS, consider compiling with /Gy and linking with /OPT:REF?
 # But MinSizeRel seems to add that automatically, so maybe disable these
-# flags instead if LLVM

[clang] 6df7ef0 - [clang-format] [NFC] update the documentation in Format.h to allow dump_format_style.py to get a little closer to being correct.

[via cfe-commits]

Author: paulhoad
Date: 2019-10-25T14:46:19+01:00
New Revision: 6df7ef0d8baac34259e2c93043d843f27812c534

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

LOG: [clang-format] [NFC] update the documentation in Format.h to allow 
dump_format_style.py to get a little closer to being correct.

Summary:
Running dump_format_style.py on the tip of the trunk causes 
ClangFormatStyleOptions.rst to have changes, which I think ideally it shouldn't.

Some recent commits have meant Format.h and ClangFormatStyleOptions.rst have 
become out of sync such that dump_format_style.py either couldn't be run or 
generated incorrect .rst files.

It's also important to remember to edit the IncludeStyles from Tooling.

Make a couple of changes {D6833} {D64695} which came from recent clang-format 
commits that missed this step. There are still a couple of other changes  from 
commit {D67541} {D68296} which also need to be looked at, but I'd like to park 
these first two changes first.

The authors of these original commits I've included in the reviewers, I'm happy 
to work on the changes, just really need to know which is the ground truth of 
the changes you want to keep (whats in the Format.h) or what is in the 
ClangFormatStyleOptions.rst

Reviewers: klimek, mitchell-stellar, owenpan, jvoung, Manikishan, sammccall

Reviewed By: mitchell-stellar

Subscribers: cfe-commits

Tags: #clang-format, #clang

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

Added: 


Modified: 
clang/include/clang/Format/Format.h
clang/include/clang/Tooling/Inclusions/IncludeStyle.h

Removed: 




diff  --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 1095821eb664..b38d1f7d9402 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -706,21 +706,19 @@ struct FormatStyle {
 ///   };
 /// \endcode
 BS_Allman,
-/// Always break before braces and add an extra level of indentation to
-/// braces of control statements, not to those of class, function
-/// or other definitions.
+/// Like ``Allman`` but always indent braces and line up code with braces.
 /// \code
-///   try
+///try
 /// {
-///   foo();
+/// foo();
 /// }
 ///   catch ()
 /// {
 /// }
 ///   void foo() { bar(); }
 ///   class foo
-///   {
-///   };
+/// {
+/// };
 ///   if (foo())
 /// {
 /// }
@@ -728,25 +726,27 @@ struct FormatStyle {
 /// {
 /// }
 ///   enum X : int
-///   {
+/// {
 /// A,
 /// B
-///   };
+/// };
 /// \endcode
 BS_Whitesmiths,
-/// Like ``Allman`` but always indent braces and line up code with braces.
+/// Always break before braces and add an extra level of indentation to
+/// braces of control statements, not to those of class, function
+/// or other definitions.
 /// \code
-///try
+///   try
 /// {
-/// foo();
+///   foo();
 /// }
 ///   catch ()
 /// {
 /// }
 ///   void foo() { bar(); }
 ///   class foo
-/// {
-/// };
+///   {
+///   };
 ///   if (foo())
 /// {
 /// }
@@ -754,10 +754,10 @@ struct FormatStyle {
 /// {
 /// }
 ///   enum X : int
-/// {
+///   {
 /// A,
 /// B
-/// };
+///   };
 /// \endcode
 BS_GNU,
 /// Like ``Attach``, but break before functions.

diff  --git a/clang/include/clang/Tooling/Inclusions/IncludeStyle.h 
b/clang/include/clang/Tooling/Inclusions/IncludeStyle.h
index 266763a5b1bd..9cade29a8545 100644
--- a/clang/include/clang/Tooling/Inclusions/IncludeStyle.h
+++ b/clang/include/clang/Tooling/Inclusions/IncludeStyle.h
@@ -84,18 +84,27 @@ struct IncludeStyle {
   /// (https://llvm.org/docs/CodingStandards.html#include-style). However, you
   /// can also assign negative priorities if you have certain headers that
   /// always need to be first.
+  /// 
+  /// There is a third and optional field ``SortPriority`` which can used while
+  /// ``IncludeBloks = IBS_Regroup`` to define the priority in which 
``#includes``
+  /// should be ordered, and value of ``Priority`` defines the order of
+  /// ``#include blocks`` and also enables to group ``#includes`` of 
diff erent
+  /// priority for order.``SortPriority`` is set to the value of ``Priority``
+  /// as default if it is not assigned.
   ///
   /// To configure this in the .clang-format file, use:
   /// \code{.yaml}
   ///   IncludeCategories:
   /// - Regex:   '^"(llvm|llvm-c|clang|clang-c)/'
   ///   Priority:2
+  ///   SortPriority:  

[PATCH] D69404: [clang-format] [NFC] update the documentation in Format.h to allow dump_format_style.py to get a little closer to being correct.

[MyDeveloperDay via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6df7ef0d8baa: [clang-format] [NFC] update the documentation 
in Format.h to allow… (authored by MyDeveloperDay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69404

Files:
  clang/include/clang/Format/Format.h
  clang/include/clang/Tooling/Inclusions/IncludeStyle.h

Index: clang/include/clang/Tooling/Inclusions/IncludeStyle.h
===
--- clang/include/clang/Tooling/Inclusions/IncludeStyle.h
+++ clang/include/clang/Tooling/Inclusions/IncludeStyle.h
@@ -84,18 +84,27 @@
   /// (https://llvm.org/docs/CodingStandards.html#include-style). However, you
   /// can also assign negative priorities if you have certain headers that
   /// always need to be first.
+  /// 
+  /// There is a third and optional field ``SortPriority`` which can used while
+  /// ``IncludeBloks = IBS_Regroup`` to define the priority in which ``#includes``
+  /// should be ordered, and value of ``Priority`` defines the order of
+  /// ``#include blocks`` and also enables to group ``#includes`` of different
+  /// priority for order.``SortPriority`` is set to the value of ``Priority``
+  /// as default if it is not assigned.
   ///
   /// To configure this in the .clang-format file, use:
   /// \code{.yaml}
   ///   IncludeCategories:
   /// - Regex:   '^"(llvm|llvm-c|clang|clang-c)/'
   ///   Priority:2
+  ///   SortPriority:2
   /// - Regex:   '^(<|"(gtest|gmock|isl|json)/)'
   ///   Priority:3
   /// - Regex:   '<[[:alnum:].]+>'
   ///   Priority:4
   /// - Regex:   '.*'
   ///   Priority:1
+  ///   SortPriority:0
   /// \endcode
   std::vector IncludeCategories;
 
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -706,21 +706,19 @@
 ///   };
 /// \endcode
 BS_Allman,
-/// Always break before braces and add an extra level of indentation to
-/// braces of control statements, not to those of class, function
-/// or other definitions.
+/// Like ``Allman`` but always indent braces and line up code with braces.
 /// \code
-///   try
+///try
 /// {
-///   foo();
+/// foo();
 /// }
 ///   catch ()
 /// {
 /// }
 ///   void foo() { bar(); }
 ///   class foo
-///   {
-///   };
+/// {
+/// };
 ///   if (foo())
 /// {
 /// }
@@ -728,25 +726,27 @@
 /// {
 /// }
 ///   enum X : int
-///   {
+/// {
 /// A,
 /// B
-///   };
+/// };
 /// \endcode
 BS_Whitesmiths,
-/// Like ``Allman`` but always indent braces and line up code with braces.
+/// Always break before braces and add an extra level of indentation to
+/// braces of control statements, not to those of class, function
+/// or other definitions.
 /// \code
-///try
+///   try
 /// {
-/// foo();
+///   foo();
 /// }
 ///   catch ()
 /// {
 /// }
 ///   void foo() { bar(); }
 ///   class foo
-/// {
-/// };
+///   {
+///   };
 ///   if (foo())
 /// {
 /// }
@@ -754,10 +754,10 @@
 /// {
 /// }
 ///   enum X : int
-/// {
+///   {
 /// A,
 /// B
-/// };
+///   };
 /// \endcode
 BS_GNU,
 /// Like ``Attach``, but break before functions.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68391: [RISCV] Improve sysroot computation if no GCC install detected

[Sam Elliott via Phabricator via cfe-commits]

lenary accepted this revision.
lenary added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: sameer.abuasal.

LGTM.


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

https://reviews.llvm.org/D68391



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

[PATCH] D69382: [clangd] Do not insert parentheses when completing a using declaration

[Ilya Biryukov via Phabricator via cfe-commits]

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

- Rename flag to GenerateSnippets, document it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69382

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/CodeCompleteConsumer.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Sema/SemaCodeComplete.cpp

Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -5330,18 +5330,21 @@
 }
 
 void Sema::CodeCompleteQualifiedId(Scope *S, CXXScopeSpec &SS,
-   bool EnteringContext, QualType BaseType,
+   bool EnteringContext,
+   bool IsUsingDeclaration, QualType BaseType,
QualType PreferredType) {
   if (SS.isEmpty() || !CodeCompleter)
 return;
 
+  CodeCompletionContext CC(CodeCompletionContext::CCC_Symbol, PreferredType);
+  CC.setIsUsingDeclaration(IsUsingDeclaration);
+  CC.setCXXScopeSpecifier(SS);
+
   // We want to keep the scope specifier even if it's invalid (e.g. the scope
   // "a::b::" is not corresponding to any context/namespace in the AST), since
   // it can be useful for global code completion which have information about
   // contexts/symbols that are not in the AST.
   if (SS.isInvalid()) {
-CodeCompletionContext CC(CodeCompletionContext::CCC_Symbol, PreferredType);
-CC.setCXXScopeSpecifier(SS);
 // As SS is invalid, we try to collect accessible contexts from the current
 // scope with a dummy lookup so that the completion consumer can try to
 // guess what the specified scope is.
@@ -5371,10 +5374,8 @@
   if (!isDependentScopeSpecifier(SS) && RequireCompleteDeclContext(SS, Ctx))
 return;
 
-  ResultBuilder Results(
-  *this, CodeCompleter->getAllocator(),
-  CodeCompleter->getCodeCompletionTUInfo(),
-  CodeCompletionContext(CodeCompletionContext::CCC_Symbol, PreferredType));
+  ResultBuilder Results(*this, CodeCompleter->getAllocator(),
+CodeCompleter->getCodeCompletionTUInfo(), CC);
   if (!PreferredType.isNull())
 Results.setPreferredType(PreferredType);
   Results.EnterNewScope();
@@ -5403,23 +5404,21 @@
CodeCompleter->loadExternal());
   }
 
-  auto CC = Results.getCompletionContext();
-  CC.setCXXScopeSpecifier(SS);
-
-  HandleCodeCompleteResults(this, CodeCompleter, CC, Results.data(),
-Results.size());
+  HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
 }
 
 void Sema::CodeCompleteUsing(Scope *S) {
   if (!CodeCompleter)
 return;
 
+  // This can be both a using alias or using declaration, in the former we
+  // expect a new name and a symbol in the latter case.
+  CodeCompletionContext Context(CodeCompletionContext::CCC_SymbolOrNewName);
+  Context.setIsUsingDeclaration(true);
+
   ResultBuilder Results(*this, CodeCompleter->getAllocator(),
-CodeCompleter->getCodeCompletionTUInfo(),
-// This can be both a using alias or using
-// declaration, in the former we expect a new name and a
-// symbol in the latter case.
-CodeCompletionContext::CCC_SymbolOrNewName,
+CodeCompleter->getCodeCompletionTUInfo(), Context,
 &ResultBuilder::IsNestedNameSpecifier);
   Results.EnterNewScope();
 
Index: clang/lib/Parse/ParseExprCXX.cpp
===
--- clang/lib/Parse/ParseExprCXX.cpp
+++ clang/lib/Parse/ParseExprCXX.cpp
@@ -143,13 +143,10 @@
 /// \param OnlyNamespace If true, only considers namespaces in lookup.
 ///
 /// \returns true if there was an error parsing a scope specifier
-bool Parser::ParseOptionalCXXScopeSpecifier(CXXScopeSpec &SS,
-ParsedType ObjectType,
-bool EnteringContext,
-bool *MayBePseudoDestructor,
-bool IsTypename,
-IdentifierInfo **LastII,
-bool OnlyNamespace) {
+bool Parser::ParseOptionalCXXScopeSpecifier(
+CXXScopeSpec &SS, ParsedType ObjectType, bool EnteringContext,
+bool *MayBePseudoDestructor, bool IsTypename, IdentifierInfo **LastII,
+bool OnlyNamespace, bool 

[PATCH] D69382: [clangd] Do not insert parentheses when completing a using declaration

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

merge_guards_bot added a comment.

Build result: pass - 59630 tests passed, 0 failed and 763 were skipped.
Log files: cmake-log.txt 
, 
ninja_check_all-log.txt 
, 
CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69382



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

[PATCH] D69316: [OpenMP 5.0] target update list items need not be contiguous (Sema)

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:14450
 
+  if (HasMeetPointer && !IsLastArraySection) {
+SemaRef.Diag(ELoc, diag::err_omp_pointer_type_not_last)

Ad the check for the version of OpenMP standard, it must be allowed only for 
OpenMP >= 50.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:14470
   AllowWholeSizeArraySection = false;
-  } else if (AllowUnitySizeArraySection && NotUnity) {
+  } else if (DKind != OMPD_target_update &&
+ (AllowUnitySizeArraySection && NotUnity)) {

Same here, allow only for OpenMP >= 50. Also, split the patch into 2 parts, one 
for pointers stuff and another one for target update directive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69316



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

[PATCH] D69263: [clangd] Implement cross-file rename.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:269
+cat(Features),
+desc("Enable cross-file rename feature."),
+init(false),

Could you document that the feature is highly experimental and may lead to 
broken code or incomplete renames for the time being?

Also, maybe make it hidden for now? 
At least until we have basic validation of ranges from the index. In the 
current state we can easily produce edits to unrelated parts of the file...



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:277
+  //   - file on VFS for bar.cc;
+  Annotations MainCode("class  [[Fo^o]] {};");
+  auto MainFilePath = testPath("main.cc");

Thinking whether we can encode these transformations in a nicer way?
If we could convert the contents dirty buffer and replacements to something 
like:
```
class [[Foo |-> newName]] {};
```
Just a proposal, maybe there's a better syntax here.

We can easily match strings instead of matching ranges in the tests. This has 
the advantage of having textual diffs in case something goes wrong - much more 
pleasant than looking at the offsets in the ranges.

WDYT?



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:302
+  testing::UnorderedElementsAre(
+  PairMatcher(Eq(FooPath),
+  EqualFileEdit(FooDirtyBuffer, FooCode.ranges())),

Instead of defining custom matchers, could we convert to standard types and use 
existing gtest matchers?
Something like `std::vector>` should work 
just fine.

Huge advantage we'll get is better output in case of errors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69263



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

[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:52-74
 enum DefaultMapAttributes {
-  DMA_unspecified,   /// Default mapping is not specified.
-  DMA_tofrom_scalar, /// Default mapping is 'tofrom:scalar'.
+  DMA_unspecified, /// Default mapping is not specified.
+  DMA_alloc_scalar,/// Default mapping is 'alloc:scalar'.
+  DMA_to_scalar,   /// Default mapping is 'to:scalar'.
+  DMA_from_scalar, /// Default mapping is 'from:scalar'.
+  DMA_tofrom_scalar,   /// Default mapping is 'tofrom:scalar'.
+  DMA_firstprivate_scalar, /// Default mapping is 'firstprivate:scalar'.

Bad decision, better to add 2 new enums, one for behavior and one for the 
variable category.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:147-148
 SourceLocation DefaultAttrLoc;
-DefaultMapAttributes DefaultMapAttr = DMA_unspecified;
-SourceLocation DefaultMapAttrLoc;
+llvm::SmallVector, 3>
+  DefaultmapMap{3, std::make_pair(DMA_unspecified, SourceLocation())};
 OpenMPDirectiveKind Directive = OMPD_unknown;

No need to use the vector here, use the regular array



Comment at: clang/lib/Sema/SemaOpenMP.cpp:2983-2992
+  // OpenMP 5.0 [2.19.7.2, defaultmap clause, Description]
+  // If implicit-behavior is none, each variable referenced in the
+  // construct that does not have a predetermined data-sharing attribute
+  // and does not appear in a to or link clause on a declare target
+  // directive must be listed in a data-mapping attribute clause, a
+  // data-haring attribute clause (including a data-sharing attribute
+  // clause on a combined construct where target. is one of the

It must be active only for OpenMP >= 50.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:4463-4890
+
   if (AStmt && !CurContext->isDependentContext()) {
 assert(isa(AStmt) && "Captured statement expected");
 
 // Check default data sharing attributes for referenced variables.
 DSAAttrChecker DSAChecker(DSAStack, *this, cast(AStmt));
+

The formatting changes better to put in a separate NFC patch if you think that 
this is required.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:4899-4903
+} else {
+  Diag(P.second->getExprLoc(), 
diag::err_omp_defaultmap_no_attr_for_variable)
+  << P.first << P.second->getSourceRange();
+  Diag(DSAStack->getDefaultDSALocation(), 
diag::note_omp_defaultmap_attr_none);
+}

Add a check for OpenMP version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204



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

[PATCH] D67031: [Clang][Bundler] Error reporting improvements

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


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

https://reviews.llvm.org/D67031



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

[PATCH] D69433: [clang-format] [NFC] update the documentation in Format.h to allow dump_format_style.py to get a little closer to being correct. (part 2)

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: klimek, mitchell-stellar, sammccall.
MyDeveloperDay added projects: clang, clang-format.

a change D67541: [ClangFormat] Future-proof Standard option, allow floating or 
pinning to arbitrary lang version  cause 
LanguageStandard to now be subtly different from all other clang-format 
options, in that the Enum value (less the prefix) is not always allowed as 
valid as the configuration option.

This caused the ClangFormatStyleOptions.rst and the Format.h to diverge so that 
the ClangFormatStyleOptions.rst could no longer be generated from the Format.h 
using dump_format_stlye.py

This fix tried to remedy that:

1. by allowing an additional comment (in Format.h) after the enum to be used as 
the `in configuration (  )`  text, and changing the dump_format_style.py to 
support that.

This makes the following code:

  enum {
  ...
  LS_Cpp03, // c++03
  LS_Cpp11, // c++11
  ...
  };

would render as:

  * ``LS_Cpp03`` (in configuration: ``c++03``)
  * ``LS_Cpp11`` (in configuration: ``c++11``)

And we also  move the deprecated alias into the text of the enum (otherwise it 
won't be added at the end as an option)

This patch includes a couple of other whitespace changes which help bring 
Format.h and ClangFormatStyleOptions.rst almost back into line and 
regeneratable...  (there is still one more)


Repository:
  rC Clang

https://reviews.llvm.org/D69433

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/tools/dump_format_style.py
  clang/include/clang/Format/Format.h

Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -708,7 +708,7 @@
 BS_Allman,
 /// Like ``Allman`` but always indent braces and line up code with braces.
 /// \code
-///try
+///   try
 /// {
 /// foo();
 /// }
@@ -850,6 +850,7 @@
 ///   {};
 /// \endcode
 bool AfterClass;
+
 /// Wrap control statements (``if``/``for``/``while``/``switch``/..).
 BraceWrappingAfterControlStatementStyle AfterControlStatement;
 /// Wrap enum definitions.
@@ -1965,7 +1966,8 @@
   bool SpacesInParentheses;
 
   /// If ``true``, spaces will be inserted after ``[`` and before ``]``.
-  /// Lambdas or unspecified size array declarations will not be affected.
+  /// Lambdas without arguments or unspecified size array declarations will not
+  /// be affected.
   /// \code
   ///true:  false:
   ///int a[ 5 ];vs. int a[5];
@@ -1982,26 +1984,29 @@
   /// The correct way to spell a specific language version is e.g. ``c++11``.
   /// The historical aliases ``Cpp03`` and ``Cpp11`` are deprecated.
   enum LanguageStandard {
-/// c++03: Parse and format as C++03.
-LS_Cpp03,
-/// c++11: Parse and format as C++11.
-LS_Cpp11,
-/// c++14: Parse and format as C++14.
-LS_Cpp14,
-/// c++17: Parse and format as C++17.
-LS_Cpp17,
-/// c++20: Parse and format as C++20.
-LS_Cpp20,
-/// Latest: Parse and format using the latest supported language version.
-/// 'Cpp11' is an alias for LS_Latest for historical reasons.
+/// Use C++03-compatible syntax.
+/// ``Cpp03``: deprecated alias for ``c++03``
+LS_Cpp03, // c++03
+/// Use C++11-compatible syntax.
+LS_Cpp11, // c++11
+/// Use C++14-compatible syntax.
+/// ``Cpp11``: deprecated alias for ``Latest``
+LS_Cpp14, // c++14
+/// Use C++17-compatible syntax.
+LS_Cpp17, // c++17
+/// Use C++20-compatible syntax.
+LS_Cpp20, // c++20
+/// Parse and format using the latest supported language version.
 LS_Latest,
-/// Auto: Automatic detection based on the input.
-/// Parse using the latest language version. Format based on detected input.
+/// Automatic detection based on the input.
 LS_Auto,
   };
 
-  /// Format compatible with this standard, e.g. use ``A >``
-  /// instead of ``A>`` for ``LS_Cpp03``.
+  /// Parse and Format C++ constructs compatible with this standard.
+  /// \code
+  ///c++03: latest:
+  ///vector > x;   vs. vector> x;
+  /// \endcode
   LanguageStandard Standard;
 
   /// The number of columns used for tab stops.
Index: clang/docs/tools/dump_format_style.py
===
--- clang/docs/tools/dump_format_style.py
+++ clang/docs/tools/dump_format_style.py
@@ -78,14 +78,15 @@
 return '\n'.join(map(str, self.values))
 
 class EnumValue(object):
-  def __init__(self, name, comment):
+  def __init__(self, name, comment, config):
 self.name = name
 self.comment = comment
+self.config = config
 
   def __str__(self):
 return '* ``%s`` (in configuration: ``%s``)\n%s' % (
 self.name,
-re.sub('

[PATCH] D69382: [clangd] Do not insert parentheses when completing a using declaration

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

oops, thought I've LGTM'd it on previous cycle :D




Comment at: clang-tools-extra/clangd/CodeComplete.cpp:483
   bool EnableFunctionArgSnippets;
+  bool CompleteArgumentList;
 };

kadircet wrote:
> maybe rather use `GenerateSnippets`? Since this doesn't generate completions 
> for the snippets, but rather disables generation of any snippets at all.
> 
> also I think it makes sense to document this one, because of the field just 
> above it.
not just function, also template arguments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69382



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

[PATCH] D69250: [ARM][AArch64] Implement __cls and __clsl intrinsics from ACLE

[Victor Campos via Phabricator via cfe-commits]

vhscampos updated this revision to Diff 226430.
vhscampos added a comment.

Add support for __clsll.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69250

Files:
  clang/include/clang/Basic/BuiltinsAArch64.def
  clang/include/clang/Basic/BuiltinsARM.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/arm_acle.h
  clang/test/CodeGen/arm_acle.c
  clang/test/CodeGen/builtins-arm.c
  clang/test/CodeGen/builtins-arm64.c
  llvm/include/llvm/IR/IntrinsicsAArch64.td
  llvm/include/llvm/IR/IntrinsicsARM.td
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/test/CodeGen/AArch64/cls.ll
  llvm/test/CodeGen/ARM/cls.ll

Index: llvm/test/CodeGen/ARM/cls.ll
===
--- /dev/null
+++ llvm/test/CodeGen/ARM/cls.ll
@@ -0,0 +1,27 @@
+; RUN: llc -mtriple=armv5 %s -o - | FileCheck %s
+
+; CHECK:  eor [[T:r[0-9]+]], [[T]], [[T]], asr #31
+; CHECK-NEXT: mov [[C1:r[0-9]+]], #1
+; CHECK-NEXT: orr [[T]], [[C1]], [[T]], lsl #1
+; CHECK-NEXT: clz [[T]], [[T]]
+define i32 @cls(i32 %t) {
+  %cls.i = call i32 @llvm.arm.cls(i32 %t)
+  ret i32 %cls.i
+}
+
+; CHECK: cmp r1, #0
+; CHECK: mvnne [[ADJUSTEDLO:r[0-9]+]], r0
+; CHECK: clz [[CLZLO:r[0-9]+]], [[ADJUSTEDLO]]
+; CHECK: eor [[A:r[0-9]+]], r1, r1, asr #31
+; CHECK: mov r1, #1
+; CHECK: orr [[A]], r1, [[A]], lsl #1
+; CHECK: clz [[CLSHI:r[0-9]+]], [[A]]
+; CHECK: cmp [[CLSHI]], #31
+; CHECK: addeq r0, [[CLZLO]], #31
+define i32 @cls64(i64 %t) {
+  %cls.i = call i32 @llvm.arm.cls64(i64 %t)
+  ret i32 %cls.i
+}
+
+declare i32 @llvm.arm.cls(i32) nounwind
+declare i32 @llvm.arm.cls64(i64) nounwind
Index: llvm/test/CodeGen/AArch64/cls.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/cls.ll
@@ -0,0 +1,20 @@
+; RUN: llc -mtriple=aarch64 %s -o - | FileCheck %s
+
+; @llvm.aarch64.cls must be directly translated into the 'cls' instruction
+
+; CHECK-LABEL: cls
+; CHECK: cls [[REG:w[0-9]+]], [[REG]]
+define i32 @cls(i32 %t) {
+  %cls.i = call i32 @llvm.aarch64.cls(i32 %t)
+  ret i32 %cls.i
+}
+
+; CHECK-LABEL: cls64
+; CHECK: cls [[REG:x[0-9]+]], [[REG]]
+define i32 @cls64(i64 %t) {
+  %cls.i = call i32 @llvm.aarch64.cls64(i64 %t)
+  ret i32 %cls.i
+}
+
+declare i32 @llvm.aarch64.cls(i32) nounwind
+declare i32 @llvm.aarch64.cls64(i64) nounwind
Index: llvm/lib/Target/ARM/ARMISelLowering.cpp
===
--- llvm/lib/Target/ARM/ARMISelLowering.cpp
+++ llvm/lib/Target/ARM/ARMISelLowering.cpp
@@ -3629,6 +3629,49 @@
 EVT PtrVT = getPointerTy(DAG.getDataLayout());
 return DAG.getNode(ARMISD::THREAD_POINTER, dl, PtrVT);
   }
+  case Intrinsic::arm_cls: {
+const SDValue &Operand = Op.getOperand(1);
+const EVT VTy = Op.getValueType();
+SDValue SRA =
+DAG.getNode(ISD::SRA, dl, VTy, Operand, DAG.getConstant(31, dl, VTy));
+SDValue XOR = DAG.getNode(ISD::XOR, dl, VTy, SRA, Operand);
+SDValue SHL =
+DAG.getNode(ISD::SHL, dl, VTy, XOR, DAG.getConstant(1, dl, VTy));
+SDValue OR =
+DAG.getNode(ISD::OR, dl, VTy, SHL, DAG.getConstant(1, dl, VTy));
+SDValue Result = DAG.getNode(ISD::CTLZ, dl, VTy, OR);
+return Result;
+  }
+  case Intrinsic::arm_cls64: {
+// cls(x) = if cls(hi(x)) != 31 then cls(hi(x))
+//  else 31 + clz(if hi(x) == 0 then lo(x) else not(lo(x)))
+const SDValue &Operand = Op.getOperand(1);
+const EVT VTy = Op.getValueType();
+
+SDValue Hi = DAG.getNode(ISD::EXTRACT_ELEMENT, dl, VTy, Operand,
+ DAG.getConstant(1, dl, VTy));
+SDValue Lo = DAG.getNode(ISD::EXTRACT_ELEMENT, dl, VTy, Operand,
+ DAG.getConstant(0, dl, VTy));
+SDValue Constant0 = DAG.getConstant(0, dl, VTy);
+SDValue Constant1 = DAG.getConstant(1, dl, VTy);
+SDValue Constant31 = DAG.getConstant(31, dl, VTy);
+SDValue SRAHi = DAG.getNode(ISD::SRA, dl, VTy, Hi, Constant31);
+SDValue XORHi = DAG.getNode(ISD::XOR, dl, VTy, SRAHi, Hi);
+SDValue SHLHi = DAG.getNode(ISD::SHL, dl, VTy, XORHi, Constant1);
+SDValue ORHi = DAG.getNode(ISD::OR, dl, VTy, SHLHi, Constant1);
+SDValue CLSHi = DAG.getNode(ISD::CTLZ, dl, VTy, ORHi);
+SDValue CheckLo =
+DAG.getSetCC(dl, MVT::i1, CLSHi, Constant31, ISD::CondCode::SETEQ);
+SDValue HiIsZero =
+DAG.getSetCC(dl, MVT::i1, Hi, Constant0, ISD::CondCode::SETEQ);
+SDValue AdjustedLo =
+DAG.getSelect(dl, VTy, HiIsZero, Lo, DAG.getNOT(dl, Lo, VTy));
+SDValue CLZAdjustedLo = DAG.getNode(ISD::CTLZ, dl, VTy, AdjustedLo);
+SDValue Result =
+DAG.getSelect(dl, VTy, CheckLo,
+  DAG.getNode(ISD::ADD, dl, VTy, CLZAdjustedLo, Constant31), CLSHi);
+return Result;
+  }
   case Intrinsic::eh_sjlj_lsda: {
 MachineFunction &MF = DAG.getMachineFunction();
 ARM

[PATCH] D69433: [clang-format] [NFC] update the documentation in Format.h to allow dump_format_style.py to get a little closer to being correct. (part 2)

[MyDeveloperDay via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Format/Format.h:1971
+  /// be affected.
   /// \code
   ///true:  false:

my assumption here was that the text as was in the .rst was correct, hence why 
I'm reflecting that text back into Format.h 



Comment at: clang/include/clang/Format/Format.h:1986
   /// The historical aliases ``Cpp03`` and ``Cpp11`` are deprecated.
   enum LanguageStandard {
+/// Use C++03-compatible syntax.

Again focus is on the rst being the desired text and getting them to be 
aligned. @sammccall  please correct me if my assumption is incorrect


Repository:
  rC Clang

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

https://reviews.llvm.org/D69433



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

[PATCH] D69433: [clang-format] [NFC] update the documentation in Format.h to allow dump_format_style.py to get a little closer to being correct. (part 2)

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

For help for the reviewers...here is the before and after

Before:
F10399858: image.png 

After:
F10399852: image.png 


Repository:
  rC Clang

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

https://reviews.llvm.org/D69433



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

[PATCH] D69250: [ARM][AArch64] Implement __cls, __clsl and __clsll intrinsics from ACLE

[Victor Campos via Phabricator via cfe-commits]

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

Added support for `__clsll` as requested.




Comment at: clang/lib/Headers/arm_acle.h:150
+__clsl(unsigned long __t) {
+#if __SIZEOF_LONG__ == 4
+  return __builtin_arm_cls(__t);

compnerd wrote:
> vhscampos wrote:
> > compnerd wrote:
> > > vhscampos wrote:
> > > > compnerd wrote:
> > > > > I don't see a pattern match for the `cls64` on ARM32, would that not 
> > > > > fail to lower?
> > > > Yes. However, for now, I am not enabling support for `cls64` on ARM32 
> > > > as it is not done yet.
> > > Is the difference not just the parameter type?  I think that implementing 
> > > it should be a trivial change to the existing implementation.  Is there a 
> > > reason that you are not implementing that?
> > At clang's side, yes, but not in the backend: Arm32 does not have a `cls` 
> > instruction, thus the CLS operations need to be custom lowered. In the 
> > `llvm.arm.cls(i32)` case, lowering is quite simple, and it's been included 
> > in this patch. For `llvm.arm.cls64(i64)`, on the other hand, it is not as 
> > trivial since it's necessary to break its logic into 32-bit instructions.
> > 
> > So the reason not to implement that (yet) is just to split work in two 
> > different efforts.
> Would it not be sufficient to do the top half (after a shift right of 
> 32-bits), and if it is exactly 32, then do the bottom 32-bits, otherwise, 
> you're done?
Sort of. How we interpret the bottom half depends on the value of the top half. 
I've added this custom lowering in the latest revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69250



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

[PATCH] D68296: [clang-format] Add ability to wrap braces after multi-line control statements

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: cfe/trunk/docs/ClangFormatStyleOptions.rst:812
+  Always wrap braces after a control statement.
+
+  .. code-block:: c++

This text cannot be generated by dump_format_style.py in clang/docs/tools from 
the Format.h

I think this is the first time we've seen an enum being used in a structure, as 
such dump_format_style doesn't support pulling the text in automatically.

The next person who runs dump_format_style could easily end up blowing away 
your documentation.




Repository:
  rL LLVM

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

https://reviews.llvm.org/D68296



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

[PATCH] D69433: [clang-format] [NFC] update the documentation in Format.h to allow dump_format_style.py to get a little closer to being correct. (part 2)

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added inline comments.



Comment at: clang/docs/tools/dump_format_style.py:175
+val = line.replace(',', '')
+pos = val.find(" // ")
+if (pos != -1):

This seems quite flimsy to me, as it depends on an undocumented comment style. 
It is true that if the file(s) in question are properly clang-formatted, then 
this would probably not fail, but it does not appear to be a very robust 
solution.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69433



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

[PATCH] D69250: [ARM][AArch64] Implement __cls, __clsl and __clsll intrinsics from ACLE

[Saleem Abdulrasool via Phabricator via cfe-commits]

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

Thanks for all the adjustments, this looks good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69250



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

[PATCH] D69022: [coroutines] Remove assert on CoroutineParameterMoves in Sema::buildCoroutineParameterMoves

[JunMa via Phabricator via cfe-commits]

junparser added a subscriber: rjmccall.
junparser added a comment.

In D69022#1720645 , @rjmccall wrote:

> Despite generally knowing about coroutines and generally knowing about Clang, 
> I actually don't know the Clang coroutine code and can't review this properly.


thanks  anyway~


Repository:
  rC Clang

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

https://reviews.llvm.org/D69022



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

[PATCH] D65433: [clangd] DefineInline action availability checks

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

The test fails in Windows: http://45.33.8.238/win/1112/step_7.txt

Ptal, and if it takes a while to investigate please revert while you look.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65433



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

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev created this revision.
AntonBikineev added a reviewer: klimek.
AntonBikineev added projects: clang, clang-tools-extra.
Herald added subscribers: mgehre, xazax.hun, mgorny.

Checks for types which can be made trivially-destructible by removing
out-of-line defaulted destructor declarations.

The check is motivated by the work on C++ garbage collector in Blink
(rendering engine in Chrome), which strives to minimize destructor usage
and improve runtime of sweeping phase.

In the entire chromium codebase the check hits over ~2500 times.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D69435

Files:
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp
  clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/performance-trivially-destructible.rst
  
clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp
@@ -0,0 +1,68 @@
+// RUN: %check_clang_tidy %s performance-trivially-destructible %t
+
+struct TriviallyDestructible1 {
+  int a;
+};
+
+struct TriviallyDestructible2 : TriviallyDestructible1 {
+  TriviallyDestructible1 b;
+};
+
+struct NonTriviallyDestructible1 : TriviallyDestructible2 {
+  ~NonTriviallyDestructible1(); // to-be-removed
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: destructor '~NonTriviallyDestructible1' can be removed to make the class trivially destructible [performance-trivially-destructible]
+  // CHECK-FIXES: {{^}}  // to-be-removed
+  TriviallyDestructible2 b;
+};
+
+NonTriviallyDestructible1::~NonTriviallyDestructible1() = default; // to-be-removed
+// CHECK-MESSAGES: :[[@LINE-1]]:28: note: destructor definition is here
+// CHECK-FIXES: {{^}}// to-be-removed
+
+// Don't emit for class template with type-dependent fields.
+template 
+struct MaybeTriviallyDestructible1 {
+  ~MaybeTriviallyDestructible1() noexcept;
+  T t;
+};
+
+template 
+MaybeTriviallyDestructible1::~MaybeTriviallyDestructible1() noexcept = default;
+
+// Don't emit for specializations.
+template struct MaybeTriviallyDestructible1;
+
+// Don't emit for class template with type-dependent bases.
+template 
+struct MaybeTriviallyDestructible2 : T {
+  ~MaybeTriviallyDestructible2() noexcept;
+};
+
+template 
+MaybeTriviallyDestructible2::~MaybeTriviallyDestructible2() noexcept = default;
+
+// Emit for templates without dependent bases and fields.
+template 
+struct MaybeTriviallyDestructible1 {
+  ~MaybeTriviallyDestructible1() noexcept; // to-be-removed
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: destructor '~MaybeTriviallyDestructible1' can be removed to make the class trivially destructible [performance-trivially-destructible]
+  // CHECK-FIXES: {{^}}  // to-be-removed
+  TriviallyDestructible1 t;
+};
+
+template 
+MaybeTriviallyDestructible1::~MaybeTriviallyDestructible1() noexcept = default; // to-be-removed
+// CHECK-MESSAGES: :[[@LINE-1]]:35: note: destructor definition is here
+// CHECK-FIXES: {{^}}// to-be-removed
+
+// Emit for explicit specializations.
+template <>
+struct MaybeTriviallyDestructible1: TriviallyDestructible1 {
+  ~MaybeTriviallyDestructible1() noexcept; // to-be-removed
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: destructor '~MaybeTriviallyDestructible1' can be removed to make the class trivially destructible [performance-trivially-destructible]
+  // CHECK-FIXES: {{^}}  // to-be-removed
+};
+
+MaybeTriviallyDestructible1::~MaybeTriviallyDestructible1() noexcept = default; // to-be-removed
+// CHECK-MESSAGES: :[[@LINE-1]]:38: note: destructor definition is here
+// CHECK-FIXES: {{^}}// to-be-removed
Index: clang-tools-extra/docs/clang-tidy/checks/performance-trivially-destructible.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/performance-trivially-destructible.rst
@@ -0,0 +1,15 @@
+.. title:: clang-tidy - performance-trivially-destructible
+
+performance-trivially-destructible
+==
+
+Finds types that could be made trivially-destrictuble by removing out-of-line
+defaulted destructor declarations.
+
+.. code-block:: c++
+
+   struct A: TrivialType {
+ ~A(); // Makes A non-trivially-destructible.
+ TrivialType trivial_fields;
+   };
+   A::~A() = default;
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tid

[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:4463-4890
+
   if (AStmt && !CurContext->isDependentContext()) {
 assert(isa(AStmt) && "Captured statement expected");
 
 // Check default data sharing attributes for referenced variables.
 DSAAttrChecker DSAChecker(DSAStack, *this, cast(AStmt));
+

ABataev wrote:
> The formatting changes better to put in a separate NFC patch if you think 
> that this is required.
I don't really understand why adding a specific diagnostic message for 
defaultmap(none: x) is just a formatting change.  For defaultmap(none: x), we 
emit error message if there are no explicity specified data-sharing attributes, 
data mapping attributes, or is_device_ptr clause, while for default(none), we 
emit error message if there are no explicitly specified data-sharing 
attributes. So the message for defaultmap(none: x) should be different from 
default(none).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204



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

[PATCH] D69433: [clang-format] [NFC] update the documentation in Format.h to allow dump_format_style.py to get a little closer to being correct. (part 2)

[MyDeveloperDay via Phabricator via cfe-commits]

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



Comment at: clang/docs/tools/dump_format_style.py:175
+val = line.replace(',', '')
+pos = val.find(" // ")
+if (pos != -1):

mitchell-stellar wrote:
> This seems quite flimsy to me, as it depends on an undocumented comment 
> style. It is true that if the file(s) in question are properly 
> clang-formatted, then this would probably not fail, but it does not appear to 
> be a very robust solution.
I'd tend to agree, but this whole dump_format_style.py is flimsy.. take a look 
at this review {D31574} 

When you added this line, you forgot the third /

```// Different ways to wrap braces after control statements.```

Also, the extra empty line in the LanguageStandard both caused the whole python 
file to fail with an exception.

Do you have a suggestion for something better? (which doesn't leave the 
Format.h looking too odd)


Repository:
  rC Clang

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

https://reviews.llvm.org/D69433



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

[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

[Chi Chun Chen via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaOpenMP.cpp:4463-4890
+
   if (AStmt && !CurContext->isDependentContext()) {
 assert(isa(AStmt) && "Captured statement expected");
 
 // Check default data sharing attributes for referenced variables.
 DSAAttrChecker DSAChecker(DSAStack, *this, cast(AStmt));
+

cchen wrote:
> ABataev wrote:
> > The formatting changes better to put in a separate NFC patch if you think 
> > that this is required.
> I don't really understand why adding a specific diagnostic message for 
> defaultmap(none: x) is just a formatting change.  For defaultmap(none: x), we 
> emit error message if there are no explicity specified data-sharing 
> attributes, data mapping attributes, or is_device_ptr clause, while for 
> default(none), we emit error message if there are no explicitly specified 
> data-sharing attributes. So the message for defaultmap(none: x) should be 
> different from default(none).
Oh, my bad, you mean this new line. I'll delete it. Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204



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

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp:22
+
+bool CheckPotentiallyTriviallyDestructible(const CXXDestructorDecl *Dtor) {
+  if (Dtor->isFirstDecl() || !Dtor->isExplicitlyDefaulted())

I believe static functions are preferred to anon namespaces;
most (if not all) of this checking should/could be done in the matcher itself,
i.e. the check() could be called to unconditionally issue diag.



Comment at: 
clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp:50
+void TriviallyDestructibleCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(cxxDestructorDecl().bind("decl"), this);
+}

https://en.cppreference.com/w/cpp/language/destructor says defaulted destructor 
is C++11 feature,
so not register if this isn't C++11?



Comment at: 
clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp:77
+  << FirstDecl << FixItHint::CreateRemoval(FirstDeclRange)
+  << FixItHint::CreateRemoval(SecondDeclRange);
+  diag(MatchedDecl->getLocation(), "destructor definition is here",

This always drops out-of-line defaulting.
Is there/can there be any case where such defaultig should just be moved 
in-line into the class?




Comment at: 
clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.h:18-19
+
+/// A check that finds out-of-line declared defaulted destructors, which would
+/// otherwise be trivial:
+/// struct A: TrivialClass {

This doesn't read right, should be closer to this maybe
```
/// A check that finds classes that would be trivial if not for the defaulted 
destructors declared out-of-line:
```



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:136
+
+  Finds types that could be made trivially-destrictuble by removing out-of-line
+  defaulted destructor declarations.

s/destrictuble/destructible/?



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-trivially-destructible.rst:6
+
+Finds types that could be made trivially-destrictuble by removing out-of-line
+defaulted destructor declarations.

s/destrictuble/destructible/?



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp:1
+// RUN: %check_clang_tidy %s performance-trivially-destructible %t
+

Do you also want to check that fix-it applies and result passes sema?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D69435



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

[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:4463-4890
+
   if (AStmt && !CurContext->isDependentContext()) {
 assert(isa(AStmt) && "Captured statement expected");
 
 // Check default data sharing attributes for referenced variables.
 DSAAttrChecker DSAChecker(DSAStack, *this, cast(AStmt));
+

cchen wrote:
> cchen wrote:
> > ABataev wrote:
> > > The formatting changes better to put in a separate NFC patch if you think 
> > > that this is required.
> > I don't really understand why adding a specific diagnostic message for 
> > defaultmap(none: x) is just a formatting change.  For defaultmap(none: x), 
> > we emit error message if there are no explicity specified data-sharing 
> > attributes, data mapping attributes, or is_device_ptr clause, while for 
> > default(none), we emit error message if there are no explicitly specified 
> > data-sharing attributes. So the message for defaultmap(none: x) should be 
> > different from default(none).
> Oh, my bad, you mean this new line. I'll delete it. Thanks
Yes, I was talking about new empty lines. Do not add them here and other places.



Comment at: 
clang/test/OpenMP/target_teams_distribute_simd_defaultmap_messages.cpp:55
   for (i = 0; i < argc; ++i) foo();
-#pragma omp target teams distribute simd defaultmap (tofrom, scalar // 
expected-error {{expected ')'}} expected-warning {{missing ':' after defaultmap 
modifier - ignoring}} expected-error {{expected 'scalar' in OpenMP clause 
'defaultmap'}} expected-note {{to match this '('}}
+#pragma omp target teams distribute simd defaultmap (tofrom, scalar // 
expected-error {{expected ')'}} expected-warning {{missing ':' after defaultmap 
modifier - ignoring}} expected-error {{expected ['scalar', 'aggregate', 
'pointer'] in OpenMP clause 'defaultmap'}} expected-note {{to match this '('}}
   for (i = 0; i < argc; ++i) foo();

We never used `[]` in messages, please remove them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204



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

[clang-tools-extra] 8e567b0 - [clangd] Revert define-inline action changes to un-break windows build-bots

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2019-10-25T18:40:01+02:00
New Revision: 8e567b0730fa55d15e6c0ab20b0352d85e96b7bb

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

LOG: [clangd] Revert define-inline action changes to un-break windows build-bots

Added: 


Modified: 
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/refactor/Tweak.cpp
clang-tools-extra/clangd/refactor/Tweak.h
clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
clang-tools-extra/clangd/unittests/TweakTesting.cpp
clang-tools-extra/clangd/unittests/TweakTesting.h
clang-tools-extra/clangd/unittests/TweakTests.cpp

Removed: 
clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp



diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index 64ccbe417c24..4f1fe8f5b08b 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -367,7 +367,7 @@ tweakSelection(const Range &Sel, const InputsAndAST &AST) {
   auto End = positionToOffset(AST.Inputs.Contents, Sel.end);
   if (!End)
 return End.takeError();
-  return Tweak::Selection(AST.Inputs.Index, AST.AST, *Begin, *End);
+  return Tweak::Selection(AST.AST, *Begin, *End);
 }
 
 void ClangdServer::enumerateTweaks(PathRef File, Range Sel,

diff  --git a/clang-tools-extra/clangd/refactor/Tweak.cpp 
b/clang-tools-extra/clangd/refactor/Tweak.cpp
index 2dc091ed762a..4f3c40d1eb13 100644
--- a/clang-tools-extra/clangd/refactor/Tweak.cpp
+++ b/clang-tools-extra/clangd/refactor/Tweak.cpp
@@ -9,7 +9,6 @@
 #include "Logger.h"
 #include "Path.h"
 #include "SourceCode.h"
-#include "index/Index.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
@@ -45,10 +44,9 @@ void validateRegistry() {
 }
 } // namespace
 
-Tweak::Selection::Selection(const SymbolIndex *Index, ParsedAST &AST,
-unsigned RangeBegin, unsigned RangeEnd)
-: Index(Index), AST(AST), SelectionBegin(RangeBegin),
-  SelectionEnd(RangeEnd),
+Tweak::Selection::Selection(ParsedAST &AST, unsigned RangeBegin,
+unsigned RangeEnd)
+: AST(AST), SelectionBegin(RangeBegin), SelectionEnd(RangeEnd),
   ASTSelection(AST.getASTContext(), AST.getTokens(), RangeBegin, RangeEnd) 
{
   auto &SM = AST.getSourceManager();
   Code = SM.getBufferData(SM.getMainFileID());

diff  --git a/clang-tools-extra/clangd/refactor/Tweak.h 
b/clang-tools-extra/clangd/refactor/Tweak.h
index de655abd98c7..42b7fb0684e8 100644
--- a/clang-tools-extra/clangd/refactor/Tweak.h
+++ b/clang-tools-extra/clangd/refactor/Tweak.h
@@ -24,7 +24,6 @@
 #include "Protocol.h"
 #include "Selection.h"
 #include "SourceCode.h"
-#include "index/Index.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringMap.h"
@@ -47,12 +46,9 @@ class Tweak {
 public:
   /// Input to prepare and apply tweaks.
   struct Selection {
-Selection(const SymbolIndex *Index, ParsedAST &AST, unsigned RangeBegin,
-  unsigned RangeEnd);
+Selection(ParsedAST &AST, unsigned RangeBegin, unsigned RangeEnd);
 /// The text of the active document.
 llvm::StringRef Code;
-/// The Index for handling codebase related queries.
-const SymbolIndex *Index = nullptr;
 /// Parsed AST of the active file.
 ParsedAST *
 /// A location of the cursor in the editor.

diff  --git a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt 
b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
index ddf10a2ca2ba..f43f132304c9 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
+++ b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
@@ -14,7 +14,6 @@ set(LLVM_LINK_COMPONENTS
 add_clang_library(clangDaemonTweaks OBJECT
   AnnotateHighlightings.cpp
   DumpAST.cpp
-  DefineInline.cpp
   ExpandAutoType.cpp
   ExpandMacro.cpp
   ExtractFunction.cpp

diff  --git a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp 
b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
deleted file mode 100644
index 8785db1a292b..
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
+++ /dev/null
@@ -1,393 +0,0 @@
-//===--- DefineInline.cpp *- 
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 "AST.h"
-#include "FindTarget.h"
-#include "Logger.h"
-#include "Selection.h"
-#include "SourceCode.h"
-#include "XRefs.h"
-#include "refactor/Tweak.

[PATCH] D68953: Enable most VFS tests on Windows

[Adrian McCarthy via Phabricator via cfe-commits]

amccarth marked 2 inline comments as done.
amccarth added a comment.

Somehow Phabricator failed to notify me that you'd already left comments.  I 
even searched my inbox to see if I'd just missed the notification.  Nope.  
Nothing.  I came here today to ping you for the review and discovered you'd 
already done your part.

Anyway, I'll update the patch and ping again when that's done.

Thanks.




Comment at: clang/lib/Lex/HeaderSearch.cpp:782-783
   // Concatenate the requested file onto the directory.
-  // FIXME: Portability.  Filename concatenation should be in sys::Path.
-  TmpDir = IncluderAndDir.second->getName();
-  TmpDir.push_back('/');
-  TmpDir.append(Filename.begin(), Filename.end());
+  TmpPath = IncluderAndDir.second->getName();
+  llvm::sys::path::append(TmpPath, Filename);
 

rnk wrote:
> I'm surprised this just works. :) You ran the whole clang test suite, and 
> nothing broke, right? I would've expected something to accidentally depend on 
> this behavior.
Yeah, I recall it all working before, but, double-checking today, I'm seeing 
some collateral damage.  Investigating.



Comment at: clang/test/VFS/external-names.c:4-5
 
 // FIXME: PR43272
 // XFAIL: system-windows
 

rnk wrote:
> You made changes to this file, but it's still XFAILed. Is that intentional?
The change removes one obstacle, but the test still fails on Windows because 
there's still an outstanding bug.  If you like, I can hoist this change out 
until I have a patch to fix the outstanding issue.



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1839
   SmallVector Components;
-  Components.push_back("/");
+  Components.push_back(sys::path::get_separator());
   getVFSEntries(*RootE, Components, CollectedEntries);

rnk wrote:
> I think this will ultimately produce paths that look like: `\C:\foo\bar\baz`. 
> Ultimately, we loop over the components above and repeatedly call 
> sys::path::append on them.
> 
> Clang only uses this function for collecting module dependency info, it 
> seems. Maybe that's what the remaining failures are about?
I didn't encounter anything like `\C:\foo`.  Perhaps the (enabled) VFS tests 
all prime the roots so that never happens here.  I'll see if I can add a test 
to uncover that problem.

And, yes, use of this function seems limited to reading VFS paths from the 
YAML, so its scope is pretty limited.  At least some of the remaining problems 
have to do with the fact that, while the target paths can always be expressed 
in the host style, the key paths can be in either Posix or Windows format, 
which leads to confusion when you try to get answers from functions like 
`sys::path::is_absolute` where you have to pick the right style to get the 
right answer.


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

https://reviews.llvm.org/D68953



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

[clang] 7a2b704 - [Sema][Typo Correction] Fix another infinite loop on ambiguity

[David Goldman via cfe-commits]

Author: David Goldman
Date: 2019-10-25T13:20:27-04:00
New Revision: 7a2b704bf0cf65f9eb46fe3668a83b75aa2d80a6

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

LOG: [Sema][Typo Correction] Fix another infinite loop on ambiguity

See also: D67515

- For the given call expression we would end up repeatedly
   trying to transform the same expression over and over again

- Fix is to keep the old TransformCache when checking for ambiguity

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

Added: 
clang/test/Sema/typo-correction-ambiguity.c

Modified: 
clang/lib/Sema/SemaExprCXX.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 18efd8335d9d..4fdd15bf466f 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -7780,8 +7780,9 @@ class TransformTypos : public 
TreeTransform {
 
 // If we found a valid result, double check to make sure it's not 
ambiguous.
 if (!IsAmbiguous && !Res.isInvalid() && !AmbiguousTypoExprs.empty()) {
-  auto SavedTransformCache = std::move(TransformCache);
-  TransformCache.clear();
+  auto SavedTransformCache =
+  llvm::SmallDenseMap(TransformCache);
+
   // Ensure none of the TypoExprs have multiple typo correction candidates
   // with the same edit length that pass all the checks and filters.
   while (!AmbiguousTypoExprs.empty()) {

diff  --git a/clang/test/Sema/typo-correction-ambiguity.c 
b/clang/test/Sema/typo-correction-ambiguity.c
new file mode 100644
index ..bebbf25ce291
--- /dev/null
+++ b/clang/test/Sema/typo-correction-ambiguity.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// Check the following typo correction behavior in C:
+// - no typos are diagnosed when a call expression has ambiguous (multiple) 
corrections
+
+int v_63;
+
+void v_2_0(int v_452, int v_454) {}
+
+int v_3_0() {
+   for (int v_345 = 0 ; v_63;)
+   v_2_0(v_195,  // expected-error {{use of undeclared identifier 'v_195'}}
+ v_231);  // expected-error {{use of undeclared identifier 
'v_231'}}
+}



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

[PATCH] D69060: [Sema][Typo Correction] Fix another infinite loop on ambiguity

[David Goldman via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7a2b704bf0cf: [Sema][Typo Correction] Fix another infinite 
loop on ambiguity (authored by dgoldman).

Changed prior to commit:
  https://reviews.llvm.org/D69060?vs=225272&id=226455#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69060

Files:
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/Sema/typo-correction-ambiguity.c


Index: clang/test/Sema/typo-correction-ambiguity.c
===
--- /dev/null
+++ clang/test/Sema/typo-correction-ambiguity.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// Check the following typo correction behavior in C:
+// - no typos are diagnosed when a call expression has ambiguous (multiple) 
corrections
+
+int v_63;
+
+void v_2_0(int v_452, int v_454) {}
+
+int v_3_0() {
+   for (int v_345 = 0 ; v_63;)
+   v_2_0(v_195,  // expected-error {{use of undeclared identifier 'v_195'}}
+ v_231);  // expected-error {{use of undeclared identifier 
'v_231'}}
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -7780,8 +7780,9 @@
 
 // If we found a valid result, double check to make sure it's not 
ambiguous.
 if (!IsAmbiguous && !Res.isInvalid() && !AmbiguousTypoExprs.empty()) {
-  auto SavedTransformCache = std::move(TransformCache);
-  TransformCache.clear();
+  auto SavedTransformCache =
+  llvm::SmallDenseMap(TransformCache);
+
   // Ensure none of the TypoExprs have multiple typo correction candidates
   // with the same edit length that pass all the checks and filters.
   while (!AmbiguousTypoExprs.empty()) {


Index: clang/test/Sema/typo-correction-ambiguity.c
===
--- /dev/null
+++ clang/test/Sema/typo-correction-ambiguity.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// Check the following typo correction behavior in C:
+// - no typos are diagnosed when a call expression has ambiguous (multiple) corrections
+
+int v_63;
+
+void v_2_0(int v_452, int v_454) {}
+
+int v_3_0() {
+   for (int v_345 = 0 ; v_63;)
+   v_2_0(v_195,  // expected-error {{use of undeclared identifier 'v_195'}}
+ v_231);  // expected-error {{use of undeclared identifier 'v_231'}}
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -7780,8 +7780,9 @@
 
 // If we found a valid result, double check to make sure it's not ambiguous.
 if (!IsAmbiguous && !Res.isInvalid() && !AmbiguousTypoExprs.empty()) {
-  auto SavedTransformCache = std::move(TransformCache);
-  TransformCache.clear();
+  auto SavedTransformCache =
+  llvm::SmallDenseMap(TransformCache);
+
   // Ensure none of the TypoExprs have multiple typo correction candidates
   // with the same edit length that pass all the checks and filters.
   while (!AmbiguousTypoExprs.empty()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp:29
+  // Check direct base classes.
+  const auto *RecordDecl = Dtor->getParent();
+  for (auto *Field : RecordDecl->fields()) {

Please don't use auto unless type is spelled in same statement or iterator.



Comment at: 
clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp:31
+  for (auto *Field : RecordDecl->fields()) {
+const auto FieldType = Field->getType();
+if (FieldType->isDependentType() ||

Please don't use auto unless type is spelled in same statement or iterator.



Comment at: 
clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp:38
+  for (const auto &BaseSpec : RecordDecl->bases()) {
+const auto BaseType = BaseSpec.getType();
+if (BaseType->isDependentType() ||

Please don't use auto unless type is spelled in same statement or iterator.



Comment at: 
clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp:61
+  const auto *FirstDecl = cast(MatchedDecl->getFirstDecl());
+  const auto FirstDeclRange = clang::CharSourceRange::getCharRange(
+  FirstDecl->getBeginLoc(),

Please don't use auto unless type is spelled in same statement or iterator.



Comment at: 
clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp:66
+  /*SkipTrailingWhitespaceAndNewLine=*/true));
+  const auto SecondDeclRange = clang::CharSourceRange::getTokenRange(
+  MatchedDecl->getBeginLoc(),

Please don't use auto unless type is spelled in same statement or iterator.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:133
 
+- New :doc:`performance-trivially-destructible
+  ` check.

Please move into new checks list (in alphabetical order).


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D69435



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

[PATCH] D69391: Add #pragma clang loop ivdep

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

This is the same as #pragma clang loop vectorize(assume_safety)?

In D69391#1720845 , @xbolva00 wrote:

> "#pragma ivdep" should work too (compatibiluty mode with intel, gcc).


The semantics are not the same, unfortunately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69391



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

[PATCH] D69420: [clang][clang-scan-deps] Add support for extracting full module dependencies.

[Kousik Kumar via Phabricator via cfe-commits]

kousikk added a comment.

A few nit comments




Comment at: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h:34
+/// The format that is output by the dependency scanner.
+enum class ScanningFormat {
+  /// This is the Makefile compatible dep format.

"ScanningOutputFormat" or "DependencyOutputFormat" ?



Comment at: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h:35
+enum class ScanningFormat {
+  /// This is the Makefile compatible dep format.
+  Make,

IIUC, this format would NOT include module dependencies right?

If so, can you explicitly state that in the comment?



Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp:94
+
+void printDependencies(std::string &S, StringRef MainFile) {
+  // Sort the modules by name to get a deterministic order.

Should output arguments be at the end?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69420



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

[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:137-140
+ SourceLocation Loc) {
+  ImplicitBehavior = IB;
+  VariableCategory = VC;
+  SLoc = Loc;

Use member initializers in constructors.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:152
 SourceLocation DefaultAttrLoc;
-DefaultMapAttributes DefaultMapAttr = DMA_unspecified;
-SourceLocation DefaultMapAttrLoc;
+std::array DefaultmapMap;
+

No, I meant just a C array, like `DefaultmapInfo DefaultMap[3];`.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:620-624
+  /// Set default data mapping attribute to 'alloc:scalar'.
+  void setDefaultDMAAllocScalar(SourceLocation Loc) {
+getTopOfStack().DefaultmapMap[OMPC_DEFAULTMAP_scalar]
+  .set(DMIB_alloc, DMVC_scalar, Loc);
+  }

Why do we need so many functions? Better to have just one function with 
additional params.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1967
   (DSAStack->isForceCaptureByReferenceInTargetExecutable() &&
!Ty->isAnyPointerType()) ||
   !Ty->isScalarType() ||

Seems to me, you're missing additional checks for pointers here.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1969-1972
+  (DSAStack->getDefaultDMIBAtLevel(Level, OMPC_DEFAULTMAP_scalar) ==
+   DMIB_alloc ||
+   DSAStack->getDefaultDMIBAtLevel(Level, OMPC_DEFAULTMAP_scalar) ==
+   DMIB_to ||

I think, alloc and to scalars also must be captured by value. Moreover, seems 
to me, alloc scalars should not be captured at all since their behavior is very 
similar to the behavior of private variables.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1978
+   DSAStack->getDefaultDMIBAtLevel(Level, OMPC_DEFAULTMAP_scalar) ==
+   DMIB_firstprivate) ||
   DSAStack->hasExplicitDSA(

If the scalars are mapped as firstprivates by default, they must be captured by 
value.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:2200-2201
+  DMIB_tofrom &&
+  DSAStack->getDefaultDMIBAtLevel(NewLevel, OMPC_DEFAULTMAP_scalar) !=
+  DMIB_firstprivate) &&
+  !(D->getType()->isPointerType() && // TODO check

Seems to me, this is wrong. If the default is firstprivate the variables must 
be marked as firstprivate. Maybe, just check if the default for scalars is not 
specified or is firstprivate instead of so many checks?



Comment at: clang/lib/Sema/SemaOpenMP.cpp:2202-2212
+  !(D->getType()->isPointerType() && // TODO check
+  (DSAStack->getDefaultDMIBAtLevel(NewLevel, OMPC_DEFAULTMAP_pointer) 
==
+  DMIB_alloc ||
+  DSAStack->getDefaultDMIBAtLevel(NewLevel, OMPC_DEFAULTMAP_pointer) ==
+  DMIB_to ||
+  DSAStack->getDefaultDMIBAtLevel(NewLevel, OMPC_DEFAULTMAP_pointer) ==
+  DMIB_from ||

1. Also, check the logic here. 
2. Why there is TODO?



Comment at: clang/lib/Sema/SemaOpenMP.cpp:3059-3082
+  (((VD->getType().getNonReferenceType()->isScalarType() &&
+   Stack->getDefaultDMIB(OMPC_DEFAULTMAP_scalar) !=
+ DMIB_alloc &&
+   Stack->getDefaultDMIB(OMPC_DEFAULTMAP_scalar) !=
+ DMIB_to &&
+   Stack->getDefaultDMIB(OMPC_DEFAULTMAP_scalar) !=
+ DMIB_from &&

I see similar cehcks in different places, Outline them into a standalone 
function.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:4466-4472
+
   if (AStmt && !CurContext->isDependentContext()) {
 assert(isa(AStmt) && "Captured statement expected");
 
 // Check default data sharing attributes for referenced variables.
 DSAAttrChecker DSAChecker(DSAStack, *this, cast(AStmt));
+

Extra blank lines



Comment at: clang/lib/Sema/TreeTransform.h:1987-2001
+  /// Build a new OpenMP 'defaultmap' clause.
+  ///
+  /// By default, performs semantic analysis to build the new OpenMP clause.
+  /// Subclasses may override this routine to provide different behavior.
+  OMPClause *RebuildOMPDefaultmapClause(OpenMPDefaultmapClauseModifier M,
+OpenMPDefaultmapClauseKind Kind,
+SourceLocation StartLoc,

Do you really need to rebuild it? It has only constants inside.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204



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

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Anton Bikineev via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp:22
+
+bool CheckPotentiallyTriviallyDestructible(const CXXDestructorDecl *Dtor) {
+  if (Dtor->isFirstDecl() || !Dtor->isExplicitlyDefaulted())

lebedev.ri wrote:
> I believe static functions are preferred to anon namespaces;
> most (if not all) of this checking should/could be done in the matcher itself,
> i.e. the check() could be called to unconditionally issue diag.
I couldn't find matchers to check triviallity of special functions or something 
like isFirstDecl(). Perhaps, would make sense to add some.



Comment at: 
clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp:50
+void TriviallyDestructibleCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(cxxDestructorDecl().bind("decl"), this);
+}

lebedev.ri wrote:
> https://en.cppreference.com/w/cpp/language/destructor says defaulted 
> destructor is C++11 feature,
> so not register if this isn't C++11?
Right. However, I think it will also make sense to extend the check for 
destructors with empty bodies.



Comment at: 
clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp:77
+  << FirstDecl << FixItHint::CreateRemoval(FirstDeclRange)
+  << FixItHint::CreateRemoval(SecondDeclRange);
+  diag(MatchedDecl->getLocation(), "destructor definition is here",

lebedev.ri wrote:
> This always drops out-of-line defaulting.
> Is there/can there be any case where such defaultig should just be moved 
> in-line into the class?
> 
I've thought about this. I think it mostly comes down to code-style preference 
(rule-of-5 vs rule-of-0). But after giving it another thought, I think, the 
current implementation may lead to more surprises for the user, so probably 
moving //= default// to the first declaration would be a better idea :)



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp:1
+// RUN: %check_clang_tidy %s performance-trivially-destructible %t
+

lebedev.ri wrote:
> Do you also want to check that fix-it applies and result passes sema?
Aren't fix-its already checked by CHECK-FIXES?
It would be great to check if the result is compilable. Should I run another 
clang_tidy instance to check that or is there another way to do so?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D69435



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

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev updated this revision to Diff 226475.
AntonBikineev marked 2 inline comments as done.
AntonBikineev added a comment.

Thanks for the suggestions! Addressed some of them.


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

https://reviews.llvm.org/D69435

Files:
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp
  clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/performance-trivially-destructible.rst
  
clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp
@@ -0,0 +1,80 @@
+// RUN: %check_clang_tidy %s performance-trivially-destructible %t
+
+struct TriviallyDestructible1 {
+  int a;
+};
+
+struct TriviallyDestructible2 : TriviallyDestructible1 {
+  TriviallyDestructible1 b;
+};
+
+struct NotTriviallyDestructible1 : TriviallyDestructible2 {
+  ~NotTriviallyDestructible1();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'NotTriviallyDestructible1' can be made trivially destructible by defaulting the destructor on it first declaration [performance-trivially-destructible]
+  // CHECK-FIXES: ~NotTriviallyDestructible1() = default;
+  TriviallyDestructible2 b;
+};
+
+NotTriviallyDestructible1::~NotTriviallyDestructible1() = default; // to-be-removed
+// CHECK-MESSAGES: :[[@LINE-1]]:28: note: destructor definition is here
+// CHECK-FIXES: {{^}}// to-be-removed
+
+// Don't emit for class template with type-dependent fields.
+template 
+struct MaybeTriviallyDestructible1 {
+  ~MaybeTriviallyDestructible1() noexcept;
+  T t;
+};
+
+template 
+MaybeTriviallyDestructible1::~MaybeTriviallyDestructible1() noexcept = default;
+
+// Don't emit for specializations.
+template struct MaybeTriviallyDestructible1;
+
+// Don't emit for class template with type-dependent bases.
+template 
+struct MaybeTriviallyDestructible2 : T {
+  ~MaybeTriviallyDestructible2() noexcept;
+};
+
+template 
+MaybeTriviallyDestructible2::~MaybeTriviallyDestructible2() noexcept = default;
+
+// Emit for templates without dependent bases and fields.
+template 
+struct MaybeTriviallyDestructible1 {
+  ~MaybeTriviallyDestructible1() noexcept;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'MaybeTriviallyDestructible1' can be made trivially destructible by defaulting the destructor on it first declaration [performance-trivially-destructible]
+  // CHECK-FIXES: ~MaybeTriviallyDestructible1() noexcept = default;
+  TriviallyDestructible1 t;
+};
+
+template 
+MaybeTriviallyDestructible1::~MaybeTriviallyDestructible1() noexcept = default; // to-be-removed
+// CHECK-MESSAGES: :[[@LINE-1]]:35: note: destructor definition is here
+// CHECK-FIXES: {{^}}// to-be-removed
+
+// Emit for explicit specializations.
+template <>
+struct MaybeTriviallyDestructible1: TriviallyDestructible1 {
+  ~MaybeTriviallyDestructible1() noexcept;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'MaybeTriviallyDestructible1' can be made trivially destructible by defaulting the destructor on it first declaration [performance-trivially-destructible]
+  // CHECK-FIXES: ~MaybeTriviallyDestructible1() noexcept = default;
+};
+
+MaybeTriviallyDestructible1::~MaybeTriviallyDestructible1() noexcept = default; // to-be-removed
+// CHECK-MESSAGES: :[[@LINE-1]]:38: note: destructor definition is here
+// CHECK-FIXES: {{^}}// to-be-removed
+
+struct NotTriviallyDestructible2 {
+  virtual ~NotTriviallyDestructible2();
+};
+
+NotTriviallyDestructible2::~NotTriviallyDestructible2() = default;
+
+struct NotTriviallyDestructible3: NotTriviallyDestructible2 {
+  ~NotTriviallyDestructible3();
+};
+
+NotTriviallyDestructible3::~NotTriviallyDestructible3() = default;
Index: clang-tools-extra/docs/clang-tidy/checks/performance-trivially-destructible.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/performance-trivially-destructible.rst
@@ -0,0 +1,15 @@
+.. title:: clang-tidy - performance-trivially-destructible
+
+performance-trivially-destructible
+==
+
+Finds types that could be made trivially-destructible by removing out-of-line
+defaulted destructor declarations.
+
+.. code-block:: c++
+
+   struct A: TrivialType {
+ ~A(); // Makes A non-trivially-destructible.
+ TrivialType trivial_fields;
+   };
+   A::~A() = default;
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clan

[PATCH] D69433: [clang-format] [NFC] update the documentation in Format.h to allow dump_format_style.py to get a little closer to being correct. (part 2)

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added inline comments.



Comment at: clang/docs/tools/dump_format_style.py:175
+val = line.replace(',', '')
+pos = val.find(" // ")
+if (pos != -1):

MyDeveloperDay wrote:
> mitchell-stellar wrote:
> > This seems quite flimsy to me, as it depends on an undocumented comment 
> > style. It is true that if the file(s) in question are properly 
> > clang-formatted, then this would probably not fail, but it does not appear 
> > to be a very robust solution.
> I'd tend to agree, but this whole dump_format_style.py is flimsy.. take a 
> look at this review {D31574} 
> 
> When you added this line, you forgot the third /
> 
> ```// Different ways to wrap braces after control statements.```
> 
> Also, the extra empty line in the LanguageStandard both caused the whole 
> python file to fail with an exception.
> 
> Do you have a suggestion for something better? (which doesn't leave the 
> Format.h looking too odd)
I would go back to the `/// c++03: Parse and format as C++03.` style. `///` is 
a Doxygen comment, and I think documentation should be generated solely from 
Doxygen comments, even if it requires a bit of post-processing. (The extra `/` 
needed after `//` in the ticket you mentioned is justified.)


Repository:
  rC Clang

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

https://reviews.llvm.org/D69433



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

[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

[Chi Chun Chen via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/TreeTransform.h:1987-2001
+  /// Build a new OpenMP 'defaultmap' clause.
+  ///
+  /// By default, performs semantic analysis to build the new OpenMP clause.
+  /// Subclasses may override this routine to provide different behavior.
+  OMPClause *RebuildOMPDefaultmapClause(OpenMPDefaultmapClauseModifier M,
+OpenMPDefaultmapClauseKind Kind,
+SourceLocation StartLoc,

ABataev wrote:
> Do you really need to rebuild it? It has only constants inside.
I think you are right, the rebuild code for defaultmap seems redundant since it 
has only constant inside the function. I added this code so that my 
defaultmap(none) check inside DSAAttrChecker could be called for the second 
time (after template initialization). Without adding this code, the check in 
DSAAttrChecker for defaultmap(none) will never be called due to the guard in 
VisitDeclRefExpr:
```
if (E->isTypeDependent() || E->isValueDependent() ||
E->containsUnexpandedParameterPack() || E->isInstantiationDependent())
```
Where do you think I should put my check for defaultmap(none) so that I don't 
need this redundant tree rebuild code? Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204



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

[PATCH] D69433: [clang-format] [NFC] update the documentation in Format.h to allow dump_format_style.py to get a little closer to being correct. (part 2)

[MyDeveloperDay via Phabricator via cfe-commits]

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



Comment at: clang/docs/tools/dump_format_style.py:175
+val = line.replace(',', '')
+pos = val.find(" // ")
+if (pos != -1):

mitchell-stellar wrote:
> MyDeveloperDay wrote:
> > mitchell-stellar wrote:
> > > This seems quite flimsy to me, as it depends on an undocumented comment 
> > > style. It is true that if the file(s) in question are properly 
> > > clang-formatted, then this would probably not fail, but it does not 
> > > appear to be a very robust solution.
> > I'd tend to agree, but this whole dump_format_style.py is flimsy.. take a 
> > look at this review {D31574} 
> > 
> > When you added this line, you forgot the third /
> > 
> > ```// Different ways to wrap braces after control statements.```
> > 
> > Also, the extra empty line in the LanguageStandard both caused the whole 
> > python file to fail with an exception.
> > 
> > Do you have a suggestion for something better? (which doesn't leave the 
> > Format.h looking too odd)
> I would go back to the `/// c++03: Parse and format as C++03.` style. `///` 
> is a Doxygen comment, and I think documentation should be generated solely 
> from Doxygen comments, even if it requires a bit of post-processing. (The 
> extra `/` needed after `//` in the ticket you mentioned is justified.)
The Doxygen documentation is used for source-level documentation, this is 
user-level documentation which the restructured text output .rst is used.

In the past the ClangFormatStyleOpions.rst has been generated from the Format.h 
via this script, we should break that.

The "In configuation" part is super important because it explains to user what 
to put into their .clang-format file.

We have to either have some form of markup that says `LS_Cpp03 == c++03` in the 
documentation


Repository:
  rC Clang

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

https://reviews.llvm.org/D69433



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

[PATCH] D69433: [clang-format] [NFC] update the documentation in Format.h to allow dump_format_style.py to get a little closer to being correct. (part 2)

[MyDeveloperDay via Phabricator via cfe-commits]

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



Comment at: clang/docs/tools/dump_format_style.py:175
+val = line.replace(',', '')
+pos = val.find(" // ")
+if (pos != -1):

MyDeveloperDay wrote:
> mitchell-stellar wrote:
> > MyDeveloperDay wrote:
> > > mitchell-stellar wrote:
> > > > This seems quite flimsy to me, as it depends on an undocumented comment 
> > > > style. It is true that if the file(s) in question are properly 
> > > > clang-formatted, then this would probably not fail, but it does not 
> > > > appear to be a very robust solution.
> > > I'd tend to agree, but this whole dump_format_style.py is flimsy.. take a 
> > > look at this review {D31574} 
> > > 
> > > When you added this line, you forgot the third /
> > > 
> > > ```// Different ways to wrap braces after control statements.```
> > > 
> > > Also, the extra empty line in the LanguageStandard both caused the whole 
> > > python file to fail with an exception.
> > > 
> > > Do you have a suggestion for something better? (which doesn't leave the 
> > > Format.h looking too odd)
> > I would go back to the `/// c++03: Parse and format as C++03.` style. `///` 
> > is a Doxygen comment, and I think documentation should be generated solely 
> > from Doxygen comments, even if it requires a bit of post-processing. (The 
> > extra `/` needed after `//` in the ticket you mentioned is justified.)
> The Doxygen documentation is used for source-level documentation, this is 
> user-level documentation which the restructured text output .rst is used.
> 
> In the past the ClangFormatStyleOpions.rst has been generated from the 
> Format.h via this script, we should break that.
> 
> The "In configuation" part is super important because it explains to user 
> what to put into their .clang-format file.
> 
> We have to either have some form of markup that says `LS_Cpp03 == c++03` in 
> the documentation
*we shouldn't break that*


Repository:
  rC Clang

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

https://reviews.llvm.org/D69433



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

[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/TreeTransform.h:1987-2001
+  /// Build a new OpenMP 'defaultmap' clause.
+  ///
+  /// By default, performs semantic analysis to build the new OpenMP clause.
+  /// Subclasses may override this routine to provide different behavior.
+  OMPClause *RebuildOMPDefaultmapClause(OpenMPDefaultmapClauseModifier M,
+OpenMPDefaultmapClauseKind Kind,
+SourceLocation StartLoc,

cchen wrote:
> ABataev wrote:
> > Do you really need to rebuild it? It has only constants inside.
> I think you are right, the rebuild code for defaultmap seems redundant since 
> it has only constant inside the function. I added this code so that my 
> defaultmap(none) check inside DSAAttrChecker could be called for the second 
> time (after template initialization). Without adding this code, the check in 
> DSAAttrChecker for defaultmap(none) will never be called due to the guard in 
> VisitDeclRefExpr:
> ```
> if (E->isTypeDependent() || E->isValueDependent() ||
> E->containsUnexpandedParameterPack() || E->isInstantiationDependent())
> ```
> Where do you think I should put my check for defaultmap(none) so that I don't 
> need this redundant tree rebuild code? Thanks!
Ok, got it. Then better to rebuild the clause. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204



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

[PATCH] D69393: [RFC][DebugInfo] emit user specified address_space in dwarf

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In D69393#1720816 , @ast wrote:

> > The address spaces envisioned by the Linux kernel appear to be more 
> > informational and not descriptive of hardware characteristics.
>
> From the kernel pov the `__user` and normal are two different address spaces 
> that must be accessed via different kernel primitives.
>  User access needs stac/clac on x86 and other precautions.
>  iirc other architectures have co-processor address space that needs its own 
> load/store equivalents.
>  `__percpu` is also different address space. It's roughly equivalent to 
> `__thread` in user space.


Ah, it has been so long since I worked on priv code that I had forgotten about 
the use of user-context access instructions.  That would be a reasonable use of 
the DW_AT_address_class attribute for kernel code, so that a kernel-mode 
debugger would do the right thing with user-space addresses.

In D69393#1720817 , @yonghong-song 
wrote:

> @probinson Thanks for the input! That is my concern too mixing the user 
> defined and language defined may not be a good idea and may actually cause 
> confusion. This is exactly this RFC for. Let us try a different dwarf 
> encoding and then we can continue to discuss.


For proper separation of concerns, it would be best to define values to use for 
the DWARF attribute independently of whatever conventions the Linux kernel 
might have.  What the debugger needs to know is how to dereference the 
pointers; this may be different than how the kernel chooses to classify 
addresses.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69393



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

Re: [PATCH] D69391: Add #pragma clang loop ivdep

[Michael Kruse via cfe-commits]

What's the motivation of even allowing `#pragma clang loop
ivdep(disable)`? It either has no effect or conflicts with other loop
hints.



Sorry, for not using Phabricator. When I try to submit a comment, I
get the following error:

You Shall Not Pass: D69391

You do not have permission to edit this object.
Users with the "Can Edit" capability:

Administrators can take this action.
The owner of a revision can always view and edit it.



Michael


Am Fr., 25. Okt. 2019 um 12:43 Uhr schrieb Hal Finkel via Phabricator
:
>
> hfinkel added a comment.
>
> This is the same as #pragma clang loop vectorize(assume_safety)?
>
> In D69391#1720845 , @xbolva00 wrote:
>
> > "#pragma ivdep" should work too (compatibiluty mode with intel, gcc).
>
>
> The semantics are not the same, unfortunately.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D69391/new/
>
> https://reviews.llvm.org/D69391
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r374288 - Recommit "[Clang] Pragma vectorize_width() implies vectorize(enable)"

[Michael Kruse via cfe-commits]

@sjoerdmeijer

Before recommitting, please re-open the patch review.

Michael

Am Do., 24. Okt. 2019 um 18:45 Uhr schrieb Jordan Rupprecht
:
>
> Reverted in 6d424a161bf3e52730371da0b9439ed93a8ce406 due to the issue 
> described here. Should hopefully be a trivial fix forward.
>
> On Tue, Oct 22, 2019 at 2:46 PM Michael Kruse  
> wrote:
>>
>> Am Mo., 21. Okt. 2019 um 23:44 Uhr schrieb Jordan Rupprecht
>> :
>> > At any rate, it sounds like this is not a codegen bug at all, but just an 
>> > over-eager warning?
>>
>> That interpretation is different from mine. Codgen emits the following
>> from vectorize(disable)
>>
>> !4 = !{!"llvm.loop.vectorize.enable", i1 true}
>>
>> which is is not what I'd expect.
>>
>> Michael
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69420: [clang][clang-scan-deps] Add support for extracting full module dependencies.

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese added inline comments.



Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp:94
+
+void printDependencies(std::string &S, StringRef MainFile) {
+  // Sort the modules by name to get a deterministic order.

kousikk wrote:
> Should output arguments be at the end?
The point was to match up with `MakeDependencyPrinterConsumer`'s 
`printDependencies`. This will also be going away with the patch to merge the 
dependencies.


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

https://reviews.llvm.org/D69420



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

[PATCH] D69420: [clang][clang-scan-deps] Add support for extracting full module dependencies.

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese updated this revision to Diff 226508.
Bigcheese marked 3 inline comments as done.
Bigcheese added a comment.

Address review comments.


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

https://reviews.llvm.org/D69420

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Tooling/DependencyScanning/CMakeLists.txt
  clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/test/ClangScanDeps/modules-full.cpp
  clang/tools/clang-scan-deps/ClangScanDeps.cpp

Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -59,6 +59,17 @@
 llvm::cl::init(ScanningMode::MinimizedSourcePreprocessing),
 llvm::cl::cat(DependencyScannerCategory));
 
+static llvm::cl::opt Format(
+"format", llvm::cl::desc("The output format for the dependencies"),
+llvm::cl::values(clEnumValN(ScanningOutputFormat::Make, "make",
+"Makefile compatible dep file"),
+ clEnumValN(ScanningOutputFormat::Full, "experimental-full",
+"Full dependency graph suitable"
+" for explicitly building modules. This format "
+"is experimental and will change.")),
+llvm::cl::init(ScanningOutputFormat::Make),
+llvm::cl::cat(DependencyScannerCategory));
+
 llvm::cl::opt
 NumThreads("j", llvm::cl::Optional,
llvm::cl::desc("Number of worker threads to use (default: use "
@@ -200,7 +211,7 @@
   // Print out the dependency results to STDOUT by default.
   SharedStream DependencyOS(llvm::outs());
 
-  DependencyScanningService Service(ScanMode, ReuseFileManager,
+  DependencyScanningService Service(ScanMode, Format, ReuseFileManager,
 SkipExcludedPPRanges);
 #if LLVM_ENABLE_THREADS
   unsigned NumWorkers =
Index: clang/test/ClangScanDeps/modules-full.cpp
===
--- /dev/null
+++ clang/test/ClangScanDeps/modules-full.cpp
@@ -0,0 +1,74 @@
+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.cdb
+// RUN: rm -rf %t.module-cache
+// RUN: mkdir -p %t.dir
+// RUN: cp %s %t.dir/modules_cdb_input.cpp
+// RUN: cp %s %t.dir/modules_cdb_input2.cpp
+// RUN: mkdir %t.dir/Inputs
+// RUN: cp %S/Inputs/header.h %t.dir/Inputs/header.h
+// RUN: cp %S/Inputs/header2.h %t.dir/Inputs/header2.h
+// RUN: cp %S/Inputs/module.modulemap %t.dir/Inputs/module.modulemap
+// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/modules_cdb.json > %t.cdb
+//
+// RUN: echo %t.dir > %t.result
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 \
+// RUN:   -mode preprocess-minimized-sources -format experimental-full >> %t.result
+// RUN: cat %t.result | FileCheck --check-prefixes=CHECK %s
+
+#include "header.h"
+
+// CHECK: [[PREFIX:(.*[/\\])+[a-zA-Z0-9.-]+]]
+// CHECK-NEXT: {
+// CHECK-NEXT:  "clang-context-hash": "[[CONTEXT_HASH:[A-Z0-9]+]]",
+// CHECK-NEXT:  "clang-module-deps": [
+// CHECK-NEXT:"header1"
+// CHECK-NEXT:  ],
+// CHECK-NEXT:  "clang-modules": [
+// CHECK-NEXT:{
+// CHECK-NEXT:  "clang-module-deps": [
+// CHECK-NEXT:"header2"
+// CHECK-NEXT:  ],
+// CHECK-NEXT:  "clang-modulemap-file": "[[PREFIX]]{{[/\\]}}Inputs{{[/\\]}}module.modulemap",
+// CHECK-NEXT:  "file-deps": [
+// CHECK-NEXT:"[[PREFIX]]{{[/\\]}}Inputs{{[/\\]}}module.modulemap",
+// CHECK-NEXT:"[[PREFIX]]{{[/\\]}}Inputs{{[/\\]}}header.h"
+// CHECK-NEXT:  ],
+// CHECK-NEXT:  "name": "header1"
+// CHECK-NEXT:},
+// CHECK-NEXT:{
+// CHECK-NEXT:  "clang-module-deps": [],
+// CHECK-NEXT:  "clang-modulemap-file": "[[PREFIX]]{{[/\\]}}Inputs{{[/\\]}}module.modulemap",
+// CHECK-NEXT:  "file-deps": [
+// CHECK-NEXT:"[[PREFIX]]{{[/\\]}}Inputs{{[/\\]}}header2.h",
+// CHECK-NEXT:"[[PREFIX]]{{[/\\]}}Inputs{{[/\\]}}module.modulemap"
+// CHECK-NEXT:  ],
+// CHECK-NEXT:  "name": "header2"
+// CHECK-NEXT:}
+// CHECK-NEXT:  ],
+// CHECK-NEXT:  "file-deps": [
+// CHECK-NEXT:"header.h"
+// CHECK-NEXT:  ],
+// CHECK-NEXT:  "input-file": "[[PREFIX]]{{[/\\]}}modules_cdb_input2.cpp"
+// CHECK-NEXT:},
+// CHECK-NEXT:{
+// CHECK-NOT:   "clang-context-hash": "[[CONTEXT_HASH]]",
+// CHECK-NEXT:  "clang-context-hash": "{{[A-Z0-9]+}}",
+// CHECK-NEXT:  "clang-module-deps": [
+// CHECK-NEXT:"header1"
+// CHECK-NEXT:  ],
+

[clang] 8da2056 - [clang][DependencyScanning] 80-col.

[Michael Spencer via cfe-commits]

Author: Michael Spencer
Date: 2019-10-25T15:43:57-07:00
New Revision: 8da20560ab0da11c47d4718712c9c455e71c2b51

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

LOG: [clang][DependencyScanning] 80-col.

Added: 


Modified: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp

Removed: 




diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
index 0c9efccb1d8b..c950cbe167cd 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -1,4 +1,4 @@
-//===- DependencyScanningTool.h - clang-scan-deps service ===//
+//===- DependencyScanningTool.h - clang-scan-deps service 
-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -26,7 +26,9 @@ class DependencyScanningTool {
   ///
   /// \param Compilations The reference to the compilation database that's
   /// used by the clang tool.
-  DependencyScanningTool(DependencyScanningService &Service, const 
clang::tooling::CompilationDatabase &Compilations);
+  DependencyScanningTool(
+  DependencyScanningService &Service,
+  const clang::tooling::CompilationDatabase &Compilations);
 
   /// Print out the dependency information into a string using the dependency
   /// file format that is specified in the options (-MD is the default) and
@@ -34,7 +36,8 @@ class DependencyScanningTool {
   ///
   /// \returns A \c StringError with the diagnostic output if clang errors
   /// occurred, dependency file contents otherwise.
-  llvm::Expected getDependencyFile(const std::string &Input, 
StringRef CWD);
+  llvm::Expected getDependencyFile(const std::string &Input,
+StringRef CWD);
 
 private:
   DependencyScanningWorker Worker;

diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
index 5a6e2118a6ee..d2af1a9d110c 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -1,4 +1,4 @@
-//===- DependencyScanningTool.cpp - clang-scan-deps service ===//
+//===- DependencyScanningTool.cpp - clang-scan-deps service 
---===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -13,8 +13,10 @@ namespace clang{
 namespace tooling{
 namespace dependencies{
 
-DependencyScanningTool::DependencyScanningTool(DependencyScanningService 
&Service,
-const tooling::CompilationDatabase &Compilations) : Worker(Service), 
Compilations(Compilations) {}
+DependencyScanningTool::DependencyScanningTool(
+DependencyScanningService &Service,
+const tooling::CompilationDatabase &Compilations)
+: Worker(Service), Compilations(Compilations) {}
 
 llvm::Expected
 DependencyScanningTool::getDependencyFile(const std::string &Input,



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

[PATCH] D68937: [clangd] Add parameter renaming to define-inline code action

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Use findExplicitReferences for decl traversals.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68937

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

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1469,6 +1469,44 @@
 )cpp");
 }
 
+TEST_F(DefineInlineTest, TransformParamNames) {
+  EXPECT_EQ(apply(R"cpp(
+void foo(int, bool b);
+
+void ^foo(int f, bool x) {})cpp"), R"cpp(
+void foo(int f, bool x){}
+
+)cpp");
+}
+
+TEST_F(DefineInlineTest, TransformTemplParamNames) {
+  EXPECT_EQ(apply(R"cpp(
+struct Foo {
+  struct Bar {
+template  class, template class Y,
+  int, int Z>
+void foo(X, Y, int W = 5 * Z + 2);
+  };
+};
+
+template  class V, template class W,
+  int X, int Y>
+void Foo::Bar::f^oo(U, W, int Q) {})cpp"),
+R"cpp(
+struct Foo {
+  struct Bar {
+template  class V, template class W,
+  int X, int Y>
+void foo(U, W, int Q = 5 * Y + 2){}
+  };
+};
+
+)cpp");
+}
+
 TEST_F(DefineInlineTest, TransformInlineNamespaces) {
   EXPECT_EQ(apply(R"cpp(
 namespace a { inline namespace b { namespace { struct Foo{}; } } }
Index: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
@@ -226,6 +226,80 @@
   return QualifiedFunc->substr(BodyBegin, BodyEnd - BodyBegin + 1);
 }
 
+/// Returns true if Dest is templated function. Fills in \p ParamToNewName with
+/// a mapping from template decls in \p Dest to theire respective names in \p
+/// Source.
+bool fillTemplateParameterMapping(
+const FunctionDecl *Dest, const FunctionDecl *Source,
+llvm::DenseMap &ParamToNewName) {
+  auto *DestTempl = Dest->getDescribedFunctionTemplate();
+  auto *SourceTempl = Source->getDescribedFunctionTemplate();
+  assert(bool(DestTempl) == bool(SourceTempl));
+  if (!DestTempl)
+return false;
+  const auto *DestTPL = DestTempl->getTemplateParameters();
+  const auto *SourceTPL = SourceTempl->getTemplateParameters();
+  assert(DestTPL->size() == SourceTPL->size());
+
+  for (size_t I = 0, EP = DestTPL->size(); I != EP; ++I)
+ParamToNewName[DestTPL->getParam(I)->getCanonicalDecl()] =
+SourceTPL->getParam(I)->getName();
+  return true;
+}
+
+/// Generates Replacements for changing template and function parameter names in
+/// \p Dest to be the same as in \p Source.
+llvm::Expected
+renameParameters(const FunctionDecl *Dest, const FunctionDecl *Source) {
+  llvm::DenseMap ParamToNewName;
+  bool HasTemplateParams =
+  fillTemplateParameterMapping(Dest, Source, ParamToNewName);
+  tooling::Replacements Replacements;
+
+  // Populate mapping for function params.
+  for (size_t I = 0, E = Dest->param_size(); I != E; ++I) {
+auto *DestParam = Dest->getParamDecl(I);
+auto *SourceParam = Source->getParamDecl(I);
+ParamToNewName[DestParam] = SourceParam->getName();
+  }
+
+  llvm::Error RenamingErrors = llvm::Error::success();
+  const SourceManager &SM = Dest->getASTContext().getSourceManager();
+  findExplicitReferences(
+  // Use function template in case of templated functions to visit template
+  // parameters.
+  HasTemplateParams
+  ? llvm::dyn_cast(Dest->getDescribedFunctionTemplate())
+  : llvm::dyn_cast(Dest),
+  [&](ReferenceLoc Ref) {
+if (Ref.Targets.size() != 1)
+  return;
+const auto *Target =
+llvm::dyn_cast(Ref.Targets.front()->getCanonicalDecl());
+auto It = ParamToNewName.find(Target);
+if (It == ParamToNewName.end())
+  return;
+const size_t OldNameLen = Target->getName().size();
+// If decl is unnamed in both source and destination, just skip.
+if (OldNameLen == 0 && It->second.size() == 0)
+  return;
+// If decl is unnamed in destination we pad the new name to avoid gluing
+// with previous token, e.g. foo(int^) shouldn't turn into foo(intx).
+auto ReplacementText = (OldNameLen == 0 ? " " : "") + It->second;
+if (auto Err = Replacements.add(tooling::Replacement(
+SM, Ref.NameLoc, OldNameLen, ReplacementText))) {
+  elog("define inline: Couldn't replace parameter name for {0} to {1}: "
+   "{2}",
+   Target->getName(), ReplacementText, Err);
+  RenamingErrors =
+  l