date:20171108

[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

2017-11-08 Thread Jonas Toth via Phabricator via cfe-commits

JonasToth added a comment.

@aaron.ballman and @alexfh ping.


https://reviews.llvm.org/D37808



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

[PATCH] D39784: OpenCL: Assume inline asm is convergent

2017-11-08 Thread Matt Arsenault via Phabricator via cfe-commits

arsenm created this revision.
Herald added subscribers: yaxunl, wdng.

Already done for CUDA.


https://reviews.llvm.org/D39784

Files:
  lib/CodeGen/CGStmt.cpp
  test/CodeGenOpenCL/convergent.cl


Index: test/CodeGenOpenCL/convergent.cl
===
--- test/CodeGenOpenCL/convergent.cl
+++ test/CodeGenOpenCL/convergent.cl
@@ -126,6 +126,14 @@
 
 // CHECK: declare spir_func void @nodupfun(){{[^#]*}} #[[attr3:[0-9]+]]
 
+// CHECK-LABEL: @assume_convergent_asm
+// CHECK: tail call void asm sideeffect "s_barrier", ""() #4
+kernel void assume_convergent_asm()
+{
+  __asm__ volatile("s_barrier");
+
+}
+
 // CHECK: attributes #0 = { noinline norecurse nounwind "
 // CHECK: attributes #1 = { {{[^}]*}}convergent{{[^}]*}} }
 // CHECK: attributes #2 = { {{[^}]*}}convergent{{[^}]*}} }
Index: lib/CodeGen/CGStmt.cpp
===
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -2149,10 +2149,11 @@
   llvm::ConstantAsMetadata::get(Loc)));
   }
 
-  if (getLangOpts().CUDA && getLangOpts().CUDAIsDevice) {
-// Conservatively, mark all inline asm blocks in CUDA as convergent
-// (meaning, they may call an intrinsically convergent op, such as 
bar.sync,
-// and so can't have certain optimizations applied around them).
+  if (getLangOpts().assumeFunctionsAreConvergent()) {
+// Conservatively, mark all inline asm blocks in CUDA or OpenCL as
+// convergent (meaning, they may call an intrinsically convergent op, such
+// as bar.sync, and so can't have certain optimizations applied around
+// them).
 Result->addAttribute(llvm::AttributeList::FunctionIndex,
  llvm::Attribute::Convergent);
   }


Index: test/CodeGenOpenCL/convergent.cl
===
--- test/CodeGenOpenCL/convergent.cl
+++ test/CodeGenOpenCL/convergent.cl
@@ -126,6 +126,14 @@
 
 // CHECK: declare spir_func void @nodupfun(){{[^#]*}} #[[attr3:[0-9]+]]
 
+// CHECK-LABEL: @assume_convergent_asm
+// CHECK: tail call void asm sideeffect "s_barrier", ""() #4
+kernel void assume_convergent_asm()
+{
+  __asm__ volatile("s_barrier");
+
+}
+
 // CHECK: attributes #0 = { noinline norecurse nounwind "
 // CHECK: attributes #1 = { {{[^}]*}}convergent{{[^}]*}} }
 // CHECK: attributes #2 = { {{[^}]*}}convergent{{[^}]*}} }
Index: lib/CodeGen/CGStmt.cpp
===
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -2149,10 +2149,11 @@
   llvm::ConstantAsMetadata::get(Loc)));
   }
 
-  if (getLangOpts().CUDA && getLangOpts().CUDAIsDevice) {
-// Conservatively, mark all inline asm blocks in CUDA as convergent
-// (meaning, they may call an intrinsically convergent op, such as bar.sync,
-// and so can't have certain optimizations applied around them).
+  if (getLangOpts().assumeFunctionsAreConvergent()) {
+// Conservatively, mark all inline asm blocks in CUDA or OpenCL as
+// convergent (meaning, they may call an intrinsically convergent op, such
+// as bar.sync, and so can't have certain optimizations applied around
+// them).
 Result->addAttribute(llvm::AttributeList::FunctionIndex,
  llvm::Attribute::Convergent);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39332: [clang-refactor] Introduce a new rename rule for qualified symbols

2017-11-08 Thread Haojian Wu via Phabricator via cfe-commits

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

Fix remaining nits.


https://reviews.llvm.org/D39332

Files:
  include/clang/Tooling/Refactoring/Rename/RenamingAction.h
  lib/Tooling/Refactoring/RefactoringActions.cpp
  lib/Tooling/Refactoring/Rename/RenamingAction.cpp
  test/Refactor/LocalRename/QualifiedRename.cpp
  tools/clang-refactor/ClangRefactor.cpp

Index: tools/clang-refactor/ClangRefactor.cpp
===
--- tools/clang-refactor/ClangRefactor.cpp
+++ tools/clang-refactor/ClangRefactor.cpp
@@ -257,20 +257,19 @@
   RefactoringActionRules ActionRules,
   cl::OptionCategory &Category)
   : SubCommand(Action->getCommand(), Action->getDescription()),
-Action(std::move(Action)), ActionRules(std::move(ActionRules)),
-HasSelection(false) {
+Action(std::move(Action)), ActionRules(std::move(ActionRules)) {
 // Check if the selection option is supported.
 for (const auto &Rule : this->ActionRules) {
-  if ((HasSelection = Rule->hasSelectionRequirement()))
+  if (Rule->hasSelectionRequirement()) {
+Selection = llvm::make_unique>(
+"selection",
+cl::desc(
+"The selected source range in which the refactoring should "
+"be initiated (::-: or "
+"::)"),
+cl::cat(Category), cl::sub(*this));
 break;
-}
-if (HasSelection) {
-  Selection = llvm::make_unique>(
-  "selection",
-  cl::desc("The selected source range in which the refactoring should "
-   "be initiated (::-: or "
-   "::)"),
-  cl::cat(Category), cl::sub(*this));
+  }
 }
 // Create the refactoring options.
 for (const auto &Rule : this->ActionRules) {
@@ -284,21 +283,18 @@
 
   const RefactoringActionRules &getActionRules() const { return ActionRules; }
 
-  /// Parses the command-line arguments that are specific to this rule.
+  /// Parses the "-selection" command-line argument.
   ///
   /// \returns true on error, false otherwise.
-  bool parseArguments() {
+  bool parseSelectionArgument() {
 if (Selection) {
   ParsedSelection = SourceSelectionArgument::fromString(*Selection);
   if (!ParsedSelection)
 return true;
 }
 return false;
   }
 
-  // Whether the selection is supported by any rule in the subcommand.
-  bool hasSelection() const { return HasSelection; }
-
   SourceSelectionArgument *getSelection() const {
 assert(Selection && "selection not supported!");
 return ParsedSelection.get();
@@ -314,8 +310,6 @@
   std::unique_ptr> Selection;
   std::unique_ptr ParsedSelection;
   RefactoringActionCommandLineOptions Options;
-  // Whether the selection is supported by any rule in the subcommand.
-  bool HasSelection;
 };
 
 class ClangRefactorConsumer final : public ClangRefactorToolConsumerInterface {
@@ -403,28 +397,33 @@
 // If the selection option is test specific, we use a test-specific
 // consumer.
 std::unique_ptr TestConsumer;
-if (SelectedSubcommand->hasSelection())
+bool HasSelection = MatchingRule->hasSelectionRequirement();
+if (HasSelection)
   TestConsumer = SelectedSubcommand->getSelection()->createCustomConsumer();
 ClangRefactorToolConsumerInterface *ActiveConsumer =
 TestConsumer ? TestConsumer.get() : Consumer.get();
 ActiveConsumer->beginTU(AST);
-// FIXME (Alex L): Implement non-selection based invocation path.
-if (SelectedSubcommand->hasSelection()) {
+
+auto InvokeRule = [&](RefactoringResultConsumer &Consumer) {
+  if (opts::Verbose)
+logInvocation(*SelectedSubcommand, Context);
+  MatchingRule->invoke(*ActiveConsumer, Context);
+};
+if (HasSelection) {
   assert(SelectedSubcommand->getSelection() &&
  "Missing selection argument?");
   if (opts::Verbose)
 SelectedSubcommand->getSelection()->print(llvm::outs());
   if (SelectedSubcommand->getSelection()->forAllRanges(
   Context.getSources(), [&](SourceRange R) {
 Context.setSelectionRange(R);
-if (opts::Verbose)
-  logInvocation(*SelectedSubcommand, Context);
-MatchingRule->invoke(*ActiveConsumer, Context);
+InvokeRule(*ActiveConsumer);
   }))
 HasFailed = true;
   ActiveConsumer->endTU();
   return;
 }
+InvokeRule(*ActiveConsumer);
 ActiveConsumer->endTU();
   }
 
@@ -529,23 +528,24 @@
   }
 
   llvm::Expected
-  getMatchingRule(const RefactoringActionSubcommand &Subcommand) {
+  getMatchingRule(RefactoringActionSubcommand &Subcommand) {
 SmallVector MatchingRules;
 llvm::StringSet<> MissingOptions;
 
 for (const auto &Rule : Subcommand.getActionRules()) {
-  bool SelectionMatches = tr

r317672 - [clang-refactor] Introduce a new rename rule for qualified symbols

2017-11-08 Thread Haojian Wu via cfe-commits

Author: hokein
Date: Wed Nov  8 00:56:56 2017
New Revision: 317672

URL: http://llvm.org/viewvc/llvm-project?rev=317672&view=rev
Log:
[clang-refactor] Introduce a new rename rule for qualified symbols

Summary: Prototype of a new rename rule for renaming qualified symbol.

Reviewers: arphaman, ioeric, sammccall

Reviewed By: arphaman, sammccall

Subscribers: jklaehn, cfe-commits, klimek

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

Added:
cfe/trunk/test/Refactor/LocalRename/QualifiedRename.cpp
Modified:
cfe/trunk/include/clang/Tooling/Refactoring/Rename/RenamingAction.h
cfe/trunk/lib/Tooling/Refactoring/RefactoringActions.cpp
cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
cfe/trunk/tools/clang-refactor/ClangRefactor.cpp

Modified: cfe/trunk/include/clang/Tooling/Refactoring/Rename/RenamingAction.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/Rename/RenamingAction.h?rev=317672&r1=317671&r2=317672&view=diff
==
--- cfe/trunk/include/clang/Tooling/Refactoring/Rename/RenamingAction.h 
(original)
+++ cfe/trunk/include/clang/Tooling/Refactoring/Rename/RenamingAction.h Wed Nov 
 8 00:56:56 2017
@@ -66,6 +66,28 @@ private:
   std::string NewName;
 };
 
+class QualifiedRenameRule final : public SourceChangeRefactoringRule {
+public:
+  static Expected initiate(RefactoringRuleContext 
&Context,
+std::string OldQualifiedName,
+std::string NewQualifiedName);
+
+  static const RefactoringDescriptor &describe();
+
+private:
+  QualifiedRenameRule(const NamedDecl *ND,
+  std::string NewQualifiedName)
+  : ND(ND), NewQualifiedName(std::move(NewQualifiedName)) {}
+
+  Expected
+  createSourceReplacements(RefactoringRuleContext &Context) override;
+
+  // A NamedDecl which indentifies the the symbol being renamed.
+  const NamedDecl *ND;
+  // The new qualified name to change the symbol to.
+  std::string NewQualifiedName;
+};
+
 /// Returns source replacements that correspond to the rename of the given
 /// symbol occurrences.
 llvm::Expected>

Modified: cfe/trunk/lib/Tooling/Refactoring/RefactoringActions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring/RefactoringActions.cpp?rev=317672&r1=317671&r2=317672&view=diff
==
--- cfe/trunk/lib/Tooling/Refactoring/RefactoringActions.cpp (original)
+++ cfe/trunk/lib/Tooling/Refactoring/RefactoringActions.cpp Wed Nov  8 
00:56:56 2017
@@ -46,6 +46,22 @@ public:
   }
 };
 
+class OldQualifiedNameOption : public RequiredRefactoringOption {
+public:
+  StringRef getName() const override { return "old-qualified-name"; }
+  StringRef getDescription() const override {
+return "The old qualified name to be renamed";
+  }
+};
+
+class NewQualifiedNameOption : public RequiredRefactoringOption {
+public:
+  StringRef getName() const override { return "new-qualified-name"; }
+  StringRef getDescription() const override {
+return "The new qualified name to change the symbol to";
+  }
+};
+
 class NewNameOption : public RequiredRefactoringOption {
 public:
   StringRef getName() const override { return "new-name"; }
@@ -70,6 +86,10 @@ public:
 RefactoringActionRules Rules;
 Rules.push_back(createRefactoringActionRule(
 SourceRangeSelectionRequirement(), 
OptionRequirement()));
+// FIXME: Use NewNameOption.
+Rules.push_back(createRefactoringActionRule(
+OptionRequirement(),
+OptionRequirement()));
 return Rules;
   }
 };

Modified: cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp?rev=317672&r1=317671&r2=317672&view=diff
==
--- cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp (original)
+++ cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp Wed Nov  8 
00:56:56 2017
@@ -31,6 +31,8 @@
 #include "clang/Tooling/Refactoring/Rename/USRLocFinder.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Errc.h"
+#include "llvm/Support/Error.h"
 #include 
 #include 
 
@@ -93,6 +95,60 @@ RenameOccurrences::createSourceReplaceme
   *Occurrences, Context.getASTContext().getSourceManager(), Name);
 }
 
+Expected
+QualifiedRenameRule::initiate(RefactoringRuleContext &Context,
+  std::string OldQualifiedName,
+  std::string NewQualifiedName) {
+  const NamedDecl *ND =
+  getNamedDeclFor(Context.getASTContext(), OldQualifiedName);
+  if (!ND)
+return llvm::make_error("Could not find symbol " +
+   OldQualifiedName,
+

[PATCH] D39332: [clang-refactor] Introduce a new rename rule for qualified symbols

2017-11-08 Thread Haojian Wu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL317672: [clang-refactor] Introduce a new rename rule for 
qualified symbols (authored by hokein).

Repository:
  rL LLVM

https://reviews.llvm.org/D39332

Files:
  cfe/trunk/include/clang/Tooling/Refactoring/Rename/RenamingAction.h
  cfe/trunk/lib/Tooling/Refactoring/RefactoringActions.cpp
  cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
  cfe/trunk/test/Refactor/LocalRename/QualifiedRename.cpp
  cfe/trunk/tools/clang-refactor/ClangRefactor.cpp

Index: cfe/trunk/test/Refactor/LocalRename/QualifiedRename.cpp
===
--- cfe/trunk/test/Refactor/LocalRename/QualifiedRename.cpp
+++ cfe/trunk/test/Refactor/LocalRename/QualifiedRename.cpp
@@ -0,0 +1,24 @@
+// RUN: clang-refactor local-rename -old-qualified-name="foo::A" -new-qualified-name="bar::B" %s -- -std=c++11 2>&1 | grep -v CHECK | FileCheck %s
+
+namespace foo {
+class A {};
+}
+// CHECK: namespace foo {
+// CHECK-NEXT: class B {};
+// CHECK-NEXT: }
+
+namespace bar {
+void f(foo::A* a) {
+  foo::A b;
+}
+// CHECK: void f(B* a) {
+// CHECK-NEXT:   B b;
+// CHECK-NEXT: }
+}
+
+void f(foo::A* a) {
+  foo::A b;
+}
+// CHECK: void f(bar::B* a) {
+// CHECK-NEXT:   bar::B b;
+// CHECK-NEXT: }
Index: cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
===
--- cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
+++ cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
@@ -31,6 +31,8 @@
 #include "clang/Tooling/Refactoring/Rename/USRLocFinder.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Errc.h"
+#include "llvm/Support/Error.h"
 #include 
 #include 
 
@@ -93,6 +95,60 @@
   *Occurrences, Context.getASTContext().getSourceManager(), Name);
 }
 
+Expected
+QualifiedRenameRule::initiate(RefactoringRuleContext &Context,
+  std::string OldQualifiedName,
+  std::string NewQualifiedName) {
+  const NamedDecl *ND =
+  getNamedDeclFor(Context.getASTContext(), OldQualifiedName);
+  if (!ND)
+return llvm::make_error("Could not find symbol " +
+   OldQualifiedName,
+   llvm::errc::invalid_argument);
+  return QualifiedRenameRule(ND, std::move(NewQualifiedName));
+}
+
+const RefactoringDescriptor &QualifiedRenameRule::describe() {
+  static const RefactoringDescriptor Descriptor = {
+  /*Name=*/"local-qualified-rename",
+  /*Title=*/"Qualified Rename",
+  /*Description=*/
+  R"(Finds and renames qualified symbols in code within a translation unit.
+It is used to move/rename a symbol to a new namespace/name:
+  * Supported symbols: classes, class members, functions, enums, and type alias.
+  * Renames all symbol occurrences from the old qualified name to the new
+qualified name. All symbol references will be correctly qualified; For
+symbol definitions, only name will be changed.
+For example, rename "A::Foo" to "B::Bar":
+  Old code:
+namespace foo {
+class A {};
+}
+
+namespace bar {
+void f(foo::A a) {}
+}
+
+  New code after rename:
+namespace foo {
+class B {};
+}
+
+namespace bar {
+void f(B b) {}
+})"
+  };
+  return Descriptor;
+}
+
+Expected
+QualifiedRenameRule::createSourceReplacements(RefactoringRuleContext &Context) {
+  auto USRs = getUSRsForDeclaration(ND, Context.getASTContext());
+  assert(!USRs.empty());
+  return tooling::createRenameAtomicChanges(
+  USRs, NewQualifiedName, Context.getASTContext().getTranslationUnitDecl());
+}
+
 Expected>
 createRenameReplacements(const SymbolOccurrences &Occurrences,
  const SourceManager &SM, const SymbolName &NewName) {
Index: cfe/trunk/lib/Tooling/Refactoring/RefactoringActions.cpp
===
--- cfe/trunk/lib/Tooling/Refactoring/RefactoringActions.cpp
+++ cfe/trunk/lib/Tooling/Refactoring/RefactoringActions.cpp
@@ -46,6 +46,22 @@
   }
 };
 
+class OldQualifiedNameOption : public RequiredRefactoringOption {
+public:
+  StringRef getName() const override { return "old-qualified-name"; }
+  StringRef getDescription() const override {
+return "The old qualified name to be renamed";
+  }
+};
+
+class NewQualifiedNameOption : public RequiredRefactoringOption {
+public:
+  StringRef getName() const override { return "new-qualified-name"; }
+  StringRef getDescription() const override {
+return "The new qualified name to change the symbol to";
+  }
+};
+
 class NewNameOption : public RequiredRefactoringOption {
 public:
   StringRef getName() const override { return "new-name"; }
@@ -70,6 +86,10 @@
 RefactoringActionRules Rules;
 Rules.push_back(createRefactoringActionRule(
 SourceRangeSelectionReq

[PATCH] D35894: [clangd] Code hover for Clangd

2017-11-08 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

Sorry for late response, was on vacation.
+1 to all @malaperle 's comments.

Here are some extra quick comments, will take a closer look later.




Comment at: clangd/ClangdServer.cpp:11
 #include "ClangdServer.h"
+#include "Protocol.h"
 #include "clang/Format/Format.h"

malaperle wrote:
> I don't know if the intention was to keep the protocol completely out of the 
> ClangdServer class. Maybe someone else can clarify. But either way, I see 
> that ClangdUnit does include "Protocol.h" so it's probably OK for now.
We don't want dependencies on JSON parsing/unparsing and the protocol itself. 
We reuse some of the LSP class definitions to avoid code duplication, though.

This include is redundant, though, as `ClangdServer.h` already include 
`Protocol.h`. 



Comment at: clangd/ClangdServer.h:281
   /// Get definition of symbol at a specified \p Line and \p Column in \p File.
-  llvm::Expected>> findDefinitions(PathRef File,
+  llvm::Expected>>> 
findDefinitions(PathRef File,
 Position Pos);

malaperle wrote:
> Location can be deduced from Decl*. I don't think this should be a pair. I 
> also think we need a more general finder that just gets either the Decl* or 
> MacroDef* at the "cursor". I think CXCursor does something similar in that it 
> stores either Decl* or MacroDef*. I'm not sure it would be worth moving 
> CXCursor from libclang but perhaps something stripped down for Clangd would 
> be OK. This new finder will be able to be reused by findDefinition, 
> highlights, hover, references and more.
> We can make one patch that refactors the existing code so that 
> findDefinitions uses this new finder class.
We should not return `Decl*` from `ClangdServer::findDefinitions`, it's an 
internal clang object (there are many other reasons why it's bad). 

It's ok the change the `findDefinitions` helper function inside 
`ClangdUnit.cpp`, though. Please change it instead.

As for the `CXCursor`, I think the simplest approach would be to return two 
vectors (`vector` and `vector`). I think clangd would 
eventually get its own `CXCursor`-like things, possible reusing code or 
wrapping CXCursor. But for now let's keep things simple.


https://reviews.llvm.org/D35894



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

[clang-tools-extra] r317673 - [clangd] loosen tests for flag-dependence revealed by r317670

2017-11-08 Thread Sam McCall via cfe-commits

Author: sammccall
Date: Wed Nov  8 01:15:01 2017
New Revision: 317673

URL: http://llvm.org/viewvc/llvm-project?rev=317673&view=rev
Log:
[clangd] loosen tests for flag-dependence revealed by r317670

Modified:
clang-tools-extra/trunk/test/clangd/completion-snippet.test
clang-tools-extra/trunk/test/clangd/completion.test

Modified: clang-tools-extra/trunk/test/clangd/completion-snippet.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/completion-snippet.test?rev=317673&r1=317672&r2=317673&view=diff
==
--- clang-tools-extra/trunk/test/clangd/completion-snippet.test (original)
+++ clang-tools-extra/trunk/test/clangd/completion-snippet.test Wed Nov  8 
01:15:01 2017
@@ -68,8 +68,9 @@ Content-Length: 148
 # CHECK-NEXT:  "label": "operator=(const fake &)",
 # CHECK-NEXT:  "sortText": "79operator="
 # CHECK-NEXT:},
-# CHECK-NEXT:{
-# CHECK-NEXT:  "detail": "void",
+# FIXME: Why do some buildbots show an extra operator==(fake&&) here?
+#  CHECK:{
+#  CHECK:  "detail": "void",
 # CHECK-NEXT:  "filterText": "~fake",
 # CHECK-NEXT:  "insertText": "~fake()",
 # CHECK-NEXT:  "insertTextFormat": 1,

Modified: clang-tools-extra/trunk/test/clangd/completion.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/completion.test?rev=317673&r1=317672&r2=317673&view=diff
==
--- clang-tools-extra/trunk/test/clangd/completion.test (original)
+++ clang-tools-extra/trunk/test/clangd/completion.test Wed Nov  8 01:15:01 2017
@@ -68,8 +68,9 @@ Content-Length: 148
 # CHECK-NEXT:  "label": "operator=(const fake &)",
 # CHECK-NEXT:  "sortText": "79operator="
 # CHECK-NEXT:},
-# CHECK-NEXT:{
-# CHECK-NEXT:  "detail": "void",
+# FIXME: Why do some buildbots show an extra operator==(fake&&) here?
+#  CHECK:{
+#  CHECK:  "detail": "void",
 # CHECK-NEXT:  "filterText": "~fake",
 # CHECK-NEXT:  "insertText": "~fake",
 # CHECK-NEXT:  "insertTextFormat": 1,
@@ -137,8 +138,8 @@ Content-Length: 148
 # CHECK-NEXT:  "label": "operator=(const fake &)",
 # CHECK-NEXT:  "sortText": "79operator="
 # CHECK-NEXT:},
-# CHECK-NEXT:{
-# CHECK-NEXT:  "detail": "void",
+#  CHECK:{
+#  CHECK:  "detail": "void",
 # CHECK-NEXT:  "filterText": "~fake",
 # CHECK-NEXT:  "insertText": "~fake",
 # CHECK-NEXT:  "insertTextFormat": 1,
@@ -184,8 +185,8 @@ Content-Length: 148
 # CHECK-NEXT:  "label": "operator=(const fancy &)",
 # CHECK-NEXT:  "sortText": "79operator="
 # CHECK-NEXT:},
-# CHECK-NEXT:{
-# CHECK-NEXT:  "detail": "void",
+#  CHECK:{
+#  CHECK:  "detail": "void",
 # CHECK-NEXT:  "filterText": "~fancy",
 # CHECK-NEXT:  "insertText": "~fancy",
 # CHECK-NEXT:  "insertTextFormat": 1,


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

[PATCH] D39676: [clangd] Add rename support.

2017-11-08 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

Just a few minor code style comments.




Comment at: clangd/ClangdServer.cpp:367
+Context.setASTContext(AST->getASTContext());
+auto rename = clang::tooling::RenameOccurrences::initiate(
+Context, SourceRange(SourceLocationBeg), NewName.str());

NIT: local vars are `UpperCamelCase`



Comment at: clangd/ClangdUnit.cpp:1399
+
+SourceLocation getBeginningOfIdentifier(ParsedAST &Unit, const Position &Pos,
+const FileEntry *FE) {

sammccall wrote:
> nit: the rest of this file defines functions outside their namespace.
> 
> TBH I prefer the style you're using here, but we should be consistent within 
> a file.
+1. Let's document and enforce the preferred style in new commits.
But better done in a separate commit for all of clangd and let's stick to the 
original style in this file for now.


https://reviews.llvm.org/D39676



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

[PATCH] D39571: [clangd] DidChangeConfiguration Notification

2017-11-08 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov requested changes to this revision.
ilya-biryukov added a comment.

I don't think we should pass the very general configuration map to 
`ClangdServer`. Especially given that we can easily update 
`DirectoryBaseCompilationDatabase` instance in `ClangdLSPServer` itself.
What are your thoughts on this?

Comment at: clangd/ClangdLSPServer.cpp:199
+Ctx C, DidChangeConfigurationParams &Params) {
+  std::map SettingsMap;
+  SettingsMap.insert(std::pair(

malaperle wrote:
> I'm thinking, maybe it's better not to have the map and just pass along the 
> ClangdConfigurationParams to the server (instead of the map). I think 
> ClangdConfigurationParams is more useful as a structure than a "flat" map 
> with all the keys being at the same level. In ClangdConfigurationParams, 
> we'll be able to add sub-configurations sections (index, code-completion, 
> more?) which is well suited to reflect the JSON format.
> 
> Unless perhaps you had another use case for the map that I'm not thinking 
> about?
I don't think `ClangdServer` should handle changes to compilation database path 
at all.
It accepts `GlobalCompilationDatabase` as a parameter and does not own it, so 
`ClangdLSPServer` can mutate it properly.

Comment at: clangd/ClangdServer.h:288
+  /// Modify configuration settings based on what is contained inside
+  /// ChangedSettings
+  void changeConfiguration(std::map ChangedSettings);

NIT: Add full stop at the end of comments.

Comment at: clangd/ClangdServer.h:289
+  /// ChangedSettings
+  void changeConfiguration(std::map ChangedSettings);
+

This function is way too general for `ClangdServer`'s interface, can we have 
more fine-grained settings here (i.e. `setCompletionParameters` etc?) and 
handle the "general" case in `ClangdLSPServer` (i.e., unknown setting names, 
invalid setting parameters, json parsing, etc.)?

I suggest we remove this function altogether.

https://reviews.llvm.org/D39571

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

r317676 - Moved QualTypeNames.h from Tooling to AST.

2017-11-08 Thread Ilya Biryukov via cfe-commits

Author: ibiryukov
Date: Wed Nov  8 02:39:03 2017
New Revision: 317676

URL: http://llvm.org/viewvc/llvm-project?rev=317676&view=rev
Log:
Moved QualTypeNames.h from Tooling to AST.

Summary:
For code reuse in SemaCodeComplete.
Note that the tests for QualTypeNames are still in Tooling as they use
Tooling's common testing code.

Reviewers: rsmith, saugustine, rnk, klimek, bkramer

Reviewed By: rnk

Subscribers: cfe-commits, mgorny

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

Added:
cfe/trunk/include/clang/AST/QualTypeNames.h
  - copied, changed from r317672, 
cfe/trunk/include/clang/Tooling/Core/QualTypeNames.h
cfe/trunk/lib/AST/QualTypeNames.cpp
  - copied, changed from r317672, 
cfe/trunk/lib/Tooling/Core/QualTypeNames.cpp
Removed:
cfe/trunk/include/clang/Tooling/Core/QualTypeNames.h
cfe/trunk/lib/Tooling/Core/QualTypeNames.cpp
Modified:
cfe/trunk/lib/AST/CMakeLists.txt
cfe/trunk/lib/Tooling/Core/CMakeLists.txt
cfe/trunk/unittests/Tooling/QualTypeNamesTest.cpp

Copied: cfe/trunk/include/clang/AST/QualTypeNames.h (from r317672, 
cfe/trunk/include/clang/Tooling/Core/QualTypeNames.h)
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/QualTypeNames.h?p2=cfe/trunk/include/clang/AST/QualTypeNames.h&p1=cfe/trunk/include/clang/Tooling/Core/QualTypeNames.h&r1=317672&r2=317676&rev=317676&view=diff
==
--- cfe/trunk/include/clang/Tooling/Core/QualTypeNames.h (original)
+++ cfe/trunk/include/clang/AST/QualTypeNames.h Wed Nov  8 02:39:03 2017
@@ -56,8 +56,8 @@
 //
 // 
===--===//
 
-#ifndef LLVM_CLANG_TOOLING_CORE_QUALTYPENAMES_H
-#define LLVM_CLANG_TOOLING_CORE_QUALTYPENAMES_H
+#ifndef LLVM_CLANG_AST_QUALTYPENAMES_H
+#define LLVM_CLANG_AST_QUALTYPENAMES_H
 
 #include "clang/AST/ASTContext.h"
 
@@ -71,9 +71,20 @@ namespace TypeName {
 /// \param[in] Ctx - the ASTContext to be used.
 /// \param[in] WithGlobalNsPrefix - If true, then the global namespace
 /// specifier "::" will be prepended to the fully qualified name.
-std::string getFullyQualifiedName(QualType QT,
-  const ASTContext &Ctx,
+std::string getFullyQualifiedName(QualType QT, const ASTContext &Ctx,
   bool WithGlobalNsPrefix = false);
-}  // end namespace TypeName
-}  // end namespace clang
-#endif  // LLVM_CLANG_TOOLING_CORE_QUALTYPENAMES_H
+
+/// \brief Generates a QualType that can be used to name the same type
+/// if used at the end of the current translation unit. This ignores
+/// issues such as type shadowing.
+///
+/// \param[in] QT - the type for which the fully qualified type will be
+/// returned.
+/// \param[in] Ctx - the ASTContext to be used.
+/// \param[in] WithGlobalNsPrefix - Indicate whether the global namespace
+/// specifier "::" should be prepended or not.
+QualType getFullyQualifiedType(QualType QT, const ASTContext &Ctx,
+   bool WithGlobalNsPrefix = false);
+} // end namespace TypeName
+} // end namespace clang
+#endif // LLVM_CLANG_TOOLING_CORE_QUALTYPENAMES_H

Removed: cfe/trunk/include/clang/Tooling/Core/QualTypeNames.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Core/QualTypeNames.h?rev=317675&view=auto
==
--- cfe/trunk/include/clang/Tooling/Core/QualTypeNames.h (original)
+++ cfe/trunk/include/clang/Tooling/Core/QualTypeNames.h (removed)
@@ -1,79 +0,0 @@
-//===--- QualTypeNames.h - Generate Complete QualType Names *- C++ -*-===//
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-// 
===--===//
-//
-// \file
-// Functionality to generate the fully-qualified names of QualTypes,
-// including recursively expanding any subtypes and template
-// parameters.
-//
-// More precisely: Generates a name that can be used to name the same
-// type if used at the end of the current translation unit--with
-// certain limitations. See below.
-//
-// This code desugars names only very minimally, so in this code:
-//
-// namespace A {
-//   struct X {};
-// }
-// using A::X;
-// namespace B {
-//   using std::tuple;
-//   typedef tuple TX;
-//   TX t;
-// }
-//
-// B::t's type is reported as "B::TX", rather than std::tuple.
-//
-// Also, this code replaces types found via using declarations with
-// their more qualified name, so for the code:
-//
-// using std::tuple;
-// tuple TInt;
-//
-// TInt's type will be named, "std::tuple".
-//
-// Limitations:
-//
-// Some types have ambiguous names at the end of a translation unit,
-// are not namable at all there, or are special cases in other ways.
-//
-// 1) Types with only local scope will have their local names:
-//
-// void foo() {
-//   struct LocalTy

[PATCH] D39224: Moved QualTypeNames.h from Tooling to AST.

2017-11-08 Thread Phabricator via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL317676: Moved QualTypeNames.h from Tooling to AST. (authored 
by ibiryukov).

Changed prior to commit:
  https://reviews.llvm.org/D39224?vs=120014&id=122044#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39224

Files:
  cfe/trunk/include/clang/AST/QualTypeNames.h
  cfe/trunk/include/clang/Tooling/Core/QualTypeNames.h
  cfe/trunk/lib/AST/CMakeLists.txt
  cfe/trunk/lib/AST/QualTypeNames.cpp
  cfe/trunk/lib/Tooling/Core/CMakeLists.txt
  cfe/trunk/lib/Tooling/Core/QualTypeNames.cpp
  cfe/trunk/unittests/Tooling/QualTypeNamesTest.cpp

Index: cfe/trunk/lib/Tooling/Core/CMakeLists.txt
===
--- cfe/trunk/lib/Tooling/Core/CMakeLists.txt
+++ cfe/trunk/lib/Tooling/Core/CMakeLists.txt
@@ -3,7 +3,6 @@
 add_clang_library(clangToolingCore
   Lookup.cpp
   Replacement.cpp
-  QualTypeNames.cpp
   Diagnostic.cpp
 
   LINK_LIBS
Index: cfe/trunk/lib/AST/CMakeLists.txt
===
--- cfe/trunk/lib/AST/CMakeLists.txt
+++ cfe/trunk/lib/AST/CMakeLists.txt
@@ -49,6 +49,7 @@
   ODRHash.cpp
   OpenMPClause.cpp
   ParentMap.cpp
+  QualTypeNames.cpp
   RawCommentList.cpp
   RecordLayout.cpp
   RecordLayoutBuilder.cpp
Index: cfe/trunk/lib/AST/QualTypeNames.cpp
===
--- cfe/trunk/lib/AST/QualTypeNames.cpp
+++ cfe/trunk/lib/AST/QualTypeNames.cpp
@@ -0,0 +1,466 @@
+//===--- QualTypeNames.cpp - Generate Complete QualType Names -===//
+//
+// The LLVM Compiler Infrastructure
+//
+//===--===//
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/AST/DeclTemplate.h"
+#include "clang/AST/DeclarationName.h"
+#include "clang/AST/GlobalDecl.h"
+#include "clang/AST/Mangle.h"
+#include "clang/AST/QualTypeNames.h"
+
+#include 
+#include 
+
+namespace clang {
+
+namespace TypeName {
+
+/// \brief Create a NestedNameSpecifier for Namesp and its enclosing
+/// scopes.
+///
+/// \param[in] Ctx - the AST Context to be used.
+/// \param[in] Namesp - the NamespaceDecl for which a NestedNameSpecifier
+/// is requested.
+/// \param[in] WithGlobalNsPrefix - Indicate whether the global namespace
+/// specifier "::" should be prepended or not.
+static NestedNameSpecifier *createNestedNameSpecifier(
+const ASTContext &Ctx,
+const NamespaceDecl *Namesp,
+bool WithGlobalNsPrefix);
+
+/// \brief Create a NestedNameSpecifier for TagDecl and its enclosing
+/// scopes.
+///
+/// \param[in] Ctx - the AST Context to be used.
+/// \param[in] TD - the TagDecl for which a NestedNameSpecifier is
+/// requested.
+/// \param[in] FullyQualify - Convert all template arguments into fully
+/// qualified names.
+/// \param[in] WithGlobalNsPrefix - Indicate whether the global namespace
+/// specifier "::" should be prepended or not.
+static NestedNameSpecifier *createNestedNameSpecifier(
+const ASTContext &Ctx, const TypeDecl *TD,
+bool FullyQualify, bool WithGlobalNsPrefix);
+
+static NestedNameSpecifier *createNestedNameSpecifierForScopeOf(
+const ASTContext &Ctx, const Decl *decl,
+bool FullyQualified, bool WithGlobalNsPrefix);
+
+static NestedNameSpecifier *getFullyQualifiedNestedNameSpecifier(
+const ASTContext &Ctx, NestedNameSpecifier *scope, bool WithGlobalNsPrefix);
+
+static bool getFullyQualifiedTemplateName(const ASTContext &Ctx,
+  TemplateName &TName,
+  bool WithGlobalNsPrefix) {
+  bool Changed = false;
+  NestedNameSpecifier *NNS = nullptr;
+
+  TemplateDecl *ArgTDecl = TName.getAsTemplateDecl();
+  // ArgTDecl won't be NULL because we asserted that this isn't a
+  // dependent context very early in the call chain.
+  assert(ArgTDecl != nullptr);
+  QualifiedTemplateName *QTName = TName.getAsQualifiedTemplateName();
+
+  if (QTName && !QTName->hasTemplateKeyword()) {
+NNS = QTName->getQualifier();
+NestedNameSpecifier *QNNS = getFullyQualifiedNestedNameSpecifier(
+Ctx, NNS, WithGlobalNsPrefix);
+if (QNNS != NNS) {
+  Changed = true;
+  NNS = QNNS;
+} else {
+  NNS = nullptr;
+}
+  } else {
+NNS = createNestedNameSpecifierForScopeOf(
+Ctx, ArgTDecl, true, WithGlobalNsPrefix);
+  }
+  if (NNS) {
+TName = Ctx.getQualifiedTemplateName(NNS,
+ /*TemplateKeyword=*/false, ArgTDecl);
+Changed = true;
+  }
+  return Changed;
+}
+
+static bool getFullyQualifiedTemplateArgument(const ASTContext &Ctx,
+  TemplateArgument &Arg,
+  bool With

[PATCH] D38538: Avoid printing some redundant name qualifiers in completion

2017-11-08 Thread Phabricator via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL317677: Avoid printing some redundant name qualifiers in 
completion (authored by ibiryukov).

Repository:
  rL LLVM

https://reviews.llvm.org/D38538

Files:
  cfe/trunk/lib/Sema/SemaCodeComplete.cpp
  cfe/trunk/test/CodeCompletion/call.cpp
  cfe/trunk/test/CodeCompletion/qualifiers-as-written.cpp
  cfe/trunk/test/CodeCompletion/uninstantiated_params.cpp
  cfe/trunk/test/Index/complete-cxx-inline-methods.cpp

Index: cfe/trunk/test/Index/complete-cxx-inline-methods.cpp
===
--- cfe/trunk/test/Index/complete-cxx-inline-methods.cpp
+++ cfe/trunk/test/Index/complete-cxx-inline-methods.cpp
@@ -25,7 +25,7 @@
 
 // RUN: c-index-test -code-completion-at=%s:4:9 -std=c++98 %s | FileCheck %s
 // RUN: c-index-test -code-completion-at=%s:13:7 -std=c++98 %s | FileCheck %s
-// CHECK:  CXXMethod:{ResultType MyCls::Vec &}{TypedText operator=}{LeftParen (}{Placeholder const MyCls::Vec &}{RightParen )} (79)
+// CHECK:  CXXMethod:{ResultType Vec &}{TypedText operator=}{LeftParen (}{Placeholder const Vec &}{RightParen )} (79)
 // CHECK-NEXT: StructDecl:{TypedText Vec}{Text ::} (75)
 // CHECK-NEXT: FieldDecl:{ResultType int}{TypedText x} (35)
 // CHECK-NEXT: FieldDecl:{ResultType int}{TypedText y} (35)
Index: cfe/trunk/test/CodeCompletion/qualifiers-as-written.cpp
===
--- cfe/trunk/test/CodeCompletion/qualifiers-as-written.cpp
+++ cfe/trunk/test/CodeCompletion/qualifiers-as-written.cpp
@@ -0,0 +1,30 @@
+struct foo {
+  typedef int type;
+
+  type method(type, foo::type, ::foo::type, ::foo::foo::type);
+};
+
+namespace ns {
+  struct bar {
+  };
+
+  struct baz {
+  };
+
+  int func(foo::type a, bar b, baz c);
+}
+
+typedef ns::bar bar;
+
+int func(foo a, bar b, ns::bar c, ns::baz d);
+using ns::func;
+
+void test() {
+  foo().method(0, 0, 0, 0);
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:23:9 %s -o - | FileCheck %s --check-prefix=CHECK-1
+  // CHECK-1: COMPLETION: method : [#type#]method(<#type#>, <#foo::type#>, <#::foo::type#>, <#::foo::foo::type#>)
+  f
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:26:3 %s -o - | FileCheck %s --check-prefix=CHECK-2
+  // CHECK-2: COMPLETION: func : [#int#]func(<#foo a#>, <#bar b#>, <#ns::bar c#>, <#ns::baz d#>
+  // CHECK-2: COMPLETION: func : [#int#]func(<#foo::type a#>, <#bar b#>, <#baz c#>
+}
Index: cfe/trunk/test/CodeCompletion/call.cpp
===
--- cfe/trunk/test/CodeCompletion/call.cpp
+++ cfe/trunk/test/CodeCompletion/call.cpp
@@ -19,10 +19,10 @@
   f(Y(), 0, 0);
   // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:19:9 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
   // CHECK-CC1: COMPLETION: Pattern : dynamic_cast<<#type#>>(<#expression#>)
-  // CHECK-CC1: f(N::Y y, <#int ZZ#>)
+  // CHECK-CC1: f(Y y, <#int ZZ#>)
   // CHECK-CC1-NEXT: f(int i, <#int j#>, int k)
   // CHECK-CC1-NEXT: f(float x, <#float y#>)
   // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:19:13 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
-  // CHECK-CC2-NOT: f(N::Y y, int ZZ)
+  // CHECK-CC2-NOT: f(Y y, int ZZ)
   // CHECK-CC2: f(int i, int j, <#int k#>)
 }
Index: cfe/trunk/test/CodeCompletion/uninstantiated_params.cpp
===
--- cfe/trunk/test/CodeCompletion/uninstantiated_params.cpp
+++ cfe/trunk/test/CodeCompletion/uninstantiated_params.cpp
@@ -9,5 +9,5 @@
   unique_ptr x;
   x.
   // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:5 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
-  // CHECK-CC1: [#void#]reset({#<#unique_ptr::pointer ptr = pointer()#>#})
+  // CHECK-CC1: [#void#]reset({#<#pointer ptr = pointer()#>#})
 }
Index: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
===
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp
@@ -14,6 +14,7 @@
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
+#include "clang/AST/QualTypeNames.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/MacroInfo.h"
@@ -1495,6 +1496,7 @@
   Policy.AnonymousTagLocations = false;
   Policy.SuppressStrongLifetime = true;
   Policy.SuppressUnwrittenScope = true;
+  Policy.SuppressScope = true;
   return Policy;
 }
 
@@ -2139,9 +2141,10 @@
   T = Method->getSendResultType(BaseType);
 else
   T = Method->getReturnType();
-  } else if (const EnumConstantDecl *Enumerator = dyn_cast(ND))
+  } else if (const EnumConstantDecl *Enumerator = dyn_cast(ND)) {
 T = Context.getTypeDeclType(cast(Enumerator->getDeclContext()));
-  else if (isa(ND)) {
+T = clang::TypeName::getFullyQualifiedType(T, Context);
+  } else if (isa(ND)) {

r317677 - Avoid printing some redundant name qualifiers in completion

2017-11-08 Thread Ilya Biryukov via cfe-commits

Author: ibiryukov
Date: Wed Nov  8 02:39:09 2017
New Revision: 317677

URL: http://llvm.org/viewvc/llvm-project?rev=317677&view=rev
Log:
Avoid printing some redundant name qualifiers in completion

Summary:
Adjusted PrintingPolicy inside code completion to avoid printing some
redundant name qualifiers.

Before this change, typedefs that were written unqualified in source
code were printed with qualifiers in completion. For example, in the
following code

struct foo {
typedef int type;
type method();
};

completion item for `method` had return type of `foo::type`, even
though the original code used `type` without qualifiers.
After this change, the completion item has return type `type`, as
originally written in the source code.

Note that this change does not suppress qualifiers written by the
user. For example, in the following code

typedef int type;
struct foo {
typedef int type;
::type method(foo::type);
};

completion item for `method` has return type of `::type` and
parameter type of `foo::type`, as originally written in the source
code.

Reviewers: arphaman, bkramer, klimek

Reviewed By: arphaman

Subscribers: mgorny, eraman, cfe-commits

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

Added:
cfe/trunk/test/CodeCompletion/qualifiers-as-written.cpp
Modified:
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/test/CodeCompletion/call.cpp
cfe/trunk/test/CodeCompletion/uninstantiated_params.cpp
cfe/trunk/test/Index/complete-cxx-inline-methods.cpp

Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=317677&r1=317676&r2=317677&view=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Wed Nov  8 02:39:09 2017
@@ -14,6 +14,7 @@
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
+#include "clang/AST/QualTypeNames.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/MacroInfo.h"
@@ -1495,6 +1496,7 @@ static PrintingPolicy getCompletionPrint
   Policy.AnonymousTagLocations = false;
   Policy.SuppressStrongLifetime = true;
   Policy.SuppressUnwrittenScope = true;
+  Policy.SuppressScope = true;
   return Policy;
 }
 
@@ -2139,9 +2141,10 @@ static void AddResultTypeChunk(ASTContex
   T = Method->getSendResultType(BaseType);
 else
   T = Method->getReturnType();
-  } else if (const EnumConstantDecl *Enumerator = 
dyn_cast(ND))
+  } else if (const EnumConstantDecl *Enumerator = 
dyn_cast(ND)) {
 T = Context.getTypeDeclType(cast(Enumerator->getDeclContext()));
-  else if (isa(ND)) {
+T = clang::TypeName::getFullyQualifiedType(T, Context);
+  } else if (isa(ND)) {
 /* Do nothing: ignore unresolved using declarations*/
   } else if (const ObjCIvarDecl *Ivar = dyn_cast(ND)) {
 if (!BaseType.isNull())

Modified: cfe/trunk/test/CodeCompletion/call.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeCompletion/call.cpp?rev=317677&r1=317676&r2=317677&view=diff
==
--- cfe/trunk/test/CodeCompletion/call.cpp (original)
+++ cfe/trunk/test/CodeCompletion/call.cpp Wed Nov  8 02:39:09 2017
@@ -19,10 +19,10 @@ void test() {
   f(Y(), 0, 0);
   // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns 
-code-completion-at=%s:19:9 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
   // CHECK-CC1: COMPLETION: Pattern : dynamic_cast<<#type#>>(<#expression#>)
-  // CHECK-CC1: f(N::Y y, <#int ZZ#>)
+  // CHECK-CC1: f(Y y, <#int ZZ#>)
   // CHECK-CC1-NEXT: f(int i, <#int j#>, int k)
   // CHECK-CC1-NEXT: f(float x, <#float y#>)
   // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:19:13 %s -o - | 
FileCheck -check-prefix=CHECK-CC2 %s
-  // CHECK-CC2-NOT: f(N::Y y, int ZZ)
+  // CHECK-CC2-NOT: f(Y y, int ZZ)
   // CHECK-CC2: f(int i, int j, <#int k#>)
 }

Added: cfe/trunk/test/CodeCompletion/qualifiers-as-written.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeCompletion/qualifiers-as-written.cpp?rev=317677&view=auto
==
--- cfe/trunk/test/CodeCompletion/qualifiers-as-written.cpp (added)
+++ cfe/trunk/test/CodeCompletion/qualifiers-as-written.cpp Wed Nov  8 02:39:09 
2017
@@ -0,0 +1,30 @@
+struct foo {
+  typedef int type;
+
+  type method(type, foo::type, ::foo::type, ::foo::foo::type);
+};
+
+namespace ns {
+  struct bar {
+  };
+
+  struct baz {
+  };
+
+  int func(foo::type a, bar b, baz c);
+}
+
+typedef ns::bar bar;
+
+int func(foo a, bar b, ns::bar c, ns::baz d);
+using ns::func;
+
+void test() {
+  foo().method(0, 0, 0, 0);
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:23:9 %s -o - | 
FileCheck %s --check-prefix=CHECK-1
+  // CHECK-1:

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

2017-11-08 Thread Roman Lebedev via Phabricator via cfe-commits

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

Ping.

In https://reviews.llvm.org/D36836#889375, @aaron.ballman wrote:

> Adding @dberlin for licensing discussion questions.


@dberlin ping? I'm wondering if you had the chance to look at this? :)


Repository:
  rL LLVM

https://reviews.llvm.org/D36836

Files:
  LICENSE.TXT
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/FunctionCognitiveComplexityCheck.LICENSE.txt
  clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
  clang-tidy/readability/FunctionCognitiveComplexityCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-function-cognitive-complexity.rst
  test/clang-tidy/readability-function-cognitive-complexity.cpp

Index: test/clang-tidy/readability-function-cognitive-complexity.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-function-cognitive-complexity.cpp
@@ -0,0 +1,954 @@
+// RUN: %check_clang_tidy %s readability-function-cognitive-complexity %t -- -config='{CheckOptions: [{key: readability-function-cognitive-complexity.Threshold, value: 0}]}' -- -std=c++11 -w
+
+// any function should be checked.
+
+extern int ext_func(int x = 0);
+
+int some_func(int x = 0);
+
+static int some_other_func(int x = 0) {}
+
+template void some_templ_func(T x = 0) {}
+
+class SomeClass {
+public:
+  int *begin(int x = 0);
+  int *end(int x = 0);
+  static int func(int x = 0);
+  template void some_templ_func(T x = 0) {}
+  SomeClass() = default;
+  SomeClass(SomeClass&) = delete;
+};
+
+// nothing ever decreases cognitive complexity, so we can check all the things
+// in one go. none of the following should increase cognitive complexity:
+void unittest_false() {
+  {};
+  ext_func();
+  some_func();
+  some_other_func();
+  some_templ_func();
+  some_templ_func();
+  SomeClass::func();
+  SomeClass C;
+  C.some_templ_func();
+  C.some_templ_func();
+  C.func();
+  C.end();
+  int i = some_func();
+  i = i;
+  i++;
+  --i;
+  i < 0;
+  int j = 0 ?: 1;
+  auto k = new int;
+  delete k;
+  throw i;
+  {
+throw i;
+  }
+end:
+  return;
+}
+
+#if 1
+#define CC100
+#else
+// this macro has cognitive complexity of 100.
+// it is needed to be able to compare the testcases with the
+// reference Sonar implementation. please place it right after the first
+// CHECK-NOTES in each function
+#define CC100 if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){}if(1){}
+#endif
+
+////
+//-- B1. Increments --//
+////
+// Check that every thing listed in B1 of the specification does indeed   //
+// recieve the base increment, and that not-body does not increase nesting//
+////
+
+// break does not increase cognitive complexity.
+// only  break LABEL  does, but it is unavaliable in C or C++
+
+// continue does not increase cognitive complexity.
+// only  continue LABEL  does, but it is unavaliable in C or C++
+
+void unittest_b1_00() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b1_00' has cognitive complexity of 33 (threshold 0) [readability-function-cognitive-complexity]
+  CC100;
+
+  if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:9: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+
+if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:11: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+} else if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:12: note: +1, nesting level increased to 2{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:18: note: +3, including nesting penalty of 2, nesting level increased to 3{{$}}
+} else {
+// CHECK-NOTES: :[[@LINE-1]]:7: note: +1, nesting level increased to 2{{$}}
+}
+  } else if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:10: note: +1, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:16: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+
+if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:11: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+} else if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:12: note: +1, nesting level increased to 2{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:18: note: +3, including nesting penalty of 2, nesting level increased to

[PATCH] D39787: [clang-tidy] Add a note about modernize-replace-random-shuffle

2017-11-08 Thread Haojian Wu via Phabricator via cfe-commits

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

LGTM.


https://reviews.llvm.org/D39787



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

[PATCH] D39676: [clangd] Add rename support.

2017-11-08 Thread Haojian Wu via Phabricator via cfe-commits

hokein updated this revision to Diff 122060.
hokein marked 10 inline comments as done.
hokein added a comment.

- address review comments.
- add error handling.


https://reviews.llvm.org/D39676

Files:
  clangd/CMakeLists.txt
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  test/clangd/initialize-params-invalid.test
  test/clangd/initialize-params.test
  test/clangd/rename.test

Index: test/clangd/rename.test
===
--- /dev/null
+++ test/clangd/rename.test
@@ -0,0 +1,50 @@
+# RUN: clangd -pretty -run-synchronously < %s | FileCheck -strict-whitespace %s
+# It is absolutely vital that this file has CRLF line endings.
+#
+Content-Length: 125
+
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+
+Content-Length: 150
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///foo.cpp","languageId":"cpp","version":1,"text":"int foo;"}}}
+
+Content-Length: 159
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/rename","params":{"textDocument":{"uri":"file:///foo.cpp"},"position":{"line":0,"character":5},"newName":"bar"}}
+#  CHECK:  "id": 1,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": {
+# CHECK-NEXT:"changes": {
+# CHECK-NEXT:  "file:///foo.cpp": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "newText": "bar",
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 7
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT:  "character": 4
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:}
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+# CHECK-NEXT:  ]
+# CHECK-NEXT:}
+# CHECK-NEXT:  }
+Content-Length: 159
+
+{"jsonrpc":"2.0","id":2,"method":"textDocument/rename","params":{"textDocument":{"uri":"file:///foo.cpp"},"position":{"line":0,"character":2},"newName":"bar"}}
+#  CHECK:  "error": {
+# CHECK-NEXT:"code": -32603,
+# CHECK-NEXT:"message": "clang diagnostic"
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "id": 2,
+# CHECK-NEXT:  "jsonrpc": "2.0"
+Content-Length: 44
+
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+Content-Length: 33
+
+{"jsonrpc":"2.0":"method":"exit"}
Index: test/clangd/initialize-params.test
===
--- test/clangd/initialize-params.test
+++ test/clangd/initialize-params.test
@@ -29,6 +29,7 @@
 # CHECK-NEXT:"clangd.applyFix"
 # CHECK-NEXT:  ]
 # CHECK-NEXT:},
+# CHECK-NEXT:   "renameProvider": true,
 # CHECK-NEXT:"signatureHelpProvider": {
 # CHECK-NEXT:  "triggerCharacters": [
 # CHECK-NEXT:"(",
Index: test/clangd/initialize-params-invalid.test
===
--- test/clangd/initialize-params-invalid.test
+++ test/clangd/initialize-params-invalid.test
@@ -29,6 +29,7 @@
 # CHECK-NEXT:"clangd.applyFix"
 # CHECK-NEXT:  ]
 # CHECK-NEXT:},
+# CHECK-NEXT:"renameProvider": true,
 # CHECK-NEXT:"signatureHelpProvider": {
 # CHECK-NEXT:  "triggerCharacters": [
 # CHECK-NEXT:"(",
Index: clangd/ProtocolHandlers.h
===
--- clangd/ProtocolHandlers.h
+++ clangd/ProtocolHandlers.h
@@ -53,6 +53,7 @@
   virtual void onSwitchSourceHeader(Ctx C, TextDocumentIdentifier &Params) = 0;
   virtual void onFileEvent(Ctx C, DidChangeWatchedFilesParams &Params) = 0;
   virtual void onCommand(Ctx C, ExecuteCommandParams &Params) = 0;
+  virtual void onRename(Ctx C, RenameParams &Parames) = 0;
 };
 
 void registerCallbackHandlers(JSONRPCDispatcher &Dispatcher, JSONOutput &Out,
Index: clangd/ProtocolHandlers.cpp
===
--- clangd/ProtocolHandlers.cpp
+++ clangd/ProtocolHandlers.cpp
@@ -71,6 +71,7 @@
   Register("textDocument/definition", &ProtocolCallbacks::onGoToDefinition);
   Register("textDocument/switchSourceHeader",
&ProtocolCallbacks::onSwitchSourceHeader);
+  Register("textDocument/rename", &ProtocolCallbacks::onRename);
   Register("workspace/didChangeWatchedFiles", &ProtocolCallbacks::onFileEvent);
   Register("workspace/executeCommand", &ProtocolCallbacks::onCommand);
 }
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -587,6 +587,20 @@
   static json::Expr unparse(const SignatureHelp &);
 };
 
+struct RenameParams {
+  /// The document that was opened.
+  TextDocumentIdentifier textDocument;
+
+  /// The position at which this request was sent.
+  Position p

[PATCH] D39676: [clangd] Add rename support.

2017-11-08 Thread Haojian Wu via Phabricator via cfe-commits

hokein added inline comments.



Comment at: clangd/ClangdServer.cpp:338
 
+std::vector
+ClangdServer::Rename(PathRef File, Position Pos, llvm::StringRef NewName) {

sammccall wrote:
> I think you can split out a private method:
> 
> Expected> 
> ClangdServer::applyRefactoring(RefactoringActionRuleBase&);
> 
> This would make this function easier to read by separating out the 
> rename-specific stuff from (some of) the boilerplate.
> (It also seems likely to be reusable if we add direct support for other 
> refactorings, but that's not the main reason I think)
I'd keep the code in `Rename` currently, as most of the code is 
rename-specific, we can refactor it out when the need arises.



Comment at: clangd/ClangdServer.cpp:347
+
+void handle(tooling::SymbolOccurrences) override {}
+

sammccall wrote:
> I don't think you need to override this, assuming you don't expect any of 
> these
We have to override this method, the default implementation (generating an 
"unsupported result" error) is not sensible for clangd.
Added a comment for it.



Comment at: clangd/ClangdUnit.cpp:1399
+
+SourceLocation getBeginningOfIdentifier(ParsedAST &Unit, const Position &Pos,
+const FileEntry *FE) {

ilya-biryukov wrote:
> sammccall wrote:
> > nit: the rest of this file defines functions outside their namespace.
> > 
> > TBH I prefer the style you're using here, but we should be consistent 
> > within a file.
> +1. Let's document and enforce the preferred style in new commits.
> But better done in a separate commit for all of clangd and let's stick to the 
> original style in this file for now.
Reserved to the original style. Yeah, +1 on change the style for this huge 
file. Currently, it is not clear for read.


https://reviews.llvm.org/D39676



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

[PATCH] D39375: [clang] Add PPCallbacks list to preprocessor when building a preacompiled preamble.

2017-11-08 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov requested changes to this revision.
ilya-biryukov added a comment.
This revision now requires changes to proceed.

Could you please take a different approach and add the callbacks you need into 
`PreambleCallbacks` class?
It already has this one:

  virtual void HandleMacroDefined(const Token &MacroNameTok,
 const MacroDirective *MD);

Otherwise we'll be creating too many ways to do the same thing.

We should also definitely get rid of all the unrelated formatting changes.


https://reviews.llvm.org/D39375



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

[clang-tools-extra] r317686 - [clangd] tolerate windows filepaths in tests

2017-11-08 Thread Sam McCall via cfe-commits

Author: sammccall
Date: Wed Nov  8 04:25:00 2017
New Revision: 317686

URL: http://llvm.org/viewvc/llvm-project?rev=317686&view=rev
Log:
[clangd] tolerate windows filepaths in tests

Modified:
clang-tools-extra/trunk/test/clangd/diagnostics-preamble.test

Modified: clang-tools-extra/trunk/test/clangd/diagnostics-preamble.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/diagnostics-preamble.test?rev=317686&r1=317685&r2=317686&view=diff
==
--- clang-tools-extra/trunk/test/clangd/diagnostics-preamble.test (original)
+++ clang-tools-extra/trunk/test/clangd/diagnostics-preamble.test Wed Nov  8 
04:25:00 2017
@@ -11,7 +11,7 @@ Content-Length: 206
 #  CHECK:  "method": "textDocument/publishDiagnostics",
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"diagnostics": [],
-# CHECK-NEXT:"uri": "file:///main.cpp"
+# CHECK-NEXT:"uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:  }
 Content-Length: 58
 


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

[PATCH] D39676: [clangd] Add rename support.

2017-11-08 Thread Sam McCall via Phabricator via cfe-commits

sammccall added inline comments.



Comment at: clangd/ClangdServer.cpp:338
 
+std::vector
+ClangdServer::Rename(PathRef File, Position Pos, llvm::StringRef NewName) {

hokein wrote:
> sammccall wrote:
> > I think you can split out a private method:
> > 
> > Expected> 
> > ClangdServer::applyRefactoring(RefactoringActionRuleBase&);
> > 
> > This would make this function easier to read by separating out the 
> > rename-specific stuff from (some of) the boilerplate.
> > (It also seems likely to be reusable if we add direct support for other 
> > refactorings, but that's not the main reason I think)
> I'd keep the code in `Rename` currently, as most of the code is 
> rename-specific, we can refactor it out when the need arises.
OK, then please find some other way to simplify/break up this method - it's too 
long and complicated.



Comment at: clangd/ClangdServer.cpp:347
+
+void handle(tooling::SymbolOccurrences) override {}
+

hokein wrote:
> sammccall wrote:
> > I don't think you need to override this, assuming you don't expect any of 
> > these
> We have to override this method, the default implementation (generating an 
> "unsupported result" error) is not sensible for clangd.
> Added a comment for it.
I don't understand.

My reading was that we expect handle(SymbolOccurrences) to never be called. If 
it was called, this is an internal problem and reporting an error seems 
reasonable.

Under what circumstances would it be called *and* the right response would be 
to ignore it?



Comment at: clangd/ClangdServer.cpp:359
+void handle(tooling::AtomicChanges SourceReplacements) override {
+  Result = std::move(SourceReplacements);
+}

the current refactoring engine doesn't call handle() multiple times, nor both 
handleError() and handle(), but I don't see that guaranteed anywhere.

At least we should assert that we're not overwriting anything.


https://reviews.llvm.org/D39676



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

[PATCH] D39793: [asan] Remove semicolon after do {} while (0)

2017-11-08 Thread Tom de Vries via Phabricator via cfe-commits

vries created this revision.
Herald added a subscriber: kubamracek.

this removes a semicolon after "do {} while (0)" in CHECK_SMALL_REGION. This 
allows the macro to be used in if-then-elses without curly braces.

[ https://gcc.gnu.org/ml/gcc-patches/2017-11/msg00298.html ]


https://reviews.llvm.org/D39793

Files:
  lib/asan/asan_poisoning.cc


Index: lib/asan/asan_poisoning.cc
===
--- lib/asan/asan_poisoning.cc
+++ lib/asan/asan_poisoning.cc
@@ -217,7 +217,7 @@
   uptr __bad = __asan_region_is_poisoned(__p, __size);\
   __asan_report_error(pc, bp, sp, __bad, isWrite, __size, 0);\
 } \
-  } while (false);\
+  } while (false)
 
 
 extern "C" SANITIZER_INTERFACE_ATTRIBUTE


Index: lib/asan/asan_poisoning.cc
===
--- lib/asan/asan_poisoning.cc
+++ lib/asan/asan_poisoning.cc
@@ -217,7 +217,7 @@
   uptr __bad = __asan_region_is_poisoned(__p, __size);\
   __asan_report_error(pc, bp, sp, __bad, isWrite, __size, 0);\
 } \
-  } while (false);\
+  } while (false)
 
 
 extern "C" SANITIZER_INTERFACE_ATTRIBUTE
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r317687 - Workaround reverse-iteration buildbot breakages. Filed PR35244.

2017-11-08 Thread Ilya Biryukov via cfe-commits

Author: ibiryukov
Date: Wed Nov  8 05:05:52 2017
New Revision: 317687

URL: http://llvm.org/viewvc/llvm-project?rev=317687&view=rev
Log:
Workaround reverse-iteration buildbot breakages. Filed PR35244.

Clang's completion output is non-deterministic, causing test failures
with turned on LLVM_REVERSE_ITERATION.
The workaround is to use CHECK-DAGs for now, will remove them when
PR35244 gets fixed.

Modified:
cfe/trunk/test/CodeCompletion/qualifiers-as-written.cpp

Modified: cfe/trunk/test/CodeCompletion/qualifiers-as-written.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeCompletion/qualifiers-as-written.cpp?rev=317687&r1=317686&r2=317687&view=diff
==
--- cfe/trunk/test/CodeCompletion/qualifiers-as-written.cpp (original)
+++ cfe/trunk/test/CodeCompletion/qualifiers-as-written.cpp Wed Nov  8 05:05:52 
2017
@@ -25,6 +25,7 @@ void test() {
   // CHECK-1: COMPLETION: method : [#type#]method(<#type#>, <#foo::type#>, 
<#::foo::type#>, <#::foo::foo::type#>)
   f
   // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:26:3 %s -o - | 
FileCheck %s --check-prefix=CHECK-2
-  // CHECK-2: COMPLETION: func : [#int#]func(<#foo a#>, <#bar b#>, <#ns::bar 
c#>, <#ns::baz d#>
-  // CHECK-2: COMPLETION: func : [#int#]func(<#foo::type a#>, <#bar b#>, <#baz 
c#>
+  // FIXME(ibiryukov): We should get rid of CHECK-DAGs here when completion 
output is made deterministic (see PR35244).
+  // CHECK-2-DAG: COMPLETION: func : [#int#]func(<#foo a#>, <#bar b#>, 
<#ns::bar c#>, <#ns::baz d#>
+  // CHECK-2-DAG: COMPLETION: func : [#int#]func(<#foo::type a#>, <#bar b#>, 
<#baz c#>
 }


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

[clang-tools-extra] r317689 - [clang-tidy] Add a note about modernize-replace-random-shuffle

2017-11-08 Thread Krasimir Georgiev via cfe-commits

Author: krasimir
Date: Wed Nov  8 05:28:53 2017
New Revision: 317689

URL: http://llvm.org/viewvc/llvm-project?rev=317689&view=rev
Log:
[clang-tidy] Add a note about modernize-replace-random-shuffle

Summary:
This adds a note warning the users that the way the suggested fix seeds the
random number generator is poor.

Reviewers: hokein

Reviewed By: hokein

Subscribers: cfe-commits, xazax.hun

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

Modified:

clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-replace-random-shuffle.rst

Modified: 
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-replace-random-shuffle.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-replace-random-shuffle.rst?rev=317689&r1=317688&r2=317689&view=diff
==
--- 
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-replace-random-shuffle.rst
 (original)
+++ 
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-replace-random-shuffle.rst
 Wed Nov  8 05:28:53 2017
@@ -26,3 +26,16 @@ Both of these examples will be replaced
 The second example will also receive a warning that ``randomFunc`` is no 
longer supported in the same way as before so if the user wants the same 
functionality, the user will need to change the implementation of the 
``randomFunc``.
 
 One thing to be aware of here is that ``std::random_device`` is quite 
expensive to initialize. So if you are using the code in a performance critical 
place, you probably want to initialize it elsewhere. 
+Another thing is that the seeding quality of the suggested fix is quite poor: 
``std::mt19937`` has an internal state of 624 32-bit integers, but is only 
seeded with a single integer. So if you require
+higher quality randomness, you should consider seeding better, for example:
+
+.. code-block:: c++
+
+  std::shuffle(v.begin(), v.end(), []() {
+std::mt19937::result_type seeds[std::mt19937::state_size];
+std::random_device device;
+std::uniform_int_distribution dist;
+std::generate(std::begin(seeds), std::end(seeds), [&] { return 
dist(device); });
+std::seed_seq seq(std::begin(seeds), std::end(seeds));
+return std::mt19937(seq);
+  }());


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

[PATCH] D39787: [clang-tidy] Add a note about modernize-replace-random-shuffle

2017-11-08 Thread Krasimir Georgiev via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL317689: [clang-tidy] Add a note about 
modernize-replace-random-shuffle (authored by krasimir).

Repository:
  rL LLVM

https://reviews.llvm.org/D39787

Files:
  
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-replace-random-shuffle.rst


Index: 
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-replace-random-shuffle.rst
===
--- 
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-replace-random-shuffle.rst
+++ 
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-replace-random-shuffle.rst
@@ -26,3 +26,16 @@
 The second example will also receive a warning that ``randomFunc`` is no 
longer supported in the same way as before so if the user wants the same 
functionality, the user will need to change the implementation of the 
``randomFunc``.
 
 One thing to be aware of here is that ``std::random_device`` is quite 
expensive to initialize. So if you are using the code in a performance critical 
place, you probably want to initialize it elsewhere. 
+Another thing is that the seeding quality of the suggested fix is quite poor: 
``std::mt19937`` has an internal state of 624 32-bit integers, but is only 
seeded with a single integer. So if you require
+higher quality randomness, you should consider seeding better, for example:
+
+.. code-block:: c++
+
+  std::shuffle(v.begin(), v.end(), []() {
+std::mt19937::result_type seeds[std::mt19937::state_size];
+std::random_device device;
+std::uniform_int_distribution dist;
+std::generate(std::begin(seeds), std::end(seeds), [&] { return 
dist(device); });
+std::seed_seq seq(std::begin(seeds), std::end(seeds));
+return std::mt19937(seq);
+  }());


Index: clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-replace-random-shuffle.rst
===
--- clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-replace-random-shuffle.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-replace-random-shuffle.rst
@@ -26,3 +26,16 @@
 The second example will also receive a warning that ``randomFunc`` is no longer supported in the same way as before so if the user wants the same functionality, the user will need to change the implementation of the ``randomFunc``.
 
 One thing to be aware of here is that ``std::random_device`` is quite expensive to initialize. So if you are using the code in a performance critical place, you probably want to initialize it elsewhere. 
+Another thing is that the seeding quality of the suggested fix is quite poor: ``std::mt19937`` has an internal state of 624 32-bit integers, but is only seeded with a single integer. So if you require
+higher quality randomness, you should consider seeding better, for example:
+
+.. code-block:: c++
+
+  std::shuffle(v.begin(), v.end(), []() {
+std::mt19937::result_type seeds[std::mt19937::state_size];
+std::random_device device;
+std::uniform_int_distribution dist;
+std::generate(std::begin(seeds), std::end(seeds), [&] { return dist(device); });
+std::seed_seq seq(std::begin(seeds), std::end(seeds));
+return std::mt19937(seq);
+  }());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39786: [clang-format] Sort using declarations by splitting on '::'

2017-11-08 Thread Krasimir Georgiev via Phabricator via cfe-commits

krasimir updated this revision to Diff 122069.
krasimir added a comment.

- Remove dead code


https://reviews.llvm.org/D39786

Files:
  docs/ClangFormatStyleOptions.rst
  docs/tools/dump_format_style.py
  include/clang/Format/Format.h
  lib/Format/UsingDeclarationsSorter.cpp
  unittests/Format/UsingDeclarationsSorterTest.cpp

Index: unittests/Format/UsingDeclarationsSorterTest.cpp
===
--- unittests/Format/UsingDeclarationsSorterTest.cpp
+++ unittests/Format/UsingDeclarationsSorterTest.cpp
@@ -50,8 +50,8 @@
 "using aa;",
 sortUsingDeclarations("using aa;\n"
   "using a;"));
-  EXPECT_EQ("using ::a;\n"
-"using a;",
+  EXPECT_EQ("using a;\n"
+"using ::a;",
 sortUsingDeclarations("using a;\n"
   "using ::a;"));
 
@@ -86,21 +86,32 @@
   "using a, b;"));
 }
 
-TEST_F(UsingDeclarationsSorterTest, SortsCaseSensitively) {
+TEST_F(UsingDeclarationsSorterTest, UsingDeclarationOrder) {
   EXPECT_EQ("using A;\n"
 "using a;",
 sortUsingDeclarations("using A;\n"
   "using a;"));
-  EXPECT_EQ("using A;\n"
-"using a;",
+  EXPECT_EQ("using a;\n"
+"using A;",
 sortUsingDeclarations("using a;\n"
   "using A;"));
-  EXPECT_EQ("using B;\n"
-"using a;",
+  EXPECT_EQ("using a;\n"
+"using B;",
 sortUsingDeclarations("using B;\n"
   "using a;"));
 
-  // Sorts '_' right before 'A'.
+  // Ignores leading '::'.
+  EXPECT_EQ("using ::a;\n"
+"using A;",
+sortUsingDeclarations("using ::a;\n"
+  "using A;"));
+
+  EXPECT_EQ("using ::A;\n"
+"using a;",
+sortUsingDeclarations("using ::A;\n"
+  "using a;"));
+
+  // Sorts '_' before 'a' and 'A'.
   EXPECT_EQ("using _;\n"
 "using A;",
 sortUsingDeclarations("using A;\n"
@@ -114,18 +125,58 @@
 sortUsingDeclarations("using a::a;\n"
   "using a::_;"));
 
+  // Sorts non-namespace names before namespace names at the same level.
   EXPECT_EQ("using ::testing::_;\n"
 "using ::testing::Aardvark;\n"
+"using ::testing::kMax;\n"
 "using ::testing::Xylophone;\n"
 "using ::testing::apple::Honeycrisp;\n"
 "using ::testing::zebra::Stripes;",
 sortUsingDeclarations("using ::testing::Aardvark;\n"
   "using ::testing::Xylophone;\n"
+  "using ::testing::kMax;\n"
   "using ::testing::_;\n"
   "using ::testing::apple::Honeycrisp;\n"
   "using ::testing::zebra::Stripes;"));
 }
 
+TEST_F(UsingDeclarationsSorterTest, SortsStably) {
+  EXPECT_EQ("using a;\n"
+"using a;\n"
+"using A;\n"
+"using a;\n"
+"using A;\n"
+"using a;\n"
+"using A;\n"
+"using a;\n"
+"using B;\n"
+"using b;\n"
+"using b;\n"
+"using B;\n"
+"using b;\n"
+"using b;\n"
+"using b;\n"
+"using B;\n"
+"using b;",
+sortUsingDeclarations("using a;\n"
+  "using B;\n"
+  "using a;\n"
+  "using b;\n"
+  "using A;\n"
+  "using a;\n"
+  "using b;\n"
+  "using B;\n"
+  "using b;\n"
+  "using A;\n"
+  "using a;\n"
+  "using b;\n"
+  "using b;\n"
+  "using B;\n"
+  "using b;\n"
+  "using A;\n"
+  "using a;"));
+}
+
 TEST_F(UsingDeclarationsSorterTest, SortsMultipleTopLevelDeclarations) {
   EXPECT_EQ("using a;\n"
 "using b;\n"
@@ -139,14 +190,14 @@
   "using c;"));
 
   EXPECT_EQ("#include \n"
-"using ::std::endl;\n"
 "using std::cin;\n"
 "using std::cout;\n"
+"using ::std::endl;\n"
 "int main();",
 sortUsingDeclarations("#include \n"
   "using std::cout;\n"
-  "using std::cin;\n"
   "using ::std::endl;\n"
+  "

[PATCH] D39796: [clang-refactor] Get rid of OccurrencesFinder in RenamingAction, NFC

2017-11-08 Thread Haojian Wu via Phabricator via cfe-commits

hokein created this revision.
Herald added a subscriber: klimek.

The OccurrencesFinder is only used in RenameOccurrences to find symbol
occurrences, there is no need to inherit RefactoringRule.

Replace it with a single utility function to avoid code misleading.


https://reviews.llvm.org/D39796

Files:
  lib/Tooling/Refactoring/Rename/RenamingAction.cpp


Index: lib/Tooling/Refactoring/Rename/RenamingAction.cpp
===
--- lib/Tooling/Refactoring/Rename/RenamingAction.cpp
+++ lib/Tooling/Refactoring/Rename/RenamingAction.cpp
@@ -43,22 +43,14 @@
 
 namespace {
 
-class OccurrenceFinder final : public FindSymbolOccurrencesRefactoringRule {
-public:
-  OccurrenceFinder(const NamedDecl *ND) : ND(ND) {}
-
-  Expected
-  findSymbolOccurrences(RefactoringRuleContext &Context) override {
-std::vector USRs =
-getUSRsForDeclaration(ND, Context.getASTContext());
-std::string PrevName = ND->getNameAsString();
-return getOccurrencesOfUSRs(
-USRs, PrevName, Context.getASTContext().getTranslationUnitDecl());
-  }
-
-private:
-  const NamedDecl *ND;
-};
+Expected
+findSymbolOccurrences(const NamedDecl *ND, RefactoringRuleContext &Context) {
+  std::vector USRs =
+  getUSRsForDeclaration(ND, Context.getASTContext());
+  std::string PrevName = ND->getNameAsString();
+  return getOccurrencesOfUSRs(USRs, PrevName,
+  
Context.getASTContext().getTranslationUnitDecl());
+}
 
 } // end anonymous namespace
 
@@ -85,8 +77,7 @@
 
 Expected
 RenameOccurrences::createSourceReplacements(RefactoringRuleContext &Context) {
-  Expected Occurrences =
-  OccurrenceFinder(ND).findSymbolOccurrences(Context);
+  Expected Occurrences = findSymbolOccurrences(ND, Context);
   if (!Occurrences)
 return Occurrences.takeError();
   // FIXME: Verify that the new name is valid.


Index: lib/Tooling/Refactoring/Rename/RenamingAction.cpp
===
--- lib/Tooling/Refactoring/Rename/RenamingAction.cpp
+++ lib/Tooling/Refactoring/Rename/RenamingAction.cpp
@@ -43,22 +43,14 @@
 
 namespace {
 
-class OccurrenceFinder final : public FindSymbolOccurrencesRefactoringRule {
-public:
-  OccurrenceFinder(const NamedDecl *ND) : ND(ND) {}
-
-  Expected
-  findSymbolOccurrences(RefactoringRuleContext &Context) override {
-std::vector USRs =
-getUSRsForDeclaration(ND, Context.getASTContext());
-std::string PrevName = ND->getNameAsString();
-return getOccurrencesOfUSRs(
-USRs, PrevName, Context.getASTContext().getTranslationUnitDecl());
-  }
-
-private:
-  const NamedDecl *ND;
-};
+Expected
+findSymbolOccurrences(const NamedDecl *ND, RefactoringRuleContext &Context) {
+  std::vector USRs =
+  getUSRsForDeclaration(ND, Context.getASTContext());
+  std::string PrevName = ND->getNameAsString();
+  return getOccurrencesOfUSRs(USRs, PrevName,
+  Context.getASTContext().getTranslationUnitDecl());
+}
 
 } // end anonymous namespace
 
@@ -85,8 +77,7 @@
 
 Expected
 RenameOccurrences::createSourceReplacements(RefactoringRuleContext &Context) {
-  Expected Occurrences =
-  OccurrenceFinder(ND).findSymbolOccurrences(Context);
+  Expected Occurrences = findSymbolOccurrences(ND, Context);
   if (!Occurrences)
 return Occurrences.takeError();
   // FIXME: Verify that the new name is valid.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39797: Relax definitions.test to accept windows file paths.

2017-11-08 Thread Sam McCall via Phabricator via cfe-commits

sammccall created this revision.

https://reviews.llvm.org/D39797

Files:
  test/clangd/definitions.test

Index: test/clangd/definitions.test
===
--- test/clangd/definitions.test
+++ test/clangd/definitions.test
@@ -27,7 +27,7 @@
 # CHECK-NEXT:  "line": 1
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 148
@@ -48,7 +48,7 @@
 # CHECK-NEXT:  "line": 1
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 214
@@ -73,7 +73,7 @@
 # CHECK-NEXT:  "line": 1
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 215
@@ -98,7 +98,7 @@
 # CHECK-NEXT:  "line": 1
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 187
@@ -123,7 +123,7 @@
 # CHECK-NEXT:  "line": 0
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 208
@@ -148,7 +148,7 @@
 # CHECK-NEXT:  "line": 0
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 231
@@ -173,7 +173,7 @@
 # CHECK-NEXT:  "line": 0
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 215
@@ -198,7 +198,7 @@
 # CHECK-NEXT:  "line": 1
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 220
@@ -223,7 +223,7 @@
 # CHECK-NEXT:  "line": 1
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 240
@@ -248,7 +248,7 @@
 # CHECK-NEXT:  "line": 2
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 254
@@ -284,7 +284,7 @@
 # CHECK-NEXT:  "line": 0
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 265
@@ -309,7 +309,7 @@
 # CHECK-NEXT:  "line": 2
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 204
@@ -334,7 +334,7 @@
 # CHECK-NEXT:  "line": 0
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 217
@@ -359,7 +359,7 @@
 # CHECK-NEXT:  "line": 0
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 148
@@ -380,7 +380,7 @@
 # CHECK-NEXT:  "line": 2
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 148
@@ -401,7 +401,7 @@
 # CHECK-NEXT:  "line": 2
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 156
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39676: [clangd] Add rename support.

2017-11-08 Thread Haojian Wu via Phabricator via cfe-commits

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

Simplify the code.


https://reviews.llvm.org/D39676

Files:
  clangd/CMakeLists.txt
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  test/clangd/initialize-params-invalid.test
  test/clangd/initialize-params.test
  test/clangd/rename.test

Index: test/clangd/rename.test
===
--- /dev/null
+++ test/clangd/rename.test
@@ -0,0 +1,50 @@
+# RUN: clangd -pretty -run-synchronously < %s | FileCheck -strict-whitespace %s
+# It is absolutely vital that this file has CRLF line endings.
+#
+Content-Length: 125
+
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+
+Content-Length: 150
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///foo.cpp","languageId":"cpp","version":1,"text":"int foo;"}}}
+
+Content-Length: 159
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/rename","params":{"textDocument":{"uri":"file:///foo.cpp"},"position":{"line":0,"character":5},"newName":"bar"}}
+#  CHECK:  "id": 1,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": {
+# CHECK-NEXT:"changes": {
+# CHECK-NEXT:  "file:///foo.cpp": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "newText": "bar",
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 7
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT:  "character": 4
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:}
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+# CHECK-NEXT:  ]
+# CHECK-NEXT:}
+# CHECK-NEXT:  }
+Content-Length: 159
+
+{"jsonrpc":"2.0","id":2,"method":"textDocument/rename","params":{"textDocument":{"uri":"file:///foo.cpp"},"position":{"line":0,"character":2},"newName":"bar"}}
+#  CHECK:  "error": {
+# CHECK-NEXT:"code": -32603,
+# CHECK-NEXT:"message": "clang diagnostic"
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "id": 2,
+# CHECK-NEXT:  "jsonrpc": "2.0"
+Content-Length: 44
+
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+Content-Length: 33
+
+{"jsonrpc":"2.0":"method":"exit"}
Index: test/clangd/initialize-params.test
===
--- test/clangd/initialize-params.test
+++ test/clangd/initialize-params.test
@@ -29,6 +29,7 @@
 # CHECK-NEXT:"clangd.applyFix"
 # CHECK-NEXT:  ]
 # CHECK-NEXT:},
+# CHECK-NEXT:   "renameProvider": true,
 # CHECK-NEXT:"signatureHelpProvider": {
 # CHECK-NEXT:  "triggerCharacters": [
 # CHECK-NEXT:"(",
Index: test/clangd/initialize-params-invalid.test
===
--- test/clangd/initialize-params-invalid.test
+++ test/clangd/initialize-params-invalid.test
@@ -29,6 +29,7 @@
 # CHECK-NEXT:"clangd.applyFix"
 # CHECK-NEXT:  ]
 # CHECK-NEXT:},
+# CHECK-NEXT:"renameProvider": true,
 # CHECK-NEXT:"signatureHelpProvider": {
 # CHECK-NEXT:  "triggerCharacters": [
 # CHECK-NEXT:"(",
Index: clangd/ProtocolHandlers.h
===
--- clangd/ProtocolHandlers.h
+++ clangd/ProtocolHandlers.h
@@ -53,6 +53,7 @@
   virtual void onSwitchSourceHeader(Ctx C, TextDocumentIdentifier &Params) = 0;
   virtual void onFileEvent(Ctx C, DidChangeWatchedFilesParams &Params) = 0;
   virtual void onCommand(Ctx C, ExecuteCommandParams &Params) = 0;
+  virtual void onRename(Ctx C, RenameParams &Parames) = 0;
 };
 
 void registerCallbackHandlers(JSONRPCDispatcher &Dispatcher, JSONOutput &Out,
Index: clangd/ProtocolHandlers.cpp
===
--- clangd/ProtocolHandlers.cpp
+++ clangd/ProtocolHandlers.cpp
@@ -71,6 +71,7 @@
   Register("textDocument/definition", &ProtocolCallbacks::onGoToDefinition);
   Register("textDocument/switchSourceHeader",
&ProtocolCallbacks::onSwitchSourceHeader);
+  Register("textDocument/rename", &ProtocolCallbacks::onRename);
   Register("workspace/didChangeWatchedFiles", &ProtocolCallbacks::onFileEvent);
   Register("workspace/executeCommand", &ProtocolCallbacks::onCommand);
 }
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -587,6 +587,20 @@
   static json::Expr unparse(const SignatureHelp &);
 };
 
+struct RenameParams {
+  /// The document that was opened.
+  TextDocumentIdentifier textDocument;
+
+  /// The position at which this request was sent.
+  Position position;
+
+  /// The new name

[PATCH] D39797: [clangd] Relax definitions.test to accept windows file paths.

2017-11-08 Thread 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.


https://reviews.llvm.org/D39797



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

[PATCH] D39676: [clangd] Add rename support.

2017-11-08 Thread Haojian Wu via Phabricator via cfe-commits

hokein added inline comments.



Comment at: clangd/ClangdServer.cpp:347
+
+void handle(tooling::SymbolOccurrences) override {}
+

sammccall wrote:
> hokein wrote:
> > sammccall wrote:
> > > I don't think you need to override this, assuming you don't expect any of 
> > > these
> > We have to override this method, the default implementation (generating an 
> > "unsupported result" error) is not sensible for clangd.
> > Added a comment for it.
> I don't understand.
> 
> My reading was that we expect handle(SymbolOccurrences) to never be called. 
> If it was called, this is an internal problem and reporting an error seems 
> reasonable.
> 
> Under what circumstances would it be called *and* the right response would be 
> to ignore it?
Oops,  you are right. I misread the code of RenameOccurrences: I thought the 
rename workflow is to find the occurrences,  pass to occurrences to the 
ResultConsumer, generate the atomic change and pass it to ResultConsumer. But 
the it turns out it just uses the occurrences to generate atomic change 
directly.



Comment at: clangd/ClangdServer.cpp:359
+void handle(tooling::AtomicChanges SourceReplacements) override {
+  Result = std::move(SourceReplacements);
+}

sammccall wrote:
> the current refactoring engine doesn't call handle() multiple times, nor both 
> handleError() and handle(), but I don't see that guaranteed anywhere.
> 
> At least we should assert that we're not overwriting anything.
I think the engine should do it (the current refactoring rule implementations 
imply it). If the refactoring succeeds, call "handle(AtomicChanges)" to pass 
the result; If any error happened during refactoring, call "handle(Error)".

But adding assert is safer.


https://reviews.llvm.org/D39676



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

[clang-tools-extra] r317692 - Relax definitions.test to accept windows file paths.

2017-11-08 Thread Sam McCall via cfe-commits

Author: sammccall
Date: Wed Nov  8 05:52:21 2017
New Revision: 317692

URL: http://llvm.org/viewvc/llvm-project?rev=317692&view=rev
Log:
Relax definitions.test to accept windows file paths.

Reviewers: ilya-biryukov

Subscribers: cfe-commits

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

Modified:
clang-tools-extra/trunk/test/clangd/definitions.test

Modified: clang-tools-extra/trunk/test/clangd/definitions.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/definitions.test?rev=317692&r1=317691&r2=317692&view=diff
==
--- clang-tools-extra/trunk/test/clangd/definitions.test (original)
+++ clang-tools-extra/trunk/test/clangd/definitions.test Wed Nov  8 05:52:21 
2017
@@ -27,7 +27,7 @@ Content-Length: 148
 # CHECK-NEXT:  "line": 1
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 148
@@ -48,7 +48,7 @@ Content-Length: 148
 # CHECK-NEXT:  "line": 1
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 214
@@ -73,7 +73,7 @@ Content-Length: 149
 # CHECK-NEXT:  "line": 1
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 215
@@ -98,7 +98,7 @@ Content-Length: 149
 # CHECK-NEXT:  "line": 1
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 187
@@ -123,7 +123,7 @@ Content-Length: 148
 # CHECK-NEXT:  "line": 0
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 208
@@ -148,7 +148,7 @@ Content-Length: 148
 # CHECK-NEXT:  "line": 0
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 231
@@ -173,7 +173,7 @@ Content-Length: 148
 # CHECK-NEXT:  "line": 0
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 215
@@ -198,7 +198,7 @@ Content-Length: 148
 # CHECK-NEXT:  "line": 1
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 220
@@ -223,7 +223,7 @@ Content-Length: 148
 # CHECK-NEXT:  "line": 1
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 240
@@ -248,7 +248,7 @@ Content-Length: 149
 # CHECK-NEXT:  "line": 2
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 254
@@ -284,7 +284,7 @@ Content-Length: 148
 # CHECK-NEXT:  "line": 0
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 265
@@ -309,7 +309,7 @@ Content-Length: 149
 # CHECK-NEXT:  "line": 2
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 204
@@ -334,7 +334,7 @@ Content-Length: 148
 # CHECK-NEXT:  "line": 0
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 217
@@ -359,7 +359,7 @@ Content-Length: 148
 # CHECK-NEXT:  "line": 0
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 148
@@ -380,7 +380,7 @@ Content-Length: 148
 # CHECK-NEXT:  "line": 2
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([

[PATCH] D39797: [clangd] Relax definitions.test to accept windows file paths.

2017-11-08 Thread Sam McCall via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL317692: Relax definitions.test to accept windows file paths. 
(authored by sammccall).

Repository:
  rL LLVM

https://reviews.llvm.org/D39797

Files:
  clang-tools-extra/trunk/test/clangd/definitions.test

Index: clang-tools-extra/trunk/test/clangd/definitions.test
===
--- clang-tools-extra/trunk/test/clangd/definitions.test
+++ clang-tools-extra/trunk/test/clangd/definitions.test
@@ -27,7 +27,7 @@
 # CHECK-NEXT:  "line": 1
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 148
@@ -48,7 +48,7 @@
 # CHECK-NEXT:  "line": 1
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 214
@@ -73,7 +73,7 @@
 # CHECK-NEXT:  "line": 1
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 215
@@ -98,7 +98,7 @@
 # CHECK-NEXT:  "line": 1
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 187
@@ -123,7 +123,7 @@
 # CHECK-NEXT:  "line": 0
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 208
@@ -148,7 +148,7 @@
 # CHECK-NEXT:  "line": 0
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 231
@@ -173,7 +173,7 @@
 # CHECK-NEXT:  "line": 0
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 215
@@ -198,7 +198,7 @@
 # CHECK-NEXT:  "line": 1
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 220
@@ -223,7 +223,7 @@
 # CHECK-NEXT:  "line": 1
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 240
@@ -248,7 +248,7 @@
 # CHECK-NEXT:  "line": 2
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 254
@@ -284,7 +284,7 @@
 # CHECK-NEXT:  "line": 0
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 265
@@ -309,7 +309,7 @@
 # CHECK-NEXT:  "line": 2
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 204
@@ -334,7 +334,7 @@
 # CHECK-NEXT:  "line": 0
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 217
@@ -359,7 +359,7 @@
 # CHECK-NEXT:  "line": 0
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 148
@@ -380,7 +380,7 @@
 # CHECK-NEXT:  "line": 2
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 148
@@ -401,7 +401,7 @@
 # CHECK-NEXT:  "line": 2
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///main.cpp"
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 Content-Length: 156
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38124: Hide some symbols to avoid a crash on shutdown when using code coverage

2017-11-08 Thread Sylvestre Ledru via Phabricator via cfe-commits

sylvestre.ledru updated this revision to Diff 122077.
sylvestre.ledru edited the summary of this revision.

https://reviews.llvm.org/D38124

Files:
  lib/profile/GCDAProfiling.c
  test/profile/Inputs/instrprof-dlopen-dlclose-main.c
  test/profile/instrprof-dlopen-dlclose-gcov.test

Index: test/profile/instrprof-dlopen-dlclose-gcov.test
===
--- /dev/null
+++ test/profile/instrprof-dlopen-dlclose-gcov.test
@@ -0,0 +1,6 @@
+RUN: mkdir -p %t.d
+RUN: %clang --coverage -o %t.d/func.shared -fPIC -shared %S/Inputs/instrprof-dlopen-func.c
+RUN: %clang --coverage -o %t.d/func2.shared -fPIC -shared %S/Inputs/instrprof-dlopen-func2.c
+RUN: %clang --coverage -o %t -fPIC -rpath %t.d %S/Inputs/instrprof-dlopen-dlclose-main.c
+
+RUN: %run %t
Index: test/profile/Inputs/instrprof-dlopen-dlclose-main.c
===
--- /dev/null
+++ test/profile/Inputs/instrprof-dlopen-dlclose-main.c
@@ -0,0 +1,26 @@
+#include 
+#include 
+#include 
+
+int main(int argc, char *argv[]) {
+  dlerror();
+  void *f1_handle = dlopen("func.shared", RTLD_LAZY | RTLD_GLOBAL);
+  if (f1_handle == NULL) {
+fprintf(stderr, "unable to open 'func.shared': %s\n", dlerror());
+return EXIT_FAILURE;
+  }
+
+  void *f2_handle = dlopen("func2.shared", RTLD_LAZY | RTLD_GLOBAL);
+  if (f2_handle == NULL) {
+fprintf(stderr, "unable to open 'func2.shared': %s\n", dlerror());
+return EXIT_FAILURE;
+  }
+
+  if (dlclose(f2_handle) != 0) {
+fprintf(stderr, "unable to close 'func2.shared': %s\n", dlerror());
+return EXIT_FAILURE;
+  }
+
+  return EXIT_SUCCESS;
+}
+
Index: lib/profile/GCDAProfiling.c
===
--- lib/profile/GCDAProfiling.c
+++ lib/profile/GCDAProfiling.c
@@ -231,6 +231,7 @@
  * profiling enabled will emit to a different file. Only one file may be
  * started at a time.
  */
+COMPILER_RT_VISIBILITY
 void llvm_gcda_start_file(const char *orig_filename, const char version[4],
   uint32_t checksum) {
   const char *mode = "r+b";
@@ -298,6 +299,7 @@
 /* Given an array of pointers to counters (counters), increment the n-th one,
  * where we're also given a pointer to n (predecessor).
  */
+COMPILER_RT_VISIBILITY
 void llvm_gcda_increment_indirect_counter(uint32_t *predecessor,
   uint64_t **counters) {
   uint64_t *counter;
@@ -320,6 +322,7 @@
 #endif
 }
 
+COMPILER_RT_VISIBILITY
 void llvm_gcda_emit_function(uint32_t ident, const char *function_name,
  uint32_t func_checksum, uint8_t use_extra_checksum,
  uint32_t cfg_checksum) {
@@ -346,6 +349,7 @@
 write_string(function_name);
 }
 
+COMPILER_RT_VISIBILITY
 void llvm_gcda_emit_arcs(uint32_t num_counters, uint64_t *counters) {
   uint32_t i;
   uint64_t *old_ctrs = NULL;
@@ -397,6 +401,7 @@
 #endif
 }
 
+COMPILER_RT_VISIBILITY
 void llvm_gcda_summary_info() {
   const uint32_t obj_summary_len = 9; /* Length for gcov compatibility. */
   uint32_t i;
@@ -450,6 +455,7 @@
 #endif
 }
 
+COMPILER_RT_VISIBILITY
 void llvm_gcda_end_file() {
   /* Write out EOF record. */
   if (output_file) {
@@ -474,6 +480,7 @@
 #endif
 }
 
+COMPILER_RT_VISIBILITY
 void llvm_register_writeout_function(writeout_fn fn) {
   struct writeout_fn_node *new_node = malloc(sizeof(struct writeout_fn_node));
   new_node->fn = fn;
@@ -487,6 +494,7 @@
   }
 }
 
+COMPILER_RT_VISIBILITY
 void llvm_writeout_files(void) {
   struct writeout_fn_node *curr = writeout_fn_head;
 
@@ -496,6 +504,7 @@
   }
 }
 
+COMPILER_RT_VISIBILITY
 void llvm_delete_writeout_function_list(void) {
   while (writeout_fn_head) {
 struct writeout_fn_node *node = writeout_fn_head;
@@ -506,6 +515,7 @@
   writeout_fn_head = writeout_fn_tail = NULL;
 }
 
+COMPILER_RT_VISIBILITY
 void llvm_register_flush_function(flush_fn fn) {
   struct flush_fn_node *new_node = malloc(sizeof(struct flush_fn_node));
   new_node->fn = fn;
@@ -519,6 +529,7 @@
   }
 }
 
+COMPILER_RT_VISIBILITY
 void __gcov_flush() {
   struct flush_fn_node *curr = flush_fn_head;
 
@@ -528,6 +539,7 @@
   }
 }
 
+COMPILER_RT_VISIBILITY
 void llvm_delete_flush_function_list(void) {
   while (flush_fn_head) {
 struct flush_fn_node *node = flush_fn_head;
@@ -538,6 +550,7 @@
   flush_fn_head = flush_fn_tail = NULL;
 }
 
+COMPILER_RT_VISIBILITY
 void llvm_gcov_init(writeout_fn wfn, flush_fn ffn) {
   static int atexit_ran = 0;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38124: Hide some symbols to avoid a crash on shutdown when using code coverage

2017-11-08 Thread Benoit Belley via Phabricator via cfe-commits

belleyb added a comment.

@sylvestre.ledru Thanks for the test!


https://reviews.llvm.org/D38124



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

[PATCH] D38124: Hide some symbols to avoid a crash on shutdown when using code coverage

2017-11-08 Thread Sylvestre Ledru via Phabricator via cfe-commits

sylvestre.ledru added a comment.

please thank Marco :)


https://reviews.llvm.org/D38124



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

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

2017-11-08 Thread Sam McCall via Phabricator via cfe-commits

sammccall created this revision.
Herald added a subscriber: ilya-biryukov.

This is an alternative to JSONCompilationDatabase for simple projects that
don't use a build system such as CMake.
(You can also drop one in ~, to make your tools use e.g. C++11 by default)

There's no facility for varying flags per-source-file or per-machine.
Possibly this could be accommodated backwards-compatibly using cpp, but even if
not the simplicity seems worthwhile for the cases that are addressed.

Tested with clangd, works great! (requires clangd restart)


https://reviews.llvm.org/D39799

Files:
  docs/JSONCompilationDatabase.rst
  include/clang/Tooling/CompilationDatabase.h
  lib/Tooling/CompilationDatabase.cpp
  test/Tooling/fixed-database.cpp

Index: test/Tooling/fixed-database.cpp
===
--- /dev/null
+++ test/Tooling/fixed-database.cpp
@@ -0,0 +1,8 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "-Dklazz=class" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-check "%t/test.cpp" 2>&1
+// RUN: not clang-check "%s" 2>&1 | FileCheck "%s" -check-prefix=NODB
+// NODB: unknown type name 'klazz'
+klazz F{};
Index: lib/Tooling/CompilationDatabase.cpp
===
--- lib/Tooling/CompilationDatabase.cpp
+++ lib/Tooling/CompilationDatabase.cpp
@@ -26,6 +26,7 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Support/Host.h"
+#include "llvm/Support/LineIterator.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
@@ -306,6 +307,22 @@
   new FixedCompilationDatabase(Directory, StrippedArgs));
 }
 
+std::unique_ptr
+FixedCompilationDatabase::loadFromFile(StringRef Path, std::string &ErrorMsg) {
+  ErrorMsg.clear();
+  llvm::ErrorOr> File =
+  llvm::MemoryBuffer::getFile(Path);
+  if (std::error_code Result = File.getError()) {
+ErrorMsg = "Error while opening fixed database: " + Result.message();
+return nullptr;
+  }
+  std::vector Args{
+  llvm::line_iterator(**File, /*SkipBlanks=*/true, /*CommentMarker=*/'#'),
+  llvm::line_iterator()};
+  return std::unique_ptr(new FixedCompilationDatabase(
+  llvm::sys::path::parent_path(Path), std::move(Args)));
+}
+
 FixedCompilationDatabase::
 FixedCompilationDatabase(Twine Directory, ArrayRef CommandLine) {
   std::vector ToolCommandLine(1, "clang-tool");
@@ -334,6 +351,24 @@
   return std::vector();
 }
 
+namespace {
+
+class FixedCompilationDatabasePlugin : public CompilationDatabasePlugin {
+  std::unique_ptr
+  loadFromDirectory(StringRef Directory, std::string &ErrorMessage) override {
+SmallString<1024> DatabasePath(Directory);
+llvm::sys::path::append(DatabasePath, "compile_flags.txt");
+return FixedCompilationDatabase::loadFromFile(DatabasePath, ErrorMessage);
+  }
+};
+
+// Register the JSONCompilationDatabasePlugin with the
+// CompilationDatabasePluginRegistry using this statically initialized variable.
+static CompilationDatabasePluginRegistry::Add
+X("fixed-compilation-database", "Reads plain-text flags file");
+
+} // namespace
+
 namespace clang {
 namespace tooling {
 
Index: include/clang/Tooling/CompilationDatabase.h
===
--- include/clang/Tooling/CompilationDatabase.h
+++ include/clang/Tooling/CompilationDatabase.h
@@ -182,6 +182,11 @@
   int &Argc, const char *const *Argv, std::string &ErrorMsg,
   Twine Directory = ".");
 
+  /// Reads flags from the given file, one-per line.
+  /// Returns nullptr and sets ErrorMessage if we can't read the file.
+  static std::unique_ptr
+  loadFromFile(StringRef Path, std::string &ErrorMsg);
+
   /// \brief Constructs a compilation data base from a specified directory
   /// and command line.
   FixedCompilationDatabase(Twine Directory, ArrayRef CommandLine);
Index: docs/JSONCompilationDatabase.rst
===
--- docs/JSONCompilationDatabase.rst
+++ docs/JSONCompilationDatabase.rst
@@ -91,3 +91,9 @@
 the top of the build directory. Clang tools are pointed to the top of
 the build directory to detect the file and use the compilation database
 to parse C++ code in the source tree.
+
+Alternatives
+
+For simple projects, Clang tools also recognize a compile_flags.txt file.
+This should contain one flag per line. The same flags will be used to compile
+any file.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

2017-11-08 Thread Manuel Klimek via Phabricator via cfe-commits

klimek added inline comments.



Comment at: include/clang/Tooling/CompilationDatabase.h:188
+  static std::unique_ptr
+  loadFromFile(StringRef Path, std::string &ErrorMsg);
+

Perhaps loadFromTextFile?



Comment at: lib/Tooling/CompilationDatabase.cpp:312
+FixedCompilationDatabase::loadFromFile(StringRef Path, std::string &ErrorMsg) {
+  ErrorMsg.clear();
+  llvm::ErrorOr> File =

I assume we can't use ErrorOr as return value?


https://reviews.llvm.org/D39799



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

[PATCH] D38737: [X86] test/testn intrinsics lowering to IR. clang side

2017-11-08 Thread Uriel Korach via Phabricator via cfe-commits

uriel.k updated this revision to Diff 122082.
uriel.k marked 2 inline comments as done.

https://reviews.llvm.org/D38737

Files:
  include/clang/Basic/BuiltinsX86.def
  lib/Headers/avx512bwintrin.h
  lib/Headers/avx512fintrin.h
  lib/Headers/avx512vlbwintrin.h
  lib/Headers/avx512vlintrin.h
  test/CodeGen/avx512bw-builtins.c
  test/CodeGen/avx512f-builtins.c
  test/CodeGen/avx512vl-builtins.c
  test/CodeGen/avx512vlbw-builtins.c

Index: test/CodeGen/avx512vlbw-builtins.c
===
--- test/CodeGen/avx512vlbw-builtins.c
+++ test/CodeGen/avx512vlbw-builtins.c
@@ -2481,97 +2481,121 @@
 }
 __mmask16 test_mm_test_epi8_mask(__m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_test_epi8_mask
-  // CHECK: @llvm.x86.avx512.ptestm.b.128
+  // CHECK: and <2 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp ne <16 x i8> %{{.*}}, %{{.*}}
   return _mm_test_epi8_mask(__A, __B); 
 }
 
 __mmask16 test_mm_mask_test_epi8_mask(__mmask16 __U, __m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_mask_test_epi8_mask
-  // CHECK: @llvm.x86.avx512.ptestm.b.128
+  // CHECK: and <2 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp ne <16 x i8> %{{.*}}, %{{.*}}
+  // CHECK: and <16 x i1> %{{.*}}, %{{.*}}
   return _mm_mask_test_epi8_mask(__U, __A, __B); 
 }
 
 __mmask32 test_mm256_test_epi8_mask(__m256i __A, __m256i __B) {
   // CHECK-LABEL: @test_mm256_test_epi8_mask
-  // CHECK: @llvm.x86.avx512.ptestm.b.256
+  // CHECK: and <4 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp ne <32 x i8> %{{.*}}, %{{.*}}
   return _mm256_test_epi8_mask(__A, __B); 
 }
 
 __mmask32 test_mm256_mask_test_epi8_mask(__mmask32 __U, __m256i __A, __m256i __B) {
   // CHECK-LABEL: @test_mm256_mask_test_epi8_mask
-  // CHECK: @llvm.x86.avx512.ptestm.b.256
+  // CHECK: and <4 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp ne <32 x i8> %{{.*}}, %{{.*}}
+  // CHECK: and <32 x i1> %{{.*}}, %{{.*}}
   return _mm256_mask_test_epi8_mask(__U, __A, __B); 
 }
 
 __mmask8 test_mm_test_epi16_mask(__m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_test_epi16_mask
-  // CHECK: @llvm.x86.avx512.ptestm.w.128
+  // CHECK: and <2 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp ne <8 x i16> %{{.*}}, %{{.*}}
   return _mm_test_epi16_mask(__A, __B); 
 }
 
 __mmask8 test_mm_mask_test_epi16_mask(__mmask8 __U, __m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_mask_test_epi16_mask
-  // CHECK: @llvm.x86.avx512.ptestm.w.128
+  // CHECK: and <2 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp ne <8 x i16> %{{.*}}, %{{.*}}
+  // CHECK: and <8 x i1> %{{.*}}, %{{.*}}
   return _mm_mask_test_epi16_mask(__U, __A, __B); 
 }
 
 __mmask16 test_mm256_test_epi16_mask(__m256i __A, __m256i __B) {
   // CHECK-LABEL: @test_mm256_test_epi16_mask
-  // CHECK: @llvm.x86.avx512.ptestm.w.256
+  // CHECK: and <4 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp ne <16 x i16> %{{.*}}, %{{.*}}
   return _mm256_test_epi16_mask(__A, __B); 
 }
 
 __mmask16 test_mm256_mask_test_epi16_mask(__mmask16 __U, __m256i __A, __m256i __B) {
   // CHECK-LABEL: @test_mm256_mask_test_epi16_mask
-  // CHECK: @llvm.x86.avx512.ptestm.w.256
+  // CHECK: and <4 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp ne <16 x i16> %{{.*}}, %{{.*}}
+  // CHECK: and <16 x i1> %{{.*}}, %{{.*}}
   return _mm256_mask_test_epi16_mask(__U, __A, __B); 
 }
 
 __mmask16 test_mm_testn_epi8_mask(__m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_testn_epi8_mask
-  // CHECK: @llvm.x86.avx512.ptestnm.b.128
+  // CHECK: and <2 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp eq <16 x i8> %{{.*}}, %{{.*}}
   return _mm_testn_epi8_mask(__A, __B); 
 }
 
 __mmask16 test_mm_mask_testn_epi8_mask(__mmask16 __U, __m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_mask_testn_epi8_mask
-  // CHECK: @llvm.x86.avx512.ptestnm.b.128
+  // CHECK: and <2 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp eq <16 x i8> %{{.*}}, %{{.*}}
+  // CHECK: and <16 x i1> %{{.*}}, %{{.*}}
   return _mm_mask_testn_epi8_mask(__U, __A, __B); 
 }
 
 __mmask32 test_mm256_testn_epi8_mask(__m256i __A, __m256i __B) {
   // CHECK-LABEL: @test_mm256_testn_epi8_mask
-  // CHECK: @llvm.x86.avx512.ptestnm.b.256
+  // CHECK: and <4 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp eq <32 x i8> %{{.*}}, %{{.*}}
   return _mm256_testn_epi8_mask(__A, __B); 
 }
 
 __mmask32 test_mm256_mask_testn_epi8_mask(__mmask32 __U, __m256i __A, __m256i __B) {
   // CHECK-LABEL: @test_mm256_mask_testn_epi8_mask
-  // CHECK: @llvm.x86.avx512.ptestnm.b.256
+  // CHECK: and <4 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp eq <32 x i8> %{{.*}}, %{{.*}}
+  // CHECK: and <32 x i1> %{{.*}}, %{{.*}}
   return _mm256_mask_testn_epi8_mask(__U, __A, __B); 
 }
 
 __mmask8 test_mm_testn_epi16_mask(__m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_testn_epi16_mask
-  // CHECK: @llvm.x86.avx512.ptestnm.w.128
+  // CHECK: and <2 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp eq <8 x i16> %{{.*}}, %{{.*}}
   return _mm_testn_epi16_mask(__A, __B); 
 }
 
 __mmask8 test_mm_mask_testn_epi16_mask(__mmask8 __U, __m128i __A, __m1

[PATCH] D38737: [X86] test/testn intrinsics lowering to IR. clang side

2017-11-08 Thread Uriel Korach via Phabricator via cfe-commits

uriel.k added inline comments.



Comment at: lib/Headers/avx512vlbwintrin.h:2425
+  return _mm_mask_cmpneq_epi8_mask (__U, _mm_and_si128 (__A, __B),
+ _mm_setzero_hi());
 }

For strange reason this line moved and is not aligned with the open paren above 
it. If there are no other corrections I'll fix it before the commit.


https://reviews.llvm.org/D38737



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

[PATCH] D38737: [X86] test/testn intrinsics lowering to IR. clang side

2017-11-08 Thread Uriel Korach via Phabricator via cfe-commits

uriel.k added inline comments.



Comment at: lib/Headers/avx512vlbwintrin.h:2425
+  return _mm_mask_cmpneq_epi8_mask (__U, _mm_and_si128 (__A, __B),
+ _mm_setzero_hi());
 }

uriel.k wrote:
> For strange reason this line moved and is not aligned with the open paren 
> above it. If there are no other corrections I'll fix it before the commit.
I meant of course for all the lines that moved


https://reviews.llvm.org/D38737



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

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

2017-11-08 Thread Sam McCall via Phabricator via cfe-commits

sammccall added inline comments.



Comment at: include/clang/Tooling/CompilationDatabase.h:188
+  static std::unique_ptr
+  loadFromFile(StringRef Path, std::string &ErrorMsg);
+

klimek wrote:
> Perhaps loadFromTextFile?
This mirrors JSONCompilationDatabase for consistency.
Happy to change this one, change both, or leave as-is. WDYT?



Comment at: lib/Tooling/CompilationDatabase.cpp:312
+FixedCompilationDatabase::loadFromFile(StringRef Path, std::string &ErrorMsg) {
+  ErrorMsg.clear();
+  llvm::ErrorOr> File =

klimek wrote:
> I assume we can't use ErrorOr as return value?
Again this mirrors JSONCompilationDatabase. (And also 
CompilationDatabasePlugin, which is harder to change)
Change this one, change both, or leave as-is?


https://reviews.llvm.org/D39799



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

[PATCH] D39676: [clangd] Add rename support.

2017-11-08 Thread 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/ClangdServer.cpp:67
+
+  // Using the handle(SymbolOccurrences) from parent class.
+  using tooling::RefactoringResultConsumer::handle;

why is this needed? shouldn't inheriting the base method be enough if it's only 
ever called through the base?


https://reviews.llvm.org/D39676



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

[PATCH] D39796: [clang-refactor] Get rid of OccurrencesFinder in RenamingAction, NFC

2017-11-08 Thread Sam McCall via Phabricator via cfe-commits

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

(This seems trivial enough to submit without review FWIW)


https://reviews.llvm.org/D39796



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

[PATCH] D39676: [clangd] Add rename support.

2017-11-08 Thread Haojian Wu via Phabricator via cfe-commits

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



Comment at: clangd/ClangdServer.cpp:67
+
+  // Using the handle(SymbolOccurrences) from parent class.
+  using tooling::RefactoringResultConsumer::handle;

sammccall wrote:
> why is this needed? shouldn't inheriting the base method be enough if it's 
> only ever called through the base?
We need this to make compiler happy, otherwise the compiler will emit 
"overloaded-virtual" error (the `handle(SymbolOccurrences)` will be hidden in 
this class).


https://reviews.llvm.org/D39676



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

r317696 - [clang-refactor] Get rid of OccurrencesFinder in RenamingAction, NFC

2017-11-08 Thread Haojian Wu via cfe-commits

Author: hokein
Date: Wed Nov  8 06:53:08 2017
New Revision: 317696

URL: http://llvm.org/viewvc/llvm-project?rev=317696&view=rev
Log:
[clang-refactor] Get rid of OccurrencesFinder in RenamingAction, NFC

Summary:
The OccurrencesFinder is only used in RenameOccurrences to find symbol
occurrences, there is no need to inherit RefactoringRule.

Replace it with a single utility function to avoid code misleading.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: klimek, cfe-commits

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

Modified:
cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp

Modified: cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp?rev=317696&r1=317695&r2=317696&view=diff
==
--- cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp (original)
+++ cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp Wed Nov  8 
06:53:08 2017
@@ -43,22 +43,14 @@ namespace tooling {
 
 namespace {
 
-class OccurrenceFinder final : public FindSymbolOccurrencesRefactoringRule {
-public:
-  OccurrenceFinder(const NamedDecl *ND) : ND(ND) {}
-
-  Expected
-  findSymbolOccurrences(RefactoringRuleContext &Context) override {
-std::vector USRs =
-getUSRsForDeclaration(ND, Context.getASTContext());
-std::string PrevName = ND->getNameAsString();
-return getOccurrencesOfUSRs(
-USRs, PrevName, Context.getASTContext().getTranslationUnitDecl());
-  }
-
-private:
-  const NamedDecl *ND;
-};
+Expected
+findSymbolOccurrences(const NamedDecl *ND, RefactoringRuleContext &Context) {
+  std::vector USRs =
+  getUSRsForDeclaration(ND, Context.getASTContext());
+  std::string PrevName = ND->getNameAsString();
+  return getOccurrencesOfUSRs(USRs, PrevName,
+  
Context.getASTContext().getTranslationUnitDecl());
+}
 
 } // end anonymous namespace
 
@@ -85,8 +77,7 @@ RenameOccurrences::initiate(RefactoringR
 
 Expected
 RenameOccurrences::createSourceReplacements(RefactoringRuleContext &Context) {
-  Expected Occurrences =
-  OccurrenceFinder(ND).findSymbolOccurrences(Context);
+  Expected Occurrences = findSymbolOccurrences(ND, Context);
   if (!Occurrences)
 return Occurrences.takeError();
   // FIXME: Verify that the new name is valid.


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

[PATCH] D39800: [analyzer] pr34404: Fix a crash on pointers to members in nested anonymous structures.

2017-11-08 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ created this revision.

pr34404 points out that given

  class C {
struct {
  int x;
};
  };

, taking pointer-to-member `&C::x` would crash the analyzer. We were not ready 
to stumble upon an `IndirectFieldDecl`, which is used for representing a field 
within an anonymous structure (or union) nested into a class.

Even if it wasn't anonymous, our new pointer-to-member support is not yet 
enabled for this case, so the value of the expression would be a conjured 
symbol of type `void *`. Being quite vague, it fits equally poorly to the 
anonymous case (in general this behavior makes very little sense, since 
//pointer-to-member must be a `NonLoc`//), so for the purposes of fixing the 
crash i guess i'd just keep it.

Actually constructing a `nonloc::PointerToMember` in the regular field case 
should be trivial. However, in the anonymous field case it requires more work, 
because `IndirectFieldDecl` is not a `DeclaratorDecl`. And simply calling 
`getAnonField()` over `IndirectFieldDecl` to obtain `FieldDecl` is incorrect, 
because it'd discard the offset to the anonymous structure within the class, 
leading to incorrect results on the attached tests for `m` and `n`.


https://reviews.llvm.org/D39800

Files:
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/pointer-to-member.cpp


Index: test/Analysis/pointer-to-member.cpp
===
--- test/Analysis/pointer-to-member.cpp
+++ test/Analysis/pointer-to-member.cpp
@@ -230,3 +230,42 @@
   clang_analyzer_eval(d2.*(static_cast(static_cast(static_cast(&B::f == 4); // expected-warning {{TRUE}}
 }
 } // end of testPointerToMemberDiamond namespace
+
+namespace testAnonymousMember {
+struct A {
+  struct {
+int x;
+  };
+  struct {
+struct {
+  int y;
+};
+  };
+  struct {
+union {
+  int z;
+};
+  };
+};
+
+void test() {
+  clang_analyzer_eval(&A::x); // expected-warning{{TRUE}}
+  clang_analyzer_eval(&A::y); // expected-warning{{TRUE}}
+  clang_analyzer_eval(&A::z); // expected-warning{{TRUE}}
+
+  // FIXME: These should be true.
+  int A::*l = &A::x, A::*m = &A::y, A::*n = &A::z;
+  clang_analyzer_eval(l); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(m); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(n); // expected-warning{{UNKNOWN}}
+
+  // FIXME: These should be true as well.
+  A a;
+  a.x = 1;
+  clang_analyzer_eval(a.*l == 1); // expected-warning{{UNKNOWN}}
+  a.y = 2;
+  clang_analyzer_eval(a.*m == 2); // expected-warning{{UNKNOWN}}
+  a.z = 3;
+  clang_analyzer_eval(a.*n == 3); // expected-warning{{UNKNOWN}}
+}
+} // end of testAnonymousMember namespace
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2108,7 +2108,7 @@
   ProgramPoint::PostLValueKind);
 return;
   }
-  if (isa(D)) {
+  if (isa(D) || isa(D)) {
 // FIXME: Compute lvalue of field pointers-to-member.
 // Right now we just use a non-null void pointer, so that it gives proper
 // results in boolean contexts.


Index: test/Analysis/pointer-to-member.cpp
===
--- test/Analysis/pointer-to-member.cpp
+++ test/Analysis/pointer-to-member.cpp
@@ -230,3 +230,42 @@
   clang_analyzer_eval(d2.*(static_cast(static_cast(static_cast(&B::f == 4); // expected-warning {{TRUE}}
 }
 } // end of testPointerToMemberDiamond namespace
+
+namespace testAnonymousMember {
+struct A {
+  struct {
+int x;
+  };
+  struct {
+struct {
+  int y;
+};
+  };
+  struct {
+union {
+  int z;
+};
+  };
+};
+
+void test() {
+  clang_analyzer_eval(&A::x); // expected-warning{{TRUE}}
+  clang_analyzer_eval(&A::y); // expected-warning{{TRUE}}
+  clang_analyzer_eval(&A::z); // expected-warning{{TRUE}}
+
+  // FIXME: These should be true.
+  int A::*l = &A::x, A::*m = &A::y, A::*n = &A::z;
+  clang_analyzer_eval(l); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(m); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(n); // expected-warning{{UNKNOWN}}
+
+  // FIXME: These should be true as well.
+  A a;
+  a.x = 1;
+  clang_analyzer_eval(a.*l == 1); // expected-warning{{UNKNOWN}}
+  a.y = 2;
+  clang_analyzer_eval(a.*m == 2); // expected-warning{{UNKNOWN}}
+  a.z = 3;
+  clang_analyzer_eval(a.*n == 3); // expected-warning{{UNKNOWN}}
+}
+} // end of testAnonymousMember namespace
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2108,7 +2108,7 @@
   ProgramPoint::PostLValueKind);
 return;
   }
-  if (isa(D)) {
+  if (isa(D) || isa(D)) {
 // FIXME: Compute lvalue of field pointers-to-member.
 // Right now we just use a non-null void pointer, so t

[PATCH] D39796: [clang-refactor] Get rid of OccurrencesFinder in RenamingAction, NFC

2017-11-08 Thread Haojian Wu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL317696: [clang-refactor] Get rid of OccurrencesFinder in 
RenamingAction, NFC (authored by hokein).

Repository:
  rL LLVM

https://reviews.llvm.org/D39796

Files:
  cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp


Index: cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
===
--- cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
+++ cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
@@ -43,22 +43,14 @@
 
 namespace {
 
-class OccurrenceFinder final : public FindSymbolOccurrencesRefactoringRule {
-public:
-  OccurrenceFinder(const NamedDecl *ND) : ND(ND) {}
-
-  Expected
-  findSymbolOccurrences(RefactoringRuleContext &Context) override {
-std::vector USRs =
-getUSRsForDeclaration(ND, Context.getASTContext());
-std::string PrevName = ND->getNameAsString();
-return getOccurrencesOfUSRs(
-USRs, PrevName, Context.getASTContext().getTranslationUnitDecl());
-  }
-
-private:
-  const NamedDecl *ND;
-};
+Expected
+findSymbolOccurrences(const NamedDecl *ND, RefactoringRuleContext &Context) {
+  std::vector USRs =
+  getUSRsForDeclaration(ND, Context.getASTContext());
+  std::string PrevName = ND->getNameAsString();
+  return getOccurrencesOfUSRs(USRs, PrevName,
+  
Context.getASTContext().getTranslationUnitDecl());
+}
 
 } // end anonymous namespace
 
@@ -85,8 +77,7 @@
 
 Expected
 RenameOccurrences::createSourceReplacements(RefactoringRuleContext &Context) {
-  Expected Occurrences =
-  OccurrenceFinder(ND).findSymbolOccurrences(Context);
+  Expected Occurrences = findSymbolOccurrences(ND, Context);
   if (!Occurrences)
 return Occurrences.takeError();
   // FIXME: Verify that the new name is valid.


Index: cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
===
--- cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
+++ cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
@@ -43,22 +43,14 @@
 
 namespace {
 
-class OccurrenceFinder final : public FindSymbolOccurrencesRefactoringRule {
-public:
-  OccurrenceFinder(const NamedDecl *ND) : ND(ND) {}
-
-  Expected
-  findSymbolOccurrences(RefactoringRuleContext &Context) override {
-std::vector USRs =
-getUSRsForDeclaration(ND, Context.getASTContext());
-std::string PrevName = ND->getNameAsString();
-return getOccurrencesOfUSRs(
-USRs, PrevName, Context.getASTContext().getTranslationUnitDecl());
-  }
-
-private:
-  const NamedDecl *ND;
-};
+Expected
+findSymbolOccurrences(const NamedDecl *ND, RefactoringRuleContext &Context) {
+  std::vector USRs =
+  getUSRsForDeclaration(ND, Context.getASTContext());
+  std::string PrevName = ND->getNameAsString();
+  return getOccurrencesOfUSRs(USRs, PrevName,
+  Context.getASTContext().getTranslationUnitDecl());
+}
 
 } // end anonymous namespace
 
@@ -85,8 +77,7 @@
 
 Expected
 RenameOccurrences::createSourceReplacements(RefactoringRuleContext &Context) {
-  Expected Occurrences =
-  OccurrenceFinder(ND).findSymbolOccurrences(Context);
+  Expected Occurrences = findSymbolOccurrences(ND, Context);
   if (!Occurrences)
 return Occurrences.takeError();
   // FIXME: Verify that the new name is valid.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39776: [libcxx] Mark test cxa_deleted_virtual.pass.cpp as failing for previous libcxx versions.

2017-11-08 Thread Akira Hatanaka via Phabricator via cfe-commits

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

LGTM


https://reviews.llvm.org/D39776



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

[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete

2017-11-08 Thread Cameron via Phabricator via cfe-commits

cameron314 added inline comments.



Comment at: cfe/trunk/lib/Sema/SemaOverload.cpp:6365
cast(FD)->getParent(), ObjectType,
-   ObjectClassification, Args.slice(1), CandidateSet,
+   ObjectClassification, FunctionArgs, CandidateSet,
SuppressUserConversions, PartialOverloading);

This breaks normal (non-static) method overload resolution, since the `this` 
argument is now passed to `AddMethodCandidate` which is not expecting it. It 
always thinks too many arguments are passed to methods with no parameters; most 
other calls to `AddMethodCandidate` in SemaOverload.cpp don't pass the implicit 
`this`.


Repository:
  rL LLVM

https://reviews.llvm.org/D36390



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

[PATCH] D39706: [refactor][extract] Initial implementation of variable captures

2017-11-08 Thread Eric Liu via Phabricator via cfe-commits

ioeric added a comment.

Implementation looks good. Just some nits.




Comment at: lib/Tooling/Refactoring/Extract/CaptureVariables.cpp:36
+  return true;
+// FIXME: Capture 'self'.
+if (!VD->isLocalVarDeclOrParm())

and `this`?



Comment at: lib/Tooling/Refactoring/Extract/CaptureVariables.cpp:50
+
+  // FIXME: Detemine 'const' qualifier.
+

nit: s/Detemine/Determine/

It might make more sense to put these `FIXME` in class-level comment.



Comment at: lib/Tooling/Refactoring/Extract/CaptureVariables.h:36
+  explicit CapturedExtractedEntity(const VarDecl *VD)
+  : Kind(CapturedVarDecl), VD(VD) {}
+

Maybe a `FIXME` here for `Kind`?



Comment at: lib/Tooling/Refactoring/Extract/CaptureVariables.h:38
+
+  /// Print the parameter declaration for the captured entity.
+  void printParamDecl(llvm::raw_ostream &OS, const PrintingPolicy &PP) const;

Does this include trailing commas? Maybe add an example in the comment? 

Same below. 



Comment at: lib/Tooling/Refactoring/Extract/CaptureVariables.h:43
+  /// invoking the extracted function.
+  void printUseExpr(llvm::raw_ostream &OS, const PrintingPolicy &PP) const;
+

Maybe `printFunctionCallArg`? It's unclear where this is used.



Comment at: lib/Tooling/Refactoring/Extract/Extract.cpp:155
+for (const auto &Capture : llvm::enumerate(Captures)) {
+  if (Capture.index())
+OS << ", ";

nit: `Capture.index() > 0` to be more explicit.


Repository:
  rL LLVM

https://reviews.llvm.org/D39706



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

[PATCH] D39803: [analyzer] pr34766: Fix a crash on explicit construction of std::initializer_list.

2017-11-08 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ created this revision.

`std::initializer_list` objects can be constructed sort of explicitly, eg. 
`(std::initializer_list){12}`. This produces an AST that looks like

  CompoundLiteralExpr 0x11987f1a0 'std::initializer_list':'class 
std::initializer_list'
  `-CXXStdInitializerListExpr 0x11987f188 'std::initializer_list':'class 
std::initializer_list'
`-MaterializeTemporaryExpr 0x11987f170 'const int [1]' xvalue
  `-InitListExpr 0x11987f128 'const int [1]'
`-IntegerLiteral 0x11987cd18 'int' 12

We crash because we did not expect to see `CompoundLiteralExpr` containing 
`CXXStdInitializerListExpr`.

It seems correct to pass the value through `CompoundLiteralExpr` transparently 
(the value is currently a conjured structure-symbol of `initializer_list` type, 
which sounds like a correct value for the expression, even if not super 
verbose), hence the patch.


https://reviews.llvm.org/D39803

Files:
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  test/Analysis/initializer.cpp


Index: test/Analysis/initializer.cpp
===
--- test/Analysis/initializer.cpp
+++ test/Analysis/initializer.cpp
@@ -211,12 +211,16 @@
 struct C {
   C(std::initializer_list list);
 };
-void foo() {
+void testPointerEscapeIntoLists() {
   C empty{}; // no-crash
 
   // Do not warn that 'x' leaks. It might have been deleted by
   // the destructor of 'c'.
   int *x = new int;
   C c{x}; // no-warning
 }
+
+void testPassListsWithExplicitConstructors() {
+  (void)(std::initializer_list){12}; // no-crash
+}
 }
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -530,7 +530,7 @@
   const Expr *Init = CL->getInitializer();
   SVal V = State->getSVal(CL->getInitializer(), LCtx);
 
-  if (isa(Init)) {
+  if (isa(Init) || isa(Init)) {
 // No work needed. Just pass the value up to this expression.
   } else {
 assert(isa(Init));


Index: test/Analysis/initializer.cpp
===
--- test/Analysis/initializer.cpp
+++ test/Analysis/initializer.cpp
@@ -211,12 +211,16 @@
 struct C {
   C(std::initializer_list list);
 };
-void foo() {
+void testPointerEscapeIntoLists() {
   C empty{}; // no-crash
 
   // Do not warn that 'x' leaks. It might have been deleted by
   // the destructor of 'c'.
   int *x = new int;
   C c{x}; // no-warning
 }
+
+void testPassListsWithExplicitConstructors() {
+  (void)(std::initializer_list){12}; // no-crash
+}
 }
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -530,7 +530,7 @@
   const Expr *Init = CL->getInitializer();
   SVal V = State->getSVal(CL->getInitializer(), LCtx);
 
-  if (isa(Init)) {
+  if (isa(Init) || isa(Init)) {
 // No work needed. Just pass the value up to this expression.
   } else {
 assert(isa(Init));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

2017-11-08 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

I can see that `FixedCompilationDatabase` does not set a working directory. Is 
this something we may want to have for `compile_flags.txt` or one would need to 
resort to `compile_commands.json` to get this?
E.g., I'd find it useful to add includes with paths relative to 
`compile_flags.txt` by putting `-Iproject-dep/include` there.




Comment at: lib/Tooling/CompilationDatabase.cpp:322
+  llvm::line_iterator()};
+  return std::unique_ptr(new 
FixedCompilationDatabase(
+  llvm::sys::path::parent_path(Path), std::move(Args)));

Maybe use `llvm:make_unique`?


https://reviews.llvm.org/D39799



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

[PATCH] D39804: [clang] [python] [tests] Update TLSKind tests for newer MSVC versions

2017-11-08 Thread Michał Górny via Phabricator via cfe-commits

mgorny created this revision.

The bump of default _MSC_VER which occurred in r315107 has caused
the '__declspec(thread)' TLSKind to change from STATIC to DYNAMIC.
Update the clang python binding tests accordingly. Test both for
behavior with old and new -fms-compatibility-version.


Repository:
  rL LLVM

https://reviews.llvm.org/D39804

Files:
  bindings/python/tests/cindex/test_tls_kind.py


Index: bindings/python/tests/cindex/test_tls_kind.py
===
--- bindings/python/tests/cindex/test_tls_kind.py
+++ bindings/python/tests/cindex/test_tls_kind.py
@@ -27,11 +27,19 @@
 # The following case tests '__declspec(thread)'.  Since it is a Microsoft
 # specific extension, specific flags are required for the parser to pick
 # these up.
-flags = ['-fms-extensions', '-target', 'x86_64-unknown-windows-win32']
+flags = ['-fms-compatibility-version=18.00', '-fms-extensions',
+ '-target', 'x86_64-unknown-windows-win32']
 tu = get_tu("""
 __declspec(thread) int tls_declspec;
 """, lang = 'cpp', flags=flags)
 
 tls_declspec = get_cursor(tu.cursor, 'tls_declspec')
 assert tls_declspec.tls_kind == TLSKind.STATIC
 
+flags = ['-fms-extensions', '-target', 'x86_64-unknown-windows-win32']
+tu = get_tu("""
+__declspec(thread) int tls_declspec;
+""", lang = 'cpp', flags=flags)
+
+tls_declspec = get_cursor(tu.cursor, 'tls_declspec')
+assert tls_declspec.tls_kind == TLSKind.DYNAMIC


Index: bindings/python/tests/cindex/test_tls_kind.py
===
--- bindings/python/tests/cindex/test_tls_kind.py
+++ bindings/python/tests/cindex/test_tls_kind.py
@@ -27,11 +27,19 @@
 # The following case tests '__declspec(thread)'.  Since it is a Microsoft
 # specific extension, specific flags are required for the parser to pick
 # these up.
-flags = ['-fms-extensions', '-target', 'x86_64-unknown-windows-win32']
+flags = ['-fms-compatibility-version=18.00', '-fms-extensions',
+ '-target', 'x86_64-unknown-windows-win32']
 tu = get_tu("""
 __declspec(thread) int tls_declspec;
 """, lang = 'cpp', flags=flags)
 
 tls_declspec = get_cursor(tu.cursor, 'tls_declspec')
 assert tls_declspec.tls_kind == TLSKind.STATIC
 
+flags = ['-fms-extensions', '-target', 'x86_64-unknown-windows-win32']
+tu = get_tu("""
+__declspec(thread) int tls_declspec;
+""", lang = 'cpp', flags=flags)
+
+tls_declspec = get_cursor(tu.cursor, 'tls_declspec')
+assert tls_declspec.tls_kind == TLSKind.DYNAMIC
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

2017-11-08 Thread Sam McCall via Phabricator via cfe-commits

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

In https://reviews.llvm.org/D39799#919388, @ilya-biryukov wrote:

> I can see that `FixedCompilationDatabase` does not set a working directory. 
> Is this something we may want to have for `compile_flags.txt` or one would 
> need to resort to `compile_commands.json` to get this?
>  E.g., I'd find it useful to add includes with paths relative to 
> `compile_flags.txt` by putting `-Iproject-dep/include` there.

Currently the working directory is always the parent of `compile_flags.txt` as 
you suggest.
Maybe this isn't great though - sometimes it might be important that the WD is 
the parent of the source file?

https://reviews.llvm.org/D39799

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

[PATCH] D39050: Add index-while-building support to Clang

2017-11-08 Thread Marc-Andre Laperle via Phabricator via cfe-commits

malaperle added inline comments.



Comment at: lib/Index/IndexRecordWriter.cpp:155
+if (!IsNew) {
+  llvm::errs() << "Index: Duplicate USR! " << SymInfo.USR << "\n";
+  // FIXME: print more information so it's easier to find the declaration.

I'm getting quite a bit of those while indexing Clangd, it looks like it comes 
from some LLVM/Support headers:
Index: Duplicate USR! c:@N@std@ST>2#NI#Nb@__try_lock_impl
Index: Duplicate USR! c:@N@llvm@ST>1#T@DenseMapInfo
Index: Duplicate USR! c:@N@llvm@ST>1#T@isPodLike
Index: Duplicate USR! c:@N@llvm@N@detail@ST>1#T@unit
Index: Duplicate USR! c:@N@llvm@ST>2#T#T@format_provider
Index: Duplicate USR! c:@N@llvm@ST>2#T#T@format_provider
Index: Duplicate USR! c:@N@llvm@ST>1#T@PointerLikeTypeTraits
Index: Duplicate USR! c:@N@llvm@ST>1#T@simplify_type
Index: Duplicate USR! c:@N@std@ST>1#T@atomic
Index: Duplicate USR! c:@N@llvm@ST>1#T@isPodLike
Index: Duplicate USR! c:@N@llvm@ST>1#T@DenseMapInfo

I think it would be good to have the file name at least in the log. I also 
assume those duplication are issues that would have to be fixed in USRGenerator 
(i.e. in separate patches) ?


https://reviews.llvm.org/D39050



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

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

2017-11-08 Thread Sam McCall via Phabricator via cfe-commits

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

llvm::make_unique


https://reviews.llvm.org/D39799

Files:
  docs/JSONCompilationDatabase.rst
  include/clang/Tooling/CompilationDatabase.h
  lib/Tooling/CompilationDatabase.cpp
  test/Tooling/fixed-database.cpp

Index: test/Tooling/fixed-database.cpp
===
--- /dev/null
+++ test/Tooling/fixed-database.cpp
@@ -0,0 +1,8 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "-Dklazz=class" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-check "%t/test.cpp" 2>&1
+// RUN: not clang-check "%s" 2>&1 | FileCheck "%s" -check-prefix=NODB
+// NODB: unknown type name 'klazz'
+klazz F{};
Index: lib/Tooling/CompilationDatabase.cpp
===
--- lib/Tooling/CompilationDatabase.cpp
+++ lib/Tooling/CompilationDatabase.cpp
@@ -26,6 +26,7 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Support/Host.h"
+#include "llvm/Support/LineIterator.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
@@ -302,8 +303,23 @@
   std::vector StrippedArgs;
   if (!stripPositionalArgs(CommandLine, StrippedArgs, ErrorMsg))
 return nullptr;
-  return std::unique_ptr(
-  new FixedCompilationDatabase(Directory, StrippedArgs));
+  return llvm::make_unique(Directory, StrippedArgs);
+}
+
+std::unique_ptr
+FixedCompilationDatabase::loadFromFile(StringRef Path, std::string &ErrorMsg) {
+  ErrorMsg.clear();
+  llvm::ErrorOr> File =
+  llvm::MemoryBuffer::getFile(Path);
+  if (std::error_code Result = File.getError()) {
+ErrorMsg = "Error while opening fixed database: " + Result.message();
+return nullptr;
+  }
+  std::vector Args{
+  llvm::line_iterator(**File, /*SkipBlanks=*/true, /*CommentMarker=*/'#'),
+  llvm::line_iterator()};
+  return llvm::make_unique(
+  llvm::sys::path::parent_path(Path), std::move(Args));
 }
 
 FixedCompilationDatabase::
@@ -334,6 +350,24 @@
   return std::vector();
 }
 
+namespace {
+
+class FixedCompilationDatabasePlugin : public CompilationDatabasePlugin {
+  std::unique_ptr
+  loadFromDirectory(StringRef Directory, std::string &ErrorMessage) override {
+SmallString<1024> DatabasePath(Directory);
+llvm::sys::path::append(DatabasePath, "compile_flags.txt");
+return FixedCompilationDatabase::loadFromFile(DatabasePath, ErrorMessage);
+  }
+};
+
+// Register the JSONCompilationDatabasePlugin with the
+// CompilationDatabasePluginRegistry using this statically initialized variable.
+static CompilationDatabasePluginRegistry::Add
+X("fixed-compilation-database", "Reads plain-text flags file");
+
+} // namespace
+
 namespace clang {
 namespace tooling {
 
Index: include/clang/Tooling/CompilationDatabase.h
===
--- include/clang/Tooling/CompilationDatabase.h
+++ include/clang/Tooling/CompilationDatabase.h
@@ -182,6 +182,11 @@
   int &Argc, const char *const *Argv, std::string &ErrorMsg,
   Twine Directory = ".");
 
+  /// Reads flags from the given file, one-per line.
+  /// Returns nullptr and sets ErrorMessage if we can't read the file.
+  static std::unique_ptr
+  loadFromFile(StringRef Path, std::string &ErrorMsg);
+
   /// \brief Constructs a compilation data base from a specified directory
   /// and command line.
   FixedCompilationDatabase(Twine Directory, ArrayRef CommandLine);
Index: docs/JSONCompilationDatabase.rst
===
--- docs/JSONCompilationDatabase.rst
+++ docs/JSONCompilationDatabase.rst
@@ -91,3 +91,9 @@
 the top of the build directory. Clang tools are pointed to the top of
 the build directory to detect the file and use the compilation database
 to parse C++ code in the source tree.
+
+Alternatives
+
+For simple projects, Clang tools also recognize a compile_flags.txt file.
+This should contain one flag per line. The same flags will be used to compile
+any file.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39804: [clang] [python] [tests] Update TLSKind tests for newer MSVC versions

2017-11-08 Thread Masud Rahman via Phabricator via cfe-commits

frutiger added a comment.

Thanks for posting this on Phabricator, I had sent it to the mailing list last 
month.


Repository:
  rL LLVM

https://reviews.llvm.org/D39804



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

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

2017-11-08 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

In https://reviews.llvm.org/D39799#919415, @sammccall wrote:

> Currently the working directory is always the parent of `compile_flags.txt` 
> as you suggest.
>  Maybe this isn't great though - sometimes it might be important that the WD 
> is the parent of the source file?

Parent of the source file seems rather inconvenient most of the time. Even when 
reading the contents of `compile_flags.txt` one would have to consider it the 
contexts of every source file.

I'm looking at CompilationDatabase.cpp:321 

 and don't see the `CompileCommand::Directory` being set at all. Maybe I'm 
missing something.

A few ideas for tests:

1. Test relative include paths (`-Irelpath/to/include`) work.
2. What happens if both `compile_commands.json` and `compile_flags.txt` are 
present? Should we test and document it? I'd expect the first one to take 
priority and the second one to come into play only if the input file is not 
found in the first one.

https://reviews.llvm.org/D39799

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

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

2017-11-08 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a comment.

In https://reviews.llvm.org/D39799#919468, @ilya-biryukov wrote:

> In https://reviews.llvm.org/D39799#919415, @sammccall wrote:
>
> > Currently the working directory is always the parent of `compile_flags.txt` 
> > as you suggest.
> >  Maybe this isn't great though - sometimes it might be important that the 
> > WD is the parent of the source file?
>
>
> Parent of the source file seems rather inconvenient most of the time. Even 
> when reading the contents of `compile_flags.txt` one would have to consider 
> it the contexts of every source file.

Understood, this is the best behavior I think. I'm just wondering whether it's 
a good enough heuristic to be silently applied.
e.g. IIUC, things like `#include "sibling.h"` won't look for the h file next to 
the cc file?

> I'm looking at CompilationDatabase.cpp:321 
> 
>  and don't see the `CompileCommand::Directory` being set at all. Maybe I'm 
> missing something.

`Result` is initialized with a copy of `CompilerCommands`, whose element is 
initialized with the directory passed to the constructor.

> A few ideas for tests:
> 
> 1. Test relative include paths (`-Irelpath/to/include`) work.

Definitely will do this.

> 2. What happens if both `compile_commands.json` and `compile_flags.txt` are 
> present? Should we test and document it? I'd expect the first one to take 
> priority and the second one to come into play only if the input file is not 
> found in the first one.

Ideally yes... i'm not sure this is so easy with the registry mechanism though 
:-(
They'd have to be present at the same level, so I'm not sure this is a huge 
deal.

https://reviews.llvm.org/D39799

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

[PATCH] D39804: [clang] [python] [tests] Update TLSKind tests for newer MSVC versions

2017-11-08 Thread Reid Kleckner via Phabricator via cfe-commits

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

lgtm


Repository:
  rL LLVM

https://reviews.llvm.org/D39804



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

r317700 - [analyzer] Fix a crash on logical operators with vectors.

2017-11-08 Thread Artem Dergachev via cfe-commits

Author: dergachev
Date: Wed Nov  8 09:27:58 2017
New Revision: 317700

URL: http://llvm.org/viewvc/llvm-project?rev=317700&view=rev
Log:
[analyzer] Fix a crash on logical operators with vectors.

Do not crash when trying to compute x && y or x || y where x and y are
of a vector type.

For now we do not seem to properly model operations with vectors. In particular,
operations && and || on a pair of vectors are not short-circuit, unlike regular
logical operators, so even our CFG is incorrect.

Avoid the crash, add respective FIXME tests for later.

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

rdar://problem/34317663

Added:
cfe/trunk/test/Analysis/vector.c
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp?rev=317700&r1=317699&r2=317700&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp Wed Nov  8 09:27:58 2017
@@ -626,6 +626,16 @@ void ExprEngine::VisitLogicalExpr(const
   StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);
   ProgramStateRef state = Pred->getState();
 
+  if (B->getType()->isVectorType()) {
+// FIXME: We do not model vector arithmetic yet. When adding support for
+// that, note that the CFG-based reasoning below does not apply, because
+// logical operators on vectors are not short-circuit. Currently they are
+// modeled as short-circuit in Clang CFG but this is incorrect.
+// Do not set the value for the expression. It'd be UnknownVal by default.
+Bldr.generateNode(B, Pred, state);
+return;
+  }
+
   ExplodedNode *N = Pred;
   while (!N->getLocation().getAs()) {
 ProgramPoint P = N->getLocation();

Added: cfe/trunk/test/Analysis/vector.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/vector.c?rev=317700&view=auto
==
--- cfe/trunk/test/Analysis/vector.c (added)
+++ cfe/trunk/test/Analysis/vector.c Wed Nov  8 09:27:58 2017
@@ -0,0 +1,28 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify 
%s
+
+typedef int __attribute__((ext_vector_type(2))) V;
+
+void clang_analyzer_numTimesReached();
+void clang_analyzer_eval(int);
+
+int flag;
+
+V pass_through_and_set_flag(V v) {
+  flag = 1;
+  return v;
+}
+
+V dont_crash_and_dont_split_state(V x, V y) {
+  flag = 0;
+  V z = x && pass_through_and_set_flag(y);
+  clang_analyzer_eval(flag); // expected-warning{{TRUE}}
+  // FIXME: For now we treat vector operator && as short-circuit,
+  // but in fact it is not. It should always evaluate
+  // pass_through_and_set_flag(). It should not split state.
+  // Now we also get FALSE on the other path.
+  // expected-warning@-5{{FALSE}}
+
+  // FIXME: Should be 1 since we should not split state.
+  clang_analyzer_numTimesReached(); // expected-warning{{2}}
+  return z;
+}


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

[PATCH] D39682: [analyzer] Fix a crash on logical operators with vectors.

2017-11-08 Thread Phabricator via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL317700: [analyzer] Fix a crash on logical operators with 
vectors. (authored by dergachev).

Changed prior to commit:
  https://reviews.llvm.org/D39682?vs=121856&id=122108#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39682

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  cfe/trunk/test/Analysis/vector.c


Index: cfe/trunk/test/Analysis/vector.c
===
--- cfe/trunk/test/Analysis/vector.c
+++ cfe/trunk/test/Analysis/vector.c
@@ -0,0 +1,28 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify 
%s
+
+typedef int __attribute__((ext_vector_type(2))) V;
+
+void clang_analyzer_numTimesReached();
+void clang_analyzer_eval(int);
+
+int flag;
+
+V pass_through_and_set_flag(V v) {
+  flag = 1;
+  return v;
+}
+
+V dont_crash_and_dont_split_state(V x, V y) {
+  flag = 0;
+  V z = x && pass_through_and_set_flag(y);
+  clang_analyzer_eval(flag); // expected-warning{{TRUE}}
+  // FIXME: For now we treat vector operator && as short-circuit,
+  // but in fact it is not. It should always evaluate
+  // pass_through_and_set_flag(). It should not split state.
+  // Now we also get FALSE on the other path.
+  // expected-warning@-5{{FALSE}}
+
+  // FIXME: Should be 1 since we should not split state.
+  clang_analyzer_numTimesReached(); // expected-warning{{2}}
+  return z;
+}
Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -626,6 +626,16 @@
   StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);
   ProgramStateRef state = Pred->getState();
 
+  if (B->getType()->isVectorType()) {
+// FIXME: We do not model vector arithmetic yet. When adding support for
+// that, note that the CFG-based reasoning below does not apply, because
+// logical operators on vectors are not short-circuit. Currently they are
+// modeled as short-circuit in Clang CFG but this is incorrect.
+// Do not set the value for the expression. It'd be UnknownVal by default.
+Bldr.generateNode(B, Pred, state);
+return;
+  }
+
   ExplodedNode *N = Pred;
   while (!N->getLocation().getAs()) {
 ProgramPoint P = N->getLocation();


Index: cfe/trunk/test/Analysis/vector.c
===
--- cfe/trunk/test/Analysis/vector.c
+++ cfe/trunk/test/Analysis/vector.c
@@ -0,0 +1,28 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+typedef int __attribute__((ext_vector_type(2))) V;
+
+void clang_analyzer_numTimesReached();
+void clang_analyzer_eval(int);
+
+int flag;
+
+V pass_through_and_set_flag(V v) {
+  flag = 1;
+  return v;
+}
+
+V dont_crash_and_dont_split_state(V x, V y) {
+  flag = 0;
+  V z = x && pass_through_and_set_flag(y);
+  clang_analyzer_eval(flag); // expected-warning{{TRUE}}
+  // FIXME: For now we treat vector operator && as short-circuit,
+  // but in fact it is not. It should always evaluate
+  // pass_through_and_set_flag(). It should not split state.
+  // Now we also get FALSE on the other path.
+  // expected-warning@-5{{FALSE}}
+
+  // FIXME: Should be 1 since we should not split state.
+  clang_analyzer_numTimesReached(); // expected-warning{{2}}
+  return z;
+}
Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -626,6 +626,16 @@
   StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);
   ProgramStateRef state = Pred->getState();
 
+  if (B->getType()->isVectorType()) {
+// FIXME: We do not model vector arithmetic yet. When adding support for
+// that, note that the CFG-based reasoning below does not apply, because
+// logical operators on vectors are not short-circuit. Currently they are
+// modeled as short-circuit in Clang CFG but this is incorrect.
+// Do not set the value for the expression. It'd be UnknownVal by default.
+Bldr.generateNode(B, Pred, state);
+return;
+  }
+
   ExplodedNode *N = Pred;
   while (!N->getLocation().getAs()) {
 ProgramPoint P = N->getLocation();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

2017-11-08 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

In https://reviews.llvm.org/D39799#919494, @sammccall wrote:

> e.g. IIUC, things like `#include "sibling.h"` won't look for the h file next 
> to the cc file?

Actually, this should work as quoted includes are always searched relative to 
the current file before looking in the include paths. No matter what the WD is.

> `Result` is initialized with a copy of `CompilerCommands`, whose element is 
> initialized with the directory passed to the constructor.

Missed that, thanks!

> Ideally yes... i'm not sure this is so easy with the registry mechanism 
> though :-(
>  They'd have to be present at the same level, so I'm not sure this is a huge 
> deal.

Yeah, looks like this would require unifying `FixedCompilationDatabase` and 
`JSONCompilationDatabase`. This doesn't seem too crazy, though. "A class that 
handles reading compilation arguments from the filesystem." seems like a valid 
entity to me.

https://reviews.llvm.org/D39799

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

[PATCH] D38110: [libunwind][MIPS]: Add support for unwinding in O32 and N64 processes.

2017-11-08 Thread John Baldwin via Phabricator via cfe-commits

bsdjhb added a comment.

Ping


https://reviews.llvm.org/D38110



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

[PATCH] D39804: [clang] [python] [tests] Update TLSKind tests for newer MSVC versions

2017-11-08 Thread Michał Górny via Phabricator via cfe-commits

mgorny added a comment.

@frutiger, do you maybe happen to have fixes for the two other test failures? 
Maybe I'm tracking them down unnecessarily ;-).


Repository:
  rL LLVM

https://reviews.llvm.org/D39804



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

[PATCH] D38221: Add CoreOption flag to "-coverage" option to make it available for clang-cl

2017-11-08 Thread Reid Kleckner via Phabricator via cfe-commits

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

lgtm


https://reviews.llvm.org/D38221



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

[PATCH] D39622: Fix type debug information generation for enum-based template specialization

2017-11-08 Thread Adrian Prantl via Phabricator via cfe-commits

aprantl added a comment.

For clarification: what is the "symbols table" you are referring to in the 
description?


Repository:
  rL LLVM

https://reviews.llvm.org/D39622



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

[PATCH] D39707: [analyzer] assume bitwise arithmetic axioms

2017-11-08 Thread Devin Coughlin via Phabricator via cfe-commits

dcoughlin added a comment.

I believe your motivating examples used errno_t, which is a signed type.

I'm fine with assuming a two's complement value representation for signed 
integers, which would make Artem's suggestion work. That assumption definitely 
deserves a comment, though.


https://reviews.llvm.org/D39707



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

[PATCH] D39810: [clang] [python] [tests] Disable the broken unique-external linkage test

2017-11-08 Thread Michał Górny via Phabricator via cfe-commits

mgorny created this revision.

Starting with https://reviews.llvm.org/rL314037, anonymous namespaces no longer 
give
unique-external linkage to variables. As a result, the relevant test no
longer works correctly. Comment it out until a replacement expression is
found.


Repository:
  rL LLVM

https://reviews.llvm.org/D39810

Files:
  bindings/python/tests/cindex/test_linkage.py


Index: bindings/python/tests/cindex/test_linkage.py
===
--- bindings/python/tests/cindex/test_linkage.py
+++ bindings/python/tests/cindex/test_linkage.py
@@ -12,7 +12,6 @@
 tu = get_tu("""
 void foo() { int no_linkage; }
 static int internal;
-namespace { extern int unique_external; }
 extern int external;
 """, lang = 'cpp')
 
@@ -22,8 +21,9 @@
 internal = get_cursor(tu.cursor, 'internal')
 assert internal.linkage == LinkageKind.INTERNAL
 
-unique_external = get_cursor(tu.cursor, 'unique_external')
-assert unique_external.linkage == LinkageKind.UNIQUE_EXTERNAL
+# TODO: find an expression producing unique-external linkage
+#unique_external = get_cursor(tu.cursor, 'unique_external')
+#assert unique_external.linkage == LinkageKind.UNIQUE_EXTERNAL
 
 external = get_cursor(tu.cursor, 'external')
 assert external.linkage == LinkageKind.EXTERNAL


Index: bindings/python/tests/cindex/test_linkage.py
===
--- bindings/python/tests/cindex/test_linkage.py
+++ bindings/python/tests/cindex/test_linkage.py
@@ -12,7 +12,6 @@
 tu = get_tu("""
 void foo() { int no_linkage; }
 static int internal;
-namespace { extern int unique_external; }
 extern int external;
 """, lang = 'cpp')
 
@@ -22,8 +21,9 @@
 internal = get_cursor(tu.cursor, 'internal')
 assert internal.linkage == LinkageKind.INTERNAL
 
-unique_external = get_cursor(tu.cursor, 'unique_external')
-assert unique_external.linkage == LinkageKind.UNIQUE_EXTERNAL
+# TODO: find an expression producing unique-external linkage
+#unique_external = get_cursor(tu.cursor, 'unique_external')
+#assert unique_external.linkage == LinkageKind.UNIQUE_EXTERNAL
 
 external = get_cursor(tu.cursor, 'external')
 assert external.linkage == LinkageKind.EXTERNAL
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39804: [clang] [python] [tests] Update TLSKind tests for newer MSVC versions

2017-11-08 Thread Masud Rahman via Phabricator via cfe-commits

frutiger added a comment.

I will commit this shortly.

@mgorny I have a fix for the `unique_external` issue here: 
https://reviews.llvm.org/D39161


Repository:
  rL LLVM

https://reviews.llvm.org/D39804



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

[PATCH] D39810: [clang] [python] [tests] Disable the broken unique-external linkage test

2017-11-08 Thread Michał Górny via Phabricator via cfe-commits

mgorny abandoned this revision.
mgorny added a comment.

Duplicate of https://reviews.llvm.org/D39161.


Repository:
  rL LLVM

https://reviews.llvm.org/D39810



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

[PATCH] D39804: [clang] [python] [tests] Update TLSKind tests for newer MSVC versions

2017-11-08 Thread Michał Górny via Phabricator via cfe-commits

mgorny added a comment.

I see that you haven't more luck than I did finding a better solution. How 
about the code completion failure? I suppose I still have to figure that one 
out on my own ;-).

I can commit this one myself, I have it ready for commit already.


Repository:
  rL LLVM

https://reviews.llvm.org/D39804



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

[PATCH] D39812: [Driver, CodeGen] pass through and apply -fassociative-math

2017-11-08 Thread Sanjay Patel via Phabricator via cfe-commits

spatel created this revision.
Herald added a subscriber: mcrosier.

There are 2 parts to getting the -fassociative-math command-line flag 
translated to LLVM FMF:

1. In the driver/frontend, we accept the flag and its 'no' inverse and deal 
with the interactions with other flags like -ffast-math. This was mostly 
already done - we just needed to pass the flag on as a codegen option. The test 
file is complicated because there are many potential combinations of flags here.

2. In codegen, we map the option to FMF in the IR builder. This is simple code 
and corresponding test.

For the motivating example from PR27372:

  float foo(float a, float x) { return ((a + x) - x); }
  
  $ ./clang -O2 27372.c -S -o - -ffast-math  -fno-associative-math -emit-llvm  
| egrep 'fadd|fsub'
%add = fadd nnan ninf nsz arcp contract float %0, %1
%sub = fsub nnan ninf nsz arcp contract float %add, %2

So 'reassoc' is off as expected (and so is the new 'afn' but that's a different 
patch). This case now works as expected end-to-end although the underlying 
logic is still wrong:

  $ ./clang  -O2 27372.c -S -o - -ffast-math  -fno-associative-math | grep xmm
addss   %xmm1, %xmm0
subss   %xmm1, %xmm0

We're not done because the case where 'reassoc' is set is ignored by optimizer 
passes. Example:

  $ ./clang  -O2 27372.c -S -o - -fassociative-math -emit-llvm  | grep fadd
%add = fadd reassoc float %0, %1
  
  $ ./clang -O2  27372.c -S -o - -fassociative-math | grep xmm
addss   %xmm1, %xmm0
subss   %xmm1, %xmm0


https://reviews.llvm.org/D39812

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CodeGenFunction.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/finite-math.c
  test/Driver/fast-math.c

Index: test/Driver/fast-math.c
===
--- test/Driver/fast-math.c
+++ test/Driver/fast-math.c
@@ -121,18 +121,23 @@
 // RUN:   | FileCheck --check-prefix=CHECK-UNSAFE-MATH %s
 // CHECK-UNSAFE-MATH: "-cc1"
 // CHECK-UNSAFE-MATH: "-menable-unsafe-fp-math"
+// CHECK-UNSAFE-MATH: "-fassociative-math"
 //
 // RUN: %clang -### -fno-fast-math -fno-math-errno -fassociative-math -freciprocal-math \
 // RUN: -fno-signed-zeros -fno-trapping-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-FAST-MATH-UNSAFE-MATH %s
 // CHECK-NO-FAST-MATH-UNSAFE-MATH: "-cc1"
 // CHECK-NO-FAST-MATH-UNSAFE-MATH: "-menable-unsafe-fp-math"
-//
+// CHECK-NO-FAST-MATH-UNSAFE-MATH: "-fassociative-math"
+
+// The 2nd -fno-fast-math overrides -fassociative-math.
+
 // RUN: %clang -### -fno-fast-math -fno-math-errno -fassociative-math -freciprocal-math \
 // RUN: -fno-fast-math -fno-signed-zeros -fno-trapping-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-UNSAFE-MATH-NO-FAST-MATH %s
 // CHECK-UNSAFE-MATH-NO-FAST-MATH: "-cc1"
 // CHECK-UNSAFE-MATH-NO-FAST-MATH-NOT: "-menable-unsafe-fp-math"
+// CHECK-UNSAFE-MATH-NO-FAST-MATH-NOT: "-fassociative-math"
 //
 // Check that various umbrella flags also enable these frontend options.
 // RUN: %clang -### -ffast-math -c %s 2>&1 \
@@ -151,7 +156,7 @@
 // One umbrella flag is *really* weird and also changes the semantics of the
 // program by adding a special preprocessor macro. Check that the frontend flag
 // modeling this semantic change is provided. Also check that the flag is not
-// present if any of the optimization is disabled.
+// present if any of the optimizations are disabled.
 // RUN: %clang -### -ffast-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FAST-MATH %s
 // RUN: %clang -### -fno-fast-math -ffast-math -c %s 2>&1 \
@@ -175,8 +180,11 @@
 // RUN:   | FileCheck --check-prefix=CHECK-NO-FAST-MATH %s
 // RUN: %clang -### -ffast-math -fmath-errno -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-FAST-MATH %s
+// RUN: %clang -### -ffast-math -fno-associative-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-NO-FAST-MATH --check-prefix=CHECK-ASSOC-MATH %s
 // CHECK-NO-FAST-MATH: "-cc1"
 // CHECK-NO-FAST-MATH-NOT: "-ffast-math"
+// CHECK-ASSOC-MATH-NOT: "-fassociative-math"
 //
 // Check various means of disabling these flags, including disabling them after
 // they've been enabled via an umbrella flag.
@@ -209,21 +217,16 @@
 // CHECK-NO-NO-NANS-NOT: "-menable-no-nans"
 // CHECK-NO-NO-NANS-NOT: "-ffinite-math-only"
 // CHECK-NO-NO-NANS: "-o"
-//
+
+// A later inverted option overrides an earlier option.
+
 // RUN: %clang -### -fassociative-math -freciprocal-math -fno-signed-zeros \
 // RUN: -fno-trapping-math -fno-associative-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
-// RUN: %clang -### -fassociative-math -freciprocal-math -fno-signed-zeros \
-// RUN: -fno-trapping-math -fno-reciprocal-math -c %s 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
-// RUN: %clang -### -fassociative-math -freciprocal-

r317706 - [bindings] fix TLS test failure

2017-11-08 Thread Masud Rahman via cfe-commits

Author: frutiger
Date: Wed Nov  8 11:17:27 2017
New Revision: 317706

URL: http://llvm.org/viewvc/llvm-project?rev=317706&view=rev
Log:
[bindings] fix TLS test failure

Since cfe commit r237337, '__declspec(thread)' and 'thread_local' have
been the same since MSVC 2015.  i.e. they are both considered to supply
a dynamic TLS kind, not a static TLS kind.

This test originally did not specify which version of MS compatibility
to assume.  As a result, the test was brittle, since changing the
default compatibility version could break the test.

This commit adds a specific version when building up the flags used to
parse the translation unit, and tests both versions.

Modified:
cfe/trunk/bindings/python/tests/cindex/test_tls_kind.py

Modified: cfe/trunk/bindings/python/tests/cindex/test_tls_kind.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/bindings/python/tests/cindex/test_tls_kind.py?rev=317706&r1=317705&r2=317706&view=diff
==
--- cfe/trunk/bindings/python/tests/cindex/test_tls_kind.py (original)
+++ cfe/trunk/bindings/python/tests/cindex/test_tls_kind.py Wed Nov  8 11:17:27 
2017
@@ -27,11 +27,21 @@ _Thread_local int tls_static;
 # The following case tests '__declspec(thread)'.  Since it is a Microsoft
 # specific extension, specific flags are required for the parser to pick
 # these up.
-flags = ['-fms-extensions', '-target', 'x86_64-unknown-windows-win32']
+flags = ['-fms-extensions', '-target', 'x86_64-unknown-windows-win32',
+ '-fms-compatibility-version=18']
 tu = get_tu("""
-__declspec(thread) int tls_declspec;
+__declspec(thread) int tls_declspec_msvc18;
 """, lang = 'cpp', flags=flags)
 
-tls_declspec = get_cursor(tu.cursor, 'tls_declspec')
-assert tls_declspec.tls_kind == TLSKind.STATIC
+tls_declspec_msvc18 = get_cursor(tu.cursor, 'tls_declspec_msvc18')
+assert tls_declspec_msvc18.tls_kind == TLSKind.STATIC
+
+flags = ['-fms-extensions', '-target', 'x86_64-unknown-windows-win32',
+ '-fms-compatibility-version=19']
+tu = get_tu("""
+__declspec(thread) int tls_declspec_msvc19;
+""", lang = 'cpp', flags=flags)
+
+tls_declspec_msvc19 = get_cursor(tu.cursor, 'tls_declspec_msvc19')
+assert tls_declspec_msvc19.tls_kind == TLSKind.DYNAMIC
 


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

[PATCH] D39804: [clang] [python] [tests] Update TLSKind tests for newer MSVC versions

2017-11-08 Thread Masud Rahman via Phabricator via cfe-commits

frutiger closed this revision.
frutiger added a comment.

Ah missed your last note.  Committed as `r317706`.


Repository:
  rL LLVM

https://reviews.llvm.org/D39804



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

[PATCH] D39812: [Driver, CodeGen] pass through and apply -fassociative-math

2017-11-08 Thread Sanjay Patel via Phabricator via cfe-commits

spatel added a comment.

I just reviewed the gcc docs:
https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html

"[-fassociative-math] requires that both -fno-signed-zeros and 
-fno-trapping-math be in effect."

If we want to match that behavior, I need to change this patch to light up 
'nsz' and the no-trapping-math function attribute.


https://reviews.llvm.org/D39812



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

[PATCH] D39812: [Driver, CodeGen] pass through and apply -fassociative-math

2017-11-08 Thread Warren Ristow via Phabricator via cfe-commits

wristow added a comment.

In https://reviews.llvm.org/D39812#919687, @spatel wrote:

> I just reviewed the gcc docs:
>  https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html
>
> "[-fassociative-math] requires that both -fno-signed-zeros and 
> -fno-trapping-math be in effect."
>
> If we want to match that behavior, I need to change this patch to light up 
> 'nsz' and the no-trapping-math function attribute.

Yes, I think we want to match that behavior.  But by "light up" 'nsz' and the 
no-trapping-math attribute, do you mean automatically turn them on when 
'-fassociative-math' is specified?  I'd think it should be the other way 
around: Suppress the effect of '-fassociative-math' unless both 
'-fno-signed-zeros' and '-fno-trapping-math' are also specified.

https://reviews.llvm.org/D39812

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

[PATCH] D39161: [bindings] remove unique_external test failure

2017-11-08 Thread Masud Rahman via Phabricator via cfe-commits

frutiger added a comment.

Friendly poke @rsmith @compnerd


https://reviews.llvm.org/D39161



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

r317716 - Add a missing "REQUIRES: system-windows" to a Windows-only test.

2017-11-08 Thread David L. Jones via cfe-commits

Author: dlj
Date: Wed Nov  8 12:03:11 2017
New Revision: 317716

URL: http://llvm.org/viewvc/llvm-project?rev=317716&view=rev
Log:
Add a missing "REQUIRES: system-windows" to a Windows-only test.

This un-breaks builds on other platforms. Otherwise, they fail due to warnings 
like:

warning: unable to find a Visual Studio installation; try running Clang from a 
developer command prompt [-Wmsvc-not-found]

Modified:
cfe/trunk/test/Driver/coverage.c

Modified: cfe/trunk/test/Driver/coverage.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/coverage.c?rev=317716&r1=317715&r2=317716&view=diff
==
--- cfe/trunk/test/Driver/coverage.c (original)
+++ cfe/trunk/test/Driver/coverage.c Wed Nov  8 12:03:11 2017
@@ -1,4 +1,5 @@
 // Test coverage flag.
+// REQUIRES: system-windows
 //
 // RUN: %clang_cl -### -coverage %s -o foo/bar.o 2>&1 | FileCheck 
-check-prefix=CLANG-CL-COVERAGE %s
 // CLANG-CL-COVERAGE-NOT: error:


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

[PATCH] D39217: [libclang, bindings]: add spelling location

2017-11-08 Thread Masud Rahman via Phabricator via cfe-commits

frutiger added a comment.

@jklaehn do you know why the referenced cursor would point to line 2?


https://reviews.llvm.org/D39217



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

[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete

2017-11-08 Thread Cameron via Phabricator via cfe-commits

cameron314 added inline comments.



Comment at: cfe/trunk/lib/Sema/SemaOverload.cpp:6396
   } else {
 AddTemplateOverloadCandidate(FunTmpl, F.getPair(),
  ExplicitTemplateArgs, Args,

The same slice that was added above needs to be done here for templated static 
methods that are accessed via a non-static object.


Repository:
  rL LLVM

https://reviews.llvm.org/D36390



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

r317719 - [OPENMP] Codegen for `#pragma omp target parallel for`.

2017-11-08 Thread Alexey Bataev via cfe-commits

Author: abataev
Date: Wed Nov  8 12:16:14 2017
New Revision: 317719

URL: http://llvm.org/viewvc/llvm-project?rev=317719&view=rev
Log:
[OPENMP] Codegen for `#pragma omp target parallel for`.

Added:
cfe/trunk/test/OpenMP/target_parallel_for_codegen_registration.cpp
cfe/trunk/test/OpenMP/target_parallel_for_codegen_registration_naming.cpp
cfe/trunk/test/OpenMP/target_parallel_for_debug_codegen.cpp
Modified:
cfe/trunk/include/clang/AST/Stmt.h
cfe/trunk/include/clang/AST/StmtOpenMP.h
cfe/trunk/lib/Basic/OpenMPKinds.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/target_parallel_for_codegen.cpp

Modified: cfe/trunk/include/clang/AST/Stmt.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Stmt.h?rev=317719&r1=317718&r2=317719&view=diff
==
--- cfe/trunk/include/clang/AST/Stmt.h (original)
+++ cfe/trunk/include/clang/AST/Stmt.h Wed Nov  8 12:16:14 2017
@@ -406,6 +406,9 @@ public:
   /// \brief Skip no-op (attributed, compound) container stmts and skip 
captured
   /// stmt at the top, if \a IgnoreCaptured is true.
   Stmt *IgnoreContainers(bool IgnoreCaptured = false);
+  const Stmt *IgnoreContainers(bool IgnoreCaptured = false) const {
+return const_cast(this)->IgnoreContainers(IgnoreCaptured);
+  }
 
   const Stmt *stripLabelLikeStatements() const;
   Stmt *stripLabelLikeStatements() {

Modified: cfe/trunk/include/clang/AST/StmtOpenMP.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/StmtOpenMP.h?rev=317719&r1=317718&r2=317719&view=diff
==
--- cfe/trunk/include/clang/AST/StmtOpenMP.h (original)
+++ cfe/trunk/include/clang/AST/StmtOpenMP.h Wed Nov  8 12:16:14 2017
@@ -899,7 +899,9 @@ public:
   }
   const Stmt *getBody() const {
 // This relies on the loop form is already checked by Sema.
-Stmt *Body = getAssociatedStmt()->IgnoreContainers(true);
+const Stmt *Body = getAssociatedStmt()->IgnoreContainers(true);
+while(const auto *CS = dyn_cast(Body))
+  Body = CS->getCapturedStmt();
 Body = cast(Body)->getBody();
 for (unsigned Cnt = 1; Cnt < CollapsedNum; ++Cnt) {
   Body = Body->IgnoreContainers();

Modified: cfe/trunk/lib/Basic/OpenMPKinds.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/OpenMPKinds.cpp?rev=317719&r1=317718&r2=317719&view=diff
==
--- cfe/trunk/lib/Basic/OpenMPKinds.cpp (original)
+++ cfe/trunk/lib/Basic/OpenMPKinds.cpp Wed Nov  8 12:16:14 2017
@@ -908,7 +908,6 @@ void clang::getOpenMPCaptureRegions(
   case OMPD_atomic:
   case OMPD_target_data:
   case OMPD_target:
-  case OMPD_target_parallel_for:
   case OMPD_target_parallel_for_simd:
   case OMPD_target_simd:
   case OMPD_task:
@@ -926,6 +925,7 @@ void clang::getOpenMPCaptureRegions(
 CaptureRegions.push_back(DKind);
 break;
   case OMPD_target_parallel:
+  case OMPD_target_parallel_for:
 CaptureRegions.push_back(OMPD_target);
 CaptureRegions.push_back(OMPD_parallel);
 break;

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=317719&r1=317718&r2=317719&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Wed Nov  8 12:16:14 2017
@@ -7121,6 +7121,10 @@ void CGOpenMPRuntime::scanForTargetRegio
   CodeGenFunction::EmitOMPTargetTeamsDeviceFunction(
   CGM, ParentName, cast(*S));
   break;
+case Stmt::OMPTargetParallelForDirectiveClass:
+  CodeGenFunction::EmitOMPTargetParallelForDeviceFunction(
+  CGM, ParentName, cast(*S));
+  break;
 default:
   llvm_unreachable("Unknown target directive for OpenMP device codegen.");
 }

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp?rev=317719&r1=317718&r2=317719&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Wed Nov  8 12:16:14 2017
@@ -276,6 +276,7 @@ getExecutionModeForDirective(CodeGenModu
   case OMPD_target_teams:
 return CGOpenMPRuntimeNVPTX::ExecutionMode::Generic;
   case OMPD_target_parallel:
+  case OMPD_target_parallel_for:
 return CGOpenMPRuntimeNVPTX::ExecutionMode::Spmd;
   default:
 llvm_unreachable("Unsupported directive on NVPTX device.");

Modified: cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
URL: 
h

[libcxx] r317722 - Added include for

2017-11-08 Thread Marshall Clow via cfe-commits

Author: marshall
Date: Wed Nov  8 12:25:47 2017
New Revision: 317722

URL: http://llvm.org/viewvc/llvm-project?rev=317722&view=rev
Log:
Added include for 

Modified:
libcxx/trunk/fuzzing/fuzzing.cpp

Modified: libcxx/trunk/fuzzing/fuzzing.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/fuzzing/fuzzing.cpp?rev=317722&r1=317721&r2=317722&view=diff
==
--- libcxx/trunk/fuzzing/fuzzing.cpp (original)
+++ libcxx/trunk/fuzzing/fuzzing.cpp Wed Nov  8 12:25:47 2017
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 


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

r317709 - Add CoreOption flag to "-coverage" option to make it available for clang-cl

2017-11-08 Thread Marco Castelluccio via cfe-commits

Author: marco
Date: Wed Nov  8 11:21:54 2017
New Revision: 317709

URL: http://llvm.org/viewvc/llvm-project?rev=317709&view=rev
Log:
Add CoreOption flag to "-coverage" option to make it available for clang-cl

Summary:
The -coverage option is not a CoreOption, so it is not available to clang-cl.
This patch adds the CoreOption flag to "-coverage" to allow it to be used with 
clang-cl.

Reviewers: rnk

Reviewed By: rnk

Subscribers: cfe-commits

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

Added:
cfe/trunk/test/Driver/coverage.c
Modified:
cfe/trunk/include/clang/Driver/Options.td

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=317709&r1=317708&r2=317709&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Wed Nov  8 11:21:54 2017
@@ -519,7 +519,7 @@ def cl_fp32_correctly_rounded_divide_sqr
 def client__name : JoinedOrSeparate<["-"], "client_name">;
 def combine : Flag<["-", "--"], "combine">, Flags<[DriverOption, Unsupported]>;
 def compatibility__version : JoinedOrSeparate<["-"], "compatibility_version">;
-def coverage : Flag<["-", "--"], "coverage">;
+def coverage : Flag<["-", "--"], "coverage">, Flags<[CoreOption]>;
 def cpp_precomp : Flag<["-"], "cpp-precomp">, Group;
 def current__version : JoinedOrSeparate<["-"], "current_version">;
 def cxx_isystem : JoinedOrSeparate<["-"], "cxx-isystem">, Group,

Added: cfe/trunk/test/Driver/coverage.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/coverage.c?rev=317709&view=auto
==
--- cfe/trunk/test/Driver/coverage.c (added)
+++ cfe/trunk/test/Driver/coverage.c Wed Nov  8 11:21:54 2017
@@ -0,0 +1,7 @@
+// Test coverage flag.
+//
+// RUN: %clang_cl -### -coverage %s -o foo/bar.o 2>&1 | FileCheck 
-check-prefix=CLANG-CL-COVERAGE %s
+// CLANG-CL-COVERAGE-NOT: error:
+// CLANG-CL-COVERAGE-NOT: warning:
+// CLANG-CL-COVERAGE-NOT: argument unused
+// CLANG-CL-COVERAGE-NOT: unknown argument


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

Re: [PATCH] Ensure std::getline always 0-terminates string.

2017-11-08 Thread Volodymyr Sapsai via cfe-commits

Thanks for the patch, Reimar. Can you please add tests to ensure this 
functionality doesn’t regress? As null character is required by the standard 
(27.7.2.3p21), a good starting point seems to be
test/std/input.output/iostream.format/input.streams/istream.unformatted/getline_pointer_size.pass.cpp
test/std/input.output/iostream.format/input.streams/istream.unformatted/getline_pointer_size_chart.pass.cpp

And what about free function std::getline that takes a stream and a string? The 
standard (21.4.8.9p7) specifies 
> […]After constructing a sentry object, if the sentry converts to true, calls 
> str.erase()[…]
It doesn’t mention explicitly effects to str if the sentry converts to false 
but it mentions that std::getline behaves as an unformatted input function 
(27.7.2.3). And 27.7.2.3p1 mentions
> […]If the sentry object returns true, when converted to a value of type bool, 
> the function endeavors to obtain the requested input. Otherwise, if the 
> sentry constructor exits by throwing an exception or if the sentry object 
> returns false, when converted to a value of type bool, the function returns 
> without attempting to obtain any input. In either case the number of 
> extracted characters is set to 0; unformatted input functions taking a 
> character array of non-zero size as an argument shall also store a null 
> character (using charT()) in the first location of the array.[…]


Technically, string is not a character array of non-zero size. But according to 
the spirit of the standard, I would expect string to be empty after reading 
into it from a stream that reached EOF. What do you think?

Regards,
Volodymyr

> On Oct 4, 2017, at 12:07, Reimar Döffinger via cfe-commits 
>  wrote:
> 
> If the sentinel failed (e.g. due to having reached
> EOF before) or an exception was caught it failed to
> do that.
> The C++14 standard says:
> "In any case, if n is greater than zero, it then stores
> a null character (using charT()) into the next
> successive location of the array."
> Other implementations like libstdc++ do 0-terminate and
> not doing so may lead security issues in applications.
> ---
> include/istream | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/include/istream b/include/istream
> index 0b8e05d95..5c73df38f 100644
> --- a/include/istream
> +++ b/include/istream
> @@ -1069,16 +1069,18 @@ basic_istream<_CharT, _Traits>::getline(char_type* 
> __s, streamsize __n, char_typ
> this->rdbuf()->sbumpc();
> ++__gc_;
> }
> -if (__n > 0)
> -*__s = char_type();
> if (__gc_ == 0)
>__err |= ios_base::failbit;
> this->setstate(__err);
> }
> +if (__n > 0)
> +*__s = char_type();
> #ifndef _LIBCPP_NO_EXCEPTIONS
> }
> catch (...)
> {
> +if (__n > 0)
> +*__s = char_type();
> this->__set_badbit_and_consider_rethrow();
> }
> #endif  // _LIBCPP_NO_EXCEPTIONS
> -- 
> 2.14.2
> 
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[PATCH] D39812: [Driver, CodeGen] pass through and apply -fassociative-math

2017-11-08 Thread Sanjay Patel via Phabricator via cfe-commits

spatel added a comment.

In https://reviews.llvm.org/D39812#919699, @wristow wrote:

> Yes, I think we want to match that behavior.  But by "light up" 'nsz' and the 
> no-trapping-math attribute, do you mean automatically turn them on when 
> '-fassociative-math' is specified?  I'd think it should be the other way 
> around: Suppress the effect of '-fassociative-math' unless both 
> '-fno-signed-zeros' and '-fno-trapping-math' are also specified.

Yes, my reading of the gcc docs was off. The code appears to match your 
understanding:
https://godbolt.org/g/Vau3cv

So we'll restrict the effect of -fassociative-math based on the presence of the 
other flags...more spaghetti needed. :)

https://reviews.llvm.org/D39812

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

r317727 - [ObjC] Boxed strings should use the nullability from stringWithUTF8String's return type

2017-11-08 Thread Alex Lorenz via cfe-commits

Author: arphaman
Date: Wed Nov  8 13:33:15 2017
New Revision: 317727

URL: http://llvm.org/viewvc/llvm-project?rev=317727&view=rev
Log:
[ObjC] Boxed strings should use the nullability from stringWithUTF8String's 
return type

Objective-C NSString has a class method stringWithUTF8String that creates a new
NSString from a C string. Objective-C box expression @(...) can be used to
create an NSString instead of invoking the stringWithUTF8String method directly
(The compiler lowers it down to the invocation though). This commit ensures that
the type of @(string-value) gets the same nullability attributes as the return
type of stringWithUTF8String to ensure that the diagnostics are consistent
between the two.

rdar://33847186

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

Added:
cfe/trunk/test/SemaObjC/transfer-boxed-string-nullability.m
Modified:
cfe/trunk/lib/Sema/SemaExprObjC.cpp

Modified: cfe/trunk/lib/Sema/SemaExprObjC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprObjC.cpp?rev=317727&r1=317726&r2=317727&view=diff
==
--- cfe/trunk/lib/Sema/SemaExprObjC.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprObjC.cpp Wed Nov  8 13:33:15 2017
@@ -564,6 +564,13 @@ ExprResult Sema::BuildObjCBoxedExpr(Sour
   
   BoxingMethod = StringWithUTF8StringMethod;
   BoxedType = NSStringPointer;
+  // Transfer the nullability from method's return type.
+  Optional Nullability =
+  BoxingMethod->getReturnType()->getNullability(Context);
+  if (Nullability)
+BoxedType = Context.getAttributedType(
+AttributedType::getNullabilityAttrKind(*Nullability), BoxedType,
+BoxedType);
 }
   } else if (ValueType->isBuiltinType()) {
 // The other types we support are numeric, char and BOOL/bool. We could 
also

Added: cfe/trunk/test/SemaObjC/transfer-boxed-string-nullability.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/transfer-boxed-string-nullability.m?rev=317727&view=auto
==
--- cfe/trunk/test/SemaObjC/transfer-boxed-string-nullability.m (added)
+++ cfe/trunk/test/SemaObjC/transfer-boxed-string-nullability.m Wed Nov  8 
13:33:15 2017
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fblocks -fobjc-arc -Wnullable-to-nonnull-conversion 
-fsyntax-only -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -fblocks -fobjc-arc -Wnullable-to-nonnull-conversion 
-fsyntax-only -verify -Wno-objc-root-class -DNOWARN %s
+
+@interface NSString
+
++ (NSString*
+#ifndef NOWARN
+  _Nullable
+#else
+  _Nonnull
+#endif
+) stringWithUTF8String:(const char*)x;
+
+@end
+
+void takesNonNull(NSString * _Nonnull ptr);
+
+void testBoxedString() {
+  const char *str = "hey";
+  takesNonNull([NSString stringWithUTF8String:str]);
+  takesNonNull(@(str));
+#ifndef NOWARN
+  // expected-warning@-3 {{implicit conversion from nullable pointer 'NSString 
* _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
+  // expected-warning@-3 {{implicit conversion from nullable pointer 'NSString 
* _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
+#else
+  // expected-no-diagnostics
+#endif
+}


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

[PATCH] D39762: [ObjC] Boxed strings should use the nullability from stringWithUTF8String's return type

2017-11-08 Thread Alex Lorenz via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL317727: [ObjC] Boxed strings should use the nullability from 
stringWithUTF8String's… (authored by arphaman).

Changed prior to commit:
  https://reviews.llvm.org/D39762?vs=121982&id=122148#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39762

Files:
  cfe/trunk/lib/Sema/SemaExprObjC.cpp
  cfe/trunk/test/SemaObjC/transfer-boxed-string-nullability.m


Index: cfe/trunk/lib/Sema/SemaExprObjC.cpp
===
--- cfe/trunk/lib/Sema/SemaExprObjC.cpp
+++ cfe/trunk/lib/Sema/SemaExprObjC.cpp
@@ -564,6 +564,13 @@
   
   BoxingMethod = StringWithUTF8StringMethod;
   BoxedType = NSStringPointer;
+  // Transfer the nullability from method's return type.
+  Optional Nullability =
+  BoxingMethod->getReturnType()->getNullability(Context);
+  if (Nullability)
+BoxedType = Context.getAttributedType(
+AttributedType::getNullabilityAttrKind(*Nullability), BoxedType,
+BoxedType);
 }
   } else if (ValueType->isBuiltinType()) {
 // The other types we support are numeric, char and BOOL/bool. We could 
also
Index: cfe/trunk/test/SemaObjC/transfer-boxed-string-nullability.m
===
--- cfe/trunk/test/SemaObjC/transfer-boxed-string-nullability.m
+++ cfe/trunk/test/SemaObjC/transfer-boxed-string-nullability.m
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fblocks -fobjc-arc -Wnullable-to-nonnull-conversion 
-fsyntax-only -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -fblocks -fobjc-arc -Wnullable-to-nonnull-conversion 
-fsyntax-only -verify -Wno-objc-root-class -DNOWARN %s
+
+@interface NSString
+
++ (NSString*
+#ifndef NOWARN
+  _Nullable
+#else
+  _Nonnull
+#endif
+) stringWithUTF8String:(const char*)x;
+
+@end
+
+void takesNonNull(NSString * _Nonnull ptr);
+
+void testBoxedString() {
+  const char *str = "hey";
+  takesNonNull([NSString stringWithUTF8String:str]);
+  takesNonNull(@(str));
+#ifndef NOWARN
+  // expected-warning@-3 {{implicit conversion from nullable pointer 'NSString 
* _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
+  // expected-warning@-3 {{implicit conversion from nullable pointer 'NSString 
* _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
+#else
+  // expected-no-diagnostics
+#endif
+}


Index: cfe/trunk/lib/Sema/SemaExprObjC.cpp
===
--- cfe/trunk/lib/Sema/SemaExprObjC.cpp
+++ cfe/trunk/lib/Sema/SemaExprObjC.cpp
@@ -564,6 +564,13 @@
   
   BoxingMethod = StringWithUTF8StringMethod;
   BoxedType = NSStringPointer;
+  // Transfer the nullability from method's return type.
+  Optional Nullability =
+  BoxingMethod->getReturnType()->getNullability(Context);
+  if (Nullability)
+BoxedType = Context.getAttributedType(
+AttributedType::getNullabilityAttrKind(*Nullability), BoxedType,
+BoxedType);
 }
   } else if (ValueType->isBuiltinType()) {
 // The other types we support are numeric, char and BOOL/bool. We could also
Index: cfe/trunk/test/SemaObjC/transfer-boxed-string-nullability.m
===
--- cfe/trunk/test/SemaObjC/transfer-boxed-string-nullability.m
+++ cfe/trunk/test/SemaObjC/transfer-boxed-string-nullability.m
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fblocks -fobjc-arc -Wnullable-to-nonnull-conversion -fsyntax-only -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -fblocks -fobjc-arc -Wnullable-to-nonnull-conversion -fsyntax-only -verify -Wno-objc-root-class -DNOWARN %s
+
+@interface NSString
+
++ (NSString*
+#ifndef NOWARN
+  _Nullable
+#else
+  _Nonnull
+#endif
+) stringWithUTF8String:(const char*)x;
+
+@end
+
+void takesNonNull(NSString * _Nonnull ptr);
+
+void testBoxedString() {
+  const char *str = "hey";
+  takesNonNull([NSString stringWithUTF8String:str]);
+  takesNonNull(@(str));
+#ifndef NOWARN
+  // expected-warning@-3 {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
+  // expected-warning@-3 {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
+#else
+  // expected-no-diagnostics
+#endif
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39050: Add index-while-building support to Clang

2017-11-08 Thread Marc-Andre Laperle via Phabricator via cfe-commits

malaperle added inline comments.



Comment at: include/indexstore/IndexStoreCXX.h:75
+  bool foreachRelation(llvm::function_ref receiver) 
{
+#if INDEXSTORE_HAS_BLOCKS
+return indexstore_occurrence_relations_apply(obj, 
^bool(indexstore_symbol_relation_t sym_rel) {

I know this is an old revision but I thought I should ask for the future 
patch... Would it be possible to not use "blocks"? This will affect portability 
of the code. I'm not familiar with blocks but I would think it would be 
possible to replace with C++11 lambdas, or something else that's standard. I 
was just playing around with this code and since I used GCC, it did not work.


https://reviews.llvm.org/D39050



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

[PATCH] D39817: [CUDA] Fix std::min on device side to return the min, not the max.

2017-11-08 Thread Justin Lebar via Phabricator via cfe-commits

jlebar created this revision.
Herald added a subscriber: sanjoy.

How embarrassing.

This is tested in the test-suite -- fix to come there in a separate
patch.


https://reviews.llvm.org/D39817

Files:
  clang/lib/Headers/cuda_wrappers/algorithm


Index: clang/lib/Headers/cuda_wrappers/algorithm
===
--- clang/lib/Headers/cuda_wrappers/algorithm
+++ clang/lib/Headers/cuda_wrappers/algorithm
@@ -80,7 +80,7 @@
 template 
 inline __device__ const __T &
 min(const __T &__a, const __T &__b) {
-  return __a < __b ? __b : __a;
+  return __a < __b ? __a : __b;
 }
 
 #ifdef _LIBCPP_END_NAMESPACE_STD


Index: clang/lib/Headers/cuda_wrappers/algorithm
===
--- clang/lib/Headers/cuda_wrappers/algorithm
+++ clang/lib/Headers/cuda_wrappers/algorithm
@@ -80,7 +80,7 @@
 template 
 inline __device__ const __T &
 min(const __T &__a, const __T &__b) {
-  return __a < __b ? __b : __a;
+  return __a < __b ? __a : __b;
 }
 
 #ifdef _LIBCPP_END_NAMESPACE_STD
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39818: [CUDA] [test-suite] Test std::min and std::max with C++11.

2017-11-08 Thread Justin Lebar via Phabricator via cfe-commits

jlebar created this revision.
Herald added a subscriber: sanjoy.

Previously we only tested on C++14 and newer, which let slip a bug where
std::min returned the max.  :(


https://reviews.llvm.org/D39818

Files:
  External/CUDA/algorithm.cu


Index: External/CUDA/algorithm.cu
===
--- External/CUDA/algorithm.cu
+++ External/CUDA/algorithm.cu
@@ -7,39 +7,42 @@
 // we can successfully compile and run the standard library's implementations
 // of these functions.
 
-#if __cplusplus >= 201402L
+#if __cplusplus >= 201103L
 
 #include 
 #include 
 #include 
 #include 
 
-__device__ void greater() {
-  assert(std::greater()(1, 0));
-}
-
 __device__ void min() {
   assert(std::min(0, 1) == 0);
-  assert(std::min({5, 1, 10}) == 1);
 }
 
 __device__ void max() {
   assert(std::max(0, 1) == 1);
-  assert(std::max({5, 1, 10}, std::less()) == 10);
 }
 
-__device__ void minmax() {
+// Clang has device-side shims implementing std::min and std::max for scalars
+// starting in C++11, but doesn't implement minimax or std::min/max on
+// initializer_lists until C++14, when it gets these for free from the standard
+// library (because they're constexpr).
+__device__ void cpp14_tests() {
+#if __cplusplus >= 201402L
+  assert(std::greater()(1, 0));
+  assert(std::min({5, 1, 10}) == 1);
+  assert(std::max({5, 1, 10}, std::less()) == 10);
+
   assert(std::minmax(1, 0).first == 0);
   assert(std::minmax(1, 0).second == 1);
   assert(std::minmax({0, 10, -10, 100}, std::less()).first == -10);
   assert(std::minmax({0, 10, -10, 100}, std::less()).second == 100);
+#endif
 }
 
 __global__ void kernel() {
-  greater();
   min();
   max();
-  minmax();
+  cpp14_tests();
 }
 
 int main() {


Index: External/CUDA/algorithm.cu
===
--- External/CUDA/algorithm.cu
+++ External/CUDA/algorithm.cu
@@ -7,39 +7,42 @@
 // we can successfully compile and run the standard library's implementations
 // of these functions.
 
-#if __cplusplus >= 201402L
+#if __cplusplus >= 201103L
 
 #include 
 #include 
 #include 
 #include 
 
-__device__ void greater() {
-  assert(std::greater()(1, 0));
-}
-
 __device__ void min() {
   assert(std::min(0, 1) == 0);
-  assert(std::min({5, 1, 10}) == 1);
 }
 
 __device__ void max() {
   assert(std::max(0, 1) == 1);
-  assert(std::max({5, 1, 10}, std::less()) == 10);
 }
 
-__device__ void minmax() {
+// Clang has device-side shims implementing std::min and std::max for scalars
+// starting in C++11, but doesn't implement minimax or std::min/max on
+// initializer_lists until C++14, when it gets these for free from the standard
+// library (because they're constexpr).
+__device__ void cpp14_tests() {
+#if __cplusplus >= 201402L
+  assert(std::greater()(1, 0));
+  assert(std::min({5, 1, 10}) == 1);
+  assert(std::max({5, 1, 10}, std::less()) == 10);
+
   assert(std::minmax(1, 0).first == 0);
   assert(std::minmax(1, 0).second == 1);
   assert(std::minmax({0, 10, -10, 100}, std::less()).first == -10);
   assert(std::minmax({0, 10, -10, 100}, std::less()).second == 100);
+#endif
 }
 
 __global__ void kernel() {
-  greater();
   min();
   max();
-  minmax();
+  cpp14_tests();
 }
 
 int main() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39817: [CUDA] Fix std::min on device side to return the min, not the max.

2017-11-08 Thread Artem Belevich via Phabricator via cfe-commits

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

Ouch. LGTM.


https://reviews.llvm.org/D39817



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

[PATCH] D39818: [CUDA] [test-suite] Test std::min and std::max with C++11.

2017-11-08 Thread Artem Belevich via Phabricator via cfe-commits

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

LGTM.


https://reviews.llvm.org/D39818



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

[PATCH] D39571: [clangd] DidChangeConfiguration Notification

2017-11-08 Thread William Enright via Phabricator via cfe-commits

Nebiroth added inline comments.



Comment at: clangd/ClangdLSPServer.cpp:51
+  "definitionProvider": true,
+  "configurationChangeProvider": true
 }})");

malaperle wrote:
> Are you sure the existing tests don't fail? usually when we change this, some 
> tests have to be adjusted.
I'll verify this when I will have the time.



Comment at: clangd/ClangdServer.h:289
+  /// ChangedSettings
+  void changeConfiguration(std::map ChangedSettings);
+

ilya-biryukov wrote:
> This function is way too general for `ClangdServer`'s interface, can we have 
> more fine-grained settings here (i.e. `setCompletionParameters` etc?) and 
> handle the "general" case in `ClangdLSPServer` (i.e., unknown setting names, 
> invalid setting parameters, json parsing, etc.)?
> 
> I suggest we remove this function altogether.
So if I understand correctly, we would have the most generic 
workspace/didChangeConfiguration handler located in ClangdLSPServer
 with a name perhaps similar to  changeConfiguration  that would pass valid 
settings to more specific functions inside ClangdServer ?



Comment at: clangd/Protocol.cpp:534
 
+llvm::Optional
+DidChangeConfigurationParams::parse(llvm::yaml::MappingNode *Params,

malaperle wrote:
> I think this needs a test, perhaps create did-change-configuration.test? It 
> can also test the ClangdConfigurationParams::parse code
> If you do, I think it's a good idea to test a few failing cases:
> - "settings" is not a mapping node. You can test this with a scalar node like 
> "settings": ""
> - Something else than "settings" is present, so that it goes through 
> logIgnoredField
> - "settings" is not set at all. In this case, because it's mandatory in the 
> protocol, return llvm::None. This can be checked after the loop after all 
> key/values were read.
> 
> There are similar tests in execute-command.test if you'd like an example.
> And of course also add a "successful" case as well  :)
Yes, I was planning on adding a simple test in the coming days. I imagine this 
test will be greatly expanded on once more settings become available to change 
on the server side.


https://reviews.llvm.org/D39571



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

[PATCH] D39812: [Driver, CodeGen] pass through and apply -fassociative-math

2017-11-08 Thread Sanjay Patel via Phabricator via cfe-commits

spatel updated this revision to Diff 122155.
spatel added a comment.

Patch updated:
Match gcc's handling of -fassociative-math with -fno-signed-zeros and 
-fno-trapping-math. I've created a new codegen option (-mreassociate) that maps 
directly to LLVM's 'reassoc' flag, so we limit the mix-and-match logic of these 
flags to the driver/frontend.

So now we have:

  $ ./clang  27372.c -S -o -  -fassociative-math -O2 -emit-llvm | egrep 
'fadd|fsub'
%add = fadd float %a, %x
%sub = fsub float %add, %x
  $ ./clang  27372.c -S -o -  -fassociative-math -fno-signed-zeros -O2 
-emit-llvm | egrep 'fadd|fsub'
%add = fadd nsz float %a, %x
%sub = fsub nsz float %add, %x
  $ ./clang  27372.c -S -o -  -fassociative-math -fno-signed-zeros 
-fno-trapping-math -O2 -emit-llvm | egrep 'fadd|fsub'
%add = fadd reassoc nsz float %a, %x
%sub = fsub reassoc nsz float %add, %x


https://reviews.llvm.org/D39812

Files:
  include/clang/Driver/CC1Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CodeGenFunction.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/finite-math.c


Index: test/CodeGen/finite-math.c
===
--- test/CodeGen/finite-math.c
+++ test/CodeGen/finite-math.c
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -ffinite-math-only -emit-llvm -o - %s | FileCheck %s 
-check-prefix=CHECK -check-prefix=FINITE
 // RUN: %clang_cc1 -fno-signed-zeros -emit-llvm -o - %s | FileCheck %s 
-check-prefix=CHECK  -check-prefix=NSZ
 // RUN: %clang_cc1 -freciprocal-math -emit-llvm -o - %s | FileCheck %s 
-check-prefix=CHECK  -check-prefix=RECIP
+// RUN: %clang_cc1 -mreassociate -emit-llvm -o - %s | FileCheck %s 
-check-prefix=CHECK  -check-prefix=REASSOC
 
 float f0, f1, f2;
 
@@ -10,6 +11,7 @@
   // FINITE: fadd nnan ninf
   // NSZ: fadd nsz
   // RECIP: fadd arcp
+  // REASSOC: fadd reassoc
   f0 = f1 + f2;
 
   // CHECK: ret
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -637,6 +637,7 @@
 Args.hasArg(OPT_cl_no_signed_zeros) ||
 Args.hasArg(OPT_cl_unsafe_math_optimizations) ||
 Args.hasArg(OPT_cl_fast_relaxed_math));
+  Opts.Reassociate = Args.hasArg(OPT_mreassociate);
   Opts.FlushDenorm = Args.hasArg(OPT_cl_denorms_are_zero);
   Opts.CorrectlyRoundedDivSqrt =
   Args.hasArg(OPT_cl_fp32_correctly_rounded_divide_sqrt);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2156,6 +2156,9 @@
   if (!SignedZeros)
 CmdArgs.push_back("-fno-signed-zeros");
 
+  if (AssociativeMath && !SignedZeros && !TrappingMath)
+CmdArgs.push_back("-mreassociate");
+
   if (ReciprocalMath)
 CmdArgs.push_back("-freciprocal-math");
 
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -101,6 +101,9 @@
   if (CGM.getCodeGenOpts().ReciprocalMath) {
 FMF.setAllowReciprocal();
   }
+  if (CGM.getCodeGenOpts().Reassociate) {
+FMF.setAllowReassoc();
+  }
   Builder.setFastMathFlags(FMF);
 }
 
Index: include/clang/Frontend/CodeGenOptions.def
===
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -110,6 +110,7 @@
 CODEGENOPT(NoImplicitFloat   , 1, 0) ///< Set when -mno-implicit-float is 
enabled.
 CODEGENOPT(NoInfsFPMath  , 1, 0) ///< Assume FP arguments, results not 
+-Inf.
 CODEGENOPT(NoSignedZeros , 1, 0) ///< Allow ignoring the signedness of FP 
zero
+CODEGENOPT(Reassociate   , 1, 0) ///< Allow reassociation of FP math ops
 CODEGENOPT(ReciprocalMath, 1, 0) ///< Allow FP divisions to be 
reassociated.
 CODEGENOPT(NoTrappingMath, 1, 0) ///< Set when -fno-trapping-math is 
enabled.
 CODEGENOPT(NoNaNsFPMath  , 1, 0) ///< Assume FP arguments, results not NaN.
Index: include/clang/Driver/CC1Options.td
===
--- include/clang/Driver/CC1Options.td
+++ include/clang/Driver/CC1Options.td
@@ -261,6 +261,8 @@
 def menable_unsafe_fp_math : Flag<["-"], "menable-unsafe-fp-math">,
   HelpText<"Allow unsafe floating-point math optimizations which may decrease "
"precision">;
+def mreassociate : Flag<["-"], "mreassociate">,
+  HelpText<"Allow reassociation transformations for floating-point 
instructions">;
 def mfloat_abi : Separate<["-"], "mfloat-abi">,
   HelpText<"The float ABI to use">;
 def mtp : Separate<["-"], "mtp">,


Index: test/CodeGen/finite-math.c
===
--- test/CodeGen/finite-math.c

[libcxx] r317734 - [libcxx] Mark test cxa_deleted_virtual.pass.cpp as failing for previous libcxx versions.

2017-11-08 Thread Volodymyr Sapsai via cfe-commits

Author: vsapsai
Date: Wed Nov  8 14:30:29 2017
New Revision: 317734

URL: http://llvm.org/viewvc/llvm-project?rev=317734&view=rev
Log:
[libcxx] Mark test cxa_deleted_virtual.pass.cpp as failing for previous libcxx 
versions.

r313500 added a fix for undefined "___cxa_deleted_virtual" symbol.
Previous libcxx versions don't have the fix and corresponding test
should be failing.

rdar://problem/34521053

Reviewers: EricWF, mclow.lists, ahatanak

Reviewed By: ahatanak

Subscribers: mehdi_amini, cfe-commits

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


Modified:
libcxx/trunk/test/libcxx/language.support/cxa_deleted_virtual.pass.cpp

Modified: libcxx/trunk/test/libcxx/language.support/cxa_deleted_virtual.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/language.support/cxa_deleted_virtual.pass.cpp?rev=317734&r1=317733&r2=317734&view=diff
==
--- libcxx/trunk/test/libcxx/language.support/cxa_deleted_virtual.pass.cpp 
(original)
+++ libcxx/trunk/test/libcxx/language.support/cxa_deleted_virtual.pass.cpp Wed 
Nov  8 14:30:29 2017
@@ -10,6 +10,15 @@
 // UNSUPPORTED: c++98, c++03
 
 // Test exporting the symbol: "__cxa_deleted_virtual" in macosx
+// But don't expect the symbol to be exported in previous versions.
+//
+// XFAIL: with_system_cxx_lib=macosx10.13
+// XFAIL: with_system_cxx_lib=macosx10.12
+// XFAIL: with_system_cxx_lib=macosx10.11
+// XFAIL: with_system_cxx_lib=macosx10.10
+// XFAIL: with_system_cxx_lib=macosx10.9
+// XFAIL: with_system_cxx_lib=macosx10.8
+// XFAIL: with_system_cxx_lib=macosx10.7
 
 struct S { virtual void f() = delete; virtual ~S() {} };
 int main() {


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

[PATCH] D39776: [libcxx] Mark test cxa_deleted_virtual.pass.cpp as failing for previous libcxx versions.

2017-11-08 Thread Volodymyr Sapsai via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL317734: [libcxx] Mark test cxa_deleted_virtual.pass.cpp as 
failing for previous libcxx… (authored by vsapsai).

Changed prior to commit:
  https://reviews.llvm.org/D39776?vs=122024&id=122157#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39776

Files:
  libcxx/trunk/test/libcxx/language.support/cxa_deleted_virtual.pass.cpp


Index: libcxx/trunk/test/libcxx/language.support/cxa_deleted_virtual.pass.cpp
===
--- libcxx/trunk/test/libcxx/language.support/cxa_deleted_virtual.pass.cpp
+++ libcxx/trunk/test/libcxx/language.support/cxa_deleted_virtual.pass.cpp
@@ -10,6 +10,15 @@
 // UNSUPPORTED: c++98, c++03
 
 // Test exporting the symbol: "__cxa_deleted_virtual" in macosx
+// But don't expect the symbol to be exported in previous versions.
+//
+// XFAIL: with_system_cxx_lib=macosx10.13
+// XFAIL: with_system_cxx_lib=macosx10.12
+// XFAIL: with_system_cxx_lib=macosx10.11
+// XFAIL: with_system_cxx_lib=macosx10.10
+// XFAIL: with_system_cxx_lib=macosx10.9
+// XFAIL: with_system_cxx_lib=macosx10.8
+// XFAIL: with_system_cxx_lib=macosx10.7
 
 struct S { virtual void f() = delete; virtual ~S() {} };
 int main() {


Index: libcxx/trunk/test/libcxx/language.support/cxa_deleted_virtual.pass.cpp
===
--- libcxx/trunk/test/libcxx/language.support/cxa_deleted_virtual.pass.cpp
+++ libcxx/trunk/test/libcxx/language.support/cxa_deleted_virtual.pass.cpp
@@ -10,6 +10,15 @@
 // UNSUPPORTED: c++98, c++03
 
 // Test exporting the symbol: "__cxa_deleted_virtual" in macosx
+// But don't expect the symbol to be exported in previous versions.
+//
+// XFAIL: with_system_cxx_lib=macosx10.13
+// XFAIL: with_system_cxx_lib=macosx10.12
+// XFAIL: with_system_cxx_lib=macosx10.11
+// XFAIL: with_system_cxx_lib=macosx10.10
+// XFAIL: with_system_cxx_lib=macosx10.9
+// XFAIL: with_system_cxx_lib=macosx10.8
+// XFAIL: with_system_cxx_lib=macosx10.7
 
 struct S { virtual void f() = delete; virtual ~S() {} };
 int main() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39812: [Driver, CodeGen] pass through and apply -fassociative-math

2017-11-08 Thread Sanjay Patel via Phabricator via cfe-commits

spatel updated this revision to Diff 122158.
spatel added a comment.

Updated again - the last upload was missing the driver's test file.


https://reviews.llvm.org/D39812

Files:
  include/clang/Driver/CC1Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CodeGenFunction.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/finite-math.c
  test/Driver/fast-math.c

Index: test/Driver/fast-math.c
===
--- test/Driver/fast-math.c
+++ test/Driver/fast-math.c
@@ -121,18 +121,23 @@
 // RUN:   | FileCheck --check-prefix=CHECK-UNSAFE-MATH %s
 // CHECK-UNSAFE-MATH: "-cc1"
 // CHECK-UNSAFE-MATH: "-menable-unsafe-fp-math"
+// CHECK-UNSAFE-MATH: "-mreassociate"
 //
 // RUN: %clang -### -fno-fast-math -fno-math-errno -fassociative-math -freciprocal-math \
 // RUN: -fno-signed-zeros -fno-trapping-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-FAST-MATH-UNSAFE-MATH %s
 // CHECK-NO-FAST-MATH-UNSAFE-MATH: "-cc1"
 // CHECK-NO-FAST-MATH-UNSAFE-MATH: "-menable-unsafe-fp-math"
-//
+// CHECK-NO-FAST-MATH-UNSAFE-MATH: "-mreassociate"
+
+// The 2nd -fno-fast-math overrides -fassociative-math.
+
 // RUN: %clang -### -fno-fast-math -fno-math-errno -fassociative-math -freciprocal-math \
 // RUN: -fno-fast-math -fno-signed-zeros -fno-trapping-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-UNSAFE-MATH-NO-FAST-MATH %s
 // CHECK-UNSAFE-MATH-NO-FAST-MATH: "-cc1"
 // CHECK-UNSAFE-MATH-NO-FAST-MATH-NOT: "-menable-unsafe-fp-math"
+// CHECK-UNSAFE-MATH-NO-FAST-MATH-NOT: "-mreassociate"
 //
 // Check that various umbrella flags also enable these frontend options.
 // RUN: %clang -### -ffast-math -c %s 2>&1 \
@@ -151,7 +156,7 @@
 // One umbrella flag is *really* weird and also changes the semantics of the
 // program by adding a special preprocessor macro. Check that the frontend flag
 // modeling this semantic change is provided. Also check that the flag is not
-// present if any of the optimization is disabled.
+// present if any of the optimizations are disabled.
 // RUN: %clang -### -ffast-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FAST-MATH %s
 // RUN: %clang -### -fno-fast-math -ffast-math -c %s 2>&1 \
@@ -175,8 +180,11 @@
 // RUN:   | FileCheck --check-prefix=CHECK-NO-FAST-MATH %s
 // RUN: %clang -### -ffast-math -fmath-errno -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-FAST-MATH %s
+// RUN: %clang -### -ffast-math -fno-associative-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-NO-FAST-MATH --check-prefix=CHECK-ASSOC-MATH %s
 // CHECK-NO-FAST-MATH: "-cc1"
 // CHECK-NO-FAST-MATH-NOT: "-ffast-math"
+// CHECK-ASSOC-MATH-NOT: "-mreassociate"
 //
 // Check various means of disabling these flags, including disabling them after
 // they've been enabled via an umbrella flag.
@@ -209,21 +217,16 @@
 // CHECK-NO-NO-NANS-NOT: "-menable-no-nans"
 // CHECK-NO-NO-NANS-NOT: "-ffinite-math-only"
 // CHECK-NO-NO-NANS: "-o"
-//
+
+// A later inverted option overrides an earlier option.
+
 // RUN: %clang -### -fassociative-math -freciprocal-math -fno-signed-zeros \
 // RUN: -fno-trapping-math -fno-associative-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
-// RUN: %clang -### -fassociative-math -freciprocal-math -fno-signed-zeros \
-// RUN: -fno-trapping-math -fno-reciprocal-math -c %s 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
-// RUN: %clang -### -fassociative-math -freciprocal-math -fno-signed-zeros \
-// RUN: -fno-trapping-math -fsigned-zeros -c %s 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
-// RUN: %clang -### -fassociative-math -freciprocal-math -fno-signed-zeros \
-// RUN: -fno-trapping-math -ftrapping-math -c %s 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
+
 // RUN: %clang -### -funsafe-math-optimizations -fno-associative-math -c %s \
 // RUN:   2>&1 | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
+
 // RUN: %clang -### -funsafe-math-optimizations -fno-reciprocal-math -c %s \
 // RUN:   2>&1 | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
 // RUN: %clang -### -funsafe-math-optimizations -fsigned-zeros -c %s 2>&1 \
@@ -235,18 +238,52 @@
 // RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
 // RUN: %clang -### -ffast-math -fno-associative-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
+
 // RUN: %clang -### -ffast-math -fno-reciprocal-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
 // RUN: %clang -### -ffast-math -fsigned-zeros -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
 // RUN: %clang -### -ffast-math -ftrapping-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
 // RUN: %clang -### -ffast-math -fno-unsafe-math-optimizations -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-

r317736 - [ObjC] Fix function signature handling for blocks literals with attributes

2017-11-08 Thread Alex Lorenz via cfe-commits

Author: arphaman
Date: Wed Nov  8 14:44:34 2017
New Revision: 317736

URL: http://llvm.org/viewvc/llvm-project?rev=317736&view=rev
Log:
[ObjC] Fix function signature handling for blocks literals with attributes

Block literals can have a type with attributes in its signature, e.g.
ns_returns_retained. The code that inspected the type loc of the block when
declaring its parameters didn't account for this fact, and only looked through
paren type loc. This commit ensures that getAsAdjusted is used instead of
IgnoreParens to find the block's FunctionProtoTypeLoc. This ensures that
block parameters are declared correctly in the block and avoids the
'undeclared identifier' error.

rdar://35416160

Added:
cfe/trunk/test/SemaObjC/block-literal-with-attribute.m
Modified:
cfe/trunk/lib/Sema/SemaExpr.cpp

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=317736&r1=317735&r2=317736&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Nov  8 14:44:34 2017
@@ -12859,8 +12859,8 @@ void Sema::ActOnBlockArguments(SourceLoc
   // Look for an explicit signature in that function type.
   FunctionProtoTypeLoc ExplicitSignature;
 
-  TypeLoc tmp = Sig->getTypeLoc().IgnoreParens();
-  if ((ExplicitSignature = tmp.getAs())) {
+  if ((ExplicitSignature =
+   Sig->getTypeLoc().getAsAdjusted())) {
 
 // Check whether that explicit signature was synthesized by
 // GetTypeForDeclarator.  If so, don't save that as part of the

Added: cfe/trunk/test/SemaObjC/block-literal-with-attribute.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/block-literal-with-attribute.m?rev=317736&view=auto
==
--- cfe/trunk/test/SemaObjC/block-literal-with-attribute.m (added)
+++ cfe/trunk/test/SemaObjC/block-literal-with-attribute.m Wed Nov  8 14:44:34 
2017
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify -fblocks -fobjc-arc
+// RUN: %clang_cc1 -fsyntax-only %s -verify -fblocks
+// FIXME: should compile
+
+__auto_type block = ^ id __attribute__((ns_returns_retained)) (id filter) {
+  return filter; // ok
+};
+__auto_type block2 = ^  __attribute__((ns_returns_retained)) id (id filter) {
+  return filter; // ok
+};
+__auto_type block3 = ^ id (id filter)  __attribute__((ns_returns_retained))  {
+  return filter; // ok
+};
+
+// expected-no-diagnostics


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

r317737 - Remove redundant copy-pasted comment in test file from r317736

2017-11-08 Thread Alex Lorenz via cfe-commits

Author: arphaman
Date: Wed Nov  8 14:47:15 2017
New Revision: 317737

URL: http://llvm.org/viewvc/llvm-project?rev=317737&view=rev
Log:
Remove redundant copy-pasted comment in test file from r317736

Modified:
cfe/trunk/test/SemaObjC/block-literal-with-attribute.m

Modified: cfe/trunk/test/SemaObjC/block-literal-with-attribute.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/block-literal-with-attribute.m?rev=317737&r1=317736&r2=317737&view=diff
==
--- cfe/trunk/test/SemaObjC/block-literal-with-attribute.m (original)
+++ cfe/trunk/test/SemaObjC/block-literal-with-attribute.m Wed Nov  8 14:47:15 
2017
@@ -1,6 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only %s -verify -fblocks -fobjc-arc
 // RUN: %clang_cc1 -fsyntax-only %s -verify -fblocks
-// FIXME: should compile
 
 __auto_type block = ^ id __attribute__((ns_returns_retained)) (id filter) {
   return filter; // ok


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

1 2 >

1 - 100 of 122 matches

Mail list logo