[PATCH] D64305: [clangd] Add path mappings functionality

2019-11-18 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.  A couple of optional comments to address as you see fit.

It looks like you're not an LLVM committer yet, do you want me to commit this 
for you?
(Usually it's reasonable to ask for this after landing a couple of patches: 
https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access)




Comment at: clang-tools-extra/clangd/PathMapping.cpp:153
+// Converts \pPath to a posix-style absolute, i.e. if it's a windows path
+// then the backward-slash notation will be converted to forward slash
+llvm::Expected parsePath(llvm::StringRef Path) {

wwagner19 wrote:
> sammccall wrote:
> > "posix-style" doesn't really describe representing `c:\foo` as `/c:/foo`. 
> > That's really *just* a URI thing AFAIK.
> > 
> > Something like "Converts a unix/windows path to the path part of a file 
> > URI".
> > But in that case, I think the implementation is just 
> > `URI::createFile(Path).body()`. Does that pass tests?
> Oh I did not realize `createFile` was a thing, however looking at the way 
> it's implemented now, won't that throw an assert if a non-absolute path is 
> passed in? If so, is that desirable at all?
> 
> IIUC, if I were to use that API, then wouldn't it make more sense for it to 
> return an `llvm::Expected`? If we want to consolidate the logic to one place, 
> I'd be happy to try and refactor the signature.
I think allowing relative paths to be specified in path mappings is needlessly 
confusing. Clangd flags are typically specified in a config file somewhere, and 
that config often doesn't know about the working directory.

We don't want to hit the assert, so I'd suggest testing if it's absolute first, 
and returning an error if not.



Comment at: clang-tools-extra/clangd/PathMapping.cpp:214
+  llvm::SmallString<256> MappedPath(Uri->body());
+  llvm::sys::path::replace_path_prefix(MappedPath, From, To);
+  auto MappedUri = URI(Uri->scheme(), Uri->authority(), 
MappedPath.c_str());

wwagner19 wrote:
> sammccall wrote:
> > Sorry, I know I suggested replace_path_prefix, but now that the mappings 
> > consist of paths in their URL form, I think the old string::replace version 
> > is what you want :-/
> Will do, I like string replace better anyway! 
> 
> I'm not a huge fan of the `mappingMatches` function, and would prefer a 
> simple string `startswith(from)`, but the only way I may see that working is 
> by appending "/" to all the path mappings internally - which would prevent 
> `/foo` matching `/foo-bar` - but appending a "/" would break directory-based 
> file URIs, I believe.
Yeah, I see what you mean.
My favorite way to structure this code would probably be to always **strip** 
trailing slashes from the mappings, and combine the check-for-match and 
apply-match for a single entry:

```optional doPathMapping(StringRef Path, StringRef From, 
StringRef To) {
  if (Path.consume_front(From) && (Path.empty() || Path.startswith('/'))
return (To + Path).str()
  return None;
}```

Totally up to you, your tests look good :-)


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

https://reviews.llvm.org/D64305



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


[PATCH] D70222: [clangd] Add support for .rsp files in compile_commands.json

2019-11-18 Thread liu hui via Phabricator via cfe-commits
lh123 updated this revision to Diff 229758.
lh123 added a comment.

update diff


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

https://reviews.llvm.org/D70222

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

Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -17,8 +17,10 @@
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
+#include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/StringSaver.h"
 #include 
 #include 
 #include 
@@ -27,6 +29,124 @@
 namespace clangd {
 namespace {
 
+bool expandResponseFile(llvm::StringRef FName, llvm::StringSaver &Saver,
+llvm::cl::TokenizerCallback Tokenizer,
+SmallVectorImpl &NewArgv) {
+  llvm::ErrorOr> MemBufOrErr =
+  llvm::MemoryBuffer::getFile(FName);
+  if (!MemBufOrErr)
+return false;
+  llvm::MemoryBuffer &MemBuf = *MemBufOrErr.get();
+  StringRef Str(MemBuf.getBufferStart(), MemBuf.getBufferSize());
+
+  // If we have a UTF-16 byte order mark, convert to UTF-8 for parsing.
+  ArrayRef BufRef(MemBuf.getBufferStart(), MemBuf.getBufferEnd());
+  std::string UTF8Buf;
+  // It is called byte order marker but the UTF-8 BOM is actually not affected
+  // by the host system's endianness.
+  auto HasUtF8ByteOrderMark = [](ArrayRef S) {
+return (S.size() >= 3 && S[0] == '\xef' && S[1] == '\xbb' &&
+S[2] == '\xbf');
+  };
+  if (llvm::hasUTF16ByteOrderMark(BufRef)) {
+if (!convertUTF16ToUTF8String(BufRef, UTF8Buf))
+  return false;
+Str = StringRef(UTF8Buf);
+  }
+  // If we see UTF-8 BOM sequence at the beginning of a file, we shall remove
+  // these bytes before parsing.
+  // Reference: http://en.wikipedia.org/wiki/UTF-8#Byte_order_mark
+  else if (HasUtF8ByteOrderMark(BufRef))
+Str = StringRef(BufRef.data() + 3, BufRef.size() - 3);
+  // Tokenize the contents into NewArgv.
+  Tokenizer(Str, Saver, NewArgv, false);
+  return true;
+}
+
+bool expandResponseFiles(tooling::CompileCommand &Cmd,
+ llvm::cl::TokenizerCallback Tokenizer) {
+  bool AllExpanded = true;
+  struct ResponseFileRecord {
+llvm::StringRef File;
+size_t End;
+  };
+  std::vector &Argv = Cmd.CommandLine;
+  // To detect recursive response files, we maintain a stack of files and the
+  // position of the last argument in the file. This position is updated
+  // dynamically as we recursively expand files.
+  SmallVector FileStack;
+
+  // Push a dummy entry that represents the initial command line, removing
+  // the need to check for an empty list.
+  FileStack.push_back({"", Argv.size()});
+
+  // Don't cache Argv.size() because it can change.
+  for (unsigned I = 0; I != Argv.size();) {
+while (I == FileStack.back().End) {
+  // Passing the end of a file's argument list, so we can remove it from the
+  // stack.
+  FileStack.pop_back();
+}
+
+std::string &Arg = Argv[I];
+
+if (Arg[0] != '@') {
+  ++I;
+  continue;
+}
+SmallString<128> ResponseFile;
+if (llvm::sys::path::is_relative(&Arg[1])) {
+  llvm::sys::path::append(ResponseFile, Cmd.Directory, &Arg[1]);
+}
+llvm::sys::path::remove_dots(ResponseFile);
+
+auto IsEquivalent = [ResponseFile](const ResponseFileRecord &RFile) {
+  return llvm::sys::fs::equivalent(RFile.File, ResponseFile);
+};
+
+// Check for recursive response files.
+if (std::any_of(FileStack.begin() + 1, FileStack.end(), IsEquivalent)) {
+  // This file is recursive, so we leave it in the argument stream and
+  // move on.
+  AllExpanded = false;
+  ++I;
+  continue;
+}
+
+// Replace this response file argument with the tokenization of its
+// contents.  Nested response files are expanded in subsequent iterations.
+SmallVector ExpandedArgv;
+llvm::BumpPtrAllocator Alloc;
+llvm::StringSaver Saver(Alloc);
+llvm::SmallVector T;
+if (!expandResponseFile(ResponseFile, Saver, Tokenizer, ExpandedArgv)) {
+  // We couldn't read this file, so we leave it in the argument stream and
+  // move on.
+  AllExpanded = false;
+  ++I;
+  continue;
+}
+
+for (ResponseFileRecord &Record : FileStack) {
+  // Increase the end of all active records by the number of newly expanded
+  // arguments, minus the response file itself.
+  Record.End += ExpandedArgv.size() - 1;
+}
+
+FileStack.push_back({ResponseFile, I + ExpandedArgv.size()});
+Argv.erase(Argv.begin() + I);
+Argv.insert(Argv.begin() + I, ExpandedArgv.begin(), ExpandedArgv.end());
+  }
+
+  // If successful, the top of the file stack will mark the end of the Argv
+  // stream. A failure her

[PATCH] D70222: [clangd] Add support for .rsp files in compile_commands.json

2019-11-18 Thread liu hui via Phabricator via cfe-commits
lh123 updated this revision to Diff 229759.
lh123 added a comment.

format patch


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

https://reviews.llvm.org/D70222

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

Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -17,8 +17,10 @@
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
+#include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/StringSaver.h"
 #include 
 #include 
 #include 
@@ -27,6 +29,124 @@
 namespace clangd {
 namespace {
 
+bool expandResponseFile(llvm::StringRef FName, llvm::StringSaver &Saver,
+llvm::cl::TokenizerCallback Tokenizer,
+SmallVectorImpl &NewArgv) {
+  llvm::ErrorOr> MemBufOrErr =
+  llvm::MemoryBuffer::getFile(FName);
+  if (!MemBufOrErr)
+return false;
+  llvm::MemoryBuffer &MemBuf = *MemBufOrErr.get();
+  StringRef Str(MemBuf.getBufferStart(), MemBuf.getBufferSize());
+
+  // If we have a UTF-16 byte order mark, convert to UTF-8 for parsing.
+  ArrayRef BufRef(MemBuf.getBufferStart(), MemBuf.getBufferEnd());
+  std::string UTF8Buf;
+  // It is called byte order marker but the UTF-8 BOM is actually not affected
+  // by the host system's endianness.
+  auto HasUtF8ByteOrderMark = [](ArrayRef S) {
+return (S.size() >= 3 && S[0] == '\xef' && S[1] == '\xbb' &&
+S[2] == '\xbf');
+  };
+  if (llvm::hasUTF16ByteOrderMark(BufRef)) {
+if (!convertUTF16ToUTF8String(BufRef, UTF8Buf))
+  return false;
+Str = StringRef(UTF8Buf);
+  }
+  // If we see UTF-8 BOM sequence at the beginning of a file, we shall remove
+  // these bytes before parsing.
+  // Reference: http://en.wikipedia.org/wiki/UTF-8#Byte_order_mark
+  else if (HasUtF8ByteOrderMark(BufRef))
+Str = StringRef(BufRef.data() + 3, BufRef.size() - 3);
+  // Tokenize the contents into NewArgv.
+  Tokenizer(Str, Saver, NewArgv, false);
+  return true;
+}
+
+bool expandResponseFiles(tooling::CompileCommand &Cmd,
+ llvm::cl::TokenizerCallback Tokenizer) {
+  bool AllExpanded = true;
+  struct ResponseFileRecord {
+llvm::StringRef File;
+size_t End;
+  };
+  std::vector &Argv = Cmd.CommandLine;
+  // To detect recursive response files, we maintain a stack of files and the
+  // position of the last argument in the file. This position is updated
+  // dynamically as we recursively expand files.
+  SmallVector FileStack;
+
+  // Push a dummy entry that represents the initial command line, removing
+  // the need to check for an empty list.
+  FileStack.push_back({"", Argv.size()});
+
+  // Don't cache Argv.size() because it can change.
+  for (unsigned I = 0; I != Argv.size();) {
+while (I == FileStack.back().End) {
+  // Passing the end of a file's argument list, so we can remove it from the
+  // stack.
+  FileStack.pop_back();
+}
+
+std::string &Arg = Argv[I];
+
+if (Arg[0] != '@') {
+  ++I;
+  continue;
+}
+SmallString<128> ResponseFile;
+if (llvm::sys::path::is_relative(&Arg[1])) {
+  llvm::sys::path::append(ResponseFile, Cmd.Directory, &Arg[1]);
+}
+llvm::sys::path::remove_dots(ResponseFile);
+
+auto IsEquivalent = [ResponseFile](const ResponseFileRecord &RFile) {
+  return llvm::sys::fs::equivalent(RFile.File, ResponseFile);
+};
+
+// Check for recursive response files.
+if (std::any_of(FileStack.begin() + 1, FileStack.end(), IsEquivalent)) {
+  // This file is recursive, so we leave it in the argument stream and
+  // move on.
+  AllExpanded = false;
+  ++I;
+  continue;
+}
+
+// Replace this response file argument with the tokenization of its
+// contents.  Nested response files are expanded in subsequent iterations.
+SmallVector ExpandedArgv;
+llvm::BumpPtrAllocator Alloc;
+llvm::StringSaver Saver(Alloc);
+llvm::SmallVector T;
+if (!expandResponseFile(ResponseFile, Saver, Tokenizer, ExpandedArgv)) {
+  // We couldn't read this file, so we leave it in the argument stream and
+  // move on.
+  AllExpanded = false;
+  ++I;
+  continue;
+}
+
+for (ResponseFileRecord &Record : FileStack) {
+  // Increase the end of all active records by the number of newly expanded
+  // arguments, minus the response file itself.
+  Record.End += ExpandedArgv.size() - 1;
+}
+
+FileStack.push_back({ResponseFile, I + ExpandedArgv.size()});
+Argv.erase(Argv.begin() + I);
+Argv.insert(Argv.begin() + I, ExpandedArgv.begin(), ExpandedArgv.end());
+  }
+
+  // If successful, the top of the file stack will mark the end of the Argv
+  // stream. A failure he

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-18 Thread Jorge Gorbe Moya via Phabricator via cfe-commits
jgorbe added a comment.

Hi,

I found a clang crash that seems to be caused by this patch. Here's a reduced 
test case:

  template 
  class a {
   public:
~a();
void b();
  };
  
  template 
  a::~a() try {
b();
  } catch (...) {
  }
  
  class d {
   public:
d(const char *, int);
a e;
  }
  
  d("", 1);

Building it with `clang -c -frounding-math` results in the following crash:

  Stack dump:
  0.  Program arguments: 
/usr/local/google/home/jgorbe/code/llvm-build/bin/clang-10 -cc1 -triple 
x86_64-unknown-linux-gnu -emit-obj -mrelax-all -disable-free -main-fi
  le-name repro.cc -mrelocation-model static -mthread-model posix 
-mframe-pointer=all -fmath-errno -frounding-math -masm-verbose 
-mconstructor-aliases -munwind-tables -fu
  se-init-array -target-cpu x86-64 -dwarf-column-info -debugger-tuning=gdb 
-resource-dir /usr/local/google/home/jgorbe/code/llvm-build/lib/clang/10.0.0 
-internal-isystem 
  /usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8 -internal-isystem 
/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/x86_64-linux-gnu/c++/8 
-internal-isystem
   /usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/x86_64-linux-gnu/c++/8 
-internal-isystem 
/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/backward -intern
  al-isystem /usr/local/include -internal-isystem 
/usr/local/google/home/jgorbe/code/llvm-build/lib/clang/10.0.0/include 
-internal-externc-isystem /usr/include/x86_64-lin
  ux-gnu -internal-externc-isystem /include -internal-externc-isystem 
/usr/include -fdeprecated-macro -fdebug-compilation-dir 
/usr/local/google/home/jgorbe/repro4 -ferror
  -limit 19 -fmessage-length 0 -fgnuc-version=4.2.1 -fobjc-runtime=gcc 
-fcxx-exceptions -fexceptions -fdiagnostics-show-option -fcolor-diagnostics 
-faddrsig -o /tmp/repro
  -c9cae0.o -x c++ repro.cc 
  1.   parser at end of file
  2.  Per-file LLVM IR generation
  3.  repro.cc:4:3: Generating code for declaration 'a::~a'
   #0 0x07fb2b27 llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Support/Unix/Signals.inc:532:11
 
   #1 0x07fb2cc9 PrintStackTraceSignalHandler(void*) 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Support/Unix/Signals.inc:593:1 

   #2 0x07fb160b llvm::sys::RunSignalHandlers() 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Support/Signals.cpp:67:5   
 
   #3 0x07fb3415 SignalHandler(int) 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Support/Unix/Signals.inc:384:1 
   
   #4 0x7f6789ec03a0 __restore_rt 
(/lib/x86_64-linux-gnu/libpthread.so.0+0x123a0) 

   #5 0x0739af45 
llvm::AttributeList::hasFnAttribute(llvm::Attribute::AttrKind) const 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/IR/Attributes.cpp:1315:10
   #6 0x051a1964 
llvm::Function::hasFnAttribute(llvm::Attribute::AttrKind) const 
/usr/local/google/home/jgorbe/code/llvm/llvm/include/llvm/IR/Function.h:324:5   
 
   #7 0x052421a1 llvm::IRBuilderBase::setConstrainedFPFunctionAttr() 
/usr/local/google/home/jgorbe/code/llvm/llvm/include/llvm/IR/IRBuilder.h:262:9
   #8 0x05241b60 
llvm::IRBuilderBase::setConstrainedFPCallAttr(llvm::CallInst*) 
/usr/local/google/home/jgorbe/code/llvm/llvm/include/llvm/IR/IRBuilder.h:271:3
   #9 0x084a516b llvm::IRBuilder::CreateCall(llvm::FunctionType*, 
llvm::Value*, llvm::ArrayRef, 
llvm::ArrayRef >, llvm::Twine const&, 
llvm::MDNode*) 
/usr/local/google/home/jgorbe/code/llvm/llvm/include/llvm/IR/IRBuilder.h:2274:9 


  #10 0x084a4488 llvm::IRBuilder::CreateCall(llvm::FunctionCallee, 
llvm::ArrayRef, 
llvm::ArrayRef >, llvm::Twine const&, 
llvm::MDNode*) 
/usr/local/google/home/jgorbe/code/llvm/llvm/include/llvm/IR/IRBuilder.h:2288:5
  #11 0x0849288c 
clang::CodeGen::CodeGenFunction::EmitRuntimeCall(llvm::FunctionCallee, 
llvm::ArrayRef, llvm::Twine const&) 
/usr/local/google/home/jgorbe/code/llvm/clang/lib/CodeGen/CGCall.cpp:3726:26
  #12 0x0849278d 
clang::CodeGen::CodeGenFunction::EmitNounwindRuntimeCall(llvm::FunctionCallee, 
llvm::ArrayRef, llvm::Twine const&) 
/usr/local/google/home/jgorbe/code/llvm/clang/lib/CodeGen/CGCall.cpp:3691:19


  #13 0x0897b3e4 (anonymous 
namespace)::ItaniumCXXABI::emitTerminateForUnexpectedException(clang::CodeGen::CodeGenFunction&,
 llvm::Value*) 
/usr/local/google/home/jgorbe/code/llvm/clang/lib/CodeGen/ItaniumCXXABI.cpp:4364:5
   
  #14 0x0865698d clang::CodeGen::CodeGenFunction::getTerminateHandler() 
/usr/local/google/home/jgorbe/code/llvm/clang/lib/CodeGen/

[PATCH] D70359: [clangd] Show values of more expressions on hover

2019-11-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall marked an inline comment as done.
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/Hover.cpp:423
+  //  - certain expressions (sizeof etc)
+  //  - built-in types
 }

lh123 wrote:
> I think we should also support hover on literal.
sure - can you explain what you'd like to see/when you'd expect it to be useful?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70359



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


[PATCH] D70377: clang-format: [JS] tests for async wrapping.

2019-11-18 Thread Martin Probst via Phabricator via cfe-commits
mprobst created this revision.
mprobst added a reviewer: krasimir.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Adds tests to ensure that `async method() ...` does not wrap between async and
the method name, which would cause automatic semicolon insertion.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70377

Files:
  clang/unittests/Format/FormatTestJS.cpp


Index: clang/unittests/Format/FormatTestJS.cpp
===
--- clang/unittests/Format/FormatTestJS.cpp
+++ clang/unittests/Format/FormatTestJS.cpp
@@ -737,6 +737,22 @@
"   function a() {\n"
"  return   1;\n"
"}  \n");
+  // clang-format must not insert breaks between async and function, otherwise
+  // automatic semicolon insertion may trigger (in particular in a class body).
+  verifyFormat("async function\n"
+   "hello(\n"
+   "myparamnameiswaytoolng) {\n"
+   "}",
+   "async function hello(myparamnameiswaytoolng) {}",
+   getGoogleJSStyleWithColumns(10));
+  verifyFormat("class C {\n"
+   "  async hello(\n"
+   "  myparamnameiswaytoolng) {\n"
+   "  }\n"
+   "}",
+   "class C {\n"
+   "  async hello(myparamnameiswaytoolng) {} }",
+   getGoogleJSStyleWithColumns(10));
   verifyFormat("async function* f() {\n"
"  yield fetch(x);\n"
"}");


Index: clang/unittests/Format/FormatTestJS.cpp
===
--- clang/unittests/Format/FormatTestJS.cpp
+++ clang/unittests/Format/FormatTestJS.cpp
@@ -737,6 +737,22 @@
"   function a() {\n"
"  return   1;\n"
"}  \n");
+  // clang-format must not insert breaks between async and function, otherwise
+  // automatic semicolon insertion may trigger (in particular in a class body).
+  verifyFormat("async function\n"
+   "hello(\n"
+   "myparamnameiswaytoolng) {\n"
+   "}",
+   "async function hello(myparamnameiswaytoolng) {}",
+   getGoogleJSStyleWithColumns(10));
+  verifyFormat("class C {\n"
+   "  async hello(\n"
+   "  myparamnameiswaytoolng) {\n"
+   "  }\n"
+   "}",
+   "class C {\n"
+   "  async hello(myparamnameiswaytoolng) {} }",
+   getGoogleJSStyleWithColumns(10));
   verifyFormat("async function* f() {\n"
"  yield fetch(x);\n"
"}");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D70317: [clangd] More sensible output for constructors/destructors in hover.

2019-11-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.cpp:598
+// Constructor's "return type" is the class type.
+HI.ReturnType = declaredType(CCD->getParent()).getAsString(Policy);
+// Don't provide any type for the constructor itself.

this looks reasonable though I'd prefer not setting the return type for 
ctor/dtor. If we decide to go down this path, we'd better make other 
"returnType" places (`Symbol::ReturnType`, CodeCompletion) align with this.



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:914
   template struct X;
   template struct X{ [[^X]](); };
   )cpp",

nit: add a test case for destructor?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70317



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


[PATCH] D69934: [clangd] Implement rename by using SelectionTree and findExplicitReferences.

2019-11-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 229766.
hokein added a comment.

rebase: renaming the filed on cxx initializer list e.g. "Foo() : Fie^ld()" is 
fixed in SelectionTree.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69934

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
@@ -38,27 +38,27 @@
   return Result;
 }
 
-TEST(RenameTest, SingleFile) {
-  // "^" points to the position of the rename, and "[[]]" ranges point to the
+TEST(RenameTest, WithinFileRename) {
+  // rename is runnning on all "^" points, and "[[]]" ranges point to the
   // identifier that is being renamed.
   llvm::StringRef Tests[] = {
-  // Rename function.
+  // rename function
   R"cpp(
-void [[foo]]() {
+void [[foo^]]() {
   [[fo^o]]();
 }
   )cpp",
 
-  // Rename type.
+  // rename type
   R"cpp(
-struct [[foo]]{};
+struct [[foo^]] {};
 [[foo]] test() {
[[f^oo]] x;
return x;
 }
   )cpp",
 
-  // Rename variable.
+  // rename variable
   R"cpp(
 void bar() {
   if (auto [[^foo]] = 5) {
@@ -66,18 +66,328 @@
   }
 }
   )cpp",
+
+  R"cpp(
+class [[F^oo]] {};
+template  void func();
+template  class Baz {};
+int main() {
+  func<[[F^oo]]>();
+  Baz<[[F^oo]]> obj;
+  return 0;
+}
+  )cpp",
+
+  // rename class, including constructor/destructor
+  R"cpp(
+class [[F^oo]] {
+  [[F^oo]]();
+  ~[[Foo]]();
+  void foo(int x);
+};
+[[Foo]]::[[Fo^o]]() {}
+void [[Foo]]::foo(int x) {}
+  )cpp",
+
+  // forward class declaration without definition
+  R"cpp(
+class [[F^oo]];
+[[Foo]] *f();
+  )cpp",
+
+  // class methods overrides
+  R"cpp(
+struct A {
+ virtual void [[f^oo]]() {}
+};
+struct B : A {
+  void [[f^oo]]() override {}
+};
+struct C : B {
+  void [[f^oo]]() override {}
+};
+
+void func() {
+  A().[[f^oo]]();
+  B().[[f^oo]]();
+  C().[[f^oo]]();
+}
+  )cpp",
+
+  // template class (partial) specializations
+  R"cpp(
+template 
+class [[F^oo]] {};
+
+template<>
+class [[F^oo]] {};
+template 
+class [[F^oo]] {};
+
+void test() {
+  [[Foo]] x;
+  [[Foo]] y;
+  [[Foo]] z;
+}
+  )cpp",
+
+  // template class instantiations
+  R"cpp(
+template 
+class [[F^oo]] {
+public:
+  T foo(T arg, T& ref, T* ptr) {
+T value;
+int number = 42;
+value = (T)number;
+value = static_cast(number);
+return value;
+  }
+  static void foo(T value) {}
+  T member;
+};
+
+template 
+void func() {
+  [[F^oo]] obj;
+  obj.member = T();
+  [[Foo]]::foo();
+}
+
+void test() {
+  [[F^oo]] i;
+  i.member = 0;
+  [[F^oo]]::foo(0);
+
+  [[F^oo]] b;
+  b.member = false;
+  [[Foo]]::foo(false);
+}
+  )cpp",
+
+  // template class methods
+  R"cpp(
+template 
+class A {
+public:
+  void [[f^oo]]() {}
+};
+
+void func() {
+  A().[[f^oo]]();
+  A().[[f^oo]]();
+  A().[[f^oo]]();
+}
+  )cpp",
+
+  // complicated class type
+  R"cpp(
+ // Forward declaration.
+class [[Fo^o]];
+class Baz {
+  virtual int getValue() const = 0;
+};
+
+class [[F^oo]] : public Baz  {
+public:
+  [[Foo]](int value = 0) : x(value) {}
+
+  [[Foo]] &operator++(int);
+
+  bool operator<([[Foo]] const &rhs);
+  int getValue() const;
+private:
+  int x;
+};
+
+void func() {
+  [[Foo]] *Pointer = 0;
+  [[Foo]] Variable = [[Foo]](10);
+  for ([[Foo]] it; it < Variable; it++);
+  const [[Foo]] *C = new [[Foo]]();
+  const_cast<[[Foo]] *>(C)->getValue();
+  [[Foo]] foo;
+  const Baz &BazReference = foo;
+  const Baz *BazPointer = &foo;
+  dynamic_cast(BazReference).getValue();
+  dynamic_cast(BazPointer)->getValue();
+  reinterpret_cast(BazPointer)->getValue();
+  static_cast(BazReference).getValue();
+  static_cast(BazPointer

[PATCH] D69543: [clangd] Add a tweak refactoring to wrap Objective-C string literals in `NSLocalizedString` macros

2019-11-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment.

implementation lgtm with a few nits.

main concern is about the new getlangopts helper




Comment at: clang-tools-extra/clangd/ParsedAST.h:80
 
+  const LangOptions &getLangOpts() const {
+return getASTContext().getLangOpts();

can we introduce this helper in a new patch, while changing other occurrences 
in clangd?



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/ObjCLocalizeStringLiteral.cpp:34
+/// After:
+///   NSLocalizedString(@"description", "")
+class ObjCLocalizeStringLiteral : public Tweak {

 NSLocalizedString(@"description", `@`"")



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/ObjCLocalizeStringLiteral.cpp:69
+  SM, CharSourceRange::getCharRange(Str->getBeginLoc()),
+  "NSLocalizedString(", Inputs.AST.getASTContext().getLangOpts()));
+  SourceLocation EndLoc = Lexer::getLocForEndOfToken(

maybe extract `Inputs.AST.getASTContext().getLangOpts()` into a variable and 
make use of it in the following places as well? (line 72 and 75)



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:131
+  // Ensure the the action can be initiated in the string literal.
+  EXPECT_AVAILABLE(R"(id x = ^@^"^t^est^";)");
+  EXPECT_AVAILABLE(R"(id x = [[@"test"]];)");

nit: you can combine all of this into a single test



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:136
+  // Ensure that the action can't be initiated in other places.
+  EXPECT_UNAVAILABLE(R"(i^d ^x ^= @"test";^)");
+  EXPECT_UNAVAILABLE(R"(id [[x]] = @"test";)");

nit: you can combine all of this into a single test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69543



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


[PATCH] D70203: [AST] Attach comment in `/** doc */ typedef struct A {} B` to B as well as A.

2019-11-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision.
kadircet marked an inline comment as done.
kadircet added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!




Comment at: clang/test/Index/annotate-comments-typedef.m:20
 } MyEnum;
-// CHECK: TypedefDecl=MyEnum:[[@LINE-1]]:3 (Definition) FullCommentAsHTML=[ Documentation for MyEnum ] FullCommentAsXML=[c:@EA@MyEnumtypedef
 enum MyEnum MyEnum Documentation for MyEnum 
]
+// CHECK: TypedefDecl=MyEnum:[[@LINE-1]]:3 (Definition) {{.*}} 
FullCommentAsHTML=[ Documentation for MyEnum ] 
FullCommentAsXML=[MyEnumc:@T@MyEnumtypedef enum 
MyEnum MyEnum Documentation for MyEnum 
]
 

sammccall wrote:
> kadircet wrote:
> > sorry for being dense, what exactly is the change here ? it looks like 
> > comment was already being attributed to `MyEnum`. 
> > As for the USR I agree that this looks a whole lot better, apparently for 
> > anon decls USR contains the name of the typedef decl instead.
> > 
> > What is the extra text that we don't care in between?
> Yeah, most of this doesn't matter AFAIK and the test is just brittle, as lit 
> tests are wont to be.
> 
> The {{.*}} is because the output now includes the RawComment= clause, that 
> previously wasn't included. (Which I think is to do with the comment being 
> immediately before the location?)
> 
> Previously the `Name` was ``, and now it's `MyEnum`. And the USR 
> has changed as you noted.
oh missed the change in `Name` thanks for pointing it out!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70203



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


[PATCH] D70325: [clangd] Fix hover 'local scope' to include class template params

2019-11-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment.

LGTM, with a question. What about default template params? I believe we would 
also like to print them, could you add a test case for that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70325



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


[PATCH] D70359: [clangd] Show values of more expressions on hover

2019-11-18 Thread liu hui via Phabricator via cfe-commits
lh123 added inline comments.



Comment at: clang-tools-extra/clangd/Hover.cpp:423
+  //  - certain expressions (sizeof etc)
+  //  - built-in types
 }

sammccall wrote:
> lh123 wrote:
> > I think we should also support hover on literal.
> sure - can you explain what you'd like to see/when you'd expect it to be 
> useful?
such as:
1. Hovering on "abc" shows char[4], we can get the length information of the 
string.
2. Get actual type for user-defined literal types.
```c++
struct foo {};
struct foo {};

foo operator""_foo(unsigned long long v) { return {}; }

int main() {
1_foo;
}

```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70359



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


[PATCH] D70359: [clangd] Show values of more expressions on hover

2019-11-18 Thread liu hui via Phabricator via cfe-commits
lh123 added inline comments.



Comment at: clang-tools-extra/clangd/Hover.cpp:423
+  //  - certain expressions (sizeof etc)
+  //  - built-in types
 }

lh123 wrote:
> sammccall wrote:
> > lh123 wrote:
> > > I think we should also support hover on literal.
> > sure - can you explain what you'd like to see/when you'd expect it to be 
> > useful?
> such as:
> 1. Hovering on "abc" shows char[4], we can get the length information of the 
> string.
> 2. Get actual type for user-defined literal types.
> ```c++
> struct foo {};
> struct foo {};
> 
> foo operator""_foo(unsigned long long v) { return {}; }
> 
> int main() {
> 1_foo;
> }
> 
> ```
> 
```
struct foo {};

foo operator""_foo(unsigned long long v) { return {}; }

int main() {
1_foo;
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70359



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


[PATCH] D69934: [clangd] Implement rename by using SelectionTree and findExplicitReferences.

2019-11-18 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

Build result: fail - 60148 tests passed, 1 failed and 729 were skipped.

  failed: LLVM.Bindings/Go/go.test

Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69934



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


[PATCH] D70297: [ARM,MVE] Add intrinsics for vector comparisons.

2019-11-18 Thread Dave Green via Phabricator via cfe-commits
dmgreen accepted this revision.
dmgreen 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/D70297/new/

https://reviews.llvm.org/D70297



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


[clang] 2d739f9 - [ARM] Allocatable Global Register Variables for ARM

2019-11-18 Thread Anna Welker via cfe-commits

Author: Anna Welker
Date: 2019-11-18T10:07:37Z
New Revision: 2d739f98d8a53e38bf9faa88cdb6b0c2a363fb77

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

LOG: [ARM] Allocatable Global Register Variables for ARM

  Provides support for using r6-r11 as globally scoped
  register variables. This requires a -ffixed-rN flag
  in order to reserve rN against general allocation.

  If for a given GRV declaration the corresponding flag
  is not found, or the the register in question is the
  target's FP, we fail with a diagnostic.

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

Added: 
clang/test/Driver/arm-reserved-reg-options.c
clang/test/Sema/arm-global-regs.c
llvm/test/CodeGen/ARM/reg-alloc-fixed-r6-vla.ll
llvm/test/CodeGen/ARM/reg-alloc-with-fixed-reg-r6-modified.ll
llvm/test/CodeGen/ARM/reg-alloc-with-fixed-reg-r6.ll
llvm/test/CodeGen/ARM/reg-alloc-wout-fixed-regs.ll
llvm/test/CodeGen/Thumb/callee_save_reserved.ll
llvm/test/Feature/reserve_global_reg.ll

Modified: 
clang/docs/ClangCommandLineReference.rst
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Basic/TargetInfo.h
clang/include/clang/Driver/Options.td
clang/lib/Basic/Targets/ARM.cpp
clang/lib/Basic/Targets/ARM.h
clang/lib/Driver/ToolChains/Arch/ARM.cpp
clang/lib/Sema/SemaDecl.cpp
llvm/lib/Target/ARM/ARM.td
llvm/lib/Target/ARM/ARMAsmPrinter.cpp
llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
llvm/lib/Target/ARM/ARMFrameLowering.cpp
llvm/lib/Target/ARM/ARMISelLowering.cpp
llvm/lib/Target/ARM/ARMSubtarget.cpp
llvm/lib/Target/ARM/ARMSubtarget.h
llvm/lib/Target/ARM/ARMTargetTransformInfo.h

Removed: 




diff  --git a/clang/docs/ClangCommandLineReference.rst 
b/clang/docs/ClangCommandLineReference.rst
index e8d561fae956..492eec71f2e4 100644
--- a/clang/docs/ClangCommandLineReference.rst
+++ b/clang/docs/ClangCommandLineReference.rst
@@ -2430,10 +2430,31 @@ Enable XNACK (AMDGPU only)
 
 ARM
 ---
+
+.. option:: -ffixed-r6
+
+Reserve the r6 register (ARM only)
+
+.. option:: -ffixed-r7
+
+Reserve the r7 register (ARM only)
+
+.. option:: -ffixed-r8
+
+Reserve the r8 register (ARM only)
+
 .. option:: -ffixed-r9
 
 Reserve the r9 register (ARM only)
 
+.. option:: -ffixed-r10
+
+Reserve the r10 register (ARM only)
+
+.. option:: -ffixed-r11
+
+Reserve the r11 register (ARM only)
+
 .. option:: -mexecute-only, -mno-execute-only, -mpure-code
 
 Disallow generation of data access to code sections (ARM only)

diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 5ff03e133563..0e309909030e 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -464,6 +464,10 @@ def warn_drv_msp430_hwmult_no_device : Warning<"no MCU 
device specified, but "
   "specify a MSP430 device, or -mhwmult to set hardware multiply type "
   "explicitly.">, InGroup;
 
+// Frame pointer reservation.
+def err_reserved_frame_pointer : Error<
+  "'%0' has been specified but '%1' is used as the frame pointer for this 
target">;
+
 def warn_drv_libstdcxx_not_found : Warning<
   "include path for libstdc++ headers not found; pass '-stdlib=libc++' on the "
   "command line to use the libc++ standard library instead">,

diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index bc66a8253074..6b83bf59ea89 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1115,3 +1115,6 @@ def CrossTU : DiagGroup<"ctu">;
 def CTADMaybeUnsupported : DiagGroup<"ctad-maybe-unsupported">;
 
 def FortifySource : DiagGroup<"fortify-source">;
+
+// Register reservation.
+def FixedRegs : DiagGroup<"fixed-registers">;

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index beb25c5a0892..49ad7c7cc462 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7716,6 +7716,8 @@ let CategoryName = "Inline Assembly Issue" in {
   def err_asm_unknown_register_name : Error<"unknown register name '%0' in 
asm">;
   def err_asm_invalid_global_var_reg : Error<"register '%0' unsuitable for "
 "global register variables on this target">;
+  def err_asm_missing_fixed_reg_opt : Error<"-ffixed-%0 is required for "
+"global named register variable declaration">;
   def err_asm_register_size_mismatch : Error<"size of register '%0' does not "
 "match variable size">;
   def err_asm_b

[clang] a433e71 - [AST] Attach comment in `/** doc */ typedef struct A {} B` to B as well as A.

2019-11-18 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2019-11-18T11:09:23+01:00
New Revision: a433e7141fb3f697e6430437ee73b19076603c1b

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

LOG: [AST] Attach comment in `/** doc */ typedef struct A {} B` to B as well as 
A.

Summary:
Semantically they're the same thing, and it's important when the underlying
struct is anonymous.

There doesn't seem to be a problem attaching the same comment to multiple things
as it already happens with `/** doc */ int a, b;`

This affects an Index test but the results look better (name present, USR points
to the typedef).

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

Reviewers: kadircet, lh123

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

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/unittests/XRefsTests.cpp
clang/lib/AST/ASTContext.cpp
clang/test/Index/annotate-comments-typedef.m
clang/test/Sema/warn-documentation.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp 
b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index cbc81805fd73..cb34dac7ec00 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1299,6 +1299,19 @@ TEST(Hover, All) {
   "]\n"
   "text[Typedef]",
   },
+  {
+  R"cpp(// Typedef with embedded definition
+typedef struct Bar {} Foo;
+int main() {
+  ^Foo bar;
+}
+  )cpp",
+  "text[Declared in]code[global namespace]\n"
+  "codeblock(cpp) [\n"
+  "typedef struct Bar Foo\n"
+  "]\n"
+  "text[Typedef with embedded definition]",
+  },
   {
   R"cpp(// Namespace
 namespace ns {

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index cda51ec755a8..6a15f99eb7d3 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -163,7 +163,9 @@ static SourceLocation getDeclLocForCommentSearch(const Decl 
*D,
   if (isa(D) || isa(D) ||
   isa(D) ||
   isa(D) ||
-  isa(D))
+  isa(D) ||
+  // Allow association with Y across {} in `typedef struct X {} Y`.
+  isa(D))
 return D->getBeginLoc();
   else {
 const SourceLocation DeclLoc = D->getLocation();

diff  --git a/clang/test/Index/annotate-comments-typedef.m 
b/clang/test/Index/annotate-comments-typedef.m
index 751cfaab4931..77c4d631850d 100644
--- a/clang/test/Index/annotate-comments-typedef.m
+++ b/clang/test/Index/annotate-comments-typedef.m
@@ -17,7 +17,7 @@
 MyEnumBar, /**< value Bar */
 MyEnumBaz, /**< value Baz */
 } MyEnum;
-// CHECK: TypedefDecl=MyEnum:[[@LINE-1]]:3 (Definition) FullCommentAsHTML=[ Documentation for MyEnum ] FullCommentAsXML=[c:@EA@MyEnumtypedef
 enum MyEnum MyEnum Documentation for MyEnum 
]
+// CHECK: TypedefDecl=MyEnum:[[@LINE-1]]:3 (Definition) {{.*}} 
FullCommentAsHTML=[ Documentation for MyEnum ] 
FullCommentAsXML=[MyEnumc:@T@MyEnumtypedef enum 
MyEnum MyEnum Documentation for MyEnum 
]
 
 
 /** Documentation for E */
@@ -35,7 +35,7 @@
 typedef struct {
  int iii;
 } Foo;
-// CHECK: TypedefDecl=Foo:[[@LINE-1]]:11 (Definition) FullCommentAsHTML=[ Comment about Foo ] FullCommentAsXML=[c:@SA@Footypedef
 struct Foo Foo Comment about Foo 
]
+// CHECK: TypedefDecl=Foo:[[@LINE-1]]:11 (Definition) {{.*}} 
FullCommentAsHTML=[ Comment about Foo ] 
FullCommentAsXML=[Fooc:@T@Footypedef struct Foo 
Foo Comment about Foo 
]
 // CHECK: StructDecl=:[[@LINE-4]]:9 (Definition) {{.*}} BriefComment=[Comment 
about Foo] FullCommentAsHTML=[ Comment about Foo ] 
FullCommentAsXML=[c:@SA@Foostruct
 {} Comment about Foo ]
 
 

diff  --git a/clang/test/Sema/warn-documentation.cpp 
b/clang/test/Sema/warn-documentation.cpp
index 66f02efdc40d..93273559a905 100644
--- a/clang/test/Sema/warn-documentation.cpp
+++ b/clang/test/Sema/warn-documentation.cpp
@@ -868,7 +868,7 @@ struct test_noattach12 *test_attach13;
 /// \brief\author Aaa
 typedef struct test_noattach14 *test_attach15;
 
-// expected-warning@+1 {{empty paragraph passed to '\brief' command}}
+// expected-warning@+1 + {{empty paragraph passed to '\brief' command}}
 /// \brief\author Aaa
 typedef struct test_attach16 { int a; } test_attach17;
 
@@ -886,7 +886,7 @@ typedef struct S *test_attach19;
 /// \brief\author Aaa
 struct test_attach20;
 
-// expected-warning@+1 {{empty paragraph passed to '\brief' command}}
+// expected-warning@+1 + {{empty paragraph passed to '\brief' command}}
 /// \brief\author Aaa
 typedef struct test_attach21 {
   // expected-warning@+1 {{empty paragraph passed to '\brief' command}}


 

[PATCH] D70203: [AST] Attach comment in `/** doc */ typedef struct A {} B` to B as well as A.

2019-11-18 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa433e7141fb3: [AST] Attach comment in `/** doc */ typedef 
struct A {} B` to B as well as A. (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D70203?vs=229157&id=229769#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70203

Files:
  clang-tools-extra/clangd/unittests/XRefsTests.cpp
  clang/lib/AST/ASTContext.cpp
  clang/test/Index/annotate-comments-typedef.m
  clang/test/Sema/warn-documentation.cpp


Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -868,7 +868,7 @@
 /// \brief\author Aaa
 typedef struct test_noattach14 *test_attach15;
 
-// expected-warning@+1 {{empty paragraph passed to '\brief' command}}
+// expected-warning@+1 + {{empty paragraph passed to '\brief' command}}
 /// \brief\author Aaa
 typedef struct test_attach16 { int a; } test_attach17;
 
@@ -886,7 +886,7 @@
 /// \brief\author Aaa
 struct test_attach20;
 
-// expected-warning@+1 {{empty paragraph passed to '\brief' command}}
+// expected-warning@+1 + {{empty paragraph passed to '\brief' command}}
 /// \brief\author Aaa
 typedef struct test_attach21 {
   // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
Index: clang/test/Index/annotate-comments-typedef.m
===
--- clang/test/Index/annotate-comments-typedef.m
+++ clang/test/Index/annotate-comments-typedef.m
@@ -17,7 +17,7 @@
 MyEnumBar, /**< value Bar */
 MyEnumBaz, /**< value Baz */
 } MyEnum;
-// CHECK: TypedefDecl=MyEnum:[[@LINE-1]]:3 (Definition) FullCommentAsHTML=[ Documentation for MyEnum ] FullCommentAsXML=[c:@EA@MyEnumtypedef
 enum MyEnum MyEnum Documentation for MyEnum 
]
+// CHECK: TypedefDecl=MyEnum:[[@LINE-1]]:3 (Definition) {{.*}} 
FullCommentAsHTML=[ Documentation for MyEnum ] 
FullCommentAsXML=[MyEnumc:@T@MyEnumtypedef enum 
MyEnum MyEnum Documentation for MyEnum 
]
 
 
 /** Documentation for E */
@@ -35,7 +35,7 @@
 typedef struct {
  int iii;
 } Foo;
-// CHECK: TypedefDecl=Foo:[[@LINE-1]]:11 (Definition) FullCommentAsHTML=[ Comment about Foo ] FullCommentAsXML=[c:@SA@Footypedef
 struct Foo Foo Comment about Foo 
]
+// CHECK: TypedefDecl=Foo:[[@LINE-1]]:11 (Definition) {{.*}} 
FullCommentAsHTML=[ Comment about Foo ] 
FullCommentAsXML=[Fooc:@T@Footypedef struct Foo 
Foo Comment about Foo 
]
 // CHECK: StructDecl=:[[@LINE-4]]:9 (Definition) {{.*}} BriefComment=[Comment 
about Foo] FullCommentAsHTML=[ Comment about Foo ] 
FullCommentAsXML=[c:@SA@Foostruct
 {} Comment about Foo ]
 
 
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -163,7 +163,9 @@
   if (isa(D) || isa(D) ||
   isa(D) ||
   isa(D) ||
-  isa(D))
+  isa(D) ||
+  // Allow association with Y across {} in `typedef struct X {} Y`.
+  isa(D))
 return D->getBeginLoc();
   else {
 const SourceLocation DeclLoc = D->getLocation();
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1300,6 +1300,19 @@
   "text[Typedef]",
   },
   {
+  R"cpp(// Typedef with embedded definition
+typedef struct Bar {} Foo;
+int main() {
+  ^Foo bar;
+}
+  )cpp",
+  "text[Declared in]code[global namespace]\n"
+  "codeblock(cpp) [\n"
+  "typedef struct Bar Foo\n"
+  "]\n"
+  "text[Typedef with embedded definition]",
+  },
+  {
   R"cpp(// Namespace
 namespace ns {
 struct Foo { static void bar(); }


Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -868,7 +868,7 @@
 /// \brief\author Aaa
 typedef struct test_noattach14 *test_attach15;
 
-// expected-warning@+1 {{empty paragraph passed to '\brief' command}}
+// expected-warning@+1 + {{empty paragraph passed to '\brief' command}}
 /// \brief\author Aaa
 typedef struct test_attach16 { int a; } test_attach17;
 
@@ -886,7 +886,7 @@
 /// \brief\author Aaa
 struct test_attach20;
 
-// expected-warning@+1 {{empty paragraph passed to '\brief' command}}
+// expected-warning@+1 + {{empty paragraph passed to '\brief' command}}
 /// \brief\author Aaa
 typedef struct test_attach21 {
   // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
Index: clan

[PATCH] D70359: [clangd] Show values of more expressions on hover

2019-11-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet marked an inline comment as done.
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/Hover.cpp:247
+  // to the enclosing call.
+  if (E->getType()->isFunctionType() || E->getType()->isFunctionPointerType() 
||
+  E->getType()->isFunctionReferenceType())

nit extract `E->getType()` into a variable



Comment at: clang-tools-extra/clangd/Hover.cpp:255
+  // Show enums symbolically, not numerically like APValue::printPretty().
+  if (E->getType()->isEnumeralType() &&
+  Constant.Val.getInt().getMinSignedBits() <= 64) {

both `Constant.Val.getInt` and `ECD->getInitVal` are `APSInt` and it has an 
`operator==`, why cast to `int64_t` in between?



Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:278
  };
+ HI.Value = "false";
  return HI;

wow, that's hilarious.



Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:450
+ HI.Type = "enum Color";
+ HI.Value = "GREEN"; // Symbolic when hovering on an expression.
}},

what about `GREEN (1)` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70359



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


[PATCH] D70380: [clangd] Expose the xref's incomplete flag to clangdServer API.

2019-11-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

so that clangd C++ API users (via ClangdServer) can access it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70380

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -2096,7 +2096,7 @@
 std::vector> ExpectedLocations;
 for (const auto &R : T.ranges())
   ExpectedLocations.push_back(RangeIs(R));
-EXPECT_THAT(findReferences(AST, T.point(), 0),
+EXPECT_THAT(findReferences(AST, T.point(), 0).Refs,
 ElementsAreArray(ExpectedLocations))
 << Test;
   }
@@ -2157,7 +2157,7 @@
 for (const auto &R : T.ranges())
   ExpectedLocations.push_back(RangeIs(R));
 ASSERT_THAT(ExpectedLocations, Not(IsEmpty()));
-EXPECT_THAT(findReferences(AST, T.point(), 0),
+EXPECT_THAT(findReferences(AST, T.point(), 0).Refs,
 ElementsAreArray(ExpectedLocations))
 << Test;
   }
@@ -2172,7 +2172,7 @@
   auto AST = TU.build();
 
   // References in main file are returned without index.
-  EXPECT_THAT(findReferences(AST, Main.point(), 0, /*Index=*/nullptr),
+  EXPECT_THAT(findReferences(AST, Main.point(), 0, /*Index=*/nullptr).Refs,
   ElementsAre(RangeIs(Main.range(;
   Annotations IndexedMain(R"cpp(
 int main() { [[f^oo]](); }
@@ -2183,17 +2183,18 @@
   IndexedTU.Code = IndexedMain.code();
   IndexedTU.Filename = "Indexed.cpp";
   IndexedTU.HeaderCode = Header;
-  EXPECT_THAT(findReferences(AST, Main.point(), 0, IndexedTU.index().get()),
-  ElementsAre(RangeIs(Main.range()), RangeIs(IndexedMain.range(;
-
-  EXPECT_EQ(1u, findReferences(AST, Main.point(), /*Limit*/ 1,
-   IndexedTU.index().get())
-.size());
+  EXPECT_THAT(
+  findReferences(AST, Main.point(), 0, IndexedTU.index().get()).Refs,
+  ElementsAre(RangeIs(Main.range()), RangeIs(IndexedMain.range(;
+  auto LimitRefs =
+  findReferences(AST, Main.point(), /*Limit*/ 1, IndexedTU.index().get());
+  EXPECT_EQ(1u, LimitRefs.Refs.size());
+  EXPECT_TRUE(LimitRefs.InComplete);
 
   // If the main file is in the index, we don't return duplicates.
   // (even if the references are in a different location)
   TU.Code = ("\n\n" + Main.code()).str();
-  EXPECT_THAT(findReferences(AST, Main.point(), 0, TU.index().get()),
+  EXPECT_THAT(findReferences(AST, Main.point(), 0, TU.index().get()).Refs,
   ElementsAre(RangeIs(Main.range(;
 }
 
Index: clang-tools-extra/clangd/XRefs.h
===
--- clang-tools-extra/clangd/XRefs.h
+++ clang-tools-extra/clangd/XRefs.h
@@ -124,11 +124,14 @@
format::FormatStyle Style,
const SymbolIndex *Index);
 
-/// Returns reference locations of the symbol at a specified \p Pos.
+struct References {
+  std::vector Refs; // reference locations of the symbol.
+  bool InComplete = false;// true if the result is incomplete.
+};
+/// Returns references of the symbol at a specified \p Pos.
 /// \p Limit limits the number of results returned (0 means no limit).
-std::vector findReferences(ParsedAST &AST, Position Pos,
- uint32_t Limit,
- const SymbolIndex *Index = nullptr);
+References findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
+  const SymbolIndex *Index = nullptr);
 
 /// Get info about symbols at \p Pos.
 std::vector getSymbolInfo(ParsedAST &AST, Position Pos);
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -883,11 +883,12 @@
   return HI;
 }
 
-std::vector findReferences(ParsedAST &AST, Position Pos,
- uint32_t Limit, const SymbolIndex *Index) {
+References findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
+  const SymbolIndex *Index) {
   if (!Limit)
 Limit = std::numeric_limits::max();
-  std::vector Results;
+  References Results;
+  // std::vector Results;
   const SourceManager &SM = AST.getSourceManager();
   auto MainFilePath =
   getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM);
@@ -922,12 +923,11 @@
   Location Result;
   Result.range =

[PATCH] D70357: [clangd] Untangle Hover from XRefs, move into own file.

2019-11-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/Hover.cpp:374
+  } else {
+auto Offset = positionToOffset(SM.getBufferData(SM.getMainFileID()), Pos);
+if (!Offset) {

nit: `SM.getFileOffset(SourceLocationBeg)` ?



Comment at: clang-tools-extra/clangd/Hover.cpp:374
+  } else {
+auto Offset = positionToOffset(SM.getBufferData(SM.getMainFileID()), Pos);
+if (!Offset) {

kadircet wrote:
> nit: `SM.getFileOffset(SourceLocationBeg)` ?
why not just expose getDeclAtPosition in `AST.h` ?



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp:80
   // if we can't resolve the type, return an error message
-  if (DeducedType == llvm::None || DeducedType->isNull()) {
+  if (DeducedType == llvm::None) {
 return createErrorMessage("Could not deduce type for 'auto' type", Inputs);

nit: while there, maybe get rid of braces as well?



Comment at: clang-tools-extra/clangd/unittests/ASTTests.cpp:13
+#include "TestTU.h"
+
+#include "clang/Basic/SourceManager.h"

unintended newline ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70357



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


[PATCH] D70380: [clangd] Expose the xref's incomplete flag to clangdServer API.

2019-11-18 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

Build result: fail - 60148 tests passed, 1 failed and 729 were skipped.

  failed: LLVM.Bindings/Go/go.test

Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70380



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


[clang] c00e5cf - [RISCV] Set triple based on -march flag

2019-11-18 Thread Simon Cook via cfe-commits

Author: Simon Cook
Date: 2019-11-18T10:44:24Z
New Revision: c00e5cf29d49e51701b00382a3f41a4dfe1c0c0f

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

LOG: [RISCV] Set triple based on -march flag

For RISC-V the value provided to -march should determine whether to
compile for 32- or 64-bit RISC-V irrespective of the target provided to
the Clang driver. This adds a test for this flag for RISC-V and sets the
Target architecture correctly in these cases.

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Driver/Driver.cpp
clang/test/Driver/riscv-arch.c

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index aa0d88db1c9f..5c8f3bd13d5a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -112,7 +112,8 @@ future versions of Clang.
 Modified Compiler Flags
 ---
 
-- ...
+- RISC-V now sets the architecture (riscv32/riscv64) based on the value 
provided
+  to the ``-march`` flag, overriding the target provided by ``-triple``.
 
 New Pragmas in Clang
 

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 3fb38a79051c..cdf4a579f431 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -541,6 +541,17 @@ static llvm::Triple computeTargetTriple(const Driver &D,
 }
   }
 
+  // If target is RISC-V adjust the target triple according to
+  // provided architecture name
+  A = Args.getLastArg(options::OPT_march_EQ);
+  if (A && Target.isRISCV()) {
+StringRef ArchName = A->getValue();
+if (ArchName.startswith_lower("rv32"))
+  Target.setArch(llvm::Triple::riscv32);
+else if (ArchName.startswith_lower("rv64"))
+  Target.setArch(llvm::Triple::riscv64);
+  }
+
   return Target;
 }
 

diff  --git a/clang/test/Driver/riscv-arch.c b/clang/test/Driver/riscv-arch.c
index 5329fe87aac7..3e1be9a011bf 100644
--- a/clang/test/Driver/riscv-arch.c
+++ b/clang/test/Driver/riscv-arch.c
@@ -315,3 +315,15 @@
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-X-S-SX-INVAL %s
 // RV32-X-S-SX-INVAL: error: invalid arch name 'rv32ixabc_sdef_sxghi',
 // RV32-X-S-SX-INVAL: unsupported non-standard user-level extension 'xabc'
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-TARGET %s
+// RUN: %clang -target riscv64-unknown-elf -march=rv32i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-TARGET %s
+// RV32-TARGET: "-triple" "riscv32-unknown-unknown-elf"
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv64i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64-TARGET %s
+// RUN: %clang -target riscv64-unknown-elf -march=rv64i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64-TARGET %s
+// RV64-TARGET: "-triple" "riscv64-unknown-unknown-elf"



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


[PATCH] D54214: [RISCV] Set triple based on -march flag

2019-11-18 Thread Simon Cook via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc00e5cf29d49: [RISCV] Set triple based on -march flag 
(authored by simoncook).

Changed prior to commit:
  https://reviews.llvm.org/D54214?vs=228261&id=229784#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54214

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/riscv-arch.c


Index: clang/test/Driver/riscv-arch.c
===
--- clang/test/Driver/riscv-arch.c
+++ clang/test/Driver/riscv-arch.c
@@ -315,3 +315,15 @@
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-X-S-SX-INVAL %s
 // RV32-X-S-SX-INVAL: error: invalid arch name 'rv32ixabc_sdef_sxghi',
 // RV32-X-S-SX-INVAL: unsupported non-standard user-level extension 'xabc'
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-TARGET %s
+// RUN: %clang -target riscv64-unknown-elf -march=rv32i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-TARGET %s
+// RV32-TARGET: "-triple" "riscv32-unknown-unknown-elf"
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv64i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64-TARGET %s
+// RUN: %clang -target riscv64-unknown-elf -march=rv64i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64-TARGET %s
+// RV64-TARGET: "-triple" "riscv64-unknown-unknown-elf"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -541,6 +541,17 @@
 }
   }
 
+  // If target is RISC-V adjust the target triple according to
+  // provided architecture name
+  A = Args.getLastArg(options::OPT_march_EQ);
+  if (A && Target.isRISCV()) {
+StringRef ArchName = A->getValue();
+if (ArchName.startswith_lower("rv32"))
+  Target.setArch(llvm::Triple::riscv32);
+else if (ArchName.startswith_lower("rv64"))
+  Target.setArch(llvm::Triple::riscv64);
+  }
+
   return Target;
 }
 
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -112,7 +112,8 @@
 Modified Compiler Flags
 ---
 
-- ...
+- RISC-V now sets the architecture (riscv32/riscv64) based on the value 
provided
+  to the ``-march`` flag, overriding the target provided by ``-triple``.
 
 New Pragmas in Clang
 


Index: clang/test/Driver/riscv-arch.c
===
--- clang/test/Driver/riscv-arch.c
+++ clang/test/Driver/riscv-arch.c
@@ -315,3 +315,15 @@
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-X-S-SX-INVAL %s
 // RV32-X-S-SX-INVAL: error: invalid arch name 'rv32ixabc_sdef_sxghi',
 // RV32-X-S-SX-INVAL: unsupported non-standard user-level extension 'xabc'
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-TARGET %s
+// RUN: %clang -target riscv64-unknown-elf -march=rv32i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-TARGET %s
+// RV32-TARGET: "-triple" "riscv32-unknown-unknown-elf"
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv64i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64-TARGET %s
+// RUN: %clang -target riscv64-unknown-elf -march=rv64i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64-TARGET %s
+// RV64-TARGET: "-triple" "riscv64-unknown-unknown-elf"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -541,6 +541,17 @@
 }
   }
 
+  // If target is RISC-V adjust the target triple according to
+  // provided architecture name
+  A = Args.getLastArg(options::OPT_march_EQ);
+  if (A && Target.isRISCV()) {
+StringRef ArchName = A->getValue();
+if (ArchName.startswith_lower("rv32"))
+  Target.setArch(llvm::Triple::riscv32);
+else if (ArchName.startswith_lower("rv64"))
+  Target.setArch(llvm::Triple::riscv64);
+  }
+
   return Target;
 }
 
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -112,7 +112,8 @@
 Modified Compiler Flags
 ---
 
-- ...
+- RISC-V now sets the architecture (riscv32/riscv64) based on the value provided
+  to the ``-march`` flag, overriding the target provided by ``-triple``.
 
 New Pragmas in Clang
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D70222: [clangd] Add support for .rsp files in compile_commands.json

2019-11-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment.

Thanks for taking a look into this, the `rsp files` issue has came up before in 
the past but there wasn't enough investment to implement it.

Haven't checked the implementation in detail yet, I believe the layering should 
be different;

This is a common problem for all of the clang-related tools, as they all share 
the same "compilation database" abstraction layer, therefore it would be better 
to implement this at that layer so that other tools (e.g, clang-tidy) can also 
benefit from this.
You can find the related code in 
`clang/include/clang/Tooling/CompilationDatabase.h` and 
`clang/lib/Tooling/CompilationDatabase.cpp`.

Also compilation databases has been historically neglecting `Virtual File 
System` abstractions, it is hard to change it now. But would be great if you 
could try to keep that in mind while performing reads.

So would you mind making such changes ?


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

https://reviews.llvm.org/D70222



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


[PATCH] D70008: [clangd] Store xref for Macros in ParsedAST.

2019-11-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment.

Thanks, looks mostly good, a few more nits.




Comment at: clang-tools-extra/clangd/CollectMacros.h:91-92
   Out.Names.insert(MacroNameTok.getIdentifierInfo()->getName());
-  Out.Ranges.push_back(*Range);
+  if (auto SID =
+   getSymbolID(MacroNameTok.getIdentifierInfo()->getName(), MI, 
SM))
+Out.MacroRefs[*SID].push_back(*Range);

nit: abstract `MacroNameTok.getIdentifierInfo()->getName()` to a variable? we 
also used this on line 90.



Comment at: clang-tools-extra/clangd/XRefs.cpp:898
   getBeginningOfIdentifier(Pos, SM, AST.getASTContext().getLangOpts()));
+
   // TODO: should we handle macros, too?

nit: remove this change.



Comment at: clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp:80
+<< Test;
+EXPECT_THAT(collectKnownReferences(AST.getMacros()), AreMacroRefsFrom(T))
+<< Test;

usaxena95 wrote:
> hokein wrote:
> > usaxena95 wrote:
> > > hokein wrote:
> > > > I might be missing something, I didn't get the motivation of using 
> > > > numbers in the annotation, the code seems just collecting all annotated 
> > > > ranges regardless the value of the number and compare to the actual 
> > > > ranges. it doesn't verify that e.g. in your 2rd cases, the two macros 
> > > > `abc` are different symbols.
> > > > 
> > > > How about just verifying a single defined macro for each case? e.g. the 
> > > > below case, we try to get the symbol id (call `locatedMacroAt`) from 
> > > > the T.point(), retrieve the responding ranges from the MainFileMacros, 
> > > > compare to the annotated ranges.
> > > > 
> > > > ```
> > > > #define [[F^oo]] 1
> > > > int abc = [[Foo]];
> > > > #undef [[Foo]]
> > > > ```
> > > Since the output of CollectMacros is a mapping from SymbolID -> vector, I 
> > > wanted to verify that we form correct groups of ranges (grouped by 
> > > SymbolID).
> > > So a single macro per test case wouldn't be able to test this.
> > > 
> > > > I didn't get the motivation of using numbers in the annotation, the 
> > > > code seems just collecting all annotated ranges regardless the value of 
> > > > the number and compare to the actual ranges. it doesn't verify that 
> > > > e.g. in your 2rd cases, the two macros abc are different symbols.
> > > 
> > > We actually group the ranges by their annotation and form `Matcher 
> > > >>>`. So it checks for the 
> > > correct grouping too.
> > >   
> > I see, having a complex `Matcher  > >>>` would hurt code readability, and the error message 
> > is also not friendly when there are failed testcases.
> > 
> > How about extending it like below? To improve the error message, I think we 
> > should do a string comparison: the test annotation v.s the raw code with 
> > the actual macro references annotated (we'd need to write some code to 
> > compute this actual annotated code, you can see 
> > SemanticHighlightingTests.cpp for reference).  
> > 
> > ```
> > #define $1[[a$1^bc]] 1
> > #undef $1[[abc]]
> > 
> > #define $2[[a$2^bc]] 2
> > #undef $2[[abc]]
> > 
> > #ifdef $Unknown[[abc]]
> > #endif
> > ```
> I have tried to check against each group now. Also verifies the SymbolID. 
> Failures will point the annotation for which the testcase failed.
> Recreating the annotated code from the raw code would become more complex as 
> it would involve maintaining a mapping from SymbolId->Annotation. Works quite 
> nicely for SemanticHighlighting as we have the whole Token stream. Creating 
> the replacements does not sound trivial to me. 
Thanks, this looks better now.



Comment at: clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp:1
+#include "Annotations.h"
+#include "CollectMacros.h"

nit: add missing license.



Comment at: clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp:28
+  )cpp",
+  // FIXME: Locating macro in duplicate definitions doesn't work. Enable
+  // this once LocateMacro is fixed.

This is interesting, by "doesn't work", you mean the function could not locate 
the correct definitions for (the first & second `abc`)?



Comment at: clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp:49
+#endif
+  )cpp",
+  R"cpp(

could you add one more test case where there is a macro reference in another 
macro argument, like:

```
#define M(X) X;
#define $1[[abc]] 123
int s = M($1[[abc]]);
```



Comment at: clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp:74
+for (int I = 1;; I++) {
+  const auto ExpectedRefs = T.ranges(llvm::to_string(I));
+  if (ExpectedRefs.empty())

nit: const auto &



Comment at: clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp:98
+} // namespace clang
\ No newline at end of file


nit: we need newline at EOF.


Repository:
  r

[PATCH] D70357: [clangd] Untangle Hover from XRefs, move into own file.

2019-11-18 Thread Christian Kühnel via Phabricator via cfe-commits
kuhnel added a comment.

I had a look at the build problems:
https://github.com/google/llvm-premerge-checks/issues/56


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70357



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


[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

2019-11-18 Thread Balázs Kéri via Phabricator via cfe-commits
balazske added a comment.

Ping. It should be accepted before I can land it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67545



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


[PATCH] D70359: [clangd] Show values of more expressions on hover

2019-11-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

"mouseover"
all_the_things 

"all the things"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70359



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


[PATCH] D70380: [clangd] Expose the xref's incomplete flag to clangdServer API.

2019-11-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

Oops, LG apart from naming nits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70380



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


[PATCH] D70380: [clangd] Expose the xref's incomplete flag to clangdServer API.

2019-11-18 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.

Let's see how many times I can miss the button


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70380



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


[PATCH] D70380: [clangd] Expose the xref's incomplete flag to clangdServer API.

2019-11-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.h:128
+struct References {
+  std::vector Refs; // reference locations of the symbol.
+  bool InComplete = false;// true if the result is incomplete.

References -> ReferenceList or ReferencesResult, and Refs -> References?

calling the struct References seems unclear, and takes away the useful 
unabbreviated variable name



Comment at: clang-tools-extra/clangd/XRefs.h:128
+struct References {
+  std::vector Refs; // reference locations of the symbol.
+  bool InComplete = false;// true if the result is incomplete.

sammccall wrote:
> References -> ReferenceList or ReferencesResult, and Refs -> References?
> 
> calling the struct References seems unclear, and takes away the useful 
> unabbreviated variable name
these comments don't seem to add much - remove?



Comment at: clang-tools-extra/clangd/XRefs.h:129
+  std::vector Refs; // reference locations of the symbol.
+  bool InComplete = false;// true if the result is incomplete.
+};

we call this `HasMore` in code complete.

(incomplete is one word)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70380



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


[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

2019-11-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri resigned from this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

In D67545#1749561 , @balazske wrote:

> Ping. It should be accepted before I can land it.


@aaron.ballman already accepted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67545



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


[PATCH] D69978: Separately track input and output denormal mode

2019-11-18 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm marked an inline comment as done.
arsenm added inline comments.



Comment at: llvm/docs/LangRef.rst:1822
 
 ``"denorm-fp-mode"``
+   This indicates the subnormal handling that may be assumed for the

andrew.w.kaylor wrote:
> I don't like the definition of this attribute. It's not reader-friendly. The 
> comma-separated pair format has no indication which value refers to inputs 
> and which refers to outputs. Also, while this predates your changes, I think 
> the meanings of the current choices are unclear. 
> 
> What would you think of a comma-separated list with the following 
> possibilities?
> 
> 
> ```
> allow-denormals (default)
> inputs-are-zero (outputs not flushed)
> inputs-are-zero, outputs-are-zero
> inputs-are-zero, outputs-are-positive-zero
> inputs-are-positivezero (outputs not flushed)
> inputs-are-positivezero, outputs-are-zero
> inputs-are-positivezero, outputs-are-positive-zero
> denormal-outputs-are-zero (inputs are unchanged)
> denormal-outputs-are-positive-zero (inputs are unchanged)
> 
> ```
> I'd also be open to abbreviations. I don't know if "daz" and "ftz" are 
> readable to everyone, but I'm more comfortable with them. That would make the 
> options something like this.
> 
> 
> ```
> allow-denormals
> daz
> daz, ftz
> daz, ftz+
> daz+
> daz+, ftz
> daz+, ftz+
> ftz
> ftz+
> ```
I'm trying to avoid needing to autoupgrade bitcode at this point, which leaving 
the names as-is accomplishes. I'm worried this could still end up not in the 
right place, and then we would need another level of auto upgrade to deal with 
it later. I think these are overly verbose (I'm also keeping in mind the fact 
that any use of these does a linear scan through all string attributes, and 
then needs to parse these).

I'm also unclear on what this weird ARM positive-zero really means. Does it 
mean inputs and outputs ignored the sign? Is there value in representing 
positive-zero on both sides?


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

https://reviews.llvm.org/D69978



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


[PATCH] D70222: [clangd] Add support for .rsp files in compile_commands.json

2019-11-18 Thread liu hui via Phabricator via cfe-commits
lh123 added a comment.

In D70222#1749533 , @kadircet wrote:

> Thanks for taking a look into this, the `rsp files` issue has came up before 
> in the past but there wasn't enough investment to implement it.
>
> Haven't checked the implementation in detail yet, I believe the layering 
> should be different;
>
> This is a common problem for all of the clang-related tools, as they all 
> share the same "compilation database" abstraction layer, therefore it would 
> be better to implement this at that layer so that other tools (e.g, 
> clang-tidy) can also benefit from this.
>  You can find the related code in 
> `clang/include/clang/Tooling/CompilationDatabase.h` and 
> `clang/lib/Tooling/CompilationDatabase.cpp`.
>
> Also compilation databases has been historically neglecting `Virtual File 
> System` abstractions, it is hard to change it now. But would be great if you 
> could try to keep that in mind while performing reads.
>
> So would you mind making such changes ?




In D70222#1749533 , @kadircet wrote:

> Thanks for taking a look into this, the `rsp files` issue has came up before 
> in the past but there wasn't enough investment to implement it.
>
> Haven't checked the implementation in detail yet, I believe the layering 
> should be different;
>
> This is a common problem for all of the clang-related tools, as they all 
> share the same "compilation database" abstraction layer, therefore it would 
> be better to implement this at that layer so that other tools (e.g, 
> clang-tidy) can also benefit from this.
>  You can find the related code in 
> `clang/include/clang/Tooling/CompilationDatabase.h` and 
> `clang/lib/Tooling/CompilationDatabase.cpp`.
>
> Also compilation databases has been historically neglecting `Virtual File 
> System` abstractions, it is hard to change it now. But would be great if you 
> could try to keep that in mind while performing reads.
>
> So would you mind making such changes ?


Ok, I will look into this.


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

https://reviews.llvm.org/D70222



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


[clang-tools-extra] 5181ada - [clangd] Expose the xref's incomplete flag to clangdServer API.

2019-11-18 Thread Haojian Wu via cfe-commits

Author: Haojian Wu
Date: 2019-11-18T13:27:19+01:00
New Revision: 5181adab6183d058509ca6da7c1306ced3a61e1c

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

LOG: [clangd] Expose the xref's incomplete flag to clangdServer API.

Summary: so that clangd C++ API users (via ClangdServer) can access it.

Reviewers: sammccall

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

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/XRefs.h
clang-tools-extra/clangd/unittests/XRefsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp 
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 9e24878dc8f6..4fe815818074 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -1152,7 +1152,13 @@ void ClangdLSPServer::onChangeConfiguration(
 void ClangdLSPServer::onReference(const ReferenceParams &Params,
   Callback> Reply) {
   Server->findReferences(Params.textDocument.uri.file(), Params.position,
- CCOpts.Limit, std::move(Reply));
+ CCOpts.Limit,
+ [Reply = std::move(Reply)](
+ llvm::Expected Refs) mutable {
+   if (!Refs)
+ return Reply(Refs.takeError());
+   return Reply(std::move(Refs->References));
+ });
 }
 
 void ClangdLSPServer::onSymbolInfo(const TextDocumentPositionParams &Params,

diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index 64ccbe417c24..5a9833d78b48 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -562,7 +562,7 @@ void ClangdServer::documentSymbols(llvm::StringRef File,
 }
 
 void ClangdServer::findReferences(PathRef File, Position Pos, uint32_t Limit,
-  Callback> CB) {
+  Callback CB) {
   auto Action = [Pos, Limit, CB = std::move(CB),
  this](llvm::Expected InpAST) mutable {
 if (!InpAST)

diff  --git a/clang-tools-extra/clangd/ClangdServer.h 
b/clang-tools-extra/clangd/ClangdServer.h
index c04dc50849f8..fdea9b389e36 100644
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/clang-tools-extra/clangd/ClangdServer.h
@@ -225,7 +225,7 @@ class ClangdServer {
 
   /// Retrieve locations for symbol references.
   void findReferences(PathRef File, Position Pos, uint32_t Limit,
-  Callback> CB);
+  Callback CB);
 
   /// Run formatting for \p Rng inside \p File with content \p Code.
   llvm::Expected formatRange(StringRef Code,

diff  --git a/clang-tools-extra/clangd/XRefs.cpp 
b/clang-tools-extra/clangd/XRefs.cpp
index 2fd147c2d255..d604379b75fc 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -883,11 +883,11 @@ llvm::Optional getHover(ParsedAST &AST, 
Position Pos,
   return HI;
 }
 
-std::vector findReferences(ParsedAST &AST, Position Pos,
- uint32_t Limit, const SymbolIndex *Index) 
{
+ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
+const SymbolIndex *Index) {
   if (!Limit)
 Limit = std::numeric_limits::max();
-  std::vector Results;
+  ReferencesResult Results;
   const SourceManager &SM = AST.getSourceManager();
   auto MainFilePath =
   getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM);
@@ -922,12 +922,11 @@ std::vector findReferences(ParsedAST &AST, 
Position Pos,
   Location Result;
   Result.range = *Range;
   Result.uri = URIForFile::canonicalize(*MainFilePath, *MainFilePath);
-  Results.push_back(std::move(Result));
+  Results.References.push_back(std::move(Result));
 }
   }
-
   // Now query the index for references from other files.
-  if (Index && Results.size() < Limit) {
+  if (Index && Results.References.size() <= Limit) {
 RefsRequest Req;
 Req.Limit = Limit;
 
@@ -942,15 +941,22 @@ std::vector findReferences(ParsedAST &AST, 
Position Pos,
 }
 if (Req.IDs.empty())
   return Results;
-Index->refs(Req, [&](const Ref &R) {
+Results.HasMore |= Index->refs(Req, [&](const Ref &R) {
+  // no need to continue process if we reach the limit.
+  if (Results.References.size() > Limit)
+return;
   auto LSPLoc = toLSPLocat

[PATCH] D70222: [clangd] Add support for .rsp files in compile_commands.json

2019-11-18 Thread liu hui via Phabricator via cfe-commits
lh123 updated this revision to Diff 229807.
lh123 added a comment.

Move the implementation to `JSONCompilationDatabase`.


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

https://reviews.llvm.org/D70222

Files:
  clang/lib/Tooling/JSONCompilationDatabase.cpp

Index: clang/lib/Tooling/JSONCompilationDatabase.cpp
===
--- clang/lib/Tooling/JSONCompilationDatabase.cpp
+++ clang/lib/Tooling/JSONCompilationDatabase.cpp
@@ -24,6 +24,7 @@
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -49,9 +50,9 @@
 /// Assumes \-escaping for quoted arguments (see the documentation of
 /// unescapeCommandLine(...)).
 class CommandLineArgumentParser {
- public:
+public:
   CommandLineArgumentParser(StringRef CommandLine)
-  : Input(CommandLine), Position(Input.begin()-1) {}
+  : Input(CommandLine), Position(Input.begin() - 1) {}
 
   std::vector parse() {
 bool HasMoreInput = true;
@@ -63,46 +64,56 @@
 return CommandLine;
   }
 
- private:
+private:
   // All private methods return true if there is more input available.
 
   bool parseStringInto(std::string &String) {
 do {
   if (*Position == '"') {
-if (!parseDoubleQuotedStringInto(String)) return false;
+if (!parseDoubleQuotedStringInto(String))
+  return false;
   } else if (*Position == '\'') {
-if (!parseSingleQuotedStringInto(String)) return false;
+if (!parseSingleQuotedStringInto(String))
+  return false;
   } else {
-if (!parseFreeStringInto(String)) return false;
+if (!parseFreeStringInto(String))
+  return false;
   }
 } while (*Position != ' ');
 return true;
   }
 
   bool parseDoubleQuotedStringInto(std::string &String) {
-if (!next()) return false;
+if (!next())
+  return false;
 while (*Position != '"') {
-  if (!skipEscapeCharacter()) return false;
+  if (!skipEscapeCharacter())
+return false;
   String.push_back(*Position);
-  if (!next()) return false;
+  if (!next())
+return false;
 }
 return next();
   }
 
   bool parseSingleQuotedStringInto(std::string &String) {
-if (!next()) return false;
+if (!next())
+  return false;
 while (*Position != '\'') {
   String.push_back(*Position);
-  if (!next()) return false;
+  if (!next())
+return false;
 }
 return next();
   }
 
   bool parseFreeStringInto(std::string &String) {
 do {
-  if (!skipEscapeCharacter()) return false;
+  if (!skipEscapeCharacter())
+return false;
   String.push_back(*Position);
-  if (!next()) return false;
+  if (!next())
+return false;
 } while (*Position != ' ' && *Position != '"' && *Position != '\'');
 return true;
   }
@@ -116,7 +127,8 @@
 
   bool nextNonWhitespace() {
 do {
-  if (!next()) return false;
+  if (!next())
+return false;
 } while (*Position == ' ');
 return true;
   }
@@ -158,6 +170,124 @@
   return parser.parse();
 }
 
+bool expandResponseFile(llvm::StringRef FName, llvm::StringSaver &Saver,
+llvm::cl::TokenizerCallback Tokenizer,
+SmallVectorImpl &NewArgv) {
+  llvm::ErrorOr> MemBufOrErr =
+  llvm::MemoryBuffer::getFile(FName);
+  if (!MemBufOrErr)
+return false;
+  llvm::MemoryBuffer &MemBuf = *MemBufOrErr.get();
+  StringRef Str(MemBuf.getBufferStart(), MemBuf.getBufferSize());
+
+  // If we have a UTF-16 byte order mark, convert to UTF-8 for parsing.
+  ArrayRef BufRef(MemBuf.getBufferStart(), MemBuf.getBufferEnd());
+  std::string UTF8Buf;
+  // It is called byte order marker but the UTF-8 BOM is actually not affected
+  // by the host system's endianness.
+  auto HasUtF8ByteOrderMark = [](ArrayRef S) {
+return (S.size() >= 3 && S[0] == '\xef' && S[1] == '\xbb' &&
+S[2] == '\xbf');
+  };
+  if (llvm::hasUTF16ByteOrderMark(BufRef)) {
+if (!convertUTF16ToUTF8String(BufRef, UTF8Buf))
+  return false;
+Str = StringRef(UTF8Buf);
+  }
+  // If we see UTF-8 BOM sequence at the beginning of a file, we shall remove
+  // these bytes before parsing.
+  // Reference: http://en.wikipedia.org/wiki/UTF-8#Byte_order_mark
+  else if (HasUtF8ByteOrderMark(BufRef))
+Str = StringRef(BufRef.data() + 3, BufRef.size() - 3);
+  // Tokenize the contents into NewArgv.
+  Tokenizer(Str, Saver, NewArgv, false);
+  return true;
+}
+
+bool expandResponseFiles(tooling::CompileCommand &Cmd,
+ llvm::cl::TokenizerCallback Tokenizer) {
+  bool AllExpanded = true;
+  struct ResponseFileRecord {
+llvm::StringRef File;
+size_t End;
+  };
+  std::vector &Argv = Cmd.CommandLine;
+  // To detect recursive res

[PATCH] D70380: [clangd] Expose the xref's incomplete flag to clangdServer API.

2019-11-18 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5181adab6183: [clangd] Expose the xref's incomplete 
flag to clangdServer API. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70380

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -2109,7 +2109,7 @@
 std::vector> ExpectedLocations;
 for (const auto &R : T.ranges())
   ExpectedLocations.push_back(RangeIs(R));
-EXPECT_THAT(findReferences(AST, T.point(), 0),
+EXPECT_THAT(findReferences(AST, T.point(), 0).References,
 ElementsAreArray(ExpectedLocations))
 << Test;
   }
@@ -2170,7 +2170,7 @@
 for (const auto &R : T.ranges())
   ExpectedLocations.push_back(RangeIs(R));
 ASSERT_THAT(ExpectedLocations, Not(IsEmpty()));
-EXPECT_THAT(findReferences(AST, T.point(), 0),
+EXPECT_THAT(findReferences(AST, T.point(), 0).References,
 ElementsAreArray(ExpectedLocations))
 << Test;
   }
@@ -2185,8 +2185,9 @@
   auto AST = TU.build();
 
   // References in main file are returned without index.
-  EXPECT_THAT(findReferences(AST, Main.point(), 0, /*Index=*/nullptr),
-  ElementsAre(RangeIs(Main.range(;
+  EXPECT_THAT(
+  findReferences(AST, Main.point(), 0, /*Index=*/nullptr).References,
+  ElementsAre(RangeIs(Main.range(;
   Annotations IndexedMain(R"cpp(
 int main() { [[f^oo]](); }
   )cpp");
@@ -2196,17 +2197,18 @@
   IndexedTU.Code = IndexedMain.code();
   IndexedTU.Filename = "Indexed.cpp";
   IndexedTU.HeaderCode = Header;
-  EXPECT_THAT(findReferences(AST, Main.point(), 0, IndexedTU.index().get()),
-  ElementsAre(RangeIs(Main.range()), RangeIs(IndexedMain.range(;
-
-  EXPECT_EQ(1u, findReferences(AST, Main.point(), /*Limit*/ 1,
-   IndexedTU.index().get())
-.size());
+  EXPECT_THAT(
+  findReferences(AST, Main.point(), 0, IndexedTU.index().get()).References,
+  ElementsAre(RangeIs(Main.range()), RangeIs(IndexedMain.range(;
+  auto LimitRefs =
+  findReferences(AST, Main.point(), /*Limit*/ 1, IndexedTU.index().get());
+  EXPECT_EQ(1u, LimitRefs.References.size());
+  EXPECT_TRUE(LimitRefs.HasMore);
 
   // If the main file is in the index, we don't return duplicates.
   // (even if the references are in a different location)
   TU.Code = ("\n\n" + Main.code()).str();
-  EXPECT_THAT(findReferences(AST, Main.point(), 0, TU.index().get()),
+  EXPECT_THAT(findReferences(AST, Main.point(), 0, TU.index().get()).References,
   ElementsAre(RangeIs(Main.range(;
 }
 
Index: clang-tools-extra/clangd/XRefs.h
===
--- clang-tools-extra/clangd/XRefs.h
+++ clang-tools-extra/clangd/XRefs.h
@@ -124,11 +124,14 @@
format::FormatStyle Style,
const SymbolIndex *Index);
 
-/// Returns reference locations of the symbol at a specified \p Pos.
+struct ReferencesResult {
+  std::vector References;
+  bool HasMore = false;
+};
+/// Returns references of the symbol at a specified \p Pos.
 /// \p Limit limits the number of results returned (0 means no limit).
-std::vector findReferences(ParsedAST &AST, Position Pos,
- uint32_t Limit,
- const SymbolIndex *Index = nullptr);
+ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
+const SymbolIndex *Index = nullptr);
 
 /// Get info about symbols at \p Pos.
 std::vector getSymbolInfo(ParsedAST &AST, Position Pos);
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -883,11 +883,11 @@
   return HI;
 }
 
-std::vector findReferences(ParsedAST &AST, Position Pos,
- uint32_t Limit, const SymbolIndex *Index) {
+ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
+const SymbolIndex *Index) {
   if (!Limit)
 Limit = std::numeric_limits::max();
-  std::vector Results;
+  ReferencesResult Results;
   const SourceManager &SM = AST.getSourceManager();
   auto MainFilePath =
   getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM);
@@ -922,12 +922,11 @@
   L

[PATCH] D70380: [clangd] Expose the xref's incomplete flag to clangdServer API.

2019-11-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 229808.
hokein marked 3 inline comments as done.
hokein added a comment.

address naming comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70380

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -2109,7 +2109,7 @@
 std::vector> ExpectedLocations;
 for (const auto &R : T.ranges())
   ExpectedLocations.push_back(RangeIs(R));
-EXPECT_THAT(findReferences(AST, T.point(), 0),
+EXPECT_THAT(findReferences(AST, T.point(), 0).References,
 ElementsAreArray(ExpectedLocations))
 << Test;
   }
@@ -2170,7 +2170,7 @@
 for (const auto &R : T.ranges())
   ExpectedLocations.push_back(RangeIs(R));
 ASSERT_THAT(ExpectedLocations, Not(IsEmpty()));
-EXPECT_THAT(findReferences(AST, T.point(), 0),
+EXPECT_THAT(findReferences(AST, T.point(), 0).References,
 ElementsAreArray(ExpectedLocations))
 << Test;
   }
@@ -2185,8 +2185,9 @@
   auto AST = TU.build();
 
   // References in main file are returned without index.
-  EXPECT_THAT(findReferences(AST, Main.point(), 0, /*Index=*/nullptr),
-  ElementsAre(RangeIs(Main.range(;
+  EXPECT_THAT(
+  findReferences(AST, Main.point(), 0, /*Index=*/nullptr).References,
+  ElementsAre(RangeIs(Main.range(;
   Annotations IndexedMain(R"cpp(
 int main() { [[f^oo]](); }
   )cpp");
@@ -2196,17 +2197,18 @@
   IndexedTU.Code = IndexedMain.code();
   IndexedTU.Filename = "Indexed.cpp";
   IndexedTU.HeaderCode = Header;
-  EXPECT_THAT(findReferences(AST, Main.point(), 0, IndexedTU.index().get()),
-  ElementsAre(RangeIs(Main.range()), RangeIs(IndexedMain.range(;
-
-  EXPECT_EQ(1u, findReferences(AST, Main.point(), /*Limit*/ 1,
-   IndexedTU.index().get())
-.size());
+  EXPECT_THAT(
+  findReferences(AST, Main.point(), 0, IndexedTU.index().get()).References,
+  ElementsAre(RangeIs(Main.range()), RangeIs(IndexedMain.range(;
+  auto LimitRefs =
+  findReferences(AST, Main.point(), /*Limit*/ 1, IndexedTU.index().get());
+  EXPECT_EQ(1u, LimitRefs.References.size());
+  EXPECT_TRUE(LimitRefs.HasMore);
 
   // If the main file is in the index, we don't return duplicates.
   // (even if the references are in a different location)
   TU.Code = ("\n\n" + Main.code()).str();
-  EXPECT_THAT(findReferences(AST, Main.point(), 0, TU.index().get()),
+  EXPECT_THAT(findReferences(AST, Main.point(), 0, TU.index().get()).References,
   ElementsAre(RangeIs(Main.range(;
 }
 
Index: clang-tools-extra/clangd/XRefs.h
===
--- clang-tools-extra/clangd/XRefs.h
+++ clang-tools-extra/clangd/XRefs.h
@@ -124,11 +124,14 @@
format::FormatStyle Style,
const SymbolIndex *Index);
 
-/// Returns reference locations of the symbol at a specified \p Pos.
+struct ReferencesResult {
+  std::vector References;
+  bool HasMore = false;
+};
+/// Returns references of the symbol at a specified \p Pos.
 /// \p Limit limits the number of results returned (0 means no limit).
-std::vector findReferences(ParsedAST &AST, Position Pos,
- uint32_t Limit,
- const SymbolIndex *Index = nullptr);
+ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
+const SymbolIndex *Index = nullptr);
 
 /// Get info about symbols at \p Pos.
 std::vector getSymbolInfo(ParsedAST &AST, Position Pos);
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -883,11 +883,11 @@
   return HI;
 }
 
-std::vector findReferences(ParsedAST &AST, Position Pos,
- uint32_t Limit, const SymbolIndex *Index) {
+ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
+const SymbolIndex *Index) {
   if (!Limit)
 Limit = std::numeric_limits::max();
-  std::vector Results;
+  ReferencesResult Results;
   const SourceManager &SM = AST.getSourceManager();
   auto MainFilePath =
   getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM);
@@ -922,12 +922,11 @@
   Location Result;
   Result.range = *Range;
   Resu

[PATCH] D70222: [clangd] Add support for .rsp files in compile_commands.json

2019-11-18 Thread liu hui via Phabricator via cfe-commits
lh123 updated this revision to Diff 229812.
lh123 added a comment.

Respect `JSONCommandLineSyntax`


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

https://reviews.llvm.org/D70222

Files:
  clang/lib/Tooling/JSONCompilationDatabase.cpp

Index: clang/lib/Tooling/JSONCompilationDatabase.cpp
===
--- clang/lib/Tooling/JSONCompilationDatabase.cpp
+++ clang/lib/Tooling/JSONCompilationDatabase.cpp
@@ -24,6 +24,7 @@
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -49,9 +50,9 @@
 /// Assumes \-escaping for quoted arguments (see the documentation of
 /// unescapeCommandLine(...)).
 class CommandLineArgumentParser {
- public:
+public:
   CommandLineArgumentParser(StringRef CommandLine)
-  : Input(CommandLine), Position(Input.begin()-1) {}
+  : Input(CommandLine), Position(Input.begin() - 1) {}
 
   std::vector parse() {
 bool HasMoreInput = true;
@@ -63,46 +64,56 @@
 return CommandLine;
   }
 
- private:
+private:
   // All private methods return true if there is more input available.
 
   bool parseStringInto(std::string &String) {
 do {
   if (*Position == '"') {
-if (!parseDoubleQuotedStringInto(String)) return false;
+if (!parseDoubleQuotedStringInto(String))
+  return false;
   } else if (*Position == '\'') {
-if (!parseSingleQuotedStringInto(String)) return false;
+if (!parseSingleQuotedStringInto(String))
+  return false;
   } else {
-if (!parseFreeStringInto(String)) return false;
+if (!parseFreeStringInto(String))
+  return false;
   }
 } while (*Position != ' ');
 return true;
   }
 
   bool parseDoubleQuotedStringInto(std::string &String) {
-if (!next()) return false;
+if (!next())
+  return false;
 while (*Position != '"') {
-  if (!skipEscapeCharacter()) return false;
+  if (!skipEscapeCharacter())
+return false;
   String.push_back(*Position);
-  if (!next()) return false;
+  if (!next())
+return false;
 }
 return next();
   }
 
   bool parseSingleQuotedStringInto(std::string &String) {
-if (!next()) return false;
+if (!next())
+  return false;
 while (*Position != '\'') {
   String.push_back(*Position);
-  if (!next()) return false;
+  if (!next())
+return false;
 }
 return next();
   }
 
   bool parseFreeStringInto(std::string &String) {
 do {
-  if (!skipEscapeCharacter()) return false;
+  if (!skipEscapeCharacter())
+return false;
   String.push_back(*Position);
-  if (!next()) return false;
+  if (!next())
+return false;
 } while (*Position != ' ' && *Position != '"' && *Position != '\'');
 return true;
   }
@@ -116,7 +127,8 @@
 
   bool nextNonWhitespace() {
 do {
-  if (!next()) return false;
+  if (!next())
+return false;
 } while (*Position == ' ');
 return true;
   }
@@ -158,6 +170,124 @@
   return parser.parse();
 }
 
+bool expandResponseFile(llvm::StringRef FName, llvm::StringSaver &Saver,
+llvm::cl::TokenizerCallback Tokenizer,
+SmallVectorImpl &NewArgv) {
+  llvm::ErrorOr> MemBufOrErr =
+  llvm::MemoryBuffer::getFile(FName);
+  if (!MemBufOrErr)
+return false;
+  llvm::MemoryBuffer &MemBuf = *MemBufOrErr.get();
+  StringRef Str(MemBuf.getBufferStart(), MemBuf.getBufferSize());
+
+  // If we have a UTF-16 byte order mark, convert to UTF-8 for parsing.
+  ArrayRef BufRef(MemBuf.getBufferStart(), MemBuf.getBufferEnd());
+  std::string UTF8Buf;
+  // It is called byte order marker but the UTF-8 BOM is actually not affected
+  // by the host system's endianness.
+  auto HasUtF8ByteOrderMark = [](ArrayRef S) {
+return (S.size() >= 3 && S[0] == '\xef' && S[1] == '\xbb' &&
+S[2] == '\xbf');
+  };
+  if (llvm::hasUTF16ByteOrderMark(BufRef)) {
+if (!convertUTF16ToUTF8String(BufRef, UTF8Buf))
+  return false;
+Str = StringRef(UTF8Buf);
+  }
+  // If we see UTF-8 BOM sequence at the beginning of a file, we shall remove
+  // these bytes before parsing.
+  // Reference: http://en.wikipedia.org/wiki/UTF-8#Byte_order_mark
+  else if (HasUtF8ByteOrderMark(BufRef))
+Str = StringRef(BufRef.data() + 3, BufRef.size() - 3);
+  // Tokenize the contents into NewArgv.
+  Tokenizer(Str, Saver, NewArgv, false);
+  return true;
+}
+
+bool expandResponseFiles(tooling::CompileCommand &Cmd,
+ llvm::cl::TokenizerCallback Tokenizer) {
+  bool AllExpanded = true;
+  struct ResponseFileRecord {
+llvm::StringRef File;
+size_t End;
+  };
+  std::vector &Argv = Cmd.CommandLine;
+  // To detect recursive response files, we mainta

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

2019-11-18 Thread Clement Courbet via Phabricator via cfe-commits
courbet created this revision.
courbet added a reviewer: aaron.ballman.
Herald added subscribers: xazax.hun, mgorny.
Herald added a project: clang.

The check flags constructs that prevent automatic move of local variables.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70390

Files:
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp
  clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.h
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst
  clang-tools-extra/test/clang-tidy/checkers/performance-no-automatic-move.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-no-automatic-move.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-no-automatic-move.cpp
@@ -0,0 +1,165 @@
+// RUN: %check_clang_tidy -std=c++11,c++14,c++17 %s performance-no-automatic-move %t
+
+namespace std {
+template 
+struct remove_reference;
+
+template 
+struct remove_reference {
+  typedef _Tp type;
+};
+
+template 
+struct remove_reference<_Tp &> {
+  typedef _Tp type;
+};
+
+template 
+struct remove_reference<_Tp &&> {
+  typedef _Tp type;
+};
+
+template 
+constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) {
+  return static_cast::type &&>(__t);
+}
+
+template 
+constexpr _Tp &&
+forward(typename remove_reference<_Tp>::type &__t) noexcept {
+  return static_cast<_Tp &&>(__t);
+}
+
+} // namespace std
+
+struct Obj {
+  Obj();
+  Obj(const Obj &);
+  Obj(Obj &&);
+  virtual ~Obj();
+};
+
+template 
+struct StatusOr {
+  StatusOr(const T &);
+  StatusOr(T &&);
+};
+
+struct NonTemplate {
+  NonTemplate(const Obj &);
+  NonTemplate(Obj &&);
+};
+
+template 
+T Make();
+
+StatusOr PositiveStatusOrConstValue() {
+  const Obj obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+NonTemplate PositiveNonTemplateConstValue() {
+  const Obj obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+Obj PositiveSelfConstValue() {
+  const Obj obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+StatusOr PositiveStatusOrRefRef() {
+  Obj &&obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: rvalue-ness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+StatusOr PositiveNonTemplateRefRef() {
+  Obj &&obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: rvalue-ness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+StatusOr PositiveSelfRefRef() {
+  Obj &&obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: rvalue-ness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+StatusOr PositiveStatusOrConstRefRef() {
+  const Obj &&obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: rvalue-ness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+NonTemplate PositiveNonTemplateConstRefRef() {
+  const Obj &&obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: rvalue-ness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+NonTemplate PositiveSelfConstRefRef() {
+  const Obj &&obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: rvalue-ness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+// FIXME: Ideally we would warn here too.
+NonTemplate PositiveNonTemplateLifetimeExtension() {
+  const Obj &obj = Make();
+  return obj;
+}
+
+// FIXME: Ideally we would warn here too.
+StatusOr PositiveStatusOrLifetimeExtension() {
+  const Obj &obj = Make();
+  return obj;
+}
+
+// Negatives.
+
+StatusOr Temporary() {
+  return Make();
+}
+
+StatusOr ConstTemporary() {
+  return Make();
+}
+
+StatusOr Nrvo() {
+  Obj obj = Make();
+  return obj;
+}
+
+StatusOr Ref() {
+  Obj &obj = Make();
+  return obj;
+}
+
+StatusOr ConstRef() {
+  const Obj &obj = Make();
+  return obj;
+}
+
+NonTemplate RefRefMove() {
+  Obj &&obj = Make();
+  return std::move(obj);
+}
+
+const Obj global;
+
+StatusOr Global() {
+  return global;
+}
+
+struct FromConstRefOnly {
+  FromConstRefOnly(const Obj &);
+};
+
+FromConstRefOnly FromConstRefOnly() {
+  const Obj obj = Make();
+  return obj;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/performance-no-a

[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

2019-11-18 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

There is a `ExprMutAnalyzer` that is able to find mutation of expressions in 
general (even though it is kinda experimental still). Maybe that should be 
utilized somehow? I think the current implementation does not cover when the 
address is taken and mutation happens through pointers/references in free 
standing functions, does it?

On the other hand it makes the check more complicated, slower. Additionally the 
most cases are catched with this version, i guess.


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

https://reviews.llvm.org/D70052



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


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

2019-11-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 229815.
hokein marked an inline comment as done.
hokein added a comment.

rebase and call getMacroArgExpandedLocation in prepareRename.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69263

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

Index: clang-tools-extra/clangd/unittests/SyncAPI.h
===
--- clang-tools-extra/clangd/unittests/SyncAPI.h
+++ clang-tools-extra/clangd/unittests/SyncAPI.h
@@ -38,8 +38,8 @@
 llvm::Expected>
 runFindDocumentHighlights(ClangdServer &Server, PathRef File, Position Pos);
 
-llvm::Expected>
-runRename(ClangdServer &Server, PathRef File, Position Pos, StringRef NewName);
+llvm::Expected runRename(ClangdServer &Server, PathRef File,
+Position Pos, StringRef NewName);
 
 std::string runDumpAST(ClangdServer &Server, PathRef File);
 
Index: clang-tools-extra/clangd/unittests/SyncAPI.cpp
===
--- clang-tools-extra/clangd/unittests/SyncAPI.cpp
+++ clang-tools-extra/clangd/unittests/SyncAPI.cpp
@@ -96,10 +96,9 @@
   return std::move(*Result);
 }
 
-llvm::Expected> runRename(ClangdServer &Server,
-PathRef File, Position Pos,
-llvm::StringRef NewName) {
-  llvm::Optional>> Result;
+llvm::Expected runRename(ClangdServer &Server, PathRef File,
+Position Pos, llvm::StringRef NewName) {
+  llvm::Optional> Result;
   Server.rename(File, Pos, NewName, /*WantFormat=*/true, capture(Result));
   return std::move(*Result);
 }
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -9,8 +9,10 @@
 #include "Annotations.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "index/Ref.h"
 #include "refactor/Rename.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "llvm/Support/MemoryBuffer.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -18,8 +20,45 @@
 namespace clangd {
 namespace {
 
-MATCHER_P2(RenameRange, Code, Range, "") {
-  return replacementToEdit(Code, arg).range == Range;
+using testing::Eq;
+using testing::Pair;
+using testing::UnorderedElementsAre;
+
+// Build a RefSlab from all marked ranges in the annotation. The ranges are
+// assumed to associate with the given SymbolName.
+std::unique_ptr buildRefSlab(const Annotations &Code,
+  llvm::StringRef SymbolName,
+  llvm::StringRef Path) {
+  RefSlab::Builder Builder;
+  TestTU TU;
+  TU.HeaderCode = Code.code();
+  auto Symbols = TU.headerSymbols();
+  const auto &SymbolID = findSymbol(Symbols, SymbolName).ID;
+  for (const auto &Range : Code.ranges()) {
+Ref R;
+R.Kind = RefKind::Reference;
+R.Location.Start.setLine(Range.start.line);
+R.Location.Start.setColumn(Range.start.character);
+R.Location.End.setLine(Range.end.line);
+R.Location.End.setColumn(Range.end.character);
+auto U = URI::create(Path).toString();
+R.Location.FileURI = U.c_str();
+Builder.insert(SymbolID, R);
+  }
+
+  return std::make_unique(std::move(Builder).build());
+}
+
+std::vector<
+std::pair>
+applyEdits(FileEdits FE) {
+  std::vector> Results;
+  for (auto &It : FE)
+Results.emplace_back(
+It.first().str(),
+llvm::cantFail(tooling::applyAllReplacements(
+It.getValue().InitialCode, It.getValue().Replacements)));
+  return Results;
 }
 
 // Generates an expected rename result by replacing all ranges in the given
@@ -73,11 +112,11 @@
 auto AST = TU.build();
 llvm::StringRef NewName = "abcde";
 auto RenameResult =
-renameWithinFile(AST, testPath(TU.Filename), Code.point(), NewName);
+rename({Code.point(), NewName, AST, testPath(TU.Filename)});
 ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError();
-auto ApplyResult = llvm::cantFail(
-tooling::applyAllReplacements(Code.code(), *RenameResult));
-EXPECT_EQ(expectedResult(Code, NewName), ApplyResult);
+ASSERT_EQ(1u, RenameResult->size());
+EXPECT_EQ(applyEdits(std::move(

[PATCH] D69934: [clangd] Implement rename by using SelectionTree and findExplicitReferences.

2019-11-18 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

Thanks, LG. The only important is about testing the `foo.inc` case separately 
from the rest of rename tests.




Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:180
 
+const NamedDecl &getPrimaryTemplateOrThis(const NamedDecl &ND) {
+  if (const auto *T = ND.getDescribedTemplate())

NIT: inline this into the single caller.
Should simplify the code.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:258
+// Filter out locations not from main file.
+// We are safe for most cases as we only visit main file AST, but not
+// always, locations could come from an #include file, e.g.

NIT: could probably shorten the whole sentence to `We traverse only main file 
decls, but locations could come from an #include file, e.g.`



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:45
   llvm::StringRef Tests[] = {
-  // Rename function.
+  // rename function
   R"cpp(

NIT: Keep periods at the end



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:371
 auto TU = TestTU::withCode(Code.code());
+TU.AdditionalFiles["foo.inc"] = R"cpp(
+  #define Macro(X) X

Maybe test this separately? It's only used in one test.
Having this added for all tests cases is somewhat confusing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69934



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


[PATCH] D70222: [clangd] Add support for .rsp files in compile_commands.json

2019-11-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment.

Thanks, layering seems better now. But as Sam mentioned in the issue it would 
be better to have something like `expandRSPFiles` similar to 
`inferMissingCompileCommands`.
As the problem is not in the `JSONCompilationDatabase` itself, it can also 
occur in `FixedCompilationDatabase` or any other models clang might've in the 
future.

The CompilationDatabase returned by `expandRSPFiles` could wrap any other `CDB` 
and upon querying of commands, it can expand any RSP files returned by the 
underlying CDB.

This will also ensure minimum changes to the existing code, reducing any 
chances of breakage. As for unittests, please see 
`clang/unittests/Tooling/CompilationDatabaseTest.cpp`

and a small nit, when modifying existing files, please only run formatting on 
the lines you touched to preserve history.


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

https://reviews.llvm.org/D70222



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


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

2019-11-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein marked an inline comment as done.
hokein added a comment.

In D69263#1738289 , @ilya-biryukov 
wrote:

> LGTM.
>
> It's probably worth collecting a list of things we need to fix before 
> enabling cross-file rename and putting it somewhere (a GitHub issue, maybe?)
>  Important things that immediately come to mind:
>
> - add range-patching heuristics, measure how good they are
> - avoid `O(N^2)` when computing edits (converting from positions to offsets)
> - handle implicit references from the index
>
>   There are definitely more.


Thanks, filed at issues#193.




Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:345
+  SourceLocation SourceLocationBeg =
+  SM.getMacroArgExpandedLocation(getBeginningOfIdentifier(
+  RInputs.Pos, SM, AST.getASTContext().getLangOpts()));

ilya-biryukov wrote:
> hokein wrote:
> > ilya-biryukov wrote:
> > > Why is this different from `prepareRename`, which does not call 
> > > `getMacroArgExpandedLocation`?
> > > 
> > I didn't change it in this patch, but you raise a good point, 
> > `prepareRename` should call `getMacroArgExpandedLocation`.
> Yep, makes sense to change it later. Could you put a FIXME, so that we don't 
> forget about it?
Done, fixed this in this patch, it is trivial.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69263



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


[PATCH] D45444: [clang-tidy] implement new check for const-correctness

2019-11-18 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.
Herald added a subscriber: mgehre.

In D45444#1685995 , @tsdgeos wrote:

> Would this warn with stuff like
>
>   double borderWidth;
>   [... code that doesn't use borderWidth ...]
>   borderWidth = border->getWidth(); 
>   [... code that reads borderWidth ...]
>  
>


Warn in what sense? That `borderWidth` could be initialized early and then be 
`const`?
No. It would only suggest `const` (if configured to make values `const`) when 
`borderWidth` is initialized and then untouched.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D45444



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


[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

2019-11-18 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment.

In D69825#1747539 , @aganea wrote:

> @hans : Simply because `ExecuteCC1Tool()` lives in the clang tool 
> (`clang/tools/driver/driver.cpp`) and we're calling it from the 
> clangDriver.lib (`clang/lib/Driver/Job.cpp`). The clangDriver.lib is linked 
> into many other tools (clang-refactor, clang-rename, clang-diff, clang-check, 
> libclang, ...) If I reference the cc1 function directly, we end of with 
> linker errors in all those tools.
>  We could //maybe// move the tool code into the clangDriver.lib, but I'm not 
> sure it's practical as the tool pulls on many other libraries. I thought the 
> callback was the path of least resistance. Let me know if we could do it 
> otherwise.


Oh, I see. I didn't think about that. That's annoying, but maybe using the 
variable makes sense then.




Comment at: clang/include/clang/Driver/Driver.h:207
 
+  /// Callback to the CC1 tool, if available
+  typedef int(*CC1ToolFunc)(ArrayRef argv);

It's not really a callback though. How about just "Pointer to the 
ExecuteCC1Tool function, if available."
It would also be good if the comment explained why the pointer is needed.
And why does it need to be thread-local? Can different threads end up with 
different values for this?



Comment at: clang/include/clang/Driver/Job.h:136
+  /// instead of creating a new process. This is an optimization for speed.
+  static LLVM_THREAD_LOCAL bool ExecuteCC1Tool;
 };

Is there another way of avoiding in-process-cc1 after a crash? For example, 
could the Job know that it's being built for generating the crash report? It 
seems unfortunate to introduce this widely-scoped variable here.



Comment at: clang/lib/Driver/Job.cpp:339
 
-  if (ResponseFile == nullptr) {
+  Driver::CC1ToolFunc CC1Main{};
+

I don't think this form of initialization is common in LLVM code.



Comment at: clang/lib/Driver/Job.cpp:347
+StringRef DriverExe = llvm::sys::path::stem(D.ClangExecutable);
+if (CommandExe.equals_lower(DriverExe))
+CC1Main = Driver::CC1Main;

Now that we're not calling main() anymore, but cc1 directly -- is this checking 
still necessary?



Comment at: clang/tools/driver/driver.cpp:308
+int ExecuteCC1Tool(ArrayRef argv) {
+  // If we re-enter the cc1 tool from the driver process, we should cleanup the
+  // usage count for the driver options (which might be used in the cc1 tool)

It's not really re-entering. Maybe "If we call the cc1 tool directly in the 
driver process" is clearer?



Comment at: llvm/include/llvm/Support/CrashRecoveryContext.h:105
+  /// In case of a crash, this is the crash identifier
+  int RetCode{};
+

I don't think this form of initialization is common in LLVM. Same below.



Comment at: llvm/lib/Support/CrashRecoveryContext.cpp:23
+// In llvm/lib/Support/Windows/Signals.inc
+LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep);
+#endif

Can we move this declaration, and the one for SignalHandler, into some header 
file? Declaring it manually like this doesn't seem so nice.


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

https://reviews.llvm.org/D69825



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


[PATCH] D45444: [clang-tidy] implement new check for const-correctness

2019-11-18 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 229818.
JonasToth added a comment.

- update license
- rebase to master and patch for adding const


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D45444

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
  clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
  clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
  clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
  clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
  clang-tools-extra/clang-tidy/utils/LexerUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-const-correctness.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/cppcoreguidelines-const-correctness-pointer-as-values.cpp
  
clang-tools-extra/test/clang-tidy/cppcoreguidelines-const-correctness-values.cpp
  clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp
  clang-tools-extra/unittests/clang-tidy/CMakeLists.txt

Index: clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
===
--- clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
+++ clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
@@ -7,6 +7,7 @@
 include_directories(${CLANG_LINT_SOURCE_DIR})
 
 add_extra_unittest(ClangTidyTests
+  AddConstTest.cpp
   ClangTidyDiagnosticConsumerTest.cpp
   ClangTidyOptionsTest.cpp
   IncludeInserterTest.cpp
Index: clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp
===
--- /dev/null
+++ clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp
@@ -0,0 +1,982 @@
+#include "../clang-tidy/utils/FixItHintUtils.h"
+#include "ClangTidyTest.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace tidy {
+
+namespace {
+using namespace clang::ast_matchers;
+using namespace utils::fixit;
+
+template 
+class ConstTransform : public ClangTidyCheck {
+public:
+  ConstTransform(StringRef CheckName, ClangTidyContext *Context)
+  : ClangTidyCheck(CheckName, Context) {}
+
+  void registerMatchers(MatchFinder *Finder) override {
+Finder->addMatcher(varDecl(hasName("target")).bind("var"), this);
+  }
+
+  void check(const MatchFinder::MatchResult &Result) override {
+const auto *D = Result.Nodes.getNodeAs("var");
+using utils::fixit::addQualifierToVarDecl;
+Optional Fix = addQualifierToVarDecl(*D, DeclSpec::TQ::TQ_const,
+CT, CP, Result.Context);
+auto Diag = diag(D->getBeginLoc(), "doing const transformation");
+if (Fix)
+  Diag << *Fix;
+  }
+};
+} // namespace
+
+namespace test {
+using PointeeLTransform =
+ConstTransform;
+using PointeeRTransform =
+ConstTransform;
+
+using ValueLTransform =
+ConstTransform;
+using ValueRTransform =
+ConstTransform;
+
+// 
+// Test Value-like types. Everything with indirection is done later.
+// 
+
+TEST(Values, Builtin) {
+  StringRef Snippet = "int target = 0;";
+
+  EXPECT_EQ("const int target = 0;", runCheckOnCode(Snippet));
+  EXPECT_EQ("const int target = 0;",
+runCheckOnCode(Snippet));
+
+  EXPECT_EQ("int const target = 0;", runCheckOnCode(Snippet));
+  EXPECT_EQ("int const target = 0;",
+runCheckOnCode(Snippet));
+}
+TEST(Values, TypedefBuiltin) {
+  StringRef T = "typedef int MyInt;";
+  StringRef S = "MyInt target = 0;";
+  auto Cat = [&T](StringRef S) { return (T + S).str(); };
+
+  EXPECT_EQ(Cat("const MyInt target = 0;"),
+runCheckOnCode(Cat(S)));
+  EXPECT_EQ(Cat("const MyInt target = 0;"),
+runCheckOnCode(Cat(S)));
+
+  EXPECT_EQ(Cat("MyInt const target = 0;"),
+runCheckOnCode(Cat(S)));
+  EXPECT_EQ(Cat("MyInt const target = 0;"),
+runCheckOnCode(Cat(S)));
+}
+TEST(Values, TypedefBuiltinPointer) {
+  StringRef T = "typedef int* MyInt;";
+  StringRef S = "MyInt target = nullptr;";
+  auto Cat = [&T](StringRef S) { return (T + S).str(); };
+
+  EXPECT_EQ(Cat("const MyInt target = nullptr;"),
+runCheckOnCode(Cat(S)));
+  EXPECT_EQ(Cat("const MyInt target = nullptr;"),
+runCheckOnCode(Cat(S)));
+
+  EXPECT_EQ(Cat("MyInt const target = nullptr;"

[PATCH] D69934: [clangd] Implement rename by using SelectionTree and findExplicitReferences.

2019-11-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 229822.
hokein marked 4 inline comments as done.
hokein added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69934

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
@@ -38,27 +38,27 @@
   return Result;
 }
 
-TEST(RenameTest, SingleFile) {
-  // "^" points to the position of the rename, and "[[]]" ranges point to the
+TEST(RenameTest, WithinFileRename) {
+  // rename is runnning on all "^" points, and "[[]]" ranges point to the
   // identifier that is being renamed.
   llvm::StringRef Tests[] = {
-  // Rename function.
+  // Function.
   R"cpp(
-void [[foo]]() {
+void [[foo^]]() {
   [[fo^o]]();
 }
   )cpp",
 
-  // Rename type.
+  // Type.
   R"cpp(
-struct [[foo]]{};
+struct [[foo^]] {};
 [[foo]] test() {
[[f^oo]] x;
return x;
 }
   )cpp",
 
-  // Rename variable.
+  // Local variable.
   R"cpp(
 void bar() {
   if (auto [[^foo]] = 5) {
@@ -66,18 +66,313 @@
   }
 }
   )cpp",
+
+  // Rename class, including constructor/destructor.
+  R"cpp(
+class [[F^oo]] {
+  [[F^oo]]();
+  ~[[Foo]]();
+  void foo(int x);
+};
+[[Foo]]::[[Fo^o]]() {}
+void [[Foo]]::foo(int x) {}
+  )cpp",
+
+  // Class in template argument.
+  R"cpp(
+class [[F^oo]] {};
+template  void func();
+template  class Baz {};
+int main() {
+  func<[[F^oo]]>();
+  Baz<[[F^oo]]> obj;
+  return 0;
+}
+  )cpp",
+
+  // Forward class declaration without definition.
+  R"cpp(
+class [[F^oo]];
+[[Foo]] *f();
+  )cpp",
+
+  // Class methods overrides.
+  R"cpp(
+struct A {
+ virtual void [[f^oo]]() {}
+};
+struct B : A {
+  void [[f^oo]]() override {}
+};
+struct C : B {
+  void [[f^oo]]() override {}
+};
+
+void func() {
+  A().[[f^oo]]();
+  B().[[f^oo]]();
+  C().[[f^oo]]();
+}
+  )cpp",
+
+  // Template class (partial) specializations.
+  R"cpp(
+template 
+class [[F^oo]] {};
+
+template<>
+class [[F^oo]] {};
+template 
+class [[F^oo]] {};
+
+void test() {
+  [[Foo]] x;
+  [[Foo]] y;
+  [[Foo]] z;
+}
+  )cpp",
+
+  // Template class instantiations.
+  R"cpp(
+template 
+class [[F^oo]] {
+public:
+  T foo(T arg, T& ref, T* ptr) {
+T value;
+int number = 42;
+value = (T)number;
+value = static_cast(number);
+return value;
+  }
+  static void foo(T value) {}
+  T member;
+};
+
+template 
+void func() {
+  [[F^oo]] obj;
+  obj.member = T();
+  [[Foo]]::foo();
+}
+
+void test() {
+  [[F^oo]] i;
+  i.member = 0;
+  [[F^oo]]::foo(0);
+
+  [[F^oo]] b;
+  b.member = false;
+  [[Foo]]::foo(false);
+}
+  )cpp",
+
+  // Template class methods.
+  R"cpp(
+template 
+class A {
+public:
+  void [[f^oo]]() {}
+};
+
+void func() {
+  A().[[f^oo]]();
+  A().[[f^oo]]();
+  A().[[f^oo]]();
+}
+  )cpp",
+
+  // Complicated class type.
+  R"cpp(
+ // Forward declaration.
+class [[Fo^o]];
+class Baz {
+  virtual int getValue() const = 0;
+};
+
+class [[F^oo]] : public Baz  {
+public:
+  [[Foo]](int value = 0) : x(value) {}
+
+  [[Foo]] &operator++(int);
+
+  bool operator<([[Foo]] const &rhs);
+  int getValue() const;
+private:
+  int x;
+};
+
+void func() {
+  [[Foo]] *Pointer = 0;
+  [[Foo]] Variable = [[Foo]](10);
+  for ([[Foo]] it; it < Variable; it++);
+  const [[Foo]] *C = new [[Foo]]();
+  const_cast<[[Foo]] *>(C)->getValue();
+  [[Foo]] foo;
+  const Baz &BazReference = foo;
+  const Baz *BazPointer = &foo;
+  dynamic_cast(BazReference).getValue();
+  dynamic_cast(BazPointer)->getValue();
+  reinterpret_cast(BazPointer)->getValue();
+  static_cast(BazReference).getValue();
+  static_cast(BazPointer)->getValue

[PATCH] D69934: [clangd] Implement rename by using SelectionTree and findExplicitReferences.

2019-11-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:371
 auto TU = TestTU::withCode(Code.code());
+TU.AdditionalFiles["foo.inc"] = R"cpp(
+  #define Macro(X) X

ilya-biryukov wrote:
> Maybe test this separately? It's only used in one test.
> Having this added for all tests cases is somewhat confusing.
moved to a separate test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69934



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


[clang-tools-extra] f21b2d8 - [clangd] Fix diagnostic warnings in the RenameTests, NFC.

2019-11-18 Thread Haojian Wu via cfe-commits

Author: Haojian Wu
Date: 2019-11-18T15:03:11+01:00
New Revision: f21b2d8e42f87ad6958599a385ed7bbc4df86de6

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

LOG: [clangd] Fix diagnostic warnings in the RenameTests, NFC.

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp 
b/clang-tools-extra/clangd/unittests/RenameTests.cpp
index c4ee649e5001..212d9c089f92 100644
--- a/clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -128,7 +128,7 @@ TEST(RenameTest, Renameable) {
"not eligible for indexing", HeaderFile, Index},
 
   {R"cpp(// disallow -- namespace symbol isn't supported
-namespace fo^o {}
+namespace n^s {}
   )cpp",
"not a supported kind", HeaderFile, Index},
 
@@ -142,7 +142,7 @@ TEST(RenameTest, Renameable) {
   {
 
   R"cpp(
-struct X { X operator++(int) {} };
+struct X { X operator++(int); };
 void f(X x) {x+^+;})cpp",
   "not a supported kind", HeaderFile, Index},
 
@@ -167,6 +167,8 @@ TEST(RenameTest, Renameable) {
   TU.ExtraArgs.push_back("-xobjective-c++-header");
 }
 auto AST = TU.build();
+EXPECT_TRUE(AST.getDiagnostics().empty())
+  << AST.getDiagnostics().front() << T.code();
 llvm::StringRef NewName = "dummyNewName";
 auto Results = renameWithinFile(AST, testPath(TU.Filename), T.point(),
 NewName, Case.Index);



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


[clang] 8c8c941 - Remove useless param tag to fix Wdocumentation warning. NFCI.

2019-11-18 Thread Simon Pilgrim via cfe-commits

Author: Simon Pilgrim
Date: 2019-11-18T14:09:11Z
New Revision: 8c8c941844080625fd2989bd4045cdd5db4bb908

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

LOG: Remove useless param tag to fix Wdocumentation warning. NFCI.

Added: 


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

Removed: 




diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
index a3a8c6988819..a0c1900f7ed9 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -23,9 +23,6 @@ namespace dependencies{
 class DependencyScanningTool {
 public:
   /// Construct a dependency scanning tool.
-  ///
-  /// \param Compilations The reference to the compilation database that's
-  /// used by the clang tool.
   DependencyScanningTool(DependencyScanningService &Service);
 
   /// Print out the dependency information into a string using the dependency



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


[clang-tools-extra] b622ff3 - [clangd] Fix some clang-tidy warnings on SourceCodeTests.cpp, NFC.

2019-11-18 Thread Haojian Wu via cfe-commits

Author: Haojian Wu
Date: 2019-11-18T15:20:42+01:00
New Revision: b622ff39c0c482494a7400ac0256b543025cd449

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

LOG: [clangd] Fix some clang-tidy warnings on SourceCodeTests.cpp, NFC.

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp 
b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
index adc09a4f311d..0dabce2a3d64 100644
--- a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -27,7 +27,6 @@ namespace {
 
 using llvm::Failed;
 using llvm::HasValue;
-using ::testing::UnorderedElementsAreArray;
 
 MATCHER_P2(Pos, Line, Col, "") {
   return arg.line == int(Line) && arg.character == int(Col);
@@ -36,18 +35,18 @@ MATCHER_P2(Pos, Line, Col, "") {
 MATCHER_P(MacroName, Name, "") { return arg.Name == Name; }
 
 /// A helper to make tests easier to read.
-Position position(int line, int character) {
+Position position(int Line, int Character) {
   Position Pos;
-  Pos.line = line;
-  Pos.character = character;
+  Pos.line = Line;
+  Pos.character = Character;
   return Pos;
 }
 
-Range range(const std::pair p1, const std::pair p2) {
-  Range range;
-  range.start = position(p1.first, p1.second);
-  range.end = position(p2.first, p2.second);
-  return range;
+Range range(const std::pair &P1, const std::pair &P2) {
+  Range Range;
+  Range.start = position(P1.first, P1.second);
+  Range.end = position(P2.first, P2.second);
+  return Range;
 }
 
 TEST(SourceCodeTests, lspLength) {



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


[PATCH] D69934: [clangd] Implement rename by using SelectionTree and findExplicitReferences.

2019-11-18 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/D69934/new/

https://reviews.llvm.org/D69934



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


[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

2019-11-18 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst:47
+std::vector&& obj = ...;
+return std::move(obj);  // calls StatusOr::StatusOr(std::vector&&)
+  }

While checking this example it seems clang already has a warning for that case?

https://godbolt.org/z/q5zzh-

What parts of this check will be more then the warnings already do?



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst:55
+
+## Semantics
+

Does this syntax work in rst files?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70390



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


[PATCH] D67536: [clangd] Inactive regions support as an extension to semantic highlighting

2019-11-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment.

thanks, mostly good, a few more nits.




Comment at: clang-tools-extra/clangd/ParsedAST.h:134
+  // Ranges skipped during preprocessing.
+  std::vector SkippedRanges;
 };

nit: it is not used anymore, remove it.



Comment at: clang-tools-extra/clangd/Protocol.h:1212
   std::string Tokens;
+  /// Is the line in an inactive preprocessor branch?
+  bool IsInactive = false;

hokein wrote:
> could you add some documentation describing this is a clangd extension?
could you document that the line-style highlightings can be combined with any 
token-style highlightings?



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:469
+  auto &AddedLine = DiffedLines.back();
+  for (auto Iter = AddedLine.Tokens.begin();
+   Iter != AddedLine.Tokens.end();) {

nridge wrote:
> hokein wrote:
> > it took me a while to understand this code,
> > 
> > If the NewLine is `IsInactive`, it just contains a single token whose range 
> > is [0, 0), can't we just?
> > 
> > ```
> > 
> > if (NewLine.back().Tokens.empty()) continue;
> > 
> > bool InactiveLine = NewLine.back().Tokens.front().Kind == InactiveCode;
> > assert(InactiveLine && NewLine.back().Tokens.size() == 1 && "IncativeCode 
> > must have a single token");
> > DiffedLines.back().IsInactive = true;
> > ```
> An inactive line can contain token highlightings as well. For example, we 
> highlight macro references in the condition of an `#ifdef`, and that line is 
> also inactive if the condition is false. Clients can merge the line style 
> (which is typically a background color) with the token styles (typically a 
> foreground color).
> 
> I did expand the comment to explain what the loop is doing more clearly.
thanks, I see. 

I think we can still simplify the code like below, this could improve the code 
readability, and avoid the comment explaining it.

```
llvm::erase_if(AddedLine.Tokens, [](const Token& T) { return T.Kind == 
InactiveCode;});
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67536



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


[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

2019-11-18 Thread Clement Courbet via Phabricator via cfe-commits
courbet marked 3 inline comments as done.
courbet added a comment.

Thanks for the comments




Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst:47
+std::vector&& obj = ...;
+return std::move(obj);  // calls StatusOr::StatusOr(std::vector&&)
+  }

JonasToth wrote:
> While checking this example it seems clang already has a warning for that 
> case?
> 
> https://godbolt.org/z/q5zzh-
> 
> What parts of this check will be more then the warnings already do?
I was not aware of that, thanks for pointing that out. I don't think the check 
does more than the warning in that case. TBH I have not seen instances of this 
while running the check on our codebase (I'm only looking at a sample of the 
mistakes though, there are too many hits to look at all of them). All mistakes 
I have seen are of the `const` kind. 

The value we get from having this in the form of a check is more control over 
which types are allowed through the clang-tidy options.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst:55
+
+## Semantics
+

JonasToth wrote:
> Does this syntax work in rst files?
Nope, this was migrated from our internal markdown syntax, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70390



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


[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

2019-11-18 Thread Clement Courbet via Phabricator via cfe-commits
courbet updated this revision to Diff 229833.
courbet marked an inline comment as done.
courbet added a comment.

Fix markdown in doc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70390

Files:
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp
  clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.h
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst
  clang-tools-extra/test/clang-tidy/checkers/performance-no-automatic-move.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-no-automatic-move.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-no-automatic-move.cpp
@@ -0,0 +1,165 @@
+// RUN: %check_clang_tidy -std=c++11,c++14,c++17 %s performance-no-automatic-move %t
+
+namespace std {
+template 
+struct remove_reference;
+
+template 
+struct remove_reference {
+  typedef _Tp type;
+};
+
+template 
+struct remove_reference<_Tp &> {
+  typedef _Tp type;
+};
+
+template 
+struct remove_reference<_Tp &&> {
+  typedef _Tp type;
+};
+
+template 
+constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) {
+  return static_cast::type &&>(__t);
+}
+
+template 
+constexpr _Tp &&
+forward(typename remove_reference<_Tp>::type &__t) noexcept {
+  return static_cast<_Tp &&>(__t);
+}
+
+} // namespace std
+
+struct Obj {
+  Obj();
+  Obj(const Obj &);
+  Obj(Obj &&);
+  virtual ~Obj();
+};
+
+template 
+struct StatusOr {
+  StatusOr(const T &);
+  StatusOr(T &&);
+};
+
+struct NonTemplate {
+  NonTemplate(const Obj &);
+  NonTemplate(Obj &&);
+};
+
+template 
+T Make();
+
+StatusOr PositiveStatusOrConstValue() {
+  const Obj obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+NonTemplate PositiveNonTemplateConstValue() {
+  const Obj obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+Obj PositiveSelfConstValue() {
+  const Obj obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+StatusOr PositiveStatusOrRefRef() {
+  Obj &&obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: rvalue-ness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+StatusOr PositiveNonTemplateRefRef() {
+  Obj &&obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: rvalue-ness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+StatusOr PositiveSelfRefRef() {
+  Obj &&obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: rvalue-ness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+StatusOr PositiveStatusOrConstRefRef() {
+  const Obj &&obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: rvalue-ness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+NonTemplate PositiveNonTemplateConstRefRef() {
+  const Obj &&obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: rvalue-ness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+NonTemplate PositiveSelfConstRefRef() {
+  const Obj &&obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: rvalue-ness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+// FIXME: Ideally we would warn here too.
+NonTemplate PositiveNonTemplateLifetimeExtension() {
+  const Obj &obj = Make();
+  return obj;
+}
+
+// FIXME: Ideally we would warn here too.
+StatusOr PositiveStatusOrLifetimeExtension() {
+  const Obj &obj = Make();
+  return obj;
+}
+
+// Negatives.
+
+StatusOr Temporary() {
+  return Make();
+}
+
+StatusOr ConstTemporary() {
+  return Make();
+}
+
+StatusOr Nrvo() {
+  Obj obj = Make();
+  return obj;
+}
+
+StatusOr Ref() {
+  Obj &obj = Make();
+  return obj;
+}
+
+StatusOr ConstRef() {
+  const Obj &obj = Make();
+  return obj;
+}
+
+NonTemplate RefRefMove() {
+  Obj &&obj = Make();
+  return std::move(obj);
+}
+
+const Obj global;
+
+StatusOr Global() {
+  return global;
+}
+
+struct FromConstRefOnly {
+  FromConstRefOnly(const Obj &);
+};
+
+FromConstRefOnly FromConstRefOnly() {
+  const Obj obj = Make();
+  return obj;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst
@@ -0

[clang] d27a16e - Revert "[DWARF5]Addition of alignment atrribute in typedef DIE."

2019-11-18 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2019-11-18T15:53:22+01:00
New Revision: d27a16eb392f39f9ee04ff5194b1eff3e189e6f8

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

LOG: Revert "[DWARF5]Addition of alignment atrribute in typedef DIE."

This reverts commit 423f541c1a322963cf482683fe9777ef0692082d, which
breaks llvm-c ABI.

Added: 


Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
llvm/include/llvm-c/DebugInfo.h
llvm/include/llvm/IR/DIBuilder.h
llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
llvm/lib/IR/DIBuilder.cpp
llvm/lib/IR/DebugInfo.cpp
llvm/tools/llvm-c-test/debuginfo.c

Removed: 
clang/test/CodeGenCXX/debug-info-template-align.cpp
llvm/test/DebugInfo/X86/debug-info-template-align.ll



diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index f204b9716926..75c4b2ae2339 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1127,8 +1127,7 @@ llvm::DIType *CGDebugInfo::CreateType(const 
TemplateSpecializationType *Ty,
   SourceLocation Loc = AliasDecl->getLocation();
   return DBuilder.createTypedef(Src, OS.str(), getOrCreateFile(Loc),
 getLineNumber(Loc),
-getDeclContextDescriptor(AliasDecl),
-/* Alignment */ None);
+getDeclContextDescriptor(AliasDecl));
 }
 
 llvm::DIType *CGDebugInfo::CreateType(const TypedefType *Ty,
@@ -1144,10 +1143,9 @@ llvm::DIType *CGDebugInfo::CreateType(const TypedefType 
*Ty,
   SourceLocation Loc = Ty->getDecl()->getLocation();
 
   // Typedefs are derived from some other type.
-  return DBuilder.createTypedef(
-  Underlying, Ty->getDecl()->getName(), getOrCreateFile(Loc),
-  getLineNumber(Loc), getDeclContextDescriptor(Ty->getDecl()),
-  getDeclAlignIfRequired(Ty->getDecl(), CGM.getContext()));
+  return DBuilder.createTypedef(Underlying, Ty->getDecl()->getName(),
+getOrCreateFile(Loc), getLineNumber(Loc),
+getDeclContextDescriptor(Ty->getDecl()));
 }
 
 static unsigned getDwarfCC(CallingConv CC) {
@@ -2326,8 +2324,7 @@ llvm::DIType *CGDebugInfo::CreateType(const 
ObjCTypeParamType *Ty,
   return DBuilder.createTypedef(
   getOrCreateType(Ty->getDecl()->getUnderlyingType(), Unit),
   Ty->getDecl()->getName(), getOrCreateFile(Loc), getLineNumber(Loc),
-  getDeclContextDescriptor(Ty->getDecl()),
-  /* Alignment */ None);
+  getDeclContextDescriptor(Ty->getDecl()));
 }
 
 /// \return true if Getter has the default name for the property PD.

diff  --git a/clang/test/CodeGenCXX/debug-info-template-align.cpp 
b/clang/test/CodeGenCXX/debug-info-template-align.cpp
deleted file mode 100644
index 42fdb269a30b..
--- a/clang/test/CodeGenCXX/debug-info-template-align.cpp
+++ /dev/null
@@ -1,14 +0,0 @@
-//  Test for debug info related to DW_AT_alignment attribute in the typedef 
operator
-// Supported: -O0, standalone DI
-// RUN: %clang_cc1 -dwarf-version=5  -emit-llvm -triple x86_64-linux-gnu %s -o 
- \
-// RUN:   -O0 -disable-llvm-passes \
-// RUN:   -debug-info-kind=standalone \
-// RUN: | FileCheck %s
-
-// CHECK: DIDerivedType(tag: DW_TAG_typedef, {{.*}}, align: 512
-
-typedef char __attribute__((__aligned__(64))) alchar;
-
-int main() {
-  alchar newChar;
-}

diff  --git a/llvm/include/llvm-c/DebugInfo.h b/llvm/include/llvm-c/DebugInfo.h
index 891faedb636f..41e9f96bbb92 100644
--- a/llvm/include/llvm-c/DebugInfo.h
+++ b/llvm/include/llvm-c/DebugInfo.h
@@ -875,7 +875,7 @@ LLVMMetadataRef
 LLVMDIBuilderCreateTypedef(LLVMDIBuilderRef Builder, LLVMMetadataRef Type,
const char *Name, size_t NameLen,
LLVMMetadataRef File, unsigned LineNo,
-   LLVMMetadataRef Scope, uint32_t AlignInBits);
+   LLVMMetadataRef Scope);
 
 /**
  * Create debugging information entry to establish inheritance relationship

diff  --git a/llvm/include/llvm/IR/DIBuilder.h 
b/llvm/include/llvm/IR/DIBuilder.h
index 3c738b8a4f9f..ad9a35b55414 100644
--- a/llvm/include/llvm/IR/DIBuilder.h
+++ b/llvm/include/llvm/IR/DIBuilder.h
@@ -237,10 +237,8 @@ namespace llvm {
 /// \param FileFile where this type is defined.
 /// \param LineNo  Line number.
 /// \param Context The surrounding context for the typedef.
-/// \param AlignInBits Alignment. (Optional)
 DIDerivedType *createTypedef(DIType *Ty, StringRef Name, DIFile *File,
- unsigned LineNo, DIScope *Context,
- Optional AlignInBits = {});
+ unsigned LineNo, DIScope *Context);
 
 /// Create debuggi

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-18 Thread Melanie Blower via Phabricator via cfe-commits
mibintc added a comment.

Thanks I see it, I'm working on a patch. Previously there was no support for 
frounding-math (unimplemented). This patch enables the option. In the IR 
builder, there's a call to a runtime function in the exception handler which is 
unexpectedly null.  I start by adding a null pointer check.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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


[PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

2019-11-18 Thread Dan McGregor via Phabricator via cfe-commits
dankm marked an inline comment as done.
dankm added inline comments.



Comment at: clang/test/Driver/debug-prefix-map.c:8
+// RUN: %clang -### -ffile-prefix-map=old=new %s 2>&1 | FileCheck %s 
-check-prefix CHECK-DEBUG-SIMPLE
+// RUN: %clang -### -ffile-prefix-map=old=new %s 2>&1 | FileCheck %s 
-check-prefix CHECK-MACRO-SIMPLE
+

Lekensteyn wrote:
> What about combining these two tests? The command is the same, maybe you 
> could have a new `-check-prefix` to reduce the number of invocations? 
> Likewise for the cases below.
The tests will need more thinking, you're probably right, but I don't have much 
time to work on this at the moment. How do you feel about addressing this in 
the future?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D49466



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


[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

2019-11-18 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:143
+
+  The check flags constructs that prevent automatic move of local variables.
+

Please omit //The check// and synchronize with first sentence of documentation.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst:10
+
+This check detects some usage patterns that prevent this optimization from
+happening. The most common one is local ``lvalue`` variables that are declared

Please omit //This check//,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70390



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


[PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

2019-11-18 Thread Dan McGregor via Phabricator via cfe-commits
dankm updated this revision to Diff 229835.
dankm added a comment.
Herald added subscribers: lldb-commits, mgorny.
Herald added a project: LLDB.

- Address feedback from @Lekensteyn


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D49466

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/PreprocessorOptions.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/test/CodeGen/debug-prefix-map.c
  clang/test/Driver/debug-prefix-map.S
  clang/test/Driver/debug-prefix-map.c
  clang/test/Preprocessor/file_test.c
  clang/test/Preprocessor/file_test.h
  lld/Common/CMakeLists.txt
  lldb/source/CMakeLists.txt
  llvm/include/llvm/Support/Path.h
  llvm/lib/Support/Path.cpp
  llvm/unittests/Support/Path.cpp

Index: llvm/unittests/Support/Path.cpp
===
--- llvm/unittests/Support/Path.cpp
+++ llvm/unittests/Support/Path.cpp
@@ -1230,7 +1230,9 @@
 TEST(Support, ReplacePathPrefix) {
   SmallString<64> Path1("/foo");
   SmallString<64> Path2("/old/foo");
+  SmallString<64> Path3("/oldnew/foo");
   SmallString<64> OldPrefix("/old");
+  SmallString<64> OldPrefixSep("/old/");
   SmallString<64> NewPrefix("/new");
   SmallString<64> NewPrefix2("/longernew");
   SmallString<64> EmptyPrefix("");
@@ -1250,6 +1252,33 @@
   Path = Path2;
   path::replace_path_prefix(Path, OldPrefix, EmptyPrefix);
   EXPECT_EQ(Path, "/foo");
+  Path = Path2;
+  path::replace_path_prefix(Path, OldPrefix, EmptyPrefix, true);
+  EXPECT_EQ(Path, "foo");
+  Path = Path3;
+  path::replace_path_prefix(Path, OldPrefix, NewPrefix, false);
+  EXPECT_EQ(Path, "/newnew/foo");
+  Path = Path3;
+  path::replace_path_prefix(Path, OldPrefix, NewPrefix, true);
+  EXPECT_EQ(Path, "/oldnew/foo");
+  Path = Path3;
+  path::replace_path_prefix(Path, OldPrefixSep, NewPrefix, true);
+  EXPECT_EQ(Path, "/oldnew/foo");
+  Path = Path1;
+  path::replace_path_prefix(Path, EmptyPrefix, NewPrefix);
+  EXPECT_EQ(Path, "/new/foo");
+  Path = OldPrefix;
+  path::replace_path_prefix(Path, OldPrefix, NewPrefix);
+  EXPECT_EQ(Path, "/new");
+  Path = OldPrefixSep;
+  path::replace_path_prefix(Path, OldPrefix, NewPrefix);
+  EXPECT_EQ(Path, "/new/");
+  Path = OldPrefix;
+  path::replace_path_prefix(Path, OldPrefixSep, NewPrefix, false);
+  EXPECT_EQ(Path, "/old");
+  Path = OldPrefix;
+  path::replace_path_prefix(Path, OldPrefixSep, NewPrefix, true);
+  EXPECT_EQ(Path, "/new");
 }
 
 TEST_F(FileSystemTest, OpenFileForRead) {
Index: llvm/lib/Support/Path.cpp
===
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -496,27 +496,50 @@
   path.append(ext.begin(), ext.end());
 }
 
-void replace_path_prefix(SmallVectorImpl &Path,
+bool replace_path_prefix(SmallVectorImpl &Path,
  const StringRef &OldPrefix, const StringRef &NewPrefix,
- Style style) {
+ Style style, bool strict) {
   if (OldPrefix.empty() && NewPrefix.empty())
-return;
+return false;
 
   StringRef OrigPath(Path.begin(), Path.size());
-  if (!OrigPath.startswith(OldPrefix))
-return;
+  StringRef OldPrefixDir;
+
+  if (!strict && OldPrefix.size() > OrigPath.size())
+return false;
+
+  // Ensure OldPrefixDir does not have a trailing separator.
+  if (!OldPrefix.empty() && is_separator(OldPrefix.back()))
+OldPrefixDir = parent_path(OldPrefix, style);
+  else
+OldPrefixDir = OldPrefix;
+
+  if (!OrigPath.startswith(OldPrefixDir))
+return false;
+
+  if (OrigPath.size() > OldPrefixDir.size())
+if (!is_separator(OrigPath[OldPrefixDir.size()], style) && strict)
+  return false;
 
   // If prefixes have the same size we can simply copy the new one over.
-  if (OldPrefix.size() == NewPrefix.size()) {
+  if (OldPrefixDir.size() == NewPrefix.size() && !strict) {
 llvm::copy(NewPrefix, Path.begin());
-return;
+return true;
   }
 
-  StringRef RelPath = OrigPath.substr(OldPrefix.size());
+  StringRef RelPath = OrigPath.substr(OldPrefixDir.size());
   SmallString<256> NewPath;
   path::append(NewPath, style, NewPrefix);
-  path::append(NewPath, style, RelPath);
+  if (!RelPath.empty()) {
+if (!is_separator(RelPath[0], style) || !strict)
+  path::append(NewPath, style, RelPath);
+else
+  path::append(NewPath, style, relative_path(RelPath, style));
+  }
+
   Path.swap(NewPath);
+
+  return true;
 }
 
 void native(const Twine &path, SmallVectorImpl &result, Style style) {
Index: llvm/include/llvm/Support/Path.h
===
--- llvm/include/

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

2019-11-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

I'm reverting because this breaks ABI for llvm-c (and the go bindings).
Feel free to reapply with that change removed, though. (If this option is 
required for the C API, I'm not sure what the best approach is)




Comment at: llvm/include/llvm-c/DebugInfo.h:878
LLVMMetadataRef File, unsigned LineNo,
-   LLVMMetadataRef Scope);
+   LLVMMetadataRef Scope, uint32_t AlignInBits);
 

These functions in llvm-c are ABI-stable AFAIK.
(This broke the go bindings which are in-tree, but could equally break 
out-of-tree bindings)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111



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


[PATCH] D70008: [clangd] Store xref for Macros in ParsedAST.

2019-11-18 Thread UTKARSH SAXENA via Phabricator via cfe-commits
usaxena95 updated this revision to Diff 229836.
usaxena95 marked 8 inline comments as done.
usaxena95 added a comment.

Addressed comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70008

Files:
  clang-tools-extra/clangd/CollectMacros.h
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
  clang-tools-extra/clangd/unittests/ParsedASTTests.cpp

Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -260,6 +260,15 @@
   // Includes macro expansions in arguments that are expressions
   ^assert(0 <= ^BAR);
 }
+
+#ifdef ^UNDEFINED
+#endif
+
+#define ^MULTIPLE_DEFINITION 1
+#undef ^MULTIPLE_DEFINITION
+
+#define ^MULTIPLE_DEFINITION 2
+#undef ^MULTIPLE_DEFINITION
   )cpp");
   auto TU = TestTU::withCode(TestCase.code());
   TU.HeaderCode = R"cpp(
@@ -274,7 +283,11 @@
   )cpp";
   ParsedAST AST = TU.build();
   std::vector MacroExpansionPositions;
-  for (const auto &R : AST.getMacros().Ranges)
+  for (const auto &SIDToRefs : AST.getMacros().MacroRefs) {
+for (const auto &R : SIDToRefs.second)
+  MacroExpansionPositions.push_back(R.start);
+  }
+  for (const auto &R : AST.getMacros().UnknownMacros)
 MacroExpansionPositions.push_back(R.start);
   EXPECT_THAT(MacroExpansionPositions,
   testing::UnorderedElementsAreArray(TestCase.points()));
Index: clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
@@ -0,0 +1,109 @@
+//===-- CollectMacrosTests.cpp --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#include "Annotations.h"
+#include "CollectMacros.h"
+#include "Matchers.h"
+#include "SourceCode.h"
+#include "TestTU.h"
+#include "index/SymbolID.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/Support/ScopedPrinter.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+using testing::UnorderedElementsAreArray;
+
+TEST(CollectMainFileMacros, SelectedMacros) {
+  // References of the same symbol must have the ranges with the same
+  // name(integer). If there are N different symbols then they must be named
+  // from 1 to N. Macros for which SymbolID cannot be computed must be named
+  // "Unknown".
+  const char *Tests[] = {
+  R"cpp(// Macros: Cursor on definition.
+#define $1[[FOO]](x,y) (x + y)
+int main() { int x = $1[[FOO]]($1[[FOO]](3, 4), $1[[FOO]](5, 6)); }
+  )cpp",
+  R"cpp(
+#define $1[[M]](X) X;
+#define $2[[abc]] 123
+int s = $1[[M]]($2[[abc]]);
+  )cpp",
+  // FIXME: Locating macro in duplicate definitions doesn't work. Enable
+  // this once LocateMacro is fixed.
+  // R"cpp(// Multiple definitions.
+  //   #define $1[[abc]] 1
+  //   int func1() { int a = $1[[abc]];}
+  //   #undef $1[[abc]]
+
+  //   #define $2[[abc]] 2
+  //   int func2() { int a = $2[[abc]];}
+  //   #undef $2[[abc]]
+  // )cpp",
+  R"cpp(
+#ifdef $Unknown[[UNDEFINED]]
+#endif
+  )cpp",
+  R"cpp(
+#ifndef $Unknown[[abc]]
+#define $1[[abc]]
+#ifdef $1[[abc]]
+#endif
+#endif
+  )cpp",
+  R"cpp(
+// Macros from token concatenations not included.
+#define $1[[CONCAT]](X) X##A()
+#define $2[[PREPEND]](X) MACRO##X()
+#define $3[[MACROA]]() 123
+int B = $1[[CONCAT]](MACRO);
+int D = $2[[PREPEND]](A)
+  )cpp",
+  R"cpp(
+// FIXME: Macro names in a definition are not detected.
+#define $1[[MACRO_ARGS2]](X, Y) X Y
+#define $2[[FOO]] BAR
+#define $3[[BAR]] 1
+int A = $2[[FOO]];
+  )cpp"};
+  for (const char *Test : Tests) {
+Annotations T(Test);
+auto AST = TestTU::withCode(T.code()).build();
+auto ActualMacroRefs = AST.getMacros();
+auto &SM = AST.getSourceManager();
+auto &PP = AST.getPreprocessor();
+
+// Known macros.
+for (int I = 1;; I++) {
+  const auto ExpectedRefs = T.ranges(llvm::to_string(I));
+  if (ExpectedRefs.empty())
+break;
+
+  auto Loc = getBeginningOfIdentifier(ExpectedRefs.begin()->start, SM,
+

[PATCH] D70008: [clangd] Store xref for Macros in ParsedAST.

2019-11-18 Thread UTKARSH SAXENA via Phabricator via cfe-commits
usaxena95 marked an inline comment as done.
usaxena95 added inline comments.



Comment at: clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp:28
+  )cpp",
+  // FIXME: Locating macro in duplicate definitions doesn't work. Enable
+  // this once LocateMacro is fixed.

hokein wrote:
> This is interesting, by "doesn't work", you mean the function could not 
> locate the correct definitions for (the first & second `abc`)?
It is not able to locate the macro for second definition.  
Causes assertion failure at line 93:  assert(Macro);



Comment at: clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp:74
+for (int I = 1;; I++) {
+  const auto ExpectedRefs = T.ranges(llvm::to_string(I));
+  if (ExpectedRefs.empty())

hokein wrote:
> nit: const auto &
ranges returns a copy and not a reference. I would expect copy elision here and 
 & would be unnecessary. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70008



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


[PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

2019-11-18 Thread Dan McGregor via Phabricator via cfe-commits
dankm added a comment.

Whoops. There are extra changes here I didn't mean to submit :/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D49466



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


[PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

2019-11-18 Thread Dan McGregor via Phabricator via cfe-commits
dankm updated this revision to Diff 229837.
dankm added a comment.

- Address feedback from @Lekensteyn


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D49466

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/PreprocessorOptions.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/test/CodeGen/debug-prefix-map.c
  clang/test/Driver/debug-prefix-map.S
  clang/test/Driver/debug-prefix-map.c
  clang/test/Preprocessor/file_test.c
  clang/test/Preprocessor/file_test.h
  llvm/include/llvm/Support/Path.h
  llvm/lib/Support/Path.cpp
  llvm/unittests/Support/Path.cpp

Index: llvm/unittests/Support/Path.cpp
===
--- llvm/unittests/Support/Path.cpp
+++ llvm/unittests/Support/Path.cpp
@@ -1230,7 +1230,9 @@
 TEST(Support, ReplacePathPrefix) {
   SmallString<64> Path1("/foo");
   SmallString<64> Path2("/old/foo");
+  SmallString<64> Path3("/oldnew/foo");
   SmallString<64> OldPrefix("/old");
+  SmallString<64> OldPrefixSep("/old/");
   SmallString<64> NewPrefix("/new");
   SmallString<64> NewPrefix2("/longernew");
   SmallString<64> EmptyPrefix("");
@@ -1250,6 +1252,33 @@
   Path = Path2;
   path::replace_path_prefix(Path, OldPrefix, EmptyPrefix);
   EXPECT_EQ(Path, "/foo");
+  Path = Path2;
+  path::replace_path_prefix(Path, OldPrefix, EmptyPrefix, true);
+  EXPECT_EQ(Path, "foo");
+  Path = Path3;
+  path::replace_path_prefix(Path, OldPrefix, NewPrefix, false);
+  EXPECT_EQ(Path, "/newnew/foo");
+  Path = Path3;
+  path::replace_path_prefix(Path, OldPrefix, NewPrefix, true);
+  EXPECT_EQ(Path, "/oldnew/foo");
+  Path = Path3;
+  path::replace_path_prefix(Path, OldPrefixSep, NewPrefix, true);
+  EXPECT_EQ(Path, "/oldnew/foo");
+  Path = Path1;
+  path::replace_path_prefix(Path, EmptyPrefix, NewPrefix);
+  EXPECT_EQ(Path, "/new/foo");
+  Path = OldPrefix;
+  path::replace_path_prefix(Path, OldPrefix, NewPrefix);
+  EXPECT_EQ(Path, "/new");
+  Path = OldPrefixSep;
+  path::replace_path_prefix(Path, OldPrefix, NewPrefix);
+  EXPECT_EQ(Path, "/new/");
+  Path = OldPrefix;
+  path::replace_path_prefix(Path, OldPrefixSep, NewPrefix, false);
+  EXPECT_EQ(Path, "/old");
+  Path = OldPrefix;
+  path::replace_path_prefix(Path, OldPrefixSep, NewPrefix, true);
+  EXPECT_EQ(Path, "/new");
 }
 
 TEST_F(FileSystemTest, OpenFileForRead) {
Index: llvm/lib/Support/Path.cpp
===
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -496,27 +496,50 @@
   path.append(ext.begin(), ext.end());
 }
 
-void replace_path_prefix(SmallVectorImpl &Path,
+bool replace_path_prefix(SmallVectorImpl &Path,
  const StringRef &OldPrefix, const StringRef &NewPrefix,
- Style style) {
+ Style style, bool strict) {
   if (OldPrefix.empty() && NewPrefix.empty())
-return;
+return false;
 
   StringRef OrigPath(Path.begin(), Path.size());
-  if (!OrigPath.startswith(OldPrefix))
-return;
+  StringRef OldPrefixDir;
+
+  if (!strict && OldPrefix.size() > OrigPath.size())
+return false;
+
+  // Ensure OldPrefixDir does not have a trailing separator.
+  if (!OldPrefix.empty() && is_separator(OldPrefix.back()))
+OldPrefixDir = parent_path(OldPrefix, style);
+  else
+OldPrefixDir = OldPrefix;
+
+  if (!OrigPath.startswith(OldPrefixDir))
+return false;
+
+  if (OrigPath.size() > OldPrefixDir.size())
+if (!is_separator(OrigPath[OldPrefixDir.size()], style) && strict)
+  return false;
 
   // If prefixes have the same size we can simply copy the new one over.
-  if (OldPrefix.size() == NewPrefix.size()) {
+  if (OldPrefixDir.size() == NewPrefix.size() && !strict) {
 llvm::copy(NewPrefix, Path.begin());
-return;
+return true;
   }
 
-  StringRef RelPath = OrigPath.substr(OldPrefix.size());
+  StringRef RelPath = OrigPath.substr(OldPrefixDir.size());
   SmallString<256> NewPath;
   path::append(NewPath, style, NewPrefix);
-  path::append(NewPath, style, RelPath);
+  if (!RelPath.empty()) {
+if (!is_separator(RelPath[0], style) || !strict)
+  path::append(NewPath, style, RelPath);
+else
+  path::append(NewPath, style, relative_path(RelPath, style));
+  }
+
   Path.swap(NewPath);
+
+  return true;
 }
 
 void native(const Twine &path, SmallVectorImpl &result, Style style) {
Index: llvm/include/llvm/Support/Path.h
===
--- llvm/include/llvm/Support/Path.h
+++ llvm/include/llvm/Support/Path.h
@@ -152,18 +152,33 @@
 ///
 /// @code
 ///   /foo, /old, /new => /foo
+///   /

[PATCH] D69620: Add AIX assembler support

2019-11-18 Thread David Tenty via Phabricator via cfe-commits
daltenty added inline comments.



Comment at: clang/lib/Driver/ToolChains/AIX.h:26
+
+  bool hasIntegratedCPP() const override { return false; }
+

stevewan wrote:
> Xiangling_L wrote:
> > I saw a lot of other target also set `hasIntegratedCPP()` as false, but I 
> > didn't find any explanation in documentation, so I am curious that what 
> > this is about?
> I also failed to find useful resources that explains the purpose of this 
> function. The main rationales of adding it were essentially that,
> 1. It's a pure virtual function in the base `Tool` class,
> 2. Most if not all of other targets had overridden this function such that it 
> returns false.
> 
> I'll leave this comment open, and see if someone could enlighten us. 
Only "Compiler" tools set hasIntegratedCPP() to true. Looking into it, it seems 
combineWithPreprocessor() uses this to decide whether the tool supports 
preprocessor actions so it may fold them in to one step.  I think we are safe 
leaving this as is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69620



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


[PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

2019-11-18 Thread Dan McGregor via Phabricator via cfe-commits
dankm added a comment.

In D49466#1681534 , @phosek wrote:

> @dankm is it OK if we take over this change to push it forward?


At this point sure. Unless it's accepted as-is now, then I don't have a commit 
bit to finish it off.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D49466



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


[PATCH] D69979: clang: Guess at some platform FTZ/DAZ default settings

2019-11-18 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment.

In D69979#1749198 , @arsenm wrote:

> I just posted the test I wrote here: https://github.com/arsenm/subnormal_test


Thanks. I tried compiling with gcc (can't trust clang since it doesn't honor 
#pragma STDC FENV_ACCESS ON?). 
And running that on a Ubuntu 17.10 x86-64 system, it's behaving as I would 
expect. If you compile without -ffast-math,  it asserts:

  With denormals disabled
  a.out: subnormal_test.cpp:33: void fp32_denorm_test(): Assertion 
`std::fpclassify(subnormal) == FP_SUBNORMAL' failed.

And if you compile with -ffast-math, it asserts:

  In default FP mode
  a.out: subnormal_test.cpp:33: void fp32_denorm_test(): Assertion 
`std::fpclassify(subnormal) == FP_SUBNORMAL' failed.

This is what I see compiling Craig's csr tester:

  $ cc -O2 csr.c && ./a.out
  1f80
  $ cc -O2 csr.c -ffast-math && ./a.out
  9fc0

FZ is bit 15 (0x8000) and DAZ is bit 6 (0x0040), so they are clear in default 
(IEEE) mode and set with -ffast-math.


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

https://reviews.llvm.org/D69979



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


[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

2019-11-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst:47
+std::vector&& obj = ...;
+return std::move(obj);  // calls StatusOr::StatusOr(std::vector&&)
+  }

courbet wrote:
> JonasToth wrote:
> > While checking this example it seems clang already has a warning for that 
> > case?
> > 
> > https://godbolt.org/z/q5zzh-
> > 
> > What parts of this check will be more then the warnings already do?
> I was not aware of that, thanks for pointing that out. I don't think the 
> check does more than the warning in that case. TBH I have not seen instances 
> of this while running the check on our codebase (I'm only looking at a sample 
> of the mistakes though, there are too many hits to look at all of them). All 
> mistakes I have seen are of the `const` kind. 
> 
> The value we get from having this in the form of a check is more control over 
> which types are allowed through the clang-tidy options.
The `const std::vector f` isn't diagnosed by the existing diag: 
https://godbolt.org/z/ZTQ3H6


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70390



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


[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

2019-11-18 Thread Clement Courbet via Phabricator via cfe-commits
courbet marked 2 inline comments as done.
courbet added a comment.

Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70390



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


[PATCH] D70008: [clangd] Store xref for Macros in ParsedAST.

2019-11-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

lgtm, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70008



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


[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

2019-11-18 Thread Clement Courbet via Phabricator via cfe-commits
courbet updated this revision to Diff 229840.
courbet added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70390

Files:
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp
  clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.h
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst
  clang-tools-extra/test/clang-tidy/checkers/performance-no-automatic-move.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-no-automatic-move.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-no-automatic-move.cpp
@@ -0,0 +1,165 @@
+// RUN: %check_clang_tidy -std=c++11,c++14,c++17 %s performance-no-automatic-move %t
+
+namespace std {
+template 
+struct remove_reference;
+
+template 
+struct remove_reference {
+  typedef _Tp type;
+};
+
+template 
+struct remove_reference<_Tp &> {
+  typedef _Tp type;
+};
+
+template 
+struct remove_reference<_Tp &&> {
+  typedef _Tp type;
+};
+
+template 
+constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) {
+  return static_cast::type &&>(__t);
+}
+
+template 
+constexpr _Tp &&
+forward(typename remove_reference<_Tp>::type &__t) noexcept {
+  return static_cast<_Tp &&>(__t);
+}
+
+} // namespace std
+
+struct Obj {
+  Obj();
+  Obj(const Obj &);
+  Obj(Obj &&);
+  virtual ~Obj();
+};
+
+template 
+struct StatusOr {
+  StatusOr(const T &);
+  StatusOr(T &&);
+};
+
+struct NonTemplate {
+  NonTemplate(const Obj &);
+  NonTemplate(Obj &&);
+};
+
+template 
+T Make();
+
+StatusOr PositiveStatusOrConstValue() {
+  const Obj obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+NonTemplate PositiveNonTemplateConstValue() {
+  const Obj obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+Obj PositiveSelfConstValue() {
+  const Obj obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+StatusOr PositiveStatusOrRefRef() {
+  Obj &&obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: rvalue-ness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+StatusOr PositiveNonTemplateRefRef() {
+  Obj &&obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: rvalue-ness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+StatusOr PositiveSelfRefRef() {
+  Obj &&obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: rvalue-ness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+StatusOr PositiveStatusOrConstRefRef() {
+  const Obj &&obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: rvalue-ness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+NonTemplate PositiveNonTemplateConstRefRef() {
+  const Obj &&obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: rvalue-ness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+NonTemplate PositiveSelfConstRefRef() {
+  const Obj &&obj = Make();
+  return obj;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: rvalue-ness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+// FIXME: Ideally we would warn here too.
+NonTemplate PositiveNonTemplateLifetimeExtension() {
+  const Obj &obj = Make();
+  return obj;
+}
+
+// FIXME: Ideally we would warn here too.
+StatusOr PositiveStatusOrLifetimeExtension() {
+  const Obj &obj = Make();
+  return obj;
+}
+
+// Negatives.
+
+StatusOr Temporary() {
+  return Make();
+}
+
+StatusOr ConstTemporary() {
+  return Make();
+}
+
+StatusOr Nrvo() {
+  Obj obj = Make();
+  return obj;
+}
+
+StatusOr Ref() {
+  Obj &obj = Make();
+  return obj;
+}
+
+StatusOr ConstRef() {
+  const Obj &obj = Make();
+  return obj;
+}
+
+NonTemplate RefRefMove() {
+  Obj &&obj = Make();
+  return std::move(obj);
+}
+
+const Obj global;
+
+StatusOr Global() {
+  return global;
+}
+
+struct FromConstRefOnly {
+  FromConstRefOnly(const Obj &);
+};
+
+FromConstRefOnly FromConstRefOnly() {
+  const Obj obj = Make();
+  return obj;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst
@@ -0,0 +1,61 @@
+.. title:: clang-tidy - performa

[clang-tools-extra] 4f80fc2 - [clangd] Implement rename by using SelectionTree and findExplicitReferences.

2019-11-18 Thread Haojian Wu via cfe-commits

Author: Haojian Wu
Date: 2019-11-18T16:16:47+01:00
New Revision: 4f80fc2491cc35730a9a84b86975278b7daa8522

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

LOG: [clangd] Implement rename by using SelectionTree and 
findExplicitReferences.

Summary:
With the new implemenation, we will have better coverage of various AST
nodes, and fix some known/potential bugs.

Also added the existing clang-renamae tests. Known changed behavior:
 - "~Fo^o()" will not trigger the rename, will fix afterwards
 - references in macro bodies are not renamed now

This fixes:
- https://github.com/clangd/clangd/issues/167
- https://github.com/clangd/clangd/issues/169
- https://github.com/clangd/clangd/issues/171

Reviewers: ilya-biryukov

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

Tags: #clang

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

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 3969f3e2e4e2..fb83083384f9 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -8,14 +8,16 @@
 
 #include "refactor/Rename.h"
 #include "AST.h"
+#include "FindTarget.h"
 #include "Logger.h"
 #include "ParsedAST.h"
+#include "Selection.h"
 #include "SourceCode.h"
 #include "index/SymbolCollector.h"
-#include "clang/Tooling/Refactoring/Rename/RenamingAction.h"
-#include "clang/Tooling/Refactoring/Rename/USRFinder.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Tooling/Refactoring/Rename/USRFindingAction.h"
-#include "clang/Tooling/Refactoring/Rename/USRLocFinder.h"
 
 namespace clang {
 namespace clangd {
@@ -34,6 +36,17 @@ llvm::Optional filePath(const SymbolLocation 
&Loc,
   return *Path;
 }
 
+// Returns true if the given location is expanded from any macro body.
+bool isInMacroBody(const SourceManager &SM, SourceLocation Loc) {
+  while (Loc.isMacroID()) {
+if (SM.isMacroBodyExpansion(Loc))
+  return true;
+Loc = SM.getImmediateMacroCallerLoc(Loc);
+  }
+
+  return false;
+}
+
 // Query the index to find some other files where the Decl is referenced.
 llvm::Optional getOtherRefFile(const Decl &D, StringRef MainFile,
 const SymbolIndex &Index) {
@@ -56,12 +69,41 @@ llvm::Optional getOtherRefFile(const Decl &D, 
StringRef MainFile,
   return OtherFile;
 }
 
+llvm::DenseSet locateDeclAt(ParsedAST &AST,
+  SourceLocation TokenStartLoc) {
+  unsigned Offset =
+  AST.getSourceManager().getDecomposedSpellingLoc(TokenStartLoc).second;
+
+  SelectionTree Selection(AST.getASTContext(), AST.getTokens(), Offset);
+  const SelectionTree::Node *SelectedNode = Selection.commonAncestor();
+  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);
+  return Result;
+}
+
 enum ReasonToReject {
   NoSymbolFound,
   NoIndexProvided,
   NonIndexable,
   UsedOutsideFile,
   UnsupportedSymbol,
+  AmbiguousSymbol,
 };
 
 // Check the symbol Decl is renameable (per the index) within the file.
@@ -125,6 +167,8 @@ llvm::Error makeError(ReasonToReject Reason) {
   return "symbol may be used in other files (not eligible for indexing)";
 case UnsupportedSymbol:
   return "symbol is not a supported kind (e.g. namespace, macro)";
+case AmbiguousSymbol:
+  return "there are multiple symbols at the given location";
 }
 llvm_unreachable("unhandled reason kind");
   };
@@ -134,22 +178,38 @@ llvm::Error makeError(ReasonToReject Reason) {
 }
 
 // Return all rename occurrences in the main file.
-tooling::SymbolOccurrences
-findOccurrencesWithinFile(ParsedAST &AST, const NamedDecl *RenameDecl) {
-  const NamedDecl *CanonicalRenameDecl =
-  tooling::getCanonicalSymbolDeclaration(RenameDecl);
-  assert(CanonicalRenameDecl && "RenameDecl must be not null");
-  std::vector RenameUSRs =
-  tooling::getUSRsForDeclara

[PATCH] D70245: [OPENMP50]Add device/kind context selector support.

2019-11-18 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev updated this revision to Diff 229843.
ABataev added a comment.

Address comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70245

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/OpenMPKinds.def
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/OpenMP/declare_variant_ast_print.c
  clang/test/OpenMP/declare_variant_ast_print.cpp
  clang/test/OpenMP/declare_variant_device_kind_codegen.cpp
  clang/test/OpenMP/declare_variant_messages.c
  clang/test/OpenMP/declare_variant_messages.cpp
  clang/test/OpenMP/declare_variant_mixed_codegen.cpp
  clang/test/OpenMP/nvptx_declare_variant_device_kind_codegen.cpp

Index: clang/test/OpenMP/nvptx_declare_variant_device_kind_codegen.cpp
===
--- /dev/null
+++ clang/test/OpenMP/nvptx_declare_variant_device_kind_codegen.cpp
@@ -0,0 +1,170 @@
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc -fopenmp-version=50 -DGPU
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -aux-triple powerpc64le-unknown-unknown -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - -fopenmp-version=50 -DGPU | FileCheck %s --implicit-check-not='ret i32 {{1|81|84}}'
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -aux-triple powerpc64le-unknown-unknown -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -emit-pch -o %t -fopenmp-version=50 -DGPU
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -aux-triple powerpc64le-unknown-unknown -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -include-pch %t -o - -fopenmp-version=50 -DGPU | FileCheck %s --implicit-check-not='ret i32 {{1|81|84}}'
+
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc -fopenmp-version=50 -DNOHOST
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -aux-triple powerpc64le-unknown-unknown -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - -fopenmp-version=50 -DNOHOST | FileCheck %s --implicit-check-not='ret i32 {{1|81|84}}'
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -aux-triple powerpc64le-unknown-unknown -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -emit-pch -o %t -fopenmp-version=50 -DNOHOST
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -aux-triple powerpc64le-unknown-unknown -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -include-pch %t -o - -fopenmp-version=50 -DNOHOST | FileCheck %s --implicit-check-not='ret i32 {{1|81|84}}'
+// expected-no-diagnostics
+
+// CHECK-NOT: ret i32 {{1|81|84}}
+// CHECK-DAG: define {{.*}}i32 @_Z3barv()
+// CHECK-DAG: define {{.*}}i32 @_ZN16SpecSpecialFuncs6MethodEv(%struct.SpecSpecialFuncs* %{{.+}})
+// CHECK-DAG: define {{.*}}i32 @_ZN12SpecialFuncs6MethodEv(%struct.SpecialFuncs* %{{.+}})
+// CHECK-DAG: define linkonce_odr {{.*}}i32 @_ZN16SpecSpecialFuncs6methodEv(%struct.SpecSpecialFuncs* %{{.+}})
+// CHECK-DAG: define linkonce_odr {{.*}}i32 @_ZN12SpecialFuncs6methodEv(%struct.SpecialFuncs* %{{.+}})
+// CHECK-DAG: define {{.*}}i32 @_Z5prio_v()
+// CHECK-DAG: define internal i32 @_ZL6prio1_v()
+// CHECK-DAG: define {{.*}}i32 @_Z4callv()
+// CHECK-DAG: define internal i32 @_ZL9stat_usedv()
+// CHECK-DAG: define {{.*}}i32 @fn_linkage()
+// CHECK-DAG: define {{.*}}i32 @_Z11fn_linkage1v()
+
+// CHECK-DAG: ret i32 2
+// CHECK-DAG: ret i32 3
+// CHECK-DAG: ret i32 4
+// CHECK-DAG: ret i32 5
+// CHECK-DAG: ret i32 6
+// CHECK-DAG: ret i32 7
+// CHECK-DAG: ret i32 82
+// CHECK-DAG: ret i32 83
+// CHECK-DAG: ret i32 85
+// CHECK-DAG: ret i32 86
+// CHECK-DAG: ret i32 87
+
+// Outputs for function members
+// CHECK-DAG: ret i32 6
+// CHECK-DAG: ret i32 7
+// CHECK-NOT: ret i32 {{1|81|84}}
+
+#ifndef HEADER
+#define HEADER
+
+#ifdef GPU
+#define CORRECT gpu
+#define SUBSET nohost, gpu
+#define WRONG cpu, gpu
+#endif // GPU
+#ifdef NOHOST
+#define CORRECT nohost
+#define SUBSET nohost, gpu
+#define WRONG nohost, host
+#endif // NOHOST
+
+int foo() { return 2; }
+int bazzz();
+int test();
+static int stat_unused_();
+static int stat_used_();
+
+#pragma omp declare target
+
+#pragma omp declare variant(foo) match(device = {kind(CORRECT)})
+int bar() { return 1; }
+
+#pragma omp declare variant(bazzz) match(device = {kind(CORRECT)})
+int baz() { return 1; }
+
+#pragma omp declare variant(test) match(device = {kind(CORRECT)})
+int call() { return 1; }

[PATCH] D69934: [clangd] Implement rename by using SelectionTree and findExplicitReferences.

2019-11-18 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4f80fc2491cc: [clangd] Implement rename by using 
SelectionTree and findExplicitReferences. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69934

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
@@ -38,27 +38,27 @@
   return Result;
 }
 
-TEST(RenameTest, SingleFile) {
-  // "^" points to the position of the rename, and "[[]]" ranges point to the
+TEST(RenameTest, WithinFileRename) {
+  // rename is runnning on all "^" points, and "[[]]" ranges point to the
   // identifier that is being renamed.
   llvm::StringRef Tests[] = {
-  // Rename function.
+  // Function.
   R"cpp(
-void [[foo]]() {
+void [[foo^]]() {
   [[fo^o]]();
 }
   )cpp",
 
-  // Rename type.
+  // Type.
   R"cpp(
-struct [[foo]]{};
+struct [[foo^]] {};
 [[foo]] test() {
[[f^oo]] x;
return x;
 }
   )cpp",
 
-  // Rename variable.
+  // Local variable.
   R"cpp(
 void bar() {
   if (auto [[^foo]] = 5) {
@@ -66,18 +66,313 @@
   }
 }
   )cpp",
+
+  // Rename class, including constructor/destructor.
+  R"cpp(
+class [[F^oo]] {
+  [[F^oo]]();
+  ~[[Foo]]();
+  void foo(int x);
+};
+[[Foo]]::[[Fo^o]]() {}
+void [[Foo]]::foo(int x) {}
+  )cpp",
+
+  // Class in template argument.
+  R"cpp(
+class [[F^oo]] {};
+template  void func();
+template  class Baz {};
+int main() {
+  func<[[F^oo]]>();
+  Baz<[[F^oo]]> obj;
+  return 0;
+}
+  )cpp",
+
+  // Forward class declaration without definition.
+  R"cpp(
+class [[F^oo]];
+[[Foo]] *f();
+  )cpp",
+
+  // Class methods overrides.
+  R"cpp(
+struct A {
+ virtual void [[f^oo]]() {}
+};
+struct B : A {
+  void [[f^oo]]() override {}
+};
+struct C : B {
+  void [[f^oo]]() override {}
+};
+
+void func() {
+  A().[[f^oo]]();
+  B().[[f^oo]]();
+  C().[[f^oo]]();
+}
+  )cpp",
+
+  // Template class (partial) specializations.
+  R"cpp(
+template 
+class [[F^oo]] {};
+
+template<>
+class [[F^oo]] {};
+template 
+class [[F^oo]] {};
+
+void test() {
+  [[Foo]] x;
+  [[Foo]] y;
+  [[Foo]] z;
+}
+  )cpp",
+
+  // Template class instantiations.
+  R"cpp(
+template 
+class [[F^oo]] {
+public:
+  T foo(T arg, T& ref, T* ptr) {
+T value;
+int number = 42;
+value = (T)number;
+value = static_cast(number);
+return value;
+  }
+  static void foo(T value) {}
+  T member;
+};
+
+template 
+void func() {
+  [[F^oo]] obj;
+  obj.member = T();
+  [[Foo]]::foo();
+}
+
+void test() {
+  [[F^oo]] i;
+  i.member = 0;
+  [[F^oo]]::foo(0);
+
+  [[F^oo]] b;
+  b.member = false;
+  [[Foo]]::foo(false);
+}
+  )cpp",
+
+  // Template class methods.
+  R"cpp(
+template 
+class A {
+public:
+  void [[f^oo]]() {}
+};
+
+void func() {
+  A().[[f^oo]]();
+  A().[[f^oo]]();
+  A().[[f^oo]]();
+}
+  )cpp",
+
+  // Complicated class type.
+  R"cpp(
+ // Forward declaration.
+class [[Fo^o]];
+class Baz {
+  virtual int getValue() const = 0;
+};
+
+class [[F^oo]] : public Baz  {
+public:
+  [[Foo]](int value = 0) : x(value) {}
+
+  [[Foo]] &operator++(int);
+
+  bool operator<([[Foo]] const &rhs);
+  int getValue() const;
+private:
+  int x;
+};
+
+void func() {
+  [[Foo]] *Pointer = 0;
+  [[Foo]] Variable = [[Foo]](10);
+  for ([[Foo]] it; it < Variable; it++);
+  const [[Foo]] *C = new [[Foo]]();
+  const_cast<[[Foo]] *>(C)->getValue();
+  [[Foo]] foo;
+  const Baz &BazReference = foo;
+  const Baz *BazPointer = &foo;
+  dynamic_cast(BazReference).getValue();
+  dynamic_cast(BazPointer)->getValue();
+  reinterpret_cast(BazPointer)->getValue();
+  stati

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

2019-11-18 Thread LevitatingLion via Phabricator via cfe-commits
LevitatingLion added inline comments.



Comment at: llvm/docs/LangRef.rst:1428
 can prove that the call/invoke cannot call a convergent function.
+``flatten``
+This attribute is similar to ``alwaysinline``, but applies recursively to

arsenm wrote:
> It's not obvious to me what the flatten name means. flatteninline? 
> recursive_alwaysinline? Something else?
I agree. What about always_inline_recurse or always_inline_recursively?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70366



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


[clang-tools-extra] 2054ed0 - [clangd] Store xref for Macros in ParsedAST.

2019-11-18 Thread Utkarsh Saxena via cfe-commits

Author: Utkarsh Saxena
Date: 2019-11-18T16:47:18+01:00
New Revision: 2054ed052f15b584e1bce57c8f765991eab2da7d

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

LOG: [clangd] Store xref for Macros in ParsedAST.

This patch adds the cross references for Macros in the MainFile.
We add references for the main file to the ParsedAST. We query the
references from it using the SymbolID.
Xref outside main file will be added to the index in a separate patch.

Added: 
clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp

Modified: 
clang-tools-extra/clangd/CollectMacros.h
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/unittests/CMakeLists.txt
clang-tools-extra/clangd/unittests/ParsedASTTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/CollectMacros.h 
b/clang-tools-extra/clangd/CollectMacros.h
index 619c9f54b58a..17e9917542bd 100644
--- a/clang-tools-extra/clangd/CollectMacros.h
+++ b/clang-tools-extra/clangd/CollectMacros.h
@@ -9,10 +9,13 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_COLLECTEDMACROS_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_COLLECTEDMACROS_H
 
+#include "AST.h"
 #include "Protocol.h"
 #include "SourceCode.h"
+#include "index/SymbolID.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Lex/PPCallbacks.h"
+#include "llvm/ADT/DenseMap.h"
 #include 
 
 namespace clang {
@@ -22,7 +25,11 @@ struct MainFileMacros {
   llvm::StringSet<> Names;
   // Instead of storing SourceLocation, we have to store the token range 
because
   // SourceManager from preamble is not available when we build the AST.
-  std::vector Ranges;
+  llvm::DenseMap> MacroRefs;
+  // Somtimes it is not possible to compute the SymbolID for the Macro, e.g. a
+  // reference to an undefined macro. Store them separately, e.g. for semantic
+  // highlighting.
+  std::vector UnknownMacros;
 };
 
 /// Collects macro references (e.g. definitions, expansions) in the main file.
@@ -80,8 +87,12 @@ class CollectMainFileMacros : public PPCallbacks {
   return;
 
 if (auto Range = getTokenRange(SM, LangOpts, Loc)) {
-  Out.Names.insert(MacroNameTok.getIdentifierInfo()->getName());
-  Out.Ranges.push_back(*Range);
+  auto Name = MacroNameTok.getIdentifierInfo()->getName();
+  Out.Names.insert(Name);
+  if (auto SID = getSymbolID(Name, MI, SM))
+Out.MacroRefs[*SID].push_back(*Range);
+  else
+Out.UnknownMacros.push_back(*Range);
 }
   }
   const SourceManager &SM;

diff  --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp 
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index 9838600584c6..4e47a83d8da0 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -311,9 +311,14 @@ std::vector 
getSemanticHighlightings(ParsedAST &AST) {
 if (auto Kind = kindForReference(R))
   Builder.addToken(R.NameLoc, *Kind);
   });
-  // Add highlightings for macro expansions.
-  for (const auto &M : AST.getMacros().Ranges)
+  // Add highlightings for macro references.
+  for (const auto &SIDToRefs : AST.getMacros().MacroRefs) {
+for (const auto &M : SIDToRefs.second)
+  Builder.addToken({HighlightingKind::Macro, M});
+  }
+  for (const auto &M : AST.getMacros().UnknownMacros)
 Builder.addToken({HighlightingKind::Macro, M});
+
   return std::move(Builder).collect();
 }
 

diff  --git a/clang-tools-extra/clangd/XRefs.cpp 
b/clang-tools-extra/clangd/XRefs.cpp
index d604379b75fc..9697b8eec19a 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -917,8 +917,7 @@ ReferencesResult findReferences(ParsedAST &AST, Position 
Pos, uint32_t Limit,
  MainFileRefs.end());
   for (const auto &Ref : MainFileRefs) {
 if (auto Range =
-getTokenRange(AST.getASTContext().getSourceManager(),
-  AST.getASTContext().getLangOpts(), Ref.Loc)) {
+getTokenRange(SM, AST.getASTContext().getLangOpts(), Ref.Loc)) {
   Location Result;
   Result.range = *Range;
   Result.uri = URIForFile::canonicalize(*MainFilePath, *MainFilePath);

diff  --git a/clang-tools-extra/clangd/unittests/CMakeLists.txt 
b/clang-tools-extra/clangd/unittests/CMakeLists.txt
index 7e298b6ad053..21c29196034f 100644
--- a/clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ b/clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -30,6 +30,7 @@ add_unittest(ClangdUnitTests ClangdTests
   ClangdTests.cpp
   CodeCompleteTests.cpp
   CodeCompletionStringsTests.cpp
+  CollectMacrosTests.cpp
   ContextTests.cpp
   DexTests.cpp
   DiagnosticsTests.cpp

diff  --git a/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp 
b/clang-tools-extra

[clang] c3eded0 - [OPENMP50]Fix PR44024: runtime assert in distribute construct.

2019-11-18 Thread Alexey Bataev via cfe-commits

Author: Alexey Bataev
Date: 2019-11-18T11:14:27-05:00
New Revision: c3eded068c64bfe614d25359927a2917ff8e4a35

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

LOG: [OPENMP50]Fix PR44024: runtime assert in distribute construct.

If the code is emitted for distribute construct, the nonmonotonic
modifier should not be added.

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/test/OpenMP/distribute_codegen.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index cd20cb83afb9..a38007001258 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -3616,7 +3616,9 @@ static int addMonoNonMonoModifier(CodeGenModule &CGM, 
OpenMPSchedType Schedule,
   if (CGM.getLangOpts().OpenMP >= 50 && Modifier == 0) {
 if (!(Schedule == OMP_sch_static_chunked || Schedule == OMP_sch_static ||
   Schedule == OMP_sch_static_balanced_chunked ||
-  Schedule == OMP_ord_static_chunked || Schedule == OMP_ord_static))
+  Schedule == OMP_ord_static_chunked || Schedule == OMP_ord_static ||
+  Schedule == OMP_dist_sch_static_chunked ||
+  Schedule == OMP_dist_sch_static))
   Modifier = OMP_sch_modifier_nonmonotonic;
   }
   return Schedule | Modifier;

diff  --git a/clang/test/OpenMP/distribute_codegen.cpp 
b/clang/test/OpenMP/distribute_codegen.cpp
index 9641b8a1359b..e3b65ca85603 100644
--- a/clang/test/OpenMP/distribute_codegen.cpp
+++ b/clang/test/OpenMP/distribute_codegen.cpp
@@ -5,6 +5,13 @@
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple i386-unknown-unknown 
-fopenmp-targets=i386-pc-linux-gnu -emit-llvm %s -o - | FileCheck %s 
--check-prefix CHECK --check-prefix CHECK-32  --check-prefix HCHECK
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple i386-unknown-unknown 
-fopenmp-targets=i386-pc-linux-gnu -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp -x c++ -triple i386-unknown-unknown 
-fopenmp-targets=i386-pc-linux-gnu -std=c++11 -include-pch %t -verify %s 
-emit-llvm -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-32 
--check-prefix HCHECK
+// Test host codegen.
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=50 -x c++ -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu 
-emit-llvm %s -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-64
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -x c++ -std=c++11 -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu 
-emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -x c++ -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu 
-std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s 
--check-prefix CHECK --check-prefix CHECK-64  --check-prefix HCHECK
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=50 -x c++ -triple 
i386-unknown-unknown -fopenmp-targets=i386-pc-linux-gnu -emit-llvm %s -o - | 
FileCheck %s --check-prefix CHECK --check-prefix CHECK-32  --check-prefix HCHECK
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -x c++ -std=c++11 -triple 
i386-unknown-unknown -fopenmp-targets=i386-pc-linux-gnu -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -x c++ -triple 
i386-unknown-unknown -fopenmp-targets=i386-pc-linux-gnu -std=c++11 -include-pch 
%t -verify %s -emit-llvm -o - | FileCheck %s --check-prefix CHECK 
--check-prefix CHECK-32 --check-prefix HCHECK
 
 // RUN: %clang_cc1 -verify -fopenmp-simd -x c++ -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu 
-emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu 
-emit-pch -o %t %s
@@ -14,6 +21,14 @@
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -triple i386-unknown-unknown 
-fopenmp-targets=i386-pc-linux-gnu -std=c++11 -include-pch %t -verify %s 
-emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // SIMD-ONLY0-NOT: {{__kmpc|__tgt}}
 
+// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-version=50 -x c++ -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu 
-emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -x c++ -std=c++11 -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu 
-emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -x c++ -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu 
-std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck 
--check-prefix SIMD-ONLY0 %s
+// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-version=50 -x c++ -trip

[clang] c85fa79 - [Attr] Fix `-ast-print` for `asm` attribute

2019-11-18 Thread Joel E. Denny via cfe-commits

Author: Joel E. Denny
Date: 2019-11-18T11:55:25-05:00
New Revision: c85fa79d3663ecb3117e178b2a79ffa721d18e32

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

LOG: [Attr] Fix `-ast-print` for `asm` attribute

Without this fix, the tests introduced here produce the following
assert fail:

```
clang: /home/jdenny/llvm/clang/include/clang/Basic/AttributeCommonInfo.h:163: 
unsigned int clang::AttributeCommonInfo::getAttributeSpellingListIndex() const: 
Assertion `(isAttributeSpellingListCalculated() || AttrName) && "Spelling 
cannot be found"' failed.
```

The bug was introduced by D67368, which caused `AsmLabelAttr`'s
spelling index to be set to `SpellingNotCalculated`.

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang/lib/Sema/SemaDecl.cpp
clang/test/AST/ast-print-attr.c

Removed: 




diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index b469217108ce..6d857e832c4b 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -7019,8 +7019,9 @@ NamedDecl *Sema::ActOnVariableDeclarator(
   }
 }
 
-NewVD->addAttr(::new (Context) AsmLabelAttr(
-Context, SE->getStrTokenLoc(0), Label, /*IsLiteralLabel=*/true));
+NewVD->addAttr(AsmLabelAttr::Create(Context, Label,
+/*IsLiteralLabel=*/true,
+SE->getStrTokenLoc(0)));
   } else if (!ExtnameUndeclaredIdentifiers.empty()) {
 llvm::DenseMap::iterator I =
   ExtnameUndeclaredIdentifiers.find(NewVD->getIdentifier());
@@ -8923,9 +8924,9 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, 
DeclContext *DC,
   if (Expr *E = (Expr*) D.getAsmLabel()) {
 // The parser guarantees this is a string.
 StringLiteral *SE = cast(E);
-NewFD->addAttr(::new (Context)
-   AsmLabelAttr(Context, SE->getStrTokenLoc(0),
-SE->getString(), /*IsLiteralLabel=*/true));
+NewFD->addAttr(AsmLabelAttr::Create(Context, SE->getString(),
+/*IsLiteralLabel=*/true,
+SE->getStrTokenLoc(0)));
   } else if (!ExtnameUndeclaredIdentifiers.empty()) {
 llvm::DenseMap::iterator I =
   ExtnameUndeclaredIdentifiers.find(NewFD->getIdentifier());

diff  --git a/clang/test/AST/ast-print-attr.c b/clang/test/AST/ast-print-attr.c
index 223e27b39790..6448437c5eab 100644
--- a/clang/test/AST/ast-print-attr.c
+++ b/clang/test/AST/ast-print-attr.c
@@ -10,3 +10,8 @@ using B = int ** __ptr32 *[3];
 // FIXME: Too many parens here!
 // CHECK: using C = int ((*))() __attribute__((cdecl));
 using C = int (*)() [[gnu::cdecl]];
+
+// CHECK: int fun_asm() asm("");
+int fun_asm() asm("");
+// CHECK: int var_asm asm("");
+int var_asm asm("");



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


[PATCH] D70349: [Attr] Fix `-ast-print` for `asm` attribute

2019-11-18 Thread Joel E. Denny via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc85fa79d3663: [Attr] Fix `-ast-print` for `asm` attribute 
(authored by jdenny).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70349

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/AST/ast-print-attr.c


Index: clang/test/AST/ast-print-attr.c
===
--- clang/test/AST/ast-print-attr.c
+++ clang/test/AST/ast-print-attr.c
@@ -10,3 +10,8 @@
 // FIXME: Too many parens here!
 // CHECK: using C = int ((*))() __attribute__((cdecl));
 using C = int (*)() [[gnu::cdecl]];
+
+// CHECK: int fun_asm() asm("");
+int fun_asm() asm("");
+// CHECK: int var_asm asm("");
+int var_asm asm("");
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -7019,8 +7019,9 @@
   }
 }
 
-NewVD->addAttr(::new (Context) AsmLabelAttr(
-Context, SE->getStrTokenLoc(0), Label, /*IsLiteralLabel=*/true));
+NewVD->addAttr(AsmLabelAttr::Create(Context, Label,
+/*IsLiteralLabel=*/true,
+SE->getStrTokenLoc(0)));
   } else if (!ExtnameUndeclaredIdentifiers.empty()) {
 llvm::DenseMap::iterator I =
   ExtnameUndeclaredIdentifiers.find(NewVD->getIdentifier());
@@ -8923,9 +8924,9 @@
   if (Expr *E = (Expr*) D.getAsmLabel()) {
 // The parser guarantees this is a string.
 StringLiteral *SE = cast(E);
-NewFD->addAttr(::new (Context)
-   AsmLabelAttr(Context, SE->getStrTokenLoc(0),
-SE->getString(), /*IsLiteralLabel=*/true));
+NewFD->addAttr(AsmLabelAttr::Create(Context, SE->getString(),
+/*IsLiteralLabel=*/true,
+SE->getStrTokenLoc(0)));
   } else if (!ExtnameUndeclaredIdentifiers.empty()) {
 llvm::DenseMap::iterator I =
   ExtnameUndeclaredIdentifiers.find(NewFD->getIdentifier());


Index: clang/test/AST/ast-print-attr.c
===
--- clang/test/AST/ast-print-attr.c
+++ clang/test/AST/ast-print-attr.c
@@ -10,3 +10,8 @@
 // FIXME: Too many parens here!
 // CHECK: using C = int ((*))() __attribute__((cdecl));
 using C = int (*)() [[gnu::cdecl]];
+
+// CHECK: int fun_asm() asm("");
+int fun_asm() asm("");
+// CHECK: int var_asm asm("");
+int var_asm asm("");
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -7019,8 +7019,9 @@
   }
 }
 
-NewVD->addAttr(::new (Context) AsmLabelAttr(
-Context, SE->getStrTokenLoc(0), Label, /*IsLiteralLabel=*/true));
+NewVD->addAttr(AsmLabelAttr::Create(Context, Label,
+/*IsLiteralLabel=*/true,
+SE->getStrTokenLoc(0)));
   } else if (!ExtnameUndeclaredIdentifiers.empty()) {
 llvm::DenseMap::iterator I =
   ExtnameUndeclaredIdentifiers.find(NewVD->getIdentifier());
@@ -8923,9 +8924,9 @@
   if (Expr *E = (Expr*) D.getAsmLabel()) {
 // The parser guarantees this is a string.
 StringLiteral *SE = cast(E);
-NewFD->addAttr(::new (Context)
-   AsmLabelAttr(Context, SE->getStrTokenLoc(0),
-SE->getString(), /*IsLiteralLabel=*/true));
+NewFD->addAttr(AsmLabelAttr::Create(Context, SE->getString(),
+/*IsLiteralLabel=*/true,
+SE->getStrTokenLoc(0)));
   } else if (!ExtnameUndeclaredIdentifiers.empty()) {
 llvm::DenseMap::iterator I =
   ExtnameUndeclaredIdentifiers.find(NewFD->getIdentifier());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D68155: [clang][NFC] Make various uses of Regex const

2019-11-18 Thread Thomas Preud'homme via Phabricator via cfe-commits
thopre accepted this revision.
thopre added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rC Clang

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

https://reviews.llvm.org/D68155



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


[PATCH] D70355: [clang-format] [NFC] add recent changes to release notes

2019-11-18 Thread Mitchell via Phabricator via cfe-commits
mitchell-stellar accepted this revision.
mitchell-stellar added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D70355



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


[clang] b4e2b11 - [Remarks][Driver] Use different remark files when targeting multiple architectures

2019-11-18 Thread Francis Visoiu Mistrih via cfe-commits

Author: Francis Visoiu Mistrih
Date: 2019-11-18T10:38:10-08:00
New Revision: b4e2b112b58154a89171df39dae80044865ff4ff

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

LOG: [Remarks][Driver] Use different remark files when targeting multiple 
architectures

When the driver is targeting multiple architectures at once, for things
like Universal Mach-Os, we need to emit different remark files for each
cc1 invocation to avoid overwriting the files from a different
invocation.

For example:

$ clang -c -o foo.o -fsave-optimization-record -arch x86_64 -arch x86_64h

will create two remark files:

* foo-x86_64.opt.yaml
* foo-x86_64h.opt.yaml

Added: 


Modified: 
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/opt-record.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 2b1c24275e3d..f5591c48d4e0 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5403,6 +5403,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
 if (A) {
   CmdArgs.push_back(A->getValue());
 } else {
+  bool hasMultipleArchs =
+  Args.getAllArgValues(options::OPT_arch).size() > 1;
   SmallString<128> F;
 
   if (Args.hasArg(options::OPT_c) || Args.hasArg(options::OPT_S)) {
@@ -5427,6 +5429,22 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
 }
   }
 
+  // If we're having more than one "-arch", we should name the files
+  // 
diff erently so that every cc1 invocation writes to a 
diff erent file.
+  // We're doing that by appending "-" with "" being the arch
+  // name from the triple.
+  if (hasMultipleArchs) {
+// First, remember the extension.
+SmallString<64> OldExtension = llvm::sys::path::extension(F);
+// then, remove it.
+llvm::sys::path::replace_extension(F, "");
+// attach - to it.
+F += "-";
+F += Triple.getArchName();
+// put back the extension.
+llvm::sys::path::replace_extension(F, OldExtension);
+  }
+
   std::string Extension = "opt.";
   if (const Arg *A =
   Args.getLastArg(options::OPT_fsave_optimization_record_EQ))

diff  --git a/clang/test/Driver/opt-record.c b/clang/test/Driver/opt-record.c
index 062d0acc17da..d8d2aa53ed40 100644
--- a/clang/test/Driver/opt-record.c
+++ b/clang/test/Driver/opt-record.c
@@ -18,6 +18,7 @@
 // RUN: %clang -### -S -o FOO -fsave-optimization-record 
-fsave-optimization-record=some-format %s 2>&1 | FileCheck %s 
-check-prefix=CHECK-EQ-FORMAT
 // RUN: %clang -### -S -o FOO -fsave-optimization-record=some-format %s 2>&1 | 
FileCheck %s -check-prefix=CHECK-EQ-FORMAT
 // RUN: %clang -### -S -o FOO -fsave-optimization-record=some-format 
-fno-save-optimization-record %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-FOPT-DISABLE-FORMAT
+// RUN: %clang -### -S -o FOO -fsave-optimization-record -arch x86_64 -arch 
x86_64h %s 2>&1 | FileCheck %s --check-prefix=CHECK-MULTIPLE-ARCH
 //
 // CHECK: "-cc1"
 // CHECK: "-opt-record-file" "FOO.opt.yaml"
@@ -41,3 +42,8 @@
 // CHECK-EQ-FORMAT: "-opt-record-format" "some-format"
 
 // CHECK-FOPT-DISABLE-FORMAT-NOT: "-fno-save-optimization-record"
+
+// CHECK-MULTIPLE-ARCH: "-cc1"
+// CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64.opt.yaml"
+// CHECK-MULTIPLE-ARCH: "-cc1"
+// CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64h.opt.yaml"



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


[clang] a77b66a - Allocate builtins table earlier to fix bug found by ubsan

2019-11-18 Thread Reid Kleckner via cfe-commits

Author: Reid Kleckner
Date: 2019-11-18T10:41:30-08:00
New Revision: a77b66a05625ea4271c2d76f65428cce02e4699e

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

LOG: Allocate builtins table earlier to fix bug found by ubsan

Follow up to 979da9a4c3ba

Added: 


Modified: 
clang/lib/Lex/Preprocessor.cpp

Removed: 




diff  --git a/clang/lib/Lex/Preprocessor.cpp b/clang/lib/Lex/Preprocessor.cpp
index 496eae980c00..0e9be3923630 100644
--- a/clang/lib/Lex/Preprocessor.cpp
+++ b/clang/lib/Lex/Preprocessor.cpp
@@ -113,6 +113,8 @@ 
Preprocessor::Preprocessor(std::shared_ptr PPOpts,
   // We haven't read anything from the external source.
   ReadMacrosFromExternalSource = false;
 
+  BuiltinInfo = std::make_unique();
+
   // "Poison" __VA_ARGS__, __VA_OPT__ which can only appear in the expansion of
   // a macro. They get unpoisoned where it is allowed.
   (Ident__VA_ARGS__ = getIdentifierInfo("__VA_ARGS__"))->setIsPoisoned();
@@ -203,7 +205,6 @@ void Preprocessor::Initialize(const TargetInfo &Target,
   this->AuxTarget = AuxTarget;
 
   // Initialize information about built-ins.
-  BuiltinInfo = std::make_unique();
   BuiltinInfo->InitializeTarget(Target, AuxTarget);
   HeaderInfo.setTarget(Target);
 



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


[clang] 30e7ee3 - Temporarily Revert "Add support for options -frounding-math, ftrapping-math, -ffp-model=, and -ffp-exception-behavior="

2019-11-18 Thread Eric Christopher via cfe-commits

Author: Eric Christopher
Date: 2019-11-18T10:46:48-08:00
New Revision: 30e7ee3c4bac4a12ea584a879aa320bd4e035cc2

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

LOG: Temporarily Revert "Add support for options -frounding-math, 
ftrapping-math, -ffp-model=, and -ffp-exception-behavior="
and a follow-up NFC rearrangement as it's causing a crash on valid. Testcase is 
on the original review thread.

This reverts commits af57dbf12e54f3a8ff48534bf1078f4de104c1cd and 
e6584b2b7b2de06f1e59aac41971760cac1e1b79

Added: 


Modified: 
clang/docs/UsersManual.rst
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/Driver/clang_f_opts.c
clang/test/Driver/fast-math.c
llvm/include/llvm/IR/IRBuilder.h
llvm/include/llvm/IR/IntrinsicInst.h
llvm/include/llvm/Target/TargetOptions.h
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
llvm/lib/IR/CMakeLists.txt
llvm/lib/IR/IntrinsicInst.cpp
llvm/unittests/IR/IRBuilderTest.cpp

Removed: 
clang/test/CodeGen/fpconstrained.c
clang/test/Driver/fp-model.c
llvm/include/llvm/IR/FPEnv.h
llvm/lib/IR/FPEnv.cpp



diff  --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index 62e2575c6b26..714681d7f4ce 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -1231,10 +1231,10 @@ are listed below.
 
 **-f[no-]trapping-math**
 
-   Control floating point exception behavior. ``-fno-trapping-math`` allows 
optimizations that assume that floating point operations cannot generate traps 
such as divide-by-zero, overflow and underflow.
-
-- The option ``-ftrapping-math`` behaves identically to 
``-ffp-exception-behavior=strict``.
-- The option ``-fno-trapping-math`` behaves identically to 
``-ffp-exception-behavior=ignore``.   This is the default.
+   ``-fno-trapping-math`` allows optimizations that assume that
+   floating point operations cannot generate traps such as divide-by-zero,
+   overflow and underflow. Defaults to ``-ftrapping-math``.
+   Currently this option has no effect.
 
 .. option:: -ffp-contract=
 
@@ -1319,52 +1319,6 @@ are listed below.
 
Defaults to ``-fno-finite-math``.
 
-.. _opt_frounding-math:
-
-**-f[no-]rounding-math**
-
-Force floating-point operations to honor the dynamically-set rounding mode by 
default.
-
-The result of a floating-point operation often cannot be exactly represented 
in the result type and therefore must be rounded.  IEEE 754 describes 
diff erent rounding modes that control how to perform this rounding, not all of 
which are supported by all implementations.  C provides interfaces 
(``fesetround`` and ``fesetenv``) for dynamically controlling the rounding 
mode, and while it also recommends certain conventions for changing the 
rounding mode, these conventions are not typically enforced in the ABI.  Since 
the rounding mode changes the numerical result of operations, the compiler must 
understand something about it in order to optimize floating point operations.
-
-Note that floating-point operations performed as part of constant 
initialization are formally performed prior to the start of the program and are 
therefore not subject to the current rounding mode.  This includes the 
initialization of global variables and local ``static`` variables.  
Floating-point operations in these contexts will be rounded using 
``FE_TONEAREST``.
-
-- The option ``-fno-rounding-math`` allows the compiler to assume that the 
rounding mode is set to ``FE_TONEAREST``.  This is the default.
-- The option ``-frounding-math`` forces the compiler to honor the 
dynamically-set rounding mode.  This prevents optimizations which might affect 
results if the rounding mode changes or is 
diff erent from the default; for example, it prevents floating-point operations 
from being reordered across most calls and prevents constant-folding when the 
result is not exactly representable.
-
-.. option:: -ffp-model=
-
-   Specify floating point behavior. ``-ffp-model`` is an umbrella
-   option that encompasses functionality provided by other, single
-   purpose, floating point options.  Valid values are: ``precise``, ``strict``,
-   and ``fast``.
-   Details:
-
-   * ``precise`` Disables optimizations that are not value-safe on 
floating-point data, although FP contraction (FMA) is enabled 
(``-ffp-contract=fast``).  This is the default behavior.
-   * ``strict`` Enables ``-frounding-math`` and 
``-ffp-exception-behavior=strict``, and disables contractions (FMA).  All of 
the ``-ffast-math`` enablements are disable

Re: [PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-18 Thread Eric Christopher via cfe-commits
Hi All,

I've gone ahead and temporarily reverted here:

echristo@jhereg ~/s/llvm-project> git push
To github.com:llvm/llvm-project.git
   a77b66a0562..30e7ee3c4ba  master -> master

and we can just reapply when the issues are fixed. Thanks :)

-eric

On Mon, Nov 18, 2019 at 6:58 AM Melanie Blower via Phabricator via
cfe-commits  wrote:
>
> mibintc added a comment.
>
> Thanks I see it, I'm working on a patch. Previously there was no support for 
> frounding-math (unimplemented). This patch enables the option. In the IR 
> builder, there's a call to a runtime function in the exception handler which 
> is unexpectedly null.  I start by adding a null pointer check.
>
>
> Repository:
>   rL LLVM
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D62731/new/
>
> https://reviews.llvm.org/D62731
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-18 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

In D62731#1749916 , @mibintc wrote:

> Thanks I see it, I'm working on a patch. Previously there was no support for 
> frounding-math (unimplemented). This patch enables the option. In the IR 
> builder, there's a call to a runtime function in the exception handler which 
> is unexpectedly null.  I start by adding a null pointer check.


Had a crash on valid here for days, let's revert and then get a fix when 
recommitting. I'll respond to the thread when reverting. Thanks :)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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


[clang] 1ff5f0c - Revert "[Remarks][Driver] Use different remark files when targeting multiple architectures"

2019-11-18 Thread Reid Kleckner via cfe-commits

Author: Reid Kleckner
Date: 2019-11-18T10:52:41-08:00
New Revision: 1ff5f0ced3163e457c799bd3bd838153806d6320

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

LOG: Revert "[Remarks][Driver] Use different remark files when targeting 
multiple architectures"

This reverts commit b4e2b112b58154a89171df39dae80044865ff4ff.

Test doesn't appear to pass on Windows, maybe all non-Mac.

Added: 


Modified: 
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/opt-record.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 2e3624e6cd24..6f25eea243ad 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5218,8 +5218,6 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
 if (A) {
   CmdArgs.push_back(A->getValue());
 } else {
-  bool hasMultipleArchs =
-  Args.getAllArgValues(options::OPT_arch).size() > 1;
   SmallString<128> F;
 
   if (Args.hasArg(options::OPT_c) || Args.hasArg(options::OPT_S)) {
@@ -5244,22 +5242,6 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
 }
   }
 
-  // If we're having more than one "-arch", we should name the files
-  // 
diff erently so that every cc1 invocation writes to a 
diff erent file.
-  // We're doing that by appending "-" with "" being the arch
-  // name from the triple.
-  if (hasMultipleArchs) {
-// First, remember the extension.
-SmallString<64> OldExtension = llvm::sys::path::extension(F);
-// then, remove it.
-llvm::sys::path::replace_extension(F, "");
-// attach - to it.
-F += "-";
-F += Triple.getArchName();
-// put back the extension.
-llvm::sys::path::replace_extension(F, OldExtension);
-  }
-
   std::string Extension = "opt.";
   if (const Arg *A =
   Args.getLastArg(options::OPT_fsave_optimization_record_EQ))

diff  --git a/clang/test/Driver/opt-record.c b/clang/test/Driver/opt-record.c
index d8d2aa53ed40..062d0acc17da 100644
--- a/clang/test/Driver/opt-record.c
+++ b/clang/test/Driver/opt-record.c
@@ -18,7 +18,6 @@
 // RUN: %clang -### -S -o FOO -fsave-optimization-record 
-fsave-optimization-record=some-format %s 2>&1 | FileCheck %s 
-check-prefix=CHECK-EQ-FORMAT
 // RUN: %clang -### -S -o FOO -fsave-optimization-record=some-format %s 2>&1 | 
FileCheck %s -check-prefix=CHECK-EQ-FORMAT
 // RUN: %clang -### -S -o FOO -fsave-optimization-record=some-format 
-fno-save-optimization-record %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-FOPT-DISABLE-FORMAT
-// RUN: %clang -### -S -o FOO -fsave-optimization-record -arch x86_64 -arch 
x86_64h %s 2>&1 | FileCheck %s --check-prefix=CHECK-MULTIPLE-ARCH
 //
 // CHECK: "-cc1"
 // CHECK: "-opt-record-file" "FOO.opt.yaml"
@@ -42,8 +41,3 @@
 // CHECK-EQ-FORMAT: "-opt-record-format" "some-format"
 
 // CHECK-FOPT-DISABLE-FORMAT-NOT: "-fno-save-optimization-record"
-
-// CHECK-MULTIPLE-ARCH: "-cc1"
-// CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64.opt.yaml"
-// CHECK-MULTIPLE-ARCH: "-cc1"
-// CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64h.opt.yaml"



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


Re: [clang] b4e2b11 - [Remarks][Driver] Use different remark files when targeting multiple architectures

2019-11-18 Thread Reid Kleckner via cfe-commits
I reverted this, the test fails for me locally. Does -arch work on non-Mac
targets?

On Mon, Nov 18, 2019 at 10:39 AM Francis Visoiu Mistrih via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> Author: Francis Visoiu Mistrih
> Date: 2019-11-18T10:38:10-08:00
> New Revision: b4e2b112b58154a89171df39dae80044865ff4ff
>
> URL:
> https://github.com/llvm/llvm-project/commit/b4e2b112b58154a89171df39dae80044865ff4ff
> DIFF:
> https://github.com/llvm/llvm-project/commit/b4e2b112b58154a89171df39dae80044865ff4ff.diff
>
> LOG: [Remarks][Driver] Use different remark files when targeting multiple
> architectures
>
> When the driver is targeting multiple architectures at once, for things
> like Universal Mach-Os, we need to emit different remark files for each
> cc1 invocation to avoid overwriting the files from a different
> invocation.
>
> For example:
>
> $ clang -c -o foo.o -fsave-optimization-record -arch x86_64 -arch x86_64h
>
> will create two remark files:
>
> * foo-x86_64.opt.yaml
> * foo-x86_64h.opt.yaml
>
> Added:
>
>
> Modified:
> clang/lib/Driver/ToolChains/Clang.cpp
> clang/test/Driver/opt-record.c
>
> Removed:
>
>
>
>
> 
> diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp
> b/clang/lib/Driver/ToolChains/Clang.cpp
> index 2b1c24275e3d..f5591c48d4e0 100644
> --- a/clang/lib/Driver/ToolChains/Clang.cpp
> +++ b/clang/lib/Driver/ToolChains/Clang.cpp
> @@ -5403,6 +5403,8 @@ void Clang::ConstructJob(Compilation &C, const
> JobAction &JA,
>  if (A) {
>CmdArgs.push_back(A->getValue());
>  } else {
> +  bool hasMultipleArchs =
> +  Args.getAllArgValues(options::OPT_arch).size() > 1;
>SmallString<128> F;
>
>if (Args.hasArg(options::OPT_c) || Args.hasArg(options::OPT_S)) {
> @@ -5427,6 +5429,22 @@ void Clang::ConstructJob(Compilation &C, const
> JobAction &JA,
>  }
>}
>
> +  // If we're having more than one "-arch", we should name the files
> +  //
> diff erently so that every cc1 invocation writes to a
> diff erent file.
> +  // We're doing that by appending "-" with "" being the
> arch
> +  // name from the triple.
> +  if (hasMultipleArchs) {
> +// First, remember the extension.
> +SmallString<64> OldExtension = llvm::sys::path::extension(F);
> +// then, remove it.
> +llvm::sys::path::replace_extension(F, "");
> +// attach - to it.
> +F += "-";
> +F += Triple.getArchName();
> +// put back the extension.
> +llvm::sys::path::replace_extension(F, OldExtension);
> +  }
> +
>std::string Extension = "opt.";
>if (const Arg *A =
>Args.getLastArg(options::OPT_fsave_optimization_record_EQ))
>
> diff  --git a/clang/test/Driver/opt-record.c
> b/clang/test/Driver/opt-record.c
> index 062d0acc17da..d8d2aa53ed40 100644
> --- a/clang/test/Driver/opt-record.c
> +++ b/clang/test/Driver/opt-record.c
> @@ -18,6 +18,7 @@
>  // RUN: %clang -### -S -o FOO -fsave-optimization-record
> -fsave-optimization-record=some-format %s 2>&1 | FileCheck %s
> -check-prefix=CHECK-EQ-FORMAT
>  // RUN: %clang -### -S -o FOO -fsave-optimization-record=some-format %s
> 2>&1 | FileCheck %s -check-prefix=CHECK-EQ-FORMAT
>  // RUN: %clang -### -S -o FOO -fsave-optimization-record=some-format
> -fno-save-optimization-record %s 2>&1 | FileCheck %s
> --check-prefix=CHECK-FOPT-DISABLE-FORMAT
> +// RUN: %clang -### -S -o FOO -fsave-optimization-record -arch x86_64
> -arch x86_64h %s 2>&1 | FileCheck %s --check-prefix=CHECK-MULTIPLE-ARCH
>  //
>  // CHECK: "-cc1"
>  // CHECK: "-opt-record-file" "FOO.opt.yaml"
> @@ -41,3 +42,8 @@
>  // CHECK-EQ-FORMAT: "-opt-record-format" "some-format"
>
>  // CHECK-FOPT-DISABLE-FORMAT-NOT: "-fno-save-optimization-record"
> +
> +// CHECK-MULTIPLE-ARCH: "-cc1"
> +// CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64.opt.yaml"
> +// CHECK-MULTIPLE-ARCH: "-cc1"
> +// CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64h.opt.yaml"
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-18 Thread Melanie Blower via Phabricator via cfe-commits
mibintc added a comment.

The incorrect code is actually in the IRBuilder which is part of a different 
patch...

In D62731#1750312 , @echristo wrote:

> In D62731#1749916 , @mibintc wrote:
>
> > Thanks I see it, I'm working on a patch. Previously there was no support 
> > for frounding-math (unimplemented). This patch enables the option. In the 
> > IR builder, there's a call to a runtime function in the exception handler 
> > which is unexpectedly null.  I start by adding a null pointer check.
>
>
> Had a crash on valid here for days, let's revert and then get a fix when 
> recommitting. I'll respond to the thread when reverting. Thanks :)





Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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


[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-18 Thread Melanie Blower via Phabricator via cfe-commits
mibintc added a comment.

In D62731#1750312 , @echristo wrote:

> In D62731#1749916 , @mibintc wrote:
>
> > Thanks I see it, I'm working on a patch. Previously there was no support 
> > for frounding-math (unimplemented). This patch enables the option. In the 
> > IR builder, there's a call to a runtime function in the exception handler 
> > which is unexpectedly null.  I start by adding a null pointer check.
>
>
> Had a crash on valid here for days, let's revert and then get a fix when 
> recommitting. I'll respond to the thread when reverting. Thanks :)


@echristo I just saw the bug was reported today, is the "crash on valid" 
visible on the bots?  Can you provide url showing same? Thanks
I opened https://bugs.llvm.org/show_bug.cgi?id=44048 for @jgorbe


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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


[PATCH] D69990: Populate CUDA flags on FreeBSD too, as many other toolchains do.

2019-11-18 Thread Dimitry Andric via Phabricator via cfe-commits
dim updated this revision to Diff 229883.
dim added a comment.

- Add cuda options test, copied from cuda-options.cu.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69990

Files:
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/FreeBSD.h
  clang/test/Driver/cuda-options-freebsd.cu

Index: clang/test/Driver/cuda-options-freebsd.cu
===
--- /dev/null
+++ clang/test/Driver/cuda-options-freebsd.cu
@@ -0,0 +1,289 @@
+// Tests CUDA compilation pipeline construction in Driver.
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+
+// Simple compilation case. Compile device-side to PTX assembly and make sure
+// we use it on the host side.
+// RUN: %clang -### -target x86_64-unknown-freebsd -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix HOST -check-prefix INCLUDES-DEVICE \
+// RUN:-check-prefix NOLINK %s
+
+// Typical compilation + link case.
+// RUN: %clang -### -target x86_64-unknown-freebsd %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix HOST -check-prefix INCLUDES-DEVICE \
+// RUN:-check-prefix LINK %s
+
+// Verify that --cuda-host-only disables device-side compilation, but doesn't
+// disable host-side compilation/linking.
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-host-only %s 2>&1 \
+// RUN: | FileCheck -check-prefix NODEVICE -check-prefix HOST \
+// RUN:-check-prefix NOINCLUDES-DEVICE -check-prefix LINK %s
+
+// Verify that --cuda-device-only disables host-side compilation and linking.
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-device-only %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix NOHOST -check-prefix NOLINK %s
+
+// Check that the last of --cuda-compile-host-device, --cuda-host-only, and
+// --cuda-device-only wins.
+
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-device-only \
+// RUN:--cuda-host-only %s 2>&1 \
+// RUN: | FileCheck -check-prefix NODEVICE -check-prefix HOST \
+// RUN:-check-prefix NOINCLUDES-DEVICE -check-prefix LINK %s
+
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-compile-host-device \
+// RUN:--cuda-host-only %s 2>&1 \
+// RUN: | FileCheck -check-prefix NODEVICE -check-prefix HOST \
+// RUN:-check-prefix NOINCLUDES-DEVICE -check-prefix LINK %s
+
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-host-only \
+// RUN:--cuda-device-only %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix NOHOST -check-prefix NOLINK %s
+
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-compile-host-device \
+// RUN:--cuda-device-only %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix NOHOST -check-prefix NOLINK %s
+
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-host-only \
+// RUN:   --cuda-compile-host-device %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix HOST -check-prefix INCLUDES-DEVICE \
+// RUN:-check-prefix LINK %s
+
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-device-only \
+// RUN:   --cuda-compile-host-device %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix HOST -check-prefix INCLUDES-DEVICE \
+// RUN:-check-prefix LINK %s
+
+// Verify that --cuda-gpu-arch option passes the correct GPU architecture to
+// device compilation.
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-gpu-arch=sm_30 -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix DEVICE-SM30 -check-prefix HOST \
+// RUN:-check-prefix INCLUDES-DEVICE -check-prefix NOLINK %s
+
+// Verify that there is one device-side compilation per --cuda-gpu-arch args
+// and that all results are included on the host side.
+// RUN: %clang -### -target x86_64-unknown-freebsd \
+// RUN:   --cuda-gpu-arch=sm_35 --cuda-gpu-arch=sm_30 -c %s 2>&1 \
+// RUN: | FileCheck -check-prefixes DEVICE,DEVICE-NOSAVE,DEVICE2 \
+// RUN: -check-prefixes DEVICE-SM30,DEVICE2-SM35 \
+// RUN: -check-prefixes INCLUDES-DEVICE,INCLUDES-DEVICE2 \
+// RUN: -check-prefixes HOST,HOST-NOSAVE,NOLINK %s
+
+// Verify that device-side results are passed to the correct tool when
+// -save-temps is used.
+// RUN: %clang -### -target x86_64-unknown-freebsd -save-temps -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-SAVE \
+// RUN:-check-prefix HOST -check-prefix HOST-SAVE -check-prefix NOLINK %s
+
+// Verify that device-side results are passed to the correct tool when
+// -fno-i

[PATCH] D69990: Populate CUDA flags on FreeBSD too, as many other toolchains do.

2019-11-18 Thread Dimitry Andric via Phabricator via cfe-commits
dim added reviewers: tra, yaxunl, ABataev.
dim added a comment.

Adding a few people who might know a bit more about CUDA specific things. 
Please take a look if this review makes any sense, thanks. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69990



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


Re: [PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-18 Thread Eric Christopher via cfe-commits
No, it's just the bug that Jorge found. I understand it might have
highlighted an existing bug, but reverting back to clean is the best
bet here and then we can recommit when we get the latent bug fixed :)

On Mon, Nov 18, 2019 at 11:08 AM Melanie Blower via Phabricator
 wrote:
>
> mibintc added a comment.
>
> In D62731#1750312 , @echristo wrote:
>
> > In D62731#1749916 , @mibintc wrote:
> >
> > > Thanks I see it, I'm working on a patch. Previously there was no support 
> > > for frounding-math (unimplemented). This patch enables the option. In the 
> > > IR builder, there's a call to a runtime function in the exception handler 
> > > which is unexpectedly null.  I start by adding a null pointer check.
> >
> >
> > Had a crash on valid here for days, let's revert and then get a fix when 
> > recommitting. I'll respond to the thread when reverting. Thanks :)
>
>
> @echristo I just saw the bug was reported today, is the "crash on valid" 
> visible on the bots?  Can you provide url showing same? Thanks
> I opened https://bugs.llvm.org/show_bug.cgi?id=44048 for @jgorbe
>
>
> Repository:
>   rL LLVM
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D62731/new/
>
> https://reviews.llvm.org/D62731
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] e15b26f - Reland: [Remarks][Driver] Use different remark files when targeting multiple architectures

2019-11-18 Thread Francis Visoiu Mistrih via cfe-commits

Author: Francis Visoiu Mistrih
Date: 2019-11-18T11:17:38-08:00
New Revision: e15b26fbbd901315a1402f97e83abf29bdce9a9f

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

LOG: Reland: [Remarks][Driver] Use different remark files when targeting 
multiple architectures

When the driver is targeting multiple architectures at once, for things
like Universal Mach-Os, we need to emit different remark files for each
cc1 invocation to avoid overwriting the files from a different
invocation.

For example:

$ clang -c -o foo.o -fsave-optimization-record -arch x86_64 -arch x86_64h

will create two remark files:

* foo-x86_64.opt.yaml
* foo-x86_64h.opt.yaml

Added: 
clang/test/Driver/darwin-opt-record.c

Modified: 
clang/lib/Driver/ToolChains/Clang.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 6f25eea243ad..3e00c323fc65 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5218,6 +5218,9 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
 if (A) {
   CmdArgs.push_back(A->getValue());
 } else {
+  bool hasMultipleArchs =
+  Triple.isOSDarwin() && // Only supported on Darwin platforms.
+  Args.getAllArgValues(options::OPT_arch).size() > 1;
   SmallString<128> F;
 
   if (Args.hasArg(options::OPT_c) || Args.hasArg(options::OPT_S)) {
@@ -5242,6 +5245,22 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
 }
   }
 
+  // If we're having more than one "-arch", we should name the files
+  // 
diff erently so that every cc1 invocation writes to a 
diff erent file.
+  // We're doing that by appending "-" with "" being the arch
+  // name from the triple.
+  if (hasMultipleArchs) {
+// First, remember the extension.
+SmallString<64> OldExtension = llvm::sys::path::extension(F);
+// then, remove it.
+llvm::sys::path::replace_extension(F, "");
+// attach - to it.
+F += "-";
+F += Triple.getArchName();
+// put back the extension.
+llvm::sys::path::replace_extension(F, OldExtension);
+  }
+
   std::string Extension = "opt.";
   if (const Arg *A =
   Args.getLastArg(options::OPT_fsave_optimization_record_EQ))

diff  --git a/clang/test/Driver/darwin-opt-record.c 
b/clang/test/Driver/darwin-opt-record.c
new file mode 100644
index ..ca0fad7ee16d
--- /dev/null
+++ b/clang/test/Driver/darwin-opt-record.c
@@ -0,0 +1,8 @@
+// REQUIRES: system-darwin
+
+// RUN: %clang -### -S -o FOO -fsave-optimization-record -arch x86_64 -arch 
x86_64h %s 2>&1 | FileCheck %s --check-prefix=CHECK-MULTIPLE-ARCH
+//
+// CHECK-MULTIPLE-ARCH: "-cc1"
+// CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64.opt.yaml"
+// CHECK-MULTIPLE-ARCH: "-cc1"
+// CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64h.opt.yaml"



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


[PATCH] D69876: Allow output constraints on "asm goto"

2019-11-18 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added inline comments.



Comment at: clang/lib/Analysis/UninitializedValues.cpp:830
+
+  for (unsigned i = 0, e = as->getNumOutputs(); i != e; ++i) {
+FindVarResult Var = findVar(as->getOutputExpr(i));

`GCCAsmStmt` inherits from `AsmStmt` which has an `outputs()` method for 
returning an iterator. Please use that an a range based for loop.



Comment at: clang/lib/Analysis/UninitializedValues.cpp:877
+if (as->isAsmGoto())
+  tf.Visit(const_cast(as));
   return vals.updateValueVectorWithScratch(block);

Don't declare `as` as `const`, then you won't need this `const_cast`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69876



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


  1   2   >