date:20170718

r308268 - Also add the option -no-pie (like -nopie)

2017-07-18 Thread Sylvestre Ledru via cfe-commits

Author: sylvestre
Date: Mon Jul 17 23:54:54 2017
New Revision: 308268

URL: http://llvm.org/viewvc/llvm-project?rev=308268&view=rev
Log:
Also add the option -no-pie (like -nopie)

Summary:
For example, this option is expected by ghc (haskell compiler). Currently, 
building with ghc will fail with:

```
clang: error: unknown argument: '-no-pie'
`gcc' failed in phase `Linker'. (Exit code: 1)
. /usr/share/haskell-devscripts/Dh_Haskell.sh && \
configure_recipe
```

This won't do anything (but won't fail with an error)


Reviewers: rafael, joerg

Reviewed By: joerg

Subscribers: joerg, cfe-commits

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

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

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=308268&r1=308267&r2=308268&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Mon Jul 17 23:54:54 2017
@@ -2119,6 +2119,7 @@ def nofixprebinding : Flag<["-"], "nofix
 def nolibc : Flag<["-"], "nolibc">;
 def nomultidefs : Flag<["-"], "nomultidefs">;
 def nopie : Flag<["-"], "nopie">;
+def no_pie : Flag<["-"], "no-pie">, Alias;
 def noprebind : Flag<["-"], "noprebind">;
 def noseglinkedit : Flag<["-"], "noseglinkedit">;
 def nostartfiles : Flag<["-"], "nostartfiles">;

Modified: cfe/trunk/test/Driver/pic.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/pic.c?rev=308268&r1=308267&r2=308268&view=diff
==
--- cfe/trunk/test/Driver/pic.c (original)
+++ cfe/trunk/test/Driver/pic.c Mon Jul 17 23:54:54 2017
@@ -247,6 +247,9 @@
 // On OpenBSD, -nopie needs to be passed through to the linker.
 // RUN: %clang %s -target i386-pc-openbsd -nopie -### 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-NOPIE-LD
+// Try with the alias
+// RUN: %clang %s -target i386-pc-openbsd -no-pie -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-NOPIE-LD
 //
 // On Android PIC is enabled by default
 // RUN: %clang -c %s -target i686-linux-android -### 2>&1 \


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

[PATCH] D35529: [COFF, ARM64] Make +reserve-x18 the default

2017-07-18 Thread Mandeep Singh Grang via Phabricator via cfe-commits

mgrang added a comment.

In https://reviews.llvm.org/D35529#812427, @mstorsjo wrote:

> Wouldn't it make more sense to set this by default within LLVM, where it's 
> already enabled by default for darwin?
>
> Currently there's this code there:
>
>   AArch64Subtarget::AArch64Subtarget(const Triple &TT, const std::string &CPU,
>  const std::string &FS,
>  const TargetMachine &TM, bool 
> LittleEndian)
>   : AArch64GenSubtargetInfo(TT, CPU, FS), ReserveX18(TT.isOSDarwin()),
>


Thanks for pointing this out. This certainly seems a cleaner way/place to do 
this. Lemme give it a shot.


https://reviews.llvm.org/D35529



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

[PATCH] D34824: clang-format: add an option -verbose to list the files being processed

2017-07-18 Thread Daniel Jasper via Phabricator via cfe-commits

djasper added inline comments.



Comment at: tools/clang-format/ClangFormat.cpp:377
 break;
   case 1:
 Error = clang::format::format(FileNames[0]);

sylvestre.ledru wrote:
> djasper wrote:
> > I think we should restructure the code to not have to duplicate what you 
> > are adding here. I think fundamentally, we should be able to change this to:
> > 
> >   if (FileNames.empty()) {
> > Error = clang::format::format("-");
> > return Error ? 1 : 0;
> >   }
> >   if (FileNames.size() == 1 && (!Offsets.empty() || !Lengths.empty() || 
> > !LineRanges.empty())) {
> > errs() << "error: -offset  "
> > return 1;
> >   }
> >   for (const auto& FileName : FileNames) {
> > if (Verbose.getNumOccurences() != 0)
> >   errs() << "Formatting " << Filename << "\n";
> > Error |= clang::format::format(FileName);
> >   }
> >   return Error ? 1 : 0;
> This isn't correct.
> You can have
> 
> ```
> bin/clang-format  -lines=1:1 foo-1.cpp foo-2.cpp 
> ```
> we won't enter into the "error: -offset" error
> 
Right. In my code I had a bug s/== 1/ != 1/. But I don't think you can drop 
that part entirely. As it is now, you cannot specify any -lines/offsets/lenghts 
even for a single file?


https://reviews.llvm.org/D34824



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

r308269 - [Index] Prevent canonical decl becoming nullptr

2017-07-18 Thread Krasimir Georgiev via cfe-commits

Author: krasimir
Date: Tue Jul 18 00:20:53 2017
New Revision: 308269

URL: http://llvm.org/viewvc/llvm-project?rev=308269&view=rev
Log:
[Index] Prevent canonical decl becoming nullptr

Summary:
This patch prevents getCanonicalDecl returning nullptr in case it finds
a canonical TemplateDeclaration with no attached TemplatedDecl.
Found by running the indexer over a version of the standard library deep inside
a template metaprogramming mess.

Reviewers: klimek, vsk

Reviewed By: vsk

Subscribers: vsk, arphaman, cfe-commits

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

Added:
cfe/trunk/test/Index/Core/no-templated-canonical-decl.cpp
Modified:
cfe/trunk/lib/Index/IndexingContext.cpp

Modified: cfe/trunk/lib/Index/IndexingContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexingContext.cpp?rev=308269&r1=308268&r2=308269&view=diff
==
--- cfe/trunk/lib/Index/IndexingContext.cpp (original)
+++ cfe/trunk/lib/Index/IndexingContext.cpp Tue Jul 18 00:20:53 2017
@@ -260,8 +260,10 @@ static const Decl *adjustParent(const De
 static const Decl *getCanonicalDecl(const Decl *D) {
   D = D->getCanonicalDecl();
   if (auto TD = dyn_cast(D)) {
-D = TD->getTemplatedDecl();
-assert(D->isCanonicalDecl());
+if (auto TTD = TD->getTemplatedDecl()) {
+  D = TTD;
+  assert(D->isCanonicalDecl());
+}
   }
 
   return D;

Added: cfe/trunk/test/Index/Core/no-templated-canonical-decl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Core/no-templated-canonical-decl.cpp?rev=308269&view=auto
==
--- cfe/trunk/test/Index/Core/no-templated-canonical-decl.cpp (added)
+++ cfe/trunk/test/Index/Core/no-templated-canonical-decl.cpp Tue Jul 18 
00:20:53 2017
@@ -0,0 +1,4 @@
+// RUN: c-index-test core -print-source-symbols -include-locals -- %s | 
FileCheck %s
+
+template  class A> class B { typedef A A_int; };
+// CHECK: [[@LINE-1]]:46 | class(Gen)/C++ | B | c:@ST>1#t>1#T@B |  
| Def | rel: 0


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

[PATCH] D35212: [Index] Prevent canonical decl becoming nullptr

2017-07-18 Thread Krasimir Georgiev via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL308269: [Index] Prevent canonical decl becoming nullptr 
(authored by krasimir).

Repository:
  rL LLVM

https://reviews.llvm.org/D35212

Files:
  cfe/trunk/lib/Index/IndexingContext.cpp
  cfe/trunk/test/Index/Core/no-templated-canonical-decl.cpp


Index: cfe/trunk/test/Index/Core/no-templated-canonical-decl.cpp
===
--- cfe/trunk/test/Index/Core/no-templated-canonical-decl.cpp
+++ cfe/trunk/test/Index/Core/no-templated-canonical-decl.cpp
@@ -0,0 +1,4 @@
+// RUN: c-index-test core -print-source-symbols -include-locals -- %s | 
FileCheck %s
+
+template  class A> class B { typedef A A_int; };
+// CHECK: [[@LINE-1]]:46 | class(Gen)/C++ | B | c:@ST>1#t>1#T@B |  
| Def | rel: 0
Index: cfe/trunk/lib/Index/IndexingContext.cpp
===
--- cfe/trunk/lib/Index/IndexingContext.cpp
+++ cfe/trunk/lib/Index/IndexingContext.cpp
@@ -260,8 +260,10 @@
 static const Decl *getCanonicalDecl(const Decl *D) {
   D = D->getCanonicalDecl();
   if (auto TD = dyn_cast(D)) {
-D = TD->getTemplatedDecl();
-assert(D->isCanonicalDecl());
+if (auto TTD = TD->getTemplatedDecl()) {
+  D = TTD;
+  assert(D->isCanonicalDecl());
+}
   }
 
   return D;


Index: cfe/trunk/test/Index/Core/no-templated-canonical-decl.cpp
===
--- cfe/trunk/test/Index/Core/no-templated-canonical-decl.cpp
+++ cfe/trunk/test/Index/Core/no-templated-canonical-decl.cpp
@@ -0,0 +1,4 @@
+// RUN: c-index-test core -print-source-symbols -include-locals -- %s | FileCheck %s
+
+template  class A> class B { typedef A A_int; };
+// CHECK: [[@LINE-1]]:46 | class(Gen)/C++ | B | c:@ST>1#t>1#T@B |  | Def | rel: 0
Index: cfe/trunk/lib/Index/IndexingContext.cpp
===
--- cfe/trunk/lib/Index/IndexingContext.cpp
+++ cfe/trunk/lib/Index/IndexingContext.cpp
@@ -260,8 +260,10 @@
 static const Decl *getCanonicalDecl(const Decl *D) {
   D = D->getCanonicalDecl();
   if (auto TD = dyn_cast(D)) {
-D = TD->getTemplatedDecl();
-assert(D->isCanonicalDecl());
+if (auto TTD = TD->getTemplatedDecl()) {
+  D = TTD;
+  assert(D->isCanonicalDecl());
+}
   }
 
   return D;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33676: Place implictly declared functions at block scope

2017-07-18 Thread Momchil Velikov via Phabricator via cfe-commits

chill added a comment.

Ping.


https://reviews.llvm.org/D33676



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

[PATCH] D35529: [COFF, ARM64] Make +reserve-x18 the default

2017-07-18 Thread Mandeep Singh Grang via Phabricator via cfe-commits

mgrang added a comment.

I pushed a new patch D35531 . This implements 
the logic in llvm AArch64SubTarget which seems to be the proper place to do 
this.
Once that patch is reviewed/accepted, I will abandon this one.


https://reviews.llvm.org/D35529



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

[PATCH] D35527: [CMake] Move CLANG_ENABLE_(ARCMT|OBJC_REWRITER|STATIC_ANALYZER) into clang/Config/config.h.

2017-07-18 Thread Vassil Vassilev via Phabricator via cfe-commits

v.g.vassilev accepted this revision.
v.g.vassilev added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rL LLVM

https://reviews.llvm.org/D35527



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

[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo

2017-07-18 Thread MinSeong Kim via Phabricator via cfe-commits

minseong.kim created this revision.
Herald added a subscriber: mgorny.

When repo is used, '--version' option does not display correct version
information (i.e. Git hashes). This change makes the parsing of the
version info correctly recognise svn, git, git-svn and repo. This
in turn enables the option displays correct version information with
repo. Tested with git, svn and repo with this patch.


https://reviews.llvm.org/D35533

Files:
  lib/Basic/CMakeLists.txt


Index: lib/Basic/CMakeLists.txt
===
--- lib/Basic/CMakeLists.txt
+++ lib/Basic/CMakeLists.txt
@@ -34,6 +34,7 @@
 "${git_path}/logs/HEAD"  # Git or Git submodule
 "${path}/.svn/wc.db" # SVN 1.7
 "${path}/.svn/entries"   # SVN 1.6
+"${path}/.git/HEAD"  # Repo
 )
 endmacro()
 


Index: lib/Basic/CMakeLists.txt
===
--- lib/Basic/CMakeLists.txt
+++ lib/Basic/CMakeLists.txt
@@ -34,6 +34,7 @@
 "${git_path}/logs/HEAD"  # Git or Git submodule
 "${path}/.svn/wc.db" # SVN 1.7
 "${path}/.svn/entries"   # SVN 1.6
+"${path}/.git/HEAD"  # Repo
 )
 endmacro()
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34439: Add GCC's noexcept-type alias for c++1z-compat-mangling

2017-07-18 Thread Raphael Isemann via Phabricator via cfe-commits

teemperor added a comment.

ping. Any objections to adding this GCC alias?


https://reviews.llvm.org/D34439



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

[PATCH] D35012: [refactor] Add the AST source selection component

2017-07-18 Thread Manuel Klimek via Phabricator via cfe-commits

klimek added inline comments.



Comment at: lib/Tooling/Refactoring/ASTSelection.cpp:100
+  SelectionStack.back().Children.push_back(std::move(Node));
+return true;
+  }

arphaman wrote:
> klimek wrote:
> > Why do we always stop traversal?
> False stops traversal, true is the default return value.
Ah, right.  Generally, I'd have expected us to stop traversal once we're past 
the range?


Repository:
  rL LLVM

https://reviews.llvm.org/D35012



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

[PATCH] D35483: clang-format: fix block OpeningLineIndex around preprocessor

2017-07-18 Thread Francois Ferrand via Phabricator via cfe-commits

Typz updated this revision to Diff 107030.
Typz marked an inline comment as done.
Typz added a comment.

Add more tests


https://reviews.llvm.org/D35483

Files:
  lib/Format/UnwrappedLineParser.cpp
  lib/Format/UnwrappedLineParser.h
  unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -509,6 +509,134 @@
 "}\n"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, AddEndCommentForNamespacesAroundMacros) {
+  // Conditional blocks around are fine
+  EXPECT_EQ("namespace A {\n"
+"#if 1\n"
+"int i;\n"
+"#endif\n"
+"}// namespace A",
+fixNamespaceEndComments("namespace A {\n"
+"#if 1\n"
+"int i;\n"
+"#endif\n"
+"}"));
+  EXPECT_EQ("#if 1\n"
+"#endif\n"
+"namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A",
+fixNamespaceEndComments("#if 1\n"
+"#endif\n"
+"namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}"));
+  EXPECT_EQ("namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A\n"
+"#if 1\n"
+"#endif",
+fixNamespaceEndComments("namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}\n"
+"#if 1\n"
+"#endif"));
+  EXPECT_EQ("#if 1\n"
+"namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A\n"
+"#endif",
+fixNamespaceEndComments("#if 1\n"
+"namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}\n"
+"#endif"));
+
+  // Macro definition has no impact
+  EXPECT_EQ("namespace A {\n"
+"#define FOO\n"
+"int i;\n"
+"}// namespace A",
+fixNamespaceEndComments("namespace A {\n"
+"#define FOO\n"
+"int i;\n"
+"}"));
+  EXPECT_EQ("#define FOO\n"
+"namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A",
+fixNamespaceEndComments("#define FOO\n"
+"namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}"));
+  EXPECT_EQ("namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A\n"
+"#define FOO\n",
+fixNamespaceEndComments("namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}\n"
+"#define FOO\n"));
+
+  // No replacement if open & close in different conditional blocks
+  EXPECT_EQ("#if 1\n"
+"namespace A {\n"
+"#endif\n"
+"int i;\n"
+"int j;\n"
+"#if 1\n"
+"}\n"
+"#endif",
+fixNamespaceEndComments("#if 1\n"
+"namespace A {\n"
+"#endif\n"
+"int i;\n"
+"int j;\n"
+"#if 1\n"
+"}\n"
+"#endif"));
+  EXPECT_EQ("#ifdef A\n"
+"namespace A {\n"
+"#endif\n"
+"int i;\n"
+"int j;\n"
+"#ifdef B\n"
+"}\n"
+"#endif",
+fixNamespaceEndComments("#ifdef A\n"
+"namespace A {\n"
+"#endif\n"
+"int i;\n"
+"int j;\n"
+"#ifdef B\n"
+"}\n"
+"#endif"));
+
+  // No replacement inside unreachable conditional block
+  EXPECT_EQ("#if 0\n"
+"namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}\n"
+

r308277 - [CMake] Move CLANG_ENABLE_(ARCMT|OBJC_REWRITER|STATIC_ANALYZER) into clang/Config/config.h.

2017-07-18 Thread NAKAMURA Takumi via cfe-commits

Author: chapuni
Date: Tue Jul 18 01:55:03 2017
New Revision: 308277

URL: http://llvm.org/viewvc/llvm-project?rev=308277&view=rev
Log:
[CMake] Move CLANG_ENABLE_(ARCMT|OBJC_REWRITER|STATIC_ANALYZER) into 
clang/Config/config.h.

LLVM_ENABLE_MODULES is sensitive of -D. Move them into config.h.

FIXME: It'd be better that they are #cmakedefine01 rather than #cmakedefine.
(#if FOO rather than #if defined(FOO))
Then we can find missing #include "clang/Config/config.h" in the future.

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

Modified:
cfe/trunk/CMakeLists.txt
cfe/trunk/include/clang/Config/config.h.cmake
cfe/trunk/lib/Frontend/Rewrite/FrontendActions.cpp
cfe/trunk/lib/Frontend/Rewrite/RewriteModernObjC.cpp
cfe/trunk/lib/Frontend/Rewrite/RewriteObjC.cpp
cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
cfe/trunk/tools/libclang/ARCMigrate.cpp

Modified: cfe/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/CMakeLists.txt?rev=308277&r1=308276&r2=308277&view=diff
==
--- cfe/trunk/CMakeLists.txt (original)
+++ cfe/trunk/CMakeLists.txt Tue Jul 18 01:55:03 2017
@@ -389,11 +389,7 @@ if(CLANG_ANALYZER_BUILD_Z3)
 endif()
 
 if(CLANG_ENABLE_ARCMT)
-  add_definitions(-DCLANG_ENABLE_ARCMT)
-  add_definitions(-DCLANG_ENABLE_OBJC_REWRITER)
-endif()
-if(CLANG_ENABLE_STATIC_ANALYZER)
-  add_definitions(-DCLANG_ENABLE_STATIC_ANALYZER)
+  set(CLANG_ENABLE_OBJC_REWRITER ON)
 endif()
 
 # Clang version information

Modified: cfe/trunk/include/clang/Config/config.h.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Config/config.h.cmake?rev=308277&r1=308276&r2=308277&view=diff
==
--- cfe/trunk/include/clang/Config/config.h.cmake (original)
+++ cfe/trunk/include/clang/Config/config.h.cmake Tue Jul 18 01:55:03 2017
@@ -56,4 +56,9 @@
 /* enable x86 relax relocations by default */
 #cmakedefine01 ENABLE_X86_RELAX_RELOCATIONS
 
+/* Enable each functionality of modules */
+#cmakedefine CLANG_ENABLE_ARCMT
+#cmakedefine CLANG_ENABLE_OBJC_REWRITER
+#cmakedefine CLANG_ENABLE_STATIC_ANALYZER
+
 #endif

Modified: cfe/trunk/lib/Frontend/Rewrite/FrontendActions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/Rewrite/FrontendActions.cpp?rev=308277&r1=308276&r2=308277&view=diff
==
--- cfe/trunk/lib/Frontend/Rewrite/FrontendActions.cpp (original)
+++ cfe/trunk/lib/Frontend/Rewrite/FrontendActions.cpp Tue Jul 18 01:55:03 2017
@@ -10,6 +10,7 @@
 #include "clang/Rewrite/Frontend/FrontendActions.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/Basic/CharInfo.h"
+#include "clang/Config/config.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Frontend/FrontendDiagnostic.h"

Modified: cfe/trunk/lib/Frontend/Rewrite/RewriteModernObjC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/Rewrite/RewriteModernObjC.cpp?rev=308277&r1=308276&r2=308277&view=diff
==
--- cfe/trunk/lib/Frontend/Rewrite/RewriteModernObjC.cpp (original)
+++ cfe/trunk/lib/Frontend/Rewrite/RewriteModernObjC.cpp Tue Jul 18 01:55:03 
2017
@@ -21,6 +21,7 @@
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Config/config.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "llvm/ADT/DenseSet.h"

Modified: cfe/trunk/lib/Frontend/Rewrite/RewriteObjC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/Rewrite/RewriteObjC.cpp?rev=308277&r1=308276&r2=308277&view=diff
==
--- cfe/trunk/lib/Frontend/Rewrite/RewriteObjC.cpp (original)
+++ cfe/trunk/lib/Frontend/Rewrite/RewriteObjC.cpp Tue Jul 18 01:55:03 2017
@@ -20,6 +20,7 @@
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Config/config.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "llvm/ADT/DenseSet.h"

Modified: cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp?rev=308277&r1=308276&r2=308277&view=diff
==
--- cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp (original)
+++ cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp Tue Jul 18 
01:55:03 2017
@@ -15,6 +15,7 @@
 #include "clang/FrontendTool/Utils.h"
 #include "clang/ARCMigrate/ARCMTActions.h"
 #include "clang/CodeGen/CodeGenAction.h"
+#include "clang/Config/config.h"
 #include

[PATCH] D35527: [CMake] Move CLANG_ENABLE_(ARCMT|OBJC_REWRITER|STATIC_ANALYZER) into clang/Config/config.h.

2017-07-18 Thread NAKAMURA Takumi via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL308277: [CMake] Move 
CLANG_ENABLE_(ARCMT|OBJC_REWRITER|STATIC_ANALYZER) into… (authored by chapuni).

Repository:
  rL LLVM

https://reviews.llvm.org/D35527

Files:
  cfe/trunk/CMakeLists.txt
  cfe/trunk/include/clang/Config/config.h.cmake
  cfe/trunk/lib/Frontend/Rewrite/FrontendActions.cpp
  cfe/trunk/lib/Frontend/Rewrite/RewriteModernObjC.cpp
  cfe/trunk/lib/Frontend/Rewrite/RewriteObjC.cpp
  cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  cfe/trunk/tools/libclang/ARCMigrate.cpp


Index: cfe/trunk/CMakeLists.txt
===
--- cfe/trunk/CMakeLists.txt
+++ cfe/trunk/CMakeLists.txt
@@ -389,11 +389,7 @@
 endif()
 
 if(CLANG_ENABLE_ARCMT)
-  add_definitions(-DCLANG_ENABLE_ARCMT)
-  add_definitions(-DCLANG_ENABLE_OBJC_REWRITER)
-endif()
-if(CLANG_ENABLE_STATIC_ANALYZER)
-  add_definitions(-DCLANG_ENABLE_STATIC_ANALYZER)
+  set(CLANG_ENABLE_OBJC_REWRITER ON)
 endif()
 
 # Clang version information
Index: cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -15,6 +15,7 @@
 #include "clang/FrontendTool/Utils.h"
 #include "clang/ARCMigrate/ARCMTActions.h"
 #include "clang/CodeGen/CodeGenAction.h"
+#include "clang/Config/config.h"
 #include "clang/Driver/Options.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
Index: cfe/trunk/lib/Frontend/Rewrite/RewriteModernObjC.cpp
===
--- cfe/trunk/lib/Frontend/Rewrite/RewriteModernObjC.cpp
+++ cfe/trunk/lib/Frontend/Rewrite/RewriteModernObjC.cpp
@@ -21,6 +21,7 @@
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Config/config.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "llvm/ADT/DenseSet.h"
Index: cfe/trunk/lib/Frontend/Rewrite/FrontendActions.cpp
===
--- cfe/trunk/lib/Frontend/Rewrite/FrontendActions.cpp
+++ cfe/trunk/lib/Frontend/Rewrite/FrontendActions.cpp
@@ -10,6 +10,7 @@
 #include "clang/Rewrite/Frontend/FrontendActions.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/Basic/CharInfo.h"
+#include "clang/Config/config.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
Index: cfe/trunk/lib/Frontend/Rewrite/RewriteObjC.cpp
===
--- cfe/trunk/lib/Frontend/Rewrite/RewriteObjC.cpp
+++ cfe/trunk/lib/Frontend/Rewrite/RewriteObjC.cpp
@@ -20,6 +20,7 @@
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Config/config.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "llvm/ADT/DenseSet.h"
Index: cfe/trunk/tools/libclang/ARCMigrate.cpp
===
--- cfe/trunk/tools/libclang/ARCMigrate.cpp
+++ cfe/trunk/tools/libclang/ARCMigrate.cpp
@@ -14,6 +14,7 @@
 #include "clang-c/Index.h"
 #include "CXString.h"
 #include "clang/ARCMigrate/ARCMT.h"
+#include "clang/Config/config.h"
 #include "clang/Frontend/TextDiagnosticBuffer.h"
 #include "llvm/Support/FileSystem.h"
 
Index: cfe/trunk/include/clang/Config/config.h.cmake
===
--- cfe/trunk/include/clang/Config/config.h.cmake
+++ cfe/trunk/include/clang/Config/config.h.cmake
@@ -56,4 +56,9 @@
 /* enable x86 relax relocations by default */
 #cmakedefine01 ENABLE_X86_RELAX_RELOCATIONS
 
+/* Enable each functionality of modules */
+#cmakedefine CLANG_ENABLE_ARCMT
+#cmakedefine CLANG_ENABLE_OBJC_REWRITER
+#cmakedefine CLANG_ENABLE_STATIC_ANALYZER
+
 #endif


Index: cfe/trunk/CMakeLists.txt
===
--- cfe/trunk/CMakeLists.txt
+++ cfe/trunk/CMakeLists.txt
@@ -389,11 +389,7 @@
 endif()
 
 if(CLANG_ENABLE_ARCMT)
-  add_definitions(-DCLANG_ENABLE_ARCMT)
-  add_definitions(-DCLANG_ENABLE_OBJC_REWRITER)
-endif()
-if(CLANG_ENABLE_STATIC_ANALYZER)
-  add_definitions(-DCLANG_ENABLE_STATIC_ANALYZER)
+  set(CLANG_ENABLE_OBJC_REWRITER ON)
 endif()
 
 # Clang version information
Index: cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -15,6 +15,7 @@
 #include "clang/FrontendTool/Utils.h"
 #include "clang/ARCMigrate/ARCMTActions.h"
 #include "clang/CodeGen/CodeGen

[PATCH] D35485: [clang-format] Fix comment levels between '}' and PPDirective

2017-07-18 Thread Daniel Jasper via Phabricator via cfe-commits

djasper added inline comments.



Comment at: lib/Format/UnwrappedLineParser.cpp:489
 
-  nextToken(); // Munch the closing brace.
+  nextToken(InitialLevel); // Munch the closing brace.
 

What happens if you instead change the Line->Level = InitialLevel; statement 
from below to before this line? That seems like the more intuitively correct 
fix.



Comment at: lib/Format/UnwrappedLineParser.cpp:2378
   ScopedLineState BlockState(*this, SwitchToPreprocessorLines);
+  if (InitialLevel) Line->Level = *InitialLevel;
   // Comments stored before the preprocessor directive need to be output

LLVM style requires a line break.



Comment at: unittests/Format/FormatTestComments.cpp:848
+"}",
+format("int f(int i) {\n"
+   "  if (true) {\n"

Generally, mess up the code in some way to ensure that it is actually being 
formatted.


https://reviews.llvm.org/D35485



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

[PATCH] D35109: [Analyzer] SValBuilder Comparison Rearrangement

2017-07-18 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware updated this revision to Diff 107032.
baloghadamsoftware added a comment.

Rearrangement of unsigned comparison removed (FIXME added). Comment regarding 
the types of integer constants added.


https://reviews.llvm.org/D35109

Files:
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  test/Analysis/svalbuilder-rearrange-comparisons.c

Index: test/Analysis/svalbuilder-rearrange-comparisons.c
===
--- /dev/null
+++ test/Analysis/svalbuilder-rearrange-comparisons.c
@@ -0,0 +1,156 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection -verify %s
+
+void clang_analyzer_dump(int x);
+void clang_analyzer_printState();
+
+int f();
+
+void compare_different_symbol() {
+  int x = f(), y = f();
+  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$5{int}}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$2{int}) - (conj_$5{int})) == 0}}
+}
+
+void compare_different_symbol_plus_left_int() {
+  int x = f()+1, y = f();
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 1}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$5{int}}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$5{int}) - (conj_$2{int})) == 1}}
+}
+
+void compare_different_symbol_minus_left_int() {
+  int x = f()-1, y = f();
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) - 1}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$5{int}}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$2{int}) - (conj_$5{int})) == 1}}
+}
+
+void compare_different_symbol_plus_right_int() {
+  int x = f(), y = f()+2;
+  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) + 2}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$2{int}) - (conj_$5{int})) == 2}}
+}
+
+void compare_different_symbol_minus_right_int() {
+  int x = f(), y = f()-2;
+  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) - 2}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$5{int}) - (conj_$2{int})) == 2}}
+}
+
+void compare_different_symbol_plus_left_plus_right_int() {
+  int x = f()+2, y = f()+1;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 2}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) + 1}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$5{int}) - (conj_$2{int})) == 1}}
+}
+
+void compare_different_symbol_plus_left_minus_right_int() {
+  int x = f()+2, y = f()-1;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 2}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) - 1}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$5{int}) - (conj_$2{int})) == 3}}
+}
+
+void compare_different_symbol_minus_left_plus_right_int() {
+  int x = f()-2, y = f()+1;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) - 2}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) + 1}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$2{int}) - (conj_$5{int})) == 3}}
+}
+
+void compare_different_symbol_minus_left_minus_right_int() {
+  int x = f()-2, y = f()-1;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) - 2}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) - 1}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$2{int}) - (conj_$5{int})) == 1}}
+}
+
+void compare_same_symbol() {
+  int x = f(), y = x;
+  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{1 S32b}}
+}
+
+void compare_same_symbol_plus_left_int() {
+  int x = f(), y = x;
+  ++x;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 1}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{0 S32b}}
+}
+
+void compare_same_symbol_minus_left_int() {
+  int x = f(), y = x;
+  --x;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) - 1}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{0 S32b}}
+}
+
+void compare_same_symbol_plus_right_int() {
+  int x = f(), y = x+1;
+  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$2{int}) + 1}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{0 S32b}}
+}
+
+void compare_same_symbol_minus_right_int() {
+  int x = f(), y = x-1;
+  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$2{int}) - 1}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{0 S32b}}
+}
+
+void compare_same_symbol_pl

[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference

2017-07-18 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware added inline comments.



Comment at: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:511
+   SSE->getLHS()->getType()->isSignedIntegerOrEnumerationType() ||
+   SSE->getLHS()->getType()->isPointerType()) {
+  return negV->Negate(BV, F);

NoQ wrote:
> baloghadamsoftware wrote:
> > NoQ wrote:
> > > Pointer types are currently treated as unsigned, so i'm not sure you want 
> > > them here.
> > For me it seems that pointer differences are still pointer types and they 
> > are signed. (The range becomes negative upon negative assumption. From test 
> > `ptr-arith.c`:
> > 
> > ```
> > void use_symbols(int *lhs, int *rhs) {
> >   clang_analyzer_eval(lhs < rhs); // expected-warning{{UNKNOWN}}
> >   if (lhs < rhs)
> > return;
> >   clang_analyzer_eval(lhs < rhs); // expected-warning{{FALSE}}
> > 
> >   clang_analyzer_eval(lhs - rhs); // expected-warning{{UNKNOWN}}
> >   if ((lhs - rhs) != 5)
> > return;
> >   clang_analyzer_eval((lhs - rhs) == 5); // expected-warning{{TRUE}}
> > }
> > ```
> > 
> > If I put `clang_analyzer_printState()` into the empty line in the middle, I 
> > get the following range for the difference: `[-9223372036854775808, 0]`. If 
> > I replace `int*` with `unsigned`, this range becomes `[0, 0]`, so `int*` is 
> > handled as a signed type here.
> Umm, yeah, i was wrong.
> 
> *looks closer*
> 
> `T` is the type of the difference, right? I don't think i'd expect pointer 
> type as the type of the difference.
> 
> Could you add test cases for pointers if you intend to support them (and 
> maybe for unsigned types)?
I do not know exactly the type, but if I remove the `T->isPointerType()` 
condition the test in `ptr_arith.c` will fail with `UNKNOWN`. So the type of 
the difference is a type that returns `true` from `T->isPointerType()`.

Pointer tests are already there in `ptr_arith.c`. Should I duplicate them?


https://reviews.llvm.org/D35110



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

[PATCH] D35485: [clang-format] Fix comment levels between '}' and PPDirective

2017-07-18 Thread Krasimir Georgiev via Phabricator via cfe-commits

krasimir added inline comments.



Comment at: lib/Format/UnwrappedLineParser.cpp:489
 
-  nextToken(); // Munch the closing brace.
+  nextToken(InitialLevel); // Munch the closing brace.
 

djasper wrote:
> What happens if you instead change the Line->Level = InitialLevel; statement 
> from below to before this line? That seems like the more intuitively correct 
> fix.
This doesn't work since comments before the right brace haven't been emitted 
yet and would get the wrong level.



Comment at: unittests/Format/FormatTestComments.cpp:848
+"}",
+format("int f(int i) {\n"
+   "  if (true) {\n"

djasper wrote:
> Generally, mess up the code in some way to ensure that it is actually being 
> formatted.
Messing up doesn't work in this case, because we rely on the original columns 
of the comment and the previous line. That's why I added a bunch of tests.


https://reviews.llvm.org/D35485



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

[PATCH] D35485: [clang-format] Fix comment levels between '}' and PPDirective

2017-07-18 Thread Krasimir Georgiev via Phabricator via cfe-commits

krasimir updated this revision to Diff 107038.
krasimir marked 2 inline comments as done.
krasimir added a comment.

- Fix formatting


https://reviews.llvm.org/D35485

Files:
  lib/Format/UnwrappedLineParser.cpp
  lib/Format/UnwrappedLineParser.h
  unittests/Format/FormatTestComments.cpp

Index: unittests/Format/FormatTestComments.cpp
===
--- unittests/Format/FormatTestComments.cpp
+++ unittests/Format/FormatTestComments.cpp
@@ -836,6 +836,25 @@
"  int j;\n"
"}"));
 
+  EXPECT_EQ("int f(int i) {\n"
+"  if (true) {\n"
+"++i;\n"
+"  }\n"
+"  // comment\n"
+"#ifdef A\n"
+"  int j;\n"
+"#endif\n"
+"}",
+format("int f(int i) {\n"
+   "  if (true) {\n"
+   "++i;\n"
+   "  }\n"
+   "  // comment\n"
+   "#ifdef A\n"
+   "  int j;\n"
+   "#endif\n"
+   "}"));
+
   // Keep the current level if there is an empty line between the comment and
   // the preprocessor directive.
   EXPECT_EQ("void f() {\n"
@@ -853,6 +872,46 @@
"  int j;\n"
"}"));
 
+  EXPECT_EQ("void f() {\n"
+"  int i;\n"
+"  return i;\n"
+"}\n"
+"// comment\n"
+"\n"
+"#ifdef A\n"
+"int i;\n"
+"#endif // A",
+format("void f() {\n"
+   "  int i;\n"
+   "  return i;\n"
+   "}\n"
+   "// comment\n"
+   "\n"
+   "#ifdef A\n"
+   "int i;\n"
+   "#endif // A"));
+
+  EXPECT_EQ("int f(int i) {\n"
+"  if (true) {\n"
+"++i;\n"
+"  }\n"
+"  // comment\n"
+"\n"
+"#ifdef A\n"
+"  int j;\n"
+"#endif\n"
+"}",
+format("int f(int i) {\n"
+   "  if (true) {\n"
+   "++i;\n"
+   "  }\n"
+   "  // comment\n"
+   "\n"
+   "#ifdef A\n"
+   "  int j;\n"
+   "#endif\n"
+   "}"));
+
   // Align with the preprocessor directive if the comment was originally aligned
   // with the preprocessor directive.
   EXPECT_EQ("void f() {\n"
@@ -867,6 +926,25 @@
"#ifdef A\n"
"  int j;\n"
"}"));
+
+  EXPECT_EQ("int f(int i) {\n"
+"  if (true) {\n"
+"++i;\n"
+"  }\n"
+"// comment\n"
+"#ifdef A\n"
+"  int j;\n"
+"#endif\n"
+"}",
+format("int f(int i) {\n"
+   "  if (true) {\n"
+   "++i;\n"
+   "  }\n"
+   "// comment\n"
+   "#ifdef A\n"
+   "  int j;\n"
+   "#endif\n"
+   "}"));
 }
 
 TEST_F(FormatTestComments, SplitsLongLinesInComments) {
@@ -2571,6 +2649,29 @@
"* long */",
getLLVMStyleWithColumns(20)));
 }
+
+TEST_F(FormatTestComments, TODO) {
+  EXPECT_EQ("void f() {\n"
+"  int i;\n"
+"  return i;\n"
+"}\n"
+"// comment\n"
+"\n"
+"#ifdef A\n"
+"int i;\n"
+"#endif // A",
+format("void f() {\n"
+   "  int i;\n"
+   "  return i;\n"
+   "}\n"
+   "// comment\n"
+   "\n"
+   "#ifdef A\n"
+   "int i;\n"
+   "#endif // A",
+   getLLVMStyleWithColumns(20)));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/UnwrappedLineParser.h
===
--- lib/Format/UnwrappedLineParser.h
+++ lib/Format/UnwrappedLineParser.h
@@ -19,6 +19,7 @@
 #include "FormatToken.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Format/Format.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/Regex.h"
 #include 
 #include 
@@ -123,9 +124,9 @@
   void tryToParseJSFunction();
   void addUnwrappedLine();
   bool eof() const;
-  void nextToken();
+  void nextToken(llvm::Optional InitialLevel = None);
   const FormatToken *getPreviousToken();
-  void readToken();
+  void readToken(llvm::Optional InitialLevel = None);
 
   // Decides which comment tokens should be added to the current line and which
   // should be added as comments before the next token.
Index: lib/Format/UnwrappedLineParser.cpp

[clang-tools-extra] r308290 - Add autoload cookies for clang-include-fixer lisp functions.

2017-07-18 Thread Manuel Klimek via cfe-commits

Author: klimek
Date: Tue Jul 18 03:15:07 2017
New Revision: 308290

URL: http://llvm.org/viewvc/llvm-project?rev=308290&view=rev
Log:
Add autoload cookies for clang-include-fixer lisp functions.

Annotate all public functions with the autoload magic cookie so that
update-directory-autoloads will find them.

Patch by Brendan O'Dea.

Modified:
clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el

Modified: clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el?rev=308290&r1=308289&r2=308290&view=diff
==
--- clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el (original)
+++ clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el Tue Jul 
18 03:15:07 2017
@@ -58,6 +58,7 @@ This string is passed as -input argument
 (defface clang-include-fixer-highlight '((t :background "green"))
   "Used for highlighting the symbol for which a header file is being added.")
 
+;;;###autoload
 (defun clang-include-fixer ()
   "Invoke the Include Fixer to insert missing C++ headers."
   (interactive)
@@ -66,6 +67,7 @@ This string is passed as -input argument
   (clang-include-fixer--start #'clang-include-fixer--add-header
   "-output-headers"))
 
+;;;###autoload
 (defun clang-include-fixer-at-point ()
   "Invoke the Clang include fixer for the symbol at point."
   (interactive)
@@ -74,6 +76,7 @@ This string is passed as -input argument
   (user-error "No symbol at current location"))
 (clang-include-fixer-from-symbol symbol)))
 
+;;;###autoload
 (defun clang-include-fixer-from-symbol (symbol)
   "Invoke the Clang include fixer for the SYMBOL.
 When called interactively, prompts the user for a symbol."


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

[PATCH] D35420: [OpenCL] Fix access qualifiers metadata for kernel arguments with typedef

2017-07-18 Thread Alexey Sotkin via Phabricator via cfe-commits

AlexeySotkin updated this revision to Diff 107044.
AlexeySotkin added a comment.

Changing case in the variable name


https://reviews.llvm.org/D35420

Files:
  lib/CodeGen/CodeGenFunction.cpp
  test/CodeGenOpenCL/kernel-arg-info.cl


Index: test/CodeGenOpenCL/kernel-arg-info.cl
===
--- test/CodeGenOpenCL/kernel-arg-info.cl
+++ test/CodeGenOpenCL/kernel-arg-info.cl
@@ -61,6 +61,21 @@
 // CHECK-NOT: !kernel_arg_name
 // ARGINFO: !kernel_arg_name ![[MD54:[0-9]+]]
 
+typedef read_only  image1d_t ROImage;
+typedef write_only image1d_t WOImage;
+typedef read_write image1d_t RWImage;
+kernel void foo6(ROImage ro, WOImage wo, RWImage rw) {
+}
+// CHECK: define spir_kernel void @foo6{{[^!]+}}
+// CHECK: !kernel_arg_addr_space ![[MD61:[0-9]+]]
+// CHECK: !kernel_arg_access_qual ![[MD62:[0-9]+]]
+// CHECK: !kernel_arg_type ![[MD63:[0-9]+]]
+// CHECK: !kernel_arg_base_type ![[MD64:[0-9]+]]
+// CHECK: !kernel_arg_type_qual ![[MD65:[0-9]+]]
+// CHECK-NOT: !kernel_arg_name
+// ARGINFO: !kernel_arg_name ![[MD66:[0-9]+]]
+
+
 // CHECK: ![[MD11]] = !{i32 1, i32 0, i32 0, i32 2, i32 1, i32 1}
 // CHECK: ![[MD12]] = !{!"none", !"none", !"none", !"none", !"none", !"none"}
 // CHECK: ![[MD13]] = !{!"int*", !"int", !"int", !"float*", !"int*", !"int*"}
@@ -87,3 +102,10 @@
 // CHECK: ![[MD53]] = !{!"image1d_t", !"image1d_t"}
 // ARGINFO: ![[MD54]] = !{!"img1", !"img2"}
 
+// CHECK: ![[MD61]] = !{i32 1, i32 1, i32 1}
+// CHECK: ![[MD62]] = !{!"read_only", !"write_only", !"read_write"}
+// CHECK: ![[MD63]] = !{!"ROImage", !"WOImage", !"RWImage"}
+// CHECK: ![[MD64]] = !{!"image1d_t", !"image1d_t", !"image1d_t"}
+// CHECK: ![[MD65]] = !{!"", !"", !""}
+// ARGINFO: ![[MD66]] = !{!"ro", !"wo", !"rw"}
+
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -620,7 +620,10 @@
 
 // Get image and pipe access qualifier:
 if (ty->isImageType()|| ty->isPipeType()) {
-  const OpenCLAccessAttr *A = parm->getAttr();
+  const Decl *PDecl = parm;
+  if (auto *TD = dyn_cast(ty))
+PDecl = TD->getDecl();
+  const OpenCLAccessAttr *A = PDecl->getAttr();
   if (A && A->isWriteOnly())
 accessQuals.push_back(llvm::MDString::get(Context, "write_only"));
   else if (A && A->isReadWrite())


Index: test/CodeGenOpenCL/kernel-arg-info.cl
===
--- test/CodeGenOpenCL/kernel-arg-info.cl
+++ test/CodeGenOpenCL/kernel-arg-info.cl
@@ -61,6 +61,21 @@
 // CHECK-NOT: !kernel_arg_name
 // ARGINFO: !kernel_arg_name ![[MD54:[0-9]+]]
 
+typedef read_only  image1d_t ROImage;
+typedef write_only image1d_t WOImage;
+typedef read_write image1d_t RWImage;
+kernel void foo6(ROImage ro, WOImage wo, RWImage rw) {
+}
+// CHECK: define spir_kernel void @foo6{{[^!]+}}
+// CHECK: !kernel_arg_addr_space ![[MD61:[0-9]+]]
+// CHECK: !kernel_arg_access_qual ![[MD62:[0-9]+]]
+// CHECK: !kernel_arg_type ![[MD63:[0-9]+]]
+// CHECK: !kernel_arg_base_type ![[MD64:[0-9]+]]
+// CHECK: !kernel_arg_type_qual ![[MD65:[0-9]+]]
+// CHECK-NOT: !kernel_arg_name
+// ARGINFO: !kernel_arg_name ![[MD66:[0-9]+]]
+
+
 // CHECK: ![[MD11]] = !{i32 1, i32 0, i32 0, i32 2, i32 1, i32 1}
 // CHECK: ![[MD12]] = !{!"none", !"none", !"none", !"none", !"none", !"none"}
 // CHECK: ![[MD13]] = !{!"int*", !"int", !"int", !"float*", !"int*", !"int*"}
@@ -87,3 +102,10 @@
 // CHECK: ![[MD53]] = !{!"image1d_t", !"image1d_t"}
 // ARGINFO: ![[MD54]] = !{!"img1", !"img2"}
 
+// CHECK: ![[MD61]] = !{i32 1, i32 1, i32 1}
+// CHECK: ![[MD62]] = !{!"read_only", !"write_only", !"read_write"}
+// CHECK: ![[MD63]] = !{!"ROImage", !"WOImage", !"RWImage"}
+// CHECK: ![[MD64]] = !{!"image1d_t", !"image1d_t", !"image1d_t"}
+// CHECK: ![[MD65]] = !{!"", !"", !""}
+// ARGINFO: ![[MD66]] = !{!"ro", !"wo", !"rw"}
+
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -620,7 +620,10 @@
 
 // Get image and pipe access qualifier:
 if (ty->isImageType()|| ty->isPipeType()) {
-  const OpenCLAccessAttr *A = parm->getAttr();
+  const Decl *PDecl = parm;
+  if (auto *TD = dyn_cast(ty))
+PDecl = TD->getDecl();
+  const OpenCLAccessAttr *A = PDecl->getAttr();
   if (A && A->isWriteOnly())
 accessQuals.push_back(llvm::MDString::get(Context, "write_only"));
   else if (A && A->isReadWrite())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35538: [CodeGen][ARM] ARM runtime helper functions are not always soft-fp

2017-07-18 Thread Peter Smith via Phabricator via cfe-commits

peter.smith created this revision.
Herald added subscribers: kristof.beyls, aemerson.

The ARM Runtime ABI document (IHI0043) defines the AEABI floating point helper 
functions in section 4.1 Floating-point library. These functions always use the 
base PCS (soft-fp). However helper functions defined outside of this document 
such as the complex-number multiply and divide helpers are not covered by this 
requirement and should use hard-float PCS if the target is hard-float. All of 
the floating point helper functions that are explicitly soft float are expanded 
in the llvm ARM backend. This change makes clang not force the BuiltinCC to 
AAPCS for AAPCS_VFP.

With this change clang matches the behavior of gcc and the expected calling 
convention of the hard-float compiled compiler-rt and libgcc.a libraries as 
these functions do not have the __attribute__((__pcs__("aapcs"))).

fixes https://bugs.llvm.org/show_bug.cgi?id=28164
fixes test failures in compiler-rt for a hard-float target "divtc3_test.c, 
divdc3_test.c, divxc3_test.c"

I'm very new to clang so the way this has been fixed maybe a bit too 
simplistic. At present none of the builtins that produce calls in clang must 
use the base PCS. If clang ever needs to make a call to one of the AEABI 
defined functions that need base AAPCS then we'll need to identify the function 
when choosing a calling convention.

References:

- GCC list of functions that must be base AAPCS 
https://gcc.gnu.org/viewcvs/gcc/trunk/gcc/config/arm/arm.c?view=co&content-type=text%2Fplain
- Runtime ABI for the ARM Architecture 
http://infocenter.arm.com/help/topic/com.arm.doc.ihi0043d/IHI0043D_rtabi.pdf


https://reviews.llvm.org/D35538

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/complex-math.c


Index: test/CodeGen/complex-math.c
===
--- test/CodeGen/complex-math.c
+++ test/CodeGen/complex-math.c
@@ -2,7 +2,8 @@
 // RUN: %clang_cc1 %s -O1 -emit-llvm -triple x86_64-pc-win64 -o - | FileCheck 
%s --check-prefix=X86
 // RUN: %clang_cc1 %s -O1 -emit-llvm -triple i686-unknown-unknown -o - | 
FileCheck %s --check-prefix=X86
 // RUN: %clang_cc1 %s -O1 -emit-llvm -triple powerpc-unknown-unknown -o - | 
FileCheck %s --check-prefix=PPC
-// RUN: %clang_cc1 %s -O1 -emit-llvm -triple armv7-none-linux-gnueabihf -o - | 
FileCheck %s --check-prefix=ARM
+// RUN %clang_cc1 %s -O1 -emit-llvm -triple armv7-none-linux-gnueabi -o - | 
FileCheck %s --check-prefix=ARM
+// RUN: %clang_cc1 %s -O1 -emit-llvm -triple armv7-none-linux-gnueabihf -o - | 
FileCheck %s --check-prefix=ARMHF
 // RUN: %clang_cc1 %s -O1 -emit-llvm -triple thumbv7k-apple-watchos2.0 -o - 
-target-abi aapcs16 | FileCheck %s --check-prefix=ARM7K
 
 float _Complex add_float_rr(float a, float b) {
@@ -476,8 +477,15 @@
 
 // Check that the libcall will obtain proper calling convention on ARM
 _Complex double foo(_Complex double a, _Complex double b) {
+  // These functions are not defined as floating point helper functions in
+  // Run-time ABI for the ARM architecture document so they must not always
+  // use the base AAPCS
+
   // ARM-LABEL: @foo(
-  // ARM: call arm_aapcscc { double, double } @__muldc3
+  // ARM: call void { double, double } @__muldc3
+
+  // ARMHF-LABEL: @foo(
+  // ARMHF: call { double, double } @__muldc3
 
   // ARM7K-LABEL: @foo(
   // ARM7K: call { double, double } @__muldc3
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -5573,17 +5573,14 @@
   // AAPCS apparently requires runtime support functions to be soft-float, but
   // that's almost certainly for historic reasons (Thumb1 not supporting VFP
   // most likely). It's more convenient for AAPCS16_VFP to be hard-float.
-  switch (getABIKind()) {
-  case APCS:
-  case AAPCS16_VFP:
-if (abiCC != getLLVMDefaultCC())
+
+  // The Run-time ABI for the ARM Architecture section 4.1.2 requires
+  // AEABI-complying FP helper functions to use the base AAPCS
+  // These AEABI functions are expanded in the ARM llvm backend, all the 
builtin
+  // support functions emitted by clang such as the _Complex helpers follow the
+  // abiCC.
+  if (abiCC != getLLVMDefaultCC())
   BuiltinCC = abiCC;
-break;
-  case AAPCS:
-  case AAPCS_VFP:
-BuiltinCC = llvm::CallingConv::ARM_AAPCS;
-break;
-  }
 }
 
 ABIArgInfo ARMABIInfo::classifyArgumentType(QualType Ty,


Index: test/CodeGen/complex-math.c
===
--- test/CodeGen/complex-math.c
+++ test/CodeGen/complex-math.c
@@ -2,7 +2,8 @@
 // RUN: %clang_cc1 %s -O1 -emit-llvm -triple x86_64-pc-win64 -o - | FileCheck %s --check-prefix=X86
 // RUN: %clang_cc1 %s -O1 -emit-llvm -triple i686-unknown-unknown -o - | FileCheck %s --check-prefix=X86
 // RUN: %clang_cc1 %s -O1 -emit-llvm -triple powerpc-unknown-unknown -o - | FileCheck %s --check-prefix=PPC
-// RUN: %clang_cc1 %s -O1 -e

[PATCH] D35541: [CMake] Use #cmakedefine01 for CLANG_ENABLE_(ARCMT|OBJC_REWRITER|STATIC_ANALYZER)

2017-07-18 Thread NAKAMURA Takumi via Phabricator via cfe-commits

chapuni created this revision.
Herald added a subscriber: mgorny.

It'd be better that they are #cmakedefine01 rather than #cmakedefine.
(#if FOO rather than #if defined(FOO))
Then we can find missing #include "clang/Config/config.h" in the future.


Repository:
  rL LLVM

https://reviews.llvm.org/D35541

Files:
  cfe/trunk/include/clang/Config/config.h.cmake
  cfe/trunk/lib/Frontend/Rewrite/FrontendActions.cpp
  cfe/trunk/lib/Frontend/Rewrite/RewriteModernObjC.cpp
  cfe/trunk/lib/Frontend/Rewrite/RewriteObjC.cpp
  cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  cfe/trunk/tools/libclang/ARCMigrate.cpp

Index: cfe/trunk/tools/libclang/ARCMigrate.cpp
===
--- cfe/trunk/tools/libclang/ARCMigrate.cpp
+++ cfe/trunk/tools/libclang/ARCMigrate.cpp
@@ -34,7 +34,7 @@
 //===--===//
 
 CXRemapping clang_getRemappings(const char *migrate_dir_path) {
-#ifndef CLANG_ENABLE_ARCMT
+#if !CLANG_ENABLE_ARCMT
   llvm::errs() << "error: feature not enabled in this build\n";
   return nullptr;
 #else
@@ -77,7 +77,7 @@
 
 CXRemapping clang_getRemappingsFromFileList(const char **filePaths,
 unsigned numFiles) {
-#ifndef CLANG_ENABLE_ARCMT
+#if !CLANG_ENABLE_ARCMT
   llvm::errs() << "error: feature not enabled in this build\n";
   return nullptr;
 #else
Index: cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -94,27 +94,27 @@
 
   case RewriteMacros:  return llvm::make_unique();
   case RewriteTest:return llvm::make_unique();
-#ifdef CLANG_ENABLE_OBJC_REWRITER
+#if CLANG_ENABLE_OBJC_REWRITER
   case RewriteObjC:return llvm::make_unique();
 #else
   case RewriteObjC:Action = "RewriteObjC"; break;
 #endif
-#ifdef CLANG_ENABLE_ARCMT
+#if CLANG_ENABLE_ARCMT
   case MigrateSource:
 return llvm::make_unique();
 #else
   case MigrateSource:  Action = "MigrateSource"; break;
 #endif
-#ifdef CLANG_ENABLE_STATIC_ANALYZER
+#if CLANG_ENABLE_STATIC_ANALYZER
   case RunAnalysis:return llvm::make_unique();
 #else
   case RunAnalysis:Action = "RunAnalysis"; break;
 #endif
   case RunPreprocessorOnly:return llvm::make_unique();
   }
 
-#if !defined(CLANG_ENABLE_ARCMT) || !defined(CLANG_ENABLE_STATIC_ANALYZER) \
-  || !defined(CLANG_ENABLE_OBJC_REWRITER)
+#if !CLANG_ENABLE_ARCMT || !CLANG_ENABLE_STATIC_ANALYZER \
+  || !CLANG_ENABLE_OBJC_REWRITER
   CI.getDiagnostics().Report(diag::err_fe_action_not_available) << Action;
   return 0;
 #else
@@ -135,7 +135,7 @@
 Act = llvm::make_unique(std::move(Act));
   }
   
-#ifdef CLANG_ENABLE_ARCMT
+#if CLANG_ENABLE_ARCMT
   if (CI.getFrontendOpts().ProgramAction != frontend::MigrateSource &&
   CI.getFrontendOpts().ProgramAction != frontend::GeneratePCH) {
 // Potentially wrap the base FE action in an ARC Migrate Tool action.
@@ -227,7 +227,7 @@
 llvm::cl::ParseCommandLineOptions(NumArgs + 1, Args.get());
   }
 
-#ifdef CLANG_ENABLE_STATIC_ANALYZER
+#if CLANG_ENABLE_STATIC_ANALYZER
   // Honor -analyzer-checker-help.
   // This should happen AFTER plugins have been loaded!
   if (Clang->getAnalyzerOpts()->ShowCheckerHelp) {
Index: cfe/trunk/lib/Frontend/Rewrite/RewriteObjC.cpp
===
--- cfe/trunk/lib/Frontend/Rewrite/RewriteObjC.cpp
+++ cfe/trunk/lib/Frontend/Rewrite/RewriteObjC.cpp
@@ -30,7 +30,7 @@
 #include "llvm/Support/raw_ostream.h"
 #include 
 
-#ifdef CLANG_ENABLE_OBJC_REWRITER
+#if CLANG_ENABLE_OBJC_REWRITER
 
 using namespace clang;
 using llvm::utostr;
Index: cfe/trunk/lib/Frontend/Rewrite/RewriteModernObjC.cpp
===
--- cfe/trunk/lib/Frontend/Rewrite/RewriteModernObjC.cpp
+++ cfe/trunk/lib/Frontend/Rewrite/RewriteModernObjC.cpp
@@ -31,7 +31,7 @@
 #include "llvm/Support/raw_ostream.h"
 #include 
 
-#ifdef CLANG_ENABLE_OBJC_REWRITER
+#if CLANG_ENABLE_OBJC_REWRITER
 
 using namespace clang;
 using llvm::utostr;
Index: cfe/trunk/lib/Frontend/Rewrite/FrontendActions.cpp
===
--- cfe/trunk/lib/Frontend/Rewrite/FrontendActions.cpp
+++ cfe/trunk/lib/Frontend/Rewrite/FrontendActions.cpp
@@ -154,7 +154,7 @@
   return true;
 }
 
-#ifdef CLANG_ENABLE_OBJC_REWRITER
+#if CLANG_ENABLE_OBJC_REWRITER
 
 std::unique_ptr
 RewriteObjCAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) {
Index: cfe/trunk/include/clang/Config/config.h.cmake
===
--- cfe/trunk/include/clang/Config/config.h.cmake
+++ cfe/trunk/include/clang/Config/config.h.cmake
@@ -57,8 +57,8 @@
 #cmakedefine01 ENABLE_

[PATCH] D35542: libcxxabi: Suppress LLVM_ENABLE_MODULES

2017-07-18 Thread NAKAMURA Takumi via Phabricator via cfe-commits

chapuni created this revision.
Herald added a subscriber: mgorny.

It's copypasto from libcxx.
There's no serious issue. I'd like just to keep module.cache clean for in-tree 
libcxx builds.


Repository:
  rL LLVM

https://reviews.llvm.org/D35542

Files:
  libcxxabi/trunk/CMakeLists.txt


Index: libcxxabi/trunk/CMakeLists.txt
===
--- libcxxabi/trunk/CMakeLists.txt
+++ libcxxabi/trunk/CMakeLists.txt
@@ -345,6 +345,12 @@
   endif()
 endif()
 
+if (LLVM_ENABLE_MODULES)
+  # Ignore that the rest of the modules flags are now unused.
+  add_compile_flags_if_supported(-Wno-unused-command-line-argument)
+  add_compile_flags(-fno-modules)
+endif()
+
 set(LIBCXXABI_HAS_UNDEFINED_SYMBOLS OFF)
 if ((NOT LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS)
 OR (LIBCXXABI_BUILD_EXTERNAL_THREAD_LIBRARY AND LIBCXXABI_ENABLE_SHARED)


Index: libcxxabi/trunk/CMakeLists.txt
===
--- libcxxabi/trunk/CMakeLists.txt
+++ libcxxabi/trunk/CMakeLists.txt
@@ -345,6 +345,12 @@
   endif()
 endif()
 
+if (LLVM_ENABLE_MODULES)
+  # Ignore that the rest of the modules flags are now unused.
+  add_compile_flags_if_supported(-Wno-unused-command-line-argument)
+  add_compile_flags(-fno-modules)
+endif()
+
 set(LIBCXXABI_HAS_UNDEFINED_SYMBOLS OFF)
 if ((NOT LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS)
 OR (LIBCXXABI_BUILD_EXTERNAL_THREAD_LIBRARY AND LIBCXXABI_ENABLE_SHARED)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35216: [analyzer] Escape symbols when creating std::initializer_list.

2017-07-18 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ added a comment.

> Approach (2): We could teach the Store to scan itself for bindings to 
> metadata-symbolic-based regions during scanReachableSymbols() whenever a 
> region turns out to be reachable. This requires no work on checker side, but 
> it sounds performance-heavy.

Nope, this approach is wrong. Metadata symbols may become out-of-date: when the 
object changes, metadata symbols attached to it aren't changing (because 
symbols simply don't change). The same metadata may have different symbols to 
denote its value in different moments of time, but at most one of them 
represents the actual metadata value. So we'd be escaping more stuff than 
necessary.

If only we had "ghost fields" 
(http://lists.llvm.org/pipermail/cfe-dev/2016-May/049000.html), it would have 
been much easier, because the ghost field would only contain the actual 
metadata, and the Store would always know about it. This example adds to my 
belief that ghost fields are exactly what we need for most C++ checkers.


https://reviews.llvm.org/D35216



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

[PATCH] D33440: clang-format: better handle statement and namespace macros

2017-07-18 Thread Daniel Jasper via Phabricator via cfe-commits

djasper added a comment.

So, there are two things in this patch: Statement macros and namespace macros. 
Lets break this out and handle them individually. They really aren't related 
that much.

Statement macros:
I think clang-format's handling here is good enough. clang-format does not 
insert the line break, but it also doesn't remove it. I am not 100% sure here, 
so I an be convinced. But I want to understand the use cases better. Do you 
expect people to run into this frequently? I am essentially trying to 
understand whether the cost of an extra option is worth the benefit it is 
giving.

Namespace macros:
How important are the automatic closing comments to you? I'd say that we should 
punt on that and leave it to the user to fix comments of these. And then, we 
could try to make the things we already have in MacroBlockBegin detect whether 
it ends with an opening brace and not need an extra list here. What do you 
think?




Comment at: unittests/Format/FormatTest.cpp:1483
 
+  EXPECT_EQ("namespace out { namespace in {\n"
+"}} // namespace out::in",

What's the difference here?


https://reviews.llvm.org/D33440



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

[PATCH] D35546: [AArch64] Produce correct defaultlib directives for windows in MSVC style

2017-07-18 Thread Martin Storsjö via Phabricator via cfe-commits

mstorsjo created this revision.
Herald added subscribers: kristof.beyls, rengolin, aemerson.

https://reviews.llvm.org/D35546

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/pragma-comment.c


Index: test/CodeGen/pragma-comment.c
===
--- test/CodeGen/pragma-comment.c
+++ test/CodeGen/pragma-comment.c
@@ -4,6 +4,7 @@
 // RUN: %clang_cc1 %s -triple thumbv7-linux-gnueabihf -fms-extensions 
-emit-llvm -o - | FileCheck -check-prefix LINUX %s
 // RUN: %clang_cc1 %s -triple i686-pc-linux -fms-extensions -emit-llvm -o - | 
FileCheck -check-prefix LINUX %s
 // RUN: %clang_cc1 %s -triple x86_64-scei-ps4 -fms-extensions -emit-llvm -o - 
| FileCheck -check-prefix PS4 %s
+// RUN: %clang_cc1 %s -triple aarch64-windows-msvc -fms-extensions -emit-llvm 
-o - | FileCheck %s
 
 #pragma comment(lib, "msvcrt.lib")
 #pragma comment(lib, "kernel32")
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -4860,6 +4860,22 @@
 
   bool doesReturnSlotInterfereWithArgs() const override { return false; }
 };
+
+class WindowsAArch64TargetCodeGenInfo : public AArch64TargetCodeGenInfo {
+public:
+  WindowsAArch64TargetCodeGenInfo(CodeGenTypes &CGT, AArch64ABIInfo::ABIKind K)
+  : AArch64TargetCodeGenInfo(CGT, K) {}
+
+  void getDependentLibraryOption(llvm::StringRef Lib,
+ llvm::SmallString<24> &Opt) const override {
+Opt = "/DEFAULTLIB:" + qualifyWindowsLibrary(Lib);
+  }
+
+  void getDetectMismatchOption(llvm::StringRef Name, llvm::StringRef Value,
+   llvm::SmallString<32> &Opt) const override {
+Opt = "/FAILIFMISMATCH:\"" + Name.str() + "=" + Value.str() + "\"";
+  }
+};
 }
 
 ABIArgInfo AArch64ABIInfo::classifyArgumentType(QualType Ty) const {
@@ -8508,7 +8524,8 @@
 if (getTarget().getABI() == "darwinpcs")
   Kind = AArch64ABIInfo::DarwinPCS;
 else if (Triple.isOSWindows())
-  Kind = AArch64ABIInfo::Win64;
+  return SetCGInfo(
+  new WindowsAArch64TargetCodeGenInfo(Types, AArch64ABIInfo::Win64));
 
 return SetCGInfo(new AArch64TargetCodeGenInfo(Types, Kind));
   }


Index: test/CodeGen/pragma-comment.c
===
--- test/CodeGen/pragma-comment.c
+++ test/CodeGen/pragma-comment.c
@@ -4,6 +4,7 @@
 // RUN: %clang_cc1 %s -triple thumbv7-linux-gnueabihf -fms-extensions -emit-llvm -o - | FileCheck -check-prefix LINUX %s
 // RUN: %clang_cc1 %s -triple i686-pc-linux -fms-extensions -emit-llvm -o - | FileCheck -check-prefix LINUX %s
 // RUN: %clang_cc1 %s -triple x86_64-scei-ps4 -fms-extensions -emit-llvm -o - | FileCheck -check-prefix PS4 %s
+// RUN: %clang_cc1 %s -triple aarch64-windows-msvc -fms-extensions -emit-llvm -o - | FileCheck %s
 
 #pragma comment(lib, "msvcrt.lib")
 #pragma comment(lib, "kernel32")
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -4860,6 +4860,22 @@
 
   bool doesReturnSlotInterfereWithArgs() const override { return false; }
 };
+
+class WindowsAArch64TargetCodeGenInfo : public AArch64TargetCodeGenInfo {
+public:
+  WindowsAArch64TargetCodeGenInfo(CodeGenTypes &CGT, AArch64ABIInfo::ABIKind K)
+  : AArch64TargetCodeGenInfo(CGT, K) {}
+
+  void getDependentLibraryOption(llvm::StringRef Lib,
+ llvm::SmallString<24> &Opt) const override {
+Opt = "/DEFAULTLIB:" + qualifyWindowsLibrary(Lib);
+  }
+
+  void getDetectMismatchOption(llvm::StringRef Name, llvm::StringRef Value,
+   llvm::SmallString<32> &Opt) const override {
+Opt = "/FAILIFMISMATCH:\"" + Name.str() + "=" + Value.str() + "\"";
+  }
+};
 }
 
 ABIArgInfo AArch64ABIInfo::classifyArgumentType(QualType Ty) const {
@@ -8508,7 +8524,8 @@
 if (getTarget().getABI() == "darwinpcs")
   Kind = AArch64ABIInfo::DarwinPCS;
 else if (Triple.isOSWindows())
-  Kind = AArch64ABIInfo::Win64;
+  return SetCGInfo(
+  new WindowsAArch64TargetCodeGenInfo(Types, AArch64ABIInfo::Win64));
 
 return SetCGInfo(new AArch64TargetCodeGenInfo(Types, Kind));
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference

2017-07-18 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ added inline comments.



Comment at: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:511
+   SSE->getLHS()->getType()->isSignedIntegerOrEnumerationType() ||
+   SSE->getLHS()->getType()->isPointerType()) {
+  return negV->Negate(BV, F);

baloghadamsoftware wrote:
> NoQ wrote:
> > baloghadamsoftware wrote:
> > > NoQ wrote:
> > > > Pointer types are currently treated as unsigned, so i'm not sure you 
> > > > want them here.
> > > For me it seems that pointer differences are still pointer types and they 
> > > are signed. (The range becomes negative upon negative assumption. From 
> > > test `ptr-arith.c`:
> > > 
> > > ```
> > > void use_symbols(int *lhs, int *rhs) {
> > >   clang_analyzer_eval(lhs < rhs); // expected-warning{{UNKNOWN}}
> > >   if (lhs < rhs)
> > > return;
> > >   clang_analyzer_eval(lhs < rhs); // expected-warning{{FALSE}}
> > > 
> > >   clang_analyzer_eval(lhs - rhs); // expected-warning{{UNKNOWN}}
> > >   if ((lhs - rhs) != 5)
> > > return;
> > >   clang_analyzer_eval((lhs - rhs) == 5); // expected-warning{{TRUE}}
> > > }
> > > ```
> > > 
> > > If I put `clang_analyzer_printState()` into the empty line in the middle, 
> > > I get the following range for the difference: `[-9223372036854775808, 
> > > 0]`. If I replace `int*` with `unsigned`, this range becomes `[0, 0]`, so 
> > > `int*` is handled as a signed type here.
> > Umm, yeah, i was wrong.
> > 
> > *looks closer*
> > 
> > `T` is the type of the difference, right? I don't think i'd expect pointer 
> > type as the type of the difference.
> > 
> > Could you add test cases for pointers if you intend to support them (and 
> > maybe for unsigned types)?
> I do not know exactly the type, but if I remove the `T->isPointerType()` 
> condition the test in `ptr_arith.c` will fail with `UNKNOWN`. So the type of 
> the difference is a type that returns `true` from `T->isPointerType()`.
> 
> Pointer tests are already there in `ptr_arith.c`. Should I duplicate them?
I don't see any failing tests when i remove `T->isPointerType()`.

Also this shouldn't be system-specific, because the target triple is hardcoded 
in `ptr-arith.c` runline.

Could you point out which test is failing and dump the type in question 
(`-ast-dump`, or `Type->dump()`, or `llvm::errs() << QualType::getAsString()`, 
or whatever)?


https://reviews.llvm.org/D35110



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

r308266 - [OpenCL] Added extended tests on metadata generation for half data type and arrays.

2017-07-18 Thread Egor Churaev via cfe-commits

Author: echuraev
Date: Mon Jul 17 23:04:01 2017
New Revision: 308266

URL: http://llvm.org/viewvc/llvm-project?rev=308266&view=rev
Log:
[OpenCL] Added extended tests on metadata generation for half data type and 
arrays.

Reviewers: Anastasia

Reviewed By: Anastasia

Subscribers: bader, cfe-commits, yaxunl

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

Modified:
cfe/trunk/test/CodeGenOpenCL/kernel-arg-info.cl

Modified: cfe/trunk/test/CodeGenOpenCL/kernel-arg-info.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/kernel-arg-info.cl?rev=308266&r1=308265&r2=308266&view=diff
==
--- cfe/trunk/test/CodeGenOpenCL/kernel-arg-info.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/kernel-arg-info.cl Mon Jul 17 23:04:01 2017
@@ -1,10 +1,24 @@
 // RUN: %clang_cc1 %s -cl-std=CL2.0 -emit-llvm -o - -triple 
spir-unknown-unknown | FileCheck %s
 // RUN: %clang_cc1 %s -cl-std=CL2.0 -emit-llvm -o - -triple 
spir-unknown-unknown -cl-kernel-arg-info | FileCheck %s -check-prefix ARGINFO
 
-kernel void foo(__global int * restrict X, const int Y, 
-volatile int anotherArg, __constant float * restrict Z,
-__global volatile int * V, __global const int * C) {
-  *X = Y + anotherArg;
+kernel void foo(global int * globalintp, global int * restrict 
globalintrestrictp,
+global const int * globalconstintp,
+global const int * restrict globalconstintrestrictp,
+constant int * constantintp, constant int * restrict 
constantintrestrictp,
+global const volatile int * globalconstvolatileintp,
+global const volatile int * restrict 
globalconstvolatileintrestrictp,
+global volatile int * globalvolatileintp,
+global volatile int * restrict globalvolatileintrestrictp,
+local int * localintp, local int * restrict localintrestrictp,
+local const int * localconstintp,
+local const int * restrict localconstintrestrictp,
+local const volatile int * localconstvolatileintp,
+local const volatile int * restrict 
localconstvolatileintrestrictp,
+local volatile int * localvolatileintp,
+local volatile int * restrict localvolatileintrestrictp,
+int X, const int constint, const volatile int constvolatileint,
+volatile int volatileint) {
+  *globalintrestrictp = constint + volatileint;
 }
 // CHECK: define spir_kernel void @foo{{[^!]+}}
 // CHECK: !kernel_arg_addr_space ![[MD11:[0-9]+]]
@@ -61,11 +75,15 @@ kernel void foo5(myImage img1, write_onl
 // CHECK-NOT: !kernel_arg_name
 // ARGINFO: !kernel_arg_name ![[MD54:[0-9]+]]
 
-// CHECK: ![[MD11]] = !{i32 1, i32 0, i32 0, i32 2, i32 1, i32 1}
-// CHECK: ![[MD12]] = !{!"none", !"none", !"none", !"none", !"none", !"none"}
-// CHECK: ![[MD13]] = !{!"int*", !"int", !"int", !"float*", !"int*", !"int*"}
-// CHECK: ![[MD14]] = !{!"restrict", !"", !"", !"restrict const", !"volatile", 
!"const"}
-// ARGINFO: ![[MD15]] = !{!"X", !"Y", !"anotherArg", !"Z", !"V", !"C"}
+typedef char char16 __attribute__((ext_vector_type(16)));
+__kernel void foo6(__global char16 arg[]) {}
+// CHECK: !kernel_arg_type ![[MD61:[0-9]+]]
+
+// CHECK: ![[MD11]] = !{i32 1, i32 1, i32 1, i32 1, i32 2, i32 2, i32 1, i32 
1, i32 1, i32 1, i32 3, i32 3, i32 3, i32 3, i32 3, i32 3, i32 3, i32 3, i32 0, 
i32 0, i32 0, i32 0}
+// CHECK: ![[MD12]] = !{!"none", !"none", !"none", !"none", !"none", !"none", 
!"none", !"none", !"none", !"none", !"none", !"none", !"none", !"none", 
!"none", !"none", !"none", !"none", !"none", !"none", !"none", !"none"}
+// CHECK: ![[MD13]] = !{!"int*", !"int*", !"int*", !"int*", !"int*", !"int*", 
!"int*", !"int*", !"int*", !"int*", !"int*", !"int*", !"int*", !"int*", 
!"int*", !"int*", !"int*", !"int*", !"int", !"int", !"int", !"int"}
+// CHECK: ![[MD14]] = !{!"", !"restrict", !"const", !"restrict const", 
!"const", !"restrict const", !"const volatile", !"restrict const volatile", 
!"volatile", !"restrict volatile", !"", !"restrict", !"const", !"restrict 
const", !"const volatile", !"restrict const volatile", !"volatile", !"restrict 
volatile", !"", !"", !"", !""}
+// ARGINFO: ![[MD15]] = !{!"globalintp", !"globalintrestrictp", 
!"globalconstintp", !"globalconstintrestrictp", !"constantintp", 
!"constantintrestrictp", !"globalconstvolatileintp", 
!"globalconstvolatileintrestrictp", !"globalvolatileintp", 
!"globalvolatileintrestrictp", !"localintp", !"localintrestrictp", 
!"localconstintp", !"localconstintrestrictp", !"localconstvolatileintp", 
!"localconstvolatileintrestrictp", !"localvolatileintp", 
!"localvolatileintrestrictp", !"X", !"constint", !"constvolatileint", 
!"volatileint"}
 // CHECK: ![[MD21]] = !{i32 1, i32 1, i32 1, i32 1}
 // CHECK: ![[MD22]] = !{!"read_only", !"read_only", !"write_only", 
!"read_write"}
 // CHECK

[PATCH] D35355: Fix templated type alias completion when using global completion cache

2017-07-18 Thread Ivan Donchevskii via Phabricator via cfe-commits

yvvan added a comment.

Ping


https://reviews.llvm.org/D35355



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

[PATCH] D33644: Add default values for function parameter chunks

2017-07-18 Thread Ivan Donchevskii via Phabricator via cfe-commits

yvvan added a comment.

So do we wait until the '=' case is more clear? This change should not break 
anything if it's fixed in either direction (if '=' will be provided always or 
never)


https://reviews.llvm.org/D33644



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

[PATCH] D33644: Add default values for function parameter chunks

2017-07-18 Thread Manuel Klimek via Phabricator via cfe-commits

klimek added a comment.

In https://reviews.llvm.org/D33644#812693, @yvvan wrote:

> So do we wait until the '=' case is more clear? This change should not break 
> anything if it's fixed in either direction (if '=' will be provided always or 
> never)


Ok, if you add a TODO I think this is fine for now.


https://reviews.llvm.org/D33644



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

[PATCH] D35548: [mips] Teach the driver to accept -m(no-)gpopt.

2017-07-18 Thread Simon Dardis via Phabricator via cfe-commits

sdardis created this revision.
Herald added a subscriber: arichardson.

This patch teaches the driver to pass -mgpopt by default to the backend when it
is supported, i.e. we are using -mno-abicalls.


https://reviews.llvm.org/D35548

Files:
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/mips-features.c


Index: test/Driver/mips-features.c
===
--- test/Driver/mips-features.c
+++ test/Driver/mips-features.c
@@ -10,6 +10,31 @@
 // RUN:   | FileCheck --check-prefix=CHECK-MNOABICALLS %s
 // CHECK-MNOABICALLS: "-target-feature" "+noabicalls"
 //
+// -mgpopt
+// RUN: %clang -target mips-linux-gnu -### -c %s -mno-gpopt -mgpopt 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MGPOPT-DEF-ABICALLS %s
+// CHECK-MGPOPT-DEF-ABICALLS-NOT: "-mllvm" "-mgpopt"
+//
+// -mabicalls -mgpopt
+// RUN: %clang -target mips-linux-gnu -### -c %s -mabicalls -mno-gpopt -mgpopt 
2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MGPOPT-EXPLICIT-ABICALLS %s
+// CHECK-MGPOPT-EXPLICIT-ABICALLS-NOT: "-mllvm" "-mgpopt"
+//
+// -mno-abicalls -mgpopt
+// RUN: %clang -target mips-linux-gnu -### -c %s -mno-abicalls -mno-gpopt 
-mgpopt 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MGPOPT %s
+// CHECK-MGPOPT: "-mllvm" "-mgpopt=1"
+//
+// -mno-abicalls -mno-gpopt
+// RUN: %clang -target mips-linux-gnu -### -c %s -mno-abicalls -mgpopt 
-mno-gpopt 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MNOGPOPT %s
+// CHECK-MNOGPOPT: "-mllvm" "-mgpopt=0"
+//
+// -mno-abicalls
+// RUN: %clang -target mips-linux-gnu -### -c %s -mno-abicalls 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MGPOPTDEF %s
+// CHECK-MGPOPTDEF: "-mllvm" "-mgpopt=1"
+//
 // -mips16
 // RUN: %clang -target mips-linux-gnu -### -c %s \
 // RUN: -mno-mips16 -mips16 2>&1 \
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -1462,6 +1462,33 @@
 A->claim();
   }
 
+  Arg *GPOpt = Args.getLastArg(options::OPT_mgpopt, options::OPT_mno_gpopt);
+  Arg *ABICalls = Args.getLastArg(options::OPT_mabicalls, 
options::OPT_mno_abicalls);
+
+  // -mabicalls is the default for many MIPS environments, even with -fno-pic.
+  // -mgpopt is the default for static, -fno-pic environments but these two
+  // options conflict. We want to be certain that -mno-abicalls -mgpopt is
+  // the only case where -mllvm -mgpopt is passed.
+  // NOTE: We need a warning here or in the backend to warn when -mgpopt is
+  //   passed explicitly when compiling something with -mabicalls
+  //   (implictly) in affect. Currently the warning is in the backend.
+  bool WantABICalls =
+  ABICalls && ABICalls->getOption().matches(options::OPT_mabicalls);
+  bool WantGPOpt = GPOpt && GPOpt->getOption().matches(options::OPT_mgpopt);
+  bool DefaultABICalls = !ABICalls;
+  if (!DefaultABICalls && !WantABICalls) {
+if (!GPOpt || WantGPOpt) {
+  CmdArgs.push_back("-mllvm");
+  CmdArgs.push_back("-mgpopt=1");
+} else {
+  CmdArgs.push_back("-mllvm");
+  CmdArgs.push_back("-mgpopt=0");
+}
+
+if (GPOpt)
+  GPOpt->claim();
+  }
+
   if (Arg *A = Args.getLastArg(options::OPT_mcompact_branches_EQ)) {
 StringRef Val = StringRef(A->getValue());
 if (mips::hasCompactBranches(CPUName)) {
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -2035,6 +2035,12 @@
   HelpText<"Use 64-bit floating point registers (MIPS only)">;
 def mfp32 : Flag<["-"], "mfp32">, Group,
   HelpText<"Use 32-bit floating point registers (MIPS only)">;
+def mgpopt : Flag<["-"], "mgpopt">, Group,
+  HelpText<"Use GP relative accesses for symbols known to be in a small"
+   " data section (MIPS)">;
+def mno_gpopt : Flag<["-"], "mno-gpopt">, Group,
+  HelpText<"Do not use GP relative accesses for symbols known to be in a small"
+   " data section (MIPS)">;
 def mnan_EQ : Joined<["-"], "mnan=">, Group;
 def mabicalls : Flag<["-"], "mabicalls">, Group,
   HelpText<"Enable SVR4-style position-independent code (Mips only)">;


Index: test/Driver/mips-features.c
===
--- test/Driver/mips-features.c
+++ test/Driver/mips-features.c
@@ -10,6 +10,31 @@
 // RUN:   | FileCheck --check-prefix=CHECK-MNOABICALLS %s
 // CHECK-MNOABICALLS: "-target-feature" "+noabicalls"
 //
+// -mgpopt
+// RUN: %clang -target mips-linux-gnu -### -c %s -mno-gpopt -mgpopt 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MGPOPT-DEF-ABICALLS %s
+// CHECK-MGPOPT-DEF-ABICALLS-NOT: "-mllvm" "-mgpopt"
+//
+// -mabicalls -mgpopt
+// RUN: %clang -target mips-linux-gnu -### -c %s -mabicalls -mno-gpopt -mgpopt 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MGPOPT-EXPLICIT-ABICALLS %s
+// CHECK-MGPOPT-EXPLICIT-A

[PATCH] D35549: [mips] Add support for -m(no-)local-sdata

2017-07-18 Thread Simon Dardis via Phabricator via cfe-commits

sdardis created this revision.
Herald added a subscriber: arichardson.

Teach the driver to support -m(no-)local-sdata. The backend already matches 
GCC's
default behaviour.


https://reviews.llvm.org/D35549

Files:
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/mips-features.c


Index: test/Driver/mips-features.c
===
--- test/Driver/mips-features.c
+++ test/Driver/mips-features.c
@@ -35,6 +35,21 @@
 // RUN:   | FileCheck --check-prefix=CHECK-MGPOPTDEF %s
 // CHECK-MGPOPTDEF: "-mllvm" "-mgpopt=1"
 //
+// -mgpopt -mno-abicalls -mlocal-sdata
+// RUN: %clang -target mips-linux-gnu -### -c %s -mno-abicalls -mno-gpopt 
-mgpopt -mno-local-sdata -mlocal-sdata 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MLOCALSDATA %s
+// CHECK-MLOCALSDATA: "-mllvm" "-mlocal-sdata=1"
+//
+// -mgpopt -mno-abicalls -mno-local-sdata
+// RUN: %clang -target mips-linux-gnu -### -c %s -mno-abicalls -mno-gpopt 
-mgpopt -mlocal-sdata -mno-local-sdata 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MNOLOCALSDATA %s
+// CHECK-MNOLOCALSDATA: "-mllvm" "-mlocal-sdata=0"
+//
+// -mgpopt -mno-abicalls
+// RUN: %clang -target mips-linux-gnu -### -c %s -mno-abicalls -mgpopt 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MLOCALSDATADEF %s
+// CHECK-MLOCALSDATADEF-NOT: "-mllvm" "-mlocal-sdata"
+//
 // -mips16
 // RUN: %clang -target mips-linux-gnu -### -c %s \
 // RUN: -mno-mips16 -mips16 2>&1 \
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -1464,7 +1464,8 @@
 
   Arg *GPOpt = Args.getLastArg(options::OPT_mgpopt, options::OPT_mno_gpopt);
   Arg *ABICalls = Args.getLastArg(options::OPT_mabicalls, 
options::OPT_mno_abicalls);
-
+  Arg *LocalSData =
+  Args.getLastArg(options::OPT_mlocal_sdata, options::OPT_mno_local_sdata);
   // -mabicalls is the default for many MIPS environments, even with -fno-pic.
   // -mgpopt is the default for static, -fno-pic environments but these two
   // options conflict. We want to be certain that -mno-abicalls -mgpopt is
@@ -1487,6 +1488,16 @@
 
 if (GPOpt)
   GPOpt->claim();
+
+if (LocalSData) {
+  CmdArgs.push_back("-mllvm");
+  if (LocalSData->getOption().matches(options::OPT_mlocal_sdata)) {
+CmdArgs.push_back("-mlocal-sdata=1");
+  } else {
+CmdArgs.push_back("-mlocal-sdata=0");
+  }
+  LocalSData->claim();
+}
   }
 
   if (Arg *A = Args.getLastArg(options::OPT_mcompact_branches_EQ)) {
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -2041,6 +2041,10 @@
 def mno_gpopt : Flag<["-"], "mno-gpopt">, Group,
   HelpText<"Do not use GP relative accesses for symbols known to be in a small"
" data section (MIPS)">;
+def mlocal_sdata : Flag<["-"], "mlocal-sdata">, Group,
+  HelpText<"Extend the -G behaviour to object local data (MIPS)">;
+def mno_local_sdata : Flag<["-"], "mno-local-sdata">, Group,
+  HelpText<"Do not extend the -G behaviour to object local data (MIPS)">;
 def mnan_EQ : Joined<["-"], "mnan=">, Group;
 def mabicalls : Flag<["-"], "mabicalls">, Group,
   HelpText<"Enable SVR4-style position-independent code (Mips only)">;


Index: test/Driver/mips-features.c
===
--- test/Driver/mips-features.c
+++ test/Driver/mips-features.c
@@ -35,6 +35,21 @@
 // RUN:   | FileCheck --check-prefix=CHECK-MGPOPTDEF %s
 // CHECK-MGPOPTDEF: "-mllvm" "-mgpopt=1"
 //
+// -mgpopt -mno-abicalls -mlocal-sdata
+// RUN: %clang -target mips-linux-gnu -### -c %s -mno-abicalls -mno-gpopt -mgpopt -mno-local-sdata -mlocal-sdata 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MLOCALSDATA %s
+// CHECK-MLOCALSDATA: "-mllvm" "-mlocal-sdata=1"
+//
+// -mgpopt -mno-abicalls -mno-local-sdata
+// RUN: %clang -target mips-linux-gnu -### -c %s -mno-abicalls -mno-gpopt -mgpopt -mlocal-sdata -mno-local-sdata 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MNOLOCALSDATA %s
+// CHECK-MNOLOCALSDATA: "-mllvm" "-mlocal-sdata=0"
+//
+// -mgpopt -mno-abicalls
+// RUN: %clang -target mips-linux-gnu -### -c %s -mno-abicalls -mgpopt 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MLOCALSDATADEF %s
+// CHECK-MLOCALSDATADEF-NOT: "-mllvm" "-mlocal-sdata"
+//
 // -mips16
 // RUN: %clang -target mips-linux-gnu -### -c %s \
 // RUN: -mno-mips16 -mips16 2>&1 \
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -1464,7 +1464,8 @@
 
   Arg *GPOpt = Args.getLastArg(options::OPT_mgpopt, options::OPT_mno_gpopt);
   Arg *ABICalls = Args.getLastArg(options::OPT_mabicalls, options::OPT_mno_abicalls);
-
+  Arg *LocalSData =
+

[PATCH] D35550: [mips] Add support for -m(no-)extern-data.

2017-07-18 Thread Simon Dardis via Phabricator via cfe-commits

sdardis created this revision.
Herald added a subscriber: arichardson.

Add support for -m(no-)extern-data when using -mgpopt in the driver. It is
enabled by default in the backend.


https://reviews.llvm.org/D35550

Files:
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/mips-features.c


Index: test/Driver/mips-features.c
===
--- test/Driver/mips-features.c
+++ test/Driver/mips-features.c
@@ -50,6 +50,21 @@
 // RUN:   | FileCheck --check-prefix=CHECK-MLOCALSDATADEF %s
 // CHECK-MLOCALSDATADEF-NOT: "-mllvm" "-mlocal-sdata"
 //
+// -mno-abicalls -mgpopt -mextern-sdata
+// RUN: %clang -target mips-linux-gnu -### -c %s -mno-abicalls -mgpopt 
-mno-extern-sdata -mextern-sdata 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MEXTERNSDATA %s
+// CHECK-MEXTERNSDATA: "-mllvm" "-mextern-sdata=1"
+//
+// -mno-abicalls -mgpopt -mno-extern-sdata
+// RUN: %clang -target mips-linux-gnu -### -c %s -mno-abicalls -mgpopt 
-mextern-sdata -mno-extern-sdata 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MNOEXTERNSDATA %s
+// CHECK-MNOEXTERNSDATA: "-mllvm" "-mextern-sdata=0"
+//
+// -mno-abicalls -mgpopt
+// RUN: %clang -target mips-linux-gnu -### -c %s -mno-abicalls -mgpopt 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MEXTERNSDATADEF %s
+// CHECK-MEXTERNSDATADEF-NOT: "-mllvm" "-mextern-sdata"
+//
 // -mips16
 // RUN: %clang -target mips-linux-gnu -### -c %s \
 // RUN: -mno-mips16 -mips16 2>&1 \
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -1466,6 +1466,7 @@
   Arg *ABICalls = Args.getLastArg(options::OPT_mabicalls, 
options::OPT_mno_abicalls);
   Arg *LocalSData =
   Args.getLastArg(options::OPT_mlocal_sdata, options::OPT_mno_local_sdata);
+  Arg *ExternSData = Args.getLastArg(options::OPT_mextern_sdata, 
options::OPT_mno_extern_sdata);
   // -mabicalls is the default for many MIPS environments, even with -fno-pic.
   // -mgpopt is the default for static, -fno-pic environments but these two
   // options conflict. We want to be certain that -mno-abicalls -mgpopt is
@@ -1498,6 +1499,16 @@
   }
   LocalSData->claim();
 }
+
+if (ExternSData) {
+  CmdArgs.push_back("-mllvm");
+  if (ExternSData->getOption().matches(options::OPT_mextern_sdata)) {
+CmdArgs.push_back("-mextern-sdata=1");
+  } else {
+CmdArgs.push_back("-mextern-sdata=0");
+  }
+  ExternSData->claim();
+}
   }
 
   if (Arg *A = Args.getLastArg(options::OPT_mcompact_branches_EQ)) {
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -2045,6 +2045,12 @@
   HelpText<"Extend the -G behaviour to object local data (MIPS)">;
 def mno_local_sdata : Flag<["-"], "mno-local-sdata">, Group,
   HelpText<"Do not extend the -G behaviour to object local data (MIPS)">;
+def mextern_sdata : Flag<["-"], "mextern-sdata">, Group,
+  HelpText<"Assume that externally defined data is in the small data if it"
+   " meets the -G  threshold (MIPS)">;
+def mno_extern_sdata : Flag<["-"], "mno-extern-sdata">, Group,
+  HelpText<"Do not assume that externally defined data is in the small data if"
+   " it meets the -G  threshold (MIPS)">;
 def mnan_EQ : Joined<["-"], "mnan=">, Group;
 def mabicalls : Flag<["-"], "mabicalls">, Group,
   HelpText<"Enable SVR4-style position-independent code (Mips only)">;


Index: test/Driver/mips-features.c
===
--- test/Driver/mips-features.c
+++ test/Driver/mips-features.c
@@ -50,6 +50,21 @@
 // RUN:   | FileCheck --check-prefix=CHECK-MLOCALSDATADEF %s
 // CHECK-MLOCALSDATADEF-NOT: "-mllvm" "-mlocal-sdata"
 //
+// -mno-abicalls -mgpopt -mextern-sdata
+// RUN: %clang -target mips-linux-gnu -### -c %s -mno-abicalls -mgpopt -mno-extern-sdata -mextern-sdata 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MEXTERNSDATA %s
+// CHECK-MEXTERNSDATA: "-mllvm" "-mextern-sdata=1"
+//
+// -mno-abicalls -mgpopt -mno-extern-sdata
+// RUN: %clang -target mips-linux-gnu -### -c %s -mno-abicalls -mgpopt -mextern-sdata -mno-extern-sdata 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MNOEXTERNSDATA %s
+// CHECK-MNOEXTERNSDATA: "-mllvm" "-mextern-sdata=0"
+//
+// -mno-abicalls -mgpopt
+// RUN: %clang -target mips-linux-gnu -### -c %s -mno-abicalls -mgpopt 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MEXTERNSDATADEF %s
+// CHECK-MEXTERNSDATADEF-NOT: "-mllvm" "-mextern-sdata"
+//
 // -mips16
 // RUN: %clang -target mips-linux-gnu -### -c %s \
 // RUN: -mno-mips16 -mips16 2>&1 \
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -146

[PATCH] D35519: Add SEMA checking to attribute 'target'

2017-07-18 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2389-2397
 def warn_unsupported_target_attribute
 : Warning<"Ignoring unsupported '%0' in the target attribute string">,
 InGroup;
+def warn_duplicate_target_attribute
+: Warning<"Ignoring duplicate '%0' in the target attribute string">,
+InGroup;
+def warn_unsupported_target_architecture

All of these (including the one you didn't have to touch) should be using 
"ignoring" instead of "Ignoring".



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2395
+InGroup;
+def warn_unsupported_target_architecture
+: Warning<"Ignoring unsupported architecture '%0' in the target attribute "

This name should have something to do with attributes. Might make sense to 
combine with `warn_unsupported_target_attribute` using a `%select` given the 
similarity in use case.



Comment at: lib/Basic/Targets.cpp:1015-1018
+  // Note: GCC recognizes the following additional cpus:
+  //  401, 403, 405, 405fp, 440fp, 464, 464fp, 476, 476fp, 505, 740, 801,
+  //  821, 823, 8540, 8548, e300c2, e300c3, e500mc64, e6500, 860, cell,
+  //  titan, rs64.

I think this comment should be hoisted up to `isValidCPUName()`.



Comment at: lib/Basic/Targets.cpp:6416
+  bool setCPU(const std::string &Name) override {
+return isValidCPUName(Name);
+  }

It's rather strange that this doesn't actually set the CPU, but I can't see 
that the original code did either, so maybe it's fine?



Comment at: lib/Basic/Targets.cpp:9617
+bool isValid = isValidCPUName(Name);
+if (isValid) this->CPU = Name;
+return isValid;

You can drop the `this`, and I would put the `CPU = Name` onto its own line 
(not certain if that's a personal preference or a general community style).



Comment at: lib/Sema/SemaDeclAttr.cpp:3004
+  TargetAttr::ParsedTargetAttr ParsedAttrs = TargetAttr::parse(AttrStr);
+  if(ParsedAttrs.DuplicateArchitecture)
+return Diag(LiteralLoc, diag::warn_duplicate_target_attribute) << "arch";

Missing a space after the `if`.



Comment at: lib/Sema/SemaDeclAttr.cpp:3005
+  if(ParsedAttrs.DuplicateArchitecture)
+return Diag(LiteralLoc, diag::warn_duplicate_target_attribute) << "arch";
+

Should this be `"arch="` to be consistent with the previous diagnostic on line 
3001?



Comment at: lib/Sema/SemaDeclAttr.cpp:3007
+
+  if (!ParsedAttrs.Architecture.empty() &&
+  !Context.getTargetInfo().isValidCPUName(ParsedAttrs.Architecture))

Is the attribute valid when the architecture is empty?



Comment at: lib/Sema/SemaDeclAttr.cpp:3012
+
+  for (auto &Feature : ParsedAttrs.Features) {
+auto CurFeature = StringRef(Feature).drop_front();// remove + or -.

Can this be made `const auto &`?



Comment at: lib/Sema/SemaDeclAttr.cpp:3013
+  for (auto &Feature : ParsedAttrs.Features) {
+auto CurFeature = StringRef(Feature).drop_front();// remove + or -.
+if (!Context.getTargetInfo().isValidFeatureName(CurFeature))

Insert a space before the comment.



Comment at: test/Sema/attr-target.c:9
+int __attribute__((target("woof"))) bark() {  return 4; }//expected-warning 
{{Ignoring unsupported 'woof' in the target attribute string}}
+
 

Please add a test case where there's a duplicate arch and an empty arch.


https://reviews.llvm.org/D35519



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

[PATCH] D33644: Add default values for function parameter chunks

2017-07-18 Thread Ivan Donchevskii via Phabricator via cfe-commits

yvvan updated this revision to Diff 107077.
yvvan added a comment.

Add TODO about '=' check


https://reviews.llvm.org/D33644

Files:
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/functions.cpp
  test/Index/code-completion.cpp
  test/Index/complete-optional-params.cpp

Index: test/Index/complete-optional-params.cpp
===
--- test/Index/complete-optional-params.cpp
+++ test/Index/complete-optional-params.cpp
@@ -6,15 +6,19 @@
 void baz(int a = 42, ...);
 struct S{ S(int a = 42, int = 42) {} };
 
+class Bar1 { public: Bar1() {} }; class Bar2;
+void foo_2(Bar1 b1 = Bar1(), Bar2 b2 = Bar2());
+
 int main() {
 foo(42, 42);
 bar(42, 42, 42);
 baz(42, 42, 42);
 S s(42, 42);
+foo_2();
 }
 
-// RUN: c-index-test -code-completion-at=%s:10:9 %s | FileCheck -check-prefix=CHECK-CC1 %s
-// CHECK-CC1: OverloadCandidate:{ResultType void}{Text foo}{LeftParen (}{Optional {CurrentParameter int a}{Optional {Comma , }{Placeholder int}}}{RightParen )} (1)
+// RUN: c-index-test -code-completion-at=%s:13:9 %s | FileCheck -check-prefix=CHECK-CC1 %s
+// CHECK-CC1: OverloadCandidate:{ResultType void}{Text foo}{LeftParen (}{Optional {CurrentParameter int a = 42}{Optional {Comma , }{Placeholder int = 42}}}{RightParen )} (1)
 // CHECK-CC1: Completion contexts:
 // CHECK-CC1-NEXT: Any type
 // CHECK-CC1-NEXT: Any value
@@ -25,8 +29,8 @@
 // CHECK-CC1-NEXT: Nested name specifier
 // CHECK-CC1-NEXT: Objective-C interface
 
-// RUN: c-index-test -code-completion-at=%s:11:9 %s | FileCheck -check-prefix=CHECK-CC2 %s
-// CHECK-CC2: OverloadCandidate:{ResultType void}{Text bar}{LeftParen (}{CurrentParameter int a}{Optional {Comma , }{Placeholder int b}{Optional {Comma , }{Placeholder int c}}}{RightParen )} (1)
+// RUN: c-index-test -code-completion-at=%s:14:9 %s | FileCheck -check-prefix=CHECK-CC2 %s
+// CHECK-CC2: OverloadCandidate:{ResultType void}{Text bar}{LeftParen (}{CurrentParameter int a}{Optional {Comma , }{Placeholder int b = 42}{Optional {Comma , }{Placeholder int c = 42}}}{RightParen )} (1)
 // CHECK-CC2: Completion contexts:
 // CHECK-CC2-NEXT: Any type
 // CHECK-CC2-NEXT: Any value
@@ -37,8 +41,8 @@
 // CHECK-CC2-NEXT: Nested name specifier
 // CHECK-CC2-NEXT: Objective-C interface
 
-// RUN: c-index-test -code-completion-at=%s:11:16 %s | FileCheck -check-prefix=CHECK-CC3 %s
-// CHECK-CC3: OverloadCandidate:{ResultType void}{Text bar}{LeftParen (}{Placeholder int a}{Optional {Comma , }{Placeholder int b}{Optional {Comma , }{CurrentParameter int c}}}{RightParen )} (1)
+// RUN: c-index-test -code-completion-at=%s:14:16 %s | FileCheck -check-prefix=CHECK-CC3 %s
+// CHECK-CC3: OverloadCandidate:{ResultType void}{Text bar}{LeftParen (}{Placeholder int a}{Optional {Comma , }{Placeholder int b = 42}{Optional {Comma , }{CurrentParameter int c = 42}}}{RightParen )} (1)
 // CHECK-CC3: Completion contexts:
 // CHECK-CC3-NEXT: Any type
 // CHECK-CC3-NEXT: Any value
@@ -49,8 +53,8 @@
 // CHECK-CC3-NEXT: Nested name specifier
 // CHECK-CC3-NEXT: Objective-C interface
 
-// RUN: c-index-test -code-completion-at=%s:12:16 %s | FileCheck -check-prefix=CHECK-CC4 %s
-// CHECK-CC4: OverloadCandidate:{ResultType void}{Text baz}{LeftParen (}{Optional {Placeholder int a}{Optional {Comma , }{CurrentParameter ...}}}{RightParen )} (1)
+// RUN: c-index-test -code-completion-at=%s:15:16 %s | FileCheck -check-prefix=CHECK-CC4 %s
+// CHECK-CC4: OverloadCandidate:{ResultType void}{Text baz}{LeftParen (}{Optional {Placeholder int a = 42}{Optional {Comma , }{CurrentParameter ...}}}{RightParen )} (1)
 // CHECK-CC4: Completion contexts:
 // CHECK-CC4-NEXT: Any type
 // CHECK-CC4-NEXT: Any value
@@ -61,8 +65,8 @@
 // CHECK-CC4-NEXT: Nested name specifier
 // CHECK-CC4-NEXT: Objective-C interface
 
-// RUN: c-index-test -code-completion-at=%s:13:9 %s | FileCheck -check-prefix=CHECK-CC5 %s
-// CHECK-CC5: OverloadCandidate:{Text S}{LeftParen (}{Optional {CurrentParameter int a}{Optional {Comma , }{Placeholder int}}}{RightParen )} (1)
+// RUN: c-index-test -code-completion-at=%s:16:9 %s | FileCheck -check-prefix=CHECK-CC5 %s
+// CHECK-CC5: OverloadCandidate:{Text S}{LeftParen (}{Optional {CurrentParameter int a = 42}{Optional {Comma , }{Placeholder int = 42}}}{RightParen )} (1)
 // CHECK-CC5: OverloadCandidate:{Text S}{LeftParen (}{CurrentParameter const S &}{RightParen )} (1)
 // CHECK-CC5: Completion contexts:
 // CHECK-CC5-NEXT: Any type
@@ -73,3 +77,15 @@
 // CHECK-CC5-NEXT: Class name
 // CHECK-CC5-NEXT: Nested name specifier
 // CHECK-CC5-NEXT: Objective-C interface
+
+// RUN: c-index-test -code-completion-at=%s:17:11 %s | FileCheck -check-prefix=CHECK-CC6 %s
+// CHECK-CC6: FunctionDecl:{ResultType void}{TypedText foo_2}{LeftParen (}{Optional {Placeholder Bar1 b1 = Bar1()}{Optional {Comma , }{Placeholder Bar2 b2}}}{RightParen )} (50)
+// CHECK-CC6: Completion contexts:
+// CHECK-CC6-NEXT: Any type
+// CHECK-CC6-NEXT: Any value
+// CHECK-CC6-NEXT: Enum tag
+// CHECK-CC6-NEXT:

[PATCH] D34439: Add GCC's noexcept-type alias for c++1z-compat-mangling

2017-07-18 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits

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

LGTM.


https://reviews.llvm.org/D34439



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

[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference

2017-07-18 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware updated this revision to Diff 107079.
baloghadamsoftware added a comment.

I think I checked the type of the left side of the difference, not the 
difference itself. Thus the difference is not a pointer type, it is a signed 
integer type, the tests pass when I remove that line.


https://reviews.llvm.org/D35110

Files:
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  test/Analysis/constraint_manager_negate_difference.c
  test/Analysis/ptr-arith.c

Index: test/Analysis/ptr-arith.c
===
--- test/Analysis/ptr-arith.c
+++ test/Analysis/ptr-arith.c
@@ -265,49 +265,26 @@
   clang_analyzer_eval((rhs - lhs) > 0); // expected-warning{{TRUE}}
 }
 
-//---
-// False positives
-//---
-
 void zero_implies_reversed_equal(int *lhs, int *rhs) {
   clang_analyzer_eval((rhs - lhs) == 0); // expected-warning{{UNKNOWN}}
   if ((rhs - lhs) == 0) {
-#ifdef ANALYZER_CM_Z3
 clang_analyzer_eval(rhs != lhs); // expected-warning{{FALSE}}
 clang_analyzer_eval(rhs == lhs); // expected-warning{{TRUE}}
-#else
-clang_analyzer_eval(rhs != lhs); // expected-warning{{UNKNOWN}}
-clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
-#endif
 return;
   }
   clang_analyzer_eval((rhs - lhs) == 0); // expected-warning{{FALSE}}
-#ifdef ANALYZER_CM_Z3
   clang_analyzer_eval(rhs == lhs); // expected-warning{{FALSE}}
   clang_analyzer_eval(rhs != lhs); // expected-warning{{TRUE}}
-#else
-  clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
-  clang_analyzer_eval(rhs != lhs); // expected-warning{{UNKNOWN}}
-#endif
 }
 
 void canonical_equal(int *lhs, int *rhs) {
   clang_analyzer_eval(lhs == rhs); // expected-warning{{UNKNOWN}}
   if (lhs == rhs) {
-#ifdef ANALYZER_CM_Z3
 clang_analyzer_eval(rhs == lhs); // expected-warning{{TRUE}}
-#else
-clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
-#endif
 return;
   }
   clang_analyzer_eval(lhs == rhs); // expected-warning{{FALSE}}
-
-#ifdef ANALYZER_CM_Z3
   clang_analyzer_eval(rhs == lhs); // expected-warning{{FALSE}}
-#else
-  clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
-#endif
 }
 
 void compare_element_region_and_base(int *p) {
Index: test/Analysis/constraint_manager_negate_difference.c
===
--- /dev/null
+++ test/Analysis/constraint_manager_negate_difference.c
@@ -0,0 +1,39 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
+
+void equal(int m, int n) {
+  if (m != n)
+return;
+  clang_analyzer_eval(n == m); // expected-warning{{TRUE}}
+}
+
+void non_equal(int m, int n) {
+  if (m == n)
+return;
+  clang_analyzer_eval(n != m); // expected-warning{{TRUE}}
+}
+
+void less_or_equal(int m, int n) {
+  if (m < n)
+return;
+  clang_analyzer_eval(n <= m); // expected-warning{{TRUE}}
+}
+
+void less(int m, int n) {
+  if (m <= n)
+return;
+  clang_analyzer_eval(n < m); // expected-warning{{TRUE}}
+}
+
+void greater_or_equal(int m, int n) {
+  if (m > n)
+return;
+  clang_analyzer_eval(n >= m); // expected-warning{{TRUE}}
+}
+
+void greater(int m, int n) {
+  if (m >= n)
+return;
+  clang_analyzer_eval(n > m); // expected-warning{{TRUE}}
+}
Index: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -256,6 +256,29 @@
 return newRanges;
   }
 
+  // Turn all [A, B] ranges to [-B, -A]. Turn minimal signed value to maximal
+  // signed value and vice versa.
+  RangeSet Negate(BasicValueFactory &BV, Factory &F) const {
+PrimRangeSet newRanges = F.getEmptySet();
+
+for (iterator i = begin(), e = end(); i != e; ++i) {
+  const llvm::APSInt &from = i->From(), &to = i->To();
+  const llvm::APSInt &newFrom = (to.isMinSignedValue() ?
+ BV.getMaxValue(to) :
+ (to.isMaxSignedValue() ?
+  BV.getMinValue(to) :
+  BV.getValue(- to)));
+  const llvm::APSInt &newTo = (from.isMinSignedValue() ?
+   BV.getMaxValue(from) :
+   (from.isMaxSignedValue() ?
+BV.getMinValue(from) :
+BV.getValue(- from)));
+  newRanges = F.add(newRanges, Range(newFrom, newTo));
+}
+
+return newRanges;
+  }
+
   void print(raw_ostream &os) const {
 bool isFirst = true;
 os << "{ ";
@@ -465,11 +488,37 @@
   if (ConstraintRangeTy::data_type *V = State->get(Sym))
 return *V;
 
-  // Lazily generate a new RangeSet representing all possible values for the
-  // given symbol type.
+  //

[PATCH] D33644: Add default values for function parameter chunks

2017-07-18 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




Comment at: lib/Sema/SemaCodeComplete.cpp:2422
+  std::string DefValue{srcText};
+  // TODO: remove this check if the Lexer::getSourceText value is fixed and
+  // this value always has (or always does not have) '=' in front of it

Final nit: TODO is spelled FIXME in llvm/clang.


https://reviews.llvm.org/D33644



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

[PATCH] D35548: [mips] Teach the driver to accept -m(no-)gpopt.

2017-07-18 Thread Simon Atanasyan via Phabricator via cfe-commits

atanasyan added inline comments.



Comment at: lib/Driver/ToolChains/Clang.cpp:1490
+  GPOpt->claim();
+  }
+

Could it be rewritten a bit shorter?

```
bool NoAbiCalls =
ABICalls && ABICalls->getOption().matches(options::OPT_mno_abicalls);
bool WantGPOpt = GPOpt && GPOpt->getOption().matches(options::OPT_mgpopt);

if (NoAbiCalls && (!GPOpt || WantGPOpt)) {
  CmdArgs.push_back("-mllvm");
  CmdArgs.push_back("-mgpopt=1");
} else {
  CmdArgs.push_back("-mllvm");
  CmdArgs.push_back("-mgpopt=0");
}

if (GPOpt)
  GPOpt->claim();
```


https://reviews.llvm.org/D35548



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

[PATCH] D35536: Don't set TUScope to null when generating a module in incremental processing mode.

2017-07-18 Thread Vassil Vassilev via Phabricator via cfe-commits

v.g.vassilev accepted this revision.
v.g.vassilev added a comment.
This revision is now accepted and ready to land.

I think we can rely on a post-commit review here. LGTM!


https://reviews.llvm.org/D35536



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

[PATCH] D35012: [refactor] Add the AST source selection component

2017-07-18 Thread Alex Lorenz via Phabricator via cfe-commits

arphaman updated this revision to Diff 107067.
arphaman marked 7 inline comments as done.
arphaman added a comment.

- Address review comments.
- Remove the `Location` parameter and `ContainsSelectionPoint` enum value.
- Stop traversing early when a declaration that ends after the selection range 
was reached.


Repository:
  rL LLVM

https://reviews.llvm.org/D35012

Files:
  include/clang/Basic/SourceLocation.h
  include/clang/Basic/SourceManager.h
  include/clang/Tooling/Refactoring/ASTSelection.h
  lib/Tooling/Refactoring/ASTSelection.cpp
  lib/Tooling/Refactoring/CMakeLists.txt
  unittests/Tooling/ASTSelectionTest.cpp
  unittests/Tooling/CMakeLists.txt

Index: unittests/Tooling/CMakeLists.txt
===
--- unittests/Tooling/CMakeLists.txt
+++ unittests/Tooling/CMakeLists.txt
@@ -11,6 +11,7 @@
 endif()
 
 add_clang_unittest(ToolingTests
+  ASTSelectionTest.cpp
   CastExprTest.cpp
   CommentHandlerTest.cpp
   CompilationDatabaseTest.cpp
Index: unittests/Tooling/ASTSelectionTest.cpp
===
--- /dev/null
+++ unittests/Tooling/ASTSelectionTest.cpp
@@ -0,0 +1,456 @@
+//===- unittest/Tooling/ASTSelectionTest.cpp --===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "TestVisitor.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Tooling/Refactoring/ASTSelection.h"
+
+using namespace clang;
+using namespace tooling;
+
+namespace {
+
+struct FileLocation {
+  unsigned Line, Column;
+
+  SourceLocation translate(const SourceManager &SM) {
+return SM.translateLineCol(SM.getMainFileID(), Line, Column);
+  }
+};
+
+using FileRange = std::pair;
+
+class SelectionFinderVisitor : public TestVisitor {
+  FileLocation Location;
+  Optional SelectionRange;
+
+public:
+  Optional Selection;
+
+  SelectionFinderVisitor(FileLocation Location,
+ Optional SelectionRange)
+  : Location(Location), SelectionRange(SelectionRange) {}
+
+  bool VisitTranslationUnitDecl(const TranslationUnitDecl *TU) {
+const ASTContext &Context = TU->getASTContext();
+const SourceManager &SM = Context.getSourceManager();
+
+SourceRange SelRange;
+if (SelectionRange) {
+  SelRange = SourceRange(SelectionRange->first.translate(SM),
+ SelectionRange->second.translate(SM));
+} else {
+  SourceLocation Loc = Location.translate(SM);
+  SelRange = SourceRange(Loc, Loc);
+}
+Selection = findSelectedASTNodes(Context, SelRange);
+return false;
+  }
+};
+
+Optional
+findSelectedASTNodes(StringRef Source, FileLocation Location,
+ Optional SelectionRange,
+ SelectionFinderVisitor::Language Language =
+ SelectionFinderVisitor::Lang_CXX11) {
+  SelectionFinderVisitor Visitor(Location, SelectionRange);
+  EXPECT_TRUE(Visitor.runOver(Source, Language));
+  return std::move(Visitor.Selection);
+}
+
+void checkNodeImpl(bool IsTypeMatched, const SelectedASTNode &Node,
+   SourceSelectionKind SelectionKind, unsigned NumChildren) {
+  ASSERT_TRUE(IsTypeMatched);
+  EXPECT_EQ(Node.Children.size(), NumChildren);
+  ASSERT_EQ(Node.SelectionKind, SelectionKind);
+}
+
+void checkDeclName(const SelectedASTNode &Node, StringRef Name) {
+  const auto *ND = Node.Node.get();
+  EXPECT_TRUE(!!ND);
+  ASSERT_EQ(ND->getName(), Name);
+}
+
+template 
+const SelectedASTNode &
+checkNode(const SelectedASTNode &StmtNode, SourceSelectionKind SelectionKind,
+  unsigned NumChildren = 0,
+  typename std::enable_if::value, T>::type
+  *StmtOverloadChecker = nullptr) {
+  checkNodeImpl(isa(StmtNode.Node.get()), StmtNode, SelectionKind,
+NumChildren);
+  return StmtNode;
+}
+
+template 
+const SelectedASTNode &
+checkNode(const SelectedASTNode &DeclNode, SourceSelectionKind SelectionKind,
+  unsigned NumChildren = 0, StringRef Name = "",
+  typename std::enable_if::value, T>::type
+  *DeclOverloadChecker = nullptr) {
+  checkNodeImpl(isa(DeclNode.Node.get()), DeclNode, SelectionKind,
+NumChildren);
+  if (!Name.empty())
+checkDeclName(DeclNode, Name);
+  return DeclNode;
+}
+
+struct ForAllChildrenOf {
+  const SelectedASTNode &Node;
+
+  static void childKindVerifier(const SelectedASTNode &Node,
+SourceSelectionKind SelectionKind) {
+for (const SelectedASTNode &Child : Node.Children) {
+  ASSERT_EQ(Node.SelectionKind, SelectionKind);
+  childKindVerifier(Child, SelectionKind);
+}
+  }
+
+public:
+  ForAllChildrenOf(const SelectedASTNode &Node) : Node(Node) {}
+
+  void shouldHaveSelectionKind(S

[PATCH] D35012: [refactor] Add the AST source selection component

2017-07-18 Thread Alex Lorenz via Phabricator via cfe-commits

arphaman added inline comments.



Comment at: include/clang/Tooling/Refactoring/ASTSelection.h:68-74
+/// Traverses the given ASTContext and creates a tree of selected AST nodes.
+///
+/// \returns None if no nodes are selected in the AST, or a selected AST node
+/// that corresponds to the TranslationUnitDecl otherwise.
+Optional findSelectedASTNodes(const ASTContext &Context,
+   SourceLocation Location,
+   SourceRange SelectionRange);

arphaman wrote:
> klimek wrote:
> > Any reason not to do multiple ranges?
> > 
> > Also, it's not clear from reading the description here why we need the 
> > Location.
> I guess multiple ranges can be supported in follow-up patches to keep this 
> one simpler.
> 
> I'll add it to the comment, but the idea is that the location corresponds to 
> the cursor/right click position in the editor. That means that location 
> doesn't have to be identical to the selection, since you can select something 
> and then right click anywhere in the editor. 
I've decided to get rid of `Location` and `ContainsSelectionPoint` at this 
level. I will instead handle this editor specific thing at a higher-level and 
make selection search range only to simplify things at this level.



Comment at: lib/Tooling/Refactoring/ASTSelection.cpp:100
+  SelectionStack.back().Children.push_back(std::move(Node));
+return true;
+  }

klimek wrote:
> arphaman wrote:
> > klimek wrote:
> > > Why do we always stop traversal?
> > False stops traversal, true is the default return value.
> Ah, right.  Generally, I'd have expected us to stop traversal once we're past 
> the range?
We already do stop traversal early in `findSelectedASTNodes` once we match 
everything that's possible (although if the selection doesn't overlap with 
anything we will traverse all top level nodes). I see the point though, we 
might as stop after the range as well. I added the check to do that in the 
updated patch.



Comment at: lib/Tooling/Refactoring/ASTSelection.cpp:164
+  unsigned NumMatches = 0;
+  for (Decl *D : Context.getTranslationUnitDecl()->decls()) {
+if (ObjCImplEndLoc.isValid() &&

klimek wrote:
> Why don't we do this as part of TraverseDecl() in the visitor?
I think it's easier to handle the Objective-C `@implementation` logic here, 
unless there's some better way that I can't see ATM.


Repository:
  rL LLVM

https://reviews.llvm.org/D35012



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

[PATCH] D35484: Add a warning for missing '#pragma pack (pop)' and suspicious '#pragma pack' uses when including files

2017-07-18 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:716
+def warn_pragma_pack_non_default_at_include : Warning<
+  "non-default #pragma pack value might change the alignment of records in the 
"
+  "included file">,

We don't use "record" in the text for diagnostics. Perhaps replace record with 
"struct or union members"?



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:718
+  "included file">,
+  InGroup;
+def warn_pragma_pack_modified_after_include : Warning<

This can move up a line.



Comment at: include/clang/Sema/Sema.h:1162
 
+  sema::SemaPPCallbacks *SemaPPCallbackHandler;
+

Please group this with the other private Sema members and give it a \brief 
comment.



Comment at: lib/Sema/Sema.cpp:95
+  if (IncludeLoc.isValid()) {
+// Entering an include.
+IncludeStack.push_back(IncludeLoc);

This comment doesn't add much value.



Comment at: lib/Sema/SemaAttr.cpp:235-236
+return;
+  for (auto I = PackStack.Stack.rbegin(), E = PackStack.Stack.rend(); I != E;
+   ++I)
+Diag(I->PragmaPushLocation, diag::warn_pragma_pack_no_pop_eof);

Can you use `for (const auto &StackSlot : llvm::reverse(PackStack.Stack))` 
instead?



Comment at: lib/Sema/SemaAttr.cpp:287-288
   if (Action & PSK_Push)
-Stack.push_back(Slot(StackSlotLabel, CurrentValue, CurrentPragmaLocation));
+Stack.push_back(Slot(StackSlotLabel, CurrentValue, CurrentPragmaLocation,
+ PragmaLocation));
   else if (Action & PSK_Pop) {

This would be better written using `emplace_back()` rather than constructing 
and using `push_back()`.


Repository:
  rL LLVM

https://reviews.llvm.org/D35484



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

[PATCH] D34444: Teach codegen to work in incremental processing mode.

2017-07-18 Thread Vassil Vassilev via Phabricator via cfe-commits

v.g.vassilev added a comment.

In https://reviews.llvm.org/D3#812418, @rjmccall wrote:

> In https://reviews.llvm.org/D3#795175, @v.g.vassilev wrote:
>
> > @rjmccall, thanks for the prompt and thorough reply.
> >
> > In https://reviews.llvm.org/D3#793311, @rjmccall wrote:
> >
> > > Okay.  In that case, I see two problems, one major and one potentially 
> > > major.
> >
> >
> >
> >
> >   This is a very accurate diagnosis which took us 5 years to discover on an 
> > empirical basis ;)
>
>
> You could've asked at any time. :)


True. I am not really sure I knew what to ask, though ;)

> 
> 
>>> The major problem is that, as Richard alludes to, you need to make sure you 
>>> emit any on-demand definitions that Sema registered with CodeGen during the 
>>> initial CGM's lifetime but used in later CGMs.
>> 
>> 
>> 
>>   We bring the CGM state to the subsequent CGMs. See 
>> https://github.com/vgvassilev/clang/blob/cling-patches/lib/CodeGen/ModuleBuilder.cpp#L138-L160
> 
> That's quite brittle, because that code is only executed in a code path that 
> only you are using, and you're not adding any tests.  I would greatly prefer 
> a change to IRGen's core assumptions, as suggested.

I am open to changing this code as well. That should probably be another review.

> 
> 
>>> The potentially major problem is that it is not possible in general to 
>>> automatically break up a single translation unit into multiple translation 
>>> units, for two reasons.  The big reason is that there is no way to 
>>> correctly handle entities with non-external linkage that are referenced 
>>> from two parts of the translation unit without implicitly finding some way 
>>> to promote them to external linkage, which might open a huge can of worms 
>>> if you have multiple "outer" translation units.
>> 
>> 
>> 
>>   We do not have an multiple 'outer' translation units. We have just one 
>> ever growing TU (which probably invalidates my previous statement that we 
>> have a distinct TUs) which we send to the RuntimeDyLD allowing only JIT to 
>> resolve symbols from it.  We aid the JIT when resolving symbols with 
>> internal linkage by changing all internal linkage to external (We haven't 
>> seen issues with that approach).
> 
> Ah, okay.  Yes, that is a completely different translation model from having 
> distinct TUs.
> 
> IRGen will generally happily emit references to undefined internal objects; 
> instead of hacking the linkage, you could just clean that up as a post-pass.  
> Although hacking the linkage as post-pass is reasonable, too.  In either 
> case, you can recognize uses of internal-linkage objects that haven't been 
> defined yet and report that back to the user.
> 
>>>   The lesser reason is that the prefix of a valid translation unit is not 
>>> necessarily a valid translation unit: for example, a static or inline 
>>> function can be defined at an arbitrary within the translation unit, i.e. 
>>> not necessarily before its first use.  But if your use case somehow defines 
>>> away these problems, this might be fine.
>> 
>> 
>> 
>>   If we end up with a module containing no definition of a symbol and such 
>> is required, then we complain. So indeed we are defining away this issue.
> 
> Ok.
> 
>>> As a minor request, if you are going to make HandleTranslationUnit no 
>>> longer the final call on CodeGenerator, please either add a new method that 
>>> *is* a guaranteed final call or add a new method that does the whole "end a 
>>> previous part of the translation unit and start a new one" step.
>> 
>> 
>> 
>>   We can have this but it would be a copy of `HandleTranslationUnit`.  The 
>> `StartModule` interface is the antagonist routine to `ReleaseModule`. If you 
>> prefer we could merge `StartModule` into `ReleaseModule` adding a flag (or 
>> reading the value of `isIncrementalProcessingEnabled`).
> 
> I feel it is important that there be a way to inform an ASTConsumer that no 
> further requests will be made of it, something other than calling its 
> destructor.  I would like you to make sure that the ASTConsumer interface 
> supports that and that that call is not made too soon in your alternate 
> processing mode.

Do you have a preference of a name of this new interface?


Repository:
  rL LLVM

https://reviews.llvm.org/D3



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

[PATCH] D35557: clang-format: merge short case labels with trailing comments

2017-07-18 Thread Francois Ferrand via Phabricator via cfe-commits

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

Allow merging short case labels when they actually end with a comment
(like a comment after the ``break``) and when followed by switch-level
comments (e.g. aligned with next case):

  switch(a) {
  case 0: break; // comment at end of case
  case 1: return value;
  // comment related to next case
  // comment related to next case
  case 2:
  }


https://reviews.llvm.org/D35557

Files:
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -897,6 +897,38 @@
"}",
Style);
   verifyFormat("switch (a) {\n"
+   "case 0: return; // comment\n"
+   "case 1: break;  // comment\n"
+   "case 2: return;\n"
+   "// comment\n"
+   "case 3: return;\n"
+   "// comment 1\n"
+   "// comment 2\n"
+   "// comment 3\n"
+   "case 4: break; /* comment */\n"
+   "case 5:\n"
+   "  // comment\n"
+   "  break;\n"
+   "}",
+   Style);
+  EXPECT_EQ("switch (a) {\n"
+"case 1:\n"
+"  x = 8;\n"
+"  // fall through\n"
+"case 2: x = 8;\n"
+"// comment\n"
+"default:\n"
+"}",
+format("switch (a) {\n"
+   "case 1: x = 8;\n"
+   "  // fall through\n"
+   "case 2:\n"
+   "  x = 8;\n"
+   "// comment\n"
+   "default:\n"
+   "}",
+   Style));
+  verifyFormat("switch (a) {\n"
"#if FOO\n"
"case 0: return 0;\n"
"#endif\n"
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -382,7 +382,9 @@
   return 0;
 unsigned NumStmts = 0;
 unsigned Length = 0;
+bool EndsWithComment = false;
 bool InPPDirective = I[0]->InPPDirective;
+const unsigned Level = I[0]->Level;
 for (; NumStmts < 3; ++NumStmts) {
   if (I + 1 + NumStmts == E)
 break;
@@ -392,9 +394,26 @@
   if (Line->First->isOneOf(tok::kw_case, tok::kw_default, tok::r_brace))
 break;
   if (Line->First->isOneOf(tok::kw_if, tok::kw_for, tok::kw_switch,
-   tok::kw_while, tok::comment) ||
-  Line->Last->is(tok::comment))
+   tok::kw_while) ||
+  EndsWithComment)
 return 0;
+  if (Line->First->is(tok::comment)) {
+if (Level != Line->Level)
+  return 0;
+SmallVectorImpl::const_iterator J = I + 2 + NumStmts;
+for (; J != E; ++J) {
+  const AnnotatedLine *Line = J[0];
+  if (Line->InPPDirective != InPPDirective)
+break;
+  if (Line->First->isOneOf(tok::kw_case, tok::kw_default, 
tok::r_brace))
+break;
+  if (Line->First->isNot(tok::comment) || Level != Line->Level)
+return 0;
+}
+break;
+  }
+  if (Line->Last->is(tok::comment))
+EndsWithComment = true;
   Length += I[1 + NumStmts]->Last->TotalLength + 1; // 1 for the space.
 }
 if (NumStmts == 0 || NumStmts == 3 || Length > Limit)


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -897,6 +897,38 @@
"}",
Style);
   verifyFormat("switch (a) {\n"
+   "case 0: return; // comment\n"
+   "case 1: break;  // comment\n"
+   "case 2: return;\n"
+   "// comment\n"
+   "case 3: return;\n"
+   "// comment 1\n"
+   "// comment 2\n"
+   "// comment 3\n"
+   "case 4: break; /* comment */\n"
+   "case 5:\n"
+   "  // comment\n"
+   "  break;\n"
+   "}",
+   Style);
+  EXPECT_EQ("switch (a) {\n"
+"case 1:\n"
+"  x = 8;\n"
+"  // fall through\n"
+"case 2: x = 8;\n"
+"// comment\n"
+"default:\n"
+"}",
+format("switch (a) {\n"
+   "case 1: x = 8;\n"
+   "  // fall through\n"
+   "case 2:\n"
+   "  x = 8;\n"
+   "// comment\n"
+   "default:\n"
+   "}",
+   Style));
+  verifyFormat("switch (a) {\n"
"#if FOO\n"
"case 0: return 0;\n"
"#endif\

[PATCH] D35559: [CMake][Modules] Tweak Modules-unfriendly builds

2017-07-18 Thread NAKAMURA Takumi via Phabricator via cfe-commits

chapuni created this revision.
Herald added a subscriber: mgorny.

I can commit changes partially.

- Add -fmodules-ignore-macro (llvm-config, libclang)

Their -DMACROs don't change headers' behavior. Just let them ignored.

- Unittests

They use -frtti. I thought they may be built w/o modules. Any better idea?

- DynamicLibraryTests

It doesn't use add_llvm_library(). Give -fno-modules with 
target_compile_options().

- libFuzzer

It doesn't depend on LLVM libraries. Just suppressing with -fno-modules.

- LLVMgold

  add_definitions( -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 )

It prevents reusing modules. I think they may be pruned since 
HandleLLVMOptions.cmake provides them conditionally.


Repository:
  rL LLVM

https://reviews.llvm.org/D35559

Files:
  cfe/trunk/tools/libclang/CMakeLists.txt
  llvm/trunk/lib/Fuzzer/CMakeLists.txt
  llvm/trunk/tools/gold/CMakeLists.txt
  llvm/trunk/tools/llvm-config/CMakeLists.txt
  llvm/trunk/unittests/Support/DynamicLibrary/CMakeLists.txt
  llvm/trunk/utils/unittest/CMakeLists.txt

Index: llvm/trunk/utils/unittest/CMakeLists.txt
===
--- llvm/trunk/utils/unittest/CMakeLists.txt
+++ llvm/trunk/utils/unittest/CMakeLists.txt
@@ -45,6 +45,11 @@
   list(APPEND LIBS pthread)
 endif()
 
+# -frtti
+if(LLVM_ENABLE_MODULES)
+  list(APPEND LLVM_COMPILE_FLAGS "-fno-modules")
+endif()
+
 add_llvm_library(gtest
   googletest/src/gtest-all.cc
   googlemock/src/gmock-all.cc
Index: llvm/trunk/unittests/Support/DynamicLibrary/CMakeLists.txt
===
--- llvm/trunk/unittests/Support/DynamicLibrary/CMakeLists.txt
+++ llvm/trunk/unittests/Support/DynamicLibrary/CMakeLists.txt
@@ -2,6 +2,10 @@
 
 add_library(DynamicLibraryLib STATIC ExportedFuncs.cxx)
 
+if(LLVM_ENABLE_MODULES)
+  target_compile_options(DynamicLibraryLib PRIVATE "-fno-modules")
+endif()
+
 add_llvm_unittest(DynamicLibraryTests DynamicLibraryTest.cpp)
 target_link_libraries(DynamicLibraryTests DynamicLibraryLib)
 export_executable_symbols(DynamicLibraryTests)
@@ -19,6 +23,10 @@
 SUFFIX ".so"
 )
 
+  if(LLVM_ENABLE_MODULES)
+target_compile_options(${NAME} PRIVATE "-fno-modules")
+  endif()
+
   add_dependencies(DynamicLibraryTests ${NAME})
 endfunction(dynlib_add_module)
 
Index: llvm/trunk/tools/llvm-config/CMakeLists.txt
===
--- llvm/trunk/tools/llvm-config/CMakeLists.txt
+++ llvm/trunk/tools/llvm-config/CMakeLists.txt
@@ -59,6 +59,12 @@
 # Set build-time environment(s).
 add_definitions(-DCMAKE_CFG_INTDIR="${CMAKE_CFG_INTDIR}")
 
+if(LLVM_ENABLE_MODULES)
+  target_compile_options(llvm-config PUBLIC
+"-fmodules-ignore-macro=CMAKE_CFG_INTDIR"
+)
+endif()
+
 # Add the dependency on the generation step.
 add_file_dependencies(${CMAKE_CURRENT_SOURCE_DIR}/llvm-config.cpp ${BUILDVARIABLES_OBJPATH})
 
Index: llvm/trunk/tools/gold/CMakeLists.txt
===
--- llvm/trunk/tools/gold/CMakeLists.txt
+++ llvm/trunk/tools/gold/CMakeLists.txt
@@ -7,6 +7,10 @@
   # ABI compatibility.
   add_definitions( -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 )
 
+  if(LLVM_ENABLE_MODULES)
+list(APPEND LLVM_COMPILE_FLAGS "-fno-modules")
+  endif()
+
   set(LLVM_LINK_COMPONENTS
  ${LLVM_TARGETS_TO_BUILD}
  Linker
Index: llvm/trunk/lib/Fuzzer/CMakeLists.txt
===
--- llvm/trunk/lib/Fuzzer/CMakeLists.txt
+++ llvm/trunk/lib/Fuzzer/CMakeLists.txt
@@ -26,6 +26,10 @@
   set(CMAKE_CXX_FLAGS "${LIBFUZZER_FLAGS_BASE} -fno-sanitize-coverage=trace-pc-guard,edge,trace-cmp,indirect-calls,8bit-counters -Werror")
 endif()
 
+if(LLVM_ENABLE_MODULES)
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-modules")
+endif()
+
 # Compile libFuzzer if the compilation is specifically requested, OR
 # if the platform is known to be working.
 if ( LLVM_USE_SANITIZE_COVERAGE OR CMAKE_SYSTEM_NAME MATCHES "Darwin|Linux" )
Index: cfe/trunk/tools/libclang/CMakeLists.txt
===
--- cfe/trunk/tools/libclang/CMakeLists.txt
+++ cfe/trunk/tools/libclang/CMakeLists.txt
@@ -51,6 +51,9 @@
   add_definitions(-DCLANG_TOOL_EXTRA_BUILD)
   list(APPEND LIBS clangTidyPlugin)
   list(APPEND LIBS clangIncludeFixerPlugin)
+  if(LLVM_ENABLE_MODULES)
+list(APPEND LLVM_COMPILE_FLAGS "-fmodules-ignore-macro=CLANG_TOOL_EXTRA_BUILD")
+  endif()
 endif ()
 
 find_library(DL_LIBRARY_PATH dl)
@@ -115,6 +118,11 @@
   PROPERTIES
   VERSION ${LIBCLANG_LIBRARY_VERSION}
   DEFINE_SYMBOL _CINDEX_LIB_)
+if(LLVM_ENABLE_MODULES)
+  target_compile_options(libclang PRIVATE
+"-fmodules-ignore-macro=_CINDEX_LIB_"
+)
+endif()
   endif()
 endif()
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listin

[PATCH] D35520: [Sema] Improve diagnostic message for unavailable c++17 aligned allocation functions

2017-07-18 Thread Alex Lorenz via Phabricator via cfe-commits

arphaman added inline comments.



Comment at: include/clang/Basic/AlignedAllocation.h:25
+
+static inline VersionTuple alignedAllocMinVersion(llvm::Triple::OSType OS) {
+  switch (OS) {

Redundant `static`?



Comment at: include/clang/Basic/AlignedAllocation.h:31
+  case llvm::Triple::MacOSX: // Earliest supporting version is 10.13.
+return VersionTuple(10U, 13U, 0U);
+  case llvm::Triple::IOS:

You can drop the last `0` here and two last zeros down below. This will also 
make the diagnostics less verbose, i.e. instead of `iOS 11.0.0` we will show 
`iOS 11`.



Comment at: include/clang/Basic/AlignedAllocation.h:42
+
+}
+

`// end namespace clang`



Comment at: include/clang/Basic/AlignedAllocation.h:44
+
+#endif

NIT: Missing `// LLVM_CLANG_BASIC_ALIGNED_ALLOCATION_H` comment



Comment at: lib/Sema/SemaExprCXX.cpp:1667
 S.Diag(Loc, diag::warn_aligned_allocation_unavailable)
- << IsDelete << FD.getType().getAsString()
- << S.getASTContext().getTargetInfo().getTriple().str();
+ << IsDelete << FD.getType().getAsString() << 
T.getOSTypeName(T.getOS())
+ << alignedAllocMinVersion(T.getOS()).getAsString();

Can you use `AvailabilityAttr::getPlatformNameSourceSpelling` instead to be 
consisted with our unguarded availability warnings?
e.g.:

```
AvailabilityAttr::getPlatformNameSourceSpelling(S.getASTContext().getTargetInfo().getPlatformName())
```

This will ensure that the diagnostics will use names like 'macOS' instead of 
'macosx'.


https://reviews.llvm.org/D35520



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

[PATCH] D35520: [Sema] Improve diagnostic message for unavailable c++17 aligned allocation functions

2017-07-18 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: include/clang/Basic/AlignedAllocation.h:12
+/// \brief Defines a function that returns the minimum OS versions supporting
+/// c++17's aligned allocation functions.
+///

c++17 -> C++17


https://reviews.llvm.org/D35520



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

[PATCH] D35485: [clang-format] Fix comment levels between '}' and PPDirective

2017-07-18 Thread Krasimir Georgiev via Phabricator via cfe-commits

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

- Remove TODO test case


https://reviews.llvm.org/D35485

Files:
  lib/Format/UnwrappedLineParser.cpp
  lib/Format/UnwrappedLineParser.h
  unittests/Format/FormatTestComments.cpp

Index: unittests/Format/FormatTestComments.cpp
===
--- unittests/Format/FormatTestComments.cpp
+++ unittests/Format/FormatTestComments.cpp
@@ -836,6 +836,25 @@
"  int j;\n"
"}"));
 
+  EXPECT_EQ("int f(int i) {\n"
+"  if (true) {\n"
+"++i;\n"
+"  }\n"
+"  // comment\n"
+"#ifdef A\n"
+"  int j;\n"
+"#endif\n"
+"}",
+format("int f(int i) {\n"
+   "  if (true) {\n"
+   "++i;\n"
+   "  }\n"
+   "  // comment\n"
+   "#ifdef A\n"
+   "  int j;\n"
+   "#endif\n"
+   "}"));
+
   // Keep the current level if there is an empty line between the comment and
   // the preprocessor directive.
   EXPECT_EQ("void f() {\n"
@@ -853,6 +872,46 @@
"  int j;\n"
"}"));
 
+  EXPECT_EQ("void f() {\n"
+"  int i;\n"
+"  return i;\n"
+"}\n"
+"// comment\n"
+"\n"
+"#ifdef A\n"
+"int i;\n"
+"#endif // A",
+format("void f() {\n"
+   "  int i;\n"
+   "  return i;\n"
+   "}\n"
+   "// comment\n"
+   "\n"
+   "#ifdef A\n"
+   "int i;\n"
+   "#endif // A"));
+
+  EXPECT_EQ("int f(int i) {\n"
+"  if (true) {\n"
+"++i;\n"
+"  }\n"
+"  // comment\n"
+"\n"
+"#ifdef A\n"
+"  int j;\n"
+"#endif\n"
+"}",
+format("int f(int i) {\n"
+   "  if (true) {\n"
+   "++i;\n"
+   "  }\n"
+   "  // comment\n"
+   "\n"
+   "#ifdef A\n"
+   "  int j;\n"
+   "#endif\n"
+   "}"));
+
   // Align with the preprocessor directive if the comment was originally aligned
   // with the preprocessor directive.
   EXPECT_EQ("void f() {\n"
@@ -867,6 +926,25 @@
"#ifdef A\n"
"  int j;\n"
"}"));
+
+  EXPECT_EQ("int f(int i) {\n"
+"  if (true) {\n"
+"++i;\n"
+"  }\n"
+"// comment\n"
+"#ifdef A\n"
+"  int j;\n"
+"#endif\n"
+"}",
+format("int f(int i) {\n"
+   "  if (true) {\n"
+   "++i;\n"
+   "  }\n"
+   "// comment\n"
+   "#ifdef A\n"
+   "  int j;\n"
+   "#endif\n"
+   "}"));
 }
 
 TEST_F(FormatTestComments, SplitsLongLinesInComments) {
Index: lib/Format/UnwrappedLineParser.h
===
--- lib/Format/UnwrappedLineParser.h
+++ lib/Format/UnwrappedLineParser.h
@@ -19,6 +19,7 @@
 #include "FormatToken.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Format/Format.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/Regex.h"
 #include 
 #include 
@@ -123,9 +124,9 @@
   void tryToParseJSFunction();
   void addUnwrappedLine();
   bool eof() const;
-  void nextToken();
+  void nextToken(llvm::Optional InitialLevel = None);
   const FormatToken *getPreviousToken();
-  void readToken();
+  void readToken(llvm::Optional InitialLevel = None);
 
   // Decides which comment tokens should be added to the current line and which
   // should be added as comments before the next token.
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -486,7 +486,7 @@
 return;
   }
 
-  nextToken(); // Munch the closing brace.
+  nextToken(InitialLevel); // Munch the closing brace.
 
   if (MacroBlock && FormatTok->is(tok::l_paren))
 parseParens();
@@ -2287,13 +2287,13 @@
   CommentsBeforeNextToken.clear();
 }
 
-void UnwrappedLineParser::nextToken() {
+void UnwrappedLineParser::nextToken(llvm::Optional InitialLevel) {
   if (eof())
 return;
   flushComments(isOnNewLine(*FormatTok));
   pushToken(FormatTok);
   if (Style.Language != FormatStyle::LK_JavaScript)
-readToken();
+readToken(InitialLevel);
   else
 readTokenWithJavaScriptASI();
 }
@@ -2362,7 +2362,7 @@
   }
 }
 
-void UnwrappedLineParser::readToken() {
+void UnwrappedLineParser::readTo

[PATCH] D33440: clang-format: better handle statement and namespace macros

2017-07-18 Thread Francois Ferrand via Phabricator via cfe-commits

Typz added a comment.

t>>! In https://reviews.llvm.org/D33440#812645, @djasper wrote:

> So, there are two things in this patch: Statement macros and namespace 
> macros. Lets break this out and handle them individually. They really aren't 
> related that much.

Indeed, the only "relation" is the implementation, which now uses the exact 
same list (internally) to match all macros. Phabricator makes it very difficult 
to work with related changes pushed as multiple reviews, so I ended up merging 
the two features.

> Statement macros:
>  I think clang-format's handling here is good enough. clang-format does not 
> insert the line break, but it also doesn't remove it. I am not 100% sure 
> here, so I an be convinced. But I want to understand the use cases better. Do 
> you expect people to run into this frequently? I am essentially trying to 
> understand whether the cost of an extra option is worth the benefit it is 
> giving.

This happens relatively often, for example when fixing "unused parameter 
warning" on an inlined function: ``int f(int a) { return 0; }`` often gets 
fixed to ``int f(int a) { Q_UNUSED(a) return 0; }`` and clang-format does not 
fix the formatting...

> Namespace macros:
>  How important are the automatic closing comments to you? I'd say that we 
> should punt on that and leave it to the user to fix comments of these. And 
> then, we could try to make the things we already have in MacroBlockBegin 
> detect whether it ends with an opening brace and not need an extra list here. 
> What do you think?

This is not just about automatic closing comments, there are may differences: 
indentation of namespaces, 'compacting' of namespaces when `CompactNamespaces` 
is used, detection of inline functions (for putting on a single line with 
SFS_Inline), handling of empty blocks (i.e. use 
`BraceWrappingFlags.SplitEmptyNamespace`)...

https://reviews.llvm.org/D33440

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

[PATCH] D34873: Fix miscompiled 32bit binaries by mingw

2017-07-18 Thread Hans Wennborg via Phabricator via cfe-commits

hans added a reviewer: rnk.
hans added a comment.

From the bug, this is related to Richard's "r289413 - Add two new AST nodes to 
represent initialization of an array in terms of initialization of each array 
element", which broke MSVC builds due to under-alignment, which Reid addressed 
with "r289575 - Align EvalInfo in ExprConstant to avoid PointerUnion 
assertions".

Perhaps there's a problem with MinGW 4.9.2's alignas? Or is this 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78936 as Reid says in 
https://bugs.llvm.org/show_bug.cgi?id=32018#c8 ?


https://reviews.llvm.org/D34873



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

[PATCH] D34873: Fix miscompiled 32bit binaries by mingw

2017-07-18 Thread Ivan Donchevskii via Phabricator via cfe-commits

yvvan added a comment.

The same alignment in other places works fine. i don't know the source of that 
issue. Should be somewhere in gcc(mingw) but it's not my focus.
So this is a workaround that we can make (probably only for mingw builds)


https://reviews.llvm.org/D34873



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

[PATCH] D34824: clang-format: add an option -verbose to list the files being processed

2017-07-18 Thread Sylvestre Ledru via Phabricator via cfe-commits

sylvestre.ledru updated this revision to Diff 107089.
sylvestre.ledru marked an inline comment as done.
sylvestre.ledru added a comment.

Fixed, thanks for spotting the mistake.
Looks like we could improve the testsuite as my mistake hasn't been catched.


https://reviews.llvm.org/D34824

Files:
  docs/ClangFormat.rst
  docs/ReleaseNotes.rst
  test/Format/verbose.cpp
  tools/clang-format/ClangFormat.cpp


Index: test/Format/verbose.cpp
===
--- test/Format/verbose.cpp
+++ test/Format/verbose.cpp
@@ -0,0 +1,8 @@
+// RUN: clang-format %s  2> %t.stderr
+// RUN: not grep "Formatting" %t.stderr
+// RUN: clang-format %s -verbose 2> %t.stderr
+// RUN: grep -E "Formatting (.*)verbose.cpp(.*)" %t.stderr
+// RUN: clang-format %s -verbose=false 2> %t.stderr
+// RUN: not grep "Formatting" %t.stderr
+
+int a;
Index: docs/ClangFormat.rst
===
--- docs/ClangFormat.rst
+++ docs/ClangFormat.rst
@@ -71,6 +71,7 @@
 Use -style="{key: value, ...}" to set specific
 parameters, e.g.:
   -style="{BasedOnStyle: llvm, IndentWidth: 8}"
+-verbose  - If set, shows the list of processed files
 
   Generic Options:
 
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -191,6 +191,9 @@
 * Comment reflow support added. Overly long comment lines will now be reflown 
with the rest of
   the paragraph instead of just broken. Option **ReflowComments** added and 
enabled by default.
 
+* Option -verbose added to the command line.
+  Shows the list of processed files.
+
 libclang
 
 
Index: tools/clang-format/ClangFormat.cpp
===
--- tools/clang-format/ClangFormat.cpp
+++ tools/clang-format/ClangFormat.cpp
@@ -102,6 +102,10 @@
  "SortIncludes style flag"),
 cl::cat(ClangFormatCategory));
 
+static cl::opt
+Verbose("verbose", cl::desc("If set, shows the list of processed files"),
+cl::cat(ClangFormatCategory));
+
 static cl::list FileNames(cl::Positional, cl::desc("[ 
...]"),
cl::cat(ClangFormatCategory));
 
@@ -365,23 +369,19 @@
   }
 
   bool Error = false;
-  switch (FileNames.size()) {
-  case 0:
+  if (FileNames.empty()) {
 Error = clang::format::format("-");
-break;
-  case 1:
-Error = clang::format::format(FileNames[0]);
-break;
-  default:
-if (!Offsets.empty() || !Lengths.empty() || !LineRanges.empty()) {
-  errs() << "error: -offset, -length and -lines can only be used for "
-"single file.\n";
-  return 1;
-}
-for (unsigned i = 0; i < FileNames.size(); ++i)
-  Error |= clang::format::format(FileNames[i]);
-break;
+return Error ? 1 : 0;
+  }
+  if (FileNames.size() != 1 && (!Offsets.empty() || !Lengths.empty() || 
!LineRanges.empty())) {
+errs() << "error: -offset, -length and -lines can only be used for "
+  "single file.\n";
+return 1;
+  }
+  for (const auto &FileName : FileNames) {
+if (Verbose)
+  errs() << "Formatting " << FileName << "\n";
+Error |= clang::format::format(FileName);
   }
   return Error ? 1 : 0;
 }
-


Index: test/Format/verbose.cpp
===
--- test/Format/verbose.cpp
+++ test/Format/verbose.cpp
@@ -0,0 +1,8 @@
+// RUN: clang-format %s  2> %t.stderr
+// RUN: not grep "Formatting" %t.stderr
+// RUN: clang-format %s -verbose 2> %t.stderr
+// RUN: grep -E "Formatting (.*)verbose.cpp(.*)" %t.stderr
+// RUN: clang-format %s -verbose=false 2> %t.stderr
+// RUN: not grep "Formatting" %t.stderr
+
+int a;
Index: docs/ClangFormat.rst
===
--- docs/ClangFormat.rst
+++ docs/ClangFormat.rst
@@ -71,6 +71,7 @@
 Use -style="{key: value, ...}" to set specific
 parameters, e.g.:
   -style="{BasedOnStyle: llvm, IndentWidth: 8}"
+-verbose  - If set, shows the list of processed files
 
   Generic Options:
 
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -191,6 +191,9 @@
 * Comment reflow support added. Overly long comment lines will now be reflown with the rest of
   the paragraph instead of just broken. Option **ReflowComments** added and enabled by default.
 
+* Option -verbose added to the command line.
+  Shows the list of processed files.
+
 libclang
 
 
Index: tools/clang-format/ClangFormat.cpp
===
--- tools/clang-format/ClangFormat.cpp
+++ tools/clan

[PATCH] D35012: [refactor] Add the AST source selection component

2017-07-18 Thread Manuel Klimek via Phabricator via cfe-commits

klimek added inline comments.



Comment at: lib/Tooling/Refactoring/ASTSelection.cpp:164
+  unsigned NumMatches = 0;
+  for (Decl *D : Context.getTranslationUnitDecl()->decls()) {
+if (ObjCImplEndLoc.isValid() &&

arphaman wrote:
> klimek wrote:
> > Why don't we do this as part of TraverseDecl() in the visitor?
> I think it's easier to handle the Objective-C `@implementation` logic here, 
> unless there's some better way that I can't see ATM.
Ok, in that case, can you write a comment at the start of the loop explaining 
that we basically only do that for the Objective-C @implementation? (I'd also 
like to understand that better in general, as I have no clue about Obj-C :)


Repository:
  rL LLVM

https://reviews.llvm.org/D35012



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

[PATCH] D35484: Add a warning for missing '#pragma pack (pop)' and suspicious '#pragma pack' uses when including files

2017-07-18 Thread Alex Lorenz via Phabricator via cfe-commits

arphaman updated this revision to Diff 107090.
arphaman marked 5 inline comments as done.
arphaman added a comment.

Address review comments.


Repository:
  rL LLVM

https://reviews.llvm.org/D35484

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  include/clang/Serialization/ASTReader.h
  lib/Parse/ParsePragma.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaAttr.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  test/PCH/pragma-pack.c
  test/PCH/suspicious-pragma-pack.c
  test/Parser/pragma-options.c
  test/Parser/pragma-options.cpp
  test/Parser/pragma-pack.c
  test/Sema/Inputs/pragma-pack1.h
  test/Sema/Inputs/pragma-pack2.h
  test/Sema/pragma-pack.c
  test/Sema/suspicious-pragma-pack.c
  test/SemaObjC/Inputs/empty.h
  test/SemaObjC/suspicious-pragma-pack.m

Index: test/SemaObjC/suspicious-pragma-pack.m
===
--- /dev/null
+++ test/SemaObjC/suspicious-pragma-pack.m
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -triple i686-apple-darwin9 -fsyntax-only -I%S/Inputs -verify %s
+
+#pragma pack (push, 1) // expected-note {{previous '#pragma pack' directive that modifies alignment is here}}
+#import "empty.h" // expected-warning {{non-default #pragma pack value might change the alignment of struct or union members in the included file}}
+
+#pragma pack (pop)
Index: test/SemaObjC/Inputs/empty.h
===
--- /dev/null
+++ test/SemaObjC/Inputs/empty.h
@@ -0,0 +1 @@
+// empty
Index: test/Sema/suspicious-pragma-pack.c
===
--- /dev/null
+++ test/Sema/suspicious-pragma-pack.c
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -triple i686-apple-darwin9 -fsyntax-only -I %S/Inputs -DSAFE -verify %s
+// RUN: %clang_cc1 -triple i686-apple-darwin9 -fsyntax-only -I %S/Inputs -DPUSH_HERE -DSAFE -verify %s
+// RUN: %clang_cc1 -triple i686-apple-darwin9 -fsyntax-only -I %S/Inputs -DPUSH_HERE -DPUSH_SET_HERE -verify %s
+// RUN: %clang_cc1 -triple i686-apple-darwin9 -fsyntax-only -I %S/Inputs -DPUSH_HERE -DRESET_HERE -DSAFE -verify %s
+// RUN: %clang_cc1 -triple i686-apple-darwin9 -fsyntax-only -I %S/Inputs -DPUSH_HERE -DSET_FIRST_HEADER -DWARN_MODIFIED_HEADER -verify %s
+// RUN: %clang_cc1 -triple i686-apple-darwin9 -fsyntax-only -I %S/Inputs -DPUSH_HERE -DRESET_HERE -DSET_FIRST_HEADER -DWARN_MODIFIED_HEADER -verify %s
+// RUN: %clang_cc1 -triple i686-apple-darwin9 -fsyntax-only -I %S/Inputs -DPUSH_HERE -DPUSH_SET_HERE -DSET_FIRST_HEADER -DWARN_MODIFIED_HEADER -verify %s
+// RUN: %clang_cc1 -triple i686-apple-darwin9 -fsyntax-only -I %S/Inputs -DPUSH_HERE -DPUSH_SET_HERE -DSET_SECOND_HEADER -DWARN_MODIFIED_HEADER -verify %s
+// RUN: %clang_cc1 -triple i686-apple-darwin9 -fsyntax-only -I %S/Inputs -DPUSH_HERE -DPUSH_SET_HERE -DSET_FIRST_HEADER -DSET_SECOND_HEADER -DWARN_MODIFIED_HEADER -verify %s
+
+// RUN: %clang_cc1 -triple i686-apple-darwin9 -fsyntax-only -I %S/Inputs -DPUSH_POP_FIRST_HEADER -DSAFE -verify %s
+
+
+#ifdef SAFE
+// expected-no-diagnostics
+#endif
+
+#ifdef PUSH_HERE
+#pragma pack (push)
+#endif
+
+#ifdef PUSH_SET_HERE
+#pragma pack (push, 4) // expected-note {{previous '#pragma pack' directive that modifies alignment is here}}
+// expected-warning@+8 {{non-default #pragma pack value might change the alignment of struct or union members in the included file}}
+#endif
+
+#ifdef RESET_HERE
+#pragma pack (4)
+#pragma pack () // no warning after reset as the value is default.
+#endif
+
+#include "pragma-pack1.h"
+
+#ifdef WARN_MODIFIED_HEADER
+// expected-warning@-3 {{the current #pragma pack aligment value is modified in the included file}}
+#endif
+
+#ifdef PUSH_SET_HERE
+#pragma pack (pop)
+#endif
+
+#ifdef PUSH_HERE
+#pragma pack (pop)
+#endif
Index: test/Sema/pragma-pack.c
===
--- test/Sema/pragma-pack.c
+++ test/Sema/pragma-pack.c
@@ -25,3 +25,8 @@
 #pragma pack(pop, 16)
 /* expected-warning {{value of #pragma pack(show) == 16}} */ #pragma pack(show)
 
+
+// Warn about unbalanced pushes.
+#pragma pack (push,4) // expected-warning {{unterminated '#pragma pack (push, ...)' at end of file}}
+#pragma pack (push)   // expected-warning {{unterminated '#pragma pack (push, ...)' at end of file}}
+#pragma pack ()
Index: test/Sema/Inputs/pragma-pack2.h
===
--- /dev/null
+++ test/Sema/Inputs/pragma-pack2.h
@@ -0,0 +1,6 @@
+
+#ifdef SET_SECOND_HEADER
+#pragma pack (8) // expected-note 2 {{previous '#pragma pack' directive that modifies alignment is here}}
+#endif
+
+struct S { int x; };
Index: test/Sema/Inputs/pragma-pack1.h
===
--- /dev/null
+++ test/Sema/Inputs/pragma-pack1.h
@@ -0,0 +1,23 @@
+
+#ifdef SET_FIRST_HEADER
+#pragma pack (16)
+#ifndef SET_SECOND_HEADER
+// expected-note@-2

[PATCH] D21508: Diagnose friend function template redefinitions

2017-07-18 Thread Serge Pavlov via Phabricator via cfe-commits

sepavloff updated this revision to Diff 107091.
sepavloff added a comment.

Simplified patch


https://reviews.llvm.org/D21508

Files:
  include/clang/AST/DeclBase.h
  lib/AST/Decl.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  test/SemaCXX/friend2.cpp

Index: test/SemaCXX/friend2.cpp
===
--- test/SemaCXX/friend2.cpp
+++ test/SemaCXX/friend2.cpp
@@ -129,6 +129,83 @@
 void func_22() {} // expected-error{{redefinition of 'func_22'}}
 
 
+// Case of template friend functions.
+
+template void func_31(T *x);
+template
+struct C31a {
+  template friend void func_31(T *x) {}
+};
+template
+struct C31b {
+  template friend void func_31(T *x) {}
+};
+
+
+template inline void func_32(T *x) {}
+template
+struct C32a {
+  template friend void func_32(T *x) {}
+};
+template
+struct C32b {
+  template friend void func_32(T *x) {}
+};
+
+
+template
+struct C33a {
+  template friend void func_33(T *x) {}
+};
+template
+struct C33b {
+  template friend void func_33(T *x) {}
+};
+
+
+template inline void func_34(T *x) {}  // expected-note{{previous definition is here}}
+template
+struct C34 {
+  template friend void func_34(T *x) {} // expected-error{{redefinition of 'func_34'}}
+};
+
+C34 v34;  // expected-note{{in instantiation of template class 'C34' requested here}}
+
+
+template inline void func_35(T *x);
+template
+struct C35a {
+  template friend void func_35(T *x) {} // expected-note{{previous definition is here}}
+};
+template
+struct C35b {
+  template friend void func_35(T *x) {} // expected-error{{redefinition of 'func_35'}}
+};
+
+C35a v35a;
+C35b v35b;  // expected-note{{in instantiation of template class 'C35b' requested here}}
+
+
+template void func_36(T *x);
+template
+struct C36 {
+  template friend void func_36(T *x) {}  // expected-error{{redefinition of 'func_36'}}
+ // expected-note@-1{{previous definition is here}}
+};
+
+C36 v36a;
+C36 v36b;  //expected-note{{in instantiation of template class 'C36' requested here}}
+
+
+template void func_37(T *x);
+template
+struct C37 {
+  template friend void func_37(T *x) {} // expected-note{{previous definition is here}}
+};
+
+C37 v37;
+template void func_37(T *x) {} // expected-error{{redefinition of 'func_37'}}
+
 
 namespace pr22307 {
 
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1795,7 +1795,9 @@
   // If the original function was part of a friend declaration,
   // inherit its namespace state and add it to the owner.
   if (isFriend) {
-PrincipalDecl->setObjectOfFriendDecl();
+Function->setObjectOfFriendDecl();
+if (FunctionTemplate)
+  FunctionTemplate->setObjectOfFriendDecl();
 DC->makeDeclVisibleInContext(PrincipalDecl);
 
 bool QueuedInstantiation = false;
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11990,6 +11990,29 @@
   }
 }
   }
+
+  if (!Definition)
+// Similar to friend functions a friend function template may be a
+// definition and do not have a body if it is instantiated in a class
+// template.
+if (FunctionTemplateDecl *FTD = FD->getDescribedFunctionTemplate()) {
+  for (auto I : FTD->redecls()) {
+auto D = cast(I);
+if (D != FTD) {
+  assert(!D->isThisDeclarationADefinition() &&
+ "Underlying function declaration must be a definition");
+  if (D->getFriendObjectKind() != Decl::FOK_None)
+if (FunctionTemplateDecl *FT =
+   D->getInstantiatedFromMemberTemplate()) {
+  if (FT->isThisDeclarationADefinition()) {
+Definition = D->getTemplatedDecl();
+break;
+  }
+}
+}
+  }
+}
+
   if (!Definition)
 return;
 
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -3288,6 +3288,14 @@
   if (auto *MFD = getInstantiatedFromMemberFunction())
 return getDefinitionOrSelf(MFD);
 
+  if (FunctionTemplateDecl *TD = getDescribedFunctionTemplate()) {
+if (TD->getFriendObjectKind() != FOK_None) {
+  while (FunctionTemplateDecl *FT = TD->getInstantiatedFromMemberTemplate())
+TD = FT;
+  return TD->getTemplatedDecl();
+}
+  }
+
   return nullptr;
 }
 
Index: include/clang/AST/DeclBase.h
===
--- include/clang/AST/DeclBase.h
+++ include/clang/AST/DeclBase.h
@@ -1032,11 +1032,11 @@
 unsigned OldNS = IdentifierNamespace;
 assert((OldNS & (IDNS_Tag | IDNS_Ordinary |
  IDNS_TagFriend | IDNS_OrdinaryFriend |
-

r308306 - clang-format: [JS] Correctly format JavaScript imports with long module paths

2017-07-18 Thread Martin Probst via cfe-commits

Author: mprobst
Date: Tue Jul 18 07:00:19 2017
New Revision: 308306

URL: http://llvm.org/viewvc/llvm-project?rev=308306&view=rev
Log:
clang-format: [JS] Correctly format JavaScript imports with long module paths

Currently the `UnwrappedLineParser` fails to correctly unwrap JavaScript
imports where the module path is not on the same line as the `from` keyword.
For example:

import {A} from
'some/path/longer/than/column/limit/module.js';```

This causes issues when in the middle a list of imports because the formatter
thinks it has reached the end of the imports, and therefore will not sort any
imports lower in the list.

The formatter will, however, split the `from` keyword and the module path if
the path exceeds the column limit, which triggers the issue the next time the
file is formatted.

Patch originally by Jared Neil - thanks!

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

Modified:
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/unittests/Format/FormatTestJS.cpp
cfe/trunk/unittests/Format/SortImportsTestJS.cpp

Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=308306&r1=308305&r2=308306&view=diff
==
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Tue Jul 18 07:00:19 2017
@@ -747,7 +747,7 @@ static bool mustBeJSIdent(const Addition
   Keywords.kw_let, Keywords.kw_var, tok::kw_const,
   Keywords.kw_abstract, Keywords.kw_extends, 
Keywords.kw_implements,
   Keywords.kw_instanceof, Keywords.kw_interface,
-  Keywords.kw_throws));
+  Keywords.kw_throws, Keywords.kw_from));
 }
 
 static bool mustBeJSIdentOrValue(const AdditionalKeywords &Keywords,

Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=308306&r1=308305&r2=308306&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Tue Jul 18 07:00:19 2017
@@ -1464,6 +1464,17 @@ TEST_F(FormatTestJS, ImportWrapping) {
"  A,\n"
"} from 'some/module.js';",
Style);
+  Style.ColumnLimit = 40;
+  // Using this version of verifyFormat because test::messUp hides the issue.
+  verifyFormat("import {\n"
+   "  A,\n"
+   "} from\n"
+   "'some/path/longer/than/column/limit/module.js';",
+   " import  {  \n"
+   "A,  \n"
+   "  }from\n"
+   "  'some/path/longer/than/column/limit/module.js'  ; ",
+   Style);
 }
 
 TEST_F(FormatTestJS, TemplateStrings) {

Modified: cfe/trunk/unittests/Format/SortImportsTestJS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/SortImportsTestJS.cpp?rev=308306&r1=308305&r2=308306&view=diff
==
--- cfe/trunk/unittests/Format/SortImportsTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/SortImportsTestJS.cpp Tue Jul 18 07:00:19 2017
@@ -283,6 +283,23 @@ TEST_F(SortImportsTestJS, SortCaseInsens
  "1;");
 }
 
+TEST_F(SortImportsTestJS, SortMultiLine) {
+  // Reproduces issue where multi-line import was not parsed correctly.
+  verifySort("import {A} from 'a';\n"
+ "import {A} from 'b';\n"
+ "\n"
+ "1;",
+ "import\n"
+ "{\n"
+ "A\n"
+ "}\n"
+ "from\n"
+ "'b';\n"
+ "import {A} from 'a';\n"
+ "\n"
+ "1;");
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang


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

[PATCH] D35012: [refactor] Add the AST source selection component

2017-07-18 Thread Alex Lorenz via Phabricator via cfe-commits

arphaman added inline comments.



Comment at: lib/Tooling/Refactoring/ASTSelection.cpp:164
+  unsigned NumMatches = 0;
+  for (Decl *D : Context.getTranslationUnitDecl()->decls()) {
+if (ObjCImplEndLoc.isValid() &&

klimek wrote:
> arphaman wrote:
> > klimek wrote:
> > > Why don't we do this as part of TraverseDecl() in the visitor?
> > I think it's easier to handle the Objective-C `@implementation` logic here, 
> > unless there's some better way that I can't see ATM.
> Ok, in that case, can you write a comment at the start of the loop explaining 
> that we basically only do that for the Objective-C @implementation? (I'd also 
> like to understand that better in general, as I have no clue about Obj-C :)
Hmm, maybe it would be better to move this logic to another layer. Like a 
wrapper around `RecursiveASTVisitor` that ensures that iteration occurs in a 
lexical order. It can then be used by other things that might need this, this 
code will get simpler and I will be able to test it better.


Repository:
  rL LLVM

https://reviews.llvm.org/D35012



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

[PATCH] D35484: Add a warning for missing '#pragma pack (pop)' and suspicious '#pragma pack' uses when including files

2017-07-18 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!


Repository:
  rL LLVM

https://reviews.llvm.org/D35484



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

[PATCH] D35012: [refactor] Add the AST source selection component

2017-07-18 Thread Manuel Klimek via Phabricator via cfe-commits

klimek added inline comments.



Comment at: lib/Tooling/Refactoring/ASTSelection.cpp:164
+  unsigned NumMatches = 0;
+  for (Decl *D : Context.getTranslationUnitDecl()->decls()) {
+if (ObjCImplEndLoc.isValid() &&

arphaman wrote:
> klimek wrote:
> > arphaman wrote:
> > > klimek wrote:
> > > > Why don't we do this as part of TraverseDecl() in the visitor?
> > > I think it's easier to handle the Objective-C `@implementation` logic 
> > > here, unless there's some better way that I can't see ATM.
> > Ok, in that case, can you write a comment at the start of the loop 
> > explaining that we basically only do that for the Objective-C 
> > @implementation? (I'd also like to understand that better in general, as I 
> > have no clue about Obj-C :)
> Hmm, maybe it would be better to move this logic to another layer. Like a 
> wrapper around `RecursiveASTVisitor` that ensures that iteration occurs in a 
> lexical order. It can then be used by other things that might need this, this 
> code will get simpler and I will be able to test it better.
Great idea! That would be useful in a bunch of use cases.


Repository:
  rL LLVM

https://reviews.llvm.org/D35012



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

[PATCH] D34440: [Clang] Expand response files before loading compilation database

2017-07-18 Thread Vladimir Plyashkun via Phabricator via cfe-commits

vladimir.plyashkun added a comment.

> If you want one unified format, the compilation database is it.

Is the `clang-tidy ... -- ` meant to be more or less a drop-in 
replacement for `clang ` (arguments-wise)? 
If yes, this expansion of response files here is an another step in this 
direction.

Our only concerns are the internal ones (instead of using our well-tested 
response file implementation, we have to do something similar but different 
with JSON-based compilation database format). 
Obviously, it is possible to use it, and if this patch won't be accepted, we 
would. 
But then, again, I see this change as a positive by itself and don't really 
understand why the compiler driver should expand response arguments and the 
fixed compilation database shouldn't.

As IDE developers we don't have full control on the format of compiler options. 
They come from users in free form which compiler can understand.
From the implementation view it would be more transparent and efficient to 
transfer them in the original form to Clang-Tidy (instead of generating 
intermediate files).


Repository:
  rL LLVM

https://reviews.llvm.org/D34440



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

[PATCH] D34440: [Clang] Expand response files before loading compilation database

2017-07-18 Thread Manuel Klimek via Phabricator via cfe-commits

klimek added a comment.

In https://reviews.llvm.org/D34440#812938, @vladimir.plyashkun wrote:

> > If you want one unified format, the compilation database is it.
>
> Is the `clang-tidy ... -- ` meant to be more or less a drop-in 
> replacement for `clang ` (arguments-wise)? 
>  If yes, this expansion of response files here is an another step in this 
> direction.


Yes. I'm still confused why in this case
clang-tidy @file -- 
would be expected to expand the response file? Am I missing something?

> Our only concerns are the internal ones (instead of using our well-tested 
> response file implementation, we have to do something similar but different 
> with JSON-based compilation database format). 
>  Obviously, it is possible to use it, and if this patch won't be accepted, we 
> would.

I don't understand this. Can you elaborate?

> But then, again, I see this change as a positive by itself and don't really 
> understand why the compiler driver should expand response arguments and the 
> fixed compilation database shouldn't.
> 
> As IDE developers we don't have full control on the format of compiler 
> options. 
>  They come from users in free form which compiler can understand.
>  From the implementation view it would be more transparent and efficient to 
> transfer them in the original form to Clang-Tidy (instead of generating 
> intermediate files).

Why are intermediate files (or your own implementation of a 
CompilationDatabase) not in the "original form". What *is* the original form?


Repository:
  rL LLVM

https://reviews.llvm.org/D34440



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

[PATCH] D34824: clang-format: add an option -verbose to list the files being processed

2017-07-18 Thread Daniel Jasper via Phabricator via cfe-commits

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

Yeah, improving the testsuite generally seems like a good idea. But unrelated 
to this patch. This looks good now.


https://reviews.llvm.org/D34824



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

[PATCH] D35485: [clang-format] Fix comment levels between '}' and PPDirective

2017-07-18 Thread Daniel Jasper via Phabricator via cfe-commits

djasper added inline comments.



Comment at: lib/Format/UnwrappedLineParser.cpp:2378
   ScopedLineState BlockState(*this, SwitchToPreprocessorLines);
+  if (InitialLevel)
+Line->Level = *InitialLevel;

What happens if we always set the Level to 0 here?



Comment at: lib/Format/UnwrappedLineParser.cpp:489
 
-  nextToken(); // Munch the closing brace.
+  nextToken(InitialLevel); // Munch the closing brace.
 

krasimir wrote:
> djasper wrote:
> > What happens if you instead change the Line->Level = InitialLevel; 
> > statement from below to before this line? That seems like the more 
> > intuitively correct fix.
> This doesn't work since comments before the right brace haven't been emitted 
> yet and would get the wrong level.
So that means this seems to be the interesting case:

  void f() {
DoSomething();
// This was a fun function.
  }
  // Cool macro:
  #define A a

Now, both comments are basically read when we are reading the "}", but they 
should have different indentation levels. I have another suggestion, see below.



Comment at: unittests/Format/FormatTestComments.cpp:848
+"}",
+format("int f(int i) {\n"
+   "  if (true) {\n"

krasimir wrote:
> djasper wrote:
> > Generally, mess up the code in some way to ensure that it is actually being 
> > formatted.
> Messing up doesn't work in this case, because we rely on the original columns 
> of the comment and the previous line. That's why I added a bunch of tests.
I meant *manually* mess up. So add spaces here and there.


https://reviews.llvm.org/D35485



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

r308313 - CodeGen: Insert addr space cast for automatic/temp var at right position

2017-07-18 Thread Yaxun Liu via cfe-commits

Author: yaxunl
Date: Tue Jul 18 07:46:03 2017
New Revision: 308313

URL: http://llvm.org/viewvc/llvm-project?rev=308313&view=rev
Log:
CodeGen: Insert addr space cast for automatic/temp var at right position

The uses of alloca may be in different blocks other than the block containing 
the alloca.
Therefore if the alloca addr space is non-zero and it needs to be casted to 
default
address space, the cast needs to be inserted in the same BB as the alloca 
insted of
the current builder insert point since the current insert point may be in a 
different BB.

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

Modified:
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/test/CodeGenCXX/amdgcn-automatic-variable.cpp

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=308313&r1=308312&r2=308313&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Tue Jul 18 07:46:03 2017
@@ -73,9 +73,12 @@ Address CodeGenFunction::CreateTempAlloc
   // cast alloca to the default address space when necessary.
   if (CastToDefaultAddrSpace && getASTAllocaAddressSpace() != LangAS::Default) 
{
 auto DestAddrSpace = getContext().getTargetAddressSpace(LangAS::Default);
+auto CurIP = Builder.saveIP();
+Builder.SetInsertPoint(AllocaInsertPt);
 V = getTargetHooks().performAddrSpaceCast(
 *this, V, getASTAllocaAddressSpace(), LangAS::Default,
 Ty->getPointerTo(DestAddrSpace), /*non-null*/ true);
+Builder.restoreIP(CurIP);
   }
 
   return Address(V, Align);

Modified: cfe/trunk/test/CodeGenCXX/amdgcn-automatic-variable.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/amdgcn-automatic-variable.cpp?rev=308313&r1=308312&r2=308313&view=diff
==
--- cfe/trunk/test/CodeGenCXX/amdgcn-automatic-variable.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/amdgcn-automatic-variable.cpp Tue Jul 18 07:46:03 
2017
@@ -13,31 +13,32 @@ void func1(int *x) {
 // CHECK-LABEL: define void @_Z5func2v()
 void func2(void) {
   // CHECK: %lv1 = alloca i32, align 4, addrspace(5)
+  // CHECK: %[[r0:.*]] = addrspacecast i32 addrspace(5)* %lv1 to i32*
   // CHECK: %lv2 = alloca i32, align 4, addrspace(5)
+  // CHECK: %[[r1:.*]] = addrspacecast i32 addrspace(5)* %lv2 to i32*
   // CHECK: %la = alloca [100 x i32], align 4, addrspace(5)
+  // CHECK: %[[r2:.*]] = addrspacecast [100 x i32] addrspace(5)* %la to [100 x 
i32]*
   // CHECK: %lp1 = alloca i32*, align 8, addrspace(5)
+  // CHECK: %[[r3:.*]] = addrspacecast i32* addrspace(5)* %lp1 to i32**
   // CHECK: %lp2 = alloca i32*, align 8, addrspace(5)
+  // CHECK: %[[r4:.*]] = addrspacecast i32* addrspace(5)* %lp2 to i32**
   // CHECK: %lvc = alloca i32, align 4, addrspace(5)
+  // CHECK: %[[r5:.*]] = addrspacecast i32 addrspace(5)* %lvc to i32*
 
-  // CHECK: %[[r0:.*]] = addrspacecast i32 addrspace(5)* %lv1 to i32*
   // CHECK: store i32 1, i32* %[[r0]]
   int lv1;
   lv1 = 1;
-  // CHECK: %[[r1:.*]] = addrspacecast i32 addrspace(5)* %lv2 to i32*
   // CHECK: store i32 2, i32* %[[r1]]
   int lv2 = 2;
 
-  // CHECK: %[[r2:.*]] = addrspacecast [100 x i32] addrspace(5)* %la to [100 x 
i32]*
   // CHECK: %[[arrayidx:.*]] = getelementptr inbounds [100 x i32], [100 x 
i32]* %[[r2]], i64 0, i64 0
   // CHECK: store i32 3, i32* %[[arrayidx]], align 4
   int la[100];
   la[0] = 3;
 
-  // CHECK: %[[r3:.*]] = addrspacecast i32* addrspace(5)* %lp1 to i32**
   // CHECK: store i32* %[[r0]], i32** %[[r3]], align 8
   int *lp1 = &lv1;
 
-  // CHECK: %[[r4:.*]] = addrspacecast i32* addrspace(5)* %lp2 to i32**
   // CHECK: %[[arraydecay:.*]] = getelementptr inbounds [100 x i32], [100 x 
i32]* %[[r2]], i32 0, i32 0
   // CHECK: store i32* %[[arraydecay]], i32** %[[r4]], align 8
   int *lp2 = la;
@@ -45,7 +46,6 @@ void func2(void) {
   // CHECK: call void @_Z5func1Pi(i32* %[[r0]])
   func1(&lv1);
 
-  // CHECK: %[[r5:.*]] = addrspacecast i32 addrspace(5)* %lvc to i32*
   // CHECK: store i32 4, i32* %[[r5]]
   // CHECK: store i32 4, i32* %[[r0]]
   const int lvc = 4;
@@ -81,4 +81,25 @@ void func4(int x) {
   func1(&x);
 }
 
+// CHECK-LABEL: define void @_Z5func5v
+void func5() {
+  return;
+  int x = 0;
+}
+
+// CHECK-LABEL: define void @_Z5func6v
+void func6() {
+  return;
+  int x;
+}
+
+// CHECK-LABEL: define void @_Z5func7v
+extern void use(int *);
+void func7() {
+  goto later;
+  int x;
+later:
+  use(&x);
+}
+
 // CHECK-NOT: !opencl.ocl.version


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

[PATCH] D35562: Support GNU extension StmtExpr in constexpr function

2017-07-18 Thread Stephan Bergmann via Phabricator via cfe-commits

sberg created this revision.

The assert.h in glibc 2.25 defines assert as a GNU-extension "statement 
expression", see 
https://sourceware.org/git/?p=glibc.git;a=commit;h=e077349ce589466eecd47213db4fae6b80ec18c4
 "assert.h: allow gcc to detect assert(a = 1) errors".  That caused Clang's 
-std=gnu++14 to no longer treat constexpr functions containing assert as 
actually being constant expressions. (And I ran into that when trying to build 
LibreOffice master with Clang trunk against Fedora 26 glibc.)


https://reviews.llvm.org/D35562

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constant-expression-cxx1y.cpp


Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -781,9 +781,8 @@
 return ({ int n; n; }); // expected-note {{object of type 'int' is not 
initialized}}
   }
 
-  // FIXME: We should handle the void statement expression case.
-  constexpr int h() { // expected-error {{never produces a constant}}
-({ if (true) {} }); // expected-note {{not supported}}
+  constexpr int h() {
+({ if (true) {} });
 return 0;
   }
 }
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -4838,16 +4838,10 @@
 
 for (CompoundStmt::const_body_iterator BI = CS->body_begin(),
BE = CS->body_end();
- /**/; ++BI) {
-  if (BI + 1 == BE) {
-const Expr *FinalExpr = dyn_cast(*BI);
-if (!FinalExpr) {
-  Info.FFDiag((*BI)->getLocStart(),
-diag::note_constexpr_stmt_expr_unsupported);
-  return false;
-}
-return this->Visit(FinalExpr);
-  }
+ BI != BE; ++BI) {
+  if (BI + 1 == BE)
+if (const Expr *FinalExpr = dyn_cast(*BI))
+  return this->Visit(FinalExpr);
 
   APValue ReturnValue;
   StmtResult Result = { ReturnValue, nullptr };
@@ -4863,7 +4857,7 @@
   }
 }
 
-llvm_unreachable("Return from function from the loop above.");
+return true;
   }
 
   /// Visit a value which is evaluated, but whose value is ignored.


Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -781,9 +781,8 @@
 return ({ int n; n; }); // expected-note {{object of type 'int' is not initialized}}
   }
 
-  // FIXME: We should handle the void statement expression case.
-  constexpr int h() { // expected-error {{never produces a constant}}
-({ if (true) {} }); // expected-note {{not supported}}
+  constexpr int h() {
+({ if (true) {} });
 return 0;
   }
 }
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -4838,16 +4838,10 @@
 
 for (CompoundStmt::const_body_iterator BI = CS->body_begin(),
BE = CS->body_end();
- /**/; ++BI) {
-  if (BI + 1 == BE) {
-const Expr *FinalExpr = dyn_cast(*BI);
-if (!FinalExpr) {
-  Info.FFDiag((*BI)->getLocStart(),
-diag::note_constexpr_stmt_expr_unsupported);
-  return false;
-}
-return this->Visit(FinalExpr);
-  }
+ BI != BE; ++BI) {
+  if (BI + 1 == BE)
+if (const Expr *FinalExpr = dyn_cast(*BI))
+  return this->Visit(FinalExpr);
 
   APValue ReturnValue;
   StmtResult Result = { ReturnValue, nullptr };
@@ -4863,7 +4857,7 @@
   }
 }
 
-llvm_unreachable("Return from function from the loop above.");
+return true;
   }
 
   /// Visit a value which is evaluated, but whose value is ignored.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35438: CodeGen: Insert addr space cast for automatic/temp var at right position

2017-07-18 Thread Yaxun Liu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL308313: CodeGen: Insert addr space cast for automatic/temp 
var at right position (authored by yaxunl).

Changed prior to commit:
  https://reviews.llvm.org/D35438?vs=106910&id=107097#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D35438

Files:
  cfe/trunk/lib/CodeGen/CGExpr.cpp
  cfe/trunk/test/CodeGenCXX/amdgcn-automatic-variable.cpp


Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -73,9 +73,12 @@
   // cast alloca to the default address space when necessary.
   if (CastToDefaultAddrSpace && getASTAllocaAddressSpace() != LangAS::Default) 
{
 auto DestAddrSpace = getContext().getTargetAddressSpace(LangAS::Default);
+auto CurIP = Builder.saveIP();
+Builder.SetInsertPoint(AllocaInsertPt);
 V = getTargetHooks().performAddrSpaceCast(
 *this, V, getASTAllocaAddressSpace(), LangAS::Default,
 Ty->getPointerTo(DestAddrSpace), /*non-null*/ true);
+Builder.restoreIP(CurIP);
   }
 
   return Address(V, Align);
Index: cfe/trunk/test/CodeGenCXX/amdgcn-automatic-variable.cpp
===
--- cfe/trunk/test/CodeGenCXX/amdgcn-automatic-variable.cpp
+++ cfe/trunk/test/CodeGenCXX/amdgcn-automatic-variable.cpp
@@ -13,39 +13,39 @@
 // CHECK-LABEL: define void @_Z5func2v()
 void func2(void) {
   // CHECK: %lv1 = alloca i32, align 4, addrspace(5)
+  // CHECK: %[[r0:.*]] = addrspacecast i32 addrspace(5)* %lv1 to i32*
   // CHECK: %lv2 = alloca i32, align 4, addrspace(5)
+  // CHECK: %[[r1:.*]] = addrspacecast i32 addrspace(5)* %lv2 to i32*
   // CHECK: %la = alloca [100 x i32], align 4, addrspace(5)
+  // CHECK: %[[r2:.*]] = addrspacecast [100 x i32] addrspace(5)* %la to [100 x 
i32]*
   // CHECK: %lp1 = alloca i32*, align 8, addrspace(5)
+  // CHECK: %[[r3:.*]] = addrspacecast i32* addrspace(5)* %lp1 to i32**
   // CHECK: %lp2 = alloca i32*, align 8, addrspace(5)
+  // CHECK: %[[r4:.*]] = addrspacecast i32* addrspace(5)* %lp2 to i32**
   // CHECK: %lvc = alloca i32, align 4, addrspace(5)
+  // CHECK: %[[r5:.*]] = addrspacecast i32 addrspace(5)* %lvc to i32*
 
-  // CHECK: %[[r0:.*]] = addrspacecast i32 addrspace(5)* %lv1 to i32*
   // CHECK: store i32 1, i32* %[[r0]]
   int lv1;
   lv1 = 1;
-  // CHECK: %[[r1:.*]] = addrspacecast i32 addrspace(5)* %lv2 to i32*
   // CHECK: store i32 2, i32* %[[r1]]
   int lv2 = 2;
 
-  // CHECK: %[[r2:.*]] = addrspacecast [100 x i32] addrspace(5)* %la to [100 x 
i32]*
   // CHECK: %[[arrayidx:.*]] = getelementptr inbounds [100 x i32], [100 x 
i32]* %[[r2]], i64 0, i64 0
   // CHECK: store i32 3, i32* %[[arrayidx]], align 4
   int la[100];
   la[0] = 3;
 
-  // CHECK: %[[r3:.*]] = addrspacecast i32* addrspace(5)* %lp1 to i32**
   // CHECK: store i32* %[[r0]], i32** %[[r3]], align 8
   int *lp1 = &lv1;
 
-  // CHECK: %[[r4:.*]] = addrspacecast i32* addrspace(5)* %lp2 to i32**
   // CHECK: %[[arraydecay:.*]] = getelementptr inbounds [100 x i32], [100 x 
i32]* %[[r2]], i32 0, i32 0
   // CHECK: store i32* %[[arraydecay]], i32** %[[r4]], align 8
   int *lp2 = la;
 
   // CHECK: call void @_Z5func1Pi(i32* %[[r0]])
   func1(&lv1);
 
-  // CHECK: %[[r5:.*]] = addrspacecast i32 addrspace(5)* %lvc to i32*
   // CHECK: store i32 4, i32* %[[r5]]
   // CHECK: store i32 4, i32* %[[r0]]
   const int lvc = 4;
@@ -81,4 +81,25 @@
   func1(&x);
 }
 
+// CHECK-LABEL: define void @_Z5func5v
+void func5() {
+  return;
+  int x = 0;
+}
+
+// CHECK-LABEL: define void @_Z5func6v
+void func6() {
+  return;
+  int x;
+}
+
+// CHECK-LABEL: define void @_Z5func7v
+extern void use(int *);
+void func7() {
+  goto later;
+  int x;
+later:
+  use(&x);
+}
+
 // CHECK-NOT: !opencl.ocl.version


Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -73,9 +73,12 @@
   // cast alloca to the default address space when necessary.
   if (CastToDefaultAddrSpace && getASTAllocaAddressSpace() != LangAS::Default) {
 auto DestAddrSpace = getContext().getTargetAddressSpace(LangAS::Default);
+auto CurIP = Builder.saveIP();
+Builder.SetInsertPoint(AllocaInsertPt);
 V = getTargetHooks().performAddrSpaceCast(
 *this, V, getASTAllocaAddressSpace(), LangAS::Default,
 Ty->getPointerTo(DestAddrSpace), /*non-null*/ true);
+Builder.restoreIP(CurIP);
   }
 
   return Address(V, Align);
Index: cfe/trunk/test/CodeGenCXX/amdgcn-automatic-variable.cpp
===
--- cfe/trunk/test/CodeGenCXX/amdgcn-automatic-variable.cpp
+++ cfe/trunk/test/CodeGenCXX/amdgcn-automatic-variable.cpp
@@ -13,39 +13,39 @@
 // CHECK-LABEL: define void @_Z5func2v()
 void func2(void) {
   // CHECK: %lv1 = alloca i32, align 4, addrspace(5

[PATCH] D35548: [mips] Teach the driver to accept -m(no-)gpopt.

2017-07-18 Thread Simon Dardis via Phabricator via cfe-commits

sdardis added inline comments.



Comment at: lib/Driver/ToolChains/Clang.cpp:1490
+  GPOpt->claim();
+  }
+

atanasyan wrote:
> Could it be rewritten a bit shorter?
> 
> ```
> bool NoAbiCalls =
> ABICalls && ABICalls->getOption().matches(options::OPT_mno_abicalls);
> bool WantGPOpt = GPOpt && GPOpt->getOption().matches(options::OPT_mgpopt);
> 
> if (NoAbiCalls && (!GPOpt || WantGPOpt)) {
>   CmdArgs.push_back("-mllvm");
>   CmdArgs.push_back("-mgpopt=1");
> } else {
>   CmdArgs.push_back("-mllvm");
>   CmdArgs.push_back("-mgpopt=0");
> }
> 
> if (GPOpt)
>   GPOpt->claim();
> ```
Yes, it can. I think I mis-handled the case where -mabicalls and -mgpopt are 
combined. I'll reflow the logic along the lines of your suggestion and add a 
warning.


https://reviews.llvm.org/D35548



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

[PATCH] D35519: Add SEMA checking to attribute 'target'

2017-07-18 Thread Erich Keane via Phabricator via cfe-commits

erichkeane updated this revision to Diff 107101.
erichkeane marked 9 inline comments as done.
erichkeane added a comment.

Thanks for the feedback Aaron.  This should fix everything you had an issue 
with.

-Erich


https://reviews.llvm.org/D35519

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/TargetInfo.h
  include/clang/Sema/Sema.h
  lib/Basic/Targets.cpp
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/attr-target.c

Index: test/Sema/attr-target.c
===
--- test/Sema/attr-target.c
+++ test/Sema/attr-target.c
@@ -2,7 +2,12 @@
 
 int __attribute__((target("avx,sse4.2,arch=ivybridge"))) foo() { return 4; }
 int __attribute__((target())) bar() { return 4; } //expected-error {{'target' attribute takes one argument}}
-int __attribute__((target("tune=sandybridge"))) baz() { return 4; } //expected-warning {{Ignoring unsupported 'tune=' in the target attribute string}}
-int __attribute__((target("fpmath=387"))) walrus() { return 4; } //expected-warning {{Ignoring unsupported 'fpmath=' in the target attribute string}}
+int __attribute__((target("tune=sandybridge"))) baz() { return 4; } //expected-warning {{ignoring unsupported 'tune=' in the target attribute string}}
+int __attribute__((target("fpmath=387"))) walrus() { return 4; } //expected-warning {{ignoring unsupported 'fpmath=' in the target attribute string}}
+int __attribute__((target("avx,sse4.2,arch=hiss"))) meow() {  return 4; }//expected-warning {{ignoring unsupported architecture 'hiss' in the target attribute string}}
+int __attribute__((target("woof"))) bark() {  return 4; }//expected-warning {{ignoring unsupported 'woof' in the target attribute string}}
+int __attribute__((target("arch="))) turtle() { return 4; } // no warning, same as saying 'nothing'.
+int __attribute__((target("arch=hiss,arch=woof"))) pine_tree() { return 4; } //expected-warning {{ignoring duplicate 'arch=' in the target attribute string}}
+
 
 
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -2993,22 +2993,42 @@
 D->addAttr(NewAttr);
 }
 
-// Check for things we'd like to warn about, no errors or validation for now.
-// TODO: Validation should use a backend target library that specifies
-// the allowable subtarget features and cpus. We could use something like a
-// TargetCodeGenInfo hook here to do validation.
-void Sema::checkTargetAttr(SourceLocation LiteralLoc, StringRef AttrStr) {
+// Check for things we'd like to warn about. Multiversioning issues are
+// handled later in the process, once we know how many exist.
+bool Sema::checkTargetAttr(SourceLocation LiteralLoc, StringRef AttrStr) {
+  // Error message enums.  Enum vs enum class since the scope is limited,
+  // and it otherwise simplifies the error messages.
+  enum FirstParam { unsupported, duplicate};
+  enum SecondParam { None, architecture };
   for (auto Str : {"tune=", "fpmath="})
 if (AttrStr.find(Str) != StringRef::npos)
-  Diag(LiteralLoc, diag::warn_unsupported_target_attribute) << Str;
+  return Diag(LiteralLoc, diag::warn_unsupported_target_attribute) << unsupported << None << Str;
+
+  TargetAttr::ParsedTargetAttr ParsedAttrs = TargetAttr::parse(AttrStr);
+  if (ParsedAttrs.DuplicateArchitecture)
+return Diag(LiteralLoc, diag::warn_unsupported_target_attribute) << duplicate << None << "arch=";
+
+  if (!ParsedAttrs.Architecture.empty() &&
+  !Context.getTargetInfo().isValidCPUName(ParsedAttrs.Architecture))
+return Diag(LiteralLoc, diag::warn_unsupported_target_attribute) << unsupported << architecture
+   << ParsedAttrs.Architecture;
+
+  for (const auto &Feature : ParsedAttrs.Features) {
+auto CurFeature = StringRef(Feature).drop_front(); // remove + or -.
+if (!Context.getTargetInfo().isValidFeatureName(CurFeature))
+  return Diag(LiteralLoc, diag::warn_unsupported_target_attribute) << unsupported << None
+ << CurFeature;
+  }
+
+  return true;
 }
 
 static void handleTargetAttr(Sema &S, Decl *D, const AttributeList &Attr) {
   StringRef Str;
   SourceLocation LiteralLoc;
-  if (!S.checkStringLiteralArgumentAttr(Attr, 0, Str, &LiteralLoc))
+  if (!S.checkStringLiteralArgumentAttr(Attr, 0, Str, &LiteralLoc) ||
+  !S.checkTargetAttr(LiteralLoc, Str))
 return;
-  S.checkTargetAttr(LiteralLoc, Str);
   unsigned Index = Attr.getAttributeSpellingListIndex();
   TargetAttr *NewAttr =
   ::new (S.Context) TargetAttr(Attr.getRange(), S.Context, Str, Index);
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -4499,18 +4499,19 @@
 
 // Make a copy of the features as passed on the command line into the
 // beginning of the additiona

[PATCH] D35485: [clang-format] Fix comment levels between '}' and PPDirective

2017-07-18 Thread Krasimir Georgiev via Phabricator via cfe-commits

krasimir marked 2 inline comments as done.
krasimir added inline comments.



Comment at: lib/Format/UnwrappedLineParser.cpp:489
 
-  nextToken(); // Munch the closing brace.
+  nextToken(InitialLevel); // Munch the closing brace.
 

djasper wrote:
> krasimir wrote:
> > djasper wrote:
> > > What happens if you instead change the Line->Level = InitialLevel; 
> > > statement from below to before this line? That seems like the more 
> > > intuitively correct fix.
> > This doesn't work since comments before the right brace haven't been 
> > emitted yet and would get the wrong level.
> So that means this seems to be the interesting case:
> 
>   void f() {
> DoSomething();
> // This was a fun function.
>   }
>   // Cool macro:
>   #define A a
> 
> Now, both comments are basically read when we are reading the "}", but they 
> should have different indentation levels. I have another suggestion, see 
> below.
Here is another breaking test in case we change `Line->Level = InitialLevel` to 
above this line:
```
switch (x) {
default: {
  // Do nothing.
}
}
```
gets reformatted as:
```
switch (x) {
default: {
// Do nothing.
}
}
```



Comment at: lib/Format/UnwrappedLineParser.cpp:2378
   ScopedLineState BlockState(*this, SwitchToPreprocessorLines);
+  if (InitialLevel)
+Line->Level = *InitialLevel;

djasper wrote:
> What happens if we always set the Level to 0 here?
If we always set the Level to 0 here, then
```
void  f() {
  int i;
  /* comment */

#define A
}
```
gets indented as
```
void  f() {
  int i;
/* comment */

#define A
}
```


https://reviews.llvm.org/D35485



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

[PATCH] D35564: Suppress -pedantic warnings about GNU extension StmtExpr in glibc's assert macro

2017-07-18 Thread Stephan Bergmann via Phabricator via cfe-commits

sberg created this revision.

See the mail thread starting at 
http://lists.llvm.org/pipermail/cfe-dev/2017-July/054666.html "[cfe-dev] 
-pedantic warnings in system headers?":

The assert.h in glibc 2.25 defines assert as a GNU-extension "statement 
expression", see 
https://sourceware.org/git/?p=glibc.git;a=commit;h=e077349ce589466eecd47213db4fae6b80ec18c4
 "assert.h: allow gcc to detect assert(a = 1) errors". That caused Clang 
-pedantic to emit -Wgnu-statement-expression warnings (unlike GCC).

This patch extends the heuristic when to suppress warnings in system headers, 
to also suppress warnings about extensions used in the body of a function-like 
macro defined in a system header.  (The "function-like" part is there so that 
the warning from the body of __LONG_LONG_MAX__ in clang/test/CXX/drs/dr4xx.cpp 
is still emitted.)

I ran into this when trying to build CoinMP (which wants to be built with 
-pedantic-errors) as part of building LibreOffice master with Clang trunk 
against Fedora 26 glibc.  I'm not sure what the best solution is, though: 
Either extend the heuristic when to suppress warnings as done here (which has 
the benefit of aligning the behavior with GCC), or change the CoinMP build 
system to not insist on -pedantic-error.


https://reviews.llvm.org/D35564

Files:
  clang/lib/Basic/DiagnosticIDs.cpp
  clang/test/SemaCXX/warn-sysheader-macro.cpp


Index: clang/test/SemaCXX/warn-sysheader-macro.cpp
===
--- clang/test/SemaCXX/warn-sysheader-macro.cpp
+++ clang/test/SemaCXX/warn-sysheader-macro.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify -fsyntax-only -Wshadow -Wold-style-cast %s
+// RUN: %clang_cc1 -verify -fsyntax-only -Wshadow -Wold-style-cast -pedantic %s
 
 // Test that macro expansions from system headers don't trigger 'syntactic'
 // warnings that are not actionable.
@@ -12,6 +12,8 @@
 
 #define OLD_STYLE_CAST(a) ((int) (a))
 
+#define ASSERT(a) ({ if (a) {} else {} })
+
 #else
 
 #define IS_SYSHEADER
@@ -32,4 +34,9 @@
   int i = OLD_STYLE_CAST(0);
 }
 
+void testExtension() {
+  ASSERT(true);
+  ASSERT(({ true; })); // expected-warning {{use of GNU statement expression 
extension}}
+}
+
 #endif
Index: clang/lib/Basic/DiagnosticIDs.cpp
===
--- clang/lib/Basic/DiagnosticIDs.cpp
+++ clang/lib/Basic/DiagnosticIDs.cpp
@@ -396,6 +396,14 @@
   return toLevel(getDiagnosticSeverity(DiagID, Loc, Diag));
 }
 
+static bool isSystemFunctionMacroBodyExpansion(SourceLocation Loc,
+   const SourceManager &SM) {
+  if (!(SM.isInSystemMacro(Loc) && SM.isMacroBodyExpansion(Loc)))
+return false;
+  auto Range = SM.getImmediateExpansionRange(Loc);
+  return Range.first != Range.second;
+}
+
 /// \brief Based on the way the client configured the Diagnostic
 /// object, classify the specified diagnostic ID into a Level, consumable by
 /// the DiagnosticClient.
@@ -469,8 +477,10 @@
   // because we also want to ignore extensions and warnings in -Werror and
   // -pedantic-errors modes, which *map* warnings/extensions to errors.
   if (State->SuppressSystemWarnings && !ShowInSystemHeader && Loc.isValid() &&
-  Diag.getSourceManager().isInSystemHeader(
-  Diag.getSourceManager().getExpansionLoc(Loc)))
+  (Diag.getSourceManager().isInSystemHeader(
+  Diag.getSourceManager().getExpansionLoc(Loc)) ||
+   (IsExtensionDiag &&
+isSystemFunctionMacroBodyExpansion(Loc, Diag.getSourceManager()
 return diag::Severity::Ignored;
 
   return Result;


Index: clang/test/SemaCXX/warn-sysheader-macro.cpp
===
--- clang/test/SemaCXX/warn-sysheader-macro.cpp
+++ clang/test/SemaCXX/warn-sysheader-macro.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify -fsyntax-only -Wshadow -Wold-style-cast %s
+// RUN: %clang_cc1 -verify -fsyntax-only -Wshadow -Wold-style-cast -pedantic %s
 
 // Test that macro expansions from system headers don't trigger 'syntactic'
 // warnings that are not actionable.
@@ -12,6 +12,8 @@
 
 #define OLD_STYLE_CAST(a) ((int) (a))
 
+#define ASSERT(a) ({ if (a) {} else {} })
+
 #else
 
 #define IS_SYSHEADER
@@ -32,4 +34,9 @@
   int i = OLD_STYLE_CAST(0);
 }
 
+void testExtension() {
+  ASSERT(true);
+  ASSERT(({ true; })); // expected-warning {{use of GNU statement expression extension}}
+}
+
 #endif
Index: clang/lib/Basic/DiagnosticIDs.cpp
===
--- clang/lib/Basic/DiagnosticIDs.cpp
+++ clang/lib/Basic/DiagnosticIDs.cpp
@@ -396,6 +396,14 @@
   return toLevel(getDiagnosticSeverity(DiagID, Loc, Diag));
 }
 
+static bool isSystemFunctionMacroBodyExpansion(SourceLocation Loc,
+   const SourceManager &SM) {
+  if (!(SM.isInSystemMacro(Loc) && SM.isMacroBodyExpansion(Loc)))
+return false;
+  auto Range = SM.getImmediateExpansionR

Re: r308044 - Add documentation for @available

2017-07-18 Thread Nico Weber via cfe-commits

On Mon, Jul 17, 2017 at 8:39 AM, Aaron Ballman 
wrote:

> On Sun, Jul 16, 2017 at 7:49 PM, Nico Weber  wrote:
> > Aaron, https://clang.llvm.org/docs/AttributeReference.html#availability
> > still doesn't have the AttrDocs.td change I made in this change 2 days
> ago.
> > Do I have to do anything to get it to update?
>
> No, it's expected to update once a day, usually pretty early in the
> morning EST (ridiculously early PST).
>
> Tanya, I've not seen any failure emails for building from AttrDocs.td.
> Did I get removed from that list when we updated? If so, I'm happy to
> continue to receive it to help resolve problems, but in the interim,
> do you happen to know more about what's going on?
>

Tanya, ping?


>
> Thanks!
>
> ~Aaron
>
> >
> > On Fri, Jul 14, 2017 at 2:40 PM, Nico Weber via cfe-commits
> >  wrote:
> >>
> >> Author: nico
> >> Date: Fri Jul 14 11:40:52 2017
> >> New Revision: 308044
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=308044&view=rev
> >> Log:
> >> Add documentation for @available
> >>
> >> https://reviews.llvm.org/D35379
> >>
> >> Modified:
> >> cfe/trunk/docs/LanguageExtensions.rst
> >> cfe/trunk/include/clang/Basic/AttrDocs.td
> >>
> >> Modified: cfe/trunk/docs/LanguageExtensions.rst
> >> URL:
> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/
> LanguageExtensions.rst?rev=308044&r1=308043&r2=308044&view=diff
> >>
> >> 
> ==
> >> --- cfe/trunk/docs/LanguageExtensions.rst (original)
> >> +++ cfe/trunk/docs/LanguageExtensions.rst Fri Jul 14 11:40:52 2017
> >> @@ -1271,6 +1271,87 @@ Further examples of these attributes are
> >>  Query for these features with ``__has_attribute(ns_consumed)``,
> >>  ``__has_attribute(ns_returns_retained)``, etc.
> >>
> >> +Objective-C @available
> >> +--
> >> +
> >> +It is possible to use the newest SDK but still build a program that can
> >> run on
> >> +older versions of macOS and iOS by passing ``-mmacosx-version-info=`` /
> >> +``--miphoneos-version-min=``.
> >> +
> >> +Before LLVM 5.0, when calling a function that exists only in the OS
> >> that's
> >> +newer than the target OS (as determined by the minimum deployment
> >> version),
> >> +programmers had to carefully check if the function exists at runtime,
> >> using
> >> +null checks for weakly-linked C functions, ``+class`` for Objective-C
> >> classes,
> >> +and ``-respondsToSelector:`` or ``+instancesRespondToSelector:`` for
> >> +Objective-C methods.  If such a check was missed, the program would
> >> compile
> >> +fine, run fine on newer systems, but crash on older systems.
> >> +
> >> +As of LLVM 5.0, ``-Wunguarded-availability`` uses the `availability
> >> attributes
> >> +`_
> >> together
> >> +with the new ``@available()`` keyword to assist with this issue.
> >> +When a method that's introduced in the OS newer than the target OS is
> >> called, a
> >> +-Wunguarded-availability warning is emitted if that call is not
> guarded:
> >> +
> >> +.. code-block:: objc
> >> +
> >> +  void my_fun(NSSomeClass* var) {
> >> +// If fancyNewMethod was added in e.g. macOS 10.12, but the code is
> >> +// built with -mmacosx-version-min=10.11, then this unconditional
> >> call
> >> +// will emit a -Wunguarded-availability warning:
> >> +[var fancyNewMethod];
> >> +  }
> >> +
> >> +To fix the warning and to avoid the crash on macOS 10.11, wrap it in
> >> +``if(@available())``:
> >> +
> >> +.. code-block:: objc
> >> +
> >> +  void my_fun(NSSomeClass* var) {
> >> +if (@available(macOS 10.12, *)) {
> >> +  [var fancyNewMethod];
> >> +} else {
> >> +  // Put fallback behavior for old macOS versions (and for non-mac
> >> +  // platforms) here.
> >> +}
> >> +  }
> >> +
> >> +The ``*`` is required and means that platforms not explicitly listed
> will
> >> take
> >> +the true branch, and the compiler will emit
> ``-Wunguarded-availability``
> >> +warnings for unlisted platforms based on those platform's deployment
> >> target.
> >> +More than one platform can be listed in ``@available()``:
> >> +
> >> +.. code-block:: objc
> >> +
> >> +  void my_fun(NSSomeClass* var) {
> >> +if (@available(macOS 10.12, iOS 10, *)) {
> >> +  [var fancyNewMethod];
> >> +}
> >> +  }
> >> +
> >> +If the caller of ``my_fun()`` already checks that ``my_fun()`` is only
> >> called
> >> +on 10.12, then add an `availability attribute
> >> +`_ to
> >> it,
> >> +which will also suppress the warning and require that calls to my_fun()
> >> are
> >> +checked:
> >> +
> >> +.. code-block:: objc
> >> +
> >> +  API_AVAILABLE(macos(10.12)) void my_fun(NSSomeClass* var) {
> >> +[var fancyNewMethod];  // Now ok.
> >> +  }
> >> +
> >> +``@available()`` is only available in Objective-C code.  To use the
> >> feature
> >> +in C and C++ code, use the ``__bui

[PATCH] D35564: Suppress -pedantic warnings about GNU extension StmtExpr in glibc's assert macro

2017-07-18 Thread David Blaikie via Phabricator via cfe-commits

dblaikie added a comment.

Ping - Richard, what'd you reckon? (This came up on another review where 
-Wzero-as-null-pointer triggered in a system header in libstdc++ I think)


https://reviews.llvm.org/D35564



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

[PATCH] D35519: Add SEMA checking to attribute 'target'

2017-07-18 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.

A few small nits, but otherwise LGTM




Comment at: lib/Sema/SemaDeclAttr.cpp:2999-3000
+bool Sema::checkTargetAttr(SourceLocation LiteralLoc, StringRef AttrStr) {
+  // Error message enums.  Enum vs enum class since the scope is limited,
+  // and it otherwise simplifies the error messages.
+  enum FirstParam { unsupported, duplicate};

I don't think this comment is needed (the code is pretty obvious), so I'd just 
drop it entirely.



Comment at: lib/Sema/SemaDeclAttr.cpp:3001-3002
+  // and it otherwise simplifies the error messages.
+  enum FirstParam { unsupported, duplicate};
+  enum SecondParam { None, architecture };
   for (auto Str : {"tune=", "fpmath="})

enumerator casing should be Unsupported, Duplicate, Architecture.


https://reviews.llvm.org/D35519



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

Re: r308044 - Add documentation for @available

2017-07-18 Thread via cfe-commits

Its getting an error. Will update in a bit when I can.

-Tanya

> On Jul 18, 2017, at 8:07 AM, Nico Weber  wrote:
> 
>> On Mon, Jul 17, 2017 at 8:39 AM, Aaron Ballman  
>> wrote:
>> On Sun, Jul 16, 2017 at 7:49 PM, Nico Weber  wrote:
>> > Aaron, https://clang.llvm.org/docs/AttributeReference.html#availability
>> > still doesn't have the AttrDocs.td change I made in this change 2 days ago.
>> > Do I have to do anything to get it to update?
>> 
>> No, it's expected to update once a day, usually pretty early in the
>> morning EST (ridiculously early PST).
>> 
>> Tanya, I've not seen any failure emails for building from AttrDocs.td.
>> Did I get removed from that list when we updated? If so, I'm happy to
>> continue to receive it to help resolve problems, but in the interim,
>> do you happen to know more about what's going on?
> 
> Tanya, ping?
>  
>> 
>> Thanks!
>> 
>> ~Aaron
>> 
>> >
>> > On Fri, Jul 14, 2017 at 2:40 PM, Nico Weber via cfe-commits
>> >  wrote:
>> >>
>> >> Author: nico
>> >> Date: Fri Jul 14 11:40:52 2017
>> >> New Revision: 308044
>> >>
>> >> URL: http://llvm.org/viewvc/llvm-project?rev=308044&view=rev
>> >> Log:
>> >> Add documentation for @available
>> >>
>> >> https://reviews.llvm.org/D35379
>> >>
>> >> Modified:
>> >> cfe/trunk/docs/LanguageExtensions.rst
>> >> cfe/trunk/include/clang/Basic/AttrDocs.td
>> >>
>> >> Modified: cfe/trunk/docs/LanguageExtensions.rst
>> >> URL:
>> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LanguageExtensions.rst?rev=308044&r1=308043&r2=308044&view=diff
>> >>
>> >> ==
>> >> --- cfe/trunk/docs/LanguageExtensions.rst (original)
>> >> +++ cfe/trunk/docs/LanguageExtensions.rst Fri Jul 14 11:40:52 2017
>> >> @@ -1271,6 +1271,87 @@ Further examples of these attributes are
>> >>  Query for these features with ``__has_attribute(ns_consumed)``,
>> >>  ``__has_attribute(ns_returns_retained)``, etc.
>> >>
>> >> +Objective-C @available
>> >> +--
>> >> +
>> >> +It is possible to use the newest SDK but still build a program that can
>> >> run on
>> >> +older versions of macOS and iOS by passing ``-mmacosx-version-info=`` /
>> >> +``--miphoneos-version-min=``.
>> >> +
>> >> +Before LLVM 5.0, when calling a function that exists only in the OS
>> >> that's
>> >> +newer than the target OS (as determined by the minimum deployment
>> >> version),
>> >> +programmers had to carefully check if the function exists at runtime,
>> >> using
>> >> +null checks for weakly-linked C functions, ``+class`` for Objective-C
>> >> classes,
>> >> +and ``-respondsToSelector:`` or ``+instancesRespondToSelector:`` for
>> >> +Objective-C methods.  If such a check was missed, the program would
>> >> compile
>> >> +fine, run fine on newer systems, but crash on older systems.
>> >> +
>> >> +As of LLVM 5.0, ``-Wunguarded-availability`` uses the `availability
>> >> attributes
>> >> +`_
>> >> together
>> >> +with the new ``@available()`` keyword to assist with this issue.
>> >> +When a method that's introduced in the OS newer than the target OS is
>> >> called, a
>> >> +-Wunguarded-availability warning is emitted if that call is not guarded:
>> >> +
>> >> +.. code-block:: objc
>> >> +
>> >> +  void my_fun(NSSomeClass* var) {
>> >> +// If fancyNewMethod was added in e.g. macOS 10.12, but the code is
>> >> +// built with -mmacosx-version-min=10.11, then this unconditional
>> >> call
>> >> +// will emit a -Wunguarded-availability warning:
>> >> +[var fancyNewMethod];
>> >> +  }
>> >> +
>> >> +To fix the warning and to avoid the crash on macOS 10.11, wrap it in
>> >> +``if(@available())``:
>> >> +
>> >> +.. code-block:: objc
>> >> +
>> >> +  void my_fun(NSSomeClass* var) {
>> >> +if (@available(macOS 10.12, *)) {
>> >> +  [var fancyNewMethod];
>> >> +} else {
>> >> +  // Put fallback behavior for old macOS versions (and for non-mac
>> >> +  // platforms) here.
>> >> +}
>> >> +  }
>> >> +
>> >> +The ``*`` is required and means that platforms not explicitly listed will
>> >> take
>> >> +the true branch, and the compiler will emit ``-Wunguarded-availability``
>> >> +warnings for unlisted platforms based on those platform's deployment
>> >> target.
>> >> +More than one platform can be listed in ``@available()``:
>> >> +
>> >> +.. code-block:: objc
>> >> +
>> >> +  void my_fun(NSSomeClass* var) {
>> >> +if (@available(macOS 10.12, iOS 10, *)) {
>> >> +  [var fancyNewMethod];
>> >> +}
>> >> +  }
>> >> +
>> >> +If the caller of ``my_fun()`` already checks that ``my_fun()`` is only
>> >> called
>> >> +on 10.12, then add an `availability attribute
>> >> +`_ to
>> >> it,
>> >> +which will also suppress the warning and require that calls to my_fun()
>> >> are
>> >> +checked:
>> >> +
>> >> +.. code-block:: objc
>> >> +
>> >> +  API

r308317 - [OPENMP] Generalization of sema analysis of reduction-based clauses,

2017-07-18 Thread Alexey Bataev via cfe-commits

Author: abataev
Date: Tue Jul 18 08:32:58 2017
New Revision: 308317

URL: http://llvm.org/viewvc/llvm-project?rev=308317&view=rev
Log:
[OPENMP] Generalization of sema analysis of reduction-based clauses,
NFC.

Modified:
cfe/trunk/lib/Sema/SemaOpenMP.cpp

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=308317&r1=308316&r2=308317&view=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Tue Jul 18 08:32:58 2017
@@ -8903,15 +8903,66 @@ buildDeclareReductionRef(Sema &SemaRef,
   return ExprEmpty();
 }
 
-OMPClause *Sema::ActOnOpenMPReductionClause(
-ArrayRef VarList, SourceLocation StartLoc, SourceLocation 
LParenLoc,
-SourceLocation ColonLoc, SourceLocation EndLoc,
-CXXScopeSpec &ReductionIdScopeSpec, const DeclarationNameInfo &ReductionId,
-ArrayRef UnresolvedReductions) {
+namespace {
+/// Data for the reduction-based clauses.
+struct ReductionData {
+  /// List of original reduction items.
+  SmallVector Vars;
+  /// List of private copies of the reduction items.
+  SmallVector Privates;
+  /// LHS expressions for the reduction_op expressions.
+  SmallVector LHSs;
+  /// RHS expressions for the reduction_op expressions.
+  SmallVector RHSs;
+  /// Reduction operation expression.
+  SmallVector ReductionOps;
+  /// List of captures for clause.
+  SmallVector ExprCaptures;
+  /// List of postupdate expressions.
+  SmallVector ExprPostUpdates;
+  ReductionData() = delete;
+  /// Reserves required memory for the reduction data.
+  ReductionData(unsigned Size) {
+Vars.reserve(Size);
+Privates.reserve(Size);
+LHSs.reserve(Size);
+RHSs.reserve(Size);
+ReductionOps.reserve(Size);
+ExprCaptures.reserve(Size);
+ExprPostUpdates.reserve(Size);
+  }
+  /// Stores reduction item and reduction operation only (required for 
dependent
+  /// reduction item).
+  void push(Expr *Item, Expr *ReductionOp) {
+Vars.emplace_back(Item);
+Privates.emplace_back(nullptr);
+LHSs.emplace_back(nullptr);
+RHSs.emplace_back(nullptr);
+ReductionOps.emplace_back(ReductionOp);
+  }
+  /// Stores reduction data.
+  void push(Expr *Item, Expr *Private, Expr *LHS, Expr *RHS,
+Expr *ReductionOp) {
+Vars.emplace_back(Item);
+Privates.emplace_back(Private);
+LHSs.emplace_back(LHS);
+RHSs.emplace_back(RHS);
+ReductionOps.emplace_back(ReductionOp);
+  }
+};
+} // namespace
+
+static bool ActOnOMPReductionKindClause(
+Sema &S, DSAStackTy *Stack, ArrayRef VarList,
+SourceLocation StartLoc, SourceLocation LParenLoc, SourceLocation ColonLoc,
+SourceLocation EndLoc, CXXScopeSpec &ReductionIdScopeSpec,
+const DeclarationNameInfo &ReductionId,
+ArrayRef UnresolvedReductions, ReductionData &RD) {
   auto DN = ReductionId.getName();
   auto OOK = DN.getCXXOverloadedOperator();
   BinaryOperatorKind BOK = BO_Comma;
 
+  ASTContext &Context = S.Context;
   // OpenMP [2.14.3.6, reduction clause]
   // C
   // reduction-identifier is either an identifier or one of the following
@@ -8995,13 +9046,6 @@ OMPClause *Sema::ActOnOpenMPReductionCla
 ReductionIdRange.setBegin(ReductionIdScopeSpec.getBeginLoc());
   ReductionIdRange.setEnd(ReductionId.getEndLoc());
 
-  SmallVector Vars;
-  SmallVector Privates;
-  SmallVector LHSs;
-  SmallVector RHSs;
-  SmallVector ReductionOps;
-  SmallVector ExprCaptures;
-  SmallVector ExprPostUpdates;
   auto IR = UnresolvedReductions.begin(), ER = UnresolvedReductions.end();
   bool FirstIter = true;
   for (auto RefExpr : VarList) {
@@ -9019,27 +9063,23 @@ OMPClause *Sema::ActOnOpenMPReductionCla
 SourceLocation ELoc;
 SourceRange ERange;
 Expr *SimpleRefExpr = RefExpr;
-auto Res = getPrivateItem(*this, SimpleRefExpr, ELoc, ERange,
+auto Res = getPrivateItem(S, SimpleRefExpr, ELoc, ERange,
   /*AllowArraySection=*/true);
 if (Res.second) {
-  // It will be analyzed later.
-  Vars.push_back(RefExpr);
-  Privates.push_back(nullptr);
-  LHSs.push_back(nullptr);
-  RHSs.push_back(nullptr);
   // Try to find 'declare reduction' corresponding construct before using
   // builtin/overloaded operators.
   QualType Type = Context.DependentTy;
   CXXCastPath BasePath;
   ExprResult DeclareReductionRef = buildDeclareReductionRef(
-  *this, ELoc, ERange, DSAStack->getCurScope(), ReductionIdScopeSpec,
+  S, ELoc, ERange, Stack->getCurScope(), ReductionIdScopeSpec,
   ReductionId, Type, BasePath, IR == ER ? nullptr : *IR);
-  if (CurContext->isDependentContext() &&
+  Expr *ReductionOp = nullptr;
+  if (S.CurContext->isDependentContext() &&
   (DeclareReductionRef.isUnset() ||
isa(DeclareReductionRef.get(
-ReductionOps.push_back(DeclareReductionRef.get());
-  else
-

[PATCH] D35519: Add SEMA checking to attribute 'target'

2017-07-18 Thread Erich Keane via Phabricator via cfe-commits

erichkeane marked 2 inline comments as done.
erichkeane added a comment.

Thanks Aaron!  Fixed those things and will be submitting soon.  Also ran format 
on the function in SemaDeclAttr. For some reason it didn't right the first time.


https://reviews.llvm.org/D35519



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

[PATCH] D30946: [ScopePrinting] Added support to print full scopes of types and declarations.

2017-07-18 Thread Simon Schroeder via Phabricator via cfe-commits

schroedersi added a comment.

Ping :)


https://reviews.llvm.org/D30946



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

[PATCH] D35329: [clang-reorder-fields] Enable reordering for plain C structs

2017-07-18 Thread Alexander Shaposhnikov via Phabricator via cfe-commits

alexshap added a comment.

ping


Repository:
  rL LLVM

https://reviews.llvm.org/D35329



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

[PATCH] D35519: Add SEMA checking to attribute 'target'

2017-07-18 Thread Eric Christopher via Phabricator via cfe-commits

echristo added a comment.

If you wouldn't mind I'd like to see this separated out a bit -
a) I think we can make the attribute info a struct rather than a pair as a 
simple change first
b) setCPU split
c) let's see where we are after that?

Also the description makes it sound like you're adding support for the feature 
rather than fixing the terrible error diagnostics - or at least to my early 
morning brain. :)

Also, one inline comment so far.

Thanks!

-eric




Comment at: test/Sema/attr-target.c:10
+int __attribute__((target("arch="))) turtle() { return 4; } // no warning, 
same as saying 'nothing'.
+int __attribute__((target("arch=hiss,arch=woof"))) pine_tree() { return 4; } 
//expected-warning {{ignoring duplicate 'arch=' in the target attribute string}}
+

Precedence issue maybe, but I'd have expected this to error on the architecture 
name rather than the duplicate?


https://reviews.llvm.org/D35519



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

[PATCH] D35519: Add SEMA checking to attribute 'target'

2017-07-18 Thread Erich Keane via Phabricator via cfe-commits

erichkeane added a comment.

In https://reviews.llvm.org/D35519#813082, @echristo wrote:

> If you wouldn't mind I'd like to see this separated out a bit -
>  a) I think we can make the attribute info a struct rather than a pair as a 
> simple change first
>  b) setCPU split
>  c) let's see where we are after that?
>
> Also the description makes it sound like you're adding support for the 
> feature rather than fixing the terrible error diagnostics - or at least to my 
> early morning brain. :)
>
> Also, one inline comment so far.
>
> Thanks!
>
> -eric


Caught me just in time :)

Sure, I can split this into a couple of patches.  I'll abandon this one and go 
from there.


https://reviews.llvm.org/D35519



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

[PATCH] D35519: Add SEMA checking to attribute 'target'

2017-07-18 Thread Erich Keane via Phabricator via cfe-commits

erichkeane abandoned this revision.
erichkeane added a comment.

Going to split this into a few patches as Eric requested.


https://reviews.llvm.org/D35519



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

[PATCH] D35449: [X86] Implement __builtin_cpu_is

2017-07-18 Thread Eric Christopher via Phabricator via cfe-commits

echristo added a subscriber: erichkeane.
echristo added a comment.

Adding Erich Keane here on this since he's working on something similar for the 
target attribute.

-eric


https://reviews.llvm.org/D35449



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

[PATCH] D35449: [X86] Implement __builtin_cpu_is

2017-07-18 Thread Eric Christopher via Phabricator via cfe-commits

echristo added a comment.

From my perspective here once you and Erich get some agreement on the checking 
between your two bits I'm fine :)

-eric


https://reviews.llvm.org/D35449



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

[PATCH] D35449: [X86] Implement __builtin_cpu_is

2017-07-18 Thread Eric Christopher via Phabricator via cfe-commits

echristo added a comment.

FWIW the duplication in CGCall.cpp of the enum set is painful if you can come 
up with anything else it'd be awesome. I don't have any good ideas, just a fond 
wish :)


https://reviews.llvm.org/D35449



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

[PATCH] D35548: [mips] Teach the driver to accept -m(no-)gpopt.

2017-07-18 Thread Simon Dardis via Phabricator via cfe-commits

sdardis updated this revision to Diff 107114.
sdardis marked an inline comment as done.
sdardis added a comment.

Addressed review comment, added warning when combining -mabicalls and -mgpopt.


https://reviews.llvm.org/D35548

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/mips-features.c
  test/Driver/mips-gpopt-warning.c

Index: test/Driver/mips-gpopt-warning.c
===
--- /dev/null
+++ test/Driver/mips-gpopt-warning.c
@@ -0,0 +1,6 @@
+// REQUIRES: mips-registered-target
+// RUN: %clang -### -c -target mips-mti-elf %s -mgpopt 2>&1 | FileCheck -check-prefix=IMPLICIT %s
+// IMPLICIT: warning: ignoring '-mgpopt' option as it cannot be used with the implicit usage of-mabicalls
+
+// RUN: %clang -### -c -target mips-mti-elf %s -mgpopt -mabicalls 2>&1 | FileCheck -check-prefix=EXPLICIT %s
+// EXPLICIT: warning: ignoring '-mgpopt' option as it cannot be used with -mabicalls
Index: test/Driver/mips-features.c
===
--- test/Driver/mips-features.c
+++ test/Driver/mips-features.c
@@ -10,6 +10,31 @@
 // RUN:   | FileCheck --check-prefix=CHECK-MNOABICALLS %s
 // CHECK-MNOABICALLS: "-target-feature" "+noabicalls"
 //
+// -mgpopt
+// RUN: %clang -target mips-linux-gnu -### -c %s -mno-gpopt -mgpopt 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MGPOPT-DEF-ABICALLS %s
+// CHECK-MGPOPT-DEF-ABICALLS-NOT: "-mllvm" "-mgpopt"
+//
+// -mabicalls -mgpopt
+// RUN: %clang -target mips-linux-gnu -### -c %s -mabicalls -mno-gpopt -mgpopt 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MGPOPT-EXPLICIT-ABICALLS %s
+// CHECK-MGPOPT-EXPLICIT-ABICALLS-NOT: "-mllvm" "-mgpopt"
+//
+// -mno-abicalls -mgpopt
+// RUN: %clang -target mips-linux-gnu -### -c %s -mno-abicalls -mno-gpopt -mgpopt 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MGPOPT %s
+// CHECK-MGPOPT: "-mllvm" "-mgpopt=1"
+//
+// -mno-abicalls -mno-gpopt
+// RUN: %clang -target mips-linux-gnu -### -c %s -mno-abicalls -mgpopt -mno-gpopt 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MNOGPOPT %s
+// CHECK-MNOGPOPT: "-mllvm" "-mgpopt=0"
+//
+// -mno-abicalls
+// RUN: %clang -target mips-linux-gnu -### -c %s -mno-abicalls 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MGPOPTDEF %s
+// CHECK-MGPOPTDEF: "-mllvm" "-mgpopt=1"
+//
 // -mips16
 // RUN: %clang -target mips-linux-gnu -### -c %s \
 // RUN: -mno-mips16 -mips16 2>&1 \
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -1462,6 +1462,32 @@
 A->claim();
   }
 
+  Arg *GPOpt = Args.getLastArg(options::OPT_mgpopt, options::OPT_mno_gpopt);
+  Arg *ABICalls = Args.getLastArg(options::OPT_mabicalls, options::OPT_mno_abicalls);
+
+  // -mabicalls is the default for many MIPS environments, even with -fno-pic.
+  // -mgpopt is the default for static, -fno-pic environments but these two
+  // options conflict. We want to be certain that -mno-abicalls -mgpopt is
+  // the only case where -mllvm -mgpopt is passed.
+  // NOTE: We need a warning here or in the backend to warn when -mgpopt is
+  //   passed explicitly when compiling something with -mabicalls
+  //   (implictly) in affect. Currently the warning is in the backend.
+  bool NoABICalls =
+  ABICalls && ABICalls->getOption().matches(options::OPT_mno_abicalls);
+  bool WantGPOpt = GPOpt && GPOpt->getOption().matches(options::OPT_mgpopt);
+  if (NoABICalls && (!GPOpt || WantGPOpt)) {
+  CmdArgs.push_back("-mllvm");
+  CmdArgs.push_back("-mgpopt=1");
+  } else {
+  CmdArgs.push_back("-mllvm");
+  CmdArgs.push_back("-mgpopt=0");
+  if ((!ABICalls || (!NoABICalls && ABICalls)) && (!GPOpt || WantGPOpt))
+D.Diag(diag::warn_drv_unsupported_gpopt) << (ABICalls ? 0 : 1);
+  }
+
+  if (GPOpt)
+GPOpt->claim();
+
   if (Arg *A = Args.getLastArg(options::OPT_mcompact_branches_EQ)) {
 StringRef Val = StringRef(A->getValue());
 if (mips::hasCompactBranches(CPUName)) {
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -2035,6 +2035,12 @@
   HelpText<"Use 64-bit floating point registers (MIPS only)">;
 def mfp32 : Flag<["-"], "mfp32">, Group,
   HelpText<"Use 32-bit floating point registers (MIPS only)">;
+def mgpopt : Flag<["-"], "mgpopt">, Group,
+  HelpText<"Use GP relative accesses for symbols known to be in a small"
+   " data section (MIPS)">;
+def mno_gpopt : Flag<["-"], "mno-gpopt">, Group,
+  HelpText<"Do not use GP relative accesses for symbols known to be in a small"
+   " data section (MIPS)">;
 def mnan_EQ : Joined<["-"], "mnan=">, Group;
 def mabicalls : Flag<["-"], "mabicalls">, Group,

[PATCH] D35012: [refactor] Add the AST source selection component

2017-07-18 Thread Alex Lorenz via Phabricator via cfe-commits

arphaman updated this revision to Diff 107121.
arphaman added a comment.

Factor out the lexical ordering code into a new visitor and simplify the 
implementation of the ast selection visitor


Repository:
  rL LLVM

https://reviews.llvm.org/D35012

Files:
  include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h
  include/clang/Basic/SourceLocation.h
  include/clang/Basic/SourceManager.h
  include/clang/Tooling/Refactoring/ASTSelection.h
  lib/Tooling/Refactoring/ASTSelection.cpp
  lib/Tooling/Refactoring/CMakeLists.txt
  unittests/Tooling/ASTSelectionTest.cpp
  unittests/Tooling/CMakeLists.txt
  unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp

Index: unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp
===
--- /dev/null
+++ unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp
@@ -0,0 +1,141 @@
+//===- unittest/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "TestVisitor.h"
+#include "clang/AST/LexicallyOrderedRecursiveASTVisitor.h"
+#include 
+
+using namespace clang;
+
+namespace {
+
+class DummyMatchVisitor;
+
+class LexicallyOrderedDeclVisitor
+: public LexicallyOrderedRecursiveASTVisitor {
+public:
+  LexicallyOrderedDeclVisitor(DummyMatchVisitor &Matcher,
+  const SourceManager &SM)
+  : LexicallyOrderedRecursiveASTVisitor(SM), Matcher(Matcher) {}
+
+  bool TraverseDecl(Decl *D) {
+TraversalStack.push_back(D);
+LexicallyOrderedRecursiveASTVisitor::TraverseDecl(D);
+TraversalStack.pop_back();
+return true;
+  }
+
+  bool VisitNamedDecl(const NamedDecl *D);
+
+private:
+  DummyMatchVisitor &Matcher;
+  llvm::SmallVector TraversalStack;
+};
+
+class DummyMatchVisitor : public ExpectedLocationVisitor {
+public:
+  bool VisitTranslationUnitDecl(TranslationUnitDecl *TU) {
+const ASTContext &Context = TU->getASTContext();
+const SourceManager &SM = Context.getSourceManager();
+LexicallyOrderedDeclVisitor SubVisitor(*this, SM);
+SubVisitor.TraverseDecl(TU);
+return false;
+  }
+
+  void match(StringRef Path, const Decl *D) { Match(Path, D->getLocStart()); }
+};
+
+bool LexicallyOrderedDeclVisitor::VisitNamedDecl(const NamedDecl *D) {
+  std::string Path;
+  llvm::raw_string_ostream OS(Path);
+  assert(TraversalStack.back() == D);
+  for (const Decl *D : TraversalStack) {
+if (isa(D)) {
+  OS << "/";
+  continue;
+}
+if (const auto *ND = dyn_cast(D))
+  OS << ND->getNameAsString();
+else
+  OS << "???";
+if (isa(D))
+  OS << "/";
+  }
+  Matcher.match(OS.str(), D);
+  return true;
+}
+
+TEST(LexicallyOrderedRecursiveASTVisitor, VisitDeclsInImplementation) {
+  StringRef Source = R"(
+@interface I
+@end
+@implementation I
+
+int nestedFunction() { }
+
+- (void) method{ }
+
+int anotherNestedFunction(int x) {
+  return x;
+}
+
+int innerVariable = 0;
+
+@end
+
+int outerVariable = 0;
+
+@implementation I(Cat)
+
+void catF() { }
+
+@end
+
+void outerFunction() { }
+)";
+  DummyMatchVisitor Visitor;
+  Visitor.DisallowMatch("/nestedFunction/", 6, 1);
+  Visitor.ExpectMatch("/I/nestedFunction/", 6, 1);
+  Visitor.ExpectMatch("/I/method/", 8, 1);
+  Visitor.DisallowMatch("/anotherNestedFunction/", 10, 1);
+  Visitor.ExpectMatch("/I/anotherNestedFunction/", 10, 1);
+  Visitor.DisallowMatch("/innerVariable", 14, 1);
+  Visitor.ExpectMatch("/I/innerVariable", 14, 1);
+  Visitor.ExpectMatch("/outerVariable", 18, 1);
+  Visitor.DisallowMatch("/catF/", 22, 1);
+  Visitor.ExpectMatch("/Cat/catF/", 22, 1);
+  Visitor.ExpectMatch("/outerFunction/", 26, 1);
+  EXPECT_TRUE(Visitor.runOver(Source, DummyMatchVisitor::Lang_OBJC));
+}
+
+TEST(LexicallyOrderedRecursiveASTVisitor, VisitMacroDeclsInImplementation) {
+  StringRef Source = R"(
+@interface I
+@end
+
+void outerFunction() { }
+
+#define MACRO_F(x) void nestedFunction##x() { }
+
+@implementation I
+
+MACRO_F(1)
+
+@end
+
+MACRO_F(2)
+)";
+  DummyMatchVisitor Visitor;
+  Visitor.ExpectMatch("/outerFunction/", 5, 1);
+  Visitor.ExpectMatch("/I/nestedFunction1/", 7, 20);
+  Visitor.ExpectMatch("/nestedFunction2/", 7, 20);
+  EXPECT_TRUE(Visitor.runOver(Source, DummyMatchVisitor::Lang_OBJC));
+}
+
+} // end anonymous namespace
Index: unittests/Tooling/CMakeLists.txt
===
--- unittests/Tooling/CMakeLists.txt
+++ unittests/Tooling/CMakeLists.txt
@@ -11,11 +11,13 @@
 endif()
 
 add_clang_unittest(ToolingTests
+  ASTSelectionTest.cpp
   CastExprTest.cpp
   CommentHandlerTest.cpp
   CompilationDatabaseTest.cpp
   DiagnosticsYamlTest.cpp
   FixItTest.cpp
+  LexicallyOrderedRecursiveASTVisitorTest.cpp
   LookupTest.cpp
   Qu

[PATCH] D35012: [refactor] Add the AST source selection component

2017-07-18 Thread Alex Lorenz via Phabricator via cfe-commits

arphaman added a comment.

Oops, I just realised that now there's a small bug with early exits. Since we 
don't actually have true lexical order for declarations in the @implementation 
we might exit early after visiting a method in the @implementation before a 
function that's actually written before that method. I will probably constraint 
early exits to avoid this case.


Repository:
  rL LLVM

https://reviews.llvm.org/D35012



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

r308327 - Add a warning for missing '#pragma pack (pop)' and suspicious uses

2017-07-18 Thread Alex Lorenz via cfe-commits

Author: arphaman
Date: Tue Jul 18 10:23:51 2017
New Revision: 308327

URL: http://llvm.org/viewvc/llvm-project?rev=308327&view=rev
Log:
Add a warning for missing '#pragma pack (pop)' and suspicious uses
of '#pragma pack' in included files

This commit adds a new -Wpragma-pack warning. It warns in the following cases:

- When a translation unit is missing terminating #pragma pack (pop) directives.
- When entering an included file if the current alignment value as determined
  by '#pragma pack' directives is different from the default alignment value.
- When leaving an included file that changed the state of the current alignment
  value.

rdar://10184173

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

Added:
cfe/trunk/test/PCH/suspicious-pragma-pack.c
cfe/trunk/test/Sema/Inputs/pragma-pack1.h
cfe/trunk/test/Sema/Inputs/pragma-pack2.h
cfe/trunk/test/Sema/suspicious-pragma-pack.c
cfe/trunk/test/SemaObjC/Inputs/empty.h
cfe/trunk/test/SemaObjC/suspicious-pragma-pack.m
Modified:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/include/clang/Serialization/ASTReader.h
cfe/trunk/lib/Parse/ParsePragma.cpp
cfe/trunk/lib/Sema/Sema.cpp
cfe/trunk/lib/Sema/SemaAttr.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp
cfe/trunk/test/OpenMP/declare_simd_messages.cpp
cfe/trunk/test/PCH/pragma-pack.c
cfe/trunk/test/Parser/pragma-options.c
cfe/trunk/test/Parser/pragma-options.cpp
cfe/trunk/test/Parser/pragma-pack.c
cfe/trunk/test/Sema/pragma-pack.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=308327&r1=308326&r2=308327&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Jul 18 10:23:51 2017
@@ -467,8 +467,9 @@ def IgnoredPragmaIntrinsic : DiagGroup<"
 def UnknownPragmas : DiagGroup<"unknown-pragmas">;
 def IgnoredPragmas : DiagGroup<"ignored-pragmas", [IgnoredPragmaIntrinsic]>;
 def PragmaClangAttribute : DiagGroup<"pragma-clang-attribute">;
+def PragmaPack : DiagGroup<"pragma-pack">;
 def Pragmas : DiagGroup<"pragmas", [UnknownPragmas, IgnoredPragmas,
-PragmaClangAttribute]>;
+PragmaClangAttribute, PragmaPack]>;
 def UnknownWarningOption : DiagGroup<"unknown-warning-option">;
 def NSobjectAttribute : DiagGroup<"NSObject-attribute">;
 def IndependentClassAttribute : DiagGroup<"IndependentClass-attribute">;

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=308327&r1=308326&r2=308327&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Jul 18 10:23:51 
2017
@@ -712,6 +712,16 @@ def err_pragma_options_align_mac68k_targ
 def warn_pragma_pack_invalid_alignment : Warning<
   "expected #pragma pack parameter to be '1', '2', '4', '8', or '16'">,
   InGroup;
+def warn_pragma_pack_non_default_at_include : Warning<
+  "non-default #pragma pack value might change the alignment of struct or "
+  "union members in the included file">, InGroup;
+def warn_pragma_pack_modified_after_include : Warning<
+  "the current #pragma pack aligment value is modified in the included "
+  "file">, InGroup;
+def warn_pragma_pack_no_pop_eof : Warning<"unterminated "
+  "'#pragma pack (push, ...)' at end of file">, InGroup;
+def note_pragma_pack_here : Note<
+  "previous '#pragma pack' directive that modifies alignment is here">;
 // Follow the Microsoft implementation.
 def warn_pragma_pack_show : Warning<"value of #pragma pack(show) == %0">;
 def warn_pragma_pack_pop_identifer_and_alignment : Warning<

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=308327&r1=308326&r2=308327&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Tue Jul 18 10:23:51 2017
@@ -208,6 +208,7 @@ namespace sema {
   class FunctionScopeInfo;
   class LambdaScopeInfo;
   class PossiblyUnreachableDiag;
+  class SemaPPCallbacks;
   class TemplateDeductionInfo;
 }
 
@@ -381,11 +382,12 @@ public:
   llvm::StringRef StackSlotLabel;
   ValueType Value;
   SourceLocation PragmaLocation;
-  Slot(llvm::StringRef StackSlotLabel,
-   ValueType Value,
-   SourceLocation PragmaLocation)
-: StackSlotLabel(StackSlotLabel

[PATCH] D35484: Add a warning for missing '#pragma pack (pop)' and suspicious '#pragma pack' uses when including files

2017-07-18 Thread Alex Lorenz via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL308327: Add a warning for missing '#pragma pack (pop)' and 
suspicious uses (authored by arphaman).

Changed prior to commit:
  https://reviews.llvm.org/D35484?vs=107090&id=107130#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D35484

Files:
  cfe/trunk/include/clang/Basic/DiagnosticGroups.td
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/include/clang/Sema/Sema.h
  cfe/trunk/include/clang/Serialization/ASTReader.h
  cfe/trunk/lib/Parse/ParsePragma.cpp
  cfe/trunk/lib/Sema/Sema.cpp
  cfe/trunk/lib/Sema/SemaAttr.cpp
  cfe/trunk/lib/Serialization/ASTReader.cpp
  cfe/trunk/lib/Serialization/ASTWriter.cpp
  cfe/trunk/test/OpenMP/declare_simd_messages.cpp
  cfe/trunk/test/PCH/pragma-pack.c
  cfe/trunk/test/PCH/suspicious-pragma-pack.c
  cfe/trunk/test/Parser/pragma-options.c
  cfe/trunk/test/Parser/pragma-options.cpp
  cfe/trunk/test/Parser/pragma-pack.c
  cfe/trunk/test/Sema/Inputs/pragma-pack1.h
  cfe/trunk/test/Sema/Inputs/pragma-pack2.h
  cfe/trunk/test/Sema/pragma-pack.c
  cfe/trunk/test/Sema/suspicious-pragma-pack.c
  cfe/trunk/test/SemaObjC/Inputs/empty.h
  cfe/trunk/test/SemaObjC/suspicious-pragma-pack.m

Index: cfe/trunk/include/clang/Sema/Sema.h
===
--- cfe/trunk/include/clang/Sema/Sema.h
+++ cfe/trunk/include/clang/Sema/Sema.h
@@ -208,6 +208,7 @@
   class FunctionScopeInfo;
   class LambdaScopeInfo;
   class PossiblyUnreachableDiag;
+  class SemaPPCallbacks;
   class TemplateDeductionInfo;
 }
 
@@ -381,11 +382,12 @@
   llvm::StringRef StackSlotLabel;
   ValueType Value;
   SourceLocation PragmaLocation;
-  Slot(llvm::StringRef StackSlotLabel,
-   ValueType Value,
-   SourceLocation PragmaLocation)
-: StackSlotLabel(StackSlotLabel), Value(Value),
-  PragmaLocation(PragmaLocation) {}
+  SourceLocation PragmaPushLocation;
+  Slot(llvm::StringRef StackSlotLabel, ValueType Value,
+   SourceLocation PragmaLocation, SourceLocation PragmaPushLocation)
+  : StackSlotLabel(StackSlotLabel), Value(Value),
+PragmaLocation(PragmaLocation),
+PragmaPushLocation(PragmaPushLocation) {}
 };
 void Act(SourceLocation PragmaLocation,
  PragmaMsStackAction Action,
@@ -416,6 +418,8 @@
 explicit PragmaStack(const ValueType &Default)
 : DefaultValue(Default), CurrentValue(Default) {}
 
+bool hasValue() const { return CurrentValue != DefaultValue; }
+
 SmallVector Stack;
 ValueType DefaultValue; // Value used for PSK_Reset action.
 ValueType CurrentValue;
@@ -437,6 +441,8 @@
   // Sentinel to represent when the stack is set to mac68k alignment.
   static const unsigned kMac68kAlignmentSentinel = ~0U;
   PragmaStack PackStack;
+  // The current #pragma pack values and locations at each #include.
+  SmallVector, 8> PackIncludeStack;
   // Segment #pragmas.
   PragmaStack DataSegStack;
   PragmaStack BSSSegStack;
@@ -8185,6 +8191,15 @@
   void ActOnPragmaPack(SourceLocation PragmaLoc, PragmaMsStackAction Action,
StringRef SlotLabel, Expr *Alignment);
 
+  enum class PragmaPackDiagnoseKind {
+NonDefaultStateAtInclude,
+ChangedStateAtExit
+  };
+
+  void DiagnoseNonDefaultPragmaPack(PragmaPackDiagnoseKind Kind,
+SourceLocation IncludeLoc);
+  void DiagnoseUnterminatedPragmaPack();
+
   /// ActOnPragmaMSStruct - Called on well formed \#pragma ms_struct [on|off].
   void ActOnPragmaMSStruct(PragmaMSStructKind Kind);
 
@@ -10378,6 +10393,12 @@
 
   IdentifierInfo *Ident_NSError = nullptr;
 
+  /// \brief The handler for the FileChanged preprocessor events.
+  ///
+  /// Used for diagnostics that implement custom semantic analysis for #include
+  /// directives, like -Wpragma-pack.
+  sema::SemaPPCallbacks *SemaPPCallbackHandler;
+
 protected:
   friend class Parser;
   friend class InitializationSequence;
Index: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td
@@ -467,8 +467,9 @@
 def UnknownPragmas : DiagGroup<"unknown-pragmas">;
 def IgnoredPragmas : DiagGroup<"ignored-pragmas", [IgnoredPragmaIntrinsic]>;
 def PragmaClangAttribute : DiagGroup<"pragma-clang-attribute">;
+def PragmaPack : DiagGroup<"pragma-pack">;
 def Pragmas : DiagGroup<"pragmas", [UnknownPragmas, IgnoredPragmas,
-PragmaClangAttribute]>;
+PragmaClangAttribute, PragmaPack]>;
 def UnknownWarningOption : DiagGroup<"unknown-warning-option">;
 def NSobjectAttribute : DiagGroup<"NSObject-attribute">;
 def IndependentClassAttribute : DiagGroup<"IndependentClass-attribute">;
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo

2017-07-18 Thread Jordan Rose via Phabricator via cfe-commits

jordan_rose added a comment.

As commented at https://reviews.llvm.org/D34955#798369, this isn't sufficient 
because it doesn't force a rebuild when new changes are merged in from 
upstream. It's not //harmful// to stick with the old version information, but 
if you can find something better that would be, um, better.


https://reviews.llvm.org/D35533



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

[PATCH] D34158: For standards compatibility, preinclude if the file is available

2017-07-18 Thread Melanie Blower via Phabricator via cfe-commits

mibintc added a comment.

OK folks, I was off the grid last week but I'm back now, and at my grindstone 
again.
I haven't received any comments since I updated the patch to remove the checks 
on "-nostdinc".  
Look OK to commit?
Many thanks for all your reviews
--Melanie


Repository:
  rL LLVM

https://reviews.llvm.org/D34158



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

r308328 - Revert r308327

2017-07-18 Thread Alex Lorenz via cfe-commits

Author: arphaman
Date: Tue Jul 18 10:36:42 2017
New Revision: 308328

URL: http://llvm.org/viewvc/llvm-project?rev=308328&view=rev
Log:
Revert r308327

I forgot to test clang-tools-extra which is now failing.

Removed:
cfe/trunk/test/PCH/suspicious-pragma-pack.c
cfe/trunk/test/Sema/Inputs/pragma-pack1.h
cfe/trunk/test/Sema/Inputs/pragma-pack2.h
cfe/trunk/test/Sema/suspicious-pragma-pack.c
cfe/trunk/test/SemaObjC/Inputs/empty.h
cfe/trunk/test/SemaObjC/suspicious-pragma-pack.m
Modified:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/include/clang/Serialization/ASTReader.h
cfe/trunk/lib/Parse/ParsePragma.cpp
cfe/trunk/lib/Sema/Sema.cpp
cfe/trunk/lib/Sema/SemaAttr.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp
cfe/trunk/test/OpenMP/declare_simd_messages.cpp
cfe/trunk/test/PCH/pragma-pack.c
cfe/trunk/test/Parser/pragma-options.c
cfe/trunk/test/Parser/pragma-options.cpp
cfe/trunk/test/Parser/pragma-pack.c
cfe/trunk/test/Sema/pragma-pack.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=308328&r1=308327&r2=308328&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Jul 18 10:36:42 2017
@@ -467,9 +467,8 @@ def IgnoredPragmaIntrinsic : DiagGroup<"
 def UnknownPragmas : DiagGroup<"unknown-pragmas">;
 def IgnoredPragmas : DiagGroup<"ignored-pragmas", [IgnoredPragmaIntrinsic]>;
 def PragmaClangAttribute : DiagGroup<"pragma-clang-attribute">;
-def PragmaPack : DiagGroup<"pragma-pack">;
 def Pragmas : DiagGroup<"pragmas", [UnknownPragmas, IgnoredPragmas,
-PragmaClangAttribute, PragmaPack]>;
+PragmaClangAttribute]>;
 def UnknownWarningOption : DiagGroup<"unknown-warning-option">;
 def NSobjectAttribute : DiagGroup<"NSObject-attribute">;
 def IndependentClassAttribute : DiagGroup<"IndependentClass-attribute">;

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=308328&r1=308327&r2=308328&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Jul 18 10:36:42 
2017
@@ -712,16 +712,6 @@ def err_pragma_options_align_mac68k_targ
 def warn_pragma_pack_invalid_alignment : Warning<
   "expected #pragma pack parameter to be '1', '2', '4', '8', or '16'">,
   InGroup;
-def warn_pragma_pack_non_default_at_include : Warning<
-  "non-default #pragma pack value might change the alignment of struct or "
-  "union members in the included file">, InGroup;
-def warn_pragma_pack_modified_after_include : Warning<
-  "the current #pragma pack aligment value is modified in the included "
-  "file">, InGroup;
-def warn_pragma_pack_no_pop_eof : Warning<"unterminated "
-  "'#pragma pack (push, ...)' at end of file">, InGroup;
-def note_pragma_pack_here : Note<
-  "previous '#pragma pack' directive that modifies alignment is here">;
 // Follow the Microsoft implementation.
 def warn_pragma_pack_show : Warning<"value of #pragma pack(show) == %0">;
 def warn_pragma_pack_pop_identifer_and_alignment : Warning<

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=308328&r1=308327&r2=308328&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Tue Jul 18 10:36:42 2017
@@ -208,7 +208,6 @@ namespace sema {
   class FunctionScopeInfo;
   class LambdaScopeInfo;
   class PossiblyUnreachableDiag;
-  class SemaPPCallbacks;
   class TemplateDeductionInfo;
 }
 
@@ -382,12 +381,11 @@ public:
   llvm::StringRef StackSlotLabel;
   ValueType Value;
   SourceLocation PragmaLocation;
-  SourceLocation PragmaPushLocation;
-  Slot(llvm::StringRef StackSlotLabel, ValueType Value,
-   SourceLocation PragmaLocation, SourceLocation PragmaPushLocation)
-  : StackSlotLabel(StackSlotLabel), Value(Value),
-PragmaLocation(PragmaLocation),
-PragmaPushLocation(PragmaPushLocation) {}
+  Slot(llvm::StringRef StackSlotLabel,
+   ValueType Value,
+   SourceLocation PragmaLocation)
+: StackSlotLabel(StackSlotLabel), Value(Value),
+  PragmaLocation(PragmaLocation) {}
 };
 void Act(SourceLocation PragmaLocation,
  PragmaMsStackAction Action,
@@ -418,8

[PATCH] D35572: Add isValidCPUName and isValidFeature to TargetInfo

2017-07-18 Thread Erich Keane via Phabricator via cfe-commits

erichkeane created this revision.

These two functions are really useful for implementations of attributes 
(including attribute-target), so add the functionality.


https://reviews.llvm.org/D35572

Files:
  include/clang/Basic/TargetInfo.h
  lib/Basic/Targets.cpp

Index: lib/Basic/Targets.cpp
===
--- lib/Basic/Targets.cpp
+++ lib/Basic/Targets.cpp
@@ -963,8 +963,8 @@
   //  401, 403, 405, 405fp, 440fp, 464, 464fp, 476, 476fp, 505, 740, 801,
   //  821, 823, 8540, 8548, e300c2, e300c3, e500mc64, e6500, 860, cell,
   //  titan, rs64.
-  bool setCPU(const std::string &Name) override {
-bool CPUKnown = llvm::StringSwitch(Name)
+  bool isValidCPUName(StringRef Name) const override {
+return llvm::StringSwitch(Name)
   .Case("generic", true)
   .Case("440", true)
   .Case("450", true)
@@ -1014,10 +1014,12 @@
   .Case("powerpc64le", true)
   .Case("ppc64le", true)
   .Default(false);
+  }
 
+  bool setCPU(const std::string &Name) override {
+bool CPUKnown = isValidCPUName(Name);
 if (CPUKnown)
   CPU = Name;
-
 return CPUKnown;
   }
 
@@ -2006,6 +2008,9 @@
 // FIXME: implement
 return TargetInfo::CharPtrBuiltinVaList;
   }
+  bool isValidCPUName(StringRef Name) const override {
+return StringToCudaArch(Name) != CudaArch::UNKNOWN;
+  }
   bool setCPU(const std::string &Name) override {
 GPU = StringToCudaArch(Name);
 return GPU != CudaArch::UNKNOWN;
@@ -2367,6 +2372,13 @@
   .Default(GK_NONE);
   }
 
+  bool isValidCPUName(StringRef Name) const override {
+if (getTriple().getArch() == llvm::Triple::amdgcn)
+  return GK_NONE !=  parseAMDGCNName(Name);
+else
+  return GK_NONE !=  parseR600Name(Name);
+  }
+
   bool setCPU(const std::string &Name) override {
 if (getTriple().getArch() == llvm::Triple::amdgcn)
   GPU = parseAMDGCNName(Name);
@@ -2873,6 +2885,87 @@
 //@}
   } CPU = CK_Generic;
 
+  bool checkCPUKind(CPUKind Kind) const {
+// Perform any per-CPU checks necessary to determine if this CPU is
+// acceptable.
+// FIXME: This results in terrible diagnostics. Clang just says the CPU is
+// invalid without explaining *why*.
+switch (Kind) {
+case CK_Generic:
+  // No processor selected!
+  return false;
+
+case CK_i386:
+case CK_i486:
+case CK_WinChipC6:
+case CK_WinChip2:
+case CK_C3:
+case CK_i586:
+case CK_Pentium:
+case CK_PentiumMMX:
+case CK_i686:
+case CK_PentiumPro:
+case CK_Pentium2:
+case CK_Pentium3:
+case CK_Pentium3M:
+case CK_PentiumM:
+case CK_Yonah:
+case CK_C3_2:
+case CK_Pentium4:
+case CK_Pentium4M:
+case CK_Lakemont:
+case CK_Prescott:
+case CK_K6:
+case CK_K6_2:
+case CK_K6_3:
+case CK_Athlon:
+case CK_AthlonThunderbird:
+case CK_Athlon4:
+case CK_AthlonXP:
+case CK_AthlonMP:
+case CK_Geode:
+  // Only accept certain architectures when compiling in 32-bit mode.
+  if (getTriple().getArch() != llvm::Triple::x86)
+return false;
+
+  // Fallthrough
+case CK_Nocona:
+case CK_Core2:
+case CK_Penryn:
+case CK_Bonnell:
+case CK_Silvermont:
+case CK_Goldmont:
+case CK_Nehalem:
+case CK_Westmere:
+case CK_SandyBridge:
+case CK_IvyBridge:
+case CK_Haswell:
+case CK_Broadwell:
+case CK_SkylakeClient:
+case CK_SkylakeServer:
+case CK_Cannonlake:
+case CK_KNL:
+case CK_Athlon64:
+case CK_Athlon64SSE3:
+case CK_AthlonFX:
+case CK_K8:
+case CK_K8SSE3:
+case CK_Opteron:
+case CK_OpteronSSE3:
+case CK_AMDFAM10:
+case CK_BTVER1:
+case CK_BTVER2:
+case CK_BDVER1:
+case CK_BDVER2:
+case CK_BDVER3:
+case CK_BDVER4:
+case CK_ZNVER1:
+case CK_x86_64:
+  return true;
+}
+llvm_unreachable("Unhandled CPU kind");
+  }
+
   CPUKind getCPUKind(StringRef CPU) const {
 return llvm::StringSwitch(CPU)
 .Case("i386", CK_i386)
@@ -3053,6 +3146,7 @@
   initFeatureMap(llvm::StringMap &Features, DiagnosticsEngine &Diags,
  StringRef CPU,
  const std::vector &FeaturesVec) const override;
+  bool isValidFeatureName(StringRef Name) const override;
   bool hasFeature(StringRef Feature) const override;
   bool handleTargetFeatures(std::vector &Features,
 DiagnosticsEngine &Diags) override;
@@ -3066,87 +3160,13 @@
   return "no-mmx";
 return "";
   }
-  bool setCPU(const std::string &Name) override {
-CPU = getCPUKind(Name);
-
-// Perform any per-CPU checks necessary to determine if this CPU is
-// acceptable.
-// FIXME: This results in terrible diagnostics. Clang just says the CPU is
-// invalid without explaining *why*.
-switch (CPU) {
-case CK_Generic:
-  // No processor selected!
-  return false;
 
-case CK_i386:
-case CK_i486:
-case CK_Wi

[PATCH] D35574: Convert attribute 'target' parsing from a 'pair' to a 'struct' to make further improvements easier

2017-07-18 Thread Erich Keane via Phabricator via cfe-commits

erichkeane created this revision.

The attribute 'target' parse function previously returned a pair.  Convert this 
to a 'pair' in order to add more functionality, and improve usability.


https://reviews.llvm.org/D35574

Files:
  include/clang/Basic/Attr.td
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CodeGenModule.cpp


Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -4499,18 +4499,19 @@
 
 // Make a copy of the features as passed on the command line into the
 // beginning of the additional features from the function to override.
-ParsedAttr.first.insert(ParsedAttr.first.begin(),
+ParsedAttr.Features.insert(ParsedAttr.Features.begin(),
 Target.getTargetOpts().FeaturesAsWritten.begin(),
 Target.getTargetOpts().FeaturesAsWritten.end());
 
-if (ParsedAttr.second != "")
-  TargetCPU = ParsedAttr.second;
+if (ParsedAttr.Architecture != "")
+  TargetCPU = ParsedAttr.Architecture ;
 
 // Now populate the feature map, first with the TargetCPU which is either
 // the default or a new one from the target attribute string. Then we'll 
use
 // the passed in features (FeaturesAsWritten) along with the new ones from
 // the attribute.
-Target.initFeatureMap(FeatureMap, getDiags(), TargetCPU, ParsedAttr.first);
+Target.initFeatureMap(FeatureMap, getDiags(), TargetCPU,
+  ParsedAttr.Features);
   } else {
 Target.initFeatureMap(FeatureMap, getDiags(), TargetCPU,
   Target.getTargetOpts().Features);
Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -1877,8 +1877,8 @@
   // the function.
   const auto *TD = FD->getAttr();
   TargetAttr::ParsedTargetAttr ParsedAttr = TD->parse();
-  if (ParsedAttr.second != "")
-TargetCPU = ParsedAttr.second;
+  if (ParsedAttr.Architecture != "")
+TargetCPU = ParsedAttr.Architecture;
   if (TargetCPU != "")
 FuncAttrs.addAttribute("target-cpu", TargetCPU);
   if (!Features.empty()) {
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -1802,11 +1802,18 @@
   let Subjects = SubjectList<[Function], ErrorDiag>;
   let Documentation = [TargetDocs];
   let AdditionalMembers = [{
-typedef std::pair, StringRef> ParsedTargetAttr;
+struct ParsedTargetAttr {
+  std::vector Features;
+  StringRef Architecture;
+  bool DuplicateArchitecture = false;
+};
 ParsedTargetAttr parse() const {
+  return parse(getFeaturesStr());
+}
+static ParsedTargetAttr parse(StringRef Features) {
   ParsedTargetAttr Ret;
   SmallVector AttrFeatures;
-  getFeaturesStr().split(AttrFeatures, ",");
+  Features.split(AttrFeatures, ",");
 
   // Grab the various features and prepend a "+" to turn on the feature to
   // the backend and add them to our existing set of features.
@@ -1823,12 +1830,15 @@
  continue;
 
 // While we're here iterating check for a different target cpu.
-if (Feature.startswith("arch="))
-  Ret.second = Feature.split("=").second.trim();
-else if (Feature.startswith("no-"))
-  Ret.first.push_back("-" + Feature.split("-").second.str());
+if (Feature.startswith("arch=")) {
+  if (!Ret.Architecture.empty())
+Ret.DuplicateArchitecture = true;
+  else
+Ret.Architecture = Feature.split("=").second.trim();
+} else if (Feature.startswith("no-"))
+  Ret.Features.push_back("-" + Feature.split("-").second.str());
 else
-  Ret.first.push_back("+" + Feature.str());
+  Ret.Features.push_back("+" + Feature.str());
   }
   return Ret;
 }


Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -4499,18 +4499,19 @@
 
 // Make a copy of the features as passed on the command line into the
 // beginning of the additional features from the function to override.
-ParsedAttr.first.insert(ParsedAttr.first.begin(),
+ParsedAttr.Features.insert(ParsedAttr.Features.begin(),
 Target.getTargetOpts().FeaturesAsWritten.begin(),
 Target.getTargetOpts().FeaturesAsWritten.end());
 
-if (ParsedAttr.second != "")
-  TargetCPU = ParsedAttr.second;
+if (ParsedAttr.Architecture != "")
+  TargetCPU = ParsedAttr.Architecture ;
 
 // Now populate the feature map, first with the TargetCPU which is either
 // the default or a new one from the target attribute string. Then we'll

[PATCH] D35573: Improve SEMA for attribute-target

2017-07-18 Thread Erich Keane via Phabricator via cfe-commits

erichkeane created this revision.

Add more diagnosis for the non-multiversioning case.


https://reviews.llvm.org/D35573

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/attr-target.c

Index: test/Sema/attr-target.c
===
--- test/Sema/attr-target.c
+++ test/Sema/attr-target.c
@@ -2,7 +2,13 @@
 
 int __attribute__((target("avx,sse4.2,arch=ivybridge"))) foo() { return 4; }
 int __attribute__((target())) bar() { return 4; } //expected-error {{'target' attribute takes one argument}}
-int __attribute__((target("tune=sandybridge"))) baz() { return 4; } //expected-warning {{Ignoring unsupported 'tune=' in the target attribute string}}
-int __attribute__((target("fpmath=387"))) walrus() { return 4; } //expected-warning {{Ignoring unsupported 'fpmath=' in the target attribute string}}
+int __attribute__((target("tune=sandybridge"))) baz() { return 4; } //expected-warning {{ignoring unsupported 'tune=' in the target attribute string}}
+int __attribute__((target("fpmath=387"))) walrus() { return 4; } //expected-warning {{ignoring unsupported 'fpmath=' in the target attribute string}}
+int __attribute__((target("avx,sse4.2,arch=hiss"))) meow() {  return 4; }//expected-warning {{ignoring unsupported architecture 'hiss' in the target attribute string}}
+int __attribute__((target("woof"))) bark() {  return 4; }//expected-warning {{ignoring unsupported 'woof' in the target attribute string}}
+int __attribute__((target("arch="))) turtle() { return 4; } // no warning, same as saying 'nothing'.
+int __attribute__((target("arch=hiss,arch=woof"))) pine_tree() { return 4; } //expected-warning {{ignoring unsupported architecture 'hiss' in the target attribute string}}
+int __attribute__((target("arch=ivybridge,arch=haswell"))) oak_tree() { return 4; } //expected-warning {{ignoring duplicate 'arch=' in the target attribute string}}
+
 
 
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -2993,22 +2993,43 @@
 D->addAttr(NewAttr);
 }
 
-// Check for things we'd like to warn about, no errors or validation for now.
-// TODO: Validation should use a backend target library that specifies
-// the allowable subtarget features and cpus. We could use something like a
-// TargetCodeGenInfo hook here to do validation.
-void Sema::checkTargetAttr(SourceLocation LiteralLoc, StringRef AttrStr) {
+// Check for things we'd like to warn about. Multiversioning issues are
+// handled later in the process, once we know how many exist.
+bool Sema::checkTargetAttr(SourceLocation LiteralLoc, StringRef AttrStr) {
+  enum FirstParam { Unsupported, Duplicate };
+  enum SecondParam { None, Architecture };
   for (auto Str : {"tune=", "fpmath="})
 if (AttrStr.find(Str) != StringRef::npos)
-  Diag(LiteralLoc, diag::warn_unsupported_target_attribute) << Str;
+  return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
+ << Unsupported << None << Str;
+
+  TargetAttr::ParsedTargetAttr ParsedAttrs = TargetAttr::parse(AttrStr);
+
+  if (!ParsedAttrs.Architecture.empty() &&
+  !Context.getTargetInfo().isValidCPUName(ParsedAttrs.Architecture))
+return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
+   << Unsupported << Architecture << ParsedAttrs.Architecture;
+
+  if (ParsedAttrs.DuplicateArchitecture)
+return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
+   << Duplicate << None << "arch=";
+
+  for (const auto &Feature : ParsedAttrs.Features) {
+auto CurFeature = StringRef(Feature).drop_front(); // remove + or -.
+if (!Context.getTargetInfo().isValidFeatureName(CurFeature))
+  return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
+ << Unsupported << None << CurFeature;
+  }
+
+  return true;
 }
 
 static void handleTargetAttr(Sema &S, Decl *D, const AttributeList &Attr) {
   StringRef Str;
   SourceLocation LiteralLoc;
-  if (!S.checkStringLiteralArgumentAttr(Attr, 0, Str, &LiteralLoc))
+  if (!S.checkStringLiteralArgumentAttr(Attr, 0, Str, &LiteralLoc) ||
+  !S.checkTargetAttr(LiteralLoc, Str))
 return;
-  S.checkTargetAttr(LiteralLoc, Str);
   unsigned Index = Attr.getAttributeSpellingListIndex();
   TargetAttr *NewAttr =
   ::new (S.Context) TargetAttr(Attr.getRange(), S.Context, Str, Index);
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -3271,7 +3271,7 @@
   unsigned ArgNum, StringRef &Str,
   SourceLocation *ArgLocation = nullptr);
   bool checkSectionName(SourceLocation LiteralLoc, StringRef Str);
-  void checkTargetAttr(SourceLocation LiteralLoc, StringRef Str);
+  bool checkTargetAttr(SourceL

[PATCH] D35012: [refactor] Add the AST source selection component

2017-07-18 Thread Alex Lorenz via Phabricator via cfe-commits

arphaman updated this revision to Diff 107138.
arphaman added a comment.

rm early exit bug


Repository:
  rL LLVM

https://reviews.llvm.org/D35012

Files:
  include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h
  include/clang/Basic/SourceLocation.h
  include/clang/Basic/SourceManager.h
  include/clang/Tooling/Refactoring/ASTSelection.h
  lib/Tooling/Refactoring/ASTSelection.cpp
  lib/Tooling/Refactoring/CMakeLists.txt
  unittests/Tooling/ASTSelectionTest.cpp
  unittests/Tooling/CMakeLists.txt
  unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp

Index: unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp
===
--- /dev/null
+++ unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp
@@ -0,0 +1,141 @@
+//===- unittest/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "TestVisitor.h"
+#include "clang/AST/LexicallyOrderedRecursiveASTVisitor.h"
+#include 
+
+using namespace clang;
+
+namespace {
+
+class DummyMatchVisitor;
+
+class LexicallyOrderedDeclVisitor
+: public LexicallyOrderedRecursiveASTVisitor {
+public:
+  LexicallyOrderedDeclVisitor(DummyMatchVisitor &Matcher,
+  const SourceManager &SM)
+  : LexicallyOrderedRecursiveASTVisitor(SM), Matcher(Matcher) {}
+
+  bool TraverseDecl(Decl *D) {
+TraversalStack.push_back(D);
+LexicallyOrderedRecursiveASTVisitor::TraverseDecl(D);
+TraversalStack.pop_back();
+return true;
+  }
+
+  bool VisitNamedDecl(const NamedDecl *D);
+
+private:
+  DummyMatchVisitor &Matcher;
+  llvm::SmallVector TraversalStack;
+};
+
+class DummyMatchVisitor : public ExpectedLocationVisitor {
+public:
+  bool VisitTranslationUnitDecl(TranslationUnitDecl *TU) {
+const ASTContext &Context = TU->getASTContext();
+const SourceManager &SM = Context.getSourceManager();
+LexicallyOrderedDeclVisitor SubVisitor(*this, SM);
+SubVisitor.TraverseDecl(TU);
+return false;
+  }
+
+  void match(StringRef Path, const Decl *D) { Match(Path, D->getLocStart()); }
+};
+
+bool LexicallyOrderedDeclVisitor::VisitNamedDecl(const NamedDecl *D) {
+  std::string Path;
+  llvm::raw_string_ostream OS(Path);
+  assert(TraversalStack.back() == D);
+  for (const Decl *D : TraversalStack) {
+if (isa(D)) {
+  OS << "/";
+  continue;
+}
+if (const auto *ND = dyn_cast(D))
+  OS << ND->getNameAsString();
+else
+  OS << "???";
+if (isa(D))
+  OS << "/";
+  }
+  Matcher.match(OS.str(), D);
+  return true;
+}
+
+TEST(LexicallyOrderedRecursiveASTVisitor, VisitDeclsInImplementation) {
+  StringRef Source = R"(
+@interface I
+@end
+@implementation I
+
+int nestedFunction() { }
+
+- (void) method{ }
+
+int anotherNestedFunction(int x) {
+  return x;
+}
+
+int innerVariable = 0;
+
+@end
+
+int outerVariable = 0;
+
+@implementation I(Cat)
+
+void catF() { }
+
+@end
+
+void outerFunction() { }
+)";
+  DummyMatchVisitor Visitor;
+  Visitor.DisallowMatch("/nestedFunction/", 6, 1);
+  Visitor.ExpectMatch("/I/nestedFunction/", 6, 1);
+  Visitor.ExpectMatch("/I/method/", 8, 1);
+  Visitor.DisallowMatch("/anotherNestedFunction/", 10, 1);
+  Visitor.ExpectMatch("/I/anotherNestedFunction/", 10, 1);
+  Visitor.DisallowMatch("/innerVariable", 14, 1);
+  Visitor.ExpectMatch("/I/innerVariable", 14, 1);
+  Visitor.ExpectMatch("/outerVariable", 18, 1);
+  Visitor.DisallowMatch("/catF/", 22, 1);
+  Visitor.ExpectMatch("/Cat/catF/", 22, 1);
+  Visitor.ExpectMatch("/outerFunction/", 26, 1);
+  EXPECT_TRUE(Visitor.runOver(Source, DummyMatchVisitor::Lang_OBJC));
+}
+
+TEST(LexicallyOrderedRecursiveASTVisitor, VisitMacroDeclsInImplementation) {
+  StringRef Source = R"(
+@interface I
+@end
+
+void outerFunction() { }
+
+#define MACRO_F(x) void nestedFunction##x() { }
+
+@implementation I
+
+MACRO_F(1)
+
+@end
+
+MACRO_F(2)
+)";
+  DummyMatchVisitor Visitor;
+  Visitor.ExpectMatch("/outerFunction/", 5, 1);
+  Visitor.ExpectMatch("/I/nestedFunction1/", 7, 20);
+  Visitor.ExpectMatch("/nestedFunction2/", 7, 20);
+  EXPECT_TRUE(Visitor.runOver(Source, DummyMatchVisitor::Lang_OBJC));
+}
+
+} // end anonymous namespace
Index: unittests/Tooling/CMakeLists.txt
===
--- unittests/Tooling/CMakeLists.txt
+++ unittests/Tooling/CMakeLists.txt
@@ -11,11 +11,13 @@
 endif()
 
 add_clang_unittest(ToolingTests
+  ASTSelectionTest.cpp
   CastExprTest.cpp
   CommentHandlerTest.cpp
   CompilationDatabaseTest.cpp
   DiagnosticsYamlTest.cpp
   FixItTest.cpp
+  LexicallyOrderedRecursiveASTVisitorTest.cpp
   LookupTest.cpp
   QualTypeNamesTest.cpp
   RecursiveASTVisitorTest.cpp
Index: unittests/Tooling/ASTSelectionTest.cpp
===

[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo

2017-07-18 Thread don hinton via Phabricator via cfe-commits

hintonda added inline comments.



Comment at: lib/Basic/CMakeLists.txt:17
 
 macro(find_first_existing_vc_file out_var path)
   set(git_path "${path}/.git")

LLVM already has a version of find_first_existing_vc_file in 
llvm/include/llvm/Support/CMakelists.txt.

Would it make sense move it to an llvm module and reuse it in clang?


https://reviews.llvm.org/D35533



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

1 2 >

1 - 100 of 176 matches

Mail list logo