[PATCH] D41228: [ObjC] Enable __strong pointers in structs under ARC

2018-04-17 Thread Stephan Bergmann via Phabricator via cfe-commits
sberg added a comment.

It looks like this caused a clang-cl regression 
https://bugs.llvm.org/show_bug.cgi?id=37146 "clang-cl emits special functions 
for non-trivial C-structs ('__destructor_8') introduced for Objective-C".


Repository:
  rL LLVM

https://reviews.llvm.org/D41228



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


[PATCH] D45662: Fuzzer, add libcxx for OpenBSD

2018-04-17 Thread David CARLIER via Phabricator via cfe-commits
devnexen added inline comments.



Comment at: lib/Driver/ToolChains/OpenBSD.cpp:189
   if (getToolChain().ShouldLinkCXXStdlib(Args))
-getToolChain().AddCXXStdlibLibArgs(Args, CmdArgs);
+ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
   if (Args.hasArg(options::OPT_pg))

dberris wrote:
> Do you actually need this change? Why isn't 
> `getToolChain().AddCXXStdlibLibArgs(...)` not sufficient here?
That s the thing, I wish it was simple as FreeBSD, but seemingly in OpenBSD 
needs both c++98 gcc runtime and libc++ for fuzzer (I tried libc++ alone 
already)


https://reviews.llvm.org/D45662



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


[PATCH] D45662: Fuzzer, add libcxx for OpenBSD

2018-04-17 Thread Dean Michael Berris via Phabricator via cfe-commits
dberris added inline comments.



Comment at: lib/Driver/ToolChains/OpenBSD.cpp:189
   if (getToolChain().ShouldLinkCXXStdlib(Args))
-getToolChain().AddCXXStdlibLibArgs(Args, CmdArgs);
+ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
   if (Args.hasArg(options::OPT_pg))

devnexen wrote:
> dberris wrote:
> > Do you actually need this change? Why isn't 
> > `getToolChain().AddCXXStdlibLibArgs(...)` not sufficient here?
> That s the thing, I wish it was simple as FreeBSD, but seemingly in OpenBSD 
> needs both c++98 gcc runtime and libc++ for fuzzer (I tried libc++ alone 
> already)
Right, but this comment is on this specific line change. I don't think you need 
to reach into `Toolchain.` direcly, since you can already use `getToolChain()` 
just from the above line (188).


https://reviews.llvm.org/D45662



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


[PATCH] D45665: [builtin-dump-struct] printf formats generation testing

2018-04-17 Thread Paul Semel via Phabricator via cfe-commits
paulsemel added a comment.

In https://reviews.llvm.org/D45665#1069075, @aaron.ballman wrote:

> LGTM!


Nice to hear ! As for the last time, as I don't have any rights to push this, I 
would really appreciate if you could do it for me ! :)


Repository:
  rC Clang

https://reviews.llvm.org/D45665



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


[PATCH] D45662: Fuzzer, add libcxx for OpenBSD

2018-04-17 Thread David CARLIER via Phabricator via cfe-commits
devnexen added inline comments.



Comment at: lib/Driver/ToolChains/OpenBSD.cpp:189
   if (getToolChain().ShouldLinkCXXStdlib(Args))
-getToolChain().AddCXXStdlibLibArgs(Args, CmdArgs);
+ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
   if (Args.hasArg(options::OPT_pg))

dberris wrote:
> devnexen wrote:
> > dberris wrote:
> > > Do you actually need this change? Why isn't 
> > > `getToolChain().AddCXXStdlibLibArgs(...)` not sufficient here?
> > That s the thing, I wish it was simple as FreeBSD, but seemingly in OpenBSD 
> > needs both c++98 gcc runtime and libc++ for fuzzer (I tried libc++ alone 
> > already)
> Right, but this comment is on this specific line change. I don't think you 
> need to reach into `Toolchain.` direcly, since you can already use 
> `getToolChain()` just from the above line (188).
Right, so I guess this diff https://reviews.llvm.org/D45662?id=142686 is 
sufficient then ?


https://reviews.llvm.org/D45662



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


[PATCH] D38615: [libclang] Only mark CXCursors for explicit attributes with a type

2018-04-17 Thread Nikolai Kosjar via Phabricator via cfe-commits
nik added a comment.

Ping


https://reviews.llvm.org/D38615



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


[PATCH] D45662: Fuzzer, add libcxx for OpenBSD

2018-04-17 Thread Dean Michael Berris via Phabricator via cfe-commits
dberris added inline comments.



Comment at: lib/Driver/ToolChains/OpenBSD.cpp:189
   if (getToolChain().ShouldLinkCXXStdlib(Args))
-getToolChain().AddCXXStdlibLibArgs(Args, CmdArgs);
+ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
   if (Args.hasArg(options::OPT_pg))

devnexen wrote:
> dberris wrote:
> > devnexen wrote:
> > > dberris wrote:
> > > > Do you actually need this change? Why isn't 
> > > > `getToolChain().AddCXXStdlibLibArgs(...)` not sufficient here?
> > > That s the thing, I wish it was simple as FreeBSD, but seemingly in 
> > > OpenBSD needs both c++98 gcc runtime and libc++ for fuzzer (I tried 
> > > libc++ alone already)
> > Right, but this comment is on this specific line change. I don't think you 
> > need to reach into `Toolchain.` direcly, since you can already use 
> > `getToolChain()` just from the above line (188).
> Right, so I guess this diff https://reviews.llvm.org/D45662?id=142686 is 
> sufficient then ?
No. Let me try and explain again.

You were on the right path, with overriding the `AddCXXStdlibLibArgs` function 
in the OpenBSD Toolchain type. It's just that you weren't handling the case for 
when the binary was being built with libc++ or libstdc++ properly. I was 
referring you to what FreeBSD was doing for their implementation of 
`AddCXXStdlibLibArgs`. This means, checking first whether the invocation of the 
compiler was using libc++ or libstdc++, and then adding the appropriate link 
spelling. That all happens in the `AddCXXStdlibLibArgs` implementation, because 
there's no need to special-case just for the sanitizers.

This means, if you're building a normal binary with `-pg` in OpenBSD against 
either libc++ or libstdc++, it wouldn't work correctly regardless of whether 
you were using libFuzzer.

Does that make more sense?


https://reviews.llvm.org/D45662



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


[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete

2018-04-17 Thread Nikolai Kosjar via Phabricator via cfe-commits
nik added a comment.
Herald added a subscriber: llvm-commits.

Ping


Repository:
  rL LLVM

https://reviews.llvm.org/D36390



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


[PATCH] D36390: Fix overloaded static functions in SemaCodeComplete

2018-04-17 Thread Nikolai Kosjar via Phabricator via cfe-commits
nik added a comment.

Huch, seems already submitted. Ignore :>


Repository:
  rL LLVM

https://reviews.llvm.org/D36390



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


[PATCH] D45662: Fuzzer, add libcxx for OpenBSD

2018-04-17 Thread David CARLIER via Phabricator via cfe-commits
devnexen added inline comments.



Comment at: lib/Driver/ToolChains/OpenBSD.cpp:189
   if (getToolChain().ShouldLinkCXXStdlib(Args))
-getToolChain().AddCXXStdlibLibArgs(Args, CmdArgs);
+ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
   if (Args.hasArg(options::OPT_pg))

dberris wrote:
> devnexen wrote:
> > dberris wrote:
> > > devnexen wrote:
> > > > dberris wrote:
> > > > > Do you actually need this change? Why isn't 
> > > > > `getToolChain().AddCXXStdlibLibArgs(...)` not sufficient here?
> > > > That s the thing, I wish it was simple as FreeBSD, but seemingly in 
> > > > OpenBSD needs both c++98 gcc runtime and libc++ for fuzzer (I tried 
> > > > libc++ alone already)
> > > Right, but this comment is on this specific line change. I don't think 
> > > you need to reach into `Toolchain.` direcly, since you can already use 
> > > `getToolChain()` just from the above line (188).
> > Right, so I guess this diff https://reviews.llvm.org/D45662?id=142686 is 
> > sufficient then ?
> No. Let me try and explain again.
> 
> You were on the right path, with overriding the `AddCXXStdlibLibArgs` 
> function in the OpenBSD Toolchain type. It's just that you weren't handling 
> the case for when the binary was being built with libc++ or libstdc++ 
> properly. I was referring you to what FreeBSD was doing for their 
> implementation of `AddCXXStdlibLibArgs`. This means, checking first whether 
> the invocation of the compiler was using libc++ or libstdc++, and then adding 
> the appropriate link spelling. That all happens in the `AddCXXStdlibLibArgs` 
> implementation, because there's no need to special-case just for the 
> sanitizers.
> 
> This means, if you're building a normal binary with `-pg` in OpenBSD against 
> either libc++ or libstdc++, it wouldn't work correctly regardless of whether 
> you were using libFuzzer.
> 
> Does that make more sense?
Ok will try a newer version later, thanks for your inputs.


https://reviews.llvm.org/D45662



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


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

2018-04-17 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

In https://reviews.llvm.org/D45444#1069488, @shuaiwang wrote:

> I've updated https://reviews.llvm.org/D45679 and I think the `isModified()` 
> function there is now sufficiently covering the cases you've covered here and 
> can be used as a good starting version if you plan to use it here.
>  I copied your const-values test cases and it now passes with the following 
> differences:
>
> - All unused local variables removed, my check will issue a warning for them 
> because technically they're not modified, but I understand why you don't want 
> to cover them. I don't feel strongly about it and I removed it from my 
> copy-pasted test cases because I just to demonstrate `isModified()`


Nice!

> - Better recall in `some_reference_taking`, both `np_local0` and 
> `r1_np_local0` can be caught, similar for `np_local1` and `non_const_ref` in 
> `handle_from_array`.

Yes. But i feel we need to gather more information. For potential checks, it 
might be nice to control to which scope modification is defined and/or emit 
`note`s for interesting events like taking a non-const handle which isnt 
modified, too.

It kinda feels like we are converging to a dependency graph for every variable 
that contains the information for dependencies and dependents + modification 
history. I dont think it is bad, but maybe complicated :)
What do you think about returning a `ModificationRecord` that contains the 
number of direct modification, the number of local non-const handles and 
escaping non-const handles. 
Including const handles would be nice in the future for more and different 
analysis.

> - `direct_class_access` `np_local5` triggers with my check (shouldn't it?)

Yes can be const: Godbolt 


> - In `range_for` non-const loop variables triggers with my check (shouldn't 
> they?)



- If the loop copies every value no modification occurs (its not a copy, its 
initialization. If the constructor would modify its value, the source cant be 
const either. Is it allowed?)
- if a handle is taken the usual rules apply.

> - In `range_for` local arrays doesn't trigger with my check, because I'm 
> treating ArrayToPointerDecay the same as taking address of a variable, and 
> looping over an array would involve ArrayToPointerDecay when the implicit 
> `__begin`/`__end` are initialized. I added a note inside `isModified` for 
> future improvements.

Given this is implemented here, can you use it?

> - My check over triggers for template (with my failed attempt to fix), but I 
> think it's more of some mistake in the check itself rather than in 
> `isModified`

I removed templates while matching for potential candidates. That can be moved 
out of the `isModified()`. `isModified()` can only reason about instantiated 
functions. For the actual template we need concepts.

> Sorry about the confusion.
>  Basically consider this example:
> 
>   class Foo {
>   public:
> void a() { x = 10; }
> void b() { // nothing }
> void c() { a(); }
> void d() { b(); }
> 
>   private:
> int x;
>   };
> 
> 
> If we check whether `isModified(dereference(cxxThisExpr()))` for each 
> `CompondStmt(hasParent(functionDecl()))`, we would conclude that:
> 
> - `a()` should stay non-const, because there's `this->x = 10` modifying 
> `*this`
> - `b()` should be changed to const, because nothing modifies `*this`
> - `c()` should stay non-const, because we're invoking non-const member 
> function on `this`
> - `d()` should also stay non-co

[PATCH] D44888: [RISCV] Default enable linker relaxation and add -mrelax, -mno-relax flags

2018-04-17 Thread Shiva Chen via Phabricator via cfe-commits
shiva0217 added a comment.

In https://reviews.llvm.org/D44888#1068806, @asb wrote:

> Thanks Kito. -mrelax and -mno-relax currently only affect the backend. For 
> completeness, I think this patch needs to pass the appropriate flag to the 
> linker depending on relax/no-relax.


Hi Alex. RISCVToolChain class does not define yet, so we can't pass --no-relax 
in RISCV::Linker::ConstructJob. Should we passing --no-relax when we introduce 
RISCVToolchain? Or there is another place we could implement passing RISCV 
linker flags?


Repository:
  rL LLVM

https://reviews.llvm.org/D44888



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


[PATCH] D45679: [clang-tidy] Add a helper function isModified, that checks whether an expression is modified within a statement.

2018-04-17 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

Short note here too: I think having `isModified` return a track record, like a 
vector for each

1. modifications
2. non-const handle taking
3. (const usages)

would be nice. Every user can decide how to react to it. Then the function 
would be more like `usageRecord` with pointers to each usage.
That allows graph building, it allows detailed diagnostics and contains all 
relevant information for the expr.

Other interesting cases we need to consider here:

1. casts -> all kinds of casts forbid kinda forbid constness.
2. rvalue references, modification happens for `std::move(v); std::forward(v); 
static_cast(v)`
3. ternary operator
4. lambdas with local usage only
5. lambdas that escape, e.g. `int i; std::thread t([&](){ while(true) i++; });` 
Hard to analyze? I think we can consider all references into lambdas as escape

Maybe you can add an assertion about template types without concepts as a 
safeguard?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45679



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


[PATCH] D45697: [clang-tidy] Fix clang-tidy doesn't read .clangtidy configuration file.

2018-04-17 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments.



Comment at: test/clang-tidy/read_file_config.cpp:1
+// RUN: mkdir -p %T/read-file-config/
+// RUN: cp %s %T/read-file-config/test.cpp

Will this work on windows?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45697



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


[PATCH] D45717: [clangd] Using index for GoToDefinition.

2018-04-17 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: MaskRay, ioeric, jkorous-apple, ilya-biryukov, klimek.

This patch adds index support for GoToDefinition -- when we don't get the
definition from local AST, we query our index (Static&Dynamic) index to
get it.

Since we currently collect top-level symbol in the index, it doesn't support all
cases (e.g. class members), we will extend the index to include more symbols in
the future.

Note: the newly-added test fails because of a DynamicIndex issue -- the symbol 
is
overwritten instead of merging.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45717

Files:
  clangd/ClangdServer.cpp
  clangd/XRefs.cpp
  clangd/XRefs.h
  unittests/clangd/XRefsTests.cpp

Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -106,6 +106,44 @@
 
 MATCHER_P(RangeIs, R, "") { return arg.range == R; }
 
+TEST(GoToDefinition, WithIndex) {
+  MockFSProvider FS;
+  IgnoreDiagnostics DiagConsumer;
+  MockCompilationDatabase CDB;
+  auto option = ClangdServer::optsForTest();
+  option.BuildDynamicSymbolIndex = true;
+  ClangdServer Server(CDB, FS, DiagConsumer, option);
+
+  const char *DefHeaderSource = "void ff();";
+  auto DefCpp = testPath("def.cpp");
+  auto DefHeader = testPath("def.h");
+  auto TestCpp = testPath("test.cpp");
+  Annotations Def(R"cpp(
+ #include "def.h"
+ void [[ff]]() {}
+  )cpp");
+  Annotations Test(R"cpp(
+ #include "def.h"
+ void f() {
+   ^ff();
+ }
+  )cpp");
+  FS.Files[DefCpp] = Def.code();
+  FS.Files[TestCpp] = Test.code();
+  FS.Files[DefHeader] = DefHeaderSource;
+
+  runAddDocument(Server, TestCpp, Test.code());
+  runAddDocument(Server, DefCpp, Def.code());
+  std::vector> ExpectedLocations;
+  for (const auto &R : Def.ranges())
+ExpectedLocations.push_back(RangeIs(R));
+
+  auto Locations = runFindDefinitions(Server, TestCpp, Test.point());
+  EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error";
+  EXPECT_THAT(*Locations, ElementsAreArray(ExpectedLocations))
+  << Test.code();
+}
+
 TEST(GoToDefinition, All) {
   const char *Tests[] = {
   R"cpp(// Local variable
Index: clangd/XRefs.h
===
--- clangd/XRefs.h
+++ clangd/XRefs.h
@@ -14,14 +14,16 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_XREFS_H
 
 #include "ClangdUnit.h"
+#include "index/Index.h"
 #include "Protocol.h"
 #include 
 
 namespace clang {
 namespace clangd {
 
 /// Get definition of symbol at a specified \p Pos.
-std::vector findDefinitions(ParsedAST &AST, Position Pos);
+std::vector findDefinitions(ParsedAST &AST, Position Pos,
+  const SymbolIndex *const Index = nullptr);
 
 /// Returns highlights for all usages of a symbol at \p Pos.
 std::vector findDocumentHighlights(ParsedAST &AST,
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -14,6 +14,7 @@
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Index/IndexDataConsumer.h"
 #include "clang/Index/IndexingAction.h"
+#include "clang/Index/USRGeneration.h"
 #include "llvm/Support/Path.h"
 namespace clang {
 namespace clangd {
@@ -128,6 +129,20 @@
   }
 };
 
+llvm::Optional
+getAbsoluteFilePath(const FileEntry *F, const SourceManager &SourceMgr) {
+  SmallString<64> FilePath = F->tryGetRealPathName();
+  if (FilePath.empty())
+FilePath = F->getName();
+  if (!llvm::sys::path::is_absolute(FilePath)) {
+if (!SourceMgr.getFileManager().makeAbsolutePath(FilePath)) {
+  log("Could not turn relative path to absolute: " + FilePath);
+  return llvm::None;
+}
+  }
+  return FilePath.str().str();
+}
+
 llvm::Optional
 makeLocation(ParsedAST &AST, const SourceRange &ValSourceRange) {
   const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
@@ -145,24 +160,19 @@
   Range R = {Begin, End};
   Location L;
 
-  SmallString<64> FilePath = F->tryGetRealPathName();
-  if (FilePath.empty())
-FilePath = F->getName();
-  if (!llvm::sys::path::is_absolute(FilePath)) {
-if (!SourceMgr.getFileManager().makeAbsolutePath(FilePath)) {
-  log("Could not turn relative path to absolute: " + FilePath);
-  return llvm::None;
-}
-  }
-
-  L.uri = URIForFile(FilePath.str());
+  auto FilePath = getAbsoluteFilePath(F, SourceMgr);
+  if (!FilePath)
+return llvm::None;
+  L.uri = URIForFile(*FilePath);
   L.range = R;
   return L;
 }
 
 } // namespace
 
-std::vector findDefinitions(ParsedAST &AST, Position Pos) {
+std::vector
+findDefinitions(ParsedAST &AST, Position Pos,
+const SymbolIndex *const Index) {
   const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
   const FileEntry *FE = SourceMgr.getFileEntryForID(SourceMgr.getMainFileID());
   if (!FE)
@@ -1

[clang-tools-extra] r330182 - [clangd] Fix "fail to create file URI" warnings in FileIndexTest.

2018-04-17 Thread Haojian Wu via cfe-commits
Author: hokein
Date: Tue Apr 17 01:34:50 2018
New Revision: 330182

URL: http://llvm.org/viewvc/llvm-project?rev=330182&view=rev
Log:
[clangd] Fix "fail to create file URI" warnings in FileIndexTest.

Summary:
When running the FileIndexTest, it shows "Failed to create an URI
for file XXX: not a valid absolute file path" warnings, and the
generated Symbols is missing Location information.

Reviewers: ioeric

Subscribers: klimek, ilya-biryukov, jkorous-apple, MaskRay, cfe-commits

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

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

Modified: clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp?rev=330182&r1=330181&r2=330182&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp Tue Apr 17 
01:34:50 2018
@@ -94,8 +94,8 @@ llvm::Optional build(llvm::St
  "BasePath must be a base file path without extension.");
   llvm::IntrusiveRefCntPtr VFS(
   new vfs::InMemoryFileSystem);
-  std::string Path = (BasePath + ".cpp").str();
-  std::string Header = (BasePath + ".h").str();
+  std::string Path = testPath((BasePath + ".cpp").str());
+  std::string Header = testPath((BasePath + ".h").str());
   VFS->addFile(Path, 0, llvm::MemoryBuffer::getMemBuffer(""));
   VFS->addFile(Header, 0, llvm::MemoryBuffer::getMemBuffer(Code));
   const char *Args[] = {"clang", "-xc++", "-include", Header.c_str(),


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


[PATCH] D45692: [clangd] Fix "fail to create file URI" warnings in FileIndexTest.

2018-04-17 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL330182: [clangd] Fix "fail to create file URI" 
warnings in FileIndexTest. (authored by hokein, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D45692

Files:
  clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp


Index: clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
@@ -94,8 +94,8 @@
  "BasePath must be a base file path without extension.");
   llvm::IntrusiveRefCntPtr VFS(
   new vfs::InMemoryFileSystem);
-  std::string Path = (BasePath + ".cpp").str();
-  std::string Header = (BasePath + ".h").str();
+  std::string Path = testPath((BasePath + ".cpp").str());
+  std::string Header = testPath((BasePath + ".h").str());
   VFS->addFile(Path, 0, llvm::MemoryBuffer::getMemBuffer(""));
   VFS->addFile(Header, 0, llvm::MemoryBuffer::getMemBuffer(Code));
   const char *Args[] = {"clang", "-xc++", "-include", Header.c_str(),


Index: clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
@@ -94,8 +94,8 @@
  "BasePath must be a base file path without extension.");
   llvm::IntrusiveRefCntPtr VFS(
   new vfs::InMemoryFileSystem);
-  std::string Path = (BasePath + ".cpp").str();
-  std::string Header = (BasePath + ".h").str();
+  std::string Path = testPath((BasePath + ".cpp").str());
+  std::string Header = testPath((BasePath + ".h").str());
   VFS->addFile(Path, 0, llvm::MemoryBuffer::getMemBuffer(""));
   VFS->addFile(Header, 0, llvm::MemoryBuffer::getMemBuffer(Code));
   const char *Args[] = {"clang", "-xc++", "-include", Header.c_str(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45679: [clang-tidy] Add a helper function isModified, that checks whether an expression is modified within a statement.

2018-04-17 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

*hust* /llvm/tools/clang/lib/Analysis/PseudoConstantAnalysis.cpp

I will check this one first, before we get crazy and implemented something 
twice.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45679



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


[PATCH] D45697: [clang-tidy] Fix clang-tidy doesn't read .clangtidy configuration file.

2018-04-17 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



Comment at: test/clang-tidy/read_file_config.cpp:1
+// RUN: mkdir -p %T/read-file-config/
+// RUN: cp %s %T/read-file-config/test.cpp

JonasToth wrote:
> Will this work on windows?
I believe it works on windows (although I don't have a windows machine) -- this 
command has been used in a few existing tests. 


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45697



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


[PATCH] D45601: Warn on bool* to bool conversion

2018-04-17 Thread Stephan Bergmann via Phabricator via cfe-commits
sberg added a comment.

A random data point from trying this patch on the LibreOffice code base:

- a little over 100 cases that are easily identified as false positives (many 
of the form "if (p) *p = ...")

- two or three cases that looked suspicious on first glance but turned out to 
be false positives too (but where the code apparently benefits from cleaning up)

- two cases of false positives that are awkward to silence (via "!= nullptr") 
in a way compatible with older C++ standards, as they are of the form "if (bool 
* p = ...)", and can be rewritten to "if (bool * p = ...; p != nullptr)" only 
for recent C++

- one false positive in system-provided /usr/include/qt5/QtCore/qstring.h

- no true positives found


https://reviews.llvm.org/D45601



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


[PATCH] D45601: Warn on bool* to bool conversion

2018-04-17 Thread Greg Parker via Phabricator via cfe-commits
gparker42 added a comment.

Note that we recently relaxed a similar diagnostic for `NSNumber *` in the 
static analyzer. Such code is semantically similar to `inttype *`.
https://reviews.llvm.org/D44044


https://reviews.llvm.org/D45601



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


[PATCH] D38615: [libclang] Only mark CXCursors for explicit attributes with a type

2018-04-17 Thread Manuel Klimek via Phabricator via cfe-commits
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.

lg


https://reviews.llvm.org/D38615



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


[PATCH] D45680: [C++2a] Add operator<=> Rewriting - Early Attempt

2018-04-17 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

Definitely some interesting questions to take to CWG here. :)




Comment at: lib/Sema/SemaOverload.cpp:9218-9219
+// --- F2 is a rewritten candidate ([over.match.oper]) and F1 is not.
+if (Cand2.getRewrittenKind() && !Cand1.getRewrittenKind())
+  return true;
+if (Cand1.getRewrittenKind() && Cand2.getRewrittenKind() &&

EricWF wrote:
> EricWF wrote:
> > EricWF wrote:
> > > rsmith wrote:
> > > > You also need to check the reverse condition and return false (the "or 
> > > > if not that" is ... somehow ... supposed to imply that).
> > > Hmm. So I'm wondering what is intended by the language `F1 and F2 are 
> > > rewritten candidates, and F2 is a synthesized candidate with reversed 
> > > order of parameters and F1 is not`. For example, what happens when 
> > > comparing two distinct member functions with only one explicit parameter?
> > > 
> > > ```
> > > struct T;
> > > struct U { auto operator<=>(T); };
> > > struct T { auto operator<=>(U); };
> > > auto r = T{} < U{}; // Are the synthesized and rewritten overloads 
> > > ambiguous? 
> > > ```
> > And what about 
> > 
> > ```
> > struct U {};
> > struct T { auto operator<=>(U); };
> > auto operator<=>(U, T);
> > auto r = T{} < U{};
> > ```
> Nevermind. I found the language. The implicit object parameter isn't 
> considered in this case. 
The parameters in scope in this rule are the parameters for the purpose of 
overload resolution, which include the implicit object parameter for a member 
function.

In your first example, both candidates have implicit conversion sequences with 
Exact Match rank for both parameters (and none of the ordering special cases 
apply), so we fall down to this tie-breaker and it selects the 
`T::operator<=>(U)` candidate, unless I'm missing something.

Your second example appears to receive the same treatment.



Comment at: lib/Sema/SemaOverload.cpp:12537-12565
+/// Rewritten candidates have been added but not checked for validity. They
+/// could still be non-viable if:
+///  (A) The rewritten call (x <=> y) is a builtin, but it will be ill-formed
+///  when built (for example it has narrowing conversions).
+///  (B) The expression (x <=> y) @ 0 is ill-formed for the result of (x <=> 
y).
+///
+/// If either is the case, this function should be considered non-viable and

EricWF wrote:
> EricWF wrote:
> > rsmith wrote:
> > > Hmm, this is an interesting approach. It's quite a divergence from our 
> > > usual approach, which is to perform the viability checks first, before 
> > > partial ordering, even if we could eliminate some candidates based on 
> > > partial ordering. And it's observable -- it will result in fewer template 
> > > instantiations being performed, which will mean we do not diagnose some 
> > > ill-formed (no diagnostic required) situations which would otherwise fail 
> > > due to an error in a non-immediate context.
> > > 
> > > Can you say something about why you chose this approach instead of doing 
> > > the lookup for operator `@` early and marking the candidate non-viable if 
> > > the lookup / overload resolution for it fails?
> > > 
> > > (I'm also vaguely wondering whether we should be pushing back on the C++ 
> > > committee for making viability of the rewritten candidate depend on 
> > > validity of the `@` expression, not just the `<=>` expression.)
> > The main reason for choosing this approach was the current expense of 
> > validating the candidates eagerly when they're added. Currently builtin 
> > candidates need to be fully built before their viability is known. My 
> > initial attempt at this made the compiler just about hang.
> > 
> > If I understand what you mean by performing lookup/overload resolution 
> > early for operator `@`, and I probably don't, then it would require 
> > performing overload resolution separately every rewritten candidate added, 
> > since each candidate could have a different return type.
> > 
> > I think rewritten builtins could be checked more eagerly by moving a bunch 
> > of the checking from `SemaExpr.cpp` to `SemaOverload.cpp`, and using it to 
> > eagerly check the bits we need to, but not actually building the expression 
> > as calling into `SemaExpr.cpp` would currently do.
> > 
> > As for the root question: Should the validity of the `@` expression be 
> > considered during overload resolution? I think the answer is tricky. For 
> > example, the following case could easily occur and break existing code, 
> > especially as more users add defaulted comparison operators. In this case 
> > if the base class adds <=> without the derived classes knowledge.
> > 
> > ```
> > struct B {
> >   using CallbackTy = void(*)();
> >   CallbackTy CB = nullptr; // B carries around a function pointer.
> > #if __cplusplus >= 201703L
> >   friend auto operator<=>(B const&, B const&) = default;
> > #else
> >   friend bool operator==(B, B);
> >   friend bool operator!=(B

[PATCH] D43341: [clang-doc] Implement reducer portion of the frontend framework

2018-04-17 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

OK, I didn't get through all the detail, but we should talk about the MR stuff 
today :)




Comment at: clang-doc/BitcodeReader.cpp:25
+void ClangDocBitcodeReader::storeData(SymbolID &Field, llvm::StringRef Blob) {
+  assert(Record[0] == 20);
+  // First position in the record is the length of the following array, so we

why is this true?
i.e. why can't I produce a malformed bitcode file that causes this assertion to 
fail?
(My usual understanding is that a failed assert is meant to indicate a 
programming error, if it means something else here like "malformed file" you 
should probably document that if you're sure it's what you want)



Comment at: clang-doc/BitcodeReader.cpp:42
+  llvm::StringRef Blob) {
+  Field = (AccessSpecifier)Record[0];
+}

Similarly, what guarantees Record[0] is in-range for AccessSpecifier? casting 
an out-of-range integer to an enum results in UB.



Comment at: clang-doc/BitcodeReader.h:11
+// This file implements a reader for parsing the clang-doc internal
+// representation to LLVM bitcode. The reader takes in a stream of bits and
+// generates the set of infos that it represents.

nit: *from* LLVM bitcode, or am I missing something?



Comment at: clang-doc/BitcodeReader.h:29
+// Class to read bitstream into an InfoSet collection
+class ClangDocBitcodeReader {
+public:

I found the implementation here a bit hard to understand at first, mostly 
because the state/invariants/responsibilities of each method weren't obvious to 
me.
Left some suggestions below to document these a bit more.

However many of these seem to in practice only share the Record buffer. It 
seems making this an explicit parameter would let you make these free 
functions, hiding them from the header and avoiding any possibility of spooky 
side-channel interactions. Up to you of course.



Comment at: clang-doc/BitcodeReader.h:46
+
+  // Reading records
+  template  bool readRecord(unsigned ID, TInfo &I);

I think this needs some elaboration :-)
e.g. Populates Record with the decoded record, and then calls one of the 
`parseRecord` overloads.

In general the data flow between these methods seems a little obscured by 
having Record be a field rather than a parameter as ID/Blob are.
(Incidentally I don't think you gain anything here from the use of SmallVector 
over std::vector, which is easier to type, but it doesn't really matter)



Comment at: clang-doc/BitcodeReader.h:49
+
+  bool parseRecord(unsigned ID, llvm::StringRef Blob, unsigned VersionNo);
+  bool parseRecord(unsigned ID, llvm::StringRef Blob, NamespaceInfo &I);

comment for these methods e.g. Populates the appropriate field of the Info 
object based on a decoded record whose content is in Record.



Comment at: clang-doc/BitcodeReader.h:59
+
+  void storeData(llvm::SmallVectorImpl &Field, llvm::StringRef Blob);
+  void storeData(bool &Field, llvm::StringRef Blob);

Comment for these like "decodes the blob part of a record" - or just rename 
these `decodeBlob` or similar?



Comment at: clang-doc/Reducer.cpp:17
+
+std::unique_ptr mergeInfos(tooling::ToolResults *Results) {
+  std::unique_ptr UniqueInfos = llvm::make_unique();

A little bit of context to document here: the fact that ToolResults values are 
bit-encoded InfoSets describing each TU in the codebase. (I think?)

(Also, why not return InfoSet by value?)



Comment at: clang-doc/Reducer.cpp:23
+ClangDocBitcodeReader Reader(Stream);
+if (!Reader.readBitstream(*UniqueInfos)) {
+  Err = true;

I don't see where any actual merging is happening.
Say we process TU 1 which has a decl of foo() with a doc comment, and TU 2 
which defines foo(). Now we're going to have something like:
ToolResults = {
 "tu1": "...bitcode...foo().../*does foo*/",
 "tu2": "...bitcode...foo()...Foo.cpp:53",
}
the first readBitstream call is going to ultimately call 
StoreData(UniqueInfos->Functions), which appends a new function to the array
then the second readBitstream call will do the same thing, and you have an 
InfoSet with two copies of foo()

what am I missing?



Comment at: clang-doc/Reducer.h:21
+// Combine occurrences of the same info across translation units.
+std::unique_ptr mergeInfos(tooling::ToolResults *Results);
+

This doesn't quite look like the typical reduce pattern. (And there's no 
framework or guidance for MRs in libtooling so this isn't surprising or maybe 
even a problem.

When the mapper emits its values, it usually chooses the keys so that a 
non-trivial merge is only required among values with the same key. (e.g. USR 
here)
This does two things:
1. it makes it really easy to parallelize: after the ma

[PATCH] D42966: Fix USR generation in the presence of #line directives or linemarkes

2018-04-17 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

Could you elaborate on why the old behaviour is broken?


Repository:
  rC Clang

https://reviews.llvm.org/D42966



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


[PATCH] D45680: [C++2a] Add operator<=> Rewriting - Early Attempt

2018-04-17 Thread Eric Fiselier via Phabricator via cfe-commits
EricWF added inline comments.



Comment at: lib/Sema/SemaOverload.cpp:9218-9219
+// --- F2 is a rewritten candidate ([over.match.oper]) and F1 is not.
+if (Cand2.getRewrittenKind() && !Cand1.getRewrittenKind())
+  return true;
+if (Cand1.getRewrittenKind() && Cand2.getRewrittenKind() &&

rsmith wrote:
> EricWF wrote:
> > EricWF wrote:
> > > EricWF wrote:
> > > > rsmith wrote:
> > > > > You also need to check the reverse condition and return false (the 
> > > > > "or if not that" is ... somehow ... supposed to imply that).
> > > > Hmm. So I'm wondering what is intended by the language `F1 and F2 are 
> > > > rewritten candidates, and F2 is a synthesized candidate with reversed 
> > > > order of parameters and F1 is not`. For example, what happens when 
> > > > comparing two distinct member functions with only one explicit 
> > > > parameter?
> > > > 
> > > > ```
> > > > struct T;
> > > > struct U { auto operator<=>(T); };
> > > > struct T { auto operator<=>(U); };
> > > > auto r = T{} < U{}; // Are the synthesized and rewritten overloads 
> > > > ambiguous? 
> > > > ```
> > > And what about 
> > > 
> > > ```
> > > struct U {};
> > > struct T { auto operator<=>(U); };
> > > auto operator<=>(U, T);
> > > auto r = T{} < U{};
> > > ```
> > Nevermind. I found the language. The implicit object parameter isn't 
> > considered in this case. 
> The parameters in scope in this rule are the parameters for the purpose of 
> overload resolution, which include the implicit object parameter for a member 
> function.
> 
> In your first example, both candidates have implicit conversion sequences 
> with Exact Match rank for both parameters (and none of the ordering special 
> cases apply), so we fall down to this tie-breaker and it selects the 
> `T::operator<=>(U)` candidate, unless I'm missing something.
> 
> Your second example appears to receive the same treatment.
Alright. So I found the wrong language. 

I think my first case meant to have `const &` parameters and `const` methods. 
That way the implicit object parameter type matched the reversed parameter 
type. The second example was meant to demonstrate the case where the implicit 
object parameter type doesn't match.

What type should we be comparing against in these cases?


Repository:
  rC Clang

https://reviews.llvm.org/D45680



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


[PATCH] D42966: Fix USR generation in the presence of #line directives or linemarkes

2018-04-17 Thread Mikhail Ramalho via Phabricator via cfe-commits
mikhail.ramalho added a comment.

Sure. Basically, the previous code would not generate the USR for the 
function's parameters.

The issue was that SM.getFileEntryForID would return NULL because there is no 
actual file, that's why I changed to get the presumedLoc and build the name 
using the column/line.

I know that using column/line not the preferable method to generate USR but I 
couldn't find a way to generate the offset of a presumed location.


Repository:
  rC Clang

https://reviews.llvm.org/D42966



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


[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

2018-04-17 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:9816
+RHS = S.ImpCastExprToType(RHS.get(), Type, CK_BitCast);
+  } else {
+// C++2a [expr.spaceship]p4

EricWF wrote:
> rsmith wrote:
> > We still need to apply the usual arithmetic conversions after converting 
> > enumerations to their underlying types (eg, `<=>` on `enum E : char` 
> > converts the operands first to `char` then to `int`). You could remove the 
> > `else` here and make this stuff unconditional, but it's probably better to 
> > sidestep the extra work and convert directly to the promoted type of the 
> > enum's underlying type.
> Do we still do usual arithmetic conversions if we have two enumerations of 
> the same type?
Formally, yes: "If both operands have the same enumeration type E, the operator 
yields the result of converting the operands to the underlying type of E and 
applying <=> to the converted operands."

The recursive application of `<=>` to the converted operands will perform the 
usual arithmetic conversions.



Comment at: lib/Sema/SemaExpr.cpp:9794
+  // other is not, the program is ill-formed.
+  if (int Count = LHSType->isBooleanType() + RHSType->isBooleanType()) {
+// TODO: What about bool non-narrowing cases? Like '0' or '1.

You could simplify this to `if (LHSType->isBooleanType() != 
RHSType->isBooleanType()) InvalidOperands`.



Comment at: lib/Sema/SemaExpr.cpp:9795
+  if (int Count = LHSType->isBooleanType() + RHSType->isBooleanType()) {
+// TODO: What about bool non-narrowing cases? Like '0' or '1.
+if (Count != 2) {

Missing `'`. Seems like a question to take to Herb and CWG (and maybe EWG), but 
I suspect the answer will be "this does what we wanted".

Looks like P0946R0 missed this case of a difference between `<=>` and other 
operators... oops.



Comment at: lib/Sema/SemaExpr.cpp:9829-9832
+if (EDecl->isScoped()) {
+  S.InvalidOperands(Loc, LHS, RHS);
+  return QualType();
+}

I don't think you need this check: the usual arithmetic conversions will fail 
to find a common type if the enum is scoped.



Comment at: lib/Sema/SemaExpr.cpp:9881-9890
   enum { StrongEquality, PartialOrdering, StrongOrdering } Ordering;
   if (Type->isAnyComplexType())
 Ordering = StrongEquality;
   else if (Type->isFloatingType())
 Ordering = PartialOrdering;
   else
 Ordering = StrongOrdering;

This is now over-complicated, since this is unreachable for `BO_Cmp`.



Comment at: lib/Sema/SemaExpr.cpp:9906
+  bool IsThreeWay = Opc == BO_Cmp;
+  auto IsPointerType = [](ExprResult E) {
+QualType Ty = E.get()->getType().getNonReferenceType();

I'd prefer a name that captures that this also recognizes member pointer types.



Comment at: lib/Sema/SemaExpr.cpp:9907
+  auto IsPointerType = [](ExprResult E) {
+QualType Ty = E.get()->getType().getNonReferenceType();
+return Ty->isPointerType() || Ty->isMemberPointerType();

You don't need a `.getNonReferenceType()` here; expressions can't have 
reference type.


https://reviews.llvm.org/D45476



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


[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

2018-04-17 Thread Whisperity via Phabricator via cfe-commits
whisperity added a comment.

In https://reviews.llvm.org/D45532#1068700, @Szelethus wrote:

> In https://reviews.llvm.org/D45532#1068647, @dkrupp wrote:
>
> > This bug report also mentions assignment operator. But for that a warning 
> > may be not so useful. In that case the members of the assigned to object 
> > should have some initialized value already which the programmer may not 
> > want to overwrite in the assignment operator.
>
>
> I believe there's a checker for that already, but I'm really not sure whether 
> `UndefinedAssignmentChecker` covers all such cases.


Indeed, there are two different targets for analysis here:

- The `rhs` of the assignment contains an uninitialised value, that is 
copied/moved into the `this` of the assignment. --> This //should// be checked 
by `UndefinedAssignmentChecker`.
- Not every field is initialised by the assignment operator. This one, I 
believe, is hard to check and prove, and also feels superfluous. As @NoQ wrote 
earlier, the amount of reports this checker emits in itself is, at least 
expected to be, huge, and while it can be a valid programming approach that a 
ctor must initialise all, tracking in the CSA on an `operator=` that the 
`this`-side had something uninitialised but it was also not initialised by the 
assignment? Hard, and also not very useful, as far as I see.



In https://reviews.llvm.org/D45532#1064652, @NoQ wrote:

> That said, your checker finds non-bugs, which is always risky. Sometimes the 
> user should be allowed to initialize the field after construction but before 
> the first use as a potentially important performance optimization. Did you 
> find any such intentionally uninitialized fields with your checker? Were 
> there many of those?


Where I can vision the usefulness of this checker **the most** is code that is 
evolving. There are many communities and codebases where coding standards and 
practices aren't laid out in a well-formed manner like LLVM has. There are also 
projects which are traditionally C code that has evolved into C++-ish code. 
When moving into C++, people start to realise they can write things like 
constructors, so they write them, but then leave out some members, and we can 
only guess (and **perhaps** make use of other checkers related to dereference 
or read of undefineds) what needs to be initialised, what is used without 
initialisation.

This checker is more close to something like a `bugprone-` Tidy matcher from 
the user's perspective, but proper analysis requires it to be executed in the 
SA.

A valid constriction of what this checker can find could be members that can 
"seemingly" be only be set in the constructor, there are no write operations or 
public exposure on them.

I have reviewed some findings of the checker, consider the following very 
trivial example:

  #define HEAD_ELEMENT_SPECIFIER 0xDEADF00D // some magic yadda
  
  struct linked_list;
  
  struct linked_list
  {
  int elem;
  linked_list* next;
  };
  
  class some_information_chain_class
  {
public:
  some_information_chain_class() : iList(new linked_list())
  {
iList->elem = HEAD_ELEMENT_SPECIFIER;
  }
  
private:
  linked_list* iList;
  };

In this case, the "next" is in many cases remain as a garbage value. Of course, 
the checker cannot know, if, for example, there is also a `count` field and we 
don't rely on `next == nullptr` to signify the end of the list. But this can 
very well be a mistake from the programmer's end.

> If a user wants to have his codebase "analyzer-clean", would he be able to 
> suppress the warning without changing the behavior of the program?

I'm not entirely sure what you mean by "changing the behaviour of the program". 
As far as I can see, this checker only reads information from the SA, so it 
should not mess up any preconception set or state potentially used by other 
checkers.

---

In https://reviews.llvm.org/D45532#1065716, @Szelethus wrote:

> I didn't implement anything explicitly to suppress warnings, so if there is 
> nothing in the CSA by default for it, I'm afraid this isn't possible (just 
> yet).


So far the only thing I can think of is coming up with new command-line 
arguments that can fine-tune the behaviour of the checker - but in this case, 
you can just as well just toggle the whole thing to be disabled.

An improvement heuristic (implemented in a later patch, toggleable with a 
command-line option perhaps?), as discussed above, would be checking //only// 
for members for whom there isn't any "setter" capability anywhere in the type.


https://reviews.llvm.org/D45532



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


[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

2018-04-17 Thread Whisperity via Phabricator via cfe-commits
whisperity requested changes to this revision.
whisperity added a comment.
This revision now requires changes to proceed.

There is something that came up in my mind:

Consider a construct like this:

  class A
  {
A()
{
  memset(X, 0, 10 * sizeof(int));
}
  
int X[10];
  };

I think it's worthy for a test case if this `X` is considered unitialised at 
the end of the constructor or not. (And if not, a ticket, or a fix for SA in a 
subpatch.)


https://reviews.llvm.org/D45532



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


[PATCH] D45697: [clang-tidy] Fix clang-tidy doesn't read .clangtidy configuration file.

2018-04-17 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.

Thank you for investigating and fixing this!

One comment re: the test below.




Comment at: test/clang-tidy/read_file_config.cpp:5
+// RUN: echo '[{"command": "cc -c -o test.o test.cpp", "directory": 
"%T/read-file-config", "file": "%T/read-file-config/test.cpp"}]' > 
%T/read-file-config/compile_commands.json
+// RUN: clang-tidy %T/read-file-config/test.cpp | not grep "clang-analyzer-*"
+

Please add a sanity check to ensure the code triggers the static analyzer 
check, when enabled explicitly: 

// RUN: clang-tidy -checks=-*,clang-analyzer-* %T/read-file-config/test.cpp | 
grep "clang-analyzer-*"


(hint: it won't pass, since your regex in grep is incorrect ;)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45697



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


[PATCH] D45718: Allow registering custom statically-linked analyzer checkers

2018-04-17 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh created this revision.
alexfh added a reviewer: george.karpenkov.
Herald added a subscriber: a.sidorin.

Add an extension point to allow registration of statically-linked Clang Static
Analyzer checkers that are not a part of the Clang tree. This extension point
employs the mechanism used when checkers are registered from dynamically loaded
plugins.


Repository:
  rC Clang

https://reviews.llvm.org/D45718

Files:
  include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h
  include/clang/StaticAnalyzer/Frontend/CheckerRegistration.h
  lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp

Index: lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
===
--- lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
+++ lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
@@ -111,16 +111,21 @@
   return checkerOpts;
 }
 
-std::unique_ptr
-ento::createCheckerManager(AnalyzerOptions &opts, const LangOptions &langOpts,
-   ArrayRef plugins,
-   DiagnosticsEngine &diags) {
+std::unique_ptr ento::createCheckerManager(
+AnalyzerOptions &opts, const LangOptions &langOpts,
+ArrayRef plugins,
+ArrayRef> checkerRegistrationFns,
+DiagnosticsEngine &diags) {
   std::unique_ptr checkerMgr(
   new CheckerManager(langOpts, opts));
 
   SmallVector checkerOpts = getCheckerOptList(opts);
 
   ClangCheckerRegistry allCheckers(plugins, &diags);
+
+  for (const auto &Fn : checkerRegistrationFns)
+Fn(allCheckers);
+
   allCheckers.initializeManager(*checkerMgr, checkerOpts);
   allCheckers.validateCheckerOptions(opts, diags);
   checkerMgr->finishedCheckerRegistration();
Index: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -164,6 +164,8 @@
   /// Bug Reporter to use while recursively visiting Decls.
   BugReporter *RecVisitorBR;
 
+  std::vector> CheckerRegistrationFns;
+
 public:
   ASTContext *Ctx;
   const Preprocessor &PP;
@@ -293,8 +295,9 @@
 
   void Initialize(ASTContext &Context) override {
 Ctx = &Context;
-checkerMgr = createCheckerManager(*Opts, PP.getLangOpts(), Plugins,
-  PP.getDiagnostics());
+checkerMgr =
+createCheckerManager(*Opts, PP.getLangOpts(), Plugins,
+ CheckerRegistrationFns, PP.getDiagnostics());
 
 Mgr = llvm::make_unique(
 *Ctx, PP.getDiagnostics(), PP.getLangOpts(), PathConsumers,
@@ -385,6 +388,10 @@
 PathConsumers.push_back(Consumer);
   }
 
+  void AddCheckerRegistrationFn(std::function Fn) override {
+CheckerRegistrationFns.push_back(std::move(Fn));
+  }
+
 private:
   void storeTopLevelDecls(DeclGroupRef DG);
   std::string getFunctionName(const Decl *D);
Index: include/clang/StaticAnalyzer/Frontend/CheckerRegistration.h
===
--- include/clang/StaticAnalyzer/Frontend/CheckerRegistration.h
+++ include/clang/StaticAnalyzer/Frontend/CheckerRegistration.h
@@ -11,6 +11,7 @@
 #define LLVM_CLANG_STATICANALYZER_FRONTEND_CHECKERREGISTRATION_H
 
 #include "clang/Basic/LLVM.h"
+#include 
 #include 
 #include 
 
@@ -21,10 +22,13 @@
 
 namespace ento {
   class CheckerManager;
+  class CheckerRegistry;
 
-  std::unique_ptr
-  createCheckerManager(AnalyzerOptions &opts, const LangOptions &langOpts,
-   ArrayRef plugins, DiagnosticsEngine &diags);
+  std::unique_ptr createCheckerManager(
+  AnalyzerOptions &opts, const LangOptions &langOpts,
+  ArrayRef plugins,
+  ArrayRef> checkerRegistrationFns,
+  DiagnosticsEngine &diags);
 
 } // end ento namespace
 
Index: include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h
===
--- include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h
+++ include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h
@@ -17,6 +17,7 @@
 
 #include "clang/AST/ASTConsumer.h"
 #include "clang/Basic/LLVM.h"
+#include 
 #include 
 
 namespace clang {
@@ -29,10 +30,24 @@
 namespace ento {
 class PathDiagnosticConsumer;
 class CheckerManager;
+class CheckerRegistry;
 
 class AnalysisASTConsumer : public ASTConsumer {
 public:
   virtual void AddDiagnosticConsumer(PathDiagnosticConsumer *Consumer) = 0;
+
+  /// This method allows registering statically linked custom checkers that are
+  /// not a part of the Clang tree. It employs the same mechanism that is used
+  /// by plugins.
+  ///
+  /// Example:
+  ///
+  ///   Consumer->AddCheckerRegistrationFn([] (CheckerRegistry& Registry) {
+  /// Registry.addChecker("example.MyCustomChecker",
+  ///  "Description");
+  ///   });
+  virtual void
+  AddCheckerRegistrationFn(std::function F

[PATCH] D45615: [builtins] __builtin_dump_struct : added more types format

2018-04-17 Thread Paul Semel via Phabricator via cfe-commits
paulsemel updated this revision to Diff 142750.
paulsemel added a comment.

Removed the two existing types.
Added the correct format sizing.
Added tests for those formats.
These version is now relying on the previous patch : 
https://reviews.llvm.org/D45665


Repository:
  rC Clang

https://reviews.llvm.org/D45615

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/dump-struct-builtin.c


Index: test/CodeGen/dump-struct-builtin.c
===
--- test/CodeGen/dump-struct-builtin.c
+++ test/CodeGen/dump-struct-builtin.c
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm %s -o - | 
FileCheck %s
 
 #include "Inputs/stdio.h"
+#include 
 
 // CHECK: @unit1.a = private unnamed_addr constant %struct.U1A { i16 12 }, 
align 2
 // CHECK-NEXT: [[STRUCT_STR_U1:@[0-9]+]] = private unnamed_addr constant [14 x 
i8] c"struct U1A {\0A\00"
@@ -94,6 +95,18 @@
 // CHECK-NEXT: [[FORMAT_U15:@[0-9]+]] = private unnamed_addr constant [4 x i8] 
c"%p\0A\00"
 // CHECK-NEXT: [[END_STRUCT_U15:@[0-9]+]] = private unnamed_addr constant [3 x 
i8] c"}\0A\00"
 
+// CHECK: @unit16.a = private unnamed_addr constant %struct.U16A { i8 12 }, 
align 1
+// CHECK-NEXT: [[STRUCT_STR_U16:@[0-9]+]] = private unnamed_addr constant [15 
x i8] c"struct U16A {\0A\00"
+// CHECK-NEXT: [[FIELD_U16:@[0-9]+]] = private unnamed_addr constant [13 x i8] 
c"uint8_t a : \00"
+// CHECK-NEXT: [[FORMAT_U16:@[0-9]+]] = private unnamed_addr constant [6 x i8] 
c"%hhu\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U16:@[0-9]+]] = private unnamed_addr constant [3 x 
i8] c"}\0A\00"
+
+// CHECK: @unit17.a = private unnamed_addr constant %struct.U17A { i8 12 }, 
align 1
+// CHECK-NEXT: [[STRUCT_STR_U17:@[0-9]+]] = private unnamed_addr constant [15 
x i8] c"struct U17A {\0A\00"
+// CHECK-NEXT: [[FIELD_U17:@[0-9]+]] = private unnamed_addr constant [12 x i8] 
c"int8_t a : \00"
+// CHECK-NEXT: [[FORMAT_U17:@[0-9]+]] = private unnamed_addr constant [6 x i8] 
c"%hhd\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U17:@[0-9]+]] = private unnamed_addr constant [3 x 
i8] c"}\0A\00"
+
 int printf(const char *fmt, ...) {
 return 0;
 }
@@ -368,6 +381,42 @@
   __builtin_dump_struct(&a, &printf);
 }
 
+void unit16() {
+  struct U16A {
+uint8_t a;
+  };
+
+  struct U16A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([15 x i8], 
[15 x i8]* [[STRUCT_STR_U16]], i32 0, i32 0))
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U16A, 
%struct.U16A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([13 x i8], 
[13 x i8]* [[FIELD_U16]], i32 0, i32 0))
+  // CHECK: [[LOAD1:%[0-9]+]] = load i8, i8* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([6 x i8], 
[6 x i8]* [[FORMAT_U16]], i32 0, i32 0), i8 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([3 x i8], 
[3 x i8]* [[END_STRUCT_U16]], i32 0, i32 0)
+  __builtin_dump_struct(&a, &printf);
+}
+
+void unit17() {
+  struct U17A {
+int8_t a;
+  };
+
+  struct U17A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([15 x i8], 
[15 x i8]* [[STRUCT_STR_U17]], i32 0, i32 0))
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U17A, 
%struct.U17A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([12 x i8], 
[12 x i8]* [[FIELD_U17]], i32 0, i32 0))
+  // CHECK: [[LOAD1:%[0-9]+]] = load i8, i8* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([6 x i8], 
[6 x i8]* [[FORMAT_U17]], i32 0, i32 0), i8 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([3 x i8], 
[3 x i8]* [[END_STRUCT_U17]], i32 0, i32 0)
+  __builtin_dump_struct(&a, &printf);
+}
+
 void test1() {
   struct T1A {
 int a;
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -949,6 +949,8 @@
   if (Types.empty()) {
 Types[Context.CharTy] = "%c";
 Types[Context.BoolTy] = "%d";
+Types[Context.SignedCharTy] = "%hhd";
+Types[Context.UnsignedCharTy] = "%hhu";
 Types[Context.IntTy] = "%d";
 Types[Context.UnsignedIntTy] = "%u";
 Types[Context.LongTy] = "%ld";


Index: test/CodeGen/dump-struct-builtin.c
===
--- test/CodeGen/dump-struct-builtin.c
+++ test/CodeGen/dump-struct-builtin.c
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
 
 #include "Inputs/stdio.h"
+#include 
 
 // CHECK: @unit1.a = private unnamed_addr constant %struct.U1A { i16 12 }, align 2
 // CHECK-NEXT: [[STRUCT_STR_U1:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U1A {\0A\00"
@@ -94,6 +95,18 @@
 // CHECK-NEXT: [[FORMAT_U15:@[0-9]+]] = private unnamed_addr constant [4 x i8] c"%p\0A\00"
 // 

r330184 - Add some infuriatingly necessary comments to this test case.

2018-04-17 Thread Chandler Carruth via cfe-commits
Author: chandlerc
Date: Tue Apr 17 04:08:05 2018
New Revision: 330184

URL: http://llvm.org/viewvc/llvm-project?rev=330184&view=rev
Log:
Add some infuriatingly necessary comments to this test case.

Without these comments, by "luck" the contents of SomeKit's SKWidget.h
are precisely the same as SomeKitCore's SomeKitCore.h. This can create
havoc if anything canonicalizes on the inode and your filesystem assigns
a common inode to files with identical file content. Alternatively, if
your build system uses symlinks into a content-addressed-storage (as
Google's does), you end up with these files being symlinks to the same
file.

The end result is that Clang deduplicates them internally, and then
believes that the SomeKit framework includes the SomeKitCore.h header,
and does not include the SKWidget.h in SomeKit. This in turn results in
warnings in this test and eventually errors as Clang becomes confused
because the umbrella header for SomeKitCore has already been included
into another framework's module (SomeKit). Yay.

If anyone has a better idea about how to avoid this, I'm all ears.
Nothing other than causing the file content to change worked for me.

Modified:

cfe/trunk/test/Modules/Inputs/exportas-link/OtherKit.framework/Headers/OtherKit.h

cfe/trunk/test/Modules/Inputs/exportas-link/SomeKit.framework/Headers/SKWidget.h

cfe/trunk/test/Modules/Inputs/exportas-link/SomeKit.framework/Headers/SomeKit.h

cfe/trunk/test/Modules/Inputs/exportas-link/SomeKitCore.framework/Headers/SomeKitCore.h

Modified: 
cfe/trunk/test/Modules/Inputs/exportas-link/OtherKit.framework/Headers/OtherKit.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/exportas-link/OtherKit.framework/Headers/OtherKit.h?rev=330184&r1=330183&r2=330184&view=diff
==
--- 
cfe/trunk/test/Modules/Inputs/exportas-link/OtherKit.framework/Headers/OtherKit.h
 (original)
+++ 
cfe/trunk/test/Modules/Inputs/exportas-link/OtherKit.framework/Headers/OtherKit.h
 Tue Apr 17 04:08:05 2018
@@ -1,3 +1,4 @@
+// Umbrella header for OtherKit.
 #import 
 
 #ifdef F

Modified: 
cfe/trunk/test/Modules/Inputs/exportas-link/SomeKit.framework/Headers/SKWidget.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/exportas-link/SomeKit.framework/Headers/SKWidget.h?rev=330184&r1=330183&r2=330184&view=diff
==
--- 
cfe/trunk/test/Modules/Inputs/exportas-link/SomeKit.framework/Headers/SKWidget.h
 (original)
+++ 
cfe/trunk/test/Modules/Inputs/exportas-link/SomeKit.framework/Headers/SKWidget.h
 Tue Apr 17 04:08:05 2018
@@ -1 +1,2 @@
+// Delegate to SomeKitCore.
 #import 

Modified: 
cfe/trunk/test/Modules/Inputs/exportas-link/SomeKit.framework/Headers/SomeKit.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/exportas-link/SomeKit.framework/Headers/SomeKit.h?rev=330184&r1=330183&r2=330184&view=diff
==
--- 
cfe/trunk/test/Modules/Inputs/exportas-link/SomeKit.framework/Headers/SomeKit.h 
(original)
+++ 
cfe/trunk/test/Modules/Inputs/exportas-link/SomeKit.framework/Headers/SomeKit.h 
Tue Apr 17 04:08:05 2018
@@ -1 +1,6 @@
+// Umbrella header for SomeKit.
+//
+// Note that this file's content must not end up to coincidentally be identical
+// to any other file in the test which can easily happen given the reduced
+// test.
 #import 

Modified: 
cfe/trunk/test/Modules/Inputs/exportas-link/SomeKitCore.framework/Headers/SomeKitCore.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/exportas-link/SomeKitCore.framework/Headers/SomeKitCore.h?rev=330184&r1=330183&r2=330184&view=diff
==
--- 
cfe/trunk/test/Modules/Inputs/exportas-link/SomeKitCore.framework/Headers/SomeKitCore.h
 (original)
+++ 
cfe/trunk/test/Modules/Inputs/exportas-link/SomeKitCore.framework/Headers/SomeKitCore.h
 Tue Apr 17 04:08:05 2018
@@ -1 +1,6 @@
+// Umbrella header for SomeKitCore.
+//
+// Note that this file's content must not end up to coincidentally be identical
+// to any other file in the test which can easily happen given the reduced
+// test.
 #import 


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


[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

2018-04-17 Thread Henry Wong via Phabricator via cfe-commits
MTC added a comment.

In https://reviews.llvm.org/D45532#1069683, @whisperity wrote:

> There is something that came up in my mind:
>
> Consider a construct like this:
>
>   class A
>   {
> A()
> {
>   memset(X, 0, 10 * sizeof(int));
> }
>  
> int X[10];
>   };
>
>
> I think it's worthy for a test case if this `X` is considered unitialised at 
> the end of the constructor or not. (And if not, a ticket, or a fix for SA in 
> a subpatch.)


Just for a reminding, there is a very primitive patch to model `memset()`, see 
D44934 . And if we think that `memset` is a 
way of initialization, we may need to distinguish between POD objects and 
non-POD Objects.


https://reviews.llvm.org/D45532



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


[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

2018-04-17 Thread Andrew V. Tischenko via Phabricator via cfe-commits
avt77 updated this revision to Diff 142755.
avt77 added a comment.

I moved FrontendTiming.cpp from lib/Basic/ to lib/Frontend/.


https://reviews.llvm.org/D45619

Files:
  include/clang/Frontend/Utils.h
  lib/CodeGen/BackendUtil.cpp
  lib/CodeGen/CodeGenAction.cpp
  lib/Frontend/CMakeLists.txt
  lib/Frontend/FrontendTiming.cpp
  test/Frontend/ftime-report-template-decl.cpp

Index: test/Frontend/ftime-report-template-decl.cpp
===
--- test/Frontend/ftime-report-template-decl.cpp
+++ test/Frontend/ftime-report-template-decl.cpp
@@ -0,0 +1,159 @@
+// RUN: %clang %s -S -o - -ftime-report  2>&1 | FileCheck %s
+// RUN: %clang %s -S -o - -fdelayed-template-parsing -DDELAYED_TEMPLATE_PARSING -ftime-report  2>&1 | FileCheck %s
+
+// Template function declarations
+template 
+void foo();
+template 
+void foo();
+
+// Template function definitions.
+template 
+void foo() {}
+
+// Template class (forward) declarations
+template 
+struct A;
+template 
+struct b;
+template 
+struct C;
+template 
+struct D;
+
+// Forward declarations with default parameters?
+template 
+class X1;
+template 
+class X2;
+
+// Forward declarations w/template template parameters
+template  class T>
+class TTP1;
+template  class>
+class TTP2;
+template  class T>
+class TTP5;
+
+// Forward declarations with non-type params
+template 
+class NTP0;
+template 
+class NTP1;
+template 
+class NTP2;
+template 
+class NTP3;
+template 
+class NTP4;
+template 
+class NTP5;
+template 
+class NTP6;
+template 
+class NTP7;
+
+// Template class declarations
+template 
+struct A {};
+template 
+struct B {};
+
+namespace PR6184 {
+namespace N {
+template 
+void bar(typename T::x);
+}
+
+template 
+void N::bar(typename T::x) {}
+}
+
+// This PR occurred only in template parsing mode.
+namespace PR17637 {
+template 
+struct L {
+  template 
+  struct O {
+template 
+static void Fun(U);
+  };
+};
+
+template 
+template 
+template 
+void L::O::Fun(U) {}
+
+void Instantiate() { L<0>::O::Fun(0); }
+}
+
+namespace explicit_partial_specializations {
+typedef char (&oneT)[1];
+typedef char (&twoT)[2];
+typedef char (&threeT)[3];
+typedef char (&fourT)[4];
+typedef char (&fiveT)[5];
+typedef char (&sixT)[6];
+
+char one[1];
+char two[2];
+char three[3];
+char four[4];
+char five[5];
+char six[6];
+
+template 
+struct bool_ { typedef int type; };
+template <>
+struct bool_ {};
+
+#define XCAT(x, y) x##y
+#define CAT(x, y) XCAT(x, y)
+#define sassert(_b_) bool_<(_b_)>::type CAT(var, __LINE__);
+
+template 
+struct L {
+  template 
+  struct O {
+template 
+static oneT Fun(U);
+  };
+};
+template 
+template 
+template 
+oneT L::O::Fun(U) { return one; }
+
+template <>
+template <>
+template 
+oneT L<0>::O::Fun(U) { return one; }
+
+void Instantiate() {
+  sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
+  sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
+}
+}
+
+template 
+struct Foo {
+  template 
+  using rebind_alloc = _Other;
+};
+template 
+struct _Wrap_alloc {
+  template 
+  using rebind_alloc = typename Foo<_Alloc>::template rebind_alloc<_Other>;
+  template 
+  using rebind = _Wrap_alloc;
+};
+_Wrap_alloc::rebind w;
+
+// CHECK: Miscellaneous Ungrouped Timers
+// CHECK: Code Generation Time
+// CHECK: LLVM IR Generation Time
+// CHECK: Total
+// CHECK: Clang front-end time report
+// CHECK: Clang front-end timer
+// CHECK: Total
Index: lib/Frontend/FrontendTiming.cpp
===
--- lib/Frontend/FrontendTiming.cpp
+++ lib/Frontend/FrontendTiming.cpp
@@ -0,0 +1,20 @@
+//===- FronendTiming.cpp - Implements Frontend timing utils  --===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file keps implementation of frontend timing utils.
+//
+//===--===//
+
+#include "llvm/Support/Timer.h"
+
+namespace clang {
+
+bool FrontendTimesIsEnabled = false;
+
+}
Index: lib/Frontend/CMakeLists.txt
===
--- lib/Frontend/CMakeLists.txt
+++ lib/Frontend/CMakeLists.txt
@@ -29,6 +29,7 @@
   FrontendAction.cpp
   FrontendActions.cpp
   FrontendOptions.cpp
+  FrontendTiming.cpp
   HeaderIncludeGen.cpp
   InitHeaderSearch.cpp
   InitPreprocessor.cpp
Index: lib/CodeGen/CodeGenAction.cpp
===
--- lib/CodeGen/CodeGenAction.cpp
+++ lib/CodeGen/CodeGenAction.cpp
@@ -126,7 +126,7 @@
   Gen(CreateLLVMCodeGen(Diags, InFile, HeaderSearchOpts, PPOpts,
 CodeGenOpts, C, CoverageInfo)),
   LinkModules(std::move(LinkModules)) {
-  llvm::TimePassesIsEnabled = TimePasses;
+  Fronte

[PATCH] D45697: [clang-tidy] Fix clang-tidy doesn't read .clangtidy configuration file.

2018-04-17 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 142759.
hokein marked an inline comment as done.
hokein added a comment.

Add one more test to ensure the test code triggering "clang-analyzer-*" checks.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45697

Files:
  clang-tidy/ClangTidyOptions.cpp
  clang-tidy/ClangTidyOptions.h
  clang-tidy/tool/ClangTidyMain.cpp
  test/clang-tidy/read_file_config.cpp

Index: test/clang-tidy/read_file_config.cpp
===
--- /dev/null
+++ test/clang-tidy/read_file_config.cpp
@@ -0,0 +1,12 @@
+// RUN: mkdir -p %T/read-file-config/
+// RUN: cp %s %T/read-file-config/test.cpp
+// RUN: echo 'Checks: "-*,modernize-use-nullptr"' > %T/read-file-config/.clang-tidy
+// RUN: echo '[{"command": "cc -c -o test.o test.cpp", "directory": "%T/read-file-config", "file": "%T/read-file-config/test.cpp"}]' > %T/read-file-config/compile_commands.json
+// RUN: clang-tidy %T/read-file-config/test.cpp | not grep "clang-analyzer-.*"
+// RUN: clang-tidy -checks="-*,clang-analyzer-*" %T/read-file-config/test.cpp | grep "clang-analyzer-.*"
+
+void f() {
+  int x;
+  x = 1;
+  x = 2;
+}
Index: clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -286,7 +286,8 @@
   OS.flush();
 }
 
-static std::unique_ptr createOptionsProvider() {
+static std::unique_ptr createOptionsProvider(
+   llvm::IntrusiveRefCntPtr FS) {
   ClangTidyGlobalOptions GlobalOptions;
   if (std::error_code Err = parseLineFilter(LineFilter, GlobalOptions)) {
 llvm::errs() << "Invalid LineFilter: " << Err.message() << "\n\nUsage:\n";
@@ -334,7 +335,7 @@
 }
   }
   return llvm::make_unique(GlobalOptions, DefaultOptions,
-OverrideOptions);
+OverrideOptions, std::move(FS));
 }
 
 llvm::IntrusiveRefCntPtr
@@ -364,8 +365,13 @@
 static int clangTidyMain(int argc, const char **argv) {
   CommonOptionsParser OptionsParser(argc, argv, ClangTidyCategory,
 cl::ZeroOrMore);
+  llvm::IntrusiveRefCntPtr BaseFS(
+  VfsOverlay.empty() ? vfs::getRealFileSystem()
+ : getVfsOverlayFromFile(VfsOverlay));
+  if (!BaseFS)
+return 1;
 
-  auto OwningOptionsProvider = createOptionsProvider();
+  auto OwningOptionsProvider = createOptionsProvider(BaseFS);
   auto *OptionsProvider = OwningOptionsProvider.get();
   if (!OptionsProvider)
 return 1;
@@ -432,12 +438,6 @@
 llvm::cl::PrintHelpMessage(/*Hidden=*/false, /*Categorized=*/true);
 return 1;
   }
-  llvm::IntrusiveRefCntPtr BaseFS(
-  VfsOverlay.empty() ? vfs::getRealFileSystem()
- : getVfsOverlayFromFile(VfsOverlay));
-  if (!BaseFS)
-return 1;
-
   ProfileData Profile;
 
   llvm::InitializeAllTargetInfos();
Index: clang-tidy/ClangTidyOptions.h
===
--- clang-tidy/ClangTidyOptions.h
+++ clang-tidy/ClangTidyOptions.h
@@ -13,7 +13,9 @@
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/Support/ErrorOr.h"
+#include "clang/Basic/VirtualFileSystem.h"
 #include 
 #include 
 #include 
@@ -221,7 +223,8 @@
   /// whatever options are read from the configuration file.
   FileOptionsProvider(const ClangTidyGlobalOptions &GlobalOptions,
   const ClangTidyOptions &DefaultOptions,
-  const ClangTidyOptions &OverrideOptions);
+  const ClangTidyOptions &OverrideOptions,
+  llvm::IntrusiveRefCntPtr FS = nullptr);
 
   /// \brief Initializes the \c FileOptionsProvider instance with a custom set
   /// of configuration file handlers.
@@ -255,6 +258,7 @@
   llvm::StringMap CachedOptions;
   ClangTidyOptions OverrideOptions;
   ConfigFileHandlers ConfigHandlers;
+  llvm::IntrusiveRefCntPtr FS;
 };
 
 /// \brief Parses LineFilter from JSON and stores it to the \p Options.
Index: clang-tidy/ClangTidyOptions.cpp
===
--- clang-tidy/ClangTidyOptions.cpp
+++ clang-tidy/ClangTidyOptions.cpp
@@ -204,9 +204,12 @@
 FileOptionsProvider::FileOptionsProvider(
 const ClangTidyGlobalOptions &GlobalOptions,
 const ClangTidyOptions &DefaultOptions,
-const ClangTidyOptions &OverrideOptions)
+const ClangTidyOptions &OverrideOptions,
+llvm::IntrusiveRefCntPtr VFS)
 : DefaultOptionsProvider(GlobalOptions, DefaultOptions),
-  OverrideOptions(OverrideOptions) {
+  OverrideOptions(OverrideOptions), FS(std::move(VFS)) {
+  if (!FS)
+FS = vfs::getRealFileSystem();
   ConfigHandlers.emplace_back(".clang-tidy", parseConfiguration);
 }
 
@@ -224,14 +227,19 @@
 std::vector
 FileOptionsProvider::getRawOptions(Str

[PATCH] D45697: [clang-tidy] Fix clang-tidy doesn't read .clangtidy configuration file.

2018-04-17 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



Comment at: test/clang-tidy/read_file_config.cpp:5
+// RUN: echo '[{"command": "cc -c -o test.o test.cpp", "directory": 
"%T/read-file-config", "file": "%T/read-file-config/test.cpp"}]' > 
%T/read-file-config/compile_commands.json
+// RUN: clang-tidy %T/read-file-config/test.cpp | not grep "clang-analyzer-*"
+

alexfh wrote:
> Please add a sanity check to ensure the code triggers the static analyzer 
> check, when enabled explicitly: 
> 
> // RUN: clang-tidy -checks=-*,clang-analyzer-* %T/read-file-config/test.cpp | 
> grep "clang-analyzer-*"
> 
> 
> (hint: it won't pass, since your regex in grep is incorrect ;)
Done. This is valid regex, but doesn't do the expect thing, we expect to match 
the whole check name, which should be `clang-analyzer-.*`


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45697



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


[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

2018-04-17 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus updated this revision to Diff 142760.
Szelethus added a comment.

In https://reviews.llvm.org/D45532#1068700, @Szelethus wrote:

> Also, I managed to cause a crash with the class `linked_ptr_internal` from 
> google's boringssl when I analyzed the grpc project. I'll look deeper into 
> this, but I have a strong suspicion that the error lies within the CSA core.


I was very much wrong on that, the checker didn't handle cyclic references. I 
fixed that and some other crashes I encountered when checking the LLVM project.

To summarize, in this diff I

- added some more test cases,
- fixed crashes,
- removed unnecessary headers,
- fixed some typos in the documentation.


https://reviews.llvm.org/D45532

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp
  test/Analysis/ctor-uninitialized-member-inheritance.cpp
  test/Analysis/ctor-uninitialized-member.cpp

Index: test/Analysis/ctor-uninitialized-member.cpp
===
--- /dev/null
+++ test/Analysis/ctor-uninitialized-member.cpp
@@ -0,0 +1,1122 @@
+//RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.CtorUninitializedMember -std=c++11 -verify %s
+
+//===--===//
+// Default constructor test.
+//===--===//
+
+class CompilerGeneratedConstructorTest {
+  int a, b, c, d, e, f, g, h, i, j;
+
+public:
+  CompilerGeneratedConstructorTest() = default;
+};
+
+void f000() {
+  CompilerGeneratedConstructorTest();
+}
+
+class DefaultConstructorTest {
+  int a; // expected-note{{uninitialized field 'this->a'}}
+
+public:
+  DefaultConstructorTest();
+};
+
+DefaultConstructorTest::DefaultConstructorTest() = default;
+
+void f00() {
+  DefaultConstructorTest(); // expected-warning{{1 uninitialized field}}
+}
+
+//===--===//
+// Initializer list test.
+//===--===//
+
+class InitListTest1 {
+  int a;
+  int b;
+
+public:
+  InitListTest1()
+  : a(1),
+b(2) {
+// All good!
+  }
+};
+
+void f01() {
+  InitListTest1();
+}
+
+class InitListTest2 {
+  int a;
+  int b; // expected-note{{uninitialized field 'this->b'}}
+
+public:
+  InitListTest2()
+  : a(3) {} // expected-warning{{1 uninitialized field}}
+};
+
+void f02() {
+  InitListTest2();
+}
+
+class InitListTest3 {
+  int a; // expected-note{{uninitialized field 'this->a'}}
+  int b;
+
+public:
+  InitListTest3()
+  : b(4) {} // expected-warning{{1 uninitialized field}}
+};
+
+void f03() {
+  InitListTest3();
+}
+
+//===--===//
+// Constructor body test.
+//===--===//
+
+class CtorBodyTest1 {
+  int a, b;
+
+public:
+  CtorBodyTest1() {
+a = 5;
+b = 6;
+// All good!
+  }
+};
+
+void f04() {
+  CtorBodyTest1();
+}
+
+class CtorBodyTest2 {
+  int a;
+  int b; // expected-note{{uninitialized field 'this->b'}}
+
+public:
+  CtorBodyTest2() {
+a = 7; // expected-warning{{1 uninitialized field}}
+  }
+};
+
+void f05() {
+  CtorBodyTest2();
+}
+
+class CtorBodyTest3 {
+  int a; // expected-note{{uninitialized field 'this->a'}}
+  int b;
+
+public:
+  CtorBodyTest3() {
+b = 8; // expected-warning{{1 uninitialized field}}
+  }
+};
+
+void f06() {
+  CtorBodyTest3();
+}
+
+class CtorBodyTest4 {
+  int a; // expected-note{{uninitialized field 'this->a'}}
+  int b; // expected-note{{uninitialized field 'this->b'}}
+
+public:
+  CtorBodyTest4() {}
+};
+
+void f07() {
+  CtorBodyTest4(); // expected-warning{{2 uninitialized fields}}
+}
+
+//===--===//
+// Constructor delegation test.
+//===--===//
+
+class CtorDelegationTest1 {
+  int a;
+  int b;
+
+public:
+  CtorDelegationTest1(int)
+  : a(9) {
+// leaves 'b' unintialized, but we'll never check this function
+  }
+
+  CtorDelegationTest1()
+  : CtorDelegationTest1(int{}) { // Initializing 'a'
+b = 10;
+// All good!
+  }
+};
+
+void f08() {
+  CtorDelegationTest1();
+}
+
+class CtorDelegationTest2 {
+  int a; // expected-note{{uninitialized field 'this->a'}}
+  int b;
+
+public:
+  CtorDelegationTest2(int)
+  : b(11) {
+// leaves 'a' unintialized, but we'll never check this function
+  }
+
+  CtorDelegationTest2()
+  : CtorDelegationTest2(int{}) { // expected-warning{{1 uninitialized field}}
+  }
+};
+
+void f09() {
+  CtorDelegationTest2();
+}
+
+//===--===//
+// Tests for classes containing records.
+//===--

r330185 - Add checks for format specifiers used by __builtin_dump_struct and added a new specifier for null-terminated constant strings.

2018-04-17 Thread Aaron Ballman via cfe-commits
Author: aaronballman
Date: Tue Apr 17 04:57:47 2018
New Revision: 330185

URL: http://llvm.org/viewvc/llvm-project?rev=330185&view=rev
Log:
Add checks for format specifiers used by __builtin_dump_struct and added a new 
specifier for null-terminated constant strings.

Patch by Paul Semel.

Modified:
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/test/CodeGen/dump-struct-builtin.c

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=330185&r1=330184&r2=330185&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Tue Apr 17 04:57:47 2018
@@ -962,6 +962,7 @@ static llvm::Value *dumpRecord(CodeGenFu
 Types[Context.DoubleTy] = "%f";
 Types[Context.LongDoubleTy] = "%Lf";
 Types[Context.getPointerType(Context.CharTy)] = "%s";
+Types[Context.getPointerType(Context.getConstType(Context.CharTy))] = "%s";
   }
 
   for (const auto *FD : RD->fields()) {

Modified: cfe/trunk/test/CodeGen/dump-struct-builtin.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/dump-struct-builtin.c?rev=330185&r1=330184&r2=330185&view=diff
==
--- cfe/trunk/test/CodeGen/dump-struct-builtin.c (original)
+++ cfe/trunk/test/CodeGen/dump-struct-builtin.c Tue Apr 17 04:57:47 2018
@@ -2,6 +2,98 @@
 
 #include "Inputs/stdio.h"
 
+// CHECK: @unit1.a = private unnamed_addr constant %struct.U1A { i16 12 }, 
align 2
+// CHECK-NEXT: [[STRUCT_STR_U1:@[0-9]+]] = private unnamed_addr constant [14 x 
i8] c"struct U1A {\0A\00"
+// CHECK-NEXT: [[FIELD_U1:@[0-9]+]] = private unnamed_addr constant [11 x i8] 
c"short a : \00"
+// CHECK-NEXT: [[FORMAT_U1:@[0-9]+]] = private unnamed_addr constant [5 x i8] 
c"%hd\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U1:@[0-9]+]] = private unnamed_addr constant [3 x 
i8] c"}\0A\00"
+
+// CHECK: @unit2.a = private unnamed_addr constant %struct.U2A { i16 12 }, 
align 2
+// CHECK-NEXT: [[STRUCT_STR_U2:@[0-9]+]] = private unnamed_addr constant [14 x 
i8] c"struct U2A {\0A\00"
+// CHECK-NEXT: [[FIELD_U2:@[0-9]+]] = private unnamed_addr constant [20 x i8] 
c"unsigned short a : \00"
+// CHECK-NEXT: [[FORMAT_U2:@[0-9]+]] = private unnamed_addr constant [5 x i8] 
c"%hu\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U2:@[0-9]+]] = private unnamed_addr constant [3 x 
i8] c"}\0A\00"
+
+// CHECK: @unit3.a = private unnamed_addr constant %struct.U3A { i32 12 }, 
align 4
+// CHECK-NEXT: [[STRUCT_STR_U3:@[0-9]+]] = private unnamed_addr constant [14 x 
i8] c"struct U3A {\0A\00"
+// CHECK-NEXT: [[FIELD_U3:@[0-9]+]] = private unnamed_addr constant [9 x i8] 
c"int a : \00"
+// CHECK-NEXT: [[FORMAT_U3:@[0-9]+]] = private unnamed_addr constant [4 x i8] 
c"%d\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U3:@[0-9]+]] = private unnamed_addr constant [3 x 
i8] c"}\0A\00"
+
+// CHECK: @unit4.a = private unnamed_addr constant %struct.U4A { i32 12 }, 
align 4
+// CHECK-NEXT: [[STRUCT_STR_U4:@[0-9]+]] = private unnamed_addr constant [14 x 
i8] c"struct U4A {\0A\00"
+// CHECK-NEXT: [[FIELD_U4:@[0-9]+]] = private unnamed_addr constant [18 x i8] 
c"unsigned int a : \00"
+// CHECK-NEXT: [[FORMAT_U4:@[0-9]+]] = private unnamed_addr constant [4 x i8] 
c"%u\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U4:@[0-9]+]] = private unnamed_addr constant [3 x 
i8] c"}\0A\00"
+
+// CHECK: @unit5.a = private unnamed_addr constant %struct.U5A { i64 12 }, 
align 8
+// CHECK-NEXT: [[STRUCT_STR_U5:@[0-9]+]] = private unnamed_addr constant [14 x 
i8] c"struct U5A {\0A\00"
+// CHECK-NEXT: [[FIELD_U5:@[0-9]+]] = private unnamed_addr constant [10 x i8] 
c"long a : \00"
+// CHECK-NEXT: [[FORMAT_U5:@[0-9]+]] = private unnamed_addr constant [5 x i8] 
c"%ld\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U5:@[0-9]+]] = private unnamed_addr constant [3 x 
i8] c"}\0A\00"
+
+// CHECK: @unit6.a = private unnamed_addr constant %struct.U6A { i64 12 }, 
align 8
+// CHECK-NEXT: [[STRUCT_STR_U6:@[0-9]+]] = private unnamed_addr constant [14 x 
i8] c"struct U6A {\0A\00"
+// CHECK-NEXT: [[FIELD_U6:@[0-9]+]] = private unnamed_addr constant [19 x i8] 
c"unsigned long a : \00"
+// CHECK-NEXT: [[FORMAT_U6:@[0-9]+]] = private unnamed_addr constant [5 x i8] 
c"%lu\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U6:@[0-9]+]] = private unnamed_addr constant [3 x 
i8] c"}\0A\00"
+
+// CHECK: @unit7.a = private unnamed_addr constant %struct.U7A { i64 12 }, 
align 8
+// CHECK-NEXT: [[STRUCT_STR_U7:@[0-9]+]] = private unnamed_addr constant [14 x 
i8] c"struct U7A {\0A\00"
+// CHECK-NEXT: [[FIELD_U7:@[0-9]+]] = private unnamed_addr constant [15 x i8] 
c"long long a : \00"
+// CHECK-NEXT: [[FORMAT_U7:@[0-9]+]] = private unnamed_addr constant [6 x i8] 
c"%lld\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U7:@[0-9]+]] = private unnamed_addr constant [3 x 
i8] c"}\0A\00"
+
+// CHECK: @unit8.a = private unnamed_addr constant %struct.U8A { i64 12 }, 
align 8
+// CHECK-NEXT

[PATCH] D45665: [builtin-dump-struct] printf formats generation testing

2018-04-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision.
aaron.ballman added a comment.

I've committed in r330185, thank you for the patch!


Repository:
  rC Clang

https://reviews.llvm.org/D45665



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


[PATCH] D45601: Warn on bool* to bool conversion

2018-04-17 Thread Nico Weber via Phabricator via cfe-commits
thakis added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:7806
+  "comparing %0 as a boolean">,
+  InGroup;
 def warn_format_argument_needs_cast : Warning<

lebedev.ri wrote:
> Also, this really really should be under it's own flag, which in turn may be 
> under `Extra`.
The guideline is actually "most warnings should be high-value and have next to 
no false positives", and because of that most warnings should be default-on. We 
don't have a clear guideline when to put warnings under `-Wall` instead of 
making them default-on, but sometimes "does the warning need to build a CFG?" 
is used for that. Almost nothing should be in Extra but not in All.


https://reviews.llvm.org/D45601



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


[PATCH] D45601: Warn on bool* to bool conversion

2018-04-17 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:7806
+  "comparing %0 as a boolean">,
+  InGroup;
 def warn_format_argument_needs_cast : Warning<

thakis wrote:
> lebedev.ri wrote:
> > Also, this really really should be under it's own flag, which in turn may 
> > be under `Extra`.
> The guideline is actually "most warnings should be high-value and have next 
> to no false positives", and because of that most warnings should be 
> default-on. We don't have a clear guideline when to put warnings under 
> `-Wall` instead of making them default-on, but sometimes "does the warning 
> need to build a CFG?" is used for that. Almost nothing should be in Extra but 
> not in All.
The point i was trying to make:
if the warning is under a global umbrella flag only (All, Extra),
it can't be selectively disabled. And that is not good regardless.


https://reviews.llvm.org/D45601



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


r330187 - [NEON] Fix the architecture condition for the crypto intrinsics

2018-04-17 Thread Ivan A. Kosarev via cfe-commits
Author: kosarev
Date: Tue Apr 17 06:37:30 2018
New Revision: 330187

URL: http://llvm.org/viewvc/llvm-project?rev=330187&view=rev
Log:
[NEON] Fix the architecture condition for the crypto intrinsics

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

Modified:
cfe/trunk/include/clang/Basic/arm_neon.td

Modified: cfe/trunk/include/clang/Basic/arm_neon.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/arm_neon.td?rev=330187&r1=330186&r2=330187&view=diff
==
--- cfe/trunk/include/clang/Basic/arm_neon.td (original)
+++ cfe/trunk/include/clang/Basic/arm_neon.td Tue Apr 17 06:37:30 2018
@@ -913,7 +913,7 @@ def VEXT_A64 : WInst<"vext", "dddi", "dQ
 
 

 // Crypto
-let ArchGuard = "__ARM_FEATURE_CRYPTO" in {
+let ArchGuard = "__ARM_ARCH >= 8 && defined(__ARM_FEATURE_CRYPTO)" in {
 def AESE : SInst<"vaese", "ddd", "QUc">;
 def AESD : SInst<"vaesd", "ddd", "QUc">;
 def AESMC : SInst<"vaesmc", "dd", "QUc">;


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


[PATCH] D45669: [NEON] Fix the architecture condition for the crypto intrinsics

2018-04-17 Thread Ivan Kosarev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC330187: [NEON] Fix the architecture condition for the crypto 
intrinsics (authored by kosarev, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D45669

Files:
  include/clang/Basic/arm_neon.td


Index: include/clang/Basic/arm_neon.td
===
--- include/clang/Basic/arm_neon.td
+++ include/clang/Basic/arm_neon.td
@@ -913,7 +913,7 @@
 
 

 // Crypto
-let ArchGuard = "__ARM_FEATURE_CRYPTO" in {
+let ArchGuard = "__ARM_ARCH >= 8 && defined(__ARM_FEATURE_CRYPTO)" in {
 def AESE : SInst<"vaese", "ddd", "QUc">;
 def AESD : SInst<"vaesd", "ddd", "QUc">;
 def AESMC : SInst<"vaesmc", "dd", "QUc">;


Index: include/clang/Basic/arm_neon.td
===
--- include/clang/Basic/arm_neon.td
+++ include/clang/Basic/arm_neon.td
@@ -913,7 +913,7 @@
 
 
 // Crypto
-let ArchGuard = "__ARM_FEATURE_CRYPTO" in {
+let ArchGuard = "__ARM_ARCH >= 8 && defined(__ARM_FEATURE_CRYPTO)" in {
 def AESE : SInst<"vaese", "ddd", "QUc">;
 def AESD : SInst<"vaesd", "ddd", "QUc">;
 def AESMC : SInst<"vaesmc", "dd", "QUc">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45669: [NEON] Fix the architecture condition for the crypto intrinsics

2018-04-17 Thread Ivan Kosarev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL330187: [NEON] Fix the architecture condition for the crypto 
intrinsics (authored by kosarev, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D45669?vs=142559&id=142766#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45669

Files:
  cfe/trunk/include/clang/Basic/arm_neon.td


Index: cfe/trunk/include/clang/Basic/arm_neon.td
===
--- cfe/trunk/include/clang/Basic/arm_neon.td
+++ cfe/trunk/include/clang/Basic/arm_neon.td
@@ -913,7 +913,7 @@
 
 

 // Crypto
-let ArchGuard = "__ARM_FEATURE_CRYPTO" in {
+let ArchGuard = "__ARM_ARCH >= 8 && defined(__ARM_FEATURE_CRYPTO)" in {
 def AESE : SInst<"vaese", "ddd", "QUc">;
 def AESD : SInst<"vaesd", "ddd", "QUc">;
 def AESMC : SInst<"vaesmc", "dd", "QUc">;


Index: cfe/trunk/include/clang/Basic/arm_neon.td
===
--- cfe/trunk/include/clang/Basic/arm_neon.td
+++ cfe/trunk/include/clang/Basic/arm_neon.td
@@ -913,7 +913,7 @@
 
 
 // Crypto
-let ArchGuard = "__ARM_FEATURE_CRYPTO" in {
+let ArchGuard = "__ARM_ARCH >= 8 && defined(__ARM_FEATURE_CRYPTO)" in {
 def AESE : SInst<"vaese", "ddd", "QUc">;
 def AESD : SInst<"vaesd", "ddd", "QUc">;
 def AESMC : SInst<"vaesmc", "dd", "QUc">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45697: [clang-tidy] Fix clang-tidy doesn't read .clangtidy configuration file.

2018-04-17 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision.
alexfh added inline comments.
This revision is now accepted and ready to land.



Comment at: test/clang-tidy/read_file_config.cpp:5
+// RUN: echo '[{"command": "cc -c -o test.o test.cpp", "directory": 
"%T/read-file-config", "file": "%T/read-file-config/test.cpp"}]' > 
%T/read-file-config/compile_commands.json
+// RUN: clang-tidy %T/read-file-config/test.cpp | not grep "clang-analyzer-*"
+

hokein wrote:
> alexfh wrote:
> > Please add a sanity check to ensure the code triggers the static analyzer 
> > check, when enabled explicitly: 
> > 
> > // RUN: clang-tidy -checks=-*,clang-analyzer-* %T/read-file-config/test.cpp 
> > | grep "clang-analyzer-*"
> > 
> > 
> > (hint: it won't pass, since your regex in grep is incorrect ;)
> Done. This is valid regex, but doesn't do the expect thing, we expect to 
> match the whole check name, which should be `clang-analyzer-.*`
I didn't say the regex is //invalid//, I said it's //incorrect// (in the sense 
that it matches a pattern different from the one useful in this test). Anyway, 
thanks for the fix!

One more idea is to expand the pattern a bit to make it obvious it matches a 
specific static analyzer warning (`"warning: .* 
[clang-analyzer-specific.checker.Name]$"`).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45697



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


[PATCH] D45668: [NEON] Define vget_high_f16() and vget_low_f16() intrinsics in AArch64 mode only

2018-04-17 Thread Ivan Kosarev via Phabricator via cfe-commits
kosarev added a comment.

The NEON Intrinsics Reference 
(http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ihi0073a/index.html)
 reads like they are AArch64-only.


https://reviews.llvm.org/D45668



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


[PATCH] D45679: [clang-tidy] Add a helper function isModified, that checks whether an expression is modified within a statement.

2018-04-17 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment.

In https://reviews.llvm.org/D45679#1069638, @JonasToth wrote:

> *hust* /llvm/tools/clang/lib/Analysis/PseudoConstantAnalysis.cpp
>
> I will check this one first, before we get crazy and implemented something 
> twice.


But this is IR-level analysis. Clang has own include/clang/Analysis.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45679



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


[PATCH] D45615: [builtins] __builtin_dump_struct : added more types format

2018-04-17 Thread 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! I will go ahead and commit on your behalf. Btw, if you're thinking of 
making additional patches, now might be a good time to ask for commit 
privileges (https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access).


Repository:
  rC Clang

https://reviews.llvm.org/D45615



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


r330188 - Add modifiers for unsigned char and signed char field printing for __builtin_dump_struct.

2018-04-17 Thread Aaron Ballman via cfe-commits
Author: aaronballman
Date: Tue Apr 17 07:00:06 2018
New Revision: 330188

URL: http://llvm.org/viewvc/llvm-project?rev=330188&view=rev
Log:
Add modifiers for unsigned char and signed char field printing for 
__builtin_dump_struct.

Patch by Paul Semel.

Modified:
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/test/CodeGen/dump-struct-builtin.c

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=330188&r1=330187&r2=330188&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Tue Apr 17 07:00:06 2018
@@ -949,6 +949,8 @@ static llvm::Value *dumpRecord(CodeGenFu
   if (Types.empty()) {
 Types[Context.CharTy] = "%c";
 Types[Context.BoolTy] = "%d";
+Types[Context.SignedCharTy] = "%hhd";
+Types[Context.UnsignedCharTy] = "%hhu";
 Types[Context.IntTy] = "%d";
 Types[Context.UnsignedIntTy] = "%u";
 Types[Context.LongTy] = "%ld";

Modified: cfe/trunk/test/CodeGen/dump-struct-builtin.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/dump-struct-builtin.c?rev=330188&r1=330187&r2=330188&view=diff
==
--- cfe/trunk/test/CodeGen/dump-struct-builtin.c (original)
+++ cfe/trunk/test/CodeGen/dump-struct-builtin.c Tue Apr 17 07:00:06 2018
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm %s -o - | 
FileCheck %s
 
 #include "Inputs/stdio.h"
+#include 
 
 // CHECK: @unit1.a = private unnamed_addr constant %struct.U1A { i16 12 }, 
align 2
 // CHECK-NEXT: [[STRUCT_STR_U1:@[0-9]+]] = private unnamed_addr constant [14 x 
i8] c"struct U1A {\0A\00"
@@ -94,6 +95,18 @@
 // CHECK-NEXT: [[FORMAT_U15:@[0-9]+]] = private unnamed_addr constant [4 x i8] 
c"%p\0A\00"
 // CHECK-NEXT: [[END_STRUCT_U15:@[0-9]+]] = private unnamed_addr constant [3 x 
i8] c"}\0A\00"
 
+// CHECK: @unit16.a = private unnamed_addr constant %struct.U16A { i8 12 }, 
align 1
+// CHECK-NEXT: [[STRUCT_STR_U16:@[0-9]+]] = private unnamed_addr constant [15 
x i8] c"struct U16A {\0A\00"
+// CHECK-NEXT: [[FIELD_U16:@[0-9]+]] = private unnamed_addr constant [13 x i8] 
c"uint8_t a : \00"
+// CHECK-NEXT: [[FORMAT_U16:@[0-9]+]] = private unnamed_addr constant [6 x i8] 
c"%hhu\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U16:@[0-9]+]] = private unnamed_addr constant [3 x 
i8] c"}\0A\00"
+
+// CHECK: @unit17.a = private unnamed_addr constant %struct.U17A { i8 12 }, 
align 1
+// CHECK-NEXT: [[STRUCT_STR_U17:@[0-9]+]] = private unnamed_addr constant [15 
x i8] c"struct U17A {\0A\00"
+// CHECK-NEXT: [[FIELD_U17:@[0-9]+]] = private unnamed_addr constant [12 x i8] 
c"int8_t a : \00"
+// CHECK-NEXT: [[FORMAT_U17:@[0-9]+]] = private unnamed_addr constant [6 x i8] 
c"%hhd\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U17:@[0-9]+]] = private unnamed_addr constant [3 x 
i8] c"}\0A\00"
+
 int printf(const char *fmt, ...) {
 return 0;
 }
@@ -368,6 +381,42 @@ void unit15() {
   __builtin_dump_struct(&a, &printf);
 }
 
+void unit16() {
+  struct U16A {
+uint8_t a;
+  };
+
+  struct U16A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([15 x i8], 
[15 x i8]* [[STRUCT_STR_U16]], i32 0, i32 0))
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U16A, 
%struct.U16A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([13 x i8], 
[13 x i8]* [[FIELD_U16]], i32 0, i32 0))
+  // CHECK: [[LOAD1:%[0-9]+]] = load i8, i8* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([6 x i8], 
[6 x i8]* [[FORMAT_U16]], i32 0, i32 0), i8 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([3 x i8], 
[3 x i8]* [[END_STRUCT_U16]], i32 0, i32 0)
+  __builtin_dump_struct(&a, &printf);
+}
+
+void unit17() {
+  struct U17A {
+int8_t a;
+  };
+
+  struct U17A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([15 x i8], 
[15 x i8]* [[STRUCT_STR_U17]], i32 0, i32 0))
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U17A, 
%struct.U17A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([12 x i8], 
[12 x i8]* [[FIELD_U17]], i32 0, i32 0))
+  // CHECK: [[LOAD1:%[0-9]+]] = load i8, i8* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([6 x i8], 
[6 x i8]* [[FORMAT_U17]], i32 0, i32 0), i8 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([3 x i8], 
[3 x i8]* [[END_STRUCT_U17]], i32 0, i32 0)
+  __builtin_dump_struct(&a, &printf);
+}
+
 void test1() {
   struct T1A {
 int a;


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


[PATCH] D45615: [builtins] __builtin_dump_struct : added more types format

2018-04-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision.
aaron.ballman added a comment.

Committed in r330188, thank you for the patch!


Repository:
  rC Clang

https://reviews.llvm.org/D45615



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


[PATCH] D45720: [X86] Lowering PACK*S (pack with saturation) intrinsics to native IR (clang side)

2018-04-17 Thread Mikhail Dvoretckii via Phabricator via cfe-commits
mike.dvoretsky created this revision.
mike.dvoretsky added reviewers: craig.topper, spatel.
Herald added a subscriber: cfe-commits.

This patch lowers the X86 vector packing with saturation intrinsics to native 
LLVM IR. Comes with an LLVM patch.


Repository:
  rC Clang

https://reviews.llvm.org/D45720

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/avx2-builtins.c
  test/CodeGen/avx512bw-builtins.c
  test/CodeGen/avx512vlbw-builtins.c
  test/CodeGen/sse2-builtins.c
  test/CodeGen/sse41-builtins.c

Index: test/CodeGen/sse41-builtins.c
===
--- test/CodeGen/sse41-builtins.c
+++ test/CodeGen/sse41-builtins.c
@@ -328,7 +328,12 @@
 
 __m128i test_mm_packus_epi32(__m128i x, __m128i y) {
   // CHECK-LABEL: test_mm_packus_epi32
-  // CHECK: call <8 x i16> @llvm.x86.sse41.packusdw(<4 x i32> %{{.*}}, <4 x i32> %{{.*}})
+  // CHECK: %{{.*}} = shufflevector <4 x i32> %{{.*}}, <4 x i32> %{{.*}}, <8 x i32> 
+  // CHECK: %{{.*}} = icmp slt <8 x i32> %{{.*}}, 
+  // CHECK: %{{.*}} = select <8 x i1> %{{.*}}, <8 x i32> %{{.*}}, <8 x i32> 
+  // CHECK: %{{.*}} = icmp sgt <8 x i32> %{{.*}}, zeroinitializer
+  // CHECK: %{{.*}} = select <8 x i1> %{{.*}}, <8 x i32> %{{.*}}, <8 x i32> zeroinitializer
+  // CHECK: %{{.*}} = trunc <8 x i32> %{{.*}} to <8 x i16>
   return _mm_packus_epi32(x, y);
 }
 
Index: test/CodeGen/sse2-builtins.c
===
--- test/CodeGen/sse2-builtins.c
+++ test/CodeGen/sse2-builtins.c
@@ -869,19 +869,34 @@
 
 __m128i test_mm_packs_epi16(__m128i A, __m128i B) {
   // CHECK-LABEL: test_mm_packs_epi16
-  // CHECK: call <16 x i8> @llvm.x86.sse2.packsswb.128(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+  // CHECK: %{{.*}} = shufflevector <8 x i16> %{{.*}}, <8 x i16> %{{.*}}, <16 x i32> 
+  // CHECK: %{{.*}} = icmp slt <16 x i16> %{{.*}}, 
+  // CHECK: %{{.*}} = select <16 x i1> %{{.*}}, <16 x i16> %{{.*}}, <16 x i16> 
+  // CHECK: %{{.*}} = icmp sgt <16 x i16> %{{.*}}, 
+  // CHECK: %{{.*}} = select <16 x i1> %{{.*}}, <16 x i16> %{{.*}}, <16 x i16> 
+  // CHECK: %{{.*}} = trunc <16 x i16> %{{.*}} to <16 x i8>
   return _mm_packs_epi16(A, B);
 }
 
 __m128i test_mm_packs_epi32(__m128i A, __m128i B) {
   // CHECK-LABEL: test_mm_packs_epi32
-  // CHECK: call <8 x i16> @llvm.x86.sse2.packssdw.128(<4 x i32> %{{.*}}, <4 x i32> %{{.*}})
+  // CHECK: %{{.*}} = shufflevector <4 x i32> %{{.*}}, <4 x i32> %{{.*}}, <8 x i32> 
+  // CHECK: %{{.*}} = icmp slt <8 x i32> %{{.*}}, 
+  // CHECK: %{{.*}} = select <8 x i1> %{{.*}}, <8 x i32> %{{.*}}, <8 x i32> 
+  // CHECK: %{{.*}} = icmp sgt <8 x i32> %{{.*}}, 
+  // CHECK: %{{.*}} = select <8 x i1> %{{.*}}, <8 x i32> %{{.*}}, <8 x i32> 
+  // CHECK: %{{.*}} = trunc <8 x i32> %{{.*}} to <8 x i16>
   return _mm_packs_epi32(A, B);
 }
 
 __m128i test_mm_packus_epi16(__m128i A, __m128i B) {
   // CHECK-LABEL: test_mm_packus_epi16
-  // CHECK: call <16 x i8> @llvm.x86.sse2.packuswb.128(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+  // CHECK: %{{.*}} = shufflevector <8 x i16> %{{.*}}, <8 x i16> %{{.*}}, <16 x i32> 
+  // CHECK: %{{.*}} = icmp slt <16 x i16> %{{.*}}, 
+  // CHECK: %{{.*}} = select <16 x i1> %{{.*}}, <16 x i16> %{{.*}}, <16 x i16> 
+  // CHECK: %{{.*}} = icmp sgt <16 x i16> %{{.*}}, zeroinitializer
+  // CHECK: %{{.*}} = select <16 x i1> %{{.*}}, <16 x i16> %{{.*}}, <16 x i16> zeroinitializer
+  // CHECK: %{{.*}} = trunc <16 x i16> %{{.*}} to <16 x i8>
   return _mm_packus_epi16(A, B);
 }
 
Index: test/CodeGen/avx512vlbw-builtins.c
===
--- test/CodeGen/avx512vlbw-builtins.c
+++ test/CodeGen/avx512vlbw-builtins.c
@@ -970,105 +970,185 @@
 
 __m128i test_mm_maskz_packs_epi32(__mmask8 __M, __m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_maskz_packs_epi32
-  // CHECK: @llvm.x86.sse2.packssdw
+  // CHECK: %{{.*}} = shufflevector <4 x i32> %{{.*}}, <4 x i32> %{{.*}}, <8 x i32> 
+  // CHECK: %{{.*}} = icmp slt <8 x i32> %{{.*}}, 
+  // CHECK: %{{.*}} = select <8 x i1> %{{.*}}, <8 x i32> %{{.*}}, <8 x i32> 
+  // CHECK: %{{.*}} = icmp sgt <8 x i32> %{{.*}}, 
+  // CHECK: %{{.*}} = select <8 x i1> %{{.*}}, <8 x i32> %{{.*}}, <8 x i32> 
+  // CHECK: %{{.*}} = trunc <8 x i32> %{{.*}} to <8 x i16>
   // CHECK: select <8 x i1> %{{.*}}, <8 x i16> %{{.*}}, <8 x i16> %{{.*}}
   return _mm_maskz_packs_epi32(__M,__A,__B); 
 }
 __m128i test_mm_mask_packs_epi32(__m128i __W, __mmask16 __M, __m128i __A,  __m128i __B) {
   // CHECK-LABEL: @test_mm_mask_packs_epi32
-  // CHECK: @llvm.x86.sse2.packssdw
+  // CHECK: %{{.*}} = shufflevector <4 x i32> %{{.*}}, <4 x i32> %{{.*}}, <8 x i32> 
+  // CHECK: %{{.*}} = icmp slt <8 x i32> %{{.*}}, 
+  // CHECK: %{{.*}} = select <8 x i1> %{{.*}}, <8 x i32> %{{.*}}, <8 x i32> 
+  // CHECK: %{{.*}} = icmp sgt <8 x i32> %{{.*}}, 
+  // CHECK: %{{.*}} = select <8 x i1> %{{.*}}, <8 x i32> %{{.*}}, <8 x i32> 
+  // CHECK: %{{.*}} = trunc <8 x i32> %{{.*}} to <8 x i16>
   // 

[PATCH] D45722: [X86] Lowering SAD (sum of absolute differences) intrinsics to native IR (clang side)

2018-04-17 Thread Mikhail Dvoretckii via Phabricator via cfe-commits
mike.dvoretsky created this revision.
mike.dvoretsky added reviewers: craig.topper, spatel.
Herald added a subscriber: cfe-commits.

This patch lowers the SAD intrinsics to native LLVM IR. Comes with an LLVM 
patch.


Repository:
  rC Clang

https://reviews.llvm.org/D45722

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/avx2-builtins.c
  test/CodeGen/avx512bw-builtins.c
  test/CodeGen/sse2-builtins.c

Index: test/CodeGen/sse2-builtins.c
===
--- test/CodeGen/sse2-builtins.c
+++ test/CodeGen/sse2-builtins.c
@@ -893,7 +893,33 @@
 
 __m128i test_mm_sad_epu8(__m128i A, __m128i B) {
   // CHECK-LABEL: test_mm_sad_epu8
-  // CHECK: call <2 x i64> @llvm.x86.sse2.psad.bw(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+  // CHECK: %{{.*}} = icmp ugt <16 x i8> %{{.*}}, %{{.*}}
+  // CHECK: %{{.*}} = sub <16 x i8> %{{.*}}, %{{.*}}
+  // CHECK: %{{.*}} = sub <16 x i8> %{{.*}}, %{{.*}}
+  // CHECK: %{{.*}} = select <16 x i1> %{{.*}}, <16 x i8> %{{.*}}, <16 x i8> %{{.*}}
+  // CHECK: %{{.*}} = shufflevector <16 x i8> %{{.*}}, <16 x i8> undef, <2 x i32> 
+  // CHECK: %{{.*}} = zext <2 x i8> %{{.*}} to <2 x i64>
+  // CHECK: %{{.*}} = shufflevector <16 x i8> %{{.*}}, <16 x i8> undef, <2 x i32> 
+  // CHECK: %{{.*}} = zext <2 x i8> %{{.*}} to <2 x i64>
+  // CHECK: %{{.*}} = add <2 x i64> %{{.*}}, %{{.*}}
+  // CHECK: %{{.*}} = shufflevector <16 x i8> %{{.*}}, <16 x i8> undef, <2 x i32> 
+  // CHECK: %{{.*}} = zext <2 x i8> %{{.*}} to <2 x i64>
+  // CHECK: %{{.*}} = add <2 x i64> %{{.*}}, %{{.*}}
+  // CHECK: %{{.*}} = shufflevector <16 x i8> %{{.*}}, <16 x i8> undef, <2 x i32> 
+  // CHECK: %{{.*}} = zext <2 x i8> %{{.*}} to <2 x i64>
+  // CHECK: %{{.*}} = add <2 x i64> %{{.*}}, %{{.*}}
+  // CHECK: %{{.*}} = shufflevector <16 x i8> %{{.*}}, <16 x i8> undef, <2 x i32> 
+  // CHECK: %{{.*}} = zext <2 x i8> %{{.*}} to <2 x i64>
+  // CHECK: %{{.*}} = add <2 x i64> %{{.*}}, %{{.*}}
+  // CHECK: %{{.*}} = shufflevector <16 x i8> %{{.*}}, <16 x i8> undef, <2 x i32> 
+  // CHECK: %{{.*}} = zext <2 x i8> %{{.*}} to <2 x i64>
+  // CHECK: %{{.*}} = add <2 x i64> %{{.*}}, %{{.*}}
+  // CHECK: %{{.*}} = shufflevector <16 x i8> %{{.*}}, <16 x i8> undef, <2 x i32> 
+  // CHECK: %{{.*}} = zext <2 x i8> %{{.*}} to <2 x i64>
+  // CHECK: %{{.*}} = add <2 x i64> %{{.*}}, %{{.*}}
+  // CHECK: %{{.*}} = shufflevector <16 x i8> %{{.*}}, <16 x i8> undef, <2 x i32> 
+  // CHECK: %{{.*}} = zext <2 x i8> %{{.*}} to <2 x i64>
+  // CHECK: %{{.*}} = add <2 x i64> %{{.*}}, %{{.*}}
   return _mm_sad_epu8(A, B);
 }
 
Index: test/CodeGen/avx512bw-builtins.c
===
--- test/CodeGen/avx512bw-builtins.c
+++ test/CodeGen/avx512bw-builtins.c
@@ -1945,7 +1945,33 @@
 
 __m512i test_mm512_sad_epu8(__m512i __A, __m512i __B) {
   // CHECK-LABEL: @test_mm512_sad_epu8
-  // CHECK: @llvm.x86.avx512.psad.bw.512
+  // CHECK: %{{.*}} = icmp ugt <64 x i8> %{{.*}}, %{{.*}}
+  // CHECK: %{{.*}} = sub <64 x i8> %{{.*}}, %{{.*}}
+  // CHECK: %{{.*}} = sub <64 x i8> %{{.*}}, %{{.*}}
+  // CHECK: %{{.*}} = select <64 x i1> %{{.*}}, <64 x i8> %{{.*}}, <64 x i8> %{{.*}}
+  // CHECK: %{{.*}} = shufflevector <64 x i8> %{{.*}}, <64 x i8> undef, <8 x i32> 
+  // CHECK: %{{.*}} = zext <8 x i8> %{{.*}} to <8 x i64>
+  // CHECK: %{{.*}} = shufflevector <64 x i8> %{{.*}}, <64 x i8> undef, <8 x i32> 
+  // CHECK: %{{.*}} = zext <8 x i8> %{{.*}} to <8 x i64>
+  // CHECK: %{{.*}} = add <8 x i64> %{{.*}}, %{{.*}}
+  // CHECK: %{{.*}} = shufflevector <64 x i8> %{{.*}}, <64 x i8> undef, <8 x i32> 
+  // CHECK: %{{.*}} = zext <8 x i8> %{{.*}} to <8 x i64>
+  // CHECK: %{{.*}} = add <8 x i64> %{{.*}}, %{{.*}}
+  // CHECK: %{{.*}} = shufflevector <64 x i8> %{{.*}}, <64 x i8> undef, <8 x i32> 
+  // CHECK: %{{.*}} = zext <8 x i8> %{{.*}} to <8 x i64>
+  // CHECK: %{{.*}} = add <8 x i64> %{{.*}}, %{{.*}}
+  // CHECK: %{{.*}} = shufflevector <64 x i8> %{{.*}}, <64 x i8> undef, <8 x i32> 
+  // CHECK: %{{.*}} = zext <8 x i8> %{{.*}} to <8 x i64>
+  // CHECK: %{{.*}} = add <8 x i64> %{{.*}}, %{{.*}}
+  // CHECK: %{{.*}} = shufflevector <64 x i8> %{{.*}}, <64 x i8> undef, <8 x i32> 
+  // CHECK: %{{.*}} = zext <8 x i8> %{{.*}} to <8 x i64>
+  // CHECK: %{{.*}} = add <8 x i64> %{{.*}}, %{{.*}}
+  // CHECK: %{{.*}} = shufflevector <64 x i8> %{{.*}}, <64 x i8> undef, <8 x i32> 
+  // CHECK: %{{.*}} = zext <8 x i8> %{{.*}} to <8 x i64>
+  // CHECK: %{{.*}} = add <8 x i64> %{{.*}}, %{{.*}}
+  // CHECK: %{{.*}} = shufflevector <64 x i8> %{{.*}}, <64 x i8> undef, <8 x i32> 
+  // CHECK: %{{.*}} = zext <8 x i8> %{{.*}} to <8 x i64>
+  // CHECK: %{{.*}} = add <8 x i64> %{{.*}}, %{{.*}}
   return _mm512_sad_epu8(__A, __B); 
 }
 
Index: test/CodeGen/avx2-builtins.c
===
--- test/CodeGen/avx2-builtins.c
+++ test/CodeGen/avx2-builtins.c
@@ -943,7 +943,33 @@
 
 __m256i test_mm256_sad_epu8(__m256i x, __m256i y) {
   // CHECK-LABEL: 

[PATCH] D45668: [NEON] Define vget_high_f16() and vget_low_f16() intrinsics in AArch64 mode only

2018-04-17 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

Yep, agreed, also on the new shiny 
https://developer.arm.com/technologies/neon/intrinsics it is listed as A64 only.


https://reviews.llvm.org/D45668



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


[PATCH] D41648: [clang-tidy] implement cppcoreguidelines macro rules

2018-04-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In https://reviews.llvm.org/D41648#1047799, @JonasToth wrote:

> I checked several codebases and implemented a little helper script to see 
> which kind of macros exist. Then I added some filter regex as configuration 
> and tried again, here are the results:
>
> For https://github.com/nlohmann/json which is a very high quality codebase, 
> the results were as expected: very clean, and good macro usage: (except a 
> `#define private public` for tests)
>
> Filter: `JSON*|NLOHMANN*`
>  F5913499: all_macros.txt 
>
> F5913498: filtered_macros.txt 
>
> CMake on the other hand uses many macros and don't obey CAPS_ONLY rules for 
> all macros.
>
> Filter:  
> `_FILE_OFFSET_BITS|AE_*|ARCHIVE_*|cal_*|CMAKE_*|cm*|CM_*|CPP_*|CURL*|E_*|exp_*|F90*|jp*|JSON_*|KW*|kw*|O_*|REQ_*|UV_*|YY*|yy*|Z_*`


This is a pretty long list of macro patterns to have to filter, but doesn't 
look to problematic.

> F5913502: warned_macros.txt 
> 
> F5913501: filtered_macros.txt 
> 
> OpenCV isn't clean either, here the results:
> 
> Filter: 
> `ASSERT*|ALL*|CAL*|CC*|CALC*|calc*|CL*|cl*|CUDA*|CV*|cv*|EXPECT*|GTEST*|FUNCTOR*|HAVE*|ICV*|IPL*|IPP*|ipp*|__itt*|ITT*|JAS*|jas*|MESSAGE*|MAX*|OCL*|opengl*|OPENCV*|TYP*`

This one worries me a bit more because of patterns like `cl*` and `calc*` -- 
those seem like they're not uncommon and the pattern may silence otherwise 
valid diagnostics.

> F5913504: all_macros.txt 
> 
> F5913503: filtered_modules_macros.txt 
> 
> libtorrent https://github.com/arvidn/libtorrent has reasonable macro usage
> 
> Filter: `DEBUG*|LIBTORRENT*|TORRENT*|UNI*`
> 
> F5913519: filtered_macros.txt 
> 
> F5913518: all_macros.txt 
> 
> LLVM lib/ defines many macros, almost all of them are ALL_CAPS
> 
> Filter: 
> `AARCH64*|X86*|ARM*|AMDGPU*|ASAN*|ATTR*|CHECK*|COMMON*|CV*|CXX*|DEBUG*|DEC*|DEFINE*|ESAN*|ENUM*|FMA*|FUZZER*|HANDLE*|INTERCEPTOR*|LSAN*|MATH*|MSAN*|OPENMP*|TSAN*|TYPE*|UBSAN*`
> 
> F5913505: all_lib_macros.txt 
>  F5913528: filtered_lib_macros.txt 
> 
> LLVM tools/ is a similar story
>  Filter: 
> `ANALYSIS*|ASSERT*|CHECK*|DEBUG*|DECL*|DEPENDENT*|DIAG*|END*|ENUM*|GENERIC*|IMAGE*|KEYWORD*|LANG*|LLVM*|NON*|OBJC*|OPENMP*|OVERLOAD*|PROC*|REGISTER*|SANITIZER*|STMT*|TYPE*|X86*`
> 
> F5913566: all_tools_macros.txt 
> 
> F5913565: filtered_tools_macros.txt 
> 
> @aaron.ballman Would you like to see other projects checked? I think this is 
> a starting point for now.

I think this is a good starting point. It's certainly demonstrated that even 
clean projects cannot conform to the C++ Core Guidelines.

> My opinion based on what i see is
> 
> - maybe two modes for this check make sense, having one requiring CAPS_ONLY 
> and the currently implemented stricter check. This allows easier migration to 
> safer macro usage

I think I agree.

> - it is possible to reduce macro usage to a minimal amount, and the complex 
> macros like AST stuff can be filtered with the regex. Furthermore, 
> restricting all macros to a "macro namespace" is possible for sure.

I'm not certain I understand what you mean by "macro namespace". Can you 
clarify?

I'm still a bit worried about using regex to filter the results. It seems like 
any real world project is going to require somewhere between a reasonable and 
an unreasonable amount of configuration to reduce the noise. Perhaps as a first 
pass, however, it will suffice. Hopefully we can add other heuristics to reduce 
the false positives as we see patterns emerge from real world usage.

> Things I would like to add to the check:
> 
> - my little filtering script is valuable for developers, that want to address 
> the macro issue. It should be added to the docs and everyone can make 
> something based on that. It will be linux centered.

Why does the usage need to be limited to Linux?

> - enforcing ALL_CAPS, including its usage

What does "including its usage" mean?

> - (adding a prefix to all macro, passing the filter, including its usage)
> 
>   Code transformation has the problems of scope and potentially breaking code 
> badly, because clang-tidy wasn't run over all of the code.
> 
> The check is chatty, as expected, because macros in header files, especially 
> central ones, pollute everything. That is the reason for the check, too. 
>  Overall I am still in favor of this approach, seeing some obscure macros 
> that should be replaced with actual language features (like overloading, 
> .inline functions, constants, ...) in the checked codebases.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41648




[PATCH] D45601: Warn on bool* to bool conversion

2018-04-17 Thread Aditya Kumar via Phabricator via cfe-commits
hiraditya updated this revision to Diff 142776.
hiraditya added a comment.

Warn on bool* to bool conversion during a call only.


https://reviews.llvm.org/D45601

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/SemaCXX/warn-bool-ptr-to-bool.cpp

Index: test/SemaCXX/warn-bool-ptr-to-bool.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-bool-ptr-to-bool.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int foo(bool *b) {
+  if (b) // no-warning
+return 10;
+  return 0;
+}
+
+int bar(bool b) {
+  return b;
+}
+
+int baz() {
+  bool *b;
+  bar(b);
+  // expected-warning@-1 {{passing 'bool *' as a boolean}}
+  return 0;
+}
+
+template
+T foo1(T *ptr) {
+  return ptr ? *ptr : T{}; // no-warning
+}
+
+bool bar1(bool *ptr) {
+  return foo1(ptr);
+}
+
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -2545,6 +2545,27 @@
   }
 }
 
+/// Diagnose an implicit cast;  purely a helper for CheckImplicitConversion.
+void DiagnoseImpCast(Sema &S, const Expr *E, QualType SourceType, QualType T,
+ SourceLocation CContext, unsigned diag,
+ bool pruneControlFlow = false) {
+  if (pruneControlFlow) {
+S.DiagRuntimeBehavior(E->getExprLoc(), E,
+  S.PDiag(diag)
+<< SourceType << T << E->getSourceRange()
+<< SourceRange(CContext));
+return;
+  }
+  S.Diag(E->getExprLoc(), diag)
+<< SourceType << T << E->getSourceRange() << SourceRange(CContext);
+}
+
+/// Diagnose an implicit cast;  purely a helper for CheckImplicitConversion.
+void DiagnoseImpCast(Sema &S, const Expr *E, QualType T, SourceLocation CContext,
+ unsigned diag, bool pruneControlFlow = false) {
+  DiagnoseImpCast(S, E, E->getType(), T, CContext, diag, pruneControlFlow);
+}
+
 /// Handles the checks for format strings, non-POD arguments to vararg
 /// functions, NULL arguments passed to non-NULL parameters, and diagnose_if
 /// attributes.
@@ -2589,6 +2610,28 @@
 }
   }
 
+  if (FD) {
+for (unsigned ArgIdx = 0; ArgIdx < Args.size(); ++ArgIdx) {
+  // Args[ArgIdx] can be null in malformed code.
+  if (const Expr *Arg = Args[ArgIdx]) {
+if (auto C = dyn_cast(Arg)) {
+  if (C->getCastKind() == CK_PointerToBoolean) {
+if (auto ICast = dyn_cast(C->getSubExpr())) {
+  if (ICast->getCastKind() == CK_LValueToRValue) {
+const Expr *Pointer = ICast->getSubExpr();
+QualType QT = Pointer->getType()->getPointeeType();
+if (!QT.isNull() && QT->isBooleanType())
+  // Warn on bool* to bool conversion.
+  DiagnoseImpCast(*this, Pointer, QT, Arg->getLocStart(),
+  diag::warn_pointer_to_bool);
+  }
+}
+  }
+}
+  }
+}
+  }
+
   if (FDecl || Proto) {
 CheckNonNullArguments(*this, FDecl, Proto, Args, Loc);
 
@@ -8941,27 +8984,6 @@
   AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc());
 }
 
-/// Diagnose an implicit cast;  purely a helper for CheckImplicitConversion.
-void DiagnoseImpCast(Sema &S, Expr *E, QualType SourceType, QualType T, 
- SourceLocation CContext, unsigned diag,
- bool pruneControlFlow = false) {
-  if (pruneControlFlow) {
-S.DiagRuntimeBehavior(E->getExprLoc(), E,
-  S.PDiag(diag)
-<< SourceType << T << E->getSourceRange()
-<< SourceRange(CContext));
-return;
-  }
-  S.Diag(E->getExprLoc(), diag)
-<< SourceType << T << E->getSourceRange() << SourceRange(CContext);
-}
-
-/// Diagnose an implicit cast;  purely a helper for CheckImplicitConversion.
-void DiagnoseImpCast(Sema &S, Expr *E, QualType T, SourceLocation CContext,
- unsigned diag, bool pruneControlFlow = false) {
-  DiagnoseImpCast(S, E, E->getType(), T, CContext, diag, pruneControlFlow);
-}
-
 
 /// Diagnose an implicit cast from a floating point value to an integer value.
 void DiagnoseFloatingImpCast(Sema &S, Expr *E, QualType T,
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -7760,6 +7760,9 @@
   "format specifies type %0 but the argument has "
   "%select{type|underlying type}2 %1">,
   InGroup;
+def warn_pointer_to_bool : Warning<
+  "passing %0 as a boolean">,
+  InGroup;
 def warn_format_argument_needs_cast : Warning<
   "%select{values of type|enum values with underlying type}2 '%0' should not "
   "be used as format arguments; add an explicit cast to %1 instead">,

[PATCH] D45670: [NEON} Define vfma_n_f32() and vfmaq_n_f32() intrinsics in AArch32 mode

2018-04-17 Thread Ivan Kosarev via Phabricator via cfe-commits
kosarev updated this revision to Diff 142784.
kosarev added a comment.

Removed checks for unused bitcasts. Thanks for catching!


https://reviews.llvm.org/D45670

Files:
  include/clang/Basic/arm_neon.td
  test/CodeGen/arm-neon-fma.c


Index: test/CodeGen/arm-neon-fma.c
===
--- test/CodeGen/arm-neon-fma.c
+++ test/CodeGen/arm-neon-fma.c
@@ -20,3 +20,27 @@
 float32x4_t test_fmaq_order(float32x4_t accum, float32x4_t lhs, float32x4_t 
rhs) {
   return vfmaq_f32(accum, lhs, rhs);
 }
+
+// CHECK-LABEL: define <2 x float> @test_vfma_n_f32(<2 x float> %a, <2 x 
float> %b, float %n) #0 {
+// CHECK:   [[VECINIT_I:%.*]] = insertelement <2 x float> undef, float %n, i32 0
+// CHECK:   [[VECINIT1_I:%.*]] = insertelement <2 x float> [[VECINIT_I]], 
float %n, i32 1
+// CHECK:   [[TMP1:%.*]] = bitcast <2 x float> %b to <8 x i8>
+// CHECK:   [[TMP2:%.*]] = bitcast <2 x float> [[VECINIT1_I]] to <8 x i8>
+// CHECK:   [[TMP3:%.*]] = call <2 x float> @llvm.fma.v2f32(<2 x float> %b, <2 
x float> [[VECINIT1_I]], <2 x float> %a)
+// CHECK:   ret <2 x float> [[TMP3]]
+float32x2_t test_vfma_n_f32(float32x2_t a, float32x2_t b, float32_t n) {
+  return vfma_n_f32(a, b, n);
+}
+
+// CHECK-LABEL: define <4 x float> @test_vfmaq_n_f32(<4 x float> %a, <4 x 
float> %b, float %n) #0 {
+// CHECK:   [[VECINIT_I:%.*]] = insertelement <4 x float> undef, float %n, i32 0
+// CHECK:   [[VECINIT1_I:%.*]] = insertelement <4 x float> [[VECINIT_I]], 
float %n, i32 1
+// CHECK:   [[VECINIT2_I:%.*]] = insertelement <4 x float> [[VECINIT1_I]], 
float %n, i32 2
+// CHECK:   [[VECINIT3_I:%.*]] = insertelement <4 x float> [[VECINIT2_I]], 
float %n, i32 3
+// CHECK:   [[TMP1:%.*]] = bitcast <4 x float> %b to <16 x i8>
+// CHECK:   [[TMP2:%.*]] = bitcast <4 x float> [[VECINIT3_I]] to <16 x i8>
+// CHECK:   [[TMP3:%.*]] = call <4 x float> @llvm.fma.v4f32(<4 x float> %b, <4 
x float> [[VECINIT3_I]], <4 x float> %a)
+// CHECK:   ret <4 x float> [[TMP3]]
+float32x4_t test_vfmaq_n_f32(float32x4_t a, float32x4_t b, float32_t n) {
+  return vfmaq_n_f32(a, b, n);
+}
Index: include/clang/Basic/arm_neon.td
===
--- include/clang/Basic/arm_neon.td
+++ include/clang/Basic/arm_neon.td
@@ -531,6 +531,7 @@
 let ArchGuard = "defined(__ARM_FEATURE_FMA)" in {
   def VFMA : SInst<"vfma", "", "fQf">;
   def VFMS : SOpInst<"vfms", "", "fQf", OP_FMLS>;
+  def FMLA_N_F32 : SOpInst<"vfma_n", "ddds", "fQf", OP_FMLA_N>;
 }
 
 

@@ -621,7 +622,7 @@
 // MUL, MLA, MLS, FMA, FMS definitions with scalar argument
 def VMUL_N_A64 : IOpInst<"vmul_n", "dds", "Qd", OP_MUL_N>;
 
-def FMLA_N : SOpInst<"vfma_n", "ddds", "fdQfQd", OP_FMLA_N>;
+def FMLA_N : SOpInst<"vfma_n", "ddds", "dQd", OP_FMLA_N>;
 def FMLS_N : SOpInst<"vfms_n", "ddds", "fdQfQd", OP_FMLS_N>;
 
 def MLA_N : SOpInst<"vmla_n", "ddds", "Qd", OP_MLA_N>;


Index: test/CodeGen/arm-neon-fma.c
===
--- test/CodeGen/arm-neon-fma.c
+++ test/CodeGen/arm-neon-fma.c
@@ -20,3 +20,27 @@
 float32x4_t test_fmaq_order(float32x4_t accum, float32x4_t lhs, float32x4_t rhs) {
   return vfmaq_f32(accum, lhs, rhs);
 }
+
+// CHECK-LABEL: define <2 x float> @test_vfma_n_f32(<2 x float> %a, <2 x float> %b, float %n) #0 {
+// CHECK:   [[VECINIT_I:%.*]] = insertelement <2 x float> undef, float %n, i32 0
+// CHECK:   [[VECINIT1_I:%.*]] = insertelement <2 x float> [[VECINIT_I]], float %n, i32 1
+// CHECK:   [[TMP1:%.*]] = bitcast <2 x float> %b to <8 x i8>
+// CHECK:   [[TMP2:%.*]] = bitcast <2 x float> [[VECINIT1_I]] to <8 x i8>
+// CHECK:   [[TMP3:%.*]] = call <2 x float> @llvm.fma.v2f32(<2 x float> %b, <2 x float> [[VECINIT1_I]], <2 x float> %a)
+// CHECK:   ret <2 x float> [[TMP3]]
+float32x2_t test_vfma_n_f32(float32x2_t a, float32x2_t b, float32_t n) {
+  return vfma_n_f32(a, b, n);
+}
+
+// CHECK-LABEL: define <4 x float> @test_vfmaq_n_f32(<4 x float> %a, <4 x float> %b, float %n) #0 {
+// CHECK:   [[VECINIT_I:%.*]] = insertelement <4 x float> undef, float %n, i32 0
+// CHECK:   [[VECINIT1_I:%.*]] = insertelement <4 x float> [[VECINIT_I]], float %n, i32 1
+// CHECK:   [[VECINIT2_I:%.*]] = insertelement <4 x float> [[VECINIT1_I]], float %n, i32 2
+// CHECK:   [[VECINIT3_I:%.*]] = insertelement <4 x float> [[VECINIT2_I]], float %n, i32 3
+// CHECK:   [[TMP1:%.*]] = bitcast <4 x float> %b to <16 x i8>
+// CHECK:   [[TMP2:%.*]] = bitcast <4 x float> [[VECINIT3_I]] to <16 x i8>
+// CHECK:   [[TMP3:%.*]] = call <4 x float> @llvm.fma.v4f32(<4 x float> %b, <4 x float> [[VECINIT3_I]], <4 x float> %a)
+// CHECK:   ret <4 x float> [[TMP3]]
+float32x4_t test_vfmaq_n_f32(float32x4_t a, float32x4_t b, float32_t n) {
+  return vfmaq_n_f32(a, b, n);
+}
Index: include/clang/Basic/arm_neon.td
===
--- include/clang/Basic/arm_neon.td
++

[PATCH] D45441: [HIP] Add predefined macros __HIPCC__ and __HIP_DEVICE_COMPILE__

2018-04-17 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl marked an inline comment as done.
yaxunl added inline comments.



Comment at: lib/Frontend/InitPreprocessor.cpp:473
+  Builder.defineMacro("__HIP_DEVICE_COMPILE__");
+  }
 }

rjmccall wrote:
> I assume these names are defined by the HIP spec.
Yes these are defined in the official HIP documentation

https://github.com/ROCm-Developer-Tools/HIP/blob/master/docs/markdown/hip_porting_guide.md


https://reviews.llvm.org/D45441



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


[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

2018-04-17 Thread Daniel Kolozsvari via Phabricator via cfe-commits
koldaniel added a comment.

In https://reviews.llvm.org/D35068#1049530, @george.karpenkov wrote:

> @koldaniel Have you evaluated this checker? On which codebases? Were the 
> warnings real security issues, or were they mostly spurious? The code seems 
> fine, but I'm not sure whether it should be in `security` or in `alpha`.


I've evaluated this checker on LLVM+Clang, there were only a few (about 15) 
warnings,  because of the C11 flag check at the beginning of the checker body. 
However, if this check was removed, number of the warnings would be increased 
significantly. I wouldn't say the findings were real security issues, most of 
the warnings were about usages of deprecated functions, which has not been 
considered unsecure (but which may cause problems if the code is modified in an 
improper way in the future).


https://reviews.llvm.org/D35068



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


[PATCH] D44882: [clangd] Implementation of workspace/symbol request

2018-04-17 Thread Marc-Andre Laperle via Phabricator via cfe-commits
malaperle added a comment.

Any objections to the latest version?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44882



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


[PATCH] D45726: Format closing braces when reformatting the line containing theopening brace.This required a couple of yaks to be shaved:1. MatchingOpeningBlockLineIndex was misused to also store the

2018-04-17 Thread Manuel Klimek via Phabricator via cfe-commits
klimek created this revision.
klimek added a reviewer: krasimir.

...doesn't

  work correctly for "} else {".

2. We needed to change the API of AffectedRangeManager to not use iterators; we 
always passed in begin / end for the whole container before, so there was no 
mismatch in generality.
3. We need an extra check to discontinue formatting at the top level, as we now 
sometimes change the indent of the closing brace, but want to bail out 
immediately afterwards, for example: void f() { if (a) { } void g(); 
Previously: void f() { if (a) { } void g(); Now: void f() { if (a) { } void g();

Format closing braces.


Repository:
  rC Clang

https://reviews.llvm.org/D45726

Files:
  lib/Format/AffectedRangeManager.cpp
  lib/Format/AffectedRangeManager.h
  lib/Format/Format.cpp
  lib/Format/NamespaceEndCommentsFixer.cpp
  lib/Format/SortJavaScriptImports.cpp
  lib/Format/TokenAnnotator.h
  lib/Format/UnwrappedLineFormatter.cpp
  lib/Format/UnwrappedLineParser.cpp
  lib/Format/UnwrappedLineParser.h
  lib/Format/UsingDeclarationsSorter.cpp
  unittests/Format/FormatTestSelective.cpp

Index: unittests/Format/FormatTestSelective.cpp
===
--- unittests/Format/FormatTestSelective.cpp
+++ unittests/Format/FormatTestSelective.cpp
@@ -177,6 +177,72 @@
0, 0));
 }
 
+TEST_F(FormatTestSelective, ContinueReindenting) {
+  // When we change an indent, we continue formatting as long as following
+  // lines are not indented correctly.
+  EXPECT_EQ("int   i;\n"
+"int b;\n"
+"int c;\n"
+"int d;\n"
+"int e;\n"
+"  int f;\n",
+format("int   i;\n"
+   "  int b;\n"
+   " int   c;\n"
+   "  int d;\n"
+   "int e;\n"
+   "  int f;\n",
+   11, 0));
+}
+
+TEST_F(FormatTestSelective, ReindentClosingBrace) {
+  EXPECT_EQ("int   i;\n"
+"int f() {\n"
+"  int a;\n"
+"  int b;\n"
+"}\n"
+" int c;\n",
+format("int   i;\n"
+   "  int f(){\n"
+   "int a;\n"
+   "int b;\n"
+   "  }\n"
+   " int c;\n",
+   11, 0));
+  EXPECT_EQ("void f() {\n"
+"  if (foo) {\n"
+"b();\n"
+"  } else {\n"
+"c();\n"
+"  }\n"
+"int d;\n"
+"}\n",
+format("void f() {\n"
+   "  if (foo) {\n"
+   "b();\n"
+   "}else{\n"
+   "c();\n"
+   "}\n"
+   "int d;\n"
+   "}\n",
+   13, 0));
+  EXPECT_EQ("int i = []() {\n"
+"  class C {\n"
+"int a;\n"
+"int b;\n"
+"  };\n"
+"  int c;\n"
+"};\n",
+format("int i = []() {\n"
+   "  class C{\n"
+   "int a;\n"
+   "int b;\n"
+   "};\n"
+   "int c;\n"
+   "  };\n",
+   17, 0));
+}
+
 TEST_F(FormatTestSelective, IndividualStatementsOfNestedBlocks) {
   EXPECT_EQ("DEBUG({\n"
 "  int i;\n"
@@ -503,7 +569,7 @@
   "  if (a) {\n"
   "g();\n"
   "h();\n"
-  "}\n"
+  "  }\n"
   "\n"
   "void g() {\n"
   "}",
Index: lib/Format/UsingDeclarationsSorter.cpp
===
--- lib/Format/UsingDeclarationsSorter.cpp
+++ lib/Format/UsingDeclarationsSorter.cpp
@@ -187,8 +187,7 @@
 TokenAnnotator &Annotator, SmallVectorImpl &AnnotatedLines,
 FormatTokenLexer &Tokens) {
   const SourceManager &SourceMgr = Env.getSourceManager();
-  AffectedRangeMgr.computeAffectedLines(AnnotatedLines.begin(),
-AnnotatedLines.end());
+  AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
   tooling::Replacements Fixes;
   SmallVector UsingDeclarations;
   for (size_t I = 0, E = AnnotatedLines.size(); I != E; ++I) {
Index: lib/Format/UnwrappedLineParser.h
===
--- lib/Format/UnwrappedLineParser.h
+++ lib/Format/UnwrappedLineParser.h
@@ -53,7 +53,10 @@
   /// \c MatchingOpeningBlockLineIndex stores the index of the corresponding
   /// opening line. Otherwise, \c MatchingOpeningBlockLineIndex must be
   /// \c kInvalidIndex.
-  size_t MatchingOpeningBlockLineIndex;
+  size_t MatchingOpeningBlockLineIndex = kInvalidIndex;
+
+  /// \brief The corresponding closing line to MatchingOpeningBlockLineIndex.
+  size_t MatchingClosingBlockLineIndex = kInvalidIndex;
 
   static const size_t kInvalidIndex = -1;
 
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/For

r330194 - [ThinLTO] Pass -save-temps to LTO backend for distributed ThinLTO builds

2018-04-17 Thread Teresa Johnson via cfe-commits
Author: tejohnson
Date: Tue Apr 17 09:39:25 2018
New Revision: 330194

URL: http://llvm.org/viewvc/llvm-project?rev=330194&view=rev
Log:
[ThinLTO] Pass -save-temps to LTO backend for distributed ThinLTO builds

Summary:
The clang driver option -save-temps was not passed to the LTO config,
so when invoking the ThinLTO backends via clang during distributed
builds there was no way to get LTO to save temp files.

Getting this to work with ThinLTO distributed builds also required
changing the driver to avoid a separate compile step to emit unoptimized
bitcode when the input was already bitcode under -save-temps. Not only is
this unnecessary in general, it is problematic for ThinLTO backends since
the temporary bitcode file to the backend would not match the module path
in the combined index, leading to incorrect ThinLTO backend index-based
optimizations.

Reviewers: pcc

Subscribers: mehdi_amini, inglorion, eraman, cfe-commits

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

Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Frontend/CodeGenOptions.h
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/CodeGen/thinlto_backend.ll
cfe/trunk/test/Driver/thinlto_backend.c

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=330194&r1=330193&r2=330194&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Tue Apr 17 09:39:25 2018
@@ -2275,7 +2275,7 @@ def fno_rtlib_add_rpath: Flag<["-"], "fn
   HelpText<"Do not add -rpath with architecture-specific resource directory to 
the linker flags">;
 def r : Flag<["-"], "r">, Flags<[LinkerInput,NoArgumentUnused]>,
 Group;
-def save_temps_EQ : Joined<["-", "--"], "save-temps=">, Flags<[DriverOption]>,
+def save_temps_EQ : Joined<["-", "--"], "save-temps=">, Flags<[CC1Option, 
DriverOption]>,
   HelpText<"Save intermediate compilation results.">;
 def save_temps : Flag<["-", "--"], "save-temps">, Flags<[DriverOption]>,
   Alias, AliasArgs<["cwd"]>,

Modified: cfe/trunk/include/clang/Frontend/CodeGenOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CodeGenOptions.h?rev=330194&r1=330193&r2=330194&view=diff
==
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.h (original)
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.h Tue Apr 17 09:39:25 2018
@@ -203,6 +203,9 @@ public:
   /// the summary and module symbol table (and not, e.g. any debug metadata).
   std::string ThinLinkBitcodeFile;
 
+  /// Prefix to use for -save-temps output.
+  std::string SaveTempsFilePrefix;
+
   /// Name of file passed with -fcuda-include-gpubinary option to forward to
   /// CUDA runtime back-end for incorporating them into host-side object file.
   std::string CudaGpuBinaryFileName;

Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=330194&r1=330193&r2=330194&view=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Tue Apr 17 09:39:25 2018
@@ -1117,6 +1117,15 @@ static void runThinLTOBackend(ModuleSumm
 return llvm::make_unique(std::move(OS));
   };
   lto::Config Conf;
+  if (CGOpts.SaveTempsFilePrefix != "") {
+if (Error E = Conf.addSaveTemps(CGOpts.SaveTempsFilePrefix + ".",
+/* UseInputModulePath */ false)) {
+  handleAllErrors(std::move(E), [&](ErrorInfoBase &EIB) {
+errs() << "Error setting up ThinLTO save-temps: " << EIB.message()
+   << '\n';
+  });
+}
+  }
   Conf.CPU = TOpts.CPU;
   Conf.CodeModel = getCodeModel(CGOpts);
   Conf.MAttrs = TOpts.Features;

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=330194&r1=330193&r2=330194&view=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Tue Apr 17 09:39:25 2018
@@ -3379,19 +3379,34 @@ class ToolSelector final {
   const Tool *combineBackendCompile(ArrayRef ActionInfo,
 const ActionList *&Inputs,
 ActionList &CollapsedOffloadAction) {
-if (ActionInfo.size() < 2 || !canCollapsePreprocessorAction())
+if (ActionInfo.size() < 2)
   return nullptr;
 auto *BJ = dyn_cast(ActionInfo[0].JA);
 auto *CJ = dyn_cast(ActionInfo[1].JA);
 if (!BJ || !CJ)
   return nullptr;
 

[PATCH] D45217: [ThinLTO] Pass -save-temps to LTO backend for distributed ThinLTO builds

2018-04-17 Thread Teresa Johnson via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL330194: [ThinLTO] Pass -save-temps to LTO backend for 
distributed ThinLTO builds (authored by tejohnson, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D45217

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/include/clang/Frontend/CodeGenOptions.h
  cfe/trunk/lib/CodeGen/BackendUtil.cpp
  cfe/trunk/lib/Driver/Driver.cpp
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/CodeGen/thinlto_backend.ll
  cfe/trunk/test/Driver/thinlto_backend.c

Index: cfe/trunk/test/CodeGen/thinlto_backend.ll
===
--- cfe/trunk/test/CodeGen/thinlto_backend.ll
+++ cfe/trunk/test/CodeGen/thinlto_backend.ll
@@ -26,8 +26,16 @@
 ; 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 | count 0
 
-; Ensure f2 was imported
-; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc
+; Ensure f2 was imported. Check for all 3 flavors of -save-temps[=cwd|obj].
+; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps=obj
+; RUN: llvm-dis %t1.s.0.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s
+; RUN: mkdir -p %T/dir1
+; RUN: (cd %T/dir1 && %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps=cwd)
+; RUN: llvm-dis %T/dir1/*1.s.0.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s
+; RUN: mkdir -p %T/dir2
+; RUN: (cd %T/dir2 && %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps)
+; RUN: llvm-dis %T/dir2/*1.s.0.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s
+; CHECK-IMPORT: define available_externally void @f2()
 ; RUN: llvm-nm %t3.o | FileCheck --check-prefix=CHECK-OBJ %s
 ; CHECK-OBJ: T f1
 ; CHECK-OBJ-NOT: U f2
Index: cfe/trunk/test/Driver/thinlto_backend.c
===
--- cfe/trunk/test/Driver/thinlto_backend.c
+++ cfe/trunk/test/Driver/thinlto_backend.c
@@ -5,6 +5,15 @@
 // RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -### 2>&1 | FileCheck %s -check-prefix=CHECK-THINLTOBE-ACTION
 // CHECK-THINLTOBE-ACTION: -fthinlto-index=
 
+// -save-temps should be passed to cc1
+// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -save-temps -### 2>&1 | FileCheck %s -check-prefix=CHECK-SAVE-TEMPS -check-prefix=CHECK-SAVE-TEMPS-CWD
+// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -save-temps=cwd -### 2>&1 | FileCheck %s -check-prefix=CHECK-SAVE-TEMPS -check-prefix=CHECK-SAVE-TEMPS-CWD
+// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -save-temps=obj -### 2>&1 | FileCheck %s -check-prefix=CHECK-SAVE-TEMPS -check-prefix=CHECK-SAVE-TEMPS-OBJ
+// CHECK-SAVE-TEMPS-NOT: -emit-llvm-bc
+// CHECK-SAVE-TEMPS-CWD: -save-temps=cwd
+// CHECK-SAVE-TEMPS-OBJ: -save-temps=obj
+// CHECK-SAVE-TEMPS-NOT: -emit-llvm-bc
+
 // Ensure clang driver gives the expected error for incorrect input type
 // RUN: not %clang -O2 -o %t1.o %s -c -fthinlto-index=%t.thinlto.bc 2>&1 | FileCheck %s -check-prefix=CHECK-WARNING
 // CHECK-WARNING: error: invalid argument '-fthinlto-index={{.*}}' only allowed with '-x ir'
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -3267,6 +3267,9 @@
 Args.AddLastArg(CmdArgs, options::OPT_fthinlto_index_EQ);
   }
 
+  if (const Arg *A = Args.getLastArg(options::OPT_save_temps_EQ))
+Args.AddLastArg(CmdArgs, options::OPT_save_temps_EQ);
+
   // Embed-bitcode option.
   if (C.getDriver().embedBitcodeInObject() && !C.getDriver().isUsingLTO() &&
   (isa(JA) || isa(JA))) {
Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -3379,19 +3379,34 @@
   const Tool *combineBackendCompile(ArrayRef ActionInfo,
 const ActionList *&Inputs,
 ActionList &CollapsedOffloadAction) {
-if (ActionInfo.size() < 2 || !canCollapsePreprocessorAction())
+if (ActionInfo.size() < 2)
   return nullptr;
 auto *BJ = dyn_cast(ActionInfo[0].JA);
 auto *CJ = dyn_cast(ActionInfo[1].JA);
 if (!BJ || !CJ)
   return nullptr;
 
+// Check if the initial input (to the compile job or its predessor if one
+// exists) is LLVM bitcode. In that case, no preprocessor step is required
+// and we can still collapse the compile and backend jobs when w

r330195 - [NEON] Define vget_high_f16() and vget_low_f16() intrinsics in AArch64 mode only

2018-04-17 Thread Ivan A. Kosarev via cfe-commits
Author: kosarev
Date: Tue Apr 17 09:43:07 2018
New Revision: 330195

URL: http://llvm.org/viewvc/llvm-project?rev=330195&view=rev
Log:
[NEON] Define vget_high_f16() and vget_low_f16() intrinsics in AArch64 mode only

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

Modified:
cfe/trunk/include/clang/Basic/arm_neon.td
cfe/trunk/test/CodeGen/arm_neon_intrinsics.c

Modified: cfe/trunk/include/clang/Basic/arm_neon.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/arm_neon.td?rev=330195&r1=330194&r2=330195&view=diff
==
--- cfe/trunk/include/clang/Basic/arm_neon.td (original)
+++ cfe/trunk/include/clang/Basic/arm_neon.td Tue Apr 17 09:43:07 2018
@@ -398,8 +398,14 @@ def VCOMBINE : NoTestOpInst<"vcombine",
 

 // E.3.21 Splitting vectors
 let InstName = "vmov" in {
-def VGET_HIGH : NoTestOpInst<"vget_high", "dk", "csilhfUcUsUiUlPcPs", OP_HI>;
-def VGET_LOW  : NoTestOpInst<"vget_low", "dk", "csilhfUcUsUiUlPcPs", OP_LO>;
+def VGET_HIGH : NoTestOpInst<"vget_high", "dk", "csilfUcUsUiUlPcPs", OP_HI>;
+def VGET_LOW  : NoTestOpInst<"vget_low", "dk", "csilfUcUsUiUlPcPs", OP_LO>;
+}
+let ArchGuard = "__ARM_ARCH >= 8 && defined(__aarch64__)" in {
+  let InstName = "vmov" in {
+  def VGET_HIGH_F16 : NoTestOpInst<"vget_high", "dk", "h", OP_HI>;
+  def VGET_LOW_F16  : NoTestOpInst<"vget_low", "dk", "h", OP_LO>;
+  }
 }
 
 


Modified: cfe/trunk/test/CodeGen/arm_neon_intrinsics.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/arm_neon_intrinsics.c?rev=330195&r1=330194&r2=330195&view=diff
==
--- cfe/trunk/test/CodeGen/arm_neon_intrinsics.c (original)
+++ cfe/trunk/test/CodeGen/arm_neon_intrinsics.c Tue Apr 17 09:43:07 2018
@@ -3254,13 +3254,6 @@ int64x1_t test_vget_high_s64(int64x2_t a
   return vget_high_s64(a);
 }
 
-// CHECK-LABEL: @test_vget_high_f16(
-// CHECK:   [[SHUFFLE_I:%.*]] = shufflevector <8 x half> %a, <8 x half> %a, <4 
x i32> 
-// CHECK:   ret <4 x half> [[SHUFFLE_I]]
-float16x4_t test_vget_high_f16(float16x8_t a) {
-  return vget_high_f16(a);
-}
-
 // CHECK-LABEL: @test_vget_high_f32(
 // CHECK:   [[SHUFFLE_I:%.*]] = shufflevector <4 x float> %a, <4 x float> %a, 
<2 x i32> 
 // CHECK:   ret <2 x float> [[SHUFFLE_I]]
@@ -3560,13 +3553,6 @@ int64x1_t test_vget_low_s64(int64x2_t a)
   return vget_low_s64(a);
 }
 
-// CHECK-LABEL: @test_vget_low_f16(
-// CHECK:   [[SHUFFLE_I:%.*]] = shufflevector <8 x half> %a, <8 x half> %a, <4 
x i32> 
-// CHECK:   ret <4 x half> [[SHUFFLE_I]]
-float16x4_t test_vget_low_f16(float16x8_t a) {
-  return vget_low_f16(a);
-}
-
 // CHECK-LABEL: @test_vget_low_f32(
 // CHECK:   [[SHUFFLE_I:%.*]] = shufflevector <4 x float> %a, <4 x float> %a, 
<2 x i32> 
 // CHECK:   ret <2 x float> [[SHUFFLE_I]]


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


[PATCH] D45668: [NEON] Define vget_high_f16() and vget_low_f16() intrinsics in AArch64 mode only

2018-04-17 Thread Ivan Kosarev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL330195: [NEON] Define vget_high_f16() and vget_low_f16() 
intrinsics in AArch64 mode only (authored by kosarev, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D45668?vs=142558&id=142789#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45668

Files:
  cfe/trunk/include/clang/Basic/arm_neon.td
  cfe/trunk/test/CodeGen/arm_neon_intrinsics.c


Index: cfe/trunk/include/clang/Basic/arm_neon.td
===
--- cfe/trunk/include/clang/Basic/arm_neon.td
+++ cfe/trunk/include/clang/Basic/arm_neon.td
@@ -398,8 +398,14 @@
 

 // E.3.21 Splitting vectors
 let InstName = "vmov" in {
-def VGET_HIGH : NoTestOpInst<"vget_high", "dk", "csilhfUcUsUiUlPcPs", OP_HI>;
-def VGET_LOW  : NoTestOpInst<"vget_low", "dk", "csilhfUcUsUiUlPcPs", OP_LO>;
+def VGET_HIGH : NoTestOpInst<"vget_high", "dk", "csilfUcUsUiUlPcPs", OP_HI>;
+def VGET_LOW  : NoTestOpInst<"vget_low", "dk", "csilfUcUsUiUlPcPs", OP_LO>;
+}
+let ArchGuard = "__ARM_ARCH >= 8 && defined(__aarch64__)" in {
+  let InstName = "vmov" in {
+  def VGET_HIGH_F16 : NoTestOpInst<"vget_high", "dk", "h", OP_HI>;
+  def VGET_LOW_F16  : NoTestOpInst<"vget_low", "dk", "h", OP_LO>;
+  }
 }
 
 

Index: cfe/trunk/test/CodeGen/arm_neon_intrinsics.c
===
--- cfe/trunk/test/CodeGen/arm_neon_intrinsics.c
+++ cfe/trunk/test/CodeGen/arm_neon_intrinsics.c
@@ -3254,13 +3254,6 @@
   return vget_high_s64(a);
 }
 
-// CHECK-LABEL: @test_vget_high_f16(
-// CHECK:   [[SHUFFLE_I:%.*]] = shufflevector <8 x half> %a, <8 x half> %a, <4 
x i32> 
-// CHECK:   ret <4 x half> [[SHUFFLE_I]]
-float16x4_t test_vget_high_f16(float16x8_t a) {
-  return vget_high_f16(a);
-}
-
 // CHECK-LABEL: @test_vget_high_f32(
 // CHECK:   [[SHUFFLE_I:%.*]] = shufflevector <4 x float> %a, <4 x float> %a, 
<2 x i32> 
 // CHECK:   ret <2 x float> [[SHUFFLE_I]]
@@ -3560,13 +3553,6 @@
   return vget_low_s64(a);
 }
 
-// CHECK-LABEL: @test_vget_low_f16(
-// CHECK:   [[SHUFFLE_I:%.*]] = shufflevector <8 x half> %a, <8 x half> %a, <4 
x i32> 
-// CHECK:   ret <4 x half> [[SHUFFLE_I]]
-float16x4_t test_vget_low_f16(float16x8_t a) {
-  return vget_low_f16(a);
-}
-
 // CHECK-LABEL: @test_vget_low_f32(
 // CHECK:   [[SHUFFLE_I:%.*]] = shufflevector <4 x float> %a, <4 x float> %a, 
<2 x i32> 
 // CHECK:   ret <2 x float> [[SHUFFLE_I]]


Index: cfe/trunk/include/clang/Basic/arm_neon.td
===
--- cfe/trunk/include/clang/Basic/arm_neon.td
+++ cfe/trunk/include/clang/Basic/arm_neon.td
@@ -398,8 +398,14 @@
 
 // E.3.21 Splitting vectors
 let InstName = "vmov" in {
-def VGET_HIGH : NoTestOpInst<"vget_high", "dk", "csilhfUcUsUiUlPcPs", OP_HI>;
-def VGET_LOW  : NoTestOpInst<"vget_low", "dk", "csilhfUcUsUiUlPcPs", OP_LO>;
+def VGET_HIGH : NoTestOpInst<"vget_high", "dk", "csilfUcUsUiUlPcPs", OP_HI>;
+def VGET_LOW  : NoTestOpInst<"vget_low", "dk", "csilfUcUsUiUlPcPs", OP_LO>;
+}
+let ArchGuard = "__ARM_ARCH >= 8 && defined(__aarch64__)" in {
+  let InstName = "vmov" in {
+  def VGET_HIGH_F16 : NoTestOpInst<"vget_high", "dk", "h", OP_HI>;
+  def VGET_LOW_F16  : NoTestOpInst<"vget_low", "dk", "h", OP_LO>;
+  }
 }
 
 
Index: cfe/trunk/test/CodeGen/arm_neon_intrinsics.c
===
--- cfe/trunk/test/CodeGen/arm_neon_intrinsics.c
+++ cfe/trunk/test/CodeGen/arm_neon_intrinsics.c
@@ -3254,13 +3254,6 @@
   return vget_high_s64(a);
 }
 
-// CHECK-LABEL: @test_vget_high_f16(
-// CHECK:   [[SHUFFLE_I:%.*]] = shufflevector <8 x half> %a, <8 x half> %a, <4 x i32> 
-// CHECK:   ret <4 x half> [[SHUFFLE_I]]
-float16x4_t test_vget_high_f16(float16x8_t a) {
-  return vget_high_f16(a);
-}
-
 // CHECK-LABEL: @test_vget_high_f32(
 // CHECK:   [[SHUFFLE_I:%.*]] = shufflevector <4 x float> %a, <4 x float> %a, <2 x i32> 
 // CHECK:   ret <2 x float> [[SHUFFLE_I]]
@@ -3560,13 +3553,6 @@
   return vget_low_s64(a);
 }
 
-// CHECK-LABEL: @test_vget_low_f16(
-// CHECK:   [[SHUFFLE_I:%.*]] = shufflevector <8 x half> %a, <8 x half> %a, <4 x i32> 
-// CHECK:   ret <4 x half> [[SHUFFLE_I]]
-float16x4_t test_vget_low_f16(float16x8_t a) {
-  return vget_low_f16(a);
-}
-
 // CHECK-LABEL: @test_vget_low_f32(
 // CHECK:   [[SHUFFLE_I:%.*]] = shufflevector <4 x float> %a, <4 x float> %a, <2 x i32> 
 // CHECK:   ret <2 x float> [[SHUFFLE_I]]
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listin

[PATCH] D45716: [XRay] Add clang builtin for xray typed events.

2018-04-17 Thread Keith via Phabricator via cfe-commits
kpw updated this revision to Diff 142790.
kpw added a comment.

Undoing formatting change.


Repository:
  rC Clang

https://reviews.llvm.org/D45716

Files:
  include/clang/Basic/Builtins.def
  include/clang/Basic/LangOptions.def
  include/clang/Basic/XRayInstr.h
  include/clang/Driver/Options.td
  include/clang/Driver/XRayArgs.h
  include/clang/Frontend/CodeGenOptions.def
  lib/Basic/XRayInstr.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Driver/XRayArgs.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/xray-always-emit-typedevent.cpp
  test/CodeGen/xray-instrumentation-bundles.cpp
  test/CodeGen/xray-typedevent.cpp

Index: test/CodeGen/xray-typedevent.cpp
===
--- /dev/null
+++ test/CodeGen/xray-typedevent.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -fxray-instrument -x c++ -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck %s
+
+// CHECK-LABEL: @_Z16alwaysInstrumentv
+[[clang::xray_always_instrument]] void alwaysInstrument() {
+  // Event types would normally come from calling __xray_register_event_type
+  // from compiler-rt
+  auto EventType = 1;
+  static constexpr char kPhase[] = "instrument";
+  __xray_typedevent(EventType, kPhase, 10);
+  // CHECK: call void @llvm.xray.typedevent(i16 {{.*}}, i8*{{.*}}, i32 10)
+}
+
+// CHECK-LABEL: @_Z15neverInstrumentv
+[[clang::xray_never_instrument]] void neverInstrument() {
+  auto EventType = 2;
+  static constexpr char kPhase[] = "never";
+  __xray_typedevent(EventType, kPhase, 5);
+  // CHECK-NOT: call void @llvm.xray.typedevent(i16 {{.*}}, i8*{{.*}}, i32 5)
+}
+
+// CHECK-LABEL: @_Z21conditionalInstrumenti
+[[clang::xray_always_instrument]] void conditionalInstrument(int v) {
+  auto TrueEventType = 3;
+  auto UntrueEventType = 4;
+  static constexpr char kTrue[] = "true";
+  static constexpr char kUntrue[] = "untrue";
+  if (v % 2)
+__xray_typedevent(TrueEventType, kTrue, 4);
+  else
+__xray_typedevent(UntrueEventType, kUntrue, 6);
+
+  // CHECK: call void @llvm.xray.typedevent(i16 {{.*}}, i8*{{.*}}, i32 4)
+  // CHECK: call void @llvm.xray.typedevent(i16 {{.*}}, i8*{{.*}}, i32 6)
+}
Index: test/CodeGen/xray-instrumentation-bundles.cpp
===
--- test/CodeGen/xray-instrumentation-bundles.cpp
+++ test/CodeGen/xray-instrumentation-bundles.cpp
@@ -1,30 +1,49 @@
 // RUN: %clang_cc1 -fxray-instrument -fxray-instrumentation-bundle=none -x c++ \
 // RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
-// RUN: | FileCheck --check-prefixes CHECK,NOFUNCTION,NOCUSTOM %s
-// RUN: %clang_cc1 -fxray-instrument \
-// RUN: -fxray-instrumentation-bundle=function -x c++ \
+// RUN: | FileCheck --check-prefixes CHECK,NOFUNCTION,NOCUSTOM,NOTYPED %s
+// RUN: %clang_cc1 -fxray-instrument -fxray-instrumentation-bundle=function -x c++ \
 // RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
-// RUN: | FileCheck --check-prefixes CHECK,FUNCTION,NOCUSTOM %s
+// RUN: | FileCheck --check-prefixes CHECK,FUNCTION,NOCUSTOM,NOTYPED %s
 // RUN: %clang_cc1 -fxray-instrument \
 // RUN: -fxray-instrumentation-bundle=custom -x c++ \
 // RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
-// RUN: | FileCheck --check-prefixes CHECK,NOFUNCTION,CUSTOM %s
+// RUN: | FileCheck --check-prefixes CHECK,NOFUNCTION,CUSTOM,NOTYPED %s
+// RUN: %clang_cc1 -fxray-instrument \
+// RUN: -fxray-instrumentation-bundle=typed -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,NOFUNCTION,NOCUSTOM,TYPED %s
+// RUN: %clang_cc1 -fxray-instrument \
+// RUN: -fxray-instrumentation-bundle=custom,typed -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,NOFUNCTION,CUSTOM,TYPED %s
 // RUN: %clang_cc1 -fxray-instrument \
 // RUN: -fxray-instrumentation-bundle=function,custom -x c++ \
 // RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
-// RUN: | FileCheck --check-prefixes CHECK,FUNCTION,CUSTOM %s
+// RUN: | FileCheck --check-prefixes CHECK,FUNCTION,CUSTOM,NOTYPED %s
+// RUN: %clang_cc1 -fxray-instrument \
+// RUN: -fxray-instrumentation-bundle=function,typed -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,FUNCTION,NOCUSTOM,TYPED %s
+// RUN: %clang_cc1 -fxray-instrument \
+// RUN: -fxray-instrumentation-bundle=function,custom,typed -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,FUNCTION,CUSTOM,TYPED %s
 // RUN: %clang_cc1 -fxray-instrument \
 // RUN: -fxray-instrumentation-bundle=function \
-// RUN: -fxray-instrumentation-bundle=custo

[PATCH] D45720: [X86] Lowering PACK*S (pack with saturation) intrinsics to native IR (clang side)

2018-04-17 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added inline comments.



Comment at: lib/CodeGen/CGBuiltin.cpp:8420
+  if (IsUnsigned) {
+MinVal = (IsDW) ? llvm::APInt::getMinValue(16).getZExtValue()
+: llvm::APInt::getMinValue(8).getZExtValue();

Why can't these just be APInts instead of uint64_t? Is this so that APInt 
widths don't have to match RTy below? I'd rather you just created the narrow 
APInt and then called sext/zext on it to get it to the right width.



Comment at: lib/CodeGen/CGBuiltin.cpp:8420
+  if (IsUnsigned) {
+MinVal = (IsDW) ? llvm::APInt::getMinValue(16).getZExtValue()
+: llvm::APInt::getMinValue(8).getZExtValue();

craig.topper wrote:
> Why can't these just be APInts instead of uint64_t? Is this so that APInt 
> widths don't have to match RTy below? I'd rather you just created the narrow 
> APInt and then called sext/zext on it to get it to the right width.
Pre-select the 8 or 16 based on IsDW. Then you don't need to check IsDW 4 
times. You just need to pass the right width.



Comment at: lib/CodeGen/CGBuiltin.cpp:8432
+  SmallVector ShuffleMask;
+  ShuffleMask.clear();
+  for (int i = 0, i1 = 0, i2 = 0, d = (IsDW) ? 4 : 8; i < NumElts; ++i)

Clearing isn't necessary if you just created it.



Comment at: lib/CodeGen/CGBuiltin.cpp:8433
+  ShuffleMask.clear();
+  for (int i = 0, i1 = 0, i2 = 0, d = (IsDW) ? 4 : 8; i < NumElts; ++i)
+if ((i / d) & 1)

This loop could probably use some comments. The multiple variables make the 
logic hard to follow



Comment at: lib/CodeGen/CGBuiltin.cpp:8443
+  Value *MaxVec = llvm::ConstantInt::get(RTy, MaxVal);
+  Res = EmitX86MinMax(CGF, ICmpInst::ICMP_SLT, {Res, MaxVec});
+  Res = EmitX86MinMax(CGF, ICmpInst::ICMP_SGT, {Res, MinVec});

Why arent' these unsigned compares for Unsigned?



Comment at: lib/CodeGen/CGBuiltin.cpp:8446
+  llvm::Type *VTy = llvm::VectorType::get(
+  (IsDW) ? CGF.Builder.getInt16Ty() : CGF.Builder.getInt8Ty(), NumElts);
+  return CGF.Builder.CreateTrunc(Res, VTy);

If you have the 8 or 16 selected above, you can use getIntNTy here I think.


Repository:
  rC Clang

https://reviews.llvm.org/D45720



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


[PATCH] D45722: [X86] Lowering SAD (sum of absolute differences) intrinsics to native IR (clang side)

2018-04-17 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added inline comments.



Comment at: lib/CodeGen/CGBuiltin.cpp:8427
+  SmallVector ShuffleMask;
+  ShuffleMask.clear();
+  for (unsigned i = 0; i < N; ++i)

This clear isn't needed.



Comment at: lib/CodeGen/CGBuiltin.cpp:8432
+  CGF.Builder.CreateShuffleVector(AD, llvm::UndefValue::get(BTy),
+  ArrayRef(ShuffleMask)),
+  VTy);

You shouldn't need to explicitly create an ArrayRef here. It should 
automatically convert. And if it doesn't makeArrayRef is what you should use. 
It will automatically infer the uint32_t from the vector.


Repository:
  rC Clang

https://reviews.llvm.org/D45722



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


[PATCH] D33844: [clang-tidy] terminating continue check

2018-04-17 Thread Daniel Kolozsvari via Phabricator via cfe-commits
koldaniel updated this revision to Diff 142793.

https://reviews.llvm.org/D33844

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/TerminatingContinueCheck.cpp
  clang-tidy/bugprone/TerminatingContinueCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-terminating-continue.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-terminating-continue.cpp

Index: test/clang-tidy/bugprone-terminating-continue.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-terminating-continue.cpp
@@ -0,0 +1,65 @@
+// RUN: %check_clang_tidy %s bugprone-terminating-continue %t
+
+void f() {
+  do {
+continue;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'continue' in loop with false condition is equivalent to 'break' [bugprone-terminating-continue]
+// CHECK-FIXES: break;
+  } while(false);
+
+  do {
+continue;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'continue' in loop with false condition is equivalent to 'break' [bugprone-terminating-continue]
+// CHECK-FIXES: break;
+  } while(0);
+
+  do {
+continue;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'continue' in loop with false condition is equivalent to 'break' [bugprone-terminating-continue]
+// CHECK-FIXES: break;
+  } while(nullptr);
+
+  do {
+continue;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'continue' in loop with false condition is equivalent to 'break' [bugprone-terminating-continue]
+// CHECK-FIXES: break;
+  } while(__null);
+
+
+  do {
+int x = 1;
+if (x > 0) continue;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'continue' in loop with false condition is equivalent to 'break' [bugprone-terminating-continue]
+// CHECK-FIXES: if (x > 0) break;
+  } while (false);
+}
+
+void g() {
+  do {
+do {
+  continue;
+  int x = 1;
+} while (1 == 1);
+  } while (false);
+
+  do {
+for (int i = 0; i < 1; ++i) {
+  continue;
+  int x = 1;
+}
+  } while (false);
+
+  do {
+while (true) {
+  continue;
+  int x = 1;
+}
+  } while (false);
+
+  int v[] = {1,2,3,34};
+  do {
+for (int n : v) {
+  if (n>2) continue;
+}
+  } while (false);
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -48,6 +48,7 @@
bugprone-suspicious-semicolon
bugprone-suspicious-string-compare
bugprone-swapped-arguments
+   bugprone-terminating-continue
bugprone-throw-keyword-missing
bugprone-undefined-memory-manipulation
bugprone-undelegated-constructor
Index: docs/clang-tidy/checks/bugprone-terminating-continue.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/bugprone-terminating-continue.rst
@@ -0,0 +1,17 @@
+.. title:: clang-tidy - bugprone-terminating-continue
+
+bugprone-terminating-continue
+=
+
+Detects `do while` loops with a condition always evaluating to false that
+have a `continue` statement, as this `continue` terminates the loop
+effectively.
+
+.. code-block:: c++
+
+  void f() {
+  do {
+  	// some code
+continue; // terminating continue
+// some other code
+  } while(false);
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -130,6 +130,11 @@
   Warns or suggests alternatives if SIMD intrinsics are used which can be replaced by
   ``std::experimental::simd`` operations.
 
+- New :doc:`bugprone-terminating-continue
+  ` check
+
+  Checks if a ``continue`` statement terminates the loop.
+
 - New :doc:`zircon-temporary-objects
   ` check
 
Index: clang-tidy/bugprone/TerminatingContinueCheck.h
===
--- /dev/null
+++ clang-tidy/bugprone/TerminatingContinueCheck.h
@@ -0,0 +1,36 @@
+//===--- TerminatingContinueCheck.h - clang-tidy-*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_TERMINATINGCONTINUECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_TERMINATINGCONTINUECHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Checks if a 'continue' statement terminates the loop (i.e. the loop has
+///	a condition which always evaluates to false).
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-terminating-continue.html
+class TerminatingContinueCheck : public ClangTidyCheck {
+public:
+  Terminati

[PATCH] D45601: Warn on bool* to bool conversion

2018-04-17 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:2623
+QualType QT = Pointer->getType()->getPointeeType();
+if (!QT.isNull() && QT->isBooleanType())
+  // Warn on bool* to bool conversion.

Have you considered skipping the isBooleanType() check?  Passing any pointer to 
a function which expects a boolean parameter seems fundamentally suspicious.


https://reviews.llvm.org/D45601



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


[PATCH] D45679: [clang-tidy] Add a helper function isModified, that checks whether an expression is modified within a statement.

2018-04-17 Thread Shuai Wang via Phabricator via cfe-commits
shuaiwang added a comment.

In https://reviews.llvm.org/D45679#1069754, @Eugene.Zelenko wrote:

> In https://reviews.llvm.org/D45679#1069638, @JonasToth wrote:
>
> > *hust* /llvm/tools/clang/lib/Analysis/PseudoConstantAnalysis.cpp
> >
> > I will check this one first, before we get crazy and implemented something 
> > twice.
>
>
> But this is IR-level analysis. Clang has own include/clang/Analysis.


I investigated a bit and it seems to be deadcode :(
It was once used by IdempotentOperationChecker which got deleted 4 years ago: 
http://llvm.org/viewvc/llvm-project?view=revision&revision=198476
I'll take a look and see if I can learn something from it anyway.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45679



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


[PATCH] D45601: Warn on bool* to bool conversion

2018-04-17 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:2623
+QualType QT = Pointer->getType()->getPointeeType();
+if (!QT.isNull() && QT->isBooleanType())
+  // Warn on bool* to bool conversion.

efriedma wrote:
> Have you considered skipping the isBooleanType() check?  Passing any pointer 
> to a function which expects a boolean parameter seems fundamentally 
> suspicious.
We already know that LLVM itself passes pointers to functions in many places, 
and they're not bugs. I mean, this isn't the first time someone's proposed that 
pointer->bool conversions are evil. :)

I know this just came up on a mailing list somewhere recently (rsmith was 
involved in the rebuttal, as was I), but all I can find on Google is:
https://groups.google.com/a/isocpp.org/d/msg/std-proposals/a3g9qXFVGCs/Wj40qj48H3wJ
Anyway, the punch line I'm thinking of is something like
```
FooBar *FB = ...;
LLVMAssert(FB, "expected fb to be set");
```
but with whatever the real names are. Found a bunch of those when I wrote the 
diagnostic. And they're fairly solidly in the "not bugs" category.


https://reviews.llvm.org/D45601



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


[PATCH] D45382: [CodeGen] Avoid destructing a struct type that has already been destructed by a delegated constructor

2018-04-17 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 142792.
ahatanak added a comment.

Deactivate the cleanups for callee-destructed parameters that are being 
forwarded to a delegated constructor.

I also added a new member (CurrentCleanupStackDepth) to CodeGenFunction that 
tracks the stack depth of the most recent RunCleanupsScope. This is needed to 
prevent popping the cleanup at the top of the stack in DeactivateCleanupBlock 
if the cleanup doesn't belong to the current RunCleanupsScope. Without this, 
the current RunCleanupsScope's CleanupStackDepth can point to a cleanup that 
doesn't exist on the stack anymore 
(CleanupStackDepth.encloses(EHStack.stable_begin()) doesn't hold true anymore), 
which can cause CodeGenFunction::PopCleanupBlocks to pop all the cleanups 
before it hits the bottom of the stack and crash.


Repository:
  rC Clang

https://reviews.llvm.org/D45382

Files:
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CGCleanup.cpp
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CodeGenFunction.h
  test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
  test/CodeGenObjCXX/arc-special-member-functions.mm
  test/CodeGenObjCXX/lambda-expressions.mm

Index: test/CodeGenObjCXX/lambda-expressions.mm
===
--- test/CodeGenObjCXX/lambda-expressions.mm
+++ test/CodeGenObjCXX/lambda-expressions.mm
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10.0.0 -emit-llvm -o - %s -fexceptions -std=c++11 -fblocks -fobjc-arc | FileCheck -check-prefix=ARC %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10.0.0 -emit-llvm -o - %s -fexceptions -std=c++11 -fblocks -fobjc-arc -fobjc-runtime-has-weak -DWEAK_SUPPORTED | FileCheck -check-prefix=ARC %s
 // RUN: %clang_cc1 -triple x86_64-apple-darwin10.0.0 -emit-llvm -o - %s -fexceptions -std=c++11 -fblocks | FileCheck -check-prefix=MRC %s
 
 typedef int (^fp)();
@@ -138,5 +138,31 @@
 }
 @end
 
+// Check that the delegating invoke function doesn't destruct the Weak object
+// that is passed.
+
+// ARC-LABEL: define internal void @"_ZZN14LambdaDelegate4testEvEN3$_58__invokeENS_4WeakE"(
+// ARC: call void @"_ZZN14LambdaDelegate4testEvENK3$_5clENS_4WeakE"(
+// ARC-NEXT: ret void
+
+// ARC-LABEL: define internal void @"_ZZN14LambdaDelegate4testEvENK3$_5clENS_4WeakE"(
+// ARC: call void @_ZN14LambdaDelegate4WeakD1Ev(
+
+#ifdef WEAK_SUPPORTED
+
+namespace LambdaDelegate {
+
+struct Weak {
+  __weak id x;
+};
+
+void test() {
+  void (*p)(Weak) = [](Weak a) { };
+}
+
+};
+
+#endif
+
 // ARC: attributes [[NUW]] = { noinline nounwind{{.*}} }
 // MRC: attributes [[NUW]] = { noinline nounwind{{.*}} }
Index: test/CodeGenObjCXX/arc-special-member-functions.mm
===
--- test/CodeGenObjCXX/arc-special-member-functions.mm
+++ test/CodeGenObjCXX/arc-special-member-functions.mm
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fobjc-arc -fblocks -triple x86_64-apple-darwin10.0.0 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -fobjc-arc -fblocks -triple x86_64-apple-darwin10.0.0 -fobjc-runtime-has-weak -emit-llvm -o - %s | FileCheck %s
 
 struct ObjCMember {
   id member;
@@ -12,6 +12,59 @@
   int (^bp)(int);
 };
 
+// CHECK: %[[STRUCT_CONTAINSWEAK:.*]] = type { %[[STRUCT_WEAK:.*]] }
+// CHECK: %[[STRUCT_WEAK]] = type { i8* }
+
+// The Weak object that is passed is destructed in this constructor.
+
+// CHECK: define void @_ZN12ContainsWeakC2E4Weak(
+// CHECK: call void @_ZN4WeakC1ERKS_(
+// CHECK: call void @_ZN4WeakD1Ev(
+
+// Check that the Weak object passed to this constructor is not destructed after
+// the delegate constructor is called.
+
+// CHECK: define void @_ZN12ContainsWeakC1E4Weak(
+// CHECK: call void @_ZN12ContainsWeakC2E4Weak(
+// CHECK-NEXT: ret void
+
+struct Weak {
+  Weak(id);
+  __weak id x;
+};
+
+struct ContainsWeak {
+  ContainsWeak(Weak);
+  Weak w;
+};
+
+ContainsWeak::ContainsWeak(Weak a) : w(a) {}
+
+// The Weak object that is passed is destructed in this constructor.
+
+// CHECK: define void @_ZN4BaseC2E4Weak(
+// CHECK: call void @_ZN4WeakD1Ev(
+// CHECK: ret void
+
+// Check that the Weak object passed to this constructor is not destructed after
+// the delegate constructor is called.
+
+// CHECK: define linkonce_odr void @_ZN7DerivedCI14BaseE4Weak(
+// CHECK: call void @_ZN7DerivedCI24BaseE4Weak(
+// CHECK-NEXT: ret void
+
+struct Base {
+  Base(Weak);
+};
+
+Base::Base(Weak a) {}
+
+struct Derived : Base {
+  using Base::Base;
+};
+
+Derived d(Weak(0));
+
 // CHECK-LABEL: define void @_Z42test_ObjCMember_default_construct_destructv(
 void test_ObjCMember_default_construct_destruct() {
   // CHECK: call void @_ZN10ObjCMemberC1Ev
@@ -111,6 +164,13 @@
 // CHECK-NEXT: call void @objc_release(i8* [[T7]])
 // CHECK-NEXT: ret
 
+// Check that the Weak object passed to this constructor is not destructed after
+// the delegate constructor is called.
+
+// CHECK: define linkonce_odr void @_ZN7DerivedCI24BaseE4Weak(
+// CHECK: call void @_ZN4BaseC2E4Weak(
+// CHE

[libclc] r330197 - exp10: Port from amd builtins

2018-04-17 Thread Jan Vesely via cfe-commits
Author: jvesely
Date: Tue Apr 17 11:08:08 2018
New Revision: 330197

URL: http://llvm.org/viewvc/llvm-project?rev=330197&view=rev
Log:
exp10: Port from amd builtins

Passes CTS on carrizo and turks.
Signed-off-by: Jan Vesely 
Reviewed and Tested (on RX 580) by: Aaron Watry 

Added:
libclc/trunk/generic/include/math/clc_exp10.h
libclc/trunk/generic/lib/math/clc_exp10.cl
Removed:
libclc/trunk/generic/lib/math/exp10.inc
Modified:
libclc/trunk/generic/lib/SOURCES
libclc/trunk/generic/lib/math/exp10.cl

Added: libclc/trunk/generic/include/math/clc_exp10.h
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/generic/include/math/clc_exp10.h?rev=330197&view=auto
==
--- libclc/trunk/generic/include/math/clc_exp10.h (added)
+++ libclc/trunk/generic/include/math/clc_exp10.h Tue Apr 17 11:08:08 2018
@@ -0,0 +1,4 @@
+#define __CLC_FUNCTION __clc_exp10
+#define __CLC_BODY 
+#include 
+#undef __CLC_FUNCTION

Modified: libclc/trunk/generic/lib/SOURCES
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/generic/lib/SOURCES?rev=330197&r1=330196&r2=330197&view=diff
==
--- libclc/trunk/generic/lib/SOURCES (original)
+++ libclc/trunk/generic/lib/SOURCES Tue Apr 17 11:08:08 2018
@@ -98,6 +98,7 @@ math/exp.cl
 math/exp_helper.cl
 math/expm1.cl
 math/exp2.cl
+math/clc_exp10.cl
 math/exp10.cl
 math/fdim.cl
 math/fmax.cl

Added: libclc/trunk/generic/lib/math/clc_exp10.cl
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/generic/lib/math/clc_exp10.cl?rev=330197&view=auto
==
--- libclc/trunk/generic/lib/math/clc_exp10.cl (added)
+++ libclc/trunk/generic/lib/math/clc_exp10.cl Tue Apr 17 11:08:08 2018
@@ -0,0 +1,149 @@
+/*
+ * Copyright (c) 2014 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include 
+
+#include "config.h"
+#include "math.h"
+#include "tables.h"
+#include "../clcmacro.h"
+
+//Algorithm:
+//
+//e^x = 2^(x/ln(2)) = 2^(x*(64/ln(2))/64)
+//
+//x*(64/ln(2)) = n + f, |f| <= 0.5, n is integer
+//n = 64*m + j,   0 <= j < 64
+//
+//e^x = 2^((64*m + j + f)/64)
+//= (2^m) * (2^(j/64)) * 2^(f/64)
+//= (2^m) * (2^(j/64)) * e^(f*(ln(2)/64))
+//
+//f = x*(64/ln(2)) - n
+//r = f*(ln(2)/64) = x - n*(ln(2)/64)
+//
+//e^x = (2^m) * (2^(j/64)) * e^r
+//
+//(2^(j/64)) is precomputed
+//
+//e^r = 1 + r + (r^2)/2! + (r^3)/3! + (r^4)/4! + (r^5)/5!
+//e^r = 1 + q
+//
+//q = r + (r^2)/2! + (r^3)/3! + (r^4)/4! + (r^5)/5!
+//
+//e^x = (2^m) * ( (2^(j/64)) + q*(2^(j/64)) )
+
+_CLC_DEF _CLC_OVERLOAD float __clc_exp10(float x)
+{
+const float X_MAX =  0x1.344134p+5f; // 128*log2/log10 : 38.53183944498959
+const float X_MIN = -0x1.66d3e8p+5f; // -149*log2/log10 : -44.8534693539332
+
+const float R_64_BY_LOG10_2 = 0x1.a934f0p+7f; // 64*log10/log2 : 
212.6033980727912
+const float R_LOG10_2_BY_64_LD = 0x1.34p-8f; // log2/(64 * log10) lead 
: 0.004699707
+const float R_LOG10_2_BY_64_TL = 0x1.04d426p-18f; // log2/(64 * log10) 
tail : 0.0388665057
+const float R_LN10 = 0x1.26bb1cp+1f;
+
+int return_nan = isnan(x);
+int return_inf = x > X_MAX;
+int return_zero = x < X_MIN;
+
+int n = convert_int(x * R_64_BY_LOG10_2);
+
+float fn = (float)n;
+int j = n & 0x3f;
+int m = n >> 6;
+int m2 = m << EXPSHIFTBITS_SP32;
+float r;
+
+r = R_LN10 * mad(fn, -R_LOG10_2_BY_64_TL, mad(fn, -R_LOG10_2_BY_64_LD, x));
+
+// Truncated Taylor series for e^r
+float z2 = mad(mad(mad(r, 0x1.56p-5f, 0x1.56p-3f), r, 
0x1.00p-1f), r*r, r);
+
+float two_to_jby64 = USE_TABLE(exp_tbl, j);
+z2 = mad(two_to_jby64, z2, two_to_jby64);
+
+float z2s = z2 * as_float(0x1 << (m + 149));
+float z2n = as_flo

[libclc] r330198 - amdgcn/fmin: Fix typos that reduced precision

2018-04-17 Thread Jan Vesely via cfe-commits
Author: jvesely
Date: Tue Apr 17 11:11:29 2018
New Revision: 330198

URL: http://llvm.org/viewvc/llvm-project?rev=330198&view=rev
Log:
amdgcn/fmin: Fix typos that reduced precision

Not sure how these sneaked in.
Fixes fminD and few other tests(fractD, cosD) on carrizo
Signed-off-by: Jan Vesely 
Reviewed-by: Aaron Watry 

Modified:
libclc/trunk/amdgcn/lib/math/fmin.cl

Modified: libclc/trunk/amdgcn/lib/math/fmin.cl
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgcn/lib/math/fmin.cl?rev=330198&r1=330197&r2=330198&view=diff
==
--- libclc/trunk/amdgcn/lib/math/fmin.cl (original)
+++ libclc/trunk/amdgcn/lib/math/fmin.cl Tue Apr 17 11:11:29 2018
@@ -19,9 +19,9 @@ _CLC_BINARY_VECTORIZE(_CLC_OVERLOAD _CLC
 
 _CLC_DEF _CLC_OVERLOAD double fmin(double x, double y)
 {
-   x = __builtin_canonicalizef(x);
-   y = __builtin_canonicalizef(y);
-   return __builtin_fminf(x, y);
+   x = __builtin_canonicalize(x);
+   y = __builtin_canonicalize(y);
+   return __builtin_fmin(x, y);
 }
 _CLC_BINARY_VECTORIZE(_CLC_OVERLOAD _CLC_DEF, double, fmin, double, double)
 


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


[PATCH] D45601: Warn on bool* to bool conversion

2018-04-17 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

The thing about the bool*-only version is that bool pointers are rare in C++, 
so I'm not sure we're gaining much.  But if we can't do something more general, 
there's still some benefit.

I see your point about false positives for the more general version. I was sort 
of considering an attribute to allow implicit conversions to bool for a 
specific function (like an assertion), and then we could ban implicit 
conversions to bool for parameters to other functions.  But that's probably 
overkill.


https://reviews.llvm.org/D45601



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


[PATCH] D45601: Warn on bool* to bool conversion

2018-04-17 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:2623
+QualType QT = Pointer->getType()->getPointeeType();
+if (!QT.isNull() && QT->isBooleanType())
+  // Warn on bool* to bool conversion.

Quuxplusone wrote:
> efriedma wrote:
> > Have you considered skipping the isBooleanType() check?  Passing any 
> > pointer to a function which expects a boolean parameter seems fundamentally 
> > suspicious.
> We already know that LLVM itself passes pointers to functions in many places, 
> and they're not bugs. I mean, this isn't the first time someone's proposed 
> that pointer->bool conversions are evil. :)
> 
> I know this just came up on a mailing list somewhere recently (rsmith was 
> involved in the rebuttal, as was I), but all I can find on Google is:
> https://groups.google.com/a/isocpp.org/d/msg/std-proposals/a3g9qXFVGCs/Wj40qj48H3wJ
> Anyway, the punch line I'm thinking of is something like
> ```
> FooBar *FB = ...;
> LLVMAssert(FB, "expected fb to be set");
> ```
> but with whatever the real names are. Found a bunch of those when I wrote the 
> diagnostic. And they're fairly solidly in the "not bugs" category.
> The thing about the bool*-only version is that bool pointers are rare in C++, 
> so I'm not sure we're gaining much. But if we can't do something more 
> general, there's still some benefit.
> I see your point about false positives for the more general version. I was 
> sort of considering an attribute to allow implicit conversions to bool for a 
> specific function (like an assertion), and then we could ban implicit 
> conversions to bool for parameters to other functions. But that's probably 
> overkill.

Based on @sberg's feedback that this warning found no true positives, and 
@Eugene.Zelenko's feedback that this warning already exists in clang-tidy 
today, personally I favor the resolution of "leave this warning in clang-tidy 
and abandon the attempt to move it into clang".


https://reviews.llvm.org/D45601



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


[PATCH] D45601: Warn on bool* to bool conversion

2018-04-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:2623
+QualType QT = Pointer->getType()->getPointeeType();
+if (!QT.isNull() && QT->isBooleanType())
+  // Warn on bool* to bool conversion.

Quuxplusone wrote:
> Quuxplusone wrote:
> > efriedma wrote:
> > > Have you considered skipping the isBooleanType() check?  Passing any 
> > > pointer to a function which expects a boolean parameter seems 
> > > fundamentally suspicious.
> > We already know that LLVM itself passes pointers to functions in many 
> > places, and they're not bugs. I mean, this isn't the first time someone's 
> > proposed that pointer->bool conversions are evil. :)
> > 
> > I know this just came up on a mailing list somewhere recently (rsmith was 
> > involved in the rebuttal, as was I), but all I can find on Google is:
> > https://groups.google.com/a/isocpp.org/d/msg/std-proposals/a3g9qXFVGCs/Wj40qj48H3wJ
> > Anyway, the punch line I'm thinking of is something like
> > ```
> > FooBar *FB = ...;
> > LLVMAssert(FB, "expected fb to be set");
> > ```
> > but with whatever the real names are. Found a bunch of those when I wrote 
> > the diagnostic. And they're fairly solidly in the "not bugs" category.
> > The thing about the bool*-only version is that bool pointers are rare in 
> > C++, so I'm not sure we're gaining much. But if we can't do something more 
> > general, there's still some benefit.
> > I see your point about false positives for the more general version. I was 
> > sort of considering an attribute to allow implicit conversions to bool for 
> > a specific function (like an assertion), and then we could ban implicit 
> > conversions to bool for parameters to other functions. But that's probably 
> > overkill.
> 
> Based on @sberg's feedback that this warning found no true positives, and 
> @Eugene.Zelenko's feedback that this warning already exists in clang-tidy 
> today, personally I favor the resolution of "leave this warning in clang-tidy 
> and abandon the attempt to move it into clang".
> ... personally I favor the resolution of "leave this warning in clang-tidy 
> and abandon the attempt to move it into clang".

This is my view as well.


https://reviews.llvm.org/D45601



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


[PATCH] D45670: [NEON] Define vfma_n_f32() and vfmaq_n_f32() intrinsics in AArch32 mode

2018-04-17 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

The corresponding LLVM tests seem be there already, so this looks all good to 
me.


https://reviews.llvm.org/D45670



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


r330199 - Add a command line option 'fregister_global_dtors_with_atexit' to

2018-04-17 Thread Akira Hatanaka via cfe-commits
Author: ahatanak
Date: Tue Apr 17 11:41:52 2018
New Revision: 330199

URL: http://llvm.org/viewvc/llvm-project?rev=330199&view=rev
Log:
Add a command line option 'fregister_global_dtors_with_atexit' to
register destructor functions annotated with __attribute__((destructor))
using __cxa_atexit or atexit.

Register destructor functions annotated with __attribute__((destructor))
calling __cxa_atexit in a synthesized constructor function instead of
emitting references to the functions in a special section.

The primary reason for adding this option is that we are planning to
deprecate the __mod_term_funcs section on Darwin in the future. This
feature is enabled by default only on Darwin. Users who do not want this
can use command line option 'fno_register_global_dtors_with_atexit' to
disable it.

rdar://problem/33887655

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

Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Frontend/CodeGenOptions.def
cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.h
cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/CodeGen/constructor-attribute.c
cfe/trunk/test/Driver/cxa-atexit.cpp
cfe/trunk/test/Driver/rewrite-legacy-objc.m
cfe/trunk/test/Driver/rewrite-objc.m

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=330199&r1=330198&r2=330199&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Tue Apr 17 11:41:52 2018
@@ -1364,6 +1364,8 @@ def fno_threadsafe_statics : Flag<["-"],
   Flags<[CC1Option]>, HelpText<"Do not emit code to make initialization of 
local statics thread safe">;
 def fno_use_cxa_atexit : Flag<["-"], "fno-use-cxa-atexit">, Group, 
Flags<[CC1Option]>,
   HelpText<"Don't use __cxa_atexit for calling destructors">;
+def fno_register_global_dtors_with_atexit : Flag<["-"], 
"fno-register-global-dtors-with-atexit">, Group,
+  HelpText<"Don't use atexit or __cxa_atexit to register global destructors">;
 def fno_use_init_array : Flag<["-"], "fno-use-init-array">, Group, 
Flags<[CC1Option]>,
   HelpText<"Don't use .init_array instead of .ctors">;
 def fno_unit_at_a_time : Flag<["-"], "fno-unit-at-a-time">, Group;
@@ -1612,6 +1614,8 @@ def funsigned_char : Flag<["-"], "funsig
 def fno_unsigned_char : Flag<["-"], "fno-unsigned-char">;
 def funwind_tables : Flag<["-"], "funwind-tables">, Group;
 def fuse_cxa_atexit : Flag<["-"], "fuse-cxa-atexit">, Group;
+def fregister_global_dtors_with_atexit : Flag<["-"], 
"fregister-global-dtors-with-atexit">, Group, Flags<[CC1Option]>,
+  HelpText<"Use atexit or __cxa_atexit to register global destructors">;
 def fuse_init_array : Flag<["-"], "fuse-init-array">, Group, 
Flags<[CC1Option]>,
   HelpText<"Use .init_array instead of .ctors">;
 def fno_var_tracking : Flag<["-"], "fno-var-tracking">, 
Group;

Modified: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CodeGenOptions.def?rev=330199&r1=330198&r2=330199&view=diff
==
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.def (original)
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def Tue Apr 17 11:41:52 2018
@@ -43,6 +43,7 @@ CODEGENOPT(CoverageExtraChecksum, 1, 0)
 CODEGENOPT(CoverageNoFunctionNamesInData, 1, 0) ///< Do not include function 
names in GCDA files.
 CODEGENOPT(CoverageExitBlockBeforeBody, 1, 0) ///< Whether to emit the exit 
block before the body blocks in GCNO files.
 CODEGENOPT(CXAAtExit , 1, 1) ///< Use __cxa_atexit for calling 
destructors.
+CODEGENOPT(RegisterGlobalDtorsWithAtExit, 1, 1) ///< Use atexit or 
__cxa_atexit to register global destructors.
 CODEGENOPT(CXXCtorDtorAliases, 1, 0) ///< Emit complete ctors/dtors as linker
  ///< aliases to base ctors when possible.
 CODEGENOPT(DataSections  , 1, 0) ///< Set when -fdata-sections is enabled.

Modified: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDeclCXX.cpp?rev=330199&r1=330198&r2=330199&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDeclCXX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDeclCXX.cpp Tue Apr 17 11:41:52 2018
@@ -236,7 +236,10 @@ void CodeGenFunction::registerGlobalDtor
llvm::Constant *addr) {
   // Create a function which calls the destructor.
   llvm::Constant *dtorStub = createAtExitStub(VD, dtor, addr);
+  regist

[PATCH] D45578: Add a command line option `fregister_global_dtors_with_atexit` to register destructor functions annotated with __attribute__((destructor)) using __cxa_atexit or atexit.

2018-04-17 Thread Akira Hatanaka via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL330199: Add a command line option 
'fregister_global_dtors_with_atexit' to (authored by ahatanak, 
committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D45578?vs=142597&id=142810#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45578

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/include/clang/Frontend/CodeGenOptions.def
  cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.h
  cfe/trunk/lib/CodeGen/CodeGenModule.cpp
  cfe/trunk/lib/CodeGen/CodeGenModule.h
  cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/CodeGen/constructor-attribute.c
  cfe/trunk/test/Driver/cxa-atexit.cpp
  cfe/trunk/test/Driver/rewrite-legacy-objc.m
  cfe/trunk/test/Driver/rewrite-objc.m

Index: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
===
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.def
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def
@@ -43,6 +43,7 @@
 CODEGENOPT(CoverageNoFunctionNamesInData, 1, 0) ///< Do not include function names in GCDA files.
 CODEGENOPT(CoverageExitBlockBeforeBody, 1, 0) ///< Whether to emit the exit block before the body blocks in GCNO files.
 CODEGENOPT(CXAAtExit , 1, 1) ///< Use __cxa_atexit for calling destructors.
+CODEGENOPT(RegisterGlobalDtorsWithAtExit, 1, 1) ///< Use atexit or __cxa_atexit to register global destructors.
 CODEGENOPT(CXXCtorDtorAliases, 1, 0) ///< Emit complete ctors/dtors as linker
  ///< aliases to base ctors when possible.
 CODEGENOPT(DataSections  , 1, 0) ///< Set when -fdata-sections is enabled.
Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1364,6 +1364,8 @@
   Flags<[CC1Option]>, HelpText<"Do not emit code to make initialization of local statics thread safe">;
 def fno_use_cxa_atexit : Flag<["-"], "fno-use-cxa-atexit">, Group, Flags<[CC1Option]>,
   HelpText<"Don't use __cxa_atexit for calling destructors">;
+def fno_register_global_dtors_with_atexit : Flag<["-"], "fno-register-global-dtors-with-atexit">, Group,
+  HelpText<"Don't use atexit or __cxa_atexit to register global destructors">;
 def fno_use_init_array : Flag<["-"], "fno-use-init-array">, Group, Flags<[CC1Option]>,
   HelpText<"Don't use .init_array instead of .ctors">;
 def fno_unit_at_a_time : Flag<["-"], "fno-unit-at-a-time">, Group;
@@ -1612,6 +1614,8 @@
 def fno_unsigned_char : Flag<["-"], "fno-unsigned-char">;
 def funwind_tables : Flag<["-"], "funwind-tables">, Group;
 def fuse_cxa_atexit : Flag<["-"], "fuse-cxa-atexit">, Group;
+def fregister_global_dtors_with_atexit : Flag<["-"], "fregister-global-dtors-with-atexit">, Group, Flags<[CC1Option]>,
+  HelpText<"Use atexit or __cxa_atexit to register global destructors">;
 def fuse_init_array : Flag<["-"], "fuse-init-array">, Group, Flags<[CC1Option]>,
   HelpText<"Use .init_array instead of .ctors">;
 def fno_var_tracking : Flag<["-"], "fno-var-tracking">, Group;
Index: cfe/trunk/test/CodeGen/constructor-attribute.c
===
--- cfe/trunk/test/CodeGen/constructor-attribute.c
+++ cfe/trunk/test/CodeGen/constructor-attribute.c
@@ -1,8 +1,39 @@
-// RUN: %clang_cc1 -emit-llvm -o %t %s
-// RUN: grep -e "global_ctors.*@A" %t
-// RUN: grep -e "global_dtors.*@B" %t
-// RUN: grep -e "global_ctors.*@C" %t
-// RUN: grep -e "global_dtors.*@D" %t
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=WITHOUTATEXIT %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fregister-global-dtors-with-atexit -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=CXAATEXIT --check-prefix=WITHATEXIT %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fno-use-cxa-atexit -fregister-global-dtors-with-atexit -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=ATEXIT --check-prefix=WITHATEXIT %s
+
+// WITHOUTATEXIT: global_ctors{{.*}}@A{{.*}}@C
+// WITHOUTATEXIT: @llvm.global_dtors = appending global [5 x { i32, void ()*, i8* }]{{.*}}@B{{.*}}@E{{.*}}@F{{.*}}@G{{.*}}@D
+// WITHATEXIT: @llvm.global_ctors = appending global [5 x { i32, void ()*, i8* }]{{.*}}i32 65535, void ()* @A,{{.*}}i32 65535, void ()* @C,{{.*}}i32 123, void ()* @__GLOBAL_init_123,{{.*}}i32 789, void ()* @[[GLOBAL_INIT_789:__GLOBAL_init_789.[0-9]+]],{{.*}}i32 65535, void ()* @__GLOBAL_init_65535,
+// WITHATEXIT-NOT: global_dtors
+
+// CHECK: define void @A()
+// CHECK: define void @B()
+// CHECK: define internal void @E()
+// CHECK: define internal void @F()
+// CHECK: define internal void @G

[PATCH] D45702: [clang-tidy] Add a new check, readability-redundant-data-call, that finds and removes redundant calls to .data().

2018-04-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

Have you run this over any large code bases to see whether the check triggers 
in practice?




Comment at: docs/clang-tidy/checks/list.rst:95
fuchsia-default-arguments
+   fuchsia-header-anon-namespaces (redirects to google-build-namespaces) 

fuchsia-multiple-inheritance

This change looks unrelated to the patch and should be handled separately.



Comment at: docs/clang-tidy/checks/readability-redundant-data-call.rst:6
+
+This check suggests removing redundant `.data()` calls.
+

What does it mean for a `data()` call to be redundant? I think the 
documentation needs to explain that a bit more.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45702



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


[PATCH] D45662: OpenBSD add C++ runtime in a driver's standpoint

2018-04-17 Thread David CARLIER via Phabricator via cfe-commits
devnexen updated this revision to Diff 142811.
devnexen retitled this revision from "Fuzzer, add libcxx for OpenBSD" to 
"OpenBSD add C++ runtime in a driver's standpoint".
devnexen edited the summary of this revision.

https://reviews.llvm.org/D45662

Files:
  lib/Driver/ToolChains/OpenBSD.cpp
  lib/Driver/ToolChains/OpenBSD.h


Index: lib/Driver/ToolChains/OpenBSD.h
===
--- lib/Driver/ToolChains/OpenBSD.h
+++ lib/Driver/ToolChains/OpenBSD.h
@@ -58,6 +58,8 @@
   bool IsMathErrnoDefault() const override { return false; }
   bool IsObjCNonFragileABIDefault() const override { return true; }
   bool isPIEDefault() const override { return true; }
+  void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
+   llvm::opt::ArgStringList &CmdArgs) const override;
 
   unsigned GetDefaultStackProtectorLevel(bool KernelOrKext) const override {
 return 2;
Index: lib/Driver/ToolChains/OpenBSD.cpp
===
--- lib/Driver/ToolChains/OpenBSD.cpp
+++ lib/Driver/ToolChains/OpenBSD.cpp
@@ -259,6 +259,14 @@
   getFilePaths().push_back("/usr/lib");
 }
 
+void OpenBSD::AddCXXStdlibLibArgs(const ArgList &Args,
+  ArgStringList &CmdArgs) const {
+  bool Profiling = Args.hasArg(options::OPT_pg);
+
+  CmdArgs.push_back(Profiling ? "-lc++_p" : "-lc++");
+  CmdArgs.push_back(Profiling ? "-lc++abi_p" : "-lc++abi");
+}
+
 Tool *OpenBSD::buildAssembler() const {
   return new tools::openbsd::Assembler(*this);
 }


Index: lib/Driver/ToolChains/OpenBSD.h
===
--- lib/Driver/ToolChains/OpenBSD.h
+++ lib/Driver/ToolChains/OpenBSD.h
@@ -58,6 +58,8 @@
   bool IsMathErrnoDefault() const override { return false; }
   bool IsObjCNonFragileABIDefault() const override { return true; }
   bool isPIEDefault() const override { return true; }
+  void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
+   llvm::opt::ArgStringList &CmdArgs) const override;
 
   unsigned GetDefaultStackProtectorLevel(bool KernelOrKext) const override {
 return 2;
Index: lib/Driver/ToolChains/OpenBSD.cpp
===
--- lib/Driver/ToolChains/OpenBSD.cpp
+++ lib/Driver/ToolChains/OpenBSD.cpp
@@ -259,6 +259,14 @@
   getFilePaths().push_back("/usr/lib");
 }
 
+void OpenBSD::AddCXXStdlibLibArgs(const ArgList &Args,
+  ArgStringList &CmdArgs) const {
+  bool Profiling = Args.hasArg(options::OPT_pg);
+
+  CmdArgs.push_back(Profiling ? "-lc++_p" : "-lc++");
+  CmdArgs.push_back(Profiling ? "-lc++abi_p" : "-lc++abi");
+}
+
 Tool *OpenBSD::buildAssembler() const {
   return new tools::openbsd::Assembler(*this);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45662: OpenBSD add C++ runtime in a driver's standpoint

2018-04-17 Thread David CARLIER via Phabricator via cfe-commits
devnexen added inline comments.



Comment at: lib/Driver/ToolChains/OpenBSD.cpp:189
   if (getToolChain().ShouldLinkCXXStdlib(Args))
-getToolChain().AddCXXStdlibLibArgs(Args, CmdArgs);
+ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
   if (Args.hasArg(options::OPT_pg))

devnexen wrote:
> dberris wrote:
> > devnexen wrote:
> > > dberris wrote:
> > > > devnexen wrote:
> > > > > dberris wrote:
> > > > > > Do you actually need this change? Why isn't 
> > > > > > `getToolChain().AddCXXStdlibLibArgs(...)` not sufficient here?
> > > > > That s the thing, I wish it was simple as FreeBSD, but seemingly in 
> > > > > OpenBSD needs both c++98 gcc runtime and libc++ for fuzzer (I tried 
> > > > > libc++ alone already)
> > > > Right, but this comment is on this specific line change. I don't think 
> > > > you need to reach into `Toolchain.` direcly, since you can already use 
> > > > `getToolChain()` just from the above line (188).
> > > Right, so I guess this diff https://reviews.llvm.org/D45662?id=142686 is 
> > > sufficient then ?
> > No. Let me try and explain again.
> > 
> > You were on the right path, with overriding the `AddCXXStdlibLibArgs` 
> > function in the OpenBSD Toolchain type. It's just that you weren't handling 
> > the case for when the binary was being built with libc++ or libstdc++ 
> > properly. I was referring you to what FreeBSD was doing for their 
> > implementation of `AddCXXStdlibLibArgs`. This means, checking first whether 
> > the invocation of the compiler was using libc++ or libstdc++, and then 
> > adding the appropriate link spelling. That all happens in the 
> > `AddCXXStdlibLibArgs` implementation, because there's no need to 
> > special-case just for the sanitizers.
> > 
> > This means, if you're building a normal binary with `-pg` in OpenBSD 
> > against either libc++ or libstdc++, it wouldn't work correctly regardless 
> > of whether you were using libFuzzer.
> > 
> > Does that make more sense?
> Ok will try a newer version later, thanks for your inputs.
So I looked at FreeBSD and makes more sense by default it s libcxx since the 
10.x releases. A release has a few years span of support. OpenBSD has complete 
different release policy, much shorter and two releases per year. clang been in 
since 6.2 (and we re at 6.3 now and the time llvm 7 comes out it will be 6.4).


https://reviews.llvm.org/D45662



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


[PATCH] D45662: OpenBSD add C++ runtime in a driver's standpoint

2018-04-17 Thread David CARLIER via Phabricator via cfe-commits
devnexen added inline comments.



Comment at: lib/Driver/ToolChains/OpenBSD.cpp:189
   if (getToolChain().ShouldLinkCXXStdlib(Args))
-getToolChain().AddCXXStdlibLibArgs(Args, CmdArgs);
+ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
   if (Args.hasArg(options::OPT_pg))

devnexen wrote:
> devnexen wrote:
> > dberris wrote:
> > > devnexen wrote:
> > > > dberris wrote:
> > > > > devnexen wrote:
> > > > > > dberris wrote:
> > > > > > > Do you actually need this change? Why isn't 
> > > > > > > `getToolChain().AddCXXStdlibLibArgs(...)` not sufficient here?
> > > > > > That s the thing, I wish it was simple as FreeBSD, but seemingly in 
> > > > > > OpenBSD needs both c++98 gcc runtime and libc++ for fuzzer (I tried 
> > > > > > libc++ alone already)
> > > > > Right, but this comment is on this specific line change. I don't 
> > > > > think you need to reach into `Toolchain.` direcly, since you can 
> > > > > already use `getToolChain()` just from the above line (188).
> > > > Right, so I guess this diff https://reviews.llvm.org/D45662?id=142686 
> > > > is sufficient then ?
> > > No. Let me try and explain again.
> > > 
> > > You were on the right path, with overriding the `AddCXXStdlibLibArgs` 
> > > function in the OpenBSD Toolchain type. It's just that you weren't 
> > > handling the case for when the binary was being built with libc++ or 
> > > libstdc++ properly. I was referring you to what FreeBSD was doing for 
> > > their implementation of `AddCXXStdlibLibArgs`. This means, checking first 
> > > whether the invocation of the compiler was using libc++ or libstdc++, and 
> > > then adding the appropriate link spelling. That all happens in the 
> > > `AddCXXStdlibLibArgs` implementation, because there's no need to 
> > > special-case just for the sanitizers.
> > > 
> > > This means, if you're building a normal binary with `-pg` in OpenBSD 
> > > against either libc++ or libstdc++, it wouldn't work correctly regardless 
> > > of whether you were using libFuzzer.
> > > 
> > > Does that make more sense?
> > Ok will try a newer version later, thanks for your inputs.
> So I looked at FreeBSD and makes more sense by default it s libcxx since the 
> 10.x releases. A release has a few years span of support. OpenBSD has 
> complete different release policy, much shorter and two releases per year. 
> clang been in since 6.2 (and we re at 6.3 now and the time llvm 7 comes out 
> it will be 6.4).
To complete a bit above explanation, softwares of certain version matches the 
OS version. In other words there won t be llvm 7 for OpenBSD 6.1


https://reviews.llvm.org/D45662



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


r330201 - Move the visitor classes that are used to traverse non-trivial C structs

2018-04-17 Thread Akira Hatanaka via cfe-commits
Author: ahatanak
Date: Tue Apr 17 12:05:17 2018
New Revision: 330201

URL: http://llvm.org/viewvc/llvm-project?rev=330201&view=rev
Log:
Move the visitor classes that are used to traverse non-trivial C structs
to a header file.

This is in preparation for using the visitor classes to warn about
memcpy'ing non-trivial C structs.

See the discussion here:
https://reviews.llvm.org/D45310

rdar://problem/36124208

Added:
cfe/trunk/include/clang/AST/NonTrivialTypeVisitor.h
Modified:
cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp

Added: cfe/trunk/include/clang/AST/NonTrivialTypeVisitor.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/NonTrivialTypeVisitor.h?rev=330201&view=auto
==
--- cfe/trunk/include/clang/AST/NonTrivialTypeVisitor.h (added)
+++ cfe/trunk/include/clang/AST/NonTrivialTypeVisitor.h Tue Apr 17 12:05:17 2018
@@ -0,0 +1,113 @@
+//===-- NonTrivialTypeVisitor.h - Visitor for non-trivial Types *- C++ 
--*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+//  This file defines the visitor classes that are used to traverse non-trivial
+//  structs.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_NON_TRIVIAL_TYPE_VISITOR_H
+#define LLVM_CLANG_NON_TRIVIAL_TYPE_VISITOR_H
+
+#include "clang/AST/Type.h"
+
+namespace clang {
+
+template  struct DestructedTypeVisitor {
+  template  RetTy visit(QualType FT, Ts &&... Args) {
+return asDerived().visitWithKind(FT.isDestructedType(), FT,
+ std::forward(Args)...);
+  }
+
+  template 
+  RetTy visitWithKind(QualType::DestructionKind DK, QualType FT,
+  Ts &&... Args) {
+switch (DK) {
+case QualType::DK_objc_strong_lifetime:
+  return asDerived().visitARCStrong(FT, std::forward(Args)...);
+case QualType::DK_nontrivial_c_struct:
+  return asDerived().visitStruct(FT, std::forward(Args)...);
+case QualType::DK_none:
+  return asDerived().visitTrivial(FT, std::forward(Args)...);
+case QualType::DK_cxx_destructor:
+  return asDerived().visitCXXDestructor(FT, std::forward(Args)...);
+case QualType::DK_objc_weak_lifetime:
+  return asDerived().visitARCWeak(FT, std::forward(Args)...);
+}
+
+llvm_unreachable("unknown destruction kind");
+  }
+
+  Derived &asDerived() { return static_cast(*this); }
+};
+
+template 
+struct DefaultInitializedTypeVisitor {
+  template  RetTy visit(QualType FT, Ts &&... Args) {
+return asDerived().visitWithKind(
+FT.isNonTrivialToPrimitiveDefaultInitialize(), FT,
+std::forward(Args)...);
+  }
+
+  template 
+  RetTy visitWithKind(QualType::PrimitiveDefaultInitializeKind PDIK,
+  QualType FT, Ts &&... Args) {
+switch (PDIK) {
+case QualType::PDIK_ARCStrong:
+  return asDerived().visitARCStrong(FT, std::forward(Args)...);
+case QualType::PDIK_ARCWeak:
+  return asDerived().visitARCWeak(FT, std::forward(Args)...);
+case QualType::PDIK_Struct:
+  return asDerived().visitStruct(FT, std::forward(Args)...);
+case QualType::PDIK_Trivial:
+  return asDerived().visitTrivial(FT, std::forward(Args)...);
+}
+
+llvm_unreachable("unknown default-initialize kind");
+  }
+
+  Derived &asDerived() { return static_cast(*this); }
+};
+
+template 
+struct CopiedTypeVisitor {
+  template  RetTy visit(QualType FT, Ts &&... Args) {
+QualType::PrimitiveCopyKind PCK =
+IsMove ? FT.isNonTrivialToPrimitiveDestructiveMove()
+   : FT.isNonTrivialToPrimitiveCopy();
+return asDerived().visitWithKind(PCK, FT, std::forward(Args)...);
+  }
+
+  template 
+  RetTy visitWithKind(QualType::PrimitiveCopyKind PCK, QualType FT,
+  Ts &&... Args) {
+asDerived().preVisit(PCK, FT, std::forward(Args)...);
+
+switch (PCK) {
+case QualType::PCK_ARCStrong:
+  return asDerived().visitARCStrong(FT, std::forward(Args)...);
+case QualType::PCK_ARCWeak:
+  return asDerived().visitARCWeak(FT, std::forward(Args)...);
+case QualType::PCK_Struct:
+  return asDerived().visitStruct(FT, std::forward(Args)...);
+case QualType::PCK_Trivial:
+  return asDerived().visitTrivial(FT, std::forward(Args)...);
+case QualType::PCK_VolatileTrivial:
+  return asDerived().visitVolatileTrivial(FT, std::forward(Args)...);
+}
+
+llvm_unreachable("unknown primitive copy kind");
+  }
+
+  Derived &asDerived() { return static_cast(*this); }
+};
+
+} // end namespace clang
+
+#endif

Modified: cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp?rev=330201&r1=330200&r2=

r330202 - [Sema] Warn about memcpy'ing non-trivial C structs.

2018-04-17 Thread Akira Hatanaka via cfe-commits
Author: ahatanak
Date: Tue Apr 17 12:13:41 2018
New Revision: 330202

URL: http://llvm.org/viewvc/llvm-project?rev=330202&view=rev
Log:
[Sema] Warn about memcpy'ing non-trivial C structs.

Issue a warning when non-trivial C structs are copied or initialized by
calls to memset, bzero, memcpy, or memmove.

rdar://problem/36124208

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

Added:
cfe/trunk/test/SemaObjC/warn-nontrivial-struct-memaccess.m
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaChecking.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=330202&r1=330201&r2=330202&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Apr 17 12:13:41 
2018
@@ -613,6 +613,13 @@ def err_function_needs_feature
 "'%2'">;
 def warn_builtin_unknown : Warning<"use of unknown builtin %0">,
   InGroup, DefaultError;
+def warn_cstruct_memaccess : Warning<
+  "%select{destination for|source of|first operand of|second operand of}0 this 
"
+  "%1 call is a pointer to record %2 that is not trivial to "
+  "%select{primitive-default-initialize|primitive-copy}3">,
+  InGroup>;
+def note_nontrivial_field : Note<
+  "field is non-trivial to %select{copy|default-initialize}0">;
 def warn_dyn_class_memaccess : Warning<
   "%select{destination for|source of|first operand of|second operand of}0 this 
"
   "%1 call is a pointer to %select{|class containing a }2dynamic class %3; "

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=330202&r1=330201&r2=330202&view=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue Apr 17 12:13:41 2018
@@ -28,6 +28,7 @@
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/ExprOpenMP.h"
 #include "clang/AST/NSAPI.h"
+#include "clang/AST/NonTrivialTypeVisitor.h"
 #include "clang/AST/OperationKinds.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/TemplateBase.h"
@@ -7378,6 +7379,98 @@ static QualType getSizeOfArgType(const E
   return QualType();
 }
 
+namespace {
+
+struct SearchNonTrivialToInitializeField
+: DefaultInitializedTypeVisitor {
+  using Super =
+  DefaultInitializedTypeVisitor;
+
+  SearchNonTrivialToInitializeField(const Expr *E, Sema &S) : E(E), S(S) {}
+
+  void visitWithKind(QualType::PrimitiveDefaultInitializeKind PDIK, QualType 
FT,
+ SourceLocation SL) {
+if (const auto *AT = asDerived().getContext().getAsArrayType(FT)) {
+  asDerived().visitArray(PDIK, AT, SL);
+  return;
+}
+
+Super::visitWithKind(PDIK, FT, SL);
+  }
+
+  void visitARCStrong(QualType FT, SourceLocation SL) {
+S.DiagRuntimeBehavior(SL, E, S.PDiag(diag::note_nontrivial_field) << 1);
+  }
+  void visitARCWeak(QualType FT, SourceLocation SL) {
+S.DiagRuntimeBehavior(SL, E, S.PDiag(diag::note_nontrivial_field) << 1);
+  }
+  void visitStruct(QualType FT, SourceLocation SL) {
+for (const FieldDecl *FD : FT->castAs()->getDecl()->fields())
+  visit(FD->getType(), FD->getLocation());
+  }
+  void visitArray(QualType::PrimitiveDefaultInitializeKind PDIK,
+  const ArrayType *AT, SourceLocation SL) {
+visit(getContext().getBaseElementType(AT), SL);
+  }
+  void visitTrivial(QualType FT, SourceLocation SL) {}
+
+  static void diag(QualType RT, const Expr *E, Sema &S) {
+SearchNonTrivialToInitializeField(E, S).visitStruct(RT, SourceLocation());
+  }
+
+  ASTContext &getContext() { return S.getASTContext(); }
+
+  const Expr *E;
+  Sema &S;
+};
+
+struct SearchNonTrivialToCopyField
+: CopiedTypeVisitor {
+  using Super = CopiedTypeVisitor;
+
+  SearchNonTrivialToCopyField(const Expr *E, Sema &S) : E(E), S(S) {}
+
+  void visitWithKind(QualType::PrimitiveCopyKind PCK, QualType FT,
+ SourceLocation SL) {
+if (const auto *AT = asDerived().getContext().getAsArrayType(FT)) {
+  asDerived().visitArray(PCK, AT, SL);
+  return;
+}
+
+Super::visitWithKind(PCK, FT, SL);
+  }
+
+  void visitARCStrong(QualType FT, SourceLocation SL) {
+S.DiagRuntimeBehavior(SL, E, S.PDiag(diag::note_nontrivial_field) << 0);
+  }
+  void visitARCWeak(QualType FT, SourceLocation SL) {
+S.DiagRuntimeBehavior(SL, E, S.PDiag(diag::note_nontrivial_field) << 0);
+  }
+  void visitStruct(QualType FT, SourceLocation SL) {
+for (const FieldDecl *FD : FT->castAs()->getDecl()->fields())
+  visit(FD->getType(), FD->getLocation());
+  }
+  void visitArray(QualType::PrimitiveCopyKind PCK, const ArrayType *AT,
+  SourceLocation SL) {
+visit(getContext().getBaseEleme

[PATCH] D45310: Warn about memcpy'ing non-trivial C structs

2018-04-17 Thread Akira Hatanaka via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC330202: [Sema] Warn about memcpy'ing non-trivial C 
structs. (authored by ahatanak, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D45310?vs=142705&id=142815#toc

Repository:
  rC Clang

https://reviews.llvm.org/D45310

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/SemaObjC/warn-nontrivial-struct-memaccess.m

Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -613,6 +613,13 @@
 "'%2'">;
 def warn_builtin_unknown : Warning<"use of unknown builtin %0">,
   InGroup, DefaultError;
+def warn_cstruct_memaccess : Warning<
+  "%select{destination for|source of|first operand of|second operand of}0 this "
+  "%1 call is a pointer to record %2 that is not trivial to "
+  "%select{primitive-default-initialize|primitive-copy}3">,
+  InGroup>;
+def note_nontrivial_field : Note<
+  "field is non-trivial to %select{copy|default-initialize}0">;
 def warn_dyn_class_memaccess : Warning<
   "%select{destination for|source of|first operand of|second operand of}0 this "
   "%1 call is a pointer to %select{|class containing a }2dynamic class %3; "
Index: test/SemaObjC/warn-nontrivial-struct-memaccess.m
===
--- test/SemaObjC/warn-nontrivial-struct-memaccess.m
+++ test/SemaObjC/warn-nontrivial-struct-memaccess.m
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fobjc-runtime-has-weak -x objective-c -fobjc-arc -verify %s
+
+void *memset(void *, int, __SIZE_TYPE__);
+void bzero(void *, __SIZE_TYPE__);
+void *memcpy(void *, const void *, __SIZE_TYPE__);
+void *memmove(void *, const void *, __SIZE_TYPE__);
+
+struct Trivial {
+  int f0;
+  volatile int f1;
+};
+
+struct NonTrivial0 {
+  int f0;
+  __weak id f1; // expected-note 2 {{non-trivial to default-initialize}} expected-note 2 {{non-trivial to copy}}
+  volatile int f2;
+  id f3[10]; // expected-note 2 {{non-trivial to default-initialize}} expected-note 2 {{non-trivial to copy}}
+};
+
+struct NonTrivial1 {
+  id f0; // expected-note 2 {{non-trivial to default-initialize}} expected-note 2 {{non-trivial to copy}}
+  int f1;
+  struct NonTrivial0 f2;
+};
+
+void testTrivial(struct Trivial *d, struct Trivial *s) {
+  memset(d, 0, sizeof(struct Trivial));
+  bzero(d, sizeof(struct Trivial));
+  memcpy(d, s, sizeof(struct Trivial));
+  memmove(d, s, sizeof(struct Trivial));
+}
+
+void testNonTrivial1(struct NonTrivial1 *d, struct NonTrivial1 *s) {
+  memset(d, 0, sizeof(struct NonTrivial1)); // expected-warning {{that is not trivial to primitive-default-initialize}} expected-note {{explicitly cast the pointer to silence}}
+  memset((void *)d, 0, sizeof(struct NonTrivial1));
+  bzero(d, sizeof(struct NonTrivial1)); // expected-warning {{that is not trivial to primitive-default-initialize}} expected-note {{explicitly cast the pointer to silence}}
+  memcpy(d, s, sizeof(struct NonTrivial1)); // expected-warning {{that is not trivial to primitive-copy}} expected-note {{explicitly cast the pointer to silence}}
+  memmove(d, s, sizeof(struct NonTrivial1)); // expected-warning {{that is not trivial to primitive-copy}} expected-note {{explicitly cast the pointer to silence}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -28,6 +28,7 @@
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/ExprOpenMP.h"
 #include "clang/AST/NSAPI.h"
+#include "clang/AST/NonTrivialTypeVisitor.h"
 #include "clang/AST/OperationKinds.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/TemplateBase.h"
@@ -7378,6 +7379,98 @@
   return QualType();
 }
 
+namespace {
+
+struct SearchNonTrivialToInitializeField
+: DefaultInitializedTypeVisitor {
+  using Super =
+  DefaultInitializedTypeVisitor;
+
+  SearchNonTrivialToInitializeField(const Expr *E, Sema &S) : E(E), S(S) {}
+
+  void visitWithKind(QualType::PrimitiveDefaultInitializeKind PDIK, QualType FT,
+ SourceLocation SL) {
+if (const auto *AT = asDerived().getContext().getAsArrayType(FT)) {
+  asDerived().visitArray(PDIK, AT, SL);
+  return;
+}
+
+Super::visitWithKind(PDIK, FT, SL);
+  }
+
+  void visitARCStrong(QualType FT, SourceLocation SL) {
+S.DiagRuntimeBehavior(SL, E, S.PDiag(diag::note_nontrivial_field) << 1);
+  }
+  void visitARCWeak(QualType FT, SourceLocation SL) {
+S.DiagRuntimeBehavior(SL, E, S.PDiag(diag::note_nontrivial_field) << 1);
+  }
+  void visitStruct(QualType FT, SourceLocation SL) {
+for (const FieldDecl *FD : FT->castAs()->getDecl()->fields())
+  visit(FD->getType(), FD->getLocation());
+  }
+  void visitArray(QualType::PrimitiveDefaultInitializeKind P

[libclc] r330207 - powr: Use denormal path only

2018-04-17 Thread Jan Vesely via cfe-commits
Author: jvesely
Date: Tue Apr 17 12:35:32 2018
New Revision: 330207

URL: http://llvm.org/viewvc/llvm-project?rev=330207&view=rev
Log:
powr: Use denormal path only

It's OK to either flush to 0 or return denormal result if the device
does not support denormals. See sec 7.2 and 7.5.3 of OCL specs
Fixes CTS on carrizo and turks.
Signed-off-by: Jan Vesely 
Reviewer: Aaron Watry 

Modified:
libclc/trunk/generic/lib/math/clc_powr.cl

Modified: libclc/trunk/generic/lib/math/clc_powr.cl
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/generic/lib/math/clc_powr.cl?rev=330207&r1=330206&r2=330207&view=diff
==
--- libclc/trunk/generic/lib/math/clc_powr.cl (original)
+++ libclc/trunk/generic/lib/math/clc_powr.cl Tue Apr 17 12:35:32 2018
@@ -165,17 +165,7 @@ _CLC_DEF _CLC_OVERLOAD float __clc_powr(
 tv = USE_TABLE(exp_tbl_ep, j);
 
 float expylogx = mad(tv.s0, poly, mad(tv.s1, poly, tv.s1)) + tv.s0;
-float sexpylogx;
-if (!__clc_fp32_subnormals_supported()) {
-   int explg = ((as_uint(expylogx) & EXPBITS_SP32 >> 23) - 127);
-   m = (23-(m + 149)) == 0 ? 1: m;
-   uint mantissa =  ((as_uint(expylogx) & 
MANTBITS_SP32)|IMPBIT_SP32) >> (23-(m + 149));
-   sexpylogx = as_float(mantissa);
-} else {
-   sexpylogx = expylogx * as_float(0x1 << (m + 149));
-}
-
-
+float sexpylogx = expylogx * as_float(0x1 << (m + 149));
 float texpylogx = as_float(as_int(expylogx) + m2);
 expylogx = m < -125 ? sexpylogx : texpylogx;
 


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


[libclc] r330206 - pown: Use denormal path only

2018-04-17 Thread Jan Vesely via cfe-commits
Author: jvesely
Date: Tue Apr 17 12:35:30 2018
New Revision: 330206

URL: http://llvm.org/viewvc/llvm-project?rev=330206&view=rev
Log:
pown: Use denormal path only

It's OK to either flush to 0 or return denormal result if the device
does not support denormals. See sec 7.2 and 7.5.3 of OCL specs
Fixes CTS on carrizo and turks.
Signed-off-by: Jan Vesely 
Reviewer: Aaron Watry 

Modified:
libclc/trunk/generic/lib/math/clc_pown.cl

Modified: libclc/trunk/generic/lib/math/clc_pown.cl
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/generic/lib/math/clc_pown.cl?rev=330206&r1=330205&r2=330206&view=diff
==
--- libclc/trunk/generic/lib/math/clc_pown.cl (original)
+++ libclc/trunk/generic/lib/math/clc_pown.cl Tue Apr 17 12:35:30 2018
@@ -167,17 +167,7 @@ _CLC_DEF _CLC_OVERLOAD float __clc_pown(
 tv = USE_TABLE(exp_tbl_ep, j);
 
 float expylogx = mad(tv.s0, poly, mad(tv.s1, poly, tv.s1)) + tv.s0;
-float sexpylogx;
-if (!__clc_fp32_subnormals_supported()) {
-   int explg = ((as_uint(expylogx) & EXPBITS_SP32 >> 23) - 127);
-   m = (23-(m + 149)) == 0 ? 1: m;
-   uint mantissa =  ((as_uint(expylogx) & 
MANTBITS_SP32)|IMPBIT_SP32) >> (23-(m + 149));
-   sexpylogx = as_float(mantissa);
-} else {
-   sexpylogx = expylogx * as_float(0x1 << (m + 149));
-}
-
-
+float sexpylogx = expylogx * as_float(0x1 << (m + 149));
 float texpylogx = as_float(as_int(expylogx) + m2);
 expylogx = m < -125 ? sexpylogx : texpylogx;
 


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


[libclc] r330205 - pow: Use denormal path only

2018-04-17 Thread Jan Vesely via cfe-commits
Author: jvesely
Date: Tue Apr 17 12:35:28 2018
New Revision: 330205

URL: http://llvm.org/viewvc/llvm-project?rev=330205&view=rev
Log:
pow: Use denormal path only

It's OK to either flush to 0 or return denormal result if the device
does not support denormals. See sec 7.2 and 7.5.3 of OCL specs
Fixes CTS on carrizo and turks.
Signed-off-by: Jan Vesely 
Reviewer: Aaron Watry 

Modified:
libclc/trunk/generic/lib/math/clc_pow.cl

Modified: libclc/trunk/generic/lib/math/clc_pow.cl
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/generic/lib/math/clc_pow.cl?rev=330205&r1=330204&r2=330205&view=diff
==
--- libclc/trunk/generic/lib/math/clc_pow.cl (original)
+++ libclc/trunk/generic/lib/math/clc_pow.cl Tue Apr 17 12:35:28 2018
@@ -169,17 +169,7 @@ _CLC_DEF _CLC_OVERLOAD float __clc_pow(f
 tv = USE_TABLE(exp_tbl_ep, j);
 
 float expylogx = mad(tv.s0, poly, mad(tv.s1, poly, tv.s1)) + tv.s0;
-float sexpylogx;
-if (!__clc_fp32_subnormals_supported()) {
-   int explg = ((as_uint(expylogx) & EXPBITS_SP32 >> 23) - 127);
-   m = (23-(m + 149)) == 0 ? 1: m;
-   uint mantissa =  ((as_uint(expylogx) & 
MANTBITS_SP32)|IMPBIT_SP32) >> (23-(m + 149));
-   sexpylogx = as_float(mantissa);
-} else {
-   sexpylogx = expylogx * as_float(0x1 << (m + 149));
-}
-
-
+float sexpylogx = expylogx * as_float(0x1 << (m + 149));
 float texpylogx = as_float(as_int(expylogx) + m2);
 expylogx = m < -125 ? sexpylogx : texpylogx;
 


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


[PATCH] D45697: [clang-tidy] Fix clang-tidy doesn't read .clangtidy configuration file.

2018-04-17 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 142817.
hokein marked an inline comment as done.
hokein added a comment.

Make grep pattern more obvious.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45697

Files:
  clang-tidy/ClangTidyOptions.cpp
  clang-tidy/ClangTidyOptions.h
  clang-tidy/tool/ClangTidyMain.cpp
  test/clang-tidy/read_file_config.cpp

Index: test/clang-tidy/read_file_config.cpp
===
--- /dev/null
+++ test/clang-tidy/read_file_config.cpp
@@ -0,0 +1,12 @@
+// RUN: mkdir -p %T/read-file-config/
+// RUN: cp %s %T/read-file-config/test.cpp
+// RUN: echo 'Checks: "-*,modernize-use-nullptr"' > %T/read-file-config/.clang-tidy
+// RUN: echo '[{"command": "cc -c -o test.o test.cpp", "directory": "%T/read-file-config", "file": "%T/read-file-config/test.cpp"}]' > %T/read-file-config/compile_commands.json
+// RUN: clang-tidy %T/read-file-config/test.cpp | not grep "warning: .*\[clang-analyzer-deadcode.DeadStores\]$"
+// RUN: clang-tidy -checks="-*,clang-analyzer-*" %T/read-file-config/test.cpp | grep "warning: .*\[clang-analyzer-deadcode.DeadStores\]$"
+
+void f() {
+  int x;
+  x = 1;
+  x = 2;
+}
Index: clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -286,7 +286,8 @@
   OS.flush();
 }
 
-static std::unique_ptr createOptionsProvider() {
+static std::unique_ptr createOptionsProvider(
+   llvm::IntrusiveRefCntPtr FS) {
   ClangTidyGlobalOptions GlobalOptions;
   if (std::error_code Err = parseLineFilter(LineFilter, GlobalOptions)) {
 llvm::errs() << "Invalid LineFilter: " << Err.message() << "\n\nUsage:\n";
@@ -334,7 +335,7 @@
 }
   }
   return llvm::make_unique(GlobalOptions, DefaultOptions,
-OverrideOptions);
+OverrideOptions, std::move(FS));
 }
 
 llvm::IntrusiveRefCntPtr
@@ -364,8 +365,13 @@
 static int clangTidyMain(int argc, const char **argv) {
   CommonOptionsParser OptionsParser(argc, argv, ClangTidyCategory,
 cl::ZeroOrMore);
+  llvm::IntrusiveRefCntPtr BaseFS(
+  VfsOverlay.empty() ? vfs::getRealFileSystem()
+ : getVfsOverlayFromFile(VfsOverlay));
+  if (!BaseFS)
+return 1;
 
-  auto OwningOptionsProvider = createOptionsProvider();
+  auto OwningOptionsProvider = createOptionsProvider(BaseFS);
   auto *OptionsProvider = OwningOptionsProvider.get();
   if (!OptionsProvider)
 return 1;
@@ -432,12 +438,6 @@
 llvm::cl::PrintHelpMessage(/*Hidden=*/false, /*Categorized=*/true);
 return 1;
   }
-  llvm::IntrusiveRefCntPtr BaseFS(
-  VfsOverlay.empty() ? vfs::getRealFileSystem()
- : getVfsOverlayFromFile(VfsOverlay));
-  if (!BaseFS)
-return 1;
-
   ProfileData Profile;
 
   llvm::InitializeAllTargetInfos();
Index: clang-tidy/ClangTidyOptions.h
===
--- clang-tidy/ClangTidyOptions.h
+++ clang-tidy/ClangTidyOptions.h
@@ -13,7 +13,9 @@
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/Support/ErrorOr.h"
+#include "clang/Basic/VirtualFileSystem.h"
 #include 
 #include 
 #include 
@@ -221,7 +223,8 @@
   /// whatever options are read from the configuration file.
   FileOptionsProvider(const ClangTidyGlobalOptions &GlobalOptions,
   const ClangTidyOptions &DefaultOptions,
-  const ClangTidyOptions &OverrideOptions);
+  const ClangTidyOptions &OverrideOptions,
+  llvm::IntrusiveRefCntPtr FS = nullptr);
 
   /// \brief Initializes the \c FileOptionsProvider instance with a custom set
   /// of configuration file handlers.
@@ -255,6 +258,7 @@
   llvm::StringMap CachedOptions;
   ClangTidyOptions OverrideOptions;
   ConfigFileHandlers ConfigHandlers;
+  llvm::IntrusiveRefCntPtr FS;
 };
 
 /// \brief Parses LineFilter from JSON and stores it to the \p Options.
Index: clang-tidy/ClangTidyOptions.cpp
===
--- clang-tidy/ClangTidyOptions.cpp
+++ clang-tidy/ClangTidyOptions.cpp
@@ -204,9 +204,12 @@
 FileOptionsProvider::FileOptionsProvider(
 const ClangTidyGlobalOptions &GlobalOptions,
 const ClangTidyOptions &DefaultOptions,
-const ClangTidyOptions &OverrideOptions)
+const ClangTidyOptions &OverrideOptions,
+llvm::IntrusiveRefCntPtr VFS)
 : DefaultOptionsProvider(GlobalOptions, DefaultOptions),
-  OverrideOptions(OverrideOptions) {
+  OverrideOptions(OverrideOptions), FS(std::move(VFS)) {
+  if (!FS)
+FS = vfs::getRealFileSystem();
   ConfigHandlers.emplace_back(".clang-tidy", parseConfiguration);
 }
 
@@ -224,14 +227,19 @@
 std::vector
 FileOptionsProvider:

[PATCH] D44984: [HIP] Add hip file type and codegen for kernel launching

2018-04-17 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl marked 2 inline comments as done.
yaxunl added inline comments.



Comment at: lib/Frontend/CompilerInvocation.cpp:2109
+  Opts.HIP = true;
+  }
+

rjmccall wrote:
> yaxunl wrote:
> > rjmccall wrote:
> > > Why is this done here?  We infer the language mode from the input kind 
> > > somewhere else.
> > It is usually done through CompilerInvocation::setLangDefaults. However, 
> > HIP does not have its own input kind nor is it defined as a language 
> > standard. Therefore it cannot use CompilerInvocation::setLangDefaults to 
> > set Opts.HIP. 
> What are the values of -x if not input kinds or language standards?
I will add hip as input kind and language standard since it really is both.


https://reviews.llvm.org/D44984



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


[PATCH] D44984: [HIP] Add hip input kind and codegen for kernel launching

2018-04-17 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 142818.
yaxunl marked an inline comment as done.
yaxunl retitled this revision from "[HIP] Add hip file type and codegen for 
kernel launching" to "[HIP] Add hip input kind and codegen for kernel 
launching".
yaxunl edited the summary of this revision.
yaxunl added a comment.

Add hip as input kind and language standard.


https://reviews.llvm.org/D44984

Files:
  include/clang/Basic/LangOptions.def
  include/clang/Frontend/FrontendOptions.h
  include/clang/Frontend/LangStandards.def
  lib/CodeGen/CGCUDANV.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/FrontendActions.cpp
  lib/Frontend/InitPreprocessor.cpp
  lib/Sema/SemaCUDA.cpp
  lib/Sema/SemaDecl.cpp
  test/CodeGenCUDA/device-stub.cu

Index: test/CodeGenCUDA/device-stub.cu
===
--- test/CodeGenCUDA/device-stub.cu
+++ test/CodeGenCUDA/device-stub.cu
@@ -1,8 +1,11 @@
 // RUN: echo "GPU binary would be here" > %t
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -fcuda-include-gpubinary %t -o - | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -fcuda-include-gpubinary %t -o -  -DNOGLOBALS \
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s \
+// RUN:   -fcuda-include-gpubinary %t -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s \
+// RUN:   -fcuda-include-gpubinary %t -o -  -DNOGLOBALS \
 // RUN:   | FileCheck %s -check-prefix=NOGLOBALS
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -o - | FileCheck %s -check-prefix=NOGPUBIN
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -o - \
+// RUN:   | FileCheck %s -check-prefix=NOGPUBIN
 
 #include "Inputs/cuda.h"
 
@@ -77,10 +80,14 @@
 // Test that we've built a function to register kernels and global vars.
 // CHECK: define internal void @__cuda_register_globals
 // CHECK: call{{.*}}cudaRegisterFunction(i8** %0, {{.*}}kernelfunc
-// CHECK-DAG: call{{.*}}cudaRegisterVar(i8** %0, {{.*}}device_var{{.*}}i32 0, i32 4, i32 0, i32 0
-// CHECK-DAG: call{{.*}}cudaRegisterVar(i8** %0, {{.*}}constant_var{{.*}}i32 0, i32 4, i32 1, i32 0
-// CHECK-DAG: call{{.*}}cudaRegisterVar(i8** %0, {{.*}}ext_device_var{{.*}}i32 1, i32 4, i32 0, i32 0
-// CHECK-DAG: call{{.*}}cudaRegisterVar(i8** %0, {{.*}}ext_constant_var{{.*}}i32 1, i32 4, i32 1, i32 0
+// CHECK-DAG: call{{.*}}cudaRegisterVar(i8** %0, {{.*}}device_var{{.*}}
+// CHECK-DAG-SAME:  i32 0, i32 4, i32 0, i32 0
+// CHECK-DAG: call{{.*}}cudaRegisterVar(i8** %0, {{.*}}constant_var{{.*}}
+// CHECK-DAG-SAME:  i32 0, i32 4, i32 1, i32 0
+// CHECK-DAG: call{{.*}}cudaRegisterVar(i8** %0, {{.*}}ext_device_var{{.*}}
+// CHECK-DAG-SAME:  i32 1, i32 4, i32 0, i32 0
+// CHECK-DAG: call{{.*}}cudaRegisterVar(i8** %0, {{.*}}ext_constant_var{{.*}}
+// CHECK-DAG-SAME:  i32 1, i32 4, i32 1, i32 0
 // CHECK: ret void
 
 // Test that we've built contructor..
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -9048,11 +9048,13 @@
 
   if (getLangOpts().CUDA) {
 IdentifierInfo *II = NewFD->getIdentifier();
-if (II && II->isStr("cudaConfigureCall") && !NewFD->isInvalidDecl() &&
+if (II &&
+((getLangOpts().HIP && II->isStr("hipConfigureCall")) ||
+ (!getLangOpts().HIP && II->isStr("cudaConfigureCall"))) &&
+!NewFD->isInvalidDecl() &&
 NewFD->getDeclContext()->getRedeclContext()->isTranslationUnit()) {
   if (!R->getAs()->getReturnType()->isScalarType())
 Diag(NewFD->getLocation(), diag::err_config_scalar_return);
-
   Context.setcudaConfigureCallDecl(NewFD);
 }
 
Index: lib/Sema/SemaCUDA.cpp
===
--- lib/Sema/SemaCUDA.cpp
+++ lib/Sema/SemaCUDA.cpp
@@ -42,8 +42,9 @@
  SourceLocation GGGLoc) {
   FunctionDecl *ConfigDecl = Context.getcudaConfigureCallDecl();
   if (!ConfigDecl)
-return ExprError(Diag(oc, diag::err_undeclared_var_use)
- << "cudaConfigureCall");
+return ExprError(
+Diag(oc, diag::err_undeclared_var_use)
+<< (getLangOpts().HIP ? "hipConfigureCall" : "cudaConfigureCall"));
   QualType ConfigQTy = ConfigDecl->getType();
 
   DeclRefExpr *ConfigDR = new (Context)
Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -465,6 +465,8 @@
 Builder.defineMacro("__ASSEMBLER__");
   if (LangOpts.CUDA)
 Builder.defineMacro("__CUDA__");
+  if (LangOpts.HIP)
+Builder.defineMacro("__HIP__");
 }
 
 /// Initialize the predefined C++ language feature test macros defined in
Index: lib/Frontend/FrontendActions.cpp
===
--- lib/Frontend/FrontendActions.cpp
+++ lib/Frontend/FrontendActions.c

[PATCH] D45217: [ThinLTO] Pass -save-temps to LTO backend for distributed ThinLTO builds

2018-04-17 Thread Bjorn Pettersson via Phabricator via cfe-commits
bjope added inline comments.



Comment at: cfe/trunk/lib/Driver/ToolChains/Clang.cpp:3270
 
+  if (const Arg *A = Args.getLastArg(options::OPT_save_temps_EQ))
+Args.AddLastArg(CmdArgs, options::OPT_save_temps_EQ);

A is not used, so this does not compile when using -Werror:

```
../tools/clang/lib/Driver/ToolChains/Clang.cpp:3270:18: error: unused variable 
'A' [-Werror,-Wunused-variable]
  if (const Arg *A = Args.getLastArg(options::OPT_save_temps_EQ))

```


Repository:
  rL LLVM

https://reviews.llvm.org/D45217



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


[PATCH] D45217: [ThinLTO] Pass -save-temps to LTO backend for distributed ThinLTO builds

2018-04-17 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson added inline comments.



Comment at: cfe/trunk/lib/Driver/ToolChains/Clang.cpp:3270
 
+  if (const Arg *A = Args.getLastArg(options::OPT_save_temps_EQ))
+Args.AddLastArg(CmdArgs, options::OPT_save_temps_EQ);

bjope wrote:
> A is not used, so this does not compile when using -Werror:
> 
> ```
> ../tools/clang/lib/Driver/ToolChains/Clang.cpp:3270:18: error: unused 
> variable 'A' [-Werror,-Wunused-variable]
>   if (const Arg *A = Args.getLastArg(options::OPT_save_temps_EQ))
> 
> ```
Sorry, will fix shortly.


Repository:
  rL LLVM

https://reviews.llvm.org/D45217



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


[PATCH] D45290: [Driver] Use the per-API level Android library directories.

2018-04-17 Thread Dan Albert via Phabricator via cfe-commits
danalbert marked an inline comment as done.
danalbert added inline comments.



Comment at: lib/Driver/ToolChains/Linux.cpp:25
 #include "llvm/Support/Path.h"
+#include "llvm/Support/ScopedPrinter.h"
 #include 

eugenis wrote:
> I don't see why this include is necessary.
`llvm::to_string`


Repository:
  rC Clang

https://reviews.llvm.org/D45290



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


r330210 - Remove unused variable

2018-04-17 Thread Teresa Johnson via cfe-commits
Author: tejohnson
Date: Tue Apr 17 13:21:53 2018
New Revision: 330210

URL: http://llvm.org/viewvc/llvm-project?rev=330210&view=rev
Log:
Remove unused variable

Fixes unused variable error introduced in r330194.

Modified:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=330210&r1=330209&r2=330210&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Tue Apr 17 13:21:53 2018
@@ -3267,7 +3267,7 @@ void Clang::ConstructJob(Compilation &C,
 Args.AddLastArg(CmdArgs, options::OPT_fthinlto_index_EQ);
   }
 
-  if (const Arg *A = Args.getLastArg(options::OPT_save_temps_EQ))
+  if (Args.getLastArg(options::OPT_save_temps_EQ))
 Args.AddLastArg(CmdArgs, options::OPT_save_temps_EQ);
 
   // Embed-bitcode option.


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


[PATCH] D45217: [ThinLTO] Pass -save-temps to LTO backend for distributed ThinLTO builds

2018-04-17 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson added inline comments.



Comment at: cfe/trunk/lib/Driver/ToolChains/Clang.cpp:3270
 
+  if (const Arg *A = Args.getLastArg(options::OPT_save_temps_EQ))
+Args.AddLastArg(CmdArgs, options::OPT_save_temps_EQ);

tejohnson wrote:
> bjope wrote:
> > A is not used, so this does not compile when using -Werror:
> > 
> > ```
> > ../tools/clang/lib/Driver/ToolChains/Clang.cpp:3270:18: error: unused 
> > variable 'A' [-Werror,-Wunused-variable]
> >   if (const Arg *A = Args.getLastArg(options::OPT_save_temps_EQ))
> > 
> > ```
> Sorry, will fix shortly.
Fixed in r330210


Repository:
  rL LLVM

https://reviews.llvm.org/D45217



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


  1   2   >