date:20191212

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

2019-12-12 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a comment.

Design question around newlines :-) Otherwise looks good.

Comment at: clang-tools-extra/clangd/FormattedString.cpp:95
+// Concatanates whitespace blocks into a single ` `.
+std::string canonicalizeSpaces(std::string Input) {
+  // Goes over the string and preserves only a single ` ` for any whitespace

kadircet wrote:
> sammccall wrote:
> > is this equivalent to
> > ```
> > SmallVector Words;
> > llvm::SplitString(Input, Words);
> > return llvm::join(Input, " ");
> > ```
> > ?
> > 
> > (That would trim leading/trailing whitespace, but I suspect we actually 
> > want that)
> yes that's exactly what we want, thanks!
> not just joining them though, since we could still merge them in-place.
I'm really not sure saving the allocation is worth the code here, but up to you.

Comment at: clang-tools-extra/clangd/FormattedString.cpp:118

-std::string FormattedString::renderAsMarkdown() const {
+enum RenderType {
+  PlainText,

(why not just pass in the fptr?)

Comment at: clang-tools-extra/clangd/FormattedString.cpp:122
+};
+std::string renderBlocks(llvm::ArrayRef> Children,
+ RenderType RT) {

I'm not sure this common codepath for plain-text and markdown will survive in 
the long run:
 - state around indentation and wrapping is likely to differ
 - separating blocks with one newline may not be the right thing in each case

Comment at: clang-tools-extra/clangd/FormattedString.cpp:130
+OS << Sep;
+Sep = "\n";
+((*C).*RenderFunc)(OS);

for markdown, this means consecutive paragraphs will be rendered as a single 
paragraph with line breaks in it. intended?

Comment at: clang-tools-extra/clangd/FormattedString.cpp:139
+public:
+  void asMarkdown(llvm::raw_ostream &OS) const override {}
+  void asPlainText(llvm::raw_ostream &OS) const override {}

this deserves a comment - the effect is produced by the surrounding newlines.
But if we change the blocks to include their whitespace it'll be more obvious.

Comment at: clang-tools-extra/clangd/FormattedString.h:29
 public:
+  virtual void asMarkdown(llvm::raw_ostream &OS) const = 0;
+  virtual void asPlainText(llvm::raw_ostream &OS) const = 0;

Do these include trailing newline(s), or not?

Based on offline discussion, they currently do not, but this causes some 
problems e.g. list followed by paragraph would get merged.

Option a) have Blocks include required trailing newlines (e.g. \n\n after lists 
in markdown, \n after paragraphs), and have Document trim trailing newlines.
Option b) have Document look at the type of the elements above/below and insert 
space appropriately.

Personally prefer option A because it encapsulates behavior better in the 
classes (less dyn_cast) and I think reflects the grammar better.

Comment at: clang-tools-extra/clangd/Hover.cpp:460
   if (!Definition.empty()) {
-Output.appendCodeBlock(Definition);
+Output.addParagraph().appendCode(Definition);
   } else {

This is a regression at the moment - complicated definitions are 
clang-formatted, and we'll canonicalize the whitespace.
(The behavior of the library is correct, we just need code block for this).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71248

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

[PATCH] D71397: [clang] Improve LLVM-style RTTI support in ExternalASTSource/ExternalSemaSource

2019-12-12 Thread Raphael Isemann via Phabricator via cfe-commits

teemperor created this revision.
teemperor added reviewers: aprantl, dblaikie, rjmccall.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We currently have some very basic LLVM-style RTTI support in the 
ExternalASTSource class hierarchy
based on the `SemaSource` bool( to discriminate it form the 
ExternalSemaSource). As ExternalASTSource 
is supposed to be subclassed we should have extendable LLVM-style RTTI in this 
class hierarchy to make life easier
for projects building on top of Clang.

Most notably the current RTTI implementation forces LLDB to implement RTTI for 
its
own ExternalASTSource class (ClangExternalASTSourceCommon) by keeping a global 
set of
ExternalASTSources that are known to be ClangExternalASTSourceCommon. Projects
using Clang currently have to dosimilar workarounds to get RTTI support for 
their subclasses.

This patch turns this into full-fledged LLVM-style RTTI based on a static `ID` 
variable similar to
other LLVM class hierarchies. Also removes the friend declaration from 
ExternalASTSource to
its child class that was only used to grant access to the `SemaSource` member.


Repository:
  rC Clang

https://reviews.llvm.org/D71397

Files:
  clang/include/clang/AST/ExternalASTSource.h
  clang/include/clang/Sema/ExternalSemaSource.h
  clang/include/clang/Sema/MultiplexExternalSemaSource.h
  clang/lib/AST/ExternalASTSource.cpp
  clang/lib/Sema/MultiplexExternalSemaSource.cpp
  clang/lib/Sema/Sema.cpp

Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -1917,6 +1917,7 @@
 
 // Pin this vtable to this file.
 ExternalSemaSource::~ExternalSemaSource() {}
+char ExternalSemaSource::ID;
 
 void ExternalSemaSource::ReadMethodPool(Selector Sel) { }
 void ExternalSemaSource::updateOutOfDateSelector(Selector Sel) { }
Index: clang/lib/Sema/MultiplexExternalSemaSource.cpp
===
--- clang/lib/Sema/MultiplexExternalSemaSource.cpp
+++ clang/lib/Sema/MultiplexExternalSemaSource.cpp
@@ -15,6 +15,8 @@
 
 using namespace clang;
 
+char MultiplexExternalSemaSource::ID;
+
 ///Constructs a new multiplexing external sema source and appends the
 /// given element to it.
 ///
Index: clang/lib/AST/ExternalASTSource.cpp
===
--- clang/lib/AST/ExternalASTSource.cpp
+++ clang/lib/AST/ExternalASTSource.cpp
@@ -24,6 +24,8 @@
 
 using namespace clang;
 
+char ExternalASTSource::ID;
+
 ExternalASTSource::~ExternalASTSource() = default;
 
 llvm::Optional
Index: clang/include/clang/Sema/MultiplexExternalSemaSource.h
===
--- clang/include/clang/Sema/MultiplexExternalSemaSource.h
+++ clang/include/clang/Sema/MultiplexExternalSemaSource.h
@@ -36,6 +36,8 @@
 /// external AST sources that also provide information for semantic
 /// analysis.
 class MultiplexExternalSemaSource : public ExternalSemaSource {
+  // LLVM-style RTTI.
+  static char ID;
 
 private:
   SmallVector Sources; // doesn't own them.
@@ -352,9 +354,11 @@
   bool MaybeDiagnoseMissingCompleteType(SourceLocation Loc,
 QualType T) override;
 
-  // isa/cast/dyn_cast support
-  static bool classof(const MultiplexExternalSemaSource*) { return true; }
-  //static bool classof(const ExternalSemaSource*) { return true; }
+  // LLVM-style RTTI.
+  bool isA(const void *ClassID) const override {
+return ClassID == &ID || ExternalSemaSource::isA(ClassID);
+  }
+  static bool classof(const ExternalASTSource *S) { return S->isA(&ID); }
 };
 
 } // end namespace clang
Index: clang/include/clang/Sema/ExternalSemaSource.h
===
--- clang/include/clang/Sema/ExternalSemaSource.h
+++ clang/include/clang/Sema/ExternalSemaSource.h
@@ -50,10 +50,11 @@
 /// external AST sources that also provide information for semantic
 /// analysis.
 class ExternalSemaSource : public ExternalASTSource {
+  // LLVM-style RTTI.
+  static char ID;
+
 public:
-  ExternalSemaSource() {
-ExternalASTSource::SemaSource = true;
-  }
+  ExternalSemaSource() = default;
 
   ~ExternalSemaSource() override;
 
@@ -222,10 +223,11 @@
 return false;
   }
 
-  // isa/cast/dyn_cast support
-  static bool classof(const ExternalASTSource *Source) {
-return Source->SemaSource;
+  // LLVM-style RTTI.
+  bool isA(const void *ClassID) const override {
+return ClassID == &ID || ExternalASTSource::isA(ClassID);
   }
+  static bool classof(const ExternalASTSource *S) { return S->isA(&ID); }
 };
 
 } // end namespace clang
Index: clang/include/clang/AST/ExternalASTSource.h
===
--- clang/include/clang/AST/ExternalASTSource.h
+++ clang/include/clang/AST/ExternalASTSource.h
@@ -66,9 +66,8 @@
   /// whenever we might have added new r

[PATCH] D71345: [clangd] Fall back to selecting token-before-cursor if token-after-cursor fails.

2019-12-12 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a comment.

In D71345#1780632 , @nridge wrote:

> I tried to do a less general version of this (for go-to-definition only) in 
> D70727  :)

Ah, I hadn't seen that. After thinking about this a bit, I think the behavior 
in that patch is OK, and it's complexity that will do us in. More cases keep 
coming up - internally people complained about this in code actions which is 
the hardest case to fix and needs most of this complexity.

How do you feel about the approach here? I like having the description of the 
problem centralized and directing callsites toward a solution.
At the same time, it does make me sad that a fairly nice abstraction becomes so 
much harder to pick up and use, the API is pretty horrendous.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71345

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

[PATCH] D71197: llvm premerge: clang format test

2019-12-12 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 60624 tests passed, 0 failed 
and 726 were skipped.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or apply this patch 
.

Build artifacts 
: 
console-log.txt 
,
 CMakeCache.txt 
,
 clang-format.patch 
,
 test-results.xml 
,
 diff.json 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71197



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

[PATCH] D70799: [OpenMP] Lower taskyield using OpenMP IR Builder

2019-12-12 Thread Roger Ferrer Ibanez via Phabricator via cfe-commits

rogfer01 updated this revision to Diff 233524.
rogfer01 added a comment.

ChangeLog:

- Rebase


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

https://reviews.llvm.org/D70799

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/taskyield_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -234,3 +234,20 @@
 
   return Builder.saveIP();
 }
+
+void OpenMPIRBuilder::emitTaskyieldImpl(const LocationDescription &Loc) {
+  // Build call __kmpc_omp_taskyield(loc, thread_id, 0);
+  Constant *SrcLocStr = getOrCreateSrcLocStr(Loc);
+  Value *Ident = getOrCreateIdent(SrcLocStr);
+  Constant *I32Null = ConstantInt::getNullValue(Int32);
+  Value *Args[] = {Ident, getOrCreateThreadID(Ident), I32Null};
+
+  Builder.CreateCall(getOrCreateRuntimeFunction(OMPRTL___kmpc_omp_taskyield),
+ Args);
+}
+
+void OpenMPIRBuilder::CreateTaskyield(const LocationDescription &Loc) {
+  if (!updateToLocation(Loc))
+return;
+  emitTaskyieldImpl(Loc);
+}
Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -168,6 +168,8 @@
 __OMP_RTL(__kmpc_global_thread_num, false, Int32, IdentPtr)
 __OMP_RTL(__kmpc_fork_call, true, Void, IdentPtr, Int32, ParallelTaskPtr)
 __OMP_RTL(omp_get_thread_num, false, Int32, )
+__OMP_RTL(__kmpc_omp_taskwait, false, Int32, IdentPtr, Int32)
+__OMP_RTL(__kmpc_omp_taskyield, false, Int32, IdentPtr, Int32, Int32)
 
 #undef __OMP_RTL
 #undef OMP_RTL
Index: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
===
--- llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -73,6 +73,11 @@
   bool ForceSimpleCall = false,
   bool CheckCancelFlag = true);
 
+  /// Generator for '#omp taskyield'
+  ///
+  /// \param Loc The location where the taskyield directive was encountered.
+  void CreateTaskyield(const LocationDescription& Loc);
+
   ///}
 
 private:
@@ -112,6 +117,11 @@
 omp::Directive DK, bool ForceSimpleCall,
 bool CheckCancelFlag);
 
+  /// Generate a taskyield runtime call.
+  ///
+  /// \param Loc The location at which the request originated and is fulfilled.
+  void emitTaskyieldImpl(const LocationDescription &Loc);
+
   /// Return the current thread ID.
   ///
   /// \param Ident The ident (ident_t*) describing the query origin.
Index: clang/test/OpenMP/taskyield_codegen.cpp
===
--- clang/test/OpenMP/taskyield_codegen.cpp
+++ clang/test/OpenMP/taskyield_codegen.cpp
@@ -1,6 +1,10 @@
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-unknown -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+//
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-enable-irbuilder -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -x c++ -std=c++11 -triple x86_64-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -x c++ -triple x86_64-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
 
 // RUN: %clang_cc1 -verify -fopenmp-simd -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -triple x86_64-unknown-unknown -emit-pch -o %t %s
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -3249,11 +3249,18 @@
 SourceLocation Loc) {
   if (!CGF.HaveInsertPoint())
 return;
-  // Build call __kmpc_omp_taskyield(loc, thread_id, 0);
-  llvm::Value *Args[] = {
-  emitUpdateLocation(CGF, Loc), getThreadID(CGF, Loc),
-  llvm::ConstantInt::get(CGM.IntTy, /*V=*/0, /*isSigned=*/true)};
-  CGF.EmitRuntimeCall(createRuntimeFunction(OMPRTL__kmpc_omp_taskyield), Args);
+  llvm::OpenMPIRBuilder *OMPBuilder = CGF.CGM.getOpenMPIRBuilder();
+  if (OMPBuilder) {
+OMPBuilder->CreateTaskyield(CGF.Builder);
+  } else {
+// Build call __kmpc_omp_taskyield(loc, thread_id

[PATCH] D71374: Improve support of GNU mempcpy

2019-12-12 Thread serge via Phabricator via cfe-commits

serge-sans-paille added a comment.

In D71374#1780402 , @serge-sans-paille 
wrote:

> On going validation: 
> https://github.com/serge-sans-paille/llvm-project/pull/5/checks


Validation succeeded, waiting for review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71374



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

[PATCH] D71397: [clang] Improve LLVM-style RTTI support in ExternalASTSource/ExternalSemaSource

2019-12-12 Thread Raphael Isemann via Phabricator via cfe-commits

teemperor added a comment.

I didn't add specific ID's to all of the subclasses in clang/clang-tools-extra 
because we never actually check for these specific classes anywhere from what I 
can see. But if anyone thinks that would be useful to have then I can update 
the patch.


Repository:
  rC Clang

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

https://reviews.llvm.org/D71397



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

[PATCH] D71397: [clang] Improve LLVM-style RTTI support in ExternalASTSource/ExternalSemaSource

2019-12-12 Thread Raphael Isemann via Phabricator via cfe-commits

teemperor added a comment.

See D71398  for the LLDB removal of our own 
RTTI workaround.


Repository:
  rC Clang

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

https://reviews.llvm.org/D71397



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

[PATCH] D71197: llvm premerge: clang format test

2019-12-12 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
console-log.txt 
,
 CMakeCache.txt 
,
 diff.json 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71197



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

[PATCH] D71197: llvm premerge: clang format test

2019-12-12 Thread Mikhail Goncharov via Phabricator via cfe-commits

goncharov updated this revision to Diff 233530.
goncharov added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71197

Files:
  clang-tools-extra/clangd/Function.h
  clang/include/clang/Analysis/AnalysisDeclContext.h
  clang/tools/clang-check/ClangCheck.cpp
  clang/tools/clang-diff/ClangDiff.cpp
  clang/tools/clang-fuzzer/ClangFuzzer.cpp
  clang/tools/clang-refactor/ClangRefactor.cpp

Index: clang/tools/clang-refactor/ClangRefactor.cpp
===
--- clang/tools/clang-refactor/ClangRefactor.cpp
+++ clang/tools/clang-refactor/ClangRefactor.cpp
@@ -90,7 +90,6 @@
   TestSourceSelectionArgument(TestSelectionRangesInFile TestSelections)
   : TestSelections(std::move(TestSelections)) {}
 
-  void print(raw_ostream &OS) override { TestSelections.dump(OS); }
 
   std::unique_ptr
   createCustomConsumer() override {
@@ -104,6 +103,9 @@
 return TestSelections.foreachRange(SM, Callback);
   }
 
+  void print(raw_ostream &OS) override {
+  TestSelections.dump(OS);
+  }
 private:
   TestSelectionRangesInFile TestSelections;
 };
Index: clang/tools/clang-fuzzer/ClangFuzzer.cpp
===
--- clang/tools/clang-fuzzer/ClangFuzzer.cpp
+++ clang/tools/clang-fuzzer/ClangFuzzer.cpp
@@ -15,11 +15,11 @@
 #include "handle-cxx/handle_cxx.h"
 
 using namespace clang_fuzzer;
-
-extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv) { return 0; }
-
 extern "C" int LLVMFuzzerTestOneInput(uint8_t *data, size_t size) {
   std::string s((const char *)data, size);
   HandleCXX(s, "./test.cc", {"-O2"});
   return 0;
 }
+
+extern "C" int LLVMFuzzerInitialize(   int *argc,
+   char ***argv) { return 0 ; }
Index: clang/tools/clang-diff/ClangDiff.cpp
===
--- clang/tools/clang-diff/ClangDiff.cpp
+++ clang/tools/clang-diff/ClangDiff.cpp
@@ -32,12 +32,10 @@
 cl::desc("Print the internal representation of the AST as JSON."),
 cl::init(false), cl::cat(ClangDiffCategory));
 
-static cl::opt PrintMatches("dump-matches",
-  cl::desc("Print the matched nodes."),
-  cl::init(false), cl::cat(ClangDiffCategory));
+static cl::opt PrintMatches("dump-matches", cl::desc("Print the matched nodes."), cl::init(false), cl::cat(ClangDiffCategory));
 
 static cl::opt HtmlDiff("html",
-  cl::desc("Output a side-by-side diff in HTML."),
+ cl::desc("Output a side-by-side diff in HTML."),
   cl::init(false), cl::cat(ClangDiffCategory));
 
 static cl::opt SourcePath(cl::Positional, cl::desc(""),
@@ -77,7 +75,10 @@
   std::make_unique(
   std::move(Compilations));
   AdjustingCompilations->appendArgumentsAdjuster(
-  getInsertArgumentAdjuster(ArgsBefore, ArgumentInsertPosition::BEGIN));
+
+  getInsertArgumentAdjuster(ArgsBefore   ,
+
+  ArgumentInsertPosition::BEGIN));
   AdjustingCompilations->appendArgumentsAdjuster(
   getInsertArgumentAdjuster(ArgsAfter, ArgumentInsertPosition::END));
   Compilations = std::move(AdjustingCompilations);
Index: clang/tools/clang-check/ClangCheck.cpp
===
--- clang/tools/clang-check/ClangCheck.cpp
+++ clang/tools/clang-check/ClangCheck.cpp
@@ -17,10 +17,10 @@
 
 #include "clang/AST/ASTConsumer.h"
 #include "clang/CodeGen/ObjectFilePCHContainerOperations.h"
-#include "clang/Driver/Options.h"
 #include "clang/Frontend/ASTConsumers.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Rewrite/Frontend/FixItRewriter.h"
+#include "clang/Driver/Options.h"
 #include "clang/Rewrite/Frontend/FrontendActions.h"
 #include "clang/StaticAnalyzer/Frontend/FrontendActions.h"
 #include "clang/Tooling/CommonOptionsParser.h"
@@ -28,8 +28,8 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Option/OptTable.h"
 #include "llvm/Support/Path.h"
-#include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
+#include "llvm/Support/Signals.h"
 
 using namespace clang::driver;
 using namespace clang::tooling;
@@ -52,7 +52,7 @@
 );
 
 static cl::OptionCategory ClangCheckCategory("clang-check options");
-static const opt::OptTable &Options = getDriverOptTable();
+staticconst opt::OptTable &Options = getDriverOptTable();
 static cl::opt
 ASTDump("ast-dump",
 cl::desc(Options.getOptionHelpText(options::OPT_ast_dump)),
@@ -148,7 +148,7 @@
   }
 };
 
-} // namespace
+}
 
 int main(int argc, const char **argv) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
Index: clang/include/clang/Analysis/AnalysisDeclContext.h
===
--- clang/include/clang/Analysis/AnalysisDeclContext.h
+++ clang/inclu

[PATCH] D71374: Improve support of GNU mempcpy

2019-12-12 Thread Jim Lin via Phabricator via cfe-commits

Jim added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2524
+else
+  return RValue::get(Builder.CreateGEP(Dest.getPointer(), SizeVal));
   }

Is it an error here?

It should be:

```
if (BuiltinID == Builtin::BImempcpy ||
BuiltinID == Builtin::BI__builtin_mempcpy)
return RValue::get(Builder.CreateGEP(Dest.getPointer(), SizeVal));
else
return RValue::get(Dest.getPointer());
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71374



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

[PATCH] D71197: llvm premerge: clang format test

2019-12-12 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
console-log.txt 
,
 CMakeCache.txt 
,
 diff.json 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71197



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

[PATCH] D71247: [clangd] Rename constructors and destructors in cross-file case

2019-12-12 Thread Kirill Bobyrev via Phabricator via cfe-commits

kbobyrev added inline comments.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:91
+  DeclRelation::Alias | DeclRelation::TemplatePattern)) {
+// If the cursor is at the underlying CXXRecordDecl of the
+// ClassTemplateDecl, ND will be the CXXRecordDecl. In this case, we need 
to

sammccall wrote:
> why is this code being moved?
So, this code was previously only in the within-file rename path and hence 
triggering rename from a constructor/destructor-s was only possible there. 
Moving the code here improves overall "symbol selection" and fixes it for 
global renames, too.


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

https://reviews.llvm.org/D71247



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

[PATCH] D71356: [Tooling/Syntax] Helpers to find spelled tokens touching a location.

2019-12-12 Thread Kirill Bobyrev via Phabricator via cfe-commits

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



Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:320
+const syntax::Token *
+spelledIdentifierTouching(SourceLocation Loc,
+  const syntax::TokenBuffer &Tokens);

sammccall wrote:
> kbobyrev wrote:
> > Maybe `llvm::Optional<>` here?
> Pointers are the idiomatic way to express "optional, by-reference" in LLVM.
> 
> By contrast, optional is illegal, and optional is unidiomatic
Ah, I see, I thought a copy is quite cheap in this case. Makes sense then.



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:254
+  const syntax::TokenBuffer &Tokens) {
+  assert(Loc.isFileID());
+  llvm::ArrayRef All =

sammccall wrote:
> kbobyrev wrote:
> > Nit: maybe mention this requirement in the header comment?
> It is mentioned - Loc must be a spelling location.
Ah, okay, I didn't know that spelling location implies exactly this property, 
my bad.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71356



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

[PATCH] D71247: [clangd] Rename constructors and destructors in cross-file case

2019-12-12 Thread Kirill Bobyrev via Phabricator via cfe-commits

kbobyrev updated this revision to Diff 233533.
kbobyrev marked 5 inline comments as done.

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

https://reviews.llvm.org/D71247

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

Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -487,11 +487,10 @@
   "not a supported kind", HeaderFile, Index},
 
   {
-
   R"cpp(
 struct X { X operator++(int); };
 void f(X x) {x+^+;})cpp",
-  "not a supported kind", HeaderFile, Index},
+  "no symbol", HeaderFile, Index},
 
   {R"cpp(// foo is declared outside the file.
 void fo^o() {}
@@ -728,7 +727,81 @@
   struct Case {
 llvm::StringRef FooH;
 llvm::StringRef FooCC;
-  } Cases [] = {
+} Cases [] = {
+  {
+// Constructor.
+  R"cpp(
+class [[Foo]] {
+  [[Foo^]]();
+  ~[[Foo]]();
+};
+  )cpp",
+  R"cpp(
+#include "foo.h"
+[[Foo]]::[[Foo]]() {}
+[[Foo]]::~[[Foo]]() {}
+
+void func() {
+  [[Foo]] foo;
+}
+  )cpp",
+},
+{
+  // Destructor (selecting before the identifier).
+  R"cpp(
+class [[Foo]] {
+  [[Foo]]();
+  ~[[^Foo]]();
+};
+  )cpp",
+  R"cpp(
+#include "foo.h"
+[[Foo]]::[[Foo]]() {}
+[[Foo]]::~[[Foo]]() {}
+
+void func() {
+  [[Foo]] foo;
+}
+  )cpp",
+},
+{
+  // Destructor (selecting within the identifier).
+  R"cpp(
+class [[Foo]] {
+  [[Foo]]();
+  ~[[F^oo]]();
+};
+  )cpp",
+  R"cpp(
+#include "foo.h"
+[[Foo]]::[[Foo]]() {}
+[[Foo]]::~[[Foo]]() {}
+
+void func() {
+  [[Foo]] foo;
+}
+  )cpp",
+},
+{
+  // Destructor (selecting after the identifier).
+  R"cpp(
+class [[Foo]] {
+  [[Foo]]();
+  virtual ~ /*~Foo?*/[[Foo^]]() {
+int a = 4;
+  }
+};
+  )cpp",
+  R"cpp(
+#include "foo.h"
+[[Foo]]::[[Foo]]() {}
+[[Foo]]::~[[Foo]]() {}
+
+void func() {
+  [[Foo]] foo;
+}
+  )cpp",
+},
 {
   // classes.
   R"cpp(
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -18,6 +18,7 @@
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Tooling/Refactoring/Rename/USRFindingAction.h"
+#include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Error.h"
@@ -83,21 +84,20 @@
   if (!SelectedNode)
 return {};
 
-  // If the location points to a Decl, we check it is actually on the name
-  // range of the Decl. This would avoid allowing rename on unrelated tokens.
-  //   ^class Foo {} // SelectionTree returns CXXRecordDecl,
-  // // we don't attempt to trigger rename on this position.
-  // FIXME: Make this work on destructors, e.g. "~F^oo()".
-  if (const auto *D = SelectedNode->ASTNode.get()) {
-if (D->getLocation() != TokenStartLoc)
-  return {};
-  }
-
   llvm::DenseSet Result;
   for (const auto *D :
targetDecl(SelectedNode->ASTNode,
-  DeclRelation::Alias | DeclRelation::TemplatePattern))
-Result.insert(D);
+  DeclRelation::Alias | DeclRelation::TemplatePattern)) {
+// If the cursor is at the underlying CXXRecordDecl of the
+// ClassTemplateDecl, ND will be the CXXRecordDecl. In this case, we need to
+// get the primary template maunally.
+// FIXME: Do proper and well-defined canonicalization.
+D = D->getDescribedTemplate() ? D->getDescribedTemplate() : D;
+const auto *ND = llvm::dyn_cast(D);
+// Get to CXXRecordDecl from constructor or destructor.
+ND = tooling::getCanonicalSymbolDeclaration(ND);
+Result.insert(ND);
+  }
   return Result;
 }
 
@@ -214,17 +214,8 @@
 // Return all rename occurrences in the main file.
 std::vector findOccurrencesWithinFile(ParsedAST &AST,
   const NamedDecl &ND) {
-  // In theory, locateDeclAt should return the primary template. However, if the
-  // cursor is under the underlying CXXRecordDecl of the ClassTemplateDecl, ND
-  // will be the CXXRecordDecl, for this case, we need to get the primary
-  // template maunally.
-  const auto &RenameDecl =
-  ND.getDescribedTemplate() ? *ND.getDescribedTemplate() : ND;
-  // getUSRsForDeclaration will find other related symbols, e.g. v

[PATCH] D71400: [RFC] [MinGW] Implicitly add .exe suffix if not provided

2019-12-12 Thread Martin Storsjö via Phabricator via cfe-commits

mstorsjo created this revision.
mstorsjo added a reviewer: rnk.
Herald added a project: clang.

GCC implicitly adds an .exe suffix if it is given an output file name, but the 
file name doesn't contain a suffix, and there are certain users of GCC that 
rely on this behaviour (and run into issues when trying to use Clang instead of 
GCC). And MSVC's cl.exe also does the same (but not link.exe).

However, GCC only does this when actually running on windows, not when 
operating as a cross compiler.

This was reported to me at https://github.com/mstorsjo/llvm-mingw/issues/60.

As GCC doesn't have this behaviour when cross compiling, we definitely 
shouldn't introduce the behaviour in such cases (as it would break at least as 
many cases as this fixes).

Is it worth adding such inconsistent behaviour (with two separate tests, one 
for running on windows and one elsewhere)?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71400

Files:
  clang/lib/Driver/ToolChains/MinGW.cpp


Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -160,7 +160,15 @@
   }
 
   CmdArgs.push_back("-o");
-  CmdArgs.push_back(Output.getFilename());
+  const char *OutputFile = Output.getFilename();
+#ifdef _WIN32
+  if (!llvm::sys::path::filename(OutputFile).contains('.'))
+CmdArgs.push_back(Args.MakeArgString(Twine(OutputFile) + ".exe"));
+  else
+CmdArgs.push_back(OutputFile);
+#else
+  CmdArgs.push_back(OutputFile);
+#endif
 
   Args.AddAllArgs(CmdArgs, options::OPT_e);
   // FIXME: add -N, -n flags


Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -160,7 +160,15 @@
   }
 
   CmdArgs.push_back("-o");
-  CmdArgs.push_back(Output.getFilename());
+  const char *OutputFile = Output.getFilename();
+#ifdef _WIN32
+  if (!llvm::sys::path::filename(OutputFile).contains('.'))
+CmdArgs.push_back(Args.MakeArgString(Twine(OutputFile) + ".exe"));
+  else
+CmdArgs.push_back(OutputFile);
+#else
+  CmdArgs.push_back(OutputFile);
+#endif
 
   Args.AddAllArgs(CmdArgs, options::OPT_e);
   // FIXME: add -N, -n flags
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71356: [Tooling/Syntax] Helpers to find spelled tokens touching a location.

2019-12-12 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added inline comments.



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:270
+  for (const syntax::Token &Tok : spelledTokensTouching(Loc, Tokens))
+if (Tok.kind() == tok::identifier)
+  return &Tok;

NIT: add braces around `if` statement


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71356



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

Re: [PATCH] D71186: Reland "[AST] Traverse the class type loc inside the member type loc.""

2019-12-12 Thread Yvan Roux via cfe-commits

On Tue, 10 Dec 2019 at 10:24, Ilya Biryukov  wrote:
>
> Ah, some older gcc versions can't handle raw string literals inside macro 
> arguments.
> +Haojian Wu, could you fix this?

When do you plan to fix this, bots are now broken for a while... (I
don't have my SVN/Github access in place to revert or fix it myself)

> On Tue, Dec 10, 2019 at 12:22 PM Yvan Roux  wrote:
>>
>> Hi Haojian,
>>
>> AArch64 bots are broken after this commit, logs are available here:
>>
>> http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/21125/steps/ninja%20check%201/logs/stdio
>>
>> Thanks
>> Yvan
>>
>> On Mon, 9 Dec 2019 at 11:30, pre-merge checks [bot] via Phabricator
>> via cfe-commits  wrote:
>> >
>> > merge_guards_bot added a comment.
>> >
>> > Build result: pass - 60555 tests passed, 0 failed and 726 were skipped.
>> >
>> > Log files: console-log.txt 
>> > ,
>> >  CMakeCache.txt 
>> > 
>> >
>> >
>> > Repository:
>> >   rG LLVM Github Monorepo
>> >
>> > CHANGES SINCE LAST ACTION
>> >   https://reviews.llvm.org/D71186/new/
>> >
>> > https://reviews.llvm.org/D71186
>> >
>> >
>> >
>> > ___
>> > cfe-commits mailing list
>> > cfe-commits@lists.llvm.org
>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
>
> --
> Regards,
> Ilya Biryukov
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71403: [clangd] Fix hover crashing on null types

2019-12-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits

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

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71403

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -506,6 +506,24 @@
  HI.NamespaceScope = "";
  HI.Value = "&\"1234\"[0]";
}},
+  {R"cpp(// Should not crash
+template 
+struct Tmpl {
+  Tmpl(int name);
+};
+
+template 
+void boom(int name) {
+  new Tmpl([[na^me]]);
+})cpp",
+   [](HoverInfo &HI) {
+ HI.Name = "name";
+ HI.Definition = "int name";
+ HI.Kind = index::SymbolKind::Parameter;
+ HI.Type = "int";
+ HI.NamespaceScope = "";
+ HI.LocalScope = "boom::";
+   }},
   };
   for (const auto &Case : Cases) {
 SCOPED_TRACE(Case.Code);
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -242,12 +242,13 @@
   // FIXME: handle variadics.
 }
 
-llvm::Optional printExprValue(const Expr *E, const ASTContext 
&Ctx) {
+llvm::Optional printExprValue(const Expr *E,
+   const ASTContext &Ctx) {
   Expr::EvalResult Constant;
   // Evaluating [[foo]]() as "&foo" isn't useful, and prevents us walking up
   // to the enclosing call.
   QualType T = E->getType();
-  if (T->isFunctionType() || T->isFunctionPointerType() ||
+  if (T.isNull() || T->isFunctionType() || T->isFunctionPointerType() ||
   T->isFunctionReferenceType())
 return llvm::None;
   // Attempt to evaluate. If expr is dependent, evaluation crashes!


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -506,6 +506,24 @@
  HI.NamespaceScope = "";
  HI.Value = "&\"1234\"[0]";
}},
+  {R"cpp(// Should not crash
+template 
+struct Tmpl {
+  Tmpl(int name);
+};
+
+template 
+void boom(int name) {
+  new Tmpl([[na^me]]);
+})cpp",
+   [](HoverInfo &HI) {
+ HI.Name = "name";
+ HI.Definition = "int name";
+ HI.Kind = index::SymbolKind::Parameter;
+ HI.Type = "int";
+ HI.NamespaceScope = "";
+ HI.LocalScope = "boom::";
+   }},
   };
   for (const auto &Case : Cases) {
 SCOPED_TRACE(Case.Code);
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -242,12 +242,13 @@
   // FIXME: handle variadics.
 }
 
-llvm::Optional printExprValue(const Expr *E, const ASTContext &Ctx) {
+llvm::Optional printExprValue(const Expr *E,
+   const ASTContext &Ctx) {
   Expr::EvalResult Constant;
   // Evaluating [[foo]]() as "&foo" isn't useful, and prevents us walking up
   // to the enclosing call.
   QualType T = E->getType();
-  if (T->isFunctionType() || T->isFunctionPointerType() ||
+  if (T.isNull() || T->isFunctionType() || T->isFunctionPointerType() ||
   T->isFunctionReferenceType())
 return llvm::None;
   // Attempt to evaluate. If expr is dependent, evaluation crashes!
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71356: [Tooling/Syntax] Helpers to find spelled tokens touching a location.

2019-12-12 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added inline comments.



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:260
+  All, [&](const syntax::Token &Tok) { return Tok.location() < Loc; });
+  bool AcceptRight = Right != All.end() && !(Loc < Right->location());
+  bool AcceptLeft = Right != All.begin() && !((Right - 1)->endLocation() < 
Loc);

NIT: why not `Right->location() <= Loc` instead of `!(Loc < Right->location)`?
Do we only overload `operator <` for `SourceLocations`?



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:261
+  bool AcceptRight = Right != All.end() && !(Loc < Right->location());
+  bool AcceptLeft = Right != All.begin() && !((Right - 1)->endLocation() < 
Loc);
+  return llvm::makeArrayRef(Right - (AcceptLeft ? 1 : 0),

Maybe compare offsets instead of locations?
It might be more code, but it would read much cleaner.

It always feels weird to compare source locations for anything other than 
storing them in `std::map`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71356



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

Re: [PATCH] D71186: Reland "[AST] Traverse the class type loc inside the member type loc.""

2019-12-12 Thread Ilya Biryukov via cfe-commits

Will fix right away, sorry for the delay.

On Thu, Dec 12, 2019 at 12:42 PM Yvan Roux  wrote:

> On Tue, 10 Dec 2019 at 10:24, Ilya Biryukov  wrote:
> >
> > Ah, some older gcc versions can't handle raw string literals inside
> macro arguments.
> > +Haojian Wu, could you fix this?
>
> When do you plan to fix this, bots are now broken for a while... (I
> don't have my SVN/Github access in place to revert or fix it myself)
>
> > On Tue, Dec 10, 2019 at 12:22 PM Yvan Roux  wrote:
> >>
> >> Hi Haojian,
> >>
> >> AArch64 bots are broken after this commit, logs are available here:
> >>
> >>
> http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/21125/steps/ninja%20check%201/logs/stdio
> >>
> >> Thanks
> >> Yvan
> >>
> >> On Mon, 9 Dec 2019 at 11:30, pre-merge checks [bot] via Phabricator
> >> via cfe-commits  wrote:
> >> >
> >> > merge_guards_bot added a comment.
> >> >
> >> > Build result: pass - 60555 tests passed, 0 failed and 726 were
> skipped.
> >> >
> >> > Log files: console-log.txt <
> http://results.llvm-merge-guard.org/amd64_debian_testing_clang8-368/console-log.txt>,
> CMakeCache.txt <
> http://results.llvm-merge-guard.org/amd64_debian_testing_clang8-368/CMakeCache.txt
> >
> >> >
> >> >
> >> > Repository:
> >> >   rG LLVM Github Monorepo
> >> >
> >> > CHANGES SINCE LAST ACTION
> >> >   https://reviews.llvm.org/D71186/new/
> >> >
> >> > https://reviews.llvm.org/D71186
> >> >
> >> >
> >> >
> >> > ___
> >> > cfe-commits mailing list
> >> > cfe-commits@lists.llvm.org
> >> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >
> >
> >
> > --
> > Regards,
> > Ilya Biryukov
>


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

[clang] 7d77898 - [Tooling] Move raw string literal out of a macro call. NFC

2019-12-12 Thread Ilya Biryukov via cfe-commits


Author: Ilya Biryukov
Date: 2019-12-12T10:53:20+01:00
New Revision: 7d7789899f4d4684dac51f265a47b049db4d09f2

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

LOG: [Tooling] Move raw string literal out of a macro call. NFC

Should fix buildbots with some older gcc versions.

Added: 


Modified: 
clang/unittests/Tooling/RecursiveASTVisitorTests/MemberPointerTypeLoc.cpp

Removed: 




diff  --git 
a/clang/unittests/Tooling/RecursiveASTVisitorTests/MemberPointerTypeLoc.cpp 
b/clang/unittests/Tooling/RecursiveASTVisitorTests/MemberPointerTypeLoc.cpp
index d30a6ad22f07..d67bd0395a67 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/MemberPointerTypeLoc.cpp
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/MemberPointerTypeLoc.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "TestVisitor.h"
+#include "llvm/ADT/StringRef.h"
 
 using namespace clang;
 
@@ -33,7 +34,7 @@ TEST(RecursiveASTVisitor, VisitTypeLocInMemberPointerTypeLoc) 
{
   MemberPointerTypeLocVisitor Visitor;
   Visitor.ExpectMatch("Bar", 4, 36);
   Visitor.ExpectMatch("T", 7, 23);
-  EXPECT_TRUE(Visitor.runOver(R"cpp(
+  llvm::StringLiteral Code = R"cpp(
  class Bar { void func(int); };
  class Foo {
void bind(const char*, void(Bar::*Foo)(int)) {}
@@ -41,15 +42,17 @@ TEST(RecursiveASTVisitor, 
VisitTypeLocInMemberPointerTypeLoc) {
template
void test(void(T::*Foo)());
  };
-  )cpp"));
+  )cpp";
+  EXPECT_TRUE(Visitor.runOver(Code));
 }
 
 TEST(RecursiveASTVisitor, NoCrash) {
   MemberPointerTypeLocVisitor Visitor;
-  EXPECT_FALSE(Visitor.runOver(R"cpp(
+  llvm::StringLiteral Code = R"cpp(
  // MemberPointerTypeLoc.getClassTInfo() is null.
  class a(b(a::*)) class
-  )cpp"));
+  )cpp";
+  EXPECT_FALSE(Visitor.runOver(Code));
 }
 
 } // end anonymous namespace



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

Re: [PATCH] D71186: Reland "[AST] Traverse the class type loc inside the member type loc.""

2019-12-12 Thread Ilya Biryukov via cfe-commits

Should be fixed in 7d7789899f4.

On Thu, Dec 12, 2019 at 12:50 PM Ilya Biryukov  wrote:

> Will fix right away, sorry for the delay.
>
> On Thu, Dec 12, 2019 at 12:42 PM Yvan Roux  wrote:
>
>> On Tue, 10 Dec 2019 at 10:24, Ilya Biryukov  wrote:
>> >
>> > Ah, some older gcc versions can't handle raw string literals inside
>> macro arguments.
>> > +Haojian Wu, could you fix this?
>>
>> When do you plan to fix this, bots are now broken for a while... (I
>> don't have my SVN/Github access in place to revert or fix it myself)
>>
>> > On Tue, Dec 10, 2019 at 12:22 PM Yvan Roux 
>> wrote:
>> >>
>> >> Hi Haojian,
>> >>
>> >> AArch64 bots are broken after this commit, logs are available here:
>> >>
>> >>
>> http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/21125/steps/ninja%20check%201/logs/stdio
>> >>
>> >> Thanks
>> >> Yvan
>> >>
>> >> On Mon, 9 Dec 2019 at 11:30, pre-merge checks [bot] via Phabricator
>> >> via cfe-commits  wrote:
>> >> >
>> >> > merge_guards_bot added a comment.
>> >> >
>> >> > Build result: pass - 60555 tests passed, 0 failed and 726 were
>> skipped.
>> >> >
>> >> > Log files: console-log.txt <
>> http://results.llvm-merge-guard.org/amd64_debian_testing_clang8-368/console-log.txt>,
>> CMakeCache.txt <
>> http://results.llvm-merge-guard.org/amd64_debian_testing_clang8-368/CMakeCache.txt
>> >
>> >> >
>> >> >
>> >> > Repository:
>> >> >   rG LLVM Github Monorepo
>> >> >
>> >> > CHANGES SINCE LAST ACTION
>> >> >   https://reviews.llvm.org/D71186/new/
>> >> >
>> >> > https://reviews.llvm.org/D71186
>> >> >
>> >> >
>> >> >
>> >> > ___
>> >> > cfe-commits mailing list
>> >> > cfe-commits@lists.llvm.org
>> >> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>> >
>> >
>> >
>> > --
>> > Regards,
>> > Ilya Biryukov
>>
>
>
> --
> Regards,
> Ilya Biryukov
>


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

[PATCH] D71403: [clangd] Fix hover crashing on null types

2019-12-12 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

Wow, `ParenListExpr` is a really weird construct...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71403



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

[PATCH] D71403: [clangd] Fix hover crashing on null types

2019-12-12 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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71403



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

[PATCH] D71403: [clangd] Fix hover crashing on null types

2019-12-12 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/Hover.cpp:251
   QualType T = E->getType();
-  if (T->isFunctionType() || T->isFunctionPointerType() ||
+  if (T.isNull() || T->isFunctionType() || T->isFunctionPointerType() ||
   T->isFunctionReferenceType())

NIT: Replace `T.isNull()` with `!T`



Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:517
+void boom(int name) {
+  new Tmpl([[na^me]]);
+})cpp",

Why is the type null here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71403



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

Re: [PATCH] D71186: Reland "[AST] Traverse the class type loc inside the member type loc.""

2019-12-12 Thread Yvan Roux via cfe-commits

On Thu, 12 Dec 2019 at 10:54, Ilya Biryukov  wrote:
>
> Should be fixed in 7d7789899f4.

Thanks a lot Ilya

> On Thu, Dec 12, 2019 at 12:50 PM Ilya Biryukov  wrote:
>>
>> Will fix right away, sorry for the delay.
>>
>> On Thu, Dec 12, 2019 at 12:42 PM Yvan Roux  wrote:
>>>
>>> On Tue, 10 Dec 2019 at 10:24, Ilya Biryukov  wrote:
>>> >
>>> > Ah, some older gcc versions can't handle raw string literals inside macro 
>>> > arguments.
>>> > +Haojian Wu, could you fix this?
>>>
>>> When do you plan to fix this, bots are now broken for a while... (I
>>> don't have my SVN/Github access in place to revert or fix it myself)
>>>
>>> > On Tue, Dec 10, 2019 at 12:22 PM Yvan Roux  wrote:
>>> >>
>>> >> Hi Haojian,
>>> >>
>>> >> AArch64 bots are broken after this commit, logs are available here:
>>> >>
>>> >> http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/21125/steps/ninja%20check%201/logs/stdio
>>> >>
>>> >> Thanks
>>> >> Yvan
>>> >>
>>> >> On Mon, 9 Dec 2019 at 11:30, pre-merge checks [bot] via Phabricator
>>> >> via cfe-commits  wrote:
>>> >> >
>>> >> > merge_guards_bot added a comment.
>>> >> >
>>> >> > Build result: pass - 60555 tests passed, 0 failed and 726 were skipped.
>>> >> >
>>> >> > Log files: console-log.txt 
>>> >> > ,
>>> >> >  CMakeCache.txt 
>>> >> > 
>>> >> >
>>> >> >
>>> >> > Repository:
>>> >> >   rG LLVM Github Monorepo
>>> >> >
>>> >> > CHANGES SINCE LAST ACTION
>>> >> >   https://reviews.llvm.org/D71186/new/
>>> >> >
>>> >> > https://reviews.llvm.org/D71186
>>> >> >
>>> >> >
>>> >> >
>>> >> > ___
>>> >> > cfe-commits mailing list
>>> >> > cfe-commits@lists.llvm.org
>>> >> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>> >
>>> >
>>> >
>>> > --
>>> > Regards,
>>> > Ilya Biryukov
>>
>>
>>
>> --
>> Regards,
>> Ilya Biryukov
>
>
>
> --
> Regards,
> Ilya Biryukov
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71403: [clangd] Fix hover crashing on null types

2019-12-12 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 60654 tests passed, 1 failed 
and 726 were skipped.

  failed: lld.ELF/linkerscript/provide-empty-section.s

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
console-log.txt 
,
 CMakeCache.txt 
,
 test-results.xml 
,
 diff.json 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71403



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

[clang] 5f62087 - [DataLayout] Fix occurrences that size and range of pointers are assumed to be the same.

2019-12-12 Thread Nicola Zaghen via cfe-commits


Author: Nicola Zaghen
Date: 2019-12-12T10:07:01Z
New Revision: 5f6208778ff92567c57d7c1e2e740c284d7e69a5

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

LOG: [DataLayout] Fix occurrences that size and range of pointers are assumed 
to be the same.

GEP index size can be specified in the DataLayout, introduced in D42123. 
However, there were still places
in which getIndexSizeInBits was used interchangeably with getPointerSizeInBits. 
This notably caused issues
with Instcombine's visitPtrToInt; but the unit tests was incorrect, so this 
remained undiscovered.

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

Patch by Joseph Faulls!

Added: 
llvm/test/Transforms/InstCombine/builtin-object-size-custom-dl.ll
llvm/test/Transforms/InstCombine/stdio-custom-dl.ll

Modified: 
clang/lib/CodeGen/CGExprScalar.cpp
llvm/include/llvm/Analysis/PtrUseVisitor.h
llvm/include/llvm/Analysis/Utils/Local.h
llvm/lib/Analysis/ConstantFolding.cpp
llvm/lib/Analysis/InlineCost.cpp
llvm/lib/Analysis/InstructionSimplify.cpp
llvm/lib/Analysis/Loads.cpp
llvm/lib/Analysis/MemoryBuiltins.cpp
llvm/lib/Analysis/ScalarEvolution.cpp
llvm/lib/Analysis/ScalarEvolutionExpander.cpp
llvm/lib/Analysis/ValueTracking.cpp
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
llvm/lib/IR/DataLayout.cpp
llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
llvm/lib/Transforms/Utils/Local.cpp
llvm/test/Transforms/InstCombine/gep-custom-dl.ll
llvm/test/Transforms/InstCombine/icmp-custom-dl.ll
llvm/test/Transforms/PhaseOrdering/scev-custom-dl.ll
llvm/test/Transforms/SimplifyCFG/switch_create-custom-dl.ll

Removed: 




diff  --git a/clang/lib/CodeGen/CGExprScalar.cpp 
b/clang/lib/CodeGen/CGExprScalar.cpp
index b1dcbc51f58e..b8846162508e 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -3264,10 +3264,10 @@ static Value *emitPointerArithmetic(CodeGenFunction 
&CGF,
expr->getRHS()))
 return CGF.Builder.CreateIntToPtr(index, pointer->getType());
 
-  if (width != DL.getTypeSizeInBits(PtrTy)) {
+  if (width != DL.getIndexTypeSizeInBits(PtrTy)) {
 // Zero-extend or sign-extend the pointer value according to
 // whether the index is signed or not.
-index = CGF.Builder.CreateIntCast(index, DL.getIntPtrType(PtrTy), isSigned,
+index = CGF.Builder.CreateIntCast(index, DL.getIndexType(PtrTy), isSigned,
   "idx.ext");
   }
 

diff  --git a/llvm/include/llvm/Analysis/PtrUseVisitor.h 
b/llvm/include/llvm/Analysis/PtrUseVisitor.h
index fbf04c841d30..05bca2226742 100644
--- a/llvm/include/llvm/Analysis/PtrUseVisitor.h
+++ b/llvm/include/llvm/Analysis/PtrUseVisitor.h
@@ -222,9 +222,9 @@ class PtrUseVisitor : protected InstVisitor,
 // offsets on this pointer.
 // FIXME: Support a vector of pointers.
 assert(I.getType()->isPointerTy());
-IntegerType *IntPtrTy = cast(DL.getIntPtrType(I.getType()));
+IntegerType *IntIdxTy = cast(DL.getIndexType(I.getType()));
 IsOffsetKnown = true;
-Offset = APInt(IntPtrTy->getBitWidth(), 0);
+Offset = APInt(IntIdxTy->getBitWidth(), 0);
 PI.reset();
 
 // Enqueue the uses of this pointer.

diff  --git a/llvm/include/llvm/Analysis/Utils/Local.h 
b/llvm/include/llvm/Analysis/Utils/Local.h
index 98b931f93451..ca505960cbeb 100644
--- a/llvm/include/llvm/Analysis/Utils/Local.h
+++ b/llvm/include/llvm/Analysis/Utils/Local.h
@@ -29,15 +29,15 @@ template 
 Value *EmitGEPOffset(IRBuilderTy *Builder, const DataLayout &DL, User *GEP,
  bool NoAssumptions = false) {
   GEPOperator *GEPOp = cast(GEP);
-  Type *IntPtrTy = DL.getIntPtrType(GEP->getType());
-  Value *Result = Constant::getNullValue(IntPtrTy);
+  Type *IntIdxTy = DL.getIndexType(GEP->getType());
+  Value *Result = Constant::getNullValue(IntIdxTy);
 
   // If the GEP is inbounds, we know that none of the addressing operations 
will
   // overflow in a signed sense.
   bool isInBounds = GEPOp->isInBounds() && !NoAssumptions;
 
   // Build a mask for high order bits.
-  unsigned IntPtrWidth = IntPtrTy->getScalarType()->getIntegerBitWidth();
+  unsigned IntPtrWidth = IntIdxTy->getScalarType()->getIntegerBitWidth();
   uint64_t PtrSizeMask =
   std::numeric_limits::max() >> (64 - IntPtrWidth);
 
@@ -56,17 +56,17 @@ Value *EmitGEPOffset(IRBuilderTy *Builder, const DataLayout 
&DL, User *GEP,
 Size = DL.getStructLayout(STy)->getElementOffset(OpValue);
 
 if (Size)
-  Result = Builder->CreateAdd(Result, ConstantInt::get(IntPtrTy, Size),
+  Re

[PATCH] D68328: Fix occurrences that size and range of pointers are assumed to be the same.

2019-12-12 Thread Nicola Zaghen via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5f6208778ff9: [DataLayout] Fix occurrences that size and 
range of pointers are assumed to be… (authored by Nicola).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68328?vs=231658&id=233541#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68328

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  llvm/include/llvm/Analysis/PtrUseVisitor.h
  llvm/include/llvm/Analysis/Utils/Local.h
  llvm/lib/Analysis/ConstantFolding.cpp
  llvm/lib/Analysis/InlineCost.cpp
  llvm/lib/Analysis/InstructionSimplify.cpp
  llvm/lib/Analysis/Loads.cpp
  llvm/lib/Analysis/MemoryBuiltins.cpp
  llvm/lib/Analysis/ScalarEvolution.cpp
  llvm/lib/Analysis/ScalarEvolutionExpander.cpp
  llvm/lib/Analysis/ValueTracking.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
  llvm/lib/IR/DataLayout.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
  llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
  llvm/lib/Transforms/Utils/Local.cpp
  llvm/test/Transforms/InstCombine/builtin-object-size-custom-dl.ll
  llvm/test/Transforms/InstCombine/gep-custom-dl.ll
  llvm/test/Transforms/InstCombine/icmp-custom-dl.ll
  llvm/test/Transforms/InstCombine/stdio-custom-dl.ll
  llvm/test/Transforms/PhaseOrdering/scev-custom-dl.ll
  llvm/test/Transforms/SimplifyCFG/switch_create-custom-dl.ll

Index: llvm/test/Transforms/SimplifyCFG/switch_create-custom-dl.ll
===
--- llvm/test/Transforms/SimplifyCFG/switch_create-custom-dl.ll
+++ llvm/test/Transforms/SimplifyCFG/switch_create-custom-dl.ll
@@ -33,10 +33,10 @@
 
 define void @test1_ptr(i32* %V) {
 ; CHECK-LABEL: @test1_ptr(
-; CHECK-NEXT:[[MAGICPTR:%.*]] = ptrtoint i32* [[V:%.*]] to i32
-; CHECK-NEXT:switch i32 [[MAGICPTR]], label [[F:%.*]] [
-; CHECK-NEXT:i32 17, label [[T:%.*]]
-; CHECK-NEXT:i32 4, label [[T]]
+; CHECK-NEXT:[[MAGICPTR:%.*]] = ptrtoint i32* [[V:%.*]] to i40
+; CHECK-NEXT:switch i40 [[MAGICPTR]], label [[F:%.*]] [
+; CHECK-NEXT:i40 17, label [[T:%.*]]
+; CHECK-NEXT:i40 4, label [[T]]
 ; CHECK-NEXT:]
 ; CHECK:   T:
 ; CHECK-NEXT:call void @foo1()
@@ -59,10 +59,10 @@
 
 define void @test1_ptr_as1(i32 addrspace(1)* %V) {
 ; CHECK-LABEL: @test1_ptr_as1(
-; CHECK-NEXT:[[MAGICPTR:%.*]] = ptrtoint i32 addrspace(1)* [[V:%.*]] to i32
-; CHECK-NEXT:switch i32 [[MAGICPTR]], label [[F:%.*]] [
-; CHECK-NEXT:i32 17, label [[T:%.*]]
-; CHECK-NEXT:i32 4, label [[T]]
+; CHECK-NEXT:[[MAGICPTR:%.*]] = ptrtoint i32 addrspace(1)* [[V:%.*]] to i40
+; CHECK-NEXT:switch i40 [[MAGICPTR]], label [[F:%.*]] [
+; CHECK-NEXT:i40 17, label [[T:%.*]]
+; CHECK-NEXT:i40 4, label [[T]]
 ; CHECK-NEXT:]
 ; CHECK:   T:
 ; CHECK-NEXT:call void @foo1()
Index: llvm/test/Transforms/PhaseOrdering/scev-custom-dl.ll
===
--- llvm/test/Transforms/PhaseOrdering/scev-custom-dl.ll
+++ llvm/test/Transforms/PhaseOrdering/scev-custom-dl.ll
@@ -65,3 +65,73 @@
 for.end:  ; preds = %for.cond
   ret void
 }
+
+@array = weak global [101 x i32] zeroinitializer, align 32		; <[100 x i32]*> [#uses=1]
+
+; CHECK: Loop %bb: backedge-taken count is 100
+
+define void @test_range_ref1a(i32 %x) {
+entry:
+	br label %bb
+
+bb:		; preds = %bb, %entry
+	%i.01.0 = phi i32 [ 100, %entry ], [ %tmp4, %bb ]		;  [#uses=2]
+	%tmp1 = getelementptr [101 x i32], [101 x i32]* @array, i32 0, i32 %i.01.0		;  [#uses=1]
+	store i32 %x, i32* %tmp1
+	%tmp4 = add i32 %i.01.0, -1		;  [#uses=2]
+	%tmp7 = icmp sgt i32 %tmp4, -1		;  [#uses=1]
+	br i1 %tmp7, label %bb, label %return
+
+return:		; preds = %bb
+	ret void
+}
+
+define i32 @test_loop_idiom_recogize(i32 %x, i32 %y, i32* %lam, i32* %alp) nounwind {
+bb1.thread:
+	br label %bb1
+
+bb1:		; preds = %bb1, %bb1.thread
+	%indvar = phi i32 [ 0, %bb1.thread ], [ %indvar.next, %bb1 ]		;  [#uses=4]
+	%i.0.reg2mem.0 = sub i32 255, %indvar		;  [#uses=2]
+	%0 = getelementptr i32, i32* %alp, i32 %i.0.reg2mem.0		;  [#uses=1]
+	%1 = load i32, i32* %0, align 4		;  [#uses=1]
+	%2 = getelementptr i32, i32* %lam, i32 %i.0.reg2mem.0		;  [#uses=1]
+	store i32 %1, i32* %2, align 4
+	%3 = sub i32 254, %indvar		;  [#uses=1]
+	%4 = icmp slt i32 %3, 0		;  [#uses=1]
+	%indvar.next = add i32 %indvar, 1		;  [#uses=1]
+	br i1 %4, label %bb2, label %bb1
+
+bb2:		; preds = %bb1
+	%tmp10 = mul i32 %indvar, %x		;  [#uses=1]
+	%z.0.reg2mem.0 = add i32 %tmp10, %y		;  [#uses=1]
+	%5 = add i32 %z.0.reg2mem.0, %x		;  [#uses=1]
+	ret i32 %5
+}
+
+declare void @use(i1)
+
+declare void @llvm.experimental.guard(i1, ...)
+
+; This tests getRangeRef acts as intended with different idx size.
+; CHECK: max backe

[PATCH] D71213: [Alignment][NFC] CreateMemSet use MaybeAlign

2019-12-12 Thread Guillaume Chatelet via Phabricator via cfe-commits

gchatelet added a comment.

@foad do you have any insights on how to go with the deprecation?
LLVM has this `LLVM_ATTRIBUTE_DEPRECATED` macro, it's convenient to get a 
warning but it only works when building without `-Wall`.

Bots and in-tree users have it set by default, it's fine as I'll be fixing 
those but how about out of tree users?
Another option would be to have a comment but then nobody will actually update 
the code before it breaks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71213



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

[PATCH] D71356: [Tooling/Syntax] Helpers to find spelled tokens touching a location.

2019-12-12 Thread Kirill Bobyrev via Phabricator via cfe-commits

kbobyrev added inline comments.



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:260
+  All, [&](const syntax::Token &Tok) { return Tok.location() < Loc; });
+  bool AcceptRight = Right != All.end() && !(Loc < Right->location());
+  bool AcceptLeft = Right != All.begin() && !((Right - 1)->endLocation() < 
Loc);

ilya-biryukov wrote:
> NIT: why not `Right->location() <= Loc` instead of `!(Loc < Right->location)`?
> Do we only overload `operator <` for `SourceLocations`?
Yes, we only have operator< :(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71356



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

[clang] 9c48c2f - [NFC] - Typo fix in test/CodeGenCXX/runtime-dllstorage.cpp

2019-12-12 Thread Gabor Buella via cfe-commits


Author: Gabor Buella
Date: 2019-12-12T11:26:54+01:00
New Revision: 9c48c2f9c477007234c5bdad0bc8c0969afa0724

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

LOG: [NFC] - Typo fix in test/CodeGenCXX/runtime-dllstorage.cpp

Reviewed By: Jim

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

Added: 


Modified: 
clang/test/CodeGenCXX/runtime-dllstorage.cpp

Removed: 




diff  --git a/clang/test/CodeGenCXX/runtime-dllstorage.cpp 
b/clang/test/CodeGenCXX/runtime-dllstorage.cpp
index c8692b7384be..864a1ab51d65 100644
--- a/clang/test/CodeGenCXX/runtime-dllstorage.cpp
+++ b/clang/test/CodeGenCXX/runtime-dllstorage.cpp
@@ -1,13 +1,13 @@
 // RUN: %clang_cc1 -triple i686-windows-msvc -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -emit-llvm -o - %s | FileCheck 
-allow-deprecated-dag-overlap %s -check-prefix CHECK-MS -check-prefix 
CHECK-MS-DYNAMIC
 // RUN: %clang_cc1 -triple i686-windows-msvc -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -flto-visibility-public-std 
-emit-llvm -o - %s | FileCheck -allow-deprecated-dag-overlap %s -check-prefix 
CHECK-MS -check-prefix CHECK-MS-STATIC
 
-// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -emit-llvm -o - %s | FileCheck 
-allow-deprecated-dag-overlap %s -check-prefix CHECK-IA -check-prefix 
CHECK-DYNAMIC-IA -check-prefix CHECK-DYNAMIC-NODECL-IA -check-prefix 
CHECK-DYANMIC-IA-CXA-ATEXIT
+// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -emit-llvm -o - %s | FileCheck 
-allow-deprecated-dag-overlap %s -check-prefix CHECK-IA -check-prefix 
CHECK-DYNAMIC-IA -check-prefix CHECK-DYNAMIC-NODECL-IA -check-prefix 
CHECK-DYNAMIC-IA-CXA-ATEXIT
 // RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -flto-visibility-public-std 
-emit-llvm -o - %s | FileCheck -allow-deprecated-dag-overlap %s -check-prefix 
CHECK-IA -check-prefix CHECK-STATIC-IA -check-prefix CHECK-STATIC-NODECL-IA 
-check-prefix CHECK-IA-STATIC-CXA-ATEXIT
-// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -DIMPORT_DECLARATIONS 
-emit-llvm -o - %s | FileCheck -allow-deprecated-dag-overlap %s -check-prefix 
CHECK-IA -check-prefix CHECK-DYNAMIC-IA -check-prefix CHECK-DYNAMIC-IMPORT-IA 
-check-prefix CHECK-DYANMIC-IA-CXA-ATEXIT
+// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -DIMPORT_DECLARATIONS 
-emit-llvm -o - %s | FileCheck -allow-deprecated-dag-overlap %s -check-prefix 
CHECK-IA -check-prefix CHECK-DYNAMIC-IA -check-prefix CHECK-DYNAMIC-IMPORT-IA 
-check-prefix CHECK-DYNAMIC-IA-CXA-ATEXIT
 // RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -flto-visibility-public-std 
-DIMPORT_DECLARATIONS -emit-llvm -o - %s | FileCheck 
-allow-deprecated-dag-overlap %s -check-prefix CHECK-IA -check-prefix 
CHECK-STATIC-IA -check-prefix CHECK-STATIC-IMPORT-IA -check-prefix 
CHECK-IA-STATIC-CXA-ATEXIT
-// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -DEXPORT_DECLARATIONS 
-emit-llvm -o - %s | FileCheck -allow-deprecated-dag-overlap %s -check-prefix 
CHECK-IA -check-prefix CHECK-DYNAMIC-IA -check-prefix 
CHECK-DYANMIC-IA-CXA-ATEXIT
+// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -DEXPORT_DECLARATIONS 
-emit-llvm -o - %s | FileCheck -allow-deprecated-dag-overlap %s -check-prefix 
CHECK-IA -check-prefix CHECK-DYNAMIC-IA -check-prefix 
CHECK-DYNAMIC-IA-CXA-ATEXIT
 // RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -flto-visibility-public-std 
-DEXPORT_DECLARATIONS -emit-llvm -o - %s | FileCheck 
-allow-deprecated-dag-overlap %s -check-prefix CHECK-IA -check-prefix 
CHECK-STATIC-IA -check-prefix CHECK-IA-STATIC-CXA-ATEXIT
-// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -DDECL -emit-llvm -o - %s | 
FileCheck -allow-deprecated-dag-overlap %s -check-prefix CHECK-IA -check-prefix 
CHECK-DYNAMIC-IA -check-prefix CHECK-DYNAMIC-DECL-IA -check-prefix 
CHECK-DYANMIC-IA-CXA-ATEXIT
+// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -DDECL -emit-llvm -o - %s | 
FileCheck -allow-deprecated-dag-overlap %s -check-prefix CHECK-IA -check-prefix 
CHECK-DYNAMIC-IA -check-prefix CHECK-DYNAMIC-DECL-IA -check-prefi

[PATCH] D48921: NFC - Typo fix in test/CodeGenCXX/runtime-dllstorage.cpp

2019-12-12 Thread Gabor Buella via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9c48c2f9c477: [NFC] - Typo fix in 
test/CodeGenCXX/runtime-dllstorage.cpp (authored by GBuella).

Changed prior to commit:
  https://reviews.llvm.org/D48921?vs=154067&id=233543#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D48921

Files:
  clang/test/CodeGenCXX/runtime-dllstorage.cpp


Index: clang/test/CodeGenCXX/runtime-dllstorage.cpp
===
--- clang/test/CodeGenCXX/runtime-dllstorage.cpp
+++ clang/test/CodeGenCXX/runtime-dllstorage.cpp
@@ -1,13 +1,13 @@
 // RUN: %clang_cc1 -triple i686-windows-msvc -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -emit-llvm -o - %s | FileCheck 
-allow-deprecated-dag-overlap %s -check-prefix CHECK-MS -check-prefix 
CHECK-MS-DYNAMIC
 // RUN: %clang_cc1 -triple i686-windows-msvc -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -flto-visibility-public-std 
-emit-llvm -o - %s | FileCheck -allow-deprecated-dag-overlap %s -check-prefix 
CHECK-MS -check-prefix CHECK-MS-STATIC
 
-// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -emit-llvm -o - %s | FileCheck 
-allow-deprecated-dag-overlap %s -check-prefix CHECK-IA -check-prefix 
CHECK-DYNAMIC-IA -check-prefix CHECK-DYNAMIC-NODECL-IA -check-prefix 
CHECK-DYANMIC-IA-CXA-ATEXIT
+// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -emit-llvm -o - %s | FileCheck 
-allow-deprecated-dag-overlap %s -check-prefix CHECK-IA -check-prefix 
CHECK-DYNAMIC-IA -check-prefix CHECK-DYNAMIC-NODECL-IA -check-prefix 
CHECK-DYNAMIC-IA-CXA-ATEXIT
 // RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -flto-visibility-public-std 
-emit-llvm -o - %s | FileCheck -allow-deprecated-dag-overlap %s -check-prefix 
CHECK-IA -check-prefix CHECK-STATIC-IA -check-prefix CHECK-STATIC-NODECL-IA 
-check-prefix CHECK-IA-STATIC-CXA-ATEXIT
-// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -DIMPORT_DECLARATIONS 
-emit-llvm -o - %s | FileCheck -allow-deprecated-dag-overlap %s -check-prefix 
CHECK-IA -check-prefix CHECK-DYNAMIC-IA -check-prefix CHECK-DYNAMIC-IMPORT-IA 
-check-prefix CHECK-DYANMIC-IA-CXA-ATEXIT
+// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -DIMPORT_DECLARATIONS 
-emit-llvm -o - %s | FileCheck -allow-deprecated-dag-overlap %s -check-prefix 
CHECK-IA -check-prefix CHECK-DYNAMIC-IA -check-prefix CHECK-DYNAMIC-IMPORT-IA 
-check-prefix CHECK-DYNAMIC-IA-CXA-ATEXIT
 // RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -flto-visibility-public-std 
-DIMPORT_DECLARATIONS -emit-llvm -o - %s | FileCheck 
-allow-deprecated-dag-overlap %s -check-prefix CHECK-IA -check-prefix 
CHECK-STATIC-IA -check-prefix CHECK-STATIC-IMPORT-IA -check-prefix 
CHECK-IA-STATIC-CXA-ATEXIT
-// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -DEXPORT_DECLARATIONS 
-emit-llvm -o - %s | FileCheck -allow-deprecated-dag-overlap %s -check-prefix 
CHECK-IA -check-prefix CHECK-DYNAMIC-IA -check-prefix 
CHECK-DYANMIC-IA-CXA-ATEXIT
+// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -DEXPORT_DECLARATIONS 
-emit-llvm -o - %s | FileCheck -allow-deprecated-dag-overlap %s -check-prefix 
CHECK-IA -check-prefix CHECK-DYNAMIC-IA -check-prefix 
CHECK-DYNAMIC-IA-CXA-ATEXIT
 // RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -flto-visibility-public-std 
-DEXPORT_DECLARATIONS -emit-llvm -o - %s | FileCheck 
-allow-deprecated-dag-overlap %s -check-prefix CHECK-IA -check-prefix 
CHECK-STATIC-IA -check-prefix CHECK-IA-STATIC-CXA-ATEXIT
-// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -DDECL -emit-llvm -o - %s | 
FileCheck -allow-deprecated-dag-overlap %s -check-prefix CHECK-IA -check-prefix 
CHECK-DYNAMIC-IA -check-prefix CHECK-DYNAMIC-DECL-IA -check-prefix 
CHECK-DYANMIC-IA-CXA-ATEXIT
+// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -DDECL -emit-llvm -o - %s | 
FileCheck -allow-deprecated-dag-overlap %s -check-prefix CHECK-IA -check-prefix 
CHECK-DYNAMIC-IA -check-prefix CHECK-DYNAMIC-DECL-IA -check-prefix 
CHECK-DYNAMIC-IA-CXA-ATEXIT
 // RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec 
-fms-compatibility -fexceptions -fcxx-exceptions -flto-visibility-publ

[clang] f798eb2 - Temporarily Revert "[DataLayout] Fix occurrences that size and range of pointers are assumed to be the same."

2019-12-12 Thread Nicola Zaghen via cfe-commits


Author: Nicola Zaghen
Date: 2019-12-12T10:29:54Z
New Revision: f798eb21eca97dc44ed40da52ece22780fb74230

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

LOG: Temporarily Revert "[DataLayout] Fix occurrences that size and range of 
pointers are assumed to be the same."

This reverts commit 5f6208778ff92567c57d7c1e2e740c284d7e69a5.

This caused failures in Transforms/PhaseOrdering/scev-custom-dl.ll
const: Assertion `getBitWidth() == CR.getBitWidth() && "ConstantRange types 
don't agree!"' failed.

Added: 


Modified: 
clang/lib/CodeGen/CGExprScalar.cpp
llvm/include/llvm/Analysis/PtrUseVisitor.h
llvm/include/llvm/Analysis/Utils/Local.h
llvm/lib/Analysis/ConstantFolding.cpp
llvm/lib/Analysis/InlineCost.cpp
llvm/lib/Analysis/InstructionSimplify.cpp
llvm/lib/Analysis/Loads.cpp
llvm/lib/Analysis/MemoryBuiltins.cpp
llvm/lib/Analysis/ScalarEvolution.cpp
llvm/lib/Analysis/ScalarEvolutionExpander.cpp
llvm/lib/Analysis/ValueTracking.cpp
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
llvm/lib/IR/DataLayout.cpp
llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
llvm/lib/Transforms/Utils/Local.cpp
llvm/test/Transforms/InstCombine/gep-custom-dl.ll
llvm/test/Transforms/InstCombine/icmp-custom-dl.ll
llvm/test/Transforms/PhaseOrdering/scev-custom-dl.ll
llvm/test/Transforms/SimplifyCFG/switch_create-custom-dl.ll

Removed: 
llvm/test/Transforms/InstCombine/builtin-object-size-custom-dl.ll
llvm/test/Transforms/InstCombine/stdio-custom-dl.ll



diff  --git a/clang/lib/CodeGen/CGExprScalar.cpp 
b/clang/lib/CodeGen/CGExprScalar.cpp
index b8846162508e..b1dcbc51f58e 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -3264,10 +3264,10 @@ static Value *emitPointerArithmetic(CodeGenFunction 
&CGF,
expr->getRHS()))
 return CGF.Builder.CreateIntToPtr(index, pointer->getType());
 
-  if (width != DL.getIndexTypeSizeInBits(PtrTy)) {
+  if (width != DL.getTypeSizeInBits(PtrTy)) {
 // Zero-extend or sign-extend the pointer value according to
 // whether the index is signed or not.
-index = CGF.Builder.CreateIntCast(index, DL.getIndexType(PtrTy), isSigned,
+index = CGF.Builder.CreateIntCast(index, DL.getIntPtrType(PtrTy), isSigned,
   "idx.ext");
   }
 

diff  --git a/llvm/include/llvm/Analysis/PtrUseVisitor.h 
b/llvm/include/llvm/Analysis/PtrUseVisitor.h
index 05bca2226742..fbf04c841d30 100644
--- a/llvm/include/llvm/Analysis/PtrUseVisitor.h
+++ b/llvm/include/llvm/Analysis/PtrUseVisitor.h
@@ -222,9 +222,9 @@ class PtrUseVisitor : protected InstVisitor,
 // offsets on this pointer.
 // FIXME: Support a vector of pointers.
 assert(I.getType()->isPointerTy());
-IntegerType *IntIdxTy = cast(DL.getIndexType(I.getType()));
+IntegerType *IntPtrTy = cast(DL.getIntPtrType(I.getType()));
 IsOffsetKnown = true;
-Offset = APInt(IntIdxTy->getBitWidth(), 0);
+Offset = APInt(IntPtrTy->getBitWidth(), 0);
 PI.reset();
 
 // Enqueue the uses of this pointer.

diff  --git a/llvm/include/llvm/Analysis/Utils/Local.h 
b/llvm/include/llvm/Analysis/Utils/Local.h
index ca505960cbeb..98b931f93451 100644
--- a/llvm/include/llvm/Analysis/Utils/Local.h
+++ b/llvm/include/llvm/Analysis/Utils/Local.h
@@ -29,15 +29,15 @@ template 
 Value *EmitGEPOffset(IRBuilderTy *Builder, const DataLayout &DL, User *GEP,
  bool NoAssumptions = false) {
   GEPOperator *GEPOp = cast(GEP);
-  Type *IntIdxTy = DL.getIndexType(GEP->getType());
-  Value *Result = Constant::getNullValue(IntIdxTy);
+  Type *IntPtrTy = DL.getIntPtrType(GEP->getType());
+  Value *Result = Constant::getNullValue(IntPtrTy);
 
   // If the GEP is inbounds, we know that none of the addressing operations 
will
   // overflow in a signed sense.
   bool isInBounds = GEPOp->isInBounds() && !NoAssumptions;
 
   // Build a mask for high order bits.
-  unsigned IntPtrWidth = IntIdxTy->getScalarType()->getIntegerBitWidth();
+  unsigned IntPtrWidth = IntPtrTy->getScalarType()->getIntegerBitWidth();
   uint64_t PtrSizeMask =
   std::numeric_limits::max() >> (64 - IntPtrWidth);
 
@@ -56,17 +56,17 @@ Value *EmitGEPOffset(IRBuilderTy *Builder, const DataLayout 
&DL, User *GEP,
 Size = DL.getStructLayout(STy)->getElementOffset(OpValue);
 
 if (Size)
-  Result = Builder->CreateAdd(Result, ConstantInt::get(IntIdxTy, Size),
+  Result = Builder->CreateAdd(Result, ConstantInt::get(IntPtrTy, Size),
   GEP->getName()+".offs");
 cont

[PATCH] D71193: [clang] Turn -fno-builtin flag into an IR Attribute

2019-12-12 Thread Guillaume Chatelet via Phabricator via cfe-commits

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



Comment at: clang/test/CodeGen/libcalls-fno-builtin.c:163
 
-// CHECK: [[ATTR]] = { nobuiltin }
+// GLOBAL: #2 = { nobuiltin "no-builtins" }
+// INDIVIDUAL: #2 = { nobuiltin "no-builtin-ceil" "no-builtin-copysign" 
"no-builtin-cos" "no-builtin-fabs" "no-builtin-floor" "no-builtin-fopen" 
"no-builtin-fread" "no-builtin-fwrite" "no-builtin-stpcpy" "no-builtin-strcat" 
"no-builtin-strchr" "no-builtin-strcmp" "no-builtin-strcpy" "no-builtin-strlen" 
"no-builtin-strncat" "no-builtin-strncmp" "no-builtin-strncpy" 
"no-builtin-strpbrk" "no-builtin-strrchr" "no-builtin-strspn" 
"no-builtin-strtod" "no-builtin-strtof" "no-builtin-strtol" 
"no-builtin-strtold" "no-builtin-strtoll" "no-builtin-strtoul" 
"no-builtin-strtoull" }

tejohnson wrote:
> Orthogonal to this patch:
> 
> Looks like there are 2 nobuiltin attributes now? AFAICT the old "nobuiltin" 
> gets applied to any and all cases where either -fno-builtin or 
> -fno-builtin-{name} applied. Is it obviated by the new attributes?
> 
> Also, why are the new ones quoted and the old one not?
Here is my understanding so far:
There are two systems for attributes.
 - In the original design attributes were boolean only and they would be packed 
in 64 bit integer. `nobuiltin` is one of these.
 - Then the attribute system got extended to allow for more than 64 entries and 
also allow for types. Attributes are now key/value pairs where value can be 
bool, int or string. `"nobuiltins"` is a such boolean attribute (note the 
trailing `s`)

Now I wanted to create a different attribute than the original `nobuiltin` 
because semantic might be different.

Obviously this is not ideal.
There are two options AFAICT:
 - conflate the two and reuse the original one to get a mode compact 
representation
 - remove the original one and release one bit for other important attribute to 
use.

For the individual ones the list is open ended so there's no way we can 
retrofit it into the previous scheme.

@efriedma, @hfinkel do you have any suggestions or a better approach here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71193



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

[PATCH] D68328: Fix occurrences that size and range of pointers are assumed to be the same.

2019-12-12 Thread Nicola Zaghen via Phabricator via cfe-commits

Nicola added a comment.

Reverted in f798eb21eca97dc44ed40da52ece22780fb74230 
 as it was 
causing failures in 
http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-debian/builds/443/steps/test-check-all/logs/stdio
 and other bots.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68328



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

[PATCH] D71403: [clangd] Fix hover crashing on null types

2019-12-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits

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



Comment at: clang-tools-extra/clangd/Hover.cpp:251
   QualType T = E->getType();
-  if (T->isFunctionType() || T->isFunctionPointerType() ||
+  if (T.isNull() || T->isFunctionType() || T->isFunctionPointerType() ||
   T->isFunctionReferenceType())

ilya-biryukov wrote:
> NIT: Replace `T.isNull()` with `!T`
I don't think `!` is defined for `QualType`.



Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:517
+void boom(int name) {
+  new Tmpl([[na^me]]);
+})cpp",

ilya-biryukov wrote:
> Why is the type null here?
because parentlistexprs, as you've noticed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71403



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

[clang-tools-extra] 75b04c7 - [clangd] Fix hover crashing on null types

2019-12-12 Thread Kadir Cetinkaya via cfe-commits


Author: Kadir Cetinkaya
Date: 2019-12-12T11:40:56+01:00
New Revision: 75b04c7af9e7c48e0128a602f2edb17272e3bfaa

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

LOG: [clangd] Fix hover crashing on null types

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

Reviewers: sammccall, ilya-biryukov

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

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Hover.cpp 
b/clang-tools-extra/clangd/Hover.cpp
index 5bc15629b05b..d89dc298ed85 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -242,12 +242,13 @@ void fillFunctionTypeAndParams(HoverInfo &HI, const Decl 
*D,
   // FIXME: handle variadics.
 }
 
-llvm::Optional printExprValue(const Expr *E, const ASTContext 
&Ctx) {
+llvm::Optional printExprValue(const Expr *E,
+   const ASTContext &Ctx) {
   Expr::EvalResult Constant;
   // Evaluating [[foo]]() as "&foo" isn't useful, and prevents us walking up
   // to the enclosing call.
   QualType T = E->getType();
-  if (T->isFunctionType() || T->isFunctionPointerType() ||
+  if (T.isNull() || T->isFunctionType() || T->isFunctionPointerType() ||
   T->isFunctionReferenceType())
 return llvm::None;
   // Attempt to evaluate. If expr is dependent, evaluation crashes!

diff  --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp 
b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 5862b1d03bf4..3cde2948e369 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -506,6 +506,24 @@ void foo())cpp";
  HI.NamespaceScope = "";
  HI.Value = "&\"1234\"[0]";
}},
+  {R"cpp(// Should not crash
+template 
+struct Tmpl {
+  Tmpl(int name);
+};
+
+template 
+void boom(int name) {
+  new Tmpl([[na^me]]);
+})cpp",
+   [](HoverInfo &HI) {
+ HI.Name = "name";
+ HI.Definition = "int name";
+ HI.Kind = index::SymbolKind::Parameter;
+ HI.Type = "int";
+ HI.NamespaceScope = "";
+ HI.LocalScope = "boom::";
+   }},
   };
   for (const auto &Case : Cases) {
 SCOPED_TRACE(Case.Code);



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

[PATCH] D71403: [clangd] Fix hover crashing on null types

2019-12-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
kadircet marked an inline comment as done.
Closed by commit rG75b04c7af9e7: [clangd] Fix hover crashing on null types 
(authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71403

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -506,6 +506,24 @@
  HI.NamespaceScope = "";
  HI.Value = "&\"1234\"[0]";
}},
+  {R"cpp(// Should not crash
+template 
+struct Tmpl {
+  Tmpl(int name);
+};
+
+template 
+void boom(int name) {
+  new Tmpl([[na^me]]);
+})cpp",
+   [](HoverInfo &HI) {
+ HI.Name = "name";
+ HI.Definition = "int name";
+ HI.Kind = index::SymbolKind::Parameter;
+ HI.Type = "int";
+ HI.NamespaceScope = "";
+ HI.LocalScope = "boom::";
+   }},
   };
   for (const auto &Case : Cases) {
 SCOPED_TRACE(Case.Code);
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -242,12 +242,13 @@
   // FIXME: handle variadics.
 }
 
-llvm::Optional printExprValue(const Expr *E, const ASTContext 
&Ctx) {
+llvm::Optional printExprValue(const Expr *E,
+   const ASTContext &Ctx) {
   Expr::EvalResult Constant;
   // Evaluating [[foo]]() as "&foo" isn't useful, and prevents us walking up
   // to the enclosing call.
   QualType T = E->getType();
-  if (T->isFunctionType() || T->isFunctionPointerType() ||
+  if (T.isNull() || T->isFunctionType() || T->isFunctionPointerType() ||
   T->isFunctionReferenceType())
 return llvm::None;
   // Attempt to evaluate. If expr is dependent, evaluation crashes!


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -506,6 +506,24 @@
  HI.NamespaceScope = "";
  HI.Value = "&\"1234\"[0]";
}},
+  {R"cpp(// Should not crash
+template 
+struct Tmpl {
+  Tmpl(int name);
+};
+
+template 
+void boom(int name) {
+  new Tmpl([[na^me]]);
+})cpp",
+   [](HoverInfo &HI) {
+ HI.Name = "name";
+ HI.Definition = "int name";
+ HI.Kind = index::SymbolKind::Parameter;
+ HI.Type = "int";
+ HI.NamespaceScope = "";
+ HI.LocalScope = "boom::";
+   }},
   };
   for (const auto &Case : Cases) {
 SCOPED_TRACE(Case.Code);
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -242,12 +242,13 @@
   // FIXME: handle variadics.
 }
 
-llvm::Optional printExprValue(const Expr *E, const ASTContext &Ctx) {
+llvm::Optional printExprValue(const Expr *E,
+   const ASTContext &Ctx) {
   Expr::EvalResult Constant;
   // Evaluating [[foo]]() as "&foo" isn't useful, and prevents us walking up
   // to the enclosing call.
   QualType T = E->getType();
-  if (T->isFunctionType() || T->isFunctionPointerType() ||
+  if (T.isNull() || T->isFunctionType() || T->isFunctionPointerType() ||
   T->isFunctionReferenceType())
 return llvm::None;
   // Attempt to evaluate. If expr is dependent, evaluation crashes!
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68213: [LTO] Support for embedding bitcode section during LTO

2019-12-12 Thread Josef Eisl via Phabricator via cfe-commits

zapster updated this revision to Diff 233548.
zapster added a comment.

Addressed suggestions from @tejohnson


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

https://reviews.llvm.org/D68213

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/Frontend/x86-embed-bitcode.ll
  llvm/include/llvm/Bitcode/BitcodeWriter.h
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/LTO/LTOBackend.cpp
  llvm/test/LTO/X86/Inputs/start-lib1.ll
  llvm/test/LTO/X86/Inputs/start-lib2.ll
  llvm/test/LTO/X86/embed-bitcode.ll

Index: llvm/test/LTO/X86/embed-bitcode.ll
===
--- /dev/null
+++ llvm/test/LTO/X86/embed-bitcode.ll
@@ -0,0 +1,28 @@
+; RUN: llvm-as %s -o %t1.o
+; RUN: llvm-as %p/Inputs/start-lib1.ll -o %t2.o
+; RUN: llvm-as %p/Inputs/start-lib2.ll -o %t3.o
+
+; RUN: llvm-lto2 run -r %t1.o,_start,px -r %t2.o,foo,px -r %t3.o,bar,px -r %t2.o,bar,px -o %t3 %t1.o %t2.o %t3.o
+; RUN: llvm-readelf -S %t3.0 | FileCheck %s --implicit-check-not=.llvmbc
+
+; RUN: llvm-lto2 run -r %t1.o,_start,px -r %t2.o,foo,px -r %t3.o,bar,px -r %t2.o,bar,px -lto-embed-bitcode=false -o %t3 %t1.o %t2.o %t3.o
+; RUN: llvm-readelf -S %t3.0 | FileCheck %s --implicit-check-not=.llvmbc
+
+; RUN: llvm-lto2 run -r %t1.o,_start,px -r %t2.o,foo,px -r %t3.o,bar,px -r %t2.o,bar,px -lto-embed-bitcode -o %t3 %t1.o %t2.o %t3.o
+; RUN: llvm-readelf -S %t3.0 | FileCheck %s --check-prefix=CHECK-ELF
+; RUN: llvm-objcopy -O binary -j .llvmbc %t3.0 %t-embedded.bc
+; RUN: llvm-dis %t-embedded.bc -o - | FileCheck %s --check-prefix=CHECK-LL
+
+; CHECK-ELF: .text
+; CHECK-ELF: .llvmbc
+
+; CHECK-LL: @_start
+; CHECK-LL: @foo
+; CHECK-LL: @bar
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @_start() {
+  ret void
+}
Index: llvm/test/LTO/X86/Inputs/start-lib2.ll
===
--- /dev/null
+++ llvm/test/LTO/X86/Inputs/start-lib2.ll
@@ -0,0 +1,6 @@
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @bar() {
+  ret void
+}
Index: llvm/test/LTO/X86/Inputs/start-lib1.ll
===
--- /dev/null
+++ llvm/test/LTO/X86/Inputs/start-lib1.ll
@@ -0,0 +1,8 @@
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @bar()
+
+define void @foo() {
+  ret void
+}
Index: llvm/lib/LTO/LTOBackend.cpp
===
--- llvm/lib/LTO/LTOBackend.cpp
+++ llvm/lib/LTO/LTOBackend.cpp
@@ -34,6 +34,7 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
+#include "llvm/Support/SmallVectorMemoryBuffer.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/raw_ostream.h"
@@ -312,11 +313,30 @@
   return !Conf.PostOptModuleHook || Conf.PostOptModuleHook(Task, Mod);
 }
 
+static cl::opt EmbedBitcode(
+"lto-embed-bitcode", cl::init(false),
+cl::desc("Embed LLVM bitcode in object files produced by LTO"));
+
+static void EmitBitcodeSection(Module &M, Config &Conf) {
+  if (!EmbedBitcode)
+return;
+  SmallVector Buffer;
+  raw_svector_ostream OS(Buffer);
+  WriteBitcodeToFile(M, OS);
+
+  std::unique_ptr Buf(
+  new SmallVectorMemoryBuffer(std::move(Buffer)));
+  llvm::EmbedBitcodeInModule(M, Buf->getMemBufferRef(), /*EmbedBitcode*/ true,
+ /*EmbedMarker*/ false, /*CmdArgs*/ nullptr);
+}
+
 void codegen(Config &Conf, TargetMachine *TM, AddStreamFn AddStream,
  unsigned Task, Module &Mod) {
   if (Conf.PreCodeGenModuleHook && !Conf.PreCodeGenModuleHook(Task, Mod))
 return;
 
+  EmitBitcodeSection(Mod, Conf);
+
   std::unique_ptr DwoOut;
   SmallString<1024> DwoFile(Conf.SplitDwarfOutput);
   if (!Conf.DwoDir.empty()) {
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -24,9 +24,10 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Bitcode/BitcodeReader.h"
+#include "llvm/Bitcode/LLVMBitCodes.h"
 #include "llvm/Bitstream/BitCodes.h"
 #include "llvm/Bitstream/BitstreamWriter.h"
-#include "llvm/Bitcode/LLVMBitCodes.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
@@ -4666,3 +4667,125 @@
 
   Out.write((char *)&Buffer.front(), Buffer.size());
 }
+
+static const char *getSectionNameForBitcode(const Triple &T) {
+  switch (T.getObjectFormat()) {
+  case Triple::MachO:
+return "__LLVM,__bitcode";
+  case Triple::COFF:
+  case Triple::ELF:
+  case Triple::Wasm:
+  case Triple::UnknownObjectFormat:
+return ".llvmb

[PATCH] D71356: [Tooling/Syntax] Helpers to find spelled tokens touching a location.

2019-12-12 Thread Sam McCall via Phabricator via cfe-commits

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

Add overloads for SourceLocation comparison, and document when they're 
meaningful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71356

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

Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- clang/unittests/Tooling/Syntax/TokensTest.cpp
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -793,4 +793,45 @@
 ActualMacroRanges.push_back(Expansion->range(SM));
   EXPECT_EQ(ExpectedMacroRanges, ActualMacroRanges);
 }
+
+TEST_F(TokenBufferTest, Touching) {
+  llvm::Annotations Code("^i^nt^ ^a^b^=^1;^");
+  recordTokens(Code.code());
+
+  auto Touching = [&](int Index) {
+SourceLocation Loc = SourceMgr->getComposedLoc(SourceMgr->getMainFileID(),
+   Code.points()[Index]);
+return spelledTokensTouching(Loc, Buffer);
+  };
+  auto Identifier = [&](int Index) {
+SourceLocation Loc = SourceMgr->getComposedLoc(SourceMgr->getMainFileID(),
+   Code.points()[Index]);
+const syntax::Token *Tok = spelledIdentifierTouching(Loc, Buffer);
+return Tok ? Tok->text(*SourceMgr) : "";
+  };
+
+  EXPECT_THAT(Touching(0), SameRange(findSpelled("int")));
+  EXPECT_EQ(Identifier(0), "");
+  EXPECT_THAT(Touching(1), SameRange(findSpelled("int")));
+  EXPECT_EQ(Identifier(1), "");
+  EXPECT_THAT(Touching(2), SameRange(findSpelled("int")));
+  EXPECT_EQ(Identifier(2), "");
+
+  EXPECT_THAT(Touching(3), SameRange(findSpelled("ab")));
+  EXPECT_EQ(Identifier(3), "ab");
+  EXPECT_THAT(Touching(4), SameRange(findSpelled("ab")));
+  EXPECT_EQ(Identifier(4), "ab");
+
+  EXPECT_THAT(Touching(5), SameRange(findSpelled("ab =")));
+  EXPECT_EQ(Identifier(5), "ab");
+
+  EXPECT_THAT(Touching(6), SameRange(findSpelled("= 1")));
+  EXPECT_EQ(Identifier(6), "");
+
+  EXPECT_THAT(Touching(7), SameRange(findSpelled(";")));
+  EXPECT_EQ(Identifier(7), "");
+
+  ASSERT_EQ(Code.points().size(), 8u);
+}
+
 } // namespace
Index: clang/lib/Tooling/Syntax/Tokens.cpp
===
--- clang/lib/Tooling/Syntax/Tokens.cpp
+++ clang/lib/Tooling/Syntax/Tokens.cpp
@@ -248,6 +248,30 @@
   return E;
 }
 
+llvm::ArrayRef
+syntax::spelledTokensTouching(SourceLocation Loc,
+  const syntax::TokenBuffer &Tokens) {
+  assert(Loc.isFileID());
+  llvm::ArrayRef All =
+  Tokens.spelledTokens(Tokens.sourceManager().getFileID(Loc));
+  // Comparing SourceLocations is well-defined within a FileID.
+  auto *Right = llvm::partition_point(
+  All, [&](const syntax::Token &Tok) { return Tok.location() < Loc; });
+  bool AcceptRight = Right != All.end() && Right->location() <= Loc;
+  bool AcceptLeft = Right != All.begin() && (Right - 1)->endLocation() >= Loc;
+  return llvm::makeArrayRef(Right - (AcceptLeft ? 1 : 0),
+Right + (AcceptRight ? 1 : 0));
+}
+
+const syntax::Token *
+syntax::spelledIdentifierTouching(SourceLocation Loc,
+  const syntax::TokenBuffer &Tokens) {
+  for (const syntax::Token &Tok : spelledTokensTouching(Loc, Tokens))
+if (Tok.kind() == tok::identifier)
+  return &Tok;
+  return nullptr;
+}
+
 std::vector
 TokenBuffer::macroExpansions(FileID FID) const {
   auto FileIt = Files.find(FID);
Index: clang/include/clang/Tooling/Syntax/Tokens.h
===
--- clang/include/clang/Tooling/Syntax/Tokens.h
+++ clang/include/clang/Tooling/Syntax/Tokens.h
@@ -309,6 +309,17 @@
   const SourceManager *SourceMgr;
 };
 
+/// The spelled tokens that overlap or touch a spelling location Loc.
+/// This always returns 0-2 tokens.
+llvm::ArrayRef
+spelledTokensTouching(SourceLocation Loc, const syntax::TokenBuffer &Tokens);
+
+/// The identifier token that overlaps or touches a spelling location Loc.
+/// If there is none, returns nullptr.
+const syntax::Token *
+spelledIdentifierTouching(SourceLocation Loc,
+  const syntax::TokenBuffer &Tokens);
+
 /// Lex the text buffer, corresponding to \p FID, in raw mode and record the
 /// resulting spelled tokens. Does minimal post-processing on raw identifiers,
 /// setting the appropriate token kind (instead of the raw_identifier reported
Index: clang/include/clang/Basic/SourceLocation.h
===
--- clang/include/clang/Basic/SourceLocation.h
+++ clang/include/clang/Basic/SourceLocation.h
@@ -188,9 +188,20 @@
   return !(LHS == RHS);
 }
 
+// Ordering is meaningful only if LHS and RHS have the same FileID!
+// Otherwis

[PATCH] D20689: [clang-tidy] Suspicious Call Argument checker

2019-12-12 Thread Whisperity via Phabricator via cfe-commits

whisperity added a comment.

I have developed a related check in D69560 . 
That one considers types, but is an //interface rule// checker, and does not 
consider (any) potential call sites. Moreover, it does not consider "swaps" 
that happen across a function call, only, as the name implies, //adjacent// 
similar-type ranges.

Maybe one could lift the "is-similar-type", or rather, 
"is-accidentally-mixable-type" related ruling to some common location, and use 
type similarity as a precondition gate in the reports of this check?




Comment at: docs/clang-tidy/checks/misc-redundant-expression.rst:18
 
-Example:
+Examples:
 

This seems to be an unrelated diff.



Comment at: test/clang-tidy/misc-redundant-expression.cpp:20
   if (X - X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are 
equivalent [misc-redundant-expression]
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are 
equivalent [misc-redundant-expression]
   if (X / X) return 1;

This entire file seems to be unrelated to the discussion at hand, perhaps a 
rebase went sideways?


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

https://reviews.llvm.org/D20689



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

[PATCH] D71197: llvm premerge: clang format test

2019-12-12 Thread Mikhail Goncharov via Phabricator via cfe-commits

goncharov updated this revision to Diff 233554.
goncharov added a comment.

add header guards back


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71197

Files:
  clang-tools-extra/clangd/Function.h
  clang/include/clang/Analysis/AnalysisDeclContext.h
  clang/tools/clang-check/ClangCheck.cpp
  clang/tools/clang-diff/ClangDiff.cpp
  clang/tools/clang-fuzzer/ClangFuzzer.cpp
  clang/tools/clang-refactor/ClangRefactor.cpp

Index: clang/tools/clang-refactor/ClangRefactor.cpp
===
--- clang/tools/clang-refactor/ClangRefactor.cpp
+++ clang/tools/clang-refactor/ClangRefactor.cpp
@@ -90,7 +90,6 @@
   TestSourceSelectionArgument(TestSelectionRangesInFile TestSelections)
   : TestSelections(std::move(TestSelections)) {}
 
-  void print(raw_ostream &OS) override { TestSelections.dump(OS); }
 
   std::unique_ptr
   createCustomConsumer() override {
@@ -104,6 +103,9 @@
 return TestSelections.foreachRange(SM, Callback);
   }
 
+  void print(raw_ostream &OS) override {
+  TestSelections.dump(OS);
+  }
 private:
   TestSelectionRangesInFile TestSelections;
 };
Index: clang/tools/clang-fuzzer/ClangFuzzer.cpp
===
--- clang/tools/clang-fuzzer/ClangFuzzer.cpp
+++ clang/tools/clang-fuzzer/ClangFuzzer.cpp
@@ -15,11 +15,11 @@
 #include "handle-cxx/handle_cxx.h"
 
 using namespace clang_fuzzer;
-
-extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv) { return 0; }
-
 extern "C" int LLVMFuzzerTestOneInput(uint8_t *data, size_t size) {
   std::string s((const char *)data, size);
   HandleCXX(s, "./test.cc", {"-O2"});
   return 0;
 }
+
+extern "C" int LLVMFuzzerInitialize(   int *argc,
+   char ***argv) { return 0 ; }
Index: clang/tools/clang-diff/ClangDiff.cpp
===
--- clang/tools/clang-diff/ClangDiff.cpp
+++ clang/tools/clang-diff/ClangDiff.cpp
@@ -32,12 +32,10 @@
 cl::desc("Print the internal representation of the AST as JSON."),
 cl::init(false), cl::cat(ClangDiffCategory));
 
-static cl::opt PrintMatches("dump-matches",
-  cl::desc("Print the matched nodes."),
-  cl::init(false), cl::cat(ClangDiffCategory));
+static cl::opt PrintMatches("dump-matches", cl::desc("Print the matched nodes."), cl::init(false), cl::cat(ClangDiffCategory));
 
 static cl::opt HtmlDiff("html",
-  cl::desc("Output a side-by-side diff in HTML."),
+ cl::desc("Output a side-by-side diff in HTML."),
   cl::init(false), cl::cat(ClangDiffCategory));
 
 static cl::opt SourcePath(cl::Positional, cl::desc(""),
@@ -77,7 +75,10 @@
   std::make_unique(
   std::move(Compilations));
   AdjustingCompilations->appendArgumentsAdjuster(
-  getInsertArgumentAdjuster(ArgsBefore, ArgumentInsertPosition::BEGIN));
+
+  getInsertArgumentAdjuster(ArgsBefore   ,
+
+  ArgumentInsertPosition::BEGIN));
   AdjustingCompilations->appendArgumentsAdjuster(
   getInsertArgumentAdjuster(ArgsAfter, ArgumentInsertPosition::END));
   Compilations = std::move(AdjustingCompilations);
Index: clang/tools/clang-check/ClangCheck.cpp
===
--- clang/tools/clang-check/ClangCheck.cpp
+++ clang/tools/clang-check/ClangCheck.cpp
@@ -17,10 +17,10 @@
 
 #include "clang/AST/ASTConsumer.h"
 #include "clang/CodeGen/ObjectFilePCHContainerOperations.h"
-#include "clang/Driver/Options.h"
 #include "clang/Frontend/ASTConsumers.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Rewrite/Frontend/FixItRewriter.h"
+#include "clang/Driver/Options.h"
 #include "clang/Rewrite/Frontend/FrontendActions.h"
 #include "clang/StaticAnalyzer/Frontend/FrontendActions.h"
 #include "clang/Tooling/CommonOptionsParser.h"
@@ -28,8 +28,8 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Option/OptTable.h"
 #include "llvm/Support/Path.h"
-#include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
+#include "llvm/Support/Signals.h"
 
 using namespace clang::driver;
 using namespace clang::tooling;
@@ -52,7 +52,7 @@
 );
 
 static cl::OptionCategory ClangCheckCategory("clang-check options");
-static const opt::OptTable &Options = getDriverOptTable();
+staticconst opt::OptTable &Options = getDriverOptTable();
 static cl::opt
 ASTDump("ast-dump",
 cl::desc(Options.getOptionHelpText(options::OPT_ast_dump)),
@@ -148,7 +148,7 @@
   }
 };
 
-} // namespace
+}
 
 int main(int argc, const char **argv) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
Index: clang/include/clang/Analysis/AnalysisDeclContext.h
===
--- clang/include/clang/Analysis/AnalysisDeclContext.h

[PATCH] D68213: [LTO] Support for embedding bitcode section during LTO

2019-12-12 Thread Josef Eisl via Phabricator via cfe-commits

zapster added a comment.

Thanks again for you reviews! Since I do not have commit rights, I would be 
grateful if someone could push it for me.


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

https://reviews.llvm.org/D68213



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

[PATCH] D57732: Correct inf typo

2019-12-12 Thread Jim Lin via Phabricator via cfe-commits

Jim added a comment.

Do you need someone to commit this change for you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57732



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

[PATCH] D71197: llvm premerge: clang format test

2019-12-12 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
console-log.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71197



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

[PATCH] D48921: NFC - Typo fix in test/CodeGenCXX/runtime-dllstorage.cpp

2019-12-12 Thread Nico Weber via Phabricator via cfe-commits

thakis added a comment.

This breaks the test everywhere, e.g. http://45.33.8.238/linux/5543/step_7.txt

Can you revert this while you investigate if the test was broken since it 
landed or if something broke it later, while it wasn't really testing what it 
was supposed to test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D48921



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

[PATCH] D71356: [Tooling/Syntax] Helpers to find spelled tokens touching a location.

2019-12-12 Thread Sam McCall via Phabricator via cfe-commits

sammccall added inline comments.



Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:320
+const syntax::Token *
+spelledIdentifierTouching(SourceLocation Loc,
+  const syntax::TokenBuffer &Tokens);

kbobyrev wrote:
> sammccall wrote:
> > kbobyrev wrote:
> > > Maybe `llvm::Optional<>` here?
> > Pointers are the idiomatic way to express "optional, by-reference" in LLVM.
> > 
> > By contrast, optional is illegal, and optional is 
> > unidiomatic
> Ah, I see, I thought a copy is quite cheap in this case. Makes sense then.
Ah, it is, so Optional would be an option. But the address is meaningful 
- spelled tokens have contiguous storage that indicates their sequence. So 
returning a pointer is actually more useful.



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:261
+  bool AcceptRight = Right != All.end() && !(Loc < Right->location());
+  bool AcceptLeft = Right != All.begin() && !((Right - 1)->endLocation() < 
Loc);
+  return llvm::makeArrayRef(Right - (AcceptLeft ? 1 : 0),

ilya-biryukov wrote:
> Maybe compare offsets instead of locations?
> It might be more code, but it would read much cleaner.
> 
> It always feels weird to compare source locations for anything other than 
> storing them in `std::map`
I find the `SM.getFileOffset(...)` everywhere pretty hard to read and obscures 
the actual locations.
But agree about the operators, I've added the rest. Having `operator<` is 
pretty evil, but I don't think the others are particularly incrementally evil.



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:270
+  for (const syntax::Token &Tok : spelledTokensTouching(Loc, Tokens))
+if (Tok.kind() == tok::identifier)
+  return &Tok;

ilya-biryukov wrote:
> NIT: add braces around `if` statement
Is there some reference for preferred LLVM style for this, or personal 
preference? (Real question, as this comes up a bunch)

I ask because that's my *least*-preferred option - no braces > braces on for > 
braces on both > braces on (only) if.

Added braces to the `for` (maybe that's what you meant?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71356



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

[PATCH] D71356: [Tooling/Syntax] Helpers to find spelled tokens touching a location.

2019-12-12 Thread Sam McCall via Phabricator via cfe-commits

sammccall updated this revision to Diff 233557.
sammccall marked 10 inline comments as done.
sammccall added a comment.

braces


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71356

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

Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- clang/unittests/Tooling/Syntax/TokensTest.cpp
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -793,4 +793,45 @@
 ActualMacroRanges.push_back(Expansion->range(SM));
   EXPECT_EQ(ExpectedMacroRanges, ActualMacroRanges);
 }
+
+TEST_F(TokenBufferTest, Touching) {
+  llvm::Annotations Code("^i^nt^ ^a^b^=^1;^");
+  recordTokens(Code.code());
+
+  auto Touching = [&](int Index) {
+SourceLocation Loc = SourceMgr->getComposedLoc(SourceMgr->getMainFileID(),
+   Code.points()[Index]);
+return spelledTokensTouching(Loc, Buffer);
+  };
+  auto Identifier = [&](int Index) {
+SourceLocation Loc = SourceMgr->getComposedLoc(SourceMgr->getMainFileID(),
+   Code.points()[Index]);
+const syntax::Token *Tok = spelledIdentifierTouching(Loc, Buffer);
+return Tok ? Tok->text(*SourceMgr) : "";
+  };
+
+  EXPECT_THAT(Touching(0), SameRange(findSpelled("int")));
+  EXPECT_EQ(Identifier(0), "");
+  EXPECT_THAT(Touching(1), SameRange(findSpelled("int")));
+  EXPECT_EQ(Identifier(1), "");
+  EXPECT_THAT(Touching(2), SameRange(findSpelled("int")));
+  EXPECT_EQ(Identifier(2), "");
+
+  EXPECT_THAT(Touching(3), SameRange(findSpelled("ab")));
+  EXPECT_EQ(Identifier(3), "ab");
+  EXPECT_THAT(Touching(4), SameRange(findSpelled("ab")));
+  EXPECT_EQ(Identifier(4), "ab");
+
+  EXPECT_THAT(Touching(5), SameRange(findSpelled("ab =")));
+  EXPECT_EQ(Identifier(5), "ab");
+
+  EXPECT_THAT(Touching(6), SameRange(findSpelled("= 1")));
+  EXPECT_EQ(Identifier(6), "");
+
+  EXPECT_THAT(Touching(7), SameRange(findSpelled(";")));
+  EXPECT_EQ(Identifier(7), "");
+
+  ASSERT_EQ(Code.points().size(), 8u);
+}
+
 } // namespace
Index: clang/lib/Tooling/Syntax/Tokens.cpp
===
--- clang/lib/Tooling/Syntax/Tokens.cpp
+++ clang/lib/Tooling/Syntax/Tokens.cpp
@@ -248,6 +248,31 @@
   return E;
 }
 
+llvm::ArrayRef
+syntax::spelledTokensTouching(SourceLocation Loc,
+  const syntax::TokenBuffer &Tokens) {
+  assert(Loc.isFileID());
+  llvm::ArrayRef All =
+  Tokens.spelledTokens(Tokens.sourceManager().getFileID(Loc));
+  // Comparing SourceLocations is well-defined within a FileID.
+  auto *Right = llvm::partition_point(
+  All, [&](const syntax::Token &Tok) { return Tok.location() < Loc; });
+  bool AcceptRight = Right != All.end() && Right->location() <= Loc;
+  bool AcceptLeft = Right != All.begin() && (Right - 1)->endLocation() >= Loc;
+  return llvm::makeArrayRef(Right - (AcceptLeft ? 1 : 0),
+Right + (AcceptRight ? 1 : 0));
+}
+
+const syntax::Token *
+syntax::spelledIdentifierTouching(SourceLocation Loc,
+  const syntax::TokenBuffer &Tokens) {
+  for (const syntax::Token &Tok : spelledTokensTouching(Loc, Tokens)) {
+if (Tok.kind() == tok::identifier)
+  return &Tok;
+  }
+  return nullptr;
+}
+
 std::vector
 TokenBuffer::macroExpansions(FileID FID) const {
   auto FileIt = Files.find(FID);
Index: clang/include/clang/Tooling/Syntax/Tokens.h
===
--- clang/include/clang/Tooling/Syntax/Tokens.h
+++ clang/include/clang/Tooling/Syntax/Tokens.h
@@ -309,6 +309,17 @@
   const SourceManager *SourceMgr;
 };
 
+/// The spelled tokens that overlap or touch a spelling location Loc.
+/// This always returns 0-2 tokens.
+llvm::ArrayRef
+spelledTokensTouching(SourceLocation Loc, const syntax::TokenBuffer &Tokens);
+
+/// The identifier token that overlaps or touches a spelling location Loc.
+/// If there is none, returns nullptr.
+const syntax::Token *
+spelledIdentifierTouching(SourceLocation Loc,
+  const syntax::TokenBuffer &Tokens);
+
 /// Lex the text buffer, corresponding to \p FID, in raw mode and record the
 /// resulting spelled tokens. Does minimal post-processing on raw identifiers,
 /// setting the appropriate token kind (instead of the raw_identifier reported
Index: clang/include/clang/Basic/SourceLocation.h
===
--- clang/include/clang/Basic/SourceLocation.h
+++ clang/include/clang/Basic/SourceLocation.h
@@ -188,9 +188,20 @@
   return !(LHS == RHS);
 }
 
+// Ordering is meaningful only if LHS and RHS have the same FileID!
+// Otherwise use SourceManager::isBe

[PATCH] D71356: [Tooling/Syntax] Helpers to find spelled tokens touching a location.

2019-12-12 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 60699 tests passed, 0 failed 
and 726 were skipped.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
console-log.txt 
,
 CMakeCache.txt 
,
 test-results.xml 
,
 diff.json 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71356



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

[PATCH] D71197: llvm premerge: clang format test

2019-12-12 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 60748 tests passed, 0 failed 
and 726 were skipped.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or apply this patch 
.

Build artifacts 
: 
console-log.txt 
,
 CMakeCache.txt 
,
 clang-format.patch 
,
 test-results.xml 
,
 diff.json 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71197



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

[PATCH] D71356: [Tooling/Syntax] Helpers to find spelled tokens touching a location.

2019-12-12 Thread Kirill Bobyrev via Phabricator via cfe-commits

kbobyrev added inline comments.



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:270
+  for (const syntax::Token &Tok : spelledTokensTouching(Loc, Tokens))
+if (Tok.kind() == tok::identifier)
+  return &Tok;

ilya-biryukov wrote:
> sammccall wrote:
> > ilya-biryukov wrote:
> > > NIT: add braces around `if` statement
> > Is there some reference for preferred LLVM style for this, or personal 
> > preference? (Real question, as this comes up a bunch)
> > 
> > I ask because that's my *least*-preferred option - no braces > braces on 
> > for > braces on both > braces on (only) if.
> > 
> > Added braces to the `for` (maybe that's what you meant?)
> Not sure if it's in LLVM style guide, but files inside Syntax folder 
> definitely use this style: put braces everywhere until you reach the last 
> level of nesting for non-leaf statements (i.e. having other statements as 
> children, e.g. loops,. if statements, etc)
> 
> 
> It's my personal preference, happy to discuss whether having this makes sense.
I guess it's a personal preference (also for me), but I don't think there is a 
strict guideline on that. Interestingly enough, I think there is a piece of 
code in the styleguide that looks exactly like the code you had: 
https://llvm.org/docs/CodingStandards.html#turn-predicate-loops-into-predicate-functions

Some Clang subprojects tend to put braces everywhere though.

That being said, I guess no braces at all would be the best option here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71356



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

[PATCH] D71247: [clangd] Rename constructors and destructors in cross-file case

2019-12-12 Thread Sam McCall via Phabricator via cfe-commits

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:91
+  DeclRelation::Alias | DeclRelation::TemplatePattern)) {
+// If the cursor is at the underlying CXXRecordDecl of the
+// ClassTemplateDecl, ND will be the CXXRecordDecl. In this case, we need 
to

kbobyrev wrote:
> sammccall wrote:
> > why is this code being moved?
> So, this code was previously only in the within-file rename path and hence 
> triggering rename from a constructor/destructor-s was only possible there. 
> Moving the code here improves overall "symbol selection" and fixes it for 
> global renames, too.
Clangd as a whole actually canonicalizes in the other direction (templatedecl 
-> templated decl).
So I think this change introduces a bug (the references you seek won't be in 
the index).

Certainly up for discussion which is better (behavior is historical) but that's 
a larger change.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:96
+D = D->getDescribedTemplate() ? D->getDescribedTemplate() : D;
+const auto *ND = llvm::dyn_cast(D);
+// Get to CXXRecordDecl from constructor or destructor.

dyn_cast then unconditionally inserting seems dodgy - if it can be null, it 
must be checked, and if it can't, it should be plain `cast`


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

https://reviews.llvm.org/D71247



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

[PATCH] D71406: [clangd] Add xref for macros to FileIndex.

2019-12-12 Thread UTKARSH SAXENA via Phabricator via cfe-commits

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

Adds macro references to the dynamic index. 
Tests added.
Also exposed a new API to convert path to URI in URI.h


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71406

Files:
  clang-tools-extra/clangd/URI.cpp
  clang-tools-extra/clangd/URI.h
  clang-tools-extra/clangd/index/FileIndex.cpp
  clang-tools-extra/clangd/index/FileIndex.h
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/unittests/FileIndexTests.cpp

Index: clang-tools-extra/clangd/unittests/FileIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/FileIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/FileIndexTests.cpp
@@ -345,6 +345,41 @@
  FileURI("unittest:///test2.cc"))}));
 }
 
+TEST(FileIndexTest, MacroRefs) {
+  Annotations HeaderCode(R"cpp(
+#define $macro[[MACRO]](X) (X+1)
+  )cpp");
+  Annotations MainCode(R"cpp(
+  void f() {
+int a = $macro[[MACRO]](1);
+  }
+  )cpp");
+
+  auto Macro = findSymbol(
+  TestTU::withHeaderCode(HeaderCode.code()).headerSymbols(), "MACRO");
+  FileIndex Index;
+  // Add test.cc
+  TestTU Test;
+  Test.HeaderCode = HeaderCode.code();
+  Test.Code = MainCode.code();
+  Test.Filename = "test.cc";
+  auto AST = Test.build();
+  Index.updateMain(Test.Filename, AST);
+  // Add test2.cc
+  TestTU Test2;
+  Test2.HeaderCode = HeaderCode.code();
+  Test2.Code = MainCode.code();
+  Test2.Filename = "test2.cc";
+  AST = Test2.build();
+  Index.updateMain(Test2.Filename, AST);
+
+  EXPECT_THAT(getRefs(Index, Macro.ID),
+  RefsAre({AllOf(RefRange(MainCode.range("macro")),
+ FileURI("unittest:///test.cc")),
+   AllOf(RefRange(MainCode.range("macro")),
+ FileURI("unittest:///test2.cc"))}));
+}
+
 TEST(FileIndexTest, CollectMacros) {
   FileIndex M;
   update(M, "f", "#define CLANGD 1");
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -45,31 +45,6 @@
   return ND;
 }
 
-// Returns a URI of \p Path. Firstly, this makes the \p Path absolute using the
-// current working directory of the given SourceManager if the Path is not an
-// absolute path. If failed, this resolves relative paths against \p FallbackDir
-// to get an absolute path. Then, this tries creating an URI for the absolute
-// path with schemes specified in \p Opts. This returns an URI with the first
-// working scheme, if there is any; otherwise, this returns None.
-//
-// The Path can be a path relative to the build directory, or retrieved from
-// the SourceManager.
-std::string toURI(const SourceManager &SM, llvm::StringRef Path,
-  const SymbolCollector::Options &Opts) {
-  llvm::SmallString<128> AbsolutePath(Path);
-  if (auto File = SM.getFileManager().getFile(Path)) {
-if (auto CanonPath = getCanonicalPath(*File, SM)) {
-  AbsolutePath = *CanonPath;
-}
-  }
-  // We don't perform is_absolute check in an else branch because makeAbsolute
-  // might return a relative path on some InMemoryFileSystems.
-  if (!llvm::sys::path::is_absolute(AbsolutePath) && !Opts.FallbackDir.empty())
-llvm::sys::fs::make_absolute(Opts.FallbackDir, AbsolutePath);
-  llvm::sys::path::remove_dots(AbsolutePath, /*remove_dot_dot=*/true);
-  return URI::create(AbsolutePath).toString();
-}
-
 // All proto generated headers should start with this line.
 static const char *PROTO_HEADER_COMMENT =
 "// Generated by the protocol buffer compiler.  DO NOT EDIT!";
@@ -157,7 +132,7 @@
   auto Path = SM.getFilename(TokLoc);
   if (Path.empty())
 return None;
-  FileURIStorage = toURI(SM, Path, Opts);
+  FileURIStorage = toURI(Path, SM, Opts.FallbackDir).toString();
   SymbolLocation Result;
   Result.FileURI = FileURIStorage.c_str();
   auto Range = getTokenRange(TokLoc, SM, LangOpts);
@@ -517,7 +492,7 @@
 auto Found = URICache.find(FID);
 if (Found == URICache.end()) {
   if (auto *FileEntry = SM.getFileEntryForID(FID)) {
-auto FileURI = toURI(SM, FileEntry->getName(), Opts);
+auto FileURI = toURI(FileEntry->getName(), SM, Opts.FallbackDir).toString();
 Found = URICache.insert({FID, FileURI}).first;
   } else {
 // Ignore cases where we can not find a corresponding file entry
@@ -679,7 +654,7 @@
 if (Canonical.startswith("<") || Canonical.startswith("\""))
   return Canonical.str();
 if (Canonical != Filename)
-  return toURI(SM, Canonical, Opts);
+  return toURI(Canonical, SM, Opts.FallbackDir).toString();
   }
   if (!isSelfContainedHeader(FID)) {
 // A .i

[PATCH] D71356: [Tooling/Syntax] Helpers to find spelled tokens touching a location.

2019-12-12 Thread Ilya Biryukov via Phabricator via cfe-commits

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



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:261
+  bool AcceptRight = Right != All.end() && !(Loc < Right->location());
+  bool AcceptLeft = Right != All.begin() && !((Right - 1)->endLocation() < 
Loc);
+  return llvm::makeArrayRef(Right - (AcceptLeft ? 1 : 0),

sammccall wrote:
> ilya-biryukov wrote:
> > Maybe compare offsets instead of locations?
> > It might be more code, but it would read much cleaner.
> > 
> > It always feels weird to compare source locations for anything other than 
> > storing them in `std::map`
> I find the `SM.getFileOffset(...)` everywhere pretty hard to read and 
> obscures the actual locations.
> But agree about the operators, I've added the rest. Having `operator<` is 
> pretty evil, but I don't think the others are particularly incrementally evil.
I find it to be the only way to write code, which unambiguously says what it 
wants to do with source locations.

Not saying it's pretty, but I the source location API was designed to be used 
this way. Arguably, using something else in special corner cases does more harm 
than good (hard-to-read, non-conventional code).

But up to you.



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:270
+  for (const syntax::Token &Tok : spelledTokensTouching(Loc, Tokens))
+if (Tok.kind() == tok::identifier)
+  return &Tok;

sammccall wrote:
> ilya-biryukov wrote:
> > NIT: add braces around `if` statement
> Is there some reference for preferred LLVM style for this, or personal 
> preference? (Real question, as this comes up a bunch)
> 
> I ask because that's my *least*-preferred option - no braces > braces on for 
> > braces on both > braces on (only) if.
> 
> Added braces to the `for` (maybe that's what you meant?)
Not sure if it's in LLVM style guide, but files inside Syntax folder definitely 
use this style: put braces everywhere until you reach the last level of nesting 
for non-leaf statements (i.e. having other statements as children, e.g. loops,. 
if statements, etc)


It's my personal preference, happy to discuss whether having this makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71356



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

[PATCH] D71356: [Tooling/Syntax] Helpers to find spelled tokens touching a location.

2019-12-12 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 60699 tests passed, 0 failed 
and 726 were skipped.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
console-log.txt 
,
 CMakeCache.txt 
,
 test-results.xml 
,
 diff.json 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71356



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

[PATCH] D71356: [Tooling/Syntax] Helpers to find spelled tokens touching a location.

2019-12-12 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added inline comments.



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:270
+  for (const syntax::Token &Tok : spelledTokensTouching(Loc, Tokens))
+if (Tok.kind() == tok::identifier)
+  return &Tok;

kbobyrev wrote:
> ilya-biryukov wrote:
> > sammccall wrote:
> > > ilya-biryukov wrote:
> > > > NIT: add braces around `if` statement
> > > Is there some reference for preferred LLVM style for this, or personal 
> > > preference? (Real question, as this comes up a bunch)
> > > 
> > > I ask because that's my *least*-preferred option - no braces > braces on 
> > > for > braces on both > braces on (only) if.
> > > 
> > > Added braces to the `for` (maybe that's what you meant?)
> > Not sure if it's in LLVM style guide, but files inside Syntax folder 
> > definitely use this style: put braces everywhere until you reach the last 
> > level of nesting for non-leaf statements (i.e. having other statements as 
> > children, e.g. loops,. if statements, etc)
> > 
> > 
> > It's my personal preference, happy to discuss whether having this makes 
> > sense.
> I guess it's a personal preference (also for me), but I don't think there is 
> a strict guideline on that. Interestingly enough, I think there is a piece of 
> code in the styleguide that looks exactly like the code you had: 
> https://llvm.org/docs/CodingStandards.html#turn-predicate-loops-into-predicate-functions
> 
> Some Clang subprojects tend to put braces everywhere though.
> 
> That being said, I guess no braces at all would be the best option here.
Yeah, so LLVM style is unclear, but has examples with no braces.
Could be the argument to get rid of those.

I'd still stick to my personal preferences where I can, but if everyone thinks 
no braces is better, happy to go with this option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71356



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

[PATCH] D71247: [clangd] Rename constructors and destructors in cross-file case

2019-12-12 Thread Kirill Bobyrev via Phabricator via cfe-commits

kbobyrev updated this revision to Diff 233563.
kbobyrev marked 3 inline comments as done.

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

https://reviews.llvm.org/D71247

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

Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -487,11 +487,10 @@
   "not a supported kind", HeaderFile, Index},
 
   {
-
   R"cpp(
 struct X { X operator++(int); };
 void f(X x) {x+^+;})cpp",
-  "not a supported kind", HeaderFile, Index},
+  "no symbol", HeaderFile, Index},
 
   {R"cpp(// foo is declared outside the file.
 void fo^o() {}
@@ -724,11 +723,85 @@
   std::vector Diagnostics) override {}
   } DiagConsumer;
   // rename is runnning on the "^" point in FooH, and "[[]]" ranges are the
-  // expcted rename occurrences.
+  // expected rename occurrences.
   struct Case {
 llvm::StringRef FooH;
 llvm::StringRef FooCC;
-  } Cases [] = {
+} Cases [] = {
+  {
+// Constructor.
+  R"cpp(
+class [[Foo]] {
+  [[Foo^]]();
+  ~[[Foo]]();
+};
+  )cpp",
+  R"cpp(
+#include "foo.h"
+[[Foo]]::[[Foo]]() {}
+[[Foo]]::~[[Foo]]() {}
+
+void func() {
+  [[Foo]] foo;
+}
+  )cpp",
+},
+{
+  // Destructor (selecting before the identifier).
+  R"cpp(
+class [[Foo]] {
+  [[Foo]]();
+  ~[[^Foo]]();
+};
+  )cpp",
+  R"cpp(
+#include "foo.h"
+[[Foo]]::[[Foo]]() {}
+[[Foo]]::~[[Foo]]() {}
+
+void func() {
+  [[Foo]] foo;
+}
+  )cpp",
+},
+{
+  // Destructor (selecting within the identifier).
+  R"cpp(
+class [[Foo]] {
+  [[Foo]]();
+  ~[[F^oo]]();
+};
+  )cpp",
+  R"cpp(
+#include "foo.h"
+[[Foo]]::[[Foo]]() {}
+[[Foo]]::~[[Foo]]() {}
+
+void func() {
+  [[Foo]] foo;
+}
+  )cpp",
+},
+{
+  // Destructor (selecting after the identifier).
+  R"cpp(
+class [[Foo]] {
+  [[Foo]]();
+  virtual ~ /*~Foo?*/[[Foo^]]() {
+int a = 4;
+  }
+};
+  )cpp",
+  R"cpp(
+#include "foo.h"
+[[Foo]]::[[Foo]]() {}
+[[Foo]]::~[[Foo]]() {}
+
+void func() {
+  [[Foo]] foo;
+}
+  )cpp",
+},
 {
   // classes.
   R"cpp(
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -18,8 +18,10 @@
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Tooling/Refactoring/Rename/USRFindingAction.h"
+#include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FormatVariadic.h"
 #include 
@@ -83,21 +85,15 @@
   if (!SelectedNode)
 return {};
 
-  // If the location points to a Decl, we check it is actually on the name
-  // range of the Decl. This would avoid allowing rename on unrelated tokens.
-  //   ^class Foo {} // SelectionTree returns CXXRecordDecl,
-  // // we don't attempt to trigger rename on this position.
-  // FIXME: Make this work on destructors, e.g. "~F^oo()".
-  if (const auto *D = SelectedNode->ASTNode.get()) {
-if (D->getLocation() != TokenStartLoc)
-  return {};
-  }
-
   llvm::DenseSet Result;
   for (const auto *D :
targetDecl(SelectedNode->ASTNode,
-  DeclRelation::Alias | DeclRelation::TemplatePattern))
-Result.insert(D);
+  DeclRelation::Alias | DeclRelation::TemplatePattern)) {
+const auto *ND = llvm::cast(D);
+// Get to CXXRecordDecl from constructor or destructor.
+ND = tooling::getCanonicalSymbolDeclaration(ND);
+Result.insert(ND);
+  }
   return Result;
 }
 
@@ -214,17 +210,13 @@
 // Return all rename occurrences in the main file.
 std::vector findOccurrencesWithinFile(ParsedAST &AST,
   const NamedDecl &ND) {
-  // In theory, locateDeclAt should return the primary template. However, if the
-  // cursor is under the underlying CXXRecordDecl of the ClassTemplateDecl, ND
-  // will be the CXXRecordDecl, for this case, we need to get the primary
-  // template maunally.
-  const auto &RenameDecl =
-  ND.getDescribedTemplate() ? *ND.getDescribedTemplate() : ND;
-  // getUSRsForDeclaration will find other related symbols, e.g. vi

[PATCH] D71356: [Tooling/Syntax] Helpers to find spelled tokens touching a location.

2019-12-12 Thread Sam McCall via Phabricator via cfe-commits

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



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:261
+  bool AcceptRight = Right != All.end() && !(Loc < Right->location());
+  bool AcceptLeft = Right != All.begin() && !((Right - 1)->endLocation() < 
Loc);
+  return llvm::makeArrayRef(Right - (AcceptLeft ? 1 : 0),

ilya-biryukov wrote:
> sammccall wrote:
> > ilya-biryukov wrote:
> > > Maybe compare offsets instead of locations?
> > > It might be more code, but it would read much cleaner.
> > > 
> > > It always feels weird to compare source locations for anything other than 
> > > storing them in `std::map`
> > I find the `SM.getFileOffset(...)` everywhere pretty hard to read and 
> > obscures the actual locations.
> > But agree about the operators, I've added the rest. Having `operator<` is 
> > pretty evil, but I don't think the others are particularly incrementally 
> > evil.
> I find it to be the only way to write code, which unambiguously says what it 
> wants to do with source locations.
> 
> Not saying it's pretty, but I the source location API was designed to be used 
> this way. Arguably, using something else in special corner cases does more 
> harm than good (hard-to-read, non-conventional code).
> 
> But up to you.
> But up to you.

I would rather use the operators, with the comment for the unusual usage, if 
that's OK.

(I agree it doesn't generalize and about the intent of the APIs, but think the 
APIs hurt readability to the extent that there's less value in consistency than 
in clear intent at individual callsites).



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:270
+  for (const syntax::Token &Tok : spelledTokensTouching(Loc, Tokens))
+if (Tok.kind() == tok::identifier)
+  return &Tok;

ilya-biryukov wrote:
> kbobyrev wrote:
> > ilya-biryukov wrote:
> > > sammccall wrote:
> > > > ilya-biryukov wrote:
> > > > > NIT: add braces around `if` statement
> > > > Is there some reference for preferred LLVM style for this, or personal 
> > > > preference? (Real question, as this comes up a bunch)
> > > > 
> > > > I ask because that's my *least*-preferred option - no braces > braces 
> > > > on for > braces on both > braces on (only) if.
> > > > 
> > > > Added braces to the `for` (maybe that's what you meant?)
> > > Not sure if it's in LLVM style guide, but files inside Syntax folder 
> > > definitely use this style: put braces everywhere until you reach the last 
> > > level of nesting for non-leaf statements (i.e. having other statements as 
> > > children, e.g. loops,. if statements, etc)
> > > 
> > > 
> > > It's my personal preference, happy to discuss whether having this makes 
> > > sense.
> > I guess it's a personal preference (also for me), but I don't think there 
> > is a strict guideline on that. Interestingly enough, I think there is a 
> > piece of code in the styleguide that looks exactly like the code you had: 
> > https://llvm.org/docs/CodingStandards.html#turn-predicate-loops-into-predicate-functions
> > 
> > Some Clang subprojects tend to put braces everywhere though.
> > 
> > That being said, I guess no braces at all would be the best option here.
> Yeah, so LLVM style is unclear, but has examples with no braces.
> Could be the argument to get rid of those.
> 
> I'd still stick to my personal preferences where I can, but if everyone 
> thinks no braces is better, happy to go with this option.
I think we're good here - I'm fine with the braces on `for`, and it sounds like 
that's what you wanted.
(I guess I misread "braces around if statement" as "braces around if body")


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71356



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

[clang] d8f4991 - [NFC] - Partially revert 9c48c2f9c477007234c

2019-12-12 Thread Gabor Buella via cfe-commits


Author: Gabor Buella
Date: 2019-12-12T12:46:17+01:00
New Revision: d8f49912847dd8a589e992e2e07d79ac77e61408

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

LOG: [NFC] - Partially revert 9c48c2f9c477007234c

Added: 


Modified: 
clang/test/CodeGenCXX/runtime-dllstorage.cpp

Removed: 




diff  --git a/clang/test/CodeGenCXX/runtime-dllstorage.cpp 
b/clang/test/CodeGenCXX/runtime-dllstorage.cpp
index 864a1ab51d65..694a9b4ea368 100644
--- a/clang/test/CodeGenCXX/runtime-dllstorage.cpp
+++ b/clang/test/CodeGenCXX/runtime-dllstorage.cpp
@@ -134,7 +134,7 @@ void l() {
 // CHECK-DYNAMIC-NODECL-IA-DAG: declare dllimport void 
@__cxa_guard_release(i64*)
 // CHECK-DYNAMIC-IMPORT-IA-DAG: declare dllimport void 
@__cxa_guard_release(i64*)
 // CHECK-DYNAMIC-EXPORT-IA-DAG: declare dllimport void 
@__cxa_guard_release(i64*)
-// CHECK-DYNAMIC-IA-DAG: declare dllimport void @_ZSt9terminatev()
+// CHECK-DYANMIC-IA-DAG: declare dllimport void @_ZSt9terminatev()
 // CHECK-DYNAMIC-NODECL-IA-DAG: declare dso_local void @_ZSt9terminatev()
 // CHECK-DYNAMIC-IMPORT-IA-DAG: declare dllimport void @_ZSt9terminatev()
 // CHECK-DYNAMIC-EXPORT-IA-DAG: declare dso_local dllexport void 
@_ZSt9terminatev()



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

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

2019-12-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits

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

- Move separation logic from container to blocks
- Add two concrete APIs to block for getting strings and update tests to use 
those.
- Rename methods


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71248

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/FormattedString.cpp
  clang-tools-extra/clangd/FormattedString.h
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/Hover.h
  clang-tools-extra/clangd/unittests/FormattedStringTests.cpp

Index: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
===
--- clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
+++ clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
@@ -8,192 +8,143 @@
 #include "FormattedString.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/raw_ostream.h"
 
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace clangd {
+namespace markup {
 namespace {
 
-TEST(FormattedString, Basic) {
-  FormattedString S;
-  EXPECT_EQ(S.renderAsPlainText(), "");
-  EXPECT_EQ(S.renderAsMarkdown(), "");
+enum RenderKind {
+  PlainText,
+  Markdown,
+};
 
-  S.appendText("foobar  ");
-  S.appendText("baz");
-  EXPECT_EQ(S.renderAsPlainText(), "foobar baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "foobar  baz");
-
-  S = FormattedString();
-  S.appendInlineCode("foobar");
-  EXPECT_EQ(S.renderAsPlainText(), "foobar");
-  EXPECT_EQ(S.renderAsMarkdown(), "`foobar`");
-
-  S = FormattedString();
-  S.appendCodeBlock("foobar");
-  EXPECT_EQ(S.renderAsPlainText(), "foobar");
-  EXPECT_EQ(S.renderAsMarkdown(), "```cpp\n"
-  "foobar\n"
-  "```\n");
-}
-
-TEST(FormattedString, CodeBlocks) {
-  FormattedString S;
-  S.appendCodeBlock("foobar");
-  S.appendCodeBlock("bazqux", "javascript");
-  S.appendText("after");
-
-  std::string ExpectedText = R"(foobar
-
-bazqux
-
-after)";
-  EXPECT_EQ(S.renderAsPlainText(), ExpectedText);
-  std::string ExpectedMarkdown = R"md(```cpp
-foobar
-```
-```javascript
-bazqux
-```
-after)md";
-  EXPECT_EQ(S.renderAsMarkdown(), ExpectedMarkdown);
-
-  S = FormattedString();
-  S.appendInlineCode("foobar");
-  S.appendInlineCode("bazqux");
-  EXPECT_EQ(S.renderAsPlainText(), "foobar bazqux");
-  EXPECT_EQ(S.renderAsMarkdown(), "`foobar` `bazqux`");
-
-  S = FormattedString();
-  S.appendText("foo");
-  S.appendInlineCode("bar");
-  S.appendText("baz");
-
-  EXPECT_EQ(S.renderAsPlainText(), "foo bar baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "foo `bar` baz");
-}
-
-TEST(FormattedString, Escaping) {
+TEST(Render, Escaping) {
   // Check some ASCII punctuation
-  FormattedString S;
-  S.appendText("*!`");
-  EXPECT_EQ(S.renderAsMarkdown(), "\\*\\!\\`");
+  Paragraph P;
+  P.appendText("*!`");
+  EXPECT_EQ(P.asMarkdown(), "\\*\\!\\`");
 
   // Check all ASCII punctuation.
-  S = FormattedString();
+  P = Paragraph();
   std::string Punctuation = R"txt(!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~)txt";
   // Same text, with each character escaped.
   std::string EscapedPunctuation;
   EscapedPunctuation.reserve(2 * Punctuation.size());
   for (char C : Punctuation)
 EscapedPunctuation += std::string("\\") + C;
-  S.appendText(Punctuation);
-  EXPECT_EQ(S.renderAsMarkdown(), EscapedPunctuation);
+  P.appendText(Punctuation);
+  EXPECT_EQ(P.asMarkdown(), EscapedPunctuation);
 
   // In code blocks we don't need to escape ASCII punctuation.
-  S = FormattedString();
-  S.appendInlineCode("* foo !+ bar * baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "`* foo !+ bar * baz`");
-  S = FormattedString();
-  S.appendCodeBlock("#define FOO\n* foo !+ bar * baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "```cpp\n"
-  "#define FOO\n* foo !+ bar * baz\n"
-  "```\n");
+  P = Paragraph();
+  P.appendCode("* foo !+ bar * baz");
+  EXPECT_EQ(P.asMarkdown(), "`* foo !+ bar * baz`");
 
   // But we have to escape the backticks.
-  S = FormattedString();
-  S.appendInlineCode("foo`bar`baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "`foo``bar``baz`");
-
-  S = FormattedString();
-  S.appendCodeBlock("foo`bar`baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "```cpp\n"
-  "foo`bar`baz\n"
-  "```\n");
+  P = Paragraph();
+  P.appendCode("foo`bar`baz");
+  EXPECT_EQ(P.asMarkdown(), "`foo``bar``baz`");
 
   // Inline code blocks starting or ending with backticks should add spaces.
-  S = FormattedString();
-  S.appendInlineCode("`foo");
-  EXPECT_EQ(S.renderAsMarkdown(), "` ``foo `");
-  S = FormattedString();
-  S.appendInlineCode("foo`");
-  EXPECT_EQ(S.renderAsMarkdown(), "` foo`` `");
-  S = FormattedString();
-  S.

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

2019-12-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits

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



Comment at: clang-tools-extra/clangd/FormattedString.cpp:122
+};
+std::string renderBlocks(llvm::ArrayRef> Children,
+ RenderType RT) {

sammccall wrote:
> I'm not sure this common codepath for plain-text and markdown will survive in 
> the long run:
>  - state around indentation and wrapping is likely to differ
>  - separating blocks with one newline may not be the right thing in each case
I suppose this will survive in the long run now, as it only prints blocks, 
without separation, and trims in the end.



Comment at: clang-tools-extra/clangd/FormattedString.cpp:130
+OS << Sep;
+Sep = "\n";
+((*C).*RenderFunc)(OS);

sammccall wrote:
> for markdown, this means consecutive paragraphs will be rendered as a single 
> paragraph with line breaks in it. intended?
yes this was intended, but as each block takes care of its own indentation now, 
this is no longer an issue.



Comment at: clang-tools-extra/clangd/FormattedString.h:29
 public:
+  virtual void asMarkdown(llvm::raw_ostream &OS) const = 0;
+  virtual void asPlainText(llvm::raw_ostream &OS) const = 0;

sammccall wrote:
> Do these include trailing newline(s), or not?
> 
> Based on offline discussion, they currently do not, but this causes some 
> problems e.g. list followed by paragraph would get merged.
> 
> Option a) have Blocks include required trailing newlines (e.g. \n\n after 
> lists in markdown, \n after paragraphs), and have Document trim trailing 
> newlines.
> Option b) have Document look at the type of the elements above/below and 
> insert space appropriately.
> 
> Personally prefer option A because it encapsulates behavior better in the 
> classes (less dyn_cast) and I think reflects the grammar better.
taking option A



Comment at: clang-tools-extra/clangd/Hover.cpp:460
   if (!Definition.empty()) {
-Output.appendCodeBlock(Definition);
+Output.addParagraph().appendCode(Definition);
   } else {

sammccall wrote:
> This is a regression at the moment - complicated definitions are 
> clang-formatted, and we'll canonicalize the whitespace.
> (The behavior of the library is correct, we just need code block for this).
not planning to land until we've got the complete implementation, including 
lists and codeblocks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71248



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

[PATCH] D48921: NFC - Typo fix in test/CodeGenCXX/runtime-dllstorage.cpp

2019-12-12 Thread Gabor Buella via Phabricator via cfe-commits

GBuella added a comment.

In D48921#1781447 , @thakis wrote:

> This breaks the test everywhere, e.g. http://45.33.8.238/linux/5543/step_7.txt
>
> Can you revert this while you investigate if the test was broken since it 
> landed or if something broke it later, while it wasn't really testing what it 
> was supposed to test?


Yes, I reverted the change on line 137, I recall the other changes didn't cause 
trouble.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D48921



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

[PATCH] D71247: [clangd] Rename constructors and destructors in cross-file case

2019-12-12 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.

LG but the cast needs to be fixed


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

https://reviews.llvm.org/D71247



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

[PATCH] D71406: [clangd] Add xref for macros to FileIndex.

2019-12-12 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 60496 tests passed, 0 failed 
and 726 were skipped.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or apply this patch 
.

Build artifacts 
: 
console-log.txt 
,
 CMakeCache.txt 
,
 clang-format.patch 
,
 test-results.xml 
,
 diff.json 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71406



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

[PATCH] D71247: [clangd] Rename constructors and destructors in cross-file case

2019-12-12 Thread Sam McCall via Phabricator via cfe-commits

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:92
+  DeclRelation::Alias | DeclRelation::TemplatePattern)) {
+const auto *ND = llvm::cast(D);
+// Get to CXXRecordDecl from constructor or destructor.

OK, this can definitely fail though - just select 'friend X' in a class body or 
so. Need to check for ND and bail out.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:223
-  ND.getDescribedTemplate() ? *ND.getDescribedTemplate() : ND;
-  // getUSRsForDeclaration will find other related symbols, e.g. virtual and 
its
-  // overriddens, primary template and all explicit specializations.

you removed this comment and the fixme, but it still applies?



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:732
+  {
+// Constructor.
+  R"cpp(

nit: put these after "class methods" rather than at the top?



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:750
+{
+  // Destructor (selecting before the identifier).
+  R"cpp(

4 is too many cases for constructor/destructor.

What about 2: Constructor at start, destructor at end?
Renaming in the middle of the token is covered by existing cases.


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

https://reviews.llvm.org/D71247



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

[clang-tools-extra] 471d9f3 - [clangd] Fix windows tests

2019-12-12 Thread Kadir Cetinkaya via cfe-commits


Author: Kadir Cetinkaya
Date: 2019-12-12T12:54:08+01:00
New Revision: 471d9f3e698108da096bfcd85ac96e2eacda509b

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

LOG: [clangd] Fix windows tests

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp 
b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 3cde2948e369..783599ad7ac0 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -531,6 +531,7 @@ void foo())cpp";
 Annotations T(Case.Code);
 TestTU TU = TestTU::withCode(T.code());
 TU.ExtraArgs.push_back("-std=c++17");
+TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
 auto AST = TU.build();
 ASSERT_TRUE(AST.getDiagnostics().empty());
 



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

[PATCH] D57732: Correct inf typo

2019-12-12 Thread Andrew Gaul via Phabricator via cfe-commits

gaul added a comment.

In D57732#1781437 , @Jim wrote:

> Do you need someone to commit this change for you?


I assume so -- this is my first PR for clang and don't know what the procedure 
is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57732



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

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

2019-12-12 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 60762 tests passed, 1 failed 
and 726 were skipped.

  failed: Clang.CodeGenCXX/runtime-dllstorage.cpp

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
console-log.txt 
,
 CMakeCache.txt 
,
 test-results.xml 
,
 diff.json 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71248



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

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

2019-12-12 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: clang-tools-extra/clangd/FormattedString.cpp:164
   }
-  return R;
+  OS << '\n';
 }

this is worth a comment - we translate Paragraphs into markdown lines, not 
markdown paragraphs



Comment at: clang-tools-extra/clangd/FormattedString.cpp:199
+Paragraph &Document::addParagraph() {
+  Children.emplace_back(std::make_unique());
+  return *static_cast(Children.back().get());

push_back (and below)



Comment at: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp:21
 
-TEST(FormattedString, Basic) {
-  FormattedString S;
-  EXPECT_EQ(S.renderAsPlainText(), "");
-  EXPECT_EQ(S.renderAsMarkdown(), "");
+enum RenderKind {
+  PlainText,

dead?



Comment at: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp:79
+  {
+  Test::PlainText,
+  "after",

I'd consider writing this as
`[&] { Para.appendText("after"); }` to make it clearer what's being tested.

Are you sure the table test is clearer than straight-line code incrementally 
building the paragrap/asserting here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71248



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

[clang] 3f8da5d - [Tooling/Syntax] Helpers to find spelled tokens touching a location.

2019-12-12 Thread Sam McCall via cfe-commits


Author: Sam McCall
Date: 2019-12-12T12:59:50+01:00
New Revision: 3f8da5d0910772dc1f6198916a9141bf1d5be885

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

LOG: [Tooling/Syntax] Helpers to find spelled tokens touching a location.

Summary: Useful when positions are used to target nodes, with before/after 
ambiguity.

Reviewers: ilya-biryukov, kbobyrev

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/include/clang/Basic/SourceLocation.h
clang/include/clang/Tooling/Syntax/Tokens.h
clang/lib/Tooling/Syntax/Tokens.cpp
clang/unittests/Tooling/Syntax/TokensTest.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/SourceLocation.h 
b/clang/include/clang/Basic/SourceLocation.h
index d6e2f6e6de94..4e569a1d0d1c 100644
--- a/clang/include/clang/Basic/SourceLocation.h
+++ b/clang/include/clang/Basic/SourceLocation.h
@@ -188,9 +188,20 @@ inline bool operator!=(const SourceLocation &LHS, const 
SourceLocation &RHS) {
   return !(LHS == RHS);
 }
 
+// Ordering is meaningful only if LHS and RHS have the same FileID!
+// Otherwise use SourceManager::isBeforeInTranslationUnit().
 inline bool operator<(const SourceLocation &LHS, const SourceLocation &RHS) {
   return LHS.getRawEncoding() < RHS.getRawEncoding();
 }
+inline bool operator>(const SourceLocation &LHS, const SourceLocation &RHS) {
+  return LHS.getRawEncoding() > RHS.getRawEncoding();
+}
+inline bool operator<=(const SourceLocation &LHS, const SourceLocation &RHS) {
+  return LHS.getRawEncoding() <= RHS.getRawEncoding();
+}
+inline bool operator>=(const SourceLocation &LHS, const SourceLocation &RHS) {
+  return LHS.getRawEncoding() >= RHS.getRawEncoding();
+}
 
 /// A trivial tuple used to represent a source range.
 class SourceRange {

diff  --git a/clang/include/clang/Tooling/Syntax/Tokens.h 
b/clang/include/clang/Tooling/Syntax/Tokens.h
index 6f4d0e0c050a..0dcb6a1b2c9f 100644
--- a/clang/include/clang/Tooling/Syntax/Tokens.h
+++ b/clang/include/clang/Tooling/Syntax/Tokens.h
@@ -309,6 +309,17 @@ class TokenBuffer {
   const SourceManager *SourceMgr;
 };
 
+/// The spelled tokens that overlap or touch a spelling location Loc.
+/// This always returns 0-2 tokens.
+llvm::ArrayRef
+spelledTokensTouching(SourceLocation Loc, const syntax::TokenBuffer &Tokens);
+
+/// The identifier token that overlaps or touches a spelling location Loc.
+/// If there is none, returns nullptr.
+const syntax::Token *
+spelledIdentifierTouching(SourceLocation Loc,
+  const syntax::TokenBuffer &Tokens);
+
 /// Lex the text buffer, corresponding to \p FID, in raw mode and record the
 /// resulting spelled tokens. Does minimal post-processing on raw identifiers,
 /// setting the appropriate token kind (instead of the raw_identifier reported

diff  --git a/clang/lib/Tooling/Syntax/Tokens.cpp 
b/clang/lib/Tooling/Syntax/Tokens.cpp
index 5941507e086d..7c7a442dff03 100644
--- a/clang/lib/Tooling/Syntax/Tokens.cpp
+++ b/clang/lib/Tooling/Syntax/Tokens.cpp
@@ -248,6 +248,31 @@ TokenBuffer::expansionStartingAt(const syntax::Token 
*Spelled) const {
   return E;
 }
 
+llvm::ArrayRef
+syntax::spelledTokensTouching(SourceLocation Loc,
+  const syntax::TokenBuffer &Tokens) {
+  assert(Loc.isFileID());
+  llvm::ArrayRef All =
+  Tokens.spelledTokens(Tokens.sourceManager().getFileID(Loc));
+  // Comparing SourceLocations is well-defined within a FileID.
+  auto *Right = llvm::partition_point(
+  All, [&](const syntax::Token &Tok) { return Tok.location() < Loc; });
+  bool AcceptRight = Right != All.end() && Right->location() <= Loc;
+  bool AcceptLeft = Right != All.begin() && (Right - 1)->endLocation() >= Loc;
+  return llvm::makeArrayRef(Right - (AcceptLeft ? 1 : 0),
+Right + (AcceptRight ? 1 : 0));
+}
+
+const syntax::Token *
+syntax::spelledIdentifierTouching(SourceLocation Loc,
+  const syntax::TokenBuffer &Tokens) {
+  for (const syntax::Token &Tok : spelledTokensTouching(Loc, Tokens)) {
+if (Tok.kind() == tok::identifier)
+  return &Tok;
+  }
+  return nullptr;
+}
+
 std::vector
 TokenBuffer::macroExpansions(FileID FID) const {
   auto FileIt = Files.find(FID);

diff  --git a/clang/unittests/Tooling/Syntax/TokensTest.cpp 
b/clang/unittests/Tooling/Syntax/TokensTest.cpp
index 2c462d49ee41..e440993aac52 100644
--- a/clang/unittests/Tooling/Syntax/TokensTest.cpp
+++ b/clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -793,4 +793,45 @@ TEST_F(TokenBufferTest, macroExpansions) {
 ActualMacroRanges.push_back(Expansion->range(SM));
   EXPECT_EQ(ExpectedMacroRanges, ActualMacroRanges);
 }
+
+TEST_F(TokenBufferTest, Touching) {
+  llvm::Annotations Code("^

[PATCH] D71247: [clangd] Rename constructors and destructors in cross-file case

2019-12-12 Thread Kirill Bobyrev via Phabricator via cfe-commits

kbobyrev updated this revision to Diff 233571.
kbobyrev marked 4 inline comments as done.

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

https://reviews.llvm.org/D71247

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

Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -487,11 +487,10 @@
   "not a supported kind", HeaderFile, Index},
 
   {
-
   R"cpp(
 struct X { X operator++(int); };
 void f(X x) {x+^+;})cpp",
-  "not a supported kind", HeaderFile, Index},
+  "no symbol", HeaderFile, Index},
 
   {R"cpp(// foo is declared outside the file.
 void fo^o() {}
@@ -720,24 +719,24 @@
   MockCompilationDatabase CDB;
   CDB.ExtraClangFlags = {"-xc++"};
   class IgnoreDiagnostics : public DiagnosticsConsumer {
-  void onDiagnosticsReady(PathRef File,
-  std::vector Diagnostics) override {}
+void onDiagnosticsReady(PathRef File,
+std::vector Diagnostics) override {}
   } DiagConsumer;
   // rename is runnning on the "^" point in FooH, and "[[]]" ranges are the
-  // expcted rename occurrences.
+  // expected rename occurrences.
   struct Case {
 llvm::StringRef FooH;
 llvm::StringRef FooCC;
-  } Cases [] = {
-{
-  // classes.
-  R"cpp(
+  } Cases[] = {
+  {
+  // classes.
+  R"cpp(
 class [[Fo^o]] {
   [[Foo]]();
   ~[[Foo]]();
 };
   )cpp",
-  R"cpp(
+  R"cpp(
 #include "foo.h"
 [[Foo]]::[[Foo]]() {}
 [[Foo]]::~[[Foo]]() {}
@@ -746,15 +745,15 @@
   [[Foo]] foo;
 }
   )cpp",
-},
-{
-  // class methods.
-  R"cpp(
+  },
+  {
+  // class methods.
+  R"cpp(
 class Foo {
   void [[f^oo]]();
 };
   )cpp",
-  R"cpp(
+  R"cpp(
 #include "foo.h"
 void Foo::[[foo]]() {}
 
@@ -762,13 +761,49 @@
   p->[[foo]]();
 }
   )cpp",
-},
-{
-  // functions.
-  R"cpp(
+  },
+  {
+  // Constructor.
+  R"cpp(
+class [[Foo]] {
+  [[^Foo]]();
+  ~[[Foo]]();
+};
+  )cpp",
+  R"cpp(
+#include "foo.h"
+[[Foo]]::[[Foo]]() {}
+[[Foo]]::~[[Foo]]() {}
+
+void func() {
+  [[Foo]] foo;
+}
+  )cpp",
+  },
+  {
+  // Destructor (selecting before the identifier).
+  R"cpp(
+class [[Foo]] {
+  [[Foo]]();
+  ~[[Foo^]]();
+};
+  )cpp",
+  R"cpp(
+#include "foo.h"
+[[Foo]]::[[Foo]]() {}
+[[Foo]]::~[[Foo]]() {}
+
+void func() {
+  [[Foo]] foo;
+}
+  )cpp",
+  },
+  {
+  // functions.
+  R"cpp(
 void [[f^oo]]();
   )cpp",
-  R"cpp(
+  R"cpp(
 #include "foo.h"
 void [[foo]]() {}
 
@@ -776,63 +811,63 @@
   [[foo]]();
 }
   )cpp",
-},
-{
-  // typedefs.
-  R"cpp(
+  },
+  {
+  // typedefs.
+  R"cpp(
   typedef int [[IN^T]];
   [[INT]] foo();
   )cpp",
-  R"cpp(
+  R"cpp(
 #include "foo.h"
 [[INT]] foo() {}
   )cpp",
-},
-{
-  // usings.
-  R"cpp(
+  },
+  {
+  // usings.
+  R"cpp(
   using [[I^NT]] = int;
   [[INT]] foo();
   )cpp",
-  R"cpp(
+  R"cpp(
 #include "foo.h"
 [[INT]] foo() {}
   )cpp",
-},
-{
-  // variables.
-  R"cpp(
+  },
+  {
+  // variables.
+  R"cpp(
   static const int [[VA^R]] = 123;
   )cpp",
-  R"cpp(
+  R"cpp(
 #include "foo.h"
 int s = [[VAR]];
   )cpp",
-},
-{
-  // scope enums.
-  R"cpp(
+  },
+  {
+  // scope enums.
+  R"cpp(
   enum class [[K^ind]] { ABC };
   )cpp",
-  R"cpp(
+  R"cpp(
 #include "foo.h"
 [[Kind]] ff() {
   return [[Kind]]::ABC;
 }
   )cpp",
-},
-{
-  // enum constants.
-  R"cpp(
+  },
+  {
+  // enum constants.
+  R"cpp(
   enum class Kind { [[A^BC]] };
   )cpp",
-  R"cpp(
+  R"cpp(
 #include "foo.h"
 Kind ff() {
   return Kind::[[ABC]];
 }
   )cpp",
-},
+  },
   };
 
   for (const auto& T : Cases) {
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -18

[PATCH] D71356: [Tooling/Syntax] Helpers to find spelled tokens touching a location.

2019-12-12 Thread Sam McCall via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
sammccall marked 2 inline comments as done.
Closed by commit rG3f8da5d09107: [Tooling/Syntax] Helpers to find spelled 
tokens touching a location. (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71356

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

Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- clang/unittests/Tooling/Syntax/TokensTest.cpp
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -793,4 +793,45 @@
 ActualMacroRanges.push_back(Expansion->range(SM));
   EXPECT_EQ(ExpectedMacroRanges, ActualMacroRanges);
 }
+
+TEST_F(TokenBufferTest, Touching) {
+  llvm::Annotations Code("^i^nt^ ^a^b^=^1;^");
+  recordTokens(Code.code());
+
+  auto Touching = [&](int Index) {
+SourceLocation Loc = SourceMgr->getComposedLoc(SourceMgr->getMainFileID(),
+   Code.points()[Index]);
+return spelledTokensTouching(Loc, Buffer);
+  };
+  auto Identifier = [&](int Index) {
+SourceLocation Loc = SourceMgr->getComposedLoc(SourceMgr->getMainFileID(),
+   Code.points()[Index]);
+const syntax::Token *Tok = spelledIdentifierTouching(Loc, Buffer);
+return Tok ? Tok->text(*SourceMgr) : "";
+  };
+
+  EXPECT_THAT(Touching(0), SameRange(findSpelled("int")));
+  EXPECT_EQ(Identifier(0), "");
+  EXPECT_THAT(Touching(1), SameRange(findSpelled("int")));
+  EXPECT_EQ(Identifier(1), "");
+  EXPECT_THAT(Touching(2), SameRange(findSpelled("int")));
+  EXPECT_EQ(Identifier(2), "");
+
+  EXPECT_THAT(Touching(3), SameRange(findSpelled("ab")));
+  EXPECT_EQ(Identifier(3), "ab");
+  EXPECT_THAT(Touching(4), SameRange(findSpelled("ab")));
+  EXPECT_EQ(Identifier(4), "ab");
+
+  EXPECT_THAT(Touching(5), SameRange(findSpelled("ab =")));
+  EXPECT_EQ(Identifier(5), "ab");
+
+  EXPECT_THAT(Touching(6), SameRange(findSpelled("= 1")));
+  EXPECT_EQ(Identifier(6), "");
+
+  EXPECT_THAT(Touching(7), SameRange(findSpelled(";")));
+  EXPECT_EQ(Identifier(7), "");
+
+  ASSERT_EQ(Code.points().size(), 8u);
+}
+
 } // namespace
Index: clang/lib/Tooling/Syntax/Tokens.cpp
===
--- clang/lib/Tooling/Syntax/Tokens.cpp
+++ clang/lib/Tooling/Syntax/Tokens.cpp
@@ -248,6 +248,31 @@
   return E;
 }
 
+llvm::ArrayRef
+syntax::spelledTokensTouching(SourceLocation Loc,
+  const syntax::TokenBuffer &Tokens) {
+  assert(Loc.isFileID());
+  llvm::ArrayRef All =
+  Tokens.spelledTokens(Tokens.sourceManager().getFileID(Loc));
+  // Comparing SourceLocations is well-defined within a FileID.
+  auto *Right = llvm::partition_point(
+  All, [&](const syntax::Token &Tok) { return Tok.location() < Loc; });
+  bool AcceptRight = Right != All.end() && Right->location() <= Loc;
+  bool AcceptLeft = Right != All.begin() && (Right - 1)->endLocation() >= Loc;
+  return llvm::makeArrayRef(Right - (AcceptLeft ? 1 : 0),
+Right + (AcceptRight ? 1 : 0));
+}
+
+const syntax::Token *
+syntax::spelledIdentifierTouching(SourceLocation Loc,
+  const syntax::TokenBuffer &Tokens) {
+  for (const syntax::Token &Tok : spelledTokensTouching(Loc, Tokens)) {
+if (Tok.kind() == tok::identifier)
+  return &Tok;
+  }
+  return nullptr;
+}
+
 std::vector
 TokenBuffer::macroExpansions(FileID FID) const {
   auto FileIt = Files.find(FID);
Index: clang/include/clang/Tooling/Syntax/Tokens.h
===
--- clang/include/clang/Tooling/Syntax/Tokens.h
+++ clang/include/clang/Tooling/Syntax/Tokens.h
@@ -309,6 +309,17 @@
   const SourceManager *SourceMgr;
 };
 
+/// The spelled tokens that overlap or touch a spelling location Loc.
+/// This always returns 0-2 tokens.
+llvm::ArrayRef
+spelledTokensTouching(SourceLocation Loc, const syntax::TokenBuffer &Tokens);
+
+/// The identifier token that overlaps or touches a spelling location Loc.
+/// If there is none, returns nullptr.
+const syntax::Token *
+spelledIdentifierTouching(SourceLocation Loc,
+  const syntax::TokenBuffer &Tokens);
+
 /// Lex the text buffer, corresponding to \p FID, in raw mode and record the
 /// resulting spelled tokens. Does minimal post-processing on raw identifiers,
 /// setting the appropriate token kind (instead of the raw_identifier reported
Index: clang/include/clang/Basic/SourceLocation.h
===
--- clang/include/clang/Basic/SourceLocation.h
+++ clang/include/clang/Basic/SourceLocation.h
@@ -188,9 +188,20 @@
   return !(LHS ==

[clang-tools-extra] ec61882 - [clangd] Rename constructors and destructors in cross-file case

2019-12-12 Thread Kirill Bobyrev via cfe-commits


Author: Kirill Bobyrev
Date: 2019-12-12T13:10:59+01:00
New Revision: ec618826dfb91c5413353ebcc54f360e43df10a0

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

LOG: [clangd] Rename constructors and destructors in cross-file case

* Use ad-hoc Decl canonicalization from Clang-Rename to allow renaming
  constructors and destructors while using cross-file rename.
* Manually handle the destructor selection
* Add unit tests to prevent regressions and ensure the correct behaviour

Reviewed by: sammccall

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

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/refactor/Rename.cpp 
b/clang-tools-extra/clangd/refactor/Rename.cpp
index 8ca90afba69a..010f7e388b55 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -18,8 +18,10 @@
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Tooling/Refactoring/Rename/USRFindingAction.h"
+#include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FormatVariadic.h"
 #include 
@@ -83,21 +85,17 @@ llvm::DenseSet locateDeclAt(ParsedAST &AST,
   if (!SelectedNode)
 return {};
 
-  // If the location points to a Decl, we check it is actually on the name
-  // range of the Decl. This would avoid allowing rename on unrelated tokens.
-  //   ^class Foo {} // SelectionTree returns CXXRecordDecl,
-  // // we don't attempt to trigger rename on this position.
-  // FIXME: Make this work on destructors, e.g. "~F^oo()".
-  if (const auto *D = SelectedNode->ASTNode.get()) {
-if (D->getLocation() != TokenStartLoc)
-  return {};
-  }
-
   llvm::DenseSet Result;
   for (const auto *D :
targetDecl(SelectedNode->ASTNode,
-  DeclRelation::Alias | DeclRelation::TemplatePattern))
-Result.insert(D);
+  DeclRelation::Alias | DeclRelation::TemplatePattern)) {
+const auto *ND = llvm::dyn_cast(D);
+if (!ND)
+  continue;
+// Get to CXXRecordDecl from constructor or destructor.
+ND = tooling::getCanonicalSymbolDeclaration(ND);
+Result.insert(ND);
+  }
   return Result;
 }
 
@@ -214,17 +212,16 @@ llvm::Error makeError(ReasonToReject Reason) {
 // Return all rename occurrences in the main file.
 std::vector findOccurrencesWithinFile(ParsedAST &AST,
   const NamedDecl &ND) {
-  // In theory, locateDeclAt should return the primary template. However, if 
the
-  // cursor is under the underlying CXXRecordDecl of the ClassTemplateDecl, ND
-  // will be the CXXRecordDecl, for this case, we need to get the primary
-  // template maunally.
-  const auto &RenameDecl =
-  ND.getDescribedTemplate() ? *ND.getDescribedTemplate() : ND;
+  // If the cursor is at the underlying CXXRecordDecl of the
+  // ClassTemplateDecl, ND will be the CXXRecordDecl. In this case, we need to
+  // get the primary template maunally.
   // getUSRsForDeclaration will find other related symbols, e.g. virtual and 
its
   // overriddens, primary template and all explicit specializations.
   // FIXME: Get rid of the remaining tooling APIs.
-  std::vector RenameUSRs = tooling::getUSRsForDeclaration(
-  tooling::getCanonicalSymbolDeclaration(&RenameDecl), 
AST.getASTContext());
+  const auto RenameDecl =
+  ND.getDescribedTemplate() ? ND.getDescribedTemplate() : &ND;
+  std::vector RenameUSRs =
+  tooling::getUSRsForDeclaration(RenameDecl, AST.getASTContext());
   llvm::DenseSet TargetIDs;
   for (auto &USR : RenameUSRs)
 TargetIDs.insert(SymbolID(USR));
@@ -455,14 +452,21 @@ llvm::Expected rename(const RenameInputs 
&RInputs) {
 
 return (*Content)->getBuffer().str();
   };
-  SourceLocation SourceLocationBeg = SM.getMacroArgExpandedLocation(
-  getBeginningOfIdentifier(RInputs.Pos, SM, AST.getLangOpts()));
+  // Try to find the tokens adjacent to the cursor position.
+  auto Loc = sourceLocationInMainFile(SM, RInputs.Pos);
+  if (!Loc)
+return Loc.takeError();
+  const syntax::Token *IdentifierToken =
+  spelledIdentifierTouching(*Loc, AST.getTokens());
+  // Renames should only triggered on identifiers.
+  if (!IdentifierToken)
+return makeError(ReasonToReject::NoSymbolFound);
   // FIXME: Renaming macros is not supported yet, the macro-handling code 
should
   // be moved to rename tooling library.
-  if (locateMacroAt(SourceLocationBeg, AST.getPreprocessor()))
+  if (locateMacroAt(IdentifierToken->location(), AST.getPreprocess

[PATCH] D71133: [OpenCL] Add ExtVectorElementExpr constant evaluation (PR42387)

2019-12-12 Thread Anastasia Stulova via Phabricator via cfe-commits

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

LGTM! Thanks!




Comment at: clang/lib/AST/ExprConstant.cpp:7057
+
+if (Val.isVector()) {
+  SmallVector Indices;

svenvh wrote:
> Anastasia wrote:
> > can Val not be vector at this point?
> Yes, for example when evaluating `const int2 fromConstexprFunc = 
> addOne(int2(2));` from the included test case we get an LValue (that won't be 
> folded by the code below).
I see. Would it be folded elsewhere or not folded at all?


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

https://reviews.llvm.org/D71133



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

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

2019-12-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp:79
+  {
+  Test::PlainText,
+  "after",

sammccall wrote:
> I'd consider writing this as
> `[&] { Para.appendText("after"); }` to make it clearer what's being tested.
> 
> Are you sure the table test is clearer than straight-line code incrementally 
> building the paragrap/asserting here?
this test was huge in the beginning and having a straight line of appends were 
making it really hard to reason with. but it seems leaner now, as it doesn't 
have codeblocks anymore.
switching back to linear testing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71248



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

[PATCH] D71247: [clangd] Rename constructors and destructors in cross-file case

2019-12-12 Thread Kirill Bobyrev via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGec618826dfb9: [clangd] Rename constructors and destructors 
in cross-file case (authored by kbobyrev).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71247

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

Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -487,11 +487,10 @@
   "not a supported kind", HeaderFile, Index},
 
   {
-
   R"cpp(
 struct X { X operator++(int); };
 void f(X x) {x+^+;})cpp",
-  "not a supported kind", HeaderFile, Index},
+  "no symbol", HeaderFile, Index},
 
   {R"cpp(// foo is declared outside the file.
 void fo^o() {}
@@ -720,24 +719,24 @@
   MockCompilationDatabase CDB;
   CDB.ExtraClangFlags = {"-xc++"};
   class IgnoreDiagnostics : public DiagnosticsConsumer {
-  void onDiagnosticsReady(PathRef File,
-  std::vector Diagnostics) override {}
+void onDiagnosticsReady(PathRef File,
+std::vector Diagnostics) override {}
   } DiagConsumer;
   // rename is runnning on the "^" point in FooH, and "[[]]" ranges are the
-  // expcted rename occurrences.
+  // expected rename occurrences.
   struct Case {
 llvm::StringRef FooH;
 llvm::StringRef FooCC;
-  } Cases [] = {
-{
-  // classes.
-  R"cpp(
+  } Cases[] = {
+  {
+  // classes.
+  R"cpp(
 class [[Fo^o]] {
   [[Foo]]();
   ~[[Foo]]();
 };
   )cpp",
-  R"cpp(
+  R"cpp(
 #include "foo.h"
 [[Foo]]::[[Foo]]() {}
 [[Foo]]::~[[Foo]]() {}
@@ -746,15 +745,15 @@
   [[Foo]] foo;
 }
   )cpp",
-},
-{
-  // class methods.
-  R"cpp(
+  },
+  {
+  // class methods.
+  R"cpp(
 class Foo {
   void [[f^oo]]();
 };
   )cpp",
-  R"cpp(
+  R"cpp(
 #include "foo.h"
 void Foo::[[foo]]() {}
 
@@ -762,13 +761,49 @@
   p->[[foo]]();
 }
   )cpp",
-},
-{
-  // functions.
-  R"cpp(
+  },
+  {
+  // Constructor.
+  R"cpp(
+class [[Foo]] {
+  [[^Foo]]();
+  ~[[Foo]]();
+};
+  )cpp",
+  R"cpp(
+#include "foo.h"
+[[Foo]]::[[Foo]]() {}
+[[Foo]]::~[[Foo]]() {}
+
+void func() {
+  [[Foo]] foo;
+}
+  )cpp",
+  },
+  {
+  // Destructor (selecting before the identifier).
+  R"cpp(
+class [[Foo]] {
+  [[Foo]]();
+  ~[[Foo^]]();
+};
+  )cpp",
+  R"cpp(
+#include "foo.h"
+[[Foo]]::[[Foo]]() {}
+[[Foo]]::~[[Foo]]() {}
+
+void func() {
+  [[Foo]] foo;
+}
+  )cpp",
+  },
+  {
+  // functions.
+  R"cpp(
 void [[f^oo]]();
   )cpp",
-  R"cpp(
+  R"cpp(
 #include "foo.h"
 void [[foo]]() {}
 
@@ -776,63 +811,63 @@
   [[foo]]();
 }
   )cpp",
-},
-{
-  // typedefs.
-  R"cpp(
+  },
+  {
+  // typedefs.
+  R"cpp(
   typedef int [[IN^T]];
   [[INT]] foo();
   )cpp",
-  R"cpp(
+  R"cpp(
 #include "foo.h"
 [[INT]] foo() {}
   )cpp",
-},
-{
-  // usings.
-  R"cpp(
+  },
+  {
+  // usings.
+  R"cpp(
   using [[I^NT]] = int;
   [[INT]] foo();
   )cpp",
-  R"cpp(
+  R"cpp(
 #include "foo.h"
 [[INT]] foo() {}
   )cpp",
-},
-{
-  // variables.
-  R"cpp(
+  },
+  {
+  // variables.
+  R"cpp(
   static const int [[VA^R]] = 123;
   )cpp",
-  R"cpp(
+  R"cpp(
 #include "foo.h"
 int s = [[VAR]];
   )cpp",
-},
-{
-  // scope enums.
-  R"cpp(
+  },
+  {
+  // scope enums.
+  R"cpp(
   enum class [[K^ind]] { ABC };
   )cpp",
-  R"cpp(
+  R"cpp(
 #include "foo.h"
 [[Kind]] ff() {
   return [[Kind]]::ABC;
 }
   )cpp",
-},
-{
-  // enum constants.
-  R"cpp(
+  },
+  {
+  // enum constants.
+  R"cpp(
   enum class Kind { [[A^BC]] };
   )cpp",
-  R"cpp(
+  R"cpp(
 #include "foo.h"
 Kind ff() {
   return Kind::[[ABC]];
 }
   )cpp",
-},
+  },
   };
 
   for (const auto& T : Cases) {
Index: clang-tools-extra/clangd/refactor/Rename.

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

2019-12-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet updated this revision to Diff 233576.
kadircet marked 5 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71248

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/FormattedString.cpp
  clang-tools-extra/clangd/FormattedString.h
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/Hover.h
  clang-tools-extra/clangd/unittests/FormattedStringTests.cpp

Index: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
===
--- clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
+++ clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
@@ -8,192 +8,109 @@
 #include "FormattedString.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/StringRef.h"
-
+#include "llvm/Support/raw_ostream.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace clangd {
+namespace markup {
 namespace {
 
-TEST(FormattedString, Basic) {
-  FormattedString S;
-  EXPECT_EQ(S.renderAsPlainText(), "");
-  EXPECT_EQ(S.renderAsMarkdown(), "");
-
-  S.appendText("foobar  ");
-  S.appendText("baz");
-  EXPECT_EQ(S.renderAsPlainText(), "foobar baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "foobar  baz");
-
-  S = FormattedString();
-  S.appendInlineCode("foobar");
-  EXPECT_EQ(S.renderAsPlainText(), "foobar");
-  EXPECT_EQ(S.renderAsMarkdown(), "`foobar`");
-
-  S = FormattedString();
-  S.appendCodeBlock("foobar");
-  EXPECT_EQ(S.renderAsPlainText(), "foobar");
-  EXPECT_EQ(S.renderAsMarkdown(), "```cpp\n"
-  "foobar\n"
-  "```\n");
-}
-
-TEST(FormattedString, CodeBlocks) {
-  FormattedString S;
-  S.appendCodeBlock("foobar");
-  S.appendCodeBlock("bazqux", "javascript");
-  S.appendText("after");
-
-  std::string ExpectedText = R"(foobar
-
-bazqux
-
-after)";
-  EXPECT_EQ(S.renderAsPlainText(), ExpectedText);
-  std::string ExpectedMarkdown = R"md(```cpp
-foobar
-```
-```javascript
-bazqux
-```
-after)md";
-  EXPECT_EQ(S.renderAsMarkdown(), ExpectedMarkdown);
-
-  S = FormattedString();
-  S.appendInlineCode("foobar");
-  S.appendInlineCode("bazqux");
-  EXPECT_EQ(S.renderAsPlainText(), "foobar bazqux");
-  EXPECT_EQ(S.renderAsMarkdown(), "`foobar` `bazqux`");
-
-  S = FormattedString();
-  S.appendText("foo");
-  S.appendInlineCode("bar");
-  S.appendText("baz");
-
-  EXPECT_EQ(S.renderAsPlainText(), "foo bar baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "foo `bar` baz");
-}
-
-TEST(FormattedString, Escaping) {
+TEST(Render, Escaping) {
   // Check some ASCII punctuation
-  FormattedString S;
-  S.appendText("*!`");
-  EXPECT_EQ(S.renderAsMarkdown(), "\\*\\!\\`");
+  Paragraph P;
+  P.appendText("*!`");
+  EXPECT_EQ(P.asMarkdown(), "\\*\\!\\`");
 
   // Check all ASCII punctuation.
-  S = FormattedString();
+  P = Paragraph();
   std::string Punctuation = R"txt(!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~)txt";
   // Same text, with each character escaped.
   std::string EscapedPunctuation;
   EscapedPunctuation.reserve(2 * Punctuation.size());
   for (char C : Punctuation)
 EscapedPunctuation += std::string("\\") + C;
-  S.appendText(Punctuation);
-  EXPECT_EQ(S.renderAsMarkdown(), EscapedPunctuation);
+  P.appendText(Punctuation);
+  EXPECT_EQ(P.asMarkdown(), EscapedPunctuation);
 
   // In code blocks we don't need to escape ASCII punctuation.
-  S = FormattedString();
-  S.appendInlineCode("* foo !+ bar * baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "`* foo !+ bar * baz`");
-  S = FormattedString();
-  S.appendCodeBlock("#define FOO\n* foo !+ bar * baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "```cpp\n"
-  "#define FOO\n* foo !+ bar * baz\n"
-  "```\n");
+  P = Paragraph();
+  P.appendCode("* foo !+ bar * baz");
+  EXPECT_EQ(P.asMarkdown(), "`* foo !+ bar * baz`");
 
   // But we have to escape the backticks.
-  S = FormattedString();
-  S.appendInlineCode("foo`bar`baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "`foo``bar``baz`");
-
-  S = FormattedString();
-  S.appendCodeBlock("foo`bar`baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "```cpp\n"
-  "foo`bar`baz\n"
-  "```\n");
+  P = Paragraph();
+  P.appendCode("foo`bar`baz");
+  EXPECT_EQ(P.asMarkdown(), "`foo``bar``baz`");
 
   // Inline code blocks starting or ending with backticks should add spaces.
-  S = FormattedString();
-  S.appendInlineCode("`foo");
-  EXPECT_EQ(S.renderAsMarkdown(), "` ``foo `");
-  S = FormattedString();
-  S.appendInlineCode("foo`");
-  EXPECT_EQ(S.renderAsMarkdown(), "` foo`` `");
-  S = FormattedString();
-  S.appendInlineCode("`foo`");
-  EXPECT_EQ(S.renderAsMarkdown(), "` ``foo`` `");
-
-  // Should also add extra spaces if the block stars and ends with spaces.
-  S = FormattedString();

[PATCH] D71199: [clang-tidy][WIP] New check readability-prefer-initialization-list

2019-12-12 Thread Balogh, Ádám via Phabricator via cfe-commits

baloghadamsoftware added a comment.

In D71199#1777887 , @aaron.ballman 
wrote:

> In D71199#1775083 , @whisperity 
> wrote:
>
> > Can you refresh my memory on whether a rule for "if init expr is constant, 
> > initialise in class body instead" exists for init list members? If so, this 
> > will be a funny "two pass needed to fix" kind of check.
>
>
> This worries me as well -- we already have checks that prefer doing an 
> in-class initialization to using a constructor member initialization list. 
> How should this check interact with `modernize-use-default-member-init`?

This check is to enforce C++ core guideline C.49 while the modernize check 
enforces guideline C.48. The two must be synchronized, and I think that this 
new check should do that: for initializations that should  be done using 
in-class initializers according to the other checker this checker must suggest 
the same. For the rest we should suggest member initializers in the constructor.

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

https://reviews.llvm.org/D71199

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

[PATCH] D71111: [Sema] Improve diagnostic about addr spaces for overload candidates

2019-12-12 Thread Anastasia Stulova via Phabricator via cfe-commits

Anastasia updated this revision to Diff 233579.
Anastasia marked an inline comment as done.
Anastasia added a comment.

- Moved "address space" printing into diagnostic engine
- Moved `LangAS::Default` into switch/case statement.


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

https://reviews.llvm.org/D7

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/Diagnostic.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ASTDiagnostic.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Diagnostic.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaCXX/address-space-references.cpp
  clang/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
  clang/test/SemaOpenCLCXX/address-space-lambda.cl
  clang/test/SemaOpenCLCXX/address-space-of-this-class-scope.cl

Index: clang/test/SemaOpenCLCXX/address-space-of-this-class-scope.cl
===
--- clang/test/SemaOpenCLCXX/address-space-of-this-class-scope.cl
+++ clang/test/SemaOpenCLCXX/address-space-of-this-class-scope.cl
@@ -7,8 +7,8 @@
 };
 
 void bar(__local C*);
-// expected-note@-1{{candidate function not viable: address space mismatch in 1st argument ('decltype(this)' (aka '__global C *')), parameter type must be '__local C *'}}
-// expected-note@-2{{candidate function not viable: address space mismatch in 1st argument ('decltype(this)' (aka 'C *')), parameter type must be '__local C *'}}
+// expected-note@-1{{candidate function not viable: cannot pass pointer to address space '__global' as a pointer to address space '__local' in 1st argument}}
+// expected-note@-2{{candidate function not viable: cannot pass pointer to default address space as a pointer to address space '__local' in 1st argument}}
 
 __global C Glob;
 void foo(){
Index: clang/test/SemaOpenCLCXX/address-space-lambda.cl
===
--- clang/test/SemaOpenCLCXX/address-space-lambda.cl
+++ clang/test/SemaOpenCLCXX/address-space-lambda.cl
@@ -12,7 +12,7 @@
   // Test lambda with default parameters
 //CHECK: CXXMethodDecl {{.*}} constexpr operator() 'void () const __generic'
   [&] {i++;} ();
-  __constant auto err = [&]() {}; //expected-note-re{{candidate function not viable: address space mismatch in 'this' argument ('__constant (lambda at {{.*}})'), parameter type must be 'const __generic (lambda at {{.*}})'}}
+  __constant auto err = [&]() {}; //expected-note{{candidate function not viable: 'this' object is in address space '__constant', but method expects object in address space '__generic'}}
   err();  //expected-error-re{{no matching function for call to object of type '__constant (lambda at {{.*}})'}}
   // FIXME: There is very limited addr space functionality
   // we can test when taking lambda type from the object.
Index: clang/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
===
--- clang/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
+++ clang/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
@@ -42,7 +42,7 @@
 #if !__OPENCL_CPP_VERSION__
 // expected-note@-3{{passing argument to parameter 'arg_glob' here}}
 #else
-// expected-note-re@-5{{candidate function not viable: address space mismatch in 1st argument ('__{{generic|constant}} int *'), parameter type must be '__global int *'}}
+// expected-note-re@-5{{candidate function not viable: cannot pass pointer to address space '__{{generic|constant}}' as a pointer to address space '__global' in 1st argument}}
 #endif
 #endif
 
@@ -50,7 +50,7 @@
 #if !__OPENCL_CPP_VERSION__
 // expected-note@-2{{passing argument to parameter 'arg_loc' here}}
 #else
-// expected-note-re@-4{{candidate function not viable: address space mismatch in 1st argument ('__{{global|generic|constant}} int *'), parameter type must be '__local int *'}}
+// expected-note-re@-4{{candidate function not viable: cannot pass pointer to address space '__{{global|generic|constant}}' as a pointer to address space '__local' in 1st argument}}
 #endif
 
 void f_const(__constant int *arg_const) {}
@@ -58,7 +58,7 @@
 #if !__OPENCL_CPP_VERSION__
 // expected-note@-3{{passing argument to parameter 'arg_const' here}}
 #else
-// expected-note-re@-5{{candidate function not viable: address space mismatch in 1st argument ('__{{global|generic}} int *'), parameter type must be '__constant int *'}}
+// expected-note-re@-5{{candidate function not viable: cannot pass pointer to address space '__{{global|generic}}' as a pointer to address space '__constant' in 1st argument}}
 #endif
 #endif
 
@@ -66,7 +66,7 @@
 #if !__OPENCL_CPP_VERSION__
 // expected-note@-2{{passing argument to parameter 'arg_priv' here}}
 #else
-// expected-note-re@-4{{candidate function not viable: address space mismatch in 1st argument ('__{{global|generic|constant}} int *'), parameter type must be 'int *'}}
+// expected-note-re@-4{{candidate function not viable:

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

2019-12-12 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 60763 tests passed, 0 failed 
and 726 were skipped.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
console-log.txt 
,
 CMakeCache.txt 
,
 test-results.xml 
,
 diff.json 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71248



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

[PATCH] D71378: Modifying ImportDeclContext(...) to ensure that we complete each FieldDecl of a RecordDecl when we are importing the definiton

2019-12-12 Thread Raphael Isemann via Phabricator via cfe-commits

teemperor added a comment.

I wonder if we have a way to fix this from with LLDB. Having Clang code that is 
only tested in LLDB is always a bit weird.

Otherwise the idea itself LGTM, thanks for working on this (and reducing the 
test case to that!)




Comment at: clang/lib/AST/ASTImporter.cpp:1688
+// If we are in the process of ImportDefinition(...) for a RecordDecl we
+// want to make sure that we are also completeing each FieldDecl. There
+// are currently cases where this does not happen and this is correctness

*completing



Comment at: 
lldb/packages/Python/lldbsuite/test/commands/expression/codegen-crash-typedefdecl-not-in_declcontext/main.cpp:27
+   void g() {
+   return; //%self.expect("p b_ref", substrs=['(B) $0 =', '(spd = NS::DW', 
'a = 0)'])
+   }

You might want to explicitly call clang-format on this example as there are few 
indentation errors (this line, the `f()` declaration, line 20, etc.). We 
disabled clang-format for tests so that may require some workarounds 
(temporarily copying out of tree is the easiest probably)


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

https://reviews.llvm.org/D71378



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

[PATCH] D71197: llvm premerge: clang format test

2019-12-12 Thread Mikhail Goncharov via Phabricator via cfe-commits

goncharov added a comment.

{icon question-circle color=gray} Unit tests: unknown.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

Build artifacts : clang-tidy.txt 
, clang-format.patch 
, CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71197



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

[PATCH] D71197: llvm premerge: clang format test

2019-12-12 Thread Mikhail Goncharov via Phabricator via cfe-commits

goncharov added a comment.

{icon question-circle color=gray} Unit tests: unknown.

{icon check-circle color=green} clang-format: pass.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

Build artifacts : clang-tidy.txt 
, clang-format.patch 
, CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71197



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

[PATCH] D71356: [Tooling/Syntax] Helpers to find spelled tokens touching a location.

2019-12-12 Thread Ilya Biryukov via Phabricator via cfe-commits

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



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:270
+  for (const syntax::Token &Tok : spelledTokensTouching(Loc, Tokens))
+if (Tok.kind() == tok::identifier)
+  return &Tok;

sammccall wrote:
> ilya-biryukov wrote:
> > kbobyrev wrote:
> > > ilya-biryukov wrote:
> > > > sammccall wrote:
> > > > > ilya-biryukov wrote:
> > > > > > NIT: add braces around `if` statement
> > > > > Is there some reference for preferred LLVM style for this, or 
> > > > > personal preference? (Real question, as this comes up a bunch)
> > > > > 
> > > > > I ask because that's my *least*-preferred option - no braces > braces 
> > > > > on for > braces on both > braces on (only) if.
> > > > > 
> > > > > Added braces to the `for` (maybe that's what you meant?)
> > > > Not sure if it's in LLVM style guide, but files inside Syntax folder 
> > > > definitely use this style: put braces everywhere until you reach the 
> > > > last level of nesting for non-leaf statements (i.e. having other 
> > > > statements as children, e.g. loops,. if statements, etc)
> > > > 
> > > > 
> > > > It's my personal preference, happy to discuss whether having this makes 
> > > > sense.
> > > I guess it's a personal preference (also for me), but I don't think there 
> > > is a strict guideline on that. Interestingly enough, I think there is a 
> > > piece of code in the styleguide that looks exactly like the code you had: 
> > > https://llvm.org/docs/CodingStandards.html#turn-predicate-loops-into-predicate-functions
> > > 
> > > Some Clang subprojects tend to put braces everywhere though.
> > > 
> > > That being said, I guess no braces at all would be the best option here.
> > Yeah, so LLVM style is unclear, but has examples with no braces.
> > Could be the argument to get rid of those.
> > 
> > I'd still stick to my personal preferences where I can, but if everyone 
> > thinks no braces is better, happy to go with this option.
> I think we're good here - I'm fine with the braces on `for`, and it sounds 
> like that's what you wanted.
> (I guess I misread "braces around if statement" as "braces around if body")
Ah, great, we're on the same page then!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71356



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

[PATCH] D71413: [OpenMP][Test] Add check for aux-triple predefined macros

2019-12-12 Thread Alexey Bader via Phabricator via cfe-commits

bader created this revision.
bader added a reviewer: ABataev.
Herald added subscribers: cfe-commits, ebevhan, guansong.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.

Make sure that auxiliary target specific macros are defined in OpenMP
mode.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71413

Files:
  clang/test/OpenMP/aux-triple-macros.cpp


Index: clang/test/OpenMP/aux-triple-macros.cpp
===
--- /dev/null
+++ clang/test/OpenMP/aux-triple-macros.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 %s -triple nvptx64 -aux-triple x86_64-unknown-linux-gnu -E 
-dM | FileCheck %s
+// RUN: %clang_cc1 %s -fopenmp -fopenmp-is-device -triple nvptx64 -aux-triple 
x86_64-unknown-linux-gnu -E -dM | FileCheck --check-prefix=CHECK-OMP-DEVICE %s
+
+// CHECK-NOT:#define __x86_64__ 1
+// CHECK-OMP-DEVICE:#define __x86_64__ 1


Index: clang/test/OpenMP/aux-triple-macros.cpp
===
--- /dev/null
+++ clang/test/OpenMP/aux-triple-macros.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 %s -triple nvptx64 -aux-triple x86_64-unknown-linux-gnu -E -dM | FileCheck %s
+// RUN: %clang_cc1 %s -fopenmp -fopenmp-is-device -triple nvptx64 -aux-triple x86_64-unknown-linux-gnu -E -dM | FileCheck --check-prefix=CHECK-OMP-DEVICE %s
+
+// CHECK-NOT:#define __x86_64__ 1
+// CHECK-OMP-DEVICE:#define __x86_64__ 1
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71378: Modifying ImportDeclContext(...) to ensure that we complete each FieldDecl of a RecordDecl when we are importing the definiton

2019-12-12 Thread Gabor Marton via Phabricator via cfe-commits

martong added a comment.

Thanks for the patch! It look almost good to me, but I have a comment about the 
error handling.




Comment at: clang/lib/AST/ASTImporter.cpp:1707
+if (Err)
+  return Err;
+}

Rather than just simply returning with the Error, I think we should append this 
error to `ChildErrors` and go on with the next field, as this is what the 
original code did.
I am thinking about something like this:
```
  if (Err && AccumulateChildErrors)
ChildErrors =  joinErrors(std::move(ChildErrors), Err);
  else
consumeError(Err);
```

And perhaps line 1713 could be just a simple `else {`


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

https://reviews.llvm.org/D71378



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

[PATCH] D71197: llvm premerge: clang format test

2019-12-12 Thread Mikhail Goncharov via Phabricator via cfe-commits

goncharov added a comment.

{icon question-circle color=gray} Unit tests: unknown.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

Build artifacts : clang-tidy.txt 
, clang-format.patch 
, CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71197



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

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

2019-12-12 Thread serge via Phabricator via cfe-commits

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

@george.burgess.iv : take into account reviews, extra testing and function 
renaming.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71082

Files:
  clang/include/clang/AST/Decl.h
  clang/lib/AST/Decl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/memcpy-nobuiltin.c
  clang/test/CodeGen/memcpy-nobuiltin.inc

Index: clang/test/CodeGen/memcpy-nobuiltin.inc
===
--- /dev/null
+++ clang/test/CodeGen/memcpy-nobuiltin.inc
@@ -0,0 +1,19 @@
+#include 
+extern void *memcpy(void *dest, void const *from, size_t n);
+
+#ifdef WITH_DECL
+inline void *memcpy(void *dest, void const *from, size_t n) {
+  char const *ifrom = from;
+  char *idest = dest;
+  while (n--)
+*idest++ = *ifrom++;
+  return dest;
+}
+#endif
+#ifdef WITH_SELF_REFERENCE_DECL
+inline void *memcpy(void *dest, void const *from, size_t n) {
+  if (n != 0)
+memcpy(dest, from, n);
+  return dest;
+}
+#endif
Index: clang/test/CodeGen/memcpy-nobuiltin.c
===
--- /dev/null
+++ clang/test/CodeGen/memcpy-nobuiltin.c
@@ -0,0 +1,19 @@
+// RUN: %clang -S -emit-llvm -o- %s -isystem %S -DWITH_DECL |& FileCheck --check-prefix=CHECK-WITH-DECL %s
+// RUN: %clang -S -emit-llvm -o- %s -isystem %S -UWITH_DECL |& FileCheck --check-prefix=CHECK-NO-DECL %s
+// RUN: %clang -S -emit-llvm -o- %s -isystem %S -DWITH_SELF_REFERENCE_DECL |& FileCheck --check-prefix=CHECK-SELF-REF-DECL %s
+//
+// CHECK-WITH-DECL: 'memcpy' will always overflow; destination buffer has size 1, but size argument is 2
+// CHECK-WITH-DECL-NOT: @llvm.memcpy
+//
+// CHECK-NO-DECL: 'memcpy' will always overflow; destination buffer has size 1, but size argument is 2
+// CHECK-NO-DECL: @llvm.memcpy
+//
+// CHECK-SELF-REF-DECL: 'memcpy' will always overflow; destination buffer has size 1, but size argument is 2
+// CHECK-SELF-REF-DECL: @llvm.memcpy
+#include 
+void test(void *dest, void const *from, size_t n) {
+  memcpy(dest, from, n);
+
+  static char buffer[1];
+  memcpy(buffer, from, 2);
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1831,6 +1831,11 @@
   else if (const auto *SA = FD->getAttr())
  F->setSection(SA->getName());
 
+  if (FD->isInlineBuiltin()) {
+F->addAttribute(llvm::AttributeList::FunctionIndex,
+llvm::Attribute::NoBuiltin);
+  }
+
   if (FD->isReplaceableGlobalAllocationFunction()) {
 // A replaceable global allocation function does not act like a builtin by
 // default, only if it is invoked by a new-expression or delete-expression.
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -4596,8 +4596,14 @@
 }
 
 static CGCallee EmitDirectCallee(CodeGenFunction &CGF, const FunctionDecl *FD) {
+
   if (auto builtinID = FD->getBuiltinID()) {
-return CGCallee::forBuiltin(builtinID, FD);
+// Replaceable builtin provide their own implementation of a builtin. Unless
+// we are in the builtin implementation itself, don't call the actual
+// builtin. If we are in the builtin implementation, avoid trivial infinite
+// recursion.
+if (!FD->isInlineBuiltin() || CGF.CurFn->getName() == FD->getName())
+  return CGCallee::forBuiltin(builtinID, FD);
   }
 
   llvm::Constant *calleePtr = EmitFunctionDeclPointer(CGF.CGM, FD);
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -3003,6 +3003,24 @@
   return Params == FPT->getNumParams();
 }
 
+bool FunctionDecl::isInlineBuiltin() const {
+  if (!getBuiltinID())
+return false;
+
+  const FunctionDecl *Definition;
+  if (hasBody(Definition)) {
+
+if (!Definition->isInlineSpecified())
+  return false;
+
+const SourceManager &SM = getASTContext().getSourceManager();
+SourceLocation SL = Definition->getLocation();
+if (SL.isValid())
+  return SM.isInSystemHeader(SL);
+  }
+  return false;
+}
+
 bool FunctionDecl::isDestroyingOperatorDelete() const {
   // C++ P0722:
   //   Within a class C, a single object deallocation function with signature
Index: clang/include/clang/AST/Decl.h
===
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -2272,6 +2272,9 @@
   /// true through IsAligned.
   bool isReplaceableGlobalAllocationFunction(bool *IsAligned = nullptr) const;
 
+  /// Determine if this function provides an inline implementation of a builtin.
+  bool isInlineBuiltin()

[PATCH] D57747: [Sema] SequenceChecker: Fix handling of operator ||, && and ?:

2019-12-12 Thread Bruno Ricci via Phabricator via cfe-commits

riccibruno added a comment.

In D57747#1774848 , @xbolva00 wrote:

> Does the whole stack of patch need to be commited at once or maybe you can 
> land them individually?

Hi, thanks for looking at this patch series !

If I remember correctly (it has been a while now) these patches could be landed 
independently. I was sort of hoping that D57660 
 would get accepted and that I could then land 
the other patches without rebasing them (the tests are especially annoying to 
rebase). That said if D57660  is a problem 
then I can totally do the work and land the other already accepted patches.

Repository:
  rC Clang

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

https://reviews.llvm.org/D57747

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

[PATCH] D71378: Modifying ImportDeclContext(...) to ensure that we complete each FieldDecl of a RecordDecl when we are importing the definiton

2019-12-12 Thread Gabor Marton via Phabricator via cfe-commits

martong added a comment.

Just one more thing, maybe that is too overkill, but I think on a long term we 
could benefit from a unittest for this case. You could create a test similar to 
`LLDBLookupTest` in ASTImporterTest.cpp. In that Fixture we use Minimal import 
and the regular lookup (that is the exact case in LLDB).
In the test itself we could call ImportDefinition on a struct that has fields 
with record types. And then we could assert that all field's type's have 
complete types.


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

https://reviews.llvm.org/D71378



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

[PATCH] D71197: llvm premerge: clang format test

2019-12-12 Thread Mikhail Goncharov via Phabricator via cfe-commits

goncharov added a comment.

{icon question-circle color=gray} Unit tests: unknown.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

Build artifacts : clang-tidy.txt 
, clang-format.patch 
, CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71197



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

[PATCH] D57660: [Sema] SequenceChecker: Handle references, members and structured bindings.

2019-12-12 Thread Bruno Ricci via Phabricator via cfe-commits

riccibruno added a comment.

In D57660#1764337 , @Mordante wrote:

> I like this improvement. However I'm not a reviewer.


Thanks for looking at the patch!

> You can land some NFC changes in separate commit.

Yep, indeed. Will do when I rebase it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57660



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

[PATCH] D70819: [ASTImporter] Support functions with placeholder return types ...

2019-12-12 Thread Gabor Marton via Phabricator via cfe-commits

martong added a comment.

In D70819#170 , @shafik wrote:

> Apologies for wacky C++ code that follows but will this also work for the 
> following cases:
>
>   auto f2() { 
> auto l = []() {
> struct X{};
> return X();
> };
>return l(); 
>}
>  
>auto f3() { 
> if ( struct X{} x; true) 
> return X();
> else return X();
>}
>  
>auto f4() {
>   for(struct X{} x;;)
>  return X();
>}
>  
>auto f5() {
>   switch(struct X{} x; 10) {
> case 10:
>  return X();
>   }
>}
>
>
> godbolt live example 


Thanks for these cases! I am going to write unit tests for these as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70819



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

[PATCH] D71197: llvm premerge: clang format test

2019-12-12 Thread Mikhail Goncharov via Phabricator via cfe-commits

goncharov added a comment.

{icon question-circle color=gray} Unit tests: unknown.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts : clang-tidy.txt 
, clang-format.patch 
, CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71197



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

[PATCH] D71413: [OpenMP][Test] Add check for aux-triple predefined macros

2019-12-12 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 60761 tests passed, 1 failed 
and 726 were skipped.

  failed: Clang.CodeGenCXX/runtime-dllstorage.cpp

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
console-log.txt 
,
 CMakeCache.txt 
,
 test-results.xml 
,
 diff.json 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71413



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

[PATCH] D71374: Improve support of GNU mempcpy

2019-12-12 Thread serge via Phabricator via cfe-commits

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

@Jim obviously :-) Thanks for spotting that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71374

Files:
  clang/include/clang/Basic/Builtins.def
  clang/lib/AST/Decl.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Analysis/bstring.c

Index: clang/test/Analysis/bstring.c
===
--- clang/test/Analysis/bstring.c
+++ clang/test/Analysis/bstring.c
@@ -222,6 +222,9 @@
   char dst[1];
 
   mempcpy(dst, src, 4); // expected-warning{{Memory copy function overflows destination buffer}}
+#ifndef VARIANT
+// expected-warning@-2{{'mempcpy' will always overflow; destination buffer has size 1, but size argument is 4}}
+#endif
 }
 
 void mempcpy3 () {
@@ -243,6 +246,9 @@
   char dst[3];
 
   mempcpy(dst+2, src+2, 2); // expected-warning{{Memory copy function overflows destination buffer}}
+#ifndef VARIANT
+// expected-warning@-2{{'mempcpy' will always overflow; destination buffer has size 1, but size argument is 2}}
+#endif
 }
 
 void mempcpy6() {
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -340,7 +340,8 @@
   case Builtin::BI__builtin___strncat_chk:
   case Builtin::BI__builtin___strncpy_chk:
   case Builtin::BI__builtin___stpncpy_chk:
-  case Builtin::BI__builtin___memccpy_chk: {
+  case Builtin::BI__builtin___memccpy_chk:
+  case Builtin::BI__builtin___mempcpy_chk: {
 DiagID = diag::warn_builtin_chk_overflow;
 IsChkVariant = true;
 SizeIndex = TheCall->getNumArgs() - 2;
@@ -379,7 +380,9 @@
   case Builtin::BImemmove:
   case Builtin::BI__builtin_memmove:
   case Builtin::BImemset:
-  case Builtin::BI__builtin_memset: {
+  case Builtin::BI__builtin_memset:
+  case Builtin::BImempcpy:
+  case Builtin::BI__builtin_mempcpy: {
 DiagID = diag::warn_fortify_source_overflow;
 SizeIndex = TheCall->getNumArgs() - 1;
 ObjectIndex = 0;
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -2506,7 +2506,9 @@
 return RValue::get(nullptr);
   }
   case Builtin::BImemcpy:
-  case Builtin::BI__builtin_memcpy: {
+  case Builtin::BI__builtin_memcpy:
+  case Builtin::BImempcpy:
+  case Builtin::BI__builtin_mempcpy: {
 Address Dest = EmitPointerWithAlignment(E->getArg(0));
 Address Src = EmitPointerWithAlignment(E->getArg(1));
 Value *SizeVal = EmitScalarExpr(E->getArg(2));
@@ -2515,7 +2517,11 @@
 EmitNonNullArgCheck(RValue::get(Src.getPointer()), E->getArg(1)->getType(),
 E->getArg(1)->getExprLoc(), FD, 1);
 Builder.CreateMemCpy(Dest, Src, SizeVal, false);
-return RValue::get(Dest.getPointer());
+if (BuiltinID == Builtin::BImempcpy ||
+BuiltinID == Builtin::BI__builtin_mempcpy)
+  return RValue::get(Builder.CreateGEP(Dest.getPointer(), SizeVal));
+else
+  return RValue::get(Dest.getPointer());
   }
 
   case Builtin::BI__builtin_char_memchr:
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -3866,6 +3866,11 @@
   case Builtin::BImemcpy:
 return Builtin::BImemcpy;
 
+  case Builtin::BI__builtin_mempcpy:
+  case Builtin::BI__builtin___mempcpy_chk:
+  case Builtin::BImempcpy:
+return Builtin::BImempcpy;
+
   case Builtin::BI__builtin_memmove:
   case Builtin::BI__builtin___memmove_chk:
   case Builtin::BImemmove:
@@ -3923,6 +3928,8 @@
 return Builtin::BImemset;
   else if (FnInfo->isStr("memcpy"))
 return Builtin::BImemcpy;
+  else if (FnInfo->isStr("mempcpy"))
+return Builtin::BImempcpy;
   else if (FnInfo->isStr("memmove"))
 return Builtin::BImemmove;
   else if (FnInfo->isStr("memcmp"))
Index: clang/include/clang/Basic/Builtins.def
===
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -984,6 +984,7 @@
 LIBBUILTIN(alloca, "v*z", "f", "stdlib.h", ALL_GNU_LANGUAGES)
 // POSIX string.h
 LIBBUILTIN(memccpy, "v*v*vC*iz",  "f", "string.h", ALL_GNU_LANGUAGES)
+LIBBUILTIN(mempcpy, "v*v*vC*z",   "f", "string.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(stpcpy, "c*c*cC*", "f", "string.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(stpncpy, "c*c*cC*z",   "f", "string.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(strdup, "c*cC*",   "f", "string.h", ALL_GNU_LANGUAGES)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71320: [IR] Split out target specific intrinsic enums into separate headers

2019-12-12 Thread Reid Kleckner via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5d986953c8b9: [IR] Split out target specific intrinsic enums 
into separate headers (authored by rnk).

Changed prior to commit:
  https://reviews.llvm.org/D71320?vs=233231&id=233480#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71320

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGException.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  llvm/include/llvm/Analysis/TargetTransformInfo.h
  llvm/include/llvm/Analysis/VectorUtils.h
  llvm/include/llvm/IR/CMakeLists.txt
  llvm/include/llvm/IR/CallSite.h
  llvm/include/llvm/IR/Function.h
  llvm/include/llvm/IR/GlobalValue.h
  llvm/include/llvm/IR/InstrTypes.h
  llvm/include/llvm/IR/Intrinsics.h
  llvm/lib/Analysis/ConstantFolding.cpp
  llvm/lib/Analysis/MemoryLocation.cpp
  llvm/lib/Analysis/ValueTracking.cpp
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
  llvm/lib/CodeGen/TypePromotion.cpp
  llvm/lib/CodeGen/WasmEHPrepare.cpp
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/lib/IR/Function.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp
  llvm/lib/Target/AArch64/AArch64StackTagging.cpp
  llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
  llvm/lib/Target/AMDGPU/AMDGPU.h
  llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
  llvm/lib/Target/AMDGPU/R600ISelLowering.cpp
  llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
  llvm/lib/Target/ARM/ARMISelDAGToDAG.cpp
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/lib/Target/ARM/ARMInstructionSelector.cpp
  llvm/lib/Target/ARM/ARMParallelDSP.cpp
  llvm/lib/Target/ARM/MVETailPredication.cpp
  llvm/lib/Target/BPF/BPFISelDAGToDAG.cpp
  llvm/lib/Target/Hexagon/HexagonGenExtract.cpp
  llvm/lib/Target/Hexagon/HexagonISelDAGToDAG.cpp
  llvm/lib/Target/Hexagon/HexagonISelDAGToDAGHVX.cpp
  llvm/lib/Target/Hexagon/HexagonISelLowering.cpp
  llvm/lib/Target/Hexagon/HexagonLoopIdiomRecognition.cpp
  llvm/lib/Target/Hexagon/HexagonOptimizeSZextends.cpp
  llvm/lib/Target/Hexagon/HexagonVectorLoopCarriedReuse.cpp
  llvm/lib/Target/Mips/MipsInstructionSelector.cpp
  llvm/lib/Target/Mips/MipsLegalizerInfo.cpp
  llvm/lib/Target/Mips/MipsSEISelDAGToDAG.cpp
  llvm/lib/Target/Mips/MipsSEISelLowering.cpp
  llvm/lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp
  llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
  llvm/lib/Target/NVPTX/NVPTXImageOptimizer.cpp
  llvm/lib/Target/NVPTX/NVPTXTargetTransformInfo.cpp
  llvm/lib/Target/NVPTX/NVVMIntrRange.cpp
  llvm/lib/Target/NVPTX/NVVMReflect.cpp
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
  llvm/lib/Target/SystemZ/SystemZTDC.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/X86/X86FastISel.cpp
  llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
  llvm/lib/Target/X86/X86InstructionSelector.cpp
  llvm/lib/Target/X86/X86IntrinsicsInfo.h
  llvm/lib/Target/X86/X86WinEHState.cpp
  llvm/lib/Target/XCore/XCoreISelDAGToDAG.cpp
  llvm/lib/Target/XCore/XCoreISelLowering.cpp
  llvm/lib/Target/XCore/XCoreLowerThreadLocal.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
  llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
  llvm/test/TableGen/intrinsic-long-name.td
  llvm/test/TableGen/intrinsic-struct.td
  llvm/unittests/IR/IRBuilderTest.cpp
  llvm/utils/TableGen/IntrinsicEmitter.cpp

Index: llvm/utils/TableGen/IntrinsicEmitter.cpp
===
--- llvm/utils/TableGen/IntrinsicEmitter.cpp
+++ llvm/utils/TableGen/IntrinsicEmitter.cpp
@@ -15,14 +15,21 @@
 #include "SequenceToOffsetTable.h"
 #include "TableGenBackends.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/StringMatcher.h"
-#include "llvm/TableGen/TableGenBackend.h"
 #include "llvm/TableGen/StringToOffsetTable.h"
+#include "llvm/TableGen/TableGenBackend.h"
 #include 
 using namespace llvm;
 
+cl::OptionCategory GenIntrinsicCat("Options for -gen-intrinsic-enums");
+cl::opt
+IntrinsicPrefix("intrinsic-prefix",
+cl::desc("Generate intrinsics with this target prefix"),
+cl::value_desc("target prefix"), cl::cat(GenIntrinsicCat));
+
 namespace {
 class IntrinsicEmitter {
   RecordKeeper &Records;
@@ -57,12 +64,12 @@
 
   CodeGenIntrinsicTable Ints(Records);
 
-  EmitPrefix(OS);
-
   if (Enums) {
 // Emit the enum information.
 EmitEnumIn

RE: [clang] f978ea4 - [clang][clang-scan-deps] Aggregate the full dependency information.

2019-12-12 Thread Nemanja Ivanovic via cfe-commits

Hi Michael,
We are happy to help troubleshoot the issue this caused on our bot. Unfortunately, this bot is not one where we can give you access so we'll have to try and work together to debug this.
Can you provide the link to the failing build so we can see which test case it was that caused the problem and we can start debugging from there?
 
Nemanja IvanovicLLVM PPC Backend DevelopmentIBM Toronto LabEmail: neman...@ca.ibm.comPhone: 905-413-3388
 
 
- Original message -From: Michael Spencer To: powerl...@ca.ibm.comCc: cfe-commits@lists.llvm.orgSubject: [EXTERNAL] Re: [clang] f978ea4 - [clang][clang-scan-deps] Aggregate the full dependency information.Date: Wed, Dec 11, 2019 7:34 PM 
On Wed, Dec 11, 2019 at 2:41 PM Michael Spencer via cfe-commits  wrote:
Author: Michael SpencerDate: 2019-12-11T14:40:51-08:00New Revision: f978ea498309adaebab8fbf1cd6e520e7e0e11f1URL: https://github.com/llvm/llvm-project/commit/f978ea498309adaebab8fbf1cd6e520e7e0e11f1DIFF: https://github.com/llvm/llvm-project/commit/f978ea498309adaebab8fbf1cd6e520e7e0e11f1.diffLOG: [clang][clang-scan-deps] Aggregate the full dependency information.Differential Revision: https://reviews.llvm.org/D70268Added:Modified:    clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h    clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h    clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp    clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp    clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp    clang/test/ClangScanDeps/Inputs/modules_cdb.json    clang/test/ClangScanDeps/modules-full.cpp    clang/tools/clang-scan-deps/ClangScanDeps.cpp
 
 
Looks like this broke clang-ppc64be-linux. Is there a good way to debug this? It's not failing anywhere else, and none of the code should care about endianness.
 
I'll revert for now.
 
- Michael Spencer
 
 

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

1 2 3 >

1 - 100 of 274 matches

Mail list logo