[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude

[Melanie Blower via Phabricator via cfe-commits]

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

I'm planning to rework this patch again. sorry for the churn.


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

RE: D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude

[Blower, Melanie via cfe-commits]

 
Hahnfeld added a subscriber: cfe-commits.
Hahnfeld edited reviewers, added: rsmith, rengolin; removed: cfe-commits.
Hahnfeld added a comment.

Some comments inline. In general you should consider posting an RFC on cfe-dev 
because this change will basically affect all compilations on GNU/Linux if the 
file is present.
Adding Richard (general maintainer) and Renato (ARM Linux) so they are aware.




Comment at: include/clang/Driver/ToolChain.h:459-462
+  /// \brief Add arguments to use system-specific GNU includes.
+  virtual void AddGnuIncludeArgs(const llvm::opt::ArgList ,
+  llvm::opt::ArgStringList ) 
+ const;
+

Why do you need to define this method in the generic `ToolChain`?



Comment at: lib/Driver/ToolChains/Gnu.cpp:2332-2349
+void Generic_GCC::addGnuIncludeArgs(const ArgList , 
+ArgStringList ) const {
+  const Generic_GCC::GCCVersion  = 
+GCCInstallation.getVersion();
+  if (!DriverArgs.hasArg(options::OPT_ffreestanding) &&
+  !DriverArgs.hasArg(clang::driver::options::OPT_nostdinc) &&
+  !Version.isOlderThan(4, 8, 0)) {
+// If stdc-predef.h exists in the sytem includes, then -include it.

If this is GNU/Linux only, why not move to the `Linux` toolchain entirely?



Comment at: lib/Driver/ToolChains/Gnu.h:328-330
+  void addGnuIncludeArgs(const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const;
+

This starts with a lower-case character and won't override the method in 
`ToolChain`. Adding the keyword `override` should reveal this mistake.



Comment at: test/Driver/gcc-predef.c:1
+// Test that gcc preincludes stdc-predef.h on Linux //

s/gcc/clang/ ?



Comment at: test/Driver/gcc-predef.c:9
+  #else
+#if !defined(  _STDC_PREDEF_H )
+  #error "stdc-predef.h should be preincluded for GNU/Linux 4.8 and higher"

The driver will only include `stdc-predef.h` if it can be found in the system. 
With that, the current version of this test will only run on such Linux system. 
Maybe add a basic tree in `test/Driver/Inputs` and test that the corresponding 
header is included?
>>>OK I understand what you mean now.


Comment at: test/Driver/gcc-predef.c:14-17
+// Test that gcc preincludes stdc-predef.h on Linux // // RUN: %clang 
+-c --target=i386-unknown-linux %s -Xclang -verify // 
+expected-no-diagnostics

What's the difference to the test above?


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

[PATCH] D34574: [Sema] Disable c++17 aligned new and delete operators if not implemented in the deployment target's c++ standard library

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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

Thanks, this looks good to me. Now if we could only figure out when aligned 
allocation is unavailable on other platforms this easily :)




Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6413
+  "if you supply your own aligned allocation functions, use "
+  "-Wno-aligned-allocation-unavailable to silence error/warning">;
 

How about "this diagnostic" instead of "error/warning"?



Comment at: include/clang/Driver/CC1Options.td:571
   
+def aligned_alloc_unavailable : Flag<["-"], "aligned-alloc-unavailable">,
+  HelpText<"Aligned allocation/deallocation functions are unavailable">;

`-faligned-alloc-unavailable` would be more consistent with other flags.


https://reviews.llvm.org/D34574



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

[libcxx] r306632 - [libc++] Hoist explicit instantiation above implicit. NFC

[Shoaib Meenai via cfe-commits]

Author: smeenai
Date: Wed Jun 28 19:52:46 2017
New Revision: 306632

URL: http://llvm.org/viewvc/llvm-project?rev=306632=rev
Log:
[libc++] Hoist explicit instantiation above implicit. NFC

The string literal operators have implicit instantiations of
basic_string and basic_string, which prevent the dllimport
on the subsequent explicit instantiation declarations from having an
effect. Hoisting the explicit instantiations above the implicit ones
fixes the issue.

I think it's pretty unfortunate that the ordering has such an effect,
and I'd fixed the same issue for dllexport in r288682. dllimport is more
complicated from a codegen perspective, however, and clang's behavior of
ignoring the dllimport when there's a previous implicit instantiation is
consistent with cl, so changing the order is our only recourse.

Modified:
libcxx/trunk/include/string

Modified: libcxx/trunk/include/string
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/string?rev=306632=306631=306632=diff
==
--- libcxx/trunk/include/string (original)
+++ libcxx/trunk/include/string Wed Jun 28 19:52:46 2017
@@ -4004,6 +4004,10 @@ basic_string<_CharT, _Traits, _Allocator
 
 #endif  // _LIBCPP_DEBUG_LEVEL >= 2
 
+_LIBCPP_EXTERN_TEMPLATE(class _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS 
basic_string)
+_LIBCPP_EXTERN_TEMPLATE(class _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS 
basic_string)
+_LIBCPP_EXTERN_TEMPLATE(string operator+(char const*, string const&))
+
 #if _LIBCPP_STD_VER > 11 
 // Literal suffixes for basic_string [basic.string.literals]
 inline namespace literals
@@ -4037,10 +4041,6 @@ inline namespace literals
 }
 #endif
 
-_LIBCPP_EXTERN_TEMPLATE(class _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS 
basic_string)
-_LIBCPP_EXTERN_TEMPLATE(class _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS 
basic_string)
-_LIBCPP_EXTERN_TEMPLATE(string operator+(char const*, string const&))
-
 _LIBCPP_END_NAMESPACE_STD
 
 _LIBCPP_POP_MACROS


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

[PATCH] D34574: [Sema] Disable c++17 aligned new and delete operators if not implemented in the deployment target's c++ standard library

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

I'll revert the changes I made to libc++ that annotates the allocation 
functions with availability later.


https://reviews.llvm.org/D34574



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

[PATCH] D34574: [Sema] Disable c++17 aligned new and delete operators if not implemented in the deployment target's c++ standard library

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 104579.
ahatanak added a comment.

The updated patch produces diagnostics if an aligned allocation function is 
selected that is not implemented in the c++ standard library of the deployment 
target (except when the function is defined by the user and its definition is 
available).

A couple of points I'd like to address about the approach I took:

- The only way to silence the warning is to use 
-Wno-aligned-allocation-unavailable, as is mentioned in the note. I initially 
considered using -faligned-allocation for that purpose, but I realized it would 
silence errors/warnings when using "-std=c++14 -faligned-allocation", which is 
not desirable unless the selected aligned functions are defined by the user.

- isReplaceableGlobalAllocationFunction takes a pointer to a bool, but it's 
also possible to have it return a pair of bools.


https://reviews.llvm.org/D34574

Files:
  include/clang/AST/Decl.h
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/LangOptions.def
  include/clang/Driver/CC1Options.td
  lib/AST/Decl.cpp
  lib/Driver/ToolChains/Darwin.cpp
  lib/Driver/ToolChains/Darwin.h
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaExprCXX.cpp
  test/Driver/unavailable_aligned_allocation.cpp
  test/SemaCXX/unavailable_aligned_allocation.cpp

Index: test/SemaCXX/unavailable_aligned_allocation.cpp
===
--- /dev/null
+++ test/SemaCXX/unavailable_aligned_allocation.cpp
@@ -0,0 +1,109 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -fexceptions -aligned-alloc-unavailable -std=c++1z -verify %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -fexceptions -std=c++1z -verify -DNO_ERRORS %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -fexceptions -faligned-allocation -aligned-alloc-unavailable -std=c++14 -verify %s
+
+namespace std {
+  typedef decltype(sizeof(0)) size_t;
+  enum class align_val_t : std::size_t {};
+  struct nothrow_t {};
+  nothrow_t nothrow;
+}
+
+void *operator new(std::size_t __sz, const std::nothrow_t&) noexcept;
+void *operator new[](std::size_t __sz, const std::nothrow_t&) noexcept;
+
+void *operator new(std::size_t __sz, std::align_val_t, const std::nothrow_t&) noexcept;
+void *operator new[](std::size_t __sz, std::align_val_t, const std::nothrow_t&) noexcept;
+void operator delete(void *, std::align_val_t, const std::nothrow_t&);
+void operator delete[](void *, std::align_val_t, const std::nothrow_t&);
+void operator delete(void*, std::size_t, std::align_val_t) noexcept;
+void operator delete[](void*, std::size_t, std::align_val_t) noexcept;
+
+void *operator new(std::size_t, std::align_val_t, long long);
+
+struct alignas(256) OveralignedS {
+  int x[16];
+};
+
+struct S {
+  int x[16];
+};
+
+void test() {
+  auto *p = new S;
+  delete p;
+  p = new (std::nothrow) S;
+
+  auto *pa = new S[4];
+  delete[] pa;
+  pa = new (std::nothrow) S[4];
+}
+
+void testOveraligned() {
+  auto *p = new OveralignedS;
+  p = new ((std::align_val_t)8) OveralignedS;
+  delete p;
+  p = new (std::nothrow) OveralignedS;
+
+  auto *pa = new OveralignedS[4];
+  pa = new ((std::align_val_t)8) OveralignedS[4];
+  delete[] pa;
+  pa = new (std::nothrow) OveralignedS[4];
+  // No error here since it is not calling a replaceable allocation function.
+  p = new ((std::align_val_t)8, 10LL) OveralignedS;
+}
+
+#ifdef NO_ERRORS
+// expected-no-diagnostics
+#else
+// expected-error@-16 {{aligned allocation function of type 'void *(unsigned long, enum std::align_val_t)' possibly unavailable on}}
+// expected-note@-17 {{if you supply your own aligned allocation functions}}
+// expected-error@-18 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' possibly unavailable on}}
+// expected-note@-19 {{if you supply your own aligned allocation functions}}
+
+// expected-error@-20 {{aligned allocation function of type 'void *(unsigned long, enum std::align_val_t)' possibly unavailable on}}
+// expected-note@-21 {{if you supply your own aligned allocation functions}}
+// expected-error@-22 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' possibly unavailable on}}
+// expected-note@-23 {{if you supply your own aligned allocation functions}}
+
+// expected-error@-24 {{aligned deallocation function of type 'void (void *, enum std::align_val_t) noexcept' possibly unavailable on}}
+// expected-note@-25 {{if you supply your own aligned allocation functions}}
+
+// expected-error@-26 {{aligned allocation function of type 'void *(std::size_t, std::align_val_t, const std::nothrow_t &) noexcept' possibly unavailable on}}
+// expected-note@-27 {{if you supply your own aligned allocation functions}}
+// expected-error@-28 {{aligned deallocation function of type 'void (void *, std::align_val_t, const std::nothrow_t &) noexcept' possibly unavailable on}}
+// expected-note@-29 

r306628 - Track the set of module maps read while building a .pcm file and reload those when preprocessing from that .pcm file.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Wed Jun 28 19:19:42 2017
New Revision: 306628

URL: http://llvm.org/viewvc/llvm-project?rev=306628=rev
Log:
Track the set of module maps read while building a .pcm file and reload those 
when preprocessing from that .pcm file.

Added:
cfe/trunk/test/Modules/Inputs/preprocess-decluse/
cfe/trunk/test/Modules/Inputs/preprocess-decluse/a.h
cfe/trunk/test/Modules/Inputs/preprocess-decluse/a.modulemap
cfe/trunk/test/Modules/Inputs/preprocess-decluse/b.h
cfe/trunk/test/Modules/Inputs/preprocess-decluse/b.modulemap
cfe/trunk/test/Modules/Inputs/preprocess-decluse/main.modulemap
cfe/trunk/test/Modules/preprocess-decluse.cpp
Modified:
cfe/trunk/include/clang/Basic/SourceManager.h
cfe/trunk/include/clang/Lex/HeaderSearch.h
cfe/trunk/include/clang/Serialization/ASTReader.h
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/lib/Frontend/DependencyFile.cpp
cfe/trunk/lib/Frontend/FrontendAction.cpp
cfe/trunk/lib/Lex/ModuleMap.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp

Modified: cfe/trunk/include/clang/Basic/SourceManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/SourceManager.h?rev=306628=306627=306628=diff
==
--- cfe/trunk/include/clang/Basic/SourceManager.h (original)
+++ cfe/trunk/include/clang/Basic/SourceManager.h Wed Jun 28 19:19:42 2017
@@ -80,9 +80,19 @@ namespace SrcMgr {
   /// system_header is seen or in various other cases.
   ///
   enum CharacteristicKind {
-C_User, C_System, C_ExternCSystem
+C_User, C_System, C_ExternCSystem, C_User_ModuleMap, C_System_ModuleMap
   };
 
+  /// Determine whether a file / directory characteristic is for system code.
+  inline bool isSystem(CharacteristicKind CK) {
+return CK != C_User && CK != C_User_ModuleMap;
+  }
+
+  /// Determine whether a file characteristic is for a module map.
+  inline bool isModuleMap(CharacteristicKind CK) {
+return CK == C_User_ModuleMap || CK == C_System_ModuleMap;
+  }
+
   /// \brief One instance of this struct is kept for every file loaded or used.
   ///
   /// This object owns the MemoryBuffer object.
@@ -251,12 +261,14 @@ namespace SrcMgr {
 /// preprocessing of this \#include, including this SLocEntry.
 ///
 /// Zero means the preprocessor didn't provide such info for this 
SLocEntry.
-unsigned NumCreatedFIDs;
+unsigned NumCreatedFIDs : 31;
+
+/// \brief Whether this FileInfo has any \#line directives.
+unsigned HasLineDirectives : 1;
 
-/// \brief Contains the ContentCache* and the bits indicating the
-/// characteristic of the file and whether it has \#line info, all
-/// bitmangled together.
-uintptr_t Data;
+/// \brief The content cache and the characteristic of the file.
+llvm::PointerIntPair
+ContentAndKind;
 
 friend class clang::SourceManager;
 friend class clang::ASTWriter;
@@ -269,10 +281,9 @@ namespace SrcMgr {
   FileInfo X;
   X.IncludeLoc = IL.getRawEncoding();
   X.NumCreatedFIDs = 0;
-  X.Data = (uintptr_t)Con;
-  assert((X.Data & 7) == 0 &&"ContentCache pointer insufficiently 
aligned");
-  assert((unsigned)FileCharacter < 4 && "invalid file character");
-  X.Data |= (unsigned)FileCharacter;
+  X.HasLineDirectives = false;
+  X.ContentAndKind.setPointer(Con);
+  X.ContentAndKind.setInt(FileCharacter);
   return X;
 }
 
@@ -280,22 +291,22 @@ namespace SrcMgr {
   return SourceLocation::getFromRawEncoding(IncludeLoc);
 }
 
-const ContentCache* getContentCache() const {
-  return reinterpret_cast(Data & ~uintptr_t(7));
+const ContentCache *getContentCache() const {
+  return ContentAndKind.getPointer();
 }
 
 /// \brief Return whether this is a system header or not.
 CharacteristicKind getFileCharacteristic() const {
-  return (CharacteristicKind)(Data & 3);
+  return ContentAndKind.getInt();
 }
 
 /// \brief Return true if this FileID has \#line directives in it.
-bool hasLineDirectives() const { return (Data & 4) != 0; }
+bool hasLineDirectives() const { return HasLineDirectives; }
 
 /// \brief Set the flag that indicates that this FileID has
 /// line table entries associated with it.
 void setHasLineDirectives() {
-  Data |= 4;
+  HasLineDirectives = true;
 }
   };
 
@@ -407,6 +418,8 @@ namespace SrcMgr {
 };
 
   public:
+SLocEntry() : Offset(), IsExpansion(), File() {}
+
 unsigned getOffset() const { return Offset; }
 
 bool isExpansion() const { return IsExpansion; }
@@ -789,9 +802,8 @@ public:
   FileID createFileID(const FileEntry *SourceFile, SourceLocation IncludePos,
   SrcMgr::CharacteristicKind FileCharacter,
   int LoadedID = 0, unsigned LoadedOffset = 0) {
-const SrcMgr::ContentCache *
-  IR = 

[PATCH] D34508: [Analyzer] Bug Reporter Visitor to Display Values of Variables - PRELIMINARY!

[Anna Zaks via Phabricator via cfe-commits]

zaks.anna added a comment.

I agree that we should not print the values of all variables. The users will be 
overwhelmed with the huge amount of information. It is very valuable to 
highlight just the right information. (I believe even the current diagnostics 
often produce too much info and highlighting the more important notes would be 
very valuable.) However, the examples you presented are **very compelling**. 
Are there ways we could highlight the same information without printing values 
of all variables?

For example, for the overflow case, we could experiment with printing notes 
along the path at the locations a variable overflows. This would be very 
valuable for the array overflow alpha checker, which often flags the bugs that 
only occur if an integer value along the path overflows. I am not sure how 
noisy this approach will be. If it is too noisy,  we could refine this further.

For the uninitialized variable example, we could have some pattern-matching 
logic that would check if the expression is an array element and if so print 
the value of the index. (Another problem with variables that are failed to be 
initialized in a function is that we do not display the path on which the 
variable is not initialized, making it hard to understand the reports. Though, 
that is a separate problem.)


https://reviews.llvm.org/D34508



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

[PATCH] D34790: [NewPM] Add a flag -fexperimental-new-pass-manager=on/off/debug for printing debug output.

[Tim Shen via Phabricator via cfe-commits]

timshen added inline comments.



Comment at: clang/include/clang/Driver/Options.td:971-973
+def fexperimental_new_pass_manager_EQ : Joined<["-"], 
"fexperimental-new-pass-manager=">,
+  Group, Flags<[CC1Option]>,
+  HelpText<"Enables an experimental new pass manager in LLVM.">, 
Values<"on,off,debug">;

chandlerc wrote:
> I don't think we want to expose this flag in the driver -- it should really 
> be a CC1-only thing for debugging.
> 
> As such, I think I'd just make it an independent flag:
> 
>   -fexperimental-new-pass-manager-debug-logging
> 
> Or some such. This also seems easier than having to define a new kind.
The two small disadvantages of your suggestion is that
1) It's more verbose to type: `clang -fexperimental-new-pass-manager 
-fexperimental-new-pass-manager-debug-logging`
2) it's the 3 states vs 4 states pattern: `clang 
-fno-experimental-new-pass-manager 
-fexperimental-new-pass-manager-debug-logging` doesn't make sense to me, and 
it's good to avoid that.

What do you think about these trade offs?


https://reviews.llvm.org/D34790



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

[PATCH] D34633: [clang-tidy] Fix a bug in android-file-open-flag

[Yan Wang via Phabricator via cfe-commits]

yawanng updated this revision to Diff 104574.
yawanng added a comment.
Herald added subscribers: mgorny, srhines.

rename the check.


https://reviews.llvm.org/D34633

Files:
  clang-tidy/android/AndroidTidyModule.cpp
  clang-tidy/android/CMakeLists.txt
  clang-tidy/android/CloexecOpenCheck.cpp
  clang-tidy/android/CloexecOpenCheck.h
  clang-tidy/android/FileOpenFlagCheck.cpp
  clang-tidy/android/FileOpenFlagCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/android-cloexec-open.rst
  docs/clang-tidy/checks/android-file-open-flag.rst
  test/clang-tidy/android-cloexec-open.cpp
  test/clang-tidy/android-file-open-flag.cpp

Index: test/clang-tidy/android-cloexec-open.cpp
===
--- test/clang-tidy/android-cloexec-open.cpp
+++ test/clang-tidy/android-cloexec-open.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s android-file-open-flag %t
+// RUN: %check_clang_tidy %s android-cloexec-open %t
 
 #define O_RDWR 1
 #define O_EXCL 2
@@ -18,7 +18,7 @@
 
 void a() {
   open("filename", O_RDWR);
-  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: 'open' should use O_CLOEXEC where possible [android-file-open-flag]
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: 'open' should use O_CLOEXEC where possible [android-cloexec-open]
   // CHECK-FIXES: O_RDWR | O_CLOEXEC
   TEMP_FAILURE_RETRY(open("filename", O_RDWR));
   // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: 'open' should use O_CLOEXEC where
@@ -33,7 +33,7 @@
 
 void b() {
   open64("filename", O_RDWR);
-  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: 'open64' should use O_CLOEXEC where possible [android-file-open-flag]
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: 'open64' should use O_CLOEXEC where possible [android-cloexec-open]
   // CHECK-FIXES: O_RDWR | O_CLOEXEC
   TEMP_FAILURE_RETRY(open64("filename", O_RDWR));
   // CHECK-MESSAGES: :[[@LINE-1]]:47: warning: 'open64' should use O_CLOEXEC where
@@ -48,7 +48,7 @@
 
 void c() {
   openat(0, "filename", O_RDWR);
-  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 'openat' should use O_CLOEXEC where possible [android-file-open-flag]
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 'openat' should use O_CLOEXEC where possible [android-cloexec-open]
   // CHECK-FIXES: O_RDWR | O_CLOEXEC
   TEMP_FAILURE_RETRY(openat(0, "filename", O_RDWR));
   // CHECK-MESSAGES: :[[@LINE-1]]:50: warning: 'openat' should use O_CLOEXEC where
@@ -63,19 +63,19 @@
 
 void f() {
   open("filename", 3);
-  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'open' should use O_CLOEXEC where possible [android-file-open-flag]
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'open' should use O_CLOEXEC where possible [android-cloexec-open]
   // CHECK-FIXES: 3 | O_CLOEXEC
   TEMP_FAILURE_RETRY(open("filename", 3));
   // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: 'open' should use O_CLOEXEC where
   // CHECK-FIXES: 3 | O_CLOEXEC
   open64("filename", 3);
-  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 'open64' should use O_CLOEXEC where possible [android-file-open-flag]
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 'open64' should use O_CLOEXEC where possible [android-cloexec-open]
   // CHECK-FIXES: 3 | O_CLOEXEC
   TEMP_FAILURE_RETRY(open64("filename", 3));
   // CHECK-MESSAGES: :[[@LINE-1]]:42: warning: 'open64' should use O_CLOEXEC where
   // CHECK-FIXES: 3 | O_CLOEXEC
   openat(0, "filename", 3);
-  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: 'openat' should use O_CLOEXEC where possible [android-file-open-flag]
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: 'openat' should use O_CLOEXEC where possible [android-cloexec-open]
   // CHECK-FIXES: 3 | O_CLOEXEC
   TEMP_FAILURE_RETRY(openat(0, "filename", 3));
   // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: 'openat' should use O_CLOEXEC where
Index: docs/clang-tidy/checks/android-cloexec-open.rst
===
--- docs/clang-tidy/checks/android-cloexec-open.rst
+++ docs/clang-tidy/checks/android-cloexec-open.rst
@@ -1,7 +1,7 @@
-.. title:: clang-tidy - android-file-open-flag
+.. title:: clang-tidy - android-cloexec-open
 
-android-file-open-flag
-==
+android-cloexec-open
+
 
 A common source of security bugs is code that opens a file without using the
 ``O_CLOEXEC`` flag.  Without that flag, an opened sensitive file would remain
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,8 +57,8 @@
 Improvements to clang-tidy
 --
 
-- New `android-file-open-flag
-  `_ check
+- New `android-cloexec-open
+  `_ check
 
   Checks if the required file flag ``O_CLOEXEC`` exists in ``open()``,
   ``open64()`` and ``openat()``.
Index: clang-tidy/android/CloexecOpenCheck.h

Buildbot numbers for the week of 06/18/2017 - 06/24/2017

[Galina Kistanova via cfe-commits]

Hello everyone,

Below are some buildbot numbers for the last week of 06/18/2017 -
06/24/2017.

Please see the same data in attached csv files:

The longest time each builder was red during the last week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the last week:

buildername | was_red
+-
 clang-s390x-linux-lnt  | 57:44:53
 clang-s390x-linux  | 57:38:10
 sanitizer-x86_64-linux-bootstrap   | 39:56:29
 sanitizer-x86_64-linux-fast| 38:47:01
 clang-x86_64-debian-fast   | 37:44:55
 clang-x86-windows-msvc2015 | 27:07:11
 aosp-O3-polly-before-vectorizer-unprofitable   | 24:06:17
 ubuntu-gcc7.1-werror   | 21:31:01
 lld-x86_64-darwin13| 19:24:56
 polly-arm-linux| 17:53:05
 polly-amd64-linux  | 17:36:58
 perf-x86_64-penryn-O3-polly-before-vectorizer-detect-only  | 14:44:14
 clang-cmake-aarch64-lld| 13:41:19
 clang-lld-x86_64-2stage| 09:16:00
 clang-ppc64be-linux-lnt| 09:01:50
 clang-ppc64be-linux| 09:00:26
 sanitizer-ppc64be-linux| 08:50:21
 clang-ppc64be-linux-multistage | 08:26:39
 perf-x86_64-penryn-O3-polly| 06:26:54
 clang-with-lto-ubuntu  | 04:24:47
 clang-hexagon-elf  | 04:20:38
 llvm-hexagon-elf   | 04:15:45
 clang-cmake-armv7-a15-selfhost-neon| 04:06:14
 clang-cmake-armv7-a15-selfhost | 03:59:42
 lldb-windows7-android  | 03:56:07
 clang-with-thin-lto-ubuntu | 03:49:02
 lldb-x86_64-darwin-13.4| 03:37:56
 clang-ppc64le-linux-multistage | 03:22:36
 clang-cuda-build   | 03:14:32
 clang-cmake-thumbv7-a15-full-sh| 03:06:27
 sanitizer-x86_64-linux | 03:05:26
 llvm-clang-x86_64-expensive-checks-win | 03:03:39
 sanitizer-ppc64le-linux| 02:51:55
 clang-x64-ninja-win7   | 02:50:49
 clang-sphinx-docs  | 02:44:35
 lldb-x86-windows-msvc2015  | 02:33:55
 perf-x86_64-penryn-O3-polly-parallel-fast  | 02:32:06
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast   | 02:26:53
 perf-x86_64-penryn-O3-polly-before-vectorizer-unprofitable | 02:23:47
 clang-cmake-aarch64-42vma  | 02:06:39
 sanitizer-windows  | 01:53:42
 clang-cmake-armv7-a15-full | 01:51:01
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast | 01:47:17
 clang-atom-d525-fedora-rel | 01:39:46
 clang-cmake-thumbv7-a15| 01:39:41
 clang-cmake-armv7-a15  | 01:39:20
 clang-with-thin-lto-windows| 01:36:19
 perf-x86_64-penryn-O3-polly-unprofitable   | 01:25:05
 clang-ppc64le-linux| 01:17:41
 lldb-x86_64-ubuntu-14.04-buildserver   | 01:11:07
 lldb-amd64-ninja-netbsd8   | 00:54:56
 clang-bpf-build| 00:46:33
 clang-native-arm-lnt   | 00:46:07
 lld-x86_64-win7| 00:43:53
 lldb-x86_64-ubuntu-14.04-cmake | 00:39:50
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx1z | 00:37:09
 clang-tools-sphinx-docs| 00:34:46
 clang-x86_64-linux-abi-test| 00:34:16
 lld-x86_64-freebsd | 00:34:10
 

Buildbot numbers for the week of 06/11/2017 - 06/17/2017

[Galina Kistanova via cfe-commits]

Hello everyone,

Below are some buildbot numbers for the week of 06/11/2017 - 06/17/2017.

Please see the same data in attached csv files:

The longest time each builder was red during the last week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the last week:

buildername | was_red
+-
 clang-x86_64-linux-selfhost-modules| 95:51:35
 clang-s390x-linux-multistage   | 61:03:09
 clang-s390x-linux-lnt  | 58:35:21
 clang-s390x-linux  | 58:25:03
 clang-ppc64be-linux-multistage | 56:02:39
 clang-ppc64be-linux| 55:35:48
 clang-ppc64be-linux-lnt| 55:24:17
 sanitizer-x86_64-linux-fast| 48:37:55
 sanitizer-x86_64-linux-bootstrap   | 34:44:35
 aosp-O3-polly-before-vectorizer-unprofitable   | 24:00:42
 clang-lld-x86_64-2stage| 23:00:13
 clang-with-thin-lto-ubuntu | 22:47:28
 clang-with-lto-ubuntu  | 22:12:10
 llvm-clang-x86_64-expensive-checks-win | 20:33:28
 clang-cmake-armv7-a15-selfhost-neon| 18:26:45
 clang-ppc64le-linux-multistage | 14:59:54
 clang-cmake-armv7-a15-selfhost | 14:59:04
 clang-hexagon-elf  | 14:43:57
 clang-x86-windows-msvc2015 | 14:32:31
 clang-x64-ninja-win7   | 14:31:08
 clang-x86_64-linux-selfhost-modules-2  | 14:20:41
 clang-bpf-build| 14:20:40
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast | 14:20:18
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast   | 14:19:17
 clang-cmake-thumbv7-a15-full-sh| 12:44:54
 clang-cmake-armv7-a15  | 12:34:57
 clang-cmake-thumbv7-a15| 12:24:45
 clang-cmake-armv7-a15-full | 12:11:09
 perf-x86_64-penryn-O3-polly-fast   | 10:03:46
 perf-x86_64-penryn-O3  | 09:41:08
 perf-x86_64-penryn-O3-polly| 09:00:55
 clang-cmake-aarch64-full   | 06:04:31
 perf-x86_64-penryn-O3-polly-before-vectorizer-unprofitable | 05:16:51
 lldb-x86_64-ubuntu-14.04-android   | 05:00:38
 clang-cmake-aarch64-lld| 04:57:36
 sanitizer-ppc64be-linux| 04:47:18
 sanitizer-ppc64le-linux| 04:38:26
 libcxx-libcxxabi-x86_64-linux-ubuntu-gcc49-cxx11   | 04:24:20
 lldb-x86_64-darwin-13.4| 04:12:10
 lldb-windows7-android  | 03:59:01
 lld-x86_64-win7| 03:08:16
 lld-x86_64-darwin13| 03:02:05
 clang-cuda-build   | 02:56:39
 lld-x86_64-freebsd | 02:33:21
 libcxx-libcxxabi-x86_64-linux-ubuntu-gcc-tot-cxx1z | 02:27:06
 perf-x86_64-penryn-O3-polly-unprofitable   | 02:14:54
 clang-cmake-aarch64-quick  | 01:59:11
 clang-x86_64-linux-abi-test| 01:57:32
 lldb-x86-windows-msvc2015  | 01:52:18
 clang-cmake-aarch64-42vma  | 01:50:24
 lldb-amd64-ninja-netbsd8   | 01:46:06
 clang-ppc64le-linux| 01:45:41
 sanitizer-windows  | 01:44:37
 sanitizer-x86_64-linux | 01:35:01
 clang-x86_64-debian-fast   | 01:29:52
 clang-atom-d525-fedora-rel | 01:20:25
 lldb-x86_64-ubuntu-14.04-buildserver   | 01:14:08
 sanitizer-x86_64-linux-autoconf| 00:56:04
 libcxx-libcxxabi-libunwind-aarch64-linux-noexceptions  | 00:53:27
 

[PATCH] D34796: upporting -f(no)-reorder-functions flag, clang side change

[Taewook Oh via Phabricator via cfe-commits]

twoh created this revision.

When profile data is given, .hot/.unlikely section prefix is
added to hot/cold functions for linker to improve code locality. GCC
controls this behavior with '-f(no)-reorder-functions' flag, while LLVM
uses opt tool option '-profile-guided-section-prefix=true/false'. This
patch is for LLVM to support the same driver flag with GCC. LLVM side
patch is https://reviews.llvm.org/D34795.


https://reviews.llvm.org/D34796

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/BackendUtil.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/Inputs/freorder-functions.prof
  test/CodeGen/freorder-functions.c
  test/Driver/function-sections.c

Index: test/Driver/function-sections.c
===
--- test/Driver/function-sections.c
+++ test/Driver/function-sections.c
@@ -6,6 +6,8 @@
 // CHECK-NODS-NOT: -fdata-sections
 // CHECK-US-NOT: -fno-unique-section-names
 // CHECK-NOUS: -fno-unique-section-names
+// CHECK-RF-NOT: -fno-reorder-functions
+// CHECK-NORF: -fno-reorder-functions
 
 // RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1   \
 // RUN: -target i386-unknown-linux \
@@ -72,3 +74,13 @@
 // RUN: -target i386-unknown-linux \
 // RUN: -fno-unique-section-names \
 // RUN:   | FileCheck --check-prefix=CHECK-NOUS %s
+
+// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1\
+// RUN: -target i386-unknown-linux \
+// RUN: -freorder-functions \
+// RUN:   | FileCheck --check-prefix=CHECK-RF %s
+
+// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1\
+// RUN: -target i386-unknown-linux \
+// RUN: -fno-reorder-functions \
+// RUN:   | FileCheck --check-prefix=CHECK-NORF %s
Index: test/CodeGen/freorder-functions.c
===
--- /dev/null
+++ test/CodeGen/freorder-functions.c
@@ -0,0 +1,22 @@
+// REQUIRES: x86-registered-target
+
+// RUN: %clang_cc1 -triple x86_64-pc-linux -S -O3 -ffunction-sections -fprofile-sample-use=%S/Inputs/freorder-functions.prof -o - < %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux -S -O3 -ffunction-sections -fprofile-sample-use=%S/Inputs/freorder-functions.prof -fno-reorder-functions -o - < %s | FileCheck --check-prefix=CHECK-NOPREFIX %s
+
+// opt tool option precedes driver option.
+// RUN: %clang_cc1 -triple x86_64-pc-linux -S -O3 -ffunction-sections -fprofile-sample-use=%S/Inputs/freorder-functions.prof -fno-reorder-functions -mllvm -profile-guided-section-prefix=true -o - < %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux -S -O3 -ffunction-sections -fprofile-sample-use=%S/Inputs/freorder-functions.prof -freorder-functions -mllvm -profile-guided-section-prefix=false -o - < %s | FileCheck --check-prefix=CHECK-NOPREFIX %s
+
+void hot_func() {
+  return;
+}
+
+void cold_func() {
+  hot_func();
+  return;
+}
+
+// CHECK: .section .text.hot.hot_func,"ax",@progbits
+// CHECK: .section .text.unlikely.cold_func,"ax",@progbits
+// CHECK-NOPREFIX: .section .text.hot_func,"ax",@progbits
+// CHECK-NOPREFIX: .section .text.cold_func,"ax",@progbits
Index: test/CodeGen/Inputs/freorder-functions.prof
===
--- /dev/null
+++ test/CodeGen/Inputs/freorder-functions.prof
@@ -0,0 +1,5 @@
+hot_func:1000:0
+ 1: 0
+cold_func:0:0
+ 1: 1
+ 2: 1
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -643,6 +643,8 @@
OPT_fno_data_sections, false);
   Opts.UniqueSectionNames = Args.hasFlag(OPT_funique_section_names,
  OPT_fno_unique_section_names, true);
+  Opts.ReorderFunctions =
+  Args.hasFlag(OPT_freorder_functions, OPT_fno_reorder_functions, true);
 
   Opts.MergeFunctions = Args.hasArg(OPT_fmerge_functions);
 
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2861,6 +2861,10 @@
 options::OPT_fno_unique_section_names, true))
 CmdArgs.push_back("-fno-unique-section-names");
 
+  if (!Args.hasFlag(options::OPT_freorder_functions,
+options::OPT_fno_reorder_functions, true))
+CmdArgs.push_back("-fno-reorder-functions");
+
   Args.AddAllArgs(CmdArgs, options::OPT_finstrument_functions);
 
   addPGOAndCoverageFlags(C, D, Output, Args, CmdArgs);
Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -431,6 +431,7 @@
   Options.FunctionSections = CodeGenOpts.FunctionSections;
   Options.DataSections = CodeGenOpts.DataSections;
   

[PATCH] D34790: [NewPM] Add a flag -fexperimental-new-pass-manager=on/off/debug for printing debug output.

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc added inline comments.



Comment at: clang/include/clang/Driver/Options.td:971-973
+def fexperimental_new_pass_manager_EQ : Joined<["-"], 
"fexperimental-new-pass-manager=">,
+  Group, Flags<[CC1Option]>,
+  HelpText<"Enables an experimental new pass manager in LLVM.">, 
Values<"on,off,debug">;

I don't think we want to expose this flag in the driver -- it should really be 
a CC1-only thing for debugging.

As such, I think I'd just make it an independent flag:

  -fexperimental-new-pass-manager-debug-logging

Or some such. This also seems easier than having to define a new kind.


https://reviews.llvm.org/D34790



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

r306622 - CodeGen: handle missed case of COMDAT handling

[Saleem Abdulrasool via cfe-commits]

Author: compnerd
Date: Wed Jun 28 17:54:44 2017
New Revision: 306622

URL: http://llvm.org/viewvc/llvm-project?rev=306622=rev
Log:
CodeGen: handle missed case of COMDAT handling

When Protocol references are constructed, we need to add the reference
symbol to a COMDAT group on non-MachO object file formats (MachO handles
this by having a coalesced attribute).  This adds the missing case.

Modified:
cfe/trunk/lib/CodeGen/CGObjCMac.cpp
cfe/trunk/test/CodeGenObjC/protocol-comdat.m

Modified: cfe/trunk/lib/CodeGen/CGObjCMac.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjCMac.cpp?rev=306622=306621=306622=diff
==
--- cfe/trunk/lib/CodeGen/CGObjCMac.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGObjCMac.cpp Wed Jun 28 17:54:44 2017
@@ -6381,16 +6381,15 @@ llvm::Value *CGObjCNonFragileABIMac::Gen
   llvm::GlobalVariable *PTGV = CGM.getModule().getGlobalVariable(ProtocolName);
   if (PTGV)
 return CGF.Builder.CreateAlignedLoad(PTGV, Align);
-  PTGV = new llvm::GlobalVariable(
-CGM.getModule(),
-Init->getType(), false,
-llvm::GlobalValue::WeakAnyLinkage,
-Init,
-ProtocolName);
+  PTGV = new llvm::GlobalVariable(CGM.getModule(), Init->getType(), false,
+  llvm::GlobalValue::WeakAnyLinkage, Init,
+  ProtocolName);
   PTGV->setSection(GetSectionName("__objc_protorefs",
   "coalesced,no_dead_strip"));
   PTGV->setVisibility(llvm::GlobalValue::HiddenVisibility);
   PTGV->setAlignment(Align.getQuantity());
+  if (!CGM.getTriple().isOSBinFormatMachO())
+PTGV->setComdat(CGM.getModule().getOrInsertComdat(ProtocolName));
   CGM.addCompilerUsedGlobal(PTGV);
   return CGF.Builder.CreateAlignedLoad(PTGV, Align);
 }

Modified: cfe/trunk/test/CodeGenObjC/protocol-comdat.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/protocol-comdat.m?rev=306622=306621=306622=diff
==
--- cfe/trunk/test/CodeGenObjC/protocol-comdat.m (original)
+++ cfe/trunk/test/CodeGenObjC/protocol-comdat.m Wed Jun 28 17:54:44 2017
@@ -4,6 +4,9 @@
 - (void) method;
 @end
 
+@protocol Q;
+@protocol R;
+
 @interface I
 @end
 
@@ -11,9 +14,14 @@
 - (void) method { }
 @end
 
+_Bool f(void) {
+  return @protocol(Q) == @protocol(R);
+}
 
 // CHECK: $"\01l_OBJC_PROTOCOL_$_P" = comdat any
 // CHECK: $"\01l_OBJC_LABEL_PROTOCOL_$_P" = comdat any
+// CHECK: $"\01l_OBJC_PROTOCOL_REFERENCE_$_Q" = comdat any
+// CHECK: $"\01l_OBJC_PROTOCOL_REFERENCE_$_R" = comdat any
 
 // CHECK: @"\01l_OBJC_PROTOCOL_$_P" = {{.*}}, comdat
 // CHECK: @"\01l_OBJC_LABEL_PROTOCOL_$_P" = {{.*}}, comdat


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

[PATCH] D34728: [ThinkLTO] Invoke build(Thin)?LTOPreLinkDefaultPipeline.

[Chandler Carruth via Phabricator via cfe-commits]

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

LGTM as well.


https://reviews.llvm.org/D34728



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

[PATCH] D34728: [ThinkLTO] Invoke build(Thin)?LTOPreLinkDefaultPipeline.

[Tim Shen via Phabricator via cfe-commits]

timshen updated this revision to Diff 104551.
timshen added a comment.

Splitted into two patches. I'll commit this one (ThinLTO change) without the 
test first, then commit the flag change with the Clang ThinLTO pipeline test.


https://reviews.llvm.org/D34728

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/test/Other/new-pm-thinlto-defaults.ll

Index: llvm/test/Other/new-pm-thinlto-defaults.ll
===
--- llvm/test/Other/new-pm-thinlto-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-defaults.ll
@@ -9,19 +9,19 @@
 ;
 ; Prelink pipelines:
 ; RUN: opt -disable-verify -debug-pass-manager \
-; RUN: -passes='thinlto-pre-link' -S %s 2>&1 \
+; RUN: -passes='thinlto-pre-link,name-anon-globals' -S %s 2>&1 \
 ; RUN: | FileCheck %s --check-prefixes=CHECK-O,CHECK-O1,CHECK-PRELINK-O,CHECK-PRELINK-O1
 ; RUN: opt -disable-verify -debug-pass-manager \
-; RUN: -passes='thinlto-pre-link' -S  %s 2>&1 \
+; RUN: -passes='thinlto-pre-link,name-anon-globals' -S  %s 2>&1 \
 ; RUN: | FileCheck %s --check-prefixes=CHECK-O,CHECK-O2,CHECK-PRELINK-O,CHECK-PRELINK-O2
 ; RUN: opt -disable-verify -debug-pass-manager \
-; RUN: -passes='thinlto-pre-link' -S  %s 2>&1 \
+; RUN: -passes='thinlto-pre-link,name-anon-globals' -S  %s 2>&1 \
 ; RUN: | FileCheck %s --check-prefixes=CHECK-O,CHECK-O3,CHECK-PRELINK-O,CHECK-PRELINK-O3
 ; RUN: opt -disable-verify -debug-pass-manager \
-; RUN: -passes='thinlto-pre-link' -S %s 2>&1 \
+; RUN: -passes='thinlto-pre-link,name-anon-globals' -S %s 2>&1 \
 ; RUN: | FileCheck %s --check-prefixes=CHECK-O,CHECK-Os,CHECK-PRELINK-O,CHECK-PRELINK-Os
 ; RUN: opt -disable-verify -debug-pass-manager \
-; RUN: -passes='thinlto-pre-link' -S %s 2>&1 \
+; RUN: -passes='thinlto-pre-link,name-anon-globals' -S %s 2>&1 \
 ; RUN: | FileCheck %s --check-prefixes=CHECK-O,CHECK-Oz,CHECK-PRELINK-O,CHECK-PRELINK-Oz
 ;
 ; Postlink pipelines:
@@ -154,7 +154,6 @@
 ; CHECK-O-NEXT: Finished CGSCC pass manager run.
 ; CHECK-O-NEXT: Finished llvm::Module pass manager run.
 ; CHECK-PRELINK-O-NEXT: Running pass: GlobalOptPass
-; CHECK-PRELINK-O-NEXT: Running pass: NameAnonGlobalPass
 ; CHECK-POSTLINK-O-NEXT: Running pass: PassManager<{{.*}}Module{{.*}}>
 ; CHECK-POSTLINK-O-NEXT: Starting llvm::Module pass manager run.
 ; CHECK-POSTLINK-O-NEXT: Running pass: GlobalOptPass
@@ -188,6 +187,7 @@
 ; CHECK-POSTLINK-O-NEXT: Running pass: ConstantMergePass
 ; CHECK-POSTLINK-O-NEXT: Finished llvm::Module pass manager run.
 ; CHECK-O-NEXT: Finished llvm::Module pass manager run.
+; CHECK-PRELINK-O-NEXT: Running pass: NameAnonGlobalPass
 ; CHECK-O-NEXT: Running pass: PrintModulePass
 
 ; Make sure we get the IR back out without changes when we print the module.
Index: llvm/lib/Passes/PassBuilder.cpp
===
--- llvm/lib/Passes/PassBuilder.cpp
+++ llvm/lib/Passes/PassBuilder.cpp
@@ -758,9 +758,6 @@
   // Reduce the size of the IR as much as possible.
   MPM.addPass(GlobalOptPass());
 
-  // Rename anon globals to be able to export them in the summary.
-  MPM.addPass(NameAnonGlobalPass());
-
   return MPM;
 }
 
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -54,6 +54,7 @@
 #include "llvm/Transforms/ObjCARC.h"
 #include "llvm/Transforms/Scalar.h"
 #include "llvm/Transforms/Scalar/GVN.h"
+#include "llvm/Transforms/Utils/NameAnonGlobals.h"
 #include "llvm/Transforms/Utils/SymbolRewriter.h"
 #include 
 using namespace clang;
@@ -881,17 +882,28 @@
   ModulePassManager MPM;
 
   if (!CodeGenOpts.DisableLLVMPasses) {
+bool IsThinLTO = CodeGenOpts.EmitSummaryIndex;
+bool IsLTO = CodeGenOpts.PrepareForLTO;
+
 if (CodeGenOpts.OptimizationLevel == 0) {
   // Build a minimal pipeline based on the semantics required by Clang,
   // which is just that always inlining occurs.
   MPM.addPass(AlwaysInlinerPass());
+  if (IsThinLTO)
+MPM.addPass(NameAnonGlobalPass());
 } else {
-  // Otherwise, use the default pass pipeline. We also have to map our
-  // optimization levels into one of the distinct levels used to configure
-  // the pipeline.
+  // Map our optimization levels into one of the distinct levels used to
+  // configure the pipeline.
   PassBuilder::OptimizationLevel Level = mapToLevel(CodeGenOpts);
 
-  MPM = PB.buildPerModuleDefaultPipeline(Level);
+  if (IsThinLTO) {
+MPM = PB.buildThinLTOPreLinkDefaultPipeline(Level);
+MPM.addPass(NameAnonGlobalPass());
+  } else if (IsLTO) {
+MPM = PB.buildLTOPreLinkDefaultPipeline(Level);
+  } else {
+MPM = PB.buildPerModuleDefaultPipeline(Level);
+  }
 }
   }
 
___
cfe-commits mailing list

[PATCH] D34790: [NewPM] Add a flag -fexperimental-new-pass-manager=on/off/debug for printing debug output.

[Tim Shen via Phabricator via cfe-commits]

timshen created this revision.
Herald added subscribers: hiraditya, mehdi_amini, sanjoy.

https://reviews.llvm.org/D34790

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/CodeGenOptions.def
  clang/include/clang/Frontend/CodeGenOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/lto-newpm-pipeline.c
  llvm/include/llvm/Option/ArgList.h
  llvm/lib/Option/ArgList.cpp

Index: llvm/lib/Option/ArgList.cpp
===
--- llvm/lib/Option/ArgList.cpp
+++ llvm/lib/Option/ArgList.cpp
@@ -95,21 +95,6 @@
   return std::vector(Values.begin(), Values.end());
 }
 
-void ArgList::AddLastArg(ArgStringList , OptSpecifier Id) const {
-  if (Arg *A = getLastArg(Id)) {
-A->claim();
-A->render(*this, Output);
-  }
-}
-
-void ArgList::AddLastArg(ArgStringList , OptSpecifier Id0,
- OptSpecifier Id1) const {
-  if (Arg *A = getLastArg(Id0, Id1)) {
-A->claim();
-A->render(*this, Output);
-  }
-}
-
 void ArgList::AddAllArgsExcept(ArgStringList ,
ArrayRef Ids,
ArrayRef ExcludeIds) const {
Index: llvm/include/llvm/Option/ArgList.h
===
--- llvm/include/llvm/Option/ArgList.h
+++ llvm/include/llvm/Option/ArgList.h
@@ -306,9 +306,13 @@
bool Default = true) const;
 
   /// AddLastArg - Render only the last argument match \p Id0, if present.
-  void AddLastArg(ArgStringList , OptSpecifier Id0) const;
-  void AddLastArg(ArgStringList , OptSpecifier Id0,
-  OptSpecifier Id1) const;
+  template 
+  void AddLastArg(ArgStringList , OptSpecifiers ...Ids) const {
+if (Arg *A = getLastArg(Ids...)) {
+  A->claim();
+  A->render(*this, Output);
+}
+  }
 
   /// AddAllArgsExcept - Render all arguments matching any of the given ids
   /// and not matching any of the excluded ids.
Index: clang/test/CodeGen/lto-newpm-pipeline.c
===
--- /dev/null
+++ clang/test/CodeGen/lto-newpm-pipeline.c
@@ -0,0 +1,26 @@
+// REQUIRES: x86-registered-target
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -c -o /dev/null -fexperimental-new-pass-manager=debug -flto=full -O0 %s 2>&1 | FileCheck %s \
+// RUN:   -check-prefix=CHECK-FULL-O0
+// RUN: %clang -target x86_64-unknown-linux-gnu -c -o /dev/null -fexperimental-new-pass-manager=debug -flto=full -O2 %s 2>&1 | FileCheck %s \
+// RUN:   -check-prefix=CHECK-FULL-O2
+// RUN: %clang -target x86_64-unknown-linux-gnu -c -o /dev/null -fexperimental-new-pass-manager=debug -flto=thin -O0 %s 2>&1 | FileCheck %s \
+// RUN:   -check-prefix=CHECK-THIN-O0
+// RUN: %clang -target x86_64-unknown-linux-gnu -c -o /dev/null -fexperimental-new-pass-manager=debug -flto=thin -O2 %s 2>&1 | FileCheck %s \
+// RUN:   -check-prefix=CHECK-THIN-O2
+
+// CHECK-FULL-O0: Running pass: AlwaysInlinerPass
+// CHECK-FULL-O0-NEXT: Running pass: BitcodeWriterPass
+
+// CHECK-THIN-O0: Running pass: AlwaysInlinerPass
+// CHECK-THIN-O0-NEXT: Running pass: NameAnonGlobalPass
+// CHECK-THIN-O0-NEXT: Running pass: ThinLTOBitcodeWriterPass
+
+// CHECK-FULL-O2: Running pass: GlobalOptPass
+// CHECK-FULL-O2: Running pass: BitcodeWriterPass
+
+// CHECK-THIN-O2: Running pass: GlobalOptPass
+// CHECK-THIN-O2: Running pass: NameAnonGlobalPass
+// CHECK-THIN-O2-NEXT: Running pass: ThinLTOBitcodeWriterPass
+
+void Foo() {}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -472,9 +472,23 @@
 }
   }
 
-  Opts.ExperimentalNewPassManager = Args.hasFlag(
-  OPT_fexperimental_new_pass_manager, OPT_fno_experimental_new_pass_manager,
-  /* Default */ false);
+  if (Args.hasFlag(OPT_fexperimental_new_pass_manager,
+   OPT_fno_experimental_new_pass_manager, false))
+Opts.setExperimentalNewPassManager(CodeGenOptions::NewPM_On);
+  else
+Opts.setExperimentalNewPassManager(CodeGenOptions::NewPM_Off);
+
+  if (Arg *A = Args.getLastArg(OPT_fexperimental_new_pass_manager_EQ)) {
+StringRef Name = A->getValue();
+if (Name == "on")
+  Opts.setExperimentalNewPassManager(CodeGenOptions::NewPM_On);
+else if (Name == "off")
+  Opts.setExperimentalNewPassManager(CodeGenOptions::NewPM_Off);
+else if (Name == "debug")
+  Opts.setExperimentalNewPassManager(CodeGenOptions::NewPM_Debug);
+else
+  Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Name;
+  }
 
   if (Arg *A = Args.getLastArg(OPT_fveclib)) {
 StringRef Name = A->getValue();
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- 

Re: r306583 - [ASTReader] Treat multiple defns of ObjC protocols the same as interfaces.

[Graydon Hoare via cfe-commits]

Yes, I wasn't sure how to test it at this level (there's a pending test at the 
swift level) but, as you pointed out in separate email, there was an adaptable 
test in the original commit this is a copy-and-modification of, so I 
copied-and-modified the test as well. Posted for review in 
https://reviews.llvm.org/D34788 

-Graydon

> On Jun 28, 2017, at 2:18 PM, Bruno Cardoso Lopes  
> wrote:
> 
> Hi Graydon,
> 
> Can you please add a testcase for this?
> 
> Thanks,
> 
> On Wed, Jun 28, 2017 at 11:36 AM, Graydon Hoare via cfe-commits 
> > wrote:
> Author: graydon
> Date: Wed Jun 28 11:36:27 2017
> New Revision: 306583
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=306583=rev 
> 
> Log:
> [ASTReader] Treat multiple defns of ObjC protocols the same as interfaces.
> 
> Summary:
> In change 2ba19793512, the ASTReader logic for ObjC interfaces was modified to
> preserve the first definition-data read, "merging" later definitions into it
> rather than overwriting it (though this "merging" is, in practice, a no-op 
> that
> discards the later definition-data).
> 
> Unfortunately this change was only made to ObjC interfaces, not protocols; 
> this
> means that when (for example) loading a protocol that references an interface,
> if both the protocol and interface are multiply defined (as can easily happen
> if the same header is read from multiple contexts), an _inconsistent_ pair of
> definitions is loaded: first-read for the interface and last-read for the
> protocol.
> 
> This in turn causes very subtle downstream bugs in the Swift ClangImporter,
> which filters the results of name lookups based on the owning module of a
> definition; inconsistency between a pair of related definitions causes name
> lookup failures at various stages of compilation.
> 
> To fix these downstream issues, this change replicates the logic applied to
> interfaces in change 2ba19793512, but for ObjC protocols.
> 
> rdar://30851899
> 
> Reviewers: doug.gregor, rsmith
> 
> Reviewed By: doug.gregor
> 
> Subscribers: jordan_rose, cfe-commits
> 
> Differential Revision: https://reviews.llvm.org/D34741 
> 
> 
> Modified:
> cfe/trunk/include/clang/AST/Redeclarable.h
> cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
> 
> Modified: cfe/trunk/include/clang/AST/Redeclarable.h
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Redeclarable.h?rev=306583=306582=306583=diff
>  
> 
> ==
> --- cfe/trunk/include/clang/AST/Redeclarable.h (original)
> +++ cfe/trunk/include/clang/AST/Redeclarable.h Wed Jun 28 11:36:27 2017
> @@ -21,6 +21,60 @@
>  namespace clang {
>  class ASTContext;
> 
> +// Some notes on redeclarables:
> +//
> +//  - Every redeclarable is on a circular linked list.
> +//
> +//  - Every decl has a pointer to the first element of the chain _and_ a
> +//DeclLink that may point to one of 3 possible states:
> +//  - the "previous" (temporal) element in the chain
> +//  - the "latest" (temporal) element in the chain
> +//  - the an "uninitialized-latest" value (when newly-constructed)
> +//
> +//  - The first element is also often called the canonical element. Every
> +//element has a pointer to it so that "getCanonical" can be fast.
> +//
> +//  - Most links in the chain point to previous, except the link out of
> +//the first; it points to latest.
> +//
> +//  - Elements are called "first", "previous", "latest" or
> +//"most-recent" when referring to temporal order: order of addition
> +//to the chain.
> +//
> +//  - To make matters confusing, the DeclLink type uses the term "next"
> +//for its pointer-storage internally (thus functions like
> +//NextIsPrevious). It's easiest to just ignore the implementation of
> +//DeclLink when making sense of the redeclaration chain.
> +//
> +//  - There's also a "definition" link for several types of
> +//redeclarable, where only one definition should exist at any given
> +//time (and the defn pointer is stored in the decl's "data" which
> +//is copied to every element on the chain when it's changed).
> +//
> +//Here is some ASCII art:
> +//
> +//  "first" "latest"
> +//  "canonical" "most recent"
> +//  ++ first+--+
> +//  || <--- |  |
> +//  ||  |  |
> +//  ||  |  |
> +//  ||   +--+   |  |

[PATCH] D34728: [ThinkLTO] Invoke build(Thin)?LTOPreLinkDefaultPipeline.

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In https://reviews.llvm.org/D34728#794508, @timshen wrote:

> Added -fexperimental-new-pass-manager=off/on/debug for printing debug 
> information.
>
> Added a Clang test.
>
> Do tell if you want me to split this patch. I didn't, becuase then I don't 
> have to write a test for each of them. :)


I think they should be split into two patches, one for the option change and 
one for the thinLTO pipeline fixes. You could just put the new 
lto-newpm-pipeline.c test in with the option changes patch and commit it right 
after the thinlto pipeline fixes (or do it first and modify that test to add 
the thinlto passes when the pipeline fix goes in).

LGTM, but please wait to see if Chandler has any more comments.




Comment at: clang/include/clang/Frontend/CodeGenOptions.h:92
 
+  enum NewPassManagerKind {
+NewPM_Off,

Document


https://reviews.llvm.org/D34728



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

[PATCH] D34788: [ASTReader] Add test for previous change r306583 / 145692e.

[Graydon Hoare via Phabricator via cfe-commits]

graydon created this revision.

Add a test for the change to ASTReader that reproduces the
logic for consolidating multiple ObjC interface definitions to the
case of multiple ObjC protocol definitions.

This test is a modified copy of the test that accompanied the original
change to interfaces, in 2ba1979.


https://reviews.llvm.org/D34788

Files:
  test/Modules/Inputs/lookup-assert-protocol/Base.h
  test/Modules/Inputs/lookup-assert-protocol/Derive.h
  test/Modules/Inputs/lookup-assert-protocol/H3.h
  test/Modules/Inputs/lookup-assert-protocol/module.map
  test/Modules/lookup-assert-protocol.m


Index: test/Modules/lookup-assert-protocol.m
===
--- /dev/null
+++ test/Modules/lookup-assert-protocol.m
@@ -0,0 +1,17 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -I 
%S/Inputs/lookup-assert-protocol %s -verify
+// expected-no-diagnostics
+
+#include "Derive.h"
+#import 
+
+__attribute__((objc_root_class))
+@interface Thing
+@end
+
+@implementation Thing
+- (void)test {
+}
+- (void)test2 {
+}
+@end
Index: test/Modules/Inputs/lookup-assert-protocol/module.map
===
--- /dev/null
+++ test/Modules/Inputs/lookup-assert-protocol/module.map
@@ -0,0 +1,4 @@
+module X {
+  header "H3.h"
+  export *
+}
Index: test/Modules/Inputs/lookup-assert-protocol/H3.h
===
--- /dev/null
+++ test/Modules/Inputs/lookup-assert-protocol/H3.h
@@ -0,0 +1 @@
+#include "Base.h"
Index: test/Modules/Inputs/lookup-assert-protocol/Derive.h
===
--- /dev/null
+++ test/Modules/Inputs/lookup-assert-protocol/Derive.h
@@ -0,0 +1,4 @@
+#include "Base.h"
+@protocol DerivedProtocol
+- (void) test2;
+@end
Index: test/Modules/Inputs/lookup-assert-protocol/Base.h
===
--- /dev/null
+++ test/Modules/Inputs/lookup-assert-protocol/Base.h
@@ -0,0 +1,3 @@
+@protocol BaseProtocol
+- (void) test;
+@end


Index: test/Modules/lookup-assert-protocol.m
===
--- /dev/null
+++ test/Modules/lookup-assert-protocol.m
@@ -0,0 +1,17 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -I %S/Inputs/lookup-assert-protocol %s -verify
+// expected-no-diagnostics
+
+#include "Derive.h"
+#import 
+
+__attribute__((objc_root_class))
+@interface Thing
+@end
+
+@implementation Thing
+- (void)test {
+}
+- (void)test2 {
+}
+@end
Index: test/Modules/Inputs/lookup-assert-protocol/module.map
===
--- /dev/null
+++ test/Modules/Inputs/lookup-assert-protocol/module.map
@@ -0,0 +1,4 @@
+module X {
+  header "H3.h"
+  export *
+}
Index: test/Modules/Inputs/lookup-assert-protocol/H3.h
===
--- /dev/null
+++ test/Modules/Inputs/lookup-assert-protocol/H3.h
@@ -0,0 +1 @@
+#include "Base.h"
Index: test/Modules/Inputs/lookup-assert-protocol/Derive.h
===
--- /dev/null
+++ test/Modules/Inputs/lookup-assert-protocol/Derive.h
@@ -0,0 +1,4 @@
+#include "Base.h"
+@protocol DerivedProtocol
+- (void) test2;
+@end
Index: test/Modules/Inputs/lookup-assert-protocol/Base.h
===
--- /dev/null
+++ test/Modules/Inputs/lookup-assert-protocol/Base.h
@@ -0,0 +1,3 @@
+@protocol BaseProtocol
+- (void) test;
+@end
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29654: [OpenMP] Integrate OpenMP target region cubin into host binary

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea updated this revision to Diff 104535.
gtbercea added a comment.

[Update regression tests] Add a test for propagating the compute capability to 
the OpenMP device offloading toolchain which targets NVIDIA GPUs.
This is a test for patch D34784  which is 
enabled by this patch.


Repository:
  rL LLVM

https://reviews.llvm.org/D29654

Files:
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/CommonArgs.cpp
  lib/Driver/ToolChains/CommonArgs.h
  lib/Driver/ToolChains/Cuda.cpp
  lib/Driver/ToolChains/Cuda.h
  lib/Driver/ToolChains/Gnu.cpp
  test/Driver/openmp-offload.c

Index: test/Driver/openmp-offload.c
===
--- test/Driver/openmp-offload.c
+++ test/Driver/openmp-offload.c
@@ -607,3 +607,34 @@
 // RUN:   | FileCheck -check-prefix=CHK-COMPUTE-CAPABILITY %s
 
 // CHK-COMPUTE-CAPABILITY: clang: warning: argument unused during compilation: '-fopenmp-target-arch=sm_35'
+
+/// ###
+
+/// Check -fopenmp-target-arch propagates compute capability to device offloading toolchain.
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda -save-temps -no-canonical-prefixes -fopenmp-target-arch=sm_35 %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-GPU-COMPUTE-CAPABILITY %s
+
+// CHK-GPU-COMPUTE-CAPABILITY: ptxas{{.*}}" "--gpu-name" "sm_35"
+// CHK-GPU-COMPUTE-CAPABILITY-NEXT: nvlink{{.*}}" "-arch" "sm_35"
+
+/// ###
+
+/// Check cubin file generation and usage by nvlink
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda -save-temps -no-canonical-prefixes %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-CUBIN %s
+
+// CHK-CUBIN: clang{{.*}}" "-o" "{{.*}}-openmp-nvptx64-nvidia-cuda.s"
+// CHK-CUBIN-NEXT: ptxas{{.*}}" "--output-file" "{{.*}}-openmp-nvptx64-nvidia-cuda.cubin" "{{.*}}-openmp-nvptx64-nvidia-cuda.s"
+// CHK-CUBIN-NEXT: nvlink" "-o" "{{.*}}-openmp-nvptx64-nvidia-cuda" {{.*}} "openmp-offload-openmp-nvptx64-nvidia-cuda.cubin"
+
+/// ###
+
+/// Check cubin file generation and usage by nvlink
+// RUN:   touch %t1.o
+// RUN:   touch %t2.o
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda -save-temps -no-canonical-prefixes %t1.o %t2.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-TWOCUBIN %s
+
+// CHK-TWOCUBIN: clang-offload-bundler" "-type=o" "{{.*}}inputs={{.*}}tmp1.o" "-outputs={{.*}}.o,{{.*}}tmp1-openmp-nvptx64-nvidia-cuda.cubin" "-unbundle"
+// CHK-TWOCUBIN-NEXT: clang-offload-bundler" "-type=o" "{{.*}}inputs={{.*}}tmp2.o" "-outputs={{.*}}.o,{{.*}}tmp2-openmp-nvptx64-nvidia-cuda.cubin" "-unbundle"
+// CHK-TWOCUBIN-NEXT: nvlink" "-o" "{{.*}}-openmp-nvptx64-nvidia-cuda" {{.*}} "openmp-offload.c.tmp1-openmp-nvptx64-nvidia-cuda.cubin" "openmp-offload.c.tmp2-openmp-nvptx64-nvidia-cuda.cubin"
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -203,131 +203,6 @@
   // The types are (hopefully) good enough.
 }
 
-/// Add OpenMP linker script arguments at the end of the argument list so that
-/// the fat binary is built by embedding each of the device images into the
-/// host. The linker script also defines a few symbols required by the code
-/// generation so that the images can be easily retrieved at runtime by the
-/// offloading library. This should be used only in tool chains that support
-/// linker scripts.
-static void AddOpenMPLinkerScript(const ToolChain , Compilation ,
-  const InputInfo ,
-  const InputInfoList ,
-  const ArgList , ArgStringList ,
-  const JobAction ) {
-
-  // If this is not an OpenMP host toolchain, we don't need to do anything.
-  if (!JA.isHostOffloading(Action::OFK_OpenMP))
-return;
-
-  // Create temporary linker script. Keep it if save-temps is enabled.
-  const char *LKS;
-  SmallString<256> Name = llvm::sys::path::filename(Output.getFilename());
-  if (C.getDriver().isSaveTempsEnabled()) {
-llvm::sys::path::replace_extension(Name, "lk");
-LKS = C.getArgs().MakeArgString(Name.c_str());
-  } else {
-llvm::sys::path::replace_extension(Name, "");
-Name = C.getDriver().GetTemporaryPath(Name, "lk");
-LKS = C.addTempFile(C.getArgs().MakeArgString(Name.c_str()));
-  }
-
-  // Add linker script option to the command.
-  CmdArgs.push_back("-T");
-  CmdArgs.push_back(LKS);
-
-  // Create a buffer to write the contents of the linker script.
-  std::string LksBuffer;
-  llvm::raw_string_ostream LksStream(LksBuffer);
-
-  // Get the OpenMP offload tool chains so that we can extract the triple
-  // 

[clang-tools-extra] r306612 - clangDaemon requires clangLex.

[NAKAMURA Takumi via cfe-commits]

Author: chapuni
Date: Wed Jun 28 15:57:15 2017
New Revision: 306612

URL: http://llvm.org/viewvc/llvm-project?rev=306612=rev
Log:
clangDaemon requires clangLex.

Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt

Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=306612=306611=306612=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Wed Jun 28 15:57:15 2017
@@ -19,6 +19,7 @@ add_clang_library(clangDaemon
   clangFormat
   clangFrontend
   clangIndex
+  clangLex
   clangSema
   clangTooling
   clangToolingCore


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

[PATCH] D34728: [ThinkLTO] Invoke build(Thin)?LTOPreLinkDefaultPipeline.

[Tim Shen via Phabricator via cfe-commits]

timshen added inline comments.



Comment at: clang/lib/CodeGen/BackendUtil.cpp:804-807
+  case 0:
+return PassBuilder::O0;
+
   case 1:

tejohnson wrote:
> chandlerc wrote:
> > Why is this change needed?
> I assume it is just cleanup since this isn't currently called under O0 (so it 
> would hit the assert under the default switch case if we ever did). Not sure 
> that is necessary though, up to you two.
Removed. I used to use it in prototyping, and forgot to remove it. It's always 
good to take a look at the patch after posting it, and I failed to do so. :P



Comment at: clang/lib/CodeGen/BackendUtil.cpp:893-894
   MPM.addPass(AlwaysInlinerPass());
+  if (IsThinLTO)
+MPM.addPass(NameAnonGlobalPass());
 } else {

chandlerc wrote:
> Why not a single addition of this pass at the end and a comment explaining 
> taht regardless of optimization level this is required for ThinLTO?
I feel more readable this way. The structure has less "joint points", aka the 
if statements form a tree, rather than a DAG. I'm fine with the current 
duplication.



Comment at: llvm/test/Other/new-pm-thinlto-defaults.ll:157
 ; CHECK-PRELINK-O-NEXT: Running pass: GlobalOptPass
-; CHECK-PRELINK-O-NEXT: Running pass: NameAnonGlobalPass
 ; CHECK-POSTLINK-O-NEXT: Running pass: PassManager<{{.*}}Module{{.*}}>

tejohnson wrote:
> chandlerc wrote:
> > tejohnson wrote:
> > > Can this be changed to check for the pass being added in its new 
> > > location, since it should still be invoked somewhere for ThinLTO? If this 
> > > change means it is no longer added under options to set up the thinlto 
> > > pipeline via opt, I'd prefer that we go back to adding this to the 
> > > pipeline in buildThinLTOPreLinkDefaultPipeline in the non-O0 case.
> > It seems somewhat unfortunate to have a *semantic* requirement on a 
> > particular placement of this pass inside of the pipeline... Especially when 
> > the semantics pretty much only stem from C++. For example, an a language 
> > without anonymous globals, you might not want this pass when doing ThinLTO.
> > 
> > Note that you can exactly model the thing Clang is doing with opt even 
> > after this:
> > 
> >   opt -passes='thinlto-pre-link,name-anon-globals'
> > For example, an a language without anonymous globals, you might not want 
> > this pass when doing ThinLTO.
> 
> Wouldn't it just be a no-op?
> 
> > opt -passes='thinlto-pre-link,name-anon-globals'
> 
> True, it just seems less user-friendly. But since this is just for testing, I 
> guess it doesn't matter much. So I am ok with your suggestion of pulling that 
> out and doing a single call to this pass when in ThinLTO mode after setting 
> up the pipelines.
I added a clang test for checking everything in the pipeline.

In this LLVM test I used 'thinlto-pre-link,name-anon-globals', but the 
order of that pass changed.


https://reviews.llvm.org/D34728



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

[PATCH] D34728: [ThinkLTO] Invoke build(Thin)?LTOPreLinkDefaultPipeline.

[Tim Shen via Phabricator via cfe-commits]

timshen updated this revision to Diff 104533.
timshen marked 5 inline comments as done.
timshen added a comment.

Added -fexperimental-new-pass-manager=off/on/debug for printing debug 
information.

Added a Clang test.

Do tell if you want me to split this patch. I didn't, becuase then I don't have 
to write a test for each of them. :)


https://reviews.llvm.org/D34728

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/CodeGenOptions.def
  clang/include/clang/Frontend/CodeGenOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/lto-newpm-pipeline.c
  llvm/include/llvm/Option/ArgList.h
  llvm/lib/Option/ArgList.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/test/Other/new-pm-thinlto-defaults.ll

Index: llvm/test/Other/new-pm-thinlto-defaults.ll
===
--- llvm/test/Other/new-pm-thinlto-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-defaults.ll
@@ -9,19 +9,19 @@
 ;
 ; Prelink pipelines:
 ; RUN: opt -disable-verify -debug-pass-manager \
-; RUN: -passes='thinlto-pre-link' -S %s 2>&1 \
+; RUN: -passes='thinlto-pre-link,name-anon-globals' -S %s 2>&1 \
 ; RUN: | FileCheck %s --check-prefixes=CHECK-O,CHECK-O1,CHECK-PRELINK-O,CHECK-PRELINK-O1
 ; RUN: opt -disable-verify -debug-pass-manager \
-; RUN: -passes='thinlto-pre-link' -S  %s 2>&1 \
+; RUN: -passes='thinlto-pre-link,name-anon-globals' -S  %s 2>&1 \
 ; RUN: | FileCheck %s --check-prefixes=CHECK-O,CHECK-O2,CHECK-PRELINK-O,CHECK-PRELINK-O2
 ; RUN: opt -disable-verify -debug-pass-manager \
-; RUN: -passes='thinlto-pre-link' -S  %s 2>&1 \
+; RUN: -passes='thinlto-pre-link,name-anon-globals' -S  %s 2>&1 \
 ; RUN: | FileCheck %s --check-prefixes=CHECK-O,CHECK-O3,CHECK-PRELINK-O,CHECK-PRELINK-O3
 ; RUN: opt -disable-verify -debug-pass-manager \
-; RUN: -passes='thinlto-pre-link' -S %s 2>&1 \
+; RUN: -passes='thinlto-pre-link,name-anon-globals' -S %s 2>&1 \
 ; RUN: | FileCheck %s --check-prefixes=CHECK-O,CHECK-Os,CHECK-PRELINK-O,CHECK-PRELINK-Os
 ; RUN: opt -disable-verify -debug-pass-manager \
-; RUN: -passes='thinlto-pre-link' -S %s 2>&1 \
+; RUN: -passes='thinlto-pre-link,name-anon-globals' -S %s 2>&1 \
 ; RUN: | FileCheck %s --check-prefixes=CHECK-O,CHECK-Oz,CHECK-PRELINK-O,CHECK-PRELINK-Oz
 ;
 ; Postlink pipelines:
@@ -154,7 +154,6 @@
 ; CHECK-O-NEXT: Finished CGSCC pass manager run.
 ; CHECK-O-NEXT: Finished llvm::Module pass manager run.
 ; CHECK-PRELINK-O-NEXT: Running pass: GlobalOptPass
-; CHECK-PRELINK-O-NEXT: Running pass: NameAnonGlobalPass
 ; CHECK-POSTLINK-O-NEXT: Running pass: PassManager<{{.*}}Module{{.*}}>
 ; CHECK-POSTLINK-O-NEXT: Starting llvm::Module pass manager run.
 ; CHECK-POSTLINK-O-NEXT: Running pass: GlobalOptPass
@@ -188,6 +187,7 @@
 ; CHECK-POSTLINK-O-NEXT: Running pass: ConstantMergePass
 ; CHECK-POSTLINK-O-NEXT: Finished llvm::Module pass manager run.
 ; CHECK-O-NEXT: Finished llvm::Module pass manager run.
+; CHECK-PRELINK-O-NEXT: Running pass: NameAnonGlobalPass
 ; CHECK-O-NEXT: Running pass: PrintModulePass
 
 ; Make sure we get the IR back out without changes when we print the module.
Index: llvm/lib/Passes/PassBuilder.cpp
===
--- llvm/lib/Passes/PassBuilder.cpp
+++ llvm/lib/Passes/PassBuilder.cpp
@@ -751,9 +751,6 @@
   // Reduce the size of the IR as much as possible.
   MPM.addPass(GlobalOptPass());
 
-  // Rename anon globals to be able to export them in the summary.
-  MPM.addPass(NameAnonGlobalPass());
-
   return MPM;
 }
 
Index: llvm/lib/Option/ArgList.cpp
===
--- llvm/lib/Option/ArgList.cpp
+++ llvm/lib/Option/ArgList.cpp
@@ -95,21 +95,6 @@
   return std::vector(Values.begin(), Values.end());
 }
 
-void ArgList::AddLastArg(ArgStringList , OptSpecifier Id) const {
-  if (Arg *A = getLastArg(Id)) {
-A->claim();
-A->render(*this, Output);
-  }
-}
-
-void ArgList::AddLastArg(ArgStringList , OptSpecifier Id0,
- OptSpecifier Id1) const {
-  if (Arg *A = getLastArg(Id0, Id1)) {
-A->claim();
-A->render(*this, Output);
-  }
-}
-
 void ArgList::AddAllArgsExcept(ArgStringList ,
ArrayRef Ids,
ArrayRef ExcludeIds) const {
Index: llvm/include/llvm/Option/ArgList.h
===
--- llvm/include/llvm/Option/ArgList.h
+++ llvm/include/llvm/Option/ArgList.h
@@ -306,9 +306,13 @@
bool Default = true) const;
 
   /// AddLastArg - Render only the last argument match \p Id0, if present.
-  void AddLastArg(ArgStringList , OptSpecifier Id0) const;
-  void AddLastArg(ArgStringList , OptSpecifier Id0,
-  OptSpecifier Id1) const;
+  template 
+  void AddLastArg(ArgStringList , OptSpecifiers ...Ids) 

[PATCH] D34784: [OpenMP] Add flag for specifying the target device architecture for OpenMP device offloading

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea updated this revision to Diff 104532.

Repository:
  rL LLVM

https://reviews.llvm.org/D34784

Files:
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/Cuda.cpp
  test/Driver/openmp-offload.c


Index: test/Driver/openmp-offload.c
===
--- test/Driver/openmp-offload.c
+++ test/Driver/openmp-offload.c
@@ -599,3 +599,11 @@
 // CHK-FOPENMP-IS-DEVICE: clang{{.*}}.i" {{.*}}" "-fopenmp-is-device"
 // CHK-FOPENMP-IS-DEVICE-NEXT: clang{{.*}}.bc" {{.*}}.i" "-fopenmp-is-device" 
"-fopenmp-host-ir-file-path"
 // CHK-FOPENMP-IS-DEVICE-NEXT: clang{{.*}}.s" {{.*}}.bc" "-fopenmp-is-device"
+
+/// ###
+
+/// Check -march propagates compute capability to device offloading toolchain.
+// RUN:   %clang -### -fopenmp=libomp 
-fopenmp-targets=powerpc64le-ibm-linux-gnu -save-temps -no-canonical-prefixes 
-fopenmp-target-arch=sm_35 %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-COMPUTE-CAPABILITY %s
+
+// CHK-COMPUTE-CAPABILITY: clang: warning: argument unused during compilation: 
'-fopenmp-target-arch=sm_35'
Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -212,8 +212,20 @@
   static_cast(getToolChain());
   assert(TC.getTriple().isNVPTX() && "Wrong platform");
 
+  StringRef GPUArchName;
+  std::vector GPUArchNames;
+  // If this is an OpenMP action we need to extract the device architecture 
from
+  // the -fopenmp-target-arch option.
+  if (JA.isDeviceOffloading(Action::OFK_OpenMP)) {
+GPUArchNames = Args.getAllArgValues(options::OPT_fopenmp_target_arch_EQ);
+assert(GPUArchNames.size() == 1 &&
+   "Exactly one GPU Arch required for ptxas.");
+GPUArchName = GPUArchNames[0];
+  } else
+GPUArchName = JA.getOffloadingArch();
+
   // Obtain architecture from the action.
-  CudaArch gpu_arch = StringToCudaArch(JA.getOffloadingArch());
+  CudaArch gpu_arch = StringToCudaArch(GPUArchName);
   assert(gpu_arch != CudaArch::UNKNOWN &&
  "Device action expected to have an architecture.");
 
@@ -342,7 +354,9 @@
 Action::OffloadKind DeviceOffloadingKind) const {
   HostTC.addClangTargetOptions(DriverArgs, CC1Args, DeviceOffloadingKind);
 
-  StringRef GpuArch = DriverArgs.getLastArgValue(options::OPT_march_EQ);
+  StringRef GpuArch = DriverArgs.getLastArgValue(
+  DeviceOffloadingKind == Action::OFK_OpenMP ?
+  options::OPT_fopenmp_target_arch_EQ : options::OPT_march_EQ);
   assert(!GpuArch.empty() && "Must have an explicit GPU arch.");
   assert((DeviceOffloadingKind == Action::OFK_OpenMP ||
   DeviceOffloadingKind == Action::OFK_Cuda) &&
@@ -405,7 +419,7 @@
 
   // For OpenMP device offloading, append derived arguments. Make sure
   // flags are not duplicated.
-  // TODO: Append the compute capability.
+  // Also append the compute capability.
   if (DeviceOffloadKind == Action::OFK_OpenMP) {
 for (Arg *A : Args){
   bool IsDuplicate = false;
@@ -418,6 +432,14 @@
   if (!IsDuplicate)
 DAL->append(A);
 }
+
+// Get the compute capability from the -fopenmp-target-arch flag.
+// The default compute capability is sm_20 since this is a CUDA
+// tool chain.
+if (Args.getAllArgValues(options::OPT_fopenmp_target_arch_EQ).empty())
+  DAL->AddJoinedArg(nullptr,
+  Opts.getOption(options::OPT_fopenmp_target_arch_EQ), "sm_20");
+
 return DAL;
   }
 
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1294,6 +1294,8 @@
   HelpText<"Specify comma-separated list of triples OpenMP offloading targets 
to be supported">;
 def fopenmp_dump_offload_linker_script : Flag<["-"], 
"fopenmp-dump-offload-linker-script">, Group, 
   Flags<[NoArgumentUnused]>;
+def fopenmp_target_arch_EQ : Joined<["-"], "fopenmp-target-arch=">, 
Flags<[DriverOption]>,
+  HelpText<"Pass a single target architecture (default for NVIDIA is sm_20) to 
be used by OpenMP device offloading.">;
 def fno_optimize_sibling_calls : Flag<["-"], "fno-optimize-sibling-calls">, 
Group;
 def foptimize_sibling_calls : Flag<["-"], "foptimize-sibling-calls">, 
Group;
 def force__cpusubtype__ALL : Flag<["-"], "force_cpusubtype_ALL">;


Index: test/Driver/openmp-offload.c
===
--- test/Driver/openmp-offload.c
+++ test/Driver/openmp-offload.c
@@ -599,3 +599,11 @@
 // CHK-FOPENMP-IS-DEVICE: clang{{.*}}.i" {{.*}}" "-fopenmp-is-device"
 // CHK-FOPENMP-IS-DEVICE-NEXT: clang{{.*}}.bc" {{.*}}.i" "-fopenmp-is-device" "-fopenmp-host-ir-file-path"
 // CHK-FOPENMP-IS-DEVICE-NEXT: clang{{.*}}.s" {{.*}}.bc" "-fopenmp-is-device"
+
+/// ###
+
+/// Check -march 

[PATCH] D34784: [OpenMP] Add flag for specifying the target device architecture for OpenMP device offloading

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea created this revision.

OpenMP has the ability to offload target regions to devices which may have 
different architectures.

A new -fopenmp-target-arch flag is introduced to specify the device 
architecture.

In this patch I use the new flag to specify the compute capability of the 
underlying NVIDIA architecture for the OpenMP offloading CUDA tool chain.

Only a host-offloading test is provided since full device offloading capability 
will only be available when D29654  lands.


Repository:
  rL LLVM

https://reviews.llvm.org/D34784

Files:
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/Cuda.cpp
  test/Driver/openmp-offload.c


Index: test/Driver/openmp-offload.c
===
--- test/Driver/openmp-offload.c
+++ test/Driver/openmp-offload.c
@@ -599,3 +599,11 @@
 // CHK-FOPENMP-IS-DEVICE: clang{{.*}}.i" {{.*}}" "-fopenmp-is-device"
 // CHK-FOPENMP-IS-DEVICE-NEXT: clang{{.*}}.bc" {{.*}}.i" "-fopenmp-is-device" 
"-fopenmp-host-ir-file-path"
 // CHK-FOPENMP-IS-DEVICE-NEXT: clang{{.*}}.s" {{.*}}.bc" "-fopenmp-is-device"
+
+/// ###
+
+/// Check -march propagates compute capability to device offloading toolchain.
+// RUN:   %clang -### -fopenmp=libomp 
-fopenmp-targets=powerpc64le-ibm-linux-gnu -save-temps -no-canonical-prefixes 
-fopenmp-target-arch=sm_35 %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-COMPUTE-CAPABILITY %s
+
+// CHK-COMPUTE-CAPABILITY: clang: warning: argument unused during compilation: 
'-fopenmp-target-arch=sm_35'
Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -212,8 +212,20 @@
   static_cast(getToolChain());
   assert(TC.getTriple().isNVPTX() && "Wrong platform");
 
+  StringRef GPUArchName;
+  std::vector GPUArchNames;
+  // If this is an OpenMP action we need to extract the device architecture 
from
+  // the -fopenmp-target-arch option.
+  if (JA.isDeviceOffloading(Action::OFK_OpenMP)) {
+GPUArchNames = Args.getAllArgValues(options::OPT_fopenmp_target_arch_EQ);
+assert(GPUArchNames.size() == 1 &&
+   "Exactly one GPU Arch required for ptxas.");
+GPUArchName = GPUArchNames[0];
+  } else
+GPUArchName = JA.getOffloadingArch();
+
   // Obtain architecture from the action.
-  CudaArch gpu_arch = StringToCudaArch(JA.getOffloadingArch());
+  CudaArch gpu_arch = StringToCudaArch(GPUArchName);
   assert(gpu_arch != CudaArch::UNKNOWN &&
  "Device action expected to have an architecture.");
 
@@ -342,7 +354,9 @@
 Action::OffloadKind DeviceOffloadingKind) const {
   HostTC.addClangTargetOptions(DriverArgs, CC1Args, DeviceOffloadingKind);
 
-  StringRef GpuArch = DriverArgs.getLastArgValue(options::OPT_march_EQ);
+  StringRef GpuArch = DriverArgs.getLastArgValue(
+  DeviceOffloadingKind == Action::OFK_OpenMP ?
+  options::OPT_fopenmp_target_arch_EQ : options::OPT_march_EQ);
   assert(!GpuArch.empty() && "Must have an explicit GPU arch.");
   assert((DeviceOffloadingKind == Action::OFK_OpenMP ||
   DeviceOffloadingKind == Action::OFK_Cuda) &&
@@ -364,7 +378,6 @@
   }
 
   std::string LibDeviceFile = CudaInstallation.getLibDeviceFile(GpuArch);
-
   if (LibDeviceFile.empty()) {
 getDriver().Diag(diag::err_drv_no_cuda_libdevice) << GpuArch;
 return;
@@ -405,7 +418,7 @@
 
   // For OpenMP device offloading, append derived arguments. Make sure
   // flags are not duplicated.
-  // TODO: Append the compute capability.
+  // Also append the compute capability.
   if (DeviceOffloadKind == Action::OFK_OpenMP) {
 for (Arg *A : Args){
   bool IsDuplicate = false;
@@ -418,6 +431,14 @@
   if (!IsDuplicate)
 DAL->append(A);
 }
+
+// Get the compute capability from the -fopenmp-target-arch flag.
+// The default compute capability is sm_20 since this is a CUDA
+// tool chain.
+if (Args.getAllArgValues(options::OPT_fopenmp_target_arch_EQ).empty())
+  DAL->AddJoinedArg(nullptr,
+  Opts.getOption(options::OPT_fopenmp_target_arch_EQ), "sm_20");
+
 return DAL;
   }
 
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1294,6 +1294,8 @@
   HelpText<"Specify comma-separated list of triples OpenMP offloading targets 
to be supported">;
 def fopenmp_dump_offload_linker_script : Flag<["-"], 
"fopenmp-dump-offload-linker-script">, Group, 
   Flags<[NoArgumentUnused]>;
+def fopenmp_target_arch_EQ : Joined<["-"], "fopenmp-target-arch=">, 
Flags<[DriverOption]>,
+  HelpText<"Pass a single target architecture (default for NVIDIA is sm_20) to 
be used by OpenMP device offloading.">;
 def fno_optimize_sibling_calls : Flag<["-"], "fno-optimize-sibling-calls">, 
Group;
 def 

[PATCH] D29647: [OpenMP] Extend CLANG target options with device offloading kind.

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea updated this revision to Diff 104527.
gtbercea added a comment.

Split previous diff into a "device offloading kind" patch (show here) and a 
**new** patch which relies on a new compiler flag.

A TODO has been added to signal that the compute capability is to be handled in 
the **new** patch mentioned above.


Repository:
  rL LLVM

https://reviews.llvm.org/D29647

Files:
  include/clang/Driver/ToolChain.h
  lib/Driver/ToolChain.cpp
  lib/Driver/ToolChains/BareMetal.cpp
  lib/Driver/ToolChains/BareMetal.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/Cuda.cpp
  lib/Driver/ToolChains/Cuda.h
  lib/Driver/ToolChains/Fuchsia.cpp
  lib/Driver/ToolChains/Fuchsia.h
  lib/Driver/ToolChains/Gnu.cpp
  lib/Driver/ToolChains/Gnu.h
  lib/Driver/ToolChains/Hexagon.cpp
  lib/Driver/ToolChains/Hexagon.h
  lib/Driver/ToolChains/WebAssembly.cpp
  lib/Driver/ToolChains/WebAssembly.h
  lib/Driver/ToolChains/XCore.cpp
  lib/Driver/ToolChains/XCore.h

Index: lib/Driver/ToolChains/XCore.h
===
--- lib/Driver/ToolChains/XCore.h
+++ lib/Driver/ToolChains/XCore.h
@@ -67,7 +67,8 @@
   AddClangSystemIncludeArgs(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ) const override;
   void addClangTargetOptions(const llvm::opt::ArgList ,
- llvm::opt::ArgStringList ) const override;
+ llvm::opt::ArgStringList ,
+ Action::OffloadKind DeviceOffloadKind) const override;
   void AddClangCXXStdlibIncludeArgs(
   const llvm::opt::ArgList ,
   llvm::opt::ArgStringList ) const override;
Index: lib/Driver/ToolChains/XCore.cpp
===
--- lib/Driver/ToolChains/XCore.cpp
+++ lib/Driver/ToolChains/XCore.cpp
@@ -124,7 +124,8 @@
 }
 
 void XCoreToolChain::addClangTargetOptions(const ArgList ,
-   ArgStringList ) const {
+   ArgStringList ,
+   Action::OffloadKind) const {
   CC1Args.push_back("-nostdsysteminc");
 }
 
Index: lib/Driver/ToolChains/WebAssembly.h
===
--- lib/Driver/ToolChains/WebAssembly.h
+++ lib/Driver/ToolChains/WebAssembly.h
@@ -53,7 +53,8 @@
   bool SupportsProfiling() const override;
   bool HasNativeLLVMSupport() const override;
   void addClangTargetOptions(const llvm::opt::ArgList ,
- llvm::opt::ArgStringList ) const override;
+ llvm::opt::ArgStringList ,
+ Action::OffloadKind DeviceOffloadKind) const override;
   RuntimeLibType GetDefaultRuntimeLibType() const override;
   CXXStdlibType GetCXXStdlibType(const llvm::opt::ArgList ) const override;
   void AddClangSystemIncludeArgs(
Index: lib/Driver/ToolChains/WebAssembly.cpp
===
--- lib/Driver/ToolChains/WebAssembly.cpp
+++ lib/Driver/ToolChains/WebAssembly.cpp
@@ -134,7 +134,8 @@
 bool WebAssembly::HasNativeLLVMSupport() const { return true; }
 
 void WebAssembly::addClangTargetOptions(const ArgList ,
-ArgStringList ) const {
+ArgStringList ,
+Action::OffloadKind) const {
   if (DriverArgs.hasFlag(clang::driver::options::OPT_fuse_init_array,
  options::OPT_fno_use_init_array, true))
 CC1Args.push_back("-fuse-init-array");
Index: lib/Driver/ToolChains/Hexagon.h
===
--- lib/Driver/ToolChains/Hexagon.h
+++ lib/Driver/ToolChains/Hexagon.h
@@ -69,7 +69,8 @@
   ~HexagonToolChain() override;
 
   void addClangTargetOptions(const llvm::opt::ArgList ,
- llvm::opt::ArgStringList ) const override;
+ llvm::opt::ArgStringList ,
+ Action::OffloadKind DeviceOffloadKind) const override;
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ) const override;
Index: lib/Driver/ToolChains/Hexagon.cpp
===
--- lib/Driver/ToolChains/Hexagon.cpp
+++ lib/Driver/ToolChains/Hexagon.cpp
@@ -428,7 +428,8 @@
 }
 
 void HexagonToolChain::addClangTargetOptions(const ArgList ,
- ArgStringList ) const {
+ ArgStringList ,
+ Action::OffloadKind) const {
   if (DriverArgs.hasArg(options::OPT_ffp_contract))
 return;
   unsigned OptLevel = getOptimizationLevel(DriverArgs);
Index: lib/Driver/ToolChains/Gnu.h
===
--- 

[PATCH] D34771: [clang-tidy] follow-up on misc-definitions-in-header check.

[Alexander Kornienko via Phabricator via cfe-commits]

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

LG


https://reviews.llvm.org/D34771



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

Re: r306583 - [ASTReader] Treat multiple defns of ObjC protocols the same as interfaces.

[Bruno Cardoso Lopes via cfe-commits]

Hi Graydon,

Can you please add a testcase for this?

Thanks,

On Wed, Jun 28, 2017 at 11:36 AM, Graydon Hoare via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: graydon
> Date: Wed Jun 28 11:36:27 2017
> New Revision: 306583
>
> URL: http://llvm.org/viewvc/llvm-project?rev=306583=rev
> Log:
> [ASTReader] Treat multiple defns of ObjC protocols the same as interfaces.
>
> Summary:
> In change 2ba19793512, the ASTReader logic for ObjC interfaces was
> modified to
> preserve the first definition-data read, "merging" later definitions into
> it
> rather than overwriting it (though this "merging" is, in practice, a no-op
> that
> discards the later definition-data).
>
> Unfortunately this change was only made to ObjC interfaces, not protocols;
> this
> means that when (for example) loading a protocol that references an
> interface,
> if both the protocol and interface are multiply defined (as can easily
> happen
> if the same header is read from multiple contexts), an _inconsistent_ pair
> of
> definitions is loaded: first-read for the interface and last-read for the
> protocol.
>
> This in turn causes very subtle downstream bugs in the Swift ClangImporter,
> which filters the results of name lookups based on the owning module of a
> definition; inconsistency between a pair of related definitions causes name
> lookup failures at various stages of compilation.
>
> To fix these downstream issues, this change replicates the logic applied to
> interfaces in change 2ba19793512, but for ObjC protocols.
>
> rdar://30851899
>
> Reviewers: doug.gregor, rsmith
>
> Reviewed By: doug.gregor
>
> Subscribers: jordan_rose, cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D34741
>
> Modified:
> cfe/trunk/include/clang/AST/Redeclarable.h
> cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
>
> Modified: cfe/trunk/include/clang/AST/Redeclarable.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/AST/Redeclarable.h?rev=306583=306582=306583=diff
> 
> ==
> --- cfe/trunk/include/clang/AST/Redeclarable.h (original)
> +++ cfe/trunk/include/clang/AST/Redeclarable.h Wed Jun 28 11:36:27 2017
> @@ -21,6 +21,60 @@
>  namespace clang {
>  class ASTContext;
>
> +// Some notes on redeclarables:
> +//
> +//  - Every redeclarable is on a circular linked list.
> +//
> +//  - Every decl has a pointer to the first element of the chain _and_ a
> +//DeclLink that may point to one of 3 possible states:
> +//  - the "previous" (temporal) element in the chain
> +//  - the "latest" (temporal) element in the chain
> +//  - the an "uninitialized-latest" value (when newly-constructed)
> +//
> +//  - The first element is also often called the canonical element. Every
> +//element has a pointer to it so that "getCanonical" can be fast.
> +//
> +//  - Most links in the chain point to previous, except the link out of
> +//the first; it points to latest.
> +//
> +//  - Elements are called "first", "previous", "latest" or
> +//"most-recent" when referring to temporal order: order of addition
> +//to the chain.
> +//
> +//  - To make matters confusing, the DeclLink type uses the term "next"
> +//for its pointer-storage internally (thus functions like
> +//NextIsPrevious). It's easiest to just ignore the implementation of
> +//DeclLink when making sense of the redeclaration chain.
> +//
> +//  - There's also a "definition" link for several types of
> +//redeclarable, where only one definition should exist at any given
> +//time (and the defn pointer is stored in the decl's "data" which
> +//is copied to every element on the chain when it's changed).
> +//
> +//Here is some ASCII art:
> +//
> +//  "first" "latest"
> +//  "canonical" "most recent"
> +//  ++ first+--+
> +//  || <--- |  |
> +//  ||  |  |
> +//  ||  |  |
> +//  ||   +--+   |  |
> +//  || first |  |   |  |
> +//  || < |  |   |  |
> +//  ||   |  |   |  |
> +//  | @class A   |  link | @interface A |  link | @class A |
> +//  | seen first | < | seen second  | < | seen third   |
> +//  ||   |  |   |  |
> +//  ++   +--+   +--+
> +//  | data   | defn  | data |  defn | data |
> +//  || > |  | < |  |
> +//  ++   +--+   +--+

[PATCH] D34779: Fix floating point promotions for overload purposes

[Tim Northover via Phabricator via cfe-commits]

t.p.northover created this revision.
Herald added a subscriber: mcrosier.

This has a couple of changes to how Clang determines whether a floating-point 
promotion has occurred for C++ (and `__attribute__((overloadable))`) purposes.

First, I think the special rules for C are based on a misconception. C99 
referred to promotions to "long double" but it actually meant conversions, and 
the only real promotions that happened were the "float -> double" default 
argument promotion for varargs (and obviously nothing to do with overload 
resolution). C11 fixed this wording to remove all mention of promotions. So 
this patch makes all overload resolution follow C++ rules, which seems like the 
least surprising behaviour for __attribute__((overloadable)).

Second, this allows "half -> double" promotion unconditionally. Since half is a 
non-standard type there's a bit of guesswork here, but it seems like the most 
consistent extension of C++'s wording to that type (with special mention going 
to allowing "half -> float" too). It definitely seems reasonable that the 
decision should not depend on NativeHalfTypes.

Does the change look reasonable to others?


Repository:
  rL LLVM

https://reviews.llvm.org/D34779

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CodeGenCXX/fp16-overload.cpp
  clang/test/Sema/overloadable-complex.c
  clang/test/SemaCXX/floating-point-promotion.cpp

Index: clang/test/SemaCXX/floating-point-promotion.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/floating-point-promotion.cpp
@@ -0,0 +1,110 @@
+// RUN: %clang_cc1 -fsyntax-only -fallow-half-arguments-and-returns -verify %s
+// RUN: %clang_cc1 -fsyntax-only -fallow-half-arguments-and-returns -fnative-half-type -verify %s
+
+#include 
+
+typedef __fp16 half;
+
+// expected-n...@floating-point-promotion.cpp:* 1+ {{candidate function}}
+
+extern void half_to_int_or_float(int x);
+extern void half_to_int_or_float(float x);
+void test1(half h) {
+  half_to_int_or_float(h);  // expected-error {{call to 'half_to_int_or_float' is ambiguous}}
+}
+
+extern void half_to_int_or_double(int x);
+extern void half_to_int_or_double(double x);
+void test2(half h) {
+  half_to_int_or_double(h);
+}
+
+extern void half_to_int_or_long_double(int x);
+extern void half_to_int_or_long_double(long double x);
+void test3(half h) {
+  half_to_int_or_long_double(h);  // expected-error {{call to 'half_to_int_or_long_double' is ambiguous}}
+}
+
+extern void half_to_float_or_double(float x);
+extern void half_to_float_or_double(double x);
+void test4(half h) {
+  half_to_float_or_double(h);
+}
+
+extern void half_to_float_or_long_double(float x);
+extern void half_to_float_or_long_double(long double x);
+void test5(half h) {
+  half_to_float_or_long_double(h);  // expected-error {{call to 'half_to_float_or_long_double' is ambiguous}}
+}
+
+extern void float_to_int_or_half(int x);
+extern void float_to_int_or_half(half x);
+void test6(float f) {
+  float_to_int_or_half(f);  // expected-error {{call to 'float_to_int_or_half' is ambiguous}}
+}
+
+extern void float_to_int_or_double(int x);
+extern void float_to_int_or_double(double x);
+void test7(float f) {
+  float_to_int_or_double(f);
+}
+
+extern void float_to_int_or_long_double(int x);
+extern void float_to_int_or_long_double(long double x);
+void test8(float f) {
+  float_to_int_or_long_double(f);  // expected-error {{call to 'float_to_int_or_long_double' is ambiguous}}
+}
+
+extern void float_to_half_or_double(half x);
+extern void float_to_half_or_double(double x);
+void test9(float f) {
+  float_to_half_or_double(f);
+}
+
+extern void float_to_half_or_long_double(half x);
+extern void float_to_half_or_long_double(long double x);
+void test10(float f) {
+  float_to_half_or_long_double(f);  // expected-error {{call to 'float_to_half_or_long_double' is ambiguous}}
+}
+
+extern void double_to_int_or_half(int x);
+extern void double_to_int_or_half(half x);
+void test11(double d) {
+  double_to_int_or_half(d);  // expected-error {{call to 'double_to_int_or_half' is ambiguous}}
+}
+
+extern void double_to_int_or_float(int x);
+extern void double_to_int_or_float(float x);
+void test12(double d) {
+  double_to_int_or_float(d);  // expected-error {{call to 'double_to_int_or_float' is ambiguous}}
+}
+
+extern void double_to_int_or_long_double(int x);
+extern void double_to_int_or_long_double(long double x);
+void test13(double d) {
+  double_to_int_or_long_double(d);  // expected-error {{call to 'double_to_int_or_long_double' is ambiguous}}
+}
+
+extern void double_to_half_or_float(half x);
+extern void double_to_half_or_float(float x);
+void test14(double d) {
+  double_to_half_or_float(d);  // expected-error {{call to 'double_to_half_or_float' is ambiguous}}
+}
+
+extern void double_to_half_or_long_double(half x);
+extern void double_to_half_or_long_double(long double x);
+void test15(double d) {
+  double_to_half_or_long_double(d);  // expected-error {{call 

[PATCH] D34777: CodeGen: Fix invalid bitcast for coerced function argument

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
Herald added a subscriber: Anastasia.

Clang assumes coerced function argument is in address space 0, which is not 
always true and results in invalid bitcasts.

This patch fixes failure in OpenCL conformance test api/get_kernel_arg_info 
with amdgcn---amdgizcl triple, where non-zero alloca address space is used.


https://reviews.llvm.org/D34777

Files:
  lib/CodeGen/CGCall.cpp
  test/CodeGenOpenCL/addr-space-struct-arg.cl


Index: test/CodeGenOpenCL/addr-space-struct-arg.cl
===
--- test/CodeGenOpenCL/addr-space-struct-arg.cl
+++ test/CodeGenOpenCL/addr-space-struct-arg.cl
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 %s -emit-llvm -o - -O0 -ffake-address-space-map -triple 
i686-pc-darwin | FileCheck %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -O0 -finclude-default-header 
-ffake-address-space-map -triple i686-pc-darwin | FileCheck 
-check-prefixes=COM,X86 %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -O0 -finclude-default-header -triple 
amdgcn-amdhsa-amd-amdgizcl | FileCheck -check-prefixes=COM,AMD %s
 
 typedef struct {
   int cells[9];
@@ -8,16 +9,57 @@
   int cells[16];
 } Mat4X4;
 
+struct StructOneMember {
+  int2 x;
+};
+
+struct StructTwoMember {
+  int2 x;
+  int2 y;
+};
+
+// COM-LABEL: define void @foo
 Mat4X4 __attribute__((noinline)) foo(Mat3X3 in) {
   Mat4X4 out;
   return out;
 }
 
+// COM-LABEL: define {{.*}} void @ker
+// Expect two mem copies: one for the argument "in", and one for
+// the return value.
+// X86: call void @llvm.memcpy.p0i8.p1i8.i32(i8*
+// X86: call void @llvm.memcpy.p1i8.p0i8.i32(i8 addrspace(1)*
+// AMD: call void @llvm.memcpy.p5i8.p1i8.i64(i8 addrspace(5)*
+// AMD: call void @llvm.memcpy.p1i8.p5i8.i64(i8 addrspace(1)*
 kernel void ker(global Mat3X3 *in, global Mat4X4 *out) {
   out[0] = foo(in[1]);
 }
 
-// Expect two mem copies: one for the argument "in", and one for
-// the return value.
-// CHECK: call void @llvm.memcpy.p0i8.p1i8.i32(i8*
-// CHECK: call void @llvm.memcpy.p1i8.p0i8.i32(i8 addrspace(1)*
+// AMD-LABEL: define void @FuncOneMember(%struct.StructOneMember addrspace(5)* 
byval align 8 %u)
+void FuncOneMember(struct StructOneMember u) {
+  u.x = (int2)(0, 0);
+}
+
+// AMD-LABEL: define amdgpu_kernel void @KernelOneMember
+// AMD-SAME:  (<2 x i32> %[[u_coerce:.*]])
+// AMD:  %[[u:.*]] = alloca %struct.StructOneMember, align 8, addrspace(5)
+// AMD:  %[[coerce_dive:.*]] = getelementptr inbounds %struct.StructOneMember, 
%struct.StructOneMember addrspace(5)* %[[u]], i32 0, i32 0
+// AMD:  store <2 x i32> %[[u_coerce]], <2 x i32> addrspace(5)* 
%[[coerce_dive]]
+// AMD:  call void @FuncOneMember(%struct.StructOneMember addrspace(5)* byval 
align 8 %[[u]])
+kernel void KernelOneMember(struct StructOneMember u) {
+  FuncOneMember(u);
+}
+
+// AMD-LABEL: define void @FuncTwoMember(%struct.StructTwoMember addrspace(5)* 
byval align 8 %u)
+void FuncTwoMember(struct StructTwoMember u) {
+  u.x = (int2)(0, 0);
+}
+
+// AMD-LABEL: define amdgpu_kernel void @KernelTwoMember
+// AMD-SAME:  (%struct.StructTwoMember %[[u_coerce:.*]])
+// AMD:  %[[u:.*]] = alloca %struct.StructTwoMember, align 8, addrspace(5)
+// AMD:  store %struct.StructTwoMember %[[u_coerce]], %struct.StructTwoMember 
addrspace(5)* %[[u]]
+// AMD:  call void @FuncTwoMember(%struct.StructTwoMember addrspace(5)* byval 
align 8 %[[u]])
+kernel void KernelTwoMember(struct StructTwoMember u) {
+  FuncTwoMember(u);
+}
Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -1297,7 +1297,7 @@
 
   // If store is legal, just bitcast the src pointer.
   if (SrcSize <= DstSize) {
-Dst = CGF.Builder.CreateBitCast(Dst, llvm::PointerType::getUnqual(SrcTy));
+Dst = CGF.Builder.CreateElementBitCast(Dst, SrcTy);
 BuildAggStore(CGF, Src, Dst, DstIsVolatile);
   } else {
 // Otherwise do coercion through memory. This is stupid, but
@@ -2412,8 +2412,7 @@
 
 Address AddrToStoreInto = Address::invalid();
 if (SrcSize <= DstSize) {
-  AddrToStoreInto =
-Builder.CreateBitCast(Ptr, llvm::PointerType::getUnqual(STy));
+  AddrToStoreInto = Builder.CreateElementBitCast(Ptr, STy);
 } else {
   AddrToStoreInto =
 CreateTempAlloca(STy, Alloca.getAlignment(), "coerce");


Index: test/CodeGenOpenCL/addr-space-struct-arg.cl
===
--- test/CodeGenOpenCL/addr-space-struct-arg.cl
+++ test/CodeGenOpenCL/addr-space-struct-arg.cl
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 %s -emit-llvm -o - -O0 -ffake-address-space-map -triple i686-pc-darwin | FileCheck %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -O0 -finclude-default-header -ffake-address-space-map -triple i686-pc-darwin | FileCheck -check-prefixes=COM,X86 %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -O0 -finclude-default-header -triple amdgcn-amdhsa-amd-amdgizcl | FileCheck 

[clang-tools-extra] r306598 - [clangd] Cleanup ClangdUnit.cpp, update docs; NFC

[Kirill Bobyrev via cfe-commits]

Author: omtcyfz
Date: Wed Jun 28 13:57:28 2017
New Revision: 306598

URL: http://llvm.org/viewvc/llvm-project?rev=306598=rev
Log:
[clangd] Cleanup ClangdUnit.cpp, update docs; NFC

* Enforce 80 characters limit where appropriate
* Use slightly more descriptive names for searched locations
* Update docs to reflect D34269, which adds "Go To Declaration" functionality

Modified:
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/docs/clangd.rst
clang-tools-extra/trunk/test/CMakeLists.txt

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=306598=306597=306598=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Wed Jun 28 13:57:28 2017
@@ -36,7 +36,8 @@ ClangdUnit::ClangdUnit(PathRef FileName,
 
   // Inject the resource dir.
   // FIXME: Don't overwrite it if it's already there.
-  Commands.front().CommandLine.push_back("-resource-dir=" + 
std::string(ResourceDir));
+  Commands.front().CommandLine.push_back("-resource-dir=" +
+ std::string(ResourceDir));
 
   IntrusiveRefCntPtr Diags =
   CompilerInstance::createDiagnostics(new DiagnosticOptions);
@@ -277,14 +278,14 @@ class DeclarationLocationsFinder : publi
 public:
   DeclarationLocationsFinder(raw_ostream ,
   const SourceLocation , ASTUnit ) :
-  SearchedLocation(SearchedLocation), Unit(Unit) {
-  }
+  SearchedLocation(SearchedLocation), Unit(Unit) {}
 
   std::vector takeLocations() {
 // Don't keep the same location multiple times.
 // This can happen when nodes in the AST are visited twice.
 std::sort(DeclarationLocations.begin(), DeclarationLocations.end());
-auto last = std::unique(DeclarationLocations.begin(), 
DeclarationLocations.end());
+auto last =
+std::unique(DeclarationLocations.begin(), DeclarationLocations.end());
 DeclarationLocations.erase(last, DeclarationLocations.end());
 return std::move(DeclarationLocations);
   }
@@ -312,13 +313,13 @@ private:
 SourceLocation LocStart = ValSourceRange.getBegin();
 SourceLocation LocEnd = Lexer::getLocForEndOfToken(ValSourceRange.getEnd(),
 0, SourceMgr, LangOpts);
-Position P1;
-P1.line = SourceMgr.getSpellingLineNumber(LocStart) - 1;
-P1.character = SourceMgr.getSpellingColumnNumber(LocStart) - 1;
-Position P2;
-P2.line = SourceMgr.getSpellingLineNumber(LocEnd) - 1;
-P2.character = SourceMgr.getSpellingColumnNumber(LocEnd) - 1;
-Range R = { P1, P2 };
+Position Begin;
+Begin.line = SourceMgr.getSpellingLineNumber(LocStart) - 1;
+Begin.character = SourceMgr.getSpellingColumnNumber(LocStart) - 1;
+Position End;
+End.line = SourceMgr.getSpellingLineNumber(LocEnd) - 1;
+End.character = SourceMgr.getSpellingColumnNumber(LocEnd) - 1;
+Range R = {Begin, End};
 Location L;
 L.uri = URI::fromFile(
 SourceMgr.getFilename(SourceMgr.getSpellingLoc(LocStart)));
@@ -326,8 +327,7 @@ private:
 DeclarationLocations.push_back(L);
   }
 
-  void finish() override
-  {
+  void finish() override {
 // Also handle possible macro at the searched location.
 Token Result;
 if (!Lexer::getRawToken(SearchedLocation, Result, Unit.getSourceManager(),
@@ -366,8 +366,8 @@ std::vector ClangdUnit::findDe
 
   SourceLocation SourceLocationBeg = getBeginningOfIdentifier(Pos, FE);
 
-  auto DeclLocationsFinder = 
std::make_shared(llvm::errs(),
-  SourceLocationBeg, *Unit);
+  auto DeclLocationsFinder = std::make_shared(
+  llvm::errs(), SourceLocationBeg, *Unit);
   index::IndexingOptions IndexOpts;
   IndexOpts.SystemSymbolFilter =
   index::IndexingOptions::SystemSymbolFilterKind::All;

Modified: clang-tools-extra/trunk/docs/clangd.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clangd.rst?rev=306598=306597=306598=diff
==
--- clang-tools-extra/trunk/docs/clangd.rst (original)
+++ clang-tools-extra/trunk/docs/clangd.rst Wed Jun 28 13:57:28 2017
@@ -54,7 +54,7 @@ features might be eventually developed o
 +-++--+ 
 | Fix-its | Yes|   Yes|
 +-++--+
-| Go to Definition| Yes|   No |
+| Go to Definition| Yes|   Yes|
 +-++--+
 | Source hover| Yes|   No |
 +-++--+
@@ -103,4 +103,4 @@ look at the `LLVM Developer Policy
 to the `Language Server Protocol
 `_ 

[PATCH] D34449: [clang-tidy] Enable inline variable definitions in headers.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: test/clang-tidy/misc-definitions-in-headers.hpp:180
+class CE {
+  constexpr static int i = 5; // OK: constexpr definition.
+};

hokein wrote:
> aaron.ballman wrote:
> > xazax.hun wrote:
> > > hokein wrote:
> > > > hokein wrote:
> > > > > aaron.ballman wrote:
> > > > > > This is not as safe as you might think. As-is, this is fine, 
> > > > > > however, if the class is given an inline function where that 
> > > > > > variable is odr-used, you will get an ODR violation.
> > > > > > 
> > > > > > I think it's mildly better to err on the side of safety here and 
> > > > > > diagnose.
> > > > > I think the current code (Line `97` in 
> > > > > `DefinitionsInHeadersCheck.cpp`) has already guaranteed this case. 
> > > > > Can you try to run it without your change in the 
> > > > > `DefinitionsInHeadersCheck.cpp`?
> > > > > 
> > > > > I think it still makes sense to add `constexpr` test cases.
> > > > > 
> > > > >   
> > > > In C++17, `constexpr static int i` is an inline variable, which is fine 
> > > > to define in C++ header -- because `inline` specifier provides a 
> > > > facility allowing definitions (functions/variables) in header that is 
> > > > included in multiple TUs. Additionally, one of the `inline variable` 
> > > > motivations is to support the development of header-only libraries, you 
> > > > can find discussions in 
> > > > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4424.pdf.
> > > > 
> > > > Therefore, I'm +1 ignore the inline variables (the same as inline 
> > > > functions). 
> > > Unfortunately, the test will fail without this modification, but I 
> > > modified it to ignore inline variables, which is a way better approach 
> > > indeed. 
> > The paper you cited was a feature not a defect, so prior to the paper's 
> > adoption in C++17, the behavior is that the constexpr variable may trigger 
> > an ODR violation, which is why I was saying this should be diagnosed rather 
> > than ignored. There was real-world motivation for that paper.
> > prior to the paper's adoption in C++17, the behavior is that the constexpr 
> > variable may trigger an ODR violation
> 
> Yeah, I did remember our discussion when I implemented this check 
> (https://stackoverflow.com/questions/23652156/how-would-use-of-unnamed-namespaces-in-headers-cause-odr-violations).
>  We allow internal linkage variables (static/const/conexpr) in the check -- 
> because we want to keep a small number of warnings as const variable 
> definitions are widely used in headers.
> 
> Maybe add an option to enable this particular case?
> Maybe add an option to enable this particular case?

An option would make sense to me. Perhaps the default value of the option can 
even be set depending on the language standard used to run the compilation?


Repository:
  rL LLVM

https://reviews.llvm.org/D34449



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

[PATCH] D34770: [Bash-autocompletion] Auto complete cc1 options if -cc1 is specified

[Yuka Takahashi via Phabricator via cfe-commits]

yamaguchi updated this revision to Diff 104490.
yamaguchi added a comment.

Update patch.


https://reviews.llvm.org/D34770

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/autocomplete.c
  clang/utils/bash-autocomplete.sh
  llvm/include/llvm/Option/OptTable.h
  llvm/lib/Option/OptTable.cpp

Index: llvm/lib/Option/OptTable.cpp
===
--- llvm/lib/Option/OptTable.cpp
+++ llvm/lib/Option/OptTable.cpp
@@ -225,11 +225,15 @@
   return {};
 }
 
-std::vector OptTable::findByPrefix(StringRef Cur) const {
+std::vector
+OptTable::findByPrefix(StringRef Cur, unsigned short DisableFlags) const {
   std::vector Ret;
   for (const Info  : OptionInfos.slice(FirstSearchableIndex)) {
 if (!In.Prefixes)
   continue;
+if (In.Flags & DisableFlags)
+  continue;
+
 for (int I = 0; In.Prefixes[I]; I++) {
   std::string S = std::string(In.Prefixes[I]) + std::string(In.Name);
   if (StringRef(S).startswith(Cur))
Index: llvm/include/llvm/Option/OptTable.h
===
--- llvm/include/llvm/Option/OptTable.h
+++ llvm/include/llvm/Option/OptTable.h
@@ -140,7 +140,8 @@
   //  to start with.
   ///
   /// \return The vector of flags which start with Cur.
-  std::vector findByPrefix(StringRef Cur) const;
+  std::vector findByPrefix(StringRef Cur,
+unsigned short DisableFlags) const;
 
   /// \brief Parse a single argument; returning the new argument and
   /// updating Index.
Index: clang/utils/bash-autocomplete.sh
===
--- clang/utils/bash-autocomplete.sh
+++ clang/utils/bash-autocomplete.sh
@@ -10,18 +10,23 @@
   # So, we need to partially undo bash tokenization here for integrity.
   local w1="${COMP_WORDS[$cword - 1]}"
   local w2="${COMP_WORDS[$cword - 2]}"
+  # Clang want to know if -cc1 option is specified or not, because we don't want to show
+  # cc1 options otherwise.
+  if [[ "${COMP_WORDS[1]}" == "-cc1" ]]; then
+arg="#"
+  fi
   if [[ "$cur" == -* ]]; then
 # -foo
-arg="$cur"
+arg="$arg$cur"
   elif [[ "$w1" == -*  && "$cur" == '=' ]]; then
 # -foo=
-arg="$w1=,"
+arg="$arg$w1=,"
   elif [[ "$w1" == -* ]]; then
 # -foo  or -foo bar
-arg="$w1,$cur"
+arg="$arg$w1,$cur"
   elif [[ "$w2" == -* && "$w1" == '=' ]]; then
 # -foo=bar
-arg="$w2=,$cur"
+arg="$arg$w2=,$cur"
   fi
 
   flags=$( "${COMP_WORDS[0]}" --autocomplete="$arg" 2>/dev/null )
Index: clang/test/Driver/autocomplete.c
===
--- clang/test/Driver/autocomplete.c
+++ clang/test/Driver/autocomplete.c
@@ -36,3 +36,7 @@
 // MTHREADMODELALL: posix single
 // RUN: %clang --autocomplete=-mrelocation-model, | FileCheck %s -check-prefix=MRELOCMODELALL
 // MRELOCMODELALL: dynamic-no-pic pic ropi ropi-rwpi rwpi static
+// RUN: %clang --autocomplete=-mrelocation-mode | FileCheck %s -check-prefix=MRELOCMODEL_CLANG
+// MRELOCMODEL_CLANG-NOT: -mrelocation-model
+// RUN: %clang --autocomplete=#-mrelocation-mode | FileCheck %s -check-prefix=MRELOCMODEL_CC1
+// MRELOCMODEL_CC1: -mrelocation-model
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1231,11 +1231,20 @@
 StringRef PassedFlags = A->getValue();
 std::vector SuggestedCompletions;
 
+unsigned short DisableFlags = options::NoDriverOption | options::Unsupported | options::Ignored;
+// We want to show cc1-only options only when clang is invoked as "clang -cc1".
+// When clang is invoked as "clang -cc1", we add "#" to ther beginning of an --autocomplete
+// option so that the clang driver can distinguish whether it is requested to show cc1-only options or not.
+if (PassedFlags[0] == '#') {
+  DisableFlags &= ~options::NoDriverOption;
+  PassedFlags = PassedFlags.substr(1);
+}
+
 if (PassedFlags.find(',') == StringRef::npos) {
   // If the flag is in the form of "--autocomplete=-foo",
   // we were requested to print out all option names that start with "-foo".
   // For example, "--autocomplete=-fsyn" is expanded to "-fsyntax-only".
-  SuggestedCompletions = Opts->findByPrefix(PassedFlags);
+  SuggestedCompletions = Opts->findByPrefix(PassedFlags, DisableFlags);
 } else {
   // If the flag is in the form of "--autocomplete=foo,bar", we were
   // requested to print out all option values for "-foo" that start with
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34771: [clang-tidy] follow-up on misc-definitions-in-header check.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
Herald added subscribers: xazax.hun, JDevlieghere.

- add `-std=c++11` to `.hpp` file by default.
- add constexpr function to test and doc.


https://reviews.llvm.org/D34771

Files:
  docs/clang-tidy/checks/misc-definitions-in-headers.rst
  test/clang-tidy/check_clang_tidy.py
  test/clang-tidy/misc-definitions-in-headers.hpp


Index: test/clang-tidy/misc-definitions-in-headers.hpp
===
--- test/clang-tidy/misc-definitions-in-headers.hpp
+++ test/clang-tidy/misc-definitions-in-headers.hpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s misc-definitions-in-headers %t -- -- -std=c++11
+// RUN: %check_clang_tidy %s misc-definitions-in-headers %t
 
 int f() {
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f' defined in a header 
file; function definitions in header files can lead to ODR violations 
[misc-definitions-in-headers]
@@ -177,3 +177,5 @@
 }
 
 constexpr int k = 1; // OK: constexpr variable has internal linkage.
+
+constexpr int f10() { return 0; } // OK: constexpr function definition.
Index: test/clang-tidy/check_clang_tidy.py
===
--- test/clang-tidy/check_clang_tidy.py
+++ test/clang-tidy/check_clang_tidy.py
@@ -58,8 +58,8 @@
 
   clang_tidy_extra_args = extra_args
   if len(clang_tidy_extra_args) == 0:
-clang_tidy_extra_args = ['--', '--std=c++11'] if extension == '.cpp' \
-   else ['--']
+clang_tidy_extra_args = ['--', '--std=c++11'] \
+if extension == '.cpp' or extension == '.hpp' else ['--']
 
   # Tests should not rely on STL being available, and instead provide mock
   # implementations of relevant APIs.
Index: docs/clang-tidy/checks/misc-definitions-in-headers.rst
===
--- docs/clang-tidy/checks/misc-definitions-in-headers.rst
+++ docs/clang-tidy/checks/misc-definitions-in-headers.rst
@@ -82,6 +82,8 @@
 
constexpr int k = 1; // OK: constexpr variable has internal linkage.
 
+   constexpr int f10() { return 0; } // OK: constexpr function definition.
+
 Options
 ---
 


Index: test/clang-tidy/misc-definitions-in-headers.hpp
===
--- test/clang-tidy/misc-definitions-in-headers.hpp
+++ test/clang-tidy/misc-definitions-in-headers.hpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s misc-definitions-in-headers %t -- -- -std=c++11
+// RUN: %check_clang_tidy %s misc-definitions-in-headers %t
 
 int f() {
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f' defined in a header file; function definitions in header files can lead to ODR violations [misc-definitions-in-headers]
@@ -177,3 +177,5 @@
 }
 
 constexpr int k = 1; // OK: constexpr variable has internal linkage.
+
+constexpr int f10() { return 0; } // OK: constexpr function definition.
Index: test/clang-tidy/check_clang_tidy.py
===
--- test/clang-tidy/check_clang_tidy.py
+++ test/clang-tidy/check_clang_tidy.py
@@ -58,8 +58,8 @@
 
   clang_tidy_extra_args = extra_args
   if len(clang_tidy_extra_args) == 0:
-clang_tidy_extra_args = ['--', '--std=c++11'] if extension == '.cpp' \
-   else ['--']
+clang_tidy_extra_args = ['--', '--std=c++11'] \
+if extension == '.cpp' or extension == '.hpp' else ['--']
 
   # Tests should not rely on STL being available, and instead provide mock
   # implementations of relevant APIs.
Index: docs/clang-tidy/checks/misc-definitions-in-headers.rst
===
--- docs/clang-tidy/checks/misc-definitions-in-headers.rst
+++ docs/clang-tidy/checks/misc-definitions-in-headers.rst
@@ -82,6 +82,8 @@
 
constexpr int k = 1; // OK: constexpr variable has internal linkage.
 
+   constexpr int f10() { return 0; } // OK: constexpr function definition.
+
 Options
 ---
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34770: [Bash-autocompletion] Auto complete cc1 options if -cc1 is specified

[Yuka Takahashi via Phabricator via cfe-commits]

yamaguchi created this revision.
Herald added a subscriber: hiraditya.

We don't want to autocomplete flags whose Flags class has `NoDriverOption` when 
argv[1] is not `-cc1`.

Another idea for this implementation is to make --autocomplete a cc1
option and handle it in clang Frontend, by porting --autocomplete
handler from Driver to Frontend, so that we can handle Driver options
and CC1 options in unified manner.


https://reviews.llvm.org/D34770

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/autocomplete.c
  clang/utils/bash-autocomplete.sh
  llvm/include/llvm/Option/OptTable.h
  llvm/lib/Option/OptTable.cpp

Index: llvm/lib/Option/OptTable.cpp
===
--- llvm/lib/Option/OptTable.cpp
+++ llvm/lib/Option/OptTable.cpp
@@ -225,11 +225,15 @@
   return {};
 }
 
-std::vector OptTable::findByPrefix(StringRef Cur) const {
+std::vector
+OptTable::findByPrefix(StringRef Cur, unsigned short DisableFlags) const {
   std::vector Ret;
   for (const Info  : OptionInfos.slice(FirstSearchableIndex)) {
 if (!In.Prefixes)
   continue;
+if (In.Flags & DisableFlags)
+  continue;
+
 for (int I = 0; In.Prefixes[I]; I++) {
   std::string S = std::string(In.Prefixes[I]) + std::string(In.Name);
   if (StringRef(S).startswith(Cur))
Index: llvm/include/llvm/Option/OptTable.h
===
--- llvm/include/llvm/Option/OptTable.h
+++ llvm/include/llvm/Option/OptTable.h
@@ -140,7 +140,8 @@
   //  to start with.
   ///
   /// \return The vector of flags which start with Cur.
-  std::vector findByPrefix(StringRef Cur) const;
+  std::vector findByPrefix(StringRef Cur,
+unsigned short DisableFlags) const;
 
   /// \brief Parse a single argument; returning the new argument and
   /// updating Index.
Index: clang/utils/bash-autocomplete.sh
===
--- clang/utils/bash-autocomplete.sh
+++ clang/utils/bash-autocomplete.sh
@@ -10,18 +10,23 @@
   # So, we need to partially undo bash tokenization here for integrity.
   local w1="${COMP_WORDS[$cword - 1]}"
   local w2="${COMP_WORDS[$cword - 2]}"
+  # Clang want to know if -cc1 option is specified or not, because we don't want to show
+  # cc1 options otherwise.
+  if [[ "${COMP_WORDS[1]}" == "-cc1" ]]; then
+arg="#"
+  fi
   if [[ "$cur" == -* ]]; then
 # -foo
-arg="$cur"
+arg="$arg$cur"
   elif [[ "$w1" == -*  && "$cur" == '=' ]]; then
 # -foo=
-arg="$w1=,"
+arg="$arg$w1=,"
   elif [[ "$w1" == -* ]]; then
 # -foo  or -foo bar
-arg="$w1,$cur"
+arg="$arg$w1,$cur"
   elif [[ "$w2" == -* && "$w1" == '=' ]]; then
 # -foo=bar
-arg="$w2=,$cur"
+arg="$arg$w2=,$cur"
   fi
 
   flags=$( "${COMP_WORDS[0]}" --autocomplete="$arg" 2>/dev/null )
Index: clang/test/Driver/autocomplete.c
===
--- clang/test/Driver/autocomplete.c
+++ clang/test/Driver/autocomplete.c
@@ -36,3 +36,7 @@
 // MTHREADMODELALL: posix single
 // RUN: %clang --autocomplete=-mrelocation-model, | FileCheck %s -check-prefix=MRELOCMODELALL
 // MRELOCMODELALL: dynamic-no-pic pic ropi ropi-rwpi rwpi static
+// RUN: %clang --autocomplete=-mrelocation-mode | FileCheck %s -check-prefix=MRELOCMODEL_CLANG
+// MRELOCMODEL_CLANG-NOT: -mrelocation-model
+// RUN: %clang --autocomplete=#-mrelocation-mode | FileCheck %s -check-prefix=MRELOCMODEL_CC1
+// MRELOCMODEL_CC1: -mrelocation-model
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1231,11 +1231,20 @@
 StringRef PassedFlags = A->getValue();
 std::vector SuggestedCompletions;
 
+unsigned short DisableFlags = options::NoDriverOption;
+// We want to show cc1-only options only when clang is invoked as "clang -cc1".
+// When clang is invoked as "clang -cc1", we add "#" to ther beginning of an --autocomplete
+// option so that the clang driver can distinguish whether it is requested to show cc1-only options or not.
+if (PassedFlags[0] == '#') {
+  DisableFlags = 0;
+  PassedFlags = PassedFlags.substr(1);
+}
+
 if (PassedFlags.find(',') == StringRef::npos) {
   // If the flag is in the form of "--autocomplete=-foo",
   // we were requested to print out all option names that start with "-foo".
   // For example, "--autocomplete=-fsyn" is expanded to "-fsyntax-only".
-  SuggestedCompletions = Opts->findByPrefix(PassedFlags);
+  SuggestedCompletions = Opts->findByPrefix(PassedFlags, DisableFlags);
 } else {
   // If the flag is in the form of "--autocomplete=foo,bar", we were
   // requested to print out all option values for "-foo" that start with
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D34449: [clang-tidy] Enable inline variable definitions in headers.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: test/clang-tidy/misc-definitions-in-headers.hpp:180
+class CE {
+  constexpr static int i = 5; // OK: constexpr definition.
+};

aaron.ballman wrote:
> xazax.hun wrote:
> > hokein wrote:
> > > hokein wrote:
> > > > aaron.ballman wrote:
> > > > > This is not as safe as you might think. As-is, this is fine, however, 
> > > > > if the class is given an inline function where that variable is 
> > > > > odr-used, you will get an ODR violation.
> > > > > 
> > > > > I think it's mildly better to err on the side of safety here and 
> > > > > diagnose.
> > > > I think the current code (Line `97` in `DefinitionsInHeadersCheck.cpp`) 
> > > > has already guaranteed this case. Can you try to run it without your 
> > > > change in the `DefinitionsInHeadersCheck.cpp`?
> > > > 
> > > > I think it still makes sense to add `constexpr` test cases.
> > > > 
> > > >   
> > > In C++17, `constexpr static int i` is an inline variable, which is fine 
> > > to define in C++ header -- because `inline` specifier provides a facility 
> > > allowing definitions (functions/variables) in header that is included in 
> > > multiple TUs. Additionally, one of the `inline variable` motivations is 
> > > to support the development of header-only libraries, you can find 
> > > discussions in 
> > > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4424.pdf.
> > > 
> > > Therefore, I'm +1 ignore the inline variables (the same as inline 
> > > functions). 
> > Unfortunately, the test will fail without this modification, but I modified 
> > it to ignore inline variables, which is a way better approach indeed. 
> The paper you cited was a feature not a defect, so prior to the paper's 
> adoption in C++17, the behavior is that the constexpr variable may trigger an 
> ODR violation, which is why I was saying this should be diagnosed rather than 
> ignored. There was real-world motivation for that paper.
> prior to the paper's adoption in C++17, the behavior is that the constexpr 
> variable may trigger an ODR violation

Yeah, I did remember our discussion when I implemented this check 
(https://stackoverflow.com/questions/23652156/how-would-use-of-unnamed-namespaces-in-headers-cause-odr-violations).
 We allow internal linkage variables (static/const/conexpr) in the check -- 
because we want to keep a small number of warnings as const variable 
definitions are widely used in headers.

Maybe add an option to enable this particular case?


Repository:
  rL LLVM

https://reviews.llvm.org/D34449



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

[PATCH] D34342: [OpenCL] Fix code generation of function-scope constant samplers.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In https://reviews.llvm.org/D34342#792350, @bader wrote:

> Note: `get_sampler_initializer` from my test case returns integer, not a 
> sampler, but having function is not relevant to the problem. 
>  Here is a bit simplified test case without function calls that still 
> reproduces the problem:
>
>   kernel void foo(int sampler_init_value) {
> const sampler_t const_smp_func_init = sampler_init_value;
>   }
>
>
> The problem is in the function that handles sampler initialization with 
> integer.
>  There should no problems with the case you provided as it doesn't require 
> additional function call injection.


This seems like a valid code which should be compiled in my view. At least I 
don't see anything in the spec that disallows this. This is not an interesting 
use case though, however it becomes more interesting with the use of ternary 
operator. Do you see any issues fixing the CodeGen for it?


https://reviews.llvm.org/D34342



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

[PATCH] D29647: [OpenMP] Extend CLANG target options with device offloading kind.

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea added inline comments.



Comment at: test/Driver/openmp-offload.c:614
+/// Check -march propagates compute capability to device offloading toolchain.
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda 
-save-temps -no-canonical-prefixes -march=sm_35 %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-COMPUTE-CAPABILITY %s

hfinkel wrote:
> gtbercea wrote:
> > hfinkel wrote:
> > > How does this work on x86 where the host also uses -march?
> > That's where the additional flag comes in. The new flag *should* be used 
> > instead. 
> Ah, okay. Please just propose a patch which adds a new flag. This workaround 
> is a bit strong for my tastes.
Completely agree! It's not a solution I am happy with it either as temporary as 
it is. I will propose a patch with a new flag.


Repository:
  rL LLVM

https://reviews.llvm.org/D29647



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

[PATCH] D34741: [ASTReader] Treat multiple defns of ObjC protocols the same as interfaces.

[Graydon Hoare via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL306583: [ASTReader] Treat multiple defns of ObjC protocols 
the same as interfaces. (authored by graydon).

Repository:
  rL LLVM

https://reviews.llvm.org/D34741

Files:
  cfe/trunk/include/clang/AST/Redeclarable.h
  cfe/trunk/lib/Serialization/ASTReaderDecl.cpp

Index: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
===
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
@@ -126,6 +126,9 @@
 void ReadObjCDefinitionData(struct ObjCInterfaceDecl::DefinitionData );
 void MergeDefinitionData(ObjCInterfaceDecl *D,
  struct ObjCInterfaceDecl::DefinitionData &);
+void ReadObjCDefinitionData(struct ObjCProtocolDecl::DefinitionData );
+void MergeDefinitionData(ObjCProtocolDecl *D,
+ struct ObjCProtocolDecl::DefinitionData &);
 
 static NamedDecl *getAnonymousDeclForMerging(ASTReader ,
  DeclContext *DC,
@@ -1045,18 +1048,8 @@
   IVD->setSynthesize(synth);
 }
 
-void ASTDeclReader::VisitObjCProtocolDecl(ObjCProtocolDecl *PD) {
-  RedeclarableResult Redecl = VisitRedeclarable(PD);
-  VisitObjCContainerDecl(PD);
-  mergeRedeclarable(PD, Redecl);
-
-  if (Record.readInt()) {
-// Read the definition.
-PD->allocateDefinitionData();
-
-// Set the definition data of the canonical declaration, so other
-// redeclarations will see it.
-PD->getCanonicalDecl()->Data = PD->Data;
+void ASTDeclReader::ReadObjCDefinitionData(
+ struct ObjCProtocolDecl::DefinitionData ) {
 
 unsigned NumProtoRefs = Record.readInt();
 SmallVector ProtoRefs;
@@ -1067,9 +1060,37 @@
 ProtoLocs.reserve(NumProtoRefs);
 for (unsigned I = 0; I != NumProtoRefs; ++I)
   ProtoLocs.push_back(ReadSourceLocation());
-PD->setProtocolList(ProtoRefs.data(), NumProtoRefs, ProtoLocs.data(),
-Reader.getContext());
+Data.ReferencedProtocols.set(ProtoRefs.data(), NumProtoRefs,
+ ProtoLocs.data(), Reader.getContext());
+}
+
+void ASTDeclReader::MergeDefinitionData(ObjCProtocolDecl *D,
+ struct ObjCProtocolDecl::DefinitionData &) {
+  // FIXME: odr checking?
+}
 
+void ASTDeclReader::VisitObjCProtocolDecl(ObjCProtocolDecl *PD) {
+  RedeclarableResult Redecl = VisitRedeclarable(PD);
+  VisitObjCContainerDecl(PD);
+  mergeRedeclarable(PD, Redecl);
+
+  if (Record.readInt()) {
+// Read the definition.
+PD->allocateDefinitionData();
+
+ReadObjCDefinitionData(PD->data());
+
+ObjCProtocolDecl *Canon = PD->getCanonicalDecl();
+if (Canon->Data.getPointer()) {
+  // If we already have a definition, keep the definition invariant and
+  // merge the data.
+  MergeDefinitionData(Canon, std::move(PD->data()));
+  PD->Data = Canon->Data;
+} else {
+  // Set the definition data of the canonical declaration, so other
+  // redeclarations will see it.
+  PD->getCanonicalDecl()->Data = PD->Data;
+}
 // Note that we have deserialized a definition.
 Reader.PendingDefinitions.insert(PD);
   } else {
Index: cfe/trunk/include/clang/AST/Redeclarable.h
===
--- cfe/trunk/include/clang/AST/Redeclarable.h
+++ cfe/trunk/include/clang/AST/Redeclarable.h
@@ -21,6 +21,60 @@
 namespace clang {
 class ASTContext;
 
+// Some notes on redeclarables:
+//
+//  - Every redeclarable is on a circular linked list.
+//
+//  - Every decl has a pointer to the first element of the chain _and_ a
+//DeclLink that may point to one of 3 possible states:
+//  - the "previous" (temporal) element in the chain
+//  - the "latest" (temporal) element in the chain
+//  - the an "uninitialized-latest" value (when newly-constructed)
+//
+//  - The first element is also often called the canonical element. Every
+//element has a pointer to it so that "getCanonical" can be fast.
+//
+//  - Most links in the chain point to previous, except the link out of
+//the first; it points to latest.
+//
+//  - Elements are called "first", "previous", "latest" or
+//"most-recent" when referring to temporal order: order of addition
+//to the chain.
+//
+//  - To make matters confusing, the DeclLink type uses the term "next"
+//for its pointer-storage internally (thus functions like
+//NextIsPrevious). It's easiest to just ignore the implementation of
+//DeclLink when making sense of the redeclaration chain.
+//
+//  - There's also a "definition" link for several types of
+//redeclarable, where only one definition should exist at any given
+//time (and the defn pointer is stored in the decl's "data" which
+//is copied to every element on the chain when it's changed).
+//
+//Here is some ASCII art:
+//
+//  "first"

r306583 - [ASTReader] Treat multiple defns of ObjC protocols the same as interfaces.

[Graydon Hoare via cfe-commits]

Author: graydon
Date: Wed Jun 28 11:36:27 2017
New Revision: 306583

URL: http://llvm.org/viewvc/llvm-project?rev=306583=rev
Log:
[ASTReader] Treat multiple defns of ObjC protocols the same as interfaces.

Summary:
In change 2ba19793512, the ASTReader logic for ObjC interfaces was modified to
preserve the first definition-data read, "merging" later definitions into it
rather than overwriting it (though this "merging" is, in practice, a no-op that
discards the later definition-data).

Unfortunately this change was only made to ObjC interfaces, not protocols; this
means that when (for example) loading a protocol that references an interface,
if both the protocol and interface are multiply defined (as can easily happen
if the same header is read from multiple contexts), an _inconsistent_ pair of
definitions is loaded: first-read for the interface and last-read for the
protocol.

This in turn causes very subtle downstream bugs in the Swift ClangImporter,
which filters the results of name lookups based on the owning module of a
definition; inconsistency between a pair of related definitions causes name
lookup failures at various stages of compilation.

To fix these downstream issues, this change replicates the logic applied to
interfaces in change 2ba19793512, but for ObjC protocols.

rdar://30851899

Reviewers: doug.gregor, rsmith

Reviewed By: doug.gregor

Subscribers: jordan_rose, cfe-commits

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

Modified:
cfe/trunk/include/clang/AST/Redeclarable.h
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp

Modified: cfe/trunk/include/clang/AST/Redeclarable.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Redeclarable.h?rev=306583=306582=306583=diff
==
--- cfe/trunk/include/clang/AST/Redeclarable.h (original)
+++ cfe/trunk/include/clang/AST/Redeclarable.h Wed Jun 28 11:36:27 2017
@@ -21,6 +21,60 @@
 namespace clang {
 class ASTContext;
 
+// Some notes on redeclarables:
+//
+//  - Every redeclarable is on a circular linked list.
+//
+//  - Every decl has a pointer to the first element of the chain _and_ a
+//DeclLink that may point to one of 3 possible states:
+//  - the "previous" (temporal) element in the chain
+//  - the "latest" (temporal) element in the chain
+//  - the an "uninitialized-latest" value (when newly-constructed)
+//
+//  - The first element is also often called the canonical element. Every
+//element has a pointer to it so that "getCanonical" can be fast.
+//
+//  - Most links in the chain point to previous, except the link out of
+//the first; it points to latest.
+//
+//  - Elements are called "first", "previous", "latest" or
+//"most-recent" when referring to temporal order: order of addition
+//to the chain.
+//
+//  - To make matters confusing, the DeclLink type uses the term "next"
+//for its pointer-storage internally (thus functions like
+//NextIsPrevious). It's easiest to just ignore the implementation of
+//DeclLink when making sense of the redeclaration chain.
+//
+//  - There's also a "definition" link for several types of
+//redeclarable, where only one definition should exist at any given
+//time (and the defn pointer is stored in the decl's "data" which
+//is copied to every element on the chain when it's changed).
+//
+//Here is some ASCII art:
+//
+//  "first" "latest"
+//  "canonical" "most recent"
+//  ++ first+--+
+//  || <--- |  |
+//  ||  |  |
+//  ||  |  |
+//  ||   +--+   |  |
+//  || first |  |   |  |
+//  || < |  |   |  |
+//  ||   |  |   |  |
+//  | @class A   |  link | @interface A |  link | @class A |
+//  | seen first | < | seen second  | < | seen third   |
+//  ||   |  |   |  |
+//  ++   +--+   +--+
+//  | data   | defn  | data |  defn | data |
+//  || > |  | < |  |
+//  ++   +--+   +--+
+//| | ^  ^
+//| |defn |  |
+//| link+-+  |
+//+-->---+
+
 /// \brief Provides common interface for the Decls that can be redeclared.
 template
 class Redeclarable {

Modified: 

[PATCH] D29647: [OpenMP] Extend CLANG target options with device offloading kind.

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added inline comments.



Comment at: test/Driver/openmp-offload.c:614
+/// Check -march propagates compute capability to device offloading toolchain.
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda 
-save-temps -no-canonical-prefixes -march=sm_35 %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-COMPUTE-CAPABILITY %s

gtbercea wrote:
> hfinkel wrote:
> > How does this work on x86 where the host also uses -march?
> That's where the additional flag comes in. The new flag *should* be used 
> instead. 
Ah, okay. Please just propose a patch which adds a new flag. This workaround is 
a bit strong for my tastes.


Repository:
  rL LLVM

https://reviews.llvm.org/D29647



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

[PATCH] D33681: [OpenCL] Allow function declaration with empty argument list.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

Sure, no problem! We might add some note on this feature to Clang manual. But 
we can do it later as well. Thanks!


https://reviews.llvm.org/D33681



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

[PATCH] D29647: [OpenMP] Extend CLANG target options with device offloading kind.

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea added inline comments.



Comment at: test/Driver/openmp-offload.c:614
+/// Check -march propagates compute capability to device offloading toolchain.
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda 
-save-temps -no-canonical-prefixes -march=sm_35 %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-COMPUTE-CAPABILITY %s

hfinkel wrote:
> How does this work on x86 where the host also uses -march?
That's where the additional flag comes in. The new flag *should* be used 
instead. 


Repository:
  rL LLVM

https://reviews.llvm.org/D29647



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

[libcxx] r306581 - Updated notest on 2974

[Marshall Clow via cfe-commits]

Author: marshall
Date: Wed Jun 28 11:19:34 2017
New Revision: 306581

URL: http://llvm.org/viewvc/llvm-project?rev=306581=rev
Log:
Updated notest on 2974

Modified:
libcxx/trunk/www/upcoming_meeting.html

Modified: libcxx/trunk/www/upcoming_meeting.html
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/www/upcoming_meeting.html?rev=306581=306580=306581=diff
==
--- libcxx/trunk/www/upcoming_meeting.html (original)
+++ libcxx/trunk/www/upcoming_meeting.html Wed Jun 28 11:19:34 2017
@@ -90,10 +90,10 @@
 2954 - I don't think there's anything to do here.
 2961 - We haven't implemented the PMR stuff yet.
 2966 - Wording cleanup; no code or test changes needed.
-2974 - I have some code lying around that does this.
+2974 - I did this in r305196. Tests added in 306580
 
 
-Last Updated: 25-Jun-2017
+Last Updated: 28-Jun-2017
 
 
 


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

[libcxx] r306580 - Added failing tests for index out of range for tuple_element<pair<T1, T2>> and variant_alternative<>

[Marshall Clow via cfe-commits]

Author: marshall
Date: Wed Jun 28 11:18:30 2017
New Revision: 306580

URL: http://llvm.org/viewvc/llvm-project?rev=306580=rev
Log:
Added failing tests for index out of range for tuple_element> and 
variant_alternative<>

Added:

libcxx/trunk/test/std/utilities/utility/pairs/pair.astuple/tuple_element.fail.cpp

libcxx/trunk/test/std/utilities/variant/variant.helpers/variant_alternative.fail.cpp

Added: 
libcxx/trunk/test/std/utilities/utility/pairs/pair.astuple/tuple_element.fail.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/utility/pairs/pair.astuple/tuple_element.fail.cpp?rev=306580=auto
==
--- 
libcxx/trunk/test/std/utilities/utility/pairs/pair.astuple/tuple_element.fail.cpp
 (added)
+++ 
libcxx/trunk/test/std/utilities/utility/pairs/pair.astuple/tuple_element.fail.cpp
 Wed Jun 28 11:18:30 2017
@@ -0,0 +1,22 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// 
+
+// template  struct pair
+
+// tuple_element >::type
+
+#include 
+
+int main()
+{
+typedef std::pair T;
+typename std::tuple_element<2, T>::type foo; // expected-error@utility:* 
{{Index out of bounds in std::tuple_element>}}
+}

Added: 
libcxx/trunk/test/std/utilities/variant/variant.helpers/variant_alternative.fail.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/variant/variant.helpers/variant_alternative.fail.cpp?rev=306580=auto
==
--- 
libcxx/trunk/test/std/utilities/variant/variant.helpers/variant_alternative.fail.cpp
 (added)
+++ 
libcxx/trunk/test/std/utilities/variant/variant.helpers/variant_alternative.fail.cpp
 Wed Jun 28 11:18:30 2017
@@ -0,0 +1,32 @@
+// -*- C++ -*-
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03, c++11, c++14
+
+// 
+
+// template  struct variant_alternative; // undefined
+// template  struct variant_alternative;
+// template  struct variant_alternative;
+// template  struct variant_alternative;
+// template 
+//   using variant_alternative_t = typename variant_alternative::type;
+//
+// template 
+//struct variant_alternative;
+
+#include 
+#include 
+#include 
+
+int main() {
+using V = std::variant;
+typename std::variant_alternative<4, V>::type foo;  // 
expected-error@variant:* {{Index out of bounds in std::variant_alternative<>}}
+}


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

[PATCH] D34680: clang-cl crashes with -fprofile-instr-use flag

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

Do you know which function clang was processing when it crashed? That would 
help us find a test case.


https://reviews.llvm.org/D34680



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

[PATCH] D33932: [clang-format] Add support for case-insensitive header matching and use it to improve support for LLVM-style include sorting.

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc added a comment.

In https://reviews.llvm.org/D33932#793994, @djasper wrote:

> Just make clang-format always do this. I don't think anyone is relying on the 
> current behavior.


Done, PTAL.


https://reviews.llvm.org/D33932



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

[PATCH] D33932: [clang-format] Add support for case-insensitive header matching and use it to improve support for LLVM-style include sorting.

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc updated this revision to Diff 104471.
chandlerc added a comment.

Update based an Daniel's feedback.


https://reviews.llvm.org/D33932

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  unittests/Format/SortIncludesTest.cpp


Index: unittests/Format/SortIncludesTest.cpp
===
--- unittests/Format/SortIncludesTest.cpp
+++ unittests/Format/SortIncludesTest.cpp
@@ -266,6 +266,29 @@
  "a.cc"));
 }
 
+TEST_F(SortIncludesTest, SupportCaseInsensitiveMatching) {
+  // Setup an regex for main includes so we can cover those as well.
+  Style.IncludeIsMainRegex = "([-_](test|unittest))?$";
+
+  // Ensure both main header detection and grouping work in a case insensitive
+  // manner.
+  EXPECT_EQ("#include \"llvm/A.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n"
+"#include \"LLVM/z.h\"\n"
+"#include \"llvm/X.h\"\n"
+"#include \"GTest/GTest.h\"\n"
+"#include \"gmock/gmock.h\"\n",
+sort("#include \"c.h\"\n"
+ "#include \"b.h\"\n"
+ "#include \"GTest/GTest.h\"\n"
+ "#include \"llvm/A.h\"\n"
+ "#include \"gmock/gmock.h\"\n"
+ "#include \"llvm/X.h\"\n"
+ "#include \"LLVM/z.h\"\n",
+ "a_TEST.cc"));
+}
+
 TEST_F(SortIncludesTest, NegativePriorities) {
   Style.IncludeCategories = {{".*important_os_header.*", -1}, {".*", 1}};
   EXPECT_EQ("#include \"important_os_header.h\"\n"
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -579,9 +579,9 @@
   LLVMStyle.ForEachMacros.push_back("Q_FOREACH");
   LLVMStyle.ForEachMacros.push_back("BOOST_FOREACH");
   LLVMStyle.IncludeCategories = {{"^\"(llvm|llvm-c|clang|clang-c)/", 2},
- {"^(<|\"(gtest|isl|json)/)", 3},
+ {"^(<|\"(gtest|gmock|isl|json)/)", 3},
  {".*", 1}};
-  LLVMStyle.IncludeIsMainRegex = "$";
+  LLVMStyle.IncludeIsMainRegex = "(Test)?$";
   LLVMStyle.IndentCaseLabels = false;
   LLVMStyle.IndentWrappedFunctionNames = false;
   LLVMStyle.IndentWidth = 2;
@@ -1409,7 +1409,7 @@
   : Style(Style), FileName(FileName) {
 FileStem = llvm::sys::path::stem(FileName);
 for (const auto  : Style.IncludeCategories)
-  CategoryRegexs.emplace_back(Category.Regex);
+  CategoryRegexs.emplace_back(Category.Regex, llvm::Regex::IgnoreCase);
 IsMainFile = FileName.endswith(".c") || FileName.endswith(".cc") ||
  FileName.endswith(".cpp") || FileName.endswith(".c++") ||
  FileName.endswith(".cxx") || FileName.endswith(".m") ||
@@ -1437,9 +1437,11 @@
   return false;
 StringRef HeaderStem =
 llvm::sys::path::stem(IncludeName.drop_front(1).drop_back(1));
-if (FileStem.startswith(HeaderStem)) {
+if (FileStem.startswith(HeaderStem) ||
+FileStem.startswith_lower(HeaderStem)) {
   llvm::Regex MainIncludeRegex(
-  (HeaderStem + Style.IncludeIsMainRegex).str());
+  (HeaderStem + Style.IncludeIsMainRegex).str(),
+  llvm::Regex::IgnoreCase);
   if (MainIncludeRegex.match(FileStem))
 return true;
 }
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -966,7 +966,7 @@
   ///   IncludeCategories:
   /// - Regex:   '^"(llvm|llvm-c|clang|clang-c)/'
   ///   Priority:2
-  /// - Regex:   '^(<|"(gtest|isl|json)/)'
+  /// - Regex:   '^(<|"(gtest|gmock|isl|json)/)'
   ///   Priority:3
   /// - Regex:   '.*'
   ///   Priority:1


Index: unittests/Format/SortIncludesTest.cpp
===
--- unittests/Format/SortIncludesTest.cpp
+++ unittests/Format/SortIncludesTest.cpp
@@ -266,6 +266,29 @@
  "a.cc"));
 }
 
+TEST_F(SortIncludesTest, SupportCaseInsensitiveMatching) {
+  // Setup an regex for main includes so we can cover those as well.
+  Style.IncludeIsMainRegex = "([-_](test|unittest))?$";
+
+  // Ensure both main header detection and grouping work in a case insensitive
+  // manner.
+  EXPECT_EQ("#include \"llvm/A.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n"
+"#include \"LLVM/z.h\"\n"
+"#include \"llvm/X.h\"\n"
+"#include \"GTest/GTest.h\"\n"
+"#include \"gmock/gmock.h\"\n",
+sort("#include \"c.h\"\n"
+ "#include \"b.h\"\n"
+ "#include \"GTest/GTest.h\"\n"
+ "#include \"llvm/A.h\"\n"
+ "#include \"gmock/gmock.h\"\n"
+ "#include \"llvm/X.h\"\n"
+ 

[PATCH] D34741: [ASTReader] Treat multiple defns of ObjC protocols the same as interfaces.

[Doug Gregor via Phabricator via cfe-commits]

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

Looks great!


https://reviews.llvm.org/D34741



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

[PATCH] D33932: [clang-format] Add support for case-insensitive header matching and use it to improve support for LLVM-style include sorting.

[Daniel Jasper via Phabricator via cfe-commits]

djasper added a comment.

Just make clang-format always do this. I don't think anyone is relying on the 
current behavior.


https://reviews.llvm.org/D33932



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

RE: D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude

[Blower, Melanie via cfe-commits]

 
Hahnfeld added a comment.

Some comments inline. In general you should consider posting an RFC on cfe-dev 
because this change will basically affect all compilations on GNU/Linux if the 
file is present.
>> Thank you

Adding Richard (general maintainer) and Renato (ARM Linux) so they are aware.




Comment at: include/clang/Driver/ToolChain.h:459-462
+  /// \brief Add arguments to use system-specific GNU includes.
+  virtual void AddGnuIncludeArgs(const llvm::opt::ArgList ,
+  llvm::opt::ArgStringList ) 
+ const;
+

Why do you need to define this method in the generic `ToolChain`?
>>You're right. I don't need this. I was imitating a similar change I saw to 
>>add include directories for a different toolchain. That's why I started out 
>>this way. Thank you.


Comment at: lib/Driver/ToolChains/Gnu.cpp:2332-2349
+void Generic_GCC::addGnuIncludeArgs(const ArgList , 
+ArgStringList ) const {
+  const Generic_GCC::GCCVersion  = 
+GCCInstallation.getVersion();
+  if (!DriverArgs.hasArg(options::OPT_ffreestanding) &&
+  !DriverArgs.hasArg(clang::driver::options::OPT_nostdinc) &&
+  !Version.isOlderThan(4, 8, 0)) {
+// If stdc-predef.h exists in the sytem includes, then -include it.

If this is GNU/Linux only, why not move to the `Linux` toolchain entirely?
>>Yes I will do that, good idea.


Comment at: lib/Driver/ToolChains/Gnu.h:328-330
+  void addGnuIncludeArgs(const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const;
+

This starts with a lower-case character and won't override the method in 
`ToolChain`. Adding the keyword `override` should reveal this mistake.
>>Thanks


Comment at: test/Driver/gcc-predef.c:1
+// Test that gcc preincludes stdc-predef.h on Linux //

s/gcc/clang/ ?

>>OK

Comment at: test/Driver/gcc-predef.c:9
+  #else
+#if !defined(  _STDC_PREDEF_H )
+  #error "stdc-predef.h should be preincluded for GNU/Linux 4.8 and higher"

The driver will only include `stdc-predef.h` if it can be found in the system. 
With that, the current version of this test will only run on such Linux system. 
Maybe add a basic tree in `test/Driver/Inputs` and test that the corresponding 
header is included?
>>I'm not sure what you're getting at "will only run on such a Linux system". 
>>Are you worried about the case where there's a linux system with gcc 4.8 and 
>>there's no such header? In that case this test would fail but I would think 
>>that's an outlier/oddball case. When I run this test case on my system, with 
>>that version and the file exists, this test did pass. I will study the tests 
>>in Driver/Inputs and see about creating a test there.


Comment at: test/Driver/gcc-predef.c:14-17
+// Test that gcc preincludes stdc-predef.h on Linux // // RUN: %clang 
+-c --target=i386-unknown-linux %s -Xclang -verify // 
+expected-no-diagnostics

What's the difference to the test above?
>>Whoops, I don't know how that happened. I will fix it. Thanks for your 
>>review, much appreciated.

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

Re: r306127 - [GSoC] Add support for CC1 options.

[Yuka Takahashi via cfe-commits]

Thank you for your feedback!

For options which have `NoDriverOption` Flags (such as -mrelocation-model),
I agree that we should expose these flags to users only when `-cc1` is also
passed.

However, as to `-mrelocation-model [tab]`, I think it is fine to provide
possible values for this option (static,pic,ropi..) because user has
already typed `-mrelocation-model` and maybe already be aware that this is
cc1 option.

I'll create a new patch and send it to review soon :)

2017-06-27 18:32 GMT+09:00 Vassil Vassilev :

> On 27/06/17 07:17, Saleem Abdulrasool via cfe-commits wrote:
>
> I think that we shouldn't be providing completion for `-cc1` options.
>  `-cc1as` options are fine as the IAS serves as a replacement for the
> traditional unix `as`.  But, the `NoDriverOption` values shouldn't be
> exposed to users.  They are internal details, with no compatibility.  If
> users start using those options, it makes it harder to prevent command line
> incompatibilities.
>
> Thanks for the feedback!
>
> We probably should only expose the cc1 options if the user typed clang
> -cc1 -f[tab], i.e. the user already is looking for something internal or
> make sure they are noted as a cc1 arguments. On the other hand, this should
> be of great help to more advanced users.
>
>
> On Fri, Jun 23, 2017 at 10:05 AM, Yuka Takahashi via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: yamaguchi
>> Date: Fri Jun 23 12:05:50 2017
>> New Revision: 306127
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=306127=rev
>> Log:
>> [GSoC] Add support for CC1 options.
>>
>> Summary:
>> Add value completion support for options which are defined in
>> CC1Options.td, because we only handled options in Options.td.
>>
>> Reviewers: ruiu, v.g.vassilev, teemperor
>>
>> Subscribers: llvm-commits
>>
>> Differential Revision: https://reviews.llvm.org/D34558
>>
>> Modified:
>> cfe/trunk/include/clang/Driver/CC1Options.td
>> cfe/trunk/test/Driver/autocomplete.c
>>
>> Modified: cfe/trunk/include/clang/Driver/CC1Options.td
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/
>> Driver/CC1Options.td?rev=306127=306126=306127=diff
>> 
>> ==
>> --- cfe/trunk/include/clang/Driver/CC1Options.td (original)
>> +++ cfe/trunk/include/clang/Driver/CC1Options.td Fri Jun 23 12:05:50 2017
>> @@ -158,7 +158,7 @@ def msave_temp_labels : Flag<["-"], "msa
>> "Note this may change .s semantics and shouldn't generally be
>> used "
>> "on compiler-generated code.">;
>>  def mrelocation_model : Separate<["-"], "mrelocation-model">,
>> -  HelpText<"The relocation model to use">;
>> +  HelpText<"The relocation model to use">, Values<"static,pic,ropi,rwpi,r
>> opi-rwpi,dynamic-no-pic">;
>>  def fno_math_builtin : Flag<["-"], "fno-math-builtin">,
>>HelpText<"Disable implicit builtin knowledge of math functions">;
>>  }
>> @@ -229,7 +229,7 @@ def no_struct_path_tbaa : Flag<["-"], "n
>>  def masm_verbose : Flag<["-"], "masm-verbose">,
>>HelpText<"Generate verbose assembly output">;
>>  def mcode_model : Separate<["-"], "mcode-model">,
>> -  HelpText<"The code model to use">;
>> +  HelpText<"The code model to use">, Values<"small,kernel,medium,la
>> rge">;
>>  def mdebug_pass : Separate<["-"], "mdebug-pass">,
>>HelpText<"Enable additional debug output">;
>>  def mdisable_fp_elim : Flag<["-"], "mdisable-fp-elim">,
>> @@ -308,7 +308,7 @@ def fsanitize_coverage_no_prune
>>HelpText<"Disable coverage pruning (i.e. instrument all
>> blocks/edges)">;
>>  def fprofile_instrument_EQ : Joined<["-"], "fprofile-instrument=">,
>>  HelpText<"Enable PGO instrumentation. The accepted value is clang,
>> llvm, "
>> - "or none">;
>> + "or none">, Values<"none,clang,llvm">;
>>  def fprofile_instrument_path_EQ : Joined<["-"],
>> "fprofile-instrument-path=">,
>>  HelpText<"Generate instrumented code to collect execution counts
>> into "
>>   " (overridden by LLVM_PROFILE_FILE env var)">;
>> @@ -348,9 +348,9 @@ def diagnostic_serialized_file : Separat
>>HelpText<"File for serializing diagnostics in a binary format">;
>>
>>  def fdiagnostics_format : Separate<["-"], "fdiagnostics-format">,
>> -  HelpText<"Change diagnostic formatting to match IDE and command line
>> tools">;
>> +  HelpText<"Change diagnostic formatting to match IDE and command line
>> tools">, Values<"clang,msvc,msvc-fallback,vi">;
>>  def fdiagnostics_show_category : Separate<["-"],
>> "fdiagnostics-show-category">,
>> -  HelpText<"Print diagnostic category">;
>> +  HelpText<"Print diagnostic category">, Values<"none,id,name">;
>>  def fno_diagnostics_use_presumed_location : Flag<["-"],
>> "fno-diagnostics-use-presumed-location">,
>>HelpText<"Ignore #line directives when displaying diagnostic
>> locations">;
>>  def ftabstop : Separate<["-"], "ftabstop">, MetaVarName<"">,
>> @@ -595,11 +595,11 @@ def 

[PATCH] D29647: [OpenMP] Extend CLANG target options with device offloading kind.

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added inline comments.



Comment at: test/Driver/openmp-offload.c:614
+/// Check -march propagates compute capability to device offloading toolchain.
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda 
-save-temps -no-canonical-prefixes -march=sm_35 %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-COMPUTE-CAPABILITY %s

How does this work on x86 where the host also uses -march?


Repository:
  rL LLVM

https://reviews.llvm.org/D29647



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

[PATCH] D34761: [Bash-autocompletion] Invoke clang where user called

[Yuka Takahashi via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL306559: [Bash-autocompletion] Invoke clang where user called 
(authored by yamaguchi).

Changed prior to commit:
  https://reviews.llvm.org/D34761?vs=104438=104439#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D34761

Files:
  cfe/trunk/utils/bash-autocomplete.sh


Index: cfe/trunk/utils/bash-autocomplete.sh
===
--- cfe/trunk/utils/bash-autocomplete.sh
+++ cfe/trunk/utils/bash-autocomplete.sh
@@ -24,7 +24,7 @@
 arg="$w2=,$cur"
   fi
 
-  flags=$( clang --autocomplete="$arg" 2>/dev/null )
+  flags=$( "${COMP_WORDS[0]}" --autocomplete="$arg" 2>/dev/null )
   # If clang is old that it does not support --autocomplete,
   # fall back to the filename completion.
   if [[ "$?" != 0 ]]; then


Index: cfe/trunk/utils/bash-autocomplete.sh
===
--- cfe/trunk/utils/bash-autocomplete.sh
+++ cfe/trunk/utils/bash-autocomplete.sh
@@ -24,7 +24,7 @@
 arg="$w2=,$cur"
   fi
 
-  flags=$( clang --autocomplete="$arg" 2>/dev/null )
+  flags=$( "${COMP_WORDS[0]}" --autocomplete="$arg" 2>/dev/null )
   # If clang is old that it does not support --autocomplete,
   # fall back to the filename completion.
   if [[ "$?" != 0 ]]; then
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r306559 - [Bash-autocompletion] Invoke clang where user called

[Yuka Takahashi via cfe-commits]

Author: yamaguchi
Date: Wed Jun 28 09:29:26 2017
New Revision: 306559

URL: http://llvm.org/viewvc/llvm-project?rev=306559=rev
Log:
[Bash-autocompletion] Invoke clang where user called

Summary:
When user build clang and used completion Eg. `build/bin/clang -fno[tab]`, we 
want to invoke `build/bin/clang --autocomplete=-fno`, rather than `clang 
--autocomplete=-fno`.

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

Modified:
cfe/trunk/utils/bash-autocomplete.sh

Modified: cfe/trunk/utils/bash-autocomplete.sh
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/bash-autocomplete.sh?rev=306559=306558=306559=diff
==
--- cfe/trunk/utils/bash-autocomplete.sh (original)
+++ cfe/trunk/utils/bash-autocomplete.sh Wed Jun 28 09:29:26 2017
@@ -24,7 +24,7 @@ _clang()
 arg="$w2=,$cur"
   fi
 
-  flags=$( clang --autocomplete="$arg" 2>/dev/null )
+  flags=$( "${COMP_WORDS[0]}" --autocomplete="$arg" 2>/dev/null )
   # If clang is old that it does not support --autocomplete,
   # fall back to the filename completion.
   if [[ "$?" != 0 ]]; then


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

[PATCH] D34761: [Bash-autocompletion] Invoke clang where user called

[Rui Ueyama via Phabricator via cfe-commits]

ruiu accepted this revision.
ruiu added a comment.

LGTM


https://reviews.llvm.org/D34761



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

[PATCH] D34761: [Bash-autocompletion] Invoke clang where user called

[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.


https://reviews.llvm.org/D34761



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

[PATCH] D34761: [Bash-autocompletion] Invoke clang where user called

[Yuka Takahashi via Phabricator via cfe-commits]

yamaguchi created this revision.

When user build clang and used completion Eg. `build/bin/clang
-fno[tab]`, we want to invoke `build/bin/clang --autocomplete=-fno`,

  rather than `clang --autocomplete=-fno`.


https://reviews.llvm.org/D34761

Files:
  clang/utils/bash-autocomplete.sh


Index: clang/utils/bash-autocomplete.sh
===
--- clang/utils/bash-autocomplete.sh
+++ clang/utils/bash-autocomplete.sh
@@ -24,7 +24,7 @@
 arg="$w2=,$cur"
   fi
 
-  flags=$( clang --autocomplete="$arg" 2>/dev/null )
+  flags=$( "${COMP_WORDS[0]}" --autocomplete="$arg" 2>/dev/null )
   # If clang is old that it does not support --autocomplete,
   # fall back to the filename completion.
   if [[ "$?" != 0 ]]; then


Index: clang/utils/bash-autocomplete.sh
===
--- clang/utils/bash-autocomplete.sh
+++ clang/utils/bash-autocomplete.sh
@@ -24,7 +24,7 @@
 arg="$w2=,$cur"
   fi
 
-  flags=$( clang --autocomplete="$arg" 2>/dev/null )
+  flags=$( "${COMP_WORDS[0]}" --autocomplete="$arg" 2>/dev/null )
   # If clang is old that it does not support --autocomplete,
   # fall back to the filename completion.
   if [[ "$?" != 0 ]]; then
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33816: [Sema][ObjC] Don't allow -Wunguarded-availability to be silenced with redeclarations

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington updated this revision to Diff 104437.
erik.pilkington added a comment.

Improve diagnostics for unnamed types.


https://reviews.llvm.org/D33816

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/DelayedDiagnostic.h
  include/clang/Sema/Sema.h
  lib/Sema/DelayedDiagnostic.cpp
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaExpr.cpp
  test/Sema/attr-availability.c
  test/Sema/attr-deprecated.c
  test/Sema/attr-unavailable-message.c
  test/SemaCXX/attr-deprecated.cpp
  test/SemaObjC/attr-availability.m
  test/SemaObjC/unguarded-availability-new.m
  test/SemaObjC/unguarded-availability.m

Index: test/SemaObjC/unguarded-availability.m
===
--- test/SemaObjC/unguarded-availability.m
+++ test/SemaObjC/unguarded-availability.m
@@ -5,6 +5,8 @@
 #define AVAILABLE_10_11 __attribute__((availability(macos, introduced = 10.11)))
 #define AVAILABLE_10_12 __attribute__((availability(macos, introduced = 10.12)))
 
+typedef int AVAILABLE_10_12 new_int; // expected-note + {{marked partial here}}
+
 int func_10_11() AVAILABLE_10_11; // expected-note 4 {{'func_10_11' has been explicitly marked partial here}}
 
 #ifdef OBJCPP
@@ -70,9 +72,9 @@
 }
 
 __attribute__((objc_root_class))
-AVAILABLE_10_11 @interface Class_10_11 {
+AVAILABLE_10_11 @interface Class_10_11 { // expected-note{{annotate 'Class_10_11' with an availability attribute to silence}}
   int_10_11 foo;
-  int_10_12 bar; // expected-warning {{'int_10_12' is partial: introduced in macOS 10.12}} expected-note{{redeclare}}
+  int_10_12 bar; // expected-warning {{'int_10_12' is partial: introduced in macOS 10.12}}
 }
 - (void)method1;
 - (void)method2;
@@ -125,7 +127,7 @@
   };
 }
 
-void test_params(int_10_12 x); // expected-warning {{'int_10_12' is partial: introduced in macOS 10.12}} expected-note{{redeclare}}
+void test_params(int_10_12 x); // expected-warning {{'int_10_12' is partial: introduced in macOS 10.12}} expected-note{{annotate 'test_params' with an availability attribute to silence}}
 
 void test_params2(int_10_12 x) AVAILABLE_10_12; // no warn
 
@@ -234,3 +236,30 @@
 }
 
 #endif
+
+struct InStruct { // expected-note{{annotate 'InStruct' with an availability attribute to silence}}
+  new_int mem; // expected-warning{{'new_int' is partial}}
+
+  struct { new_int mem; } anon; // expected-warning{{'new_int' is partial}} expected-note{{annotate anonymous struct with an availability attribute}}
+};
+
+#ifdef OBJCPP
+static constexpr int AVAILABLE_10_12 SomeConstexprValue = 2; // expected-note{{marked partial here}}
+typedef enum { // expected-note{{annotate anonymous enum with an availability attribute}}
+  SomeValue = SomeConstexprValue // expected-warning{{'SomeConstexprValue' is partial}} 
+} SomeEnum;
+#endif
+
+@interface InInterface
+-(new_int)meth; // expected-warning{{'new_int' is partial}} expected-note{{annotate 'meth' with an availability attribute}}
+@end
+
+@interface Proper // expected-note{{annotate 'Proper' with an availability attribute}}
+@property (class) new_int x; // expected-warning{{'new_int' is partial}}
+@end
+
+void with_local_struct() {
+  struct local { // expected-note{{annotate 'local' with an availability attribute}}
+new_int x; // expected-warning{{'new_int' is partial}}
+  };
+}
Index: test/SemaObjC/unguarded-availability-new.m
===
--- test/SemaObjC/unguarded-availability-new.m
+++ test/SemaObjC/unguarded-availability-new.m
@@ -96,16 +96,16 @@
 FUNC_AVAILABLE new_int x;
 #ifndef NO_WARNING
 #ifdef MAC
-  // expected-warning@-3 {{'new_int' is partial: introduced in macOS 10.14}} expected-note@-3 {{explicitly redeclare 'new_int' to silence this warning}}
+  // expected-warning@-3 {{'new_int' is partial: introduced in macOS 10.14}} expected-note@-3 {{annotate 'x' with an availability attribute to silence}}
 #endif
 #ifdef IOS
-  // expected-warning@-6 {{'new_int' is partial: introduced in iOS 12}} expected-note@-6 {{explicitly redeclare 'new_int' to silence this warning}}
+  // expected-warning@-6 {{'new_int' is partial: introduced in iOS 12}} expected-note@-6 {{annotate 'x' with an availability attribute to silence}}
 #endif
 #ifdef TVOS
-  // expected-warning@-9 {{'new_int' is partial: introduced in tvOS 13}} expected-note@-9 {{explicitly redeclare 'new_int' to silence this warning}}
+  // expected-warning@-9 {{'new_int' is partial: introduced in tvOS 13}} expected-note@-9 {{annotate 'x' with an availability attribute to silence}}
 #endif
 #ifdef WATCHOS
-  // expected-warning@-12 {{'new_int' is partial: introduced in watchOS 5}} expected-note@-12 {{explicitly redeclare 'new_int' to silence this warning}}
+  // expected-warning@-12 {{'new_int' is partial: introduced in watchOS 5}} expected-note@-12 {{annotate 'x' with an availability attribute to silence}}
 #endif
 #endif
 
Index: test/SemaObjC/attr-availability.m

[clang-tools-extra] r306558 - [clangd] Add "Go to Declaration" functionality

[Marc-Andre Laperle via cfe-commits]

Author: malaperle
Date: Wed Jun 28 09:12:10 2017
New Revision: 306558

URL: http://llvm.org/viewvc/llvm-project?rev=306558=rev
Log:
[clangd] Add "Go to Declaration" functionality

Summary: This change allows to navigate to most identifiers' declarations in 
code. This is a first step towards implementing "Go to Definition". It reuses 
clangIndex in order to detect which occurrences corresponds to the position 
requested. The occurrences' Decls are then used to generate locations suitable 
for navigating to the declarations.

Reviewers: krasimir, bkramer, ilya-biryukov

Reviewed By: ilya-biryukov

Subscribers: cfe-commits, mgorny

Tags: #clang-tools-extra

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

Added:
clang-tools-extra/trunk/test/clangd/definitions.test
Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.h
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/Protocol.h
clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp
clang-tools-extra/trunk/clangd/ProtocolHandlers.h
clang-tools-extra/trunk/test/clangd/formatting.test

Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=306558=306557=306558=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Wed Jun 28 09:12:10 2017
@@ -18,6 +18,7 @@ add_clang_library(clangDaemon
   clangBasic
   clangFormat
   clangFrontend
+  clangIndex
   clangSema
   clangTooling
   clangToolingCore

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=306558=306557=306558=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Wed Jun 28 09:12:10 2017
@@ -69,6 +69,8 @@ public:
 JSONOutput ) override;
   void onCompletion(TextDocumentPositionParams Params, StringRef ID,
 JSONOutput ) override;
+  void onGoToDefinition(TextDocumentPositionParams Params, StringRef ID,
+JSONOutput ) override;
 
 private:
   ClangdLSPServer 
@@ -84,7 +86,8 @@ void ClangdLSPServer::LSPProtocolCallbac
   "documentRangeFormattingProvider": true,
   "documentOnTypeFormattingProvider": 
{"firstTriggerCharacter":"}","moreTriggerCharacter":[]},
   "codeActionProvider": true,
-  "completionProvider": {"resolveProvider": false, 
"triggerCharacters": [".",">"]}
+  "completionProvider": {"resolveProvider": false, 
"triggerCharacters": [".",">"]},
+  "definitionProvider": true
 }}})");
 }
 
@@ -191,6 +194,25 @@ void ClangdLSPServer::LSPProtocolCallbac
   R"(,"result":[)" + Completions + R"(]})");
 }
 
+void ClangdLSPServer::LSPProtocolCallbacks::onGoToDefinition(
+TextDocumentPositionParams Params, StringRef ID, JSONOutput ) {
+
+  auto Items = LangServer.Server.findDefinitions(
+  Params.textDocument.uri.file,
+  Position{Params.position.line, Params.position.character}).Value;
+
+  std::string Locations;
+  for (const auto  : Items) {
+Locations += Location::unparse(Item);
+Locations += ",";
+  }
+  if (!Locations.empty())
+Locations.pop_back();
+  Out.writeMessage(
+  R"({"jsonrpc":"2.0","id":)" + ID.str() +
+  R"(,"result":[)" + Locations + R"(]})");
+}
+
 ClangdLSPServer::ClangdLSPServer(JSONOutput , bool RunSynchronously)
 : Out(Out), DiagConsumer(*this),
   Server(CDB, DiagConsumer, FSProvider, RunSynchronously) {}

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=306558=306557=306558=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Wed Jun 28 09:12:10 2017
@@ -271,3 +271,17 @@ std::string ClangdServer::dumpAST(PathRe
   });
   return DumpFuture.get();
 }
+
+Tagged
+ClangdServer::findDefinitions(PathRef File, Position Pos) {
+  auto FileContents = DraftMgr.getDraft(File);
+  assert(FileContents.Draft && "findDefinitions is called for non-added 
document");
+
+  std::vector Result;
+  auto TaggedFS = FSProvider.getTaggedFileSystem(File);
+  Units.runOnUnit(File, *FileContents.Draft, ResourceDir, CDB, PCHs,
+  TaggedFS.Value, [&](ClangdUnit ) {
+Result = 

[PATCH] D29651: [OpenMP] Consider LIBRARY_PATH when selecting library paths for NVPTX targets in OpenMP mode.

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea abandoned this revision.
gtbercea added a comment.

Not needed.
These changes are related to looking up the .bc library for inlining purposes. 
I believe @arpith-jacob has already handled this in trunk. Therefore this is 
obsolete code.


Repository:
  rL LLVM

https://reviews.llvm.org/D29651



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

[PATCH] D34607: [Bash-autocompletion] Check clang version in Bash

[Yuka Takahashi via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL306555: [Bash-autocompletion] Check clang version in Bash 
(authored by yamaguchi).

Changed prior to commit:
  https://reviews.llvm.org/D34607?vs=103884=104435#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D34607

Files:
  cfe/trunk/utils/bash-autocomplete.sh


Index: cfe/trunk/utils/bash-autocomplete.sh
===
--- cfe/trunk/utils/bash-autocomplete.sh
+++ cfe/trunk/utils/bash-autocomplete.sh
@@ -1,7 +1,7 @@
 # Please add "source /path/to/bash-autocomplete.sh" to your .bashrc to use 
this.
 _clang()
 {
-  local cur prev words cword arg
+  local cur prev words cword arg flags
   _init_completion -n : || return
 
   # bash always separates '=' as a token even if there's no space before/after 
'='.
@@ -24,7 +24,14 @@
 arg="$w2=,$cur"
   fi
 
-  local flags=$( clang --autocomplete="$arg" )
+  flags=$( clang --autocomplete="$arg" 2>/dev/null )
+  # If clang is old that it does not support --autocomplete,
+  # fall back to the filename completion.
+  if [[ "$?" != 0 ]]; then
+_filedir
+return
+  fi
+
   if [[ "$cur" == '=' ]]; then
 COMPREPLY=( $( compgen -W "$flags" -- "") )
   elif [[ "$flags" == "" || "$arg" == "" ]]; then


Index: cfe/trunk/utils/bash-autocomplete.sh
===
--- cfe/trunk/utils/bash-autocomplete.sh
+++ cfe/trunk/utils/bash-autocomplete.sh
@@ -1,7 +1,7 @@
 # Please add "source /path/to/bash-autocomplete.sh" to your .bashrc to use this.
 _clang()
 {
-  local cur prev words cword arg
+  local cur prev words cword arg flags
   _init_completion -n : || return
 
   # bash always separates '=' as a token even if there's no space before/after '='.
@@ -24,7 +24,14 @@
 arg="$w2=,$cur"
   fi
 
-  local flags=$( clang --autocomplete="$arg" )
+  flags=$( clang --autocomplete="$arg" 2>/dev/null )
+  # If clang is old that it does not support --autocomplete,
+  # fall back to the filename completion.
+  if [[ "$?" != 0 ]]; then
+_filedir
+return
+  fi
+
   if [[ "$cur" == '=' ]]; then
 COMPREPLY=( $( compgen -W "$flags" -- "") )
   elif [[ "$flags" == "" || "$arg" == "" ]]; then
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r306555 - [Bash-autocompletion] Check clang version in Bash

[Yuka Takahashi via cfe-commits]

Author: yamaguchi
Date: Wed Jun 28 08:59:55 2017
New Revision: 306555

URL: http://llvm.org/viewvc/llvm-project?rev=306555=rev
Log:
[Bash-autocompletion] Check clang version in Bash

Summary:
Add check if user's clang version supports --autocomplete or not.
If not, we just autocomplete files.

Reviewers: ruiu, v.g.vassilev, teemperor

Subscribers: cfe-commits

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

Modified:
cfe/trunk/utils/bash-autocomplete.sh

Modified: cfe/trunk/utils/bash-autocomplete.sh
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/bash-autocomplete.sh?rev=306555=306554=306555=diff
==
--- cfe/trunk/utils/bash-autocomplete.sh (original)
+++ cfe/trunk/utils/bash-autocomplete.sh Wed Jun 28 08:59:55 2017
@@ -1,7 +1,7 @@
 # Please add "source /path/to/bash-autocomplete.sh" to your .bashrc to use 
this.
 _clang()
 {
-  local cur prev words cword arg
+  local cur prev words cword arg flags
   _init_completion -n : || return
 
   # bash always separates '=' as a token even if there's no space before/after 
'='.
@@ -24,7 +24,14 @@ _clang()
 arg="$w2=,$cur"
   fi
 
-  local flags=$( clang --autocomplete="$arg" )
+  flags=$( clang --autocomplete="$arg" 2>/dev/null )
+  # If clang is old that it does not support --autocomplete,
+  # fall back to the filename completion.
+  if [[ "$?" != 0 ]]; then
+_filedir
+return
+  fi
+
   if [[ "$cur" == '=' ]]; then
 COMPREPLY=( $( compgen -W "$flags" -- "") )
   elif [[ "$flags" == "" || "$arg" == "" ]]; then


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

[PATCH] D16971: [Sema] PR26077 Fixed crash when partial specialization is missing required parameters

[don hinton via Phabricator via cfe-commits]

hintonda added a comment.

ping...


https://reviews.llvm.org/D16971



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

[PATCH] D29647: [OpenMP] Extend CLANG target options with device offloading kind.

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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



Comment at: lib/Driver/ToolChains/Cuda.cpp:217
+  std::vector GPUArchNames;
+  // If this is an OpenMP action we need to extract the device architecture 
from
+  // the -march option.

hfinkel wrote:
> Do we have a test case for this?
Just added a test case for this.



Comment at: lib/Driver/ToolChains/Cuda.cpp:368
+  assert(!GpuArch.empty() && "Must have an explicit GPU arch.");
+  std::string LibDeviceFile;
 

Hahnfeld wrote:
> You can move the declaration down to the initialization
Done! Thanks for spotting that.


Repository:
  rL LLVM

https://reviews.llvm.org/D29647



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

[PATCH] D29647: [OpenMP] Extend CLANG target options with device offloading kind.

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea updated this revision to Diff 104427.
gtbercea added a comment.
Herald added subscribers: aheejin, jgravelle-google.

Updated diff to address comments.


Repository:
  rL LLVM

https://reviews.llvm.org/D29647

Files:
  include/clang/Driver/ToolChain.h
  lib/Driver/ToolChain.cpp
  lib/Driver/ToolChains/BareMetal.cpp
  lib/Driver/ToolChains/BareMetal.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/Cuda.cpp
  lib/Driver/ToolChains/Cuda.h
  lib/Driver/ToolChains/Fuchsia.cpp
  lib/Driver/ToolChains/Fuchsia.h
  lib/Driver/ToolChains/Gnu.cpp
  lib/Driver/ToolChains/Gnu.h
  lib/Driver/ToolChains/Hexagon.cpp
  lib/Driver/ToolChains/Hexagon.h
  lib/Driver/ToolChains/WebAssembly.cpp
  lib/Driver/ToolChains/WebAssembly.h
  lib/Driver/ToolChains/XCore.cpp
  lib/Driver/ToolChains/XCore.h
  test/Driver/openmp-offload.c

Index: test/Driver/openmp-offload.c
===
--- test/Driver/openmp-offload.c
+++ test/Driver/openmp-offload.c
@@ -607,3 +607,12 @@
 // CHK-FOPENMP-IS-DEVICE: clang{{.*}}.i" {{.*}}" "-fopenmp-is-device"
 // CHK-FOPENMP-IS-DEVICE-NEXT: clang{{.*}}.bc" {{.*}}.i" "-fopenmp-is-device" "-fopenmp-host-ir-file-path"
 // CHK-FOPENMP-IS-DEVICE-NEXT: clang{{.*}}.s" {{.*}}.bc" "-fopenmp-is-device"
+
+/// ###
+
+/// Check -march propagates compute capability to device offloading toolchain.
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda -save-temps -no-canonical-prefixes -march=sm_35 %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-COMPUTE-CAPABILITY %s
+
+// CHK-COMPUTE-CAPABILITY: ptxas{{.*}}" "--gpu-name" "sm_35"
+// CHK-COMPUTE-CAPABILITY-NEXT: nvlink{{.*}}" "-arch" "sm_35"
Index: lib/Driver/ToolChains/XCore.h
===
--- lib/Driver/ToolChains/XCore.h
+++ lib/Driver/ToolChains/XCore.h
@@ -67,7 +67,8 @@
   AddClangSystemIncludeArgs(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ) const override;
   void addClangTargetOptions(const llvm::opt::ArgList ,
- llvm::opt::ArgStringList ) const override;
+ llvm::opt::ArgStringList ,
+ Action::OffloadKind DeviceOffloadKind) const override;
   void AddClangCXXStdlibIncludeArgs(
   const llvm::opt::ArgList ,
   llvm::opt::ArgStringList ) const override;
Index: lib/Driver/ToolChains/XCore.cpp
===
--- lib/Driver/ToolChains/XCore.cpp
+++ lib/Driver/ToolChains/XCore.cpp
@@ -124,7 +124,8 @@
 }
 
 void XCoreToolChain::addClangTargetOptions(const ArgList ,
-   ArgStringList ) const {
+   ArgStringList ,
+   Action::OffloadKind) const {
   CC1Args.push_back("-nostdsysteminc");
 }
 
Index: lib/Driver/ToolChains/WebAssembly.h
===
--- lib/Driver/ToolChains/WebAssembly.h
+++ lib/Driver/ToolChains/WebAssembly.h
@@ -53,7 +53,8 @@
   bool SupportsProfiling() const override;
   bool HasNativeLLVMSupport() const override;
   void addClangTargetOptions(const llvm::opt::ArgList ,
- llvm::opt::ArgStringList ) const override;
+ llvm::opt::ArgStringList ,
+ Action::OffloadKind DeviceOffloadKind) const override;
   RuntimeLibType GetDefaultRuntimeLibType() const override;
   CXXStdlibType GetCXXStdlibType(const llvm::opt::ArgList ) const override;
   void AddClangSystemIncludeArgs(
Index: lib/Driver/ToolChains/WebAssembly.cpp
===
--- lib/Driver/ToolChains/WebAssembly.cpp
+++ lib/Driver/ToolChains/WebAssembly.cpp
@@ -134,7 +134,8 @@
 bool WebAssembly::HasNativeLLVMSupport() const { return true; }
 
 void WebAssembly::addClangTargetOptions(const ArgList ,
-ArgStringList ) const {
+ArgStringList ,
+Action::OffloadKind) const {
   if (DriverArgs.hasFlag(clang::driver::options::OPT_fuse_init_array,
  options::OPT_fno_use_init_array, true))
 CC1Args.push_back("-fuse-init-array");
Index: lib/Driver/ToolChains/Hexagon.h
===
--- lib/Driver/ToolChains/Hexagon.h
+++ lib/Driver/ToolChains/Hexagon.h
@@ -69,7 +69,8 @@
   ~HexagonToolChain() override;
 
   void addClangTargetOptions(const llvm::opt::ArgList ,
- llvm::opt::ArgStringList ) const override;
+ llvm::opt::ArgStringList ,
+ Action::OffloadKind DeviceOffloadKind) const override;
   void
   

[PATCH] D34469: Use vfs::FileSystem in ASTUnit when creating CompilerInvocation.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL306549: Use vfs::FileSystem in ASTUnit when creating 
CompilerInvocation. (authored by ibiryukov).

Repository:
  rL LLVM

https://reviews.llvm.org/D34469

Files:
  cfe/trunk/include/clang/Frontend/Utils.h
  cfe/trunk/lib/Frontend/ASTUnit.cpp
  cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp


Index: cfe/trunk/include/clang/Frontend/Utils.h
===
--- cfe/trunk/include/clang/Frontend/Utils.h
+++ cfe/trunk/include/clang/Frontend/Utils.h
@@ -184,10 +184,11 @@
 ///
 /// \return A CompilerInvocation, or 0 if none was built for the given
 /// argument vector.
-std::unique_ptr
-createInvocationFromCommandLine(ArrayRef Args,
-IntrusiveRefCntPtr Diags =
-IntrusiveRefCntPtr());
+std::unique_ptr createInvocationFromCommandLine(
+ArrayRef Args,
+IntrusiveRefCntPtr Diags =
+IntrusiveRefCntPtr(),
+IntrusiveRefCntPtr VFS = nullptr);
 
 /// Return the value of the last argument as an integer, or a default. If Diags
 /// is non-null, emits an error if the argument is given, but non-integral.
Index: cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp
===
--- cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp
+++ cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp
@@ -31,8 +31,8 @@
 /// \return A CompilerInvocation, or 0 if none was built for the given
 /// argument vector.
 std::unique_ptr clang::createInvocationFromCommandLine(
-ArrayRef ArgList,
-IntrusiveRefCntPtr Diags) {
+ArrayRef ArgList, IntrusiveRefCntPtr 
Diags,
+IntrusiveRefCntPtr VFS) {
   if (!Diags.get()) {
 // No diagnostics engine was provided, so create our own diagnostics object
 // with the default options.
@@ -46,7 +46,7 @@
 
   // FIXME: We shouldn't have to pass in the path info.
   driver::Driver TheDriver(Args[0], llvm::sys::getDefaultTargetTriple(),
-   *Diags);
+   *Diags, VFS);
 
   // Don't check that inputs exist, they may have been remapped.
   TheDriver.setCheckInputsExist(false);
Index: cfe/trunk/lib/Frontend/ASTUnit.cpp
===
--- cfe/trunk/lib/Frontend/ASTUnit.cpp
+++ cfe/trunk/lib/Frontend/ASTUnit.cpp
@@ -1638,7 +1638,7 @@
   , nullptr);
 
 CI = clang::createInvocationFromCommandLine(
-llvm::makeArrayRef(ArgBegin, ArgEnd), Diags);
+llvm::makeArrayRef(ArgBegin, ArgEnd), Diags, VFS);
 if (!CI)
   return nullptr;
   }


Index: cfe/trunk/include/clang/Frontend/Utils.h
===
--- cfe/trunk/include/clang/Frontend/Utils.h
+++ cfe/trunk/include/clang/Frontend/Utils.h
@@ -184,10 +184,11 @@
 ///
 /// \return A CompilerInvocation, or 0 if none was built for the given
 /// argument vector.
-std::unique_ptr
-createInvocationFromCommandLine(ArrayRef Args,
-IntrusiveRefCntPtr Diags =
-IntrusiveRefCntPtr());
+std::unique_ptr createInvocationFromCommandLine(
+ArrayRef Args,
+IntrusiveRefCntPtr Diags =
+IntrusiveRefCntPtr(),
+IntrusiveRefCntPtr VFS = nullptr);
 
 /// Return the value of the last argument as an integer, or a default. If Diags
 /// is non-null, emits an error if the argument is given, but non-integral.
Index: cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp
===
--- cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp
+++ cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp
@@ -31,8 +31,8 @@
 /// \return A CompilerInvocation, or 0 if none was built for the given
 /// argument vector.
 std::unique_ptr clang::createInvocationFromCommandLine(
-ArrayRef ArgList,
-IntrusiveRefCntPtr Diags) {
+ArrayRef ArgList, IntrusiveRefCntPtr Diags,
+IntrusiveRefCntPtr VFS) {
   if (!Diags.get()) {
 // No diagnostics engine was provided, so create our own diagnostics object
 // with the default options.
@@ -46,7 +46,7 @@
 
   // FIXME: We shouldn't have to pass in the path info.
   driver::Driver TheDriver(Args[0], llvm::sys::getDefaultTargetTriple(),
-   *Diags);
+   *Diags, VFS);
 
   // Don't check that inputs exist, they may have been remapped.
   TheDriver.setCheckInputsExist(false);
Index: cfe/trunk/lib/Frontend/ASTUnit.cpp
===
--- cfe/trunk/lib/Frontend/ASTUnit.cpp
+++ cfe/trunk/lib/Frontend/ASTUnit.cpp
@@ -1638,7 +1638,7 @@
   , nullptr);
 
 CI = clang::createInvocationFromCommandLine(
-llvm::makeArrayRef(ArgBegin, 

r306549 - Use vfs::FileSystem in ASTUnit when creating CompilerInvocation.

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Wed Jun 28 08:06:34 2017
New Revision: 306549

URL: http://llvm.org/viewvc/llvm-project?rev=306549=rev
Log:
Use vfs::FileSystem in ASTUnit when creating CompilerInvocation.

Summary: It used to always call into the RealFileSystem before.

Reviewers: bkramer, krasimir, klimek, bruno

Reviewed By: klimek

Subscribers: bruno, cfe-commits

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

Modified:
cfe/trunk/include/clang/Frontend/Utils.h
cfe/trunk/lib/Frontend/ASTUnit.cpp
cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp

Modified: cfe/trunk/include/clang/Frontend/Utils.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/Utils.h?rev=306549=306548=306549=diff
==
--- cfe/trunk/include/clang/Frontend/Utils.h (original)
+++ cfe/trunk/include/clang/Frontend/Utils.h Wed Jun 28 08:06:34 2017
@@ -184,10 +184,11 @@ createChainedIncludesSource(CompilerInst
 ///
 /// \return A CompilerInvocation, or 0 if none was built for the given
 /// argument vector.
-std::unique_ptr
-createInvocationFromCommandLine(ArrayRef Args,
-IntrusiveRefCntPtr Diags =
-IntrusiveRefCntPtr());
+std::unique_ptr createInvocationFromCommandLine(
+ArrayRef Args,
+IntrusiveRefCntPtr Diags =
+IntrusiveRefCntPtr(),
+IntrusiveRefCntPtr VFS = nullptr);
 
 /// Return the value of the last argument as an integer, or a default. If Diags
 /// is non-null, emits an error if the argument is given, but non-integral.

Modified: cfe/trunk/lib/Frontend/ASTUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTUnit.cpp?rev=306549=306548=306549=diff
==
--- cfe/trunk/lib/Frontend/ASTUnit.cpp (original)
+++ cfe/trunk/lib/Frontend/ASTUnit.cpp Wed Jun 28 08:06:34 2017
@@ -1638,7 +1638,7 @@ ASTUnit *ASTUnit::LoadFromCommandLine(
   , nullptr);
 
 CI = clang::createInvocationFromCommandLine(
-llvm::makeArrayRef(ArgBegin, ArgEnd), Diags);
+llvm::makeArrayRef(ArgBegin, ArgEnd), Diags, VFS);
 if (!CI)
   return nullptr;
   }

Modified: cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp?rev=306549=306548=306549=diff
==
--- cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp (original)
+++ cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp Wed Jun 28 
08:06:34 2017
@@ -31,8 +31,8 @@ using namespace llvm::opt;
 /// \return A CompilerInvocation, or 0 if none was built for the given
 /// argument vector.
 std::unique_ptr clang::createInvocationFromCommandLine(
-ArrayRef ArgList,
-IntrusiveRefCntPtr Diags) {
+ArrayRef ArgList, IntrusiveRefCntPtr 
Diags,
+IntrusiveRefCntPtr VFS) {
   if (!Diags.get()) {
 // No diagnostics engine was provided, so create our own diagnostics object
 // with the default options.
@@ -46,7 +46,7 @@ std::unique_ptr clan
 
   // FIXME: We shouldn't have to pass in the path info.
   driver::Driver TheDriver(Args[0], llvm::sys::getDefaultTargetTriple(),
-   *Diags);
+   *Diags, VFS);
 
   // Don't check that inputs exist, they may have been remapped.
   TheDriver.setCheckInputsExist(false);


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

[PATCH] D34755: [clangd] Added a test, checking that gcc install is searched via VFS.

[Manuel Klimek via Phabricator via cfe-commits]

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

lg


https://reviews.llvm.org/D34755



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

[PATCH] D29647: [OpenMP] Extend CLANG target options with device offloading kind.

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.cpp:435
+
+// TODO: get the compute capability from offloading arguments when not
+// using the default compute capability of sm_20.

hfinkel wrote:
> gtbercea wrote:
> > hfinkel wrote:
> > > gtbercea wrote:
> > > > hfinkel wrote:
> > > > > Why is this a TODO? Is the necessary information not currently 
> > > > > available in the offloading arguments, or are we just not grabbing it 
> > > > > in this patch?
> > > > > 
> > > > This is for a future patch in which we will grab the compute capability 
> > > > from a special flag. I believe there has already been some discussion 
> > > > on the dev mailing list as to what that flag should be but I don't 
> > > > think there was a general consensus as to how that flag should be named.
> > > Which mailing list thread discussed this?
> > I believe it was openmp-dev. The proposal was initially posted by Gregory 
> > Rodgers.
> Can you please find it and post the link from here: 
> http://lists.llvm.org/pipermail/openmp-dev/
> 
> I do recall discussing this at some point somewhere, but I'm not finding it 
> in my email.
I managed to find the e-mail at last. It looks like it was a side conversation 
which ended with Justin Lebar suggesting Greg to post the discussion on the 
llvm-dev list. That post never happened. :-(


Repository:
  rL LLVM

https://reviews.llvm.org/D29647



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

[PATCH] D34755: [clangd] Added a test, checking that gcc install is searched via VFS.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

I wonder if it's gonna fail on Windows. Maybe enable it only on Linux?


https://reviews.llvm.org/D34755



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

[PATCH] D34755: [clangd] Added a test, checking that gcc install is searched via VFS.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

I wonder if it's gonna fail on Windows. Maybe enable it only on Linux?


https://reviews.llvm.org/D34755



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

[PATCH] D34749: [clang-format] Fix parsing of msg{field}-style proto options

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added a comment.

I think this is ready for a review.


https://reviews.llvm.org/D34749



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

[PATCH] D34755: [clangd] Added a test, checking that gcc install is searched via VFS.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.

https://reviews.llvm.org/D34755

Files:
  unittests/clangd/ClangdTests.cpp


Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -166,8 +166,22 @@
 public:
   std::vector
   getCompileCommands(PathRef File) override {
-return {};
+if (ExtraClangFlags.empty())
+  return {};
+
+std::vector CommandLine;
+CommandLine.reserve(3 + ExtraClangFlags.size());
+CommandLine.insert(CommandLine.end(), {"clang", "-fsyntax-only"});
+CommandLine.insert(CommandLine.end(), ExtraClangFlags.begin(),
+   ExtraClangFlags.end());
+CommandLine.push_back(File.str());
+
+return {tooling::CompileCommand(llvm::sys::path::parent_path(File),
+llvm::sys::path::filename(File),
+CommandLine, "")};
   }
+
+  std::vector ExtraClangFlags;
 };
 
 class MockFSProvider : public FileSystemProvider {
@@ -394,6 +408,53 @@
   EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).Tag, FS.Tag);
 }
 
+TEST_F(ClangdVFSTest, SearchLibDir) {
+  // Checks that searches for GCC installation is done through vfs.
+  MockFSProvider FS;
+  ErrorCheckingDiagConsumer DiagConsumer;
+  MockCompilationDatabase CDB;
+  CDB.ExtraClangFlags = {"-xc++", "-target", "x86_64-linux-unknown", "-m64"};
+  ClangdServer Server(CDB, DiagConsumer, FS,
+  /*RunSynchronously=*/true);
+
+  // Just a random gcc version string
+  SmallString<8> Version("4.9.3");
+
+  // A lib dir for gcc installation
+  SmallString<64> LibDir("/usr/lib/gcc/x86_64-linux-gnu");
+  llvm::sys::path::append(LibDir, Version);
+
+  // Put crtbegin.o into LibDir/64 to trick clang into thinking there's a gcc
+  // installation there.
+  SmallString<64> DummyLibFile;
+  llvm::sys::path::append(DummyLibFile, LibDir, "64", "crtbegin.o");
+  FS.Files[DummyLibFile] = "";
+
+  SmallString<64> IncludeDir("/usr/include/c++");
+  llvm::sys::path::append(IncludeDir, Version);
+
+  SmallString<64> StringPath;
+  llvm::sys::path::append(StringPath, IncludeDir, "string");
+  FS.Files[StringPath] = "class mock_string {};";
+
+  auto FooCpp = getVirtualTestFilePath("foo.cpp");
+  const auto SourceContents = R"cpp(
+#include 
+mock_string x;
+)cpp";
+  FS.Files[FooCpp] = SourceContents;
+
+  Server.addDocument(FooCpp, SourceContents);
+  EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
+
+  const auto SourceContentsWithError = R"cpp(
+#include 
+std::string x;
+)cpp";
+  Server.addDocument(FooCpp, SourceContentsWithError);
+  EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags());
+}
+
 class ClangdCompletionTest : public ClangdVFSTest {
 protected:
   bool ContainsItem(std::vector const , StringRef Name) {


Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -166,8 +166,22 @@
 public:
   std::vector
   getCompileCommands(PathRef File) override {
-return {};
+if (ExtraClangFlags.empty())
+  return {};
+
+std::vector CommandLine;
+CommandLine.reserve(3 + ExtraClangFlags.size());
+CommandLine.insert(CommandLine.end(), {"clang", "-fsyntax-only"});
+CommandLine.insert(CommandLine.end(), ExtraClangFlags.begin(),
+   ExtraClangFlags.end());
+CommandLine.push_back(File.str());
+
+return {tooling::CompileCommand(llvm::sys::path::parent_path(File),
+llvm::sys::path::filename(File),
+CommandLine, "")};
   }
+
+  std::vector ExtraClangFlags;
 };
 
 class MockFSProvider : public FileSystemProvider {
@@ -394,6 +408,53 @@
   EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).Tag, FS.Tag);
 }
 
+TEST_F(ClangdVFSTest, SearchLibDir) {
+  // Checks that searches for GCC installation is done through vfs.
+  MockFSProvider FS;
+  ErrorCheckingDiagConsumer DiagConsumer;
+  MockCompilationDatabase CDB;
+  CDB.ExtraClangFlags = {"-xc++", "-target", "x86_64-linux-unknown", "-m64"};
+  ClangdServer Server(CDB, DiagConsumer, FS,
+  /*RunSynchronously=*/true);
+
+  // Just a random gcc version string
+  SmallString<8> Version("4.9.3");
+
+  // A lib dir for gcc installation
+  SmallString<64> LibDir("/usr/lib/gcc/x86_64-linux-gnu");
+  llvm::sys::path::append(LibDir, Version);
+
+  // Put crtbegin.o into LibDir/64 to trick clang into thinking there's a gcc
+  // installation there.
+  SmallString<64> DummyLibFile;
+  llvm::sys::path::append(DummyLibFile, LibDir, "64", "crtbegin.o");
+  FS.Files[DummyLibFile] = "";
+
+  SmallString<64> IncludeDir("/usr/include/c++");
+  llvm::sys::path::append(IncludeDir, Version);
+
+  SmallString<64> StringPath;
+  llvm::sys::path::append(StringPath, IncludeDir, "string");
+  FS.Files[StringPath] = "class mock_string {};";
+
+  

[PATCH] D34728: [ThinkLTO] Invoke build(Thin)?LTOPreLinkDefaultPipeline.

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In https://reviews.llvm.org/D34728#793131, @timshen wrote:

> A question I have is that I don't know how to test this. Ideally we want 
> -debug-pass-manager from opt, but that flag is not part of the LLVM libraries.


How about add a clang test that builds with "-mllvm -debug-pass=Structure" and 
-flto=thin, and check for the passes that are now expected to be added with 
this patch.




Comment at: clang/lib/CodeGen/BackendUtil.cpp:804-807
+  case 0:
+return PassBuilder::O0;
+
   case 1:

chandlerc wrote:
> Why is this change needed?
I assume it is just cleanup since this isn't currently called under O0 (so it 
would hit the assert under the default switch case if we ever did). Not sure 
that is necessary though, up to you two.



Comment at: llvm/test/Other/new-pm-thinlto-defaults.ll:157
 ; CHECK-PRELINK-O-NEXT: Running pass: GlobalOptPass
-; CHECK-PRELINK-O-NEXT: Running pass: NameAnonGlobalPass
 ; CHECK-POSTLINK-O-NEXT: Running pass: PassManager<{{.*}}Module{{.*}}>

chandlerc wrote:
> tejohnson wrote:
> > Can this be changed to check for the pass being added in its new location, 
> > since it should still be invoked somewhere for ThinLTO? If this change 
> > means it is no longer added under options to set up the thinlto pipeline 
> > via opt, I'd prefer that we go back to adding this to the pipeline in 
> > buildThinLTOPreLinkDefaultPipeline in the non-O0 case.
> It seems somewhat unfortunate to have a *semantic* requirement on a 
> particular placement of this pass inside of the pipeline... Especially when 
> the semantics pretty much only stem from C++. For example, an a language 
> without anonymous globals, you might not want this pass when doing ThinLTO.
> 
> Note that you can exactly model the thing Clang is doing with opt even after 
> this:
> 
>   opt -passes='thinlto-pre-link,name-anon-globals'
> For example, an a language without anonymous globals, you might not want this 
> pass when doing ThinLTO.

Wouldn't it just be a no-op?

> opt -passes='thinlto-pre-link,name-anon-globals'

True, it just seems less user-friendly. But since this is just for testing, I 
guess it doesn't matter much. So I am ok with your suggestion of pulling that 
out and doing a single call to this pass when in ThinLTO mode after setting up 
the pipelines.


https://reviews.llvm.org/D34728



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

[PATCH] D29647: [OpenMP] Extend CLANG target options with device offloading kind.

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.cpp:435
+
+// TODO: get the compute capability from offloading arguments when not
+// using the default compute capability of sm_20.

gtbercea wrote:
> hfinkel wrote:
> > gtbercea wrote:
> > > hfinkel wrote:
> > > > Why is this a TODO? Is the necessary information not currently 
> > > > available in the offloading arguments, or are we just not grabbing it 
> > > > in this patch?
> > > > 
> > > This is for a future patch in which we will grab the compute capability 
> > > from a special flag. I believe there has already been some discussion on 
> > > the dev mailing list as to what that flag should be but I don't think 
> > > there was a general consensus as to how that flag should be named.
> > Which mailing list thread discussed this?
> I believe it was openmp-dev. The proposal was initially posted by Gregory 
> Rodgers.
Can you please find it and post the link from here: 
http://lists.llvm.org/pipermail/openmp-dev/

I do recall discussing this at some point somewhere, but I'm not finding it in 
my email.


Repository:
  rL LLVM

https://reviews.llvm.org/D29647



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

[PATCH] D29647: [OpenMP] Extend CLANG target options with device offloading kind.

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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



Comment at: lib/Driver/ToolChains/Cuda.cpp:435
+
+// TODO: get the compute capability from offloading arguments when not
+// using the default compute capability of sm_20.

hfinkel wrote:
> gtbercea wrote:
> > hfinkel wrote:
> > > Why is this a TODO? Is the necessary information not currently available 
> > > in the offloading arguments, or are we just not grabbing it in this patch?
> > > 
> > This is for a future patch in which we will grab the compute capability 
> > from a special flag. I believe there has already been some discussion on 
> > the dev mailing list as to what that flag should be but I don't think there 
> > was a general consensus as to how that flag should be named.
> Which mailing list thread discussed this?
I believe it was openmp-dev. The proposal was initially posted by Gregory 
Rodgers.


Repository:
  rL LLVM

https://reviews.llvm.org/D29647



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

[PATCH] D34269: [clangd] Add "Go to Declaration" functionality

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D34269



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

[PATCH] D29647: [OpenMP] Extend CLANG target options with device offloading kind.

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.cpp:435
+
+// TODO: get the compute capability from offloading arguments when not
+// using the default compute capability of sm_20.

gtbercea wrote:
> hfinkel wrote:
> > Why is this a TODO? Is the necessary information not currently available in 
> > the offloading arguments, or are we just not grabbing it in this patch?
> > 
> This is for a future patch in which we will grab the compute capability from 
> a special flag. I believe there has already been some discussion on the dev 
> mailing list as to what that flag should be but I don't think there was a 
> general consensus as to how that flag should be named.
Which mailing list thread discussed this?


Repository:
  rL LLVM

https://reviews.llvm.org/D29647



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

[PATCH] D29647: [OpenMP] Extend CLANG target options with device offloading kind.

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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



Comment at: lib/Driver/ToolChains/Cuda.cpp:435
+
+// TODO: get the compute capability from offloading arguments when not
+// using the default compute capability of sm_20.

hfinkel wrote:
> Why is this a TODO? Is the necessary information not currently available in 
> the offloading arguments, or are we just not grabbing it in this patch?
> 
This is for a future patch in which we will grab the compute capability from a 
special flag. I believe there has already been some discussion on the dev 
mailing list as to what that flag should be but I don't think there was a 
general consensus as to how that flag should be named.


Repository:
  rL LLVM

https://reviews.llvm.org/D29647



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

[PATCH] D34269: [clangd] Add "Go to Declaration" functionality

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle-ericsson updated this revision to Diff 104413.
malaperle-ericsson added a comment.

Remove use of unique_ptr


https://reviews.llvm.org/D34269

Files:
  clangd/CMakeLists.txt
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  test/clangd/definitions.test
  test/clangd/formatting.test

Index: test/clangd/formatting.test
===
--- test/clangd/formatting.test
+++ test/clangd/formatting.test
@@ -4,14 +4,15 @@
 Content-Length: 125
 
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
-# CHECK: Content-Length: 424
+# CHECK: Content-Length: 462
 # CHECK: {"jsonrpc":"2.0","id":0,"result":{"capabilities":{
 # CHECK:   "textDocumentSync": 1,
 # CHECK:   "documentFormattingProvider": true,
 # CHECK:   "documentRangeFormattingProvider": true,
 # CHECK:   "documentOnTypeFormattingProvider": {"firstTriggerCharacter":"}","moreTriggerCharacter":[]},
 # CHECK:   "codeActionProvider": true,
-# CHECK:   "completionProvider": {"resolveProvider": false, "triggerCharacters": [".",">"]}
+# CHECK:   "completionProvider": {"resolveProvider": false, "triggerCharacters": [".",">"]},
+# CHECK:   "definitionProvider": true
 # CHECK: }}}
 #
 Content-Length: 193
Index: test/clangd/definitions.test
===
--- /dev/null
+++ test/clangd/definitions.test
@@ -0,0 +1,168 @@
+# RUN: clangd -run-synchronously < %s | FileCheck %s
+# It is absolutely vital that this file has CRLF line endings.
+#
+Content-Length: 125
+
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+
+Content-Length: 172
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///main.cpp","languageId":"cpp","version":1,"text":"int main() {\nint a;\na;\n}\n"}}}
+
+Content-Length: 148
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/definition","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":2,"character":0}}}
+# Go to local variable
+# CHECK: {"jsonrpc":"2.0","id":1,"result":[{"uri": "file:///main.cpp", "range": {"start": {"line": 1, "character": 0}, "end": {"line": 1, "character": 5}}}]}
+
+Content-Length: 148
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/definition","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":2,"character":1}}}
+# Go to local variable, end of token
+# CHECK: {"jsonrpc":"2.0","id":1,"result":[{"uri": "file:///main.cpp", "range": {"start": {"line": 1, "character": 0}, "end": {"line": 1, "character": 5}}}]}
+
+Content-Length: 214
+
+{"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"uri":"file:///main.cpp","version":2},"contentChanges":[{"text":"struct Foo {\nint x;\n};\nint main() {\n  Foo bar = { x : 1 };\n}\n"}]}}
+
+Content-Length: 149
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/definition","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":4,"character":14}}}
+# Go to field, GNU old-style field designator 
+# CHECK: {"jsonrpc":"2.0","id":1,"result":[{"uri": "file:///main.cpp", "range": {"start": {"line": 1, "character": 0}, "end": {"line": 1, "character": 5}}}]}
+
+Content-Length: 215
+
+{"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"uri":"file:///main.cpp","version":3},"contentChanges":[{"text":"struct Foo {\nint x;\n};\nint main() {\n  Foo baz = { .x = 2 };\n}\n"}]}}
+
+Content-Length: 149
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/definition","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":4,"character":15}}}
+# Go to field, field designator 
+# CHECK: {"jsonrpc":"2.0","id":1,"result":[{"uri": "file:///main.cpp", "range": {"start": {"line": 1, "character": 0}, "end": {"line": 1, "character": 5}}}]}
+
+Content-Length: 187
+
+{"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"uri":"file:///main.cpp","version":4},"contentChanges":[{"text":"int main() {\n   main();\n   return 0;\n}"}]}}
+
+Content-Length: 148
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/definition","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":1,"character":3}}}
+# Go to function declaration, function call 
+# CHECK: {"jsonrpc":"2.0","id":1,"result":[{"uri": "file:///main.cpp", "range": {"start": {"line": 0, "character": 0}, "end": {"line": 3, "character": 1}}}]}
+
+Content-Length: 208
+
+{"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"uri":"file:///main.cpp","version":5},"contentChanges":[{"text":"struct Foo {\n};\nint main() {\n   Foo bar;\n   return 0;\n}\n"}]}}
+
+Content-Length: 148
+

[PATCH] D34749: [clang-format] Fix parsing of msg{field}-style proto options

[Krasimir Georgiev via Phabricator via cfe-commits]

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

- Add a msg-style test too


https://reviews.llvm.org/D34749

Files:
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTestProto.cpp


Index: unittests/Format/FormatTestProto.cpp
===
--- unittests/Format/FormatTestProto.cpp
+++ unittests/Format/FormatTestProto.cpp
@@ -201,13 +201,26 @@
"  field_c: \"OK\"\n"
"  msg_field{field_d: 123}\n"
"};");
+  verifyFormat("option (MyProto.options) = {\n"
+   "  field_a: OK\n"
+   "  field_b{field_c: OK}\n"
+   "  field_d: OKOKOK\n"
+   "  field_e: OK\n"
+   "}");
 
   // Support syntax with <> instead of {}.
   verifyFormat("option (MyProto.options) = {\n"
"  field_c: \"OK\",\n"
"  msg_field: \n"
"};");
 
+  verifyFormat("option (MyProto.options) = {\n"
+   "  field_a: OK\n"
+   "  field_b\n"
+   "  field_d: OKOKOK\n"
+   "  field_e: OK\n"
+   "}");
+
   verifyFormat("option (MyProto.options) = {\n"
"  msg_field: <>\n"
"  field_c: \"OK\",\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1570,8 +1570,10 @@
   const FormatToken *NextNonComment = Current->getNextNonComment();
   if (Current->is(TT_ConditionalExpr))
 return prec::Conditional;
-  if (NextNonComment && NextNonComment->is(tok::colon) &&
-  NextNonComment->is(TT_DictLiteral))
+  if (NextNonComment && Current->is(TT_SelectorName) &&
+  (NextNonComment->is(TT_DictLiteral) ||
+   (Style.Language == FormatStyle::LK_Proto &&
+NextNonComment->is(tok::less
 return prec::Assignment;
   if (Current->is(TT_JsComputedPropertyName))
 return prec::Assignment;


Index: unittests/Format/FormatTestProto.cpp
===
--- unittests/Format/FormatTestProto.cpp
+++ unittests/Format/FormatTestProto.cpp
@@ -201,13 +201,26 @@
"  field_c: \"OK\"\n"
"  msg_field{field_d: 123}\n"
"};");
+  verifyFormat("option (MyProto.options) = {\n"
+   "  field_a: OK\n"
+   "  field_b{field_c: OK}\n"
+   "  field_d: OKOKOK\n"
+   "  field_e: OK\n"
+   "}");
 
   // Support syntax with <> instead of {}.
   verifyFormat("option (MyProto.options) = {\n"
"  field_c: \"OK\",\n"
"  msg_field: \n"
"};");
 
+  verifyFormat("option (MyProto.options) = {\n"
+   "  field_a: OK\n"
+   "  field_b\n"
+   "  field_d: OKOKOK\n"
+   "  field_e: OK\n"
+   "}");
+
   verifyFormat("option (MyProto.options) = {\n"
"  msg_field: <>\n"
"  field_c: \"OK\",\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1570,8 +1570,10 @@
   const FormatToken *NextNonComment = Current->getNextNonComment();
   if (Current->is(TT_ConditionalExpr))
 return prec::Conditional;
-  if (NextNonComment && NextNonComment->is(tok::colon) &&
-  NextNonComment->is(TT_DictLiteral))
+  if (NextNonComment && Current->is(TT_SelectorName) &&
+  (NextNonComment->is(TT_DictLiteral) ||
+   (Style.Language == FormatStyle::LK_Proto &&
+NextNonComment->is(tok::less
 return prec::Assignment;
   if (Current->is(TT_JsComputedPropertyName))
 return prec::Assignment;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34749: [clang-format] Fix parsing of msg{field}-style proto options

[Krasimir Georgiev via Phabricator via cfe-commits]

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

This patch makes the `{` in `msg_field{field: OK}` in a proto option scope be
treated as an assignment operator. Previosly the added test case was formatted
as:

  option (MyProto.options) = {
field_a: OK
field_b{field_c: OK} field_d: OKOKOK field_e: OK
  }


https://reviews.llvm.org/D34749

Files:
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTestProto.cpp


Index: unittests/Format/FormatTestProto.cpp
===
--- unittests/Format/FormatTestProto.cpp
+++ unittests/Format/FormatTestProto.cpp
@@ -201,6 +201,12 @@
"  field_c: \"OK\"\n"
"  msg_field{field_d: 123}\n"
"};");
+  verifyFormat("option (MyProto.options) = {\n"
+   "  field_a: OK\n"
+   "  field_b{field_c: OK}\n"
+   "  field_d: OKOKOK\n"
+   "  field_e: OK\n"
+   "}");
 
   // Support syntax with <> instead of {}.
   verifyFormat("option (MyProto.options) = {\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1570,7 +1570,7 @@
   const FormatToken *NextNonComment = Current->getNextNonComment();
   if (Current->is(TT_ConditionalExpr))
 return prec::Conditional;
-  if (NextNonComment && NextNonComment->is(tok::colon) &&
+  if (NextNonComment && Current->is(TT_SelectorName) &&
   NextNonComment->is(TT_DictLiteral))
 return prec::Assignment;
   if (Current->is(TT_JsComputedPropertyName))


Index: unittests/Format/FormatTestProto.cpp
===
--- unittests/Format/FormatTestProto.cpp
+++ unittests/Format/FormatTestProto.cpp
@@ -201,6 +201,12 @@
"  field_c: \"OK\"\n"
"  msg_field{field_d: 123}\n"
"};");
+  verifyFormat("option (MyProto.options) = {\n"
+   "  field_a: OK\n"
+   "  field_b{field_c: OK}\n"
+   "  field_d: OKOKOK\n"
+   "  field_e: OK\n"
+   "}");
 
   // Support syntax with <> instead of {}.
   verifyFormat("option (MyProto.options) = {\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1570,7 +1570,7 @@
   const FormatToken *NextNonComment = Current->getNextNonComment();
   if (Current->is(TT_ConditionalExpr))
 return prec::Conditional;
-  if (NextNonComment && NextNonComment->is(tok::colon) &&
+  if (NextNonComment && Current->is(TT_SelectorName) &&
   NextNonComment->is(TT_DictLiteral))
 return prec::Assignment;
   if (Current->is(TT_JsComputedPropertyName))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34696: [refactor] Move the core of clang-rename to lib/Tooling/Refactoring

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

In https://reviews.llvm.org/D34696#793613, @klimek wrote:

> The main thing I'm concerned about is having the main code in core, but 
> having all tests in tools-extra. I think if we go that route we should also 
> move clang-rename and its tests to core. Thoughts?


Agreed. And if we are also moving rename-related libraries, it might also make 
sense to put them into a sub-directory like lib/Tooling/Refactoring/Rename.


Repository:
  rL LLVM

https://reviews.llvm.org/D34696



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

[PATCH] D34721: [PM] Add support for sample PGO in the new pass manager (clang-side)

[Teresa Johnson via Phabricator via cfe-commits]

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

LGTM for when the dependent patches are in.


https://reviews.llvm.org/D34721



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

[PATCH] D34686: [AArch64] ARMV8-A archkind and target defines helper functions

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 104393.
SjoerdMeijer retitled this revision from "[AArch64] Add hasFP16VectorArithmetic 
helper function. NFCI" to "[AArch64]  ARMV8-A archkind and target defines 
helper functions".
SjoerdMeijer edited the summary of this revision.
SjoerdMeijer added a comment.
Herald added a subscriber: javed.absar.

Actually, I've decided to make the functional change here because I am touching 
and modifying this code again.

So this is now a functional change:  ARM_FEATURE_QRDMX is now defined also for 
ARMv8.2. This is a define for some NEON additions to ARMv8.1, which should also 
be included to ARMv8.2.
I've created helper functions to include defines for the different architecture 
kinds, and a call to include the v8.2 defines also include the v8.1 ones.  
Regression tests have been added for this.

I have also modified the title/summary of this ticket.


https://reviews.llvm.org/D34686

Files:
  lib/Basic/Targets.cpp
  test/Preprocessor/aarch64-target-features.c
  test/Preprocessor/arm-target-features.c

Index: test/Preprocessor/arm-target-features.c
===
--- test/Preprocessor/arm-target-features.c
+++ test/Preprocessor/arm-target-features.c
@@ -441,4 +441,5 @@
 // CHECK-V82A: #define __ARM_ARCH 8
 // CHECK-V82A: #define __ARM_ARCH_8_2A__ 1
 // CHECK-V82A: #define __ARM_ARCH_PROFILE 'A'
+// CHECK-V82A: #define __ARM_FEATURE_QRDMX 1
 // CHECK-V82A: #define __ARM_FP 0xE
Index: test/Preprocessor/aarch64-target-features.c
===
--- test/Preprocessor/aarch64-target-features.c
+++ test/Preprocessor/aarch64-target-features.c
@@ -71,8 +71,9 @@
 // CHECK-NEON: __ARM_NEON 1
 // CHECK-NEON: __ARM_NEON_FP 0xE
 
-// RUN: %clang -target aarch64-none-eabi -march=armv8.1-a -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-V81A %s
-// CHECK-V81A: __ARM_FEATURE_QRDMX 1
+// RUN: %clang -target aarch64-none-eabi -march=armv8.1-a -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-QRDMX %s
+// RUN: %clang -target aarch64-none-eabi -march=armv8.2-a -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-QRDMX %s
+// CHECK-QRDMX: __ARM_FEATURE_QRDMX 1
 
 // RUN: %clang -target aarch64 -march=arm64 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-ARCH-NOT-ACCEPT %s
 // RUN: %clang -target aarch64 -march=aarch64 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-ARCH-NOT-ACCEPT %s
Index: lib/Basic/Targets.cpp
===
--- lib/Basic/Targets.cpp
+++ lib/Basic/Targets.cpp
@@ -5594,6 +5594,17 @@
 
   bool setFPMath(StringRef Name) override;
 
+  void getTargetDefinesARMV81A(const LangOptions ,
+   MacroBuilder ) const {
+Builder.defineMacro("__ARM_FEATURE_QRDMX", "1");
+  }
+
+  void getTargetDefinesARMV82A(const LangOptions ,
+   MacroBuilder ) const {
+// Also include the ARMv8.1-A defines
+getTargetDefinesARMV81A(Opts, Builder);
+  }
+
   void getTargetDefines(const LangOptions ,
 MacroBuilder ) const override {
 // Target identification.
@@ -5792,8 +5803,15 @@
 if (Opts.UnsafeFPMath)
   Builder.defineMacro("__ARM_FP_FAST", "1");
 
-if (ArchKind == llvm::ARM::AK_ARMV8_1A)
-  Builder.defineMacro("__ARM_FEATURE_QRDMX", "1");
+switch(ArchKind) {
+default: break;
+case llvm::ARM::AK_ARMV8_1A:
+  getTargetDefinesARMV81A(Opts, Builder);
+  break;
+case llvm::ARM::AK_ARMV8_2A:
+  getTargetDefinesARMV82A(Opts, Builder);
+  break;
+}
   }
 
   ArrayRef getTargetBuiltins() const override {
@@ -6186,9 +6204,8 @@
   unsigned CRC;
   unsigned Crypto;
   unsigned Unaligned;
-  unsigned V8_1A;
-  unsigned V8_2A;
   unsigned HasFullFP16;
+  llvm::AArch64::ArchKind ArchKind;
 
   static const Builtin::Info BuiltinInfo[];
 
@@ -6254,6 +6271,20 @@
static_cast(llvm::AArch64::ArchKind::AK_INVALID);
   }
 
+  void getTargetDefinesARMV81A(const LangOptions ,
+MacroBuilder ) const {
+Builder.defineMacro("__ARM_FEATURE_QRDMX", "1");
+  }
+
+  void getTargetDefinesARMV82A(const LangOptions ,
+MacroBuilder ) const {
+// Also include the ARMv8.1 defines
+getTargetDefinesARMV81A(Opts, Builder);
+
+if (FPU == NeonMode && HasFullFP16)
+  Builder.defineMacro("__ARM_FEATURE_FP16_VECTOR_ARITHMETIC", "1");
+  }
+
   void getTargetDefines(const LangOptions ,
 MacroBuilder ) const override {
 // Target identification.
@@ -6318,10 +6349,15 @@
 if (Unaligned)
   Builder.defineMacro("__ARM_FEATURE_UNALIGNED", "1");
 
-if (V8_1A)
-  Builder.defineMacro("__ARM_FEATURE_QRDMX", "1");
-if (V8_2A && FPU == NeonMode && HasFullFP16)
-  Builder.defineMacro("__ARM_FEATURE_FP16_VECTOR_ARITHMETIC", "1");
+switch(ArchKind) {
+default: break;
+case 

[PATCH] D34329: [clang-diff] Initial implementation.

[Johannes Altmanninger via Phabricator via cfe-commits]

johannes updated this revision to Diff 104401.

https://reviews.llvm.org/D34329

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  include/clang/Tooling/ASTDiff/ASTDiffInternal.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  lib/Tooling/ASTDiff/CMakeLists.txt
  lib/Tooling/CMakeLists.txt
  test/Tooling/clang-diff-basic.cpp
  tools/CMakeLists.txt
  tools/clang-diff/CMakeLists.txt
  tools/clang-diff/ClangDiff.cpp

Index: tools/clang-diff/ClangDiff.cpp
===
--- /dev/null
+++ tools/clang-diff/ClangDiff.cpp
@@ -0,0 +1,110 @@
+//===- ClangDiff.cpp - compare source files by AST nodes --*- C++ -*- -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file implements a tool for syntax tree based comparison using
+// Tooling/ASTDiff.
+//
+//===--===//
+
+#include "clang/Tooling/ASTDiff/ASTDiff.h"
+#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/CommandLine.h"
+
+using namespace llvm;
+using namespace clang;
+using namespace clang::tooling;
+
+static cl::OptionCategory ClangDiffCategory("clang-diff options");
+
+static cl::opt
+DumpAST("ast-dump",
+cl::desc("Print the internal representation of the AST as JSON."),
+cl::init(false), cl::cat(ClangDiffCategory));
+
+static cl::opt NoCompilationDatabase(
+"no-compilation-database",
+cl::desc(
+"Do not attempt to load build settings from a compilation database"),
+cl::init(false), cl::cat(ClangDiffCategory));
+
+static cl::opt SourcePath(cl::Positional, cl::desc(""),
+   cl::Required,
+   cl::cat(ClangDiffCategory));
+
+static cl::opt DestinationPath(cl::Positional,
+cl::desc(""),
+cl::Optional,
+cl::cat(ClangDiffCategory));
+
+static std::unique_ptr getAST(const StringRef Filename) {
+  std::string ErrorMessage;
+  std::unique_ptr Compilations;
+  if (!NoCompilationDatabase)
+Compilations =
+CompilationDatabase::autoDetectFromSource(Filename, ErrorMessage);
+  if (!Compilations) {
+if (!NoCompilationDatabase)
+  llvm::errs()
+  << "Error while trying to load a compilation database, running "
+ "without flags.\n"
+  << ErrorMessage;
+Compilations = llvm::make_unique(
+".", std::vector());
+  }
+  std::array Files = {{Filename}};
+  ClangTool Tool(*Compilations, Files);
+  std::vector ASTs;
+  Tool.buildASTs(ASTs);
+  if (ASTs.size() != Files.size())
+return nullptr;
+  return std::move(ASTs[0]);
+}
+
+int main(int argc, const char **argv) {
+  cl::HideUnrelatedOptions(ClangDiffCategory);
+  if (!cl::ParseCommandLineOptions(argc, argv)) {
+cl::PrintOptionValues();
+return 1;
+  }
+
+  if (DumpAST) {
+if (!DestinationPath.empty()) {
+  llvm::errs() << "Error: Please specify exactly one filename.\n";
+  return 1;
+}
+std::unique_ptr AST = getAST(SourcePath);
+if (!AST)
+  return 1;
+diff::SyntaxTree Tree(AST->getASTContext());
+Tree.printAsJson(llvm::outs());
+return 0;
+  }
+
+  if (DestinationPath.empty()) {
+llvm::errs() << "Error: Exactly two paths are required.\n";
+return 1;
+  }
+
+  std::unique_ptr Src = getAST(SourcePath);
+  std::unique_ptr Dst = getAST(DestinationPath);
+  if (!Src || !Dst)
+return 1;
+
+  diff::ComparisonOptions Options;
+  diff::SyntaxTree SrcTree(Src->getASTContext());
+  diff::SyntaxTree DstTree(Dst->getASTContext());
+  diff::ASTDiff DiffTool(SrcTree, DstTree, Options);
+  for (const auto  : DiffTool.getMatches())
+DiffTool.printMatch(llvm::outs(), Match);
+  for (const auto  : DiffTool.getChanges())
+DiffTool.printChange(llvm::outs(), Change);
+
+  return 0;
+}
Index: tools/clang-diff/CMakeLists.txt
===
--- /dev/null
+++ tools/clang-diff/CMakeLists.txt
@@ -0,0 +1,13 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_clang_executable(clang-diff
+  ClangDiff.cpp
+  )
+
+target_link_libraries(clang-diff
+  clangFrontend
+  clangTooling
+  clangToolingASTDiff
+  )
Index: tools/CMakeLists.txt
===
--- tools/CMakeLists.txt
+++ tools/CMakeLists.txt
@@ -2,6 +2,7 @@
 
 add_clang_subdirectory(diagtool)
 add_clang_subdirectory(driver)
+add_clang_subdirectory(clang-diff)
 add_clang_subdirectory(clang-format)
 add_clang_subdirectory(clang-format-vs)
 add_clang_subdirectory(clang-fuzzer)
Index: 

[PATCH] D34449: [clang-tidy] Enable inline variable definitions in headers.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: test/clang-tidy/misc-definitions-in-headers.hpp:180
+class CE {
+  constexpr static int i = 5; // OK: constexpr definition.
+};

xazax.hun wrote:
> hokein wrote:
> > hokein wrote:
> > > aaron.ballman wrote:
> > > > This is not as safe as you might think. As-is, this is fine, however, 
> > > > if the class is given an inline function where that variable is 
> > > > odr-used, you will get an ODR violation.
> > > > 
> > > > I think it's mildly better to err on the side of safety here and 
> > > > diagnose.
> > > I think the current code (Line `97` in `DefinitionsInHeadersCheck.cpp`) 
> > > has already guaranteed this case. Can you try to run it without your 
> > > change in the `DefinitionsInHeadersCheck.cpp`?
> > > 
> > > I think it still makes sense to add `constexpr` test cases.
> > > 
> > >   
> > In C++17, `constexpr static int i` is an inline variable, which is fine to 
> > define in C++ header -- because `inline` specifier provides a facility 
> > allowing definitions (functions/variables) in header that is included in 
> > multiple TUs. Additionally, one of the `inline variable` motivations is to 
> > support the development of header-only libraries, you can find discussions 
> > in http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4424.pdf.
> > 
> > Therefore, I'm +1 ignore the inline variables (the same as inline 
> > functions). 
> Unfortunately, the test will fail without this modification, but I modified 
> it to ignore inline variables, which is a way better approach indeed. 
The paper you cited was a feature not a defect, so prior to the paper's 
adoption in C++17, the behavior is that the constexpr variable may trigger an 
ODR violation, which is why I was saying this should be diagnosed rather than 
ignored. There was real-world motivation for that paper.


Repository:
  rL LLVM

https://reviews.llvm.org/D34449



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

[PATCH] D34329: [clang-diff] Initial implementation.

[Johannes Altmanninger via Phabricator via cfe-commits]

johannes updated this revision to Diff 104395.

https://reviews.llvm.org/D34329

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  include/clang/Tooling/ASTDiff/ASTDiffInternal.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  lib/Tooling/ASTDiff/CMakeLists.txt
  lib/Tooling/CMakeLists.txt
  test/Tooling/clang-diff-basic.cpp
  tools/CMakeLists.txt
  tools/clang-diff/CMakeLists.txt
  tools/clang-diff/ClangDiff.cpp

Index: tools/clang-diff/ClangDiff.cpp
===
--- /dev/null
+++ tools/clang-diff/ClangDiff.cpp
@@ -0,0 +1,110 @@
+//===- ClangDiff.cpp - compare source files by AST nodes --*- C++ -*- -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file implements a tool for syntax tree based comparison using
+// Tooling/ASTDiff.
+//
+//===--===//
+
+#include "clang/Tooling/ASTDiff/ASTDiff.h"
+#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/CommandLine.h"
+
+using namespace llvm;
+using namespace clang;
+using namespace clang::tooling;
+
+static cl::OptionCategory ClangDiffCategory("clang-diff options");
+
+static cl::opt
+DumpAST("ast-dump",
+cl::desc("Print the internal representation of the AST as JSON."),
+cl::init(false), cl::cat(ClangDiffCategory));
+
+static cl::opt NoCompilationDatabase(
+"no-compilation-database",
+cl::desc(
+"Do not attempt to load build settings from a compilation database"),
+cl::init(false), cl::cat(ClangDiffCategory));
+
+static cl::opt SourcePath(cl::Positional, cl::desc(""),
+   cl::Required,
+   cl::cat(ClangDiffCategory));
+
+static cl::opt DestinationPath(cl::Positional,
+cl::desc(""),
+cl::Optional,
+cl::cat(ClangDiffCategory));
+
+static std::unique_ptr getAST(const StringRef Filename) {
+  std::string ErrorMessage;
+  std::unique_ptr Compilations;
+  if (!NoCompilationDatabase)
+Compilations =
+CompilationDatabase::autoDetectFromSource(Filename, ErrorMessage);
+  if (!Compilations) {
+if (!NoCompilationDatabase)
+  llvm::errs()
+  << "Error while trying to load a compilation database, running "
+ "without flags.\n"
+  << ErrorMessage;
+Compilations = llvm::make_unique(
+".", std::vector());
+  }
+  std::array Files = {{Filename}};
+  ClangTool Tool(*Compilations, Files);
+  std::vector ASTs;
+  Tool.buildASTs(ASTs);
+  if (ASTs.size() != Files.size())
+return nullptr;
+  return std::move(ASTs[0]);
+}
+
+int main(int argc, const char **argv) {
+  cl::HideUnrelatedOptions(ClangDiffCategory);
+  if (!cl::ParseCommandLineOptions(argc, argv)) {
+cl::PrintOptionValues();
+return 1;
+  }
+
+  if (DumpAST) {
+if (!DestinationPath.empty()) {
+  llvm::errs() << "Error: Please specify exactly one filename.\n";
+  return 1;
+}
+std::unique_ptr AST = getAST(SourcePath);
+if (!AST)
+  return 1;
+diff::SyntaxTree Tree(AST->getASTContext());
+Tree.printAsJson(llvm::outs());
+return 0;
+  }
+
+  if (DestinationPath.empty()) {
+llvm::errs() << "Error: Exactly two paths are required.\n";
+return 1;
+  }
+
+  std::unique_ptr Src = getAST(SourcePath);
+  std::unique_ptr Dst = getAST(DestinationPath);
+  if (!Src || !Dst)
+return 1;
+
+  diff::ComparisonOptions Options;
+  diff::SyntaxTree SrcTree(Src->getASTContext());
+  diff::SyntaxTree DstTree(Dst->getASTContext());
+  diff::ASTDiff DiffTool(SrcTree, DstTree, Options);
+  for (const auto  : DiffTool.getMatches())
+DiffTool.printMatch(llvm::outs(), Match);
+  for (const auto  : DiffTool.getChanges())
+DiffTool.printChange(llvm::outs(), Change);
+
+  return 0;
+}
Index: tools/clang-diff/CMakeLists.txt
===
--- /dev/null
+++ tools/clang-diff/CMakeLists.txt
@@ -0,0 +1,13 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_clang_executable(clang-diff
+  ClangDiff.cpp
+  )
+
+target_link_libraries(clang-diff
+  clangFrontend
+  clangTooling
+  clangToolingASTDiff
+  )
Index: tools/CMakeLists.txt
===
--- tools/CMakeLists.txt
+++ tools/CMakeLists.txt
@@ -2,6 +2,7 @@
 
 add_clang_subdirectory(diagtool)
 add_clang_subdirectory(driver)
+add_clang_subdirectory(clang-diff)
 add_clang_subdirectory(clang-format)
 add_clang_subdirectory(clang-format-vs)
 add_clang_subdirectory(clang-fuzzer)
Index: 

[PATCH] D33278: [LLVM][x86][Inline Asm] - Enum support for MS syntax

[Matan via Phabricator via cfe-commits]

mharoush marked 4 inline comments as done.
mharoush added inline comments.



Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1382
+if (const MCConstantExpr *CE = 
+dyn_cast_or_null(Val)) {
+  StringRef ErrMsg;

rnk wrote:
> rnk wrote:
> > Please use clang-format here and elsewhere
> not addressed?
Persistent text wrapping of my editor, fixed.



Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1378
+// Check if the parsed identifier was a constant Integer. Here we
+// assume Val is of type MCConstantExpr only when it is safe to replace
+// the identifier with its constant value.

coby wrote:
> assumption ~~> assertion
I can assert the Identifier Info is indeed an enum. However, this is a general 
handle for any kind of value that may by passed back from the identifier 
parsing. There is no point in this assertion here since the value of type 
MCConstantExper is always safe. We simply make the decision at the parse 
identifier method.



Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1380
+// the identifier with its constant value.
+if (const MCConstantExpr *CE =
+dyn_cast_or_null(Val)) {

coby wrote:
> I think this whole section better suites within its own function. something 
> like 'ParseInlineAsmEnumValue'
not much added value here and only a few lines can be moved (~5). Its not 
really worth it.



Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1383
+  StringRef ErrMsg;
+  // SM should treat the value as it would an explicit integer in the 
+  // expression.

coby wrote:
> rephrase
Your comment is too general, I made some clarifications.



Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1388
+  // In case we are called on a bracketed expression,
+  if (isParsingInlineAsm() && SM.getAddImmPrefix()) {
+// A single rewrite of the integer value is preformed for each enum

coby wrote:
> 'isParsingInlineAsm()' is unnecessary here (you can only reach this piece of 
> code when parsing inline asm)
This is just for readability and consistency with SM.onInteger(). Replaced with 
assertion to ease your mind



Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1826
   }
-
-  // Rewrite the type operator and the C or C++ type or variable in terms of an
-  // immediate.  E.g. TYPE foo -> $$4
-  unsigned Len = End.getPointer() - TypeLoc.getPointer();
-  InstInfo->AsmRewrites->emplace_back(AOK_Imm, TypeLoc, Len, CVal);
-
+  // Only when in bracketed mode, preform explicit rewrite
+  if (AddImmPrefix) {

coby wrote:
> Not keen to the use of SM.getAddImmPrefix() as a mean of distinguish whether 
> we are parsing a bracketed expression. I know it is currently turned on when 
> parsing it, but it isn't asserted/guaranteed.
> Regardless - I'm pretty sure we can manage without this rewrite, or at the 
> very least - should, now that TYPE/LENGTH/SIZE are part of the State Machine.
I meant this flag is only set when parsing bracketed expressions. However, I 
will rephrase the comment since we only care about the AddImmPrefix flag, which 
reflects the SM matching flag. The objective here is to match the behavior 
required on integer constants when the SM flag is set.



Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1907
 unsigned Len = Tok.getLoc().getPointer() - Start.getPointer();
-if (StartTok.getString().size() == Len)
-  // Just add a prefix if this wasn't a complex immediate expression.
-  InstInfo->AsmRewrites->emplace_back(AOK_ImmPrefix, Start);
-else
-  // Otherwise, rewrite the complex expression as a single immediate.
+if (StartTok.getString().size() != Len || ReplaceEnumIdentifier)
+  // Rewrite the complex expression as a single immediate.

coby wrote:
> you may just perform an AOK_Imm rewrite regardless the complexity of the 
> immediate expression, and neglect 'ReplaceEnumIdentifier'
Thanks, this seems fine after the condition refinement. Just had to change some 
of the older inline asm tests which expects the IR statement to contain a 
binary/octal/hex base immediate value which this rewrite will replace with the 
decimal value.


Repository:
  rL LLVM

https://reviews.llvm.org/D33278



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

[PATCH] D34329: [clang-diff] Initial implementation.

[Johannes Altmanninger via Phabricator via cfe-commits]

johannes updated this revision to Diff 104394.
johannes added a comment.

- remove unused struct
- rename getNodeValueI -> getNodeValueImpl


https://reviews.llvm.org/D34329

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  include/clang/Tooling/ASTDiff/ASTDiffInternal.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  lib/Tooling/ASTDiff/CMakeLists.txt
  lib/Tooling/CMakeLists.txt
  test/Tooling/clang-diff-basic.cpp
  tools/CMakeLists.txt
  tools/clang-diff/CMakeLists.txt
  tools/clang-diff/ClangDiff.cpp

Index: tools/clang-diff/ClangDiff.cpp
===
--- /dev/null
+++ tools/clang-diff/ClangDiff.cpp
@@ -0,0 +1,110 @@
+//===- ClangDiff.cpp - compare source files by AST nodes --*- C++ -*- -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file implements a tool for syntax tree based comparison using
+// Tooling/ASTDiff.
+//
+//===--===//
+
+#include "clang/Tooling/ASTDiff/ASTDiff.h"
+#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/CommandLine.h"
+
+using namespace llvm;
+using namespace clang;
+using namespace clang::tooling;
+
+static cl::OptionCategory ClangDiffCategory("clang-diff options");
+
+static cl::opt
+DumpAST("ast-dump",
+cl::desc("Print the internal representation of the AST as JSON."),
+cl::init(false), cl::cat(ClangDiffCategory));
+
+static cl::opt NoCompilationDatabase(
+"no-compilation-database",
+cl::desc(
+"Do not attempt to load build settings from a compilation database"),
+cl::init(false), cl::cat(ClangDiffCategory));
+
+static cl::opt SourcePath(cl::Positional, cl::desc(""),
+   cl::Required,
+   cl::cat(ClangDiffCategory));
+
+static cl::opt DestinationPath(cl::Positional,
+cl::desc(""),
+cl::Optional,
+cl::cat(ClangDiffCategory));
+
+static std::unique_ptr getAST(const StringRef Filename) {
+  std::string ErrorMessage;
+  std::unique_ptr Compilations;
+  if (!NoCompilationDatabase)
+Compilations =
+CompilationDatabase::autoDetectFromSource(Filename, ErrorMessage);
+  if (!Compilations) {
+if (!NoCompilationDatabase)
+  llvm::errs()
+  << "Error while trying to load a compilation database, running "
+ "without flags.\n"
+  << ErrorMessage;
+Compilations = llvm::make_unique(
+".", std::vector());
+  }
+  std::array Files = {{Filename}};
+  ClangTool Tool(*Compilations, Files);
+  std::vector ASTs;
+  Tool.buildASTs(ASTs);
+  if (ASTs.size() != Files.size())
+return nullptr;
+  return std::move(ASTs[0]);
+}
+
+int main(int argc, const char **argv) {
+  cl::HideUnrelatedOptions(ClangDiffCategory);
+  if (!cl::ParseCommandLineOptions(argc, argv)) {
+cl::PrintOptionValues();
+return 1;
+  }
+
+  if (DumpAST) {
+if (!DestinationPath.empty()) {
+  llvm::errs() << "Error: Please specify exactly one filename.\n";
+  return 1;
+}
+std::unique_ptr AST = getAST(SourcePath);
+if (!AST)
+  return 1;
+diff::SyntaxTree Tree(AST->getASTContext());
+Tree.printAsJson(llvm::outs());
+return 0;
+  }
+
+  if (DestinationPath.empty()) {
+llvm::errs() << "Error: Exactly two paths are required.\n";
+return 1;
+  }
+
+  std::unique_ptr Src = getAST(SourcePath);
+  std::unique_ptr Dst = getAST(DestinationPath);
+  if (!Src || !Dst)
+return 1;
+
+  diff::ComparisonOptions Options;
+  diff::SyntaxTree SrcTree(Src->getASTContext());
+  diff::SyntaxTree DstTree(Dst->getASTContext());
+  diff::ASTDiff DiffTool(SrcTree, DstTree, Options);
+  for (const auto  : DiffTool.getMatches())
+DiffTool.printMatch(llvm::outs(), Match);
+  for (const auto  : DiffTool.getChanges())
+DiffTool.printChange(llvm::outs(), Change);
+
+  return 0;
+}
Index: tools/clang-diff/CMakeLists.txt
===
--- /dev/null
+++ tools/clang-diff/CMakeLists.txt
@@ -0,0 +1,13 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_clang_executable(clang-diff
+  ClangDiff.cpp
+  )
+
+target_link_libraries(clang-diff
+  clangFrontend
+  clangTooling
+  clangToolingASTDiff
+  )
Index: tools/CMakeLists.txt
===
--- tools/CMakeLists.txt
+++ tools/CMakeLists.txt
@@ -2,6 +2,7 @@
 
 add_clang_subdirectory(diagtool)
 add_clang_subdirectory(driver)
+add_clang_subdirectory(clang-diff)
 add_clang_subdirectory(clang-format)
 

[PATCH] D33278: [LLVM][x86][Inline Asm] - Enum support for MS syntax

[Matan via Phabricator via cfe-commits]

mharoush updated this revision to Diff 104391.
mharoush marked an inline comment as done.
mharoush added a comment.

simplified the rewrite condition of complex expressions and eliminated the need 
to use the ReplaceEnumIdentifier flag. This requires making small adjustments 
to some of the older test cases seen in [https://reviews.llvm.org/D33277]. Made 
some minor alterations and clarifications to satisfy reviewer comments.


Repository:
  rL LLVM

https://reviews.llvm.org/D33278

Files:
  include/llvm/MC/MCParser/MCAsmParser.h
  lib/Target/X86/AsmParser/X86AsmParser.cpp

Index: lib/Target/X86/AsmParser/X86AsmParser.cpp
===
--- lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -713,7 +713,7 @@
   std::unique_ptr ParseIntelOffsetOfOperator();
   bool ParseIntelDotOperator(const MCExpr *Disp, const MCExpr *);
   unsigned IdentifyIntelOperator(StringRef Name);
-  unsigned ParseIntelOperator(unsigned OpKind);
+  unsigned ParseIntelOperator(unsigned OpKind, bool AddImmPrefix);
   std::unique_ptr
   ParseIntelSegmentOverride(unsigned SegReg, SMLoc Start, unsigned Size);
   std::unique_ptr ParseRoundingModeOp(SMLoc Start, SMLoc End);
@@ -1187,7 +1187,7 @@
 InlineAsmIdentifierInfo , bool AllowBetterSizeMatch) {
   // If we found a decl other than a VarDecl, then assume it is a FuncDecl or
   // some other label reference.
-  if (isa(Disp) && Info.OpDecl && !Info.IsVarDecl) {
+  if (isa(Disp) && Info.OpDecl && !Info.isVarDecl()) {
 // Insert an explicit size if the user didn't have one.
 if (!Size) {
   Size = getPointerWidth();
@@ -1358,7 +1358,7 @@
 if (OpKind == IOK_OFFSET) 
   return Error(IdentLoc, "Dealing OFFSET operator as part of"
 "a compound immediate expression is yet to be supported");
-int64_t Val = ParseIntelOperator(OpKind);
+int64_t Val = ParseIntelOperator(OpKind,SM.getAddImmPrefix());
 if (!Val)
   return true;
 StringRef ErrMsg;
@@ -1368,13 +1368,40 @@
 PrevTK == AsmToken::RBrac) {
   return false;
   } else {
-InlineAsmIdentifierInfo  = SM.getIdentifierInfo();
+InlineAsmIdentifierInfo Info;
 if (ParseIntelIdentifier(Val, Identifier, Info,
- /*Unevaluated=*/false, End))
+  /*Unevaluated=*/false, End))
   return true;
-SM.onIdentifierExpr(Val, Identifier);
-  }
-  break;
+// Check if the parsed identifier was a constant Integer. Here we
+// assume Val is of type MCConstantExpr only when it is safe to replace
+// the identifier with its constant value.
+if (const MCConstantExpr *CE =
+dyn_cast_or_null(Val)) {
+  StringRef ErrMsg;
+  // Pass the enum identifier integer value to the SM calculator.
+  if (SM.onInteger(CE->getValue(), ErrMsg))
+return Error(IdentLoc, ErrMsg);
+  // Match the behavior of integer tokens when getAddImmPrefix flag is
+  // set.
+  if (SM.getAddImmPrefix()) {
+assert(isParsingInlineAsm() &&
+   "Expected to be parsing inline assembly.");
+// A single rewrite of the integer value is preformed for each enum
+// identifier. This is only done when we are inside a bracketed
+// expression.
+size_t Len = End.getPointer() - IdentLoc.getPointer();
+InstInfo->AsmRewrites->emplace_back(AOK_Imm, IdentLoc, Len,
+CE->getValue());
+break;
+  }
+}
+else {
+  // Notify the SM a variable identifier was found.
+  InlineAsmIdentifierInfo  = SM.getIdentifierInfo();
+  SMInfo = Info;
+  SM.onIdentifierExpr(Val, Identifier);
+}
+  }  break;
 }
 case AsmToken::Integer: {
   StringRef ErrMsg;
@@ -1452,6 +1479,7 @@
   // may have already parsed an immediate displacement before the bracketed
   // expression.
   IntelExprStateMachine SM(ImmDisp, /*StopOnLBrac=*/false, /*AddImmPrefix=*/true);
+  
   if (ParseIntelExpression(SM, End))
 return nullptr;
 
@@ -1560,6 +1588,14 @@
   assert((End.getPointer() == EndPtr || !Result) &&
  "frontend claimed part of a token?");
 
+  // Check if the search yielded a constant integer (enum identifier).
+  if (Result && Info.isConstEnum()) {
+// By creating MCConstantExpr we let the user of Val know it is safe
+// to use as an explicit constant with value = ConstVal.
+Val = MCConstantExpr::create(Info.ConstIntValue.getSExtValue(),
+ getParser().getContext());
+return false;
+  }
   // If the identifier lookup was unsuccessful, assume that we are dealing with
   // a label.
   if (!Result) {
@@ -1758,7 +1794,7 @@
 /// variable.  A variable's size is the product of its LENGTH 

[PATCH] D33277: [Clang][x86][Inline Asm] - Enum support for MS syntax

[Matan via Phabricator via cfe-commits]

mharoush updated this revision to Diff 104390.
mharoush added a comment.

Updated inline asm tests to look for decimal immediate value instead of looking 
for the original string e.g. 10 vs 0xA and other variations.
Also updated the test cases to use check-same etc.


Repository:
  rL LLVM

https://reviews.llvm.org/D33277

Files:
  lib/Sema/SemaStmtAsm.cpp
  test/CodeGen/ms-inline-asm.c
  test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
  test/CodeGenCXX/ms-inline-asm-return.cpp

Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -644,8 +644,8 @@
   // Referring to parameters is not allowed in naked functions.
   if (CheckNakedParmReference(Result.get(), *this))
 return ExprError();
-
-  QualType T = Result.get()->getType();
+  Expr *Res = Result.get();
+  QualType T = Res->getType();
 
   if (T->isDependentType()) {
 return Result;
@@ -657,16 +657,26 @@
   }
 
   // Otherwise, it needs to be a complete type.
-  if (RequireCompleteExprType(Result.get(), diag::err_asm_incomplete_type)) {
+  if (RequireCompleteExprType(Res, diag::err_asm_incomplete_type)) {
 return ExprError();
   }
 
   fillInlineAsmTypeInfo(Context, T, Info);
 
   // We can work with the expression as long as it's not an r-value.
-  if (!Result.get()->isRValue())
-Info.IsVarDecl = true;
+  if (!Res->isRValue()) {
+Info.setKindVariable();
+return Result;
+  }
 
+  Expr::EvalResult EvlResult;
+  // Try to evaluate the identifier as enum constant, currently we do not allow
+  // other constant integers to be folded.
+  if (isa(T) &&
+Res->EvaluateAsRValue(EvlResult, getASTContext())) {
+Info.ConstIntValue = EvlResult.Val.getInt();
+Info.setKindConstEnum();
+  }
   return Result;
 }
 
@@ -773,7 +783,7 @@
   fillInlineAsmTypeInfo(Context, Result.get()->getType(), Info);
 
   // Fields are "variables" as far as inline assembly is concerned.
-  Info.IsVarDecl = true;
+  Info.setKindVariable();
 
   return Result;
 }
Index: test/CodeGen/ms-inline-asm.c
===
--- test/CodeGen/ms-inline-asm.c
+++ test/CodeGen/ms-inline-asm.c
@@ -42,7 +42,7 @@
 void t6(void) {
   __asm int 0x2c
 // CHECK: t6
-// CHECK: call void asm sideeffect inteldialect "int $$0x2c", "~{dirflag},~{fpsr},~{flags}"()
+// CHECK: call void asm sideeffect inteldialect "int $$44", "~{dirflag},~{fpsr},~{flags}"()
 }
 
 void t7() {
@@ -61,7 +61,7 @@
 mov eax, ebx
   }
 // CHECK: t7
-// CHECK: call void asm sideeffect inteldialect "int $$0x2cU", "~{dirflag},~{fpsr},~{flags}"()
+// CHECK: call void asm sideeffect inteldialect "int $$44", "~{dirflag},~{fpsr},~{flags}"()
 // CHECK: call void asm sideeffect inteldialect "", "~{dirflag},~{fpsr},~{flags}"()
 // CHECK: call void asm sideeffect inteldialect "mov eax, ebx", "~{eax},~{dirflag},~{fpsr},~{flags}"()
 }
@@ -94,7 +94,7 @@
 // CHECK: t9
 // CHECK: call void asm sideeffect inteldialect
 // CHECK-SAME: push ebx
-// CHECK-SAME: mov ebx, $$0x07
+// CHECK-SAME: mov ebx, $$7
 // CHECK-SAME: pop ebx
 // CHECK-SAME: "~{ebx},~{esp},~{dirflag},~{fpsr},~{flags}"()
 }
@@ -265,7 +265,7 @@
 // CHECK: t21
 // CHECK: call void asm sideeffect inteldialect
 // CHECK-SAME: push ebx
-// CHECK-SAME: mov ebx, $$07H
+// CHECK-SAME: mov ebx, $$7
 // CHECK-SAME: pop ebx
 // CHECK-SAME: "~{ebx},~{esp},~{dirflag},~{fpsr},~{flags}"()
 }
@@ -312,13 +312,13 @@
 void t25() {
 // CHECK: t25
   __asm mov eax, 0h
-// CHECK: mov eax, $$0h
+// CHECK: mov eax, $$4294967295
   __asm mov eax, 0fhU
 // CHECK: mov eax, $$15
   __asm mov eax, 0a2h
-// CHECK: mov eax, $$0a2h
+// CHECK: mov eax, $$162
   __asm mov eax, 10100010b
-// CHECK: mov eax, $$10100010b
+// CHECK: mov eax, $$162
   __asm mov eax, 10100010BU
 // CHECK: mov eax, $$162
 // CHECK: "~{eax},~{dirflag},~{fpsr},~{flags}"()
Index: test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
===
--- test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
+++ test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
@@ -0,0 +1,60 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang_cc1 %s -fasm-blocks -emit-llvm -o - | FileCHECK %s
+namespace x {
+enum { A = 12 };
+struct y_t {
+	enum { A = 17 };
+	int r;
+} y;
+}
+// CHECK-LABEL: x86_enum_only
+void x86_enum_only(){
+  const int a = 0;
+  // CHECK-NOT: mov eax, [$$0]
+  // Other constant type folding is currently unwanted.
+  __asm mov eax, [a]
+  }
+
+// CHECK-LABEL: x86_enum_namespaces
+void x86_enum_namespaces() {
+  enum { A = 1 };
+  // CHECK: call void asm
+  // CHECK-SAME: mov eax, $$12
+  __asm mov eax, x::A
+  // CHECK-SAME: mov eax, $$17
+  __asm mov eax, x::y_t::A
+  // CHECK-NEXT: call void asm
+  // CHECK-SAME: mov eax, $$1
+  __asm {mov eax, A}
+}
+
+// CHECK-LABEL: x86_enum_arithmethic
+void x86_enum_arithmethic() {
+  enum { A = 1, B };
+  // CHECK: call void asm
+  // CHECK-SAME: mov eax, $$21

[PATCH] D34449: [clang-tidy] Enable inline variable definitions in headers.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL306538: [clang-tidy] Enable inline variable definitions in 
headers (authored by xazax).

Changed prior to commit:
  https://reviews.llvm.org/D34449?vs=104379=104389#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D34449

Files:
  clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
  clang-tools-extra/trunk/docs/clang-tidy/checks/misc-definitions-in-headers.rst
  clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers-1z.hpp
  clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp


Index: clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
@@ -139,6 +139,9 @@
 // Ignore variable definition within function scope.
 if (VD->hasLocalStorage() || VD->isStaticLocal())
   return;
+// Ignore inline variables.
+if (VD->isInline())
+  return;
 
 diag(VD->getLocation(),
  "variable %0 defined in a header file; "
Index: 
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-definitions-in-headers.rst
===
--- 
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-definitions-in-headers.rst
+++ 
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-definitions-in-headers.rst
@@ -74,6 +74,14 @@
template 
void B::f1() {}
 
+   class CE {
+ constexpr static int i = 5; // OK: inline variable definition.
+   };
+
+   inline int i = 5; // OK: inline variable definition.
+
+   constexpr int k = 1; // OK: constexpr variable has internal linkage.
+
 Options
 ---
 
Index: clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp
===
--- clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp
+++ clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s misc-definitions-in-headers %t
+// RUN: %check_clang_tidy %s misc-definitions-in-headers %t -- -- -std=c++11
 
 int f() {
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f' defined in a header 
file; function definitions in header files can lead to ODR violations 
[misc-definitions-in-headers]
@@ -175,3 +175,5 @@
 int CD::f() { // OK: partial template specialization.
   return 0;
 }
+
+constexpr int k = 1; // OK: constexpr variable has internal linkage.
Index: 
clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers-1z.hpp
===
--- clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers-1z.hpp
+++ clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers-1z.hpp
@@ -0,0 +1,10 @@
+// RUN: %check_clang_tidy %s misc-definitions-in-headers %t -- -- -std=c++1z
+
+class CE {
+  constexpr static int i = 5; // OK: inline variable definition.
+};
+
+inline int i = 5; // OK: inline variable definition.
+
+int b = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'b' defined in a header 
file; variable definitions in header files can lead to ODR violations 
[misc-definitions-in-headers]


Index: clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
@@ -139,6 +139,9 @@
 // Ignore variable definition within function scope.
 if (VD->hasLocalStorage() || VD->isStaticLocal())
   return;
+// Ignore inline variables.
+if (VD->isInline())
+  return;
 
 diag(VD->getLocation(),
  "variable %0 defined in a header file; "
Index: clang-tools-extra/trunk/docs/clang-tidy/checks/misc-definitions-in-headers.rst
===
--- clang-tools-extra/trunk/docs/clang-tidy/checks/misc-definitions-in-headers.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/misc-definitions-in-headers.rst
@@ -74,6 +74,14 @@
template 
void B::f1() {}
 
+   class CE {
+ constexpr static int i = 5; // OK: inline variable definition.
+   };
+
+   inline int i = 5; // OK: inline variable definition.
+
+   constexpr int k = 1; // OK: constexpr variable has internal linkage.
+
 Options
 ---
 
Index: clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp
===
--- clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp
+++ clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s 

[clang-tools-extra] r306538 - [clang-tidy] Enable inline variable definitions in headers

[Gabor Horvath via cfe-commits]

Author: xazax
Date: Wed Jun 28 05:47:35 2017
New Revision: 306538

URL: http://llvm.org/viewvc/llvm-project?rev=306538=rev
Log:
[clang-tidy] Enable inline variable definitions in headers

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

Added:
clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers-1z.hpp
Modified:
clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp

clang-tools-extra/trunk/docs/clang-tidy/checks/misc-definitions-in-headers.rst
clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp

Modified: clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp?rev=306538=306537=306538=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp Wed 
Jun 28 05:47:35 2017
@@ -139,6 +139,9 @@ void DefinitionsInHeadersCheck::check(co
 // Ignore variable definition within function scope.
 if (VD->hasLocalStorage() || VD->isStaticLocal())
   return;
+// Ignore inline variables.
+if (VD->isInline())
+  return;
 
 diag(VD->getLocation(),
  "variable %0 defined in a header file; "

Modified: 
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-definitions-in-headers.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/misc-definitions-in-headers.rst?rev=306538=306537=306538=diff
==
--- 
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-definitions-in-headers.rst 
(original)
+++ 
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-definitions-in-headers.rst 
Wed Jun 28 05:47:35 2017
@@ -74,6 +74,14 @@ from multiple translation units.
template 
void B::f1() {}
 
+   class CE {
+ constexpr static int i = 5; // OK: inline variable definition.
+   };
+
+   inline int i = 5; // OK: inline variable definition.
+
+   constexpr int k = 1; // OK: constexpr variable has internal linkage.
+
 Options
 ---
 

Added: 
clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers-1z.hpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers-1z.hpp?rev=306538=auto
==
--- clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers-1z.hpp 
(added)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers-1z.hpp 
Wed Jun 28 05:47:35 2017
@@ -0,0 +1,10 @@
+// RUN: %check_clang_tidy %s misc-definitions-in-headers %t -- -- -std=c++1z
+
+class CE {
+  constexpr static int i = 5; // OK: inline variable definition.
+};
+
+inline int i = 5; // OK: inline variable definition.
+
+int b = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'b' defined in a header 
file; variable definitions in header files can lead to ODR violations 
[misc-definitions-in-headers]

Modified: 
clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp?rev=306538=306537=306538=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp Wed 
Jun 28 05:47:35 2017
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s misc-definitions-in-headers %t
+// RUN: %check_clang_tidy %s misc-definitions-in-headers %t -- -- -std=c++11
 
 int f() {
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f' defined in a header 
file; function definitions in header files can lead to ODR violations 
[misc-definitions-in-headers]
@@ -175,3 +175,5 @@ template 
 int CD::f() { // OK: partial template specialization.
   return 0;
 }
+
+constexpr int k = 1; // OK: constexpr variable has internal linkage.


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

[PATCH] D34696: [refactor] Move the core of clang-rename to lib/Tooling/Refactoring

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

The main thing I'm concerned about is having the main code in core, but having 
all tests in tools-extra. I think if we go that route we should also move 
clang-rename and its tests to core. Thoughts?


Repository:
  rL LLVM

https://reviews.llvm.org/D34696



___
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

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

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

> In https://reviews.llvm.org/D33644#793594, @klimek wrote:
>
> > In https://reviews.llvm.org/D33644#793577, @yvvan wrote:
> >
> > > In https://reviews.llvm.org/D33644#793573, @klimek wrote:
> > >
> > > > In https://reviews.llvm.org/D33644#783903, @yvvan wrote:
> > > >
> > > > > Do not evaluate numbers.
> > > > >  Check for != "=" is needed not to mess with invalid default 
> > > > > arguments or their types (without it I get "const Bar& bar = =" when 
> > > > > Bar is not defined)
> > > >
> > > >
> > > > Shouldn't we than instead check that error case?
> > >
> > >
> > > I don't know the proper way to do that :) I also don't know the full 
> > > amount of errors that might cause such behavior.
> > >  You can suggest the solution if you have some idea. Current one is safe 
> > > because we just ignore any case that causes empty default string.
> >
> >
> > Taking your example "const Bar& bar = =" when Bar is not defined:
> >  What happens now?  Will it be "const Bar& bar ="? I argue that is not 
> > better than "const Bar& bar = =".
> >  Btw, can you add tests? I think that will make it easier to see what's 
> > actually happening.
>
>
> It will be just "const Bar& bar" in that case with this patch applied. So it 
> is better than both "const Bar& bar =" and "const Bar& bar = =" :)
>  About tests - I agree that it's nice to have them. Can I find something 
> similar in the current test set (as an example) and where is it better to put 
> my tests?


All of test/Index/complete-* basically test this, I think. Try to find a good 
spot or write a new test.


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

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added a comment.

In https://reviews.llvm.org/D33644#793594, @klimek wrote:

> In https://reviews.llvm.org/D33644#793577, @yvvan wrote:
>
> > In https://reviews.llvm.org/D33644#793573, @klimek wrote:
> >
> > > In https://reviews.llvm.org/D33644#783903, @yvvan wrote:
> > >
> > > > Do not evaluate numbers.
> > > >  Check for != "=" is needed not to mess with invalid default arguments 
> > > > or their types (without it I get "const Bar& bar = =" when Bar is not 
> > > > defined)
> > >
> > >
> > > Shouldn't we than instead check that error case?
> >
> >
> > I don't know the proper way to do that :) I also don't know the full amount 
> > of errors that might cause such behavior.
> >  You can suggest the solution if you have some idea. Current one is safe 
> > because we just ignore any case that causes empty default string.
>
>
> Taking your example "const Bar& bar = =" when Bar is not defined:
>  What happens now?  Will it be "const Bar& bar ="? I argue that is not better 
> than "const Bar& bar = =".
>  Btw, can you add tests? I think that will make it easier to see what's 
> actually happening.


It will be just "const Bar& bar" in that case with this patch applied. So it is 
better than both "const Bar& bar =" and "const Bar& bar = =" :)
About tests - I agree that it's nice to have them. Can I find something similar 
in the current test set (as an example) and where is it better to put my tests?


https://reviews.llvm.org/D33644



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