[PATCH] D52880: [clang-tidy] fix PR39167, bugprone-exception-escape hangs-up

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

This patch does not fix the underlying issue. I do have a candidate that might 
cause it, but I am not sure if it is functionally preserving (tests seems to 
work though!). I think it is best if @baloghadamsoftware takes a look at it as 
well!


Repository:
  rL LLVM

https://reviews.llvm.org/D52880



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

[PATCH] D45045: [DebugInfo] Generate debug information for labels.

[Hsiangkai Wang via Phabricator via cfe-commits]

HsiangKai added a comment.

Fix the bug in https://reviews.llvm.org/D52927.


Repository:
  rL LLVM

https://reviews.llvm.org/D45045



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

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In https://reviews.llvm.org/D51340#1254898, @takuto.ikuta wrote:

> Ping?


Sorry, I know I'm being slow to look at this :-( I have not forgotten, it's 
just a lot of other things happening at the same time.


https://reviews.llvm.org/D51340



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

[PATCH] D52872: [clangd] Make binary index format the default, remove dead flag.

[Haojian Wu via Phabricator via cfe-commits]

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

looks good.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52872



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

[PATCH] D52445: [Index] Use locations to uniquify function-scope BindingDecl USR

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Can you add a test?


Repository:
  rC Clang

https://reviews.llvm.org/D52445



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

[clang-tools-extra] r343841 - [clangd] Make binary index format the default, remove dead flag.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Fri Oct  5 02:05:28 2018
New Revision: 343841

URL: http://llvm.org/viewvc/llvm-project?rev=343841&view=rev
Log:
[clangd] Make binary index format the default, remove dead flag.

Reviewers: hokein

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

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

Modified:
clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp

Modified: clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp?rev=343841&r1=343840&r2=343841&view=diff
==
--- clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp Fri Oct  5 02:05:28 
2018
@@ -7,8 +7,7 @@
 //
 
//===--===//
 //
-// GlobalSymbolBuilder is a tool to extract symbols from a whole project.
-// This tool is **experimental** only. Don't use it in production code.
+// clangd-indexer is a tool to gather index data (symbols, xrefs) from source.
 //
 
//===--===//
 
@@ -21,7 +20,6 @@
 #include "clang/Tooling/Execution.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/CommandLine.h"
-#include "llvm/Support/Path.h"
 #include "llvm/Support/Signals.h"
 
 using namespace llvm;
@@ -32,22 +30,13 @@ namespace clang {
 namespace clangd {
 namespace {
 
-static llvm::cl::opt AssumedHeaderDir(
-"assume-header-dir",
-llvm::cl::desc("The index includes header that a symbol is defined in. "
-   "If the absolute path cannot be determined (e.g. an "
-   "in-memory VFS) then the relative path is resolved against "
-   "this directory, which must be absolute. If this flag is "
-   "not given, such headers will have relative paths."),
-llvm::cl::init(""));
-
 static llvm::cl::opt
 Format("format", llvm::cl::desc("Format of the index to be written"),
llvm::cl::values(clEnumValN(IndexFileFormat::YAML, "yaml",
"human-readable YAML format"),
 clEnumValN(IndexFileFormat::RIFF, "binary",
"binary RIFF format")),
-   llvm::cl::init(IndexFileFormat::YAML));
+   llvm::cl::init(IndexFileFormat::RIFF));
 
 class IndexActionFactory : public tooling::FrontendActionFactory {
 public:
@@ -55,7 +44,6 @@ public:
 
   clang::FrontendAction *create() override {
 SymbolCollector::Options Opts;
-Opts.FallbackDir = AssumedHeaderDir;
 return createStaticIndexingAction(
Opts,
[&](SymbolSlab S) {
@@ -102,15 +90,14 @@ int main(int argc, const char **argv) {
 
   const char *Overview = R"(
   Creates an index of symbol information etc in a whole project.
-  This is **experimental** and not production-ready!
 
   Example usage for a project using CMake compile commands:
 
-  $ clangd-indexer --executor=all-TUs compile_commands.json > index.yaml
+  $ clangd-indexer --executor=all-TUs compile_commands.json > clangd.dex
 
   Example usage for file sequence index without flags:
 
-  $ clangd-indexer File1.cpp File2.cpp ... FileN.cpp > index.yaml
+  $ clangd-indexer File1.cpp File2.cpp ... FileN.cpp > clangd.dex
 
   Note: only symbols from header files will be indexed.
   )";
@@ -123,12 +110,6 @@ int main(int argc, const char **argv) {
 return 1;
   }
 
-  if (!clang::clangd::AssumedHeaderDir.empty() &&
-  !llvm::sys::path::is_absolute(clang::clangd::AssumedHeaderDir)) {
-llvm::errs() << "--assume-header-dir must be an absolute path.\n";
-return 1;
-  }
-
   // Collect symbols found in each translation unit, merging as we go.
   clang::clangd::IndexFileIn Data;
   auto Err = Executor->get()->execute(


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

[PATCH] D52872: [clangd] Make binary index format the default, remove dead flag.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL343841: [clangd] Make binary index format the default, 
remove dead flag. (authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52872?vs=168287&id=168444#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52872

Files:
  clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp


Index: clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
===
--- clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
+++ clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
@@ -7,8 +7,7 @@
 //
 
//===--===//
 //
-// GlobalSymbolBuilder is a tool to extract symbols from a whole project.
-// This tool is **experimental** only. Don't use it in production code.
+// clangd-indexer is a tool to gather index data (symbols, xrefs) from source.
 //
 
//===--===//
 
@@ -21,7 +20,6 @@
 #include "clang/Tooling/Execution.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/CommandLine.h"
-#include "llvm/Support/Path.h"
 #include "llvm/Support/Signals.h"
 
 using namespace llvm;
@@ -32,30 +30,20 @@
 namespace clangd {
 namespace {
 
-static llvm::cl::opt AssumedHeaderDir(
-"assume-header-dir",
-llvm::cl::desc("The index includes header that a symbol is defined in. "
-   "If the absolute path cannot be determined (e.g. an "
-   "in-memory VFS) then the relative path is resolved against "
-   "this directory, which must be absolute. If this flag is "
-   "not given, such headers will have relative paths."),
-llvm::cl::init(""));
-
 static llvm::cl::opt
 Format("format", llvm::cl::desc("Format of the index to be written"),
llvm::cl::values(clEnumValN(IndexFileFormat::YAML, "yaml",
"human-readable YAML format"),
 clEnumValN(IndexFileFormat::RIFF, "binary",
"binary RIFF format")),
-   llvm::cl::init(IndexFileFormat::YAML));
+   llvm::cl::init(IndexFileFormat::RIFF));
 
 class IndexActionFactory : public tooling::FrontendActionFactory {
 public:
   IndexActionFactory(IndexFileIn &Result) : Result(Result) {}
 
   clang::FrontendAction *create() override {
 SymbolCollector::Options Opts;
-Opts.FallbackDir = AssumedHeaderDir;
 return createStaticIndexingAction(
Opts,
[&](SymbolSlab S) {
@@ -102,15 +90,14 @@
 
   const char *Overview = R"(
   Creates an index of symbol information etc in a whole project.
-  This is **experimental** and not production-ready!
 
   Example usage for a project using CMake compile commands:
 
-  $ clangd-indexer --executor=all-TUs compile_commands.json > index.yaml
+  $ clangd-indexer --executor=all-TUs compile_commands.json > clangd.dex
 
   Example usage for file sequence index without flags:
 
-  $ clangd-indexer File1.cpp File2.cpp ... FileN.cpp > index.yaml
+  $ clangd-indexer File1.cpp File2.cpp ... FileN.cpp > clangd.dex
 
   Note: only symbols from header files will be indexed.
   )";
@@ -123,12 +110,6 @@
 return 1;
   }
 
-  if (!clang::clangd::AssumedHeaderDir.empty() &&
-  !llvm::sys::path::is_absolute(clang::clangd::AssumedHeaderDir)) {
-llvm::errs() << "--assume-header-dir must be an absolute path.\n";
-return 1;
-  }
-
   // Collect symbols found in each translation unit, merging as we go.
   clang::clangd::IndexFileIn Data;
   auto Err = Executor->get()->execute(


Index: clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
===
--- clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
+++ clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
@@ -7,8 +7,7 @@
 //
 //===--===//
 //
-// GlobalSymbolBuilder is a tool to extract symbols from a whole project.
-// This tool is **experimental** only. Don't use it in production code.
+// clangd-indexer is a tool to gather index data (symbols, xrefs) from source.
 //
 //===--===//
 
@@ -21,7 +20,6 @@
 #include "clang/Tooling/Execution.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/CommandLine.h"
-#include "llvm/Support/Path.h"
 #include "llvm/Support/Signals.h"
 
 using namespace llvm;
@@ -32,30 +30,20 @@
 namespace clangd {
 namespace {
 
-static llvm::cl::opt AssumedHeaderDir(
-"assume-header-dir",
-llvm::cl::desc("The index includes header that a symbol is defined in. "
-   "If the absolute path cannot be determined (e.g. an "
-   "in-memory VFS) then the 

RE: [clang-tools-extra] r343778 - [clangd] clangd-indexer gathers refs and stores them in index files.

[via cfe-commits]

Hi Sam,

Our internal build bot hit an assertion failure in the changes you made in this 
commit that I was able to reproduce by building your change on my machine on 
Windows 7 using Visual Studio 2015. None of the public build bots seem to have 
hit this issue, though I am not sure why at the moment.

Here is the assertion failure that is hit while running ClangdTests.exe:

[--] 2 tests from SerializationTest
[ RUN  ] SerializationTest.YAMLConversions
[   OK ] SerializationTest.YAMLConversions (0 ms)
[ RUN  ] SerializationTest.BinaryConversions
Assertion failed: OutBufCur == OutBufStart && "raw_ostream destructor called 
with non-empty buffer!", file 
C:\src\upstream\llvm2\lib\Support\raw_ostream.cpp, line 73
0x00013FA74405 (0x0016 0x0200 0x06D100230006 
0x07FEEA9BAA51), HandleAbort() + 0x5 bytes(s), 
c:\src\upstream\llvm2\lib\support\windows\signals.inc, line 409
0x07FEEAA1DC17 (0x0001 0x0001 0x 
0x009BF460), raise() + 0x1E7 bytes(s)
0x07FEEAA1EAA1 (0x07FE0003 0x07FE0003 0x0001413AF810 
0x0001413AF790), abort() + 0x31 bytes(s)
0x07FEEAA20751 (0x0049 0x0001413AF810 0x0122 
0x07FEEA9C05D6), _get_wpgmptr() + 0x1BE1 bytes(s)
0x07FEEAA20A5F (0x009BF890 0x009BF680 0x00B05AB0 
0x00014126F8C5), _wassert() + 0x3F bytes(s)
0x00013FA40677 (0x 0x00A8 0x009BF890 
0x00013FA41A2E), llvm::raw_ostream::~raw_ostream() + 0x37 bytes(s), 
c:\src\upstream\llvm2\lib\support\raw_ostream.cpp, line 75
0x00013FA4057C (0x04BA1FF0 0x009BF680 0x00B05AB0 
0x00B05AB0), llvm::raw_fd_ostream::~raw_fd_ostream() + 0x11C bytes(s), 
c:\src\upstream\llvm2\lib\support\raw_ostream.cpp, line 617
0x00013F96A4CC (0x04BA1FF0 0x00B05AB0 0x00B05AB0 
0x04BA1FF0), clang::clangd::`anonymous 
namespace'::SerializationTest_BinaryConversions_Test::TestBody() + 0x34C 
bytes(s), 
c:\src\upstream\llvm2\tools\clang\tools\extra\unittests\clangd\serializationtests.cpp,
 line 166
0x00013FAE201C (0x04BA1FF0 0x00B05AB0 0x0001413C4B68 
0x07FEEAA954B8), 
testing::internal::HandleSehExceptionsInMethodIfSupported() 
+ 0xC bytes(s), c:\src\upstream\llvm2\utils\unittest\googletest\src\gtest.cc, 
line 2387 + 0x2 byte(s)
0x00013FB09E5B (0x04BA1FF0 0x00B05AB0 0x0001413C4D30 
0x009BF9E8), testing::Test::Run() + 0x7B bytes(s), 
c:\src\upstream\llvm2\utils\unittest\googletest\src\gtest.cc, line 2481
0x00013FB0A0DB (0x016641B94CAF 0x00B05AB0 0x00B1DE30 
0x00AFF560), testing::TestInfo::Run() + 0xAB bytes(s), 
c:\src\upstream\llvm2\utils\unittest\googletest\src\gtest.cc, line 2660
0x00013FB09F72 (0x0023 0x 0x00B05AB0 
0x00AFF560), testing::TestCase::Run() + 0xB2 bytes(s), 
c:\src\upstream\llvm2\utils\unittest\googletest\src\gtest.cc, line 2774 + 0x12 
byte(s)
0x00013FB0A529 (0x00AFF410 0x 0x0001 
0x07FE0001), testing::internal::UnitTestImpl::RunAllTests() + 0x229 
bytes(s), c:\src\upstream\llvm2\utils\unittest\googletest\src\gtest.cc, line 
4649 + 0x39 byte(s)
0x00013FAE213C (0x00AFF410 0x 0x0001413C54A8 
0x), 
testing::internal::HandleSehExceptionsInMethodIfSupported()
 + 0xC bytes(s), c:\src\upstream\llvm2\utils\unittest\googletest\src\gtest.cc, 
line 2387 + 0x2 byte(s)
0x00013FB0A27C (0x8002 0x 0x 
0x), testing::UnitTest::Run() + 0xEC bytes(s), 
c:\src\upstream\llvm2\utils\unittest\googletest\src\gtest.cc, line 4257 + 0x17 
byte(s)
0x000141299397 (0x00010001 0x 0x 
0x01D45C4625BC735E), main() + 0xB7 bytes(s), 
c:\src\upstream\llvm2\utils\unittest\unittestmain\testmain.cpp, line 52
0x0001412707C1 (0x 0x 0x 
0x), __scrt_common_main_seh() + 0x11D bytes(s), 
f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl, line 253 + 0x22 byte(s)
0x76C159CD (0x 0x 0x 
0x), BaseThreadInitThunk() + 0xD bytes(s)
0x76E7385D (0x 0x 0x 
0x), RtlUserThreadStart() + 0x1D bytes(s)

Could you take a look into this?

Douglas Yung

> -Original Message-
> From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf
> Of Sam McCall via cfe-commits
> Sent: Thursday, October 04, 2018 7:10
> To: cfe-commits@lists.llvm.org
> Subject: [clang-tools-extra] r343778 - [clangd] clangd-indexer gathers
> refs and stores them in index files.
> 
> Author: sammccall
> Date: Thu Oct  4 07:09:55 2018
> New Revision: 343778
> 
> URL: http://ll

[PATCH] D52928: [clangd] Fix a subtle case for GetBeginningOfIdentifier.

[Haojian Wu via Phabricator via cfe-commits]

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

The following case 1 was incorrectly regarded as case 2:

  MACRO(foo->^function())

Also add few more tests for macro.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52928

Files:
  clangd/ClangdUnit.cpp
  unittests/clangd/ClangdUnitTests.cpp


Index: unittests/clangd/ClangdUnitTests.cpp
===
--- unittests/clangd/ClangdUnitTests.cpp
+++ unittests/clangd/ClangdUnitTests.cpp
@@ -205,6 +205,13 @@
 }
 
 TEST(ClangdUnitTest, GetBeginningOfIdentifier) {
+  StringRef Common = R"cpp(
+struct Foo { int func(); };
+#define MACRO(X) void f() { X; }
+Foo* foo;
+Foo bar;
+)cpp";
+
   // First ^ is the expected beginning, last is the search position.
   for (const char *Text : {
"int ^f^oo();", // inside identifier
@@ -214,15 +221,27 @@
"^int foo();",  // beginning of file (can't back up)
"int ^f0^0();", // after a digit (lexing at N-1 is wrong)
"int ^λλ^λ();", // UTF-8 handled properly when backing up
+
+   // testcases where identifier is in macro.
+   "MACRO(foo->^func())", // beginning of identifier
+   "MACRO(foo->^fun^c())", // inside identifier
+   "MACRO(foo->^func^())", // end of identifier
+   "MACRO(^foo->func())", // begin identifier
+   "MACRO(^foo^->func())", // end identifier
+   "^MACRO(foo->funct())", // beginning of macro name
+   "^MAC^RO(foo->funct())", // inside macro name
+   "^MACRO^(foo->funct())", // end of macro name
}) {
-Annotations TestCase(Text);
+std::string TestCode = (Common + Text).str();
+Annotations TestCase(TestCode);
 auto AST = TestTU::withCode(TestCase.code()).build();
 const auto &SourceMgr = AST.getASTContext().getSourceManager();
 SourceLocation Actual = getBeginningOfIdentifier(
 AST, TestCase.points().back(), SourceMgr.getMainFileID());
-Position ActualPos =
-offsetToPosition(TestCase.code(), SourceMgr.getFileOffset(Actual));
-EXPECT_EQ(TestCase.points().front(), ActualPos) << Text;
+Position ActualPos = offsetToPosition(
+TestCase.code(),
+SourceMgr.getFileOffset(SourceMgr.getSpellingLoc(Actual)));
+EXPECT_EQ(TestCase.points().front(), ActualPos) << TestCode;
   }
 }
 
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -407,8 +407,15 @@
   SourceMgr.getMacroArgExpandedLocation(InputLoc.getLocWithOffset(-1));
   Before = Lexer::GetBeginningOfToken(Before, SourceMgr, AST.getLangOpts());
   Token Tok;
+  // A subtle case 1:
+  //   MACRO(foo->^function())
+  // ^~ Before
+  // getRawToken intentionally returns the macro name (whose token is
+  // identifier), we do want the token at Before loc, to achieve that, we pass 
a
+  // spelling location to getRawToken.
   if (Before.isValid() &&
-  !Lexer::getRawToken(Before, Tok, SourceMgr, AST.getLangOpts(), false) &&
+  !Lexer::getRawToken(SourceMgr.getSpellingLoc(Before), Tok, SourceMgr,
+  AST.getLangOpts(), false) &&
   Tok.is(tok::raw_identifier))
 return Before;// Case 2.
   return SourceMgr.getMacroArgExpandedLocation(InputLoc); // Case 1 or 3.


Index: unittests/clangd/ClangdUnitTests.cpp
===
--- unittests/clangd/ClangdUnitTests.cpp
+++ unittests/clangd/ClangdUnitTests.cpp
@@ -205,6 +205,13 @@
 }
 
 TEST(ClangdUnitTest, GetBeginningOfIdentifier) {
+  StringRef Common = R"cpp(
+struct Foo { int func(); };
+#define MACRO(X) void f() { X; }
+Foo* foo;
+Foo bar;
+)cpp";
+
   // First ^ is the expected beginning, last is the search position.
   for (const char *Text : {
"int ^f^oo();", // inside identifier
@@ -214,15 +221,27 @@
"^int foo();",  // beginning of file (can't back up)
"int ^f0^0();", // after a digit (lexing at N-1 is wrong)
"int ^λλ^λ();", // UTF-8 handled properly when backing up
+
+   // testcases where identifier is in macro.
+   "MACRO(foo->^func())", // beginning of identifier
+   "MACRO(foo->^fun^c())", // inside identifier
+   "MACRO(foo->^func^())", // end of identifier
+   "MACRO(^foo->func())", // begin identifier
+   "MACRO(^foo^->func())", // end identifier
+   "^MACRO(foo->funct())", // beginning of macro name
+   "^MAC^RO(foo->funct())", // inside macro name
+   "^MACRO^(foo->funct())", // end of macro name
}) {
-Annotations TestCase(Text);
+std::string TestCode = (Common + Text).str();
+Annotations TestCase(TestCode);
 auto AST = TestTU::withCode(TestCase.code()).build();

[PATCH] D52892: [Clang-tidy] readability check to convert numerical constants to std::numeric_limits

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

@aaron.ballman I have a question for an expert: How would bitfields relate to 
this check? Can there be a similar pattern for them and do they need to be 
handled here?




Comment at: clang-tidy/readability/NumericalCostantsToMaxIntCheck.cpp:62
+CompilerInstance &Compiler) {
+  if (this->getLangOpts().CPlusPlus) {
+Compiler.getPreprocessor().addPPCallbacks(

you dont need `this->` and please use the same return early pattern as in the 
other registering call



Comment at: clang-tidy/readability/NumericalCostantsToMaxIntCheck.cpp:64
+Compiler.getPreprocessor().addPPCallbacks(
+::llvm::make_unique(InsertLimits,
+&IncludeLocation));

its uncommong to specify the global namespace (first `::`), you can remove these



Comment at: clang-tidy/readability/NumericalCostantsToMaxIntCheck.cpp:77
+hasOperatorName("-"),
+hasUnaryOperand(integerLiteral(equals(1)).bind("Lit"),
+hasInitializer(ignoringImpCasts(unaryOperator(

Please use the full name for the bind, `Literal` or `IntLiteral` (please 
consistent with `VarDecl`, so CamelCase). I feel that this is more readable (no 
strong opinion though if you want to leave it as is)



Comment at: clang-tidy/readability/NumericalCostantsToMaxIntCheck.cpp:93
+
+  SourceManager *SM = Result.SourceManager;
+  std::string InsteadOf = "-1";

Please declar this variable later, directly before usage (locality), and add 
`const SourceManager* SM` as you don't seem to manipulate `SM`



Comment at: clang-tidy/readability/NumericalCostantsToMaxIntCheck.cpp:95
+  std::string InsteadOf = "-1";
+  std::string Replacement =
+  ("std::numeric_limits<" +

You can use `llvm::Twine` here which is very efficient for concatenation.

```
std::string Replacement = (Twine("std::numeric_limits<") + 
Decl->getType().getAtomicUnqualifiedType().getAsString() + ">::max()").str();
```



Comment at: clang-tidy/readability/NumericalCostantsToMaxIntCheck.cpp:107
+  << FixItHint::CreateReplacement(
+ SourceRange(Lit->getBeginLoc().getLocWithOffset(-1),
+ Lit->getEndLoc()),

I think its better to create the source-range before to deal with exceptional 
cases (invalid ranges, macros)



Comment at: docs/clang-tidy/checks/list.rst:12
abseil-redundant-strcat-calls
-   abseil-string-find-startswith
abseil-str-cat-append

spurious change, does it still happen on master? i thought this was ordered at 
some point already



Comment at: docs/clang-tidy/checks/list.rst:219
portability-simd-intrinsics
+   readability-NumericalCostantsToMaxInt
readability-avoid-const-params-in-decls

this does not follow the naming convention (hyphens and not CamelCase)



Comment at: 
docs/clang-tidy/checks/readability-numerical-costants-to-max-int.rst:4
+readability-numerical-costants-to-max-int
+==
+

please make `===` full length.



Comment at: 
docs/clang-tidy/checks/readability-numerical-costants-to-max-int.rst:10
+
+If necessary it adds the library 'limits'.
+

This line is slightly inaccurate. The library is the standard library, 
`s/library/header-file`



Comment at: 
docs/clang-tidy/checks/readability-numerical-costants-to-max-int.rst:37
+  
\ No newline at end of file


Please remove the empty lines at the end



Comment at: test/clang-tidy/readability-numerical-costants-to-max-int.cpp:9
+
+unsigned const long Uval2 = -1;
+// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: use 'std::numeric_limits::max()' instead of '-1' for unsigned constant int 
[readability-numerical-costants-to-max-int]

Please add testcases for macros (literal in macro, variable declaration in 
macro, other possibilities you might find) and for templates.

```
template 
void f() {
Foo f = -1;
}

template 
void f2() {
decltype(NonType) new_var = -1;
}
```
and variations. Please try to cover as many possible constructs as you can.


Another interesting case are enums, example:

```
enum FooEnum : unsigned {
   bar = 0,
   foobar = 1,
   maximum = -1
}
``` 
You don't need to handle this case yet, but please add a testcase to 
demonstrate the behaviour of the check for it. Adding a `TODO:` would be 
perfect to signal future extendability.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52892



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

r343843 - [AArch64] Use filecheck captures for metadata node numbers in test. NFC

[David Green via cfe-commits]

Author: dmgreen
Date: Fri Oct  5 03:21:25 2018
New Revision: 343843

URL: http://llvm.org/viewvc/llvm-project?rev=343843&view=rev
Log:
[AArch64] Use filecheck captures for metadata node numbers in test. NFC

Just a quick fix for cases where extra metadata members are present.

Modified:
cfe/trunk/test/CodeGen/arm64-microsoft-intrinsics.c

Modified: cfe/trunk/test/CodeGen/arm64-microsoft-intrinsics.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/arm64-microsoft-intrinsics.c?rev=343843&r1=343842&r2=343843&view=diff
==
--- cfe/trunk/test/CodeGen/arm64-microsoft-intrinsics.c (original)
+++ cfe/trunk/test/CodeGen/arm64-microsoft-intrinsics.c Fri Oct  5 03:21:25 2018
@@ -74,7 +74,7 @@ unsigned __int64 check__getReg() {
   return reg;
 }
 
-// CHECK-MSVC: call i64 @llvm.read_register.i64(metadata !2)
-// CHECK-MSVC: call i64 @llvm.read_register.i64(metadata !3)
-// CHECK-MSVC: !2 = !{!"x18"}
-// CHECK-MSVC: !3 = !{!"sp"}
+// CHECK-MSVC: call i64 @llvm.read_register.i64(metadata ![[MD2:.*]])
+// CHECK-MSVC: call i64 @llvm.read_register.i64(metadata ![[MD3:.*]])
+// CHECK-MSVC: ![[MD2]] = !{!"x18"}
+// CHECK-MSVC: ![[MD3]] = !{!"sp"}


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

[PATCH] D51332: [clang-tidy] Replace deprecated std::ios_base aliases

[Bence Ando via Phabricator via cfe-commits]

andobence updated this revision to Diff 168452.
andobence added a comment.

Rebased on top of master.
I don't have commit rights.


https://reviews.llvm.org/D51332

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/DeprecatedIosBaseAliasesCheck.cpp
  clang-tidy/modernize/DeprecatedIosBaseAliasesCheck.h
  clang-tidy/modernize/ModernizeTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-deprecated-ios-base-aliases.rst
  test/clang-tidy/modernize-deprecated-ios-base-aliases.cpp

Index: test/clang-tidy/modernize-deprecated-ios-base-aliases.cpp
===
--- test/clang-tidy/modernize-deprecated-ios-base-aliases.cpp
+++ test/clang-tidy/modernize-deprecated-ios-base-aliases.cpp
@@ -0,0 +1,239 @@
+// RUN: %check_clang_tidy %s modernize-deprecated-ios-base-aliases %t
+
+namespace std {
+class ios_base {
+public:
+  typedef int io_state;
+  typedef int open_mode;
+  typedef int seek_dir;
+
+  typedef int streampos;
+  typedef int streamoff;
+};
+
+template 
+class basic_ios : public ios_base {
+};
+} // namespace std
+
+// Test function return values (declaration)
+std::ios_base::io_state f_5();
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'std::ios_base::io_state' is deprecated; use 'std::ios_base::iostate' instead [modernize-deprecated-ios-base-aliases]
+// CHECK-FIXES: std::ios_base::iostate f_5();
+
+// Test function parameters.
+void f_6(std::ios_base::open_mode);
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: 'std::ios_base::open_mode' is deprecated
+// CHECK-FIXES: void f_6(std::ios_base::openmode);
+void f_7(const std::ios_base::seek_dir &);
+// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 'std::ios_base::seek_dir' is deprecated
+// CHECK-FIXES: void f_7(const std::ios_base::seekdir &);
+
+// Test on record type fields.
+struct A {
+  std::ios_base::io_state field;
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'std::ios_base::io_state' is deprecated
+  // CHECK-FIXES: std::ios_base::iostate field;
+
+  typedef std::ios_base::io_state int_ptr_type;
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: 'std::ios_base::io_state' is deprecated
+  // CHECK-FIXES: typedef std::ios_base::iostate int_ptr_type;
+};
+
+struct B : public std::ios_base {
+  io_state a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'std::ios_base::io_state' is deprecated
+  // CHECK-FIXES: iostate a;
+};
+
+struct C : public std::basic_ios {
+  io_state a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'std::ios_base::io_state' is deprecated
+  // CHECK-FIXES: iostate a;
+};
+
+void f_1() {
+  std::ios_base::io_state a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'std::ios_base::io_state' is deprecated
+  // CHECK-FIXES: std::ios_base::iostate a;
+
+  // Check that spaces aren't modified unnecessarily.
+  std :: ios_base :: io_state b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 'std::ios_base::io_state' is deprecated
+  // CHECK-FIXES: std :: ios_base :: iostate b;
+
+  // Test construction from a temporary.
+  std::ios_base::io_state c = std::ios_base::io_state{};
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'std::ios_base::io_state' is deprecated
+  // CHECK-MESSAGES: :[[@LINE-2]]:46: warning: 'std::ios_base::io_state' is deprecated
+  // CHECK-FIXES: std::ios_base::iostate c = std::ios_base::iostate{};
+
+  typedef std::ios_base::io_state alias1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: 'std::ios_base::io_state' is deprecated
+  // CHECK-FIXES: typedef std::ios_base::iostate alias1;
+  alias1 d(a);
+
+  using alias2 = std::ios_base::io_state;
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: 'std::ios_base::io_state' is deprecated
+  // CHECK-FIXES: using alias2 = std::ios_base::iostate;
+  alias2 e;
+
+  // Test pointers.
+  std::ios_base::io_state *f;
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'std::ios_base::io_state' is deprecated
+  // CHECK-FIXES: std::ios_base::iostate *f;
+
+  // Test 'static' declarations.
+  static std::ios_base::io_state g;
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: 'std::ios_base::io_state' is deprecated
+  // CHECK-FIXES: static std::ios_base::iostate g;
+
+  // Test with cv-qualifiers.
+  const std::ios_base::io_state h(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 'std::ios_base::io_state' is deprecated
+  // CHECK-FIXES: const std::ios_base::iostate h(0);
+  volatile std::ios_base::io_state i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 'std::ios_base::io_state' is deprecated
+  // CHECK-FIXES: volatile std::ios_base::iostate i;
+  const volatile std::ios_base::io_state j(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: 'std::ios_base::io_state' is deprecated
+  // CHECK-FIXES: const volatile std::ios_base::iostate j(0);
+
+  // Test auto and initializer-list.
+  auto k = std::ios_base::io_state{};
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 'std::ios_base::io_state' is deprecated
+  // CHECK-FIXES: auto k = std::ios_base::iostate{};
+

[PATCH] D52888: Thread safety analysis: Handle conditional expression in getTrylockCallExpr

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D52888#1255862, @aaronpuchert wrote:

> Additional changes (including some non-tail recursion unfortunately) would 
> allow the following to work:
>
>   void foo16() {
> if (cond ? mu.TryLock() : false)
>   mu.Unlock();
>   }
>   
>   void foo17() {
> if (cond ? true : !mu.TryLock())
>   return;
> mu.Unlock();
>   }
>   
>
> Worth the effort, or is that too exotic?


`foo16()` looks like code I could plausibly see in the wild. Consider the case 
where the mutex is a pointer and you want to test it for null before calling 
`TryLock()` on it (`M ? M->TryLock() : false`). However, I don't have a good 
feeling for how much work it would be to support that case.




Comment at: test/SemaCXX/warn-thread-safety-analysis.cpp:1879-1880
+  void foo13() {
+if (mu.TryLock() ? 1 : 0)
+  mu.Unlock();
+  }

aaronpuchert wrote:
> aaron.ballman wrote:
> > aaronpuchert wrote:
> > > aaron.ballman wrote:
> > > > Can you add a test that shows we get it right even if the user does 
> > > > something less than practical, like:
> > > > ```
> > > > if (mu.TryLock() ? mu.TryLock() : false); // Warn about double lock
> > > > if (mu.TryLock() ? mu.Unlock(), 1 : 0)
> > > >   mu.Unlock(); // Warn about unlocking unheld lock
> > > > if (mu.TryLock() ? 1 : mu.Unlock(), 0)
> > > >   mu.Unlock(); // Warn about unlocking an unheld lock
> > > > if (mu.TryLock() ? (true ? mu.TryLock() : false) : false); // Warn 
> > > > about double lock
> > > > ```
> > > I'm afraid these don't work as expected if we don't branch on `?:` as 
> > > before. Basically we treat this as if both conditions where evaluated 
> > > unconditionally.
> > > 
> > > On calling a try-lock function, no lock is immediately acquired. Only 
> > > when we branch on the result of that try-lock call is the mutex added to 
> > > the capability set on the appropriate branch. Now if we branch on `?:` 
> > > (the situation before this change), we are going to complain about 
> > > conditionally held locks on the join point in @hokein's use case, and if 
> > > we don't branch (the behavior after this change), we are not treating 
> > > your examples correctly. I'm not sure we can treat both as intended.
> > > 
> > > I'm slightly leaning towards not branching: the C++ standard says that 
> > > only the [used expression is actually 
> > > evaluated](https://en.cppreference.com/w/cpp/language/operator_other#Conditional_operator),
> > >  but I think it's not considered good practice to have side effects in a 
> > > ternary operator. (I can't actually find it anywhere besides a discussion 
> > > on [Hacker News](https://news.ycombinator.com/item?id=14810994). 
> > > Personally I would prefer to use an if-statement if one of the 
> > > expressions has side-effects.)
> > > 
> > > Tell me what you think.
> > I agree that we don't want to encourage complex code in ternary operators, 
> > but that complexity does come up often as a result of macro expansion. It 
> > feels like there isn't a good answer here because either approach will have 
> > unexpected results.
> > 
> > I'll let @delesley be the tie-breaker on whether we follow ternary branches 
> > or not, but if we decide to accept this patch and not follow branches, I 
> > think we should capture those test cases as expected false negatives with a 
> > comment about why they happen and why it's expected behavior.
> > 
> > Related, does this patch then impact the behavior of code like:
> > ```
> > void f(Mutex &M) {
> >   M.Lock();
> > }
> > 
> > void g() {
> >   Mutex mu;
> >   (void)(true ? f(mu) : (void)0);
> >   mu.Unlock(); // Ok
> > }
> > ```
> > 
> No, this is not affected. There will be a warning independently of this 
> patch. We notice that the lock is held on only one branch after joining them.
> 
> I think my wording was a bit unfortunate: we still follow the ordinary CFG. 
> On try-lock the lock is not acquired immediately, only when we branch on the 
> return value. So we weaken the usual requirement of "no conditional locks" 
> for the period between the try-lock call and the branch. That was the 
> situation before this patch already.
> 
> With this patch `?:` branches on the return value of a try-lock are no longer 
> considered as possible acquisition points. We postpone the acquisition even 
> further to another (more obvious) branch like an `if` statement that uses the 
> try-lock return result. To that end we inspect possible `ConditionalExpr`s so 
> that we can derive which branch holds the lock even when these are used.
Ah, okay, thank you for the further explanation. That makes me a bit more 
comfortable with the proposed approach.


Repository:
  rC Clang

https://reviews.llvm.org/D52888



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

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

[Takuto Ikuta via Phabricator via cfe-commits]

takuto.ikuta added a comment.

In https://reviews.llvm.org/D51340#1256299, @hans wrote:

> In https://reviews.llvm.org/D51340#1254898, @takuto.ikuta wrote:
>
> > Ping?
>
>
> Sorry, I know I'm being slow to look at this :-( I have not forgotten, it's 
> just a lot of other things happening at the same time.


I see. Sorry for rushing you. I wait until you have time.


https://reviews.llvm.org/D51340



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

[clang-tools-extra] r343844 - [clangd] Fix a subtle case for GetBeginningOfIdentifier.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Fri Oct  5 05:08:06 2018
New Revision: 343844

URL: http://llvm.org/viewvc/llvm-project?rev=343844&view=rev
Log:
[clangd] Fix a subtle case for GetBeginningOfIdentifier.

Calling getMacroArgExpansionLocation too early was causing
Lexer::getRawToken to do the wrong thing - lexing the macro name instead
of the arg contents.

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

Modified:
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=343844&r1=343843&r2=343844&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Fri Oct  5 05:08:06 2018
@@ -390,7 +390,6 @@ SourceLocation clangd::getBeginningOfIde
 log("getBeginningOfIdentifier: {0}", Offset.takeError());
 return SourceLocation();
   }
-  SourceLocation InputLoc = SourceMgr.getComposedLoc(FID, *Offset);
 
   // GetBeginningOfToken(pos) is almost what we want, but does the wrong thing
   // if the cursor is at the end of the identifier.
@@ -401,15 +400,16 @@ SourceLocation clangd::getBeginningOfIde
   //  3) anywhere outside an identifier, we'll get some non-identifier thing.
   // We can't actually distinguish cases 1 and 3, but returning the original
   // location is correct for both!
+  SourceLocation InputLoc = SourceMgr.getComposedLoc(FID, *Offset);
   if (*Offset == 0) // Case 1 or 3.
 return SourceMgr.getMacroArgExpandedLocation(InputLoc);
-  SourceLocation Before =
-  SourceMgr.getMacroArgExpandedLocation(InputLoc.getLocWithOffset(-1));
+  SourceLocation Before = SourceMgr.getComposedLoc(FID, *Offset - 1);
+
   Before = Lexer::GetBeginningOfToken(Before, SourceMgr, AST.getLangOpts());
   Token Tok;
   if (Before.isValid() &&
   !Lexer::getRawToken(Before, Tok, SourceMgr, AST.getLangOpts(), false) &&
   Tok.is(tok::raw_identifier))
-return Before;// Case 2.
+return SourceMgr.getMacroArgExpandedLocation(Before); // Case 2.
   return SourceMgr.getMacroArgExpandedLocation(InputLoc); // Case 1 or 3.
 }

Modified: clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp?rev=343844&r1=343843&r2=343844&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp Fri Oct  5 
05:08:06 2018
@@ -205,8 +205,13 @@ main.cpp:2:3: error: something terrible
 }
 
 TEST(ClangdUnitTest, GetBeginningOfIdentifier) {
+  std::string Preamble = R"cpp(
+struct Bar { int func(); };
+#define MACRO(X) void f() { X; }
+Bar* bar;
+  )cpp";
   // First ^ is the expected beginning, last is the search position.
-  for (const char *Text : {
+  for (std::string Text : std::vector{
"int ^f^oo();", // inside identifier
"int ^foo();",  // beginning of identifier
"int ^foo^();", // end of identifier
@@ -214,14 +219,26 @@ TEST(ClangdUnitTest, GetBeginningOfIdent
"^int foo();",  // beginning of file (can't back up)
"int ^f0^0();", // after a digit (lexing at N-1 is wrong)
"int ^λλ^λ();", // UTF-8 handled properly when backing up
+
+   // identifier in macro arg
+   "MACRO(bar->^func())",  // beginning of identifier
+   "MACRO(bar->^fun^c())", // inside identifier
+   "MACRO(bar->^func^())", // end of identifier
+   "MACRO(^bar->func())",  // begin identifier
+   "MACRO(^bar^->func())", // end identifier
+   "^MACRO(bar->func())",  // beginning of macro name
+   "^MAC^RO(bar->func())", // inside macro name
+   "^MACRO^(bar->func())", // end of macro name
}) {
-Annotations TestCase(Text);
+std::string WithPreamble = Preamble + Text;
+Annotations TestCase(WithPreamble);
 auto AST = TestTU::withCode(TestCase.code()).build();
 const auto &SourceMgr = AST.getASTContext().getSourceManager();
 SourceLocation Actual = getBeginningOfIdentifier(
 AST, TestCase.points().back(), SourceMgr.getMainFileID());
-Position ActualPos =
-offsetToPosition(TestCase.code(), SourceMgr.getFileOffset(Actual));
+Position ActualPos = offsetToPosition(
+TestCase.code(),
+SourceMgr.getFileOffset(SourceMgr.getSpellingLoc(Actual)));
 EXPECT_EQ(TestCase.points().front(), ActualPos) << Text;
   }
 }


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

[PATCH] D52928: [clangd] Fix a subtle case for GetBeginningOfIdentifier.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

(Stealing this to make some revisions after pair-programming)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52928



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

[PATCH] D52928: [clangd] Fix a subtle case for GetBeginningOfIdentifier.

[Sam McCall via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE343844: [clangd] Fix a subtle case for 
GetBeginningOfIdentifier. (authored by sammccall, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52928?vs=168445&id=168455#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52928

Files:
  clangd/ClangdUnit.cpp
  unittests/clangd/ClangdUnitTests.cpp


Index: unittests/clangd/ClangdUnitTests.cpp
===
--- unittests/clangd/ClangdUnitTests.cpp
+++ unittests/clangd/ClangdUnitTests.cpp
@@ -205,23 +205,40 @@
 }
 
 TEST(ClangdUnitTest, GetBeginningOfIdentifier) {
+  std::string Preamble = R"cpp(
+struct Bar { int func(); };
+#define MACRO(X) void f() { X; }
+Bar* bar;
+  )cpp";
   // First ^ is the expected beginning, last is the search position.
-  for (const char *Text : {
+  for (std::string Text : std::vector{
"int ^f^oo();", // inside identifier
"int ^foo();",  // beginning of identifier
"int ^foo^();", // end of identifier
"int foo(^);",  // non-identifier
"^int foo();",  // beginning of file (can't back up)
"int ^f0^0();", // after a digit (lexing at N-1 is wrong)
"int ^λλ^λ();", // UTF-8 handled properly when backing up
+
+   // identifier in macro arg
+   "MACRO(bar->^func())",  // beginning of identifier
+   "MACRO(bar->^fun^c())", // inside identifier
+   "MACRO(bar->^func^())", // end of identifier
+   "MACRO(^bar->func())",  // begin identifier
+   "MACRO(^bar^->func())", // end identifier
+   "^MACRO(bar->func())",  // beginning of macro name
+   "^MAC^RO(bar->func())", // inside macro name
+   "^MACRO^(bar->func())", // end of macro name
}) {
-Annotations TestCase(Text);
+std::string WithPreamble = Preamble + Text;
+Annotations TestCase(WithPreamble);
 auto AST = TestTU::withCode(TestCase.code()).build();
 const auto &SourceMgr = AST.getASTContext().getSourceManager();
 SourceLocation Actual = getBeginningOfIdentifier(
 AST, TestCase.points().back(), SourceMgr.getMainFileID());
-Position ActualPos =
-offsetToPosition(TestCase.code(), SourceMgr.getFileOffset(Actual));
+Position ActualPos = offsetToPosition(
+TestCase.code(),
+SourceMgr.getFileOffset(SourceMgr.getSpellingLoc(Actual)));
 EXPECT_EQ(TestCase.points().front(), ActualPos) << Text;
   }
 }
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -390,7 +390,6 @@
 log("getBeginningOfIdentifier: {0}", Offset.takeError());
 return SourceLocation();
   }
-  SourceLocation InputLoc = SourceMgr.getComposedLoc(FID, *Offset);
 
   // GetBeginningOfToken(pos) is almost what we want, but does the wrong thing
   // if the cursor is at the end of the identifier.
@@ -401,15 +400,16 @@
   //  3) anywhere outside an identifier, we'll get some non-identifier thing.
   // We can't actually distinguish cases 1 and 3, but returning the original
   // location is correct for both!
+  SourceLocation InputLoc = SourceMgr.getComposedLoc(FID, *Offset);
   if (*Offset == 0) // Case 1 or 3.
 return SourceMgr.getMacroArgExpandedLocation(InputLoc);
-  SourceLocation Before =
-  SourceMgr.getMacroArgExpandedLocation(InputLoc.getLocWithOffset(-1));
+  SourceLocation Before = SourceMgr.getComposedLoc(FID, *Offset - 1);
+
   Before = Lexer::GetBeginningOfToken(Before, SourceMgr, AST.getLangOpts());
   Token Tok;
   if (Before.isValid() &&
   !Lexer::getRawToken(Before, Tok, SourceMgr, AST.getLangOpts(), false) &&
   Tok.is(tok::raw_identifier))
-return Before;// Case 2.
+return SourceMgr.getMacroArgExpandedLocation(Before); // Case 2.
   return SourceMgr.getMacroArgExpandedLocation(InputLoc); // Case 1 or 3.
 }


Index: unittests/clangd/ClangdUnitTests.cpp
===
--- unittests/clangd/ClangdUnitTests.cpp
+++ unittests/clangd/ClangdUnitTests.cpp
@@ -205,23 +205,40 @@
 }
 
 TEST(ClangdUnitTest, GetBeginningOfIdentifier) {
+  std::string Preamble = R"cpp(
+struct Bar { int func(); };
+#define MACRO(X) void f() { X; }
+Bar* bar;
+  )cpp";
   // First ^ is the expected beginning, last is the search position.
-  for (const char *Text : {
+  for (std::string Text : std::vector{
"int ^f^oo();", // inside identifier
"int ^foo();",  // beginning of identifier
"int ^foo^();", // end of identifier
"int foo(^);",  // non-identifier
"^int foo();",  // beginning of file (can't back up)
"int ^f0^0();", // after a digit (lexing at N-1 is wrong)
  

[PATCH] Add some automatic tests for DivideZero checker

[Tamás Zolnai via cfe-commits]

Hi all,

I'm a new contributor in clang / llvm. I'm planning to work on clang static
analyzer: maybe add new checker, imporve the exisiting one, etc. As the
first step I checked how core.DivideZero checker works now and added some
test cases for regression testing (see attached patch).
The patch contains not only test cases when the checker catches an issue,
but also use cases when the checker misses a division by zero situation,
showing that there is some points where the checker can be improved.

Best Regards,
Tamás Zolnai


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

[clang-tools-extra] r343845 - [clangd] Remove debugging output in test

[Sam McCall via cfe-commits]

Author: sammccall
Date: Fri Oct  5 05:22:40 2018
New Revision: 343845

URL: http://llvm.org/viewvc/llvm-project?rev=343845&view=rev
Log:
[clangd] Remove debugging output in test

Modified:
clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp

Modified: clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp?rev=343845&r1=343844&r2=343845&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp Fri Oct  5 
05:22:40 2018
@@ -157,11 +157,6 @@ TEST(SerializationTest, BinaryConversion
   IndexFileOut Out(*In);
   Out.Format = IndexFileFormat::RIFF;
   std::string Serialized = llvm::to_string(Out);
-  {
-std::error_code EC;
-llvm::raw_fd_ostream F("/tmp/foo", EC);
-F << Serialized;
-  }
 
   auto In2 = readIndexFile(Serialized);
   ASSERT_TRUE(bool(In2)) << In.takeError();


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

Re: [clang-tools-extra] r343778 - [clangd] clangd-indexer gathers refs and stores them in index files.

[Sam McCall via cfe-commits]

Thanks! There was actually some old debugging code that got left over in a
test. Sorry about that!
Fixed in r343845.

On Fri, Oct 5, 2018 at 11:34 AM  wrote:

> Hi Sam,
>
> Our internal build bot hit an assertion failure in the changes you made in
> this commit that I was able to reproduce by building your change on my
> machine on Windows 7 using Visual Studio 2015. None of the public build
> bots seem to have hit this issue, though I am not sure why at the moment.
>
> Here is the assertion failure that is hit while running ClangdTests.exe:
>
> [--] 2 tests from SerializationTest
> [ RUN  ] SerializationTest.YAMLConversions
> [   OK ] SerializationTest.YAMLConversions (0 ms)
> [ RUN  ] SerializationTest.BinaryConversions
> Assertion failed: OutBufCur == OutBufStart && "raw_ostream destructor
> called with non-empty buffer!", file
> C:\src\upstream\llvm2\lib\Support\raw_ostream.cpp, line 73
> 0x00013FA74405 (0x0016 0x0200
> 0x06D100230006 0x07FEEA9BAA51), HandleAbort() + 0x5 bytes(s),
> c:\src\upstream\llvm2\lib\support\windows\signals.inc, line 409
> 0x07FEEAA1DC17 (0x0001 0x0001
> 0x 0x009BF460), raise() + 0x1E7 bytes(s)
> 0x07FEEAA1EAA1 (0x07FE0003 0x07FE0003
> 0x0001413AF810 0x0001413AF790), abort() + 0x31 bytes(s)
> 0x07FEEAA20751 (0x0049 0x0001413AF810
> 0x0122 0x07FEEA9C05D6), _get_wpgmptr() + 0x1BE1 bytes(s)
> 0x07FEEAA20A5F (0x009BF890 0x009BF680
> 0x00B05AB0 0x00014126F8C5), _wassert() + 0x3F bytes(s)
> 0x00013FA40677 (0x 0x00A8
> 0x009BF890 0x00013FA41A2E), llvm::raw_ostream::~raw_ostream() +
> 0x37 bytes(s), c:\src\upstream\llvm2\lib\support\raw_ostream.cpp, line 75
> 0x00013FA4057C (0x04BA1FF0 0x009BF680
> 0x00B05AB0 0x00B05AB0),
> llvm::raw_fd_ostream::~raw_fd_ostream() + 0x11C bytes(s),
> c:\src\upstream\llvm2\lib\support\raw_ostream.cpp, line 617
> 0x00013F96A4CC (0x04BA1FF0 0x00B05AB0
> 0x00B05AB0 0x04BA1FF0), clang::clangd::`anonymous
> namespace'::SerializationTest_BinaryConversions_Test::TestBody() + 0x34C
> bytes(s),
> c:\src\upstream\llvm2\tools\clang\tools\extra\unittests\clangd\serializationtests.cpp,
> line 166
> 0x00013FAE201C (0x04BA1FF0 0x00B05AB0
> 0x0001413C4B68 0x07FEEAA954B8),
> testing::internal::HandleSehExceptionsInMethodIfSupported()
> + 0xC bytes(s),
> c:\src\upstream\llvm2\utils\unittest\googletest\src\gtest.cc, line 2387 +
> 0x2 byte(s)
> 0x00013FB09E5B (0x04BA1FF0 0x00B05AB0
> 0x0001413C4D30 0x009BF9E8), testing::Test::Run() + 0x7B
> bytes(s), c:\src\upstream\llvm2\utils\unittest\googletest\src\gtest.cc,
> line 2481
> 0x00013FB0A0DB (0x016641B94CAF 0x00B05AB0
> 0x00B1DE30 0x00AFF560), testing::TestInfo::Run() + 0xAB
> bytes(s), c:\src\upstream\llvm2\utils\unittest\googletest\src\gtest.cc,
> line 2660
> 0x00013FB09F72 (0x0023 0x
> 0x00B05AB0 0x00AFF560), testing::TestCase::Run() + 0xB2
> bytes(s), c:\src\upstream\llvm2\utils\unittest\googletest\src\gtest.cc,
> line 2774 + 0x12 byte(s)
> 0x00013FB0A529 (0x00AFF410 0x
> 0x0001 0x07FE0001),
> testing::internal::UnitTestImpl::RunAllTests() + 0x229 bytes(s),
> c:\src\upstream\llvm2\utils\unittest\googletest\src\gtest.cc, line 4649 +
> 0x39 byte(s)
> 0x00013FAE213C (0x00AFF410 0x
> 0x0001413C54A8 0x),
> testing::internal::HandleSehExceptionsInMethodIfSupported()
> + 0xC bytes(s),
> c:\src\upstream\llvm2\utils\unittest\googletest\src\gtest.cc, line 2387 +
> 0x2 byte(s)
> 0x00013FB0A27C (0x8002 0x
> 0x 0x), testing::UnitTest::Run() + 0xEC
> bytes(s), c:\src\upstream\llvm2\utils\unittest\googletest\src\gtest.cc,
> line 4257 + 0x17 byte(s)
> 0x000141299397 (0x00010001 0x
> 0x 0x01D45C4625BC735E), main() + 0xB7 bytes(s),
> c:\src\upstream\llvm2\utils\unittest\unittestmain\testmain.cpp, line 52
> 0x0001412707C1 (0x 0x
> 0x 0x), __scrt_common_main_seh() + 0x11D
> bytes(s), f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl, line 253
> + 0x22 byte(s)
> 0x76C159CD (0x 0x
> 0x 0x), BaseThreadInitThunk() + 0xD bytes(s)
> 0x76E7385D (0x 0x
> 0x 0x), RtlUserThreadStart() + 0x1D bytes(s)
>
> Could you take a look into this?
>
> Douglas Yung
>
> > -Original Message-
> > From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf
> > Of Sam McCall via cfe-c

[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Patch is missing tests -- perhaps you could dump the AST and check the casting 
notation from the output?


https://reviews.llvm.org/D52918



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

r343846 - Fix llvm-clang-x86_64-expensive-checks-win build by setting bigobj flag.

[Simon Pilgrim via cfe-commits]

Author: rksimon
Date: Fri Oct  5 05:33:57 2018
New Revision: 343846

URL: http://llvm.org/viewvc/llvm-project?rev=343846&view=rev
Log:
Fix llvm-clang-x86_64-expensive-checks-win build by setting bigobj flag.

Modified:
cfe/trunk/lib/Sema/CMakeLists.txt

Modified: cfe/trunk/lib/Sema/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/CMakeLists.txt?rev=343846&r1=343845&r2=343846&view=diff
==
--- cfe/trunk/lib/Sema/CMakeLists.txt (original)
+++ cfe/trunk/lib/Sema/CMakeLists.txt Fri Oct  5 05:33:57 2018
@@ -5,6 +5,7 @@ set(LLVM_LINK_COMPONENTS
 if (MSVC)
   set_source_files_properties(SemaDeclAttr.cpp PROPERTIES COMPILE_FLAGS 
/bigobj)
   set_source_files_properties(SemaExpr.cpp PROPERTIES COMPILE_FLAGS /bigobj)
+  set_source_files_properties(SemaExprCXX.cpp PROPERTIES COMPILE_FLAGS /bigobj)
   set_source_files_properties(SemaTemplate.cpp PROPERTIES COMPILE_FLAGS 
/bigobj)
 endif()
 


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

[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I can't help but notice how badly C.133 and C.9 interact with C.131 and I'm 
worried we will wind up with clang-tidy checks that leave the user in an 
impossible situation where they need to make data members private and provide 
trivial accessors for them. Do you have thoughts on how to avoid that? The C++ 
Core Guidelines seem silent on the matter -- this might be worth raising with 
the authors.

The HIC++ standard has an exception that requires the data type to be in an 
`extern "C"` context, but I don't see that reflected in the tests or code.




Comment at: clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp:22
+
+AST_MATCHER(clang::CXXRecordDecl, hasMethods) {
+  return std::distance(Node.method_begin(), Node.method_end()) != 0;

Can drop the `clang::` qualifiers -- you're in the `namespace clang` scope 
already.



Comment at: clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp:56
+  auto RecordIsInteresting =
+  allOf(anyOf(isStruct(), isClass()), hasMethods(), hasNonStaticMethod(),
+IgnoreClassesWithAllMemberVariablesBeingPublic

Neither the C++ Core Guidelines nor HIC++ carve out an exception for union 
types. I think you should talk to the guideline authors to see what they think. 
Perhaps first try to diagnose unions the same as structs and classes and see 
how the results look to you -- if they look bad, you can incorporate those as 
examples when discussing with the authors.



Comment at: clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp:82
+   "member variable '%0' of %1 '%2' has %3 visibility")
+  << Field->getName() << Record->getKindName() << Record->getName()
+  << Field->getAccess();

Drop the single quotes above and just pass in `Field` and `Record` directly -- 
the diagnostic printer will do the right thing.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52771



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

[clang-tools-extra] r343848 - [clang-tidy] Replace deprecated std::ios_base aliases

[Jonas Toth via cfe-commits]

Author: jonastoth
Date: Fri Oct  5 06:36:00 2018
New Revision: 343848

URL: http://llvm.org/viewvc/llvm-project?rev=343848&view=rev
Log:
[clang-tidy] Replace deprecated std::ios_base aliases

This check warns the uses of the deprecated member types of std::ios_base
and replaces those that have a non-deprecated equivalent.

Patch by andobence!

Reviewd by: alexfh

Revision ID: https://reviews.llvm.org/D51332

Added:

clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedIosBaseAliasesCheck.cpp
clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedIosBaseAliasesCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-deprecated-ios-base-aliases.rst

clang-tools-extra/trunk/test/clang-tidy/modernize-deprecated-ios-base-aliases.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt?rev=343848&r1=343847&r2=343848&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt Fri Oct  5 
06:36:00 2018
@@ -4,6 +4,7 @@ add_clang_library(clangTidyModernizeModu
   AvoidBindCheck.cpp
   ConcatNestedNamespacesCheck.cpp
   DeprecatedHeadersCheck.cpp
+  DeprecatedIosBaseAliasesCheck.cpp
   LoopConvertCheck.cpp
   LoopConvertUtils.cpp
   MakeSharedCheck.cpp

Added: 
clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedIosBaseAliasesCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedIosBaseAliasesCheck.cpp?rev=343848&view=auto
==
--- 
clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedIosBaseAliasesCheck.cpp 
(added)
+++ 
clang-tools-extra/trunk/clang-tidy/modernize/DeprecatedIosBaseAliasesCheck.cpp 
Fri Oct  5 06:36:00 2018
@@ -0,0 +1,80 @@
+//===--- DeprecatedIosBaseAliasesCheck.cpp - 
clang-tidy===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "DeprecatedIosBaseAliasesCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+static const std::array DeprecatedTypes = {
+"::std::ios_base::io_state",  "::std::ios_base::open_mode",
+"::std::ios_base::seek_dir",  "::std::ios_base::streamoff",
+"::std::ios_base::streampos",
+};
+
+static const llvm::StringMap ReplacementTypes = {
+{"io_state", "iostate"},
+{"open_mode", "openmode"},
+{"seek_dir", "seekdir"}};
+
+void DeprecatedIosBaseAliasesCheck::registerMatchers(MatchFinder *Finder) {
+  // Only register the matchers for C++; the functionality currently does not
+  // provide any benefit to other languages, despite being benign.
+  if (!getLangOpts().CPlusPlus)
+return;
+
+  auto IoStateDecl = typedefDecl(hasAnyName(DeprecatedTypes)).bind("TypeDecl");
+  auto IoStateType =
+  qualType(hasDeclaration(IoStateDecl), unless(elaboratedType()));
+
+  Finder->addMatcher(typeLoc(loc(IoStateType)).bind("TypeLoc"), this);
+}
+
+void DeprecatedIosBaseAliasesCheck::check(
+const MatchFinder::MatchResult &Result) {
+  SourceManager &SM = *Result.SourceManager;
+
+  const auto *Typedef = Result.Nodes.getNodeAs("TypeDecl");
+  StringRef TypeName = Typedef->getName();
+  bool HasReplacement = ReplacementTypes.count(TypeName);
+
+  const auto *TL = Result.Nodes.getNodeAs("TypeLoc");
+  SourceLocation IoStateLoc = TL->getBeginLoc();
+
+  // Do not generate fixits for matches depending on template arguments and
+  // macro expansions.
+  bool Fix = HasReplacement && !TL->getType()->isDependentType();
+  if (IoStateLoc.isMacroID()) {
+IoStateLoc = SM.getSpellingLoc(IoStateLoc);
+Fix = false;
+  }
+
+  SourceLocation EndLoc = IoStateLoc.getLocWithOffset(TypeName.size() - 1);
+
+  if (HasReplacement) {
+auto FixName = ReplacementTypes.lookup(TypeName);
+auto Builder = diag(IoStateLoc, "'std::ios_base::%0' is deprecated; use "
+"'std::ios_base::%1' instead")
+   << TypeName << FixName;
+
+if (Fix)
+  Builder << FixItHint::CreateReplacement(SourceRange(IoStateLoc, EndLoc),
+  FixName);
+  } else
+diag(IoStateLoc, "'std::ios_base::%0' is deprecated") << TypeNa

[PATCH] D51332: [clang-tidy] Replace deprecated std::ios_base aliases

[Jonas Toth via Phabricator via cfe-commits]

JonasToth closed this revision.
JonasToth added a comment.

Commited in https://reviews.llvm.org/rL343848.
Thank you for the patch!


https://reviews.llvm.org/D51332



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

[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp:26
+
+AST_MATCHER(clang::CXXRecordDecl, hasNonStaticMethod) {
+  return hasMethod(unless(isStaticStorageClass()))

lebedev.ri wrote:
> JonasToth wrote:
> > I think this and the next matcher can be a normal variable.
> > 
> > ```
> > auto HasNonStaticMethod = hasMethod(unless(isStaticStorageClass()));
> > 
> > ...
> > ... allOf(anyOf(isStruct(), isClass()), HasMethods, HasNonStaticMethod)
> > ...
> > ```
> They could, but these have a very meaningful meaning, so i'd argue it would 
> not be worse to keep them 
> out here refactored as proper matchers, in case someone else needs them 
> elsewhere (so he could just
> move them from here into more general place, as opposed to writing a 
> duplicating matcher.)
Ok.



Comment at: test/clang-tidy/misc-non-private-member-variables-in-classes.cpp:1
+// RUN: %check_clang_tidy -check-suffix=NONPRIVATE %s 
misc-non-private-member-variables-in-classes %t
+// RUN: %check_clang_tidy -check-suffix=NONPRIVATE %s 
misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: 
[{key: 
misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, 
value: 0}, {key: 
misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic,
 value: 0}]}' --

lebedev.ri wrote:
> JonasToth wrote:
> > I would prefer multiple test-files instead of mixing them all together.
> Hmm, no. Then the tests will have to be duplicated in N files,
> because i really do want to see what each of these 4 configurations do on the 
> *same* input.
> 
> It only looks ugly because the script only supports `-check-suffix=`,
> and not `-check-suffixes=`, which would significantly cut down on check-lines.
Ok, if you prefer this.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52771



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

[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In https://reviews.llvm.org/D52771#1256432, @aaron.ballman wrote:

> I can't help but notice how badly C.133 and C.9 interact with C.131 and I'm 
> worried we will wind up with clang-tidy checks that leave the user in an 
> impossible situation where they need to make data members private and provide 
> trivial accessors for them. Do you have thoughts on how to avoid that? The 
> C++ Core Guidelines seem silent on the matter -- this might be worth raising 
> with the authors.


I have an idea to avoid trivial set/get methods: Flag public data if it is 
modified internally. This could serve as a heuristic for pre/postconditions on 
that variable. 
In general one could resolve all data-dependencies inside the class and figure 
out if a private variable would need to change if a public variable got 
changed. But that is way more complex!

Still worth asking the authors.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52771



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

[PATCH] D52690: [clang-tidy] NFC use CHECK-NOTES in tests for misc-misplaced-const

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: test/clang-tidy/misc-misplaced-const.c:18
+  // CHECK-NOTES: :[[@LINE-1]]:12: warning: 'i3' declared with a 
const-qualified typedef type; results in the type being 'int *const' instead of 
'const int *'
+  // CHECK-NOTES: :[[@LINE-14]]:14: note: typedef declared here
 

alexfh wrote:
> JonasToth wrote:
> > alexfh wrote:
> > > These notes are also just marginally useful and make it harder to change 
> > > the test. I wonder whether an absolute line number would make more sense 
> > > here. Or maybe just add a test for one of the notes and leave out the 
> > > rest (and keep CHECK-MESSAGES)?
> > Absolute line number makes sense. IMHO the tests should cover all generated 
> > diagnostics including the notes. Would you accept sticking with 
> > `CHECK-NOTES` but with absolute line numbers?
> > IMHO the tests should cover all generated diagnostics including the notes.
> 
> I wouldn't call this the most important goal. I'd say that tests should cover 
> important aspects of the output, not every single character of it. Another 
> useful feature is that tests should be easy to create, read, and change.  In 
> cases like this - where the benefit of the change is not obvious - I would 
> leave the decision to the author of the check.
> 
> Aaron, WDYT?
@aaron.ballman not sure if you overlooked that note


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52690



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

[clang-tools-extra] r343849 - [clangd] Remove last usage of ast matchers from SymbolCollector. NFC

[Sam McCall via cfe-commits]

Author: sammccall
Date: Fri Oct  5 07:03:04 2018
New Revision: 343849

URL: http://llvm.org/viewvc/llvm-project?rev=343849&view=rev
Log:
[clangd] Remove last usage of ast matchers from SymbolCollector. NFC

Modified:
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp

Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=343849&r1=343848&r2=343849&view=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Fri Oct  5 
07:03:04 2018
@@ -19,7 +19,6 @@
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Index/IndexSymbol.h"
@@ -229,10 +228,10 @@ getTokenLocation(SourceLocation TokLoc,
 // the first seen declaration as canonical declaration is not a good enough
 // heuristic.
 bool isPreferredDeclaration(const NamedDecl &ND, index::SymbolRoleSet Roles) {
-  using namespace clang::ast_matchers;
+  const auto& SM = ND.getASTContext().getSourceManager();
   return (Roles & static_cast(index::SymbolRole::Definition)) &&
  llvm::isa(&ND) &&
- match(decl(isExpansionInMainFile()), ND, ND.getASTContext()).empty();
+ !SM.isWrittenInMainFile(SM.getExpansionLoc(ND.getLocation()));
 }
 
 RefKind toRefKind(index::SymbolRoleSet Roles) {
@@ -260,7 +259,6 @@ void SymbolCollector::initialize(ASTCont
 bool SymbolCollector::shouldCollectSymbol(const NamedDecl &ND,
   ASTContext &ASTCtx,
   const Options &Opts) {
-  using namespace clang::ast_matchers;
   if (ND.isImplicit())
 return false;
   // Skip anonymous declarations, e.g (anonymous enum/class/struct).


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

[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D52771#1256511, @JonasToth wrote:

> In https://reviews.llvm.org/D52771#1256432, @aaron.ballman wrote:
>
> > I can't help but notice how badly C.133 and C.9 interact with C.131 and I'm 
> > worried we will wind up with clang-tidy checks that leave the user in an 
> > impossible situation where they need to make data members private and 
> > provide trivial accessors for them. Do you have thoughts on how to avoid 
> > that? The C++ Core Guidelines seem silent on the matter -- this might be 
> > worth raising with the authors.
>
>
> I have an idea to avoid trivial set/get methods: Flag public data if it is 
> modified internally. This could serve as a heuristic for pre/postconditions 
> on that variable. 
>  In general one could resolve all data-dependencies inside the class and 
> figure out if a private variable would need to change if a public variable 
> got changed. But that is way more complex!


Hmm, I don't know that it would help with code like this:

  class Base {
int f; // Suggested by C.133/C.9
  
  protected:
  //  int f; // Disallowed by C.133
  
int getF() const { return f; } // Suggested by C.133, disallowed by C.131
void setF(int NewF) { f = NewF; }  // Suggested by C.133, disallowed by 
C.131
  };

> Still worth asking the authors.




Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52771



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

[PATCH] D52684: [clang-tidy] NFC refactor lexer-utils slightly to be easier to use

[Alexander Kornienko via Phabricator via cfe-commits]

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

In https://reviews.llvm.org/D52684#1250885, @JonasToth wrote:

> This patch is related to https://reviews.llvm.org/D51949
>
> To isolate variable declarations (split `int * p, v;` up) it is necessary to 
> do a lot of work with source location and requires some forward and backwards 
> lexing. The functions there just use the LangOpts and the SourceManager and 
> don't have a `ASTContext` available, that why I changed this interface.


Makes sense. But then I'd change "to be easier to use" to "to be callable 
without ASTContext" or something similar in the patch description.

Otherwise LG.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52684



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

[clang-tools-extra] r343850 - [clang-tidy] NFC refactor lexer-utils to be usable without ASTContext

[Jonas Toth via cfe-commits]

Author: jonastoth
Date: Fri Oct  5 07:15:19 2018
New Revision: 343850

URL: http://llvm.org/viewvc/llvm-project?rev=343850&view=rev
Log:
[clang-tidy] NFC refactor lexer-utils to be usable without ASTContext

Summary:
This patch is a small refactoring necessary for
'readability-isolate-declaration' and does not introduce functional changes.
It allows to use the utility functions without a full `ASTContext` and requires 
only the `SourceManager` and the `LangOpts`.

Reviewers: alexfh, aaron.ballman, hokein

Reviewed By: alexfh

Subscribers: nemanjai, xazax.hun, kbarton, cfe-commits

Tags: #clang-tools-extra

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

Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
clang-tools-extra/trunk/clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.cpp
clang-tools-extra/trunk/clang-tidy/utils/LexerUtils.cpp
clang-tools-extra/trunk/clang-tidy/utils/LexerUtils.h

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp?rev=343850&r1=343849&r2=343850&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp Fri 
Oct  5 07:15:19 2018
@@ -91,8 +91,9 @@ static std::vector> Comments;
   while (Loc.isValid()) {
-clang::Token Tok =
-utils::lexer::getPreviousToken(*Ctx, Loc, /*SkipComments=*/false);
+clang::Token Tok = utils::lexer::getPreviousToken(
+Loc, Ctx->getSourceManager(), Ctx->getLangOpts(),
+/*SkipComments=*/false);
 if (Tok.isNot(tok::comment))
   break;
 Loc = Tok.getLocation();

Modified: 
clang-tools-extra/trunk/clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp?rev=343850&r1=343849&r2=343850&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp 
Fri Oct  5 07:15:19 2018
@@ -40,7 +40,8 @@ void SuspiciousSemicolonCheck::check(con
 return;
 
   ASTContext &Ctxt = *Result.Context;
-  auto Token = utils::lexer::getPreviousToken(Ctxt, LocStart);
+  auto Token = utils::lexer::getPreviousToken(LocStart, 
Ctxt.getSourceManager(),
+  Ctxt.getLangOpts());
   auto &SM = *Result.SourceManager;
   unsigned SemicolonLine = SM.getSpellingLineNumber(LocStart);
 

Modified: 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp?rev=343850&r1=343849&r2=343850&view=diff
==
--- 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp 
(original)
+++ 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp 
Fri Oct  5 07:15:19 2018
@@ -120,12 +120,14 @@ struct IntializerInsertion {
 switch (Placement) {
 case InitializerPlacement::New:
   Location = utils::lexer::getPreviousToken(
- Context, Constructor.getBody()->getBeginLoc())
+ Constructor.getBody()->getBeginLoc(),
+ Context.getSourceManager(), Context.getLangOpts())
  .getLocation();
   break;
 case InitializerPlacement::Before:
   Location = utils::lexer::getPreviousToken(
- Context, Where->getSourceRange().getBegin())
+ Where->getSourceRange().getBegin(),
+ Context.getSourceManager(), Context.getLangOpts())
  .getLocation();
   break;
 case InitializerPlacement::After:

Modified: clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.cpp?rev=343850&r1=343849&r2=343850&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.cpp Fri Oct  5 
07:15:19 2018
@@ -18,7 +18,8 @@ namespace fixit {
 
 FixItHint changeVarDeclToReference(const VarDecl &Var, ASTContext &Context) {
   SourceLocation AmpLocation = Var.getLocation();
-  auto Token = utils::lexer::getPreviousToken(Context, AmpLocation);
+  auto Token = utils::lexe

[PATCH] D52684: [clang-tidy] NFC refactor lexer-utils to be usable without ASTContext

[Jonas Toth via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL343850: [clang-tidy] NFC refactor lexer-utils to be usable 
without ASTContext (authored by JonasToth, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D52684

Files:
  clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
  clang-tools-extra/trunk/clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp
  
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
  clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.cpp
  clang-tools-extra/trunk/clang-tidy/utils/LexerUtils.cpp
  clang-tools-extra/trunk/clang-tidy/utils/LexerUtils.h

Index: clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
@@ -91,8 +91,9 @@
 getCommentsBeforeLoc(ASTContext *Ctx, SourceLocation Loc) {
   std::vector> Comments;
   while (Loc.isValid()) {
-clang::Token Tok =
-utils::lexer::getPreviousToken(*Ctx, Loc, /*SkipComments=*/false);
+clang::Token Tok = utils::lexer::getPreviousToken(
+Loc, Ctx->getSourceManager(), Ctx->getLangOpts(),
+/*SkipComments=*/false);
 if (Tok.isNot(tok::comment))
   break;
 Loc = Tok.getLocation();
Index: clang-tools-extra/trunk/clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp
@@ -40,7 +40,8 @@
 return;
 
   ASTContext &Ctxt = *Result.Context;
-  auto Token = utils::lexer::getPreviousToken(Ctxt, LocStart);
+  auto Token = utils::lexer::getPreviousToken(LocStart, Ctxt.getSourceManager(),
+  Ctxt.getLangOpts());
   auto &SM = *Result.SourceManager;
   unsigned SemicolonLine = SM.getSpellingLineNumber(LocStart);
 
Index: clang-tools-extra/trunk/clang-tidy/utils/LexerUtils.h
===
--- clang-tools-extra/trunk/clang-tidy/utils/LexerUtils.h
+++ clang-tools-extra/trunk/clang-tidy/utils/LexerUtils.h
@@ -19,8 +19,8 @@
 namespace lexer {
 
 /// Returns previous token or ``tok::unknown`` if not found.
-Token getPreviousToken(const ASTContext &Context, SourceLocation Location,
-   bool SkipComments = true);
+Token getPreviousToken(SourceLocation Location, const SourceManager &SM,
+   const LangOptions &LangOpts, bool SkipComments = true);
 
 } // namespace lexer
 } // namespace utils
Index: clang-tools-extra/trunk/clang-tidy/utils/LexerUtils.cpp
===
--- clang-tools-extra/trunk/clang-tidy/utils/LexerUtils.cpp
+++ clang-tools-extra/trunk/clang-tidy/utils/LexerUtils.cpp
@@ -14,19 +14,15 @@
 namespace utils {
 namespace lexer {
 
-Token getPreviousToken(const ASTContext &Context, SourceLocation Location,
-   bool SkipComments) {
-  const auto &SourceManager = Context.getSourceManager();
+Token getPreviousToken(SourceLocation Location, const SourceManager &SM,
+   const LangOptions &LangOpts, bool SkipComments) {
   Token Token;
   Token.setKind(tok::unknown);
   Location = Location.getLocWithOffset(-1);
-  auto StartOfFile =
-  SourceManager.getLocForStartOfFile(SourceManager.getFileID(Location));
+  auto StartOfFile = SM.getLocForStartOfFile(SM.getFileID(Location));
   while (Location != StartOfFile) {
-Location = Lexer::GetBeginningOfToken(Location, SourceManager,
-  Context.getLangOpts());
-if (!Lexer::getRawToken(Location, Token, SourceManager,
-Context.getLangOpts()) &&
+Location = Lexer::GetBeginningOfToken(Location, SM, LangOpts);
+if (!Lexer::getRawToken(Location, Token, SM, LangOpts) &&
 (!SkipComments || !Token.is(tok::comment))) {
   break;
 }
Index: clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.cpp
===
--- clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.cpp
+++ clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.cpp
@@ -18,7 +18,8 @@
 
 FixItHint changeVarDeclToReference(const VarDecl &Var, ASTContext &Context) {
   SourceLocation AmpLocation = Var.getLocation();
-  auto Token = utils::lexer::getPreviousToken(Context, AmpLocation);
+  auto Token = utils::lexer::getPreviousToken(
+  AmpLocation, Context.getSourceManager(), Context.getLangOpts());
   if (!Token.is(tok::unknown))
 AmpLocation = Lexer::getLocForEndOfToken(Token.getLocation(), 0,
  Context.getSou

[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

C.131 seems to imply a minimal amount of trivial getters/setters before
diagnosing.

I feel that the CPPCG just want to force the programmer to think twice
instead of forbidding it totally.
It might be worth to have a more chatty/specific check for the CPPCG and
a strict "Don't do it" for HICPP.

From my experience with asking the CPPCG authors it takes a while for a
response and that in the end is
not necessarily telling which way to go. Experience might differ
depending on topic though.

> Hmm, I don't know that it would help with code like this:
> 
>   class Base {
> int f; // Suggested by C.133/C.9
>   
>   protected:
>   //  int f; // Disallowed by C.133
>   
> int getF() const { return f; } // Suggested by C.133, disallowed by C.131
> void setF(int NewF) { f = NewF; }  // Suggested by C.133, disallowed by 
> C.131
>   };
> 
>> Still worth asking the authors.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52771



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

[PATCH] D52598: [OpenCL] Fixed address space cast in C style cast of C++ parsing

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 168471.
Anastasia added a comment.

Change AS checking function to be a method of  `CastOperation`.


https://reviews.llvm.org/D52598

Files:
  lib/Sema/SemaCast.cpp
  test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
  test/SemaOpenCL/address-spaces.cl

Index: test/SemaOpenCL/address-spaces.cl
===
--- test/SemaOpenCL/address-spaces.cl
+++ test/SemaOpenCL/address-spaces.cl
@@ -9,46 +9,47 @@
   __local int lj = 2; // expected-error {{'__local' variable cannot have an initializer}}
 
   int *ip;
-// FIXME: Temporarily disable part of the test that doesn't work for C++ yet.
-#if !__OPENCL_CPP_VERSION__
-#if __OPENCL_C_VERSION__ < 200
+#if ((!__OPENCL_CPP_VERSION__) && (__OPENCL_C_VERSION__ < 200))
   ip = gip; // expected-error {{assigning '__global int *' to 'int *' changes address space of pointer}}
   ip = &li; // expected-error {{assigning '__local int *' to 'int *' changes address space of pointer}}
   ip = &ci; // expected-error {{assigning '__constant int *' to 'int *' changes address space of pointer}}
 #else
   ip = gip;
   ip = &li;
-  ip = &ci; // expected-error {{assigning '__constant int *' to '__generic int *' changes address space of pointer}}
+  ip = &ci;
+#if !__OPENCL_CPP_VERSION__
+// expected-error@-2 {{assigning '__constant int *' to '__generic int *' changes address space of pointer}}
+#else
+// expected-error@-4 {{assigning to '__generic int *' from incompatible type '__constant int *'}}
+#endif
 #endif
 }
 
-void explicit_cast(global int* g, local int* l, constant int* c, private int* p, const constant int *cc)
-{
-  g = (global int*) l;// expected-error {{casting '__local int *' to type '__global int *' changes address space of pointer}}
-  g = (global int*) c;// expected-error {{casting '__constant int *' to type '__global int *' changes address space of pointer}}
-  g = (global int*) cc;   // expected-error {{casting 'const __constant int *' to type '__global int *' changes address space of pointer}}
-  g = (global int*) p;// expected-error {{casting 'int *' to type '__global int *' changes address space of pointer}}
+void explicit_cast(__global int *g, __local int *l, __constant int *c, __private int *p, const __constant int *cc) {
+  g = (__global int *)l;  // expected-error {{casting '__local int *' to type '__global int *' changes address space of pointer}}
+  g = (__global int *)c;  // expected-error {{casting '__constant int *' to type '__global int *' changes address space of pointer}}
+  g = (__global int *)cc; // expected-error {{casting 'const __constant int *' to type '__global int *' changes address space of pointer}}
+  g = (__global int *)p;  // expected-error {{casting 'int *' to type '__global int *' changes address space of pointer}}
 
-  l = (local int*) g; // expected-error {{casting '__global int *' to type '__local int *' changes address space of pointer}}
-  l = (local int*) c; // expected-error {{casting '__constant int *' to type '__local int *' changes address space of pointer}}
-  l = (local int*) cc;// expected-error {{casting 'const __constant int *' to type '__local int *' changes address space of pointer}}
-  l = (local int*) p; // expected-error {{casting 'int *' to type '__local int *' changes address space of pointer}}
+  l = (__local int *)g;  // expected-error {{casting '__global int *' to type '__local int *' changes address space of pointer}}
+  l = (__local int *)c;  // expected-error {{casting '__constant int *' to type '__local int *' changes address space of pointer}}
+  l = (__local int *)cc; // expected-error {{casting 'const __constant int *' to type '__local int *' changes address space of pointer}}
+  l = (__local int *)p;  // expected-error {{casting 'int *' to type '__local int *' changes address space of pointer}}
 
-  c = (constant int*) g;  // expected-error {{casting '__global int *' to type '__constant int *' changes address space of pointer}}
-  c = (constant int*) l;  // expected-error {{casting '__local int *' to type '__constant int *' changes address space of pointer}}
-  c = (constant int*) p;  // expected-error {{casting 'int *' to type '__constant int *' changes address space of pointer}}
+  c = (__constant int *)g; // expected-error {{casting '__global int *' to type '__constant int *' changes address space of pointer}}
+  c = (__constant int *)l; // expected-error {{casting '__local int *' to type '__constant int *' changes address space of pointer}}
+  c = (__constant int *)p; // expected-error {{casting 'int *' to type '__constant int *' changes address space of pointer}}
 
-  p = (private int*) g;   // expected-error {{casting '__global int *' to type 'int *' changes address space of pointer}}
-  p = (private int*) l;   // expected-error {{casting '__local int *' to type 'int *' changes address space of pointer}}
-  p = (private int*) c;   // expected-error {{casting '__constant int *' 

[PATCH] D49244: Always search sysroot for GCC installs

[David Greene via Phabricator via cfe-commits]

greened updated this revision to Diff 168472.
greened added a comment.

Updated to implement option 2.  I'm not totally happy with passing a StringRef 
just to check if it's non-empty but opted to reduce the size of the diff rather 
than refactor a bunch of stuff.


Repository:
  rC Clang

https://reviews.llvm.org/D49244

Files:
  lib/Driver/ToolChains/Gnu.cpp


Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -1618,10 +1618,18 @@
   return GoodVersion;
 }
 
-static llvm::StringRef getGCCToolchainDir(const ArgList &Args) {
+static llvm::StringRef getGCCToolchainDir(const ArgList &Args,
+  llvm::StringRef SysRoot) {
   const Arg *A = Args.getLastArg(clang::driver::options::OPT_gcc_toolchain);
   if (A)
 return A->getValue();
+
+  // If we have a SysRoot, ignore GCC_INSTALL_PREFIX.
+  // GCC_INSTALL_PREFIX specifies the gcc installation for the default
+  // sysroot and is likely not valid with a different sysroot.
+  if (!SysRoot.empty())
+return "";
+
   return GCC_INSTALL_PREFIX;
 }
 
@@ -1653,7 +1661,7 @@
   SmallVector Prefixes(D.PrefixDirs.begin(),
D.PrefixDirs.end());
 
-  StringRef GCCToolchainDir = getGCCToolchainDir(Args);
+  StringRef GCCToolchainDir = getGCCToolchainDir(Args, D.SysRoot);
   if (GCCToolchainDir != "") {
 if (GCCToolchainDir.back() == '/')
   GCCToolchainDir = GCCToolchainDir.drop_back(); // remove the /


Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -1618,10 +1618,18 @@
   return GoodVersion;
 }
 
-static llvm::StringRef getGCCToolchainDir(const ArgList &Args) {
+static llvm::StringRef getGCCToolchainDir(const ArgList &Args,
+  llvm::StringRef SysRoot) {
   const Arg *A = Args.getLastArg(clang::driver::options::OPT_gcc_toolchain);
   if (A)
 return A->getValue();
+
+  // If we have a SysRoot, ignore GCC_INSTALL_PREFIX.
+  // GCC_INSTALL_PREFIX specifies the gcc installation for the default
+  // sysroot and is likely not valid with a different sysroot.
+  if (!SysRoot.empty())
+return "";
+
   return GCC_INSTALL_PREFIX;
 }
 
@@ -1653,7 +1661,7 @@
   SmallVector Prefixes(D.PrefixDirs.begin(),
D.PrefixDirs.end());
 
-  StringRef GCCToolchainDir = getGCCToolchainDir(Args);
+  StringRef GCCToolchainDir = getGCCToolchainDir(Args, D.SysRoot);
   if (GCCToolchainDir != "") {
 if (GCCToolchainDir.back() == '/')
   GCCToolchainDir = GCCToolchainDir.drop_back(); // remove the /
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52598: [OpenCL] Fixed address space cast in C style cast of C++ parsing

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: lib/Sema/SemaCast.cpp:2288
+  SrcType->isPointerType()) {
+const PointerType *DestPtr = DestType->getAs();
+if (!DestPtr->isAddressSpaceOverlapping(*SrcType->getAs())) {

Anastasia wrote:
> rjmccall wrote:
> > Please test the result of `getAs` instead of separately testing 
> > `isPointerType`.
> > 
> > Why is this check OpenCL-specific?  Address spaces are a general language 
> > feature.
> I think mainly because I am factoring out from the existing OpenCL check. But 
> it probably makes sense that the semantics of this is not different in other 
> languages. I will update it! Thanks!
After playing with this for a bit longer I discovered that I have to keep the 
OpenCL check unfortunately.

I found this old commit (`d4c5f84`/`r129583`) that says:
  C-style casts can add/remove/change address spaces through the 
reinterpret_cast mechanism.
That's not the same as in OpenCL because it seems for C++ you can cast any AS 
to any other AS. Therefore, no checks are needed at all. I am not sure if we 
can come up with a common function for the moment.

The following  tests are checking this:
   CodeGenCXX/address-space-cast.cpp
   SemaCXX/address-space-conversion.cpp

Do you think it would make sense to rename this method with OpenCL-something or 
keep in case may be CUDA or some other languages might need similar 
functionality...



https://reviews.llvm.org/D52598



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

[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In https://reviews.llvm.org/D52918#1256420, @aaron.ballman wrote:

> Patch is missing tests -- perhaps you could dump the AST and check the 
> casting notation from the output?


It would appear that which casts get emitted in C mode is almost completely 
untested. I added tests just for this change in a new file, and left a TODO to 
fill it out for the remainder of those functions.


https://reviews.llvm.org/D52918



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

[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.

[James Y Knight via Phabricator via cfe-commits]

jyknight updated this revision to Diff 168477.
jyknight added a comment.

Added test.


https://reviews.llvm.org/D52918

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/c-casts.c


Index: clang/test/Sema/c-casts.c
===
--- /dev/null
+++ clang/test/Sema/c-casts.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -w -ast-dump %s | FileCheck %s
+
+// The cast construction code both for implicit and c-style casts is very
+// different in C vs C++. This file is intended to test the C behavior.
+
+// TODO: add tests covering the rest of the code in
+// Sema::CheckAssignmentConstraints and Sema::PrepareScalarCast
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_cvr_pointer
+void cast_cvr_pointer(char volatile * __restrict * const * p) {
+  char*** x;
+  // CHECK: ImplicitCastExpr {{.*}} 'char ***' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'char ***' 
+  x = (char***)p;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_pointer_type
+void cast_pointer_type(char *p) {
+  void *x;
+  // CHECK: ImplicitCastExpr {{.*}} 'void *' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'void *' 
+  x = (void*)p;
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -5862,6 +5862,8 @@
   LangAS DestAS = DestTy->getPointeeType().getAddressSpace();
   if (SrcAS != DestAS)
 return CK_AddressSpaceConversion;
+  if (Context.hasCvrSimilarType(SrcTy, DestTy))
+return CK_NoOp;
   return CK_BitCast;
 }
 case Type::STK_BlockPointer:
@@ -7762,7 +7764,12 @@
 if (isa(RHSType)) {
   LangAS AddrSpaceL = LHSPointer->getPointeeType().getAddressSpace();
   LangAS AddrSpaceR = RHSType->getPointeeType().getAddressSpace();
-  Kind = AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion : CK_BitCast;
+  if (AddrSpaceL != AddrSpaceR)
+Kind = CK_AddressSpaceConversion;
+  else if (Context.hasCvrSimilarType(RHSType, LHSType))
+Kind = CK_NoOp;
+  else
+Kind = CK_BitCast;
   return checkPointerTypesForAssignment(*this, LHSType, RHSType);
 }
 
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -5861,11 +5861,7 @@
 // permitted in constant expressions in C++11. Bitcasts from cv void* are
 // also static_casts, but we disallow them as a resolution to DR1312.
 if (!E->getType()->isVoidPointerType()) {
-  // If we changed anything other than cvr-qualifiers, we can't use this
-  // value for constant folding. FIXME: Qualification conversions should
-  // always be CK_NoOp, but we get this wrong in C.
-  if (!Info.Ctx.hasCvrSimilarType(E->getType(), 
E->getSubExpr()->getType()))
-Result.Designator.setInvalid();
+  Result.Designator.setInvalid();
   if (SubExpr->getType()->isVoidPointerType())
 CCEDiag(E, diag::note_constexpr_invalid_cast)
   << 3 << SubExpr->getType();


Index: clang/test/Sema/c-casts.c
===
--- /dev/null
+++ clang/test/Sema/c-casts.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -w -ast-dump %s | FileCheck %s
+
+// The cast construction code both for implicit and c-style casts is very
+// different in C vs C++. This file is intended to test the C behavior.
+
+// TODO: add tests covering the rest of the code in
+// Sema::CheckAssignmentConstraints and Sema::PrepareScalarCast
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_cvr_pointer
+void cast_cvr_pointer(char volatile * __restrict * const * p) {
+  char*** x;
+  // CHECK: ImplicitCastExpr {{.*}} 'char ***' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'char ***' 
+  x = (char***)p;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_pointer_type
+void cast_pointer_type(char *p) {
+  void *x;
+  // CHECK: ImplicitCastExpr {{.*}} 'void *' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'void *' 
+  x = (void*)p;
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -5862,6 +5862,8 @@
   LangAS DestAS = DestTy->getPointeeType().getAddressSpace();
   if (SrcAS != DestAS)
 return CK_AddressSpaceConversion;
+  if (Context.hasCvrSimilarType(SrcTy, DestTy))
+return CK_NoOp;
   return CK_BitCast;
 }
 case Type::STK_BlockPointer:
@@ -7762,7 +7764,12 @@
 if (isa(RHSType)) {
   LangAS AddrSpaceL = LHSPointer->getPointeeType().getAddressSpace();
   LangAS AddrSpaceR = RHSType->getPointeeType().getAddressSpace();
-  Kind = AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion : CK_BitCast;
+  if (AddrSpaceL != AddrSpaceR)
+Kind = CK_AddressSpaceConversion;
+  else if (Context.hasCvrSimilarType(RHSTyp

r343856 - [OPENMP] Fix emission of the __kmpc_global_thread_num.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Fri Oct  5 08:08:53 2018
New Revision: 343856

URL: http://llvm.org/viewvc/llvm-project?rev=343856&view=rev
Log:
[OPENMP] Fix emission of the __kmpc_global_thread_num.

Fixed emission of the __kmpc_global_thread_num() so that it is not
messed up with alloca instructions anymore. Plus, fixes emission of the
__kmpc_global_thread_num() functions in the target outlined regions so
that they are not called before runtime is initialized.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp

cfe/trunk/test/OpenMP/nvptx_target_teams_distribute_parallel_for_generic_mode_codegen.cpp
cfe/trunk/test/OpenMP/parallel_if_codegen.cpp
cfe/trunk/test/OpenMP/single_codegen.cpp
cfe/trunk/test/OpenMP/single_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/taskgroup_task_reduction_codegen.cpp
cfe/trunk/test/OpenMP/taskloop_reduction_codegen.cpp
cfe/trunk/test/OpenMP/taskloop_simd_reduction_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=343856&r1=343855&r2=343856&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Fri Oct  5 08:08:53 2018
@@ -1485,6 +1485,31 @@ Address CGOpenMPRuntime::getOrCreateDefa
   return Address(Entry, Align);
 }
 
+void CGOpenMPRuntime::setLocThreadIdInsertPt(CodeGenFunction &CGF,
+ bool AtCurrentPoint) {
+  auto &Elem = OpenMPLocThreadIDMap.FindAndConstruct(CGF.CurFn);
+  assert(!Elem.second.ServiceInsertPt && "Insert point is set already.");
+
+  llvm::Value *Undef = llvm::UndefValue::get(CGF.Int32Ty);
+  if (AtCurrentPoint) {
+Elem.second.ServiceInsertPt = new llvm::BitCastInst(
+Undef, CGF.Int32Ty, "svcpt", CGF.Builder.GetInsertBlock());
+  } else {
+Elem.second.ServiceInsertPt =
+new llvm::BitCastInst(Undef, CGF.Int32Ty, "svcpt");
+Elem.second.ServiceInsertPt->insertAfter(CGF.AllocaInsertPt);
+  }
+}
+
+void CGOpenMPRuntime::clearLocThreadIdInsertPt(CodeGenFunction &CGF) {
+  auto &Elem = OpenMPLocThreadIDMap.FindAndConstruct(CGF.CurFn);
+  if (Elem.second.ServiceInsertPt) {
+llvm::Instruction *Ptr = Elem.second.ServiceInsertPt;
+Elem.second.ServiceInsertPt = nullptr;
+Ptr->eraseFromParent();
+  }
+}
+
 llvm::Value *CGOpenMPRuntime::emitUpdateLocation(CodeGenFunction &CGF,
  SourceLocation Loc,
  unsigned Flags) {
@@ -1511,8 +1536,10 @@ llvm::Value *CGOpenMPRuntime::emitUpdate
 Elem.second.DebugLoc = AI.getPointer();
 LocValue = AI;
 
+if (!Elem.second.ServiceInsertPt)
+  setLocThreadIdInsertPt(CGF);
 CGBuilderTy::InsertPointGuard IPG(CGF.Builder);
-CGF.Builder.SetInsertPoint(CGF.AllocaInsertPt);
+CGF.Builder.SetInsertPoint(Elem.second.ServiceInsertPt);
 CGF.Builder.CreateMemCpy(LocValue, getOrCreateDefaultLocation(Flags),
  CGF.getTypeSize(IdentQTy));
   }
@@ -1582,21 +1609,25 @@ llvm::Value *CGOpenMPRuntime::getThreadI
   // kmpc_global_thread_num(ident_t *loc).
   // Generate thread id value and cache this value for use across the
   // function.
+  auto &Elem = OpenMPLocThreadIDMap.FindAndConstruct(CGF.CurFn);
+  if (!Elem.second.ServiceInsertPt)
+setLocThreadIdInsertPt(CGF);
   CGBuilderTy::InsertPointGuard IPG(CGF.Builder);
-  CGF.Builder.SetInsertPoint(CGF.AllocaInsertPt);
+  CGF.Builder.SetInsertPoint(Elem.second.ServiceInsertPt);
   llvm::CallInst *Call = CGF.Builder.CreateCall(
   createRuntimeFunction(OMPRTL__kmpc_global_thread_num),
   emitUpdateLocation(CGF, Loc));
   Call->setCallingConv(CGF.getRuntimeCC());
-  auto &Elem = OpenMPLocThreadIDMap.FindAndConstruct(CGF.CurFn);
   Elem.second.ThreadID = Call;
   return Call;
 }
 
 void CGOpenMPRuntime::functionFinished(CodeGenFunction &CGF) {
   assert(CGF.CurFn && "No function in current CodeGenFunction.");
-  if (OpenMPLocThreadIDMap.count(CGF.CurFn))
+  if (OpenMPLocThreadIDMap.count(CGF.CurFn)) {
+clearLocThreadIdInsertPt(CGF);
 OpenMPLocThreadIDMap.erase(CGF.CurFn);
+  }
   if (FunctionUDRMap.count(CGF.CurFn) > 0) {
 for(auto *D : FunctionUDRMap[CGF.CurFn])
   UDRMap.erase(D);

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h?rev=343856&r1=343855&r2=343856&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h Fri Oct  5 08:08:53 2018
@@ -278,6 +278,10 @@ protected:
   /// stored.
   virtual Address emitThreadIDAddress(CodeGenFunction &CGF, SourceLocation 
Loc);
 
+  void setL

r343857 - [OPENMP][NVPTX] Fix emission of __kmpc_global_thread_num() for non-SPMD

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Fri Oct  5 08:27:47 2018
New Revision: 343857

URL: http://llvm.org/viewvc/llvm-project?rev=343857&view=rev
Log:
[OPENMP][NVPTX] Fix emission of __kmpc_global_thread_num() for non-SPMD
mode.

__kmpc_global_thread_num() should be called before initialization of the
runtime.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/test/OpenMP/nvptx_teams_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp?rev=343857&r1=343856&r2=343857&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Fri Oct  5 08:27:47 2018
@@ -1083,12 +1083,15 @@ void CGOpenMPRuntimeNVPTX::emitNonSPMDKe
  CGOpenMPRuntimeNVPTX::WorkerFunctionState &WST)
 : EST(EST), WST(WST) {}
 void Enter(CodeGenFunction &CGF) override {
-  static_cast(CGF.CGM.getOpenMPRuntime())
-  .emitNonSPMDEntryHeader(CGF, EST, WST);
+  auto &RT = static_cast(CGF.CGM.getOpenMPRuntime());
+  RT.emitNonSPMDEntryHeader(CGF, EST, WST);
+  // Skip target region initialization.
+  RT.setLocThreadIdInsertPt(CGF, /*AtCurrentPoint=*/true);
 }
 void Exit(CodeGenFunction &CGF) override {
-  static_cast(CGF.CGM.getOpenMPRuntime())
-  .emitNonSPMDEntryFooter(CGF, EST);
+  auto &RT = static_cast(CGF.CGM.getOpenMPRuntime());
+  RT.clearLocThreadIdInsertPt(CGF);
+  RT.emitNonSPMDEntryFooter(CGF, EST);
 }
   } Action(EST, WST);
   CodeGen.setAction(Action);

Modified: cfe/trunk/test/OpenMP/nvptx_teams_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/nvptx_teams_codegen.cpp?rev=343857&r1=343856&r2=343857&view=diff
==
--- cfe/trunk/test/OpenMP/nvptx_teams_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/nvptx_teams_codegen.cpp Fri Oct  5 08:27:47 2018
@@ -105,7 +105,6 @@ int main (int argc, char **argv) {
 // CK2: [[AADDR:%.+]] = alloca i{{[0-9]+}},
 // CK2: [[BADDR:%.+]] = alloca i{{[0-9]+}},
 // CK2: [[ARGCADDR:%.+]] = alloca i{{[0-9]+}},
-// CK2:  {{%.+}} = call i32 @__kmpc_global_thread_num(
 // CK2: store i{{[0-9]+}} [[A_IN]], i{{[0-9]+}}* [[AADDR]],
 // CK2: store i{{[0-9]+}} [[B_IN]], i{{[0-9]+}}* [[BADDR]],
 // CK2: store i{{[0-9]+}} [[ARGC_IN]], i{{[0-9]+}}* [[ARGCADDR]],
@@ -117,6 +116,7 @@ int main (int argc, char **argv) {
 // CK2-32:  [[ARG:%.+]] = load i{{[0-9]+}}, i{{[0-9]+}}* [[ARGCADDR]]
 // CK2:  [[ARGCADDR:%.+]] = getelementptr inbounds %struct.{{.*}}, 
%struct.{{.*}}* %{{.*}}, i{{[0-9]+}} 0, i{{[0-9]+}} 0
 // CK2:  store i{{[0-9]+}} [[ARG]], i{{[0-9]+}}* [[ARGCADDR]],
+// CK2:  {{%.+}} = call i32 @__kmpc_global_thread_num(
 // CK2:  store i{{[0-9]+}}* [[ARGCADDR]], i{{[0-9]+}}** [[ARGCADDR_PTR]],
 // CK2:  [[ARGCADDR_PTR_REF:%.+]] = load i{{[0-9]+}}*, i{{[0-9]+}}** 
[[ARGCADDR_PTR]],
 // CK2: store i{{[0-9]+}} 0, i{{[0-9]+}}* [[ARGCADDR_PTR_REF]],
@@ -129,7 +129,6 @@ int main (int argc, char **argv) {
 // CK2: [[AADDR:%.+]] = alloca i{{[0-9]+}},
 // CK2: [[BADDR:%.+]] = alloca i{{[0-9]+}},
 // CK2: [[ARGCADDR:%.+]] = alloca i{{[0-9]+}}**,
-// CK2: {{%.+}} = call i32 @__kmpc_global_thread_num(
 // CK2: store i{{[0-9]+}} [[A_IN]], i{{[0-9]+}}* [[AADDR]],
 // CK2: store i{{[0-9]+}} [[B_IN]], i{{[0-9]+}}* [[BADDR]],
 // CK2: store i{{[0-9]+}}** [[ARGC]], i{{[0-9]+}}*** [[ARGCADDR]],
@@ -137,6 +136,7 @@ int main (int argc, char **argv) {
 // CK2: [[ARG:%.+]] = load i{{[0-9]+}}**, i{{[0-9]+}}*** [[ARGCADDR]]
 // CK2: [[ARGCADDR:%.+]] = getelementptr inbounds %struct.{{.*}}, 
%struct.{{.*}}* %{{.*}}, i{{[0-9]+}} 0, i{{[0-9]+}} 0
 // CK2: store i{{[0-9]+}}** [[ARG]], i{{[0-9]+}}*** [[ARGCADDR]],
+// CK2: {{%.+}} = call i32 @__kmpc_global_thread_num(
 // CK2: store i{{[0-9]+}}*** [[ARGCADDR]], i{{[0-9]+}} [[ARGCADDR_PTR]],
 // CK2: [[ARGCADDR_PTR_REF:%.+]] = load i{{[0-9]+}}***, i{{[0-9]+}} 
[[ARGCADDR_PTR]],
 // CK2: store i{{[0-9]+}}** null, i{{[0-9]+}}*** [[ARGCADDR_PTR_REF]],


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

[PATCH] D52658: [OpenCL] Remove PIPE_RESERVE_ID_VALID_BIT from opencl-c.h

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rC Clang

https://reviews.llvm.org/D52658



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

[PATCH] D52873: Remove unwanted signedness conversion from tests

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rC Clang

https://reviews.llvm.org/D52873



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

[PATCH] D52879: Derive builtin return type from its definition

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

LGTM from OpenCL side!


Repository:
  rC Clang

https://reviews.llvm.org/D52879



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

[PATCH] D52800: Java import sorting in clang-format

[Krasimir Georgiev via Phabricator via cfe-commits]

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

This gooks great! Thanks for the contribution!
If you don't have commit access to Clang I can commit this for you.


Repository:
  rC Clang

https://reviews.llvm.org/D52800



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

Re: r343790 - [clang] Add the exclude_from_explicit_instantiation attribute

[Louis Dionne via cfe-commits]

I just saw this. Simon Pilgrim already fixed it in r343846. Thanks Simon!

Louis

> On Oct 4, 2018, at 22:02, Galina Kistanova  wrote:
> 
> Hello Louis,
> 
> This commit broke build step on one of our builders:
> http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/13042
>  
> 
> 
> . . .
> FAILED: tools/clang/lib/Sema/CMakeFiles/clangSema.dir/SemaExprCXX.cpp.obj 
> C:\PROGRA~2\MICROS~1.0\VC\bin\amd64\cl.exe  /nologo /TP -DEXPENSIVE_CHECKS 
> -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE 
> -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE 
> -D_CRT_SECURE_NO_WARNINGS -D_GLIBCXX_DEBUG -D_GNU_SOURCE -D_HAS_EXCEPTIONS=0 
> -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE 
> -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
> -Itools\clang\lib\Sema 
> -IC:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\lib\Sema
>  
> -IC:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\include
>  -Itools\clang\include -Iinclude 
> -IC:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\include 
> /DWIN32 /D_WINDOWS   /Zc:inline /Zc:strictStrings /Oi /Zc:rvalueCast /W4 
> -wd4141 -wd4146 -wd4180 -wd4244 -wd4258 -wd4267 -wd4291 -wd4345 -wd4351 
> -wd4355 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4800 
> -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 
> -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 
> -wd4592 -wd4319 -wd4324 -w14062 -we4238 /MDd /Zi /Ob0 /Od /RTC1/EHs-c- 
> /GR- /showIncludes 
> /Fotools\clang\lib\Sema\CMakeFiles\clangSema.dir\SemaExprCXX.cpp.obj 
> /Fdtools\clang\lib\Sema\CMakeFiles\clangSema.dir\clangSema.pdb /FS -c 
> C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\lib\Sema\SemaExprCXX.cpp
> C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\lib\Sema\SemaExprCXX.cpp
>  : fatal error C1128: number of sections exceeded object file format limit: 
> compile with /bigobj
> 
> 
> Please have a look?
> The builder was already red and did not send notifications on this.
> 
> Thanks
> 
> Galina
> 
> On Thu, Oct 4, 2018 at 8:51 AM Louis Dionne via cfe-commits 
> mailto:cfe-commits@lists.llvm.org>> wrote:
> Author: ldionne
> Date: Thu Oct  4 08:49:42 2018
> New Revision: 343790
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=343790&view=rev 
> 
> Log:
> [clang] Add the exclude_from_explicit_instantiation attribute
> 
> Summary:
> This attribute allows excluding a member of a class template from being part
> of an explicit template instantiation of that class template. This also makes
> sure that code using such a member will not take for granted that an external
> instantiation exists in another translation unit. The attribute was discussed
> on cfe-dev at [1] and is primarily motivated by the removal of always_inline
> in libc++ to control what's part of the ABI (see links in [1]).
> 
> [1]: http://lists.llvm.org/pipermail/cfe-dev/2018-August/059024.html 
> 
> 
> rdar://problem/43428125
> 
> Reviewers: rsmith
> 
> Subscribers: dexonsmith, cfe-commits
> 
> Differential Revision: https://reviews.llvm.org/D51789 
> 
> 
> Added:
> 
> cfe/trunk/test/CodeGenCXX/attr-exclude_from_explicit_instantiation.dont_assume_extern_instantiation.cpp
> 
> cfe/trunk/test/SemaCXX/attr-exclude_from_explicit_instantiation.diagnose_on_undefined_entity.cpp
> 
> cfe/trunk/test/SemaCXX/attr-exclude_from_explicit_instantiation.explicit_instantiation.cpp
> 
> cfe/trunk/test/SemaCXX/attr-exclude_from_explicit_instantiation.extern_declaration.cpp
> 
> cfe/trunk/test/SemaCXX/attr-exclude_from_explicit_instantiation.merge_redeclarations.cpp
> Modified:
> cfe/trunk/include/clang/Basic/Attr.td
> cfe/trunk/include/clang/Basic/AttrDocs.td
> cfe/trunk/lib/Sema/Sema.cpp
> cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
> cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
> 
> Modified: cfe/trunk/include/clang/Basic/Attr.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=343790&r1=343789&r2=343790&view=diff
>  
> 
> ==
> --- cfe/trunk/include/clang/Basic/Attr.td (original)
> +++ cfe/trunk/include/clang/Basic/Attr.td Thu Oct  4 08:49:42 2018
> @@ -3042,6 +3042,13 @@ def InternalLinkage : InheritableAttr {
>let Documentation = [InternalLinkageDocs];
>  }
> 
> +def ExcludeFromExplicitInstantiation : InheritableAttr {
> +  let Spellings = [C

[PATCH] D52888: Thread safety analysis: Handle conditional expression in getTrylockCallExpr

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

In https://reviews.llvm.org/D52888#1256395, @aaron.ballman wrote:

> In https://reviews.llvm.org/D52888#1255862, @aaronpuchert wrote:
>
> > Additional changes (including some non-tail recursion unfortunately) would 
> > allow the following to work:
> >
> >   void foo16() {
> > if (cond ? mu.TryLock() : false)
> >   mu.Unlock();
> >   }
> >   
> >   void foo17() {
> > if (cond ? true : !mu.TryLock())
> >   return;
> > mu.Unlock();
> >   }
> >   
> >
> > Worth the effort, or is that too exotic?
>
>
> `foo16()` looks like code I could plausibly see in the wild. Consider the 
> case where the mutex is a pointer and you want to test it for null before 
> calling `TryLock()` on it (`M ? M->TryLock() : false`). However, I don't have 
> a good feeling for how much work it would be to support that case.


It's relatively short, but not exactly straightforward, because we have to 
check if both branches are "compatible". However, thinking about this again I 
think most programmers would write `foo16` as

  void foo16() {
if (cond && mu.TryLock())
  mu.Unlock();
  }

which is functionally equivalent, and which we already support. So I'd propose 
to add support for this only when someone asks for it, and leave it as it is 
for now.


Repository:
  rC Clang

https://reviews.llvm.org/D52888



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

[PATCH] D51809: [CUDA][HIP] Fix ShouldDeleteSpecialMember for inherited constructors

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 168479.
yaxunl retitled this revision from "[CUDA][HIP] Fix assertion in 
LookupSpecialMember" to "[CUDA][HIP] Fix ShouldDeleteSpecialMember for 
inherited constructors".
yaxunl edited the summary of this revision.
yaxunl added a comment.

Revised by Justin's comments.

Handle the situation where an inherited constructor is faked to be a default 
constructor but it is really not.


https://reviews.llvm.org/D51809

Files:
  lib/Sema/SemaDeclCXX.cpp
  test/SemaCUDA/implicit-member-target-inherited.cu
  test/SemaCUDA/inherited-ctor.cu

Index: test/SemaCUDA/inherited-ctor.cu
===
--- /dev/null
+++ test/SemaCUDA/inherited-ctor.cu
@@ -0,0 +1,89 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+
+// Inherit a valid non-default ctor.
+namespace NonDefaultCtorValid {
+  struct A {
+A(const int &x) {}
+  };
+
+  struct B : A {
+using A::A;
+  };
+
+  struct C {
+struct B b;
+C() : b(0) {}
+  };
+
+  void test() {
+B b(0);
+C c;
+  }
+}
+
+// Inherit an invalid non-default ctor.
+// The inherited ctor is invalid because it is unable to initialize s.
+namespace NonDefaultCtorInvalid {
+  struct S {
+S() = delete;
+  };
+  struct A {
+A(const int &x) {}
+  };
+
+  struct B : A {
+using A::A;
+S s;
+  };
+
+  struct C {
+struct B b;
+C() : b(0) {} // expected-error{{constructor inherited by 'B' from base class 'A' is implicitly deleted}}
+  // expected-note@-6{{constructor inherited by 'B' is implicitly deleted because field 's' has a deleted corresponding constructor}}
+  // expected-note@-15{{'S' has been explicitly marked deleted here}}
+  };
+}
+
+// Inherit a valid default ctor.
+namespace DefaultCtorValid {
+  struct A {
+A() {}
+  };
+
+  struct B : A {
+using A::A;
+  };
+
+  struct C {
+struct B b;
+C() {}
+  };
+
+  void test() {
+B b;
+C c;
+  }
+}
+
+// Inherit an invalid default ctor.
+// The inherited ctor is invalid because it is unable to initialize s.
+namespace DefaultCtorInvalid {
+  struct S {
+S() = delete;
+  };
+  struct A {
+A() {}
+  };
+
+  struct B : A {
+using A::A;
+S s;
+  };
+
+  struct C {
+struct B b;
+C() {} // expected-error{{call to implicitly-deleted default constructor of 'struct B'}}
+   // expected-note@-6{{default constructor of 'B' is implicitly deleted because field 's' has a deleted default constructor}}
+   // expected-note@-15{{'S' has been explicitly marked deleted here}}
+  };
+}
Index: test/SemaCUDA/implicit-member-target-inherited.cu
===
--- /dev/null
+++ test/SemaCUDA/implicit-member-target-inherited.cu
@@ -0,0 +1,205 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s -Wno-defaulted-function-deleted
+
+#include "Inputs/cuda.h"
+
+//--
+// Test 1: infer inherited default ctor to be host.
+
+struct A1_with_host_ctor {
+  A1_with_host_ctor() {}
+};
+// expected-note@-3 {{candidate constructor (the implicit copy constructor) not viable}}
+// expected-note@-4 {{candidate constructor (the implicit move constructor) not viable}}
+
+// The inherited default constructor is inferred to be host, so we'll encounter
+// an error when calling it from a __device__ function, but not from a __host__
+// function.
+struct B1_with_implicit_default_ctor : A1_with_host_ctor {
+  using A1_with_host_ctor::A1_with_host_ctor;
+};
+
+// expected-note@-4 {{call to __host__ function from __device__}}
+// expected-note@-5 {{candidate constructor (the implicit copy constructor) not viable}}
+// expected-note@-6 {{candidate constructor (the implicit move constructor) not viable}}
+// expected-note@-6 2{{constructor from base class 'A1_with_host_ctor' inherited here}}
+
+void hostfoo() {
+  B1_with_implicit_default_ctor b;
+}
+
+__device__ void devicefoo() {
+  B1_with_implicit_default_ctor b; // expected-error {{no matching constructor}}
+}
+
+//--
+// Test 2: infer inherited default ctor to be device.
+
+struct A2_with_device_ctor {
+  __device__ A2_with_device_ctor() {}
+};
+// expected-note@-3 {{candidate constructor (the implicit copy constructor) not viable}}
+// expected-note@-4 {{candidate constructor (the implicit move constructor) not viable}}
+
+struct B2_with_implicit_default_ctor : A2_with_device_ctor {
+  using A2_with_device_ctor::A2_with_device_ctor;
+};
+
+// expected-note@-4 {{call to __device__ function from __host__}}
+// expected-note@-5 {{candidate constructor (the implicit copy constructor) not viable}}
+// expected-note@-6 {{candidate constructor (the implicit move constructor) not viable}}
+// expected-note@-6 2{{constructor from base class 'A2_with_device_ctor' inherited here}}
+
+void hostfoo2() {
+  B2_wi

[PATCH] D52936: Add some automatic tests for DivideZero checker

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas created this revision.
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D52936

Files:
  test/Analysis/div-zero.cpp

Index: test/Analysis/div-zero.cpp
===
--- test/Analysis/div-zero.cpp
+++ test/Analysis/div-zero.cpp
@@ -11,3 +11,301 @@
   return (a % (qX-1)); // expected-warning {{Division by zero}}
 
 }
+
+
+void testDiv1() {
+  (void)(42 / 0);
+  // expected-warning@-1 {{division by zero is undefined}}
+  // expected-warning@-2 {{Division by zero}}
+}
+
+
+void testDiv2() {
+  (void)(42 / false);
+  // expected-warning@-1 {{division by zero is undefined}}
+  // expected-warning@-2 {{Division by zero}}
+}
+
+
+void testDiv3() {
+  (void)(42 / !1);
+  // expected-warning@-1 {{division by zero is undefined}}
+  // expected-warning@-2 {{Division by zero}}
+}
+
+
+void testDiv4() {
+  (void)(42 / (1 - 1));
+  // expected-warning@-1 {{division by zero is undefined}}
+  // expected-warning@-2 {{Division by zero}}
+}
+
+
+void testDiv5() {
+  (void)(42 / !(1 + 1));
+  // expected-warning@-1 {{division by zero is undefined}}
+  // expected-warning@-2 {{Division by zero}}
+}
+
+
+void testDiv6a() {
+  (void)(42 / (int)0.0);
+  // expected-warning@-1 {{division by zero is undefined}}
+  // Missing Division by zero warning
+}
+
+
+void testDiv6b() {
+  int x = (int)0.0;
+  (void)(42 / x);
+  // Missing Division by zero warning
+}
+
+
+void testDiv6c() {
+  float y = 0.2;
+  float z = 0.1;
+  int x = (int)(z + y);
+  (void)(42 / x);
+  // Missing Division by zero warning
+}
+
+
+void testDiv7() {
+  (void)(true / 0);
+  // expected-warning@-1 {{division by zero is undefined}}
+  // expected-warning@-2 {{Division by zero}}
+}
+
+
+void testDiv8() {
+  (void)(!1 / 0);
+  // expected-warning@-1 {{division by zero is undefined}}
+  // expected-warning@-2 {{Division by zero}}
+}
+
+
+void testDiv9() {
+  (void)((int)(9.0) / 0);
+  // expected-warning@-1 {{division by zero is undefined}}
+  // expected-warning@-2 {{Division by zero}}
+}
+
+
+void testDiv10() {
+  (void)((10 - 1) / 0);
+  // expected-warning@-1 {{division by zero is undefined}}
+  // expected-warning@-2 {{Division by zero}}
+}
+
+
+void testRem() {
+  (void)(42 % 0);
+  // expected-warning@-1 {{remainder by zero is undefined}}
+  // expected-warning@-2 {{Division by zero}}
+}
+
+
+int testDivAssign(int x) {
+  return (x /= 0);
+  // expected-warning@-1 {{division by zero is undefined}}
+  // expected-warning@-2 {{Division by zero}}
+}
+
+
+int testRemAssign(int x) {
+  return (x %= 0);
+  // expected-warning@-1 {{remainder by zero is undefined}}
+  // expected-warning@-2 {{Division by zero}}
+}
+
+
+void testFloatDiv() {
+  (void)(42.0 / 0);
+  // No warning, the checker handles scalar types only
+}
+
+
+void testDivPath() {
+  int x = 2;
+  int y = 2;
+  int z = 10;
+
+  x = y / z;
+  (void)(42 / x); // expected-warning {{Division by zero}}
+}
+
+
+void testDivPath2() {
+  int x = 2;
+  int y = 0;
+  int z = 10;
+
+  x = y / z;
+  (void)(42 / x); // expected-warning {{Division by zero}}
+}
+
+
+void testRemPath() {
+  int x = 2;
+  int y = 2;
+  int z = 10;
+
+  x = z % y;
+  (void)(42 / x); // expected-warning {{Division by zero}}
+}
+
+
+void testRemPath2() {
+  int x = 2;
+  int y = 2;
+  int z = 0;
+
+  x = z % y;
+  (void)(42 / x); // expected-warning {{Division by zero}}
+}
+
+
+void testAdditionPath() {
+  int x = 2;
+  int y = 2;
+  int z = -2;
+
+  x = z + y;
+  (void)(42 / x); // expected-warning {{Division by zero}}
+}
+
+
+void testSubtractionPath() {
+  int x = 2;
+  int y = 2;
+  int z = 2;
+
+  x = z - y;
+  (void)(42 / x); // expected-warning {{Division by zero}}
+}
+
+
+void testNegationPath() {
+  int x = 2;
+  int y = 2;
+  int z = 2;
+
+  x = - y;
+  x = x + z;
+  (void)(42 / x); // expected-warning {{Division by zero}}
+}
+
+
+void testIfThenPath() {
+  int x = 2;
+  int y = 2;
+  int z = 2;
+
+  if(y > 0)
+x = z - y;
+  (void)(42 / x); // expected-warning {{Division by zero}}
+}
+
+
+void testIfElsePath() {
+  int x = 2;
+  int y = 2;
+  int z = 2;
+
+  if(y < 0) {
+x = z - y + 1;
+  } else {
+x = z - y;
+  }
+  (void)(42 / x); // expected-warning {{Division by zero}}
+}
+
+
+void testSwitchPath() {
+  int x = 2;
+  int y = 2;
+  int z = 2;
+
+  switch(y) {
+case 1: x = y; break;
+case 3: x = z; break;
+default: x %= z; break;
+  }
+
+  (void)(42 / x); // expected-warning {{Division by zero}}
+}
+
+
+void testForLoopPath1() {
+  int x = 5;
+
+  for(int i = 0; i < 5; ++i) {
+x = x - 1;
+  }
+  (void)(42 / x); // Missing warning
+}
+
+
+void testForLoopPath2() {
+  int x = 0;
+
+  for(int i = 0; i < 5; ++i) {}
+  (void)(42 / x); // Missing warning
+}
+
+
+void testForLoopPath3() {
+  int x = 0;
+
+  for(int i = 0; i < 1; ++i) {}
+
+  (void)(42 / x); // expected-warning {{Division by zero}}
+}
+
+
+void testWhileLoopPath() {
+  int x = 0;
+
+  int i = 0;
+  while(i < 5) {
+++i;
+  }
+  (void)(42

[PATCH] D52937: [clangd] Add clangd.serverInfo command to inspect server state.

[Sam McCall via Phabricator via cfe-commits]

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

Initially just export the information that's easily available.
(I want to measure changes in dynamic index size, so this is good enough for 
now)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52937

Files:
  clangd/ClangdLSPServer.cpp
  clangd/Protocol.cpp
  clangd/Protocol.h


Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -661,6 +661,8 @@
 struct ExecuteCommandParams {
   // Command to apply fix-its. Uses WorkspaceEdit as argument.
   const static llvm::StringLiteral CLANGD_APPLY_FIX_COMMAND;
+  // Retrieve information about the server state. No arguments.
+  const static llvm::StringLiteral CLANGD_SERVER_INFO;
 
   /// The command identifier, e.g. CLANGD_APPLY_FIX_COMMAND
   std::string command;
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -408,6 +408,8 @@
 
 const llvm::StringLiteral ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND =
 "clangd.applyFix";
+const llvm::StringLiteral ExecuteCommandParams::CLANGD_SERVER_INFO =
+"clangd.serverInfo";
 bool fromJSON(const json::Value &Params, ExecuteCommandParams &R) {
   json::ObjectMapper O(Params);
   if (!O || !O.map("command", R.command))
@@ -417,6 +419,8 @@
   if (R.command == ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND) {
 return Args && Args->size() == 1 &&
fromJSON(Args->front(), R.workspaceEdit);
+  } else if (R.command == ExecuteCommandParams::CLANGD_SERVER_INFO) {
+return true; // No args.
   }
   return false; // Unrecognized command.
 }
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -12,6 +12,7 @@
 #include "JSONRPCDispatcher.h"
 #include "SourceCode.h"
 #include "URI.h"
+#include "clang/Basic/Version.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/FormatVariadic.h"
@@ -214,6 +215,28 @@
 
 reply("Fix applied.");
 ApplyEdit(*Params.workspaceEdit);
+  } else if (Params.command == ExecuteCommandParams::CLANGD_SERVER_INFO) {
+json::Object Info{
+{"version", getClangToolFullVersion("clangd")},
+};
+if (const auto *DynIndex = Server->dynamicIndex()) {
+  Info["dynamicIndex"] = json::Object{
+  {"memory", int64_t(DynIndex->estimateMemoryUsage())},
+  };
+}
+json::Object TrackedFiles;
+auto MemUse = Server->getUsedBytesPerFile();
+DenseMap MemUseMap = {MemUse.begin(), MemUse.end()};
+for (const auto &File : DraftMgr.getActiveFiles()) {
+  json::Object FileInfo;
+  if (auto Data = DraftMgr.getDraft(File))
+FileInfo["size"] = int64_t(Data->size());
+  if (auto Mem = MemUseMap.lookup(File))
+FileInfo["memory"] = int64_t(Mem);
+  TrackedFiles[File] = std::move(FileInfo);
+}
+Info["files"] = std::move(TrackedFiles);
+reply(std::move(Info));
   } else {
 // We should not get here because ExecuteCommandParams would not have
 // parsed in the first place and this handler should not be called. But if


Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -661,6 +661,8 @@
 struct ExecuteCommandParams {
   // Command to apply fix-its. Uses WorkspaceEdit as argument.
   const static llvm::StringLiteral CLANGD_APPLY_FIX_COMMAND;
+  // Retrieve information about the server state. No arguments.
+  const static llvm::StringLiteral CLANGD_SERVER_INFO;
 
   /// The command identifier, e.g. CLANGD_APPLY_FIX_COMMAND
   std::string command;
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -408,6 +408,8 @@
 
 const llvm::StringLiteral ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND =
 "clangd.applyFix";
+const llvm::StringLiteral ExecuteCommandParams::CLANGD_SERVER_INFO =
+"clangd.serverInfo";
 bool fromJSON(const json::Value &Params, ExecuteCommandParams &R) {
   json::ObjectMapper O(Params);
   if (!O || !O.map("command", R.command))
@@ -417,6 +419,8 @@
   if (R.command == ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND) {
 return Args && Args->size() == 1 &&
fromJSON(Args->front(), R.workspaceEdit);
+  } else if (R.command == ExecuteCommandParams::CLANGD_SERVER_INFO) {
+return true; // No args.
   }
   return false; // Unrecognized command.
 }
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -12,6 +12,7 @@
 #include "JSONRPCDispatcher.h"
 #include "SourceCode.h"
 #include "URI.h

[PATCH] D52598: [OpenCL] Fixed address space cast in C style cast of C++ parsing

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaCast.cpp:2288
+  SrcType->isPointerType()) {
+const PointerType *DestPtr = DestType->getAs();
+if (!DestPtr->isAddressSpaceOverlapping(*SrcType->getAs())) {

Anastasia wrote:
> Anastasia wrote:
> > rjmccall wrote:
> > > Please test the result of `getAs` instead of separately testing 
> > > `isPointerType`.
> > > 
> > > Why is this check OpenCL-specific?  Address spaces are a general language 
> > > feature.
> > I think mainly because I am factoring out from the existing OpenCL check. 
> > But it probably makes sense that the semantics of this is not different in 
> > other languages. I will update it! Thanks!
> After playing with this for a bit longer I discovered that I have to keep the 
> OpenCL check unfortunately.
> 
> I found this old commit (`d4c5f84`/`r129583`) that says:
>   C-style casts can add/remove/change address spaces through the 
> reinterpret_cast mechanism.
> That's not the same as in OpenCL because it seems for C++ you can cast any AS 
> to any other AS. Therefore, no checks are needed at all. I am not sure if we 
> can come up with a common function for the moment.
> 
> The following  tests are checking this:
>CodeGenCXX/address-space-cast.cpp
>SemaCXX/address-space-conversion.cpp
> 
> Do you think it would make sense to rename this method with OpenCL-something 
> or keep in case may be CUDA or some other languages might need similar 
> functionality...
> 
I think you can leave it alone for now, but please add a comment explaining the 
reasoning as best you see it, and feel free to express uncertainty about the 
right rule.

Don't take `QualType` by `const &`, by the way.  It's a cheap-to-copy value 
type and should always be passed by value.


https://reviews.llvm.org/D52598



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

Re: [PATCH] Add some automatic tests for DivideZero checker

[Tamás Zolnai via cfe-commits]

Hi,

I uploaded this patch to phabricator:
https://reviews.llvm.org/D52936

Best Regards,
Tamás

Tamás Zolnai  ezt írta (időpont: 2018. okt. 5.,
P, 14:14):

> Hi all,
>
> I'm a new contributor in clang / llvm. I'm planning to work on clang
> static analyzer: maybe add new checker, imporve the exisiting one, etc. As
> the first step I checked how core.DivideZero checker works now and added
> some test cases for regression testing (see attached patch).
> The patch contains not only test cases when the checker catches an issue,
> but also use cases when the checker misses a division by zero situation,
> showing that there is some points where the checker can be improved.
>
> Best Regards,
> Tamás Zolnai
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52842: clang-format: Don't insert spaces in front of :: for Java 8 Method References.

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added a comment.

I'll take a look. I'm a bit worried that this might potentially affect C++ 
files too: I'll run an experiment over some random files to confirm that we're 
not missing something.


https://reviews.llvm.org/D52842



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

[PATCH] D46441: [clang][CodeGenCXX] Noalias attr for copy/move constructor arguments

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I'm fine with being more aggressive about this, and I agree that the standard 
should be making aliasing UB here.  We use a similarly aggressive rule with 
return values: NRVO can allow direct access to the return slot, which we mark 
`noalias`, but which can in fact be aliased if we're doing copy-elision in the 
caller.  IIRC the standard has analogously weak wording about what's supposed 
to happen in that case, but UB is really the only sensible rule.


Repository:
  rC Clang

https://reviews.llvm.org/D46441



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

[PATCH] D52892: [Clang-tidy] readability check to convert numerical constants to std::numeric_limits

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/readability/NumericalCostantsToMaxIntCheck.cpp:62
+CompilerInstance &Compiler) {
+  if (this->getLangOpts().CPlusPlus) {
+Compiler.getPreprocessor().addPPCallbacks(

JonasToth wrote:
> you dont need `this->` and please use the same return early pattern as in the 
> other registering call
By the word, see PR32774 for such check suggestion.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52892



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

[PATCH] D46441: [clang][CodeGenCXX] Noalias attr for copy/move constructor arguments

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGCall.cpp:1893
+
+IsCtor = isa(TargetDecl);
   }

I feel like you should just use `TargetDecl && 
isa(TargetDecl)` below; it's more obvious.

Is there not an analogous rule for destructors?


Repository:
  rC Clang

https://reviews.llvm.org/D46441



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

[PATCH] D52676: [clang-format] tweaked another case of lambda formatting

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added a comment.

In https://reviews.llvm.org/D52676#1251391, @oleg.smolsky wrote:

> In https://reviews.llvm.org/D52676#1251342, @krasimir wrote:
>
> > Digging a bit further, seems like the behavior you're looking for could be 
> > achieved by setting the `AlignAfterOpenBracket` option to `DontAlign` or 
> > `AlwaysBreak`:
> >
> >   % clang-format -style='{BasedOnStyle: google, AlignAfterOpenBracket: 
> > AlwaysBreak}' test.cc
> >void f() {
> >  something->Method2s(
> >  1,
> >  [this] {
> >Do1();
> >Do2();
> >  },
> >  1);
> >}
> >% clang-format -style='{BasedOnStyle: google, AlignAfterOpenBracket: 
> > DontAlign}' test.cc
> >void f() {
> >  something->Method2s(1,
> >  [this] {
> >Do1();
> >Do2();
> >  },
> >  1);
> >}
> >
> > Does this work for you?
>
>
> This is interesting. It obviously does what I want with the lambda, but 
> forces this:
>
>   void f() {
> auto stub = PopulateContextHereAndHereSomethi(GetSomethingHere(),
>   GetSomethingElseHere());
>   }
>
>
> into this:
>
>   void f() {
> auto stub = PopulateContextHereAndHereSomethi(
> GetSomethingHere(), GetSomethingElseHere());
>   }
>
> The former looks better to me and that's what Emacs does when you press Tab. 
> I think people here at work will balk at this formatting...
>
> > I don't think modifying the behavior as posed in this change based on the 
> > existence of multiline lambdas and the value of the `BinPackArguments` 
> > option is better than what we have now, especially when 
> > `AlignAfterOpenBracket=Align`.
>
> Yeah, I hear you. The patch is intended to work with 
> `AlignAfterOpenBracket=Align` and `BinPackArguments=false` and only tweaks 
> the lambdas' placement.
>
> How about I key the new behavior off a new value of `AlignAfterOpenBracket`, 
> such as `AlignAfterOpenBracket=AlignExceptLambda`?


Sorry, I don't think we want to support something this specific. I'd like to 
hear djasper@ and klimek@ opinions about this.


Repository:
  rC Clang

https://reviews.llvm.org/D52676



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

r343862 - [clang-format] Java import sorting in clang-format

[Krasimir Georgiev via cfe-commits]

Author: krasimir
Date: Fri Oct  5 10:19:26 2018
New Revision: 343862

URL: http://llvm.org/viewvc/llvm-project?rev=343862&view=rev
Log:
[clang-format] Java import sorting in clang-format

Contributed by SamMaier!

Added:
cfe/trunk/unittests/Format/SortImportsTestJava.cpp
Modified:
cfe/trunk/include/clang/Format/Format.h
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/unittests/Format/CMakeLists.txt

Modified: cfe/trunk/include/clang/Format/Format.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=343862&r1=343861&r2=343862&view=diff
==
--- cfe/trunk/include/clang/Format/Format.h (original)
+++ cfe/trunk/include/clang/Format/Format.h Fri Oct  5 10:19:26 2018
@@ -1130,6 +1130,35 @@ struct FormatStyle {
   /// \endcode
   bool IndentWrappedFunctionNames;
 
+  /// A vector of prefixes ordered by the desired groups for Java imports.
+  ///
+  /// Each group is seperated by a newline. Static imports will also follow the
+  /// same grouping convention above all non-static imports. One group's prefix
+  /// can be a subset of another - the longest prefix is always matched. Within
+  /// a group, the imports are ordered lexicographically.
+  ///
+  /// In the .clang-format configuration file, this can be configured like:
+  /// \code{.yaml}
+  ///   JavaImportGroups: ['com.example', 'com', 'org']
+  /// \endcode
+  /// Which will result in imports being formatted as so:
+  /// \code{.java}
+  ///import static com.example.function1;
+  ///
+  ///import static com.test.function2;
+  ///
+  ///import static org.example.function3;
+  ///
+  ///import com.example.ClassA;
+  ///import com.example.Test;
+  ///import com.example.a.ClassB;
+  ///
+  ///import com.test.ClassC;
+  ///
+  ///import org.example.ClassD;
+  /// \endcode
+  std::vector JavaImportGroups;
+
   /// Quotation styles for JavaScript strings. Does not affect template
   /// strings.
   enum JavaScriptQuoteStyle {
@@ -1734,6 +1763,7 @@ struct FormatStyle {
IndentPPDirectives == R.IndentPPDirectives &&
IndentWidth == R.IndentWidth && Language == R.Language &&
IndentWrappedFunctionNames == R.IndentWrappedFunctionNames &&
+   JavaImportGroups == R.JavaImportGroups &&
JavaScriptQuotes == R.JavaScriptQuotes &&
JavaScriptWrapImports == R.JavaScriptWrapImports &&
KeepEmptyLinesAtTheStartOfBlocks ==

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=343862&r1=343861&r2=343862&view=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Fri Oct  5 10:19:26 2018
@@ -414,6 +414,7 @@ template <> struct MappingTraitshttps://chromium.googlesource.com/chromium/src/+/master/styleguide/java/java.md#Import-Order
+ChromiumStyle.JavaImportGroups = {
+"android",
+"com",
+"dalvik",
+"junit",
+"org",
+"com.google.android.apps.chrome",
+"org.chromium",
+"java",
+"javax",
+};
+ChromiumStyle.SortIncludes = true;
   } else if (Language == FormatStyle::LK_JavaScript) {
 ChromiumStyle.AllowShortIfStatementsOnASingleLine = false;
 ChromiumStyle.AllowShortLoopsOnASingleLine = false;
@@ -1608,6 +1623,14 @@ struct IncludeDirective {
   int Category;
 };
 
+struct JavaImportDirective {
+  StringRef Identifier;
+  StringRef Text;
+  unsigned Offset;
+  std::vector AssociatedCommentLines;
+  bool IsStatic;
+};
+
 } // end anonymous namespace
 
 // Determines whether 'Ranges' intersects with ('Start', 'End').
@@ -1726,7 +1749,7 @@ static void sortCppIncludes(const Format
 
 namespace {
 
-const char IncludeRegexPattern[] =
+const char CppIncludeRegexPattern[] =
 R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
 
 } // anonymous namespace
@@ -1738,7 +1761,7 @@ tooling::Replacements sortCppIncludes(co
   unsigned *Cursor) {
   unsigned Prev = 0;
   unsigned SearchFrom = 0;
-  llvm::Regex IncludeRegex(IncludeRegexPattern);
+  llvm::Regex IncludeRegex(CppIncludeRegexPattern);
   SmallVector Matches;
   SmallVector IncludesInBlock;
 
@@ -1797,6 +1820,149 @@ tooling::Replacements sortCppIncludes(co
   return Replaces;
 }
 
+// Returns group number to use as a first order sort on imports. Gives UINT_MAX
+// if the import does not match any given groups.
+static unsigned findJavaImportGroup(const FormatStyle &Style,
+StringRef ImportIdentifier) {
+  unsigned LongestMatchIndex = UINT_MAX;
+  unsigned LongestMatchLength = 0;
+  for (unsigned I = 0; I < Style.JavaImportGroups.size(); I++) {
+std::string GroupPrefix = Style.JavaImportGroups[I];
+if (ImportIdentifier.startswith(GroupPrefix) &&
+GroupPre

[PATCH] D52445: [Index] Use locations to uniquify function-scope BindingDecl USR

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 168486.
MaskRay added a comment.

Add test to Core/index-source.cpp


Repository:
  rC Clang

https://reviews.llvm.org/D52445

Files:
  lib/Index/USRGeneration.cpp
  test/Index/Core/index-source.cpp


Index: test/Index/Core/index-source.cpp
===
--- test/Index/Core/index-source.cpp
+++ test/Index/Core/index-source.cpp
@@ -1,4 +1,5 @@
 // RUN: c-index-test core -print-source-symbols -- %s -std=c++1z -target 
x86_64-apple-macosx10.7 | FileCheck %s
+// RUN: c-index-test core -print-source-symbols -include-locals -- %s 
-std=c++1z -target x86_64-apple-macosx10.7 | FileCheck -check-prefix=LOCAL %s
 
 // CHECK: [[@LINE+1]]:7 | class/C++ | Cls | [[Cls_USR:.*]] |  | Def 
| rel: 0
 class Cls { public:
@@ -493,6 +494,7 @@
 // CHECK: [[@LINE-1]]:69 | variable/C++ | structuredBinding2 | 
c:@N@cpp17structuredBinding@structuredBinding2 |  | Ref,Read,RelCont 
| rel: 1
 // CHECK-NEXT: RelCont | localStructuredBindingAndRef | 
c:@N@cpp17structuredBinding@F@localStructuredBindingAndRef#
 // CHECK-NOT: localBinding
+// LOCAL: [[@LINE-4]]:9 | variable(local)/C++ | localBinding1 | 
c:index-source.cpp@25382@N@cpp17structuredBinding@F@localStructuredBindingAndRef#@localBinding1
 }
 
 }
Index: lib/Index/USRGeneration.cpp
===
--- lib/Index/USRGeneration.cpp
+++ lib/Index/USRGeneration.cpp
@@ -97,6 +97,7 @@
   void VisitTypedefDecl(const TypedefDecl *D);
   void VisitTemplateTypeParmDecl(const TemplateTypeParmDecl *D);
   void VisitVarDecl(const VarDecl *D);
+  void VisitBindingDecl(const BindingDecl *D);
   void VisitNonTypeTemplateParmDecl(const NonTypeTemplateParmDecl *D);
   void VisitTemplateTemplateParmDecl(const TemplateTemplateParmDecl *D);
   void VisitUnresolvedUsingValueDecl(const UnresolvedUsingValueDecl *D);
@@ -334,6 +335,12 @@
   }
 }
 
+void USRGenerator::VisitBindingDecl(const BindingDecl *D) {
+  if (D->getParentFunctionOrMethod() && GenLoc(D, /*IncludeOffset=*/true))
+return;
+  VisitNamedDecl(D);
+}
+
 void USRGenerator::VisitNonTypeTemplateParmDecl(
 const NonTypeTemplateParmDecl *D) {
   GenLoc(D, /*IncludeOffset=*/true);


Index: test/Index/Core/index-source.cpp
===
--- test/Index/Core/index-source.cpp
+++ test/Index/Core/index-source.cpp
@@ -1,4 +1,5 @@
 // RUN: c-index-test core -print-source-symbols -- %s -std=c++1z -target x86_64-apple-macosx10.7 | FileCheck %s
+// RUN: c-index-test core -print-source-symbols -include-locals -- %s -std=c++1z -target x86_64-apple-macosx10.7 | FileCheck -check-prefix=LOCAL %s
 
 // CHECK: [[@LINE+1]]:7 | class/C++ | Cls | [[Cls_USR:.*]] |  | Def | rel: 0
 class Cls { public:
@@ -493,6 +494,7 @@
 // CHECK: [[@LINE-1]]:69 | variable/C++ | structuredBinding2 | c:@N@cpp17structuredBinding@structuredBinding2 |  | Ref,Read,RelCont | rel: 1
 // CHECK-NEXT: RelCont | localStructuredBindingAndRef | c:@N@cpp17structuredBinding@F@localStructuredBindingAndRef#
 // CHECK-NOT: localBinding
+// LOCAL: [[@LINE-4]]:9 | variable(local)/C++ | localBinding1 | c:index-source.cpp@25382@N@cpp17structuredBinding@F@localStructuredBindingAndRef#@localBinding1
 }
 
 }
Index: lib/Index/USRGeneration.cpp
===
--- lib/Index/USRGeneration.cpp
+++ lib/Index/USRGeneration.cpp
@@ -97,6 +97,7 @@
   void VisitTypedefDecl(const TypedefDecl *D);
   void VisitTemplateTypeParmDecl(const TemplateTypeParmDecl *D);
   void VisitVarDecl(const VarDecl *D);
+  void VisitBindingDecl(const BindingDecl *D);
   void VisitNonTypeTemplateParmDecl(const NonTypeTemplateParmDecl *D);
   void VisitTemplateTemplateParmDecl(const TemplateTemplateParmDecl *D);
   void VisitUnresolvedUsingValueDecl(const UnresolvedUsingValueDecl *D);
@@ -334,6 +335,12 @@
   }
 }
 
+void USRGenerator::VisitBindingDecl(const BindingDecl *D) {
+  if (D->getParentFunctionOrMethod() && GenLoc(D, /*IncludeOffset=*/true))
+return;
+  VisitNamedDecl(D);
+}
+
 void USRGenerator::VisitNonTypeTemplateParmDecl(
 const NonTypeTemplateParmDecl *D) {
   GenLoc(D, /*IncludeOffset=*/true);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52920: Introduce code_model macros

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: include/clang/Basic/TargetOptions.h:72
+  // The code model to be used as specified by the user. Corresponds to
+  // Model enum defined in include/llvm/Support/CodeGen.h, plus "default" for
+  // the case when the user has not explicitly specified a code model.

`Model` -> `CodeModel::Model` because there are `Model` in other namespaces

```
  namespace Reloc {
  enum Model { Static, PIC_, DynamicNoPIC, ROPI, RWPI, ROPI_RWPI };
  }

  // Code model types.
  namespace CodeModel {
// Sync changes with CodeGenCWrappers.h.
  enum Model { Tiny, Small, Kernel, Medium, Large };
  }
```



Comment at: lib/Basic/Targets/X86.cpp:866
+// For compatibility with gcc.
+CodeModel = "small";
+  Builder.defineMacro("__code_model_" + CodeModel + "_");

The comment `// For compatibility with gcc.` needs changing.

Small code model is the fastest and expected to be suitable for vast majority 
of programs.

It is chosen here:

https://github.com/llvm-mirror/llvm/tree/master/lib/Target/X86/X86TargetMachine.cpp#L208

static CodeModel::Model getEffectiveCodeModel(Optional CM,
  bool JIT, bool Is64Bit) {
  if (CM)
return *CM;
  if (JIT)
return Is64Bit ? CodeModel::Large : CodeModel::Small;
  return CodeModel::Small;
}



Repository:
  rC Clang

https://reviews.llvm.org/D52920



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

[PATCH] D52938: [CUDA] Use all 64 bits of GUID in __nv_module_id

[Artem Belevich via Phabricator via cfe-commits]

tra created this revision.
tra added a reviewer: Hahnfeld.
Herald added subscribers: bixia, jlebar, sanjoy.

getGUID() returns an uint64_t and "%x" only prints 32 bits of it.
Use PRIx64 format string to print all 64 bits.


https://reviews.llvm.org/D52938

Files:
  clang/lib/CodeGen/CGCUDANV.cpp


Index: clang/lib/CodeGen/CGCUDANV.cpp
===
--- clang/lib/CodeGen/CGCUDANV.cpp
+++ clang/lib/CodeGen/CGCUDANV.cpp
@@ -520,7 +520,7 @@
 // Generate a unique module ID.
 SmallString<64> ModuleID;
 llvm::raw_svector_ostream OS(ModuleID);
-OS << ModuleIDPrefix << llvm::format("%x", FatbinWrapper->getGUID());
+OS << ModuleIDPrefix << llvm::format("%" PRIx64, FatbinWrapper->getGUID());
 llvm::Constant *ModuleIDConstant =
 makeConstantString(ModuleID.str(), "", ModuleIDSectionName, 32);
 


Index: clang/lib/CodeGen/CGCUDANV.cpp
===
--- clang/lib/CodeGen/CGCUDANV.cpp
+++ clang/lib/CodeGen/CGCUDANV.cpp
@@ -520,7 +520,7 @@
 // Generate a unique module ID.
 SmallString<64> ModuleID;
 llvm::raw_svector_ostream OS(ModuleID);
-OS << ModuleIDPrefix << llvm::format("%x", FatbinWrapper->getGUID());
+OS << ModuleIDPrefix << llvm::format("%" PRIx64, FatbinWrapper->getGUID());
 llvm::Constant *ModuleIDConstant =
 makeConstantString(ModuleID.str(), "", ModuleIDSectionName, 32);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52891: [AMDGPU] Add -fvisibility-amdgpu-non-kernel-functions

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In https://reviews.llvm.org/D52891#1256207, @arsenm wrote:

> I think the name needs work, but I'm not sure what it should be. I think it 
> should avoid using "non" and "amdgpu"


I think dropping amdgpu is fine since we can add (AMDGUP only) to the 
description of the option, following the precedence of

  -ffixed-r9  Reserve the r9 register (ARM only)

However it is difficult to coin a different term for 'non-kernel-function'. 
Also, I saw precedence of using 'non' in option name:

  -objcmt-ns-nonatomic-iosonly

So, probably we could use `-fvisibility-nonkernel-function` ?


https://reviews.llvm.org/D52891



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

r343867 - Emit diagnostic note when calling an invalid function declaration.

[James Y Knight via cfe-commits]

Author: jyknight
Date: Fri Oct  5 10:49:48 2018
New Revision: 343867

URL: http://llvm.org/viewvc/llvm-project?rev=343867&view=rev
Log:
Emit diagnostic note when calling an invalid function declaration.

The comment said it was intentionally not emitting any diagnostic
because the declaration itself was already diagnosed. However,
everywhere else that wants to not emit a diagnostic without an extra
note emits note_invalid_subexpr_in_const_expr instead, which gets
suppressed later.

This was the only place which did not emit a diagnostic note.

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

Modified:
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/test/SemaCXX/enable_if.cpp

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=343867&r1=343866&r2=343867&view=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Fri Oct  5 10:49:48 2018
@@ -4330,10 +4330,13 @@ static bool CheckConstexprFunction(EvalI
   Declaration->isConstexpr())
 return false;
 
-  // Bail out with no diagnostic if the function declaration itself is invalid.
-  // We will have produced a relevant diagnostic while parsing it.
-  if (Declaration->isInvalidDecl())
+  // Bail out if the function declaration itself is invalid.  We will
+  // have produced a relevant diagnostic while parsing it, so just
+  // note the problematic sub-expression.
+  if (Declaration->isInvalidDecl()) {
+Info.FFDiag(CallLoc, diag::note_invalid_subexpr_in_const_expr);
 return false;
+  }
 
   // Can we evaluate this function call?
   if (Definition && Definition->isConstexpr() &&

Modified: cfe/trunk/test/SemaCXX/enable_if.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/enable_if.cpp?rev=343867&r1=343866&r2=343867&view=diff
==
--- cfe/trunk/test/SemaCXX/enable_if.cpp (original)
+++ cfe/trunk/test/SemaCXX/enable_if.cpp Fri Oct  5 10:49:48 2018
@@ -414,7 +414,8 @@ static_assert(templated<1>() == 1, "");
 
 template  constexpr int callTemplated() { return templated(); }
 
-constexpr int B = callTemplated<0>(); // expected-error{{initialized by a 
constant expression}} expected-error@-2{{no matching function for call to 
'templated'}} expected-note{{in instantiation of function template}} 
expected-note@-9{{candidate disabled}}
+constexpr int B = 10 + // the carat for the error should be pointing to the 
problematic call (on the next line), not here.
+callTemplated<0>(); // expected-error{{initialized by a constant 
expression}} expected-error@-3{{no matching function for call to 'templated'}} 
expected-note{{in instantiation of function template}} 
expected-note@-10{{candidate disabled}}
 static_assert(callTemplated<1>() == 1, "");
 }
 


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

[PATCH] D52891: [AMDGPU] Add -fvisibility-amdgpu-non-kernel-functions

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

Can you also fix HIP toolchain? It is in HIPToolChain::addClangTargetOptions. 
Thanks.


https://reviews.llvm.org/D52891



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

[PATCH] D52919: Emit diagnostic note when calling an invalid function declaration.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC343867: Emit diagnostic note when calling an invalid 
function declaration. (authored by jyknight, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52919?vs=168417&id=168490#toc

Repository:
  rC Clang

https://reviews.llvm.org/D52919

Files:
  lib/AST/ExprConstant.cpp
  test/SemaCXX/enable_if.cpp


Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -4330,10 +4330,13 @@
   Declaration->isConstexpr())
 return false;
 
-  // Bail out with no diagnostic if the function declaration itself is invalid.
-  // We will have produced a relevant diagnostic while parsing it.
-  if (Declaration->isInvalidDecl())
+  // Bail out if the function declaration itself is invalid.  We will
+  // have produced a relevant diagnostic while parsing it, so just
+  // note the problematic sub-expression.
+  if (Declaration->isInvalidDecl()) {
+Info.FFDiag(CallLoc, diag::note_invalid_subexpr_in_const_expr);
 return false;
+  }
 
   // Can we evaluate this function call?
   if (Definition && Definition->isConstexpr() &&
Index: test/SemaCXX/enable_if.cpp
===
--- test/SemaCXX/enable_if.cpp
+++ test/SemaCXX/enable_if.cpp
@@ -414,7 +414,8 @@
 
 template  constexpr int callTemplated() { return templated(); }
 
-constexpr int B = callTemplated<0>(); // expected-error{{initialized by a 
constant expression}} expected-error@-2{{no matching function for call to 
'templated'}} expected-note{{in instantiation of function template}} 
expected-note@-9{{candidate disabled}}
+constexpr int B = 10 + // the carat for the error should be pointing to the 
problematic call (on the next line), not here.
+callTemplated<0>(); // expected-error{{initialized by a constant 
expression}} expected-error@-3{{no matching function for call to 'templated'}} 
expected-note{{in instantiation of function template}} 
expected-note@-10{{candidate disabled}}
 static_assert(callTemplated<1>() == 1, "");
 }
 


Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -4330,10 +4330,13 @@
   Declaration->isConstexpr())
 return false;
 
-  // Bail out with no diagnostic if the function declaration itself is invalid.
-  // We will have produced a relevant diagnostic while parsing it.
-  if (Declaration->isInvalidDecl())
+  // Bail out if the function declaration itself is invalid.  We will
+  // have produced a relevant diagnostic while parsing it, so just
+  // note the problematic sub-expression.
+  if (Declaration->isInvalidDecl()) {
+Info.FFDiag(CallLoc, diag::note_invalid_subexpr_in_const_expr);
 return false;
+  }
 
   // Can we evaluate this function call?
   if (Definition && Definition->isConstexpr() &&
Index: test/SemaCXX/enable_if.cpp
===
--- test/SemaCXX/enable_if.cpp
+++ test/SemaCXX/enable_if.cpp
@@ -414,7 +414,8 @@
 
 template  constexpr int callTemplated() { return templated(); }
 
-constexpr int B = callTemplated<0>(); // expected-error{{initialized by a constant expression}} expected-error@-2{{no matching function for call to 'templated'}} expected-note{{in instantiation of function template}} expected-note@-9{{candidate disabled}}
+constexpr int B = 10 + // the carat for the error should be pointing to the problematic call (on the next line), not here.
+callTemplated<0>(); // expected-error{{initialized by a constant expression}} expected-error@-3{{no matching function for call to 'templated'}} expected-note{{in instantiation of function template}} expected-note@-10{{candidate disabled}}
 static_assert(callTemplated<1>() == 1, "");
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52842: clang-format: Don't insert spaces in front of :: for Java 8 Method References.

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.



Comment at: lib/Format/TokenAnnotator.cpp:2559
 // A.>>DoSomething();
+// A::>>DoSomething();
 return false;

nit: please add a comment that this example comes from Java.


https://reviews.llvm.org/D52842



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

[PATCH] D52842: clang-format: Don't insert spaces in front of :: for Java 8 Method References.

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added a comment.

Looks good!
I didn't find any instances where this messes-up C++ code (and it looks fine to 
me for Java code).


https://reviews.llvm.org/D52842



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

[PATCH] D52920: Introduce code_model macros

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: test/Preprocessor/init.c:7992
+//
+// RUN: %clang -xc - -E -dD -mcmodel=medium --target=i386-unknown-linux < 
/dev/null | FileCheck -match-full-lines -check-prefix X86_MEDIUM %s
+// X86_MEDIUM:#define __code_model_medium_ 1

`-dM` here is better because it suppresses other CPP output 
(https://github.com/llvm-mirror/clang/tree/BindingDecl/lib/Frontend/CompilerInvocation.cpp#L2962)



Repository:
  rC Clang

https://reviews.llvm.org/D52920



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

[PATCH] D52938: [CUDA] Use all 64 bits of GUID in __nv_module_id

[Jonas Hahnfeld via Phabricator via cfe-commits]

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

LG.

Out of interest: Is this fixing a particular issue?


https://reviews.llvm.org/D52938



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

[PATCH] D52578: Thread safety analysis: Allow scoped releasing of capabilities

[Victor Costan via Phabricator via cfe-commits]

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

test/SemaCXX/warn-thread-safety-analysis.cpp LGTM -- this is the functionality 
that we need in Chrome to use thread safety annotations for AutoUnlock.

very non-authoritative opinion - I think the tests would be a bit easier to 
follow if Lock() would be the EXCLUSIVE_LOCK_FUNCTION and Unlock() would be the 
EXCLUSIVE_UNLOCK_FUNCTION.

aaronpuchert: Thank you for the patch! I'd be happy and grateful to have this 
functionality in Chrome.
delesley: Thank you for your review and guidance!


Repository:
  rC Clang

https://reviews.llvm.org/D52578



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

[PATCH] D52939: ExprConstant: Propagate correct return values from constant evaluation.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
jyknight added a reviewer: rsmith.

The constant evaluation now returns false whenever indicating failure
would be appropriate for the requested mode, instead of returning
"true" for success, and depending on the caller examining the various
status variables after the fact, in some circumstances.

In EM_ConstantExpression mode, evaluation is aborted as soon as a
diagnostic note is generated, and therefore, a "true" return value now
actually indicates that the expression was a constexpr. (You no longer
have to additionally check for a lack of diagnostic messages.)

In EM_ConstantFold, evaluation is now aborted upon running into a
side-effect -- so a "true" return value now actually indicates that
there were no side-effects

Also -- update and clarify documentation for the evaluation modes.

This is a cleanup, not intending to change the functionality, as
callers had been checking those properties manually before.


https://reviews.llvm.org/D52939

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

Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -412,52 +412,10 @@
   MostDerivedArraySize = 2;
   MostDerivedPathLength = Entries.size();
 }
-void diagnoseUnsizedArrayPointerArithmetic(EvalInfo &Info, const Expr *E);
 void diagnosePointerArithmetic(EvalInfo &Info, const Expr *E,
const APSInt &N);
 /// Add N to the address of this subobject.
-void adjustIndex(EvalInfo &Info, const Expr *E, APSInt N) {
-  if (Invalid || !N) return;
-  uint64_t TruncatedN = N.extOrTrunc(64).getZExtValue();
-  if (isMostDerivedAnUnsizedArray()) {
-diagnoseUnsizedArrayPointerArithmetic(Info, E);
-// Can't verify -- trust that the user is doing the right thing (or if
-// not, trust that the caller will catch the bad behavior).
-// FIXME: Should we reject if this overflows, at least?
-Entries.back().ArrayIndex += TruncatedN;
-return;
-  }
-
-  // [expr.add]p4: For the purposes of these operators, a pointer to a
-  // nonarray object behaves the same as a pointer to the first element of
-  // an array of length one with the type of the object as its element type.
-  bool IsArray = MostDerivedPathLength == Entries.size() &&
- MostDerivedIsArrayElement;
-  uint64_t ArrayIndex =
-  IsArray ? Entries.back().ArrayIndex : (uint64_t)IsOnePastTheEnd;
-  uint64_t ArraySize =
-  IsArray ? getMostDerivedArraySize() : (uint64_t)1;
-
-  if (N < -(int64_t)ArrayIndex || N > ArraySize - ArrayIndex) {
-// Calculate the actual index in a wide enough type, so we can include
-// it in the note.
-N = N.extend(std::max(N.getBitWidth() + 1, 65));
-(llvm::APInt&)N += ArrayIndex;
-assert(N.ugt(ArraySize) && "bounds check failed for in-bounds index");
-diagnosePointerArithmetic(Info, E, N);
-setInvalid();
-return;
-  }
-
-  ArrayIndex += TruncatedN;
-  assert(ArrayIndex <= ArraySize &&
- "bounds check succeeded for out-of-bounds index");
-
-  if (IsArray)
-Entries.back().ArrayIndex = ArrayIndex;
-  else
-IsOnePastTheEnd = (ArrayIndex != 0);
-}
+bool adjustIndex(EvalInfo &Info, const Expr *E, APSInt N);
   };
 
   /// A stack frame in the constexpr call stack.
@@ -721,43 +679,68 @@
 bool IsSpeculativelyEvaluating;
 
 enum EvaluationMode {
-  /// Evaluate as a constant expression. Stop if we find that the expression
-  /// is not a constant expression.
+  /* The various EvaluationMode kinds vary in terms of what sorts of
+   * expressions they accept.
+
+ Key for the following table:
+ * D = Diagnostic notes (about non-constexpr, but still evaluable
+   constructs) are OK (keepEvaluatingAfterNote)
+ * S = Failure to evaluate which only occurs in expressions used for
+   side-effect are okay.  (keepEvaluatingAfterSideEffect)
+ * F = Failure to evaluate is okay. (keepEvaluatingAfterFailure)
+ * E = Eagerly evaluate certain builtins to a value which would normally
+   defer until after optimizations.
+
+ +-+---+---+---+---+
+ | | D | S | F | E |
+ +-+---+---+---+---+
+ |EM_ConstantExpression| . | . | . | . |
+ |EM_ConstantFold  | Y | . | . | . |
+ |EM_ConstantFoldUnevaluated   | Y | . | . | Y |
+ |EM_IgnoreSideEffects | Y | Y | . | . |
+ |EM_PotentialConstantExpression   | . | Y | Y | . |
+ |EM_PotentialConstan

[PATCH] D52842: clang-format: Don't insert spaces in front of :: for Java 8 Method References.

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Thanks! Will land with tweaked comment.




Comment at: lib/Format/TokenAnnotator.cpp:2559
 // A.>>DoSomething();
+// A::>>DoSomething();
 return false;

krasimir wrote:
> nit: please add a comment that this example comes from Java.
Will do. I believe the existing example is Java too.


https://reviews.llvm.org/D52842



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

[PATCH] D52920: Introduce code_model macros

[Ali Tamur via Phabricator via cfe-commits]

tamur updated this revision to Diff 168494.

Repository:
  rC Clang

https://reviews.llvm.org/D52920

Files:
  include/clang/Basic/TargetOptions.h
  lib/Basic/Targets/X86.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/Preprocessor/init.c

Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -47,21 +47,21 @@
 // CXX11:#define __cplusplus 201103L
 // CXX11:#define __private_extern__ extern
 //
-// 
+//
 // RUN: %clang_cc1 -x c++ -std=c++98 -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix CXX98 %s
-// 
+//
 // CXX98:#define __GNUG__ {{.*}}
 // CXX98:#define __GXX_RTTI 1
 // CXX98:#define __GXX_WEAK__ 1
 // CXX98:#define __cplusplus 199711L
 // CXX98:#define __private_extern__ extern
 //
-// 
+//
 // RUN: %clang_cc1 -fdeprecated-macro -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix DEPRECATED %s
 //
 // DEPRECATED:#define __DEPRECATED 1
 //
-// 
+//
 // RUN: %clang_cc1 -std=c99 -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix C99 %s
 //
 // C99:#define __STDC_VERSION__ 199901L
@@ -71,7 +71,7 @@
 // C99-NOT: __GXX_WEAK__
 // C99-NOT: __cplusplus
 //
-// 
+//
 // RUN: %clang_cc1 -std=c11 -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix C11 %s
 // RUN: %clang_cc1 -std=c1x -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix C11 %s
 // RUN: %clang_cc1 -std=iso9899:2011 -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix C11 %s
@@ -86,7 +86,7 @@
 // C11-NOT: __GXX_WEAK__
 // C11-NOT: __cplusplus
 //
-// 
+//
 // RUN: %clang_cc1 -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix COMMON %s
 //
 // COMMON:#define __CONSTANT_CFSTRINGS__ 1
@@ -113,7 +113,7 @@
 // RUN: %clang_cc1 -E -dM -triple=x86_64-pc-linux-gnu < /dev/null | FileCheck -match-full-lines -check-prefix C-DEFAULT %s
 // RUN: %clang_cc1 -E -dM -triple=x86_64-apple-darwin < /dev/null | FileCheck -match-full-lines -check-prefix C-DEFAULT %s
 // RUN: %clang_cc1 -E -dM -triple=armv7a-apple-darwin < /dev/null | FileCheck -match-full-lines -check-prefix C-DEFAULT %s
-// 
+//
 // C-DEFAULT:#define __STDC_VERSION__ 201112L
 //
 // RUN: %clang_cc1 -ffreestanding -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix FREESTANDING %s
@@ -158,12 +158,12 @@
 // GXX98:#define __cplusplus 199711L
 // GXX98:#define __private_extern__ extern
 //
-// 
+//
 // RUN: %clang_cc1 -std=iso9899:199409 -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix C94 %s
 //
 // C94:#define __STDC_VERSION__ 199409L
 //
-// 
+//
 // RUN: %clang_cc1 -fms-extensions -triple i686-pc-win32 -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix MSEXT %s
 //
 // MSEXT-NOT:#define __STDC__
@@ -185,7 +185,7 @@
 // MSEXT-CXX-NOWCHAR-NOT:#define _WCHAR_T_DEFINED 1
 // MSEXT-CXX-NOWCHAR:#define __BOOL_DEFINED 1
 //
-// 
+//
 // RUN: %clang_cc1 -x objective-c -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix OBJC %s
 //
 // OBJC:#define OBJC_NEW_PROPERTIES 1
@@ -197,7 +197,7 @@
 //
 // OBJCGC:#define __OBJC_GC__ 1
 //
-// 
+//
 // RUN: %clang_cc1 -x objective-c -fobjc-exceptions -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix NONFRAGILE %s
 //
 // NONFRAGILE:#define OBJC_ZEROCOST_EXCEPTIONS 1
@@ -246,9 +246,9 @@
 //
 // PASCAL:#define __PASCAL_STRINGS__ 1
 //
-// 
+//
 // RUN: %clang_cc1 -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix SCHAR %s
-// 
+//
 // SCHAR:#define __STDC__ 1
 // SCHAR-NOT:#define __UNSIGNED_CHAR__
 // SCHAR:#define __clang__ 1
@@ -7978,6 +7978,7 @@
 // X86_64:#define __WINT_WIDTH__ 32
 // X86_64:#define __amd64 1
 // X86_64:#define __amd64__ 1
+// X86_64:#define __code_model_small_ 1
 // X86_64:#define __x86_64 1
 // X86_64:#define __x86_64__ 1
 //
@@ -7987,7 +7988,10 @@
 // X86_64H:#define __x86_64__ 1
 // X86_64H:#define __x86_64h 1
 // X86_64H:#define __x86_64h__ 1
-
+//
+// RUN: %clang -xc - -E -dM -mcmodel=medium --target=i386-unknown-linux < /dev/null | FileCheck -match-full-lines -check-prefix X86_MEDIUM %s
+// X86_MEDIUM:#define __code_model_medium_ 1
+//
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=x86_64-none-none-gnux32 < /dev/null | FileCheck -match-full-lines -check-prefix X32 %s
 // RUN: %clang_cc1 -x c++ -E -dM -ffreestanding -triple=x86_64-none-none-gnux32 < /dev/null | FileCheck -match-full-lines -check-prefix X32 -check-prefix X32-CXX %s
 //
@@ -9830,16 +9834,16 @@
 // AVR:#define __GCC_ATOMIC_TEST_AND_SET_TRUEVAL 1
 // AVR:#define __GCC_ATOMIC_WCHAR_T_LOCK_FREE 1
 // AVR:#define __GXX_ABI_VERSION 1002
-// AVR:#define __INT16_C_SUFFIX__ 
+// AVR:#define __INT16_C_SUFFIX__
 // AVR:#define __INT16_MAX__ 32767
 // AVR:#define __INT16_TYPE__ short
 // AVR:#define __INT32_C_SUFFIX__ L
 // AVR:#define __INT32_MAX__ 2147483647L
 // AVR:#define __INT32_TYPE__ long int
 // AVR:#define __INT64_C_SUFFIX__ LL
 // AVR:#define __INT64_MAX__ 9223372036854775807LL
 // AVR:#define __INT64_TYPE__ long long int
-/

r343872 - clang-format: Don't insert spaces in front of :: for Java 8 Method References.

[Nico Weber via cfe-commits]

Author: nico
Date: Fri Oct  5 11:22:21 2018
New Revision: 343872

URL: http://llvm.org/viewvc/llvm-project?rev=343872&view=rev
Log:
clang-format: Don't insert spaces in front of :: for Java 8 Method References.

The existing code kept the space if it was there for identifiers, and it didn't
handle `this`. After this patch, for Java `this` is handled in addition to
identifiers, and existing space is always stripped between identifier and `::`.

Also accept `::` in addition to `.` in front of `<` in `foo::bar` generic
calls.

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

Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestJava.cpp

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=343872&r1=343871&r2=343872&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Fri Oct  5 11:22:21 2018
@@ -2555,8 +2555,11 @@ bool TokenAnnotator::spaceRequiredBetwee
 return false;
   if (Left.is(TT_TemplateCloser) && Left.MatchingParen &&
   Left.MatchingParen->Previous &&
-  Left.MatchingParen->Previous->is(tok::period))
+  (Left.MatchingParen->Previous->is(tok::period) ||
+   Left.MatchingParen->Previous->is(tok::coloncolon)))
+// Java call to generic function with explicit type:
 // A.>>DoSomething();
+// A::>>DoSomething();  // With a Java 8 method reference.
 return false;
   if (Left.is(TT_TemplateCloser) && Right.is(tok::l_square))
 return false;
@@ -2776,6 +2779,9 @@ bool TokenAnnotator::spaceRequiredBefore
   if (!Style.SpaceBeforeAssignmentOperators &&
   Right.getPrecedence() == prec::Assignment)
 return false;
+  if (Style.Language == FormatStyle::LK_Java && Right.is(tok::coloncolon) &&
+  (Left.is(tok::identifier) || Left.is(tok::kw_this)))
+return false;
   if (Right.is(tok::coloncolon) && Left.is(tok::identifier))
 // Generally don't remove existing spaces between an identifier and "::".
 // The identifier might actually be a macro name such as ALWAYS_INLINE. If

Modified: cfe/trunk/unittests/Format/FormatTestJava.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJava.cpp?rev=343872&r1=343871&r2=343872&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestJava.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJava.cpp Fri Oct  5 11:22:21 2018
@@ -443,6 +443,22 @@ TEST_F(FormatTestJava, MethodDeclaration
getStyleWithColumns(40));
 }
 
+TEST_F(FormatTestJava, MethodReference) {
+  EXPECT_EQ(
+  "private void foo() {\n"
+  "  f(this::methodReference);\n"
+  "  f(C.super::methodReference);\n"
+  "  Consumer c = System.out::println;\n"
+  "  Iface mRef = Ty::meth;\n"
+  "}",
+  format("private void foo() {\n"
+ "  f(this ::methodReference);\n"
+ "  f(C.super ::methodReference);\n"
+ "  Consumer c = System.out ::println;\n"
+ "  Iface mRef = Ty ::  meth;\n"
+ "}"));
+}
+
 TEST_F(FormatTestJava, CppKeywords) {
   verifyFormat("public void union(Type a, Type b);");
   verifyFormat("public void struct(Object o);");


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

[PATCH] D52842: clang-format: Don't insert spaces in front of :: for Java 8 Method References.

[Phabricator via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL343872: clang-format: Don't insert spaces in front of 
:: for Java 8 Method References. (authored by nico, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52842?vs=168161&id=168495#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52842

Files:
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/unittests/Format/FormatTestJava.cpp


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2555,8 +2555,11 @@
 return false;
   if (Left.is(TT_TemplateCloser) && Left.MatchingParen &&
   Left.MatchingParen->Previous &&
-  Left.MatchingParen->Previous->is(tok::period))
+  (Left.MatchingParen->Previous->is(tok::period) ||
+   Left.MatchingParen->Previous->is(tok::coloncolon)))
+// Java call to generic function with explicit type:
 // A.>>DoSomething();
+// A::>>DoSomething();  // With a Java 8 method reference.
 return false;
   if (Left.is(TT_TemplateCloser) && Right.is(tok::l_square))
 return false;
@@ -2776,6 +2779,9 @@
   if (!Style.SpaceBeforeAssignmentOperators &&
   Right.getPrecedence() == prec::Assignment)
 return false;
+  if (Style.Language == FormatStyle::LK_Java && Right.is(tok::coloncolon) &&
+  (Left.is(tok::identifier) || Left.is(tok::kw_this)))
+return false;
   if (Right.is(tok::coloncolon) && Left.is(tok::identifier))
 // Generally don't remove existing spaces between an identifier and "::".
 // The identifier might actually be a macro name such as ALWAYS_INLINE. If
Index: cfe/trunk/unittests/Format/FormatTestJava.cpp
===
--- cfe/trunk/unittests/Format/FormatTestJava.cpp
+++ cfe/trunk/unittests/Format/FormatTestJava.cpp
@@ -443,6 +443,22 @@
getStyleWithColumns(40));
 }
 
+TEST_F(FormatTestJava, MethodReference) {
+  EXPECT_EQ(
+  "private void foo() {\n"
+  "  f(this::methodReference);\n"
+  "  f(C.super::methodReference);\n"
+  "  Consumer c = System.out::println;\n"
+  "  Iface mRef = Ty::meth;\n"
+  "}",
+  format("private void foo() {\n"
+ "  f(this ::methodReference);\n"
+ "  f(C.super ::methodReference);\n"
+ "  Consumer c = System.out ::println;\n"
+ "  Iface mRef = Ty ::  meth;\n"
+ "}"));
+}
+
 TEST_F(FormatTestJava, CppKeywords) {
   verifyFormat("public void union(Type a, Type b);");
   verifyFormat("public void struct(Object o);");


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2555,8 +2555,11 @@
 return false;
   if (Left.is(TT_TemplateCloser) && Left.MatchingParen &&
   Left.MatchingParen->Previous &&
-  Left.MatchingParen->Previous->is(tok::period))
+  (Left.MatchingParen->Previous->is(tok::period) ||
+   Left.MatchingParen->Previous->is(tok::coloncolon)))
+// Java call to generic function with explicit type:
 // A.>>DoSomething();
+// A::>>DoSomething();  // With a Java 8 method reference.
 return false;
   if (Left.is(TT_TemplateCloser) && Right.is(tok::l_square))
 return false;
@@ -2776,6 +2779,9 @@
   if (!Style.SpaceBeforeAssignmentOperators &&
   Right.getPrecedence() == prec::Assignment)
 return false;
+  if (Style.Language == FormatStyle::LK_Java && Right.is(tok::coloncolon) &&
+  (Left.is(tok::identifier) || Left.is(tok::kw_this)))
+return false;
   if (Right.is(tok::coloncolon) && Left.is(tok::identifier))
 // Generally don't remove existing spaces between an identifier and "::".
 // The identifier might actually be a macro name such as ALWAYS_INLINE. If
Index: cfe/trunk/unittests/Format/FormatTestJava.cpp
===
--- cfe/trunk/unittests/Format/FormatTestJava.cpp
+++ cfe/trunk/unittests/Format/FormatTestJava.cpp
@@ -443,6 +443,22 @@
getStyleWithColumns(40));
 }
 
+TEST_F(FormatTestJava, MethodReference) {
+  EXPECT_EQ(
+  "private void foo() {\n"
+  "  f(this::methodReference);\n"
+  "  f(C.super::methodReference);\n"
+  "  Consumer c = System.out::println;\n"
+  "  Iface mRef = Ty::meth;\n"
+  "}",
+  format("private void foo() {\n"
+ "  f(this ::methodReference);\n"
+ "  f(C.super ::methodReference);\n"
+ "  Consumer c = System.out ::println;\n"
+ "  Iface mRef = Ty ::  meth;\n"
+ "}"));
+}
+
 TEST_F(FormatTestJava, CppKeywords) {
   verifyFormat("public void union(Type 

[PATCH] D52938: [CUDA] Use all 64 bits of GUID in __nv_module_id

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

This particular change is largely cosmetic. I've just spotted this nit while I 
was debugging a different problem.

It's also related to module ID.

We're trying to compile NCCL 2.3 with -fcuda-rdc and we were getting duplicate 
symbols when we tried to link multiple object files compiled from the same 
source file.
E.g.

  clang++ -DPART1 -o foo-1.o  foo.cu
  clang++ -DPART2 -o foo-2.o  foo.cu
  ...
  nvlink 

The stubs generated by nvlink ended up with conflicting names (based on module 
ID) that were identical for all foo-*.o.

It appears that clang generates ID based on the source file name only, so all 
foo-*.o end up with identical ID. 
NVCC, on the other hand, appears to generate ID based on some other factors 
(compiler flags? preprocessed TU source?) so each object file gets a unique ID.

For now we've worked around this by renaming the source files before 
compilation of each part, but we will need to find a better solution.


https://reviews.llvm.org/D52938



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

r343875 - [CUDA] Use all 64 bits of GUID in __nv_module_id

[Artem Belevich via cfe-commits]

Author: tra
Date: Fri Oct  5 11:39:58 2018
New Revision: 343875

URL: http://llvm.org/viewvc/llvm-project?rev=343875&view=rev
Log:
[CUDA] Use all 64 bits of GUID in __nv_module_id

getGUID() returns an uint64_t and "%x" only prints 32 bits of it.
Use PRIx64 format string to print all 64 bits.

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

Modified:
cfe/trunk/lib/CodeGen/CGCUDANV.cpp

Modified: cfe/trunk/lib/CodeGen/CGCUDANV.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCUDANV.cpp?rev=343875&r1=343874&r2=343875&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCUDANV.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCUDANV.cpp Fri Oct  5 11:39:58 2018
@@ -520,7 +520,7 @@ llvm::Function *CGNVCUDARuntime::makeMod
 // Generate a unique module ID.
 SmallString<64> ModuleID;
 llvm::raw_svector_ostream OS(ModuleID);
-OS << ModuleIDPrefix << llvm::format("%x", FatbinWrapper->getGUID());
+OS << ModuleIDPrefix << llvm::format("%" PRIx64, FatbinWrapper->getGUID());
 llvm::Constant *ModuleIDConstant =
 makeConstantString(ModuleID.str(), "", ModuleIDSectionName, 32);
 


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

[PATCH] D52938: [CUDA] Use all 64 bits of GUID in __nv_module_id

[Artem Belevich via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL343875: [CUDA] Use all 64 bits of GUID in __nv_module_id 
(authored by tra, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52938?vs=168489&id=168498#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52938

Files:
  cfe/trunk/lib/CodeGen/CGCUDANV.cpp


Index: cfe/trunk/lib/CodeGen/CGCUDANV.cpp
===
--- cfe/trunk/lib/CodeGen/CGCUDANV.cpp
+++ cfe/trunk/lib/CodeGen/CGCUDANV.cpp
@@ -520,7 +520,7 @@
 // Generate a unique module ID.
 SmallString<64> ModuleID;
 llvm::raw_svector_ostream OS(ModuleID);
-OS << ModuleIDPrefix << llvm::format("%x", FatbinWrapper->getGUID());
+OS << ModuleIDPrefix << llvm::format("%" PRIx64, FatbinWrapper->getGUID());
 llvm::Constant *ModuleIDConstant =
 makeConstantString(ModuleID.str(), "", ModuleIDSectionName, 32);
 


Index: cfe/trunk/lib/CodeGen/CGCUDANV.cpp
===
--- cfe/trunk/lib/CodeGen/CGCUDANV.cpp
+++ cfe/trunk/lib/CodeGen/CGCUDANV.cpp
@@ -520,7 +520,7 @@
 // Generate a unique module ID.
 SmallString<64> ModuleID;
 llvm::raw_svector_ostream OS(ModuleID);
-OS << ModuleIDPrefix << llvm::format("%x", FatbinWrapper->getGUID());
+OS << ModuleIDPrefix << llvm::format("%" PRIx64, FatbinWrapper->getGUID());
 llvm::Constant *ModuleIDConstant =
 makeConstantString(ModuleID.str(), "", ModuleIDSectionName, 32);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, thank you for adding the test and the TODO!


https://reviews.llvm.org/D52918



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

[PATCH] D51809: [CUDA][HIP] Fix ShouldDeleteSpecialMember for inherited constructors

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 168500.
yaxunl added a comment.

fix a typo.


https://reviews.llvm.org/D51809

Files:
  lib/Sema/SemaDeclCXX.cpp
  test/SemaCUDA/implicit-member-target-inherited.cu
  test/SemaCUDA/inherited-ctor.cu

Index: test/SemaCUDA/inherited-ctor.cu
===
--- /dev/null
+++ test/SemaCUDA/inherited-ctor.cu
@@ -0,0 +1,89 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+
+// Inherit a valid non-default ctor.
+namespace NonDefaultCtorValid {
+  struct A {
+A(const int &x) {}
+  };
+
+  struct B : A {
+using A::A;
+  };
+
+  struct C {
+struct B b;
+C() : b(0) {}
+  };
+
+  void test() {
+B b(0);
+C c;
+  }
+}
+
+// Inherit an invalid non-default ctor.
+// The inherited ctor is invalid because it is unable to initialize s.
+namespace NonDefaultCtorInvalid {
+  struct S {
+S() = delete;
+  };
+  struct A {
+A(const int &x) {}
+  };
+
+  struct B : A {
+using A::A;
+S s;
+  };
+
+  struct C {
+struct B b;
+C() : b(0) {} // expected-error{{constructor inherited by 'B' from base class 'A' is implicitly deleted}}
+  // expected-note@-6{{constructor inherited by 'B' is implicitly deleted because field 's' has a deleted corresponding constructor}}
+  // expected-note@-15{{'S' has been explicitly marked deleted here}}
+  };
+}
+
+// Inherit a valid default ctor.
+namespace DefaultCtorValid {
+  struct A {
+A() {}
+  };
+
+  struct B : A {
+using A::A;
+  };
+
+  struct C {
+struct B b;
+C() {}
+  };
+
+  void test() {
+B b;
+C c;
+  }
+}
+
+// Inherit an invalid default ctor.
+// The inherited ctor is invalid because it is unable to initialize s.
+namespace DefaultCtorInvalid {
+  struct S {
+S() = delete;
+  };
+  struct A {
+A() {}
+  };
+
+  struct B : A {
+using A::A;
+S s;
+  };
+
+  struct C {
+struct B b;
+C() {} // expected-error{{call to implicitly-deleted default constructor of 'struct B'}}
+   // expected-note@-6{{default constructor of 'B' is implicitly deleted because field 's' has a deleted default constructor}}
+   // expected-note@-15{{'S' has been explicitly marked deleted here}}
+  };
+}
Index: test/SemaCUDA/implicit-member-target-inherited.cu
===
--- /dev/null
+++ test/SemaCUDA/implicit-member-target-inherited.cu
@@ -0,0 +1,205 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s -Wno-defaulted-function-deleted
+
+#include "Inputs/cuda.h"
+
+//--
+// Test 1: infer inherited default ctor to be host.
+
+struct A1_with_host_ctor {
+  A1_with_host_ctor() {}
+};
+// expected-note@-3 {{candidate constructor (the implicit copy constructor) not viable}}
+// expected-note@-4 {{candidate constructor (the implicit move constructor) not viable}}
+
+// The inherited default constructor is inferred to be host, so we'll encounter
+// an error when calling it from a __device__ function, but not from a __host__
+// function.
+struct B1_with_implicit_default_ctor : A1_with_host_ctor {
+  using A1_with_host_ctor::A1_with_host_ctor;
+};
+
+// expected-note@-4 {{call to __host__ function from __device__}}
+// expected-note@-5 {{candidate constructor (the implicit copy constructor) not viable}}
+// expected-note@-6 {{candidate constructor (the implicit move constructor) not viable}}
+// expected-note@-6 2{{constructor from base class 'A1_with_host_ctor' inherited here}}
+
+void hostfoo() {
+  B1_with_implicit_default_ctor b;
+}
+
+__device__ void devicefoo() {
+  B1_with_implicit_default_ctor b; // expected-error {{no matching constructor}}
+}
+
+//--
+// Test 2: infer inherited default ctor to be device.
+
+struct A2_with_device_ctor {
+  __device__ A2_with_device_ctor() {}
+};
+// expected-note@-3 {{candidate constructor (the implicit copy constructor) not viable}}
+// expected-note@-4 {{candidate constructor (the implicit move constructor) not viable}}
+
+struct B2_with_implicit_default_ctor : A2_with_device_ctor {
+  using A2_with_device_ctor::A2_with_device_ctor;
+};
+
+// expected-note@-4 {{call to __device__ function from __host__}}
+// expected-note@-5 {{candidate constructor (the implicit copy constructor) not viable}}
+// expected-note@-6 {{candidate constructor (the implicit move constructor) not viable}}
+// expected-note@-6 2{{constructor from base class 'A2_with_device_ctor' inherited here}}
+
+void hostfoo2() {
+  B2_with_implicit_default_ctor b;  // expected-error {{no matching constructor}}
+}
+
+__device__ void devicefoo2() {
+  B2_with_implicit_default_ctor b;
+}
+
+//--
+// Test 3: infer inherited copy ctor
+
+struct A3_with_device_ctors {
+  __host__ A3_with_device_ctors

[PATCH] D52807: [COFF, ARM64] Add _InterlockedCompareExchangePointer_nf intrinsic

[Eli Friedman via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D52807



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

[PATCH] D52395: Thread safety analysis: Require exclusive lock for passing by non-const reference

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert updated this revision to Diff 168505.
aaronpuchert added a comment.

Rebase on top of https://reviews.llvm.org/D52443. We also check the move 
constructor argument for write access, as suggested in a review.

This isn't intended to be merged (yet?), it should be seen as an RFC.


Repository:
  rC Clang

https://reviews.llvm.org/D52395

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Analysis/ThreadSafety.cpp
  test/SemaCXX/warn-thread-safety-analysis.cpp

Index: test/SemaCXX/warn-thread-safety-analysis.cpp
===
--- test/SemaCXX/warn-thread-safety-analysis.cpp
+++ test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -5058,27 +5058,27 @@
 
   void test1() {
 copy(foo); // expected-warning {{reading variable 'foo' requires holding mutex 'mu'}}
-write1(foo);   // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
-write2(10, foo);   // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
+write1(foo);   // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
+write2(10, foo);   // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
 read1(foo);// expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
 read2(10, foo);// expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
-destroy(mymove(foo));  // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
+destroy(mymove(foo));  // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
 
 copyVariadic(foo); // expected-warning {{reading variable 'foo' requires holding mutex 'mu'}}
 readVariadic(foo); // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
-writeVariadic(foo);// expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
+writeVariadic(foo);// expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
 copyVariadicC(1, foo); // expected-warning {{reading variable 'foo' requires holding mutex 'mu'}}
 
 FooRead reader(foo);   // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
-FooWrite writer(foo);  // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
+FooWrite writer(foo);  // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
 
-mwrite1(foo);   // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
-mwrite2(10, foo);   // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
+mwrite1(foo);   // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
+mwrite2(10, foo);   // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
 mread1(foo);// expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
 mread2(10, foo);// expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
 
-smwrite1(foo);   // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
-smwrite2(10, foo);   // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
+smwrite1(foo);   // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
+smwrite2(10, foo);   // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
 smread1(foo);// expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
 smread2(10, foo);// expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
 
@@ -5094,12 +5094,13 @@
 (*this) << foo;  // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
 
 copy(*foop); // expected-warning {{reading the value pointed to by 'foop' requires holding mutex 'mu'}}
-write1(*foop);   // expected-warning {{passing the value that 'foop' points to by reference requires holding mutex 'mu'}}
-write2(10, *foop);   // expected-warning {{passing the value that 'foop' points to by reference requires holding mutex 'mu'}}
+write1(*foop);   // expected-warning {{passing the value that 'foop' points to by non-const reference requires holding mutex 'mu' exclusively}}
+write2

[PATCH] D52578: Thread safety analysis: Allow scoped releasing of capabilities

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert planned changes to this revision.
aaronpuchert added a comment.

I think I'll try to simplify this and address @delesley's comments before we 
commit this. I'll admit that the semantics are somewhat counter-intuitive, but 
as I explained I think it's more consistent this way. Because the scoped unlock 
releases a lock on construction, the operation on the underlying lock mirrors 
the operation on the scoped capability: releasing the scoped capability (on 
destruction, for example) acquires the lock again.


Repository:
  rC Clang

https://reviews.llvm.org/D52578



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

r343881 - [COFF, ARM64] Add _InterlockedCompareExchangePointer_nf intrinsic

[Mandeep Singh Grang via cfe-commits]

Author: mgrang
Date: Fri Oct  5 12:49:36 2018
New Revision: 343881

URL: http://llvm.org/viewvc/llvm-project?rev=343881&view=rev
Log:
[COFF, ARM64] Add _InterlockedCompareExchangePointer_nf intrinsic

Reviewers: rnk, mstorsjo, compnerd, TomTan, haripul, efriedma

Reviewed By: efriedma

Subscribers: efriedma, kristof.beyls, chrib, jfb, cfe-commits

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

Modified:
cfe/trunk/include/clang/Basic/Builtins.def
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/test/CodeGen/ms-intrinsics.c

Modified: cfe/trunk/include/clang/Basic/Builtins.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=343881&r1=343880&r2=343881&view=diff
==
--- cfe/trunk/include/clang/Basic/Builtins.def (original)
+++ cfe/trunk/include/clang/Basic/Builtins.def Fri Oct  5 12:49:36 2018
@@ -786,6 +786,7 @@ LANGBUILTIN(_InterlockedCompareExchange1
 LANGBUILTIN(_InterlockedCompareExchange,"NiNiD*NiNi", "n", 
ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedCompareExchange64,  "LLiLLiD*LLiLLi", "n", 
ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedCompareExchangePointer, "v*v*D*v*v*", "n", 
ALL_MS_LANGUAGES)
+LANGBUILTIN(_InterlockedCompareExchangePointer_nf, "v*v*D*v*v*", "n", 
ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedDecrement16,"ssD*", "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedDecrement,  "NiNiD*",   "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedExchange,   "NiNiD*Ni", "n", 
ALL_MS_LANGUAGES)

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=343881&r1=343880&r2=343881&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Fri Oct  5 12:49:36 2018
@@ -2999,7 +2999,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(
   case Builtin::BI_InterlockedExchangePointer:
 return RValue::get(
 EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchange, E));
-  case Builtin::BI_InterlockedCompareExchangePointer: {
+  case Builtin::BI_InterlockedCompareExchangePointer:
+  case Builtin::BI_InterlockedCompareExchangePointer_nf: {
 llvm::Type *RTy;
 llvm::IntegerType *IntType =
   IntegerType::get(getLLVMContext(),
@@ -3016,10 +3017,12 @@ RValue CodeGenFunction::EmitBuiltinExpr(
 llvm::Value *Comparand =
   Builder.CreatePtrToInt(EmitScalarExpr(E->getArg(2)), IntType);
 
-auto Result =
-Builder.CreateAtomicCmpXchg(Destination, Comparand, Exchange,
-AtomicOrdering::SequentiallyConsistent,
-AtomicOrdering::SequentiallyConsistent);
+auto Ordering =
+  BuiltinID == Builtin::BI_InterlockedCompareExchangePointer_nf ?
+  AtomicOrdering::Monotonic : AtomicOrdering::SequentiallyConsistent;
+
+auto Result = Builder.CreateAtomicCmpXchg(Destination, Comparand, Exchange,
+  Ordering, Ordering);
 Result->setVolatile(true);
 
 return 
RValue::get(Builder.CreateIntToPtr(Builder.CreateExtractValue(Result,

Modified: cfe/trunk/test/CodeGen/ms-intrinsics.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-intrinsics.c?rev=343881&r1=343880&r2=343881&view=diff
==
--- cfe/trunk/test/CodeGen/ms-intrinsics.c (original)
+++ cfe/trunk/test/CodeGen/ms-intrinsics.c Fri Oct  5 12:49:36 2018
@@ -235,6 +235,21 @@ void *test_InterlockedCompareExchangePoi
 // CHECK:   ret i8* %[[RESULT:[0-9]+]]
 // CHECK: }
 
+void *test_InterlockedCompareExchangePointer_nf(void * volatile *Destination,
+ void *Exchange, void *Comparand) {
+  return _InterlockedCompareExchangePointer_nf(Destination, Exchange, 
Comparand);
+}
+
+// CHECK: define{{.*}}i8* @test_InterlockedCompareExchangePointer_nf(i8** 
{{[a-z_ ]*}}%Destination, i8* {{[a-z_ ]*}}%Exchange, i8* {{[a-z_ 
]*}}%Comparand){{.*}}{
+// CHECK:   %[[DEST:[0-9]+]] = bitcast i8** %Destination to [[iPTR]]*
+// CHECK:   %[[EXCHANGE:[0-9]+]] = ptrtoint i8* %Exchange to [[iPTR]]
+// CHECK:   %[[COMPARAND:[0-9]+]] = ptrtoint i8* %Comparand to [[iPTR]]
+// CHECK:   %[[XCHG:[0-9]+]] = cmpxchg volatile [[iPTR]]* %[[DEST:[0-9]+]], 
[[iPTR]] %[[COMPARAND:[0-9]+]], [[iPTR]] %[[EXCHANGE:[0-9]+]] monotonic 
monotonic
+// CHECK:   %[[EXTRACT:[0-9]+]] = extractvalue { [[iPTR]], i1 } %[[XCHG]], 0
+// CHECK:   %[[RESULT:[0-9]+]] = inttoptr [[iPTR]] %[[EXTRACT]] to i8*
+// CHECK:   ret i8* %[[RESULT:[0-9]+]]
+// CHECK: }
+
 char test_InterlockedExchange8(char volatile *value, char mask) {
   return _InterlockedExchange8(value, mask);
 }


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/

[PATCH] D52807: [COFF, ARM64] Add _InterlockedCompareExchangePointer_nf intrinsic

[Mandeep Singh Grang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC343881: [COFF, ARM64] Add 
_InterlockedCompareExchangePointer_nf intrinsic (authored by mgrang, committed 
by ).

Repository:
  rC Clang

https://reviews.llvm.org/D52807

Files:
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/ms-intrinsics.c


Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -2999,7 +2999,8 @@
   case Builtin::BI_InterlockedExchangePointer:
 return RValue::get(
 EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchange, E));
-  case Builtin::BI_InterlockedCompareExchangePointer: {
+  case Builtin::BI_InterlockedCompareExchangePointer:
+  case Builtin::BI_InterlockedCompareExchangePointer_nf: {
 llvm::Type *RTy;
 llvm::IntegerType *IntType =
   IntegerType::get(getLLVMContext(),
@@ -3016,10 +3017,12 @@
 llvm::Value *Comparand =
   Builder.CreatePtrToInt(EmitScalarExpr(E->getArg(2)), IntType);
 
-auto Result =
-Builder.CreateAtomicCmpXchg(Destination, Comparand, Exchange,
-AtomicOrdering::SequentiallyConsistent,
-AtomicOrdering::SequentiallyConsistent);
+auto Ordering =
+  BuiltinID == Builtin::BI_InterlockedCompareExchangePointer_nf ?
+  AtomicOrdering::Monotonic : AtomicOrdering::SequentiallyConsistent;
+
+auto Result = Builder.CreateAtomicCmpXchg(Destination, Comparand, Exchange,
+  Ordering, Ordering);
 Result->setVolatile(true);
 
 return 
RValue::get(Builder.CreateIntToPtr(Builder.CreateExtractValue(Result,
Index: include/clang/Basic/Builtins.def
===
--- include/clang/Basic/Builtins.def
+++ include/clang/Basic/Builtins.def
@@ -786,6 +786,7 @@
 LANGBUILTIN(_InterlockedCompareExchange,"NiNiD*NiNi", "n", 
ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedCompareExchange64,  "LLiLLiD*LLiLLi", "n", 
ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedCompareExchangePointer, "v*v*D*v*v*", "n", 
ALL_MS_LANGUAGES)
+LANGBUILTIN(_InterlockedCompareExchangePointer_nf, "v*v*D*v*v*", "n", 
ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedDecrement16,"ssD*", "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedDecrement,  "NiNiD*",   "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedExchange,   "NiNiD*Ni", "n", 
ALL_MS_LANGUAGES)
Index: test/CodeGen/ms-intrinsics.c
===
--- test/CodeGen/ms-intrinsics.c
+++ test/CodeGen/ms-intrinsics.c
@@ -235,6 +235,21 @@
 // CHECK:   ret i8* %[[RESULT:[0-9]+]]
 // CHECK: }
 
+void *test_InterlockedCompareExchangePointer_nf(void * volatile *Destination,
+ void *Exchange, void *Comparand) {
+  return _InterlockedCompareExchangePointer_nf(Destination, Exchange, 
Comparand);
+}
+
+// CHECK: define{{.*}}i8* @test_InterlockedCompareExchangePointer_nf(i8** 
{{[a-z_ ]*}}%Destination, i8* {{[a-z_ ]*}}%Exchange, i8* {{[a-z_ 
]*}}%Comparand){{.*}}{
+// CHECK:   %[[DEST:[0-9]+]] = bitcast i8** %Destination to [[iPTR]]*
+// CHECK:   %[[EXCHANGE:[0-9]+]] = ptrtoint i8* %Exchange to [[iPTR]]
+// CHECK:   %[[COMPARAND:[0-9]+]] = ptrtoint i8* %Comparand to [[iPTR]]
+// CHECK:   %[[XCHG:[0-9]+]] = cmpxchg volatile [[iPTR]]* %[[DEST:[0-9]+]], 
[[iPTR]] %[[COMPARAND:[0-9]+]], [[iPTR]] %[[EXCHANGE:[0-9]+]] monotonic 
monotonic
+// CHECK:   %[[EXTRACT:[0-9]+]] = extractvalue { [[iPTR]], i1 } %[[XCHG]], 0
+// CHECK:   %[[RESULT:[0-9]+]] = inttoptr [[iPTR]] %[[EXTRACT]] to i8*
+// CHECK:   ret i8* %[[RESULT:[0-9]+]]
+// CHECK: }
+
 char test_InterlockedExchange8(char volatile *value, char mask) {
   return _InterlockedExchange8(value, mask);
 }


Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -2999,7 +2999,8 @@
   case Builtin::BI_InterlockedExchangePointer:
 return RValue::get(
 EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchange, E));
-  case Builtin::BI_InterlockedCompareExchangePointer: {
+  case Builtin::BI_InterlockedCompareExchangePointer:
+  case Builtin::BI_InterlockedCompareExchangePointer_nf: {
 llvm::Type *RTy;
 llvm::IntegerType *IntType =
   IntegerType::get(getLLVMContext(),
@@ -3016,10 +3017,12 @@
 llvm::Value *Comparand =
   Builder.CreatePtrToInt(EmitScalarExpr(E->getArg(2)), IntType);
 
-auto Result =
-Builder.CreateAtomicCmpXchg(Destination, Comparand, Exchange,
-AtomicOrdering::SequentiallyConsistent,
-AtomicOrdering::SequentiallyConsistent);
+auto Ordering =
+  BuiltinID == Builtin::BI_InterlockedCompareExchangePointer_nf ?
+  AtomicOrder

[PATCH] D52807: [COFF, ARM64] Add _InterlockedCompareExchangePointer_nf intrinsic

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: lib/CodeGen/CGBuiltin.cpp:3003
+  case Builtin::BI_InterlockedCompareExchangePointer:
+  case Builtin::BI_InterlockedCompareExchangePointer_nf: {
 llvm::Type *RTy;

efriedma wrote:
> efriedma wrote:
> > rnk wrote:
> > > mgrang wrote:
> > > > rnk wrote:
> > > > > Is the no fence version really equivalent to this seq_cst version 
> > > > > here?
> > > > I don't see InterlockedCompareExchangePointer creating a fence but it 
> > > > should. (since it is the fence version). So maybe that needs to be 
> > > > fixed (and possibly other fence functions).
> > > > 
> > > > InterlockedCompareExchangePointer_nf should not create a fence so the 
> > > > current implementation seems correct.
> > > Hm, if we're supposed to have fences, we're probably missing them 
> > > everywhere. I thought the atomicrmw orderings were enough, but that shows 
> > > what I know about the C++ memory model. =p
> > > 
> > > We don't generate a fence for this intrinsic when MSVC does:
> > > ```
> > > unsigned char test_arm(long *base, long idx) {
> > >   return _interlockedbittestandreset_acq(base, idx);
> > > }
> > > ```
> > > 
> > > @efriedma, is MSVC over-emitting extra fences, or are we under-emitting? 
> > > If this is their "bug", should we be bug-for-bug compatible with it so we 
> > > get the same observable memory states?
> > "create a fence" is a little confusing because the AArch64 ldaxr/stlxr have 
> > builtin fences... but presumably the "nf" version should use ldxr/stxr 
> > instead.  At the IR level, that corresponds to "monotonic".
> In terms of emitting fences, maybe we are actually missing a fence.
> 
> An ldaxr+stlxr pair isn't a complete fence, at least in theory; it stops 
> loads from being hosted past it, and stores from being sunk past it, but 
> stores can be hoisted and loads can be sunk.  That said, a stronger fence 
> isn't necessary to model C++11 atomics; C++11 only requires sequential 
> consistency with other sequentially consistent operations, plus an 
> acquire/release barrier.  And a program that isn't using C++11 relaxed 
> atomics can't depend on a stronger barrier without triggering a data race.
> 
> A program written before C++11 atomics were generally available could 
> theoretically depend on the barrier through a series of operations that cause 
> a data race under the C++11 rules.  Since there were no clear rules before 
> C++11, programs did things like emulate relaxed atomics on top of volatile 
> pointers; using such a construct, a program could depend on the implied 
> barrier.  For this reason, gcc emits a fence on AArch64 after `__sync_*` (but 
> not `__atomic_*`).  And I guess MSVC does something similar for 
> `_Interlocked*`.
> 
> That said, LLVM has never emitted a dmb for `__sync_*` operations on AArch64, 
> and it hasn't led to any practical problems as far as I know.
> it stops loads from being hosted past it, and stores from being sunk past it, 
> but stores can be hoisted and loads can be sunk.

Looking over it again, this bit is a really bad description of what actually 
happens. Really there are two issues: one, an operation from after an 
ldaxr/stlxr pair could end up betwen the ldaxr and stlxr, and two, a cmpxchg 
doesn't always execute the stlxr.


Repository:
  rC Clang

https://reviews.llvm.org/D52807



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

[PATCH] D52578: Thread safety analysis: Allow scoped releasing of capabilities

[Victor Costan via Phabricator via cfe-commits]

pwnall added a comment.

Thank you for clarifying, Aaron!

I probably used Phabricator incorrectly. My intent was to state that the tests 
LGTM and express support for the functionality in this patch. Please definitely 
address all review comments before landing.


Repository:
  rC Clang

https://reviews.llvm.org/D52578



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

[PATCH] D52811: [COFF, ARM64] Add _InterlockedAdd intrinsic

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang updated this revision to Diff 168517.
mgrang added a comment.
Herald added a reviewer: javed.absar.

Limited the intrinsic only for AArch64 and fixed the implementation to return 
the sum instead of the old value of the Addend. Thanks @efriedma for the 
pointers.


https://reviews.llvm.org/D52811

Files:
  include/clang/Basic/BuiltinsAArch64.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/intrin.h
  test/CodeGen/arm64-microsoft-intrinsics.c


Index: test/CodeGen/arm64-microsoft-intrinsics.c
===
--- test/CodeGen/arm64-microsoft-intrinsics.c
+++ test/CodeGen/arm64-microsoft-intrinsics.c
@@ -4,6 +4,19 @@
 // RUN: not %clang_cc1 -triple arm64-linux -Werror -S -o /dev/null %s 2>&1 \
 // RUN:| FileCheck %s -check-prefix CHECK-LINUX
 
+long test_InterlockedAdd(long volatile *Addend, long Value) {
+  return _InterlockedAdd(Addend, Value);
+}
+
+// CHECK-LABEL: define {{.*}} i32 @test_InterlockedAdd(i32* %Addend, i32 
%Value) {{.*}} {
+// CHECK-MSVC: [[ADDEND:%[0-9]+]] = load i32*, i32** %Addend.addr, align 8
+// CHECK-MSVC: [[VALUE:%[0-9]+]] = load i32, i32* %Value.addr, align 4
+// CHECK-MSVC: [[ATOMICADD:%[0-9]+]] = atomicrmw add i32* [[ADDEND:%[0-9]+]], 
i32 [[VALUE:%[0-9]+]] seq_cst
+// CHECK-MSVC: [[VALUE:%[0-9]+]] = load i32, i32* %Value.addr, align 4
+// CHECK-MSVC: [[RESULT:%[0-9]+]] = add i32 [[ATOMICADD:%[0-9]+]], 
[[VALUE:%[0-9]+]]
+// CHECK-MSVC: ret i32 [[RESULT:%[0-9]+]]
+// CHECK-LINUX: error: implicit declaration of function '_InterlockedAdd'
+
 void check__dmb(void) {
   __dmb(0);
 }
Index: lib/Headers/intrin.h
===
--- lib/Headers/intrin.h
+++ lib/Headers/intrin.h
@@ -869,6 +869,7 @@
 
\**/
 #if defined(__aarch64__)
 unsigned __int64 __getReg(int);
+long _InterlockedAdd(long volatile *Addend, long Value);
 #endif
 
 
/**\
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -8514,6 +8514,11 @@
 return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedDecrement, E);
   case AArch64::BI_InterlockedIncrement64:
 return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedIncrement, E);
+
+  case AArch64::BI_InterlockedAdd: {
+Value *RMWI = MakeBinaryAtomicValue(*this, AtomicRMWInst::Add, E);
+return Builder.CreateAdd(RMWI, EmitScalarExpr(E->getArg(1)));
+  }
   }
 }
 
Index: include/clang/Basic/BuiltinsAArch64.def
===
--- include/clang/Basic/BuiltinsAArch64.def
+++ include/clang/Basic/BuiltinsAArch64.def
@@ -94,6 +94,7 @@
 TARGET_HEADER_BUILTIN(_BitScanForward64, "UcUNi*ULLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_BitScanReverse64, "UcUNi*ULLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 
+TARGET_HEADER_BUILTIN(_InterlockedAdd,   "LiLiD*Li","nh", 
"intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedAnd64, "LLiLLiD*LLi", "nh", 
"intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedDecrement64,   "LLiLLiD*","nh", 
"intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedExchange64,"LLiLLiD*LLi", "nh", 
"intrin.h", ALL_MS_LANGUAGES, "")


Index: test/CodeGen/arm64-microsoft-intrinsics.c
===
--- test/CodeGen/arm64-microsoft-intrinsics.c
+++ test/CodeGen/arm64-microsoft-intrinsics.c
@@ -4,6 +4,19 @@
 // RUN: not %clang_cc1 -triple arm64-linux -Werror -S -o /dev/null %s 2>&1 \
 // RUN:| FileCheck %s -check-prefix CHECK-LINUX
 
+long test_InterlockedAdd(long volatile *Addend, long Value) {
+  return _InterlockedAdd(Addend, Value);
+}
+
+// CHECK-LABEL: define {{.*}} i32 @test_InterlockedAdd(i32* %Addend, i32 %Value) {{.*}} {
+// CHECK-MSVC: [[ADDEND:%[0-9]+]] = load i32*, i32** %Addend.addr, align 8
+// CHECK-MSVC: [[VALUE:%[0-9]+]] = load i32, i32* %Value.addr, align 4
+// CHECK-MSVC: [[ATOMICADD:%[0-9]+]] = atomicrmw add i32* [[ADDEND:%[0-9]+]], i32 [[VALUE:%[0-9]+]] seq_cst
+// CHECK-MSVC: [[VALUE:%[0-9]+]] = load i32, i32* %Value.addr, align 4
+// CHECK-MSVC: [[RESULT:%[0-9]+]] = add i32 [[ATOMICADD:%[0-9]+]], [[VALUE:%[0-9]+]]
+// CHECK-MSVC: ret i32 [[RESULT:%[0-9]+]]
+// CHECK-LINUX: error: implicit declaration of function '_InterlockedAdd'
+
 void check__dmb(void) {
   __dmb(0);
 }
Index: lib/Headers/intrin.h
===
--- lib/Headers/intrin.h
+++ lib/Headers/intrin.h
@@ -869,6 +869,7 @@
 \**/
 #if defined(__aarch64__)
 unsigned __int64 __getReg(int);
+long _InterlockedAdd(long volatile *Addend, long Value);
 #endif
 
 /*

[PATCH] D52811: [COFF, ARM64] Add _InterlockedAdd intrinsic

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang updated this revision to Diff 168518.

https://reviews.llvm.org/D52811

Files:
  include/clang/Basic/BuiltinsAArch64.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/intrin.h
  test/CodeGen/arm64-microsoft-intrinsics.c


Index: test/CodeGen/arm64-microsoft-intrinsics.c
===
--- test/CodeGen/arm64-microsoft-intrinsics.c
+++ test/CodeGen/arm64-microsoft-intrinsics.c
@@ -4,6 +4,16 @@
 // RUN: not %clang_cc1 -triple arm64-linux -Werror -S -o /dev/null %s 2>&1 \
 // RUN:| FileCheck %s -check-prefix CHECK-LINUX
 
+long test_InterlockedAdd(long volatile *Addend, long Value) {
+  return _InterlockedAdd(Addend, Value);
+}
+
+// CHECK-LABEL: define {{.*}} i32 @test_InterlockedAdd(i32* %Addend, i32 
%Value) {{.*}} {
+// CHECK-MSVC: %[[OLDVAL:[0-9]+]] = atomicrmw add i32* %1, i32 %2 seq_cst
+// CHECK-MSVC: %[[NEWVAL:[0-9]+]] = add i32 %[[OLDVAL:[0-9]+]], %2
+// CHECK-MSVC: ret i32 %[[NEWVAL:[0-9]+]]
+// CHECK-LINUX: error: implicit declaration of function '_InterlockedAdd'
+
 void check__dmb(void) {
   __dmb(0);
 }
Index: lib/Headers/intrin.h
===
--- lib/Headers/intrin.h
+++ lib/Headers/intrin.h
@@ -869,6 +869,7 @@
 
\**/
 #if defined(__aarch64__)
 unsigned __int64 __getReg(int);
+long _InterlockedAdd(long volatile *Addend, long Value);
 #endif
 
 
/**\
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -8514,6 +8514,15 @@
 return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedDecrement, E);
   case AArch64::BI_InterlockedIncrement64:
 return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedIncrement, E);
+
+  case AArch64::BI_InterlockedAdd: {
+Value *Arg0 = EmitScalarExpr(E->getArg(0));
+Value *Arg1 = EmitScalarExpr(E->getArg(1));
+AtomicRMWInst *RMWI = Builder.CreateAtomicRMW(
+  AtomicRMWInst::Add, Arg0, Arg1,
+  llvm::AtomicOrdering::SequentiallyConsistent);
+return Builder.CreateAdd(RMWI, Arg1);
+  }
   }
 }
 
Index: include/clang/Basic/BuiltinsAArch64.def
===
--- include/clang/Basic/BuiltinsAArch64.def
+++ include/clang/Basic/BuiltinsAArch64.def
@@ -94,6 +94,7 @@
 TARGET_HEADER_BUILTIN(_BitScanForward64, "UcUNi*ULLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_BitScanReverse64, "UcUNi*ULLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 
+TARGET_HEADER_BUILTIN(_InterlockedAdd,   "LiLiD*Li","nh", 
"intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedAnd64, "LLiLLiD*LLi", "nh", 
"intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedDecrement64,   "LLiLLiD*","nh", 
"intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedExchange64,"LLiLLiD*LLi", "nh", 
"intrin.h", ALL_MS_LANGUAGES, "")


Index: test/CodeGen/arm64-microsoft-intrinsics.c
===
--- test/CodeGen/arm64-microsoft-intrinsics.c
+++ test/CodeGen/arm64-microsoft-intrinsics.c
@@ -4,6 +4,16 @@
 // RUN: not %clang_cc1 -triple arm64-linux -Werror -S -o /dev/null %s 2>&1 \
 // RUN:| FileCheck %s -check-prefix CHECK-LINUX
 
+long test_InterlockedAdd(long volatile *Addend, long Value) {
+  return _InterlockedAdd(Addend, Value);
+}
+
+// CHECK-LABEL: define {{.*}} i32 @test_InterlockedAdd(i32* %Addend, i32 %Value) {{.*}} {
+// CHECK-MSVC: %[[OLDVAL:[0-9]+]] = atomicrmw add i32* %1, i32 %2 seq_cst
+// CHECK-MSVC: %[[NEWVAL:[0-9]+]] = add i32 %[[OLDVAL:[0-9]+]], %2
+// CHECK-MSVC: ret i32 %[[NEWVAL:[0-9]+]]
+// CHECK-LINUX: error: implicit declaration of function '_InterlockedAdd'
+
 void check__dmb(void) {
   __dmb(0);
 }
Index: lib/Headers/intrin.h
===
--- lib/Headers/intrin.h
+++ lib/Headers/intrin.h
@@ -869,6 +869,7 @@
 \**/
 #if defined(__aarch64__)
 unsigned __int64 __getReg(int);
+long _InterlockedAdd(long volatile *Addend, long Value);
 #endif
 
 /**\
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -8514,6 +8514,15 @@
 return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedDecrement, E);
   case AArch64::BI_InterlockedIncrement64:
 return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedIncrement, E);
+
+  case AArch64::BI_InterlockedAdd: {
+Value *Arg0 = EmitScalarExpr(E->getArg(0));
+Value *Arg1 = EmitScalarExpr(E->getArg(1));
+AtomicRMWInst *RMWI = Builder.CreateAtomicRMW(
+  AtomicRM

[PATCH] D52811: [COFF, ARM64] Add _InterlockedAdd intrinsic

[Eli Friedman via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D52811



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

r343883 - [DebugInfo] Add support for DWARF5 call site-related attributes

[Vedant Kumar via cfe-commits]

Author: vedantk
Date: Fri Oct  5 13:37:17 2018
New Revision: 343883

URL: http://llvm.org/viewvc/llvm-project?rev=343883&view=rev
Log:
[DebugInfo] Add support for DWARF5 call site-related attributes

DWARF v5 introduces DW_AT_call_all_calls, a subprogram attribute which
indicates that all calls (both regular and tail) within the subprogram
have call site entries. The information within these call site entries
can be used by a debugger to populate backtraces with synthetic tail
call frames.

Tail calling frames go missing in backtraces because the frame of the
caller is reused by the callee. Call site entries allow a debugger to
reconstruct a sequence of (tail) calls which led from one function to
another. This improves backtrace quality. There are limitations: tail
recursion isn't handled, variables within synthetic frames may not
survive to be inspected, etc. This approach is not novel, see:

  
https://gcc.gnu.org/wiki/summit2010?action=AttachFile&do=get&target=jelinek.pdf

This patch adds an IR-level flag (DIFlagAllCallsDescribed) which lowers
to DW_AT_call_all_calls. It adds the minimal amount of DWARF generation
support needed to emit standards-compliant call site entries. For easier
deployment, when the debugger tuning is LLDB, the DWARF requirement is
adjusted to v4.

Testing: Apart from check-{llvm, clang}, I built a stage2 RelWithDebInfo
clang binary. Its dSYM passed verification and grew by 1.4% compared to
the baseline. 151,879 call site entries were added.

rdar://42001377

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

Added:
cfe/trunk/test/CodeGenCXX/dbg-info-all-calls-described.cpp
Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.h

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=343883&r1=343882&r2=343883&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Fri Oct  5 13:37:17 2018
@@ -3168,6 +3168,7 @@ llvm::DISubprogram *CGDebugInfo::getFunc
   QualType FnType = CGM.getContext().getFunctionType(
   FD->getReturnType(), ArgTypes, FunctionProtoType::ExtProtoInfo(CC));
   if (Stub) {
+Flags |= getCallSiteRelatedAttrs();
 return DBuilder.createFunction(
 DContext, Name, LinkageName, Unit, Line,
 getOrCreateFunctionType(GD.getDecl(), FnType, Unit),
@@ -3407,6 +3408,8 @@ void CGDebugInfo::EmitFunctionStart(Glob
   if (CurFuncIsThunk)
 Flags |= llvm::DINode::FlagThunk;
 
+  llvm::DINode::DIFlags FlagsForDef = Flags | getCallSiteRelatedAttrs();
+
   unsigned LineNo = getLineNumber(Loc);
   unsigned ScopeLine = getLineNumber(ScopeLoc);
 
@@ -3418,7 +3421,7 @@ void CGDebugInfo::EmitFunctionStart(Glob
   llvm::DISubprogram *SP = DBuilder.createFunction(
   FDContext, Name, LinkageName, Unit, LineNo,
   getOrCreateFunctionType(D, FnType, Unit), Fn->hasLocalLinkage(),
-  true /*definition*/, ScopeLine, Flags, CGM.getLangOpts().Optimize,
+  true /*definition*/, ScopeLine, FlagsForDef, CGM.getLangOpts().Optimize,
   TParamsArray.get(), getFunctionDeclaration(D));
   Fn->setSubprogram(SP);
   // We might get here with a VarDecl in the case we're generating
@@ -4422,3 +4425,22 @@ llvm::DebugLoc CGDebugInfo::SourceLocToD
   llvm::MDNode *Scope = LexicalBlockStack.back();
   return llvm::DebugLoc::get(getLineNumber(Loc), getColumnNumber(Loc), Scope);
 }
+
+llvm::DINode::DIFlags CGDebugInfo::getCallSiteRelatedAttrs() const {
+  // Call site-related attributes are only useful in optimized programs, and
+  // when there's a possibility of debugging backtraces.
+  if (!CGM.getLangOpts().Optimize || DebugKind == codegenoptions::NoDebugInfo 
||
+  DebugKind == codegenoptions::LocTrackingOnly)
+return llvm::DINode::FlagZero;
+
+  // Call site-related attributes are available in DWARF v5. Some debuggers,
+  // while not fully DWARF v5-compliant, may accept these attributes as if they
+  // were part of DWARF v4.
+  bool SupportsDWARFv4Ext =
+  CGM.getCodeGenOpts().DwarfVersion == 4 &&
+  CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::LLDB;
+  if (!SupportsDWARFv4Ext && CGM.getCodeGenOpts().DwarfVersion < 5)
+return llvm::DINode::FlagZero;
+
+  return llvm::DINode::FlagAllCallsDescribed;
+}

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.h?rev=343883&r1=343882&r2=343883&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.h (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.h Fri Oct  5 13:37:17 2018
@@ -608,6 +608,11 @@ private:
  unsigned LineNo, StringRef LinkageName,
  llvm::GlobalVariable *Var, llvm::DIScope *DContext);
 
+
+  /// Return flags which enable debug info e

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

In https://reviews.llvm.org/D52193#1243456, @thakis wrote:

> ...and to reword this a bit: Clang taking a long time to start up in some 
> configurations is a bug we should profile and fix :-)


This is time spent in `ntdll.dll` loading various low-level libraries like 
`kernel32.dll`, `KernelBase.dll`, `combase.dll` and so on. So at least 20ms 
just getting to main(). Then, about ~10ms spent loading `dbghelp.dll` while 
installing the exception handler in `llvm::RegisterHandler`. So a lot of wasted 
time. The easiest would be to just not invoke a new child process!



New timings without the child `clang-cl.exe` being invoked (hacked from 
https://reviews.llvm.org/D52411). The improvement **is significant**. Tested at 
r343846.

**Config 1:** Intel Xeon Haswell 6 cores / 12 HW threads, 3.5 GHz, 15M cache, 
128 GB RAM, SSD 550 MB/s

__Ninja:__

| MSVC cl  | (37min 19sec)  
   |
| clang-cl //(built with Clang)//  | //(in progress-will update a bit 
later)// |
| clang-cl no child //(built with Clang)// | //(in progress-will update a bit 
later)// |
|

**Config 2:** Intel Xeon Skylake 18 cores / 36 HW threads, x2 (Dual CPU), 72 HW 
threads total, 2.3 GHz, 24.75M cache, 128 GB RAM, NVMe SSD 4.6 GB/s

__Ninja:__

| MSVC cl  | (7min 33sec)|
| clang-cl //(built with Clang)//  | (9min 2sec) |
| **clang-cl no child //(built with Clang)//** | **(7min 9sec)** |
|

This asks whether the improvement will be of the same order, if invoking just 
one `clang-cl.exe` for the whole compilation process. A sort of local 
compilation-as-a-service.
In that scenario, Ninja could invoke `clang-cl.exe` and pass it all the files 
to be compiled, and let clang iterate and multi-thread internally. That could 
be quite a significant change, however the improvements //could be// important.
However that would probably break the concept behind //Goma //I suppose. Unless 
you split the invocations to precisely one `clang-cl` per available (remote) 
agent.


Repository:
  rC Clang

https://reviews.llvm.org/D52193



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

r343887 - [llvm-nm] Write "no symbol" output to stderr

[Petr Hosek via cfe-commits]

Author: phosek
Date: Fri Oct  5 14:10:03 2018
New Revision: 343887

URL: http://llvm.org/viewvc/llvm-project?rev=343887&view=rev
Log:
[llvm-nm] Write "no symbol" output to stderr

This matches the output of binutils' nm and ensures that any scripts
or tools that use nm and expect empty output in case there no symbols
don't break.

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

Modified:
cfe/trunk/test/CodeGen/thinlto_backend.ll

Modified: cfe/trunk/test/CodeGen/thinlto_backend.ll
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/thinlto_backend.ll?rev=343887&r1=343886&r2=343887&view=diff
==
--- cfe/trunk/test/CodeGen/thinlto_backend.ll (original)
+++ cfe/trunk/test/CodeGen/thinlto_backend.ll Fri Oct  5 14:10:03 2018
@@ -25,7 +25,7 @@
 ; be empty file.
 ; RUN: opt -o %t5.o %s
 ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t4.o -x ir %t5.o -c 
-fthinlto-index=%t.thinlto.bc
-; RUN: llvm-nm %t4.o | FileCheck %s -check-prefix=NO-SYMBOLS
+; RUN: llvm-nm %t4.o 2>&1 | FileCheck %s -check-prefix=NO-SYMBOLS
 ; NO-SYMBOLS: no symbols
 
 ; Ensure f2 was imported. Check for all 3 flavors of -save-temps[=cwd|obj].


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

[PATCH] D52949: [Diagnostics] Implement -Wsizeof-pointer-div

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 created this revision.
Herald added a subscriber: cfe-commits.

void test(int *arr) {

  int arr_len = sizeof(arr) / sizeof(*arr);  // warn, incorrect way to compute 
number of array elements

}

Enabled under -Wall (same behaviour as GCC)


Repository:
  rC Clang

https://reviews.llvm.org/D52949

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  test/Sema/div-sizeof-ptr.c

Index: test/Sema/div-sizeof-ptr.c
===
--- test/Sema/div-sizeof-ptr.c
+++ test/Sema/div-sizeof-ptr.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 %s -verify -Wsizeof-pointer-div -fsyntax-only
+
+void test(int *p, int **q) {
+int a = sizeof(p) / sizeof(*p);  // expected-warning {{division produces the incorrect number of array elements}}
+int b = sizeof p / sizeof *p;// expected-warning {{division produces the incorrect number of array elements}}
+int c = sizeof(*q) / sizeof(**q);// expected-warning {{division produces the incorrect number of array elements}}
+int d = sizeof(p) / sizeof(int); // expected-warning {{division produces the incorrect number of array elements}}
+int e = sizeof(int *) / sizeof(int); // expected-warning {{division produces the incorrect number of array elements}}
+  
+int f = sizeof(p) / sizeof(p);
+int g = sizeof(*q) / sizeof(q);
+}
+
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -8670,6 +8670,42 @@
   << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
 }
 
+static void DiagnoseDivisionSizeofPointer(Sema &S, Expr *LHS, Expr *RHS,
+  SourceLocation Loc) {
+  UnaryExprOrTypeTraitExpr *LUE =
+  dyn_cast_or_null(LHS);
+  UnaryExprOrTypeTraitExpr *RUE =
+  dyn_cast_or_null(RHS);
+
+  if (!LUE || !RUE)
+return;
+  if (LUE->getKind() != UETT_SizeOf || RUE->getKind() != UETT_SizeOf)
+return;
+
+  QualType LHSTy;
+  QualType RHSTy;
+  if (LUE->isArgumentType()) {
+LHSTy = LUE->getArgumentType();
+  } else {
+LHS = LUE->getArgumentExpr()->IgnoreParens();
+LHSTy = LHS->getType();
+  }
+
+  if (RUE->isArgumentType()) {
+RHSTy = RUE->getArgumentType();
+  } else {
+RHS = RUE->getArgumentExpr()->IgnoreParens();
+RHSTy = RHS->getType();
+  }
+
+  if (!LHSTy->isPointerType() || RHSTy->isPointerType())
+return;
+  if (LHSTy->getPointeeType() != RHSTy)
+return;
+
+  S.Diag(Loc, diag::warn_division_sizeof_ptr) << LHS->getSourceRange();
+}
+
 static void DiagnoseBadDivideOrRemainderValues(Sema& S, ExprResult &LHS,
ExprResult &RHS,
SourceLocation Loc, bool IsDiv) {
@@ -8700,8 +8736,10 @@
 
   if (compType.isNull() || !compType->isArithmeticType())
 return InvalidOperands(Loc, LHS, RHS);
-  if (IsDiv)
+  if (IsDiv) {
 DiagnoseBadDivideOrRemainderValues(*this, LHS, RHS, Loc, IsDiv);
+DiagnoseDivisionSizeofPointer(*this, LHS.get(), RHS.get(), Loc);
+  }
   return compType;
 }
 
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -3291,6 +3291,10 @@
   InGroup;
 def note_reference_is_return_value : Note<"%0 returns a reference">;
 
+def warn_division_sizeof_ptr : Warning<
+  "division produces the incorrect number of array elements">,
+  InGroup;
+
 def note_function_warning_silence : Note<
 "prefix with the address-of operator to silence this warning">;
 def note_function_to_function_call : Note<
Index: include/clang/Basic/DiagnosticGroups.td
===
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -142,6 +142,7 @@
 def : DiagGroup<"discard-qual">;
 def DivZero : DiagGroup<"division-by-zero">;
 def : DiagGroup<"div-by-zero", [DivZero]>;
+def DivSizeofPtr : DiagGroup<"sizeof-pointer-div">;
 
 def DocumentationHTML : DiagGroup<"documentation-html">;
 def DocumentationUnknownCommand : DiagGroup<"documentation-unknown-command">;
@@ -785,6 +786,7 @@
 SelfMove,
 SizeofArrayArgument,
 SizeofArrayDecay,
+DivSizeofPtr,
 StringPlusInt,
 Trigraphs,
 Uninitialized,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52949: [Diagnostics] Implement -Wsizeof-pointer-div

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 updated this revision to Diff 168539.

https://reviews.llvm.org/D52949

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  test/Sema/div-sizeof-ptr.c

Index: test/Sema/div-sizeof-ptr.c
===
--- test/Sema/div-sizeof-ptr.c
+++ test/Sema/div-sizeof-ptr.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 %s -verify -Wsizeof-pointer-div -fsyntax-only
+
+void test(int *p, int **q) {
+int a = sizeof(p) / sizeof(*p);  // expected-warning {{division produces the incorrect number of array elements}}
+int b = sizeof p / sizeof *p;// expected-warning {{division produces the incorrect number of array elements}}
+int c = sizeof(*q) / sizeof(**q);// expected-warning {{division produces the incorrect number of array elements}}
+int d = sizeof(p) / sizeof(int); // expected-warning {{division produces the incorrect number of array elements}}
+
+int e = sizeof(int *) / sizeof(int);
+int f = sizeof(p) / sizeof(p);
+int g = sizeof(*q) / sizeof(q);
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -8670,6 +8670,41 @@
   << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
 }
 
+static void DiagnoseDivisionSizeofPointer(Sema &S, Expr *LHS, Expr *RHS,
+  SourceLocation Loc) {
+  UnaryExprOrTypeTraitExpr *LUE =
+  dyn_cast_or_null(LHS);
+  UnaryExprOrTypeTraitExpr *RUE =
+  dyn_cast_or_null(RHS);
+
+  if (!LUE || !RUE)
+return;
+  if (LUE->getKind() != UETT_SizeOf || RUE->getKind() != UETT_SizeOf)
+return;
+
+  QualType LHSTy;
+  QualType RHSTy;
+  if (LUE->isArgumentType())
+return;
+
+  LHS = LUE->getArgumentExpr()->IgnoreParens();
+  LHSTy = LHS->getType();
+
+  if (RUE->isArgumentType()) {
+RHSTy = RUE->getArgumentType();
+  } else {
+RHS = RUE->getArgumentExpr()->IgnoreParens();
+RHSTy = RHS->getType();
+  }
+
+  if (!LHSTy->isPointerType() || RHSTy->isPointerType())
+return;
+  if (LHSTy->getPointeeType() != RHSTy)
+return;
+
+  S.Diag(Loc, diag::warn_division_sizeof_ptr) << LHS->getSourceRange();
+}
+
 static void DiagnoseBadDivideOrRemainderValues(Sema& S, ExprResult &LHS,
ExprResult &RHS,
SourceLocation Loc, bool IsDiv) {
@@ -8700,8 +8735,10 @@
 
   if (compType.isNull() || !compType->isArithmeticType())
 return InvalidOperands(Loc, LHS, RHS);
-  if (IsDiv)
+  if (IsDiv) {
 DiagnoseBadDivideOrRemainderValues(*this, LHS, RHS, Loc, IsDiv);
+DiagnoseDivisionSizeofPointer(*this, LHS.get(), RHS.get(), Loc);
+  }
   return compType;
 }
 
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -3291,6 +3291,10 @@
   InGroup;
 def note_reference_is_return_value : Note<"%0 returns a reference">;
 
+def warn_division_sizeof_ptr : Warning<
+  "division produces the incorrect number of array elements">,
+  InGroup;
+
 def note_function_warning_silence : Note<
 "prefix with the address-of operator to silence this warning">;
 def note_function_to_function_call : Note<
Index: include/clang/Basic/DiagnosticGroups.td
===
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -142,6 +142,7 @@
 def : DiagGroup<"discard-qual">;
 def DivZero : DiagGroup<"division-by-zero">;
 def : DiagGroup<"div-by-zero", [DivZero]>;
+def DivSizeofPtr : DiagGroup<"sizeof-pointer-div">;
 
 def DocumentationHTML : DiagGroup<"documentation-html">;
 def DocumentationUnknownCommand : DiagGroup<"documentation-unknown-command">;
@@ -785,6 +786,7 @@
 SelfMove,
 SizeofArrayArgument,
 SizeofArrayDecay,
+DivSizeofPtr,
 StringPlusInt,
 Trigraphs,
 Uninitialized,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52949: [Diagnostics] Implement -Wsizeof-pointer-div

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added inline comments.



Comment at: test/Sema/div-sizeof-ptr.c:9
+
+int e = sizeof(int *) / sizeof(int);
+int f = sizeof(p) / sizeof(p);

GCC warns also in this case, but it is weird...


https://reviews.llvm.org/D52949



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

r343892 - Emit CK_NoOp casts in C mode, not just C++.

[James Y Knight via cfe-commits]

Author: jyknight
Date: Fri Oct  5 14:53:51 2018
New Revision: 343892

URL: http://llvm.org/viewvc/llvm-project?rev=343892&view=rev
Log:
Emit CK_NoOp casts in C mode, not just C++.

Previously, it had been using CK_BitCast even for casts that only
change const/restrict/volatile. Now it will use CK_Noop where
appropriate.

This is an alternate solution to r336746.

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

Added:
cfe/trunk/test/Sema/c-casts.c
Modified:
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=343892&r1=343891&r2=343892&view=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Fri Oct  5 14:53:51 2018
@@ -5864,11 +5864,7 @@ bool PointerExprEvaluator::VisitCastExpr
 // permitted in constant expressions in C++11. Bitcasts from cv void* are
 // also static_casts, but we disallow them as a resolution to DR1312.
 if (!E->getType()->isVoidPointerType()) {
-  // If we changed anything other than cvr-qualifiers, we can't use this
-  // value for constant folding. FIXME: Qualification conversions should
-  // always be CK_NoOp, but we get this wrong in C.
-  if (!Info.Ctx.hasCvrSimilarType(E->getType(), 
E->getSubExpr()->getType()))
-Result.Designator.setInvalid();
+  Result.Designator.setInvalid();
   if (SubExpr->getType()->isVoidPointerType())
 CCEDiag(E, diag::note_constexpr_invalid_cast)
   << 3 << SubExpr->getType();

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=343892&r1=343891&r2=343892&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Fri Oct  5 14:53:51 2018
@@ -5862,6 +5862,8 @@ CastKind Sema::PrepareScalarCast(ExprRes
   LangAS DestAS = DestTy->getPointeeType().getAddressSpace();
   if (SrcAS != DestAS)
 return CK_AddressSpaceConversion;
+  if (Context.hasCvrSimilarType(SrcTy, DestTy))
+return CK_NoOp;
   return CK_BitCast;
 }
 case Type::STK_BlockPointer:
@@ -7762,7 +7764,12 @@ Sema::CheckAssignmentConstraints(QualTyp
 if (isa(RHSType)) {
   LangAS AddrSpaceL = LHSPointer->getPointeeType().getAddressSpace();
   LangAS AddrSpaceR = RHSType->getPointeeType().getAddressSpace();
-  Kind = AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion : CK_BitCast;
+  if (AddrSpaceL != AddrSpaceR)
+Kind = CK_AddressSpaceConversion;
+  else if (Context.hasCvrSimilarType(RHSType, LHSType))
+Kind = CK_NoOp;
+  else
+Kind = CK_BitCast;
   return checkPointerTypesForAssignment(*this, LHSType, RHSType);
 }
 

Added: cfe/trunk/test/Sema/c-casts.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/c-casts.c?rev=343892&view=auto
==
--- cfe/trunk/test/Sema/c-casts.c (added)
+++ cfe/trunk/test/Sema/c-casts.c Fri Oct  5 14:53:51 2018
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -w -ast-dump %s | FileCheck %s
+
+// The cast construction code both for implicit and c-style casts is very
+// different in C vs C++. This file is intended to test the C behavior.
+
+// TODO: add tests covering the rest of the code in
+// Sema::CheckAssignmentConstraints and Sema::PrepareScalarCast
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_cvr_pointer
+void cast_cvr_pointer(char volatile * __restrict * const * p) {
+  char*** x;
+  // CHECK: ImplicitCastExpr {{.*}} 'char ***' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'char ***' 
+  x = (char***)p;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_pointer_type
+void cast_pointer_type(char *p) {
+  void *x;
+  // CHECK: ImplicitCastExpr {{.*}} 'void *' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'void *' 
+  x = (void*)p;
+}


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

[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC343892: Emit CK_NoOp casts in C mode, not just C++. 
(authored by jyknight, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52918?vs=168477&id=168541#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52918

Files:
  lib/AST/ExprConstant.cpp
  lib/Sema/SemaExpr.cpp
  test/Sema/c-casts.c


Index: test/Sema/c-casts.c
===
--- test/Sema/c-casts.c
+++ test/Sema/c-casts.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -w -ast-dump %s | FileCheck %s
+
+// The cast construction code both for implicit and c-style casts is very
+// different in C vs C++. This file is intended to test the C behavior.
+
+// TODO: add tests covering the rest of the code in
+// Sema::CheckAssignmentConstraints and Sema::PrepareScalarCast
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_cvr_pointer
+void cast_cvr_pointer(char volatile * __restrict * const * p) {
+  char*** x;
+  // CHECK: ImplicitCastExpr {{.*}} 'char ***' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'char ***' 
+  x = (char***)p;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_pointer_type
+void cast_pointer_type(char *p) {
+  void *x;
+  // CHECK: ImplicitCastExpr {{.*}} 'void *' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'void *' 
+  x = (void*)p;
+}
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -5864,11 +5864,7 @@
 // permitted in constant expressions in C++11. Bitcasts from cv void* are
 // also static_casts, but we disallow them as a resolution to DR1312.
 if (!E->getType()->isVoidPointerType()) {
-  // If we changed anything other than cvr-qualifiers, we can't use this
-  // value for constant folding. FIXME: Qualification conversions should
-  // always be CK_NoOp, but we get this wrong in C.
-  if (!Info.Ctx.hasCvrSimilarType(E->getType(), 
E->getSubExpr()->getType()))
-Result.Designator.setInvalid();
+  Result.Designator.setInvalid();
   if (SubExpr->getType()->isVoidPointerType())
 CCEDiag(E, diag::note_constexpr_invalid_cast)
   << 3 << SubExpr->getType();
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -5862,6 +5862,8 @@
   LangAS DestAS = DestTy->getPointeeType().getAddressSpace();
   if (SrcAS != DestAS)
 return CK_AddressSpaceConversion;
+  if (Context.hasCvrSimilarType(SrcTy, DestTy))
+return CK_NoOp;
   return CK_BitCast;
 }
 case Type::STK_BlockPointer:
@@ -7762,7 +7764,12 @@
 if (isa(RHSType)) {
   LangAS AddrSpaceL = LHSPointer->getPointeeType().getAddressSpace();
   LangAS AddrSpaceR = RHSType->getPointeeType().getAddressSpace();
-  Kind = AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion : CK_BitCast;
+  if (AddrSpaceL != AddrSpaceR)
+Kind = CK_AddressSpaceConversion;
+  else if (Context.hasCvrSimilarType(RHSType, LHSType))
+Kind = CK_NoOp;
+  else
+Kind = CK_BitCast;
   return checkPointerTypesForAssignment(*this, LHSType, RHSType);
 }
 


Index: test/Sema/c-casts.c
===
--- test/Sema/c-casts.c
+++ test/Sema/c-casts.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -w -ast-dump %s | FileCheck %s
+
+// The cast construction code both for implicit and c-style casts is very
+// different in C vs C++. This file is intended to test the C behavior.
+
+// TODO: add tests covering the rest of the code in
+// Sema::CheckAssignmentConstraints and Sema::PrepareScalarCast
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_cvr_pointer
+void cast_cvr_pointer(char volatile * __restrict * const * p) {
+  char*** x;
+  // CHECK: ImplicitCastExpr {{.*}} 'char ***' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'char ***' 
+  x = (char***)p;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_pointer_type
+void cast_pointer_type(char *p) {
+  void *x;
+  // CHECK: ImplicitCastExpr {{.*}} 'void *' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'void *' 
+  x = (void*)p;
+}
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -5864,11 +5864,7 @@
 // permitted in constant expressions in C++11. Bitcasts from cv void* are
 // also static_casts, but we disallow them as a resolution to DR1312.
 if (!E->getType()->isVoidPointerType()) {
-  // If we changed anything other than cvr-qualifiers, we can't use this
-  // value for constant folding. FIXME: Qualification conversions should
-  // always be CK_NoOp, but we get this wrong in C.
-  if (!Info.Ctx.hasCvrSimilarType(E->getType(), E->getSubExpr()->getType()))
-Result.Designator.setInvalid();
+  Resu

[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL343892: Emit CK_NoOp casts in C mode, not just C++. 
(authored by jyknight, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52918?vs=168477&id=168542#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52918

Files:
  cfe/trunk/lib/AST/ExprConstant.cpp
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/test/Sema/c-casts.c


Index: cfe/trunk/lib/AST/ExprConstant.cpp
===
--- cfe/trunk/lib/AST/ExprConstant.cpp
+++ cfe/trunk/lib/AST/ExprConstant.cpp
@@ -5864,11 +5864,7 @@
 // permitted in constant expressions in C++11. Bitcasts from cv void* are
 // also static_casts, but we disallow them as a resolution to DR1312.
 if (!E->getType()->isVoidPointerType()) {
-  // If we changed anything other than cvr-qualifiers, we can't use this
-  // value for constant folding. FIXME: Qualification conversions should
-  // always be CK_NoOp, but we get this wrong in C.
-  if (!Info.Ctx.hasCvrSimilarType(E->getType(), 
E->getSubExpr()->getType()))
-Result.Designator.setInvalid();
+  Result.Designator.setInvalid();
   if (SubExpr->getType()->isVoidPointerType())
 CCEDiag(E, diag::note_constexpr_invalid_cast)
   << 3 << SubExpr->getType();
Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -5862,6 +5862,8 @@
   LangAS DestAS = DestTy->getPointeeType().getAddressSpace();
   if (SrcAS != DestAS)
 return CK_AddressSpaceConversion;
+  if (Context.hasCvrSimilarType(SrcTy, DestTy))
+return CK_NoOp;
   return CK_BitCast;
 }
 case Type::STK_BlockPointer:
@@ -7762,7 +7764,12 @@
 if (isa(RHSType)) {
   LangAS AddrSpaceL = LHSPointer->getPointeeType().getAddressSpace();
   LangAS AddrSpaceR = RHSType->getPointeeType().getAddressSpace();
-  Kind = AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion : CK_BitCast;
+  if (AddrSpaceL != AddrSpaceR)
+Kind = CK_AddressSpaceConversion;
+  else if (Context.hasCvrSimilarType(RHSType, LHSType))
+Kind = CK_NoOp;
+  else
+Kind = CK_BitCast;
   return checkPointerTypesForAssignment(*this, LHSType, RHSType);
 }
 
Index: cfe/trunk/test/Sema/c-casts.c
===
--- cfe/trunk/test/Sema/c-casts.c
+++ cfe/trunk/test/Sema/c-casts.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -w -ast-dump %s | FileCheck %s
+
+// The cast construction code both for implicit and c-style casts is very
+// different in C vs C++. This file is intended to test the C behavior.
+
+// TODO: add tests covering the rest of the code in
+// Sema::CheckAssignmentConstraints and Sema::PrepareScalarCast
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_cvr_pointer
+void cast_cvr_pointer(char volatile * __restrict * const * p) {
+  char*** x;
+  // CHECK: ImplicitCastExpr {{.*}} 'char ***' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'char ***' 
+  x = (char***)p;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_pointer_type
+void cast_pointer_type(char *p) {
+  void *x;
+  // CHECK: ImplicitCastExpr {{.*}} 'void *' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'void *' 
+  x = (void*)p;
+}


Index: cfe/trunk/lib/AST/ExprConstant.cpp
===
--- cfe/trunk/lib/AST/ExprConstant.cpp
+++ cfe/trunk/lib/AST/ExprConstant.cpp
@@ -5864,11 +5864,7 @@
 // permitted in constant expressions in C++11. Bitcasts from cv void* are
 // also static_casts, but we disallow them as a resolution to DR1312.
 if (!E->getType()->isVoidPointerType()) {
-  // If we changed anything other than cvr-qualifiers, we can't use this
-  // value for constant folding. FIXME: Qualification conversions should
-  // always be CK_NoOp, but we get this wrong in C.
-  if (!Info.Ctx.hasCvrSimilarType(E->getType(), E->getSubExpr()->getType()))
-Result.Designator.setInvalid();
+  Result.Designator.setInvalid();
   if (SubExpr->getType()->isVoidPointerType())
 CCEDiag(E, diag::note_constexpr_invalid_cast)
   << 3 << SubExpr->getType();
Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -5862,6 +5862,8 @@
   LangAS DestAS = DestTy->getPointeeType().getAddressSpace();
   if (SrcAS != DestAS)
 return CK_AddressSpaceConversion;
+  if (Context.hasCvrSimilarType(SrcTy, DestTy))
+return CK_NoOp;
   return CK_BitCast;
 }
 case Type::STK_BlockPointer:
@@ -7762,7 +7764,12 @@
 if (isa(RHSType)) {
   LangAS AddrSpaceL = LHSPointer->getPointeeType().getAddressSpace()

r343894 - [COFF, ARM64] Add _InterlockedAdd intrinsic

[Mandeep Singh Grang via cfe-commits]

Author: mgrang
Date: Fri Oct  5 14:57:41 2018
New Revision: 343894

URL: http://llvm.org/viewvc/llvm-project?rev=343894&view=rev
Log:
[COFF, ARM64] Add _InterlockedAdd intrinsic

Reviewers: rnk, mstorsjo, compnerd, TomTan, haripul, javed.absar, efriedma

Reviewed By: efriedma

Subscribers: efriedma, kristof.beyls, chrib, jfb, cfe-commits

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

Modified:
cfe/trunk/include/clang/Basic/BuiltinsAArch64.def
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/Headers/intrin.h
cfe/trunk/test/CodeGen/arm64-microsoft-intrinsics.c

Modified: cfe/trunk/include/clang/Basic/BuiltinsAArch64.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsAArch64.def?rev=343894&r1=343893&r2=343894&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsAArch64.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsAArch64.def Fri Oct  5 14:57:41 2018
@@ -94,6 +94,7 @@ TARGET_HEADER_BUILTIN(_BitScanReverse, "
 TARGET_HEADER_BUILTIN(_BitScanForward64, "UcUNi*ULLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_BitScanReverse64, "UcUNi*ULLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 
+TARGET_HEADER_BUILTIN(_InterlockedAdd,   "LiLiD*Li","nh", 
"intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedAnd64, "LLiLLiD*LLi", "nh", 
"intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedDecrement64,   "LLiLLiD*","nh", 
"intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedExchange64,"LLiLLiD*LLi", "nh", 
"intrin.h", ALL_MS_LANGUAGES, "")

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=343894&r1=343893&r2=343894&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Fri Oct  5 14:57:41 2018
@@ -8517,6 +8517,15 @@ Value *CodeGenFunction::EmitAArch64Built
 return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedDecrement, E);
   case AArch64::BI_InterlockedIncrement64:
 return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedIncrement, E);
+
+  case AArch64::BI_InterlockedAdd: {
+Value *Arg0 = EmitScalarExpr(E->getArg(0));
+Value *Arg1 = EmitScalarExpr(E->getArg(1));
+AtomicRMWInst *RMWI = Builder.CreateAtomicRMW(
+  AtomicRMWInst::Add, Arg0, Arg1,
+  llvm::AtomicOrdering::SequentiallyConsistent);
+return Builder.CreateAdd(RMWI, Arg1);
+  }
   }
 }
 

Modified: cfe/trunk/lib/Headers/intrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/intrin.h?rev=343894&r1=343893&r2=343894&view=diff
==
--- cfe/trunk/lib/Headers/intrin.h (original)
+++ cfe/trunk/lib/Headers/intrin.h Fri Oct  5 14:57:41 2018
@@ -869,6 +869,7 @@ __nop(void) {
 
\**/
 #if defined(__aarch64__)
 unsigned __int64 __getReg(int);
+long _InterlockedAdd(long volatile *Addend, long Value);
 #endif
 
 
/**\

Modified: cfe/trunk/test/CodeGen/arm64-microsoft-intrinsics.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/arm64-microsoft-intrinsics.c?rev=343894&r1=343893&r2=343894&view=diff
==
--- cfe/trunk/test/CodeGen/arm64-microsoft-intrinsics.c (original)
+++ cfe/trunk/test/CodeGen/arm64-microsoft-intrinsics.c Fri Oct  5 14:57:41 2018
@@ -4,6 +4,16 @@
 // RUN: not %clang_cc1 -triple arm64-linux -Werror -S -o /dev/null %s 2>&1 \
 // RUN:| FileCheck %s -check-prefix CHECK-LINUX
 
+long test_InterlockedAdd(long volatile *Addend, long Value) {
+  return _InterlockedAdd(Addend, Value);
+}
+
+// CHECK-LABEL: define {{.*}} i32 @test_InterlockedAdd(i32* %Addend, i32 
%Value) {{.*}} {
+// CHECK-MSVC: %[[OLDVAL:[0-9]+]] = atomicrmw add i32* %1, i32 %2 seq_cst
+// CHECK-MSVC: %[[NEWVAL:[0-9]+]] = add i32 %[[OLDVAL:[0-9]+]], %2
+// CHECK-MSVC: ret i32 %[[NEWVAL:[0-9]+]]
+// CHECK-LINUX: error: implicit declaration of function '_InterlockedAdd'
+
 void check__dmb(void) {
   __dmb(0);
 }


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