[PATCH] D57787: [clang-tidy] modernize-avoid-c-arrays: avoid main function (PR40604)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tidy/modernize/AvoidCArraysCheck.cpp:35
+  const clang::DeclContext *DC = Node.getDeclContext();
+  const clang::FunctionDecl *FD = llvm::dyn_cast(DC);
+  if (!FD)

lebedev.ri wrote:
> JonasToth wrote:
> > There is `FunctionDecl->castToDeclContext()` which is probably a better fit 
> > here.
> I'm guessing you meant `cast*From*DeclContext()`.
> Interesting, that function is never once used.
> And it uses `static_cast<>()`..
I'm not too sure about this.
Given `ParmVarDecl`, are we sure it's `DeclContext` is *always* `FunctionDecl`?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57787



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

[PATCH] D57787: [clang-tidy] modernize-avoid-c-arrays: avoid main function (PR40604)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 185499.
lebedev.ri added a comment.

Add an assert to ensure that `clang::FunctionDecl::castFromDeclContext()` is 
safe to do.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57787

Files:
  clang-tidy/modernize/AvoidCArraysCheck.cpp
  docs/clang-tidy/checks/modernize-avoid-c-arrays.rst
  test/clang-tidy/modernize-avoid-c-arrays-ignores-main.cpp
  test/clang-tidy/modernize-avoid-c-arrays-ignores-three-arg-main.cpp


Index: test/clang-tidy/modernize-avoid-c-arrays-ignores-three-arg-main.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-avoid-c-arrays-ignores-three-arg-main.cpp
@@ -0,0 +1,20 @@
+// RUN: %check_clang_tidy %s modernize-avoid-c-arrays %t
+
+int not_main(int argc, char *argv[], char *argw[]) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not declare C-style arrays, 
use std::array<> instead
+  // CHECK-MESSAGES: :[[@LINE-2]]:38: warning: do not declare C-style arrays, 
use std::array<> instead
+  int f4[] = {1, 2};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, 
use std::array<> instead
+}
+
+int main(int argc, char *argv[], char *argw[]) {
+  int f5[] = {1, 2};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, 
use std::array<> instead
+
+  auto not_main = [](int argc, char *argv[], char *argw[]) {
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: do not declare C-style 
arrays, use std::array<> instead
+// CHECK-MESSAGES: :[[@LINE-2]]:46: warning: do not declare C-style 
arrays, use std::array<> instead
+int f6[] = {1, 2};
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not declare C-style arrays, 
use std::array<> instead
+  };
+}
Index: test/clang-tidy/modernize-avoid-c-arrays-ignores-main.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-avoid-c-arrays-ignores-main.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s modernize-avoid-c-arrays %t
+
+int not_main(int argc, char *argv[]) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not declare C-style arrays, 
use std::array<> instead
+  int f4[] = {1, 2};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, 
use std::array<> instead
+}
+
+int main(int argc, char *argv[]) {
+  int f5[] = {1, 2};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, 
use std::array<> instead
+
+  auto not_main = [](int argc, char *argv[]) {
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: do not declare C-style 
arrays, use std::array<> instead
+int f6[] = {1, 2};
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not declare C-style arrays, 
use std::array<> instead
+  };
+}
Index: docs/clang-tidy/checks/modernize-avoid-c-arrays.rst
===
--- docs/clang-tidy/checks/modernize-avoid-c-arrays.rst
+++ docs/clang-tidy/checks/modernize-avoid-c-arrays.rst
@@ -54,3 +54,6 @@
   }
 
   }
+
+Similarly, the ``main()`` function is ignored. It's second and third params can
+be either ``char* argv[]`` or ``char** argv``, but can not be ``std::array<>``.
Index: clang-tidy/modernize/AvoidCArraysCheck.cpp
===
--- clang-tidy/modernize/AvoidCArraysCheck.cpp
+++ clang-tidy/modernize/AvoidCArraysCheck.cpp
@@ -30,6 +30,14 @@
   return Node.isExternCContext();
 }
 
+AST_MATCHER(clang::ParmVarDecl, isArgvOfMain) {
+  const clang::DeclContext *DC = Node.getDeclContext();
+  assert(llvm::isa(DC) &&
+ "ParmVarDecl with non-FunctionDecl DeclContext");
+  const clang::FunctionDecl *FD = clang::FunctionDecl::castFromDeclContext(DC);
+  return FD->isMain();
+}
+
 } // namespace
 
 namespace clang {
@@ -43,7 +51,8 @@
 
   Finder->addMatcher(
   typeLoc(hasValidBeginLoc(), hasType(arrayType()),
-  unless(anyOf(hasParent(varDecl(isExternC())),
+  unless(anyOf(hasParent(parmVarDecl(isArgvOfMain())),
+   hasParent(varDecl(isExternC())),
hasParent(fieldDecl(
hasParent(recordDecl(isExternCContext(),
hasAncestor(functionDecl(isExternC())


Index: test/clang-tidy/modernize-avoid-c-arrays-ignores-three-arg-main.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-avoid-c-arrays-ignores-three-arg-main.cpp
@@ -0,0 +1,20 @@
+// RUN: %check_clang_tidy %s modernize-avoid-c-arrays %t
+
+int not_main(int argc, char *argv[], char *argw[]) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not declare C-style arrays, use std::array<> instead
+  // CHECK-MESSAGES: :[[@LINE-2]]:38: warning: do not declare C-style arrays, use std::array<> instead
+  int f4[] = {1, 2};
+  // CHECK-MESSAG

[PATCH] D57021: [clangd] Suggest adding missing includes for typos (like include-fixer).

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clangd/IncludeFixer.cpp:209
+  std::vector Scopes;
+  if (Typo.SS) {
+Scopes.push_back(*Typo.SS);

ioeric wrote:
> sammccall wrote:
> > why do we do this here rather than compute the scopes/name already in 
> > CorrectTypo()?
> > 
> > Passing around more of these AST objects and doing the computations later 
> > seems a bit more fragile.
> Because not all typos will result in diagnostics. Computing scopes could be 
> expensive, so we would only want to do that when necessary. For example, sema 
> can perform typo corrections for both `x` and `y` in `x::y`, while only one 
> diagnostic (for either `x` or `y`) would be generated.
If the goal is to make the scope computation lazy, `CorrectTypo` can store them 
as a `std::function()>` - that way we can still group the 
related code and dependencies together.

This function can capture the Sema instance by pointer - this deserves a 
comment that sema must be alive to access the scopes.



Comment at: clangd/IncludeFixer.cpp:82
+  case diag::err_no_template_suggest:
+if (LastUnresolvedName) {
+  // Try to fix typos caused by missing declaraion.

you never seem to clear LastUnresolvedName - I guess you at least want to clear 
it after using it.
It would be more robust to clear after every diagnostic - would this work?



Comment at: clangd/IncludeFixer.cpp:161
+Fixes.push_back(
+Fix{llvm::formatv("Add include {0} for symbol {1}{2}",
+  ToInclude->first, Sym.Scope, Sym.Name),

note that if you have symbols in different scopes from the same header, you're 
only generating one fix and it will only mention one of the symbols.

It's fine to decide this case doesn't matter, but it's worth adding a comment.
(Alternatively, using a set of pair to track 
dupes would avoid this)



Comment at: clangd/IncludeFixer.cpp:186
+ const ObjCObjectPointerType *OPT) override {
+assert(S && "Sema must have been set.");
+if (SemaPtr->isSFINAEContext())

SemaPtr?



Comment at: clangd/IncludeFixer.cpp:199
+
+// FIXME: support invalid scope before a type name. In the following
+// example, namespace "clang::tidy::" hasn't been declared/imported.

The FIXME makes sense, but it's not clear to me what the code below it does or 
how it relates to the comment.

maybe separate with a blank line and then "Extract the typed scope" or so?



Comment at: clangd/IncludeFixer.cpp:228
+
+// Never return a valid correction to try to recorver. Our suggested fixes
+// always require a rebuild.

nit: recover



Comment at: clangd/IncludeFixer.cpp:233
+
+  llvm::Optional lastUnresolvedName() const {
+return LastUnresolvedName;

unused



Comment at: clangd/IncludeFixer.h:60
+std::string Name;// The typo identifier e.g. "X" in ns::X.
+SourceLocation Loc;  // Location of the typo.
+Scope *S;// Scope in which the typo is found.

nit: the comment on Loc and LookupKind don't say anything, remove them?



Comment at: clangd/IncludeFixer.h:61
+SourceLocation Loc;  // Location of the typo.
+Scope *S;// Scope in which the typo is found.
+llvm::Optional SS;  // The scope qualifier before the typo.

please use real names for these fields (EnclosingScope, SpecifiedScope?)

Maybe it would help documenting here to have a more complete example e.g.

`namespace ns { int Y = foo::x }`

Name is x
Loc is points at x (or f?)
EnclosingScope is ns::
SpecifiedScope is either ns::foo:: or foo:: (depending how 'foo' was resolved)



Comment at: clangd/IncludeFixer.h:72
+  /// and the typo is the last typo we've seen during the Sema run.
+  std::vector fixUnresolvedName(const UnresolvedName &Unresolved) const;
+

nit: why take a parameter here, when it's always *LastUnresolvedName?



Comment at: clangd/IncludeFixer.h:35
 public:
-  IncludeFixer(llvm::StringRef File, std::shared_ptr Inserter,
+  IncludeFixer(CompilerInstance &Compiler, llvm::StringRef File,
+   std::shared_ptr Inserter,

ioeric wrote:
> sammccall wrote:
> > Having to inject the whole compiler into the include fixer doesn't seem 
> > right. Apart from the huge scope, we're also going to register parts of the 
> > include fixer with the compiler.
> > 
> > A few observations:
> >  - all we actually need here is Sema and the SM (available through Sema)
> >  - it's only the typo recorder that needs access to Sema
> >  - the typo recorder gets fed the Sema instance 
> > (`ExternalSemaSource::InitializeSema`)
> >  - the fact that 

[PATCH] D57755: [clangd] Some minor fixes.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353283: [clangd] Some minor fixes. (authored by hokein, 
committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57755

Files:
  clang-tools-extra/trunk/clangd/Selection.cpp
  clang-tools-extra/trunk/clangd/refactor/Tweak.h
  clang-tools-extra/trunk/unittests/clangd/SelectionTests.cpp
  clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp


Index: clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp
@@ -1,9 +1,8 @@
 //===-- TweakTests.cpp --*- C++ 
-*-===//
 //
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
+// 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
 //
 
//===--===//
 
Index: clang-tools-extra/trunk/unittests/clangd/SelectionTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/SelectionTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/SelectionTests.cpp
@@ -1,4 +1,4 @@
-//===-- RIFFTests.cpp - Binary container unit tests 
---===//
+//===-- SelectionTests.cpp - 
--===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
Index: clang-tools-extra/trunk/clangd/Selection.cpp
===
--- clang-tools-extra/trunk/clangd/Selection.cpp
+++ clang-tools-extra/trunk/clangd/Selection.cpp
@@ -1,4 +1,4 @@
-//===--- Selection.h 
--===//
+//===--- Selection.cpp 
===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
Index: clang-tools-extra/trunk/clangd/refactor/Tweak.h
===
--- clang-tools-extra/trunk/clangd/refactor/Tweak.h
+++ clang-tools-extra/trunk/clangd/refactor/Tweak.h
@@ -56,8 +56,8 @@
   /// defining the Tweak. Definition is provided automatically by
   /// REGISTER_TWEAK.
   virtual const char *id() const = 0;
-  /// Run the first stage of the action. The non-None result indicates that the
-  /// action is available and should be shown to the user. Returns None if the
+  /// Run the first stage of the action. Returns true indicating that the
+  /// action is available and should be shown to the user. Returns false if the
   /// action is not available.
   /// This function should be fast, if the action requires non-trivial work it
   /// should be moved into 'apply'.


Index: clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp
@@ -1,9 +1,8 @@
 //===-- TweakTests.cpp --*- C++ -*-===//
 //
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
+// 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
 //
 //===--===//
 
Index: clang-tools-extra/trunk/unittests/clangd/SelectionTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/SelectionTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/SelectionTests.cpp
@@ -1,4 +1,4 @@
-//===-- RIFFTests.cpp - Binary container unit tests ---===//
+//===-- SelectionTests.cpp - --===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
Index: clang-tools-extra/trunk/clangd/Selection.cpp
===
--- clang-tools-extra/trunk/clangd/Selection.cpp
+++ clang-tools-extra

[clang-tools-extra] r353283 - [clangd] Some minor fixes.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Wed Feb  6 01:08:26 2019
New Revision: 353283

URL: http://llvm.org/viewvc/llvm-project?rev=353283&view=rev
Log:
[clangd] Some minor fixes.

Reviewers: ilya-biryukov

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/Selection.cpp
clang-tools-extra/trunk/clangd/refactor/Tweak.h
clang-tools-extra/trunk/unittests/clangd/SelectionTests.cpp
clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp

Modified: clang-tools-extra/trunk/clangd/Selection.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Selection.cpp?rev=353283&r1=353282&r2=353283&view=diff
==
--- clang-tools-extra/trunk/clangd/Selection.cpp (original)
+++ clang-tools-extra/trunk/clangd/Selection.cpp Wed Feb  6 01:08:26 2019
@@ -1,4 +1,4 @@
-//===--- Selection.h 
--===//
+//===--- Selection.cpp 
===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.

Modified: clang-tools-extra/trunk/clangd/refactor/Tweak.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/Tweak.h?rev=353283&r1=353282&r2=353283&view=diff
==
--- clang-tools-extra/trunk/clangd/refactor/Tweak.h (original)
+++ clang-tools-extra/trunk/clangd/refactor/Tweak.h Wed Feb  6 01:08:26 2019
@@ -56,8 +56,8 @@ public:
   /// defining the Tweak. Definition is provided automatically by
   /// REGISTER_TWEAK.
   virtual const char *id() const = 0;
-  /// Run the first stage of the action. The non-None result indicates that the
-  /// action is available and should be shown to the user. Returns None if the
+  /// Run the first stage of the action. Returns true indicating that the
+  /// action is available and should be shown to the user. Returns false if the
   /// action is not available.
   /// This function should be fast, if the action requires non-trivial work it
   /// should be moved into 'apply'.

Modified: clang-tools-extra/trunk/unittests/clangd/SelectionTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SelectionTests.cpp?rev=353283&r1=353282&r2=353283&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/SelectionTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SelectionTests.cpp Wed Feb  6 
01:08:26 2019
@@ -1,4 +1,4 @@
-//===-- RIFFTests.cpp - Binary container unit tests 
---===//
+//===-- SelectionTests.cpp - 
--===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.

Modified: clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp?rev=353283&r1=353282&r2=353283&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp Wed Feb  6 01:08:26 
2019
@@ -1,9 +1,8 @@
 //===-- TweakTests.cpp --*- C++ 
-*-===//
 //
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
+// 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
 //
 
//===--===//
 


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

[clang-tools-extra] r353284 - [clangd] Add CLI flag "-clang-tidy" to enable/disable running clang-tidy checks.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Wed Feb  6 01:10:47 2019
New Revision: 353284

URL: http://llvm.org/viewvc/llvm-project?rev=353284&view=rev
Log:
[clangd] Add CLI flag "-clang-tidy" to enable/disable running clang-tidy checks.

Reviewers: sammccall

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp

Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=353284&r1=353283&r2=353284&view=diff
==
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Wed Feb  6 01:10:47 2019
@@ -203,10 +203,16 @@ static llvm::cl::opt EnableFunctio
 
 static llvm::cl::opt ClangTidyChecks(
 "clang-tidy-checks",
-llvm::cl::desc("List of clang-tidy checks to run (this will override "
-   ".clang-tidy files)"),
+llvm::cl::desc(
+"List of clang-tidy checks to run (this will override "
+".clang-tidy files). Only meaningful when -clang-tidy flag is on."),
 llvm::cl::init(""));
 
+static llvm::cl::opt EnableClangTidy(
+"clang-tidy",
+llvm::cl::desc("Enable clang-tidy diagnostics."),
+llvm::cl::init(false));
+
 static llvm::cl::opt SuggestMissingIncludes(
 "suggest-missing-includes",
 llvm::cl::desc("Attempts to fix diagnostic errors caused by missing "
@@ -441,13 +447,16 @@ int main(int argc, char *argv[]) {
   }
 
   // Create an empty clang-tidy option.
-  auto OverrideClangTidyOptions = tidy::ClangTidyOptions::getDefaults();
-  OverrideClangTidyOptions.Checks = ClangTidyChecks;
-  tidy::FileOptionsProvider ClangTidyOptProvider(
-  tidy::ClangTidyGlobalOptions(),
-  /* Default */ tidy::ClangTidyOptions::getDefaults(),
-  /* Override */ OverrideClangTidyOptions, FSProvider.getFileSystem());
-  Opts.ClangTidyOptProvider = &ClangTidyOptProvider;
+  std::unique_ptr ClangTidyOptProvider;
+  if (EnableClangTidy) {
+auto OverrideClangTidyOptions = tidy::ClangTidyOptions::getDefaults();
+OverrideClangTidyOptions.Checks = ClangTidyChecks;
+ClangTidyOptProvider = llvm::make_unique(
+tidy::ClangTidyGlobalOptions(),
+/* Default */ tidy::ClangTidyOptions::getDefaults(),
+/* Override */ OverrideClangTidyOptions, FSProvider.getFileSystem());
+  }
+  Opts.ClangTidyOptProvider = ClangTidyOptProvider.get();
   Opts.SuggestMissingIncludes = SuggestMissingIncludes;
   ClangdLSPServer LSPServer(
   *TransportLayer, FSProvider, CCOpts, CompileCommandsDirPath,


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

[PATCH] D57746: [clangd] Add CLI flag "-clang-tidy" to enable/disable running clang-tidy checks.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353284: [clangd] Add CLI flag "-clang-tidy" to 
enable/disable running clang-tidy checks. (authored by hokein, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57746

Files:
  clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
@@ -203,10 +203,16 @@
 
 static llvm::cl::opt ClangTidyChecks(
 "clang-tidy-checks",
-llvm::cl::desc("List of clang-tidy checks to run (this will override "
-   ".clang-tidy files)"),
+llvm::cl::desc(
+"List of clang-tidy checks to run (this will override "
+".clang-tidy files). Only meaningful when -clang-tidy flag is on."),
 llvm::cl::init(""));
 
+static llvm::cl::opt EnableClangTidy(
+"clang-tidy",
+llvm::cl::desc("Enable clang-tidy diagnostics."),
+llvm::cl::init(false));
+
 static llvm::cl::opt SuggestMissingIncludes(
 "suggest-missing-includes",
 llvm::cl::desc("Attempts to fix diagnostic errors caused by missing "
@@ -441,13 +447,16 @@
   }
 
   // Create an empty clang-tidy option.
-  auto OverrideClangTidyOptions = tidy::ClangTidyOptions::getDefaults();
-  OverrideClangTidyOptions.Checks = ClangTidyChecks;
-  tidy::FileOptionsProvider ClangTidyOptProvider(
-  tidy::ClangTidyGlobalOptions(),
-  /* Default */ tidy::ClangTidyOptions::getDefaults(),
-  /* Override */ OverrideClangTidyOptions, FSProvider.getFileSystem());
-  Opts.ClangTidyOptProvider = &ClangTidyOptProvider;
+  std::unique_ptr ClangTidyOptProvider;
+  if (EnableClangTidy) {
+auto OverrideClangTidyOptions = tidy::ClangTidyOptions::getDefaults();
+OverrideClangTidyOptions.Checks = ClangTidyChecks;
+ClangTidyOptProvider = llvm::make_unique(
+tidy::ClangTidyGlobalOptions(),
+/* Default */ tidy::ClangTidyOptions::getDefaults(),
+/* Override */ OverrideClangTidyOptions, FSProvider.getFileSystem());
+  }
+  Opts.ClangTidyOptProvider = ClangTidyOptProvider.get();
   Opts.SuggestMissingIncludes = SuggestMissingIncludes;
   ClangdLSPServer LSPServer(
   *TransportLayer, FSProvider, CCOpts, CompileCommandsDirPath,


Index: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
@@ -203,10 +203,16 @@
 
 static llvm::cl::opt ClangTidyChecks(
 "clang-tidy-checks",
-llvm::cl::desc("List of clang-tidy checks to run (this will override "
-   ".clang-tidy files)"),
+llvm::cl::desc(
+"List of clang-tidy checks to run (this will override "
+".clang-tidy files). Only meaningful when -clang-tidy flag is on."),
 llvm::cl::init(""));
 
+static llvm::cl::opt EnableClangTidy(
+"clang-tidy",
+llvm::cl::desc("Enable clang-tidy diagnostics."),
+llvm::cl::init(false));
+
 static llvm::cl::opt SuggestMissingIncludes(
 "suggest-missing-includes",
 llvm::cl::desc("Attempts to fix diagnostic errors caused by missing "
@@ -441,13 +447,16 @@
   }
 
   // Create an empty clang-tidy option.
-  auto OverrideClangTidyOptions = tidy::ClangTidyOptions::getDefaults();
-  OverrideClangTidyOptions.Checks = ClangTidyChecks;
-  tidy::FileOptionsProvider ClangTidyOptProvider(
-  tidy::ClangTidyGlobalOptions(),
-  /* Default */ tidy::ClangTidyOptions::getDefaults(),
-  /* Override */ OverrideClangTidyOptions, FSProvider.getFileSystem());
-  Opts.ClangTidyOptProvider = &ClangTidyOptProvider;
+  std::unique_ptr ClangTidyOptProvider;
+  if (EnableClangTidy) {
+auto OverrideClangTidyOptions = tidy::ClangTidyOptions::getDefaults();
+OverrideClangTidyOptions.Checks = ClangTidyChecks;
+ClangTidyOptProvider = llvm::make_unique(
+tidy::ClangTidyGlobalOptions(),
+/* Default */ tidy::ClangTidyOptions::getDefaults(),
+/* Override */ OverrideClangTidyOptions, FSProvider.getFileSystem());
+  }
+  Opts.ClangTidyOptProvider = ClangTidyOptProvider.get();
   Opts.SuggestMissingIncludes = SuggestMissingIncludes;
   ClangdLSPServer LSPServer(
   *TransportLayer, FSProvider, CCOpts, CompileCommandsDirPath,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57746: [clangd] Add CLI flag "-clang-tidy" to enable/disable running clang-tidy checks.

[Haojian Wu via Phabricator via cfe-commits]

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

Set the flag to false by default.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57746

Files:
  clangd/tool/ClangdMain.cpp


Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -203,10 +203,16 @@
 
 static llvm::cl::opt ClangTidyChecks(
 "clang-tidy-checks",
-llvm::cl::desc("List of clang-tidy checks to run (this will override "
-   ".clang-tidy files)"),
+llvm::cl::desc(
+"List of clang-tidy checks to run (this will override "
+".clang-tidy files). Only meaningful when -clang-tidy flag is on."),
 llvm::cl::init(""));
 
+static llvm::cl::opt EnableClangTidy(
+"clang-tidy",
+llvm::cl::desc("Enable clang-tidy diagnostics."),
+llvm::cl::init(false));
+
 static llvm::cl::opt SuggestMissingIncludes(
 "suggest-missing-includes",
 llvm::cl::desc("Attempts to fix diagnostic errors caused by missing "
@@ -441,13 +447,16 @@
   }
 
   // Create an empty clang-tidy option.
-  auto OverrideClangTidyOptions = tidy::ClangTidyOptions::getDefaults();
-  OverrideClangTidyOptions.Checks = ClangTidyChecks;
-  tidy::FileOptionsProvider ClangTidyOptProvider(
-  tidy::ClangTidyGlobalOptions(),
-  /* Default */ tidy::ClangTidyOptions::getDefaults(),
-  /* Override */ OverrideClangTidyOptions, FSProvider.getFileSystem());
-  Opts.ClangTidyOptProvider = &ClangTidyOptProvider;
+  std::unique_ptr ClangTidyOptProvider;
+  if (EnableClangTidy) {
+auto OverrideClangTidyOptions = tidy::ClangTidyOptions::getDefaults();
+OverrideClangTidyOptions.Checks = ClangTidyChecks;
+ClangTidyOptProvider = llvm::make_unique(
+tidy::ClangTidyGlobalOptions(),
+/* Default */ tidy::ClangTidyOptions::getDefaults(),
+/* Override */ OverrideClangTidyOptions, FSProvider.getFileSystem());
+  }
+  Opts.ClangTidyOptProvider = ClangTidyOptProvider.get();
   Opts.SuggestMissingIncludes = SuggestMissingIncludes;
   ClangdLSPServer LSPServer(
   *TransportLayer, FSProvider, CCOpts, CompileCommandsDirPath,


Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -203,10 +203,16 @@
 
 static llvm::cl::opt ClangTidyChecks(
 "clang-tidy-checks",
-llvm::cl::desc("List of clang-tidy checks to run (this will override "
-   ".clang-tidy files)"),
+llvm::cl::desc(
+"List of clang-tidy checks to run (this will override "
+".clang-tidy files). Only meaningful when -clang-tidy flag is on."),
 llvm::cl::init(""));
 
+static llvm::cl::opt EnableClangTidy(
+"clang-tidy",
+llvm::cl::desc("Enable clang-tidy diagnostics."),
+llvm::cl::init(false));
+
 static llvm::cl::opt SuggestMissingIncludes(
 "suggest-missing-includes",
 llvm::cl::desc("Attempts to fix diagnostic errors caused by missing "
@@ -441,13 +447,16 @@
   }
 
   // Create an empty clang-tidy option.
-  auto OverrideClangTidyOptions = tidy::ClangTidyOptions::getDefaults();
-  OverrideClangTidyOptions.Checks = ClangTidyChecks;
-  tidy::FileOptionsProvider ClangTidyOptProvider(
-  tidy::ClangTidyGlobalOptions(),
-  /* Default */ tidy::ClangTidyOptions::getDefaults(),
-  /* Override */ OverrideClangTidyOptions, FSProvider.getFileSystem());
-  Opts.ClangTidyOptProvider = &ClangTidyOptProvider;
+  std::unique_ptr ClangTidyOptProvider;
+  if (EnableClangTidy) {
+auto OverrideClangTidyOptions = tidy::ClangTidyOptions::getDefaults();
+OverrideClangTidyOptions.Checks = ClangTidyChecks;
+ClangTidyOptProvider = llvm::make_unique(
+tidy::ClangTidyGlobalOptions(),
+/* Default */ tidy::ClangTidyOptions::getDefaults(),
+/* Override */ OverrideClangTidyOptions, FSProvider.getFileSystem());
+  }
+  Opts.ClangTidyOptProvider = ClangTidyOptProvider.get();
   Opts.SuggestMissingIncludes = SuggestMissingIncludes;
   ClangdLSPServer LSPServer(
   *TransportLayer, FSProvider, CCOpts, CompileCommandsDirPath,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57768: [SYCL] Add SYCL device compilation flow.

[Victor Lomuller via Phabricator via cfe-commits]

Naghasan accepted this revision.
Naghasan added a subscriber: Anastasia.
Naghasan added a comment.

LGTM

Side note: might be good to also involve @Anastasia, as some of the future 
patches will overlap with OpenCL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57768



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

[PATCH] D57787: [clang-tidy] modernize-avoid-c-arrays: avoid main function (PR40604)

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/modernize/AvoidCArraysCheck.cpp:35
+  const clang::DeclContext *DC = Node.getDeclContext();
+  const clang::FunctionDecl *FD = llvm::dyn_cast(DC);
+  if (!FD)

lebedev.ri wrote:
> lebedev.ri wrote:
> > JonasToth wrote:
> > > There is `FunctionDecl->castToDeclContext()` which is probably a better 
> > > fit here.
> > I'm guessing you meant `cast*From*DeclContext()`.
> > Interesting, that function is never once used.
> > And it uses `static_cast<>()`..
> I'm not too sure about this.
> Given `ParmVarDecl`, are we sure it's `DeclContext` is *always* 
> `FunctionDecl`?
From Doc "Represents a parameter to a function. " so i think it has always a 
`FunctionDecl` (or subclass) as DeclContext.

Maybe that function is a relict? I just saw it in the docs too and thought it 
makes sense to use it. No opinion on that, @aaron.ballman do you know more on 
that?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57787



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

[PATCH] D57740: [ASTImporter] Import every Decl in lambda record

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 185509.
gamesh411 added a comment.

Remove unnecessary ToTU variable from test case.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57740

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -2417,6 +2417,26 @@
   EXPECT_EQ(ToDFOutOfClass->getPreviousDecl(), ToDFInClass);
 }
 
+TEST_P(ImportFunctions, ImportImplicitFunctionsInLambda) {
+  Decl *FromTU = getTuDecl(
+  R"(
+  void foo() {
+(void)[]() { ; };
+  }
+  )",
+  Lang_CXX11);
+  auto *FromD = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasName("foo")));
+  auto *ToD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ToD);
+  CXXRecordDecl *LambdaRec =
+  cast(cast(
+   *cast(ToD->getBody())->body_begin())
+   ->getSubExpr())
+  ->getLambdaClass();
+  EXPECT_TRUE(LambdaRec->getDestructor());
+}
+
 struct ImportFriendFunctions : ImportFunctions {};
 
 TEST_P(ImportFriendFunctions, ImportFriendFunctionRedeclChainProto) {
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -7382,13 +7382,9 @@
   // NOTE: lambda classes are created with BeingDefined flag set up.
   // It means that ImportDefinition doesn't work for them and we should fill it
   // manually.
-  if (ToClass->isBeingDefined()) {
-for (auto FromField : FromClass->fields()) {
-  auto ToFieldOrErr = import(FromField);
-  if (!ToFieldOrErr)
-return ToFieldOrErr.takeError();
-}
-  }
+  if (ToClass->isBeingDefined())
+if (Error Err = ImportDeclContext(FromClass, /*ForceImport = */ true))
+  return std::move(Err);
 
   auto ToCallOpOrErr = import(E->getCallOperator());
   if (!ToCallOpOrErr)


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -2417,6 +2417,26 @@
   EXPECT_EQ(ToDFOutOfClass->getPreviousDecl(), ToDFInClass);
 }
 
+TEST_P(ImportFunctions, ImportImplicitFunctionsInLambda) {
+  Decl *FromTU = getTuDecl(
+  R"(
+  void foo() {
+(void)[]() { ; };
+  }
+  )",
+  Lang_CXX11);
+  auto *FromD = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasName("foo")));
+  auto *ToD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ToD);
+  CXXRecordDecl *LambdaRec =
+  cast(cast(
+   *cast(ToD->getBody())->body_begin())
+   ->getSubExpr())
+  ->getLambdaClass();
+  EXPECT_TRUE(LambdaRec->getDestructor());
+}
+
 struct ImportFriendFunctions : ImportFunctions {};
 
 TEST_P(ImportFriendFunctions, ImportFriendFunctionRedeclChainProto) {
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -7382,13 +7382,9 @@
   // NOTE: lambda classes are created with BeingDefined flag set up.
   // It means that ImportDefinition doesn't work for them and we should fill it
   // manually.
-  if (ToClass->isBeingDefined()) {
-for (auto FromField : FromClass->fields()) {
-  auto ToFieldOrErr = import(FromField);
-  if (!ToFieldOrErr)
-return ToFieldOrErr.takeError();
-}
-  }
+  if (ToClass->isBeingDefined())
+if (Error Err = ImportDeclContext(FromClass, /*ForceImport = */ true))
+  return std::move(Err);
 
   auto ToCallOpOrErr = import(E->getCallOperator());
   if (!ToCallOpOrErr)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libunwind] r353287 - Merging r352016:

[Hans Wennborg via cfe-commits]

Author: hans
Date: Wed Feb  6 02:22:11 2019
New Revision: 353287

URL: http://llvm.org/viewvc/llvm-project?rev=353287&view=rev
Log:
Merging r352016:

r352016 | phosek | 2019-01-24 04:04:42 +0100 (Thu, 24 Jan 2019) | 12 lines

[libunwind] Don't abort if encoutering invalid .eh_frame_hdr

Recent Linux kernel release has introduced a bug as part of the ORC
rollout where the vDSO has a valid .eh_frame section, but it's missing
the .eh_frame_hdr section and GNU_EH_FRAME segment has zero size. This
causes libunwind to abort which breaks programs that use libunwind.

The other unwinder implementation (libgcc, non-gnu) instead silently
bail out unless being compiled as debug. This change modifies libunwind
to use the same strategy.

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


Modified:
libunwind/branches/release_80/   (props changed)
libunwind/branches/release_80/src/AddressSpace.hpp
libunwind/branches/release_80/src/EHHeaderParser.hpp

Propchange: libunwind/branches/release_80/
--
svn:mergeinfo = /libunwind/trunk:352016

Modified: libunwind/branches/release_80/src/AddressSpace.hpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/branches/release_80/src/AddressSpace.hpp?rev=353287&r1=353286&r2=353287&view=diff
==
--- libunwind/branches/release_80/src/AddressSpace.hpp (original)
+++ libunwind/branches/release_80/src/AddressSpace.hpp Wed Feb  6 02:22:11 2019
@@ -534,11 +534,11 @@ inline bool LocalAddressSpace::findUnwin
 #endif
 cbdata->sects->dwarf_index_section = eh_frame_hdr_start;
 cbdata->sects->dwarf_index_section_length = phdr->p_memsz;
-EHHeaderParser::decodeEHHdr(
+found_hdr = EHHeaderParser::decodeEHHdr(
 *cbdata->addressSpace, eh_frame_hdr_start, phdr->p_memsz,
 hdrInfo);
-cbdata->sects->dwarf_section = hdrInfo.eh_frame_ptr;
-found_hdr = true;
+if (found_hdr)
+  cbdata->sects->dwarf_section = hdrInfo.eh_frame_ptr;
   }
 }
 

Modified: libunwind/branches/release_80/src/EHHeaderParser.hpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/branches/release_80/src/EHHeaderParser.hpp?rev=353287&r1=353286&r2=353287&view=diff
==
--- libunwind/branches/release_80/src/EHHeaderParser.hpp (original)
+++ libunwind/branches/release_80/src/EHHeaderParser.hpp Wed Feb  6 02:22:11 
2019
@@ -36,7 +36,7 @@ public:
 uint8_t table_enc;
   };
 
-  static void decodeEHHdr(A &addressSpace, pint_t ehHdrStart, pint_t ehHdrEnd,
+  static bool decodeEHHdr(A &addressSpace, pint_t ehHdrStart, pint_t ehHdrEnd,
   EHHeaderInfo &ehHdrInfo);
   static bool findFDE(A &addressSpace, pint_t pc, pint_t ehHdrStart,
   uint32_t sectionLength,
@@ -53,12 +53,14 @@ private:
 };
 
 template 
-void EHHeaderParser::decodeEHHdr(A &addressSpace, pint_t ehHdrStart,
+bool EHHeaderParser::decodeEHHdr(A &addressSpace, pint_t ehHdrStart,
 pint_t ehHdrEnd, EHHeaderInfo &ehHdrInfo) {
   pint_t p = ehHdrStart;
   uint8_t version = addressSpace.get8(p++);
-  if (version != 1)
-_LIBUNWIND_ABORT("Unsupported .eh_frame_hdr version");
+  if (version != 1) {
+_LIBUNWIND_LOG0("Unsupported .eh_frame_hdr version");
+return false;
+  }
 
   uint8_t eh_frame_ptr_enc = addressSpace.get8(p++);
   uint8_t fde_count_enc = addressSpace.get8(p++);
@@ -71,6 +73,8 @@ void EHHeaderParser::decodeEHHdr(A &a
   ? 0
   : addressSpace.getEncodedP(p, ehHdrEnd, fde_count_enc, ehHdrStart);
   ehHdrInfo.table = p;
+
+  return true;
 }
 
 template 
@@ -102,7 +106,9 @@ bool EHHeaderParser::findFDE(A &addre
   pint_t ehHdrEnd = ehHdrStart + sectionLength;
 
   EHHeaderParser::EHHeaderInfo hdrInfo;
-  EHHeaderParser::decodeEHHdr(addressSpace, ehHdrStart, ehHdrEnd, hdrInfo);
+  if (!EHHeaderParser::decodeEHHdr(addressSpace, ehHdrStart, ehHdrEnd,
+  hdrInfo))
+return false;
 
   size_t tableEntrySize = getTableEntrySize(hdrInfo.table_enc);
   pint_t tableEntry;


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

Re: [libunwind] r352016 - [libunwind] Don't abort if encoutering invalid .eh_frame_hdr

[Hans Wennborg via cfe-commits]

It seems like a good change, so merged in r353287. Please let me know
if you have any concerns.

On Tue, Jan 29, 2019 at 3:11 PM Hans Wennborg  wrote:
>
> Should we merge this to 8.0?
>
> On Wed, Jan 23, 2019 at 10:04 PM Petr Hosek via cfe-commits
>  wrote:
> >
> > Author: phosek
> > Date: Wed Jan 23 19:04:42 2019
> > New Revision: 352016
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=352016&view=rev
> > Log:
> > [libunwind] Don't abort if encoutering invalid .eh_frame_hdr
> >
> > Recent Linux kernel release has introduced a bug as part of the ORC
> > rollout where the vDSO has a valid .eh_frame section, but it's missing
> > the .eh_frame_hdr section and GNU_EH_FRAME segment has zero size. This
> > causes libunwind to abort which breaks programs that use libunwind.
> >
> > The other unwinder implementation (libgcc, non-gnu) instead silently
> > bail out unless being compiled as debug. This change modifies libunwind
> > to use the same strategy.
> >
> > Differential Revision: https://reviews.llvm.org/D57081
> >
> > Modified:
> > libunwind/trunk/src/AddressSpace.hpp
> > libunwind/trunk/src/EHHeaderParser.hpp
> >
> > Modified: libunwind/trunk/src/AddressSpace.hpp
> > URL: 
> > http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/AddressSpace.hpp?rev=352016&r1=352015&r2=352016&view=diff
> > ==
> > --- libunwind/trunk/src/AddressSpace.hpp (original)
> > +++ libunwind/trunk/src/AddressSpace.hpp Wed Jan 23 19:04:42 2019
> > @@ -535,11 +535,11 @@ inline bool LocalAddressSpace::findUnwin
> >  #endif
> >  cbdata->sects->dwarf_index_section = eh_frame_hdr_start;
> >  cbdata->sects->dwarf_index_section_length = phdr->p_memsz;
> > -EHHeaderParser::decodeEHHdr(
> > +found_hdr = EHHeaderParser::decodeEHHdr(
> >  *cbdata->addressSpace, eh_frame_hdr_start, phdr->p_memsz,
> >  hdrInfo);
> > -cbdata->sects->dwarf_section = hdrInfo.eh_frame_ptr;
> > -found_hdr = true;
> > +if (found_hdr)
> > +  cbdata->sects->dwarf_section = hdrInfo.eh_frame_ptr;
> >}
> >  }
> >
> >
> > Modified: libunwind/trunk/src/EHHeaderParser.hpp
> > URL: 
> > http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/EHHeaderParser.hpp?rev=352016&r1=352015&r2=352016&view=diff
> > ==
> > --- libunwind/trunk/src/EHHeaderParser.hpp (original)
> > +++ libunwind/trunk/src/EHHeaderParser.hpp Wed Jan 23 19:04:42 2019
> > @@ -35,7 +35,7 @@ public:
> >  uint8_t table_enc;
> >};
> >
> > -  static void decodeEHHdr(A &addressSpace, pint_t ehHdrStart, pint_t 
> > ehHdrEnd,
> > +  static bool decodeEHHdr(A &addressSpace, pint_t ehHdrStart, pint_t 
> > ehHdrEnd,
> >EHHeaderInfo &ehHdrInfo);
> >static bool findFDE(A &addressSpace, pint_t pc, pint_t ehHdrStart,
> >uint32_t sectionLength,
> > @@ -52,12 +52,14 @@ private:
> >  };
> >
> >  template 
> > -void EHHeaderParser::decodeEHHdr(A &addressSpace, pint_t ehHdrStart,
> > +bool EHHeaderParser::decodeEHHdr(A &addressSpace, pint_t ehHdrStart,
> >  pint_t ehHdrEnd, EHHeaderInfo 
> > &ehHdrInfo) {
> >pint_t p = ehHdrStart;
> >uint8_t version = addressSpace.get8(p++);
> > -  if (version != 1)
> > -_LIBUNWIND_ABORT("Unsupported .eh_frame_hdr version");
> > +  if (version != 1) {
> > +_LIBUNWIND_LOG0("Unsupported .eh_frame_hdr version");
> > +return false;
> > +  }
> >
> >uint8_t eh_frame_ptr_enc = addressSpace.get8(p++);
> >uint8_t fde_count_enc = addressSpace.get8(p++);
> > @@ -70,6 +72,8 @@ void EHHeaderParser::decodeEHHdr(A &a
> >? 0
> >: addressSpace.getEncodedP(p, ehHdrEnd, fde_count_enc, 
> > ehHdrStart);
> >ehHdrInfo.table = p;
> > +
> > +  return true;
> >  }
> >
> >  template 
> > @@ -101,7 +105,9 @@ bool EHHeaderParser::findFDE(A &addre
> >pint_t ehHdrEnd = ehHdrStart + sectionLength;
> >
> >EHHeaderParser::EHHeaderInfo hdrInfo;
> > -  EHHeaderParser::decodeEHHdr(addressSpace, ehHdrStart, ehHdrEnd, 
> > hdrInfo);
> > +  if (!EHHeaderParser::decodeEHHdr(addressSpace, ehHdrStart, ehHdrEnd,
> > +  hdrInfo))
> > +return false;
> >
> >size_t tableEntrySize = getTableEntrySize(hdrInfo.table_enc);
> >pint_t tableEntry;
> >
> >
> > ___
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57427: [CodeGen][ObjC] Fix assert on calling `__builtin_constant_p` with ObjC objects.

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

What's the status of this patch? Is it something we should merge onto the 8.0 
release branch?


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

https://reviews.llvm.org/D57427



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

[PATCH] D57768: [SYCL] Add SYCL device compilation flow.

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

In D57768#1386754 , @Naghasan wrote:

> LGTM
>
> Side note: might be good to also involve @Anastasia, as some of the future 
> patches will overlap with OpenCL.


Sure. I'll add @Anastasia as a reviewer to the relevant patches.

Thanks,
Alexey


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57768



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

[PATCH] D57464: Generalize method overloading on addr spaces to C++

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added a comment.

In D57464#1386614 , @rjmccall wrote:

> Moving parsed attributes between lists isn't unreasonable if that's what you 
> have to do; we already do that when processing the ObjC ARC qualifiers.  The 
> ambiguity with function attributes is pretty much inherent.


The problem isn't strictly the moving of attributes; this would normally be 
possible. However, the problem is that the attributes cannot be parsed in the 
first place, as some attributes take a member variable argument and must 
therefore be parsed after a class is finalized to get proper member lookup.

Doing this deferral in `ParseFunctionDeclarator` is not possible at the moment 
without passing down a `LateParsedAttributes` from 
`ParseCXXMemberDeclaratorBeforeInitializer`, and then further passing it into 
`ParseTypeQualifierListOpt` and `ParseGNUAttributes`. Then the member 
lookup-dependent attributes can be deferred, the AS attribute will be parsed on 
the spot, and any other attributes will be added to the `FnAttrs` for the 
function declarator.

I think this would work, but it's a larger change, and it introduces a bit of 
complexity into the declarator parsing chain (suddenly, 
ParseDeclarator/ParseDirectDeclarator/ParseFunctionDeclarator will be made 
aware of whether or not a class is being parsed).

Perhaps LateAttrs for a class could be stored in a class parsing scope and 
fetched directly in ParseFunctionDeclarator? I'm not terribly familiar with the 
scope handling part of the code.


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

https://reviews.llvm.org/D57464



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

[PATCH] D57813: [clangd] Enable clangd on ObjC in VSCode

[Ilya Biryukov via Phabricator via cfe-commits]

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

Thanks to Andreas Ostermeyer for raising this on the mailing list.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D57813

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


Index: clang-tools-extra/clangd/clients/clangd-vscode/package.json
===
--- clang-tools-extra/clangd/clients/clangd-vscode/package.json
+++ clang-tools-extra/clangd/clients/clangd-vscode/package.json
@@ -21,8 +21,10 @@
 "LLVM"
 ],
 "activationEvents": [
+"onLanguage:c",
 "onLanguage:cpp",
-"onLanguage:c"
+"onLanguage:objective-c",
+"onLanguage:objective-cpp"
 ],
 "main": "./out/src/extension",
 "scripts": {


Index: clang-tools-extra/clangd/clients/clangd-vscode/package.json
===
--- clang-tools-extra/clangd/clients/clangd-vscode/package.json
+++ clang-tools-extra/clangd/clients/clangd-vscode/package.json
@@ -21,8 +21,10 @@
 "LLVM"
 ],
 "activationEvents": [
+"onLanguage:c",
 "onLanguage:cpp",
-"onLanguage:c"
+"onLanguage:objective-c",
+"onLanguage:objective-cpp"
 ],
 "main": "./out/src/extension",
 "scripts": {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57814: [clangd] Update clangd-vscode dependencies

[Ilya Biryukov via Phabricator via cfe-commits]

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

Also add 'package-lock.json' to the repo, it's a common practice to
check in those files into the repository.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D57814

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

Index: clang-tools-extra/clangd/clients/clangd-vscode/package.json
===
--- clang-tools-extra/clangd/clients/clangd-vscode/package.json
+++ clang-tools-extra/clangd/clients/clangd-vscode/package.json
@@ -36,11 +36,11 @@
 "vscode-languageserver": "^5.2.0"
 },
 "devDependencies": {
-"typescript": "^2.0.3",
-"vscode": "^1.1.0",
-"mocha": "^2.3.3",
+"@types/mocha": "^2.2.32",
 "@types/node": "^6.0.40",
-"@types/mocha": "^2.2.32"
+"mocha": "^5.2.0",
+"typescript": "^2.0.3",
+"vscode": "^1.1.0"
 },
 "repository": {
 "type": "svn",
Index: clang-tools-extra/clangd/clients/clangd-vscode/package-lock.json
===
--- /dev/null
+++ clang-tools-extra/clangd/clients/clangd-vscode/package-lock.json
@@ -0,0 +1,2027 @@
+{
+"name": "vscode-clangd",
+"version": "0.0.10",
+"lockfileVersion": 1,
+"requires": true,
+"dependencies": {
+"@types/mocha": {
+"version": "2.2.48",
+"resolved": "https://registry.npmjs.org/@types/mocha/-/mocha-2.2.48.tgz";,
+"integrity": "sha512-nlK/iyETgafGli8Zh9zJVCTicvU3iajSkRwOh3Hhiva598CMqNJ4NcVCGMTGKpGpTYj/9R8RLzS9NAykSSCqGw==",
+"dev": true
+},
+"@types/node": {
+"version": "6.14.2",
+"resolved": "https://registry.npmjs.org/@types/node/-/node-6.14.2.tgz";,
+"integrity": "sha512-JWB3xaVfsfnFY8Ofc9rTB/op0fqqTSqy4vBcVk1LuRJvta7KTX+D//fCkiTMeLGhdr2EbFZzQjC97gvmPilk9Q==",
+"dev": true
+},
+"ajv": {
+"version": "6.8.1",
+"resolved": "https://registry.npmjs.org/ajv/-/ajv-6.8.1.tgz";,
+"integrity": "sha512-eqxCp82P+JfqL683wwsL73XmFs1eG6qjw+RD3YHx+Jll1r0jNd4dh8QG9NYAeNGA/hnZjeEDgtTskgJULbxpWQ==",
+"dev": true,
+"requires": {
+"fast-deep-equal": "^2.0.1",
+"fast-json-stable-stringify": "^2.0.0",
+"json-schema-traverse": "^0.4.1",
+"uri-js": "^4.2.2"
+}
+},
+"ansi-cyan": {
+"version": "0.1.1",
+"resolved": "https://registry.npmjs.org/ansi-cyan/-/ansi-cyan-0.1.1.tgz";,
+"integrity": "sha1-U4rlKK+JgvKK4w2G8vF0VtJgmHM=",
+"dev": true,
+"requires": {
+"ansi-wrap": "0.1.0"
+}
+},
+"ansi-red": {
+"version": "0.1.1",
+"resolved": "https://registry.npmjs.org/ansi-red/-/ansi-red-0.1.1.tgz";,
+"integrity": "sha1-jGOPnRCAgAo1PJwoyKgcpHBdlGw=",
+"dev": true,
+"requires": {
+"ansi-wrap": "0.1.0"
+}
+},
+"ansi-wrap": {
+"version": "0.1.0",
+"resolved": "https://registry.npmjs.org/ansi-wrap/-/ansi-wrap-0.1.0.tgz";,
+"integrity": "sha1-qCJQ3bABXponyoLoLqYDu/pF768=",
+"dev": true
+},
+"append-buffer": {
+"version": "1.0.2",
+"resolved": "https://registry.npmjs.org/append-buffer/-/append-buffer-1.0.2.tgz";,
+"integrity": "sha1-2CIM9GYIFSXv6lBhTz3mUU36WPE=",
+"dev": true,
+"requires": {
+"buffer-equal": "^1.0.0"
+}
+},
+"arr-diff": {
+"version": "1.1.0",
+"resolved": "https://registry.npmjs.org/arr-diff/-/arr-diff-1.1.0.tgz";,
+"integrity": "sha1-aHwydYFjWI/vfeezb6vklesaOZo=",
+"dev": true,
+"requires": {
+"arr-flatten": "^1.0.1",
+"array-slice": "^0.2.3"
+}
+},
+"arr-flatten": {
+"version": "1.1.0",
+"resolved": "https://registry.npmjs.org/arr-flatten/-/arr-flatten-1.1.0.tgz";,
+"integrity": "sha512-L3hKV5R/p5o81R7O02IGnwpDmkp6E982XhtbuwSe3O4qOtMMMtodicASA1Cny2U+aCXcNpml+m4dPsvsJ3jatg==",
+"dev": true
+},
+"arr-union": {
+"version": "2.1.0",
+"resolved": "https://registry.npmjs.org/arr-union/-/arr-union-2.1.0.tgz";,
+"integrity": "sha1-IPnqtexw9cfSFbEHexw5Fh0pLH0=",
+"dev": true
+},
+"array-differ": {
+"version": "1

[PATCH] D57814: [clangd] Update clangd-vscode dependencies

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Update the description to add some context


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57814



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

[PATCH] D57768: [SYCL] Add SYCL device compilation flow.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added subscribers: rjmccall, rsmith.
Anastasia added a comment.

In D57768#1386791 , @bader wrote:

> In D57768#1386754 , @Naghasan wrote:
>
> > LGTM
> >
> > Side note: might be good to also involve @Anastasia, as some of the future 
> > patches will overlap with OpenCL.
>
>
> Sure. I'll add @Anastasia as a reviewer to the relevant patches.
>
> Thanks,
> Alexey


Hi,

Sorry for the delay. I was planning to reply on the RFC this week after 
finishing looking at your prototype in github. Seems quite a substantial amount 
of work!

There are a number of big architectural aspects of Clang that this work 
affects. My personal feeling is that some more senior developers in Clang 
architecture should provide feedback before we go ahead with detailed reviews.

Particularly, the following needs clarification:

- SYCL seem to require adding tight dependencies from the standard libraries 
into the compiler because many language features are hidden behind library 
classes. This is not common for Clang. We had a discussion about this issue 
during the implementation of OpenCL C++ and it was decided not to go this route 
for upstream Clang. Can you explain your current approach to implement this? I 
think @rjmccall  or @rsmith might need to be involved in this.

- I am not sure how the change of direction for OpenCL C++ to just enabling C++ 
in OpenCL would affect your work now? Particularly we are establishing a lot of 
rules in the areas of interplay between OpenCL features and  C++.  Address 
space handling is one example here. As far as I am aware SYCL doesn't detail 
many of these rules either. So I am wondering how it would work... would you 
just inherit the same rules? Also keep in mind they are not documented anywhere 
yet other than the source code.

- What is your solution for integration with SPIR-V and how does it relate to 
our previous discussions in October: 
http://lists.llvm.org/pipermail/cfe-dev/2018-October/059974.html

- Can you explain the purpose of 
https://github.com/intel/llvm/tree/sycl/clang/lib/CodeGen/OclCxxRewrite that 
you are adding to Clang?

I will follow up on RFC as well so we can have a wider discussion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57768



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

[PATCH] D57739: [clangd] Format tweak's replacements.

[Haojian Wu via Phabricator via cfe-commits]

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

Move format to the tweak.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57739

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/Compiler.h
  clangd/refactor/Tweak.cpp
  clangd/refactor/Tweak.h
  clangd/refactor/tweaks/SwapIfBranches.cpp
  unittests/clangd/TweakTests.cpp

Index: unittests/clangd/TweakTests.cpp
===
--- unittests/clangd/TweakTests.cpp
+++ unittests/clangd/TweakTests.cpp
@@ -53,7 +53,8 @@
   auto CheckOver = [&](Range Selection) {
 unsigned Begin = cantFail(positionToOffset(Code.code(), Selection.start));
 unsigned End = cantFail(positionToOffset(Code.code(), Selection.end));
-auto T = prepareTweak(ID, Tweak::Selection(AST, Begin, End));
+auto T = prepareTweak(
+ID, Tweak::Selection(AST, Begin, End, clang::format::getLLVMStyle()));
 if (Available)
   EXPECT_THAT_EXPECTED(T, Succeeded())
   << "code is " << markRange(Code.code(), Selection);
@@ -93,7 +94,7 @@
   ParsedAST AST = TU.build();
   unsigned Begin = cantFail(positionToOffset(Code.code(), SelectionRng.start));
   unsigned End = cantFail(positionToOffset(Code.code(), SelectionRng.end));
-  Tweak::Selection S(AST, Begin, End);
+  Tweak::Selection S(AST, Begin, End, clang::format::getLLVMStyle());
 
   auto T = prepareTweak(ID, S);
   if (!T)
@@ -127,12 +128,40 @@
 
   llvm::StringLiteral Input = R"cpp(
 void test() {
-  ^if (true) { return 100; } else { continue; }
+  ^if (true) {
+return 100;
+  } else {
+continue;
+  }
 }
   )cpp";
   llvm::StringLiteral Output = R"cpp(
 void test() {
-  if (true) { continue; } else { return 100; }
+  if (true) {
+continue;
+  } else {
+return 100;
+  }
+}
+  )cpp";
+  checkTransform(ID, Input, Output);
+
+  Input = R"cpp(
+void test() {
+  ^if () {
+return 100;
+  } else {
+continue;
+  }
+}
+  )cpp";
+  Output = R"cpp(
+void test() {
+  if () {
+continue;
+  } else {
+return 100;
+  }
 }
   )cpp";
   checkTransform(ID, Input, Output);
@@ -144,7 +173,11 @@
   )cpp";
   Output = R"cpp(
 void test() {
-  if () { continue; } else { return 100; }
+  if () {
+continue;
+  } else {
+return 100;
+  }
 }
   )cpp";
   checkTransform(ID, Input, Output);
Index: clangd/refactor/tweaks/SwapIfBranches.cpp
===
--- clangd/refactor/tweaks/SwapIfBranches.cpp
+++ clangd/refactor/tweaks/SwapIfBranches.cpp
@@ -37,9 +37,11 @@
   const char *id() const override final;
 
   bool prepare(const Selection &Inputs) override;
-  Expected apply(const Selection &Inputs) override;
   std::string title() const override;
 
+protected:
+  Expected execute(const Selection &Inputs) override;
+
 private:
   const IfStmt *If = nullptr;
 };
@@ -60,7 +62,8 @@
  dyn_cast_or_null(If->getElse());
 }
 
-Expected SwapIfBranches::apply(const Selection &Inputs) {
+Expected
+SwapIfBranches::execute(const Selection &Inputs) {
   auto &Ctx = Inputs.AST.getASTContext();
   auto &SrcMgr = Ctx.getSourceManager();
 
Index: clangd/refactor/Tweak.h
===
--- clangd/refactor/Tweak.h
+++ clangd/refactor/Tweak.h
@@ -40,16 +40,20 @@
 public:
   /// Input to prepare and apply tweaks.
   struct Selection {
-Selection(ParsedAST &AST, unsigned RangeBegin, unsigned RangeEnd);
+Selection(ParsedAST &AST, unsigned RangeBegin, unsigned RangeEnd,
+  format::FormatStyle Style);
 /// The text of the active document.
 llvm::StringRef Code;
 /// Parsed AST of the active file.
 ParsedAST &AST;
 /// A location of the cursor in the editor.
 SourceLocation Cursor;
-// The AST nodes that were selected.
+/// The AST nodes that were selected.
 SelectionTree ASTSelection;
 // FIXME: provide a way to get sources and ASTs for other files.
+
+/// The style to format generated changes.
+format::FormatStyle Style;
   };
   virtual ~Tweak() = default;
   /// A unique id of the action, it is always equal to the name of the class
@@ -63,13 +67,18 @@
   /// should be moved into 'apply'.
   /// Returns true iff the action is available and apply() can be called on it.
   virtual bool prepare(const Selection &Sel) = 0;
-  /// Run the second stage of the action that would produce the actual changes.
-  /// EXPECTS: prepare() was called and returned true.
-  virtual Expected apply(const Selection &Sel) = 0;
+  /// Format and apply the actual changes generated from the second stage of the
+  /// action.
+  Expected apply(const Selection &Sel);
   /// A one-line title of the action that should be shown to the users in the
   /// UI.
   /// 

[PATCH] D57739: [clangd] Format tweak's replacements.

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: clangd/refactor/Tweak.h:81
+  /// EXPECTS: prepare() was called and returned true.
+  virtual Expected execute(const Selection &Sel) = 0;
 };

I think the current name is slightly better than `doApply`, I also have a few 
candidates in mind `perform`, `invoke`.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57739



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

[PATCH] D57813: [clangd] Enable clangd on Objective-C in VSCode

[Haojian Wu via Phabricator via cfe-commits]

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

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57813



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

[PATCH] D57814: [clangd] Update clangd-vscode dependencies

[Haojian Wu via Phabricator via cfe-commits]

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

This patch updates **dev** dependencies, please correct the commit title to 
avoid confusion :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57814



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

[PATCH] D57739: [clangd] Format tweak's replacements.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/refactor/Tweak.h:56
+/// The style to format generated changes.
+format::FormatStyle Style;
   };

NIT: Maybe make this a second argument of `apply`?
This would convey the idea that `execute()` should not do formatting on its own.



Comment at: clangd/refactor/Tweak.h:72
+  /// action.
+  Expected apply(const Selection &Sel);
   /// A one-line title of the action that should be shown to the users in the

Could you duplicate the `EXPECTS:` comments here? It's an important part of the 
public API.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57739



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

[PATCH] D57764: [AArch64] Add Cortex-A76 and Cortex-A76AE Support

[Oliver Stannard via Phabricator via cfe-commits]

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

LGTM, but please remember to upload with more context.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57764



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

[PATCH] D57768: [SYCL] Add SYCL device compilation flow.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev requested changes to this revision.
ABataev added a comment.
This revision now requires changes to proceed.

This definitely requires a test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57768



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

[PATCH] D57765: [ARM] Add Cortex-M35P Support

[Oliver Stannard via Phabricator via cfe-commits]

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



Comment at: test/Driver/arm-cortex-cpus.c:826
 // RUN: %clang -target arm -mcpu=cortex-m33 -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-CPUV8MMAIN %s
+// RUN: %clang -target arm -mcpu=cortex-m35p -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-CPUV8MMAIN %s
 // CHECK-CPUV8MMAIN:  "-cc1"{{.*}} "-triple" "thumbv8m.main-

I think this should also check that the correct CPU is passed to cc1, because 
you've defined a ProcessorModel in the backend.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57765



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

[PATCH] D57787: [clang-tidy] modernize-avoid-c-arrays: avoid main function (PR40604)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/modernize/AvoidCArraysCheck.cpp:35
+  const clang::DeclContext *DC = Node.getDeclContext();
+  const clang::FunctionDecl *FD = llvm::dyn_cast(DC);
+  if (!FD)

JonasToth wrote:
> lebedev.ri wrote:
> > lebedev.ri wrote:
> > > JonasToth wrote:
> > > > There is `FunctionDecl->castToDeclContext()` which is probably a better 
> > > > fit here.
> > > I'm guessing you meant `cast*From*DeclContext()`.
> > > Interesting, that function is never once used.
> > > And it uses `static_cast<>()`..
> > I'm not too sure about this.
> > Given `ParmVarDecl`, are we sure it's `DeclContext` is *always* 
> > `FunctionDecl`?
> From Doc "Represents a parameter to a function. " so i think it has always a 
> `FunctionDecl` (or subclass) as DeclContext.
> 
> Maybe that function is a relict? I just saw it in the docs too and thought it 
> makes sense to use it. No opinion on that, @aaron.ballman do you know more on 
> that?
> From Doc "Represents a parameter to a function. " so i think it has always a 
> FunctionDecl (or subclass) as DeclContext.

`ObjCMethodDecl` is not a subclass of `FunctionDecl`, yet it contains 
`ParmVarDecl` objects and is a `DeclContext`.

I would use `dyn_cast` here instead.



Comment at: docs/clang-tidy/checks/modernize-avoid-c-arrays.rst:58
+
+Similarly, the ``main()`` function is ignored. It's second and third params can
+be either ``char* argv[]`` or ``char** argv``, but can not be ``std::array<>``.

It's -> Its
params -> parameters


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57787



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

[PATCH] D57739: [clangd] Format tweak's replacements.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/refactor/Tweak.h:81
+  /// EXPECTS: prepare() was called and returned true.
+  virtual Expected execute(const Selection &Sel) = 0;
 };

hokein wrote:
> I think the current name is slightly better than `doApply`, I also have a few 
> candidates in mind `perform`, `invoke`.
`execute()` LGTM!


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57739



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

[PATCH] D57768: [SYCL] Add SYCL device compilation flow.

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

In D57768#1386924 , @ABataev wrote:

> This definitely requires a test.


@ABataev, I tried to find some tests on similar `-fcuda-is-device` and 
`-fopenmp-is-device` options, but I wasn't able to find a dedicated test. Could 
you suggest some examples testing similar functionality, please?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57768



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

[PATCH] D57739: [clangd] Format tweak's replacements.

[Haojian Wu via Phabricator via cfe-commits]

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

Address review comments.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57739

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/Compiler.h
  clangd/refactor/Tweak.cpp
  clangd/refactor/Tweak.h
  clangd/refactor/tweaks/SwapIfBranches.cpp
  unittests/clangd/TweakTests.cpp

Index: unittests/clangd/TweakTests.cpp
===
--- unittests/clangd/TweakTests.cpp
+++ unittests/clangd/TweakTests.cpp
@@ -53,7 +53,8 @@
   auto CheckOver = [&](Range Selection) {
 unsigned Begin = cantFail(positionToOffset(Code.code(), Selection.start));
 unsigned End = cantFail(positionToOffset(Code.code(), Selection.end));
-auto T = prepareTweak(ID, Tweak::Selection(AST, Begin, End));
+auto T = prepareTweak(
+ID, Tweak::Selection(AST, Begin, End));
 if (Available)
   EXPECT_THAT_EXPECTED(T, Succeeded())
   << "code is " << markRange(Code.code(), Selection);
@@ -98,7 +99,7 @@
   auto T = prepareTweak(ID, S);
   if (!T)
 return T.takeError();
-  auto Replacements = (*T)->apply(S);
+  auto Replacements = (*T)->apply(S, clang::format::getLLVMStyle());
   if (!Replacements)
 return Replacements.takeError();
   return applyAllReplacements(Code.code(), *Replacements);
@@ -127,12 +128,40 @@
 
   llvm::StringLiteral Input = R"cpp(
 void test() {
-  ^if (true) { return 100; } else { continue; }
+  ^if (true) {
+return 100;
+  } else {
+continue;
+  }
 }
   )cpp";
   llvm::StringLiteral Output = R"cpp(
 void test() {
-  if (true) { continue; } else { return 100; }
+  if (true) {
+continue;
+  } else {
+return 100;
+  }
+}
+  )cpp";
+  checkTransform(ID, Input, Output);
+
+  Input = R"cpp(
+void test() {
+  ^if () {
+return 100;
+  } else {
+continue;
+  }
+}
+  )cpp";
+  Output = R"cpp(
+void test() {
+  if () {
+continue;
+  } else {
+return 100;
+  }
 }
   )cpp";
   checkTransform(ID, Input, Output);
@@ -144,7 +173,11 @@
   )cpp";
   Output = R"cpp(
 void test() {
-  if () { continue; } else { return 100; }
+  if () {
+continue;
+  } else {
+return 100;
+  }
 }
   )cpp";
   checkTransform(ID, Input, Output);
Index: clangd/refactor/tweaks/SwapIfBranches.cpp
===
--- clangd/refactor/tweaks/SwapIfBranches.cpp
+++ clangd/refactor/tweaks/SwapIfBranches.cpp
@@ -37,9 +37,11 @@
   const char *id() const override final;
 
   bool prepare(const Selection &Inputs) override;
-  Expected apply(const Selection &Inputs) override;
   std::string title() const override;
 
+protected:
+  Expected execute(const Selection &Inputs) override;
+
 private:
   const IfStmt *If = nullptr;
 };
@@ -60,7 +62,8 @@
  dyn_cast_or_null(If->getElse());
 }
 
-Expected SwapIfBranches::apply(const Selection &Inputs) {
+Expected
+SwapIfBranches::execute(const Selection &Inputs) {
   auto &Ctx = Inputs.AST.getASTContext();
   auto &SrcMgr = Ctx.getSourceManager();
 
Index: clangd/refactor/Tweak.h
===
--- clangd/refactor/Tweak.h
+++ clangd/refactor/Tweak.h
@@ -22,6 +22,7 @@
 #include "ClangdUnit.h"
 #include "Protocol.h"
 #include "Selection.h"
+#include "clang/Format/Format.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
@@ -47,7 +48,7 @@
 ParsedAST &AST;
 /// A location of the cursor in the editor.
 SourceLocation Cursor;
-// The AST nodes that were selected.
+/// The AST nodes that were selected.
 SelectionTree ASTSelection;
 // FIXME: provide a way to get sources and ASTs for other files.
   };
@@ -63,13 +64,20 @@
   /// should be moved into 'apply'.
   /// Returns true iff the action is available and apply() can be called on it.
   virtual bool prepare(const Selection &Sel) = 0;
-  /// Run the second stage of the action that would produce the actual changes.
+  /// Format and apply the actual changes generated from the second stage of the
+  /// action.
   /// EXPECTS: prepare() was called and returned true.
-  virtual Expected apply(const Selection &Sel) = 0;
+  Expected apply(const Selection &Sel,
+const format::FormatStyle &Style);
   /// A one-line title of the action that should be shown to the users in the
   /// UI.
   /// EXPECTS: prepare() was called and returned true.
   virtual std::string title() const = 0;
+
+protected:
+  /// Run the second stage of the action that would produce the actual changes.
+  /// EXPECTS: prepare() was called and returned true.
+  virtual Expected execute(const Selection &Se

[PATCH] D57768: [SYCL] Add SYCL device compilation flow.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D57768#1386933 , @bader wrote:

> In D57768#1386924 , @ABataev wrote:
>
> > This definitely requires a test.
>
>
> @ABataev, I tried to find some tests on similar `-fcuda-is-device` and 
> `-fopenmp-is-device` options, but I wasn't able to find a dedicated test. 
> Could you suggest some examples testing similar functionality, please?


There are several similar tests:
OpenMP/driver.c, Driver/openmp-offload.c, Driver/openmp-offload-gpu.c. There is 
no absolutely the same test for OpenMP, since OpenMP has mo similar req for the 
offloading.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57768



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

[PATCH] D57739: [clangd] Format tweak's replacements.

[Haojian Wu via Phabricator via cfe-commits]

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

Add missing file.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57739

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/Compiler.h
  clangd/Format.h
  clangd/refactor/Tweak.cpp
  clangd/refactor/Tweak.h
  clangd/refactor/tweaks/SwapIfBranches.cpp
  unittests/clangd/TweakTests.cpp

Index: unittests/clangd/TweakTests.cpp
===
--- unittests/clangd/TweakTests.cpp
+++ unittests/clangd/TweakTests.cpp
@@ -53,7 +53,8 @@
   auto CheckOver = [&](Range Selection) {
 unsigned Begin = cantFail(positionToOffset(Code.code(), Selection.start));
 unsigned End = cantFail(positionToOffset(Code.code(), Selection.end));
-auto T = prepareTweak(ID, Tweak::Selection(AST, Begin, End));
+auto T = prepareTweak(
+ID, Tweak::Selection(AST, Begin, End));
 if (Available)
   EXPECT_THAT_EXPECTED(T, Succeeded())
   << "code is " << markRange(Code.code(), Selection);
@@ -98,7 +99,7 @@
   auto T = prepareTweak(ID, S);
   if (!T)
 return T.takeError();
-  auto Replacements = (*T)->apply(S);
+  auto Replacements = (*T)->apply(S, clang::format::getLLVMStyle());
   if (!Replacements)
 return Replacements.takeError();
   return applyAllReplacements(Code.code(), *Replacements);
@@ -127,12 +128,40 @@
 
   llvm::StringLiteral Input = R"cpp(
 void test() {
-  ^if (true) { return 100; } else { continue; }
+  ^if (true) {
+return 100;
+  } else {
+continue;
+  }
 }
   )cpp";
   llvm::StringLiteral Output = R"cpp(
 void test() {
-  if (true) { continue; } else { return 100; }
+  if (true) {
+continue;
+  } else {
+return 100;
+  }
+}
+  )cpp";
+  checkTransform(ID, Input, Output);
+
+  Input = R"cpp(
+void test() {
+  ^if () {
+return 100;
+  } else {
+continue;
+  }
+}
+  )cpp";
+  Output = R"cpp(
+void test() {
+  if () {
+continue;
+  } else {
+return 100;
+  }
 }
   )cpp";
   checkTransform(ID, Input, Output);
@@ -144,7 +173,11 @@
   )cpp";
   Output = R"cpp(
 void test() {
-  if () { continue; } else { return 100; }
+  if () {
+continue;
+  } else {
+return 100;
+  }
 }
   )cpp";
   checkTransform(ID, Input, Output);
Index: clangd/refactor/tweaks/SwapIfBranches.cpp
===
--- clangd/refactor/tweaks/SwapIfBranches.cpp
+++ clangd/refactor/tweaks/SwapIfBranches.cpp
@@ -37,9 +37,11 @@
   const char *id() const override final;
 
   bool prepare(const Selection &Inputs) override;
-  Expected apply(const Selection &Inputs) override;
   std::string title() const override;
 
+protected:
+  Expected execute(const Selection &Inputs) override;
+
 private:
   const IfStmt *If = nullptr;
 };
@@ -60,7 +62,8 @@
  dyn_cast_or_null(If->getElse());
 }
 
-Expected SwapIfBranches::apply(const Selection &Inputs) {
+Expected
+SwapIfBranches::execute(const Selection &Inputs) {
   auto &Ctx = Inputs.AST.getASTContext();
   auto &SrcMgr = Ctx.getSourceManager();
 
Index: clangd/refactor/Tweak.h
===
--- clangd/refactor/Tweak.h
+++ clangd/refactor/Tweak.h
@@ -22,6 +22,7 @@
 #include "ClangdUnit.h"
 #include "Protocol.h"
 #include "Selection.h"
+#include "clang/Format/Format.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
@@ -47,7 +48,7 @@
 ParsedAST &AST;
 /// A location of the cursor in the editor.
 SourceLocation Cursor;
-// The AST nodes that were selected.
+/// The AST nodes that were selected.
 SelectionTree ASTSelection;
 // FIXME: provide a way to get sources and ASTs for other files.
   };
@@ -63,13 +64,20 @@
   /// should be moved into 'apply'.
   /// Returns true iff the action is available and apply() can be called on it.
   virtual bool prepare(const Selection &Sel) = 0;
-  /// Run the second stage of the action that would produce the actual changes.
+  /// Format and apply the actual changes generated from the second stage of the
+  /// action.
   /// EXPECTS: prepare() was called and returned true.
-  virtual Expected apply(const Selection &Sel) = 0;
+  Expected apply(const Selection &Sel,
+const format::FormatStyle &Style);
   /// A one-line title of the action that should be shown to the users in the
   /// UI.
   /// EXPECTS: prepare() was called and returned true.
   virtual std::string title() const = 0;
+
+protected:
+  /// Run the second stage of the action that would produce the actual changes.
+  /// EXPECTS: prepare() was called and returned true.
+  virtual Expected execute(const Selection &Sel) = 0;
 };
 
 // All tweaks m

[PATCH] D57739: [clangd] Format tweak's replacements.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clangd/refactor/Tweak.h:56
+/// The style to format generated changes.
+format::FormatStyle Style;
   };

ilya-biryukov wrote:
> NIT: Maybe make this a second argument of `apply`?
> This would convey the idea that `execute()` should not do formatting on its 
> own.
SG, this also makes the `format` intention of `apply` more explicitly.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57739



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

[clang-tools-extra] r353295 - [clangd] Enable clangd on Objective-C in VSCode

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Wed Feb  6 05:47:49 2019
New Revision: 353295

URL: http://llvm.org/viewvc/llvm-project?rev=353295&view=rev
Log:
[clangd] Enable clangd on Objective-C in VSCode

Summary: Thanks to Andreas Ostermeyer for raising this on the mailing list.

Reviewers: hokein

Reviewed By: hokein

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

Tags: #clang

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

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

Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json?rev=353295&r1=353294&r2=353295&view=diff
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json (original)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json Wed Feb  
6 05:47:49 2019
@@ -21,8 +21,10 @@
 "LLVM"
 ],
 "activationEvents": [
+"onLanguage:c",
 "onLanguage:cpp",
-"onLanguage:c"
+"onLanguage:objective-c",
+"onLanguage:objective-cpp"
 ],
 "main": "./out/src/extension",
 "scripts": {


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

[PATCH] D57814: [clangd] Update dev dependencies of clangd-vscode

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D57814#1386902 , @hokein wrote:

> This patch updates **dev** dependencies, please correct the commit title to 
> avoid confusion :)


Done, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57814



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

[PATCH] D57813: [clangd] Enable clangd on Objective-C in VSCode

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE353295: [clangd] Enable clangd on Objective-C in VSCode 
(authored by ibiryukov, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57813?vs=185518&id=185534#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57813

Files:
  clangd/clients/clangd-vscode/package.json


Index: clangd/clients/clangd-vscode/package.json
===
--- clangd/clients/clangd-vscode/package.json
+++ clangd/clients/clangd-vscode/package.json
@@ -21,8 +21,10 @@
 "LLVM"
 ],
 "activationEvents": [
+"onLanguage:c",
 "onLanguage:cpp",
-"onLanguage:c"
+"onLanguage:objective-c",
+"onLanguage:objective-cpp"
 ],
 "main": "./out/src/extension",
 "scripts": {


Index: clangd/clients/clangd-vscode/package.json
===
--- clangd/clients/clangd-vscode/package.json
+++ clangd/clients/clangd-vscode/package.json
@@ -21,8 +21,10 @@
 "LLVM"
 ],
 "activationEvents": [
+"onLanguage:c",
 "onLanguage:cpp",
-"onLanguage:c"
+"onLanguage:objective-c",
+"onLanguage:objective-cpp"
 ],
 "main": "./out/src/extension",
 "scripts": {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57815: [clangd] Add type boost to fuzzy find in Dex.

[Eric Liu via Phabricator via cfe-commits]

ioeric created this revision.
ioeric added reviewers: sammccall, ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

No noticeable impact on code completions overall except some improvement on
cross-namespace completion.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D57815

Files:
  clangd/CodeComplete.cpp
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/dex/Dex.cpp
  clangd/index/dex/Token.h
  unittests/clangd/DexTests.cpp

Index: unittests/clangd/DexTests.cpp
===
--- unittests/clangd/DexTests.cpp
+++ unittests/clangd/DexTests.cpp
@@ -688,6 +688,28 @@
   EXPECT_THAT(Files, ElementsAre(AnyOf("foo.h", "foo.cc")));
 }
 
+TEST(DexTest, PreferredTypesBoosting) {
+  auto Sym1 = symbol("t1");
+  Sym1.Type = "T1";
+  auto Sym2 = symbol("t2");
+  Sym2.Type = "T2";
+
+  std::vector Symbols{Sym1, Sym2};
+  Dex I(Symbols, RefSlab());
+
+  FuzzyFindRequest Req;
+  Req.AnyScope = true;
+  Req.Query = "t";
+  // The best candidate can change depending on the proximity paths.
+  Req.Limit = 1;
+
+  Req.PreferredTypes = {Sym1.Type};
+  EXPECT_THAT(match(I, Req), ElementsAre("t1"));
+
+  Req.PreferredTypes = {Sym2.Type};
+  EXPECT_THAT(match(I, Req), ElementsAre("t2"));
+}
+
 } // namespace
 } // namespace dex
 } // namespace clangd
Index: clangd/index/dex/Token.h
===
--- clangd/index/dex/Token.h
+++ clangd/index/dex/Token.h
@@ -62,11 +62,11 @@
 /// Example: "file:///path/to/clang-tools-extra/clangd/index/SymbolIndex.h"
 /// and some amount of its parents.
 ProximityURI,
+/// Type of symbol (see `Symbol::Type`).
+Type,
 /// Internal Token type for invalid/special tokens, e.g. empty tokens for
 /// llvm::DenseMap.
 Sentinel,
-/// FIXME(kbobyrev): Add other Token Kinds
-/// * Type with qualified type name or its USR
   };
 
   Token(Kind TokenKind, llvm::StringRef Data)
@@ -91,6 +91,9 @@
 case Kind::ProximityURI:
   OS << "U=";
   break;
+case Kind::Type:
+  OS << "Ty=";
+  break;
 case Kind::Sentinel:
   OS << "?=";
   break;
Index: clangd/index/dex/Dex.cpp
===
--- clangd/index/dex/Dex.cpp
+++ clangd/index/dex/Dex.cpp
@@ -42,7 +42,6 @@
 // Returns the tokens which are given symbols's characteristics. For example,
 // trigrams and scopes.
 // FIXME(kbobyrev): Support more token types:
-// * Types
 // * Namespace proximity
 std::vector generateSearchTokens(const Symbol &Sym) {
   std::vector Result = generateIdentifierTrigrams(Sym.Name);
@@ -54,6 +53,8 @@
   Result.emplace_back(Token::Kind::ProximityURI, ProximityURI);
   if (Sym.Flags & Symbol::IndexedForCodeCompletion)
 Result.emplace_back(RestrictedForCodeCompletion);
+  if (!Sym.Type.empty())
+Result.emplace_back(Token::Kind::Type, Sym.Type);
   return Result;
 }
 
@@ -97,6 +98,27 @@
   return Corpus.unionOf(std::move(BoostingIterators));
 }
 
+// Constructs BOOST iterators for preferred types.
+std::unique_ptr createTypeBoostingIterator(
+llvm::ArrayRef Types,
+const llvm::DenseMap &InvertedIndex,
+const Corpus &Corpus) {
+  std::vector> BoostingIterators;
+  SymbolRelevanceSignals PreferredTypeSignals;
+  PreferredTypeSignals.TypeMatchesPreferred = true;
+  auto Boost = PreferredTypeSignals.evaluate();
+  for (const auto &T : Types) {
+Token Tok(Token::Kind::Type, T);
+const auto It = InvertedIndex.find(Tok);
+if (It != InvertedIndex.end()) {
+  BoostingIterators.push_back(
+  Corpus.boost(It->second.iterator(&It->first), Boost));
+}
+  }
+  BoostingIterators.push_back(Corpus.all());
+  return Corpus.unionOf(std::move(BoostingIterators));
+}
+
 } // namespace
 
 void Dex::buildIndex() {
@@ -176,6 +198,9 @@
   // Add proximity paths boosting (all symbols, some boosted).
   Criteria.push_back(
   createFileProximityIterator(Req.ProximityPaths, InvertedIndex, Corpus));
+  // Add boosting for preferred types.
+  Criteria.push_back(
+  createTypeBoostingIterator(Req.PreferredTypes, InvertedIndex, Corpus));
 
   if (Req.RestrictForCodeCompletion)
 Criteria.push_back(iterator(RestrictedForCodeCompletion));
Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -454,14 +454,15 @@
   /// Contextually relevant files (e.g. the file we're code-completing in).
   /// Paths should be absolute.
   std::vector ProximityPaths;
-
-  // FIXME(ibiryukov): add expected type to the request.
+  /// Preferred types of symbols.
+  std::vector PreferredTypes;
 
   bool operator==(const FuzzyFindRequest &Req) const {
 return std::tie(Query, Scopes, Limit, RestrictForCodeCompletion,
-ProximityPaths) ==
+ProximityPaths, PreferredTypes

[clang-tools-extra] r353296 - [clangd] Update dev dependencies of clangd-vscode

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Wed Feb  6 05:53:24 2019
New Revision: 353296

URL: http://llvm.org/viewvc/llvm-project?rev=353296&view=rev
Log:
[clangd] Update dev dependencies of clangd-vscode

Summary:
The version bumps are a result of running `npm audit`, which found
3 security issues in previous versions of our dependencies.

Also add 'package-lock.json' to the repo, it's a common practice to
check in those files into the repository to get consistent versions of
dependencies when running on different machines.

Reviewers: hokein

Reviewed By: hokein

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

Tags: #clang

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

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

Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/.gitignore
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/.gitignore?rev=353296&r1=353295&r2=353296&view=diff
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/.gitignore (original)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/.gitignore Wed Feb  6 
05:53:24 2019
@@ -1,3 +1,2 @@
 out
 node_modules
-package-lock.json

Added: clang-tools-extra/trunk/clangd/clients/clangd-vscode/package-lock.json
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/package-lock.json?rev=353296&view=auto
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/package-lock.json 
(added)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/package-lock.json Wed 
Feb  6 05:53:24 2019
@@ -0,0 +1,2027 @@
+{
+"name": "vscode-clangd",
+"version": "0.0.10",
+"lockfileVersion": 1,
+"requires": true,
+"dependencies": {
+"@types/mocha": {
+"version": "2.2.48",
+"resolved": 
"https://registry.npmjs.org/@types/mocha/-/mocha-2.2.48.tgz";,
+"integrity": 
"sha512-nlK/iyETgafGli8Zh9zJVCTicvU3iajSkRwOh3Hhiva598CMqNJ4NcVCGMTGKpGpTYj/9R8RLzS9NAykSSCqGw==",
+"dev": true
+},
+"@types/node": {
+"version": "6.14.2",
+"resolved": 
"https://registry.npmjs.org/@types/node/-/node-6.14.2.tgz";,
+"integrity": 
"sha512-JWB3xaVfsfnFY8Ofc9rTB/op0fqqTSqy4vBcVk1LuRJvta7KTX+D//fCkiTMeLGhdr2EbFZzQjC97gvmPilk9Q==",
+"dev": true
+},
+"ajv": {
+"version": "6.8.1",
+"resolved": "https://registry.npmjs.org/ajv/-/ajv-6.8.1.tgz";,
+"integrity": 
"sha512-eqxCp82P+JfqL683wwsL73XmFs1eG6qjw+RD3YHx+Jll1r0jNd4dh8QG9NYAeNGA/hnZjeEDgtTskgJULbxpWQ==",
+"dev": true,
+"requires": {
+"fast-deep-equal": "^2.0.1",
+"fast-json-stable-stringify": "^2.0.0",
+"json-schema-traverse": "^0.4.1",
+"uri-js": "^4.2.2"
+}
+},
+"ansi-cyan": {
+"version": "0.1.1",
+"resolved": 
"https://registry.npmjs.org/ansi-cyan/-/ansi-cyan-0.1.1.tgz";,
+"integrity": "sha1-U4rlKK+JgvKK4w2G8vF0VtJgmHM=",
+"dev": true,
+"requires": {
+"ansi-wrap": "0.1.0"
+}
+},
+"ansi-red": {
+"version": "0.1.1",
+"resolved": 
"https://registry.npmjs.org/ansi-red/-/ansi-red-0.1.1.tgz";,
+"integrity": "sha1-jGOPnRCAgAo1PJwoyKgcpHBdlGw=",
+"dev": true,
+"requires": {
+"ansi-wrap": "0.1.0"
+}
+},
+"ansi-wrap": {
+"version": "0.1.0",
+"resolved": 
"https://registry.npmjs.org/ansi-wrap/-/ansi-wrap-0.1.0.tgz";,
+"integrity": "sha1-qCJQ3bABXponyoLoLqYDu/pF768=",
+"dev": true
+},
+"append-buffer": {
+"version": "1.0.2",
+"resolved": 
"https://registry.npmjs.org/append-buffer/-/append-buffer-1.0.2.tgz";,
+"integrity": "sha1-2CIM9GYIFSXv6lBhTz3mUU36WPE=",
+"dev": true,
+"requires": {
+"buffer-equal": "^1.0.0"
+}
+},
+"arr-diff": {
+"version": "1.1.0",
+"resolved": 
"https://registry.npmjs.org/arr-diff/-/arr-diff-1.1.0.tgz";,
+"integrity": "sha1-aHwydYFjWI/vfeezb6vklesaOZo=",
+"dev": true,
+"requires": {
+"arr-flatten": "^1.0.1",
+"array-slice": "^0.2.3"
+}
+},
+"arr-flatten": {
+"version": "1.1.0",
+"resolved": 
"https://registry.npmjs.org/arr-flatten/-/arr-flatten-1.1.0.tgz";,
+

[PATCH] D57814: [clangd] Update dev dependencies of clangd-vscode

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353296: [clangd] Update dev dependencies of clangd-vscode 
(authored by ibiryukov, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57814?vs=185520&id=185536#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57814

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

Index: clang-tools-extra/trunk/clangd/clients/clangd-vscode/package-lock.json
===
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/package-lock.json
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/package-lock.json
@@ -0,0 +1,2027 @@
+{
+"name": "vscode-clangd",
+"version": "0.0.10",
+"lockfileVersion": 1,
+"requires": true,
+"dependencies": {
+"@types/mocha": {
+"version": "2.2.48",
+"resolved": "https://registry.npmjs.org/@types/mocha/-/mocha-2.2.48.tgz";,
+"integrity": "sha512-nlK/iyETgafGli8Zh9zJVCTicvU3iajSkRwOh3Hhiva598CMqNJ4NcVCGMTGKpGpTYj/9R8RLzS9NAykSSCqGw==",
+"dev": true
+},
+"@types/node": {
+"version": "6.14.2",
+"resolved": "https://registry.npmjs.org/@types/node/-/node-6.14.2.tgz";,
+"integrity": "sha512-JWB3xaVfsfnFY8Ofc9rTB/op0fqqTSqy4vBcVk1LuRJvta7KTX+D//fCkiTMeLGhdr2EbFZzQjC97gvmPilk9Q==",
+"dev": true
+},
+"ajv": {
+"version": "6.8.1",
+"resolved": "https://registry.npmjs.org/ajv/-/ajv-6.8.1.tgz";,
+"integrity": "sha512-eqxCp82P+JfqL683wwsL73XmFs1eG6qjw+RD3YHx+Jll1r0jNd4dh8QG9NYAeNGA/hnZjeEDgtTskgJULbxpWQ==",
+"dev": true,
+"requires": {
+"fast-deep-equal": "^2.0.1",
+"fast-json-stable-stringify": "^2.0.0",
+"json-schema-traverse": "^0.4.1",
+"uri-js": "^4.2.2"
+}
+},
+"ansi-cyan": {
+"version": "0.1.1",
+"resolved": "https://registry.npmjs.org/ansi-cyan/-/ansi-cyan-0.1.1.tgz";,
+"integrity": "sha1-U4rlKK+JgvKK4w2G8vF0VtJgmHM=",
+"dev": true,
+"requires": {
+"ansi-wrap": "0.1.0"
+}
+},
+"ansi-red": {
+"version": "0.1.1",
+"resolved": "https://registry.npmjs.org/ansi-red/-/ansi-red-0.1.1.tgz";,
+"integrity": "sha1-jGOPnRCAgAo1PJwoyKgcpHBdlGw=",
+"dev": true,
+"requires": {
+"ansi-wrap": "0.1.0"
+}
+},
+"ansi-wrap": {
+"version": "0.1.0",
+"resolved": "https://registry.npmjs.org/ansi-wrap/-/ansi-wrap-0.1.0.tgz";,
+"integrity": "sha1-qCJQ3bABXponyoLoLqYDu/pF768=",
+"dev": true
+},
+"append-buffer": {
+"version": "1.0.2",
+"resolved": "https://registry.npmjs.org/append-buffer/-/append-buffer-1.0.2.tgz";,
+"integrity": "sha1-2CIM9GYIFSXv6lBhTz3mUU36WPE=",
+"dev": true,
+"requires": {
+"buffer-equal": "^1.0.0"
+}
+},
+"arr-diff": {
+"version": "1.1.0",
+"resolved": "https://registry.npmjs.org/arr-diff/-/arr-diff-1.1.0.tgz";,
+"integrity": "sha1-aHwydYFjWI/vfeezb6vklesaOZo=",
+"dev": true,
+"requires": {
+"arr-flatten": "^1.0.1",
+"array-slice": "^0.2.3"
+}
+},
+"arr-flatten": {
+"version": "1.1.0",
+"resolved": "https://registry.npmjs.org/arr-flatten/-/arr-flatten-1.1.0.tgz";,
+"integrity": "sha512-L3hKV5R/p5o81R7O02IGnwpDmkp6E982XhtbuwSe3O4qOtMMMtodicASA1Cny2U+aCXcNpml+m4dPsvsJ3jatg==",
+"dev": true
+},
+"arr-union": {
+"version": "2.1.0",
+"resolved": "https://registry.npmjs.org/arr-union/-/arr-union-2.1.0.tgz";,
+"integrity": "sha1-IPnqtexw9cfSFbEHexw5Fh0pLH0=",
+"dev": true
+},
+"array-differ": {
+"version": "1.0.0",
+"resolved": "https://registry.npmjs.org/array-differ/-/array-differ-1.0.0.tgz";,
+"integrity": "sha1-7/UuN1gknTO+QCuLuOVkuytdQDE=",
+"dev": true
+},
+"array-slice": {
+"version": "0.2.3",
+"resolved": "https://registry.npmjs.org/array-slice/-/array-slice-0.2.3.tgz";,
+"integrity": "sha1-3Tz7gO15c6dRF82sabC5nshhhvU=",
+"dev": true
+},
+"array-union": {
+"version": "1.0.2",

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

[Kees Cook via Phabricator via cfe-commits]

kees added a comment.

Not sure if this is the fault of the LLVM half or the Clang half, but I'm 
seeing mis-compilations in the current patches (llvm 
ca1e713fdd4fab5273b36ba6f292a844fca4cb2d with D53765 
.185490 and clang 
01879634f01bdbfac4636ebe03b68e85b20cd664 with D56571 
.185489). My earlier builds were okay (llvm 
b1650507d25d28a03f30626843b7b133796597b4 with D53765 
.183738 and clang 
61738985ebe78eeff6cfae7f97543d3456bac25a with D56571 
.181973).

I narrowed the failure down to the kernel's move_addr_to_user() function:

  static int move_addr_to_user(struct sockaddr_storage *kaddr, int klen,
   void __user *uaddr, int __user *ulen)
  {
  int err;
  int len;
  
  BUG_ON(klen > sizeof(struct sockaddr_storage));
  err = get_user(len, ulen);
  if (err)
  return err;
  if (len > klen)
  len = klen;
  if (len < 0)
  return -EINVAL;
  if (len) {
  if (audit_sockaddr(klen, kaddr))
  return -ENOMEM;
  if (copy_to_user(uaddr, kaddr, len))
  return -EFAULT;
  }
  return __put_user(klen, ulen);
  }

Working build produces:

  81c217a0 :
  81c217a0:   e8 5b fc 3d 00  callq  82001400 
<__fentry__>
  81c217a5:   81 fe 81 00 00 00   cmp$0x81,%esi
  81c217ab:   0f 83 c8 00 00 00   jae81c21879 

  81c217b1:   55  push   %rbp
  81c217b2:   48 89 e5mov%rsp,%rbp
  81c217b5:   41 57   push   %r15
  81c217b7:   41 56   push   %r14
  81c217b9:   41 55   push   %r13
  81c217bb:   41 54   push   %r12
  81c217bd:   53  push   %rbx
  81c217be:   49 89 cemov%rcx,%r14
  81c217c1:   49 89 d7mov%rdx,%r15
  81c217c4:   41 89 f5mov%esi,%r13d
  81c217c7:   49 89 fcmov%rdi,%r12
  81c217ca:   48 c7 c7 36 35 88 82mov
$0x82883536,%rdi
  81c217d1:   be da 00 00 00  mov$0xda,%esi
  81c217d6:   e8 25 8b 5c ff  callq  811ea300 
<__might_fault>
  81c217db:   4c 89 f0mov%r14,%rax
  81c217de:   e8 2d f6 2f 00  callq  81f20e10 
<__get_user_4>
  81c217e3:   85 c0   test   %eax,%eax
  81c217e5:   75 68   jne81c2184f 

  81c217e7:   48 89 d3mov%rdx,%rbx
  81c217ea:   44 39 ebcmp%r13d,%ebx
  81c217ed:   41 0f 4f dd cmovg  %r13d,%ebx
  81c217f1:   85 db   test   %ebx,%ebx
  81c217f3:   78 65   js 81c2185a 

  81c217f5:   74 48   je 81c2183f 

  81c217f7:   65 48 8b 04 25 00 4emov%gs:0x14e00,%rax
  81c217fe:   01 00 
  81c21800:   48 8b 80 38 07 00 00mov0x738(%rax),%rax
  81c21807:   48 85 c0test   %rax,%rax
  81c2180a:   74 05   je 81c21811 

  81c2180c:   83 38 00cmpl   $0x0,(%rax)
  81c2180f:   74 50   je 81c21861 

  81c21811:   48 63 dbmovslq %ebx,%rbx
  81c21814:   4c 89 e7mov%r12,%rdi
  81c21817:   48 89 demov%rbx,%rsi
  81c2181a:   ba 01 00 00 00  mov$0x1,%edx
  81c2181f:   e8 3c 75 5f ff  callq  81218d60 
<__check_object_size>
  81c21824:   4c 89 ffmov%r15,%rdi
  81c21827:   4c 89 e6mov%r12,%rsi
  81c2182a:   48 89 damov%rbx,%rdx
  81c2182d:   e8 ae 84 99 ff  callq  815b9ce0 
<_copy_to_user>
  81c21832:   48 89 c1mov%rax,%rcx
  81c21835:   b8 f2 ff ff ff  mov$0xfff2,%eax
  81c2183a:   48 85 c9test   %rcx,%rcx
  81c2183d:   75 10   jne81c2184f 

  81c2183f:   90  nop
  81c21840:   90  nop
  81c21841:   90  nop
  81c21842:   b8 f2 ff ff ff  mov$0xfff2,%eax
  81c21847:  

[PATCH] D57815: [clangd] Add type boost to fuzzy find in Dex.

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clangd/index/Index.h:457
   std::vector ProximityPaths;
-
-  // FIXME(ibiryukov): add expected type to the request.
+  /// Preferred types of symbols.
+  std::vector PreferredTypes;

should these be OpaqueType?

I think the only reason we use StringRef in Symbol is for the non-owning aspect.

otherwise indicate encoding in comment.



Comment at: clangd/index/dex/Dex.cpp:112
+Token Tok(Token::Kind::Type, T);
+const auto It = InvertedIndex.find(Tok);
+if (It != InvertedIndex.end()) {

`BoostingIterators.push_back(Corpus.boost(iterator(Tok)))`?



Comment at: unittests/clangd/DexTests.cpp:703
+  Req.Query = "t";
+  // The best candidate can change depending on the proximity paths.
+  Req.Limit = 1;

proximity paths?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57815



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

[PATCH] D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/bugprone/ArgumentCommentCheck.cpp:228-236
+static bool isStringLiteral(const Expr *Arg) {
+  const auto *Cast = dyn_cast(Arg);
+  return Cast ? isa(Cast->getSubExpr()) : false;
+}
+
+static bool isNullPtrLiteral(const Expr *Arg) {
+  const auto *Cast = dyn_cast(Arg);

MyDeveloperDay wrote:
> aaron.ballman wrote:
> > What's going on with these? Why not `return 
> > isa(Arg->IgnoreImpCasts());` (at which point, no need for the 
> > separate functions).
> OK, my bad, I was just looking at the ast-dump on godbolt.org thinking... how 
> do I get past that ImplicitCasrExpr, learning these tricks in the AST isn't 
> always obvious despite me looking in doxygen, when you don't know what to 
> look for its hard to know..but this is a neat trick and I'm happy to learn.
No worries, I was just wondering if there was something special about a 
`FullExpr` that you didn't want to look though it for some reason. Glad to show 
you a new trick!



Comment at: clang-tidy/bugprone/ArgumentCommentCheck.cpp:1
-//===--- ArgumentCommentCheck.cpp - clang-tidy 
===//
 //

Why did this get removed?



Comment at: clang-tidy/bugprone/ArgumentCommentCheck.cpp:231
+bool ArgumentCommentCheck::shouldAddComment(const Expr *Arg) const {
+  return (CommentBoolLiterals && isa(Arg)) ||
+ (CommentIntegerLiterals && isa(Arg)) ||

I think we may want to do some additional work here. Consider:
```
#define FOO 1

void g(int);
void h(double);
void i(const char *);

void f() {
  g(FOO); // Probably don't want to suggest comments here
  g((1)); // Probably do want to suggest comments here
  h(1.0f); // Probably do want to suggest comments here
  i(__FILE__); // Probably don't want to suggest comments here
}
```
I think this code should do two things: 1) check whether the location of the 
arg is a macro expansion (if so, return false), 2) do `Arg = 
Arg->IgnoreParenImpCasts();` at the start of the function and drop the 
`Arg->IgnoreImpCasts()` for the string and pointer literal cases. However, that 
may be a bridge too far, so you should add a test case like this:
```
g(_Generic(0, int : 0)); // Definitely do not want to see comments here
```
If it turns out the above case gives the comment suggestions, then I'd go with 
`IgnoreImpCasts()` rather than `IgnoreParenImpCasts()`.



Comment at: clang-tidy/bugprone/ArgumentCommentCheck.cpp:257
 Ctx->getLangOpts());
   };
 

MyDeveloperDay wrote:
> @aaron.ballman, slight aside (and not in the code I introduced), when I run 
> clang-tidy on the code I'm sending for review, I get the following...
> 
> ```
> C:\clang\llvm\tools\clang\tools\extra\clang-tidy\bugprone\ArgumentCommentCheck.cpp:253:8:
>  warning: invalid case style for variable 'makeFileCharRange' 
> [readability-identifier-naming]
>   auto makeFileCharRange = [Ctx](SourceLocation Begin, SourceLocation End) {
>^
>MakeFileCharRange
> 
> ```
> 
> according to the .clang-tidy, a variable should be CamelCase, and a function 
> camelBack, as its a Lambda what do we consider it should be? and does this 
> really require an improvement in readability-identifier-naming? (might be 
> something I'd be happy to look at next)
> 
> ```
> Checks: 
> '-*,clang-diagnostic-*,llvm-*,misc-*,-misc-unused-parameters,readability-identifier-naming'
> CheckOptions:
>   - key: readability-identifier-naming.ClassCase
> value:   CamelCase
>   - key: readability-identifier-naming.EnumCase
> value:   CamelCase
>   - key: readability-identifier-naming.FunctionCase
> value:   camelBack
>   - key: readability-identifier-naming.MemberCase
> value:   CamelCase
>   - key: readability-identifier-naming.ParameterCase
> value:   CamelCase
>   - key: readability-identifier-naming.UnionCase
> value:   CamelCase
>   - key: readability-identifier-naming.VariableCase
> value:   CamelCase
> ```
> 
> 
> 
> 
> 
> 
> 
> 
> according to the .clang-tidy, a variable should be CamelCase, and a function 
> camelBack, as its a Lambda what do we consider it should be? and does this 
> really require an improvement in readability-identifier-naming? (might be 
> something I'd be happy to look at next)

Lambdas are variables, so I would expect those to follow the variable naming 
conventions. This is consistent with how we treat variables of function pointer 
type as well.



Comment at: clang-tidy/bugprone/ArgumentCommentCheck.h:43
 private:
   const bool StrictMode;
+  const unsigned CommentBoolLiterals : 1;

Can you include this in the bit-field packing as well (with type `unsigned`)?



Comment at: 

[PATCH] D57267: [AST] Factor out the logic of the various Expr::Ignore*

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno updated this revision to Diff 185539.
riccibruno added a comment.

Go back to using `dyn_cast`s.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57267

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

Index: lib/AST/Expr.cpp
===
--- lib/AST/Expr.cpp
+++ lib/AST/Expr.cpp
@@ -2556,185 +2556,173 @@
   return QualType();
 }
 
-Expr *Expr::IgnoreImpCasts() {
-  Expr *E = this;
-  while (true) {
-if (auto *ICE = dyn_cast(E))
-  E = ICE->getSubExpr();
-else if (auto *FE = dyn_cast(E))
-  E = FE->getSubExpr();
-else
-  break;
-  }
+static Expr *IgnoreImpCastsSingleStep(Expr *E) {
+  if (auto *ICE = dyn_cast(E))
+return ICE->getSubExpr();
+
+  if (auto *FE = dyn_cast(E))
+return FE->getSubExpr();
+
   return E;
 }
 
-Expr *Expr::IgnoreImplicit() {
-  Expr *E = this;
-  Expr *LastE = nullptr;
-  while (E != LastE) {
-LastE = E;
+static Expr *IgnoreImpCastsExtraSingleStep(Expr *E) {
+  // FIXME: Skip MaterializeTemporaryExpr and SubstNonTypeTemplateParmExpr in
+  // addition to what IgnoreImpCasts() skips to account for the current
+  // behaviour of IgnoreParenImpCasts().
+  Expr *SubE = IgnoreImpCastsSingleStep(E);
+  if (SubE != E)
+return SubE;
 
-if (auto *ICE = dyn_cast(E))
-  E = ICE->getSubExpr();
+  if (auto *MTE = dyn_cast(E))
+return MTE->GetTemporaryExpr();
 
-if (auto *FE = dyn_cast(E))
-  E = FE->getSubExpr();
+  if (auto *NTTP = dyn_cast(E))
+return NTTP->getReplacement();
 
-if (auto *MTE = dyn_cast(E))
-  E = MTE->GetTemporaryExpr();
+  return E;
+}
+
+static Expr *IgnoreCastsSingleStep(Expr *E) {
+  if (auto *CE = dyn_cast(E))
+return CE->getSubExpr();
+
+  if (auto *FE = dyn_cast(E))
+return FE->getSubExpr();
+
+  if (auto *MTE = dyn_cast(E))
+return MTE->GetTemporaryExpr();
+
+  if (auto *NTTP = dyn_cast(E))
+return NTTP->getReplacement();
 
-if (auto *BTE = dyn_cast(E))
-  E = BTE->getSubExpr();
-  }
   return E;
 }
 
-Expr *Expr::IgnoreParens() {
-  Expr *E = this;
-  while (true) {
-if (auto *PE = dyn_cast(E)) {
-  E = PE->getSubExpr();
-  continue;
-}
-if (auto *UO = dyn_cast(E)) {
-  if (UO->getOpcode() == UO_Extension) {
-E = UO->getSubExpr();
-continue;
-  }
-}
-if (auto *GSE = dyn_cast(E)) {
-  if (!GSE->isResultDependent()) {
-E = GSE->getResultExpr();
-continue;
-  }
-}
-if (auto *CE = dyn_cast(E)) {
-  if (!CE->isConditionDependent()) {
-E = CE->getChosenSubExpr();
-continue;
-  }
-}
-if (auto *CE = dyn_cast(E)) {
-  E = CE->getSubExpr();
-  continue;
-}
-return E;
-  }
+static Expr *IgnoreLValueCastsSingleStep(Expr *E) {
+  // Skip what IgnoreCastsSingleStep skips, except that only
+  // lvalue-to-rvalue casts are skipped.
+  if (auto *CE = dyn_cast(E))
+if (CE->getCastKind() != CK_LValueToRValue)
+  return E;
+
+  return IgnoreCastsSingleStep(E);
 }
 
-/// IgnoreParenCasts - Ignore parentheses and casts.  Strip off any ParenExpr
-/// or CastExprs or ImplicitCastExprs, returning their operand.
-Expr *Expr::IgnoreParenCasts() {
-  Expr *E = this;
-  while (true) {
-E = E->IgnoreParens();
-if (auto *CE = dyn_cast(E)) {
-  E = CE->getSubExpr();
-  continue;
-}
-if (auto *MTE = dyn_cast(E)) {
-  E = MTE->GetTemporaryExpr();
-  continue;
-}
-if (auto *NTTP = dyn_cast(E)) {
-  E = NTTP->getReplacement();
-  continue;
-}
-if (auto *FE = dyn_cast(E)) {
-  E = FE->getSubExpr();
-  continue;
-}
-return E;
-  }
+static Expr *IgnoreBaseCastsSingleStep(Expr *E) {
+  if (auto *CE = dyn_cast(E))
+if (CE->getCastKind() == CK_DerivedToBase ||
+CE->getCastKind() == CK_UncheckedDerivedToBase ||
+CE->getCastKind() == CK_NoOp)
+  return CE->getSubExpr();
+
+  return E;
 }
 
-Expr *Expr::IgnoreCasts() {
-  Expr *E = this;
-  while (true) {
-if (auto *CE = dyn_cast(E)) {
-  E = CE->getSubExpr();
-  continue;
-}
-if (auto *MTE = dyn_cast(E)) {
-  E = MTE->GetTemporaryExpr();
-  continue;
-}
-if (auto *NTTP = dyn_cast(E)) {
-  E = NTTP->getReplacement();
-  continue;
-}
-if (auto *FE = dyn_cast(E)) {
-  E = FE->getSubExpr();
-  continue;
-}
-return E;
+static Expr *IgnoreImplicitSingleStep(Expr *E) {
+  Expr *SubE = IgnoreImpCastsSingleStep(E);
+  if (SubE != E)
+return SubE;
+
+  if (auto *MTE = dyn_cast(E))
+return MTE->GetTemporaryExpr();
+
+  if (auto *BTE = dyn_cast(E))
+return BTE->getSubExpr();
+
+  return E;
+}
+
+static Expr *IgnoreParensSingleStep(Expr *E) {
+  if (auto *PE = dyn_cast(E))
+return PE->getSubExpr();
+
+  if (auto *UO = dyn_cast(E)) {
+if (UO->getOpcode() == UO_Extension)
+  return UO->getSubExpr();
+  }
+
+  else if (auto 

[PATCH] D45978: dllexport const variables must have external linkage.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 185541.

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

https://reviews.llvm.org/D45978

Files:
  lib/Sema/SemaDecl.cpp
  test/Sema/dllexport-1.cpp
  test/Sema/dllexport-2.cpp


Index: test/Sema/dllexport-2.cpp
===
--- /dev/null
+++ test/Sema/dllexport-2.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -triple i686-win32 -fsyntax-only -fms-extensions -verify %s
+// RUN: %clang_cc1 -triple x86_64-win32   -fsyntax-only -fms-extensions 
-verify -std=c++11 %s
+
+// Export const variable.
+
+__declspec(dllexport) int const j; // expected-error {{default initialization 
of an object of const type 'const int'}} // expected-error {{'j' must have 
external linkage when declared 'dllexport'}}
Index: test/Sema/dllexport-1.cpp
===
--- /dev/null
+++ test/Sema/dllexport-1.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple i686-win32 -fsyntax-only -fms-extensions -verify %s
+// RUN: %clang_cc1 -triple x86_64-win32   -fsyntax-only -fms-extensions 
-verify -std=c++11 %s
+
+// CHECK: @"?x@@3HB" = dso_local dllexport constant i32 3, align 4
+
+// Export const variable initialization.
+
+// expected-no-diagnostics
+__declspec(dllexport) int const x = 3;
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11367,6 +11367,16 @@
 !isTemplateInstantiation(VDecl->getTemplateSpecializationKind()))
   Diag(VDecl->getLocation(), diag::warn_extern_init);
 
+// In Microsoft C++ mode, a const variable defined in namespace scope has
+// external linkage by default if the variable is declared with
+// __declspec(dllexport).
+if (Context.getTargetInfo().getCXXABI().isMicrosoft() &&
+getLangOpts().CPlusPlus &&
+VDecl->getType().isLocalConstQualified() &&
+VDecl->hasAttr() &&
+VDecl->getDefinition())
+  VDecl->setStorageClass(SC_Extern);
+
 // C99 6.7.8p4. All file scoped initializers need to be constant.
 if (!getLangOpts().CPlusPlus && !VDecl->isInvalidDecl())
   CheckForConstantInitializer(Init, DclT);


Index: test/Sema/dllexport-2.cpp
===
--- /dev/null
+++ test/Sema/dllexport-2.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -triple i686-win32 -fsyntax-only -fms-extensions -verify %s
+// RUN: %clang_cc1 -triple x86_64-win32   -fsyntax-only -fms-extensions -verify -std=c++11 %s
+
+// Export const variable.
+
+__declspec(dllexport) int const j; // expected-error {{default initialization of an object of const type 'const int'}} // expected-error {{'j' must have external linkage when declared 'dllexport'}}
Index: test/Sema/dllexport-1.cpp
===
--- /dev/null
+++ test/Sema/dllexport-1.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple i686-win32 -fsyntax-only -fms-extensions -verify %s
+// RUN: %clang_cc1 -triple x86_64-win32   -fsyntax-only -fms-extensions -verify -std=c++11 %s
+
+// CHECK: @"?x@@3HB" = dso_local dllexport constant i32 3, align 4
+
+// Export const variable initialization.
+
+// expected-no-diagnostics
+__declspec(dllexport) int const x = 3;
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11367,6 +11367,16 @@
 !isTemplateInstantiation(VDecl->getTemplateSpecializationKind()))
   Diag(VDecl->getLocation(), diag::warn_extern_init);
 
+// In Microsoft C++ mode, a const variable defined in namespace scope has
+// external linkage by default if the variable is declared with
+// __declspec(dllexport).
+if (Context.getTargetInfo().getCXXABI().isMicrosoft() &&
+getLangOpts().CPlusPlus &&
+VDecl->getType().isLocalConstQualified() &&
+VDecl->hasAttr() &&
+VDecl->getDefinition())
+  VDecl->setStorageClass(SC_Extern);
+
 // C99 6.7.8p4. All file scoped initializers need to be constant.
 if (!getLangOpts().CPlusPlus && !VDecl->isInvalidDecl())
   CheckForConstantInitializer(Init, DclT);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45978: dllexport const variables must have external linkage.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.



> That seems like a reasonable place to try, to me.

Ok. Let's see if this does the trick. Thanks.


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

https://reviews.llvm.org/D45978



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

[PATCH] D57739: [clangd] Format tweak's replacements.

[Ilya Biryukov via Phabricator via cfe-commits]

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

LGTM! Also a few NITs.




Comment at: clangd/ClangdServer.cpp:155
 Opts.ClangTidyOpts = ClangTidyOptProvider->getOptions(File);
+  // FIXME: cache this
+  Opts.Style =

NIT: put `.` to the end of the comment.



Comment at: clangd/ClangdServer.cpp:378
   return CB(A.takeError());
 // FIXME: run formatter on top of resulting replacements.
+return CB((*A)->apply(*Selection, InpAST->Inputs.Opts.Style));

This FIXME is stale now, remove?



Comment at: clangd/Format.h:19
+inline llvm::Expected
+cleanupAndFormat(StringRef Code, const tooling::Replacements &Replaces,
+ const format::FormatStyle &Style) {

NIT: Maybe move to `SourceCode.h`? We have helpers to turn replacements into 
edits there, having this would be a good addition.
Or inline now that this function has only one call-site?

The extra file does not seem to carry its weight.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57739



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

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

[James Clarke via Phabricator via cfe-commits]

jrtc27 requested changes to this revision.
jrtc27 added inline comments.
This revision now requires changes to proceed.
Herald added a project: clang.



Comment at: test/CodeGen/riscv-inline-asm.c:32
+// CHECK-LABEL: define void @test_A(i32* %p)
+// CHECK: call void asm volatile "", "*A"(i32* %p)
+  asm volatile("" :: "A"(*p));

Should be `sideeffect`


Repository:
  rC Clang

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

https://reviews.llvm.org/D54295



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

[PATCH] D57021: [clangd] Suggest adding missing includes for typos (like include-fixer).

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 185546.
ioeric marked 15 inline comments as done.
ioeric added a comment.

- address review comments


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57021

Files:
  clangd/ClangdUnit.cpp
  clangd/IncludeFixer.cpp
  clangd/IncludeFixer.h
  clangd/SourceCode.h
  unittests/clangd/DiagnosticsTests.cpp

Index: unittests/clangd/DiagnosticsTests.cpp
===
--- unittests/clangd/DiagnosticsTests.cpp
+++ unittests/clangd/DiagnosticsTests.cpp
@@ -30,6 +30,11 @@
   return Field(&Diag::Fixes, ElementsAre(FixMatcher));
 }
 
+testing::Matcher WithFix(testing::Matcher FixMatcher1,
+   testing::Matcher FixMatcher2) {
+  return Field(&Diag::Fixes, UnorderedElementsAre(FixMatcher1, FixMatcher2));
+}
+
 testing::Matcher WithNote(testing::Matcher NoteMatcher) {
   return Field(&Diag::Notes, ElementsAre(NoteMatcher));
 }
@@ -281,6 +286,26 @@
   Pair(EqualToLSPDiag(NoteInMainLSP), IsEmpty(;
 }
 
+struct SymbolWithHeader {
+  std::string QName;
+  std::string DeclaringFile;
+  std::string IncludeHeader;
+};
+
+std::unique_ptr
+buildIndexWithSymbol(llvm::ArrayRef Syms) {
+  SymbolSlab::Builder Slab;
+  for (const auto &S : Syms) {
+Symbol Sym = cls(S.QName);
+Sym.Flags |= Symbol::IndexedForCodeCompletion;
+Sym.CanonicalDeclaration.FileURI = S.DeclaringFile.c_str();
+Sym.Definition.FileURI = S.DeclaringFile.c_str();
+Sym.IncludeHeaders.emplace_back(S.IncludeHeader, 1);
+Slab.insert(Sym);
+  }
+  return MemIndex::build(std::move(Slab).build(), RefSlab());
+}
+
 TEST(IncludeFixerTest, IncompleteType) {
   Annotations Test(R"cpp(
 $insert[[]]namespace ns {
@@ -293,15 +318,8 @@
 }
   )cpp");
   auto TU = TestTU::withCode(Test.code());
-  Symbol Sym = cls("ns::X");
-  Sym.Flags |= Symbol::IndexedForCodeCompletion;
-  Sym.CanonicalDeclaration.FileURI = "unittest:///x.h";
-  Sym.Definition.FileURI = "unittest:///x.h";
-  Sym.IncludeHeaders.emplace_back("\"x.h\"", 1);
-
-  SymbolSlab::Builder Slab;
-  Slab.insert(Sym);
-  auto Index = MemIndex::build(std::move(Slab).build(), RefSlab());
+  auto Index = buildIndexWithSymbol(
+  {SymbolWithHeader{"ns::X", "unittest:///x.h", "\"x.h\""}});
   TU.ExternalIndex = Index.get();
 
   EXPECT_THAT(
@@ -346,6 +364,65 @@
"member access into incomplete type 'ns::X'")));
 }
 
+TEST(IncludeFixerTest, Typo) {
+  Annotations Test(R"cpp(
+$insert[[]]namespace ns {
+void foo() {
+  $unqualified[[X]] x;
+}
+}
+void bar() {
+  ns::$qualified[[X]] x; // ns:: is valid.
+  ::$global[[Global]] glob;
+}
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  auto Index = buildIndexWithSymbol(
+  {SymbolWithHeader{"ns::X", "unittest:///x.h", "\"x.h\""},
+   SymbolWithHeader{"Global", "unittest:///global.h", "\"global.h\""}});
+  TU.ExternalIndex = Index.get();
+
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  UnorderedElementsAre(
+  AllOf(Diag(Test.range("unqualified"), "unknown type name 'X'"),
+WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
+"Add include \"x.h\" for symbol ns::X"))),
+  AllOf(Diag(Test.range("qualified"),
+ "no type named 'X' in namespace 'ns'"),
+WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
+"Add include \"x.h\" for symbol ns::X"))),
+  AllOf(Diag(Test.range("global"),
+ "no type named 'Global' in the global namespace"),
+WithFix(Fix(Test.range("insert"), "#include \"global.h\"\n",
+"Add include \"global.h\" for symbol Global");
+}
+
+TEST(IncludeFixerTest, MultipleMatchedSymbols) {
+  Annotations Test(R"cpp(
+$insert[[]]namespace na {
+namespace nb {
+void foo() {
+  $unqualified[[X]] x;
+}
+}
+}
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  auto Index = buildIndexWithSymbol(
+  {SymbolWithHeader{"na::X", "unittest:///a.h", "\"a.h\""},
+   SymbolWithHeader{"na::nb::X", "unittest:///b.h", "\"b.h\""}});
+  TU.ExternalIndex = Index.get();
+
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  UnorderedElementsAre(AllOf(
+  Diag(Test.range("unqualified"), "unknown type name 'X'"),
+  WithFix(Fix(Test.range("insert"), "#include \"a.h\"\n",
+  "Add include \"a.h\" for symbol na::X"),
+  Fix(Test.range("insert"), "#include \"b.h\"\n",
+  "Add include \"b.h\" for symbol na::nb::X");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/SourceCode.h
===
--- clangd/SourceCode.h
+++ clangd/SourceCode.h
@@ -47,7 +47,7 @@
 /// The returned value is in the rang

[PATCH] D57819: [clangd] Reduce number of threads used by BackgroundIndex to number of physical cores.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

clangd is using as many threads as logical cores for BackgroundIndex
by default. Due to hyper-threading this causes cache/branch-prediction misses
for AST and Preamble builts which slows them down.

This patch aims to change that default to number of physical cores to get rid of
that slow-down.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D57819

Files:
  clangd/index/Background.h


Index: clangd/index/Background.h
===
--- clangd/index/Background.h
+++ clangd/index/Background.h
@@ -67,11 +67,12 @@
   /// If BuildIndexPeriodMs is greater than 0, the symbol index will only be
   /// rebuilt periodically (one per \p BuildIndexPeriodMs); otherwise, index is
   /// rebuilt for each indexed file.
-  BackgroundIndex(Context BackgroundContext, const FileSystemProvider &,
-  const GlobalCompilationDatabase &CDB,
-  BackgroundIndexStorage::Factory IndexStorageFactory,
-  size_t BuildIndexPeriodMs = 0,
-  size_t ThreadPoolSize = llvm::hardware_concurrency());
+  BackgroundIndex(
+  Context BackgroundContext, const FileSystemProvider &,
+  const GlobalCompilationDatabase &CDB,
+  BackgroundIndexStorage::Factory IndexStorageFactory,
+  size_t BuildIndexPeriodMs = 0,
+  size_t ThreadPoolSize = llvm::heavyweight_hardware_concurrency());
   ~BackgroundIndex(); // Blocks while the current task finishes.
 
   // Enqueue translation units for indexing.


Index: clangd/index/Background.h
===
--- clangd/index/Background.h
+++ clangd/index/Background.h
@@ -67,11 +67,12 @@
   /// If BuildIndexPeriodMs is greater than 0, the symbol index will only be
   /// rebuilt periodically (one per \p BuildIndexPeriodMs); otherwise, index is
   /// rebuilt for each indexed file.
-  BackgroundIndex(Context BackgroundContext, const FileSystemProvider &,
-  const GlobalCompilationDatabase &CDB,
-  BackgroundIndexStorage::Factory IndexStorageFactory,
-  size_t BuildIndexPeriodMs = 0,
-  size_t ThreadPoolSize = llvm::hardware_concurrency());
+  BackgroundIndex(
+  Context BackgroundContext, const FileSystemProvider &,
+  const GlobalCompilationDatabase &CDB,
+  BackgroundIndexStorage::Factory IndexStorageFactory,
+  size_t BuildIndexPeriodMs = 0,
+  size_t ThreadPoolSize = llvm::heavyweight_hardware_concurrency());
   ~BackgroundIndex(); // Blocks while the current task finishes.
 
   // Enqueue translation units for indexing.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57739: [clangd] Format tweak's replacements.

[Haojian Wu via Phabricator via cfe-commits]

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

Address comments.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57739

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/Compiler.h
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/refactor/Tweak.cpp
  clangd/refactor/Tweak.h
  clangd/refactor/tweaks/SwapIfBranches.cpp
  unittests/clangd/TweakTests.cpp

Index: unittests/clangd/TweakTests.cpp
===
--- unittests/clangd/TweakTests.cpp
+++ unittests/clangd/TweakTests.cpp
@@ -53,7 +53,8 @@
   auto CheckOver = [&](Range Selection) {
 unsigned Begin = cantFail(positionToOffset(Code.code(), Selection.start));
 unsigned End = cantFail(positionToOffset(Code.code(), Selection.end));
-auto T = prepareTweak(ID, Tweak::Selection(AST, Begin, End));
+auto T = prepareTweak(
+ID, Tweak::Selection(AST, Begin, End));
 if (Available)
   EXPECT_THAT_EXPECTED(T, Succeeded())
   << "code is " << markRange(Code.code(), Selection);
@@ -98,7 +99,7 @@
   auto T = prepareTweak(ID, S);
   if (!T)
 return T.takeError();
-  auto Replacements = (*T)->apply(S);
+  auto Replacements = (*T)->apply(S, clang::format::getLLVMStyle());
   if (!Replacements)
 return Replacements.takeError();
   return applyAllReplacements(Code.code(), *Replacements);
@@ -127,12 +128,40 @@
 
   llvm::StringLiteral Input = R"cpp(
 void test() {
-  ^if (true) { return 100; } else { continue; }
+  ^if (true) {
+return 100;
+  } else {
+continue;
+  }
 }
   )cpp";
   llvm::StringLiteral Output = R"cpp(
 void test() {
-  if (true) { continue; } else { return 100; }
+  if (true) {
+continue;
+  } else {
+return 100;
+  }
+}
+  )cpp";
+  checkTransform(ID, Input, Output);
+
+  Input = R"cpp(
+void test() {
+  ^if () {
+return 100;
+  } else {
+continue;
+  }
+}
+  )cpp";
+  Output = R"cpp(
+void test() {
+  if () {
+continue;
+  } else {
+return 100;
+  }
 }
   )cpp";
   checkTransform(ID, Input, Output);
@@ -144,7 +173,11 @@
   )cpp";
   Output = R"cpp(
 void test() {
-  if () { continue; } else { return 100; }
+  if () {
+continue;
+  } else {
+return 100;
+  }
 }
   )cpp";
   checkTransform(ID, Input, Output);
Index: clangd/refactor/tweaks/SwapIfBranches.cpp
===
--- clangd/refactor/tweaks/SwapIfBranches.cpp
+++ clangd/refactor/tweaks/SwapIfBranches.cpp
@@ -37,9 +37,11 @@
   const char *id() const override final;
 
   bool prepare(const Selection &Inputs) override;
-  Expected apply(const Selection &Inputs) override;
   std::string title() const override;
 
+protected:
+  Expected execute(const Selection &Inputs) override;
+
 private:
   const IfStmt *If = nullptr;
 };
@@ -60,7 +62,8 @@
  dyn_cast_or_null(If->getElse());
 }
 
-Expected SwapIfBranches::apply(const Selection &Inputs) {
+Expected
+SwapIfBranches::execute(const Selection &Inputs) {
   auto &Ctx = Inputs.AST.getASTContext();
   auto &SrcMgr = Ctx.getSourceManager();
 
Index: clangd/refactor/Tweak.h
===
--- clangd/refactor/Tweak.h
+++ clangd/refactor/Tweak.h
@@ -22,6 +22,7 @@
 #include "ClangdUnit.h"
 #include "Protocol.h"
 #include "Selection.h"
+#include "clang/Format/Format.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
@@ -47,7 +48,7 @@
 ParsedAST &AST;
 /// A location of the cursor in the editor.
 SourceLocation Cursor;
-// The AST nodes that were selected.
+/// The AST nodes that were selected.
 SelectionTree ASTSelection;
 // FIXME: provide a way to get sources and ASTs for other files.
   };
@@ -63,13 +64,20 @@
   /// should be moved into 'apply'.
   /// Returns true iff the action is available and apply() can be called on it.
   virtual bool prepare(const Selection &Sel) = 0;
-  /// Run the second stage of the action that would produce the actual changes.
+  /// Format and apply the actual changes generated from the second stage of the
+  /// action.
   /// EXPECTS: prepare() was called and returned true.
-  virtual Expected apply(const Selection &Sel) = 0;
+  Expected apply(const Selection &Sel,
+const format::FormatStyle &Style);
   /// A one-line title of the action that should be shown to the users in the
   /// UI.
   /// EXPECTS: prepare() was called and returned true.
   virtual std::string title() const = 0;
+
+protected:
+  /// Run the second stage of the action that would produce the actual changes.
+  /// EXPECTS: prepare() was called and returned true.
+  virtu

[PATCH] D57739: [clangd] Format tweak's replacements.

[Haojian Wu via Phabricator via cfe-commits]

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

Remove accident change.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57739

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/Compiler.h
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/refactor/Tweak.cpp
  clangd/refactor/Tweak.h
  clangd/refactor/tweaks/SwapIfBranches.cpp
  unittests/clangd/TweakTests.cpp

Index: unittests/clangd/TweakTests.cpp
===
--- unittests/clangd/TweakTests.cpp
+++ unittests/clangd/TweakTests.cpp
@@ -98,7 +98,7 @@
   auto T = prepareTweak(ID, S);
   if (!T)
 return T.takeError();
-  auto Replacements = (*T)->apply(S);
+  auto Replacements = (*T)->apply(S, clang::format::getLLVMStyle());
   if (!Replacements)
 return Replacements.takeError();
   return applyAllReplacements(Code.code(), *Replacements);
@@ -127,12 +127,40 @@
 
   llvm::StringLiteral Input = R"cpp(
 void test() {
-  ^if (true) { return 100; } else { continue; }
+  ^if (true) {
+return 100;
+  } else {
+continue;
+  }
 }
   )cpp";
   llvm::StringLiteral Output = R"cpp(
 void test() {
-  if (true) { continue; } else { return 100; }
+  if (true) {
+continue;
+  } else {
+return 100;
+  }
+}
+  )cpp";
+  checkTransform(ID, Input, Output);
+
+  Input = R"cpp(
+void test() {
+  ^if () {
+return 100;
+  } else {
+continue;
+  }
+}
+  )cpp";
+  Output = R"cpp(
+void test() {
+  if () {
+continue;
+  } else {
+return 100;
+  }
 }
   )cpp";
   checkTransform(ID, Input, Output);
@@ -144,7 +172,11 @@
   )cpp";
   Output = R"cpp(
 void test() {
-  if () { continue; } else { return 100; }
+  if () {
+continue;
+  } else {
+return 100;
+  }
 }
   )cpp";
   checkTransform(ID, Input, Output);
Index: clangd/refactor/tweaks/SwapIfBranches.cpp
===
--- clangd/refactor/tweaks/SwapIfBranches.cpp
+++ clangd/refactor/tweaks/SwapIfBranches.cpp
@@ -37,9 +37,11 @@
   const char *id() const override final;
 
   bool prepare(const Selection &Inputs) override;
-  Expected apply(const Selection &Inputs) override;
   std::string title() const override;
 
+protected:
+  Expected execute(const Selection &Inputs) override;
+
 private:
   const IfStmt *If = nullptr;
 };
@@ -60,7 +62,8 @@
  dyn_cast_or_null(If->getElse());
 }
 
-Expected SwapIfBranches::apply(const Selection &Inputs) {
+Expected
+SwapIfBranches::execute(const Selection &Inputs) {
   auto &Ctx = Inputs.AST.getASTContext();
   auto &SrcMgr = Ctx.getSourceManager();
 
Index: clangd/refactor/Tweak.h
===
--- clangd/refactor/Tweak.h
+++ clangd/refactor/Tweak.h
@@ -22,6 +22,7 @@
 #include "ClangdUnit.h"
 #include "Protocol.h"
 #include "Selection.h"
+#include "clang/Format/Format.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
@@ -47,7 +48,7 @@
 ParsedAST *
 /// A location of the cursor in the editor.
 SourceLocation Cursor;
-// The AST nodes that were selected.
+/// The AST nodes that were selected.
 SelectionTree ASTSelection;
 // FIXME: provide a way to get sources and ASTs for other files.
   };
@@ -63,13 +64,20 @@
   /// should be moved into 'apply'.
   /// Returns true iff the action is available and apply() can be called on it.
   virtual bool prepare(const Selection &Sel) = 0;
-  /// Run the second stage of the action that would produce the actual changes.
+  /// Format and apply the actual changes generated from the second stage of the
+  /// action.
   /// EXPECTS: prepare() was called and returned true.
-  virtual Expected apply(const Selection &Sel) = 0;
+  Expected apply(const Selection &Sel,
+const format::FormatStyle &Style);
   /// A one-line title of the action that should be shown to the users in the
   /// UI.
   /// EXPECTS: prepare() was called and returned true.
   virtual std::string title() const = 0;
+
+protected:
+  /// Run the second stage of the action that would produce the actual changes.
+  /// EXPECTS: prepare() was called and returned true.
+  virtual Expected execute(const Selection &Sel) = 0;
 };
 
 // All tweaks must be registered in the .cpp file next to their definition.
Index: clangd/refactor/Tweak.cpp
===
--- clangd/refactor/Tweak.cpp
+++ clangd/refactor/Tweak.cpp
@@ -46,6 +46,14 @@
   Cursor = SM.getComposedLoc(SM.getMainFileID(), RangeBegin);
 }
 
+Expected Tweak::apply(const Selection &Sel,
+ const format::FormatStyle &Style) {
+  auto RawReplace

[clang-tools-extra] r353306 - [clangd] Format tweak's replacements.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Wed Feb  6 07:24:50 2019
New Revision: 353306

URL: http://llvm.org/viewvc/llvm-project?rev=353306&view=rev
Log:
[clangd] Format tweak's replacements.

Modified:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/Compiler.h
clang-tools-extra/trunk/clangd/SourceCode.cpp
clang-tools-extra/trunk/clangd/SourceCode.h
clang-tools-extra/trunk/clangd/refactor/Tweak.cpp
clang-tools-extra/trunk/clangd/refactor/Tweak.h
clang-tools-extra/trunk/clangd/refactor/tweaks/SwapIfBranches.cpp
clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=353306&r1=353305&r2=353306&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Wed Feb  6 07:24:50 2019
@@ -152,6 +152,9 @@ void ClangdServer::addDocument(PathRef F
   Opts.ClangTidyOpts = tidy::ClangTidyOptions::getDefaults();
   if (ClangTidyOptProvider)
 Opts.ClangTidyOpts = ClangTidyOptProvider->getOptions(File);
+  // FIXME: cache this.
+  Opts.Style =
+  getFormatStyleForFile(File, Contents, FSProvider.getFileSystem().get());
   Opts.SuggestMissingIncludes = SuggestMissingIncludes;
   // FIXME: some build systems like Bazel will take time to preparing
   // environment to build the file, it would be nice if we could emit a
@@ -372,8 +375,7 @@ void ClangdServer::applyTweak(PathRef Fi
 auto A = prepareTweak(TweakID, *Selection);
 if (!A)
   return CB(A.takeError());
-// FIXME: run formatter on top of resulting replacements.
-return CB((*A)->apply(*Selection));
+return CB((*A)->apply(*Selection, InpAST->Inputs.Opts.Style));
   };
   WorkScheduler.runWithAST(
   "ApplyTweak", File,

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=353306&r1=353305&r2=353306&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Wed Feb  6 07:24:50 2019
@@ -309,9 +309,8 @@ ParsedAST::build(std::unique_ptr FixIncludes;
   auto BuildDir = VFS->getCurrentWorkingDirectory();
   if (Opts.SuggestMissingIncludes && Index && !BuildDir.getError()) {
-auto Style = getFormatStyleForFile(MainInput.getFile(), Content, 
VFS.get());
 auto Inserter = std::make_shared(
-MainInput.getFile(), Content, Style, BuildDir.get(),
+MainInput.getFile(), Content, Opts.Style, BuildDir.get(),
 Clang->getPreprocessor().getHeaderSearchInfo());
 if (Preamble) {
   for (const auto &Inc : Preamble->Includes.MainFileIncludes)

Modified: clang-tools-extra/trunk/clangd/Compiler.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Compiler.h?rev=353306&r1=353305&r2=353306&view=diff
==
--- clang-tools-extra/trunk/clangd/Compiler.h (original)
+++ clang-tools-extra/trunk/clangd/Compiler.h Wed Feb  6 07:24:50 2019
@@ -17,6 +17,7 @@
 
 #include "../clang-tidy/ClangTidyOptions.h"
 #include "index/Index.h"
+#include "clang/Format/Format.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/PrecompiledPreamble.h"
@@ -38,6 +39,7 @@ public:
 struct ParseOptions {
   tidy::ClangTidyOptions ClangTidyOpts;
   bool SuggestMissingIncludes = false;
+  format::FormatStyle Style;
 };
 
 /// Information required to run clang, e.g. to parse AST or do code completion.

Modified: clang-tools-extra/trunk/clangd/SourceCode.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.cpp?rev=353306&r1=353305&r2=353306&view=diff
==
--- clang-tools-extra/trunk/clangd/SourceCode.cpp (original)
+++ clang-tools-extra/trunk/clangd/SourceCode.cpp Wed Feb  6 07:24:50 2019
@@ -335,5 +335,14 @@ format::FormatStyle getFormatStyleForFil
   return *Style;
 }
 
+llvm::Expected
+cleanupAndFormat(StringRef Code, const tooling::Replacements &Replaces,
+ const format::FormatStyle &Style) {
+  auto CleanReplaces = cleanupAroundReplacements(Code, Replaces, Style);
+  if (!CleanReplaces)
+return CleanReplaces;
+  return formatReplacements(Code, std::move(*CleanReplaces), Style);
+}
+
 } // namespace clangd
 } // namespace clang

Modified: clang-tools-extra/trunk/clangd/SourceCode.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.h?rev=353306&r1=353305&r2=353306&view=diff
==

[clang-tools-extra] r353309 - [clangd] Bump vscode-clangd v0.0.11

[Haojian Wu via cfe-commits]

Author: hokein
Date: Wed Feb  6 07:29:54 2019
New Revision: 353309

URL: http://llvm.org/viewvc/llvm-project?rev=353309&view=rev
Log:
[clangd] Bump vscode-clangd v0.0.11

CHANGELOG:
- activate the extension on ObjC files.

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

Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json?rev=353309&r1=353308&r2=353309&view=diff
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json (original)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json Wed Feb  
6 07:29:54 2019
@@ -2,7 +2,7 @@
 "name": "vscode-clangd",
 "displayName": "vscode-clangd",
 "description": "Clang Language Server",
-"version": "0.0.10",
+"version": "0.0.11",
 "publisher": "llvm-vs-code-extensions",
 "homepage": "https://clang.llvm.org/extra/clangd.html";,
 "engines": {


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

[PATCH] D57815: [clangd] Add type boost to fuzzy find in Dex.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 185554.
ioeric marked 4 inline comments as done.
ioeric added a comment.

- address comments plus some refactoring.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57815

Files:
  clangd/CodeComplete.cpp
  clangd/ExpectedTypes.h
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/dex/Dex.cpp
  clangd/index/dex/Dex.h
  clangd/index/dex/Token.h
  unittests/clangd/DexTests.cpp

Index: unittests/clangd/DexTests.cpp
===
--- unittests/clangd/DexTests.cpp
+++ unittests/clangd/DexTests.cpp
@@ -688,6 +688,28 @@
   EXPECT_THAT(Files, ElementsAre(AnyOf("foo.h", "foo.cc")));
 }
 
+TEST(DexTest, PreferredTypesBoosting) {
+  auto Sym1 = symbol("t1");
+  Sym1.Type = "T1";
+  auto Sym2 = symbol("t2");
+  Sym2.Type = "T2";
+
+  std::vector Symbols{Sym1, Sym2};
+  Dex I(Symbols, RefSlab());
+
+  FuzzyFindRequest Req;
+  Req.AnyScope = true;
+  Req.Query = "t";
+  // The best candidate can change depending on the preferred type.
+  Req.Limit = 1;
+
+  Req.PreferredTypes = {Sym1.Type};
+  EXPECT_THAT(match(I, Req), ElementsAre("t1"));
+
+  Req.PreferredTypes = {Sym2.Type};
+  EXPECT_THAT(match(I, Req), ElementsAre("t2"));
+}
+
 } // namespace
 } // namespace dex
 } // namespace clangd
Index: clangd/index/dex/Token.h
===
--- clangd/index/dex/Token.h
+++ clangd/index/dex/Token.h
@@ -62,11 +62,11 @@
 /// Example: "file:///path/to/clang-tools-extra/clangd/index/SymbolIndex.h"
 /// and some amount of its parents.
 ProximityURI,
+/// Type of symbol (see `Symbol::Type`).
+Type,
 /// Internal Token type for invalid/special tokens, e.g. empty tokens for
 /// llvm::DenseMap.
 Sentinel,
-/// FIXME(kbobyrev): Add other Token Kinds
-/// * Type with qualified type name or its USR
   };
 
   Token(Kind TokenKind, llvm::StringRef Data)
@@ -91,6 +91,9 @@
 case Kind::ProximityURI:
   OS << "U=";
   break;
+case Kind::Type:
+  OS << "Ty=";
+  break;
 case Kind::Sentinel:
   OS << "?=";
   break;
Index: clangd/index/dex/Dex.h
===
--- clangd/index/dex/Dex.h
+++ clangd/index/dex/Dex.h
@@ -77,6 +77,10 @@
 private:
   void buildIndex();
   std::unique_ptr iterator(const Token &Tok) const;
+  std::unique_ptr
+  createFileProximityIterator(llvm::ArrayRef ProximityPaths) const;
+  std::unique_ptr
+  createTypeBoostingIterator(llvm::ArrayRef Types) const;
 
   /// Stores symbols sorted in the descending order of symbol quality..
   std::vector Symbols;
Index: clangd/index/dex/Dex.cpp
===
--- clangd/index/dex/Dex.cpp
+++ clangd/index/dex/Dex.cpp
@@ -42,7 +42,6 @@
 // Returns the tokens which are given symbols's characteristics. For example,
 // trigrams and scopes.
 // FIXME(kbobyrev): Support more token types:
-// * Types
 // * Namespace proximity
 std::vector generateSearchTokens(const Symbol &Sym) {
   std::vector Result = generateIdentifierTrigrams(Sym.Name);
@@ -54,49 +53,11 @@
   Result.emplace_back(Token::Kind::ProximityURI, ProximityURI);
   if (Sym.Flags & Symbol::IndexedForCodeCompletion)
 Result.emplace_back(RestrictedForCodeCompletion);
+  if (!Sym.Type.empty())
+Result.emplace_back(Token::Kind::Type, Sym.Type);
   return Result;
 }
 
-// Constructs BOOST iterators for Path Proximities.
-std::unique_ptr createFileProximityIterator(
-llvm::ArrayRef ProximityPaths,
-const llvm::DenseMap &InvertedIndex,
-const Corpus &Corpus) {
-  std::vector> BoostingIterators;
-  // Deduplicate parent URIs extracted from the ProximityPaths.
-  llvm::StringSet<> ParentURIs;
-  llvm::StringMap Sources;
-  for (const auto &Path : ProximityPaths) {
-Sources[Path] = SourceParams();
-auto PathURI = URI::create(Path);
-const auto PathProximityURIs = generateProximityURIs(PathURI.toString());
-for (const auto &ProximityURI : PathProximityURIs)
-  ParentURIs.insert(ProximityURI);
-  }
-  // Use SymbolRelevanceSignals for symbol relevance evaluation: use defaults
-  // for all parameters except for Proximity Path distance signal.
-  SymbolRelevanceSignals PathProximitySignals;
-  // DistanceCalculator will find the shortest distance from ProximityPaths to
-  // any URI extracted from the ProximityPaths.
-  URIDistance DistanceCalculator(Sources);
-  PathProximitySignals.FileProximityMatch = &DistanceCalculator;
-  // Try to build BOOST iterator for each Proximity Path provided by
-  // ProximityPaths. Boosting factor should depend on the distance to the
-  // Proximity Path: the closer processed path is, the higher boosting factor.
-  for (const auto &ParentURI : ParentURIs.keys()) {
-Token Tok(Token::Kind::ProximityURI, ParentURI);
-const auto It = InvertedIndex.

[clang-tools-extra] r353310 - [clangd] Add type boost to fuzzy find in Dex.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Wed Feb  6 07:36:23 2019
New Revision: 353310

URL: http://llvm.org/viewvc/llvm-project?rev=353310&view=rev
Log:
[clangd] Add type boost to fuzzy find in Dex.

Summary:
No noticeable impact on code completions overall except some improvement on
cross-namespace completion.

Reviewers: sammccall, ilya-biryukov

Reviewed By: sammccall

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/index/Index.cpp
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/dex/Dex.cpp
clang-tools-extra/trunk/clangd/index/dex/Dex.h
clang-tools-extra/trunk/clangd/index/dex/Token.h
clang-tools-extra/trunk/unittests/clangd/DexTests.cpp

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=353310&r1=353309&r2=353310&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Wed Feb  6 07:36:23 2019
@@ -1333,6 +1333,8 @@ private:
 Req.AnyScope = AllScopes;
 // FIXME: we should send multiple weighted paths here.
 Req.ProximityPaths.push_back(FileName);
+if (PreferredType)
+  Req.PreferredTypes.push_back(PreferredType->raw());
 vlog("Code complete: fuzzyFind({0:2})", toJSON(Req));
 
 if (SpecFuzzyFind)

Modified: clang-tools-extra/trunk/clangd/index/Index.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.cpp?rev=353310&r1=353309&r2=353310&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Index.cpp Wed Feb  6 07:36:23 2019
@@ -179,7 +179,8 @@ bool fromJSON(const llvm::json::Value &P
   O && O.map("Query", Request.Query) && O.map("Scopes", Request.Scopes) &&
   O.map("AnyScope", Request.AnyScope) && O.map("Limit", Limit) &&
   O.map("RestrictForCodeCompletion", Request.RestrictForCodeCompletion) &&
-  O.map("ProximityPaths", Request.ProximityPaths);
+  O.map("ProximityPaths", Request.ProximityPaths) &&
+  O.map("PreferredTypes", Request.PreferredTypes);
   if (OK && Limit <= std::numeric_limits::max())
 Request.Limit = Limit;
   return OK;
@@ -193,6 +194,7 @@ llvm::json::Value toJSON(const FuzzyFind
   {"Limit", Request.Limit},
   {"RestrictForCodeCompletion", Request.RestrictForCodeCompletion},
   {"ProximityPaths", llvm::json::Array{Request.ProximityPaths}},
+  {"PreferredTypes", llvm::json::Array{Request.PreferredTypes}},
   };
 }
 

Modified: clang-tools-extra/trunk/clangd/index/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=353310&r1=353309&r2=353310&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.h (original)
+++ clang-tools-extra/trunk/clangd/index/Index.h Wed Feb  6 07:36:23 2019
@@ -454,14 +454,15 @@ struct FuzzyFindRequest {
   /// Contextually relevant files (e.g. the file we're code-completing in).
   /// Paths should be absolute.
   std::vector ProximityPaths;
-
-  // FIXME(ibiryukov): add expected type to the request.
+  /// Preferred types of symbols. These are raw representation of `OpaqueType`.
+  std::vector PreferredTypes;
 
   bool operator==(const FuzzyFindRequest &Req) const {
 return std::tie(Query, Scopes, Limit, RestrictForCodeCompletion,
-ProximityPaths) ==
+ProximityPaths, PreferredTypes) ==
std::tie(Req.Query, Req.Scopes, Req.Limit,
-Req.RestrictForCodeCompletion, Req.ProximityPaths);
+Req.RestrictForCodeCompletion, Req.ProximityPaths,
+Req.PreferredTypes);
   }
   bool operator!=(const FuzzyFindRequest &Req) const { return !(*this == Req); 
}
 };

Modified: clang-tools-extra/trunk/clangd/index/dex/Dex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Dex.cpp?rev=353310&r1=353309&r2=353310&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/Dex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Dex.cpp Wed Feb  6 07:36:23 2019
@@ -42,7 +42,6 @@ const Token RestrictedForCodeCompletion
 // Returns the tokens which are given symbols's characteristics. For example,
 // trigrams and scopes.
 // FIXME(kbobyrev): Support more token types:
-// * Types
 // * Namespace proximity
 std::vector generateSearchTokens(const Symbol &Sym) {
   std::vector Result = generateIdentifierTrigrams(Sym.Name);
@@ -54,49 +53,11 @

[PATCH] D57815: [clangd] Add type boost to fuzzy find in Dex.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 18.
ioeric added a comment.

- revert unintended change.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57815

Files:
  clangd/CodeComplete.cpp
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/dex/Dex.cpp
  clangd/index/dex/Dex.h
  clangd/index/dex/Token.h
  unittests/clangd/DexTests.cpp

Index: unittests/clangd/DexTests.cpp
===
--- unittests/clangd/DexTests.cpp
+++ unittests/clangd/DexTests.cpp
@@ -688,6 +688,28 @@
   EXPECT_THAT(Files, ElementsAre(AnyOf("foo.h", "foo.cc")));
 }
 
+TEST(DexTest, PreferredTypesBoosting) {
+  auto Sym1 = symbol("t1");
+  Sym1.Type = "T1";
+  auto Sym2 = symbol("t2");
+  Sym2.Type = "T2";
+
+  std::vector Symbols{Sym1, Sym2};
+  Dex I(Symbols, RefSlab());
+
+  FuzzyFindRequest Req;
+  Req.AnyScope = true;
+  Req.Query = "t";
+  // The best candidate can change depending on the preferred type.
+  Req.Limit = 1;
+
+  Req.PreferredTypes = {Sym1.Type};
+  EXPECT_THAT(match(I, Req), ElementsAre("t1"));
+
+  Req.PreferredTypes = {Sym2.Type};
+  EXPECT_THAT(match(I, Req), ElementsAre("t2"));
+}
+
 } // namespace
 } // namespace dex
 } // namespace clangd
Index: clangd/index/dex/Token.h
===
--- clangd/index/dex/Token.h
+++ clangd/index/dex/Token.h
@@ -62,11 +62,11 @@
 /// Example: "file:///path/to/clang-tools-extra/clangd/index/SymbolIndex.h"
 /// and some amount of its parents.
 ProximityURI,
+/// Type of symbol (see `Symbol::Type`).
+Type,
 /// Internal Token type for invalid/special tokens, e.g. empty tokens for
 /// llvm::DenseMap.
 Sentinel,
-/// FIXME(kbobyrev): Add other Token Kinds
-/// * Type with qualified type name or its USR
   };
 
   Token(Kind TokenKind, llvm::StringRef Data)
@@ -91,6 +91,9 @@
 case Kind::ProximityURI:
   OS << "U=";
   break;
+case Kind::Type:
+  OS << "Ty=";
+  break;
 case Kind::Sentinel:
   OS << "?=";
   break;
Index: clangd/index/dex/Dex.h
===
--- clangd/index/dex/Dex.h
+++ clangd/index/dex/Dex.h
@@ -77,6 +77,10 @@
 private:
   void buildIndex();
   std::unique_ptr iterator(const Token &Tok) const;
+  std::unique_ptr
+  createFileProximityIterator(llvm::ArrayRef ProximityPaths) const;
+  std::unique_ptr
+  createTypeBoostingIterator(llvm::ArrayRef Types) const;
 
   /// Stores symbols sorted in the descending order of symbol quality..
   std::vector Symbols;
Index: clangd/index/dex/Dex.cpp
===
--- clangd/index/dex/Dex.cpp
+++ clangd/index/dex/Dex.cpp
@@ -42,7 +42,6 @@
 // Returns the tokens which are given symbols's characteristics. For example,
 // trigrams and scopes.
 // FIXME(kbobyrev): Support more token types:
-// * Types
 // * Namespace proximity
 std::vector generateSearchTokens(const Symbol &Sym) {
   std::vector Result = generateIdentifierTrigrams(Sym.Name);
@@ -54,49 +53,11 @@
   Result.emplace_back(Token::Kind::ProximityURI, ProximityURI);
   if (Sym.Flags & Symbol::IndexedForCodeCompletion)
 Result.emplace_back(RestrictedForCodeCompletion);
+  if (!Sym.Type.empty())
+Result.emplace_back(Token::Kind::Type, Sym.Type);
   return Result;
 }
 
-// Constructs BOOST iterators for Path Proximities.
-std::unique_ptr createFileProximityIterator(
-llvm::ArrayRef ProximityPaths,
-const llvm::DenseMap &InvertedIndex,
-const Corpus &Corpus) {
-  std::vector> BoostingIterators;
-  // Deduplicate parent URIs extracted from the ProximityPaths.
-  llvm::StringSet<> ParentURIs;
-  llvm::StringMap Sources;
-  for (const auto &Path : ProximityPaths) {
-Sources[Path] = SourceParams();
-auto PathURI = URI::create(Path);
-const auto PathProximityURIs = generateProximityURIs(PathURI.toString());
-for (const auto &ProximityURI : PathProximityURIs)
-  ParentURIs.insert(ProximityURI);
-  }
-  // Use SymbolRelevanceSignals for symbol relevance evaluation: use defaults
-  // for all parameters except for Proximity Path distance signal.
-  SymbolRelevanceSignals PathProximitySignals;
-  // DistanceCalculator will find the shortest distance from ProximityPaths to
-  // any URI extracted from the ProximityPaths.
-  URIDistance DistanceCalculator(Sources);
-  PathProximitySignals.FileProximityMatch = &DistanceCalculator;
-  // Try to build BOOST iterator for each Proximity Path provided by
-  // ProximityPaths. Boosting factor should depend on the distance to the
-  // Proximity Path: the closer processed path is, the higher boosting factor.
-  for (const auto &ParentURI : ParentURIs.keys()) {
-Token Tok(Token::Kind::ProximityURI, ParentURI);
-const auto It = InvertedIndex.find(Tok);
-if (It != InvertedIndex.end()) {
-  // FIXME(kbobyrev): Appe

[PATCH] D57815: [clangd] Add type boost to fuzzy find in Dex.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE353310: [clangd] Add type boost to fuzzy find in Dex. 
(authored by ioeric, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57815?vs=18&id=185556#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57815

Files:
  clangd/CodeComplete.cpp
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/dex/Dex.cpp
  clangd/index/dex/Dex.h
  clangd/index/dex/Token.h
  unittests/clangd/DexTests.cpp

Index: unittests/clangd/DexTests.cpp
===
--- unittests/clangd/DexTests.cpp
+++ unittests/clangd/DexTests.cpp
@@ -688,6 +688,28 @@
   EXPECT_THAT(Files, ElementsAre(AnyOf("foo.h", "foo.cc")));
 }
 
+TEST(DexTest, PreferredTypesBoosting) {
+  auto Sym1 = symbol("t1");
+  Sym1.Type = "T1";
+  auto Sym2 = symbol("t2");
+  Sym2.Type = "T2";
+
+  std::vector Symbols{Sym1, Sym2};
+  Dex I(Symbols, RefSlab());
+
+  FuzzyFindRequest Req;
+  Req.AnyScope = true;
+  Req.Query = "t";
+  // The best candidate can change depending on the preferred type.
+  Req.Limit = 1;
+
+  Req.PreferredTypes = {Sym1.Type};
+  EXPECT_THAT(match(I, Req), ElementsAre("t1"));
+
+  Req.PreferredTypes = {Sym2.Type};
+  EXPECT_THAT(match(I, Req), ElementsAre("t2"));
+}
+
 } // namespace
 } // namespace dex
 } // namespace clangd
Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -454,14 +454,15 @@
   /// Contextually relevant files (e.g. the file we're code-completing in).
   /// Paths should be absolute.
   std::vector ProximityPaths;
-
-  // FIXME(ibiryukov): add expected type to the request.
+  /// Preferred types of symbols. These are raw representation of `OpaqueType`.
+  std::vector PreferredTypes;
 
   bool operator==(const FuzzyFindRequest &Req) const {
 return std::tie(Query, Scopes, Limit, RestrictForCodeCompletion,
-ProximityPaths) ==
+ProximityPaths, PreferredTypes) ==
std::tie(Req.Query, Req.Scopes, Req.Limit,
-Req.RestrictForCodeCompletion, Req.ProximityPaths);
+Req.RestrictForCodeCompletion, Req.ProximityPaths,
+Req.PreferredTypes);
   }
   bool operator!=(const FuzzyFindRequest &Req) const { return !(*this == Req); }
 };
Index: clangd/index/Index.cpp
===
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -179,7 +179,8 @@
   O && O.map("Query", Request.Query) && O.map("Scopes", Request.Scopes) &&
   O.map("AnyScope", Request.AnyScope) && O.map("Limit", Limit) &&
   O.map("RestrictForCodeCompletion", Request.RestrictForCodeCompletion) &&
-  O.map("ProximityPaths", Request.ProximityPaths);
+  O.map("ProximityPaths", Request.ProximityPaths) &&
+  O.map("PreferredTypes", Request.PreferredTypes);
   if (OK && Limit <= std::numeric_limits::max())
 Request.Limit = Limit;
   return OK;
@@ -193,6 +194,7 @@
   {"Limit", Request.Limit},
   {"RestrictForCodeCompletion", Request.RestrictForCodeCompletion},
   {"ProximityPaths", llvm::json::Array{Request.ProximityPaths}},
+  {"PreferredTypes", llvm::json::Array{Request.PreferredTypes}},
   };
 }
 
Index: clangd/index/dex/Token.h
===
--- clangd/index/dex/Token.h
+++ clangd/index/dex/Token.h
@@ -62,11 +62,11 @@
 /// Example: "file:///path/to/clang-tools-extra/clangd/index/SymbolIndex.h"
 /// and some amount of its parents.
 ProximityURI,
+/// Type of symbol (see `Symbol::Type`).
+Type,
 /// Internal Token type for invalid/special tokens, e.g. empty tokens for
 /// llvm::DenseMap.
 Sentinel,
-/// FIXME(kbobyrev): Add other Token Kinds
-/// * Type with qualified type name or its USR
   };
 
   Token(Kind TokenKind, llvm::StringRef Data)
@@ -91,6 +91,9 @@
 case Kind::ProximityURI:
   OS << "U=";
   break;
+case Kind::Type:
+  OS << "Ty=";
+  break;
 case Kind::Sentinel:
   OS << "?=";
   break;
Index: clangd/index/dex/Dex.h
===
--- clangd/index/dex/Dex.h
+++ clangd/index/dex/Dex.h
@@ -77,6 +77,10 @@
 private:
   void buildIndex();
   std::unique_ptr iterator(const Token &Tok) const;
+  std::unique_ptr
+  createFileProximityIterator(llvm::ArrayRef ProximityPaths) const;
+  std::unique_ptr
+  createTypeBoostingIterator(llvm::ArrayRef Types) const;
 
   /// Stores symbols sorted in the descending order of symbol quality..
   std::vector Symbols;
Index: clangd/index/dex/Dex.cpp
===
--- clangd/index/dex/Dex.cpp
+++ clangd/index/dex/Dex.cpp
@@ -42,7 +42,6 @@
 // 

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

[Kees Cook via Phabricator via cfe-commits]

kees added a comment.

I reduced the C code to this:

  volatile int mystery = 0;
  
  static int noinline demo(int klen)
  {
  int err;
  int len;
  
  err = mystery * klen;
  if (err)
  return err;
  if (len > klen)
  len = klen;
  if (len < 0)
  return -EINVAL;
  if (len)
  return -ENOMEM;
  return __put_user(klen, (int __user *)NULL);
  }

Something in the __put_user() use of asm-goto is broken.


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

https://reviews.llvm.org/D56571



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

[PATCH] D57824: [OpenCL][PR40603] Align the use of extensions in C++ to be backwards compatible with OpenCL C v2.0

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.
Anastasia added reviewers: svenvh, rjmccall.
Herald added subscribers: ebevhan, jfb, yaxunl.

One of the goal of C++ mode for OpenCL is to preserve backwards compatibility 
with OpenCL C v2.0!

This patch aligns extensions!


https://reviews.llvm.org/D57824

Files:
  lib/Frontend/InitPreprocessor.cpp
  lib/Parse/ParsePragma.cpp
  lib/Sema/Sema.cpp
  test/SemaOpenCL/extension-version.cl
  test/SemaOpenCL/extensions.cl

Index: test/SemaOpenCL/extensions.cl
===
--- test/SemaOpenCL/extensions.cl
+++ test/SemaOpenCL/extensions.cl
@@ -28,6 +28,7 @@
 // enabled by default with -cl-std=CL2.0).
 //
 // RUN: %clang_cc1 %s -triple amdgcn-unknown-unknown -verify -pedantic -fsyntax-only -cl-std=CL2.0 -finclude-default-header
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-std=c++
 
 #ifdef _OPENCL_H_
 // expected-no-diagnostics
@@ -37,10 +38,14 @@
 // expected-no-diagnostics
 #endif
 
-#if __OPENCL_C_VERSION__ < 120
+#ifdef __OPENCL_CPP_VERSION__
+// expected-no-diagnostics
+#endif
+
+#if (defined(__OPENCL_C_VERSION__) && __OPENCL_C_VERSION__ < 120)
 void f1(double da) { // expected-error {{type 'double' requires cl_khr_fp64 extension}}
   double d; // expected-error {{type 'double' requires cl_khr_fp64 extension}}
-  (void) 1.0; // expected-warning {{double precision constant requires cl_khr_fp64}}
+  (void)1.0; // expected-warning {{double precision constant requires cl_khr_fp64}}
 }
 #endif
 
@@ -89,7 +94,7 @@
 // expected-warning@-2{{unsupported OpenCL extension 'cl_khr_fp64' - ignoring}}
 #endif
 
-#if __OPENCL_C_VERSION__ < 120
+#if (defined(__OPENCL_C_VERSION__) && __OPENCL_C_VERSION__ < 120)
 void f3(void) {
   double d; // expected-error {{type 'double' requires cl_khr_fp64 extension}}
 }
Index: test/SemaOpenCL/extension-version.cl
===
--- test/SemaOpenCL/extension-version.cl
+++ test/SemaOpenCL/extension-version.cl
@@ -2,12 +2,14 @@
 // RUN: %clang_cc1 -x cl -cl-std=CL1.1 %s -verify -triple spir-unknown-unknown
 // RUN: %clang_cc1 -x cl -cl-std=CL1.2 %s -verify -triple spir-unknown-unknown
 // RUN: %clang_cc1 -x cl -cl-std=CL2.0 %s -verify -triple spir-unknown-unknown
+// RUN: %clang_cc1 -x cl -cl-std=c++ %s -verify -triple spir-unknown-unknown
 // RUN: %clang_cc1 -x cl -cl-std=CL %s -verify -triple spir-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES
 // RUN: %clang_cc1 -x cl -cl-std=CL1.1 %s -verify -triple spir-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES
 // RUN: %clang_cc1 -x cl -cl-std=CL1.2 %s -verify -triple spir-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES
 // RUN: %clang_cc1 -x cl -cl-std=CL2.0 %s -verify -triple spir-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES
+// RUN: %clang_cc1 -x cl -cl-std=c++ %s -verify -triple spir-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES
 
-#if __OPENCL_C_VERSION__ >= 200 && ! defined TEST_CORE_FEATURES
+#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200) && !defined(TEST_CORE_FEATURES)
 // expected-no-diagnostics
 #endif
 
@@ -47,44 +49,44 @@
 #ifndef cl_khr_byte_addressable_store
 #error "Missing cl_khr_byte_addressable_store define"
 #endif
-#pragma OPENCL EXTENSION cl_khr_byte_addressable_store: enable
-#if (__OPENCL_C_VERSION__ >= 110) && defined TEST_CORE_FEATURES
+#pragma OPENCL EXTENSION cl_khr_byte_addressable_store : enable
+#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 110) && defined TEST_CORE_FEATURES
 // expected-warning@-2{{OpenCL extension 'cl_khr_byte_addressable_store' is core feature or supported optional core feature - ignoring}}
 #endif
 
 #ifndef cl_khr_global_int32_base_atomics
 #error "Missing cl_khr_global_int32_base_atomics define"
 #endif
-#pragma OPENCL EXTENSION cl_khr_global_int32_base_atomics: enable
-#if (__OPENCL_C_VERSION__ >= 110) && defined TEST_CORE_FEATURES
+#pragma OPENCL EXTENSION cl_khr_global_int32_base_atomics : enable
+#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 110) && defined TEST_CORE_FEATURES
 // expected-warning@-2{{OpenCL extension 'cl_khr_global_int32_base_atomics' is core feature or supported optional core feature - ignoring}}
 #endif
 
 #ifndef cl_khr_global_int32_extended_atomics
 #error "Missing cl_khr_global_int32_extended_atomics define"
 #endif
-#pragma OPENCL EXTENSION cl_khr_global_int32_extended_atomics: enable
-#if (__OPENCL_C_VERSION__ >= 110) && defined TEST_CORE_FEATURES
+#pragma OPENCL EXTENSION cl_khr_global_int32_extended_atomics : enable
+#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 110) && defined TEST_CORE_FEATURES
 // expected-warning@-2{{OpenCL extension 'cl_khr_global_int32_extended_atomics' is core feature or supported optional core feature - ignoring}}
 #endif
 
 #ifndef cl_khr_local_int32_base_atomics
 #error "Missing cl_khr_local_int32_base_at

[PATCH] D53928: Enable builtins necessary for SLEEF [AArch64] vectorized trigonometry libm functions

[Stefan Teleman via Phabricator via cfe-commits]

steleman updated this revision to Diff 185578.
steleman added a comment.
Herald added a project: clang.

This is a small - but signifcant - update to the original changeset.

What's new in this update:

${top_srcdir}/lib/Frontend/CompilerInvocation.cpp:

When -fveclib=SLEEF was passed on compile-line, neither clang nor LLVM emit any 
warnings if SLEEF is not supported on the Target architecture. SLEEF is 
currently supported only on AArch64. So, for other ISA's, -fveclib=SLEEF would 
be quietly ignored, without any hints.

This new version of the changeset adds a diagnostic if -fveclib=SLEEF is called 
on an ISA that doesn't (yet) support SLEEF.


Repository:
  rC Clang

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

https://reviews.llvm.org/D53928

Files:
  include/clang/Basic/Builtins.def
  include/clang/Basic/CodeGenOptions.h
  include/clang/Driver/Options.td
  lib/CodeGen/BackendUtil.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Frontend/CompilerInvocation.cpp

Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1407,7 +1407,7 @@
   Group, Flags<[CC1Option]>,
   HelpText<"Disables an experimental new pass manager in LLVM.">;
 def fveclib : Joined<["-"], "fveclib=">, Group, Flags<[CC1Option]>,
-HelpText<"Use the given vector functions library">, Values<"Accelerate,SVML,none">;
+HelpText<"Use the given vector functions library">, Values<"Accelerate,SVML,SLEEF,none">;
 def fno_lax_vector_conversions : Flag<["-"], "fno-lax-vector-conversions">, Group,
   HelpText<"Disallow implicit conversions between vectors with a different number of elements or different element types">, Flags<[CC1Option]>;
 def fno_merge_all_constants : Flag<["-"], "fno-merge-all-constants">, Group,
Index: include/clang/Basic/CodeGenOptions.h
===
--- include/clang/Basic/CodeGenOptions.h
+++ include/clang/Basic/CodeGenOptions.h
@@ -53,7 +53,9 @@
   enum VectorLibrary {
 NoLibrary,  // Don't use any vector library.
 Accelerate, // Use the Accelerate framework.
-SVML// Intel short vector math library.
+SVML,   // Intel short vector math library.
+SLEEF   // SLEEF - SIMD Library for Evaluating Elementary Functions.
+// (Experimental).
   };
 
 
Index: include/clang/Basic/Builtins.def
===
--- include/clang/Basic/Builtins.def
+++ include/clang/Basic/Builtins.def
@@ -197,6 +197,9 @@
 BUILTIN(__builtin_exp2 , "dd"  , "Fne")
 BUILTIN(__builtin_exp2f, "ff"  , "Fne")
 BUILTIN(__builtin_exp2l, "LdLd", "Fne")
+BUILTIN(__builtin_exp10 , "dd"  , "Fne")
+BUILTIN(__builtin_exp10f, "ff"  , "Fne")
+BUILTIN(__builtin_exp10l, "LdLd", "Fne")
 BUILTIN(__builtin_expm1 , "dd", "Fne")
 BUILTIN(__builtin_expm1f, "ff", "Fne")
 BUILTIN(__builtin_expm1l, "LdLd", "Fne")
@@ -1146,6 +1149,10 @@
 LIBBUILTIN(expm1f, "ff", "fne", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(expm1l, "LdLd", "fne", "math.h", ALL_LANGUAGES)
 
+LIBBUILTIN(exp10, "dd", "fne", "math.h", ALL_LANGUAGES)
+LIBBUILTIN(exp10f, "ff", "fne", "math.h", ALL_LANGUAGES)
+LIBBUILTIN(exp10l, "LdLd", "fne", "math.h", ALL_LANGUAGES)
+
 LIBBUILTIN(fdim, "ddd", "fne", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(fdimf, "fff", "fne", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(fdiml, "LdLdLd", "fne", "math.h", ALL_LANGUAGES)
@@ -1379,10 +1386,6 @@
 LIBBUILTIN(__tanpi, "dd", "fne", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(__tanpif, "ff", "fne", "math.h", ALL_LANGUAGES)
 
-// Similarly, __exp10 is OS X only
-LIBBUILTIN(__exp10, "dd", "fne", "math.h", ALL_LANGUAGES)
-LIBBUILTIN(__exp10f, "ff", "fne", "math.h", ALL_LANGUAGES)
-
 // Blocks runtime Builtin math library functions
 LIBBUILTIN(_Block_object_assign, "vv*vC*iC", "f", "Blocks.h", ALL_LANGUAGES)
 LIBBUILTIN(_Block_object_dispose, "vvC*iC", "f", "Blocks.h", ALL_LANGUAGES)
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -670,7 +670,14 @@
   Opts.setVecLib(CodeGenOptions::Accelerate);
 else if (Name == "SVML")
   Opts.setVecLib(CodeGenOptions::SVML);
-else if (Name == "none")
+else if (Name == "SLEEF") {
+  if (Triple.getArch() == llvm::Triple::aarch64 ||
+  Triple.getArch() == llvm::Triple::aarch64_be)
+Opts.setVecLib(CodeGenOptions::SLEEF);
+  else
+Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args)
+  << Name;
+} else if (Name == "none")
   Opts.setVecLib(CodeGenOptions::NoLibrary);
 else
   Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Name;
Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp

[PATCH] D57464: Generalize method overloading on addr spaces to C++

[Anastasia Stulova via Phabricator via cfe-commits]

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

Btw, if we would implement something like this:
http://lists.llvm.org/pipermail/cfe-dev/2019-January/060798.html

Would it allow you to switch to more generic address space attributes and 
therefore help to decrease diverging from upstream too much?




Comment at: lib/Parse/ParseDeclCXX.cpp:2313
+}
+  }
 

ebevhan wrote:
> Anastasia wrote:
> > ebevhan wrote:
> > > Anastasia wrote:
> > > > ebevhan wrote:
> > > > > Anastasia wrote:
> > > > > > ebevhan wrote:
> > > > > > > Anastasia wrote:
> > > > > > > > Anastasia wrote:
> > > > > > > > > ebevhan wrote:
> > > > > > > > > > Is there a reason that the attributes are parsed here and 
> > > > > > > > > > not in `ParseFunctionDeclarator` like the rest of the 
> > > > > > > > > > ref-qualifiers (and the OpenCL++ ASes)?
> > > > > > > > > > 
> > > > > > > > > > That is possibly why the parser is getting confused with 
> > > > > > > > > > the trailing return.
> > > > > > > > > Good question! I have a feeling that this was done to unify 
> > > > > > > > > parsing of all CXX members, not just methods. For collecting 
> > > > > > > > > the method qualifiers it would certainly help making flow 
> > > > > > > > > more coherent if this was moved to `ParseFunctionDeclarator`. 
> > > > > > > > > I will try to experiment with this a bit more. What I found 
> > > > > > > > > strange that the attributes here are attached to the function 
> > > > > > > > > type itself instead of its  qualifiers. May be @rjmccall can 
> > > > > > > > > shed some more light on the overall flow...
> > > > > > > > I looked at this a bit more and it seems that all the GNU 
> > > > > > > > attributes other than addr spaces belong to the function (not 
> > > > > > > > to be applied to `this`) for example 
> > > > > > > > https://blog.timac.org/2016/0716-constructor-and-destructor-attributes/.
> > > > > > > >  It seems correct to parse them here and apply to the function 
> > > > > > > > type. Although we could separate parsing of addr space 
> > > > > > > > attribute only and move into `ParseFunctionDeclarator`.  
> > > > > > > > However this will only be needed for the function type not for 
> > > > > > > > the data members. So not sure whether it will make the flow any 
> > > > > > > > cleaner.
> > > > > > > > I looked at this a bit more and it seems that all the GNU 
> > > > > > > > attributes other than addr spaces belong to the function (not 
> > > > > > > > to be applied to this) 
> > > > > > > 
> > > > > > > Hm, I know what you mean. It's why Clang usually complains when 
> > > > > > > you try placing an AS attribute after a function declarator, 
> > > > > > > because it's trying to apply it to the function rather than the 
> > > > > > > method qualifiers.
> > > > > > > 
> > > > > > > > However this will only be needed for the function type not for 
> > > > > > > > the data members. 
> > > > > > > 
> > > > > > > But we aren't really interested in data members, are we? Like 
> > > > > > > struct fields, non-static data members cannot be qualified with 
> > > > > > > ASes (they should 'inherit' the AS from the object type), and 
> > > > > > > static ones should probably just accept ASes as part of the 
> > > > > > > member type itself.
> > > > > > > 
> > > > > > > These patches are to enable AS-qualifiers on methods, so it only 
> > > > > > > stands to reason that those ASes would be parsed along with the 
> > > > > > > other method qualifiers.
> > > > > > > 
> > > > > > > ParseFunctionDeclarator calls ParseTypeQualifierListOpt to get 
> > > > > > > the cv-qualifier-seq for the method qualifiers. Currently it's 
> > > > > > > set to `AR_NoAttributesParsed`. If we enable parsing attributes 
> > > > > > > there, then the qualifier parsing might 'eat' attributes that 
> > > > > > > were possibly meant for the function.
> > > > > > > 
> > > > > > > This is just a guess, but what I think you could do is include 
> > > > > > > attributes in the cv-qualifier parsing with 
> > > > > > > `AR_GNUAttributesParsed`, look for any AS attributes, take their 
> > > > > > > AS and mark them invalid, then move the attributes (somehow) from 
> > > > > > > the qualifier-DeclSpec to the `FnAttrs` function attribute list.
> > > > > > > 
> > > > > > > (The reason I'm concerned about where/how the qualifiers are 
> > > > > > > parsed is because this approach only works for custom ASes 
> > > > > > > applied with the GNU attribute directly. It won't work if custom 
> > > > > > > address spaces are given with a custom keyword qualifier, like in 
> > > > > > > OpenCL. If the ASes are parsed into the DeclSpec used for the 
> > > > > > > other ref-qualifiers, then both of these cases should 'just 
> > > > > > > work'.)
> > > > > > > But we aren't really interested in data members, are we? Like 
> > > > > > > struct fields, non-static data members cannot be qualified with 
> > > > > > > ASes (they should 'inherit' the A

[PATCH] D36806: Switch to cantFail(), since it does the same assertion.

[Stephen Hines via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353318: Switch to cantFail(), since it does the same 
assertion. (authored by srhines, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D36806

Files:
  cfe/trunk/lib/Tooling/Core/Replacement.cpp


Index: cfe/trunk/lib/Tooling/Core/Replacement.cpp
===
--- cfe/trunk/lib/Tooling/Core/Replacement.cpp
+++ cfe/trunk/lib/Tooling/Core/Replacement.cpp
@@ -519,12 +519,11 @@
 return MergedRanges;
   tooling::Replacements FakeReplaces;
   for (const auto &R : MergedRanges) {
-auto Err = FakeReplaces.add(Replacement(Replaces.begin()->getFilePath(),
-R.getOffset(), R.getLength(),
-std::string(R.getLength(), ' ')));
-assert(!Err &&
-   "Replacements must not conflict since ranges have been merged.");
-llvm::consumeError(std::move(Err));
+llvm::cantFail(
+FakeReplaces.add(Replacement(Replaces.begin()->getFilePath(),
+ R.getOffset(), R.getLength(),
+ std::string(R.getLength(), ' '))),
+"Replacements must not conflict since ranges have been merged.");
   }
   return FakeReplaces.merge(Replaces).getAffectedRanges();
 }


Index: cfe/trunk/lib/Tooling/Core/Replacement.cpp
===
--- cfe/trunk/lib/Tooling/Core/Replacement.cpp
+++ cfe/trunk/lib/Tooling/Core/Replacement.cpp
@@ -519,12 +519,11 @@
 return MergedRanges;
   tooling::Replacements FakeReplaces;
   for (const auto &R : MergedRanges) {
-auto Err = FakeReplaces.add(Replacement(Replaces.begin()->getFilePath(),
-R.getOffset(), R.getLength(),
-std::string(R.getLength(), ' ')));
-assert(!Err &&
-   "Replacements must not conflict since ranges have been merged.");
-llvm::consumeError(std::move(Err));
+llvm::cantFail(
+FakeReplaces.add(Replacement(Replaces.begin()->getFilePath(),
+ R.getOffset(), R.getLength(),
+ std::string(R.getLength(), ' '))),
+"Replacements must not conflict since ranges have been merged.");
   }
   return FakeReplaces.merge(Replaces).getAffectedRanges();
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r353318 - Switch to cantFail(), since it does the same assertion.

[Stephen Hines via cfe-commits]

Author: srhines
Date: Wed Feb  6 09:59:39 2019
New Revision: 353318

URL: http://llvm.org/viewvc/llvm-project?rev=353318&view=rev
Log:
Switch to cantFail(), since it does the same assertion.

Reviewers: cfe-commits, lhames

Reviewed By: lhames

Subscribers: hintonda, klimek, pirama

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

Modified:
cfe/trunk/lib/Tooling/Core/Replacement.cpp

Modified: cfe/trunk/lib/Tooling/Core/Replacement.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Core/Replacement.cpp?rev=353318&r1=353317&r2=353318&view=diff
==
--- cfe/trunk/lib/Tooling/Core/Replacement.cpp (original)
+++ cfe/trunk/lib/Tooling/Core/Replacement.cpp Wed Feb  6 09:59:39 2019
@@ -519,12 +519,11 @@ calculateRangesAfterReplacements(const R
 return MergedRanges;
   tooling::Replacements FakeReplaces;
   for (const auto &R : MergedRanges) {
-auto Err = FakeReplaces.add(Replacement(Replaces.begin()->getFilePath(),
-R.getOffset(), R.getLength(),
-std::string(R.getLength(), ' ')));
-assert(!Err &&
-   "Replacements must not conflict since ranges have been merged.");
-llvm::consumeError(std::move(Err));
+llvm::cantFail(
+FakeReplaces.add(Replacement(Replaces.begin()->getFilePath(),
+ R.getOffset(), R.getLength(),
+ std::string(R.getLength(), ' '))),
+"Replacements must not conflict since ranges have been merged.");
   }
   return FakeReplaces.merge(Replaces).getAffectedRanges();
 }


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

r353320 - Test commit. NFC

[Patrick Lyster via cfe-commits]

Author: plyster
Date: Wed Feb  6 10:18:02 2019
New Revision: 353320

URL: http://llvm.org/viewvc/llvm-project?rev=353320&view=rev
Log:
Test commit. NFC

Modified:
cfe/trunk/lib/Sema/SemaOpenMP.cpp

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=353320&r1=353319&r2=353320&view=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Wed Feb  6 10:18:02 2019
@@ -134,7 +134,7 @@ private:
 /// get the data (loop counters etc.) about enclosing loop-based construct.
 /// This data is required during codegen.
 DoacrossDependMapTy DoacrossDepends;
-/// first argument (Expr *) contains optional argument of the
+/// First argument (Expr *) contains optional argument of the
 /// 'ordered' clause, the second one is true if the regions has 'ordered'
 /// clause, false otherwise.
 llvm::Optional> OrderedRegion;


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

LLVM buildmaster will be restarted tonight

[Galina Kistanova via cfe-commits]

 Hello everyone,

LLVM buildmaster will be updated and restarted after 6PM Pacific time today.

Thanks

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

[PATCH] D57801: [opaque pointer types] Pass through function types for TLS initialization and global destructor calls.

[David Blaikie via Phabricator via cfe-commits]

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

Great (:


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57801



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

[PATCH] D57804: [opaque pointer types] Make EmitCall pass Function Types to CreateCall/Invoke.

[David Blaikie via Phabricator via cfe-commits]

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

Cool, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57804



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

[PATCH] D57787: [clang-tidy] modernize-avoid-c-arrays: avoid main function (PR40604)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 185587.
lebedev.ri marked 4 inline comments as done and an inline comment as not done.
lebedev.ri added a comment.

And back to `dyn_cast()`.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57787

Files:
  clang-tidy/modernize/AvoidCArraysCheck.cpp
  docs/clang-tidy/checks/modernize-avoid-c-arrays.rst
  test/clang-tidy/modernize-avoid-c-arrays-ignores-main.cpp
  test/clang-tidy/modernize-avoid-c-arrays-ignores-three-arg-main.cpp


Index: test/clang-tidy/modernize-avoid-c-arrays-ignores-three-arg-main.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-avoid-c-arrays-ignores-three-arg-main.cpp
@@ -0,0 +1,20 @@
+// RUN: %check_clang_tidy %s modernize-avoid-c-arrays %t
+
+int not_main(int argc, char *argv[], char *argw[]) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not declare C-style arrays, 
use std::array<> instead
+  // CHECK-MESSAGES: :[[@LINE-2]]:38: warning: do not declare C-style arrays, 
use std::array<> instead
+  int f4[] = {1, 2};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, 
use std::array<> instead
+}
+
+int main(int argc, char *argv[], char *argw[]) {
+  int f5[] = {1, 2};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, 
use std::array<> instead
+
+  auto not_main = [](int argc, char *argv[], char *argw[]) {
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: do not declare C-style 
arrays, use std::array<> instead
+// CHECK-MESSAGES: :[[@LINE-2]]:46: warning: do not declare C-style 
arrays, use std::array<> instead
+int f6[] = {1, 2};
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not declare C-style arrays, 
use std::array<> instead
+  };
+}
Index: test/clang-tidy/modernize-avoid-c-arrays-ignores-main.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-avoid-c-arrays-ignores-main.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s modernize-avoid-c-arrays %t
+
+int not_main(int argc, char *argv[]) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not declare C-style arrays, 
use std::array<> instead
+  int f4[] = {1, 2};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, 
use std::array<> instead
+}
+
+int main(int argc, char *argv[]) {
+  int f5[] = {1, 2};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, 
use std::array<> instead
+
+  auto not_main = [](int argc, char *argv[]) {
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: do not declare C-style 
arrays, use std::array<> instead
+int f6[] = {1, 2};
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not declare C-style arrays, 
use std::array<> instead
+  };
+}
Index: docs/clang-tidy/checks/modernize-avoid-c-arrays.rst
===
--- docs/clang-tidy/checks/modernize-avoid-c-arrays.rst
+++ docs/clang-tidy/checks/modernize-avoid-c-arrays.rst
@@ -54,3 +54,7 @@
   }
 
   }
+
+Similarly, the ``main()`` function is ignored. Its second and third parameters
+can be either ``char* argv[]`` or ``char** argv``, but can not be
+``std::array<>``.
Index: clang-tidy/modernize/AvoidCArraysCheck.cpp
===
--- clang-tidy/modernize/AvoidCArraysCheck.cpp
+++ clang-tidy/modernize/AvoidCArraysCheck.cpp
@@ -30,6 +30,14 @@
   return Node.isExternCContext();
 }
 
+AST_MATCHER(clang::ParmVarDecl, isArgvOfMain) {
+  const clang::DeclContext *DC = Node.getDeclContext();
+  const auto *FD = llvm::dyn_cast(DC);
+  if (!FD) // If not a FunctionDecl, then certainly not main entry function.
+return false;
+  return FD->isMain();
+}
+
 } // namespace
 
 namespace clang {
@@ -43,7 +51,8 @@
 
   Finder->addMatcher(
   typeLoc(hasValidBeginLoc(), hasType(arrayType()),
-  unless(anyOf(hasParent(varDecl(isExternC())),
+  unless(anyOf(hasParent(parmVarDecl(isArgvOfMain())),
+   hasParent(varDecl(isExternC())),
hasParent(fieldDecl(
hasParent(recordDecl(isExternCContext(),
hasAncestor(functionDecl(isExternC())


Index: test/clang-tidy/modernize-avoid-c-arrays-ignores-three-arg-main.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-avoid-c-arrays-ignores-three-arg-main.cpp
@@ -0,0 +1,20 @@
+// RUN: %check_clang_tidy %s modernize-avoid-c-arrays %t
+
+int not_main(int argc, char *argv[], char *argw[]) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not declare C-style arrays, use std::array<> instead
+  // CHECK-MESSAGES: :[[@LINE-2]]:38: warning: do not declare C-style arrays, use std::array<> instead
+  int f4[] = {1, 2};
+  // CHECK-MESSAGES: :[[@LINE

[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2953
+  "have bases of non-trivial class types|have virtual bases|"
+  "have __weak fields under ARC|have fields of non-trivial class types}0">;
 

ahatanak wrote:
> Quuxplusone wrote:
> > nit: "of non-trivial class types" should be "of non-trivial class type" in 
> > both places.
> > 
> > And I would write "are not move-constructible" rather than "don't have 
> > non-deleted copy/move constructors". Double negations aren't non-bad.
> > 
> > Actually I would rephrase this as `'trivial_abi' is disallowed on this 
> > class because it %select{is not move-constructible|is polymorphic|has a 
> > base of non-trivial class type|has a virtual base|has a __weak field|has a 
> > field of non-trivial class type}`, i.e., we're not just giving information 
> > about "classes" in general, we're talking about "this class" specifically. 
> > We could even name the class if we're feeling generous.
> Does not being move-constructible imply that the class doesn't have a 
> *public* copy or move constructor that isn't deleted? If it does, that is 
> slightly different than saying the copy and move constructors of the class 
> are all deleted. When the users see the message "is not move-constructible", 
> they might think the copy or move constructor that isn't deleted has to be 
> public in order to annotate the class with `trivial_abi`. For `trivial_abi`, 
> a private one is good enough.
> 
> I think your other suggestions are all good ideas.
> Does not being move-constructible imply that the class doesn't have a 
> *public* copy or move constructor that isn't deleted?

Yeah, I suppose so. Okay.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57626



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

[PATCH] D57829: [HIP] Disable emitting llvm.linker.options in device compilation

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: tra, rjmccall.

HIP toolchain does not support llvm.linker.options in device compilation, 
therefore disable it.


https://reviews.llvm.org/D57829

Files:
  lib/CodeGen/CodeGenModule.cpp
  test/CodeGenCUDA/linker-options.cu


Index: test/CodeGenCUDA/linker-options.cu
===
--- /dev/null
+++ test/CodeGenCUDA/linker-options.cu
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -emit-llvm -o - -fcuda-is-device -x hip %s | FileCheck %s
+
+// CHECK-NOT: llvm.linker.options
+#pragma comment(lib, "a.so")
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -438,6 +438,7 @@
 SanStats->finish();
 
   if (CodeGenOpts.Autolink &&
+  !(Context.getLangOpts().CUDAIsDevice && Context.getLangOpts().HIP) &&
   (Context.getLangOpts().Modules || !LinkerOptionsMetadata.empty())) {
 EmitModuleLinkOptions();
   }


Index: test/CodeGenCUDA/linker-options.cu
===
--- /dev/null
+++ test/CodeGenCUDA/linker-options.cu
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -emit-llvm -o - -fcuda-is-device -x hip %s | FileCheck %s
+
+// CHECK-NOT: llvm.linker.options
+#pragma comment(lib, "a.so")
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -438,6 +438,7 @@
 SanStats->finish();
 
   if (CodeGenOpts.Autolink &&
+  !(Context.getLangOpts().CUDAIsDevice && Context.getLangOpts().HIP) &&
   (Context.getLangOpts().Modules || !LinkerOptionsMetadata.empty())) {
 EmitModuleLinkOptions();
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45978: dllexport const variables must have external linkage.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Can you add tests for C mode as well, as it seems the behavior differs there. 
Given:

  __declspec(dllexport) int const x = 3;
  __declspec(dllexport) const int y;
  extern int const z = 4;
  
  int main() {
int a = x + y + z;
return a;
  }

I get:

  
C:\Users\aballman.GRAMMATECH\source\repos\ConsoleApplication1\ConsoleApplication1>cl
 -c ConsoleApplication1.cpp
  Microsoft (R) C/C++ Optimizing Compiler Version 19.16.27026.1 for x64
  Copyright (C) Microsoft Corporation.  All rights reserved.
  
  ConsoleApplication1.cpp
  ConsoleApplication1.cpp(2): error C2734: 'y': 'const' object must be 
initialized if not 'extern'
  
  
C:\Users\aballman.GRAMMATECH\source\repos\ConsoleApplication1\ConsoleApplication1>cl
 -c /Tc ConsoleApplication1.cpp
  Microsoft (R) C/C++ Optimizing Compiler Version 19.16.27026.1 for x64
  Copyright (C) Microsoft Corporation.  All rights reserved.
  
  ConsoleApplication1.cpp
  
  
C:\Users\aballman.GRAMMATECH\source\repos\ConsoleApplication1\ConsoleApplication1>dumpbin
 /symbols ConsoleApplication1.obj | grep External
  008  SECT3  notype   External | x
  009 0004 UNDEF  notype   External | y
  00A 0004 SECT3  notype   External | z
  00D  SECT4  notype ()External | main

After commenting out the declaration and use of `y` and recompiling in C++ mode:

  
C:\Users\aballman.GRAMMATECH\source\repos\ConsoleApplication1\ConsoleApplication1>dumpbin
 /symbols ConsoleApplication1.obj | grep External
  008  SECT3  notype   External | ?z@@3HB (int const z)
  00B  SECT4  notype ()External | main




Comment at: test/Sema/dllexport-1.cpp:2
+// RUN: %clang_cc1 -triple i686-win32 -fsyntax-only -fms-extensions -verify %s
+// RUN: %clang_cc1 -triple x86_64-win32   -fsyntax-only -fms-extensions 
-verify -std=c++11 %s
+

Can remove some extra whitespace before `-fsyntax-only` (same is true in the 
other test as well).


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

https://reviews.llvm.org/D45978



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

[PATCH] D57831: AMDGPU: set wchar_t and wint_t to be unsigned short on windows

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added a reviewer: b-sumner.
Herald added subscribers: t-tye, Anastasia, tpr, dstuttard, nhaehnle, wdng, 
jvesely, kzhuravl.

In MSVC wchar_t and wint_t are unsigned short. There is static_assert in MSVC 
headers checking that.

Since HIP and OpenCL share the same device library on windows, we have to 
define wchar_t and
wint_t to match MSVC on windows.

This should not affect OpenCL since OpenCL does not use wchar_t or wint_t.


https://reviews.llvm.org/D57831

Files:
  lib/Basic/Targets/AMDGPU.cpp
  test/SemaCXX/amdgpu-wchar.cxx


Index: test/SemaCXX/amdgpu-wchar.cxx
===
--- /dev/null
+++ test/SemaCXX/amdgpu-wchar.cxx
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple amdgcn -std=c++11 %s
+
+typedef __WINT_TYPE__ wint_t;
+
+#if _WIN32
+static_assert(sizeof(wchar_t)==2, "fail");
+static_assert(sizeof(wint_t)==2, "fail");
+#else
+static_assert(sizeof(wchar_t)==4, "fail");
+static_assert(sizeof(wint_t)==4, "fail");
+#endif
Index: lib/Basic/Targets/AMDGPU.cpp
===
--- lib/Basic/Targets/AMDGPU.cpp
+++ lib/Basic/Targets/AMDGPU.cpp
@@ -260,6 +260,10 @@
   }
 
   MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
+#if _WIN32
+  WCharType = UnsignedShort;
+  WIntType = UnsignedShort;
+#endif
 }
 
 void AMDGPUTargetInfo::adjust(LangOptions &Opts) {


Index: test/SemaCXX/amdgpu-wchar.cxx
===
--- /dev/null
+++ test/SemaCXX/amdgpu-wchar.cxx
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple amdgcn -std=c++11 %s
+
+typedef __WINT_TYPE__ wint_t;
+
+#if _WIN32
+static_assert(sizeof(wchar_t)==2, "fail");
+static_assert(sizeof(wint_t)==2, "fail");
+#else
+static_assert(sizeof(wchar_t)==4, "fail");
+static_assert(sizeof(wint_t)==4, "fail");
+#endif
Index: lib/Basic/Targets/AMDGPU.cpp
===
--- lib/Basic/Targets/AMDGPU.cpp
+++ lib/Basic/Targets/AMDGPU.cpp
@@ -260,6 +260,10 @@
   }
 
   MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
+#if _WIN32
+  WCharType = UnsignedShort;
+  WIntType = UnsignedShort;
+#endif
 }
 
 void AMDGPUTargetInfo::adjust(LangOptions &Opts) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57787: [clang-tidy] modernize-avoid-c-arrays: avoid main function (PR40604)

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from a nit.




Comment at: clang-tidy/modernize/AvoidCArraysCheck.cpp:36-38
+  if (!FD) // If not a FunctionDecl, then certainly not main entry function.
+return false;
+  return FD->isMain();

`return FD ? FD->isMain() : false;` ?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57787



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

[PATCH] D57265: [PM/CC1] Add -f[no-]split-cold-code CC1 options to toggle splitting

[Aditya Kumar via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D57265



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

[PATCH] D57829: [HIP] Disable emitting llvm.linker.options in device compilation

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

Could you elaborate on why you want to disable this metadata? I think the 
original idea of llvm.linker.options was that it should be ignored if the 
back-end does not support it.




Comment at: lib/CodeGen/CodeGenModule.cpp:441
   if (CodeGenOpts.Autolink &&
+  !(Context.getLangOpts().CUDAIsDevice && Context.getLangOpts().HIP) &&
   (Context.getLangOpts().Modules || !LinkerOptionsMetadata.empty())) {

If we do need to disable it, it may be better to do it in 
`ShouldDisableAutolink()` function in clang/lib/Driver/ToolChains/Clang.cpp




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

https://reviews.llvm.org/D57829



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

[PATCH] D57787: [clang-tidy] modernize-avoid-c-arrays: avoid main function (PR40604)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D57787#1387406 , @aaron.ballman 
wrote:

> LGTM aside from a nit.


Thank you for the review.




Comment at: clang-tidy/modernize/AvoidCArraysCheck.cpp:35
+  const clang::DeclContext *DC = Node.getDeclContext();
+  const clang::FunctionDecl *FD = llvm::dyn_cast(DC);
+  if (!FD)

aaron.ballman wrote:
> JonasToth wrote:
> > lebedev.ri wrote:
> > > lebedev.ri wrote:
> > > > JonasToth wrote:
> > > > > There is `FunctionDecl->castToDeclContext()` which is probably a 
> > > > > better fit here.
> > > > I'm guessing you meant `cast*From*DeclContext()`.
> > > > Interesting, that function is never once used.
> > > > And it uses `static_cast<>()`..
> > > I'm not too sure about this.
> > > Given `ParmVarDecl`, are we sure it's `DeclContext` is *always* 
> > > `FunctionDecl`?
> > From Doc "Represents a parameter to a function. " so i think it has always 
> > a `FunctionDecl` (or subclass) as DeclContext.
> > 
> > Maybe that function is a relict? I just saw it in the docs too and thought 
> > it makes sense to use it. No opinion on that, @aaron.ballman do you know 
> > more on that?
> > From Doc "Represents a parameter to a function. " so i think it has always 
> > a FunctionDecl (or subclass) as DeclContext.
> 
> `ObjCMethodDecl` is not a subclass of `FunctionDecl`, yet it contains 
> `ParmVarDecl` objects and is a `DeclContext`.
> 
> I would use `dyn_cast` here instead.
Aha!


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57787



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

[PATCH] D57829: [HIP] Disable emitting llvm.linker.options in device compilation

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D57829#1387412 , @tra wrote:

> Could you elaborate on why you want to disable this metadata? I think the 
> original idea of llvm.linker.options was that it should be ignored if the 
> back-end does not support it.


If backend does not support it, it goes to 
TargetLoweringObjectFileELF::emitModuleMetadata and causes codegen to fail.


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

https://reviews.llvm.org/D57829



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

[clang-tools-extra] r353327 - [clang-tidy] modernize-avoid-c-arrays: avoid main function (PR40604)

[Roman Lebedev via cfe-commits]

Author: lebedevri
Date: Wed Feb  6 11:17:30 2019
New Revision: 353327

URL: http://llvm.org/viewvc/llvm-project?rev=353327&view=rev
Log:
[clang-tidy] modernize-avoid-c-arrays: avoid main function (PR40604)

Summary:
The check should ignore the main function, the program entry point.
It is not possible to use `std::array<>` for the `argv`.
The alternative is to use `char** argv`.

Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=40604 | PR40604 ]]

Reviewers: JonasToth, aaron.ballman

Reviewed By: aaron.ballman

Subscribers: xazax.hun, hans, cfe-commits

Tags: #clang-tools-extra, #clang

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

Added:

clang-tools-extra/trunk/test/clang-tidy/modernize-avoid-c-arrays-ignores-main.cpp

clang-tools-extra/trunk/test/clang-tidy/modernize-avoid-c-arrays-ignores-three-arg-main.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/modernize/AvoidCArraysCheck.cpp
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-avoid-c-arrays.rst

Modified: clang-tools-extra/trunk/clang-tidy/modernize/AvoidCArraysCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/AvoidCArraysCheck.cpp?rev=353327&r1=353326&r2=353327&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/AvoidCArraysCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/AvoidCArraysCheck.cpp Wed Feb  
6 11:17:30 2019
@@ -30,6 +30,12 @@ AST_MATCHER(clang::RecordDecl, isExternC
   return Node.isExternCContext();
 }
 
+AST_MATCHER(clang::ParmVarDecl, isArgvOfMain) {
+  const clang::DeclContext *DC = Node.getDeclContext();
+  const auto *FD = llvm::dyn_cast(DC);
+  return FD ? FD->isMain() : false;
+}
+
 } // namespace
 
 namespace clang {
@@ -43,7 +49,8 @@ void AvoidCArraysCheck::registerMatchers
 
   Finder->addMatcher(
   typeLoc(hasValidBeginLoc(), hasType(arrayType()),
-  unless(anyOf(hasParent(varDecl(isExternC())),
+  unless(anyOf(hasParent(parmVarDecl(isArgvOfMain())),
+   hasParent(varDecl(isExternC())),
hasParent(fieldDecl(
hasParent(recordDecl(isExternCContext(),
hasAncestor(functionDecl(isExternC())

Modified: 
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-avoid-c-arrays.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-avoid-c-arrays.rst?rev=353327&r1=353326&r2=353327&view=diff
==
--- clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-avoid-c-arrays.rst 
(original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-avoid-c-arrays.rst 
Wed Feb  6 11:17:30 2019
@@ -54,3 +54,7 @@ such headers between C code, and C++ cod
   }
 
   }
+
+Similarly, the ``main()`` function is ignored. Its second and third parameters
+can be either ``char* argv[]`` or ``char** argv``, but can not be
+``std::array<>``.

Added: 
clang-tools-extra/trunk/test/clang-tidy/modernize-avoid-c-arrays-ignores-main.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-avoid-c-arrays-ignores-main.cpp?rev=353327&view=auto
==
--- 
clang-tools-extra/trunk/test/clang-tidy/modernize-avoid-c-arrays-ignores-main.cpp
 (added)
+++ 
clang-tools-extra/trunk/test/clang-tidy/modernize-avoid-c-arrays-ignores-main.cpp
 Wed Feb  6 11:17:30 2019
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s modernize-avoid-c-arrays %t
+
+int not_main(int argc, char *argv[]) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not declare C-style arrays, 
use std::array<> instead
+  int f4[] = {1, 2};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, 
use std::array<> instead
+}
+
+int main(int argc, char *argv[]) {
+  int f5[] = {1, 2};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, 
use std::array<> instead
+
+  auto not_main = [](int argc, char *argv[]) {
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: do not declare C-style 
arrays, use std::array<> instead
+int f6[] = {1, 2};
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not declare C-style arrays, 
use std::array<> instead
+  };
+}

Added: 
clang-tools-extra/trunk/test/clang-tidy/modernize-avoid-c-arrays-ignores-three-arg-main.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-avoid-c-arrays-ignores-three-arg-main.cpp?rev=353327&view=auto
==
--- 
clang-tools-extra/trunk/test/clang-tidy/modernize-avoid-c-arrays-ignores-three-arg-main.cpp
 (added)
+++ 
clang-tools-extra/trunk/test/clang-tidy/modernize-avoid-c-arrays-ignores-three-arg-main.cpp
 Wed Feb  6 11:17:30 201

[PATCH] D57787: [clang-tidy] modernize-avoid-c-arrays: avoid main function (PR40604)

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353327: [clang-tidy] modernize-avoid-c-arrays: avoid main 
function (PR40604) (authored by lebedevri, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57787?vs=185587&id=185598#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57787

Files:
  clang-tools-extra/trunk/clang-tidy/modernize/AvoidCArraysCheck.cpp
  clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-avoid-c-arrays.rst
  
clang-tools-extra/trunk/test/clang-tidy/modernize-avoid-c-arrays-ignores-main.cpp
  
clang-tools-extra/trunk/test/clang-tidy/modernize-avoid-c-arrays-ignores-three-arg-main.cpp


Index: 
clang-tools-extra/trunk/test/clang-tidy/modernize-avoid-c-arrays-ignores-three-arg-main.cpp
===
--- 
clang-tools-extra/trunk/test/clang-tidy/modernize-avoid-c-arrays-ignores-three-arg-main.cpp
+++ 
clang-tools-extra/trunk/test/clang-tidy/modernize-avoid-c-arrays-ignores-three-arg-main.cpp
@@ -0,0 +1,20 @@
+// RUN: %check_clang_tidy %s modernize-avoid-c-arrays %t
+
+int not_main(int argc, char *argv[], char *argw[]) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not declare C-style arrays, 
use std::array<> instead
+  // CHECK-MESSAGES: :[[@LINE-2]]:38: warning: do not declare C-style arrays, 
use std::array<> instead
+  int f4[] = {1, 2};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, 
use std::array<> instead
+}
+
+int main(int argc, char *argv[], char *argw[]) {
+  int f5[] = {1, 2};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, 
use std::array<> instead
+
+  auto not_main = [](int argc, char *argv[], char *argw[]) {
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: do not declare C-style 
arrays, use std::array<> instead
+// CHECK-MESSAGES: :[[@LINE-2]]:46: warning: do not declare C-style 
arrays, use std::array<> instead
+int f6[] = {1, 2};
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not declare C-style arrays, 
use std::array<> instead
+  };
+}
Index: 
clang-tools-extra/trunk/test/clang-tidy/modernize-avoid-c-arrays-ignores-main.cpp
===
--- 
clang-tools-extra/trunk/test/clang-tidy/modernize-avoid-c-arrays-ignores-main.cpp
+++ 
clang-tools-extra/trunk/test/clang-tidy/modernize-avoid-c-arrays-ignores-main.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s modernize-avoid-c-arrays %t
+
+int not_main(int argc, char *argv[]) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not declare C-style arrays, 
use std::array<> instead
+  int f4[] = {1, 2};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, 
use std::array<> instead
+}
+
+int main(int argc, char *argv[]) {
+  int f5[] = {1, 2};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, 
use std::array<> instead
+
+  auto not_main = [](int argc, char *argv[]) {
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: do not declare C-style 
arrays, use std::array<> instead
+int f6[] = {1, 2};
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not declare C-style arrays, 
use std::array<> instead
+  };
+}
Index: clang-tools-extra/trunk/clang-tidy/modernize/AvoidCArraysCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/modernize/AvoidCArraysCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/modernize/AvoidCArraysCheck.cpp
@@ -30,6 +30,12 @@
   return Node.isExternCContext();
 }
 
+AST_MATCHER(clang::ParmVarDecl, isArgvOfMain) {
+  const clang::DeclContext *DC = Node.getDeclContext();
+  const auto *FD = llvm::dyn_cast(DC);
+  return FD ? FD->isMain() : false;
+}
+
 } // namespace
 
 namespace clang {
@@ -43,7 +49,8 @@
 
   Finder->addMatcher(
   typeLoc(hasValidBeginLoc(), hasType(arrayType()),
-  unless(anyOf(hasParent(varDecl(isExternC())),
+  unless(anyOf(hasParent(parmVarDecl(isArgvOfMain())),
+   hasParent(varDecl(isExternC())),
hasParent(fieldDecl(
hasParent(recordDecl(isExternCContext(),
hasAncestor(functionDecl(isExternC())
Index: 
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-avoid-c-arrays.rst
===
--- clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-avoid-c-arrays.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-avoid-c-arrays.rst
@@ -54,3 +54,7 @@
   }
 
   }
+
+Similarly, the ``main()`` function is ignored. Its second and third parameters
+can be either ``char* argv[]`` or ``char** argv``, but can not be
+``std::array<>``.


Index: clang-tools-extra/trunk/test/cl

[PATCH] D57267: [AST] Factor out the logic of the various Expr::Ignore*

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM! You should give @rsmith a few days in case he wants to express an opinion 
though.




Comment at: lib/AST/Expr.cpp:2643-2645
+  }
+
+  else if (auto *GSE = dyn_cast(E)) {

Formatting (same elsewhere).


Repository:
  rC Clang

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

https://reviews.llvm.org/D57267



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

[PATCH] D57829: [HIP] Disable emitting llvm.linker.options in device compilation

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D57829#1387416 , @yaxunl wrote:

> In D57829#1387412 , @tra wrote:
>
> > Could you elaborate on why you want to disable this metadata? I think the 
> > original idea of llvm.linker.options was that it should be ignored if the 
> > back-end does not support it.
>
>
> If backend does not support it, it goes to 
> TargetLoweringObjectFileELF::emitModuleMetadata and causes codegen to fail.


Fails how? AFAICT, for llvm.linker.options  emitModuleMetadata() just creates 
an ELF section ".linker-options" which should just work.
Are you saying that some of HIP tools can't deal with the unknown section type?


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

https://reviews.llvm.org/D57829



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

[PATCH] D57831: AMDGPU: set wchar_t and wint_t to be unsigned short on windows

[Brian Sumner via Phabricator via cfe-commits]

b-sumner added a comment.

Maybe there are already other types like this, but it saddens me that an 
offline compiled code object could potentially not work properly if the 
application is using any of these types.  Or should the runtime try to detect a 
problem using argument metadata?


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

https://reviews.llvm.org/D57831



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

[PATCH] D57831: AMDGPU: set wchar_t and wint_t to be unsigned short on windows

[Saleem Abdulrasool via Phabricator via cfe-commits]

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



Comment at: lib/Basic/Targets/AMDGPU.cpp:263
   MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
+#if _WIN32
+  WCharType = UnsignedShort;

This seems wrong.  This means that if you built on Linux, you get a different 
`wchar_t` type.  You should be checking the triple and setting this accordingly.


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

https://reviews.llvm.org/D57831



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

[PATCH] D57021: [clangd] Suggest adding missing includes for typos (like include-fixer).

[Sam McCall via Phabricator via cfe-commits]

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

Great stuff! Sorry for the delay here.




Comment at: clangd/IncludeFixer.h:59
+SourceLocation Loc; // Start location of the unresolved name.
+// Callback to get possible scopes of the unresolved name. `Sema` must be
+// alive when this is called.

nit: this isn't really a callback, I think.
Just "Lazily get the possible scopes of the unresolved name"?



Comment at: clangd/IncludeFixer.h:64
+
+  /// Records the last unresolved name (i.e. typo) seen by Sema.
+  class UnresolvedNameRecorder;

"i.e. typo" is confusing here, as these aren't actually typos


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57021



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

[PATCH] D57631: [COFF, ARM64] Add ARM64 support for MS intrinsic _fastfail

[Tom Tan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353337: [COFF, ARM64] Add ARM64 support for MS intrinsic 
_fastfail (authored by TomTan, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57631?vs=184869&id=185609#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57631

Files:
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/test/CodeGen/ms-intrinsics.c


Index: cfe/trunk/test/CodeGen/ms-intrinsics.c
===
--- cfe/trunk/test/CodeGen/ms-intrinsics.c
+++ cfe/trunk/test/CodeGen/ms-intrinsics.c
@@ -9,7 +9,7 @@
 // RUN: | FileCheck %s --check-prefixes 
CHECK,CHECK-X64,CHECK-ARM-X64,CHECK-INTEL
 // RUN: %clang_cc1 -ffreestanding -fms-extensions -fms-compatibility 
-fms-compatibility-version=17.00 \
 // RUN: -triple aarch64-windows -Oz -emit-llvm %s -o - \
-// RUN: | FileCheck %s --check-prefixes CHECK-ARM-ARM64,CHECK-ARM-X64
+// RUN: | FileCheck %s --check-prefixes 
CHECK-ARM-ARM64,CHECK-ARM-X64,CHECK-ARM64
 
 // intrin.h needs size_t, but -ffreestanding prevents us from getting it from
 // stddef.h.  Work around it with this typedef.
@@ -1342,15 +1342,14 @@
 // CHECK-ARM-ARM64: }
 #endif
 
-#if !defined(__aarch64__)
 void test__fastfail() {
   __fastfail(42);
 }
 // CHECK-LABEL: define{{.*}} void @test__fastfail()
 // CHECK-ARM: call void asm sideeffect "udf #251", "{r0}"(i32 42) 
#[[NORETURN:[0-9]+]]
 // CHECK-INTEL: call void asm sideeffect "int $$0x29", "{cx}"(i32 42) 
#[[NORETURN]]
+// CHECK-ARM64: call void asm sideeffect "brk #0xF003", "{w0}"(i32 42) 
#[[NORETURN:[0-9]+]]
 
 // Attributes come last.
 
 // CHECK: attributes #[[NORETURN]] = { noreturn{{.*}} }
-#endif
Index: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
===
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp
@@ -997,6 +997,9 @@
   Asm = "udf #251";
   Constraints = "{r0}";
   break;
+case llvm::Triple::aarch64:
+  Asm = "brk #0xF003";
+  Constraints = "{w0}";
 }
 llvm::FunctionType *FTy = llvm::FunctionType::get(VoidTy, {Int32Ty}, 
false);
 llvm::InlineAsm *IA =


Index: cfe/trunk/test/CodeGen/ms-intrinsics.c
===
--- cfe/trunk/test/CodeGen/ms-intrinsics.c
+++ cfe/trunk/test/CodeGen/ms-intrinsics.c
@@ -9,7 +9,7 @@
 // RUN: | FileCheck %s --check-prefixes CHECK,CHECK-X64,CHECK-ARM-X64,CHECK-INTEL
 // RUN: %clang_cc1 -ffreestanding -fms-extensions -fms-compatibility -fms-compatibility-version=17.00 \
 // RUN: -triple aarch64-windows -Oz -emit-llvm %s -o - \
-// RUN: | FileCheck %s --check-prefixes CHECK-ARM-ARM64,CHECK-ARM-X64
+// RUN: | FileCheck %s --check-prefixes CHECK-ARM-ARM64,CHECK-ARM-X64,CHECK-ARM64
 
 // intrin.h needs size_t, but -ffreestanding prevents us from getting it from
 // stddef.h.  Work around it with this typedef.
@@ -1342,15 +1342,14 @@
 // CHECK-ARM-ARM64: }
 #endif
 
-#if !defined(__aarch64__)
 void test__fastfail() {
   __fastfail(42);
 }
 // CHECK-LABEL: define{{.*}} void @test__fastfail()
 // CHECK-ARM: call void asm sideeffect "udf #251", "{r0}"(i32 42) #[[NORETURN:[0-9]+]]
 // CHECK-INTEL: call void asm sideeffect "int $$0x29", "{cx}"(i32 42) #[[NORETURN]]
+// CHECK-ARM64: call void asm sideeffect "brk #0xF003", "{w0}"(i32 42) #[[NORETURN:[0-9]+]]
 
 // Attributes come last.
 
 // CHECK: attributes #[[NORETURN]] = { noreturn{{.*}} }
-#endif
Index: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
===
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp
@@ -997,6 +997,9 @@
   Asm = "udf #251";
   Constraints = "{r0}";
   break;
+case llvm::Triple::aarch64:
+  Asm = "brk #0xF003";
+  Constraints = "{w0}";
 }
 llvm::FunctionType *FTy = llvm::FunctionType::get(VoidTy, {Int32Ty}, false);
 llvm::InlineAsm *IA =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r353337 - [COFF, ARM64] Add ARM64 support for MS intrinsic _fastfail

[Tom Tan via cfe-commits]

Author: tomtan
Date: Wed Feb  6 12:08:26 2019
New Revision: 353337

URL: http://llvm.org/viewvc/llvm-project?rev=353337&view=rev
Log:
[COFF, ARM64] Add ARM64 support for MS intrinsic _fastfail

The MSDN document was also updated to reflect this, but it probably will take a 
few days to show in below link.

https://docs.microsoft.com/en-us/cpp/intrinsics/fastfail

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

Modified:
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/test/CodeGen/ms-intrinsics.c

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=353337&r1=353336&r2=353337&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Wed Feb  6 12:08:26 2019
@@ -997,6 +997,9 @@ Value *CodeGenFunction::EmitMSVCBuiltinE
   Asm = "udf #251";
   Constraints = "{r0}";
   break;
+case llvm::Triple::aarch64:
+  Asm = "brk #0xF003";
+  Constraints = "{w0}";
 }
 llvm::FunctionType *FTy = llvm::FunctionType::get(VoidTy, {Int32Ty}, 
false);
 llvm::InlineAsm *IA =

Modified: cfe/trunk/test/CodeGen/ms-intrinsics.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-intrinsics.c?rev=353337&r1=353336&r2=353337&view=diff
==
--- cfe/trunk/test/CodeGen/ms-intrinsics.c (original)
+++ cfe/trunk/test/CodeGen/ms-intrinsics.c Wed Feb  6 12:08:26 2019
@@ -9,7 +9,7 @@
 // RUN: | FileCheck %s --check-prefixes 
CHECK,CHECK-X64,CHECK-ARM-X64,CHECK-INTEL
 // RUN: %clang_cc1 -ffreestanding -fms-extensions -fms-compatibility 
-fms-compatibility-version=17.00 \
 // RUN: -triple aarch64-windows -Oz -emit-llvm %s -o - \
-// RUN: | FileCheck %s --check-prefixes CHECK-ARM-ARM64,CHECK-ARM-X64
+// RUN: | FileCheck %s --check-prefixes 
CHECK-ARM-ARM64,CHECK-ARM-X64,CHECK-ARM64
 
 // intrin.h needs size_t, but -ffreestanding prevents us from getting it from
 // stddef.h.  Work around it with this typedef.
@@ -1342,15 +1342,14 @@ __int64 test_InterlockedDecrement64_nf(_
 // CHECK-ARM-ARM64: }
 #endif
 
-#if !defined(__aarch64__)
 void test__fastfail() {
   __fastfail(42);
 }
 // CHECK-LABEL: define{{.*}} void @test__fastfail()
 // CHECK-ARM: call void asm sideeffect "udf #251", "{r0}"(i32 42) 
#[[NORETURN:[0-9]+]]
 // CHECK-INTEL: call void asm sideeffect "int $$0x29", "{cx}"(i32 42) 
#[[NORETURN]]
+// CHECK-ARM64: call void asm sideeffect "brk #0xF003", "{w0}"(i32 42) 
#[[NORETURN:[0-9]+]]
 
 // Attributes come last.
 
 // CHECK: attributes #[[NORETURN]] = { noreturn{{.*}} }
-#endif


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

[PATCH] D57835: Fix -ftime-report with -x ir

[Matt Arsenault via Phabricator via cfe-commits]

arsenm created this revision.
Herald added a subscriber: wdng.

This was only printing the clang frontend timer, and none of the
backend timers. Set the llvm global for enabling the backend time
report when creating the frontend timers, rather than in the
BackendConsumer constructor.

  

There's still something wrong with how the timers are managed though,
since the report seems to be printed twice for each one.


https://reviews.llvm.org/D57835

Files:
  lib/CodeGen/CodeGenAction.cpp
  lib/Frontend/CompilerInstance.cpp
  test/Frontend/ftime-report-bitcode.ll


Index: test/Frontend/ftime-report-bitcode.ll
===
--- /dev/null
+++ test/Frontend/ftime-report-bitcode.ll
@@ -0,0 +1,14 @@
+; RUN: %clang_cc1 -triple x86_64-apple-darwin10 -ftime-report -emit-obj -o 
/dev/null %s 2>&1 | FileCheck %s
+
+target triple ="x86_64-apple-darwin10"
+
+; Make sure the backend time reports are produced when compiling an IR
+; input
+
+; CHECK: Pass execution timing report
+; CHECK: LLVM IR Parsing
+; CHECK: Clang front-end time report
+
+define i32 @foo() {
+  ret i32 0
+}
Index: lib/Frontend/CompilerInstance.cpp
===
--- lib/Frontend/CompilerInstance.cpp
+++ lib/Frontend/CompilerInstance.cpp
@@ -941,8 +941,10 @@
<< " based upon " << BACKEND_PACKAGE_STRING
<< " default target " << llvm::sys::getDefaultTargetTriple() << "\n";
 
-  if (getFrontendOpts().ShowTimers)
+  if (getFrontendOpts().ShowTimers) {
+llvm::TimePassesIsEnabled = true;
 createFrontendTimer();
+  }
 
   if (getFrontendOpts().ShowStats || !getFrontendOpts().StatsFile.empty())
 llvm::EnableStatistics(false);
Index: lib/CodeGen/CodeGenAction.cpp
===
--- lib/CodeGen/CodeGenAction.cpp
+++ lib/CodeGen/CodeGenAction.cpp
@@ -126,7 +126,6 @@
 CodeGenOpts, C, CoverageInfo)),
   LinkModules(std::move(LinkModules)) {
   FrontendTimesIsEnabled = TimePasses;
-  llvm::TimePassesIsEnabled = TimePasses;
 }
 llvm::Module *getModule() const { return Gen->GetModule(); }
 std::unique_ptr takeModule() {


Index: test/Frontend/ftime-report-bitcode.ll
===
--- /dev/null
+++ test/Frontend/ftime-report-bitcode.ll
@@ -0,0 +1,14 @@
+; RUN: %clang_cc1 -triple x86_64-apple-darwin10 -ftime-report -emit-obj -o /dev/null %s 2>&1 | FileCheck %s
+
+target triple ="x86_64-apple-darwin10"
+
+; Make sure the backend time reports are produced when compiling an IR
+; input
+
+; CHECK: Pass execution timing report
+; CHECK: LLVM IR Parsing
+; CHECK: Clang front-end time report
+
+define i32 @foo() {
+  ret i32 0
+}
Index: lib/Frontend/CompilerInstance.cpp
===
--- lib/Frontend/CompilerInstance.cpp
+++ lib/Frontend/CompilerInstance.cpp
@@ -941,8 +941,10 @@
<< " based upon " << BACKEND_PACKAGE_STRING
<< " default target " << llvm::sys::getDefaultTargetTriple() << "\n";
 
-  if (getFrontendOpts().ShowTimers)
+  if (getFrontendOpts().ShowTimers) {
+llvm::TimePassesIsEnabled = true;
 createFrontendTimer();
+  }
 
   if (getFrontendOpts().ShowStats || !getFrontendOpts().StatsFile.empty())
 llvm::EnableStatistics(false);
Index: lib/CodeGen/CodeGenAction.cpp
===
--- lib/CodeGen/CodeGenAction.cpp
+++ lib/CodeGen/CodeGenAction.cpp
@@ -126,7 +126,6 @@
 CodeGenOpts, C, CoverageInfo)),
   LinkModules(std::move(LinkModules)) {
   FrontendTimesIsEnabled = TimePasses;
-  llvm::TimePassesIsEnabled = TimePasses;
 }
 llvm::Module *getModule() const { return Gen->GetModule(); }
 std::unique_ptr takeModule() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57838: [clang-cl] support /Oy- on aarch64

[Nathan Froyd via Phabricator via cfe-commits]

froydnj created this revision.
froydnj added a reviewer: mstorsjo.
Herald added subscribers: cfe-commits, kristof.beyls, javed.absar.
Herald added a project: clang.

MSVC supports /Oy- on aarch64, so clang-cl should too.


Repository:
  rC Clang

https://reviews.llvm.org/D57838

Files:
  lib/Driver/ToolChains/MSVC.cpp
  test/Driver/cl-options.c


Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -178,6 +178,10 @@
 // Oy_2: -momit-leaf-frame-pointer
 // Oy_2: -O2
 
+// RUN: %clang_cl --target=aarch64-pc-windows-msvc -Werror /Oy- /O2 -### -- %s 
2>&1 | FileCheck -check-prefix=Oy_aarch64 %s
+// Oy_aarch64: -mdisable-fp-elim
+// Oy_aarch64: -O2
+
 // RUN: %clang_cl --target=i686-pc-win32 -Werror /O2 /O2 -### -- %s 2>&1 | 
FileCheck -check-prefix=O2O2 %s
 // O2O2: "-O2"
 
Index: lib/Driver/ToolChains/MSVC.cpp
===
--- lib/Driver/ToolChains/MSVC.cpp
+++ lib/Driver/ToolChains/MSVC.cpp
@@ -1407,10 +1407,10 @@
   DAL.AddFlagArg(
   A, Opts.getOption(options::OPT_fno_omit_frame_pointer));
   } else {
-// Don't warn about /Oy- in 64-bit builds (where
+// Don't warn about /Oy- in x86-64 builds (where
 // SupportsForcingFramePointer is false).  The flag having no effect
 // there is a compiler-internal optimization, and people shouldn't have
-// to special-case their build files for 64-bit clang-cl.
+// to special-case their build files for x86-64 clang-cl.
 A->claim();
   }
   break;
@@ -1441,8 +1441,8 @@
   DerivedArgList *DAL = new DerivedArgList(Args.getBaseArgs());
   const OptTable &Opts = getDriver().getOpts();
 
-  // /Oy and /Oy- only has an effect under X86-32.
-  bool SupportsForcingFramePointer = getArch() == llvm::Triple::x86;
+  // /Oy and /Oy- don't have an effect on X86-64
+  bool SupportsForcingFramePointer = getArch() != llvm::Triple::x86_64;
 
   // The -O[12xd] flag actually expands to several flags.  We must desugar the
   // flags so that options embedded can be negated.  For example, the '-O2' 
flag


Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -178,6 +178,10 @@
 // Oy_2: -momit-leaf-frame-pointer
 // Oy_2: -O2
 
+// RUN: %clang_cl --target=aarch64-pc-windows-msvc -Werror /Oy- /O2 -### -- %s 2>&1 | FileCheck -check-prefix=Oy_aarch64 %s
+// Oy_aarch64: -mdisable-fp-elim
+// Oy_aarch64: -O2
+
 // RUN: %clang_cl --target=i686-pc-win32 -Werror /O2 /O2 -### -- %s 2>&1 | FileCheck -check-prefix=O2O2 %s
 // O2O2: "-O2"
 
Index: lib/Driver/ToolChains/MSVC.cpp
===
--- lib/Driver/ToolChains/MSVC.cpp
+++ lib/Driver/ToolChains/MSVC.cpp
@@ -1407,10 +1407,10 @@
   DAL.AddFlagArg(
   A, Opts.getOption(options::OPT_fno_omit_frame_pointer));
   } else {
-// Don't warn about /Oy- in 64-bit builds (where
+// Don't warn about /Oy- in x86-64 builds (where
 // SupportsForcingFramePointer is false).  The flag having no effect
 // there is a compiler-internal optimization, and people shouldn't have
-// to special-case their build files for 64-bit clang-cl.
+// to special-case their build files for x86-64 clang-cl.
 A->claim();
   }
   break;
@@ -1441,8 +1441,8 @@
   DerivedArgList *DAL = new DerivedArgList(Args.getBaseArgs());
   const OptTable &Opts = getDriver().getOpts();
 
-  // /Oy and /Oy- only has an effect under X86-32.
-  bool SupportsForcingFramePointer = getArch() == llvm::Triple::x86;
+  // /Oy and /Oy- don't have an effect on X86-64
+  bool SupportsForcingFramePointer = getArch() != llvm::Triple::x86_64;
 
   // The -O[12xd] flag actually expands to several flags.  We must desugar the
   // flags so that options embedded can be negated.  For example, the '-O2' flag
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57835: Fix -ftime-report with -x ir

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

I partially solved the problem with double printing the reports.  It's from 
this in cc1_main:

  // If any timers were active but haven't been destroyed yet, print theirp
  // results now.  This happens in -disable-free mode.
  llvm::TimerGroup::printAll(llvm::errs());

On a source file, -ftime-report  -disable-free prints one report. -ftime-report 
without disable-free prints 2. For the IR file, both happen either way and it 
double prints. The printing destructor can still be called from the 
ManagedStatic in PassTimingInfo


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

https://reviews.llvm.org/D57835



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

[PATCH] D57740: [ASTImporter] Import every Decl in lambda record

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

LGTM although I wish we had more lambda tests.

Please remember to check the LLDB build bots when landing the patch.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57740



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

[PATCH] D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 185632.
MyDeveloperDay marked 5 inline comments as done.
MyDeveloperDay added a comment.

-Address review comments
-Ignore argument comment addition for macro inputs
-Add unit tests to cover the above
-Fix clang-tidy suggestion (from readability-identifier-naming)
-Ensure file is clang-formatted


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

https://reviews.llvm.org/D57674

Files:
  clang-tidy/bugprone/ArgumentCommentCheck.cpp
  clang-tidy/bugprone/ArgumentCommentCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-argument-comment.rst
  test/clang-tidy/bugprone-argument-comment-literals.cpp

Index: test/clang-tidy/bugprone-argument-comment-literals.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-argument-comment-literals.cpp
@@ -0,0 +1,122 @@
+// RUN: %check_clang_tidy %s bugprone-argument-comment %t -- \
+// RUN:   -config="{CheckOptions: [{key: CommentBoolLiterals, value: 1},{key: CommentIntegerLiterals, value: 1}, {key: CommentFloatLiterals, value: 1}, {key: CommentUserDefinedLiterals, value: 1}, {key: CommentStringLiterals, value: 1}, {key: CommentNullPtrs, value: 1}, {key: CommentCharacterLiterals, value: 1}]}" --
+
+struct A {
+  void foo(bool abc);
+  void foo(bool abc, bool cde);
+  void foo(const char *, bool abc);
+  void foo(int iabc);
+  void foo(float fabc);
+  void foo(double dabc);
+  void foo(const char *strabc);
+  void fooW(const wchar_t *wstrabc);
+  void fooPtr(A *ptrabc);
+  void foo(char chabc);
+};
+
+#define FOO 1
+
+void g(int a);
+void h(double b);
+void i(const char *c);
+
+double operator"" _km(long double);
+
+void test() {
+  A a;
+
+  a.foo(true);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/true);
+
+  a.foo(false);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false);
+
+  a.foo(true, false);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-MESSAGES: [[@LINE-2]]:15: warning: argument comment missing for literal argument 'cde' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/true, /*cde=*/false);
+
+  a.foo(false, true);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-MESSAGES: [[@LINE-2]]:16: warning: argument comment missing for literal argument 'cde' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false, /*cde=*/true);
+
+  a.foo(/*abc=*/false, true);
+  // CHECK-MESSAGES: [[@LINE-1]]:24: warning: argument comment missing for literal argument 'cde' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false, /*cde=*/true);
+
+  a.foo(false, /*cde=*/true);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false, /*cde=*/true);
+
+  bool val1 = true;
+  bool val2 = false;
+  a.foo(val1, val2);
+
+  a.foo("", true);
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo("", /*abc=*/true);
+
+  a.foo(0);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'iabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*iabc=*/0);
+
+  a.foo(1.0f);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'fabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*fabc=*/1.0f);
+
+  a.foo(1.0);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'dabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*dabc=*/1.0);
+
+  int val3 = 10;
+  a.foo(val3);
+
+  float val4 = 10.0;
+  a.foo(val4);
+
+  double val5 = 10.0;
+  a.foo(val5);
+
+  a.foo("Hello World");
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'strabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*strabc=*/"Hello World");
+  //
+  a.fooW(L"Hello World");
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: argument comment missing for literal argument 'wstrabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.fooW(/*wstrabc=*/L"Hello World");
+
+  a.fooPtr(nullptr);
+  // CHECK-MESSAGES: [[@LINE-1]]:12: warning: argument comment missing for literal argument 'ptrabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.fooPtr(/*ptrabc=*/nullptr);
+
+  a.foo(402.0_km);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'dabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*dabc=*/402.0_km);
+
+  a.foo('A');
+  // CHECK-MESSAGES: [[@LINE-1]

[PATCH] D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

[MyDeveloperDay via Phabricator via cfe-commits]

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



Comment at: clang-tidy/bugprone/ArgumentCommentCheck.cpp:1
-//===--- ArgumentCommentCheck.cpp - clang-tidy 
===//
 //

aaron.ballman wrote:
> Why did this get removed?
Sorry I suspect poor vim skills



Comment at: clang-tidy/bugprone/ArgumentCommentCheck.cpp:231
+bool ArgumentCommentCheck::shouldAddComment(const Expr *Arg) const {
+  return (CommentBoolLiterals && isa(Arg)) ||
+ (CommentIntegerLiterals && isa(Arg)) ||

aaron.ballman wrote:
> I think we may want to do some additional work here. Consider:
> ```
> #define FOO 1
> 
> void g(int);
> void h(double);
> void i(const char *);
> 
> void f() {
>   g(FOO); // Probably don't want to suggest comments here
>   g((1)); // Probably do want to suggest comments here
>   h(1.0f); // Probably do want to suggest comments here
>   i(__FILE__); // Probably don't want to suggest comments here
> }
> ```
> I think this code should do two things: 1) check whether the location of the 
> arg is a macro expansion (if so, return false), 2) do `Arg = 
> Arg->IgnoreParenImpCasts();` at the start of the function and drop the 
> `Arg->IgnoreImpCasts()` for the string and pointer literal cases. However, 
> that may be a bridge too far, so you should add a test case like this:
> ```
> g(_Generic(0, int : 0)); // Definitely do not want to see comments here
> ```
> If it turns out the above case gives the comment suggestions, then I'd go 
> with `IgnoreImpCasts()` rather than `IgnoreParenImpCasts()`.
I agree, tried all combinations but g((1); and g(_Generic(0, int : 0)); would 
otherwise get comments


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

https://reviews.llvm.org/D57674



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

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

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

rsmith added inline comments.



Comment at: lib/AST/Stmt.cpp:628
+  DiagOffs = CurPtr-StrStart-1;
+  return diag::err_asm_invalid_operand_for_goto_labels;
+}

jyu2 wrote:
> rsmith wrote:
> > I'm curious why we're checking this here, when all the other validation for 
> > the modifier letter happens in LLVM. Does this check need to be done 
> > differently from the others?
> This is to verify following in spec:
> =
> To reference a label in the assembler template, prefix it with ‘%l’ 
> (lowercase ‘L’) followed by its (zero-based) position in GotoLabels plus the 
> number of input operands. For example, if the asm has three inputs and 
> references two labels, refer to the first label as ‘%l3’ and the second as 
> ‘%l4’).
> 
> Was request from Eli.
Sorry, I think my question was unclear. I understand why you need to do this 
check somewhere. My question is why this check needs to be *here*, given that 
all the other checks for the modifier letter are performed elsewhere. Splitting 
the checks up between multiple locations makes the system harder to maintain.

As it happens, there's a FIXME for this here: 
http://llvm.org/doxygen/AsmPrinterInlineAsm_8cpp_source.html#l00436 -- and a 
test case like `void f(void *a) { asm("mov %l0, %%eax" :: "r"(a)); }` currently 
causes LLVM to assert in a function that's supposed to be validating these 
modifier letters and producing a clean error if they're bad. So I think that's 
an LLVM bug, and fixing that LLVM bug would enforce the property we need here 
(that `%lN` only names labels).


[It's not clear to me whether we should be enforcing that `%lN` only names 
label //operands//, which would require the check to be in Clang rather than 
LLVM; GCC doesn't do that, and for instance accepts

```
void f() { asm goto("jmp %l0" :: "i"(&&foo)::foo); foo:; }
```

(at least for x86_64), though this construct doesn't seem hugely useful given 
that the input operand would need to be a literal address-of-label expression 
and you'd need to name the target label as a label operand anyway.]


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

https://reviews.llvm.org/D56571



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

[PATCH] D57265: [PM/CC1] Add -f[no-]split-cold-code CC1 options to toggle splitting

[Vedant Kumar via Phabricator via cfe-commits]

vsk added reviewers: fedor.sergeev, philip.pfaffe.
vsk added a comment.

Thanks @hiraditya. I'd also like to get a +1 from someone who's worked on the 
NewPM infrastructure extensively, just to make sure those changes look ok.


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

https://reviews.llvm.org/D57265



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

[PATCH] D57265: [PM/CC1] Add -f[no-]split-cold-code CC1 options to toggle splitting

[Vedant Kumar via Phabricator via cfe-commits]

vsk updated this revision to Diff 185642.
vsk added a comment.

Rebase.


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

https://reviews.llvm.org/D57265

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/split-cold-code.c
  llvm/include/llvm/Passes/PassBuilder.h
  llvm/include/llvm/Transforms/IPO/PassManagerBuilder.h
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Transforms/IPO/PassManagerBuilder.cpp

Index: llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
===
--- llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
+++ llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
@@ -168,6 +168,7 @@
 VerifyInput = false;
 VerifyOutput = false;
 MergeFunctions = false;
+SplitColdCode = false;
 PrepareForLTO = false;
 EnablePGOInstrGen = false;
 PGOInstrGen = "";
@@ -531,7 +532,7 @@
 
   // Split out cold code before inlining. See comment in the new PM
   // (\ref buildModuleSimplificationPipeline).
-  if (EnableHotColdSplit && DefaultOrPreLinkPipeline)
+  if ((EnableHotColdSplit || SplitColdCode) && DefaultOrPreLinkPipeline)
 MPM.add(createHotColdSplittingPass());
 
   // We add a module alias analysis pass here. In part due to bugs in the
Index: llvm/lib/Passes/PassBuilder.cpp
===
--- llvm/lib/Passes/PassBuilder.cpp
+++ llvm/lib/Passes/PassBuilder.cpp
@@ -707,7 +707,7 @@
   // not grow after inlining, and 2) inhibiting inlining of cold code improves
   // code size & compile time. Split after Mem2Reg to make code model estimates
   // more accurate, but before InstCombine to allow it to clean things up.
-  if (EnableHotColdSplit && Phase != ThinLTOPhase::PostLink)
+  if ((EnableHotColdSplit || SplitColdCode) && Phase != ThinLTOPhase::PostLink)
 MPM.addPass(HotColdSplittingPass());
 
   // Require the GlobalsAA analysis for the module so we can query it within
@@ -2120,3 +2120,7 @@
 
   return Error::success();
 }
+
+void PassBuilder::setEnableHotColdSplitting(bool Enabled) {
+  SplitColdCode = Enabled;
+}
Index: llvm/include/llvm/Transforms/IPO/PassManagerBuilder.h
===
--- llvm/include/llvm/Transforms/IPO/PassManagerBuilder.h
+++ llvm/include/llvm/Transforms/IPO/PassManagerBuilder.h
@@ -152,6 +152,7 @@
   bool VerifyInput;
   bool VerifyOutput;
   bool MergeFunctions;
+  bool SplitColdCode;
   bool PrepareForLTO;
   bool PrepareForThinLTO;
   bool PerformThinLTO;
Index: llvm/include/llvm/Passes/PassBuilder.h
===
--- llvm/include/llvm/Passes/PassBuilder.h
+++ llvm/include/llvm/Passes/PassBuilder.h
@@ -577,6 +577,10 @@
 TopLevelPipelineParsingCallbacks.push_back(C);
   }
 
+  /// Enable or disable the hot/cold splitting optimization. By default, it is
+  /// disabled.
+  void setEnableHotColdSplitting(bool Enabled);
+
 private:
   static Optional>
   parsePipelineText(StringRef Text);
@@ -664,6 +668,8 @@
   // AA callbacks
   SmallVector, 2>
   AAParsingCallbacks;
+  // Tunable passes
+  bool SplitColdCode = false;
 };
 
 /// This utility template takes care of adding require<> and invalidate<>
Index: clang/test/CodeGen/split-cold-code.c
===
--- /dev/null
+++ clang/test/CodeGen/split-cold-code.c
@@ -0,0 +1,81 @@
+// === Old PM ===
+// No splitting at -O0.
+// RUN: %clang_cc1 -O0 -fsplit-cold-code -mllvm -debug-pass=Structure \
+// RUN:   -emit-llvm -o /dev/null %s 2>&1 | FileCheck --check-prefix=OLDPM-NO-SPLIT %s
+//
+// No splitting at -Oz.
+// RUN: %clang_cc1 -Oz -fsplit-cold-code -mllvm -debug-pass=Structure \
+// RUN:   -emit-llvm -o /dev/null %s 2>&1 | FileCheck --check-prefix=OLDPM-NO-SPLIT %s
+//
+// No splitting by default, even at -O3.
+// RUN: %clang_cc1 -O3 -mllvm -debug-pass=Structure \
+// RUN:   -emit-llvm -o /dev/null %s 2>&1 | FileCheck --check-prefix=OLDPM-NO-SPLIT %s
+//
+// No splitting when it's explicitly disabled.
+// RUN: %clang_cc1 -O3 -fno-split-cold-code -mllvm -debug-pass=Structure \
+// RUN:   -emit-llvm -o /dev/null %s 2>&1 | FileCheck --check-prefix=OLDPM-NO-SPLIT %s
+//
+// No splitting when LLVM passes are disabled.
+// RUN: %clang_cc1 -O3 -fsplit-cold-code -disable-llvm-passes -mllvm -debug-pass=Structure \
+// RUN:   -emit-llvm -o /dev/null %s 2>&1 | FileCheck --check-prefix=OLDPM-NO-SPLIT %s
+//
+// Split at -O1.
+// RUN: %clang_cc1 -O1 -fsplit-cold-code -mllvm -debug-pass=Structure \
+// RUN:   -emit-llvm -o /dev/null %s 2>&1 | FileCheck --check-prefix=OLDPM-SPLIT %s
+//
+// Split at -Os.
+// RUN: %clang_cc1 -Os -fsplit-cold-code -mllvm -debug-pass=Structure \
+// RUN:   -emit-llvm -o /dev/null %s 2>&1 | FileCheck --check-prefix=OLDPM-SPLIT %s
+//
+// Split at -O2.
+// RUN: %

[PATCH] D57230: [analyzer] Toning down invalidation a bit

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

There seem to be a few regressions - weird memory leaks of inner objects in C++ 
destructors. Trying to investigate/reproduce.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57230



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

[PATCH] D57850: [analyzer] Emit an error rather than assert on invalid checker option input

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

Szelethus created this revision.
Szelethus added reviewers: george.karpenkov, NoQ, xazax.hun, rnkovacs, 
baloghadamsoftware.
Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, szepet, whisperity.
Herald added a reviewer: teemperor.
Herald added a project: clang.

Asserting on invalid input isn't very nice, hence the patch to emit an error 
instead.

This is the first of many patches to overhaul the way we handle checker 
options, and I admit that it's a kind of "things that didn't fit anywhere else" 
patch.


Repository:
  rC Clang

https://reviews.llvm.org/D57850

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/StaticAnalyzer/Core/CheckerManager.h
  lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
  lib/StaticAnalyzer/Checkers/CloneChecker.cpp
  lib/StaticAnalyzer/Checkers/MoveChecker.cpp
  lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
  test/Analysis/copypaste/suspicious-clones.cpp
  test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp
  test/Analysis/outofbound.c
  test/Analysis/padding_c.c
  test/Analysis/undef-buffers.c
  test/Analysis/use-after-move.cpp

Index: test/Analysis/use-after-move.cpp
===
--- test/Analysis/use-after-move.cpp
+++ test/Analysis/use-after-move.cpp
@@ -27,6 +27,17 @@
 // RUN:  -analyzer-config cplusplus.Move:WarnOn=All -DAGGRESSIVE\
 // RUN:  -analyzer-checker debug.ExprInspection
 
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=cplusplus.Move \
+// RUN:   -analyzer-config cplusplus.Move:WarnOn="a bunch of things" \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-MOVE-INVALID-VALUE
+
+// CHECK-MOVE-INVALID-VALUE: (frontend): invalid input for checker option
+// CHECK-MOVE-INVALID-VALUE-SAME: 'cplusplus.Move:WarnOn', that expects either
+// CHECK-MOVE-INVALID-VALUE-SAME: "KnownsOnly", "KnownsAndLocals" or "All"
+// CHECK-MOVE-INVALID-VALUE-SAME: string value
+
 #include "Inputs/system-header-simulator-cxx.h"
 
 void clang_analyzer_warnIfReached();
Index: test/Analysis/undef-buffers.c
===
--- test/Analysis/undef-buffers.c
+++ test/Analysis/undef-buffers.c
@@ -2,7 +2,7 @@
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=unix \
 // RUN:   -analyzer-checker=core.uninitialized \
-// RUN:   -analyzer-config unix:Optimistic=true
+// RUN:   -analyzer-config unix.DynamicMemoryModeling:Optimistic=true
 
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
Index: test/Analysis/padding_c.c
===
--- test/Analysis/padding_c.c
+++ test/Analysis/padding_c.c
@@ -1,4 +1,16 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=optin.performance -analyzer-config optin.performance.Padding:AllowedPad=2 -verify %s
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=optin.performance \
+// RUN:   -analyzer-config optin.performance.Padding:AllowedPad=2
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=optin.performance.Padding \
+// RUN:   -analyzer-config optin.performance.Padding:AllowedPad=-10 \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-PAD-NEGATIVE-VALUE
+
+// CHECK-PAD-NEGATIVE-VALUE: (frontend): invalid input for checker option
+// CHECK-PAD-NEGATIVE-VALUE-SAME: 'optin.performance.Padding:AllowedPad', that
+// CHECK-PAD-NEGATIVE-VALUE-SAME: expects a non-negative value
 
 #if __has_include()
 #include 
Index: test/Analysis/outofbound.c
===
--- test/Analysis/outofbound.c
+++ test/Analysis/outofbound.c
@@ -2,7 +2,7 @@
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=unix \
 // RUN:   -analyzer-checker=alpha.security.ArrayBound \
-// RUN:   -analyzer-config unix:Optimistic=true
+// RUN:   -analyzer-config unix.DynamicMemoryModeling:Optimistic=true
 
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
Index: test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp
===
--- test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp
+++ test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp
@@ -3,6 +3,20 @@
 // RUN:   -analyzer-config alpha.cplusplus.UninitializedObject:IgnoreRecordsWithField="[Tt]ag|[Kk]ind" \
 // RUN:   -std=c++11 -verify  %s
 
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=alpha.cplusplus.UninitializedObject \
+// RUN:   -analyzer-config \
+// RUN: alpha.cplusplus.UninitializedObject:IgnoreRecordsWithField="([)]" \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-UNINIT-INVALID-REGEX
+
+// CHECK-UNINIT-INVALID-REGEX: (frontend)

[PATCH] D57850: [analyzer] Emit an error rather than assert on invalid checker option input

[Raphael Isemann via Phabricator via cfe-commits]

teemperor resigned from this revision.
teemperor added a comment.

LGTM for the CloneChecker changes.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57850



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